commit f8ff5c0a81bd74e3ca094f5b02bcd4a26b3a11ea
parent 15a26b4d9ef9a3ecf24d276b85f3f20197d142a9
Author: Antoine A <>
Date: Wed, 15 Nov 2023 13:05:54 +0000
Fix cashout API not checking ownership
Diffstat:
5 files changed, 81 insertions(+), 33 deletions(-)
diff --git a/bank/src/main/kotlin/tech/libeufin/bank/CoreBankApi.kt b/bank/src/main/kotlin/tech/libeufin/bank/CoreBankApi.kt
@@ -320,13 +320,10 @@ private fun Routing.coreBankTransactionsApi(db: Database, ctx: BankConfig) {
}
get("/accounts/{USERNAME}/transactions/{T_ID}") {
val tId = call.longUriComponent("T_ID")
- val bankAccount = call.bankAccount(db)
- val (tx, accountId) = db.bankTransactionGetFromInternalId(tId) ?: throw notFound(
+ val tx = db.bankTransactionGetFromInternalId(tId, username) ?: throw notFound(
"Bank transaction '$tId' not found",
TalerErrorCode.BANK_TRANSACTION_NOT_FOUND
)
- if (accountId != bankAccount.bankAccountId) // TODO not found ?
- throw unauthorized("Client has no rights over the bank transaction: $tId")
call.respond(tx)
}
}
@@ -535,7 +532,7 @@ private fun Routing.coreBankCashoutApi(db: Database, ctx: BankConfig) = conditio
}
post("/accounts/{USERNAME}/cashouts/{CASHOUT_ID}/abort") {
val id = call.longUriComponent("CASHOUT_ID")
- when (db.cashout.abort(id)) {
+ when (db.cashout.abort(id, username)) {
AbortResult.NOT_FOUND -> throw notFound(
"Cashout operation $id not found",
TalerErrorCode.BANK_TRANSACTION_NOT_FOUND
@@ -552,6 +549,7 @@ private fun Routing.coreBankCashoutApi(db: Database, ctx: BankConfig) = conditio
val id = call.longUriComponent("CASHOUT_ID")
when (db.cashout.confirm(
id = id,
+ login = username,
tanCode = req.tan,
timestamp = Instant.now()
)) {
@@ -591,7 +589,7 @@ private fun Routing.coreBankCashoutApi(db: Database, ctx: BankConfig) = conditio
auth(db, TokenScope.readonly) {
get("/accounts/{USERNAME}/cashouts/{CASHOUT_ID}") {
val id = call.longUriComponent("CASHOUT_ID")
- val cashout = db.cashout.get(id) ?: throw notFound(
+ val cashout = db.cashout.get(id, username) ?: throw notFound(
"Cashout operation $id not found",
TalerErrorCode.BANK_TRANSACTION_NOT_FOUND
)
diff --git a/bank/src/main/kotlin/tech/libeufin/bank/db/CashoutDAO.kt b/bank/src/main/kotlin/tech/libeufin/bank/db/CashoutDAO.kt
@@ -133,14 +133,16 @@ class CashoutDAO(private val db: Database) {
stmt.executeQueryCheck()
}
- suspend fun abort(id: Long): AbortResult = db.serializable { conn ->
+ suspend fun abort(id: Long, login: String): AbortResult = db.serializable { conn ->
val stmt = conn.prepareStatement("""
UPDATE cashout_operations
SET aborted = local_transaction IS NULL
- WHERE cashout_id=?
+ FROM bank_accounts JOIN customers ON customer_id=owning_customer_id
+ WHERE cashout_id=? AND bank_account=bank_account_id AND login=?
RETURNING local_transaction IS NOT NULL
""")
stmt.setLong(1, id)
+ stmt.setString(2, login)
when (stmt.oneOrNull { it.getBoolean(1) }) {
null -> AbortResult.NOT_FOUND
true -> AbortResult.CONFIRMED
@@ -150,6 +152,7 @@ class CashoutDAO(private val db: Database) {
suspend fun confirm(
id: Long,
+ login: String,
tanCode: String,
timestamp: Instant
): CashoutConfirmationResult = db.serializable { conn ->
@@ -162,11 +165,12 @@ class CashoutDAO(private val db: Database) {
out_aborted,
out_no_retry,
out_no_cashout_payto
- FROM cashout_confirm(?, ?, ?);
+ FROM cashout_confirm(?, ?, ?, ?);
""")
stmt.setLong(1, id)
- stmt.setString(2, tanCode)
- stmt.setLong(3, timestamp.toDbMicros() ?: throw faultyTimestampByBank())
+ stmt.setString(2, login)
+ stmt.setString(3, tanCode)
+ stmt.setLong(4, timestamp.toDbMicros() ?: throw faultyTimestampByBank())
stmt.executeQuery().use {
when {
!it.next() ->
@@ -203,7 +207,7 @@ class CashoutDAO(private val db: Database) {
}
}
- suspend fun get(id: Long): CashoutStatusResponse? = db.conn { conn ->
+ suspend fun get(id: Long, login: String): CashoutStatusResponse? = db.conn { conn ->
val stmt = conn.prepareStatement("""
SELECT
CASE
@@ -219,10 +223,13 @@ class CashoutDAO(private val db: Database) {
,creation_time
,transaction_date as confirmation_date
FROM cashout_operations
+ JOIN bank_accounts ON bank_account=bank_account_id
+ JOIN customers ON owning_customer_id=customer_id
LEFT JOIN bank_account_transactions ON local_transaction=bank_transaction_id
- WHERE cashout_id=?
+ WHERE cashout_id=? AND login=?
""")
stmt.setLong(1, id)
+ stmt.setString(2, login)
stmt.oneOrNull {
CashoutStatusResponse(
status = CashoutStatus.valueOf(it.getString("status")),
diff --git a/bank/src/main/kotlin/tech/libeufin/bank/db/Database.kt b/bank/src/main/kotlin/tech/libeufin/bank/db/Database.kt
@@ -711,7 +711,7 @@ class Database(dbConfig: String, internal val bankCurrency: String, internal val
}
// Get the bank transaction whose row ID is rowId
- suspend fun bankTransactionGetFromInternalId(rowId: Long): Pair<BankAccountTransactionInfo, Long>? = conn { conn ->
+ suspend fun bankTransactionGetFromInternalId(rowId: Long, login: String): BankAccountTransactionInfo? = conn { conn ->
val stmt = conn.prepareStatement("""
SELECT
creditor_payto_uri
@@ -721,28 +721,27 @@ class Database(dbConfig: String, internal val bankCurrency: String, internal val
,(amount).frac AS amount_frac
,transaction_date
,direction
- ,bank_account_id
,bank_transaction_id
FROM bank_account_transactions
- WHERE bank_transaction_id=?
+ JOIN bank_accounts ON bank_account_transactions.bank_account_id=bank_accounts.bank_account_id
+ JOIN customers ON customer_id=owning_customer_id
+ WHERE bank_transaction_id=? AND login=?
""")
stmt.setLong(1, rowId)
+ stmt.setString(2, login)
stmt.oneOrNull {
- Pair(
- BankAccountTransactionInfo(
- creditor_payto_uri = it.getString("creditor_payto_uri"),
- debtor_payto_uri = it.getString("debtor_payto_uri"),
- amount = TalerAmount(
- it.getLong("amount_val"),
- it.getInt("amount_frac"),
- bankCurrency
- ),
- direction = TransactionDirection.valueOf(it.getString("direction")),
- subject = it.getString("subject"),
- date = TalerProtocolTimestamp(it.getLong("transaction_date").microsToJavaInstant() ?: throw faultyTimestampByBank()),
- row_id = it.getLong("bank_transaction_id")
+ BankAccountTransactionInfo(
+ creditor_payto_uri = it.getString("creditor_payto_uri"),
+ debtor_payto_uri = it.getString("debtor_payto_uri"),
+ amount = TalerAmount(
+ it.getLong("amount_val"),
+ it.getInt("amount_frac"),
+ bankCurrency
),
- it.getLong("bank_account_id")
+ direction = TransactionDirection.valueOf(it.getString("direction")),
+ subject = it.getString("subject"),
+ date = TalerProtocolTimestamp(it.getLong("transaction_date").microsToJavaInstant() ?: throw faultyTimestampByBank()),
+ row_id = it.getLong("bank_transaction_id")
)
}
}
diff --git a/bank/src/test/kotlin/CoreBankApiTest.kt b/bank/src/test/kotlin/CoreBankApiTest.kt
@@ -647,10 +647,10 @@ class CoreBankTransactionsApiTest {
client.get("/accounts/merchant/transactions/3") {
basicAuth("merchant", "merchant-password")
}.assertNotFound(TalerErrorCode.BANK_TRANSACTION_NOT_FOUND)
- // Check wrong transaction
+ // Check another user's transaction
client.get("/accounts/merchant/transactions/2") {
basicAuth("merchant", "merchant-password")
- }.assertUnauthorized() // Should be NOT_FOUND ?
+ }.assertNotFound(TalerErrorCode.BANK_TRANSACTION_NOT_FOUND)
}
// POST /transactions
@@ -1173,6 +1173,19 @@ class CoreBankCashoutApiTest {
basicAuth("customer", "customer-password")
jsonBody { "tan" to "code" }
}.assertNotFound(TalerErrorCode.BANK_TRANSACTION_NOT_FOUND)
+
+ // Check abort another user's operation
+ client.post("/accounts/customer/cashouts") {
+ basicAuth("customer", "customer-password")
+ jsonBody(json(req) { "request_uid" to randShortHashCode() })
+ }.assertOk().run {
+ val id = json<CashoutPending>().cashout_id
+
+ // Check error
+ client.post("/accounts/merchant/cashouts/$id/abort") {
+ basicAuth("merchant", "merchant-password")
+ }.assertNotFound(TalerErrorCode.BANK_TRANSACTION_NOT_FOUND)
+ }
}
// POST /accounts/{USERNAME}/cashouts/{CASHOUT_ID}/confirm
@@ -1228,6 +1241,23 @@ class CoreBankCashoutApiTest {
}.assertNoContent()
}
+ // Check confirm another user's operation
+ client.post("/accounts/customer/cashouts") {
+ basicAuth("customer", "customer-password")
+ jsonBody(json(req) {
+ "request_uid" to randShortHashCode()
+ "amount_credit" to convert("KUDOS:1")
+ })
+ }.assertOk().run {
+ val id = json<CashoutPending>().cashout_id
+
+ // Check error
+ client.post("/accounts/merchant/cashouts/$id/confirm") {
+ basicAuth("merchant", "merchant-password")
+ jsonBody { "tan" to "unused" }
+ }.assertNotFound(TalerErrorCode.BANK_TRANSACTION_NOT_FOUND)
+ }
+
// Check bad conversion
client.post("/accounts/customer/cashouts") {
basicAuth("customer", "customer-password")
@@ -1364,6 +1394,19 @@ class CoreBankCashoutApiTest {
client.get("/accounts/customer/cashouts/42") {
basicAuth("customer", "customer-password")
}.assertNotFound(TalerErrorCode.BANK_TRANSACTION_NOT_FOUND)
+
+ // Check get another user's operation
+ client.post("/accounts/customer/cashouts") {
+ basicAuth("customer", "customer-password")
+ jsonBody(json(req) { "request_uid" to randShortHashCode() })
+ }.assertOk().run {
+ val id = json<CashoutPending>().cashout_id
+
+ // Check error
+ client.get("/accounts/merchant/cashouts/$id") {
+ basicAuth("merchant", "merchant-password")
+ }.assertNotFound(TalerErrorCode.BANK_TRANSACTION_NOT_FOUND)
+ }
}
// GET /accounts/{USERNAME}/cashouts
diff --git a/database-versioning/libeufin-bank-procedures.sql b/database-versioning/libeufin-bank-procedures.sql
@@ -1168,6 +1168,7 @@ END $$;
CREATE OR REPLACE FUNCTION cashout_confirm(
IN in_cashout_id BIGINT,
+ IN in_login TEXT,
IN in_tan_code TEXT,
IN in_now_date BIGINT,
OUT out_no_op BOOLEAN,
@@ -1207,7 +1208,7 @@ SELECT
FROM cashout_operations
JOIN bank_accounts ON bank_account_id=bank_account
JOIN customers ON customer_id=owning_customer_id
- WHERE cashout_id=in_cashout_id;
+ WHERE cashout_id=in_cashout_id AND login=in_login;
IF NOT FOUND THEN
out_no_op=TRUE;
RETURN;
