From 173af62decc4d0753c3a1a677d78bc66bff28f2d Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Fri, 21 Jan 2022 10:59:15 -0300 Subject: [PATCH 01/11] Add method for adding file to mailbox --- .../briarproject/bramble/test/TestUtils.java | 23 +++++++++ .../bramble/mailbox/MailboxApi.java | 11 +++++ .../bramble/mailbox/MailboxApiImpl.java | 34 ++++++++++--- .../bramble/mailbox/MailboxApiTest.java | 49 +++++++++++++++++++ 4 files changed, 111 insertions(+), 6 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 3fd19912a..862be0ca9 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 @@ -25,7 +25,11 @@ import org.briarproject.bramble.api.sync.Message; import org.briarproject.bramble.api.sync.MessageId; import org.briarproject.bramble.util.IoUtils; +import java.io.ByteArrayOutputStream; import java.io.File; +import java.io.FileInputStream; +import java.io.FileOutputStream; +import java.io.IOException; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; @@ -46,6 +50,7 @@ import static org.briarproject.bramble.api.properties.TransportPropertyConstants 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.IoUtils.copyAndClose; import static org.briarproject.bramble.util.StringUtils.getRandomString; import static org.briarproject.bramble.util.StringUtils.toHexString; @@ -215,6 +220,24 @@ public class TestUtils { return toHexString(getRandomBytes(32)).toLowerCase(Locale.US); } + public static void writeBytes(File file, byte[] bytes) + throws IOException { + FileOutputStream outputStream = new FileOutputStream(file); + //noinspection TryFinallyCanBeTryWithResources + try { + outputStream.write(bytes); + } finally { + outputStream.close(); + } + } + + public static byte[] readBytes(File file) throws IOException { + ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); + FileInputStream inputStream = new FileInputStream(file); + copyAndClose(inputStream, outputStream); + return outputStream.toByteArray(); + } + 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 be08cc11a..73760208e 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 @@ -5,6 +5,7 @@ import com.fasterxml.jackson.databind.annotation.JsonSerialize; import org.briarproject.bramble.api.contact.ContactId; import org.briarproject.bramble.api.mailbox.MailboxProperties; +import java.io.File; import java.io.IOException; import java.util.Collection; @@ -57,6 +58,16 @@ interface MailboxApi { Collection getContacts(MailboxProperties properties) throws IOException, ApiException; + /** + * Used by contacts to send files to the owner + * and by the owner to send files to contacts. + *

+ * The owner can add files to the contacts' inboxes + * and the contacts can add files to their own outbox. + */ + void addFile(MailboxProperties properties, String folderId, + File file) 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 ba24a852d..53b2e5836 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 @@ -9,6 +9,7 @@ import org.briarproject.bramble.api.contact.ContactId; import org.briarproject.bramble.api.mailbox.MailboxProperties; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; +import java.io.File; import java.io.IOException; import java.util.ArrayList; import java.util.Collection; @@ -37,6 +38,8 @@ class MailboxApiImpl implements MailboxApi { .build(); private static final MediaType JSON = requireNonNull(MediaType.parse("application/json; charset=utf-8")); + private static final MediaType FILE = + requireNonNull(MediaType.parse("application/octet-stream")); @Inject MailboxApiImpl(WeakSingletonProvider httpClientProvider) { @@ -94,6 +97,8 @@ class MailboxApiImpl implements MailboxApi { return response.isSuccessful(); } + /* Contact Management API (owner only) */ + @Override public void addContact(MailboxProperties properties, MailboxContact contact) throws IOException, ApiException, @@ -101,12 +106,7 @@ class MailboxApiImpl implements MailboxApi { 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(); + Response response = sendPostRequest(properties, "/contacts", body); if (response.code() == 409) throw new TolerableFailureException(); if (!response.isSuccessful()) throw new ApiException(); } @@ -155,6 +155,18 @@ class MailboxApiImpl implements MailboxApi { } } + /* File Management (owner and contacts) */ + + @Override + public void addFile(MailboxProperties properties, String folderId, + File file) throws IOException, ApiException { + String path = "/files/" + folderId; + RequestBody body = RequestBody.create(FILE, file); + Response response = sendPostRequest(properties, path, body); + if (response.code() != 200) throw new ApiException(); + } + /* Helper Functions */ + private Response sendGetRequest(MailboxProperties properties, String path) throws IOException { Request request = getRequestBuilder(properties.getAuthToken()) @@ -164,6 +176,16 @@ class MailboxApiImpl implements MailboxApi { return client.newCall(request).execute(); } + private Response sendPostRequest(MailboxProperties properties, String path, + RequestBody body) throws IOException { + Request request = getRequestBuilder(properties.getAuthToken()) + .url(properties.getOnionAddress() + path) + .post(body) + .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 c27e7ef42..3e88c2d8d 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 @@ -7,8 +7,11 @@ import org.briarproject.bramble.mailbox.MailboxApi.ApiException; import org.briarproject.bramble.mailbox.MailboxApi.MailboxContact; import org.briarproject.bramble.mailbox.MailboxApi.TolerableFailureException; import org.briarproject.bramble.test.BrambleTestCase; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.TemporaryFolder; +import java.io.File; import java.util.ArrayList; import java.util.List; @@ -24,7 +27,10 @@ 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; +import static org.briarproject.bramble.test.TestUtils.getRandomBytes; +import static org.briarproject.bramble.test.TestUtils.writeBytes; import static org.briarproject.bramble.util.StringUtils.getRandomString; +import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; @@ -33,6 +39,9 @@ import static org.junit.Assert.assertTrue; public class MailboxApiTest extends BrambleTestCase { + @Rule + public TemporaryFolder folder = new TemporaryFolder(); + private final OkHttpClient client = new OkHttpClient.Builder() .socketFactory(SocketFactory.getDefault()) .connectTimeout(60_000, MILLISECONDS) @@ -370,6 +379,46 @@ public class MailboxApiTest extends BrambleTestCase { ); } + @Test + public void testAddFile() throws Exception { + File file = folder.newFile(); + byte[] bytes = getRandomBytes(1337); + writeBytes(file, bytes); + + MockWebServer server = new MockWebServer(); + server.enqueue(new MockResponse()); + 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); + + // file gets uploaded as expected + api.addFile(properties, contactInboxId, file); + RecordedRequest request1 = server.takeRequest(); + assertEquals("/files/" + contactInboxId, request1.getPath()); + assertEquals("POST", request1.getMethod()); + assertToken(request1, token); + assertArrayEquals(bytes, request1.getBody().readByteArray()); + + // request is not successful + assertThrows(ApiException.class, () -> + api.addFile(properties, contactInboxId, file)); + RecordedRequest request2 = server.takeRequest(); + assertEquals("/files/" + contactInboxId, request2.getPath()); + assertEquals("POST", request1.getMethod()); + assertToken(request2, token); + + // server error + assertThrows(ApiException.class, () -> + api.addFile(properties, contactInboxId, file)); + RecordedRequest request3 = server.takeRequest(); + assertEquals("/files/" + contactInboxId, request3.getPath()); + assertEquals("POST", request1.getMethod()); + assertToken(request3, token); + } + private String getBaseUrl(MockWebServer server) { String baseUrl = server.url("").toString(); return baseUrl.substring(0, baseUrl.length() - 1); From 76599a8d04bb255793edb19f189aef8886d089b7 Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Fri, 21 Jan 2022 11:31:01 -0300 Subject: [PATCH 02/11] Add method for listing files from mailbox --- .../bramble/mailbox/MailboxApi.java | 19 +++ .../bramble/mailbox/MailboxApiImpl.java | 41 +++++++ .../bramble/mailbox/MailboxApiTest.java | 114 ++++++++++++++++++ 3 files changed, 174 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 73760208e..2c4577603 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 @@ -8,6 +8,7 @@ import org.briarproject.bramble.api.mailbox.MailboxProperties; import java.io.File; import java.io.IOException; import java.util.Collection; +import java.util.List; import javax.annotation.concurrent.Immutable; @@ -68,6 +69,14 @@ interface MailboxApi { void addFile(MailboxProperties properties, String folderId, File file) throws IOException, ApiException; + /** + * Used by owner and contacts to list their files to retrieve. + *

+ * Returns 200 OK with the list of files in JSON. + */ + List getFiles(MailboxProperties properties, String folderId) + throws IOException, ApiException; + @Immutable @JsonSerialize class MailboxContact { @@ -85,6 +94,16 @@ interface MailboxApi { } } + class MailboxFile { + public final String name; + public final long time; + + public MailboxFile(String name, long time) { + this.name = name; + this.time = time; + } + } + @Immutable 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 53b2e5836..83efd7ed5 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 @@ -3,6 +3,7 @@ package org.briarproject.bramble.mailbox; import com.fasterxml.jackson.core.JacksonException; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.json.JsonMapper; +import com.fasterxml.jackson.databind.node.ObjectNode; import org.briarproject.bramble.api.WeakSingletonProvider; import org.briarproject.bramble.api.contact.ContactId; @@ -165,6 +166,46 @@ class MailboxApiImpl implements MailboxApi { Response response = sendPostRequest(properties, path, body); if (response.code() != 200) throw new ApiException(); } + + @Override + public List getFiles(MailboxProperties properties, + String folderId) throws IOException, ApiException { + String path = "/files/" + folderId; + Response response = sendGetRequest(properties, path); + 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 filesNode = node.get("files"); + if (filesNode == null || !filesNode.isArray()) { + throw new ApiException(); + } + List list = new ArrayList<>(); + for (JsonNode fileNode : filesNode) { + if (!fileNode.isObject()) throw new ApiException(); + ObjectNode objectNode = (ObjectNode) fileNode; + JsonNode nameNode = objectNode.get("name"); + JsonNode timeNode = objectNode.get("time"); + if (nameNode == null || !nameNode.isTextual()) { + throw new ApiException(); + } + if (timeNode == null || !timeNode.isNumber()) { + throw new ApiException(); + } + String name = nameNode.asText(); + long time = timeNode.asLong(); + if (!isValidToken(name)) throw new ApiException(); + if (time < 1) throw new ApiException(); + list.add(new MailboxFile(name, time)); + } + return list; + } catch (JacksonException e) { + throw new ApiException(); + } + } + /* Helper Functions */ private Response sendGetRequest(MailboxProperties properties, String path) 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 3e88c2d8d..87707df7f 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,10 +1,13 @@ package org.briarproject.bramble.mailbox; +import com.fasterxml.jackson.databind.ObjectMapper; + 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.MailboxFile; import org.briarproject.bramble.mailbox.MailboxApi.TolerableFailureException; import org.briarproject.bramble.test.BrambleTestCase; import org.junit.Rule; @@ -419,6 +422,117 @@ public class MailboxApiTest extends BrambleTestCase { assertToken(request3, token); } + @Test + public void testGetFiles() throws Exception { + MailboxFile mailboxFile1 = new MailboxFile(getMailboxSecret(), 1337); + MailboxFile mailboxFile2 = + new MailboxFile(getMailboxSecret(), System.currentTimeMillis()); + String fileResponse1 = + new ObjectMapper().writeValueAsString(mailboxFile1); + String fileResponse2 = + new ObjectMapper().writeValueAsString(mailboxFile2); + String validResponse1 = "{\"files\": [" + fileResponse1 + "] }"; + String validResponse2 = "{\"files\": [" + fileResponse1 + ", " + + fileResponse2 + "] }"; + String invalidResponse1 = "{\"files\":\"bar\"}"; + String invalidResponse2 = "{\"files\":{\"foo\":\"bar\"}}"; + String invalidResponse3 = "{\"files\": [" + fileResponse1 + ", 1] }"; + String invalidResponse4 = "{\"contacts\": [ 1, 2 ] }"; + + 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().setBody(invalidResponse4)); + 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 one file + List received1 = api.getFiles(properties, contactInboxId); + assertEquals(1, received1.size()); + assertEquals(mailboxFile1.name, received1.get(0).name); + assertEquals(mailboxFile1.time, received1.get(0).time); + RecordedRequest request1 = server.takeRequest(); + assertEquals("/files/" + contactInboxId, request1.getPath()); + assertEquals("GET", request1.getMethod()); + assertToken(request1, token); + + // valid response with two files + List received2 = api.getFiles(properties, contactInboxId); + assertEquals(2, received2.size()); + assertEquals(mailboxFile1.name, received2.get(0).name); + assertEquals(mailboxFile1.time, received2.get(0).time); + assertEquals(mailboxFile2.name, received2.get(1).name); + assertEquals(mailboxFile2.time, received2.get(1).time); + RecordedRequest request2 = server.takeRequest(); + assertEquals("/files/" + contactInboxId, request1.getPath()); + assertEquals("GET", request2.getMethod()); + assertToken(request2, token); + + // empty body + assertThrows(ApiException.class, () -> + api.getFiles(properties, contactInboxId)); + RecordedRequest request3 = server.takeRequest(); + assertEquals("/files/" + contactInboxId, request3.getPath()); + assertEquals("GET", request3.getMethod()); + assertToken(request3, token); + + // invalid response: string instead of list + assertThrows(ApiException.class, () -> + api.getFiles(properties, contactInboxId)); + RecordedRequest request4 = server.takeRequest(); + assertEquals("/files/" + contactInboxId, request4.getPath()); + assertEquals("GET", request4.getMethod()); + assertToken(request4, token); + + // invalid response: object instead of list + assertThrows(ApiException.class, () -> + api.getFiles(properties, contactInboxId)); + RecordedRequest request5 = server.takeRequest(); + assertEquals("/files/" + contactInboxId, request5.getPath()); + assertEquals("GET", request5.getMethod()); + assertToken(request5, token); + + // invalid response: list with non-objects + assertThrows(ApiException.class, () -> + api.getFiles(properties, contactInboxId)); + RecordedRequest request6 = server.takeRequest(); + assertEquals("/files/" + contactInboxId, request6.getPath()); + assertEquals("GET", request6.getMethod()); + assertToken(request6, token); + + // no files key in root object + assertThrows(ApiException.class, () -> + api.getFiles(properties, contactInboxId)); + RecordedRequest request7 = server.takeRequest(); + assertEquals("/files/" + contactInboxId, request7.getPath()); + assertEquals("GET", request7.getMethod()); + assertToken(request7, token); + + // 401 not authorized + assertThrows(ApiException.class, () -> + api.getFiles(properties, contactInboxId)); + RecordedRequest request8 = server.takeRequest(); + assertEquals("/files/" + contactInboxId, request8.getPath()); + assertEquals("GET", request8.getMethod()); + assertToken(request8, token); + + // 500 internal server error + assertThrows(ApiException.class, + () -> api.getFiles(properties, contactInboxId)); + RecordedRequest request9 = server.takeRequest(); + assertEquals("/files/" + contactInboxId, request9.getPath()); + assertEquals("GET", request9.getMethod()); + assertToken(request9, token); + } + private String getBaseUrl(MockWebServer server) { String baseUrl = server.url("").toString(); return baseUrl.substring(0, baseUrl.length() - 1); From 0cb2dcf6b7d87f08f7ccdc54f17adf10c95c6ef4 Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Fri, 21 Jan 2022 11:59:25 -0300 Subject: [PATCH 03/11] Add method for downloading a file from a mailbox --- .../bramble/mailbox/MailboxApi.java | 11 +++++ .../bramble/mailbox/MailboxApiImpl.java | 15 ++++++ .../bramble/mailbox/MailboxApiTest.java | 49 +++++++++++++++++++ 3 files changed, 75 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 2c4577603..6d5eec1df 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 @@ -77,6 +77,17 @@ interface MailboxApi { List getFiles(MailboxProperties properties, String folderId) throws IOException, ApiException; + /** + * Used by owner and contacts to retrieve a file. + *

+ * Returns 200 OK if successful with the files' raw bytes + * in the response body. + * + * @param file the empty file the response bytes will be written into. + */ + void getFile(MailboxProperties properties, String folderId, + String fileId, File file) 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 83efd7ed5..751600ad4 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 @@ -11,6 +11,7 @@ import org.briarproject.bramble.api.mailbox.MailboxProperties; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import java.io.File; +import java.io.FileOutputStream; import java.io.IOException; import java.util.ArrayList; import java.util.Collection; @@ -28,6 +29,7 @@ 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.IoUtils.copyAndClose; import static org.briarproject.bramble.util.StringUtils.fromHexString; @NotNullByDefault @@ -206,6 +208,19 @@ class MailboxApiImpl implements MailboxApi { } } + @Override + public void getFile(MailboxProperties properties, String folderId, + String fileId, File file) throws IOException, ApiException { + String path = "/files/" + folderId + "/" + fileId; + Response response = sendGetRequest(properties, path); + if (response.code() != 200) throw new ApiException(); + + ResponseBody body = response.body(); + if (body == null) throw new ApiException(); + FileOutputStream outputStream = new FileOutputStream(file); + copyAndClose(body.byteStream(), outputStream); + } + /* Helper Functions */ private Response sendGetRequest(MailboxProperties properties, String path) 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 87707df7f..4c0c53b44 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 @@ -25,12 +25,14 @@ import okhttp3.OkHttpClient; import okhttp3.mockwebserver.MockResponse; import okhttp3.mockwebserver.MockWebServer; import okhttp3.mockwebserver.RecordedRequest; +import okio.Buffer; 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; import static org.briarproject.bramble.test.TestUtils.getRandomBytes; +import static org.briarproject.bramble.test.TestUtils.readBytes; import static org.briarproject.bramble.test.TestUtils.writeBytes; import static org.briarproject.bramble.util.StringUtils.getRandomString; import static org.junit.Assert.assertArrayEquals; @@ -533,6 +535,53 @@ public class MailboxApiTest extends BrambleTestCase { assertToken(request9, token); } + @Test + public void testGetFile() throws Exception { + String name = getMailboxSecret(); + File file1 = folder.newFile(); + File file2 = folder.newFile(); + File file3 = folder.newFile(); + byte[] bytes = getRandomBytes(1337); + + MockWebServer server = new MockWebServer(); + server.enqueue(new MockResponse().setBody(new Buffer().write(bytes))); + 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); + + // file gets downloaded as expected + api.getFile(properties, contactOutboxId, name, file1); + RecordedRequest request1 = server.takeRequest(); + assertEquals("/files/" + contactOutboxId + "/" + name, + request1.getPath()); + assertEquals("GET", request1.getMethod()); + assertToken(request1, token); + assertArrayEquals(bytes, readBytes(file1)); + + // request is not successful + assertThrows(ApiException.class, () -> + api.getFile(properties, contactOutboxId, name, file2)); + RecordedRequest request2 = server.takeRequest(); + assertEquals("/files/" + contactOutboxId + "/" + name, + request2.getPath()); + assertEquals("GET", request1.getMethod()); + assertToken(request2, token); + assertEquals(0, readBytes(file2).length); + + // server error + assertThrows(ApiException.class, () -> + api.getFile(properties, contactOutboxId, name, file3)); + RecordedRequest request3 = server.takeRequest(); + assertEquals("/files/" + contactOutboxId + "/" + name, + request3.getPath()); + assertEquals("GET", request1.getMethod()); + assertToken(request3, token); + assertEquals(0, readBytes(file3).length); + } + private String getBaseUrl(MockWebServer server) { String baseUrl = server.url("").toString(); return baseUrl.substring(0, baseUrl.length() - 1); From 482258fc92c2b9c384f0782a11d9679b02fd6a4d Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Fri, 21 Jan 2022 12:08:52 -0300 Subject: [PATCH 04/11] Add method for deleting a file from a mailbox --- .../bramble/mailbox/MailboxApi.java | 8 ++++ .../bramble/mailbox/MailboxApiImpl.java | 13 ++++++ .../bramble/mailbox/MailboxApiTest.java | 40 +++++++++++++++++++ 3 files changed, 61 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 6d5eec1df..6a5b946f9 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 @@ -88,6 +88,14 @@ interface MailboxApi { void getFile(MailboxProperties properties, String folderId, String fileId, File file) throws IOException, ApiException; + /** + * Used by owner and contacts to delete files. + *

+ * Returns 200 OK (no exception) if deletion was successful. + */ + void deleteFile(MailboxProperties properties, String folderId, + String fileId) 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 751600ad4..a027f41eb 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 @@ -221,6 +221,19 @@ class MailboxApiImpl implements MailboxApi { copyAndClose(body.byteStream(), outputStream); } + @Override + public void deleteFile(MailboxProperties properties, String folderId, + String fileId) throws IOException, ApiException { + String path = "/files/" + folderId + "/" + fileId; + Request request = getRequestBuilder(properties.getAuthToken()) + .delete() + .url(properties.getOnionAddress() + path) + .build(); + OkHttpClient client = httpClientProvider.get(); + Response response = client.newCall(request).execute(); + if (response.code() != 200) throw new ApiException(); + } + /* Helper Functions */ private Response sendGetRequest(MailboxProperties properties, String path) 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 4c0c53b44..082a81df1 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 @@ -582,6 +582,46 @@ public class MailboxApiTest extends BrambleTestCase { assertEquals(0, readBytes(file3).length); } + @Test + public void testDeleteFile() throws Exception { + String name = getMailboxSecret(); + + 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); + + // file gets deleted as expected + api.deleteFile(properties, contactInboxId, name); + RecordedRequest request1 = server.takeRequest(); + assertEquals("DELETE", request1.getMethod()); + assertEquals("/files/" + contactInboxId + "/" + name, + request1.getPath()); + assertToken(request1, token); + + // request is not returning 200 + assertThrows(ApiException.class, () -> + api.deleteFile(properties, contactInboxId, name)); + RecordedRequest request2 = server.takeRequest(); + assertEquals("DELETE", request2.getMethod()); + assertEquals("/files/" + contactInboxId + "/" + name, + request2.getPath()); + assertToken(request2, token); + + // request is not authorized + assertThrows(ApiException.class, () -> + api.deleteFile(properties, contactInboxId, name)); + RecordedRequest request3 = server.takeRequest(); + assertEquals("DELETE", request3.getMethod()); + assertEquals("/files/" + contactInboxId + "/" + name, + request3.getPath()); + assertToken(request3, token); + } + private String getBaseUrl(MockWebServer server) { String baseUrl = server.url("").toString(); return baseUrl.substring(0, baseUrl.length() - 1); From 3a191908c02a517419b8255628f7edc994805188 Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Fri, 21 Jan 2022 13:58:39 -0300 Subject: [PATCH 05/11] Add method for listing folders with files available for download (owner only) --- .../bramble/mailbox/MailboxApi.java | 11 ++ .../bramble/mailbox/MailboxApiImpl.java | 33 ++++++ .../bramble/mailbox/MailboxApiTest.java | 101 ++++++++++++++++++ 3 files changed, 145 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 6a5b946f9..29f94e55f 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 @@ -96,6 +96,17 @@ interface MailboxApi { void deleteFile(MailboxProperties properties, String folderId, String fileId) throws IOException, ApiException; + /** + * Lists all contact outboxes that have files available + * for the owner to download. + * + * @return a list of folder names + * to be used with {@link #getFiles(MailboxProperties, String)}. + * @throws IllegalArgumentException if used by non-owner. + */ + List getFolders(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 a027f41eb..60ee4858f 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 @@ -234,6 +234,39 @@ class MailboxApiImpl implements MailboxApi { if (response.code() != 200) throw new ApiException(); } + @Override + public List getFolders(MailboxProperties properties) + throws IOException, ApiException { + if (!properties.isOwner()) throw new IllegalArgumentException(); + Response response = sendGetRequest(properties, "/folders"); + 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 filesNode = node.get("folders"); + if (filesNode == null || !filesNode.isArray()) { + throw new ApiException(); + } + List list = new ArrayList<>(); + for (JsonNode fileNode : filesNode) { + if (!fileNode.isObject()) throw new ApiException(); + ObjectNode objectNode = (ObjectNode) fileNode; + JsonNode idNode = objectNode.get("id"); + if (idNode == null || !idNode.isTextual()) { + throw new ApiException(); + } + String id = idNode.asText(); + if (!isValidToken(id)) throw new ApiException(); + list.add(id); + } + return list; + } catch (JacksonException e) { + throw new ApiException(); + } + } + /* Helper Functions */ private Response sendGetRequest(MailboxProperties properties, String path) 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 082a81df1..2e90525af 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 @@ -16,6 +16,7 @@ import org.junit.rules.TemporaryFolder; import java.io.File; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import javax.annotation.Nonnull; @@ -622,6 +623,106 @@ public class MailboxApiTest extends BrambleTestCase { assertToken(request3, token); } + @Test + public void testGetFolders() throws Exception { + String id1 = getMailboxSecret(); + String id2 = getMailboxSecret(); + String validResponse1 = "{\"folders\": [ {\"id\": \"" + id1 + "\"} ] }"; + String validResponse2 = "{\"folders\": [ {\"id\": \"" + id1 + "\"}, " + + "{ \"id\": \"" + id2 + "\"} ] }"; + String invalidResponse1 = "{\"folders\":\"bar\"}"; + String invalidResponse2 = "{\"folders\":{\"foo\":\"bar\"}}"; + String invalidResponse3 = + "{\"folders\": [ {\"id\": \"" + id1 + "\", 1] }"; + String invalidResponse4 = "{\"files\": [ 1, 2 ] }"; + + 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().setBody(invalidResponse4)); + 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 one folders + assertEquals(singletonList(id1), api.getFolders(properties)); + RecordedRequest request1 = server.takeRequest(); + assertEquals("/folders", request1.getPath()); + assertEquals("GET", request1.getMethod()); + assertToken(request1, token); + + // valid response with two folders + assertEquals(Arrays.asList(id1, id2), api.getFolders(properties)); + RecordedRequest request2 = server.takeRequest(); + assertEquals("/folders", request1.getPath()); + assertEquals("GET", request2.getMethod()); + assertToken(request2, token); + + // empty body + assertThrows(ApiException.class, () -> api.getFolders(properties)); + RecordedRequest request3 = server.takeRequest(); + assertEquals("/folders", request3.getPath()); + assertEquals("GET", request3.getMethod()); + assertToken(request3, token); + + // invalid response: string instead of list + assertThrows(ApiException.class, () -> api.getFolders(properties)); + RecordedRequest request4 = server.takeRequest(); + assertEquals("/folders", request4.getPath()); + assertEquals("GET", request4.getMethod()); + assertToken(request4, token); + + // invalid response: object instead of list + assertThrows(ApiException.class, () -> api.getFolders(properties)); + RecordedRequest request5 = server.takeRequest(); + assertEquals("/folders", request5.getPath()); + assertEquals("GET", request5.getMethod()); + assertToken(request5, token); + + // invalid response: list with non-objects + assertThrows(ApiException.class, () -> api.getFolders(properties)); + RecordedRequest request6 = server.takeRequest(); + assertEquals("/folders", request6.getPath()); + assertEquals("GET", request6.getMethod()); + assertToken(request6, token); + + // no folders key in root object + assertThrows(ApiException.class, () -> api.getFolders(properties)); + RecordedRequest request7 = server.takeRequest(); + assertEquals("/folders", request7.getPath()); + assertEquals("GET", request7.getMethod()); + assertToken(request7, token); + + // 401 not authorized + assertThrows(ApiException.class, () -> api.getFolders(properties)); + RecordedRequest request8 = server.takeRequest(); + assertEquals("/folders", request8.getPath()); + assertEquals("GET", request8.getMethod()); + assertToken(request8, token); + + // 500 internal server error + assertThrows(ApiException.class, () -> api.getFolders(properties)); + RecordedRequest request9 = server.takeRequest(); + assertEquals("/folders", request9.getPath()); + assertEquals("GET", request9.getMethod()); + assertToken(request9, token); + } + + @Test + public void testGetFoldersOnlyForOwner() { + MailboxProperties properties = + new MailboxProperties("", token, false); + assertThrows(IllegalArgumentException.class, () -> + api.getFolders(properties)); + } + private String getBaseUrl(MockWebServer server) { String baseUrl = server.url("").toString(); return baseUrl.substring(0, baseUrl.length() - 1); From 0fba65a722c1a6bf6b2003451f0d8062d7d816da Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Fri, 21 Jan 2022 14:58:28 -0300 Subject: [PATCH 06/11] Add integration test for File Management API --- .../mailbox/MailboxIntegrationTest.java | 129 ++++++++++++++++++ 1 file changed, 129 insertions(+) diff --git a/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxIntegrationTest.java b/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxIntegrationTest.java index 19fa9a64c..f02b3f3a6 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxIntegrationTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxIntegrationTest.java @@ -5,14 +5,19 @@ 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.MailboxFile; import org.briarproject.bramble.mailbox.MailboxApi.TolerableFailureException; import org.briarproject.bramble.test.BrambleTestCase; import org.junit.Before; import org.junit.BeforeClass; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.TemporaryFolder; +import java.io.File; import java.io.IOException; import java.util.Arrays; +import java.util.List; import javax.annotation.Nonnull; import javax.net.SocketFactory; @@ -23,7 +28,11 @@ import static java.util.Collections.emptyList; import static java.util.Collections.singletonList; import static java.util.concurrent.TimeUnit.MILLISECONDS; import static org.briarproject.bramble.test.TestUtils.getMailboxSecret; +import static org.briarproject.bramble.test.TestUtils.getRandomBytes; import static org.briarproject.bramble.test.TestUtils.isOptionalTestEnabled; +import static org.briarproject.bramble.test.TestUtils.readBytes; +import static org.briarproject.bramble.test.TestUtils.writeBytes; +import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; @@ -31,6 +40,9 @@ import static org.junit.Assume.assumeTrue; public class MailboxIntegrationTest extends BrambleTestCase { + @Rule + public TemporaryFolder folder = new TemporaryFolder(); + private final static String URL_BASE = "http://127.0.0.1:8000"; private final static String SETUP_TOKEN = "54686973206973206120736574757020746f6b656e20666f722042726961722e"; @@ -101,6 +113,123 @@ public class MailboxIntegrationTest extends BrambleTestCase { () -> api.deleteContact(ownerProperties, contactId2)); } + @Test + public void testFileManagementApi() throws Exception { + // add contact, so we can leave each other files + ContactId contactId = new ContactId(1); + MailboxContact contact = getMailboxContact(contactId); + MailboxProperties contactProperties = new MailboxProperties( + ownerProperties.getOnionAddress(), contact.token, false); + api.addContact(ownerProperties, contact); + + // upload a file for our contact + File file1 = folder.newFile(); + byte[] bytes1 = getRandomBytes(2048); + writeBytes(file1, bytes1); + api.addFile(ownerProperties, contact.inboxId, file1); + + // contact checks files + List files1 = + api.getFiles(contactProperties, contact.inboxId); + assertEquals(1, files1.size()); + String fileName1 = files1.get(0).name; + + // owner can't check files + assertThrows(ApiException.class, () -> + api.getFiles(ownerProperties, contact.inboxId)); + + // contact downloads file + File file1downloaded = folder.newFile(); + api.getFile(contactProperties, contact.inboxId, fileName1, + file1downloaded); + assertArrayEquals(bytes1, readBytes(file1downloaded)); + + // owner can't download file, even if knowing name + File file1forbidden = folder.newFile(); + assertThrows(ApiException.class, () -> api.getFile(ownerProperties, + contact.inboxId, fileName1, file1forbidden)); + assertEquals(0, file1forbidden.length()); + + // owner can't delete file + assertThrows(ApiException.class, () -> + api.deleteFile(ownerProperties, contact.inboxId, fileName1)); + + // contact deletes file + api.deleteFile(contactProperties, contact.inboxId, fileName1); + assertEquals(0, + api.getFiles(contactProperties, contact.inboxId).size()); + + // contact uploads two files for the owner + File file2 = folder.newFile(); + File file3 = folder.newFile(); + byte[] bytes2 = getRandomBytes(2048); + byte[] bytes3 = getRandomBytes(1024); + writeBytes(file2, bytes2); + writeBytes(file3, bytes3); + api.addFile(contactProperties, contact.outboxId, file2); + api.addFile(contactProperties, contact.outboxId, file3); + + // owner checks folders with available files + List folders = api.getFolders(ownerProperties); + assertEquals(singletonList(contact.outboxId), folders); + + // owner lists files in contact's outbox + List files2 = + api.getFiles(ownerProperties, contact.outboxId); + assertEquals(2, files2.size()); + String file2name = files2.get(0).name; + String file3name = files2.get(1).name; + + // contact can't list files in contact's outbox + assertThrows(ApiException.class, () -> + api.getFiles(contactProperties, contact.outboxId)); + + // owner downloads both files from contact's outbox + File file2downloaded = folder.newFile(); + File file3downloaded = folder.newFile(); + api.getFile(ownerProperties, contact.outboxId, file2name, + file2downloaded); + api.getFile(ownerProperties, contact.outboxId, file3name, + file3downloaded); + byte[] downloadedBytes2 = readBytes(file2downloaded); + byte[] downloadedBytes3 = readBytes(file3downloaded); + // file order is not preserved, so we don't know what file is which + if (downloadedBytes2.length == bytes2.length) { + assertArrayEquals(bytes2, downloadedBytes2); + assertArrayEquals(bytes3, downloadedBytes3); + } else { + assertArrayEquals(bytes2, downloadedBytes3); + assertArrayEquals(bytes3, downloadedBytes2); + } + + // contact can't download files again, even if knowing name + File file2forbidden = folder.newFile(); + File file3forbidden = folder.newFile(); + assertThrows(ApiException.class, () -> api.getFile(contactProperties, + contact.outboxId, file2name, file2forbidden)); + assertThrows(ApiException.class, () -> api.getFile(contactProperties, + contact.outboxId, file3name, file3forbidden)); + assertEquals(0, file1forbidden.length()); + assertEquals(0, file2forbidden.length()); + + // contact can't delete files in outbox + assertThrows(ApiException.class, () -> + api.deleteFile(contactProperties, contact.outboxId, file2name)); + assertThrows(ApiException.class, () -> + api.deleteFile(contactProperties, contact.outboxId, file3name)); + + // owner deletes files + api.deleteFile(ownerProperties, contact.outboxId, file2name); + api.deleteFile(ownerProperties, contact.outboxId, file3name); + assertEquals(emptyList(), + api.getFiles(ownerProperties, contact.outboxId)); + assertEquals(emptyList(), api.getFolders(ownerProperties)); + + // owner deletes contact again to leave clean state for other tests + api.deleteContact(ownerProperties, contactId); + assertEquals(emptyList(), api.getContacts(ownerProperties)); + } + private MailboxContact getMailboxContact(ContactId contactId) { return new MailboxContact(contactId, getMailboxSecret(), getMailboxSecret(), getMailboxSecret()); From 61ea7ff8dee0df31936a5aefafa272a30f6bc7bd Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Fri, 21 Jan 2022 15:04:51 -0300 Subject: [PATCH 07/11] Make deleting a non-existent file is tolerable --- .../org/briarproject/bramble/mailbox/MailboxApi.java | 6 +++++- .../briarproject/bramble/mailbox/MailboxApiImpl.java | 4 +++- .../briarproject/bramble/mailbox/MailboxApiTest.java | 10 ++++++++++ .../bramble/mailbox/MailboxIntegrationTest.java | 4 ++++ 4 files changed, 22 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 29f94e55f..5f2e2ee91 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 @@ -92,9 +92,13 @@ interface MailboxApi { * Used by owner and contacts to delete files. *

* Returns 200 OK (no exception) if deletion was successful. + * + * @throws TolerableFailureException on 404 response, + * because file was most likely deleted already. */ void deleteFile(MailboxProperties properties, String folderId, - String fileId) throws IOException, ApiException; + String fileId) + throws IOException, ApiException, TolerableFailureException; /** * Lists all contact outboxes that have files available 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 60ee4858f..1042b559f 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 @@ -223,7 +223,8 @@ class MailboxApiImpl implements MailboxApi { @Override public void deleteFile(MailboxProperties properties, String folderId, - String fileId) throws IOException, ApiException { + String fileId) + throws IOException, ApiException, TolerableFailureException { String path = "/files/" + folderId + "/" + fileId; Request request = getRequestBuilder(properties.getAuthToken()) .delete() @@ -231,6 +232,7 @@ class MailboxApiImpl implements MailboxApi { .build(); OkHttpClient client = httpClientProvider.get(); Response response = client.newCall(request).execute(); + if (response.code() == 404) throw new TolerableFailureException(); if (response.code() != 200) throw new ApiException(); } 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 2e90525af..35d15943a 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 @@ -591,6 +591,7 @@ public class MailboxApiTest extends BrambleTestCase { server.enqueue(new MockResponse()); server.enqueue(new MockResponse().setResponseCode(205)); server.enqueue(new MockResponse().setResponseCode(401)); + server.enqueue(new MockResponse().setResponseCode(404)); server.start(); String baseUrl = getBaseUrl(server); MailboxProperties properties = @@ -621,6 +622,15 @@ public class MailboxApiTest extends BrambleTestCase { assertEquals("/files/" + contactInboxId + "/" + name, request3.getPath()); assertToken(request3, token); + + // file not found is tolerable + assertThrows(TolerableFailureException.class, () -> + api.deleteFile(properties, contactInboxId, name)); + RecordedRequest request4 = server.takeRequest(); + assertEquals("DELETE", request4.getMethod()); + assertEquals("/files/" + contactInboxId + "/" + name, + request4.getPath()); + assertToken(request4, token); } @Test diff --git a/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxIntegrationTest.java b/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxIntegrationTest.java index f02b3f3a6..7fd133d05 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxIntegrationTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxIntegrationTest.java @@ -225,6 +225,10 @@ public class MailboxIntegrationTest extends BrambleTestCase { api.getFiles(ownerProperties, contact.outboxId)); assertEquals(emptyList(), api.getFolders(ownerProperties)); + // deleting a non-existent file is tolerable + assertThrows(TolerableFailureException.class, () -> + api.deleteFile(ownerProperties, contact.outboxId, file3name)); + // owner deletes contact again to leave clean state for other tests api.deleteContact(ownerProperties, contactId); assertEquals(emptyList(), api.getContacts(ownerProperties)); From f057f0859b3df8f80e0dcc67dbcc5158253e27e8 Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Mon, 24 Jan 2022 13:50:58 -0300 Subject: [PATCH 08/11] Use MailboxId instead of String for type-safety --- bramble-api/build.gradle | 1 + .../bramble/api/mailbox/MailboxId.java | 47 +++++++++++++++++++ .../api/mailbox/MailboxProperties.java | 7 +-- .../briarproject/bramble/test/TestUtils.java | 7 ++- .../bramble/mailbox/MailboxApi.java | 32 +++++++------ .../bramble/mailbox/MailboxApiImpl.java | 29 +++++++----- .../mailbox/MailboxSettingsManagerImpl.java | 6 ++- .../bramble/mailbox/MailboxApiTest.java | 27 ++++++----- .../mailbox/MailboxIntegrationTest.java | 21 +++++---- .../MailboxSettingsManagerImplTest.java | 13 +++-- 10 files changed, 125 insertions(+), 65 deletions(-) create mode 100644 bramble-api/src/main/java/org/briarproject/bramble/api/mailbox/MailboxId.java diff --git a/bramble-api/build.gradle b/bramble-api/build.gradle index 0cd52e31a..926386b02 100644 --- a/bramble-api/build.gradle +++ b/bramble-api/build.gradle @@ -9,6 +9,7 @@ apply from: 'witness.gradle' dependencies { implementation "com.google.dagger:dagger:$dagger_version" implementation 'com.google.code.findbugs:jsr305:3.0.2' + implementation "com.fasterxml.jackson.core:jackson-annotations:$jackson_version" testImplementation "junit:junit:$junit_version" testImplementation "org.jmock:jmock:$jmock_version" diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/mailbox/MailboxId.java b/bramble-api/src/main/java/org/briarproject/bramble/api/mailbox/MailboxId.java new file mode 100644 index 000000000..0be97a321 --- /dev/null +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/mailbox/MailboxId.java @@ -0,0 +1,47 @@ +package org.briarproject.bramble.api.mailbox; + +import com.fasterxml.jackson.annotation.JsonValue; + +import org.briarproject.bramble.api.UniqueId; +import org.briarproject.bramble.api.nullsafety.NotNullByDefault; + +import java.util.Locale; + +import javax.annotation.concurrent.ThreadSafe; + +import static org.briarproject.bramble.util.StringUtils.fromHexString; +import static org.briarproject.bramble.util.StringUtils.toHexString; + +@ThreadSafe +@NotNullByDefault +public class MailboxId extends UniqueId { + + public MailboxId(byte[] id) { + super(id); + } + + /** + * Creates a {@link MailboxId} from the given string. + * + * @throws IllegalArgumentException if token is not valid. + */ + public static MailboxId fromString(String token) { + if (token.length() != 64) throw new IllegalArgumentException(); + return new MailboxId(fromHexString(token)); + } + + /** + * Returns the string representation expected by the mailbox API. + * Also used for serialization. + */ + @Override + @JsonValue + public String toString() { + return toHexString(getBytes()).toLowerCase(Locale.US); + } + + @Override + public boolean equals(Object o) { + return o instanceof MailboxId && super.equals(o); + } +} diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/mailbox/MailboxProperties.java b/bramble-api/src/main/java/org/briarproject/bramble/api/mailbox/MailboxProperties.java index 723c5aabc..133ce58c5 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/mailbox/MailboxProperties.java +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/mailbox/MailboxProperties.java @@ -8,10 +8,11 @@ import javax.annotation.concurrent.Immutable; @NotNullByDefault public class MailboxProperties { - private final String onionAddress, authToken; + private final String onionAddress; + private final MailboxId authToken; private final boolean owner; - public MailboxProperties(String onionAddress, String authToken, + public MailboxProperties(String onionAddress, MailboxId authToken, boolean owner) { this.onionAddress = onionAddress; this.authToken = authToken; @@ -22,7 +23,7 @@ public class MailboxProperties { return onionAddress; } - public String getAuthToken() { + public MailboxId getAuthToken() { return authToken; } 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 862be0ca9..b6aff596a 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 @@ -16,6 +16,7 @@ import org.briarproject.bramble.api.identity.Author; import org.briarproject.bramble.api.identity.AuthorId; import org.briarproject.bramble.api.identity.Identity; import org.briarproject.bramble.api.identity.LocalAuthor; +import org.briarproject.bramble.api.mailbox.MailboxId; import org.briarproject.bramble.api.plugin.TransportId; import org.briarproject.bramble.api.properties.TransportProperties; import org.briarproject.bramble.api.sync.ClientId; @@ -35,7 +36,6 @@ 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; @@ -52,7 +52,6 @@ import static org.briarproject.bramble.api.sync.SyncConstants.MAX_GROUP_DESCRIPT import static org.briarproject.bramble.api.sync.SyncConstants.MAX_MESSAGE_BODY_LENGTH; import static org.briarproject.bramble.util.IoUtils.copyAndClose; import static org.briarproject.bramble.util.StringUtils.getRandomString; -import static org.briarproject.bramble.util.StringUtils.toHexString; public class TestUtils { @@ -216,8 +215,8 @@ public class TestUtils { getAgreementPublicKey(), verified); } - public static String getMailboxSecret() { - return toHexString(getRandomBytes(32)).toLowerCase(Locale.US); + public static MailboxId getMailboxId() { + return new MailboxId(getRandomId()); } public static void writeBytes(File file, byte[] bytes) 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 5f2e2ee91..9e378277b 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 @@ -3,6 +3,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.MailboxId; import org.briarproject.bramble.api.mailbox.MailboxProperties; import java.io.File; @@ -21,7 +22,7 @@ interface MailboxApi { * @return the owner token * @throws ApiException for 401 response. */ - String setup(MailboxProperties properties) + MailboxId setup(MailboxProperties properties) throws IOException, ApiException; /** @@ -66,7 +67,7 @@ interface MailboxApi { * The owner can add files to the contacts' inboxes * and the contacts can add files to their own outbox. */ - void addFile(MailboxProperties properties, String folderId, + void addFile(MailboxProperties properties, MailboxId folderId, File file) throws IOException, ApiException; /** @@ -74,7 +75,7 @@ interface MailboxApi { *

* Returns 200 OK with the list of files in JSON. */ - List getFiles(MailboxProperties properties, String folderId) + List getFiles(MailboxProperties properties, MailboxId folderId) throws IOException, ApiException; /** @@ -85,8 +86,8 @@ interface MailboxApi { * * @param file the empty file the response bytes will be written into. */ - void getFile(MailboxProperties properties, String folderId, - String fileId, File file) throws IOException, ApiException; + void getFile(MailboxProperties properties, MailboxId folderId, + MailboxId fileId, File file) throws IOException, ApiException; /** * Used by owner and contacts to delete files. @@ -96,8 +97,8 @@ interface MailboxApi { * @throws TolerableFailureException on 404 response, * because file was most likely deleted already. */ - void deleteFile(MailboxProperties properties, String folderId, - String fileId) + void deleteFile(MailboxProperties properties, MailboxId folderId, + MailboxId fileId) throws IOException, ApiException, TolerableFailureException; /** @@ -105,22 +106,22 @@ interface MailboxApi { * for the owner to download. * * @return a list of folder names - * to be used with {@link #getFiles(MailboxProperties, String)}. + * to be used with {@link #getFiles(MailboxProperties, MailboxId)}. * @throws IllegalArgumentException if used by non-owner. */ - List getFolders(MailboxProperties properties) + List getFolders(MailboxProperties properties) throws IOException, ApiException; @Immutable @JsonSerialize class MailboxContact { public final int contactId; - public final String token, inboxId, outboxId; + public final MailboxId token, inboxId, outboxId; MailboxContact(ContactId contactId, - String token, - String inboxId, - String outboxId) { + MailboxId token, + MailboxId inboxId, + MailboxId outboxId) { this.contactId = contactId.getInt(); this.token = token; this.inboxId = inboxId; @@ -128,11 +129,12 @@ interface MailboxApi { } } + @JsonSerialize class MailboxFile { - public final String name; + public final MailboxId name; public final long time; - public MailboxFile(String name, long time) { + public MailboxFile(MailboxId name, long time) { this.name = name; this.time = time; } 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 1042b559f..cd6480486 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 @@ -7,6 +7,7 @@ import com.fasterxml.jackson.databind.node.ObjectNode; import org.briarproject.bramble.api.WeakSingletonProvider; import org.briarproject.bramble.api.contact.ContactId; +import org.briarproject.bramble.api.mailbox.MailboxId; import org.briarproject.bramble.api.mailbox.MailboxProperties; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; @@ -50,7 +51,7 @@ class MailboxApiImpl implements MailboxApi { } @Override - public String setup(MailboxProperties properties) + public MailboxId setup(MailboxProperties properties) throws IOException, ApiException { if (!properties.isOwner()) throw new IllegalArgumentException(); Request request = getRequestBuilder(properties.getAuthToken()) @@ -74,12 +75,14 @@ class MailboxApiImpl implements MailboxApi { if (ownerToken == null || !isValidToken(ownerToken)) { throw new ApiException(); } - return ownerToken; + return MailboxId.fromString(ownerToken); } catch (JacksonException e) { throw new ApiException(); } } + // TODO find a batter way to validate (regex?) + // that doesn't do hex string conversion twice private boolean isValidToken(String token) { if (token.length() != 64) return false; try { @@ -161,7 +164,7 @@ class MailboxApiImpl implements MailboxApi { /* File Management (owner and contacts) */ @Override - public void addFile(MailboxProperties properties, String folderId, + public void addFile(MailboxProperties properties, MailboxId folderId, File file) throws IOException, ApiException { String path = "/files/" + folderId; RequestBody body = RequestBody.create(FILE, file); @@ -171,7 +174,7 @@ class MailboxApiImpl implements MailboxApi { @Override public List getFiles(MailboxProperties properties, - String folderId) throws IOException, ApiException { + MailboxId folderId) throws IOException, ApiException { String path = "/files/" + folderId; Response response = sendGetRequest(properties, path); if (response.code() != 200) throw new ApiException(); @@ -200,7 +203,7 @@ class MailboxApiImpl implements MailboxApi { long time = timeNode.asLong(); if (!isValidToken(name)) throw new ApiException(); if (time < 1) throw new ApiException(); - list.add(new MailboxFile(name, time)); + list.add(new MailboxFile(MailboxId.fromString(name), time)); } return list; } catch (JacksonException e) { @@ -209,8 +212,8 @@ class MailboxApiImpl implements MailboxApi { } @Override - public void getFile(MailboxProperties properties, String folderId, - String fileId, File file) throws IOException, ApiException { + public void getFile(MailboxProperties properties, MailboxId folderId, + MailboxId fileId, File file) throws IOException, ApiException { String path = "/files/" + folderId + "/" + fileId; Response response = sendGetRequest(properties, path); if (response.code() != 200) throw new ApiException(); @@ -222,8 +225,8 @@ class MailboxApiImpl implements MailboxApi { } @Override - public void deleteFile(MailboxProperties properties, String folderId, - String fileId) + public void deleteFile(MailboxProperties properties, MailboxId folderId, + MailboxId fileId) throws IOException, ApiException, TolerableFailureException { String path = "/files/" + folderId + "/" + fileId; Request request = getRequestBuilder(properties.getAuthToken()) @@ -237,7 +240,7 @@ class MailboxApiImpl implements MailboxApi { } @Override - public List getFolders(MailboxProperties properties) + public List getFolders(MailboxProperties properties) throws IOException, ApiException { if (!properties.isOwner()) throw new IllegalArgumentException(); Response response = sendGetRequest(properties, "/folders"); @@ -251,7 +254,7 @@ class MailboxApiImpl implements MailboxApi { if (filesNode == null || !filesNode.isArray()) { throw new ApiException(); } - List list = new ArrayList<>(); + List list = new ArrayList<>(); for (JsonNode fileNode : filesNode) { if (!fileNode.isObject()) throw new ApiException(); ObjectNode objectNode = (ObjectNode) fileNode; @@ -261,7 +264,7 @@ class MailboxApiImpl implements MailboxApi { } String id = idNode.asText(); if (!isValidToken(id)) throw new ApiException(); - list.add(id); + list.add(MailboxId.fromString(id)); } return list; } catch (JacksonException e) { @@ -290,7 +293,7 @@ class MailboxApiImpl implements MailboxApi { return client.newCall(request).execute(); } - private Request.Builder getRequestBuilder(String token) { + private Request.Builder getRequestBuilder(MailboxId token) { return new Request.Builder() .addHeader("Authorization", "Bearer " + token); } diff --git a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxSettingsManagerImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxSettingsManagerImpl.java index dbe62a73a..be18f1b6a 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxSettingsManagerImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxSettingsManagerImpl.java @@ -3,6 +3,7 @@ package org.briarproject.bramble.mailbox; import org.briarproject.bramble.api.contact.ContactId; import org.briarproject.bramble.api.db.DbException; import org.briarproject.bramble.api.db.Transaction; +import org.briarproject.bramble.api.mailbox.MailboxId; import org.briarproject.bramble.api.mailbox.MailboxProperties; import org.briarproject.bramble.api.mailbox.MailboxSettingsManager; import org.briarproject.bramble.api.mailbox.MailboxStatus; @@ -43,7 +44,8 @@ class MailboxSettingsManagerImpl implements MailboxSettingsManager { String onion = s.get(SETTINGS_KEY_ONION); String token = s.get(SETTINGS_KEY_TOKEN); if (isNullOrEmpty(onion) || isNullOrEmpty(token)) return null; - return new MailboxProperties(onion, token, true); + MailboxId tokenId = MailboxId.fromString(token); + return new MailboxProperties(onion, tokenId, true); } @Override @@ -51,7 +53,7 @@ class MailboxSettingsManagerImpl implements MailboxSettingsManager { throws DbException { Settings s = new Settings(); s.put(SETTINGS_KEY_ONION, p.getOnionAddress()); - s.put(SETTINGS_KEY_TOKEN, p.getAuthToken()); + s.put(SETTINGS_KEY_TOKEN, p.getAuthToken().toString()); settingsManager.mergeSettings(txn, s, SETTINGS_NAMESPACE); } 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 35d15943a..05a8b2695 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 @@ -4,6 +4,7 @@ import com.fasterxml.jackson.databind.ObjectMapper; import org.briarproject.bramble.api.WeakSingletonProvider; import org.briarproject.bramble.api.contact.ContactId; +import org.briarproject.bramble.api.mailbox.MailboxId; import org.briarproject.bramble.api.mailbox.MailboxProperties; import org.briarproject.bramble.mailbox.MailboxApi.ApiException; import org.briarproject.bramble.mailbox.MailboxApi.MailboxContact; @@ -31,7 +32,7 @@ import okio.Buffer; 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; +import static org.briarproject.bramble.test.TestUtils.getMailboxId; import static org.briarproject.bramble.test.TestUtils.getRandomBytes; import static org.briarproject.bramble.test.TestUtils.readBytes; import static org.briarproject.bramble.test.TestUtils.writeBytes; @@ -62,12 +63,12 @@ public class MailboxApiTest extends BrambleTestCase { }; private final MailboxApiImpl api = new MailboxApiImpl(httpClientProvider); - private final String token = getMailboxSecret(); - private final String token2 = getMailboxSecret(); + private final MailboxId token = getMailboxId(); + private final MailboxId token2 = getMailboxId(); private final ContactId contactId = getContactId(); - private final String contactToken = getMailboxSecret(); - private final String contactInboxId = getMailboxSecret(); - private final String contactOutboxId = getMailboxSecret(); + private final MailboxId contactToken = getMailboxId(); + private final MailboxId contactInboxId = getMailboxId(); + private final MailboxId contactOutboxId = getMailboxId(); private final MailboxContact mailboxContact = new MailboxContact( contactId, contactToken, contactInboxId, contactOutboxId); @@ -427,9 +428,9 @@ public class MailboxApiTest extends BrambleTestCase { @Test public void testGetFiles() throws Exception { - MailboxFile mailboxFile1 = new MailboxFile(getMailboxSecret(), 1337); + MailboxFile mailboxFile1 = new MailboxFile(getMailboxId(), 1337); MailboxFile mailboxFile2 = - new MailboxFile(getMailboxSecret(), System.currentTimeMillis()); + new MailboxFile(getMailboxId(), System.currentTimeMillis()); String fileResponse1 = new ObjectMapper().writeValueAsString(mailboxFile1); String fileResponse2 = @@ -538,7 +539,7 @@ public class MailboxApiTest extends BrambleTestCase { @Test public void testGetFile() throws Exception { - String name = getMailboxSecret(); + MailboxId name = getMailboxId(); File file1 = folder.newFile(); File file2 = folder.newFile(); File file3 = folder.newFile(); @@ -585,7 +586,7 @@ public class MailboxApiTest extends BrambleTestCase { @Test public void testDeleteFile() throws Exception { - String name = getMailboxSecret(); + MailboxId name = getMailboxId(); MockWebServer server = new MockWebServer(); server.enqueue(new MockResponse()); @@ -635,8 +636,8 @@ public class MailboxApiTest extends BrambleTestCase { @Test public void testGetFolders() throws Exception { - String id1 = getMailboxSecret(); - String id2 = getMailboxSecret(); + MailboxId id1 = getMailboxId(); + MailboxId id2 = getMailboxId(); String validResponse1 = "{\"folders\": [ {\"id\": \"" + id1 + "\"} ] }"; String validResponse2 = "{\"folders\": [ {\"id\": \"" + id1 + "\"}, " + "{ \"id\": \"" + id2 + "\"} ] }"; @@ -738,7 +739,7 @@ public class MailboxApiTest extends BrambleTestCase { return baseUrl.substring(0, baseUrl.length() - 1); } - private void assertToken(RecordedRequest request, String token) { + private void assertToken(RecordedRequest request, MailboxId token) { assertNotNull(request.getHeader("Authorization")); assertEquals("Bearer " + token, request.getHeader("Authorization")); } diff --git a/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxIntegrationTest.java b/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxIntegrationTest.java index 7fd133d05..2912023eb 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxIntegrationTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxIntegrationTest.java @@ -2,6 +2,7 @@ package org.briarproject.bramble.mailbox; import org.briarproject.bramble.api.WeakSingletonProvider; import org.briarproject.bramble.api.contact.ContactId; +import org.briarproject.bramble.api.mailbox.MailboxId; import org.briarproject.bramble.api.mailbox.MailboxProperties; import org.briarproject.bramble.mailbox.MailboxApi.ApiException; import org.briarproject.bramble.mailbox.MailboxApi.MailboxContact; @@ -27,7 +28,7 @@ import okhttp3.OkHttpClient; import static java.util.Collections.emptyList; import static java.util.Collections.singletonList; import static java.util.concurrent.TimeUnit.MILLISECONDS; -import static org.briarproject.bramble.test.TestUtils.getMailboxSecret; +import static org.briarproject.bramble.test.TestUtils.getMailboxId; import static org.briarproject.bramble.test.TestUtils.getRandomBytes; import static org.briarproject.bramble.test.TestUtils.isOptionalTestEnabled; import static org.briarproject.bramble.test.TestUtils.readBytes; @@ -44,8 +45,8 @@ public class MailboxIntegrationTest extends BrambleTestCase { public TemporaryFolder folder = new TemporaryFolder(); private final static String URL_BASE = "http://127.0.0.1:8000"; - private final static String SETUP_TOKEN = - "54686973206973206120736574757020746f6b656e20666f722042726961722e"; + private final static MailboxId SETUP_TOKEN = MailboxId.fromString( + "54686973206973206120736574757020746f6b656e20666f722042726961722e"); private final OkHttpClient client = new OkHttpClient.Builder() .socketFactory(SocketFactory.getDefault()) @@ -76,7 +77,7 @@ public class MailboxIntegrationTest extends BrambleTestCase { if (ownerProperties != null) return; MailboxProperties setupProperties = new MailboxProperties(URL_BASE, SETUP_TOKEN, true); - String ownerToken = api.setup(setupProperties); + MailboxId ownerToken = api.setup(setupProperties); ownerProperties = new MailboxProperties(URL_BASE, ownerToken, true); } @@ -132,7 +133,7 @@ public class MailboxIntegrationTest extends BrambleTestCase { List files1 = api.getFiles(contactProperties, contact.inboxId); assertEquals(1, files1.size()); - String fileName1 = files1.get(0).name; + MailboxId fileName1 = files1.get(0).name; // owner can't check files assertThrows(ApiException.class, () -> @@ -170,15 +171,15 @@ public class MailboxIntegrationTest extends BrambleTestCase { api.addFile(contactProperties, contact.outboxId, file3); // owner checks folders with available files - List folders = api.getFolders(ownerProperties); + List folders = api.getFolders(ownerProperties); assertEquals(singletonList(contact.outboxId), folders); // owner lists files in contact's outbox List files2 = api.getFiles(ownerProperties, contact.outboxId); assertEquals(2, files2.size()); - String file2name = files2.get(0).name; - String file3name = files2.get(1).name; + MailboxId file2name = files2.get(0).name; + MailboxId file3name = files2.get(1).name; // contact can't list files in contact's outbox assertThrows(ApiException.class, () -> @@ -235,8 +236,8 @@ public class MailboxIntegrationTest extends BrambleTestCase { } private MailboxContact getMailboxContact(ContactId contactId) { - return new MailboxContact(contactId, getMailboxSecret(), - getMailboxSecret(), getMailboxSecret()); + return new MailboxContact(contactId, getMailboxId(), getMailboxId(), + getMailboxId()); } } diff --git a/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxSettingsManagerImplTest.java b/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxSettingsManagerImplTest.java index 424554c97..3df58bef5 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxSettingsManagerImplTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxSettingsManagerImplTest.java @@ -2,6 +2,7 @@ package org.briarproject.bramble.mailbox; import org.briarproject.bramble.api.contact.ContactId; import org.briarproject.bramble.api.db.Transaction; +import org.briarproject.bramble.api.mailbox.MailboxId; import org.briarproject.bramble.api.mailbox.MailboxProperties; import org.briarproject.bramble.api.mailbox.MailboxSettingsManager; import org.briarproject.bramble.api.mailbox.MailboxStatus; @@ -20,6 +21,7 @@ import static org.briarproject.bramble.mailbox.MailboxSettingsManagerImpl.SETTIN import static org.briarproject.bramble.mailbox.MailboxSettingsManagerImpl.SETTINGS_KEY_TOKEN; import static org.briarproject.bramble.mailbox.MailboxSettingsManagerImpl.SETTINGS_NAMESPACE; import static org.briarproject.bramble.mailbox.MailboxSettingsManagerImpl.SETTINGS_UPLOADS_NAMESPACE; +import static org.briarproject.bramble.test.TestUtils.getMailboxId; import static org.briarproject.bramble.util.StringUtils.getRandomString; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; @@ -35,7 +37,7 @@ public class MailboxSettingsManagerImplTest extends BrambleMockTestCase { new MailboxSettingsManagerImpl(settingsManager); private final Random random = new Random(); private final String onion = getRandomString(64); - private final String token = getRandomString(64); + private final MailboxId token = getMailboxId(); private final ContactId contactId1 = new ContactId(random.nextInt()); private final ContactId contactId2 = new ContactId(random.nextInt()); private final ContactId contactId3 = new ContactId(random.nextInt()); @@ -62,7 +64,7 @@ public class MailboxSettingsManagerImplTest extends BrambleMockTestCase { Transaction txn = new Transaction(null, true); Settings settings = new Settings(); settings.put(SETTINGS_KEY_ONION, onion); - settings.put(SETTINGS_KEY_TOKEN, token); + settings.put(SETTINGS_KEY_TOKEN, token.toString()); context.checking(new Expectations() {{ oneOf(settingsManager).getSettings(txn, SETTINGS_NAMESPACE); @@ -81,7 +83,7 @@ public class MailboxSettingsManagerImplTest extends BrambleMockTestCase { Transaction txn = new Transaction(null, false); Settings expectedSettings = new Settings(); expectedSettings.put(SETTINGS_KEY_ONION, onion); - expectedSettings.put(SETTINGS_KEY_TOKEN, token); + expectedSettings.put(SETTINGS_KEY_TOKEN, token.toString()); MailboxProperties properties = new MailboxProperties(onion, token, true); @@ -180,7 +182,7 @@ public class MailboxSettingsManagerImplTest extends BrambleMockTestCase { Transaction txn = new Transaction(null, true); Settings settings = new Settings(); settings.put(String.valueOf(contactId1.getInt()), onion); - settings.put(String.valueOf(contactId2.getInt()), token); + settings.put(String.valueOf(contactId2.getInt()), token.toString()); settings.put(String.valueOf(contactId3.getInt()), ""); context.checking(new Expectations() {{ @@ -192,7 +194,8 @@ public class MailboxSettingsManagerImplTest extends BrambleMockTestCase { String filename1 = manager.getPendingUpload(txn, contactId1); assertEquals(onion, filename1); String filename2 = manager.getPendingUpload(txn, contactId2); - assertEquals(token, filename2); + assertNotNull(filename2); + assertEquals(token, MailboxId.fromString(filename2)); String filename3 = manager.getPendingUpload(txn, contactId3); assertNull(filename3); String filename4 = From d3beb850ef65894ba6bd5ce445c01e2c4aebcbb3 Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Mon, 24 Jan 2022 14:03:48 -0300 Subject: [PATCH 09/11] Factor out getArray() for easier JSON parsing --- .../bramble/mailbox/MailboxApiImpl.java | 26 ++++++++++--------- 1 file changed, 14 insertions(+), 12 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 cd6480486..c084a15e0 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 @@ -3,6 +3,7 @@ package org.briarproject.bramble.mailbox; import com.fasterxml.jackson.core.JacksonException; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.json.JsonMapper; +import com.fasterxml.jackson.databind.node.ArrayNode; import com.fasterxml.jackson.databind.node.ObjectNode; import org.briarproject.bramble.api.WeakSingletonProvider; @@ -144,10 +145,7 @@ class MailboxApiImpl implements MailboxApi { 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(); - } + ArrayNode contactsNode = getArray(node, "contacts"); List list = new ArrayList<>(); for (JsonNode contactNode : contactsNode) { if (!contactNode.isNumber()) throw new ApiException(); @@ -183,10 +181,7 @@ class MailboxApiImpl implements MailboxApi { if (body == null) throw new ApiException(); try { JsonNode node = mapper.readTree(body.string()); - JsonNode filesNode = node.get("files"); - if (filesNode == null || !filesNode.isArray()) { - throw new ApiException(); - } + ArrayNode filesNode = getArray(node, "files"); List list = new ArrayList<>(); for (JsonNode fileNode : filesNode) { if (!fileNode.isObject()) throw new ApiException(); @@ -250,10 +245,7 @@ class MailboxApiImpl implements MailboxApi { if (body == null) throw new ApiException(); try { JsonNode node = mapper.readTree(body.string()); - JsonNode filesNode = node.get("folders"); - if (filesNode == null || !filesNode.isArray()) { - throw new ApiException(); - } + ArrayNode filesNode = getArray(node, "folders"); List list = new ArrayList<>(); for (JsonNode fileNode : filesNode) { if (!fileNode.isObject()) throw new ApiException(); @@ -298,4 +290,14 @@ class MailboxApiImpl implements MailboxApi { .addHeader("Authorization", "Bearer " + token); } + /* JSON helpers */ + + private ArrayNode getArray(JsonNode node, String name) throws ApiException { + JsonNode arrayNode = node.get(name); + if (arrayNode == null || !arrayNode.isArray()) { + throw new ApiException(); + } + return (ArrayNode) arrayNode; + } + } From 5c153aeb6c3e509ecce3255c1eeba481eac4ac47 Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Mon, 7 Feb 2022 09:36:48 -0300 Subject: [PATCH 10/11] Sort files returned by getFiles by time (oldest first). --- .../org/briarproject/bramble/mailbox/MailboxApi.java | 10 +++++++++- .../briarproject/bramble/mailbox/MailboxApiImpl.java | 2 ++ .../bramble/mailbox/MailboxIntegrationTest.java | 12 ++++-------- 3 files changed, 15 insertions(+), 9 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 9e378277b..966cf1056 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 @@ -11,6 +11,7 @@ import java.io.IOException; import java.util.Collection; import java.util.List; +import javax.annotation.Nonnull; import javax.annotation.concurrent.Immutable; interface MailboxApi { @@ -130,7 +131,7 @@ interface MailboxApi { } @JsonSerialize - class MailboxFile { + class MailboxFile implements Comparable { public final MailboxId name; public final long time; @@ -138,6 +139,13 @@ interface MailboxApi { this.name = name; this.time = time; } + + @Override + public int compareTo(@Nonnull MailboxApi.MailboxFile mailboxFile) { + //noinspection UseCompareMethod + return time < mailboxFile.time ? -1 : + (time == mailboxFile.time ? 0 : 1); + } } @Immutable 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 c084a15e0..a09bba09c 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 @@ -17,6 +17,7 @@ import java.io.FileOutputStream; import java.io.IOException; import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; import java.util.List; import javax.inject.Inject; @@ -200,6 +201,7 @@ class MailboxApiImpl implements MailboxApi { if (time < 1) throw new ApiException(); list.add(new MailboxFile(MailboxId.fromString(name), time)); } + Collections.sort(list); return list; } catch (JacksonException e) { throw new ApiException(); diff --git a/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxIntegrationTest.java b/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxIntegrationTest.java index 2912023eb..7d98e93c2 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxIntegrationTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxIntegrationTest.java @@ -194,14 +194,10 @@ public class MailboxIntegrationTest extends BrambleTestCase { file3downloaded); byte[] downloadedBytes2 = readBytes(file2downloaded); byte[] downloadedBytes3 = readBytes(file3downloaded); - // file order is not preserved, so we don't know what file is which - if (downloadedBytes2.length == bytes2.length) { - assertArrayEquals(bytes2, downloadedBytes2); - assertArrayEquals(bytes3, downloadedBytes3); - } else { - assertArrayEquals(bytes2, downloadedBytes3); - assertArrayEquals(bytes3, downloadedBytes2); - } + // file order is preserved (sorted by time), + // so we know what file is which + assertArrayEquals(bytes2, downloadedBytes2); + assertArrayEquals(bytes3, downloadedBytes3); // contact can't download files again, even if knowing name File file2forbidden = folder.newFile(); From 16b503dd7becf3df8db66ab95b0d7c26c746b1f8 Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Mon, 7 Feb 2022 15:57:05 -0300 Subject: [PATCH 11/11] Introduce MailboxId sub-classes for even more type-safety --- .../mailbox/InvalidMailboxIdException.java | 8 +++ .../bramble/api/mailbox/MailboxAuthToken.java | 24 ++++++++ .../bramble/api/mailbox/MailboxFileId.java | 24 ++++++++ .../bramble/api/mailbox/MailboxFolderId.java | 24 ++++++++ .../bramble/api/mailbox/MailboxId.java | 28 +++++----- .../api/mailbox/MailboxProperties.java | 6 +- .../briarproject/bramble/test/TestUtils.java | 5 -- .../bramble/mailbox/MailboxApi.java | 37 ++++++------ .../bramble/mailbox/MailboxApiImpl.java | 56 +++++++------------ .../mailbox/MailboxSettingsManagerImpl.java | 11 +++- .../bramble/mailbox/MailboxApiTest.java | 32 +++++++---- .../mailbox/MailboxIntegrationTest.java | 35 ++++++++---- .../MailboxSettingsManagerImplTest.java | 8 +-- 13 files changed, 194 insertions(+), 104 deletions(-) create mode 100644 bramble-api/src/main/java/org/briarproject/bramble/api/mailbox/InvalidMailboxIdException.java create mode 100644 bramble-api/src/main/java/org/briarproject/bramble/api/mailbox/MailboxAuthToken.java create mode 100644 bramble-api/src/main/java/org/briarproject/bramble/api/mailbox/MailboxFileId.java create mode 100644 bramble-api/src/main/java/org/briarproject/bramble/api/mailbox/MailboxFolderId.java diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/mailbox/InvalidMailboxIdException.java b/bramble-api/src/main/java/org/briarproject/bramble/api/mailbox/InvalidMailboxIdException.java new file mode 100644 index 000000000..dfb54d84e --- /dev/null +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/mailbox/InvalidMailboxIdException.java @@ -0,0 +1,8 @@ +package org.briarproject.bramble.api.mailbox; + +import org.briarproject.bramble.api.nullsafety.NotNullByDefault; + +@NotNullByDefault +public class InvalidMailboxIdException extends Exception { + +} diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/mailbox/MailboxAuthToken.java b/bramble-api/src/main/java/org/briarproject/bramble/api/mailbox/MailboxAuthToken.java new file mode 100644 index 000000000..d2b932969 --- /dev/null +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/mailbox/MailboxAuthToken.java @@ -0,0 +1,24 @@ +package org.briarproject.bramble.api.mailbox; + +import org.briarproject.bramble.api.nullsafety.NotNullByDefault; + +import javax.annotation.Nullable; +import javax.annotation.concurrent.ThreadSafe; + +@ThreadSafe +@NotNullByDefault +public class MailboxAuthToken extends MailboxId { + public MailboxAuthToken(byte[] id) { + super(id); + } + + /** + * Creates a {@link MailboxAuthToken} from the given string. + * + * @throws InvalidMailboxIdException if token is not valid. + */ + public static MailboxAuthToken fromString(@Nullable String token) + throws InvalidMailboxIdException { + return new MailboxAuthToken(bytesFromString(token)); + } +} diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/mailbox/MailboxFileId.java b/bramble-api/src/main/java/org/briarproject/bramble/api/mailbox/MailboxFileId.java new file mode 100644 index 000000000..7814658da --- /dev/null +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/mailbox/MailboxFileId.java @@ -0,0 +1,24 @@ +package org.briarproject.bramble.api.mailbox; + +import org.briarproject.bramble.api.nullsafety.NotNullByDefault; + +import javax.annotation.Nullable; +import javax.annotation.concurrent.ThreadSafe; + +@ThreadSafe +@NotNullByDefault +public class MailboxFileId extends MailboxId { + public MailboxFileId(byte[] id) { + super(id); + } + + /** + * Creates a {@link MailboxFileId} from the given string. + * + * @throws IllegalArgumentException if token is not valid. + */ + public static MailboxFileId fromString(@Nullable String token) + throws InvalidMailboxIdException { + return new MailboxFileId(bytesFromString(token)); + } +} diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/mailbox/MailboxFolderId.java b/bramble-api/src/main/java/org/briarproject/bramble/api/mailbox/MailboxFolderId.java new file mode 100644 index 000000000..7a1819d04 --- /dev/null +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/mailbox/MailboxFolderId.java @@ -0,0 +1,24 @@ +package org.briarproject.bramble.api.mailbox; + +import org.briarproject.bramble.api.nullsafety.NotNullByDefault; + +import javax.annotation.Nullable; +import javax.annotation.concurrent.ThreadSafe; + +@ThreadSafe +@NotNullByDefault +public class MailboxFolderId extends MailboxId { + public MailboxFolderId(byte[] id) { + super(id); + } + + /** + * Creates a {@link MailboxFolderId} from the given string. + * + * @throws IllegalArgumentException if token is not valid. + */ + public static MailboxFolderId fromString(@Nullable String token) + throws InvalidMailboxIdException { + return new MailboxFolderId(bytesFromString(token)); + } +} diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/mailbox/MailboxId.java b/bramble-api/src/main/java/org/briarproject/bramble/api/mailbox/MailboxId.java index 0be97a321..06719fb80 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/mailbox/MailboxId.java +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/mailbox/MailboxId.java @@ -7,6 +7,7 @@ import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import java.util.Locale; +import javax.annotation.Nullable; import javax.annotation.concurrent.ThreadSafe; import static org.briarproject.bramble.util.StringUtils.fromHexString; @@ -14,20 +15,26 @@ import static org.briarproject.bramble.util.StringUtils.toHexString; @ThreadSafe @NotNullByDefault -public class MailboxId extends UniqueId { - - public MailboxId(byte[] id) { +public abstract class MailboxId extends UniqueId { + MailboxId(byte[] id) { super(id); } /** - * Creates a {@link MailboxId} from the given string. + * Returns valid {@link MailboxId} bytes from the given string. * - * @throws IllegalArgumentException if token is not valid. + * @throws InvalidMailboxIdException if token is not valid. */ - public static MailboxId fromString(String token) { - if (token.length() != 64) throw new IllegalArgumentException(); - return new MailboxId(fromHexString(token)); + static byte[] bytesFromString(@Nullable String token) + throws InvalidMailboxIdException { + if (token == null || token.length() != 64) { + throw new InvalidMailboxIdException(); + } + try { + return fromHexString(token); + } catch (IllegalArgumentException e) { + throw new InvalidMailboxIdException(); + } } /** @@ -39,9 +46,4 @@ public class MailboxId extends UniqueId { public String toString() { return toHexString(getBytes()).toLowerCase(Locale.US); } - - @Override - public boolean equals(Object o) { - return o instanceof MailboxId && super.equals(o); - } } diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/mailbox/MailboxProperties.java b/bramble-api/src/main/java/org/briarproject/bramble/api/mailbox/MailboxProperties.java index 133ce58c5..26025fc8e 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/mailbox/MailboxProperties.java +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/mailbox/MailboxProperties.java @@ -9,10 +9,10 @@ import javax.annotation.concurrent.Immutable; public class MailboxProperties { private final String onionAddress; - private final MailboxId authToken; + private final MailboxAuthToken authToken; private final boolean owner; - public MailboxProperties(String onionAddress, MailboxId authToken, + public MailboxProperties(String onionAddress, MailboxAuthToken authToken, boolean owner) { this.onionAddress = onionAddress; this.authToken = authToken; @@ -23,7 +23,7 @@ public class MailboxProperties { return onionAddress; } - public MailboxId getAuthToken() { + public MailboxAuthToken getAuthToken() { return authToken; } 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 b6aff596a..f5d1b9ed1 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 @@ -16,7 +16,6 @@ import org.briarproject.bramble.api.identity.Author; import org.briarproject.bramble.api.identity.AuthorId; import org.briarproject.bramble.api.identity.Identity; import org.briarproject.bramble.api.identity.LocalAuthor; -import org.briarproject.bramble.api.mailbox.MailboxId; import org.briarproject.bramble.api.plugin.TransportId; import org.briarproject.bramble.api.properties.TransportProperties; import org.briarproject.bramble.api.sync.ClientId; @@ -215,10 +214,6 @@ public class TestUtils { getAgreementPublicKey(), verified); } - public static MailboxId getMailboxId() { - return new MailboxId(getRandomId()); - } - public static void writeBytes(File file, byte[] bytes) throws IOException { FileOutputStream outputStream = new FileOutputStream(file); 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 966cf1056..82a628766 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 @@ -3,7 +3,9 @@ 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.MailboxId; +import org.briarproject.bramble.api.mailbox.MailboxAuthToken; +import org.briarproject.bramble.api.mailbox.MailboxFileId; +import org.briarproject.bramble.api.mailbox.MailboxFolderId; import org.briarproject.bramble.api.mailbox.MailboxProperties; import java.io.File; @@ -23,7 +25,7 @@ interface MailboxApi { * @return the owner token * @throws ApiException for 401 response. */ - MailboxId setup(MailboxProperties properties) + MailboxAuthToken setup(MailboxProperties properties) throws IOException, ApiException; /** @@ -68,7 +70,7 @@ interface MailboxApi { * The owner can add files to the contacts' inboxes * and the contacts can add files to their own outbox. */ - void addFile(MailboxProperties properties, MailboxId folderId, + void addFile(MailboxProperties properties, MailboxFolderId folderId, File file) throws IOException, ApiException; /** @@ -76,8 +78,8 @@ interface MailboxApi { *

* Returns 200 OK with the list of files in JSON. */ - List getFiles(MailboxProperties properties, MailboxId folderId) - throws IOException, ApiException; + List getFiles(MailboxProperties properties, + MailboxFolderId folderId) throws IOException, ApiException; /** * Used by owner and contacts to retrieve a file. @@ -87,8 +89,8 @@ interface MailboxApi { * * @param file the empty file the response bytes will be written into. */ - void getFile(MailboxProperties properties, MailboxId folderId, - MailboxId fileId, File file) throws IOException, ApiException; + void getFile(MailboxProperties properties, MailboxFolderId folderId, + MailboxFileId fileId, File file) throws IOException, ApiException; /** * Used by owner and contacts to delete files. @@ -98,8 +100,8 @@ interface MailboxApi { * @throws TolerableFailureException on 404 response, * because file was most likely deleted already. */ - void deleteFile(MailboxProperties properties, MailboxId folderId, - MailboxId fileId) + void deleteFile(MailboxProperties properties, MailboxFolderId folderId, + MailboxFileId fileId) throws IOException, ApiException, TolerableFailureException; /** @@ -107,22 +109,23 @@ interface MailboxApi { * for the owner to download. * * @return a list of folder names - * to be used with {@link #getFiles(MailboxProperties, MailboxId)}. + * to be used with {@link #getFiles(MailboxProperties, MailboxFolderId)}. * @throws IllegalArgumentException if used by non-owner. */ - List getFolders(MailboxProperties properties) + List getFolders(MailboxProperties properties) throws IOException, ApiException; @Immutable @JsonSerialize class MailboxContact { public final int contactId; - public final MailboxId token, inboxId, outboxId; + public final MailboxAuthToken token; + public final MailboxFolderId inboxId, outboxId; MailboxContact(ContactId contactId, - MailboxId token, - MailboxId inboxId, - MailboxId outboxId) { + MailboxAuthToken token, + MailboxFolderId inboxId, + MailboxFolderId outboxId) { this.contactId = contactId.getInt(); this.token = token; this.inboxId = inboxId; @@ -132,10 +135,10 @@ interface MailboxApi { @JsonSerialize class MailboxFile implements Comparable { - public final MailboxId name; + public final MailboxFileId name; public final long time; - public MailboxFile(MailboxId name, long time) { + public MailboxFile(MailboxFileId name, long time) { this.name = name; this.time = time; } 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 a09bba09c..9c80bdda7 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 @@ -8,6 +8,10 @@ import com.fasterxml.jackson.databind.node.ObjectNode; import org.briarproject.bramble.api.WeakSingletonProvider; import org.briarproject.bramble.api.contact.ContactId; +import org.briarproject.bramble.api.mailbox.InvalidMailboxIdException; +import org.briarproject.bramble.api.mailbox.MailboxAuthToken; +import org.briarproject.bramble.api.mailbox.MailboxFileId; +import org.briarproject.bramble.api.mailbox.MailboxFolderId; import org.briarproject.bramble.api.mailbox.MailboxId; import org.briarproject.bramble.api.mailbox.MailboxProperties; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; @@ -33,7 +37,6 @@ import static com.fasterxml.jackson.databind.MapperFeature.BLOCK_UNSAFE_POLYMORP import static java.util.Objects.requireNonNull; import static okhttp3.internal.Util.EMPTY_REQUEST; import static org.briarproject.bramble.util.IoUtils.copyAndClose; -import static org.briarproject.bramble.util.StringUtils.fromHexString; @NotNullByDefault class MailboxApiImpl implements MailboxApi { @@ -53,7 +56,7 @@ class MailboxApiImpl implements MailboxApi { } @Override - public MailboxId setup(MailboxProperties properties) + public MailboxAuthToken setup(MailboxProperties properties) throws IOException, ApiException { if (!properties.isOwner()) throw new IllegalArgumentException(); Request request = getRequestBuilder(properties.getAuthToken()) @@ -74,28 +77,12 @@ class MailboxApiImpl implements MailboxApi { throw new ApiException(); } String ownerToken = tokenNode.textValue(); - if (ownerToken == null || !isValidToken(ownerToken)) { - throw new ApiException(); - } - return MailboxId.fromString(ownerToken); - } catch (JacksonException e) { + return MailboxAuthToken.fromString(ownerToken); + } catch (JacksonException | InvalidMailboxIdException e) { throw new ApiException(); } } - // TODO find a batter way to validate (regex?) - // that doesn't do hex string conversion twice - private boolean isValidToken(String token) { - if (token.length() != 64) return false; - try { - // try to convert to bytes - fromHexString(token); - return true; - } catch (IllegalArgumentException e) { - return false; - } - } - @Override public boolean checkStatus(MailboxProperties properties) throws IOException, ApiException { @@ -109,8 +96,7 @@ class MailboxApiImpl implements MailboxApi { @Override public void addContact(MailboxProperties properties, MailboxContact contact) - throws IOException, ApiException, - TolerableFailureException { + throws IOException, ApiException, TolerableFailureException { if (!properties.isOwner()) throw new IllegalArgumentException(); byte[] bodyBytes = mapper.writeValueAsBytes(contact); RequestBody body = RequestBody.create(JSON, bodyBytes); @@ -163,7 +149,7 @@ class MailboxApiImpl implements MailboxApi { /* File Management (owner and contacts) */ @Override - public void addFile(MailboxProperties properties, MailboxId folderId, + public void addFile(MailboxProperties properties, MailboxFolderId folderId, File file) throws IOException, ApiException { String path = "/files/" + folderId; RequestBody body = RequestBody.create(FILE, file); @@ -173,7 +159,7 @@ class MailboxApiImpl implements MailboxApi { @Override public List getFiles(MailboxProperties properties, - MailboxId folderId) throws IOException, ApiException { + MailboxFolderId folderId) throws IOException, ApiException { String path = "/files/" + folderId; Response response = sendGetRequest(properties, path); if (response.code() != 200) throw new ApiException(); @@ -197,20 +183,19 @@ class MailboxApiImpl implements MailboxApi { } String name = nameNode.asText(); long time = timeNode.asLong(); - if (!isValidToken(name)) throw new ApiException(); if (time < 1) throw new ApiException(); - list.add(new MailboxFile(MailboxId.fromString(name), time)); + list.add(new MailboxFile(MailboxFileId.fromString(name), time)); } Collections.sort(list); return list; - } catch (JacksonException e) { + } catch (JacksonException | InvalidMailboxIdException e) { throw new ApiException(); } } @Override - public void getFile(MailboxProperties properties, MailboxId folderId, - MailboxId fileId, File file) throws IOException, ApiException { + public void getFile(MailboxProperties properties, MailboxFolderId folderId, + MailboxFileId fileId, File file) throws IOException, ApiException { String path = "/files/" + folderId + "/" + fileId; Response response = sendGetRequest(properties, path); if (response.code() != 200) throw new ApiException(); @@ -222,8 +207,8 @@ class MailboxApiImpl implements MailboxApi { } @Override - public void deleteFile(MailboxProperties properties, MailboxId folderId, - MailboxId fileId) + public void deleteFile(MailboxProperties properties, + MailboxFolderId folderId, MailboxFileId fileId) throws IOException, ApiException, TolerableFailureException { String path = "/files/" + folderId + "/" + fileId; Request request = getRequestBuilder(properties.getAuthToken()) @@ -237,7 +222,7 @@ class MailboxApiImpl implements MailboxApi { } @Override - public List getFolders(MailboxProperties properties) + public List getFolders(MailboxProperties properties) throws IOException, ApiException { if (!properties.isOwner()) throw new IllegalArgumentException(); Response response = sendGetRequest(properties, "/folders"); @@ -248,7 +233,7 @@ class MailboxApiImpl implements MailboxApi { try { JsonNode node = mapper.readTree(body.string()); ArrayNode filesNode = getArray(node, "folders"); - List list = new ArrayList<>(); + List list = new ArrayList<>(); for (JsonNode fileNode : filesNode) { if (!fileNode.isObject()) throw new ApiException(); ObjectNode objectNode = (ObjectNode) fileNode; @@ -257,11 +242,10 @@ class MailboxApiImpl implements MailboxApi { throw new ApiException(); } String id = idNode.asText(); - if (!isValidToken(id)) throw new ApiException(); - list.add(MailboxId.fromString(id)); + list.add(MailboxFolderId.fromString(id)); } return list; - } catch (JacksonException e) { + } catch (JacksonException | InvalidMailboxIdException e) { throw new ApiException(); } } diff --git a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxSettingsManagerImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxSettingsManagerImpl.java index be18f1b6a..bfb9f085e 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxSettingsManagerImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxSettingsManagerImpl.java @@ -3,7 +3,8 @@ package org.briarproject.bramble.mailbox; import org.briarproject.bramble.api.contact.ContactId; import org.briarproject.bramble.api.db.DbException; import org.briarproject.bramble.api.db.Transaction; -import org.briarproject.bramble.api.mailbox.MailboxId; +import org.briarproject.bramble.api.mailbox.InvalidMailboxIdException; +import org.briarproject.bramble.api.mailbox.MailboxAuthToken; import org.briarproject.bramble.api.mailbox.MailboxProperties; import org.briarproject.bramble.api.mailbox.MailboxSettingsManager; import org.briarproject.bramble.api.mailbox.MailboxStatus; @@ -44,8 +45,12 @@ class MailboxSettingsManagerImpl implements MailboxSettingsManager { String onion = s.get(SETTINGS_KEY_ONION); String token = s.get(SETTINGS_KEY_TOKEN); if (isNullOrEmpty(onion) || isNullOrEmpty(token)) return null; - MailboxId tokenId = MailboxId.fromString(token); - return new MailboxProperties(onion, tokenId, true); + try { + MailboxAuthToken tokenId = MailboxAuthToken.fromString(token); + return new MailboxProperties(onion, tokenId, true); + } catch (InvalidMailboxIdException e) { + throw new DbException(e); + } } @Override 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 05a8b2695..3f6cbafd3 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 @@ -4,6 +4,9 @@ import com.fasterxml.jackson.databind.ObjectMapper; import org.briarproject.bramble.api.WeakSingletonProvider; import org.briarproject.bramble.api.contact.ContactId; +import org.briarproject.bramble.api.mailbox.MailboxAuthToken; +import org.briarproject.bramble.api.mailbox.MailboxFileId; +import org.briarproject.bramble.api.mailbox.MailboxFolderId; import org.briarproject.bramble.api.mailbox.MailboxId; import org.briarproject.bramble.api.mailbox.MailboxProperties; import org.briarproject.bramble.mailbox.MailboxApi.ApiException; @@ -32,8 +35,8 @@ import okio.Buffer; 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.getMailboxId; import static org.briarproject.bramble.test.TestUtils.getRandomBytes; +import static org.briarproject.bramble.test.TestUtils.getRandomId; import static org.briarproject.bramble.test.TestUtils.readBytes; import static org.briarproject.bramble.test.TestUtils.writeBytes; import static org.briarproject.bramble.util.StringUtils.getRandomString; @@ -63,12 +66,15 @@ public class MailboxApiTest extends BrambleTestCase { }; private final MailboxApiImpl api = new MailboxApiImpl(httpClientProvider); - private final MailboxId token = getMailboxId(); - private final MailboxId token2 = getMailboxId(); + private final MailboxAuthToken token = new MailboxAuthToken(getRandomId()); + private final MailboxAuthToken token2 = new MailboxAuthToken(getRandomId()); private final ContactId contactId = getContactId(); - private final MailboxId contactToken = getMailboxId(); - private final MailboxId contactInboxId = getMailboxId(); - private final MailboxId contactOutboxId = getMailboxId(); + private final MailboxAuthToken contactToken = + new MailboxAuthToken(getRandomId()); + private final MailboxFolderId contactInboxId = + new MailboxFolderId(getRandomId()); + private final MailboxFolderId contactOutboxId = + new MailboxFolderId(getRandomId()); private final MailboxContact mailboxContact = new MailboxContact( contactId, contactToken, contactInboxId, contactOutboxId); @@ -428,9 +434,11 @@ public class MailboxApiTest extends BrambleTestCase { @Test public void testGetFiles() throws Exception { - MailboxFile mailboxFile1 = new MailboxFile(getMailboxId(), 1337); + MailboxFile mailboxFile1 = + new MailboxFile(new MailboxFileId(getRandomId()), 1337); MailboxFile mailboxFile2 = - new MailboxFile(getMailboxId(), System.currentTimeMillis()); + new MailboxFile(new MailboxFileId(getRandomId()), + System.currentTimeMillis()); String fileResponse1 = new ObjectMapper().writeValueAsString(mailboxFile1); String fileResponse2 = @@ -539,7 +547,7 @@ public class MailboxApiTest extends BrambleTestCase { @Test public void testGetFile() throws Exception { - MailboxId name = getMailboxId(); + MailboxFileId name = new MailboxFileId(getRandomId()); File file1 = folder.newFile(); File file2 = folder.newFile(); File file3 = folder.newFile(); @@ -586,7 +594,7 @@ public class MailboxApiTest extends BrambleTestCase { @Test public void testDeleteFile() throws Exception { - MailboxId name = getMailboxId(); + MailboxFileId name = new MailboxFileId(getRandomId()); MockWebServer server = new MockWebServer(); server.enqueue(new MockResponse()); @@ -636,8 +644,8 @@ public class MailboxApiTest extends BrambleTestCase { @Test public void testGetFolders() throws Exception { - MailboxId id1 = getMailboxId(); - MailboxId id2 = getMailboxId(); + MailboxFolderId id1 = new MailboxFolderId(getRandomId()); + MailboxFolderId id2 = new MailboxFolderId(getRandomId()); String validResponse1 = "{\"folders\": [ {\"id\": \"" + id1 + "\"} ] }"; String validResponse2 = "{\"folders\": [ {\"id\": \"" + id1 + "\"}, " + "{ \"id\": \"" + id2 + "\"} ] }"; diff --git a/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxIntegrationTest.java b/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxIntegrationTest.java index 7d98e93c2..eccb311ed 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxIntegrationTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxIntegrationTest.java @@ -2,7 +2,10 @@ package org.briarproject.bramble.mailbox; import org.briarproject.bramble.api.WeakSingletonProvider; import org.briarproject.bramble.api.contact.ContactId; -import org.briarproject.bramble.api.mailbox.MailboxId; +import org.briarproject.bramble.api.mailbox.InvalidMailboxIdException; +import org.briarproject.bramble.api.mailbox.MailboxAuthToken; +import org.briarproject.bramble.api.mailbox.MailboxFileId; +import org.briarproject.bramble.api.mailbox.MailboxFolderId; import org.briarproject.bramble.api.mailbox.MailboxProperties; import org.briarproject.bramble.mailbox.MailboxApi.ApiException; import org.briarproject.bramble.mailbox.MailboxApi.MailboxContact; @@ -28,8 +31,8 @@ import okhttp3.OkHttpClient; import static java.util.Collections.emptyList; import static java.util.Collections.singletonList; import static java.util.concurrent.TimeUnit.MILLISECONDS; -import static org.briarproject.bramble.test.TestUtils.getMailboxId; import static org.briarproject.bramble.test.TestUtils.getRandomBytes; +import static org.briarproject.bramble.test.TestUtils.getRandomId; import static org.briarproject.bramble.test.TestUtils.isOptionalTestEnabled; import static org.briarproject.bramble.test.TestUtils.readBytes; import static org.briarproject.bramble.test.TestUtils.writeBytes; @@ -45,8 +48,16 @@ public class MailboxIntegrationTest extends BrambleTestCase { public TemporaryFolder folder = new TemporaryFolder(); private final static String URL_BASE = "http://127.0.0.1:8000"; - private final static MailboxId SETUP_TOKEN = MailboxId.fromString( - "54686973206973206120736574757020746f6b656e20666f722042726961722e"); + private final static MailboxAuthToken SETUP_TOKEN; + + static { + try { + SETUP_TOKEN = MailboxAuthToken.fromString( + "54686973206973206120736574757020746f6b656e20666f722042726961722e"); + } catch (InvalidMailboxIdException e) { + throw new IllegalStateException(); + } + } private final OkHttpClient client = new OkHttpClient.Builder() .socketFactory(SocketFactory.getDefault()) @@ -77,7 +88,7 @@ public class MailboxIntegrationTest extends BrambleTestCase { if (ownerProperties != null) return; MailboxProperties setupProperties = new MailboxProperties(URL_BASE, SETUP_TOKEN, true); - MailboxId ownerToken = api.setup(setupProperties); + MailboxAuthToken ownerToken = api.setup(setupProperties); ownerProperties = new MailboxProperties(URL_BASE, ownerToken, true); } @@ -133,7 +144,7 @@ public class MailboxIntegrationTest extends BrambleTestCase { List files1 = api.getFiles(contactProperties, contact.inboxId); assertEquals(1, files1.size()); - MailboxId fileName1 = files1.get(0).name; + MailboxFileId fileName1 = files1.get(0).name; // owner can't check files assertThrows(ApiException.class, () -> @@ -171,15 +182,15 @@ public class MailboxIntegrationTest extends BrambleTestCase { api.addFile(contactProperties, contact.outboxId, file3); // owner checks folders with available files - List folders = api.getFolders(ownerProperties); + List folders = api.getFolders(ownerProperties); assertEquals(singletonList(contact.outboxId), folders); // owner lists files in contact's outbox List files2 = api.getFiles(ownerProperties, contact.outboxId); assertEquals(2, files2.size()); - MailboxId file2name = files2.get(0).name; - MailboxId file3name = files2.get(1).name; + MailboxFileId file2name = files2.get(0).name; + MailboxFileId file3name = files2.get(1).name; // contact can't list files in contact's outbox assertThrows(ApiException.class, () -> @@ -232,8 +243,10 @@ public class MailboxIntegrationTest extends BrambleTestCase { } private MailboxContact getMailboxContact(ContactId contactId) { - return new MailboxContact(contactId, getMailboxId(), getMailboxId(), - getMailboxId()); + MailboxAuthToken authToken = new MailboxAuthToken(getRandomId()); + MailboxFolderId inboxId = new MailboxFolderId(getRandomId()); + MailboxFolderId outboxId = new MailboxFolderId(getRandomId()); + return new MailboxContact(contactId, authToken, inboxId, outboxId); } } diff --git a/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxSettingsManagerImplTest.java b/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxSettingsManagerImplTest.java index 3df58bef5..57aa8894a 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxSettingsManagerImplTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxSettingsManagerImplTest.java @@ -2,7 +2,7 @@ package org.briarproject.bramble.mailbox; import org.briarproject.bramble.api.contact.ContactId; import org.briarproject.bramble.api.db.Transaction; -import org.briarproject.bramble.api.mailbox.MailboxId; +import org.briarproject.bramble.api.mailbox.MailboxAuthToken; import org.briarproject.bramble.api.mailbox.MailboxProperties; import org.briarproject.bramble.api.mailbox.MailboxSettingsManager; import org.briarproject.bramble.api.mailbox.MailboxStatus; @@ -21,7 +21,7 @@ import static org.briarproject.bramble.mailbox.MailboxSettingsManagerImpl.SETTIN import static org.briarproject.bramble.mailbox.MailboxSettingsManagerImpl.SETTINGS_KEY_TOKEN; import static org.briarproject.bramble.mailbox.MailboxSettingsManagerImpl.SETTINGS_NAMESPACE; import static org.briarproject.bramble.mailbox.MailboxSettingsManagerImpl.SETTINGS_UPLOADS_NAMESPACE; -import static org.briarproject.bramble.test.TestUtils.getMailboxId; +import static org.briarproject.bramble.test.TestUtils.getRandomId; import static org.briarproject.bramble.util.StringUtils.getRandomString; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; @@ -37,7 +37,7 @@ public class MailboxSettingsManagerImplTest extends BrambleMockTestCase { new MailboxSettingsManagerImpl(settingsManager); private final Random random = new Random(); private final String onion = getRandomString(64); - private final MailboxId token = getMailboxId(); + private final MailboxAuthToken token = new MailboxAuthToken(getRandomId()); private final ContactId contactId1 = new ContactId(random.nextInt()); private final ContactId contactId2 = new ContactId(random.nextInt()); private final ContactId contactId3 = new ContactId(random.nextInt()); @@ -195,7 +195,7 @@ public class MailboxSettingsManagerImplTest extends BrambleMockTestCase { assertEquals(onion, filename1); String filename2 = manager.getPendingUpload(txn, contactId2); assertNotNull(filename2); - assertEquals(token, MailboxId.fromString(filename2)); + assertEquals(token.toString(), filename2); String filename3 = manager.getPendingUpload(txn, contactId3); assertNull(filename3); String filename4 =