From cb3d3c3c859e1a20f0f42fb2684941a2ceffb707 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=A9bastien=20Blin?=
<sebastien.blin@savoirfairelinux.com>
Date: Wed, 17 Aug 2022 15:45:46 -0400
Subject: [PATCH] swarm: avoid duplicated conversation after a ban
Several improvements are included with this patch:
1. confirmed is resetted when account is removed, this allow to
send a request when conversation is added back and reset the
whole matrix, avoiding duplicates
2. Banned can be due to harrasement. In this case, keeping the
conversation like the old behavior (pre-swarm) is better, to keep
proofs. Also re-adding back a contact is immediate if it was a
mistake as the conversation is kept back.
https://git.jami.net/savoirfairelinux/jami-project/-/issues/1449
Change-Id: I13da8ce9bd431b91ce7b7d455dae561358c62f10
---
src/jamidht/contact_list.cpp | 1 +
src/jamidht/conversation_module.cpp | 4 +-
.../conversation/conversationMembersEvent.cpp | 131 ++++++++++++++++++
.../conversation/conversationRequest.cpp | 2 +-
4 files changed, 136 insertions(+), 2 deletions(-)
diff --git a/src/jamidht/contact_list.cpp b/src/jamidht/contact_list.cpp
index 49328d3a28..755327a16d 100644
--- a/src/jamidht/contact_list.cpp
+++ b/src/jamidht/contact_list.cpp
@@ -114,6 +114,7 @@ ContactList::removeContact(const dht::InfoHash& h, bool ban)
else if (not c->second.isActive() and c->second.banned == ban)
return false;
c->second.removed = std::time(nullptr);
+ c->second.confirmed = false;
c->second.banned = ban;
auto uri = h.toString();
trust_.setCertificateStatus(uri,
diff --git a/src/jamidht/conversation_module.cpp b/src/jamidht/conversation_module.cpp
index 03b9415461..841ded0512 100644
--- a/src/jamidht/conversation_module.cpp
+++ b/src/jamidht/conversation_module.cpp
@@ -1686,7 +1686,7 @@ ConversationModule::isBannedDevice(const std::string& convId, const std::string&
}
void
-ConversationModule::removeContact(const std::string& uri, bool)
+ConversationModule::removeContact(const std::string& uri, bool banned)
{
// Remove linked conversation's requests
{
@@ -1706,6 +1706,8 @@ ConversationModule::removeContact(const std::string& uri, bool)
if (update)
pimpl_->saveConvRequests();
}
+ if (banned)
+ return; // Keep the conversation in banned model
// Remove related conversation
auto isSelf = uri == pimpl_->username_;
std::vector<std::string> toRm;
diff --git a/test/unitTest/conversation/conversationMembersEvent.cpp b/test/unitTest/conversation/conversationMembersEvent.cpp
index 9c4ad3845a..4e47c28a65 100644
--- a/test/unitTest/conversation/conversationMembersEvent.cpp
+++ b/test/unitTest/conversation/conversationMembersEvent.cpp
@@ -85,6 +85,7 @@ public:
void testAvoidTwoOneToOneMultiDevices();
void testRemoveRequestBannedMultiDevices();
void testBanUnbanMultiDevice();
+ void testBanUnbanGotFirstConv();
std::string aliceId;
std::string bobId;
@@ -123,6 +124,7 @@ private:
CPPUNIT_TEST(testAvoidTwoOneToOneMultiDevices);
CPPUNIT_TEST(testRemoveRequestBannedMultiDevices);
CPPUNIT_TEST(testBanUnbanMultiDevice);
+ CPPUNIT_TEST(testBanUnbanGotFirstConv);
CPPUNIT_TEST_SUITE_END();
};
@@ -2389,6 +2391,135 @@ ConversationMembersEventTest::testBanUnbanMultiDevice()
cv.wait_for(lk, 30s, [&]() { return conversationReadyBob && conversationReadyBob2; }));
}
+void
+ConversationMembersEventTest::testBanUnbanGotFirstConv()
+{
+ auto aliceAccount = Manager::instance().getAccount<JamiAccount>(aliceId);
+ auto bobAccount = Manager::instance().getAccount<JamiAccount>(bobId);
+ auto bobUri = bobAccount->getUsername();
+ auto aliceUri = aliceAccount->getUsername();
+
+ std::mutex mtx;
+ std::unique_lock<std::mutex> lk {mtx};
+ std::condition_variable cv;
+ std::map<std::string, std::shared_ptr<DRing::CallbackWrapperBase>> confHandlers;
+ auto requestReceived = false, requestReceivedBob2 = false;
+ confHandlers.insert(
+ DRing::exportable_callback<DRing::ConversationSignal::ConversationRequestReceived>(
+ [&](const std::string& accountId,
+ const std::string& conversationId,
+ std::map<std::string, std::string> /*metadatas*/) {
+ if (accountId == bobId)
+ requestReceived = true;
+ else if (accountId == bob2Id)
+ requestReceivedBob2 = true;
+ cv.notify_one();
+ }));
+ std::string convId;
+ auto conversationReadyBob = false, conversationReadyBob2 = false;
+ confHandlers.insert(DRing::exportable_callback<DRing::ConversationSignal::ConversationReady>(
+ [&](const std::string& accountId, const std::string& conversationId) {
+ if (accountId == aliceId) {
+ convId = conversationId;
+ } else if (accountId == bobId && conversationId == convId) {
+ conversationReadyBob = true;
+ } else if (accountId == bob2Id && conversationId == convId) {
+ conversationReadyBob2 = true;
+ }
+ cv.notify_one();
+ }));
+ auto bob2Started = false;
+ confHandlers.insert(
+ DRing::exportable_callback<DRing::ConfigurationSignal::VolatileDetailsChanged>(
+ [&](const std::string& accountId, const std::map<std::string, std::string>& details) {
+ if (accountId == bob2Id) {
+ auto daemonStatus = details.at(
+ DRing::Account::VolatileProperties::DEVICE_ANNOUNCED);
+ if (daemonStatus == "true")
+ bob2Started = true;
+ }
+ cv.notify_one();
+ }));
+ auto bob2ContactRemoved = false;
+ confHandlers.insert(DRing::exportable_callback<DRing::ConfigurationSignal::ContactRemoved>(
+ [&](const std::string& accountId, const std::string& uri, bool banned) {
+ if (accountId == bob2Id && uri == aliceUri && banned) {
+ bob2ContactRemoved = true;
+ }
+ cv.notify_one();
+ }));
+ auto bobMsgReceived = false, bob2MsgReceived = false;
+ confHandlers.insert(DRing::exportable_callback<DRing::ConversationSignal::MessageReceived>(
+ [&](const std::string& accountId,
+ const std::string& conversationId,
+ std::map<std::string, std::string> message) {
+ if (accountId == bobId && conversationId == convId)
+ bobMsgReceived = true;
+ else if (accountId == bob2Id && conversationId == convId)
+ bob2MsgReceived = true;
+ cv.notify_one();
+ }));
+ auto contactAddedBob = false, contactAddedBob2 = false;
+ confHandlers.insert(DRing::exportable_callback<DRing::ConfigurationSignal::ContactAdded>(
+ [&](const std::string& accountId, const std::string& uri, bool) {
+ if (accountId == bobId && uri == aliceUri) {
+ contactAddedBob = true;
+ } else if (accountId == bob2Id && uri == aliceUri) {
+ contactAddedBob2 = true;
+ }
+ cv.notify_one();
+ }));
+ DRing::registerSignalHandlers(confHandlers);
+
+ // Bob creates a second device
+ auto bobArchive = std::filesystem::current_path().string() + "/bob.gz";
+ std::remove(bobArchive.c_str());
+ bobAccount->exportArchive(bobArchive);
+ std::map<std::string, std::string> details = DRing::getAccountTemplate("RING");
+ details[ConfProperties::TYPE] = "RING";
+ details[ConfProperties::DISPLAYNAME] = "BOB2";
+ details[ConfProperties::ALIAS] = "BOB2";
+ details[ConfProperties::UPNP_ENABLED] = "true";
+ details[ConfProperties::ARCHIVE_PASSWORD] = "";
+ details[ConfProperties::ARCHIVE_PIN] = "";
+ details[ConfProperties::ARCHIVE_PATH] = bobArchive;
+ bob2Id = Manager::instance().addAccount(details);
+
+ CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return bob2Started; }));
+
+ // Alice adds bob
+ requestReceived = false;
+ aliceAccount->addContact(bobUri);
+ aliceAccount->sendTrustRequest(bobUri, {});
+ CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return requestReceived && requestReceivedBob2; }));
+ CPPUNIT_ASSERT(DRing::getConversationRequests(bob2Id).size() == 1);
+
+ // Accepts requests
+ DRing::acceptConversationRequest(bobId, convId);
+ CPPUNIT_ASSERT(
+ cv.wait_for(lk, 30s, [&]() { return conversationReadyBob && conversationReadyBob2; }));
+
+ // Bob bans alice, should update bob2
+ bobAccount->removeContact(aliceUri, true);
+ CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return bob2ContactRemoved; }));
+
+ // Alice sends messages, bob & bob2 should not get it!
+ bobMsgReceived = false, bob2MsgReceived = false;
+ DRing::sendMessage(aliceId, convId, "hi"s, "");
+ CPPUNIT_ASSERT(!cv.wait_for(lk, 30s, [&]() { return bobMsgReceived && bob2MsgReceived; }));
+
+ // Bobs re-add Alice
+ contactAddedBob = false, contactAddedBob2 = false;
+ bobAccount->addContact(aliceUri);
+ CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return contactAddedBob && contactAddedBob2; }));
+ std::this_thread::sleep_for(10s);
+
+ // Alice can sends some messages now
+ bobMsgReceived = false, bob2MsgReceived = false;
+ DRing::sendMessage(aliceId, convId, "hi"s, "");
+ CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return bobMsgReceived && bob2MsgReceived; }));
+}
+
} // namespace test
} // namespace jami
diff --git a/test/unitTest/conversation/conversationRequest.cpp b/test/unitTest/conversation/conversationRequest.cpp
index 4187cc2d91..c7cf8b5744 100644
--- a/test/unitTest/conversation/conversationRequest.cpp
+++ b/test/unitTest/conversation/conversationRequest.cpp
@@ -774,7 +774,7 @@ ConversationRequestTest::testBanContact()
!cv.wait_for(lk, 30s, [&]() { return memberMessageGenerated; }));
auto repoPath = fileutils::get_data_dir() + DIR_SEPARATOR_STR + bobAccount->getAccountID()
+ DIR_SEPARATOR_STR + "conversations" + DIR_SEPARATOR_STR + convId;
- CPPUNIT_ASSERT(!fileutils::isDirectory(repoPath));
+ CPPUNIT_ASSERT(fileutils::isDirectory(repoPath));
}
void
--
GitLab