From ee8ee7af82b92dcf91f894bbadff4b2d7ddb101a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Gonz=C3=A1lez?= <7822554+AlexTMjugador@users.noreply.github.com> Date: Wed, 11 Jun 2025 23:59:21 +0200 Subject: [PATCH] feat(labrinth): ignore email case differences in password recovery flow (#3771) * feat(labrinth): ignore email case differences in password recovery flow * chore(labrinth): run `sqlx prepare` --- ...dc9b378ca9c97a128366cae97649503d5dfdf.json | 22 +++ ...50611164523_lowercase-user-email-index.sql | 1 + .../labrinth/src/database/models/user_item.rs | 28 +++- apps/labrinth/src/routes/internal/flows.rs | 130 ++++++++++++------ 4 files changed, 133 insertions(+), 48 deletions(-) create mode 100644 apps/labrinth/.sqlx/query-889a4f79b7031436b3ed31d1005dc9b378ca9c97a128366cae97649503d5dfdf.json create mode 100644 apps/labrinth/migrations/20250611164523_lowercase-user-email-index.sql diff --git a/apps/labrinth/.sqlx/query-889a4f79b7031436b3ed31d1005dc9b378ca9c97a128366cae97649503d5dfdf.json b/apps/labrinth/.sqlx/query-889a4f79b7031436b3ed31d1005dc9b378ca9c97a128366cae97649503d5dfdf.json new file mode 100644 index 000000000..7c0dbbdb8 --- /dev/null +++ b/apps/labrinth/.sqlx/query-889a4f79b7031436b3ed31d1005dc9b378ca9c97a128366cae97649503d5dfdf.json @@ -0,0 +1,22 @@ +{ + "db_name": "PostgreSQL", + "query": "\n SELECT id FROM users\n WHERE LOWER(email) = LOWER($1)\n ", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "id", + "type_info": "Int8" + } + ], + "parameters": { + "Left": [ + "Text" + ] + }, + "nullable": [ + false + ] + }, + "hash": "889a4f79b7031436b3ed31d1005dc9b378ca9c97a128366cae97649503d5dfdf" +} diff --git a/apps/labrinth/migrations/20250611164523_lowercase-user-email-index.sql b/apps/labrinth/migrations/20250611164523_lowercase-user-email-index.sql new file mode 100644 index 000000000..80ff690a2 --- /dev/null +++ b/apps/labrinth/migrations/20250611164523_lowercase-user-email-index.sql @@ -0,0 +1 @@ +CREATE INDEX users_lowercase_email ON users (LOWER(email)); diff --git a/apps/labrinth/src/database/models/user_item.rs b/apps/labrinth/src/database/models/user_item.rs index 4f9af3b88..d38ed8460 100644 --- a/apps/labrinth/src/database/models/user_item.rs +++ b/apps/labrinth/src/database/models/user_item.rs @@ -224,24 +224,46 @@ impl DBUser { Ok(val) } - pub async fn get_email<'a, E>( + pub async fn get_by_email<'a, E>( email: &str, exec: E, ) -> Result, sqlx::Error> where E: sqlx::Executor<'a, Database = sqlx::Postgres> + Copy, { - let user_pass = sqlx::query!( + let user = sqlx::query!( " SELECT id FROM users WHERE email = $1 ", email ) + .map(|row| DBUserId(row.id)) .fetch_optional(exec) .await?; - Ok(user_pass.map(|x| DBUserId(x.id))) + Ok(user) + } + + pub async fn get_by_case_insensitive_email<'a, E>( + email: &str, + exec: E, + ) -> Result, sqlx::Error> + where + E: sqlx::Executor<'a, Database = sqlx::Postgres> + Copy, + { + let users = sqlx::query!( + " + SELECT id FROM users + WHERE LOWER(email) = LOWER($1) + ", + email + ) + .map(|row| DBUserId(row.id)) + .fetch_all(exec) + .await?; + + Ok(users) } pub async fn get_projects<'a, E>( diff --git a/apps/labrinth/src/routes/internal/flows.rs b/apps/labrinth/src/routes/internal/flows.rs index b8ef122cc..1117c9516 100644 --- a/apps/labrinth/src/routes/internal/flows.rs +++ b/apps/labrinth/src/routes/internal/flows.rs @@ -1,6 +1,7 @@ use crate::auth::email::send_email; use crate::auth::validate::get_user_record_from_bearer_token; use crate::auth::{AuthProvider, AuthenticationError, get_user_from_headers}; +use crate::database::models::DBUser; use crate::database::models::flow_item::DBFlow; use crate::database::redis::RedisPool; use crate::file_hosting::FileHost; @@ -76,7 +77,7 @@ impl TempUser { redis: &RedisPool, ) -> Result { if let Some(email) = &self.email { - if crate::database::models::DBUser::get_email(email, client) + if crate::database::models::DBUser::get_by_email(email, client) .await? .is_some() { @@ -1385,9 +1386,12 @@ pub async fn create_account_with_password( .hash_password(new_account.password.as_bytes(), &salt)? .to_string(); - if crate::database::models::DBUser::get_email(&new_account.email, &**pool) - .await? - .is_some() + if crate::database::models::DBUser::get_by_email( + &new_account.email, + &**pool, + ) + .await? + .is_some() { return Err(ApiError::InvalidInput( "Email is already registered on Modrinth!".to_string(), @@ -1450,7 +1454,8 @@ pub async fn create_account_with_password( #[derive(Deserialize, Validate)] pub struct Login { - pub username: String, + #[serde(rename = "username")] + pub username_or_email: String, pub password: String, pub challenge: String, } @@ -1466,14 +1471,17 @@ pub async fn login_password( return Err(ApiError::Turnstile); } - let user = if let Some(user) = - crate::database::models::DBUser::get(&login.username, &**pool, &redis) - .await? + let user = if let Some(user) = crate::database::models::DBUser::get( + &login.username_or_email, + &**pool, + &redis, + ) + .await? { user } else { - let user = crate::database::models::DBUser::get_email( - &login.username, + let user = crate::database::models::DBUser::get_by_email( + &login.username_or_email, &**pool, ) .await? @@ -1903,7 +1911,8 @@ pub async fn remove_2fa( #[derive(Deserialize)] pub struct ResetPassword { - pub username: String, + #[serde(rename = "username")] + pub username_or_email: String, pub challenge: String, } @@ -1918,46 +1927,77 @@ pub async fn reset_password_begin( return Err(ApiError::Turnstile); } - let user = if let Some(user_id) = - crate::database::models::DBUser::get_email( - &reset_password.username, + let user = + match crate::database::models::DBUser::get_by_case_insensitive_email( + &reset_password.username_or_email, &**pool, ) - .await? - { - crate::database::models::DBUser::get_id(user_id, &**pool, &redis) - .await? - } else { - crate::database::models::DBUser::get( - &reset_password.username, - &**pool, - &redis, - ) - .await? - }; + .await?[..] + { + [] => { + // Try finding by username or ID + crate::database::models::DBUser::get( + &reset_password.username_or_email, + &**pool, + &redis, + ) + .await? + } + [user_id] => { + // If there is only one user with the given email, ignoring case, + // we can assume it's the user we want to reset the password for + crate::database::models::DBUser::get_id( + user_id, &**pool, &redis, + ) + .await? + } + _ => { + // When several users use variations of the same email with + // different cases, we cannot reliably tell which user should + // receive the password reset email, so fall back to case sensitive + // search to avoid spamming multiple users + if let Some(user_id) = + crate::database::models::DBUser::get_by_email( + &reset_password.username_or_email, + &**pool, + ) + .await? + { + crate::database::models::DBUser::get_id( + user_id, &**pool, &redis, + ) + .await? + } else { + None + } + } + }; - if let Some(user) = user { - let flow = DBFlow::ForgotPassword { user_id: user.id } + if let Some(DBUser { + id: user_id, + email: Some(email), + .. + }) = user + { + let flow = DBFlow::ForgotPassword { user_id } .insert(Duration::hours(24), &redis) .await?; - if let Some(email) = user.email { - send_email( - email, - "Reset your password", - "Please visit the following link below to reset your password. If the button does not work, you can copy the link and paste it into your browser.", - "If you did not request for your password to be reset, you can safely ignore this email.", - Some(( - "Reset password", - &format!( - "{}/{}?flow={}", - dotenvy::var("SITE_URL")?, - dotenvy::var("SITE_RESET_PASSWORD_PATH")?, - flow - ), - )), - )?; - } + send_email( + email, + "Reset your password", + "Please visit the following link below to reset your password. If the button does not work, you can copy the link and paste it into your browser.", + "If you did not request for your password to be reset, you can safely ignore this email.", + Some(( + "Reset password", + &format!( + "{}/{}?flow={}", + dotenvy::var("SITE_URL")?, + dotenvy::var("SITE_RESET_PASSWORD_PATH")?, + flow + ), + )), + )?; } Ok(HttpResponse::Ok().finish())