[Security] Fix OIDC discovery when using multiple HttpClient instances

Author: Ali-HENDA

AccessToken/Oidc/OidcTokenHandler.php

@@ -149,24 +149,31 @@ public function getUserBadgeFrom(string $accessToken): UserBadge
     public function computeDiscoveryKeys(ItemInterface $item): array
     {
         $clients = $this->discoveryClients;
+        if (!$clients) {
+            throw new \LogicException('No OIDC discovery client configured.');
+        }
         $logger = $this->logger;
-
         try {
+            $keys = [];
+            $minTtl = null;
             $configResponses = [];
+            $jwkSetResponses = [];
+
             foreach ($clients as $client) {
-                $configResponses[] = $client->request('GET', '.well-known/openid-configuration', [
-                    'user_data' => $client,
-                ]);
+                $configResponses[] = [$client, $client->request('GET', '.well-known/openid-configuration')];
             }
 
-            $jwkSetResponses = [];
-            foreach ($client->stream($configResponses) as $response => $chunk) {
-                if ($chunk->isLast()) {
-                    $jwkSetResponses[] = $response->getInfo('user_data')->request('GET', $response->toArray()['jwks_uri']);
+            foreach ($configResponses as [$client, $response]) {
+                $config = $response->toArray();
+
+                $jwksUri = $config['jwks_uri'] ?? null;
+                if (!\is_string($jwksUri) || '' === $jwksUri) {
+                    throw new \RuntimeException('The "jwks_uri" is missing from the OIDC discovery document.');
                 }
+
+                $jwkSetResponses[] = $client->request('GET', $jwksUri);
             }
-            $keys = [];
-            $minTtl = null;
+
             foreach ($jwkSetResponses as $response) {
                 $headers = $response->getHeaders();
                 if (preg_match('/max-age=(\d+)/', $headers['cache-control'][0] ?? '', $m)) {
@@ -181,7 +188,7 @@ public function computeDiscoveryKeys(ItemInterface $item): array
                 }
 
                 foreach ($response->toArray()['keys'] as $key) {
-                    if ('sig' === $key['use']) {
+                    if ('sig' === ($key['use'] ?? null)) {
                         $keys[] = $key;
                     }
                 }
@@ -198,6 +205,7 @@ public function computeDiscoveryKeys(ItemInterface $item): array
                 'error' => $e->getMessage(),
                 'trace' => $e->getTraceAsString(),
             ]);
+
             throw new BadCredentialsException('Invalid credentials.', $e->getCode(), $e);
         }
     }

Tests/AccessToken/Oidc/OidcTokenHandlerTest.php

@@ -28,6 +28,7 @@
 use Symfony\Component\Security\Core\User\OidcUser;
 use Symfony\Component\Security\Http\AccessToken\Oidc\OidcTokenHandler;
 use Symfony\Component\Security\Http\Authenticator\Passport\Badge\UserBadge;
+use Symfony\Contracts\Cache\ItemInterface;
 
 #[RequiresPhpExtension('openssl')]
 class OidcTokenHandlerTest extends TestCase
@@ -316,7 +317,8 @@ public function testDiscoveryCachesJwksAccordingToCacheControl()
     {
         $time = time();
         $claims = [
-            'iat' => $time, 'nbf' => $time,
+            'iat' => $time,
+            'nbf' => $time,
             'exp' => $time + 3600,
             'iss' => 'https://www.example.com',
             'aud' => self::AUDIENCE,
@@ -355,7 +357,8 @@ public function testDiscoveryCachesJwksAccordingToExpires()
     {
         $time = time();
         $claims = [
-            'iat' => $time, 'nbf' => $time,
+            'iat' => $time,
+            'nbf' => $time,
             'exp' => $time + 3600,
             'iss' => 'https://www.example.com',
             'aud' => self::AUDIENCE,
@@ -390,4 +393,80 @@ public function testDiscoveryCachesJwksAccordingToExpires()
         $this->assertSame('user-expires', $handler->getUserBadgeFrom($token)->getUserIdentifier());
         $this->assertSame(2, $requestCount);
     }
+
+    public function testComputeDiscoveryKeysReturnsEmptyWhenNoClients()
+    {
+        $cache = new ArrayAdapter();
+        $handler = new OidcTokenHandler(
+            new AlgorithmManager([new ES256()]),
+            null,
+            self::AUDIENCE,
+            ['https://www.example.com']
+        );
+
+        $handler->enableDiscovery($cache, [], 'oidc_empty_clients');
+
+        $item = $this->createMock(ItemInterface::class);
+        $item->expects($this->never())->method('expiresAfter');
+
+        $this->expectException(\LogicException::class);
+        $this->expectExceptionMessage('No OIDC discovery client configured.');
+        $handler->computeDiscoveryKeys($item);
+    }
+
+    public function testDiscoveryThrowsWhenJwksUriIsMissing()
+    {
+        $time = time();
+        $claims = [
+            'iat' => $time,
+            'nbf' => $time,
+            'exp' => $time + 3600,
+            'iss' => 'https://www.example.com',
+            'aud' => self::AUDIENCE,
+            'sub' => 'user-missing-jwks-uri',
+        ];
+        $token = self::buildJWS(json_encode($claims));
+
+        $httpClient = new MockHttpClient([
+            new JsonMockResponse(['issuer' => 'https://www.example.com']),
+        ]);
+
+        $cache = new ArrayAdapter();
+        $handler = new OidcTokenHandler(
+            new AlgorithmManager([new ES256()]),
+            null,
+            self::AUDIENCE,
+            ['https://www.example.com']
+        );
+        $handler->enableDiscovery($cache, $httpClient, 'oidc_missing_jwks_uri');
+
+        $this->expectException(BadCredentialsException::class);
+        $handler->getUserBadgeFrom($token);
+    }
+
+    public function testDiscoveryIgnoresNonSignatureKeys()
+    {
+        $httpClient = new MockHttpClient([
+            new JsonMockResponse(['jwks_uri' => 'https://www.example.com/jwks.json']),
+            new JsonMockResponse([
+                'keys' => [
+                    array_merge(self::getJWK()->all(), ['use' => 'enc']),
+                    array_merge(self::getSecondJWK()->all(), []),
+                ],
+            ]),
+        ]);
+
+        $cache = new ArrayAdapter();
+        $handler = new OidcTokenHandler(
+            new AlgorithmManager([new ES256()]),
+            null,
+            self::AUDIENCE,
+            ['https://www.example.com']
+        );
+        $handler->enableDiscovery($cache, $httpClient, 'oidc_non_sig_keys');
+
+        $item = $this->createMock(ItemInterface::class);
+        $item->expects($this->never())->method('expiresAfter');
+        $this->assertSame([], $handler->computeDiscoveryKeys($item));
+    }
 }