diff --git a/src/chat.rs b/src/chat.rs index f8edc35315..79ccafe614 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -24,6 +24,7 @@ use crate::config::Config; use crate::constants::{ self, Blocked, Chattype, DC_CHAT_ID_ALLDONE_HINT, DC_CHAT_ID_ARCHIVED_LINK, DC_CHAT_ID_LAST_SPECIAL, DC_CHAT_ID_TRASH, DC_RESEND_USER_AVATAR_DAYS, + TIMESTAMP_SENT_TOLERANCE, }; use crate::contact::{self, Contact, ContactId, Origin}; use crate::context::Context; @@ -1962,6 +1963,34 @@ impl Chat { } } + /// Returns chat member list timestamp. + pub(crate) async fn member_list_timestamp(&self, context: &Context) -> Result { + if let Some(member_list_timestamp) = self.param.get_i64(Param::MemberListTimestamp) { + Ok(member_list_timestamp) + } else { + let creation_timestamp: i64 = context + .sql + .query_get_value("SELECT created_timestamp FROM chats WHERE id=?", (self.id,)) + .await + .context("SQL error querying created_timestamp")? + .context("Chat not found")?; + Ok(creation_timestamp) + } + } + + /// Returns true if member list is stale, + /// i.e. has not been updated for 60 days. + /// + /// This is used primarily to detect the case + /// where the user just restored an old backup. + pub(crate) async fn member_list_is_stale(&self, context: &Context) -> Result { + let now = time(); + let member_list_ts = self.member_list_timestamp(context).await?; + let is_stale = now.saturating_add(TIMESTAMP_SENT_TOLERANCE) + >= member_list_ts.saturating_add(60 * 24 * 3600); + Ok(is_stale) + } + /// Adds missing values to the msg object, /// writes the record to the database and returns its msg_id. /// @@ -3468,6 +3497,7 @@ pub async fn get_chat_contacts(context: &Context, chat_id: ChatId) -> Result Result> { + let now = time(); let list = context .sql .query_map( @@ -3475,9 +3505,11 @@ pub async fn get_past_chat_contacts(context: &Context, chat_id: ChatId) -> Resul FROM chats_contacts cc LEFT JOIN contacts c ON c.id=cc.contact_id - WHERE cc.chat_id=? AND cc.add_timestamp < cc.remove_timestamp + WHERE cc.chat_id=? + AND cc.add_timestamp < cc.remove_timestamp + AND ? < cc.remove_timestamp ORDER BY c.id=1, c.last_seen DESC, c.id DESC", - (chat_id,), + (chat_id, now.saturating_sub(60 * 24 * 3600)), |row| row.get::<_, ContactId>(0), |ids| ids.collect::, _>>().map_err(Into::into), ) diff --git a/src/chat/chat_tests.rs b/src/chat/chat_tests.rs index fba4be118c..ac3cbc8d83 100644 --- a/src/chat/chat_tests.rs +++ b/src/chat/chat_tests.rs @@ -2,6 +2,7 @@ use super::*; use crate::chatlist::get_archived_cnt; use crate::constants::{DC_GCL_ARCHIVED_ONLY, DC_GCL_NO_SPECIALS}; use crate::headerdef::HeaderDef; +use crate::imex::{has_backup, imex, ImexMode}; use crate::message::{delete_msgs, MessengerMessage}; use crate::receive_imf::receive_imf; use crate::test_utils::{sync, TestContext, TestContextManager, TimeShiftFalsePositiveNote}; @@ -3365,3 +3366,160 @@ async fn unpromoted_group_no_tombstones() -> Result<()> { Ok(()) } + +/// Test that past members expire after 60 days. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_expire_past_members_after_60_days() -> Result<()> { + let mut tcm = TestContextManager::new(); + + let alice = &tcm.alice().await; + let fiona_addr = "fiona@example.net"; + let alice_fiona_contact_id = Contact::create(alice, "Fiona", fiona_addr).await?; + + let alice_chat_id = + create_group_chat(alice, ProtectionStatus::Unprotected, "Group chat").await?; + add_contact_to_chat(alice, alice_chat_id, alice_fiona_contact_id).await?; + alice + .send_text(alice_chat_id, "Hi! I created a group.") + .await; + remove_contact_from_chat(alice, alice_chat_id, alice_fiona_contact_id).await?; + assert_eq!(get_past_chat_contacts(alice, alice_chat_id).await?.len(), 1); + + SystemTime::shift(Duration::from_secs(60 * 24 * 60 * 60 + 1)); + assert_eq!(get_past_chat_contacts(alice, alice_chat_id).await?.len(), 0); + + let bob = &tcm.bob().await; + let bob_addr = bob.get_config(Config::Addr).await?.unwrap(); + let alice_bob_contact_id = Contact::create(alice, "Bob", &bob_addr).await?; + add_contact_to_chat(alice, alice_chat_id, alice_bob_contact_id).await?; + + let add_message = alice.pop_sent_msg().await; + assert_eq!(add_message.payload.contains(fiona_addr), false); + let bob_add_message = bob.recv_msg(&add_message).await; + let bob_chat_id = bob_add_message.chat_id; + assert_eq!(get_chat_contacts(bob, bob_chat_id).await?.len(), 2); + assert_eq!(get_past_chat_contacts(bob, bob_chat_id).await?.len(), 0); + + Ok(()) +} + +/// Test the case when Alice restores a backup older than 60 days +/// with outdated member list. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_restore_backup_after_60_days() -> Result<()> { + let backup_dir = tempfile::tempdir()?; + + let mut tcm = TestContextManager::new(); + + let alice = &tcm.alice().await; + let bob = &tcm.bob().await; + let fiona = &tcm.fiona().await; + + let bob_addr = bob.get_config(Config::Addr).await?.unwrap(); + let alice_bob_contact_id = Contact::create(alice, "Bob", &bob_addr).await?; + + let charlie_addr = "charlie@example.com"; + let alice_charlie_contact_id = Contact::create(alice, "Charlie", charlie_addr).await?; + + let alice_chat_id = + create_group_chat(alice, ProtectionStatus::Unprotected, "Group chat").await?; + add_contact_to_chat(alice, alice_chat_id, alice_bob_contact_id).await?; + add_contact_to_chat(alice, alice_chat_id, alice_charlie_contact_id).await?; + + let alice_sent_promote = alice + .send_text(alice_chat_id, "Hi! I created a group.") + .await; + let bob_rcvd_promote = bob.recv_msg(&alice_sent_promote).await; + let bob_chat_id = bob_rcvd_promote.chat_id; + bob_chat_id.accept(bob).await?; + + // Alice exports a backup. + imex(alice, ImexMode::ExportBackup, backup_dir.path(), None).await?; + + remove_contact_from_chat(alice, alice_chat_id, alice_charlie_contact_id).await?; + assert_eq!(get_chat_contacts(alice, alice_chat_id).await?.len(), 2); + assert_eq!(get_past_chat_contacts(alice, alice_chat_id).await?.len(), 1); + + let remove_message = alice.pop_sent_msg().await; + assert_eq!(remove_message.payload.contains(charlie_addr), true); + bob.recv_msg(&remove_message).await; + + // 60 days pass. + SystemTime::shift(Duration::from_secs(60 * 24 * 60 * 60 + 1)); + + assert_eq!(get_past_chat_contacts(alice, alice_chat_id).await?.len(), 0); + + // Bob adds Fiona to the chat. + let fiona_addr = fiona.get_config(Config::Addr).await?.unwrap(); + let bob_fiona_contact_id = Contact::create(bob, "Fiona", &fiona_addr).await?; + add_contact_to_chat(bob, bob_chat_id, bob_fiona_contact_id).await?; + + let add_message = bob.pop_sent_msg().await; + alice.recv_msg(&add_message).await; + let fiona_add_message = fiona.recv_msg(&add_message).await; + let fiona_chat_id = fiona_add_message.chat_id; + fiona_chat_id.accept(fiona).await?; + + // Fiona does not learn about Charlie, + // even from `Chat-Group-Past-Members`, because tombstone has expired. + assert_eq!(get_chat_contacts(fiona, fiona_chat_id).await?.len(), 3); + assert_eq!(get_past_chat_contacts(fiona, fiona_chat_id).await?.len(), 0); + + // Fiona sends a message + // so chat is not stale for Bob again. + // Alice also receives the message, + // but will import a backup immediately afterwards, + // so it does not matter. + let fiona_sent_message = fiona.send_text(fiona_chat_id, "Hi!").await; + alice.recv_msg(&fiona_sent_message).await; + bob.recv_msg(&fiona_sent_message).await; + + tcm.section("Alice imports old backup"); + let alice = &tcm.unconfigured().await; + let backup = has_backup(alice, backup_dir.path()).await?; + imex(alice, ImexMode::ImportBackup, backup.as_ref(), None).await?; + + // Alice thinks Charlie is in the chat, but does not know about Fiona. + assert_eq!(get_chat_contacts(alice, alice_chat_id).await?.len(), 3); + assert_eq!(get_past_chat_contacts(alice, alice_chat_id).await?.len(), 0); + + assert_eq!(get_chat_contacts(bob, bob_chat_id).await?.len(), 3); + assert_eq!(get_past_chat_contacts(bob, bob_chat_id).await?.len(), 0); + + assert_eq!(get_chat_contacts(fiona, fiona_chat_id).await?.len(), 3); + assert_eq!(get_past_chat_contacts(fiona, fiona_chat_id).await?.len(), 0); + + // Bob sends a text message to the chat, without a tombstone for Charlie. + // Alice learns about Fiona. + let bob_sent_text = bob.send_text(bob_chat_id, "Message.").await; + + tcm.section("Alice sends a message to stale chat"); + let alice_sent_text = alice + .send_text(alice_chat_id, "Hi! I just restored a backup.") + .await; + + tcm.section("Alice sent a message to stale chat"); + alice.recv_msg(&bob_sent_text).await; + fiona.recv_msg(&bob_sent_text).await; + + bob.recv_msg(&alice_sent_text).await; + fiona.recv_msg(&alice_sent_text).await; + + // Alice should have learned about Charlie not being part of the group + // by receiving Bob's message. + assert_eq!(get_chat_contacts(alice, alice_chat_id).await?.len(), 3); + assert!(!is_contact_in_chat(alice, alice_chat_id, alice_charlie_contact_id).await?); + assert_eq!(get_past_chat_contacts(alice, alice_chat_id).await?.len(), 0); + + // This should not add or restore Charlie for Bob and Fiona, + // Charlie is not part of the chat. + assert_eq!(get_chat_contacts(bob, bob_chat_id).await?.len(), 3); + assert_eq!(get_past_chat_contacts(bob, bob_chat_id).await?.len(), 0); + let bob_charlie_contact_id = Contact::create(bob, "Charlie", charlie_addr).await?; + assert!(!is_contact_in_chat(bob, bob_chat_id, bob_charlie_contact_id).await?); + + assert_eq!(get_chat_contacts(fiona, fiona_chat_id).await?.len(), 3); + assert_eq!(get_past_chat_contacts(fiona, fiona_chat_id).await?.len(), 0); + + Ok(()) +} diff --git a/src/mimefactory.rs b/src/mimefactory.rs index 9a2a525d2f..5e59834270 100644 --- a/src/mimefactory.rs +++ b/src/mimefactory.rs @@ -155,6 +155,7 @@ fn new_address_with_name(name: &str, address: String) -> Address { impl MimeFactory { pub async fn from_msg(context: &Context, msg: Message) -> Result { + let now = time(); let chat = Chat::load_from_db(context, msg.chat_id).await?; let attach_profile_data = Self::should_attach_profile_data(&msg); let undisclosed_recipients = chat.typ == Chattype::Broadcast; @@ -240,7 +241,7 @@ impl MimeFactory { } } recipient_ids.insert(id); - } else { + } else if remove_timestamp.saturating_add(60 * 24 * 3600) > now { // Row is a tombstone, // member is not actually part of the group. if !recipients_contain_addr(&past_members, &addr) { @@ -621,7 +622,13 @@ impl MimeFactory { ); } - if !self.member_timestamps.is_empty() { + let chat_memberlist_is_stale = if let Loaded::Message { chat, .. } = &self.loaded { + chat.member_list_is_stale(context).await? + } else { + false + }; + + if !self.member_timestamps.is_empty() && !chat_memberlist_is_stale { headers.push( Header::new_with_value( "Chat-Group-Member-Timestamps".into(), diff --git a/src/param.rs b/src/param.rs index e9255d69f4..9200357284 100644 --- a/src/param.rs +++ b/src/param.rs @@ -183,8 +183,6 @@ pub enum Param { GroupNameTimestamp = b'g', /// For Chats: timestamp of member list update. - /// - /// Deprecated 2025-01-07. MemberListTimestamp = b'k', /// For Webxdc Message Instances: Current document name diff --git a/src/receive_imf.rs b/src/receive_imf.rs index 51cb475a4e..01edd00759 100644 --- a/src/receive_imf.rs +++ b/src/receive_imf.rs @@ -2328,7 +2328,40 @@ async fn apply_group_changes( } if is_from_in_chat { - if let Some(ref chat_group_member_timestamps) = mime_parser.chat_group_member_timestamps() { + if chat.member_list_is_stale(context).await? { + info!(context, "Member list is stale."); + let mut new_members: HashSet = HashSet::from_iter(to_ids.iter().copied()); + new_members.insert(ContactId::SELF); + if !from_id.is_special() { + new_members.insert(from_id); + } + + context + .sql + .transaction(|transaction| { + // Remove all contacts and tombstones. + transaction.execute( + "DELETE FROM chats_contacts + WHERE chat_id=?", + (chat_id,), + )?; + + // Insert contacts with default timestamps of 0. + let mut statement = transaction.prepare( + "INSERT INTO chats_contacts (chat_id, contact_id) + VALUES (?, ?)", + )?; + for contact_id in &new_members { + statement.execute((chat_id, contact_id))?; + } + + Ok(()) + }) + .await?; + send_event_chat_modified = true; + } else if let Some(ref chat_group_member_timestamps) = + mime_parser.chat_group_member_timestamps() + { send_event_chat_modified |= update_chats_contacts_timestamps( context, chat_id, @@ -2378,6 +2411,14 @@ async fn apply_group_changes( send_event_chat_modified = true; } } + + chat_id + .update_timestamp( + context, + Param::MemberListTimestamp, + mime_parser.timestamp_sent, + ) + .await?; } let new_chat_contacts = HashSet::::from_iter( diff --git a/src/receive_imf/receive_imf_tests.rs b/src/receive_imf/receive_imf_tests.rs index 4f4e479e84..8e980ed285 100644 --- a/src/receive_imf/receive_imf_tests.rs +++ b/src/receive_imf/receive_imf_tests.rs @@ -4328,35 +4328,50 @@ async fn test_dont_readd_with_normal_msg() -> Result<()> { async fn test_mua_cant_remove() -> Result<()> { let alice = TestContext::new_alice().await; + let now = time(); + // Alice creates chat with 3 contacts + let date = chrono::DateTime::::from_timestamp(now - 2000, 0) + .unwrap() + .to_rfc2822(); let msg = receive_imf( &alice, - b"Subject: =?utf-8?q?Message_from_alice=40example=2Eorg?=\r\n\ - From: alice@example.org\r\n\ - To: , , \r\n\ - Date: Mon, 12 Dec 2022 14:30:39 +0000\r\n\ - Message-ID: \r\n\ - Chat-Version: 1.0\r\n\ - \r\n\ - tst\r\n", + format!( + "Subject: =?utf-8?q?Message_from_alice=40example=2Eorg?=\r\n\ + From: alice@example.org\r\n\ + To: , , \r\n\ + Date: {date}\r\n\ + Message-ID: \r\n\ + Chat-Version: 1.0\r\n\ + \r\n\ + tst\r\n" + ) + .as_bytes(), false, ) .await? .unwrap(); let alice_chat = Chat::load_from_db(&alice, msg.chat_id).await?; assert_eq!(alice_chat.typ, Chattype::Group); + assert_eq!(alice_chat.member_list_is_stale(&alice).await?, false); // Bob uses a classical MUA to answer, removing a recipient. + let date = chrono::DateTime::::from_timestamp(now - 1000, 0) + .unwrap() + .to_rfc2822(); let bob_removes = receive_imf( &alice, - b"Subject: Re: Message from alice\r\n\ - From: \r\n\ - To: , \r\n\ - Date: Mon, 12 Dec 2022 14:32:39 +0000\r\n\ - Message-ID: \r\n\ - In-Reply-To: \r\n\ - \r\n\ - Hi back!\r\n", + format!( + "Subject: Re: Message from alice\r\n\ + From: \r\n\ + To: , \r\n\ + Date: {date}\r\n\ + Message-ID: \r\n\ + In-Reply-To: \r\n\ + \r\n\ + Hi back!\r\n" + ) + .as_bytes(), false, ) .await? @@ -4364,22 +4379,29 @@ async fn test_mua_cant_remove() -> Result<()> { assert_eq!(bob_removes.chat_id, alice_chat.id); let group_chat = Chat::load_from_db(&alice, bob_removes.chat_id).await?; assert_eq!(group_chat.typ, Chattype::Group); + assert_eq!(group_chat.member_list_is_stale(&alice).await?, false); assert_eq!( chat::get_chat_contacts(&alice, group_chat.id).await?.len(), 4 ); // But if the parent message is missing, the message must goto a new ad-hoc group. + let date = chrono::DateTime::::from_timestamp(now, 0) + .unwrap() + .to_rfc2822(); let bob_removes = receive_imf( &alice, - b"Subject: Re: Message from alice\r\n\ - From: \r\n\ - To: , \r\n\ - Date: Mon, 12 Dec 2022 14:32:40 +0000\r\n\ - Message-ID: \r\n\ - In-Reply-To: \r\n\ - \r\n\ - Hi back!\r\n", + format!( + "Subject: Re: Message from alice\r\n\ + From: \r\n\ + To: , \r\n\ + Date: {date}\r\n\ + Message-ID: \r\n\ + In-Reply-To: \r\n\ + \r\n\ + Hi back!\r\n" + ) + .as_bytes(), false, ) .await? @@ -4398,39 +4420,51 @@ async fn test_mua_cant_remove() -> Result<()> { async fn test_mua_can_add() -> Result<()> { let alice = TestContext::new_alice().await; + let now = time(); + // Alice creates chat with 3 contacts + let date = chrono::DateTime::::from_timestamp(now - 2000, 0) + .unwrap() + .to_rfc2822(); let msg = receive_imf( &alice, - b"Subject: =?utf-8?q?Message_from_alice=40example=2Eorg?=\r\n\ - From: alice@example.org\r\n\ - To: , , \r\n\ - Date: Mon, 12 Dec 2022 14:30:39 +0000\r\n\ - Message-ID: \r\n\ - Chat-Version: 1.0\r\n\ - \r\n\ - Hi!\r\n", + format!( + "Subject: =?utf-8?q?Message_from_alice=40example=2Eorg?=\r\n\ + From: alice@example.org\r\n\ + To: , , \r\n\ + Date: {date}\r\n\ + Message-ID: \r\n\ + Chat-Version: 1.0\r\n\ + \r\n\ + Hi!\r\n" + ) + .as_bytes(), false, ) .await? .unwrap(); let alice_chat = Chat::load_from_db(&alice, msg.chat_id).await?; assert_eq!(alice_chat.typ, Chattype::Group); + assert_eq!(alice_chat.member_list_is_stale(&alice).await?, false); // Bob uses a classical MUA to answer, adding a recipient. + let date = chrono::DateTime::::from_timestamp(now - 1000, 0) + .unwrap() + .to_rfc2822(); let bob_adds = receive_imf( - &alice, - b"Subject: Re: Message from alice\r\n\ - From: \r\n\ - To: , , , \r\n\ - Date: Mon, 12 Dec 2022 14:32:39 +0000\r\n\ - Message-ID: \r\n\ - In-Reply-To: \r\n\ - \r\n\ - Hi back!\r\n", - false, - ) - .await? - .unwrap(); + &alice, + format!("Subject: Re: Message from alice\r\n\ + From: \r\n\ + To: , , , \r\n\ + Date: {date}\r\n\ + Message-ID: \r\n\ + In-Reply-To: \r\n\ + \r\n\ + Hi back!\r\n").as_bytes(), + false, + ) + .await? + .unwrap(); let group_chat = Chat::load_from_db(&alice, bob_adds.chat_id).await?; assert_eq!(group_chat.typ, Chattype::Group);