-
Notifications
You must be signed in to change notification settings - Fork 173
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
Add support for Client Credentials Grant Type to OAuth2 authentication #944
base: 7.x-2.x
Are you sure you want to change the base?
Conversation
OAuth2 support requires that only tokens associated with users may be used with authenticated resources. However, OAuth2 also support a client credentials grant type which does not require any user interaction. This is typically used directly by applications to access such API. This does not work with RESTful as once a resource is marked as authenticated, there has to be an associated user. This change allows plugin developers to specify a user which will be used when a client credentials token is used. The user may be specified with the uid or name. The uid has the higher priority.
Currently, we use two pieces of information for OAuth2 authentication - server and scope. We define them as separate items in the plugin definition. There might be more items required in the future (one of which could be support for client credentials in RESTful-Drupal#944). It might be cleaner to change all the definitions to an array.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This implementation is getting complex. That is fine because this is a complex integration.
However we missed in the last iteration one of the key factors of RESTful, which is providing an Example on how to use all these features.
This PR is blocked by #945. Once that one is merged this one should be adapted to use the same structure.
THANKS for the great work. Sorry it took so long, but I am currently spread too thin across multiple contribs and core.
🙌
$result['user_id'] = $oauth2_info['client_credentials_uid']; | ||
} | ||
elseif (!empty($oauth2_info['client_credentials_user'])) { | ||
$result['user_id'] = user_load_by_name($oauth2_info['client_credentials_user'])->uid; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are loading the user by name and then by id. We should load the user only once.
return [ | ||
'server' => $server, | ||
'scope' => $scope, | ||
'client_credentials_user' => $cc_user, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any impediment to make it camel case? If not, I'd rather camel case since it's consistent with the rest of the annotation keys.
OAuth2 support requires that only tokens associated with users may be used with authenticated resources. However, OAuth2 also support a client credentials grant type which
does not require any user interaction. This is typically used directly by applications to access such API. This does not work with RESTful as once a resource is marked as
authenticated, there has to be an associated user.
This change allows plugin developers to specify a user which will be used when a client credentials token is used. The user may be specified with the uid or name. The uid
has the higher priority.