diff --git a/src/data_transfer.cpp b/src/data_transfer.cpp index eb28a6a3e07b80507ed06bd77cf3cad2eb0ccb35..41fa2e747c7538a1a5d6023a8145034f87599270 100644 --- a/src/data_transfer.cpp +++ b/src/data_transfer.cpp @@ -1340,10 +1340,11 @@ TransferManager::onIncomingProfile(const std::shared_ptr<ChannelSocket>& channel auto itO = pimpl->vcards_.find({deviceId, uri}); if (itO != pimpl->vcards_.end()) pimpl->vcards_.erase(itO); - if (code == uint32_t(DRing::DataTransferEventCode::finished)) + if (code == uint32_t(DRing::DataTransferEventCode::finished)) { emitSignal<DRing::ConfigurationSignal::ProfileReceived>(accountId, uri, path); + } } }); }); @@ -1354,8 +1355,8 @@ TransferManager::onIncomingProfile(const std::shared_ptr<ChannelSocket>& channel std::string TransferManager::profilePath(const std::string& contactId) const { - // TODO Android? iOS? - return pimpl_->profilesPath_ + DIR_SEPARATOR_STR + base64::encode(contactId) + ".vcf"; + // TODO iOS? + return fmt::format("{}/{}.vcf", pimpl_->profilesPath_, base64::encode(contactId)); } std::vector<WaitingRequest> diff --git a/src/jamidht/conversation.cpp b/src/jamidht/conversation.cpp index bf0e8468361398ea7faa575f985f8e0ab6b919f3..c9e11216af554271e856439b5ed41602987b9cfd 100644 --- a/src/jamidht/conversation.cpp +++ b/src/jamidht/conversation.cpp @@ -1042,7 +1042,7 @@ Conversation::sync(const std::string& member, downloadFile(wr.interactionId, wr.fileId, wr.path, member, deviceId); // VCard sync for member if (auto account = pimpl_->account_.lock()) { - if (not account->needToSendProfile(deviceId)) { + if (not account->needToSendProfile(member, deviceId)) { JAMI_INFO() << "Peer " << deviceId << " already got an up-to-date vcard"; return; } diff --git a/src/jamidht/conversation_module.cpp b/src/jamidht/conversation_module.cpp index 5d4b8b79cff9ba3a5b3b4ca248d3210933b6991b..ded480f57b4843d02ecf0d0f5b2c8bac10d63132 100644 --- a/src/jamidht/conversation_module.cpp +++ b/src/jamidht/conversation_module.cpp @@ -404,56 +404,57 @@ ConversationModule::Impl::fetchNewCommits(const std::string& peer, return; } syncCnt.fetch_add(1); - onNeedSocket_(conversationId, - deviceId, - [this, - conversationId = std::move(conversationId), - peer = std::move(peer), - deviceId = std::move(deviceId), - commitId = std::move(commitId)](const auto& channel) { - auto conversation = conversations_.find(conversationId); - auto acc = account_.lock(); - if (!channel || !acc || conversation == conversations_.end() - || !conversation->second) { - std::lock_guard<std::mutex> lk(pendingConversationsFetchMtx_); - stopFetch(conversationId, deviceId); - syncCnt.fetch_sub(1); - return false; - } - acc->addGitSocket(channel->deviceId(), conversationId, channel); - conversation->second->sync( - peer, - deviceId, - [this, - conversationId = std::move(conversationId), - peer = std::move(peer), - deviceId = std::move(deviceId), - commitId = std::move(commitId)](bool ok) { - if (!ok) { - JAMI_WARN("[Account %s] Could not fetch new commit from " - "%s for %s, other " - "peer may be disconnected", - accountId_.c_str(), - deviceId.c_str(), - conversationId.c_str()); - JAMI_INFO("[Account %s] Relaunch sync with %s for %s", - accountId_.c_str(), - deviceId.c_str(), - conversationId.c_str()); - } - { - std::lock_guard<std::mutex> lk(pendingConversationsFetchMtx_); - pendingConversationsFetch_.erase(conversationId); - } - if (syncCnt.fetch_sub(1) == 1) { - if (auto account = account_.lock()) - emitSignal<DRing::ConversationSignal::ConversationSyncFinished>( - account->getAccountID().c_str()); - } - }, - commitId); - return true; - }); + onNeedSocket_( + conversationId, + deviceId, + [this, + conversationId = std::move(conversationId), + peer = std::move(peer), + deviceId = std::move(deviceId), + commitId = std::move(commitId)](const auto& channel) { + auto conversation = conversations_.find(conversationId); + auto acc = account_.lock(); + if (!channel || !acc || conversation == conversations_.end() + || !conversation->second) { + std::lock_guard<std::mutex> lk(pendingConversationsFetchMtx_); + stopFetch(conversationId, deviceId); + syncCnt.fetch_sub(1); + return false; + } + acc->addGitSocket(channel->deviceId(), conversationId, channel); + conversation->second->sync( + peer, + deviceId, + [this, + conversationId = std::move(conversationId), + peer = std::move(peer), + deviceId = std::move(deviceId), + commitId = std::move(commitId)](bool ok) { + if (!ok) { + JAMI_WARN("[Account %s] Could not fetch new commit from " + "%s for %s, other " + "peer may be disconnected", + accountId_.c_str(), + deviceId.c_str(), + conversationId.c_str()); + JAMI_INFO("[Account %s] Relaunch sync with %s for %s", + accountId_.c_str(), + deviceId.c_str(), + conversationId.c_str()); + } + { + std::lock_guard<std::mutex> lk(pendingConversationsFetchMtx_); + pendingConversationsFetch_.erase(conversationId); + } + if (syncCnt.fetch_sub(1) == 1) { + if (auto account = account_.lock()) + emitSignal<DRing::ConversationSignal::ConversationSyncFinished>( + account->getAccountID().c_str()); + } + }, + commitId); + return true; + }); } else { if (getRequest(conversationId) != std::nullopt) return; @@ -579,11 +580,17 @@ ConversationModule::Impl::handlePendingConversation(const std::string& conversat true); }); // Download members profile on first sync - if (auto cert = tls::CertificateStore::instance().getCertificate(deviceId)) { - if (cert->issuer && cert->issuer->getId().toString() == username_) { - if (auto acc = account_.lock()) { - for (const auto& member : conversation->memberUris(username_)) - acc->askForProfile(conversationId, deviceId, member); + auto isOneOne = conversation->mode() == ConversationMode::ONE_TO_ONE; + auto askForProfile = isOneOne; + if (!isOneOne) { + // If not 1:1 only download profiles from self (to avoid non checked files) + auto cert = tls::CertificateStore::instance().getCertificate(deviceId); + askForProfile = cert && cert->issuer && cert->issuer->getId().toString() == username_; + } + if (askForProfile) { + if (auto acc = account_.lock()) { + for (const auto& member : conversation->memberUris(username_)) { + acc->askForProfile(conversationId, deviceId, member); } } } @@ -1375,7 +1382,8 @@ ConversationModule::syncConversations(const std::string& peer, const std::string pimpl_->cloneConversation(deviceId, peer, cid); if (pimpl_->syncCnt.load() == 0) { if (auto acc = pimpl_->account_.lock()) - emitSignal<DRing::ConversationSignal::ConversationSyncFinished>(acc->getAccountID().c_str()); + emitSignal<DRing::ConversationSignal::ConversationSyncFinished>( + acc->getAccountID().c_str()); } } diff --git a/src/jamidht/jamiaccount.cpp b/src/jamidht/jamiaccount.cpp index f253b08d729ad4aa2c1dfe2f8f20a0715ba005c0..fe15eedb27d2f8438007595b84d736fc2790af63 100644 --- a/src/jamidht/jamiaccount.cpp +++ b/src/jamidht/jamiaccount.cpp @@ -1143,6 +1143,7 @@ JamiAccount::loadAccount(const std::string& archive_password, time_t received) { if (!id_.first) return; + clearProfileCache(uri); if (conversationId.empty()) { // Old path emitSignal<DRing::ConfigurationSignal::IncomingTrustRequest>(getAccountID(), @@ -2993,12 +2994,10 @@ void JamiAccount::removeContact(const std::string& uri, bool ban) { std::lock_guard<std::recursive_mutex> lock(configurationMutex_); - if (accountManager_) { + if (accountManager_) accountManager_->removeContact(uri, ban); - } else { + else JAMI_WARN("[Account %s] removeContact: account not loaded", getAccountID().c_str()); - return; - } } bool @@ -3931,12 +3930,12 @@ JamiAccount::requestSIPConnection(const std::string& peerId, } bool -JamiAccount::needToSendProfile(const std::string& deviceId) +JamiAccount::needToSendProfile(const std::string& peerUri, const std::string& deviceId) { - auto vCardMd5 = fileutils::sha3File(idPath_ + DIR_SEPARATOR_STR + "profile.vcf"); + auto vCardMd5 = fileutils::sha3File(fmt::format("{}/profile.vcf", idPath_)); std::string currentMd5 {}; - auto vCardPath = cachePath_ + DIR_SEPARATOR_STR + "vcard"; - auto sha3Path = vCardPath + DIR_SEPARATOR_STR + "sha3"; + auto vCardPath = fmt::format("{}/vcard", cachePath_); + auto sha3Path = fmt::format("{}/sha3", vCardPath); fileutils::check_dir(vCardPath.c_str(), 0700); try { currentMd5 = fileutils::loadTextFile(sha3Path); @@ -3951,7 +3950,8 @@ JamiAccount::needToSendProfile(const std::string& deviceId) fileutils::saveFile(sha3Path, {vCardMd5.begin(), vCardMd5.end()}, 0600); return true; } - return not fileutils::isFile(vCardPath + DIR_SEPARATOR_STR + deviceId); + fileutils::recursive_mkdir(fmt::format("{}/{}/", vCardPath, peerUri)); + return not fileutils::isFile(fmt::format("{}/{}/{}", vCardPath, peerUri, deviceId)); } bool @@ -4045,11 +4045,11 @@ JamiAccount::sendSIPMessage(SipConnection& conn, } void -JamiAccount::sendProfile(const std::string& deviceId) +JamiAccount::sendProfile(const std::string& peerUri, const std::string& deviceId) { try { - if (not needToSendProfile(deviceId)) { - JAMI_INFO() << "Peer " << deviceId << " already got an up-to-date vcard"; + if (not needToSendProfile(peerUri, deviceId)) { + JAMI_DEBUG("Peer {:s} ({:s}) already got an up-to-date vcard", deviceId, peerUri); return; } @@ -4070,6 +4070,12 @@ JamiAccount::sendProfile(const std::string& deviceId) } } +void +JamiAccount::clearProfileCache(const std::string& peerUri) +{ + fileutils::removeAll(fmt::format("{}/vcard/{}", cachePath_, peerUri)); +} + std::string JamiAccount::profilePath() const { @@ -4122,7 +4128,7 @@ JamiAccount::cacheSIPConnection(std::shared_ptr<ChannelSocket>&& socket, deviceId.to_c_str()); lk.unlock(); - sendProfile(deviceId.toString()); + sendProfile(peerId, deviceId.toString()); convModule()->syncConversations(peerId, deviceId.toString()); @@ -4270,7 +4276,11 @@ JamiAccount::transferFile(const std::string& conversationId, size_t start, size_t end) { - auto channelName = DATA_TRANSFER_URI + conversationId + "/" + currentDeviceId() + "/" + fileId; + auto channelName = fmt::format("{}{}/{}/{}", + DATA_TRANSFER_URI, + conversationId, + currentDeviceId(), + fileId); std::lock_guard<std::mutex> lkCM(connManagerMtx_); if (!connectionManager_) return; @@ -4309,8 +4319,11 @@ JamiAccount::askForFileChannel(const std::string& conversationId, if (!connectionManager_) return; - auto channelName = DATA_TRANSFER_URI + conversationId + "/" + currentDeviceId() + "/" - + fileId; + auto channelName = fmt::format("{}{}/{}/{}", + DATA_TRANSFER_URI, + conversationId, + currentDeviceId(), + fileId); if (start != 0 || end != 0) { channelName += "?start=" + std::to_string(start) + "&end=" + std::to_string(end); } diff --git a/src/jamidht/jamiaccount.h b/src/jamidht/jamiaccount.h index 555350becadf828906c0016ffec4418f8c9f169d..057a197a11bdcaa8d29623e540ebade5cfaa4acb 100644 --- a/src/jamidht/jamiaccount.h +++ b/src/jamidht/jamiaccount.h @@ -580,15 +580,22 @@ public: /** * Send Profile via cached SIP connection + * @param peerUri Uri that will receive the profile * @param deviceId Device that will receive the profile */ // Note: when swarm will be merged, this can be moved in transferManager - bool needToSendProfile(const std::string& deviceId); + bool needToSendProfile(const std::string& peerUri, const std::string& deviceId); /** * Send profile via cached SIP connection + * @param peerUri Uri that will receive the profile * @param deviceId Device that will receive the profile */ - void sendProfile(const std::string& deviceId); + void sendProfile(const std::string& peerUri, const std::string& deviceId); + /** + * Clear sent profiles (because of a removed contact or new trust request) + * @param peerUri Uri used to clear cache + */ + void clearProfileCache(const std::string& peerUri); std::string profilePath() const; diff --git a/src/jamidht/transfer_channel_handler.cpp b/src/jamidht/transfer_channel_handler.cpp index ae8ea008c93221b23e6eefbecf9e225f728818da..ef1307f7af5a92de1c66f440741640fbbcb07d08 100644 --- a/src/jamidht/transfer_channel_handler.cpp +++ b/src/jamidht/transfer_channel_handler.cpp @@ -45,7 +45,8 @@ TransferChannelHandler::connect(const DeviceId& deviceId, {} bool -TransferChannelHandler::onRequest(const std::shared_ptr<dht::crypto::Certificate>& cert, const std::string& name) +TransferChannelHandler::onRequest(const std::shared_ptr<dht::crypto::Certificate>& cert, + const std::string& name) { auto acc = account_.lock(); if (!acc || !cert || !cert->issuer) @@ -66,7 +67,7 @@ TransferChannelHandler::onRequest(const std::shared_ptr<dht::crypto::Certificate } // Check if peer is member of the conversation - if (fileId == "profile.vcf") { + if (fileId == fmt::format("{}.vcf", acc->getUsername())) { auto members = acc->convModule()->getConversationMembers(conversationId); return std::find_if(members.begin(), members.end(), [&](auto m) { return m["uri"] == uri; }) != members.end(); @@ -113,8 +114,8 @@ TransferChannelHandler::onReady(const std::shared_ptr<dht::crypto::Certificate>& fileId = fileId.substr(0, sep); } - if (fileId == "profile.vcf") { - std::string path = fileutils::sha3File(idPath_ + DIR_SEPARATOR_STR + "profile.vcf"); + if (fileId == fmt::format("{}.vcf", acc->getUsername())) { + std::string path = fmt::format("{}/profile.vcf", idPath_); acc->dataTransfer()->transferFile(channel, fileId, "", path); return; } else if (isContactProfile && fileId.find(".vcf") != std::string::npos) { diff --git a/test/unitTest/conversation/conversation.cpp b/test/unitTest/conversation/conversation.cpp index 49b89f32b2b2489fa5536949b8a2ebe6739a720e..40ad6d0bffaa86b7cf56946a4d848b41826ad0f6 100644 --- a/test/unitTest/conversation/conversation.cpp +++ b/test/unitTest/conversation/conversation.cpp @@ -2796,6 +2796,21 @@ ConversationTest::testRemoveReaddMultipleDevice() auto bobUri = bobAccount->getUsername(); auto aliceUri = aliceAccount->getUsername(); + std::string vcard = "BEGIN:VCARD\n\ +VERSION:2.1\n\ +FN:ALICE\n\ +DESCRIPTION:DESC\n\ +END:VCARD"; + auto vCardPath = fmt::format("{}/{}/profile.vcf", fileutils::get_data_dir(), aliceId); + // Add file + auto p = std::filesystem::path(vCardPath); + fileutils::recursive_mkdir(p.parent_path()); + std::ofstream file(p); + if (file.is_open()) { + file << vcard; + file.close(); + } + std::mutex mtx; std::unique_lock<std::mutex> lk {mtx}; std::condition_variable cv; @@ -2857,6 +2872,16 @@ ConversationTest::testRemoveReaddMultipleDevice() conversationRmBob2 = true; cv.notify_one(); })); + auto aliceProfileReceivedBob = false, aliceProfileReceivedBob2 = false; + confHandlers.insert(DRing::exportable_callback<DRing::ConfigurationSignal::ProfileReceived>( + [&](const std::string& accountId, const std::string& peerId, const std::string& path) { + if (accountId == bobId && peerId == aliceUri) { + aliceProfileReceivedBob = true; + } else if (accountId == bob2Id && peerId == aliceUri) { + aliceProfileReceivedBob2 = true; + } + cv.notify_one(); + })); DRing::registerSignalHandlers(confHandlers); // Bob creates a second device @@ -2900,9 +2925,13 @@ ConversationTest::testRemoveReaddMultipleDevice() // Re-Add contact should accept and clone the conversation on all devices conversationReadyBob = false; conversationReadyBob2 = false; + aliceProfileReceivedBob = false; + aliceProfileReceivedBob2 = false; DRing::acceptConversationRequest(bobId, convId); - CPPUNIT_ASSERT( - cv.wait_for(lk, 30s, [&]() { return conversationReadyBob && conversationReadyBob2; })); + CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { + return conversationReadyBob && conversationReadyBob2 && aliceProfileReceivedBob + && aliceProfileReceivedBob2; + })); } void