Repository: thephpleague/oauth2-client (php)

Issue:
2.8.0 breaks exiting scope handling
Last week's release introduced the following in the `getAccessToken()` function:

https://github.com/thephpleague/oauth2-client/pull/1030/files#diff-e8490e4bb8acb102745699d2bd7aa0a298d836c92d00d2ed57dd4c7ad8b24282

```
if (empty($options['scope'])) {
    $options['scope'] = $this->getDefaultScopes();
}

if (is_array($options['scope'])) {
    $separator = $this->getScopeSeparator();
    $options['scope'] = implode($separator, $options['scope']);
}
```

This broke my existing integration because previously added scopes were lost and I got an API error: `ACCESS_TOKEN_SCOPE_INSUFFICIENT`.

To "fix" the issue, I had to add the scopes like this while refreshing the token:

```
$newAccessToken = $this->provider->getAccessToken('refresh_token', [
    'refresh_token' => $existingAccessToken->getRefreshToken(),
    'scope' => ['openid', 'email', 'profile', 'https://www.googleapis.com/auth/drive.file'],
]);
```

All the default (`'openid', 'email', 'profile'`) scopes, plus the one I already added while calling `getAuthorizationUrl()`. So to me, 2.8.0 is a breaking change that should have been mentioned.

Pull request:
Revert BC break by only providing scopes in access token when set in options
Partially reverts #1030 
This will still allow to set a `scope` on the access token as array and format it properly, but it will not add the default scopes by default.

Setting the scope in the access token request is optional according to https://www.rfc-editor.org/rfc/rfc6749#section-3.3
In practice it seems to limit the scopes that are set in the authorization flow to a subset of the original scopes. But this is depending on the implementation. 

Hopefully fixes #1052, #1051, https://github.com/RiskioFr/oauth2-auth0/issues/28 https://github.com/Weble/ZohoClient/pull/34

cc @sandervanhooft @liayn

For libraries needing to add default scopes to the access request, I would suggest something like this in your own provider:

```php
public function getAccessToken($grant, array $options = [])
{
    if (empty($options['scope'])) {
        $options['scope'] = $this->getDefaultScopes();
    }
    
    return parent::getAccessToken($grant, $options);
}

```

Patch:
diff --git a/docs/Gemfile.lock b/docs/Gemfile.lock
index 04743a5..ecd09b9 100644
--- a/docs/Gemfile.lock
+++ b/docs/Gemfile.lock
@@ -235,7 +235,7 @@ GEM
     minitest (5.25.4)
     net-http (0.6.0)
       uri
-    nokogiri (1.17.0)
+    nokogiri (1.18.3)
       mini_portile2 (~> 2.8.2)
       racc (~> 1.4)
     octokit (4.25.1)
diff --git a/docs/providers/thirdparty.md b/docs/providers/thirdparty.md
index 7c555a1..98097ce 100755
--- a/docs/providers/thirdparty.md
+++ b/docs/providers/thirdparty.md
@@ -53,6 +53,7 @@ Gateway | Composer Package | Maintainer
 [Dribbble](https://github.com/crewlabs/oauth2-dribbble) | crewlabs/oauth2-dribbble | [Crew Labs](https://crew.co/labs)
 [Dropbox](https://github.com/stevenmaguire/oauth2-dropbox) | stevenmaguire/oauth2-dropbox | [Steven Maguire](https://github.com/stevenmaguire)
 [Drupal](https://github.com/chrishemmings/oauth2-drupal) | chrishemmings/oauth2-drupal | [Chris Hemmings](https://github.com/chrishemmings)
+[Ebay](https://github.com/Gugiman/oauth2-ebay) | gugiman/oauth2-ebay | [Dennis Steffen](https://github.com/gugiman)
 [Ecwid](https://github.com/mugnate/oauth2-ecwid) | mugnate/oauth2-ecwid | [Nikolay Votintsev](https://github.com/votintsev)
 [Edenred](https://github.com/jzecca/oauth2-edenred) | jzecca/oauth2-edenred | [Jérôme Zecca](https://github.com/jzecca)
 [Elance](https://github.com/stevenmaguire/oauth2-elance) | stevenmaguire/oauth2-elance | [Steven Maguire](https://github.com/stevenmaguire)
diff --git a/src/Provider/AbstractProvider.php b/src/Provider/AbstractProvider.php
index 0120ad7..eb62417 100644
--- a/src/Provider/AbstractProvider.php
+++ b/src/Provider/AbstractProvider.php
@@ -17,6 +17,7 @@ namespace League\OAuth2\Client\Provider;
 use GuzzleHttp\Client as HttpClient;
 use GuzzleHttp\ClientInterface as HttpClientInterface;
 use GuzzleHttp\Exception\BadResponseException;
+use GuzzleHttp\Exception\GuzzleException;
 use InvalidArgumentException;
 use League\OAuth2\Client\Grant\AbstractGrant;
 use League\OAuth2\Client\Grant\GrantFactory;
@@ -619,16 +620,13 @@ abstract class AbstractProvider
      * @return AccessTokenInterface
      * @throws IdentityProviderException
      * @throws UnexpectedValueException
+     * @throws GuzzleException
      */
     public function getAccessToken($grant, array $options = [])
     {
         $grant = $this->verifyGrant($grant);
 
-        if (empty($options['scope'])) {
-            $options['scope'] = $this->getDefaultScopes();
-        }
-
-        if (is_array($options['scope'])) {
+        if (isset($options['scope']) && is_array($options['scope'])) {
             $separator = $this->getScopeSeparator();
             $options['scope'] = implode($separator, $options['scope']);
         }
@@ -713,6 +711,7 @@ abstract class AbstractProvider
      *
      * @param  RequestInterface $request
      * @return ResponseInterface
+     * @throws GuzzleException
      */
     public function getResponse(RequestInterface $request)
     {
@@ -726,6 +725,7 @@ abstract class AbstractProvider
      * @return mixed
      * @throws IdentityProviderException
      * @throws UnexpectedValueException
+     * @throws GuzzleException
      */
     public function getParsedResponse(RequestInterface $request)
     {
@@ -875,6 +875,7 @@ abstract class AbstractProvider
      * @return ResourceOwnerInterface
      * @throws IdentityProviderException
      * @throws UnexpectedValueException
+     * @throws GuzzleException
      */
     public function getResourceOwner(AccessToken $token)
     {
@@ -890,6 +891,7 @@ abstract class AbstractProvider
      * @return mixed
      * @throws IdentityProviderException
      * @throws UnexpectedValueException
+     * @throws GuzzleException
      */
     protected function fetchResourceOwnerDetails(AccessToken $token)
     {


Test patch:
diff --git a/test/src/Grant/PasswordTest.php b/test/src/Grant/PasswordTest.php
index b10f177..e7ead77 100644
--- a/test/src/Grant/PasswordTest.php
+++ b/test/src/Grant/PasswordTest.php
@@ -20,8 +20,7 @@ class PasswordTest extends GrantTestCase
             return !empty($body['grant_type'])
                 && $body['grant_type'] === 'password'
                 && !empty($body['username'])
-                && !empty($body['password'])
-                && !empty($body['scope']);
+                && !empty($body['password']);
         };
     }
 
diff --git a/test/src/Provider/AbstractProviderTest.php b/test/src/Provider/AbstractProviderTest.php
index 842e1fe..49c970f 100644
--- a/test/src/Provider/AbstractProviderTest.php
+++ b/test/src/Provider/AbstractProviderTest.php
@@ -632,7 +632,7 @@ class AbstractProviderTest extends TestCase
             ->once()
             ->with(
                 ['client_id' => 'mock_client_id', 'client_secret' => 'mock_secret', 'redirect_uri' => 'none'],
-                ['code' => 'mock_authorization_code', 'scope' => 'test']
+                ['code' => 'mock_authorization_code']
             )
             ->andReturn([]);
 
@@ -675,6 +675,71 @@ class AbstractProviderTest extends TestCase
             });
     }
 
+    /**
+     * @dataProvider getAccessTokenMethodProvider
+     */
+    #[DataProvider('getAccessTokenMethodProvider')]
+    public function testGetAccessTokenWithScope($method)
+    {
+        $provider = new MockProvider([
+            'clientId' => 'mock_client_id',
+            'clientSecret' => 'mock_secret',
+            'redirectUri' => 'none',
+        ]);
+
+        $provider->setAccessTokenMethod($method);
+
+        $raw_response = ['access_token' => 'okay', 'expires' => time() + 3600, 'resource_owner_id' => 3];
+
+        $grant = Mockery::mock(AbstractGrant::class);
+        $grant
+            ->shouldReceive('prepareRequestParameters')
+            ->once()
+            ->with(
+                ['client_id' => 'mock_client_id', 'client_secret' => 'mock_secret', 'redirect_uri' => 'none'],
+                ['code' => 'mock_authorization_code', 'scope' => 'foo,bar']
+            )
+            ->andReturn([]);
+
+        $stream = Mockery::mock(StreamInterface::class);
+        $stream
+            ->shouldReceive('__toString')
+            ->once()
+            ->andReturn(json_encode($raw_response));
+
+        $response = Mockery::mock(ResponseInterface::class);
+        $response
+            ->shouldReceive('getBody')
+            ->once()
+            ->andReturn($stream);
+        $response
+            ->shouldReceive('getHeader')
+            ->once()
+            ->with('content-type')
+            ->andReturn(['application/json']);
+
+        $client = Mockery::spy(ClientInterface::class, [
+            'send' => $response,
+        ]);
+
+        $provider->setHttpClient($client);
+        $token = $provider->getAccessToken($grant, ['code' => 'mock_authorization_code', 'scope' => ['foo', 'bar']]);
+
+        $this->assertInstanceOf(AccessTokenInterface::class, $token);
+
+        $this->assertSame($raw_response['resource_owner_id'], $token->getResourceOwnerId());
+        $this->assertSame($raw_response['access_token'], $token->getToken());
+        $this->assertSame($raw_response['expires'], $token->getExpires());
+
+        $client
+            ->shouldHaveReceived('send')
+            ->once()
+            ->withArgs(function ($request) use ($provider) {
+                return $request->getMethod() === $provider->getAccessTokenMethod()
+                    && (string) $request->getUri() === $provider->getBaseAccessTokenUrl([]);
+            });
+    }
+
     public function testGetAccessTokenWithNonJsonResponse()
     {
         $provider = $this->getMockProvider();