Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[7.0] Allow '*' scope to be used with Client Credentials #949

Merged

Conversation

philcross
Copy link
Contributor

As mentioned in #516, using the * character to request all scopes for the Client Credentials grant type is valid.

This PR will allow the character to be used when requesting an access token with the Client Credentials Grant.

@billriess
Copy link
Contributor

Isn't * sort of implied by the nature of how client_credentials work in the current version of passport? Any client can request any scope and be given it automatically. I guess this just furthers that some but there is no back-end check to validate the client and it's allowed scopes without adding your own logic.

@philcross
Copy link
Contributor Author

As far as I can tell from the documentation, providing 'scopes' => ['*'] is meant to allow the access token all registered scopes.

For other grant types, like password grant, it's correctly stored in the oauth_access_tokens table as ["*"], but if you try to pass * in when using client credentials, it's stored as [] and therefore no scopes are given to the access token.

There's nothing in the documentation that I can see that indicates client credentials shouldn't be allowed *, and there was another test (https://github.com/laravel/passport/blob/7.0/tests/CheckClientCredentialsForAnyScopeTest.php#L26) which gave me the impression * is valid for CC (along with apparently * being a valid scope character in oauth specs (mentioned in #516))

@billriess
Copy link
Contributor

billriess commented Jan 28, 2019

@philcross Under https://laravel.com/docs/5.7/passport#password-grant-tokens "Requesting All Scopes" it does say This scope may only be assigned to a token that is issued using the password grant:

@philcross
Copy link
Contributor Author

Ha, I completely missed that. I'm not sure why it's restricted though, especially not to Client credentials - I'll leave the PR as it is, if it's refused then fair enough, but feels a bit weird to have to manually specify all scopes for client credentials, which I would assume is a more trusted grant type than Password Grant

@billriess
Copy link
Contributor

I agree it should be allowed on this scope and I think the PR is good. I just think there are some fundamental issues with Passport in general that should also be addressed. client_credentials should have a whitelist of scopes. client_id should not be sequential... and so on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants