diff --git a/crates/storage-pg/.sqlx/query-94fd96446b237c87bd6bf741f3c42b37ee751b87b7fcc459602bdf8c46962443.json b/crates/storage-pg/.sqlx/query-7f8335cc94347bc3a15afe7051658659347a1bf71dd62335df046708f19c967e.json similarity index 64% rename from crates/storage-pg/.sqlx/query-94fd96446b237c87bd6bf741f3c42b37ee751b87b7fcc459602bdf8c46962443.json rename to crates/storage-pg/.sqlx/query-7f8335cc94347bc3a15afe7051658659347a1bf71dd62335df046708f19c967e.json index f415823cc..9567c1555 100644 --- a/crates/storage-pg/.sqlx/query-94fd96446b237c87bd6bf741f3c42b37ee751b87b7fcc459602bdf8c46962443.json +++ b/crates/storage-pg/.sqlx/query-7f8335cc94347bc3a15afe7051658659347a1bf71dd62335df046708f19c967e.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "\n SELECT EXISTS(\n SELECT 1 FROM users WHERE username = $1\n ) AS \"exists!\"\n ", + "query": "\n SELECT EXISTS(\n SELECT 1 FROM users WHERE LOWER(username) = LOWER($1)\n ) AS \"exists!\"\n ", "describe": { "columns": [ { @@ -18,5 +18,5 @@ null ] }, - "hash": "94fd96446b237c87bd6bf741f3c42b37ee751b87b7fcc459602bdf8c46962443" + "hash": "7f8335cc94347bc3a15afe7051658659347a1bf71dd62335df046708f19c967e" } diff --git a/crates/storage-pg/.sqlx/query-48213d718a256a12540c0aec595ca3e436be423f2d0c868700c6397745ed0455.json b/crates/storage-pg/.sqlx/query-d2a4f5c01603463b78198529d295f7f121769ea5730d01c20c0ddbcdc79a5716.json similarity index 88% rename from crates/storage-pg/.sqlx/query-48213d718a256a12540c0aec595ca3e436be423f2d0c868700c6397745ed0455.json rename to crates/storage-pg/.sqlx/query-d2a4f5c01603463b78198529d295f7f121769ea5730d01c20c0ddbcdc79a5716.json index 52c7ab0bc..171a83623 100644 --- a/crates/storage-pg/.sqlx/query-48213d718a256a12540c0aec595ca3e436be423f2d0c868700c6397745ed0455.json +++ b/crates/storage-pg/.sqlx/query-d2a4f5c01603463b78198529d295f7f121769ea5730d01c20c0ddbcdc79a5716.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "\n SELECT user_id\n , username\n , created_at\n , locked_at\n , deactivated_at\n , can_request_admin\n FROM users\n WHERE username = $1\n ", + "query": "\n SELECT user_id\n , username\n , created_at\n , locked_at\n , deactivated_at\n , can_request_admin\n FROM users\n WHERE LOWER(username) = LOWER($1)\n ", "describe": { "columns": [ { @@ -48,5 +48,5 @@ false ] }, - "hash": "48213d718a256a12540c0aec595ca3e436be423f2d0c868700c6397745ed0455" + "hash": "d2a4f5c01603463b78198529d295f7f121769ea5730d01c20c0ddbcdc79a5716" } diff --git a/crates/storage-pg/migrations/20250410121612_users_lower_username_idx.sql b/crates/storage-pg/migrations/20250410121612_users_lower_username_idx.sql new file mode 100644 index 000000000..625e07e1b --- /dev/null +++ b/crates/storage-pg/migrations/20250410121612_users_lower_username_idx.sql @@ -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)); diff --git a/crates/storage-pg/src/user/mod.rs b/crates/storage-pg/src/user/mod.rs index 2f5036134..659a2172a 100644 --- a/crates/storage-pg/src/user/mod.rs +++ b/crates/storage-pg/src/user/mod.rs @@ -165,6 +165,9 @@ impl UserRepository for PgUserRepository<'_> { err, )] async fn find_by_username(&mut self, username: &str) -> Result, 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 diff --git a/crates/storage-pg/src/user/tests.rs b/crates/storage-pg/src/user/tests.rs index d6f26c885..c37c7ff8e 100644 --- a/crates/storage-pg/src/user/tests.rs +++ b/crates/storage-pg/src/user/tests.rs @@ -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) { diff --git a/crates/storage/src/user/mod.rs b/crates/storage/src/user/mod.rs index 01feebbc4..395c6e615 100644 --- a/crates/storage/src/user/mod.rs +++ b/crates/storage/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, 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 ///