From 5fae2022d0a513b499040f98cb0450fc5f3d65e1 Mon Sep 17 00:00:00 2001 From: Franjo Zadelj Date: Wed, 14 Oct 2020 14:22:29 +0200 Subject: [PATCH 1/9] Add cleanup for revoked tokens with appropriate indexes --- Command/ClearRevokedTokensCommand.php | 115 +++++++++++ Manager/AccessTokenManagerInterface.php | 3 + Manager/Doctrine/AccessTokenManager.php | 9 + Manager/Doctrine/AuthorizationCodeManager.php | 9 + Manager/Doctrine/RefreshTokenManager.php | 9 + Manager/InMemory/AccessTokenManager.php | 11 + Manager/InMemory/AuthorizationCodeManager.php | 11 + Manager/InMemory/RefreshTokenManager.php | 11 + .../config/doctrine/model/AccessToken.orm.xml | 5 + .../doctrine/model/AuthorizationCode.orm.xml | 5 + .../doctrine/model/RefreshToken.orm.xml | 5 + Resources/config/services.xml | 7 + .../ClearRevokedTokensCommandTest.php | 193 ++++++++++++++++++ Tests/Fixtures/FixtureFactory.php | 9 + 14 files changed, 402 insertions(+) create mode 100644 Command/ClearRevokedTokensCommand.php create mode 100644 Tests/Acceptance/ClearRevokedTokensCommandTest.php diff --git a/Command/ClearRevokedTokensCommand.php b/Command/ClearRevokedTokensCommand.php new file mode 100644 index 00000000..c4dfa540 --- /dev/null +++ b/Command/ClearRevokedTokensCommand.php @@ -0,0 +1,115 @@ +accessTokenManager = $accessTokenManager; + $this->refreshTokenManager = $refreshTokenManager; + $this->authorizationCodeManager = $authorizationCodeManager; + } + + protected function configure(): void + { + $this + ->setDescription('Clears all revoked access and/or refresh tokens and/or auth codes') + ->addOption( + 'access-tokens', + 'a', + InputOption::VALUE_NONE, + 'Clear revoked access tokens.' + ) + ->addOption( + 'refresh-tokens', + 'r', + InputOption::VALUE_NONE, + 'Clear revoked refresh tokens.' + ) + ->addOption( + 'auth-codes', + 'c', + InputOption::VALUE_NONE, + 'Clear revoked auth codes.' + ) + ; + } + + protected function execute(InputInterface $input, OutputInterface $output): int + { + $clearExpiredAccessTokens = $input->getOption('access-tokens'); + $clearExpiredRefreshTokens = $input->getOption('refresh-tokens'); + $clearExpiredAuthCodes = $input->getOption('auth-codes'); + + if (!$clearExpiredAccessTokens && !$clearExpiredRefreshTokens && !$clearExpiredAuthCodes) { + $clearExpiredAccessTokens = true; + $clearExpiredRefreshTokens = true; + $clearExpiredAuthCodes = true; + } + + if (true === $clearExpiredAccessTokens) { + $affected = $this->accessTokenManager->clearRevoked(); + $output->writeln( + sprintf( + 'Access tokens deleted: %s.', + $affected + ) + ); + } + + if (true === $clearExpiredRefreshTokens) { + $affected = $this->refreshTokenManager->clearRevoked(); + $output->writeln( + sprintf( + 'Refresh tokens deleted: %s.', + $affected + ) + ); + } + + if (true === $clearExpiredAuthCodes) { + $affected = $this->authorizationCodeManager->clearRevoked(); + $output->writeln( + sprintf( + 'Auth codes deleted: %s.', + $affected + ) + ); + } + + return 0; + } +} diff --git a/Manager/AccessTokenManagerInterface.php b/Manager/AccessTokenManagerInterface.php index fc0b94db..def25615 100644 --- a/Manager/AccessTokenManagerInterface.php +++ b/Manager/AccessTokenManagerInterface.php @@ -6,6 +6,9 @@ use Trikoder\Bundle\OAuth2Bundle\Model\AccessToken; +/** + * @method int clearRevoked() + */ interface AccessTokenManagerInterface { public function find(string $identifier): ?AccessToken; diff --git a/Manager/Doctrine/AccessTokenManager.php b/Manager/Doctrine/AccessTokenManager.php index 41921af8..4508544d 100644 --- a/Manager/Doctrine/AccessTokenManager.php +++ b/Manager/Doctrine/AccessTokenManager.php @@ -47,4 +47,13 @@ public function clearExpired(): int ->getQuery() ->execute(); } + + public function clearRevoked(): int + { + return $this->entityManager->createQueryBuilder() + ->delete(AccessToken::class, 'at') + ->where('at.revoked = 1') + ->getQuery() + ->execute(); + } } diff --git a/Manager/Doctrine/AuthorizationCodeManager.php b/Manager/Doctrine/AuthorizationCodeManager.php index b0aa7c3b..871a76dd 100644 --- a/Manager/Doctrine/AuthorizationCodeManager.php +++ b/Manager/Doctrine/AuthorizationCodeManager.php @@ -47,4 +47,13 @@ public function clearExpired(): int ->getQuery() ->execute(); } + + public function clearRevoked(): int + { + return $this->entityManager->createQueryBuilder() + ->delete(AuthorizationCode::class, 'at') + ->where('at.revoked = 1') + ->getQuery() + ->execute(); + } } diff --git a/Manager/Doctrine/RefreshTokenManager.php b/Manager/Doctrine/RefreshTokenManager.php index 2f8172f2..00d72ca2 100644 --- a/Manager/Doctrine/RefreshTokenManager.php +++ b/Manager/Doctrine/RefreshTokenManager.php @@ -47,4 +47,13 @@ public function clearExpired(): int ->getQuery() ->execute(); } + + public function clearRevoked(): int + { + return $this->entityManager->createQueryBuilder() + ->delete(RefreshToken::class, 'at') + ->where('at.revoked = 1') + ->getQuery() + ->execute(); + } } diff --git a/Manager/InMemory/AccessTokenManager.php b/Manager/InMemory/AccessTokenManager.php index 9fd7ff63..7a1f2cd8 100644 --- a/Manager/InMemory/AccessTokenManager.php +++ b/Manager/InMemory/AccessTokenManager.php @@ -42,4 +42,15 @@ public function clearExpired(): int return $count - \count($this->accessTokens); } + + public function clearRevoked(): int + { + $count = \count($this->accessTokens); + + $this->accessTokens = array_filter($this->accessTokens, static function (AccessToken $accessToken): bool { + return !$accessToken->isRevoked(); + }); + + return $count - \count($this->accessTokens); + } } diff --git a/Manager/InMemory/AuthorizationCodeManager.php b/Manager/InMemory/AuthorizationCodeManager.php index f8d42b2b..31ec7271 100644 --- a/Manager/InMemory/AuthorizationCodeManager.php +++ b/Manager/InMemory/AuthorizationCodeManager.php @@ -36,4 +36,15 @@ public function clearExpired(): int return $count - \count($this->authorizationCodes); } + + public function clearRevoked(): int + { + $count = \count($this->authorizationCodes); + + $this->accessTokens = array_filter($this->authorizationCodes, static function (AuthorizationCode $accessToken): bool { + return !$accessToken->isRevoked(); + }); + + return $count - \count($this->accessTokens); + } } diff --git a/Manager/InMemory/RefreshTokenManager.php b/Manager/InMemory/RefreshTokenManager.php index be410b9b..766d27b2 100644 --- a/Manager/InMemory/RefreshTokenManager.php +++ b/Manager/InMemory/RefreshTokenManager.php @@ -42,4 +42,15 @@ public function clearExpired(): int return $count - \count($this->refreshTokens); } + + public function clearRevoked(): int + { + $count = \count($this->refreshTokens); + + $this->accessTokens = array_filter($this->refreshTokens, static function (RefreshToken $accessToken): bool { + return !$accessToken->isRevoked(); + }); + + return $count - \count($this->accessTokens); + } } diff --git a/Resources/config/doctrine/model/AccessToken.orm.xml b/Resources/config/doctrine/model/AccessToken.orm.xml index 192c6d9c..da7e5bbf 100644 --- a/Resources/config/doctrine/model/AccessToken.orm.xml +++ b/Resources/config/doctrine/model/AccessToken.orm.xml @@ -14,6 +14,11 @@ + + + + + diff --git a/Resources/config/doctrine/model/AuthorizationCode.orm.xml b/Resources/config/doctrine/model/AuthorizationCode.orm.xml index 2dfc0c40..cb0c7041 100644 --- a/Resources/config/doctrine/model/AuthorizationCode.orm.xml +++ b/Resources/config/doctrine/model/AuthorizationCode.orm.xml @@ -14,6 +14,11 @@ + + + + + diff --git a/Resources/config/doctrine/model/RefreshToken.orm.xml b/Resources/config/doctrine/model/RefreshToken.orm.xml index bcd9cefc..53b3e12b 100644 --- a/Resources/config/doctrine/model/RefreshToken.orm.xml +++ b/Resources/config/doctrine/model/RefreshToken.orm.xml @@ -12,6 +12,11 @@ + + + + + diff --git a/Resources/config/services.xml b/Resources/config/services.xml index a7720b67..09295d02 100644 --- a/Resources/config/services.xml +++ b/Resources/config/services.xml @@ -218,6 +218,13 @@ The "%alias_id%" service alias is deprecated and will be removed in v4. + + + + + + + diff --git a/Tests/Acceptance/ClearRevokedTokensCommandTest.php b/Tests/Acceptance/ClearRevokedTokensCommandTest.php new file mode 100644 index 00000000..d7678571 --- /dev/null +++ b/Tests/Acceptance/ClearRevokedTokensCommandTest.php @@ -0,0 +1,193 @@ +client->getContainer()->get(ScopeManagerInterface::class), + $this->client->getContainer()->get(ClientManagerInterface::class), + $this->client->getContainer()->get(AccessTokenManagerInterface::class), + $this->client->getContainer()->get(RefreshTokenManagerInterface::class), + $this->client->getContainer()->get(AuthorizationCodeManagerInterface::class) + ); + } + + protected function tearDown(): void + { + timecop_return(); + + parent::tearDown(); + } + + public function testClearRevokedAccessAndRefreshTokensAndAuthCodes(): void + { + $this->assertNotNull( + $this->client->getContainer()->get(AccessTokenManagerInterface::class)->find( + FixtureFactory::FIXTURE_ACCESS_TOKEN_REVOKED + ) + ); + $this->assertNotNull( + $this->client->getContainer()->get(RefreshTokenManagerInterface::class)->find( + FixtureFactory::FIXTURE_REFRESH_TOKEN_REVOKED + ) + ); + $this->assertNotNull( + $this->client->getContainer()->get(AuthorizationCodeManagerInterface::class)->find( + FixtureFactory::FIXTURE_AUTH_CODE_REVOKED + ) + ); + + $output = $this->executeCommand(); + + $this->assertStringContainsString('Access tokens deleted: 1.', $output); + $this->assertStringContainsString('Refresh tokens deleted: 1.', $output); + $this->assertStringContainsString('Auth codes deleted: 1.', $output); + + $this->assertNull( + $this->client->getContainer()->get(AccessTokenManagerInterface::class)->find( + FixtureFactory::FIXTURE_ACCESS_TOKEN_REVOKED + ) + ); + $this->assertNull( + $this->client->getContainer()->get(RefreshTokenManagerInterface::class)->find( + FixtureFactory::FIXTURE_REFRESH_TOKEN_REVOKED + ) + ); + $this->assertNull( + $this->client->getContainer()->get(AuthorizationCodeManagerInterface::class)->find( + FixtureFactory::FIXTURE_AUTH_CODE_REVOKED + ) + ); + } + + public function testClearRevokedAccessTokens(): void + { + $output = $this->executeCommand([ + '--access-tokens' => true, + ]); + + $this->assertStringContainsString('Access tokens deleted: 1.', $output); + $this->assertStringNotContainsString('Refresh tokens deleted', $output); + $this->assertStringNotContainsString('Auth codes deleted', $output); + + $this->assertNull( + $this->client->getContainer()->get(AccessTokenManagerInterface::class)->find( + FixtureFactory::FIXTURE_ACCESS_TOKEN_REVOKED + ) + ); + $this->assertInstanceOf( + RefreshToken::class, + $this->client->getContainer()->get(RefreshTokenManagerInterface::class)->find( + FixtureFactory::FIXTURE_REFRESH_TOKEN_REVOKED + ) + ); + $this->assertInstanceOf( + AuthorizationCode::class, + $this->client->getContainer()->get(AuthorizationCodeManagerInterface::class)->find( + FixtureFactory::FIXTURE_AUTH_CODE_REVOKED + ) + ); + } + + public function testClearExpiredRefreshTokens(): void + { + $output = $this->executeCommand([ + '--refresh-tokens' => true, + ]); + + $this->assertStringNotContainsString('Access tokens deleted', $output); + $this->assertStringContainsString('Refresh tokens deleted: 1.', $output); + $this->assertStringNotContainsString('Auth codes deleted', $output); + + $this->assertInstanceOf( + AccessToken::class, + $this->client->getContainer()->get(AccessTokenManagerInterface::class)->find( + FixtureFactory::FIXTURE_ACCESS_TOKEN_REVOKED + ) + ); + $this->assertNull( + $this->client->getContainer()->get(RefreshTokenManagerInterface::class)->find( + FixtureFactory::FIXTURE_REFRESH_TOKEN_REVOKED + ) + ); + $this->assertInstanceOf( + AuthorizationCode::class, + $this->client->getContainer()->get(AuthorizationCodeManagerInterface::class)->find( + FixtureFactory::FIXTURE_AUTH_CODE_REVOKED + ) + ); + } + + public function testClearExpiredAuthCodes(): void + { + $output = $this->executeCommand([ + '--auth-codes' => true, + ]); + + $this->assertStringNotContainsString('Access tokens deleted', $output); + $this->assertStringNotContainsString('Refresh tokens deleted', $output); + $this->assertStringContainsString('Auth codes deleted: 1.', $output); + + $this->assertInstanceOf( + AccessToken::class, + $this->client->getContainer()->get(AccessTokenManagerInterface::class)->find( + FixtureFactory::FIXTURE_ACCESS_TOKEN_REVOKED + ) + ); + $this->assertInstanceOf( + RefreshToken::class, + $this->client->getContainer()->get(RefreshTokenManagerInterface::class)->find( + FixtureFactory::FIXTURE_REFRESH_TOKEN_REVOKED + ) + ); + $this->assertNull( + $this->client->getContainer()->get(AuthorizationCodeManagerInterface::class)->find( + FixtureFactory::FIXTURE_AUTH_CODE_REVOKED + ) + ); + } + + private function executeCommand(array $params = [], int $expectedExitCode = 0): string + { + $command = $this->application->find('trikoder:oauth2:clear-revoked-tokens'); + $commandTester = new CommandTester($command); + $exitCode = $commandTester->execute(array_merge( + [ + 'command' => $command->getName(), + ], + $params + )); + $this->assertSame($expectedExitCode, $exitCode); + $this->clearEntityManager(); + + return $commandTester->getDisplay(true); + } + + private function clearEntityManager(): void + { + /** @var EntityManagerInterface $em */ + $em = $this->client->getContainer()->get('doctrine.orm.entity_manager'); + $em->clear(); + } +} diff --git a/Tests/Fixtures/FixtureFactory.php b/Tests/Fixtures/FixtureFactory.php index 279e6202..1155d92a 100644 --- a/Tests/Fixtures/FixtureFactory.php +++ b/Tests/Fixtures/FixtureFactory.php @@ -44,6 +44,7 @@ final class FixtureFactory public const FIXTURE_AUTH_CODE_PUBLIC_CLIENT = 'xaa70e8152259988b3c8e9e8cff604019bb986eb226bd126da189829b95a2be631e2506042064e12'; public const FIXTURE_AUTH_CODE_DIFFERENT_CLIENT = 'e8fe264053cb346f4437af05c8cc9036931cfec3a0d5b54bdae349304ca4a83fd2f4590afd51e559'; public const FIXTURE_AUTH_CODE_EXPIRED = 'a7bdbeb26c9f095d842f5e5b8e313b24318d6b26728d1c543136727bbe9525f7a7930305a09b7401'; + public const FIXTURE_AUTH_CODE_REVOKED = 'a89b6df0f51fd955a578fd00f28a99976401589f788097fa22ef6bef4ccb3281b7cead70dca8d8f4'; public const FIXTURE_CLIENT_FIRST = 'foo'; public const FIXTURE_CLIENT_SECOND = 'bar'; @@ -247,6 +248,14 @@ public static function createAuthorizationCodes(ClientManagerInterface $clientMa [] ); + $authorizationCodes[] = (new AuthorizationCode( + self::FIXTURE_AUTH_CODE_REVOKED, + new DateTimeImmutable('+5 minute'), + $clientManager->find(self::FIXTURE_CLIENT_FIRST), + self::FIXTURE_USER, + [] + ))->revoke(); + return $authorizationCodes; } From 5651f6c58f1cdbbb787edbe75faa54c39cc733be Mon Sep 17 00:00:00 2001 From: Franjo Zadelj Date: Mon, 19 Oct 2020 11:13:58 +0200 Subject: [PATCH 2/9] Remove indexes to avoid problems with other naming strategies --- Resources/config/doctrine/model/AccessToken.orm.xml | 5 ----- Resources/config/doctrine/model/AuthorizationCode.orm.xml | 5 ----- Resources/config/doctrine/model/RefreshToken.orm.xml | 5 ----- 3 files changed, 15 deletions(-) diff --git a/Resources/config/doctrine/model/AccessToken.orm.xml b/Resources/config/doctrine/model/AccessToken.orm.xml index da7e5bbf..192c6d9c 100644 --- a/Resources/config/doctrine/model/AccessToken.orm.xml +++ b/Resources/config/doctrine/model/AccessToken.orm.xml @@ -14,11 +14,6 @@ - - - - - diff --git a/Resources/config/doctrine/model/AuthorizationCode.orm.xml b/Resources/config/doctrine/model/AuthorizationCode.orm.xml index cb0c7041..2dfc0c40 100644 --- a/Resources/config/doctrine/model/AuthorizationCode.orm.xml +++ b/Resources/config/doctrine/model/AuthorizationCode.orm.xml @@ -14,11 +14,6 @@ - - - - - diff --git a/Resources/config/doctrine/model/RefreshToken.orm.xml b/Resources/config/doctrine/model/RefreshToken.orm.xml index 53b3e12b..bcd9cefc 100644 --- a/Resources/config/doctrine/model/RefreshToken.orm.xml +++ b/Resources/config/doctrine/model/RefreshToken.orm.xml @@ -12,11 +12,6 @@ - - - - - From 69d8be2b147dae83be8e0b1bc24809d0984018d1 Mon Sep 17 00:00:00 2001 From: Franjo Zadelj Date: Mon, 19 Oct 2020 11:18:37 +0200 Subject: [PATCH 3/9] Add tests for tokens revoke --- Command/ClearRevokedTokensCommand.php | 39 ++- Manager/AccessTokenManagerInterface.php | 2 +- Manager/AuthorizationCodeManagerInterface.php | 3 + Manager/Doctrine/AccessTokenManager.php | 2 +- Manager/Doctrine/AuthorizationCodeManager.php | 4 +- Manager/Doctrine/RefreshTokenManager.php | 4 +- Manager/InMemory/AuthorizationCodeManager.php | 6 +- Manager/InMemory/RefreshTokenManager.php | 6 +- Manager/RefreshTokenManagerInterface.php | 3 + .../ClearRevokedTokensCommandTest.php | 4 +- .../DoctrineAccessTokenManagerTest.php | 247 ++++++++++++++---- .../DoctrineAuthCodeManagerTest.php | 131 ++++++++-- .../DoctrineRefreshTokenManagerTest.php | 133 ++++++++-- Tests/Unit/InMemoryAccessTokenManagerTest.php | 124 +++++++-- Tests/Unit/InMemoryAuthCodeManagerTest.php | 129 +++++++-- .../Unit/InMemoryRefreshTokenManagerTest.php | 124 +++++++-- 16 files changed, 791 insertions(+), 170 deletions(-) diff --git a/Command/ClearRevokedTokensCommand.php b/Command/ClearRevokedTokensCommand.php index c4dfa540..86931433 100644 --- a/Command/ClearRevokedTokensCommand.php +++ b/Command/ClearRevokedTokensCommand.php @@ -70,17 +70,17 @@ protected function configure(): void protected function execute(InputInterface $input, OutputInterface $output): int { - $clearExpiredAccessTokens = $input->getOption('access-tokens'); - $clearExpiredRefreshTokens = $input->getOption('refresh-tokens'); - $clearExpiredAuthCodes = $input->getOption('auth-codes'); - - if (!$clearExpiredAccessTokens && !$clearExpiredRefreshTokens && !$clearExpiredAuthCodes) { - $clearExpiredAccessTokens = true; - $clearExpiredRefreshTokens = true; - $clearExpiredAuthCodes = true; + $clearRevokedAccessTokens = $input->getOption('access-tokens'); + $clearRevokedRefreshTokens = $input->getOption('refresh-tokens'); + $clearRevokedAuthCodes = $input->getOption('auth-codes'); + + if (!$clearRevokedAccessTokens && !$clearRevokedRefreshTokens && !$clearRevokedAuthCodes) { + $clearRevokedAccessTokens = true; + $clearRevokedRefreshTokens = true; + $clearRevokedAuthCodes = true; } - if (true === $clearExpiredAccessTokens) { + if (true === $clearRevokedAccessTokens && $this->checkMethod($output, $this->accessTokenManager)) { $affected = $this->accessTokenManager->clearRevoked(); $output->writeln( sprintf( @@ -90,7 +90,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int ); } - if (true === $clearExpiredRefreshTokens) { + if (true === $clearRevokedRefreshTokens && $this->checkMethod($output, $this->refreshTokenManager)) { $affected = $this->refreshTokenManager->clearRevoked(); $output->writeln( sprintf( @@ -100,7 +100,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int ); } - if (true === $clearExpiredAuthCodes) { + if (true === $clearRevokedAuthCodes && $this->checkMethod($output, $this->authorizationCodeManager)) { $affected = $this->authorizationCodeManager->clearRevoked(); $output->writeln( sprintf( @@ -112,4 +112,21 @@ protected function execute(InputInterface $input, OutputInterface $output): int return 0; } + + private function checkMethod(OutputInterface $output, object $obj, string $methodName = 'clearRevoked'): bool + { + $exists = method_exists($obj, $methodName); + + if (!$exists) { + $output->writeln( + sprintf( + 'Method "%s:%s()" will be required in the next release. Skipping for now...', + get_class($obj), + $methodName + ) + ); + } + + return $exists; + } } diff --git a/Manager/AccessTokenManagerInterface.php b/Manager/AccessTokenManagerInterface.php index def25615..31f1158b 100644 --- a/Manager/AccessTokenManagerInterface.php +++ b/Manager/AccessTokenManagerInterface.php @@ -7,7 +7,7 @@ use Trikoder\Bundle\OAuth2Bundle\Model\AccessToken; /** - * @method int clearRevoked() + * @method int clearRevoked() not defining this method is deprecated since version 3.2 */ interface AccessTokenManagerInterface { diff --git a/Manager/AuthorizationCodeManagerInterface.php b/Manager/AuthorizationCodeManagerInterface.php index 9454d595..c39c78df 100644 --- a/Manager/AuthorizationCodeManagerInterface.php +++ b/Manager/AuthorizationCodeManagerInterface.php @@ -6,6 +6,9 @@ use Trikoder\Bundle\OAuth2Bundle\Model\AuthorizationCode; +/** + * @method int clearRevoked() not defining this method is deprecated since version 3.2 + */ interface AuthorizationCodeManagerInterface { public function find(string $identifier): ?AuthorizationCode; diff --git a/Manager/Doctrine/AccessTokenManager.php b/Manager/Doctrine/AccessTokenManager.php index 4508544d..40ab3f7d 100644 --- a/Manager/Doctrine/AccessTokenManager.php +++ b/Manager/Doctrine/AccessTokenManager.php @@ -52,7 +52,7 @@ public function clearRevoked(): int { return $this->entityManager->createQueryBuilder() ->delete(AccessToken::class, 'at') - ->where('at.revoked = 1') + ->where('at.revoked = true') ->getQuery() ->execute(); } diff --git a/Manager/Doctrine/AuthorizationCodeManager.php b/Manager/Doctrine/AuthorizationCodeManager.php index 871a76dd..766800af 100644 --- a/Manager/Doctrine/AuthorizationCodeManager.php +++ b/Manager/Doctrine/AuthorizationCodeManager.php @@ -51,8 +51,8 @@ public function clearExpired(): int public function clearRevoked(): int { return $this->entityManager->createQueryBuilder() - ->delete(AuthorizationCode::class, 'at') - ->where('at.revoked = 1') + ->delete(AuthorizationCode::class, 'ac') + ->where('ac.revoked = true') ->getQuery() ->execute(); } diff --git a/Manager/Doctrine/RefreshTokenManager.php b/Manager/Doctrine/RefreshTokenManager.php index 00d72ca2..29e4b6ca 100644 --- a/Manager/Doctrine/RefreshTokenManager.php +++ b/Manager/Doctrine/RefreshTokenManager.php @@ -51,8 +51,8 @@ public function clearExpired(): int public function clearRevoked(): int { return $this->entityManager->createQueryBuilder() - ->delete(RefreshToken::class, 'at') - ->where('at.revoked = 1') + ->delete(RefreshToken::class, 'rt') + ->where('rt.revoked = true') ->getQuery() ->execute(); } diff --git a/Manager/InMemory/AuthorizationCodeManager.php b/Manager/InMemory/AuthorizationCodeManager.php index 31ec7271..8e0bff9c 100644 --- a/Manager/InMemory/AuthorizationCodeManager.php +++ b/Manager/InMemory/AuthorizationCodeManager.php @@ -41,10 +41,10 @@ public function clearRevoked(): int { $count = \count($this->authorizationCodes); - $this->accessTokens = array_filter($this->authorizationCodes, static function (AuthorizationCode $accessToken): bool { - return !$accessToken->isRevoked(); + $this->authorizationCodes = array_filter($this->authorizationCodes, static function (AuthorizationCode $authorizationCode): bool { + return !$authorizationCode->isRevoked(); }); - return $count - \count($this->accessTokens); + return $count - \count($this->authorizationCodes); } } diff --git a/Manager/InMemory/RefreshTokenManager.php b/Manager/InMemory/RefreshTokenManager.php index 766d27b2..864db995 100644 --- a/Manager/InMemory/RefreshTokenManager.php +++ b/Manager/InMemory/RefreshTokenManager.php @@ -47,10 +47,10 @@ public function clearRevoked(): int { $count = \count($this->refreshTokens); - $this->accessTokens = array_filter($this->refreshTokens, static function (RefreshToken $accessToken): bool { - return !$accessToken->isRevoked(); + $this->refreshTokens = array_filter($this->refreshTokens, static function (RefreshToken $refreshToken): bool { + return !$refreshToken->isRevoked(); }); - return $count - \count($this->accessTokens); + return $count - \count($this->refreshTokens); } } diff --git a/Manager/RefreshTokenManagerInterface.php b/Manager/RefreshTokenManagerInterface.php index 1a4729d8..b1025e2c 100644 --- a/Manager/RefreshTokenManagerInterface.php +++ b/Manager/RefreshTokenManagerInterface.php @@ -6,6 +6,9 @@ use Trikoder\Bundle\OAuth2Bundle\Model\RefreshToken; +/** + * @method int clearRevoked() not defining this method is deprecated since version 3.2 + */ interface RefreshTokenManagerInterface { public function find(string $identifier): ?RefreshToken; diff --git a/Tests/Acceptance/ClearRevokedTokensCommandTest.php b/Tests/Acceptance/ClearRevokedTokensCommandTest.php index d7678571..6080bebf 100644 --- a/Tests/Acceptance/ClearRevokedTokensCommandTest.php +++ b/Tests/Acceptance/ClearRevokedTokensCommandTest.php @@ -110,7 +110,7 @@ public function testClearRevokedAccessTokens(): void ); } - public function testClearExpiredRefreshTokens(): void + public function testClearRevokedRefreshTokens(): void { $output = $this->executeCommand([ '--refresh-tokens' => true, @@ -139,7 +139,7 @@ public function testClearExpiredRefreshTokens(): void ); } - public function testClearExpiredAuthCodes(): void + public function testClearRevokedAuthCodes(): void { $output = $this->executeCommand([ '--auth-codes' => true, diff --git a/Tests/Acceptance/DoctrineAccessTokenManagerTest.php b/Tests/Acceptance/DoctrineAccessTokenManagerTest.php index c961d1c2..7425e050 100644 --- a/Tests/Acceptance/DoctrineAccessTokenManagerTest.php +++ b/Tests/Acceptance/DoctrineAccessTokenManagerTest.php @@ -31,7 +31,12 @@ public function testClearExpired(): void timecop_freeze(new DateTimeImmutable()); try { - $testData = $this->buildClearExpiredTestData($client); + $testData = $this->buildTestData( + $client, + function (array $item): bool { + return !$item['expired']; + } + ); /** @var AccessToken $token */ foreach ($testData['input'] as $token) { @@ -49,36 +54,149 @@ public function testClearExpired(): void ); } - private function buildClearExpiredTestData(Client $client): array + public function testClearRevoked(): void { - $validAccessTokens = [ - $this->buildAccessToken('1111', '+1 day', $client), - $this->buildAccessToken('2222', '+1 hour', $client), - $this->buildAccessToken('3333', '+1 second', $client), - $this->buildAccessToken('4444', 'now', $client), - ]; + /** @var EntityManagerInterface $em */ + $em = $this->client->getContainer()->get('doctrine.orm.entity_manager'); - $expiredAccessTokens = [ - $this->buildAccessToken('5555', '-1 day', $client), - $this->buildAccessToken('6666', '-1 hour', $client), - $this->buildAccessToken('7777', '-1 second', $client), - ]; + $doctrineAccessTokenManager = new DoctrineAccessTokenManager($em); + + $client = new Client('client', 'secret'); + $em->persist($client); + $em->flush(); + + timecop_freeze(new DateTimeImmutable()); + + try { + $testData = $this->buildTestData( + $client, + function (array $item): bool { + return !$item['revoked']; + } + ); + + /** @var AccessToken $token */ + foreach ($testData['input'] as $token) { + $doctrineAccessTokenManager->save($token); + } + + $this->assertSame(4, $doctrineAccessTokenManager->clearRevoked()); + } finally { + timecop_return(); + } + + $this->assertSame( + $testData['output'], + $em->getRepository(AccessToken::class)->findBy([], ['identifier' => 'ASC']) + ); + } + private function getData(): array + { return [ - 'input' => array_merge($validAccessTokens, $expiredAccessTokens), - 'output' => $validAccessTokens, + [ + 'identifier' => '1111', + 'dateOffset' => '+1 day', + 'revoked' => true, + 'expired' => false, + ], + [ + 'identifier' => '2222', + 'dateOffset' => '+1 hour', + 'revoked' => false, + 'expired' => false, + ], + [ + 'identifier' => '3333', + 'dateOffset' => '+1 second', + 'revoked' => true, + 'expired' => false, + ], + [ + 'identifier' => '4444', + 'dateOffset' => 'now', + 'revoked' => false, + 'expired' => false, + ], + [ + 'identifier' => '5555', + 'dateOffset' => '-1 day', + 'revoked' => true, + 'expired' => true, + ], + [ + 'identifier' => '6666', + 'dateOffset' => '-1 hour', + 'revoked' => false, + 'expired' => true, + ], + [ + 'identifier' => '7777', + 'dateOffset' => '-1 second', + 'revoked' => true, + 'expired' => true, + ] ]; } - private function buildAccessToken(string $identifier, string $modify, Client $client): AccessToken + private function buildTestData(Client $client, callable $successFunction): array + { + $response = []; + foreach ($this->getData() as $item) { + $identifier = $item['identifier']; + $accessToken = $this->buildAccessToken( + $client, + $identifier, + $item['dateOffset'], + $item['revoked'] + ); + $response['input'][] = $accessToken; + + if ($successFunction($item)) { + $response['output'][] = $accessToken; + } + } + + return $response; + } + + private function buildTestDataWithRefreshToken(Client $client, callable $successFunction): array + { + $response = []; + foreach ($this->getData() as $item) { + $identifier = $item['identifier']; + $accessToken = $this->buildRefreshToken( + $client, + $identifier, + $item['dateOffset'], + $item['revoked'] + ); + $response['input'][] = $accessToken; + + if ($successFunction($item)) { + $response['output'][] = $accessToken; + } + } + + return $response; + } + + + private function buildAccessToken(Client $client, string $identifier, string $modify, bool $revoked = false): AccessToken { - return new AccessToken( + $accessToken = new AccessToken( $identifier, new DateTimeImmutable($modify), $client, null, [] ); + + if ($revoked) { + $accessToken->revoke(); + } + + return $accessToken; } public function testClearExpiredWithRefreshToken(): void @@ -94,7 +212,12 @@ public function testClearExpiredWithRefreshToken(): void timecop_freeze(new DateTimeImmutable()); try { - $testData = $this->buildClearExpiredTestDataWithRefreshToken($client); + $testData = $this->buildTestDataWithRefreshToken( + $client, + function (array $item): bool { + return $item['expired']; + } + ); /** @var RefreshToken $token */ foreach ($testData['input'] as $token) { @@ -109,45 +232,79 @@ public function testClearExpiredWithRefreshToken(): void timecop_return(); } + $em->clear(); + + $mapFunction = function (RefreshToken $refreshToken): string { + return $refreshToken->getIdentifier(); + }; + $this->assertSame( - $testData['output'], - $em->getRepository(RefreshToken::class)->findBy(['accessToken' => null], ['identifier' => 'ASC']) + array_map($mapFunction, $testData['output']), + array_map($mapFunction, $em->getRepository(RefreshToken::class)->findBy(['accessToken' => null], ['identifier' => 'ASC'])) ); } - private function buildClearExpiredTestDataWithRefreshToken(Client $client): array + public function testClearRevokedWithRefreshToken(): void { - $validRefreshTokens = [ - $this->buildRefreshToken('1111', '+1 day', $client), - $this->buildRefreshToken('2222', '+1 hour', $client), - $this->buildRefreshToken('3333', '+1 second', $client), - $this->buildRefreshToken('4444', 'now', $client), - ]; + /** @var EntityManagerInterface $em */ + $em = $this->client->getContainer()->get('doctrine.orm.entity_manager'); + $doctrineAccessTokenManager = new DoctrineAccessTokenManager($em); - $expiredRefreshTokens = [ - $this->buildRefreshToken('5555', '-1 day', $client), - $this->buildRefreshToken('6666', '-1 hour', $client), - $this->buildRefreshToken('7777', '-1 second', $client), - ]; + $client = new Client('client', 'secret'); + $em->persist($client); + $em->flush(); - return [ - 'input' => array_merge($validRefreshTokens, $expiredRefreshTokens), - 'output' => $expiredRefreshTokens, - ]; + timecop_freeze(new DateTimeImmutable()); + + try { + $testData = $this->buildTestDataWithRefreshToken( + $client, + function (array $item): bool { + return !$item['revoked']; + } + ); + + /** @var RefreshToken $token */ + foreach ($testData['input'] as $token) { + $doctrineAccessTokenManager->save($token->getAccessToken()); + $em->persist($token); + } + + $em->flush(); + + $this->assertSame(4, $doctrineAccessTokenManager->clearRevoked()); + } finally { + timecop_return(); + } + + $savedData = $em->getRepository(RefreshToken::class)->findBy(['revoked' => false], ['identifier' => 'ASC']); + + $this->assertSame( + $testData['output'], + $savedData + ); } - private function buildRefreshToken(string $identifier, string $modify, Client $client): RefreshToken + private function buildRefreshToken(Client $client, string $identifier, string $modify, bool $revoked): RefreshToken { - return new RefreshToken( + $accessToken = new AccessToken( + $identifier, + new DateTimeImmutable($modify), + $client, + null, + [] + ); + $refreshToken = new RefreshToken( $identifier, new DateTimeImmutable('+1 day'), - new AccessToken( - $identifier, - new DateTimeImmutable($modify), - $client, - null, - [] - ) + $accessToken ); + + if ($revoked) { + $refreshToken->revoke(); + $accessToken->revoke(); + } + + return $refreshToken; } } diff --git a/Tests/Acceptance/DoctrineAuthCodeManagerTest.php b/Tests/Acceptance/DoctrineAuthCodeManagerTest.php index bda19680..977b68bc 100644 --- a/Tests/Acceptance/DoctrineAuthCodeManagerTest.php +++ b/Tests/Acceptance/DoctrineAuthCodeManagerTest.php @@ -29,7 +29,12 @@ public function testClearExpired(): void timecop_freeze(new DateTimeImmutable()); try { - $testData = $this->buildClearExpiredTestData($client); + $testData = $this->buildTestData( + $client, + function (array $item): bool { + return !$item['expired']; + } + ); /** @var AuthorizationCode $authCode */ foreach ($testData['input'] as $authCode) { @@ -49,35 +54,127 @@ public function testClearExpired(): void ); } - private function buildClearExpiredTestData(Client $client): array + public function testClearRevoked(): void { - $validAuthCodes = [ - $this->buildAuthCode('1111', '+1 day', $client), - $this->buildAuthCode('2222', '+1 hour', $client), - $this->buildAuthCode('3333', '+1 second', $client), - $this->buildAuthCode('4444', 'now', $client), - ]; + /** @var EntityManagerInterface $em */ + $em = $this->client->getContainer()->get('doctrine.orm.entity_manager'); - $expiredAuthCodes = [ - $this->buildAuthCode('5555', '-1 day', $client), - $this->buildAuthCode('6666', '-1 hour', $client), - $this->buildAuthCode('7777', '-1 second', $client), - ]; + $doctrineAuthCodeManager = new DoctrineAuthCodeManager($em); + + $client = new Client('client', 'secret'); + $em->persist($client); + + timecop_freeze(new DateTimeImmutable()); + try { + $testData = $this->buildTestData( + $client, + function (array $item): bool { + return !$item['revoked']; + } + ); + + /** @var AuthorizationCode $authCode */ + foreach ($testData['input'] as $authCode) { + $doctrineAuthCodeManager->save($authCode); + } + + $em->flush(); + + $this->assertSame(4, $doctrineAuthCodeManager->clearRevoked()); + } finally { + timecop_return(); + } + + $this->assertSame( + $testData['output'], + $em->getRepository(AuthorizationCode::class)->findBy([], ['identifier' => 'ASC']) + ); + } + + private function getData(): array + { return [ - 'input' => array_merge($validAuthCodes, $expiredAuthCodes), - 'output' => $validAuthCodes, + [ + 'identifier' => '1111', + 'dateOffset' => '+1 day', + 'revoked' => true, + 'expired' => false, + ], + [ + 'identifier' => '2222', + 'dateOffset' => '+1 hour', + 'revoked' => false, + 'expired' => false, + ], + [ + 'identifier' => '3333', + 'dateOffset' => '+1 second', + 'revoked' => true, + 'expired' => false, + ], + [ + 'identifier' => '4444', + 'dateOffset' => 'now', + 'revoked' => false, + 'expired' => false, + ], + [ + 'identifier' => '5555', + 'dateOffset' => '-1 day', + 'revoked' => true, + 'expired' => true, + ], + [ + 'identifier' => '6666', + 'dateOffset' => '-1 hour', + 'revoked' => false, + 'expired' => true, + ], + [ + 'identifier' => '7777', + 'dateOffset' => '-1 second', + 'revoked' => true, + 'expired' => true, + ] ]; } - private function buildAuthCode(string $identifier, string $modify, Client $client): AuthorizationCode + private function buildTestData(Client $client, callable $successFunction): array { - return new AuthorizationCode( + $response = []; + foreach ($this->getData() as $item) { + $identifier = $item['identifier']; + $accessToken = $this->buildAuthCode( + $client, + $identifier, + $item['dateOffset'], + $item['revoked'] + ); + $response['input'][] = $accessToken; + + if ($successFunction($item)) { + $response['output'][] = $accessToken; + } + } + + return $response; + } + + private function buildAuthCode(Client $client, string $identifier, string $modify, bool $revoked): AuthorizationCode + { + $authorizationCode = new AuthorizationCode( $identifier, new DateTimeImmutable($modify), $client, null, [] ); + + if ($revoked) { + $authorizationCode->revoke(); + } + + return $authorizationCode; } } diff --git a/Tests/Acceptance/DoctrineRefreshTokenManagerTest.php b/Tests/Acceptance/DoctrineRefreshTokenManagerTest.php index a3359d87..b0b326e6 100644 --- a/Tests/Acceptance/DoctrineRefreshTokenManagerTest.php +++ b/Tests/Acceptance/DoctrineRefreshTokenManagerTest.php @@ -31,7 +31,12 @@ public function testClearExpired(): void timecop_freeze(new DateTimeImmutable()); try { - $testData = $this->buildClearExpiredTestData($client); + $testData = $this->buildTestData( + $client, + function (array $item): bool { + return !$item['expired']; + } + ); /** @var RefreshToken $token */ foreach ($testData['input'] as $token) { @@ -52,30 +57,118 @@ public function testClearExpired(): void ); } - private function buildClearExpiredTestData(Client $client): array + public function testClearRevoked(): void { - $validRefreshTokens = [ - $this->buildRefreshToken('1111', '+1 day', $client), - $this->buildRefreshToken('2222', '+1 hour', $client), - $this->buildRefreshToken('3333', '+1 second', $client), - $this->buildRefreshToken('4444', 'now', $client), - ]; + /** @var EntityManagerInterface $em */ + $em = $this->client->getContainer()->get('doctrine.orm.entity_manager'); - $expiredRefreshTokens = [ - $this->buildRefreshToken('5555', '-1 day', $client), - $this->buildRefreshToken('6666', '-1 hour', $client), - $this->buildRefreshToken('7777', '-1 second', $client), - ]; + $doctrineRefreshTokenManager = new DoctrineRefreshTokenManager($em); + + $client = new Client('client', 'secret'); + $em->persist($client); + $em->flush(); + + timecop_freeze(new DateTimeImmutable()); + + try { + $testData = $this->buildTestData( + $client, + function (array $item): bool { + return !$item['revoked']; + } + ); + + /** @var RefreshToken $token */ + foreach ($testData['input'] as $token) { + $em->persist($token->getAccessToken()); + $doctrineRefreshTokenManager->save($token); + } + + $em->flush(); + $this->assertSame(4, $doctrineRefreshTokenManager->clearRevoked()); + } finally { + timecop_return(); + } + + $this->assertSame( + $testData['output'], + $em->getRepository(RefreshToken::class)->findBy([], ['identifier' => 'ASC']) + ); + } + + private function getData(): array + { return [ - 'input' => array_merge($validRefreshTokens, $expiredRefreshTokens), - 'output' => $validRefreshTokens, + [ + 'identifier' => '1111', + 'dateOffset' => '+1 day', + 'revoked' => true, + 'expired' => false, + ], + [ + 'identifier' => '2222', + 'dateOffset' => '+1 hour', + 'revoked' => false, + 'expired' => false, + ], + [ + 'identifier' => '3333', + 'dateOffset' => '+1 second', + 'revoked' => true, + 'expired' => false, + ], + [ + 'identifier' => '4444', + 'dateOffset' => 'now', + 'revoked' => false, + 'expired' => false, + ], + [ + 'identifier' => '5555', + 'dateOffset' => '-1 day', + 'revoked' => true, + 'expired' => true, + ], + [ + 'identifier' => '6666', + 'dateOffset' => '-1 hour', + 'revoked' => false, + 'expired' => true, + ], + [ + 'identifier' => '7777', + 'dateOffset' => '-1 second', + 'revoked' => true, + 'expired' => true, + ] ]; } - private function buildRefreshToken(string $identifier, string $modify, Client $client): RefreshToken + private function buildTestData(Client $client, callable $successFunction): array { - return new RefreshToken( + $response = []; + foreach ($this->getData() as $item) { + $identifier = $item['identifier']; + $accessToken = $this->buildRefreshToken( + $client, + $identifier, + $item['dateOffset'], + $item['revoked'] + ); + $response['input'][] = $accessToken; + + if ($successFunction($item)) { + $response['output'][] = $accessToken; + } + } + + return $response; + } + + private function buildRefreshToken(Client $client, string $identifier, string $modify, bool $revoked): RefreshToken + { + $refreshToken = new RefreshToken( $identifier, new DateTimeImmutable($modify), new AccessToken( @@ -86,5 +179,11 @@ private function buildRefreshToken(string $identifier, string $modify, Client $c [] ) ); + + if ($revoked) { + $refreshToken->revoke(); + } + + return $refreshToken; } } diff --git a/Tests/Unit/InMemoryAccessTokenManagerTest.php b/Tests/Unit/InMemoryAccessTokenManagerTest.php index c7986538..5b773a25 100644 --- a/Tests/Unit/InMemoryAccessTokenManagerTest.php +++ b/Tests/Unit/InMemoryAccessTokenManagerTest.php @@ -20,52 +20,134 @@ public function testClearExpired(): void timecop_freeze(new DateTimeImmutable()); try { - $testData = $this->buildClearExpiredTestData(); + $testData = $this->buildTestData( + function (array $item): bool { + return !$item['expired']; + } + ); foreach ($testData['input'] as $token) { $inMemoryAccessTokenManager->save($token); } $this->assertSame(3, $inMemoryAccessTokenManager->clearExpired()); + $this->compareOutput($testData['output'], $inMemoryAccessTokenManager); } finally { timecop_return(); } + } - $reflectionProperty = new ReflectionProperty(InMemoryAccessTokenManager::class, 'accessTokens'); - $reflectionProperty->setAccessible(true); + public function testClearRevoked(): void + { + $inMemoryAccessTokenManager = new InMemoryAccessTokenManager(); + + timecop_freeze(new DateTimeImmutable()); - $this->assertSame($testData['output'], $reflectionProperty->getValue($inMemoryAccessTokenManager)); + try { + $testData = $this->buildTestData( + function (array $item): bool { + return !$item['revoked']; + } + ); + + foreach ($testData['input'] as $token) { + $inMemoryAccessTokenManager->save($token); + } + + $this->assertSame(4, $inMemoryAccessTokenManager->clearRevoked()); + $this->compareOutput($testData['output'], $inMemoryAccessTokenManager); + } finally { + timecop_return(); + } } - private function buildClearExpiredTestData(): array + private function buildTestData(callable $successFunction): array { - $validAccessTokens = [ - '1111' => $this->buildAccessToken('1111', '+1 day'), - '2222' => $this->buildAccessToken('2222', '+1 hour'), - '3333' => $this->buildAccessToken('3333', '+1 second'), - '4444' => $this->buildAccessToken('4444', 'now'), + $data = [ + [ + 'identifier' => '1111', + 'dateOffset' => '+1 day', + 'revoked' => true, + 'expired' => false, + ], + [ + 'identifier' => '2222', + 'dateOffset' => '+1 hour', + 'revoked' => false, + 'expired' => false, + ], + [ + 'identifier' => '3333', + 'dateOffset' => '+1 second', + 'revoked' => true, + 'expired' => false, + ], + [ + 'identifier' => '4444', + 'dateOffset' => 'now', + 'revoked' => false, + 'expired' => false, + ], + [ + 'identifier' => '5555', + 'dateOffset' => '-1 day', + 'revoked' => true, + 'expired' => true, + ], + [ + 'identifier' => '6666', + 'dateOffset' => '-1 hour', + 'revoked' => false, + 'expired' => true, + ], + [ + 'identifier' => '7777', + 'dateOffset' => '-1 second', + 'revoked' => true, + 'expired' => true, + ] ]; - $expiredAccessTokens = [ - '5555' => $this->buildAccessToken('5555', '-1 day'), - '6666' => $this->buildAccessToken('6666', '-1 hour'), - '7777' => $this->buildAccessToken('7777', '-1 second'), - ]; + $response = []; + foreach ($data as $item) { + $identifier = $item['identifier']; + $accessToken = $this->buildAccessToken( + $identifier, + $item['dateOffset'], + $item['revoked'] + ); + $response['input'][$identifier] = $accessToken; - return [ - 'input' => $validAccessTokens + $expiredAccessTokens, - 'output' => $validAccessTokens, - ]; + if ($successFunction($item)) { + $response['output'][$identifier] = $accessToken; + } + } + + return $response; } - private function buildAccessToken(string $identifier, string $modify): AccessToken + private function buildAccessToken(string $identifier, string $modify, bool $revoked): AccessToken { - return new AccessToken( + $accessToken = new AccessToken( $identifier, new DateTimeImmutable($modify), new Client('client', 'secret'), null, [] ); + + if ($revoked) { + $accessToken->revoke(); + } + + return $accessToken; + } + + private function compareOutput(array $output, InMemoryAccessTokenManager $inMemoryAccessTokenManager): void + { + $reflectionProperty = new ReflectionProperty(InMemoryAccessTokenManager::class, 'accessTokens'); + $reflectionProperty->setAccessible(true); + + $this->assertSame($output, $reflectionProperty->getValue($inMemoryAccessTokenManager)); } } diff --git a/Tests/Unit/InMemoryAuthCodeManagerTest.php b/Tests/Unit/InMemoryAuthCodeManagerTest.php index df7ad6ba..e94dd181 100644 --- a/Tests/Unit/InMemoryAuthCodeManagerTest.php +++ b/Tests/Unit/InMemoryAuthCodeManagerTest.php @@ -20,53 +20,134 @@ public function testClearExpired(): void timecop_freeze(new DateTimeImmutable()); try { - $testData = $this->buildClearExpiredTestData(); + $testData = $this->buildTestData( + function (array $item): bool { + return !$item['expired']; + } + ); - /** @var AuthorizationCode $authCode */ - foreach ($testData['input'] as $authCode) { - $inMemoryAuthCodeManager->save($authCode); + foreach ($testData['input'] as $token) { + $inMemoryAuthCodeManager->save($token); } $this->assertSame(3, $inMemoryAuthCodeManager->clearExpired()); + $this->compareOutput($testData['output'], $inMemoryAuthCodeManager); } finally { timecop_return(); } + } - $reflectionProperty = new ReflectionProperty(InMemoryAuthCodeManager::class, 'authorizationCodes'); - $reflectionProperty->setAccessible(true); + public function testClearRevoked(): void + { + $inMemoryAuthCodeManager = new InMemoryAuthCodeManager(); + + timecop_freeze(new DateTimeImmutable()); - $this->assertSame($testData['output'], $reflectionProperty->getValue($inMemoryAuthCodeManager)); + try { + $testData = $this->buildTestData( + function (array $item): bool { + return !$item['revoked']; + } + ); + + foreach ($testData['input'] as $token) { + $inMemoryAuthCodeManager->save($token); + } + + $this->assertSame(4, $inMemoryAuthCodeManager->clearRevoked()); + $this->compareOutput($testData['output'], $inMemoryAuthCodeManager); + } finally { + timecop_return(); + } } - private function buildClearExpiredTestData(): array + private function buildTestData(callable $successFunction): array { - $validAuthCodes = [ - '1111' => $this->buildAuthCode('1111', '+1 day'), - '2222' => $this->buildAuthCode('2222', '+1 hour'), - '3333' => $this->buildAuthCode('3333', '+1 second'), - '4444' => $this->buildAuthCode('4444', 'now'), + $data = [ + [ + 'identifier' => '1111', + 'dateOffset' => '+1 day', + 'revoked' => true, + 'expired' => false, + ], + [ + 'identifier' => '2222', + 'dateOffset' => '+1 hour', + 'revoked' => false, + 'expired' => false, + ], + [ + 'identifier' => '3333', + 'dateOffset' => '+1 second', + 'revoked' => true, + 'expired' => false, + ], + [ + 'identifier' => '4444', + 'dateOffset' => 'now', + 'revoked' => false, + 'expired' => false, + ], + [ + 'identifier' => '5555', + 'dateOffset' => '-1 day', + 'revoked' => true, + 'expired' => true, + ], + [ + 'identifier' => '6666', + 'dateOffset' => '-1 hour', + 'revoked' => false, + 'expired' => true, + ], + [ + 'identifier' => '7777', + 'dateOffset' => '-1 second', + 'revoked' => true, + 'expired' => true, + ] ]; - $expiredAuthCodes = [ - '5555' => $this->buildAuthCode('5555', '-1 day'), - '6666' => $this->buildAuthCode('6666', '-1 hour'), - '7777' => $this->buildAuthCode('7777', '-1 second'), - ]; + $response = []; + foreach ($data as $item) { + $identifier = $item['identifier']; + $AuthCode = $this->buildAuthCode( + $identifier, + $item['dateOffset'], + $item['revoked'] + ); + $response['input'][$identifier] = $AuthCode; - return [ - 'input' => $validAuthCodes + $expiredAuthCodes, - 'output' => $validAuthCodes, - ]; + if ($successFunction($item)) { + $response['output'][$identifier] = $AuthCode; + } + } + + return $response; } - private function buildAuthCode(string $identifier, string $modify): AuthorizationCode + private function buildAuthCode(string $identifier, string $modify, bool $revoked): AuthorizationCode { - return new AuthorizationCode( + $authorizationCode = new AuthorizationCode( $identifier, new DateTimeImmutable($modify), new Client('client', 'secret'), null, [] ); + + if ($revoked) { + $authorizationCode->revoke(); + } + + return $authorizationCode; + } + + private function compareOutput(array $output, InMemoryAuthCodeManager $inMemoryAuthCodeManager): void + { + $reflectionProperty = new ReflectionProperty(InMemoryAuthCodeManager::class, 'authorizationCodes'); + $reflectionProperty->setAccessible(true); + + $this->assertSame($output, $reflectionProperty->getValue($inMemoryAuthCodeManager)); } } diff --git a/Tests/Unit/InMemoryRefreshTokenManagerTest.php b/Tests/Unit/InMemoryRefreshTokenManagerTest.php index 62e958d9..4686ea7b 100644 --- a/Tests/Unit/InMemoryRefreshTokenManagerTest.php +++ b/Tests/Unit/InMemoryRefreshTokenManagerTest.php @@ -21,47 +21,115 @@ public function testClearExpired(): void timecop_freeze(new DateTimeImmutable()); try { - $testData = $this->buildClearExpiredTestData(); + $testData = $this->buildTestData( + function (array $item): bool { + return !$item['expired']; + } + ); foreach ($testData['input'] as $token) { $inMemoryRefreshTokenManager->save($token); } $this->assertSame(3, $inMemoryRefreshTokenManager->clearExpired()); + $this->compareOutput($testData['output'], $inMemoryRefreshTokenManager); } finally { timecop_return(); } + } - $reflectionProperty = new ReflectionProperty(InMemoryRefreshTokenManager::class, 'refreshTokens'); - $reflectionProperty->setAccessible(true); + public function testClearRevoked(): void + { + $inMemoryRefreshTokenManager = new InMemoryRefreshTokenManager(); + + timecop_freeze(new DateTimeImmutable()); + + try { + $testData = $this->buildTestData( + function (array $item): bool { + return !$item['revoked']; + } + ); + + foreach ($testData['input'] as $token) { + $inMemoryRefreshTokenManager->save($token); + } - $this->assertSame($testData['output'], $reflectionProperty->getValue($inMemoryRefreshTokenManager)); + $this->assertSame(4, $inMemoryRefreshTokenManager->clearRevoked()); + $this->compareOutput($testData['output'], $inMemoryRefreshTokenManager); + } finally { + timecop_return(); + } } - private function buildClearExpiredTestData(): array + private function buildTestData(callable $successFunction): array { - $validRefreshTokens = [ - '1111' => $this->buildRefreshToken('1111', '+1 day'), - '2222' => $this->buildRefreshToken('2222', '+1 hour'), - '3333' => $this->buildRefreshToken('3333', '+1 second'), - '4444' => $this->buildRefreshToken('4444', 'now'), + $data = [ + [ + 'identifier' => '1111', + 'dateOffset' => '+1 day', + 'revoked' => true, + 'expired' => false, + ], + [ + 'identifier' => '2222', + 'dateOffset' => '+1 hour', + 'revoked' => false, + 'expired' => false, + ], + [ + 'identifier' => '3333', + 'dateOffset' => '+1 second', + 'revoked' => true, + 'expired' => false, + ], + [ + 'identifier' => '4444', + 'dateOffset' => 'now', + 'revoked' => false, + 'expired' => false, + ], + [ + 'identifier' => '5555', + 'dateOffset' => '-1 day', + 'revoked' => true, + 'expired' => true, + ], + [ + 'identifier' => '6666', + 'dateOffset' => '-1 hour', + 'revoked' => false, + 'expired' => true, + ], + [ + 'identifier' => '7777', + 'dateOffset' => '-1 second', + 'revoked' => true, + 'expired' => true, + ] ]; - $expiredRefreshTokens = [ - '5555' => $this->buildRefreshToken('5555', '-1 day'), - '6666' => $this->buildRefreshToken('6666', '-1 hour'), - '7777' => $this->buildRefreshToken('7777', '-1 second'), - ]; + $response = []; + foreach ($data as $item) { + $identifier = $item['identifier']; + $RefreshToken = $this->buildRefreshToken( + $identifier, + $item['dateOffset'], + $item['revoked'] + ); + $response['input'][$identifier] = $RefreshToken; - return [ - 'input' => $validRefreshTokens + $expiredRefreshTokens, - 'output' => $validRefreshTokens, - ]; + if ($successFunction($item)) { + $response['output'][$identifier] = $RefreshToken; + } + } + + return $response; } - private function buildRefreshToken(string $identifier, string $modify): RefreshToken + private function buildRefreshToken(string $identifier, string $modify, bool $revoked): RefreshToken { - return new RefreshToken( + $refreshToken = new RefreshToken( $identifier, new DateTimeImmutable($modify), new AccessToken( @@ -72,5 +140,19 @@ private function buildRefreshToken(string $identifier, string $modify): RefreshT [] ) ); + + if ($revoked) { + $refreshToken->revoke(); + } + + return $refreshToken; + } + + private function compareOutput(array $output, InMemoryRefreshTokenManager $inMemoryRefreshTokenManager): void + { + $reflectionProperty = new ReflectionProperty(InMemoryRefreshTokenManager::class, 'refreshTokens'); + $reflectionProperty->setAccessible(true); + + $this->assertSame($output, $reflectionProperty->getValue($inMemoryRefreshTokenManager)); } } From fe2c4411c996d11c991613f5369204e677534ad0 Mon Sep 17 00:00:00 2001 From: Franjo Zadelj Date: Tue, 20 Oct 2020 11:42:35 +0200 Subject: [PATCH 4/9] Simplify test data definitions --- Command/ClearRevokedTokensCommand.php | 15 +- .../ClearRevokedTokensCommandTest.php | 9 - .../DoctrineAccessTokenManagerTest.php | 229 +++++++----------- .../DoctrineAuthCodeManagerTest.php | 131 ++++------ .../DoctrineRefreshTokenManagerTest.php | 133 ++++------ Tests/Unit/InMemoryAccessTokenManagerTest.php | 30 +-- Tests/Unit/InMemoryAuthCodeManagerTest.php | 36 ++- .../Unit/InMemoryRefreshTokenManagerTest.php | 36 ++- 8 files changed, 229 insertions(+), 390 deletions(-) diff --git a/Command/ClearRevokedTokensCommand.php b/Command/ClearRevokedTokensCommand.php index 86931433..75221b29 100644 --- a/Command/ClearRevokedTokensCommand.php +++ b/Command/ClearRevokedTokensCommand.php @@ -80,7 +80,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int $clearRevokedAuthCodes = true; } - if (true === $clearRevokedAccessTokens && $this->checkMethod($output, $this->accessTokenManager)) { + if (true === $clearRevokedAccessTokens && $this->clearRevokedMethodExists($output, $this->accessTokenManager)) { $affected = $this->accessTokenManager->clearRevoked(); $output->writeln( sprintf( @@ -90,7 +90,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int ); } - if (true === $clearRevokedRefreshTokens && $this->checkMethod($output, $this->refreshTokenManager)) { + if (true === $clearRevokedRefreshTokens && $this->clearRevokedMethodExists($output, $this->refreshTokenManager)) { $affected = $this->refreshTokenManager->clearRevoked(); $output->writeln( sprintf( @@ -100,7 +100,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int ); } - if (true === $clearRevokedAuthCodes && $this->checkMethod($output, $this->authorizationCodeManager)) { + if (true === $clearRevokedAuthCodes && $this->clearRevokedMethodExists($output, $this->authorizationCodeManager)) { $affected = $this->authorizationCodeManager->clearRevoked(); $output->writeln( sprintf( @@ -113,15 +113,16 @@ protected function execute(InputInterface $input, OutputInterface $output): int return 0; } - private function checkMethod(OutputInterface $output, object $obj, string $methodName = 'clearRevoked'): bool + private function clearRevokedMethodExists(OutputInterface $output, object $manager): bool { - $exists = method_exists($obj, $methodName); + $methodName = 'clearRevoked'; + $exists = method_exists($manager, $methodName); if (!$exists) { $output->writeln( sprintf( - 'Method "%s:%s()" will be required in the next release. Skipping for now...', - get_class($obj), + 'Method "%s:%s()" will be required in the next major release. Skipping for now...', + \get_class($manager), $methodName ) ); diff --git a/Tests/Acceptance/ClearRevokedTokensCommandTest.php b/Tests/Acceptance/ClearRevokedTokensCommandTest.php index 6080bebf..aab13ff1 100644 --- a/Tests/Acceptance/ClearRevokedTokensCommandTest.php +++ b/Tests/Acceptance/ClearRevokedTokensCommandTest.php @@ -22,8 +22,6 @@ protected function setUp(): void { parent::setUp(); - timecop_freeze(new \DateTimeImmutable()); - FixtureFactory::initializeFixtures( $this->client->getContainer()->get(ScopeManagerInterface::class), $this->client->getContainer()->get(ClientManagerInterface::class), @@ -33,13 +31,6 @@ protected function setUp(): void ); } - protected function tearDown(): void - { - timecop_return(); - - parent::tearDown(); - } - public function testClearRevokedAccessAndRefreshTokensAndAuthCodes(): void { $this->assertNotNull( diff --git a/Tests/Acceptance/DoctrineAccessTokenManagerTest.php b/Tests/Acceptance/DoctrineAccessTokenManagerTest.php index 7425e050..2fa6ccb1 100644 --- a/Tests/Acceptance/DoctrineAccessTokenManagerTest.php +++ b/Tests/Acceptance/DoctrineAccessTokenManagerTest.php @@ -31,12 +31,7 @@ public function testClearExpired(): void timecop_freeze(new DateTimeImmutable()); try { - $testData = $this->buildTestData( - $client, - function (array $item): bool { - return !$item['expired']; - } - ); + $testData = $this->buildClearExpiredTestData($client); /** @var AccessToken $token */ foreach ($testData['input'] as $token) { @@ -54,6 +49,27 @@ function (array $item): bool { ); } + private function buildClearExpiredTestData(Client $client): array + { + $validAccessTokens = [ + $this->buildAccessToken($client, '1111', '+1 day'), + $this->buildAccessToken($client, '2222', '+1 hour'), + $this->buildAccessToken($client, '3333', '+1 second'), + $this->buildAccessToken($client, '4444', 'now'), + ]; + + $expiredAccessTokens = [ + $this->buildAccessToken($client, '5555', '-1 day'), + $this->buildAccessToken($client, '6666', '-1 hour'), + $this->buildAccessToken($client, '7777', '-1 second'), + ]; + + return [ + 'input' => array_merge($validAccessTokens, $expiredAccessTokens), + 'output' => $validAccessTokens, + ]; + } + public function testClearRevoked(): void { /** @var EntityManagerInterface $em */ @@ -65,123 +81,40 @@ public function testClearRevoked(): void $em->persist($client); $em->flush(); - timecop_freeze(new DateTimeImmutable()); - - try { - $testData = $this->buildTestData( - $client, - function (array $item): bool { - return !$item['revoked']; - } - ); - - /** @var AccessToken $token */ - foreach ($testData['input'] as $token) { - $doctrineAccessTokenManager->save($token); - } + $testData = $this->buildClearRevokedTestData($client); - $this->assertSame(4, $doctrineAccessTokenManager->clearRevoked()); - } finally { - timecop_return(); + /** @var AccessToken $token */ + foreach ($testData['input'] as $token) { + $doctrineAccessTokenManager->save($token); } + $this->assertSame(2, $doctrineAccessTokenManager->clearRevoked()); + $this->assertSame( $testData['output'], $em->getRepository(AccessToken::class)->findBy([], ['identifier' => 'ASC']) ); } - private function getData(): array + private function buildClearRevokedTestData(Client $client): array { - return [ - [ - 'identifier' => '1111', - 'dateOffset' => '+1 day', - 'revoked' => true, - 'expired' => false, - ], - [ - 'identifier' => '2222', - 'dateOffset' => '+1 hour', - 'revoked' => false, - 'expired' => false, - ], - [ - 'identifier' => '3333', - 'dateOffset' => '+1 second', - 'revoked' => true, - 'expired' => false, - ], - [ - 'identifier' => '4444', - 'dateOffset' => 'now', - 'revoked' => false, - 'expired' => false, - ], - [ - 'identifier' => '5555', - 'dateOffset' => '-1 day', - 'revoked' => true, - 'expired' => true, - ], - [ - 'identifier' => '6666', - 'dateOffset' => '-1 hour', - 'revoked' => false, - 'expired' => true, - ], - [ - 'identifier' => '7777', - 'dateOffset' => '-1 second', - 'revoked' => true, - 'expired' => true, - ] + $validAccessTokens = [ + $this->buildAccessToken($client, '1111', '+1 day'), + $this->buildAccessToken($client, '2222', '-1 hour'), + $this->buildAccessToken($client, '3333', '+1 second'), ]; - } - - private function buildTestData(Client $client, callable $successFunction): array - { - $response = []; - foreach ($this->getData() as $item) { - $identifier = $item['identifier']; - $accessToken = $this->buildAccessToken( - $client, - $identifier, - $item['dateOffset'], - $item['revoked'] - ); - $response['input'][] = $accessToken; - - if ($successFunction($item)) { - $response['output'][] = $accessToken; - } - } - return $response; - } - - private function buildTestDataWithRefreshToken(Client $client, callable $successFunction): array - { - $response = []; - foreach ($this->getData() as $item) { - $identifier = $item['identifier']; - $accessToken = $this->buildRefreshToken( - $client, - $identifier, - $item['dateOffset'], - $item['revoked'] - ); - $response['input'][] = $accessToken; - - if ($successFunction($item)) { - $response['output'][] = $accessToken; - } - } + $revokedAccessTokens = [ + $this->buildAccessToken($client, '5555', '-1 day', true), + $this->buildAccessToken($client, '6666', '+1 hour', true), + ]; - return $response; + return [ + 'input' => array_merge($validAccessTokens, $revokedAccessTokens), + 'output' => $validAccessTokens, + ]; } - private function buildAccessToken(Client $client, string $identifier, string $modify, bool $revoked = false): AccessToken { $accessToken = new AccessToken( @@ -212,12 +145,7 @@ public function testClearExpiredWithRefreshToken(): void timecop_freeze(new DateTimeImmutable()); try { - $testData = $this->buildTestDataWithRefreshToken( - $client, - function (array $item): bool { - return $item['expired']; - } - ); + $testData = $this->buildClearExpiredTestDataWithRefreshToken($client); /** @var RefreshToken $token */ foreach ($testData['input'] as $token) { @@ -232,8 +160,6 @@ function (array $item): bool { timecop_return(); } - $em->clear(); - $mapFunction = function (RefreshToken $refreshToken): string { return $refreshToken->getIdentifier(); }; @@ -244,6 +170,27 @@ function (array $item): bool { ); } + private function buildClearExpiredTestDataWithRefreshToken(Client $client): array + { + $validRefreshTokens = [ + $this->buildRefreshToken($client, '1111', '+1 day'), + $this->buildRefreshToken($client, '2222', '+1 hour'), + $this->buildRefreshToken($client, '3333', '+1 second'), + $this->buildRefreshToken($client, '4444', 'now'), + ]; + + $expiredRefreshTokens = [ + $this->buildRefreshToken($client, '5555', '-1 day'), + $this->buildRefreshToken($client, '6666', '-1 hour'), + $this->buildRefreshToken($client, '7777', '-1 second'), + ]; + + return [ + 'input' => array_merge($validRefreshTokens, $expiredRefreshTokens), + 'output' => $expiredRefreshTokens, + ]; + } + public function testClearRevokedWithRefreshToken(): void { /** @var EntityManagerInterface $em */ @@ -254,30 +201,19 @@ public function testClearRevokedWithRefreshToken(): void $em->persist($client); $em->flush(); - timecop_freeze(new DateTimeImmutable()); - - try { - $testData = $this->buildTestDataWithRefreshToken( - $client, - function (array $item): bool { - return !$item['revoked']; - } - ); + $testData = $this->buildClearRevokedTestDataWithRefreshToken($client); - /** @var RefreshToken $token */ - foreach ($testData['input'] as $token) { - $doctrineAccessTokenManager->save($token->getAccessToken()); - $em->persist($token); - } + /** @var RefreshToken $token */ + foreach ($testData['input'] as $token) { + $doctrineAccessTokenManager->save($token->getAccessToken()); + $em->persist($token); + } - $em->flush(); + $em->flush(); - $this->assertSame(4, $doctrineAccessTokenManager->clearRevoked()); - } finally { - timecop_return(); - } + $this->assertSame(2, $doctrineAccessTokenManager->clearRevoked()); - $savedData = $em->getRepository(RefreshToken::class)->findBy(['revoked' => false], ['identifier' => 'ASC']); + $savedData = $em->getRepository(RefreshToken::class)->findBy(['revoked' => true], ['identifier' => 'ASC']); $this->assertSame( $testData['output'], @@ -285,7 +221,26 @@ function (array $item): bool { ); } - private function buildRefreshToken(Client $client, string $identifier, string $modify, bool $revoked): RefreshToken + private function buildClearRevokedTestDataWithRefreshToken(Client $client): array + { + $validRefreshTokens = [ + $this->buildRefreshToken($client, '1111', '+1 day'), + $this->buildRefreshToken($client, '2222', '+1 hour'), + $this->buildRefreshToken($client, '3333', '+1 second'), + ]; + + $revokedRefreshTokens = [ + $this->buildRefreshToken($client, '5555', '-1 day', true), + $this->buildRefreshToken($client, '6666', '-1 hour', true), + ]; + + return [ + 'input' => array_merge($validRefreshTokens, $revokedRefreshTokens), + 'output' => $revokedRefreshTokens, + ]; + } + + private function buildRefreshToken(Client $client, string $identifier, string $modify, bool $revoked = false): RefreshToken { $accessToken = new AccessToken( $identifier, diff --git a/Tests/Acceptance/DoctrineAuthCodeManagerTest.php b/Tests/Acceptance/DoctrineAuthCodeManagerTest.php index 977b68bc..8fc6f05a 100644 --- a/Tests/Acceptance/DoctrineAuthCodeManagerTest.php +++ b/Tests/Acceptance/DoctrineAuthCodeManagerTest.php @@ -29,12 +29,7 @@ public function testClearExpired(): void timecop_freeze(new DateTimeImmutable()); try { - $testData = $this->buildTestData( - $client, - function (array $item): bool { - return !$item['expired']; - } - ); + $testData = $this->buildClearExpiredTestData($client); /** @var AuthorizationCode $authCode */ foreach ($testData['input'] as $authCode) { @@ -54,6 +49,27 @@ function (array $item): bool { ); } + private function buildClearExpiredTestData(Client $client): array + { + $validAuthCodes = [ + $this->buildAuthCode($client, '1111', '+1 day'), + $this->buildAuthCode($client, '2222', '+1 hour'), + $this->buildAuthCode($client, '3333', '+1 second'), + $this->buildAuthCode($client, '4444', 'now'), + ]; + + $expiredAuthCodes = [ + $this->buildAuthCode($client, '5555', '-1 day'), + $this->buildAuthCode($client, '6666', '-1 hour'), + $this->buildAuthCode($client, '7777', '-1 second'), + ]; + + return [ + 'input' => array_merge($validAuthCodes, $expiredAuthCodes), + 'output' => $validAuthCodes, + ]; + } + public function testClearRevoked(): void { /** @var EntityManagerInterface $em */ @@ -64,27 +80,16 @@ public function testClearRevoked(): void $client = new Client('client', 'secret'); $em->persist($client); - timecop_freeze(new DateTimeImmutable()); - - try { - $testData = $this->buildTestData( - $client, - function (array $item): bool { - return !$item['revoked']; - } - ); + $testData = $this->buildClearRevokedTestData($client); - /** @var AuthorizationCode $authCode */ - foreach ($testData['input'] as $authCode) { - $doctrineAuthCodeManager->save($authCode); - } + /** @var AuthorizationCode $authCode */ + foreach ($testData['input'] as $authCode) { + $doctrineAuthCodeManager->save($authCode); + } - $em->flush(); + $em->flush(); - $this->assertSame(4, $doctrineAuthCodeManager->clearRevoked()); - } finally { - timecop_return(); - } + $this->assertSame(2, $doctrineAuthCodeManager->clearRevoked()); $this->assertSame( $testData['output'], @@ -92,76 +97,26 @@ function (array $item): bool { ); } - private function getData(): array + private function buildClearRevokedTestData(Client $client): array { - return [ - [ - 'identifier' => '1111', - 'dateOffset' => '+1 day', - 'revoked' => true, - 'expired' => false, - ], - [ - 'identifier' => '2222', - 'dateOffset' => '+1 hour', - 'revoked' => false, - 'expired' => false, - ], - [ - 'identifier' => '3333', - 'dateOffset' => '+1 second', - 'revoked' => true, - 'expired' => false, - ], - [ - 'identifier' => '4444', - 'dateOffset' => 'now', - 'revoked' => false, - 'expired' => false, - ], - [ - 'identifier' => '5555', - 'dateOffset' => '-1 day', - 'revoked' => true, - 'expired' => true, - ], - [ - 'identifier' => '6666', - 'dateOffset' => '-1 hour', - 'revoked' => false, - 'expired' => true, - ], - [ - 'identifier' => '7777', - 'dateOffset' => '-1 second', - 'revoked' => true, - 'expired' => true, - ] + $validAuthCodes = [ + $this->buildAuthCode($client, '1111', '+1 day'), + $this->buildAuthCode($client, '2222', '-1 hour'), + $this->buildAuthCode($client, '3333', '+1 second'), ]; - } - private function buildTestData(Client $client, callable $successFunction): array - { - $response = []; - foreach ($this->getData() as $item) { - $identifier = $item['identifier']; - $accessToken = $this->buildAuthCode( - $client, - $identifier, - $item['dateOffset'], - $item['revoked'] - ); - $response['input'][] = $accessToken; - - if ($successFunction($item)) { - $response['output'][] = $accessToken; - } - } + $revokedAuthCodes = [ + $this->buildAuthCode($client, '5555', '-1 day', true), + $this->buildAuthCode($client, '6666', '+1 hour', true), + ]; - return $response; + return [ + 'input' => array_merge($validAuthCodes, $revokedAuthCodes), + 'output' => $validAuthCodes, + ]; } - private function buildAuthCode(Client $client, string $identifier, string $modify, bool $revoked): AuthorizationCode + private function buildAuthCode(Client $client, string $identifier, string $modify, bool $revoked = false): AuthorizationCode { $authorizationCode = new AuthorizationCode( $identifier, diff --git a/Tests/Acceptance/DoctrineRefreshTokenManagerTest.php b/Tests/Acceptance/DoctrineRefreshTokenManagerTest.php index b0b326e6..c837c36d 100644 --- a/Tests/Acceptance/DoctrineRefreshTokenManagerTest.php +++ b/Tests/Acceptance/DoctrineRefreshTokenManagerTest.php @@ -31,12 +31,7 @@ public function testClearExpired(): void timecop_freeze(new DateTimeImmutable()); try { - $testData = $this->buildTestData( - $client, - function (array $item): bool { - return !$item['expired']; - } - ); + $testData = $this->buildClearExpiredTestData($client); /** @var RefreshToken $token */ foreach ($testData['input'] as $token) { @@ -57,6 +52,27 @@ function (array $item): bool { ); } + private function buildClearExpiredTestData(Client $client): array + { + $validRefreshTokens = [ + $this->buildRefreshToken($client, '1111', '+1 day'), + $this->buildRefreshToken($client, '2222', '+1 hour'), + $this->buildRefreshToken($client, '3333', '+1 second'), + $this->buildRefreshToken($client, '4444', 'now'), + ]; + + $expiredRefreshTokens = [ + $this->buildRefreshToken($client, '5555', '-1 day'), + $this->buildRefreshToken($client, '6666', '-1 hour'), + $this->buildRefreshToken($client, '7777', '-1 second'), + ]; + + return [ + 'input' => array_merge($validRefreshTokens, $expiredRefreshTokens), + 'output' => $validRefreshTokens, + ]; + } + public function testClearRevoked(): void { /** @var EntityManagerInterface $em */ @@ -68,28 +84,17 @@ public function testClearRevoked(): void $em->persist($client); $em->flush(); - timecop_freeze(new DateTimeImmutable()); - - try { - $testData = $this->buildTestData( - $client, - function (array $item): bool { - return !$item['revoked']; - } - ); + $testData = $this->buildClearRevokedTestData($client); - /** @var RefreshToken $token */ - foreach ($testData['input'] as $token) { - $em->persist($token->getAccessToken()); - $doctrineRefreshTokenManager->save($token); - } + /** @var RefreshToken $token */ + foreach ($testData['input'] as $token) { + $em->persist($token->getAccessToken()); + $doctrineRefreshTokenManager->save($token); + } - $em->flush(); + $em->flush(); - $this->assertSame(4, $doctrineRefreshTokenManager->clearRevoked()); - } finally { - timecop_return(); - } + $this->assertSame(2, $doctrineRefreshTokenManager->clearRevoked()); $this->assertSame( $testData['output'], @@ -97,76 +102,26 @@ function (array $item): bool { ); } - private function getData(): array + private function buildClearRevokedTestData(Client $client): array { - return [ - [ - 'identifier' => '1111', - 'dateOffset' => '+1 day', - 'revoked' => true, - 'expired' => false, - ], - [ - 'identifier' => '2222', - 'dateOffset' => '+1 hour', - 'revoked' => false, - 'expired' => false, - ], - [ - 'identifier' => '3333', - 'dateOffset' => '+1 second', - 'revoked' => true, - 'expired' => false, - ], - [ - 'identifier' => '4444', - 'dateOffset' => 'now', - 'revoked' => false, - 'expired' => false, - ], - [ - 'identifier' => '5555', - 'dateOffset' => '-1 day', - 'revoked' => true, - 'expired' => true, - ], - [ - 'identifier' => '6666', - 'dateOffset' => '-1 hour', - 'revoked' => false, - 'expired' => true, - ], - [ - 'identifier' => '7777', - 'dateOffset' => '-1 second', - 'revoked' => true, - 'expired' => true, - ] + $validRefreshTokens = [ + $this->buildRefreshToken($client, '1111', '+1 day'), + $this->buildRefreshToken($client, '2222', '-1 hour'), + $this->buildRefreshToken($client, '3333', '+1 second'), ]; - } - private function buildTestData(Client $client, callable $successFunction): array - { - $response = []; - foreach ($this->getData() as $item) { - $identifier = $item['identifier']; - $accessToken = $this->buildRefreshToken( - $client, - $identifier, - $item['dateOffset'], - $item['revoked'] - ); - $response['input'][] = $accessToken; - - if ($successFunction($item)) { - $response['output'][] = $accessToken; - } - } + $revokedRefreshTokens = [ + $this->buildRefreshToken($client, '5555', '-1 day', true), + $this->buildRefreshToken($client, '6666', '+1 hour', true), + ]; - return $response; + return [ + 'input' => array_merge($validRefreshTokens, $revokedRefreshTokens), + 'output' => $validRefreshTokens, + ]; } - private function buildRefreshToken(Client $client, string $identifier, string $modify, bool $revoked): RefreshToken + private function buildRefreshToken(Client $client, string $identifier, string $modify, bool $revoked = false): RefreshToken { $refreshToken = new RefreshToken( $identifier, diff --git a/Tests/Unit/InMemoryAccessTokenManagerTest.php b/Tests/Unit/InMemoryAccessTokenManagerTest.php index 5b773a25..42542776 100644 --- a/Tests/Unit/InMemoryAccessTokenManagerTest.php +++ b/Tests/Unit/InMemoryAccessTokenManagerTest.php @@ -31,7 +31,7 @@ function (array $item): bool { } $this->assertSame(3, $inMemoryAccessTokenManager->clearExpired()); - $this->compareOutput($testData['output'], $inMemoryAccessTokenManager); + $this->assertManagerContainsExpectedData($testData['output'], $inMemoryAccessTokenManager); } finally { timecop_return(); } @@ -41,24 +41,18 @@ public function testClearRevoked(): void { $inMemoryAccessTokenManager = new InMemoryAccessTokenManager(); - timecop_freeze(new DateTimeImmutable()); - - try { - $testData = $this->buildTestData( - function (array $item): bool { - return !$item['revoked']; - } - ); - - foreach ($testData['input'] as $token) { - $inMemoryAccessTokenManager->save($token); + $testData = $this->buildTestData( + function (array $item): bool { + return !$item['revoked']; } + ); - $this->assertSame(4, $inMemoryAccessTokenManager->clearRevoked()); - $this->compareOutput($testData['output'], $inMemoryAccessTokenManager); - } finally { - timecop_return(); + foreach ($testData['input'] as $token) { + $inMemoryAccessTokenManager->save($token); } + + $this->assertSame(4, $inMemoryAccessTokenManager->clearRevoked()); + $this->assertManagerContainsExpectedData($testData['output'], $inMemoryAccessTokenManager); } private function buildTestData(callable $successFunction): array @@ -105,7 +99,7 @@ private function buildTestData(callable $successFunction): array 'dateOffset' => '-1 second', 'revoked' => true, 'expired' => true, - ] + ], ]; $response = []; @@ -143,7 +137,7 @@ private function buildAccessToken(string $identifier, string $modify, bool $revo return $accessToken; } - private function compareOutput(array $output, InMemoryAccessTokenManager $inMemoryAccessTokenManager): void + private function assertManagerContainsExpectedData(array $output, InMemoryAccessTokenManager $inMemoryAccessTokenManager): void { $reflectionProperty = new ReflectionProperty(InMemoryAccessTokenManager::class, 'accessTokens'); $reflectionProperty->setAccessible(true); diff --git a/Tests/Unit/InMemoryAuthCodeManagerTest.php b/Tests/Unit/InMemoryAuthCodeManagerTest.php index e94dd181..c1e83946 100644 --- a/Tests/Unit/InMemoryAuthCodeManagerTest.php +++ b/Tests/Unit/InMemoryAuthCodeManagerTest.php @@ -31,7 +31,7 @@ function (array $item): bool { } $this->assertSame(3, $inMemoryAuthCodeManager->clearExpired()); - $this->compareOutput($testData['output'], $inMemoryAuthCodeManager); + $this->assertManagerContainsExpectedData($testData['output'], $inMemoryAuthCodeManager); } finally { timecop_return(); } @@ -41,24 +41,18 @@ public function testClearRevoked(): void { $inMemoryAuthCodeManager = new InMemoryAuthCodeManager(); - timecop_freeze(new DateTimeImmutable()); - - try { - $testData = $this->buildTestData( - function (array $item): bool { - return !$item['revoked']; - } - ); - - foreach ($testData['input'] as $token) { - $inMemoryAuthCodeManager->save($token); + $testData = $this->buildTestData( + function (array $item): bool { + return !$item['revoked']; } + ); - $this->assertSame(4, $inMemoryAuthCodeManager->clearRevoked()); - $this->compareOutput($testData['output'], $inMemoryAuthCodeManager); - } finally { - timecop_return(); + foreach ($testData['input'] as $token) { + $inMemoryAuthCodeManager->save($token); } + + $this->assertSame(4, $inMemoryAuthCodeManager->clearRevoked()); + $this->assertManagerContainsExpectedData($testData['output'], $inMemoryAuthCodeManager); } private function buildTestData(callable $successFunction): array @@ -105,21 +99,21 @@ private function buildTestData(callable $successFunction): array 'dateOffset' => '-1 second', 'revoked' => true, 'expired' => true, - ] + ], ]; $response = []; foreach ($data as $item) { $identifier = $item['identifier']; - $AuthCode = $this->buildAuthCode( + $authCode = $this->buildAuthCode( $identifier, $item['dateOffset'], $item['revoked'] ); - $response['input'][$identifier] = $AuthCode; + $response['input'][$identifier] = $authCode; if ($successFunction($item)) { - $response['output'][$identifier] = $AuthCode; + $response['output'][$identifier] = $authCode; } } @@ -143,7 +137,7 @@ private function buildAuthCode(string $identifier, string $modify, bool $revoked return $authorizationCode; } - private function compareOutput(array $output, InMemoryAuthCodeManager $inMemoryAuthCodeManager): void + private function assertManagerContainsExpectedData(array $output, InMemoryAuthCodeManager $inMemoryAuthCodeManager): void { $reflectionProperty = new ReflectionProperty(InMemoryAuthCodeManager::class, 'authorizationCodes'); $reflectionProperty->setAccessible(true); diff --git a/Tests/Unit/InMemoryRefreshTokenManagerTest.php b/Tests/Unit/InMemoryRefreshTokenManagerTest.php index 4686ea7b..5483e6bc 100644 --- a/Tests/Unit/InMemoryRefreshTokenManagerTest.php +++ b/Tests/Unit/InMemoryRefreshTokenManagerTest.php @@ -32,7 +32,7 @@ function (array $item): bool { } $this->assertSame(3, $inMemoryRefreshTokenManager->clearExpired()); - $this->compareOutput($testData['output'], $inMemoryRefreshTokenManager); + $this->assertManagerContainsExpectedData($testData['output'], $inMemoryRefreshTokenManager); } finally { timecop_return(); } @@ -42,24 +42,18 @@ public function testClearRevoked(): void { $inMemoryRefreshTokenManager = new InMemoryRefreshTokenManager(); - timecop_freeze(new DateTimeImmutable()); - - try { - $testData = $this->buildTestData( - function (array $item): bool { - return !$item['revoked']; - } - ); - - foreach ($testData['input'] as $token) { - $inMemoryRefreshTokenManager->save($token); + $testData = $this->buildTestData( + function (array $item): bool { + return !$item['revoked']; } + ); - $this->assertSame(4, $inMemoryRefreshTokenManager->clearRevoked()); - $this->compareOutput($testData['output'], $inMemoryRefreshTokenManager); - } finally { - timecop_return(); + foreach ($testData['input'] as $token) { + $inMemoryRefreshTokenManager->save($token); } + + $this->assertSame(4, $inMemoryRefreshTokenManager->clearRevoked()); + $this->assertManagerContainsExpectedData($testData['output'], $inMemoryRefreshTokenManager); } private function buildTestData(callable $successFunction): array @@ -106,21 +100,21 @@ private function buildTestData(callable $successFunction): array 'dateOffset' => '-1 second', 'revoked' => true, 'expired' => true, - ] + ], ]; $response = []; foreach ($data as $item) { $identifier = $item['identifier']; - $RefreshToken = $this->buildRefreshToken( + $refreshToken = $this->buildRefreshToken( $identifier, $item['dateOffset'], $item['revoked'] ); - $response['input'][$identifier] = $RefreshToken; + $response['input'][$identifier] = $refreshToken; if ($successFunction($item)) { - $response['output'][$identifier] = $RefreshToken; + $response['output'][$identifier] = $refreshToken; } } @@ -148,7 +142,7 @@ private function buildRefreshToken(string $identifier, string $modify, bool $rev return $refreshToken; } - private function compareOutput(array $output, InMemoryRefreshTokenManager $inMemoryRefreshTokenManager): void + private function assertManagerContainsExpectedData(array $output, InMemoryRefreshTokenManager $inMemoryRefreshTokenManager): void { $reflectionProperty = new ReflectionProperty(InMemoryRefreshTokenManager::class, 'refreshTokens'); $reflectionProperty->setAccessible(true); From 0fc23d0b92409463289929ea796236e9b5520655 Mon Sep 17 00:00:00 2001 From: Franjo Zadelj Date: Thu, 22 Oct 2020 15:28:00 +0200 Subject: [PATCH 5/9] Simplify test data definitions --- .../DoctrineAccessTokenManagerTest.php | 54 ++++----- .../DoctrineAuthCodeManagerTest.php | 26 ++--- .../DoctrineRefreshTokenManagerTest.php | 26 ++--- Tests/Unit/InMemoryAccessTokenManagerTest.php | 109 ++++++------------ Tests/Unit/InMemoryAuthCodeManagerTest.php | 109 ++++++------------ .../Unit/InMemoryRefreshTokenManagerTest.php | 109 ++++++------------ 6 files changed, 167 insertions(+), 266 deletions(-) diff --git a/Tests/Acceptance/DoctrineAccessTokenManagerTest.php b/Tests/Acceptance/DoctrineAccessTokenManagerTest.php index 2fa6ccb1..723bb44c 100644 --- a/Tests/Acceptance/DoctrineAccessTokenManagerTest.php +++ b/Tests/Acceptance/DoctrineAccessTokenManagerTest.php @@ -52,16 +52,16 @@ public function testClearExpired(): void private function buildClearExpiredTestData(Client $client): array { $validAccessTokens = [ - $this->buildAccessToken($client, '1111', '+1 day'), - $this->buildAccessToken($client, '2222', '+1 hour'), - $this->buildAccessToken($client, '3333', '+1 second'), - $this->buildAccessToken($client, '4444', 'now'), + $this->buildAccessToken('1111', '+1 day', $client), + $this->buildAccessToken('2222', '+1 hour', $client), + $this->buildAccessToken('3333', '+1 second', $client), + $this->buildAccessToken('4444', 'now', $client), ]; $expiredAccessTokens = [ - $this->buildAccessToken($client, '5555', '-1 day'), - $this->buildAccessToken($client, '6666', '-1 hour'), - $this->buildAccessToken($client, '7777', '-1 second'), + $this->buildAccessToken('5555', '-1 day', $client), + $this->buildAccessToken('6666', '-1 hour', $client), + $this->buildAccessToken('7777', '-1 second', $client), ]; return [ @@ -99,14 +99,14 @@ public function testClearRevoked(): void private function buildClearRevokedTestData(Client $client): array { $validAccessTokens = [ - $this->buildAccessToken($client, '1111', '+1 day'), - $this->buildAccessToken($client, '2222', '-1 hour'), - $this->buildAccessToken($client, '3333', '+1 second'), + $this->buildAccessToken('1111', '+1 day', $client), + $this->buildAccessToken('2222', '-1 hour', $client), + $this->buildAccessToken('3333', '+1 second', $client), ]; $revokedAccessTokens = [ - $this->buildAccessToken($client, '5555', '-1 day', true), - $this->buildAccessToken($client, '6666', '+1 hour', true), + $this->buildAccessToken('5555', '-1 day', $client, true), + $this->buildAccessToken('6666', '+1 hour', $client, true), ]; return [ @@ -115,7 +115,7 @@ private function buildClearRevokedTestData(Client $client): array ]; } - private function buildAccessToken(Client $client, string $identifier, string $modify, bool $revoked = false): AccessToken + private function buildAccessToken(string $identifier, string $modify, Client $client, bool $revoked = false): AccessToken { $accessToken = new AccessToken( $identifier, @@ -160,7 +160,7 @@ public function testClearExpiredWithRefreshToken(): void timecop_return(); } - $mapFunction = function (RefreshToken $refreshToken): string { + $mapFunction = static function (RefreshToken $refreshToken): string { return $refreshToken->getIdentifier(); }; @@ -173,16 +173,16 @@ public function testClearExpiredWithRefreshToken(): void private function buildClearExpiredTestDataWithRefreshToken(Client $client): array { $validRefreshTokens = [ - $this->buildRefreshToken($client, '1111', '+1 day'), - $this->buildRefreshToken($client, '2222', '+1 hour'), - $this->buildRefreshToken($client, '3333', '+1 second'), - $this->buildRefreshToken($client, '4444', 'now'), + $this->buildRefreshToken('1111', '+1 day', $client), + $this->buildRefreshToken('2222', '+1 hour', $client), + $this->buildRefreshToken('3333', '+1 second', $client), + $this->buildRefreshToken('4444', 'now', $client), ]; $expiredRefreshTokens = [ - $this->buildRefreshToken($client, '5555', '-1 day'), - $this->buildRefreshToken($client, '6666', '-1 hour'), - $this->buildRefreshToken($client, '7777', '-1 second'), + $this->buildRefreshToken('5555', '-1 day', $client), + $this->buildRefreshToken('6666', '-1 hour', $client), + $this->buildRefreshToken('7777', '-1 second', $client), ]; return [ @@ -224,14 +224,14 @@ public function testClearRevokedWithRefreshToken(): void private function buildClearRevokedTestDataWithRefreshToken(Client $client): array { $validRefreshTokens = [ - $this->buildRefreshToken($client, '1111', '+1 day'), - $this->buildRefreshToken($client, '2222', '+1 hour'), - $this->buildRefreshToken($client, '3333', '+1 second'), + $this->buildRefreshToken('1111', '+1 day', $client), + $this->buildRefreshToken('2222', '+1 hour', $client), + $this->buildRefreshToken('3333', '+1 second', $client), ]; $revokedRefreshTokens = [ - $this->buildRefreshToken($client, '5555', '-1 day', true), - $this->buildRefreshToken($client, '6666', '-1 hour', true), + $this->buildRefreshToken('5555', '-1 day', $client, true), + $this->buildRefreshToken('6666', '-1 hour', $client, true), ]; return [ @@ -240,7 +240,7 @@ private function buildClearRevokedTestDataWithRefreshToken(Client $client): arra ]; } - private function buildRefreshToken(Client $client, string $identifier, string $modify, bool $revoked = false): RefreshToken + private function buildRefreshToken(string $identifier, string $modify, Client $client, bool $revoked = false): RefreshToken { $accessToken = new AccessToken( $identifier, diff --git a/Tests/Acceptance/DoctrineAuthCodeManagerTest.php b/Tests/Acceptance/DoctrineAuthCodeManagerTest.php index 8fc6f05a..520b07af 100644 --- a/Tests/Acceptance/DoctrineAuthCodeManagerTest.php +++ b/Tests/Acceptance/DoctrineAuthCodeManagerTest.php @@ -52,16 +52,16 @@ public function testClearExpired(): void private function buildClearExpiredTestData(Client $client): array { $validAuthCodes = [ - $this->buildAuthCode($client, '1111', '+1 day'), - $this->buildAuthCode($client, '2222', '+1 hour'), - $this->buildAuthCode($client, '3333', '+1 second'), - $this->buildAuthCode($client, '4444', 'now'), + $this->buildAuthCode('1111', '+1 day', $client), + $this->buildAuthCode('2222', '+1 hour', $client), + $this->buildAuthCode('3333', '+1 second', $client), + $this->buildAuthCode('4444', 'now', $client), ]; $expiredAuthCodes = [ - $this->buildAuthCode($client, '5555', '-1 day'), - $this->buildAuthCode($client, '6666', '-1 hour'), - $this->buildAuthCode($client, '7777', '-1 second'), + $this->buildAuthCode('5555', '-1 day', $client), + $this->buildAuthCode('6666', '-1 hour', $client), + $this->buildAuthCode('7777', '-1 second', $client), ]; return [ @@ -100,14 +100,14 @@ public function testClearRevoked(): void private function buildClearRevokedTestData(Client $client): array { $validAuthCodes = [ - $this->buildAuthCode($client, '1111', '+1 day'), - $this->buildAuthCode($client, '2222', '-1 hour'), - $this->buildAuthCode($client, '3333', '+1 second'), + $this->buildAuthCode('1111', '+1 day', $client), + $this->buildAuthCode('2222', '-1 hour', $client), + $this->buildAuthCode('3333', '+1 second', $client), ]; $revokedAuthCodes = [ - $this->buildAuthCode($client, '5555', '-1 day', true), - $this->buildAuthCode($client, '6666', '+1 hour', true), + $this->buildAuthCode('5555', '-1 day', $client, true), + $this->buildAuthCode('6666', '+1 hour', $client, true), ]; return [ @@ -116,7 +116,7 @@ private function buildClearRevokedTestData(Client $client): array ]; } - private function buildAuthCode(Client $client, string $identifier, string $modify, bool $revoked = false): AuthorizationCode + private function buildAuthCode(string $identifier, string $modify, Client $client, bool $revoked = false): AuthorizationCode { $authorizationCode = new AuthorizationCode( $identifier, diff --git a/Tests/Acceptance/DoctrineRefreshTokenManagerTest.php b/Tests/Acceptance/DoctrineRefreshTokenManagerTest.php index c837c36d..59500bf1 100644 --- a/Tests/Acceptance/DoctrineRefreshTokenManagerTest.php +++ b/Tests/Acceptance/DoctrineRefreshTokenManagerTest.php @@ -55,16 +55,16 @@ public function testClearExpired(): void private function buildClearExpiredTestData(Client $client): array { $validRefreshTokens = [ - $this->buildRefreshToken($client, '1111', '+1 day'), - $this->buildRefreshToken($client, '2222', '+1 hour'), - $this->buildRefreshToken($client, '3333', '+1 second'), - $this->buildRefreshToken($client, '4444', 'now'), + $this->buildRefreshToken('1111', '+1 day', $client), + $this->buildRefreshToken('2222', '+1 hour', $client), + $this->buildRefreshToken('3333', '+1 second', $client), + $this->buildRefreshToken('4444', 'now', $client), ]; $expiredRefreshTokens = [ - $this->buildRefreshToken($client, '5555', '-1 day'), - $this->buildRefreshToken($client, '6666', '-1 hour'), - $this->buildRefreshToken($client, '7777', '-1 second'), + $this->buildRefreshToken('5555', '-1 day', $client), + $this->buildRefreshToken('6666', '-1 hour', $client), + $this->buildRefreshToken('7777', '-1 second', $client), ]; return [ @@ -105,14 +105,14 @@ public function testClearRevoked(): void private function buildClearRevokedTestData(Client $client): array { $validRefreshTokens = [ - $this->buildRefreshToken($client, '1111', '+1 day'), - $this->buildRefreshToken($client, '2222', '-1 hour'), - $this->buildRefreshToken($client, '3333', '+1 second'), + $this->buildRefreshToken('1111', '+1 day', $client), + $this->buildRefreshToken('2222', '-1 hour', $client), + $this->buildRefreshToken('3333', '+1 second', $client), ]; $revokedRefreshTokens = [ - $this->buildRefreshToken($client, '5555', '-1 day', true), - $this->buildRefreshToken($client, '6666', '+1 hour', true), + $this->buildRefreshToken('5555', '-1 day', $client, true), + $this->buildRefreshToken('6666', '+1 hour', $client, true), ]; return [ @@ -121,7 +121,7 @@ private function buildClearRevokedTestData(Client $client): array ]; } - private function buildRefreshToken(Client $client, string $identifier, string $modify, bool $revoked = false): RefreshToken + private function buildRefreshToken(string $identifier, string $modify, Client $client, bool $revoked = false): RefreshToken { $refreshToken = new RefreshToken( $identifier, diff --git a/Tests/Unit/InMemoryAccessTokenManagerTest.php b/Tests/Unit/InMemoryAccessTokenManagerTest.php index 42542776..87af70df 100644 --- a/Tests/Unit/InMemoryAccessTokenManagerTest.php +++ b/Tests/Unit/InMemoryAccessTokenManagerTest.php @@ -20,11 +20,7 @@ public function testClearExpired(): void timecop_freeze(new DateTimeImmutable()); try { - $testData = $this->buildTestData( - function (array $item): bool { - return !$item['expired']; - } - ); + $testData = $this->buildClearExpiredTestData(); foreach ($testData['input'] as $token) { $inMemoryAccessTokenManager->save($token); @@ -37,90 +33,61 @@ function (array $item): bool { } } + private function buildClearExpiredTestData(): array + { + $validAccessTokens = [ + '1111' => $this->buildAccessToken('1111', '+1 day'), + '2222' => $this->buildAccessToken('2222', '+1 hour'), + '3333' => $this->buildAccessToken('3333', '+1 second'), + '4444' => $this->buildAccessToken('4444', 'now'), + ]; + + $expiredAccessTokens = [ + '5555' => $this->buildAccessToken('5555', '-1 day'), + '6666' => $this->buildAccessToken('6666', '-1 hour'), + '7777' => $this->buildAccessToken('7777', '-1 second'), + ]; + + return [ + 'input' => $validAccessTokens + $expiredAccessTokens, + 'output' => $validAccessTokens, + ]; + } + public function testClearRevoked(): void { $inMemoryAccessTokenManager = new InMemoryAccessTokenManager(); - $testData = $this->buildTestData( - function (array $item): bool { - return !$item['revoked']; - } - ); + $testData = $this->buildClearRevokedTestData(); foreach ($testData['input'] as $token) { $inMemoryAccessTokenManager->save($token); } - $this->assertSame(4, $inMemoryAccessTokenManager->clearRevoked()); + $this->assertSame(2, $inMemoryAccessTokenManager->clearRevoked()); $this->assertManagerContainsExpectedData($testData['output'], $inMemoryAccessTokenManager); } - private function buildTestData(callable $successFunction): array + private function buildClearRevokedTestData(): array { - $data = [ - [ - 'identifier' => '1111', - 'dateOffset' => '+1 day', - 'revoked' => true, - 'expired' => false, - ], - [ - 'identifier' => '2222', - 'dateOffset' => '+1 hour', - 'revoked' => false, - 'expired' => false, - ], - [ - 'identifier' => '3333', - 'dateOffset' => '+1 second', - 'revoked' => true, - 'expired' => false, - ], - [ - 'identifier' => '4444', - 'dateOffset' => 'now', - 'revoked' => false, - 'expired' => false, - ], - [ - 'identifier' => '5555', - 'dateOffset' => '-1 day', - 'revoked' => true, - 'expired' => true, - ], - [ - 'identifier' => '6666', - 'dateOffset' => '-1 hour', - 'revoked' => false, - 'expired' => true, - ], - [ - 'identifier' => '7777', - 'dateOffset' => '-1 second', - 'revoked' => true, - 'expired' => true, - ], + $validAccessTokens = [ + '1111' => $this->buildAccessToken('1111', '+1 day'), + '2222' => $this->buildAccessToken('2222', '-1 hour'), + '3333' => $this->buildAccessToken('3333', '+1 second'), ]; - $response = []; - foreach ($data as $item) { - $identifier = $item['identifier']; - $accessToken = $this->buildAccessToken( - $identifier, - $item['dateOffset'], - $item['revoked'] - ); - $response['input'][$identifier] = $accessToken; - - if ($successFunction($item)) { - $response['output'][$identifier] = $accessToken; - } - } + $revokedAccessTokens = [ + '5555' => $this->buildAccessToken('5555', '-1 day', true), + '6666' => $this->buildAccessToken('6666', '+1 hour', true), + ]; - return $response; + return [ + 'input' => $validAccessTokens + $revokedAccessTokens, + 'output' => $validAccessTokens, + ]; } - private function buildAccessToken(string $identifier, string $modify, bool $revoked): AccessToken + private function buildAccessToken(string $identifier, string $modify, bool $revoked = false): AccessToken { $accessToken = new AccessToken( $identifier, diff --git a/Tests/Unit/InMemoryAuthCodeManagerTest.php b/Tests/Unit/InMemoryAuthCodeManagerTest.php index c1e83946..f4cc61d4 100644 --- a/Tests/Unit/InMemoryAuthCodeManagerTest.php +++ b/Tests/Unit/InMemoryAuthCodeManagerTest.php @@ -20,11 +20,7 @@ public function testClearExpired(): void timecop_freeze(new DateTimeImmutable()); try { - $testData = $this->buildTestData( - function (array $item): bool { - return !$item['expired']; - } - ); + $testData = $this->buildClearExpiredTestData(); foreach ($testData['input'] as $token) { $inMemoryAuthCodeManager->save($token); @@ -37,90 +33,61 @@ function (array $item): bool { } } + private function buildClearExpiredTestData(): array + { + $validAuthCodes = [ + '1111' => $this->buildAuthCode('1111', '+1 day'), + '2222' => $this->buildAuthCode('2222', '+1 hour'), + '3333' => $this->buildAuthCode('3333', '+1 second'), + '4444' => $this->buildAuthCode('4444', 'now'), + ]; + + $expiredAuthCodes = [ + '5555' => $this->buildAuthCode('5555', '-1 day'), + '6666' => $this->buildAuthCode('6666', '-1 hour'), + '7777' => $this->buildAuthCode('7777', '-1 second'), + ]; + + return [ + 'input' => $validAuthCodes + $expiredAuthCodes, + 'output' => $validAuthCodes, + ]; + } + public function testClearRevoked(): void { $inMemoryAuthCodeManager = new InMemoryAuthCodeManager(); - $testData = $this->buildTestData( - function (array $item): bool { - return !$item['revoked']; - } - ); + $testData = $this->buildClearRevokedTestData(); foreach ($testData['input'] as $token) { $inMemoryAuthCodeManager->save($token); } - $this->assertSame(4, $inMemoryAuthCodeManager->clearRevoked()); + $this->assertSame(2, $inMemoryAuthCodeManager->clearRevoked()); $this->assertManagerContainsExpectedData($testData['output'], $inMemoryAuthCodeManager); } - private function buildTestData(callable $successFunction): array + private function buildClearRevokedTestData(): array { - $data = [ - [ - 'identifier' => '1111', - 'dateOffset' => '+1 day', - 'revoked' => true, - 'expired' => false, - ], - [ - 'identifier' => '2222', - 'dateOffset' => '+1 hour', - 'revoked' => false, - 'expired' => false, - ], - [ - 'identifier' => '3333', - 'dateOffset' => '+1 second', - 'revoked' => true, - 'expired' => false, - ], - [ - 'identifier' => '4444', - 'dateOffset' => 'now', - 'revoked' => false, - 'expired' => false, - ], - [ - 'identifier' => '5555', - 'dateOffset' => '-1 day', - 'revoked' => true, - 'expired' => true, - ], - [ - 'identifier' => '6666', - 'dateOffset' => '-1 hour', - 'revoked' => false, - 'expired' => true, - ], - [ - 'identifier' => '7777', - 'dateOffset' => '-1 second', - 'revoked' => true, - 'expired' => true, - ], + $validAuthCodes = [ + '1111' => $this->buildAuthCode('1111', '+1 day'), + '2222' => $this->buildAuthCode('2222', '+1 hour'), + '3333' => $this->buildAuthCode('3333', '+1 second'), ]; - $response = []; - foreach ($data as $item) { - $identifier = $item['identifier']; - $authCode = $this->buildAuthCode( - $identifier, - $item['dateOffset'], - $item['revoked'] - ); - $response['input'][$identifier] = $authCode; - - if ($successFunction($item)) { - $response['output'][$identifier] = $authCode; - } - } + $revokedAuthCodes = [ + '5555' => $this->buildAuthCode('5555', '-1 day', true), + '6666' => $this->buildAuthCode('6666', '-1 hour', true), + ]; - return $response; + return [ + 'input' => $validAuthCodes + $revokedAuthCodes, + 'output' => $validAuthCodes, + ]; } - private function buildAuthCode(string $identifier, string $modify, bool $revoked): AuthorizationCode + private function buildAuthCode(string $identifier, string $modify, bool $revoked = false): AuthorizationCode { $authorizationCode = new AuthorizationCode( $identifier, diff --git a/Tests/Unit/InMemoryRefreshTokenManagerTest.php b/Tests/Unit/InMemoryRefreshTokenManagerTest.php index 5483e6bc..2b37c2c9 100644 --- a/Tests/Unit/InMemoryRefreshTokenManagerTest.php +++ b/Tests/Unit/InMemoryRefreshTokenManagerTest.php @@ -21,11 +21,7 @@ public function testClearExpired(): void timecop_freeze(new DateTimeImmutable()); try { - $testData = $this->buildTestData( - function (array $item): bool { - return !$item['expired']; - } - ); + $testData = $this->buildClearExpiredTestData(); foreach ($testData['input'] as $token) { $inMemoryRefreshTokenManager->save($token); @@ -38,90 +34,61 @@ function (array $item): bool { } } + private function buildClearExpiredTestData(): array + { + $validRefreshTokens = [ + '1111' => $this->buildRefreshToken('1111', '+1 day'), + '2222' => $this->buildRefreshToken('2222', '+1 hour'), + '3333' => $this->buildRefreshToken('3333', '+1 second'), + '4444' => $this->buildRefreshToken('4444', 'now'), + ]; + + $expiredRefreshTokens = [ + '5555' => $this->buildRefreshToken('5555', '-1 day'), + '6666' => $this->buildRefreshToken('6666', '-1 hour'), + '7777' => $this->buildRefreshToken('7777', '-1 second'), + ]; + + return [ + 'input' => $validRefreshTokens + $expiredRefreshTokens, + 'output' => $validRefreshTokens, + ]; + } + public function testClearRevoked(): void { $inMemoryRefreshTokenManager = new InMemoryRefreshTokenManager(); - $testData = $this->buildTestData( - function (array $item): bool { - return !$item['revoked']; - } - ); + $testData = $this->buildClearRevokedTestData(); foreach ($testData['input'] as $token) { $inMemoryRefreshTokenManager->save($token); } - $this->assertSame(4, $inMemoryRefreshTokenManager->clearRevoked()); + $this->assertSame(2, $inMemoryRefreshTokenManager->clearRevoked()); $this->assertManagerContainsExpectedData($testData['output'], $inMemoryRefreshTokenManager); } - private function buildTestData(callable $successFunction): array + private function buildClearRevokedTestData(): array { - $data = [ - [ - 'identifier' => '1111', - 'dateOffset' => '+1 day', - 'revoked' => true, - 'expired' => false, - ], - [ - 'identifier' => '2222', - 'dateOffset' => '+1 hour', - 'revoked' => false, - 'expired' => false, - ], - [ - 'identifier' => '3333', - 'dateOffset' => '+1 second', - 'revoked' => true, - 'expired' => false, - ], - [ - 'identifier' => '4444', - 'dateOffset' => 'now', - 'revoked' => false, - 'expired' => false, - ], - [ - 'identifier' => '5555', - 'dateOffset' => '-1 day', - 'revoked' => true, - 'expired' => true, - ], - [ - 'identifier' => '6666', - 'dateOffset' => '-1 hour', - 'revoked' => false, - 'expired' => true, - ], - [ - 'identifier' => '7777', - 'dateOffset' => '-1 second', - 'revoked' => true, - 'expired' => true, - ], + $validRefreshTokens = [ + '1111' => $this->buildRefreshToken('1111', '+1 day'), + '2222' => $this->buildRefreshToken('2222', '+1 hour'), + '3333' => $this->buildRefreshToken('3333', '+1 second'), ]; - $response = []; - foreach ($data as $item) { - $identifier = $item['identifier']; - $refreshToken = $this->buildRefreshToken( - $identifier, - $item['dateOffset'], - $item['revoked'] - ); - $response['input'][$identifier] = $refreshToken; - - if ($successFunction($item)) { - $response['output'][$identifier] = $refreshToken; - } - } + $revokedRefreshTokens = [ + '5555' => $this->buildRefreshToken('5555', '-1 day', true), + '6666' => $this->buildRefreshToken('6666', '-1 hour', true), + ]; - return $response; + return [ + 'input' => $validRefreshTokens + $revokedRefreshTokens, + 'output' => $validRefreshTokens, + ]; } - private function buildRefreshToken(string $identifier, string $modify, bool $revoked): RefreshToken + private function buildRefreshToken(string $identifier, string $modify, bool $revoked = false): RefreshToken { $refreshToken = new RefreshToken( $identifier, From bf9f43a1b5e908fc069508ca28f14499a3914651 Mon Sep 17 00:00:00 2001 From: Franjo Zadelj Date: Fri, 23 Oct 2020 12:13:35 +0200 Subject: [PATCH 6/9] Cover with tests warning is issued if new method doesn't exist --- .../ClearRevokedTokensCommandTest.php | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/Tests/Acceptance/ClearRevokedTokensCommandTest.php b/Tests/Acceptance/ClearRevokedTokensCommandTest.php index aab13ff1..2da49e57 100644 --- a/Tests/Acceptance/ClearRevokedTokensCommandTest.php +++ b/Tests/Acceptance/ClearRevokedTokensCommandTest.php @@ -5,7 +5,10 @@ namespace Trikoder\Bundle\OAuth2Bundle\Tests\Acceptance; use Doctrine\ORM\EntityManagerInterface; +use stdClass; +use Symfony\Component\Console\Output\OutputInterface; use Symfony\Component\Console\Tester\CommandTester; +use Trikoder\Bundle\OAuth2Bundle\Command\ClearRevokedTokensCommand; use Trikoder\Bundle\OAuth2Bundle\Manager\AccessTokenManagerInterface; use Trikoder\Bundle\OAuth2Bundle\Manager\AuthorizationCodeManagerInterface; use Trikoder\Bundle\OAuth2Bundle\Manager\ClientManagerInterface; @@ -159,6 +162,33 @@ public function testClearRevokedAuthCodes(): void ); } + public function testWarningIsIssuedIfClearRevokedMethodIsNotImplemented(): void + { + $reflection = new \ReflectionClass(ClearRevokedTokensCommand::class); + $clearRevokedMethodExists = $reflection->getMethod('clearRevokedMethodExists'); + $clearRevokedMethodExists->setAccessible(true); + + $outputProphecy = $this->prophesize(OutputInterface::class); + + $outputProphecy + ->writeln('Method "stdClass:clearRevoked()" will be required in the next major release. Skipping for now...') + ->shouldBeCalledOnce() + ; + + $success = $clearRevokedMethodExists->invokeArgs( + new ClearRevokedTokensCommand( + $this->client->getContainer()->get(AccessTokenManagerInterface::class), + $this->client->getContainer()->get(RefreshTokenManagerInterface::class), + $this->client->getContainer()->get(AuthorizationCodeManagerInterface::class), + ), + [ + $outputProphecy->reveal(), + new stdClass(), + ] + ); + $this->assertFalse($success); + } + private function executeCommand(array $params = [], int $expectedExitCode = 0): string { $command = $this->application->find('trikoder:oauth2:clear-revoked-tokens'); From 6092d95b9867952097faeb93036f6b75c4baf938 Mon Sep 17 00:00:00 2001 From: Franjo Zadelj Date: Fri, 23 Oct 2020 12:53:43 +0200 Subject: [PATCH 7/9] Revert test testClearExpiredWithRefreshToken --- Tests/Acceptance/DoctrineAccessTokenManagerTest.php | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/Tests/Acceptance/DoctrineAccessTokenManagerTest.php b/Tests/Acceptance/DoctrineAccessTokenManagerTest.php index 723bb44c..0f1e7448 100644 --- a/Tests/Acceptance/DoctrineAccessTokenManagerTest.php +++ b/Tests/Acceptance/DoctrineAccessTokenManagerTest.php @@ -160,13 +160,9 @@ public function testClearExpiredWithRefreshToken(): void timecop_return(); } - $mapFunction = static function (RefreshToken $refreshToken): string { - return $refreshToken->getIdentifier(); - }; - $this->assertSame( - array_map($mapFunction, $testData['output']), - array_map($mapFunction, $em->getRepository(RefreshToken::class)->findBy(['accessToken' => null], ['identifier' => 'ASC'])) + $testData['output'], + $em->getRepository(RefreshToken::class)->findBy(['accessToken' => null], ['identifier' => 'ASC']) ); } From 6ff30ada5417fcd0719fb3a9e2a22c7e46457ea7 Mon Sep 17 00:00:00 2001 From: Franjo Zadelj Date: Fri, 23 Oct 2020 13:48:28 +0200 Subject: [PATCH 8/9] Add explicit call to clear entity manager in ClearRevokedTokensCommandTest --- Tests/Acceptance/ClearRevokedTokensCommandTest.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Tests/Acceptance/ClearRevokedTokensCommandTest.php b/Tests/Acceptance/ClearRevokedTokensCommandTest.php index 2da49e57..a4693f18 100644 --- a/Tests/Acceptance/ClearRevokedTokensCommandTest.php +++ b/Tests/Acceptance/ClearRevokedTokensCommandTest.php @@ -53,6 +53,7 @@ public function testClearRevokedAccessAndRefreshTokensAndAuthCodes(): void ); $output = $this->executeCommand(); + $this->clearEntityManager(); $this->assertStringContainsString('Access tokens deleted: 1.', $output); $this->assertStringContainsString('Refresh tokens deleted: 1.', $output); @@ -80,6 +81,7 @@ public function testClearRevokedAccessTokens(): void $output = $this->executeCommand([ '--access-tokens' => true, ]); + $this->clearEntityManager(); $this->assertStringContainsString('Access tokens deleted: 1.', $output); $this->assertStringNotContainsString('Refresh tokens deleted', $output); @@ -109,6 +111,7 @@ public function testClearRevokedRefreshTokens(): void $output = $this->executeCommand([ '--refresh-tokens' => true, ]); + $this->clearEntityManager(); $this->assertStringNotContainsString('Access tokens deleted', $output); $this->assertStringContainsString('Refresh tokens deleted: 1.', $output); @@ -138,6 +141,7 @@ public function testClearRevokedAuthCodes(): void $output = $this->executeCommand([ '--auth-codes' => true, ]); + $this->clearEntityManager(); $this->assertStringNotContainsString('Access tokens deleted', $output); $this->assertStringNotContainsString('Refresh tokens deleted', $output); @@ -200,7 +204,6 @@ private function executeCommand(array $params = [], int $expectedExitCode = 0): $params )); $this->assertSame($expectedExitCode, $exitCode); - $this->clearEntityManager(); return $commandTester->getDisplay(true); } From 759d01255cff9a05f481e5468ad3a0155cbbbc07 Mon Sep 17 00:00:00 2001 From: Franjo Zadelj Date: Fri, 23 Oct 2020 15:15:21 +0200 Subject: [PATCH 9/9] Fix PHP 7.2 compatibility --- Tests/Acceptance/ClearRevokedTokensCommandTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/Acceptance/ClearRevokedTokensCommandTest.php b/Tests/Acceptance/ClearRevokedTokensCommandTest.php index a4693f18..e8036c3f 100644 --- a/Tests/Acceptance/ClearRevokedTokensCommandTest.php +++ b/Tests/Acceptance/ClearRevokedTokensCommandTest.php @@ -183,7 +183,7 @@ public function testWarningIsIssuedIfClearRevokedMethodIsNotImplemented(): void new ClearRevokedTokensCommand( $this->client->getContainer()->get(AccessTokenManagerInterface::class), $this->client->getContainer()->get(RefreshTokenManagerInterface::class), - $this->client->getContainer()->get(AuthorizationCodeManagerInterface::class), + $this->client->getContainer()->get(AuthorizationCodeManagerInterface::class) ), [ $outputProphecy->reveal(),