Be more specific about errors when adding pending contact

Following the docs at https://code.briarproject.org/briar/briar/-/blob/beta-1.2.14/bramble-api/src/main/java/org/briarproject/bramble/api/contact/ContactManager.java#L110 Fixes #1825
2026-02-15 12:19:54 +01:00 · 2021-02-02 12:00:00 +00:00
parent 6fbc82ee27
commit 7fab97d26c
4 changed files with 195 additions and 6 deletions
--- 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
@@ -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,27 @@ 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")
+            return ctx.json(details)
+        } catch (e: PendingContactExistsException) {
+            ctx.status(FORBIDDEN_403)
+            val details = mapOf("error" to "PENDING_EXISTS")
+            return ctx.json(details)
+        }
        return ctx.json(pendingContact.output())
    }

--- a/briar-headless/src/test/java/org/briarproject/briar/headless/contact/ContactControllerIntegrationTest.kt
+++ b/briar-headless/src/test/java/org/briarproject/briar/headless/contact/ContactControllerIntegrationTest.kt
@@ -98,6 +98,45 @@ 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)
+
+        response = post("$url/contacts/add/pending", json)
+        assertEquals(403, response.statusCode)
+        assertEquals("PENDING_EXISTS", response.jsonObject.getString("error"))
+    }
+
    @Test
    fun `removing a pending contact needs authentication token`() {
        val response = deleteWithWrongToken("$url/contacts/add/pending")
--- 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,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,69 @@ 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, null)
+        every { ctx.json(mapOf("error" to "CONTACT_EXISTS")) } 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(null)
+        every { ctx.json(mapOf("error" to "PENDING_EXISTS")) } returns ctx
+        controller.addPendingContact(ctx)
+        verify { ctx.status(403) }
+    }
+
    @Test
    fun testListPendingContacts() {
        every { contactManager.pendingContacts } returns listOf(