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
This commit is contained in:
Hocuri
2025-03-17 18:12:35 +01:00
committed by GitHub
parent 94187f7ee1
commit dc17f2692c
4 changed files with 92 additions and 13 deletions

View File

@@ -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."""

View File

@@ -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()

View File

@@ -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()

View File

@@ -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?;