From c6e321e6b581bcbc8daede4471936743bb3b5526 Mon Sep 17 00:00:00 2001 From: link2xt Date: Mon, 24 Aug 2026 17:20:58 +0000 Subject: [PATCH] feat: do not create device messages for IMAP authentication errors Authentication failures may happen because of internal server errors. Device message saying "Please check if the email address and the password are correct" was written for classic email setups when the user knows the password. For users of chatmail relays this message is not actionable, but still appears when relay fails to check the password. --- deltachat-ffi/deltachat.h | 5 +-- python/tests/test_1_online.py | 10 +++--- src/config.rs | 6 ---- src/configure.rs | 5 +-- src/context.rs | 3 -- src/context/context_tests.rs | 1 - src/imap.rs | 58 +++++------------------------------ src/stock_str.rs | 10 ------ 8 files changed, 14 insertions(+), 84 deletions(-) diff --git a/deltachat-ffi/deltachat.h b/deltachat-ffi/deltachat.h index 721882842f..a60514c4be 100644 --- a/deltachat-ffi/deltachat.h +++ b/deltachat-ffi/deltachat.h @@ -6672,10 +6672,7 @@ void dc_event_unref(dc_event_t* event); /// Used as the name for the corresponding chatlist entry. #define DC_STR_ARCHIVEDCHATS 40 -/// "Cannot login as %1$s." -/// -/// Used in error strings. -/// - %1$s will be replaced by the failing login name +/// @deprecated 2026-08-24 #define DC_STR_CANNOT_LOGIN 60 /// "Location streaming enabled." diff --git a/python/tests/test_1_online.py b/python/tests/test_1_online.py index 04957d98bc..d88746ea9c 100644 --- a/python/tests/test_1_online.py +++ b/python/tests/test_1_online.py @@ -1133,8 +1133,9 @@ def test_configure_error_msgs_wrong_pw(acfactory): print(f"Configuration progress: {ev.data1}") if ev.data1 == 0: break - # Password is wrong so it definitely has to say something about "password" - assert "password" in ev.data2 + # Password is wrong so the error should be about authentication + # and not e.g. connection failure. + assert "Authentication" in ev.data2 ac1.stop_io() ac1.set_config("mail_pw", "abc") # Wrong mail pw @@ -1144,10 +1145,7 @@ def test_configure_error_msgs_wrong_pw(acfactory): print(f"Configuration progress: {ev.data1}") if ev.data1 == 0: break - assert "password" in ev.data2 - # Account will continue to work with the old password, so if it becomes wrong, a notification - # must be shown. - assert ac1.get_config("notify_about_wrong_pw") == "1" + assert "Authentication" in ev.data2 def test_configure_error_msgs_invalid_server(acfactory): diff --git a/src/config.rs b/src/config.rs index 8cf59b025a..5241cf0fbc 100644 --- a/src/config.rs +++ b/src/config.rs @@ -343,11 +343,6 @@ pub enum Config { #[strum(props(default = "0"))] SkipStartMessages, - /// Whether we send a warning if the password is wrong (set to false when we send a warning - /// because we do not want to send a second warning) - #[strum(props(default = "0"))] - NotifyAboutWrongPw, - /// Timestamp of the last time housekeeping was run LastHousekeeping, @@ -663,7 +658,6 @@ impl Context { | Config::MdnsEnabled | Config::Configured | Config::Bot - | Config::NotifyAboutWrongPw | Config::SyncMsgs | Config::DisableIdle => { ensure!( diff --git a/src/configure.rs b/src/configure.rs index faca48b837..6bf0bf6e61 100644 --- a/src/configure.rs +++ b/src/configure.rs @@ -334,8 +334,6 @@ impl Context { ); return Err(error); }; - self.set_config_internal(Config::NotifyAboutWrongPw, Some("1")) - .await?; if provider::legacy_settings_for_addr(¶m.addr)?.worse_media_quality && !self.config_exists(Config::MediaQuality).await? { @@ -560,8 +558,7 @@ pub(crate) async fn configure( let transport_id = 0; let (_s, r) = async_channel::bounded(1); let mut imap = Imap::new(ctx, transport_id, configured_param.clone(), r).await?; - let configuring = true; - let imap_session = match imap.connect(ctx, configuring).await { + let imap_session = match imap.connect(ctx).await { Ok(imap_session) => imap_session, Err(err) => { bail!("{}", nicer_configuration_error(ctx, format!("{err:#}"))); diff --git a/src/context.rs b/src/context.rs index fc50fa2c06..85eacae0ac 100644 --- a/src/context.rs +++ b/src/context.rs @@ -231,8 +231,6 @@ pub struct InnerContext { /// This is a global mutex-like state for operations which should be modal in the /// clients. running_state: RwLock, - /// Mutex to prevent a race condition when a "your pw is wrong" warning is sent, resulting in multiple messages being sent. - pub(crate) wrong_pw_warning_mutex: Mutex<()>, /// Mutex to prevent running housekeeping or relay management from multiple threads at once. pub(crate) background_task_mutex: Mutex<()>, @@ -486,7 +484,6 @@ impl Context { blobdir, running_state: RwLock::new(Default::default()), sql: Sql::new(dbfile), - wrong_pw_warning_mutex: Mutex::new(()), background_task_mutex: Mutex::new(()), fetch_msgs_mutex: Mutex::new(()), translated_stockstrings: stockstrings, diff --git a/src/context/context_tests.rs b/src/context/context_tests.rs index c372552b44..c6c5e33c24 100644 --- a/src/context/context_tests.rs +++ b/src/context/context_tests.rs @@ -278,7 +278,6 @@ async fn test_get_info_completeness() { "mail_pw", "mail_port", "mail_security", - "notify_about_wrong_pw", "selfstatus", "send_server", "send_user", diff --git a/src/imap.rs b/src/imap.rs index c211b8b3d6..fd4be4f31f 100644 --- a/src/imap.rs +++ b/src/imap.rs @@ -28,7 +28,6 @@ use crate::context::Context; use crate::ensure_and_debug_assert; use crate::events::EventType; use crate::headerdef::{HeaderDef, HeaderDefMap}; -use crate::log::LogExt; use crate::log::warn; use crate::message::{self, Message}; use crate::mimeparser; @@ -37,7 +36,6 @@ use crate::net::session::SessionStream; use crate::push::encrypt_device_token; use crate::receive_imf::{ReceivedMsg, from_field_to_contact_id, receive_imf_inner}; use crate::scheduler::connectivity::ConnectivityStore; -use crate::stock_str; use crate::tools::{self, create_id, duration_to_str, time}; use crate::transport::{ ConfiguredLoginParam, ConfiguredServerLoginParam, prioritize_server_login_params, @@ -87,8 +85,6 @@ pub(crate) struct Imap { /// Watched folder. pub(crate) folder: String, - authentication_failed_once: bool, - pub(crate) connectivity: ConnectivityStore, conn_last_try: tools::Time, @@ -234,7 +230,6 @@ impl Imap { proxy_config, strict_tls, folder, - authentication_failed_once: false, connectivity: Default::default(), conn_last_try: UNIX_EPOCH, conn_backoff_ms: 0, @@ -267,11 +262,7 @@ impl Imap { /// Calling this function is not enough to perform IMAP operations. Use [`Imap::prepare`] /// instead if you are going to actually use connection rather than trying connection /// parameters. - pub(crate) async fn connect( - &mut self, - context: &Context, - configuring: bool, - ) -> Result { + pub(crate) async fn connect(&mut self, context: &Context) -> Result { let now = tools::Time::now(); let until_can_send = max( min(self.conn_last_try, now) @@ -342,7 +333,10 @@ impl Imap { let imap_pw: &str = &self.password; info!(context, "Logging into IMAP server with LOGIN."); - let login_res = client.login(imap_user, imap_pw).await; + let login_res = client + .login(imap_user, imap_pw) + .await + .with_context(|| format!("IMAP failed to login as {imap_user}")); match login_res { Ok((mut session, login_capabilities_opt)) => { @@ -395,7 +389,6 @@ impl Imap { let mut lock = context.server_id.write().await; lock.clone_from(&session.capabilities.server_id); - self.authentication_failed_once = false; context.emit_event(EventType::ImapConnected(format!( "IMAP-LOGIN as {}", lp.user @@ -406,42 +399,8 @@ impl Imap { } Err(err) => { - let imap_user = lp.user.to_owned(); - let message = stock_str::cannot_login(context, &imap_user); - - warn!(context, "IMAP failed to login: {err:#}."); - first_error.get_or_insert(format_err!("{message} ({err:#})")); - - // If it looks like the password is wrong, send a notification: - let _lock = context.wrong_pw_warning_mutex.lock().await; - if err.to_string().to_lowercase().contains("authentication") { - if self.authentication_failed_once - && !configuring - && context.get_config_bool(Config::NotifyAboutWrongPw).await? - { - let mut msg = Message::new_text(message); - if let Err(e) = chat::add_device_msg_with_importance( - context, - None, - Some(&mut msg), - true, - ) - .await - { - warn!(context, "Failed to add device message: {e:#}."); - } else { - context - .set_config_internal(Config::NotifyAboutWrongPw, None) - .await - .log_err(context) - .ok(); - } - } else { - self.authentication_failed_once = true; - } - } else { - self.authentication_failed_once = false; - } + warn!(context, "{err:#}."); + first_error.get_or_insert(err); } } } @@ -454,8 +413,7 @@ impl Imap { /// This creates a new IMAP connection and ensures /// that folders are created and IMAP capabilities are determined. pub(crate) async fn prepare(&mut self, context: &Context) -> Result { - let configuring = false; - let session = match self.connect(context, configuring).await { + let session = match self.connect(context).await { Ok(session) => session, Err(err) => { self.connectivity.set_err(context, format!("{err:#}")); diff --git a/src/stock_str.rs b/src/stock_str.rs index b6ffdc60d9..6f2ccc1853 100644 --- a/src/stock_str.rs +++ b/src/stock_str.rs @@ -75,11 +75,6 @@ pub enum StockMessage { #[strum(props(fallback = "Archived chats"))] ArchivedChats = 40, - #[strum(props( - fallback = "Cannot login as \"%1$s\". Please check if the email address and the password are correct." - ))] - CannotLogin = 60, - #[strum(props(fallback = "Location streaming enabled."))] MsgLocationEnabled = 64, @@ -903,11 +898,6 @@ pub(crate) fn sync_msg_body(context: &Context) -> String { translated(context, StockMessage::SyncMsgBody) } -/// Stock string: `Cannot login as \"%1$s\". Please check...`. -pub(crate) fn cannot_login(context: &Context, user: &str) -> String { - translated(context, StockMessage::CannotLogin).replace1(user) -} - /// Stock string: `Location streaming enabled.`. pub(crate) fn msg_location_enabled(context: &Context) -> String { translated(context, StockMessage::MsgLocationEnabled)