Enforce UK consent check on aggregate endpoints, upgrade 403 assertions, fix build-script hang

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.

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"

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))

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),

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 @@
 // 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 @@
     }
   }
   // ── 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 @@
     }
   }
   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,34 +250,23 @@
     }
   }
   // ── 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)
     }
   }
 
   // ── 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 @@
       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)