Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -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));
28 changes: 22 additions & 6 deletions crates/storage-pg/src/user/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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#"
Expand All @@ -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(
Expand Down Expand Up @@ -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
Expand Down
44 changes: 44 additions & 0 deletions crates/storage-pg/src/user/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion crates/storage/src/user/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
///
Expand Down
Loading