From 6515c73ab29e7abd045defae3e7f93b6c50809b9 Mon Sep 17 00:00:00 2001 From: turegjorup Date: Tue, 12 May 2026 12:22:07 +0200 Subject: [PATCH 1/2] Narrow validateIdToken claims and assorted test types MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three small narrowings to clear the remaining `mixed` flow that PHPStan flags at `level: max`: - `getJwtVerificationKeys()` declares `@return array` matching the shape it actually builds. The cache-hit branch's `(array) $item->get()` is annotated with the same shape via inline `@var` — we only ever store this structure, so the trust is consistent with the method's contract. - `validateIdToken()`'s `$claims` is annotated with a `\stdClass&object{aud: string|array, iss: string, nonce: string}` shape after `JWT::decode()`. The fields are required string-typed claims per the OIDC spec; `firebase/php-jwt` already enforces JWT validity before this point. No runtime change. - `testCreateResourceOwner` adds an `assertInstanceOf(...)` guard so the `$owner->getId()` call type-checks (the value comes back from `ReflectionMethod::invoke()` which PHPStan sees as `mixed`). - `loadMockFixture()`'s `@return` is relaxed from `array` to `array` — `json_decode($content, true)` doesn't statically guarantee string keys, and the callers cast/narrow as needed for their specific fixture. - 11 `$mockJWT = \Mockery::mock('overload:...')` sites in the test file get an inline `@var \Mockery\MockInterface` annotation. The `overload:` mock-syntax isn't recognised by `phpstan/phpstan-mockery` (overload mocks are special-cased to never instantiate the target class, so the extension's class-resolution doesn't fire); the annotation tells PHPStan the value is a `MockInterface` so the chained `shouldReceive()->...` calls type-check. After this PR rebases onto develop (post-PR #43 / PR A merge), all max-level errors are gone. The `phpstan.neon` `level: 8` → `level: max` bump is left as a follow-up one-line PR so the bump and its prerequisites land independently. Co-Authored-By: Claude Opus 4.7 (1M context) --- CHANGELOG.md | 12 ++++++++++++ src/Security/OpenIdConfigurationProvider.php | 6 ++++-- tests/Security/OpenIdConfigurationProviderTest.php | 14 +++++++++++++- 3 files changed, 29 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e405cdb..2e6e7fd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -75,6 +75,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 after the document type rather than the individual key is more accurate. Consumers catching the marker are unaffected; consumers catching the concrete class need to swap the name. +- `OpenIdConfigurationProvider::getJwtVerificationKeys` declares its + return type as `array` (was just `array`), matching + the actual shape the method builds. Lets `validateIdToken` pass + the cached keys to `JWT::decode` without a `mixed` flow at + `level: max`. +- `OpenIdConfigurationProvider::validateIdToken` narrows its + `$claims` local via inline `@var \stdClass&object{aud, iss, + nonce}` so the spec-required claim accesses + (`$claims->aud` / `$claims->iss` / `$claims->nonce`) type-check at + `level: max`. No runtime change — these values are guaranteed + present and string-typed by the OIDC spec and `firebase/php-jwt` + already enforces JWT validity. ### Documentation diff --git a/src/Security/OpenIdConfigurationProvider.php b/src/Security/OpenIdConfigurationProvider.php index 8e89bf6..b57ad06 100644 --- a/src/Security/OpenIdConfigurationProvider.php +++ b/src/Security/OpenIdConfigurationProvider.php @@ -222,6 +222,7 @@ public function validateIdToken(string $idToken, string $nonce): object // NB: JWT::$leeway is a static property shared across all instances. // Always set it immediately before decode to ensure the correct value. JWT::$leeway = $this->leeway; + /** @var \stdClass&object{aud: string|array, iss: string, nonce: string} $claims */ $claims = JWT::decode($idToken, $keys); // "aud" may be an array of strings or a single string // (cf. https://openid.net/specs/openid-connect-core-1_0.html#IDToken). @@ -356,8 +357,8 @@ protected function createResourceOwner(array $response, AccessToken $token): Res /** * Get JWT verification keys from Azure Active Directory. * - * @return array - * Array of keys + * @return array + * Array of keys indexed by JWK `kid` * * @throws OpenIdConnectExceptionInterface */ @@ -372,6 +373,7 @@ private function getJwtVerificationKeys(): array $item = $this->cacheItemPool->getItem($cacheKey); if ($item->isHit()) { + /** @var array $keys (we only ever store this shape) */ $keys = (array) $item->get(); } else { $keysUri = $this->getConfiguration('jwks_uri'); diff --git a/tests/Security/OpenIdConfigurationProviderTest.php b/tests/Security/OpenIdConfigurationProviderTest.php index d3be35f..30541fb 100644 --- a/tests/Security/OpenIdConfigurationProviderTest.php +++ b/tests/Security/OpenIdConfigurationProviderTest.php @@ -226,6 +226,7 @@ public function testGetBaseAccessTokenUrl(): void public function testValidateIdTokenSuccess(): void { + /** @var \Mockery\MockInterface $mockJWT */ $mockJWT = \Mockery::mock('overload:Firebase\JWT\JWT', MockJWT::class); $mockClaims = $this->getMockClaims(); @@ -249,6 +250,7 @@ public function testValidateIdTokenSuccess(): void public function testValidateIdTokenFailure(): void { + /** @var \Mockery\MockInterface $mockJWT */ $mockJWT = \Mockery::mock('overload:Firebase\JWT\JWT', MockJWT::class); $mockJWT->shouldReceive('decode')->andThrow(SignatureInvalidException::class, 'Signature verification failed'); @@ -260,6 +262,7 @@ public function testValidateIdTokenFailure(): void public function testValidateIdTokenAudience(): void { + /** @var \Mockery\MockInterface $mockJWT */ $mockJWT = \Mockery::mock('overload:Firebase\JWT\JWT', MockJWT::class); $mockClaims = $this->getMockClaims(); $mockClaims->aud = 'incorrect aud'; @@ -274,6 +277,7 @@ public function testValidateIdTokenAudience(): void public function testValidateIdTokenIssuer(): void { + /** @var \Mockery\MockInterface $mockJWT */ $mockJWT = \Mockery::mock('overload:Firebase\JWT\JWT', MockJWT::class); $mockClaims = $this->getMockClaims(); $mockClaims->iss = 'incorrect iss'; @@ -288,6 +292,7 @@ public function testValidateIdTokenIssuer(): void public function testValidateIdTokenNonce(): void { + /** @var \Mockery\MockInterface $mockJWT */ $mockJWT = \Mockery::mock('overload:Firebase\JWT\JWT', MockJWT::class); $mockClaims = $this->getMockClaims(); $mockClaims->nonce = 'incorrect nonce'; @@ -429,11 +434,13 @@ public function testCreateResourceOwner(): void $method = new \ReflectionMethod(OpenIdConfigurationProvider::class, 'createResourceOwner'); $owner = $method->invoke($this->provider, ['id' => '123', 'name' => 'Test'], $token); + $this->assertInstanceOf(\League\OAuth2\Client\Provider\ResourceOwnerInterface::class, $owner); $this->assertSame('123', $owner->getId()); } public function testValidateIdTokenArrayAudience(): void { + /** @var \Mockery\MockInterface $mockJWT */ $mockJWT = \Mockery::mock('overload:Firebase\JWT\JWT', MockJWT::class); $mockClaims = $this->getMockClaims(); $mockClaims->aud = [self::CLIENT_ID, 'other_client']; @@ -449,6 +456,7 @@ public function testValidateIdTokenArrayAudience(): void public function testValidateIdTokenArrayAudienceInvalid(): void { + /** @var \Mockery\MockInterface $mockJWT */ $mockJWT = \Mockery::mock('overload:Firebase\JWT\JWT', MockJWT::class); $mockClaims = $this->getMockClaims(); $mockClaims->aud = ['wrong_client_1', 'wrong_client_2']; @@ -847,6 +855,7 @@ public function testGetJwtVerificationKeysRejectsNonStringKid(): void 'httpClient' => $mockHttpClient, ]); + /** @var \Mockery\MockInterface $mockJWT */ $mockJWT = \Mockery::mock('overload:Firebase\JWT\JWT', MockJWT::class); $this->expectException(JwksException::class); @@ -892,6 +901,7 @@ public function testGetJwtVerificationKeysUnsupportedKeyType(): void 'httpClient' => $mockHttpClient, ]); + /** @var \Mockery\MockInterface $mockJWT */ $mockJWT = \Mockery::mock('overload:Firebase\JWT\JWT', MockJWT::class); $this->expectException(JwksException::class); @@ -937,6 +947,7 @@ public function testGetJwtVerificationKeysCacheHit(): void 'httpClient' => $mockHttpClient, ]); + /** @var \Mockery\MockInterface $mockJWT */ $mockJWT = \Mockery::mock('overload:Firebase\JWT\JWT', MockJWT::class); $mockClaims = $this->getMockClaims(); $mockJWT->shouldReceive('decode')->andReturn($mockClaims); @@ -1050,6 +1061,7 @@ public function testBase64urlDecodeFailure(): void 'httpClient' => $mockHttpClient, ]); + /** @var \Mockery\MockInterface $mockJWT */ $mockJWT = \Mockery::mock('overload:Firebase\JWT\JWT', MockJWT::class); $this->expectException(\ItkDev\OpenIdConnect\Exception\DecodeException::class); @@ -1070,7 +1082,7 @@ public function testBase64urlDecodeFailure(): void * unreadable / not valid JSON, rather than letting `false` or `null` flow * silently into the assertion under test. * - * @return array + * @return array top-level decoded JSON; callers cast / narrow as needed */ private function loadMockFixture(string $filename): array { From 9ef18603fe0dbb34774db1a4feed22c1d2de21c4 Mon Sep 17 00:00:00 2001 From: turegjorup Date: Tue, 12 May 2026 12:49:22 +0200 Subject: [PATCH 2/2] Fix markdown-lint MD024: merge duplicate Documentation sections MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `[Unreleased]` had two `### Documentation` subsections — one added in PR #44 for the README "Exception handling" section, one added later for the per-class PHPDoc audit. markdownlint flags duplicate sibling headings (MD024) and the CI step was failing. Merge the README bullet into the per-class section so there's a single `### Documentation` heading. Co-Authored-By: Claude Opus 4.7 (1M context) --- CHANGELOG.md | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2e6e7fd..768e63b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -90,6 +90,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Documentation +- Added a new "Exception handling" section to `README.md` describing the + marker interface, the SPL parents of each concrete, the PSR-18 + co-implementation on `HttpException`, and the 4.x → 5.0 catch-block + migration. Also fixed the `validateIdToken` example to catch the + marker interface instead of the now-deprecated abstract. - Added class-level PHPDoc to every concrete exception in `src/Exception/` describing what it represents, when it's thrown, the rationale for its SPL parent type, and the boundary against @@ -135,14 +140,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 class (catch `OpenIdConnectExceptionInterface` instead). Kept through 5.x; removal scheduled for 6.0. -### Documentation - -- Added a new "Exception handling" section to `README.md` describing the - marker interface, the SPL parents of each concrete, the PSR-18 - co-implementation on `HttpException`, and the 4.x → 5.0 catch-block - migration. Also fixed the `validateIdToken` example to catch the - marker interface instead of the now-deprecated abstract. - ### Tooling - PHPStan now scans `tests/` in addition to `src/` at level 8, with