From 907742793a6f0148b8b50404b1da89adae892cb4 Mon Sep 17 00:00:00 2001 From: Hongwei Date: Wed, 15 Jul 2026 12:55:21 +0200 Subject: [PATCH 01/21] fix: enforce UK consent check on aggregate AISP balances/transactions endpoints GET /aisp/balances and GET /aisp/transactions fetched every private account for the caller without validating the bearer token's UK consent binding first, unlike every sibling AISP endpoint (including their own single-account counterparts). A DirectLogin or OAuth2 token with no bound consent fell through into the account-fetch path and surfaced as a 500 Unknown Error instead of the expected 403 OBP-35035, and callers with real consent got real account data with no consent enforcement at all. Add the same checkUKConsent + passesPsd2Aisp guard already used by getAccountsAccountIdBalances and getAccountsAccountIdTransactions so all five real-data AISP endpoints share one consent contract. Also fix createTransactionsJsonNew to take the AccountId from the request path instead of deriving it from the first transaction's bank account, which returned a null AccountId whenever the account had no transactions. --- .../v4_0_1/Http4sUKOBv401AccountInfo.scala | 6 ++- .../JSONFactory_UKOpenBanking_401.scala | 2 +- .../UKOpenBankingV401AccountInfoTests.scala | 54 ++++++------------- 3 files changed, 22 insertions(+), 40 deletions(-) diff --git a/obp-api/src/main/scala/code/api/UKOpenBanking/v4_0_1/Http4sUKOBv401AccountInfo.scala b/obp-api/src/main/scala/code/api/UKOpenBanking/v4_0_1/Http4sUKOBv401AccountInfo.scala index 3d02a40b4c..3380f6a84d 100644 --- a/obp-api/src/main/scala/code/api/UKOpenBanking/v4_0_1/Http4sUKOBv401AccountInfo.scala +++ b/obp-api/src/main/scala/code/api/UKOpenBanking/v4_0_1/Http4sUKOBv401AccountInfo.scala @@ -2236,7 +2236,7 @@ object Http4sUKOBv401AccountInfo extends MdcLoggable { transactions.map(_.id), view.viewId, Some(cc)) - } yield JSONFactory_UKOpenBanking_401.createTransactionsJsonNew(account.bankId, transactions, moderatedAttributes, view) + } yield JSONFactory_UKOpenBanking_401.createTransactionsJsonNew(accountId.value, account.bankId, transactions, moderatedAttributes, view) } } resourceDocs += ResourceDoc( @@ -2305,6 +2305,8 @@ object Http4sUKOBv401AccountInfo extends MdcLoggable { case req @ GET -> `ukV401Prefix` / "aisp" / "balances" => EndpointHelpers.withUser(req) { (u, cc) => for { + _ <- NewStyle.function.checkUKConsent(u, Some(cc)) + _ <- passesPsd2Aisp(Some(cc)) availablePrivateAccounts <- Views.views.vend.getPrivateBankAccountsFuture(u) (accounts, _) <- NewStyle.function.getBankAccounts(availablePrivateAccounts, Some(cc)) } yield JSONFactory_UKOpenBanking_401.createBalancesJSON(accounts) @@ -3446,6 +3448,8 @@ object Http4sUKOBv401AccountInfo extends MdcLoggable { case req @ GET -> `ukV401Prefix` / "aisp" / "transactions" => EndpointHelpers.withUser(req) { (u, cc) => for { + _ <- NewStyle.function.checkUKConsent(u, Some(cc)) + _ <- passesPsd2Aisp(Some(cc)) (bank, _) <- NewStyle.function.getBank(BankId(defaultBankId), Some(cc)) availablePrivateAccounts <- Views.views.vend.getPrivateBankAccountsFuture(u) (accounts, _) <- NewStyle.function.getBankAccounts(availablePrivateAccounts, Some(cc)) diff --git a/obp-api/src/main/scala/code/api/UKOpenBanking/v4_0_1/JSONFactory_UKOpenBanking_401.scala b/obp-api/src/main/scala/code/api/UKOpenBanking/v4_0_1/JSONFactory_UKOpenBanking_401.scala index f8fb2ec4ce..97aad8f019 100644 --- a/obp-api/src/main/scala/code/api/UKOpenBanking/v4_0_1/JSONFactory_UKOpenBanking_401.scala +++ b/obp-api/src/main/scala/code/api/UKOpenBanking/v4_0_1/JSONFactory_UKOpenBanking_401.scala @@ -293,12 +293,12 @@ object JSONFactory_UKOpenBanking_401 extends CustomJsonFormats { } def createTransactionsJsonNew( + accountId: String, bankId: BankId, moderatedTransactions: List[ModeratedTransaction], attributes: List[TransactionAttribute], view: View ): TransactionsUKV401 = { - val accountId = moderatedTransactions.headOption.flatMap(_.bankAccount.map(_.accountId.value)).orNull val transactions = moderatedTransactions.map(t => transactionJson(accountId, bankId, t, attributes, Some(view))) TransactionsUKV401( Data = TransactionDataV401(transactions), diff --git a/obp-api/src/test/scala/code/api/UKOpenBanking/v4_0_1/UKOpenBankingV401AccountInfoTests.scala b/obp-api/src/test/scala/code/api/UKOpenBanking/v4_0_1/UKOpenBankingV401AccountInfoTests.scala index d90a65a074..bc2e9f29a5 100644 --- a/obp-api/src/test/scala/code/api/UKOpenBanking/v4_0_1/UKOpenBankingV401AccountInfoTests.scala +++ b/obp-api/src/test/scala/code/api/UKOpenBanking/v4_0_1/UKOpenBankingV401AccountInfoTests.scala @@ -12,12 +12,16 @@ import org.scalatest.Tag // seeded data -> 200/201/204 with real field values, error paths (unknown // consent/account), and a full consent create -> get -> delete -> get lifecycle. // -// getAccounts / getAccountsAccountIdBalances / getAccountsAccountIdTransactions -// call NewStyle.function.checkUKConsent, which requires a live Hydra OAuth2 -// introspection endpoint (see code.api.util.ConsentUtil.checkUKConsent) — there -// is no local test double for Hydra, so (mirroring UKOpenBankingV310AisTests' -// precedent for the same three v3.1 endpoints) only "unauthenticated -> 401" -// and "authenticated -> not 401" are asserted for those three. +// getAccounts / getAccountsAccountIdBalances / getAccountsAccountIdTransactions / +// getBalances / getTransactions call NewStyle.function.checkUKConsent, which +// requires a live Hydra OAuth2 introspection endpoint (see +// code.api.util.ConsentUtil.checkUKConsent) — there is no local test double for +// Hydra, so (mirroring UKOpenBankingV310AisTests' precedent for the same v3.1 +// endpoints) only "unauthenticated -> 401" and "authenticated -> not 401" are +// asserted for those five. getBalances / getTransactions previously skipped the +// consent check entirely and returned real data straight from a DirectLogin +// token, which let a token with no bound consent reach 500 instead of the +// 403 OBP-35035 every other AISP data endpoint gives it. // // The remaining 80 endpoints are still static spec-faithful stubs; their tests // are unchanged (two scenarios: authenticated -> fixed code, unauthenticated -> 401). @@ -247,23 +251,10 @@ class UKOpenBankingV401AccountInfoTests extends UKOpenBankingV401ServerSetup { } // ── BalancesApi ──────────────────────────────────────────────────── + // DATA-DEPENDENT: checkUKConsent requires live Hydra (see class doc above). feature("UKOB v4.0.1 GET /aisp/balances") { - scenario("authenticated with real account -> 200 real balance", UKOpenBankingV401AccountInfo) { - val response = getAuthed("aisp", "balances") - response.code should equal(200) - // resourceUser1 owns accounts on several test banks (see TestConnectorSetup. - // createAccountRelevantResources), so testAccountId1's entry isn't necessarily - // first — find it rather than assume position 0. - val balances = (response.body \ "Data" \ "Balance").children - balances should not be empty - val myBalance = balances.find(b => (b \ "AccountId").extract[String] == acc) - myBalance shouldBe defined - (myBalance.get \ "Amount" \ "Currency").extract[String] should equal("EUR") - } - scenario("authenticated with no private accounts -> 200 empty Balance list", UKOpenBankingV401AccountInfo) { - val response = getAuthedAsUser2("aisp", "balances") - response.code should equal(200) - (response.body \ "Data" \ "Balance").children should be(empty) + scenario("authenticated", UKOpenBankingV401AccountInfo) { + getAuthed("aisp", "balances").code should not equal (401) } scenario("unauthenticated -> 401", UKOpenBankingV401AccountInfo) { getUnauthed("aisp", "balances").code should equal(401) @@ -333,23 +324,10 @@ class UKOpenBankingV401AccountInfoTests extends UKOpenBankingV401ServerSetup { getUnauthed("aisp", "statements").code should equal(401) } } + // DATA-DEPENDENT: checkUKConsent requires live Hydra (see class doc above). feature("UKOB v4.0.1 GET /aisp/transactions") { - scenario("authenticated with seeded transactions -> 200 real data", UKOpenBankingV401AccountInfo) { - val seeded = seedTransactions(testAccountId1) - val response = getAuthed("aisp", "transactions") - response.code should equal(200) - // resourceUser1 owns accounts on several test banks, but only testAccountId1 - // has seeded transactions here, so every returned entry should belong to it. - val transactions = (response.body \ "Data" \ "Transaction").children - transactions should not be empty - transactions.foreach(t => (t \ "AccountId").extract[String] should equal(acc)) - (transactions.head \ "Amount" \ "Currency").extract[String] should equal("EUR") - transactions.map(t => (t \ "TransactionId").extract[String]) should contain (seeded.head.id.value) - } - scenario("authenticated with no private accounts -> 200 empty Transaction list", UKOpenBankingV401AccountInfo) { - val response = getAuthedAsUser2("aisp", "transactions") - response.code should equal(200) - (response.body \ "Data" \ "Transaction").children should be(empty) + scenario("authenticated", UKOpenBankingV401AccountInfo) { + getAuthed("aisp", "transactions").code should not equal (401) } scenario("unauthenticated -> 401", UKOpenBankingV401AccountInfo) { getUnauthed("aisp", "transactions").code should equal(401) From 462e6088ed43356c83013f7620eb94fe17283bbe Mon Sep 17 00:00:00 2001 From: Hongwei Date: Wed, 15 Jul 2026 15:02:33 +0200 Subject: [PATCH 02/21] fix: stop background server launch from hanging callers via process substitution MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Redirecting to > >(tee "$RUNTIME_LOG") makes the tee process inherit this script's own stdout. A caller that captures this script's output with out=$(./flushall_build_and_run.sh --background ...) never sees EOF, because the long-running server (and the tee backing the substitution) never exits on its own — the command substitution hangs forever even though the server started successfully. Redirect straight to the log file instead. --- flushall_build_and_run.sh | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/flushall_build_and_run.sh b/flushall_build_and_run.sh index 13a8bfa6c5..dfffbf35db 100755 --- a/flushall_build_and_run.sh +++ b/flushall_build_and_run.sh @@ -155,15 +155,20 @@ JAVA_OPTS="--add-opens java.base/java.lang=ALL-UNNAMED \ RUNTIME_LOG=/tmp/obp-api.log if [ "$RUN_BACKGROUND" = true ]; then - # Run in background with output to log file (tee'd to /tmp as well) - nohup java $JAVA_OPTS -jar obp-api/target/obp-api.jar > >(tee "$RUNTIME_LOG") 2>&1 & + # Run in background with output redirected straight to a log file. Do NOT + # use `> >(tee "$RUNTIME_LOG")` here: process substitution's tee inherits + # this script's own stdout, so a caller that captures this script's output + # via `out=$(./flushall_build_and_run.sh --background ...)` never sees EOF + # — the substitution hangs forever, since the server (and thus the tee + # process backing the substitution) never exits on its own. + nohup java $JAVA_OPTS -jar obp-api/target/obp-api.jar > "$RUNTIME_LOG" 2>&1 & SERVER_PID=$! echo "✓ HTTP4S server started in background" echo " PID: $SERVER_PID" - echo " Log: http4s-server.log (also $RUNTIME_LOG)" + echo " Log: $RUNTIME_LOG" echo "" echo "To stop the server: kill $SERVER_PID" - echo "To view logs: tail -f http4s-server.log" + echo "To view logs: tail -f $RUNTIME_LOG" else # Run in foreground (Ctrl+C to stop). Also tee output to /tmp so it can be # tailed from another terminal without taking over this one. From f26bd07177f8da688ffd81a02b75fb0fd3d753a8 Mon Sep 17 00:00:00 2001 From: Hongwei Date: Wed, 15 Jul 2026 15:05:30 +0200 Subject: [PATCH 03/21] docs: remove stale Hydra references from UK v4.0.1 test comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit checkUKConsent no longer needs a live Hydra introspection endpoint — it checks the Bearer token's consent_id claim against an AUTHORISED consent. Two comments describing the balances/transactions scenarios still referred to the removed Hydra dependency; align them with the class doc above, which was already corrected during the merge from Simon/develop. --- .../v4_0_1/UKOpenBankingV401AccountInfoTests.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/obp-api/src/test/scala/code/api/UKOpenBanking/v4_0_1/UKOpenBankingV401AccountInfoTests.scala b/obp-api/src/test/scala/code/api/UKOpenBanking/v4_0_1/UKOpenBankingV401AccountInfoTests.scala index 2919b673c7..c32bcdeb2b 100644 --- a/obp-api/src/test/scala/code/api/UKOpenBanking/v4_0_1/UKOpenBankingV401AccountInfoTests.scala +++ b/obp-api/src/test/scala/code/api/UKOpenBanking/v4_0_1/UKOpenBankingV401AccountInfoTests.scala @@ -252,7 +252,7 @@ class UKOpenBankingV401AccountInfoTests extends UKOpenBankingV401ServerSetup { } // ── BalancesApi ──────────────────────────────────────────────────── - // DATA-DEPENDENT: checkUKConsent requires live Hydra (see class doc above). + // DATA-DEPENDENT: checkUKConsent requires a consent-bound token (see class doc above). feature("UKOB v4.0.1 GET /aisp/balances") { scenario("authenticated", UKOpenBankingV401AccountInfo) { getAuthed("aisp", "balances").code should not equal (401) @@ -325,7 +325,7 @@ class UKOpenBankingV401AccountInfoTests extends UKOpenBankingV401ServerSetup { getUnauthed("aisp", "statements").code should equal(401) } } - // DATA-DEPENDENT: checkUKConsent requires live Hydra (see class doc above). + // DATA-DEPENDENT: checkUKConsent requires a consent-bound token (see class doc above). feature("UKOB v4.0.1 GET /aisp/transactions") { scenario("authenticated", UKOpenBankingV401AccountInfo) { getAuthed("aisp", "transactions").code should not equal (401) From 11635278dc48a93d2f9d3ceb4b6019b484593043 Mon Sep 17 00:00:00 2001 From: Hongwei Date: Wed, 15 Jul 2026 15:45:13 +0200 Subject: [PATCH 04/21] test(uk-open-banking): assert real 403 for the 3 checkUKConsent-gated v4.0.1 endpoints MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit develop's Hydra removal (PR #2866) rewrote ConsentUtil.checkUKConsent to read the consent_id claim directly off the Bearer access token instead of calling an external Hydra introspection endpoint. That was the only blocker keeping getAccounts/getAccountsAccountIdBalances/getAccountsAccountIdTransactions on a weak "authenticated -> not 401" assertion (mirroring the same limitation in UKOpenBankingV310AisTests for the equivalent v3.1 endpoints). The OAuth1-signed test requests carry no Bearer JWT, so the consent_id claim lookup now fails deterministically with 403 ConsentIdClaimMissing — asserted directly (status code + error message) instead of the previous placeholder. --- .../UKOpenBankingV401AccountInfoTests.scala | 38 ++++++++++++------- 1 file changed, 24 insertions(+), 14 deletions(-) diff --git a/obp-api/src/test/scala/code/api/UKOpenBanking/v4_0_1/UKOpenBankingV401AccountInfoTests.scala b/obp-api/src/test/scala/code/api/UKOpenBanking/v4_0_1/UKOpenBankingV401AccountInfoTests.scala index d90a65a074..2a35118849 100644 --- a/obp-api/src/test/scala/code/api/UKOpenBanking/v4_0_1/UKOpenBankingV401AccountInfoTests.scala +++ b/obp-api/src/test/scala/code/api/UKOpenBanking/v4_0_1/UKOpenBankingV401AccountInfoTests.scala @@ -1,7 +1,9 @@ package code.api.UKOpenBanking.v4_0_1 import code.api.util.APIUtil.DateWithDayFormat +import code.api.util.ErrorMessages.ConsentIdClaimMissing import code.consent.Consents +import com.openbankproject.commons.model.ErrorMessage import org.json4s._ import org.scalatest.Tag @@ -12,12 +14,11 @@ import org.scalatest.Tag // seeded data -> 200/201/204 with real field values, error paths (unknown // consent/account), and a full consent create -> get -> delete -> get lifecycle. // -// getAccounts / getAccountsAccountIdBalances / getAccountsAccountIdTransactions -// call NewStyle.function.checkUKConsent, which requires a live Hydra OAuth2 -// introspection endpoint (see code.api.util.ConsentUtil.checkUKConsent) — there -// is no local test double for Hydra, so (mirroring UKOpenBankingV310AisTests' -// precedent for the same three v3.1 endpoints) only "unauthenticated -> 401" -// and "authenticated -> not 401" are asserted for those three. +// getAccounts / getAccountsAccountIdBalances / getAccountsAccountIdTransactions call +// NewStyle.function.checkUKConsent (code.api.util.ConsentUtil.checkUKConsent), which reads +// the `consent_id` claim off the Bearer access token — no external identity-provider call. +// These OAuth1-signed test requests carry no Bearer JWT, so the claim lookup deterministically +// fails with a 403 ConsentIdClaimMissing (see the scenario comments on those three features). // // The remaining 80 endpoints are still static spec-faithful stubs; their tests // are unchanged (two scenarios: authenticated -> fixed code, unauthenticated -> 401). @@ -103,10 +104,15 @@ class UKOpenBankingV401AccountInfoTests extends UKOpenBankingV401ServerSetup { } } // ── AccountsApi ──────────────────────────────────────────────────── - // DATA-DEPENDENT: checkUKConsent requires live Hydra (see class doc above). + // checkUKConsent extracts the `consent_id` claim from the Bearer access token (no external + // Hydra call since Consent.checkUKConsent dropped the Hydra dependency). These OAuth1-signed + // test requests carry no Bearer JWT at all, so the claim lookup deterministically fails -> + // 403 ConsentIdClaimMissing, mirroring "authenticated but no bound consent" in production. feature("UKOB v4.0.1 GET /aisp/accounts") { - scenario("authenticated", UKOpenBankingV401AccountInfo) { - getAuthed("aisp", "accounts").code should not equal (401) + scenario("authenticated without a consent-bound token -> 403", UKOpenBankingV401AccountInfo) { + val response = getAuthed("aisp", "accounts") + response.code should equal(403) + response.body.extract[ErrorMessage].message.trim should equal(ConsentIdClaimMissing.trim) } scenario("unauthenticated -> 401", UKOpenBankingV401AccountInfo) { getUnauthed("aisp", "accounts").code should equal(401) @@ -132,8 +138,10 @@ class UKOpenBankingV401AccountInfoTests extends UKOpenBankingV401ServerSetup { } } feature("UKOB v4.0.1 GET /aisp/accounts/ACCOUNT_ID/balances") { - scenario("authenticated", UKOpenBankingV401AccountInfo) { - getAuthed("aisp", "accounts", acc, "balances").code should not equal (401) + scenario("authenticated without a consent-bound token -> 403", UKOpenBankingV401AccountInfo) { + val response = getAuthed("aisp", "accounts", acc, "balances") + response.code should equal(403) + response.body.extract[ErrorMessage].message.trim should equal(ConsentIdClaimMissing.trim) } scenario("unauthenticated -> 401", UKOpenBankingV401AccountInfo) { getUnauthed("aisp", "accounts", acc, "balances").code should equal(401) @@ -236,10 +244,12 @@ class UKOpenBankingV401AccountInfoTests extends UKOpenBankingV401ServerSetup { } } // ── TransactionsApi ──────────────────────────────────────────────── - // DATA-DEPENDENT: checkUKConsent requires live Hydra (see class doc above). + // See the "no external Hydra call" note above feature("UKOB v4.0.1 GET /aisp/accounts"). feature("UKOB v4.0.1 GET /aisp/accounts/ACCOUNT_ID/transactions") { - scenario("authenticated", UKOpenBankingV401AccountInfo) { - getAuthed("aisp", "accounts", acc, "transactions").code should not equal (401) + scenario("authenticated without a consent-bound token -> 403", UKOpenBankingV401AccountInfo) { + val response = getAuthed("aisp", "accounts", acc, "transactions") + response.code should equal(403) + response.body.extract[ErrorMessage].message.trim should equal(ConsentIdClaimMissing.trim) } scenario("unauthenticated -> 401", UKOpenBankingV401AccountInfo) { getUnauthed("aisp", "accounts", acc, "transactions").code should equal(401) From f10d8b6e14ca4600ecead5721acfd8ec176c6fd8 Mon Sep 17 00:00:00 2001 From: Hongwei Date: Wed, 15 Jul 2026 19:59:00 +0200 Subject: [PATCH 05/21] fix: print the bound PORT from background build/run scripts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Neither flushall_build_and_run.sh nor flushall_fast_build_and_run.sh printed the port the server actually binds, only its PID. smoke_test.sh's start_and_test() parses "PID: " and "PORT: " from the script's captured output to know where to poll /root; with no PORT line it fails fast with "未打印实际监听端口" even after the server started and bound successfully, leaving the server running with nothing to verify it or clean it up. Print the actual dev.port value read from props/default.props (falling back to 8080) right after the PID line in both scripts' background-mode branch. --- flushall_build_and_run.sh | 4 ++++ flushall_fast_build_and_run.sh | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/flushall_build_and_run.sh b/flushall_build_and_run.sh index dfffbf35db..2d92353971 100755 --- a/flushall_build_and_run.sh +++ b/flushall_build_and_run.sh @@ -163,8 +163,12 @@ if [ "$RUN_BACKGROUND" = true ]; then # process backing the substitution) never exits on its own. nohup java $JAVA_OPTS -jar obp-api/target/obp-api.jar > "$RUNTIME_LOG" 2>&1 & SERVER_PID=$! + # Report the port the server will actually bind (dev.port in props), so callers + # that capture this script's output (e.g. smoke_test.sh) can parse it out. + SERVER_PORT=$(grep -E '^dev.port=' obp-api/src/main/resources/props/default.props 2>/dev/null | cut -d= -f2) echo "✓ HTTP4S server started in background" echo " PID: $SERVER_PID" + echo " PORT: ${SERVER_PORT:-8080}" echo " Log: $RUNTIME_LOG" echo "" echo "To stop the server: kill $SERVER_PID" diff --git a/flushall_fast_build_and_run.sh b/flushall_fast_build_and_run.sh index 88669ab6e9..61449c3c1f 100755 --- a/flushall_fast_build_and_run.sh +++ b/flushall_fast_build_and_run.sh @@ -324,8 +324,12 @@ if [ "$RUN_BACKGROUND" = true ]; then # Run in background with output to log file nohup java $JAVA_OPTS -jar obp-api/target/obp-api.jar > http4s-server.log 2>&1 & SERVER_PID=$! + # Report the port the server will actually bind (dev.port in props), so callers + # that capture this script's output (e.g. smoke_test.sh) can parse it out. + SERVER_PORT=$(grep -E '^dev.port=' obp-api/src/main/resources/props/default.props 2>/dev/null | cut -d= -f2) echo "✓ HTTP4S server started in background" echo " PID: $SERVER_PID" + echo " PORT: ${SERVER_PORT:-8080}" echo " Log: http4s-server.log" echo "" echo "To stop the server: kill $SERVER_PID" From 21e57e4aa8f9b61f0df94414f4764836954be30a Mon Sep 17 00:00:00 2001 From: Hongwei Date: Fri, 17 Jul 2026 10:50:32 +0200 Subject: [PATCH 06/21] fix: put bankId before accountId in createTransactionsJsonNew params Matches the codebase-wide convention of bank-scoped params before account-scoped ones. --- .../api/UKOpenBanking/v4_0_1/Http4sUKOBv401AccountInfo.scala | 2 +- .../UKOpenBanking/v4_0_1/JSONFactory_UKOpenBanking_401.scala | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/obp-api/src/main/scala/code/api/UKOpenBanking/v4_0_1/Http4sUKOBv401AccountInfo.scala b/obp-api/src/main/scala/code/api/UKOpenBanking/v4_0_1/Http4sUKOBv401AccountInfo.scala index 3380f6a84d..039b31485d 100644 --- a/obp-api/src/main/scala/code/api/UKOpenBanking/v4_0_1/Http4sUKOBv401AccountInfo.scala +++ b/obp-api/src/main/scala/code/api/UKOpenBanking/v4_0_1/Http4sUKOBv401AccountInfo.scala @@ -2236,7 +2236,7 @@ object Http4sUKOBv401AccountInfo extends MdcLoggable { transactions.map(_.id), view.viewId, Some(cc)) - } yield JSONFactory_UKOpenBanking_401.createTransactionsJsonNew(accountId.value, account.bankId, transactions, moderatedAttributes, view) + } yield JSONFactory_UKOpenBanking_401.createTransactionsJsonNew(account.bankId, accountId.value, transactions, moderatedAttributes, view) } } resourceDocs += ResourceDoc( diff --git a/obp-api/src/main/scala/code/api/UKOpenBanking/v4_0_1/JSONFactory_UKOpenBanking_401.scala b/obp-api/src/main/scala/code/api/UKOpenBanking/v4_0_1/JSONFactory_UKOpenBanking_401.scala index 97aad8f019..9be3cd5c42 100644 --- a/obp-api/src/main/scala/code/api/UKOpenBanking/v4_0_1/JSONFactory_UKOpenBanking_401.scala +++ b/obp-api/src/main/scala/code/api/UKOpenBanking/v4_0_1/JSONFactory_UKOpenBanking_401.scala @@ -293,8 +293,8 @@ object JSONFactory_UKOpenBanking_401 extends CustomJsonFormats { } def createTransactionsJsonNew( - accountId: String, bankId: BankId, + accountId: String, moderatedTransactions: List[ModeratedTransaction], attributes: List[TransactionAttribute], view: View From 94befc22df067445f7d76879552d923a01e57c87 Mon Sep 17 00:00:00 2001 From: Hongwei Date: Sat, 18 Jul 2026 08:06:45 +0200 Subject: [PATCH 07/21] fix: bind UK consent permissions to real accounts on authorisation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit createUKConsentJWT wrote every granted permission as ConsentView(bank_id=null, account_id=null, view_id=permission) at consent-creation time, before any account is known. That row can never match a real account (User.hasAccountAccess does plain bank_id/account_id equality, no wildcard), so a UK consent's declared Permissions had no effect on what could actually be read — access depended entirely on unrelated pre-existing AccountAccess grants. Add Consent.grantUKConsentAccountAccess, called from authoriseUKConsent once the PSU has selected accounts (via a new account_ids field on the authorise request body), mirroring how updateViewsOfBerlinGroupConsentJWT resolves Berlin Group's IBAN-keyed access into real per-account grants. Since UK consents are exercised via an opaque OAuth2 Bearer token rather than BG's per-request Consent-JWT header, the AccountAccess rows are granted eagerly at authorisation time instead of re-derived per request. Add a regression test: a consent scoped to ReadAccountsBasic grants that view but leaves ReadBalances locked. --- .../scala/code/api/util/ConsentUtil.scala | 61 +++++++++++++++++++ .../scala/code/api/v5_1_0/Http4s510.scala | 38 ++++++++---- .../UKOpenBankingV401AccountInfoTests.scala | 51 +++++++++++++++- 3 files changed, 138 insertions(+), 12 deletions(-) diff --git a/obp-api/src/main/scala/code/api/util/ConsentUtil.scala b/obp-api/src/main/scala/code/api/util/ConsentUtil.scala index d0ba73f44c..6573bf059d 100644 --- a/obp-api/src/main/scala/code/api/util/ConsentUtil.scala +++ b/obp-api/src/main/scala/code/api/util/ConsentUtil.scala @@ -1107,6 +1107,67 @@ object Consent extends MdcLoggable { CertificateUtil.jwtWithHmacProtection(jwtClaims, secret) } + /** + * Binds a UK Open Banking account-access consent to the PSU-selected accounts. + * + * createUKConsentJWT (called from POST /account-access-consents, before any account is known) + * writes every granted permission as ConsentView(bank_id=null, account_id=null, view_id=permission) + * — a row that can never match a real account (see User.hasAccountAccess: plain equality on + * bank_id/account_id, no wildcard). Call this once the PSU has selected which accounts the + * consent applies to (currently: the UK authorise step, since OBP has no separate + * ASPSP-hosted account-selection UI) to replace those dead rows with real per-account + * ConsentViews, and to eagerly grant the corresponding AccountAccess rows — mirroring how + * updateViewsOfBerlinGroupConsentJWT resolves BG's IBAN-keyed access into real accounts. + */ + def grantUKConsentAccountAccess(user: User, + bankId: BankId, + accountIds: List[String], + consent: MappedConsent, + callContext: Option[CallContext]): Future[Box[MappedConsent]] = { + implicit val dateFormats = CustomJsonFormats.formats + val payloadToUpdate: Box[ConsentJWT] = JwtUtil.getSignedPayloadAsJson(consent.jsonWebToken) + .map(com.openbankproject.commons.util.JsonAliases.parse(_).extract[ConsentJWT]) + + val permissions: List[String] = payloadToUpdate match { + case Full(consentJwt) => consentJwt.views.map(_.view_id).distinct + case _ => Nil + } + + val accountChecks: List[Future[Box[BankAccount]]] = accountIds.distinct.map { accountId => + Connector.connector.vend.checkBankAccountExists(bankId, AccountId(accountId), callContext).map(_._1) + } + + Future.sequence(accountChecks).map { boxes => + val error = s"$BankAccountNotFound BankId(${bankId.value})" + val validatedAccountIds: List[String] = boxes.map(_.openOrThrowException(error)).map(_.accountId.value) + + val newViews: List[ConsentView] = for { + accountId <- validatedAccountIds + permission <- permissions + } yield ConsentView(bank_id = bankId.value, account_id = accountId, view_id = permission, None) + + if (newViews.isEmpty) { + Empty + } else { + val updatedPayload = payloadToUpdate.map(_.copy(views = newViews)) + val jwtPayloadAsJson = compactRender(Extraction.decompose(updatedPayload)) + val jwtClaims: JWTClaimsSet = JWTClaimsSet.parse(jwtPayloadAsJson) + val jwt = CertificateUtil.jwtWithHmacProtection(jwtClaims, consent.secret) + // Eagerly grant real AccountAccess now: UK consents are exercised via an opaque OAuth2 + // Bearer token (checkUKConsent), not the Consent-JWT header BG/OBP consents use to + // lazily re-derive access on every call — so the grant has to happen once, here. + updatedPayload.foreach { consentJwt => + grantAccessToViews(user, consentJwt) match { + case Failure(msg, _, _) => + logger.warn(s"grantUKConsentAccountAccess: grantAccessToViews reported: $msg") + case _ => + } + } + Consents.consentProvider.vend.setJsonWebToken(consent.consentId, jwt) + } + } + } + private def checkConsumerIsActiveAndMatchedUK(consent: ConsentJWT, consumerIdOfLoggedInUser: Option[String]): Box[Boolean] = { Consumers.consumers.vend.getConsumerByConsumerId(consent.aud) match { case Full(consumerFromConsent) if consumerFromConsent.isActive.get == true => // Consumer is active diff --git a/obp-api/src/main/scala/code/api/v5_1_0/Http4s510.scala b/obp-api/src/main/scala/code/api/v5_1_0/Http4s510.scala index 1f172a3e95..60e21e337f 100644 --- a/obp-api/src/main/scala/code/api/v5_1_0/Http4s510.scala +++ b/obp-api/src/main/scala/code/api/v5_1_0/Http4s510.scala @@ -87,7 +87,9 @@ import scala.language.{higherKinds, implicitConversions} // UK Open Banking consent SCA (see authoriseUKConsentChallenge / authoriseUKConsent): // the challenge-start endpoint returns this; the authorise endpoint consumes the answer. case class UKConsentScaChallengeJsonV510(challenge_id: String, sca_status: String, sca_method: String) -case class PostUKConsentAuthoriseJsonV510(challenge_id: String, answer: String) +// account_ids: the accounts the PSU is selecting for this consent's granted permissions — +// see the Gap 4 remediation note above authoriseUKConsent (bankId comes from the URL BANK_ID). +case class PostUKConsentAuthoriseJsonV510(challenge_id: String, answer: String, account_ids: List[String]) object Http4s510 { @@ -4337,7 +4339,7 @@ object Http4s510 { // flip it to AUTHORISED — the missing "authorisation binding" step of the UK flow. // User-authenticated but role-free (the account holder is approving their own consent). val authoriseUKConsent: HttpRoutes[IO] = HttpRoutes.of[IO] { - case req @ POST -> `prefixPath` / "banks" / _ / "consents" / consentId / "authorise" => + case req @ POST -> `prefixPath` / "banks" / bankIdStr / "consents" / consentId / "authorise" => EndpointHelpers.executeFuture(req) { implicit val cc: code.api.util.CallContext = req.callContext for { @@ -4355,6 +4357,12 @@ object Http4s510 { authJson <- NewStyle.function.tryons(s"$InvalidJsonFormat The Json body should be the $PostUKConsentAuthoriseJsonV510 ", 400, Some(cc)) { com.openbankproject.commons.util.JsonAliases.parse(cc.httpBody.getOrElse("")).extract[PostUKConsentAuthoriseJsonV510] } + // The PSU must select at least one account for the consented permissions to bind to — + // see grantUKConsentAccountAccess (Gap 4 remediation: previously the consent's + // Permissions were never bound to a real account and had zero enforcement effect). + _ <- Helper.booleanToFuture(s"$InvalidJsonFormat The Json body should be the $PostUKConsentAuthoriseJsonV510 (account_ids must not be empty) ", 400, Some(cc)) { + authJson.account_ids.nonEmpty + } (_, _) <- NewStyle.function.getChallenge(authJson.challenge_id, Some(cc)) (challenge, _) <- NewStyle.function.validateChallengeAnswerC4( ChallengeType.OBP_CONSENT_CHALLENGE, @@ -4377,9 +4385,14 @@ object Http4s510 { // createdByUserId (via ConsentJWT.copy) — the UK permission views are preserved. updatedJwt <- Future(Consent.updateUserIdOfBerlinGroupConsentJWT(user.userId, consentAfterBind, Some(cc))) .map(i => connectorEmptyResponse(i, Some(cc))) - _ <- Future(Consents.consentProvider.vend.setJsonWebToken(consentId, updatedJwt)) + consentWithUser <- Future(Consents.consentProvider.vend.setJsonWebToken(consentId, updatedJwt)) + .map(i => connectorEmptyResponse(i, Some(cc))) + // Bind the consented permissions to the PSU-selected accounts, replacing the + // (bank_id=null, account_id=null) dead views createUKConsentJWT wrote at consent + // creation time, and eagerly grant the corresponding AccountAccess rows. + consentWithAccountAccess <- Consent.grantUKConsentAccountAccess(user, BankId(bankIdStr), authJson.account_ids, consentWithUser, Some(cc)) .map(i => connectorEmptyResponse(i, Some(cc))) - updatedConsent <- Future(Consents.consentProvider.vend.updateConsentStatus(consentId, ConsentStatus.AUTHORISED)) + updatedConsent <- Future(Consents.consentProvider.vend.updateConsentStatus(consentWithAccountAccess.consentId, ConsentStatus.AUTHORISED)) .map(i => connectorEmptyResponse(i, Some(cc))) } yield ConsentJsonV310(updatedConsent.consentId, updatedConsent.jsonWebToken, updatedConsent.status) } @@ -4395,22 +4408,25 @@ object Http4s510 { |Authorise a UK Open Banking account-access consent as the current (PSU) user, after SCA. | |The TPP first lodges the intent via `POST /account-access-consents`; the consent is - |created in ${ConsentStatus.AWAITINGAUTHORISATION} state with no bound user. The PSU then - |starts SCA via `POST .../authorise/challenge` and submits the resulting `challenge_id` - |plus the OTP `answer` here. On a valid answer this binds the consent to the PSU and - |transitions it to ${ConsentStatus.AUTHORISED}, so subsequent UK data calls whose access - |token carries the `consent_id` claim pass the consent check. + |created in ${ConsentStatus.AWAITINGAUTHORISATION} state with no bound user and no bound + |accounts. The PSU then starts SCA via `POST .../authorise/challenge` and submits the + |resulting `challenge_id` plus the OTP `answer`, together with the `account_ids` the PSU + |is selecting for this consent, here. On a valid answer this binds the consent to the PSU + |and to those accounts — every permission the consent declared is granted on each selected + |account — and transitions it to ${ConsentStatus.AUTHORISED}, so subsequent UK data calls + |whose access token carries the `consent_id` claim pass the consent check and are scoped to + |exactly the accounts and permissions the PSU approved. | |${userAuthenticationMessage(true)} | |""", - PostUKConsentAuthoriseJsonV510("74a8ebda-9e5a-4c3f-9b0b-1a2b3c4d5e6f", "123"), + PostUKConsentAuthoriseJsonV510("74a8ebda-9e5a-4c3f-9b0b-1a2b3c4d5e6f", "123", List("8ca8a7e4-6d05-4b21-a165-c02c39d77e55")), ConsentJsonV310( "9d429899-24f5-42c8-8565-943ffa6a7945", "eyJhbGciOiJIUzI1NiJ9.eyJ2aWV3cyI6W119.signature", "AUTHORISED" ), - List($AuthenticatedUserIsRequired, ConsentNotFound, ConsentStatusIssue, InvalidJsonFormat, InvalidChallengeAnswer, InvalidConnectorResponse, UnknownError), + List($AuthenticatedUserIsRequired, ConsentNotFound, ConsentStatusIssue, InvalidJsonFormat, InvalidChallengeAnswer, $BankAccountNotFound, InvalidConnectorResponse, UnknownError), apiTagConsent :: apiTagPSD2AIS :: Nil, None, http4sPartialFunction = Some(authoriseUKConsent) diff --git a/obp-api/src/test/scala/code/api/UKOpenBanking/v4_0_1/UKOpenBankingV401AccountInfoTests.scala b/obp-api/src/test/scala/code/api/UKOpenBanking/v4_0_1/UKOpenBankingV401AccountInfoTests.scala index 50ce214ca5..4f0a02844b 100644 --- a/obp-api/src/test/scala/code/api/UKOpenBanking/v4_0_1/UKOpenBankingV401AccountInfoTests.scala +++ b/obp-api/src/test/scala/code/api/UKOpenBanking/v4_0_1/UKOpenBankingV401AccountInfoTests.scala @@ -1,12 +1,19 @@ package code.api.UKOpenBanking.v4_0_1 +import code.api.Constant import code.api.util.APIUtil.DateWithDayFormat import code.api.util.ErrorMessages.ConsentIdClaimMissing +import code.api.util.Consent import code.consent.Consents -import com.openbankproject.commons.model.ErrorMessage +import code.model.UserExtended +import code.views.Views +import com.openbankproject.commons.model.{BankIdAccountId, ErrorMessage, ViewId} import org.json4s._ import org.scalatest.Tag +import scala.concurrent.Await +import scala.concurrent.duration._ + // Test suite for UK Open Banking Read/Write v4.0.1 (AccountInfo). // // The 9 endpoints wired to real connector data (see Http4sUKOBv401AccountInfo) @@ -109,6 +116,48 @@ class UKOpenBankingV401AccountInfoTests extends UKOpenBankingV401ServerSetup { deleteUnauthed("aisp", "account-access-consents", "fake-consentid").code should equal(401) } } + // ── Consent.grantUKConsentAccountAccess (Gap 4 fix) ─────────────────── + // Regression test for the previously-unverified scenario: before this fix, + // createUKConsentJWT wrote every permission as ConsentView(bank_id=null, + // account_id=null, view_id=permission) — a row that could never match a real + // account (see User.hasAccountAccess: plain bank_id/account_id equality, no + // wildcard) — so a UK consent's declared Permissions had zero effect on what + // could actually be read. This exercises the fix at the same access-check + // layer checkViewAccessAndReturnView (and therefore every UK data endpoint) + // relies on, since the full HTTP path requires a Bearer JWT with a consent_id + // claim that this OAuth1-signed test suite cannot mint (see the comment above + // "GET /aisp/accounts" below). + feature("UKOB v4.0.1 Consent.grantUKConsentAccountAccess binds permissions to the selected account only") { + scenario("consent scoped to ReadAccountsBasic grants that view but not ReadBalances", UKOpenBankingV401AccountInfo) { + val userExtended = UserExtended(resourceUser1) + val bankIdAccountId = BankIdAccountId(testBankId1, testAccountId1) + + // Baseline: ServerSetupWithTestData's default view grants don't include the UK read views. + userExtended.hasAccountAccess( + Views.views.vend.getOrCreateSystemView(Constant.SYSTEM_READ_ACCOUNTS_BASIC_VIEW_ID).openOrThrowException("view"), + bankIdAccountId, None) should equal(false) + + val consentId = createRealConsent() // permissions = List("ReadAccountsBasic") only + val consent = Consents.consentProvider.vend.getConsentByConsentId(consentId).openOrThrowException("consent") + + val result = Await.result( + Consent.grantUKConsentAccountAccess(resourceUser1, testBankId1, List(acc), consent, None), + 10.seconds) + result.isDefined should equal(true) + + // Granted: the account now has a real (non-null) AccountAccess row for the consented view. + userExtended.hasAccountAccess( + Views.views.vend.getOrCreateSystemView(Constant.SYSTEM_READ_ACCOUNTS_BASIC_VIEW_ID).openOrThrowException("view"), + bankIdAccountId, None) should equal(true) + + // Not granted: ReadBalances was never in the consent's Permissions, so it must stay locked — + // this is the check GET /aisp/accounts/ACCOUNT_ID/balances relies on (checkViewAccessAndReturnView). + userExtended.hasAccountAccess( + Views.views.vend.getOrCreateSystemView(Constant.SYSTEM_READ_BALANCES_VIEW_ID).openOrThrowException("view"), + bankIdAccountId, None) should equal(false) + } + } + // ── AccountsApi ──────────────────────────────────────────────────── // checkUKConsent extracts the `consent_id` claim from the Bearer access token (no external // Hydra call since Consent.checkUKConsent dropped the Hydra dependency). These OAuth1-signed From 2c138a2a8cc76b2480c457b949ef14ce7ac6c2ba Mon Sep 17 00:00:00 2001 From: Hongwei Date: Sat, 18 Jul 2026 08:23:01 +0200 Subject: [PATCH 08/21] fix: differentiate can_* permissions for UK and Berlin Group system views MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 6 UK Open Banking v4.0.1 system views (ReadAccountsBasic/Detail, ReadBalances, ReadTransactionsBasic/Debits/Detail) previously all shared the generic SYSTEM_VIEW_PERMISSION_COMMON set, so Detail granted nothing beyond Basic and a consent scoped to Balances alone still exposed full transaction and counterparty data. Berlin Group's ReadAccountsBerlinGroup and ReadBalancesBerlinGroup had zero ViewPermission rows at all (pure membership gating), unlike ReadTransactionsBerlinGroup which already has a full permission set. Add one can_* permission constant per view in constant.scala, mapped from the UK v4.0.1 spec's Detail Permissions table, and wire each into MapperViews.applyDefaultsForSystemView in place of the shared COMMON/untouched branches. accountant keeps SYSTEM_VIEW_PERMISSION_COMMON unchanged, split into its own case. Note for reviewers: existing deployments with these views already created at boot will have stale ViewPermission rows from the old COMMON set (or none, for the two BG views) — factoryResetSystemView needs to be re-run per view, or a migration added, to pick up the new defaults. --- .../scala/code/api/constant/constant.scala | 70 +++++++++++++++++++ .../main/scala/code/views/MapperViews.scala | 61 +++++++++++++--- 2 files changed, 122 insertions(+), 9 deletions(-) diff --git a/obp-api/src/main/scala/code/api/constant/constant.scala b/obp-api/src/main/scala/code/api/constant/constant.scala index db2143d8d0..5150eee4a7 100644 --- a/obp-api/src/main/scala/code/api/constant/constant.scala +++ b/obp-api/src/main/scala/code/api/constant/constant.scala @@ -632,6 +632,76 @@ object Constant extends MdcLoggable { CAN_ANSWER_TRANSACTION_REQUEST_CHALLENGE ) + // UK Open Banking v4.0.1 system views — previously all six shared the generic + // SYSTEM_VIEW_PERMISSION_COMMON set, so "Detail" granted nothing beyond "Basic" and a + // consent scoped to Balances alone still exposed full transaction/counterparty data. + // Mapped from the UK v4.0.1 spec's Detail Permissions table (see + // standards-permissions-research/uk-v401/account-and-transaction-api-profile.html). + final val SYSTEM_READ_ACCOUNTS_BASIC_VIEW_PERMISSION = List( + CAN_SEE_BANK_ACCOUNT_LABEL, + CAN_SEE_BANK_ACCOUNT_TYPE, + CAN_SEE_BANK_ACCOUNT_CURRENCY, + CAN_SEE_BANK_ACCOUNT_BANK_NAME + ) + + final val SYSTEM_READ_ACCOUNTS_DETAIL_VIEW_PERMISSION = SYSTEM_READ_ACCOUNTS_BASIC_VIEW_PERMISSION ++ List( + CAN_SEE_BANK_ACCOUNT_IBAN, + CAN_SEE_BANK_ACCOUNT_NUMBER, + CAN_SEE_BANK_ACCOUNT_SWIFT_BIC, + CAN_SEE_BANK_ACCOUNT_ROUTING_SCHEME, + CAN_SEE_BANK_ACCOUNT_ROUTING_ADDRESS + ) + + final val SYSTEM_READ_BALANCES_VIEW_PERMISSION = List( + CAN_SEE_BANK_ACCOUNT_BALANCE, + CAN_QUERY_AVAILABLE_FUNDS + ) + + final val SYSTEM_READ_TRANSACTIONS_BASIC_VIEW_PERMISSION = List( + CAN_SEE_TRANSACTION_THIS_BANK_ACCOUNT, + CAN_SEE_TRANSACTION_AMOUNT, + CAN_SEE_TRANSACTION_TYPE, + CAN_SEE_TRANSACTION_CURRENCY, + CAN_SEE_TRANSACTION_START_DATE, + CAN_SEE_TRANSACTION_FINISH_DATE, + CAN_SEE_TRANSACTION_STATUS + ) + + // ReadTransactionsDebits is direction-filtered at the query layer (see Gap 2 in the plan + // above) — it is not itself a wider or narrower view, so it shares Basic's field visibility. + final val SYSTEM_READ_TRANSACTIONS_DEBITS_VIEW_PERMISSION = SYSTEM_READ_TRANSACTIONS_BASIC_VIEW_PERMISSION + + final val SYSTEM_READ_TRANSACTIONS_DETAIL_VIEW_PERMISSION = SYSTEM_READ_TRANSACTIONS_BASIC_VIEW_PERMISSION ++ List( + CAN_SEE_TRANSACTION_OTHER_BANK_ACCOUNT, + CAN_SEE_OTHER_ACCOUNT_IBAN, + CAN_SEE_OTHER_ACCOUNT_BANK_NAME, + CAN_SEE_OTHER_ACCOUNT_NUMBER, + CAN_SEE_OTHER_ACCOUNT_KIND, + CAN_SEE_OTHER_ACCOUNT_SWIFT_BIC, + CAN_SEE_OTHER_ACCOUNT_NATIONAL_IDENTIFIER, + CAN_SEE_OTHER_ACCOUNT_ROUTING_SCHEME, + CAN_SEE_OTHER_ACCOUNT_ROUTING_ADDRESS, + CAN_SEE_OTHER_BANK_ROUTING_SCHEME, + CAN_SEE_OTHER_BANK_ROUTING_ADDRESS + ) + + // Berlin Group NextGenPSD2 system views — these two previously had zero ViewPermission rows + // at all (pure membership gating), unlike ReadTransactionsBerlinGroup which already has a + // full permission set above. BG's access object is IBAN-keyed, so unlike UK's Basic/Detail + // split, the account identifier (IBAN) is included by default here. + final val SYSTEM_READ_ACCOUNTS_BERLIN_GROUP_VIEW_PERMISSION = List( + CAN_SEE_BANK_ACCOUNT_LABEL, + CAN_SEE_BANK_ACCOUNT_TYPE, + CAN_SEE_BANK_ACCOUNT_CURRENCY, + CAN_SEE_BANK_ACCOUNT_BANK_NAME, + CAN_SEE_BANK_ACCOUNT_IBAN + ) + + final val SYSTEM_READ_BALANCES_BERLIN_GROUP_VIEW_PERMISSION = List( + CAN_SEE_BANK_ACCOUNT_BALANCE, + CAN_QUERY_AVAILABLE_FUNDS + ) + // Auditor system view: read-only on the account itself. The auditor can // see everything but cannot modify account data, counterparties, images, // locations, aliases, URLs, or initiate / approve payments. diff --git a/obp-api/src/main/scala/code/views/MapperViews.scala b/obp-api/src/main/scala/code/views/MapperViews.scala index 93812381e3..a8b5ded66b 100644 --- a/obp-api/src/main/scala/code/views/MapperViews.scala +++ b/obp-api/src/main/scala/code/views/MapperViews.scala @@ -782,8 +782,17 @@ object MapperViews extends Views with MdcLoggable { SYSTEM_VIEW_PERMISSION_COMMON ) entity.isFirehose_(true) - case SYSTEM_READ_ACCOUNTS_BERLIN_GROUP_VIEW_ID | - SYSTEM_READ_BALANCES_BERLIN_GROUP_VIEW_ID => + case SYSTEM_READ_ACCOUNTS_BERLIN_GROUP_VIEW_ID => + ViewPermission.resetViewPermissions( + entity, + SYSTEM_READ_ACCOUNTS_BERLIN_GROUP_VIEW_PERMISSION + ) + entity + case SYSTEM_READ_BALANCES_BERLIN_GROUP_VIEW_ID => + ViewPermission.resetViewPermissions( + entity, + SYSTEM_READ_BALANCES_BERLIN_GROUP_VIEW_PERMISSION + ) entity case SYSTEM_READ_TRANSACTIONS_BERLIN_GROUP_VIEW_ID => ViewPermission.resetViewPermissions( @@ -803,18 +812,52 @@ object MapperViews extends Views with MdcLoggable { SYSTEM_AUDITOR_VIEW_PERMISSION ) entity - case SYSTEM_ACCOUNTANT_VIEW_ID | - SYSTEM_READ_ACCOUNTS_BASIC_VIEW_ID | - SYSTEM_READ_ACCOUNTS_DETAIL_VIEW_ID | - SYSTEM_READ_BALANCES_VIEW_ID | - SYSTEM_READ_TRANSACTIONS_BASIC_VIEW_ID | - SYSTEM_READ_TRANSACTIONS_DEBITS_VIEW_ID | - SYSTEM_READ_TRANSACTIONS_DETAIL_VIEW_ID => + case SYSTEM_ACCOUNTANT_VIEW_ID => ViewPermission.resetViewPermissions( entity, SYSTEM_VIEW_PERMISSION_COMMON ) entity + // UK Open Banking v4.0.1 system views — each gets the can_* set matching its place in + // the spec's Basic/Detail permission hierarchy (see Constant.SYSTEM_READ_*_VIEW_PERMISSION + // definitions). Previously all six shared SYSTEM_VIEW_PERMISSION_COMMON, so "Detail" + // granted nothing beyond "Basic" and Balances alone exposed full transaction data. + case SYSTEM_READ_ACCOUNTS_BASIC_VIEW_ID => + ViewPermission.resetViewPermissions( + entity, + SYSTEM_READ_ACCOUNTS_BASIC_VIEW_PERMISSION + ) + entity + case SYSTEM_READ_ACCOUNTS_DETAIL_VIEW_ID => + ViewPermission.resetViewPermissions( + entity, + SYSTEM_READ_ACCOUNTS_DETAIL_VIEW_PERMISSION + ) + entity + case SYSTEM_READ_BALANCES_VIEW_ID => + ViewPermission.resetViewPermissions( + entity, + SYSTEM_READ_BALANCES_VIEW_PERMISSION + ) + entity + case SYSTEM_READ_TRANSACTIONS_BASIC_VIEW_ID => + ViewPermission.resetViewPermissions( + entity, + SYSTEM_READ_TRANSACTIONS_BASIC_VIEW_PERMISSION + ) + entity + case SYSTEM_READ_TRANSACTIONS_DEBITS_VIEW_ID => + ViewPermission.resetViewPermissions( + entity, + SYSTEM_READ_TRANSACTIONS_DEBITS_VIEW_PERMISSION + ) + entity + case SYSTEM_READ_TRANSACTIONS_DETAIL_VIEW_ID => + ViewPermission.resetViewPermissions( + entity, + SYSTEM_READ_TRANSACTIONS_DETAIL_VIEW_PERMISSION + ) + entity case _ => entity } From 2aa2478c5f874af7b17e07228a8b843c8a5830be Mon Sep 17 00:00:00 2001 From: Hongwei Date: Sat, 18 Jul 2026 09:06:33 +0200 Subject: [PATCH 09/21] feat: add ReadTransactionsCredits UK Open Banking system view MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The UK v4.0.1 Permissions enum requires ReadTransactionsCredits as an independently-selectable code alongside ReadTransactionsDebits, but only Debits (and Basic/Detail) existed as a view constant. Add the missing view id and can_* permission set (shared with ReadTransactionsDebits and ReadTransactionsBasic, since direction is a query-parameter-shaped concern, not a field-visibility one), wire it into MapperViews.applyDefaultsForSystemView and the additional_system_views boot whitelist, and create it unconditionally in the sandbox data importer alongside the other UK/BG views for consistency. Direction filtering itself (returning only credit- or debit-direction transactions per which view the consent granted) is not wired into the transactions endpoint yet — documented as a follow-up in constant.scala, since it also depends on fixing a separate, pre-existing bug where JSONFactory_UKOpenBanking_401 always reports CreditDebitIndicator as "Credit" regardless of the actual transaction. Add exact permission-set regression coverage for all 6 UK + 2 BG system views to MappedViewsTest. --- .../main/scala/bootstrap/liftweb/Boot.scala | 1 + .../scala/code/api/constant/constant.scala | 20 ++++++++++-- .../scala/code/sandbox/OBPDataImport.scala | 8 +++-- .../main/scala/code/views/MapperViews.scala | 6 ++++ .../scala/code/views/MappedViewsTest.scala | 32 +++++++++++++++++++ 5 files changed, 62 insertions(+), 5 deletions(-) diff --git a/obp-api/src/main/scala/bootstrap/liftweb/Boot.scala b/obp-api/src/main/scala/bootstrap/liftweb/Boot.scala index 54bcc7ae48..42013e4622 100644 --- a/obp-api/src/main/scala/bootstrap/liftweb/Boot.scala +++ b/obp-api/src/main/scala/bootstrap/liftweb/Boot.scala @@ -324,6 +324,7 @@ class Boot extends MdcLoggable { SYSTEM_READ_BALANCES_VIEW_ID, SYSTEM_READ_TRANSACTIONS_BASIC_VIEW_ID, SYSTEM_READ_TRANSACTIONS_DEBITS_VIEW_ID, + SYSTEM_READ_TRANSACTIONS_CREDITS_VIEW_ID, SYSTEM_READ_TRANSACTIONS_DETAIL_VIEW_ID, SYSTEM_READ_ACCOUNTS_BERLIN_GROUP_VIEW_ID, SYSTEM_READ_BALANCES_BERLIN_GROUP_VIEW_ID, diff --git a/obp-api/src/main/scala/code/api/constant/constant.scala b/obp-api/src/main/scala/code/api/constant/constant.scala index 5150eee4a7..16dd57d036 100644 --- a/obp-api/src/main/scala/code/api/constant/constant.scala +++ b/obp-api/src/main/scala/code/api/constant/constant.scala @@ -161,6 +161,7 @@ object Constant extends MdcLoggable { final val SYSTEM_READ_BALANCES_VIEW_ID = "ReadBalances" final val SYSTEM_READ_TRANSACTIONS_BASIC_VIEW_ID = "ReadTransactionsBasic" final val SYSTEM_READ_TRANSACTIONS_DEBITS_VIEW_ID = "ReadTransactionsDebits" + final val SYSTEM_READ_TRANSACTIONS_CREDITS_VIEW_ID = "ReadTransactionsCredits" final val SYSTEM_READ_TRANSACTIONS_DETAIL_VIEW_ID = "ReadTransactionsDetail" // Berlin Group final val SYSTEM_READ_ACCOUNTS_BERLIN_GROUP_VIEW_ID = "ReadAccountsBerlinGroup" @@ -182,6 +183,7 @@ object Constant extends MdcLoggable { SYSTEM_READ_BALANCES_VIEW_ID:: SYSTEM_READ_TRANSACTIONS_BASIC_VIEW_ID:: SYSTEM_READ_TRANSACTIONS_DEBITS_VIEW_ID:: + SYSTEM_READ_TRANSACTIONS_CREDITS_VIEW_ID:: SYSTEM_READ_TRANSACTIONS_DETAIL_VIEW_ID:: SYSTEM_READ_ACCOUNTS_BERLIN_GROUP_VIEW_ID:: SYSTEM_READ_BALANCES_BERLIN_GROUP_VIEW_ID:: @@ -667,9 +669,23 @@ object Constant extends MdcLoggable { CAN_SEE_TRANSACTION_STATUS ) - // ReadTransactionsDebits is direction-filtered at the query layer (see Gap 2 in the plan - // above) — it is not itself a wider or narrower view, so it shares Basic's field visibility. + // ReadTransactionsDebits / ReadTransactionsCredits (UK v4.0.1 spec: independently-selectable + // Permissions codes) are direction-filtered, not field-filtered — a PSU who only grants + // ReadTransactionsCredits should see the same transaction fields as ReadTransactionsBasic, + // just restricted to credit-direction rows. Neither view widens or narrows field visibility, + // so both share Basic's permission set here. + // + // Enforcement mechanism (decided, not yet wired into the transactions endpoint): filter by + // which of the two view_ids the consent granted, resolved the same way + // Http4sUKOBv401AccountInfo.getAccountsAccountIdTransactions already resolves Basic-or-Detail + // via ViewNewStyle.checkViewsAccessAndReturnView — no new can_* permission string, since + // direction is a query-parameter-shaped concern, not a field-visibility one. Wiring this in + // requires the endpoint to also filter the returned transaction list by amount sign, which in + // turn depends on fixing JSONFactory_UKOpenBanking_401.transactionJson's separate, + // pre-existing CreditDebitIndicator hardcoding (always "Credit", never derived from the + // transaction) — tracked as a follow-up, out of scope for this view/permission gap. final val SYSTEM_READ_TRANSACTIONS_DEBITS_VIEW_PERMISSION = SYSTEM_READ_TRANSACTIONS_BASIC_VIEW_PERMISSION + final val SYSTEM_READ_TRANSACTIONS_CREDITS_VIEW_PERMISSION = SYSTEM_READ_TRANSACTIONS_BASIC_VIEW_PERMISSION final val SYSTEM_READ_TRANSACTIONS_DETAIL_VIEW_PERMISSION = SYSTEM_READ_TRANSACTIONS_BASIC_VIEW_PERMISSION ++ List( CAN_SEE_TRANSACTION_OTHER_BANK_ACCOUNT, diff --git a/obp-api/src/main/scala/code/sandbox/OBPDataImport.scala b/obp-api/src/main/scala/code/sandbox/OBPDataImport.scala index 08d343d5eb..eee5069a61 100644 --- a/obp-api/src/main/scala/code/sandbox/OBPDataImport.scala +++ b/obp-api/src/main/scala/code/sandbox/OBPDataImport.scala @@ -409,6 +409,7 @@ trait OBPDataImport extends MdcLoggable { val readBalancesView = Views.views.vend.getOrCreateSystemView(SYSTEM_READ_BALANCES_VIEW_ID).asInstanceOf[Box[ViewType]] val readTransactionsBasicView = Views.views.vend.getOrCreateSystemView(SYSTEM_READ_TRANSACTIONS_BASIC_VIEW_ID).asInstanceOf[Box[ViewType]] val readTransactionsDebitsView = Views.views.vend.getOrCreateSystemView(SYSTEM_READ_TRANSACTIONS_DEBITS_VIEW_ID).asInstanceOf[Box[ViewType]] + val readTransactionsCreditsView = Views.views.vend.getOrCreateSystemView(SYSTEM_READ_TRANSACTIONS_CREDITS_VIEW_ID).asInstanceOf[Box[ViewType]] val readTransactionsDetailView = Views.views.vend.getOrCreateSystemView(SYSTEM_READ_TRANSACTIONS_DETAIL_VIEW_ID).asInstanceOf[Box[ViewType]] // Berlin Group val readAccountsBerlinGroupView = Views.views.vend.getOrCreateSystemView(SYSTEM_READ_ACCOUNTS_BERLIN_GROUP_VIEW_ID).asInstanceOf[Box[ViewType]] @@ -427,9 +428,10 @@ trait OBPDataImport extends MdcLoggable { readAccountsBasicView, readAccountsDetailView, readBalancesView, - readTransactionsBasicView, - readTransactionsDebitsView, - readTransactionsDetailView, + readTransactionsBasicView, + readTransactionsDebitsView, + readTransactionsCreditsView, + readTransactionsDetailView, readAccountsBerlinGroupView, readBalancesBerlinGroupView, readTransactionsBerlinGroupView, diff --git a/obp-api/src/main/scala/code/views/MapperViews.scala b/obp-api/src/main/scala/code/views/MapperViews.scala index a8b5ded66b..06d32c8eb4 100644 --- a/obp-api/src/main/scala/code/views/MapperViews.scala +++ b/obp-api/src/main/scala/code/views/MapperViews.scala @@ -852,6 +852,12 @@ object MapperViews extends Views with MdcLoggable { SYSTEM_READ_TRANSACTIONS_DEBITS_VIEW_PERMISSION ) entity + case SYSTEM_READ_TRANSACTIONS_CREDITS_VIEW_ID => + ViewPermission.resetViewPermissions( + entity, + SYSTEM_READ_TRANSACTIONS_CREDITS_VIEW_PERMISSION + ) + entity case SYSTEM_READ_TRANSACTIONS_DETAIL_VIEW_ID => ViewPermission.resetViewPermissions( entity, diff --git a/obp-api/src/test/scala/code/views/MappedViewsTest.scala b/obp-api/src/test/scala/code/views/MappedViewsTest.scala index 35f58da174..9ea83e2934 100644 --- a/obp-api/src/test/scala/code/views/MappedViewsTest.scala +++ b/obp-api/src/test/scala/code/views/MappedViewsTest.scala @@ -95,7 +95,39 @@ class MappedViewsTest extends ServerSetup with DefaultUsers{ MapperViews.factoryResetSystemView(ViewId("does-not-exist")) shouldBe Empty } + // Regression coverage for the UK Open Banking / Berlin Group views-permissions gap + // remediation (Gap 1, 2, 5): each of these views previously either shared the generic + // SYSTEM_VIEW_PERMISSION_COMMON set (so "Detail" granted nothing beyond "Basic") or had no + // ViewPermission rows at all (the two BG views). Assert each view's allowed_actions match + // its target set exactly — no more, no less. + scenario("UK and Berlin Group system views have exact, differentiated can_* permission sets") { + // UK/BG views are opt-in (created on demand), not unconditionally present like auditor — + // getOrCreateSystemView creates them fresh with current code defaults; afterEach's + // ViewDefinition.bulkDelete_!! guarantees no stale permissions leak in between scenarios. + def actionsOf(viewId: String): Set[String] = + MapperViews.getOrCreateSystemView(viewId) + .openOrThrowException(s"$viewId should be a known system view") + .allowed_actions.toSet + actionsOf(Constant.SYSTEM_READ_ACCOUNTS_BASIC_VIEW_ID) should equal(Constant.SYSTEM_READ_ACCOUNTS_BASIC_VIEW_PERMISSION.toSet) + actionsOf(Constant.SYSTEM_READ_ACCOUNTS_DETAIL_VIEW_ID) should equal(Constant.SYSTEM_READ_ACCOUNTS_DETAIL_VIEW_PERMISSION.toSet) + actionsOf(Constant.SYSTEM_READ_BALANCES_VIEW_ID) should equal(Constant.SYSTEM_READ_BALANCES_VIEW_PERMISSION.toSet) + actionsOf(Constant.SYSTEM_READ_TRANSACTIONS_BASIC_VIEW_ID) should equal(Constant.SYSTEM_READ_TRANSACTIONS_BASIC_VIEW_PERMISSION.toSet) + actionsOf(Constant.SYSTEM_READ_TRANSACTIONS_DEBITS_VIEW_ID) should equal(Constant.SYSTEM_READ_TRANSACTIONS_DEBITS_VIEW_PERMISSION.toSet) + actionsOf(Constant.SYSTEM_READ_TRANSACTIONS_CREDITS_VIEW_ID) should equal(Constant.SYSTEM_READ_TRANSACTIONS_CREDITS_VIEW_PERMISSION.toSet) + actionsOf(Constant.SYSTEM_READ_TRANSACTIONS_DETAIL_VIEW_ID) should equal(Constant.SYSTEM_READ_TRANSACTIONS_DETAIL_VIEW_PERMISSION.toSet) + actionsOf(Constant.SYSTEM_READ_ACCOUNTS_BERLIN_GROUP_VIEW_ID) should equal(Constant.SYSTEM_READ_ACCOUNTS_BERLIN_GROUP_VIEW_PERMISSION.toSet) + actionsOf(Constant.SYSTEM_READ_BALANCES_BERLIN_GROUP_VIEW_ID) should equal(Constant.SYSTEM_READ_BALANCES_BERLIN_GROUP_VIEW_PERMISSION.toSet) + + Then("Detail must be a strict superset of Basic (never narrower), for both Accounts and Transactions") + Constant.SYSTEM_READ_ACCOUNTS_DETAIL_VIEW_PERMISSION.toSet should contain allElementsOf Constant.SYSTEM_READ_ACCOUNTS_BASIC_VIEW_PERMISSION + Constant.SYSTEM_READ_ACCOUNTS_DETAIL_VIEW_PERMISSION.toSet.size should be > Constant.SYSTEM_READ_ACCOUNTS_BASIC_VIEW_PERMISSION.toSet.size + Constant.SYSTEM_READ_TRANSACTIONS_DETAIL_VIEW_PERMISSION.toSet should contain allElementsOf Constant.SYSTEM_READ_TRANSACTIONS_BASIC_VIEW_PERMISSION + Constant.SYSTEM_READ_TRANSACTIONS_DETAIL_VIEW_PERMISSION.toSet.size should be > Constant.SYSTEM_READ_TRANSACTIONS_BASIC_VIEW_PERMISSION.toSet.size + + Then("Balances must not carry transaction- or counterparty-visibility permissions") + actionsOf(Constant.SYSTEM_READ_BALANCES_VIEW_ID) should equal(Set(Constant.CAN_SEE_BANK_ACCOUNT_BALANCE, Constant.CAN_QUERY_AVAILABLE_FUNDS)) + } } From 961addbb0c7392fa10f3f102b26ac73d6b980127 Mon Sep 17 00:00:00 2001 From: Hongwei Date: Sat, 18 Jul 2026 09:23:32 +0200 Subject: [PATCH 10/21] docs: fix stale additional_system_views value list in props template The sample.props.template's documented list of valid additional_system_views values had drifted from Boot.scala's actual whitelist: it was missing ReadBalancesBerlinGroup and ReadTransactionsBerlinGroup (already live for a while) and the newly-added ReadTransactionsCredits. --- obp-api/src/main/resources/props/sample.props.template | 3 +++ 1 file changed, 3 insertions(+) diff --git a/obp-api/src/main/resources/props/sample.props.template b/obp-api/src/main/resources/props/sample.props.template index 12356d2cb6..37629af6d9 100644 --- a/obp-api/src/main/resources/props/sample.props.template +++ b/obp-api/src/main/resources/props/sample.props.template @@ -1275,8 +1275,11 @@ database_messages_scheduler_interval=3600 ReadBalances,\ ReadTransactionsBasic,\ ReadTransactionsDebits,\ + ReadTransactionsCredits,\ ReadTransactionsDetail, \ ReadAccountsBerlinGroup, \ + ReadBalancesBerlinGroup, \ + ReadTransactionsBerlinGroup, \ InitiatePaymentsBerlinGroup # ----------------------------------------------------------------------------- From fdb91f72ec73e4b929d2b382af7036a12c2e5d92 Mon Sep 17 00:00:00 2001 From: Hongwei Date: Sat, 18 Jul 2026 09:29:50 +0200 Subject: [PATCH 11/21] docs: document the consent-layer vs view-layer permission boundary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Not a bug fix — a design guard-rail. Berlin Group's frequencyPerDay, recurringIndicator, and validUntil, and UK's TransactionFromDateTime, ToDateTime, and ExpirationDateTime already live on the consent record (ConsentJWT), never as a view can_* permission. As the UK/BG system view permission sets get filled in, it's easy to reach for a can_* string to express a consent's time-boxing or access-frequency limit (e.g. a hypothetical can_see_transactions_last_90_days) — that conflates the two layers. Add a note at both ends (ConsentJWT and the SYSTEM_READ_*_VIEW_PERMISSION definitions) so future changes don't cross this boundary. --- obp-api/src/main/scala/code/api/constant/constant.scala | 7 +++++++ obp-api/src/main/scala/code/api/util/ConsentUtil.scala | 8 ++++++++ 2 files changed, 15 insertions(+) diff --git a/obp-api/src/main/scala/code/api/constant/constant.scala b/obp-api/src/main/scala/code/api/constant/constant.scala index 16dd57d036..eddc6ecb04 100644 --- a/obp-api/src/main/scala/code/api/constant/constant.scala +++ b/obp-api/src/main/scala/code/api/constant/constant.scala @@ -634,6 +634,13 @@ object Constant extends MdcLoggable { CAN_ANSWER_TRANSACTION_REQUEST_CHALLENGE ) + // Design boundary for every SYSTEM_READ_*_VIEW_PERMISSION set below: these express what + // *fields* a view exposes, never a consent's time-boxing or access-frequency limit. BG's + // frequencyPerDay/recurringIndicator/validUntil and UK's TransactionFromDateTime/ToDateTime/ + // ExpirationDateTime belong on the consent record (see ConsentJWT in ConsentUtil.scala), not + // here — do not add a can_* string like "can_see_transactions_last_90_days" to narrow a view + // by date range or usage count; that's the consent's job, applied before the view is reached. + // UK Open Banking v4.0.1 system views — previously all six shared the generic // SYSTEM_VIEW_PERMISSION_COMMON set, so "Detail" granted nothing beyond "Basic" and a // consent scoped to Balances alone still exposed full transaction/counterparty data. diff --git a/obp-api/src/main/scala/code/api/util/ConsentUtil.scala b/obp-api/src/main/scala/code/api/util/ConsentUtil.scala index 6573bf059d..94ff5ef545 100644 --- a/obp-api/src/main/scala/code/api/util/ConsentUtil.scala +++ b/obp-api/src/main/scala/code/api/util/ConsentUtil.scala @@ -40,6 +40,14 @@ import java.util.Date import scala.collection.immutable.{List, Nil} import scala.concurrent.Future +// Design boundary (not enforced by the compiler — keep it that way by convention): consent-layer +// attributes belong on the consent record, never as a View can_* permission. BG's +// frequencyPerDay/recurringIndicator/validUntil and UK's TransactionFromDateTime/ToDateTime/ +// ExpirationDateTime (exp/nbf above) already live here for that reason. Do not add a can_* string +// like "can_see_transactions_last_90_days" to a system view's permission set to express a +// consent's time-boxing or access-frequency limit — that's a property of *this* consent, not of +// the account's view. See MapperViews.applyDefaultsForSystemView / Constant.SYSTEM_READ_*_VIEW_PERMISSION +// for where view-layer can_* sets are defined. case class ConsentJWT(createdByUserId: String, sub: String, // An identifier for the user, unique among all OBP-API users and never reused iss: String, // The Issuer Identifier for the Issuer of the response. From c0fe7f16373dcada5addeadc263ee2958cff564e Mon Sep 17 00:00:00 2001 From: Hongwei Date: Sat, 18 Jul 2026 09:44:29 +0200 Subject: [PATCH 12/21] fix(berlin-group): unblock consent SCA authorisation flow GET /obp/v5.1.0/user/current/consents/CONSENT_ID hard-required an exact mUserId match, so a PSU could never view a Berlin Group consent before completing SCA (BG consents are created via client_credentials with no owner yet). Relax the check to also allow consents that have no owner assigned. Also fix createStartConsentAuthorisationJson returning authenticationMethodId as the authorisationId instead of challengeId -- the PUT endpoint resolves the authorisation by challengeId, so any caller following the documented response field got a 400 on the confirmation step. --- .../api/berlin/group/v1_3/JSONFactory_BERLIN_GROUP_1_3.scala | 2 +- obp-api/src/main/scala/code/api/v5_1_0/Http4s510.scala | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/obp-api/src/main/scala/code/api/berlin/group/v1_3/JSONFactory_BERLIN_GROUP_1_3.scala b/obp-api/src/main/scala/code/api/berlin/group/v1_3/JSONFactory_BERLIN_GROUP_1_3.scala index 3a36351c17..709b14a4ed 100644 --- a/obp-api/src/main/scala/code/api/berlin/group/v1_3/JSONFactory_BERLIN_GROUP_1_3.scala +++ b/obp-api/src/main/scala/code/api/berlin/group/v1_3/JSONFactory_BERLIN_GROUP_1_3.scala @@ -724,7 +724,7 @@ object JSONFactory_BERLIN_GROUP_1_3 extends CustomJsonFormats with MdcLoggable{ def createStartConsentAuthorisationJson(consent: ConsentTrait, challenge: ChallengeTrait) : StartConsentAuthorisationJson = { StartConsentAuthorisationJson( scaStatus = challenge.scaStatus.map(_.toString).getOrElse("None"), - authorisationId = challenge.authenticationMethodId.getOrElse("None"), + authorisationId = challenge.challengeId, pushMessage = "started", //TODO Not implement how to fill this. _links = ScaStatusJsonV13(s"/${ConstantsBG.berlinGroupVersion1.apiShortVersion}/consents/${consent.consentId}/authorisations/${challenge.challengeId}")//TODO, Not sure, what is this for?? ) diff --git a/obp-api/src/main/scala/code/api/v5_1_0/Http4s510.scala b/obp-api/src/main/scala/code/api/v5_1_0/Http4s510.scala index 60e21e337f..e2affd0100 100644 --- a/obp-api/src/main/scala/code/api/v5_1_0/Http4s510.scala +++ b/obp-api/src/main/scala/code/api/v5_1_0/Http4s510.scala @@ -4594,7 +4594,7 @@ object Http4s510 { consent <- Future(Consents.consentProvider.vend.getConsentByConsentId(consentId)) .map(unboxFullOrFail(_, Some(cc), ConsentNotFound, 404)) _ <- Helper.booleanToFuture(failMsg = ConsentNotFound, failCode = 404, cc = Some(cc)) { - consent.mUserId == cc.userId + consent.mUserId == cc.userId || Option(consent.userId).forall(_.isBlank) } } yield JSONFactory510.getConsentInfoJson(consent) } From e20136576679a943b0bc3ae4774570f099c9e37c Mon Sep 17 00:00:00 2001 From: Hongwei Date: Sat, 18 Jul 2026 09:44:51 +0200 Subject: [PATCH 13/21] test(berlin-group): cover the unclaimed-consent SCA happy path end to end Create a PSU-less Berlin Group consent directly via the provider (mirroring how POST /consents builds one for a client_credentials caller), then exercise the full authorisation flow: another logged-in user can view it before SCA, starting the authorisation returns a resolvable authorisationId, and submitting the correct OTP claims the consent for the answering PSU. A second scenario documents that a wrong OTP is rejected with 400 and leaves the consent unclaimed. --- .../AccountInformationServiceAISApiTest.scala | 123 +++++++++++++++++- 1 file changed, 121 insertions(+), 2 deletions(-) diff --git a/obp-api/src/test/scala/code/api/berlin/group/v1_3/AccountInformationServiceAISApiTest.scala b/obp-api/src/test/scala/code/api/berlin/group/v1_3/AccountInformationServiceAISApiTest.scala index f9d5a6b697..140999ffc1 100644 --- a/obp-api/src/test/scala/code/api/berlin/group/v1_3/AccountInformationServiceAISApiTest.scala +++ b/obp-api/src/test/scala/code/api/berlin/group/v1_3/AccountInformationServiceAISApiTest.scala @@ -3,13 +3,16 @@ package code.api.berlin.group.v1_3 import org.json4s._ import code.api.Constant import code.api.Constant.{SYSTEM_READ_ACCOUNTS_BERLIN_GROUP_VIEW_ID, SYSTEM_READ_BALANCES_BERLIN_GROUP_VIEW_ID, SYSTEM_READ_TRANSACTIONS_BERLIN_GROUP_VIEW_ID} +import code.api.berlin.group.ConstantsBG import code.api.berlin.group.v1_3.JSONFactory_BERLIN_GROUP_1_3._ import code.api.berlin.group.v1_3.Http4sBGv13AIS import code.api.util.APIUtil import code.api.util.APIUtil.OAuth._ +import code.api.berlin.group.v1_3.model.ScaStatusResponse +import code.api.util.Consent import code.api.util.ErrorMessages._ import code.api.v4_0_0.PostViewJsonV400 -import code.consent.ConsentStatus +import code.consent.{ConsentStatus, ConsentTrait, Consents} import code.model.dataAccess.BankAccountRouting import code.setup.{APIResponse, DefaultUsers} import com.github.dwickern.macros.NameOf.nameOf @@ -21,6 +24,8 @@ import org.scalatest.Tag import java.time.LocalDate import java.time.format.DateTimeFormatter +import scala.concurrent.Await +import scala.concurrent.duration._ class AccountInformationServiceAISApiTest extends BerlinGroupServerSetupV1_3 with DefaultUsers { @@ -762,6 +767,120 @@ class AccountInformationServiceAISApiTest extends BerlinGroupServerSetupV1_3 wit val responseStartConsentAuthorisation = makePutRequest(requestStartConsentAuthorisation, """{"confirmationCode":"confirmationCode"}""") responseStartConsentAuthorisation.code should be (200) } - } + } + + // Builds an unclaimed (PSU-less) Berlin Group consent directly via the provider, mirroring + // how POST /consents builds one for a client_credentials caller (createdByUser = None). + def createUnclaimedBerlinGroupConsent(): ConsentTrait = { + val accountsRoutingIban = BankAccountRouting.findAll(By(BankAccountRouting.AccountRoutingScheme, AccountRoutingScheme.IBAN.toString)) + val acountRoutingIban = accountsRoutingIban.head + val postJsonBody = PostConsentJson( + access = ConsentAccessJson( + accounts = Option(List(ConsentAccessAccountsJson( + iban = Some(acountRoutingIban.accountRouting.address), + bban = None, + pan = None, + maskedPan = None, + msisdn = None, + currency = None, + ))), + balances = None, + transactions = None, + availableAccounts = None, + allPsd2 = None + ), + recurringIndicator = true, + validUntil = getNextMonthDate(), + frequencyPerDay = 4, + combinedServiceIndicator = Some(false) + ) + val validUntilDate = BgSpecValidation.getDate(postJsonBody.validUntil) + + val createdConsent = Consents.consentProvider.vend.createBerlinGroupConsent( + user = None, + consumer = Some(testConsumer), + recurringIndicator = postJsonBody.recurringIndicator, + validUntil = validUntilDate, + frequencyPerDay = postJsonBody.frequencyPerDay, + combinedServiceIndicator = postJsonBody.combinedServiceIndicator.getOrElse(false), + apiStandard = Some(ConstantsBG.berlinGroupVersion1.apiStandard), + apiVersion = Some(ConstantsBG.berlinGroupVersion1.apiShortVersion) + ).openOrThrowException("test consent creation failed") + + val consentJWT = Await.result( + Consent.createBerlinGroupConsentJWT( + None, + postJsonBody, + createdConsent.secret, + createdConsent.consentId, + Some(testConsumer.consumerId.get), + Some(validUntilDate), + None + ), + 10.seconds + ).openOrThrowException("test consent JWT creation failed") + Consents.consentProvider.vend.setJsonWebToken(createdConsent.consentId, consentJWT) + + createdConsent + } + + feature(s"BG v1.3 - unclaimed consent SCA (regression: GET /obp/v5.1.0/user/current/consents/CONSENT_ID 404 before SCA, wrong authorisationId from ${startConsentAuthorisationTransactionAuthorisation.name})") { + scenario("Unclaimed consent: viewable pre-SCA by any user, authorisable, and claimed by the answering PSU on correct OTP", BerlinGroupV1_3, startConsentAuthorisationTransactionAuthorisation, updateConsentsPsuDataTransactionAuthorisation) { + setPropsValues("suggested_default_sca_method" -> "DUMMY") + + val createdConsent = createUnclaimedBerlinGroupConsent() + Option(createdConsent.userId).forall(_.isBlank) should be (true) + val consentId = createdConsent.consentId + + Then("A different logged-in user can GET the unclaimed consent — this used to 404 (Bug A)") + val requestGetConsent = (baseRequest / "obp" / "v5.1.0" / "user" / "current" / "consents" / consentId).GET <@ (user2) + val responseGetConsent = makeGetRequest(requestGetConsent) + responseGetConsent.code should be (200) + + Then(s"We test the $startConsentAuthorisationTransactionAuthorisation") + val requestStartConsentAuthorisation = (V1_3_BG / "consents" / consentId / "authorisations").POST <@ (user1) + val responseStartConsentAuthorisation = makePostRequest(requestStartConsentAuthorisation, """{"scaAuthenticationData":""}""") + responseStartConsentAuthorisation.code should be (201) + val authorisationId = responseStartConsentAuthorisation.body.extract[StartConsentAuthorisationJson].authorisationId + + Then("The returned authorisationId must resolve on GET — this used to be the wrong field (Bug C)") + val requestGetConsentScaStatus = (V1_3_BG / "consents" / consentId / "authorisations" / authorisationId).GET <@ (user1) + val responseGetConsentScaStatus = makeGetRequest(requestGetConsentScaStatus) + responseGetConsentScaStatus.code should be (200) + + Then(s"We submit the correct OTP to $updateConsentsPsuDataTransactionAuthorisation and the consent becomes valid, owned by the answering PSU") + val requestUpdatePsuData = (V1_3_BG / "consents" / consentId / "authorisations" / authorisationId).PUT <@ (user1) + val responseUpdatePsuData = makePutRequest(requestUpdatePsuData, """{"scaAuthenticationData":"123"}""") + responseUpdatePsuData.code should be (200) + responseUpdatePsuData.body.extract[ScaStatusResponse].scaStatus should be ("valid") + + val updatedConsent = Consents.consentProvider.vend.getConsentByConsentId(consentId).openOrThrowException("test consent lookup failed") + updatedConsent.userId should be (resourceUser1.userId) + updatedConsent.status should be (ConsentStatus.valid.toString) + } + + scenario("Unclaimed consent: an incorrect OTP is rejected with 400 and the consent stays unclaimed (documents that updateConsentUser in updateConsentsPsuDataAll is never reached on a failed challenge answer, unrelated to this fix)", BerlinGroupV1_3, updateConsentsPsuDataTransactionAuthorisation) { + setPropsValues("suggested_default_sca_method" -> "DUMMY") + + val createdConsent = createUnclaimedBerlinGroupConsent() + val consentId = createdConsent.consentId + + val requestStartConsentAuthorisation = (V1_3_BG / "consents" / consentId / "authorisations").POST <@ (user1) + val responseStartConsentAuthorisation = makePostRequest(requestStartConsentAuthorisation, """{"scaAuthenticationData":""}""") + responseStartConsentAuthorisation.code should be (201) + val authorisationId = responseStartConsentAuthorisation.body.extract[StartConsentAuthorisationJson].authorisationId + + Then("We submit a wrong OTP") + val requestUpdatePsuData = (V1_3_BG / "consents" / consentId / "authorisations" / authorisationId).PUT <@ (user1) + val responseUpdatePsuData = makePutRequest(requestUpdatePsuData, """{"scaAuthenticationData":"wrong-otp"}""") + responseUpdatePsuData.code should be (400) + responseUpdatePsuData.body.extract[ErrorMessagesBG].tppMessages.head.text should include ("OBP-40016") + + Then("The consent is not claimed — validateChallengeAnswerC4 fails the Box before updateConsentUser runs") + val updatedConsent = Consents.consentProvider.vend.getConsentByConsentId(consentId).openOrThrowException("test consent lookup failed") + Option(updatedConsent.userId).forall(_.isBlank) should be (true) + updatedConsent.status should be (ConsentStatus.received.toString) + } + } } \ No newline at end of file From c653f60f1a1e59252d4cafba4b10014f265aacfb Mon Sep 17 00:00:00 2001 From: Hongwei Date: Sat, 18 Jul 2026 13:26:49 +0200 Subject: [PATCH 14/21] fix: return spec four-letter status codes on UK v4.0.1 consent responses UK v4.0.1 requires AWAU/AUTH/RJCT/CANC/EXPD on the wire, but OBP was returning the internal long enum names (AWAITINGAUTHORISATION, AUTHORISED, REVOKED) verbatim -- disagreeing with the endpoint's own documented example response, which already showed AWAU. Add a status-code mapping at the JSON serialization boundary only; internal storage keeps the long names, matching how BG and OBP standard consents already work. REVOKED maps to CANC since OBP does not otherwise distinguish PSU-dashboard-cancel from AISP-DELETE-revoke and the spec has no other status for that distinction. Also add the StatusReason array required by OBReadConsentResponse1, which was previously present only in the hardcoded ResourceDoc example, never actually populated in real responses. This is a breaking wire-format change for any caller currently depending on the long status-name strings. --- .../JSONFactory_UKOpenBanking_401.scala | 34 ++++++++++++++++++- .../UKOpenBankingV401AccountInfoTests.scala | 8 ++++- 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/obp-api/src/main/scala/code/api/UKOpenBanking/v4_0_1/JSONFactory_UKOpenBanking_401.scala b/obp-api/src/main/scala/code/api/UKOpenBanking/v4_0_1/JSONFactory_UKOpenBanking_401.scala index 9be3cd5c42..950cac825a 100644 --- a/obp-api/src/main/scala/code/api/UKOpenBanking/v4_0_1/JSONFactory_UKOpenBanking_401.scala +++ b/obp-api/src/main/scala/code/api/UKOpenBanking/v4_0_1/JSONFactory_UKOpenBanking_401.scala @@ -132,10 +132,15 @@ object JSONFactory_UKOpenBanking_401 extends CustomJsonFormats { // --------------------------------------------------------------------- // Consent // --------------------------------------------------------------------- + case class StatusReasonV401( + StatusReasonCode: String, + StatusReasonDescription: String + ) case class ConsentDataV401( ConsentId: String, CreationDateTime: String, Status: String, + StatusReason: List[StatusReasonV401], StatusUpdateDateTime: String, Permissions: List[String], ExpirationDateTime: String, @@ -319,6 +324,31 @@ object JSONFactory_UKOpenBanking_401 extends CustomJsonFormats { ) } + // v4.0.1 uses ISO 20022-style four-letter status codes on the wire (AWAU/AUTH/RJCT/CANC/EXPD), + // whereas OBP stores the long enum names (AWAITINGAUTHORISATION/AUTHORISED/REJECTED/REVOKED/EXPIRED). + // Map at the serialization boundary only — storage keeps the long names. REVOKED maps to CANC + // because the spec's revoke-side status is CANC and OBP does not distinguish dashboard-cancel from + // AISP-DELETE-revoke. Unknown/other statuses pass through unchanged so nothing is silently hidden. + def ukConsentStatusCode(status: String): String = status.toUpperCase match { + case "AWAITINGAUTHORISATION" => "AWAU" + case "AUTHORISED" => "AUTH" + case "REJECTED" => "RJCT" + case "REVOKED" => "CANC" + case "EXPIRED" => "EXPD" + case _ => status + } + + // Minimal StatusReason per the current status, mirroring the spec's own example wording/codes + // (OBReadConsentResponse1). Richer per-transition reasons can follow later. + private def ukConsentStatusReason(fourLetterCode: String): List[StatusReasonV401] = fourLetterCode match { + case "AWAU" => List(StatusReasonV401("U036", "Waiting for completion of consent authorisation to be completed by user")) + case "AUTH" => List(StatusReasonV401("U110", "The account access consent has been successfully authorised")) + case "RJCT" => List(StatusReasonV401("U111", "The account access consent has been rejected")) + case "CANC" => List(StatusReasonV401("U112", "The account access consent has been cancelled")) + case "EXPD" => List(StatusReasonV401("U113", "The account access consent has passed its expiry date")) + case _ => Nil + } + def createConsentResponseJSON( consentId: String, creationDateTime: String, @@ -330,11 +360,13 @@ object JSONFactory_UKOpenBanking_401 extends CustomJsonFormats { transactionToDateTime: String, selfPath: String ): ConsentResponseV401 = { + val statusCode = ukConsentStatusCode(status) ConsentResponseV401( Data = ConsentDataV401( ConsentId = consentId, CreationDateTime = creationDateTime, - Status = status, + Status = statusCode, + StatusReason = ukConsentStatusReason(statusCode), StatusUpdateDateTime = statusUpdateDateTime, Permissions = permissions, ExpirationDateTime = expirationDateTime, diff --git a/obp-api/src/test/scala/code/api/UKOpenBanking/v4_0_1/UKOpenBankingV401AccountInfoTests.scala b/obp-api/src/test/scala/code/api/UKOpenBanking/v4_0_1/UKOpenBankingV401AccountInfoTests.scala index 4f0a02844b..5ab9997c38 100644 --- a/obp-api/src/test/scala/code/api/UKOpenBanking/v4_0_1/UKOpenBankingV401AccountInfoTests.scala +++ b/obp-api/src/test/scala/code/api/UKOpenBanking/v4_0_1/UKOpenBankingV401AccountInfoTests.scala @@ -77,6 +77,9 @@ class UKOpenBankingV401AccountInfoTests extends UKOpenBankingV401ServerSetup { consentId should not be empty Consents.consentProvider.vend.getConsentByConsentId(consentId).isDefined should equal(true) (response.body \ "Data" \ "Permissions").extract[List[String]] should equal(consentPermissions) + // v4.0.1 four-letter status codes on the wire (not the stored long name) + StatusReason. + (response.body \ "Data" \ "Status").extract[String] should equal("AWAU") + (response.body \ "Data" \ "StatusReason" \ "StatusReasonCode").extract[List[String]] should equal(List("U036")) } scenario("unauthenticated -> 401", UKOpenBankingV401AccountInfo) { postUnauthed(consentPostBody, "aisp", "account-access-consents").code should equal(401) @@ -89,6 +92,8 @@ class UKOpenBankingV401AccountInfoTests extends UKOpenBankingV401ServerSetup { response.code should equal(200) (response.body \ "Data" \ "ConsentId").extract[String] should equal(consentId) (response.body \ "Data" \ "Permissions").extract[List[String]] should equal(consentPermissions) + // freshly-created consent is AWAITINGAUTHORISATION → wire code AWAU + (response.body \ "Data" \ "Status").extract[String] should equal("AWAU") } scenario("authenticated with unknown consent -> 400", UKOpenBankingV401AccountInfo) { getAuthed("aisp", "account-access-consents", "fake-consentid").code should equal(400) @@ -107,7 +112,8 @@ class UKOpenBankingV401AccountInfoTests extends UKOpenBankingV401ServerSetup { val afterDelete = getAuthed("aisp", "account-access-consents", consentId) afterDelete.code should equal(200) - (afterDelete.body \ "Data" \ "Status").extract[String] should equal("REVOKED") + // stored status is REVOKED, but the v4.0.1 wire format reports the spec's CANC code + (afterDelete.body \ "Data" \ "Status").extract[String] should equal("CANC") } scenario("authenticated with unknown consent -> 400", UKOpenBankingV401AccountInfo) { deleteAuthed("aisp", "account-access-consents", "fake-consentid").code should equal(400) From 39b9825644e1a7c861992d39761583b3a01e485b Mon Sep 17 00:00:00 2001 From: Hongwei Date: Sat, 18 Jul 2026 14:07:43 +0200 Subject: [PATCH 15/21] fix: make UK consent datetime fields optional, fix ISO-8601 parsing ExpirationDateTime/TransactionFromDateTime/TransactionToDateTime are 0..1 per the UK spec's OBReadConsent1 (open-ended if absent), but the shared v3.1.0/v4.0.1 request body class declared them as required Strings -- omitting any one failed json4s extraction. Thread Option[Date] through saveUKConsent/createUKConsentJWT (mirrors the Option[Date] validUntil pattern createBerlinGroupConsentJWT already uses) so a missing ExpirationDateTime means the consent never expires -- represented as Long.MaxValue in the JWT's exp claim, not "now" (which is what the BG sibling function defaults to when its own validUntil is absent; that default is wrong for "no limit" and not copied here). Nothing currently reads this claim for UK consents (checkUKConsent doesn't check expiry yet), so this only sets up correct behaviour for that future gap. Also fix DateWithDayFormat's silent truncation: it's a bare "yyyy-MM-dd" SimpleDateFormat, so a full ISO-8601 datetime like "2020-01-01T00:00:00+00:00" parsed leniently and silently dropped the time and offset, and a malformed value threw an uncaught ParseException that surfaced as 500 instead of 400. Add parseIso8601OrDayDate (tries full ISO-8601 first, falls back to a bare date) and route it through NewStyle.function.tryons so malformed input reaches 400 -- a bare Future.fromTry(Try(...)) doesn't get mapped to 400 by ErrorResponseConverter, since it only preserves the code from APIFailureNewStyle. Both v3.1.0 and v4.0.1 create/get handlers updated (the request body class and parser are shared); GET responses null-guard the now-possibly-absent stored dates instead of stringifying a null Date. --- .../v3_1_0/Http4sUKOBv310AccountAccess.scala | 37 ++++++++---- .../JSONFactory_UKOpenBanking_310.scala | 8 ++- .../v4_0_1/Http4sUKOBv401AccountInfo.scala | 29 +++++++--- .../JSONFactory_UKOpenBanking_401.scala | 12 ++-- .../main/scala/code/api/util/APIUtil.scala | 15 +++++ .../scala/code/api/util/ConsentUtil.scala | 13 +++-- .../scala/code/consent/ConsentProvider.scala | 6 +- .../scala/code/consent/MappedConsent.scala | 18 +++--- .../UKOpenBankingV401AccountInfoTests.scala | 56 ++++++++++++++++++- 9 files changed, 146 insertions(+), 48 deletions(-) diff --git a/obp-api/src/main/scala/code/api/UKOpenBanking/v3_1_0/Http4sUKOBv310AccountAccess.scala b/obp-api/src/main/scala/code/api/UKOpenBanking/v3_1_0/Http4sUKOBv310AccountAccess.scala index 03032bc919..36351ec65e 100644 --- a/obp-api/src/main/scala/code/api/UKOpenBanking/v3_1_0/Http4sUKOBv310AccountAccess.scala +++ b/obp-api/src/main/scala/code/api/UKOpenBanking/v3_1_0/Http4sUKOBv310AccountAccess.scala @@ -5,13 +5,13 @@ import cats.data.{Kleisli, OptionT} import cats.effect.IO import code.api.Constant import code.api.UKOpenBanking.v3_1_0.JSONFactory_UKOpenBanking_310.ConsentPostBodyUKV310 -import code.api.util.APIUtil.{EmptyBody, ResourceDoc, connectorEmptyResponse, mockedDataText, passesPsd2Aisp, unboxFullOrFail, DateWithDayFormat} +import code.api.util.APIUtil.{EmptyBody, ResourceDoc, connectorEmptyResponse, mockedDataText, passesPsd2Aisp, unboxFullOrFail, parseIso8601OrDayDate} import code.api.util.ApiTag import code.api.util.CustomJsonFormats -import code.api.util.ErrorMessages.{AuthenticatedUserIsRequired, ConsentNotFound, ConsentViewNotFund, UnknownError} +import code.api.util.ErrorMessages.{AuthenticatedUserIsRequired, ConsentNotFound, ConsentViewNotFund, InvalidJsonFormat, UnknownError} import code.api.util.http4s.Http4sRequestAttributes.{EndpointHelpers, RequestOps} import code.api.util.CallContext -import code.api.util.{ConsentJWT, JwtUtil} +import code.api.util.{ConsentJWT, JwtUtil, NewStyle} import code.consent.Consents import code.util.Helper.MdcLoggable import com.github.dwickern.macros.NameOf.nameOf @@ -54,6 +54,19 @@ object Http4sUKOBv310AccountAccess extends MdcLoggable { consentJson <- Future.fromTry(scala.util.Try( com.openbankproject.commons.util.JsonAliases.parse(cc.httpBody.getOrElse("{}")).extract[ConsentPostBodyUKV310] )) + // Separate step (not inlined into the saveUKConsent call below) so a bad date string + // fails here -> 400. NewStyle.function.tryons (not Future.fromTry) is required for + // that: ErrorResponseConverter only special-cases APIFailureNewStyle to preserve a set + // HTTP code -- tryons wraps failures that way, a bare Future.fromTry(Try(...)) doesn't + // and falls through to unknownErrorToResponse, i.e. 500. + (expirationDateTime, transactionFromDateTime, transactionToDateTime) <- NewStyle.function.tryons( + s"$InvalidJsonFormat The Json body should have valid ISO-8601 ExpirationDateTime/TransactionFromDateTime/TransactionToDateTime values ", 400, Some(cc)) { + ( + consentJson.Data.ExpirationDateTime.map(parseIso8601OrDayDate), + consentJson.Data.TransactionFromDateTime.map(parseIso8601OrDayDate), + consentJson.Data.TransactionToDateTime.map(parseIso8601OrDayDate) + ) + } consumerId = cc.consumer.map(_.consumerId.get) _ <- passesPsd2Aisp(Some(cc)) createdConsent <- Future(Consents.consentProvider.vend.saveUKConsent( @@ -62,9 +75,9 @@ object Http4sUKOBv310AccountAccess extends MdcLoggable { accountIds = None, consumerId = consumerId, permissions = consentJson.Data.Permissions, - expirationDateTime = DateWithDayFormat.parse(consentJson.Data.ExpirationDateTime), - transactionFromDateTime = DateWithDayFormat.parse(consentJson.Data.TransactionFromDateTime), - transactionToDateTime = DateWithDayFormat.parse(consentJson.Data.TransactionToDateTime), + expirationDateTime = expirationDateTime, + transactionFromDateTime = transactionFromDateTime, + transactionToDateTime = transactionToDateTime, apiStandard = Some("UKOpenBanking"), apiVersion = Some("3.1.0") )) map { i => connectorEmptyResponse(i, Some(cc)) } @@ -83,11 +96,11 @@ object Http4sUKOBv310AccountAccess extends MdcLoggable { "Status" : "${createdConsent.status}", "StatusUpdateDateTime" : "${createdConsent.statusUpdateDateTime}", "CreationDateTime" : "${createdConsent.creationDateTime}", - "TransactionToDateTime" : "${consentJson.Data.TransactionToDateTime}", - "ExpirationDateTime" : "${consentJson.Data.ExpirationDateTime}", + "TransactionToDateTime" : "${consentJson.Data.TransactionToDateTime.getOrElse("")}", + "ExpirationDateTime" : "${consentJson.Data.ExpirationDateTime.getOrElse("")}", "Permissions" : ${consentJson.Data.Permissions.mkString("[\"", "\",\"", "\"]")}, "ConsentId" : "${createdConsent.consentId}", - "TransactionFromDateTime" : "${consentJson.Data.TransactionFromDateTime}" + "TransactionFromDateTime" : "${consentJson.Data.TransactionFromDateTime.getOrElse("")}" } }""") } @@ -196,11 +209,11 @@ object Http4sUKOBv310AccountAccess extends MdcLoggable { "Status" : "${consent.status}", "StatusUpdateDateTime" : "${consent.statusUpdateDateTime}", "CreationDateTime" : "${consent.creationDateTime}", - "TransactionToDateTime" : "${consent.transactionToDateTime}", - "ExpirationDateTime" : "${consent.expirationDateTime}", + "TransactionToDateTime" : "${Option(consent.transactionToDateTime).getOrElse("")}", + "ExpirationDateTime" : "${Option(consent.expirationDateTime).getOrElse("")}", "Permissions" : ${consentViews.mkString("[\"", "\",\"", "\"]")}, "ConsentId" : "${consent.consentId}", - "TransactionFromDateTime" : "${consent.transactionFromDateTime}" + "TransactionFromDateTime" : "${Option(consent.transactionFromDateTime).getOrElse("")}" } }""") } diff --git a/obp-api/src/main/scala/code/api/UKOpenBanking/v3_1_0/JSONFactory_UKOpenBanking_310.scala b/obp-api/src/main/scala/code/api/UKOpenBanking/v3_1_0/JSONFactory_UKOpenBanking_310.scala index 396aa2fdfc..ef9c7e986d 100644 --- a/obp-api/src/main/scala/code/api/UKOpenBanking/v3_1_0/JSONFactory_UKOpenBanking_310.scala +++ b/obp-api/src/main/scala/code/api/UKOpenBanking/v3_1_0/JSONFactory_UKOpenBanking_310.scala @@ -204,11 +204,13 @@ object JSONFactory_UKOpenBanking_310 extends CustomJsonFormats { Links: LinksV310, Risk: String ) + // The three datetimes are 0..1 (open-ended if absent) per the UK spec's OBReadConsent1 — + // shared by the v3.1.0 and v4.0.1 consent-creation handlers. case class ConsentPostBodyDataUKV310( - TransactionToDateTime: String, - ExpirationDateTime: String, + TransactionToDateTime: Option[String], + ExpirationDateTime: Option[String], Permissions: List[String], - TransactionFromDateTime: String + TransactionFromDateTime: Option[String] ) case class ConsentPostBodyUKV310( Data: ConsentPostBodyDataUKV310, diff --git a/obp-api/src/main/scala/code/api/UKOpenBanking/v4_0_1/Http4sUKOBv401AccountInfo.scala b/obp-api/src/main/scala/code/api/UKOpenBanking/v4_0_1/Http4sUKOBv401AccountInfo.scala index 039b31485d..0152026cd9 100644 --- a/obp-api/src/main/scala/code/api/UKOpenBanking/v4_0_1/Http4sUKOBv401AccountInfo.scala +++ b/obp-api/src/main/scala/code/api/UKOpenBanking/v4_0_1/Http4sUKOBv401AccountInfo.scala @@ -5,11 +5,11 @@ import cats.effect.IO import code.api.APIFailureNewStyle import code.api.Constant import code.api.UKOpenBanking.v3_1_0.JSONFactory_UKOpenBanking_310.ConsentPostBodyUKV310 -import code.api.util.APIUtil.{EmptyBody, ResourceDoc, HTTPParam, connectorEmptyResponse, createQueriesByHttpParams, defaultBankId, fullBoxOrException, passesPsd2Aisp, unboxFull, unboxFullOrFail, DateWithDayFormat} +import code.api.util.APIUtil.{EmptyBody, ResourceDoc, HTTPParam, connectorEmptyResponse, createQueriesByHttpParams, defaultBankId, fullBoxOrException, passesPsd2Aisp, unboxFull, unboxFullOrFail, parseIso8601OrDayDate} import code.api.util.ApiTag import code.api.util.CallContext import code.api.util.CustomJsonFormats -import code.api.util.ErrorMessages.{AuthenticatedUserIsRequired, ConsentNotFound, ConsentViewNotFund, UnknownError} +import code.api.util.ErrorMessages.{AuthenticatedUserIsRequired, ConsentNotFound, ConsentViewNotFund, InvalidJsonFormat, UnknownError} import code.api.util.http4s.Http4sRequestAttributes.{EndpointHelpers, RequestOps} import code.api.util.newstyle.ViewNewStyle import code.api.util.{APIUtil, ConsentJWT, JwtUtil, NewStyle} @@ -101,6 +101,19 @@ object Http4sUKOBv401AccountInfo extends MdcLoggable { consentJson <- Future.fromTry(scala.util.Try( JsonAliases.parse(cc.httpBody.getOrElse("{}")).extract[ConsentPostBodyUKV310] )) + // Separate step (not inlined into the saveUKConsent call below) so a bad date string + // fails here -> 400. NewStyle.function.tryons (not Future.fromTry) is required for + // that: ErrorResponseConverter only special-cases APIFailureNewStyle to preserve a set + // HTTP code -- tryons wraps failures that way, a bare Future.fromTry(Try(...)) doesn't + // and falls through to unknownErrorToResponse, i.e. 500. + (expirationDateTime, transactionFromDateTime, transactionToDateTime) <- NewStyle.function.tryons( + s"$InvalidJsonFormat The Json body should have valid ISO-8601 ExpirationDateTime/TransactionFromDateTime/TransactionToDateTime values ", 400, Some(cc)) { + ( + consentJson.Data.ExpirationDateTime.map(parseIso8601OrDayDate), + consentJson.Data.TransactionFromDateTime.map(parseIso8601OrDayDate), + consentJson.Data.TransactionToDateTime.map(parseIso8601OrDayDate) + ) + } consumerId = cc.consumer.map(_.consumerId.get) _ <- passesPsd2Aisp(Some(cc)) createdConsent <- Future(Consents.consentProvider.vend.saveUKConsent( @@ -109,9 +122,9 @@ object Http4sUKOBv401AccountInfo extends MdcLoggable { accountIds = None, consumerId = consumerId, permissions = consentJson.Data.Permissions, - expirationDateTime = DateWithDayFormat.parse(consentJson.Data.ExpirationDateTime), - transactionFromDateTime = DateWithDayFormat.parse(consentJson.Data.TransactionFromDateTime), - transactionToDateTime = DateWithDayFormat.parse(consentJson.Data.TransactionToDateTime), + expirationDateTime = expirationDateTime, + transactionFromDateTime = transactionFromDateTime, + transactionToDateTime = transactionToDateTime, apiStandard = Some("UKOpenBanking"), apiVersion = Some("4.0.1") )) map { i => connectorEmptyResponse(i, Some(cc)) } @@ -195,9 +208,9 @@ object Http4sUKOBv401AccountInfo extends MdcLoggable { status = consent.status, statusUpdateDateTime = consent.statusUpdateDateTime.toString, permissions = consentViews, - expirationDateTime = consent.expirationDateTime.toString, - transactionFromDateTime = consent.transactionFromDateTime.toString, - transactionToDateTime = consent.transactionToDateTime.toString, + expirationDateTime = Option(consent.expirationDateTime).map(_.toString), + transactionFromDateTime = Option(consent.transactionFromDateTime).map(_.toString), + transactionToDateTime = Option(consent.transactionToDateTime).map(_.toString), selfPath = s"/aisp/account-access-consents/$consentId" ) } diff --git a/obp-api/src/main/scala/code/api/UKOpenBanking/v4_0_1/JSONFactory_UKOpenBanking_401.scala b/obp-api/src/main/scala/code/api/UKOpenBanking/v4_0_1/JSONFactory_UKOpenBanking_401.scala index 950cac825a..fb2fe5790e 100644 --- a/obp-api/src/main/scala/code/api/UKOpenBanking/v4_0_1/JSONFactory_UKOpenBanking_401.scala +++ b/obp-api/src/main/scala/code/api/UKOpenBanking/v4_0_1/JSONFactory_UKOpenBanking_401.scala @@ -143,9 +143,9 @@ object JSONFactory_UKOpenBanking_401 extends CustomJsonFormats { StatusReason: List[StatusReasonV401], StatusUpdateDateTime: String, Permissions: List[String], - ExpirationDateTime: String, - TransactionFromDateTime: String, - TransactionToDateTime: String + ExpirationDateTime: Option[String], + TransactionFromDateTime: Option[String], + TransactionToDateTime: Option[String] ) case class ConsentResponseV401( Data: ConsentDataV401, @@ -355,9 +355,9 @@ object JSONFactory_UKOpenBanking_401 extends CustomJsonFormats { status: String, statusUpdateDateTime: String, permissions: List[String], - expirationDateTime: String, - transactionFromDateTime: String, - transactionToDateTime: String, + expirationDateTime: Option[String], + transactionFromDateTime: Option[String], + transactionToDateTime: Option[String], selfPath: String ): ConsentResponseV401 = { val statusCode = ukConsentStatusCode(status) diff --git a/obp-api/src/main/scala/code/api/util/APIUtil.scala b/obp-api/src/main/scala/code/api/util/APIUtil.scala index cd283759a1..c899eecb44 100644 --- a/obp-api/src/main/scala/code/api/util/APIUtil.scala +++ b/obp-api/src/main/scala/code/api/util/APIUtil.scala @@ -174,6 +174,21 @@ object APIUtil extends MdcLoggable with CustomJsonFormats{ def rfc7231Date: SimpleDateFormat = rfc7231DateTL.get() + /** + * Parses a full ISO-8601 datetime with offset (e.g. "2020-01-01T00:00:00+00:00") or, failing + * that, a bare "yyyy-MM-dd" date (midnight UTC). Unlike DateWithDayFormat (a SimpleDateFormat + * with "yyyy-MM-dd"), this does not silently truncate a full datetime's time/offset — it parses + * the whole value or throws java.time.format.DateTimeParseException on genuinely malformed input. + */ + def parseIso8601OrDayDate(s: String): Date = { + try { + Date.from(java.time.OffsetDateTime.parse(s).toInstant) + } catch { + case _: java.time.format.DateTimeParseException => + Date.from(java.time.LocalDate.parse(s).atStartOfDay(java.time.ZoneOffset.UTC).toInstant) + } + } + val DateWithYearExampleString: String = "1100" val DateWithMonthExampleString: String = "1100-01" val DateWithDayExampleString: String = "1100-01-01" diff --git a/obp-api/src/main/scala/code/api/util/ConsentUtil.scala b/obp-api/src/main/scala/code/api/util/ConsentUtil.scala index 94ff5ef545..a51d958704 100644 --- a/obp-api/src/main/scala/code/api/util/ConsentUtil.scala +++ b/obp-api/src/main/scala/code/api/util/ConsentUtil.scala @@ -1048,9 +1048,9 @@ object Consent extends MdcLoggable { bankId: Option[String], accountIds: Option[List[String]], permissions: List[String], - expirationDateTime: Date, - transactionFromDateTime: Date, - transactionToDateTime: Date, + expirationDateTime: Option[Date], + transactionFromDateTime: Option[Date], + transactionToDateTime: Option[Date], secret: String, consentId: String, consumerId: Option[String] @@ -1059,7 +1059,12 @@ object Consent extends MdcLoggable { val createdByUserId = user.map(_.userId).getOrElse("None") val currentConsumerId = Consumer.findAll(By(Consumer.createdByUserId, createdByUserId)).map(_.consumerId.get).headOption.getOrElse("") val currentTimeInSeconds = System.currentTimeMillis / 1000 - val validUntilTimeInSeconds = expirationDateTime.getTime() / 1000 + // No ExpirationDateTime means the consent never expires (UK spec: 0..1, open-ended if absent). + // Use Long.MaxValue rather than e.g. "now" (the convention createBerlinGroupConsentJWT falls + // back to for its own optional validUntil) since that would make the JWT read as already + // expired -- wrong for "no limit". Nothing currently reads this exp claim for UK consents + // (checkUKConsent doesn't check expiry at all yet), so this only matters once that's added. + val validUntilTimeInSeconds = expirationDateTime.map(_.getTime / 1000).getOrElse(Long.MaxValue) // Write Consent's Auth Context to the DB user map { u => val authContexts = UserAuthContextProvider.userAuthContextProvider.vend.getUserAuthContextsBox(u.userId) diff --git a/obp-api/src/main/scala/code/consent/ConsentProvider.scala b/obp-api/src/main/scala/code/consent/ConsentProvider.scala index 51b63906d1..3932ed12bc 100644 --- a/obp-api/src/main/scala/code/consent/ConsentProvider.scala +++ b/obp-api/src/main/scala/code/consent/ConsentProvider.scala @@ -48,9 +48,9 @@ trait ConsentProvider { accountIds: Option[List[String]],//for UK Open Banking endpoints, there is no accountIds there. consumerId: Option[String], permissions: List[String], - expirationDateTime: Date, - transactionFromDateTime: Date, - transactionToDateTime: Date, + expirationDateTime: Option[Date], //0..1 per spec: None = open-ended / never expires + transactionFromDateTime: Option[Date], //0..1 per spec: None = no restriction on history start + transactionToDateTime: Option[Date], //0..1 per spec: None = no restriction on history end apiStandard: Option[String], apiVersion: Option[String] ): Box[ConsentTrait] diff --git a/obp-api/src/main/scala/code/consent/MappedConsent.scala b/obp-api/src/main/scala/code/consent/MappedConsent.scala index 8e0cb38918..960cb731a3 100644 --- a/obp-api/src/main/scala/code/consent/MappedConsent.scala +++ b/obp-api/src/main/scala/code/consent/MappedConsent.scala @@ -267,9 +267,9 @@ object MappedConsentProvider extends ConsentProvider with code.util.Helper.MdcLo accountIds: Option[List[String]],//for UK Open Banking endpoints, there is no accountIds there. consumerId: Option[String], permissions: List[String], - expirationDateTime: Date, - transactionFromDateTime: Date, - transactionToDateTime: Date, + expirationDateTime: Option[Date], + transactionFromDateTime: Option[Date], + transactionToDateTime: Option[Date], apiStandard: Option[String], apiVersion: Option[String] ) ={ @@ -279,9 +279,9 @@ object MappedConsentProvider extends ConsentProvider with code.util.Helper.MdcLo .mUserId(user.map(_.userId).getOrElse(null)) .mConsumerId(consumerId.getOrElse(null)) .mStatus(ConsentStatus.AWAITINGAUTHORISATION.toString) - .mExpirationDateTime(expirationDateTime) - .mTransactionFromDateTime(transactionFromDateTime) - .mTransactionToDateTime(transactionToDateTime) + .mExpirationDateTime(expirationDateTime.orNull) + .mTransactionFromDateTime(transactionFromDateTime.orNull) + .mTransactionToDateTime(transactionToDateTime.orNull) .mStatusUpdateDateTime(now) .mApiVersion(apiVersion.getOrElse(null)) .mApiStandard(apiStandard.getOrElse(null)) @@ -291,9 +291,9 @@ object MappedConsentProvider extends ConsentProvider with code.util.Helper.MdcLo bankId: Option[String], accountIds: Option[List[String]], permissions: List[String], - expirationDateTime: Date, - transactionFromDateTime: Date, - transactionToDateTime: Date, + expirationDateTime: Option[Date], + transactionFromDateTime: Option[Date], + transactionToDateTime: Option[Date], secret = consent.secret, consentId = consent.consentId, consumerId: Option[String] diff --git a/obp-api/src/test/scala/code/api/UKOpenBanking/v4_0_1/UKOpenBankingV401AccountInfoTests.scala b/obp-api/src/test/scala/code/api/UKOpenBanking/v4_0_1/UKOpenBankingV401AccountInfoTests.scala index 5ab9997c38..257441575c 100644 --- a/obp-api/src/test/scala/code/api/UKOpenBanking/v4_0_1/UKOpenBankingV401AccountInfoTests.scala +++ b/obp-api/src/test/scala/code/api/UKOpenBanking/v4_0_1/UKOpenBankingV401AccountInfoTests.scala @@ -59,9 +59,9 @@ class UKOpenBankingV401AccountInfoTests extends UKOpenBankingV401ServerSetup { accountIds = None, consumerId = None, permissions = consentPermissions, - expirationDateTime = DateWithDayFormat.parse("2030-01-01"), - transactionFromDateTime = DateWithDayFormat.parse("2020-01-01"), - transactionToDateTime = DateWithDayFormat.parse("2030-01-01"), + expirationDateTime = Some(DateWithDayFormat.parse("2030-01-01")), + transactionFromDateTime = Some(DateWithDayFormat.parse("2020-01-01")), + transactionToDateTime = Some(DateWithDayFormat.parse("2030-01-01")), apiStandard = Some("UKOpenBanking"), apiVersion = Some("4.0.1") ).openOrThrowException("test consent creation failed") @@ -84,6 +84,56 @@ class UKOpenBankingV401AccountInfoTests extends UKOpenBankingV401ServerSetup { scenario("unauthenticated -> 401", UKOpenBankingV401AccountInfo) { postUnauthed(consentPostBody, "aisp", "account-access-consents").code should equal(401) } + scenario("all three datetime fields omitted -> 201, open-ended (no expiry/date restriction)", UKOpenBankingV401AccountInfo) { + val bodyWithoutDates = + """{ + | "Data": { + | "Permissions": ["ReadAccountsBasic"] + | }, + | "Risk": {} + |}""".stripMargin + val response = postAuthed(bodyWithoutDates, "aisp", "account-access-consents") + response.code should equal(201) + val consentId = (response.body \ "Data" \ "ConsentId").extract[String] + val consent = Consents.consentProvider.vend.getConsentByConsentId(consentId).openOrThrowException("consent") + // MappedDateTime fields left unset by a null write come back as Java null, not a sentinel date. + consent.expirationDateTime should equal(null) + consent.transactionFromDateTime should equal(null) + consent.transactionToDateTime should equal(null) + } + scenario("full ISO-8601 datetime with time and offset is preserved, not truncated to a bare date", UKOpenBankingV401AccountInfo) { + val bodyWithFullDatetime = + """{ + | "Data": { + | "Permissions": ["ReadAccountsBasic"], + | "ExpirationDateTime": "2030-06-15T13:45:30+02:00", + | "TransactionFromDateTime": "2020-01-01", + | "TransactionToDateTime": "2030-01-01" + | }, + | "Risk": {} + |}""".stripMargin + val response = postAuthed(bodyWithFullDatetime, "aisp", "account-access-consents") + response.code should equal(201) + val consentId = (response.body \ "Data" \ "ConsentId").extract[String] + val consent = Consents.consentProvider.vend.getConsentByConsentId(consentId).openOrThrowException("consent") + // 2030-06-15T13:45:30+02:00 == 2030-06-15T11:45:30Z -- if the parser truncated to the bare + // date (the pre-fix DateWithDayFormat behaviour), this would be 2030-06-15T00:00:00Z instead. + consent.expirationDateTime.getTime should equal( + java.time.OffsetDateTime.parse("2030-06-15T13:45:30+02:00").toInstant.toEpochMilli) + } + scenario("malformed datetime -> 400, not 500", UKOpenBankingV401AccountInfo) { + val bodyWithBadDate = + """{ + | "Data": { + | "Permissions": ["ReadAccountsBasic"], + | "ExpirationDateTime": "not-a-date", + | "TransactionFromDateTime": "2020-01-01", + | "TransactionToDateTime": "2030-01-01" + | }, + | "Risk": {} + |}""".stripMargin + postAuthed(bodyWithBadDate, "aisp", "account-access-consents").code should equal(400) + } } feature("UKOB v4.0.1 GET /aisp/account-access-consents/CONSENT_ID") { scenario("authenticated with real consent -> 200 real data", UKOpenBankingV401AccountInfo) { From 051167e578846c4a95250da15964482fbccbd6e6 Mon Sep 17 00:00:00 2001 From: Hongwei Date: Sat, 18 Jul 2026 14:24:26 +0200 Subject: [PATCH 16/21] fix: expire UK Open Banking consents past their ExpirationDateTime An AUTHORISED UK consent was effectively perpetual: checkUKConsent only checked not-before (creationDateTime), never the consent's own ExpirationDateTime or the JWT exp, and ConsentScheduler's expiry tasks filtered on apiStandard=BG / apiStandard=obp, excluding UK entirely. A PSU who consented to "until date X" kept granting access indefinitely unless the consent was manually revoked -- a minimum-necessary-access / data-minimisation problem. Add both layers: - Reactive (the actual security control): checkUKConsent now rejects an AUTHORISED consent whose ExpirationDateTime has passed, with the existing OBP-35003 ConsentExpiredIssue (401). A null ExpirationDateTime means the consent never expires (0..1 per spec, open-ended if absent) and is skipped. This closes the gap immediately regardless of scheduler cadence. - Proactive: a new ConsentScheduler.expiredUKConsents task flips long-past-expiry AUTHORISED UK consents to EXPIRED so the stored status stays accurate for GET/dashboard reads. Interval prop uk_open_banking_expired_consents_interval_in_seconds (default 601, 0 to disable), mirroring the existing BG/OBP expiry tasks. A null mExpirationDateTime never matches By_< against a Date, so perpetual consents are correctly never selected. --- .../scala/code/api/util/ConsentUtil.scala | 6 +++ .../code/scheduler/ConsentScheduler.scala | 49 ++++++++++++++++++- .../UKOpenBankingV401AccountInfoTests.scala | 45 +++++++++++++++-- 3 files changed, 96 insertions(+), 4 deletions(-) diff --git a/obp-api/src/main/scala/code/api/util/ConsentUtil.scala b/obp-api/src/main/scala/code/api/util/ConsentUtil.scala index a51d958704..819f4dc500 100644 --- a/obp-api/src/main/scala/code/api/util/ConsentUtil.scala +++ b/obp-api/src/main/scala/code/api/util/ConsentUtil.scala @@ -1266,6 +1266,12 @@ object Consent extends MdcLoggable { System.currentTimeMillis match { case currentTimeMillis if currentTimeMillis < c.creationDateTime.getTime => Failure(ErrorMessages.ConsentNotBeforeIssue) + // A null expirationDateTime means the consent never expires (0..1 per spec, + // open-ended if absent -- see createUKConsentJWT). ConsentScheduler.expiredUKConsents + // proactively flips long-past-expiry consents to EXPIRED, but that runs on an + // interval; this reactive check closes the gap immediately regardless of timing. + case currentTimeMillis if Option(c.expirationDateTime).exists(_.getTime < currentTimeMillis) => + Failure(ErrorMessages.ConsentExpiredIssue) case _ if c.mUserId.get != user.userId => Failure(ErrorMessages.ConsentDoesNotMatchUser) case _ => diff --git a/obp-api/src/main/scala/code/scheduler/ConsentScheduler.scala b/obp-api/src/main/scala/code/scheduler/ConsentScheduler.scala index d44d222899..6f9dd17aa5 100644 --- a/obp-api/src/main/scala/code/scheduler/ConsentScheduler.scala +++ b/obp-api/src/main/scala/code/scheduler/ConsentScheduler.scala @@ -1,7 +1,7 @@ package code.scheduler import code.api.berlin.group.ConstantsBG -import code.api.util.APIUtil +import code.api.util.{APIUtil, Consent} import code.consent.{ConsentStatus, MappedConsent} import code.util.Helper.MdcLoggable import com.openbankproject.commons.util.{ApiStandards, ApiVersion} @@ -50,6 +50,17 @@ object ConsentScheduler extends MdcLoggable { } else { logger.warn("|---> Skipping expiredObpConsents task: obp_expired_consents_interval_in_seconds set to 0") } + + // UK Open Banking. checkUKConsent (ConsentUtil.scala) also reactively rejects an expired + // AUTHORISED consent on every request regardless of this task's timing -- this proactive + // sweep just keeps the stored status accurate for GET/dashboard purposes. + val ukExpiredInterval = APIUtil.getPropsAsIntValue("uk_open_banking_expired_consents_interval_in_seconds", 601) + if (ukExpiredInterval > 0) { + SchedulerUtil.startTask(interval = ukExpiredInterval, () => expiredUKConsents(), initialDelay) + initialDelay = initialDelay + 10 + } else { + logger.warn("|---> Skipping expiredUKConsents task: uk_open_banking_expired_consents_interval_in_seconds set to 0") + } } @@ -162,5 +173,41 @@ object ConsentScheduler extends MdcLoggable { case Success(_) => logger.debug("|---> Task executed successfully") } } + private def expiredUKConsents(): Unit = { + Try { + logger.debug("|---> Checking for expired UK Open Banking consents...") + + // A null mExpirationDateTime (never set -- 0..1 per spec, open-ended if absent) never + // matches By_< against a real Date, so perpetual consents are correctly never selected here. + val expiredConsents = MappedConsent.findAll( + By(MappedConsent.mStatus, ConsentStatus.AUTHORISED.toString), + By(MappedConsent.mApiStandard, Consent.ConsentStandardUK), + By_<(MappedConsent.mExpirationDateTime, new Date()) + ) + + logger.debug(s"|---> Found ${expiredConsents.size} expired consents") + + expiredConsents.foreach { consent => + Try { + val message = s"|---> Changed status from ${consent.status} to ${ConsentStatus.EXPIRED.toString} for consent ID: ${consent.id}" + val newNote = s"$currentDate\n$message\n" + Option(consent.note).getOrElse("") + val rows = code.bankconnectors.DoobieConsentSchedulerQueries.conditionallyUpdateStatus( + consentPrimaryKey = consent.id.get, + guardStatus = ConsentStatus.AUTHORISED.toString, + newStatus = ConsentStatus.EXPIRED.toString, + newNote = newNote + ) + if (rows > 0) logger.warn(message) + else logger.debug(s"|---> Skipped stale update for UK consent ${consent.id}: status already changed") + } match { + case Failure(ex) => logger.error(s"Failed to update consent ID: ${consent.id}", ex) + case Success(_) => // Already logged + } + } + } match { + case Failure(ex) => logger.error("Error in expiredUKConsents!", ex) + case Success(_) => logger.debug("|---> Task executed successfully") + } + } } diff --git a/obp-api/src/test/scala/code/api/UKOpenBanking/v4_0_1/UKOpenBankingV401AccountInfoTests.scala b/obp-api/src/test/scala/code/api/UKOpenBanking/v4_0_1/UKOpenBankingV401AccountInfoTests.scala index 257441575c..57cc8dfcf0 100644 --- a/obp-api/src/test/scala/code/api/UKOpenBanking/v4_0_1/UKOpenBankingV401AccountInfoTests.scala +++ b/obp-api/src/test/scala/code/api/UKOpenBanking/v4_0_1/UKOpenBankingV401AccountInfoTests.scala @@ -2,12 +2,13 @@ package code.api.UKOpenBanking.v4_0_1 import code.api.Constant import code.api.util.APIUtil.DateWithDayFormat -import code.api.util.ErrorMessages.ConsentIdClaimMissing -import code.api.util.Consent -import code.consent.Consents +import code.api.util.ErrorMessages.{ConsentExpiredIssue, ConsentIdClaimMissing} +import code.api.util.{CallContext, CertificateUtil, Consent} +import code.consent.{ConsentStatus, Consents} import code.model.UserExtended import code.views.Views import com.openbankproject.commons.model.{BankIdAccountId, ErrorMessage, ViewId} +import net.liftweb.common.{Failure, Full} import org.json4s._ import org.scalatest.Tag @@ -214,6 +215,44 @@ class UKOpenBankingV401AccountInfoTests extends UKOpenBankingV401ServerSetup { } } + // Regression coverage for Gap 14 (UK consents previously never expired): checkUKConsent must + // reactively reject an AUTHORISED consent whose ExpirationDateTime has passed, independent of + // ConsentScheduler's proactive sweep timing. Exercised by calling Consent.checkUKConsent + // directly with a hand-built CallContext carrying a self-signed Bearer JWT with a consent_id + // claim -- JwtUtil.getOptionalClaim only parses the JWT structurally (SignedJWT.parse), it does + // not verify the signature, so this doesn't need to be signed with the real shared secret. This + // sidesteps the suite-wide limitation noted above (OAuth1-signed test requests carry no real + // Bearer JWT) for the one scenario that specifically needs one. + feature("UKOB v4.0.1 Consent.checkUKConsent rejects an authorised consent past its ExpirationDateTime") { + scenario("expired consent -> Failure(ConsentExpiredIssue), not silently accepted", UKOpenBankingV401AccountInfo) { + val consent = Consents.consentProvider.vend.saveUKConsent( + user = Some(resourceUser1), + bankId = None, + accountIds = None, + consumerId = None, + permissions = consentPermissions, + expirationDateTime = Some(new java.util.Date(System.currentTimeMillis() - 60000L)), // 1 minute ago + transactionFromDateTime = None, + transactionToDateTime = None, + apiStandard = Some("UKOpenBanking"), + apiVersion = Some("4.0.1") + ).openOrThrowException("consent creation failed") + Consents.consentProvider.vend.updateConsentUser(consent.consentId, resourceUser1) + Consents.consentProvider.vend.updateConsentStatus(consent.consentId, ConsentStatus.AUTHORISED) + + val claimsSet = new com.nimbusds.jwt.JWTClaimsSet.Builder() + .claim("consent_id", consent.consentId) + .build() + val jwt = CertificateUtil.jwtWithHmacProtection(claimsSet) + val callContext = CallContext(authReqHeaderField = Full(s"Bearer $jwt")) + + Consent.checkUKConsent(resourceUser1, Some(callContext)) match { + case Failure(msg, _, _) => msg.contains(ConsentExpiredIssue) should equal(true) + case other => fail(s"expected Failure(ConsentExpiredIssue), got $other") + } + } + } + // ── AccountsApi ──────────────────────────────────────────────────── // checkUKConsent extracts the `consent_id` claim from the Bearer access token (no external // Hydra call since Consent.checkUKConsent dropped the Hydra dependency). These OAuth1-signed From ccba546d793c0f757a0bbacefaa90970b6eddaec Mon Sep 17 00:00:00 2001 From: Hongwei Date: Sat, 18 Jul 2026 14:54:32 +0200 Subject: [PATCH 17/21] feat: allow re-authentication of a UK Open Banking consent authoriseUKConsentChallenge and authoriseUKConsent hard-guarded status == AWAITINGAUTHORISATION, so once a consent was AUTHORISED nothing could reopen it -- a TPP whose access token was lost had to create a brand-new consent. The UK spec allows re-authentication of the same ConsentId while its status is AUTH or CANC (OBP's REVOKED) and its ExpirationDateTime has not elapsed. Relax both endpoints' status guard to ukReAuthableStatuses (AWAITINGAUTHORISATION, AUTHORISED, REVOKED); EXPIRED and REJECTED stay terminal. Add two guards that the previous single-status check made unnecessary but re-auth now requires: - not-expired: reject re-auth of a consent past its ExpirationDateTime (null = never expires), consistent with the reactive expiry check added to checkUKConsent. - same-PSU: a consent already bound to a user may only be re-authorised by that same user, so a different user of the same consumer cannot hijack it (updateConsentUser rebinds mUserId unconditionally). Re-running the SCA + grantUKConsentAccountAccess flow is already idempotent (grantAccessToViews revokes-then-regrants per view), so a second authorise cannot double-grant. Tests cover the two new rejection guards at the challenge-start step. The successful re-auth happy path needs a completed SCA/OTP ceremony, which has no test harness in this repo yet (these authorise endpoints shipped without coverage), so it is not asserted here. --- .../scala/code/api/v5_1_0/Http4s510.scala | 51 +++++++++++++++---- .../UKOpenBankingV401AccountInfoTests.scala | 43 +++++++++++++++- 2 files changed, 84 insertions(+), 10 deletions(-) diff --git a/obp-api/src/main/scala/code/api/v5_1_0/Http4s510.scala b/obp-api/src/main/scala/code/api/v5_1_0/Http4s510.scala index e2affd0100..163b638831 100644 --- a/obp-api/src/main/scala/code/api/v5_1_0/Http4s510.scala +++ b/obp-api/src/main/scala/code/api/v5_1_0/Http4s510.scala @@ -106,6 +106,17 @@ object Http4s510 { val prefixPath = Root / ApiPathZero.toString / implementedInApiVersion.toString + // Statuses from which a UK Open Banking consent may be (re-)authorised. AWAITINGAUTHORISATION + // is the initial authorise; AUTHORISED and REVOKED (wire: CANC) are the re-authentication + // cases per the UK spec ("re-authenticate ... if the account-access-consent has a Status of + // AUTH or CANC and the ExpirationDateTime has not elapsed"). EXPIRED and REJECTED are terminal + // -- the TPP must create a new consent rather than re-authenticate. + private val ukReAuthableStatuses: Set[String] = Set( + ConsentStatus.AWAITINGAUTHORISATION.toString, + ConsentStatus.AUTHORISED.toString, + ConsentStatus.REVOKED.toString + ) + // Used by lifted consumer-management endpoint descriptions. private def consumerDisabledText(): String = { if (APIUtil.getPropsAsBoolValue("consumers_enabled_by_default", false) == false) { @@ -4283,8 +4294,18 @@ object Http4s510 { user <- Future.successful(cc.user.openOrThrowException(AuthenticatedUserIsRequired)) consent <- Future(Consents.consentProvider.vend.getConsentByConsentId(consentId)) .map(unboxFullOrFail(_, Some(cc), s"$ConsentNotFound ($consentId)", 404)) - _ <- Helper.booleanToFuture(s"$ConsentStatusIssue${ConsentStatus.AWAITINGAUTHORISATION.toString} to start SCA (current: ${consent.status}).", 400, Some(cc)) { - consent.status.toUpperCase == ConsentStatus.AWAITINGAUTHORISATION.toString + _ <- Helper.booleanToFuture(s"$ConsentStatusIssue one of ${ukReAuthableStatuses.mkString(", ")} to start SCA (current: ${consent.status}).", 400, Some(cc)) { + ukReAuthableStatuses.contains(consent.status.toUpperCase) + } + // Re-authentication is only allowed before the consent's ExpirationDateTime (a null + // ExpirationDateTime = never expires); an expired consent must be recreated. + _ <- Helper.booleanToFuture(s"$ConsentExpiredIssue", 400, Some(cc)) { + Option(consent.expirationDateTime).forall(_.getTime >= System.currentTimeMillis) + } + // A consent already bound to a PSU may only be (re-)authorised by that same PSU -- + // otherwise a different user of the same consumer could hijack it. + _ <- Helper.booleanToFuture(s"$ConsentDoesNotMatchUser", 403, Some(cc)) { + Option(consent.userId).forall(_.isBlank) || consent.userId == user.userId } (challenges, _) <- NewStyle.function.createChallengesC2( List(user.userId), @@ -4324,7 +4345,7 @@ object Http4s510 { |""", EmptyBody, UKConsentScaChallengeJsonV510("74a8ebda-9e5a-4c3f-9b0b-1a2b3c4d5e6f", "received", "SMS"), - List($AuthenticatedUserIsRequired, ConsentNotFound, ConsentStatusIssue, InvalidConnectorResponse, UnknownError), + List($AuthenticatedUserIsRequired, ConsentNotFound, ConsentStatusIssue, ConsentExpiredIssue, ConsentDoesNotMatchUser, InvalidConnectorResponse, UnknownError), apiTagConsent :: apiTagPSD2AIS :: Nil, None, http4sPartialFunction = Some(authoriseUKConsentChallenge) @@ -4346,11 +4367,23 @@ object Http4s510 { user <- Future.successful(cc.user.openOrThrowException(AuthenticatedUserIsRequired)) consent <- Future(Consents.consentProvider.vend.getConsentByConsentId(consentId)) .map(unboxFullOrFail(_, Some(cc), s"$ConsentNotFound ($consentId)", 404)) - // Only a consent still awaiting authorisation can be authorised. This status - // is UK-specific (Berlin Group uses "received", OBP uses INITIATED), so the - // guard also effectively scopes this endpoint to the UK flow. - _ <- Helper.booleanToFuture(s"$ConsentStatusIssue${ConsentStatus.AWAITINGAUTHORISATION.toString} to be authorised (current: ${consent.status}).", 400, Some(cc)) { - consent.status.toUpperCase == ConsentStatus.AWAITINGAUTHORISATION.toString + // The initial authorisation (AWAITINGAUTHORISATION) and re-authentication of an + // already-authorised or dashboard-revoked (wire: CANC) consent are both allowed -- + // see ukReAuthableStatuses. EXPIRED/REJECTED are terminal. These statuses are + // UK-specific (Berlin Group uses "received", OBP uses INITIATED), so the guard also + // effectively scopes this endpoint to the UK flow. + _ <- Helper.booleanToFuture(s"$ConsentStatusIssue one of ${ukReAuthableStatuses.mkString(", ")} to be authorised (current: ${consent.status}).", 400, Some(cc)) { + ukReAuthableStatuses.contains(consent.status.toUpperCase) + } + // Re-authentication is only allowed before the consent's ExpirationDateTime (a null + // ExpirationDateTime = never expires); an expired consent must be recreated. + _ <- Helper.booleanToFuture(s"$ConsentExpiredIssue", 400, Some(cc)) { + Option(consent.expirationDateTime).forall(_.getTime >= System.currentTimeMillis) + } + // A consent already bound to a PSU may only be (re-)authorised by that same PSU -- + // otherwise a different user of the same consumer could hijack it. + _ <- Helper.booleanToFuture(s"$ConsentDoesNotMatchUser", 403, Some(cc)) { + Option(consent.userId).forall(_.isBlank) || consent.userId == user.userId } // Verify the SCA challenge answer before authorising (dynamic linking to this consent). // The challenge must have been started via POST .../authorise/challenge. @@ -4426,7 +4459,7 @@ object Http4s510 { "eyJhbGciOiJIUzI1NiJ9.eyJ2aWV3cyI6W119.signature", "AUTHORISED" ), - List($AuthenticatedUserIsRequired, ConsentNotFound, ConsentStatusIssue, InvalidJsonFormat, InvalidChallengeAnswer, $BankAccountNotFound, InvalidConnectorResponse, UnknownError), + List($AuthenticatedUserIsRequired, ConsentNotFound, ConsentStatusIssue, ConsentExpiredIssue, ConsentDoesNotMatchUser, InvalidJsonFormat, InvalidChallengeAnswer, $BankAccountNotFound, InvalidConnectorResponse, UnknownError), apiTagConsent :: apiTagPSD2AIS :: Nil, None, http4sPartialFunction = Some(authoriseUKConsent) diff --git a/obp-api/src/test/scala/code/api/UKOpenBanking/v4_0_1/UKOpenBankingV401AccountInfoTests.scala b/obp-api/src/test/scala/code/api/UKOpenBanking/v4_0_1/UKOpenBankingV401AccountInfoTests.scala index 57cc8dfcf0..a38f466d4e 100644 --- a/obp-api/src/test/scala/code/api/UKOpenBanking/v4_0_1/UKOpenBankingV401AccountInfoTests.scala +++ b/obp-api/src/test/scala/code/api/UKOpenBanking/v4_0_1/UKOpenBankingV401AccountInfoTests.scala @@ -2,7 +2,7 @@ package code.api.UKOpenBanking.v4_0_1 import code.api.Constant import code.api.util.APIUtil.DateWithDayFormat -import code.api.util.ErrorMessages.{ConsentExpiredIssue, ConsentIdClaimMissing} +import code.api.util.ErrorMessages.{ConsentDoesNotMatchUser, ConsentExpiredIssue, ConsentIdClaimMissing} import code.api.util.{CallContext, CertificateUtil, Consent} import code.consent.{ConsentStatus, Consents} import code.model.UserExtended @@ -253,6 +253,47 @@ class UKOpenBankingV401AccountInfoTests extends UKOpenBankingV401ServerSetup { } } + // Gap 12 (re-authentication) security guards on POST /obp/v5.1.0/banks/BANK_ID/consents/ + // CONSENT_ID/authorise/challenge -- the first step of the ceremony, so it can be exercised + // without a completed SCA/OTP. The successful re-auth path (start challenge -> answer OTP -> + // stays AUTHORISED) needs the full OTP ceremony, which has no test harness in this repo yet + // (the authorise endpoints shipped with no coverage), so it's not asserted here; these cover + // the security-relevant rejection branches the re-auth relaxation introduces. + private def scaChallengeRequest(consentId: String) = + (baseRequest / "obp" / "v5.1.0" / "banks" / testBankId1.value / "consents" / consentId / "authorise" / "challenge").POST + private def createUKConsent(user: com.openbankproject.commons.model.User, expiration: Option[java.util.Date]): String = { + val consent = Consents.consentProvider.vend.saveUKConsent( + user = Some(user), bankId = None, accountIds = None, consumerId = None, + permissions = consentPermissions, expirationDateTime = expiration, + transactionFromDateTime = None, transactionToDateTime = None, + apiStandard = Some("UKOpenBanking"), apiVersion = Some("4.0.1") + ).openOrThrowException("consent creation failed") + consent.consentId + } + feature("UKOB v4.0.1 re-authentication guards on the SCA challenge endpoint") { + scenario("challenge-start on an already-authorised consent bound to a different PSU -> 403", UKOpenBankingV401AccountInfo) { + val consentId = createUKConsent(resourceUser1, Some(new java.util.Date(System.currentTimeMillis() + 3600000L))) + Consents.consentProvider.vend.updateConsentUser(consentId, resourceUser1) + Consents.consentProvider.vend.updateConsentStatus(consentId, ConsentStatus.AUTHORISED) + // user2 != the bound resourceUser1 -> hijack guard rejects + val response = makePostRequest(scaChallengeRequest(consentId) <@ (user2), "") + response.code should equal(403) + response.body.extract[ErrorMessage].message.contains(ConsentDoesNotMatchUser) should equal(true) + } + scenario("challenge-start on an authorised consent past its ExpirationDateTime -> 400 ConsentExpiredIssue", UKOpenBankingV401AccountInfo) { + val consentId = createUKConsent(resourceUser1, Some(new java.util.Date(System.currentTimeMillis() - 60000L))) + Consents.consentProvider.vend.updateConsentUser(consentId, resourceUser1) + Consents.consentProvider.vend.updateConsentStatus(consentId, ConsentStatus.AUTHORISED) + val response = makePostRequest(scaChallengeRequest(consentId) <@ (user1), "") + response.code should equal(400) + response.body.extract[ErrorMessage].message.contains(ConsentExpiredIssue) should equal(true) + } + // Note: the terminal-status rejection (EXPIRED/REJECTED can't be re-authed) is enforced by + // ukReAuthableStatuses not containing those; it isn't asserted via a third HTTP scenario here + // because a second same-user OAuth1 call within this block collides on the test harness's + // nonce/timestamp replay check (a harness artifact -- production UK uses OAuth2 Bearer). + } + // ── AccountsApi ──────────────────────────────────────────────────── // checkUKConsent extracts the `consent_id` claim from the Bearer access token (no external // Hydra call since Consent.checkUKConsent dropped the Hydra dependency). These OAuth1-signed From c6eae4192c3e1f2fdace4e16374a99860979effd Mon Sep 17 00:00:00 2001 From: Hongwei Date: Sat, 18 Jul 2026 15:08:00 +0200 Subject: [PATCH 18/21] fix: allow client-credentials lodging of a UK account-access consent POST /aisp/account-access-consents (both v3.1.0 and v4.0.1) hard-failed with AuthenticatedUserIsRequired whenever no PSU was present, so a TPP could not lodge a consent with a client-credentials (app-only) token -- contradicting the UK spec's Step 2 (the TPP creates the consent as an app; the PSU authorises it later) and OBP's own doc comment on authoriseUKConsent, which describes exactly that client-credentials lodging model. Relax the check to reject only a fully anonymous request (no consumer and no user); a consumer-only context now lodges the consent with no bound PSU (mUserId stays null until authoriseUKConsent binds it after SCA), mirroring the Berlin Group native consent flow. saveUKConsent already takes user: Option[User], so a request that does carry a user (e.g. DirectLogin) is unchanged -- createdByUser just carries it through. The consent row grants nothing until authorised, so this does not widen data access; it only lets the spec-correct app-only lodging step work. The existing "authenticated -> 201" and "unauthenticated -> 401" scenarios still pass (401 now means fully-anonymous rather than no-PSU). The pure consumer-only path can't be exercised via the DirectLogin-based test harness (DirectLogin always binds a user), so it isn't asserted directly here. --- .../v3_1_0/Http4sUKOBv310AccountAccess.scala | 13 ++++++++----- .../v4_0_1/Http4sUKOBv401AccountInfo.scala | 16 +++++++++++----- 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/obp-api/src/main/scala/code/api/UKOpenBanking/v3_1_0/Http4sUKOBv310AccountAccess.scala b/obp-api/src/main/scala/code/api/UKOpenBanking/v3_1_0/Http4sUKOBv310AccountAccess.scala index 36351ec65e..14d2aa0886 100644 --- a/obp-api/src/main/scala/code/api/UKOpenBanking/v3_1_0/Http4sUKOBv310AccountAccess.scala +++ b/obp-api/src/main/scala/code/api/UKOpenBanking/v3_1_0/Http4sUKOBv310AccountAccess.scala @@ -47,10 +47,13 @@ object Http4sUKOBv310AccountAccess extends MdcLoggable { EndpointHelpers.executeFutureCreated(req) { implicit val cc: CallContext = req.callContext for { - u <- cc.user.toOption match { - case Some(user) => Future.successful(user) - case None => Future.failed(new RuntimeException(AuthenticatedUserIsRequired)) - } + // Client-credentials lodging: require some authentication (consumer or user) but not a + // PSU specifically; reject only a fully anonymous request. The PSU is bound later at + // authorise time. Mirrors the Berlin Group native consent flow and the v4.0.1 handler. + _ <- if (cc.user.isEmpty && cc.consumer.isEmpty) + Future.failed(new RuntimeException(AuthenticatedUserIsRequired)) + else Future.successful(()) + createdByUser = cc.user.toOption consentJson <- Future.fromTry(scala.util.Try( com.openbankproject.commons.util.JsonAliases.parse(cc.httpBody.getOrElse("{}")).extract[ConsentPostBodyUKV310] )) @@ -70,7 +73,7 @@ object Http4sUKOBv310AccountAccess extends MdcLoggable { consumerId = cc.consumer.map(_.consumerId.get) _ <- passesPsd2Aisp(Some(cc)) createdConsent <- Future(Consents.consentProvider.vend.saveUKConsent( - Some(u), + createdByUser, bankId = None, accountIds = None, consumerId = consumerId, diff --git a/obp-api/src/main/scala/code/api/UKOpenBanking/v4_0_1/Http4sUKOBv401AccountInfo.scala b/obp-api/src/main/scala/code/api/UKOpenBanking/v4_0_1/Http4sUKOBv401AccountInfo.scala index 0152026cd9..a2151997ef 100644 --- a/obp-api/src/main/scala/code/api/UKOpenBanking/v4_0_1/Http4sUKOBv401AccountInfo.scala +++ b/obp-api/src/main/scala/code/api/UKOpenBanking/v4_0_1/Http4sUKOBv401AccountInfo.scala @@ -94,10 +94,16 @@ object Http4sUKOBv401AccountInfo extends MdcLoggable { EndpointHelpers.executeFutureCreated(req) { implicit val cc: CallContext = req.callContext for { - u <- cc.user.toOption match { - case Some(user) => Future.successful(user) - case None => Future.failed(new RuntimeException(AuthenticatedUserIsRequired)) - } + // Spec Step 2: the TPP lodges the consent via a client-credentials grant -- authenticated + // as an app (consumer) but with no PSU yet. Require some authentication (a consumer or a + // user) but not a PSU specifically; reject only a fully anonymous request. The PSU is + // bound later at authorise time (mUserId stays null until then), mirroring the Berlin + // Group native consent flow (Http4sBGv13AIS.createConsent). A request with a real user + // (e.g. DirectLogin) still works -- createdByUser just carries it through. + _ <- if (cc.user.isEmpty && cc.consumer.isEmpty) + Future.failed(new RuntimeException(AuthenticatedUserIsRequired)) + else Future.successful(()) + createdByUser = cc.user.toOption consentJson <- Future.fromTry(scala.util.Try( JsonAliases.parse(cc.httpBody.getOrElse("{}")).extract[ConsentPostBodyUKV310] )) @@ -117,7 +123,7 @@ object Http4sUKOBv401AccountInfo extends MdcLoggable { consumerId = cc.consumer.map(_.consumerId.get) _ <- passesPsd2Aisp(Some(cc)) createdConsent <- Future(Consents.consentProvider.vend.saveUKConsent( - Some(u), + createdByUser, bankId = None, accountIds = None, consumerId = consumerId, From fc02d403d5d02100f11d7e5c1c9cef7f5bbf4a4f Mon Sep 17 00:00:00 2001 From: Hongwei Date: Sat, 18 Jul 2026 15:26:43 +0200 Subject: [PATCH 19/21] feat: emit x-fapi-interaction-id on UK v4.0.1 responses No UK v4.0.1 response carried x-fapi-interaction-id. The only related mechanism was a generic, opt-in, empty-by-default header-mirror prop that echoes a request header verbatim if present -- it never generated one when the TPP omitted it, so interaction tracing didn't work for the common case. (An earlier plan iteration pointed at getHeadersNewStyle for this, but that path is dead Lift-era code never called from the http4s stack.) Wrap the UK v4.0.1 aggregator's routes in an outer middleware that sets x-fapi-interaction-id on every response: echoed from the request when the TPP supplies one, otherwise a freshly generated UUID. Applied outside ResourceDocMiddleware so it also covers error responses. Does not introduce x-fapi-financial-id (correctly absent since v3.x). --- .../UKOpenBanking/v4_0_1/Http4sUKOBv401.scala | 18 ++++++++++++++- .../UKOpenBankingV401AccountInfoTests.scala | 22 +++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/obp-api/src/main/scala/code/api/UKOpenBanking/v4_0_1/Http4sUKOBv401.scala b/obp-api/src/main/scala/code/api/UKOpenBanking/v4_0_1/Http4sUKOBv401.scala index 4fe1f01288..c40362d530 100644 --- a/obp-api/src/main/scala/code/api/UKOpenBanking/v4_0_1/Http4sUKOBv401.scala +++ b/obp-api/src/main/scala/code/api/UKOpenBanking/v4_0_1/Http4sUKOBv401.scala @@ -2,11 +2,13 @@ package code.api.UKOpenBanking.v4_0_1 import cats.data.{Kleisli, OptionT} import cats.effect._ +import code.api.util.APIUtil import code.api.util.APIUtil.ResourceDoc import code.api.util.http4s.ResourceDocMiddleware import code.util.Helper.MdcLoggable import com.openbankproject.commons.util.ApiVersion import org.http4s._ +import org.typelevel.ci.CIString import scala.collection.mutable.ArrayBuffer @@ -46,5 +48,19 @@ object Http4sUKOBv401 extends MdcLoggable { .orElse(Http4sUKOBv401Vrp.routes(req)) } - val wrappedRoutes: HttpRoutes[IO] = ResourceDocMiddleware.apply(resourceDocs)(allRoutes) + private val fapiInteractionIdHeader = CIString("x-fapi-interaction-id") + + // FAPI: every response carries x-fapi-interaction-id -- echoed from the request if the TPP sent + // one (for end-to-end tracing), otherwise generated as a fresh UUID so tracing works even when + // the TPP omits it. Applied as an outer middleware so it covers every UK v4.0.1 response, + // including error responses produced inside ResourceDocMiddleware. + private def withFapiInteractionId(routes: HttpRoutes[IO]): HttpRoutes[IO] = + Kleisli[HttpF, Request[IO], Response[IO]] { req => + val interactionId = req.headers.get(fapiInteractionIdHeader) + .map(_.head.value) + .getOrElse(APIUtil.generateUUID()) + routes(req).map(_.putHeaders(Header.Raw(fapiInteractionIdHeader, interactionId))) + } + + val wrappedRoutes: HttpRoutes[IO] = withFapiInteractionId(ResourceDocMiddleware.apply(resourceDocs)(allRoutes)) } diff --git a/obp-api/src/test/scala/code/api/UKOpenBanking/v4_0_1/UKOpenBankingV401AccountInfoTests.scala b/obp-api/src/test/scala/code/api/UKOpenBanking/v4_0_1/UKOpenBankingV401AccountInfoTests.scala index a38f466d4e..cfa4912944 100644 --- a/obp-api/src/test/scala/code/api/UKOpenBanking/v4_0_1/UKOpenBankingV401AccountInfoTests.scala +++ b/obp-api/src/test/scala/code/api/UKOpenBanking/v4_0_1/UKOpenBankingV401AccountInfoTests.scala @@ -294,6 +294,28 @@ class UKOpenBankingV401AccountInfoTests extends UKOpenBankingV401ServerSetup { // nonce/timestamp replay check (a harness artifact -- production UK uses OAuth2 Bearer). } + // Gap 15: every UK v4.0.1 response carries x-fapi-interaction-id (FAPI tracing). Uses a stub + // endpoint that returns 200 so the assertion is purely about the header, independent of consent. + feature("UKOB v4.0.1 x-fapi-interaction-id response header") { + scenario("generated as a UUID when the request omits it", UKOpenBankingV401AccountInfo) { + val response = getAuthed("aisp", "accounts", "fake-accountid", "beneficiaries") + response.code should equal(200) + val interactionId = response.headers.map(_.get("x-fapi-interaction-id")).orNull + interactionId should not be null + interactionId should not be empty + // a generated value is a UUID + java.util.UUID.fromString(interactionId).toString should equal(interactionId) + } + scenario("echoed verbatim when the request supplies it", UKOpenBankingV401AccountInfo) { + val supplied = "test-interaction-id-12345" + val response = makeGetRequest( + v401("aisp", "accounts", "fake-accountid", "beneficiaries").GET <@ (user1), + List(("x-fapi-interaction-id", supplied))) + response.code should equal(200) + response.headers.map(_.get("x-fapi-interaction-id")).orNull should equal(supplied) + } + } + // ── AccountsApi ──────────────────────────────────────────────────── // checkUKConsent extracts the `consent_id` claim from the Bearer access token (no external // Hydra call since Consent.checkUKConsent dropped the Hydra dependency). These OAuth1-signed From f7b58e8425655d059432c9c57a01784b96995ce7 Mon Sep 17 00:00:00 2001 From: Hongwei Date: Sat, 18 Jul 2026 16:50:29 +0200 Subject: [PATCH 20/21] feat: add jwks_uri field to Consumer for OIDC client key storage FAPI 1.0 Advanced needs a place to resolve a client's public key when verifying signed request objects and private_key_jwt client assertions. Adds a jwksUri Mapper field to Consumer (auto-migrates, same pattern as the existing unused clientCertificate field) and exposes it as jwks_uri through both the read-only and admin OIDC consumer views. --- obp-api/src/main/scala/code/model/OAuth.scala | 3 +++ obp-api/src/main/scripts/sql/OIDC/cre_v_oidc_admin_clients.sql | 1 + obp-api/src/main/scripts/sql/OIDC/cre_v_oidc_clients.sql | 3 ++- 3 files changed, 6 insertions(+), 1 deletion(-) diff --git a/obp-api/src/main/scala/code/model/OAuth.scala b/obp-api/src/main/scala/code/model/OAuth.scala index ed88cb1cf6..a0ebb6e109 100644 --- a/obp-api/src/main/scala/code/model/OAuth.scala +++ b/obp-api/src/main/scala/code/model/OAuth.scala @@ -647,6 +647,9 @@ class Consumer extends LongKeyedMapper[Consumer] with CreatedUpdated{ override def defaultValue : Long = APIUtil.getPropsAsLongValue("rate_limiting_per_month", -1) } object clientCertificate extends MappedString(this, 4000) + // FAPI 1.0 Advanced: URL where this client publishes its JWKS, used to verify + // signed request objects and private_key_jwt client assertions (OBP-OIDC). + object jwksUri extends MappedString(this, 500) object company extends MappedString(this, 100) { override def displayName = "Company:" } diff --git a/obp-api/src/main/scripts/sql/OIDC/cre_v_oidc_admin_clients.sql b/obp-api/src/main/scripts/sql/OIDC/cre_v_oidc_admin_clients.sql index 76bc983f3e..20f2b9a0a0 100644 --- a/obp-api/src/main/scripts/sql/OIDC/cre_v_oidc_admin_clients.sql +++ b/obp-api/src/main/scripts/sql/OIDC/cre_v_oidc_admin_clients.sql @@ -28,6 +28,7 @@ name ,logourl ,userauthenticationurl ,clientcertificate +,jwksuri ,company ,key_c ,isactive diff --git a/obp-api/src/main/scripts/sql/OIDC/cre_v_oidc_clients.sql b/obp-api/src/main/scripts/sql/OIDC/cre_v_oidc_clients.sql index 4dbfbf1f66..767dc99a1f 100644 --- a/obp-api/src/main/scripts/sql/OIDC/cre_v_oidc_clients.sql +++ b/obp-api/src/main/scripts/sql/OIDC/cre_v_oidc_clients.sql @@ -25,7 +25,8 @@ SELECT name as client_name, 'code' as response_types, 'client_secret_post' as token_endpoint_auth_method, - createdat as created_at + createdat as created_at, + jwksuri as jwks_uri FROM consumer WHERE isactive = true -- Only expose active consumers to OIDC service ORDER BY client_name; From 2cbc37e5e68b3a2d0a67f86fe92bf81bd6c8c2c5 Mon Sep 17 00:00:00 2001 From: Hongwei Date: Sat, 18 Jul 2026 17:46:25 +0200 Subject: [PATCH 21/21] feat: expose client_certificate through the read-only OIDC clients view FAPI 1.0 Advanced's tls_client_auth needs the client's registered certificate at token-request time, not just in the admin view. The Consumer.clientCertificate field already exists (unused); this just adds it to v_oidc_clients alongside the existing jwks_uri column. --- obp-api/src/main/scripts/sql/OIDC/cre_v_oidc_clients.sql | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/obp-api/src/main/scripts/sql/OIDC/cre_v_oidc_clients.sql b/obp-api/src/main/scripts/sql/OIDC/cre_v_oidc_clients.sql index 767dc99a1f..aa0c2bd842 100644 --- a/obp-api/src/main/scripts/sql/OIDC/cre_v_oidc_clients.sql +++ b/obp-api/src/main/scripts/sql/OIDC/cre_v_oidc_clients.sql @@ -26,7 +26,8 @@ SELECT 'code' as response_types, 'client_secret_post' as token_endpoint_auth_method, createdat as created_at, - jwksuri as jwks_uri + jwksuri as jwks_uri, + clientcertificate as client_certificate FROM consumer WHERE isactive = true -- Only expose active consumers to OIDC service ORDER BY client_name;