diff --git a/app/Access/Oidc/OidcIdToken.php b/app/Access/Oidc/OidcIdToken.php index 5a395022a..b1da998a5 100644 --- a/app/Access/Oidc/OidcIdToken.php +++ b/app/Access/Oidc/OidcIdToken.php @@ -2,7 +2,7 @@ namespace BookStack\Access\Oidc; -class OidcIdToken +class OidcIdToken implements ProvidesClaims { protected array $header; protected array $payload; @@ -71,10 +71,8 @@ class OidcIdToken /** * Fetch a specific claim from this token. * Returns null if it is null or does not exist. - * - * @return mixed|null */ - public function getClaim(string $claim) + public function getClaim(string $claim): mixed { return $this->payload[$claim] ?? null; } diff --git a/app/Access/Oidc/OidcService.php b/app/Access/Oidc/OidcService.php index 5a73484c1..fba6dc9a8 100644 --- a/app/Access/Oidc/OidcService.php +++ b/app/Access/Oidc/OidcService.php @@ -194,35 +194,10 @@ class OidcService try { $idToken->validate($settings->clientId); } catch (OidcInvalidTokenException $exception) { - throw new OidcException("ID token validate failed with error: {$exception->getMessage()}"); - } - - $userDetails = OidcUserDetails::fromToken( - $idToken, - $this->config()['external_id_claim'], - $this->config()['display_name_claims'] ?? '', - $this->config()['groups_claim'] ?? '' - ); - - // TODO - This should not affect id token validation - if (!$userDetails->isFullyPopulated($this->shouldSyncGroups()) && !empty($settings->userinfoEndpoint)) { - $provider = $this->getProvider($settings); - $request = $provider->getAuthenticatedRequest('GET', $settings->userinfoEndpoint, $accessToken->getToken()); - $response = $provider->getParsedResponse($request); - // TODO - Ensure response content-type is "application/json" before using in this way (5.3.2) - // TODO - The sub Claim in the UserInfo Response MUST be verified to exactly match the sub Claim in the ID Token; if they do not match, the UserInfo Response values MUST NOT be used. (5.3.2) - // 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]. - $claims = $idToken->getAllClaims(); - foreach ($response as $key => $value) { - $claims[$key] = $value; - } - // TODO - Should maybe remain separate from IdToken completely - $idToken->replaceClaims($claims); + throw new OidcException("ID token validation failed with error: {$exception->getMessage()}"); } + $userDetails = $this->getUserDetailsFromToken($idToken, $accessToken, $settings); if (empty($userDetails->email)) { throw new OidcException(trans('errors.oidc_no_email_address')); } @@ -252,6 +227,41 @@ class OidcService return $user; } + /** + * @throws OidcException + */ + protected function getUserDetailsFromToken(OidcIdToken $idToken, OidcAccessToken $accessToken, OidcProviderSettings $settings): OidcUserDetails + { + $userDetails = new OidcUserDetails(); + $userDetails->populate( + $idToken, + $this->config()['external_id_claim'], + $this->config()['display_name_claims'] ?? '', + $this->config()['groups_claim'] ?? '' + ); + + if (!$userDetails->isFullyPopulated($this->shouldSyncGroups()) && !empty($settings->userinfoEndpoint)) { + $provider = $this->getProvider($settings); + $request = $provider->getAuthenticatedRequest('GET', $settings->userinfoEndpoint, $accessToken->getToken()); + $response = new OidcUserinfoResponse($provider->getResponse($request)); + + try { + $response->validate($idToken->getClaim('sub')); + } catch (OidcInvalidTokenException $exception) { + throw new OidcException("Userinfo endpoint response validation failed with error: {$exception->getMessage()}"); + } + + $userDetails->populate( + $response, + $this->config()['external_id_claim'], + $this->config()['display_name_claims'] ?? '', + $this->config()['groups_claim'] ?? '' + ); + } + + return $userDetails; + } + /** * Get the OIDC config from the application. */ diff --git a/app/Access/Oidc/OidcUserDetails.php b/app/Access/Oidc/OidcUserDetails.php index 1fb40ddc2..172bc9ceb 100644 --- a/app/Access/Oidc/OidcUserDetails.php +++ b/app/Access/Oidc/OidcUserDetails.php @@ -30,23 +30,19 @@ class OidcUserDetails /** * Populate user details from OidcIdToken data. */ - public static function fromToken( - OidcIdToken $token, + public function populate( + ProvidesClaims $claims, string $idClaim, string $displayNameClaims, string $groupsClaim, - ): static { - $id = $token->getClaim($idClaim); - - return new self( - externalId: $id, - email: $token->getClaim('email'), - name: static::getUserDisplayName($displayNameClaims, $token, $id), - groups: static::getUserGroups($groupsClaim, $token), - ); + ): void { + $this->externalId = $claims->getClaim($idClaim) ?? $this->externalId; + $this->email = $claims->getClaim('email') ?? $this->email; + $this->name = static::getUserDisplayName($displayNameClaims, $claims, $this->externalId) ?? $this->name; + $this->groups = static::getUserGroups($groupsClaim, $claims) ?? $this->groups; } - protected static function getUserDisplayName(string $displayNameClaims, OidcIdToken $token, string $defaultValue): string + protected static function getUserDisplayName(string $displayNameClaims, ProvidesClaims $token, string $defaultValue): string { $displayNameClaimParts = explode('|', $displayNameClaims); @@ -65,7 +61,7 @@ class OidcUserDetails return implode(' ', $displayName); } - protected static function getUserGroups(string $groupsClaim, OidcIdToken $token): array + protected static function getUserGroups(string $groupsClaim, ProvidesClaims $token): array { if (empty($groupsClaim)) { return []; diff --git a/app/Access/Oidc/OidcUserinfoResponse.php b/app/Access/Oidc/OidcUserinfoResponse.php new file mode 100644 index 000000000..7c7760434 --- /dev/null +++ b/app/Access/Oidc/OidcUserinfoResponse.php @@ -0,0 +1,54 @@ +getHeader('Content-Type')[0] === 'application/json') { + $this->claims = json_decode($response->getBody()->getContents(), true); + } + + // TODO - Support JWTs + // 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]. + } + + /** + * @throws OidcInvalidTokenException + */ + public function validate(string $idTokenSub): bool + { + $sub = $this->getClaim('sub'); + + // Spec: v1.0 5.3.2: The sub (subject) Claim MUST always be returned in the UserInfo Response. + if (!is_string($sub) || empty($sub)) { + throw new OidcInvalidTokenException("No valid subject value found in userinfo data"); + } + + // Spec: v1.0 5.3.2: The sub Claim in the UserInfo Response MUST be verified to exactly match the sub Claim in the ID Token; + // if they do not match, the UserInfo Response values MUST NOT be used. + if ($idTokenSub !== $sub) { + throw new OidcInvalidTokenException("Subject value provided in the userinfo endpoint does not match the provided ID token value"); + } + + return true; + } + + public function getClaim(string $claim): mixed + { + return $this->claims[$claim] ?? null; + } + + public function getAllClaims(): array + { + return $this->claims; + } +} diff --git a/app/Access/Oidc/ProvidesClaims.php b/app/Access/Oidc/ProvidesClaims.php new file mode 100644 index 000000000..a3cf51655 --- /dev/null +++ b/app/Access/Oidc/ProvidesClaims.php @@ -0,0 +1,17 @@ +