Validate state before ID Token request

CHANGELOG.md

@@ -7,19 +7,28 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ## [unreleased]
 
 ### Added
+- Add unit tests for verifyJWTClaims and different aud claims. #443
 - Support to change the `leeway` time for JWT verification using `setLeeway` #483
 
 ### Changed
+- Validate state before ID Token request. #447
 - Stop adding ?schema=openid to userinfo endpoint URL. #449
 
 ### Fixed
+- Fix protected responseContentType to allow overloading of fetchUrl function. #446
+- Fix TypeError in verifyJWTClaims. #442
 - Check existence of subject when verifying JWT #474
 - exp verification when verifying Logout Token claims #482
 
-## [1.0.1] - 2024-09-13
+## [1.0.2] - 2024-09-13
+
+### Added
+- Add unit test for SERVER_PORT type cast. #438
 
 ### Fixed
 - Cast `$_SERVER['SERVER_PORT']` to integer to prevent adding 80 or 443 port to redirect URL. #437
+- Fix protected $responseCode to allow proper overloading of fetchURL(). #433
+- Fix bring back #404. #437
 
 ## [1.0.1] - 2024-09-05
 

src/OpenIDConnectClient.php

@@ -306,7 +306,15 @@ public function authenticate(): bool
 
         // If we have an authorization code then proceed to request a token
         if (isset($_REQUEST['code'])) {
+            // Do an OpenID Connect session check
+            if (!isset($_REQUEST['state']) || ($_REQUEST['state'] !== $this->getState())) {
+                throw new OpenIDConnectClientException('Unable to determine state');
+            }
 
+            // Cleanup state
+            $this->unsetState();
+
+            // Request ID Token
             $code = $_REQUEST['code'];
             $token_json = $this->requestTokens($code);
 
@@ -318,14 +326,6 @@ public function authenticate(): bool
                 throw new OpenIDConnectClientException('Got response: ' . $token_json->error);
             }
 
-            // Do an OpenID Connect session check
-	    if (!isset($_REQUEST['state']) || ($_REQUEST['state'] !== $this->getState())) {
-                throw new OpenIDConnectClientException('Unable to determine state');
-            }
-
-            // Cleanup state
-            $this->unsetState();
-
             if (!property_exists($token_json, 'id_token')) {
                 throw new OpenIDConnectClientException('User did not authorize openid scope.');
             }
@@ -379,7 +379,7 @@ public function authenticate(): bool
             $accessToken = $_REQUEST['access_token'] ?? null;
 
             // Do an OpenID Connect session check
-    	    if (!isset($_REQUEST['state']) || ($_REQUEST['state'] !== $this->getState())) {
+            if (!isset($_REQUEST['state']) || ($_REQUEST['state'] !== $this->getState())) {
                 throw new OpenIDConnectClientException('Unable to determine state');
             }
 

tests/OpenIDConnectClientTest.php

@@ -157,6 +157,72 @@ public function testAuthenticateDoesNotThrowExceptionIfClaimsIsMissingNonce()
         }
     }
 
+    public function testAuthenticateWithCodeThrowsExceptionIfStateDoesNotMatch()
+    {
+        $_REQUEST['code'] = 'some-code';
+        $_REQUEST['state'] = "incorrect-state-from-user";
+        $_SESSION['openid_connect_state'] = "random-generated-state";
+
+        $client = new OpenIDConnectClient();
+
+        try {
+            $client->authenticate();
+        } catch ( OpenIDConnectClientException $e ) {
+            $this->assertEquals('Unable to determine state', $e->getMessage());
+            return;
+        }
+
+        $this->fail('OpenIDConnectClientException was not thrown when it should have been.');
+    }
+
+    public function testAuthenticateWithCodeMockedVerify()
+    {
+        $mockCode = 'some-code';
+        $mockState = 'some-code';
+
+        $_REQUEST['code'] = $mockCode;
+        $_REQUEST['state'] = $mockState;
+
+        $mockClaims = (object)['email' => '[email protected]'];
+        $mockIdToken = implode('.', [base64_encode('{}'), base64_encode(json_encode($mockClaims)), '']);
+        $mockAccessToken = 'some-access-token';
+        $mockRefreshToken = 'some-access-token';
+
+        $mockTokenResponse = (object)[
+            'id_token' => $mockIdToken,
+            'access_token' => $mockAccessToken,
+            'refresh_token' => $mockRefreshToken,
+        ];
+
+        $client = $this->getMockBuilder(OpenIDConnectClient::class)
+            ->setMethods(['requestTokens', 'verifySignatures', 'verifyJWTClaims', 'getState'])
+            ->getMock();
+        $client->method('getState')
+            ->willReturn($mockState);
+        $client->method('requestTokens')
+            ->with($mockCode)
+            ->willReturn($mockTokenResponse);
+        $client->method('verifySignatures')
+            ->with($mockIdToken);
+        $client->method('verifyJWTClaims')
+            ->with($mockClaims, $mockAccessToken)
+            ->willReturn(true);
+
+        try {
+            // In this mocked case we should be authenticated
+            // because we are not actually verifying the JWT
+            $authenticated = $client->authenticate();
+            $this->assertTrue($authenticated);
+            $this->assertEquals($mockIdToken, $client->getIdToken());
+            $this->assertEquals($mockAccessToken, $client->getAccessToken());
+            $this->assertEquals($mockTokenResponse, $client->getTokenResponse());
+            $this->assertEquals($mockClaims, $client->getVerifiedClaims());
+            $this->assertEquals($mockRefreshToken, $client->getRefreshToken());
+        } catch ( OpenIDConnectClientException $e ) {
+            $this->fail('OpenIDConnectClientException was thrown when it should not have been. Received exception: ' . $e->getMessage());
+        }
+    }
+
     public function testSerialize()
     {
         $client = new OpenIDConnectClient('https://example.com', 'foo', 'bar', 'baz');