🇳🇱Netherlands @bojan_dev

In your notes, I see that you selected the following grant types: Refresh Token, Client Credentials, Mobile, and Password.
Please note that the Mobile and Password grant types are not supported in simple_oauth. Are you using custom plugins or perhaps the simple_oauth_password_grant module?

This has been a long-standing discussion with multiple MRs, so it’s hard to determine credit fairly. I’ve updated the credits and tried my best to be fair. If you believe you should be credited, please don’t hesitate to reach out.

Postponing for now until 📌 Deprecate RoleStorage::isPermissionInRoles() Active lands.

This seems related to: 🐛 PathValidator won't work on POST-only requests Needs review .

Based on the available info I can't determine whats wrong. I would need more info:

• Did you run database updates?
• From which version did you upgrade simple_oauth, or is it a fresh install?
• Which Drupal version are you using?
• Are there any PHP errors in the DB log (/admin/reports/dblog)?
• Please add screenshots from consumer, scopes and settings.

And enabled associated grant types on the dynamic scopes?

We went with MR138. If you need token invalidation to be triggered under different conditions, you can replace the TokenExpiryTriggerHandler service.

Did you create any scopes (dynamic/static)? Please check on /admin/config/people/simple_oauth/oauth2_scope.

Nice catch e0ipso, I think option 1 is fine. We can move forward on the remaining tasks.

Thank you for reporting and solving the issue @pfrenssen.

The code has become more static and strict. For now, you can add an oauth2_scope_reference field to the consumer entity specifically for your custom grant type, similar to how authorization_code_scopes is handled. Then, you can map this field to SCOPE_FIELD, as you mentioned.

To make this more dynamic, we could introduce an optional method in the grant type plugin that defines the default scopes field on the consumer for each grant type. In the ScopeRepository, we could then rely on the plugin (grant type) IDs for this mapping.

Nice catch @tylermc, merged and tagged in 6.0.8.

bojan_dev → made their first commit to this issue’s fork.

greggles → credited bojan_dev → .

Thank you all for your contributions!

Looks good to me!

I simplified the loadByGrantType() implementation since the code felt a bit overcomplicated. We only need to load scopes for one grant type anyway, so this should be the final step. If someone could take a look at my last commit, we can go ahead and merge it.

in MR !35 the langcode property has been moved from entity object to the root of the response. Is there a reason for this? It breaks currently next-drupal, see: https://github.com/chapter-three/next-drupal/blob/main/packages/next-dru....

The issue is outdated and will be closed for now. Please reopen it if more information becomes available.

I just released 6.0.5 with the fix.

Nice work all!

Picked up the feedback, please review again.

I have addressed the feedback, please review.

bojan_dev → made their first commit to this issue’s fork.

It appears there are cases where a route is not available, but OAuth2 still needs to be applied (see 🐛 REST routes broken again Active and 🐛 REST routes crashing, claiming unsupported format Active ).
When comparing this with Basic Auth, the authenticate method returns NULL when the credentials are incorrect. However, in the simple_oauth case, it throws exceptions in different cases, which is relevant for interpretation by the requester. Marking this as won’t fix for now.

My bad, I have reverted 🐛 404 error not output correctly. Active and made a new release tag.

The issue is outdated. If anyone has experience with Moodle or have any input, feel free to reopen it.

The issue is outdated. If this issue still persists, feel free to reopen it.

Released in 6.0.2

You can take a look at https://www.drupal.org/project/simple_oauth/issues/2946882 📌 Auth revoke on profile update Needs work . There are two solutions available (not merged yet), both of which provide the ability to alter the behavior you are describing.

The matchRequest was breaking test coverage for the client credentials by returning the wrong status code. It seems to be performing some access checks. I reverted to the getUrlIfValidWithoutAccessCheck() approach but added a try-catch block around it. This should resolve the issue. Can someone please verify?

I had to create a custom RestResource plugin to be able to reproduce this.

I’ve made an MR available based on the patch in #6, with a try-catch block to mitigate the issue reported in #11.

The associated test coverage still needs to be updated; once that’s done, we can merge this.

I can't reproduce this. Am I missing something?

• Install simple_oauth:6.0.1, rest, and restui modules.
• Configure a consumer with the client_credentials grant type.
• Configure a REST content resource with format set to json and authentication set to oauth2.
• Send a request via Postman with OAuth2 authentication to /node/<id>?_format=json.
• Receive a 200 response.

This means default scopes will be merged with scopes passed to request in order to get the final request scopes?

I wouldn’t merge it; this way, authorization can be requested with fewer scopes than the default if needed.

Limiting the allowed scopes only makes sense for client credentials. For more information, see: https://www.drupal.org/project/simple_oauth/issues/3495325 🐛 No way to limit allowed scopes for a consumer using client credentialss Active

Client credentials can both limit allowed scopes and provide default scopes. Because of this, we can’t make the existing scope field generic, since a consumer may have multiple grant types enabled with different scope policies. Therefore, we need to introduce a dedicated scopes field on the consumer for the authorization code.

Regarding failing the authorization request when at least one scope is not provided, either by the request or by the default scope set on the consumer, this is a valid approach for the authorize controller.

Did you try clearing the cache? I can’t reproduce this. Also, the tests should fail if this were true, but they are not.

@claudiu.cristea valid point, release 6.0.1 is now available!

@arlanschouwstra That sounds like you are using the client_credentials grant type, which is the expected behavior in simple_oauth:6.0. This allows us to distinguish access tokens issued via the authorization code flow from those issued via the client credentials grant type.

Removing all entities and configs related to simple_oauth and then upgrading could be a potential workaround. I understand it’s not ideal, so if anyone can provide more information about how their existing consumers, scopes, and settings are configured, what errors pop up during the upgrade, and what is shown on the status report page, it would help me reproduce the issue.

It’s indeed a massive change, which is why I need a review from other developers before merging my MR.
So please, review away!

bojan_dev → created an issue.

Oke check, then just replace the assertion to the interface. Can you please provide a MR for this?

TY for the patch.

bojan_dev → made their first commit to this issue’s fork.

Is this a custom decorator? If it's the decorator of the RoleStorage, I would assume that it implements the RoleStorageInterface , it would then suffice todo the assertion base on the interface instead.

@justafish thank you for reporting and the MR.

Alright in that case, if the community can do the heavy lifting, I'm willing to review/merge D11 support for 5.2. Please make a MR available.

Unfortunately it will be more work to maintain two versions with different architectures, also the other maintainers are no longer active. I rather not introduces any new features to 5.2.x or add D11 support, which would mean support will be extended.

Perhaps you can explain your issue with Next.js, maybe I can help.

Thank you @dunjincan for reporting and providing a patch, I have updated the associated unit tests, because there were breaking due to this change.

bojan_dev → made their first commit to this issue’s fork.

Thanks all for the work on this! @bojan_dev Any preference for type of test/where? I believe just test coverage that saving AccountInterface or ConsumerInterface triggers the token deletion right, but the idea that a service can be overridden would not need testing, is that your expectation?

There is a new service introduced TokenExpiryTriggerHandler, all the methods need to have test coverage, checking if they are triggered on specific events and have the right parameters. For example: handleConsumerUpdate() is triggered on consumer update and provides the associated consumer object.

Thank you coffeemakr for reporting and and making a fork available. I have compared the logic with dynamic scopes and I can confirm that this is inconsistent behaviour with static scopes. I have updated fork to still leverage the getInstances method, due the fact it has logic when not providing ids. Could you please make a MR available next time, it was not clear for me if there was a fix available.

bojan_dev → made their first commit to this issue’s fork.

I tested how Basic Auth does this, and a non existing URL will get 404 even if the credentials are incorrect, so I think it's a valid point. Please review the MR !185, it does not apply OAuth2 anymore if the URL is not valid.

bojan_dev → made their first commit to this issue’s fork.

Could you please make a MR or patch?

Can someone please review MR !172?

The feature request makes sense, also the MR looks good to me, nice work tstoeckler!

bojan_dev → made their first commit to this issue’s fork.

I agree with coffeemkr, this is the original issue so lets keep it, also the MR in the cloned issue has more phpcs issues.

TY for contributing.

You could use the /oauth/debug endpoint to retrieve the roles from the authenticated user based on the token.

Patch from coffeemakr looks good, the MR has many file changes also the MR description is from a different issue. I think your fork is outdated.

Created issue on solarium repo: https://github.com/solariumphp/solarium/issues/1156

Looks good to me, TY!

Hi Taras,

Nice work! Looks good to me, the tests that fail are indeed unrelated.

Perhaps we should remove the simple_oauth_update_8604, like I said before it's a migration from old scopes (roles) to the new scope entity, which perhaps doesn't cover all different cases. A manual migration/configuration is the safest way to upgrade from 5.2 to 6.0.

Hi Bram,

Thanks for the clear explanation of the feature request.

I agree that this would be a valuable feature for the community, also the suggested opt-out mechanism would not introduce any BC and its implementation is pretty straightforward.

So I would say go for it and I would be happy to review any contribution on this feature.

Oke this is my take on this long discussion:
The goal of this issue is to have the ability to revoke tokens in specific cases. The only 2 options that I see that could potentially land to resolve the issue are MR138 and MR164. Both MR’s are comparable; the only difference is that 164 introduces a config layer which makes it easier for the community to configure the obvious cases mentioned in this issue (without having to write code) and optionally can of course enrich it by themselves. I don’t see the maintenance burden as mentioned, as it’s not a complex implementation.

I'm pretty much up for both MR’s, the only thing that is holding me back to merge MR138: it’s missing test coverage.

I find it not clear how to reproduce your issue, so it's hard for me to figure this one out. The hook_update: simple_oauth_update_8604 tries to migrate existing (old) scope settings from your consumers to the new dynamic scopes, so if this didn't work you could always just manually configure your dynamic scopes and consumers.

The password grant is no longer supported in simple_oauth 6.0, due the following reasons: https://datatracker.ietf.org/doc/html/draft-ietf-oauth-security-topics#s...

But you could checkout this module: https://www.drupal.org/project/simple_oauth_password_grant →

@yobottehg could you please check if the fix in MR 177 resolves the issue?

I think we should add some extra checks in that hook:

• If role is not NULL.
• If scope already exists.
• If scopes are already referenced on the associated consumer.

This way we can rerun the hook_update.

Could you please check on which line the id() is giving null?

@grasmash did you run drush updb? If the config setting invalidate_tokens is giving NULL back, it probably did not run the simple_oauth_update_10001 hook_update.

We could filter those unchecked values (on submit), to make the array cleaner when retrieving values from the invalidate_tokens.

It appears the following change in thephpleague/oauth2-server is the reason for the BC: https://github.com/thephpleague/oauth2-server/pull/1094.

finalizeScopes is now being called in the RefreshTokenGrant, which does not set the $userIdentifier, this means that no scopes are being returned. This has impact on all grant types that support refresh tokens. Sorry that I missed this, also I have read your comments @m.stenta on 🌱 Move to thephpleague/oauth2-server 9.0 Active , those are valid points I will definitely make a major version release next time.

Merging MR !176 and will roll out a new stable release.

I was assuming you were only using the grant types that are provided by simple_oauth, based on that assumption that would mean that only the Authorization code grant type supports refresh tokens. But I see you are using the Password grant, which is not supported by simple_oauth: 6.0 due to the following reason: https://datatracker.ietf.org/doc/html/draft-ietf-oauth-security-topics#s...

You could take a look in 5.2 which still supports the password grant.

@skyejohnson no, the community can review it as well, so if you have the time please take a look.

@drunken monkey thank you for you input and you have some valid points, I could just create a custom plugin extend the "search_api_solr_suggester" and make those adjustment like you said. But trimming by default spaces in the "search_api_solr_suggester" plugin feels to me like the query is specific for some Solr lookup implementations, vs just allowing spaces and having the query to the Solr suggester raw as possible.

If you still find this an edge case, we can close both issue's and I will do a project specific implementation for it.

bojan_dev → created an issue.

bojan_dev → created an issue.

Do you mean it's missing from /oauth/userinfo route/endpoint?
Did you try to update to beta10?

Take a look on the following KernelTest, which illustrates how to retrieve user info from a token: https://git.drupalcode.org/project/simple_oauth/-/blob/6.0.0-beta10/test...

Closing issue because simple_oauth 8.x-4.x is EOL.