diff --git a/extended-tests/src/test/kotlin/org/stellar/anchor/platform/extendedtest/clients/db/ClientsDbAttributionTests.kt b/extended-tests/src/test/kotlin/org/stellar/anchor/platform/extendedtest/clients/db/ClientsDbAttributionTests.kt index fe3f80c3ad..8717171970 100644 --- a/extended-tests/src/test/kotlin/org/stellar/anchor/platform/extendedtest/clients/db/ClientsDbAttributionTests.kt +++ b/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() diff --git a/platform/src/main/java/org/stellar/anchor/platform/data/JdbcClientService.java b/platform/src/main/java/org/stellar/anchor/platform/data/JdbcClientService.java index 59fda106db..f9b84b4f5c 100644 --- a/platform/src/main/java/org/stellar/anchor/platform/data/JdbcClientService.java +++ b/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 diff --git a/platform/src/test/kotlin/org/stellar/anchor/platform/data/JdbcClientServiceConsumersTest.kt b/platform/src/test/kotlin/org/stellar/anchor/platform/data/JdbcClientServiceConsumersTest.kt new file mode 100644 index 0000000000..33f89f2436 --- /dev/null +++ b/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 { 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 { + 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 { + 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) + } +} diff --git a/platform/src/test/kotlin/org/stellar/anchor/platform/data/JdbcClientServiceTest.kt b/platform/src/test/kotlin/org/stellar/anchor/platform/data/JdbcClientServiceTest.kt index c5ee9d1732..48ab0eb3cc 100644 --- a/platform/src/test/kotlin/org/stellar/anchor/platform/data/JdbcClientServiceTest.kt +++ b/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) + } }