Add JSON error for common 404 and 405 errors, and more strict status/error checks

5 files changed, 74 insertions, 23 deletions

diff --git a/bank/src/main/kotlin/tech/libeufin/bank/Main.kt b/bank/src/main/kotlin/tech/libeufin/bank/Main.kt
index 5da9149f..4b2c63fc 100644
--- a/bank/src/main/kotlin/tech/libeufin/bank/Main.kt
+++ b/bank/src/main/kotlin/tech/libeufin/bank/Main.kt
@@ -157,6 +157,20 @@ fun Application.corebankWebApp(db: Database, ctx: BankConfig) {
         })
     }
     install(StatusPages) {
+        status(HttpStatusCode.NotFound) { call, status ->
+            call.err(
+                status,
+                "There is no endpoint defined for the URL provided by the client. Check if you used the correct URL and/or file a report with the developers of the client software.",
+                TalerErrorCode.GENERIC_ENDPOINT_UNKNOWN
+            )
+        }
+        status(HttpStatusCode.MethodNotAllowed) { call, status ->
+            call.err(
+                status,
+                "The HTTP method used is invalid for this endpoint. This is likely a bug in the client implementation. Check if you are using the latest available version and/or file a report with the developers.",
+                TalerErrorCode.GENERIC_METHOD_INVALID
+            )
+        }
         exception<Exception> { call, cause ->
             logger.debug("request failed", cause)
             when (cause) {
diff --git a/bank/src/test/kotlin/CommonApiTest.kt b/bank/src/test/kotlin/CommonApiTest.kt
new file mode 100644
index 00000000..645dbaa8
--- /dev/null
+++ b/bank/src/test/kotlin/CommonApiTest.kt
@@ -0,0 +1,41 @@
+/*
+ * This file is part of LibEuFin.
+ * Copyright (C) 2024 Taler Systems S.A.
+
+ * LibEuFin is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation; either version 3, or
+ * (at your option) any later version.
+
+ * LibEuFin is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ * or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU Affero General
+ * Public License for more details.
+
+ * You should have received a copy of the GNU Affero General Public
+ * License along with LibEuFin; see the file COPYING.  If not, see
+ * <http://www.gnu.org/licenses/>
+ */
+
+import io.ktor.client.request.*
+import io.ktor.client.statement.*
+import io.ktor.http.*
+import io.ktor.server.testing.*
+import kotlinx.serialization.json.JsonElement
+import org.junit.Test
+import tech.libeufin.bank.*
+import tech.libeufin.common.*
+import java.time.Duration
+import java.time.Instant
+import java.util.*
+import kotlin.test.assertEquals
+import kotlin.test.assertNotNull
+import kotlin.test.assertNull
+
+class CommonApiTest {
+    @Test
+    fun commonErr() = bankSetup { _ -> 
+        client.get("/unknown").assertNotFound(TalerErrorCode.GENERIC_ENDPOINT_UNKNOWN)
+        client.post("/config").assertStatus(HttpStatusCode.MethodNotAllowed, TalerErrorCode.GENERIC_METHOD_INVALID)
+    }
+}
\ No newline at end of file
diff --git a/bank/src/test/kotlin/ConversionApiTest.kt b/bank/src/test/kotlin/ConversionApiTest.kt
index 89de59d1..9a8f2787 100644
--- a/bank/src/test/kotlin/ConversionApiTest.kt
+++ b/bank/src/test/kotlin/ConversionApiTest.kt
@@ -108,9 +108,9 @@ class ConversionApiTest {
         client.get("/conversion-info/config")
             .assertNotImplemented()
         client.get("/conversion-info/cashin-rate")
-            .assertBadRequest()
+            .assertBadRequest(TalerErrorCode.GENERIC_PARAMETER_MISSING)
         client.get("/conversion-info/cashout-rate")
-            .assertBadRequest()
+            .assertBadRequest(TalerErrorCode.GENERIC_PARAMETER_MISSING)
         client.get("/conversion-info/cashin-rate?amount_credit=KUDOS:1")
             .assertNotImplemented()
         client.get("/conversion-info/cashout-rate?amount_credit=EUR:1")
diff --git a/bank/src/test/kotlin/CoreBankApiTest.kt b/bank/src/test/kotlin/CoreBankApiTest.kt
index fbc3492a..9c561391 100644
--- a/bank/src/test/kotlin/CoreBankApiTest.kt
+++ b/bank/src/test/kotlin/CoreBankApiTest.kt
@@ -49,7 +49,7 @@ class CoreBankConfigTest {
         }.assertOk()
         client.get("/monitor?timeframe=day=which=25") {
             pwAuth("admin")
-        }.assertBadRequest()
+        }.assertBadRequest(TalerErrorCode.GENERIC_PARAMETER_MALFORMED)
     }
 }
 
@@ -219,7 +219,7 @@ class CoreBankAccountsApiTest {
         }.let { req ->
             client.post("/accounts") {
                 json(req)
-            }.assertErr(TalerErrorCode.BANK_NON_ADMIN_PATCH_DEBT_LIMIT)
+            }.assertConflict(TalerErrorCode.BANK_NON_ADMIN_PATCH_DEBT_LIMIT)
             client.post("/accounts") {
                 json(req)
                 pwAuth("admin")
@@ -238,7 +238,7 @@ class CoreBankAccountsApiTest {
         }.let { req ->
             client.post("/accounts") {
                 json(req)
-            }.assertErr(TalerErrorCode.BANK_NON_ADMIN_SET_TAN_CHANNEL)
+            }.assertConflict(TalerErrorCode.BANK_NON_ADMIN_SET_TAN_CHANNEL)
             client.post("/accounts") {
                 json(req)
                 pwAuth("admin")
@@ -255,7 +255,7 @@ class CoreBankAccountsApiTest {
                     "name" to "Bat"
                     "tan_channel" to channel
                 }
-            }.assertErr(TalerErrorCode.BANK_MISSING_TAN_INFO)
+            }.assertConflict(TalerErrorCode.BANK_MISSING_TAN_INFO)
         }
 
         // Reserved account
@@ -488,7 +488,7 @@ class CoreBankAccountsApiTest {
         for (channel in listOf("sms", "email")) {
             client.patchA("/accounts/merchant") {
                 json { "tan_channel" to channel }
-            }.assertErr(TalerErrorCode.BANK_MISSING_TAN_INFO)
+            }.assertConflict(TalerErrorCode.BANK_MISSING_TAN_INFO)
         }
 
         // Successful attempt now
@@ -520,7 +520,7 @@ class CoreBankAccountsApiTest {
         client.patch("/accounts/merchant") {
             pwAuth("admin")
             json(req) { "debit_threshold" to "EUR:100" }
-        }.assertBadRequest()
+        }.assertBadRequest(TalerErrorCode.GENERIC_CURRENCY_MISMATCH)
 
         // Check patch
         client.getA("/accounts/merchant").assertOkJson<AccountData> { obj ->
diff --git a/bank/src/test/kotlin/helpers.kt b/bank/src/test/kotlin/helpers.kt
index b50b5e00..303bd909 100644
--- a/bank/src/test/kotlin/helpers.kt
+++ b/bank/src/test/kotlin/helpers.kt
@@ -317,7 +317,10 @@ suspend fun tanCode(info: String): String? {
 
 suspend fun HttpResponse.assertStatus(status: HttpStatusCode, err: TalerErrorCode?): HttpResponse {
     assertEquals(status, this.status, "$err")
-    if (err != null) assertErr(err)
+    if (err != null) {
+        val body = json<TalerError>()
+        assertEquals(err.code, body.code)
+    }
     return this
 }
 suspend fun HttpResponse.assertOk(): HttpResponse
@@ -326,26 +329,19 @@ suspend fun HttpResponse.assertNoContent(): HttpResponse
     = assertStatus(HttpStatusCode.NoContent, null)
 suspend fun HttpResponse.assertAccepted(): HttpResponse 
     = assertStatus(HttpStatusCode.Accepted, null)
-suspend fun HttpResponse.assertNotFound(err: TalerErrorCode?): HttpResponse 
+suspend fun HttpResponse.assertNotFound(err: TalerErrorCode): HttpResponse 
     = assertStatus(HttpStatusCode.NotFound, err)
-suspend fun HttpResponse.assertUnauthorized(): HttpResponse 
-    = assertStatus(HttpStatusCode.Unauthorized, null)
-suspend fun HttpResponse.assertConflict(err: TalerErrorCode?): HttpResponse 
+suspend fun HttpResponse.assertUnauthorized(err: TalerErrorCode = TalerErrorCode.GENERIC_UNAUTHORIZED): HttpResponse 
+    = assertStatus(HttpStatusCode.Unauthorized, err)
+suspend fun HttpResponse.assertConflict(err: TalerErrorCode): HttpResponse 
     = assertStatus(HttpStatusCode.Conflict, err)
-suspend fun HttpResponse.assertBadRequest(err: TalerErrorCode? = null): HttpResponse 
+suspend fun HttpResponse.assertBadRequest(err: TalerErrorCode = TalerErrorCode.GENERIC_JSON_INVALID): HttpResponse 
     = assertStatus(HttpStatusCode.BadRequest, err)
-suspend fun HttpResponse.assertForbidden(err: TalerErrorCode? = null): HttpResponse 
+suspend fun HttpResponse.assertForbidden(err: TalerErrorCode): HttpResponse 
     = assertStatus(HttpStatusCode.Forbidden, err)
-suspend fun HttpResponse.assertNotImplemented(err: TalerErrorCode? = null): HttpResponse 
+suspend fun HttpResponse.assertNotImplemented(err: TalerErrorCode = TalerErrorCode.END): HttpResponse 
     = assertStatus(HttpStatusCode.NotImplemented, err)
 
-
-suspend fun HttpResponse.assertErr(code: TalerErrorCode): HttpResponse {
-    val err = json<TalerError>()
-    assertEquals(code.code, err.code)
-    return this
-}
-
 suspend fun HttpResponse.maybeChallenge(): HttpResponse {
     return if (this.status == HttpStatusCode.Accepted) {
         this.assertChallenge()