diff --git a/bindings_ffi/src/mls.rs b/bindings_ffi/src/mls.rs index c64c70107..a9dfbdbc8 100644 --- a/bindings_ffi/src/mls.rs +++ b/bindings_ffi/src/mls.rs @@ -3662,6 +3662,90 @@ mod tests { assert_eq!(client2_members.len(), 2); } + // ... existing code ... + + #[tokio::test(flavor = "multi_thread", worker_threads = 5)] + async fn test_create_new_installation_can_see_dm() { + // Create two wallets + let wallet1_key = &mut rng(); + let wallet1 = xmtp_cryptography::utils::LocalWallet::new(wallet1_key); + let wallet2_key = &mut rng(); + let wallet2 = xmtp_cryptography::utils::LocalWallet::new(wallet2_key); + + // Create initial clients + let client1 = new_test_client_with_wallet(wallet1.clone()).await; + let client2 = new_test_client_with_wallet(wallet2).await; + + // Create DM from client1 to client2 + let dm_group = client1 + .conversations() + .create_dm(client2.account_address.clone()) + .await + .unwrap(); + + // Sync both clients + client1.conversations().sync().await.unwrap(); + client2.conversations().sync().await.unwrap(); + + // Verify both clients can see the DM + let client1_groups = client1 + .conversations() + .list_dms(FfiListConversationsOptions::default()) + .unwrap(); + let client2_groups = client2 + .conversations() + .list_dms(FfiListConversationsOptions::default()) + .unwrap(); + assert_eq!(client1_groups.len(), 1, "Client1 should see 1 conversation"); + assert_eq!(client2_groups.len(), 1, "Client2 should see 1 conversation"); + + // Create a second client1 with same wallet + let client1_second = new_test_client_with_wallet(wallet1).await; + + // Verify client1_second starts with no conversations + let initial_conversations = client1_second + .conversations() + .list(FfiListConversationsOptions::default()) + .unwrap(); + assert_eq!( + initial_conversations.len(), + 0, + "New client should start with no conversations" + ); + + // Send message from client1 to client2 + dm_group + .send("Hello from client1".as_bytes().to_vec()) + .await + .unwrap(); + + // Sync all clients + client1.conversations().sync().await.unwrap(); + // client2.conversations().sync().await.unwrap(); + + tracing::info!( + "ABOUT TO SYNC CLIENT 1 SECOND: {}", + client1_second.inbox_id().to_string() + ); + client1_second.conversations().sync().await.unwrap(); + + // Verify second client1 can see the DM + let client1_second_groups = client1_second + .conversations() + .list_dms(FfiListConversationsOptions::default()) + .unwrap(); + assert_eq!( + client1_second_groups.len(), + 1, + "Second client1 should see 1 conversation" + ); + assert_eq!( + client1_second_groups[0].conversation.id(), + dm_group.id(), + "Second client1's conversation should match original DM" + ); + } + #[tokio::test(flavor = "multi_thread", worker_threads = 5)] async fn test_create_new_installations_does_not_fork_group() { let bo_wallet_key = &mut rng(); diff --git a/xmtp_mls/src/groups/mod.rs b/xmtp_mls/src/groups/mod.rs index ce62993dc..7ec7ce91f 100644 --- a/xmtp_mls/src/groups/mod.rs +++ b/xmtp_mls/src/groups/mod.rs @@ -1699,30 +1699,48 @@ fn validate_dm_group( mls_group: &OpenMlsGroup, added_by_inbox: &str, ) -> Result<(), GroupError> { + // Validate dm specific immutable metadata let metadata = extract_group_metadata(mls_group)?; - // Check if the conversation type is DM + // 1) Check if the conversation type is DM if metadata.conversation_type != ConversationType::Dm { return Err(GroupError::Generic( "Invalid conversation type for DM group".to_string(), )); } - // Check if DmMembers are set and validate their contents - if let Some(dm_members) = metadata.dm_members { - let our_inbox_id = client.inbox_id(); - if !((dm_members.member_one_inbox_id == added_by_inbox - && dm_members.member_two_inbox_id == our_inbox_id) - || (dm_members.member_one_inbox_id == our_inbox_id - && dm_members.member_two_inbox_id == added_by_inbox)) + // 2) If `dm_members` is not set, return an error immediately + let dm_members = match &metadata.dm_members { + Some(dm) => dm, + None => { + return Err(GroupError::Generic( + "DM group must have DmMembers set".to_string(), + )); + } + }; + + // 3) If the inbox that added this group is our inbox, make sure that + // one of the `dm_members` is our inbox id + if added_by_inbox == client.inbox_id() { + if !(dm_members.member_one_inbox_id == client.inbox_id() + || dm_members.member_two_inbox_id == client.inbox_id()) { return Err(GroupError::Generic( - "DM members do not match expected inboxes".to_string(), + "DM group must have our inbox as one of the dm members".to_string(), )); } - } else { + return Ok(()); + } + + // 4) Otherwise, make sure one of the `dm_members` is ours, and the other is `added_by_inbox` + let is_expected_pair = (dm_members.member_one_inbox_id == added_by_inbox + && dm_members.member_two_inbox_id == client.inbox_id()) + || (dm_members.member_one_inbox_id == client.inbox_id() + && dm_members.member_two_inbox_id == added_by_inbox); + + if !is_expected_pair { return Err(GroupError::Generic( - "DM group must have DmMembers set".to_string(), + "DM members do not match expected inboxes".to_string(), )); }