From a76bf7f006edf1ec2d8ed2f87f6e53c5783ff1f4 Mon Sep 17 00:00:00 2001 From: tischsoic Date: Tue, 8 Sep 2026 07:44:33 +0200 Subject: [PATCH 1/2] IBX-11973: Kept blank login fields distinguishable on authentication failure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The failure handler flattens every BadCredentialsException to a single "Bad credentials." message so that an unknown user cannot be told apart from a wrong password. That also hid which submitted field was left empty, which the login screen needs to mark the field invalid. Blank fields are read from the submitted request rather than from the authenticator's message, so both can be reported at once — Symfony stops at the username. The message and the flattening of everything else are unchanged, so templates that only render the message are unaffected. Co-Authored-By: Claude --- .../DefaultAuthenticationFailureHandler.php | 41 ++++++++++-- .../Exception/BlankCredentialsException.php | 62 +++++++++++++++++++ 2 files changed, 98 insertions(+), 5 deletions(-) create mode 100644 src/bundle/Security/Exception/BlankCredentialsException.php diff --git a/src/bundle/Security/Authentication/DefaultAuthenticationFailureHandler.php b/src/bundle/Security/Authentication/DefaultAuthenticationFailureHandler.php index 9d6c799..44db6f6 100644 --- a/src/bundle/Security/Authentication/DefaultAuthenticationFailureHandler.php +++ b/src/bundle/Security/Authentication/DefaultAuthenticationFailureHandler.php @@ -8,6 +8,7 @@ namespace Ibexa\Bundle\User\Security\Authentication; +use Ibexa\Bundle\User\Security\Exception\BlankCredentialsException; use Ibexa\Contracts\Core\Repository\Exceptions\PasswordInUnsupportedFormatException; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; @@ -17,6 +18,10 @@ final class DefaultAuthenticationFailureHandler extends HttpDefaultAuthenticationFailureHandler { + private const string USERNAME_PARAMETER = '_username'; + + private const string PASSWORD_PARAMETER = '_password'; + #[\Override] public function onAuthenticationFailure(Request $request, AuthenticationException $exception): Response { @@ -30,14 +35,40 @@ public function onAuthenticationFailure(Request $request, AuthenticationExceptio if ($exception instanceof BadCredentialsException) { $previous = $exception->getPrevious(); $code = $previous ? $previous->getCode() : 0; + $blankFields = $this->getBlankCredentialFields($request); - $exception = new BadCredentialsException( - 'Bad credentials.', - $code, - $previous - ); + $exception = $blankFields === [] + ? new BadCredentialsException( + 'Bad credentials.', + $code, + $previous + ) + : new BlankCredentialsException( + $blankFields, + 'Bad credentials.', + $code, + $previous + ); } return parent::onAuthenticationFailure($request, $exception); } + + /** + * @return list + */ + private function getBlankCredentialFields(Request $request): array + { + $blankFields = []; + + if (trim((string)$request->request->get(self::USERNAME_PARAMETER, '')) === '') { + $blankFields[] = BlankCredentialsException::FIELD_USERNAME; + } + + if ((string)$request->request->get(self::PASSWORD_PARAMETER, '') === '') { + $blankFields[] = BlankCredentialsException::FIELD_PASSWORD; + } + + return $blankFields; + } } diff --git a/src/bundle/Security/Exception/BlankCredentialsException.php b/src/bundle/Security/Exception/BlankCredentialsException.php new file mode 100644 index 0000000..a647870 --- /dev/null +++ b/src/bundle/Security/Exception/BlankCredentialsException.php @@ -0,0 +1,62 @@ + */ + private array $blankFields; + + /** + * @param list $blankFields + */ + public function __construct( + array $blankFields, + string $message = '', + int $code = 0, + ?Throwable $previous = null + ) { + parent::__construct($message, $code, $previous); + + $this->blankFields = $blankFields; + } + + /** + * @return list + */ + public function getBlankFields(): array + { + return $this->blankFields; + } + + /** + * @return array{list, array} + */ + public function __serialize(): array + { + return [$this->blankFields, parent::__serialize()]; + } + + /** + * @param array{list, array} $data + */ + public function __unserialize(array $data): void + { + [$this->blankFields, $parentData] = $data; + + parent::__unserialize($parentData); + } +} From fbe72bebc1a0bc1e33eb5f7b1590856aa6cd9aed Mon Sep 17 00:00:00 2001 From: tischsoic Date: Tue, 8 Sep 2026 09:58:32 +0200 Subject: [PATCH 2/2] IBX-11973: Covered blank credential reporting with tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds the cases the failure handler now distinguishes: each field blank on its own, both at once, and the asymmetry it inherits from the authenticator — a whitespace-only login counts as blank, a whitespace-only password does not. The existing message test now posts filled credentials, so it keeps covering the plain path rather than silently taking the new one. Co-Authored-By: Claude --- ...efaultAuthenticationFailureHandlerTest.php | 99 ++++++++++++++++++- 1 file changed, 96 insertions(+), 3 deletions(-) diff --git a/tests/lib/Security/Authentication/DefaultAuthenticationFailureHandlerTest.php b/tests/lib/Security/Authentication/DefaultAuthenticationFailureHandlerTest.php index 1a34b6c..89a8434 100644 --- a/tests/lib/Security/Authentication/DefaultAuthenticationFailureHandlerTest.php +++ b/tests/lib/Security/Authentication/DefaultAuthenticationFailureHandlerTest.php @@ -9,6 +9,7 @@ namespace Ibexa\Tests\Bundle\User\Security\Authentication; use Ibexa\Bundle\User\Security\Authentication\DefaultAuthenticationFailureHandler; +use Ibexa\Bundle\User\Security\Exception\BlankCredentialsException; use Ibexa\Contracts\Core\Repository\Exceptions\PasswordInUnsupportedFormatException; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; @@ -59,13 +60,14 @@ public function testOnAuthenticationFailureAltersBadCredentialsExceptionMessage( '_security.last_error', self::callback(static function (AuthenticationException $exception): bool { self::assertInstanceOf(BadCredentialsException::class, $exception); + self::assertNotInstanceOf(BlankCredentialsException::class, $exception); self::assertSame('Bad credentials.', $exception->getMessage()); return true; }) ); - $request = $this->getRequest($session); + $request = $this->getRequest($session, 'admin', 'secret'); $originalException = new BadCredentialsException('Original message'); $this->httpUtils @@ -76,9 +78,100 @@ public function testOnAuthenticationFailureAltersBadCredentialsExceptionMessage( $this->handler->onAuthenticationFailure($request, $originalException); } - private function getRequest(?Session $session = null): Request + /** + * @dataProvider dataProviderForBlankCredentials + * + * @param list $expectedBlankFields + */ + public function testOnAuthenticationFailureReportsBlankCredentialFields( + string $username, + string $password, + array $expectedBlankFields + ): void { + $session = $this->getSession(); + $session + ->expects(self::once()) + ->method('set') + ->with( + '_security.last_error', + self::callback( + static function (AuthenticationException $exception) use ($expectedBlankFields): bool { + self::assertInstanceOf(BlankCredentialsException::class, $exception); + self::assertSame($expectedBlankFields, $exception->getBlankFields()); + self::assertSame('Bad credentials.', $exception->getMessage()); + + return true; + } + ) + ); + + $request = $this->getRequest($session, $username, $password); + + $this->handler->onAuthenticationFailure($request, new BadCredentialsException('Original message')); + } + + public function testOnAuthenticationFailureTreatsWhitespacePasswordAsFilled(): void { - $request = new Request(); + $session = $this->getSession(); + $session + ->expects(self::once()) + ->method('set') + ->with( + '_security.last_error', + self::callback(static function (AuthenticationException $exception): bool { + self::assertNotInstanceOf(BlankCredentialsException::class, $exception); + + return true; + }) + ); + + $request = $this->getRequest($session, 'admin', ' '); + + $this->handler->onAuthenticationFailure($request, new BadCredentialsException('Original message')); + } + + /** + * @return array}> + */ + public function dataProviderForBlankCredentials(): array + { + return [ + 'both fields blank' => [ + '', + '', + [BlankCredentialsException::FIELD_USERNAME, BlankCredentialsException::FIELD_PASSWORD], + ], + 'blank username only' => [ + '', + 'secret', + [BlankCredentialsException::FIELD_USERNAME], + ], + 'blank password only' => [ + 'admin', + '', + [BlankCredentialsException::FIELD_PASSWORD], + ], + 'whitespace-only username' => [ + ' ', + 'secret', + [BlankCredentialsException::FIELD_USERNAME], + ], + ]; + } + + private function getRequest(?Session $session = null, ?string $username = null, ?string $password = null): Request + { + $parameters = []; + + if ($username !== null) { + $parameters['_username'] = $username; + } + + if ($password !== null) { + $parameters['_password'] = $password; + } + + $request = new Request([], $parameters); $request->setSession($session ?? $this->getSession()); return $request;