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.

Sign up for GitHub

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

Jump to bottom

Set approvedScopes attribute #225

Closed
wants to merge 1 commit into from
Closed

Set approvedScopes attribute #225

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions src/OAuth2/AbstractProvider.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,9 @@ public function user()
}

return $this->user->setToken($token)
->setRefreshToken($this->parseRefreshToken($response))
->setExpiresIn($this->parseExpiresIn($response));
->setRefreshToken($this->parseRefreshToken($response))
->setExpiresIn($this->parseExpiresIn($response))
->setApprovedScopes(explode($this->scopeSeparator, Arr::get($response, 'scope', '')));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the scope param always going to be a string?
Do all providers return this?

Copy link
Author

@omarcinkonis omarcinkonis Feb 5, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Happy to accept this PR, if you add a condition to check if it's a string first?

}

/**
Expand Down