[ANCHOR-1405]: Resolve db-mode client lookups by client type

extended-tests/src/test/kotlin/org/stellar/anchor/platform/extendedtest/clients/db/ClientsDbAttributionTests.kt

@@ -126,6 +126,35 @@ class ClientsDbAttributionTests : IntegrationTestBase(TestConfig()) {
     assertNull(authenticate(keypair).clientName)
   }
 
+  @Test
+  fun `test SEP-10 leaves client_name unset for the signing key of a noncustodial client`() {
+    val name = uniqueClientName()
+    val keypair = KeyPair.random()
+    val createJson =
+      gson.toJson(
+        mapOf(
+          "type" to "noncustodial",
+          "domains" to listOf("$name.example.com"),
+          "signingKeys" to listOf(keypair.accountId),
+          "allowAnyDestination" to true,
+        )
+      )
+
+    try {
+      assertEquals(
+        200,
+        httpClient
+          .newCall(OkHttpUtil.buildJsonPutRequest("$clientsUrl/$name", createJson))
+          .execute()
+          .code,
+      )
+
+      assertNull(authenticate(keypair).clientName)
+    } finally {
+      delete(name)
+    }
+  }
+
   @Test
   fun `test SEP-10 no longer attributes client_name after the client is deleted`() {
     val name = uniqueClientName()

platform/src/main/java/org/stellar/anchor/platform/data/JdbcClientService.java

@@ -44,13 +44,17 @@ public ClientConfig getClientConfigByName(String clientName) {
   @Override
   public CustodialClient getClientConfigBySigningKey(String signingKey) {
     JdbcClientConfig found = repo.findBySigningKey(signingKey);
-    return found == null ? null : toCustodialClient(found);
+    return found != null && found.getType() == ClientType.CUSTODIAL
+        ? toCustodialClient(found)
+        : null;
   }
 
   @Override
   public NonCustodialClient getClientConfigByDomain(String domain) {
     JdbcClientConfig found = repo.findByDomain(domain);
-    return found == null ? null : toNonCustodialClient(found);
+    return found != null && found.getType() == ClientType.NONCUSTODIAL
+        ? toNonCustodialClient(found)
+        : null;
   }
 
   @Override

platform/src/test/kotlin/org/stellar/anchor/platform/data/JdbcClientServiceConsumersTest.kt

@@ -0,0 +1,116 @@
+package org.stellar.anchor.platform.data
+
+import io.mockk.MockKAnnotations
+import io.mockk.every
+import io.mockk.impl.annotations.MockK
+import org.junit.jupiter.api.Assertions.assertEquals
+import org.junit.jupiter.api.Assertions.assertNull
+import org.junit.jupiter.api.BeforeEach
+import org.junit.jupiter.api.Test
+import org.junit.jupiter.api.assertThrows
+import org.stellar.anchor.api.exception.SepNotAuthorizedException
+import org.stellar.anchor.api.exception.SepValidationException
+import org.stellar.anchor.asset.AssetService
+import org.stellar.anchor.client.ClientConfig.ClientType
+import org.stellar.anchor.client.ClientFinder
+import org.stellar.anchor.config.Sep10Config
+import org.stellar.anchor.util.SepRequestValidator
+import org.stellar.anchor.util.SepRequestValidator.ERR_TOKEN_ACCOUNT_MISMATCH
+import org.stellar.sdk.KeyPair
+
+/**
+ * The reporter's row of HackerOne #4108778: a noncustodial client that also carries a signing key,
+ * a destination list and allow_any_destination, resolved through the real finder and gate.
+ */
+class JdbcClientServiceConsumersTest {
+
+  @MockK private lateinit var repo: JdbcClientConfigRepo
+  @MockK(relaxed = true) private lateinit var sep10Config: Sep10Config
+  @MockK private lateinit var assetService: AssetService
+
+  private lateinit var service: JdbcClientService
+  private lateinit var clientFinder: ClientFinder
+  private lateinit var validator: SepRequestValidator
+
+  private val walletKey = KeyPair.random().accountId
+  private val destination = KeyPair.random().accountId
+
+  @BeforeEach
+  fun setUp() {
+    MockKAnnotations.init(this)
+    service = JdbcClientService(repo)
+    clientFinder = ClientFinder(sep10Config, service)
+    validator = SepRequestValidator(assetService, service)
+    every { sep10Config.allowedClientNames } returns listOf("VIBRANT", "MGI")
+    every { repo.findByDomain(any()) } returns null
+  }
+
+  private fun noncustodialRowWithKey() =
+    JdbcClientConfig().apply {
+      name = "VIBRANT"
+      type = ClientType.NONCUSTODIAL
+      domains = mutableSetOf("vibrantapp.com")
+      signingKeys = mutableSetOf(walletKey)
+      destinationAccounts = mutableSetOf(destination)
+      setAllowAnyDestination(true)
+    }
+
+  private fun custodialRow() =
+    JdbcClientConfig().apply {
+      name = "MGI"
+      type = ClientType.CUSTODIAL
+      signingKeys = mutableSetOf(walletKey)
+      setAllowAnyDestination(true)
+    }
+
+  @Test
+  fun `client finder returns no client name for a noncustodial key when attribution is optional`() {
+    every { sep10Config.isClientAttributionRequired } returns false
+    every { repo.findBySigningKey(walletKey) } returns noncustodialRowWithKey()
+
+    assertNull(clientFinder.getClientName(null, walletKey))
+  }
+
+  @Test
+  fun `client finder refuses a noncustodial key when attribution is required`() {
+    every { sep10Config.isClientAttributionRequired } returns true
+    every { repo.findBySigningKey(walletKey) } returns noncustodialRowWithKey()
+
+    val ex = assertThrows<SepNotAuthorizedException> { clientFinder.getClientName(null, walletKey) }
+
+    assertEquals("Client not found", ex.message)
+  }
+
+  @Test
+  fun `destination gate stays closed for a noncustodial row with allowAnyDestination`() {
+    every { repo.findBySigningKey(walletKey) } returns
+      noncustodialRowWithKey().apply { destinationAccounts = mutableSetOf() }
+
+    val ex =
+      assertThrows<SepValidationException> {
+        validator.validateDestinationAccount(walletKey, destination)
+      }
+
+    assertEquals(ERR_TOKEN_ACCOUNT_MISMATCH, ex.message)
+  }
+
+  @Test
+  fun `destination gate stays closed for a destination listed on a noncustodial row`() {
+    every { repo.findBySigningKey(walletKey) } returns
+      noncustodialRowWithKey().apply { setAllowAnyDestination(false) }
+
+    val ex =
+      assertThrows<SepValidationException> {
+        validator.validateDestinationAccount(walletKey, destination)
+      }
+
+    assertEquals(ERR_TOKEN_ACCOUNT_MISMATCH, ex.message)
+  }
+
+  @Test
+  fun `destination gate still opens for a custodial row with allowAnyDestination`() {
+    every { repo.findBySigningKey(walletKey) } returns custodialRow()
+
+    validator.validateDestinationAccount(walletKey, destination)
+  }
+}

platform/src/test/kotlin/org/stellar/anchor/platform/data/JdbcClientServiceTest.kt

@@ -128,4 +128,125 @@ class JdbcClientServiceTest {
 
     assertEquals(setOf("MGI", "VIBRANT"), names)
   }
+
+  @Test
+  fun `getClientConfigBySigningKey returns null for a noncustodial row that carries the key`() {
+    every { repo.findBySigningKey("GWALLET") } returns
+      nonCustodialRow("VIBRANT", setOf("vibrantapp.com")).apply {
+        signingKeys = mutableSetOf("GWALLET")
+        setAllowAnyDestination(true)
+        destinationAccounts = mutableSetOf("GDEST")
+      }
+
+    assertNull(service.getClientConfigBySigningKey("GWALLET"))
+  }
+
+  @Test
+  fun `getClientConfigBySigningKey returns null when no row holds the key`() {
+    every { repo.findBySigningKey("GUNKNOWN") } returns null
+
+    assertNull(service.getClientConfigBySigningKey("GUNKNOWN"))
+  }
+
+  @Test
+  fun `getClientConfigBySigningKey maps every field of a custodial row`() {
+    every { repo.findBySigningKey("GALICE") } returns
+      custodialRow("MGI", setOf("GALICE", "GBOB")).apply {
+        destinationAccounts = mutableSetOf("GDEST")
+        callbackUrlSep6 = "https://example.com/sep6"
+        callbackUrlSep24 = "https://example.com/sep24"
+        callbackUrlSep12 = "https://example.com/sep12"
+      }
+
+    val result = service.getClientConfigBySigningKey("GALICE")!!
+
+    assertEquals("MGI", result.name)
+    assertEquals(setOf("GALICE", "GBOB"), result.signingKeys)
+    assertEquals(setOf("GDEST"), result.destinationAccounts)
+    assertTrue(result.isAllowAnyDestination)
+    assertEquals("https://example.com/sep6", result.callbackUrls.sep6)
+    assertEquals("https://example.com/sep24", result.callbackUrls.sep24)
+    assertEquals("https://example.com/sep31", result.callbackUrls.sep31)
+    assertEquals("https://example.com/sep12", result.callbackUrls.sep12)
+  }
+
+  @Test
+  fun `getClientConfigBySigningKey answers only for keys of getCustodialClients`() {
+    val custodial = custodialRow("MGI", setOf("GALICE", "GBOB"))
+    val noncustodialWithKey =
+      nonCustodialRow("VIBRANT", setOf("vibrantapp.com")).apply {
+        signingKeys = mutableSetOf("GWALLET")
+      }
+    every { repo.findByType(ClientType.CUSTODIAL) } returns listOf(custodial)
+    every { repo.findBySigningKey("GALICE") } returns custodial
+    every { repo.findBySigningKey("GBOB") } returns custodial
+    every { repo.findBySigningKey("GWALLET") } returns noncustodialWithKey
+    every { repo.findBySigningKey("GUNKNOWN") } returns null
+
+    val custodialKeys = service.custodialClients.flatMap { it.signingKeys }.toSet()
+    val resolvedKeys =
+      listOf("GALICE", "GBOB", "GWALLET", "GUNKNOWN")
+        .filter { service.getClientConfigBySigningKey(it) != null }
+        .toSet()
+
+    assertEquals(setOf("GALICE", "GBOB"), custodialKeys)
+    assertEquals(custodialKeys, resolvedKeys)
+  }
+
+  @Test
+  fun `getClientConfigByDomain returns null for a custodial row that carries the domain`() {
+    every { repo.findByDomain("custodial.example") } returns
+      custodialRow("MGI", setOf("GALICE")).apply { domains = mutableSetOf("custodial.example") }
+
+    assertNull(service.getClientConfigByDomain("custodial.example"))
+  }
+
+  @Test
+  fun `a custodial row carrying a domain resolves by its signing key only`() {
+    val row = custodialRow("MGI", setOf("GALICE")).apply { domains = mutableSetOf("mgi.example") }
+    every { repo.findBySigningKey("GALICE") } returns row
+    every { repo.findByDomain("mgi.example") } returns row
+
+    assertEquals("MGI", service.getClientConfigBySigningKey("GALICE")!!.name)
+    assertNull(service.getClientConfigByDomain("mgi.example"))
+  }
+
+  @Test
+  fun `a noncustodial row carrying a signing key resolves by its domain only`() {
+    val row =
+      nonCustodialRow("VIBRANT", setOf("vibrantapp.com")).apply {
+        signingKeys = mutableSetOf("GWALLET")
+      }
+    every { repo.findBySigningKey("GWALLET") } returns row
+    every { repo.findByDomain("vibrantapp.com") } returns row
+
+    assertEquals("VIBRANT", service.getClientConfigByDomain("vibrantapp.com")!!.name)
+    assertNull(service.getClientConfigBySigningKey("GWALLET"))
+  }
+
+  @Test
+  fun `getClientConfigByDomainAndAccount returns null for a noncustodial key and an unknown domain`() {
+    every { repo.findByDomain("unknown.com") } returns null
+    every { repo.findBySigningKey("GWALLET") } returns
+      nonCustodialRow("VIBRANT", setOf("vibrantapp.com")).apply {
+        signingKeys = mutableSetOf("GWALLET")
+      }
+
+    assertNull(service.getClientConfigByDomainAndAccount("unknown.com", "GWALLET"))
+  }
+
+  @Test
+  fun `a client reclassified between types resolves by the new type on the next lookup`() {
+    val row = custodialRow("MGI", setOf("GALICE")).apply { domains = mutableSetOf("mgi.example") }
+    every { repo.findBySigningKey("GALICE") } returns row
+    every { repo.findByDomain("mgi.example") } returns row
+
+    assertEquals("MGI", service.getClientConfigBySigningKey("GALICE")!!.name)
+    assertNull(service.getClientConfigByDomain("mgi.example"))
+
+    row.type = ClientType.NONCUSTODIAL
+
+    assertNull(service.getClientConfigBySigningKey("GALICE"))
+    assertEquals("MGI", service.getClientConfigByDomain("mgi.example")!!.name)
+  }
 }