From d02241eea9e7d8040b6ec3f865c8a0411605f066 Mon Sep 17 00:00:00 2001 From: turegjorup Date: Thu, 11 Jun 2026 10:09:52 +0200 Subject: [PATCH] test: verify provider key lookup, claims contract and redirect route args Mutation testing exposed argument-blind stubs across the Security tests: the router stub ignored the redirect_route_parameters when building a provider redirect URI, the provider manager stub matched any key (so a mangled session provider key went unnoticed), the claims returned by validateClaims were never asserted to include the open_id_connect_provider key, and a request with no loginToken at all was untested (only the empty-string case was). Kills all 4 escaped mutants in src/Security. Co-Authored-By: Claude Fable 5 --- CHANGELOG.md | 8 ++++++++ .../Security/CliLoginTokenAuthenticatorTest.php | 13 +++++++++++++ .../OpenIdConfigurationProviderManagerTest.php | 8 +++++++- tests/Security/OpenIdLoginAuthenticatorTest.php | 16 ++++++++++++++-- tests/Security/TestAuthenticator.php | 9 +++++++++ 5 files changed, 51 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e51abb7..43f0005 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- Dev: strengthened Security tests based on mutation testing findings — + the redirect-route parameters are asserted to reach the router when + building a provider redirect URI, `validateClaims` is asserted to look + up the exact provider key from the session and to merge + `open_id_connect_provider` into the returned claims, and a request + without any `loginToken` parameter is asserted to be rejected as + unauthorized. No effect on the published package. + - CI: bumped `codecov/codecov-action` from `v5` to `v7` (restores Codecov's GPG signing key after the `codecovsecurity` account was removed, and moves the bundled `github-script` to Node 24) and set `fail_ci_if_error: false` diff --git a/tests/Security/CliLoginTokenAuthenticatorTest.php b/tests/Security/CliLoginTokenAuthenticatorTest.php index ece39d2..7a925a7 100644 --- a/tests/Security/CliLoginTokenAuthenticatorTest.php +++ b/tests/Security/CliLoginTokenAuthenticatorTest.php @@ -61,6 +61,19 @@ public function testAuthenticateWithEmptyToken(): void $this->authenticator->authenticate($request); } + public function testAuthenticateWithMissingToken(): void + { + // No loginToken query parameter at all: the null from query->get() + // must be coerced to '' and rejected as "no token provided", not + // passed on to the login helper. + $request = new Request(); + + $this->expectException(CustomUserMessageAuthenticationException::class); + $this->expectExceptionMessage('No login token provided'); + + $this->authenticator->authenticate($request); + } + public function testAuthenticateWithInvalidToken(): void { $this->stubCliLoginHelper diff --git a/tests/Security/OpenIdConfigurationProviderManagerTest.php b/tests/Security/OpenIdConfigurationProviderManagerTest.php index cd6b4f4..8610db8 100644 --- a/tests/Security/OpenIdConfigurationProviderManagerTest.php +++ b/tests/Security/OpenIdConfigurationProviderManagerTest.php @@ -9,6 +9,7 @@ use PHPUnit\Framework\MockObject\Stub; use PHPUnit\Framework\TestCase; use Symfony\Component\Cache\Adapter\ArrayAdapter; +use Symfony\Component\Routing\Generator\UrlGeneratorInterface; use Symfony\Component\Routing\RouterInterface; class OpenIdConfigurationProviderManagerTest extends TestCase @@ -77,9 +78,14 @@ public function testGetProviderThrowsOnInvalidKey(): void public function testGetProviderWithRedirectRoute(): void { - $this->stubRouter + // Expect the exact arguments so dropping the route parameters (or the + // route itself) when building the redirect URI fails the test. + $mockRouter = $this->createMock(RouterInterface::class); + $mockRouter->expects($this->once()) ->method('generate') + ->with('my_route', ['param' => 'value'], UrlGeneratorInterface::ABSOLUTE_URL) ->willReturn('https://app.com/callback'); + $this->stubRouter = $mockRouter; $manager = $this->createManager([ 'test' => $this->getBaseProviderConfig() + [ diff --git a/tests/Security/OpenIdLoginAuthenticatorTest.php b/tests/Security/OpenIdLoginAuthenticatorTest.php index 320c0b2..2145468 100644 --- a/tests/Security/OpenIdLoginAuthenticatorTest.php +++ b/tests/Security/OpenIdLoginAuthenticatorTest.php @@ -114,14 +114,26 @@ public function testValidateClaimsSuccess(): void $claims->name = 'Test Tester'; $stubProvider->method('validateIdToken')->willReturn($claims); - $this->stubProviderManager->method('getProvider')->willReturn($stubProvider); + // Expect the exact provider key from the session, so a lookup with a + // mangled key fails the test instead of silently matching any key. + $mockProviderManager = $this->createMock(OpenIdConfigurationProviderManager::class); + $mockProviderManager->expects($this->once()) + ->method('getProvider') + ->with('test_provider_1') + ->willReturn($stubProvider); + $authenticator = new TestAuthenticator($mockProviderManager); $request = new Request(query: ['state' => 'test_state', 'code' => 'test_code']); $this->setSessionOnRequest($request); - $passport = $this->authenticator->authenticate($request); + $passport = $authenticator->authenticate($request); $this->assertSame('test@test.com', $passport->getUser()->getUserIdentifier()); + + // The claims contract: the IdP claims plus the provider key that + // authenticated the user. + $this->assertSame('Test Tester', $authenticator->lastClaims['name'] ?? null); + $this->assertSame('test_provider_1', $authenticator->lastClaims['open_id_connect_provider'] ?? null); } private function setSessionOnRequest(Request $request, ?string $nonce = 'test_nonce'): void diff --git a/tests/Security/TestAuthenticator.php b/tests/Security/TestAuthenticator.php index 976f6e5..17ec159 100644 --- a/tests/Security/TestAuthenticator.php +++ b/tests/Security/TestAuthenticator.php @@ -13,9 +13,18 @@ class TestAuthenticator extends OpenIdLoginAuthenticator { + /** + * Claims returned by the last validateClaims() call, exposed so tests + * can assert on the full claims array (validateClaims is protected). + * + * @var array + */ + public array $lastClaims = []; + public function authenticate(Request $request): Passport { $claims = $this->validateClaims($request); + $this->lastClaims = $claims; return new SelfValidatingPassport( new UserBadge(