From dc17f2692cf6877d0702667a90a4c5bb64819d95 Mon Sep 17 00:00:00 2001 From: Hocuri Date: Mon, 17 Mar 2025 18:12:35 +0100 Subject: [PATCH] fix: Fix setting up a profile and immediately transferring to a second device (#6657) Found and fixed a bug while investigating https://github.com/chatmail/core/issues/6656. It's not the same bug, though. Steps to reproduce this bug: - Create a new profile - Transfer it to a second device - Send a message from the first device - -> It will never arrive on the second device, instead a warning will be printed that you are using DC on multiple devices. The bug was that the key wasn't created before the backup transfer, so that the second device then created its own key instead of using the same key as the first device. In order to regression-test, this PR now changes `clone()` to use "Add second device" instead of exporting and importing a backup. Exporting and importing a backup has enough tests already. This PR also adds an unrelated test `test_selfavatar_sync()`. The bug was introduced by https://github.com/chatmail/core/pull/6574 in v1.156.0 --- .../src/deltachat_rpc_client/account.py | 25 +++++----- .../src/deltachat_rpc_client/pytestplugin.py | 48 +++++++++++++++++++ deltachat-rpc-client/tests/test_something.py | 25 ++++++++++ src/imex/transfer.rs | 7 ++- 4 files changed, 92 insertions(+), 13 deletions(-) diff --git a/deltachat-rpc-client/src/deltachat_rpc_client/account.py b/deltachat-rpc-client/src/deltachat_rpc_client/account.py index 861e3a536..11dbe8f06 100644 --- a/deltachat-rpc-client/src/deltachat_rpc_client/account.py +++ b/deltachat-rpc-client/src/deltachat_rpc_client/account.py @@ -1,8 +1,6 @@ from __future__ import annotations from dataclasses import dataclass -from pathlib import Path -from tempfile import TemporaryDirectory from typing import TYPE_CHECKING, Optional, Union from warnings import warn @@ -28,9 +26,12 @@ class Account: def _rpc(self) -> "Rpc": return self.manager.rpc - def wait_for_event(self) -> AttrDict: + def wait_for_event(self, event_type=None) -> AttrDict: """Wait until the next event and return it.""" - return AttrDict(self._rpc.wait_for_event(self.id)) + while True: + next_event = AttrDict(self._rpc.wait_for_event(self.id)) + if event_type is None or next_event.kind == event_type: + return next_event def clear_all_events(self): """Removes all queued-up events for a given account. Useful for tests.""" @@ -41,14 +42,14 @@ class Account: self._rpc.remove_account(self.id) def clone(self) -> "Account": - """Clone given account.""" - with TemporaryDirectory() as tmp_dir: - tmp_path = Path(tmp_dir) - self.export_backup(tmp_path) - files = list(tmp_path.glob("*.tar")) - new_account = self.manager.add_account() - new_account.import_backup(files[0]) - return new_account + """Clone given account. + This uses backup-transfer via iroh, i.e. the 'Add second device' feature.""" + future = self._rpc.provide_backup.future(self.id) + qr = self._rpc.get_backup_qr(self.id) + new_account = self.manager.add_account() + new_account._rpc.get_backup(new_account.id, qr) + future() + return new_account def start_io(self) -> None: """Start the account I/O.""" diff --git a/deltachat-rpc-client/src/deltachat_rpc_client/pytestplugin.py b/deltachat-rpc-client/src/deltachat_rpc_client/pytestplugin.py index 63f42b1b3..777657560 100644 --- a/deltachat-rpc-client/src/deltachat_rpc_client/pytestplugin.py +++ b/deltachat-rpc-client/src/deltachat_rpc_client/pytestplugin.py @@ -4,6 +4,7 @@ import os import random from typing import AsyncGenerator, Optional +import py import pytest from . import Account, AttrDict, Bot, Chat, Client, DeltaChat, EventType, Message @@ -124,3 +125,50 @@ def rpc(tmp_path) -> AsyncGenerator: @pytest.fixture def acfactory(rpc) -> AsyncGenerator: return ACFactory(DeltaChat(rpc)) + + +@pytest.fixture +def data(): + """Test data.""" + + class Data: + def __init__(self) -> None: + for path in reversed(py.path.local(__file__).parts()): + datadir = path.join("test-data") + if datadir.isdir(): + self.path = datadir + return + raise Exception("Data path cannot be found") + + def get_path(self, bn): + """return path of file or None if it doesn't exist.""" + fn = os.path.join(self.path, *bn.split("/")) + assert os.path.exists(fn) + return fn + + def read_path(self, bn, mode="r"): + fn = self.get_path(bn) + if fn is not None: + with open(fn, mode) as f: + return f.read() + return None + + return Data() + + +@pytest.fixture +def log(): + """Log printer fixture.""" + + class Printer: + def section(self, msg: str) -> None: + print() + print("=" * 10, msg, "=" * 10) + + def step(self, msg: str) -> None: + print("-" * 5, "step " + msg, "-" * 5) + + def indent(self, msg: str) -> None: + print(" " + msg) + + return Printer() diff --git a/deltachat-rpc-client/tests/test_something.py b/deltachat-rpc-client/tests/test_something.py index d4969b9cc..177ecf19d 100644 --- a/deltachat-rpc-client/tests/test_something.py +++ b/deltachat-rpc-client/tests/test_something.py @@ -287,6 +287,31 @@ def test_message(acfactory) -> None: assert reactions == snapshot.reactions +def test_selfavatar_sync(acfactory, data, log) -> None: + alice = acfactory.get_online_account() + + log.section("Alice adds a second device") + alice2 = alice.clone() + + log.section("Second device goes online") + alice2.start_io() + + log.section("First device changes avatar") + image = data.get_path("image/avatar1000x1000.jpg") + alice.set_config("selfavatar", image) + avatar_config = alice.get_config("selfavatar") + avatar_hash = os.path.basename(avatar_config) + print("Info: avatar hash is ", avatar_hash) + + log.section("First device receives avatar change") + alice2.wait_for_event(EventType.SELFAVATAR_CHANGED) + avatar_config2 = alice2.get_config("selfavatar") + avatar_hash2 = os.path.basename(avatar_config2) + print("Info: avatar hash on second device is ", avatar_hash2) + assert avatar_hash == avatar_hash2 + assert avatar_config != avatar_config2 + + def test_reaction_seen_on_another_dev(acfactory) -> None: alice, bob = acfactory.get_online_accounts(2) alice2 = alice.clone() diff --git a/src/imex/transfer.rs b/src/imex/transfer.rs index 5295ced21..1d4f8d596 100644 --- a/src/imex/transfer.rs +++ b/src/imex/transfer.rs @@ -45,7 +45,7 @@ use crate::message::Message; use crate::qr::Qr; use crate::stock_str::backup_transfer_msg_body; use crate::tools::{create_id, time, TempPathGuard}; -use crate::EventType; +use crate::{e2ee, EventType}; use super::{export_backup_stream, export_database, import_backup_stream, DBFILE_BACKUP_NAME}; @@ -109,6 +109,11 @@ impl BackupProvider { .parent() .context("Context dir not found")?; + // before we export, make sure the private key exists + e2ee::ensure_secret_key_exists(context) + .await + .context("Cannot create private key or private key not available")?; + let dbfile = context_dir.join(DBFILE_BACKUP_NAME); if fs::metadata(&dbfile).await.is_ok() { fs::remove_file(&dbfile).await?;