From 0958909cd999be772c045aada5bc426dffb0a0b1 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Fri, 19 Apr 2024 15:05:00 +0100 Subject: [PATCH] OIDC Userinfo: Added additional tests to cover jwks usage --- app/Access/Oidc/OidcJwtWithClaims.php | 4 +- app/Access/Oidc/OidcUserinfoResponse.php | 15 +++--- tests/Auth/OidcTest.php | 62 ++++++++++++++++++++++++ 3 files changed, 73 insertions(+), 8 deletions(-) diff --git a/app/Access/Oidc/OidcJwtWithClaims.php b/app/Access/Oidc/OidcJwtWithClaims.php index cc13936ab..393ac5f0e 100644 --- a/app/Access/Oidc/OidcJwtWithClaims.php +++ b/app/Access/Oidc/OidcJwtWithClaims.php @@ -59,7 +59,7 @@ class OidcJwtWithClaims implements ProvidesClaims * * @throws OidcInvalidTokenException */ - protected function validateCommonTokenDetails(): bool + public function validateCommonTokenDetails(): bool { $this->validateTokenStructure(); $this->validateTokenSignature(); @@ -151,7 +151,7 @@ class OidcJwtWithClaims implements ProvidesClaims * * @throws OidcInvalidTokenException */ - public function validateCommonClaims(): void + protected function validateCommonClaims(): void { // 1. The Issuer Identifier for the OpenID Provider (which is typically obtained during Discovery) // MUST exactly match the value of the iss (issuer) Claim. diff --git a/app/Access/Oidc/OidcUserinfoResponse.php b/app/Access/Oidc/OidcUserinfoResponse.php index bb6c2454a..0026d2f0a 100644 --- a/app/Access/Oidc/OidcUserinfoResponse.php +++ b/app/Access/Oidc/OidcUserinfoResponse.php @@ -20,11 +20,6 @@ class OidcUserinfoResponse implements ProvidesClaims $this->jwt = new OidcJwtWithClaims($response->getBody()->getContents(), $issuer, $keys); $this->claims = $this->jwt->getAllClaims(); } - - // TODO - Response validation (5.3.4): - // TODO - Verify that the OP that responded was the intended OP through a TLS server certificate check, per RFC 6125 [RFC6125]. - // TODO - If the Client has provided a userinfo_encrypted_response_alg parameter during Registration, decrypt the UserInfo Response using the keys specified during Registration. - // TODO - If the response was signed, the Client SHOULD validate the signature according to JWS [JWS]. } /** @@ -33,7 +28,7 @@ class OidcUserinfoResponse implements ProvidesClaims public function validate(string $idTokenSub): bool { if (!is_null($this->jwt)) { - $this->jwt->validateCommonClaims(); + $this->jwt->validateCommonTokenDetails(); } $sub = $this->getClaim('sub'); @@ -49,6 +44,14 @@ class OidcUserinfoResponse implements ProvidesClaims throw new OidcInvalidTokenException("Subject value provided in the userinfo endpoint does not match the provided ID token value"); } + // Spec v1.0 5.3.4 Defines the following: + // Verify that the OP that responded was the intended OP through a TLS server certificate check, per RFC 6125 [RFC6125]. + // This is effectively done as part of the HTTP request we're making through CURLOPT_SSL_VERIFYHOST on the request. + // If the Client has provided a userinfo_encrypted_response_alg parameter during Registration, decrypt the UserInfo Response using the keys specified during Registration. + // We don't currently support JWT encryption for OIDC + // If the response was signed, the Client SHOULD validate the signature according to JWS [JWS]. + // This is done as part of the validateCommonClaims above. + return true; } diff --git a/tests/Auth/OidcTest.php b/tests/Auth/OidcTest.php index 9ed3fa7b9..9bde71c80 100644 --- a/tests/Auth/OidcTest.php +++ b/tests/Auth/OidcTest.php @@ -787,6 +787,68 @@ class OidcTest extends TestCase $this->assertTrue($user->hasRole($roleA->id)); } + public function test_userinfo_endpoint_jwks_response_handled() + { + $userinfoResponseData = OidcJwtHelper::idToken(['name' => 'Barry Jwks']); + $userinfoResponse = new Response(200, ['Content-Type' => 'application/jwt'], $userinfoResponseData); + + $resp = $this->runLogin(['name' => null], [$userinfoResponse]); + $resp->assertRedirect('/'); + + $user = User::where('email', OidcJwtHelper::defaultPayload()['email'])->first(); + $this->assertEquals('Barry Jwks', $user->name); + } + + public function test_userinfo_endpoint_jwks_response_returning_no_sub_throws() + { + $userinfoResponseData = OidcJwtHelper::idToken(['sub' => null]); + $userinfoResponse = new Response(200, ['Content-Type' => 'application/jwt'], $userinfoResponseData); + + $resp = $this->runLogin(['name' => null], [$userinfoResponse]); + $resp->assertRedirect('/login'); + $this->assertSessionError('Userinfo endpoint response validation failed with error: No valid subject value found in userinfo data'); + } + + public function test_userinfo_endpoint_jwks_response_returning_non_matching_sub_throws() + { + $userinfoResponseData = OidcJwtHelper::idToken(['sub' => 'zzz123']); + $userinfoResponse = new Response(200, ['Content-Type' => 'application/jwt'], $userinfoResponseData); + + $resp = $this->runLogin(['name' => null], [$userinfoResponse]); + $resp->assertRedirect('/login'); + $this->assertSessionError('Userinfo endpoint response validation failed with error: Subject value provided in the userinfo endpoint does not match the provided ID token value'); + } + + public function test_userinfo_endpoint_jwks_response_with_invalid_signature_throws() + { + $userinfoResponseData = OidcJwtHelper::idToken(); + $exploded = explode('.', $userinfoResponseData); + $exploded[2] = base64_encode(base64_decode($exploded[2]) . 'ABC'); + $userinfoResponse = new Response(200, ['Content-Type' => 'application/jwt'], implode('.', $exploded)); + + $resp = $this->runLogin(['name' => null], [$userinfoResponse]); + $resp->assertRedirect('/login'); + $this->assertSessionError('Userinfo endpoint response validation failed with error: Token signature could not be validated using the provided keys'); + } + + public function test_userinfo_endpoint_jwks_response_with_invalid_signature_alg_throws() + { + $userinfoResponseData = OidcJwtHelper::idToken([], ['alg' => 'ZZ512']); + $userinfoResponse = new Response(200, ['Content-Type' => 'application/jwt'], $userinfoResponseData); + + $resp = $this->runLogin(['name' => null], [$userinfoResponse]); + $resp->assertRedirect('/login'); + $this->assertSessionError('Userinfo endpoint response validation failed with error: Only RS256 signature validation is supported. Token reports using ZZ512'); + } + + public function test_userinfo_endpoint_response_with_invalid_content_type_throws() + { + $userinfoResponse = new Response(200, ['Content-Type' => 'application/beans'], json_encode(OidcJwtHelper::defaultPayload())); + $resp = $this->runLogin(['name' => null], [$userinfoResponse]); + $resp->assertRedirect('/login'); + $this->assertSessionError('Userinfo endpoint response validation failed with error: No valid subject value found in userinfo data'); + } + protected function withAutodiscovery(): void { config()->set([