From e2ce36dc6dd0f0e2ccc3f932fe66d97eda159b2b Mon Sep 17 00:00:00 2001 From: Lewis Date: Wed, 30 Sep 2026 10:56:56 +0300 Subject: [PATCH] comms: card-carrying token codes & verification Better discipline in constructing verificationtarget for verifying comms channels. It's still not perfect and there's still email-defaultism but I want this bandaid in before we do more work later on to make email not special. Better hygiene here also for secrecy vs not secret notifs. More type upgrades to come after this! Lewis: May this revision serve well! --- crates/tranquil-api/src/common.rs | 46 +- crates/tranquil-api/src/identity/provision.rs | 14 +- crates/tranquil-api/src/notification_prefs.rs | 9 +- crates/tranquil-api/src/server/email.rs | 4 +- crates/tranquil-api/src/server/session.rs | 15 +- .../tranquil-api/src/server/verify_email.rs | 6 +- .../tranquil-api/src/server/verify_token.rs | 6 +- .../src/sso_endpoints.rs | 2 +- crates/tranquil-pds/src/comms/mod.rs | 10 +- crates/tranquil-pds/src/comms/notice.rs | 507 ++++++++++++++++++ crates/tranquil-pds/src/comms/service.rs | 191 +------ .../tranquil-pds/tests/it/password_reset.rs | 7 +- 12 files changed, 602 insertions(+), 215 deletions(-) create mode 100644 crates/tranquil-pds/src/comms/notice.rs diff --git a/crates/tranquil-api/src/common.rs b/crates/tranquil-api/src/common.rs index 87cf66ba..fd4e3984 100644 --- a/crates/tranquil-api/src/common.rs +++ b/crates/tranquil-api/src/common.rs @@ -167,39 +167,43 @@ pub fn extract_verification_recipient( match channel { CommsChannel::Email => { let raw = trimmed(input.email).ok_or(ApiError::MissingEmail)?; - let address = tranquil_types::EmailAddress::new(raw)?; - Ok(tranquil_pds::comms::VerificationTarget::direct( - tranquil_db_traits::Recipient::Email(address), + Ok(tranquil_pds::comms::VerificationTarget::Email( + tranquil_types::EmailAddress::new(raw)?, )) } CommsChannel::Signal => { let raw = trimmed(input.signal_username).ok_or(ApiError::MissingSignalNumber)?; - let username = SignalUsername::new(raw)?; - Ok(tranquil_pds::comms::VerificationTarget::direct( - tranquil_db_traits::Recipient::Signal(username), + Ok(tranquil_pds::comms::VerificationTarget::Signal( + SignalUsername::new(raw)?, )) } CommsChannel::Telegram => { let raw = trimmed(input.telegram_username).ok_or(ApiError::MissingTelegramUsername)?; - let username = TelegramUsername::new(raw)?; - tranquil_pds::comms::VerificationTarget::resolve( - channel, - username.as_str(), - input.email, - ) + let identifier = + tranquil_types::ChannelIdentifier::Telegram(TelegramUsername::new(raw)?); + Ok(tranquil_pds::comms::VerificationTarget::ViaEmail { + identifier, + address: fallback_email(input.email)?, + }) } CommsChannel::Discord => { let raw = trimmed(input.discord_username).ok_or(ApiError::MissingDiscordId)?; - let username = DiscordUsername::new(raw)?; - tranquil_pds::comms::VerificationTarget::resolve( - channel, - username.as_str(), - input.email, - ) + let identifier = tranquil_types::ChannelIdentifier::Discord(DiscordUsername::new(raw)?); + Ok(tranquil_pds::comms::VerificationTarget::ViaEmail { + identifier, + address: fallback_email(input.email)?, + }) } } } +fn fallback_email(raw: Option<&str>) -> Result { + let trimmed = trimmed(raw).ok_or(ApiError::InvalidRequest( + "Verification over this channel couldn't happen for some reason. Try messaging the bot first".into(), + ))?; + tranquil_types::EmailAddress::new(trimmed).map_err(|_| ApiError::InvalidEmail) +} + fn trimmed(raw: Option<&str>) -> Option<&str> { raw.map(str::trim).filter(|value| !value.is_empty()) } @@ -214,7 +218,11 @@ pub async fn implicitly_verify_channel( let Ok(Some(prefs)) = user_repo.get_comms_prefs(user_id).await else { return; }; - let Some(recipient) = tranquil_pds::comms::recipient_for(&prefs, preferred_channel) else { + let Some(recipient) = tranquil_pds::comms::recipient_for( + &prefs, + preferred_channel, + tranquil_pds::comms::Secrecy::Secret, + ) else { warn!( did = %did, preferred = ?preferred_channel, diff --git a/crates/tranquil-api/src/identity/provision.rs b/crates/tranquil-api/src/identity/provision.rs index 0f0afa26..9c43db88 100644 --- a/crates/tranquil-api/src/identity/provision.rs +++ b/crates/tranquil-api/src/identity/provision.rs @@ -318,8 +318,11 @@ pub async fn enqueue_signup_verification( channel: CommsChannel, target: &tranquil_pds::comms::VerificationTarget, ) { - let token = - tranquil_pds::auth::verification_token::generate_signup_token(did, channel, &target.id); + let token = tranquil_pds::auth::verification_token::generate_signup_token( + did, + channel, + target.identifier_str(), + ); let formatted = tranquil_pds::auth::verification_token::format_token_for_display(&token); let hostname = &tranquil_config::get().server.hostname; if let Err(e) = tranquil_pds::comms::comms_repo::enqueue_signup_verification( @@ -343,8 +346,11 @@ pub async fn enqueue_migration_verification( channel: CommsChannel, target: &tranquil_pds::comms::VerificationTarget, ) { - let token = - tranquil_pds::auth::verification_token::generate_migration_token(did, channel, &target.id); + let token = tranquil_pds::auth::verification_token::generate_migration_token( + did, + channel, + target.identifier_str(), + ); let formatted = tranquil_pds::auth::verification_token::format_token_for_display(&token); let hostname = &tranquil_config::get().server.hostname; if let Err(e) = tranquil_pds::comms::comms_repo::enqueue_migration_verification( diff --git a/crates/tranquil-api/src/notification_prefs.rs b/crates/tranquil-api/src/notification_prefs.rs index b06e6e13..75df6cc4 100644 --- a/crates/tranquil-api/src/notification_prefs.rs +++ b/crates/tranquil-api/src/notification_prefs.rs @@ -175,12 +175,17 @@ pub async fn request_channel_verification( .map_err(|e| ApiError::InternalError(Some(e.to_string())))?; } _ => { - let fallback_handle = handle.map(|h| h.as_str()).unwrap_or_else(|| did.as_str()); + let fallback_email = handle + .map(|h| h.as_str()) + .or_else(|| Some(did.as_str())) + .and_then(|raw| tranquil_types::EmailAddress::new(raw).ok()); + let target = VerificationTarget::parse(channel, id, fallback_email) + .ok_or(ApiError::InvalidEmail)?; tranquil_pds::comms::comms_repo::enqueue_channel_verification( state.repos.user.as_ref(), state.repos.infra.as_ref(), user_id, - &VerificationTarget::resolve(channel, id, Some(fallback_handle))?, + &target, &formatted_token, &tranquil_config::get().server.hostname, channel, diff --git a/crates/tranquil-api/src/server/email.rs b/crates/tranquil-api/src/server/email.rs index 6b3e3b20..6e0836e7 100644 --- a/crates/tranquil-api/src/server/email.rs +++ b/crates/tranquil-api/src/server/email.rs @@ -361,9 +361,7 @@ pub async fn update_email( let formatted_token = tranquil_pds::auth::verification_token::format_token_for_display(&verification_token); let hostname = &tranquil_config::get().server.hostname; - let target = tranquil_pds::comms::VerificationTarget::direct( - tranquil_db_traits::Recipient::Email(new_email.clone()), - ); + let target = tranquil_pds::comms::VerificationTarget::Email(new_email.clone()); if let Err(e) = tranquil_pds::comms::comms_repo::enqueue_signup_verification( state.repos.user.as_ref(), state.repos.infra.as_ref(), diff --git a/crates/tranquil-api/src/server/session.rs b/crates/tranquil-api/src/server/session.rs index 54f735b4..0f58499b 100644 --- a/crates/tranquil-api/src/server/session.rs +++ b/crates/tranquil-api/src/server/session.rs @@ -16,13 +16,14 @@ use tranquil_pds::auth::{ Active, Auth, NormalizedLoginIdentifier, Permissive, require_legacy_session_mfa, require_reauth_window, }; +use tranquil_pds::comms::VerificationTarget; use tranquil_pds::rate_limit::{ LoginLimit, RateLimited, RefreshSessionLimit, TotpVerifyLimit, check_user_rate_limit_with_message, }; use tranquil_pds::state::AppState; use tranquil_pds::types::{AccountState, AtIdentifier, Did, Handle, PlainPassword}; -use tranquil_types::{Locale, TokenId}; +use tranquil_types::{EmailAddress, Locale, TokenId}; pub fn verification_blocks_login(channel_verification: &ChannelVerificationStatus) -> bool { !tranquil_config::get() @@ -990,8 +991,14 @@ async fn resend_signup_verification( warn!(did = %did, channel = ?row.channel, "We skipped {context} because the account doesn't have a recipient on file"); return false; }; - match tranquil_pds::comms::VerificationTarget::resolve(row.channel, id, row.email.as_deref()) { - Ok(target) => { + match VerificationTarget::parse( + row.channel, + id, + row.email + .as_deref() + .and_then(|raw| EmailAddress::new(raw).ok()), + ) { + Some(target) => { crate::identity::provision::enqueue_signup_verification( state, row.id, @@ -1002,7 +1009,7 @@ async fn resend_signup_verification( .await; true } - Err(_) => { + None => { warn!(did = %did, channel = ?row.channel, "We skipped {context} because the account doesn't have a valid recipient"); false } diff --git a/crates/tranquil-api/src/server/verify_email.rs b/crates/tranquil-api/src/server/verify_email.rs index 93370ac3..770a32bf 100644 --- a/crates/tranquil-api/src/server/verify_email.rs +++ b/crates/tranquil-api/src/server/verify_email.rs @@ -73,7 +73,11 @@ pub async fn resend_migration_verification( if user.email_verified { return Ok(Json(ResendMigrationVerificationOutput { sent: true })); } - let target = tranquil_pds::comms::VerificationTarget::resolve(channel, &id, Some(&id)).ok(); + let target = tranquil_pds::comms::VerificationTarget::parse( + channel, + &id, + tranquil_types::EmailAddress::new(&id).ok(), + ); if let Some(target) = target { crate::identity::provision::enqueue_migration_verification( &state, user.id, &user.did, channel, &target, diff --git a/crates/tranquil-api/src/server/verify_token.rs b/crates/tranquil-api/src/server/verify_token.rs index 0626ed1e..084cfc7c 100644 --- a/crates/tranquil-api/src/server/verify_token.rs +++ b/crates/tranquil-api/src/server/verify_token.rs @@ -197,7 +197,11 @@ async fn notify_channel_verified(state: &AppState, user_id: uuid::Uuid, channel: return; } }; - let Some(recipient) = tranquil_pds::comms::recipient_for(&prefs, channel) else { + let Some(recipient) = tranquil_pds::comms::recipient_for( + &prefs, + channel, + tranquil_pds::comms::Secrecy::NotSecret, + ) else { warn!( user_id = %user_id, channel = ?channel, diff --git a/crates/tranquil-oauth-server/src/sso_endpoints.rs b/crates/tranquil-oauth-server/src/sso_endpoints.rs index 3bef4ad2..462d4ef1 100644 --- a/crates/tranquil-oauth-server/src/sso_endpoints.rs +++ b/crates/tranquil-oauth-server/src/sso_endpoints.rs @@ -1391,7 +1391,7 @@ pub async fn complete_registration( let verification_token = tranquil_pds::auth::verification_token::generate_signup_token( &did, verification_channel, - &target.id, + target.identifier_str(), ); let formatted_token = tranquil_pds::auth::verification_token::format_token_for_display(&verification_token); diff --git a/crates/tranquil-pds/src/comms/mod.rs b/crates/tranquil-pds/src/comms/mod.rs index 29fe475e..4598c881 100644 --- a/crates/tranquil-pds/src/comms/mod.rs +++ b/crates/tranquil-pds/src/comms/mod.rs @@ -1,10 +1,12 @@ +mod notice; mod service; -pub use service::repo::Notice; -pub use service::{ - CommsService, NotificationError, VerificationTarget, recipient_for, repo as comms_repo, +pub use notice::{ + DeletionCode, InvalidCode, MigrationTarget, PasswordResetCode, PlcToken, RecoveryToken, + Secrecy, ShortToken, TwoFactorCode, VerificationCode, VerificationTarget, recipient_for, }; -pub use tranquil_comms::locale::{ChannelVerificationBody, ChannelVerificationSubject}; +pub use service::repo::Notice; +pub use service::{CommsService, NotificationError, repo as comms_repo}; pub use tranquil_comms::{ CommsChannel, CommsMessageType, CommsSender, CommsStatus, DEFAULT_LOCALE, DiscordSender, EmailSender, NotificationStrings, QueuedComms, SendError, SignalSender, TelegramSender, diff --git a/crates/tranquil-pds/src/comms/notice.rs b/crates/tranquil-pds/src/comms/notice.rs new file mode 100644 index 00000000..2776e850 --- /dev/null +++ b/crates/tranquil-pds/src/comms/notice.rs @@ -0,0 +1,507 @@ +use base64::Engine as _; +use base64::engine::general_purpose::URL_SAFE_NO_PAD; +use tracing::warn; +use tranquil_db_traits::{CommsChannel, Recipient, UserCommsPrefs}; +use tranquil_types::{ + ChannelIdentifier, DiscordUserId, DiscordUsername, EmailAddress, SignalUsername, + TelegramChatId, TelegramUsername, +}; +use uuid::Uuid; + +macro_rules! display_code { + ($name:ident, $shape:ident) => { + #[derive(Debug, Clone)] + pub struct $name(String); + + impl $name { + pub fn new(code: &str) -> Result { + $shape(code) + .then(|| Self(code.to_owned())) + .ok_or(InvalidCode) + } + + pub fn as_str(&self) -> &str { + &self.0 + } + } + }; +} + +display_code!(PasswordResetCode, is_human_token_code); +display_code!(TwoFactorCode, is_two_factor_code); +display_code!(DeletionCode, is_uuid_code); +display_code!(PlcToken, is_human_token_code); +display_code!(ShortToken, is_human_token_code); +display_code!(VerificationCode, is_verification_token); +display_code!(RecoveryToken, is_recovery_token); + +impl PasswordResetCode { + pub fn mint() -> Self { + Self::new(&crate::util::generate_token_code()) + .expect("`generate_token_code` ought to mint a token that looks ok for a human") + } +} + +impl PlcToken { + pub fn mint() -> Self { + Self::new(&crate::util::generate_token_code()) + .expect("`generate_token_code` ought to mint a token that looks ok for a human") + } +} + +impl TwoFactorCode { + pub fn from_challenge(code: &str) -> Self { + Self::new(code) + .expect("Challenge codes get minted looking like 6 digits *or* human-friendly tokens") + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct InvalidCode; + +// "human-friendly" +fn is_human_token_code(code: &str) -> bool { + let bytes = code.as_bytes(); + bytes.len() == 11 + && bytes[5] == b'-' + && bytes[..5] + .iter() + .chain(bytes[6..].iter()) + .all(|b| matches!(b, b'A'..=b'Z' | b'2'..=b'7')) +} + +fn is_two_factor_code(code: &str) -> bool { + is_human_token_code(code) || (code.len() == 6 && code.bytes().all(|b| b.is_ascii_digit())) +} + +fn is_uuid_code(code: &str) -> bool { + Uuid::parse_str(code).is_ok_and(|parsed| parsed.hyphenated().to_string() == code) +} + +fn is_verification_token(code: &str) -> bool { + URL_SAFE_NO_PAD + .decode(code) + .ok() + .and_then(|bytes| String::from_utf8(bytes).ok()) + .is_some_and(|payload| { + let mut fields = payload.split('|'); + fields.next() == Some("1") && fields.count() == 6 + }) +} + +fn is_recovery_token(code: &str) -> bool { + code.len() == 32 + && code + .bytes() + .all(|b| b.is_ascii_digit() || b.is_ascii_lowercase()) +} + +// Think about the situation on Telegram and Discord where the user must message a given bot *first* in order to hydrate a chat ID into our system so that we can in fact send things. +// If we can think of a better way to simply error-out later, instead of falling back to email when say Telegram is in an aborted state, let's do that. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum VerificationTarget { + Email(EmailAddress), + Signal(SignalUsername), + ViaEmail { + identifier: ChannelIdentifier, + address: EmailAddress, + }, +} + +impl VerificationTarget { + pub fn parse( + channel: CommsChannel, + identifier: &str, + fallback_email: Option, + ) -> Option { + match channel { + CommsChannel::Email => EmailAddress::new(identifier).ok().map(Self::Email), + CommsChannel::Signal => SignalUsername::new(identifier).ok().map(Self::Signal), + CommsChannel::Telegram => Some(Self::ViaEmail { + identifier: ChannelIdentifier::Telegram(TelegramUsername::new(identifier).ok()?), + address: fallback_email?, + }), + CommsChannel::Discord => Some(Self::ViaEmail { + identifier: ChannelIdentifier::Discord(DiscordUsername::new(identifier).ok()?), + address: fallback_email?, + }), + } + } + + pub fn identifier_str(&self) -> &str { + match self { + Self::Email(address) => address.as_str(), + Self::Signal(username) => username.as_str(), + Self::ViaEmail { identifier, .. } => identifier.as_str(), + } + } + + pub fn recipient(&self) -> Recipient { + match self { + Self::Email(address) => Recipient::Email(address.clone()), + Self::Signal(username) => Recipient::Signal(username.clone()), + Self::ViaEmail { address, .. } => Recipient::Email(address.clone()), + } + } + + pub fn channel(&self) -> CommsChannel { + match self { + Self::Email(_) => CommsChannel::Email, + Self::Signal(_) => CommsChannel::Signal, + Self::ViaEmail { identifier, .. } => identifier.channel(), + } + } +} + +pub enum MigrationTarget { + Email(EmailAddress), + Signal(SignalUsername), +} + +impl From for MigrationTarget { + fn from(target: VerificationTarget) -> Self { + match target { + VerificationTarget::Email(address) | VerificationTarget::ViaEmail { address, .. } => { + Self::Email(address) + } + VerificationTarget::Signal(username) => Self::Signal(username), + } + } +} + +impl MigrationTarget { + pub fn identifier(&self) -> &str { + match self { + Self::Email(address) => address.as_str(), + Self::Signal(username) => username.as_str(), + } + } + + pub fn recipient(&self) -> Recipient { + match self { + Self::Email(address) => Recipient::Email(address.clone()), + Self::Signal(username) => Recipient::Signal(username.clone()), + } + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Secrecy { + Secret, + NotSecret, +} + +pub fn recipient_for( + prefs: &UserCommsPrefs, + channel: CommsChannel, + secrecy: Secrecy, +) -> Option { + let fallback = || email_recipient(prefs, secrecy); + match channel { + CommsChannel::Email => fallback(), + CommsChannel::Telegram => telegram_recipient(prefs).or_else(fallback), + CommsChannel::Discord => discord_recipient(prefs).or_else(fallback), + CommsChannel::Signal => signal_recipient(prefs, secrecy).or_else(fallback), + } +} + +fn telegram_recipient(prefs: &UserCommsPrefs) -> Option { + let chat_id = prefs.telegram_chat_id?; + match TelegramChatId::from_i64(chat_id) { + Some(id) => Some(Recipient::Telegram(id)), + None => { + warn!( + chat_id, + // I know I know; even though email isn't special I would say it's at least somewhat more likely to exist than the others. + "Telegram chat ID isn't usable, falling back to email" + ); + None + } + } +} + +fn discord_recipient(prefs: &UserCommsPrefs) -> Option { + let raw = prefs.discord_id.as_deref()?; + match DiscordUserId::new(raw) { + Ok(id) => Some(Recipient::Discord(id)), + Err(_) => { + warn!( + discord_id = raw, + "Discord user ID isn't usable, falling back to email" + ); + None + } + } +} + +fn signal_recipient(prefs: &UserCommsPrefs, secrecy: Secrecy) -> Option { + match secrecy { + Secrecy::Secret if !prefs.signal_verified => None, + _ => { + let raw = prefs.signal_username.as_deref()?; + match SignalUsername::new(raw) { + Ok(username) => Some(Recipient::Signal(username)), + Err(_) => { + warn!( + signal_username = raw, + "Signal username isn't usable, falling back to email" + ); + None + } + } + } + } +} + +fn email_recipient(prefs: &UserCommsPrefs, secrecy: Secrecy) -> Option { + match secrecy { + Secrecy::Secret if !prefs.email_verified => None, + _ => prefs + .email + .as_deref() + .and_then(|email| EmailAddress::new(email).ok()) + .map(Recipient::Email), + } +} + +#[cfg(test)] +mod verification_target_tests { + use super::*; + + #[test] + fn bot_handles_send_over_fallback_email_for_now() { + let target = VerificationTarget::ViaEmail { + identifier: ChannelIdentifier::Telegram( + tranquil_types::TelegramUsername::new("oys_01").unwrap(), + ), + address: EmailAddress::new("user@jola.dev").unwrap(), + }; + assert_eq!(target.recipient().channel(), CommsChannel::Email); + assert_eq!(target.identifier_str(), "oys_01"); + + let target = VerificationTarget::ViaEmail { + identifier: ChannelIdentifier::Discord( + tranquil_types::DiscordUsername::new("oys").unwrap(), + ), + address: EmailAddress::new("user@jola.dev").unwrap(), + }; + assert_eq!(target.recipient().channel(), CommsChannel::Email); + } + + #[test] + fn direct_channels_deliver_to_channel() { + let email = VerificationTarget::Email(EmailAddress::new("user@nel.pet").unwrap()); + assert_eq!(email.recipient().as_str(), "user@nel.pet"); + assert_eq!(email.channel(), CommsChannel::Email); + + let signal = VerificationTarget::Signal(SignalUsername::new("oys.01").unwrap()); + assert_eq!(signal.recipient().channel(), CommsChannel::Signal); + assert_eq!(signal.identifier_str(), "oys.01"); + } + + #[test] + fn migration_targets_bind_delivery_addr() { + let signup = VerificationTarget::ViaEmail { + identifier: ChannelIdentifier::Telegram( + tranquil_types::TelegramUsername::new("oys_01").unwrap(), + ), + address: EmailAddress::new("user@jola.dev").unwrap(), + }; + let migration = MigrationTarget::from(signup); + assert_eq!(migration.identifier(), "user@jola.dev"); + assert_eq!(migration.recipient().channel(), CommsChannel::Email); + + let signal = MigrationTarget::from(VerificationTarget::Signal( + SignalUsername::new("oys.01").unwrap(), + )); + assert_eq!(signal.identifier(), "oys.01"); + assert_eq!(signal.recipient().channel(), CommsChannel::Signal); + } + + #[test] + fn parse_builds_nice_targets() { + let email = VerificationTarget::parse(CommsChannel::Email, "user@nel.pet", None); + assert_eq!( + email, + Some(VerificationTarget::Email( + EmailAddress::new("user@nel.pet").unwrap() + )) + ); + + let signal = VerificationTarget::parse(CommsChannel::Signal, "oys.01", None); + assert_eq!( + signal, + Some(VerificationTarget::Signal( + SignalUsername::new("oys.01").unwrap() + )) + ); + + let fallback = EmailAddress::new("user@jola.dev").unwrap(); + let telegram = + VerificationTarget::parse(CommsChannel::Telegram, "oys_01", Some(fallback.clone())); + assert_eq!( + telegram, + Some(VerificationTarget::ViaEmail { + identifier: ChannelIdentifier::Telegram(TelegramUsername::new("oys_01").unwrap()), + address: fallback, + }) + ); + + assert_eq!( + VerificationTarget::parse(CommsChannel::Telegram, "oys_01", None), + None + ); + assert_eq!( + VerificationTarget::parse(CommsChannel::Discord, "not a handle", None), + None + ); + assert_eq!( + VerificationTarget::parse(CommsChannel::Email, "garbage", None), + None + ); + } +} + +#[cfg(test)] +mod recipient_for_tests { + use super::*; + use tranquil_types::Locale; + + fn undeliverable_prefs() -> UserCommsPrefs { + UserCommsPrefs { + email: None, + email_verified: false, + handle: "oys.nel.pet".parse().unwrap(), + preferred_channel: CommsChannel::Telegram, + preferred_locale: Locale::default(), + telegram_chat_id: None, + discord_id: None, + signal_username: None, + signal_verified: false, + } + } + + #[test] + fn undeliverable_prefs_dont_resolve_on_all_channels() { + let prefs = undeliverable_prefs(); + assert_eq!( + recipient_for(&prefs, CommsChannel::Telegram, Secrecy::Secret), + None + ); + assert_eq!( + recipient_for(&prefs, CommsChannel::Email, Secrecy::NotSecret), + None + ); + } + + #[test] + fn chat_id_number_zero_falls_back_to_email_for_now() { + let prefs = UserCommsPrefs { + telegram_chat_id: Some(0), + email: Some("oys@jola.dev".into()), + email_verified: true, + ..undeliverable_prefs() + }; + assert_eq!( + recipient_for(&prefs, CommsChannel::Telegram, Secrecy::Secret), + Some(Recipient::Email(EmailAddress::new("oys@jola.dev").unwrap())) + ); + } + + #[test] + fn secrets_reject_unverified_email() { + let prefs = UserCommsPrefs { + email: Some("oys@jola.dev".into()), + email_verified: false, + ..undeliverable_prefs() + }; + assert_eq!( + recipient_for(&prefs, CommsChannel::Email, Secrecy::Secret), + None + ); + assert_eq!( + recipient_for(&prefs, CommsChannel::Telegram, Secrecy::Secret), + None + ); + assert_eq!( + recipient_for(&prefs, CommsChannel::Email, Secrecy::NotSecret), + Some(Recipient::Email(EmailAddress::new("oys@jola.dev").unwrap())) + ); + } + + #[test] + fn secrets_reject_unverified_signal_username() { + let prefs = UserCommsPrefs { + signal_username: Some("oys.01".into()), + email: Some("oys@jola.dev".into()), + email_verified: true, + ..undeliverable_prefs() + }; + assert_eq!( + recipient_for(&prefs, CommsChannel::Signal, Secrecy::NotSecret), + Some(Recipient::Signal(SignalUsername::new("oys.01").unwrap())) + ); + assert_eq!( + recipient_for(&prefs, CommsChannel::Signal, Secrecy::Secret), + Some(Recipient::Email(EmailAddress::new("oys@jola.dev").unwrap())) + ); + + let stranded = UserCommsPrefs { + signal_username: Some("oys.01".into()), + ..undeliverable_prefs() + }; + assert_eq!( + recipient_for(&stranded, CommsChannel::Signal, Secrecy::Secret), + None + ); + } +} + +#[cfg(test)] +mod code_newtype_tests { + use super::*; + use tranquil_types::Did; + + #[test] + fn codes_accept_well_formed_values() { + assert!(PasswordResetCode::new("ABCDE-FGHIJ").is_ok()); + assert!(ShortToken::new("ABCDE-FGHIJ").is_ok()); + assert!(PlcToken::new("ABCDE-FGHIJ").is_ok()); + assert!(TwoFactorCode::new("ABCDE-FGHIJ").is_ok()); + assert!(TwoFactorCode::new("042042").is_ok()); + assert!(DeletionCode::new("0f0e8d64-6d91-4c33-8f2a-9db09b86a6e0").is_ok()); + assert!(RecoveryToken::new("a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6").is_ok()); + + tranquil_config::ensure_test_defaults(); + let did = Did::new("did:plc:code01".to_string()).unwrap(); + let token = crate::auth::verification_token::generate_signup_token( + &did, + CommsChannel::Email, + "user@nel.pet", + ); + assert!(VerificationCode::new(&token).is_ok()); + } + + #[test] + fn codes_reject_weird_values() { + assert_eq!(PasswordResetCode::mint().as_str().len(), 11); + assert_eq!(PlcToken::mint().as_str().len(), 11); + assert_eq!(TwoFactorCode::from_challenge("042042").as_str(), "042042"); + assert!(PasswordResetCode::new("042042").is_err()); + assert!(TwoFactorCode::new("not-a-code").is_err()); + assert!(DeletionCode::new("ABCDE-FGHIJ").is_err()); + assert!(DeletionCode::new("0f0e8d646d914c338f2a9db09b86a6e0").is_err()); + assert!(VerificationCode::new("ABCDEFGHIJKLMNOP").is_err()); + assert!(RecoveryToken::new("short").is_err()); + assert!(!is_human_token_code("abcde-fghij")); + assert!(!is_human_token_code("ABCDEFGH-I")); + } + + #[test] + fn no_cross_code_type_contamination() { + let human_code = "ABCDE-FGHIJ"; + assert!(PasswordResetCode::new(human_code).is_ok()); + assert!(VerificationCode::new(human_code).is_err()); + assert!(DeletionCode::new(human_code).is_err()); + } +} diff --git a/crates/tranquil-pds/src/comms/service.rs b/crates/tranquil-pds/src/comms/service.rs index 9170a900..5f297fe2 100644 --- a/crates/tranquil-pds/src/comms/service.rs +++ b/crates/tranquil-pds/src/comms/service.rs @@ -25,11 +25,13 @@ use tranquil_db_traits::{ DbError, InfraRepository, OutboundMessage, QueuedComms, Recipient, UserCommsPrefs, UserRepository, }; -use tranquil_types::{DiscordUserId, EmailAddress, Locale, SignalUsername, TelegramChatId}; +use tranquil_types::{EmailAddress, Locale}; use uuid::Uuid; use crate::auth::verification_token::VerificationPurpose; +use super::notice::{Secrecy, VerificationTarget, recipient_for}; + pub struct CommsService { infra_repo: Arc, senders: HashMap>, @@ -210,81 +212,6 @@ impl CommsService { } } -// Think about the situation on Telegram and Discord where the user must message a given bot *first* in order to hydrate a chat ID into our system so that we can in fact send things. -// If we can think of a better way to simply error-out later, instead of falling back to email when say Telegram is in an aborted state, let's do that. -pub struct VerificationTarget { - pub id: String, - pub recipient: Recipient, -} - -impl VerificationTarget { - pub fn direct(recipient: Recipient) -> Self { - Self { - id: recipient.as_str().to_string(), - recipient, - } - } - - pub fn resolve( - channel: CommsChannel, - id: &str, - fallback: Option<&str>, - ) -> Result { - let direct = (!channel.verifies_via_bot()) - .then(|| Recipient::new(channel, id)) - .and_then(Result::ok); - let recipient = direct.map_or_else(|| fallback_recipient(fallback), Ok)?; - Ok(Self { - id: id.to_string(), - recipient, - }) - } -} - -fn fallback_recipient(fallback: Option<&str>) -> Result { - let raw = fallback - .map(str::trim) - .filter(|email| !email.is_empty()) - .ok_or(crate::api::error::ApiError::InvalidRequest( - "Verification over this channel needs an email address. Message the bot first".into(), - ))?; - EmailAddress::new(raw) - .map(Recipient::Email) - .map_err(|_| crate::api::error::ApiError::InvalidEmail) -} - -pub fn recipient_for(prefs: &UserCommsPrefs, channel: CommsChannel) -> Option { - let fallback = || email_recipient(prefs); - match channel { - CommsChannel::Email => fallback(), - CommsChannel::Telegram => prefs - .telegram_chat_id - .and_then(TelegramChatId::from_i64) - .map(Recipient::Telegram) - .or_else(fallback), - CommsChannel::Discord => prefs - .discord_id - .as_deref() - .and_then(|id| DiscordUserId::new(id).ok()) - .map(Recipient::Discord) - .or_else(fallback), - CommsChannel::Signal => prefs - .signal_username - .as_deref() - .and_then(|name| SignalUsername::new(name).ok()) - .map(Recipient::Signal) - .or_else(fallback), - } -} - -fn email_recipient(prefs: &UserCommsPrefs) -> Option { - prefs - .email - .as_deref() - .and_then(|email| EmailAddress::new(email).ok()) - .map(Recipient::Email) -} - #[derive(Debug)] pub enum NotificationError { InvalidRequest(String), @@ -349,6 +276,12 @@ pub mod repo { _ => None, } } + fn secrecy(&self) -> Secrecy { + match self { + Self::Welcome | Self::LegacyLoginAlert { .. } => Secrecy::NotSecret, + _ => Secrecy::Secret, + } + } fn rendered( &self, @@ -509,7 +442,7 @@ pub mod repo { ) -> Result, NotificationError> { let prefs = get_comms_prefs(user_repo, user_id).await?; let channel = notice.channel().unwrap_or(prefs.preferred_channel); - let Some(recipient) = recipient_for(&prefs, channel) else { + let Some(recipient) = recipient_for(&prefs, channel, notice.secrecy()) else { warn!( user_id = %user_id, channel = ?channel, @@ -613,7 +546,7 @@ pub mod repo { let prefs = get_comms_prefs(user_repo, user_id).await?; let locale = locale_of(&prefs); let strings = get_strings(locale); - let encoded_id = urlencoding::encode(&target.id); + let encoded_id = urlencoding::encode(target.identifier_str()); let encoded_token = urlencoding::encode(token); let verify_page = format!("https://{hostname}/app/verify"); let verify_link = format!( @@ -644,7 +577,7 @@ pub mod repo { infra_repo .enqueue_comms(OutboundMessage { user_id, - recipient: target.recipient.clone(), + recipient: target.recipient(), kind: CommsMessageType::MigrationVerification, subject: Some(rendered.subject), body: rendered.body, @@ -672,7 +605,7 @@ pub mod repo { let locale = prefs.as_ref().map(locale_of).unwrap_or_default(); let strings = get_strings(locale); let encoded_token = urlencoding::encode(code); - let encoded_id = urlencoding::encode(&target.id); + let encoded_id = urlencoding::encode(target.identifier_str()); let verify_page = format!("https://{hostname}/app/verify"); let verify_link = format!( "https://{}/app/verify?token={}&identifier={}", @@ -702,7 +635,7 @@ pub mod repo { infra_repo .enqueue_comms(OutboundMessage { user_id, - recipient: target.recipient.clone(), + recipient: target.recipient(), kind: CommsMessageType::EmailVerification, subject: Some(rendered.subject), body: rendered.body, @@ -735,7 +668,7 @@ pub mod repo { let strings = get_strings(locale); // TODO clean up identifier encoding within the rendering let encoded_token = urlencoding::encode(code); - let encoded_id = urlencoding::encode(&target.id); + let encoded_id = urlencoding::encode(target.identifier_str()); let verify_link = format!( "https://{}/app/verify?token={}&identifier={}", hostname, encoded_token, encoded_id @@ -758,7 +691,7 @@ pub mod repo { infra_repo .enqueue_comms(OutboundMessage { user_id, - recipient: target.recipient.clone(), + recipient: target.recipient(), kind: CommsMessageType::ChannelVerification, subject: Some(rendered.subject), body: rendered.body, @@ -950,95 +883,3 @@ pub mod repo { } } } - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn bot_channel_recipients_fall_back_to_email() { - let telegram = - VerificationTarget::resolve(CommsChannel::Telegram, "123456789", Some("user@jola.dev")) - .unwrap(); - assert_eq!(telegram.recipient.channel(), CommsChannel::Email); - - let discord = VerificationTarget::resolve( - CommsChannel::Discord, - "274656283714826240", - Some("user@jola.dev"), - ) - .unwrap(); - assert_eq!(discord.recipient.channel(), CommsChannel::Email); - } - - #[test] - fn resolution_keeps_id_for_bot_channels() { - let target = - VerificationTarget::resolve(CommsChannel::Telegram, "oys_01", Some("user@jola.dev")) - .unwrap(); - assert_eq!(target.id, "oys_01"); - assert_eq!(target.recipient.channel(), CommsChannel::Email); - } - - #[test] - fn direct_channels_parse_id() { - let email = VerificationTarget::resolve(CommsChannel::Email, "user@nel.pet", None).unwrap(); - assert_eq!(email.recipient.as_str(), "user@nel.pet"); - - let signal = VerificationTarget::resolve(CommsChannel::Signal, "oys.01", None).unwrap(); - assert_eq!(signal.recipient.channel(), CommsChannel::Signal); - } - - #[test] - fn signal_falls_back_when_id_isnt_username() { - let target = - VerificationTarget::resolve(CommsChannel::Signal, "oys", Some("user@jola.dev")) - .unwrap(); - assert_eq!(target.recipient.channel(), CommsChannel::Email); - } - - #[test] - fn resolve_fails_without_fallback() { - assert!(VerificationTarget::resolve(CommsChannel::Telegram, "oys_01", None).is_err()); - assert!(VerificationTarget::resolve(CommsChannel::Signal, "oys", None).is_err()); - } -} - -#[cfg(test)] -mod recipient_for_tests { - use super::*; - - fn undeliverable_prefs() -> UserCommsPrefs { - UserCommsPrefs { - email: None, - email_verified: false, - handle: "oys.nel.pet".parse().unwrap(), - preferred_channel: CommsChannel::Telegram, - preferred_locale: Locale::default(), - telegram_chat_id: None, - discord_id: None, - signal_username: None, - signal_verified: false, - } - } - - #[test] - fn undeliverable_prefs_resolve_to_none_on_every_channel() { - let prefs = undeliverable_prefs(); - assert_eq!(recipient_for(&prefs, CommsChannel::Telegram), None); - assert_eq!(recipient_for(&prefs, CommsChannel::Email), None); - } - - #[test] - fn zero_chat_id_falls_back_to_email() { - let prefs = UserCommsPrefs { - telegram_chat_id: Some(0), - email: Some("oys@jola.dev".into()), - ..undeliverable_prefs() - }; - assert_eq!( - recipient_for(&prefs, CommsChannel::Telegram), - Some(Recipient::Email(EmailAddress::new("oys@jola.dev").unwrap())) - ); - } -} diff --git a/crates/tranquil-pds/tests/it/password_reset.rs b/crates/tranquil-pds/tests/it/password_reset.rs index 7136acc5..9005d449 100644 --- a/crates/tranquil-pds/tests/it/password_reset.rs +++ b/crates/tranquil-pds/tests/it/password_reset.rs @@ -1,7 +1,7 @@ use crate::helpers::verify_new_account; use reqwest::StatusCode; use serde_json::{Value, json}; -use tranquil_db_traits::CommsMessageType; +use tranquil_db_traits::{CommsChannel, CommsMessageType}; #[tokio::test] async fn test_request_password_reset_creates_code() { @@ -362,6 +362,11 @@ async fn test_reset_password_creates_notification() { .await .expect("failed to look up user") .expect("user not found"); + repos + .user + .set_channel_verified(&user.did, CommsChannel::Email) + .await + .expect("verify email"); let initial_count = repos .infra .count_comms_by_type(user.id, CommsMessageType::PasswordReset)