Allow default scopes to be set regardless of grant type

I made a few changes to support the default scopes in all grant types. Remaining tasks:

• Check if we still need the default scope logic in the ScopeRepository::finalizeScopes.
• Add tests for all grant types.

I just want to note that this is not a bug but rather a feature request (though I'll leave changing the category to others if you agree).

As already shown in the issue summary:

If the client omits the scope parameter when requesting
authorization, the authorization server MUST either process the
request using a pre-defined default value or fail the request
indicating an invalid scope. The authorization server SHOULD
document its scope requirements and default value (if defined).

In the initial implementation we specifically chose to go for the "fail the request indicating an invalid scope" route.

In any change we make to this we should ensure that the module can be configured to disallow the use of default scopes and force clients to request some.

In one of our projects, we have a consumer factory and we need to assign scopes to consumers that have also the Authorization Code grant type. Client should not be aware of scopes when performing the request because scopes should be already contained in the consumer. This issue is allowing that (I think in earlier versions of the module this was possible as I saw in one of the YouTube tutorials).

Regarding the failing test, I'm not sure what is the expectancy.

Given this requirement...

If the client omits the scope parameter when requesting
authorization, the authorization server MUST either process the
request using a pre-defined default value or fail the request
indicating an invalid scope. The authorization server SHOULD
document its scope requirements and default value (if defined).

I think we should fail when at least one scope is not provided either by request, or as default scope set in the consumer. Or I'm missing something?

Of course, some tests are failing now. With client_credentials grant type, if the scopes were already configured in the consumer, it was not possible to pass an additional one in request. But authorization_code didn't had this constraint. If we apply the same policy to authorization_code I don't think it is a BC break, as, until now, it wasn't possible to store scopes inside consumer for this grant type.

This needs to get some feedback from the maintainers.

Limiting the allowed scopes only makes sense for client credentials. For more information, see: https://www.drupal.org/project/simple_oauth/issues/3495325

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.

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.

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

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.

Updating the IS after clarified the scope of this issue in #13.

More discussions took place on Slack, see https://drupal.slack.com/archives/C1BMUQ9U6/p1757918379890819

I have reviewed the patch and works fine. I do not like the redundant comments like "Log in the user" but I see that all tests are like that so it is fine to stay for consistency even if it is unumbered.

The only thing that bothers me is that the word "creds" had to be taken into CSpell ignore while it is only used in tests/src/Functional/AuthCodeFunctionalTest.php , in the name of a variable and also in a test entity's ID.

At the same time, as far as I understand, all the points raised by @bojan_dev are addressed. I give an RTBC in advance but let him decide about the next steps here.

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.

Looks good move to RTBC.

Thank you all for your contributions!

Nice to see this completed. Thanks to all involved