Problem/Motivation
When the simple_oauth module is installed, authenticated users with valid session cookies are incorrectly treated as anonymous on certain requests, particularly those with role-based access restrictions (e.g., Page
Manager pages with user role conditions).
The issue occurs because `SimpleOauthAuthenticationProvider::applies()` performs route lookups (via `pathValidator->getUrlIfValidWithoutAccessCheck()`) **before** checking if the request contains an OAuth2 Bearer token.
Since simple_oauth has authentication provider priority 35 (higher than cookie authentication's priority 0), these route lookups happen before cookie authentication can resolve the user session, interfering with the
authentication flow.
Proposed resolution
Reorder the logic in `SimpleOauthAuthenticationProvider::applies()` to:
1. Check for OAuth2 Bearer token FIRST
2. Return FALSE immediately if no Bearer token is present
3. Only perform route lookups if the request IS an OAuth2 request
Issue fork simple_oauth-3553759
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
alorencComment #4
alorencComment #5
alorencComment #6
alorenc## Performance impact
**Before fix:** Route lookups (`pathValidator->getUrlIfValidWithoutAccessCheck()` + `routeProvider->getRouteByName()`) occur on EVERY request, regardless of whether OAuth is being used.
**After fix:** Route lookups only occur when a Bearer token is present (typically <1% of requests on most sites). For non-OAuth requests, the method returns FALSE immediately after a simple header check.
Comment #7
alorencComment #8
claudiu.cristeaI can reproduce the issue with some page_manager routes. After installing simple_oauth, suddenly, the routes created by page_manager, normally accessible by authenticated user, are returning 403. I see this as Critical as breaks an existing site.
Comment #9
claudiu.cristeaThank you for the fix. I have tested manually the MR and works as expect. However, given the impact of this bug, I'm sure it deserves a regression test.
On MR I've also proposed some minor improvements to documentation.
Comment #11
bojan_dev commentedI have addressed the feedback, please review.
Comment #12
claudiu.cristeaI'm going to review the changes
Comment #13
claudiu.cristeaWe need a test that replicates the bug when the fix is not applied
Comment #14
bojan_dev commentedPicked up the feedback, please review again.
Comment #15
alorencComment #16
claudiu.cristeaThis looks good, thank you all
@bojan_dev, if merged, could you, please, cut a new release?
Comment #18
bojan_dev commentedI just released 6.0.5 with the fix.
Nice work all!