gnunet-svn
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[libeufin] branch master updated: Fixing POST /accounts idempotency.


From: gnunet
Subject: [libeufin] branch master updated: Fixing POST /accounts idempotency.
Date: Thu, 14 Sep 2023 09:10:35 +0200

This is an automated email from the git hooks/post-receive script.

ms pushed a commit to branch master
in repository libeufin.

The following commit(s) were added to refs/heads/master by this push:
     new 4b54eb04 Fixing POST /accounts idempotency.
4b54eb04 is described below

commit 4b54eb04a938fa6662ce8b61be740878ee5f3281
Author: MS <ms@taler.net>
AuthorDate: Thu Sep 14 09:09:55 2023 +0200

    Fixing POST /accounts idempotency.
---
 .idea/workspace.xml                                | 28 ++++-----
 .../src/main/kotlin/tech/libeufin/bank/Database.kt | 29 ---------
 bank/src/main/kotlin/tech/libeufin/bank/Helpers.kt | 13 ++---
 bank/src/main/kotlin/tech/libeufin/bank/Main.kt    | 24 +++++---
 bank/src/test/kotlin/AmountTest.kt                 |  8 ++-
 bank/src/test/kotlin/LibeuFinApiTest.kt            | 68 +++++++++++++++++-----
 6 files changed, 95 insertions(+), 75 deletions(-)

diff --git a/.idea/workspace.xml b/.idea/workspace.xml
index ccdc86c3..9cfe445d 100644
--- 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 @@
     &quot;settings.editor.splitter.proportion&quot;: &quot;0.31419808&quot;
   }
 }</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="&quot;DatabaseTest.bankAccountTest&quot;" />
+            <option value="&quot;CryptoUtilTest.passwordHashing&quot;" />
           </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="&quot;JsonTest.deserializationTest&quot;" />
+            <option value="&quot;DatabaseTest.bankAccountTest&quot;" />
           </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
index 2239a50c..baa1de43 100644
--- 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
index 19b1150c..9ee21a3c 100644
--- 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
index 2967732c..b0153781 100644
--- 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
index 2d8ffab0..b8c8f036 100644
--- 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
index 227af75f..ec09c813 100644
--- 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

-- 
To stop receiving notification emails like this one, please contact
gnunet@gnunet.org.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]