Browse Source

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
pull/7381/head
max 3 weeks ago
parent
commit
055cb61888
  1. 62
      src/api/identity.rs
  2. 14
      src/mail.rs

62
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?;
}
// ---

14
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),
}),
)?;

Loading…
Cancel
Save