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. 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/main/scala/code/api/util/ConsentUtil.scala b/obp-api/src/main/scala/code/api/util/ConsentUtil.scala index ef4c4ab9a6..d0ba73f44c 100644 --- a/obp-api/src/main/scala/code/api/util/ConsentUtil.scala +++ b/obp-api/src/main/scala/code/api/util/ConsentUtil.scala @@ -426,7 +426,15 @@ object Consent extends MdcLoggable { logger.debug(s"applyConsentRulesCommonOldStyle.getSignedPayloadAsJson.End of com.openbankproject.commons.util.JsonAliases.parse(jsonAsString).extract[ConsentJWT]: $consent") checkConsent(consent, consentIdAsJwt, calContext) match { // Check is it Consent-JWT expired case (Full(true)) => // OK - applyConsentRules(consent) + // A Consent-JWT / Consent-Id is an OBP-native gate: reject a consent created by + // another standard. A JWT with no backing MappedConsent row is grandfathered. + Consents.consentProvider.vend.getConsentByConsentId(consent.jti) match { + case Full(mc) => assertConsentStandard(mc, ConsentStandardOBP) match { + case Some(failure) => failure + case None => applyConsentRules(consent) + } + case _ => applyConsentRules(consent) + } case failure@Failure(_, _, _) => // Handled errors failure case _ => // Unexpected errors @@ -506,7 +514,15 @@ object Consent extends MdcLoggable { logger.debug(s"applyConsentRulesCommon.End of com.openbankproject.commons.util.JsonAliases.parse(jsonAsString).extract[ConsentJWT]: $consent") checkConsent(consent, consentAsJwt, callContext) match { // Check is it Consent-JWT expired case (Full(true)) => // OK - applyConsentRules(consent) + // A Consent-JWT / Consent-Id is an OBP-native gate: reject a consent created by + // another standard. A JWT with no backing MappedConsent row is grandfathered. + Consents.consentProvider.vend.getConsentByConsentId(consent.jti) match { + case Full(mc) => assertConsentStandard(mc, ConsentStandardOBP) match { + case Some(failure) => Future(failure, Some(callContext)) + case None => applyConsentRules(consent) + } + case _ => applyConsentRules(consent) + } case failure@Failure(_, _, _) => // Handled errors Future(failure, Some(callContext)) case _ => // Unexpected errors @@ -618,6 +634,9 @@ object Consent extends MdcLoggable { // 1st we need to find a Consent via the field MappedConsent.consentId Consents.consentProvider.vend.getConsentByConsentId(consentId) match { + // A consent may only be exercised by its own standard: reject a non-BG consent here. + case Full(storedConsent) if assertConsentStandard(storedConsent, ConsentStandardBG).isDefined => + Future(assertConsentStandard(storedConsent, ConsentStandardBG).get, Some(callContext)) case Full(storedConsent) => val user = Users.users.vend.getUserByUserId(storedConsent.userId) logger.debug(s"applyBerlinGroupConsentRulesCommon.storedConsent.user : $user") @@ -1107,6 +1126,38 @@ object Consent extends MdcLoggable { } + // Canonical apiStandard values stamped on a MappedConsent at creation time. A consent may + // only be *exercised* (used for data access) through endpoints of the standard that created + // it — enforced by assertConsentStandard at each exercise gate. Reduction (revoke/expire) and + // read/audit deliberately stay cross-standard; the OBP-hosted authorise ceremony acts on a + // consent as its own standard, so it is not blocked here. + val ConsentStandardOBP: String = ApiStandards.obp.toString // "obp" + val ConsentStandardBG: String = ConstantsBG.berlinGroupVersion1.apiStandard // "BG" + val ConsentStandardUK: String = "UKOpenBanking" + + /** + * Check whether a stored consent may be exercised by the standard now using it. + * Returns None when allowed, Some(Failure) on a known mismatch. Returning the Failure (a + * `Box[Nothing]`) lets callers propagate it into any `Box[User]` / `Box[Boolean]` result. + * + * Option A (grandfathering): a legacy consent whose apiStandard is blank/null is allowed + * (with a warning) so pre-existing consents keep working; only a KNOWN mismatch is rejected. + * Every live creator now stamps a standard, so blank means "old row" — backfill mApiStandard + * to 'obp' to remove the grandfathering path and tighten to strict equality later. + */ + def assertConsentStandard(storedConsent: MappedConsent, expectedStandard: String): Option[Failure] = { + val actualStandard = Option(storedConsent.apiStandard).map(_.trim).getOrElse("") + if (actualStandard.isEmpty) { + logger.warn(s"assertConsentStandard: consent ${storedConsent.consentId} has no apiStandard; " + + s"grandfathering it for '$expectedStandard' access. Consider backfilling mApiStandard.") + None + } else if (actualStandard == expectedStandard) { + None + } else { + Some(Failure(s"${ErrorMessages.ConsentDoesNotMatchStandard}Consent standard: $actualStandard, required: $expectedStandard.")) + } + } + /** * Validates the UK Open Banking AIS consent bound to the presented OAuth2 access token. * @@ -1115,8 +1166,13 @@ object Consent extends MdcLoggable { * authorisation flow. Signature, issuer and expiry of the token were already verified by the * authentication layer (OAuth2Login issuer dispatch) before any endpoint reaches this check, * so only the claim is extracted here. The OBP database remains authoritative for consent - * state: the status/consumer checks below run on every request, so revoking a consent takes - * effect immediately even though an already-issued JWT cannot itself be revoked. + * state: the status/user/consumer checks below run on every request, so revoking a consent + * takes effect immediately even though an already-issued JWT cannot itself be revoked. + * + * The user-binding check matters because the consent_id claim is NOT validated by the IdP: + * it must only pass for the PSU who authorised the consent (bound via updateConsentUser in + * the authorise step), otherwise any user of the same consumer could present a foreign + * consent_id and exercise a consent they never authorised. */ def checkUKConsent(user: User, calContext: Option[CallContext]): Box[Boolean] = { val accessToken = calContext.flatMap(_.authReqHeaderField) @@ -1129,22 +1185,29 @@ object Consent extends MdcLoggable { } boxedConsent match { - case Full(c) if c.mStatus.toString().toUpperCase() == ConsentStatus.AUTHORISED.toString => - System.currentTimeMillis match { - case currentTimeMillis if currentTimeMillis < c.creationDateTime.getTime => - Failure(ErrorMessages.ConsentNotBeforeIssue) + case Full(c) => assertConsentStandard(c, ConsentStandardUK) match { + case Some(failure) => failure // Wrong standard — reject before status/user checks + case None => c.mStatus.toString().toUpperCase() match { + case status if status == ConsentStatus.AUTHORISED.toString => + System.currentTimeMillis match { + case currentTimeMillis if currentTimeMillis < c.creationDateTime.getTime => + Failure(ErrorMessages.ConsentNotBeforeIssue) + case _ if c.mUserId.get != user.userId => + Failure(ErrorMessages.ConsentDoesNotMatchUser) + case _ => + val consumerIdOfLoggedInUser: Option[String] = calContext.flatMap(_.consumer.map(_.consumerId.get)) + implicit val dateFormats = CustomJsonFormats.formats + val consent: Box[ConsentJWT] = JwtUtil.getSignedPayloadAsJson(c.jsonWebToken) + .map(parse(_).extract[ConsentJWT]) + checkConsumerIsActiveAndMatchedUK( + consent.openOrThrowException("Parsing of the consent failed."), + consumerIdOfLoggedInUser + ) + } case _ => - val consumerIdOfLoggedInUser: Option[String] = calContext.flatMap(_.consumer.map(_.consumerId.get)) - implicit val dateFormats = CustomJsonFormats.formats - val consent: Box[ConsentJWT] = JwtUtil.getSignedPayloadAsJson(c.jsonWebToken) - .map(parse(_).extract[ConsentJWT]) - checkConsumerIsActiveAndMatchedUK( - consent.openOrThrowException("Parsing of the consent failed."), - consumerIdOfLoggedInUser - ) + Failure(s"${ErrorMessages.ConsentStatusIssue}${ConsentStatus.AUTHORISED.toString}.") } - case Full(c) if c.mStatus.toString().toUpperCase() != ConsentStatus.AUTHORISED.toString => - Failure(s"${ErrorMessages.ConsentStatusIssue}${ConsentStatus.AUTHORISED.toString}.") + } case _ => Failure(ErrorMessages.ConsentNotFound) } diff --git a/obp-api/src/main/scala/code/api/util/ErrorMessages.scala b/obp-api/src/main/scala/code/api/util/ErrorMessages.scala index cd330ed417..b11cf0fbb6 100644 --- a/obp-api/src/main/scala/code/api/util/ErrorMessages.scala +++ b/obp-api/src/main/scala/code/api/util/ErrorMessages.scala @@ -769,6 +769,7 @@ object ErrorMessages { val RolesForbiddenInConsent = s"OBP-35033: Consents cannot contain the following Roles: ${canCreateEntitlementAtOneBank} and ${canCreateEntitlementAtAnyBank}." val UserAuthContextUpdateRequestAllowedScaMethods = "OBP-35034: Unsupported as SCA method. " val ConsentIdClaimMissing = "OBP-35035: The access token is not bound to a Consent. The identity provider must include a consent_id claim in access tokens issued via the consent authorisation flow. " + val ConsentDoesNotMatchStandard = "OBP-35036: The Consent was created by a different API standard than the endpoint using it. A consent may only be used by endpoints of the standard that created it. " //Authorisations val AuthorisationNotFound = "OBP-36001: Authorisation not found. Please specify valid values for PAYMENT_ID and AUTHORISATION_ID. " @@ -1068,6 +1069,8 @@ object ErrorMessages { ConsentDisabled -> 401, // 403 not 401: the token authenticated fine, it just isn't bound to a consent (UK OB: insufficient authorisation) ConsentIdClaimMissing -> 403, + // 403: authenticated + a real consent, but it belongs to another API standard — forbidden here. + ConsentDoesNotMatchStandard -> 403, InternalServerError -> 500, ) 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..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 @@ -12,12 +12,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 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 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 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). @@ -103,7 +108,7 @@ class UKOpenBankingV401AccountInfoTests extends UKOpenBankingV401ServerSetup { } } // ── AccountsApi ──────────────────────────────────────────────────── - // DATA-DEPENDENT: checkUKConsent requires live Hydra (see class doc above). + // DATA-DEPENDENT: checkUKConsent requires a consent-bound token (consent_id claim — see class doc above). feature("UKOB v4.0.1 GET /aisp/accounts") { scenario("authenticated", UKOpenBankingV401AccountInfo) { getAuthed("aisp", "accounts").code should not equal (401) @@ -236,7 +241,7 @@ class UKOpenBankingV401AccountInfoTests extends UKOpenBankingV401ServerSetup { } } // ── TransactionsApi ──────────────────────────────────────────────── - // DATA-DEPENDENT: checkUKConsent requires live Hydra (see class doc above). + // DATA-DEPENDENT: checkUKConsent requires a consent-bound token (consent_id claim — see class doc above). feature("UKOB v4.0.1 GET /aisp/accounts/ACCOUNT_ID/transactions") { scenario("authenticated", UKOpenBankingV401AccountInfo) { getAuthed("aisp", "accounts", acc, "transactions").code should not equal (401) @@ -247,23 +252,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) @@ -333,23 +325,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)