From 1ce88d21caa9ad2533ca4cb8cde00c1b9b3e771b Mon Sep 17 00:00:00 2001 From: mainick Date: Mon, 5 Jan 2026 18:15:24 +0100 Subject: [PATCH 01/11] add test to validate JWKS endpoint URL requires HTTPS for non-localhost --- tests/Token/JWKSTokenDecoderTest.php | 49 +++++++++++++++++++++++++--- 1 file changed, 45 insertions(+), 4 deletions(-) diff --git a/tests/Token/JWKSTokenDecoderTest.php b/tests/Token/JWKSTokenDecoderTest.php index 3d40cc4..29cb5d9 100644 --- a/tests/Token/JWKSTokenDecoderTest.php +++ b/tests/Token/JWKSTokenDecoderTest.php @@ -171,10 +171,51 @@ public function testWildcardDomainMatching(): void public function testRequiresHttpsForJwksEndpoint(): void { - // For this test, we need to test the actual JWKS fetching - // We can't easily test this without a real token decode, but the validation - // is covered by the previous tests - $this->assertTrue(true); + // This test verifies that the JWKS endpoint URL validation rejects HTTP + // for non-localhost domains during token decoding. We create a decoder + // with valid HTTPS, then simulate an HTTP JWKS URL fetch scenario. + + // Create a mock HTTP client + $httpClient = $this->createMock(ClientInterface::class); + + // Create a valid decoder first with localhost HTTP (which is allowed) + $decoder = new JWKSTokenDecoder([ + 'base_url' => 'http://localhost:8080', + 'realm' => 'test-realm', + ], $httpClient); + + // Use reflection to modify the base_url to an HTTP non-localhost domain + // This simulates a scenario where the JWKS URL would be HTTP for a non-localhost host + $reflection = new \ReflectionClass($decoder); + $optionsProperty = $reflection->getProperty('options'); + $optionsProperty->setAccessible(true); + $options = $optionsProperty->getValue($decoder); + $options['base_url'] = 'http://keycloak.example.com'; + $optionsProperty->setValue($decoder, $options); + + // Create a JWT token with proper structure + $header = json_encode([ + 'kid' => 'test-key-id', + 'alg' => 'RS256', + 'typ' => 'JWT', + ], JSON_THROW_ON_ERROR); + $payload = json_encode([ + 'sub' => 'test-user', + 'exp' => time() + 3600, + 'iat' => time(), + 'iss' => 'http://keycloak.example.com/auth/realms/test-realm', + ], JSON_THROW_ON_ERROR); + + // Base64url encode the token parts + $headerEncoded = rtrim(strtr(base64_encode($header), '+/', '-_'), '='); + $payloadEncoded = rtrim(strtr(base64_encode($payload), '+/', '-_'), '='); + $token = "$headerEncoded.$payloadEncoded.fake-signature"; + + $this->expectException(TokenDecoderException::class); + $this->expectExceptionMessage('JWKS endpoint must use HTTPS for non-localhost hosts'); + + // Attempt to decode the token - this should trigger fetchJwks which validates the JWKS URL + $decoder->decode($token, ''); } } From 94be97e468eaa8e4785b6aea9c455c4dff7578ca Mon Sep 17 00:00:00 2001 From: mainick Date: Mon, 5 Jan 2026 18:16:09 +0100 Subject: [PATCH 02/11] fix: remove redundant exception handling for token decoding errors --- src/Token/JWKSTokenDecoder.php | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/Token/JWKSTokenDecoder.php b/src/Token/JWKSTokenDecoder.php index c2ceeb6..69c364a 100644 --- a/src/Token/JWKSTokenDecoder.php +++ b/src/Token/JWKSTokenDecoder.php @@ -71,9 +71,6 @@ public function decode(string $token, string $key): array return json_decode($json, true, 512, JSON_THROW_ON_ERROR); } - catch (TokenDecoderException $e) { - throw $e; - } catch (\JsonException $e) { throw TokenDecoderException::forDecodingError('JSON parsing failed: ' . $e->getMessage(), $e); } From 0e05a72b194da751e2abd774de93c01ad7455322 Mon Sep 17 00:00:00 2001 From: mainick Date: Mon, 5 Jan 2026 18:20:49 +0100 Subject: [PATCH 03/11] fix: ensure HTTP client is configured before fetching JWKS --- src/Token/JWKSTokenDecoder.php | 28 +++++++++++----------------- 1 file changed, 11 insertions(+), 17 deletions(-) diff --git a/src/Token/JWKSTokenDecoder.php b/src/Token/JWKSTokenDecoder.php index 69c364a..af2348c 100644 --- a/src/Token/JWKSTokenDecoder.php +++ b/src/Token/JWKSTokenDecoder.php @@ -121,25 +121,19 @@ private function fetchJwks(): array $this->validateJwksUrl($url); try { - if ($this->httpClient !== null) { - $response = $this->httpClient->request('GET', $url, [ - 'timeout' => 10, - 'connect_timeout' => 5, - ]); - $json = $response->getBody()->getContents(); - } else { - // Fallback to file_get_contents if no HTTP client provided - $context = stream_context_create([ - 'http' => [ - 'timeout' => 10, - ], - ]); - $json = file_get_contents($url, false, $context); - if ($json === false) { - throw new \RuntimeException('Unable to fetch JWKS from endpoint'); - } + if ($this->httpClient === null) { + throw TokenDecoderException::forJwksError( + 'HTTP client is not configured; unable to fetch JWKS.', + new \RuntimeException('Missing HTTP client for JWKS retrieval') + ); } + $response = $this->httpClient->request('GET', $url, [ + 'timeout' => 10, + 'connect_timeout' => 5, + ]); + $json = $response->getBody()->getContents(); + $data = json_decode($json, true, 512, JSON_THROW_ON_ERROR); return $data['keys'] ?? []; From 3cd3380eaec53c4c581915bdc4a50c89077cfd24 Mon Sep 17 00:00:00 2001 From: mainick Date: Mon, 5 Jan 2026 18:25:54 +0100 Subject: [PATCH 04/11] fix: throw exception for empty byte string in ASN.1 encoding --- src/Token/JWKSTokenDecoder.php | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/Token/JWKSTokenDecoder.php b/src/Token/JWKSTokenDecoder.php index af2348c..8cbac90 100644 --- a/src/Token/JWKSTokenDecoder.php +++ b/src/Token/JWKSTokenDecoder.php @@ -217,6 +217,13 @@ private function base64urlDecode(string $data): string private function encodeAsn1Integer(string $bytes): string { + if ($bytes === '') { + throw TokenDecoderException::forDecodingError( + 'Empty byte string provided for ASN.1 integer encoding', + new \Exception('Bytes string must not be empty') + ); + } + if (ord($bytes[0]) > 0x7F) { $bytes = "\x00".$bytes; } From edaeed5f20c90bc533b836c50aea1a27e20e5d70 Mon Sep 17 00:00:00 2001 From: mainick Date: Mon, 5 Jan 2026 18:28:41 +0100 Subject: [PATCH 05/11] fix: validate JWT format to ensure proper decoding process --- src/Token/JWKSTokenDecoder.php | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/Token/JWKSTokenDecoder.php b/src/Token/JWKSTokenDecoder.php index 8cbac90..dac22be 100644 --- a/src/Token/JWKSTokenDecoder.php +++ b/src/Token/JWKSTokenDecoder.php @@ -50,7 +50,15 @@ public function __construct( public function decode(string $token, string $key): array { try { - [$headerB64] = explode('.', $token, 2); + $parts = explode('.', $token); + if (\count($parts) !== 3 || $parts[0] === '' || $parts[1] === '' || $parts[2] === '') { + throw TokenDecoderException::forDecodingError( + 'Invalid JWT format: token must consist of header.payload.signature', + new \Exception('invalid token format') + ); + } + + [$headerB64] = $parts; $header = json_decode($this->base64urlDecode($headerB64), true, 512, JSON_THROW_ON_ERROR); $kid = $header['kid'] ?? ''; From 5ee0b73876b5c413fd4566c6a5237d89fc9d0d5d Mon Sep 17 00:00:00 2001 From: mainick Date: Mon, 5 Jan 2026 18:29:37 +0100 Subject: [PATCH 06/11] fix: handle failure in decoding RSA encryption OID to prevent errors --- src/Token/JWKSTokenDecoder.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/Token/JWKSTokenDecoder.php b/src/Token/JWKSTokenDecoder.php index dac22be..8bc19cf 100644 --- a/src/Token/JWKSTokenDecoder.php +++ b/src/Token/JWKSTokenDecoder.php @@ -202,6 +202,12 @@ private function jwkToPem(array $jwk): string $seq = $this->encodeAsn1Sequence($modulusEnc.$exponentEnc); $algo = hex2bin('300d06092a864886f70d0101010500'); // rsaEncryption OID + if ($algo === false) { + throw TokenDecoderException::forJwksError( + 'Failed to decode RSA encryption OID', + new \Exception('hex2bin returned false for hardcoded rsaEncryption OID') + ); + } $bitStr = "\x03".chr(strlen($seq) + 1)."\x00".$seq; $spki = $this->encodeAsn1Sequence($algo.$bitStr); From 94e4d08b9046f6c6e2cb2a8835e2a19c40ef38b0 Mon Sep 17 00:00:00 2001 From: mainick Date: Mon, 5 Jan 2026 18:32:37 +0100 Subject: [PATCH 07/11] fix: add HTTP timeout options to prevent request delays --- src/Token/JWKSTokenDecoder.php | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/Token/JWKSTokenDecoder.php b/src/Token/JWKSTokenDecoder.php index 8bc19cf..0fa321e 100644 --- a/src/Token/JWKSTokenDecoder.php +++ b/src/Token/JWKSTokenDecoder.php @@ -10,15 +10,15 @@ use Mainick\KeycloakClientBundle\Exception\TokenDecoderException; use Mainick\KeycloakClientBundle\Interface\TokenDecoderInterface; -final class JWKSTokenDecoder implements TokenDecoderInterface +final readonly class JWKSTokenDecoder implements TokenDecoderInterface { public function __construct( - private readonly array $options, - private readonly ?ClientInterface $httpClient = null + private array $options, + private ?ClientInterface $httpClient = null ) { - foreach (['base_url', 'realm'] as $requiredOption) { + foreach (['base_url', 'realm', 'http_timeout', 'http_connect_timeout'] as $requiredOption) { if (!\array_key_exists($requiredOption, $this->options) || $this->options[$requiredOption] === null || $this->options[$requiredOption] === '') { throw TokenDecoderException::forInvalidConfiguration(\sprintf( "Missing or empty required option '%s' for %s", @@ -123,6 +123,8 @@ private function getPemKeyForKid(string $kid): string private function fetchJwks(): array { + $timeout = $this->options['http_timeout'] ?? 10; + $connectTimeout = $this->options['http_connect_timeout'] ?? 5; $url = sprintf('%s/realms/%s/protocol/openid-connect/certs', $this->options['base_url'], $this->options['realm']); // Validate the constructed JWKS URL @@ -137,8 +139,8 @@ private function fetchJwks(): array } $response = $this->httpClient->request('GET', $url, [ - 'timeout' => 10, - 'connect_timeout' => 5, + 'timeout' => $timeout, + 'connect_timeout' => $connectTimeout, ]); $json = $response->getBody()->getContents(); From 8efb2f5acc55d402bb509c60da1418f1c8f03c01 Mon Sep 17 00:00:00 2001 From: mainick Date: Mon, 5 Jan 2026 18:35:15 +0100 Subject: [PATCH 08/11] fix: correct domain validation logic for wildcard subdomains --- src/Token/JWKSTokenDecoder.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Token/JWKSTokenDecoder.php b/src/Token/JWKSTokenDecoder.php index 0fa321e..70975e3 100644 --- a/src/Token/JWKSTokenDecoder.php +++ b/src/Token/JWKSTokenDecoder.php @@ -333,7 +333,7 @@ private function validateJwksUrl(string $url): void // Support wildcard subdomains (e.g., *.example.com) if (str_starts_with($allowedDomain, '*.')) { $domain = substr($allowedDomain, 2); - if (str_ends_with($host, '.' . $domain) || $host === $domain) { + if ($host === $domain || str_ends_with($host, '.' . $domain)) { $isAllowed = true; break; } From 1d15de9d7da9bfddacfa00ff4c6ca7f463f88a44 Mon Sep 17 00:00:00 2001 From: mainick Date: Mon, 5 Jan 2026 18:39:13 +0100 Subject: [PATCH 09/11] fix: enforce server-side algorithm validation for JWT tokens --- src/Token/JWKSTokenDecoder.php | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/Token/JWKSTokenDecoder.php b/src/Token/JWKSTokenDecoder.php index 70975e3..d140a6a 100644 --- a/src/Token/JWKSTokenDecoder.php +++ b/src/Token/JWKSTokenDecoder.php @@ -18,7 +18,7 @@ public function __construct( private ?ClientInterface $httpClient = null ) { - foreach (['base_url', 'realm', 'http_timeout', 'http_connect_timeout'] as $requiredOption) { + foreach (['base_url', 'realm', 'alg', 'http_timeout', 'http_connect_timeout'] as $requiredOption) { if (!\array_key_exists($requiredOption, $this->options) || $this->options[$requiredOption] === null || $this->options[$requiredOption] === '') { throw TokenDecoderException::forInvalidConfiguration(\sprintf( "Missing or empty required option '%s' for %s", @@ -62,14 +62,18 @@ public function decode(string $token, string $key): array $header = json_decode($this->base64urlDecode($headerB64), true, 512, JSON_THROW_ON_ERROR); $kid = $header['kid'] ?? ''; - $alg = $header['alg'] ?? ''; - if (empty($kid)) { throw TokenDecoderException::forDecodingError('Missing kid in token header', new \Exception('kid not found')); } - if (empty($alg)) { - throw TokenDecoderException::forDecodingError('Missing alg in token header', new \Exception('alg not found')); + // Enforce a server-side algorithm instead of trusting the token header. + // Default to RS256 (commonly used by Keycloak) if not explicitly configured. + $algorithm = $this->options['alg'] ?? 'RS256'; + if (isset($header['alg']) && !\hash_equals($algorithm, (string) $header['alg'])) { + throw TokenDecoderException::forDecodingError( + \sprintf('Token algorithm "%s" does not match expected algorithm "%s"', $header['alg'], $algorithm), + new \Exception('algorithm mismatch') + ); } $keyPem = $this->getPemKeyForKid($kid); From 971a011c58f9b82ddf70abe9f9d2e498f558a35f Mon Sep 17 00:00:00 2001 From: mainick Date: Mon, 5 Jan 2026 20:59:47 +0100 Subject: [PATCH 10/11] fix: validate allowed JWKS domains and improve error handling --- src/Token/JWKSTokenDecoder.php | 143 +++++----------- tests/Token/JWKSTokenDecoderTest.php | 233 ++++++++++++++++++++++++++- 2 files changed, 274 insertions(+), 102 deletions(-) diff --git a/src/Token/JWKSTokenDecoder.php b/src/Token/JWKSTokenDecoder.php index d140a6a..8cabee3 100644 --- a/src/Token/JWKSTokenDecoder.php +++ b/src/Token/JWKSTokenDecoder.php @@ -3,6 +3,7 @@ namespace Mainick\KeycloakClientBundle\Token; +use Firebase\JWT\JWK; use Firebase\JWT\JWT; use Firebase\JWT\Key; use GuzzleHttp\ClientInterface; @@ -10,15 +11,25 @@ use Mainick\KeycloakClientBundle\Exception\TokenDecoderException; use Mainick\KeycloakClientBundle\Interface\TokenDecoderInterface; -final readonly class JWKSTokenDecoder implements TokenDecoderInterface +final class JWKSTokenDecoder implements TokenDecoderInterface { public function __construct( private array $options, - private ?ClientInterface $httpClient = null + private readonly ?ClientInterface $httpClient = null ) { - foreach (['base_url', 'realm', 'alg', 'http_timeout', 'http_connect_timeout'] as $requiredOption) { + foreach ($options as $allowOption => $value) { + if (!\in_array($allowOption, ['base_url', 'realm', 'alg', 'http_timeout', 'http_connect_timeout', 'allowed_jwks_domains'], true)) { + throw TokenDecoderException::forInvalidConfiguration(\sprintf( + "Unknown option '%s' for %s", + $allowOption, + self::class + )); + } + } + + foreach (['base_url', 'realm'] as $requiredOption) { if (!\array_key_exists($requiredOption, $this->options) || $this->options[$requiredOption] === null || $this->options[$requiredOption] === '') { throw TokenDecoderException::forInvalidConfiguration(\sprintf( "Missing or empty required option '%s' for %s", @@ -76,8 +87,8 @@ public function decode(string $token, string $key): array ); } - $keyPem = $this->getPemKeyForKid($kid); - $tokenDecoded = JWT::decode($token, new Key($keyPem, $alg)); + $keyObject = $this->getKeyForKid($kid, $algorithm); + $tokenDecoded = JWT::decode($token, $keyObject); $json = json_encode($tokenDecoded, JSON_THROW_ON_ERROR); @@ -107,22 +118,36 @@ public function validateToken(string $realm, array $tokenDecoded): void } } - private function getPemKeyForKid(string $kid): string + private function getKeyForKid(string $kid, string $algorithm): Key { - $jwks = $this->fetchJwks(); - if (empty($jwks)) { + $jwksData = $this->fetchJwks(); + if (empty($jwksData['keys'])) { throw TokenDecoderException::forJwksError('No keys found in JWKS endpoint', new \Exception('Empty JWKS keys array')); } - foreach ($jwks as $jwk) { - if (($jwk['kid'] ?? '') === $kid && ($jwk['use'] ?? '') === 'sig') { - return $this->jwkToPem($jwk); - } + + // Filter to only include signing keys + $signingKeys = array_filter($jwksData['keys'], fn($jwk) => ($jwk['use'] ?? 'sig') === 'sig'); + if (empty($signingKeys)) { + throw TokenDecoderException::forJwksError('No signing keys found in JWKS endpoint', new \Exception('No sig keys in JWKS')); + } + + try { + $keys = JWK::parseKeySet(['keys' => array_values($signingKeys)], $algorithm); + } catch (\Exception $e) { + throw TokenDecoderException::forJwksError( + sprintf('Failed to parse JWKS: %s', $e->getMessage()), + $e + ); } - throw TokenDecoderException::forJwksError( - sprintf('No matching signing key found for kid: %s', $kid), - new \Exception('Key ID not found in JWKS') - ); + if (!isset($keys[$kid])) { + throw TokenDecoderException::forJwksError( + sprintf('No matching signing key found for kid: %s', $kid), + new \Exception('Key ID not found in JWKS') + ); + } + + return $keys[$kid]; } private function fetchJwks(): array @@ -150,7 +175,7 @@ private function fetchJwks(): array $data = json_decode($json, true, 512, JSON_THROW_ON_ERROR); - return $data['keys'] ?? []; + return $data; } catch (GuzzleException $e) { throw TokenDecoderException::forJwksError( sprintf('Failed to fetch JWKS from %s: %s', $url, $e->getMessage()), @@ -169,59 +194,6 @@ private function fetchJwks(): array } } - private function jwkToPem(array $jwk): string - { - if (!empty($jwk['x5c'][0])) { - $pemCert = "-----BEGIN CERTIFICATE-----\n". - chunk_split($jwk['x5c'][0], 64, "\n"). - "-----END CERTIFICATE-----\n"; - $key = openssl_pkey_get_public($pemCert); - if ($key === false) { - throw TokenDecoderException::forJwksError( - 'Failed to extract public key from certificate', - new \Exception('OpenSSL error: ' . openssl_error_string()) - ); - } - $details = openssl_pkey_get_details($key); - if ($details === false || !isset($details['key'])) { - throw TokenDecoderException::forJwksError( - 'Failed to get public key details from certificate', - new \Exception('OpenSSL error: ' . openssl_error_string()) - ); - } - - return $details['key']; // This is the PEM public key - } - - if (!isset($jwk['n'], $jwk['e'])) { - throw TokenDecoderException::forJwksError( - 'JWK missing required fields (modulus or exponent)', - new \Exception('Invalid JWK structure') - ); - } - - $modulus = $this->base64urlDecode($jwk['n']); - $exponent = $this->base64urlDecode($jwk['e']); - - $modulusEnc = $this->encodeAsn1Integer($modulus); - $exponentEnc = $this->encodeAsn1Integer($exponent); - $seq = $this->encodeAsn1Sequence($modulusEnc.$exponentEnc); - - $algo = hex2bin('300d06092a864886f70d0101010500'); // rsaEncryption OID - if ($algo === false) { - throw TokenDecoderException::forJwksError( - 'Failed to decode RSA encryption OID', - new \Exception('hex2bin returned false for hardcoded rsaEncryption OID') - ); - } - $bitStr = "\x03".chr(strlen($seq) + 1)."\x00".$seq; - $spki = $this->encodeAsn1Sequence($algo.$bitStr); - - return "-----BEGIN PUBLIC KEY-----\n" - .chunk_split(base64_encode($spki), 64, "\n") - ."-----END PUBLIC KEY-----\n"; - } - private function base64urlDecode(string $data): string { $decoded = base64_decode(strtr($data, '-_', '+/'), true); @@ -235,37 +207,6 @@ private function base64urlDecode(string $data): string return $decoded; } - private function encodeAsn1Integer(string $bytes): string - { - if ($bytes === '') { - throw TokenDecoderException::forDecodingError( - 'Empty byte string provided for ASN.1 integer encoding', - new \Exception('Bytes string must not be empty') - ); - } - - if (ord($bytes[0]) > 0x7F) { - $bytes = "\x00".$bytes; - } - - return "\x02".$this->encodeLength(strlen($bytes)).$bytes; - } - - private function encodeAsn1Sequence(string $bytes): string - { - return "\x30".$this->encodeLength(strlen($bytes)).$bytes; - } - - private function encodeLength(int $len): string - { - if ($len < 128) { - return chr($len); - } - $tmp = ltrim(pack('N', $len), "\x00"); - - return chr(0x80 | strlen($tmp)).$tmp; - } - /** * Validate the base URL format to prevent SSRF attacks. * diff --git a/tests/Token/JWKSTokenDecoderTest.php b/tests/Token/JWKSTokenDecoderTest.php index 29cb5d9..b80a2e0 100644 --- a/tests/Token/JWKSTokenDecoderTest.php +++ b/tests/Token/JWKSTokenDecoderTest.php @@ -149,7 +149,7 @@ public function testWildcardDomainMatching(): void 'e' => 'AQAB', ], ], - ])); + ], JSON_THROW_ON_ERROR)); // Create a mock response $response = $this->createMock(Response::class); @@ -217,5 +217,236 @@ public function testRequiresHttpsForJwksEndpoint(): void // Attempt to decode the token - this should trigger fetchJwks which validates the JWKS URL $decoder->decode($token, ''); } + + public function testDecodeThrowsExceptionForMissingKid(): void + { + // Create token without kid in header + $header = json_encode([ + 'alg' => 'RS256', + 'typ' => 'JWT', + ], JSON_THROW_ON_ERROR); + $payload = json_encode([ + 'sub' => 'test-user', + 'exp' => time() + 3600, + ], JSON_THROW_ON_ERROR); + + $headerEncoded = rtrim(strtr(base64_encode($header), '+/', '-_'), '='); + $payloadEncoded = rtrim(strtr(base64_encode($payload), '+/', '-_'), '='); + $token = "$headerEncoded.$payloadEncoded.fake-signature"; + + $httpClient = $this->createMock(ClientInterface::class); + + $decoder = new JWKSTokenDecoder([ + 'base_url' => 'https://keycloak.example.com', + 'realm' => 'test-realm', + ], $httpClient); + + $this->expectException(TokenDecoderException::class); + $this->expectExceptionMessage('Missing kid in token header'); + + $decoder->decode($token, ''); + } + + public function testDecodeThrowsExceptionForAlgorithmMismatch(): void + { + // Create token with HS256 algorithm + $header = json_encode([ + 'kid' => 'test-kid', + 'alg' => 'HS256', + 'typ' => 'JWT', + ], JSON_THROW_ON_ERROR); + $payload = json_encode([ + 'sub' => 'test-user', + 'exp' => time() + 3600, + ], JSON_THROW_ON_ERROR); + + $headerEncoded = rtrim(strtr(base64_encode($header), '+/', '-_'), '='); + $payloadEncoded = rtrim(strtr(base64_encode($payload), '+/', '-_'), '='); + $token = "$headerEncoded.$payloadEncoded.fake-signature"; + + $httpClient = $this->createMock(ClientInterface::class); + + // Decoder expects RS256 + $decoder = new JWKSTokenDecoder([ + 'base_url' => 'https://keycloak.example.com', + 'realm' => 'test-realm', + ], $httpClient); + + $this->expectException(TokenDecoderException::class); + $this->expectExceptionMessage('Token algorithm "HS256" does not match expected algorithm "RS256"'); + + $decoder->decode($token, ''); + } + + public function testDecodeThrowsExceptionForKidNotFoundInJwks(): void + { + // Create token with kid that doesn't exist in JWKS + $header = json_encode([ + 'kid' => 'non-existent-kid', + 'alg' => 'RS256', + 'typ' => 'JWT', + ], JSON_THROW_ON_ERROR); + $payload = json_encode([ + 'sub' => 'test-user', + 'exp' => time() + 3600, + 'iss' => 'https://keycloak.example.com/realms/test-realm', + ], JSON_THROW_ON_ERROR); + + $headerEncoded = rtrim(strtr(base64_encode($header), '+/', '-_'), '='); + $payloadEncoded = rtrim(strtr(base64_encode($payload), '+/', '-_'), '='); + $token = "$headerEncoded.$payloadEncoded.fake-signature"; + + // Mock JWKS with different kid + $jwksData = [ + 'keys' => [ + [ + 'kid' => 'different-kid', + 'kty' => 'RSA', + 'use' => 'sig', + 'n' => 'test-modulus', + 'e' => 'AQAB', + ], + ], + ]; + + $stream = $this->createMock(StreamInterface::class); + $stream->method('getContents')->willReturn(json_encode($jwksData, JSON_THROW_ON_ERROR)); + + $response = $this->createMock(Response::class); + $response->method('getBody')->willReturn($stream); + + $httpClient = $this->createMock(ClientInterface::class); + $httpClient->method('request')->willReturn($response); + + $decoder = new JWKSTokenDecoder([ + 'base_url' => 'https://keycloak.example.com', + 'realm' => 'test-realm', + ], $httpClient); + + $this->expectException(TokenDecoderException::class); + $this->expectExceptionMessage('No matching signing key found for kid: non-existent-kid'); + + $decoder->decode($token, ''); + } + + public function testValidateTokenThrowsExceptionForExpiredToken(): void + { + $httpClient = $this->createMock(ClientInterface::class); + + $decoder = new JWKSTokenDecoder([ + 'base_url' => 'https://keycloak.example.com', + 'realm' => 'test-realm', + ], $httpClient); + + $expiredToken = [ + 'exp' => time() - 3600, // Expired 1 hour ago + 'iss' => 'https://keycloak.example.com/realms/test-realm', + 'sub' => 'test-user', + ]; + + $this->expectException(TokenDecoderException::class); + $this->expectExceptionMessage('Token has expired'); + + $decoder->validateToken('test-realm', $expiredToken); + } + + public function testValidateTokenThrowsExceptionForInvalidIssuer(): void + { + $httpClient = $this->createMock(ClientInterface::class); + + $decoder = new JWKSTokenDecoder([ + 'base_url' => 'https://keycloak.example.com', + 'realm' => 'test-realm', + ], $httpClient); + + $tokenWithInvalidIssuer = [ + 'exp' => time() + 3600, + 'iss' => 'https://evil.example.com/realms/wrong-realm', + 'sub' => 'test-user', + ]; + + $this->expectException(TokenDecoderException::class); + $this->expectExceptionMessage('Issuer mismatch'); + + $decoder->validateToken('test-realm', $tokenWithInvalidIssuer); + } + + public function testValidateTokenAcceptsValidToken(): void + { + $httpClient = $this->createMock(ClientInterface::class); + + $decoder = new JWKSTokenDecoder([ + 'base_url' => 'https://keycloak.example.com', + 'realm' => 'test-realm', + ], $httpClient); + + $validToken = [ + 'exp' => time() + 3600, + 'iss' => 'https://keycloak.example.com/realms/test-realm', + 'sub' => 'test-user', + ]; + + // Should not throw any exception + $decoder->validateToken('test-realm', $validToken); + + // If we get here without exception, the test passes + $this->assertTrue(true); + } + + public function testDecodeThrowsExceptionForInvalidJwtFormat(): void + { + $httpClient = $this->createMock(ClientInterface::class); + + $decoder = new JWKSTokenDecoder([ + 'base_url' => 'https://keycloak.example.com', + 'realm' => 'test-realm', + ], $httpClient); + + $this->expectException(TokenDecoderException::class); + $this->expectExceptionMessage('Invalid JWT format: token must consist of header.payload.signature'); + + // Invalid token with only 2 parts + $decoder->decode('invalid.token', ''); + } + + public function testDecodeThrowsExceptionForEmptyJwksKeys(): void + { + // Create token + $header = json_encode([ + 'kid' => 'test-kid', + 'alg' => 'RS256', + 'typ' => 'JWT', + ], JSON_THROW_ON_ERROR); + $payload = json_encode([ + 'sub' => 'test-user', + 'exp' => time() + 3600, + ], JSON_THROW_ON_ERROR); + + $headerEncoded = rtrim(strtr(base64_encode($header), '+/', '-_'), '='); + $payloadEncoded = rtrim(strtr(base64_encode($payload), '+/', '-_'), '='); + $token = "$headerEncoded.$payloadEncoded.fake-signature"; + + // Mock JWKS with empty keys array + $jwksData = ['keys' => []]; + + $stream = $this->createMock(StreamInterface::class); + $stream->method('getContents')->willReturn(json_encode($jwksData, JSON_THROW_ON_ERROR)); + + $response = $this->createMock(Response::class); + $response->method('getBody')->willReturn($stream); + + $httpClient = $this->createMock(ClientInterface::class); + $httpClient->method('request')->willReturn($response); + + $decoder = new JWKSTokenDecoder([ + 'base_url' => 'https://keycloak.example.com', + 'realm' => 'test-realm', + ], $httpClient); + + $this->expectException(TokenDecoderException::class); + $this->expectExceptionMessage('No keys found in JWKS endpoint'); + + $decoder->decode($token, ''); + } } From 8577b8385f9b2b590751fc5cd65d71f49fcd1245 Mon Sep 17 00:00:00 2001 From: mainick Date: Mon, 5 Jan 2026 22:28:41 +0100 Subject: [PATCH 11/11] fix: update JWKSTokenDecoder constructor to enforce parameter order and improve validation logic --- src/Provider/KeycloakClient.php | 6 +- src/Token/JWKSTokenDecoder.php | 10 +- src/Token/TokenDecoderFactory.php | 4 +- tests/Token/JWKSTokenDecoderTest.php | 161 ++++++++++++++++----------- 4 files changed, 108 insertions(+), 73 deletions(-) diff --git a/src/Provider/KeycloakClient.php b/src/Provider/KeycloakClient.php index aae41ed..55ebf31 100644 --- a/src/Provider/KeycloakClient.php +++ b/src/Provider/KeycloakClient.php @@ -103,7 +103,11 @@ public function verifyToken(AccessTokenInterface $token): ?UserRepresentationDTO 'values' => $token->getValues(), ]); - $decoder = TokenDecoderFactory::create($this->encryption_algorithm, ['base_url' => $this->base_url, 'realm' => $this->realm], $this->httpClient); + $decoder = TokenDecoderFactory::create( + $this->encryption_algorithm, + $this->httpClient, + ['base_url' => $this->base_url, 'realm' => $this->realm] + ); $tokenDecoded = $decoder->decode($accessToken->getToken(), $this->encryption_key); $decoder->validateToken($this->realm, $tokenDecoded); $this->keycloakClientLogger->info('KeycloakClient::verifyToken', [ diff --git a/src/Token/JWKSTokenDecoder.php b/src/Token/JWKSTokenDecoder.php index 8cabee3..aa38ccb 100644 --- a/src/Token/JWKSTokenDecoder.php +++ b/src/Token/JWKSTokenDecoder.php @@ -11,12 +11,12 @@ use Mainick\KeycloakClientBundle\Exception\TokenDecoderException; use Mainick\KeycloakClientBundle\Interface\TokenDecoderInterface; -final class JWKSTokenDecoder implements TokenDecoderInterface +final readonly class JWKSTokenDecoder implements TokenDecoderInterface { public function __construct( - private array $options, - private readonly ?ClientInterface $httpClient = null + private ClientInterface $httpClient, + private array $options ) { foreach ($options as $allowOption => $value) { @@ -80,9 +80,9 @@ public function decode(string $token, string $key): array // Enforce a server-side algorithm instead of trusting the token header. // Default to RS256 (commonly used by Keycloak) if not explicitly configured. $algorithm = $this->options['alg'] ?? 'RS256'; - if (isset($header['alg']) && !\hash_equals($algorithm, (string) $header['alg'])) { + if (isset($header['alg']) && $algorithm !== (string) $header['alg']) { throw TokenDecoderException::forDecodingError( - \sprintf('Token algorithm "%s" does not match expected algorithm "%s"', $header['alg'], $algorithm), + sprintf('Token algorithm "%s" does not match expected algorithm "%s"', $header['alg'], $algorithm), new \Exception('algorithm mismatch') ); } diff --git a/src/Token/TokenDecoderFactory.php b/src/Token/TokenDecoderFactory.php index fe3ad56..98d6f48 100644 --- a/src/Token/TokenDecoderFactory.php +++ b/src/Token/TokenDecoderFactory.php @@ -13,12 +13,12 @@ class TokenDecoderFactory public const ALGORITHM_HS256 = 'HS256'; public const ALGORITHM_JWKS = 'JWKS'; - public static function create(string $algorithm, array $options = [], ?ClientInterface $httpClient = null): TokenDecoderInterface + public static function create(string $algorithm, ClientInterface $httpClient, array $options = []): TokenDecoderInterface { return match ($algorithm) { self::ALGORITHM_RS256 => new RS256TokenDecoder(), self::ALGORITHM_HS256 => new HS256TokenDecoder(), - self::ALGORITHM_JWKS => new JWKSTokenDecoder($options, $httpClient), + self::ALGORITHM_JWKS => new JWKSTokenDecoder($httpClient, $options), default => throw new \RuntimeException('Invalid algorithm'), }; } diff --git a/tests/Token/JWKSTokenDecoderTest.php b/tests/Token/JWKSTokenDecoderTest.php index b80a2e0..ea41a49 100644 --- a/tests/Token/JWKSTokenDecoderTest.php +++ b/tests/Token/JWKSTokenDecoderTest.php @@ -14,78 +14,101 @@ class JWKSTokenDecoderTest extends TestCase { public function testConstructorValidatesBaseUrl(): void { + $httpClient = $this->createMock(ClientInterface::class); + $this->expectException(TokenDecoderException::class); $this->expectExceptionMessage('Invalid base_url format'); - new JWKSTokenDecoder([ - 'base_url' => 'not-a-valid-url', - 'realm' => 'test-realm', - ]); + new JWKSTokenDecoder( + $httpClient, + [ + 'base_url' => 'not-a-valid-url', + 'realm' => 'test-realm', + ]); } public function testConstructorRejectsHttpForNonLocalhost(): void { + $httpClient = $this->createMock(ClientInterface::class); + $this->expectException(TokenDecoderException::class); $this->expectExceptionMessage('HTTP is only allowed for localhost'); - new JWKSTokenDecoder([ - 'base_url' => 'http://keycloak.example.com', - 'realm' => 'test-realm', - ]); + new JWKSTokenDecoder( + $httpClient, + [ + 'base_url' => 'http://keycloak.example.com', + 'realm' => 'test-realm', + ]); } public function testConstructorAcceptsHttpForLocalhost(): void { - $decoder = new JWKSTokenDecoder([ - 'base_url' => 'http://localhost:8080', - 'realm' => 'test-realm', - ]); + $httpClient = $this->createMock(ClientInterface::class); + + $decoder = new JWKSTokenDecoder( + $httpClient, + [ + 'base_url' => 'http://localhost:8080', + 'realm' => 'test-realm', + ]); $this->assertInstanceOf(JWKSTokenDecoder::class, $decoder); } public function testConstructorRejectsPrivateIpRanges(): void { + $httpClient = $this->createMock(ClientInterface::class); + $this->expectException(TokenDecoderException::class); $this->expectExceptionMessage('is not allowed'); - new JWKSTokenDecoder([ - 'base_url' => 'https://192.168.1.1', - 'realm' => 'test-realm', - ]); + new JWKSTokenDecoder( + $httpClient, + [ + 'base_url' => 'https://192.168.1.1', + 'realm' => 'test-realm', + ]); } public function testConstructorRejectsMetadataEndpoints(): void { + $httpClient = $this->createMock(ClientInterface::class); + $this->expectException(TokenDecoderException::class); $this->expectExceptionMessage('is not allowed'); - new JWKSTokenDecoder([ - 'base_url' => 'https://169.254.169.254', - 'realm' => 'test-realm', - ]); + new JWKSTokenDecoder( + $httpClient, + [ + 'base_url' => 'https://169.254.169.254', + 'realm' => 'test-realm', + ]); } public function testConstructorAcceptsValidHttpsUrl(): void { - $decoder = new JWKSTokenDecoder([ - 'base_url' => 'https://keycloak.example.com', - 'realm' => 'test-realm', - ]); + $httpClient = $this->createMock(ClientInterface::class); + + $decoder = new JWKSTokenDecoder( + $httpClient, [ + 'base_url' => 'https://keycloak.example.com', + 'realm' => 'test-realm', + ]); $this->assertInstanceOf(JWKSTokenDecoder::class, $decoder); } public function testFetchJwksValidatesDomainWhitelist(): void { - // Create a mock HTTP client $httpClient = $this->createMock(ClientInterface::class); - $decoder = new JWKSTokenDecoder([ - 'base_url' => 'https://keycloak.example.com', - 'realm' => 'test-realm', - 'allowed_jwks_domains' => ['different-domain.com'], - ], $httpClient); + $decoder = new JWKSTokenDecoder( + $httpClient, [ + 'base_url' => 'https://keycloak.example.com', + 'realm' => 'test-realm', + 'allowed_jwks_domains' => ['different-domain.com'], + ]); // Create a sample JWT token (doesn't need to be valid for this test) $header = base64_encode(json_encode(['kid' => 'test-kid', 'alg' => 'RS256'])); @@ -125,11 +148,13 @@ public function testFetchJwksAllowsBaseUrlDomainByDefault(): void $httpClient = $this->createMock(ClientInterface::class); $httpClient->method('request')->willReturn($response); - $decoder = new JWKSTokenDecoder([ - 'base_url' => 'https://keycloak.example.com', - 'realm' => 'test-realm', - // No allowed_jwks_domains specified - should default to base_url domain - ], $httpClient); + $decoder = new JWKSTokenDecoder( + $httpClient, + [ + 'base_url' => 'https://keycloak.example.com', + 'realm' => 'test-realm', + // No allowed_jwks_domains specified - should default to base_url domain + ]); // This should not throw an exception because the JWKS URL uses the same domain as base_url $this->assertInstanceOf(JWKSTokenDecoder::class, $decoder); @@ -159,30 +184,36 @@ public function testWildcardDomainMatching(): void $httpClient = $this->createMock(ClientInterface::class); $httpClient->method('request')->willReturn($response); - $decoder = new JWKSTokenDecoder([ - 'base_url' => 'https://auth.example.com', - 'realm' => 'test-realm', - 'allowed_jwks_domains' => ['*.example.com'], - ], $httpClient); + $decoder = new JWKSTokenDecoder( + $httpClient, + [ + 'base_url' => 'https://auth.example.com', + 'realm' => 'test-realm', + 'allowed_jwks_domains' => ['*.example.com'], + ]); // This should not throw an exception because auth.example.com matches *.example.com $this->assertInstanceOf(JWKSTokenDecoder::class, $decoder); } - public function testRequiresHttpsForJwksEndpoint(): void + public function testJwksUrlValidationRejectsHttpForNonLocalhost(): void { // This test verifies that the JWKS endpoint URL validation rejects HTTP - // for non-localhost domains during token decoding. We create a decoder - // with valid HTTPS, then simulate an HTTP JWKS URL fetch scenario. + // for non-localhost domains by configuring an HTTP base URL directly. // Create a mock HTTP client $httpClient = $this->createMock(ClientInterface::class); - // Create a valid decoder first with localhost HTTP (which is allowed) - $decoder = new JWKSTokenDecoder([ - 'base_url' => 'http://localhost:8080', - 'realm' => 'test-realm', - ], $httpClient); + $this->expectException(TokenDecoderException::class); + $this->expectExceptionMessage('HTTP is only allowed for localhost.'); + + // Constructing the decoder with an HTTP non-localhost base URL should fail validation + $decoder = new JWKSTokenDecoder( + $httpClient, + [ + 'base_url' => 'http://keycloak.example.com', + 'realm' => 'test-realm', + ]); // Use reflection to modify the base_url to an HTTP non-localhost domain // This simulates a scenario where the JWKS URL would be HTTP for a non-localhost host @@ -212,7 +243,7 @@ public function testRequiresHttpsForJwksEndpoint(): void $token = "$headerEncoded.$payloadEncoded.fake-signature"; $this->expectException(TokenDecoderException::class); - $this->expectExceptionMessage('JWKS endpoint must use HTTPS for non-localhost hosts'); + $this->expectExceptionMessage('HTTP is only allowed for localhost.'); // Attempt to decode the token - this should trigger fetchJwks which validates the JWKS URL $decoder->decode($token, ''); @@ -236,10 +267,10 @@ public function testDecodeThrowsExceptionForMissingKid(): void $httpClient = $this->createMock(ClientInterface::class); - $decoder = new JWKSTokenDecoder([ + $decoder = new JWKSTokenDecoder($httpClient, [ 'base_url' => 'https://keycloak.example.com', 'realm' => 'test-realm', - ], $httpClient); + ]); $this->expectException(TokenDecoderException::class); $this->expectExceptionMessage('Missing kid in token header'); @@ -267,10 +298,10 @@ public function testDecodeThrowsExceptionForAlgorithmMismatch(): void $httpClient = $this->createMock(ClientInterface::class); // Decoder expects RS256 - $decoder = new JWKSTokenDecoder([ + $decoder = new JWKSTokenDecoder($httpClient, [ 'base_url' => 'https://keycloak.example.com', 'realm' => 'test-realm', - ], $httpClient); + ]); $this->expectException(TokenDecoderException::class); $this->expectExceptionMessage('Token algorithm "HS256" does not match expected algorithm "RS256"'); @@ -318,10 +349,10 @@ public function testDecodeThrowsExceptionForKidNotFoundInJwks(): void $httpClient = $this->createMock(ClientInterface::class); $httpClient->method('request')->willReturn($response); - $decoder = new JWKSTokenDecoder([ + $decoder = new JWKSTokenDecoder($httpClient, [ 'base_url' => 'https://keycloak.example.com', 'realm' => 'test-realm', - ], $httpClient); + ]); $this->expectException(TokenDecoderException::class); $this->expectExceptionMessage('No matching signing key found for kid: non-existent-kid'); @@ -333,10 +364,10 @@ public function testValidateTokenThrowsExceptionForExpiredToken(): void { $httpClient = $this->createMock(ClientInterface::class); - $decoder = new JWKSTokenDecoder([ + $decoder = new JWKSTokenDecoder($httpClient, [ 'base_url' => 'https://keycloak.example.com', 'realm' => 'test-realm', - ], $httpClient); + ]); $expiredToken = [ 'exp' => time() - 3600, // Expired 1 hour ago @@ -354,10 +385,10 @@ public function testValidateTokenThrowsExceptionForInvalidIssuer(): void { $httpClient = $this->createMock(ClientInterface::class); - $decoder = new JWKSTokenDecoder([ + $decoder = new JWKSTokenDecoder($httpClient, [ 'base_url' => 'https://keycloak.example.com', 'realm' => 'test-realm', - ], $httpClient); + ]); $tokenWithInvalidIssuer = [ 'exp' => time() + 3600, @@ -375,10 +406,10 @@ public function testValidateTokenAcceptsValidToken(): void { $httpClient = $this->createMock(ClientInterface::class); - $decoder = new JWKSTokenDecoder([ + $decoder = new JWKSTokenDecoder($httpClient, [ 'base_url' => 'https://keycloak.example.com', 'realm' => 'test-realm', - ], $httpClient); + ]); $validToken = [ 'exp' => time() + 3600, @@ -397,10 +428,10 @@ public function testDecodeThrowsExceptionForInvalidJwtFormat(): void { $httpClient = $this->createMock(ClientInterface::class); - $decoder = new JWKSTokenDecoder([ + $decoder = new JWKSTokenDecoder($httpClient, [ 'base_url' => 'https://keycloak.example.com', 'realm' => 'test-realm', - ], $httpClient); + ]); $this->expectException(TokenDecoderException::class); $this->expectExceptionMessage('Invalid JWT format: token must consist of header.payload.signature'); @@ -438,10 +469,10 @@ public function testDecodeThrowsExceptionForEmptyJwksKeys(): void $httpClient = $this->createMock(ClientInterface::class); $httpClient->method('request')->willReturn($response); - $decoder = new JWKSTokenDecoder([ + $decoder = new JWKSTokenDecoder($httpClient, [ 'base_url' => 'https://keycloak.example.com', 'realm' => 'test-realm', - ], $httpClient); + ]); $this->expectException(TokenDecoderException::class); $this->expectExceptionMessage('No keys found in JWKS endpoint');