-
Notifications
You must be signed in to change notification settings - Fork 453
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
Socialite 5.11 introduced conflicting refreshToken method #1125
Comments
Please PR an update :) |
I think, that we can remove the definition from SocialiteProviders\Apple\Provider: /**
* Acquire a new access token using the refresh token.
*
* Refer to the documentation for the response structure (the `refresh_token` will be missing from the new response).
*
* @see https://developer.apple.com/documentation/sign_in_with_apple/tokenresponse
*
* @param string $refreshToken
* @return ResponseInterface
*
* @throws \GuzzleHttp\Exception\GuzzleException
*/
public function refreshToken(string $refreshToken): ResponseInterface
{
return $this->getHttpClient()->post($this->getTokenUrl(), [
RequestOptions::FORM_PARAMS => [
'client_id' => $this->clientId,
'client_secret' => $this->clientSecret,
'grant_type' => 'refresh_token',
'refresh_token' => $refreshToken,
],
]);
} because the parent class Laravel\Socialite\Two\AbstractProvider have: /**
* Refresh a user's access token with a refresh token.
*
* @param string $refreshToken
* @return \Laravel\Socialite\Two\Token
*/
public function refreshToken($refreshToken)
{
$response = $this->getRefreshTokenResponse($refreshToken);
return new Token(
Arr::get($response, 'access_token'),
Arr::get($response, 'refresh_token'),
Arr::get($response, 'expires_in'),
explode($this->scopeSeparator, Arr::get($response, 'scope', ''))
);
}
/**
* Get the refresh token response for the given refresh token.
*
* @param string $refreshToken
* @return array
*/
protected function getRefreshTokenResponse($refreshToken)
{
return json_decode($this->getHttpClient()->post($this->getTokenUrl(), [
RequestOptions::HEADERS => ['Accept' => 'application/json'],
RequestOptions::FORM_PARAMS => [
'grant_type' => 'refresh_token',
'refresh_token' => $refreshToken,
'client_id' => $this->clientId,
'client_secret' => $this->clientSecret,
],
])->getBody(), true);
} What do you think? |
Seems to affect many providers. Since socialite updates can break compatibility with providers, one sustainable approach would be to cap the dependency to versions known to work. |
It's somewhat hard as people use the main providers from socialite proper, which need updates. And this wasn't a major version bump from their end either. I have tagged a new apple version, please PR other providers. |
Is there any other providers with this issue? Apple is resolved now. |
At least Okta as per the linked issue from socialite proper. |
I believe all of these are resolved now. |
See laravel/socialite#677
Declaration of SocialiteProviders\Apple\Provider::refreshToken(string $refreshToken): Psr\Http\Message\ResponseInterface must be compatible with Laravel\Socialite\Two\AbstractProvider::refreshToken($refreshToken)
The text was updated successfully, but these errors were encountered: