Merge branch '1825-pending-contact-error' into 'master'

Be more specific about errors when adding pending contact

Closes #1825

See merge request briar/briar!1354

briar-headless/README.md

@@ -105,7 +105,7 @@ The link and the alias should be posted as a JSON object:
 }
 ```
 
-This starts the process of adding the contact.
+Adding a pending contact starts the process of adding the contact.
 Until it is completed, a pending contact is returned as JSON:
 
 ```json
@@ -116,6 +116,71 @@ Until it is completed, a pending contact is returned as JSON:
 }
 ```
 
+Possible errors when adding a pending contact are:
+
+#### 400: Pending contact's link is invalid
+
+```json
+{
+    "error": "INVALID_LINK"
+}
+```
+
+#### 400: Pending contact's handshake public key is invalid
+
+```json
+{
+    "error": "INVALID_PUBLIC_KEY"
+}
+```
+
+#### 403: A contact with the same handshake public key already exists
+
+This error may be caused by someone attacking the user with the goal
+of discovering the contacts of the user.
+
+In the Android client, upon encountering this issue a message dialog
+is shown that asks whether the contact and the just added pending contact
+are the same person. If that's the case, a message is shown that the
+contact already exists and the pending contact isn't added.
+If that's not the case and they are two different persons, the Android
+client
+[shows the following message](https://code.briarproject.org/briar/briar/-/blob/beta-1.2.14/briar-android/src/main/res/values/strings.xml#L271)
+when this happens:
+> [Alice] and [Bob] sent you the same link.
+>
+> One of them may be trying to discover who your contacts are.
+>
+> Don't tell them you received the same link from someone else.
+
+```json
+{
+    "error": "CONTACT_EXISTS",
+    "remoteAuthorName": "Bob"
+}
+```
+
+#### 403: A pending contact with the same handshake public key already exists
+
+This error, too, may be caused by someone attacking the user with the goal
+of discovering the contacts of the user.
+
+Just like above, upon encountering this issue a message dialog is shown in
+the Android client that asks whether the contact and the just added pending
+contact are the same person. If that's the case, the pending contact gets
+updated. If that's not the case and they are two different persons, the
+Android client shows the same message as above, warning the user about the
+possible attack.
+
+```json
+{
+    "error": "PENDING_EXISTS",
+    "pendingContactId": "jsTgWcsEQ2g9rnomeK1g/hmO8M1Ix6ZIGWAjgBtlS9U=",
+    "pendingContactAlias": "Alice"
+}
+```
+-----------
+
 Before users can send messages to contacts, they become pending contacts.
 In this state Briar still needs to do some work in the background (e.g.
 spinning up a dedicated hidden service and letting the contact connect to it).

briar-headless/src/main/java/org/briarproject/briar/headless/contact/ContactControllerImpl.kt

@@ -3,6 +3,8 @@ package org.briarproject.briar.headless.contact
 import com.fasterxml.jackson.databind.ObjectMapper
 import io.javalin.http.BadRequestResponse
 import io.javalin.http.Context
+import io.javalin.http.ForbiddenResponse
+import io.javalin.http.HttpResponseException
 import io.javalin.http.NotFoundResponse
 import org.briarproject.bramble.api.connection.ConnectionRegistry
 import org.briarproject.bramble.api.contact.ContactManager
@@ -12,8 +14,10 @@ import org.briarproject.bramble.api.contact.event.ContactAddedEvent
 import org.briarproject.bramble.api.contact.event.PendingContactAddedEvent
 import org.briarproject.bramble.api.contact.event.PendingContactRemovedEvent
 import org.briarproject.bramble.api.contact.event.PendingContactStateChangedEvent
+import org.briarproject.bramble.api.db.ContactExistsException
 import org.briarproject.bramble.api.db.NoSuchContactException
 import org.briarproject.bramble.api.db.NoSuchPendingContactException
+import org.briarproject.bramble.api.db.PendingContactExistsException
 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
@@ -25,8 +29,11 @@ import org.briarproject.briar.headless.event.WebSocketController
 import org.briarproject.briar.headless.getContactIdFromPathParam
 import org.briarproject.briar.headless.getFromJson
 import org.briarproject.briar.headless.json.JsonDict
+import org.eclipse.jetty.http.HttpStatus.BAD_REQUEST_400
+import org.eclipse.jetty.http.HttpStatus.FORBIDDEN_403
 import org.spongycastle.util.encoders.Base64
 import org.spongycastle.util.encoders.DecoderException
+import java.security.GeneralSecurityException
 import javax.annotation.concurrent.Immutable
 import javax.inject.Inject
 import javax.inject.Singleton
@@ -91,9 +98,32 @@ 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")
+        if (!LINK_REGEX.matcher(link).find()) {
+            ctx.status(BAD_REQUEST_400)
+            val details = mapOf("error" to "INVALID_LINK")
+            return ctx.json(details)
+        }
         checkAliasLength(alias)
-        val pendingContact = contactManager.addPendingContact(link, alias)
+        val pendingContact = try {
+            contactManager.addPendingContact(link, alias)
+        } catch (e: GeneralSecurityException) {
+            ctx.status(BAD_REQUEST_400)
+            val details = mapOf("error" to "INVALID_PUBLIC_KEY")
+            return ctx.json(details)
+        } catch (e: ContactExistsException) {
+            ctx.status(FORBIDDEN_403)
+            val details =
+                mapOf("error" to "CONTACT_EXISTS", "remoteAuthorName" to e.remoteAuthor.name)
+            return ctx.json(details)
+        } catch (e: PendingContactExistsException) {
+            ctx.status(FORBIDDEN_403)
+            val details = mapOf(
+                "error" to "PENDING_EXISTS",
+                "pendingContactId" to e.pendingContact.id.bytes,
+                "pendingContactAlias" to e.pendingContact.alias
+            )
+            return ctx.json(details)
+        }
         return ctx.json(pendingContact.output())
     }
 

briar-headless/src/test/java/org/briarproject/briar/headless/contact/ContactControllerIntegrationTest.kt

@@ -98,6 +98,49 @@ class ContactControllerIntegrationTest: IntegrationTest() {
         assertEquals(401, response.statusCode)
     }
 
+    @Test
+    fun `adding a pending contact with invalid link`() {
+        val alias = "AliasFoo"
+        val json = """{
+            "link": "briar://invalid",
+            "alias": "$alias"
+        }"""
+        val response = post("$url/contacts/add/pending", json)
+        assertEquals(400, response.statusCode)
+        assertEquals("INVALID_LINK", response.jsonObject.getString("error"))
+    }
+
+    @Test
+    fun `adding a pending contact with invalid public key`() {
+        val alias = "AliasFoo"
+        val json = """{
+            "link": "briar://aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
+            "alias": "$alias"
+        }"""
+        val response = post("$url/contacts/add/pending", json)
+        assertEquals(400, response.statusCode)
+        assertEquals("INVALID_PUBLIC_KEY", response.jsonObject.getString("error"))
+    }
+
+    @Test
+    fun `adding a pending contact that already exists as pending contact`() {
+        val alias = "AliasFoo"
+        val json = """{
+            "link": "${getRealHandshakeLink(crypto)}",
+            "alias": "$alias"
+        }"""
+        var response = post("$url/contacts/add/pending", json)
+        assertEquals(200, response.statusCode)
+
+        val pendingContactId = response.jsonObject.getString("pendingContactId")
+
+        response = post("$url/contacts/add/pending", json)
+        assertEquals(403, response.statusCode)
+        assertEquals("PENDING_EXISTS", response.jsonObject.getString("error"))
+        assertEquals(pendingContactId, response.jsonObject.getString("pendingContactId"))
+        assertEquals(alias, response.jsonObject.getString("pendingContactAlias"))
+    }
+
     @Test
     fun `removing a pending contact needs authentication token`() {
         val response = deleteWithWrongToken("$url/contacts/add/pending")

briar-headless/src/test/java/org/briarproject/briar/headless/contact/ContactControllerTest.kt

@@ -1,6 +1,8 @@
 package org.briarproject.briar.headless.contact
 
 import io.javalin.http.BadRequestResponse
+import io.javalin.http.ForbiddenResponse
+import io.javalin.http.HttpResponseException
 import io.javalin.http.NotFoundResponse
 import io.javalin.plugin.json.JavalinJson.toJson
 import io.mockk.Runs
@@ -8,6 +10,7 @@ import io.mockk.every
 import io.mockk.just
 import io.mockk.mockkStatic
 import io.mockk.runs
+import io.mockk.verify
 import org.briarproject.bramble.api.Pair
 import org.briarproject.bramble.api.contact.Contact
 import org.briarproject.bramble.api.contact.ContactId
@@ -18,8 +21,10 @@ import org.briarproject.bramble.api.contact.event.ContactAddedEvent
 import org.briarproject.bramble.api.contact.event.PendingContactAddedEvent
 import org.briarproject.bramble.api.contact.event.PendingContactRemovedEvent
 import org.briarproject.bramble.api.contact.event.PendingContactStateChangedEvent
+import org.briarproject.bramble.api.db.ContactExistsException
 import org.briarproject.bramble.api.db.NoSuchContactException
 import org.briarproject.bramble.api.db.NoSuchPendingContactException
+import org.briarproject.bramble.api.db.PendingContactExistsException
 import org.briarproject.bramble.api.identity.AuthorConstants.MAX_AUTHOR_NAME_LENGTH
 import org.briarproject.bramble.api.plugin.event.ContactConnectedEvent
 import org.briarproject.bramble.api.plugin.event.ContactDisconnectedEvent
@@ -30,9 +35,11 @@ import org.briarproject.bramble.util.StringUtils.getRandomString
 import org.briarproject.briar.headless.ControllerTest
 import org.briarproject.briar.headless.getFromJson
 import org.briarproject.briar.headless.json.JsonDict
+import org.junit.jupiter.api.Assertions.assertEquals
 import org.junit.jupiter.api.Assertions.assertNotNull
 import org.junit.jupiter.api.Assertions.assertThrows
 import org.junit.jupiter.api.Test
+import java.security.GeneralSecurityException
 import kotlin.random.Random
 
 internal class ContactControllerTest : ControllerTest() {
@@ -96,9 +103,10 @@ internal class ContactControllerTest : ControllerTest() {
             "alias": "$alias"
         }"""
         every { ctx.body() } returns body
-        assertThrows(BadRequestResponse::class.java) {
-            controller.addPendingContact(ctx)
-        }
+        every { ctx.status(400) } returns ctx
+        every { ctx.json(mapOf("error" to "INVALID_LINK")) } returns ctx
+        controller.addPendingContact(ctx)
+        verify { ctx.status(400) }
     }
 
     @Test
@@ -139,6 +147,84 @@ internal class ContactControllerTest : ControllerTest() {
         }
     }
 
+    @Test
+    fun testAddPendingContactPublicKeyInvalid() {
+        val link = "briar://adnsyffpsenoc3yzlhr24aegfq2pwan7kkselocill2choov6sbhs"
+        val alias = "Alias123"
+        val body = """{
+            "link": "$link",
+            "alias": "$alias"
+        }"""
+        every { ctx.body() } returns body
+        every { ctx.status(400) } returns ctx
+        every {
+            contactManager.addPendingContact(
+                link,
+                alias
+            )
+        } throws GeneralSecurityException()
+        every { ctx.json(mapOf("error" to "INVALID_PUBLIC_KEY")) } returns ctx
+        controller.addPendingContact(ctx)
+        verify { ctx.status(400) }
+    }
+
+    @Test
+    fun testAddPendingContactSameContactKey() {
+        val link = "briar://adnsyffpsenoc3yzlhr24aegfq2pwan7kkselocill2choov6sbhs"
+        val alias = "Alias123"
+        val body = """{
+            "link": "$link",
+            "alias": "$alias"
+        }"""
+        every { ctx.body() } returns body
+        every { ctx.status(403) } returns ctx
+        every {
+            contactManager.addPendingContact(
+                link,
+                alias
+            )
+        } throws ContactExistsException(null, author)
+        every {
+            ctx.json(
+                mapOf(
+                    "error" to "CONTACT_EXISTS",
+                    "remoteAuthorName" to author.name
+                )
+            )
+        } returns ctx
+        controller.addPendingContact(ctx)
+        verify { ctx.status(403) }
+    }
+
+    @Test
+    fun testAddPendingContactSamePendingContactKey() {
+        val link = "briar://adnsyffpsenoc3yzlhr24aegfq2pwan7kkselocill2choov6sbhs"
+        val alias = "Alias123"
+        val body = """{
+            "link": "$link",
+            "alias": "$alias"
+        }"""
+        every { ctx.body() } returns body
+        every { ctx.status(403) } returns ctx
+        every {
+            contactManager.addPendingContact(
+                link,
+                alias
+            )
+        } throws PendingContactExistsException(pendingContact)
+        every {
+            ctx.json(
+                mapOf(
+                    "error" to "PENDING_EXISTS",
+                    "pendingContactId" to pendingContact.id.bytes,
+                    "pendingContactAlias" to pendingContact.alias
+                )
+            )
+        } returns ctx
+        controller.addPendingContact(ctx)
+        verify { ctx.status(403) }
+    }
+
     @Test
     fun testListPendingContacts() {
         every { contactManager.pendingContacts } returns listOf(