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..97d1ec205e 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 @@ -13,11 +13,12 @@ import org.scalatest.Tag // 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. +// 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. // // The remaining 80 endpoints are still static spec-faithful stubs; their tests // are unchanged (two scenarios: authenticated -> fixed code, unauthenticated -> 401). @@ -103,7 +104,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 +237,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)