From f25badc18c1534ea45f6b53660c228caab360177 Mon Sep 17 00:00:00 2001
From: Torsten Grote <t@grobox.de>
Date: Mon, 17 Jul 2017 12:59:00 -0300
Subject: [PATCH] Move responsibility for pre-sharing blogs to sharing manager
to have all the code related to that in one place,
so it is easier to maintain and to spot bugs.
This also checks that only blogs without an existing sharing session
are shared and initialized again.
It extends an existing test to catch the missing check.
This removes some debugging information from the previous commit
to not leak private information via the sharing sessions.
Fixes #979
---
.../briar/blog/BlogManagerImpl.java | 21 +++----------------
.../briarproject/briar/blog/BlogModule.java | 1 -
.../briar/sharing/BlogSharingManagerImpl.java | 12 +++++++++--
.../briar/sharing/SharingManagerImpl.java | 17 ++++++++++++++-
.../briar/blog/BlogManagerImplTest.java | 21 -------------------
.../sharing/BlogSharingIntegrationTest.java | 21 +++++++++++++++++++
6 files changed, 50 insertions(+), 43 deletions(-)
diff --git a/briar-core/src/main/java/org/briarproject/briar/blog/BlogManagerImpl.java b/briar-core/src/main/java/org/briarproject/briar/blog/BlogManagerImpl.java
index e42932e194..11a7c84ebf 100644
--- a/briar-core/src/main/java/org/briarproject/briar/blog/BlogManagerImpl.java
+++ b/briar-core/src/main/java/org/briarproject/briar/blog/BlogManagerImpl.java
@@ -48,9 +48,7 @@ import java.util.concurrent.CopyOnWriteArrayList;
import javax.annotation.Nullable;
import javax.inject.Inject;
-import static org.briarproject.bramble.api.contact.ContactManager.AddContactHook;
import static org.briarproject.bramble.api.contact.ContactManager.RemoveContactHook;
-import static org.briarproject.bramble.api.sync.Group.Visibility.SHARED;
import static org.briarproject.briar.api.blog.BlogConstants.KEY_AUTHOR;
import static org.briarproject.briar.api.blog.BlogConstants.KEY_AUTHOR_ID;
import static org.briarproject.briar.api.blog.BlogConstants.KEY_AUTHOR_NAME;
@@ -72,7 +70,7 @@ import static org.briarproject.briar.blog.BlogPostValidator.authorToBdfDictionar
@NotNullByDefault
class BlogManagerImpl extends BdfIncomingMessageHook implements BlogManager,
- AddContactHook, RemoveContactHook, Client {
+ RemoveContactHook, Client {
private final IdentityManager identityManager;
private final BlogFactory blogFactory;
@@ -96,26 +94,13 @@ class BlogManagerImpl extends BdfIncomingMessageHook implements BlogManager,
// Create our personal blog if necessary
LocalAuthor a = identityManager.getLocalAuthor(txn);
Blog b = blogFactory.createBlog(a);
- db.addGroup(txn, b.getGroup());
- // Ensure that we have the personal blogs of all contacts
- for (Contact c : db.getContacts(txn)) addingContact(txn, c);
- }
-
- @Override
- public void addingContact(Transaction txn, Contact c) throws DbException {
- // Add the personal blog of the contact and share it with the contact
- Blog b = blogFactory.createBlog(c.getAuthor());
- addBlog(txn, b);
- db.setGroupVisibility(txn, c.getId(), b.getId(), SHARED);
- // Share our personal blog with the contact
- LocalAuthor a = identityManager.getLocalAuthor(txn);
- Blog b2 = blogFactory.createBlog(a);
- db.setGroupVisibility(txn, c.getId(), b2.getId(), SHARED);
+ db.addGroup(txn, b.getGroup()); // does nothing, if group exists
}
@Override
public void removingContact(Transaction txn, Contact c) throws DbException {
Blog b = blogFactory.createBlog(c.getAuthor());
+ // TODO we might want to reconsider removing b, if otherwise shared
if (db.containsGroup(txn, b.getId())) removeBlog(txn, b);
}
diff --git a/briar-core/src/main/java/org/briarproject/briar/blog/BlogModule.java b/briar-core/src/main/java/org/briarproject/briar/blog/BlogModule.java
index 4a6fde0806..c6459cb57a 100644
--- a/briar-core/src/main/java/org/briarproject/briar/blog/BlogModule.java
+++ b/briar-core/src/main/java/org/briarproject/briar/blog/BlogModule.java
@@ -38,7 +38,6 @@ public class BlogModule {
ValidationManager validationManager) {
lifecycleManager.registerClient(blogManager);
- contactManager.registerAddContactHook(blogManager);
contactManager.registerRemoveContactHook(blogManager);
validationManager.registerIncomingMessageHook(CLIENT_ID, blogManager);
return blogManager;
diff --git a/briar-core/src/main/java/org/briarproject/briar/sharing/BlogSharingManagerImpl.java b/briar-core/src/main/java/org/briarproject/briar/sharing/BlogSharingManagerImpl.java
index 6664c67dcd..f095aba466 100644
--- a/briar-core/src/main/java/org/briarproject/briar/sharing/BlogSharingManagerImpl.java
+++ b/briar-core/src/main/java/org/briarproject/briar/sharing/BlogSharingManagerImpl.java
@@ -51,15 +51,23 @@ class BlogSharingManagerImpl extends SharingManagerImpl<Blog>
return CLIENT_ID;
}
+ /**
+ * This is called during each startup for each existing Contact.
+ */
@Override
public void addingContact(Transaction txn, Contact c) throws DbException {
+ // creates a group to share with the contact
super.addingContact(txn, c);
+
+ // get our blog and that of Contact c
LocalAuthor localAuthor = identityManager.getLocalAuthor(txn);
Blog ourBlog = blogManager.getPersonalBlog(localAuthor);
Blog theirBlog = blogManager.getPersonalBlog(c.getAuthor());
+
+ // pre-share both blogs, if they have not been shared already
try {
- initializeSharedSession(txn, c, ourBlog);
- initializeSharedSession(txn, c, theirBlog);
+ preShareShareable(txn, c, ourBlog);
+ preShareShareable(txn, c, theirBlog);
} catch (FormatException e) {
throw new DbException(e);
}
diff --git a/briar-core/src/main/java/org/briarproject/briar/sharing/SharingManagerImpl.java b/briar-core/src/main/java/org/briarproject/briar/sharing/SharingManagerImpl.java
index 461937caab..743fe46836 100644
--- a/briar-core/src/main/java/org/briarproject/briar/sharing/SharingManagerImpl.java
+++ b/briar-core/src/main/java/org/briarproject/briar/sharing/SharingManagerImpl.java
@@ -139,9 +139,24 @@ abstract class SharingManagerImpl<S extends Shareable>
return false;
}
- void initializeSharedSession(Transaction txn, Contact c, S shareable)
+ /**
+ * Adds the given Shareable and initializes a session between us
+ * and the Contact c in state SHARING.
+ * If a session already exists, this does nothing.
+ */
+ void preShareShareable(Transaction txn, Contact c, S shareable)
throws DbException, FormatException {
+ // return if a session already exists with that Contact
GroupId contactGroupId = getContactGroup(c).getId();
+ StoredSession existingSession = getSession(txn, contactGroupId,
+ getSessionId(shareable.getId()));
+ if (existingSession != null) return;
+
+ // add and shares the shareable with the Contact
+ db.addGroup(txn, shareable.getGroup());
+ db.setGroupVisibility(txn, c.getId(), shareable.getId(), SHARED);
+
+ // initialize session in sharing state
Session session = new Session(SHARING, contactGroupId,
shareable.getId(), null, null, 0, 0);
MessageId storageId = createStorageId(txn, contactGroupId);
diff --git a/briar-core/src/test/java/org/briarproject/briar/blog/BlogManagerImplTest.java b/briar-core/src/test/java/org/briarproject/briar/blog/BlogManagerImplTest.java
index b3c8c75862..2077a0902f 100644
--- a/briar-core/src/test/java/org/briarproject/briar/blog/BlogManagerImplTest.java
+++ b/briar-core/src/test/java/org/briarproject/briar/blog/BlogManagerImplTest.java
@@ -31,15 +31,11 @@ import org.jmock.Expectations;
import org.jmock.Mockery;
import org.junit.Test;
-import java.util.Collection;
-import java.util.Collections;
-
import static org.briarproject.bramble.api.identity.Author.Status.NONE;
import static org.briarproject.bramble.api.identity.Author.Status.OURSELVES;
import static org.briarproject.bramble.api.identity.Author.Status.VERIFIED;
import static org.briarproject.bramble.api.identity.AuthorConstants.MAX_AUTHOR_NAME_LENGTH;
import static org.briarproject.bramble.api.identity.AuthorConstants.MAX_PUBLIC_KEY_LENGTH;
-import static org.briarproject.bramble.api.sync.Group.Visibility.SHARED;
import static org.briarproject.bramble.api.sync.SyncConstants.MAX_MESSAGE_LENGTH;
import static org.briarproject.bramble.test.TestUtils.getRandomBytes;
import static org.briarproject.bramble.test.TestUtils.getRandomId;
@@ -117,29 +113,12 @@ public class BlogManagerImplTest extends BriarTestCase {
public void testCreateLocalState() throws DbException {
final Transaction txn = new Transaction(null, false);
- final ContactId contactId = new ContactId(0);
-
- Contact contact = new Contact(contactId, blog2.getAuthor(),
- blog1.getAuthor().getId(), true, true);
- final Collection<Contact> contacts = Collections.singletonList(contact);
-
context.checking(new Expectations() {{
oneOf(identityManager).getLocalAuthor(txn);
will(returnValue(blog1.getAuthor()));
oneOf(blogFactory).createBlog(blog1.getAuthor());
will(returnValue(blog1));
oneOf(db).addGroup(txn, blog1.getGroup());
- oneOf(db).getContacts(txn);
- will(returnValue(contacts));
- oneOf(blogFactory).createBlog(blog2.getAuthor());
- will(returnValue(blog2));
- oneOf(db).addGroup(txn, blog2.getGroup());
- oneOf(db).setGroupVisibility(txn, contactId, blog2.getId(), SHARED);
- oneOf(identityManager).getLocalAuthor(txn);
- will(returnValue(blog1.getAuthor()));
- oneOf(blogFactory).createBlog(blog1.getAuthor());
- will(returnValue(blog1));
- oneOf(db).setGroupVisibility(txn, contactId, blog1.getId(), SHARED);
}});
blogManager.createLocalState(txn);
diff --git a/briar-core/src/test/java/org/briarproject/briar/sharing/BlogSharingIntegrationTest.java b/briar-core/src/test/java/org/briarproject/briar/sharing/BlogSharingIntegrationTest.java
index b66b98124d..6a394f6d87 100644
--- a/briar-core/src/test/java/org/briarproject/briar/sharing/BlogSharingIntegrationTest.java
+++ b/briar-core/src/test/java/org/briarproject/briar/sharing/BlogSharingIntegrationTest.java
@@ -5,6 +5,7 @@ import net.jodah.concurrentunit.Waiter;
import org.briarproject.bramble.api.contact.Contact;
import org.briarproject.bramble.api.db.DbException;
import org.briarproject.bramble.api.db.NoSuchGroupException;
+import org.briarproject.bramble.api.db.Transaction;
import org.briarproject.bramble.api.event.Event;
import org.briarproject.bramble.api.event.EventListener;
import org.briarproject.bramble.api.nullsafety.NotNullByDefault;
@@ -492,6 +493,26 @@ public class BlogSharingIntegrationTest
// 1 can again share blog 1 with 0
assertTrue(
blogSharingManager1.canBeShared(blog1.getId(), contact0From1));
+
+ // re-create local state (simulates sign-out and sign-in)
+ Transaction txn0 = db0.startTransaction(false);
+ ((BlogSharingManagerImpl) blogSharingManager0).createLocalState(txn0);
+ db0.commitTransaction(txn0);
+ db0.endTransaction(txn0);
+ Transaction txn1 = db1.startTransaction(false);
+ ((BlogSharingManagerImpl) blogSharingManager1).createLocalState(txn1);
+ db1.commitTransaction(txn1);
+ db1.endTransaction(txn1);
+
+ // ensure there's no error with the sessions by checking shareability
+ assertFalse(
+ blogSharingManager0.canBeShared(blog1.getId(), contact1From0));
+ assertTrue(
+ blogSharingManager1.canBeShared(blog1.getId(), contact0From1));
+
+ // contacts should still be able to remove each other without errors
+ contactManager0.removeContact(contactId1From0);
+ contactManager1.removeContact(contactId0From1);
}
@Test
--
GitLab