From 06a4f159954be82860bfdd9cca56d8dc1209064c Mon Sep 17 00:00:00 2001 From: Hocuri Date: Sat, 13 Jun 2020 13:33:58 +0200 Subject: [PATCH 1/8] Better warning if the pw is wrong --- src/imap/mod.rs | 6 ++---- src/scheduler.rs | 2 +- src/stock.rs | 4 +++- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/imap/mod.rs b/src/imap/mod.rs index 493e8bd0c..59970e4fd 100644 --- a/src/imap/mod.rs +++ b/src/imap/mod.rs @@ -303,10 +303,8 @@ impl Imap { .stock_string_repl_str(StockMessage::CannotLogin, &imap_user) .await; - emit_event!( - context, - Event::ErrorNetwork(format!("{} ({})", message, err)) - ); + error!(context, "{}", message); + self.trigger_reconnect(); Err(Error::LoginFailed(format!("cannot login as {}", imap_user))) } diff --git a/src/scheduler.rs b/src/scheduler.rs index 30aa731e6..121c383ee 100644 --- a/src/scheduler.rs +++ b/src/scheduler.rs @@ -122,7 +122,7 @@ async fn fetch_idle(ctx: &Context, connection: &mut Imap, folder: Config) -> Int Some(watch_folder) => { // connect and fake idle if unable to connect if let Err(err) = connection.connect_configured(&ctx).await { - error!(ctx, "imap connection failed: {}", err); + warn!(ctx, "imap connection failed: {}", err); return connection.fake_idle(&ctx, None).await; } diff --git a/src/stock.rs b/src/stock.rs index 9754b1d3a..84ef72f09 100644 --- a/src/stock.rs +++ b/src/stock.rs @@ -130,7 +130,9 @@ pub enum StockMessage { ))] AcSetupMsgBody = 43, - #[strum(props(fallback = "Cannot login as %1$s."))] + #[strum(props( + fallback = "Cannot login as \"%1$s\". Please check if the email address and the password are correct." + ))] CannotLogin = 60, #[strum(props(fallback = "Could not connect to %1$s: %2$s"))] From 3f2e67f07aaf5dce09949d25a305bd66e20b44a4 Mon Sep 17 00:00:00 2001 From: Hocuri Date: Sat, 13 Jun 2020 17:41:32 +0200 Subject: [PATCH 2/8] First try for notification --- src/chat.rs | 18 +++++++++++++++++- src/config.rs | 3 +++ src/configure/mod.rs | 1 + src/imap/mod.rs | 20 +++++++++++++++++++- 4 files changed, 40 insertions(+), 2 deletions(-) diff --git a/src/chat.rs b/src/chat.rs index f30714d0f..11184b5cd 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -2670,10 +2670,11 @@ pub(crate) async fn get_chat_id_by_grpid( /// Adds a message to device chat. /// /// Optional `label` can be provided to ensure that message is added only once. -pub async fn add_device_msg( +pub async fn add_device_msg_with_importance( context: &Context, label: Option<&str>, msg: Option<&mut Message>, + important: bool, ) -> Result { ensure!( label.is_some() || msg.is_some(), @@ -2697,7 +2698,14 @@ pub async fn add_device_msg( let rfc724_mid = dc_create_outgoing_rfc724_mid(None, "@device"); msg.try_calc_and_set_dimensions(context).await.ok(); prepare_msg_blob(context, msg).await?; + chat_id.unarchive(context).await?; + let muted = if important { + MuteDuration::NotMuted + } else { + MuteDuration::Forever + }; + set_muted(context, chat_id, muted).await?; context.sql.execute( "INSERT INTO msgs (chat_id,from_id,to_id, timestamp,type,state, txt,param,rfc724_mid) \ @@ -2739,6 +2747,14 @@ pub async fn add_device_msg( Ok(msg_id) } +pub async fn add_device_msg( + context: &Context, + label: Option<&str>, + msg: Option<&mut Message>, +) -> Result { + add_device_msg_with_importance(context, label, msg, false).await +} + pub async fn was_device_msg_ever_added(context: &Context, label: &str) -> Result { ensure!(!label.is_empty(), "empty label"); if let Ok(()) = context diff --git a/src/config.rs b/src/config.rs index 2c59d6435..1fd6016b7 100644 --- a/src/config.rs +++ b/src/config.rs @@ -117,6 +117,9 @@ pub enum Config { #[strum(serialize = "sys.config_keys")] SysConfigKeys, + + #[strum(props(default = "0"))] + WarnedAboutWrongPw, } impl Context { diff --git a/src/configure/mod.rs b/src/configure/mod.rs index 7b45e5386..d76e410ab 100644 --- a/src/configure/mod.rs +++ b/src/configure/mod.rs @@ -102,6 +102,7 @@ impl Context { match success { Ok(_) => { + self.set_config(Config::WarnedAboutWrongPw, None).await?; progress!(self, 1000); Ok(()) } diff --git a/src/imap/mod.rs b/src/imap/mod.rs index 59970e4fd..85cf92a85 100644 --- a/src/imap/mod.rs +++ b/src/imap/mod.rs @@ -30,7 +30,7 @@ use crate::mimeparser; use crate::oauth2::dc_get_oauth2_access_token; use crate::param::Params; use crate::provider::get_provider_info; -use crate::{scheduler::InterruptInfo, stock::StockMessage}; +use crate::{chat, scheduler::InterruptInfo, stock::StockMessage}; mod client; mod idle; @@ -38,6 +38,7 @@ pub mod select_folder; mod session; use client::Client; +use message::Message; use session::Session; type Result = std::result::Result; @@ -118,6 +119,7 @@ pub struct Imap { connected: bool, interrupt: Option, should_reconnect: bool, + login_failed_once: bool, } #[derive(Debug)] @@ -191,6 +193,7 @@ impl Imap { connected: Default::default(), interrupt: Default::default(), should_reconnect: Default::default(), + login_failed_once: Default::default(), } } @@ -295,8 +298,10 @@ impl Imap { // needs to be set here to ensure it is set on reconnects. self.connected = true; self.session = Some(session); + self.login_failed_once = false; Ok(()) } + Err((err, _)) => { let imap_user = self.config.imap_user.to_owned(); let message = context @@ -304,6 +309,19 @@ impl Imap { .await; error!(context, "{}", message); + if self.login_failed_once + && self.fetch + && !context.get_config_bool(Config::WarnedAboutWrongPw).await + { + context + .set_config(Config::WarnedAboutWrongPw, Some("1")) + .await; + let mut msg = Message::new(Viewtype::Text); + msg.text = Some(message); + chat::add_device_msg_with_importance(context, None, Some(&mut msg), true).await; + } else { + self.login_failed_once = true; + } self.trigger_reconnect(); Err(Error::LoginFailed(format!("cannot login as {}", imap_user))) From 2c23433185c8247d1ca28924b3c449163d4247f4 Mon Sep 17 00:00:00 2001 From: Hocuri Date: Sat, 13 Jun 2020 19:45:59 +0200 Subject: [PATCH 3/8] Make it work Add a mutex to prevent a race condition when a "your pw is wrong" warning is sent, resulting in multiple messeges being sent. Do not mute the device chat but "only" send MsgsChanged event when no notification shall be shown. More logging. --- src/chat.rs | 13 +++++-------- src/context.rs | 3 +++ src/imap/mod.rs | 20 ++++++++++++++++---- 3 files changed, 24 insertions(+), 12 deletions(-) diff --git a/src/chat.rs b/src/chat.rs index 11184b5cd..fd4a2807c 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -2698,14 +2698,7 @@ pub async fn add_device_msg_with_importance( let rfc724_mid = dc_create_outgoing_rfc724_mid(None, "@device"); msg.try_calc_and_set_dimensions(context).await.ok(); prepare_msg_blob(context, msg).await?; - chat_id.unarchive(context).await?; - let muted = if important { - MuteDuration::NotMuted - } else { - MuteDuration::Forever - }; - set_muted(context, chat_id, muted).await?; context.sql.execute( "INSERT INTO msgs (chat_id,from_id,to_id, timestamp,type,state, txt,param,rfc724_mid) \ @@ -2741,7 +2734,11 @@ pub async fn add_device_msg_with_importance( } if !msg_id.is_unset() { - context.emit_event(Event::IncomingMsg { chat_id, msg_id }); + if important { + context.emit_event(Event::IncomingMsg { chat_id, msg_id }); + } else { + context.emit_event(Event::MsgsChanged { chat_id, msg_id }); + } } Ok(msg_id) diff --git a/src/context.rs b/src/context.rs index 016c27d4f..5458282bf 100644 --- a/src/context.rs +++ b/src/context.rs @@ -53,6 +53,8 @@ pub struct InnerContext { pub(crate) generating_key_mutex: Mutex<()>, /// Mutex to enforce only a single running oauth2 is running. pub(crate) oauth2_mutex: Mutex<()>, + /// Mutex to prevent a race condition when a "your pw is wrong" warning is sent, resulting in multiple messeges being sent. + pub(crate) wrong_pw_warning_mutex: Mutex<()>, pub(crate) translated_stockstrings: RwLock>, pub(crate) events: Events, @@ -120,6 +122,7 @@ impl Context { last_smeared_timestamp: RwLock::new(0), generating_key_mutex: Mutex::new(()), oauth2_mutex: Mutex::new(()), + wrong_pw_warning_mutex: Mutex::new(()), translated_stockstrings: RwLock::new(HashMap::new()), events: Events::default(), scheduler: RwLock::new(Scheduler::Stopped), diff --git a/src/imap/mod.rs b/src/imap/mod.rs index 85cf92a85..e515d31a4 100644 --- a/src/imap/mod.rs +++ b/src/imap/mod.rs @@ -308,17 +308,29 @@ impl Imap { .stock_string_repl_str(StockMessage::CannotLogin, &imap_user) .await; + warn!(context, "{} ({})", message, err); error!(context, "{}", message); + + let lock = context.wrong_pw_warning_mutex.lock().await; if self.login_failed_once - && self.fetch && !context.get_config_bool(Config::WarnedAboutWrongPw).await { - context + if let Err(e) = context .set_config(Config::WarnedAboutWrongPw, Some("1")) - .await; + .await + { + warn!(context, "{}", e); + } + drop(lock); + let mut msg = Message::new(Viewtype::Text); msg.text = Some(message); - chat::add_device_msg_with_importance(context, None, Some(&mut msg), true).await; + if let Err(e) = + chat::add_device_msg_with_importance(context, None, Some(&mut msg), true) + .await + { + warn!(context, "{}", e); + } } else { self.login_failed_once = true; } From ae2fd4014aecc4806a8ec742c6f2b78e12994a03 Mon Sep 17 00:00:00 2001 From: Hocuri Date: Sat, 13 Jun 2020 20:04:57 +0200 Subject: [PATCH 4/8] Emit Event::ErrorNetwork again --- src/imap/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/imap/mod.rs b/src/imap/mod.rs index e515d31a4..0b386bf61 100644 --- a/src/imap/mod.rs +++ b/src/imap/mod.rs @@ -309,7 +309,7 @@ impl Imap { .await; warn!(context, "{} ({})", message, err); - error!(context, "{}", message); + emit_event!(context, Event::ErrorNetwork(message.clone())); let lock = context.wrong_pw_warning_mutex.lock().await; if self.login_failed_once From 86bc54508f2eec03812e1051d95ea13061bbc290 Mon Sep 17 00:00:00 2001 From: Hocuri Date: Sat, 13 Jun 2020 20:26:16 +0200 Subject: [PATCH 5/8] More explicit --- src/chat.rs | 1 + src/config.rs | 4 +++- src/configure/mod.rs | 4 +++- src/imap/mod.rs | 7 ++----- 4 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/chat.rs b/src/chat.rs index fd4a2807c..928c781f1 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -2670,6 +2670,7 @@ pub(crate) async fn get_chat_id_by_grpid( /// Adds a message to device chat. /// /// Optional `label` can be provided to ensure that message is added only once. +/// If `important` is true, a notification will be sent. pub async fn add_device_msg_with_importance( context: &Context, label: Option<&str>, diff --git a/src/config.rs b/src/config.rs index 1fd6016b7..ddb266e7b 100644 --- a/src/config.rs +++ b/src/config.rs @@ -119,7 +119,9 @@ pub enum Config { SysConfigKeys, #[strum(props(default = "0"))] - WarnedAboutWrongPw, + /// 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) + NotifyAboutWrongPw, } impl Context { diff --git a/src/configure/mod.rs b/src/configure/mod.rs index d76e410ab..89bf858eb 100644 --- a/src/configure/mod.rs +++ b/src/configure/mod.rs @@ -72,6 +72,7 @@ impl Context { let mut param = LoginParam::from_database(self, "").await; let success = configure(self, &mut param).await; + self.set_config(Config::NotifyAboutWrongPw, None).await?; if let Some(provider) = provider::get_provider_info(¶m.addr) { if let Some(config_defaults) = &provider.config_defaults { @@ -102,7 +103,8 @@ impl Context { match success { Ok(_) => { - self.set_config(Config::WarnedAboutWrongPw, None).await?; + self.set_config(Config::NotifyAboutWrongPw, Some("1")) + .await?; progress!(self, 1000); Ok(()) } diff --git a/src/imap/mod.rs b/src/imap/mod.rs index 0b386bf61..b6b5981b3 100644 --- a/src/imap/mod.rs +++ b/src/imap/mod.rs @@ -313,12 +313,9 @@ impl Imap { let lock = context.wrong_pw_warning_mutex.lock().await; if self.login_failed_once - && !context.get_config_bool(Config::WarnedAboutWrongPw).await + && context.get_config_bool(Config::NotifyAboutWrongPw).await { - if let Err(e) = context - .set_config(Config::WarnedAboutWrongPw, Some("1")) - .await - { + if let Err(e) = context.set_config(Config::NotifyAboutWrongPw, None).await { warn!(context, "{}", e); } drop(lock); From 68e3bce60eaef48a6fef0e63241c2a8453ef65de Mon Sep 17 00:00:00 2001 From: Hocuri Date: Sun, 14 Jun 2020 18:56:45 +0200 Subject: [PATCH 6/8] Remove error!() from https://github.com/deltachat/deltachat-core-rust/pull/1539 it led to a less clear error message being shown when the configure failed. --- src/configure/mod.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/configure/mod.rs b/src/configure/mod.rs index 89bf858eb..8be3d7284 100644 --- a/src/configure/mod.rs +++ b/src/configure/mod.rs @@ -109,7 +109,6 @@ impl Context { Ok(()) } Err(err) => { - error!(self, "Configure Failed: {}", err); progress!(self, 0); Err(err) } From 9f7567c1d19c9573fd9da990bd0ad8b74c5f7294 Mon Sep 17 00:00:00 2001 From: Hocuri Date: Mon, 15 Jun 2020 14:48:14 +0200 Subject: [PATCH 7/8] Abort on failing imap configuring --- src/configure/mod.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/configure/mod.rs b/src/configure/mod.rs index 8be3d7284..0a17d3759 100644 --- a/src/configure/mod.rs +++ b/src/configure/mod.rs @@ -461,7 +461,10 @@ async fn try_imap_connections( .await .is_ok() { - return Ok(()); // we directly return here if it was autoconfig or the connection succeeded + return Ok(()); + } + if was_autoconfig { + bail!("autoconfig did not succeed"); } progress!(context, 670); @@ -495,7 +498,7 @@ async fn try_imap_connection( return Ok(()); } if was_autoconfig { - return Ok(()); + bail!("autoconfig did not succeed"); } progress!(context, 650 + variation * 30); From b50410ab1597a2a2cdc1d23d8ad30a67204ff3d4 Mon Sep 17 00:00:00 2001 From: Hocuri Date: Sat, 11 Jul 2020 17:38:48 +0200 Subject: [PATCH 8/8] Fix #1687 --- src/configure/mod.rs | 2 +- src/smtp/mod.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/configure/mod.rs b/src/configure/mod.rs index 0a17d3759..bd3a2b4d9 100644 --- a/src/configure/mod.rs +++ b/src/configure/mod.rs @@ -558,7 +558,7 @@ async fn try_smtp_connections( return Ok(()); } if was_autoconfig { - return Ok(()); + bail!("No SMTP connection"); } progress!(context, 850); diff --git a/src/smtp/mod.rs b/src/smtp/mod.rs index 9bdc69235..6d223bc7d 100644 --- a/src/smtp/mod.rs +++ b/src/smtp/mod.rs @@ -181,7 +181,7 @@ impl Smtp { .stock_string_repl_str2( StockMessage::ServerResponse, format!("SMTP {}:{}", domain, port), - format!("{}, ({:?})", err.to_string(), err), + err.to_string(), ) .await;