diff --git a/Security/Core/Authentication/Token/AbstractOAuthToken.php b/Security/Core/Authentication/Token/AbstractOAuthToken.php index 43dd9fdf0..c4997159b 100644 --- a/Security/Core/Authentication/Token/AbstractOAuthToken.php +++ b/Security/Core/Authentication/Token/AbstractOAuthToken.php @@ -112,6 +112,10 @@ public function __unserialize(array $data): void } } + public function copyPersistentDataFrom(self $token): void + { + } + /** * @return mixed|void */ diff --git a/Security/Http/Authenticator/OAuthAuthenticator.php b/Security/Http/Authenticator/OAuthAuthenticator.php index 93de00d35..b91162579 100644 --- a/Security/Http/Authenticator/OAuthAuthenticator.php +++ b/Security/Http/Authenticator/OAuthAuthenticator.php @@ -16,6 +16,7 @@ use HWI\Bundle\OAuthBundle\Security\Core\Authentication\Token\OAuthToken; use HWI\Bundle\OAuthBundle\Security\Core\Exception\OAuthAwareExceptionInterface; use HWI\Bundle\OAuthBundle\Security\Core\User\OAuthAwareUserProviderInterface; +use HWI\Bundle\OAuthBundle\Security\Http\Authenticator\Passport\SelfValidatedOAuthPassport; use HWI\Bundle\OAuthBundle\Security\Http\ResourceOwnerMapInterface; use Symfony\Component\HttpFoundation\RedirectResponse; use Symfony\Component\HttpFoundation\Request; @@ -29,9 +30,7 @@ use Symfony\Component\Security\Http\Authentication\AuthenticationFailureHandlerInterface; use Symfony\Component\Security\Http\Authentication\AuthenticationSuccessHandlerInterface; use Symfony\Component\Security\Http\Authenticator\AuthenticatorInterface; -use Symfony\Component\Security\Http\Authenticator\Passport\Badge\UserBadge; use Symfony\Component\Security\Http\Authenticator\Passport\Passport; -use Symfony\Component\Security\Http\Authenticator\Passport\SelfValidatingPassport; use Symfony\Component\Security\Http\EntryPoint\AuthenticationEntryPointInterface; use Symfony\Component\Security\Http\HttpUtils; @@ -52,10 +51,6 @@ final class OAuthAuthenticator implements AuthenticatorInterface, Authentication */ private array $checkPaths; - private ?array $rawToken = null; - private ?string $resourceOwnerName = null; - private ?string $refreshToken = null; - private ?int $createdAt = null; private array $options; public function __construct( @@ -144,8 +139,40 @@ public function authenticate(Request $request): Passport $token = new OAuthToken($accessToken); $token->setResourceOwnerName($resourceOwner->getName()); + return new SelfValidatedOAuthPassport($this->refreshToken($token)); + } + + /** + * This function can be used for refreshing an expired token + * or for custom "password grant" authenticator, if site owner also owns oauth instance. + * + * @template T of OAuthToken + * + * @param T $token + * + * @return T + */ + public function refreshToken(OAuthToken $token): OAuthToken + { + $resourceOwner = $this->resourceOwnerMap->getResourceOwnerByName($token->getResourceOwnerName()); + if ($token->isExpired()) { - $token = $this->refreshToken($token, $resourceOwner); + $expiredToken = $token; + if ($refreshToken = $expiredToken->getRefreshToken()) { + $tokenClass = \get_class($expiredToken); + $token = new $tokenClass($resourceOwner->refreshAccessToken($refreshToken)); + $token->setResourceOwnerName($expiredToken->getResourceOwnerName()); + if (!$token->getRefreshToken()) { + $token->setRefreshToken($expiredToken->getRefreshToken()); + } + $token->copyPersistentDataFrom($expiredToken); + } else { + // if you cannot refresh token, you do not need to make user_info request to oauth-resource + if (null !== $expiredToken->getUser()) { + return $expiredToken; + } + } + unset($expiredToken); } $userResponse = $resourceOwner->getUserInformation($token->getRawToken()); @@ -163,73 +190,73 @@ public function authenticate(Request $request): Passport throw new AuthenticationServiceException('loadUserByOAuthUserResponse() must return a UserInterface.'); } - $this->rawToken = $token->getRawToken(); - $this->resourceOwnerName = $resourceOwner->getName(); - $this->refreshToken = $token->getRefreshToken(); - $this->createdAt = $token->getCreatedAt(); - - return new SelfValidatingPassport( - class_exists(UserBadge::class) - ? new UserBadge( - // @phpstan-ignore-next-line Symfony <5.4 BC layer - method_exists($user, 'getUserIdentifier') ? $user->getUserIdentifier() : $user->getUsername(), - static function () use ($user) { return $user; } - ) - : $user - ); + return $this->recreateToken($token, $user); } /** - * @param Passport|SelfValidatingPassport $passport + * @template T of OAuthToken + * + * @param T $token + * @param ?UserInterface $user + * + * @return T */ - public function createAuthenticatedToken($passport, string $firewallName): TokenInterface + public function recreateToken(OAuthToken $token, ?UserInterface $user = null): OAuthToken { - $token = $this->createToken($passport, $firewallName); + $user = $user instanceof UserInterface ? $user : $token->getUser(); + + $tokenClass = \get_class($token); + if ($user) { + $newToken = new $tokenClass( + $token->getRawToken(), + method_exists($user, 'getRoles') ? $user->getRoles() : [] + ); + $newToken->setUser($user); + } else { + $newToken = new $tokenClass($token->getRawToken()); + } - $this->rawToken = null; - $this->resourceOwnerName = null; - $this->refreshToken = null; - $this->createdAt = null; + $newToken->setResourceOwnerName($token->getResourceOwnerName()); + $newToken->setRefreshToken($token->getRefreshToken()); + $newToken->setCreatedAt($token->getCreatedAt()); + $newToken->setTokenSecret($token->getTokenSecret()); + $newToken->setAttributes($token->getAttributes()); - return $token; - } + // required for compatibility with Symfony 5.4 + if (method_exists($newToken, 'setAuthenticated')) { + $newToken->setAuthenticated((bool) $user, false); + } - public function onAuthenticationSuccess(Request $request, TokenInterface $token, string $firewallName): ?Response - { - return $this->successHandler->onAuthenticationSuccess($request, $token); - } + $newToken->copyPersistentDataFrom($token); - public function onAuthenticationFailure(Request $request, AuthenticationException $exception): Response - { - return $this->failureHandler->onAuthenticationFailure($request, $exception); + return $newToken; } public function createToken(Passport $passport, string $firewallName): TokenInterface { - $token = new OAuthToken($this->rawToken, $passport->getUser()->getRoles()); - $token->setResourceOwnerName($this->resourceOwnerName); - $token->setUser($passport->getUser()); - $token->setRefreshToken($this->refreshToken); - $token->setCreatedAt($this->createdAt); + return $this->createAuthenticatedToken($passport, $firewallName); + } - // required for compatibility with Symfony 5.4 - if (method_exists($token, 'setAuthenticated')) { - $token->setAuthenticated(true, false); + /** + * @param Passport|SelfValidatedOAuthPassport $passport + */ + public function createAuthenticatedToken($passport, string $firewallName): TokenInterface + { + if ($passport instanceof SelfValidatedOAuthPassport) { + return $passport->getToken(); } - return $token; + throw new \LogicException(sprintf('The first argument of "%s" must be instance of "%s", "%s" provided.', __METHOD__, SelfValidatedOAuthPassport::class, \get_class($passport))); } - private function refreshToken(OAuthToken $expiredToken, ResourceOwnerInterface $resourceOwner): OAuthToken + public function onAuthenticationSuccess(Request $request, TokenInterface $token, string $firewallName): ?Response { - if (!$expiredToken->getRefreshToken()) { - return $expiredToken; - } - - $token = new OAuthToken($resourceOwner->refreshAccessToken($expiredToken->getRefreshToken())); - $token->setRefreshToken($expiredToken->getRefreshToken()); + return $this->successHandler->onAuthenticationSuccess($request, $token); + } - return $token; + public function onAuthenticationFailure(Request $request, AuthenticationException $exception): Response + { + return $this->failureHandler->onAuthenticationFailure($request, $exception); } private function extractCsrfTokenFromState(?string $stateParameter): ?string diff --git a/Security/Http/Authenticator/Passport/SelfValidatedOAuthPassport.php b/Security/Http/Authenticator/Passport/SelfValidatedOAuthPassport.php new file mode 100644 index 000000000..c0dcdf2d4 --- /dev/null +++ b/Security/Http/Authenticator/Passport/SelfValidatedOAuthPassport.php @@ -0,0 +1,51 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace HWI\Bundle\OAuthBundle\Security\Http\Authenticator\Passport; + +use HWI\Bundle\OAuthBundle\Security\Core\Authentication\Token\OAuthToken; +use Symfony\Component\Security\Http\Authenticator\Passport\Badge\BadgeInterface; +use Symfony\Component\Security\Http\Authenticator\Passport\Badge\UserBadge; +use Symfony\Component\Security\Http\Authenticator\Passport\SelfValidatingPassport; + +/** + * SelfValidatingPassport contained OAuthToken. + */ +class SelfValidatedOAuthPassport extends SelfValidatingPassport +{ + private OAuthToken $token; + + /** + * Token already contains authenticated user. No need to create trivial UserBadge outside. + * + * @param BadgeInterface[] $badges + */ + public function __construct(OAuthToken $token, array $badges = []) + { + $this->token = $token; + + $user = $token->getUser(); + + $userBadge = class_exists(UserBadge::class) + ? new UserBadge( + method_exists($user, 'getUserIdentifier') ? $user->getUserIdentifier() : $user->getUsername(), + static function () use ($user) { return $user; } + ) + : $user; + + parent::__construct($userBadge, $badges); + } + + public function getToken(): OAuthToken + { + return $this->token; + } +} diff --git a/Tests/Fixtures/CustomOAuthToken.php b/Tests/Fixtures/CustomOAuthToken.php index 2000db713..07882130e 100644 --- a/Tests/Fixtures/CustomOAuthToken.php +++ b/Tests/Fixtures/CustomOAuthToken.php @@ -11,14 +11,15 @@ namespace HWI\Bundle\OAuthBundle\Tests\Fixtures; +use HWI\Bundle\OAuthBundle\Security\Core\Authentication\Token\AbstractOAuthToken; use HWI\Bundle\OAuthBundle\Security\Core\Authentication\Token\OAuthToken; final class CustomOAuthToken extends OAuthToken { - public function __construct() + public function __construct(array $accessToken = []) { parent::__construct( - [ + $accessToken + [ 'access_token' => 'access_token_data', ], [ @@ -28,4 +29,15 @@ public function __construct() $this->setUser(new User()); } + + public function copyPersistentDataFrom(AbstractOAuthToken $token): void + { + if ($token instanceof self) { + if ($token->hasAttribute('persistent_key')) { + $this->setAttribute('persistent_key', $token->getAttribute('persistent_key')); + } + } + + parent::copyPersistentDataFrom($token); + } } diff --git a/Tests/Security/Http/Authenticator/OAuthAuthenticatorTest.php b/Tests/Security/Http/Authenticator/OAuthAuthenticatorTest.php index 3e1321c69..fd0623c18 100644 --- a/Tests/Security/Http/Authenticator/OAuthAuthenticatorTest.php +++ b/Tests/Security/Http/Authenticator/OAuthAuthenticatorTest.php @@ -18,6 +18,8 @@ use HWI\Bundle\OAuthBundle\Security\Core\User\OAuthAwareUserProviderInterface; use HWI\Bundle\OAuthBundle\Security\Http\Authenticator\OAuthAuthenticator; use HWI\Bundle\OAuthBundle\Security\Http\ResourceOwnerMap; +use HWI\Bundle\OAuthBundle\Security\Http\ResourceOwnerMapInterface; +use HWI\Bundle\OAuthBundle\Tests\Fixtures\CustomOAuthToken; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Symfony\Component\DependencyInjection\ServiceLocator; @@ -101,7 +103,7 @@ public function testAuthenticate(): void ->willReturn(true); $serviceLocator = $this->createMock(ServiceLocator::class); - $serviceLocator->expects($this->once()) + $serviceLocator->expects($this->exactly(2)) ->method('get') ->with($resourceOwnerName) ->willReturn($resourceOwnerMock); @@ -200,6 +202,137 @@ public function testOnAuthenticationSuccess(): void $this->assertSame($response, $authenticator->onAuthenticationSuccess($request, $token, 'main')); } + public function testRecreateToken() + { + $authenticator = new OAuthAuthenticator( + $this->getHttpUtilsMock(), + $this->getOAuthAwareUserProviderMock(), + $this->getResourceOwnerMap(), + ['/a', '/b'], + $this->getAuthenticationSuccessHandlerMock(), + $this->getAuthenticationFailureHandlerMock(), + $this->createMock(HttpKernelInterface::class), + [] + ); + + $token = new CustomOAuthToken([ + 'refresh_token' => 'refresh token data', + 'expires' => 666, + 'oauth_token_secret' => 'oauth secret', + ]); + $this->assertFalse($token->isExpired()); + $user = $token->getUser(); + $token->setResourceOwnerName('keycloak'); + $token->setCreatedAt(10); + $token->setAttribute('attr a', 'attr a'); + + $newToken = $authenticator->recreateToken($token); + + $this->assertInstanceOf(CustomOAuthToken::class, $newToken); + $this->assertNotSame($token, $newToken); + $this->assertSame($user, $newToken->getUser()); + $this->assertEquals('keycloak', $newToken->getResourceOwnerName()); + $this->assertEquals('access_token_data', $newToken->getAccessToken()); + $this->assertEquals('refresh token data', $newToken->getRefreshToken()); + $this->assertEquals(10, $newToken->getCreatedAt()); + $this->assertEquals(666, $newToken->getExpiresIn()); + $this->assertEquals('oauth secret', $newToken->getTokenSecret()); + $this->assertTrue($newToken->hasAttribute('attr a')); + $this->assertEquals('attr a', $newToken->getAttribute('attr a')); + $this->assertFalse($newToken->hasAttribute('non exists attr')); + } + + public function testRefreshTokenExpiredAndNotContainsRefreshToken() + { + $resourceOwnerName = 'keycloak'; + + $resourceOwnerMock = $this->getResourceOwnerMock(); + $resourceOwnerMock->expects($this->never()) + ->method('getUserInformation'); + + $resourceOwnerMapMock = $this->getResourceOwnerMapMock(); + $resourceOwnerMapMock->expects($this->once()) + ->method('getResourceOwnerByName') + ->willReturn($resourceOwnerMock); + + $authenticator = new OAuthAuthenticator( + $this->getHttpUtilsMock(), + $this->getOAuthAwareUserProviderMock(), + $resourceOwnerMapMock, + ['/a', '/b'], + $this->getAuthenticationSuccessHandlerMock(), + $this->getAuthenticationFailureHandlerMock(), + $this->createMock(HttpKernelInterface::class), + [] + ); + + $token = new CustomOAuthToken([ + 'expires' => 666, + ]); + $token->setResourceOwnerName($resourceOwnerName); + $token->setCreatedAt(10); // expire it + + $newToken = $authenticator->refreshToken($token); + $this->assertSame($newToken, $token, 'Token missing refresh token data will not be refreshed if it already contains an user'); + } + + public function testRefreshTokenExpired() + { + $resourceOwnerName = 'keycloak'; + + $userProviderMock = $this->getOAuthAwareUserProviderMock(); + $userProviderMock->expects($this->once()) + ->method('loadUserByOAuthUserResponse') + ->willReturn($this->createUser()); + + $resourceOwnerMock = $this->getResourceOwnerMock(); + $resourceOwnerMock->expects($this->once()) + ->method('refreshAccessToken') + ->willReturn([ + 'access_token' => 'access_token', + 'refresh_token' => 'refresh_token', + 'expires_in' => '666', + 'oauth_token_secret' => 'secret', + ]); + $resourceOwnerMock->expects($this->once()) + ->method('getUserInformation') + ->willReturn($this->getUserResponseMock()); + + $resourceOwnerMapMock = $this->getResourceOwnerMapMock(); + $resourceOwnerMapMock->expects($this->once()) + ->method('getResourceOwnerByName')->willReturn($resourceOwnerMock); + + $authenticator = new OAuthAuthenticator( + $this->getHttpUtilsMock(), + $userProviderMock, + $resourceOwnerMapMock, + ['/a', '/b'], + $this->getAuthenticationSuccessHandlerMock(), + $this->getAuthenticationFailureHandlerMock(), + $this->createMock(HttpKernelInterface::class), + [] + ); + + $token = new CustomOAuthToken([ + 'expires' => 666, + 'refresh_token' => 'refresh token data', + ]); + $token->setResourceOwnerName($resourceOwnerName); + $token->setCreatedAt(10); // expire it + $user = $token->getUser(); + + $token->setAttribute('non_persistent_key', 'some non persistent value'); + $token->setAttribute('persistent_key', 'some persistent value'); + + $newToken = $authenticator->refreshToken($token); + $this->assertNotSame($newToken, $token); + $this->assertNotSame($newToken->getUser(), $user); // in real live may be rather the same + + $this->assertFalse($newToken->hasAttribute('non_persistent_key')); + $this->assertTrue($newToken->hasAttribute('persistent_key')); + $this->assertEquals('some persistent value', $newToken->getAttribute('persistent_key')); + } + public function testOnAuthenticationFailure(): void { $request = Request::create('/auth'); @@ -259,6 +392,14 @@ private function getAuthenticationFailureHandlerMock(): AuthenticationFailureHan return $this->createMock(AuthenticationFailureHandlerInterface::class); } + /** + * @return ResourceOwnerMapInterface&MockObject + */ + private function getResourceOwnerMapMock(): ResourceOwnerMapInterface + { + return $this->createMock(ResourceOwnerMapInterface::class); + } + /** * @return ResourceOwnerInterface&MockObject */