Check new tan parameters using a 2FA challenge when an user modifies them

10 files changed, 347 insertions, 163 deletions

diff --git a/bank/conf/test.conf b/bank/conf/test.conf
index b4eb3953..4a7a4476 100644
--- a/bank/conf/test.conf
+++ b/bank/conf/test.conf
@@ -8,7 +8,7 @@ ALLOW_EDIT_CASHOUT_PAYTO_URI = yes
 allow_conversion = YES
 FIAT_CURRENCY = EUR
 tan_sms = libeufin-tan-file.sh
-tan_email = libeufin-tan-fail.sh
+tan_email = libeufin-tan-file.sh
 
 [libeufin-bankdb-postgres]
 CONFIG = postgresql:///libeufincheck
diff --git a/bank/conf/test_no_tan.conf b/bank/conf/test_tan_err.conf
index 52e824b2..faaf9883 100644
--- a/bank/conf/test_no_tan.conf
+++ b/bank/conf/test_tan_err.conf
@@ -6,6 +6,8 @@ FIAT_CURRENCY = EUR
 ALLOW_REGISTRATION = yes
 ALLOW_ACCOUNT_DELETION = yes
 ALLOW_EDIT_CASHOUT_PAYTO_URI = yes
+tan_sms = libeufin-tan-fail.sh
+tan_email =
 
 [libeufin-bankdb-postgres]
 CONFIG = postgresql:///libeufincheck
diff --git a/bank/src/main/kotlin/tech/libeufin/bank/CoreBankApi.kt b/bank/src/main/kotlin/tech/libeufin/bank/CoreBankApi.kt
index 17587d5f..ea71cd40 100644
--- a/bank/src/main/kotlin/tech/libeufin/bank/CoreBankApi.kt
+++ b/bank/src/main/kotlin/tech/libeufin/bank/CoreBankApi.kt
@@ -192,7 +192,16 @@ suspend fun createAccount(db: Database, ctx: BankConfig, req: RegisterAccountReq
     }
 }
 
-suspend fun patchAccount(db: Database, ctx: BankConfig, req: AccountReconfiguration, username: String, isAdmin: Boolean, is2fa: Boolean): AccountPatchResult {
+suspend fun patchAccount(
+    db: Database, 
+    ctx: BankConfig, 
+    req: AccountReconfiguration, 
+    username: String, 
+    isAdmin: Boolean, 
+    is2fa: Boolean, 
+    channel: TanChannel? = null, 
+    info: String? = null
+): AccountPatchResult {
     req.debit_threshold?.run { ctx.checkRegionalCurrency(this) }
     val contactData = req.contact_data ?: req.challenge_contact_data
 
@@ -217,16 +226,27 @@ suspend fun patchAccount(db: Database, ctx: BankConfig, req: AccountReconfigurat
         debtLimit = req.debit_threshold,
         isAdmin = isAdmin,
         is2fa = is2fa,
+        faChannel = channel,
+        faInfo = info,
         allowEditName = ctx.allowEditName,
         allowEditCashout = ctx.allowEditCashout
     )
 }
 
-suspend fun ApplicationCall.patchAccountHttp(db: Database, ctx: BankConfig, req: AccountReconfiguration, is2fa: Boolean) {
-    val res = patchAccount(db, ctx, req, username, isAdmin, is2fa)
+suspend fun ApplicationCall.patchAccountHttp(
+    db: Database, 
+    ctx: BankConfig, 
+    req: AccountReconfiguration, 
+    is2fa: Boolean,
+    channel: TanChannel? = null, 
+    info: String? = null
+) {
+    val res = patchAccount(db, ctx, req, username, isAdmin, is2fa, channel, info)
     when (res) {
         AccountPatchResult.Success -> respond(HttpStatusCode.NoContent)
-        AccountPatchResult.TanRequired -> respondChallenge(db, Operation.account_reconfig, req)
+        is AccountPatchResult.TanRequired -> {
+            respondChallenge(db, Operation.account_reconfig, req, res.channel, res.info)
+        }
         AccountPatchResult.UnknownAccount -> throw unknownAccount(username)
         AccountPatchResult.NonAdminName -> throw conflict(
             "non-admin user cannot change their legal name",
@@ -240,10 +260,6 @@ suspend fun ApplicationCall.patchAccountHttp(db: Database, ctx: BankConfig, req:
             "non-admin user cannot change their debt limit",
             TalerErrorCode.BANK_NON_ADMIN_PATCH_DEBT_LIMIT
         )
-        AccountPatchResult.NonAdminContact -> throw conflict(
-            "non-admin user cannot change their contact info",
-            TalerErrorCode.BANK_NON_ADMIN_PATCH_CONTACT
-        )
         AccountPatchResult.MissingTanInfo -> throw conflict(
             "missing info for tan channel ${req.tan_channel.get()}",
             TalerErrorCode.BANK_MISSING_TAN_INFO
@@ -779,7 +795,7 @@ private fun Routing.coreBankTanApi(db: Database, ctx: BankConfig) {
                 is TanSolveResult.Success -> when (res.op) {
                     Operation.account_reconfig -> {
                         val req = Json.decodeFromString<AccountReconfiguration>(res.body);
-                        call.patchAccountHttp(db, ctx, req, true)
+                        call.patchAccountHttp(db, ctx, req, true, res.channel, res.info)
                     }
                     Operation.account_delete -> {
                         call.deleteAccountHttp(db, ctx, true)
diff --git a/bank/src/main/kotlin/tech/libeufin/bank/Main.kt b/bank/src/main/kotlin/tech/libeufin/bank/Main.kt
index 483a25cf..bfe30d96 100644
--- a/bank/src/main/kotlin/tech/libeufin/bank/Main.kt
+++ b/bank/src/main/kotlin/tech/libeufin/bank/Main.kt
@@ -398,8 +398,7 @@ class EditAccount : CliktCommand(
                 AccountPatchResult.NonAdminName,
                     AccountPatchResult.NonAdminCashout,
                     AccountPatchResult.NonAdminDebtLimit,
-                    AccountPatchResult.NonAdminContact,
-                    AccountPatchResult.TanRequired  -> {
+                    is AccountPatchResult.TanRequired  -> {
                         // Unreachable as we edit account as admin
                     }
             }
diff --git a/bank/src/main/kotlin/tech/libeufin/bank/Tan.kt b/bank/src/main/kotlin/tech/libeufin/bank/Tan.kt
index 63b40a6b..a366ce13 100644
--- a/bank/src/main/kotlin/tech/libeufin/bank/Tan.kt
+++ b/bank/src/main/kotlin/tech/libeufin/bank/Tan.kt
@@ -29,7 +29,13 @@ import io.ktor.server.response.*
 import io.ktor.server.application.*
 
 
-inline suspend fun <reified B> ApplicationCall.respondChallenge(db: Database, op: Operation, body: B) {
+inline suspend fun <reified B> ApplicationCall.respondChallenge(
+    db: Database, 
+    op: Operation, 
+    body: B, 
+    channel: TanChannel? = null,
+    info: String? = null
+) {
     val json = Json.encodeToString(kotlinx.serialization.serializer<B>(), body); 
     val code = Tan.genCode()
     val id = db.tan.new(
@@ -39,7 +45,9 @@ inline suspend fun <reified B> ApplicationCall.respondChallenge(db: Database, op
         code = code,
         now = Instant.now(), 
         retryCounter = TAN_RETRY_COUNTER,
-        validityPeriod = TAN_VALIDITY_PERIOD
+        validityPeriod = TAN_VALIDITY_PERIOD,
+        channel = channel,
+        info = info
     )
     respond(
         status = HttpStatusCode.Accepted,
diff --git a/bank/src/main/kotlin/tech/libeufin/bank/db/AccountDAO.kt b/bank/src/main/kotlin/tech/libeufin/bank/db/AccountDAO.kt
index 2e702b66..52c207f7 100644
--- a/bank/src/main/kotlin/tech/libeufin/bank/db/AccountDAO.kt
+++ b/bank/src/main/kotlin/tech/libeufin/bank/db/AccountDAO.kt
@@ -202,15 +202,14 @@ class AccountDAO(private val db: Database) {
     }
 
     /** Result status of customer account patch */
-    enum class AccountPatchResult {
-        UnknownAccount,
-        NonAdminName,
-        NonAdminCashout,
-        NonAdminDebtLimit,
-        NonAdminContact,
-        MissingTanInfo,
-        TanRequired,
-        Success
+    sealed class AccountPatchResult {
+        data object UnknownAccount: AccountPatchResult()
+        data object NonAdminName: AccountPatchResult()
+        data object NonAdminCashout: AccountPatchResult()
+        data object NonAdminDebtLimit: AccountPatchResult()
+        data object MissingTanInfo: AccountPatchResult()
+        data class TanRequired(val channel: TanChannel?, val info: String?): AccountPatchResult()
+        data object Success: AccountPatchResult()
     }
 
     /** Change account [login] informations */
@@ -225,30 +224,28 @@ class AccountDAO(private val db: Database) {
         debtLimit: TalerAmount?,
         isAdmin: Boolean,
         is2fa: Boolean,
+        faChannel: TanChannel?,
+        faInfo: String?,
         allowEditName: Boolean,
         allowEditCashout: Boolean,
     ): AccountPatchResult = db.serializable { it.transaction { conn ->
         val checkName = !isAdmin && !allowEditName && name != null
         val checkCashout = !isAdmin && !allowEditCashout && cashoutPayto.isSome()
         val checkDebtLimit = !isAdmin && debtLimit != null
-        val checkPhone = !isAdmin && phone.isSome()
-        val checkEmail = !isAdmin && email.isSome()
 
         // Get user ID and check reconfig rights
-        val customer_id = conn.prepareStatement("""
+        val (customerId, currChannel, currInfo) = conn.prepareStatement("""
             SELECT
                 customer_id
                 ,(${ if (checkName) "name != ?" else "false" }) as name_change
                 ,(${ if (checkCashout) "cashout_payto IS DISTINCT FROM ?" else "false" }) as cashout_change
                 ,(${ if (checkDebtLimit) "max_debt != (?, ?)::taler_amount" else "false" }) as debt_limit_change
-                ,(${ if (checkPhone) "phone IS DISTINCT FROM ?" else "false" }) as phone_change
-                ,(${ if (checkEmail) "email IS DISTINCT FROM ?" else "false" }) as email_change
                 ,(${ when (tan_channel.get()) {
                     null -> "false"
                     TanChannel.sms -> if (phone.get() != null) "false" else "phone IS NULL"
                     TanChannel.email -> if (email.get() != null) "false" else "email IS NULL"
                 }}) as missing_tan_info
-                ,(tan_channel IS NOT NULL) as tan_required
+                ,tan_channel, phone, email
             FROM customers
                 JOIN bank_accounts 
                 ON customer_id=owning_customer_id
@@ -265,12 +262,6 @@ class AccountDAO(private val db: Database) {
                 setLong(idx, debtLimit!!.value); idx++
                 setInt(idx, debtLimit.frac); idx++
             }
-            if (checkPhone) {
-                setString(idx, phone.get()); idx++
-            }
-            if (checkEmail) {
-                setString(idx, email.get()); idx++
-            }
             setString(idx, login)
             executeQuery().use {
                 when {
@@ -278,14 +269,49 @@ class AccountDAO(private val db: Database) {
                     it.getBoolean("name_change") -> return@transaction AccountPatchResult.NonAdminName
                     it.getBoolean("cashout_change") -> return@transaction AccountPatchResult.NonAdminCashout
                     it.getBoolean("debt_limit_change") -> return@transaction AccountPatchResult.NonAdminDebtLimit
-                    it.getBoolean("phone_change") -> return@transaction AccountPatchResult.NonAdminContact
-                    it.getBoolean("email_change") -> return@transaction AccountPatchResult.NonAdminContact
                     it.getBoolean("missing_tan_info") -> return@transaction AccountPatchResult.MissingTanInfo
-                    it.getBoolean("tan_required") && !is2fa && !isAdmin -> return@transaction AccountPatchResult.TanRequired
-                    else -> it.getLong("customer_id")
+                    else -> {
+                        val currChannel = it.getString("tan_channel")?.run { TanChannel.valueOf(this) }
+                        Triple(
+                            it.getLong("customer_id"),
+                            currChannel,
+                            when (tan_channel.get() ?: currChannel) {
+                                TanChannel.sms -> it.getString("phone")
+                                TanChannel.email -> it.getString("email")
+                                null -> null
+                            }
+                        )
+                    }
                 }
             }
         }
+ 
+        val newChannel = tan_channel.get();
+        val newInfo = when (newChannel ?: currChannel) {
+            TanChannel.sms -> phone.get()
+            TanChannel.email -> email.get()
+            null -> null
+        }
+
+        // Tan channel verification
+        if (!isAdmin) {
+            // Check performed 2fa check
+            if (currChannel != null && !is2fa) {
+                // Perform challenge with current settings
+                return@transaction AccountPatchResult.TanRequired(channel = null, info = null)
+            }
+            // If channel or info changed and the 2fa challenge is performed with old settings perform a new challenge with new settings
+            if ((newChannel != null && newChannel != faChannel) || (newInfo != null && newInfo != faInfo)) {
+                return@transaction AccountPatchResult.TanRequired(channel = newChannel ?: currChannel, info = newInfo ?: currInfo)
+            }
+        }
+
+        // Invalidate current challenges
+        if (newChannel != null || newInfo != null) {
+            val stmt = conn.prepareStatement("UPDATE tan_challenges SET expiration_date=0 WHERE customer=?")
+            stmt.setLong(1, customerId)
+            stmt.execute()
+        }
 
         // Update bank info
         conn.dynamicUpdate(
@@ -298,7 +324,7 @@ class AccountDAO(private val db: Database) {
             sequence {
                 isPublic?.let { yield(it) }
                 debtLimit?.let { yield(it.value); yield(it.frac) }
-                yield(customer_id)
+                yield(customerId)
             }
         )
 
@@ -319,7 +345,7 @@ class AccountDAO(private val db: Database) {
                 email.some { yield(it) }
                 tan_channel.some { yield(it?.name) }
                 name?.let { yield(it) }
-                yield(customer_id)
+                yield(customerId)
             }
         )
 
diff --git a/bank/src/main/kotlin/tech/libeufin/bank/db/TanDAO.kt b/bank/src/main/kotlin/tech/libeufin/bank/db/TanDAO.kt
index 3dc22ab2..631329ce 100644
--- a/bank/src/main/kotlin/tech/libeufin/bank/db/TanDAO.kt
+++ b/bank/src/main/kotlin/tech/libeufin/bank/db/TanDAO.kt
@@ -35,9 +35,11 @@ class TanDAO(private val db: Database) {
         code: String,
         now: Instant,
         retryCounter: Int,
-        validityPeriod: Duration
+        validityPeriod: Duration,
+        channel: TanChannel? = null,
+        info: String? = null
     ): Long = db.serializable { conn ->
-        val stmt = conn.prepareStatement("SELECT tan_challenge_create(?, ?::op_enum, ?, ?, ?, ?, ?, NULL, NULL)")
+        val stmt = conn.prepareStatement("SELECT tan_challenge_create(?,?::op_enum,?,?,?,?,?,?::tan_enum,?)")
         stmt.setString(1, body)
         stmt.setString(2, op.name)
         stmt.setString(3, code)
@@ -45,6 +47,8 @@ class TanDAO(private val db: Database) {
         stmt.setLong(5, TimeUnit.MICROSECONDS.convert(validityPeriod))
         stmt.setInt(6, retryCounter)
         stmt.setString(7, login)
+        stmt.setString(8, channel?.name)
+        stmt.setString(9, info)
         stmt.oneOrNull {
             it.getLong(1)
         } ?: throw internalServerError("TAN challenge returned nothing.")
@@ -76,7 +80,7 @@ class TanDAO(private val db: Database) {
             when {
                 !it.next() -> throw internalServerError("TAN send returned nothing.")
                 it.getBoolean("out_no_op") -> TanSendResult.NotFound
-                else -> TanSendResult.Success(
+                else ->  TanSendResult.Success(
                     tanInfo = it.getString("out_tan_info"),
                     tanChannel = it.getString("out_tan_channel").run { TanChannel.valueOf(this) },
                     tanCode = it.getString("out_tan_code")
@@ -100,7 +104,7 @@ class TanDAO(private val db: Database) {
 
     /** Result of TAN challenge solution */
     sealed class TanSolveResult {
-        data class Success(val body: String, val op: Operation): TanSolveResult()
+        data class Success(val body: String, val op: Operation, val channel: TanChannel?, val info: String?): TanSolveResult()
         data object NotFound: TanSolveResult()
         data object NoRetry: TanSolveResult()
         data object Expired: TanSolveResult()
@@ -117,7 +121,7 @@ class TanDAO(private val db: Database) {
         val stmt = conn.prepareStatement("""
             SELECT 
                 out_ok, out_no_op, out_no_retry, out_expired,
-                out_body, out_op
+                out_body, out_op, out_channel, out_info
             FROM tan_challenge_try(?,?,?,?)""")
         stmt.setLong(1, id)
         stmt.setString(2, login)
@@ -128,7 +132,9 @@ class TanDAO(private val db: Database) {
                 !it.next() -> throw internalServerError("TAN try returned nothing")
                 it.getBoolean("out_ok") -> TanSolveResult.Success(
                     body = it.getString("out_body"),
-                    op = Operation.valueOf(it.getString("out_op"))
+                    op = Operation.valueOf(it.getString("out_op")),
+                    channel = it.getString("out_channel")?.run { TanChannel.valueOf(this) },
+                    info = it.getString("out_info")
                 )
                 it.getBoolean("out_no_op") -> TanSolveResult.NotFound
                 it.getBoolean("out_no_retry") -> TanSolveResult.NoRetry
diff --git a/bank/src/test/kotlin/CoreBankApiTest.kt b/bank/src/test/kotlin/CoreBankApiTest.kt
index ff2548e2..3d95096a 100644
--- a/bank/src/test/kotlin/CoreBankApiTest.kt
+++ b/bank/src/test/kotlin/CoreBankApiTest.kt
@@ -395,12 +395,23 @@ class CoreBankAccountsApiTest {
     fun reconfig() = bankSetup { _ -> 
         authRoutine(HttpMethod.Patch, "/accounts/merchant", allowAdmin = true)
 
+        // Check tan info
+        for (channel in listOf("sms", "email")) {
+            client.patchA("/accounts/merchant") {
+                json { "tan_channel" to channel }
+            }.assertErr(TalerErrorCode.BANK_MISSING_TAN_INFO)
+        }
+
         // Successful attempt now
         val cashout = IbanPayTo(genIbanPaytoUri())
         val req = obj {
             "cashout_payto_uri" to cashout.canonical
             "name" to "Roger"
-            "is_public" to true 
+            "is_public" to true
+            "contact_data" to obj {
+                "phone" to "+99"
+                "email" to "foo@example.com"
+            }
         }
         client.patchA("/accounts/merchant") {
             json(req)
@@ -410,25 +421,11 @@ class CoreBankAccountsApiTest {
             json(req)
         }.assertNoContent()
 
-        // Check tan info
-        for (channel in listOf("sms", "email")) {
-            client.patchA("/accounts/merchant") {
-                json { "tan_channel" to channel }
-            }.assertErr(TalerErrorCode.BANK_MISSING_TAN_INFO)
-        }
         
         checkAdminOnly(
             obj(req) { "debit_threshold" to "KUDOS:100" },
             TalerErrorCode.BANK_NON_ADMIN_PATCH_DEBT_LIMIT
         )
-        checkAdminOnly(
-            obj(req) { "contact_data" to obj { "phone" to "+99" } },
-            TalerErrorCode.BANK_NON_ADMIN_PATCH_CONTACT
-        )
-        checkAdminOnly(
-            obj(req) { "contact_data" to obj { "email" to "foo@example.com" } },
-            TalerErrorCode.BANK_NON_ADMIN_PATCH_CONTACT
-        )
         
         // Check currency
         client.patch("/accounts/merchant") {
@@ -472,7 +469,7 @@ class CoreBankAccountsApiTest {
         fillTanInfo("merchant")
         client.patchA("/accounts/merchant") {
             json { "is_public" to false }
-        }.assertChallenge {
+        }.assertChallenge { _, _ ->
             client.getA("/accounts/merchant").assertOkJson<AccountData> { obj ->
                 assert(obj.is_public)
             }
@@ -508,11 +505,11 @@ class CoreBankAccountsApiTest {
 
     // Test TAN check account patch
     @Test
-    fun patchNoTan() = bankSetup(conf = "test_no_tan.conf") { _ -> 
+    fun patchTanErr() = bankSetup(conf = "test_tan_err.conf") { _ -> 
         // Check unsupported TAN channel
         client.patchA("/accounts/customer") {
             json {
-                "tan_channel" to "sms"
+                "tan_channel" to "email"
             }
         }.assertConflict(TalerErrorCode.BANK_TAN_CHANNEL_NOT_SUPPORTED)
     }
@@ -979,24 +976,24 @@ class CoreBankCashoutApiTest {
             }
         }.assertNoContent()
 
-        // Check email TAN error
-        client.postA("/accounts/customer/cashouts") {
+        // Check email TAN error TODO use 2FA
+        /*client.postA("/accounts/customer/cashouts") {
             json(req) {
                 "tan_channel" to "email"
             }
-        }.assertStatus(HttpStatusCode.BadGateway, TalerErrorCode.BANK_TAN_CHANNEL_SCRIPT_FAILED)
+        }.assertStatus(HttpStatusCode.BadGateway, TalerErrorCode.BANK_TAN_CHANNEL_SCRIPT_FAILED)*/
 
         // Check OK
         client.postA("/accounts/customer/cashouts") {
             json(req) 
         }.assertOkJson<CashoutPending> { first ->
-            smsCode("+99")
+            tanCode("+99")
             // Check idempotency
             client.postA("/accounts/customer/cashouts") {
                 json(req) 
             }.assertOkJson<CashoutPending> { second ->
                 assertEquals(first.cashout_id, second.cashout_id)
-                assertNull(smsCode("+99"))     
+                assertNull(tanCode("+99"))     
             }
         }
 
@@ -1041,23 +1038,6 @@ class CoreBankCashoutApiTest {
         }.assertBadRequest(TalerErrorCode.GENERIC_CURRENCY_MISMATCH)
     }
 
-    // POST /accounts/{USERNAME}/cashouts
-    @Test
-    fun createNoTan() = bankSetup("test_no_tan.conf") { _ ->
-        val req = obj {
-            "request_uid" to randShortHashCode()
-            "amount_debit" to "KUDOS:1"
-            "amount_credit" to convert("KUDOS:1")
-        }
-
-        fillCashoutInfo("customer")
-
-        // Check unsupported TAN channel
-        client.postA("/accounts/customer/cashouts") {
-            json(req) 
-        }.assertStatus(HttpStatusCode.NotImplemented, TalerErrorCode.BANK_TAN_CHANNEL_NOT_SUPPORTED)
-    }
-
     // POST /accounts/{USERNAME}/cashouts/{CASHOUT_ID}/abort
     @Test
     fun abort() = bankSetup { _ ->
@@ -1092,7 +1072,7 @@ class CoreBankCashoutApiTest {
             val id = it.cashout_id
 
             client.postA("/accounts/customer/cashouts/$id/confirm") {
-                json { "tan" to smsCode("+99") } 
+                json { "tan" to tanCode("+99") } 
             }.assertNoContent()
 
             // Check error
@@ -1159,7 +1139,7 @@ class CoreBankCashoutApiTest {
                 json { "tan" to "nice-try" } 
             }.assertConflict(TalerErrorCode.BANK_TAN_CHALLENGE_FAILED)
 
-            val code = smsCode("+99")
+            val code = tanCode("+99")
            
             // Check OK
             client.postA("/accounts/customer/cashouts/$id/confirm") {
@@ -1208,7 +1188,7 @@ class CoreBankCashoutApiTest {
             }.assertNoContent()
 
             client.postA("/accounts/customer/cashouts/$id/confirm"){
-                json { "tan" to smsCode("+99") } 
+                json { "tan" to tanCode("+99") } 
             }.assertConflict(TalerErrorCode.BANK_BAD_CONVERSION)
 
             // Check can abort because not confirmed
@@ -1227,7 +1207,7 @@ class CoreBankCashoutApiTest {
             // Send too much money
             tx("customer", "KUDOS:9", "merchant")
             client.postA("/accounts/customer/cashouts/$id/confirm"){
-                json { "tan" to smsCode("+99") } 
+                json { "tan" to tanCode("+99") } 
             }.assertConflict(TalerErrorCode.BANK_UNALLOWED_DEBIT)
 
             // Check can abort because not confirmed
@@ -1274,7 +1254,7 @@ class CoreBankCashoutApiTest {
             }
 
             client.postA("/accounts/customer/cashouts/$id/confirm") {
-                json { "tan" to smsCode("+99") }
+                json { "tan" to tanCode("+99") }
             }.assertNoContent()
             client.getA("/accounts/customer/cashouts/$id")
                 .assertOkJson<CashoutStatusResponse> {
@@ -1355,43 +1335,138 @@ class CoreBankCashoutApiTest {
 class CoreBankTanApiTest {
     // POST /accounts/{USERNAME}/challenge/{challenge_id}
     @Test
-    fun create() = bankSetup { _ ->
+    fun send() = bankSetup { _ ->
         authRoutine(HttpMethod.Post, "/accounts/merchant/challenge/42")
 
-        client.patch("/accounts/merchant") {
-            pwAuth("admin")
+        suspend fun HttpResponse.expectChallenge(channel: TanChannel, info: String): HttpResponse {
+            return assertChallenge { tanChannel, tanInfo ->
+                assertEquals(channel, tanChannel)
+                assertEquals(info, tanInfo)
+            }
+        }
+
+        suspend fun HttpResponse.expectTransmission(channel: TanChannel, info: String) {
+            this.assertOkJson<TanTransmission> {
+                assertEquals(it.tan_channel, channel)
+                assertEquals(it.tan_info, info)
+            }
+        }
+
+        // Set up 2fa 
+        client.patchA("/accounts/merchant") {
             json { 
                 "contact_data" to obj {
                     "phone" to "+99"
+                    "email" to "email@example.com"
                 }
                 "tan_channel" to "sms"
             }
+        }.expectChallenge(TanChannel.sms, "+99")
+            .assertNoContent()
+        
+        // Update 2fa settings - first 2FA challenge then new tan channel check
+        client.patchA("/accounts/merchant") {
+            json { // Info change
+                "contact_data" to obj { "phone" to "+98" }
+            }
+        }.expectChallenge(TanChannel.sms, "+99")
+            .expectChallenge(TanChannel.sms, "+98")
+            .assertNoContent()
+        client.patchA("/accounts/merchant") {
+            json { // Channel change
+                "tan_channel" to "email"
+            }
+        }.expectChallenge(TanChannel.sms, "+98")
+            .expectChallenge(TanChannel.email, "email@example.com")
+            .assertNoContent()
+        client.patchA("/accounts/merchant") {
+            json { // Both change
+                "contact_data" to obj { "phone" to "+97" }
+                "tan_channel" to "sms"
+            }
+        }.expectChallenge(TanChannel.email, "email@example.com")
+            .expectChallenge(TanChannel.sms, "+97")
+            .assertNoContent()
+
+        // Disable 2fa
+        client.patchA("/accounts/merchant") {
+            json { "tan_channel" to null as String? }
+        }.expectChallenge(TanChannel.sms, "+97")
+            .assertNoContent()
+
+        // Admin has no 2FA
+        client.patch("/accounts/merchant") {
+            pwAuth("admin")
+            json { 
+                "contact_data" to obj { "phone" to "+99" }
+                "tan_channel" to "sms"
+            }
+        }.assertNoContent()
+        client.patch("/accounts/merchant") {
+            pwAuth("admin")
+            json { "tan_channel" to "email" }
         }.assertNoContent()
+        client.patch("/accounts/merchant") {
+            pwAuth("admin")
+            json { "tan_channel" to null as String? }
+        }.assertNoContent()
+
+        // Check retry and invalidate
+        client.patchA("/accounts/merchant") {
+            json { 
+                "contact_data" to obj { "phone" to "+88" }
+                "tan_channel" to "sms"
+            }
+        }.assertChallenge().assertNoContent()
         client.patchA("/accounts/merchant") {
             json { "is_public" to false }
         }.assertAcceptedJson<TanChallenge> { 
             // Check ok
             client.postA("/accounts/merchant/challenge/${it.challenge_id}")
-                .assertOkJson<TanTransmission> {
-                    assertEquals(it.tan_info, "+99")
-                    assertEquals(it.tan_channel.name, "sms")
-                }
+                .expectTransmission(TanChannel.sms, "+88")
+            assertNotNull(tanCode("+88"))
             // Check retry
             client.postA("/accounts/merchant/challenge/${it.challenge_id}")
-                .assertOkJson<TanTransmission> {
-                    assertEquals(it.tan_info, "+99")
-                    assertEquals(it.tan_channel.name, "sms")
+                .expectTransmission(TanChannel.sms, "+88")
+            assertNull(tanCode("+88"))
+            // Idempotent patch does nothing
+            client.patchA("/accounts/merchant") {
+                json { 
+                    "contact_data" to obj { "phone" to "+88" }
+                    "tan_channel" to "sms"
+                }
+            }
+            client.postA("/accounts/merchant/challenge/${it.challenge_id}")
+                .expectTransmission(TanChannel.sms, "+88")
+            assertNull(tanCode("+88"))
+            // Change 2fa settings
+            client.patchA("/accounts/merchant") {
+                json { 
+                    "tan_channel" to "email"
                 }
+            }.expectChallenge(TanChannel.sms, "+88")
+                .expectChallenge(TanChannel.email, "email@example.com")
+                .assertNoContent()
+            // Check invalidated
+            client.postA("/accounts/merchant/challenge/${it.challenge_id}")
+                .expectTransmission(TanChannel.email, "email@example.com")
+            assertNotNull(tanCode("email@example.com"))
         }
 
+        // Unknown challenge
+        client.postA("/accounts/merchant/challenge/42")
+            .assertNotFound(TalerErrorCode.BANK_TRANSACTION_NOT_FOUND)
+    }
+
+    // POST /accounts/{USERNAME}/challenge/{challenge_id}
+    @Test
+    fun sendTanErr() = bankSetup("test_tan_err.conf") { _ ->
         // Check fail
         client.patch("/accounts/merchant") {
             pwAuth("admin")
             json { 
-                "contact_data" to obj {
-                    "email" to "test@test.com"
-                }
-                "tan_channel" to "email"
+                "contact_data" to obj { "phone" to "+1234" }
+                "tan_channel" to "sms"
             }
         }.assertNoContent()
         client.patchA("/accounts/merchant") {
@@ -1400,12 +1475,6 @@ class CoreBankTanApiTest {
             client.postA("/accounts/merchant/challenge/${it.challenge_id}")
                 .assertStatus(HttpStatusCode.BadGateway, TalerErrorCode.BANK_TAN_CHANNEL_SCRIPT_FAILED)
         }
-
-        // TODO check what happens when tan info or tan channel change
-
-        // Unknown challenge
-        client.postA("/accounts/merchant/challenge/42")
-            .assertNotFound(TalerErrorCode.BANK_TRANSACTION_NOT_FOUND)
     }
 
     // POST /accounts/{USERNAME}/challenge/{challenge_id}/confirm
@@ -1413,45 +1482,75 @@ class CoreBankTanApiTest {
     fun confirm() = bankSetup { _ ->
         authRoutine(HttpMethod.Post, "/accounts/merchant/challenge/42/confirm")
 
-        client.patch("/accounts/merchant") {
-            pwAuth("admin")
-            json { 
-                "contact_data" to obj {
-                    "phone" to "+99"
-                }
-                "tan_channel" to "sms"
-            }
-        }.assertNoContent()
-        val id = client.patchA("/accounts/merchant") {
+        fillTanInfo("merchant")
+
+        // Check simple case
+        client.patchA("/accounts/merchant") {
             json { "is_public" to false }
-        }.assertAcceptedJson<TanChallenge>().challenge_id
-        client.postA("/accounts/merchant/challenge/$id").assertOk()
-        val code = smsCode("+99")
-
-        // Check bad TAN code
-        client.postA("/accounts/merchant/challenge/$id/confirm") {
-            json { "tan" to "nice-try" } 
-        }.assertConflict(TalerErrorCode.BANK_TAN_CHALLENGE_FAILED)
-
-        // Check wrong account
-        client.postA("/accounts/customer/challenge/$id/confirm") {
-            json { "tan" to "nice-try" } 
-        }.assertNotFound(TalerErrorCode.BANK_TRANSACTION_NOT_FOUND)
-       
-        // Check OK
-        client.postA("/accounts/merchant/challenge/$id/confirm") {
-            json { "tan" to code }
-        }.assertNoContent()
-        // Check idempotence
-        client.postA("/accounts/merchant/challenge/$id/confirm") {
-            json { "tan" to code }
-        }.assertNoContent()
+        }.assertAcceptedJson<TanChallenge> {
+            val id = it.challenge_id
+            val info = client.postA("/accounts/merchant/challenge/$id")
+                .assertOkJson<TanTransmission>().tan_info
+            val code = tanCode(info)
+
+            // Check bad TAN code
+            client.postA("/accounts/merchant/challenge/$id/confirm") {
+                json { "tan" to "nice-try" } 
+            }.assertConflict(TalerErrorCode.BANK_TAN_CHALLENGE_FAILED)
+
+            // Check wrong account
+            client.postA("/accounts/customer/challenge/$id/confirm") {
+                json { "tan" to "nice-try" } 
+            }.assertNotFound(TalerErrorCode.BANK_TRANSACTION_NOT_FOUND)
         
-        // TODO check what happens when tan info or tan channel change
+            // Check OK
+            client.postA("/accounts/merchant/challenge/$id/confirm") {
+                json { "tan" to code }
+            }.assertNoContent()
+            // Check idempotence
+            client.postA("/accounts/merchant/challenge/$id/confirm") {
+                json { "tan" to code }
+            }.assertNoContent()
 
-        // Unknown challenge
-        client.postA("/accounts/merchant/challenge/42/confirm") {
-            json { "tan" to code }
-        }.assertNotFound(TalerErrorCode.BANK_TRANSACTION_NOT_FOUND)
+            // Unknown challenge
+            client.postA("/accounts/merchant/challenge/42/confirm") {
+                json { "tan" to code }
+            }.assertNotFound(TalerErrorCode.BANK_TRANSACTION_NOT_FOUND)
+        }
+        
+        // Check invalidation
+        client.patchA("/accounts/merchant") {
+            json { "is_public" to true }
+        }.assertAcceptedJson<TanChallenge> {
+            val id = it.challenge_id
+            val info = client.postA("/accounts/merchant/challenge/$id")
+                .assertOkJson<TanTransmission>().tan_info
+             
+            // Check invalidated
+            fillTanInfo("merchant")
+            client.postA("/accounts/merchant/challenge/$id/confirm") {
+                json { "tan" to tanCode(info) }
+            }.assertConflict(TalerErrorCode.BANK_TAN_CHALLENGE_FAILED)
+
+            val new = client.postA("/accounts/merchant/challenge/$id")
+                .assertOkJson<TanTransmission>().tan_info
+            val code = tanCode(new)
+            // Idempotent patch does nothing
+            client.patchA("/accounts/merchant") {
+                json { 
+                    "contact_data" to obj { "phone" to "+88" }
+                    "tan_channel" to "sms"
+                }
+            }
+            client.postA("/accounts/merchant/challenge/$id/confirm") {
+                json { "tan" to code }
+            }.assertNoContent()
+            
+            // Solved challenge remain solved
+            fillTanInfo("merchant")
+            client.postA("/accounts/merchant/challenge/$id/confirm") {
+                json { "tan" to code }
+            }.assertNoContent()
+        }
     }
 }
\ No newline at end of file
diff --git a/bank/src/test/kotlin/helpers.kt b/bank/src/test/kotlin/helpers.kt
index 71c5cf23..d0882c80 100644
--- a/bank/src/test/kotlin/helpers.kt
+++ b/bank/src/test/kotlin/helpers.kt
@@ -26,6 +26,7 @@ import java.io.ByteArrayOutputStream
 import java.io.File
 import java.util.zip.DeflaterOutputStream
 import kotlin.test.*
+import kotlin.random.Random
 import kotlinx.coroutines.*
 import kotlinx.serialization.json.*
 import net.taler.common.errorcodes.TalerErrorCode
@@ -168,6 +169,18 @@ suspend fun ApplicationTestBuilder.assertBalance(account: String, amount: String
     }
 }
 
+/** Check [account] tan channel and info */
+suspend fun ApplicationTestBuilder.tanInfo(account: String): Pair<TanChannel?, String?> {
+    val res = client.getA("/accounts/$account").assertOkJson<AccountData>()
+    val channel: TanChannel? = res.tan_channel
+    return Pair(channel, when (channel) {
+        TanChannel.sms -> res.contact_data!!.phone.get()
+        TanChannel.email -> res.contact_data!!.email.get()
+        null -> null
+        else -> null
+    })
+}
+
 /** Perform a bank transaction of [amount] [from] account [to] account with [subject} */
 suspend fun ApplicationTestBuilder.tx(from: String, amount: String, to: String, subject: String = "payout"): Long {
     return client.postA("/accounts/$from/transactions") {
@@ -225,7 +238,7 @@ suspend fun ApplicationTestBuilder.cashout(amount: String) {
         res
     }.assertOkJson<CashoutPending> {
         client.postA("/accounts/customer/cashouts/${it.cashout_id}/confirm") {
-            json { "tan" to smsCode("+99") }
+            json { "tan" to tanCode("+99") }
         }.assertNoContent()
     }
 }
@@ -259,7 +272,7 @@ suspend fun ApplicationTestBuilder.fillTanInfo(account: String) {
         pwAuth("admin")
         json {
             "contact_data" to obj {
-                "phone" to "+42"
+                "phone" to "+${Random.nextInt(0, 10000)}"
             }
             "tan_channel" to "sms"
         }
@@ -280,7 +293,7 @@ suspend fun ApplicationTestBuilder.convert(amount: String): TalerAmount {
         .assertOkJson<ConversionResponse>().amount_credit
 }
 
-suspend fun smsCode(info: String): String? {
+suspend fun tanCode(info: String): String? {
     val file = File("/tmp/tan-$info.txt");
     if (file.exists()) {
         val code = file.readText()
@@ -325,13 +338,25 @@ suspend fun HttpResponse.assertErr(code: TalerErrorCode): HttpResponse {
     return this
 }
 
-suspend fun HttpResponse.assertChallenge(check: suspend () -> Unit = {}): HttpResponse {
+suspend fun HttpResponse.challenge(): HttpResponse {
+    return if (this.status == HttpStatusCode.Accepted) {
+        this.assertChallenge()
+    } else {
+        this
+    }
+}
+
+suspend fun HttpResponse.assertChallenge(
+    check: suspend (TanChannel, String) -> Unit = { _, _ -> }
+): HttpResponse {
     val id = this.assertAcceptedJson<TanChallenge>().challenge_id
     val username = this.call.request.url.pathSegments[2]
-    this.call.client.postA("/accounts/$username/challenge/$id").assertOk()
-    check()
+    val res = this.call.client.postA("/accounts/$username/challenge/$id").assertOkJson<TanTransmission>()
+    check(res.tan_channel, res.tan_info)
+    val code = tanCode(res.tan_info)
+    assertNotNull(code)
     return this.call.client.postA("/accounts/$username/challenge/$id/confirm") {
-        json { "tan" to smsCode("+42") }
+        json { "tan" to code }
     }
 }
 
diff --git a/database-versioning/libeufin-bank-procedures.sql b/database-versioning/libeufin-bank-procedures.sql
index f6c6d86d..ae3278c8 100644
--- a/database-versioning/libeufin-bank-procedures.sql
+++ b/database-versioning/libeufin-bank-procedures.sql
@@ -1374,11 +1374,11 @@ retransmit BOOLEAN;
 BEGIN
 -- Retreive account id
 SELECT customer_id, tan_channel, CASE 
-      WHEN tan_channel = 'sms'   THEN phone
-      WHEN tan_channel = 'email' THEN email
-    END
-  INTO account_id, out_tan_channel, out_tan_info
-  FROM customers WHERE login = in_login;
+    WHEN tan_channel = 'sms'   THEN phone
+    WHEN tan_channel = 'email' THEN email
+  END
+INTO account_id, out_tan_channel, out_tan_info
+FROM customers WHERE login = in_login;
 
 -- Recover expiration date
 SELECT 
@@ -1429,7 +1429,9 @@ CREATE FUNCTION tan_challenge_try (
   OUT out_expired BOOLEAN,
   -- Success return
   OUT out_op op_enum,
-  OUT out_body TEXT
+  OUT out_body TEXT,
+  OUT out_channel tan_enum,
+  OUT out_info TEXT
 )
 LANGUAGE plpgsql as $$
 DECLARE
@@ -1458,7 +1460,8 @@ ELSIF NOT out_ok OR out_no_retry OR out_expired THEN
 END IF;
 
 -- Recover body and op from challenge
-SELECT body, op INTO out_body, out_op
+SELECT body, op, tan_channel, tan_info
+  INTO out_body, out_op, out_channel, out_info
   FROM tan_challenges WHERE challenge_id = in_challenge_id;
 END $$;
 COMMENT ON FUNCTION tan_challenge_try IS 'Try to confirm a challenge, return true if the challenge have been confirmed';