Set approvedScopes attribute #225
Conversation
| ->setExpiresIn($this->parseExpiresIn($response)); | ||
| ->setRefreshToken($this->parseRefreshToken($response)) | ||
| ->setExpiresIn($this->parseExpiresIn($response)) | ||
| ->setApprovedScopes(explode($this->scopeSeparator, Arr::get($response, 'scope', ''))); |
There was a problem hiding this comment.
Is the scope param always going to be a string?
Do all providers return this?
There was a problem hiding this comment.
https://datatracker.ietf.org/doc/html/rfc6749#section-3.3 proposed standard:
"scope" must be returned in response if the scopes granted don't match the scopes requested. Additional logic could be added to return requested scopes if "scope" is not set in response.
According to https://datatracker.ietf.org/doc/html/rfc6749#appendix-A.4 - "scope" should always be a string of this format: scope-token *( SP scope-token )
Explanation:
scope-token- at least one scope-token*( SP scope-token )- optional 0 to many space delimited scope-tokens
However, this RFC is only proposed, also "scope" is retrieved from json, decoded in vendor/laravel/socialite/src/Two/AbstractProvider.php:299, so there is a chance it won't be a string.
A more defensive approach should be probably be implemented. I'll look into it, maybe I'll open a PR to https://github.com/laravel/socialite/blob/5.x/src/Two/AbstractProvider.php#L261
There was a problem hiding this comment.
Happy to accept this PR, if you add a condition to check if it's a string first?
|
Closed in #229 |
Fixes #224
Changes proposed in this pull request: