From 435b43488a706ac38d6c7e069330297af54ca48c Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Wed, 15 May 2019 12:36:33 -0300 Subject: [PATCH] [headless] address review comments for remote contact adding --- .../briar/test/BriarTestUtils.java | 3 +- briar-headless/README.md | 22 +++--- .../bramble/identity/OutputAuthor.kt | 2 +- .../org/briarproject/briar/headless/Router.kt | 2 +- .../headless/contact/ContactController.kt | 2 +- .../headless/contact/ContactControllerImpl.kt | 10 ++- .../briar/headless/contact/OutputContact.kt | 9 ++- .../headless/contact/ContactControllerTest.kt | 72 +++++++++++++++++-- 8 files changed, 102 insertions(+), 20 deletions(-) diff --git a/briar-core/src/test/java/org/briarproject/briar/test/BriarTestUtils.java b/briar-core/src/test/java/org/briarproject/briar/test/BriarTestUtils.java index b99ab2c36..8f6f97226 100644 --- a/briar-core/src/test/java/org/briarproject/briar/test/BriarTestUtils.java +++ b/briar-core/src/test/java/org/briarproject/briar/test/BriarTestUtils.java @@ -49,9 +49,8 @@ public class BriarTestUtils { public static String getRealHandshakeLink(CryptoComponent cryptoComponent) { KeyPair keyPair = cryptoComponent.generateAgreementKeyPair(); byte[] linkBytes = new byte[RAW_LINK_BYTES]; - byte[] version = new byte[] {FORMAT_VERSION}; byte[] publicKey = keyPair.getPublic().getEncoded(); - arraycopy(version,0, linkBytes, 0, 1); + linkBytes[0] = FORMAT_VERSION; arraycopy(publicKey,0, linkBytes, 1, RAW_LINK_BYTES - 1); return ("briar://" + Base32.encode(linkBytes)).toLowerCase(); } diff --git a/briar-headless/README.md b/briar-headless/README.md index a6ac23cdf..02a7d40d2 100644 --- a/briar-headless/README.md +++ b/briar-headless/README.md @@ -65,6 +65,8 @@ Returns a JSON array of contacts: "publicKey": "BDu6h1S02bF4W6rgoZfZ6BMjTj/9S9hNN7EQoV05qUo=" }, "contactId": 1, + "alias" : "A local nickname", + "handshakePublicKey": "XnYRd7a7E4CTqgAvh4hCxh/YZ0EPscxknB9ZcEOpSzY=", "verified": true } ``` @@ -282,14 +284,18 @@ it will send a JSON object representing the new contact to connected websocket c ```json { "data": { - "author": { - "formatVersion": 1, - "id": "y1wkIzAimAbYoCGgWxkWlr6vnq1F8t1QRA/UMPgI0E0=", - "name": "Test", - "publicKey": "BDu6h1S02bF4W6rgoZfZ6BMjTj/9S9hNN7EQoV05qUo=" - }, - "contactId": 1, - "verified": true + "contact": { + "author": { + "formatVersion": 1, + "id": "y1wkIzAimAbYoCGgWxkWlr6vnq1F8t1QRA/UMPgI0E0=", + "name": "Test", + "publicKey": "BDu6h1S02bF4W6rgoZfZ6BMjTj/9S9hNN7EQoV05qUo=" + }, + "contactId": 1, + "alias" : "A local nickname", + "handshakePublicKey": "XnYRd7a7E4CTqgAvh4hCxh/YZ0EPscxknB9ZcEOpSzY=", + "verified": true + } }, "name": "ContactAddedRemotelyEvent", "type": "event" diff --git a/briar-headless/src/main/java/org/briarproject/bramble/identity/OutputAuthor.kt b/briar-headless/src/main/java/org/briarproject/bramble/identity/OutputAuthor.kt index 3b6617582..cbdb0387e 100644 --- a/briar-headless/src/main/java/org/briarproject/bramble/identity/OutputAuthor.kt +++ b/briar-headless/src/main/java/org/briarproject/bramble/identity/OutputAuthor.kt @@ -8,7 +8,7 @@ fun Author.output() = JsonDict( "formatVersion" to formatVersion, "id" to id.bytes, "name" to name, - "publicKey" to publicKey + "publicKey" to publicKey.encoded ) fun AuthorInfo.Status.output() = name.toLowerCase() diff --git a/briar-headless/src/main/java/org/briarproject/briar/headless/Router.kt b/briar-headless/src/main/java/org/briarproject/briar/headless/Router.kt index 12afce4f5..bb1f49cae 100644 --- a/briar-headless/src/main/java/org/briarproject/briar/headless/Router.kt +++ b/briar-headless/src/main/java/org/briarproject/briar/headless/Router.kt @@ -66,7 +66,7 @@ constructor( get { ctx -> contactController.list(ctx) } path("add") { path("link") { - get { ctx -> contactController.link(ctx) } + get { ctx -> contactController.getLink(ctx) } } path("pending") { get { ctx -> contactController.listPendingContacts(ctx) } diff --git a/briar-headless/src/main/java/org/briarproject/briar/headless/contact/ContactController.kt b/briar-headless/src/main/java/org/briarproject/briar/headless/contact/ContactController.kt index cb454ddba..120015f02 100644 --- a/briar-headless/src/main/java/org/briarproject/briar/headless/contact/ContactController.kt +++ b/briar-headless/src/main/java/org/briarproject/briar/headless/contact/ContactController.kt @@ -5,7 +5,7 @@ import io.javalin.Context interface ContactController { fun list(ctx: Context): Context - fun link(ctx: Context): Context + fun getLink(ctx: Context): Context fun addPendingContact(ctx: Context): Context fun listPendingContacts(ctx: Context): Context fun removePendingContact(ctx: Context): Context diff --git a/briar-headless/src/main/java/org/briarproject/briar/headless/contact/ContactControllerImpl.kt b/briar-headless/src/main/java/org/briarproject/briar/headless/contact/ContactControllerImpl.kt index 16a17a532..657649260 100644 --- a/briar-headless/src/main/java/org/briarproject/briar/headless/contact/ContactControllerImpl.kt +++ b/briar-headless/src/main/java/org/briarproject/briar/headless/contact/ContactControllerImpl.kt @@ -1,9 +1,11 @@ package org.briarproject.briar.headless.contact import com.fasterxml.jackson.databind.ObjectMapper +import io.javalin.BadRequestResponse import io.javalin.Context import io.javalin.NotFoundResponse import org.briarproject.bramble.api.contact.ContactManager +import org.briarproject.bramble.api.contact.HandshakeLinkConstants.LINK_REGEX import org.briarproject.bramble.api.contact.PendingContactId import org.briarproject.bramble.api.contact.event.ContactAddedRemotelyEvent import org.briarproject.bramble.api.contact.event.PendingContactRemovedEvent @@ -12,6 +14,8 @@ import org.briarproject.bramble.api.db.NoSuchContactException import org.briarproject.bramble.api.db.NoSuchPendingContactException import org.briarproject.bramble.api.event.Event import org.briarproject.bramble.api.event.EventListener +import org.briarproject.bramble.api.identity.AuthorConstants.MAX_AUTHOR_NAME_LENGTH +import org.briarproject.bramble.util.StringUtils.toUtf8 import org.briarproject.briar.headless.event.WebSocketController import org.briarproject.briar.headless.getContactIdFromPathParam import org.briarproject.briar.headless.getFromJson @@ -57,7 +61,7 @@ constructor( return ctx.json(contacts) } - override fun link(ctx: Context): Context { + override fun getLink(ctx: Context): Context { val linkDict = JsonDict("link" to contactManager.handshakeLink) return ctx.json(linkDict) } @@ -65,6 +69,10 @@ constructor( override fun addPendingContact(ctx: Context): Context { val link = ctx.getFromJson(objectMapper, "link") val alias = ctx.getFromJson(objectMapper, "alias") + if (!LINK_REGEX.matcher(link).find()) throw BadRequestResponse("Invalid Link") + val aliasUtf8 = toUtf8(alias) + if (aliasUtf8.isEmpty() || aliasUtf8.size > MAX_AUTHOR_NAME_LENGTH) + throw BadRequestResponse("Invalid Alias") val pendingContact = contactManager.addPendingContact(link, alias) return ctx.json(pendingContact.output()) } diff --git a/briar-headless/src/main/java/org/briarproject/briar/headless/contact/OutputContact.kt b/briar-headless/src/main/java/org/briarproject/briar/headless/contact/OutputContact.kt index 232ce74dc..e4b6cf7e1 100644 --- a/briar-headless/src/main/java/org/briarproject/briar/headless/contact/OutputContact.kt +++ b/briar-headless/src/main/java/org/briarproject/briar/headless/contact/OutputContact.kt @@ -8,7 +8,12 @@ import org.briarproject.briar.headless.json.JsonDict internal fun Contact.output() = JsonDict( "contactId" to id.int, "author" to author.output(), + "alias" to alias, "verified" to isVerified -) +).apply { + handshakePublicKey?.let { put("handshakePublicKey", it.encoded) } +} -internal fun ContactAddedRemotelyEvent.output() = contact.output() +internal fun ContactAddedRemotelyEvent.output() = JsonDict( + "contact" to contact.output() +) diff --git a/briar-headless/src/test/java/org/briarproject/briar/headless/contact/ContactControllerTest.kt b/briar-headless/src/test/java/org/briarproject/briar/headless/contact/ContactControllerTest.kt index 118c279f6..05128f28a 100644 --- a/briar-headless/src/test/java/org/briarproject/briar/headless/contact/ContactControllerTest.kt +++ b/briar-headless/src/test/java/org/briarproject/briar/headless/contact/ContactControllerTest.kt @@ -1,5 +1,6 @@ package org.briarproject.briar.headless.contact +import io.javalin.BadRequestResponse import io.javalin.NotFoundResponse import io.javalin.json.JavalinJson.toJson import io.mockk.Runs @@ -15,11 +16,14 @@ import org.briarproject.bramble.api.contact.event.PendingContactRemovedEvent import org.briarproject.bramble.api.contact.event.PendingContactStateChangedEvent import org.briarproject.bramble.api.db.NoSuchContactException import org.briarproject.bramble.api.db.NoSuchPendingContactException +import org.briarproject.bramble.api.identity.AuthorConstants.MAX_AUTHOR_NAME_LENGTH import org.briarproject.bramble.identity.output import org.briarproject.bramble.test.TestUtils.getPendingContact import org.briarproject.bramble.test.TestUtils.getRandomBytes +import org.briarproject.bramble.util.StringUtils.getRandomString import org.briarproject.briar.headless.ControllerTest import org.briarproject.briar.headless.json.JsonDict +import org.junit.jupiter.api.Assertions.assertNotNull import org.junit.jupiter.api.Assertions.assertThrows import org.junit.jupiter.api.Test @@ -49,12 +53,12 @@ internal class ContactControllerTest : ControllerTest() { val link = "briar://link" every { contactManager.handshakeLink } returns link every { ctx.json(JsonDict("link" to link)) } returns ctx - controller.link(ctx) + controller.getLink(ctx) } @Test fun testAddPendingContact() { - val link = "briar://link123" + val link = "briar://briar://adnsyffpsenoc3yzlhr24aegfq2pwan7kkselocill2choov6sbhs" val alias = "Alias123" val body = """{ "link": "$link", @@ -66,6 +70,58 @@ internal class ContactControllerTest : ControllerTest() { controller.addPendingContact(ctx) } + @Test + fun testAddPendingContactInvalidLink() { + val link = "briar://link123" + val alias = "Alias123" + val body = """{ + "link": "$link", + "alias": "$alias" + }""" + every { ctx.body() } returns body + assertThrows(BadRequestResponse::class.java) { + controller.addPendingContact(ctx) + } + } + + @Test + fun testAddPendingContactMissingLink() { + val alias = "Alias123" + val body = """{ + "alias": "$alias" + }""" + every { ctx.body() } returns body + assertThrows(BadRequestResponse::class.java) { + controller.addPendingContact(ctx) + } + } + + @Test + fun testAddPendingContactInvalidAlias() { + val link = "briar://briar://adnsyffpsenoc3yzlhr24aegfq2pwan7kkselocill2choov6sbhs" + val alias = getRandomString(MAX_AUTHOR_NAME_LENGTH + 1) + val body = """{ + "link": "$link", + "alias": "$alias" + }""" + every { ctx.body() } returns body + assertThrows(BadRequestResponse::class.java) { + controller.addPendingContact(ctx) + } + } + + @Test + fun testAddPendingContactMissingAlias() { + val link = "briar://adnsyffpsenoc3yzlhr24aegfq2pwan7kkselocill2choov6sbhs" + val body = """{ + "link": "$link" + }""" + every { ctx.body() } returns body + assertThrows(BadRequestResponse::class.java) { + controller.addPendingContact(ctx) + } + } + @Test fun testListPendingContacts() { every { contactManager.pendingContacts } returns listOf(pendingContact) @@ -185,10 +241,13 @@ internal class ContactControllerTest : ControllerTest() { @Test fun testOutputContact() { + assertNotNull(contact.handshakePublicKey) val json = """ { "contactId": ${contact.id.int}, "author": ${toJson(author.output())}, + "alias" : "${contact.alias}", + "handshakePublicKey": ${toJson(contact.handshakePublicKey!!.encoded)}, "verified": ${contact.isVerified} } """ @@ -202,7 +261,7 @@ internal class ContactControllerTest : ControllerTest() { "formatVersion": 1, "id": ${toJson(author.id.bytes)}, "name": "${author.name}", - "publicKey": ${toJson(author.publicKey)} + "publicKey": ${toJson(author.publicKey.encoded)} } """ assertJsonEquals(json, author.output()) @@ -211,7 +270,12 @@ internal class ContactControllerTest : ControllerTest() { @Test fun testOutputContactAddedRemotelyEvent() { val event = ContactAddedRemotelyEvent(contact) - assertJsonEquals(toJson(contact.output()), event.output()) + val json = """ + { + "contact": ${toJson(contact.output())} + } + """ + assertJsonEquals(json, event.output()) } @Test