From 21532cb6bcbd67dcc497dacc6a4135d9f8d51268 Mon Sep 17 00:00:00 2001 From: Guillaume Smolders Date: Wed, 26 Jul 2023 22:51:25 +0200 Subject: [PATCH] Fix breaking change in AccessTokenAuthenticator fixes #50511 --- .../Tests/Functional/AccessTokenTest.php | 12 ++ .../AccessToken/config_custom_user_loader.yml | 32 ++++ .../AccessToken/Oidc/OidcTokenHandler.php | 3 +- .../Oidc/OidcUserInfoTokenHandler.php | 3 +- .../AccessTokenAuthenticator.php | 2 +- .../Http/Authenticator/FallbackUserLoader.php | 32 ++++ .../AccessToken/Oidc/OidcTokenHandlerTest.php | 3 +- .../Oidc/OidcUserInfoTokenHandlerTest.php | 3 +- .../AccessTokenAuthenticatorTest.php | 162 ++++++++++++++++++ 9 files changed, 247 insertions(+), 5 deletions(-) create mode 100644 src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/AccessToken/config_custom_user_loader.yml create mode 100644 src/Symfony/Component/Security/Http/Authenticator/FallbackUserLoader.php create mode 100644 src/Symfony/Component/Security/Http/Tests/Authenticator/AccessTokenAuthenticatorTest.php diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/AccessTokenTest.php b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/AccessTokenTest.php index 3deb91402165e..6cc2b1f0fb150 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/AccessTokenTest.php +++ b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/AccessTokenTest.php @@ -333,6 +333,18 @@ public function testSelfContainedTokens() $this->assertSame(['message' => 'Welcome @dunglas!'], json_decode($response->getContent(), true)); } + public function testCustomUserLoader() + { + $client = $this->createClient(['test_case' => 'AccessToken', 'root_config' => 'config_custom_user_loader.yml']); + $client->catchExceptions(false); + $client->request('GET', '/foo', [], [], ['HTTP_AUTHORIZATION' => 'Bearer SELF_CONTAINED_ACCESS_TOKEN']); + $response = $client->getResponse(); + + $this->assertInstanceOf(Response::class, $response); + $this->assertSame(200, $response->getStatusCode()); + $this->assertSame(['message' => 'Welcome @dunglas!'], json_decode($response->getContent(), true)); + } + /** * @requires extension openssl */ diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/AccessToken/config_custom_user_loader.yml b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/AccessToken/config_custom_user_loader.yml new file mode 100644 index 0000000000000..2027656b4d83c --- /dev/null +++ b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/AccessToken/config_custom_user_loader.yml @@ -0,0 +1,32 @@ +imports: + - { resource: ./../config/framework.yml } + +framework: + http_method_override: false + serializer: ~ + +security: + password_hashers: + Symfony\Component\Security\Core\User\InMemoryUser: plaintext + + providers: + in_memory: + memory: + users: + dunglas: { password: foo, roles: [ROLE_MISSING] } + + firewalls: + main: + pattern: ^/ + stateless: true + access_token: + token_handler: access_token.access_token_handler + token_extractors: 'header' + realm: 'My API' + + access_control: + - { path: ^/foo, roles: ROLE_USER } + +services: + access_token.access_token_handler: + class: Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\AccessTokenBundle\Security\Handler\AccessTokenHandler diff --git a/src/Symfony/Component/Security/Http/AccessToken/Oidc/OidcTokenHandler.php b/src/Symfony/Component/Security/Http/AccessToken/Oidc/OidcTokenHandler.php index d595bfa88d4c9..94184e3f84ed0 100644 --- a/src/Symfony/Component/Security/Http/AccessToken/Oidc/OidcTokenHandler.php +++ b/src/Symfony/Component/Security/Http/AccessToken/Oidc/OidcTokenHandler.php @@ -27,6 +27,7 @@ use Symfony\Component\Security\Http\AccessToken\AccessTokenHandlerInterface; use Symfony\Component\Security\Http\AccessToken\Oidc\Exception\InvalidSignatureException; use Symfony\Component\Security\Http\AccessToken\Oidc\Exception\MissingClaimException; +use Symfony\Component\Security\Http\Authenticator\FallbackUserLoader; use Symfony\Component\Security\Http\Authenticator\Passport\Badge\UserBadge; /** @@ -93,7 +94,7 @@ public function getUserBadgeFrom(string $accessToken): UserBadge } // UserLoader argument can be overridden by a UserProvider on AccessTokenAuthenticator::authenticate - return new UserBadge($claims[$this->claim], fn () => $this->createUser($claims), $claims); + return new UserBadge($claims[$this->claim], new FallbackUserLoader(fn () => $this->createUser($claims)), $claims); } catch (\Exception $e) { $this->logger?->error('An error occurred while decoding and validating the token.', [ 'error' => $e->getMessage(), diff --git a/src/Symfony/Component/Security/Http/AccessToken/Oidc/OidcUserInfoTokenHandler.php b/src/Symfony/Component/Security/Http/AccessToken/Oidc/OidcUserInfoTokenHandler.php index 26279ebf19e68..191e460b55216 100644 --- a/src/Symfony/Component/Security/Http/AccessToken/Oidc/OidcUserInfoTokenHandler.php +++ b/src/Symfony/Component/Security/Http/AccessToken/Oidc/OidcUserInfoTokenHandler.php @@ -15,6 +15,7 @@ use Symfony\Component\Security\Core\Exception\BadCredentialsException; use Symfony\Component\Security\Http\AccessToken\AccessTokenHandlerInterface; use Symfony\Component\Security\Http\AccessToken\Oidc\Exception\MissingClaimException; +use Symfony\Component\Security\Http\Authenticator\FallbackUserLoader; use Symfony\Component\Security\Http\Authenticator\Passport\Badge\UserBadge; use Symfony\Contracts\HttpClient\HttpClientInterface; @@ -48,7 +49,7 @@ public function getUserBadgeFrom(string $accessToken): UserBadge } // UserLoader argument can be overridden by a UserProvider on AccessTokenAuthenticator::authenticate - return new UserBadge($claims[$this->claim], fn () => $this->createUser($claims), $claims); + return new UserBadge($claims[$this->claim], new FallbackUserLoader(fn () => $this->createUser($claims)), $claims); } catch (\Exception $e) { $this->logger?->error('An error occurred on OIDC server.', [ 'error' => $e->getMessage(), diff --git a/src/Symfony/Component/Security/Http/Authenticator/AccessTokenAuthenticator.php b/src/Symfony/Component/Security/Http/Authenticator/AccessTokenAuthenticator.php index c925e00050bed..7b769870749e5 100644 --- a/src/Symfony/Component/Security/Http/Authenticator/AccessTokenAuthenticator.php +++ b/src/Symfony/Component/Security/Http/Authenticator/AccessTokenAuthenticator.php @@ -59,7 +59,7 @@ public function authenticate(Request $request): Passport } $userBadge = $this->accessTokenHandler->getUserBadgeFrom($accessToken); - if ($this->userProvider) { + if ($this->userProvider && (null === $userBadge->getUserLoader() || $userBadge->getUserLoader() instanceof FallbackUserLoader)) { $userBadge->setUserLoader($this->userProvider->loadUserByIdentifier(...)); } diff --git a/src/Symfony/Component/Security/Http/Authenticator/FallbackUserLoader.php b/src/Symfony/Component/Security/Http/Authenticator/FallbackUserLoader.php new file mode 100644 index 0000000000000..65392781518ce --- /dev/null +++ b/src/Symfony/Component/Security/Http/Authenticator/FallbackUserLoader.php @@ -0,0 +1,32 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Security\Http\Authenticator; + +use Symfony\Component\Security\Core\User\UserInterface; + +/** + * This wrapper serves as a marker interface to indicate badge user loaders that should not be overridden by the + * default user provider. + * + * @internal + */ +final class FallbackUserLoader +{ + public function __construct(private $inner) + { + } + + public function __invoke(mixed ...$args): ?UserInterface + { + return ($this->inner)(...$args); + } +} diff --git a/src/Symfony/Component/Security/Http/Tests/AccessToken/Oidc/OidcTokenHandlerTest.php b/src/Symfony/Component/Security/Http/Tests/AccessToken/Oidc/OidcTokenHandlerTest.php index 8c8d86284b59a..ccf11e49862b6 100644 --- a/src/Symfony/Component/Security/Http/Tests/AccessToken/Oidc/OidcTokenHandlerTest.php +++ b/src/Symfony/Component/Security/Http/Tests/AccessToken/Oidc/OidcTokenHandlerTest.php @@ -21,6 +21,7 @@ use Symfony\Component\Security\Core\Exception\BadCredentialsException; use Symfony\Component\Security\Core\User\OidcUser; use Symfony\Component\Security\Http\AccessToken\Oidc\OidcTokenHandler; +use Symfony\Component\Security\Http\Authenticator\FallbackUserLoader; use Symfony\Component\Security\Http\Authenticator\Passport\Badge\UserBadge; /** @@ -61,7 +62,7 @@ public function testGetsUserIdentifierFromSignedToken(string $claim, string $exp ))->getUserBadgeFrom($token); $actualUser = $userBadge->getUserLoader()(); - $this->assertEquals(new UserBadge($expected, fn () => $expectedUser, $claims), $userBadge); + $this->assertEquals(new UserBadge($expected, new FallbackUserLoader(fn () => $expectedUser), $claims), $userBadge); $this->assertInstanceOf(OidcUser::class, $actualUser); $this->assertEquals($expectedUser, $actualUser); $this->assertEquals($claims, $userBadge->getAttributes()); diff --git a/src/Symfony/Component/Security/Http/Tests/AccessToken/Oidc/OidcUserInfoTokenHandlerTest.php b/src/Symfony/Component/Security/Http/Tests/AccessToken/Oidc/OidcUserInfoTokenHandlerTest.php index 3b96174a0d63e..2c8d9ae803f9d 100644 --- a/src/Symfony/Component/Security/Http/Tests/AccessToken/Oidc/OidcUserInfoTokenHandlerTest.php +++ b/src/Symfony/Component/Security/Http/Tests/AccessToken/Oidc/OidcUserInfoTokenHandlerTest.php @@ -16,6 +16,7 @@ use Symfony\Component\Security\Core\Exception\BadCredentialsException; use Symfony\Component\Security\Core\User\OidcUser; use Symfony\Component\Security\Http\AccessToken\Oidc\OidcUserInfoTokenHandler; +use Symfony\Component\Security\Http\Authenticator\FallbackUserLoader; use Symfony\Component\Security\Http\Authenticator\Passport\Badge\UserBadge; use Symfony\Contracts\HttpClient\HttpClientInterface; use Symfony\Contracts\HttpClient\ResponseInterface; @@ -47,7 +48,7 @@ public function testGetsUserIdentifierFromOidcServerResponse(string $claim, stri $userBadge = (new OidcUserInfoTokenHandler($clientMock, null, $claim))->getUserBadgeFrom($accessToken); $actualUser = $userBadge->getUserLoader()(); - $this->assertEquals(new UserBadge($expected, fn () => $expectedUser, $claims), $userBadge); + $this->assertEquals(new UserBadge($expected, new FallbackUserLoader(fn () => $expectedUser), $claims), $userBadge); $this->assertInstanceOf(OidcUser::class, $actualUser); $this->assertEquals($expectedUser, $actualUser); $this->assertEquals($claims, $userBadge->getAttributes()); diff --git a/src/Symfony/Component/Security/Http/Tests/Authenticator/AccessTokenAuthenticatorTest.php b/src/Symfony/Component/Security/Http/Tests/Authenticator/AccessTokenAuthenticatorTest.php new file mode 100644 index 0000000000000..4f010000429dd --- /dev/null +++ b/src/Symfony/Component/Security/Http/Tests/Authenticator/AccessTokenAuthenticatorTest.php @@ -0,0 +1,162 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Authenticator; + +use PHPUnit\Framework\TestCase; +use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\Security\Core\Exception\BadCredentialsException; +use Symfony\Component\Security\Core\User\InMemoryUser; +use Symfony\Component\Security\Core\User\InMemoryUserProvider; +use Symfony\Component\Security\Http\AccessToken\AccessTokenExtractorInterface; +use Symfony\Component\Security\Http\AccessToken\AccessTokenHandlerInterface; +use Symfony\Component\Security\Http\Authenticator\AccessTokenAuthenticator; +use Symfony\Component\Security\Http\Authenticator\FallbackUserLoader; +use Symfony\Component\Security\Http\Authenticator\Passport\Badge\UserBadge; + +class AccessTokenAuthenticatorTest extends TestCase +{ + private AccessTokenHandlerInterface $accessTokenHandler; + private AccessTokenExtractorInterface $accessTokenExtractor; + private InMemoryUserProvider $userProvider; + + protected function setUp(): void + { + $this->accessTokenHandler = $this->createMock(AccessTokenHandlerInterface::class); + $this->accessTokenExtractor = $this->createMock(AccessTokenExtractorInterface::class); + $this->userProvider = new InMemoryUserProvider(['test' => ['password' => 's$cr$t']]); + } + + public function testAuthenticateWithoutAccessToken() + { + $this->expectException(BadCredentialsException::class); + $this->expectExceptionMessage('Invalid credentials.'); + + $request = Request::create('/test'); + + $this->accessTokenExtractor + ->expects($this->once()) + ->method('extractAccessToken') + ->with($request) + ->willReturn(null); + + $authenticator = new AccessTokenAuthenticator( + $this->accessTokenHandler, + $this->accessTokenExtractor, + ); + + $authenticator->authenticate($request); + } + + public function testAuthenticateWithoutProvider() + { + $request = Request::create('/test'); + + $this->accessTokenExtractor + ->expects($this->once()) + ->method('extractAccessToken') + ->with($request) + ->willReturn('test'); + $this->accessTokenHandler + ->expects($this->once()) + ->method('getUserBadgeFrom') + ->with('test') + ->willReturn(new UserBadge('john', fn () => new InMemoryUser('john', null))); + + $authenticator = new AccessTokenAuthenticator( + $this->accessTokenHandler, + $this->accessTokenExtractor, + $this->userProvider, + ); + + $passport = $authenticator->authenticate($request); + + $this->assertEquals('john', $passport->getUser()->getUserIdentifier()); + } + + public function testAuthenticateWithoutUserLoader() + { + $request = Request::create('/test'); + + $this->accessTokenExtractor + ->expects($this->once()) + ->method('extractAccessToken') + ->with($request) + ->willReturn('test'); + $this->accessTokenHandler + ->expects($this->once()) + ->method('getUserBadgeFrom') + ->with('test') + ->willReturn(new UserBadge('test')); + + $authenticator = new AccessTokenAuthenticator( + $this->accessTokenHandler, + $this->accessTokenExtractor, + $this->userProvider, + ); + + $passport = $authenticator->authenticate($request); + + $this->assertEquals('test', $passport->getUser()->getUserIdentifier()); + } + + public function testAuthenticateWithUserLoader() + { + $request = Request::create('/test'); + + $this->accessTokenExtractor + ->expects($this->once()) + ->method('extractAccessToken') + ->with($request) + ->willReturn('test'); + $this->accessTokenHandler + ->expects($this->once()) + ->method('getUserBadgeFrom') + ->with('test') + ->willReturn(new UserBadge('john', fn () => new InMemoryUser('john', null))); + + $authenticator = new AccessTokenAuthenticator( + $this->accessTokenHandler, + $this->accessTokenExtractor, + $this->userProvider, + ); + + $passport = $authenticator->authenticate($request); + + $this->assertEquals('john', $passport->getUser()->getUserIdentifier()); + } + + public function testAuthenticateWithFallbackUserLoader() + { + $request = Request::create('/test'); + + $this->accessTokenExtractor + ->expects($this->once()) + ->method('extractAccessToken') + ->with($request) + ->willReturn('test'); + $this->accessTokenHandler + ->expects($this->once()) + ->method('getUserBadgeFrom') + ->with('test') + ->willReturn(new UserBadge('test', new FallbackUserLoader(fn () => new InMemoryUser('john', null)))); + + $authenticator = new AccessTokenAuthenticator( + $this->accessTokenHandler, + $this->accessTokenExtractor, + $this->userProvider, + ); + + $passport = $authenticator->authenticate($request); + + $this->assertEquals('test', $passport->getUser()->getUserIdentifier()); + } +}