[PR #7381] Don't block the login response on the new-device email #37072

New Issue

GiteaMirror · 2026-07-13T21:34:53-05:00

GiteaMirror commented

2026-07-13 21:34:53 -05:00

📋 Pull Request Information

Original PR: https://github.com/dani-garcia/vaultwarden/pull/7381
Author: @maximilize
Created: 6/28/2026
Status: 🔄 Open

Base: main ← Head: fix/5856-new-device-email-async

📝 Commits (1)

055cb61 Don't block the login response on the new-device email

📊 Changes

2 files changed (+47 additions, -29 deletions)

View changed files

📝 src/api/identity.rs (+37 -25)
📝 src/mail.rs (+10 -4)

📄 Description

Fixes #5856.

When a new device logs in and SMTP is enabled, authenticated_response (and user_api_key_login) awaited send_new_device_logged_in before building the HTTP response. A slow SMTP server therefore delays the login response, and since the Bitwarden clients time out at ~10s (Android, empirically), slow mail turns a successful login into a client-side error / very slow first login.

The notification only needs to block when require_device_email is enabled — there a send failure must abort the login. When it's disabled (the default) the email is best-effort (the error was already only logged), so there's no reason to make the user wait on it.

This adds a send_new_device_email helper that:

keeps the existing awaited + err! path when require_device_email is enabled, and
spawns the email on a background task (tokio::task::spawn, mirroring the fire-and-forget pattern already used for push in push.rs) when it's disabled, so a slow SMTP server can no longer block the login response.

send_new_device_logged_in now takes the device name + type by value instead of &Device, so the spawned task can own everything it needs (Device isn't Clone). Both call sites — authenticated_response (which covers password + SSO login) and user_api_key_login — go through the helper. Logging behavior in the best-effort branch is unchanged (the error is still error!-logged, now from within the task).

Validated with cargo fmt --check, cargo clippy --features sqlite -- -D warnings, and cargo test --features sqlite in a clean rust:1.96 container.

Note: I didn't add an automated test for "response returns before SMTP completes" — the existing suite has no harness for the login HTTP path against a slow mail server. Verification is by code review plus the existing tests staying green; the change is a contained await→spawn for the best-effort branch only.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/dani-garcia/vaultwarden/pull/7381 **Author:** [@maximilize](https://github.com/maximilize) **Created:** 6/28/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `fix/5856-new-device-email-async` --- ### 📝 Commits (1) - [`055cb61`](https://github.com/dani-garcia/vaultwarden/commit/055cb618885276c39e93c80df69cf5933b723ba9) Don't block the login response on the new-device email ### 📊 Changes **2 files changed** (+47 additions, -29 deletions) <details> <summary>View changed files</summary> 📝 `src/api/identity.rs` (+37 -25) 📝 `src/mail.rs` (+10 -4) </details> ### 📄 Description Fixes #5856. When a new device logs in and SMTP is enabled, `authenticated_response` (and `user_api_key_login`) awaited `send_new_device_logged_in` before building the HTTP response. A slow SMTP server therefore delays the login response, and since the Bitwarden clients time out at ~10s (Android, empirically), slow mail turns a successful login into a client-side error / very slow first login. The notification only needs to block when `require_device_email` is enabled — there a send failure must abort the login. When it's disabled (the default) the email is best-effort (the error was already only logged), so there's no reason to make the user wait on it. This adds a `send_new_device_email` helper that: - keeps the existing awaited + `err!` path when `require_device_email` is enabled, and - spawns the email on a background task (`tokio::task::spawn`, mirroring the fire-and-forget pattern already used for push in `push.rs`) when it's disabled, so a slow SMTP server can no longer block the login response. `send_new_device_logged_in` now takes the device name + type by value instead of `&Device`, so the spawned task can own everything it needs (`Device` isn't `Clone`). Both call sites — `authenticated_response` (which covers password + SSO login) and `user_api_key_login` — go through the helper. Logging behavior in the best-effort branch is unchanged (the error is still `error!`-logged, now from within the task). Validated with `cargo fmt --check`, `cargo clippy --features sqlite -- -D warnings`, and `cargo test --features sqlite` in a clean `rust:1.96` container. Note: I didn't add an automated test for "response returns before SMTP completes" — the existing suite has no harness for the login HTTP path against a slow mail server. Verification is by code review plus the existing tests staying green; the change is a contained await→spawn for the best-effort branch only. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

GiteaMirror added the pull-request label 2026-07-13 21:34:53 -05:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: github-starred/vaultwarden#37072