diff --git a/CHANGELOG.md b/CHANGELOG.md index 470efa0..13d2d49 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -67,6 +67,24 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 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 + `reportIgnoresWithoutComments: true` so unexplained + `@phpstan-ignore` directives fail CI. +- Added `phpstan/phpstan-mockery` to `require-dev` for stubs covering + Mockery's fluent `shouldReceive(...)->andReturn(...)` API. +- Cleaned the 46 pre-existing level-8 issues in `tests/`: dropped the + unused nullable from `$this->provider`, narrowed `validateIdToken` + claim accesses with `object{nonce, aud}` `@var` shapes, replaced + silent `(string)` coercion of `file_get_contents` / `parse_url` + failures with `assertNotFalse` / `assertIsString` boundary guards, + swapped `assertTrue(true)` tautologies for + `expectNotToPerformAssertions`, and replaced the constant-folded + `is_subclass_of` marker check with a `ReflectionClass` lookup so + PHPStan can't fold it into a tautology. `phpstan-baseline.neon` + consequently shrinks to zero and is deleted. + ## [4.1.2] - 2026-05-11 - Chained `previous` consistently in `OpenIdConfigurationProvider` catch diff --git a/composer.json b/composer.json index 1bf1ecb..bb3321b 100644 --- a/composer.json +++ b/composer.json @@ -32,6 +32,7 @@ "friendsofphp/php-cs-fixer": "^3.75", "mockery/mockery": "^1.6.12", "phpstan/phpstan": "^2.1.41", + "phpstan/phpstan-mockery": "^2.0", "phpunit/php-code-coverage": "^12", "phpunit/phpunit": "^12" }, diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon deleted file mode 100644 index 9d14590..0000000 --- a/phpstan-baseline.neon +++ /dev/null @@ -1,139 +0,0 @@ -parameters: - ignoreErrors: - - - message: '#^Call to function is_subclass_of\(\) with ''ItkDev\\\\OpenIdConnect\\\\Exception\\\\ItkOpenIdConnectException'' and ''ItkDev\\\\OpenIdConnect\\\\Exception\\\\OpenIdConnectExceptionInterface'' will always evaluate to true\.$#' - identifier: function.alreadyNarrowedType - count: 1 - path: tests/Exception/ExceptionHierarchyTest.php - - - - message: '#^Call to method PHPUnit\\Framework\\Assert\:\:assertTrue\(\) with true will always evaluate to true\.$#' - identifier: method.alreadyNarrowedType - count: 1 - path: tests/Exception/ExceptionHierarchyTest.php - - - - message: '#^Property Tests\\Security\\MockJWT\:\:\$leeway has no type specified\.$#' - identifier: missingType.property - count: 1 - path: tests/Security/MockJWT.php - - - - message: '#^Access to an undefined property object\:\:\$aud\.$#' - identifier: property.notFound - count: 2 - path: tests/Security/OpenIdConfigurationProviderTest.php - - - - message: '#^Access to an undefined property object\:\:\$nonce\.$#' - identifier: property.notFound - count: 3 - path: tests/Security/OpenIdConfigurationProviderTest.php - - - - message: '#^Call to an undefined method Mockery\\ExpectationInterface\|Mockery\\HigherOrderMessage\:\:andReturn\(\)\.$#' - identifier: method.notFound - count: 6 - path: tests/Security/OpenIdConfigurationProviderTest.php - - - - message: '#^Call to an undefined method Mockery\\ExpectationInterface\|Mockery\\HigherOrderMessage\:\:andThrow\(\)\.$#' - identifier: method.notFound - count: 1 - path: tests/Security/OpenIdConfigurationProviderTest.php - - - - message: '#^Call to an undefined method Mockery\\ExpectationInterface\|Mockery\\HigherOrderMessage\:\:with\(\)\.$#' - identifier: method.notFound - count: 1 - path: tests/Security/OpenIdConfigurationProviderTest.php - - - - message: '#^Call to method PHPUnit\\Framework\\Assert\:\:assertTrue\(\) with true will always evaluate to true\.$#' - identifier: method.alreadyNarrowedType - count: 1 - path: tests/Security/OpenIdConfigurationProviderTest.php - - - - message: '#^Cannot call method generateNonce\(\) on ItkDev\\OpenIdConnect\\Security\\OpenIdConfigurationProvider\|null\.$#' - identifier: method.nonObject - count: 1 - path: tests/Security/OpenIdConfigurationProviderTest.php - - - - message: '#^Cannot call method generateState\(\) on ItkDev\\OpenIdConnect\\Security\\OpenIdConfigurationProvider\|null\.$#' - identifier: method.nonObject - count: 1 - path: tests/Security/OpenIdConfigurationProviderTest.php - - - - message: '#^Cannot call method getAuthorizationUrl\(\) on ItkDev\\OpenIdConnect\\Security\\OpenIdConfigurationProvider\|null\.$#' - identifier: method.nonObject - count: 3 - path: tests/Security/OpenIdConfigurationProviderTest.php - - - - message: '#^Cannot call method getBaseAccessTokenUrl\(\) on ItkDev\\OpenIdConnect\\Security\\OpenIdConfigurationProvider\|null\.$#' - identifier: method.nonObject - count: 1 - path: tests/Security/OpenIdConfigurationProviderTest.php - - - - message: '#^Cannot call method getBaseAuthorizationUrl\(\) on ItkDev\\OpenIdConnect\\Security\\OpenIdConfigurationProvider\|null\.$#' - identifier: method.nonObject - count: 1 - path: tests/Security/OpenIdConfigurationProviderTest.php - - - - message: '#^Cannot call method getDefaultScopes\(\) on ItkDev\\OpenIdConnect\\Security\\OpenIdConfigurationProvider\|null\.$#' - identifier: method.nonObject - count: 1 - path: tests/Security/OpenIdConfigurationProviderTest.php - - - - message: '#^Cannot call method getEndSessionUrl\(\) on ItkDev\\OpenIdConnect\\Security\\OpenIdConfigurationProvider\|null\.$#' - identifier: method.nonObject - count: 6 - path: tests/Security/OpenIdConfigurationProviderTest.php - - - - message: '#^Cannot call method getGuarded\(\) on ItkDev\\OpenIdConnect\\Security\\OpenIdConfigurationProvider\|null\.$#' - identifier: method.nonObject - count: 1 - path: tests/Security/OpenIdConfigurationProviderTest.php - - - - message: '#^Cannot call method getResourceOwnerDetailsUrl\(\) on ItkDev\\OpenIdConnect\\Security\\OpenIdConfigurationProvider\|null\.$#' - identifier: method.nonObject - count: 1 - path: tests/Security/OpenIdConfigurationProviderTest.php - - - - message: '#^Cannot call method getState\(\) on ItkDev\\OpenIdConnect\\Security\\OpenIdConfigurationProvider\|null\.$#' - identifier: method.nonObject - count: 1 - path: tests/Security/OpenIdConfigurationProviderTest.php - - - - message: '#^Cannot call method validateIdToken\(\) on ItkDev\\OpenIdConnect\\Security\\OpenIdConfigurationProvider\|null\.$#' - identifier: method.nonObject - count: 7 - path: tests/Security/OpenIdConfigurationProviderTest.php - - - - message: '#^Parameter \#1 \$json of function json_decode expects string, string\|false given\.$#' - identifier: argument.type - count: 3 - path: tests/Security/OpenIdConfigurationProviderTest.php - - - - message: '#^Parameter \#1 \$string of function parse_str expects string, string\|false\|null given\.$#' - identifier: argument.type - count: 1 - path: tests/Security/OpenIdConfigurationProviderTest.php - - - - message: '#^Property Tests\\Security\\OpenIdConfigurationProviderTest\:\:\$provider \(ItkDev\\OpenIdConnect\\Security\\OpenIdConfigurationProvider\|null\) is never assigned null so it can be removed from the property type\.$#' - identifier: property.unusedType - count: 1 - path: tests/Security/OpenIdConfigurationProviderTest.php diff --git a/phpstan.neon b/phpstan.neon index b0a1a59..6e3ce04 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -1,5 +1,5 @@ includes: - - phpstan-baseline.neon + - vendor/phpstan/phpstan-mockery/extension.neon parameters: level: 8 diff --git a/tests/Exception/ExceptionHierarchyTest.php b/tests/Exception/ExceptionHierarchyTest.php index 06fdd92..658cd65 100644 --- a/tests/Exception/ExceptionHierarchyTest.php +++ b/tests/Exception/ExceptionHierarchyTest.php @@ -121,11 +121,15 @@ public function testAbstractBaseImplementsMarker(): void // Catch sites that wrote `catch (ItkOpenIdConnectException $e)` should // migrate to the marker interface; this assertion guards the marker // implementation while the deprecation window is open. - $this->assertTrue( - is_subclass_of( - \ItkDev\OpenIdConnect\Exception\ItkOpenIdConnectException::class, - OpenIdConnectExceptionInterface::class, - ), + // + // ReflectionClass keeps the check at runtime so PHPStan can't fold it + // into a constant tautology — the value of the test is catching a + // *future* regression that removes the marker from the abstract. + $reflection = new \ReflectionClass(\ItkDev\OpenIdConnect\Exception\ItkOpenIdConnectException::class); + $this->assertContains( + OpenIdConnectExceptionInterface::class, + $reflection->getInterfaceNames(), + 'Deprecated abstract must still implement the marker for 5.x BC.', ); } } diff --git a/tests/Security/MockJWT.php b/tests/Security/MockJWT.php index 95a8cde..cdd40e7 100644 --- a/tests/Security/MockJWT.php +++ b/tests/Security/MockJWT.php @@ -4,5 +4,5 @@ class MockJWT { - public static $leeway; + public static ?int $leeway = null; } diff --git a/tests/Security/OpenIdConfigurationProviderTest.php b/tests/Security/OpenIdConfigurationProviderTest.php index c3ef46b..7c91d1e 100644 --- a/tests/Security/OpenIdConfigurationProviderTest.php +++ b/tests/Security/OpenIdConfigurationProviderTest.php @@ -37,7 +37,7 @@ class OpenIdConfigurationProviderTest extends TestCase private const REDIRECT_URI = 'https://redirect.url'; private const NONCE = '12345678'; - private ?OpenIdConfigurationProvider $provider; + private OpenIdConfigurationProvider $provider; public function setUp(): void { @@ -168,7 +168,9 @@ public function testGetAuthorizationUrl(): void $authUrl = $this->provider->getAuthorizationUrl(['state' => $state, 'nonce' => $nonce]); $query = []; - parse_str(parse_url($authUrl, PHP_URL_QUERY), $query); + $queryString = parse_url($authUrl, PHP_URL_QUERY); + $this->assertIsString($queryString, 'Generated authorization URL must have a query string'); + parse_str($queryString, $query); $this->assertSame('openid', $query['scope']); $this->assertSame('id_token', $query['response_type']); @@ -238,6 +240,7 @@ public function testValidateIdTokenSuccess(): void ) )->andReturn($mockClaims); + /** @var object{nonce: string, aud: string|list} $claims */ $claims = $this->provider->validateIdToken('token', self::NONCE); $this->assertEquals(self::NONCE, $claims->nonce); @@ -370,6 +373,8 @@ public function testGetResourceOwnerDetailsUrl(): void public function testCheckResponseSuccess(): void { + $this->expectNotToPerformAssertions(); + $response = $this->createStub(ResponseInterface::class); $response->method('getStatusCode')->willReturn(200); @@ -377,7 +382,6 @@ public function testCheckResponseSuccess(): void // Should not throw $method->invoke($this->provider, $response, ['data' => 'value']); - $this->assertTrue(true); } public function testCheckResponseWithErrorString(): void @@ -436,6 +440,7 @@ public function testValidateIdTokenArrayAudience(): void $mockJWT->shouldReceive('decode')->andReturn($mockClaims); + /** @var object{nonce: string, aud: string|list} $claims */ $claims = $this->provider->validateIdToken('token', self::NONCE); $this->assertEquals(self::NONCE, $claims->nonce); @@ -542,10 +547,7 @@ public function testGetIdTokenFailure(): void public function testGetConfigurationCacheHit(): void { - $configuration = json_decode( - file_get_contents(__DIR__.'/../MockData/mockOpenIDConfiguration.json'), - true - ); + $configuration = $this->loadMockFixture('mockOpenIDConfiguration.json'); $mockCacheItem = $this->createStub(CacheItemInterface::class); $mockCacheItem->method('isHit')->willReturn(true); @@ -730,10 +732,7 @@ public function testGetJwtVerificationKeysCacheHit(): void { $openIDConnectMetadataUrl = 'https://some.url/openid-configuration'; - $configuration = json_decode( - file_get_contents(__DIR__.'/../MockData/mockOpenIDConfiguration.json'), - true - ); + $configuration = $this->loadMockFixture('mockOpenIDConfiguration.json'); $cachedKeys = ['key1' => new Key('public-key-data', 'RS256')]; @@ -770,6 +769,7 @@ public function testGetJwtVerificationKeysCacheHit(): void $mockClaims = $this->getMockClaims(); $mockJWT->shouldReceive('decode')->andReturn($mockClaims); + /** @var object{nonce: string} $claims */ $claims = $provider->validateIdToken('token', self::NONCE); $this->assertEquals(self::NONCE, $claims->nonce); } @@ -804,10 +804,7 @@ public function testGetConfigurationCacheInvalidArgument(): void public function testGetJwtVerificationKeysCacheInvalidArgument(): void { $openIDConnectMetadataUrl = 'https://some.url/openid-configuration'; - $configuration = json_decode( - file_get_contents(__DIR__.'/../MockData/mockOpenIDConfiguration.json'), - true - ); + $configuration = $this->loadMockFixture('mockOpenIDConfiguration.json'); $configCacheItem = $this->createStub(CacheItemInterface::class); $configCacheItem->method('isHit')->willReturn(true); @@ -892,12 +889,28 @@ public function testBase64urlDecodeFailure(): void /** * Get a mock success response with mock date. * - * @param string $mockResponseDataPath - * Path to the file containing the mock response data - * * @return ResponseInterface * A success ("200") response with mock body data */ + /** + * Load a JSON fixture from tests/MockData and decode it as an associative + * array. Fails the test with an explicit message if the file is missing / + * unreadable / not valid JSON, rather than letting `false` or `null` flow + * silently into the assertion under test. + * + * @return array + */ + private function loadMockFixture(string $filename): array + { + $path = __DIR__.'/../MockData/'.$filename; + $contents = file_get_contents($path); + $this->assertNotFalse($contents, sprintf('Mock fixture not readable: %s', $path)); + $decoded = json_decode($contents, true); + $this->assertIsArray($decoded, sprintf('Mock fixture is not valid JSON: %s', $path)); + + return $decoded; + } + private function getMockHttpSuccessResponse(string $mockResponseDataPath): ResponseInterface { $mockResponseData = file_get_contents(__DIR__.$mockResponseDataPath);