OIDC Userinfo: Added userinfo data validation, seperated from id token

Wrapped userinfo response in its own class for additional handling and
validation.
Updated userdetails to take abstract claim data, to be populated by
either userinfo data or id token data.

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;
     }

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.
      */

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 [];

app/Access/Oidc/OidcUserinfoResponse.php

@@ -0,0 +1,54 @@
+<?php
+
+namespace BookStack\Access\Oidc;
+
+use Psr\Http\Message\ResponseInterface;
+
+class OidcUserinfoResponse implements ProvidesClaims
+{
+    protected array $claims = [];
+
+    public function __construct(ResponseInterface $response)
+    {
+        if ($response->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;
+    }
+}

app/Access/Oidc/ProvidesClaims.php

@@ -0,0 +1,17 @@
+<?php
+
+namespace BookStack\Access\Oidc;
+
+interface ProvidesClaims
+{
+    /**
+     * Fetch a specific claim.
+     * Returns null if it is null or does not exist.
+     */
+    public function getClaim(string $claim): mixed;
+
+    /**
+     * Get all contained claims.
+     */
+    public function getAllClaims(): array;
+}