Skip to content

Commit

Permalink
fix: Set Config::NotifyAboutWrongPw before saving configuration (#5896)
Browse files Browse the repository at this point in the history
Let's not complicate the logic and always set `Config::NotifyAboutWrongPw` before saving
configuration, better if a wrong password notification is shown once more than not shown at all. It
shouldn't be a big problem because reconfiguration is a manual action and isn't done frequently.

Also for the same reason reset `Config::NotifyAboutWrongPw` only after a successful addition of the
appropriate device message.
  • Loading branch information
iequidoo committed Sep 17, 2024
1 parent afb01e3 commit 3e32980
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 21 deletions.
13 changes: 13 additions & 0 deletions python/tests/test_1_online.py
Original file line number Diff line number Diff line change
Expand Up @@ -2201,6 +2201,19 @@ def test_configure_error_msgs_wrong_pw(acfactory):
# Password is wrong so it definitely has to say something about "password"
assert "password" in ev.data2

ac1.stop_io()
ac1.set_config("mail_pw", "abc") # Wrong mail pw
ac1.configure()
while True:
ev = ac1._evtracker.get_matching("DC_EVENT_CONFIGURE_PROGRESS")
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.
ac1.get_config("notify_about_wrong_pw") == "1"


def test_configure_error_msgs_invalid_server(acfactory):
ac2 = acfactory.get_unconfigured_account()
Expand Down
10 changes: 3 additions & 7 deletions src/configure.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,15 +114,10 @@ impl Context {

let param = EnteredLoginParam::load(self).await?;
let old_addr = self.get_config(Config::ConfiguredAddr).await?;

let configured_param_res = configure(self, &param).await;
self.set_config_internal(Config::NotifyAboutWrongPw, None)
.await?;

on_configure_completed(self, configured_param_res?, old_addr).await?;

let configured_param = configure(self, &param).await?;
self.set_config_internal(Config::NotifyAboutWrongPw, Some("1"))
.await?;
on_configure_completed(self, configured_param, old_addr).await?;
Ok(())
}
}
Expand Down Expand Up @@ -420,6 +415,7 @@ async fn configure(ctx: &Context, param: &EnteredLoginParam) -> Result<Configure
configured_param.oauth2,
r,
);
imap.login_failed_once = None;
let mut imap_session = match imap.connect(ctx).await {
Ok(session) => session,
Err(err) => bail!("{}", nicer_configuration_error(ctx, err.to_string()).await),
Expand Down
27 changes: 13 additions & 14 deletions src/imap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ use crate::contact::{Contact, ContactId, Modifier, Origin};
use crate::context::Context;
use crate::events::EventType;
use crate::headerdef::{HeaderDef, HeaderDefMap};
use crate::log::LogExt;
use crate::login_param::{
prioritize_server_login_params, ConfiguredLoginParam, ConfiguredServerLoginParam,
};
Expand Down Expand Up @@ -87,7 +88,7 @@ pub(crate) struct Imap {

oauth2: bool,

login_failed_once: bool,
pub(crate) login_failed_once: Option<bool>,

pub(crate) connectivity: ConnectivityStore,

Expand Down Expand Up @@ -252,7 +253,7 @@ impl Imap {
proxy_config,
strict_tls,
oauth2,
login_failed_once: false,
login_failed_once: Some(false),
connectivity: Default::default(),
conn_last_try: UNIX_EPOCH,
conn_backoff_ms: 0,
Expand Down Expand Up @@ -380,7 +381,7 @@ impl Imap {
let mut lock = context.server_id.write().await;
lock.clone_from(&session.capabilities.server_id);

self.login_failed_once = false;
self.login_failed_once = self.login_failed_once.map(|_| false);
context.emit_event(EventType::ImapConnected(format!(
"IMAP-LOGIN as {}",
lp.user
Expand All @@ -398,19 +399,11 @@ impl Imap {
warn!(context, "IMAP failed to login: {err:#}.");
first_error.get_or_insert(format_err!("{message} ({err:#})"));

let lock = context.wrong_pw_warning_mutex.lock().await;
if self.login_failed_once
let _lock = context.wrong_pw_warning_mutex.lock().await;
if self.login_failed_once.unwrap_or(false)
&& err_str.to_lowercase().contains("authentication")
&& context.get_config_bool(Config::NotifyAboutWrongPw).await?
{
if let Err(e) = context
.set_config_internal(Config::NotifyAboutWrongPw, None)
.await
{
warn!(context, "{e:#}.");
}
drop(lock);

let mut msg = Message::new(Viewtype::Text);
msg.text.clone_from(&message);
if let Err(e) = chat::add_device_msg_with_importance(
Expand All @@ -422,9 +415,15 @@ impl Imap {
.await
{
warn!(context, "Failed to add device message: {e:#}.");
} else {
context
.set_config_internal(Config::NotifyAboutWrongPw, None)
.await
.log_err(context)
.ok();
}
} else {
self.login_failed_once = true;
self.login_failed_once = self.login_failed_once.map(|_| true);
}
}
}
Expand Down

0 comments on commit 3e32980

Please sign in to comment.