From f9dfbe3fa528bf79f42e671e21350d20af5331b4 Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Tue, 26 Mar 2019 14:35:19 -0300 Subject: [PATCH 1/3] Don't show remote introduction responses after declining locally Fixes #1514 --- .../briar/introduction/IntroduceeProtocolEngine.java | 11 ----------- .../introduction/IntroductionIntegrationTest.java | 6 +++--- 2 files changed, 3 insertions(+), 14 deletions(-) diff --git a/briar-core/src/main/java/org/briarproject/briar/introduction/IntroduceeProtocolEngine.java b/briar-core/src/main/java/org/briarproject/briar/introduction/IntroduceeProtocolEngine.java index ae055bb9b..68a0a8d41 100644 --- a/briar-core/src/main/java/org/briarproject/briar/introduction/IntroduceeProtocolEngine.java +++ b/briar-core/src/main/java/org/briarproject/briar/introduction/IntroduceeProtocolEngine.java @@ -381,17 +381,6 @@ class IntroduceeProtocolEngine if (isInvalidDependency(s, m.getPreviousMessageId())) return abort(txn, s); - // Mark the response visible in the UI - markMessageVisibleInUi(txn, m.getMessageId()); - - // Track the incoming message - messageTracker - .trackMessage(txn, m.getGroupId(), m.getTimestamp(), false); - - // Broadcast IntroductionResponseReceivedEvent - broadcastIntroductionResponseReceivedEvent(txn, s, - s.getIntroducer().getId(), s.getRemote().author, m); - // Move to START state return IntroduceeSession.clear(s, START, s.getLastLocalMessageId(), s.getLocalTimestamp(), m.getMessageId()); diff --git a/briar-core/src/test/java/org/briarproject/briar/introduction/IntroductionIntegrationTest.java b/briar-core/src/test/java/org/briarproject/briar/introduction/IntroductionIntegrationTest.java index 4c65c6e9c..fe48ba387 100644 --- a/briar-core/src/test/java/org/briarproject/briar/introduction/IntroductionIntegrationTest.java +++ b/briar-core/src/test/java/org/briarproject/briar/introduction/IntroductionIntegrationTest.java @@ -399,7 +399,7 @@ public class IntroductionIntegrationTest assertEquals(3, messages.size()); messages = db2.transactionWithResult(true, txn -> introductionManager2.getMessageHeaders(txn, contactId0From2)); - assertEquals(3, messages.size()); + assertEquals(2, messages.size()); assertFalse(listener0.aborted); assertFalse(listener1.aborted); assertFalse(listener2.aborted); @@ -553,10 +553,10 @@ public class IntroductionIntegrationTest introductionManager0.getMessageHeaders(txn, contactId2From0)) .size()); assertGroupCount(messageTracker0, g2.getId(), 2, 1); - assertEquals(3, db1.transactionWithResult(true, txn -> + assertEquals(2, db1.transactionWithResult(true, txn -> introductionManager1.getMessageHeaders(txn, contactId0From1)) .size()); - assertGroupCount(messageTracker1, g1.getId(), 3, 2); + assertGroupCount(messageTracker1, g1.getId(), 2, 1); assertEquals(3, db2.transactionWithResult(true, txn -> introductionManager2.getMessageHeaders(txn, contactId0From2)) .size()); From 3b4a92f66c03ca72d4083772e7924144eef2b607 Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Tue, 26 Mar 2019 15:59:26 -0300 Subject: [PATCH 2/3] Fix introduction after one was declined When we received a remote decline we always went into the REMOTE_DECLINED state while there's two cases where we need to go into the START state instead. So when the new request arrived, we weren't in START and thus aborted the protocol. This commit fixes this. Fixes #1516 --- .../IntroduceeProtocolEngine.java | 11 +++-- .../IntroductionIntegrationTest.java | 45 +++++++++++++++++++ 2 files changed, 52 insertions(+), 4 deletions(-) diff --git a/briar-core/src/main/java/org/briarproject/briar/introduction/IntroduceeProtocolEngine.java b/briar-core/src/main/java/org/briarproject/briar/introduction/IntroduceeProtocolEngine.java index 68a0a8d41..995c64b99 100644 --- a/briar-core/src/main/java/org/briarproject/briar/introduction/IntroduceeProtocolEngine.java +++ b/briar-core/src/main/java/org/briarproject/briar/introduction/IntroduceeProtocolEngine.java @@ -365,10 +365,13 @@ class IntroduceeProtocolEngine broadcastIntroductionResponseReceivedEvent(txn, s, s.getIntroducer().getId(), s.getRemote().author, m); - // Move to REMOTE_DECLINED state - return IntroduceeSession.clear(s, REMOTE_DECLINED, - s.getLastLocalMessageId(), s.getLocalTimestamp(), - m.getMessageId()); + // Determine next state + IntroduceeState state = + s.getState() == AWAIT_RESPONSES ? REMOTE_DECLINED : START; + + // Move to the next state + return IntroduceeSession.clear(s, state, s.getLastLocalMessageId(), + s.getLocalTimestamp(), m.getMessageId()); } private IntroduceeSession onRemoteResponseWhenDeclined(Transaction txn, diff --git a/briar-core/src/test/java/org/briarproject/briar/introduction/IntroductionIntegrationTest.java b/briar-core/src/test/java/org/briarproject/briar/introduction/IntroductionIntegrationTest.java index fe48ba387..eafd3db73 100644 --- a/briar-core/src/test/java/org/briarproject/briar/introduction/IntroductionIntegrationTest.java +++ b/briar-core/src/test/java/org/briarproject/briar/introduction/IntroductionIntegrationTest.java @@ -405,6 +405,51 @@ public class IntroductionIntegrationTest assertFalse(listener2.aborted); } + @Test + public void testNewIntroductionAfterDecline() throws Exception { + addListeners(false, true); + + // make introduction + long time = clock.currentTimeMillis(); + introductionManager0 + .makeIntroduction(contact1From0, contact2From0, null, time); + + // sync request messages + sync0To1(1, true); + sync0To2(1, true); + eventWaiter.await(TIMEOUT, 2); + + // sync first response + sync1To0(1, true); + eventWaiter.await(TIMEOUT, 1); + + // sync second response + sync2To0(1, true); + eventWaiter.await(TIMEOUT, 1); + + // sync both forwarded response + sync0To2(1, true); + sync0To1(1, true); + eventWaiter.await(TIMEOUT, 1); + + assertFalse(listener0.aborted); + assertFalse(listener1.aborted); + assertFalse(listener2.aborted); + + time = clock.currentTimeMillis(); + introductionManager0 + .makeIntroduction(contact1From0, contact2From0, null, time); + + // sync request messages + sync0To1(1, true); + sync0To2(1, true); + eventWaiter.await(TIMEOUT, 2); + + assertFalse(listener0.aborted); + assertFalse(listener1.aborted); + assertFalse(listener2.aborted); + } + @Test public void testResponseAndAuthInOneSync() throws Exception { addListeners(true, true); From d40cfd30a2f480e8ca88143bd279ad9bcf9bff02 Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Tue, 26 Mar 2019 14:58:59 -0300 Subject: [PATCH 3/3] Let IntroductionResponse know if introduction can succeed and use this information in the android UI for showing that the user needs to wait or not. --- .../android/conversation/ConversationVisitor.java | 8 ++++---- .../briar/api/introduction/IntroductionResponse.java | 9 ++++++++- .../briar/introduction/AbstractProtocolEngine.java | 6 +++--- .../briar/introduction/IntroduceeProtocolEngine.java | 2 +- .../briar/introduction/IntroducerProtocolEngine.java | 8 ++++---- .../briar/introduction/IntroductionManagerImpl.java | 10 +++++++++- .../introduction/IntroductionIntegrationTest.java | 5 +++++ 7 files changed, 34 insertions(+), 14 deletions(-) diff --git a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationVisitor.java b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationVisitor.java index 3d6a85db9..87ec94f07 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationVisitor.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationVisitor.java @@ -252,12 +252,12 @@ class ConversationVisitor implements if (r.isLocal()) { String text; if (r.wasAccepted()) { + String suffix = r.canSucceed() ? "\n\n" + ctx.getString( + R.string.introduction_response_accepted_sent_info, + introducedAuthor) : ""; text = ctx.getString( R.string.introduction_response_accepted_sent, - introducedAuthor) - + "\n\n" + ctx.getString( - R.string.introduction_response_accepted_sent_info, - introducedAuthor); + introducedAuthor) + suffix; } else { text = ctx.getString( R.string.introduction_response_declined_sent, diff --git a/briar-api/src/main/java/org/briarproject/briar/api/introduction/IntroductionResponse.java b/briar-api/src/main/java/org/briarproject/briar/api/introduction/IntroductionResponse.java index 7e73da471..18c68c159 100644 --- a/briar-api/src/main/java/org/briarproject/briar/api/introduction/IntroductionResponse.java +++ b/briar-api/src/main/java/org/briarproject/briar/api/introduction/IntroductionResponse.java @@ -20,16 +20,18 @@ public class IntroductionResponse extends ConversationResponse { private final Author introducedAuthor; private final AuthorInfo introducedAuthorInfo; private final Role ourRole; + private final boolean canSucceed; public IntroductionResponse(MessageId messageId, GroupId groupId, long time, boolean local, boolean read, boolean sent, boolean seen, SessionId sessionId, boolean accepted, Author author, - AuthorInfo introducedAuthorInfo, Role role) { + AuthorInfo introducedAuthorInfo, Role role, boolean canSucceed) { super(messageId, groupId, time, local, read, sent, seen, sessionId, accepted); this.introducedAuthor = author; this.introducedAuthorInfo = introducedAuthorInfo; this.ourRole = role; + this.canSucceed = canSucceed; } public Author getIntroducedAuthor() { @@ -40,6 +42,10 @@ public class IntroductionResponse extends ConversationResponse { return introducedAuthorInfo; } + public boolean canSucceed() { + return canSucceed; + } + public boolean isIntroducer() { return ourRole == INTRODUCER; } @@ -48,4 +54,5 @@ public class IntroductionResponse extends ConversationResponse { public T accept(ConversationMessageVisitor v) { return v.visitIntroductionResponse(this); } + } diff --git a/briar-core/src/main/java/org/briarproject/briar/introduction/AbstractProtocolEngine.java b/briar-core/src/main/java/org/briarproject/briar/introduction/AbstractProtocolEngine.java index 53ad74861..635021e49 100644 --- a/briar-core/src/main/java/org/briarproject/briar/introduction/AbstractProtocolEngine.java +++ b/briar-core/src/main/java/org/briarproject/briar/introduction/AbstractProtocolEngine.java @@ -146,8 +146,8 @@ abstract class AbstractProtocolEngine } void broadcastIntroductionResponseReceivedEvent(Transaction txn, Session s, - AuthorId sender, Author otherAuthor, AbstractIntroductionMessage m) - throws DbException { + AuthorId sender, Author otherAuthor, AbstractIntroductionMessage m, + boolean canSucceed) throws DbException { AuthorId localAuthorId = identityManager.getLocalAuthor(txn).getId(); Contact c = contactManager.getContact(txn, sender, localAuthorId); AuthorInfo otherAuthorInfo = @@ -156,7 +156,7 @@ abstract class AbstractProtocolEngine new IntroductionResponse(m.getMessageId(), m.getGroupId(), m.getTimestamp(), false, false, false, false, s.getSessionId(), m instanceof AcceptMessage, - otherAuthor, otherAuthorInfo, s.getRole()); + otherAuthor, otherAuthorInfo, s.getRole(), canSucceed); IntroductionResponseReceivedEvent e = new IntroductionResponseReceivedEvent(response, c.getId()); txn.attach(e); diff --git a/briar-core/src/main/java/org/briarproject/briar/introduction/IntroduceeProtocolEngine.java b/briar-core/src/main/java/org/briarproject/briar/introduction/IntroduceeProtocolEngine.java index 995c64b99..5639e848f 100644 --- a/briar-core/src/main/java/org/briarproject/briar/introduction/IntroduceeProtocolEngine.java +++ b/briar-core/src/main/java/org/briarproject/briar/introduction/IntroduceeProtocolEngine.java @@ -363,7 +363,7 @@ class IntroduceeProtocolEngine // Broadcast IntroductionResponseReceivedEvent broadcastIntroductionResponseReceivedEvent(txn, s, - s.getIntroducer().getId(), s.getRemote().author, m); + s.getIntroducer().getId(), s.getRemote().author, m, false); // Determine next state IntroduceeState state = diff --git a/briar-core/src/main/java/org/briarproject/briar/introduction/IntroducerProtocolEngine.java b/briar-core/src/main/java/org/briarproject/briar/introduction/IntroducerProtocolEngine.java index 69e6579a9..fa468a939 100644 --- a/briar-core/src/main/java/org/briarproject/briar/introduction/IntroducerProtocolEngine.java +++ b/briar-core/src/main/java/org/briarproject/briar/introduction/IntroducerProtocolEngine.java @@ -294,7 +294,7 @@ class IntroducerProtocolEngine // Broadcast IntroductionResponseReceivedEvent broadcastIntroductionResponseReceivedEvent(txn, s, sender.getId(), - other, m); + other, m, true); // Move to the next state return new IntroducerSession(s.getSessionId(), state, @@ -349,7 +349,7 @@ class IntroducerProtocolEngine // Broadcast IntroductionResponseReceivedEvent broadcastIntroductionResponseReceivedEvent(txn, s, sender.getId(), - other, m); + other, m, false); return new IntroducerSession(s.getSessionId(), START, s.getRequestTimestamp(), introduceeA, introduceeB); @@ -403,7 +403,7 @@ class IntroducerProtocolEngine // Broadcast IntroductionResponseReceivedEvent broadcastIntroductionResponseReceivedEvent(txn, s, sender.getId(), - other, m); + other, m, false); return new IntroducerSession(s.getSessionId(), state, s.getRequestTimestamp(), introduceeA, introduceeB); @@ -451,7 +451,7 @@ class IntroducerProtocolEngine // Broadcast IntroductionResponseReceivedEvent broadcastIntroductionResponseReceivedEvent(txn, s, sender.getId(), - other, m); + other, m, false); return new IntroducerSession(s.getSessionId(), START, s.getRequestTimestamp(), introduceeA, introduceeB); diff --git a/briar-core/src/main/java/org/briarproject/briar/introduction/IntroductionManagerImpl.java b/briar-core/src/main/java/org/briarproject/briar/introduction/IntroductionManagerImpl.java index 560651ed6..fd9f00804 100644 --- a/briar-core/src/main/java/org/briarproject/briar/introduction/IntroductionManagerImpl.java +++ b/briar-core/src/main/java/org/briarproject/briar/introduction/IntroductionManagerImpl.java @@ -52,6 +52,9 @@ import javax.inject.Inject; import static org.briarproject.briar.api.introduction.Role.INTRODUCEE; import static org.briarproject.briar.api.introduction.Role.INTRODUCER; +import static org.briarproject.briar.introduction.IntroduceeState.REMOTE_DECLINED; +import static org.briarproject.briar.introduction.IntroducerState.A_DECLINED; +import static org.briarproject.briar.introduction.IntroducerState.B_DECLINED; import static org.briarproject.briar.introduction.IntroducerState.START; import static org.briarproject.briar.introduction.IntroductionConstants.GROUP_KEY_CONTACT_ID; import static org.briarproject.briar.introduction.MessageType.ABORT; @@ -466,6 +469,7 @@ class IntroductionManagerImpl extends ConversationClientImpl Role role = sessionParser.getRole(bdfSession); SessionId sessionId; Author author; + boolean canSucceed; if (role == INTRODUCER) { IntroducerSession session = sessionParser.parseIntroducerSession(bdfSession); @@ -475,11 +479,15 @@ class IntroductionManagerImpl extends ConversationClientImpl } else { author = session.getIntroduceeA().author; } + IntroducerState s = session.getState(); + canSucceed = s != START && s != A_DECLINED && s != B_DECLINED; } else if (role == INTRODUCEE) { IntroduceeSession session = sessionParser .parseIntroduceeSession(contactGroupId, bdfSession); sessionId = session.getSessionId(); author = session.getRemote().author; + IntroduceeState s = session.getState(); + canSucceed = s != IntroduceeState.START && s != REMOTE_DECLINED; } else throw new AssertionError(); AuthorInfo authorInfo = authorInfos.get(author.getId()); if (authorInfo == null) { @@ -488,7 +496,7 @@ class IntroductionManagerImpl extends ConversationClientImpl } return new IntroductionResponse(m, contactGroupId, meta.getTimestamp(), meta.isLocal(), meta.isRead(), status.isSent(), status.isSeen(), - sessionId, accept, author, authorInfo, role); + sessionId, accept, author, authorInfo, role, canSucceed); } private void removeSessionWithIntroducer(Transaction txn, diff --git a/briar-core/src/test/java/org/briarproject/briar/introduction/IntroductionIntegrationTest.java b/briar-core/src/test/java/org/briarproject/briar/introduction/IntroductionIntegrationTest.java index eafd3db73..06b4c22a9 100644 --- a/briar-core/src/test/java/org/briarproject/briar/introduction/IntroductionIntegrationTest.java +++ b/briar-core/src/test/java/org/briarproject/briar/introduction/IntroductionIntegrationTest.java @@ -185,6 +185,7 @@ public class IntroductionIntegrationTest assertEquals(introducee2.getAuthor().getName(), listener0.getResponse().getIntroducedAuthor().getName()); assertGroupCount(messageTracker0, g1.getId(), 2, 1); + assertTrue(listener0.getResponse().canSucceed()); // sync second ACCEPT message sync2To0(1, true); @@ -193,6 +194,7 @@ public class IntroductionIntegrationTest assertEquals(introducee1.getAuthor().getName(), listener0.getResponse().getIntroducedAuthor().getName()); assertGroupCount(messageTracker0, g2.getId(), 2, 1); + assertTrue(listener0.getResponse().canSucceed()); // sync forwarded ACCEPT messages to introducees sync0To1(1, true); @@ -290,6 +292,7 @@ public class IntroductionIntegrationTest // assert that the name on the decline event is correct assertEquals(introducee2.getAuthor().getName(), listener0.getResponse().getIntroducedAuthor().getName()); + assertFalse(listener0.getResponse().canSucceed()); // sync second response sync2To0(1, true); @@ -307,6 +310,7 @@ public class IntroductionIntegrationTest eventWaiter.await(TIMEOUT, 1); assertEquals(introducee1.getAuthor().getName(), listener2.getResponse().getIntroducedAuthor().getName()); + assertFalse(listener2.getResponse().canSucceed()); // note how the introducer does not forward the second response, // because after the first decline the protocol finished @@ -381,6 +385,7 @@ public class IntroductionIntegrationTest eventWaiter.await(TIMEOUT, 1); assertEquals(contact2From0.getAuthor().getName(), listener1.getResponse().getIntroducedAuthor().getName()); + assertFalse(listener1.getResponse().canSucceed()); assertFalse(contactManager1 .contactExists(author2.getId(), author1.getId()));