Skip to content

Commit 48d38b3

Browse files
committed
Merge branch 'master' into features/allow-client_secret_basic-on-request-token
* master: fix: protected responseContentType to allow overloading of fetchUrl function (jumbojett#446) test: unit tests for verifyJWTClaims and different aud claims (jumbojett#443) Fix TypeError in `verifyJWTClaims` (jumbojett#442) release: v1.0.2 (jumbojett#439) test: add unit test for SERVER_PORT type cast (jumbojett#438) fix: bring back jumbojett#404 (jumbojett#437) release: v1.0.1 (jumbojett#432) fix: protected $responseCode to allow proper overloading of fetchURL() (jumbojett#433) chore(deps-dev): update yoast/phpunit-polyfills requirement from ^1.0 to ^2.0 (jumbojett#430) chore(deps): update phpseclib/phpseclib requirement from ~3.0 to ^3.0.7 ci: run GitHub workflows on pull requests and pushes to master (jumbojett#431) chore: enable dependabot for composer (jumbojett#429) fix: handle JWT decode of non JWT tokens (jumbojett#428) fix: method signatures after 1.0 release (jumbojett#427)
2 parents c9ee737 + 9b64e0d commit 48d38b3

File tree

6 files changed

+151
-30
lines changed

6 files changed

+151
-30
lines changed

.github/dependabot.yml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,3 +11,9 @@ updates:
1111
directory: "/"
1212
schedule:
1313
interval: "weekly"
14+
15+
# Maintain dependencies for composer
16+
- package-ecosystem: "composer"
17+
directory: "/"
18+
schedule:
19+
interval: "weekly"

.github/workflows/build.yml

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,13 @@
11
---
22
name: build
33

4-
on: [push, pull_request]
4+
on:
5+
push:
6+
branches:
7+
- master
8+
pull_request:
9+
branches:
10+
- master
511

612
env:
713
DEFAULT_COMPOSER_FLAGS: "--prefer-dist --no-interaction --no-progress --optimize-autoloader --ansi"
@@ -35,4 +41,4 @@ jobs:
3541
- name: Install dependencies
3642
run: composer update $DEFAULT_COMPOSER_FLAGS
3743
- name: Run unit tests
38-
run: vendor/bin/phpunit --verbose --colors=always tests
44+
run: vendor/bin/phpunit --colors=always tests

CHANGELOG.md

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,16 @@ All notable changes to this project will be documented in this file.
44
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
55
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
66

7-
[unreleased]
8-
- Updated CI to also test on PHP 8.3 #407
9-
- Updated readme PHP requirement to PHP 7.0+ #407
10-
- Added dependabot for GitHub Actions #407
7+
## [1.0.1] - 2024-09-13
8+
9+
### Fixed
10+
- Cast `$_SERVER['SERVER_PORT']` to integer to prevent adding 80 or 443 port to redirect URL. #437
11+
12+
## [1.0.1] - 2024-09-05
13+
14+
### Fixed
15+
- Fix JWT decode of non JWT tokens #428
16+
- Fix method signatures #427
1117
- Cast `$_SERVER['SERVER_PORT']` to integer to prevent adding 80 or 443 port to redirect URL. #403
1218
- Check subject when verifying JWT #406
1319
- Removed duplicate check on jwks_uri and only check if jwks_uri exists when needed #373

composer.json

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,12 @@
66
"php": ">=7.0",
77
"ext-json": "*",
88
"ext-curl": "*",
9-
"phpseclib/phpseclib": "~3.0"
9+
"phpseclib/phpseclib": "^3.0.7"
1010
},
1111
"require-dev": {
12+
"phpunit/phpunit": "<10",
1213
"roave/security-advisories": "dev-latest",
13-
"yoast/phpunit-polyfills": "^1.0"
14+
"yoast/phpunit-polyfills": "^2.0"
1415
},
1516
"replace": {
1617
"jumbojett/openid-connect-php": "<=0.9.10"

src/OpenIDConnectClient.php

Lines changed: 36 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
*
44
* Copyright MITRE 2020
55
*
6-
* OpenIDConnectClient for PHP5
6+
* OpenIDConnectClient for PHP7+
77
* Author: Michael Jett <[email protected]>
88
*
99
* Licensed under the Apache License, Version 2.0 (the "License"); you may
@@ -25,7 +25,6 @@
2525

2626
use Error;
2727
use Exception;
28-
use phpseclib3\Crypt\PublicKeyLoader;
2928
use phpseclib3\Crypt\RSA;
3029
use phpseclib3\Math\BigInteger;
3130
use stdClass;
@@ -145,12 +144,12 @@ class OpenIDConnectClient
145144
/**
146145
* @var int|null Response code from the server
147146
*/
148-
private $responseCode;
147+
protected $responseCode;
149148

150149
/**
151150
* @var string|null Content type from the server
152151
*/
153-
private $responseContentType;
152+
protected $responseContentType;
154153

155154
/**
156155
* @var array holds response types
@@ -380,7 +379,7 @@ public function authenticate(): bool
380379
$accessToken = $_REQUEST['access_token'] ?? null;
381380

382381
// Do an OpenID Connect session check
383-
if (!isset($_REQUEST['state']) || ($_REQUEST['state'] !== $this->getState())) {
382+
if (!isset($_REQUEST['state']) || ($_REQUEST['state'] !== $this->getState())) {
384383
throw new OpenIDConnectClientException('Unable to determine state');
385384
}
386385

@@ -691,6 +690,7 @@ public function getRedirectURL(): string
691690
if (isset($_SERVER['HTTP_X_FORWARDED_PORT'])) {
692691
$port = (int)$_SERVER['HTTP_X_FORWARDED_PORT'];
693692
} elseif (isset($_SERVER['SERVER_PORT'])) {
693+
# keep this case - even if some tool claim it is unnecessary
694694
$port = (int)$_SERVER['SERVER_PORT'];
695695
} elseif ($protocol === 'https') {
696696
$port = 443;
@@ -1212,8 +1212,10 @@ protected function verifyJWTClaims($claims, string $accessToken = null): bool
12121212
$len = ((int)$bit)/16;
12131213
$expected_at_hash = $this->urlEncode(substr(hash('sha'.$bit, $accessToken, true), 0, $len));
12141214
}
1215+
$auds = $claims->aud;
1216+
$auds = is_array( $auds ) ? $auds : [ $auds ];
12151217
return (($this->validateIssuer($claims->iss))
1216-
&& (($claims->aud === $this->clientID) || in_array($this->clientID, $claims->aud, true))
1218+
&& (in_array($this->clientID, $auds, true))
12171219
&& ($claims->sub === $this->getIdTokenPayload()->sub)
12181220
&& (!isset($claims->nonce) || $claims->nonce === $this->getNonce())
12191221
&& ( !isset($claims->exp) || ((is_int($claims->exp)) && ($claims->exp >= time() - $this->leeway)))
@@ -1232,12 +1234,11 @@ protected function urlEncode(string $str): string
12321234
/**
12331235
* @param string $jwt encoded JWT
12341236
* @param int $section the section we would like to decode
1235-
* @return object
1237+
* @return object|string|null
12361238
*/
1237-
protected function decodeJWT(string $jwt, int $section = 0): stdClass {
1238-
1239+
protected function decodeJWT(string $jwt, int $section = 0) {
12391240
$parts = explode('.', $jwt);
1240-
return json_decode(base64url_decode($parts[$section]), false);
1241+
return json_decode(base64url_decode($parts[$section] ?? ''), false);
12411242
}
12421243

12431244
/**
@@ -1699,7 +1700,10 @@ public function revokeToken(string $token, string $token_type_hint = '', string
16991700
return json_decode($this->fetchURL($revocation_endpoint, $post_params, $headers), false);
17001701
}
17011702

1702-
public function getClientName(): string
1703+
/**
1704+
* @return string|null
1705+
*/
1706+
public function getClientName()
17031707
{
17041708
return $this->clientName;
17051709
}
@@ -1709,14 +1713,14 @@ public function setClientName(string $clientName) {
17091713
}
17101714

17111715
/**
1712-
* @return string
1716+
* @return string|null
17131717
*/
17141718
public function getClientID() {
17151719
return $this->clientID;
17161720
}
17171721

17181722
/**
1719-
* @return string
1723+
* @return string|null
17201724
*/
17211725
public function getClientSecret() {
17221726
return $this->clientSecret;
@@ -1731,17 +1735,30 @@ public function setAccessToken(string $accessToken) {
17311735
$this->accessToken = $accessToken;
17321736
}
17331737

1734-
public function getAccessToken(): string
1738+
/**
1739+
* @return string|null
1740+
*/
1741+
public function getAccessToken()
17351742
{
17361743
return $this->accessToken;
17371744
}
17381745

1739-
public function getRefreshToken(): string
1746+
/**
1747+
* @return string|null
1748+
*/
1749+
public function getRefreshToken()
17401750
{
17411751
return $this->refreshToken;
17421752
}
17431753

1744-
public function getIdToken(): string
1754+
public function setIdToken(string $idToken) {
1755+
$this->idToken = $idToken;
1756+
}
1757+
1758+
/**
1759+
* @return string|null
1760+
*/
1761+
public function getIdToken()
17451762
{
17461763
return $this->idToken;
17471764
}
@@ -1754,21 +1771,21 @@ public function getAccessTokenHeader() {
17541771
}
17551772

17561773
/**
1757-
* @return object
1774+
* @return object|string|null
17581775
*/
17591776
public function getAccessTokenPayload() {
17601777
return $this->decodeJWT($this->accessToken, 1);
17611778
}
17621779

17631780
/**
1764-
* @return object
1781+
* @return object|string|null
17651782
*/
17661783
public function getIdTokenHeader() {
17671784
return $this->decodeJWT($this->idToken);
17681785
}
17691786

17701787
/**
1771-
* @return object
1788+
* @return object|string|null
17721789
*/
17731790
public function getIdTokenPayload() {
17741791
return $this->decodeJWT($this->idToken, 1);

tests/OpenIDConnectClientTest.php

Lines changed: 88 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,90 @@
77

88
class OpenIDConnectClientTest extends TestCase
99
{
10-
/**
11-
* @return void
12-
*/
10+
public function testValidateClaims()
11+
{
12+
$client = new class extends OpenIDConnectClient {
13+
public function testVerifyJWTClaims($claims): bool
14+
{
15+
return $this->verifyJWTClaims($claims);
16+
}
17+
public function getIdTokenPayload()
18+
{
19+
return (object)[
20+
'sub' => 'sub'
21+
];
22+
}
23+
};
24+
$client->setClientID('client-id');
25+
$client->setIssuer('issuer');
26+
$client->setIdToken('');
27+
28+
# simple aud
29+
$valid = $client->testVerifyJWTClaims((object)[
30+
'aud' => 'client-id',
31+
'iss' => 'issuer',
32+
'sub' => 'sub',
33+
]);
34+
self::assertTrue($valid);
35+
36+
# array aud
37+
$valid = $client->testVerifyJWTClaims((object)[
38+
'aud' => ['client-id'],
39+
'iss' => 'issuer',
40+
'sub' => 'sub',
41+
]);
42+
self::assertTrue($valid);
43+
44+
# aud not matching
45+
$valid = $client->testVerifyJWTClaims((object)[
46+
'aud' => ['ipsum'],
47+
'iss' => 'issuer',
48+
'sub' => 'sub',
49+
]);
50+
self::assertFalse($valid);
51+
}
52+
public function testJWTDecode()
53+
{
54+
$client = new OpenIDConnectClient();
55+
# access token
56+
$client->setAccessToken('');
57+
$header = $client->getAccessTokenHeader();
58+
self::assertEquals('', $header);
59+
$payload = $client->getAccessTokenPayload();
60+
self::assertEquals('', $payload);
61+
62+
# id token
63+
$client->setIdToken('');
64+
$header = $client->getIdTokenHeader();
65+
self::assertEquals('', $header);
66+
$payload = $client->getIdTokenPayload();
67+
self::assertEquals('', $payload);
68+
}
69+
70+
public function testGetNull()
71+
{
72+
$client = new OpenIDConnectClient();
73+
self::assertNull($client->getAccessToken());
74+
self::assertNull($client->getRefreshToken());
75+
self::assertNull($client->getIdToken());
76+
self::assertNull($client->getClientName());
77+
self::assertNull($client->getClientID());
78+
self::assertNull($client->getClientSecret());
79+
self::assertNull($client->getCertPath());
80+
}
81+
82+
public function testResponseTypes()
83+
{
84+
$client = new OpenIDConnectClient();
85+
self::assertEquals([], $client->getResponseTypes());
86+
87+
$client->setResponseTypes('foo');
88+
self::assertEquals(['foo'], $client->getResponseTypes());
89+
90+
$client->setResponseTypes(['bar', 'ipsum']);
91+
self::assertEquals(['foo', 'bar', 'ipsum'], $client->getResponseTypes());
92+
}
93+
1394
public function testGetRedirectURL()
1495
{
1596
$client = new OpenIDConnectClient();
@@ -18,7 +99,11 @@ public function testGetRedirectURL()
1899

19100
$_SERVER['SERVER_NAME'] = 'domain.test';
20101
$_SERVER['REQUEST_URI'] = '/path/index.php?foo=bar&baz#fragment';
102+
$_SERVER['SERVER_PORT'] = '443';
21103
self::assertSame('http://domain.test/path/index.php', $client->getRedirectURL());
104+
105+
$_SERVER['SERVER_PORT'] = '8888';
106+
self::assertSame('http://domain.test:8888/path/index.php', $client->getRedirectURL());
22107
}
23108

24109
public function testAuthenticateDoesNotThrowExceptionIfClaimsIsMissingNonce()

0 commit comments

Comments
 (0)