From c9264dde2e5432b4054105b7d3132814b4088a18 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Mon, 15 Jun 2026 17:10:55 +0200 Subject: [PATCH] fix(auth): avoid caching invalid OIDC discovery documents --- app/Auth/Oidc/OidcDiscoveryService.php | 16 ++++++-------- tests/Unit/OidcDiscoveryServiceTest.php | 29 +++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 9 deletions(-) diff --git a/app/Auth/Oidc/OidcDiscoveryService.php b/app/Auth/Oidc/OidcDiscoveryService.php index e3223ea98..0847afc9a 100644 --- a/app/Auth/Oidc/OidcDiscoveryService.php +++ b/app/Auth/Oidc/OidcDiscoveryService.php @@ -17,7 +17,7 @@ class OidcDiscoveryService $issuerUrl = rtrim($issuerUrl, '/'); $cacheKey = 'oidc:discovery:'.hash('sha256', $issuerUrl); - $payload = Cache::remember($cacheKey, 3600, function () use ($issuerUrl): array { + return Cache::remember($cacheKey, 3600, function () use ($issuerUrl): OidcDiscoveryDocument { $url = $issuerUrl.'/.well-known/openid-configuration'; try { @@ -35,15 +35,13 @@ class OidcDiscoveryService throw new OidcDiscoveryException('Discovery endpoint returned invalid JSON.'); } - return $json; + $discovery = OidcDiscoveryDocument::fromArray($json); + if (rtrim($discovery->issuer, '/') !== $issuerUrl) { + throw new OidcDiscoveryException('Discovery issuer does not match the configured issuer URL.'); + } + + return $discovery; }); - - $discovery = OidcDiscoveryDocument::fromArray($payload); - if (rtrim($discovery->issuer, '/') !== $issuerUrl) { - throw new OidcDiscoveryException('Discovery issuer does not match the configured issuer URL.'); - } - - return $discovery; } /** diff --git a/tests/Unit/OidcDiscoveryServiceTest.php b/tests/Unit/OidcDiscoveryServiceTest.php index 458dfeede..18c358fd1 100644 --- a/tests/Unit/OidcDiscoveryServiceTest.php +++ b/tests/Unit/OidcDiscoveryServiceTest.php @@ -38,6 +38,35 @@ it('fetches and caches discovery documents and jwks', function () { Http::assertSentCount(2); }); +it('does not cache discovery documents with mismatched issuers', function () { + Cache::flush(); + Http::fakeSequence('https://idp.example.com/.well-known/openid-configuration') + ->push([ + 'issuer' => 'https://evil.example.com', + 'authorization_endpoint' => 'https://idp.example.com/auth', + 'token_endpoint' => 'https://idp.example.com/token', + 'userinfo_endpoint' => 'https://idp.example.com/userinfo', + 'jwks_uri' => 'https://idp.example.com/jwks', + ]) + ->push([ + 'issuer' => 'https://idp.example.com', + 'authorization_endpoint' => 'https://idp.example.com/auth', + 'token_endpoint' => 'https://idp.example.com/token', + 'userinfo_endpoint' => 'https://idp.example.com/userinfo', + 'jwks_uri' => 'https://idp.example.com/jwks', + ]); + + $service = app(OidcDiscoveryService::class); + $cacheKey = 'oidc:discovery:'.hash('sha256', 'https://idp.example.com'); + + expect(fn () => $service->discover('https://idp.example.com')) + ->toThrow(OidcDiscoveryException::class, 'Discovery issuer does not match the configured issuer URL.') + ->and(Cache::has($cacheKey))->toBeFalse() + ->and($service->discover('https://idp.example.com')->issuer)->toBe('https://idp.example.com'); + + Http::assertSentCount(2); +}); + it('refetches jwks once on forced refresh to pick up rotated keys', function () { Cache::flush(); Http::fakeSequence('https://idp.example.com/jwks')