diff --git a/src/client/conversation_interface.cpp b/src/client/conversation_interface.cpp index cde0d11fa77ef21ef2993337707c095742edc603..1e6458cf64ada474f52fd60250af708d65674039 100644 --- a/src/client/conversation_interface.cpp +++ b/src/client/conversation_interface.cpp @@ -56,8 +56,7 @@ void declineConversationRequest(const std::string& accountId, const std::string& conversationId) { if (auto acc = jami::Manager::instance().getAccount<jami::JamiAccount>(accountId)) - if (auto convModule = acc->convModule()) - convModule->declineConversationRequest(conversationId); + acc->declineConversationRequest(conversationId); } bool diff --git a/src/jamidht/conversation_module.cpp b/src/jamidht/conversation_module.cpp index 513075aadeaa659669cc7809a2c9f54ce46c13bf..39c6152b175be060d17e4b89b64933f56ee9aec7 100644 --- a/src/jamidht/conversation_module.cpp +++ b/src/jamidht/conversation_module.cpp @@ -1042,6 +1042,39 @@ ConversationModule::loadConversations() pimpl_->saveConvInfos(); lk.unlock(); + + //////////////////////////////////////////////////////////////// + // Note: This is only to homogeneize trust and convRequests + std::vector<std::string> invalidPendingRequests; + { + std::lock_guard<std::mutex> lk(pimpl_->conversationsRequestsMtx_); + for (const auto& request: acc->getTrustRequests()) { + auto itConvId = request.find(libjami::Account::TrustRequest::CONVERSATIONID); + auto itConvFrom = request.find(libjami::Account::TrustRequest::FROM); + if (itConvId != request.end() && itConvFrom != request.end()) + { + // Check if requests exists or is declined. + auto itReq = pimpl_->conversationsRequests_.find(itConvId->second); + auto declined = itReq == pimpl_->conversationsRequests_.end() || itReq->second.declined; + if (declined) { + JAMI_WARNING("Invalid trust request found: {:s}", itConvId->second); + invalidPendingRequests.emplace_back(itConvFrom->second); + } + } + } + } + dht::ThreadPool::io().run([w=pimpl_->weak(), invalidPendingRequests = std::move(invalidPendingRequests)] () { + // Will lock account manager + auto shared = w.lock(); + if (!shared) + return; + if (auto acc = shared->account_.lock()) { + for (const auto& invalidPendingRequest : invalidPendingRequests) + acc->discardTrustRequest(invalidPendingRequest); + } + }); + + //////////////////////////////////////////////////////////////// for (const auto& conv : toRm) { JAMI_ERROR("Remove conversation ({})", conv); removeConversation(conv); @@ -1174,6 +1207,17 @@ ConversationModule::onConversationRequest(const std::string& from, const Json::V reqMap); } +std::string +ConversationModule::peerFromConversationRequest(const std::string& convId) const +{ + std::lock_guard<std::mutex> lk(pimpl_->conversationsRequestsMtx_); + auto it = pimpl_->conversationsRequests_.find(convId); + if (it != pimpl_->conversationsRequests_.end()) { + return it->second.from; + } + return {}; +} + void ConversationModule::onNeedConversationRequest(const std::string& from, const std::string& conversationId) diff --git a/src/jamidht/conversation_module.h b/src/jamidht/conversation_module.h index 27f5bb4cec8cc976131a80fc84d0d811075bc6e9..ef226458f82faf778b7a15b7d31d0978a034a7da 100644 --- a/src/jamidht/conversation_module.h +++ b/src/jamidht/conversation_module.h @@ -20,6 +20,7 @@ #pragma once +#include "conversation.h" #include "scheduled_executor.h" #include "jamidht/account_manager.h" #include "jamidht/conversation.h" @@ -108,6 +109,13 @@ public: */ void onConversationRequest(const std::string& from, const Json::Value& value); + /** + * Retrieve author of a conversation request + * @param convId Conversation's id + * @return the author of the conversation request + */ + std::string peerFromConversationRequest(const std::string& convId) const; + /** * Called when a peer needs an invite for a conversation (generally after that they received * a commit notification for a conversation they don't have yet) diff --git a/src/jamidht/jamiaccount.cpp b/src/jamidht/jamiaccount.cpp index e0c5ed1903f6c2fdfd051c2cdd8a365d07ed021d..d934b57ac76c84b2e652e98588eacab8f061f9db 100644 --- a/src/jamidht/jamiaccount.cpp +++ b/src/jamidht/jamiaccount.cpp @@ -2885,10 +2885,29 @@ JamiAccount::discardTrustRequest(const std::string& from) std::lock_guard<std::recursive_mutex> lock(configurationMutex_); if (accountManager_) return accountManager_->discardTrustRequest(from); - JAMI_WARN("[Account %s] discardTrustRequest: account not loaded", getAccountID().c_str()); + JAMI_WARNING("[Account {:s}] discardTrustRequest: account not loaded", getAccountID()); return false; } +void +JamiAccount::declineConversationRequest(const std::string& conversationId) +{ + auto peerId = convModule()->peerFromConversationRequest(conversationId); + convModule()->declineConversationRequest(conversationId); + if (!peerId.empty()) { + std::lock_guard<std::recursive_mutex> lock(configurationMutex_); + if (auto info = accountManager_->getInfo()) { + // Verify if we have a trust request with this peer + convId + auto req = info->contacts->getTrustRequest(dht::InfoHash(peerId)); + if (req.find(libjami::Account::TrustRequest::CONVERSATIONID) != req.end() + && req.at(libjami::Account::TrustRequest::CONVERSATIONID) == conversationId) { + accountManager_->discardTrustRequest(peerId); + JAMI_DEBUG("[Account {:s}] declined trust request with {:s}", getAccountID(), peerId); + } + } + } +} + void JamiAccount::sendTrustRequest(const std::string& to, const std::vector<uint8_t>& payload) { diff --git a/src/jamidht/jamiaccount.h b/src/jamidht/jamiaccount.h index 72761daac1a71a3bef7f232c85f75a099023d16e..750c1389229fdf1f943ad16bede25d6b854ae5ab 100644 --- a/src/jamidht/jamiaccount.h +++ b/src/jamidht/jamiaccount.h @@ -286,6 +286,7 @@ public: // Note: includeConversation used for compatibility test. Do not change bool acceptTrustRequest(const std::string& from, bool includeConversation = true); bool discardTrustRequest(const std::string& from); + void declineConversationRequest(const std::string& conversationId); /** * Add contact to the account contact list. diff --git a/test/unitTest/conversation/conversationRequest.cpp b/test/unitTest/conversation/conversationRequest.cpp index bb502ebc3cc4a6230b7d7d061199f11f770e6cc4..3b7299eec02ced65eff8c9a8d9b26df4624ffb8b 100644 --- a/test/unitTest/conversation/conversationRequest.cpp +++ b/test/unitTest/conversation/conversationRequest.cpp @@ -59,6 +59,8 @@ public: void testGetRequests(); void testDeclineRequest(); void testAddContact(); + void testDeclineConversationRequestRemoveTrustRequest(); + void testMalformedTrustRequest(); void testAddContactDeleteAndReAdd(); void testInviteFromMessageAfterRemoved(); void testRemoveContact(); @@ -86,6 +88,8 @@ private: CPPUNIT_TEST(testGetRequests); CPPUNIT_TEST(testDeclineRequest); CPPUNIT_TEST(testAddContact); + CPPUNIT_TEST(testDeclineConversationRequestRemoveTrustRequest); + CPPUNIT_TEST(testMalformedTrustRequest); CPPUNIT_TEST(testAddContactDeleteAndReAdd); CPPUNIT_TEST(testInviteFromMessageAfterRemoved); CPPUNIT_TEST(testRemoveContact); @@ -361,6 +365,129 @@ ConversationRequestTest::testAddContact() CPPUNIT_ASSERT(fileutils::isFile(bobMember)); } +void +ConversationRequestTest::testDeclineConversationRequestRemoveTrustRequest() +{ + 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<libjami::CallbackWrapperBase>> confHandlers; + bool conversationReady = false, requestReceived = false, memberMessageGenerated = false; + std::string convId = ""; + confHandlers.insert(libjami::exportable_callback<libjami::ConfigurationSignal::IncomingTrustRequest>( + [&](const std::string& account_id, + const std::string& /*from*/, + const std::string& /*conversationId*/, + const std::vector<uint8_t>& /*payload*/, + time_t /*received*/) { + if (account_id == bobId) + requestReceived = true; + cv.notify_one(); + })); + confHandlers.insert(libjami::exportable_callback<libjami::ConversationSignal::ConversationReady>( + [&](const std::string& accountId, const std::string& conversationId) { + if (accountId == aliceId) { + convId = conversationId; + } else if (accountId == bobId) { + conversationReady = true; + } + cv.notify_one(); + })); + confHandlers.insert(libjami::exportable_callback<libjami::ConversationSignal::MessageReceived>( + [&](const std::string& accountId, + const std::string& conversationId, + std::map<std::string, std::string> message) { + if (accountId == aliceId && conversationId == convId && message["type"] == "member") { + memberMessageGenerated = true; + cv.notify_one(); + } + })); + libjami::registerSignalHandlers(confHandlers); + aliceAccount->addContact(bobUri); + aliceAccount->sendTrustRequest(bobUri, {}); + CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return !convId.empty() && requestReceived; })); + + // Decline request + auto requests = libjami::getConversationRequests(bobId); + CPPUNIT_ASSERT(requests.size() == 1); + auto trustRequests = libjami::getTrustRequests(bobId); + CPPUNIT_ASSERT(trustRequests.size() == 1); + libjami::declineConversationRequest(bobId, convId); + requests = libjami::getConversationRequests(bobId); + CPPUNIT_ASSERT(requests.size() == 0); + trustRequests = libjami::getTrustRequests(bobId); + CPPUNIT_ASSERT(trustRequests.size() == 0); +} + +void +ConversationRequestTest::testMalformedTrustRequest() +{ + 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<libjami::CallbackWrapperBase>> confHandlers; + bool conversationReady = false, requestReceived = false, memberMessageGenerated = false; + std::string convId = ""; + confHandlers.insert(libjami::exportable_callback<libjami::ConfigurationSignal::IncomingTrustRequest>( + [&](const std::string& account_id, + const std::string& /*from*/, + const std::string& /*conversationId*/, + const std::vector<uint8_t>& /*payload*/, + time_t /*received*/) { + if (account_id == bobId) + requestReceived = true; + cv.notify_one(); + })); + confHandlers.insert(libjami::exportable_callback<libjami::ConversationSignal::ConversationReady>( + [&](const std::string& accountId, const std::string& conversationId) { + if (accountId == aliceId) { + convId = conversationId; + } else if (accountId == bobId) { + conversationReady = true; + } + cv.notify_one(); + })); + confHandlers.insert(libjami::exportable_callback<libjami::ConversationSignal::MessageReceived>( + [&](const std::string& accountId, + const std::string& conversationId, + std::map<std::string, std::string> message) { + if (accountId == aliceId && conversationId == convId && message["type"] == "member") { + memberMessageGenerated = true; + cv.notify_one(); + } + })); + libjami::registerSignalHandlers(confHandlers); + aliceAccount->addContact(bobUri); + aliceAccount->sendTrustRequest(bobUri, {}); + CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return !convId.empty() && requestReceived; })); + + // Decline request + auto requests = libjami::getConversationRequests(bobId); + CPPUNIT_ASSERT(requests.size() == 1); + auto trustRequests = libjami::getTrustRequests(bobId); + CPPUNIT_ASSERT(trustRequests.size() == 1); + // This will let the trust request (not libjami::declineConversationRequest) + bobAccount->convModule()->declineConversationRequest(convId); + requests = libjami::getConversationRequests(bobId); + CPPUNIT_ASSERT(requests.size() == 0); + trustRequests = libjami::getTrustRequests(bobId); + CPPUNIT_ASSERT(trustRequests.size() == 1); + // Reload conversation will fix the state + bobAccount->convModule()->loadConversations(); + requests = libjami::getConversationRequests(bobId); + CPPUNIT_ASSERT(requests.size() == 0); + trustRequests = libjami::getTrustRequests(bobId); + CPPUNIT_ASSERT(trustRequests.size() == 0); +} + void ConversationRequestTest::testAddContactDeleteAndReAdd() {