diff --git a/app/Auth/Oidc/Exceptions/OidcSigningKeyNotFoundException.php b/app/Auth/Oidc/Exceptions/OidcSigningKeyNotFoundException.php new file mode 100644 index 000000000..f57ddae55 --- /dev/null +++ b/app/Auth/Oidc/Exceptions/OidcSigningKeyNotFoundException.php @@ -0,0 +1,5 @@ + */ - public function jwks(string $jwksUri): array + public function jwks(string $jwksUri, bool $forceRefresh = false): array { $this->assertHttpsUrl($jwksUri, new OidcJwksException('JWKS URI must be an absolute HTTPS URL.')); $cacheKey = 'oidc:jwks:'.hash('sha256', $jwksUri); + if ($forceRefresh) { + $cooldownKey = $cacheKey.':refresh'; + if (Cache::add($cooldownKey, true, 60)) { + Cache::forget($cacheKey); + } + } + return Cache::remember($cacheKey, 21600, function () use ($jwksUri): array { try { $response = Http::timeout(5)->connectTimeout(3)->acceptJson()->get($jwksUri); diff --git a/app/Auth/Oidc/OidcTokenValidator.php b/app/Auth/Oidc/OidcTokenValidator.php index ee13cf712..a8563611d 100644 --- a/app/Auth/Oidc/OidcTokenValidator.php +++ b/app/Auth/Oidc/OidcTokenValidator.php @@ -2,10 +2,21 @@ namespace App\Auth\Oidc; +use App\Auth\Oidc\Exceptions\OidcSigningKeyNotFoundException; use App\Auth\Oidc\Exceptions\OidcTokenException; +use Firebase\JWT\JWK; +use Firebase\JWT\JWT; +use Throwable; class OidcTokenValidator { + /** + * Algorithms we accept for id_token signatures. RS256 only — this is the + * OIDC baseline and a strict allowlist prevents algorithm-confusion and + * "none" attacks. + */ + private const ALLOWED_ALGORITHM = 'RS256'; + /** * @param array $jwks * @return array @@ -18,10 +29,79 @@ public function validate( ?string $expectedNonce = null, int $clockSkewSeconds = 60, ): array { - [$header, $claims, $signatureInput, $signature] = $this->parse($idToken); + $kid = $this->extractKid($idToken); - $algorithm = $header['alg'] ?? null; - if ($algorithm !== 'RS256') { + try { + $keys = JWK::parseKeySet($this->signingKeysOnly($jwks), self::ALLOWED_ALGORITHM); + } catch (Throwable $e) { + throw new OidcTokenException("Unable to parse JWKS: {$e->getMessage()}", previous: $e); + } + + // Surface an unknown signing key distinctly so the caller can refresh + // the JWKS once (key rotation) before giving up. + if (! array_key_exists($kid, $keys)) { + throw new OidcSigningKeyNotFoundException('No matching JWKS key found for id_token kid.'); + } + + $previousLeeway = JWT::$leeway; + JWT::$leeway = $clockSkewSeconds; + + try { + // Validates signature, header alg against the key alg (RS256), + // exp, nbf and iat. Throws on any failure. + $claims = (array) JWT::decode($idToken, $keys); + } catch (OidcTokenException $e) { + throw $e; + } catch (Throwable $e) { + throw new OidcTokenException("id_token validation failed: {$e->getMessage()}", previous: $e); + } finally { + JWT::$leeway = $previousLeeway; + } + + $this->assertExpiry($claims); + $this->assertIssuer($claims, $discovery->issuer); + $this->assertAudience($claims, $clientId); + $this->assertNonce($claims, $expectedNonce); + $this->assertSubject($claims); + + return $claims; + } + + /** + * Drop JWKS entries explicitly marked for anything other than signing + * (e.g. "use":"enc") so they can never verify an id_token signature. + * firebase/php-jwt does not honour the "use" parameter on its own. + * + * @param array $jwks + * @return array + */ + private function signingKeysOnly(array $jwks): array + { + $keys = array_values(array_filter( + $jwks['keys'] ?? [], + fn ($jwk): bool => is_array($jwk) && (! isset($jwk['use']) || $jwk['use'] === 'sig'), + )); + + return ['keys' => $keys]; + } + + /** + * Decode just the JWT header to read the kid before signature + * verification, so an unknown key can be reported as a rotation miss. + */ + private function extractKid(string $idToken): string + { + $segments = explode('.', $idToken); + if (count($segments) !== 3) { + throw new OidcTokenException('Malformed id_token.'); + } + + $header = json_decode($this->base64UrlDecode($segments[0]), true); + if (! is_array($header)) { + throw new OidcTokenException('id_token header contains invalid JSON.'); + } + + if (($header['alg'] ?? null) !== self::ALLOWED_ALGORITHM) { throw new OidcTokenException('id_token uses a disallowed algorithm.'); } @@ -30,20 +110,33 @@ public function validate( throw new OidcTokenException('id_token header is missing kid.'); } - $jwk = $this->findJwk($jwks, $kid); - $publicKey = RsaJwk::toPem($jwk); + return $kid; + } - if (openssl_verify($signatureInput, $signature, $publicKey, OPENSSL_ALGO_SHA256) !== 1) { - throw new OidcTokenException('id_token signature is invalid.'); + private function base64UrlDecode(string $value): string + { + $remainder = strlen($value) % 4; + if ($remainder !== 0) { + $value .= str_repeat('=', 4 - $remainder); } - $this->assertIssuer($claims, $discovery->issuer); - $this->assertAudience($claims, $clientId); - $this->assertTimestamps($claims, $clockSkewSeconds); - $this->assertNonce($claims, $expectedNonce); - $this->assertSubject($claims); + $decoded = base64_decode(strtr($value, '-_', '+/'), true); + if ($decoded === false) { + throw new OidcTokenException('Invalid base64url value in id_token header.'); + } - return $claims; + return $decoded; + } + + /** + * @param array $claims + */ + private function assertExpiry(array $claims): void + { + // Firebase enforces the exp window when present; OIDC requires it to exist. + if (! is_numeric($claims['exp'] ?? null)) { + throw new OidcTokenException('id_token is missing the exp claim.'); + } } /** @@ -57,41 +150,6 @@ private function assertSubject(array $claims): void } } - /** - * @return array{0: array, 1: array, 2: string, 3: string} - */ - private function parse(string $idToken): array - { - $segments = explode('.', $idToken); - if (count($segments) !== 3) { - throw new OidcTokenException('Malformed id_token.'); - } - - $header = json_decode(RsaJwk::base64UrlDecode($segments[0]), true); - $claims = json_decode(RsaJwk::base64UrlDecode($segments[1]), true); - - if (! is_array($header) || ! is_array($claims)) { - throw new OidcTokenException('id_token contains invalid JSON.'); - } - - return [$header, $claims, $segments[0].'.'.$segments[1], RsaJwk::base64UrlDecode($segments[2])]; - } - - /** - * @param array $jwks - * @return array - */ - private function findJwk(array $jwks, string $kid): array - { - foreach ($jwks['keys'] ?? [] as $jwk) { - if (is_array($jwk) && ($jwk['kid'] ?? null) === $kid) { - return $jwk; - } - } - - throw new OidcTokenException('No matching JWKS key found for id_token kid.'); - } - /** * @param array $claims */ @@ -125,23 +183,6 @@ private function assertAudience(array $claims, string $clientId): void } } - /** - * @param array $claims - */ - private function assertTimestamps(array $claims, int $clockSkewSeconds): void - { - $now = time(); - $expiration = $claims['exp'] ?? null; - if (! is_int($expiration) || ($expiration + $clockSkewSeconds) < $now) { - throw new OidcTokenException('id_token is expired.'); - } - - $issuedAt = $claims['iat'] ?? null; - if (! is_int($issuedAt) || ($issuedAt - $clockSkewSeconds) > $now) { - throw new OidcTokenException('id_token issued at time is invalid.'); - } - } - /** * @param array $claims */ diff --git a/app/Auth/Oidc/RsaJwk.php b/app/Auth/Oidc/RsaJwk.php deleted file mode 100644 index 3faef77b2..000000000 --- a/app/Auth/Oidc/RsaJwk.php +++ /dev/null @@ -1,112 +0,0 @@ - $jwk - */ - public static function toPem(array $jwk): string - { - if (($jwk['kty'] ?? null) !== 'RSA' || ! is_string($jwk['n'] ?? null) || ! is_string($jwk['e'] ?? null)) { - throw new OidcTokenException('JWKS key is not a valid RSA signing key.'); - } - - $modulus = self::base64UrlDecode($jwk['n']); - $exponent = self::base64UrlDecode($jwk['e']); - - $sequence = self::encodeSequence( - self::encodeInteger($modulus). - self::encodeInteger($exponent) - ); - - $bitString = self::encodeBitString($sequence); - $algorithmIdentifier = self::encodeSequence( - self::encodeObjectIdentifier('1.2.840.113549.1.1.1'). - self::encodeNull() - ); - - $subjectPublicKeyInfo = self::encodeSequence($algorithmIdentifier.$bitString); - - return "-----BEGIN PUBLIC KEY-----\n". - chunk_split(base64_encode($subjectPublicKeyInfo), 64, "\n"). - "-----END PUBLIC KEY-----\n"; - } - - public static function base64UrlDecode(string $value): string - { - $remainder = strlen($value) % 4; - if ($remainder !== 0) { - $value .= str_repeat('=', 4 - $remainder); - } - - $decoded = base64_decode(strtr($value, '-_', '+/'), true); - if ($decoded === false) { - throw new OidcTokenException('Invalid base64url value.'); - } - - return $decoded; - } - - private static function encodeLength(int $length): string - { - if ($length < 128) { - return chr($length); - } - - $encoded = ltrim(pack('N', $length), "\x00"); - - return chr(0x80 | strlen($encoded)).$encoded; - } - - private static function encodeInteger(string $value): string - { - $value = ltrim($value, "\x00"); - if ($value === '') { - $value = "\x00"; - } - if ((ord($value[0]) & 0x80) !== 0) { - $value = "\x00".$value; - } - - return "\x02".self::encodeLength(strlen($value)).$value; - } - - private static function encodeSequence(string $value): string - { - return "\x30".self::encodeLength(strlen($value)).$value; - } - - private static function encodeBitString(string $value): string - { - $value = "\x00".$value; - - return "\x03".self::encodeLength(strlen($value)).$value; - } - - private static function encodeNull(): string - { - return "\x05\x00"; - } - - private static function encodeObjectIdentifier(string $oid): string - { - $parts = array_map('intval', explode('.', $oid)); - $encoded = chr((40 * $parts[0]) + $parts[1]); - - foreach (array_slice($parts, 2) as $part) { - $stack = [chr($part & 0x7F)]; - $part >>= 7; - while ($part > 0) { - array_unshift($stack, chr(($part & 0x7F) | 0x80)); - $part >>= 7; - } - $encoded .= implode('', $stack); - } - - return "\x06".self::encodeLength(strlen($encoded)).$encoded; - } -} diff --git a/app/Auth/Oidc/Socialite/OidcProvider.php b/app/Auth/Oidc/Socialite/OidcProvider.php index 34bb2a112..b9fc16f9c 100644 --- a/app/Auth/Oidc/Socialite/OidcProvider.php +++ b/app/Auth/Oidc/Socialite/OidcProvider.php @@ -3,6 +3,7 @@ namespace App\Auth\Oidc\Socialite; use App\Auth\Oidc\Exceptions\OidcException; +use App\Auth\Oidc\Exceptions\OidcSigningKeyNotFoundException; use App\Auth\Oidc\OidcConfig; use App\Auth\Oidc\OidcDiscoveryDocument; use App\Auth\Oidc\OidcDiscoveryService; @@ -144,16 +145,17 @@ public function user() throw new OidcException('OIDC login session expired. Please try again.'); } - $claims = $this->tokenValidator->validate( - idToken: $idToken, - discovery: $discovery, - jwks: $this->discoveryService->jwks($discovery->jwksUri), - clientId: $config->clientId, - expectedNonce: $expectedNonce, - clockSkewSeconds: $config->clockSkewSeconds, - ); + $claims = $this->validateIdToken($idToken, $discovery, $config, $expectedNonce); $userinfo = $this->getUserByToken($accessToken); + + // OIDC core §5.3.2: the userinfo sub MUST match the id_token sub. + // Reject the response rather than trust unsigned userinfo claims. + $userinfoSub = $userinfo['sub'] ?? null; + if (is_string($userinfoSub) && $userinfoSub !== '' && $userinfoSub !== ($claims['sub'] ?? null)) { + throw new OidcException('OIDC userinfo subject does not match the id_token subject.'); + } + $merged = array_merge($userinfo, $claims); /** @var OidcUser $user */ @@ -166,6 +168,39 @@ public function user() return $this->user = $user; } + /** + * Validate the id_token, retrying once against a freshly fetched JWKS when + * the signing key is unknown. This keeps logins working immediately after + * the IdP rotates keys instead of failing until the JWKS cache expires. + * + * @return array + */ + protected function validateIdToken( + string $idToken, + OidcDiscoveryDocument $discovery, + OidcConfig $config, + ?string $expectedNonce, + ): array { + foreach ([false, true] as $forceRefresh) { + try { + return $this->tokenValidator->validate( + idToken: $idToken, + discovery: $discovery, + jwks: $this->discoveryService->jwks($discovery->jwksUri, $forceRefresh), + clientId: $config->clientId, + expectedNonce: $expectedNonce, + clockSkewSeconds: $config->clockSkewSeconds, + ); + } catch (OidcSigningKeyNotFoundException $e) { + if ($forceRefresh) { + throw $e; + } + } + } + + throw new OidcSigningKeyNotFoundException('No matching JWKS key found for id_token kid.'); + } + /** * @return array */ diff --git a/app/Services/Auth/OauthLoginService.php b/app/Services/Auth/OauthLoginService.php index 25c9c465b..b0c47a395 100644 --- a/app/Services/Auth/OauthLoginService.php +++ b/app/Services/Auth/OauthLoginService.php @@ -66,7 +66,7 @@ private function resolveOidcUser(object $oauthUser, OauthSetting $oauthSetting, throw new HttpException(403, 'OIDC provider did not verify the email address'); } - return DB::transaction(function () use ($oauthUser, $oauthSetting, $email, $issuer, $subject) { + return DB::transaction(function () use ($oauthUser, $oauthSetting, $email, $issuer, $subject, $emailVerified) { $identity = OauthIdentity::where([ 'provider' => 'oidc', 'issuer' => $issuer, @@ -84,6 +84,15 @@ private function resolveOidcUser(object $oauthUser, OauthSetting $oauthSetting, } $user = User::whereEmail($email)->first(); + + // Linking a new OIDC identity to an existing local account by email + // is account takeover unless the provider attests the email. This + // guard is independent of the require_email_verified toggle, which + // only governs the broader login flow. + if ($user && ! $emailVerified) { + throw new HttpException(403, 'OIDC provider must verify the email address before linking to an existing account'); + } + if (! $user) { if (! $this->canCreateUser($oauthSetting)) { throw new HttpException(403, 'Registration is disabled'); diff --git a/composer.json b/composer.json index 9415aa624..923bc841b 100644 --- a/composer.json +++ b/composer.json @@ -14,6 +14,7 @@ "php": "^8.4", "danharrin/livewire-rate-limiting": "^2.1.0", "doctrine/dbal": "^4.4.1", + "firebase/php-jwt": "7.1.0", "guzzlehttp/guzzle": "^7.10.0", "laravel/fortify": "^1.34.0", "laravel/framework": "^12.49.0", diff --git a/composer.lock b/composer.lock index 7d958a9cc..9866c8474 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "64b77285a7140ce68e83db2659e9a21d", + "content-hash": "115a84e56a483f0c336c220b4d0246d7", "packages": [ { "name": "aws/aws-crt-php", @@ -1035,16 +1035,16 @@ }, { "name": "firebase/php-jwt", - "version": "v7.0.5", + "version": "v7.1.0", "source": { "type": "git", "url": "https://github.com/googleapis/php-jwt.git", - "reference": "47ad26bab5e7c70ae8a6f08ed25ff83631121380" + "reference": "b374a5d1a4f1f67fadc2165cdb284645945e2fc0" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/googleapis/php-jwt/zipball/47ad26bab5e7c70ae8a6f08ed25ff83631121380", - "reference": "47ad26bab5e7c70ae8a6f08ed25ff83631121380", + "url": "https://api.github.com/repos/googleapis/php-jwt/zipball/b374a5d1a4f1f67fadc2165cdb284645945e2fc0", + "reference": "b374a5d1a4f1f67fadc2165cdb284645945e2fc0", "shasum": "" }, "require": { @@ -1053,6 +1053,7 @@ "require-dev": { "guzzlehttp/guzzle": "^7.4", "phpfastcache/phpfastcache": "^9.2", + "phpseclib/phpseclib": "~3.0", "phpspec/prophecy-phpunit": "^2.0", "phpunit/phpunit": "^9.5", "psr/cache": "^2.0||^3.0", @@ -1061,7 +1062,8 @@ }, "suggest": { "ext-sodium": "Support EdDSA (Ed25519) signatures", - "paragonie/sodium_compat": "Support EdDSA (Ed25519) signatures when libsodium is not present" + "paragonie/sodium_compat": "Support EdDSA (Ed25519) signatures when libsodium is not present", + "phpseclib/phpseclib": "Support PS256 (RSASSA-PSS) signatures" }, "type": "library", "autoload": { @@ -1086,16 +1088,16 @@ } ], "description": "A simple library to encode and decode JSON Web Tokens (JWT) in PHP. Should conform to the current spec.", - "homepage": "https://github.com/firebase/php-jwt", + "homepage": "https://github.com/googleapis/php-jwt", "keywords": [ "jwt", "php" ], "support": { "issues": "https://github.com/googleapis/php-jwt/issues", - "source": "https://github.com/googleapis/php-jwt/tree/v7.0.5" + "source": "https://github.com/googleapis/php-jwt/tree/v7.1.0" }, - "time": "2026-04-01T20:38:03+00:00" + "time": "2026-06-11T17:54:14+00:00" }, { "name": "fruitcake/php-cors", @@ -18072,5 +18074,5 @@ "php": "^8.4" }, "platform-dev": {}, - "plugin-api-version": "2.9.0" + "plugin-api-version": "2.6.0" } diff --git a/tests/Feature/OidcOauthControllerTest.php b/tests/Feature/OidcOauthControllerTest.php index b37694bbc..6851ba97a 100644 --- a/tests/Feature/OidcOauthControllerTest.php +++ b/tests/Feature/OidcOauthControllerTest.php @@ -93,6 +93,21 @@ function fakeOidcProvider(array $claims = []): void ]); }); +it('rejects linking an unverified oidc email to an existing local account', function () { + $user = User::factory()->create(['email' => 'victim@example.com']); + + fakeOidcProvider(['email' => 'victim@example.com', 'email_verified' => false]); + + $response = $this->from('/login')->get(route('auth.callback', 'oidc')); + + $response->assertRedirect('/login'); + $this->assertGuest(); + $this->assertDatabaseMissing('oauth_identities', [ + 'user_id' => $user->id, + 'provider' => 'oidc', + ]); +}); + it('rejects new oidc users when neither normal nor provider registration is enabled', function () { fakeOidcProvider(['email' => 'blocked@example.com']); diff --git a/tests/Unit/OidcDiscoveryServiceTest.php b/tests/Unit/OidcDiscoveryServiceTest.php index 79173a415..458dfeede 100644 --- a/tests/Unit/OidcDiscoveryServiceTest.php +++ b/tests/Unit/OidcDiscoveryServiceTest.php @@ -38,6 +38,25 @@ 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') + ->push(['keys' => [['kid' => 'old']]]) + ->push(['keys' => [['kid' => 'new']]]); + + $service = app(OidcDiscoveryService::class); + + expect($service->jwks('https://idp.example.com/jwks')['keys'][0]['kid'])->toBe('old'); + + // Forced refresh bypasses the cache and sees the rotated key. + expect($service->jwks('https://idp.example.com/jwks', true)['keys'][0]['kid'])->toBe('new'); + Http::assertSentCount(2); + + // Cooldown prevents a second immediate upstream fetch; cached value returned. + expect($service->jwks('https://idp.example.com/jwks', true)['keys'][0]['kid'])->toBe('new'); + Http::assertSentCount(2); +}); + it('rejects invalid discovery and jwks payloads', function () { Cache::flush(); Http::fake([ diff --git a/tests/Unit/OidcTokenValidatorTest.php b/tests/Unit/OidcTokenValidatorTest.php index e2546efe1..3ca021de5 100644 --- a/tests/Unit/OidcTokenValidatorTest.php +++ b/tests/Unit/OidcTokenValidatorTest.php @@ -1,5 +1,6 @@ validate($token, oidc_discovery(), $keyset['jwks'], 'client-id'); })->throws(OidcTokenException::class); + +it('throws a dedicated exception when the signing key is unknown', function () { + $keyset = oidc_keyset('current-key'); + $token = oidc_token([ + 'iss' => 'https://idp.example.com', + 'aud' => 'client-id', + 'sub' => 'okta-user-1', + 'iat' => time(), + 'exp' => time() + 600, + ], $keyset['private_pem'], 'rotated-key'); + + app(OidcTokenValidator::class)->validate($token, oidc_discovery(), $keyset['jwks'], 'client-id'); +})->throws(OidcSigningKeyNotFoundException::class); + +it('rejects a jwks key not designated for signing', function () { + $keyset = oidc_keyset(); + $keyset['jwks']['keys'][0]['use'] = 'enc'; + $now = time(); + $token = oidc_token([ + 'iss' => 'https://idp.example.com', + 'aud' => 'client-id', + 'sub' => 'okta-user-1', + 'iat' => $now, + 'exp' => $now + 600, + ], $keyset['private_pem']); + + // An encryption-only key is dropped from the keyset, so the kid no longer resolves. + app(OidcTokenValidator::class)->validate($token, oidc_discovery(), $keyset['jwks'], 'client-id'); +})->throws(OidcTokenException::class);