diff --git a/flushall_build_and_run.sh b/flushall_build_and_run.sh index 13a8bfa6c5..2d92353971 100755 --- a/flushall_build_and_run.sh +++ b/flushall_build_and_run.sh @@ -155,15 +155,24 @@ 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=$! + # 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 " Log: http4s-server.log (also $RUNTIME_LOG)" + echo " PORT: ${SERVER_PORT:-8080}" + 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. 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" 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..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(account.bankId, transactions, moderatedAttributes, view) + } yield JSONFactory_UKOpenBanking_401.createTransactionsJsonNew(account.bankId, accountId.value, 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..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 @@ -294,11 +294,11 @@ object JSONFactory_UKOpenBanking_401 extends CustomJsonFormats { def createTransactionsJsonNew( bankId: BankId, + accountId: String, 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 97d1ec205e..50ce214ca5 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,13 +14,17 @@ 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 the Bearer access token -// to be a JWT carrying a consent_id claim bound to an AUTHORISED consent of the -// calling user+consumer (see code.api.util.ConsentUtil.checkUKConsent). The test -// framework's DirectLogin tokens carry no consent_id, 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). +// getBalances / getTransactions (the account-aggregate variants) share the same +// checkUKConsent guard but only assert "unauthenticated -> 401" / "authenticated -> +// not 401" (mirroring UKOpenBankingV310AisTests' precedent): they 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). @@ -104,10 +110,15 @@ class UKOpenBankingV401AccountInfoTests extends UKOpenBankingV401ServerSetup { } } // ── AccountsApi ──────────────────────────────────────────────────── - // DATA-DEPENDENT: checkUKConsent requires a consent-bound token (consent_id claim — 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) @@ -133,8 +144,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) @@ -237,10 +250,12 @@ class UKOpenBankingV401AccountInfoTests extends UKOpenBankingV401ServerSetup { } } // ── TransactionsApi ──────────────────────────────────────────────── - // DATA-DEPENDENT: checkUKConsent requires a consent-bound token (consent_id claim — 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) @@ -248,23 +263,10 @@ class UKOpenBankingV401AccountInfoTests extends UKOpenBankingV401ServerSetup { } // ── BalancesApi ──────────────────────────────────────────────────── + // DATA-DEPENDENT: checkUKConsent requires a consent-bound token (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) @@ -334,23 +336,10 @@ class UKOpenBankingV401AccountInfoTests extends UKOpenBankingV401ServerSetup { getUnauthed("aisp", "statements").code should equal(401) } } + // DATA-DEPENDENT: checkUKConsent requires a consent-bound token (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)