commit 4b54eb04a938fa6662ce8b61be740878ee5f3281
parent 4f8b94faf1822d24ae1a4e9ece93bda2f797ab59
Author: MS <ms@taler.net>
Date: Thu, 14 Sep 2023 09:09:55 +0200
Fixing POST /accounts idempotency.
Diffstat:
6 files changed, 95 insertions(+), 75 deletions(-)
diff --git a/.idea/workspace.xml b/.idea/workspace.xml
@@ -5,16 +5,12 @@
</component>
<component name="ChangeListManager">
<list default="true" id="9436eb1e-de48-4f11-8ff7-f359340cb458" name="Changes" comment="">
- <change afterPath="$PROJECT_DIR$/bank/src/main/kotlin/tech/libeufin/bank/Helpers.kt" afterDir="false" />
- <change afterPath="$PROJECT_DIR$/bank/src/test/kotlin/AmountTest.kt" afterDir="false" />
- <change beforePath="$PROJECT_DIR$/.idea/gradle.xml" beforeDir="false" afterPath="$PROJECT_DIR$/.idea/gradle.xml" afterDir="false" />
<change beforePath="$PROJECT_DIR$/.idea/workspace.xml" beforeDir="false" afterPath="$PROJECT_DIR$/.idea/workspace.xml" afterDir="false" />
<change beforePath="$PROJECT_DIR$/bank/src/main/kotlin/tech/libeufin/bank/Database.kt" beforeDir="false" afterPath="$PROJECT_DIR$/bank/src/main/kotlin/tech/libeufin/bank/Database.kt" afterDir="false" />
+ <change beforePath="$PROJECT_DIR$/bank/src/main/kotlin/tech/libeufin/bank/Helpers.kt" beforeDir="false" afterPath="$PROJECT_DIR$/bank/src/main/kotlin/tech/libeufin/bank/Helpers.kt" afterDir="false" />
<change beforePath="$PROJECT_DIR$/bank/src/main/kotlin/tech/libeufin/bank/Main.kt" beforeDir="false" afterPath="$PROJECT_DIR$/bank/src/main/kotlin/tech/libeufin/bank/Main.kt" afterDir="false" />
- <change beforePath="$PROJECT_DIR$/bank/src/test/kotlin/DatabaseTest.kt" beforeDir="false" afterPath="$PROJECT_DIR$/bank/src/test/kotlin/DatabaseTest.kt" afterDir="false" />
+ <change beforePath="$PROJECT_DIR$/bank/src/test/kotlin/AmountTest.kt" beforeDir="false" afterPath="$PROJECT_DIR$/bank/src/test/kotlin/AmountTest.kt" afterDir="false" />
<change beforePath="$PROJECT_DIR$/bank/src/test/kotlin/LibeuFinApiTest.kt" beforeDir="false" afterPath="$PROJECT_DIR$/bank/src/test/kotlin/LibeuFinApiTest.kt" afterDir="false" />
- <change beforePath="$PROJECT_DIR$/contrib/wallet-core" beforeDir="false" afterPath="$PROJECT_DIR$/contrib/wallet-core" afterDir="false" />
- <change beforePath="$PROJECT_DIR$/util/src/test/kotlin/AmountTest.kt" beforeDir="false" />
</list>
<option name="SHOW_DIALOG" value="false" />
<option name="HIGHLIGHT_CONFLICTS" value="true" />
@@ -76,7 +72,7 @@
"settings.editor.splitter.proportion": "0.31419808"
}
}</component>
- <component name="RunManager" selected="Gradle.DatabaseTest.bankAccountTest">
+ <component name="RunManager" selected="Gradle.LibeuFinApiTest.createAccountTest">
<configuration name="AmountTest.parseAmountTest" type="GradleRunConfiguration" factoryName="Gradle" temporary="true">
<ExternalSystemSettings>
<option name="executionName" />
@@ -123,7 +119,7 @@
<DebugAllEnabled>false</DebugAllEnabled>
<method v="2" />
</configuration>
- <configuration name="DatabaseTest.bankAccountTest" type="GradleRunConfiguration" factoryName="Gradle" temporary="true">
+ <configuration name="CryptoUtilTest.passwordHashing" type="GradleRunConfiguration" factoryName="Gradle" temporary="true">
<ExternalSystemSettings>
<option name="executionName" />
<option name="externalProjectPath" value="$PROJECT_DIR$" />
@@ -134,9 +130,9 @@
</option>
<option name="taskNames">
<list>
- <option value=":bank:test" />
+ <option value=":util:test" />
<option value="--tests" />
- <option value=""DatabaseTest.bankAccountTest"" />
+ <option value=""CryptoUtilTest.passwordHashing"" />
</list>
</option>
<option name="vmOptions" />
@@ -146,12 +142,12 @@
<DebugAllEnabled>false</DebugAllEnabled>
<method v="2" />
</configuration>
- <configuration name="JsonTest.deserializationTest" type="GradleRunConfiguration" factoryName="Gradle" temporary="true">
+ <configuration name="DatabaseTest.bankAccountTest" type="GradleRunConfiguration" factoryName="Gradle" temporary="true">
<ExternalSystemSettings>
<option name="executionName" />
<option name="externalProjectPath" value="$PROJECT_DIR$" />
<option name="externalSystemIdString" value="GRADLE" />
- <option name="scriptParameters" value="--quiet" />
+ <option name="scriptParameters" value="" />
<option name="taskDescriptions">
<list />
</option>
@@ -159,7 +155,7 @@
<list>
<option value=":bank:test" />
<option value="--tests" />
- <option value=""JsonTest.deserializationTest"" />
+ <option value=""DatabaseTest.bankAccountTest"" />
</list>
</option>
<option name="vmOptions" />
@@ -193,19 +189,19 @@
<method v="2" />
</configuration>
<list>
+ <item itemvalue="Gradle.CryptoUtilTest.passwordHashing" />
<item itemvalue="Gradle.DatabaseTest.bankAccountTest" />
<item itemvalue="Gradle.AmountTest.parseTalerAmountTest" />
<item itemvalue="Gradle.AmountTest.parseAmountTest" />
- <item itemvalue="Gradle.JsonTest.deserializationTest" />
<item itemvalue="Gradle.LibeuFinApiTest.createAccountTest" />
</list>
<recent_temporary>
<list>
- <item itemvalue="Gradle.DatabaseTest.bankAccountTest" />
<item itemvalue="Gradle.LibeuFinApiTest.createAccountTest" />
+ <item itemvalue="Gradle.CryptoUtilTest.passwordHashing" />
<item itemvalue="Gradle.AmountTest.parseTalerAmountTest" />
+ <item itemvalue="Gradle.DatabaseTest.bankAccountTest" />
<item itemvalue="Gradle.AmountTest.parseAmountTest" />
- <item itemvalue="Gradle.JsonTest.deserializationTest" />
</list>
</recent_temporary>
</component>
diff --git a/bank/src/main/kotlin/tech/libeufin/bank/Database.kt b/bank/src/main/kotlin/tech/libeufin/bank/Database.kt
@@ -236,35 +236,6 @@ class Database(private val dbConfig: String) {
}
}
- fun customerPwAuth(login: String, pwHash: String): Customer? {
- reconnect()
- val stmt = prepare("""
- SELECT
- name,
- email,
- phone,
- cashout_payto,
- cashout_currency
- FROM customers
- WHERE login=? AND password_hash=?
- """)
- stmt.setString(1, login)
- stmt.setString(2, pwHash)
- val rs = stmt.executeQuery()
- rs.use {
- if (!rs.next()) return null
- return Customer(
- login = login,
- passwordHash = pwHash,
- name = it.getString("name"),
- phone = it.getString("phone"),
- email = it.getString("email"),
- cashoutCurrency = it.getString("cashout_currency"),
- cashoutPayto = it.getString("cashout_payto")
- )
- }
- }
-
// Mostly used to get customers out of bearer tokens.
fun customerGetFromRowId(customer_id: Long): Customer? {
reconnect()
diff --git a/bank/src/main/kotlin/tech/libeufin/bank/Helpers.kt b/bank/src/main/kotlin/tech/libeufin/bank/Helpers.kt
@@ -41,15 +41,10 @@ fun parseTalerAmount(
fracDigits: FracDigits = FracDigits.EIGHT
): TalerAmount {
val format = when (fracDigits) {
- FracDigits.TWO ->
- Pair("^([A-Z]+):([0-9])(\\.[0-9][0-9]?)?$", 100)
- FracDigits.EIGHT ->
- Pair(
- "^([A-Z]+):([0-9])(\\.[0-9][0-9]?[0-9]?[0-9]?[0-9]?[0-9]?[0-9]?[0-9]?)?\$",
- 100000000
- )
+ FracDigits.TWO -> "^([A-Z]+):([0-9]+)(\\.[0-9][0-9]?)?$"
+ FracDigits.EIGHT -> "^([A-Z]+):([0-9]+)(\\.[0-9][0-9]?[0-9]?[0-9]?[0-9]?[0-9]?[0-9]?[0-9]?)?$"
}
- val match = Regex(format.first).find(amount) ?: throw LibeufinBankException(
+ val match = Regex(format).find(amount) ?: throw LibeufinBankException(
httpStatus = HttpStatusCode.BadRequest,
talerError = TalerError(
code = BANK_BAD_FORMAT_AMOUNT,
@@ -59,7 +54,7 @@ fun parseTalerAmount(
// Fraction is at most 8 digits, so it's always < than MAX_INT.
val fraction: Int = match.destructured.component3().run {
var frac = 0
- var power = format.second
+ var power = 100000000
if (this.isNotEmpty())
// Skips the dot and processes the fractional chars.
this.substring(1).forEach { chr ->
diff --git a/bank/src/main/kotlin/tech/libeufin/bank/Main.kt b/bank/src/main/kotlin/tech/libeufin/bank/Main.kt
@@ -114,7 +114,9 @@ fun doBasicAuth(encodedCredentials: String): Customer? {
)
val login = userAndPassSplit[0]
val plainPassword = userAndPassSplit[1]
- return db.customerPwAuth(login, CryptoUtil.hashpw(plainPassword))
+ val maybeCustomer = db.customerGetFromLogin(login) ?: return null
+ if (!CryptoUtil.checkpw(plainPassword, maybeCustomer.passwordHash)) return null
+ return maybeCustomer
}
/* Performs the bearer-token authentication. Returns the
@@ -246,12 +248,11 @@ val webApp: Application.() -> Unit = {
if (maybeOnlyAdmin?.lowercase() == "yes") {
val customer: Customer? = call.myAuth(TokenScope.readwrite)
if (customer == null || customer.login != "admin")
- // OK to leak the only-admin policy here?
throw LibeufinBankException(
httpStatus = HttpStatusCode.Unauthorized,
talerError = TalerError(
code = BANK_LOGIN_FAILED,
- hint = "Only admin allowed."
+ hint = "Either 'admin' not authenticated or an ordinary user tried this operation."
)
)
}
@@ -281,12 +282,21 @@ val webApp: Application.() -> Unit = {
maybeCustomerExists.email == req.challenge_contact_data?.email &&
maybeCustomerExists.phone == req.challenge_contact_data?.phone &&
maybeCustomerExists.cashoutPayto == req.cashout_payto_uri &&
- maybeCustomerExists.passwordHash == CryptoUtil.hashpw(req.password) &&
+ CryptoUtil.checkpw(req.password, maybeCustomerExists.passwordHash) &&
maybeHasBankAccount.isPublic == req.is_public &&
maybeHasBankAccount.isTalerExchange == req.is_taler_exchange &&
maybeHasBankAccount.internalPaytoUri == req.internal_payto_uri
- if (isIdentic) call.respond(HttpStatusCode.Created)
- call.respond(HttpStatusCode.Conflict)
+ if (isIdentic) {
+ call.respond(HttpStatusCode.Created)
+ return@post
+ }
+ throw LibeufinBankException(
+ httpStatus = HttpStatusCode.Conflict,
+ talerError = TalerError(
+ code = GENERIC_UNDEFINED, // GANA needs this.
+ hint = "Idempotency check failed."
+ )
+ )
}
// From here: fresh user being added.
val newCustomer = Customer(
@@ -295,7 +305,7 @@ val webApp: Application.() -> Unit = {
email = req.challenge_contact_data?.email,
phone = req.challenge_contact_data?.phone,
cashoutPayto = req.cashout_payto_uri,
- // Following could be gone, if included in cashout_payto
+ // Following could be gone, if included in cashout_payto_uri
cashoutCurrency = db.configGet("cashout_currency"),
passwordHash = CryptoUtil.hashpw(req.password)
)
diff --git a/bank/src/test/kotlin/AmountTest.kt b/bank/src/test/kotlin/AmountTest.kt
@@ -1,9 +1,11 @@
import org.junit.Test
+import tech.libeufin.bank.FracDigits
import tech.libeufin.bank.parseTalerAmount
class AmountTest {
@Test
fun parseTalerAmountTest() {
+ parseTalerAmount("KUDOS:11")
val one = "EUR:1"
var obj = parseTalerAmount(one)
assert(obj.value == 1L && obj.frac == 0)
@@ -13,8 +15,12 @@ class AmountTest {
val onePointZeroOne = "EUR:1.01"
obj = parseTalerAmount(onePointZeroOne)
assert(obj.value == 1L && obj.frac == 1000000)
- // Invalid tries
obj = parseTalerAmount("EUR:0.00000001")
assert(obj.value == 0L && obj.frac == 1)
+ // Setting two fractional digits.
+ obj = parseTalerAmount("EUR:0.01", FracDigits.TWO) // one cent
+ assert(obj.value == 0L && obj.frac == 1000000)
+ obj = parseTalerAmount("EUR:0.1", FracDigits.TWO) // ten cents
+ assert(obj.value == 0L && obj.frac == 10000000)
}
}
\ No newline at end of file
diff --git a/bank/src/test/kotlin/LibeuFinApiTest.kt b/bank/src/test/kotlin/LibeuFinApiTest.kt
@@ -4,10 +4,8 @@ import io.ktor.http.*
import io.ktor.server.testing.*
import kotlinx.serialization.json.Json
import org.junit.Test
-import tech.libeufin.bank.Customer
-import tech.libeufin.bank.Database
-import tech.libeufin.bank.RegisterAccountRequest
-import tech.libeufin.bank.webApp
+import tech.libeufin.bank.*
+import tech.libeufin.util.CryptoUtil
import tech.libeufin.util.execCommand
class LibeuFinApiTest {
@@ -28,29 +26,73 @@ class LibeuFinApiTest {
val db = Database("jdbc:postgresql:///libeufincheck")
return db
}
+
+ /**
+ * Testing the account creation, its idempotency and
+ * the restriction to admin to create accounts.
+ */
@Test
fun createAccountTest() {
testApplication {
val db = initDb()
+ val ibanPayto = genIbanPaytoUri()
+ // Bank needs that to operate:
db.configSet("max_debt_ordinary_customers", "KUDOS:11")
- db.configSet("only_admin_registrations", "yes")
+ application(webApp)
+ var resp = client.post("/accounts") {
+ expectSuccess = false
+ contentType(ContentType.Application.Json)
+ setBody("""{
+ "username": "foo",
+ "password": "bar",
+ "name": "Jane",
+ "internal_payto_uri": "$ibanPayto"
+ }""".trimIndent())
+ }
+ assert(resp.status == HttpStatusCode.Created)
+ // Testing idempotency.
+ resp = client.post("/accounts") {
+ expectSuccess = false
+ contentType(ContentType.Application.Json)
+ setBody("""{
+ "username": "foo",
+ "password": "bar",
+ "name": "Jane",
+ "internal_payto_uri": "$ibanPayto"
+ }""".trimIndent())
+ }
+ assert(resp.status == HttpStatusCode.Created)
+ // Creating the administrator.
db.customerCreate(Customer(
"admin",
- "pass",
+ CryptoUtil.hashpw("pass"),
"CFO"
))
- application(webApp)
- val resp = client.post("/accounts") {
+ db.configSet("only_admin_registrations", "yes")
+ // Ordinary user tries, should fail.
+ resp = client.post("/accounts") {
expectSuccess = false
+ basicAuth("foo", "bar")
contentType(ContentType.Application.Json)
- basicAuth("admin", "bar")
setBody("""{
- "username": "foo",
- "password": "bar",
- "name": "Jane"
+ "username": "baz",
+ "password": "xyz",
+ "name": "Mallory"
+ }""".trimIndent())
+ }
+ assert(resp.status == HttpStatusCode.Unauthorized)
+ // admin tries, should succeed
+ resp = client.post("/accounts") {
+ expectSuccess = false
+ basicAuth("admin", "pass")
+ contentType(ContentType.Application.Json)
+ setBody("""{
+ "username": "baz",
+ "password": "xyz",
+ "name": "Mallory"
}""".trimIndent())
}
- println("Resp status code: ${resp.status}")
+ assert(resp.status == HttpStatusCode.Created)
}
}
}
\ No newline at end of file
