From bc5303eb0aacb4f3000b7f8f5b19af78c7307b21 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=A9my=20Munsch?= Date: Wed, 26 Aug 2026 10:01:43 +0200 Subject: [PATCH] fix(security): make nonce validation and consumption atomic via CacheRepository add (F-2026-0008) --- .../Services/CachedNonceValidator.php | 30 +++++-------------- .../Services/CachedNonceValidatorTest.php | 25 ++++++---------- 2 files changed, 17 insertions(+), 38 deletions(-) diff --git a/src/Infra/Adapters/Services/CachedNonceValidator.php b/src/Infra/Adapters/Services/CachedNonceValidator.php index 4300a7a..84c9d34 100644 --- a/src/Infra/Adapters/Services/CachedNonceValidator.php +++ b/src/Infra/Adapters/Services/CachedNonceValidator.php @@ -8,8 +8,8 @@ use Application\Security\Models\ValueObjects\ClientIdentifier; use Application\Security\Models\ValueObjects\Request\Nonce; use Application\Security\Ports\Services\NonceValidator; use Application\Security\Ports\Services\TimestampValidator; +use Illuminate\Contracts\Cache\Repository as CacheRepository; use Psr\Log\LoggerInterface; -use Psr\SimpleCache\CacheInterface; final readonly class CachedNonceValidator implements NonceValidator { @@ -18,8 +18,8 @@ final readonly class CachedNonceValidator implements NonceValidator private const int CACHE_TTL_SECONDS = TimestampValidator::ALLOWED_TIMESTAMP_SKEW_SECONDS; public function __construct( - private readonly LoggerInterface $logger, - private readonly CacheInterface $cacheStore, + private LoggerInterface $logger, + private CacheRepository $cacheStore, ) { } @@ -28,7 +28,9 @@ final readonly class CachedNonceValidator implements NonceValidator { $key = $this->getCacheKey($forClientIdentifier, $nonce); - if ($this->isKeyConsumed($key)) { + $added = $this->cacheStore->add($key, 'used', self::CACHE_TTL_SECONDS); + + if (! $added) { $this->logger->debug( 'Nonce already used for client: {clientIdentifier}, nonce: {nonce}', [ @@ -40,28 +42,12 @@ final readonly class CachedNonceValidator implements NonceValidator return false; } - $this->consumeKey($key); - - return true; - } - - private function isKeyConsumed(string $key): bool - { - return $this->cacheStore->has($key); - } - - private function consumeKey(string $key): void - { - $this->cacheStore->set( - $key, - 'used', - self::CACHE_TTL_SECONDS - ); - $this->logger->debug( 'Nonce consumed for key: {key}', ['key' => $key] ); + + return true; } private function getCacheKey(ClientIdentifier $forClientIdentifier, Nonce $nonce): string diff --git a/tests/Unit/Infra/Adapters/Services/CachedNonceValidatorTest.php b/tests/Unit/Infra/Adapters/Services/CachedNonceValidatorTest.php index 51e42e3..b38afa0 100644 --- a/tests/Unit/Infra/Adapters/Services/CachedNonceValidatorTest.php +++ b/tests/Unit/Infra/Adapters/Services/CachedNonceValidatorTest.php @@ -6,18 +6,18 @@ namespace Tests\Unit\Infra\Adapters\Services; use Application\Security\Models\ValueObjects\ClientIdentifier; use Application\Security\Models\ValueObjects\Request\Nonce; +use Illuminate\Contracts\Cache\Repository as CacheRepository; use Infra\Adapters\Services\CachedNonceValidator; use Mockery; use Psr\Log\NullLogger; -use Psr\SimpleCache\CacheInterface; use Ramsey\Uuid\Uuid; covers(CachedNonceValidator::class); describe('CachedNonceValidator Unit Tests', function (): void { - it('validates and consumes a new nonce', function (): void { + it('validates and consumes a new nonce atomically', function (): void { // GIVEN - $cacheStore = Mockery::mock(CacheInterface::class); + $cacheStore = Mockery::mock(CacheRepository::class); $logger = new NullLogger(); $validator = new CachedNonceValidator($logger, $cacheStore); @@ -26,12 +26,7 @@ describe('CachedNonceValidator Unit Tests', function (): void { $expectedKey = 'nonce:' . $clientIdentifier->value . ':' . $nonce->value->toString(); - $cacheStore->expects('has') - ->once() - ->with($expectedKey) - ->andReturn(false); - - $cacheStore->expects('set') + $cacheStore->expects('add') ->once() ->with($expectedKey, 'used', 300) ->andReturn(true); @@ -43,9 +38,9 @@ describe('CachedNonceValidator Unit Tests', function (): void { expect($isValid)->toBeTrue(); }); - it('rejects an already consumed nonce', function (): void { + it('rejects an already consumed nonce when atomic add returns false', function (): void { // GIVEN - $cacheStore = Mockery::mock(CacheInterface::class); + $cacheStore = Mockery::mock(CacheRepository::class); $logger = new NullLogger(); $validator = new CachedNonceValidator($logger, $cacheStore); @@ -54,12 +49,10 @@ describe('CachedNonceValidator Unit Tests', function (): void { $expectedKey = 'nonce:' . $clientIdentifier->value . ':' . $nonce->value->toString(); - $cacheStore->expects('has') + $cacheStore->expects('add') ->once() - ->with($expectedKey) - ->andReturn(true); - - $cacheStore->shouldNotReceive('set'); + ->with($expectedKey, 'used', 300) + ->andReturn(false); // WHEN $isValid = $validator->validateAndConsume($nonce, $clientIdentifier);