From 1e854438f32075f0670b7e48c9ab3587c9913a70 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= <1005065+DeepDiver1975@users.noreply.github.com> Date: Thu, 5 Sep 2024 13:53:08 +0200 Subject: [PATCH 01/14] fix: method signatures after 1.0 release (#427) --- src/OpenIDConnectClient.php | 36 ++++++++++++++++++++----------- tests/OpenIDConnectClientTest.php | 35 +++++++++++++++++++++++++++--- 2 files changed, 55 insertions(+), 16 deletions(-) diff --git a/src/OpenIDConnectClient.php b/src/OpenIDConnectClient.php index adabf80c..60f89bd8 100644 --- a/src/OpenIDConnectClient.php +++ b/src/OpenIDConnectClient.php @@ -3,7 +3,7 @@ * * Copyright MITRE 2020 * - * OpenIDConnectClient for PHP5 + * OpenIDConnectClient for PHP7+ * Author: Michael Jett * * Licensed under the Apache License, Version 2.0 (the "License"); you may @@ -25,7 +25,6 @@ use Error; use Exception; -use phpseclib3\Crypt\PublicKeyLoader; use phpseclib3\Crypt\RSA; use phpseclib3\Math\BigInteger; use stdClass; @@ -380,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'); } @@ -691,7 +690,7 @@ public function getRedirectURL(): string if (isset($_SERVER['HTTP_X_FORWARDED_PORT'])) { $port = (int)$_SERVER['HTTP_X_FORWARDED_PORT']; } elseif (isset($_SERVER['SERVER_PORT'])) { - $port = (int)$_SERVER['SERVER_PORT']; + $port = $_SERVER['SERVER_PORT']; } elseif ($protocol === 'https') { $port = 443; } else { @@ -1221,10 +1220,9 @@ protected function urlEncode(string $str): string /** * @param string $jwt encoded JWT * @param int $section the section we would like to decode - * @return object + * @return object|null */ - protected function decodeJWT(string $jwt, int $section = 0): stdClass { - + protected function decodeJWT(string $jwt, int $section = 0) { $parts = explode('.', $jwt); return json_decode(base64url_decode($parts[$section]), false); } @@ -1688,7 +1686,10 @@ public function revokeToken(string $token, string $token_type_hint = '', string return json_decode($this->fetchURL($revocation_endpoint, $post_params, $headers), false); } - public function getClientName(): string + /** + * @return string|null + */ + public function getClientName() { return $this->clientName; } @@ -1698,14 +1699,14 @@ public function setClientName(string $clientName) { } /** - * @return string + * @return string|null */ public function getClientID() { return $this->clientID; } /** - * @return string + * @return string|null */ public function getClientSecret() { return $this->clientSecret; @@ -1720,17 +1721,26 @@ public function setAccessToken(string $accessToken) { $this->accessToken = $accessToken; } - public function getAccessToken(): string + /** + * @return string|null + */ + public function getAccessToken() { return $this->accessToken; } - public function getRefreshToken(): string + /** + * @return string|null + */ + public function getRefreshToken() { return $this->refreshToken; } - public function getIdToken(): string + /** + * @return string|null + */ + public function getIdToken() { return $this->idToken; } diff --git a/tests/OpenIDConnectClientTest.php b/tests/OpenIDConnectClientTest.php index f895879c..88d98989 100644 --- a/tests/OpenIDConnectClientTest.php +++ b/tests/OpenIDConnectClientTest.php @@ -7,9 +7,38 @@ class OpenIDConnectClientTest extends TestCase { - /** - * @return void - */ + public function testJWTDecode() + { + $client = new OpenIDConnectClient(); + $client->setAccessToken(''); + $header = $client->getAccessTokenHeader(); + self::assertEquals('', $header); + } + + public function testGetNull() + { + $client = new OpenIDConnectClient(); + self::assertNull($client->getAccessToken()); + self::assertNull($client->getRefreshToken()); + self::assertNull($client->getIdToken()); + self::assertNull($client->getClientName()); + self::assertNull($client->getClientID()); + self::assertNull($client->getClientSecret()); + self::assertNull($client->getCertPath()); + } + + public function testResponseTypes() + { + $client = new OpenIDConnectClient(); + self::assertEquals([], $client->getResponseTypes()); + + $client->setResponseTypes('foo'); + self::assertEquals(['foo'], $client->getResponseTypes()); + + $client->setResponseTypes(['bar', 'ipsum']); + self::assertEquals(['foo', 'bar', 'ipsum'], $client->getResponseTypes()); + } + public function testGetRedirectURL() { $client = new OpenIDConnectClient(); From 0509be83cae7984e011ad3ee26f2dae4af67edf2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= <1005065+DeepDiver1975@users.noreply.github.com> Date: Thu, 5 Sep 2024 16:29:25 +0200 Subject: [PATCH 02/14] fix: handle JWT decode of non JWT tokens (#428) --- CHANGELOG.md | 2 ++ src/OpenIDConnectClient.php | 14 +++++++++----- tests/OpenIDConnectClientTest.php | 11 +++++++++++ 3 files changed, 22 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4055711f..fa7fddf6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). [unreleased] +- Fix JWT decode of non JWT tokens #428 +- Fix method signatures #427 - Updated CI to also test on PHP 8.3 #407 - Updated readme PHP requirement to PHP 7.0+ #407 - Added dependabot for GitHub Actions #407 diff --git a/src/OpenIDConnectClient.php b/src/OpenIDConnectClient.php index 60f89bd8..16c3d656 100644 --- a/src/OpenIDConnectClient.php +++ b/src/OpenIDConnectClient.php @@ -1220,11 +1220,11 @@ protected function urlEncode(string $str): string /** * @param string $jwt encoded JWT * @param int $section the section we would like to decode - * @return object|null + * @return object|string|null */ protected function decodeJWT(string $jwt, int $section = 0) { $parts = explode('.', $jwt); - return json_decode(base64url_decode($parts[$section]), false); + return json_decode(base64url_decode($parts[$section] ?? ''), false); } /** @@ -1737,6 +1737,10 @@ public function getRefreshToken() return $this->refreshToken; } + public function setIdToken(string $idToken) { + $this->idToken = $idToken; + } + /** * @return string|null */ @@ -1753,21 +1757,21 @@ public function getAccessTokenHeader() { } /** - * @return object + * @return object|string|null */ public function getAccessTokenPayload() { return $this->decodeJWT($this->accessToken, 1); } /** - * @return object + * @return object|string|null */ public function getIdTokenHeader() { return $this->decodeJWT($this->idToken); } /** - * @return object + * @return object|string|null */ public function getIdTokenPayload() { return $this->decodeJWT($this->idToken, 1); diff --git a/tests/OpenIDConnectClientTest.php b/tests/OpenIDConnectClientTest.php index 88d98989..3dc4709f 100644 --- a/tests/OpenIDConnectClientTest.php +++ b/tests/OpenIDConnectClientTest.php @@ -10,9 +10,20 @@ class OpenIDConnectClientTest extends TestCase public function testJWTDecode() { $client = new OpenIDConnectClient(); + # access token $client->setAccessToken(''); $header = $client->getAccessTokenHeader(); self::assertEquals('', $header); + $payload = $client->getAccessTokenPayload(); + self::assertEquals('', $payload); + + # id token + $client->setIdToken(''); + $header = $client->getIdTokenHeader(); + self::assertEquals('', $header); + $payload = $client->getIdTokenPayload(); + self::assertEquals('', $payload); + } public function testGetNull() From 0fbf8f2533b4529c9fd14b1bf3cb2d29cb1ef061 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= <1005065+DeepDiver1975@users.noreply.github.com> Date: Thu, 5 Sep 2024 16:33:44 +0200 Subject: [PATCH 03/14] chore: enable dependabot for composer (#429) --- .github/dependabot.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.github/dependabot.yml b/.github/dependabot.yml index 0f11f4a3..a54473b4 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -11,3 +11,9 @@ updates: directory: "/" schedule: interval: "weekly" + + # Maintain dependencies for composer + - package-ecosystem: "composer" + directory: "/" + schedule: + interval: "weekly" From 036530bb69c32849478a6bde2fe6b4c89e60b3b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= <1005065+DeepDiver1975@users.noreply.github.com> Date: Thu, 5 Sep 2024 16:39:11 +0200 Subject: [PATCH 04/14] ci: run GitHub workflows on pull requests and pushes to master (#431) --- .github/workflows/build.yml | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index c27126e2..31d0ed11 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -1,7 +1,13 @@ --- name: build -on: [push, pull_request] +on: + push: + branches: + - master + pull_request: + branches: + - master env: DEFAULT_COMPOSER_FLAGS: "--prefer-dist --no-interaction --no-progress --optimize-autoloader --ansi" From e316397c876a602ff87d62bee2adb96cdb497f9a Mon Sep 17 00:00:00 2001 From: Artem Boyko Date: Thu, 5 Sep 2024 17:43:09 +0300 Subject: [PATCH 05/14] chore(deps): update phpseclib/phpseclib requirement from ~3.0 to ^3.0.7 * Update phpseclib/phpseclib to minimum 2.0.31 or 3.0.7 * Update composer.json --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 3fa6d231..3cd6fc38 100644 --- a/composer.json +++ b/composer.json @@ -6,7 +6,7 @@ "php": ">=7.0", "ext-json": "*", "ext-curl": "*", - "phpseclib/phpseclib": "~3.0" + "phpseclib/phpseclib": "^3.0.7" }, "require-dev": { "roave/security-advisories": "dev-latest", From 22560300d349b32c17e073198ec3b2f6dd2c0230 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 5 Sep 2024 16:55:24 +0200 Subject: [PATCH 06/14] chore(deps-dev): update yoast/phpunit-polyfills requirement from ^1.0 to ^2.0 (#430) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * chore(deps-dev): update yoast/phpunit-polyfills requirement Updates the requirements on [yoast/phpunit-polyfills](https://github.com/Yoast/PHPUnit-Polyfills) to permit the latest version. - [Release notes](https://github.com/Yoast/PHPUnit-Polyfills/releases) - [Changelog](https://github.com/Yoast/PHPUnit-Polyfills/blob/2.x/CHANGELOG.md) - [Commits](https://github.com/Yoast/PHPUnit-Polyfills/compare/1.0.0...2.0.1) --- updated-dependencies: - dependency-name: yoast/phpunit-polyfills dependency-type: direct:development ... Signed-off-by: dependabot[bot] * fix: remove --verbose from phpunit * fix: force usage of phpunit < 10 --------- Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Thomas Müller <1005065+DeepDiver1975@users.noreply.github.com> --- .github/workflows/build.yml | 2 +- composer.json | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 31d0ed11..38080af1 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -41,4 +41,4 @@ jobs: - name: Install dependencies run: composer update $DEFAULT_COMPOSER_FLAGS - name: Run unit tests - run: vendor/bin/phpunit --verbose --colors=always tests + run: vendor/bin/phpunit --colors=always tests diff --git a/composer.json b/composer.json index 3cd6fc38..64825884 100644 --- a/composer.json +++ b/composer.json @@ -9,8 +9,9 @@ "phpseclib/phpseclib": "^3.0.7" }, "require-dev": { + "phpunit/phpunit": "<10", "roave/security-advisories": "dev-latest", - "yoast/phpunit-polyfills": "^1.0" + "yoast/phpunit-polyfills": "^2.0" }, "archive" : { "exclude" : [ From 765ddbd65643d69edce461617ea3c620d582714e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= <1005065+DeepDiver1975@users.noreply.github.com> Date: Fri, 6 Sep 2024 08:16:24 +0200 Subject: [PATCH 07/14] fix: protected $responseCode to allow proper overloading of fetchURL() (#433) --- src/OpenIDConnectClient.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OpenIDConnectClient.php b/src/OpenIDConnectClient.php index 16c3d656..e12ad9c6 100644 --- a/src/OpenIDConnectClient.php +++ b/src/OpenIDConnectClient.php @@ -144,7 +144,7 @@ class OpenIDConnectClient /** * @var int|null Response code from the server */ - private $responseCode; + protected $responseCode; /** * @var string|null Content type from the server From 75693117e99c209b9aef70fc7b279fe561304cb2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= <1005065+DeepDiver1975@users.noreply.github.com> Date: Fri, 6 Sep 2024 08:22:32 +0200 Subject: [PATCH 08/14] release: v1.0.1 (#432) --- CHANGELOG.md | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fa7fddf6..1b39eead 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,12 +4,11 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). -[unreleased] +## [1.0.1] - 2024-09-05 + +### Fixed - Fix JWT decode of non JWT tokens #428 - Fix method signatures #427 -- Updated CI to also test on PHP 8.3 #407 -- Updated readme PHP requirement to PHP 7.0+ #407 -- Added dependabot for GitHub Actions #407 - Cast `$_SERVER['SERVER_PORT']` to integer to prevent adding 80 or 443 port to redirect URL. #403 - Check subject when verifying JWT #406 - Removed duplicate check on jwks_uri and only check if jwks_uri exists when needed #373 From db1ed8b5f1664db3af99feba114de34542a10a1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= <1005065+DeepDiver1975@users.noreply.github.com> Date: Fri, 13 Sep 2024 08:52:32 +0200 Subject: [PATCH 09/14] fix: bring back #404 (#437) --- src/OpenIDConnectClient.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/OpenIDConnectClient.php b/src/OpenIDConnectClient.php index e12ad9c6..e3f9d3f2 100644 --- a/src/OpenIDConnectClient.php +++ b/src/OpenIDConnectClient.php @@ -690,7 +690,8 @@ public function getRedirectURL(): string if (isset($_SERVER['HTTP_X_FORWARDED_PORT'])) { $port = (int)$_SERVER['HTTP_X_FORWARDED_PORT']; } elseif (isset($_SERVER['SERVER_PORT'])) { - $port = $_SERVER['SERVER_PORT']; + # keep this case - even if some tool claim it is unnecessary + $port = (int)$_SERVER['SERVER_PORT']; } elseif ($protocol === 'https') { $port = 443; } else { From a5994e793a72b19747da880b6428b55638a3ebcc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= <1005065+DeepDiver1975@users.noreply.github.com> Date: Fri, 13 Sep 2024 09:07:45 +0200 Subject: [PATCH 10/14] test: add unit test for SERVER_PORT type cast (#438) --- tests/OpenIDConnectClientTest.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/OpenIDConnectClientTest.php b/tests/OpenIDConnectClientTest.php index 3dc4709f..45adc7b3 100644 --- a/tests/OpenIDConnectClientTest.php +++ b/tests/OpenIDConnectClientTest.php @@ -58,7 +58,11 @@ public function testGetRedirectURL() $_SERVER['SERVER_NAME'] = 'domain.test'; $_SERVER['REQUEST_URI'] = '/path/index.php?foo=bar&baz#fragment'; + $_SERVER['SERVER_PORT'] = '443'; self::assertSame('http://domain.test/path/index.php', $client->getRedirectURL()); + + $_SERVER['SERVER_PORT'] = '8888'; + self::assertSame('http://domain.test:8888/path/index.php', $client->getRedirectURL()); } public function testAuthenticateDoesNotThrowExceptionIfClaimsIsMissingNonce() From 9af21bd04f5b564bdd18993a2b15d88b7594f152 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= <1005065+DeepDiver1975@users.noreply.github.com> Date: Fri, 13 Sep 2024 09:09:33 +0200 Subject: [PATCH 11/14] release: v1.0.2 (#439) --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1b39eead..0e1c7105 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,11 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [1.0.1] - 2024-09-13 + +### Fixed +- Cast `$_SERVER['SERVER_PORT']` to integer to prevent adding 80 or 443 port to redirect URL. #437 + ## [1.0.1] - 2024-09-05 ### Fixed From 60919af91a29b61d728d8b5a02d8af84271d0eaa Mon Sep 17 00:00:00 2001 From: Robert Vogel <1201528+osnard@users.noreply.github.com> Date: Tue, 17 Sep 2024 17:48:09 +0200 Subject: [PATCH 12/14] Fix TypeError in `verifyJWTClaims` (#442) ... when ClientID does not match Co-authored-by: Robert Vogel --- src/OpenIDConnectClient.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/OpenIDConnectClient.php b/src/OpenIDConnectClient.php index e3f9d3f2..b38a81cd 100644 --- a/src/OpenIDConnectClient.php +++ b/src/OpenIDConnectClient.php @@ -1201,8 +1201,10 @@ protected function verifyJWTClaims($claims, string $accessToken = null): bool $len = ((int)$bit)/16; $expected_at_hash = $this->urlEncode(substr(hash('sha'.$bit, $accessToken, true), 0, $len)); } + $auds = $claims->aud; + $auds = is_array( $auds ) ? $auds : [ $auds ]; return (($this->validateIssuer($claims->iss)) - && (($claims->aud === $this->clientID) || in_array($this->clientID, $claims->aud, true)) + && (in_array($this->clientID, $auds, true)) && ($claims->sub === $this->getIdTokenPayload()->sub) && (!isset($claims->nonce) || $claims->nonce === $this->getNonce()) && ( !isset($claims->exp) || ((is_int($claims->exp)) && ($claims->exp >= time() - $this->leeway))) From 97adbcee4b519700ad14abc9f73962077a4c6b80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= <1005065+DeepDiver1975@users.noreply.github.com> Date: Wed, 18 Sep 2024 09:06:55 +0200 Subject: [PATCH 13/14] test: unit tests for verifyJWTClaims and different aud claims (#443) --- tests/OpenIDConnectClientTest.php | 43 ++++++++++++++++++++++++++++++- 1 file changed, 42 insertions(+), 1 deletion(-) diff --git a/tests/OpenIDConnectClientTest.php b/tests/OpenIDConnectClientTest.php index 45adc7b3..4b46923d 100644 --- a/tests/OpenIDConnectClientTest.php +++ b/tests/OpenIDConnectClientTest.php @@ -7,6 +7,48 @@ class OpenIDConnectClientTest extends TestCase { + public function testValidateClaims() + { + $client = new class extends OpenIDConnectClient { + public function testVerifyJWTClaims($claims): bool + { + return $this->verifyJWTClaims($claims); + } + public function getIdTokenPayload() + { + return (object)[ + 'sub' => 'sub' + ]; + } + }; + $client->setClientID('client-id'); + $client->setIssuer('issuer'); + $client->setIdToken(''); + + # simple aud + $valid = $client->testVerifyJWTClaims((object)[ + 'aud' => 'client-id', + 'iss' => 'issuer', + 'sub' => 'sub', + ]); + self::assertTrue($valid); + + # array aud + $valid = $client->testVerifyJWTClaims((object)[ + 'aud' => ['client-id'], + 'iss' => 'issuer', + 'sub' => 'sub', + ]); + self::assertTrue($valid); + + # aud not matching + $valid = $client->testVerifyJWTClaims((object)[ + 'aud' => ['ipsum'], + 'iss' => 'issuer', + 'sub' => 'sub', + ]); + self::assertFalse($valid); + } public function testJWTDecode() { $client = new OpenIDConnectClient(); @@ -23,7 +65,6 @@ public function testJWTDecode() self::assertEquals('', $header); $payload = $client->getIdTokenPayload(); self::assertEquals('', $payload); - } public function testGetNull() From f7c91b9079bf96323b5eb3ab4383f297f21d98d1 Mon Sep 17 00:00:00 2001 From: Rick Lambrechts Date: Fri, 27 Sep 2024 13:01:32 +0200 Subject: [PATCH 14/14] fix: protected responseContentType to allow overloading of fetchUrl function (#446) --- src/OpenIDConnectClient.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OpenIDConnectClient.php b/src/OpenIDConnectClient.php index b38a81cd..07c00540 100644 --- a/src/OpenIDConnectClient.php +++ b/src/OpenIDConnectClient.php @@ -149,7 +149,7 @@ class OpenIDConnectClient /** * @var string|null Content type from the server */ - private $responseContentType; + protected $responseContentType; /** * @var array holds response types