diff --git a/src/jamidht/conversation.h b/src/jamidht/conversation.h index 46812fcb7ae23e23fbf0ca67fb87183017e8a202..96d778ea88cdf30e8bf3de82b2b9ee9f20e551e3 100644 --- a/src/jamidht/conversation.h +++ b/src/jamidht/conversation.h @@ -87,6 +87,13 @@ struct ConversationRequest return m.size() == om.size() && std::equal(m.begin(), m.end(), om.begin()); } + bool isOneToOne() const { + try { + return metadatas.at("mode") == "0"; + } catch (...) {} + return true; + } + MSGPACK_DEFINE_MAP(from, conversationId, metadatas, received, declined) }; diff --git a/src/jamidht/conversation_module.cpp b/src/jamidht/conversation_module.cpp index bc1887ba494334cc8f00d6de98ba77562badc70d..59175593d96f5d27a53c6dd7572b7def8d3bb5f1 100644 --- a/src/jamidht/conversation_module.cpp +++ b/src/jamidht/conversation_module.cpp @@ -269,15 +269,21 @@ public: { ConversationModule::saveConvRequests(accountId_, conversationsRequests_); } + void declineOtherConversationWith(const std::string& uri) noexcept; bool addConversationRequest(const std::string& id, const ConversationRequest& req) { std::lock_guard<std::mutex> lk(conversationsRequestsMtx_); + // Check that we're not adding a second one to one trust request + // NOTE: If a new one to one request is received, we can decline the previous one. + if (req.isOneToOne()) + declineOtherConversationWith(req.from); auto it = conversationsRequests_.find(id); if (it != conversationsRequests_.end()) { // Check if updated if (req == it->second) return false; } + JAMI_DEBUG("Adding conversation request from {} ({})", req.from, id); conversationsRequests_[id] = req; saveConvRequests(); return true; @@ -710,6 +716,22 @@ ConversationModule::Impl::getOneToOneConversation(const std::string& uri) const return {}; } +void +ConversationModule::Impl::declineOtherConversationWith(const std::string& uri) noexcept +{ + // conversationsRequestsMtx_ MUST BE LOCKED + for (auto& [id, request] : conversationsRequests_) { + if (request.declined) + continue; // Ignore already declined requests + if (request.isOneToOne() && request.from == uri) { + JAMI_WARNING("Decline conversation request ({}) from {}", id, uri); + request.declined = std::time(nullptr); + emitSignal<libjami::ConversationSignal::ConversationRequestDeclined>(accountId_, + id); + } + } +} + std::vector<std::map<std::string, std::string>> ConversationModule::Impl::getConversationMembers(const std::string& conversationId, bool includeBanned) const @@ -1384,11 +1406,6 @@ ConversationModule::onTrustRequest(const std::string& uri, pimpl_->accountId_.c_str()); return; } - emitSignal<libjami::ConfigurationSignal::IncomingTrustRequest>(pimpl_->accountId_, - conversationId, - uri, - payload, - received); ConversationRequest req; req.from = uri; req.conversationId = conversationId; @@ -1396,10 +1413,16 @@ ConversationModule::onTrustRequest(const std::string& uri, req.metadatas = ConversationRepository::infosFromVCard(vCard::utils::toMap( std::string_view(reinterpret_cast<const char*>(payload.data()), payload.size()))); auto reqMap = req.toMap(); - pimpl_->addConversationRequest(conversationId, std::move(req)); - emitSignal<libjami::ConversationSignal::ConversationRequestReceived>(pimpl_->accountId_, - conversationId, - reqMap); + if (pimpl_->addConversationRequest(conversationId, std::move(req))) { + emitSignal<libjami::ConfigurationSignal::IncomingTrustRequest>(pimpl_->accountId_, + conversationId, + uri, + payload, + received); + emitSignal<libjami::ConversationSignal::ConversationRequestReceived>(pimpl_->accountId_, + conversationId, + reqMap); + } } void @@ -1424,13 +1447,14 @@ ConversationModule::onConversationRequest(const std::string& from, const Json::V } req.received = std::time(nullptr); auto reqMap = req.toMap(); - pimpl_->addConversationRequest(convId, std::move(req)); - // Note: no need to sync here because other connected devices should receive - // the same conversation request. Will sync when the conversation will be added + if (pimpl_->addConversationRequest(convId, std::move(req))) { + // Note: no need to sync here because other connected devices should receive + // the same conversation request. Will sync when the conversation will be added - emitSignal<libjami::ConversationSignal::ConversationRequestReceived>(pimpl_->accountId_, - convId, - reqMap); + emitSignal<libjami::ConversationSignal::ConversationRequestReceived>(pimpl_->accountId_, + convId, + reqMap); + } } std::string diff --git a/test/unitTest/conversation/conversationMembersEvent.cpp b/test/unitTest/conversation/conversationMembersEvent.cpp index 3ddeafa18e699b8c826d9f9d581e857b8627667f..ca636280815397d52809f4d60ec925932e487c31 100644 --- a/test/unitTest/conversation/conversationMembersEvent.cpp +++ b/test/unitTest/conversation/conversationMembersEvent.cpp @@ -88,6 +88,7 @@ public: void testBanUnbanGotFirstConv(); void testBanHostWhileHosting(); void testRemoveContactTwice(); + void testAddContactTwice(); std::string aliceId; std::string bobId; @@ -132,6 +133,7 @@ private: CPPUNIT_TEST(testBanUnbanGotFirstConv); CPPUNIT_TEST(testBanHostWhileHosting); CPPUNIT_TEST(testRemoveContactTwice); + CPPUNIT_TEST(testAddContactTwice); CPPUNIT_TEST_SUITE_END(); }; @@ -2666,6 +2668,63 @@ ConversationMembersEventTest::testRemoveContactTwice() CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return contactRemoved; })); } +void +ConversationMembersEventTest::testAddContactTwice() +{ + std::cout << "\nRunning test: " << __func__ << std::endl; + + auto aliceAccount = Manager::instance().getAccount<JamiAccount>(aliceId); + auto bobAccount = Manager::instance().getAccount<JamiAccount>(bobId); + auto bobUri = bobAccount->getUsername(); + auto aliceUri = aliceAccount->getUsername(); + std::map<std::string, std::shared_ptr<libjami::CallbackWrapperBase>> confHandlers; + bool requestReceived = false; + confHandlers.insert( + libjami::exportable_callback<libjami::ConversationSignal::ConversationRequestReceived>( + [&](const std::string& accountId, + const std::string&, + std::map<std::string, std::string> /*metadatas*/) { + if (accountId == bobId) + requestReceived = true; + cv.notify_one(); + })); + std::string convId = ""; + confHandlers.insert(libjami::exportable_callback<libjami::ConversationSignal::ConversationReady>( + [&](const std::string& accountId, const std::string& conversationId) { + if (accountId == aliceId) + convId = conversationId; + cv.notify_one(); + })); + auto requestDeclined = false; + confHandlers.insert( + libjami::exportable_callback<libjami::ConversationSignal::ConversationRequestDeclined>( + [&](const std::string& accountId, const std::string&) { + if (accountId == bobId) + requestDeclined = true; + cv.notify_one(); + })); + auto contactRemoved = false; + confHandlers.insert(libjami::exportable_callback<libjami::ConfigurationSignal::ContactRemoved>( + [&](const std::string& accountId, const std::string& uri, bool) { + if (accountId == aliceId && uri == bobUri) + contactRemoved = true; + cv.notify_one(); + })); + libjami::registerSignalHandlers(confHandlers); + requestReceived = false; + aliceAccount->addContact(bobUri); + aliceAccount->sendTrustRequest(bobUri, {}); + CPPUNIT_ASSERT(cv.wait_for(lk, 10s, [&]() { return requestReceived; })); + requestReceived = false; + aliceAccount->removeContact(bobUri, false); + CPPUNIT_ASSERT(cv.wait_for(lk, 10s, [&]() { return contactRemoved; })); + // wait that connections are closed. + std::this_thread::sleep_for(10s); + aliceAccount->addContact(bobUri); + aliceAccount->sendTrustRequest(bobUri, {}); + CPPUNIT_ASSERT(cv.wait_for(lk, 10s, [&]() { return requestDeclined && requestReceived; })); +} + } // namespace test } // namespace jami diff --git a/test/unitTest/syncHistory/syncHistory.cpp b/test/unitTest/syncHistory/syncHistory.cpp index 53309d85d30b06156c97ceed953dab891152d838..f40b8190deb931b6e1471f9149b9f6892d047f23 100644 --- a/test/unitTest/syncHistory/syncHistory.cpp +++ b/test/unitTest/syncHistory/syncHistory.cpp @@ -729,10 +729,11 @@ SyncHistoryTest::testSyncCreateAccountExportDeleteReimportWithConvReq() })); confHandlers.insert( libjami::exportable_callback<libjami::ConversationSignal::ConversationRequestReceived>( - [&](const std::string& /*accountId*/, + [&](const std::string& accountId, const std::string& /* conversationId */, std::map<std::string, std::string> /*metadatas*/) { - requestReceived = true; + if (accountId == aliceId) + requestReceived = true; cv.notify_one(); })); confHandlers.insert(libjami::exportable_callback<libjami::ConversationSignal::ConversationReady>( @@ -783,6 +784,7 @@ SyncHistoryTest::testSyncCreateAccountExportDeleteReimportWithConvReq() CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&] { return alice2Ready; })); // Should get the same request as before. + messageBobReceived = 0; libjami::acceptConversationRequest(alice2Id, convId); CPPUNIT_ASSERT( cv.wait_for(lk, 30s, [&]() { return conversationReady && messageBobReceived == 1; }));