Check login in CoreBank Withdrawal APIdev/antoine/secure_withdrawal

5 files changed, 45 insertions, 10 deletions

diff --git a/bank/src/main/kotlin/tech/libeufin/bank/Constants.kt b/bank/src/main/kotlin/tech/libeufin/bank/Constants.kt
index 58969209..f6500270 100644
--- a/bank/src/main/kotlin/tech/libeufin/bank/Constants.kt
+++ b/bank/src/main/kotlin/tech/libeufin/bank/Constants.kt
@@ -39,7 +39,7 @@ val RESERVED_ACCOUNTS = setOf("admin", "bank")
 const val MAX_BODY_LENGTH: Long = 4 * 1024 // 4kB
 
 // API version
-const val COREBANK_API_VERSION: String = "2:1:2"
+const val COREBANK_API_VERSION: String = "2:2:1"
 const val CONVERSION_API_VERSION: String = "0:0:0"
 const val INTEGRATION_API_VERSION: String = "1:0:1"
 const val WIRE_GATEWAY_API_VERSION: String = "0:1:0"
\ No newline at end of file
diff --git a/bank/src/main/kotlin/tech/libeufin/bank/CoreBankApi.kt b/bank/src/main/kotlin/tech/libeufin/bank/CoreBankApi.kt
index c294416b..e578785c 100644
--- a/bank/src/main/kotlin/tech/libeufin/bank/CoreBankApi.kt
+++ b/bank/src/main/kotlin/tech/libeufin/bank/CoreBankApi.kt
@@ -425,7 +425,7 @@ private fun Routing.coreBankWithdrawalApi(db: Database, ctx: BankConfig) {
         }
         post("/accounts/{USERNAME}/withdrawals/{withdrawal_id}/abort") {
             val opId = call.uuidUriComponent("withdrawal_id")
-            when (db.withdrawal.abort(opId)) {
+            when (db.withdrawal.abort(opId, username)) {
                 AbortResult.UnknownOperation -> throw notFound(
                     "Withdrawal operation $opId not found",
                     TalerErrorCode.BANK_TRANSACTION_NOT_FOUND
@@ -439,7 +439,7 @@ private fun Routing.coreBankWithdrawalApi(db: Database, ctx: BankConfig) {
         }
         post("/accounts/{USERNAME}/withdrawals/{withdrawal_id}/confirm") {
             val opId = call.uuidUriComponent("withdrawal_id")
-            when (db.withdrawal.confirm(opId, Instant.now())) {
+            when (db.withdrawal.confirm(opId, username, Instant.now())) {
                 WithdrawalConfirmationResult.UnknownOperation -> throw notFound(
                     "Withdrawal operation $opId not found",
                     TalerErrorCode.BANK_TRANSACTION_NOT_FOUND
diff --git a/bank/src/main/kotlin/tech/libeufin/bank/db/WithdrawalDAO.kt b/bank/src/main/kotlin/tech/libeufin/bank/db/WithdrawalDAO.kt
index 5bf1c4ec..ed2ca893 100644
--- a/bank/src/main/kotlin/tech/libeufin/bank/db/WithdrawalDAO.kt
+++ b/bank/src/main/kotlin/tech/libeufin/bank/db/WithdrawalDAO.kt
@@ -67,15 +67,16 @@ class WithdrawalDAO(private val db: Database) {
     }
 
     /** Abort withdrawal operation [uuid] */
-    suspend fun abort(uuid: UUID): AbortResult = db.serializable { conn ->
+    suspend fun abort(uuid: UUID, login: String,): AbortResult = db.serializable { conn ->
         // TODO login check
         val stmt = conn.prepareStatement("""
             SELECT
                 out_no_op,
                 out_already_confirmed
-            FROM abort_taler_withdrawal(?)
+            FROM abort_taler_withdrawal(?, ?)
         """)
         stmt.setObject(1, uuid)
+        stmt.setString(2, login)
         stmt.executeQuery().use {
             when {
                 !it.next() ->
@@ -145,6 +146,7 @@ class WithdrawalDAO(private val db: Database) {
     /** Confirm withdrawal operation [uuid] */
     suspend fun confirm(
         uuid: UUID,
+        login: String,
         now: Instant
     ): WithdrawalConfirmationResult = db.serializable { conn ->
          // TODO login check
@@ -155,11 +157,12 @@ class WithdrawalDAO(private val db: Database) {
               out_balance_insufficient,
               out_not_selected,
               out_aborted
-            FROM confirm_taler_withdrawal(?, ?);
+            FROM confirm_taler_withdrawal(?, ?, ?);
         """
         )
         stmt.setObject(1, uuid)
-        stmt.setLong(2, now.toDbMicros() ?: throw faultyTimestampByBank())
+        stmt.setString(2, login)
+        stmt.setLong(3, now.toDbMicros() ?: throw faultyTimestampByBank())
         stmt.executeQuery().use {
             when {
                 !it.next() ->
diff --git a/bank/src/test/kotlin/CoreBankApiTest.kt b/bank/src/test/kotlin/CoreBankApiTest.kt
index eb18eab9..469f898e 100644
--- a/bank/src/test/kotlin/CoreBankApiTest.kt
+++ b/bank/src/test/kotlin/CoreBankApiTest.kt
@@ -884,6 +884,18 @@ class CoreBankWithdrawalApiTest {
         // Check unknown
         client.postA("/accounts/merchant/withdrawals/${UUID.randomUUID()}/abort")
             .assertNotFound(TalerErrorCode.BANK_TRANSACTION_NOT_FOUND)
+
+        // Check abort another user's operation
+        client.postA("/accounts/customer/withdrawals") {
+            json { "amount" to "KUDOS:5" } 
+        }.assertOkJson<BankAccountCreateWithdrawalResponse> {
+            val uuid = it.taler_withdraw_uri.split("/").last()
+            withdrawalSelect(uuid)
+
+            // Check error
+            client.postA("/accounts/merchant/withdrawals/$uuid/abort")
+                .assertNotFound(TalerErrorCode.BANK_TRANSACTION_NOT_FOUND)
+        }
     }
 
     // POST /accounts/USERNAME/withdrawals/withdrawal_id/confirm
@@ -949,6 +961,18 @@ class CoreBankWithdrawalApiTest {
         // Check unknown
         client.postA("/accounts/merchant/withdrawals/${UUID.randomUUID()}/confirm")
             .assertNotFound(TalerErrorCode.BANK_TRANSACTION_NOT_FOUND)
+
+        // Check confirm another user's operation
+        client.postA("/accounts/customer/withdrawals") {
+            json { "amount" to "KUDOS:5" } 
+        }.assertOkJson<BankAccountCreateWithdrawalResponse> {
+            val uuid = it.taler_withdraw_uri.split("/").last()
+            withdrawalSelect(uuid)
+
+            // Check error
+            client.postA("/accounts/merchant/withdrawals/$uuid/confirm")
+                .assertNotFound(TalerErrorCode.BANK_TRANSACTION_NOT_FOUND)
+        }
     }
 }
 
diff --git a/database-versioning/libeufin-bank-procedures.sql b/database-versioning/libeufin-bank-procedures.sql
index ea493857..f10f5579 100644
--- a/database-versioning/libeufin-bank-procedures.sql
+++ b/database-versioning/libeufin-bank-procedures.sql
@@ -614,14 +614,19 @@ COMMENT ON FUNCTION select_taler_withdrawal IS 'Set details of a withdrawal oper
 
 CREATE FUNCTION abort_taler_withdrawal(
   IN in_withdrawal_uuid uuid,
+  IN in_login TEXT,
   OUT out_no_op BOOLEAN,
   OUT out_already_confirmed BOOLEAN
 )
 LANGUAGE plpgsql AS $$
 BEGIN
-UPDATE taler_withdrawal_operations
+UPDATE taler_withdrawal_operations 
   SET aborted = NOT confirmation_done
-  WHERE withdrawal_uuid=in_withdrawal_uuid
+  FROM bank_accounts
+    JOIN customers ON customer_id=owning_customer_id
+  WHERE withdrawal_uuid=in_withdrawal_uuid 
+    AND bank_account_id=wallet_bank_account -- JOIN
+    AND login=in_login
   RETURNING confirmation_done
   INTO out_already_confirmed;
 IF NOT FOUND THEN
@@ -638,6 +643,7 @@ COMMENT ON FUNCTION abort_taler_withdrawal IS 'Abort a withdrawal operation.';
 
 CREATE FUNCTION confirm_taler_withdrawal(
   IN in_withdrawal_uuid uuid,
+  IN in_login TEXT,
   IN in_confirmation_date BIGINT,
   OUT out_no_op BOOLEAN,
   OUT out_balance_insufficient BOOLEAN,
@@ -672,7 +678,9 @@ SELECT -- Really no-star policy and instead DECLARE almost one var per column?
     wallet_bank_account_local,
     amount_local.val, amount_local.frac
   FROM taler_withdrawal_operations
-  WHERE withdrawal_uuid=in_withdrawal_uuid;
+    JOIN bank_accounts ON bank_account_id=wallet_bank_account
+    JOIN customers ON customer_id=owning_customer_id
+  WHERE withdrawal_uuid=in_withdrawal_uuid AND login=in_login;
 IF NOT FOUND THEN
   out_no_op=TRUE;
   RETURN;