From 6e42377b747e53d20f704f5ba154aa0a62e361ea Mon Sep 17 00:00:00 2001
From: akwizgran <michael@briarproject.org>
Date: Sat, 28 Apr 2018 00:11:45 +0100
Subject: [PATCH] Don't automatically respond to declined introduction.
---
.../android/contact/ConversationActivity.java | 24 -----
.../IntroduceeProtocolEngine.java | 79 +++++++-------
.../briar/introduction/IntroduceeState.java | 9 +-
.../IntroductionIntegrationTest.java | 101 ++++--------------
4 files changed, 71 insertions(+), 142 deletions(-)
diff --git a/briar-android/src/main/java/org/briarproject/briar/android/contact/ConversationActivity.java b/briar-android/src/main/java/org/briarproject/briar/android/contact/ConversationActivity.java
index 67572ff2bd..f926fe54d7 100644
--- a/briar-android/src/main/java/org/briarproject/briar/android/contact/ConversationActivity.java
+++ b/briar-android/src/main/java/org/briarproject/briar/android/contact/ConversationActivity.java
@@ -575,10 +575,6 @@ public class ConversationActivity extends BriarActivity
@Override
public void onSuccess(String contactName) {
runOnUiThreadUnlessDestroyed(() -> {
- // If the other introducee declined, we can no longer
- // respond to the request
- if (!m.isIntroducer() && !m.wasAccepted())
- markRequestAnswered(m.getSessionId());
ConversationItem item = ConversationItem
.from(ConversationActivity.this, contactName, m);
addConversationItem(item);
@@ -631,26 +627,6 @@ public class ConversationActivity extends BriarActivity
});
}
- private void markRequestAnswered(SessionId sessionId) {
- int size = adapter.getItemCount();
- for (int i = 0; i < size; i++) {
- ConversationItem item = adapter.getItemAt(i);
- if (item instanceof ConversationRequestItem) {
- ConversationRequestItem req = (ConversationRequestItem) item;
- if (req.getSessionId().equals(sessionId)
- && !req.wasAnswered()) {
- LOG.info("Marking request answered");
- req.setAnswered(true);
- int position = adapter.findItemPosition(req);
- if (position != INVALID_POSITION)
- adapter.notifyItemChanged(position, req);
- // There shouldn't be more than one unanswered request
- return;
- }
- }
- }
- }
-
private void markMessages(Collection<MessageId> messageIds,
boolean sent, boolean seen) {
runOnUiThreadUnlessDestroyed(() -> {
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 e2ea32ed62..d93238dd8d 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
@@ -46,6 +46,7 @@ import static org.briarproject.briar.introduction.IntroduceeState.AWAIT_RESPONSE
import static org.briarproject.briar.introduction.IntroduceeState.LOCAL_ACCEPTED;
import static org.briarproject.briar.introduction.IntroduceeState.LOCAL_DECLINED;
import static org.briarproject.briar.introduction.IntroduceeState.REMOTE_ACCEPTED;
+import static org.briarproject.briar.introduction.IntroduceeState.REMOTE_DECLINED;
import static org.briarproject.briar.introduction.IntroduceeState.START;
@Immutable
@@ -94,6 +95,7 @@ class IntroduceeProtocolEngine
IntroduceeSession session, long timestamp) throws DbException {
switch (session.getState()) {
case AWAIT_RESPONSES:
+ case REMOTE_DECLINED:
case REMOTE_ACCEPTED:
return onLocalAccept(txn, session, timestamp);
case START:
@@ -112,6 +114,7 @@ class IntroduceeProtocolEngine
IntroduceeSession session, long timestamp) throws DbException {
switch (session.getState()) {
case AWAIT_RESPONSES:
+ case REMOTE_DECLINED:
case REMOTE_ACCEPTED:
return onLocalDecline(txn, session, timestamp);
case START:
@@ -133,6 +136,7 @@ class IntroduceeProtocolEngine
return onRemoteRequest(txn, session, m);
case AWAIT_RESPONSES:
case LOCAL_DECLINED:
+ case REMOTE_DECLINED:
case LOCAL_ACCEPTED:
case REMOTE_ACCEPTED:
case AWAIT_AUTH:
@@ -153,6 +157,7 @@ class IntroduceeProtocolEngine
case LOCAL_ACCEPTED:
return onRemoteAccept(txn, session, m);
case START:
+ case REMOTE_DECLINED:
case REMOTE_ACCEPTED:
case AWAIT_AUTH:
case AWAIT_ACTIVATE:
@@ -172,6 +177,7 @@ class IntroduceeProtocolEngine
case LOCAL_ACCEPTED:
return onRemoteDecline(txn, session, m);
case START:
+ case REMOTE_DECLINED:
case REMOTE_ACCEPTED:
case AWAIT_AUTH:
case AWAIT_ACTIVATE:
@@ -190,6 +196,7 @@ class IntroduceeProtocolEngine
case START:
case AWAIT_RESPONSES:
case LOCAL_DECLINED:
+ case REMOTE_DECLINED:
case LOCAL_ACCEPTED:
case REMOTE_ACCEPTED:
case AWAIT_ACTIVATE:
@@ -208,6 +215,7 @@ class IntroduceeProtocolEngine
case START:
case AWAIT_RESPONSES:
case LOCAL_DECLINED:
+ case REMOTE_DECLINED:
case LOCAL_ACCEPTED:
case REMOTE_ACCEPTED:
case AWAIT_AUTH:
@@ -272,26 +280,28 @@ class IntroduceeProtocolEngine
transportPropertyManager.getLocalProperties(txn);
// Send a ACCEPT message
- long localTimestamp =
- Math.max(timestamp + 1, getLocalTimestamp(s));
+ long localTimestamp = Math.max(timestamp + 1, getLocalTimestamp(s));
Message sent = sendAcceptMessage(txn, s, localTimestamp, publicKey,
localTimestamp, transportProperties, true);
// Track the message
messageTracker.trackOutgoingMessage(txn, sent);
// Determine the next state
- IntroduceeState state =
- s.getState() == AWAIT_RESPONSES ? LOCAL_ACCEPTED : AWAIT_AUTH;
- IntroduceeSession sNew = IntroduceeSession
- .addLocalAccept(s, state, sent, publicKey, privateKey,
- localTimestamp, transportProperties);
-
- if (state == AWAIT_AUTH) {
- // Move to the AWAIT_AUTH state
- return onLocalAuth(txn, sNew);
+ switch (s.getState()) {
+ case AWAIT_RESPONSES:
+ return IntroduceeSession.addLocalAccept(s, LOCAL_ACCEPTED, sent,
+ publicKey, privateKey, localTimestamp,
+ transportProperties);
+ case REMOTE_DECLINED:
+ return IntroduceeSession.clear(s, START, sent.getId(),
+ localTimestamp, s.getLastRemoteMessageId());
+ case REMOTE_ACCEPTED:
+ return onLocalAuth(txn, IntroduceeSession.addLocalAccept(s,
+ AWAIT_AUTH, sent, publicKey, privateKey, localTimestamp,
+ transportProperties));
+ default:
+ throw new AssertionError();
}
- // Move to the LOCAL_ACCEPTED state
- return sNew;
}
private IntroduceeSession onLocalDecline(Transaction txn,
@@ -308,10 +318,9 @@ class IntroduceeProtocolEngine
// Move to the START or LOCAL_DECLINED state, if still awaiting response
IntroduceeState state =
- s.getState() == REMOTE_ACCEPTED ? START : LOCAL_DECLINED;
- return IntroduceeSession
- .clear(s, state, sent.getId(), sent.getTimestamp(),
- s.getLastRemoteMessageId());
+ s.getState() == AWAIT_RESPONSES ? LOCAL_DECLINED : START;
+ return IntroduceeSession.clear(s, state, sent.getId(),
+ sent.getTimestamp(), s.getLastRemoteMessageId());
}
private IntroduceeSession onRemoteAccept(Transaction txn,
@@ -357,22 +366,10 @@ class IntroduceeProtocolEngine
broadcastIntroductionResponseReceivedEvent(txn, s,
s.getIntroducer().getId(), s.getRemote().author, m);
- if (s.getState() == AWAIT_RESPONSES) {
- // Mark the request message unavailable to answer
- markRequestsUnavailableToAnswer(txn, s);
-
- // Send a DECLINE message
- Message sent =
- sendDeclineMessage(txn, s, getLocalTimestamp(s), false);
-
- // Move back to START state
- return IntroduceeSession.clear(s, START, sent.getId(),
- sent.getTimestamp(), m.getMessageId());
- } else {
- // Move back to START state
- return IntroduceeSession.clear(s, START, s.getLastLocalMessageId(),
- s.getLocalTimestamp(), m.getMessageId());
- }
+ // Move to REMOTE_DECLINED state
+ return IntroduceeSession.clear(s, REMOTE_DECLINED,
+ s.getLastLocalMessageId(), s.getLocalTimestamp(),
+ m.getMessageId());
}
private IntroduceeSession onRemoteResponseWhenDeclined(Transaction txn,
@@ -385,6 +382,17 @@ 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());
@@ -525,9 +533,8 @@ class IntroduceeProtocolEngine
txn.attach(new IntroductionAbortedEvent(s.getSessionId()));
// Reset the session back to initial state
- return IntroduceeSession
- .clear(s, START, sent.getId(), sent.getTimestamp(),
- s.getLastRemoteMessageId());
+ return IntroduceeSession.clear(s, START, sent.getId(),
+ sent.getTimestamp(), s.getLastRemoteMessageId());
}
private boolean isInvalidDependency(IntroduceeSession s,
diff --git a/briar-core/src/main/java/org/briarproject/briar/introduction/IntroduceeState.java b/briar-core/src/main/java/org/briarproject/briar/introduction/IntroduceeState.java
index 7a54abde8d..3a3e16af36 100644
--- a/briar-core/src/main/java/org/briarproject/briar/introduction/IntroduceeState.java
+++ b/briar-core/src/main/java/org/briarproject/briar/introduction/IntroduceeState.java
@@ -12,10 +12,11 @@ enum IntroduceeState implements State {
START(0),
AWAIT_RESPONSES(1),
LOCAL_DECLINED(2),
- LOCAL_ACCEPTED(3),
- REMOTE_ACCEPTED(4),
- AWAIT_AUTH(5),
- AWAIT_ACTIVATE(6);
+ REMOTE_DECLINED(3),
+ LOCAL_ACCEPTED(4),
+ REMOTE_ACCEPTED(5),
+ AWAIT_AUTH(6),
+ AWAIT_ACTIVATE(7);
private final int value;
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 b8e5b646bc..7441186e8b 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
@@ -17,7 +17,6 @@ import org.briarproject.bramble.api.identity.Author;
import org.briarproject.bramble.api.nullsafety.MethodsNotNullByDefault;
import org.briarproject.bramble.api.nullsafety.NotNullByDefault;
import org.briarproject.bramble.api.nullsafety.ParametersNotNullByDefault;
-import org.briarproject.bramble.api.properties.TransportProperties;
import org.briarproject.bramble.api.properties.TransportPropertyManager;
import org.briarproject.bramble.api.sync.Group;
import org.briarproject.bramble.api.sync.Message;
@@ -39,15 +38,12 @@ import org.junit.Test;
import java.io.IOException;
import java.util.Collection;
-import java.util.Collections;
import java.util.Map;
import java.util.concurrent.TimeoutException;
-import java.util.logging.Logger;
import static org.briarproject.bramble.api.identity.AuthorConstants.MAX_PUBLIC_KEY_LENGTH;
import static org.briarproject.bramble.test.TestPluginConfigModule.TRANSPORT_ID;
import static org.briarproject.bramble.test.TestUtils.getRandomBytes;
-import static org.briarproject.bramble.test.TestUtils.getTransportId;
import static org.briarproject.bramble.test.TestUtils.getTransportProperties;
import static org.briarproject.bramble.test.TestUtils.getTransportPropertiesMap;
import static org.briarproject.briar.api.introduction.IntroductionManager.CLIENT_ID;
@@ -86,9 +82,6 @@ public class IntroductionIntegrationTest
private IntroduceeListener listener1;
private IntroduceeListener listener2;
- private static final Logger LOG =
- Logger.getLogger(IntroductionIntegrationTest.class.getName());
-
interface StateVisitor {
AcceptMessage visit(AcceptMessage response);
}
@@ -374,11 +367,10 @@ public class IntroductionIntegrationTest
assertEquals(2,
introductionManager0.getIntroductionMessages(contactId2From0)
.size());
- // introducee1 also sees the decline response from introducee2
assertEquals(3,
introductionManager1.getIntroductionMessages(contactId0From1)
.size());
- assertEquals(2,
+ assertEquals(3,
introductionManager2.getIntroductionMessages(contactId0From2)
.size());
assertFalse(listener0.aborted);
@@ -386,58 +378,6 @@ public class IntroductionIntegrationTest
assertFalse(listener2.aborted);
}
- @Test
- public void testIntroductionSessionDelayedFirstDecline() throws Exception {
- addListeners(false, false);
-
- // make introduction
- long time = clock.currentTimeMillis();
- introductionManager0
- .makeIntroduction(contact1From0, contact2From0, null, time);
-
- // sync request messages
- sync0To1(1, true);
- sync0To2(1, true);
-
- // wait for requests to arrive
- eventWaiter.await(TIMEOUT, 2);
- assertTrue(listener1.requestReceived);
- assertTrue(listener2.requestReceived);
-
- // sync first response
- sync1To0(1, true);
- eventWaiter.await(TIMEOUT, 1);
- assertTrue(listener0.response1Received);
-
- // sync fake transport properties back to 1, so Message ACK can arrive
- // and the assertDefaultUiMessages() check at the end will not fail
- TransportProperties tp = new TransportProperties(
- Collections.singletonMap("key", "value"));
- c0.getTransportPropertyManager()
- .mergeLocalProperties(getTransportId(), tp);
- sync0To1(1, true);
-
- // sync second response
- sync2To0(1, true);
- eventWaiter.await(TIMEOUT, 1);
- assertTrue(listener0.response2Received);
-
- // sync first forwarded response
- sync0To2(1, true);
-
- // note how the second response will not be forwarded anymore
-
- assertFalse(contactManager1
- .contactExists(author2.getId(), author1.getId()));
- assertFalse(contactManager2
- .contactExists(author1.getId(), author2.getId()));
-
- assertDefaultUiMessages();
- assertFalse(listener0.aborted);
- assertFalse(listener1.aborted);
- assertFalse(listener2.aborted);
- }
-
@Test
public void testResponseAndAuthInOneSync() throws Exception {
addListeners(true, true);
@@ -467,9 +407,8 @@ public class IntroductionIntegrationTest
assertTrue(listener2.requestReceived);
// answer request manually
- introductionManager2
- .respondToIntroduction(contactId0From2, listener2.sessionId, time,
- true);
+ introductionManager2.respondToIntroduction(contactId0From2,
+ listener2.sessionId, time, true);
// sync second response and AUTH
sync2To0(2, true);
@@ -479,7 +418,7 @@ public class IntroductionIntegrationTest
// Forward AUTH
sync0To1(1, true);
- // Second AUTH and ACTIATE and forward them
+ // Second AUTH and ACTIVATE and forward them
sync1To0(2, true);
sync0To2(2, true);
@@ -495,13 +434,12 @@ public class IntroductionIntegrationTest
}
/**
- * When an introducee declines an introduction,
- * the other introducee needs to respond before returning to START state,
- * otherwise a subsequent attempt at introducing the same contacts will fail
+ * When an introducee declines an introduction, the other introducee needs
+ * to respond before returning to the START state, otherwise a subsequent
+ * attempt at introducing the same contacts will fail.
*/
@Test
- public void testAutomaticSecondDecline() throws Exception {
- // introducee1 declines automatically and introducee2 doesn't answer
+ public void testIntroductionSessionManualDecline() throws Exception {
addListeners(false, true);
listener2.answerRequests = false;
@@ -547,9 +485,18 @@ public class IntroductionIntegrationTest
// assert that introducee2 is in correct state
introduceeSession = getIntroduceeSession(c2);
+ assertEquals(IntroduceeState.REMOTE_DECLINED,
+ introduceeSession.getState());
+
+ // answer request manually
+ introductionManager2.respondToIntroduction(contactId0From2,
+ listener2.sessionId, time, false);
+
+ // now introducee2 should have returned to the START state
+ introduceeSession = getIntroduceeSession(c2);
assertEquals(IntroduceeState.START, introduceeSession.getState());
- // second response should be an immediate automatic DECLINE
+ // sync second response
sync2To0(1, true);
eventWaiter.await(TIMEOUT, 1);
assertTrue(listener0.response2Received);
@@ -562,7 +509,7 @@ public class IntroductionIntegrationTest
introduceeSession = getIntroduceeSession(c1);
assertEquals(LOCAL_DECLINED, introduceeSession.getState());
- // forward automatic decline
+ // forward second response
sync0To1(1, true);
// introducee1 can finally move to the START
@@ -579,16 +526,14 @@ public class IntroductionIntegrationTest
introductionManager0.getIntroductionMessages(contactId2From0)
.size());
assertGroupCount(messageTracker0, g2.getId(), 2, 1);
- assertEquals(2,
+ assertEquals(3,
introductionManager1.getIntroductionMessages(contactId0From1)
.size());
- assertGroupCount(messageTracker1, g1.getId(), 2, 1);
- // the automatic DECLINE is invisible in the UI
- // so there's just the remote REQUEST and remote DECLINE
- assertEquals(2,
+ assertGroupCount(messageTracker1, g1.getId(), 3, 2);
+ assertEquals(3,
introductionManager2.getIntroductionMessages(contactId0From2)
.size());
- assertGroupCount(messageTracker2, g2.getId(), 2, 2);
+ assertGroupCount(messageTracker2, g2.getId(), 3, 2);
assertFalse(listener0.aborted);
assertFalse(listener1.aborted);
--
GitLab