From 4e4cbdffda42058bd97f8273498380f3a16c9084 Mon Sep 17 00:00:00 2001 From: Tatyana Koleva Date: Mon, 21 Oct 2024 19:03:44 +0300 Subject: [PATCH 1/2] OXDEV-8407 Check expiration field during token validation --- src/Infrastructure/Token.php | 20 ++++++++++++++++++++ src/Service/TokenValidator.php | 7 ++++++- tests/Integration/TestCase.php | 5 +++++ tests/Unit/Service/TokenValidatorTest.php | 12 ++++++------ 4 files changed, 37 insertions(+), 7 deletions(-) diff --git a/src/Infrastructure/Token.php b/src/Infrastructure/Token.php index 16065e91..34ef1d8b 100644 --- a/src/Infrastructure/Token.php +++ b/src/Infrastructure/Token.php @@ -49,6 +49,26 @@ public function isTokenRegistered(string $tokenId): bool return $storage->isLoaded(); } + public function isTokenExpired(string $tokenId): bool + { + $queryBuilder = $this->queryBuilderFactory->create() + ->select('oxid') + ->from('oegraphqltoken') + ->where('OXID = :tokenId') + ->andWhere('EXPIRES_AT <= NOW()') + ->setParameters([ + 'tokenId' => $tokenId, + ]); + + $result = $queryBuilder->execute(); + + if (is_object($result)) { + return $result->fetchOne() > 0; + } + + return false; + } + public function removeExpiredTokens(UserInterface $user): void { $queryBuilder = $this->queryBuilderFactory->create() diff --git a/src/Service/TokenValidator.php b/src/Service/TokenValidator.php index 02aa358f..fcebc0df 100644 --- a/src/Service/TokenValidator.php +++ b/src/Service/TokenValidator.php @@ -41,7 +41,7 @@ public function __construct( */ public function validateToken(UnencryptedToken $token): void { - if (!$this->areConstraintsValid($token)) { + if (!$this->areConstraintsValid($token) || $this->isTokenExpired($token)) { throw new InvalidToken(); } @@ -62,6 +62,11 @@ private function areConstraintsValid(UnencryptedToken $token): bool return $validator->validate($token, ...$config->validationConstraints()); } + private function isTokenExpired(UnencryptedToken $token): bool + { + return $this->tokenInfrastructure->isTokenExpired($token->claims()->get(Token::CLAIM_TOKENID)); + } + private function isUserBlocked(?string $userId): bool { $groups = $this->legacyInfrastructure->getUserGroupIds($userId); diff --git a/tests/Integration/TestCase.php b/tests/Integration/TestCase.php index a428ca61..772ac35a 100644 --- a/tests/Integration/TestCase.php +++ b/tests/Integration/TestCase.php @@ -323,6 +323,11 @@ public function isTokenRegistered(string $tokenId): bool return true; } + public function isTokenExpired(string $tokenId): bool + { + return false; + } + public function registerToken(UnencryptedToken $token, DateTimeImmutable $time, DateTimeImmutable $expire): void { } diff --git a/tests/Unit/Service/TokenValidatorTest.php b/tests/Unit/Service/TokenValidatorTest.php index ed4e051d..37f116ac 100644 --- a/tests/Unit/Service/TokenValidatorTest.php +++ b/tests/Unit/Service/TokenValidatorTest.php @@ -26,7 +26,7 @@ public function testTokenShopIdValidation(): void $tokenInfrastructure = $this->createPartialMock( TokenInfrastructure::class, - ['registerToken', 'isTokenRegistered', 'removeExpiredTokens', 'canIssueToken'] + ['registerToken', 'isTokenRegistered', 'isTokenExpired', 'removeExpiredTokens', 'canIssueToken'] ); $tokenInfrastructure->method('isTokenRegistered')->willReturn(true); $tokenInfrastructure->method('canIssueToken')->willReturn(true); @@ -54,7 +54,7 @@ public function testTokenShopUrlValidation(): void $tokenInfrastructure = $this->createPartialMock( TokenInfrastructure::class, - ['registerToken', 'isTokenRegistered', 'removeExpiredTokens', 'canIssueToken'] + ['registerToken', 'isTokenRegistered', 'isTokenExpired', 'removeExpiredTokens', 'canIssueToken'] ); $tokenInfrastructure->method('isTokenRegistered')->willReturn(true); $tokenInfrastructure->method('canIssueToken')->willReturn(true); @@ -85,7 +85,7 @@ public function testTokenUserInBlockedGroup(): void $tokenInfrastructure = $this->createPartialMock( TokenInfrastructure::class, - ['registerToken', 'isTokenRegistered', 'removeExpiredTokens', 'canIssueToken'] + ['registerToken', 'isTokenRegistered', 'isTokenExpired', 'removeExpiredTokens', 'canIssueToken'] ); $tokenInfrastructure->method('isTokenRegistered')->willReturn(true); $tokenInfrastructure->method('canIssueToken')->willReturn(true); @@ -108,7 +108,7 @@ public function testExpiredToken(): void $tokenInfrastructure = $this->createPartialMock( TokenInfrastructure::class, - ['registerToken', 'isTokenRegistered', 'removeExpiredTokens', 'canIssueToken'] + ['registerToken', 'isTokenRegistered', 'isTokenExpired', 'removeExpiredTokens', 'canIssueToken'] ); $tokenInfrastructure->method('isTokenRegistered')->willReturn(true); $tokenInfrastructure->method('canIssueToken')->willReturn(true); @@ -134,7 +134,7 @@ public function testDeletedToken(): void $tokenInfrastructure = $this->createPartialMock( TokenInfrastructure::class, - ['registerToken', 'isTokenRegistered', 'removeExpiredTokens', 'canIssueToken'] + ['registerToken', 'isTokenRegistered', 'isTokenExpired', 'removeExpiredTokens', 'canIssueToken'] ); $tokenInfrastructure->method('isTokenRegistered')->willReturn(false); $tokenInfrastructure->method('canIssueToken')->willReturn(true); @@ -157,7 +157,7 @@ public function testAnonymousToken(): void $tokenInfrastructure = $this->createPartialMock( TokenInfrastructure::class, - ['registerToken', 'isTokenRegistered', 'removeExpiredTokens', 'canIssueToken'] + ['registerToken', 'isTokenRegistered', 'isTokenExpired', 'removeExpiredTokens', 'canIssueToken'] ); $tokenInfrastructure->method('canIssueToken')->willReturn(true); $validator = $this->getTokenValidator($legacy, $tokenInfrastructure); From f3eb22c7860973e87a29f9077e146ee1a16a24cb Mon Sep 17 00:00:00 2001 From: Tatyana Koleva Date: Wed, 23 Oct 2024 20:26:07 +0300 Subject: [PATCH 2/2] OXDEV-8407 Add tests for isTokenExpired --- CHANGELOG-v10.md | 1 + .../Integration/Infrastructure/TokenTest.php | 44 +++++++++++-------- 2 files changed, 26 insertions(+), 19 deletions(-) diff --git a/CHANGELOG-v10.md b/CHANGELOG-v10.md index 76bca0ff..8d75bb76 100644 --- a/CHANGELOG-v10.md +++ b/CHANGELOG-v10.md @@ -30,6 +30,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - New methods: - `OxidEsales\GraphQL\Base\Infrastructure\RefreshTokenRepositoryInterface::invalidateUserTokens` - `OxidEsales\GraphQL\Base\Infrastructure\Token::invalidateUserTokens` + - `OxidEsales\GraphQL\Base\Infrastructure\Token::isTokenExpired` - New event subscriber: - `OxidEsales\GraphQL\Base\Event\Subscriber\PasswordChangeSubscriber` diff --git a/tests/Integration/Infrastructure/TokenTest.php b/tests/Integration/Infrastructure/TokenTest.php index 6d89c7dd..ffce3513 100644 --- a/tests/Integration/Infrastructure/TokenTest.php +++ b/tests/Integration/Infrastructure/TokenTest.php @@ -13,13 +13,13 @@ use Lcobucci\JWT\Token\DataSet; use Lcobucci\JWT\UnencryptedToken; use OxidEsales\Eshop\Application\Model\User; -use OxidEsales\EshopCommunity\Internal\Framework\Database\ConnectionProviderInterface; use OxidEsales\EshopCommunity\Tests\Integration\IntegrationTestCase; use OxidEsales\EshopCommunity\Tests\TestContainerFactory; use OxidEsales\GraphQL\Base\DataType\Token as TokenDataType; use OxidEsales\GraphQL\Base\DataType\User as UserDataType; use OxidEsales\GraphQL\Base\Infrastructure\Model\Token as TokenModel; use OxidEsales\GraphQL\Base\Infrastructure\Token as TokenInfrastructure; +use OxidEsales\GraphQL\Base\Service\Token; use OxidEsales\GraphQL\Base\Service\Token as TokenService; class TokenTest extends IntegrationTestCase @@ -31,9 +31,6 @@ class TokenTest extends IntegrationTestCase /** @var TokenInfrastructure */ private $tokenInfrastructure; - /** @var ConnectionProviderInterface */ - private $connection; - public function setUp(): void { parent::setUp(); @@ -41,7 +38,6 @@ public function setUp(): void $container = $containerFactory->create(); $container->compile(); $this->tokenInfrastructure = $container->get(TokenInfrastructure::class); - $this->connection = $container->get(ConnectionProviderInterface::class)->get(); } public function testRegisterToken(): void @@ -74,6 +70,28 @@ public function testIsTokenRegisteredNo(): void $this->assertFalse($this->tokenInfrastructure->isTokenRegistered('not_registered_token')); } + public function testIsTokenExpired(): void + { + $this->tokenInfrastructure->registerToken( + $this->getTokenMock('valid_token'), + new DateTimeImmutable('now'), + new DateTimeImmutable('+8 hours') + ); + + $this->assertFalse($this->tokenInfrastructure->isTokenExpired('valid_token')); + } + + public function testIsTokenExpiredNo(): void + { + $this->tokenInfrastructure->registerToken( + $this->getTokenMock('expired_token'), + new DateTimeImmutable('now'), + new DateTimeImmutable('-8 hours') + ); + + $this->assertTrue($this->tokenInfrastructure->isTokenExpired('expired_token')); + } + public function testRemoveExpiredTokens(): void { $this->tokenInfrastructure->registerToken( @@ -323,13 +341,7 @@ public function testInvalidateAccessTokens(): void ); $this->tokenInfrastructure->invalidateUserTokens($userId); - $result = $this->connection->executeQuery( - "select expires_at from `oegraphqltoken` where oxid=:tokenId", - ['tokenId' => $token] - ); - $expiresAtAfterChange = $result->fetchOne(); - - $this->assertTrue(new DateTimeImmutable($expiresAtAfterChange) <= new DateTimeImmutable('now')); + $this->assertTrue($this->tokenInfrastructure->isTokenExpired($token)); } public function testInvalidateAccessTokensWrongUserId(): void @@ -344,13 +356,7 @@ public function testInvalidateAccessTokensWrongUserId(): void ); $this->tokenInfrastructure->invalidateUserTokens('wrong_user_id'); - $result = $this->connection->executeQuery( - "select expires_at from `oegraphqltoken` where oxid=:tokenId", - ['tokenId' => $token] - ); - $expiresAtAfterChange = $result->fetchOne(); - - $this->assertFalse(new DateTimeImmutable($expiresAtAfterChange) <= new DateTimeImmutable('now')); + $this->assertFalse($this->tokenInfrastructure->isTokenExpired($token)); } private function getTokenMock(