Skip to content

Commit

Permalink
feat: expire past members after 60 days
Browse files Browse the repository at this point in the history
test: add failing test

fixup the test

fix: use member list timestamp

Improve the test

move member_list_timestamp() from chat id to chat

New member_list_is_stale function

apply member list from To if member list is stale

remove tombstones when updating stale member list

fixup the test

test non-stale member list with MUA

clippy

formatting fixes
  • Loading branch information
link2xt committed Jan 22, 2025
1 parent 8f58c47 commit e55171e
Show file tree
Hide file tree
Showing 6 changed files with 322 additions and 52 deletions.
36 changes: 34 additions & 2 deletions src/chat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -1962,6 +1963,34 @@ impl Chat {
}
}

/// Returns chat member list timestamp.
pub(crate) async fn member_list_timestamp(&self, context: &Context) -> Result<i64> {
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<bool> {
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.
///
Expand Down Expand Up @@ -3468,16 +3497,19 @@ pub async fn get_chat_contacts(context: &Context, chat_id: ChatId) -> Result<Vec

/// Returns a vector of contact IDs for given chat ID that are no longer part of the group.
pub async fn get_past_chat_contacts(context: &Context, chat_id: ChatId) -> Result<Vec<ContactId>> {
let now = time();
let list = context
.sql
.query_map(
"SELECT cc.contact_id
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::<Result<Vec<_>, _>>().map_err(Into::into),
)
Expand Down
158 changes: 158 additions & 0 deletions src/chat/chat_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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(())
}
11 changes: 9 additions & 2 deletions src/mimefactory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<MimeFactory> {
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;
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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(),
Expand Down
2 changes: 0 additions & 2 deletions src/param.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
43 changes: 42 additions & 1 deletion src/receive_imf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<ContactId> = 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,
Expand Down Expand Up @@ -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::<ContactId>::from_iter(
Expand Down
Loading

0 comments on commit e55171e

Please sign in to comment.