Skip to content

Commit 867706f

Browse files
authored
Merge pull request #52 from itk-dev/feature/preserve-authentication-failure-cause
fix: preserve authentication failure cause in onAuthenticationFailure
2 parents af98ef0 + b501e1f commit 867706f

3 files changed

Lines changed: 18 additions & 6 deletions

File tree

CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1515

1616
### Changed
1717

18+
- `OpenIdLoginAuthenticator::onAuthenticationFailure()` now chains the
19+
original exception via `previous` and includes its message, so logs and
20+
error reporters retain the cause (timeout vs. signature mismatch vs.
21+
wrong nonce). Symfony's security component still renders only the safe
22+
message key to the user.
1823
- Strengthened tests guided by mutation testing; mutation score raised to
1924
100% with a CI threshold of 95 (`minCoveredMsi` in `infection.json5`)
2025
- Test fixtures use RFC 2606 reserved domains (`provider.example.org`,

src/Security/OpenIdLoginAuthenticator.php

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,9 @@ protected function validateClaims(Request $request): array
7878

7979
public function onAuthenticationFailure(Request $request, AuthenticationException $exception): ?Response
8080
{
81-
throw new AuthenticationException('Error occurred validating openid login');
81+
// Preserve the cause so logs and error reporters can see what actually
82+
// failed (timeout, signature mismatch, wrong nonce, etc.). Symfony's
83+
// security component renders only the safe message key to the user.
84+
throw new AuthenticationException(sprintf('Error occurred validating openid login: %s', $exception->getMessage()), $exception->getCode(), $exception);
8285
}
8386
}

tests/Security/OpenIdLoginAuthenticatorTest.php

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -40,13 +40,17 @@ public function testSupports(): void
4040
$this->assertTrue($this->authenticator->supports($request));
4141
}
4242

43-
public function testOnAuthenticationFailure(): void
43+
public function testOnAuthenticationFailurePreservesCause(): void
4444
{
45-
$this->expectException(AuthenticationException::class);
45+
$cause = new AuthenticationException('Original cause message');
4646

47-
$exception = new AuthenticationException();
48-
49-
$this->authenticator->onAuthenticationFailure(new Request(), $exception);
47+
try {
48+
$this->authenticator->onAuthenticationFailure(new Request(), $cause);
49+
$this->fail('Expected AuthenticationException');
50+
} catch (AuthenticationException $thrown) {
51+
$this->assertSame($cause, $thrown->getPrevious(), 'Original exception must be chained as previous');
52+
$this->assertStringContainsString('Original cause message', $thrown->getMessage(), 'Cause message must be preserved for logs');
53+
}
5054
}
5155

5256
public function testValidateClaimsWrongState(): void

0 commit comments

Comments
 (0)