From e3d66216f66e25d27a017232205c08dec94e0d32 Mon Sep 17 00:00:00 2001 From: Timshel Date: Wed, 28 May 2025 16:24:56 +0200 Subject: [PATCH] 2FA email and device creation change --- src/api/core/two_factor/email.rs | 21 ++++++----- src/api/identity.rs | 48 +++++++++++-------------- src/db/models/device.rs | 61 ++++++++++++++++++++------------ src/db/models/user.rs | 13 +++++++ 4 files changed, 82 insertions(+), 61 deletions(-) diff --git a/src/api/core/two_factor/email.rs b/src/api/core/two_factor/email.rs index 110856ab..46ff4599 100644 --- a/src/api/core/two_factor/email.rs +++ b/src/api/core/two_factor/email.rs @@ -10,7 +10,7 @@ use crate::{ auth::Headers, crypto, db::{ - models::{EventType, TwoFactor, TwoFactorType, User, UserId}, + models::{DeviceId, EventType, TwoFactor, TwoFactorType, User, UserId}, DbConn, }, error::{Error, MapResult}, @@ -24,11 +24,15 @@ pub fn routes() -> Vec { #[derive(Deserialize)] #[serde(rename_all = "camelCase")] struct SendEmailLoginData { - // DeviceIdentifier: String, // Currently not used + device_identifier: DeviceId, + + #[allow(unused)] #[serde(alias = "Email")] - email: String, + email: Option, + + #[allow(unused)] #[serde(alias = "MasterPasswordHash")] - master_password_hash: String, + master_password_hash: Option, } /// User is trying to login and wants to use email 2FA. @@ -40,15 +44,10 @@ async fn send_email_login(data: Json, mut conn: DbConn) -> E use crate::db::models::User; // Get the user - let Some(user) = User::find_by_mail(&data.email, &mut conn).await else { - err!("Username or password is incorrect. Try again.") + let Some(user) = User::find_by_device_id(&data.device_identifier, &mut conn).await else { + err!("Cannot find user. Try again.") }; - // Check password - if !user.check_valid_password(&data.master_password_hash) { - err!("Username or password is incorrect. Try again.") - } - if !CONFIG._enable_email_2fa() { err!("Email 2FA is disabled") } diff --git a/src/api/identity.rs b/src/api/identity.rs index 9206f620..8e4097a3 100644 --- a/src/api/identity.rs +++ b/src/api/identity.rs @@ -147,7 +147,7 @@ async fn _refresh_login(data: ConnectData, conn: &mut DbConn, ip: &ClientIp) -> err_code!(format!("Unable to refresh login credentials: {}", err.message()), Status::Unauthorized.code) } Ok((mut device, auth_tokens)) => { - // Save to update `device.updated_at` to track usage + // Save to update `device.updated_at` to track usage and toggle new status device.save(conn).await?; let result = json!({ @@ -221,7 +221,7 @@ async fn _sso_login( let now = Utc::now().naive_utc(); // Will trigger 2FA flow if needed - let (user, mut device, new_device, twofactor_token, sso_user) = match user_with_sso { + let (user, mut device, twofactor_token, sso_user) = match user_with_sso { None => { if !CONFIG.is_email_domain_allowed(&user_infos.email) { err!( @@ -253,9 +253,9 @@ async fn _sso_login( user.verified_at = Some(now); user.save(conn).await?; - let (device, new_device) = get_device(&data, conn, &user).await?; + let device = get_device(&data, conn, &user).await?; - (user, device, new_device, None, None) + (user, device, None, None) } Some((user, _)) if !user.enabled => { err!( @@ -267,7 +267,7 @@ async fn _sso_login( ) } Some((mut user, sso_user)) => { - let (mut device, new_device) = get_device(&data, conn, &user).await?; + let mut device = get_device(&data, conn, &user).await?; let twofactor_token = twofactor_auth(&user, &data, &mut device, ip, client_version, conn).await?; if user.private_key.is_none() { @@ -287,7 +287,7 @@ async fn _sso_login( info!("User {} email changed in SSO provider from {} to {}", user.uuid, user.email, user_infos.email); } - (user, device, new_device, twofactor_token, sso_user) + (user, device, twofactor_token, sso_user) } }; @@ -314,7 +314,7 @@ async fn _sso_login( auth_user.expires_in, )?; - authenticated_response(&user, &mut device, new_device, auth_tokens, twofactor_token, &now, conn, ip).await + authenticated_response(&user, &mut device, auth_tokens, twofactor_token, &now, conn, ip).await } async fn _password_login( @@ -430,27 +430,26 @@ async fn _password_login( ) } - let (mut device, new_device) = get_device(&data, conn, &user).await?; + let mut device = get_device(&data, conn, &user).await?; let twofactor_token = twofactor_auth(&user, &data, &mut device, ip, client_version, conn).await?; let auth_tokens = auth::AuthTokens::new(&device, &user, AuthMethod::Password, data.client_id); - authenticated_response(&user, &mut device, new_device, auth_tokens, twofactor_token, &now, conn, ip).await + authenticated_response(&user, &mut device, auth_tokens, twofactor_token, &now, conn, ip).await } #[allow(clippy::too_many_arguments)] async fn authenticated_response( user: &User, device: &mut Device, - new_device: bool, auth_tokens: auth::AuthTokens, twofactor_token: Option, now: &NaiveDateTime, conn: &mut DbConn, ip: &ClientIp, ) -> JsonResult { - if CONFIG.mail_enabled() && new_device { + if CONFIG.mail_enabled() && device.is_new() { if let Err(e) = mail::send_new_device_logged_in(&user.email, &ip.ip.to_string(), now, device).await { error!("Error sending new device email: {e:#?}"); @@ -466,11 +465,11 @@ async fn authenticated_response( } // register push device - if !new_device { + if !device.is_new() { register_push_device(device, conn).await?; } - // Save to update `device.updated_at` to track usage + // Save to update `device.updated_at` to track usage and toggle new status device.save(conn).await?; let mp_policy = master_password_policy(user, conn).await; @@ -566,9 +565,9 @@ async fn _user_api_key_login( ) } - let (mut device, new_device) = get_device(&data, conn, &user).await?; + let mut device = get_device(&data, conn, &user).await?; - if CONFIG.mail_enabled() && new_device { + if CONFIG.mail_enabled() && device.is_new() { let now = Utc::now().naive_utc(); if let Err(e) = mail::send_new_device_logged_in(&user.email, &ip.ip.to_string(), &now, &device).await { error!("Error sending new device email: {e:#?}"); @@ -592,7 +591,7 @@ async fn _user_api_key_login( // let orgs = Membership::find_confirmed_by_user(&user.uuid, conn).await; let access_claims = auth::LoginJwtClaims::default(&device, &user, &AuthMethod::UserApiKey, data.client_id); - // Save to update `device.updated_at` to track usage + // Save to update `device.updated_at` to track usage and toggle new status device.save(conn).await?; info!("User {} logged in successfully via API key. IP: {}", user.email, ip.ip); @@ -646,25 +645,18 @@ async fn _organization_api_key_login(data: ConnectData, conn: &mut DbConn, ip: & } /// Retrieves an existing device or creates a new device from ConnectData and the User -async fn get_device(data: &ConnectData, conn: &mut DbConn, user: &User) -> ApiResult<(Device, bool)> { +async fn get_device(data: &ConnectData, conn: &mut DbConn, user: &User) -> ApiResult { // On iOS, device_type sends "iOS", on others it sends a number // When unknown or unable to parse, return 14, which is 'Unknown Browser' let device_type = util::try_parse_string(data.device_type.as_ref()).unwrap_or(14); let device_id = data.device_identifier.clone().expect("No device id provided"); let device_name = data.device_name.clone().expect("No device name provided"); - let mut new_device = false; // Find device or create new - let device = match Device::find_by_uuid_and_user(&device_id, &user.uuid, conn).await { - Some(device) => device, - None => { - let device = Device::new(device_id, user.uuid.clone(), device_name, device_type); - new_device = true; - device - } - }; - - Ok((device, new_device)) + match Device::find_by_uuid_and_user(&device_id, &user.uuid, conn).await { + Some(device) => Ok(device), + None => Device::new(device_id, user.uuid.clone(), device_name, device_type, conn).await, + } } async fn twofactor_auth( diff --git a/src/db/models/device.rs b/src/db/models/device.rs index 051292c3..91cc1e18 100644 --- a/src/db/models/device.rs +++ b/src/db/models/device.rs @@ -35,25 +35,6 @@ db_object! { /// Local methods impl Device { - pub fn new(uuid: DeviceId, user_uuid: UserId, name: String, atype: i32) -> Self { - let now = Utc::now().naive_utc(); - - Self { - uuid, - created_at: now, - updated_at: now, - - user_uuid, - name, - atype, - - push_uuid: Some(PushId(get_uuid())), - push_token: None, - refresh_token: crypto::encode_random_bytes::<64>(BASE64URL), - twofactor_remember: None, - } - } - pub fn to_json(&self) -> Value { json!({ "id": self.uuid, @@ -77,6 +58,11 @@ impl Device { self.twofactor_remember = None; } + // This rely on the fact we only update the device after a successful login + pub fn is_new(&self) -> bool { + self.created_at == self.updated_at + } + pub fn is_push_device(&self) -> bool { matches!(DeviceType::from_i32(self.atype), DeviceType::Android | DeviceType::Ios) } @@ -120,14 +106,39 @@ impl DeviceWithAuthRequest { } use crate::db::DbConn; -use crate::api::EmptyResult; +use crate::api::{ApiResult, EmptyResult}; use crate::error::MapResult; /// Database methods impl Device { - pub async fn save(&mut self, conn: &mut DbConn) -> EmptyResult { - self.updated_at = Utc::now().naive_utc(); + pub async fn new( + uuid: DeviceId, + user_uuid: UserId, + name: String, + atype: i32, + conn: &mut DbConn, + ) -> ApiResult { + let now = Utc::now().naive_utc(); + + let device = Self { + uuid, + created_at: now, + updated_at: now, + user_uuid, + name, + atype, + + push_uuid: Some(PushId(get_uuid())), + push_token: None, + refresh_token: crypto::encode_random_bytes::<64>(BASE64URL), + twofactor_remember: None, + }; + + device.inner_save(conn).await.map(|()| device) + } + + async fn inner_save(&self, conn: &mut DbConn) -> EmptyResult { db_run! { conn: sqlite, mysql { crate::util::retry( @@ -145,6 +156,12 @@ impl Device { } } + // Should only be called after user has passed authentication + pub async fn save(&mut self, conn: &mut DbConn) -> EmptyResult { + self.updated_at = Utc::now().naive_utc(); + self.inner_save(conn).await + } + pub async fn delete_all_by_user(user_uuid: &UserId, conn: &mut DbConn) -> EmptyResult { db_run! { conn: { diesel::delete(devices::table.filter(devices::user_uuid.eq(user_uuid))) diff --git a/src/db/models/user.rs b/src/db/models/user.rs index 9c4deddb..3a3b5157 100644 --- a/src/db/models/user.rs +++ b/src/db/models/user.rs @@ -8,6 +8,7 @@ use super::{ use crate::{ api::EmptyResult, crypto, + db::models::DeviceId, db::DbConn, error::MapResult, sso::OIDCIdentifier, @@ -393,6 +394,18 @@ impl User { }} } + pub async fn find_by_device_id(device_uuid: &DeviceId, conn: &mut DbConn) -> Option { + db_run! { conn: { + users::table + .inner_join(devices::table.on(devices::user_uuid.eq(users::uuid))) + .filter(devices::uuid.eq(device_uuid)) + .select(users::all_columns) + .first::(conn) + .ok() + .from_db() + }} + } + pub async fn get_all(conn: &mut DbConn) -> Vec<(User, Option)> { db_run! {conn: { users::table