bug #62495 [Security][Http] Fix OIDC discovery when multiple HttpClient instances are used (Ali-HENDA)

This PR was merged into the 7.4 branch.

Discussion
----------

[Security][Http] Fix OIDC discovery when multiple HttpClient instances are used

| Q             | A
| ------------- | ---
| Branch?       | 7.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Issues        | Fix
| License       | MIT

This PR fixes an issue in the OIDC JWKS discovery logic revealed in the discussion of #62369.
$client->stream() was used incorrectly:
$client could be undefined, and responses must be streamed using the same client instance that created them, which breaks when multiple HttpClientInterface instances are configured.
The logic now performs sequential discovery per client, avoiding cross-client streaming and ensuring correctness.

This PR also hardens the "use" check ($key['use'] ?? null).

Commits
-------

7a885d29993 [Security] Fix OIDC discovery when using multiple HttpClient instances

Author: nicolas-grekas

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));
+    }
 }