From e82d52e5301eb5cfcdd067c72c86d428ae5278ef Mon Sep 17 00:00:00 2001 From: James Maes Date: Fri, 31 Jul 2026 17:59:18 -0500 Subject: [PATCH] refactor(proto): model account transport as a oneof Make an invalid account transport mix unrepresentable: an account now carries exactly one transport instead of a flat bag of optional imap/jmap/dav fields. - proto: AccountConfig gains `oneof transport { ImapSmtpTransport | JmapTransport | DavTransport }`; the flat fields (protocol, server_host/port, use_tls, imap/smtp tls modes, smtp_host/port, collection_url) are removed and their field numbers reserved. Golden descriptor regenerated; contract_stability passes. - core: AccountConfig carries a `Transport` enum; validate() checks the active variant and preserves the cleartext-to-remote-host refusal per transport. - store: physical accounts columns stay flat; the existing `protocol` column is the transport-kind discriminator. Each variant round-trips. - daemon: grpc maps core Transport <-> proto oneof both ways; an unset transport returns typed VALIDATION_FAILED, never a panic. - cli: `account add --protocol` gates transport flags; list/show render the active transport; --json stays coherent. Refs #278 --- crates/nuncio-cli/src/runner.rs | 284 +++++++++----- crates/nuncio-core/src/config.rs | 362 ++++++++++++------ crates/nuncio-core/src/lib.rs | 5 +- .../proto/nuncio/v1/descriptor.bin | Bin 15298 -> 15645 bytes .../nuncio-proto/proto/nuncio/v1/nuncio.proto | 91 +++-- crates/nuncio-store/src/db.rs | 304 ++++++++++----- crates/nuncio-store/src/recovery.rs | 87 +++-- crates/nunciod/src/calendar_sync.rs | 2 +- crates/nunciod/src/grpc.rs | 340 ++++++++-------- crates/nunciod/src/orchestrator.rs | 16 +- crates/nunciod/src/outbox.rs | 9 +- crates/nunciod/src/send.rs | 30 +- crates/nunciod/src/sync.rs | 49 ++- crates/nunciod/src/sync_dispatcher.rs | 19 +- .../tests/accounts_lifecycle_e2e_test.rs | 45 ++- .../tests/caldav_production_e2e_test.rs | 16 +- .../nunciod/tests/outbox_executor_e2e_test.rs | 16 +- crates/nunciod/tests/spine_e2e_test.rs | 22 +- 18 files changed, 1082 insertions(+), 615 deletions(-) diff --git a/crates/nuncio-cli/src/runner.rs b/crates/nuncio-cli/src/runner.rs index 7afe09a..a74ef3a 100644 --- a/crates/nuncio-cli/src/runner.rs +++ b/crates/nuncio-cli/src/runner.rs @@ -53,6 +53,64 @@ fn parse_account_protocol(protocol: &str) -> Result serde_json::Value { + use nuncio_proto::v1::account_config::Transport; + match &config.transport { + Some(Transport::ImapSmtp(t)) => json!({ + "protocol": "imap-smtp", + "imap_host": t.imap_host, + "imap_port": t.imap_port, + "imap_tls_mode": t.imap_tls_mode().as_str_name(), + "smtp_host": t.smtp_host, + "smtp_port": t.smtp_port, + "smtp_tls_mode": t.smtp_tls_mode().as_str_name(), + }), + Some(Transport::Jmap(t)) => json!({ + "protocol": "jmap", + "endpoint_host": t.endpoint_host, + }), + Some(Transport::Dav(t)) => json!({ + "protocol": "caldav", + "collection_url": t.collection_url, + }), + None => json!({ "protocol": "unspecified" }), + } +} + +/// Merges the top-level object fields of `extra` into `base` (both must be +/// JSON objects). Used to fold an account's transport-specific fields into its +/// common-field object without hand-writing every combination. +fn merge_json(base: &mut serde_json::Value, extra: serde_json::Value) { + if let (Some(base), serde_json::Value::Object(extra)) = (base.as_object_mut(), extra) { + for (k, v) in extra { + base.insert(k, v); + } + } +} + +/// Renders an account's active transport as a single human-readable summary +/// line fragment for `account list`/`show`. +fn account_transport_summary(config: &nuncio_proto::v1::AccountConfig) -> String { + use nuncio_proto::v1::account_config::Transport; + match &config.transport { + Some(Transport::ImapSmtp(t)) => format!( + "IMAP {}:{} ({}) SMTP {}:{} ({})", + t.imap_host, + t.imap_port, + t.imap_tls_mode().as_str_name(), + t.smtp_host, + t.smtp_port, + t.smtp_tls_mode().as_str_name(), + ), + Some(Transport::Jmap(t)) => format!("JMAP {}", t.endpoint_host), + Some(Transport::Dav(t)) => format!("CalDAV {}", t.collection_url), + None => "(no transport configured)".to_string(), + } +} + /// Parses a `filter export --format` CLI string into its wire-format /// `nuncio.v1.RuleExportFormat` value. Rejects anything else rather than /// silently falling back to one rendering, since a typo'd format should @@ -1869,30 +1927,45 @@ impl HeadlessRunner { let keyring_key = format!("nuncio/{}", email); let account_id = format!("acct-{}", email.replace('@', "-at-").replace('.', "-")); - // A CalDAV account is addressed by its collection URL, not by mail - // host/port endpoints, so those are left empty for it. + // Build the single transport the account speaks. Each transport carries + // only its own endpoint fields, so a nonsensical mix is unrepresentable. + let transport = match account_protocol { + nuncio_proto::v1::AccountProtocol::ImapSmtp => { + nuncio_proto::v1::account_config::Transport::ImapSmtp( + nuncio_proto::v1::ImapSmtpTransport { + imap_host: imap_host.to_string(), + imap_port: u32::from(imap_port), + imap_tls_mode: map_tls_mode_to_proto(imap_tls_mode).into(), + smtp_host: smtp_host.to_string(), + smtp_port: u32::from(smtp_port), + smtp_tls_mode: map_tls_mode_to_proto(smtp_tls_mode).into(), + }, + ) + } + nuncio_proto::v1::AccountProtocol::Jmap => { + // A JMAP account addresses its provider by a session endpoint + // host; the `--imap-host` flag supplies it. + nuncio_proto::v1::account_config::Transport::Jmap(nuncio_proto::v1::JmapTransport { + endpoint_host: imap_host.to_string(), + }) + } + nuncio_proto::v1::AccountProtocol::Caldav => { + nuncio_proto::v1::account_config::Transport::Dav(nuncio_proto::v1::DavTransport { + collection_url: collection_url.unwrap_or_default().to_string(), + }) + } + nuncio_proto::v1::AccountProtocol::Unspecified => { + return Self::render_error("account protocol is required", json_mode) + } + }; + let proto_config = nuncio_proto::v1::AccountConfig { id: account_id.clone(), name: email.to_string(), email_address: email.to_string(), - protocol: account_protocol.into(), - server_host: if is_caldav { - String::new() - } else { - imap_host.to_string() - }, - server_port: if is_caldav { 0 } else { u32::from(imap_port) }, - imap_tls_mode: map_tls_mode_to_proto(imap_tls_mode).into(), - smtp_tls_mode: map_tls_mode_to_proto(smtp_tls_mode).into(), keyring_secret_key: keyring_key.clone(), sync_interval: Some(nuncio_proto::time::duration_from_secs(300)), - smtp_host: if is_caldav { - String::new() - } else { - smtp_host.to_string() - }, - smtp_port: if is_caldav { 0 } else { u32::from(smtp_port) }, - collection_url: collection_url.unwrap_or_default().to_string(), + transport: Some(transport), }; let mut client = match self.connect_accounts_client().await { @@ -1916,6 +1989,7 @@ impl HeadlessRunner { .config .map(|c| c.id) .unwrap_or(account_id); + let is_jmap = account_protocol == nuncio_proto::v1::AccountProtocol::Jmap; if is_caldav { let collection = collection_url.unwrap_or_default(); if json_mode { @@ -1932,6 +2006,21 @@ impl HeadlessRunner { "CalDAV account '{email}' (ID: {account_id}) added via nunciod daemon with collection URL {collection}" ) } + } else if is_jmap { + if json_mode { + format_json(&json!({ + "configured": true, + "account_id": account_id, + "email": email, + "protocol": "jmap", + "endpoint_host": imap_host, + "keyring_key": keyring_key + })) + } else { + format!( + "JMAP account '{email}' (ID: {account_id}) added via nunciod daemon with endpoint host {imap_host}" + ) + } } else if json_mode { format_json(&json!({ "configured": true, @@ -1983,21 +2072,15 @@ impl HeadlessRunner { let accounts_json: Vec = accounts .iter() .map(|a| { - json!({ + let mut base = json!({ "id": a.id, "name": a.name, "email_address": a.email_address, - "protocol": a.protocol().as_str_name(), - "server_host": a.server_host, - "server_port": a.server_port, - "smtp_host": a.smtp_host, - "smtp_port": a.smtp_port, - "imap_tls_mode": a.imap_tls_mode().as_str_name(), - "smtp_tls_mode": a.smtp_tls_mode().as_str_name(), "keyring_secret_key": a.keyring_secret_key, "sync_interval_secs": duration_secs(&a.sync_interval), - "collection_url": a.collection_url, - }) + }); + merge_json(&mut base, account_transport_json(a)); + base }) .collect(); format_json(&json!({ "accounts": accounts_json })) @@ -2007,25 +2090,13 @@ impl HeadlessRunner { accounts.len() ); for a in &accounts { - if a.protocol() == nuncio_proto::v1::AccountProtocol::Caldav { - out.push_str(&format!( - "\n [{}] {} <{}> CalDAV {}", - a.id, a.name, a.email_address, a.collection_url, - )); - } else { - out.push_str(&format!( - "\n [{}] {} <{}> IMAP {}:{} ({}) SMTP {}:{} ({})", - a.id, - a.name, - a.email_address, - a.server_host, - a.server_port, - a.imap_tls_mode().as_str_name(), - a.smtp_host, - a.smtp_port, - a.smtp_tls_mode().as_str_name(), - )); - } + out.push_str(&format!( + "\n [{}] {} <{}> {}", + a.id, + a.name, + a.email_address, + account_transport_summary(a), + )); } out } @@ -2078,32 +2149,22 @@ impl HeadlessRunner { }; if json_mode { - format_json(&json!({ + let mut base = json!({ "id": account.id, "name": account.name, "email_address": account.email_address, - "protocol": account.protocol().as_str_name(), - "server_host": account.server_host, - "server_port": account.server_port, - "smtp_host": account.smtp_host, - "smtp_port": account.smtp_port, - "imap_tls_mode": account.imap_tls_mode().as_str_name(), - "smtp_tls_mode": account.smtp_tls_mode().as_str_name(), "keyring_secret_key": account.keyring_secret_key, "sync_interval_secs": duration_secs(&account.sync_interval), - })) + }); + merge_json(&mut base, account_transport_json(&account)); + format_json(&base) } else { format!( - "Account [{}] {} <{}> IMAP {}:{} ({}) SMTP {}:{} ({})", + "Account [{}] {} <{}> {}", account.id, account.name, account.email_address, - account.server_host, - account.server_port, - account.imap_tls_mode().as_str_name(), - account.smtp_host, - account.smtp_port, - account.smtp_tls_mode().as_str_name(), + account_transport_summary(&account), ) } } @@ -2134,28 +2195,51 @@ impl HeadlessRunner { if let Some(email) = email { config.email_address = email.to_string(); } - if let Some(host) = imap_host { - config.server_host = host.to_string(); - } - if let Some(port) = imap_port { - config.server_port = u32::from(port); - } - if let Some(host) = smtp_host { - config.smtp_host = host.to_string(); - } - if let Some(port) = smtp_port { - config.smtp_port = u32::from(port); - } - if let Some(mode) = imap_mode { - match parse_tls_mode(mode) { - Ok(mode) => config.imap_tls_mode = map_tls_mode_to_proto(mode).into(), - Err(e) => return Self::render_error(&e, json_mode), - } - } - if let Some(mode) = smtp_mode { - match parse_tls_mode(mode) { - Ok(mode) => config.smtp_tls_mode = map_tls_mode_to_proto(mode).into(), - Err(e) => return Self::render_error(&e, json_mode), + + // The IMAP/SMTP endpoint flags only apply to an imap-smtp account; the + // transport oneof makes the alternatives (JMAP endpoint, DAV URL) + // structurally distinct, so applying these to another transport would + // be meaningless -- reject it rather than silently ignoring the flags. + let touches_imap_smtp_fields = imap_host.is_some() + || imap_port.is_some() + || smtp_host.is_some() + || smtp_port.is_some() + || imap_mode.is_some() + || smtp_mode.is_some(); + if touches_imap_smtp_fields { + match &mut config.transport { + Some(nuncio_proto::v1::account_config::Transport::ImapSmtp(t)) => { + if let Some(host) = imap_host { + t.imap_host = host.to_string(); + } + if let Some(port) = imap_port { + t.imap_port = u32::from(port); + } + if let Some(host) = smtp_host { + t.smtp_host = host.to_string(); + } + if let Some(port) = smtp_port { + t.smtp_port = u32::from(port); + } + if let Some(mode) = imap_mode { + match parse_tls_mode(mode) { + Ok(mode) => t.imap_tls_mode = map_tls_mode_to_proto(mode).into(), + Err(e) => return Self::render_error(&e, json_mode), + } + } + if let Some(mode) = smtp_mode { + match parse_tls_mode(mode) { + Ok(mode) => t.smtp_tls_mode = map_tls_mode_to_proto(mode).into(), + Err(e) => return Self::render_error(&e, json_mode), + } + } + } + _ => { + return Self::render_error( + "the IMAP/SMTP flags only apply to an imap-smtp account", + json_mode, + ) + } } } @@ -3050,11 +3134,10 @@ mod tests { async fn account_add_and_list_round_trip_over_grpc_to_a_stub_daemon() { use nuncio_proto::v1::accounts_server::{Accounts as AccountsService, AccountsServer}; use nuncio_proto::v1::{ - AccountConfig as AccountConfigProto, AccountProtocol as AccountProtocolProto, - AddAccountRequest, AddAccountResponse, ListAccountsRequest, ListAccountsResponse, - RemoveAccountRequest, RemoveAccountResponse, TestAccountConnectionRequest, - TestAccountConnectionResponse, TlsMode as TlsModeProto, UpdateAccountRequest, - UpdateAccountResponse, + AccountConfig as AccountConfigProto, AddAccountRequest, AddAccountResponse, + ListAccountsRequest, ListAccountsResponse, RemoveAccountRequest, RemoveAccountResponse, + TestAccountConnectionRequest, TestAccountConnectionResponse, TlsMode as TlsModeProto, + UpdateAccountRequest, UpdateAccountResponse, }; use std::sync::Mutex; @@ -3117,16 +3200,18 @@ mod tests { id: "acct-stub-1".to_string(), name: "Stub Account".to_string(), email_address: "stub@nuncio.mx".to_string(), - protocol: AccountProtocolProto::ImapSmtp.into(), - server_host: "imap.nuncio.mx".to_string(), - server_port: 993, - imap_tls_mode: TlsModeProto::ImplicitTls.into(), - smtp_tls_mode: TlsModeProto::ImplicitTls.into(), keyring_secret_key: "nuncio/acct-stub-1".to_string(), sync_interval: Some(nuncio_proto::time::duration_from_secs(300)), - smtp_host: "smtp.nuncio.mx".to_string(), - smtp_port: 465, - collection_url: String::new(), + transport: Some(nuncio_proto::v1::account_config::Transport::ImapSmtp( + nuncio_proto::v1::ImapSmtpTransport { + imap_host: "imap.nuncio.mx".to_string(), + imap_port: 993, + imap_tls_mode: TlsModeProto::ImplicitTls.into(), + smtp_host: "smtp.nuncio.mx".to_string(), + smtp_port: 465, + smtp_tls_mode: TlsModeProto::ImplicitTls.into(), + }, + )), }], })) } @@ -3201,7 +3286,8 @@ mod tests { .await; assert!(list_out.contains("acct-stub-1")); assert!(list_out.contains("stub@nuncio.mx")); - assert!(list_out.contains(r#""protocol":"ACCOUNT_PROTOCOL_IMAP_SMTP""#)); + assert!(list_out.contains(r#""protocol":"imap-smtp""#)); + assert!(list_out.contains(r#""imap_host":"imap.nuncio.mx""#)); let list_out_text = runner .execute_command( diff --git a/crates/nuncio-core/src/config.rs b/crates/nuncio-core/src/config.rs index 348bbec..8c5fc99 100644 --- a/crates/nuncio-core/src/config.rs +++ b/crates/nuncio-core/src/config.rs @@ -74,7 +74,74 @@ pub enum ConfigError { }, } -/// Complete configuration schema for a mail & calendar account. +/// IMAP/SMTP transport parameters: inbound mail is fetched over IMAP4rev1 and +/// outbound mail is submitted over SMTP. Inbound and outbound carry +/// independent host/port/TLS settings because a provider's IMAP and SMTP +/// endpoints commonly differ. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct ImapSmtpTransport { + /// IMAP server hostname or IP address (inbound mail). + pub imap_host: String, + /// IMAP server port number. + pub imap_port: u16, + /// IMAP connection security transport mode. + pub imap_tls_mode: TlsMode, + /// SMTP server hostname or IP address (outbound mail). + pub smtp_host: String, + /// SMTP server port number. + pub smtp_port: u16, + /// SMTP connection security transport mode. + pub smtp_tls_mode: TlsMode, +} + +/// JMAP transport parameters (RFC 8620 / RFC 8621). A JMAP account addresses +/// its provider by a session endpoint host; the daemon derives the well-known +/// session URL from it. JMAP always runs over HTTPS, so there is no separate +/// cleartext TLS mode to configure. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct JmapTransport { + /// JMAP session endpoint host (the daemon derives the session URL from it). + pub endpoint_host: String, +} + +/// DAV transport parameters (CalDAV RFC 4791 today, CardDAV later). A DAV +/// account is addressed by a fully-qualified collection URL rather than a mail +/// host/port, because a DAV client dispatches requests directly at that URL. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct DavTransport { + /// Fully-qualified collection URL (scheme + host + path). + pub collection_url: String, +} + +/// The single transport an account speaks. Modeling it as an enum makes a +/// nonsensical mix of transport settings unrepresentable: an account carries +/// exactly one variant, so the IMAP/SMTP, JMAP, and DAV parameters can never +/// be populated simultaneously. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "kebab-case")] +pub enum Transport { + /// IMAP inbound + SMTP outbound mail. + ImapSmtp(ImapSmtpTransport), + /// JMAP mail. + Jmap(JmapTransport), + /// WebDAV collection (CalDAV today, CardDAV later). + Dav(DavTransport), +} + +impl Transport { + /// The [`AccountProtocol`] discriminant this transport corresponds to. + pub fn protocol(&self) -> AccountProtocol { + match self { + Transport::ImapSmtp(_) => AccountProtocol::ImapSmtp, + Transport::Jmap(_) => AccountProtocol::Jmap, + Transport::Dav(_) => AccountProtocol::CalDav, + } + } +} + +/// Complete configuration schema for a mail & calendar account. The account's +/// common identity fields live here; its transport-specific parameters live in +/// exactly one [`Transport`] variant (see that type's doc comment). #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct AccountConfig { /// Unique identifier for the account configuration. @@ -83,32 +150,12 @@ pub struct AccountConfig { pub name: String, /// Primary email address associated with the account. pub email_address: String, - /// Protocol engine type. - pub protocol: AccountProtocol, - /// Server hostname or IP address (IMAP/JMAP). - pub server_host: String, - /// Server port number (IMAP/JMAP). - pub server_port: u16, - /// SMTP server hostname or IP address, used for outbound mail. Distinct - /// from `server_host` because a mail provider's inbound (IMAP/JMAP) and - /// outbound (SMTP) endpoints commonly differ. - pub smtp_host: String, - /// SMTP server port number, used for outbound mail. - pub smtp_port: u16, - /// IMAP connection security transport mode. - pub imap_tls_mode: TlsMode, - /// SMTP connection security transport mode. - pub smtp_tls_mode: TlsMode, /// Keyring entry key used to retrieve OS vault credentials. pub keyring_secret_key: String, /// Background sync polling interval in seconds (minimum 10s). pub sync_interval_secs: u64, - /// Fully-qualified collection URL for DAV-protocol accounts (CalDAV - /// today, CardDAV later). Set for accounts whose protocol speaks WebDAV; - /// empty for mail accounts. This is a full URL (scheme + host + path), - /// not a bare host, because a DAV client dispatches requests directly at - /// the collection URL. - pub collection_url: String, + /// The account's single transport. + pub transport: Transport, } /// Whether `host` refers to the local loopback interface, either as a @@ -144,7 +191,37 @@ impl AccountConfig { /// Minimum allowed background sync interval (10 seconds). pub const MIN_SYNC_INTERVAL_SECS: u64 = 10; - /// Validate the account configuration fields. + /// The [`AccountProtocol`] discriminant of this account's transport. + pub fn protocol(&self) -> AccountProtocol { + self.transport.protocol() + } + + /// Whether this account speaks WebDAV against a collection URL. + pub fn is_dav(&self) -> bool { + matches!(self.transport, Transport::Dav(_)) + } + + /// The account's IMAP/SMTP transport parameters, present only for an + /// IMAP/SMTP account (the only transport with an SMTP outbound endpoint). + pub fn imap_smtp(&self) -> Option<&ImapSmtpTransport> { + match &self.transport { + Transport::ImapSmtp(t) => Some(t), + _ => None, + } + } + + /// The account's DAV collection URL, present only for a DAV account. + pub fn dav_collection_url(&self) -> Option<&str> { + match &self.transport { + Transport::Dav(t) => Some(t.collection_url.as_str()), + _ => None, + } + } + + /// Validate the account configuration fields. Exactly one transport is + /// always present (guaranteed by the [`Transport`] enum); each variant's + /// endpoint fields are validated against the rules that apply to it, + /// including the cleartext-to-remote-host refusal for any plain endpoint. pub fn validate(&self) -> Result<(), ConfigError> { if self.id.trim().is_empty() { return Err(ConfigError::EmptyField { field: "id" }); @@ -165,57 +242,67 @@ impl AccountConfig { { return Err(ConfigError::InvalidEmailFormat); } - if self.protocol.is_dav() { - // A DAV-protocol account is addressed by its collection URL, not - // by mail host/port endpoints, so it validates the URL instead of - // the IMAP/SMTP fields (which are left empty for such accounts). - let url = self.collection_url.trim(); - if url.is_empty() { - return Err(ConfigError::EmptyField { - field: "collection_url", - }); - } - if !(url.starts_with("http://") || url.starts_with("https://")) { - return Err(ConfigError::InvalidCollectionUrl); - } - // Plain `http://` is only tolerated against a loopback host (a - // local test server); any remote host must use `https://` to - // avoid sending the account's Basic-auth credentials in cleartext. - if url.starts_with("http://") { - let host = url_host(url).unwrap_or_default(); - if !is_loopback_host(host) { + match &self.transport { + Transport::ImapSmtp(t) => { + if t.imap_host.trim().is_empty() { + return Err(ConfigError::EmptyField { field: "imap_host" }); + } + if t.imap_port == 0 { + return Err(ConfigError::InvalidPort); + } + if t.smtp_host.trim().is_empty() { + return Err(ConfigError::EmptyField { field: "smtp_host" }); + } + if t.smtp_port == 0 { + return Err(ConfigError::InvalidPort); + } + // Cleartext `Plain` is only tolerated against a loopback host + // (a local test server); a remote host would send the + // account's credential over the network unencrypted. + if t.imap_tls_mode == TlsMode::Plain && !is_loopback_host(&t.imap_host) { return Err(ConfigError::CleartextToRemoteHost { - endpoint: "caldav", - host: host.to_string(), + endpoint: "imap", + host: t.imap_host.clone(), + }); + } + if t.smtp_tls_mode == TlsMode::Plain && !is_loopback_host(&t.smtp_host) { + return Err(ConfigError::CleartextToRemoteHost { + endpoint: "smtp", + host: t.smtp_host.clone(), }); } } - } else { - if self.server_host.trim().is_empty() { - return Err(ConfigError::EmptyField { - field: "server_host", - }); + Transport::Jmap(t) => { + if t.endpoint_host.trim().is_empty() { + return Err(ConfigError::EmptyField { + field: "endpoint_host", + }); + } } - if self.server_port == 0 { - return Err(ConfigError::InvalidPort); - } - if self.smtp_host.trim().is_empty() { - return Err(ConfigError::EmptyField { field: "smtp_host" }); - } - if self.smtp_port == 0 { - return Err(ConfigError::InvalidPort); - } - if self.imap_tls_mode == TlsMode::Plain && !is_loopback_host(&self.server_host) { - return Err(ConfigError::CleartextToRemoteHost { - endpoint: "imap", - host: self.server_host.clone(), - }); - } - if self.smtp_tls_mode == TlsMode::Plain && !is_loopback_host(&self.smtp_host) { - return Err(ConfigError::CleartextToRemoteHost { - endpoint: "smtp", - host: self.smtp_host.clone(), - }); + Transport::Dav(t) => { + // A DAV account is addressed by its collection URL, not by mail + // host/port endpoints. + let url = t.collection_url.trim(); + if url.is_empty() { + return Err(ConfigError::EmptyField { + field: "collection_url", + }); + } + if !(url.starts_with("http://") || url.starts_with("https://")) { + return Err(ConfigError::InvalidCollectionUrl); + } + // Plain `http://` is only tolerated against a loopback host; any + // remote host must use `https://` to avoid sending the + // account's Basic-auth credentials in cleartext. + if url.starts_with("http://") { + let host = url_host(url).unwrap_or_default(); + if !is_loopback_host(host) { + return Err(ConfigError::CleartextToRemoteHost { + endpoint: "caldav", + host: host.to_string(), + }); + } + } } } if self.keyring_secret_key.trim().is_empty() { @@ -235,21 +322,41 @@ impl AccountConfig { mod tests { use super::*; + fn valid_imap_smtp_transport() -> ImapSmtpTransport { + ImapSmtpTransport { + imap_host: "imap.nuncio.mx".to_string(), + imap_port: 993, + imap_tls_mode: TlsMode::ImplicitTls, + smtp_host: "smtp.nuncio.mx".to_string(), + smtp_port: 465, + smtp_tls_mode: TlsMode::ImplicitTls, + } + } + + /// A valid IMAP/SMTP account. Named `valid_account` (rather than a + /// transport-specific name) because most common-field validation tests + /// below use it as their baseline. fn valid_account() -> AccountConfig { AccountConfig { id: "acct-123".to_string(), name: "Personal Mail".to_string(), email_address: "user@nuncio.mx".to_string(), - protocol: AccountProtocol::Jmap, - server_host: "jmap.nuncio.mx".to_string(), - server_port: 443, - smtp_host: "smtp.nuncio.mx".to_string(), - smtp_port: 465, - imap_tls_mode: TlsMode::ImplicitTls, - smtp_tls_mode: TlsMode::ImplicitTls, keyring_secret_key: "nuncio/acct-123".to_string(), sync_interval_secs: 60, - collection_url: String::new(), + transport: Transport::ImapSmtp(valid_imap_smtp_transport()), + } + } + + fn valid_jmap_account() -> AccountConfig { + AccountConfig { + id: "acct-jmap-1".to_string(), + name: "JMAP Mail".to_string(), + email_address: "user@nuncio.mx".to_string(), + keyring_secret_key: "nuncio/acct-jmap-1".to_string(), + sync_interval_secs: 60, + transport: Transport::Jmap(JmapTransport { + endpoint_host: "jmap.nuncio.mx".to_string(), + }), } } @@ -258,16 +365,20 @@ mod tests { id: "acct-cal-1".to_string(), name: "Work Calendar".to_string(), email_address: "user@nuncio.mx".to_string(), - protocol: AccountProtocol::CalDav, - server_host: String::new(), - server_port: 0, - smtp_host: String::new(), - smtp_port: 0, - imap_tls_mode: TlsMode::ImplicitTls, - smtp_tls_mode: TlsMode::ImplicitTls, keyring_secret_key: "nuncio/acct-cal-1".to_string(), sync_interval_secs: 300, - collection_url: "https://caldav.example.com/calendars/user/work/".to_string(), + transport: Transport::Dav(DavTransport { + collection_url: "https://caldav.example.com/calendars/user/work/".to_string(), + }), + } + } + + /// Convenience: the account's IMAP/SMTP transport, for tests that mutate a + /// transport field on a baseline built by [`valid_account`]. + fn imap_smtp_mut(config: &mut AccountConfig) -> &mut ImapSmtpTransport { + match &mut config.transport { + Transport::ImapSmtp(t) => t, + _ => panic!("expected an IMAP/SMTP transport"), } } @@ -277,10 +388,17 @@ mod tests { assert!(config.validate().is_ok()); } + /// Rebuild `config` with a DAV transport carrying `url`. + fn with_dav_url(config: &mut AccountConfig, url: &str) { + config.transport = Transport::Dav(DavTransport { + collection_url: url.to_string(), + }); + } + #[test] fn caldav_account_requires_collection_url() { let mut config = valid_caldav_account(); - config.collection_url = " ".to_string(); + with_dav_url(&mut config, " "); assert_eq!( config.validate().unwrap_err(), ConfigError::EmptyField { @@ -292,7 +410,7 @@ mod tests { #[test] fn caldav_account_rejects_non_http_collection_url() { let mut config = valid_caldav_account(); - config.collection_url = "ftp://caldav.example.com/work/".to_string(); + with_dav_url(&mut config, "ftp://caldav.example.com/work/"); assert_eq!( config.validate().unwrap_err(), ConfigError::InvalidCollectionUrl @@ -302,7 +420,7 @@ mod tests { #[test] fn caldav_account_rejects_cleartext_http_to_remote_host() { let mut config = valid_caldav_account(); - config.collection_url = "http://remote/dav/".to_string(); + with_dav_url(&mut config, "http://remote/dav/"); assert_eq!( config.validate().unwrap_err(), ConfigError::CleartextToRemoteHost { @@ -315,7 +433,7 @@ mod tests { #[test] fn caldav_account_accepts_https_to_remote_host() { let mut config = valid_caldav_account(); - config.collection_url = "https://remote/dav/".to_string(); + with_dav_url(&mut config, "https://remote/dav/"); assert!(config.validate().is_ok()); } @@ -323,7 +441,7 @@ mod tests { fn caldav_account_accepts_cleartext_http_to_loopback_host() { for url in ["http://127.0.0.1/dav/", "http://localhost/dav/"] { let mut config = valid_caldav_account(); - config.collection_url = url.to_string(); + with_dav_url(&mut config, url); assert!(config.validate().is_ok(), "url {url} should be allowed"); } } @@ -335,10 +453,34 @@ mod tests { assert!(!AccountProtocol::ImapSmtp.is_dav()); } + #[test] + fn transport_reports_its_protocol_discriminant() { + assert_eq!(valid_account().protocol(), AccountProtocol::ImapSmtp); + assert_eq!(valid_jmap_account().protocol(), AccountProtocol::Jmap); + assert_eq!(valid_caldav_account().protocol(), AccountProtocol::CalDav); + assert!(valid_caldav_account().is_dav()); + assert!(!valid_account().is_dav()); + } + #[test] fn valid_account_config_passes_validation() { - let config = valid_account(); - assert!(config.validate().is_ok()); + assert!(valid_account().validate().is_ok()); + assert!(valid_jmap_account().validate().is_ok()); + assert!(valid_caldav_account().validate().is_ok()); + } + + #[test] + fn jmap_account_requires_endpoint_host() { + let mut config = valid_jmap_account(); + config.transport = Transport::Jmap(JmapTransport { + endpoint_host: " ".to_string(), + }); + assert_eq!( + config.validate().unwrap_err(), + ConfigError::EmptyField { + field: "endpoint_host" + } + ); } #[test] @@ -390,28 +532,26 @@ mod tests { } #[test] - fn empty_server_host_fails_validation() { + fn empty_imap_host_fails_validation() { let mut config = valid_account(); - config.server_host = " ".to_string(); + imap_smtp_mut(&mut config).imap_host = " ".to_string(); assert_eq!( config.validate().unwrap_err(), - ConfigError::EmptyField { - field: "server_host" - } + ConfigError::EmptyField { field: "imap_host" } ); } #[test] fn zero_port_fails_validation() { let mut config = valid_account(); - config.server_port = 0; + imap_smtp_mut(&mut config).imap_port = 0; assert_eq!(config.validate().unwrap_err(), ConfigError::InvalidPort); } #[test] fn empty_smtp_host_fails_validation() { let mut config = valid_account(); - config.smtp_host = " ".to_string(); + imap_smtp_mut(&mut config).smtp_host = " ".to_string(); assert_eq!( config.validate().unwrap_err(), ConfigError::EmptyField { field: "smtp_host" } @@ -421,7 +561,7 @@ mod tests { #[test] fn zero_smtp_port_fails_validation() { let mut config = valid_account(); - config.smtp_port = 0; + imap_smtp_mut(&mut config).smtp_port = 0; assert_eq!(config.validate().unwrap_err(), ConfigError::InvalidPort); } @@ -450,12 +590,12 @@ mod tests { #[test] fn plain_imap_to_remote_host_fails_validation() { let mut config = valid_account(); - config.imap_tls_mode = TlsMode::Plain; + imap_smtp_mut(&mut config).imap_tls_mode = TlsMode::Plain; assert_eq!( config.validate().unwrap_err(), ConfigError::CleartextToRemoteHost { endpoint: "imap", - host: "jmap.nuncio.mx".to_string(), + host: "imap.nuncio.mx".to_string(), } ); } @@ -463,7 +603,7 @@ mod tests { #[test] fn plain_smtp_to_remote_host_fails_validation() { let mut config = valid_account(); - config.smtp_tls_mode = TlsMode::Plain; + imap_smtp_mut(&mut config).smtp_tls_mode = TlsMode::Plain; assert_eq!( config.validate().unwrap_err(), ConfigError::CleartextToRemoteHost { @@ -477,10 +617,11 @@ mod tests { fn plain_imap_and_smtp_to_loopback_hosts_pass_validation() { for host in ["127.0.0.1", "localhost", "::1", "LOCALHOST"] { let mut config = valid_account(); - config.server_host = host.to_string(); - config.smtp_host = host.to_string(); - config.imap_tls_mode = TlsMode::Plain; - config.smtp_tls_mode = TlsMode::Plain; + let t = imap_smtp_mut(&mut config); + t.imap_host = host.to_string(); + t.smtp_host = host.to_string(); + t.imap_tls_mode = TlsMode::Plain; + t.smtp_tls_mode = TlsMode::Plain; assert!(config.validate().is_ok(), "host {host} should be allowed"); } } @@ -488,8 +629,9 @@ mod tests { #[test] fn implicit_tls_and_starttls_to_remote_host_pass_validation() { let mut config = valid_account(); - config.imap_tls_mode = TlsMode::ImplicitTls; - config.smtp_tls_mode = TlsMode::StartTls; + let t = imap_smtp_mut(&mut config); + t.imap_tls_mode = TlsMode::ImplicitTls; + t.smtp_tls_mode = TlsMode::StartTls; assert!(config.validate().is_ok()); } diff --git a/crates/nuncio-core/src/lib.rs b/crates/nuncio-core/src/lib.rs index 4c0ca72..f72c4b7 100644 --- a/crates/nuncio-core/src/lib.rs +++ b/crates/nuncio-core/src/lib.rs @@ -10,7 +10,10 @@ pub mod model; pub mod update; pub mod worm_audit; -pub use config::{AccountConfig, AccountProtocol, ConfigError, TlsMode}; +pub use config::{ + AccountConfig, AccountProtocol, ConfigError, DavTransport, ImapSmtpTransport, JmapTransport, + TlsMode, Transport, +}; pub use export::{ExportEngine, ExportError, ExportFormat, ExportSummary}; pub use mcp_policy::{AgentPermissions, DataType, McpAgentPolicy}; pub use model::{Attachment, CalendarEvent, Contact, DaemonTelemetry, Email, Folder}; diff --git a/crates/nuncio-proto/proto/nuncio/v1/descriptor.bin b/crates/nuncio-proto/proto/nuncio/v1/descriptor.bin index 21045b8042fb3ee6a230729a84bae43aa4d01b36..f86c1175b676558b7b70987758876f4ac86bf1d1 100644 GIT binary patch delta 691 zcmZWn%SyvQ6g6$yCX-G(Er>x~NJSBZprUR>ufL=}btb5#016;!5xz zg#LlQ;|I9)PSU4t4w-Y$J@=Kbmyg#^DVKYXNBNvoy8n?-t=+eRZl4D?p+%#Bg}lCo zR1e7B6N|VmZAd$+sR*J4?`cpd^&A9o7Q7aj#|WpH+>d&F=D2!3`Bt~Dz%YUenW3B+ z3SwfI22;aAW+;e>VH%to7Qt{eIYrvAafomSx|;R9iLgtZfO(Y9_gBwqxnb;v x>1L;ofZc`MEDa*J2PZNYuW#ao+nKYUVEidgZvI(+!$P&D<0NP0=7?%Hd%p_B(r5qx delta 263 zcmbPRb*Ov;BNNm9;?2xVjO>gHCg*TP)thl~6cpu`E)H?C1>XA zl^NVv9F3HT#3oXj=V&PyH;0R(bElvd) KySb5PjsXB72~X<) diff --git a/crates/nuncio-proto/proto/nuncio/v1/nuncio.proto b/crates/nuncio-proto/proto/nuncio/v1/nuncio.proto index 6275953..0100fcd 100644 --- a/crates/nuncio-proto/proto/nuncio/v1/nuncio.proto +++ b/crates/nuncio-proto/proto/nuncio/v1/nuncio.proto @@ -250,10 +250,54 @@ enum TlsMode { TLS_MODE_PLAIN = 3; } -// Mirrors `nuncio_core::AccountConfig` field-for-field, MINUS the password -// credential: this message is used both as the request payload (alongside -// a separate `password` field) and as the response payload of -// `ListAccounts`, and the password MUST NEVER appear in the latter. +// IMAP/SMTP transport parameters: an account whose inbound mail is fetched +// over IMAP4rev1 and whose outbound mail is submitted over SMTP. Inbound and +// outbound carry independent host/port/TLS settings because a provider's +// IMAP and SMTP endpoints commonly differ. +message ImapSmtpTransport { + // IMAP server hostname or IP address (inbound mail). + string imap_host = 1; + // IMAP server port number (1..=65535). + uint32 imap_port = 2; + // IMAP connection security transport mode. + TlsMode imap_tls_mode = 3; + // SMTP server hostname or IP address (outbound mail). + string smtp_host = 4; + // SMTP server port number (1..=65535). + uint32 smtp_port = 5; + // SMTP connection security transport mode. + TlsMode smtp_tls_mode = 6; +} + +// JMAP transport parameters (RFC 8620 / RFC 8621). A JMAP account addresses +// its provider by a session endpoint host; the daemon derives the well-known +// session URL from it. JMAP always runs over HTTPS, so there is no separate +// cleartext TLS mode to configure here. +message JmapTransport { + // JMAP session endpoint host (the daemon derives the session URL from it). + string endpoint_host = 1; +} + +// DAV transport parameters (CalDAV RFC 4791 today, CardDAV later). A DAV +// account is addressed by a fully-qualified collection URL rather than a mail +// host/port, because a DAV client dispatches requests directly at that URL. +message DavTransport { + // Fully-qualified collection URL (scheme + host + path), e.g. a CalDAV + // calendar collection or, later, a CardDAV address-book collection. + string collection_url = 1; +} + +// Mirrors `nuncio_core::AccountConfig`, MINUS the password credential: this +// message is used both as the request payload (alongside a separate +// `password` field) and as the response payload of `ListAccounts`, and the +// password MUST NEVER appear in the latter. +// +// An account has EXACTLY ONE transport, carried in the `transport` oneof. This +// makes a nonsensical mix (e.g. IMAP host + JMAP endpoint + DAV url all set at +// once) structurally unrepresentable rather than merely discouraged: the +// transport-specific fields live inside their variant sub-message, so only one +// variant's fields can ever be populated. A request with no transport case set +// is rejected as a validation failure. message AccountConfig { // Unique identifier for the account configuration. OPAQUE and // server-assigned: clients MUST NOT parse or construct this value, only @@ -264,34 +308,25 @@ message AccountConfig { string name = 2; // Primary email address associated with the account. string email_address = 3; - // Protocol engine type. - AccountProtocol protocol = 4; - // Server hostname or IP address (IMAP/JMAP). - string server_host = 5; - // Server port number (1..=65535) (IMAP/JMAP). - uint32 server_port = 6; - reserved 7; - reserved "use_tls"; - // IMAP connection security transport mode. - TlsMode imap_tls_mode = 8; - // SMTP connection security transport mode. - TlsMode smtp_tls_mode = 9; + // Field numbers 4-9 and 12-14 held the flat, transport-specific fields + // (protocol, server_host, server_port, use_tls, imap/smtp TLS modes, + // smtp_host, smtp_port, collection_url) before they moved into the + // per-transport sub-messages carried by the `transport` oneof. They are + // reserved so those numbers are never reused with an incompatible meaning. + reserved 4, 5, 6, 7, 8, 9, 12, 13, 14; + reserved "protocol", "server_host", "server_port", "use_tls", "imap_tls_mode", + "smtp_tls_mode", "smtp_host", "smtp_port", "collection_url"; // Keyring entry key used to retrieve OS vault credentials. string keyring_secret_key = 10; // Background sync polling interval (minimum 10s). google.protobuf.Duration sync_interval = 11; - // SMTP server hostname or IP address, used for outbound mail. Distinct - // from `server_host` because a mail provider's inbound (IMAP/JMAP) and - // outbound (SMTP) endpoints commonly differ. - string smtp_host = 12; - // SMTP server port number (1..=65535), used for outbound mail. - uint32 smtp_port = 13; - // Fully-qualified collection URL for DAV-protocol accounts (e.g. a CalDAV - // calendar collection or, later, a CardDAV address-book collection). Set - // for accounts whose protocol speaks WebDAV; empty for mail accounts. This - // is a full URL (scheme + host + path), not a bare host, because a DAV - // client dispatches requests directly at the collection URL. - string collection_url = 14; + // The account's single transport. Exactly one case is set; a request with + // none set is rejected. + oneof transport { + ImapSmtpTransport imap_smtp = 15; + JmapTransport jmap = 16; + DavTransport dav = 17; + } } // AddAccountRequest carries the full account configuration plus the raw diff --git a/crates/nuncio-store/src/db.rs b/crates/nuncio-store/src/db.rs index 2ff1c62..898bfa5 100644 --- a/crates/nuncio-store/src/db.rs +++ b/crates/nuncio-store/src/db.rs @@ -954,7 +954,52 @@ impl DatabaseEngine { &self, config: &nuncio_core::AccountConfig, ) -> Result<(), DatabaseError> { - let protocol_str = serde_json::to_string(&config.protocol).unwrap_or_default(); + use nuncio_core::{TlsMode, Transport}; + + let protocol_str = serde_json::to_string(&config.protocol()).unwrap_or_default(); + // Flatten the single active transport onto the physical columns. The + // `protocol` column doubles as the transport-kind discriminator that + // `list_accounts` reads back to reconstruct the exact variant. Columns + // that a given transport does not use are written empty/zero (and a + // default TLS mode for the NOT NULL `*_tls_mode` columns); they are + // ignored on load for that transport. + let ( + server_host, + server_port, + imap_tls_mode, + smtp_host, + smtp_port, + smtp_tls_mode, + collection_url, + ) = match &config.transport { + Transport::ImapSmtp(t) => ( + t.imap_host.clone(), + t.imap_port, + t.imap_tls_mode, + t.smtp_host.clone(), + t.smtp_port, + t.smtp_tls_mode, + String::new(), + ), + Transport::Jmap(t) => ( + t.endpoint_host.clone(), + 0u16, + TlsMode::ImplicitTls, + String::new(), + 0u16, + TlsMode::ImplicitTls, + String::new(), + ), + Transport::Dav(t) => ( + String::new(), + 0u16, + TlsMode::ImplicitTls, + String::new(), + 0u16, + TlsMode::ImplicitTls, + t.collection_url.clone(), + ), + }; sqlx::query( r#" INSERT OR REPLACE INTO accounts @@ -966,8 +1011,8 @@ impl DatabaseEngine { .bind(&config.name) .bind(&config.email_address) .bind(protocol_str) - .bind(&config.server_host) - .bind(config.server_port as i64) + .bind(&server_host) + .bind(i64::from(server_port)) // `use_tls` is a retained-but-unread on-disk column (see the // `CREATE TABLE`'s comment) -- written as a fixed placeholder since // no `AccountConfig` field backs it anymore, satisfying its @@ -977,11 +1022,11 @@ impl DatabaseEngine { .bind(1i64) .bind(&config.keyring_secret_key) .bind(config.sync_interval_secs as i64) - .bind(&config.smtp_host) - .bind(config.smtp_port as i64) - .bind(tls_mode_to_db(config.imap_tls_mode)) - .bind(tls_mode_to_db(config.smtp_tls_mode)) - .bind(&config.collection_url) + .bind(&smtp_host) + .bind(i64::from(smtp_port)) + .bind(tls_mode_to_db(imap_tls_mode)) + .bind(tls_mode_to_db(smtp_tls_mode)) + .bind(&collection_url) .execute(&self.pool) .await .map_err(DatabaseError::Query)?; @@ -1057,31 +1102,50 @@ impl DatabaseEngine { )| { let protocol = serde_json::from_str(&protocol_str) .unwrap_or(nuncio_core::AccountProtocol::ImapSmtp); - // Backfill-safe fallback: a row - // written before `smtp_host`/`smtp_port` existed has - // `NULL` in both columns (see - // `Self::ensure_accounts_smtp_columns`). Rather than - // surface an incomplete/invalid config, fall back to the - // same host/port already used for IMAP/JMAP -- the best - // available default for an account configured before - // outbound mail had its own endpoint. - let resolved_smtp_host = smtp_host.unwrap_or_else(|| server_host.clone()); - let resolved_smtp_port = - smtp_port.map(|p| p as u16).unwrap_or(server_port as u16); + // Reconstruct the exact transport variant from the + // `protocol` discriminator column, reading only the columns + // that variant uses (see `save_account`). + let transport = match protocol { + nuncio_core::AccountProtocol::ImapSmtp => { + // Backfill-safe fallback: a row written before + // `smtp_host`/`smtp_port` existed has `NULL` in both + // columns (see `Self::ensure_accounts_smtp_columns`). + // Rather than surface an incomplete/invalid config, + // fall back to the same host/port already used for + // IMAP -- the best available default for an account + // configured before outbound mail had its own + // endpoint. + let resolved_smtp_host = + smtp_host.unwrap_or_else(|| server_host.clone()); + let resolved_smtp_port = + smtp_port.map(|p| p as u16).unwrap_or(server_port as u16); + nuncio_core::Transport::ImapSmtp(nuncio_core::ImapSmtpTransport { + imap_host: server_host, + imap_port: server_port as u16, + imap_tls_mode: tls_mode_from_db(&imap_tls_mode), + smtp_host: resolved_smtp_host, + smtp_port: resolved_smtp_port, + smtp_tls_mode: tls_mode_from_db(&smtp_tls_mode), + }) + } + nuncio_core::AccountProtocol::Jmap => { + nuncio_core::Transport::Jmap(nuncio_core::JmapTransport { + endpoint_host: server_host, + }) + } + nuncio_core::AccountProtocol::CalDav => { + nuncio_core::Transport::Dav(nuncio_core::DavTransport { + collection_url: collection_url.unwrap_or_default(), + }) + } + }; nuncio_core::AccountConfig { id, name, email_address, - protocol, - server_host, - server_port: server_port as u16, - smtp_host: resolved_smtp_host, - smtp_port: resolved_smtp_port, - imap_tls_mode: tls_mode_from_db(&imap_tls_mode), - smtp_tls_mode: tls_mode_from_db(&smtp_tls_mode), keyring_secret_key, sync_interval_secs: sync_interval_secs as u64, - collection_url: collection_url.unwrap_or_default(), + transport, } }, ) @@ -2767,16 +2831,16 @@ mod tests { id: "acct-contended-1".to_string(), name: "Contended Account".to_string(), email_address: "contended@nuncio.mx".to_string(), - protocol: nuncio_core::AccountProtocol::ImapSmtp, - server_host: "imap.nuncio.mx".to_string(), - server_port: 993, - smtp_host: "smtp.nuncio.mx".to_string(), - smtp_port: 465, - imap_tls_mode: nuncio_core::TlsMode::ImplicitTls, - smtp_tls_mode: nuncio_core::TlsMode::ImplicitTls, keyring_secret_key: "nuncio/acct-contended-1".to_string(), sync_interval_secs: 60, - collection_url: String::new(), + transport: nuncio_core::Transport::ImapSmtp(nuncio_core::ImapSmtpTransport { + imap_host: "imap.nuncio.mx".to_string(), + imap_port: 993, + imap_tls_mode: nuncio_core::TlsMode::ImplicitTls, + smtp_host: "smtp.nuncio.mx".to_string(), + smtp_port: 465, + smtp_tls_mode: nuncio_core::TlsMode::ImplicitTls, + }), }; engine.save_account(&acct).await.unwrap(); engine.close().await; @@ -3346,16 +3410,16 @@ mod tests { id: "acct-test-1".to_string(), name: "Work Account".to_string(), email_address: "work@nuncio.mx".to_string(), - protocol: nuncio_core::AccountProtocol::ImapSmtp, - server_host: "imap.nuncio.mx".to_string(), - server_port: 993, - smtp_host: "smtp.nuncio.mx".to_string(), - smtp_port: 465, - imap_tls_mode: nuncio_core::TlsMode::ImplicitTls, - smtp_tls_mode: nuncio_core::TlsMode::ImplicitTls, keyring_secret_key: "nuncio/acct-test-1".to_string(), sync_interval_secs: 60, - collection_url: String::new(), + transport: nuncio_core::Transport::ImapSmtp(nuncio_core::ImapSmtpTransport { + imap_host: "imap.nuncio.mx".to_string(), + imap_port: 993, + imap_tls_mode: nuncio_core::TlsMode::ImplicitTls, + smtp_host: "smtp.nuncio.mx".to_string(), + smtp_port: 465, + smtp_tls_mode: nuncio_core::TlsMode::ImplicitTls, + }), }; engine @@ -3369,8 +3433,9 @@ mod tests { assert_eq!(accounts.len(), 1); assert_eq!(accounts[0].id, "acct-test-1"); assert_eq!(accounts[0].email_address, "work@nuncio.mx"); - assert_eq!(accounts[0].smtp_host, "smtp.nuncio.mx"); - assert_eq!(accounts[0].smtp_port, 465); + let t = accounts[0].imap_smtp().expect("imap-smtp transport"); + assert_eq!(t.smtp_host, "smtp.nuncio.mx"); + assert_eq!(t.smtp_port, 465); } /// Regression proof that the per-protocol TLS transport modes are @@ -3389,32 +3454,34 @@ mod tests { id: "acct-tls-1".to_string(), name: "Mixed TLS Account".to_string(), email_address: "mixed@nuncio.mx".to_string(), - protocol: nuncio_core::AccountProtocol::ImapSmtp, - server_host: "imap.nuncio.mx".to_string(), - server_port: 143, - smtp_host: "smtp.nuncio.mx".to_string(), - smtp_port: 25, - imap_tls_mode: nuncio_core::TlsMode::StartTls, - smtp_tls_mode: nuncio_core::TlsMode::Plain, keyring_secret_key: "nuncio/acct-tls-1".to_string(), sync_interval_secs: 60, - collection_url: String::new(), + transport: nuncio_core::Transport::ImapSmtp(nuncio_core::ImapSmtpTransport { + imap_host: "imap.nuncio.mx".to_string(), + imap_port: 143, + imap_tls_mode: nuncio_core::TlsMode::StartTls, + smtp_host: "smtp.nuncio.mx".to_string(), + smtp_port: 25, + smtp_tls_mode: nuncio_core::TlsMode::Plain, + }), }; engine.save_account(&acct).await.expect("save succeeds"); let listed = engine.list_accounts().await.expect("list succeeds"); assert_eq!(listed.len(), 1); - assert_eq!(listed[0].imap_tls_mode, nuncio_core::TlsMode::StartTls); - assert_eq!(listed[0].smtp_tls_mode, nuncio_core::TlsMode::Plain); + let lt = listed[0].imap_smtp().expect("imap-smtp transport"); + assert_eq!(lt.imap_tls_mode, nuncio_core::TlsMode::StartTls); + assert_eq!(lt.smtp_tls_mode, nuncio_core::TlsMode::Plain); let fetched = engine .get_account("acct-tls-1") .await .expect("get succeeds") .expect("account present"); - assert_eq!(fetched.imap_tls_mode, nuncio_core::TlsMode::StartTls); - assert_eq!(fetched.smtp_tls_mode, nuncio_core::TlsMode::Plain); + let ft = fetched.imap_smtp().expect("imap-smtp transport"); + assert_eq!(ft.imap_tls_mode, nuncio_core::TlsMode::StartTls); + assert_eq!(ft.smtp_tls_mode, nuncio_core::TlsMode::Plain); assert_eq!(fetched, acct); } @@ -3429,16 +3496,11 @@ mod tests { id: "acct-caldav-store-1".to_string(), name: "Work Calendar".to_string(), email_address: "cal@nuncio.mx".to_string(), - protocol: nuncio_core::AccountProtocol::CalDav, - server_host: String::new(), - server_port: 0, - smtp_host: String::new(), - smtp_port: 0, - imap_tls_mode: nuncio_core::TlsMode::ImplicitTls, - smtp_tls_mode: nuncio_core::TlsMode::ImplicitTls, keyring_secret_key: "nuncio/acct-caldav-store-1".to_string(), sync_interval_secs: 300, - collection_url: "https://dav.example.com/calendars/user/work/".to_string(), + transport: nuncio_core::Transport::Dav(nuncio_core::DavTransport { + collection_url: "https://dav.example.com/calendars/user/work/".to_string(), + }), }; engine.save_account(&caldav).await.expect("save succeeds"); @@ -3448,13 +3510,77 @@ mod tests { .expect("get succeeds") .expect("account present"); assert_eq!(fetched, caldav); - assert_eq!(fetched.protocol, nuncio_core::AccountProtocol::CalDav); + assert_eq!(fetched.protocol(), nuncio_core::AccountProtocol::CalDav); assert_eq!( - fetched.collection_url, - "https://dav.example.com/calendars/user/work/" + fetched.dav_collection_url(), + Some("https://dav.example.com/calendars/user/work/") ); } + /// Regression proof that accounts of DIFFERENT transports coexist on disk + /// and each loads back as its exact original transport variant -- the flat + /// physical columns must not blur one transport into another on reload. + #[tokio::test] + async fn distinct_transport_accounts_round_trip_to_their_own_variant() { + let (engine, _dir) = DatabaseEngine::connect_ephemeral().await.unwrap(); + + let imap = nuncio_core::AccountConfig { + id: "acct-mix-imap".to_string(), + name: "IMAP Account".to_string(), + email_address: "imap@nuncio.mx".to_string(), + keyring_secret_key: "nuncio/acct-mix-imap".to_string(), + sync_interval_secs: 60, + transport: nuncio_core::Transport::ImapSmtp(nuncio_core::ImapSmtpTransport { + imap_host: "imap.nuncio.mx".to_string(), + imap_port: 143, + imap_tls_mode: nuncio_core::TlsMode::StartTls, + smtp_host: "smtp.nuncio.mx".to_string(), + smtp_port: 587, + smtp_tls_mode: nuncio_core::TlsMode::StartTls, + }), + }; + let jmap = nuncio_core::AccountConfig { + id: "acct-mix-jmap".to_string(), + name: "JMAP Account".to_string(), + email_address: "jmap@nuncio.mx".to_string(), + keyring_secret_key: "nuncio/acct-mix-jmap".to_string(), + sync_interval_secs: 120, + transport: nuncio_core::Transport::Jmap(nuncio_core::JmapTransport { + endpoint_host: "jmap.nuncio.mx".to_string(), + }), + }; + let dav = nuncio_core::AccountConfig { + id: "acct-mix-dav".to_string(), + name: "DAV Account".to_string(), + email_address: "dav@nuncio.mx".to_string(), + keyring_secret_key: "nuncio/acct-mix-dav".to_string(), + sync_interval_secs: 300, + transport: nuncio_core::Transport::Dav(nuncio_core::DavTransport { + collection_url: "https://dav.example.com/cal/".to_string(), + }), + }; + + for acct in [&imap, &jmap, &dav] { + engine.save_account(acct).await.expect("save succeeds"); + } + + let reload = |id: &str| { + let engine = &engine; + let id = id.to_string(); + async move { + engine + .get_account(&id) + .await + .expect("get succeeds") + .expect("account present") + } + }; + + assert_eq!(reload("acct-mix-imap").await, imap); + assert_eq!(reload("acct-mix-jmap").await, jmap); + assert_eq!(reload("acct-mix-dav").await, dav); + } + /// Proves `get_account` resolves a saved account by id and returns `None` /// for an unknown id, and that `delete_account` genuinely removes the row /// (and is idempotent for an already-absent id). @@ -3466,16 +3592,16 @@ mod tests { id: "acct-del-1".to_string(), name: "Deletable".to_string(), email_address: "del@nuncio.mx".to_string(), - protocol: nuncio_core::AccountProtocol::ImapSmtp, - server_host: "imap.nuncio.mx".to_string(), - server_port: 993, - smtp_host: "smtp.nuncio.mx".to_string(), - smtp_port: 465, - imap_tls_mode: nuncio_core::TlsMode::ImplicitTls, - smtp_tls_mode: nuncio_core::TlsMode::ImplicitTls, keyring_secret_key: "nuncio/acct-del-1".to_string(), sync_interval_secs: 60, - collection_url: String::new(), + transport: nuncio_core::Transport::ImapSmtp(nuncio_core::ImapSmtpTransport { + imap_host: "imap.nuncio.mx".to_string(), + imap_port: 993, + imap_tls_mode: nuncio_core::TlsMode::ImplicitTls, + smtp_host: "smtp.nuncio.mx".to_string(), + smtp_port: 465, + smtp_tls_mode: nuncio_core::TlsMode::ImplicitTls, + }), }; engine.save_account(&acct).await.expect("save succeeds"); @@ -3640,8 +3766,9 @@ mod tests { assert_eq!(acct.id, "acct-pre-migration"); // Falls back to the IMAP/JMAP endpoint since smtp_host/smtp_port // were NULL for this pre-existing row. - assert_eq!(acct.smtp_host, "imap.nuncio.mx"); - assert_eq!(acct.smtp_port, 993); + let acct_t = acct.imap_smtp().expect("imap-smtp transport"); + assert_eq!(acct_t.smtp_host, "imap.nuncio.mx"); + assert_eq!(acct_t.smtp_port, 993); // Step 3: re-running migrate() (e.g. a second daemon startup against // the same file) must be a no-op, not an error. @@ -3657,16 +3784,16 @@ mod tests { id: "acct-post-migration".to_string(), name: "Post Migration Account".to_string(), email_address: "post@nuncio.mx".to_string(), - protocol: nuncio_core::AccountProtocol::ImapSmtp, - server_host: "imap.nuncio.mx".to_string(), - server_port: 993, - smtp_host: "smtp.nuncio.mx".to_string(), - smtp_port: 587, - imap_tls_mode: nuncio_core::TlsMode::ImplicitTls, - smtp_tls_mode: nuncio_core::TlsMode::StartTls, keyring_secret_key: "nuncio/acct-post-migration".to_string(), sync_interval_secs: 60, - collection_url: String::new(), + transport: nuncio_core::Transport::ImapSmtp(nuncio_core::ImapSmtpTransport { + imap_host: "imap.nuncio.mx".to_string(), + imap_port: 993, + imap_tls_mode: nuncio_core::TlsMode::ImplicitTls, + smtp_host: "smtp.nuncio.mx".to_string(), + smtp_port: 587, + smtp_tls_mode: nuncio_core::TlsMode::StartTls, + }), }; engine .save_account(&new_acct) @@ -3677,8 +3804,9 @@ mod tests { .iter() .find(|a| a.id == "acct-post-migration") .expect("newly saved account present"); - assert_eq!(post.smtp_host, "smtp.nuncio.mx"); - assert_eq!(post.smtp_port, 587); + let post_t = post.imap_smtp().expect("imap-smtp transport"); + assert_eq!(post_t.smtp_host, "smtp.nuncio.mx"); + assert_eq!(post_t.smtp_port, 587); engine.close().await; } diff --git a/crates/nuncio-store/src/recovery.rs b/crates/nuncio-store/src/recovery.rs index 30a1fa0..177793b 100644 --- a/crates/nuncio-store/src/recovery.rs +++ b/crates/nuncio-store/src/recovery.rs @@ -422,20 +422,38 @@ impl SqliteRecoveryEngine { Self::parse_salvaged_tls_mode(&id, "imap_tls_mode", &imap_tls_mode_raw); let smtp_tls_mode = Self::parse_salvaged_tls_mode(&id, "smtp_tls_mode", &smtp_tls_mode_raw); + // Reconstruct the transport variant from the salvaged + // `protocol` discriminator, mirroring + // `DatabaseEngine::list_accounts`. + let transport = match protocol { + nuncio_core::AccountProtocol::ImapSmtp => { + nuncio_core::Transport::ImapSmtp(nuncio_core::ImapSmtpTransport { + imap_host: server_host, + imap_port: server_port as u16, + imap_tls_mode, + smtp_host: resolved_smtp_host, + smtp_port: resolved_smtp_port, + smtp_tls_mode, + }) + } + nuncio_core::AccountProtocol::Jmap => { + nuncio_core::Transport::Jmap(nuncio_core::JmapTransport { + endpoint_host: server_host, + }) + } + nuncio_core::AccountProtocol::CalDav => { + nuncio_core::Transport::Dav(nuncio_core::DavTransport { + collection_url: collection_url.unwrap_or_default(), + }) + } + }; nuncio_core::AccountConfig { id, name, email_address, - protocol, - server_host, - server_port: server_port as u16, - smtp_host: resolved_smtp_host, - smtp_port: resolved_smtp_port, - imap_tls_mode, - smtp_tls_mode, keyring_secret_key, sync_interval_secs: sync_interval_secs as u64, - collection_url: collection_url.unwrap_or_default(), + transport, } }, ) @@ -598,16 +616,16 @@ mod tests { id: "acct-test-1".to_string(), name: "Work Account".to_string(), email_address: "work@nuncio.mx".to_string(), - protocol: nuncio_core::AccountProtocol::ImapSmtp, - server_host: "imap.nuncio.mx".to_string(), - server_port: 993, - smtp_host: "smtp.nuncio.mx".to_string(), - smtp_port: 465, - imap_tls_mode: nuncio_core::TlsMode::ImplicitTls, - smtp_tls_mode: nuncio_core::TlsMode::ImplicitTls, keyring_secret_key: "nuncio/acct-test-1".to_string(), sync_interval_secs: 60, - collection_url: String::new(), + transport: nuncio_core::Transport::ImapSmtp(nuncio_core::ImapSmtpTransport { + imap_host: "imap.nuncio.mx".to_string(), + imap_port: 993, + imap_tls_mode: nuncio_core::TlsMode::ImplicitTls, + smtp_host: "smtp.nuncio.mx".to_string(), + smtp_port: 465, + smtp_tls_mode: nuncio_core::TlsMode::ImplicitTls, + }), }; engine.save_account(&acct).await.unwrap(); assert!(engine.check_integrity().await.unwrap()); @@ -659,16 +677,16 @@ mod tests { id: "acct-salvage-1".to_string(), name: "Salvage Account".to_string(), email_address: "salvage@nuncio.mx".to_string(), - protocol: nuncio_core::AccountProtocol::ImapSmtp, - server_host: "imap.nuncio.mx".to_string(), - server_port: 993, - smtp_host: "smtp.nuncio.mx".to_string(), - smtp_port: 465, - imap_tls_mode: nuncio_core::TlsMode::ImplicitTls, - smtp_tls_mode: nuncio_core::TlsMode::ImplicitTls, keyring_secret_key: "nuncio/acct-salvage-1".to_string(), sync_interval_secs: 60, - collection_url: String::new(), + transport: nuncio_core::Transport::ImapSmtp(nuncio_core::ImapSmtpTransport { + imap_host: "imap.nuncio.mx".to_string(), + imap_port: 993, + imap_tls_mode: nuncio_core::TlsMode::ImplicitTls, + smtp_host: "smtp.nuncio.mx".to_string(), + smtp_port: 465, + smtp_tls_mode: nuncio_core::TlsMode::ImplicitTls, + }), }; engine.save_account(&acct).await.unwrap(); @@ -796,16 +814,16 @@ mod tests { id: "acct-starttls-1".to_string(), name: "STARTTLS Account".to_string(), email_address: "starttls@nuncio.mx".to_string(), - protocol: nuncio_core::AccountProtocol::ImapSmtp, - server_host: "imap.nuncio.mx".to_string(), - server_port: 143, - smtp_host: "smtp.nuncio.mx".to_string(), - smtp_port: 587, - imap_tls_mode: nuncio_core::TlsMode::StartTls, - smtp_tls_mode: nuncio_core::TlsMode::Plain, keyring_secret_key: "nuncio/acct-starttls-1".to_string(), sync_interval_secs: 60, - collection_url: String::new(), + transport: nuncio_core::Transport::ImapSmtp(nuncio_core::ImapSmtpTransport { + imap_host: "imap.nuncio.mx".to_string(), + imap_port: 143, + imap_tls_mode: nuncio_core::TlsMode::StartTls, + smtp_host: "smtp.nuncio.mx".to_string(), + smtp_port: 587, + smtp_tls_mode: nuncio_core::TlsMode::Plain, + }), }; engine.save_account(&acct).await.unwrap(); } @@ -824,8 +842,9 @@ mod tests { .unwrap(); let accounts = fresh.list_accounts().await.unwrap(); assert_eq!(accounts.len(), 1); - assert_eq!(accounts[0].imap_tls_mode, nuncio_core::TlsMode::StartTls); - assert_eq!(accounts[0].smtp_tls_mode, nuncio_core::TlsMode::Plain); + let salvaged_t = accounts[0].imap_smtp().expect("imap-smtp transport"); + assert_eq!(salvaged_t.imap_tls_mode, nuncio_core::TlsMode::StartTls); + assert_eq!(salvaged_t.smtp_tls_mode, nuncio_core::TlsMode::Plain); } // NOTE: there is deliberately no test combining *header* corruption (bytes 0-16 -- the diff --git a/crates/nunciod/src/calendar_sync.rs b/crates/nunciod/src/calendar_sync.rs index 0eb7f3d..3b9c52f 100644 --- a/crates/nunciod/src/calendar_sync.rs +++ b/crates/nunciod/src/calendar_sync.rs @@ -74,7 +74,7 @@ pub async fn sync_caldav_account( ) -> Result { let client = CalDavClient::new(CalDavAccountConfig { account_id: account.id.clone(), - caldav_url: account.collection_url.clone(), + caldav_url: account.dav_collection_url().unwrap_or_default().to_string(), username: account.email_address.clone(), auth_token: password.to_string(), }); diff --git a/crates/nunciod/src/grpc.rs b/crates/nunciod/src/grpc.rs index 913e3bc..8fdf6cd 100644 --- a/crates/nunciod/src/grpc.rs +++ b/crates/nunciod/src/grpc.rs @@ -14,6 +14,7 @@ use nuncio_core::{CoreCommand, CoreEvent, EventBus}; use nuncio_filter::{FilterEngine, NsqlParser, NsqlValidator, ValidationOptions}; use nuncio_mail::{ImapEngine, MailBackend, MessageSender, SmtpTransportEngine}; use nuncio_proto::errors; +use nuncio_proto::v1::account_config::Transport as TransportProto; use nuncio_proto::v1::accounts_server::{Accounts, AccountsServer}; use nuncio_proto::v1::audit_server::{Audit, AuditServer}; use nuncio_proto::v1::calendar_server::{Calendar, CalendarServer}; @@ -25,25 +26,25 @@ use nuncio_proto::v1::mail_server::{Mail, MailServer}; use nuncio_proto::v1::system_server::{System, SystemServer}; use nuncio_proto::v1::ErrorReason; use nuncio_proto::v1::{ - export_request, AccountConfig as AccountConfigProto, AccountProtocol as AccountProtocolProto, - AddAccountRequest, AddAccountResponse, Attachment as AttachmentProto, - AuditRecord as AuditRecordProto, BatchFilterProgress, CalendarEvent as CalendarEventProto, - CalendarSyncRequest, CalendarSyncResponse, Contact as ContactProto, - ContactEmail as ContactEmailProto, ContactPhone as ContactPhoneProto, ContactsSyncRequest, - ContactsSyncResponse, CreateContactRequest, CreateContactResponse, CreateRuleRequest, - CreateRuleResponse, DatabaseRecovered, DeleteRuleRequest, DeleteRuleResponse, Event, - EventError, ExportFormat as ExportFormatProto, ExportRequest, ExportResponse, - ExportRulesRequest, ExportRulesResponse, FilterExecuted, + export_request, AccountConfig as AccountConfigProto, AddAccountRequest, AddAccountResponse, + Attachment as AttachmentProto, AuditRecord as AuditRecordProto, BatchFilterProgress, + CalendarEvent as CalendarEventProto, CalendarSyncRequest, CalendarSyncResponse, + Contact as ContactProto, ContactEmail as ContactEmailProto, ContactPhone as ContactPhoneProto, + ContactsSyncRequest, ContactsSyncResponse, CreateContactRequest, CreateContactResponse, + CreateRuleRequest, CreateRuleResponse, DatabaseRecovered, DavTransport as DavTransportProto, + DeleteRuleRequest, DeleteRuleResponse, Event, EventError, ExportFormat as ExportFormatProto, + ExportRequest, ExportResponse, ExportRulesRequest, ExportRulesResponse, FilterExecuted, FilterExecutionLog as FilterExecutionLogProto, FilterRule as FilterRuleProto, Folder as FolderProto, GetContactRequest, GetContactResponse, GetEventRequest, GetEventResponse, GetExecutionLogsRequest, GetExecutionLogsResponse, GetMessageRequest, - GetMessageResponse, GetStatusRequest, GetStatusResponse, ImportRulesRequest, - ImportRulesResponse, ListAccountsRequest, ListAccountsResponse, ListContactsRequest, - ListContactsResponse, ListEventsRequest, ListEventsResponse, ListFoldersRequest, - ListFoldersResponse, ListMessagesRequest, ListMessagesResponse, ListRecordsRequest, - ListRecordsResponse, ListRulesRequest, ListRulesResponse, MarkReadRequest, MarkReadResponse, - Message as MessageProto, MessageFlagsChanged, MessageSearchHit, PreviewRuleRequest, - PreviewRuleResponse, RemoveAccountRequest, RemoveAccountResponse, + GetMessageResponse, GetStatusRequest, GetStatusResponse, + ImapSmtpTransport as ImapSmtpTransportProto, ImportRulesRequest, ImportRulesResponse, + JmapTransport as JmapTransportProto, ListAccountsRequest, ListAccountsResponse, + ListContactsRequest, ListContactsResponse, ListEventsRequest, ListEventsResponse, + ListFoldersRequest, ListFoldersResponse, ListMessagesRequest, ListMessagesResponse, + ListRecordsRequest, ListRecordsResponse, ListRulesRequest, ListRulesResponse, MarkReadRequest, + MarkReadResponse, Message as MessageProto, MessageFlagsChanged, MessageSearchHit, + PreviewRuleRequest, PreviewRuleResponse, RemoveAccountRequest, RemoveAccountResponse, RuleExportFormat as RuleExportFormatProto, SearchMessagesRequest, SearchMessagesResponse, SendMessageRequest, SendMessageResponse, ShuttingDown, SubscribeRequest, SyncCompleted, SyncRequest, SyncResponse, SyncStarted, TestAccountConnectionRequest, @@ -275,33 +276,6 @@ impl System for SystemGrpcService { } } -/// Maps a `nuncio_core::AccountProtocol` onto its wire-format -/// `nuncio.v1.AccountProtocol` enum value. -fn map_account_protocol_to_proto(protocol: nuncio_core::AccountProtocol) -> AccountProtocolProto { - match protocol { - nuncio_core::AccountProtocol::Jmap => AccountProtocolProto::Jmap, - nuncio_core::AccountProtocol::ImapSmtp => AccountProtocolProto::ImapSmtp, - nuncio_core::AccountProtocol::CalDav => AccountProtocolProto::Caldav, - } -} - -/// Maps a wire-format `nuncio.v1.AccountProtocol` enum value back onto -/// `nuncio_core::AccountProtocol`. `Unspecified` is treated as an invalid -/// request rather than silently defaulting to a protocol the caller never -/// asked for. -fn map_account_protocol_from_proto( - protocol: AccountProtocolProto, -) -> Result { - match protocol { - AccountProtocolProto::Jmap => Ok(nuncio_core::AccountProtocol::Jmap), - AccountProtocolProto::ImapSmtp => Ok(nuncio_core::AccountProtocol::ImapSmtp), - AccountProtocolProto::Caldav => Ok(nuncio_core::AccountProtocol::CalDav), - AccountProtocolProto::Unspecified => { - Err(Status::invalid_argument("account protocol is required")) - } - } -} - /// Maps a `nuncio_core::TlsMode` onto its wire-format `nuncio.v1.TlsMode` /// enum value. fn map_tls_mode_to_proto(mode: nuncio_core::TlsMode) -> TlsModeProto { @@ -330,56 +304,93 @@ fn map_tls_mode_from_proto(mode: TlsModeProto) -> Result AccountConfigProto { + let transport = match config.transport { + nuncio_core::Transport::ImapSmtp(t) => TransportProto::ImapSmtp(ImapSmtpTransportProto { + imap_host: t.imap_host, + imap_port: u32::from(t.imap_port), + imap_tls_mode: map_tls_mode_to_proto(t.imap_tls_mode).into(), + smtp_host: t.smtp_host, + smtp_port: u32::from(t.smtp_port), + smtp_tls_mode: map_tls_mode_to_proto(t.smtp_tls_mode).into(), + }), + nuncio_core::Transport::Jmap(t) => TransportProto::Jmap(JmapTransportProto { + endpoint_host: t.endpoint_host, + }), + nuncio_core::Transport::Dav(t) => TransportProto::Dav(DavTransportProto { + collection_url: t.collection_url, + }), + }; AccountConfigProto { id: config.id, name: config.name, email_address: config.email_address, - protocol: map_account_protocol_to_proto(config.protocol).into(), - server_host: config.server_host, - server_port: u32::from(config.server_port), - imap_tls_mode: map_tls_mode_to_proto(config.imap_tls_mode).into(), - smtp_tls_mode: map_tls_mode_to_proto(config.smtp_tls_mode).into(), keyring_secret_key: config.keyring_secret_key, sync_interval: Some(nuncio_proto::time::duration_from_secs( config.sync_interval_secs, )), - smtp_host: config.smtp_host, - smtp_port: u32::from(config.smtp_port), - collection_url: config.collection_url, + transport: Some(transport), } } /// Maps a wire-format `nuncio.v1.AccountConfig` request payload back onto -/// `nuncio_core::AccountConfig`. Rejects a `server_port` outside `1..=65535` -/// with `Status::invalid_argument` rather than silently truncating it. +/// `nuncio_core::AccountConfig`. An unset `transport` oneof is a typed +/// `VALIDATION_FAILED` error rather than a panic or a silent default; a port +/// outside `1..=65535` is likewise rejected rather than silently truncated. fn map_account_config_from_proto( config: AccountConfigProto, ) -> Result { - let protocol = map_account_protocol_from_proto(config.protocol())?; - let imap_tls_mode = map_tls_mode_from_proto(config.imap_tls_mode())?; - let smtp_tls_mode = map_tls_mode_from_proto(config.smtp_tls_mode())?; - let server_port = u16::try_from(config.server_port) - .map_err(|_| Status::invalid_argument("server_port must be in range 1..=65535"))?; - let smtp_port = u16::try_from(config.smtp_port) - .map_err(|_| Status::invalid_argument("smtp_port must be in range 1..=65535"))?; - let sync_interval = config - .sync_interval - .ok_or_else(|| Status::invalid_argument("sync_interval is required"))?; + let sync_interval = config.sync_interval.ok_or_else(|| { + errors::status(ErrorReason::ValidationFailed, "sync_interval is required") + })?; + + let transport = match config.transport { + Some(TransportProto::ImapSmtp(t)) => { + let imap_port = u16::try_from(t.imap_port).map_err(|_| { + errors::status( + ErrorReason::ValidationFailed, + "imap_port must be in range 1..=65535", + ) + })?; + let smtp_port = u16::try_from(t.smtp_port).map_err(|_| { + errors::status( + ErrorReason::ValidationFailed, + "smtp_port must be in range 1..=65535", + ) + })?; + // Resolve the TLS-mode enums (which borrow `t`) before moving the + // owned host strings out of `t`. + let imap_tls_mode = map_tls_mode_from_proto(t.imap_tls_mode())?; + let smtp_tls_mode = map_tls_mode_from_proto(t.smtp_tls_mode())?; + nuncio_core::Transport::ImapSmtp(nuncio_core::ImapSmtpTransport { + imap_host: t.imap_host, + imap_port, + imap_tls_mode, + smtp_host: t.smtp_host, + smtp_port, + smtp_tls_mode, + }) + } + Some(TransportProto::Jmap(t)) => nuncio_core::Transport::Jmap(nuncio_core::JmapTransport { + endpoint_host: t.endpoint_host, + }), + Some(TransportProto::Dav(t)) => nuncio_core::Transport::Dav(nuncio_core::DavTransport { + collection_url: t.collection_url, + }), + None => { + return Err(errors::status( + ErrorReason::ValidationFailed, + "account transport is required (one of imap_smtp, jmap, dav must be set)", + )) + } + }; Ok(nuncio_core::AccountConfig { id: config.id, name: config.name, email_address: config.email_address, - protocol, - server_host: config.server_host, - server_port, - smtp_host: config.smtp_host, - smtp_port, - imap_tls_mode, - smtp_tls_mode, keyring_secret_key: config.keyring_secret_key, sync_interval_secs: nuncio_proto::time::duration_to_secs(&sync_interval), - collection_url: config.collection_url, + transport, }) } @@ -452,44 +463,50 @@ impl AccountConnectionTester for RealAccountConnectionTester { config: &nuncio_core::AccountConfig, password: &str, ) -> AccountConnectionReport { - let imap = match config.protocol { - nuncio_core::AccountProtocol::ImapSmtp => { - let engine = ImapEngine::with_credentials( - &config.id, - &config.server_host, - config.server_port, - config.imap_tls_mode, - &config.email_address, - password, - ); - match engine.probe_connection().await { - Ok(()) => ProtocolProbe::ok(), - Err(e) => ProtocolProbe::failed(e.to_string()), + // Only an IMAP/SMTP account has mail endpoints to probe. A JMAP or DAV + // account has no IMAP/SMTP legs, so report that honestly rather than + // dialing fields that do not exist for it. + let transport = match &config.transport { + nuncio_core::Transport::ImapSmtp(t) => t, + nuncio_core::Transport::Jmap(_) => { + return AccountConnectionReport { + imap: ProtocolProbe::failed( + "IMAP connection probe is not applicable to a JMAP account".to_string(), + ), + smtp: ProtocolProbe::failed( + "SMTP connection probe is not applicable to a JMAP account".to_string(), + ), + } + } + nuncio_core::Transport::Dav(_) => { + return AccountConnectionReport { + imap: ProtocolProbe::failed( + "IMAP connection probe is not applicable to a CalDAV account".to_string(), + ), + smtp: ProtocolProbe::failed( + "SMTP connection probe is not applicable to a CalDAV account".to_string(), + ), } } - nuncio_core::AccountProtocol::Jmap => ProtocolProbe::failed( - "IMAP connection probe is not applicable to a JMAP account".to_string(), - ), - nuncio_core::AccountProtocol::CalDav => ProtocolProbe::failed( - "IMAP connection probe is not applicable to a CalDAV account".to_string(), - ), }; - // A CalDAV account has no SMTP endpoint to probe; report that honestly - // rather than dialing the (empty) mail endpoint fields. - if config.protocol == nuncio_core::AccountProtocol::CalDav { - return AccountConnectionReport { - imap, - smtp: ProtocolProbe::failed( - "SMTP connection probe is not applicable to a CalDAV account".to_string(), - ), - }; - } + let engine = ImapEngine::with_credentials( + &config.id, + &transport.imap_host, + transport.imap_port, + transport.imap_tls_mode, + &config.email_address, + password, + ); + let imap = match engine.probe_connection().await { + Ok(()) => ProtocolProbe::ok(), + Err(e) => ProtocolProbe::failed(e.to_string()), + }; let smtp = match SmtpTransportEngine::new( - &config.smtp_host, - config.smtp_port, - config.smtp_tls_mode, + &transport.smtp_host, + transport.smtp_port, + transport.smtp_tls_mode, &config.email_address, password, ) { @@ -1037,7 +1054,7 @@ impl CalendarGrpcService { .await .map_err(|e| Status::internal(format!("failed to list accounts: {e}")))? .into_iter() - .filter(|a| a.protocol == nuncio_core::AccountProtocol::CalDav) + .filter(|a| a.is_dav()) .filter(|a| req.account_id.is_empty() || a.id == req.account_id) .collect(); @@ -3493,19 +3510,38 @@ mod tests { id: id.to_string(), name: "Restart Test Account".to_string(), email_address: format!("{id}@nuncio.mx"), - protocol: AccountProtocolProto::ImapSmtp.into(), - server_host: "imap.nuncio.mx".to_string(), - server_port: 993, - imap_tls_mode: TlsModeProto::ImplicitTls.into(), - smtp_tls_mode: TlsModeProto::ImplicitTls.into(), keyring_secret_key: keyring_secret_key.to_string(), sync_interval: Some(nuncio_proto::time::duration_from_secs(60)), - collection_url: String::new(), - smtp_host: "smtp.nuncio.mx".to_string(), - smtp_port: 465, + transport: Some(TransportProto::ImapSmtp(ImapSmtpTransportProto { + imap_host: "imap.nuncio.mx".to_string(), + imap_port: 993, + imap_tls_mode: TlsModeProto::ImplicitTls.into(), + smtp_host: "smtp.nuncio.mx".to_string(), + smtp_port: 465, + smtp_tls_mode: TlsModeProto::ImplicitTls.into(), + })), } } + #[test] + fn account_config_from_proto_rejects_unset_transport_as_validation_failed() { + let config = AccountConfigProto { + id: "acct-no-transport".to_string(), + name: "No Transport".to_string(), + email_address: "no-transport@nuncio.mx".to_string(), + keyring_secret_key: "nuncio/acct-no-transport".to_string(), + sync_interval: Some(nuncio_proto::time::duration_from_secs(60)), + transport: None, + }; + let err = map_account_config_from_proto(config) + .expect_err("an account with no transport must be rejected"); + assert_eq!(err.code(), tonic::Code::InvalidArgument); + assert_eq!( + nuncio_proto::errors::error_reason(&err), + Some(ErrorReason::ValidationFailed), + ); + } + #[tokio::test] async fn add_account_rejects_missing_bearer_token() { // Confirms `Accounts` is mounted behind its own `BearerAuthInterceptor` @@ -3885,16 +3921,16 @@ mod tests { id: "acct-rotate-1".to_string(), name: "Rotate Account".to_string(), email_address: "rotate@nuncio.mx".to_string(), - protocol: nuncio_core::AccountProtocol::ImapSmtp, - server_host: "imap.nuncio.mx".to_string(), - server_port: 993, - smtp_host: "smtp.nuncio.mx".to_string(), - smtp_port: 465, - imap_tls_mode: nuncio_core::TlsMode::ImplicitTls, - smtp_tls_mode: nuncio_core::TlsMode::ImplicitTls, keyring_secret_key: key.to_string(), sync_interval_secs: 60, - collection_url: String::new(), + transport: nuncio_core::Transport::ImapSmtp(nuncio_core::ImapSmtpTransport { + imap_host: "imap.nuncio.mx".to_string(), + imap_port: 993, + imap_tls_mode: nuncio_core::TlsMode::ImplicitTls, + smtp_host: "smtp.nuncio.mx".to_string(), + smtp_port: 465, + smtp_tls_mode: nuncio_core::TlsMode::ImplicitTls, + }), }; // Force `save_account` to fail for real by closing the pool, exactly @@ -3928,16 +3964,16 @@ mod tests { id: "acct-rotate-2".to_string(), name: "Rotate Account 2".to_string(), email_address: "rotate2@nuncio.mx".to_string(), - protocol: nuncio_core::AccountProtocol::ImapSmtp, - server_host: "imap.nuncio.mx".to_string(), - server_port: 993, - smtp_host: "smtp.nuncio.mx".to_string(), - smtp_port: 465, - imap_tls_mode: nuncio_core::TlsMode::ImplicitTls, - smtp_tls_mode: nuncio_core::TlsMode::ImplicitTls, keyring_secret_key: key.to_string(), sync_interval_secs: 60, - collection_url: String::new(), + transport: nuncio_core::Transport::ImapSmtp(nuncio_core::ImapSmtpTransport { + imap_host: "imap.nuncio.mx".to_string(), + imap_port: 993, + imap_tls_mode: nuncio_core::TlsMode::ImplicitTls, + smtp_host: "smtp.nuncio.mx".to_string(), + smtp_port: 465, + smtp_tls_mode: nuncio_core::TlsMode::ImplicitTls, + }), }; db.close().await; @@ -4542,16 +4578,16 @@ mod tests { id: "acct-send-1".to_string(), name: "Send Test Account".to_string(), email_address: "sender@nuncio.mx".to_string(), - protocol: nuncio_core::AccountProtocol::ImapSmtp, - server_host: "imap.nuncio.mx".to_string(), - server_port: 993, - smtp_host: "127.0.0.1".to_string(), - smtp_port: 1, - imap_tls_mode: nuncio_core::TlsMode::ImplicitTls, - smtp_tls_mode: nuncio_core::TlsMode::ImplicitTls, keyring_secret_key: "nuncio/acct-send-1".to_string(), sync_interval_secs: 60, - collection_url: String::new(), + transport: nuncio_core::Transport::ImapSmtp(nuncio_core::ImapSmtpTransport { + imap_host: "imap.nuncio.mx".to_string(), + imap_port: 993, + imap_tls_mode: nuncio_core::TlsMode::ImplicitTls, + smtp_host: "127.0.0.1".to_string(), + smtp_port: 1, + smtp_tls_mode: nuncio_core::TlsMode::ImplicitTls, + }), }; db.save_account(&config).await.expect("save account"); secrets @@ -4710,16 +4746,16 @@ mod tests { id: "acct-injected-send-1".to_string(), name: "Injected Send Test Account".to_string(), email_address: "sender@nuncio.mx".to_string(), - protocol: nuncio_core::AccountProtocol::ImapSmtp, - server_host: "imap.nuncio.mx".to_string(), - server_port: 993, - smtp_host: "smtp.nuncio.mx".to_string(), - smtp_port: 465, - imap_tls_mode: nuncio_core::TlsMode::ImplicitTls, - smtp_tls_mode: nuncio_core::TlsMode::ImplicitTls, keyring_secret_key: "nuncio/acct-injected-send-1".to_string(), sync_interval_secs: 60, - collection_url: String::new(), + transport: nuncio_core::Transport::ImapSmtp(nuncio_core::ImapSmtpTransport { + imap_host: "imap.nuncio.mx".to_string(), + imap_port: 993, + imap_tls_mode: nuncio_core::TlsMode::ImplicitTls, + smtp_host: "smtp.nuncio.mx".to_string(), + smtp_port: 465, + smtp_tls_mode: nuncio_core::TlsMode::ImplicitTls, + }), }; // Deliberately never stores a keyring credential -- this path must // never need one. @@ -4826,16 +4862,16 @@ mod tests { id: "acct-injected-send-fail-1".to_string(), name: "Injected Send Failure Test Account".to_string(), email_address: "sender@nuncio.mx".to_string(), - protocol: nuncio_core::AccountProtocol::ImapSmtp, - server_host: "imap.nuncio.mx".to_string(), - server_port: 993, - smtp_host: "smtp.nuncio.mx".to_string(), - smtp_port: 465, - imap_tls_mode: nuncio_core::TlsMode::ImplicitTls, - smtp_tls_mode: nuncio_core::TlsMode::ImplicitTls, keyring_secret_key: "nuncio/acct-injected-send-fail-1".to_string(), sync_interval_secs: 60, - collection_url: String::new(), + transport: nuncio_core::Transport::ImapSmtp(nuncio_core::ImapSmtpTransport { + imap_host: "imap.nuncio.mx".to_string(), + imap_port: 993, + imap_tls_mode: nuncio_core::TlsMode::ImplicitTls, + smtp_host: "smtp.nuncio.mx".to_string(), + smtp_port: 465, + smtp_tls_mode: nuncio_core::TlsMode::ImplicitTls, + }), }; db.save_account(&config).await.expect("save account"); diff --git a/crates/nunciod/src/orchestrator.rs b/crates/nunciod/src/orchestrator.rs index 5147754..24af9aa 100644 --- a/crates/nunciod/src/orchestrator.rs +++ b/crates/nunciod/src/orchestrator.rs @@ -152,16 +152,16 @@ mod tests { id: "acct-orch-1".to_string(), name: "Orch Account".to_string(), email_address: "orch@nuncio.mx".to_string(), - protocol: nuncio_core::AccountProtocol::ImapSmtp, - server_host: "imap.nuncio.mx".to_string(), - server_port: 993, - smtp_host: "smtp.nuncio.mx".to_string(), - smtp_port: 465, - imap_tls_mode: nuncio_core::TlsMode::ImplicitTls, - smtp_tls_mode: nuncio_core::TlsMode::ImplicitTls, keyring_secret_key: "nuncio/acct-orch-1".to_string(), sync_interval_secs: 60, - collection_url: String::new(), + transport: nuncio_core::Transport::ImapSmtp(nuncio_core::ImapSmtpTransport { + imap_host: "imap.nuncio.mx".to_string(), + imap_port: 993, + imap_tls_mode: nuncio_core::TlsMode::ImplicitTls, + smtp_host: "smtp.nuncio.mx".to_string(), + smtp_port: 465, + smtp_tls_mode: nuncio_core::TlsMode::ImplicitTls, + }), }; engine.save_account(&acct).await.unwrap(); engine.close().await; diff --git a/crates/nunciod/src/outbox.rs b/crates/nunciod/src/outbox.rs index 0d5bb8b..813a659 100644 --- a/crates/nunciod/src/outbox.rs +++ b/crates/nunciod/src/outbox.rs @@ -470,10 +470,13 @@ impl RemoteExecutionEnv for ProductionExecutionEnv { ) -> Result, OutboxExecuteError> { let config = self.account(account_id).await?; let password = self.secrets.get_secret(&config.keyring_secret_key)?; + let transport = config + .imap_smtp() + .ok_or_else(|| OutboxExecuteError::NotAMailAccount(account_id.to_string()))?; let engine = SmtpTransportEngine::new( - &config.smtp_host, - config.smtp_port, - config.smtp_tls_mode, + &transport.smtp_host, + transport.smtp_port, + transport.smtp_tls_mode, &config.email_address, &password, )?; diff --git a/crates/nunciod/src/send.rs b/crates/nunciod/src/send.rs index ee800db..eebcba4 100644 --- a/crates/nunciod/src/send.rs +++ b/crates/nunciod/src/send.rs @@ -68,6 +68,11 @@ pub enum SendError { /// rather than silently sending from a different account. #[error("no account configured with id {0:?}")] AccountNotFound(String), + /// The resolved account does not have an SMTP outbound endpoint (only an + /// IMAP/SMTP account can send outbound mail over SMTP; a JMAP or DAV + /// account cannot). + #[error("account {0:?} has no SMTP transport and cannot send outbound mail")] + NotAnSmtpAccount(String), /// Failed to read the account's credential from the secret vault. #[error("failed to read account credential from the secret vault: {0}")] Vault(#[from] VaultError), @@ -120,10 +125,13 @@ fn build_smtp_sender( config: &AccountConfig, password: &str, ) -> Result { + let transport = config + .imap_smtp() + .ok_or_else(|| SendError::NotAnSmtpAccount(config.id.clone()))?; let engine = SmtpTransportEngine::new( - &config.smtp_host, - config.smtp_port, - config.smtp_tls_mode, + &transport.smtp_host, + transport.smtp_port, + transport.smtp_tls_mode, &config.email_address, password, )?; @@ -233,16 +241,16 @@ mod tests { id: id.to_string(), name: "Send Test Account".to_string(), email_address: format!("{id}@nuncio.mx"), - protocol: nuncio_core::AccountProtocol::ImapSmtp, - server_host: "imap.nuncio.mx".to_string(), - server_port: 993, - smtp_host: "127.0.0.1".to_string(), - smtp_port: 1, - imap_tls_mode: TlsMode::ImplicitTls, - smtp_tls_mode: TlsMode::ImplicitTls, keyring_secret_key: format!("nuncio/{id}"), sync_interval_secs: 60, - collection_url: String::new(), + transport: nuncio_core::Transport::ImapSmtp(nuncio_core::ImapSmtpTransport { + imap_host: "imap.nuncio.mx".to_string(), + imap_port: 993, + imap_tls_mode: TlsMode::ImplicitTls, + smtp_host: "127.0.0.1".to_string(), + smtp_port: 1, + smtp_tls_mode: TlsMode::ImplicitTls, + }), } } diff --git a/crates/nunciod/src/sync.rs b/crates/nunciod/src/sync.rs index df2cf81..0c5036a 100644 --- a/crates/nunciod/src/sync.rs +++ b/crates/nunciod/src/sync.rs @@ -4,8 +4,8 @@ //! / `CoreCommand::SyncAccount` only moved `EngineStatus` to `Syncing` and //! published `SyncStarted`, and nothing ever fetched a real message from a //! real server into the store. This module closes that gap: for a configured -//! account it connects a real [`MailBackend`] (a real [`ImapEngine`] for -//! [`AccountProtocol::ImapSmtp`], built from [`AccountConfig`] plus the +//! account it connects a real [`MailBackend`] (a real [`ImapEngine`] for an +//! IMAP/SMTP transport, built from [`AccountConfig`] plus the //! account's keyring-stored password), fetches folders and messages, and //! persists every message through [`DatabaseEngine::save_email`]. //! @@ -20,7 +20,7 @@ //! all-accounts sync fans these per-account runs out through //! [`crate::sync_dispatcher::sync_all_configured`], so concurrency and the //! per-account timeout are bounded in one place rather than serialized here. -use nuncio_core::{AccountConfig, AccountProtocol, CoreCommand, CoreEvent, EventBus}; +use nuncio_core::{AccountConfig, CoreCommand, CoreEvent, EventBus, Transport}; use nuncio_filter::{FilterEngine, OutboxManager, RuleAction}; use nuncio_mail::{ImapEngine, JmapEngine, MailBackend, MailError}; use nuncio_store::db::DatabaseError; @@ -347,24 +347,24 @@ pub(crate) fn build_mail_backend( config: &AccountConfig, password: &str, ) -> Result, SyncError> { - match config.protocol { - AccountProtocol::ImapSmtp => Ok(Box::new(ImapEngine::with_credentials( + match &config.transport { + Transport::ImapSmtp(t) => Ok(Box::new(ImapEngine::with_credentials( &config.id, - &config.server_host, - config.server_port, - config.imap_tls_mode, + &t.imap_host, + t.imap_port, + t.imap_tls_mode, &config.email_address, password, ))), - AccountProtocol::Jmap => Ok(Box::new(JmapEngine::with_credentials( + Transport::Jmap(t) => Ok(Box::new(JmapEngine::with_credentials( &config.id, - &config.server_host, + &t.endpoint_host, &config.email_address, password, ))), // A DAV-protocol account (e.g. CalDAV) has no inbound mail backend; // it is synced through its own domain service, not the mail path. - AccountProtocol::CalDav => Err(SyncError::NotAMailAccount(config.id.clone())), + Transport::Dav(_) => Err(SyncError::NotAMailAccount(config.id.clone())), } } @@ -436,16 +436,11 @@ mod tests { id: id.to_string(), name: "JMAP Test Account".to_string(), email_address: format!("{id}@nuncio.mx"), - protocol: AccountProtocol::Jmap, - server_host: host.to_string(), - server_port: 443, - smtp_host: "smtp.nuncio.mx".to_string(), - smtp_port: 465, - imap_tls_mode: TlsMode::ImplicitTls, - smtp_tls_mode: TlsMode::ImplicitTls, keyring_secret_key: format!("nuncio/{id}"), sync_interval_secs: 60, - collection_url: String::new(), + transport: Transport::Jmap(nuncio_core::JmapTransport { + endpoint_host: host.to_string(), + }), } } @@ -527,16 +522,16 @@ mod tests { id: id.to_string(), name: "IMAP Test Account".to_string(), email_address: format!("{id}@nuncio.mx"), - protocol: AccountProtocol::ImapSmtp, - server_host: "imap.nuncio.mx".to_string(), - server_port: 993, - smtp_host: "smtp.nuncio.mx".to_string(), - smtp_port: 465, - imap_tls_mode: TlsMode::ImplicitTls, - smtp_tls_mode: TlsMode::ImplicitTls, keyring_secret_key: format!("nuncio/{id}"), sync_interval_secs: 60, - collection_url: String::new(), + transport: nuncio_core::Transport::ImapSmtp(nuncio_core::ImapSmtpTransport { + imap_host: "imap.nuncio.mx".to_string(), + imap_port: 993, + imap_tls_mode: TlsMode::ImplicitTls, + smtp_host: "smtp.nuncio.mx".to_string(), + smtp_port: 465, + smtp_tls_mode: TlsMode::ImplicitTls, + }), } } diff --git a/crates/nunciod/src/sync_dispatcher.rs b/crates/nunciod/src/sync_dispatcher.rs index 29718f5..2278c64 100644 --- a/crates/nunciod/src/sync_dispatcher.rs +++ b/crates/nunciod/src/sync_dispatcher.rs @@ -362,7 +362,7 @@ pub async fn sync_all_configured( let account_ids = match db.list_accounts().await { Ok(accounts) => accounts .into_iter() - .filter(|a| !a.protocol.is_dav()) + .filter(|a| !a.is_dav()) .map(|a| a.id) .collect::>(), Err(e) => { @@ -381,7 +381,7 @@ pub async fn sync_all_configured( #[cfg(test)] mod tests { use super::*; - use nuncio_core::{AccountConfig, AccountProtocol, CoreEvent, TlsMode}; + use nuncio_core::{AccountConfig, CoreEvent, DavTransport, JmapTransport, Transport}; use std::collections::HashSet; use std::sync::atomic::{AtomicUsize, Ordering}; use tokio::time::{sleep, Duration}; @@ -628,22 +628,19 @@ mod tests { id: id.to_string(), name: "Test".to_string(), email_address: format!("{id}@nuncio.mx"), - protocol: AccountProtocol::Jmap, - server_host: "jmap.nuncio.mx".to_string(), - server_port: 443, - smtp_host: "smtp.nuncio.mx".to_string(), - smtp_port: 465, - imap_tls_mode: TlsMode::ImplicitTls, - smtp_tls_mode: TlsMode::ImplicitTls, keyring_secret_key: format!("nuncio/{id}"), sync_interval_secs: 60, - collection_url: String::new(), + transport: Transport::Jmap(JmapTransport { + endpoint_host: "jmap.nuncio.mx".to_string(), + }), } } fn caldav_account(id: &str) -> AccountConfig { AccountConfig { - protocol: AccountProtocol::CalDav, + transport: Transport::Dav(DavTransport { + collection_url: "https://dav.nuncio.mx/cal/".to_string(), + }), ..jmap_account(id) } } diff --git a/crates/nunciod/tests/accounts_lifecycle_e2e_test.rs b/crates/nunciod/tests/accounts_lifecycle_e2e_test.rs index 68c53d8..020bae1 100644 --- a/crates/nunciod/tests/accounts_lifecycle_e2e_test.rs +++ b/crates/nunciod/tests/accounts_lifecycle_e2e_test.rs @@ -29,7 +29,7 @@ use nuncio_core::EventBus; use nuncio_filter::FilterEngine; use nuncio_proto::v1::{ - AccountConfig, AccountProtocol, AddAccountRequest, ListAccountsRequest, RemoveAccountRequest, + AccountConfig, AddAccountRequest, ListAccountsRequest, RemoveAccountRequest, TestAccountConnectionRequest, TlsMode, UpdateAccountRequest, }; use nuncio_store::db::DatabaseEngine; @@ -42,6 +42,15 @@ use nunciod::grpc::{ use std::sync::Arc; use tokio::net::TcpListener; +/// Extract the IMAP/SMTP transport sub-message from a wire `AccountConfig`, +/// panicking if the account is not an IMAP/SMTP account (test-only helper). +fn imap_smtp_transport(config: &AccountConfig) -> &nuncio_proto::v1::ImapSmtpTransport { + match &config.transport { + Some(nuncio_proto::v1::account_config::Transport::ImapSmtp(t)) => t, + _ => panic!("expected an IMAP/SMTP account"), + } +} + const ACCOUNT_ID: &str = "acct-lifecycle-e2e-1"; const ACCOUNT_EMAIL: &str = "lifecycle-e2e@nuncio.mx"; const KEYRING_KEY: &str = "nuncio/acct-lifecycle-e2e-1"; @@ -79,16 +88,18 @@ fn sample_account_config() -> AccountConfig { id: ACCOUNT_ID.to_string(), name: "Lifecycle E2E Account".to_string(), email_address: ACCOUNT_EMAIL.to_string(), - protocol: AccountProtocol::ImapSmtp.into(), - server_host: "imap.nuncio.mx".to_string(), - server_port: 143, - imap_tls_mode: TlsMode::StartTls.into(), - smtp_tls_mode: TlsMode::Plain.into(), keyring_secret_key: KEYRING_KEY.to_string(), sync_interval: Some(nuncio_proto::time::duration_from_secs(60)), - smtp_host: "127.0.0.1".to_string(), - smtp_port: 25, - collection_url: String::new(), + transport: Some(nuncio_proto::v1::account_config::Transport::ImapSmtp( + nuncio_proto::v1::ImapSmtpTransport { + imap_host: "imap.nuncio.mx".to_string(), + imap_port: 143, + imap_tls_mode: TlsMode::StartTls.into(), + smtp_host: "127.0.0.1".to_string(), + smtp_port: 25, + smtp_tls_mode: TlsMode::Plain.into(), + }, + )), } } @@ -179,13 +190,18 @@ async fn account_lifecycle_round_trips_over_authenticated_grpc_offline() { .into_inner() .accounts; assert_eq!(listed.len(), 1); - assert_eq!(listed[0].imap_tls_mode(), TlsMode::StartTls); - assert_eq!(listed[0].smtp_tls_mode(), TlsMode::Plain); + let listed_t = imap_smtp_transport(&listed[0]); + assert_eq!(listed_t.imap_tls_mode(), TlsMode::StartTls); + assert_eq!(listed_t.smtp_tls_mode(), TlsMode::Plain); // ---- (c) UpdateAccount genuinely mutates the persisted config ---- let mut updated = sample_account_config(); updated.name = "Renamed Lifecycle Account".to_string(); - updated.imap_tls_mode = TlsMode::ImplicitTls.into(); + if let Some(nuncio_proto::v1::account_config::Transport::ImapSmtp(t)) = + updated.transport.as_mut() + { + t.imap_tls_mode = TlsMode::ImplicitTls.into(); + } accounts_client .update_account(UpdateAccountRequest { config: Some(updated), @@ -202,9 +218,10 @@ async fn account_lifecycle_round_trips_over_authenticated_grpc_offline() { .accounts; assert_eq!(relisted.len(), 1); assert_eq!(relisted[0].name, "Renamed Lifecycle Account"); - assert_eq!(relisted[0].imap_tls_mode(), TlsMode::ImplicitTls); + let relisted_t = imap_smtp_transport(&relisted[0]); + assert_eq!(relisted_t.imap_tls_mode(), TlsMode::ImplicitTls); // Untouched fields survive the update. - assert_eq!(relisted[0].smtp_tls_mode(), TlsMode::Plain); + assert_eq!(relisted_t.smtp_tls_mode(), TlsMode::Plain); // ---- (d) TestAccountConnection reports the GENUINE per-protocol result ---- let report = accounts_client diff --git a/crates/nunciod/tests/caldav_production_e2e_test.rs b/crates/nunciod/tests/caldav_production_e2e_test.rs index 566c911..8576be5 100644 --- a/crates/nunciod/tests/caldav_production_e2e_test.rs +++ b/crates/nunciod/tests/caldav_production_e2e_test.rs @@ -20,8 +20,7 @@ use nuncio_core::EventBus; use nuncio_filter::FilterEngine; use nuncio_proto::v1::{ - AccountConfig as AccountConfigProto, AccountProtocol as AccountProtocolProto, - AddAccountRequest, CalendarSyncRequest, ListEventsRequest, TlsMode as TlsModeProto, + AccountConfig as AccountConfigProto, AddAccountRequest, CalendarSyncRequest, ListEventsRequest, }; use nuncio_store::db::DatabaseEngine; use nuncio_store::vault::{SecretManager, GRPC_TOKEN_ACCOUNT}; @@ -158,16 +157,13 @@ async fn caldav_production_sync_builds_real_client_and_persists_events() { id: ACCOUNT_ID.to_string(), name: "Work Calendar".to_string(), email_address: ACCOUNT_EMAIL.to_string(), - protocol: AccountProtocolProto::Caldav.into(), - server_host: String::new(), - server_port: 0, - imap_tls_mode: TlsModeProto::ImplicitTls.into(), - smtp_tls_mode: TlsModeProto::ImplicitTls.into(), keyring_secret_key: KEYRING_KEY.to_string(), sync_interval: Some(nuncio_proto::time::duration_from_secs(300)), - smtp_host: String::new(), - smtp_port: 0, - collection_url: collection_url.clone(), + transport: Some(nuncio_proto::v1::account_config::Transport::Dav( + nuncio_proto::v1::DavTransport { + collection_url: collection_url.clone(), + }, + )), }), password: CALDAV_PASSWORD.to_string(), }) diff --git a/crates/nunciod/tests/outbox_executor_e2e_test.rs b/crates/nunciod/tests/outbox_executor_e2e_test.rs index d4a4e4d..5fccdcd 100644 --- a/crates/nunciod/tests/outbox_executor_e2e_test.rs +++ b/crates/nunciod/tests/outbox_executor_e2e_test.rs @@ -99,16 +99,16 @@ fn sample_account() -> nuncio_core::AccountConfig { id: ACCOUNT_ID.to_string(), name: "Outbox E2E Account".to_string(), email_address: "owner@nuncio.mx".to_string(), - protocol: nuncio_core::AccountProtocol::ImapSmtp, - server_host: "imap.nuncio.mx".to_string(), - server_port: 993, - smtp_host: "smtp.nuncio.mx".to_string(), - smtp_port: 465, - imap_tls_mode: nuncio_core::TlsMode::ImplicitTls, - smtp_tls_mode: nuncio_core::TlsMode::ImplicitTls, keyring_secret_key: format!("nuncio/{ACCOUNT_ID}"), sync_interval_secs: 60, - collection_url: String::new(), + transport: nuncio_core::Transport::ImapSmtp(nuncio_core::ImapSmtpTransport { + imap_host: "imap.nuncio.mx".to_string(), + imap_port: 993, + imap_tls_mode: nuncio_core::TlsMode::ImplicitTls, + smtp_host: "smtp.nuncio.mx".to_string(), + smtp_port: 465, + smtp_tls_mode: nuncio_core::TlsMode::ImplicitTls, + }), } } diff --git a/crates/nunciod/tests/spine_e2e_test.rs b/crates/nunciod/tests/spine_e2e_test.rs index be1b8d2..b15c2ae 100644 --- a/crates/nunciod/tests/spine_e2e_test.rs +++ b/crates/nunciod/tests/spine_e2e_test.rs @@ -37,8 +37,8 @@ use nuncio_filter::FilterEngine; use nuncio_mail::{MockMailBackend, MockMessageSender}; use nuncio_proto::v1::event::Kind; use nuncio_proto::v1::{ - AccountConfig, AccountProtocol, AddAccountRequest, GetMessageRequest, ListAccountsRequest, - ListMessagesRequest, MarkReadRequest, SendMessageRequest, SyncRequest, TlsMode, + AccountConfig, AddAccountRequest, GetMessageRequest, ListAccountsRequest, ListMessagesRequest, + MarkReadRequest, SendMessageRequest, SyncRequest, TlsMode, }; use nuncio_store::db::DatabaseEngine; use nuncio_store::vault::{SecretManager, GRPC_TOKEN_ACCOUNT}; @@ -58,16 +58,18 @@ fn sample_account_config() -> AccountConfig { id: ACCOUNT_ID.to_string(), name: "Spine E2E Test Account".to_string(), email_address: ACCOUNT_EMAIL.to_string(), - protocol: AccountProtocol::ImapSmtp.into(), - server_host: "imap.nuncio.mx".to_string(), - server_port: 993, - imap_tls_mode: TlsMode::ImplicitTls.into(), - smtp_tls_mode: TlsMode::ImplicitTls.into(), keyring_secret_key: KEYRING_KEY.to_string(), sync_interval: Some(nuncio_proto::time::duration_from_secs(60)), - smtp_host: "smtp.nuncio.mx".to_string(), - smtp_port: 465, - collection_url: String::new(), + transport: Some(nuncio_proto::v1::account_config::Transport::ImapSmtp( + nuncio_proto::v1::ImapSmtpTransport { + imap_host: "imap.nuncio.mx".to_string(), + imap_port: 993, + imap_tls_mode: TlsMode::ImplicitTls.into(), + smtp_host: "smtp.nuncio.mx".to_string(), + smtp_port: 465, + smtp_tls_mode: TlsMode::ImplicitTls.into(), + }, + )), } } -- 2.54.0