From 03e815d480aad88457c843f965f0f2d778e51efd Mon Sep 17 00:00:00 2001 From: Florent Blaison Date: Sat, 1 May 2021 14:25:35 +0200 Subject: [PATCH] add disable access token saving feature --- docs/index.md | 3 + src/DependencyInjection/Configuration.php | 4 + .../LeagueOAuth2ServerExtension.php | 21 ++++- src/Manager/InMemory/AccessTokenManager.php | 20 +++++ src/Manager/Null/AccessTokenManager.php | 25 ++++++ src/Persistence/Mapping/Driver.php | 20 +++-- src/Repository/NullAccessTokenRepository.php | 54 ++++++++++++ src/Resources/config/access_token/default.php | 24 ++++++ src/Resources/config/access_token/null.php | 21 +++++ src/Resources/config/services.php | 10 --- src/Resources/config/storage/doctrine.php | 2 + .../DoctrineAccessTokenManagerTest.php | 86 ++++++++++++++++--- tests/Integration/AbstractIntegrationTest.php | 2 +- tests/Unit/InMemoryAccessTokenManagerTest.php | 20 ++++- 14 files changed, 280 insertions(+), 32 deletions(-) create mode 100644 src/Manager/Null/AccessTokenManager.php create mode 100644 src/Repository/NullAccessTokenRepository.php create mode 100644 src/Resources/config/access_token/default.php create mode 100644 src/Resources/config/access_token/null.php diff --git a/docs/index.md b/docs/index.md index f0c5532a..6c901f6a 100644 --- a/docs/index.md +++ b/docs/index.md @@ -72,6 +72,9 @@ For implementation into Symfony projects, please see [bundle documentation](docs # Whether to require code challenge for public clients for the auth code grant require_code_challenge_for_public_clients: true + # Whether to enable access token saving to persistence layer (default to true) + persist_access_token: true + resource_server: # Required # Full path to the public key file diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index 4765155b..16f29dcd 100644 --- a/src/DependencyInjection/Configuration.php +++ b/src/DependencyInjection/Configuration.php @@ -106,6 +106,10 @@ private function createAuthorizationServerNode(): NodeDefinition ->info('Whether to enable the implicit grant') ->defaultTrue() ->end() + ->booleanNode('persist_access_token') + ->info('Whether to enable access token saving to persistence layer') + ->defaultTrue() + ->end() ->end() ; diff --git a/src/DependencyInjection/LeagueOAuth2ServerExtension.php b/src/DependencyInjection/LeagueOAuth2ServerExtension.php index c5a0c16e..b7ff22b2 100644 --- a/src/DependencyInjection/LeagueOAuth2ServerExtension.php +++ b/src/DependencyInjection/LeagueOAuth2ServerExtension.php @@ -14,6 +14,7 @@ use League\Bundle\OAuth2ServerBundle\Manager\Doctrine\AuthorizationCodeManager; use League\Bundle\OAuth2ServerBundle\Manager\Doctrine\ClientManager; use League\Bundle\OAuth2ServerBundle\Manager\Doctrine\RefreshTokenManager; +use League\Bundle\OAuth2ServerBundle\Manager\InMemory\AccessTokenManager as InMemoryAccessTokenManager; use League\Bundle\OAuth2ServerBundle\Manager\ScopeManagerInterface; use League\Bundle\OAuth2ServerBundle\Model\Scope as ScopeModel; use League\Bundle\OAuth2ServerBundle\Persistence\Mapping\Driver; @@ -54,6 +55,7 @@ public function load(array $configs, ContainerBuilder $container) $config = $this->processConfiguration(new Configuration(), $configs); + $this->configureAccessTokenSaving($loader, $config['authorization_server']); $this->configurePersistence($loader, $container, $config); $this->configureAuthorizationServer($container, $config['authorization_server']); $this->configureResourceServer($container, $config['resource_server']); @@ -206,6 +208,15 @@ private function configureGrants(ContainerBuilder $container, array $config): vo ; } + private function configureAccessTokenSaving(LoaderInterface $loader, array $config): void + { + if ($config['persist_access_token']) { + $loader->load('access_token/default.php'); + } else { + $loader->load('access_token/null.php'); + } + } + /** * @throws \Exception */ @@ -221,7 +232,7 @@ private function configurePersistence(LoaderInterface $loader, ContainerBuilder switch ($persistenceMethod) { case 'in_memory': $loader->load('storage/in_memory.php'); - $this->configureInMemoryPersistence($container); + $this->configureInMemoryPersistence($container, $config); break; case 'doctrine': $loader->load('storage/doctrine.php'); @@ -241,6 +252,7 @@ private function configureDoctrinePersistence(ContainerBuilder $container, array $container ->findDefinition(AccessTokenManager::class) ->replaceArgument(0, $entityManager) + ->replaceArgument(1, $config['authorization_server']['persist_access_token']) ; $container @@ -267,14 +279,19 @@ private function configureDoctrinePersistence(ContainerBuilder $container, array $container ->findDefinition(Driver::class) ->replaceArgument(0, $config['client']['classname']) + ->replaceArgument(1, $config['authorization_server']['persist_access_token']) ; $container->setParameter('league.oauth2_server.persistence.doctrine.enabled', true); $container->setParameter('league.oauth2_server.persistence.doctrine.manager', $entityManagerName); } - private function configureInMemoryPersistence(ContainerBuilder $container): void + private function configureInMemoryPersistence(ContainerBuilder $container, array $config): void { + $container + ->findDefinition(InMemoryAccessTokenManager::class) + ->replaceArgument(0, $config['authorization_server']['persist_access_token']) + ; $container->setParameter('league.oauth2_server.persistence.in_memory.enabled', true); } diff --git a/src/Manager/InMemory/AccessTokenManager.php b/src/Manager/InMemory/AccessTokenManager.php index a6308d69..220a2392 100644 --- a/src/Manager/InMemory/AccessTokenManager.php +++ b/src/Manager/InMemory/AccessTokenManager.php @@ -14,21 +14,41 @@ final class AccessTokenManager implements AccessTokenManagerInterface */ private $accessTokens = []; + /** @var bool */ + private $persistAccessToken; + + public function __construct(bool $persistAccessToken) + { + $this->persistAccessToken = $persistAccessToken; + } + /** * @psalm-mutation-free */ public function find(string $identifier): ?AccessToken { + if (!$this->persistAccessToken) { + return null; + } + return $this->accessTokens[$identifier] ?? null; } public function save(AccessToken $accessToken): void { + if (!$this->persistAccessToken) { + return; + } + $this->accessTokens[$accessToken->getIdentifier()] = $accessToken; } public function clearExpired(): int { + if (!$this->persistAccessToken) { + return 0; + } + $count = \count($this->accessTokens); $now = new \DateTimeImmutable(); diff --git a/src/Manager/Null/AccessTokenManager.php b/src/Manager/Null/AccessTokenManager.php new file mode 100644 index 00000000..1476e887 --- /dev/null +++ b/src/Manager/Null/AccessTokenManager.php @@ -0,0 +1,25 @@ +clientClass = $clientClass; + $this->persistAccessToken = $persistAccessToken; } public function loadMetadataForClass($className, ClassMetadata $metadata): void @@ -63,11 +67,11 @@ public function getAllClassNames(): array return array_merge( [ AbstractClient::class, - AccessToken::class, AuthorizationCode::class, RefreshToken::class, ], - Client::class === $this->clientClass ? [Client::class] : [] + Client::class === $this->clientClass ? [Client::class] : [], + $this->persistAccessToken ? [AccessToken::class] : [] ); } @@ -126,12 +130,18 @@ private function buildClientMetadata(ClassMetadata $metadata): void private function buildRefreshTokenMetadata(ClassMetadata $metadata): void { - (new ClassMetadataBuilder($metadata)) + $classMetadataBuilder = (new ClassMetadataBuilder($metadata)) ->setTable('oauth2_refresh_token') ->createField('identifier', 'string')->makePrimaryKey()->length(80)->option('fixed', true)->build() ->addField('expiry', 'datetime_immutable') ->addField('revoked', 'boolean') - ->createManyToOne('accessToken', AccessToken::class)->addJoinColumn('access_token', 'identifier', true, false, 'SET NULL')->build() ; + + if ($this->persistAccessToken) { + $classMetadataBuilder->createManyToOne('accessToken', AccessToken::class) + ->addJoinColumn('access_token', 'identifier', true, false, 'SET NULL') + ->build() + ; + } } } diff --git a/src/Repository/NullAccessTokenRepository.php b/src/Repository/NullAccessTokenRepository.php new file mode 100644 index 00000000..6061d1de --- /dev/null +++ b/src/Repository/NullAccessTokenRepository.php @@ -0,0 +1,54 @@ +setClient($clientEntity); + $accessToken->setUserIdentifier($userIdentifier); + + foreach ($scopes as $scope) { + $accessToken->addScope($scope); + } + + return $accessToken; + } + + /** + * {@inheritdoc} + */ + public function persistNewAccessToken(AccessTokenEntityInterface $accessTokenEntity): void + { + // do nothing + } + + /** + * {@inheritdoc} + */ + public function revokeAccessToken($tokenId): void + { + // do nothing + } + + /** + * {@inheritdoc} + */ + public function isAccessTokenRevoked($tokenId): bool + { + return false; + } +} diff --git a/src/Resources/config/access_token/default.php b/src/Resources/config/access_token/default.php new file mode 100644 index 00000000..890a05d1 --- /dev/null +++ b/src/Resources/config/access_token/default.php @@ -0,0 +1,24 @@ +services() + + ->set('league.oauth2_server.repository.access_token', AccessTokenRepository::class) + ->args([ + service(AccessTokenManagerInterface::class), + service(ClientManagerInterface::class), + service(ScopeConverterInterface::class), + ]) + ->alias(AccessTokenRepositoryInterface::class, 'league.oauth2_server.repository.access_token') + ->alias(AccessTokenRepository::class, 'league.oauth2_server.repository.access_token'); +}; diff --git a/src/Resources/config/access_token/null.php b/src/Resources/config/access_token/null.php new file mode 100644 index 00000000..f3f9f0ae --- /dev/null +++ b/src/Resources/config/access_token/null.php @@ -0,0 +1,21 @@ +services() + ->set('league.oauth2_server.repository.access_token', NullAccessTokenRepository::class) + ->alias(AccessTokenRepositoryInterface::class, 'league.oauth2_server.repository.access_token') + ->alias(AccessTokenRepository::class, 'league.oauth2_server.repository.access_token') + + ->set('league.oauth2_server.manager.null.access_token', AccessTokenManager::class) + ->alias(AccessTokenManagerInterface::class, 'league.oauth2_server.manager.null.access_token') + ->alias(AccessTokenManager::class, 'league.oauth2_server.manager.null.access_token'); +}; diff --git a/src/Resources/config/services.php b/src/Resources/config/services.php index cf65bd72..c68181b4 100644 --- a/src/Resources/config/services.php +++ b/src/Resources/config/services.php @@ -28,7 +28,6 @@ use League\Bundle\OAuth2ServerBundle\Manager\RefreshTokenManagerInterface; use League\Bundle\OAuth2ServerBundle\Manager\ScopeManagerInterface; use League\Bundle\OAuth2ServerBundle\OAuth2Events; -use League\Bundle\OAuth2ServerBundle\Repository\AccessTokenRepository; use League\Bundle\OAuth2ServerBundle\Repository\AuthCodeRepository; use League\Bundle\OAuth2ServerBundle\Repository\ClientRepository; use League\Bundle\OAuth2ServerBundle\Repository\RefreshTokenRepository; @@ -70,15 +69,6 @@ ->alias(ClientRepositoryInterface::class, 'league.oauth2_server.repository.client') ->alias(ClientRepository::class, 'league.oauth2_server.repository.client') - ->set('league.oauth2_server.repository.access_token', AccessTokenRepository::class) - ->args([ - service(AccessTokenManagerInterface::class), - service(ClientManagerInterface::class), - service(ScopeConverterInterface::class), - ]) - ->alias(AccessTokenRepositoryInterface::class, 'league.oauth2_server.repository.access_token') - ->alias(AccessTokenRepository::class, 'league.oauth2_server.repository.access_token') - ->set('league.oauth2_server.repository.refresh_token', RefreshTokenRepository::class) ->args([ service(RefreshTokenManagerInterface::class), diff --git a/src/Resources/config/storage/doctrine.php b/src/Resources/config/storage/doctrine.php index 38415e6d..6c317056 100644 --- a/src/Resources/config/storage/doctrine.php +++ b/src/Resources/config/storage/doctrine.php @@ -23,6 +23,7 @@ ->set('league.oauth2_server.persistence.driver', Driver::class) ->args([ null, + null, ]) ->alias(Driver::class, 'league.oauth2_server.persistence.driver') @@ -38,6 +39,7 @@ ->set('league.oauth2_server.manager.doctrine.access_token', AccessTokenManager::class) ->args([ null, + null, ]) ->alias(AccessTokenManagerInterface::class, 'league.oauth2_server.manager.doctrine.access_token') ->alias(AccessTokenManager::class, 'league.oauth2_server.manager.doctrine.access_token') diff --git a/tests/Acceptance/DoctrineAccessTokenManagerTest.php b/tests/Acceptance/DoctrineAccessTokenManagerTest.php index a0c70441..97247d68 100644 --- a/tests/Acceptance/DoctrineAccessTokenManagerTest.php +++ b/tests/Acceptance/DoctrineAccessTokenManagerTest.php @@ -6,6 +6,7 @@ use Doctrine\ORM\EntityManagerInterface; use League\Bundle\OAuth2ServerBundle\Manager\Doctrine\AccessTokenManager as DoctrineAccessTokenManager; +use League\Bundle\OAuth2ServerBundle\Manager\Null\AccessTokenManager as NullAccessTokenManager; use League\Bundle\OAuth2ServerBundle\Model\AccessToken; use League\Bundle\OAuth2ServerBundle\Model\Client; use League\Bundle\OAuth2ServerBundle\Model\RefreshToken; @@ -42,6 +43,32 @@ public function testClearExpired(): void ); } + public function testClearExpiredWithoutSavingAccessToken(): void + { + /** @var EntityManagerInterface $em */ + $em = $this->client->getContainer()->get('doctrine.orm.entity_manager'); + + $client = new Client('name', 'client', 'secret'); + $em->persist($client); + $em->flush(); + + $doctrineAccessTokenManager = new NullAccessTokenManager(); + + $testData = $this->buildClearExpiredTestData($client); + + /** @var AccessToken $token */ + foreach ($testData['input'] as $token) { + $doctrineAccessTokenManager->save($token); + } + + $this->assertSame(0, $doctrineAccessTokenManager->clearExpired()); + + $this->assertSame( + [], + $em->getRepository(AccessToken::class)->findBy([], ['identifier' => 'ASC']) + ); + } + private function buildClearExpiredTestData(Client $client): array { $validAccessTokens = [ @@ -102,18 +129,46 @@ public function testClearExpiredWithRefreshToken(): void ); } - private function buildClearExpiredTestDataWithRefreshToken(Client $client): array + public function testClearExpiredWithRefreshTokenWithoutSavingAccessToken(): void + { + /** @var EntityManagerInterface $em */ + $em = $this->client->getContainer()->get('doctrine.orm.entity_manager'); + + $client = new Client('name', 'client', 'secret'); + $em->persist($client); + $em->flush(); + + $doctrineAccessTokenManager = new NullAccessTokenManager(); + + $testData = $this->buildClearExpiredTestDataWithRefreshToken($client, false); + + /** @var RefreshToken $token */ + foreach ($testData['input'] as $token) { + $em->persist($token); + } + + $em->flush(); + + $this->assertSame(0, $doctrineAccessTokenManager->clearExpired()); + + $this->assertSame( + $testData['input'], + $em->getRepository(RefreshToken::class)->findBy(['accessToken' => null], ['identifier' => 'ASC']) + ); + } + + private function buildClearExpiredTestDataWithRefreshToken(Client $client, bool $withAccessToken = true): array { $validRefreshTokens = [ - $this->buildRefreshToken('1111', '+1 day', $client), - $this->buildRefreshToken('2222', '+1 hour', $client), - $this->buildRefreshToken('3333', '+5 second', $client), + $this->buildRefreshToken('1111', '+1 day', $client, $withAccessToken), + $this->buildRefreshToken('2222', '+1 hour', $client, $withAccessToken), + $this->buildRefreshToken('3333', '+5 second', $client, $withAccessToken), ]; $expiredRefreshTokens = [ - $this->buildRefreshToken('5555', '-1 day', $client), - $this->buildRefreshToken('6666', '-1 hour', $client), - $this->buildRefreshToken('7777', '-1 second', $client), + $this->buildRefreshToken('5555', '-1 day', $client, $withAccessToken), + $this->buildRefreshToken('6666', '-1 hour', $client, $withAccessToken), + $this->buildRefreshToken('7777', '-1 second', $client, $withAccessToken), ]; return [ @@ -122,18 +177,23 @@ private function buildClearExpiredTestDataWithRefreshToken(Client $client): arra ]; } - private function buildRefreshToken(string $identifier, string $modify, Client $client): RefreshToken + private function buildRefreshToken(string $identifier, string $modify, Client $client, bool $withAccessToken = true): RefreshToken { - return new RefreshToken( - $identifier, - new \DateTimeImmutable('+1 day'), - new AccessToken( + $accessToken = null; + if ($withAccessToken) { + $accessToken = new AccessToken( $identifier, new \DateTimeImmutable($modify), $client, null, [] - ) + ); + } + + return new RefreshToken( + $identifier, + new \DateTimeImmutable('+1 day'), + $accessToken ); } } diff --git a/tests/Integration/AbstractIntegrationTest.php b/tests/Integration/AbstractIntegrationTest.php index cc5c10ac..00d708f3 100644 --- a/tests/Integration/AbstractIntegrationTest.php +++ b/tests/Integration/AbstractIntegrationTest.php @@ -110,7 +110,7 @@ protected function setUp(): void $this->eventDispatcher = new EventDispatcher(); $this->scopeManager = new ScopeManager(); $this->clientManager = new ClientManager($this->eventDispatcher); - $this->accessTokenManager = new AccessTokenManager(); + $this->accessTokenManager = new AccessTokenManager(true); $this->refreshTokenManager = new RefreshTokenManager(); $this->authCodeManager = new AuthorizationCodeManager(); diff --git a/tests/Unit/InMemoryAccessTokenManagerTest.php b/tests/Unit/InMemoryAccessTokenManagerTest.php index 4664b9b8..913864e7 100644 --- a/tests/Unit/InMemoryAccessTokenManagerTest.php +++ b/tests/Unit/InMemoryAccessTokenManagerTest.php @@ -16,7 +16,7 @@ final class InMemoryAccessTokenManagerTest extends TestCase { public function testClearExpired(): void { - $inMemoryAccessTokenManager = new InMemoryAccessTokenManager(); + $inMemoryAccessTokenManager = new InMemoryAccessTokenManager(true); $testData = $this->buildClearExpiredTestData(); @@ -32,6 +32,24 @@ public function testClearExpired(): void $this->assertSame($testData['output'], $reflectionProperty->getValue($inMemoryAccessTokenManager)); } + public function testClearExpiredWithoutSavingAccessToken(): void + { + $inMemoryAccessTokenManager = new InMemoryAccessTokenManager(false); + + $testData = $this->buildClearExpiredTestData(); + + foreach ($testData['input'] as $token) { + $inMemoryAccessTokenManager->save($token); + } + + $this->assertSame(0, $inMemoryAccessTokenManager->clearExpired()); + + $reflectionProperty = new \ReflectionProperty(InMemoryAccessTokenManager::class, 'accessTokens'); + $reflectionProperty->setAccessible(true); + + $this->assertSame([], $reflectionProperty->getValue($inMemoryAccessTokenManager)); + } + private function buildClearExpiredTestData(): array { $validAccessTokens = [