From 8b14a701a4792ecc0f8dc500d78e5810597eb4ac Mon Sep 17 00:00:00 2001
From: Dan Brown <ssddanbrown@googlemail.com>
Date: Fri, 19 Apr 2024 16:43:51 +0100
Subject: [PATCH] OIDC Userinfo: Fixed issues with validation logic from
 changes

Also updated test to suit validation changes
---
 app/Access/Oidc/OidcIdToken.php          | 2 +-
 app/Access/Oidc/OidcJwtWithClaims.php    | 8 ++++----
 app/Access/Oidc/OidcService.php          | 2 +-
 app/Access/Oidc/OidcUserinfoResponse.php | 4 ++--
 tests/Unit/OidcIdTokenTest.php           | 2 +-
 5 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/app/Access/Oidc/OidcIdToken.php b/app/Access/Oidc/OidcIdToken.php
index f3fad0e1e..68a8aa611 100644
--- a/app/Access/Oidc/OidcIdToken.php
+++ b/app/Access/Oidc/OidcIdToken.php
@@ -11,7 +11,7 @@ class OidcIdToken extends OidcJwtWithClaims implements ProvidesClaims
      */
     public function validate(string $clientId): bool
     {
-        parent::validateCommonClaims();
+        parent::validateCommonTokenDetails($clientId);
         $this->validateTokenClaims($clientId);
 
         return true;
diff --git a/app/Access/Oidc/OidcJwtWithClaims.php b/app/Access/Oidc/OidcJwtWithClaims.php
index 393ac5f0e..06c04d81e 100644
--- a/app/Access/Oidc/OidcJwtWithClaims.php
+++ b/app/Access/Oidc/OidcJwtWithClaims.php
@@ -59,11 +59,11 @@ class OidcJwtWithClaims implements ProvidesClaims
      *
      * @throws OidcInvalidTokenException
      */
-    public function validateCommonTokenDetails(): bool
+    public function validateCommonTokenDetails(string $clientId): bool
     {
         $this->validateTokenStructure();
         $this->validateTokenSignature();
-        $this->validateCommonClaims();
+        $this->validateCommonClaims($clientId);
 
         return true;
     }
@@ -151,7 +151,7 @@ class OidcJwtWithClaims implements ProvidesClaims
      *
      * @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)
         // 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'];
-        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');
         }
     }
diff --git a/app/Access/Oidc/OidcService.php b/app/Access/Oidc/OidcService.php
index 6bb326e4b..7c1760649 100644
--- a/app/Access/Oidc/OidcService.php
+++ b/app/Access/Oidc/OidcService.php
@@ -253,7 +253,7 @@ class OidcService
             );
 
             try {
-                $response->validate($idToken->getClaim('sub'));
+                $response->validate($idToken->getClaim('sub'), $settings->clientId);
             } catch (OidcInvalidTokenException $exception) {
                 throw new OidcException("Userinfo endpoint response validation failed with error: {$exception->getMessage()}");
             }
diff --git a/app/Access/Oidc/OidcUserinfoResponse.php b/app/Access/Oidc/OidcUserinfoResponse.php
index 0026d2f0a..9aded654e 100644
--- a/app/Access/Oidc/OidcUserinfoResponse.php
+++ b/app/Access/Oidc/OidcUserinfoResponse.php
@@ -25,10 +25,10 @@ class OidcUserinfoResponse implements ProvidesClaims
     /**
      * @throws OidcInvalidTokenException
      */
-    public function validate(string $idTokenSub): bool
+    public function validate(string $idTokenSub, string $clientId): bool
     {
         if (!is_null($this->jwt)) {
-            $this->jwt->validateCommonTokenDetails();
+            $this->jwt->validateCommonTokenDetails($clientId);
         }
 
         $sub = $this->getClaim('sub');
diff --git a/tests/Unit/OidcIdTokenTest.php b/tests/Unit/OidcIdTokenTest.php
index 6302f84c7..739323266 100644
--- a/tests/Unit/OidcIdTokenTest.php
+++ b/tests/Unit/OidcIdTokenTest.php
@@ -113,7 +113,7 @@ class OidcIdTokenTest extends TestCase
             // 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']]],
+            ['Token audience value has 2 values, Expected 1', ['aud' => ['xxyyzz.aaa.bbccdd.123', '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