From 14e7d3e842cac7ae27e875f4c5db44a33258bc6f Mon Sep 17 00:00:00 2001 From: Antoine A <> Date: Fri, 1 Dec 2023 14:03:21 +0000 Subject: Fix idempotency for account creation --- .../main/kotlin/tech/libeufin/bank/CoreBankApi.kt | 3 +- bank/src/main/kotlin/tech/libeufin/bank/Main.kt | 3 +- .../kotlin/tech/libeufin/bank/db/AccountDAO.kt | 15 ++++---- bank/src/main/kotlin/tech/libeufin/bank/helpers.kt | 3 +- bank/src/test/kotlin/CoreBankApiTest.kt | 42 ++++++++++++++++------ bank/src/test/kotlin/helpers.kt | 9 +++-- 6 files changed, 53 insertions(+), 22 deletions(-) diff --git a/bank/src/main/kotlin/tech/libeufin/bank/CoreBankApi.kt b/bank/src/main/kotlin/tech/libeufin/bank/CoreBankApi.kt index 2410e95a..2f80e4aa 100644 --- a/bank/src/main/kotlin/tech/libeufin/bank/CoreBankApi.kt +++ b/bank/src/main/kotlin/tech/libeufin/bank/CoreBankApi.kt @@ -159,7 +159,8 @@ private fun Routing.coreBankAccountsApi(db: Database, ctx: BankConfig) { isTalerExchange = req.is_taler_exchange, maxDebt = ctx.defaultCustomerDebtLimit, bonus = if (!req.is_taler_exchange) ctx.registrationBonus - else TalerAmount(0, 0, ctx.regionalCurrency) + else TalerAmount(0, 0, ctx.regionalCurrency), + checkPaytoIdempotent = req.internal_payto_uri != null ) when (result) { diff --git a/bank/src/main/kotlin/tech/libeufin/bank/Main.kt b/bank/src/main/kotlin/tech/libeufin/bank/Main.kt index 71bddf3d..71edb958 100644 --- a/bank/src/main/kotlin/tech/libeufin/bank/Main.kt +++ b/bank/src/main/kotlin/tech/libeufin/bank/Main.kt @@ -356,7 +356,8 @@ class CreateAccount : CliktCommand("Create an account", name = "create-account") isTalerExchange = json.is_taler_exchange, maxDebt = ctx.defaultCustomerDebtLimit, bonus = if (!json.is_taler_exchange) ctx.registrationBonus - else TalerAmount(0, 0, ctx.regionalCurrency) + else TalerAmount(0, 0, ctx.regionalCurrency), + checkPaytoIdempotent = json.internal_payto_uri != null ) when (result) { diff --git a/bank/src/main/kotlin/tech/libeufin/bank/db/AccountDAO.kt b/bank/src/main/kotlin/tech/libeufin/bank/db/AccountDAO.kt index 1a824c7b..2cf85520 100644 --- a/bank/src/main/kotlin/tech/libeufin/bank/db/AccountDAO.kt +++ b/bank/src/main/kotlin/tech/libeufin/bank/db/AccountDAO.kt @@ -45,7 +45,9 @@ class AccountDAO(private val db: Database) { isPublic: Boolean, isTalerExchange: Boolean, maxDebt: TalerAmount, - bonus: TalerAmount + bonus: TalerAmount, + // Whether to check [internalPaytoUri] for idempotency + checkPaytoIdempotent: Boolean ): AccountCreationResult = db.serializable { it -> val now = Instant.now().toDbMicros() ?: throw faultyTimestampByBank(); it.transaction { conn -> @@ -54,7 +56,7 @@ class AccountDAO(private val db: Database) { AND email IS NOT DISTINCT FROM ? AND phone IS NOT DISTINCT FROM ? AND cashout_payto IS NOT DISTINCT FROM ? - AND internal_payto_uri=? + AND (NOT ? OR internal_payto_uri=?) AND is_public=? AND is_taler_exchange=? FROM customers @@ -66,10 +68,11 @@ class AccountDAO(private val db: Database) { setString(2, email) setString(3, phone) setString(4, cashoutPayto?.canonical) - setString(5, internalPaytoUri.canonical) - setBoolean(6, isPublic) - setBoolean(7, isTalerExchange) - setString(8, login) + setBoolean(5, checkPaytoIdempotent) + setString(6, internalPaytoUri.canonical) + setBoolean(7, isPublic) + setBoolean(8, isTalerExchange) + setString(9, login) oneOrNull { CryptoUtil.checkpw(password, it.getString(1)) && it.getBoolean(2) } diff --git a/bank/src/main/kotlin/tech/libeufin/bank/helpers.kt b/bank/src/main/kotlin/tech/libeufin/bank/helpers.kt index 08fbd084..dc65fd44 100644 --- a/bank/src/main/kotlin/tech/libeufin/bank/helpers.kt +++ b/bank/src/main/kotlin/tech/libeufin/bank/helpers.kt @@ -132,7 +132,8 @@ suspend fun maybeCreateAdminAccount(db: Database, ctx: BankConfig, pw: String? = isPublic = false, isTalerExchange = false, maxDebt = ctx.defaultAdminDebtLimit, - bonus = TalerAmount(0, 0, ctx.regionalCurrency) + bonus = TalerAmount(0, 0, ctx.regionalCurrency), + checkPaytoIdempotent = false ) } diff --git a/bank/src/test/kotlin/CoreBankApiTest.kt b/bank/src/test/kotlin/CoreBankApiTest.kt index f17e6414..ad4e5e62 100644 --- a/bank/src/test/kotlin/CoreBankApiTest.kt +++ b/bank/src/test/kotlin/CoreBankApiTest.kt @@ -165,6 +165,35 @@ class CoreBankAccountsApiTest { // Testing the account creation and its idempotency @Test fun create() = bankSetup { _ -> + // Check generated payto + obj { + "username" to "john" + "password" to "password" + "name" to "John" + }.let { req -> + // Check Ok + val payto = client.post("/accounts") { + json(req) + }.assertOkJson().internal_payto_uri + // Check idempotency + client.post("/accounts") { + json(req) + }.assertOk() + // Check idempotency with payto + client.post("/accounts") { + json(req) { + "internal_payto_uri" to payto + } + }.assertOk() + // Check payto conflict + client.post("/accounts") { + json(req) { + "internal_payto_uri" to genIbanPaytoUri() + } + }.assertConflict(TalerErrorCode.BANK_REGISTER_USERNAME_REUSE) + } + + // Check given payto val ibanPayto = genIbanPaytoUri() val req = obj { "username" to "foo" @@ -177,21 +206,14 @@ class CoreBankAccountsApiTest { // Check Ok client.post("/accounts") { json(req) - }.assertOk() + }.assertOkJson { + assertEquals(ibanPayto, it.internal_payto_uri) + } // Testing idempotency client.post("/accounts") { json(req) }.assertOk() - // Test generate payto_uri - client.post("/accounts") { - json { - "username" to "jor" - "password" to "password" - "name" to "Joe" - } - }.assertOk() - // Reserved account RESERVED_ACCOUNTS.forEach { client.post("/accounts") { diff --git a/bank/src/test/kotlin/helpers.kt b/bank/src/test/kotlin/helpers.kt index e56a994f..7d65e2f8 100644 --- a/bank/src/test/kotlin/helpers.kt +++ b/bank/src/test/kotlin/helpers.kt @@ -77,7 +77,8 @@ fun bankSetup( maxDebt = TalerAmount("KUDOS:10"), isTalerExchange = false, isPublic = false, - bonus = bonus + bonus = bonus, + checkPaytoIdempotent = false )) assertEquals(AccountCreationResult.Success, db.account.create( login = "exchange", @@ -87,7 +88,8 @@ fun bankSetup( maxDebt = TalerAmount("KUDOS:10"), isTalerExchange = true, isPublic = false, - bonus = bonus + bonus = bonus, + checkPaytoIdempotent = false )) assertEquals(AccountCreationResult.Success, db.account.create( login = "customer", @@ -97,7 +99,8 @@ fun bankSetup( maxDebt = TalerAmount("KUDOS:10"), isTalerExchange = false, isPublic = false, - bonus = bonus + bonus = bonus, + checkPaytoIdempotent = false )) // Create admin account assertEquals(AccountCreationResult.Success, maybeCreateAdminAccount(db, ctx, "admin-password")) -- cgit v1.2.3