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" diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index c27126e2..38080af1 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" @@ -35,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/CHANGELOG.md b/CHANGELOG.md index 98ecaf7e..c6c34a9a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,10 +4,16 @@ 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] -- 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 +## [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 +- Fix JWT decode of non JWT tokens #428 +- Fix method signatures #427 - 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 diff --git a/composer.json b/composer.json index a0d5ee9e..ca9dc579 100644 --- a/composer.json +++ b/composer.json @@ -6,11 +6,12 @@ "php": ">=7.0", "ext-json": "*", "ext-curl": "*", - "phpseclib/phpseclib": "~3.0" + "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" }, "replace": { "jumbojett/openid-connect-php": "<=0.9.10" diff --git a/src/OpenIDConnectClient.php b/src/OpenIDConnectClient.php index 41315ced..46f2f496 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; @@ -145,12 +144,12 @@ class OpenIDConnectClient /** * @var int|null Response code from the server */ - private $responseCode; + protected $responseCode; /** * @var string|null Content type from the server */ - private $responseContentType; + protected $responseContentType; /** * @var array holds response types @@ -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,6 +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'])) { + # keep this case - even if some tool claim it is unnecessary $port = (int)$_SERVER['SERVER_PORT']; } elseif ($protocol === 'https') { $port = 443; @@ -1212,8 +1212,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))) @@ -1232,12 +1234,11 @@ 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|string|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); + return json_decode(base64url_decode($parts[$section] ?? ''), false); } /** @@ -1699,7 +1700,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; } @@ -1709,14 +1713,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; @@ -1731,17 +1735,30 @@ 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 + public function setIdToken(string $idToken) { + $this->idToken = $idToken; + } + + /** + * @return string|null + */ + public function getIdToken() { return $this->idToken; } @@ -1754,21 +1771,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 f895879c..4b46923d 100644 --- a/tests/OpenIDConnectClientTest.php +++ b/tests/OpenIDConnectClientTest.php @@ -7,9 +7,90 @@ class OpenIDConnectClientTest extends TestCase { - /** - * @return void - */ + 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(); + # 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() + { + $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(); @@ -18,7 +99,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()