diff options
author | MS <ms@taler.net> | 2023-10-04 15:28:08 +0200 |
---|---|---|
committer | MS <ms@taler.net> | 2023-10-04 15:28:08 +0200 |
commit | d84b94617f562056d7a87d38f5302c1701e7d3e2 (patch) | |
tree | e44943868bdb96085dd013523b5daed699c1173b | |
parent | f022b73168432b7cb425a5a69f41b616ce34829f (diff) | |
download | libeufin-d84b94617f562056d7a87d38f5302c1701e7d3e2.tar.gz libeufin-d84b94617f562056d7a87d38f5302c1701e7d3e2.tar.bz2 libeufin-d84b94617f562056d7a87d38f5302c1701e7d3e2.zip |
PATCHing accounts.
HTTP level tests and handling of null inputs.
-rw-r--r-- | bank/src/main/kotlin/tech/libeufin/bank/BankMessages.kt | 2 | ||||
-rw-r--r-- | bank/src/main/kotlin/tech/libeufin/bank/CorebankApiHandlers.kt | 25 | ||||
-rw-r--r-- | bank/src/main/kotlin/tech/libeufin/bank/Database.kt | 27 | ||||
-rw-r--r-- | bank/src/test/kotlin/DatabaseTest.kt | 40 | ||||
-rw-r--r-- | bank/src/test/kotlin/LibeuFinApiTest.kt | 63 | ||||
-rw-r--r-- | database-versioning/procedures.sql | 19 |
6 files changed, 149 insertions, 27 deletions
diff --git a/bank/src/main/kotlin/tech/libeufin/bank/BankMessages.kt b/bank/src/main/kotlin/tech/libeufin/bank/BankMessages.kt index 34b03472..ee5cf866 100644 --- a/bank/src/main/kotlin/tech/libeufin/bank/BankMessages.kt +++ b/bank/src/main/kotlin/tech/libeufin/bank/BankMessages.kt @@ -741,7 +741,7 @@ data class AccountReconfiguration( val challenge_contact_data: ChallengeContactData?, val cashout_address: String?, val name: String?, - val is_exchange: Boolean + val is_exchange: Boolean? ) /** diff --git a/bank/src/main/kotlin/tech/libeufin/bank/CorebankApiHandlers.kt b/bank/src/main/kotlin/tech/libeufin/bank/CorebankApiHandlers.kt index 8b269b15..5cfbfb49 100644 --- a/bank/src/main/kotlin/tech/libeufin/bank/CorebankApiHandlers.kt +++ b/bank/src/main/kotlin/tech/libeufin/bank/CorebankApiHandlers.kt @@ -351,7 +351,7 @@ fun Routing.accountsMgmtHandlers(db: Database, ctx: BankApplicationContext) { talerEc = TalerErrorCode.TALER_EC_END // FIXME, define EC. ) // Check if a non-admin user tried to change their legal name - if (c.login != "admin" && req.name != accountCustomer.name) + if ((c.login != "admin") && (req.name != null) && (req.name != accountCustomer.name)) throw forbidden("non-admin user cannot change their legal name") // Preventing identical data to be overridden. val bankAccount = db.bankAccountGetFromOwnerId(accountCustomer.expectRowId()) @@ -366,8 +366,27 @@ fun Routing.accountsMgmtHandlers(db: Database, ctx: BankApplicationContext) { call.respond(HttpStatusCode.NoContent) return@patch } - // Not identical, go on writing the DB. - throw NotImplementedError("DB part missing.") + val dbRes = db.accountReconfig( + login = accountCustomer.login, + name = req.name, + cashoutPayto = req.cashout_address, + emailAddress = req.challenge_contact_data?.email, + isTalerExchange = req.is_exchange, + phoneNumber = req.challenge_contact_data?.phone + ) + when (dbRes) { + AccountReconfigDBResult.SUCCESS -> call.respond(HttpStatusCode.NoContent) + AccountReconfigDBResult.CUSTOMER_NOT_FOUND -> { + // Rare case. Only possible if a deletion happened before the flow reaches here. + logger.warn("Authenticated customer wasn't found any more in the database") + throw notFound("Customer not found", TalerErrorCode.TALER_EC_END) // FIXME: needs EC + } + AccountReconfigDBResult.BANK_ACCOUNT_NOT_FOUND -> { + // Bank's fault: no customer should lack a bank account. + throw internalServerError("Customer '${accountCustomer.login}' lacks bank account") + } + } + return@patch } // WITHDRAWAL ENDPOINTS post("/accounts/{USERNAME}/withdrawals") { diff --git a/bank/src/main/kotlin/tech/libeufin/bank/Database.kt b/bank/src/main/kotlin/tech/libeufin/bank/Database.kt index dc7eb04b..066d0cc2 100644 --- a/bank/src/main/kotlin/tech/libeufin/bank/Database.kt +++ b/bank/src/main/kotlin/tech/libeufin/bank/Database.kt @@ -28,10 +28,7 @@ import tech.libeufin.util.microsToJavaInstant import tech.libeufin.util.stripIbanPayto import tech.libeufin.util.toDbMicros import java.io.File -import java.sql.DriverManager -import java.sql.PreparedStatement -import java.sql.ResultSet -import java.sql.SQLException +import java.sql.* import java.time.Instant import java.util.* import kotlin.math.abs @@ -404,16 +401,22 @@ class Database(private val dbConfig: String, private val bankCurrency: String) { * The 'login' parameter decides which customer and bank account rows * will get the update. * + * Meaning of null in the parameters: when 'name' and 'isTalerExchange' + * are null, NOTHING gets changed. If any of the other values are null, + * WARNING: their value will be overridden with null. No parameter gets + * null as the default, as to always keep the caller aware of what gets in + * the database. + * * The return type expresses either success, or that the target rows * could not be found. */ fun accountReconfig( login: String, - name: String, - cashoutPayto: String, - phoneNumber: String, - emailAddress: String, - isTalerExchange: Boolean + name: String?, + cashoutPayto: String?, + phoneNumber: String?, + emailAddress: String?, + isTalerExchange: Boolean? ): AccountReconfigDBResult { reconnect() val stmt = prepare(""" @@ -427,7 +430,11 @@ class Database(private val dbConfig: String, private val bankCurrency: String) { stmt.setString(3, phoneNumber) stmt.setString(4, emailAddress) stmt.setString(5, cashoutPayto) - stmt.setBoolean(6, isTalerExchange) + + if (isTalerExchange == null) + stmt.setNull(6, Types.NULL) + else stmt.setBoolean(6, isTalerExchange) + val res = stmt.executeQuery() res.use { if (!it.next()) throw internalServerError("accountReconfig() returned nothing") diff --git a/bank/src/test/kotlin/DatabaseTest.kt b/bank/src/test/kotlin/DatabaseTest.kt index 23423eb5..93196193 100644 --- a/bank/src/test/kotlin/DatabaseTest.kt +++ b/bank/src/test/kotlin/DatabaseTest.kt @@ -28,6 +28,7 @@ import java.util.UUID import kotlin.experimental.inv import kotlin.test.assertEquals import kotlin.test.assertNotEquals +import kotlin.test.assertNotNull import kotlin.test.assertTrue // Foo pays Bar with custom subject. @@ -486,9 +487,9 @@ class DatabaseTest { "+99", "foo@example.com", true - ).apply { assertEquals(this, AccountReconfigDBResult.CUSTOMER_NOT_FOUND) } + ).apply { assertEquals(AccountReconfigDBResult.CUSTOMER_NOT_FOUND, this) } // creating the customer - assertNotEquals(db.customerCreate(customerFoo), null) + assertNotNull(db.customerCreate(customerFoo)) // asserting for the bank account not being found. db.accountReconfig( @@ -498,7 +499,7 @@ class DatabaseTest { "+99", "foo@example.com", true - ).apply { assertEquals(this, AccountReconfigDBResult.BANK_ACCOUNT_NOT_FOUND) } + ).apply { assertEquals(AccountReconfigDBResult.BANK_ACCOUNT_NOT_FOUND, this) } // Giving foo a bank account assert(db.bankAccountCreate(bankAccountFoo) != null) // asserting for success. @@ -512,16 +513,40 @@ class DatabaseTest { ).apply { assertEquals(this, AccountReconfigDBResult.SUCCESS) } // Getting the updated account from the database and checking values. db.customerGetFromLogin("foo").apply { - assertNotEquals(this, null) - assert((this!!.login == "foo") && + assertNotNull(this) + assert((this.login == "foo") && (this.name == "Bar") && (this.cashoutPayto) == "payto://cashout" && (this.email) == "foo@example.com" && this.phone == "+99" ) db.bankAccountGetFromOwnerId(this.expectRowId()).apply { - assertNotEquals(this, null) - assertTrue(this!!.isTalerExchange) + assertNotNull(this) + assertTrue(this.isTalerExchange) + } + } + // Testing the null cases. + // Sets everything to null, leaving name and the Taler exchange flag untouched. + assertEquals(db.accountReconfig( + "foo", + null, + null, + null, + null, + null), + AccountReconfigDBResult.SUCCESS + ) + db.customerGetFromLogin("foo").apply { + assertNotNull(this) + assert((this.login == "foo") && + (this.name == "Bar") && + (this.cashoutPayto) == null && + (this.email) == null && + this.phone == null + ) + db.bankAccountGetFromOwnerId(this.expectRowId()).apply { + assertNotNull(this) + assertTrue(this.isTalerExchange) } } } @@ -529,4 +554,3 @@ class DatabaseTest { - diff --git a/bank/src/test/kotlin/LibeuFinApiTest.kt b/bank/src/test/kotlin/LibeuFinApiTest.kt index 9984a72c..229414d2 100644 --- a/bank/src/test/kotlin/LibeuFinApiTest.kt +++ b/bank/src/test/kotlin/LibeuFinApiTest.kt @@ -17,6 +17,9 @@ import java.time.Duration import java.time.Instant import java.time.temporal.ChronoUnit import kotlin.random.Random +import kotlin.test.assertEquals +import kotlin.test.assertNotEquals +import kotlin.test.assertNotNull class LibeuFinApiTest { private val customerFoo = Customer( @@ -708,6 +711,66 @@ class LibeuFinApiTest { } /** + * Tests reconfiguration of account data. + */ + @Test + fun accountReconfig() { + val db = initDb() + val ctx = getTestContext() + testApplication { + application { + corebankWebApp(db, ctx) + } + assertNotNull(db.customerCreate(customerFoo)) + val validReq = json { + put("is_exchange", true) + } + // First call expects 500, because foo lacks a bank account + client.patch("/accounts/foo") { + basicAuth("foo", "pw") + jsonBody(validReq) + }.assertStatus(HttpStatusCode.InternalServerError) + // Creating foo's bank account. + assertNotNull(db.bankAccountCreate(genBankAccount(1L))) + // Successful attempt now. + client.patch("/accounts/foo") { + basicAuth("foo", "pw") + jsonBody(AccountReconfiguration( + cashout_address = "payto://new-cashout-address", + challenge_contact_data = ChallengeContactData( + email = "new@example.com", + phone = "+987" + ), + is_exchange = true, + name = null + )) + }.assertStatus(HttpStatusCode.NoContent) + // Checking ordinary user doesn't get to patch their name. + client.patch("/accounts/foo") { + basicAuth("foo", "pw") + jsonBody(json { + put("name", "Another Foo") + }) + }.assertStatus(HttpStatusCode.Forbidden) + // Finally checking that admin does get to patch foo's name. + assertNotNull(db.customerCreate(Customer( + login = "admin", + passwordHash = CryptoUtil.hashpw("secret"), + name = "CFO" + ))) + client.patch("/accounts/foo") { + basicAuth("admin", "secret") + jsonBody(json { + put("name", "Another Foo") + }) + }.assertStatus(HttpStatusCode.NoContent) + val fooFromDb = db.customerGetFromLogin("foo") + assertNotNull(fooFromDb) + assertEquals("Another Foo", fooFromDb.name) + } + } + + /** * Tests the GET /accounts endpoint. */ @Test diff --git a/database-versioning/procedures.sql b/database-versioning/procedures.sql index 802ca7f4..cdf875aa 100644 --- a/database-versioning/procedures.sql +++ b/database-versioning/procedures.sql @@ -113,25 +113,34 @@ THEN END IF; out_nx_customer=FALSE; -UPDATE bank_accounts - SET is_taler_exchange = in_is_taler_exchange - WHERE owning_customer_id = my_customer_id; -IF NOT FOUND +-- optionally updating the Taler exchange flag +IF in_is_taler_exchange IS NOT NULL +THEN + UPDATE bank_accounts + SET is_taler_exchange = in_is_taler_exchange + WHERE owning_customer_id = my_customer_id; +END IF; +IF in_is_taler_exchange IS NOT NULL AND NOT FOUND THEN out_nx_bank_account=TRUE; RETURN; END IF; out_nx_bank_account=FALSE; + -- bank account patching worked, custom must as well -- since this runs in a DB transaction and the customer -- was found earlier in this function. UPDATE customers SET - name=in_name, cashout_payto=in_cashout_payto, phone=in_phone, email=in_email WHERE customer_id = my_customer_id; +-- optionally updating the name +IF in_name IS NOT NULL +THEN + UPDATE customers SET name=in_name WHERE customer_id = my_customer_id; +END IF; END $$; COMMENT ON FUNCTION account_reconfig(TEXT, TEXT, TEXT, TEXT, TEXT, BOOLEAN) |