Replace (string) casts on JSON payload values with is_string guards

CHANGELOG.md

@@ -40,13 +40,32 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   re-wrapped as `CodeException`. Both implement the marker, so a
   consumer catching that is unaffected; a consumer catching only
   `CodeException` will need to widen to the marker for this code path.
+- `OpenIdConfigurationProvider` now throws `KeyException` when a JWK
+  entry is missing a string `kid` (RFC 7517 §4.5), and the new
+  `MetadataException` when an OIDC discovery document is missing a
+  required key or has a non-string value at one. Previously the
+  non-string value was silently coerced via `(string)` cast, and both
+  validation failures bubbled as `CacheException` — semantically
+  misleading since the failure is the IdP-returned payload not
+  conforming to the OIDC Discovery schema, not the cache layer
+  misbehaving. All three throw types implement the marker interface,
+  so consumers catching that are unaffected; consumers catching
+  `CacheException` specifically for the missing-key case will need to
+  widen to the marker or to `MetadataException`.
 
 ### Added
 
 - `ItkDev\OpenIdConnect\Exception\OpenIdConnectExceptionInterface`
   marker for catching every OIDC failure from this library.
 - `ItkDev\OpenIdConnect\Exception\ConfigurationException` for missing
   or invalid constructor options.
+- `ItkDev\OpenIdConnect\Exception\MetadataException` (extends
+  `\RuntimeException`, implements the marker) for IdP-returned OIDC
+  discovery documents that parse as JSON but don't conform to the
+  OIDC Discovery spec (missing required key, wrong type at a required
+  key). Distinct from `JsonException` (parse failure) and
+  `CacheException` (PSR-6 cache layer failure) — different remediation
+  paths (retry doesn't help; the IdP needs to fix its payload).
 - `tests/Exception/ExceptionHierarchyTest.php` locks the contract:
   every concrete implements the marker, extends the correct SPL parent,
   and is caught by a `catch (OpenIdConnectExceptionInterface $e)`

README.md

@@ -218,7 +218,7 @@ category, so a `catch` block scoped to that SPL type will also match:
 
 | SPL parent | Concrete types | Category |
 | ---------- | -------------- | -------- |
-| `\RuntimeException` | `CacheException`, `HttpException`, `JsonException`, `DecodeException`, `KeyException`, `CodeException`, `ValidationException`, `ClaimsException` | Network, cache, token validation, claims mismatch — transient or data-shape failures |
+| `\RuntimeException` | `CacheException`, `HttpException`, `JsonException`, `DecodeException`, `KeyException`, `CodeException`, `ValidationException`, `ClaimsException`, `MetadataException` | Network, cache, token validation, claims mismatch — transient or data-shape failures |
 | `\LogicException` | `BadUrlException`, `IllegalSchemeException`, `MissingParameterException` | Programmer/config bugs — should be fixed in code |
 | `\InvalidArgumentException` | `ConfigurationException`, `NegativeCacheDurationException`, `NegativeLeewayException` | Invalid input to the constructor / setters |
 

src/Exception/MetadataException.php

@@ -0,0 +1,17 @@
+<?php
+
+namespace ItkDev\OpenIdConnect\Exception;
+
+/**
+ * Thrown when the IdP-returned OIDC discovery metadata does not conform to the
+ * OIDC Discovery spec — for example, a required key is missing or has the
+ * wrong type. Distinct from `JsonException` (the document didn't even parse)
+ * and `CacheException` (the cache layer failed): the JSON parsed fine, the
+ * cache is fine, but the document's contents are structurally invalid.
+ *
+ * Consumers typically can't recover by retrying — the IdP needs to fix the
+ * payload — so a `catch (MetadataException $e)` block normally logs + alerts.
+ */
+class MetadataException extends \RuntimeException implements OpenIdConnectExceptionInterface
+{
+}

src/Security/OpenIdConfigurationProvider.php

@@ -16,6 +16,7 @@
 use ItkDev\OpenIdConnect\Exception\IllegalSchemeException;
 use ItkDev\OpenIdConnect\Exception\JsonException;
 use ItkDev\OpenIdConnect\Exception\KeyException;
+use ItkDev\OpenIdConnect\Exception\MetadataException;
 use ItkDev\OpenIdConnect\Exception\MissingParameterException;
 use ItkDev\OpenIdConnect\Exception\NegativeCacheDurationException;
 use ItkDev\OpenIdConnect\Exception\NegativeLeewayException;
@@ -108,6 +109,7 @@ public function getGuarded(): array
      * @throws CacheException
      * @throws HttpException
      * @throws JsonException
+     * @throws MetadataException
      */
     public function getBaseAuthorizationUrl(): string
     {
@@ -158,6 +160,7 @@ public function getAuthorizationUrl(array $options = []): string
      * @throws CacheException
      * @throws HttpException
      * @throws JsonException
+     * @throws MetadataException
      */
     public function getEndSessionUrl(?string $postLogoutRedirectUri = null, ?string $state = null, ?string $idToken = null): string
     {
@@ -209,6 +212,7 @@ public function getEndSessionUrl(?string $postLogoutRedirectUri = null, ?string
      * @throws HttpException
      * @throws JsonException
      * @throws KeyException
+     * @throws MetadataException
      * @throws ValidationException
      */
     public function validateIdToken(string $idToken, string $nonce): object
@@ -370,7 +374,10 @@ private function getJwtVerificationKeys(): array
                 $jwks = $this->fetchJsonResource($keysUri);
 
                 foreach ($jwks['keys'] as $key) {
-                    $kid = (string) $key['kid'];
+                    if (!is_string($key['kid'] ?? null)) {
+                        throw new KeyException('JWK entry missing string "kid" (RFC 7517 §4.5)');
+                    }
+                    $kid = $key['kid'];
                     if ('RSA' === $key['kty']) {
                         $e = self::base64urlDecode($key['e']);
                         $n = self::base64urlDecode($key['n']);
@@ -451,6 +458,7 @@ private function fetchJsonResource(string $resourceUrl): array
      * @throws CacheException
      * @throws HttpException
      * @throws JsonException
+     * @throws MetadataException
      */
     private function getConfiguration(string $key): string
     {
@@ -468,13 +476,14 @@ private function getConfiguration(string $key): string
                 $this->cacheItemPool->save($item);
             }
 
-            if (isset($configuration[$key])) {
-                $value = (string) $configuration[$key];
-            } else {
-                throw new CacheException('Required config key not defined: '.$key);
+            if (!isset($configuration[$key])) {
+                throw new MetadataException('OIDC discovery document missing required key: '.$key);
+            }
+            if (!is_string($configuration[$key])) {
+                throw new MetadataException(sprintf('OIDC discovery document value for "%s" is not a string (got %s)', $key, get_debug_type($configuration[$key])));
             }
 
-            return $value;
+            return $configuration[$key];
         } catch (InvalidArgumentException $e) {
             throw new CacheException($e->getMessage(), 0, $e);
         }

tests/Exception/ExceptionHierarchyTest.php

@@ -12,6 +12,7 @@
 use ItkDev\OpenIdConnect\Exception\IllegalSchemeException;
 use ItkDev\OpenIdConnect\Exception\JsonException;
 use ItkDev\OpenIdConnect\Exception\KeyException;
+use ItkDev\OpenIdConnect\Exception\MetadataException;
 use ItkDev\OpenIdConnect\Exception\MissingParameterException;
 use ItkDev\OpenIdConnect\Exception\NegativeCacheDurationException;
 use ItkDev\OpenIdConnect\Exception\NegativeLeewayException;
@@ -62,6 +63,7 @@ public static function concreteProvider(): iterable
         yield 'CodeException' => [CodeException::class, \RuntimeException::class];
         yield 'ValidationException' => [ValidationException::class, \RuntimeException::class];
         yield 'ClaimsException' => [ClaimsException::class, \RuntimeException::class];
+        yield 'MetadataException' => [MetadataException::class, \RuntimeException::class];
     }
 
     /**

tests/Security/OpenIdConfigurationProviderTest.php

@@ -574,13 +574,39 @@ public function testGetConfigurationCacheHit(): void
 
     public function testGetConfigurationMissingKey(): void
     {
-        $this->expectException(CacheException::class);
-        $this->expectExceptionMessage('Required config key not defined: nonexistent_key');
+        $this->expectException(\ItkDev\OpenIdConnect\Exception\MetadataException::class);
+        $this->expectExceptionMessage('OIDC discovery document missing required key: nonexistent_key');
 
         $method = new \ReflectionMethod(OpenIdConfigurationProvider::class, 'getConfiguration');
         $method->invoke($this->provider, 'nonexistent_key');
     }
 
+    public function testGetConfigurationNonStringValue(): void
+    {
+        $mockCacheItem = $this->createStub(CacheItemInterface::class);
+        $mockCacheItem->method('isHit')->willReturn(true);
+        $mockCacheItem->method('get')->willReturn(['authorization_endpoint' => 42]);
+
+        $mockCacheItemPool = $this->createStub(CacheItemPoolInterface::class);
+        $mockCacheItemPool->method('getItem')->willReturn($mockCacheItem);
+
+        $provider = new OpenIdConfigurationProvider([
+            'openIDConnectMetadataUrl' => 'https://some.url/openid-configuration',
+            'cacheItemPool' => $mockCacheItemPool,
+            'clientId' => self::CLIENT_ID,
+            'clientSecret' => self::CLIENT_SECRET,
+            'redirectUri' => self::REDIRECT_URI,
+        ], [
+            'httpClient' => $this->createStub(ClientInterface::class),
+        ]);
+
+        $this->expectException(\ItkDev\OpenIdConnect\Exception\MetadataException::class);
+        $this->expectExceptionMessage('OIDC discovery document value for "authorization_endpoint" is not a string (got int)');
+
+        $method = new \ReflectionMethod(OpenIdConfigurationProvider::class, 'getConfiguration');
+        $method->invoke($provider, 'authorization_endpoint');
+    }
+
     public function testFetchJsonResourceNon200(): void
     {
         $openIDConnectMetadataUrl = 'https://some.url/openid-configuration';
@@ -683,6 +709,52 @@ public function testFetchJsonResourceInvalidJson(): void
         $provider->getBaseAuthorizationUrl();
     }
 
+    public function testGetJwtVerificationKeysRejectsNonStringKid(): void
+    {
+        $openIDConnectMetadataUrl = 'https://some.url/openid-configuration';
+        $jwks_uri = 'https://azure_b2c_test.b2clogin.com/azure_b2c_test.onmicrosoft.com/discovery/v2.0/keys?p=test-policy';
+
+        $mockConfigResponse = $this->getMockHttpSuccessResponse('/../MockData/mockOpenIDConfiguration.json');
+
+        // JWK with an int `kid` — violates RFC 7517 §4.5 (kid must be a string).
+        $badJwks = json_encode(['keys' => [['kid' => 42, 'kty' => 'RSA', 'e' => 'AQAB', 'n' => 'abc']]]);
+        $mockKeysStream = $this->createStub(StreamInterface::class);
+        $mockKeysStream->method('getContents')->willReturn($badJwks);
+
+        $mockKeysResponse = $this->createStub(ResponseInterface::class);
+        $mockKeysResponse->method('getStatusCode')->willReturn(200);
+        $mockKeysResponse->method('getBody')->willReturn($mockKeysStream);
+
+        $mockHttpClient = $this->createStub(ClientInterface::class);
+        $mockHttpClient->method('request')->willReturnMap([
+            ['GET', $openIDConnectMetadataUrl, [], $mockConfigResponse],
+            ['GET', $jwks_uri, [], $mockKeysResponse],
+        ]);
+
+        $mockCacheItem = $this->createStub(CacheItemInterface::class);
+        $mockCacheItem->method('isHit')->willReturn(false);
+
+        $mockCacheItemPool = $this->createStub(CacheItemPoolInterface::class);
+        $mockCacheItemPool->method('getItem')->willReturn($mockCacheItem);
+
+        $provider = new OpenIdConfigurationProvider([
+            'openIDConnectMetadataUrl' => $openIDConnectMetadataUrl,
+            'cacheItemPool' => $mockCacheItemPool,
+            'clientId' => self::CLIENT_ID,
+            'clientSecret' => self::CLIENT_SECRET,
+            'redirectUri' => self::REDIRECT_URI,
+        ], [
+            'httpClient' => $mockHttpClient,
+        ]);
+
+        $mockJWT = \Mockery::mock('overload:Firebase\JWT\JWT', MockJWT::class);
+
+        $this->expectException(KeyException::class);
+        $this->expectExceptionMessage('JWK entry missing string "kid" (RFC 7517 §4.5)');
+
+        $provider->validateIdToken('token', self::NONCE);
+    }
+
     public function testGetJwtVerificationKeysUnsupportedKeyType(): void
     {
         $openIDConnectMetadataUrl = 'https://some.url/openid-configuration';