bug #209 Fix client credentials comparing oauth_user_id and `oauth_…

…client_id` (ajgarlag)

This PR was merged into the 0.9-dev branch.

Discussion
----------

Fix client credentials comparing `oauth_user_id` and `oauth_client_id`

In `league/server-bundle` version `0.8`, when the client_credentials grant is used, the `sub` claim of the JWT is an empty string, but in version `0.9` is filled with the client ID.

In [Section 5](https://datatracker.ietf.org/doc/html/rfc9068#SecurityConsiderations) of RFC9068, there is a recommendation to prevent the collision between `sub` claim values when the resource owner is either a client or a user.

So when client_id (derived from `aud[0]` claim) and user_id (derived from `sub` claim) are equal, the resource owner must be a client.

Fix #207

Commits
-------

bb38bb3 Fix client credentials

src/Security/Authenticator/OAuth2Authenticator.php

@@ -87,8 +87,8 @@ public function doAuthenticate(Request $request) /* : Passport */
         $oauthClientId = $psr7Request->getAttribute('oauth_client_id', '');
 
         /** @psalm-suppress MixedInferredReturnType */
-        $userLoader = function (string $userIdentifier): UserInterface {
-            if ('' === $userIdentifier) {
+        $userLoader = function (string $userIdentifier) use ($oauthClientId): UserInterface {
+            if ('' === $userIdentifier || $oauthClientId === $userIdentifier) {
                 return new NullUser();
             }
             if (!method_exists($this->userProvider, 'loadUserByIdentifier')) {

tests/Fixtures/FixtureFactory.php

@@ -16,6 +16,7 @@
 use League\Bundle\OAuth2ServerBundle\ValueObject\Grant;
 use League\Bundle\OAuth2ServerBundle\ValueObject\RedirectUri;
 use League\Bundle\OAuth2ServerBundle\ValueObject\Scope;
+use League\OAuth2\Server\RequestTypes\AuthorizationRequestInterface;
 
 /**
  * Development hints:
@@ -143,15 +144,15 @@ private static function createAccessTokens(ScopeManagerInterface $scopeManager,
             self::FIXTURE_ACCESS_TOKEN_PUBLIC,
             new \DateTimeImmutable('+1 hour'),
             $clientManager->find(self::FIXTURE_CLIENT_FIRST),
-            null,
+            interface_exists(AuthorizationRequestInterface::class) ? self::FIXTURE_CLIENT_FIRST : null,
             []
         );
 
         $accessTokens[] = (new AccessToken(
             self::FIXTURE_ACCESS_TOKEN_WITH_SCOPES,
             new \DateTimeImmutable('+1 hour'),
             $clientManager->find(self::FIXTURE_CLIENT_FIRST),
-            null,
+            interface_exists(AuthorizationRequestInterface::class) ? self::FIXTURE_CLIENT_FIRST : null,
             [$scopeManager->find(self::FIXTURE_SCOPE_FIRST)]
         ));
 

tests/Integration/ResourceServerTest.php

@@ -6,6 +6,7 @@
 
 use League\Bundle\OAuth2ServerBundle\Tests\Fixtures\FixtureFactory;
 use League\Bundle\OAuth2ServerBundle\Tests\TestHelper;
+use League\OAuth2\Server\RequestTypes\AuthorizationRequestInterface;
 
 final class ResourceServerTest extends AbstractIntegrationTest
 {
@@ -34,7 +35,7 @@ public function testValidAccessToken(): void
 
         $this->assertSame(FixtureFactory::FIXTURE_ACCESS_TOKEN_PUBLIC, $request->getAttribute('oauth_access_token_id'));
         $this->assertSame(FixtureFactory::FIXTURE_CLIENT_FIRST, $request->getAttribute('oauth_client_id'));
-        $this->assertSame('', $request->getAttribute('oauth_user_id'));
+        $this->assertSame(interface_exists(AuthorizationRequestInterface::class) ? FixtureFactory::FIXTURE_CLIENT_FIRST : '', $request->getAttribute('oauth_user_id'));
         $this->assertSame([], $request->getAttribute('oauth_scopes'));
     }
 
@@ -50,7 +51,7 @@ public function testValidAccessTokenWithScopes(): void
 
         $this->assertSame(FixtureFactory::FIXTURE_ACCESS_TOKEN_WITH_SCOPES, $request->getAttribute('oauth_access_token_id'));
         $this->assertSame(FixtureFactory::FIXTURE_CLIENT_FIRST, $request->getAttribute('oauth_client_id'));
-        $this->assertSame('', $request->getAttribute('oauth_user_id'));
+        $this->assertSame(interface_exists(AuthorizationRequestInterface::class) ? FixtureFactory::FIXTURE_CLIENT_FIRST : '', $request->getAttribute('oauth_user_id'));
         $this->assertSame([FixtureFactory::FIXTURE_SCOPE_FIRST], $request->getAttribute('oauth_scopes'));
     }