Skip to content

Commit 161ba05

Browse files
committed
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
2 parents 3f88e38 + bb38bb3 commit 161ba05

File tree

3 files changed

+8
-6
lines changed

3 files changed

+8
-6
lines changed

src/Security/Authenticator/OAuth2Authenticator.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,8 +87,8 @@ public function doAuthenticate(Request $request) /* : Passport */
8787
$oauthClientId = $psr7Request->getAttribute('oauth_client_id', '');
8888

8989
/** @psalm-suppress MixedInferredReturnType */
90-
$userLoader = function (string $userIdentifier): UserInterface {
91-
if ('' === $userIdentifier) {
90+
$userLoader = function (string $userIdentifier) use ($oauthClientId): UserInterface {
91+
if ('' === $userIdentifier || $oauthClientId === $userIdentifier) {
9292
return new NullUser();
9393
}
9494
if (!method_exists($this->userProvider, 'loadUserByIdentifier')) {

tests/Fixtures/FixtureFactory.php

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
use League\Bundle\OAuth2ServerBundle\ValueObject\Grant;
1717
use League\Bundle\OAuth2ServerBundle\ValueObject\RedirectUri;
1818
use League\Bundle\OAuth2ServerBundle\ValueObject\Scope;
19+
use League\OAuth2\Server\RequestTypes\AuthorizationRequestInterface;
1920

2021
/**
2122
* Development hints:
@@ -143,15 +144,15 @@ private static function createAccessTokens(ScopeManagerInterface $scopeManager,
143144
self::FIXTURE_ACCESS_TOKEN_PUBLIC,
144145
new \DateTimeImmutable('+1 hour'),
145146
$clientManager->find(self::FIXTURE_CLIENT_FIRST),
146-
null,
147+
interface_exists(AuthorizationRequestInterface::class) ? self::FIXTURE_CLIENT_FIRST : null,
147148
[]
148149
);
149150

150151
$accessTokens[] = (new AccessToken(
151152
self::FIXTURE_ACCESS_TOKEN_WITH_SCOPES,
152153
new \DateTimeImmutable('+1 hour'),
153154
$clientManager->find(self::FIXTURE_CLIENT_FIRST),
154-
null,
155+
interface_exists(AuthorizationRequestInterface::class) ? self::FIXTURE_CLIENT_FIRST : null,
155156
[$scopeManager->find(self::FIXTURE_SCOPE_FIRST)]
156157
));
157158

tests/Integration/ResourceServerTest.php

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
use League\Bundle\OAuth2ServerBundle\Tests\Fixtures\FixtureFactory;
88
use League\Bundle\OAuth2ServerBundle\Tests\TestHelper;
9+
use League\OAuth2\Server\RequestTypes\AuthorizationRequestInterface;
910

1011
final class ResourceServerTest extends AbstractIntegrationTest
1112
{
@@ -34,7 +35,7 @@ public function testValidAccessToken(): void
3435

3536
$this->assertSame(FixtureFactory::FIXTURE_ACCESS_TOKEN_PUBLIC, $request->getAttribute('oauth_access_token_id'));
3637
$this->assertSame(FixtureFactory::FIXTURE_CLIENT_FIRST, $request->getAttribute('oauth_client_id'));
37-
$this->assertSame('', $request->getAttribute('oauth_user_id'));
38+
$this->assertSame(interface_exists(AuthorizationRequestInterface::class) ? FixtureFactory::FIXTURE_CLIENT_FIRST : '', $request->getAttribute('oauth_user_id'));
3839
$this->assertSame([], $request->getAttribute('oauth_scopes'));
3940
}
4041

@@ -50,7 +51,7 @@ public function testValidAccessTokenWithScopes(): void
5051

5152
$this->assertSame(FixtureFactory::FIXTURE_ACCESS_TOKEN_WITH_SCOPES, $request->getAttribute('oauth_access_token_id'));
5253
$this->assertSame(FixtureFactory::FIXTURE_CLIENT_FIRST, $request->getAttribute('oauth_client_id'));
53-
$this->assertSame('', $request->getAttribute('oauth_user_id'));
54+
$this->assertSame(interface_exists(AuthorizationRequestInterface::class) ? FixtureFactory::FIXTURE_CLIENT_FIRST : '', $request->getAttribute('oauth_user_id'));
5455
$this->assertSame([FixtureFactory::FIXTURE_SCOPE_FIRST], $request->getAttribute('oauth_scopes'));
5556
}
5657

0 commit comments

Comments
 (0)