diff --git a/libs/api-bundle/src/Security/AttributeAuthenticator.php b/libs/api-bundle/src/Security/AttributeAuthenticator.php index 87b5dd062..56ed436ff 100644 --- a/libs/api-bundle/src/Security/AttributeAuthenticator.php +++ b/libs/api-bundle/src/Security/AttributeAuthenticator.php @@ -42,13 +42,16 @@ public function authenticate(Request $request): SelfValidatingPassport $authenticator = $this->authenticators->get($authAttribute->getName()); assert($authenticator instanceof TokenAuthenticatorInterface); - $tokenHeader = $authenticator->getTokenHeader(); - $token = $request->headers->get($tokenHeader); + $tokenHeaders = $authenticator instanceof MultiHeaderTokenAuthenticatorInterface + ? $authenticator->getTokenHeaders() + : [$authenticator->getTokenHeader()]; + + $token = $this->getTokenFromHeaders($request, $tokenHeaders); if ($token === null) { $error = new CustomUserMessageAuthenticationException(sprintf( 'Authentication header "%s" is missing', - $tokenHeader, + implode('" or "', $tokenHeaders), )); continue; } @@ -125,4 +128,21 @@ private function getControllerAuthAttributes(Request $request): array ReflectionAttribute::IS_INSTANCEOF, ); } + + /** + * @param list $tokenHeaders + */ + private function getTokenFromHeaders(Request $request, array $tokenHeaders): ?string + { + foreach ($tokenHeaders as $header) { + $value = $request->headers->get($header); + if ($value !== null) { + if ($header === 'Authorization' && str_starts_with($value, 'Bearer ')) { + return substr($value, 7); + } + return $value; + } + } + return null; + } } diff --git a/libs/api-bundle/src/Security/MultiHeaderTokenAuthenticatorInterface.php b/libs/api-bundle/src/Security/MultiHeaderTokenAuthenticatorInterface.php new file mode 100644 index 000000000..eb961dc95 --- /dev/null +++ b/libs/api-bundle/src/Security/MultiHeaderTokenAuthenticatorInterface.php @@ -0,0 +1,24 @@ + + */ +interface MultiHeaderTokenAuthenticatorInterface extends TokenAuthenticatorInterface +{ + /** + * Returns a list of authentication headers to try, in priority order. + * The first header that has a non-null value will be used for authentication. + * + * @return list + */ + public function getTokenHeaders(): array; +} diff --git a/libs/api-bundle/src/Security/StorageApiToken/StorageApiTokenAuthenticator.php b/libs/api-bundle/src/Security/StorageApiToken/StorageApiTokenAuthenticator.php index f4ceca986..d039ae988 100644 --- a/libs/api-bundle/src/Security/StorageApiToken/StorageApiTokenAuthenticator.php +++ b/libs/api-bundle/src/Security/StorageApiToken/StorageApiTokenAuthenticator.php @@ -6,7 +6,7 @@ use Keboola\ApiBundle\Attribute\AuthAttributeInterface; use Keboola\ApiBundle\Attribute\StorageApiTokenAuth; -use Keboola\ApiBundle\Security\TokenAuthenticatorInterface; +use Keboola\ApiBundle\Security\MultiHeaderTokenAuthenticatorInterface; use Keboola\ApiBundle\Security\TokenInterface; use Keboola\StorageApi\ClientException; use Keboola\StorageApiBranch\Factory\StorageClientRequestFactory; @@ -15,9 +15,9 @@ use Symfony\Component\Security\Core\Exception\CustomUserMessageAuthenticationException; /** - * @implements TokenAuthenticatorInterface + * @implements MultiHeaderTokenAuthenticatorInterface */ -class StorageApiTokenAuthenticator implements TokenAuthenticatorInterface +class StorageApiTokenAuthenticator implements MultiHeaderTokenAuthenticatorInterface { public function __construct( private readonly StorageClientRequestFactory $clientRequestFactory, @@ -30,6 +30,11 @@ public function getTokenHeader(): string return 'X-StorageApi-Token'; } + public function getTokenHeaders(): array + { + return ['Authorization', 'X-StorageApi-Token']; + } + public function authenticateToken(AuthAttributeInterface $authAttribute, string $token): StorageApiToken { assert($authAttribute instanceof StorageApiTokenAuth); diff --git a/libs/api-bundle/tests/Security/AttributeAuthenticatorTest.php b/libs/api-bundle/tests/Security/AttributeAuthenticatorTest.php index d27cac778..fb0ed0899 100644 --- a/libs/api-bundle/tests/Security/AttributeAuthenticatorTest.php +++ b/libs/api-bundle/tests/Security/AttributeAuthenticatorTest.php @@ -7,6 +7,7 @@ use Keboola\ApiBundle\Attribute\ManageApiTokenAuth; use Keboola\ApiBundle\Attribute\StorageApiTokenAuth; use Keboola\ApiBundle\Security\AttributeAuthenticator; +use Keboola\ApiBundle\Security\MultiHeaderTokenAuthenticatorInterface; use Keboola\ApiBundle\Security\TokenAuthenticatorInterface; use Keboola\ApiBundle\Security\TokenInterface; use Keboola\ApiBundle\Util\ControllerReflector; @@ -376,4 +377,182 @@ private function createAuthenticatorWithFailingAuthorization( return $authenticator; } + + public function testAuthenticateRequestWithBearerToken(): void + { + $controller = new + #[StorageApiTokenAuth(['foo-feature'])] + class { + public function __invoke(): void {} + }; + + $request = $this->createControllerRequest($controller, [ + 'Authorization' => 'Bearer my-oauth-token', + ]); + + $token = $this->createToken('user-id'); + + $authenticator = $this->createAuthenticator( + $controller, + [ + StorageApiTokenAuth::class => $this->createMultiHeaderSuccessAuthenticator( + $token, + 'my-oauth-token', + ), + ], + ); + $passport = $authenticator->authenticate($request); + + self::assertSame($token, $passport->getUser()); + } + + public function testAuthenticateRequestWithXStorageApiTokenFallback(): void + { + $controller = new + #[StorageApiTokenAuth(['foo-feature'])] + class { + public function __invoke(): void {} + }; + + $request = $this->createControllerRequest($controller, [ + 'X-StorageApi-Token' => 'my-storage-token', + ]); + + $token = $this->createToken('user-id'); + + $authenticator = $this->createAuthenticator( + $controller, + [ + StorageApiTokenAuth::class => $this->createMultiHeaderSuccessAuthenticator( + $token, + 'my-storage-token', + ), + ], + ); + $passport = $authenticator->authenticate($request); + + self::assertSame($token, $passport->getUser()); + } + + public function testAuthenticateRequestBearerTokenTakesPrecedence(): void + { + $controller = new + #[StorageApiTokenAuth(['foo-feature'])] + class { + public function __invoke(): void {} + }; + + // Both headers present - Bearer should take precedence + $request = $this->createControllerRequest($controller, [ + 'Authorization' => 'Bearer my-oauth-token', + 'X-StorageApi-Token' => 'my-storage-token', + ]); + + $token = $this->createToken('user-id'); + + // The authenticator should receive the Bearer token, not the X-StorageApi-Token + $authenticator = $this->createAuthenticator( + $controller, + [ + StorageApiTokenAuth::class => $this->createMultiHeaderSuccessAuthenticator( + $token, + 'my-oauth-token', // Bearer token value (without "Bearer " prefix) + ), + ], + ); + $passport = $authenticator->authenticate($request); + + self::assertSame($token, $passport->getUser()); + } + + public function testAuthenticateRequestWithMultiHeaderAuthenticatorNoToken(): void + { + $controller = new + #[StorageApiTokenAuth(['foo-feature'])] + class { + public function __invoke(): void {} + }; + + $request = $this->createControllerRequest($controller, []); + + $multiHeaderAuthenticator = $this->createMock(MultiHeaderTokenAuthenticatorInterface::class); + $multiHeaderAuthenticator->expects(self::once()) + ->method('getTokenHeaders') + ->willReturn(['Authorization', 'X-StorageApi-Token']) + ; + + $authenticator = $this->createAuthenticator( + $controller, + [ + StorageApiTokenAuth::class => $multiHeaderAuthenticator, + ], + ); + + $this->expectException(CustomUserMessageAuthenticationException::class); + $this->expectExceptionMessage('Authentication header "Authorization" or "X-StorageApi-Token" is missing'); + + $authenticator->authenticate($request); + } + + public function testAuthenticateRequestWithAuthorizationHeaderWithoutBearerPrefix(): void + { + $controller = new + #[StorageApiTokenAuth(['foo-feature'])] + class { + public function __invoke(): void {} + }; + + // Authorization header without "Bearer " prefix should be used as-is + $request = $this->createControllerRequest($controller, [ + 'Authorization' => 'Basic some-basic-auth', + ]); + + $token = $this->createToken('user-id'); + + $authenticator = $this->createAuthenticator( + $controller, + [ + StorageApiTokenAuth::class => $this->createMultiHeaderSuccessAuthenticator( + $token, + 'Basic some-basic-auth', // Full value since it's not Bearer + ), + ], + ); + $passport = $authenticator->authenticate($request); + + self::assertSame($token, $passport->getUser()); + } + + /** + * @return MultiHeaderTokenAuthenticatorInterface + */ + private function createMultiHeaderSuccessAuthenticator( + TokenInterface $token, + string $expectedTokenValue, + ): MultiHeaderTokenAuthenticatorInterface { + $authenticator = $this->createMock(MultiHeaderTokenAuthenticatorInterface::class); + $authenticator->expects(self::once()) + ->method('getTokenHeaders') + ->willReturn(['Authorization', 'X-StorageApi-Token']) + ; + + $authenticator->expects(self::once()) + ->method('authenticateToken') + ->with( + $this->isInstanceOf(StorageApiTokenAuth::class), + $expectedTokenValue, + ) + ->willReturn($token) + ; + + $authenticator->expects(self::once()) + ->method('authorizeToken') + ->with( + $this->isInstanceOf(StorageApiTokenAuth::class), + $token, + ) + ; + + return $authenticator; + } }