From 45baff6a4cbbda78a8574d315b91d897289066f6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=A9bastien=20Blin?=
<sebastien.blin@savoirfairelinux.com>
Date: Thu, 21 Jul 2022 09:53:23 -0400
Subject: [PATCH] jamiaccount: remove connections and data on banned
A contact can be banned from another device. In this case,
JamiAccount::removeContact is not called and the daemon only call the
dedicated callback.
So, remove the conversation requests and connections to the banned
contact (and avoid future incoming calls if connected) when the
callback is called instead in JamiAccount::removeContact() as it
will be called in a multi-device context.
Change-Id: I1ac8b441268d7bfc9bb6627fbbbc7b83e2e75052
---
src/jamidht/jamiaccount.cpp | 38 +++++----
.../conversation/conversationMembersEvent.cpp | 77 +++++++++++++++++++
2 files changed, 99 insertions(+), 16 deletions(-)
diff --git a/src/jamidht/jamiaccount.cpp b/src/jamidht/jamiaccount.cpp
index 52a4e9189a..5e8ae81136 100644
--- a/src/jamidht/jamiaccount.cpp
+++ b/src/jamidht/jamiaccount.cpp
@@ -1127,8 +1127,27 @@ JamiAccount::loadAccount(const std::string& archive_password,
[this](const std::string& uri, bool banned) {
if (!id_.first)
return;
- runOnMainThread([id = getAccountID(), uri, banned] {
- emitSignal<DRing::ConfigurationSignal::ContactRemoved>(id, uri, banned);
+
+ dht::ThreadPool::io().run([w = weak(), uri, banned] {
+ if (auto shared = w.lock()) {
+ // Erase linked conversation's requests
+ if (auto convModule = shared->convModule())
+ convModule->removeContact(uri, banned);
+
+ // Remove current connections with contact
+ std::unique_lock<std::mutex> lk(shared->sipConnsMtx_);
+ for (auto it = shared->sipConns_.begin(); it != shared->sipConns_.end();) {
+ const auto& [key, value] = *it;
+ if (key.first == uri)
+ it = shared->sipConns_.erase(it);
+ else
+ ++it;
+ }
+ // Update client.
+ emitSignal<DRing::ConfigurationSignal::ContactRemoved>(shared->getAccountID(),
+ uri,
+ banned);
+ }
});
},
[this](const std::string& uri,
@@ -2982,26 +3001,13 @@ JamiAccount::addContact(const std::string& uri, bool confirmed)
void
JamiAccount::removeContact(const std::string& uri, bool ban)
{
- std::unique_lock<std::recursive_mutex> lock(configurationMutex_);
+ std::lock_guard<std::recursive_mutex> lock(configurationMutex_);
if (accountManager_) {
accountManager_->removeContact(uri, ban);
} else {
JAMI_WARN("[Account %s] removeContact: account not loaded", getAccountID().c_str());
return;
}
- lock.unlock();
-
- convModule()->removeContact(uri, ban);
-
- // Remove current connections with contact
- std::unique_lock<std::mutex> lk(sipConnsMtx_);
- for (auto it = sipConns_.begin(); it != sipConns_.end();) {
- const auto& [key, value] = *it;
- if (key.first == uri)
- it = sipConns_.erase(it);
- else
- ++it;
- }
}
bool
diff --git a/test/unitTest/conversation/conversationMembersEvent.cpp b/test/unitTest/conversation/conversationMembersEvent.cpp
index 9eb06ba4dd..12e69db0ea 100644
--- a/test/unitTest/conversation/conversationMembersEvent.cpp
+++ b/test/unitTest/conversation/conversationMembersEvent.cpp
@@ -83,6 +83,7 @@ public:
void testGetConversationMembersWithSelfOneOne();
void testAvoidTwoOneToOne();
void testAvoidTwoOneToOneMultiDevices();
+ void testRemoveRequestBannedMultiDevices();
std::string aliceId;
std::string bobId;
@@ -119,6 +120,7 @@ private:
CPPUNIT_TEST(testGetConversationMembersWithSelfOneOne);
CPPUNIT_TEST(testAvoidTwoOneToOne);
CPPUNIT_TEST(testAvoidTwoOneToOneMultiDevices);
+ CPPUNIT_TEST(testRemoveRequestBannedMultiDevices);
CPPUNIT_TEST_SUITE_END();
};
@@ -2193,6 +2195,81 @@ ConversationMembersEventTest::testAvoidTwoOneToOneMultiDevices()
cv.wait_for(lk, 30s, [&]() { return conversationReadyBob && conversationReadyBob2; }));
}
+void
+ConversationMembersEventTest::testRemoveRequestBannedMultiDevices()
+{
+ 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();
+ }));
+ 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();
+ }));
+ 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);
+
+ // Bob bans alice, should update bob2
+ bobAccount->removeContact(aliceUri, true);
+ CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return bob2ContactRemoved; }));
+ CPPUNIT_ASSERT(DRing::getConversationRequests(bob2Id).size() == 0);
+}
+
} // namespace test
} // namespace jami
--
GitLab