From 347144fbf0eaad26b79990222f86426fed317c46 Mon Sep 17 00:00:00 2001 From: Orazio Maico Date: Mon, 28 Oct 2024 21:18:20 +0100 Subject: [PATCH 1/3] Add token validation and logging to HS256TokenDecoder * **Token Validation** - Add signature validation using `firebase/php-jwt` library - Add expiration check by verifying the `exp` claim in the token payload - Add issuer validation by checking the `iss` claim in the token payload - Add audience validation by checking the `aud` claim in the token payload * **Logging** - Add logging for invalid tokens using the existing logging infrastructure * **TokenDecoderException** - Add specific error codes for different types of token validation failures --- src/Exception/TokenDecoderException.php | 20 +++++++++++ src/Token/HS256TokenDecoder.php | 44 ++++++++++++++++++++----- 2 files changed, 56 insertions(+), 8 deletions(-) diff --git a/src/Exception/TokenDecoderException.php b/src/Exception/TokenDecoderException.php index 99c05be..87d8759 100644 --- a/src/Exception/TokenDecoderException.php +++ b/src/Exception/TokenDecoderException.php @@ -12,4 +12,24 @@ public function __construct(string $string, \Exception $e) { parent::__construct($string, self::CODE, $e); } + + public static function forSignatureValidationFailure(\Exception $e): self + { + return new self('Signature validation failed', $e); + } + + public static function forExpiration(\Exception $e): self + { + return new self('Token has expired', $e); + } + + public static function forIssuerMismatch(\Exception $e): self + { + return new self('Issuer mismatch', $e); + } + + public static function forAudienceMismatch(\Exception $e): self + { + return new self('Audience mismatch', $e); + } } diff --git a/src/Token/HS256TokenDecoder.php b/src/Token/HS256TokenDecoder.php index 93357f4..4d81c8d 100644 --- a/src/Token/HS256TokenDecoder.php +++ b/src/Token/HS256TokenDecoder.php @@ -4,24 +4,52 @@ namespace Mainick\KeycloakClientBundle\Token; +use Firebase\JWT\JWT; +use Firebase\JWT\Key; use Mainick\KeycloakClientBundle\Exception\TokenDecoderException; use Mainick\KeycloakClientBundle\Interface\TokenDecoderInterface; +use Psr\Log\LoggerInterface; class HS256TokenDecoder implements TokenDecoderInterface { - public function decode(string $token, string $key): array + private LoggerInterface $logger; + + public function __construct(LoggerInterface $logger) { - // https://github.com/firebase/php-jwt#example-encodedecode-headers - [$headersB64, $payloadB64, $sig] = explode('.', $token); - $tokenDecoded = json_decode(base64_decode($payloadB64), true, 512, JSON_THROW_ON_ERROR); + $this->logger = $logger; + } + public function decode(string $token, string $key): array + { try { - $json = json_encode($tokenDecoded, JSON_THROW_ON_ERROR); + $decoded = JWT::decode($token, new Key($key, 'HS256')); - return json_decode($json, true, 512, JSON_THROW_ON_ERROR); - } - catch (\Exception $e) { + $this->validateToken($decoded); + + return (array) $decoded; + } catch (\Exception $e) { + $this->logger->error('Error decoding token', ['exception' => $e]); throw new TokenDecoderException('Error decoding token', $e); } } + + private function validateToken($token): void + { + $now = time(); + + if ($token->exp < $now) { + $this->logger->error('Token has expired', ['exp' => $token->exp, 'now' => $now]); + throw new TokenDecoderException('Token has expired'); + } + + if ($token->iss !== 'trusted-issuer') { + $this->logger->error('Invalid token issuer', ['iss' => $token->iss]); + throw new TokenDecoderException('Invalid token issuer'); + } + + if ($token->aud !== 'your-audience') { + $this->logger->error('Invalid token audience', ['aud' => $token->aud]); + throw new TokenDecoderException('Invalid token audience'); + } + } } From d1ba41cd7671bbeb2922b8aadb1aa7d926c2e9b9 Mon Sep 17 00:00:00 2001 From: mainick Date: Tue, 19 Nov 2024 12:50:36 +0100 Subject: [PATCH 2/3] fix factory to token decoder --- src/Token/TokenDecoderFactory.php | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/Token/TokenDecoderFactory.php b/src/Token/TokenDecoderFactory.php index 0465118..c439506 100644 --- a/src/Token/TokenDecoderFactory.php +++ b/src/Token/TokenDecoderFactory.php @@ -12,13 +12,10 @@ class TokenDecoderFactory #[Pure] public static function create($algorithm): TokenDecoderInterface { - switch ($algorithm) { - case 'RS256': - return new RS256TokenDecoder(); - case 'HS256': - return new HS256TokenDecoder(); - default: - throw new \RuntimeException('Invalid algorithm'); - } + return match ($algorithm) { + 'RS256' => new RS256TokenDecoder(), + 'HS256' => new HS256TokenDecoder(), + default => throw new \RuntimeException('Invalid algorithm'), + }; } } From 4d1a11938ff048fe6d05aa3646399bf218172006 Mon Sep 17 00:00:00 2001 From: mainick Date: Tue, 19 Nov 2024 16:27:50 +0100 Subject: [PATCH 3/3] Add token validation and logging to HS256TokenDecoder --- src/Interface/TokenDecoderInterface.php | 6 +++++ src/Provider/KeycloakClient.php | 1 + src/Token/HS256TokenDecoder.php | 35 +++++++++---------------- src/Token/RS256TokenDecoder.php | 19 ++++++++++---- src/Token/TokenDecoderFactory.php | 7 +++-- tests/Provider/KeycloakClientTest.php | 4 ++- 6 files changed, 41 insertions(+), 31 deletions(-) diff --git a/src/Interface/TokenDecoderInterface.php b/src/Interface/TokenDecoderInterface.php index de18ae7..1463f8b 100644 --- a/src/Interface/TokenDecoderInterface.php +++ b/src/Interface/TokenDecoderInterface.php @@ -10,4 +10,10 @@ interface TokenDecoderInterface * @return array */ public function decode(string $token, string $key): array; + + /** + * @param string $realm + * @param array $tokenDecoded + */ + public function validateToken(string $realm, array $tokenDecoded): void; } diff --git a/src/Provider/KeycloakClient.php b/src/Provider/KeycloakClient.php index 5ab5859..4d066ab 100644 --- a/src/Provider/KeycloakClient.php +++ b/src/Provider/KeycloakClient.php @@ -103,6 +103,7 @@ public function verifyToken(AccessTokenInterface $token): ?UserRepresentationDTO $decoder = TokenDecoderFactory::create($this->encryption_algorithm); $tokenDecoded = $decoder->decode($accessToken->getToken(), $this->encryption_key); + $decoder->validateToken($this->realm, $tokenDecoded); $this->keycloakClientLogger->info('KeycloakClient::verifyToken', [ 'tokenDecoded' => $tokenDecoded, ]); diff --git a/src/Token/HS256TokenDecoder.php b/src/Token/HS256TokenDecoder.php index 4d81c8d..f1521d9 100644 --- a/src/Token/HS256TokenDecoder.php +++ b/src/Token/HS256TokenDecoder.php @@ -8,48 +8,37 @@ use Firebase\JWT\Key; use Mainick\KeycloakClientBundle\Exception\TokenDecoderException; use Mainick\KeycloakClientBundle\Interface\TokenDecoderInterface; -use Psr\Log\LoggerInterface; class HS256TokenDecoder implements TokenDecoderInterface { - private LoggerInterface $logger; - - public function __construct(LoggerInterface $logger) - { - $this->logger = $logger; - } public function decode(string $token, string $key): array { try { - $decoded = JWT::decode($token, new Key($key, 'HS256')); + $tokenDecoded = JWT::decode($token, new Key($key, 'HS256')); - $this->validateToken($decoded); + $json = json_encode($tokenDecoded, JSON_THROW_ON_ERROR); - return (array) $decoded; + return json_decode($json, true, 512, JSON_THROW_ON_ERROR); } catch (\Exception $e) { - $this->logger->error('Error decoding token', ['exception' => $e]); throw new TokenDecoderException('Error decoding token', $e); } } - private function validateToken($token): void + public function validateToken(string $realm, array $tokenDecoded): void { $now = time(); - if ($token->exp < $now) { - $this->logger->error('Token has expired', ['exp' => $token->exp, 'now' => $now]); - throw new TokenDecoderException('Token has expired'); - } - - if ($token->iss !== 'trusted-issuer') { - $this->logger->error('Invalid token issuer', ['iss' => $token->iss]); - throw new TokenDecoderException('Invalid token issuer'); + if ($tokenDecoded['exp'] < $now) { + throw TokenDecoderException::forExpiration(new \Exception('Token has expired')); } - if ($token->aud !== 'your-audience') { - $this->logger->error('Invalid token audience', ['aud' => $token->aud]); - throw new TokenDecoderException('Invalid token audience'); + if (str_contains($tokenDecoded['iss'], $realm) === false) { + throw TokenDecoderException::forIssuerMismatch(new \Exception('Invalid token issuer')); } +// +// if ($tokenDecoded['aud'] !== 'account') { +// throw TokenDecoderException::forAudienceMismatch(new \Exception('Invalid token audience')); +// } } } diff --git a/src/Token/RS256TokenDecoder.php b/src/Token/RS256TokenDecoder.php index 417b5fa..98dcdb0 100644 --- a/src/Token/RS256TokenDecoder.php +++ b/src/Token/RS256TokenDecoder.php @@ -13,17 +13,17 @@ class RS256TokenDecoder implements TokenDecoderInterface { public function decode(string $token, string $key): array { - $publicKeyPem = << new RS256TokenDecoder(), - 'HS256' => new HS256TokenDecoder(), + self::ALGORITHM_RS256 => new RS256TokenDecoder(), + self::ALGORITHM_HS256 => new HS256TokenDecoder(), default => throw new \RuntimeException('Invalid algorithm'), }; } diff --git a/tests/Provider/KeycloakClientTest.php b/tests/Provider/KeycloakClientTest.php index 6f4fba2..ba05ef0 100644 --- a/tests/Provider/KeycloakClientTest.php +++ b/tests/Provider/KeycloakClientTest.php @@ -34,7 +34,7 @@ class KeycloakClientTest extends TestCase "exp": "%s", "iat": "%s", "jti": "e11a85c8-aa91-4f75-9088-57db4586f8b9", - "iss": "https://example.org/auth/realms/test-realm", + "iss": "https://example.org/auth/realms/mock_realm", "aud": "account", "nbf": "%s", "sub": "4332085e-b944-4acc-9eb1-27d8f5405f3e", @@ -93,6 +93,8 @@ protected function setUp(): void 'test-app', 'mock_secret', 'none', + self::ENCRYPTION_ALGORITHM, + self::ENCRYPTION_KEY ); $jwt_tmp = sprintf($this->jwtTemplate, time() + 3600, time(), time());