Page MenuHomePhorge
Files F85630488

D335.1786224135.diff
No OneTemporary
Actions

Size
15 KB
Referenced Files
None
Subscribers
None

D335.1786224135.diff
View Options

diff --git a/src/client/actions/encryption.cpp b/src/client/actions/encryption.cpp
--- a/src/client/actions/encryption.cpp
+++ b/src/client/actions/encryption.cpp
@@ -542,6 +542,19 @@
m.deviceLists.addDevice(userId, deviceId, deviceInfo);
}
if (wantedToFetchAllForUser(userId)) {
+ // We requested all devices for this user, so any
+ // device currently tracked but absent from the
+ // response has been deleted by its owner. Mark it
+ // as deleted rather than removing it, so we can still
+ // verify past decrypted events from that device.
+ auto existingDevices = m.deviceLists.devicesFor(userId);
+ for (auto [deviceId, info] : existingDevices) {
+ if (!deviceMap.count(deviceId)) {
+ kzo.client.dbg() << "Marking deleted device " << userId
+ << "/" << deviceId << std::endl;
+ m.deviceLists.markDeviceAsDeleted(userId, deviceId);
+ }
+ }
m.deviceLists.markUpToDate(userId);
}
}
diff --git a/src/client/client-model.cpp b/src/client/client-model.cpp
--- a/src/client/client-model.cpp
+++ b/src/client/client-model.cpp
@@ -289,7 +289,8 @@
// XXX: preliminary approach
auto shouldSendP = [=](auto deviceInfo, auto /* deviceMap */) {
- return deviceInfo.trustLevel >= trustLevelNeeded;
+ return deviceInfo.trustLevel >= trustLevelNeeded
+ && !deviceInfo.deleted;
};
diff --git a/src/client/device-list-tracker.hpp b/src/client/device-list-tracker.hpp
--- a/src/client/device-list-tracker.hpp
+++ b/src/client/device-list-tracker.hpp
@@ -38,12 +38,17 @@
std::string curve25519Key;
std::optional<std::string> displayName;
DeviceTrustLevel trustLevel{Unseen};
+ /// Whether this device has been deleted by its owner.
+ /// Deleted devices are kept in the tracker for verifying
+ /// past decrypted events, but are excluded from
+ /// devicesToSendKeys().
+ bool deleted{false};
friend bool operator==(const DeviceKeyInfo &a, const DeviceKeyInfo &b) = default;
friend bool operator!=(const DeviceKeyInfo &a, const DeviceKeyInfo &b) = default;
};
template<class Archive>
- void serialize(Archive &ar, DeviceKeyInfo &i, std::uint32_t const /*version*/)
+ void serialize(Archive &ar, DeviceKeyInfo &i, std::uint32_t const version)
{
ar
& i.deviceId
@@ -52,6 +57,11 @@
& i.displayName
& i.trustLevel
;
+ if (version >= 1) {
+ ar & i.deleted;
+ } else {
+ i.deleted = false;
+ }
}
struct DeviceListTracker
@@ -83,6 +93,11 @@
void addVerifiedDeviceKeyInfo(std::string userId, std::string deviceId, DeviceKeyInfo info);
+ /// Mark a device as deleted by its owner. The device is kept
+ /// in the tracker (for verifying past decrypted events) but
+ /// will be excluded from devicesToSendKeys().
+ void markDeviceAsDeleted(std::string userId, std::string deviceId);
+
void markUpToDate(std::string userId);
DeviceMapT devicesFor(std::string userId) const;
@@ -118,5 +133,5 @@
}
}
-BOOST_CLASS_VERSION(Kazv::DeviceKeyInfo, 0)
+BOOST_CLASS_VERSION(Kazv::DeviceKeyInfo, 1)
BOOST_CLASS_VERSION(Kazv::DeviceListTracker, 0)
diff --git a/src/client/device-list-tracker.cpp b/src/client/device-list-tracker.cpp
--- a/src/client/device-list-tracker.cpp
+++ b/src/client/device-list-tracker.cpp
@@ -27,6 +27,8 @@
b.displayName = std::nullopt;
a.trustLevel = Unseen;
b.trustLevel = Unseen;
+ a.deleted = false;
+ b.deleted = false;
return std::move(a) == std::move(b);
}
@@ -130,6 +132,16 @@
});
}
+ void DeviceListTracker::markDeviceAsDeleted(std::string userId, std::string deviceId)
+ {
+ deviceLists = std::move(deviceLists)
+ .update(userId, [=](auto deviceMap) {
+ auto info = deviceMap[deviceId];
+ info.deleted = true;
+ return std::move(deviceMap).set(deviceId, info);
+ });
+ }
+
void DeviceListTracker::markUpToDate(std::string userId)
{
usersToTrackDeviceLists = std::move(usersToTrackDeviceLists).set(userId, false);
diff --git a/src/tests/client/device-list-tracker-test.cpp b/src/tests/client/device-list-tracker-test.cpp
--- a/src/tests/client/device-list-tracker-test.cpp
+++ b/src/tests/client/device-list-tracker-test.cpp
@@ -15,6 +15,238 @@
using namespace Kazv;
using namespace Kazv::Factory;
+TEST_CASE("DeviceListTracker marks devices as deleted but keeps them", "[client][device-list-tracker]")
+{
+ DeviceListTracker tracker;
+ auto info1 = makeDeviceKeyInfo(withDeviceId("device1") | withDeviceTrustLevel(Verified));
+ auto info2 = makeDeviceKeyInfo(withDeviceId("device2") | withDeviceTrustLevel(Verified));
+ auto userId = "@user:example.com";
+
+ tracker.addVerifiedDeviceKeyInfo(userId, "device1", info1);
+ tracker.addVerifiedDeviceKeyInfo(userId, "device2", info2);
+
+ REQUIRE(tracker.devicesFor(userId).size() == 2);
+
+ // Mark device2 as deleted
+ tracker.markDeviceAsDeleted(userId, "device2");
+
+ // The device is still in the tracker (for verifying past events)
+ REQUIRE(tracker.devicesFor(userId).size() == 2);
+ REQUIRE(tracker.get(userId, "device2").has_value());
+
+ // But it is marked as deleted
+ REQUIRE(tracker.get(userId, "device2").value().deleted);
+ REQUIRE(!tracker.get(userId, "device1").value().deleted);
+
+ // findByCurve25519Key still finds the deleted device
+ REQUIRE(tracker.findByCurve25519Key(userId, info2.curve25519Key).has_value());
+ REQUIRE(tracker.findByCurve25519Key(userId, info2.curve25519Key).value().deleted);
+}
+
+TEST_CASE("devicesToSendKeys excludes deleted devices", "[client][device-list-tracker]")
+{
+ auto info1 = makeDeviceKeyInfo(withDeviceId("device1") | withDeviceTrustLevel(Verified));
+ auto info2 = makeDeviceKeyInfo(withDeviceId("device2") | withDeviceTrustLevel(Verified));
+ auto userId = "@user:example.com";
+
+ auto client = makeClient(
+ withDevice(userId, info1)
+ | withDevice(userId, info2)
+ );
+ client.trustLevelNeededToSendKeys = Verified;
+
+ REQUIRE(client.deviceLists.devicesFor(userId).size() == 2);
+ REQUIRE(client.devicesToSendKeys(userId).size() == 2);
+
+ // Mark device2 as deleted
+ client.deviceLists.markDeviceAsDeleted(userId, "device2");
+
+ // devicesToSendKeys should now exclude the deleted device
+ auto devices = client.devicesToSendKeys(userId);
+ REQUIRE(devices.size() == 1);
+ REQUIRE(devices[0] == "device1");
+}
+
+static json makeSignedDeviceInfo(const ClientModel &client)
+{
+ auto [next, _] = updateClient(client, UploadIdentityKeysAction{});
+ return json::parse(std::get<Bytes>(next.nextJobs[0].requestBody()))["device_keys"];
+}
+
+TEST_CASE("processResponse(QueryKeysResponse) marks deleted devices on full query", "[client][device-list-tracker]")
+{
+ // Set up two device identity cryptos for the same user
+ auto device1Crypto = makeCrypto();
+ auto device1Client = makeClient(withCrypto(device1Crypto));
+ device1Client.userId = "@alice:example.com";
+ device1Client.deviceId = "device1";
+
+ auto device2Crypto = makeCrypto();
+ auto device2Client = makeClient(withCrypto(device2Crypto));
+ device2Client.userId = "@alice:example.com";
+ device2Client.deviceId = "device2";
+
+ // Build a full key query response that contains BOTH devices
+ auto bothDevicesRespJson = json{
+ {"device_keys", {{"@alice:example.com", {
+ {"device1", makeSignedDeviceInfo(device1Client)},
+ {"device2", makeSignedDeviceInfo(device2Client)},
+ }}}},
+ };
+
+ // Our client tracking Alice's devices
+ auto client = makeClient(withCrypto(makeCrypto()));
+ client.userId = "@bob:example.com";
+ client.deviceId = "bobdevice";
+
+ // Process the response with both devices
+ std::tie(client, std::ignore) = processResponse(client, QueryKeysResponse(
+ makeResponse("QueryKeys",
+ withResponseJsonBody(bothDevicesRespJson)
+ | withResponseDataKV("deviceKeys",
+ json::object({{"@alice:example.com", json::array()}})))));
+
+ REQUIRE(client.deviceLists.devicesFor("@alice:example.com").size() == 2);
+ REQUIRE(client.deviceLists.get("@alice:example.com", "device1").has_value());
+ REQUIRE(client.deviceLists.get("@alice:example.com", "device2").has_value());
+ REQUIRE(!client.deviceLists.get("@alice:example.com", "device2").value().deleted);
+
+ // Now Alice deletes device2. We get a new full key query response
+ // that only contains device1.
+ auto onlyDevice1RespJson = json{
+ {"device_keys", {{"@alice:example.com", {
+ {"device1", makeSignedDeviceInfo(device1Client)},
+ }}}},
+ };
+
+ std::tie(client, std::ignore) = processResponse(client, QueryKeysResponse(
+ makeResponse("QueryKeys2",
+ withResponseJsonBody(onlyDevice1RespJson)
+ | withResponseDataKV("deviceKeys",
+ json::object({{"@alice:example.com", json::array()}})))));
+
+ // device2 is kept in the tracker (for verifying past events) but marked deleted
+ REQUIRE(client.deviceLists.devicesFor("@alice:example.com").size() == 2);
+ REQUIRE(client.deviceLists.get("@alice:example.com", "device2").has_value());
+ REQUIRE(client.deviceLists.get("@alice:example.com", "device2").value().deleted);
+
+ // device1 is not deleted
+ REQUIRE(!client.deviceLists.get("@alice:example.com", "device1").value().deleted);
+
+ // devicesToSendKeys excludes the deleted device
+ client.trustLevelNeededToSendKeys = Unseen;
+ auto devices = client.devicesToSendKeys("@alice:example.com");
+ REQUIRE(devices.size() == 1);
+ REQUIRE(devices[0] == "device1");
+}
+
+static ClientModel makeDeviceClient(std::string userId, std::string deviceId)
+{
+ auto client = makeClient(withCrypto(makeCrypto()));
+ client.userId = userId;
+ client.deviceId = deviceId;
+ return client;
+}
+
+TEST_CASE("mixed full and specific queries: full marks deleted, specific preserves unqueried", "[client][device-list-tracker]")
+{
+ // Create crypto identities for all devices of two users
+ auto aliceDev1 = makeDeviceClient("@alice:example.com", "device1");
+ auto aliceDev2 = makeDeviceClient("@alice:example.com", "device2");
+ auto aliceDev3 = makeDeviceClient("@alice:example.com", "device3");
+ auto bobDev1 = makeDeviceClient("@bob:example.com", "device1");
+ auto bobDev2 = makeDeviceClient("@bob:example.com", "device2");
+
+ // Our client tracking both Alice and Bob
+ auto client = makeClient(withCrypto(makeCrypto()));
+ client.userId = "@carol:example.com";
+ client.deviceId = "caroldevice";
+
+ // Process an initial full query that discovers all devices for both users
+ auto initialRespJson = json{
+ {"device_keys", {
+ {"@alice:example.com", {
+ {"device1", makeSignedDeviceInfo(aliceDev1)},
+ {"device2", makeSignedDeviceInfo(aliceDev2)},
+ {"device3", makeSignedDeviceInfo(aliceDev3)},
+ }},
+ {"@bob:example.com", {
+ {"device1", makeSignedDeviceInfo(bobDev1)},
+ {"device2", makeSignedDeviceInfo(bobDev2)},
+ }},
+ }},
+ };
+
+ std::tie(client, std::ignore) = processResponse(client, QueryKeysResponse(
+ makeResponse("QueryKeys",
+ withResponseJsonBody(initialRespJson)
+ | withResponseDataKV("deviceKeys",
+ json::object({
+ {"@alice:example.com", json::array()},
+ {"@bob:example.com", json::array()},
+ })))));
+
+ // All devices are now tracked
+ REQUIRE(client.deviceLists.devicesFor("@alice:example.com").size() == 3);
+ REQUIRE(client.deviceLists.devicesFor("@bob:example.com").size() == 2);
+
+ // Now Alice deletes device3. We do a mixed query in a single request:
+ // - Full query for Alice (empty list = all devices)
+ // -> response has only device1, device2 (device3 was deleted)
+ // - Specific query for Bob with ["device1"] only
+ // -> response has only device1 (device2 was not queried)
+ auto mixedRespJson = json{
+ {"device_keys", {
+ {"@alice:example.com", {
+ {"device1", makeSignedDeviceInfo(aliceDev1)},
+ {"device2", makeSignedDeviceInfo(aliceDev2)},
+ // device3 deleted, not in response
+ }},
+ {"@bob:example.com", {
+ {"device1", makeSignedDeviceInfo(bobDev1)},
+ // device2 not queried, not in response
+ }},
+ }},
+ };
+
+ std::tie(client, std::ignore) = processResponse(client, QueryKeysResponse(
+ makeResponse("QueryKeys2",
+ withResponseJsonBody(mixedRespJson)
+ | withResponseDataKV("deviceKeys",
+ json::object({
+ {"@alice:example.com", json::array()}, // full query: all devices
+ {"@bob:example.com", json::array({"device1"})}, // specific query: device1 only
+ })))));
+
+ // For Alice (full query): device3 was deleted and NOT in the response.
+ // device3 is kept in the tracker (for verifying past events) but marked
+ // as deleted.
+ REQUIRE(client.deviceLists.devicesFor("@alice:example.com").size() == 3);
+ REQUIRE(client.deviceLists.get("@alice:example.com", "device3").has_value());
+ REQUIRE(client.deviceLists.get("@alice:example.com", "device3").value().deleted);
+ REQUIRE(!client.deviceLists.get("@alice:example.com", "device1").value().deleted);
+ REQUIRE(!client.deviceLists.get("@alice:example.com", "device2").value().deleted);
+
+ // For Bob (specific query for device1 only): device2 was NOT in the
+ // request and NOT in the response. Since we did not query for all
+ // of Bob's devices, absence from the response does not mean deletion.
+ // device2 must be preserved and NOT marked as deleted.
+ REQUIRE(client.deviceLists.devicesFor("@bob:example.com").size() == 2);
+ REQUIRE(client.deviceLists.get("@bob:example.com", "device2").has_value());
+ REQUIRE(!client.deviceLists.get("@bob:example.com", "device2").value().deleted);
+ REQUIRE(client.deviceLists.get("@bob:example.com", "device1").has_value());
+ REQUIRE(!client.deviceLists.get("@bob:example.com", "device1").value().deleted);
+
+ // devicesToSendKeys excludes Alice's deleted device3
+ client.trustLevelNeededToSendKeys = Unseen;
+ auto aliceDevices = client.devicesToSendKeys("@alice:example.com");
+ REQUIRE(aliceDevices.size() == 2);
+
+ // Bob's devicesToSendKeys correctly includes both devices
+ auto bobDevices = client.devicesToSendKeys("@bob:example.com");
+ REQUIRE(bobDevices.size() == 2);
+}
+
TEST_CASE("DeviceListTracker::findByCurve25519Key")
{
auto info = makeDeviceKeyInfo(withDeviceId("device1"));

File Metadata

Mime Type
text/plain
Expires
Sat, Aug 8, 2:22 PM (8 h, 11 m)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
1724659
Default Alt Text
D335.1786224135.diff (15 KB)

Event Timeline