Validate and simplify LoginParam struct

This makes sure that under normal circumstances the LoginParam struct
is always fully validated, ensure future use does not have to be
careful with this.

The brittle handling of `server_flags` is also abstraced away from
users of it and is now handled entirely internally, as the flags is
really only a boolean a lot of the flag parsing complexity is removed.
The OAuth2 flag is moved into the ServerLoginParam struct as it really
belongs in there.
This commit is contained in:
Floris Bruynooghe
2022-05-10 20:59:17 +02:00
parent 2361042ede
commit 8033966a70
5 changed files with 57 additions and 94 deletions

View File

@@ -11,7 +11,6 @@ use async_std::task;
use percent_encoding::{utf8_percent_encode, NON_ALPHANUMERIC}; use percent_encoding::{utf8_percent_encode, NON_ALPHANUMERIC};
use crate::config::Config; use crate::config::Config;
use crate::constants::{DC_LP_AUTH_FLAGS, DC_LP_AUTH_NORMAL, DC_LP_AUTH_OAUTH2};
use crate::context::Context; use crate::context::Context;
use crate::dc_tools::{time, EmailAddress}; use crate::dc_tools::{time, EmailAddress};
use crate::imap::Imap; use crate::imap::Imap;
@@ -145,28 +144,6 @@ impl Context {
async fn configure(ctx: &Context, param: &mut LoginParam) -> Result<()> { async fn configure(ctx: &Context, param: &mut LoginParam) -> Result<()> {
progress!(ctx, 1); progress!(ctx, 1);
// Check basic settings.
ensure!(!param.addr.is_empty(), "Please enter an email address.");
// Only check for IMAP password, SMTP password is an "advanced" setting.
ensure!(!param.imap.password.is_empty(), "Please enter a password.");
if param.smtp.password.is_empty() {
param.smtp.password = param.imap.password.clone()
}
// Normalize authentication flags.
let oauth2 = match param.server_flags & DC_LP_AUTH_FLAGS as i32 {
DC_LP_AUTH_OAUTH2 => true,
DC_LP_AUTH_NORMAL => false,
_ => false,
};
param.server_flags &= !(DC_LP_AUTH_FLAGS as i32);
param.server_flags |= if oauth2 {
DC_LP_AUTH_OAUTH2 as i32
} else {
DC_LP_AUTH_NORMAL as i32
};
let socks5_config = param.socks5_config.clone(); let socks5_config = param.socks5_config.clone();
let socks5_enabled = socks5_config.is_some(); let socks5_enabled = socks5_config.is_some();
@@ -176,8 +153,9 @@ async fn configure(ctx: &Context, param: &mut LoginParam) -> Result<()> {
// Step 1: Load the parameters and check email-address and password // Step 1: Load the parameters and check email-address and password
// Do oauth2 only if socks5 is disabled. As soon as we have a http library that can do // Do oauth2 only if socks5 is disabled. As soon as we have a http library that can do
// socks5 requests, this can work with socks5 too // socks5 requests, this can work with socks5 too. OAuth is always set either for both
if oauth2 && !socks5_enabled { // IMAP and SMTP or not at all.
if param.imap.oauth2 && !socks5_enabled {
// the used oauth2 addr may differ, check this. // the used oauth2 addr may differ, check this.
// if dc_get_oauth2_addr() is not available in the oauth2 implementation, just use the given one. // if dc_get_oauth2_addr() is not available in the oauth2 implementation, just use the given one.
progress!(ctx, 10); progress!(ctx, 10);
@@ -358,7 +336,6 @@ async fn configure(ctx: &Context, param: &mut LoginParam) -> Result<()> {
&smtp_param, &smtp_param,
&socks5_config, &socks5_config,
&smtp_addr, &smtp_addr,
oauth2,
provider_strict_tls, provider_strict_tls,
&mut smtp, &mut smtp,
) )
@@ -406,7 +383,6 @@ async fn configure(ctx: &Context, param: &mut LoginParam) -> Result<()> {
&param.imap, &param.imap,
&param.socks5_config, &param.socks5_config,
&param.addr, &param.addr,
oauth2,
provider_strict_tls, provider_strict_tls,
) )
.await .await
@@ -562,26 +538,22 @@ async fn try_imap_one_param(
param: &ServerLoginParam, param: &ServerLoginParam,
socks5_config: &Option<Socks5Config>, socks5_config: &Option<Socks5Config>,
addr: &str, addr: &str,
oauth2: bool,
provider_strict_tls: bool, provider_strict_tls: bool,
) -> Result<Imap, ConfigurationError> { ) -> Result<Imap, ConfigurationError> {
let inf = format!( let inf = format!(
"imap: {}@{}:{} security={} certificate_checks={} oauth2={}", "imap: {}@{}:{} security={} certificate_checks={} oauth2={}",
param.user, param.server, param.port, param.security, param.certificate_checks, oauth2 param.user,
param.server,
param.port,
param.security,
param.certificate_checks,
param.oauth2
); );
info!(context, "Trying: {}", inf); info!(context, "Trying: {}", inf);
let (_s, r) = async_std::channel::bounded(1); let (_s, r) = async_std::channel::bounded(1);
let mut imap = match Imap::new( let mut imap = match Imap::new(param, socks5_config.clone(), addr, provider_strict_tls, r).await
param,
socks5_config.clone(),
addr,
oauth2,
provider_strict_tls,
r,
)
.await
{ {
Err(err) => { Err(err) => {
info!(context, "failure: {}", err); info!(context, "failure: {}", err);
@@ -613,7 +585,6 @@ async fn try_smtp_one_param(
param: &ServerLoginParam, param: &ServerLoginParam,
socks5_config: &Option<Socks5Config>, socks5_config: &Option<Socks5Config>,
addr: &str, addr: &str,
oauth2: bool,
provider_strict_tls: bool, provider_strict_tls: bool,
smtp: &mut Smtp, smtp: &mut Smtp,
) -> Result<(), ConfigurationError> { ) -> Result<(), ConfigurationError> {
@@ -624,7 +595,7 @@ async fn try_smtp_one_param(
param.port, param.port,
param.security, param.security,
param.certificate_checks, param.certificate_checks,
oauth2, param.oauth2,
if let Some(socks5_config) = socks5_config { if let Some(socks5_config) = socks5_config {
socks5_config.to_string() socks5_config.to_string()
} else { } else {
@@ -634,14 +605,7 @@ async fn try_smtp_one_param(
info!(context, "Trying: {}", inf); info!(context, "Trying: {}", inf);
if let Err(err) = smtp if let Err(err) = smtp
.connect( .connect(context, param, socks5_config, addr, provider_strict_tls)
context,
param,
socks5_config,
addr,
oauth2,
provider_strict_tls,
)
.await .await
{ {
info!(context, "failure: {}", err); info!(context, "failure: {}", err);

View File

@@ -342,7 +342,7 @@ impl Context {
pub async fn get_info(&self) -> Result<BTreeMap<&'static str, String>> { pub async fn get_info(&self) -> Result<BTreeMap<&'static str, String>> {
let unset = "0"; let unset = "0";
let l = LoginParam::load_candidate_params(self).await?; let l = LoginParam::load_candidate_params_unchecked(self).await?;
let l2 = LoginParam::load_configured_params(self).await?; let l2 = LoginParam::load_configured_params(self).await?;
let secondary_addrs = self.get_secondary_self_addrs().await?.join(", "); let secondary_addrs = self.get_secondary_self_addrs().await?.join(", ");
let displayname = self.get_config(Config::Displayname).await?; let displayname = self.get_config(Config::Displayname).await?;

View File

@@ -22,7 +22,6 @@ use crate::chat::{self, ChatId, ChatIdBlocked};
use crate::config::Config; use crate::config::Config;
use crate::constants::{ use crate::constants::{
Blocked, Chattype, ShowEmails, DC_FETCH_EXISTING_MSGS_COUNT, DC_FOLDERS_CONFIGURED_VERSION, Blocked, Chattype, ShowEmails, DC_FETCH_EXISTING_MSGS_COUNT, DC_FOLDERS_CONFIGURED_VERSION,
DC_LP_AUTH_OAUTH2,
}; };
use crate::contact::{normalize_name, Contact, ContactId, Modifier, Origin}; use crate::contact::{normalize_name, Contact, ContactId, Modifier, Origin};
use crate::context::Context; use crate::context::Context;
@@ -154,7 +153,6 @@ struct ImapConfig {
pub lp: ServerLoginParam, pub lp: ServerLoginParam,
pub socks5_config: Option<Socks5Config>, pub socks5_config: Option<Socks5Config>,
pub strict_tls: bool, pub strict_tls: bool,
pub oauth2: bool,
pub selected_folder: Option<String>, pub selected_folder: Option<String>,
pub selected_mailbox: Option<Mailbox>, pub selected_mailbox: Option<Mailbox>,
pub selected_folder_needs_expunge: bool, pub selected_folder_needs_expunge: bool,
@@ -243,7 +241,6 @@ impl Imap {
lp: &ServerLoginParam, lp: &ServerLoginParam,
socks5_config: Option<Socks5Config>, socks5_config: Option<Socks5Config>,
addr: &str, addr: &str,
oauth2: bool,
provider_strict_tls: bool, provider_strict_tls: bool,
idle_interrupt: Receiver<InterruptInfo>, idle_interrupt: Receiver<InterruptInfo>,
) -> Result<Self> { ) -> Result<Self> {
@@ -262,7 +259,6 @@ impl Imap {
lp: lp.clone(), lp: lp.clone(),
socks5_config, socks5_config,
strict_tls, strict_tls,
oauth2,
selected_folder: None, selected_folder: None,
selected_mailbox: None, selected_mailbox: None,
selected_folder_needs_expunge: false, selected_folder_needs_expunge: false,
@@ -301,7 +297,6 @@ impl Imap {
&param.imap, &param.imap,
param.socks5_config.clone(), param.socks5_config.clone(),
&param.addr, &param.addr,
param.server_flags & DC_LP_AUTH_OAUTH2 != 0,
param param
.provider .provider
.map_or(param.socks5_config.is_some(), |provider| { .map_or(param.socks5_config.is_some(), |provider| {
@@ -334,7 +329,7 @@ impl Imap {
self.connectivity.set_connecting(context).await; self.connectivity.set_connecting(context).await;
let oauth2 = self.config.oauth2; let oauth2 = self.config.lp.oauth2;
let connection_res: Result<Client> = if self.config.lp.security == Socket::Starttls let connection_res: Result<Client> = if self.config.lp.security == Socket::Starttls
|| self.config.lp.security == Socket::Plain || self.config.lp.security == Socket::Plain

View File

@@ -4,9 +4,10 @@ use std::borrow::Cow;
use std::fmt; use std::fmt;
use std::time::Duration; use std::time::Duration;
use crate::constants::{DC_LP_AUTH_FLAGS, DC_LP_AUTH_NORMAL, DC_LP_AUTH_OAUTH2};
use crate::provider::{get_provider_by_id, Provider}; use crate::provider::{get_provider_by_id, Provider};
use crate::{context::Context, provider::Socket}; use crate::{context::Context, provider::Socket};
use anyhow::Result; use anyhow::{ensure, Result};
use async_std::io; use async_std::io;
use async_std::net::TcpStream; use async_std::net::TcpStream;
@@ -47,6 +48,7 @@ pub struct ServerLoginParam {
pub password: String, pub password: String,
pub port: u16, pub port: u16,
pub security: Socket, pub security: Socket,
pub oauth2: bool,
/// TLS options: whether to allow invalid certificates and/or /// TLS options: whether to allow invalid certificates and/or
/// invalid hostnames /// invalid hostnames
@@ -133,7 +135,6 @@ pub struct LoginParam {
pub addr: String, pub addr: String,
pub imap: ServerLoginParam, pub imap: ServerLoginParam,
pub smtp: ServerLoginParam, pub smtp: ServerLoginParam,
pub server_flags: i32,
pub provider: Option<&'static Provider>, pub provider: Option<&'static Provider>,
pub socks5_config: Option<Socks5Config>, pub socks5_config: Option<Socks5Config>,
} }
@@ -141,6 +142,23 @@ pub struct LoginParam {
impl LoginParam { impl LoginParam {
/// Load entered (candidate) account settings /// Load entered (candidate) account settings
pub async fn load_candidate_params(context: &Context) -> Result<Self> { pub async fn load_candidate_params(context: &Context) -> Result<Self> {
let mut param = Self::load_candidate_params_unchecked(context).await?;
ensure!(!param.addr.is_empty(), "Missing email address.");
// Only check for IMAP password, SMTP password is an "advanced" setting.
ensure!(!param.imap.password.is_empty(), "Missing (IMAP) password.");
if param.smtp.password.is_empty() {
param.smtp.password = param.imap.password.clone()
}
Ok(param)
}
/// Load entered (candidate) account settings without validation.
///
/// This will result in a potentially invalid [`LoginParam`] struct as the values are
/// not validated. Only use this if you want to show this directly to the user e.g. in
/// [`Context::get_info`].
pub async fn load_candidate_params_unchecked(context: &Context) -> Result<Self> {
LoginParam::from_database(context, "").await LoginParam::from_database(context, "").await
} }
@@ -217,6 +235,7 @@ impl LoginParam {
let key = format!("{}server_flags", prefix); let key = format!("{}server_flags", prefix);
let server_flags = sql.get_raw_config_int(key).await?.unwrap_or_default(); let server_flags = sql.get_raw_config_int(key).await?.unwrap_or_default();
let oauth2 = matches!(server_flags & DC_LP_AUTH_FLAGS, DC_LP_AUTH_OAUTH2);
let key = format!("{}provider", prefix); let key = format!("{}provider", prefix);
let provider = sql let provider = sql
@@ -234,6 +253,7 @@ impl LoginParam {
password: mail_pw, password: mail_pw,
port: mail_port as u16, port: mail_port as u16,
security: mail_security, security: mail_security,
oauth2,
certificate_checks: imap_certificate_checks, certificate_checks: imap_certificate_checks,
}, },
smtp: ServerLoginParam { smtp: ServerLoginParam {
@@ -242,10 +262,10 @@ impl LoginParam {
password: send_pw, password: send_pw,
port: send_port as u16, port: send_port as u16,
security: send_security, security: send_security,
oauth2,
certificate_checks: smtp_certificate_checks, certificate_checks: smtp_certificate_checks,
}, },
provider, provider,
server_flags,
socks5_config, socks5_config,
}) })
} }
@@ -299,8 +319,13 @@ impl LoginParam {
sql.set_raw_config_int(key, self.smtp.certificate_checks as i32) sql.set_raw_config_int(key, self.smtp.certificate_checks as i32)
.await?; .await?;
// The OAuth2 flag is either set for both IMAP and SMTP or not at all.
let key = format!("{}server_flags", prefix); let key = format!("{}server_flags", prefix);
sql.set_raw_config_int(key, self.server_flags).await?; let server_flags = match self.imap.oauth2 {
true => DC_LP_AUTH_OAUTH2,
false => DC_LP_AUTH_NORMAL,
};
sql.set_raw_config_int(key, server_flags).await?;
if let Some(provider) = self.provider { if let Some(provider) = self.provider {
let key = format!("{}provider", prefix); let key = format!("{}provider", prefix);
@@ -316,11 +341,9 @@ impl fmt::Display for LoginParam {
let unset = "0"; let unset = "0";
let pw = "***"; let pw = "***";
let flags_readable = get_readable_flags(self.server_flags);
write!( write!(
f, f,
"{} imap:{}:{}:{}:{}:cert_{} smtp:{}:{}:{}:{}:cert_{} {}", "{} imap:{}:{}:{}:{}:cert_{}:{} smtp:{}:{}:{}:{}:cert_{}:{}",
unset_empty(&self.addr), unset_empty(&self.addr),
unset_empty(&self.imap.user), unset_empty(&self.imap.user),
if !self.imap.password.is_empty() { if !self.imap.password.is_empty() {
@@ -331,6 +354,11 @@ impl fmt::Display for LoginParam {
unset_empty(&self.imap.server), unset_empty(&self.imap.server),
self.imap.port, self.imap.port,
self.imap.certificate_checks, self.imap.certificate_checks,
if self.imap.oauth2 {
"OAUTH2"
} else {
"AUTH_NORMAL"
},
unset_empty(&self.smtp.user), unset_empty(&self.smtp.user),
if !self.smtp.password.is_empty() { if !self.smtp.password.is_empty() {
pw pw
@@ -340,7 +368,11 @@ impl fmt::Display for LoginParam {
unset_empty(&self.smtp.server), unset_empty(&self.smtp.server),
self.smtp.port, self.smtp.port,
self.smtp.certificate_checks, self.smtp.certificate_checks,
flags_readable, if self.smtp.oauth2 {
"OAUTH2"
} else {
"AUTH_NORMAL"
},
) )
} }
} }
@@ -354,32 +386,6 @@ fn unset_empty(s: &String) -> Cow<String> {
} }
} }
#[allow(clippy::useless_let_if_seq)]
fn get_readable_flags(flags: i32) -> String {
let mut res = String::new();
for bit in 0..31 {
if 0 != flags & 1 << bit {
let mut flag_added = false;
if 1 << bit == 0x2 {
res += "OAUTH2 ";
flag_added = true;
}
if 1 << bit == 0x4 {
res += "AUTH_NORMAL ";
flag_added = true;
}
if flag_added {
res += &format!("{:#0x}", 1 << bit);
}
}
}
if res.is_empty() {
res += "0";
}
res
}
// this certificate is missing on older android devices (eg. lg with android6 from 2017) // this certificate is missing on older android devices (eg. lg with android6 from 2017)
// certificate downloaded from https://letsencrypt.org/certificates/ // certificate downloaded from https://letsencrypt.org/certificates/
static LETSENCRYPT_ROOT: Lazy<Certificate> = Lazy::new(|| { static LETSENCRYPT_ROOT: Lazy<Certificate> = Lazy::new(|| {
@@ -430,6 +436,7 @@ mod tests {
password: "foo".to_string(), password: "foo".to_string(),
port: 123, port: 123,
security: Socket::Starttls, security: Socket::Starttls,
oauth2: false,
certificate_checks: CertificateChecks::Strict, certificate_checks: CertificateChecks::Strict,
}, },
smtp: ServerLoginParam { smtp: ServerLoginParam {
@@ -438,9 +445,9 @@ mod tests {
password: "bar".to_string(), password: "bar".to_string(),
port: 456, port: 456,
security: Socket::Ssl, security: Socket::Ssl,
oauth2: false,
certificate_checks: CertificateChecks::AcceptInvalidCertificates, certificate_checks: CertificateChecks::AcceptInvalidCertificates,
}, },
server_flags: 0,
provider: get_provider_by_id("example.com"), provider: get_provider_by_id("example.com"),
// socks5_config is not saved by `save_to_database`, using default value // socks5_config is not saved by `save_to_database`, using default value
socks5_config: None, socks5_config: None,

View File

@@ -11,7 +11,6 @@ use async_smtp::{smtp, EmailAddress, ServerAddress};
use async_std::task; use async_std::task;
use crate::config::Config; use crate::config::Config;
use crate::constants::DC_LP_AUTH_OAUTH2;
use crate::contact::{Contact, ContactId}; use crate::contact::{Contact, ContactId};
use crate::events::EventType; use crate::events::EventType;
use crate::login_param::{ use crate::login_param::{
@@ -103,7 +102,6 @@ impl Smtp {
&lp.smtp, &lp.smtp,
&lp.socks5_config, &lp.socks5_config,
&lp.addr, &lp.addr,
lp.server_flags & DC_LP_AUTH_OAUTH2 != 0,
lp.provider lp.provider
.map_or(lp.socks5_config.is_some(), |provider| provider.strict_tls), .map_or(lp.socks5_config.is_some(), |provider| provider.strict_tls),
) )
@@ -117,7 +115,6 @@ impl Smtp {
lp: &ServerLoginParam, lp: &ServerLoginParam,
socks5_config: &Option<Socks5Config>, socks5_config: &Option<Socks5Config>,
addr: &str, addr: &str,
oauth2: bool,
provider_strict_tls: bool, provider_strict_tls: bool,
) -> Result<()> { ) -> Result<()> {
if self.is_connected().await { if self.is_connected().await {
@@ -146,7 +143,7 @@ impl Smtp {
let tls_config = dc_build_tls(strict_tls); let tls_config = dc_build_tls(strict_tls);
let tls_parameters = ClientTlsParameters::new(domain.to_string(), tls_config); let tls_parameters = ClientTlsParameters::new(domain.to_string(), tls_config);
let (creds, mechanism) = if oauth2 { let (creds, mechanism) = if lp.oauth2 {
// oauth2 // oauth2
let send_pw = &lp.password; let send_pw = &lp.password;
let access_token = dc_get_oauth2_access_token(context, addr, send_pw, false).await?; let access_token = dc_get_oauth2_access_token(context, addr, send_pw, false).await?;