From c4e7674d9f0a87c4d2349ee2065d14538bbf846f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Ivan=C4=8Di=C4=87?= Date: Wed, 16 Oct 2024 18:53:23 +0200 Subject: [PATCH 1/4] Move to dedicated RefreshTokenEntity Factory --- src/Entities/RefreshTokenEntity.php | 39 ++++-------- .../Entities/RefreshTokenEntityFactory.php | 63 +++++++++++++++++++ src/Repositories/RefreshTokenRepository.php | 6 +- src/Services/Container.php | 10 ++- .../src/Entities/RefreshTokenEntityTest.php | 42 +++++++------ .../RefreshTokenRepositoryTest.php | 36 +++++++++-- 6 files changed, 142 insertions(+), 54 deletions(-) create mode 100644 src/Factories/Entities/RefreshTokenEntityFactory.php diff --git a/src/Entities/RefreshTokenEntity.php b/src/Entities/RefreshTokenEntity.php index 047dfa48..c2094c12 100644 --- a/src/Entities/RefreshTokenEntity.php +++ b/src/Entities/RefreshTokenEntity.php @@ -24,8 +24,6 @@ use SimpleSAML\Module\oidc\Entities\Interfaces\RefreshTokenEntityInterface; use SimpleSAML\Module\oidc\Entities\Traits\AssociateWithAuthCodeTrait; use SimpleSAML\Module\oidc\Entities\Traits\RevokeTokenTrait; -use SimpleSAML\Module\oidc\Server\Exceptions\OidcServerException; -use SimpleSAML\Module\oidc\Utils\TimestampGenerator; class RefreshTokenEntity implements RefreshTokenEntityInterface { @@ -34,31 +32,18 @@ class RefreshTokenEntity implements RefreshTokenEntityInterface use RevokeTokenTrait; use AssociateWithAuthCodeTrait; - /** - * @throws \Exception - * @throws \SimpleSAML\Module\oidc\Server\Exceptions\OidcServerException - */ - public static function fromState(array $state): RefreshTokenEntityInterface - { - $refreshToken = new self(); - - if ( - !is_string($state['id']) || - !is_string($state['expires_at']) || - !is_a($state['access_token'], AccessTokenEntityInterface::class) - ) { - throw OidcServerException::serverError('Invalid Refresh Token state'); - } - - $refreshToken->identifier = $state['id']; - $refreshToken->expiryDateTime = DateTimeImmutable::createFromMutable( - TimestampGenerator::utc($state['expires_at']), - ); - $refreshToken->accessToken = $state['access_token']; - $refreshToken->isRevoked = (bool) $state['is_revoked']; - $refreshToken->authCodeId = empty($state['auth_code_id']) ? null : (string)$state['auth_code_id']; - - return $refreshToken; + public function __construct( + string $id, + DateTimeImmutable $expiryDateTime, + AccessTokenEntityInterface $accessTokenEntity, + ?string $authCodeId = null, + bool $isRevoked = false, + ) { + $this->setIdentifier($id); + $this->setExpiryDateTime($expiryDateTime); + $this->setAccessToken($accessTokenEntity); + $this->setAuthCodeId($authCodeId); + $this->isRevoked = $isRevoked; } public function getState(): array diff --git a/src/Factories/Entities/RefreshTokenEntityFactory.php b/src/Factories/Entities/RefreshTokenEntityFactory.php new file mode 100644 index 00000000..5c5af49a --- /dev/null +++ b/src/Factories/Entities/RefreshTokenEntityFactory.php @@ -0,0 +1,63 @@ +helpers->dateTime()->getUtc($state['expires_at']); + $accessToken = $state['access_token']; + $isRevoked = (bool) $state['is_revoked']; + $authCodeId = empty($state['auth_code_id']) ? null : (string)$state['auth_code_id']; + + return $this->fromData( + $id, + $expiryDateTime, + $accessToken, + $authCodeId, + $isRevoked, + ); + } +} diff --git a/src/Repositories/RefreshTokenRepository.php b/src/Repositories/RefreshTokenRepository.php index ef95b864..30e6ab1e 100644 --- a/src/Repositories/RefreshTokenRepository.php +++ b/src/Repositories/RefreshTokenRepository.php @@ -21,6 +21,7 @@ use RuntimeException; use SimpleSAML\Module\oidc\Entities\Interfaces\RefreshTokenEntityInterface; use SimpleSAML\Module\oidc\Entities\RefreshTokenEntity; +use SimpleSAML\Module\oidc\Factories\Entities\RefreshTokenEntityFactory; use SimpleSAML\Module\oidc\ModuleConfig; use SimpleSAML\Module\oidc\Repositories\Interfaces\RefreshTokenRepositoryInterface; use SimpleSAML\Module\oidc\Repositories\Traits\RevokeTokenByAuthCodeIdTrait; @@ -35,6 +36,7 @@ class RefreshTokenRepository extends AbstractDatabaseRepository implements Refre public function __construct( ModuleConfig $moduleConfig, protected readonly AccessTokenRepository $accessTokenRepository, + protected readonly RefreshTokenEntityFactory $refreshTokenEntityFactory, ) { parent::__construct($moduleConfig); } @@ -52,7 +54,7 @@ public function getTableName(): string */ public function getNewRefreshToken(): RefreshTokenEntityInterface { - return new RefreshTokenEntity(); + throw new RuntimeException('Not implemented. Use RefreshTokenEntityFactory instead.'); } /** @@ -99,7 +101,7 @@ public function findById(string $tokenId): ?RefreshTokenEntityInterface $data = current($rows); $data['access_token'] = $this->accessTokenRepository->findById((string)$data['access_token_id']); - return RefreshTokenEntity::fromState($data); + return $this->refreshTokenEntityFactory->fromState($data); } /** diff --git a/src/Services/Container.php b/src/Services/Container.php index 7cc08c13..202c936e 100644 --- a/src/Services/Container.php +++ b/src/Services/Container.php @@ -42,6 +42,7 @@ use SimpleSAML\Module\oidc\Factories\Entities\AuthCodeEntityFactory; use SimpleSAML\Module\oidc\Factories\Entities\ClaimSetEntityFactory; use SimpleSAML\Module\oidc\Factories\Entities\ClientEntityFactory; +use SimpleSAML\Module\oidc\Factories\Entities\RefreshTokenEntityFactory; use SimpleSAML\Module\oidc\Factories\FederationFactory; use SimpleSAML\Module\oidc\Factories\FormFactory; use SimpleSAML\Module\oidc\Factories\Grant\AuthCodeGrantFactory; @@ -238,7 +239,14 @@ public function __construct() ); $this->services[AccessTokenRepository::class] = $accessTokenRepository; - $refreshTokenRepository = new RefreshTokenRepository($moduleConfig, $accessTokenRepository); + $refreshTokenEntityFactory = new RefreshTokenEntityFactory($helpers); + $this->services[RefreshTokenEntityFactory::class] = $refreshTokenEntityFactory; + + $refreshTokenRepository = new RefreshTokenRepository( + $moduleConfig, + $accessTokenRepository, + $refreshTokenEntityFactory, + ); $this->services[RefreshTokenRepository::class] = $refreshTokenRepository; $scopeRepository = new ScopeRepository($moduleConfig); diff --git a/tests/unit/src/Entities/RefreshTokenEntityTest.php b/tests/unit/src/Entities/RefreshTokenEntityTest.php index 8c7e4ef7..7abed99f 100644 --- a/tests/unit/src/Entities/RefreshTokenEntityTest.php +++ b/tests/unit/src/Entities/RefreshTokenEntityTest.php @@ -4,6 +4,8 @@ namespace SimpleSAML\Test\Module\oidc\unit\Entities; +use DateTimeImmutable; +use DateTimeZone; use PDO; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; @@ -16,33 +18,37 @@ */ class RefreshTokenEntityTest extends TestCase { + protected string $id; + protected DateTimeImmutable $expiryDateTime; protected MockObject $accessTokenEntityMock; - protected array $state; + protected false $isRevoked; + protected string $authCodeId; /** * @throws \Exception */ protected function setUp(): void { + $this->id = 'id'; + $this->expiryDateTime = new DateTimeImmutable('1970-01-01 00:00:00', new DateTimeZone('UTC')); $this->accessTokenEntityMock = $this->createMock(AccessTokenEntity::class); $this->accessTokenEntityMock->method('getIdentifier')->willReturn('access_token_id'); - - $this->state = [ - 'id' => 'id', - 'expires_at' => '1970-01-01 00:00:00', - 'access_token' => $this->accessTokenEntityMock, - 'is_revoked' => false, - 'auth_code_id' => '123', - ]; + $this->isRevoked = false; + $this->authCodeId = 'auth_code_id'; } /** * @throws \SimpleSAML\Module\oidc\Server\Exceptions\OidcServerException */ - protected function prepareMockedInstance(array $state = null): RefreshTokenEntityInterface + protected function mock(): RefreshTokenEntityInterface { - $state ??= $this->state; - return RefreshTokenEntity::fromState($state); + return new RefreshTokenEntity( + $this->id, + $this->expiryDateTime, + $this->accessTokenEntityMock, + $this->authCodeId, + $this->isRevoked, + ); } /** @@ -52,7 +58,7 @@ public function testItIsInitializable(): void { $this->assertInstanceOf( RefreshTokenEntity::class, - $this->prepareMockedInstance(), + $this->mock(), ); } @@ -62,13 +68,13 @@ public function testItIsInitializable(): void public function testCanGetState(): void { $this->assertSame( - $this->prepareMockedInstance()->getState(), + $this->mock()->getState(), [ - 'id' => 'id', + 'id' => $this->id, 'expires_at' => '1970-01-01 00:00:00', - 'access_token_id' => 'access_token_id', - 'is_revoked' => [$this->state['is_revoked'], PDO::PARAM_BOOL], - 'auth_code_id' => '123', + 'access_token_id' => $this->accessTokenEntityMock->getIdentifier(), + 'is_revoked' => [$this->isRevoked, PDO::PARAM_BOOL], + 'auth_code_id' => $this->authCodeId, ], ); } diff --git a/tests/unit/src/Repositories/RefreshTokenRepositoryTest.php b/tests/unit/src/Repositories/RefreshTokenRepositoryTest.php index eaaf25d2..8eb21fba 100644 --- a/tests/unit/src/Repositories/RefreshTokenRepositoryTest.php +++ b/tests/unit/src/Repositories/RefreshTokenRepositoryTest.php @@ -16,16 +16,18 @@ namespace SimpleSAML\Test\Module\oidc\unit\Repositories; use DateTimeImmutable; +use DateTimeZone; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use RuntimeException; use SimpleSAML\Configuration; use SimpleSAML\Module\oidc\Entities\AccessTokenEntity; +use SimpleSAML\Module\oidc\Entities\RefreshTokenEntity; +use SimpleSAML\Module\oidc\Factories\Entities\RefreshTokenEntityFactory; use SimpleSAML\Module\oidc\ModuleConfig; use SimpleSAML\Module\oidc\Repositories\AccessTokenRepository; use SimpleSAML\Module\oidc\Repositories\RefreshTokenRepository; use SimpleSAML\Module\oidc\Services\DatabaseMigration; -use SimpleSAML\Module\oidc\Utils\TimestampGenerator; /** * @covers \SimpleSAML\Module\oidc\Repositories\RefreshTokenRepository @@ -40,6 +42,8 @@ class RefreshTokenRepositoryTest extends TestCase protected RefreshTokenRepository $repository; protected MockObject $accessTokenMock; protected MockObject $accessTokenRepositoryMock; + protected MockObject $refreshTokenEntityFactoryMock; + protected MockObject $refreshTokenEntityMock; /** * @throws \League\OAuth2\Server\Exception\UniqueTokenIdentifierConstraintViolationException @@ -67,10 +71,14 @@ protected function setUp(): void $this->accessTokenMock = $this->createMock(AccessTokenEntity::class); $this->accessTokenMock->method('getIdentifier')->willReturn(self::ACCESS_TOKEN_ID); $this->accessTokenRepositoryMock = $this->createMock(AccessTokenRepository::class); + $this->refreshTokenEntityFactoryMock = $this->createMock(RefreshTokenEntityFactory::class); + + $this->refreshTokenEntityMock = $this->createMock(RefreshTokenEntity::class); $this->repository = new RefreshTokenRepository( new ModuleConfig(), $this->accessTokenRepositoryMock, + $this->refreshTokenEntityFactoryMock, ); } @@ -87,13 +95,19 @@ public function testGetTableName(): void */ public function testAddAndFound(): void { - $refreshToken = $this->repository->getNewRefreshToken(); - $refreshToken->setIdentifier(self::REFRESH_TOKEN_ID); - $refreshToken->setExpiryDateTime(DateTimeImmutable::createFromMutable(TimestampGenerator::utc('yesterday'))); - $refreshToken->setAccessToken($this->accessTokenMock); - + $refreshToken = new RefreshTokenEntity( + self::REFRESH_TOKEN_ID, + new DateTimeImmutable('yesterday', new DateTimeZone('UTC')), + $this->accessTokenMock, + ); $this->repository->persistNewRefreshToken($refreshToken); + $this->refreshTokenEntityFactoryMock->expects($this->once()) + ->method('fromState') + ->with($this->callback(function (array $state): bool { + return $state['id'] === self::REFRESH_TOKEN_ID; + }))->willReturn($refreshToken); + $this->accessTokenRepositoryMock->method('findById')->willReturn($this->accessTokenMock); $foundRefreshToken = $this->repository->findById(self::REFRESH_TOKEN_ID); @@ -115,7 +129,17 @@ public function testAddAndNotFound(): void */ public function testRevokeToken(): void { + $revokedRefreshTokenMock = $this->createMock(RefreshTokenEntity::class); + $revokedRefreshTokenMock->method('isRevoked')->willReturn(true); $this->accessTokenRepositoryMock->method('findById')->willReturn($this->accessTokenMock); + $this->refreshTokenEntityMock->expects($this->once())->method('revoke'); + $this->refreshTokenEntityFactoryMock->expects($this->atLeastOnce()) + ->method('fromState') + ->with($this->callback(function (array $state): bool { + return $state['id'] === self::REFRESH_TOKEN_ID; + })) + ->willReturnOnConsecutiveCalls($this->refreshTokenEntityMock, $revokedRefreshTokenMock); + $this->repository->revokeRefreshToken(self::REFRESH_TOKEN_ID); $isRevoked = $this->repository->isRefreshTokenRevoked(self::REFRESH_TOKEN_ID); From 14380928b391f5222a92cd9f1e911a5e0bbc04bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Ivan=C4=8Di=C4=87?= Date: Wed, 16 Oct 2024 19:19:17 +0200 Subject: [PATCH 2/4] Update issueRefreshToken method for AuthCodeGrant --- src/Server/Grants/AuthCodeGrant.php | 30 ++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/src/Server/Grants/AuthCodeGrant.php b/src/Server/Grants/AuthCodeGrant.php index 59695c67..c5b9cd03 100644 --- a/src/Server/Grants/AuthCodeGrant.php +++ b/src/Server/Grants/AuthCodeGrant.php @@ -26,6 +26,7 @@ use SimpleSAML\Module\oidc\Entities\UserEntity; use SimpleSAML\Module\oidc\Factories\Entities\AccessTokenEntityFactory; use SimpleSAML\Module\oidc\Factories\Entities\AuthCodeEntityFactory; +use SimpleSAML\Module\oidc\Factories\Entities\RefreshTokenEntityFactory; use SimpleSAML\Module\oidc\Repositories\Interfaces\AccessTokenRepositoryInterface; use SimpleSAML\Module\oidc\Repositories\Interfaces\AuthCodeRepositoryInterface; use SimpleSAML\Module\oidc\Repositories\Interfaces\RefreshTokenRepositoryInterface; @@ -56,6 +57,7 @@ use SimpleSAML\Module\oidc\Server\ResponseTypes\Interfaces\AuthTimeResponseTypeInterface; use SimpleSAML\Module\oidc\Server\ResponseTypes\Interfaces\NonceResponseTypeInterface; use SimpleSAML\Module\oidc\Server\ResponseTypes\Interfaces\SessionIdResponseTypeInterface; +use SimpleSAML\Module\oidc\Services\LoggerService; use SimpleSAML\Module\oidc\Utils\Arr; use SimpleSAML\Module\oidc\Utils\RequestParamsResolver; use SimpleSAML\Module\oidc\Utils\ScopeHelper; @@ -162,6 +164,8 @@ public function __construct( protected RequestParamsResolver $requestParamsResolver, AccessTokenEntityFactory $accessTokenEntityFactory, protected AuthCodeEntityFactory $authCodeEntityFactory, + protected RefreshTokenEntityFactory $refreshTokenEntityFactory, + protected LoggerService $logger, ) { parent::__construct($authCodeRepository, $refreshTokenRepository, $authCodeTTL); @@ -751,23 +755,18 @@ protected function issueRefreshToken( throw OidcServerException::serverError('Unexpected refresh token repository entity type.'); } - $refreshToken = $this->refreshTokenRepository->getNewRefreshToken(); - - if ($refreshToken === null) { - return null; - } - - $refreshToken->setExpiryDateTime((new DateTimeImmutable())->add($this->refreshTokenTTL)); - $refreshToken->setAccessToken($accessToken); - $refreshToken->setAuthCodeId($authCodeId); - $maxGenerationAttempts = self::MAX_RANDOM_TOKEN_GENERATION_ATTEMPTS; while ($maxGenerationAttempts-- > 0) { - $refreshToken->setIdentifier($this->generateUniqueIdentifier()); try { + $refreshToken = $this->refreshTokenEntityFactory->fromData( + $this->generateUniqueIdentifier(), + (new DateTimeImmutable())->add($this->refreshTokenTTL), + $accessToken, + $authCodeId, + ); $this->refreshTokenRepository->persistNewRefreshToken($refreshToken); - break; + return $refreshToken; } catch (UniqueTokenIdentifierConstraintViolationException $e) { if ($maxGenerationAttempts === 0) { throw $e; @@ -775,6 +774,11 @@ protected function issueRefreshToken( } } - return $refreshToken; + $this->logger->error('Unable to issue refresh token.', [ + 'accessTokenId' => $accessToken->getIdentifier(), + 'authCodeId' => $authCodeId, + ]); + + return null; } } From b84f87b4e364d12f3eeec2a68d82c47b5abcb3e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Ivan=C4=8Di=C4=87?= Date: Fri, 18 Oct 2024 19:31:23 +0200 Subject: [PATCH 3/4] Update AuthCodeGrant --- src/Factories/Grant/AuthCodeGrantFactory.php | 6 ++++++ src/Server/Grants/AuthCodeGrant.php | 4 ++++ src/Services/Container.php | 2 ++ .../unit/src/Server/Grants/AuthCodeGrantTest.php | 15 ++++++++++++--- 4 files changed, 24 insertions(+), 3 deletions(-) diff --git a/src/Factories/Grant/AuthCodeGrantFactory.php b/src/Factories/Grant/AuthCodeGrantFactory.php index e2d811ed..a8b51dea 100644 --- a/src/Factories/Grant/AuthCodeGrantFactory.php +++ b/src/Factories/Grant/AuthCodeGrantFactory.php @@ -18,12 +18,14 @@ use SimpleSAML\Module\oidc\Factories\Entities\AccessTokenEntityFactory; use SimpleSAML\Module\oidc\Factories\Entities\AuthCodeEntityFactory; +use SimpleSAML\Module\oidc\Factories\Entities\RefreshTokenEntityFactory; use SimpleSAML\Module\oidc\ModuleConfig; use SimpleSAML\Module\oidc\Repositories\AccessTokenRepository; use SimpleSAML\Module\oidc\Repositories\AuthCodeRepository; use SimpleSAML\Module\oidc\Repositories\RefreshTokenRepository; use SimpleSAML\Module\oidc\Server\Grants\AuthCodeGrant; use SimpleSAML\Module\oidc\Server\RequestRules\RequestRulesManager; +use SimpleSAML\Module\oidc\Services\LoggerService; use SimpleSAML\Module\oidc\Utils\RequestParamsResolver; class AuthCodeGrantFactory @@ -37,6 +39,8 @@ public function __construct( private readonly RequestParamsResolver $requestParamsResolver, private readonly AccessTokenEntityFactory $accessTokenEntityFactory, private readonly AuthCodeEntityFactory $authCodeEntityFactory, + private readonly RefreshTokenEntityFactory $refreshTokenEntityFactory, + private readonly LoggerService $logger, ) { } @@ -54,6 +58,8 @@ public function build(): AuthCodeGrant $this->requestParamsResolver, $this->accessTokenEntityFactory, $this->authCodeEntityFactory, + $this->refreshTokenEntityFactory, + $this->logger, ); $authCodeGrant->setRefreshTokenTTL($this->moduleConfig->getRefreshTokenDuration()); diff --git a/src/Server/Grants/AuthCodeGrant.php b/src/Server/Grants/AuthCodeGrant.php index c5b9cd03..28177669 100644 --- a/src/Server/Grants/AuthCodeGrant.php +++ b/src/Server/Grants/AuthCodeGrant.php @@ -21,6 +21,7 @@ use League\OAuth2\Server\ResponseTypes\ResponseTypeInterface; use LogicException; use Psr\Http\Message\ServerRequestInterface; +use SimpleSAML\Module\oidc\Entities\Interfaces\AccessTokenEntityInterface; use SimpleSAML\Module\oidc\Entities\Interfaces\AuthCodeEntityInterface; use SimpleSAML\Module\oidc\Entities\Interfaces\RefreshTokenEntityInterface; use SimpleSAML\Module\oidc\Entities\UserEntity; @@ -754,6 +755,9 @@ protected function issueRefreshToken( if (! is_a($this->refreshTokenRepository, RefreshTokenRepositoryInterface::class)) { throw OidcServerException::serverError('Unexpected refresh token repository entity type.'); } + if (! is_a($accessToken, AccessTokenEntityInterface::class)) { + throw OidcServerException::serverError('Unexpected access token entity type.'); + } $maxGenerationAttempts = self::MAX_RANDOM_TOKEN_GENERATION_ATTEMPTS; diff --git a/src/Services/Container.php b/src/Services/Container.php index 202c936e..6149af27 100644 --- a/src/Services/Container.php +++ b/src/Services/Container.php @@ -363,6 +363,8 @@ public function __construct() $requestParamsResolver, $accessTokenEntityFactory, $authCodeEntityFactory, + $refreshTokenEntityFactory, + $loggerService, ); $this->services[AuthCodeGrant::class] = $authCodeGrantFactory->build(); diff --git a/tests/unit/src/Server/Grants/AuthCodeGrantTest.php b/tests/unit/src/Server/Grants/AuthCodeGrantTest.php index fcb2f061..9423617b 100644 --- a/tests/unit/src/Server/Grants/AuthCodeGrantTest.php +++ b/tests/unit/src/Server/Grants/AuthCodeGrantTest.php @@ -5,16 +5,19 @@ namespace SimpleSAML\Test\Module\oidc\unit\Server\Grants; use DateInterval; +use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\MockObject\Stub; use PHPUnit\Framework\TestCase; use SimpleSAML\Module\oidc\Factories\Entities\AccessTokenEntityFactory; use SimpleSAML\Module\oidc\Factories\Entities\AuthCodeEntityFactory; +use SimpleSAML\Module\oidc\Factories\Entities\RefreshTokenEntityFactory; use SimpleSAML\Module\oidc\ModuleConfig; use SimpleSAML\Module\oidc\Repositories\Interfaces\AccessTokenRepositoryInterface; use SimpleSAML\Module\oidc\Repositories\Interfaces\AuthCodeRepositoryInterface; use SimpleSAML\Module\oidc\Repositories\Interfaces\RefreshTokenRepositoryInterface; use SimpleSAML\Module\oidc\Server\Grants\AuthCodeGrant; use SimpleSAML\Module\oidc\Server\RequestRules\RequestRulesManager; +use SimpleSAML\Module\oidc\Services\LoggerService; use SimpleSAML\Module\oidc\Utils\RequestParamsResolver; /** @@ -30,7 +33,9 @@ class AuthCodeGrantTest extends TestCase protected Stub $moduleConfigStub; protected Stub $requestParamsResolverStub; protected Stub $accessTokenEntityFactoryStub; - protected Stub $authCodeEntityFactory; + protected Stub $authCodeEntityFactoryStub; + protected Stub $refreshTokenEntityFactoryStub; + protected MockObject $loggerMock; /** * @throws \Exception @@ -45,7 +50,9 @@ protected function setUp(): void $this->moduleConfigStub = $this->createStub(ModuleConfig::class); $this->requestParamsResolverStub = $this->createStub(RequestParamsResolver::class); $this->accessTokenEntityFactoryStub = $this->createStub(AccessTokenEntityFactory::class); - $this->authCodeEntityFactory = $this->createStub(AuthcodeEntityFactory::class); + $this->authCodeEntityFactoryStub = $this->createStub(AuthcodeEntityFactory::class); + $this->refreshTokenEntityFactoryStub = $this->createStub(RefreshTokenEntityFactory::class); + $this->loggerMock = $this->createMock(LoggerService::class); } /** @@ -63,7 +70,9 @@ public function testCanCreateInstance(): void $this->requestRulesManagerStub, $this->requestParamsResolverStub, $this->accessTokenEntityFactoryStub, - $this->authCodeEntityFactory, + $this->authCodeEntityFactoryStub, + $this->refreshTokenEntityFactoryStub, + $this->loggerMock, ), ); } From c59a0365bae590791f653fe65dfdcf8512657afa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Ivan=C4=8Di=C4=87?= Date: Mon, 21 Oct 2024 10:57:15 +0200 Subject: [PATCH 4/4] Introduce RefreshTokenIssuer --- routing/services/services.yml | 3 + src/Factories/Grant/AuthCodeGrantFactory.php | 9 +-- .../Grant/RefreshTokenGrantFactory.php | 9 ++- src/Helpers.php | 7 ++ src/Helpers/Random.php | 27 +++++++ src/Server/Grants/AuthCodeGrant.php | 40 ++-------- src/Server/Grants/RefreshTokenGrant.php | 21 ++++++ .../TokenIssuers/AbstractTokenIssuer.php | 17 +++++ .../TokenIssuers/RefreshTokenIssuer.php | 74 +++++++++++++++++++ src/Services/Container.php | 13 +++- src/Utils/UniqueIdentifierGenerator.php | 3 + .../src/Server/Grants/AuthCodeGrantTest.php | 13 +--- 12 files changed, 186 insertions(+), 50 deletions(-) create mode 100644 src/Helpers/Random.php create mode 100644 src/Server/TokenIssuers/AbstractTokenIssuer.php create mode 100644 src/Server/TokenIssuers/RefreshTokenIssuer.php diff --git a/routing/services/services.yml b/routing/services/services.yml index da36c690..0d5e8a48 100644 --- a/routing/services/services.yml +++ b/routing/services/services.yml @@ -35,6 +35,9 @@ services: SimpleSAML\Module\oidc\Bridges\: resource: '../../src/Bridges/*' + SimpleSAML\Module\oidc\Server\TokenIssuers\: + resource: '../../src/Server/TokenIssuers/*' + SimpleSAML\Module\oidc\ModuleConfig: ~ SimpleSAML\Module\oidc\Helpers: ~ SimpleSAML\Module\oidc\Forms\Controls\CsrfProtection: ~ diff --git a/src/Factories/Grant/AuthCodeGrantFactory.php b/src/Factories/Grant/AuthCodeGrantFactory.php index a8b51dea..f519b687 100644 --- a/src/Factories/Grant/AuthCodeGrantFactory.php +++ b/src/Factories/Grant/AuthCodeGrantFactory.php @@ -18,14 +18,13 @@ use SimpleSAML\Module\oidc\Factories\Entities\AccessTokenEntityFactory; use SimpleSAML\Module\oidc\Factories\Entities\AuthCodeEntityFactory; -use SimpleSAML\Module\oidc\Factories\Entities\RefreshTokenEntityFactory; use SimpleSAML\Module\oidc\ModuleConfig; use SimpleSAML\Module\oidc\Repositories\AccessTokenRepository; use SimpleSAML\Module\oidc\Repositories\AuthCodeRepository; use SimpleSAML\Module\oidc\Repositories\RefreshTokenRepository; use SimpleSAML\Module\oidc\Server\Grants\AuthCodeGrant; use SimpleSAML\Module\oidc\Server\RequestRules\RequestRulesManager; -use SimpleSAML\Module\oidc\Services\LoggerService; +use SimpleSAML\Module\oidc\Server\TokenIssuers\RefreshTokenIssuer; use SimpleSAML\Module\oidc\Utils\RequestParamsResolver; class AuthCodeGrantFactory @@ -39,8 +38,7 @@ public function __construct( private readonly RequestParamsResolver $requestParamsResolver, private readonly AccessTokenEntityFactory $accessTokenEntityFactory, private readonly AuthCodeEntityFactory $authCodeEntityFactory, - private readonly RefreshTokenEntityFactory $refreshTokenEntityFactory, - private readonly LoggerService $logger, + private readonly RefreshTokenIssuer $refreshTokenIssuer, ) { } @@ -58,8 +56,7 @@ public function build(): AuthCodeGrant $this->requestParamsResolver, $this->accessTokenEntityFactory, $this->authCodeEntityFactory, - $this->refreshTokenEntityFactory, - $this->logger, + $this->refreshTokenIssuer, ); $authCodeGrant->setRefreshTokenTTL($this->moduleConfig->getRefreshTokenDuration()); diff --git a/src/Factories/Grant/RefreshTokenGrantFactory.php b/src/Factories/Grant/RefreshTokenGrantFactory.php index c598c8cb..c12c26bb 100644 --- a/src/Factories/Grant/RefreshTokenGrantFactory.php +++ b/src/Factories/Grant/RefreshTokenGrantFactory.php @@ -19,6 +19,7 @@ use SimpleSAML\Module\oidc\ModuleConfig; use SimpleSAML\Module\oidc\Repositories\RefreshTokenRepository; use SimpleSAML\Module\oidc\Server\Grants\RefreshTokenGrant; +use SimpleSAML\Module\oidc\Server\TokenIssuers\RefreshTokenIssuer; class RefreshTokenGrantFactory { @@ -26,12 +27,18 @@ public function __construct( private readonly ModuleConfig $moduleConfig, private readonly RefreshTokenRepository $refreshTokenRepository, private readonly AccessTokenEntityFactory $accessTokenEntityFactory, + private readonly RefreshTokenIssuer $refreshTokenIssuer, ) { } public function build(): RefreshTokenGrant { - $refreshTokenGrant = new RefreshTokenGrant($this->refreshTokenRepository, $this->accessTokenEntityFactory); + $refreshTokenGrant = new RefreshTokenGrant( + $this->refreshTokenRepository, + $this->accessTokenEntityFactory, + $this->refreshTokenIssuer, + ); + $refreshTokenGrant->setRefreshTokenTTL($this->moduleConfig->getRefreshTokenDuration()); return $refreshTokenGrant; diff --git a/src/Helpers.php b/src/Helpers.php index 80817630..7a5e153a 100644 --- a/src/Helpers.php +++ b/src/Helpers.php @@ -8,6 +8,7 @@ use SimpleSAML\Module\oidc\Helpers\Client; use SimpleSAML\Module\oidc\Helpers\DateTime; use SimpleSAML\Module\oidc\Helpers\Http; +use SimpleSAML\Module\oidc\Helpers\Random; use SimpleSAML\Module\oidc\Helpers\Str; class Helpers @@ -17,6 +18,7 @@ class Helpers protected static ?DateTime $dateTIme = null; protected static ?Str $str = null; protected static ?Arr $arr = null; + protected static ?Random $random = null; public function http(): Http { @@ -44,4 +46,9 @@ public function arr(): Arr { return static::$arr ??= new Arr(); } + + public function random(): Random + { + return static::$random ??= new Random(); + } } diff --git a/src/Helpers/Random.php b/src/Helpers/Random.php new file mode 100644 index 00000000..f6c0b68d --- /dev/null +++ b/src/Helpers/Random.php @@ -0,0 +1,27 @@ +refreshTokenRepository, RefreshTokenRepositoryInterface::class)) { - throw OidcServerException::serverError('Unexpected refresh token repository entity type.'); - } if (! is_a($accessToken, AccessTokenEntityInterface::class)) { throw OidcServerException::serverError('Unexpected access token entity type.'); } - $maxGenerationAttempts = self::MAX_RANDOM_TOKEN_GENERATION_ATTEMPTS; - - while ($maxGenerationAttempts-- > 0) { - try { - $refreshToken = $this->refreshTokenEntityFactory->fromData( - $this->generateUniqueIdentifier(), - (new DateTimeImmutable())->add($this->refreshTokenTTL), - $accessToken, - $authCodeId, - ); - $this->refreshTokenRepository->persistNewRefreshToken($refreshToken); - return $refreshToken; - } catch (UniqueTokenIdentifierConstraintViolationException $e) { - if ($maxGenerationAttempts === 0) { - throw $e; - } - } - } - - $this->logger->error('Unable to issue refresh token.', [ - 'accessTokenId' => $accessToken->getIdentifier(), - 'authCodeId' => $authCodeId, - ]); - - return null; + return $this->refreshTokenIssuer->issue( + $accessToken, + $this->refreshTokenTTL, + $authCodeId, + self::MAX_RANDOM_TOKEN_GENERATION_ATTEMPTS, + ); } } diff --git a/src/Server/Grants/RefreshTokenGrant.php b/src/Server/Grants/RefreshTokenGrant.php index 75cde90c..ec7b58d1 100644 --- a/src/Server/Grants/RefreshTokenGrant.php +++ b/src/Server/Grants/RefreshTokenGrant.php @@ -5,13 +5,17 @@ namespace SimpleSAML\Module\oidc\Server\Grants; use Exception; +use League\OAuth2\Server\Entities\AccessTokenEntityInterface as OAuth2AccessTokenEntityInterface; use League\OAuth2\Server\Grant\RefreshTokenGrant as OAuth2RefreshTokenGrant; use League\OAuth2\Server\Repositories\RefreshTokenRepositoryInterface; use League\OAuth2\Server\RequestEvent; use Psr\Http\Message\ServerRequestInterface; +use SimpleSAML\Module\oidc\Entities\Interfaces\AccessTokenEntityInterface; +use SimpleSAML\Module\oidc\Entities\Interfaces\RefreshTokenEntityInterface; use SimpleSAML\Module\oidc\Factories\Entities\AccessTokenEntityFactory; use SimpleSAML\Module\oidc\Server\Exceptions\OidcServerException; use SimpleSAML\Module\oidc\Server\Grants\Traits\IssueAccessTokenTrait; +use SimpleSAML\Module\oidc\Server\TokenIssuers\RefreshTokenIssuer; use function is_null; use function json_decode; @@ -48,6 +52,7 @@ class RefreshTokenGrant extends OAuth2RefreshTokenGrant public function __construct( RefreshTokenRepositoryInterface $refreshTokenRepository, AccessTokenEntityFactory $accessTokenEntityFactory, + protected readonly RefreshTokenIssuer $refreshTokenIssuer, ) { parent::__construct($refreshTokenRepository); $this->accessTokenEntityFactory = $accessTokenEntityFactory; @@ -95,4 +100,20 @@ protected function validateOldRefreshToken(ServerRequestInterface $request, $cli return $refreshTokenData; } + + protected function issueRefreshToken( + OAuth2AccessTokenEntityInterface $accessToken, + string $authCodeId = null, + ): ?RefreshTokenEntityInterface { + if (! is_a($accessToken, AccessTokenEntityInterface::class)) { + throw OidcServerException::serverError('Unexpected access token entity type.'); + } + + return $this->refreshTokenIssuer->issue( + $accessToken, + $this->refreshTokenTTL, + $authCodeId, + self::MAX_RANDOM_TOKEN_GENERATION_ATTEMPTS, + ); + } } diff --git a/src/Server/TokenIssuers/AbstractTokenIssuer.php b/src/Server/TokenIssuers/AbstractTokenIssuer.php new file mode 100644 index 00000000..5b13d703 --- /dev/null +++ b/src/Server/TokenIssuers/AbstractTokenIssuer.php @@ -0,0 +1,17 @@ + 0) { + try { + $refreshToken = $this->refreshTokenEntityFactory->fromData( + $this->helpers->random()->getIdentifier(), + (new DateTimeImmutable())->add($refreshTokenTtl), + $accessToken, + $authCodeId, + ); + $this->refreshTokenRepository->persistNewRefreshToken($refreshToken); + return $refreshToken; + } catch (UniqueTokenIdentifierConstraintViolationException $e) { + if ($maxGenerationAttempts === 0) { + $this->logger->error('Maximum generation attempts reached.', [ + 'maxGenerationAttempts' => $maxGenerationAttempts, + 'accessTokenId' => $accessToken->getIdentifier(), + 'authCodeId' => $authCodeId, + ]); + throw $e; + } + } + } + + $this->logger->error('Unable to issue refresh token.', [ + 'accessTokenId' => $accessToken->getIdentifier(), + 'authCodeId' => $authCodeId, + ]); + + return null; + } +} diff --git a/src/Services/Container.php b/src/Services/Container.php index 6149af27..b4400a87 100644 --- a/src/Services/Container.php +++ b/src/Services/Container.php @@ -93,6 +93,7 @@ use SimpleSAML\Module\oidc\Server\RequestRules\Rules\StateRule; use SimpleSAML\Module\oidc\Server\RequestRules\Rules\UiLocalesRule; use SimpleSAML\Module\oidc\Server\ResponseTypes\IdTokenResponse; +use SimpleSAML\Module\oidc\Server\TokenIssuers\RefreshTokenIssuer; use SimpleSAML\Module\oidc\Server\Validators\BearerTokenValidator; use SimpleSAML\Module\oidc\Stores\Session\LogoutTicketStoreBuilder; use SimpleSAML\Module\oidc\Stores\Session\LogoutTicketStoreDb; @@ -354,6 +355,14 @@ public function __construct() $this->services[Helpers::class] = $helpers; + $refreshTokenIssuer = new RefreshTokenIssuer( + $helpers, + $refreshTokenRepository, + $refreshTokenEntityFactory, + $loggerService, + ); + $this->services[RefreshTokenIssuer::class] = $refreshTokenIssuer; + $authCodeGrantFactory = new AuthCodeGrantFactory( $moduleConfig, $authCodeRepository, @@ -363,8 +372,7 @@ public function __construct() $requestParamsResolver, $accessTokenEntityFactory, $authCodeEntityFactory, - $refreshTokenEntityFactory, - $loggerService, + $refreshTokenIssuer, ); $this->services[AuthCodeGrant::class] = $authCodeGrantFactory->build(); @@ -385,6 +393,7 @@ public function __construct() $moduleConfig, $refreshTokenRepository, $accessTokenEntityFactory, + $refreshTokenIssuer, ); $this->services[RefreshTokenGrant::class] = $refreshTokenGrantFactory->build(); diff --git a/src/Utils/UniqueIdentifierGenerator.php b/src/Utils/UniqueIdentifierGenerator.php index 5c82c49a..d160caff 100644 --- a/src/Utils/UniqueIdentifierGenerator.php +++ b/src/Utils/UniqueIdentifierGenerator.php @@ -7,6 +7,9 @@ use SimpleSAML\Module\oidc\Server\Exceptions\OidcServerException; use Throwable; +/** + * TODO mivanci Move to Helpers\Random + */ class UniqueIdentifierGenerator { /** diff --git a/tests/unit/src/Server/Grants/AuthCodeGrantTest.php b/tests/unit/src/Server/Grants/AuthCodeGrantTest.php index 9423617b..a0f1b8ed 100644 --- a/tests/unit/src/Server/Grants/AuthCodeGrantTest.php +++ b/tests/unit/src/Server/Grants/AuthCodeGrantTest.php @@ -5,19 +5,17 @@ namespace SimpleSAML\Test\Module\oidc\unit\Server\Grants; use DateInterval; -use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\MockObject\Stub; use PHPUnit\Framework\TestCase; use SimpleSAML\Module\oidc\Factories\Entities\AccessTokenEntityFactory; use SimpleSAML\Module\oidc\Factories\Entities\AuthCodeEntityFactory; -use SimpleSAML\Module\oidc\Factories\Entities\RefreshTokenEntityFactory; use SimpleSAML\Module\oidc\ModuleConfig; use SimpleSAML\Module\oidc\Repositories\Interfaces\AccessTokenRepositoryInterface; use SimpleSAML\Module\oidc\Repositories\Interfaces\AuthCodeRepositoryInterface; use SimpleSAML\Module\oidc\Repositories\Interfaces\RefreshTokenRepositoryInterface; use SimpleSAML\Module\oidc\Server\Grants\AuthCodeGrant; use SimpleSAML\Module\oidc\Server\RequestRules\RequestRulesManager; -use SimpleSAML\Module\oidc\Services\LoggerService; +use SimpleSAML\Module\oidc\Server\TokenIssuers\RefreshTokenIssuer; use SimpleSAML\Module\oidc\Utils\RequestParamsResolver; /** @@ -34,8 +32,7 @@ class AuthCodeGrantTest extends TestCase protected Stub $requestParamsResolverStub; protected Stub $accessTokenEntityFactoryStub; protected Stub $authCodeEntityFactoryStub; - protected Stub $refreshTokenEntityFactoryStub; - protected MockObject $loggerMock; + protected Stub $refreshTokenIssuerStub; /** * @throws \Exception @@ -51,8 +48,7 @@ protected function setUp(): void $this->requestParamsResolverStub = $this->createStub(RequestParamsResolver::class); $this->accessTokenEntityFactoryStub = $this->createStub(AccessTokenEntityFactory::class); $this->authCodeEntityFactoryStub = $this->createStub(AuthcodeEntityFactory::class); - $this->refreshTokenEntityFactoryStub = $this->createStub(RefreshTokenEntityFactory::class); - $this->loggerMock = $this->createMock(LoggerService::class); + $this->refreshTokenIssuerStub = $this->createStub(RefreshTokenIssuer::class); } /** @@ -71,8 +67,7 @@ public function testCanCreateInstance(): void $this->requestParamsResolverStub, $this->accessTokenEntityFactoryStub, $this->authCodeEntityFactoryStub, - $this->refreshTokenEntityFactoryStub, - $this->loggerMock, + $this->refreshTokenIssuerStub, ), ); }