Infra-Mirrors/BookStack
1
0
Fork 0
mirror of https://github.com/BookStackApp/BookStack.git synced 2024-10-01 01:36:00 -04:00
Code Issues Packages Projects Releases Wiki Activity

OIDC Userinfo: Fixed issues with validation logic from changes

Browse Source 
Also updated test to suit validation changes
This commit is contained in:
Dan Brown 2024-04-19 16:43:51 +01:00
parent 0958909cd9
commit 8b14a701a4
No known key found for this signature in database
GPG Key ID: 46D9F943C24A2EF9
5 changed files with 9 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
app/Access/Oidc/OidcIdToken.php
View File

@ -11,7 +11,7 @@ class OidcIdToken extends OidcJwtWithClaims implements ProvidesClaims
*/ */
public function validate(string $clientId): bool public function validate(string $clientId): bool
{ {
parent::validateCommonClaims(); parent::validateCommonTokenDetails($clientId);
$this->validateTokenClaims($clientId); $this->validateTokenClaims($clientId);
return true; return true;

8
app/Access/Oidc/OidcJwtWithClaims.php
View File

@ -59,11 +59,11 @@ class OidcJwtWithClaims implements ProvidesClaims
* *
* @throws OidcInvalidTokenException * @throws OidcInvalidTokenException
*/ */
public function validateCommonTokenDetails(): bool public function validateCommonTokenDetails(string $clientId): bool
{ {
$this->validateTokenStructure(); $this->validateTokenStructure();
$this->validateTokenSignature(); $this->validateTokenSignature();
$this->validateCommonClaims(); $this->validateCommonClaims($clientId);
return true; return true;
} }
@ -151,7 +151,7 @@ class OidcJwtWithClaims implements ProvidesClaims
* *
* @throws OidcInvalidTokenException * @throws OidcInvalidTokenException
*/ */
protected function validateCommonClaims(): void protected function validateCommonClaims(string $clientId): void
{ {
// 1. The Issuer Identifier for the OpenID Provider (which is typically obtained during Discovery) // 1. The Issuer Identifier for the OpenID Provider (which is typically obtained during Discovery)
// MUST exactly match the value of the iss (issuer) Claim. // MUST exactly match the value of the iss (issuer) Claim.
@ -167,7 +167,7 @@ class OidcJwtWithClaims implements ProvidesClaims
} }
$aud = is_string($this->payload['aud']) ? [$this->payload['aud']] : $this->payload['aud']; $aud = is_string($this->payload['aud']) ? [$this->payload['aud']] : $this->payload['aud'];
if (!in_array($this->payload['aud'], $aud, true)) { if (!in_array($clientId, $aud, true)) {
throw new OidcInvalidTokenException('Token audience value did not match the expected client_id'); throw new OidcInvalidTokenException('Token audience value did not match the expected client_id');
} }
} }

2
app/Access/Oidc/OidcService.php
View File

@ -253,7 +253,7 @@ class OidcService
); );
try { try {
$response->validate($idToken->getClaim('sub')); $response->validate($idToken->getClaim('sub'), $settings->clientId);
} catch (OidcInvalidTokenException $exception) { } catch (OidcInvalidTokenException $exception) {
throw new OidcException("Userinfo endpoint response validation failed with error: {$exception->getMessage()}"); throw new OidcException("Userinfo endpoint response validation failed with error: {$exception->getMessage()}");
} }

4
app/Access/Oidc/OidcUserinfoResponse.php
View File

@ -25,10 +25,10 @@ class OidcUserinfoResponse implements ProvidesClaims
/** /**
* @throws OidcInvalidTokenException * @throws OidcInvalidTokenException
*/ */
public function validate(string $idTokenSub): bool public function validate(string $idTokenSub, string $clientId): bool
{ {
if (!is_null($this->jwt)) { if (!is_null($this->jwt)) {
$this->jwt->validateCommonTokenDetails(); $this->jwt->validateCommonTokenDetails($clientId);
} }
$sub = $this->getClaim('sub'); $sub = $this->getClaim('sub');

2
tests/Unit/OidcIdTokenTest.php
View File

@ -113,7 +113,7 @@ class OidcIdTokenTest extends TestCase
// 2. aud claim present // 2. aud claim present
['Missing token audience value', ['aud' => null]], ['Missing token audience value', ['aud' => null]],
// 2. aud claim validates all values against those expected (Only expect single) // 2. aud claim validates all values against those expected (Only expect single)
['Token audience value has 2 values, Expected 1', ['aud' => ['abc', 'def']]], ['Token audience value has 2 values, Expected 1', ['aud' => ['xxyyzz.aaa.bbccdd.123', 'def']]],
// 2. aud claim matches client id // 2. aud claim matches client id
['Token audience value did not match the expected client_id', ['aud' => 'xxyyzz.aaa.bbccdd.456']], ['Token audience value did not match the expected client_id', ['aud' => 'xxyyzz.aaa.bbccdd.456']],
// 4. azp claim matches client id if present // 4. azp claim matches client id if present