From 790723dfc52ca2169234bb86450e03aea51b3676 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Tue, 12 Oct 2021 16:48:54 +0100 Subject: [PATCH] Added further OIDC core class testing --- .../OpenIdConnect/OpenIdConnectIdToken.php | 4 +- .../OpenIdConnect/OpenIdConnectService.php | 2 +- tests/Unit/OpenIdConnectIdTokenTest.php | 174 +++++++++++++++++- 3 files changed, 168 insertions(+), 12 deletions(-) diff --git a/app/Auth/Access/OpenIdConnect/OpenIdConnectIdToken.php b/app/Auth/Access/OpenIdConnect/OpenIdConnectIdToken.php index 0ee43e663..1c1d0913d 100644 --- a/app/Auth/Access/OpenIdConnect/OpenIdConnectIdToken.php +++ b/app/Auth/Access/OpenIdConnect/OpenIdConnectIdToken.php @@ -97,7 +97,7 @@ class OpenIdConnectIdToken /** * Get all returned claims within the token. */ - public function claims(): array + public function getAllClaims(): array { return $this->payload; } @@ -174,7 +174,7 @@ class OpenIdConnectIdToken $aud = is_string($this->payload['aud']) ? [$this->payload['aud']] : $this->payload['aud']; if (count($aud) !== 1) { - throw new InvalidTokenException('Token audience value has ' . count($aud) . ' values. Expected 1.'); + throw new InvalidTokenException('Token audience value has ' . count($aud) . ' values, Expected 1'); } if ($aud[0] !== $clientId) { diff --git a/app/Auth/Access/OpenIdConnect/OpenIdConnectService.php b/app/Auth/Access/OpenIdConnect/OpenIdConnectService.php index 0f9fed006..7471a5007 100644 --- a/app/Auth/Access/OpenIdConnect/OpenIdConnectService.php +++ b/app/Auth/Access/OpenIdConnect/OpenIdConnectService.php @@ -136,7 +136,7 @@ class OpenIdConnectService ); if ($this->config['dump_user_details']) { - throw new JsonDebugException($idToken->claims()); + throw new JsonDebugException($idToken->getAllClaims()); } try { diff --git a/tests/Unit/OpenIdConnectIdTokenTest.php b/tests/Unit/OpenIdConnectIdTokenTest.php index fdde54f27..d3394daf6 100644 --- a/tests/Unit/OpenIdConnectIdTokenTest.php +++ b/tests/Unit/OpenIdConnectIdTokenTest.php @@ -2,25 +2,169 @@ namespace Tests\Unit; +use BookStack\Auth\Access\OpenIdConnect\InvalidTokenException; use BookStack\Auth\Access\OpenIdConnect\OpenIdConnectIdToken; use phpseclib3\Crypt\RSA; use Tests\TestCase; class OpenIdConnectIdTokenTest extends TestCase { - public function test_valid_token_passes_validation() { - $token = new OpenIdConnectIdToken($this->getValidToken(), 'https://auth.example.com', [ + $token = new OpenIdConnectIdToken($this->idToken(), 'https://auth.example.com', [ $this->jwkKeyArray() ]); $this->assertTrue($token->validate('xxyyzz.aaa.bbccdd.123')); } - protected function getValidToken($payloadOverrides = []): string + public function test_get_claim_returns_value_if_existing() { - $defaultPayload = [ + $token = new OpenIdConnectIdToken($this->idToken(), 'https://auth.example.com', []); + $this->assertEquals('bscott@example.com', $token->getClaim('email')); + } + + public function test_get_claim_returns_null_if_not_existing() + { + $token = new OpenIdConnectIdToken($this->idToken(), 'https://auth.example.com', []); + $this->assertEquals(null, $token->getClaim('emails')); + } + + public function test_get_all_claims_returns_all_payload_claims() + { + $defaultPayload = $this->getDefaultPayload(); + $token = new OpenIdConnectIdToken($this->idToken($defaultPayload), 'https://auth.example.com', []); + $this->assertEquals($defaultPayload, $token->getAllClaims()); + } + + public function test_token_structure_error_cases() + { + $idToken = $this->idToken(); + $idTokenExploded = explode('.', $idToken); + + $messagesAndTokenValues = [ + ['Could not parse out a valid header within the provided token', ''], + ['Could not parse out a valid header within the provided token', 'cat'], + ['Could not parse out a valid payload within the provided token', $idTokenExploded[0]], + ['Could not parse out a valid payload within the provided token', $idTokenExploded[0] . '.' . 'dog'], + ['Could not parse out a valid signature within the provided token', $idTokenExploded[0] . '.' . $idTokenExploded[1]], + ['Could not parse out a valid signature within the provided token', $idTokenExploded[0] . '.' . $idTokenExploded[1] . '.' . '@$%'], + ]; + + foreach ($messagesAndTokenValues as [$message, $tokenValue]) { + $token = new OpenIdConnectIdToken($tokenValue, 'https://auth.example.com', []); + $err = null; + try { + $token->validate('abc'); + } catch (\Exception $exception) { + $err = $exception; + } + + $this->assertInstanceOf(InvalidTokenException::class, $err, $message); + $this->assertEquals($message, $err->getMessage()); + } + } + + public function test_error_thrown_if_token_signature_not_validated_from_no_keys() + { + $token = new OpenIdConnectIdToken($this->idToken(), 'https://auth.example.com', []); + $this->expectException(InvalidTokenException::class); + $this->expectExceptionMessage('Token signature could not be validated using the provided keys'); + $token->validate('abc'); + } + + public function test_error_thrown_if_token_signature_not_validated_from_non_matching_key() + { + $token = new OpenIdConnectIdToken($this->idToken(), 'https://auth.example.com', [ + array_merge($this->jwkKeyArray(), [ + 'n' => 'iqK-1QkICMf_cusNLpeNnN-bhT0-9WLBvzgwKLALRbrevhdi5ttrLHIQshaSL0DklzfyG2HWRmAnJ9Q7sweEjuRiiqRcSUZbYu8cIv2hLWYu7K_NH67D2WUjl0EnoHEuiVLsZhQe1CmdyLdx087j5nWkd64K49kXRSdxFQUlj8W3NeK3CjMEUdRQ3H4RZzJ4b7uuMiFA29S2ZhMNG20NPbkUVsFL-jiwTd10KSsPT8yBYipI9O7mWsUWt_8KZs1y_vpM_k3SyYihnWpssdzDm1uOZ8U3mzFr1xsLAO718GNUSXk6npSDzLl59HEqa6zs4O9awO2qnSHvcmyELNk31w' + ]) + ]); + $this->expectException(InvalidTokenException::class); + $this->expectExceptionMessage('Token signature could not be validated using the provided keys'); + $token->validate('abc'); + } + + public function test_error_thrown_if_token_signature_not_validated_from_invalid_key() + { + $token = new OpenIdConnectIdToken($this->idToken(), 'https://auth.example.com', ['url://example.com']); + $this->expectException(InvalidTokenException::class); + $this->expectExceptionMessage('Token signature could not be validated using the provided keys'); + $token->validate('abc'); + } + + public function test_error_thrown_if_token_algorithm_is_not_rs256() + { + $token = new OpenIdConnectIdToken($this->idToken([], ['alg' => 'HS256']), 'https://auth.example.com', []); + $this->expectException(InvalidTokenException::class); + $this->expectExceptionMessage("Only RS256 signature validation is supported. Token reports using HS256"); + $token->validate('abc'); + } + + public function test_token_claim_error_cases() + { + /** @var array $claimOverridesByErrorMessage */ + $claimOverridesByErrorMessage = [ + // 1. iss claim present + ['Missing or non-matching token issuer value', ['iss' => null]], + // 1. iss claim matches provided issuer + ['Missing or non-matching token issuer value', ['iss' => 'https://auth.example.co.uk']], + // 2. aud claim present + ['Missing token audience value', ['aud' => null]], + // 2. aud claim validates all values against those expected (Only expect single) + ['Token audience value has 2 values, Expected 1', ['aud' => ['abc', 'def']]], + // 2. aud claim matches client id + ['Token audience value did not match the expected client_id', ['aud' => 'xxyyzz.aaa.bbccdd.456']], + // 4. azp claim matches client id if present + ['Token authorized party exists but does not match the expected client_id', ['azp' => 'xxyyzz.aaa.bbccdd.456']], + // 5. exp claim present + ['Missing token expiration time value', ['exp' => null]], + // 5. exp claim not expired + ['Token has expired', ['exp' => time() - 360]], + // 6. iat claim present + ['Missing token issued at time value', ['iat' => null]], + // 6. iat claim too far in the future + ['Token issue at time is not recent or is invalid', ['iat' => time() + 600]], + // 6. iat claim too far in the past + ['Token issue at time is not recent or is invalid', ['iat' => time() - 172800]], + + // Custom: sub is present + ['Missing token subject value', ['sub' => null]], + ]; + + foreach ($claimOverridesByErrorMessage as [$message, $overrides]) { + $token = new OpenIdConnectIdToken($this->idToken($overrides), 'https://auth.example.com', [ + $this->jwkKeyArray() + ]); + + $err = null; + try { + $token->validate('xxyyzz.aaa.bbccdd.123'); + } catch (\Exception $exception) { + $err = $exception; + } + + $this->assertInstanceOf(InvalidTokenException::class, $err, $message); + $this->assertEquals($message, $err->getMessage()); + } + } + + public function test_keys_can_be_a_local_file_reference_to_pem_key() + { + $file = tmpfile(); + $testFilePath = 'file://' . stream_get_meta_data($file)['uri']; + file_put_contents($testFilePath, $this->pemKey()); + $token = new OpenIdConnectIdToken($this->idToken(), 'https://auth.example.com', [ + $testFilePath + ]); + + $this->assertTrue($token->validate('xxyyzz.aaa.bbccdd.123')); + unlink($testFilePath); + } + + protected function getDefaultPayload(): array + { + return [ "sub" => "abc1234def", "name" => "Barry Scott", "email" => "bscott@example.com", @@ -36,24 +180,36 @@ class OpenIdConnectIdTokenTest extends TestCase "auth_time" => time(), "at_hash" => "sT4jbsdSGy9w12pq3iNYDA", ]; + } - $payload = array_merge($defaultPayload, $payloadOverrides); - $header = [ + protected function idToken($payloadOverrides = [], $headerOverrides = []): string + { + $payload = array_merge($this->getDefaultPayload(), $payloadOverrides); + $header = array_merge([ 'kid' => 'xyz456', 'alg' => 'RS256', - ]; + ], $headerOverrides); $top = implode('.', [ $this->base64UrlEncode(json_encode($header)), $this->base64UrlEncode(json_encode($payload)), ]); - $privateKey = RSA::loadPrivateKey($this->privatePemKey())->withPadding(RSA::SIGNATURE_PKCS1); + $privateKey = $this->getPrivateKey(); $signature = $privateKey->sign($top); - return $top . '.' . $this->base64UrlEncode($signature); } + protected function getPrivateKey() + { + static $key; + if (is_null($key)) { + $key = RSA::loadPrivateKey($this->privatePemKey())->withPadding(RSA::SIGNATURE_PKCS1); + } + + return $key; + } + protected function base64UrlEncode(string $decoded): string { return strtr(base64_encode($decoded), '+/', '-_');