diff --git a/core/conversations/src/conversation/group_v1.rs b/core/conversations/src/conversation/group_v1.rs index b562a4a..3e364ac 100644 --- a/core/conversations/src/conversation/group_v1.rs +++ b/core/conversations/src/conversation/group_v1.rs @@ -254,7 +254,10 @@ impl Convo for GroupV1Convo { let msg_hash = blake2b_hex::(&[bytes.as_ref()]); if self.outbound_msgs.contains(&msg_hash) { debug!("Dropping message, sent from self"); - return Ok(ConvoOutcome::empty(self.convo_id.to_string())); + return Ok(ConvoOutcome::empty( + self.convo_id.to_string(), + crate::ConversationClass::Group, + )); } let mls_message: MlsMessageIn = @@ -266,7 +269,10 @@ impl Convo for GroupV1Convo { if protocol_message.epoch() < self.mls_group.epoch() { // TODO: (P1) Add logging for messages arriving from past epoch. - return Ok(ConvoOutcome::empty(self.id().to_string())); + return Ok(ConvoOutcome::empty( + self.id().to_string(), + crate::ConversationClass::Group, + )); } let processed = self @@ -299,6 +305,7 @@ impl Convo for GroupV1Convo { Ok(ConvoOutcome { convo_id: self.id().to_string(), content, + class: crate::ConversationClass::Group, }) } diff --git a/core/conversations/src/conversation/group_v2.rs b/core/conversations/src/conversation/group_v2.rs index 1532145..5942730 100644 --- a/core/conversations/src/conversation/group_v2.rs +++ b/core/conversations/src/conversation/group_v2.rs @@ -239,7 +239,12 @@ where let frame = GroupV2Frame::decode(bytes.as_ref()).map_err(ChatError::generic)?; let inner = match frame.payload { Some(GroupV2Payload::DeMlsWrapper(b)) => b.to_vec(), - _ => return Ok(ConvoOutcome::empty(self.convo_id.clone())), + _ => { + return Ok(ConvoOutcome::empty( + self.convo_id.clone(), + crate::ConversationClass::Group, + )); + } }; self.conversation.process_inbound( @@ -256,7 +261,10 @@ where Some(o) => Ok(o), None => { warn!("returning None as ConvoOutcome"); - Ok(ConvoOutcome::empty(self.convo_id.to_string())) + Ok(ConvoOutcome::empty( + self.convo_id.to_string(), + crate::ConversationClass::Group, + )) } } } @@ -387,6 +395,7 @@ impl GroupV2Convo { bytes: cm.message.clone(), encoded_credential: cm.sender.clone(), }), + class: crate::ConversationClass::Group, }), _ => None, }) diff --git a/core/conversations/src/conversation/privatev1.rs b/core/conversations/src/conversation/privatev1.rs index a8a9054..e65024c 100644 --- a/core/conversations/src/conversation/privatev1.rs +++ b/core/conversations/src/conversation/privatev1.rs @@ -276,6 +276,7 @@ impl Convo for PrivateV1Convo { Ok(ConvoOutcome { convo_id: self.id().to_string(), content, + class: crate::ConversationClass::Private, }) } diff --git a/core/conversations/src/outcomes.rs b/core/conversations/src/outcomes.rs index 3f77294..134f38b 100644 --- a/core/conversations/src/outcomes.rs +++ b/core/conversations/src/outcomes.rs @@ -22,13 +22,18 @@ pub struct Content { pub struct ConvoOutcome { pub convo_id: ConversationId, pub content: Option, + /// Class of the conversation this outcome belongs to. Surfaced so a + /// consumer can tell an anonymous PrivateV1 message (no sender credential + /// by design) from a group message that is missing one. + pub class: ConversationClass, } impl ConvoOutcome { - pub fn empty(convo_id: ConversationId) -> Self { + pub fn empty(convo_id: ConversationId, class: ConversationClass) -> Self { Self { convo_id, content: None, + class, } } } diff --git a/crates/client/src/client.rs b/crates/client/src/client.rs index 13c5449..6dfaf54 100644 --- a/crates/client/src/client.rs +++ b/crates/client/src/client.rs @@ -5,8 +5,9 @@ use components::{ThreadedWakeupService, WakeupEvent}; use crossbeam_channel::{Receiver, Sender, select}; use crypto::Ed25519VerifyingKey; use libchat::{ - AccountDirectory, ConversationId, ConvoOutcome, Core, DeliveryService, IdentId, IdentIdRef, - IdentityProvider, InboxOutcome, Introduction, PayloadOutcome, RegistrationService, + AccountDirectory, ConversationClass, ConversationId, ConvoOutcome, Core, DeliveryService, + IdentId, IdentIdRef, IdentityProvider, InboxOutcome, Introduction, PayloadOutcome, + RegistrationService, }; use parking_lot::Mutex; use storage::ChatStore; @@ -260,8 +261,9 @@ fn account_key_from_hex(addr: &str) -> Option { /// Why a message's sender could not be accepted, so the message is dropped. #[derive(Debug, PartialEq, Eq)] enum SenderError { - /// No credential at all, so no sender can be attributed. Every delivered - /// message must carry an explicit sender. + /// No credential at all, so no sender can be attributed. The caller decides + /// what this means per conversation class: dropped for a group, delivered + /// without a sender for an anonymous PrivateV1 intro. Missing, /// Credential bytes were not valid hex. NotHex, @@ -278,10 +280,11 @@ enum SenderError { /// Decode and verify a message's sender from its credential, checked against the /// account → device directory (our account store). /// -/// `Ok(sender)` — deliver with the sender; its `account` is set only when the -/// directory confirmed the device, so it is always verified. `Err` — drop the -/// message (including when no credential is present, since every delivered -/// message must carry an explicit sender). +/// `Ok(sender)` — the sender was attributed; its `account` is set only when the +/// directory confirmed the device, so it is always verified. `Err` — no sender +/// could be attributed (see [`SenderError`]). Whether an unattributed message is +/// dropped or delivered without a sender is decided by [`message_sender`] +/// according to the conversation class. fn decode_sender( directory: &impl AccountDirectory, encoded: &[u8], @@ -328,11 +331,36 @@ fn decode_sender( } } +/// Resolve the sender to attach to a received message, honouring the +/// conversation class. Returns `None` to drop the message, `Some(None)` to +/// deliver it with no sender (an anonymous PrivateV1 message, which binds no +/// credential by design), and `Some(Some(sender))` for a credential-bearing +/// message whose sender verified. +fn message_sender( + directory: &impl AccountDirectory, + encoded_credential: &[u8], + class: ConversationClass, +) -> Option> { + match decode_sender(directory, encoded_credential) { + Ok(sender) => Some(Some(sender)), + // PrivateV1 is an out-of-band X3DH intro and attaches no credential, so + // surface its messages with no sender. For any other class an absent + // credential is a protocol violation, dropped along with every other + // unverifiable-sender case. + Err(SenderError::Missing) if class == ConversationClass::Private => Some(None), + Err(_) => None, + } +} + fn convo_events(outcome: ConvoOutcome, directory: &impl AccountDirectory) -> Vec { - let ConvoOutcome { convo_id, content } = outcome; + let ConvoOutcome { + convo_id, + content, + class, + } = outcome; content .and_then(|c| { - let sender = decode_sender(directory, &c.encoded_credential).ok()?; + let sender = message_sender(directory, &c.encoded_credential, class)?; Some(Event::MessageReceived { convo_id: Arc::from(convo_id), content: c.bytes, @@ -355,7 +383,8 @@ fn inbox_events(outcome: InboxOutcome, directory: &impl AccountDirectory) -> Vec class: new_conversation.class, }); if let Some(c) = initial.and_then(|co| co.content) - && let Ok(sender) = decode_sender(directory, &c.encoded_credential) + && let Some(sender) = + message_sender(directory, &c.encoded_credential, new_conversation.class) { events.push(Event::MessageReceived { convo_id: Arc::clone(&id), diff --git a/crates/client/src/event.rs b/crates/client/src/event.rs index eaf5dbe..f751e7d 100644 --- a/crates/client/src/event.rs +++ b/crates/client/src/event.rs @@ -32,11 +32,13 @@ pub enum Event { convo_id: Arc, class: ConversationClass, }, - /// User content arrived on an existing conversation. + /// User content arrived on an existing conversation. `sender` is `None` for + /// an anonymous PrivateV1 message: that conversation is an out-of-band X3DH + /// intro and binds no sender credential, so no identity can be resolved. MessageReceived { convo_id: Arc, content: Vec, - sender: MessageSender, + sender: Option, }, InboundError { message: String, diff --git a/crates/client/tests/saro_and_raya.rs b/crates/client/tests/saro_and_raya.rs index 091083c..ac4a628 100644 --- a/crates/client/tests/saro_and_raya.rs +++ b/crates/client/tests/saro_and_raya.rs @@ -130,7 +130,9 @@ fn direct_v1_standalone_integration() { .expect("payload mismatch"); expect_event(&raya_events, "MessageReceived", |e| match e { Event::MessageReceived { - content, sender, .. + content, + sender: Some(sender), + .. } => { assert_eq!(content.as_slice(), b"Hey from saro"); // saro associated an account and published a matching bundle, so the @@ -173,7 +175,7 @@ fn saro_raya_message_exchange() { Event::MessageReceived { convo_id, content, - sender, + sender: Some(sender), } => { assert_eq!(convo_id, raya_convo_id); assert_eq!(content.as_slice(), b"hello raya"); @@ -229,6 +231,56 @@ fn saro_raya_message_exchange() { assert_eq!(raya.list_conversations().unwrap().len(), 1); } +/// PrivateV1 (intro-bundle) is an out-of-band X3DH intro that binds no sender +/// credential, so its messages must still surface — with no sender — rather +/// than be dropped. Covers both receive paths: the recipient's initial message +/// (inbox) and the reply on the established conversation (convo). +#[test] +fn private_v1_integration() { + let bus = MessageBus::default(); + let reg_service = EphemeralRegistry::new(); + + let (mut saro, saro_events) = + create_test_client(bus.clone(), reg_service.clone()).expect("client create"); + let (mut raya, raya_events) = + create_test_client(bus.clone(), reg_service.clone()).expect("client create"); + + let raya_bundle = raya.create_intro_bundle().expect("intro bundle"); + saro.create_conversation(&raya_bundle, b"hello raya") + .expect("convo create"); + + let raya_convo_id = expect_event(&raya_events, "ConversationStarted", |e| match e { + Event::ConversationStarted { convo_id, .. } => Ok(convo_id), + other => Err(other), + }); + expect_event(&raya_events, "MessageReceived", |e| match e { + Event::MessageReceived { + content, sender, .. + } => { + assert_eq!(content.as_slice(), b"hello raya"); + assert!( + sender.is_none(), + "PrivateV1 message must surface with no sender" + ); + Ok(()) + } + other => Err(other), + }); + + raya.send_message(&raya_convo_id, b"hi saro") + .expect("reply"); + expect_event(&saro_events, "MessageReceived", |e| match e { + Event::MessageReceived { + content, sender, .. + } => { + assert_eq!(content.as_slice(), b"hi saro"); + assert!(sender.is_none()); + Ok(()) + } + other => Err(other), + }); +} + #[derive(Debug)] struct FailingDelivery { inbound_tx: Sender>,