From 588890f98e20aaea85161d2559bc92c68a2c2e5b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=A9bastien=20Blin?=
<sebastien.blin@savoirfairelinux.com>
Date: Thu, 22 Jul 2021 14:26:13 -0400
Subject: [PATCH] conversationrepository: add member left for one to one
This breaks one to one swarm. Now, invited/ is added to detect
contacts that joins and left a conversation. This is wanted
to show read-only conversations.
Change-Id: I519094e75872062538fe56dd3a32378da814005b
GitLab: #595
---
src/jamidht/conversation.cpp | 4 +-
src/jamidht/conversation.h | 4 +-
src/jamidht/conversationrepository.cpp | 61 +++++++++++++++------
src/jamidht/conversationrepository.h | 4 +-
src/jamidht/jamiaccount.cpp | 2 +-
test/unitTest/conversation/conversation.cpp | 15 ++++-
6 files changed, 67 insertions(+), 23 deletions(-)
diff --git a/src/jamidht/conversation.cpp b/src/jamidht/conversation.cpp
index 3654904236..b802bd87d3 100644
--- a/src/jamidht/conversation.cpp
+++ b/src/jamidht/conversation.cpp
@@ -571,7 +571,7 @@ Conversation::removeMember(const std::string& contactUri, bool isDevice, const O
}
std::vector<std::map<std::string, std::string>>
-Conversation::getMembers(bool includeInvited) const
+Conversation::getMembers(bool includeInvited, bool includeLeft) const
{
std::vector<std::map<std::string, std::string>> result;
@@ -585,6 +585,8 @@ Conversation::getMembers(bool includeInvited) const
continue;
if (member.role == MemberRole::INVITED && !includeInvited)
continue;
+ if (member.role == MemberRole::LEFT && !includeLeft)
+ continue;
auto mm = member.map();
std::string lastDisplayed;
auto itDisplayed = pimpl_->lastDisplayed_.find(member.uri);
diff --git a/src/jamidht/conversation.h b/src/jamidht/conversation.h
index 4deacf7f5c..fa0fa2047e 100644
--- a/src/jamidht/conversation.h
+++ b/src/jamidht/conversation.h
@@ -106,6 +106,7 @@ public:
void removeMember(const std::string& contactUri, bool isDevice, const OnDoneCb& cb = {});
/**
* @param includeInvited If we want invited members
+ * @param includeLeft If we want left members
* @return a vector of member details:
* {
* "uri":"xxx",
@@ -114,7 +115,8 @@ public:
* ...
* }
*/
- std::vector<std::map<std::string, std::string>> getMembers(bool includeInvited = false) const;
+ std::vector<std::map<std::string, std::string>> getMembers(bool includeInvited = false,
+ bool includeLeft = false) const;
/**
* Join a conversation
diff --git a/src/jamidht/conversationrepository.cpp b/src/jamidht/conversationrepository.cpp
index 88f8015141..28455dbd50 100644
--- a/src/jamidht/conversationrepository.cpp
+++ b/src/jamidht/conversationrepository.cpp
@@ -197,12 +197,16 @@ git_add_all(git_repository* repo)
* @return if files were added successfully
*/
bool
-add_initial_files(GitRepository& repo, const std::shared_ptr<JamiAccount>& account)
+add_initial_files(GitRepository& repo,
+ const std::shared_ptr<JamiAccount>& account,
+ ConversationMode mode,
+ const std::string& otherMember = "")
{
auto deviceId = account->currentDeviceId();
std::string repoPath = git_repository_workdir(repo.get());
std::string adminsPath = repoPath + "admins";
std::string devicesPath = repoPath + "devices";
+ std::string invitedPath = repoPath + "invited";
std::string crlsPath = repoPath + "CRLs" + DIR_SEPARATOR_STR + deviceId;
if (!fileutils::recursive_mkdir(adminsPath, 0700)) {
@@ -263,6 +267,25 @@ add_initial_files(GitRepository& repo, const std::shared_ptr<JamiAccount>& accou
file.close();
}
+ // /invited for one to one
+ if (mode == ConversationMode::ONE_TO_ONE) {
+ if (!fileutils::recursive_mkdir(invitedPath, 0700)) {
+ JAMI_ERR("Error when creating %s.", invitedPath.c_str());
+ return false;
+ }
+ std::string invitedMemberPath = invitedPath + "/" + otherMember;
+ if (fileutils::isFile(invitedMemberPath)) {
+ JAMI_WARN("Member %s already present!", otherMember.c_str());
+ return false;
+ }
+
+ auto file = fileutils::ofstream(invitedMemberPath, std::ios::trunc | std::ios::binary);
+ if (!file.is_open()) {
+ JAMI_ERR("Could not write data to %s", invitedMemberPath.c_str());
+ return false;
+ }
+ }
+
if (!git_add_all(repo.get())) {
return false;
}
@@ -878,11 +901,9 @@ ConversationRepository::Impl::checkValidJoins(const std::string& userDevice,
auto userUri = cert->getIssuerUID();
// Check no other files changed
auto changedFiles = ConversationRepository::changedFiles(diffStats(commitId, parentId));
- auto oneone = mode() == ConversationMode::ONE_TO_ONE;
- std::size_t wantedChanged = oneone ? 2 : 3;
- if (changedFiles.size() != wantedChanged) {
+ std::size_t wantedChanged = 3;
+ if (changedFiles.size() != wantedChanged)
return false;
- }
auto invitedFile = std::string("invited") + "/" + uriMember;
auto membersFile = std::string("members") + "/" + uriMember + ".crt";
@@ -897,16 +918,14 @@ ConversationRepository::Impl::checkValidJoins(const std::string& userDevice,
if (not treeNew or not treeOld)
return false;
- // Check /invited removed
- if (!oneone) {
- if (fileAtTree(invitedFile, treeNew)) {
- JAMI_ERR("%s invited not removed", userUri.c_str());
- return false;
- }
- if (!fileAtTree(invitedFile, treeOld)) {
- JAMI_ERR("%s invited not found", userUri.c_str());
- return false;
- }
+ // Check /invited
+ if (fileAtTree(invitedFile, treeNew)) {
+ JAMI_ERR("%s invited not removed", userUri.c_str());
+ return false;
+ }
+ if (!fileAtTree(invitedFile, treeOld)) {
+ JAMI_ERR("%s invited not found", userUri.c_str());
+ return false;
}
// Check /members added
@@ -1151,6 +1170,9 @@ bool
ConversationRepository::Impl::checkInitialCommit(const std::string& userDevice,
const std::string& commitId) const
{
+ auto account = account_.lock();
+ if (!account)
+ return false;
auto cert = tls::CertificateStore::instance().getCertificate(userDevice);
if (!cert) {
JAMI_ERR("Cannot find certificate for %s", userDevice.c_str());
@@ -1170,11 +1192,13 @@ ConversationRepository::Impl::checkInitialCommit(const std::string& userDevice,
auto hasDevice = false, hasAdmin = false;
std::string adminsFile = std::string("admins") + "/" + userUri + ".crt";
std::string deviceFile = std::string("devices") + "/" + userDevice + ".crt";
+ std::string invitedFile = std::string("invited") + "/" + account->getUsername();
std::string crlFile = std::string("CRLs") + "/" + userUri;
// Check that admin cert is added
// Check that device cert is added
// Check CRLs added
// Check that no other file is added
+ // Check if invited file present for one to one.
for (const auto& changedFile : changedFiles) {
if (changedFile == adminsFile) {
hasAdmin = true;
@@ -1182,6 +1206,8 @@ ConversationRepository::Impl::checkInitialCommit(const std::string& userDevice,
hasDevice = true;
} else if (changedFile == crlFile) {
// Nothing to do
+ } else if (changedFile == invitedFile && mode_ == ConversationMode::ONE_TO_ONE) {
+ // Nothing to do
} else {
// Invalid file detected
JAMI_ERR("Invalid add file detected: %s", changedFile.c_str());
@@ -1801,7 +1827,8 @@ ConversationRepository::Impl::initMembers()
for (const auto& member : getInitialMembers()) {
auto it = std::find(uris.begin(), uris.end(), member);
if (it == uris.end()) {
- members_.emplace_back(ConversationMember {member, MemberRole::INVITED});
+ // If member is in initial commit, but not in invited, this means that user left.
+ members_.emplace_back(ConversationMember {member, MemberRole::LEFT});
}
}
}
@@ -1860,7 +1887,7 @@ ConversationRepository::createConversation(const std::weak_ptr<JamiAccount>& acc
}
// Add initial files
- if (!add_initial_files(repo, shared)) {
+ if (!add_initial_files(repo, shared, mode, otherMember)) {
JAMI_ERR("Error when adding initial files");
fileutils::removeAll(tmpPath, true);
return {};
diff --git a/src/jamidht/conversationrepository.h b/src/jamidht/conversationrepository.h
index 63e747d957..cec04ea9c6 100644
--- a/src/jamidht/conversationrepository.h
+++ b/src/jamidht/conversationrepository.h
@@ -72,7 +72,7 @@ struct ConversationCommit
int64_t timestamp {0};
};
-enum class MemberRole { ADMIN = 0, MEMBER, INVITED, BANNED };
+enum class MemberRole { ADMIN = 0, MEMBER, INVITED, BANNED, LEFT };
struct ConversationMember
{
@@ -90,6 +90,8 @@ struct ConversationMember
rolestr = "invited";
} else if (role == MemberRole::BANNED) {
rolestr = "banned";
+ } else if (role == MemberRole::LEFT) {
+ rolestr = "left"; // For one to one
}
return {{"uri", uri}, {"role", rolestr}};
diff --git a/src/jamidht/jamiaccount.cpp b/src/jamidht/jamiaccount.cpp
index 53176ef2ab..d384a7be35 100644
--- a/src/jamidht/jamiaccount.cpp
+++ b/src/jamidht/jamiaccount.cpp
@@ -4064,7 +4064,7 @@ JamiAccount::getConversationMembers(const std::string& conversationId) const
std::unique_lock<std::mutex> lk(conversationsMtx_);
auto conversation = conversations_.find(conversationId);
if (conversation != conversations_.end() && conversation->second)
- return conversation->second->getMembers(true);
+ return conversation->second->getMembers(true, true);
lk.unlock();
if (auto info = accountManager_->getInfo()) {
diff --git a/test/unitTest/conversation/conversation.cpp b/test/unitTest/conversation/conversation.cpp
index 4ca231c915..3bb5ee97ee 100644
--- a/test/unitTest/conversation/conversation.cpp
+++ b/test/unitTest/conversation/conversation.cpp
@@ -3607,9 +3607,10 @@ ConversationTest::testRemoveContact()
DRing::registerSignalHandlers(confHandlers);
aliceAccount->addContact(bobUri);
aliceAccount->sendTrustRequest(bobUri, {});
- CPPUNIT_ASSERT(cv.wait_for(lk, std::chrono::seconds(5), [&]() { return !convId.empty(); }));
// Check created files
- CPPUNIT_ASSERT(cv.wait_for(lk, std::chrono::seconds(30), [&]() { return requestReceived; }));
+ CPPUNIT_ASSERT(cv.wait_for(lk, std::chrono::seconds(30), [&]() {
+ return !convId.empty() && requestReceived;
+ }));
memberMessageGenerated = false;
CPPUNIT_ASSERT(bobAccount->acceptTrustRequest(aliceUri));
CPPUNIT_ASSERT(cv.wait_for(lk, std::chrono::seconds(30), [&]() {
@@ -3620,6 +3621,16 @@ ConversationTest::testRemoveContact()
bobAccount->removeContact(aliceUri, false);
CPPUNIT_ASSERT(
cv.wait_for(lk, std::chrono::seconds(30), [&]() { return memberMessageGenerated; }));
+
+ // Check that getConversationMembers return "role":"left"
+ auto members = aliceAccount->getConversationMembers(convId);
+ CPPUNIT_ASSERT(std::find_if(members.begin(),
+ members.end(),
+ [&](auto member) {
+ return member["uri"] == bobUri && member["role"] == "left";
+ })
+ != members.end());
+
aliceAccount->removeContact(bobUri, false);
cv.wait_for(lk, std::chrono::seconds(20));
--
GitLab