From bb38bb350f571ca8864533bc952bdd1a40f6317f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Antonio=20J=2E=20Garc=C3=ADa=20Lagar?= <aj@garcialagar.es>
Date: Sun, 19 Jan 2025 22:34:07 +0100
Subject: [PATCH] Fix client credentials

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.
---
 src/Security/Authenticator/OAuth2Authenticator.php | 4 ++--
 tests/Fixtures/FixtureFactory.php                  | 5 +++--
 tests/Integration/ResourceServerTest.php           | 5 +++--
 3 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/src/Security/Authenticator/OAuth2Authenticator.php b/src/Security/Authenticator/OAuth2Authenticator.php
index 9577edfb..cf4caf31 100644
--- a/src/Security/Authenticator/OAuth2Authenticator.php
+++ b/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')) {
diff --git a/tests/Fixtures/FixtureFactory.php b/tests/Fixtures/FixtureFactory.php
index f9f75894..4b189fa1 100644
--- a/tests/Fixtures/FixtureFactory.php
+++ b/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,7 +144,7 @@ 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,
             []
         );
 
@@ -151,7 +152,7 @@ private static function createAccessTokens(ScopeManagerInterface $scopeManager,
             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)]
         ));
 
diff --git a/tests/Integration/ResourceServerTest.php b/tests/Integration/ResourceServerTest.php
index 966933e0..6e687ca7 100644
--- a/tests/Integration/ResourceServerTest.php
+++ b/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'));
     }