From 835e9f6994b6793b6aca482ae50caae8e07d23e3 Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Wed, 15 Dec 2021 14:44:06 -0300 Subject: [PATCH 1/3] Add mailbox API endpoint for adding a contact --- .../briarproject/bramble/test/TestUtils.java | 6 ++ .../bramble/mailbox/MailboxApi.java | 35 ++++++++++ .../bramble/mailbox/MailboxApiImpl.java | 22 +++++++ .../bramble/mailbox/MailboxApiTest.java | 65 +++++++++++++++++-- 4 files changed, 124 insertions(+), 4 deletions(-) diff --git a/bramble-api/src/test/java/org/briarproject/bramble/test/TestUtils.java b/bramble-api/src/test/java/org/briarproject/bramble/test/TestUtils.java index 3e171904e..3fd19912a 100644 --- a/bramble-api/src/test/java/org/briarproject/bramble/test/TestUtils.java +++ b/bramble-api/src/test/java/org/briarproject/bramble/test/TestUtils.java @@ -31,6 +31,7 @@ import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.Random; import java.util.concurrent.atomic.AtomicInteger; @@ -46,6 +47,7 @@ import static org.briarproject.bramble.api.sync.ClientId.MAX_CLIENT_ID_LENGTH; import static org.briarproject.bramble.api.sync.SyncConstants.MAX_GROUP_DESCRIPTOR_LENGTH; import static org.briarproject.bramble.api.sync.SyncConstants.MAX_MESSAGE_BODY_LENGTH; import static org.briarproject.bramble.util.StringUtils.getRandomString; +import static org.briarproject.bramble.util.StringUtils.toHexString; public class TestUtils { @@ -209,6 +211,10 @@ public class TestUtils { getAgreementPublicKey(), verified); } + public static String getMailboxSecret() { + return toHexString(getRandomBytes(32)).toLowerCase(Locale.US); + } + public static double getMedian(Collection samples) { int size = samples.size(); if (size == 0) throw new IllegalArgumentException(); diff --git a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxApi.java b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxApi.java index 6c5999990..21bd43593 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxApi.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxApi.java @@ -1,5 +1,6 @@ package org.briarproject.bramble.mailbox; +import org.briarproject.bramble.api.contact.ContactId; import org.briarproject.bramble.api.mailbox.MailboxProperties; import java.io.IOException; @@ -27,7 +28,41 @@ interface MailboxApi { boolean checkStatus(MailboxProperties properties) throws IOException, PermanentFailureException; + /** + * Adds a new contact to the mailbox. + * + * @throws TolerableFailureException if response code is 409 + * (contact was already added). + */ + void addContact(MailboxProperties properties, MailboxContact contact) + throws IOException, PermanentFailureException, + TolerableFailureException; + + @Immutable + class MailboxContact { + public final int contactId; + public final String token, inboxId, outboxId; + + MailboxContact(ContactId contactId, + String token, + String inboxId, + String outboxId) { + this.contactId = contactId.getInt(); + this.token = token; + this.inboxId = inboxId; + this.outboxId = outboxId; + } + } + @Immutable class PermanentFailureException extends Exception { } + + /** + * A failure that does not need to be retried, + * e.g. when adding a contact that already exists. + */ + @Immutable + class TolerableFailureException extends Exception { + } } diff --git a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxApiImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxApiImpl.java index 3c0906fb5..007ef468d 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxApiImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxApiImpl.java @@ -12,12 +12,15 @@ import java.io.IOException; import javax.inject.Inject; +import okhttp3.MediaType; import okhttp3.OkHttpClient; import okhttp3.Request; +import okhttp3.RequestBody; import okhttp3.Response; import okhttp3.ResponseBody; import static com.fasterxml.jackson.databind.MapperFeature.BLOCK_UNSAFE_POLYMORPHIC_BASE_TYPES; +import static java.util.Objects.requireNonNull; import static okhttp3.internal.Util.EMPTY_REQUEST; import static org.briarproject.bramble.util.StringUtils.fromHexString; @@ -28,6 +31,8 @@ class MailboxApiImpl implements MailboxApi { private final JsonMapper mapper = JsonMapper.builder() .enable(BLOCK_UNSAFE_POLYMORPHIC_BASE_TYPES) .build(); + private static final MediaType JSON = + requireNonNull(MediaType.parse("application/json; charset=utf-8")); @Inject MailboxApiImpl(WeakSingletonProvider httpClientProvider) { @@ -89,6 +94,23 @@ class MailboxApiImpl implements MailboxApi { return response.isSuccessful(); } + @Override + public void addContact(MailboxProperties properties, MailboxContact contact) + throws IOException, PermanentFailureException, + TolerableFailureException { + if (!properties.isOwner()) throw new IllegalArgumentException(); + byte[] bodyBytes = mapper.writeValueAsBytes(contact); + RequestBody body = RequestBody.create(JSON, bodyBytes); + Request request = getRequestBuilder(properties.getAuthToken()) + .url(properties.getOnionAddress() + "/contacts") + .post(body) + .build(); + OkHttpClient client = httpClientProvider.get(); + Response response = client.newCall(request).execute(); + if (response.code() == 409) throw new TolerableFailureException(); + if (!response.isSuccessful()) throw new IOException(); + } + private Request.Builder getRequestBuilder(String token) { return new Request.Builder() .addHeader("Authorization", "Bearer " + token); diff --git a/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxApiTest.java b/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxApiTest.java index c74570a62..c61e1613d 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxApiTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxApiTest.java @@ -1,11 +1,16 @@ package org.briarproject.bramble.mailbox; import org.briarproject.bramble.api.WeakSingletonProvider; +import org.briarproject.bramble.api.contact.ContactId; import org.briarproject.bramble.api.mailbox.MailboxProperties; +import org.briarproject.bramble.mailbox.MailboxApi.MailboxContact; import org.briarproject.bramble.mailbox.MailboxApi.PermanentFailureException; +import org.briarproject.bramble.mailbox.MailboxApi.TolerableFailureException; import org.briarproject.bramble.test.BrambleTestCase; import org.junit.Test; +import java.io.IOException; + import javax.annotation.Nonnull; import javax.net.SocketFactory; @@ -15,9 +20,9 @@ import okhttp3.mockwebserver.MockWebServer; import okhttp3.mockwebserver.RecordedRequest; import static java.util.concurrent.TimeUnit.MILLISECONDS; -import static org.briarproject.bramble.test.TestUtils.getRandomBytes; +import static org.briarproject.bramble.test.TestUtils.getContactId; +import static org.briarproject.bramble.test.TestUtils.getMailboxSecret; import static org.briarproject.bramble.util.StringUtils.getRandomString; -import static org.briarproject.bramble.util.StringUtils.toHexString; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; @@ -40,8 +45,14 @@ public class MailboxApiTest extends BrambleTestCase { }; private final MailboxApiImpl api = new MailboxApiImpl(httpClientProvider); - private final String token = toHexString(getRandomBytes(32)); - private final String token2 = toHexString(getRandomBytes(32)); + private final String token = getMailboxSecret(); + private final String token2 = getMailboxSecret(); + private final ContactId contactId = getContactId(); + private final String contactToken = getMailboxSecret(); + private final String contactInboxId = getMailboxSecret(); + private final String contactOutboxId = getMailboxSecret(); + private final MailboxContact mailboxContact = new MailboxContact( + contactId, contactToken, contactInboxId, contactOutboxId); @Test public void testSetup() throws Exception { @@ -175,6 +186,52 @@ public class MailboxApiTest extends BrambleTestCase { ); } + @Test + public void testAddContact() throws Exception { + MockWebServer server = new MockWebServer(); + server.enqueue(new MockResponse()); + server.enqueue(new MockResponse().setResponseCode(401)); + server.enqueue(new MockResponse().setResponseCode(409)); + server.start(); + String baseUrl = getBaseUrl(server); + MailboxProperties properties = + new MailboxProperties(baseUrl, token, true); + + // contact gets added as expected + api.addContact(properties, mailboxContact); + RecordedRequest request1 = server.takeRequest(); + assertEquals("/contacts", request1.getPath()); + assertToken(request1, token); + String expected = "{\"contactId\":" + contactId.getInt() + + ",\"token\":\"" + contactToken + + "\",\"inboxId\":\"" + contactInboxId + + "\",\"outboxId\":\"" + contactOutboxId + + "\"}"; + assertEquals(expected, request1.getBody().readUtf8()); + + // request is not successful + assertThrows(IOException.class, () -> + api.addContact(properties, mailboxContact)); + RecordedRequest request2 = server.takeRequest(); + assertEquals("/contacts", request2.getPath()); + assertToken(request2, token); + + // contact already exists + assertThrows(TolerableFailureException.class, () -> + api.addContact(properties, mailboxContact)); + RecordedRequest request3 = server.takeRequest(); + assertEquals("/contacts", request3.getPath()); + assertToken(request3, token); + } + + @Test + public void testAddContactOnlyForOwner() { + MailboxProperties properties = + new MailboxProperties("", token, false); + assertThrows(IllegalArgumentException.class, () -> + api.addContact(properties, mailboxContact)); + } + private String getBaseUrl(MockWebServer server) { String baseUrl = server.url("").toString(); return baseUrl.substring(0, baseUrl.length() - 1); From e52c5ddc8e742cbafc29286bc959bb596ef2b492 Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Fri, 7 Jan 2022 10:12:15 -0300 Subject: [PATCH 2/3] Rename PermanentFailureException to ApiException --- .../bramble/mailbox/MailboxApi.java | 12 ++++----- .../bramble/mailbox/MailboxApiImpl.java | 20 +++++++------- .../bramble/mailbox/MailboxApiTest.java | 26 ++++++------------- 3 files changed, 24 insertions(+), 34 deletions(-) diff --git a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxApi.java b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxApi.java index 21bd43593..34a740c3e 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxApi.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxApi.java @@ -14,19 +14,19 @@ interface MailboxApi { * * @param properties MailboxProperties with the setup token * @return the owner token - * @throws PermanentFailureException for 401 response. + * @throws ApiException for 401 response. */ String setup(MailboxProperties properties) - throws IOException, PermanentFailureException; + throws IOException, ApiException; /** * Checks the status of the mailbox. * * @return true if the status is OK, false otherwise. - * @throws PermanentFailureException for 401 response. + * @throws ApiException for 401 response. */ boolean checkStatus(MailboxProperties properties) - throws IOException, PermanentFailureException; + throws IOException, ApiException; /** * Adds a new contact to the mailbox. @@ -35,7 +35,7 @@ interface MailboxApi { * (contact was already added). */ void addContact(MailboxProperties properties, MailboxContact contact) - throws IOException, PermanentFailureException, + throws IOException, ApiException, TolerableFailureException; @Immutable @@ -55,7 +55,7 @@ interface MailboxApi { } @Immutable - class PermanentFailureException extends Exception { + class ApiException extends Exception { } /** diff --git a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxApiImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxApiImpl.java index 007ef468d..74012ae51 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxApiImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxApiImpl.java @@ -41,7 +41,7 @@ class MailboxApiImpl implements MailboxApi { @Override public String setup(MailboxProperties properties) - throws IOException, PermanentFailureException { + throws IOException, ApiException { if (!properties.isOwner()) throw new IllegalArgumentException(); Request request = getRequestBuilder(properties.getAuthToken()) .url(properties.getOnionAddress() + "/setup") @@ -50,23 +50,23 @@ class MailboxApiImpl implements MailboxApi { OkHttpClient client = httpClientProvider.get(); Response response = client.newCall(request).execute(); // TODO consider throwing a special exception for the 401 case - if (response.code() == 401) throw new PermanentFailureException(); - if (!response.isSuccessful()) throw new PermanentFailureException(); + if (response.code() == 401) throw new ApiException(); + if (!response.isSuccessful()) throw new ApiException(); ResponseBody body = response.body(); - if (body == null) throw new PermanentFailureException(); + if (body == null) throw new ApiException(); try { JsonNode node = mapper.readTree(body.string()); JsonNode tokenNode = node.get("token"); if (tokenNode == null) { - throw new PermanentFailureException(); + throw new ApiException(); } String ownerToken = tokenNode.textValue(); if (ownerToken == null || !isValidToken(ownerToken)) { - throw new PermanentFailureException(); + throw new ApiException(); } return ownerToken; } catch (JacksonException e) { - throw new PermanentFailureException(); + throw new ApiException(); } } @@ -83,20 +83,20 @@ class MailboxApiImpl implements MailboxApi { @Override public boolean checkStatus(MailboxProperties properties) - throws IOException, PermanentFailureException { + throws IOException, ApiException { if (!properties.isOwner()) throw new IllegalArgumentException(); Request request = getRequestBuilder(properties.getAuthToken()) .url(properties.getOnionAddress() + "/status") .build(); OkHttpClient client = httpClientProvider.get(); Response response = client.newCall(request).execute(); - if (response.code() == 401) throw new PermanentFailureException(); + if (response.code() == 401) throw new ApiException(); return response.isSuccessful(); } @Override public void addContact(MailboxProperties properties, MailboxContact contact) - throws IOException, PermanentFailureException, + throws IOException, ApiException, TolerableFailureException { if (!properties.isOwner()) throw new IllegalArgumentException(); byte[] bodyBytes = mapper.writeValueAsBytes(contact); diff --git a/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxApiTest.java b/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxApiTest.java index c61e1613d..5ab0bcbf2 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxApiTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxApiTest.java @@ -3,8 +3,8 @@ package org.briarproject.bramble.mailbox; import org.briarproject.bramble.api.WeakSingletonProvider; import org.briarproject.bramble.api.contact.ContactId; import org.briarproject.bramble.api.mailbox.MailboxProperties; +import org.briarproject.bramble.mailbox.MailboxApi.ApiException; import org.briarproject.bramble.mailbox.MailboxApi.MailboxContact; -import org.briarproject.bramble.mailbox.MailboxApi.PermanentFailureException; import org.briarproject.bramble.mailbox.MailboxApi.TolerableFailureException; import org.briarproject.bramble.test.BrambleTestCase; import org.junit.Test; @@ -85,50 +85,42 @@ public class MailboxApiTest extends BrambleTestCase { assertToken(request1, token); // empty body - assertThrows(PermanentFailureException.class, - () -> api.setup(properties)); + assertThrows(ApiException.class, () -> api.setup(properties)); RecordedRequest request2 = server.takeRequest(); assertEquals("/setup", request2.getPath()); assertEquals("PUT", request2.getMethod()); assertToken(request2, token); // invalid response - assertThrows(PermanentFailureException.class, - () -> api.setup(properties)); + assertThrows(ApiException.class, () -> api.setup(properties)); RecordedRequest request3 = server.takeRequest(); assertEquals("/setup", request3.getPath()); assertEquals("PUT", request3.getMethod()); assertToken(request3, token); // 401 response - assertThrows(PermanentFailureException.class, - () -> api.setup(properties2)); + assertThrows(ApiException.class, () -> api.setup(properties2)); RecordedRequest request4 = server.takeRequest(); assertEquals("/setup", request4.getPath()); assertEquals("PUT", request4.getMethod()); assertToken(request4, token2); // 500 response - assertThrows(PermanentFailureException.class, - () -> api.setup(properties)); + assertThrows(ApiException.class, () -> api.setup(properties)); RecordedRequest request5 = server.takeRequest(); assertEquals("/setup", request5.getPath()); assertEquals("PUT", request5.getMethod()); assertToken(request5, token); // invalid json dict token response - assertThrows(PermanentFailureException.class, - () -> api.setup(properties) - ); + assertThrows(ApiException.class, () -> api.setup(properties)); RecordedRequest request6 = server.takeRequest(); assertEquals("/setup", request6.getPath()); assertEquals("PUT", request6.getMethod()); assertToken(request6, token); // invalid non-hex string token response - assertThrows(PermanentFailureException.class, - () -> api.setup(properties) - ); + assertThrows(ApiException.class, () -> api.setup(properties)); RecordedRequest request7 = server.takeRequest(); assertEquals("/setup", request7.getPath()); assertEquals("PUT", request7.getMethod()); @@ -163,9 +155,7 @@ public class MailboxApiTest extends BrambleTestCase { assertEquals("/status", request1.getPath()); assertToken(request1, token); - assertThrows(PermanentFailureException.class, () -> - api.checkStatus(properties2) - ); + assertThrows(ApiException.class, () -> api.checkStatus(properties2)); RecordedRequest request2 = server.takeRequest(); assertEquals("/status", request2.getPath()); assertToken(request2, token2); From f400cf5aa0e57122ca3282a6f2f2f35ab3730bb5 Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Fri, 7 Jan 2022 10:22:22 -0300 Subject: [PATCH 3/3] Throw ApiException when adding contact is not successful --- .../java/org/briarproject/bramble/mailbox/MailboxApiImpl.java | 2 +- .../java/org/briarproject/bramble/mailbox/MailboxApiTest.java | 4 +--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxApiImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxApiImpl.java index 74012ae51..218283a42 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxApiImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxApiImpl.java @@ -108,7 +108,7 @@ class MailboxApiImpl implements MailboxApi { OkHttpClient client = httpClientProvider.get(); Response response = client.newCall(request).execute(); if (response.code() == 409) throw new TolerableFailureException(); - if (!response.isSuccessful()) throw new IOException(); + if (!response.isSuccessful()) throw new ApiException(); } private Request.Builder getRequestBuilder(String token) { diff --git a/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxApiTest.java b/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxApiTest.java index 5ab0bcbf2..5f4399ef3 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxApiTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxApiTest.java @@ -9,8 +9,6 @@ import org.briarproject.bramble.mailbox.MailboxApi.TolerableFailureException; import org.briarproject.bramble.test.BrambleTestCase; import org.junit.Test; -import java.io.IOException; - import javax.annotation.Nonnull; import javax.net.SocketFactory; @@ -200,7 +198,7 @@ public class MailboxApiTest extends BrambleTestCase { assertEquals(expected, request1.getBody().readUtf8()); // request is not successful - assertThrows(IOException.class, () -> + assertThrows(ApiException.class, () -> api.addContact(properties, mailboxContact)); RecordedRequest request2 = server.takeRequest(); assertEquals("/contacts", request2.getPath());