Skip to content

Commit 3dfe841

Browse files
nicolas-grekasnicolas-grekas
committed
Merge branch '7.4' into 8.0
* 7.4: [Security] Set OIDC JWKS cache TTL from provider headers [DependencyInjection] Call default index method when index is not provided by tag [Console] Remove a redundant local variable in the console Application class. [Cache] Recognize commit events as writes in `CacheDataCollector` [Routing] Align routing.schema.json with YamlFileLoader behavior Bump Symfony version to 7.4.0 Update VERSION for 7.4.0-RC1 Update CHANGELOG for 7.4.0-RC1
2 parents d9473cc + 8c595fd commit 3dfe841

File tree

2 files changed

+145
-37
lines changed

2 files changed

+145
-37
lines changed

‎AccessToken/Oidc/OidcTokenHandler.php‎

Lines changed: 66 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
use Symfony\Component\Security\Http\Authenticator\FallbackUserLoader;
3434
use Symfony\Component\Security\Http\Authenticator\Passport\Badge\UserBadge;
3535
use Symfony\Contracts\Cache\CacheInterface;
36+
use Symfony\Contracts\Cache\ItemInterface;
3637
use Symfony\Contracts\HttpClient\HttpClientInterface;
3738

3839
/**
@@ -93,43 +94,7 @@ public function getUserBadgeFrom(string $accessToken): UserBadge
9394

9495
$jwkset = $this->signatureKeyset;
9596
if ($this->discoveryClients) {
96-
$clients = $this->discoveryClients;
97-
$logger = $this->logger;
98-
$keys = $this->discoveryCache->get($this->oidcConfigurationCacheKey, static function () use ($clients, $logger): array {
99-
try {
100-
$configResponses = [];
101-
foreach ($clients as $client) {
102-
$configResponses[] = $client->request('GET', '.well-known/openid-configuration', [
103-
'user_data' => $client,
104-
]);
105-
}
106-
107-
$jwkSetResponses = [];
108-
foreach ($client->stream($configResponses) as $response => $chunk) {
109-
if ($chunk->isLast()) {
110-
$jwkSetResponses[] = $response->getInfo('user_data')->request('GET', $response->toArray()['jwks_uri']);
111-
}
112-
}
113-
114-
$keys = [];
115-
foreach ($jwkSetResponses as $response) {
116-
foreach ($response->toArray()['keys'] as $key) {
117-
if ('sig' === $key['use']) {
118-
$keys[] = $key;
119-
}
120-
}
121-
}
122-
123-
return $keys;
124-
} catch (\Exception $e) {
125-
$logger?->error('An error occurred while requesting OIDC certs.', [
126-
'error' => $e->getMessage(),
127-
'trace' => $e->getTraceAsString(),
128-
]);
129-
130-
throw new BadCredentialsException('Invalid credentials.', $e->getCode(), $e);
131-
}
132-
});
97+
$keys = $this->discoveryCache->get($this->oidcConfigurationCacheKey, [$this, 'computeDiscoveryKeys']);
13398

13499
$jwkset = JWKSet::createFromKeyData(['keys' => $keys]);
135100
}
@@ -159,6 +124,70 @@ public function getUserBadgeFrom(string $accessToken): UserBadge
159124
}
160125
}
161126

127+
/**
128+
* Computes the JWKS and sets the cache item TTL from provider headers.
129+
*
130+
* The cache entry lifetime is automatically adjusted based on the lowest TTL
131+
* advertised by the providers (via "Cache-Control: max-age" or "Expires" headers).
132+
*
133+
* @internal this method is public to enable async offline cache population
134+
*/
135+
public function computeDiscoveryKeys(ItemInterface $item): array
136+
{
137+
$clients = $this->discoveryClients;
138+
$logger = $this->logger;
139+
140+
try {
141+
$configResponses = [];
142+
foreach ($clients as $client) {
143+
$configResponses[] = $client->request('GET', '.well-known/openid-configuration', [
144+
'user_data' => $client,
145+
]);
146+
}
147+
148+
$jwkSetResponses = [];
149+
foreach ($client->stream($configResponses) as $response => $chunk) {
150+
if ($chunk->isLast()) {
151+
$jwkSetResponses[] = $response->getInfo('user_data')->request('GET', $response->toArray()['jwks_uri']);
152+
}
153+
}
154+
$keys = [];
155+
$minTtl = null;
156+
foreach ($jwkSetResponses as $response) {
157+
$headers = $response->getHeaders();
158+
if (preg_match('/max-age=(\d+)/', $headers['cache-control'][0] ?? '', $m)) {
159+
$currentTtl = (int) $m[1];
160+
} elseif (0 >= $currentTtl = strtotime($headers['expires'][0] ?? '@0') - time()) {
161+
$currentTtl = null;
162+
}
163+
164+
// Apply the lowest TTL found to ensure all keys in the set are still valid
165+
if (null !== $currentTtl && (null === $minTtl || $currentTtl < $minTtl)) {
166+
$minTtl = $currentTtl;
167+
}
168+
169+
foreach ($response->toArray()['keys'] as $key) {
170+
if ('sig' === $key['use']) {
171+
$keys[] = $key;
172+
}
173+
}
174+
}
175+
176+
if (0 < ($minTtl ?? -1)) {
177+
// Cap the TTL to 30 days to avoid keeping JWKS indefinitely
178+
$item->expiresAfter(min($minTtl, 30 * 24 * 60 * 60));
179+
}
180+
181+
return $keys;
182+
} catch (\Exception $e) {
183+
$logger?->error('An error occurred while requesting OIDC certs.', [
184+
'error' => $e->getMessage(),
185+
'trace' => $e->getTraceAsString(),
186+
]);
187+
throw new BadCredentialsException('Invalid credentials.', $e->getCode(), $e);
188+
}
189+
}
190+
162191
private function loadAndVerifyJws(string $accessToken, JWKSet $jwkset): array
163192
{
164193
// Decode the token

‎Tests/AccessToken/Oidc/OidcTokenHandlerTest.php‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -311,4 +311,83 @@ private static function buildJWSWithKey(string $payload, JWK $jwk): string
311311
->build()
312312
);
313313
}
314+
315+
public function testDiscoveryCachesJwksAccordingToCacheControl()
316+
{
317+
$time = time();
318+
$claims = [
319+
'iat' => $time, 'nbf' => $time,
320+
'exp' => $time + 3600,
321+
'iss' => 'https://www.example.com',
322+
'aud' => self::AUDIENCE,
323+
'sub' => 'user-cache-control',
324+
];
325+
$token = self::buildJWS(json_encode($claims));
326+
327+
$requestCount = 0;
328+
$httpClient = new MockHttpClient(function ($method, $url) use (&$requestCount) {
329+
++$requestCount;
330+
if (str_contains($url, 'openid-configuration')) {
331+
return new JsonMockResponse(['jwks_uri' => 'https://www.example.com/jwks.json']);
332+
}
333+
334+
return new JsonMockResponse(
335+
['keys' => [array_merge(self::getJWK()->all(), ['use' => 'sig'])]],
336+
['response_headers' => ['Cache-Control' => 'public, max-age=120']]
337+
);
338+
});
339+
340+
$cache = new ArrayAdapter();
341+
$handler = new OidcTokenHandler(
342+
new AlgorithmManager([new ES256()]),
343+
null,
344+
self::AUDIENCE,
345+
['https://www.example.com']
346+
);
347+
$handler->enableDiscovery($cache, $httpClient, 'oidc_ttl_cc');
348+
$this->assertSame('user-cache-control', $handler->getUserBadgeFrom($token)->getUserIdentifier());
349+
$this->assertSame(2, $requestCount);
350+
$this->assertSame('user-cache-control', $handler->getUserBadgeFrom($token)->getUserIdentifier());
351+
$this->assertSame(2, $requestCount);
352+
}
353+
354+
public function testDiscoveryCachesJwksAccordingToExpires()
355+
{
356+
$time = time();
357+
$claims = [
358+
'iat' => $time, 'nbf' => $time,
359+
'exp' => $time + 3600,
360+
'iss' => 'https://www.example.com',
361+
'aud' => self::AUDIENCE,
362+
'sub' => 'user-expires',
363+
];
364+
365+
$token = self::buildJWS(json_encode($claims));
366+
367+
$requestCount = 0;
368+
$httpClient = new MockHttpClient(function ($method, $url) use (&$requestCount) {
369+
++$requestCount;
370+
if (str_contains($url, 'openid-configuration')) {
371+
return new JsonMockResponse(['jwks_uri' => 'https://www.example.com/jwks.json']);
372+
}
373+
374+
return new JsonMockResponse(
375+
['keys' => [array_merge(self::getJWK()->all(), ['use' => 'sig'])]],
376+
['response_headers' => ['Expires' => gmdate('D, d M Y H:i:s \G\M\T', time() + 60)]]
377+
);
378+
});
379+
380+
$cache = new ArrayAdapter();
381+
$handler = new OidcTokenHandler(
382+
new AlgorithmManager([new ES256()]),
383+
null,
384+
self::AUDIENCE,
385+
['https://www.example.com']
386+
);
387+
$handler->enableDiscovery($cache, $httpClient, 'oidc_ttl_expires');
388+
$this->assertSame('user-expires', $handler->getUserBadgeFrom($token)->getUserIdentifier());
389+
$this->assertSame(2, $requestCount);
390+
$this->assertSame('user-expires', $handler->getUserBadgeFrom($token)->getUserIdentifier());
391+
$this->assertSame(2, $requestCount);
392+
}
314393
}

0 commit comments

Comments
 (0)