diff --git a/src/jamidht/conversation.cpp b/src/jamidht/conversation.cpp
index 3c4a6711907228a1a864b581382dbde7b1fed284..fad842a02d2e59a4ed0aeaaf46e18fa9b25e0806 100644
--- a/src/jamidht/conversation.cpp
+++ b/src/jamidht/conversation.cpp
@@ -557,6 +557,13 @@ public:
gitSocketList_.erase(deviceSockets);
}
+ /**
+ * Remove all git sockets and all DRT nodes associated with the given peer.
+ * This is used when a swarm member is banned to ensure that we stop syncing
+ * with them or sending them message notifications.
+ */
+ void disconnectFromPeer(const std::string& peerUri);
+
std::vector<std::map<std::string, std::string>> getMembers(bool includeInvited,
bool includeLeft) const;
@@ -674,6 +681,24 @@ Conversation::Impl::isAdmin() const
return std::filesystem::is_regular_file(fileutils::getFullPath(adminsPath, userId_ + ".crt"));
}
+void
+Conversation::Impl::disconnectFromPeer(const std::string& peerUri)
+{
+ // Remove nodes from swarmManager
+ const auto nodes = swarmManager_->getRoutingTable().getAllNodes();
+ std::vector<NodeId> toRemove;
+ for (const auto node : nodes)
+ if (peerUri == repository_->uriFromDevice(node.toString()))
+ toRemove.emplace_back(node);
+ swarmManager_->deleteNode(toRemove);
+
+ // Remove git sockets with this member
+ std::vector<DeviceId> gitToRm;
+ for (const auto& [deviceId, _] : gitSocketList_)
+ if (peerUri == repository_->uriFromDevice(deviceId.toString()))
+ removeGitSocket(deviceId);
+}
+
std::vector<std::map<std::string, std::string>>
Conversation::Impl::getMembers(bool includeInvited, bool includeLeft, bool includeBanned) const
{
@@ -1370,21 +1395,7 @@ Conversation::removeMember(const std::string& contactUri, bool isDevice, const O
JAMI_WARN("Vote solved for %s. %s banned",
contactUri.c_str(),
isDevice ? "Device" : "Member");
-
- const auto nodes = sthis->pimpl_->swarmManager_->getRoutingTable().getAllNodes();
- // Remove nodes from swarmManager
- std::vector<NodeId> toRemove;
- for (const auto node : nodes)
- if (contactUri == sthis->uriFromDevice(node.toString()))
- toRemove.emplace_back(node);
- sthis->pimpl_->swarmManager_->deleteNode(toRemove);
- // Remove git sockets with this member
- std::vector<DeviceId> gitToRm;
- for (const auto& [deviceId, _] : sthis->pimpl_->gitSocketList_)
- if (contactUri == sthis->uriFromDevice(deviceId.toString()))
- gitToRm.emplace_back(deviceId);
- for (const auto& did : gitToRm)
- sthis->removeGitSocket(did);
+ sthis->pimpl_->disconnectFromPeer(contactUri);
}
sthis->pimpl_->announce(commits, true);
@@ -1652,10 +1663,13 @@ Conversation::Impl::mergeHistory(const std::string& uri)
JAMI_DEBUG("Successfully merge history with {:s}", uri);
auto result = repository_->convCommitsToMap(newCommits);
- for (const auto& commit : result) {
+ for (auto& commit : result) {
auto it = commit.find("type");
if (it != commit.end() && it->second == "member") {
repository_->refreshMembers();
+
+ if (commit["action"] == "ban")
+ disconnectFromPeer(commit["uri"]);
}
}
return result;
@@ -2269,8 +2283,10 @@ Conversation::bootstrap(std::function<void()> onBootstraped,
// If it works, the callback onConnectionChanged will be called with ok=true
pimpl_->bootstrapCb_ = std::move(onBootstraped);
std::vector<DeviceId> devices = knownDevices;
- for (const auto& m : pimpl_->repository_->devices())
- devices.insert(devices.end(), m.second.begin(), m.second.end());
+ for (const auto& [member, memberDevices] : pimpl_->repository_->devices()) {
+ if (!isBanned(member))
+ devices.insert(devices.end(), memberDevices.begin(), memberDevices.end());
+ }
JAMI_DEBUG("{}[SwarmManager {}] Bootstrap with {} devices",
pimpl_->toString(),
fmt::ptr(pimpl_->swarmManager_),
diff --git a/test/unitTest/conversation/conversationMembersEvent.cpp b/test/unitTest/conversation/conversationMembersEvent.cpp
index 7865729c6a25a825dbd4430398daa2020876bda7..95e5d20a07f42230bd215d610312712593f92a2f 100644
--- a/test/unitTest/conversation/conversationMembersEvent.cpp
+++ b/test/unitTest/conversation/conversationMembersEvent.cpp
@@ -82,7 +82,8 @@ public:
void testAddAcceptOfflineThenConnects();
void testGetMembers();
void testRemoveMember();
- void testRemovedMemberDoesNotReceiveMessage();
+ void testRemovedMemberDoesNotReceiveMessageFromAdmin();
+ void testRemovedMemberDoesNotReceiveMessageFromPeer();
void testRemoveInvitedMember();
void testMemberBanNoBadFile();
void testMemberTryToRemoveAdmin();
@@ -135,7 +136,8 @@ private:
CPPUNIT_TEST(testAddAcceptOfflineThenConnects);
CPPUNIT_TEST(testGetMembers);
CPPUNIT_TEST(testRemoveMember);
- CPPUNIT_TEST(testRemovedMemberDoesNotReceiveMessage);
+ CPPUNIT_TEST(testRemovedMemberDoesNotReceiveMessageFromAdmin);
+ CPPUNIT_TEST(testRemovedMemberDoesNotReceiveMessageFromPeer);
CPPUNIT_TEST(testRemoveInvitedMember);
CPPUNIT_TEST(testMemberBanNoBadFile);
CPPUNIT_TEST(testMemberTryToRemoveAdmin);
@@ -698,7 +700,7 @@ ConversationMembersEventTest::testRemoveMember()
}
void
-ConversationMembersEventTest::testRemovedMemberDoesNotReceiveMessage()
+ConversationMembersEventTest::testRemovedMemberDoesNotReceiveMessageFromAdmin()
{
connectSignals();
@@ -712,7 +714,7 @@ ConversationMembersEventTest::testRemovedMemberDoesNotReceiveMessage()
libjami::acceptConversationRequest(bobId, convId);
CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return aliceMsgSize + 1 == aliceData.messages.size(); }));
- // Now check that alice, has the only admin, can remove bob
+ // Now check that alice, as the only admin, can remove bob
libjami::removeConversationMember(aliceId, convId, bobUri);
CPPUNIT_ASSERT(
cv.wait_for(lk, 30s, [&]() { return aliceData.messages.size() == aliceMsgSize + 3 /* vote + ban */; }));
@@ -723,6 +725,61 @@ ConversationMembersEventTest::testRemovedMemberDoesNotReceiveMessage()
CPPUNIT_ASSERT(!cv.wait_for(lk, 10s, [&]() { return bobMsgSize + 1 == bobData.messages.size(); }));
}
+void
+ConversationMembersEventTest::testRemovedMemberDoesNotReceiveMessageFromPeer()
+{
+ connectSignals();
+
+ auto bobAccount = Manager::instance().getAccount<JamiAccount>(bobId);
+ auto bobUri = bobAccount->getUsername();
+ auto carlaAccount = Manager::instance().getAccount<JamiAccount>(carlaId);
+ auto carlaUri = carlaAccount->getUsername();
+ Manager::instance().sendRegister(carlaId, true);
+ CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return carlaData.deviceAnnounced; }));
+
+ // Make Carla send a contact request to Bob and wait until she receives a message from
+ // him confirming that he has accepted the request. The point of this is to make sure
+ // that Bob and Carla are connected; otherwise the test below where we check that Bob
+ // hasn't received a message from Carla could pass for the wrong reason.
+ carlaAccount->addContact(bobUri);
+ carlaAccount->sendTrustRequest(bobUri, {});
+ CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return bobData.requestReceived; }));
+ auto carlaMsgSize = carlaData.messages.size();
+ CPPUNIT_ASSERT(bobAccount->acceptTrustRequest(carlaUri));
+ CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return carlaMsgSize + 1 == carlaData.messages.size(); }));
+
+ // Alice creates a swarm and adds Bob and Carla to it
+ bobData.requestReceived = false;
+ auto convId = libjami::startConversation(aliceId);
+ libjami::addConversationMember(aliceId, convId, bobUri);
+ libjami::addConversationMember(aliceId, convId, carlaUri);
+ CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return bobData.requestReceived && carlaData.requestReceived; }));
+
+ libjami::acceptConversationRequest(bobId, convId);
+ libjami::acceptConversationRequest(carlaId, convId);
+ auto messageReceived = [](UserData& userData, const std::string& action, const std::string& uri) {
+ for (auto& message : userData.messages) {
+ if (message.type == "member" && message.body["action"] == action && message.body["uri"] == uri)
+ return true;
+ }
+ return false;
+ };
+ CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return messageReceived(aliceData, "join", bobUri); }));
+ CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return messageReceived(carlaData, "join", bobUri); }));
+
+ // Alice bans Bob
+ libjami::removeConversationMember(aliceId, convId, bobUri);
+ CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return messageReceived(aliceData, "ban", bobUri); }));
+ CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return messageReceived(carlaData, "ban", bobUri); }));
+
+ // Carla's messages should now be received by Alice, but not Bob
+ auto aliceMsgSize = aliceData.messages.size();
+ auto bobMsgSize = bobData.messages.size();
+ libjami::sendMessage(carlaId, convId, "hello"s, "");
+ CPPUNIT_ASSERT(!cv.wait_for(lk, 10s, [&]() { return bobMsgSize < bobData.messages.size(); }));
+ CPPUNIT_ASSERT(cv.wait_for(lk, 10s, [&]() { return aliceMsgSize + 1 == aliceData.messages.size(); }));
+}
+
void
ConversationMembersEventTest::testRemoveInvitedMember()
{