From 055cb618885276c39e93c80df69cf5933b723ba9 Mon Sep 17 00:00:00 2001 From: max Date: Sun, 28 Jun 2026 21:07:45 +0200 Subject: [PATCH] Don't block the login response on the new-device email When SMTP is enabled, a new-device login awaited the notification email before returning the HTTP response, so a slow SMTP server could exceed the clients' ~10s timeout and turn a successful login into an error. Send the email in the background (tokio::task::spawn) when require_device_email is disabled (the default), where it is best-effort; keep the awaited and failing path when it is enabled. send_new_device_logged_in now takes the device name and type by value so the spawned task can own its data (Device isn't Clone). Fixes #5856 --- src/api/identity.rs | 62 +++++++++++++++++++++++++++------------------ src/mail.rs | 14 +++++++--- 2 files changed, 47 insertions(+), 29 deletions(-) diff --git a/src/api/identity.rs b/src/api/identity.rs index 3962827d..bae0631d 100644 --- a/src/api/identity.rs +++ b/src/api/identity.rs @@ -1,4 +1,4 @@ -use chrono::Utc; +use chrono::{NaiveDateTime, Utc}; use num_traits::FromPrimitive; use rocket::{ Route, @@ -467,6 +467,40 @@ async fn password_login( authenticated_response(&user, &mut device, auth_tokens, twofactor_token, conn, ip).await } +// Notify the user that a new device logged in. +// +// When `require_device_email` is enabled this stays awaited, so a failure to send +// the notification aborts the login. When it is disabled (the default) the email +// is best-effort and is spawned onto a background task, so a slow SMTP server can +// no longer block (and thereby time out) the login response. See #5856. +async fn send_new_device_email(user: &User, device: &Device, now: NaiveDateTime, ip: &ClientIp) -> EmptyResult { + if CONFIG.require_device_email() { + if let Err(e) = + mail::send_new_device_logged_in(&user.email, &ip.ip.to_string(), &now, &device.name, device.atype).await + { + error!("Error sending new device email: {e:#?}"); + err!( + "Could not send login notification email. Please contact your administrator.", + ErrorEvent { + event: EventType::UserFailedLogIn + } + ) + } + } else { + let address = user.email.clone(); + let ip = ip.ip.to_string(); + let device_name = device.name.clone(); + let device_type = device.atype; + tokio::task::spawn(async move { + if let Err(e) = mail::send_new_device_logged_in(&address, &ip, &now, &device_name, device_type).await { + error!("Error sending new device email: {e:#?}"); + } + }); + } + + Ok(()) +} + async fn authenticated_response( user: &User, device: &mut Device, @@ -477,18 +511,7 @@ async fn authenticated_response( ) -> JsonResult { 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:#?}"); - - if CONFIG.require_device_email() { - err!( - "Could not send login notification email. Please contact your administrator.", - ErrorEvent { - event: EventType::UserFailedLogIn - } - ) - } - } + send_new_device_email(user, device, now, ip).await?; } // register push device @@ -625,18 +648,7 @@ async fn user_api_key_login( 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:#?}"); - - if CONFIG.require_device_email() { - err!( - "Could not send login notification email. Please contact your administrator.", - ErrorEvent { - event: EventType::UserFailedLogIn - } - ) - } - } + send_new_device_email(&user, &device, now, ip).await?; } // --- diff --git a/src/mail.rs b/src/mail.rs index f31234d7..bd0746cf 100644 --- a/src/mail.rs +++ b/src/mail.rs @@ -17,7 +17,7 @@ use crate::{ encode_jwt, generate_delete_claims, generate_emergency_access_invite_claims, generate_invite_claims, generate_verify_email_claims, }, - db::models::{Device, DeviceType, EmergencyAccessId, MembershipId, OrganizationId, User, UserId}, + db::models::{DeviceType, EmergencyAccessId, MembershipId, OrganizationId, User, UserId}, error::Error, util::upcase_first, }; @@ -505,7 +505,13 @@ pub async fn send_invite_confirmed(address: &str, org_name: &str) -> EmptyResult send_email(address, &subject, body_html, body_text).await } -pub async fn send_new_device_logged_in(address: &str, ip: &str, dt: &NaiveDateTime, device: &Device) -> EmptyResult { +pub async fn send_new_device_logged_in( + address: &str, + ip: &str, + dt: &NaiveDateTime, + device_name: &str, + device_type: i32, +) -> EmptyResult { let fmt = "%A, %B %_d, %Y at %r %Z"; let (subject, body_html, body_text) = get_text( "email/new_device_logged_in", @@ -513,8 +519,8 @@ pub async fn send_new_device_logged_in(address: &str, ip: &str, dt: &NaiveDateTi "url": CONFIG.domain(), "img_src": CONFIG._smtp_img_src(), "ip": ip, - "device_name": upcase_first(&device.name), - "device_type": DeviceType::from_i32(device.atype).to_string(), + "device_name": upcase_first(device_name), + "device_type": DeviceType::from_i32(device_type).to_string(), "datetime": crate::util::format_naive_datetime_local(dt, fmt), }), )?;