From ec3f7657279b06aa389e0f56b230201009136b1d Mon Sep 17 00:00:00 2001 From: link2xt Date: Thu, 23 Oct 2025 05:27:57 +0000 Subject: [PATCH] refactor(add_transport_from_qr): do not set deprecated config values --- src/configure.rs | 18 +++--- src/qr.rs | 41 +++++++++----- src/qr/dclogin_scheme.rs | 107 ++++++++---------------------------- src/qr/qr_tests.rs | 115 +++++++++++++-------------------------- 4 files changed, 95 insertions(+), 186 deletions(-) diff --git a/src/configure.rs b/src/configure.rs index 91aff2c5c..c048a7c56 100644 --- a/src/configure.rs +++ b/src/configure.rs @@ -36,6 +36,7 @@ use crate::login_param::{ use crate::message::Message; use crate::oauth2::get_oauth2_addr; use crate::provider::{Protocol, Provider, Socket, UsernamePattern}; +use crate::qr::{login_param_from_account_qr, login_param_from_login_qr}; use crate::smtp::Smtp; use crate::sync::Sync::*; use crate::tools::time; @@ -117,7 +118,7 @@ impl Context { Ok(()) } - async fn add_transport_inner(&self, param: &mut EnteredLoginParam) -> Result<()> { + pub(crate) async fn add_transport_inner(&self, param: &mut EnteredLoginParam) -> Result<()> { ensure!( !self.scheduler.is_running().await, "cannot configure, already running" @@ -162,20 +163,15 @@ impl Context { pub async fn add_transport_from_qr(&self, qr: &str) -> Result<()> { self.stop_io().await; - // This code first sets the deprecated Config::Addr, Config::MailPw, etc. - // and then calls configure(), which loads them again. - // At some point, we will remove configure() - // and then simplify the code - // to directly create an EnteredLoginParam. let result = async move { - match crate::qr::check_qr(self, qr).await? { - crate::qr::Qr::Account { .. } => crate::qr::set_account_from_qr(self, qr).await?, + let mut param = match crate::qr::check_qr(self, qr).await? { + crate::qr::Qr::Account { .. } => login_param_from_account_qr(self, qr).await?, crate::qr::Qr::Login { address, options } => { - crate::qr::configure_from_login_qr(self, &address, options).await? + login_param_from_login_qr(&address, options)? } _ => bail!("QR code does not contain account"), - } - self.configure().await?; + }; + self.add_transport_inner(&mut param).await?; Ok(()) } .await; diff --git a/src/qr.rs b/src/qr.rs index aa2cfc19f..a1b780ec3 100644 --- a/src/qr.rs +++ b/src/qr.rs @@ -6,16 +6,17 @@ use std::sync::LazyLock; use anyhow::{Context as _, Result, anyhow, bail, ensure}; pub use dclogin_scheme::LoginOptions; +pub(crate) use dclogin_scheme::login_param_from_login_qr; use deltachat_contact_tools::{ContactAddress, addr_normalize, may_be_valid_addr}; use percent_encoding::{NON_ALPHANUMERIC, percent_decode_str, percent_encode}; use serde::Deserialize; -pub(crate) use self::dclogin_scheme::configure_from_login_qr; use crate::config::Config; use crate::contact::{Contact, ContactId, Origin}; use crate::context::Context; use crate::events::EventType; use crate::key::Fingerprint; +use crate::login_param::{EnteredCertificateChecks, EnteredLoginParam, EnteredServerLoginParam}; use crate::net::http::post_empty; use crate::net::proxy::{DEFAULT_SOCKS_PORT, ProxyConfig}; use crate::token; @@ -651,10 +652,13 @@ struct CreateAccountErrorResponse { reason: String, } -/// take a qr of the type DC_QR_ACCOUNT, parse it's parameters, -/// download additional information from the contained url and set the parameters. -/// on success, a configure::configure() should be able to log in to the account -pub(crate) async fn set_account_from_qr(context: &Context, qr: &str) -> Result<()> { +/// Takes a QR with `DCACCOUNT:` scheme, parses its parameters, +/// downloads additional information from the contained URL +/// and returns the login parameters. +pub(crate) async fn login_param_from_account_qr( + context: &Context, + qr: &str, +) -> Result { let url_str = qr .get(DCACCOUNT_SCHEME.len()..) .context("Invalid DCACCOUNT scheme")?; @@ -669,14 +673,19 @@ pub(crate) async fn set_account_from_qr(context: &Context, qr: &str) -> Result<( .with_context(|| { format!("Cannot create account, response is malformed:\n{response_text:?}") })?; - context - .set_config_internal(Config::Addr, Some(&email)) - .await?; - context - .set_config_internal(Config::MailPw, Some(&password)) - .await?; - Ok(()) + let param = EnteredLoginParam { + addr: email, + imap: EnteredServerLoginParam { + password, + ..Default::default() + }, + smtp: Default::default(), + certificate_checks: EnteredCertificateChecks::Strict, + oauth2: false, + }; + + Ok(param) } else { match serde_json::from_str::(&response_text) { Ok(error) => Err(anyhow!(error.reason)), @@ -693,7 +702,10 @@ pub(crate) async fn set_account_from_qr(context: &Context, qr: &str) -> Result<( /// Sets configuration values from a QR code. pub async fn set_config_from_qr(context: &Context, qr: &str) -> Result<()> { match check_qr(context, qr).await? { - Qr::Account { .. } => set_account_from_qr(context, qr).await?, + Qr::Account { .. } => { + let mut param = login_param_from_account_qr(context, qr).await?; + context.add_transport_inner(&mut param).await? + } Qr::Proxy { url, .. } => { let old_proxy_url_value = context .get_config(Config::ProxyUrl) @@ -781,7 +793,8 @@ pub async fn set_config_from_qr(context: &Context, qr: &str) -> Result<()> { context.scheduler.interrupt_inbox().await; } Qr::Login { address, options } => { - configure_from_login_qr(context, &address, options).await? + let mut param = login_param_from_login_qr(&address, options)?; + context.add_transport_inner(&mut param).await? } _ => bail!("QR code does not contain config"), } diff --git a/src/qr/dclogin_scheme.rs b/src/qr/dclogin_scheme.rs index cdbbac352..cd4352498 100644 --- a/src/qr/dclogin_scheme.rs +++ b/src/qr/dclogin_scheme.rs @@ -3,12 +3,9 @@ use std::collections::HashMap; use anyhow::{Context as _, Result, bail}; use deltachat_contact_tools::may_be_valid_addr; -use num_traits::cast::ToPrimitive; use super::{DCLOGIN_SCHEME, Qr}; -use crate::config::Config; -use crate::context::Context; -use crate::login_param::EnteredCertificateChecks; +use crate::login_param::{EnteredCertificateChecks, EnteredLoginParam, EnteredServerLoginParam}; use crate::provider::Socket; /// Options for `dclogin:` scheme. @@ -157,15 +154,10 @@ fn parse_certificate_checks( }) } -pub(crate) async fn configure_from_login_qr( - context: &Context, - address: &str, +pub(crate) fn login_param_from_login_qr( + addr: &str, options: LoginOptions, -) -> Result<()> { - context - .set_config_internal(Config::Addr, Some(address)) - .await?; - +) -> Result { match options { LoginOptions::V1 { mail_pw, @@ -181,77 +173,26 @@ pub(crate) async fn configure_from_login_qr( smtp_security, certificate_checks, } => { - context - .set_config_internal(Config::MailPw, Some(&mail_pw)) - .await?; - if let Some(value) = imap_host { - context - .set_config_internal(Config::MailServer, Some(&value)) - .await?; - } - if let Some(value) = imap_port { - context - .set_config_internal(Config::MailPort, Some(&value.to_string())) - .await?; - } - if let Some(value) = imap_username { - context - .set_config_internal(Config::MailUser, Some(&value)) - .await?; - } - if let Some(value) = imap_password { - context - .set_config_internal(Config::MailPw, Some(&value)) - .await?; - } - if let Some(value) = imap_security { - let code = value - .to_u8() - .context("could not convert imap security value to number")?; - context - .set_config_internal(Config::MailSecurity, Some(&code.to_string())) - .await?; - } - if let Some(value) = smtp_host { - context - .set_config_internal(Config::SendServer, Some(&value)) - .await?; - } - if let Some(value) = smtp_port { - context - .set_config_internal(Config::SendPort, Some(&value.to_string())) - .await?; - } - if let Some(value) = smtp_username { - context - .set_config_internal(Config::SendUser, Some(&value)) - .await?; - } - if let Some(value) = smtp_password { - context - .set_config_internal(Config::SendPw, Some(&value)) - .await?; - } - if let Some(value) = smtp_security { - let code = value - .to_u8() - .context("could not convert smtp security value to number")?; - context - .set_config_internal(Config::SendSecurity, Some(&code.to_string())) - .await?; - } - if let Some(value) = certificate_checks { - let code = value - .to_u32() - .context("could not convert certificate checks value to number")?; - context - .set_config_internal(Config::ImapCertificateChecks, Some(&code.to_string())) - .await?; - context - .set_config_internal(Config::SmtpCertificateChecks, Some(&code.to_string())) - .await?; - } - Ok(()) + let param = EnteredLoginParam { + addr: addr.to_string(), + imap: EnteredServerLoginParam { + server: imap_host.unwrap_or_default(), + port: imap_port.unwrap_or_default(), + security: imap_security.unwrap_or_default(), + user: imap_username.unwrap_or_default(), + password: imap_password.unwrap_or(mail_pw), + }, + smtp: EnteredServerLoginParam { + server: smtp_host.unwrap_or_default(), + port: smtp_port.unwrap_or_default(), + security: smtp_security.unwrap_or_default(), + user: smtp_username.unwrap_or_default(), + password: smtp_password.unwrap_or_default(), + }, + certificate_checks: certificate_checks.unwrap_or_default(), + oauth2: false, + }; + Ok(param) } _ => bail!( "DeltaChat does not understand this QR Code yet, please update the app and try again." diff --git a/src/qr/qr_tests.rs b/src/qr/qr_tests.rs index 41d55d4fe..4accac634 100644 --- a/src/qr/qr_tests.rs +++ b/src/qr/qr_tests.rs @@ -1,6 +1,8 @@ use super::*; use crate::chat::create_group; use crate::config::Config; +use crate::login_param::EnteredCertificateChecks; +use crate::provider::Socket; use crate::securejoin::get_securejoin_qr; use crate::test_utils::{TestContext, TestContextManager, sync}; @@ -581,101 +583,58 @@ async fn test_withdraw_multidevice() -> Result<()> { } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn test_decode_and_apply_dclogin() -> Result<()> { - let ctx = TestContext::new().await; +async fn test_decode_dclogin() -> Result<()> { + let ctx = &TestContext::new().await; - let result = check_qr(&ctx.ctx, "dclogin:usename+extension@host?p=1234&v=1").await?; - if let Qr::Login { address, options } = result { - assert_eq!(address, "usename+extension@host".to_owned()); + let Qr::Login { address, options } = + check_qr(ctx, "dclogin:username+extension@host?p=1234&v=1").await? + else { + unreachable!(); + }; - if let LoginOptions::V1 { mail_pw, .. } = options { - assert_eq!(mail_pw, "1234".to_owned()); - } else { - bail!("wrong type") - } + assert_eq!(address, "username+extension@host".to_owned()); + + if let LoginOptions::V1 { ref mail_pw, .. } = options { + assert_eq!(mail_pw, "1234"); } else { bail!("wrong type") } - assert!(ctx.ctx.get_config(Config::Addr).await?.is_none()); - assert!(ctx.ctx.get_config(Config::MailPw).await?.is_none()); - - set_config_from_qr(&ctx.ctx, "dclogin:username+extension@host?p=1234&v=1").await?; - assert_eq!( - ctx.ctx.get_config(Config::Addr).await?, - Some("username+extension@host".to_owned()) - ); - assert_eq!( - ctx.ctx.get_config(Config::MailPw).await?, - Some("1234".to_owned()) - ); + let param = login_param_from_login_qr(&address, options)?; + assert_eq!(param.addr, "username+extension@host"); + assert_eq!(param.imap.password, "1234"); Ok(()) } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn test_decode_and_apply_dclogin_advanced_options() -> Result<()> { - let ctx = TestContext::new().await; - set_config_from_qr(&ctx.ctx, "dclogin:username+extension@host?p=1234&spw=4321&sh=send.host&sp=7273&su=SendUser&ih=host.tld&ip=4343&iu=user&ipw=password&is=ssl&ic=1&sc=3&ss=plain&v=1").await?; - assert_eq!( - ctx.ctx.get_config(Config::Addr).await?, - Some("username+extension@host".to_owned()) - ); +async fn test_decode_dclogin_advanced_options() -> Result<()> { + let ctx = &TestContext::new().await; + + let Qr::Login { address, options } = check_qr(ctx, "dclogin:username+extension@host?p=1234&spw=4321&sh=send.host&sp=7273&su=SendUser&ih=host.tld&ip=4343&iu=user&ipw=password&is=ssl&ic=1&sc=3&ss=plain&v=1").await? else { + unreachable!(); + }; + + assert_eq!(address, "username+extension@host"); + + let param = login_param_from_login_qr(&address, options)?; + assert_eq!(param.imap.server, "host.tld"); + assert_eq!(param.imap.port, 4343); + assert_eq!(param.imap.user, "user"); // `p=1234` is ignored, because `ipw=password` is set + assert_eq!(param.imap.password, "password"); + assert_eq!(param.imap.security, Socket::Ssl); - assert_eq!( - ctx.ctx.get_config(Config::MailServer).await?, - Some("host.tld".to_owned()) - ); - assert_eq!( - ctx.ctx.get_config(Config::MailPort).await?, - Some("4343".to_owned()) - ); - assert_eq!( - ctx.ctx.get_config(Config::MailUser).await?, - Some("user".to_owned()) - ); - assert_eq!( - ctx.ctx.get_config(Config::MailPw).await?, - Some("password".to_owned()) - ); - assert_eq!( - ctx.ctx.get_config(Config::MailSecurity).await?, - Some("1".to_owned()) // ssl - ); - assert_eq!( - ctx.ctx.get_config(Config::ImapCertificateChecks).await?, - Some("1".to_owned()) - ); - - assert_eq!( - ctx.ctx.get_config(Config::SendPw).await?, - Some("4321".to_owned()) - ); - assert_eq!( - ctx.ctx.get_config(Config::SendServer).await?, - Some("send.host".to_owned()) - ); - assert_eq!( - ctx.ctx.get_config(Config::SendPort).await?, - Some("7273".to_owned()) - ); - assert_eq!( - ctx.ctx.get_config(Config::SendUser).await?, - Some("SendUser".to_owned()) - ); + assert_eq!(param.smtp.password, "4321"); + assert_eq!(param.smtp.server, "send.host"); + assert_eq!(param.smtp.port, 7273); + assert_eq!(param.smtp.user, "SendUser"); + assert_eq!(param.smtp.security, Socket::Plain); // `sc` option is actually ignored and `ic` is used instead // because `smtp_certificate_checks` is deprecated. - assert_eq!( - ctx.ctx.get_config(Config::SmtpCertificateChecks).await?, - Some("1".to_owned()) - ); - assert_eq!( - ctx.ctx.get_config(Config::SendSecurity).await?, - Some("3".to_owned()) // plain - ); + assert_eq!(param.certificate_checks, EnteredCertificateChecks::Strict); Ok(()) }