Lookup usernames case insensitively (#4378)

Author: sandhose

crates/storage-pg/.sqlx/query-94fd96446b237c87bd6bf741f3c42b37ee751b87b7fcc459602bdf8c46962443.json renamed to crates/storage-pg/.sqlx/query-7f8335cc94347bc3a15afe7051658659347a1bf71dd62335df046708f19c967e.json

@@ -0,0 +1,10 @@
+-- no-transaction
+-- Copyright 2025 New Vector Ltd.
+--
+-- SPDX-License-Identifier: AGPL-3.0-only
+-- Please see LICENSE in the repository root for full details.
+
+-- Create an index on the username column, lower-cased, so that we can lookup
+-- usernames in a case-insensitive manner.
+CREATE INDEX CONCURRENTLY users_lower_username_idx
+    ON users (LOWER(username));

crates/storage-pg/.sqlx/query-48213d718a256a12540c0aec595ca3e436be423f2d0c868700c6397745ed0455.json renamed to crates/storage-pg/.sqlx/query-d2a4f5c01603463b78198529d295f7f121769ea5730d01c20c0ddbcdc79a5716.json

@@ -165,6 +165,9 @@ impl UserRepository for PgUserRepository<'_> {
         err,
     )]
     async fn find_by_username(&mut self, username: &str) -> Result<Option<User>, Self::Error> {
+        // We may have multiple users with the same username, but with a different
+        // casing. In this case, we want to return the one which matches the exact
+        // casing
         let res = sqlx::query_as!(
             UserLookup,
             r#"
@@ -175,17 +178,30 @@ impl UserRepository for PgUserRepository<'_> {
                      , deactivated_at
                      , can_request_admin
                 FROM users
-                WHERE username = $1
+                WHERE LOWER(username) = LOWER($1)
             "#,
             username,
         )
         .traced()
-        .fetch_optional(&mut *self.conn)
+        .fetch_all(&mut *self.conn)
         .await?;
 
-        let Some(res) = res else { return Ok(None) };
-
-        Ok(Some(res.into()))
+        match &res[..] {
+            // Happy path: there is only one user matching the username…
+            [user] => Ok(Some(user.clone().into())),
+            // …or none.
+            [] => Ok(None),
+            list => {
+                // If there are multiple users with the same username, we want to
+                // return the one which matches the exact casing
+                if let Some(user) = list.iter().find(|user| user.username == username) {
+                    Ok(Some(user.clone().into()))
+                } else {
+                    // If none match exactly, we prefer to return nothing
+                    Ok(None)
+                }
+            }
+        }
     }
 
     #[tracing::instrument(
@@ -250,7 +266,7 @@ impl UserRepository for PgUserRepository<'_> {
         let exists = sqlx::query_scalar!(
             r#"
                 SELECT EXISTS(
-                    SELECT 1 FROM users WHERE username = $1
+                    SELECT 1 FROM users WHERE LOWER(username) = LOWER($1)
                 ) AS "exists!"
             "#,
             username

crates/storage-pg/migrations/20250410121612_users_lower_username_idx.sql

@@ -216,6 +216,50 @@ async fn test_user_repo(pool: PgPool) {
     repo.save().await.unwrap();
 }
 
+/// Test [`UserRepository::find_by_username`] with different casings.
+#[sqlx::test(migrator = "crate::MIGRATOR")]
+async fn test_user_repo_find_by_username(pool: PgPool) {
+    let mut repo = PgRepository::from_pool(&pool).await.unwrap().boxed();
+    let mut rng = ChaChaRng::seed_from_u64(42);
+    let clock = MockClock::default();
+
+    let alice = repo
+        .user()
+        .add(&mut rng, &clock, "Alice".to_owned())
+        .await
+        .unwrap();
+    let bob1 = repo
+        .user()
+        .add(&mut rng, &clock, "Bob".to_owned())
+        .await
+        .unwrap();
+    let bob2 = repo
+        .user()
+        .add(&mut rng, &clock, "BOB".to_owned())
+        .await
+        .unwrap();
+
+    // This is fine, we can do a case-insensitive search
+    assert_eq!(
+        repo.user().find_by_username("alice").await.unwrap(),
+        Some(alice)
+    );
+
+    // In case there are multiple users with the same username, we should return the
+    // one that matches the exact casing
+    assert_eq!(
+        repo.user().find_by_username("Bob").await.unwrap(),
+        Some(bob1)
+    );
+    assert_eq!(
+        repo.user().find_by_username("BOB").await.unwrap(),
+        Some(bob2)
+    );
+
+    // If none match, we should return None
+    assert!(repo.user().find_by_username("bob").await.unwrap().is_none());
+}
+
 /// Test the user email repository, by trying out most of its methods
 #[sqlx::test(migrator = "crate::MIGRATOR")]
 async fn test_user_email_repo(pool: PgPool) {

crates/storage-pg/src/user/mod.rs

@@ -155,7 +155,7 @@ pub trait UserRepository: Send + Sync {
     /// Returns [`Self::Error`] if the underlying repository fails
     async fn lookup(&mut self, id: Ulid) -> Result<Option<User>, Self::Error>;
 
-    /// Find a [`User`] by its username
+    /// Find a [`User`] by its username, in a case-insensitive manner
     ///
     /// Returns `None` if no [`User`] was found
     ///