commit cb7094fe27afb1ea1ecd4fcbda3d075c05f7aa16
parent 61c49361c004bfe59a05e00cc51aae43af213193
Author: Antoine A <>
Date: Mon, 13 Nov 2023 18:47:44 +0000
Improve accounts API
Diffstat:
6 files changed, 154 insertions(+), 35 deletions(-)
diff --git a/bank/src/main/kotlin/tech/libeufin/bank/CoreBankApi.kt b/bank/src/main/kotlin/tech/libeufin/bank/CoreBankApi.kt
@@ -221,7 +221,10 @@ private fun Routing.coreBankAccountsMgmtApi(db: Database, ctx: BankConfig) {
req.debit_threshold?.run { ctx.checkRegionalCurrency(this) }
if (req.is_taler_exchange != null && username == "admin")
- throw forbidden("admin account cannot be an exchange")
+ throw conflict(
+ "admin account cannot be an exchange",
+ TalerErrorCode.BANK_PATCH_ADMIN_EXCHANGE
+ )
val res = db.accountReconfig(
login = username,
@@ -239,27 +242,38 @@ private fun Routing.coreBankAccountsMgmtApi(db: Database, ctx: BankConfig) {
"Account '$username' not found",
TalerErrorCode.BANK_UNKNOWN_ACCOUNT
)
- CustomerPatchResult.CONFLICT_LEGAL_NAME ->
- throw forbidden("non-admin user cannot change their legal name")
- CustomerPatchResult.CONFLICT_DEBT_LIMIT ->
- throw forbidden("non-admin user cannot change their debt limit")
+ CustomerPatchResult.CONFLICT_LEGAL_NAME -> throw conflict(
+ "non-admin user cannot change their legal name",
+ TalerErrorCode.BANK_NON_ADMIN_PATCH_LEGAL_NAME
+ )
+ CustomerPatchResult.CONFLICT_DEBT_LIMIT -> throw conflict(
+ "non-admin user cannot change their debt limit",
+ TalerErrorCode.BANK_NON_ADMIN_PATCH_DEBT_LIMIT
+ )
}
}
- }
- auth(db, TokenScope.readwrite) {
patch("/accounts/{USERNAME}/auth") {
val req = call.receive<AccountPasswordChange>()
- val hashedPassword = CryptoUtil.hashpw(req.new_password)
- if (!db.customerChangePassword(username, hashedPassword))
- throw notFound(
+ if (!isAdmin && req.old_password == null) {
+ throw conflict(
+ "non-admin user cannot change password without providing old password",
+ TalerErrorCode.BANK_NON_ADMIN_PATCH_MISSING_OLD_PASSWORD
+ )
+ }
+ when (db.accountReconfigPassword(username, req.new_password, req.old_password)) {
+ CustomerPatchAuthResult.SUCCESS -> call.respond(HttpStatusCode.NoContent)
+ CustomerPatchAuthResult.ACCOUNT_NOT_FOUND -> throw notFound(
"Account '$username' not found",
TalerErrorCode.BANK_UNKNOWN_ACCOUNT
)
- call.respond(HttpStatusCode.NoContent)
+ CustomerPatchAuthResult.CONFLICT_BAD_PASSWORD -> throw conflict(
+ "old password does not match",
+ TalerErrorCode.BANK_PATCH_BAD_OLD_PASSWORD
+ )
+ }
}
}
get("/public-accounts") {
- // no authentication here.
val publicAccounts = db.accountsGetPublic(ctx.currency)
if (publicAccounts.isEmpty()) {
call.respond(HttpStatusCode.NoContent)
diff --git a/bank/src/main/kotlin/tech/libeufin/bank/Main.kt b/bank/src/main/kotlin/tech/libeufin/bank/Main.kt
@@ -331,7 +331,7 @@ class ChangePw : CliktCommand("Change account password", name = "passwd") {
if (!maybeCreateAdminAccount(db, ctx)) // logs provided by the helper
exitProcess(1)
- if (!db.customerChangePassword(account, CryptoUtil.hashpw(password))) {
+ if (db.accountReconfigPassword(account, password, null) != CustomerPatchAuthResult.SUCCESS) {
println("password change failed")
exitProcess(1)
} else {
diff --git a/bank/src/main/kotlin/tech/libeufin/bank/TalerMessage.kt b/bank/src/main/kotlin/tech/libeufin/bank/TalerMessage.kt
@@ -609,5 +609,6 @@ data class PublicAccount(
*/
@Serializable
data class AccountPasswordChange(
- val new_password: String
+ val new_password: String,
+ val old_password: String? = null
)
\ No newline at end of file
diff --git a/bank/src/main/kotlin/tech/libeufin/bank/db/Database.kt b/bank/src/main/kotlin/tech/libeufin/bank/db/Database.kt
@@ -110,15 +110,6 @@ class Database(dbConfig: String, internal val bankCurrency: String, internal val
}
}
- suspend fun customerChangePassword(customerName: String, passwordHash: String): Boolean = conn { conn ->
- val stmt = conn.prepareStatement("""
- UPDATE customers SET password_hash=? where login=?
- """)
- stmt.setString(1, passwordHash)
- stmt.setString(2, customerName)
- stmt.executeUpdateCheck()
- }
-
suspend fun customerPasswordHashFromLogin(login: String): String? = conn { conn ->
val stmt = conn.prepareStatement("""
SELECT password_hash FROM customers WHERE login=?
@@ -438,6 +429,30 @@ class Database(dbConfig: String, internal val bankCurrency: String, internal val
}
}
+ suspend fun accountReconfigPassword(login: String, newPw: String, oldPw: String?): CustomerPatchAuthResult = conn {
+ it.transaction { conn ->
+ val currentPwh = conn.prepareStatement("""
+ SELECT password_hash FROM customers WHERE login=?
+ """).run {
+ setString(1, login)
+ oneOrNull { it.getString(1) }
+ }
+ if (currentPwh == null) {
+ CustomerPatchAuthResult.ACCOUNT_NOT_FOUND
+ } else if (oldPw != null && !CryptoUtil.checkpw(oldPw, currentPwh)) {
+ CustomerPatchAuthResult.CONFLICT_BAD_PASSWORD
+ } else {
+ val stmt = conn.prepareStatement("""
+ UPDATE customers SET password_hash=? where login=?
+ """)
+ stmt.setString(1, CryptoUtil.hashpw(newPw))
+ stmt.setString(2, login)
+ stmt.executeUpdateCheck()
+ CustomerPatchAuthResult.SUCCESS
+ }
+ }
+ }
+
/**
* Gets the list of public accounts in the system.
* internalCurrency is the bank's currency and loginFilter is
@@ -943,6 +958,13 @@ enum class CustomerPatchResult {
SUCCESS
}
+/** Result status of customer account auth patch */
+enum class CustomerPatchAuthResult {
+ ACCOUNT_NOT_FOUND,
+ CONFLICT_BAD_PASSWORD,
+ SUCCESS
+}
+
/** Result status of customer account deletion */
enum class CustomerDeletionResult {
SUCCESS,
diff --git a/bank/src/test/kotlin/CoreBankApiTest.kt b/bank/src/test/kotlin/CoreBankApiTest.kt
@@ -352,12 +352,12 @@ class CoreBankAccountsMgmtApiTest {
jsonBody(req)
}.assertNoContent()
- suspend fun checkAdminOnly(req: JsonElement) {
+ suspend fun checkAdminOnly(req: JsonElement, error: TalerErrorCode) {
// Checking ordinary user doesn't get to patch
client.patch("/accounts/merchant") {
basicAuth("merchant", "merchant-password")
jsonBody(req)
- }.assertForbidden()
+ }.assertConflict(error)
// Finally checking that admin does get to patch
client.patch("/accounts/merchant") {
basicAuth("admin", "admin-password")
@@ -365,14 +365,20 @@ class CoreBankAccountsMgmtApiTest {
}.assertNoContent()
}
- checkAdminOnly(json(req) { "name" to "Another Foo" })
- checkAdminOnly(json(req) { "debit_threshold" to "KUDOS:100" })
+ checkAdminOnly(
+ json(req) { "name" to "Another Foo" },
+ TalerErrorCode.BANK_NON_ADMIN_PATCH_LEGAL_NAME
+ )
+ checkAdminOnly(
+ json(req) { "debit_threshold" to "KUDOS:100" },
+ TalerErrorCode.BANK_NON_ADMIN_PATCH_DEBT_LIMIT
+ )
// Check admin account cannot be exchange
client.patch("/accounts/admin") {
basicAuth("admin", "admin-password")
jsonBody { "is_taler_exchange" to true }
- }.assertForbidden()
+ }.assertConflict(TalerErrorCode.BANK_PATCH_ADMIN_EXCHANGE)
// But we can change its debt limit
client.patch("/accounts/admin") {
basicAuth("admin", "admin-password")
@@ -402,21 +408,49 @@ class CoreBankAccountsMgmtApiTest {
@Test
fun passwordChangeTest() = bankSetup { _ ->
// Changing the password.
- client.patch("/accounts/merchant/auth") {
- basicAuth("merchant", "merchant-password")
+ client.patch("/accounts/customer/auth") {
+ basicAuth("customer", "customer-password")
jsonBody {
+ "old_password" to "customer-password"
"new_password" to "new-password"
}
}.assertNoContent()
// Previous password should fail.
- client.patch("/accounts/merchant/auth") {
- basicAuth("merchant", "merchant-password")
+ client.patch("/accounts/customer/auth") {
+ basicAuth("customer", "customer-password")
}.assertUnauthorized()
// New password should succeed.
- client.patch("/accounts/merchant/auth") {
- basicAuth("merchant", "new-password")
+ client.patch("/accounts/customer/auth") {
+ basicAuth("customer", "new-password")
jsonBody {
- "new_password" to "merchant-password"
+ "old_password" to "new-password"
+ "new_password" to "customer-password"
+ }
+ }.assertNoContent()
+
+
+ // Check require test old password
+ client.patch("/accounts/customer/auth") {
+ basicAuth("customer", "customer-password")
+ jsonBody {
+ "old_password" to "bad-password"
+ "new_password" to "new-password"
+ }
+ }.assertConflict(TalerErrorCode.BANK_PATCH_BAD_OLD_PASSWORD)
+
+ // Check require old password for user
+ client.patch("/accounts/customer/auth") {
+ basicAuth("customer", "customer-password")
+ jsonBody {
+ "new_password" to "new-password"
+ }
+ }.assertConflict(TalerErrorCode.BANK_NON_ADMIN_PATCH_MISSING_OLD_PASSWORD)
+
+ // Check admin
+ client.patch("/accounts/customer/auth") {
+ basicAuth("admin", "admin-password")
+ jsonBody {
+ "new_password" to "new-password"
}
}.assertNoContent()
}
diff --git a/util/src/main/kotlin/TalerErrorCode.kt b/util/src/main/kotlin/TalerErrorCode.kt
@@ -666,6 +666,14 @@ enum class TalerErrorCode(val code: Int) {
/**
+ * The exchange failed to start a KYC attribute conversion helper process. It is likely configured incorrectly.
+ * Returned with an HTTP status code of #MHD_HTTP_INTERNAL_SERVER_ERROR (500).
+ * (A value of 0 indicates that the error is generated client-side).
+ */
+ EXCHANGE_GENERIC_KYC_CONVERTER_FAILED(1037),
+
+
+ /**
* The exchange did not find information about the specified transaction in the database.
* Returned with an HTTP status code of #MHD_HTTP_NOT_FOUND (404).
* (A value of 0 indicates that the error is generated client-side).
@@ -3426,6 +3434,46 @@ enum class TalerErrorCode(val code: Int) {
/**
+ * A non-admin user has tried to change their legal name.
+ * Returned with an HTTP status code of #MHD_HTTP_CONFLICT (409).
+ * (A value of 0 indicates that the error is generated client-side).
+ */
+ BANK_NON_ADMIN_PATCH_LEGAL_NAME(5135),
+
+
+ /**
+ * A non-admin user has tried to change their debt limit.
+ * Returned with an HTTP status code of #MHD_HTTP_CONFLICT (409).
+ * (A value of 0 indicates that the error is generated client-side).
+ */
+ BANK_NON_ADMIN_PATCH_DEBT_LIMIT(5136),
+
+
+ /**
+ * A non-admin user has tried to change their password whihout providing the current one.
+ * Returned with an HTTP status code of #MHD_HTTP_CONFLICT (409).
+ * (A value of 0 indicates that the error is generated client-side).
+ */
+ BANK_NON_ADMIN_PATCH_MISSING_OLD_PASSWORD(5137),
+
+
+ /**
+ * Provided old password does not match current password.
+ * Returned with an HTTP status code of #MHD_HTTP_CONFLICT (409).
+ * (A value of 0 indicates that the error is generated client-side).
+ */
+ BANK_PATCH_BAD_OLD_PASSWORD(5138),
+
+
+ /**
+ * An admin user has tried to become an exchange.
+ * Returned with an HTTP status code of #MHD_HTTP_CONFLICT (409).
+ * (A value of 0 indicates that the error is generated client-side).
+ */
+ BANK_PATCH_ADMIN_EXCHANGE(5139),
+
+
+ /**
* The sync service failed find the account in its database.
* Returned with an HTTP status code of #MHD_HTTP_NOT_FOUND (404).
* (A value of 0 indicates that the error is generated client-side).
