Merge pull request #578 from components-web-app/fix/574-trim-identifier

Trim the identifier in user repository lookups (#574)

Author: silverbackdan

CHANGELOG.md

@@ -10,6 +10,7 @@ Every change that reaches `main` adds a line under **Unreleased**. When a versio
 ### Fixed
 - A user referenced by IRI from another resource's write is no longer processed as a new registration, which reset their `emailAddressVerified` ([#575](https://github.com/components-web-app/api-components-bundle/pull/575))
 - A `plainPassword` set through the API is validated (6 to 4096 characters, 422 on `plainPassword`) before it is hashed, and setting a password no longer marks the email address verified; only a password reset does ([#577](https://github.com/components-web-app/api-components-bundle/pull/577))
+- User lookups trim the identifier, so a username or email address with a trailing space (a mobile keyboard's) still signs in and still finds the user for password reset and resend verification ([#578](https://github.com/components-web-app/api-components-bundle/pull/578))
 
 ## [2.0.0-alpha.39](https://github.com/components-web-app/api-components-bundle/compare/2.0.0-alpha.38...2.0.0-alpha.39) - 2026-10-30
 

CLAUDE.md

@@ -304,7 +304,7 @@ Resources are fetched and cached **individually**. Never embed related data outs
 - **A failed send returns false whether or not a logger is wired** (#340): the logger is optional in `UserMailer`'s service locator, so check `has('logger')`; `get()` on a missing id throws.
 - **The `…RequestedAt` timestamp starts the throttle, so `UserMailer` sets it only after `send()` succeeds**, and flushes it with the token then. A failed send (503) saves nothing: the action refreshes the user, so the token is not replaced by one nobody received.
 - Actions calling `UserDataProcessor::findUserByUsername()` must catch its `InvalidArgumentException` and return 404; it throws rather than returning null.
-- **Code typed against `UserRepositoryInterface` may call only what it declares** (#266, #336). The `@method` tags are not a contract, and PHPStan trusts them, so it will not catch a call to `find()` or `findOneBy()`. `loadUserByIdentifier()` matches username *or* email, so check the returned username and treat `NonUniqueResultException` as not found; `Repository\User\FindUserByUsernameTrait` does both. Test against `tests/Command/InMemoryUserRepository.php`, which implements only the interface.
+- **Code typed against `UserRepositoryInterface` may call only what it declares** (#266, #336). The `@method` tags are not a contract, and PHPStan trusts them, so it will not catch a call to `find()` or `findOneBy()`. `loadUserByIdentifier()` matches username *or* email, trimmed and lower-cased like every `UserRepository` lookup (#574: a mobile keyboard's trailing space), so check the returned username, compared the same way and treat `NonUniqueResultException` as not found; `Repository\User\FindUserByUsernameTrait` does both. Test against `tests/Command/InMemoryUserRepository.php`, which implements only the interface.
 - `user:create` throws `ValidationFailedException` rather than writing an invalid or duplicate user (#254). There is no DB unique constraint on username or email.
 - Security routes: one path each. A duplicate path fails nothing and one route silently never runs; check with `debug:router`.
 - `GET /_/health` (#312) is a plain Symfony route (not an AP operation, which would add JSON-LD, cache headers and tags), `no-store`, 200 or 503 with the DBAL message logged at warning and never returned. It is imported by `routing/all.php`.

src/Repository/User/FindUserByUsernameTrait.php

@@ -24,6 +24,6 @@ private function findUserByUsernameIn(UserRepositoryInterface $repository, strin
             return null;
         }
 
-        return null !== $user && strtolower((string) $user->getUsername()) === strtolower($username) ? $user : null;
+        return null !== $user && strtolower((string) $user->getUsername()) === strtolower(trim($username)) ? $user : null;
     }
 }

src/Repository/User/UserRepository.php

@@ -43,7 +43,7 @@ public function findOneByEmail(string $value): ?AbstractUser
     {
         return $this->createQueryBuilder('u')
             ->andWhere('LOWER(u.emailAddress) = :val')
-            ->setParameter('val', strtolower($value))
+            ->setParameter('val', strtolower(trim($value)))
             ->getQuery()
             ->getOneOrNullResult();
     }
@@ -57,7 +57,7 @@ public function findOneWithPasswordResetToken(string $username): ?AbstractUser
             ->andWhere('LOWER(u.username) = :username')
             ->andWhere('u.newPasswordConfirmationToken IS NOT NULL')
             ->andWhere('u.passwordRequestedAt > :minimumDateTime')
-            ->setParameter('username', strtolower($username))
+            ->setParameter('username', strtolower(trim($username)))
             ->setParameter('minimumDateTime', $minimumRequestDateTime)
             ->getQuery()
             ->getOneOrNullResult();
@@ -73,8 +73,8 @@ public function findOneByUsernameAndNewEmailAddress(string $username, string $em
             ->andWhere('LOWER(u.newEmailAddress) = :email')
             ->andWhere('u.newEmailConfirmationToken IS NOT NULL')
             ->andWhere('u.newEmailAddressChangeRequestedAt > :minimumDateTime')
-            ->setParameter('username', strtolower($username))
-            ->setParameter('email', strtolower($email))
+            ->setParameter('username', strtolower(trim($username)))
+            ->setParameter('email', strtolower(trim($email)))
             ->setParameter('minimumDateTime', $minimumRequestDateTime)
             ->getQuery()
             ->getOneOrNullResult();
@@ -90,7 +90,7 @@ public function loadUserByIdentifier(string $identifier): ?AbstractUser
         return $this->createQueryBuilder('u')
             ->andWhere('LOWER(u.username) = :username')
             ->orWhere('LOWER(u.emailAddress) = :username')
-            ->setParameter('username', strtolower($identifier))
+            ->setParameter('username', strtolower(trim($identifier)))
             ->getQuery()
             ->getOneOrNullResult();
     }

tests/Functional/User/ForgotPasswordTest.php

@@ -32,6 +32,17 @@ public function test_i_can_request_a_new_password(): void
         self::assertEmailOfTypeSent('password_reset', 'test.user@example.com');
     }
 
+    public function test_a_username_with_a_trailing_space_still_requests_a_new_password(): void
+    {
+        $client = static::createClient();
+        self::createUserWithRole('my_username', self::generatedSecret(), 'ROLE_USER');
+
+        self::requestJsonLd($client, 'GET', '/password/reset/request/my_username%20', null, self::referer());
+
+        self::assertResponseStatusCodeSame(200);
+        self::assertEmailOfTypeSent('password_reset', 'test.user@example.com');
+    }
+
     public function test_i_get_the_appropriate_status_code_if_the_user_is_not_found(): void
     {
         $client = static::createClient();

tests/Functional/User/SecurityTest.php

@@ -63,6 +63,29 @@ public function test_a_successful_login(): void
         self::assertMercureCookieAndTokenExpireIn(604800);
     }
 
+    #[DataProvider('identifiersWithSurroundingSpace')]
+    public function test_a_login_identifier_with_surrounding_space_still_signs_in(string $identifier): void
+    {
+        $client = static::createClient();
+        $password = self::generatedSecret();
+        self::createUserWithRole('user', $password, 'ROLE_USER', 'user@example.com');
+
+        self::login($client, $identifier, $password);
+
+        self::assertResponseStatusCodeSame(204);
+        self::assertRefreshTokenCount(1);
+    }
+
+    /**
+     * @return iterable<string, array{string}>
+     */
+    public static function identifiersWithSurroundingSpace(): iterable
+    {
+        yield 'a username with a trailing space' => ['user '];
+        yield 'a username with a leading space and capitals' => [' USER'];
+        yield 'an email address with a trailing space' => ['user@example.com '];
+    }
+
     public function test_a_logged_in_user_can_fetch_a_list_of_all_users(): void
     {
         $client = static::createClient();

tests/Functional/User/VerifyEmailAddressTest.php

@@ -73,6 +73,18 @@ public function test_i_can_request_a_new_email_address_verification_email(): voi
         self::assertEmailOfTypeSent('verify_email');
     }
 
+    public function test_a_username_with_a_trailing_space_still_requests_a_new_verification_email(): void
+    {
+        $client = static::createClient();
+        $user = self::createUserWithRole('my_username', self::generatedSecret(), 'ROLE_USER', 'user@example.com');
+        self::unverifyEmail($user);
+
+        self::requestJsonLd($client, 'GET', '/resend-verify-email/my_username%20', null, self::REFERER);
+
+        self::assertResponseStatusCodeSame(200);
+        self::assertEmailOfTypeSent('verify_email');
+    }
+
     public function test_requesting_a_verification_email_for_an_unknown_username_is_a_404_not_an_error(): void
     {
         $client = static::createClient();

tests/Repository/User/FindUserByUsernameTraitTest.php

@@ -0,0 +1,64 @@
+<?php
+
+/*
+ * This file is part of the Silverback API Components Bundle Project
+ *
+ * (c) Daniel West <daniel@silverback.is>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Silverback\ApiComponentsBundle\Tests\Repository\User;
+
+use Doctrine\ORM\NonUniqueResultException;
+use PHPUnit\Framework\Attributes\DataProvider;
+use PHPUnit\Framework\TestCase;
+use Silverback\ApiComponentsBundle\Entity\User\AbstractUser;
+use Silverback\ApiComponentsBundle\Repository\User\FindUserByUsernameTrait;
+use Silverback\ApiComponentsBundle\Repository\User\UserRepositoryInterface;
+use Silverback\ApiComponentsBundle\Tests\Functional\TestBundle\Entity\User;
+
+final class FindUserByUsernameTraitTest extends TestCase
+{
+    /**
+     * @return iterable<string, array{string, string, bool}>
+     */
+    public static function usernames(): iterable
+    {
+        yield 'the username' => ['my_username', 'my_username', true];
+        yield 'the username in capitals with surrounding space' => ['my_username', ' MY_USERNAME ', true];
+        yield 'a username stored in capitals' => ['My_Username', 'my_username', true];
+        yield 'the email address the identifier also matches' => ['my_username', 'user@example.com', false];
+    }
+
+    #[DataProvider('usernames')]
+    public function test_only_the_users_own_username_finds_them(string $storedUsername, string $username, bool $found): void
+    {
+        $user = new User($storedUsername, 'user@example.com');
+        $repository = $this->createStub(UserRepositoryInterface::class);
+        $repository->method('loadUserByIdentifier')->willReturn($user);
+
+        self::assertSame($found ? $user : null, $this->finder()->find($repository, $username));
+    }
+
+    public function test_a_username_matching_several_users_finds_none(): void
+    {
+        $repository = $this->createStub(UserRepositoryInterface::class);
+        $repository->method('loadUserByIdentifier')->willThrowException(new NonUniqueResultException());
+
+        self::assertNull($this->finder()->find($repository, 'my_username'));
+    }
+
+    private function finder(): object
+    {
+        return new class {
+            use FindUserByUsernameTrait;
+
+            public function find(UserRepositoryInterface $repository, string $username): ?AbstractUser
+            {
+                return $this->findUserByUsernameIn($repository, $username);
+            }
+        };
+    }
+}

tests/Repository/User/UserRepositoryTest.php

@@ -0,0 +1,69 @@
+<?php
+
+/*
+ * This file is part of the Silverback API Components Bundle Project
+ *
+ * (c) Daniel West <daniel@silverback.is>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Silverback\ApiComponentsBundle\Tests\Repository\User;
+
+use PHPUnit\Framework\Attributes\DataProvider;
+use Silverback\ApiComponentsBundle\Repository\User\UserRepository;
+use Silverback\ApiComponentsBundle\Tests\Functional\TestBundle\Entity\User;
+use Silverback\ApiComponentsBundle\Tests\Helper\OrphanedResource\OrphanedResourceDatabaseTestCase;
+
+final class UserRepositoryTest extends OrphanedResourceDatabaseTestCase
+{
+    /**
+     * @return iterable<string, array{\Closure(UserRepository): ?User}>
+     */
+    public static function lookupsWithSurroundingSpace(): iterable
+    {
+        yield 'a username identifier' => [static fn (UserRepository $repository) => $repository->loadUserByIdentifier(" My_Username \t")];
+        yield 'an email address identifier' => [static fn (UserRepository $repository) => $repository->loadUserByIdentifier(' User@Example.com ')];
+        yield 'a username through the deprecated loader' => [static fn (UserRepository $repository) => $repository->loadUserByUsername('MY_username ')];
+        yield 'an email address' => [static fn (UserRepository $repository) => $repository->findOneByEmail(' User@Example.COM ')];
+        yield 'a username with a password reset token' => [static fn (UserRepository $repository) => $repository->findOneWithPasswordResetToken(' My_Username ')];
+        yield 'a username and new email address' => [static fn (UserRepository $repository) => $repository->findOneByUsernameAndNewEmailAddress(' My_Username ', ' New@Example.com ')];
+    }
+
+    #[DataProvider('lookupsWithSurroundingSpace')]
+    public function test_a_lookup_ignores_surrounding_whitespace_and_case(\Closure $lookup): void
+    {
+        $user = $this->persistUser();
+
+        self::assertSame($user, $lookup($this->repository()));
+    }
+
+    public function test_an_identifier_that_matches_no_one_finds_no_user(): void
+    {
+        $this->persistUser();
+
+        self::assertNull($this->repository()->loadUserByIdentifier('my user'));
+    }
+
+    private function persistUser(): User
+    {
+        $user = new User('my_username', 'user@example.com');
+        $user
+            ->setNewPasswordConfirmationToken('token')
+            ->setPasswordRequestedAt(new \DateTime())
+            ->setNewEmailAddress('new@example.com')
+            ->setNewEmailConfirmationToken('token')
+            ->setNewEmailAddressChangeRequestedAt(new \DateTime());
+        $this->timestampedDataPersister->persistTimestampedFields($user, true);
+        $this->entityManager->persist($user);
+        $this->entityManager->flush();
+
+        return $user;
+    }
+
+    private function repository(): UserRepository
+    {
+        return new UserRepository($this->registry, User::class, 3600, 3600);
+    }
+}