From 82443d9708ae17291baddff4556bf467d2a1455f Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Fri, 7 Jan 2022 10:46:43 -0300 Subject: [PATCH 1/3] Add method for deleting a contact from own mailbox --- .../bramble/mailbox/MailboxApi.java | 13 +++++- .../bramble/mailbox/MailboxApiImpl.java | 16 +++++++ .../bramble/mailbox/MailboxApiTest.java | 43 +++++++++++++++++++ 3 files changed, 70 insertions(+), 2 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 34a740c3e..4ccaa4c90 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,7 @@ package org.briarproject.bramble.mailbox; +import com.fasterxml.jackson.databind.annotation.JsonSerialize; + import org.briarproject.bramble.api.contact.ContactId; import org.briarproject.bramble.api.mailbox.MailboxProperties; @@ -35,10 +37,17 @@ interface MailboxApi { * (contact was already added). */ void addContact(MailboxProperties properties, MailboxContact contact) - throws IOException, ApiException, - TolerableFailureException; + throws IOException, ApiException, TolerableFailureException; + + /** + * Deletes a contact from the mailbox. + * This should get called after a contact was removed from Briar. + */ + void deleteContact(MailboxProperties properties, ContactId contactId) + throws IOException, ApiException; @Immutable + @JsonSerialize class MailboxContact { public final int contactId; public final String token, inboxId, outboxId; 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 218283a42..09c29c5bc 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 @@ -5,6 +5,7 @@ import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.json.JsonMapper; import org.briarproject.bramble.api.WeakSingletonProvider; +import org.briarproject.bramble.api.contact.ContactId; import org.briarproject.bramble.api.mailbox.MailboxProperties; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; @@ -111,6 +112,21 @@ class MailboxApiImpl implements MailboxApi { if (!response.isSuccessful()) throw new ApiException(); } + @Override + public void deleteContact(MailboxProperties properties, ContactId contactId) + throws IOException, ApiException { + if (!properties.isOwner()) throw new IllegalArgumentException(); + String url = properties.getOnionAddress() + "/contacts/" + + contactId.getInt(); + Request request = getRequestBuilder(properties.getAuthToken()) + .delete() + .url(url) + .build(); + OkHttpClient client = httpClientProvider.get(); + Response response = client.newCall(request).execute(); + if (response.code() != 200) throw new ApiException(); + } + 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 5f4399ef3..b670b9998 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 @@ -220,6 +220,49 @@ public class MailboxApiTest extends BrambleTestCase { api.addContact(properties, mailboxContact)); } + @Test + public void testDeleteContact() throws Exception { + MockWebServer server = new MockWebServer(); + server.enqueue(new MockResponse()); + server.enqueue(new MockResponse().setResponseCode(205)); + server.enqueue(new MockResponse().setResponseCode(401)); + server.start(); + String baseUrl = getBaseUrl(server); + MailboxProperties properties = + new MailboxProperties(baseUrl, token, true); + + // contact gets deleted as expected + api.deleteContact(properties, contactId); + RecordedRequest request1 = server.takeRequest(); + assertEquals("DELETE", request1.getMethod()); + assertEquals("/contacts/" + contactId.getInt(), request1.getPath()); + assertToken(request1, token); + + // request is not returning 200 + assertThrows(ApiException.class, () -> + api.deleteContact(properties, contactId)); + RecordedRequest request2 = server.takeRequest(); + assertEquals("DELETE", request2.getMethod()); + assertEquals("/contacts/" + contactId.getInt(), request2.getPath()); + assertToken(request2, token); + + // request is not authorized + assertThrows(ApiException.class, () -> + api.deleteContact(properties, contactId)); + RecordedRequest request3 = server.takeRequest(); + assertEquals("DELETE", request3.getMethod()); + assertEquals("/contacts/" + contactId.getInt(), request3.getPath()); + assertToken(request3, token); + } + + @Test + public void testDeleteContactOnlyForOwner() { + MailboxProperties properties = + new MailboxProperties("", token, false); + assertThrows(IllegalArgumentException.class, () -> + api.deleteContact(properties, contactId)); + } + private String getBaseUrl(MockWebServer server) { String baseUrl = server.url("").toString(); return baseUrl.substring(0, baseUrl.length() - 1); From df4e6aa2079fe0b42779d173d1be2948921ca771 Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Fri, 7 Jan 2022 11:33:57 -0300 Subject: [PATCH 2/3] Add method for retrieving contact list from own mailbox --- .../bramble/mailbox/MailboxApi.java | 8 ++ .../bramble/mailbox/MailboxApiImpl.java | 35 +++++++ .../bramble/mailbox/MailboxApiTest.java | 98 +++++++++++++++++++ 3 files changed, 141 insertions(+) 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 4ccaa4c90..8a3e11d6a 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 @@ -6,6 +6,7 @@ import org.briarproject.bramble.api.contact.ContactId; import org.briarproject.bramble.api.mailbox.MailboxProperties; import java.io.IOException; +import java.util.Collection; import javax.annotation.concurrent.Immutable; @@ -46,6 +47,13 @@ interface MailboxApi { void deleteContact(MailboxProperties properties, ContactId contactId) throws IOException, ApiException; + /** + * Gets a list of {@link ContactId}s from the mailbox. + * These are the contacts that the mailbox already knows about. + */ + Collection getContacts(MailboxProperties properties) + throws IOException, ApiException; + @Immutable @JsonSerialize class MailboxContact { 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 09c29c5bc..076452c61 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 @@ -10,6 +10,9 @@ import org.briarproject.bramble.api.mailbox.MailboxProperties; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import java.io.IOException; +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; import javax.inject.Inject; @@ -127,6 +130,38 @@ class MailboxApiImpl implements MailboxApi { if (response.code() != 200) throw new ApiException(); } + @Override + public Collection getContacts(MailboxProperties properties) + throws IOException, ApiException { + if (!properties.isOwner()) throw new IllegalArgumentException(); + Request request = getRequestBuilder(properties.getAuthToken()) + .url(properties.getOnionAddress() + "/contacts") + .build(); + OkHttpClient client = httpClientProvider.get(); + Response response = client.newCall(request).execute(); + if (response.code() != 200) throw new ApiException(); + + ResponseBody body = response.body(); + if (body == null) throw new ApiException(); + try { + JsonNode node = mapper.readTree(body.string()); + JsonNode contactsNode = node.get("contacts"); + if (contactsNode == null || !contactsNode.isArray()) { + throw new ApiException(); + } + List list = new ArrayList<>(); + for (JsonNode contactNode : contactsNode) { + if (!contactNode.isNumber()) throw new ApiException(); + int id = contactNode.intValue(); + if (id < 1) throw new ApiException(); + list.add(new ContactId(id)); + } + return list; + } catch (JacksonException e) { + throw new ApiException(); + } + } + 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 b670b9998..97cb63b26 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,6 +9,9 @@ import org.briarproject.bramble.mailbox.MailboxApi.TolerableFailureException; import org.briarproject.bramble.test.BrambleTestCase; import org.junit.Test; +import java.util.ArrayList; +import java.util.List; + import javax.annotation.Nonnull; import javax.net.SocketFactory; @@ -17,6 +20,7 @@ import okhttp3.mockwebserver.MockResponse; import okhttp3.mockwebserver.MockWebServer; import okhttp3.mockwebserver.RecordedRequest; +import static java.util.Collections.singletonList; import static java.util.concurrent.TimeUnit.MILLISECONDS; import static org.briarproject.bramble.test.TestUtils.getContactId; import static org.briarproject.bramble.test.TestUtils.getMailboxSecret; @@ -263,6 +267,100 @@ public class MailboxApiTest extends BrambleTestCase { api.deleteContact(properties, contactId)); } + @Test + public void testGetContacts() throws Exception { + ContactId contactId2 = getContactId(); + String validResponse1 = "{\"contacts\": [" + contactId.getInt() + "] }"; + String validResponse2 = "{\"contacts\": [" + contactId.getInt() + ", " + + contactId2.getInt() + "] }"; + String invalidResponse1 = "{\"foo\":\"bar\"}"; + String invalidResponse2 = "{\"contacts\":{\"foo\":\"bar\"}}"; + String invalidResponse3 = "{\"contacts\": [1, 2, \"foo\"] }"; + + MockWebServer server = new MockWebServer(); + server.enqueue(new MockResponse().setBody(validResponse1)); + server.enqueue(new MockResponse().setBody(validResponse2)); + server.enqueue(new MockResponse()); + server.enqueue(new MockResponse().setBody(invalidResponse1)); + server.enqueue(new MockResponse().setBody(invalidResponse2)); + server.enqueue(new MockResponse().setBody(invalidResponse3)); + server.enqueue(new MockResponse().setResponseCode(401)); + server.enqueue(new MockResponse().setResponseCode(500)); + server.start(); + String baseUrl = getBaseUrl(server); + MailboxProperties properties = + new MailboxProperties(baseUrl, token, true); + + // valid response with two contacts + assertEquals(singletonList(contactId), api.getContacts(properties)); + RecordedRequest request1 = server.takeRequest(); + assertEquals("/contacts", request1.getPath()); + assertEquals("GET", request1.getMethod()); + assertToken(request1, token); + + // valid response with two contacts + List contacts = new ArrayList<>(); + contacts.add(contactId); + contacts.add(contactId2); + assertEquals(contacts, api.getContacts(properties)); + RecordedRequest request2 = server.takeRequest(); + assertEquals("/contacts", request2.getPath()); + assertEquals("GET", request2.getMethod()); + assertToken(request2, token); + + // empty body + assertThrows(ApiException.class, () -> api.getContacts(properties)); + RecordedRequest request3 = server.takeRequest(); + assertEquals("/contacts", request3.getPath()); + assertEquals("GET", request3.getMethod()); + assertToken(request3, token); + + // invalid response: no contacts key + assertThrows(ApiException.class, () -> api.getContacts(properties)); + RecordedRequest request4 = server.takeRequest(); + assertEquals("/contacts", request4.getPath()); + assertEquals("GET", request4.getMethod()); + assertToken(request4, token); + + // invalid response: no list in contacts + assertThrows(ApiException.class, () -> api.getContacts(properties)); + RecordedRequest request5 = server.takeRequest(); + assertEquals("/contacts", request5.getPath()); + assertEquals("GET", request5.getMethod()); + assertToken(request5, token); + + // invalid response: list with non-numbers + assertThrows(ApiException.class, () -> api.getContacts(properties)); + RecordedRequest request6 = server.takeRequest(); + assertEquals("/contacts", request6.getPath()); + assertEquals("GET", request6.getMethod()); + assertToken(request6, token); + + // 401 not authorized + assertThrows(ApiException.class, () -> api.getContacts(properties)); + RecordedRequest request7 = server.takeRequest(); + assertEquals("/contacts", request7.getPath()); + assertEquals("GET", request7.getMethod()); + assertToken(request7, token); + + // 500 internal server error + assertThrows(ApiException.class, () -> api.getContacts(properties)); + RecordedRequest request8 = server.takeRequest(); + assertEquals("/contacts", request8.getPath()); + assertEquals("GET", request8.getMethod()); + assertToken(request8, token); + } + + @Test + public void testGetContactsOnlyForOwner() { + MailboxProperties properties = + new MailboxProperties("", token, false); + assertThrows( + IllegalArgumentException.class, + () -> api.getContacts(properties) + ); + } + private String getBaseUrl(MockWebServer server) { String baseUrl = server.url("").toString(); return baseUrl.substring(0, baseUrl.length() - 1); From f5cdad910053e7296850c3479ac3b3b211c7b4c8 Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Fri, 7 Jan 2022 12:03:21 -0300 Subject: [PATCH 3/3] Throw TolerableFailureException when deleting a contact returns 404 --- .../bramble/mailbox/MailboxApi.java | 5 +++- .../bramble/mailbox/MailboxApiImpl.java | 24 ++++++++++--------- .../bramble/mailbox/MailboxApiTest.java | 9 +++++++ 3 files changed, 26 insertions(+), 12 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 8a3e11d6a..4040df48b 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 @@ -50,9 +50,12 @@ interface MailboxApi { /** * Gets a list of {@link ContactId}s from the mailbox. * These are the contacts that the mailbox already knows about. + * + * @throws TolerableFailureException if response code is 404 + * (contact probably was already deleted). */ Collection getContacts(MailboxProperties properties) - throws IOException, ApiException; + throws IOException, ApiException, TolerableFailureException; @Immutable @JsonSerialize 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 076452c61..3a42d9e6f 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 @@ -89,11 +89,7 @@ class MailboxApiImpl implements MailboxApi { public boolean checkStatus(MailboxProperties properties) 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(); + Response response = sendGetRequest(properties, "/status"); if (response.code() == 401) throw new ApiException(); return response.isSuccessful(); } @@ -132,13 +128,10 @@ class MailboxApiImpl implements MailboxApi { @Override public Collection getContacts(MailboxProperties properties) - throws IOException, ApiException { + throws IOException, ApiException, TolerableFailureException { if (!properties.isOwner()) throw new IllegalArgumentException(); - Request request = getRequestBuilder(properties.getAuthToken()) - .url(properties.getOnionAddress() + "/contacts") - .build(); - OkHttpClient client = httpClientProvider.get(); - Response response = client.newCall(request).execute(); + Response response = sendGetRequest(properties, "/contacts"); + if (response.code() == 404) throw new TolerableFailureException(); if (response.code() != 200) throw new ApiException(); ResponseBody body = response.body(); @@ -162,6 +155,15 @@ class MailboxApiImpl implements MailboxApi { } } + private Response sendGetRequest(MailboxProperties properties, String path) + throws IOException { + Request request = getRequestBuilder(properties.getAuthToken()) + .url(properties.getOnionAddress() + path) + .build(); + OkHttpClient client = httpClientProvider.get(); + return client.newCall(request).execute(); + } + 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 97cb63b26..a62d012d6 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 @@ -286,6 +286,7 @@ public class MailboxApiTest extends BrambleTestCase { server.enqueue(new MockResponse().setBody(invalidResponse3)); server.enqueue(new MockResponse().setResponseCode(401)); server.enqueue(new MockResponse().setResponseCode(500)); + server.enqueue(new MockResponse().setResponseCode(404)); server.start(); String baseUrl = getBaseUrl(server); MailboxProperties properties = @@ -349,6 +350,14 @@ public class MailboxApiTest extends BrambleTestCase { assertEquals("/contacts", request8.getPath()); assertEquals("GET", request8.getMethod()); assertToken(request8, token); + + // tolerable 404 not found error + assertThrows(TolerableFailureException.class, + () -> api.getContacts(properties)); + RecordedRequest request9 = server.takeRequest(); + assertEquals("/contacts", request9.getPath()); + assertEquals("GET", request9.getMethod()); + assertToken(request9, token); } @Test