Skip to content

Commit 58551c9

Browse files
committed
Handle the case where there are multiple users with the same username, but with a different casing.
1 parent 7012fd3 commit 58551c9

File tree

2 files changed

+64
-4
lines changed

2 files changed

+64
-4
lines changed

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

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,9 @@ impl UserRepository for PgUserRepository<'_> {
165165
err,
166166
)]
167167
async fn find_by_username(&mut self, username: &str) -> Result<Option<User>, Self::Error> {
168+
// We may have multiple users with the same username, but with a different
169+
// casing. In this case, we want to return the one which matches the exact
170+
// casing
168171
let res = sqlx::query_as!(
169172
UserLookup,
170173
r#"
@@ -180,12 +183,25 @@ impl UserRepository for PgUserRepository<'_> {
180183
username,
181184
)
182185
.traced()
183-
.fetch_optional(&mut *self.conn)
186+
.fetch_all(&mut *self.conn)
184187
.await?;
185188

186-
let Some(res) = res else { return Ok(None) };
187-
188-
Ok(Some(res.into()))
189+
match &res[..] {
190+
// Happy path: there is only one user matching the username…
191+
[user] => Ok(Some(user.clone().into())),
192+
// …or none.
193+
[] => Ok(None),
194+
list => {
195+
// If there are multiple users with the same username, we want to
196+
// return the one which matches the exact casing
197+
if let Some(user) = list.iter().find(|user| user.username == username) {
198+
Ok(Some(user.clone().into()))
199+
} else {
200+
// If none match exactly, we prefer to return nothing
201+
Ok(None)
202+
}
203+
}
204+
}
189205
}
190206

191207
#[tracing::instrument(

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

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,50 @@ async fn test_user_repo(pool: PgPool) {
216216
repo.save().await.unwrap();
217217
}
218218

219+
/// Test [`UserRepository::find_by_username`] with different casings.
220+
#[sqlx::test(migrator = "crate::MIGRATOR")]
221+
async fn test_user_repo_find_by_username(pool: PgPool) {
222+
let mut repo = PgRepository::from_pool(&pool).await.unwrap().boxed();
223+
let mut rng = ChaChaRng::seed_from_u64(42);
224+
let clock = MockClock::default();
225+
226+
let alice = repo
227+
.user()
228+
.add(&mut rng, &clock, "Alice".to_owned())
229+
.await
230+
.unwrap();
231+
let bob1 = repo
232+
.user()
233+
.add(&mut rng, &clock, "Bob".to_owned())
234+
.await
235+
.unwrap();
236+
let bob2 = repo
237+
.user()
238+
.add(&mut rng, &clock, "BOB".to_owned())
239+
.await
240+
.unwrap();
241+
242+
// This is fine, we can do a case-insensitive search
243+
assert_eq!(
244+
repo.user().find_by_username("alice").await.unwrap(),
245+
Some(alice)
246+
);
247+
248+
// In case there are multiple users with the same username, we should return the
249+
// one that matches the exact casing
250+
assert_eq!(
251+
repo.user().find_by_username("Bob").await.unwrap(),
252+
Some(bob1)
253+
);
254+
assert_eq!(
255+
repo.user().find_by_username("BOB").await.unwrap(),
256+
Some(bob2)
257+
);
258+
259+
// If none match, we should return None
260+
assert!(repo.user().find_by_username("bob").await.unwrap().is_none());
261+
}
262+
219263
/// Test the user email repository, by trying out most of its methods
220264
#[sqlx::test(migrator = "crate::MIGRATOR")]
221265
async fn test_user_email_repo(pool: PgPool) {

0 commit comments

Comments
 (0)