Skip to content

Commit 058f45d

Browse files
nicolas-grekasnicolas-grekas
committed
feature #62043 [Security] Allow multiple OIDC discovery endpoints (ruudk)
This PR was squashed before being merged into the 7.4 branch. Discussion ---------- [Security] Allow multiple OIDC discovery endpoints | Q | A | ------------- | --- | Branch? | 7.4 | Bug fix? | no | New feature? | yes | Deprecations? | no | Issues | | License | MIT When a firewall accepts tokens from multiple identity providers, it needs to validate tokens against different OIDC discovery endpoints. This change allows configuring multiple named discovery servers instead of just one, while keeping backward compatibility. With this change, we allow configuring multiple discovery endpoints with different (or the same) cache storage. They all use a different cache key. It then builds a JWTSet of all the keys fetched, from the multiple endpoints, and then validates the JWT against that total JWTSet. /cc `@vincentchalamon` `@Jean`-Beru `@chalasr` `@Spomky` Tagging you as you were all involved in previous OIDC PR's. Curious to hear your opinions about this 🙏 Commits ------- bc7424f [Security] Allow multiple OIDC discovery endpoints
2 parents c354c61 + bc7424f commit 058f45d

File tree

7 files changed

+287
-60
lines changed

7 files changed

+287
-60
lines changed

‎src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/AccessToken/OidcTokenHandlerFactory.php‎

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -48,16 +48,18 @@ public function create(ContainerBuilder $container, string $id, array|string $co
4848

4949
// disable JWKSet argument
5050
$tokenHandlerDefinition->replaceArgument(1, null);
51-
$tokenHandlerDefinition->addMethodCall(
52-
'enableDiscovery',
53-
[
54-
new Reference($config['discovery']['cache']['id']),
55-
(new ChildDefinition('security.access_token_handler.oidc_discovery.http_client'))
56-
->replaceArgument(0, ['base_uri' => $config['discovery']['base_uri']]),
57-
"$id.oidc_configuration",
58-
"$id.oidc_jwk_set",
59-
]
60-
);
51+
52+
$clients = [];
53+
foreach ($config['discovery']['base_uri'] as $uri) {
54+
$clients[] = (new ChildDefinition('security.access_token_handler.oidc_discovery.http_client'))
55+
->replaceArgument(0, ['base_uri' => $uri]);
56+
}
57+
58+
$tokenHandlerDefinition->addMethodCall('enableDiscovery', [
59+
new Reference($config['discovery']['cache']['id']),
60+
$clients,
61+
"$id.oidc_configuration",
62+
]);
6163

6264
return;
6365
}
@@ -93,7 +95,7 @@ public function create(ContainerBuilder $container, string $id, array|string $co
9395
;
9496
}
9597

96-
$firewall = substr($id, strlen('security.access_token_handler.'));
98+
$firewall = substr($id, \strlen('security.access_token_handler.'));
9799
$container->getDefinition('security.access_token_handler.oidc.command.generate')
98100
->addMethodCall('addGenerator', [
99101
$firewall,
@@ -157,10 +159,11 @@ public function addConfiguration(NodeBuilder $node): void
157159
->arrayNode('discovery')
158160
->info('Enable the OIDC discovery.')
159161
->children()
160-
->scalarNode('base_uri')
162+
->arrayNode('base_uri')
163+
->acceptAndWrap(['string'])
161164
->info('Base URI of the OIDC server.')
162165
->isRequired()
163-
->cannotBeEmpty()
166+
->scalarPrototype()->end()
164167
->end()
165168
->arrayNode('cache')
166169
->children()

‎src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/php/access_token_oidc_user_info_multiple_discovery.php‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
<?php
2+
3+
$container->loadFromExtension('security', [
4+
'providers' => [
5+
'default' => [
6+
'memory' => null,
7+
],
8+
],
9+
'firewalls' => [
10+
'firewall1' => [
11+
'provider' => 'default',
12+
'access_token' => [
13+
'token_handler' => [
14+
'oidc_user_info' => [
15+
'base_uri' => [
16+
'https://www.example.com/realms/demo/protocol/openid-connect/userinfo',
17+
'https://www.github.com/realms/demo/protocol/openid-connect/userinfo',
18+
],
19+
'discovery' => [
20+
'cache' => [
21+
'id' => 'oidc_cache',
22+
],
23+
],
24+
],
25+
],
26+
],
27+
],
28+
],
29+
]);

‎src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Security/Factory/AccessTokenFactoryTest.php‎

Lines changed: 60 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -346,7 +346,6 @@ public function testInvalidOidcTokenHandlerConfigurationMissingAlgorithm()
346346
public function testOidcTokenHandlerConfigurationWithDiscovery()
347347
{
348348
$container = new ContainerBuilder();
349-
$jwkset = '{"keys":[{"kty":"EC","crv":"P-256","x":"FtgMtrsKDboRO-Zo0XC7tDJTATHVmwuf9GK409kkars","y":"rWDE0ERU2SfwGYCo1DWWdgFEbZ0MiAXLRBBOzBgs_jY","d":"4G7bRIiKih0qrFxc0dtvkHUll19tTyctoCR3eIbOrO0"},{"kty":"EC","crv":"P-256","x":"0QEAsI1wGI-dmYatdUZoWSRWggLEpyzopuhwk-YUnA4","y":"KYl-qyZ26HobuYwlQh-r0iHX61thfP82qqEku7i0woo","d":"iA_TV2zvftni_9aFAQwFO_9aypfJFCSpcCyevDvz220"}]}';
350349
$config = [
351350
'token_handler' => [
352351
'oidc' => [
@@ -384,10 +383,68 @@ public function testOidcTokenHandlerConfigurationWithDiscovery()
384383
'enableDiscovery',
385384
[
386385
new Reference('oidc_cache'),
387-
(new ChildDefinition('security.access_token_handler.oidc_discovery.http_client'))
386+
[
387+
(new ChildDefinition('security.access_token_handler.oidc_discovery.http_client'))
388388
->replaceArgument(0, ['base_uri' => 'https://www.example.com/realms/demo/']),
389+
],
390+
'security.access_token_handler.firewall1.oidc_configuration',
391+
],
392+
],
393+
];
394+
$this->assertEquals($expectedArgs, $container->getDefinition('security.access_token_handler.firewall1')->getArguments());
395+
$this->assertEquals($expectedCalls, $container->getDefinition('security.access_token_handler.firewall1')->getMethodCalls());
396+
}
397+
398+
public function testOidcTokenHandlerConfigurationWithMultipleDiscoveryBaseUri()
399+
{
400+
$container = new ContainerBuilder();
401+
$config = [
402+
'token_handler' => [
403+
'oidc' => [
404+
'discovery' => [
405+
'base_uri' => [
406+
'https://www.example.com/realms/demo/',
407+
'https://www.api.com/realms/api/',
408+
],
409+
'cache' => [
410+
'id' => 'oidc_cache',
411+
],
412+
],
413+
'algorithms' => ['RS256', 'ES256'],
414+
'issuers' => ['https://www.example.com'],
415+
'audience' => 'audience',
416+
],
417+
],
418+
];
419+
420+
$factory = new AccessTokenFactory($this->createTokenHandlerFactories());
421+
$finalizedConfig = $this->processConfig($config, $factory);
422+
423+
$factory->createAuthenticator($container, 'firewall1', $finalizedConfig, 'userprovider');
424+
425+
$this->assertTrue($container->hasDefinition('security.authenticator.access_token.firewall1'));
426+
$this->assertTrue($container->hasDefinition('security.access_token_handler.firewall1'));
427+
428+
$expectedArgs = [
429+
'index_0' => (new ChildDefinition('security.access_token_handler.oidc.signature'))
430+
->replaceArgument(0, ['RS256', 'ES256']),
431+
'index_1' => null,
432+
'index_2' => 'audience',
433+
'index_3' => ['https://www.example.com'],
434+
'index_4' => 'sub',
435+
];
436+
$expectedCalls = [
437+
[
438+
'enableDiscovery',
439+
[
440+
new Reference('oidc_cache'),
441+
[
442+
(new ChildDefinition('security.access_token_handler.oidc_discovery.http_client'))
443+
->replaceArgument(0, ['base_uri' => 'https://www.example.com/realms/demo/']),
444+
(new ChildDefinition('security.access_token_handler.oidc_discovery.http_client'))
445+
->replaceArgument(0, ['base_uri' => 'https://www.api.com/realms/api/']),
446+
],
389447
'security.access_token_handler.firewall1.oidc_configuration',
390-
'security.access_token_handler.firewall1.oidc_jwk_set',
391448
],
392449
],
393450
];

‎src/Symfony/Bundle/SecurityBundle/composer.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929
"symfony/password-hasher": "^6.4|^7.0|^8.0",
3030
"symfony/security-core": "^7.4|^8.0",
3131
"symfony/security-csrf": "^6.4|^7.0|^8.0",
32-
"symfony/security-http": "^7.3|^8.0",
32+
"symfony/security-http": "^7.4|^8.0",
3333
"symfony/service-contracts": "^2.5|^3"
3434
},
3535
"require-dev": {

‎src/Symfony/Component/Security/Http/AccessToken/Oidc/OidcTokenHandler.php‎

Lines changed: 55 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -48,9 +48,12 @@ final class OidcTokenHandler implements AccessTokenHandlerInterface
4848
private bool $enforceEncryption = false;
4949

5050
private ?CacheInterface $discoveryCache = null;
51-
private ?HttpClientInterface $discoveryClient = null;
5251
private ?string $oidcConfigurationCacheKey = null;
53-
private ?string $oidcJWKSetCacheKey = null;
52+
53+
/**
54+
* @var HttpClientInterface[]
55+
*/
56+
private array $discoveryClients = [];
5457

5558
public function __construct(
5659
private Algorithm|AlgorithmManager $signatureAlgorithm,
@@ -78,12 +81,18 @@ public function enableJweSupport(JWKSet $decryptionKeyset, AlgorithmManager $dec
7881
$this->enforceEncryption = $enforceEncryption;
7982
}
8083

81-
public function enableDiscovery(CacheInterface $cache, HttpClientInterface $client, string $oidcConfigurationCacheKey, string $oidcJWKSetCacheKey): void
84+
/**
85+
* @param HttpClientInterface|HttpClientInterface[] $client
86+
*/
87+
public function enableDiscovery(CacheInterface $cache, array|HttpClientInterface $client, string $oidcConfigurationCacheKey, ?string $oidcJWKSetCacheKey = null): void
8288
{
89+
if (null !== $oidcJWKSetCacheKey) {
90+
trigger_deprecation('symfony/security-http', '7.4', 'Passing $oidcJWKSetCacheKey parameter to "%s()" is deprecated.', __METHOD__);
91+
}
92+
8393
$this->discoveryCache = $cache;
84-
$this->discoveryClient = $client;
94+
$this->discoveryClients = \is_array($client) ? $client : [$client];
8595
$this->oidcConfigurationCacheKey = $oidcConfigurationCacheKey;
86-
$this->oidcJWKSetCacheKey = $oidcJWKSetCacheKey;
8796
}
8897

8998
public function getUserBadgeFrom(string $accessToken): UserBadge
@@ -92,45 +101,51 @@ public function getUserBadgeFrom(string $accessToken): UserBadge
92101
throw new \LogicException('You cannot use the "oidc" token handler since "web-token/jwt-signature" and "web-token/jwt-checker" are not installed. Try running "composer require web-token/jwt-signature web-token/jwt-checker".');
93102
}
94103

95-
if (!$this->discoveryCache && !$this->signatureKeyset) {
104+
if (!$this->discoveryClients && !$this->signatureKeyset) {
96105
throw new \LogicException('You cannot use the "oidc" token handler without JWKSet nor "discovery". Please configure JWKSet in the constructor, or call "enableDiscovery" method.');
97106
}
98107

99108
$jwkset = $this->signatureKeyset;
100-
if ($this->discoveryCache) {
101-
try {
102-
$oidcConfiguration = json_decode($this->discoveryCache->get($this->oidcConfigurationCacheKey, function (): string {
103-
$response = $this->discoveryClient->request('GET', '.well-known/openid-configuration');
104-
105-
return $response->getContent();
106-
}), true, 512, \JSON_THROW_ON_ERROR);
107-
} catch (\Throwable $e) {
108-
$this->logger?->error('An error occurred while requesting OIDC configuration.', [
109-
'error' => $e->getMessage(),
110-
'trace' => $e->getTraceAsString(),
111-
]);
112-
113-
throw new BadCredentialsException('Invalid credentials.', $e->getCode(), $e);
114-
}
115-
116-
try {
117-
$jwkset = JWKSet::createFromJson(
118-
$this->discoveryCache->get($this->oidcJWKSetCacheKey, function () use ($oidcConfiguration): string {
119-
$response = $this->discoveryClient->request('GET', $oidcConfiguration['jwks_uri']);
120-
// we only need signature key
121-
$keys = array_filter($response->toArray()['keys'], static fn (array $key) => 'sig' === $key['use']);
122-
123-
return json_encode(['keys' => $keys]);
124-
})
125-
);
126-
} catch (\Throwable $e) {
127-
$this->logger?->error('An error occurred while requesting OIDC certs.', [
128-
'error' => $e->getMessage(),
129-
'trace' => $e->getTraceAsString(),
130-
]);
131-
132-
throw new BadCredentialsException('Invalid credentials.', $e->getCode(), $e);
133-
}
109+
if ($this->discoveryClients) {
110+
$clients = $this->discoveryClients;
111+
$logger = $this->logger;
112+
$keys = $this->discoveryCache->get($this->oidcConfigurationCacheKey, static function () use ($clients, $logger): array {
113+
try {
114+
$configResponses = [];
115+
foreach ($clients as $client) {
116+
$configResponses[] = $client->request('GET', '.well-known/openid-configuration', [
117+
'user_data' => $client,
118+
]);
119+
}
120+
121+
$jwkSetResponses = [];
122+
foreach ($client->stream($configResponses) as $response => $chunk) {
123+
if ($chunk->isLast()) {
124+
$jwkSetResponses[] = $response->getInfo('user_data')->request('GET', $response->toArray()['jwks_uri']);
125+
}
126+
}
127+
128+
$keys = [];
129+
foreach ($jwkSetResponses as $response) {
130+
foreach ($response->toArray()['keys'] as $key) {
131+
if ('sig' === $key['use']) {
132+
$keys[] = $key;
133+
}
134+
}
135+
}
136+
137+
return $keys;
138+
} catch (\Exception $e) {
139+
$logger?->error('An error occurred while requesting OIDC certs.', [
140+
'error' => $e->getMessage(),
141+
'trace' => $e->getTraceAsString(),
142+
]);
143+
144+
throw new BadCredentialsException('Invalid credentials.', $e->getCode(), $e);
145+
}
146+
});
147+
148+
$jwkset = JWKSet::createFromKeyData(['keys' => $keys]);
134149
}
135150

136151
try {

‎src/Symfony/Component/Security/Http/CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ CHANGELOG
3636
* Allow subclassing `#[IsGranted]`
3737
* Add `$tokenSource` argument to `#[IsCsrfTokenValid]` to support reading tokens from the query string or headers
3838
* Deprecate `RememberMeDetails::getUserFqcn()`, the user FQCN will be removed from the remember-me cookie in 8.0
39+
* Allow configuring multiple OIDC discovery base URIs
3940

4041
7.3
4142
---

0 commit comments

Comments
 (0)