PDO\Storage getDefaultScope never returns bool

[marcovanest]

Hi guys,

I'm playing with this great library and one of the things I try to accomplish is to enforce the scope parameter when requesting a token.

I scanned trough the code and saw that to enforce a scope parameter you need to insert a record into the oauth_scopes with the value (false, true) as can been seen in OAuth2/Controller/TokenController::grantAccessToken

The method PDO\Storage::getDefaultScope (in my case) is responsible for getting all the default scopes, however this method either returns a string (scopes separated by spaces) or null if there aren't any default scopes.

Because of this the if statement is never touched because of the typesafe comparison.

Am I missing something or is this a small bug?

[phindmarsh]

Yeah I noticed that too, managed to get around it because I had a Grant Type. I think it is a bug, the doc block says one thing but the code does another.

[bshaffer]

This is not a small bug. The library basically says "if you want YOUR application to require scope, have your ScopeStorage return false". But, the default behavior of this library is to say "no scope = all scope", ie if no scope is requested then all permission is granted.

If this is either unclear in the docs or not the desired behavior, I am totally OK with changing it. Perhaps requiring scope should be the default. Let me know what you think.

[marcovanest]

@bshaffer You're right, I looked it up in the specs section 3.3

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).

Changing the behavior to require a scope would be the best solution in a perfect world :), but this would break a lot of applications out there which aren't using a scope parameter and which are also using the default PDO\Storage / Library.

With the latter in mind the practical solution would be updating the docs to make this clear. Anyone who want to enforce the scope, should implement there own Storage or TokenController imho

[bshaffer]

It may be an even better solution to configure the controllers with a new parameter 'allow_null_scope' => (bool), rather than putting this logic in the scope storage.

[marcovanest]

That would indeed be another solution, in the mean time I've implemented a tokenController that extends the default tokenController and overrules the method grantAccessToken

[bshaffer]

What does your updated tokenController do exactly? You really shouldn't need to do that. At the most, you will need to have your ScopeStorage or your ScopeUtil return false instead of null