Merge pull request #48 from itk-dev/test/mutation-security

test: verify provider key lookup, claims contract and redirect route args

Author: turegjorup

CHANGELOG.md

@@ -18,6 +18,13 @@ 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.
 - Dev: strengthened CLI login flow tests based on mutation testing
   findings — redeeming an unknown token is asserted to throw
   `TokenNotFoundException` specifically, both cache entries (token and

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

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() + [

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

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<string, string>
+     */
+    public array $lastClaims = [];
+
     public function authenticate(Request $request): Passport
     {
         $claims = $this->validateClaims($request);
+        $this->lastClaims = $claims;
 
         return new SelfValidatingPassport(
             new UserBadge(