From 37131c6545c579f38254c1ada0cc3f493ae9152f Mon Sep 17 00:00:00 2001 From: Thomas Durand Date: Sat, 10 Feb 2024 11:01:49 +0100 Subject: [PATCH 1/3] Use symfony/clock component when available instead of non-testable time() --- Command/GenerateTokenCommand.php | 8 +++++++- Security/Http/Cookie/JWTCookieProvider.php | 8 +++++++- Services/JWSProvider/DefaultJWSProvider.php | 11 +++++++++-- Services/JWSProvider/LcobucciJWSProvider.php | 11 ++++++++--- Signature/LoadedJWS.php | 17 +++++++++++++++-- ...onalAccessTokenClaimsAndHeaderSubscriber.php | 13 ++++++++++--- composer.json | 4 +++- 7 files changed, 59 insertions(+), 13 deletions(-) diff --git a/Command/GenerateTokenCommand.php b/Command/GenerateTokenCommand.php index 3d96fe6d..165e911b 100644 --- a/Command/GenerateTokenCommand.php +++ b/Command/GenerateTokenCommand.php @@ -3,6 +3,7 @@ namespace Lexik\Bundle\JWTAuthenticationBundle\Command; use Lexik\Bundle\JWTAuthenticationBundle\Services\JWTTokenManagerInterface; +use Symfony\Component\Clock\Clock; use Symfony\Component\Console\Attribute\AsCommand; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputArgument; @@ -95,7 +96,12 @@ protected function execute(InputInterface $input, OutputInterface $output): int if (null !== $input->getOption('ttl') && ((int) $input->getOption('ttl')) == 0) { $payload['exp'] = 0; } elseif (null !== $input->getOption('ttl') && ((int) $input->getOption('ttl')) > 0) { - $payload['exp'] = time() + $input->getOption('ttl'); + if (class_exists(Clock::class)) { + $now = Clock::get()->now()->getTimestamp(); + } else { + $now = time(); + } + $payload['exp'] = $now + $input->getOption('ttl'); } $token = $this->tokenManager->createFromPayload($user, $payload); diff --git a/Security/Http/Cookie/JWTCookieProvider.php b/Security/Http/Cookie/JWTCookieProvider.php index 5d211104..139aa736 100644 --- a/Security/Http/Cookie/JWTCookieProvider.php +++ b/Security/Http/Cookie/JWTCookieProvider.php @@ -3,6 +3,7 @@ namespace Lexik\Bundle\JWTAuthenticationBundle\Security\Http\Cookie; use Lexik\Bundle\JWTAuthenticationBundle\Helper\JWTSplitter; +use Symfony\Component\Clock\Clock; use Symfony\Component\HttpFoundation\Cookie; use Symfony\Component\HttpKernel\Kernel; @@ -62,7 +63,12 @@ public function createCookie(string $jwt, ?string $name = null, $expiresAt = nul $jwt = $jwtParts->getParts($split ?: $this->defaultSplit); if (null === $expiresAt) { - $expiresAt = 0 === $this->defaultLifetime ? 0 : (time() + $this->defaultLifetime); + if (class_exists(Clock::class)) { + $now = Clock::get()->now()->getTimestamp(); + } else { + $now = time(); + } + $expiresAt = 0 === $this->defaultLifetime ? 0 : ($now + $this->defaultLifetime); } return Cookie::create( diff --git a/Services/JWSProvider/DefaultJWSProvider.php b/Services/JWSProvider/DefaultJWSProvider.php index 9c50f5ae..96a4c7cf 100644 --- a/Services/JWSProvider/DefaultJWSProvider.php +++ b/Services/JWSProvider/DefaultJWSProvider.php @@ -8,6 +8,7 @@ use Lexik\Bundle\JWTAuthenticationBundle\Signature\CreatedJWS; use Lexik\Bundle\JWTAuthenticationBundle\Signature\LoadedJWS; use Namshi\JOSE\JWS; +use Symfony\Component\Clock\Clock; /** * JWS Provider, Namshi\JOSE library integration. @@ -82,13 +83,19 @@ public function __construct(KeyLoaderInterface $keyLoader, $cryptoEngine, $signa */ public function create(array $payload, array $header = []) { + if (class_exists(Clock::class)) { + $now = Clock::get()->now()->getTimestamp(); + } else { + $now = time(); + } + $header['alg'] = $this->signatureAlgorithm; $jws = new JWS($header, $this->cryptoEngine); - $claims = ['iat' => time()]; + $claims = ['iat' => $now]; if (null !== $this->ttl && !isset($payload['exp'])) { - $claims['exp'] = time() + $this->ttl; + $claims['exp'] = $now + $this->ttl; } $jws->setPayload($payload + $claims); diff --git a/Services/JWSProvider/LcobucciJWSProvider.php b/Services/JWSProvider/LcobucciJWSProvider.php index b3394743..c61631f6 100644 --- a/Services/JWSProvider/LcobucciJWSProvider.php +++ b/Services/JWSProvider/LcobucciJWSProvider.php @@ -2,7 +2,6 @@ namespace Lexik\Bundle\JWTAuthenticationBundle\Services\JWSProvider; -use Lcobucci\Clock\Clock; use Lcobucci\Clock\SystemClock; use Lcobucci\JWT\Builder; use Lcobucci\JWT\Encoding\ChainedFormatter; @@ -30,6 +29,8 @@ use Lexik\Bundle\JWTAuthenticationBundle\Services\KeyLoader\RawKeyLoader; use Lexik\Bundle\JWTAuthenticationBundle\Signature\CreatedJWS; use Lexik\Bundle\JWTAuthenticationBundle\Signature\LoadedJWS; +use Symfony\Component\Clock\Clock as SymfonyClock; +use Psr\Clock\ClockInterface as Clock; /** * @final @@ -77,7 +78,11 @@ public function __construct(KeyLoaderInterface $keyLoader, string $cryptoEngine, throw new \InvalidArgumentException(sprintf('The %s provider supports only "openssl" as crypto engine.', self::class)); } if (null === $clock) { - $clock = new SystemClock(new \DateTimeZone('UTC')); + if (class_exists(SymfonyClock::class)) { + $clock = SymfonyClock::get(); + } else { + $clock = new SystemClock(new \DateTimeZone('UTC')); + } } $this->keyLoader = $keyLoader; @@ -103,7 +108,7 @@ public function create(array $payload, array $header = []) $jws = $jws->withHeader($k, $v); } - $now = time(); + $now = $this->clock->now()->getTimestamp(); $issuedAt = $payload['iat'] ?? $now; unset($payload['iat']); diff --git a/Signature/LoadedJWS.php b/Signature/LoadedJWS.php index 2b090f0b..f3e0fc90 100644 --- a/Signature/LoadedJWS.php +++ b/Signature/LoadedJWS.php @@ -2,6 +2,8 @@ namespace Lexik\Bundle\JWTAuthenticationBundle\Signature; +use Symfony\Component\Clock\Clock; + /** * Object representation of a JSON Web Signature loaded from an * existing JSON Web Token. @@ -74,7 +76,13 @@ private function checkExpiration(): void return; } - if ($this->clockSkew <= time() - $this->payload['exp']) { + if (class_exists(Clock::class)) { + $now = Clock::get()->now()->getTimestamp(); + } else { + $now = time(); + } + + if ($this->clockSkew <= $now - $this->payload['exp']) { $this->state = self::EXPIRED; } } @@ -84,7 +92,12 @@ private function checkExpiration(): void */ private function checkIssuedAt() { - if (isset($this->payload['iat']) && (int) $this->payload['iat'] - $this->clockSkew > time()) { + if (class_exists(Clock::class)) { + $now = Clock::get()->now()->getTimestamp(); + } else { + $now = time(); + } + if (isset($this->payload['iat']) && (int) $this->payload['iat'] - $this->clockSkew > $now) { return $this->state = self::INVALID; } } diff --git a/Subscriber/AdditionalAccessTokenClaimsAndHeaderSubscriber.php b/Subscriber/AdditionalAccessTokenClaimsAndHeaderSubscriber.php index 18fdcbc4..bf8b3198 100644 --- a/Subscriber/AdditionalAccessTokenClaimsAndHeaderSubscriber.php +++ b/Subscriber/AdditionalAccessTokenClaimsAndHeaderSubscriber.php @@ -4,6 +4,7 @@ use Lexik\Bundle\JWTAuthenticationBundle\Event\JWTCreatedEvent; use Lexik\Bundle\JWTAuthenticationBundle\Events; +use Symfony\Component\Clock\Clock; use Symfony\Component\EventDispatcher\EventSubscriberInterface; final class AdditionalAccessTokenClaimsAndHeaderSubscriber implements EventSubscriberInterface @@ -29,14 +30,20 @@ public static function getSubscribedEvents(): array public function addClaims(JWTCreatedEvent $event): void { + if (class_exists(Clock::class)) { + $now = Clock::get()->now()->getTimestamp(); + } else { + $now = time(); + } + $claims = [ 'jti' => uniqid('', true), - 'iat' => time(), - 'nbf' => time(), + 'iat' => $now, + 'nbf' => $now, ]; $data = $event->getData(); if (!array_key_exists('exp', $data) && $this->ttl > 0) { - $claims['exp'] = time() + $this->ttl; + $claims['exp'] = $now + $this->ttl; } $event->setData(array_merge($claims, $data)); } diff --git a/composer.json b/composer.json index 618bee82..bbce83af 100644 --- a/composer.json +++ b/composer.json @@ -55,6 +55,7 @@ "require-dev": { "symfony/browser-kit": "^5.4|^6.0|^7.0", "symfony/console": "^4.4|^5.4|^6.0|^7.0", + "symfony/clock": "^6.0|^7.0", "symfony/dom-crawler": "^5.4|^6.0|^7.0", "symfony/filesystem": "^4.4|^5.4|^6.0|^7.0", "symfony/framework-bundle": "^4.4|^5.4|^6.0|^7.0", @@ -68,7 +69,8 @@ }, "suggest": { "gesdinet/jwt-refresh-token-bundle": "Implements a refresh token system over Json Web Tokens in Symfony", - "spomky-labs/lexik-jose-bridge": "Provides a JWT Token encoder with encryption support" + "spomky-labs/lexik-jose-bridge": "Provides a JWT Token encoder with encryption support", + "symfony/clock": "Allow to test JWT with a shared and expectable clock" }, "autoload": { "psr-4": { From cd45958bb5965fe517de014c8b1323dadcc7c2b4 Mon Sep 17 00:00:00 2001 From: Thomas Durand Date: Sat, 10 Feb 2024 11:11:39 +0100 Subject: [PATCH 2/3] Removed symfony/clock dev requirement, to test php < 8.1 tests --- composer.json | 1 - 1 file changed, 1 deletion(-) diff --git a/composer.json b/composer.json index bbce83af..e9623032 100644 --- a/composer.json +++ b/composer.json @@ -55,7 +55,6 @@ "require-dev": { "symfony/browser-kit": "^5.4|^6.0|^7.0", "symfony/console": "^4.4|^5.4|^6.0|^7.0", - "symfony/clock": "^6.0|^7.0", "symfony/dom-crawler": "^5.4|^6.0|^7.0", "symfony/filesystem": "^4.4|^5.4|^6.0|^7.0", "symfony/framework-bundle": "^4.4|^5.4|^6.0|^7.0", From 097fb89964d8662332e15fda87c07128536a6e60 Mon Sep 17 00:00:00 2001 From: Thomas Durand Date: Sat, 10 Feb 2024 11:16:19 +0100 Subject: [PATCH 3/3] Fixed flawky UT --- Tests/Signature/LoadedJWSTest.php | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/Tests/Signature/LoadedJWSTest.php b/Tests/Signature/LoadedJWSTest.php index 8b97689f..eed3cff8 100644 --- a/Tests/Signature/LoadedJWSTest.php +++ b/Tests/Signature/LoadedJWSTest.php @@ -6,6 +6,8 @@ use Lexik\Bundle\JWTAuthenticationBundle\Tests\ForwardCompatTestCaseTrait; use PHPUnit\Framework\TestCase; use Symfony\Bridge\PhpUnit\ClockMock; +use Symfony\Component\Clock\Clock; +use Symfony\Component\Clock\MockClock; /** * Tests the CreatedJWS model class. @@ -128,7 +130,11 @@ public function testIsNotExpiredDaySavingTransition() { // 2020-10-25 00:16:13 UTC+0 $timestamp = 1603584973; - ClockMock::withClockMock($timestamp); + if (class_exists(Clock::class)) { + Clock::set(new MockClock("@$timestamp")); + } else { + ClockMock::withClockMock($timestamp); + } $dstPayload = [ 'username' => 'test',