feat(mail): select an explicit account for SendMessage #232
No files matched your search
@@ -308,6 +308,10 @@ pub enum MailSubcommand {
|
||||
/// Message body text content.
|
||||
#[arg(short, long, help = "Message body text content")]
|
||||
body: String,
|
||||
/// Identifier of the account to send from (defaults to the first
|
||||
/// configured account when omitted).
|
||||
#[arg(short, long, help = "Identifier of the account to send from")]
|
||||
account: Option<String>,
|
||||
},
|
||||
/// Full-text search across all cached email messages.
|
||||
Search {
|
||||
|
||||
@@ -322,8 +322,14 @@ impl HeadlessRunner {
|
||||
MailSubcommand::Sync => self.handle_sync(json_mode).await,
|
||||
MailSubcommand::List { folder } => self.handle_list_folder(folder, json_mode).await,
|
||||
MailSubcommand::Read { id } => self.handle_read_message(id, json_mode).await,
|
||||
MailSubcommand::Send { to, subject, body } => {
|
||||
self.handle_send_email(to, subject, body, json_mode).await
|
||||
MailSubcommand::Send {
|
||||
to,
|
||||
subject,
|
||||
body,
|
||||
account,
|
||||
} => {
|
||||
self.handle_send_email(to, subject, body, account.as_deref(), json_mode)
|
||||
.await
|
||||
}
|
||||
MailSubcommand::Search { query } => self.handle_search(query, json_mode).await,
|
||||
MailSubcommand::Mark { id, read, unread } => {
|
||||
@@ -695,6 +701,7 @@ impl HeadlessRunner {
|
||||
to: &str,
|
||||
subject: &str,
|
||||
body: &str,
|
||||
account: Option<&str>,
|
||||
json_mode: bool,
|
||||
) -> String {
|
||||
let mut client = match self.connect_mail_client().await {
|
||||
@@ -710,6 +717,7 @@ impl HeadlessRunner {
|
||||
body_text: body.to_string(),
|
||||
body_html: None,
|
||||
attachments: Vec::new(),
|
||||
account_id: account.map(str::to_string),
|
||||
})
|
||||
.await
|
||||
{
|
||||
@@ -3165,6 +3173,7 @@ mod tests {
|
||||
to: "alice@nuncio.mx".to_string(),
|
||||
subject: "Quarterly Roadmap".to_string(),
|
||||
body: "Let's discuss the roadmap.".to_string(),
|
||||
account: None,
|
||||
},
|
||||
},
|
||||
true,
|
||||
@@ -3182,6 +3191,35 @@ mod tests {
|
||||
assert_eq!(recorded_send.to, "alice@nuncio.mx");
|
||||
assert_eq!(recorded_send.subject, "Quarterly Roadmap");
|
||||
assert_eq!(recorded_send.body_text, "Let's discuss the roadmap.");
|
||||
assert_eq!(recorded_send.account_id, None);
|
||||
|
||||
// `mail send --account <id>` threads the explicit selector through
|
||||
// to the daemon over the wire, proving the CLI flag is not merely
|
||||
// parsed but actually reaches `SendMessageRequest.account_id`.
|
||||
let mail_send_with_account = runner
|
||||
.execute_command(
|
||||
&Commands::Mail {
|
||||
action: MailSubcommand::Send {
|
||||
to: "alice@nuncio.mx".to_string(),
|
||||
subject: "Quarterly Roadmap".to_string(),
|
||||
body: "Let's discuss the roadmap.".to_string(),
|
||||
account: Some("acct-work".to_string()),
|
||||
},
|
||||
},
|
||||
true,
|
||||
)
|
||||
.await;
|
||||
assert!(mail_send_with_account.contains(r#""sent":true"#));
|
||||
|
||||
let recorded_send_with_account = send_probe
|
||||
.lock()
|
||||
.unwrap_or_else(|e| e.into_inner())
|
||||
.clone()
|
||||
.expect("stub daemon received a second send_message request");
|
||||
assert_eq!(
|
||||
recorded_send_with_account.account_id,
|
||||
Some("acct-work".to_string())
|
||||
);
|
||||
}
|
||||
|
||||
/// Reference-client proof: boots a stub `nuncio.v1.Calendar` gRPC server
|
||||
|
||||
Binary file not shown.
@@ -321,7 +321,11 @@ service Mail {
|
||||
// SendMessage composes and sends a real outbound email over SMTP, built
|
||||
// from the configured account's SMTP endpoint
|
||||
// (`AccountConfig.smtp_host`/`smtp_port`) and its keyring-stored
|
||||
// password. Returns successfully ONLY when
|
||||
// password. When `account_id` is set, that specific account is used as
|
||||
// the sender, and a request naming an account that does not exist fails
|
||||
// rather than silently falling back to a different account. When
|
||||
// `account_id` is omitted, the first configured account is used. Returns
|
||||
// successfully ONLY when
|
||||
// the SMTP transport genuinely accepted the message for delivery --
|
||||
// never a fabricated success. A transport/authentication failure surfaces
|
||||
// as a `Status::internal` (or `Status::invalid_argument` for a malformed
|
||||
@@ -469,6 +473,10 @@ message SendMessageRequest {
|
||||
optional string body_html = 5;
|
||||
// Optional file attachments.
|
||||
repeated Attachment attachments = 6;
|
||||
// Optional identifier of the account to send from. When omitted, the
|
||||
// first configured account is used (preserving prior behavior); when
|
||||
// set, the named account must exist or the request fails.
|
||||
optional string account_id = 7;
|
||||
}
|
||||
|
||||
// SendMessageResponse. A successful response means the SMTP transport
|
||||
|
||||
@@ -1355,6 +1355,7 @@ impl Mail for MailGrpcService {
|
||||
.into_iter()
|
||||
.map(map_attachment_from_proto)
|
||||
.collect(),
|
||||
account_id: req.account_id,
|
||||
};
|
||||
|
||||
let message_id = if let Some(sender) = &self.overrides.message_sender {
|
||||
@@ -3295,6 +3296,7 @@ mod tests {
|
||||
body_text: "Body".to_string(),
|
||||
body_html: None,
|
||||
attachments: Vec::new(),
|
||||
account_id: None,
|
||||
})
|
||||
.await
|
||||
.expect_err("missing bearer token must be rejected");
|
||||
@@ -3552,6 +3554,7 @@ mod tests {
|
||||
body_text: "Let's discuss the roadmap.".to_string(),
|
||||
body_html: None,
|
||||
attachments: Vec::new(),
|
||||
account_id: None,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -3839,6 +3842,7 @@ mod tests {
|
||||
body_text: "Injected send body".to_string(),
|
||||
body_html: None,
|
||||
attachments: Vec::new(),
|
||||
account_id: None,
|
||||
}))
|
||||
.await
|
||||
.expect("send_message succeeds")
|
||||
|
||||
+112
-16
@@ -12,13 +12,13 @@
|
||||
//!
|
||||
//! # Account selection
|
||||
//!
|
||||
//! The `nuncio.v1.Mail` service (like every other RPC on it --
|
||||
//! `ListMessages`, `GetMessage`, `SearchMessages`, etc.) does not yet
|
||||
//! disambiguate between multiple configured accounts; it operates over a
|
||||
//! single, global store. Consistent with that existing scope, `SendMessage`
|
||||
//! sends from the first account returned by [`DatabaseEngine::list_accounts`].
|
||||
//! Supporting an explicit multi-account choice is future work, alongside
|
||||
//! the rest of the `Mail` service's single-account scope.
|
||||
//! `SendMessage` accepts an optional `account_id`. When the caller supplies
|
||||
//! one, that specific account is resolved and used as the sender -- a
|
||||
//! request naming an account that does not exist fails honestly rather than
|
||||
//! silently falling back to a different identity. When `account_id` is
|
||||
//! omitted, the account resolves to the first one returned by
|
||||
//! [`DatabaseEngine::list_accounts`], preserving behavior for callers that
|
||||
//! predate explicit selection.
|
||||
//!
|
||||
//! # Testability
|
||||
//!
|
||||
@@ -61,6 +61,13 @@ pub enum SendError {
|
||||
/// No account is configured to send mail from.
|
||||
#[error("no account is configured to send mail from")]
|
||||
NoAccountConfigured,
|
||||
/// The caller specified an `account_id` that does not match any
|
||||
/// configured account. Distinct from [`SendError::NoAccountConfigured`]
|
||||
/// (which means zero accounts exist at all): this means at least one
|
||||
/// account exists, just not the one requested, so the request must fail
|
||||
/// rather than silently sending from a different account.
|
||||
#[error("no account configured with id {0:?}")]
|
||||
AccountNotFound(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),
|
||||
@@ -90,6 +97,9 @@ pub struct ComposeRequest {
|
||||
pub body_html: Option<String>,
|
||||
/// File attachments.
|
||||
pub attachments: Vec<Attachment>,
|
||||
/// Identifier of the account to send from. `None` preserves the prior
|
||||
/// "first configured account" behavior.
|
||||
pub account_id: Option<String>,
|
||||
}
|
||||
|
||||
/// Send `message` through `sender`. Pure seam shared by production and
|
||||
@@ -121,14 +131,24 @@ fn build_smtp_sender(
|
||||
}
|
||||
|
||||
/// Resolves the account to send mail from. See the module-level "Account
|
||||
/// selection" doc above for why this is currently the first configured
|
||||
/// account rather than an explicit per-request choice.
|
||||
async fn resolve_sending_account(db: &DatabaseEngine) -> Result<AccountConfig, SendError> {
|
||||
/// selection" doc above: an explicit `account_id` must match a configured
|
||||
/// account or the request fails; `None` preserves the "first configured
|
||||
/// account" behavior.
|
||||
async fn resolve_sending_account(
|
||||
db: &DatabaseEngine,
|
||||
account_id: Option<&str>,
|
||||
) -> Result<AccountConfig, SendError> {
|
||||
let accounts = db.list_accounts().await?;
|
||||
accounts
|
||||
.into_iter()
|
||||
.next()
|
||||
.ok_or(SendError::NoAccountConfigured)
|
||||
match account_id {
|
||||
Some(id) => accounts
|
||||
.into_iter()
|
||||
.find(|a| a.id == id)
|
||||
.ok_or_else(|| SendError::AccountNotFound(id.to_string())),
|
||||
None => accounts
|
||||
.into_iter()
|
||||
.next()
|
||||
.ok_or(SendError::NoAccountConfigured),
|
||||
}
|
||||
}
|
||||
|
||||
/// Generates a daemon-local identifier for a sent message, for reference in
|
||||
@@ -154,7 +174,7 @@ pub async fn send_message_for_account(
|
||||
secrets: &SecretManager,
|
||||
request: ComposeRequest,
|
||||
) -> Result<String, SendError> {
|
||||
let config = resolve_sending_account(db).await?;
|
||||
let config = resolve_sending_account(db, request.account_id.as_deref()).await?;
|
||||
let password = secrets.get_secret(&config.keyring_secret_key)?;
|
||||
let sender = build_smtp_sender(&config, &password)?;
|
||||
|
||||
@@ -186,7 +206,7 @@ pub async fn send_message_with_injected_sender(
|
||||
sender: &dyn MessageSender,
|
||||
request: ComposeRequest,
|
||||
) -> Result<String, SendError> {
|
||||
let config = resolve_sending_account(db).await?;
|
||||
let config = resolve_sending_account(db, request.account_id.as_deref()).await?;
|
||||
|
||||
let outbound = OutboundMessage {
|
||||
from: config.email_address,
|
||||
@@ -286,6 +306,7 @@ mod tests {
|
||||
body_text: Some("Body".to_string()),
|
||||
body_html: None,
|
||||
attachments: Vec::new(),
|
||||
account_id: None,
|
||||
},
|
||||
)
|
||||
.await
|
||||
@@ -314,6 +335,7 @@ mod tests {
|
||||
body_text: Some("Body".to_string()),
|
||||
body_html: None,
|
||||
attachments: Vec::new(),
|
||||
account_id: None,
|
||||
},
|
||||
)
|
||||
.await
|
||||
@@ -352,6 +374,7 @@ mod tests {
|
||||
body_text: Some("Body".to_string()),
|
||||
body_html: None,
|
||||
attachments: Vec::new(),
|
||||
account_id: None,
|
||||
},
|
||||
)
|
||||
.await
|
||||
@@ -392,6 +415,7 @@ mod tests {
|
||||
body_text: Some("Body via injected sender.".to_string()),
|
||||
body_html: None,
|
||||
attachments: Vec::new(),
|
||||
account_id: None,
|
||||
},
|
||||
)
|
||||
.await
|
||||
@@ -423,6 +447,7 @@ mod tests {
|
||||
body_text: Some("Body".to_string()),
|
||||
body_html: None,
|
||||
attachments: Vec::new(),
|
||||
account_id: None,
|
||||
},
|
||||
)
|
||||
.await
|
||||
@@ -430,4 +455,75 @@ mod tests {
|
||||
assert!(matches!(err, SendError::NoAccountConfigured));
|
||||
assert!(mock.sent_messages().is_empty());
|
||||
}
|
||||
|
||||
/// Proves that with multiple configured accounts, an explicit
|
||||
/// `account_id` selects that SPECIFIC account as the sender -- not
|
||||
/// whichever account happens to be first -- by asserting the captured
|
||||
/// outbound message's `From:` matches the requested account.
|
||||
#[tokio::test]
|
||||
async fn send_message_with_injected_sender_uses_the_explicitly_selected_account() {
|
||||
let (db, _dir) = DatabaseEngine::connect_ephemeral()
|
||||
.await
|
||||
.expect("ephemeral db");
|
||||
|
||||
let first = sample_account("acct-first");
|
||||
let second = sample_account("acct-second");
|
||||
db.save_account(&first).await.expect("save first account");
|
||||
db.save_account(&second).await.expect("save second account");
|
||||
|
||||
let mock = MockMessageSender::new();
|
||||
send_message_with_injected_sender(
|
||||
&db,
|
||||
&mock,
|
||||
ComposeRequest {
|
||||
to: "bob@nuncio.mx".to_string(),
|
||||
cc: None,
|
||||
subject: "Hi".to_string(),
|
||||
body_text: Some("Body".to_string()),
|
||||
body_html: None,
|
||||
attachments: Vec::new(),
|
||||
account_id: Some(second.id.clone()),
|
||||
},
|
||||
)
|
||||
.await
|
||||
.expect("send succeeds via explicitly selected account");
|
||||
|
||||
let sent = mock.sent_messages();
|
||||
assert_eq!(sent.len(), 1);
|
||||
assert_eq!(sent[0].from, second.email_address);
|
||||
assert_ne!(sent[0].from, first.email_address);
|
||||
}
|
||||
|
||||
/// An `account_id` naming an account that does not exist must fail with
|
||||
/// the specific [`SendError::AccountNotFound`] variant -- never a silent
|
||||
/// fallback to the first configured account, and never confusable with
|
||||
/// [`SendError::NoAccountConfigured`] (which means zero accounts exist).
|
||||
#[tokio::test]
|
||||
async fn send_message_with_injected_sender_rejects_unknown_account_id() {
|
||||
let (db, _dir) = DatabaseEngine::connect_ephemeral()
|
||||
.await
|
||||
.expect("ephemeral db");
|
||||
|
||||
let config = sample_account("acct-only");
|
||||
db.save_account(&config).await.expect("save account");
|
||||
|
||||
let mock = MockMessageSender::new();
|
||||
let err = send_message_with_injected_sender(
|
||||
&db,
|
||||
&mock,
|
||||
ComposeRequest {
|
||||
to: "bob@nuncio.mx".to_string(),
|
||||
cc: None,
|
||||
subject: "Hi".to_string(),
|
||||
body_text: Some("Body".to_string()),
|
||||
body_html: None,
|
||||
attachments: Vec::new(),
|
||||
account_id: Some("acct-does-not-exist".to_string()),
|
||||
},
|
||||
)
|
||||
.await
|
||||
.expect_err("unknown account id must fail");
|
||||
assert!(matches!(err, SendError::AccountNotFound(id) if id == "acct-does-not-exist"));
|
||||
assert!(mock.sent_messages().is_empty());
|
||||
}
|
||||
}
|
||||
@@ -305,6 +305,7 @@ async fn full_mail_spine_round_trips_over_authenticated_grpc_with_mocked_protoco
|
||||
body_text: "Let's discuss the roadmap.".to_string(),
|
||||
body_html: None,
|
||||
attachments: Vec::new(),
|
||||
account_id: None,
|
||||
})
|
||||
.await
|
||||
.expect("send_message succeeds")
|
||||
|
||||
Reference in new issue
Block a user