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

add/set approvedScopes in User #572

Merged
merged 5 commits into from Jan 27, 2022

Conversation

lsnider
Copy link
Contributor

@lsnider lsnider commented Jan 18, 2022

It's possible for a user to grant a subset of the requested scopes. Google has been pushing this recently...see the attached screenshot of their latest Oauth page. Their latest default is to have no scopes checked making it quite easy for a user to select a subset of the requested scopes.

Part of the Oauth flow suggests the providers should return the approved scopes as part of the code -> access_token exchange if the user auth'ed a subset of the requested scopes. In my testing of Google, M365, Slack, and Zoom, the scope field is always returned as part of the code -> access_token exchange.

There was a recent issue asking for this feature.

As pointed out in the link within issue#568, we should expect code -> access_token, refresh_token (optional), expires_at, scope (optional)

There was a recent PR which brought the User class up from just access_token to also include refresh_token, and expires_at.

This PR also brings in scope to the User object. That would complete the User object for storing everything the code -> access_token exchange returns.

Would you like to merge this up stream? If so, I can do whatever I can to make this acceptable up steam. I tested this with the providers we are using. I can also put in a PR for the SocialiteProviders manager. In SocialiteProviders, it's much like this PR but we set the scopes here and add a parseApprovedScopes function like the others in that file.

Tested with native Oauth Provider:

  • Google Personal
  • Google Workspace/Gsuite

Tested with SocialiteProviders (needs a simple PR):

  • M365
  • Slack (we have a fork of the Slack Provider)

Google Oauth Screen

If it's helpful, here are a few documents showing the scope field gets returned from a auth code to access token exchange

@lsnider lsnider changed the title add/set approvedScopes in User WIP: add/set approvedScopes in User Jan 18, 2022
Lindsay Snider added 2 commits January 18, 2022 16:38
@lsnider lsnider marked this pull request as draft January 18, 2022 21:43
@lsnider lsnider marked this pull request as ready for review January 18, 2022 22:52
@lsnider lsnider changed the title WIP: add/set approvedScopes in User add/set approvedScopes in User Jan 18, 2022
@taylorotwell
Copy link
Member

I don't seem to have permission to edit this PR?

@taylorotwell taylorotwell marked this pull request as draft January 19, 2022 15:23
@lsnider
Copy link
Contributor Author

lsnider commented Jan 19, 2022

@taylorotwell Were you just having trouble marking it as a draft? I don't see any permissions controls on my side regarding the PR. Did add you as a writer to this forked repo if you were looking to push to the branch.

@lsnider
Copy link
Contributor Author

lsnider commented Jan 26, 2022

If this PR gets merged, here is the corresponding PR for SocialliteProviders

@taylorotwell taylorotwell merged commit 2885515 into laravel:5.x Jan 27, 2022
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.

None yet

2 participants