SocialiteProviders / Providers

A Collection of Providers for Laravel Socialite
https://socialiteproviders.com
MIT License
488 stars 436 forks source link

Socialite 5.11 introduced conflicting refreshToken method #1125

Closed ian-nisbet closed 5 months ago

ian-nisbet commented 7 months ago

See https://github.com/laravel/socialite/issues/677

Declaration of SocialiteProviders\Apple\Provider::refreshToken(string $refreshToken): Psr\Http\Message\ResponseInterface must be compatible with Laravel\Socialite\Two\AbstractProvider::refreshToken($refreshToken)

atymic commented 7 months ago

Please PR an update :)

hamrak commented 7 months ago

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?

johanehnberg commented 7 months ago

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.

atymic commented 7 months ago

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.

atymic commented 7 months ago

Is there any other providers with this issue? Apple is resolved now.

johanehnberg commented 7 months ago

At least Okta as per the linked issue from socialite proper.

atymic commented 5 months ago

I believe all of these are resolved now.