diff --git a/src/chat.rs b/src/chat.rs index 1752e54a6..0ec812f00 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -4795,12 +4795,14 @@ pub(crate) async fn get_chat_id_by_grpid( /// /// Optional `label` can be provided to ensure that message is added only once. /// If `important` is true, a notification will be sent. +/// `timestamp_sent` is the time shown on the message; it does not affect ordering. #[expect(clippy::arithmetic_side_effects)] pub async fn add_device_msg_with_importance( context: &Context, label: Option<&str>, msg: Option<&mut Message>, important: bool, + timestamp_sent: i64, ) -> Result { ensure!( label.is_some() || msg.is_some(), @@ -4820,11 +4822,10 @@ pub async fn add_device_msg_with_importance( chat_id = ChatId::get_for_contact(context, ContactId::DEVICE).await?; let rfc724_mid = create_outgoing_rfc724_mid(); - let timestamp_sent = time(); // makes sure, the added message is the last one, // even if the date is wrong (useful esp. when warning about bad dates) - msg.timestamp_sort = timestamp_sent; + msg.timestamp_sort = time(); if let Some(last_msg_time) = chat_id.get_timestamp(context).await? && msg.timestamp_sort <= last_msg_time { @@ -4892,7 +4893,7 @@ pub async fn add_device_msg( label: Option<&str>, msg: Option<&mut Message>, ) -> Result { - add_device_msg_with_importance(context, label, msg, false).await + add_device_msg_with_importance(context, label, msg, false, time()).await } /// Returns true if device message with a given label was ever added to the device chat. @@ -4920,6 +4921,12 @@ pub(crate) async fn delete_and_reset_all_device_msgs(context: &Context) -> Resul .execute("DELETE FROM msgs WHERE from_id=?;", (ContactId::DEVICE,)) .await?; context.sql.execute("DELETE FROM devmsglabels;", ()).await?; + context + .set_config_internal(Config::BackupTransferMsgId, None) + .await?; + context + .set_config_internal(Config::BackupTransferTimestamp, None) + .await?; // Insert labels for welcome messages to avoid them being re-added on reconfiguration. context diff --git a/src/config.rs b/src/config.rs index df8711622..dd08cc49f 100644 --- a/src/config.rs +++ b/src/config.rs @@ -336,6 +336,17 @@ pub enum Config { /// Timestamp of the last time housekeeping was run LastHousekeeping, + /// ID of the device message added after backup transfer. + /// + /// If the message does not exist on the first housekeeping + /// after `READD_BACKUP_TRANSFER_MSG_DELAY`, it is re-added. + BackupTransferMsgId, + + /// Timestamp of the device message added after backup transfer. + /// + /// Needed to check against `READD_BACKUP_TRANSFER_MSG_DELAY`. + BackupTransferTimestamp, + /// Timestamp of the last time accumulated broadcast channel reactions were sent LastReactionsBroadcast, diff --git a/src/context/context_tests.rs b/src/context/context_tests.rs index fb955c584..2906db9c7 100644 --- a/src/context/context_tests.rs +++ b/src/context/context_tests.rs @@ -294,6 +294,8 @@ async fn test_get_info_completeness() { // too sensitive or summarized in another item. let skip_from_get_info = vec![ "addr", + "backup_transfer_msg_id", + "backup_transfer_timestamp", "displayname", "imap_certificate_checks", "mail_server", diff --git a/src/imex.rs b/src/imex.rs index 47a9f9bd3..00afc432c 100644 --- a/src/imex.rs +++ b/src/imex.rs @@ -30,6 +30,7 @@ use crate::tools::{ mod transfer; use ::pgp::types::KeyDetails; +pub(crate) use transfer::maybe_readd_backup_transfer_msg; pub use transfer::{BackupProvider, get_backup}; // Name of the database file in the backup. diff --git a/src/imex/transfer.rs b/src/imex/transfer.rs index be598b86d..0a86a5791 100644 --- a/src/imex/transfer.rs +++ b/src/imex/transfer.rs @@ -40,21 +40,68 @@ use tokio::task::JoinHandle; use tokio_util::sync::CancellationToken; use crate::EventType; -use crate::chat::add_device_msg; +use crate::chat::add_device_msg_with_importance; +use crate::config::Config; use crate::context::Context; use crate::imex::BlobDirContents; use crate::key; use crate::log::warn; -use crate::message::Message; +use crate::message::{Message, MsgId}; use crate::qr::Qr; use crate::stock_str::backup_transfer_msg_body; -use crate::tools::{TempPathGuard, create_id}; +use crate::tools::{TempPathGuard, create_id, time}; use super::{DBFILE_BACKUP_NAME, export_backup_stream, export_database, import_backup_stream}; /// ALPN protocol identifier for the backup transfer protocol. const BACKUP_ALPN: &[u8] = b"/deltachat/backup"; +/// Minimum time after a backup transfer before we check +/// whether the "second device added" device message still exists. +/// +/// The check is done in the first housekeeping after this delay; +/// if the message was deleted until then, it is re-added with the original timestamp. +/// After the check, the user can delete the message as usual without it being re-added. +/// +/// This makes it easier to spot unwanted "add second device" actions. +/// It is clear that this does not catch all eventualities, it is best-effort. +/// First line of defense is device locking and asking for secret explicitly before adding a second device. +const READD_BACKUP_TRANSFER_MSG_DELAY: i64 = 60 * 60; + +/// Checks `Config::BackupTransferMsgId` +/// and re-adds the backup transfer device message if it does not exist and some time passed. +pub(crate) async fn maybe_readd_backup_transfer_msg(context: &Context) -> Result<()> { + let Some(msg_id) = context + .get_config_parsed::(Config::BackupTransferMsgId) + .await? + else { + return Ok(()); + }; + + let timestamp = context + .get_config_i64(Config::BackupTransferTimestamp) + .await?; + if time() < timestamp.saturating_add(READD_BACKUP_TRANSFER_MSG_DELAY) { + return Ok(()); + } + + context + .set_config_internal(Config::BackupTransferMsgId, None) + .await?; + context + .set_config_internal(Config::BackupTransferTimestamp, None) + .await?; + if Message::load_from_db_optional(context, MsgId::new(msg_id)) + .await? + .is_none() + { + let mut msg = Message::new_text(backup_transfer_msg_body(context)); + add_device_msg_with_importance(context, None, Some(&mut msg), false, timestamp).await?; + } + + Ok(()) +} + /// Provide or send a backup of this device. /// /// This creates a backup of the current device and starts a service which offers another @@ -212,8 +259,23 @@ impl BackupProvider { info!(context, "Received backup reception acknowledgement."); context.emit_event(EventType::ImexProgress(1000)); + let timestamp = time(); let mut msg = Message::new_text(backup_transfer_msg_body(&context)); - add_device_msg(&context, None, Some(&mut msg)).await?; + let msg_id = + add_device_msg_with_importance(&context, None, Some(&mut msg), false, timestamp) + .await?; + context + .set_config_internal( + Config::BackupTransferTimestamp, + Some(×tamp.to_string()), + ) + .await?; + context + .set_config_internal( + Config::BackupTransferMsgId, + Some(&msg_id.to_u32().to_string()), + ) + .await?; Ok(()) } @@ -393,14 +455,17 @@ pub async fn get_backup(context: &Context, qr: Qr) -> Result<()> { mod tests { use std::time::Duration; - use crate::chat::{ChatItem, get_chat_msgs, send_msg}; - use crate::message::Viewtype; + use crate::chat::{ChatId, ChatItem, get_chat_msgs, send_msg}; + use crate::contact::ContactId; + use crate::message::{Viewtype, delete_msgs}; + use crate::sql::housekeeping; use crate::test_utils::TestContextManager; + use crate::tools::SystemTime; use super::*; #[tokio::test(flavor = "multi_thread", worker_threads = 2)] - async fn test_send_receive() { + async fn test_send_receive() -> Result<()> { let mut tcm = TestContextManager::new(); // Create first device. @@ -472,6 +537,31 @@ mod tests { .get_matching(|ev| matches!(ev, EventType::ImexProgress(1000))) .await; } + + // When deleting the backup transfer message on the sending device, + // it is re-added once on housekeeping, + // but not within the first hour after the transfer. + let device_chat_id = ChatId::get_for_contact(&ctx0, ContactId::DEVICE).await?; + let original_msg = ctx0.get_last_msg_in(device_chat_id).await; + assert_eq!(original_msg.text, backup_transfer_msg_body(&ctx0)); + delete_msgs(&ctx0, &[original_msg.id]).await?; + housekeeping(&ctx0).await?; + assert!(get_chat_msgs(&ctx0, device_chat_id).await?.is_empty()); // re-adding not done in first hour + + SystemTime::shift(Duration::from_secs(60 * 60)); + housekeeping(&ctx0).await?; + let readded_msg = ctx0.get_last_msg_in(device_chat_id).await; + assert_ne!(readded_msg.id, original_msg.id); + assert_eq!(readded_msg.get_text(), original_msg.get_text()); + assert_eq!(readded_msg.get_timestamp(), original_msg.get_timestamp()); + assert!(readded_msg.get_sort_timestamp() > original_msg.get_sort_timestamp()); + + delete_msgs(&ctx0, &[readded_msg.id]).await?; + housekeeping(&ctx0).await?; + let device_msgs = get_chat_msgs(&ctx0, device_chat_id).await?; + assert!(device_msgs.is_empty()); // re-adding is done only once + + Ok(()) } /// Tests that trying to accidentally overwrite a profile diff --git a/src/sql.rs b/src/sql.rs index ff1e722e7..89c7d881e 100644 --- a/src/sql.rs +++ b/src/sql.rs @@ -14,7 +14,7 @@ use crate::config::Config; use crate::context::Context; use crate::debug_logging::set_debug_logging_xdc; use crate::ephemeral::start_ephemeral_timers; -use crate::imex::BLOBS_BACKUP_NAME; +use crate::imex::{self, BLOBS_BACKUP_NAME}; use crate::location; use crate::log::{LogExt, warn}; use crate::message::MsgId; @@ -827,6 +827,12 @@ pub async fn housekeeping(context: &Context) -> Result<()> { ); } + imex::maybe_readd_backup_transfer_msg(context) + .await + .context("Failed to re-add backup transfer message") + .log_err(context) + .ok(); + if let Err(err) = incremental_vacuum(context).await { warn!(context, "Failed to run incremental vacuum: {err:#}."); } diff --git a/src/tools.rs b/src/tools.rs index f4050a49f..381e1b6de 100644 --- a/src/tools.rs +++ b/src/tools.rs @@ -220,6 +220,7 @@ async fn maybe_warn_on_bad_time(context: &Context, now: i64, known_past_timestam ), Some(&mut msg), true, + time(), ) .await .ok();