OIDC Userinfo: Fixed issues with validation logic from changes

Also updated test to suit validation changes

diff --git a/app/Access/Oidc/OidcIdToken.php b/app/Access/Oidc/OidcIdToken.php
index f3fad0e1eeb97d431149b68682d992da774fa280..68a8aa611689e0a8bfc3ab9eb21f514f7a215b18 100644 (file)
--- 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 393ac5f0e257035010216404a17b297b44473542..06c04d81eb71178c63654dde9b7d85d3ee06cec4 100644 (file)
--- 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 6bb326e4bdc24bcf6434ec678625fc987a5f1f0c..7c1760649b5bb5bfc8600cb38fe56fe98282df18 100644 (file)
--- 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 0026d2f0aedf569a03eefc28898126cbcfa0f722..9aded654e31b7ee7ce754556e5272f0f7b9aaf52 100644 (file)
--- 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 6302f84c75f93be10f51f74a69dfb4758f0f3ce7..739323266942be0f4e8d7e3473699a6ddb42cd19 100644 (file)
--- 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