From 24caf7ce9a0d07b1c4011e612ab20b85049ca7f0 Mon Sep 17 00:00:00 2001 From: simonredfern Date: Fri, 16 Oct 2026 17:15:39 +0200 Subject: [PATCH 01/30] Glossary item for Sandbox Populator --- .../main/scala/code/api/util/Glossary.scala | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/obp-api/src/main/scala/code/api/util/Glossary.scala b/obp-api/src/main/scala/code/api/util/Glossary.scala index 3467c5b399..8659803538 100644 --- a/obp-api/src/main/scala/code/api/util/Glossary.scala +++ b/obp-api/src/main/scala/code/api/util/Glossary.scala @@ -3195,6 +3195,44 @@ object Glossary extends MdcLoggable { ${getWebUiPropsValue("webui_sandbox_introduction", "")} """) + glossaryItems += GlossaryItem( + title = "Sandbox Populator", + description = + s"""| +|The OBP Sandbox Populator is used to create bank level data for a logged in developer that needs test data. +| +|The User logs in to it via OAuth2, and it calls OBP endpoints on behalf of that User. Everything it creates is therefore normal OBP data that can be explored afterwards with the API Explorer, API Manager or any REST client. +| +|### What it creates +| +|* Banks +|* Accounts (with Customer Account Links) +|* Customers (individual and corporate), Customer Links and User Customer Links +|* Counterparties +|* FX Rates +|* Transactions (via Transaction Requests or historical Transactions) +| +|Data sets are available for several countries (e.g. Botswana and Singapore) which determine the business names, customer templates, currencies and transaction patterns used. +| +|### How to use it +| +|* **Populate:** choose the number of Banks, the number of Accounts per Bank, a country, and which entities to create, then run. The results link to the API Explorer for verification. +|* **CSV Import:** upload CSV files of Banks, Accounts, Customers and Transactions to create them in bulk. +| +|### Roles +| +|The logged in User needs the relevant Entitlements for the data being created, for instance CanCreateCustomer, CanCreateAccount, CanCreateCustomerAccountLink, CanCreateUserCustomerLink and CanCreateHistoricalTransactionAtBank. OBP error messages (including missing Role errors) are shown as returned by the API. +| +|### Running it +| +|The Sandbox Populator is a SvelteKit application that uses Redis for session storage. It can be run with Docker Compose alongside a local OBP API and Redis, or locally with Node. +| +|Source code and documentation: [OBP Sandbox Populator](https://github.com/OpenBankProject/OBP-Sandbox-Populator) +| +|See also: ${getGlossaryItemLink("Sandbox Introduction")} +| +""") + glossaryItems += GlossaryItem( title = "Data Model Overview", From c774b1cb7218a75cdd6cf4c3041cc65920322180 Mon Sep 17 00:00:00 2001 From: simonredfern Date: Fri, 16 Oct 2026 17:29:23 +0200 Subject: [PATCH 02/30] =?UTF-8?q?fix:=20return=20400=20for=20malformed=20E?= =?UTF-8?q?TH=5FSEND=5FRAW=5FTRANSACTION=20params=20=E2=80=94=20DecodeRawT?= =?UTF-8?q?x=20now=20throws=20IllegalArgumentException=20and=20the=20conne?= =?UTF-8?q?ctor=20wraps=20the=20decode=20in=20tryons=20+=20adds=20unit=20a?= =?UTF-8?q?nd=20v6.0.0=20endpoint=20tests.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../LocalMappedConnectorInternal.scala | 8 +- .../bankconnectors/ethereum/DecodeRawTx.scala | 14 +-- .../EthSendRawTransactionRequestTest.scala | 85 +++++++++++++++++++ .../ethereum/DecodeRawTxTest.scala | 18 ++++ 4 files changed, 117 insertions(+), 8 deletions(-) create mode 100644 obp-api/src/test/scala/code/api/v6_0_0/EthSendRawTransactionRequestTest.scala diff --git a/obp-api/src/main/scala/code/bankconnectors/LocalMappedConnectorInternal.scala b/obp-api/src/main/scala/code/bankconnectors/LocalMappedConnectorInternal.scala index ee97aec326..922afb8b63 100644 --- a/obp-api/src/main/scala/code/bankconnectors/LocalMappedConnectorInternal.scala +++ b/obp-api/src/main/scala/code/bankconnectors/LocalMappedConnectorInternal.scala @@ -729,7 +729,13 @@ object LocalMappedConnectorInternal extends MdcLoggable { json.extract[TransactionRequestBodyEthSendRawTransactionJsonV600] } // Decode raw transaction to extract 'from' address - decodedTx = DecodeRawTx.decodeRawTxToJson(transactionRequestBodyEthSendRawTransactionJsonV600.params) + decodedTx <- NewStyle.function.tryons( + s"$InvalidJsonValue params must be a valid signed raw Ethereum transaction", + 400, + callContext + ) { + DecodeRawTx.decodeRawTxToJson(transactionRequestBodyEthSendRawTransactionJsonV600.params) + } from = decodedTx.from _ <- Helper.booleanToFuture( s"$BankAccountNotFoundByAccountId Ethereum 'from' address must be the same as the accountId", diff --git a/obp-api/src/main/scala/code/bankconnectors/ethereum/DecodeRawTx.scala b/obp-api/src/main/scala/code/bankconnectors/ethereum/DecodeRawTx.scala index 93fe938e8e..d9e26c11fa 100644 --- a/obp-api/src/main/scala/code/bankconnectors/ethereum/DecodeRawTx.scala +++ b/obp-api/src/main/scala/code/bankconnectors/ethereum/DecodeRawTx.scala @@ -28,6 +28,7 @@ TESOBE (http://www.tesobe.com/) package code.bankconnectors.ethereum import java.math.BigInteger +import scala.util.control.NonFatal import org.web3j.crypto.{Hash, RawTransaction, TransactionDecoder, Sign, SignedRawTransaction} import org.web3j.utils.{Numeric => W3Numeric} import org.json4s._ @@ -69,12 +70,10 @@ object DecodeRawTx { estimatedFee: Option[String] ) - private def fatal(msg: String): Nothing = { - Console.err.println(msg) - sys.exit(1) - } - - // File/stdin helpers removed; input is provided as a function parameter now. + // This decoder runs on request threads, so invalid input must fail the call, never the process: + // the caller turns the exception into a 400. + private def fatal(msg: String): Nothing = + throw new IllegalArgumentException(msg) private def normalizeHex(hex: String): String = { val h = Option(hex).getOrElse("").trim @@ -111,6 +110,7 @@ object DecodeRawTx { /** * Decode raw Ethereum transaction hex and return a JSON string summarizing the fields. * The input must be a 0x-prefixed hex string; no file or stdin reading is performed. + * Throws IllegalArgumentException when the input is not valid hex or does not decode as a transaction. * * Response is serialized from DecodedTxResponse case class with types: * - type, chainId, nonce, value are numeric (where available) @@ -125,7 +125,7 @@ object DecodeRawTx { val decoded: RawTransaction = try TransactionDecoder.decode(rawHex) catch { - case e: Throwable => fatal(s"decode failed: ${e.getMessage}") + case NonFatal(e) => fatal(s"decode failed: ${e.getMessage}") } val (fromOpt, chainIdOpt, vHexOpt, rHexOpt, sHexOpt): diff --git a/obp-api/src/test/scala/code/api/v6_0_0/EthSendRawTransactionRequestTest.scala b/obp-api/src/test/scala/code/api/v6_0_0/EthSendRawTransactionRequestTest.scala new file mode 100644 index 0000000000..e8792ca199 --- /dev/null +++ b/obp-api/src/test/scala/code/api/v6_0_0/EthSendRawTransactionRequestTest.scala @@ -0,0 +1,85 @@ +/** +Open Bank Project - API +Copyright (C) 2011-2026, TESOBE GmbH. + +This program is free software: you can redistribute it and/or modify +it under the terms of the GNU Affero General Public License as published by +the Free Software Foundation, either version 3 of the License, or +(at your option) any later version. + +This program is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU Affero General Public License for more details. + +You should have received a copy of the GNU Affero General Public License +along with this program. If not, see . + +Email: contact@tesobe.com +TESOBE GmbH. +Osloer Strasse 16/17 +Berlin 13359, Germany + +This product includes software developed at +TESOBE (http://www.tesobe.com/) + + */ +package code.api.v6_0_0 + +import org.json4s._ +import code.api.Constant +import code.api.util.APIUtil.OAuth._ +import code.api.util.ErrorMessages._ +import code.api.v6_0_0.Http4s600.Implementations6_0_0 +import com.github.dwickern.macros.NameOf.nameOf +import com.openbankproject.commons.model.ErrorMessage +import com.openbankproject.commons.util.ApiVersion +import org.json4s.native.Serialization.write +import org.scalatest.Tag + +/** + * This suite checks that an ETH_SEND_RAW_TRANSACTION request whose params are not a valid signed raw + * Ethereum transaction is answered with a 400. The raw transaction decoder used to call sys.exit(1) on + * such input, so one bad request stopped the whole server; the last scenario checks the server still + * answers after the bad requests. + */ +class EthSendRawTransactionRequestTest extends V600ServerSetup { + + object VersionOfApi extends Tag(ApiVersion.v6_0_0.toString) + object CreateTransactionRequestEthSendRawTransaction extends Tag(nameOf(Implementations6_0_0.createTransactionRequestEthSendRawTransaction)) + + private def createEthSendRawTransactionRequest(params: String) = { + setPropsValues("transactionRequests_enabled" -> "true") + setPropsValues("transactionRequests_supported_types" -> "ETH_SEND_RAW_TRANSACTION") + val request = (v6_0_0_Request / "banks" / testBankId1.value / "accounts" / testAccountId1.value / + Constant.SYSTEM_OWNER_VIEW_ID / "transaction-request-types" / "ETH_SEND_RAW_TRANSACTION" / "transaction-requests").POST <@ (user1) + makePostRequest(request, write(TransactionRequestBodyEthSendRawTransactionJsonV600(params = params, description = "malformed raw transaction"))) + } + + feature("Create ETH_SEND_RAW_TRANSACTION transaction request with malformed params - v6.0.0") { + + List( + "hello", // no 0x prefix + "0xzz", // non-hex characters + "0xabc", // odd length + "0x00ff00ff00ff" // valid hex that does not decode as a transaction + ).foreach { malformedParams => + scenario(s"params '$malformedParams' are answered with 400", CreateTransactionRequestEthSendRawTransaction, VersionOfApi) { + When(s"We create an ETH_SEND_RAW_TRANSACTION transaction request with params '$malformedParams'") + val response = createEthSendRawTransactionRequest(malformedParams) + Then("We should get a 400") + response.code should equal(400) + And("error should be " + InvalidJsonValue) + response.body.extract[ErrorMessage].message should include(InvalidJsonValue) + } + } + + scenario("The server still answers after malformed params", CreateTransactionRequestEthSendRawTransaction, VersionOfApi) { + When("We send malformed params and then a second request") + createEthSendRawTransactionRequest("0xzz").code should equal(400) + val response = makeGetRequest((v6_0_0_Request / "root").GET) + Then("The second request is answered with 200") + response.code should equal(200) + } + } +} diff --git a/obp-api/src/test/scala/code/bankconnectors/ethereum/DecodeRawTxTest.scala b/obp-api/src/test/scala/code/bankconnectors/ethereum/DecodeRawTxTest.scala index 91fbfc41f1..5c9afbf105 100644 --- a/obp-api/src/test/scala/code/bankconnectors/ethereum/DecodeRawTxTest.scala +++ b/obp-api/src/test/scala/code/bankconnectors/ethereum/DecodeRawTxTest.scala @@ -76,5 +76,23 @@ class DecodeRawTxTest extends FeatureSpec with Matchers with GivenWhenThen { response.estimatedFee shouldBe defined response.estimatedFee.get should startWith ("0x") } + + scenario("Malformed input throws IllegalArgumentException instead of stopping the JVM") { + Given("inputs that are not valid signed raw transaction hex") + val malformedInputs = List( + "hello", // no 0x prefix + "0xzz", // non-hex characters + "0xabc", // odd length + "0x", // empty body + "0x00ff00ff00ff", // valid hex that does not decode as a transaction + null + ) + + malformedInputs.foreach { malformedInput => + When(s"we decode '$malformedInput'") + Then("an IllegalArgumentException is thrown, so the caller can answer 400") + an [IllegalArgumentException] should be thrownBy DecodeRawTx.decodeRawTxToJson(malformedInput) + } + } } } \ No newline at end of file From 4193f77d19428a49e7ce38c7b0ac9ce3705c30d1 Mon Sep 17 00:00:00 2001 From: simonredfern Date: Sat, 17 Oct 2026 14:12:16 +0200 Subject: [PATCH 03/30] fix: pass the CallContext into createTransactionRequest instead of reading the user from the Lift-era SS thread-global. v4.0.0 routes drop their SS.init wrappers and the unused SS object is removed. --- .../main/scala/code/api/util/APIUtil.scala | 98 +------------------ .../scala/code/api/v4_0_0/Http4s400.scala | 32 ++---- .../scala/code/api/v6_0_0/Http4s600.scala | 2 +- .../LocalMappedConnectorInternal.scala | 5 +- 4 files changed, 15 insertions(+), 122 deletions(-) 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 ad07526837..83e2855fd6 100644 --- a/obp-api/src/main/scala/code/api/util/APIUtil.scala +++ b/obp-api/src/main/scala/code/api/util/APIUtil.scala @@ -1823,7 +1823,7 @@ object APIUtil extends MdcLoggable with CustomJsonFormats{ * runtime-compiled dynamic-endpoint dispatch. Runs the SAME ordered checks — authentication, * obp-id format, bank, roles, account, view, counterparty — with the same predicates and *Fun * helpers, returning the enriched CallContext (user set) for a native handler instead of - * wrapping a Lift OBPEndpoint. No S.init / SS / Box[JsonResponse]; the compiled native body + * wrapping a Lift OBPEndpoint. No S.init / Box[JsonResponse]; the compiled native body * looks up bank/account/view itself, so the entities validated here serve only 404/403 gating, * exactly as the Lift checks did. Auth/role/lookup failures fail the Future (the dispatcher * converts them to a response via ErrorResponseConverter). @@ -1891,102 +1891,6 @@ object APIUtil extends MdcLoggable with CustomJsonFormats{ }else operationId } - /** - * Simulate S pass request and session, this object pass bank, account and view. - * This method invoke must fulfill three point: - * 1. be invoked at endpoint's body - * 2. be invoked out of for comprehension - * 3. endpoint's corresponding ResourceDoc.errorResponseBodies must contains $BankNotFound, $BankAccountNotFound or $UserNoPermissionAccessView - */ - object SS { - private val _user = new ThreadGlobal[User] - private val _bank = new ThreadGlobal[Bank] - private val _bankAccount = new ThreadGlobal[BankAccount] - private val _view = new ThreadGlobal[View] - private val _callContext = new ThreadGlobal[Option[CallContext]] - - def init[B](boxUser: Box[User], bank: Bank, bankAccount: BankAccount, view: View, callContext: Option[CallContext])(f: => B):B = { - _user.doWith(boxUser.orNull){ - _bank.doWith(bank) { - _bankAccount.doWith(bankAccount) { - _view.doWith(view) { - _callContext.doWith(callContext){ - f - } - } - } - } - } - } - - private def bank: Bank = _bank.box.openOrThrowException(buildErrorMsg(nameOf($BankNotFound))) - private def bankAccount: BankAccount = _bankAccount.box.openOrThrowException(buildErrorMsg(nameOf($BankAccountNotFound))) - private def getView: View = _view.box.openOrThrowException(buildErrorMsg(nameOf($UserNoPermissionAccessView))) - private def callContext: Option[CallContext] = _callContext.box.orNull - - /** - * Get current login user, recommend call cc.loggedInUser instead. - * Note, the same as S.session of lift framework, the method only can be called at first line of endpoint for comprehension or out of for comprehension. - * @return current login user - */ - def user: Future[(Box[User], Option[CallContext])] = Future.successful(_user.box -> callContext) - - /** - * Get current login user, and bank find by url /BANK_ID/. - * Validation of id exists passed. - * Note, the same as S.session of lift framework, the method only can be called at first line of endpoint for comprehension or out of for comprehension. - */ - def userBank: Future[(Box[User], Bank, Option[CallContext])] = Future.successful((_user.box, bank, callContext)) - - def userBankView: Future[(Box[User], Bank, View, Option[CallContext])] = Future.successful((_user.box, bank, getView, callContext)) - - /** - * Get current login user and bank find by url /BANK_ID/ and account find by /ACCOUNT_ID/. - * Validation of id exists passed. - * Note, the same as S.session of lift framework, the method only can be called at first line of endpoint for comprehension or out of for comprehension. - */ - def userBankAccount: Future[(Box[User], Bank, BankAccount, Option[CallContext])] = Future.successful { - (_user.box, bank, bankAccount, callContext) - } - - /** - * Get current login user, and bank find by url /BANK_ID/, and account find by /ACCOUNT_ID/, and view find by /VIEW_ID/. - * Validation of id exists passed. - * Note, the same as S.session of lift framework, the method only can be called at first line of endpoint for comprehension or out of for comprehension. - */ - def userBankAccountView: Future[(Box[User], Bank, BankAccount, View, Option[CallContext])] = Future.successful { - (_user.box, bank, bankAccount, getView, callContext) - } - - /** - * Get view find by /VIEW_ID/. - * Validation of id exists passed. - * Note, the same as S.session of lift framework, the method only can be called at first line of endpoint for comprehension or out of for comprehension. - */ - def view: Future[(View, Option[CallContext])] = Future.successful(getView -> callContext) - /** - * Get current login user, and view find by /VIEW_ID/. - * Validation of id exists passed. - * Note, the same as S.session of lift framework, the method only can be called at first line of endpoint for comprehension or out of for comprehension. - */ - def userView: Future[(Box[User], View, Option[CallContext])] = Future.successful((_user.box, getView, callContext)) - - /** - * Get current login user and account find by /ACCOUNT_ID/. - * Validation of id exists passed. - * Note, the same as S.session of lift framework, the method only can be called at first line of endpoint for comprehension or out of for comprehension. - */ - def userAccount: Future[(Box[User], BankAccount, Option[CallContext])] = Future.successful((_user.box, bankAccount, callContext)) - - private def buildErrorMsg(msg: String) = - s""" - |This method invoke must fulfill three point: - | 1. be invoked at endpoint's body - | 2. be invoked out of for comprehension - | 3. endpoint's corresponding ResourceDoc.errorResponseBodies must contains $msg - |""".stripMargin - } - /** * The Glossary as served by GET /api/glossary: the static Glossary Items compiled into * Glossary.scala, unioned with the Dynamic Glossary Items held in the database. A Dynamic Item diff --git a/obp-api/src/main/scala/code/api/v4_0_0/Http4s400.scala b/obp-api/src/main/scala/code/api/v4_0_0/Http4s400.scala index 6f0f2463df..51670f2ef3 100644 --- a/obp-api/src/main/scala/code/api/v4_0_0/Http4s400.scala +++ b/obp-api/src/main/scala/code/api/v4_0_0/Http4s400.scala @@ -3089,10 +3089,8 @@ object Http4s400 { // `body.challenges.size != 0`. // // All v4 types delegate to the same connector helper — - // `LocalMappedConnectorInternal.createTransactionRequest` — which depends on - // `SS.user` (Lift's thread-globals). We wrap the call in `SS.init` so the helper's - // first synchronous read of `SS.user` captures the cc.user, then the Future chain - // runs normally on any thread. + // `LocalMappedConnectorInternal.createTransactionRequest` — which reads the + // authenticated user from the CallContext passed to it. // Resolves the view createTransactionRequest needs, unit-testable without a live Mapper // connection: `lookup` is production's real `Views.views.vend.systemView(...).or(...)` call @@ -3167,15 +3165,9 @@ object Http4s400 { Views.views.vend.systemView(ViewId(viewIdStr)) .or(Views.views.vend.customView(ViewId(viewIdStr), BankIdAccountId(account.bankId, account.accountId))) ) - // SS.init populates Lift thread-globals (used by `SS.user` inside the - // connector). The connector's first line `SS.user` resolves synchronously - // inside this block, capturing the user; subsequent flatMap stages run on - // other threads but the value is already bound. - innerResult <- APIUtil.SS.init(Full(user), bank, account, view, Some(cc)) { - code.bankconnectors.LocalMappedConnectorInternal.createTransactionRequest( - BankId(bankIdStr), AccountId(accountIdStr), ViewId(viewIdStr), - transactionRequestType, json) - } + innerResult <- code.bankconnectors.LocalMappedConnectorInternal.createTransactionRequest( + BankId(bankIdStr), AccountId(accountIdStr), ViewId(viewIdStr), + transactionRequestType, json, Some(cc)) } yield innerResult._1 } } @@ -3486,8 +3478,8 @@ object Http4s400 { // createTransactionRequestCard uses a different URL pattern (no bank/account/view) // and the Lift body calls the connector with empty BankId/AccountId. Add as its own - // route + ResourceDoc. The connector reads the user via SS.user, so prime SS with - // the user only; bank/account/view are connector-resolved from card details. + // route + ResourceDoc. The connector reads the user from the CallContext; + // bank/account/view are connector-resolved from card details. lazy val createTransactionRequestCard: Http4sRoute = Http4sRoute { case req @ POST -> `prefixPath` / "transaction-request-types" / "CARD" / "transaction-requests" => implicit val cc: CallContext = req.callContext @@ -3500,13 +3492,9 @@ object Http4s400 { com.openbankproject.commons.util.JsonAliases.parse(bodyStr) } transactionRequestType = TransactionRequestType("CARD") - innerResult <- APIUtil.SS.init(Full(user), - null.asInstanceOf[Bank], null.asInstanceOf[BankAccount], - null.asInstanceOf[View], Some(cc)) { - code.bankconnectors.LocalMappedConnectorInternal.createTransactionRequest( - BankId(""), AccountId(""), ViewId(Constant.SYSTEM_OWNER_VIEW_ID), - transactionRequestType, json) - } + innerResult <- code.bankconnectors.LocalMappedConnectorInternal.createTransactionRequest( + BankId(""), AccountId(""), ViewId(Constant.SYSTEM_OWNER_VIEW_ID), + transactionRequestType, json, Some(cc)) } yield innerResult._1 } } diff --git a/obp-api/src/main/scala/code/api/v6_0_0/Http4s600.scala b/obp-api/src/main/scala/code/api/v6_0_0/Http4s600.scala index 61ea354884..8315e12baf 100644 --- a/obp-api/src/main/scala/code/api/v6_0_0/Http4s600.scala +++ b/obp-api/src/main/scala/code/api/v6_0_0/Http4s600.scala @@ -4751,7 +4751,7 @@ object Http4s600 { for { json <- Future(com.openbankproject.commons.util.JsonAliases.parse(rawBody)) (resp, _) <- code.bankconnectors.LocalMappedConnectorInternal - .createTransactionRequest(bankId, accountId, viewId, txType, json) + .createTransactionRequest(bankId, accountId, viewId, txType, json, Some(cc)) } yield resp } } diff --git a/obp-api/src/main/scala/code/bankconnectors/LocalMappedConnectorInternal.scala b/obp-api/src/main/scala/code/bankconnectors/LocalMappedConnectorInternal.scala index 922afb8b63..1524e3c9f1 100644 --- a/obp-api/src/main/scala/code/bankconnectors/LocalMappedConnectorInternal.scala +++ b/obp-api/src/main/scala/code/bankconnectors/LocalMappedConnectorInternal.scala @@ -645,9 +645,10 @@ object LocalMappedConnectorInternal extends MdcLoggable { val sharedChargePolicy = ChargePolicy.withName("SHARED") - def createTransactionRequest(bankId: BankId, accountId: AccountId, viewId: ViewId, transactionRequestType: TransactionRequestType, json: JValue): Future[(TransactionRequestWithChargeJSON400, Option[CallContext])] = { + // The caller passes the request's CallContext, which must carry the authenticated user. + def createTransactionRequest(bankId: BankId, accountId: AccountId, viewId: ViewId, transactionRequestType: TransactionRequestType, json: JValue, callContext: Option[CallContext]): Future[(TransactionRequestWithChargeJSON400, Option[CallContext])] = { for { - (Full(u), callContext) <- SS.user + u <- Future(unboxFullOrFail(callContext.map(_.user).getOrElse(Empty), callContext, AuthenticatedUserIsRequired, 401)) transactionRequestTypeValue <- NewStyle.function.tryons(s"$InvalidTransactionRequestType: '${transactionRequestType.value}'. OBP does not support it.", 400, callContext) { TransactionRequestTypes.withName(transactionRequestType.value) From 9c15ac9e25992e3af85612d9f71a07c3144d7060 Mon Sep 17 00:00:00 2001 From: simonredfern Date: Sat, 17 Oct 2026 23:49:21 +0200 Subject: [PATCH 04/30] test: tag the real-payment Cardano scenarios RequiresCardanoWallet and exclude that tag by default in pom.xml, so they run only when included explicitly. --- obp-api/pom.xml | 6 +- .../CardanoTransactionRequestTest.scala | 584 ++++++------------ 2 files changed, 207 insertions(+), 383 deletions(-) diff --git a/obp-api/pom.xml b/obp-api/pom.xml index fc8fcaac90..c528526d3d 100644 --- a/obp-api/pom.xml +++ b/obp-api/pom.xml @@ -19,8 +19,10 @@ command-line -Dtest.tagsToExclude= override actually takes effect: Maven only lets -D override a POM-declared value when the config element is a ${property} expression, not a fixed string literal. Default excludes PropGatedPublicEndpoint - (see run_tests_parallel.sh / the CI job that overrides this to include it). --> - code.external,GetBanksPerf,PropGatedPublicEndpoint + (see run_tests_parallel.sh / the CI job that overrides this to include it), and + RequiresCardanoWallet, whose scenarios send real payments through a cardano-wallet + server that CI does not have (see CardanoTransactionRequestTest). --> + code.external,GetBanksPerf,PropGatedPublicEndpoint,RequiresCardanoWallet diff --git a/obp-api/src/test/scala/code/api/v6_0_0/CardanoTransactionRequestTest.scala b/obp-api/src/test/scala/code/api/v6_0_0/CardanoTransactionRequestTest.scala index 3f4b82e24a..42c91bea24 100644 --- a/obp-api/src/test/scala/code/api/v6_0_0/CardanoTransactionRequestTest.scala +++ b/obp-api/src/test/scala/code/api/v6_0_0/CardanoTransactionRequestTest.scala @@ -27,417 +27,239 @@ package code.api.v6_0_0 import org.json4s._ import code.api.Constant -import code.api.ResourceDocs1_4_0.SwaggerDefinitionsJSON -import code.api.util.ApiRole +import code.api.util.APIUtil.OAuth._ import code.api.util.ApiRole._ import code.api.util.ErrorMessages._ import code.api.v4_0_0.TransactionRequestWithChargeJSON400 import code.entitlement.Entitlement import code.methodrouting.MethodRoutingCommons +import code.api.v6_0_0.Http4s600.Implementations6_0_0 import com.github.dwickern.macros.NameOf.nameOf -import com.openbankproject.commons.model.{AmountOfMoneyJsonV121, ErrorMessage} +import com.openbankproject.commons.model.{AccountId, AmountOfMoneyJsonV121, ErrorMessage} import com.openbankproject.commons.util.ApiVersion import org.json4s.native.Serialization.write import org.scalatest.Tag -import code.api.util.APIUtil.OAuth._ -import code.api.util.ApiRole -import code.api.v6_0_0.Http4s600.Implementations6_0_0 -import code.entitlement.Entitlement - - +/** + * This suite checks the v6.0.0 CARDANO transaction request endpoint. + * + * Most scenarios are rejected by OBP before any payment is attempted (not logged in, no access to the view, + * invalid body), so they need no Cardano infrastructure and run in every build. + * + * The scenarios tagged RequiresCardanoWallet send a real payment: they route makePaymentv210 to the + * cardano_vJun2025 connector, which posts to a cardano-wallet server at http://localhost:8090. They need that + * server, connected to a synced Cardano test network, with the wallet `testAccountId` restored with + * `passphrase` and funded. The tag is excluded by default in obp-api/pom.xml; to run them, pass + * -DtagsToInclude=RequiresCardanoWallet -Dtest.tagsToExclude= on a machine that has such a wallet. + */ class CardanoTransactionRequestTest extends V600ServerSetup { - /** - * Test tags - * Example: To run tests with tag "getPermissions": - * mvn test -D tagsToInclude - * - * This is made possible by the scalatest maven plugin - */ object VersionOfApi extends Tag(ApiVersion.v6_0_0.toString) object CreateTransactionRequestCardano extends Tag(nameOf(Implementations6_0_0.createTransactionRequestCardano)) - + object RequiresCardanoWallet extends Tag("RequiresCardanoWallet") val testBankId = testBankId1.value - // This is a test account for Cardano transaction request tests, testAccountId0 is the walletId, passphrase is the passphrase for the wallet + // The account id is the cardano-wallet walletId, and passphrase is that wallet's passphrase. val testAccountId = "62b27359c25d4f2a5f97acee521ac1df7ac5a606" val passphrase = "StrongPassword123!" + val testAddress = "addr_test1qpv3se9ghq87ud29l0a8asy8nlqwd765e5zt4rc2z4mktqulwagn832cuzcjknfyxwzxz2p2kumx6n58tskugny6mrqs7fd23z" - val putCreateAccountJSONV310 = SwaggerDefinitionsJSON.createAccountRequestJsonV310.copy( - user_id = resourceUser1.userId, - balance = AmountOfMoneyJsonV121("lovelace", "0"), + private def cardanoRequest = (v6_0_0_Request / "banks" / testBankId / "accounts" / testAccountId / + Constant.SYSTEM_OWNER_VIEW_ID / "transaction-request-types" / "CARDANO" / "transaction-requests").POST + + private def basicTransferBody(description: String = "Basic ADA transfer") = TransactionRequestBodyCardanoJsonV600( + to = CardanoPaymentJsonV600( + address = testAddress, + amount = CardanoAmountJsonV600(quantity = 1000000, unit = "lovelace") + ), + value = AmountOfMoneyJsonV121("lovelace", "1000000"), + passphrase = passphrase, + description = description ) - + // Test data is wiped before every scenario, so each scenario that reaches the account creates it here, + // with resourceUser1 (user1) as its owner. + private def createWalletAccountAndEnableCardano(): Unit = { + setPropsValues("transactionRequests_enabled" -> "true") + setPropsValues("transactionRequests_supported_types" -> "CARDANO") + createAccountRelevantResource(Some(resourceUser1), testBankId1, AccountId(testAccountId), "lovelace") + } + + private def routePaymentsToCardanoWallet(): Unit = { + Entitlement.entitlement.vend.addEntitlement("", resourceUser1.userId, CanCreateMethodRouting.toString) + val cardanoMethodRouting = MethodRoutingCommons( + methodName = "makePaymentv210", + connectorName = "cardano_vJun2025", + isBankIdExactMatch = false, + bankIdPattern = Some("*"), + parameters = List() + ) + val response = makePostRequest((v6_0_0_Request / "management" / "method_routings").POST <@ (user1), write(cardanoMethodRouting)) + response.code should equal(201) + } + + private def postExpecting400(body: String, expectedMessage: String) = { + createWalletAccountAndEnableCardano() + val response = makePostRequest(cardanoRequest <@ (user1), body) + Then("We should get a 400") + response.code should equal(400) + And("error should include " + expectedMessage) + response.body.extract[ErrorMessage].message should include(expectedMessage) + } + feature("Create Cardano Transaction Request - v6.0.0") { - + scenario("We will create Cardano transaction request - user is NOT logged in", CreateTransactionRequestCardano, VersionOfApi) { When("We make a request v6.0.0") - val request600 = (v6_0_0_Request / "banks" / testBankId / "accounts" / testAccountId / Constant.SYSTEM_OWNER_VIEW_ID / "transaction-request-types" / "CARDANO" / "transaction-requests").POST - val cardanoTransactionRequestBody = TransactionRequestBodyCardanoJsonV600( - to = CardanoPaymentJsonV600( - address = "addr_test1qpv3se9ghq87ud29l0a8asy8nlqwd765e5zt4rc2z4mktqulwagn832cuzcjknfyxwzxz2p2kumx6n58tskugny6mrqs7fd23z", - amount = CardanoAmountJsonV600( - quantity = 1000000, - unit = "lovelace" - ) - ), - value = AmountOfMoneyJsonV121("lovelace", "1000000"), - passphrase = passphrase, - description = "Basic ADA transfer" - ) - val response600 = makePostRequest(request600, write(cardanoTransactionRequestBody)) + val response600 = makePostRequest(cardanoRequest, write(basicTransferBody())) Then("We should get a 401") response600.code should equal(401) And("error should be " + AuthenticatedUserIsRequired) response600.body.extract[ErrorMessage].message should equal(AuthenticatedUserIsRequired) } -// scenario("We will create Cardano transaction request - user is logged in", CreateTransactionRequestCardano, VersionOfApi) { -// Entitlement.entitlement.vend.addEntitlement(testBankId, resourceUser1.userId, ApiRole.canCreateAccount.toString()) -// val request = (v6_0_0_Request / "banks" / testBankId / "accounts" / testAccountId ).PUT <@(user1) -// val response = makePutRequest(request, write(putCreateAccountJSONV310)) -// Then("We should get a 201") -// response.code should equal(201) -// -// When("We create a method routing for makePaymentv210 to cardano_vJun2025") -// val cardanoMethodRouting = MethodRoutingCommons( -// methodName = "makePaymentv210", -// connectorName = "cardano_vJun2025", -// isBankIdExactMatch = false, -// bankIdPattern = Some("*"), -// parameters = List() -// ) -// val request310 = (v6_0_0_Request / "management" / "method_routings").POST <@(user1) -// Entitlement.entitlement.vend.addEntitlement("", resourceUser1.userId, CanCreateMethodRouting.toString) -// val response310 = makePostRequest(request310, write(cardanoMethodRouting)) -// response310.code should equal(201) -// -// When("We make a request v6.0.0") -// val request600 = (v6_0_0_Request / "banks" / testBankId / "accounts" / testAccountId / Constant.SYSTEM_OWNER_VIEW_ID / "transaction-request-types" / "CARDANO" / "transaction-requests").POST <@(user1) -// val cardanoTransactionRequestBody = TransactionRequestBodyCardanoJsonV600( -// to = CardanoPaymentJsonV600( -// address = "addr_test1qpv3se9ghq87ud29l0a8asy8nlqwd765e5zt4rc2z4mktqulwagn832cuzcjknfyxwzxz2p2kumx6n58tskugny6mrqs7fd23z", -// amount = CardanoAmountJsonV600( -// quantity = 1000000, -// unit = "lovelace" -// ) -// ), -// value = AmountOfMoneyJsonV121("lovelace", "1000000"), -// passphrase = passphrase, -// description = "Basic ADA transfer" -// ) -// val response600 = makePostRequest(request600, write(cardanoTransactionRequestBody)) -// Then("We should get a 201") -// response600.code should equal(201) -// And("response should contain transaction request") -// val transactionRequest = response600.body.extract[TransactionRequestWithChargeJSON400] -// transactionRequest.status should not be empty -// } -// -// scenario("We will create Cardano transaction request with metadata - user is logged in", CreateTransactionRequestCardano, VersionOfApi) { -// Entitlement.entitlement.vend.addEntitlement(testBankId, resourceUser1.userId, ApiRole.canCreateAccount.toString()) -// val request = (v6_0_0_Request / "banks" / testBankId / "accounts" / testAccountId ).PUT <@(user1) -// val response = makePutRequest(request, write(putCreateAccountJSONV310)) -// Then("We should get a 201") -// response.code should equal(201) -// -// When("We create a method routing for makePaymentv210 to cardano_vJun2025") -// val cardanoMethodRouting = MethodRoutingCommons( -// methodName = "makePaymentv210", -// connectorName = "cardano_vJun2025", -// isBankIdExactMatch = false, -// bankIdPattern = Some("*"), -// parameters = List() -// ) -// val request310 = (v6_0_0_Request / "management" / "method_routings").POST <@(user1) -// Entitlement.entitlement.vend.addEntitlement("", resourceUser1.userId, CanCreateMethodRouting.toString) -// val response310 = makePostRequest(request310, write(cardanoMethodRouting)) -// response310.code should equal(201) -// -// When("We make a request v6.0.0 with metadata") -// val request600 = (v6_0_0_Request / "banks" / testBankId / "accounts" / testAccountId / Constant.SYSTEM_OWNER_VIEW_ID / "transaction-request-types" / "CARDANO" / "transaction-requests").POST <@(user1) -// val cardanoTransactionRequestBody = TransactionRequestBodyCardanoJsonV600( -// to = CardanoPaymentJsonV600( -// address = "addr_test1qpv3se9ghq87ud29l0a8asy8nlqwd765e5zt4rc2z4mktqulwagn832cuzcjknfyxwzxz2p2kumx6n58tskugny6mrqs7fd23z", -// amount = CardanoAmountJsonV600( -// quantity = 1000000, -// unit = "lovelace" -// ) -// ), -// value = AmountOfMoneyJsonV121("lovelace", "1000000"), -// passphrase = passphrase, -// description = "ADA transfer with metadata", -// metadata = Some(Map("202507022319" -> CardanoMetadataStringJsonV600("Hello Cardano"))) -// ) -// val response600 = makePostRequest(request600, write(cardanoTransactionRequestBody)) -// Then("We should get a 201") -// response600.code should equal(201) -// And("response should contain transaction request") -// val transactionRequest = response600.body.extract[TransactionRequestWithChargeJSON400] -// transactionRequest.status should not be empty -// } -// -// scenario("We will create Cardano transaction request with token - user is logged in", CreateTransactionRequestCardano, VersionOfApi) { -// Entitlement.entitlement.vend.addEntitlement(testBankId, resourceUser1.userId, ApiRole.canCreateAccount.toString()) -// val request = (v6_0_0_Request / "banks" / testBankId / "accounts" / testAccountId ).PUT <@(user1) -// val response = makePutRequest(request, write(putCreateAccountJSONV310)) -// Then("We should get a 201") -// response.code should equal(201) -// -// When("We create a method routing for makePaymentv210 to cardano_vJun2025") -// val cardanoMethodRouting = MethodRoutingCommons( -// methodName = "makePaymentv210", -// connectorName = "cardano_vJun2025", -// isBankIdExactMatch = false, -// bankIdPattern = Some("*"), -// parameters = List() -// ) -// val request310 = (v6_0_0_Request / "management" / "method_routings").POST <@(user1) -// Entitlement.entitlement.vend.addEntitlement("", resourceUser1.userId, CanCreateMethodRouting.toString) -// val response310 = makePostRequest(request310, write(cardanoMethodRouting)) -// response310.code should equal(201) -// -// When("We make a request v6.0.0 with token") -// val request600 = (v6_0_0_Request / "banks" / testBankId / "accounts" / testAccountId / Constant.SYSTEM_OWNER_VIEW_ID / "transaction-request-types" / "CARDANO" / "transaction-requests").POST <@(user1) -// val cardanoTransactionRequestBody = TransactionRequestBodyCardanoJsonV600( -// to = CardanoPaymentJsonV600( -// address = "addr_test1qpv3se9ghq87ud29l0a8asy8nlqwd765e5zt4rc2z4mktqulwagn832cuzcjknfyxwzxz2p2kumx6n58tskugny6mrqs7fd23z", -// amount = CardanoAmountJsonV600( -// quantity = 1000000, -// unit = "lovelace" -// ), -// assets = Some(List(CardanoAssetJsonV600( -// policy_id = "ef1954d3a058a96d89d959939aeb5b2948a3df2eb40f9a78d61e3d4f", -// asset_name = "OGCRA", -// quantity = 10 -// ))) -// ), -// value = AmountOfMoneyJsonV121("lovelace", "1000000"), -// passphrase = passphrase, -// description = "Token-only transfer" -// ) -// val response600 = makePostRequest(request600, write(cardanoTransactionRequestBody)) -// Then("We should get a 201") -// response600.code should equal(201) -// And("response should contain transaction request") -// val transactionRequest = response600.body.extract[TransactionRequestWithChargeJSON400] -// transactionRequest.status should not be empty -// } -// -// scenario("We will create Cardano transaction request with token and metadata - user is logged in", CreateTransactionRequestCardano, VersionOfApi) { -// When("We make a request v6.0.0 with token and metadata") -// val request600 = (v6_0_0_Request / "banks" / testBankId / "accounts" / testAccountId / Constant.SYSTEM_OWNER_VIEW_ID / "transaction-request-types" / "CARDANO" / "transaction-requests").POST <@(user1) -// val cardanoTransactionRequestBody = TransactionRequestBodyCardanoJsonV600( -// to = CardanoPaymentJsonV600( -// address = "addr_test1qpv3se9ghq87ud29l0a8asy8nlqwd765e5zt4rc2z4mktqulwagn832cuzcjknfyxwzxz2p2kumx6n58tskugny6mrqs7fd23z", -// amount = CardanoAmountJsonV600( -// quantity = 5000000, -// unit = "lovelace" -// ), -// assets = Some(List(CardanoAssetJsonV600( -// policy_id = "ef1954d3a058a96d89d959939aeb5b2948a3df2eb40f9a78d61e3d4f", -// asset_name = "OGCRA", -// quantity = 10 -// ))) -// ), -// value = AmountOfMoneyJsonV121("lovelace", "1000000"), -// passphrase = passphrase, -// description = "ADA transfer with token and metadata", -// metadata = Some(Map("202507022319" -> CardanoMetadataStringJsonV600("Hello Cardano with Token"))) -// ) -// val response600 = makePostRequest(request600, write(cardanoTransactionRequestBody)) -// Then("We should get a 201") -// response600.code should equal(201) -// And("response should contain transaction request") -// val transactionRequest = response600.body.extract[TransactionRequestWithChargeJSON400] -// transactionRequest.status should not be empty -// } -// -// scenario("We will try to create Cardano transaction request for someone else account - user is logged in", CreateTransactionRequestCardano, VersionOfApi) { -// When("We make a request v6.0.0") -// val request600 = (v6_0_0_Request / "banks" / testBankId / "accounts" / testAccountId / Constant.SYSTEM_OWNER_VIEW_ID / "transaction-request-types" / "CARDANO" / "transaction-requests").POST <@(user2) -// val cardanoTransactionRequestBody = TransactionRequestBodyCardanoJsonV600( -// to = CardanoPaymentJsonV600( -// address = "addr_test1qpv3se9ghq87ud29l0a8asy8nlqwd765e5zt4rc2z4mktqulwagn832cuzcjknfyxwzxz2p2kumx6n58tskugny6mrqs7fd23z", -// amount = CardanoAmountJsonV600( -// quantity = 1000000, -// unit = "lovelace" -// ) -// ), -// value = AmountOfMoneyJsonV121("lovelace", "1000000"), -// passphrase = passphrase, -// description = "Basic ADA transfer" -// ) -// val response600 = makePostRequest(request600, write(cardanoTransactionRequestBody)) -// Then("We should get a 403") -// response600.code should equal(403) -// And("error should be " + UserNoPermissionAccessView) -// response600.body.extract[ErrorMessage].message contains (UserNoPermissionAccessView) shouldBe (true) -// } -// -// scenario("We will try to create Cardano transaction request with invalid address format", CreateTransactionRequestCardano, VersionOfApi) { -// When("We make a request v6.0.0 with invalid address") -// val request600 = (v6_0_0_Request / "banks" / testBankId / "accounts" / testAccountId / Constant.SYSTEM_OWNER_VIEW_ID / "transaction-request-types" / "CARDANO" / "transaction-requests").POST <@(user1) -// val cardanoTransactionRequestBody = TransactionRequestBodyCardanoJsonV600( -// to = CardanoPaymentJsonV600( -// address = "invalid_address_format", -// amount = CardanoAmountJsonV600( -// quantity = 1000000, -// unit = "lovelace" -// ) -// ), -// value = AmountOfMoneyJsonV121("lovelace", "1000000"), -// passphrase = passphrase, -// description = "Basic ADA transfer" -// ) -// val response600 = makePostRequest(request600, write(cardanoTransactionRequestBody)) -// Then("We should get a 400") -// response600.code should equal(400) -// And("error should contain invalid address message") -// response600.body.extract[ErrorMessage].message should include("Cardano address format is invalid") -// } -// -// scenario("We will try to create Cardano transaction request with missing amount", CreateTransactionRequestCardano, VersionOfApi) { -// When("We make a request v6.0.0 with missing amount") -// val request600 = (v6_0_0_Request / "banks" / testBankId / "accounts" / testAccountId / Constant.SYSTEM_OWNER_VIEW_ID / "transaction-request-types" / "CARDANO" / "transaction-requests").POST <@(user1) -// val invalidJson = """ -// { -// "to": { -// "address": "addr_test1qpv3se9ghq87ud29l0a8asy8nlqwd765e5zt4rc2z4mktqulwagn832cuzcjknfyxwzxz2p2kumx6n58tskugny6mrqs7fd23z" -// }, -// "value": { -// "currency": "lovelace", -// "amount": "1000000" -// }, -// "passphrase": "StrongPassword123!", -// "description": "Basic ADA transfer" -// } -// """ -// val response600 = makePostRequest(request600, invalidJson) -// Then("We should get a 400") -// response600.code should equal(400) -// And("error should contain invalid json format message") -// response600.body.extract[ErrorMessage].message should include("InvalidJsonFormat") -// } -// -// scenario("We will try to create Cardano transaction request with negative amount", CreateTransactionRequestCardano, VersionOfApi) { -// When("We make a request v6.0.0 with negative amount") -// val request600 = (v6_0_0_Request / "banks" / testBankId / "accounts" / testAccountId / Constant.SYSTEM_OWNER_VIEW_ID / "transaction-request-types" / "CARDANO" / "transaction-requests").POST <@(user1) -// val cardanoTransactionRequestBody = TransactionRequestBodyCardanoJsonV600( -// to = CardanoPaymentJsonV600( -// address = "addr_test1qpv3se9ghq87ud29l0a8asy8nlqwd765e5zt4rc2z4mktqulwagn832cuzcjknfyxwzxz2p2kumx6n58tskugny6mrqs7fd23z", -// amount = CardanoAmountJsonV600( -// quantity = -1000000, -// unit = "lovelace" -// ) -// ), -// value = AmountOfMoneyJsonV121("lovelace", "1000000"), -// passphrase = passphrase, -// description = "Basic ADA transfer" -// ) -// val response600 = makePostRequest(request600, write(cardanoTransactionRequestBody)) -// Then("We should get a 400") -// response600.code should equal(400) -// And("error should contain invalid amount message") -// response600.body.extract[ErrorMessage].message should include("Cardano amount quantity must be non-negative") -// } -// -// scenario("We will try to create Cardano transaction request with invalid amount unit", CreateTransactionRequestCardano, VersionOfApi) { -// When("We make a request v6.0.0 with invalid amount unit") -// val request600 = (v6_0_0_Request / "banks" / testBankId / "accounts" / testAccountId / Constant.SYSTEM_OWNER_VIEW_ID / "transaction-request-types" / "CARDANO" / "transaction-requests").POST <@(user1) -// val cardanoTransactionRequestBody = TransactionRequestBodyCardanoJsonV600( -// to = CardanoPaymentJsonV600( -// address = "addr_test1qpv3se9ghq87ud29l0a8asy8nlqwd765e5zt4rc2z4mktqulwagn832cuzcjknfyxwzxz2p2kumx6n58tskugny6mrqs7fd23z", -// amount = CardanoAmountJsonV600( -// quantity = 1000000, -// unit = "abc" // Invalid unit, should be "lovelace" -// ) -// ), -// value = AmountOfMoneyJsonV121("lovelace", "1000000"), -// passphrase = passphrase, -// description = "Basic ADA transfer" -// ) -// val response600 = makePostRequest(request600, write(cardanoTransactionRequestBody)) -// Then("We should get a 400") -// response600.code should equal(400) -// And("error should contain invalid unit message") -// response600.body.extract[ErrorMessage].message should include("Cardano amount unit must be 'lovelace'") -// } -// -// scenario("We will try to create Cardano transaction request with zero amount but no assets", CreateTransactionRequestCardano, VersionOfApi) { -// When("We make a request v6.0.0 with zero amount but no assets") -// val request600 = (v6_0_0_Request / "banks" / testBankId / "accounts" / testAccountId / Constant.SYSTEM_OWNER_VIEW_ID / "transaction-request-types" / "CARDANO" / "transaction-requests").POST <@(user1) -// val cardanoTransactionRequestBody = TransactionRequestBodyCardanoJsonV600( -// to = CardanoPaymentJsonV600( -// address = "addr_test1qpv3se9ghq87ud29l0a8asy8nlqwd765e5zt4rc2z4mktqulwagn832cuzcjknfyxwzxz2p2kumx6n58tskugny6mrqs7fd23z", -// amount = CardanoAmountJsonV600( -// quantity = 0, -// unit = "lovelace" -// ) -// ), -// value = AmountOfMoneyJsonV121("lovelace", "0.0"), -// passphrase = passphrase, -// description = "Zero amount without assets" -// ) -// val response600 = makePostRequest(request600, write(cardanoTransactionRequestBody)) -// Then("We should get a 400") -// response600.code should equal(400) -// And("error should contain invalid amount message") -// response600.body.extract[ErrorMessage].message should include("Cardano transfer with zero amount must include assets") -// } -// -// scenario("We will try to create Cardano transaction request with invalid assets", CreateTransactionRequestCardano, VersionOfApi) { -// When("We make a request v6.0.0 with invalid assets") -// val request600 = (v6_0_0_Request / "banks" / testBankId / "accounts" / testAccountId / Constant.SYSTEM_OWNER_VIEW_ID / "transaction-request-types" / "CARDANO" / "transaction-requests").POST <@(user1) -// val cardanoTransactionRequestBody = TransactionRequestBodyCardanoJsonV600( -// to = CardanoPaymentJsonV600( -// address = "addr_test1qpv3se9ghq87ud29l0a8asy8nlqwd765e5zt4rc2z4mktqulwagn832cuzcjknfyxwzxz2p2kumx6n58tskugny6mrqs7fd23z", -// amount = CardanoAmountJsonV600( -// quantity = 0, -// unit = "lovelace" -// ), -// assets = Some(List(CardanoAssetJsonV600( -// policy_id = "", -// asset_name = "", -// quantity = 0 -// ))) -// ), -// value = AmountOfMoneyJsonV121("lovelace", "0.0"), -// passphrase = passphrase, -// description = "Invalid assets" -// ) -// val response600 = makePostRequest(request600, write(cardanoTransactionRequestBody)) -// Then("We should get a 400") -// response600.code should equal(400) -// And("error should contain invalid assets message") -// response600.body.extract[ErrorMessage].message should include("Cardano assets must have valid policy_id and asset_name") -// } -// -// scenario("We will try to create Cardano transaction request with invalid metadata", CreateTransactionRequestCardano, VersionOfApi) { -// When("We make a request v6.0.0 with invalid metadata") -// val request600 = (v6_0_0_Request / "banks" / testBankId / "accounts" / testAccountId / Constant.SYSTEM_OWNER_VIEW_ID / "transaction-request-types" / "CARDANO" / "transaction-requests").POST <@(user1) -// val cardanoTransactionRequestBody = TransactionRequestBodyCardanoJsonV600( -// to = CardanoPaymentJsonV600( -// address = "addr_test1qpv3se9ghq87ud29l0a8asy8nlqwd765e5zt4rc2z4mktqulwagn832cuzcjknfyxwzxz2p2kumx6n58tskugny6mrqs7fd23z", -// amount = CardanoAmountJsonV600( -// quantity = 1000000, -// unit = "lovelace" -// ) -// ), -// value = AmountOfMoneyJsonV121("lovelace", "1000000"), -// passphrase = passphrase, -// description = "ADA transfer with invalid metadata", -// metadata = Some(Map("" -> CardanoMetadataStringJsonV600(""))) -// ) -// val response600 = makePostRequest(request600, write(cardanoTransactionRequestBody)) -// Then("We should get a 400") -// response600.code should equal(400) -// And("error should contain invalid metadata message") -// response600.body.extract[ErrorMessage].message should include("Cardano metadata must have valid structure") -// } + scenario("We will try to create Cardano transaction request for someone else account - user is logged in", CreateTransactionRequestCardano, VersionOfApi) { + createWalletAccountAndEnableCardano() + When("user2, who has no view on the account, makes a request v6.0.0") + val response600 = makePostRequest(cardanoRequest <@ (user2), write(basicTransferBody())) + Then("We should get a 403") + response600.code should equal(403) + And("error should be " + UserNoPermissionAccessView) + response600.body.extract[ErrorMessage].message should include(UserNoPermissionAccessView) + } + + scenario("We will try to create Cardano transaction request with invalid address format", CreateTransactionRequestCardano, VersionOfApi) { + When("We make a request v6.0.0 with invalid address") + val body = basicTransferBody().copy(to = basicTransferBody().to.copy(address = "invalid_address_format")) + postExpecting400(write(body), "Cardano address format is invalid") + } + + scenario("We will try to create Cardano transaction request with missing amount", CreateTransactionRequestCardano, VersionOfApi) { + When("We make a request v6.0.0 with missing amount") + val invalidJson = + s""" + { + "to": { + "address": "$testAddress" + }, + "value": { + "currency": "lovelace", + "amount": "1000000" + }, + "passphrase": "$passphrase", + "description": "Basic ADA transfer" + } + """ + postExpecting400(invalidJson, InvalidJsonFormat) + } + + scenario("We will try to create Cardano transaction request with negative amount", CreateTransactionRequestCardano, VersionOfApi) { + When("We make a request v6.0.0 with negative amount") + val body = basicTransferBody().copy(to = basicTransferBody().to.copy(amount = CardanoAmountJsonV600(quantity = -1000000, unit = "lovelace"))) + postExpecting400(write(body), "Cardano amount quantity must be non-negative") + } + + scenario("We will try to create Cardano transaction request with invalid amount unit", CreateTransactionRequestCardano, VersionOfApi) { + When("We make a request v6.0.0 with invalid amount unit") + val body = basicTransferBody().copy(to = basicTransferBody().to.copy(amount = CardanoAmountJsonV600(quantity = 1000000, unit = "abc"))) + postExpecting400(write(body), "Cardano amount unit must be 'lovelace'") + } + + scenario("We will try to create Cardano transaction request with zero amount but no assets", CreateTransactionRequestCardano, VersionOfApi) { + When("We make a request v6.0.0 with zero amount but no assets") + val body = basicTransferBody("Zero amount without assets").copy( + to = basicTransferBody().to.copy(amount = CardanoAmountJsonV600(quantity = 0, unit = "lovelace")), + value = AmountOfMoneyJsonV121("lovelace", "0.0") + ) + // The check that value.amount is positive, common to every transaction request type, runs before the + // CARDANO-specific checks, so it is the one that rejects this request. + postExpecting400(write(body), NotPositiveAmount) + } + + scenario("We will try to create Cardano transaction request with invalid assets", CreateTransactionRequestCardano, VersionOfApi) { + When("We make a request v6.0.0 with invalid assets") + val body = basicTransferBody("Invalid assets").copy( + to = basicTransferBody().to.copy(assets = Some(List(CardanoAssetJsonV600(policy_id = "", asset_name = "", quantity = 0)))) + ) + postExpecting400(write(body), "Cardano assets must have valid policy_id and asset_name") + } + + scenario("We will try to create Cardano transaction request with invalid metadata", CreateTransactionRequestCardano, VersionOfApi) { + When("We make a request v6.0.0 with invalid metadata") + val body = basicTransferBody("ADA transfer with invalid metadata").copy( + metadata = Some(Map("" -> CardanoMetadataStringJsonV600(""))) + ) + postExpecting400(write(body), "Cardano metadata must have valid structure") + } + + // The scenarios below send a real payment through cardano-wallet; see the class comment. + + scenario("We will create Cardano transaction request - user is logged in", CreateTransactionRequestCardano, VersionOfApi, RequiresCardanoWallet) { + createWalletAccountAndEnableCardano() + routePaymentsToCardanoWallet() + When("We make a request v6.0.0") + val response600 = makePostRequest(cardanoRequest <@ (user1), write(basicTransferBody())) + Then("We should get a 201") + response600.code should equal(201) + And("response should contain transaction request") + response600.body.extract[TransactionRequestWithChargeJSON400].status should not be empty + } + + scenario("We will create Cardano transaction request with metadata - user is logged in", CreateTransactionRequestCardano, VersionOfApi, RequiresCardanoWallet) { + createWalletAccountAndEnableCardano() + routePaymentsToCardanoWallet() + When("We make a request v6.0.0 with metadata") + val body = basicTransferBody("ADA transfer with metadata").copy( + metadata = Some(Map("202507022319" -> CardanoMetadataStringJsonV600("Hello Cardano"))) + ) + val response600 = makePostRequest(cardanoRequest <@ (user1), write(body)) + Then("We should get a 201") + response600.code should equal(201) + And("response should contain transaction request") + response600.body.extract[TransactionRequestWithChargeJSON400].status should not be empty + } + + scenario("We will create Cardano transaction request with token - user is logged in", CreateTransactionRequestCardano, VersionOfApi, RequiresCardanoWallet) { + createWalletAccountAndEnableCardano() + routePaymentsToCardanoWallet() + When("We make a request v6.0.0 with token") + val body = basicTransferBody("Token-only transfer").copy( + to = basicTransferBody().to.copy(assets = Some(List(CardanoAssetJsonV600( + policy_id = "ef1954d3a058a96d89d959939aeb5b2948a3df2eb40f9a78d61e3d4f", + asset_name = "OGCRA", + quantity = 10 + )))) + ) + val response600 = makePostRequest(cardanoRequest <@ (user1), write(body)) + Then("We should get a 201") + response600.code should equal(201) + And("response should contain transaction request") + response600.body.extract[TransactionRequestWithChargeJSON400].status should not be empty + } + + scenario("We will create Cardano transaction request with token and metadata - user is logged in", CreateTransactionRequestCardano, VersionOfApi, RequiresCardanoWallet) { + createWalletAccountAndEnableCardano() + routePaymentsToCardanoWallet() + When("We make a request v6.0.0 with token and metadata") + val body = basicTransferBody("ADA transfer with token and metadata").copy( + to = CardanoPaymentJsonV600( + address = testAddress, + amount = CardanoAmountJsonV600(quantity = 5000000, unit = "lovelace"), + assets = Some(List(CardanoAssetJsonV600( + policy_id = "ef1954d3a058a96d89d959939aeb5b2948a3df2eb40f9a78d61e3d4f", + asset_name = "OGCRA", + quantity = 10 + ))) + ), + metadata = Some(Map("202507022319" -> CardanoMetadataStringJsonV600("Hello Cardano with Token"))) + ) + val response600 = makePostRequest(cardanoRequest <@ (user1), write(body)) + Then("We should get a 201") + response600.code should equal(201) + And("response should contain transaction request") + response600.body.extract[TransactionRequestWithChargeJSON400].status should not be empty + } } -} \ No newline at end of file +} From 0a750af979df3ccc985f151caf3068b838ece6c8 Mon Sep 17 00:00:00 2001 From: simonredfern Date: Sun, 18 Oct 2026 22:57:49 +0100 Subject: [PATCH 05/30] Get chat message checks the message is in the room --- obp-api/src/main/scala/code/api/v6_0_0/Http4s600.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/obp-api/src/main/scala/code/api/v6_0_0/Http4s600.scala b/obp-api/src/main/scala/code/api/v6_0_0/Http4s600.scala index 8315e12baf..b20c00b000 100644 --- a/obp-api/src/main/scala/code/api/v6_0_0/Http4s600.scala +++ b/obp-api/src/main/scala/code/api/v6_0_0/Http4s600.scala @@ -3896,7 +3896,7 @@ object Http4s600 { _ <- Future(unboxFullOrFail(roomBox, Some(cc), ChatRoomNotFound, 404)) partBox <- Future(code.chat.ChatPermissions.isParticipant(chatRoomId, user.userId)) _ <- Future(unboxFullOrFail(partBox, Some(cc), NotChatRoomParticipant, 403)) - msgBox <- Future(code.chat.ChatMessageTrait.chatMessageProvider.vend.getMessage(chatMessageId)) + msgBox <- Future(code.chat.ChatMessageTrait.chatMessageProvider.vend.getMessage(chatMessageId).filter(_.chatRoomId == chatRoomId)) msg <- Future(unboxFullOrFail(msgBox, Some(cc), ChatMessageNotFound, 404)) reactions <- Future(code.chat.ReactionTrait.reactionProvider.vend.getReactions(chatMessageId).openOr(List.empty)) } yield JSONFactory600.createChatMessageJson(msg, reactions) @@ -3911,7 +3911,7 @@ object Http4s600 { _ <- Future(unboxFullOrFail(roomBox, Some(cc), ChatRoomNotFound, 404)) partBox <- Future(code.chat.ChatPermissions.isParticipant(chatRoomId, user.userId)) _ <- Future(unboxFullOrFail(partBox, Some(cc), NotChatRoomParticipant, 403)) - msgBox <- Future(code.chat.ChatMessageTrait.chatMessageProvider.vend.getMessage(chatMessageId)) + msgBox <- Future(code.chat.ChatMessageTrait.chatMessageProvider.vend.getMessage(chatMessageId).filter(_.chatRoomId == chatRoomId)) msg <- Future(unboxFullOrFail(msgBox, Some(cc), ChatMessageNotFound, 404)) reactions <- Future(code.chat.ReactionTrait.reactionProvider.vend.getReactions(chatMessageId).openOr(List.empty)) } yield JSONFactory600.createChatMessageJson(msg, reactions) From c68bfdb865dcd4cb9189316b0a2618cc0d8bea0d Mon Sep 17 00:00:00 2001 From: simonredfern Date: Sun, 18 Oct 2026 23:51:00 +0100 Subject: [PATCH 06/30] Fix: Update Customer Address checks the bank and customer --- .../scala/code/api/v3_1_0/Http4s310.scala | 19 +++++++- .../code/api/v3_1_0/CustomerAddressTest.scala | 44 +++++++++++++++++++ .../parity_allowlist.json | 8 ++++ 3 files changed, 69 insertions(+), 2 deletions(-) diff --git a/obp-api/src/main/scala/code/api/v3_1_0/Http4s310.scala b/obp-api/src/main/scala/code/api/v3_1_0/Http4s310.scala index fe29485dba..e40f06e588 100644 --- a/obp-api/src/main/scala/code/api/v3_1_0/Http4s310.scala +++ b/obp-api/src/main/scala/code/api/v3_1_0/Http4s310.scala @@ -2325,7 +2325,19 @@ object Http4s310 { EndpointHelpers.withUserAndBankAndBody[PostCustomerAddressJsonV310, Any](req) { (user, bank, postedData, cc) => for { _ <- NewStyle.function.hasEntitlement(bank.bankId.value, user.userId, canCreateCustomer, Some(cc)) - (_, _) <- NewStyle.function.getCustomerByCustomerId(customerIdStr, Some(cc)) + // Keep the customer: its bank is checked next + (customer, _) <- NewStyle.function.getCustomerByCustomerId(customerIdStr, Some(cc)) + // The customer must be at the bank in the path, where the Role is held, else the Role at one bank + // would let the caller change customers at any bank + // 404, as for a customer that does not exist, so nothing is told about other banks' customers + _ <- code.util.Helper.booleanToFuture(CustomerNotFoundByCustomerId, 404, Some(cc)) { + customer.bankId == bank.bankId.value + } + // The address must be one of this customer's, else any address id could be overwritten + (addresses, _) <- NewStyle.function.getCustomerAddress(customerIdStr, Some(cc)) + _ <- code.util.Helper.booleanToFuture(CustomerAddressNotFound, 404, Some(cc)) { + addresses.exists(_.customerAddressId == customerAddressIdStr) + } (address, _) <- NewStyle.function.updateCustomerAddress( customerAddressIdStr, postedData.line_1, postedData.line_2, postedData.line_3, @@ -2352,7 +2364,10 @@ object Http4s310 { |""", postCustomerAddressJsonV310, customerAddressJsonV310, - List(AuthenticatedUserIsRequired, UserHasMissingRoles, InvalidJsonFormat, UnknownError), + // CustomerNotFoundByCustomerId and CustomerAddressNotFound: the customer is not at BANK_ID, or the address + // is not this customer's (the guards above) + List(AuthenticatedUserIsRequired, UserHasMissingRoles, InvalidJsonFormat, CustomerNotFoundByCustomerId, + CustomerAddressNotFound, UnknownError), List(apiTagCustomer), Some(List(canCreateCustomer)), http4sPartialFunction = Some(updateCustomerAddress) diff --git a/obp-api/src/test/scala/code/api/v3_1_0/CustomerAddressTest.scala b/obp-api/src/test/scala/code/api/v3_1_0/CustomerAddressTest.scala index fac71901b3..fd973e8579 100644 --- a/obp-api/src/test/scala/code/api/v3_1_0/CustomerAddressTest.scala +++ b/obp-api/src/test/scala/code/api/v3_1_0/CustomerAddressTest.scala @@ -189,5 +189,49 @@ class CustomerAddressTest extends V310ServerSetup { } } + // The Update endpoint changes an address only of the customer in the path, and only at the bank in the path + feature("Update Customer Address v3.1.0 stays within the bank and the customer in the path") { + val bankA = testBankId1.value + val bankB = testBankId2.value + + // Creates a customer at the bank, with one address, through the endpoints; returns their ids + def customerWithAddress(bank: String): (String, String) = { + val customer = makePostRequest((v3_1_0_Request / "banks" / bank / "customers").POST <@(user1), write(postCustomerJson)) + customer.code should equal(201) + val customerId = customer.body.extract[CustomerJsonV310].customer_id + val address = makePostRequest((v3_1_0_Request / "banks" / bank / "customers" / customerId / "address").POST <@(user1), + write(postCustomerAddressJson)) + address.code should equal(201) + (customerId, address.body.extract[CustomerAddressJsonV310].customer_address_id) + } + + scenario("A customer at another bank than the one in the path is not found", ApiEndpoint4, VersionOfApi) { + for (bank <- List(bankA, bankB); role <- List(CanCreateCustomer, CanCreateCustomerAddress)) + Entitlement.entitlement.vend.addEntitlement(bank, resourceUser1.userId, role.toString) + val (customerAtB, addressAtB) = customerWithAddress(bankB) + When("We update bank B's customer address through bank A's path") + val request310 = (v3_1_0_Request / "banks" / bankA / "customers" / customerAtB / "addresses" / addressAtB).PUT <@(user1) + val response310 = makePutRequest(request310, write(postCustomerAddressJson.copy(city = "Elsewhere"))) + Then("We should get a 404") + response310.code should equal(404) + And("error should be " + CustomerNotFoundByCustomerId) + response310.body.extract[ErrorMessage].message should startWith (CustomerNotFoundByCustomerId) + } + + scenario("An address of another customer than the one in the path is not found", ApiEndpoint4, VersionOfApi) { + for (role <- List(CanCreateCustomer, CanCreateCustomerAddress)) + Entitlement.entitlement.vend.addEntitlement(bankA, resourceUser1.userId, role.toString) + val (customer1, _) = customerWithAddress(bankA) + val (_, addressOfCustomer2) = customerWithAddress(bankA) + When("We update customer 2's address through customer 1's path") + val request310 = (v3_1_0_Request / "banks" / bankA / "customers" / customer1 / "addresses" / addressOfCustomer2).PUT <@(user1) + val response310 = makePutRequest(request310, write(postCustomerAddressJson.copy(city = "Elsewhere"))) + Then("We should get a 404") + response310.code should equal(404) + And("error should be " + CustomerAddressNotFound) + response310.body.extract[ErrorMessage].message should startWith (CustomerAddressNotFound) + } + } + } diff --git a/scripts/resource_doc_baseline/parity_allowlist.json b/scripts/resource_doc_baseline/parity_allowlist.json index ca6aa15b4a..d80307196f 100644 --- a/scripts/resource_doc_baseline/parity_allowlist.json +++ b/scripts/resource_doc_baseline/parity_allowlist.json @@ -2195,6 +2195,14 @@ "lift_digest": "6a0652dbac6383372df2418f0456676eb1ab4ea8d7e3d5e10efa2e2bb81ff510", "http4s_digest": "1769e014d63887938573f77828aaa7669b69c23f8c4de3bb286767aad5185754" }, + { + "version": "v3_1_0", + "endpoint": "updateCustomerAddress", + "field": "errorResponseBodies", + "reason": "Lists CustomerNotFoundByCustomerId (404: the customer is not at the BANK_ID in the URL) and CustomerAddressNotFound (404: the address is not one of the CUSTOMER_ID's), so a Role at one bank no longer lets the caller change addresses at another.", + "lift_digest": "647479a642a49ee3a80a4bfc28ab2ae3413796d26a3eecbc48411d6e05620ad5", + "http4s_digest": "3a6402f32d558c47caf71d2eda8cf531d4e6a5228039bb567f09d57d66441cfa" + }, { "version": "v5_0_0", "endpoint": "createUserAuthContextUpdateRequest", From 82075b00c0dc2cb93fae9ac401b50d188adf0815 Mon Sep 17 00:00:00 2001 From: simonredfern Date: Mon, 19 Oct 2026 00:02:33 +0100 Subject: [PATCH 07/30] Fix: Get Customer by CUSTOMER_ID checks the customer's bank --- .../main/scala/code/api/v3_1_0/Http4s310.scala | 8 ++++++++ .../scala/code/api/v3_1_0/CustomerTest.scala | 17 +++++++++++++++++ .../resource_doc_baseline/parity_allowlist.json | 8 ++++++++ 3 files changed, 33 insertions(+) diff --git a/obp-api/src/main/scala/code/api/v3_1_0/Http4s310.scala b/obp-api/src/main/scala/code/api/v3_1_0/Http4s310.scala index e40f06e588..b7acfb1d43 100644 --- a/obp-api/src/main/scala/code/api/v3_1_0/Http4s310.scala +++ b/obp-api/src/main/scala/code/api/v3_1_0/Http4s310.scala @@ -772,6 +772,12 @@ object Http4s310 { for { _ <- NewStyle.function.hasEntitlement(bank.bankId.value, user.userId, canGetCustomersAtOneBank, Some(cc)) (customer, _) <- NewStyle.function.getCustomerByCustomerId(customerIdStr, Some(cc)) + // The customer must be at the bank in the path, where the Role is held, else the Role at one bank + // would let the caller read customers at any bank + // 404, as for a customer that does not exist, so nothing is told about other banks' customers + _ <- code.util.Helper.booleanToFuture(CustomerNotFoundByCustomerId, 404, Some(cc)) { + customer.bankId == bank.bankId.value + } (customerAttributes, _) <- NewStyle.function.getCustomerAttributes( bank.bankId, CustomerId(customerIdStr), Some(cc)) } yield JSONFactory310.createCustomerWithAttributesJson(customer, customerAttributes) @@ -796,6 +802,8 @@ object Http4s310 { AuthenticatedUserIsRequired, UserHasMissingRoles, UserCustomerLinksNotFoundForUser, + // 404 when the customer does not exist or is at another bank than the one in the path + CustomerNotFoundByCustomerId, UnknownError ), List(apiTagCustomer), diff --git a/obp-api/src/test/scala/code/api/v3_1_0/CustomerTest.scala b/obp-api/src/test/scala/code/api/v3_1_0/CustomerTest.scala index 1235aea5a7..04d42cf626 100644 --- a/obp-api/src/test/scala/code/api/v3_1_0/CustomerTest.scala +++ b/obp-api/src/test/scala/code/api/v3_1_0/CustomerTest.scala @@ -158,6 +158,23 @@ class CustomerTest extends V310ServerSetup with PropsReset{ And("error should be " + CustomerNotFoundByCustomerId) response310.body.extract[ErrorMessage].message should startWith (CustomerNotFoundByCustomerId) } + // The Role at bank A must not read a customer of bank B through bank A's path + scenario("A customer at another bank than the one in the path is not found", ApiEndpoint1, VersionOfApi) { + val bankA = testBankId1.value + val bankB = testBankId2.value + Entitlement.entitlement.vend.addEntitlement(bankB, resourceUser1.userId, CanCreateCustomer.toString) + Entitlement.entitlement.vend.addEntitlement(bankA, resourceUser1.userId, CanGetCustomersAtOneBank.toString) + val customerAtB = makePostRequest((v3_1_0_Request / "banks" / bankB / "customers").POST <@(user1), write(postCustomerJson)) + customerAtB.code should equal(201) + val customerIdAtB = customerAtB.body.extract[CustomerJsonV310].customer_id + When("We get bank B's customer through bank A's path, holding " + canGetCustomersAtOneBank + " at bank A") + val request310 = (v3_1_0_Request / "banks" / bankA / "customers" / customerIdAtB).GET <@(user1) + val response310 = makeGetRequest(request310) + Then("We should get a 404") + response310.code should equal(404) + And("error should be " + CustomerNotFoundByCustomerId) + response310.body.extract[ErrorMessage].message should startWith (CustomerNotFoundByCustomerId) + } } feature("Get Customer by customer number v3.1.0 - Unauthorized access") { diff --git a/scripts/resource_doc_baseline/parity_allowlist.json b/scripts/resource_doc_baseline/parity_allowlist.json index d80307196f..7cd7bbbc7e 100644 --- a/scripts/resource_doc_baseline/parity_allowlist.json +++ b/scripts/resource_doc_baseline/parity_allowlist.json @@ -627,6 +627,14 @@ "lift_digest": "0b18308242af8b26c8ed51c3dda373102a35ddc0fc88a0512f972788cc1c410c", "http4s_digest": "9a09487c0a48159914955edf656a8c22cd1b959f54216841b08dcc5bb7f56130" }, + { + "version": "v3_1_0", + "endpoint": "getCustomerByCustomerId", + "field": "errorResponseBodies", + "reason": "Added CustomerNotFoundByCustomerId: 404 when the customer does not exist or belongs to another bank than BANK_ID", + "lift_digest": "ae81ce0e83dc0ab8e4b334d14fbcc820c87319c8e7a77cc656f38b602732807d", + "http4s_digest": "562688c414d1fb8d3622b3ea3677c0b148549cb369f9f35e28c10bf8ff7a87f4" + }, { "version": "v3_1_0", "endpoint": "getFirehoseCustomers", From e2b9d5e7c00ffcdbb221fbe4751fde55b7ab928d Mon Sep 17 00:00:00 2001 From: simonredfern Date: Mon, 19 Oct 2026 00:12:14 +0100 Subject: [PATCH 08/30] Fix: Update Account Attribute checks the attribute's bank --- .../scala/code/api/util/ErrorMessages.scala | 2 ++ .../scala/code/api/v3_1_0/Http4s310.scala | 12 ++++++++-- .../api/v3_1_0/AccountAttributeTest.scala | 22 +++++++++++++++++++ .../parity_allowlist.json | 8 +++++++ 4 files changed, 42 insertions(+), 2 deletions(-) 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 b3b26adb26..b8aef81b5a 100644 --- a/obp-api/src/main/scala/code/api/util/ErrorMessages.scala +++ b/obp-api/src/main/scala/code/api/util/ErrorMessages.scala @@ -501,6 +501,8 @@ object ErrorMessages { val InvestigationReportNotAvailable = "OBP-30150: Investigation Report is only available in mapped mode (connector=mapped)." val NotificationWebhookNotFound = "OBP-30151: Account Notification Webhook not found. Please specify a valid value for WEBHOOK_ID." val DeleteWebhookError = "OBP-30152: Could not delete the Webhook." + // an account attribute that does not exist, or does not belong to the BANK_ID / ACCOUNT_ID / PRODUCT_CODE in the path + val AccountAttributeNotFound = "OBP-30153: Account Attribute not found. Please specify a valid value for ACCOUNT_ATTRIBUTE_ID for this BANK_ID, ACCOUNT_ID and PRODUCT_CODE." val CreateWebhookError = "OBP-30047: Cannot create Webhook" val GetWebhooksError = "OBP-30048: Cannot get Webhooks" diff --git a/obp-api/src/main/scala/code/api/v3_1_0/Http4s310.scala b/obp-api/src/main/scala/code/api/v3_1_0/Http4s310.scala index b7acfb1d43..e2619766a6 100644 --- a/obp-api/src/main/scala/code/api/v3_1_0/Http4s310.scala +++ b/obp-api/src/main/scala/code/api/v3_1_0/Http4s310.scala @@ -3348,7 +3348,14 @@ object Http4s310 { 400, Some(cc)) { AccountAttributeType.withName(postedData.`type`) } (_, _) <- NewStyle.function.getBankAccount(BankId(bankIdStr), AccountId(accountIdStr), Some(cc)) (_, _) <- NewStyle.function.getProduct(BankId(bankIdStr), ProductCode(productCodeStr), Some(cc)) - (_, _) <- NewStyle.function.getAccountAttributeById(accountAttributeIdStr, Some(cc)) + // keep the existing attribute so it can be checked against the path + (existingAttribute, _) <- NewStyle.function.getAccountAttributeById(accountAttributeIdStr, Some(cc)) + // the attribute must belong to the bank, account and product in the path, else a role at one bank overwrites and moves any bank's attribute + _ <- code.util.Helper.booleanToFuture(AccountAttributeNotFound, 404, Some(cc)) { + existingAttribute.bankId.value == bankIdStr && + existingAttribute.accountId.value == accountIdStr && + existingAttribute.productCode.value == productCodeStr + } (accountAttribute, _) <- NewStyle.function.createOrUpdateAccountAttribute( BankId(bankIdStr), AccountId(accountIdStr), ProductCode(productCodeStr), Some(accountAttributeIdStr), postedData.name, accountAttributeType, postedData.value, @@ -3385,7 +3392,8 @@ object Http4s310 { |""", accountAttributeJson, accountAttributeResponseJson, - List(AuthenticatedUserIsRequired, InvalidJsonFormat, UnknownError), + // AccountAttributeNotFound: the attribute is not under the BANK_ID / ACCOUNT_ID / PRODUCT_CODE in the path + List(AuthenticatedUserIsRequired, InvalidJsonFormat, AccountAttributeNotFound, UnknownError), List(apiTagAccount, apiTagAccountAttribute, apiTagAttribute), Some(List(canUpdateAccountAttribute)), http4sPartialFunction = Some(updateAccountAttribute) diff --git a/obp-api/src/test/scala/code/api/v3_1_0/AccountAttributeTest.scala b/obp-api/src/test/scala/code/api/v3_1_0/AccountAttributeTest.scala index 4fdb6b8081..495c435ed2 100644 --- a/obp-api/src/test/scala/code/api/v3_1_0/AccountAttributeTest.scala +++ b/obp-api/src/test/scala/code/api/v3_1_0/AccountAttributeTest.scala @@ -171,6 +171,28 @@ class AccountAttributeTest extends V310ServerSetup { errorMessage contains (UserHasMissingRoles) should be (true) errorMessage contains (canUpdateAccountAttribute.toString()) should be (true) } + // a role at one bank must not let the caller overwrite (and move) an attribute of another bank's account + scenario("We will call the Update endpoint with an attribute of another bank", ApiEndpoint2, VersionOfApi) { + val productCode = APIUtil.generateUUID() + createProduct(bankId = testBankId1.value, code = productCode, json = parentPostPutProductJsonV310) + createProduct(bankId = testBankId2.value, code = productCode, json = parentPostPutProductJsonV310) + Entitlement.entitlement.vend.addEntitlement(testBankId2.value, resourceUser1.userId, CanCreateAccountAttributeAtOneBank.toString) + val requestCreate310 = (v3_1_0_Request / "banks" / testBankId2.value / "accounts" / testAccountId0.value / + "products" / productCode / "attribute").POST <@(user1) + val responseCreate310 = makePostRequest(requestCreate310, write(postAccountAttributeJson)) + responseCreate310.code should equal(201) + val otherBanksAttribute = responseCreate310.body.extract[AccountAttributeResponseJson] + + Entitlement.entitlement.vend.addEntitlement(testBankId1.value, resourceUser1.userId, CanUpdateAccountAttribute.toString) + When(s"We make a request $VersionOfApi at bank 1 with the attribute of bank 2") + val requestPut310 = (v3_1_0_Request / "banks" / testBankId1.value / "accounts" / testAccountId0.value + / "products" / productCode / "attributes" / otherBanksAttribute.account_attribute_id).PUT <@(user1) + val responsePut310 = makePutRequest(requestPut310, write(putAccountAttributeJson)) + Then("We should get a 404") + responsePut310.code should equal(404) + And("error should be " + AccountAttributeNotFound) + responsePut310.body.extract[ErrorMessage].message should equal (AccountAttributeNotFound) + } } } diff --git a/scripts/resource_doc_baseline/parity_allowlist.json b/scripts/resource_doc_baseline/parity_allowlist.json index 7cd7bbbc7e..32db1509a8 100644 --- a/scripts/resource_doc_baseline/parity_allowlist.json +++ b/scripts/resource_doc_baseline/parity_allowlist.json @@ -2211,6 +2211,14 @@ "lift_digest": "647479a642a49ee3a80a4bfc28ab2ae3413796d26a3eecbc48411d6e05620ad5", "http4s_digest": "3a6402f32d558c47caf71d2eda8cf531d4e6a5228039bb567f09d57d66441cfa" }, + { + "version": "v3_1_0", + "endpoint": "updateAccountAttribute", + "field": "errorResponseBodies", + "reason": "Added AccountAttributeNotFound (404): the attribute must belong to the BANK_ID, ACCOUNT_ID and PRODUCT_CODE in the path, else a role at one bank overwrites and moves another bank's attribute", + "lift_digest": "ab6b9a504c9dafbcc134976d9f0b976cbe222c9054b9c811c5570adfb79ae6ce", + "http4s_digest": "7c341afe281a974e145362105b5ad7be060660743ac6e3b064fdaa02e586c97b" + }, { "version": "v5_0_0", "endpoint": "createUserAuthContextUpdateRequest", From 45b05558f260c18be4f29328354cdf35d7cc201f Mon Sep 17 00:00:00 2001 From: simonredfern Date: Mon, 19 Oct 2026 00:42:49 +0100 Subject: [PATCH 09/30] Fix: Get account balances v5.1.0 needs a balance view --- .../src/main/scala/code/api/v5_1_0/Http4s510.scala | 12 ++++++++++-- .../code/api/v5_1_0/BankAccountBalanceTest.scala | 8 ++++++++ scripts/resource_doc_baseline/parity_allowlist.json | 8 ++++++++ 3 files changed, 26 insertions(+), 2 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 7740cfa059..c733e25e04 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 @@ -966,7 +966,14 @@ object Http4s510 { implicit val cc: code.api.util.CallContext = req.callContext val accountId = AccountId(accountIdStr) for { - _ <- Future.successful(cc.user.openOrThrowException(AuthenticatedUserIsRequired)) + user <- Future.successful(cc.user.openOrThrowException(AuthenticatedUserIsRequired)) + // The caller needs a view on this account that can see balances, else any logged-in user reads any account's balances + _ <- Helper.booleanToFuture( + ViewDoesNotPermitAccess + s" You need the `${CAN_SEE_BANK_ACCOUNT_BALANCE}` permission on any of your views", + 403, cc = Some(cc)) { + Views.views.vend.privateViewsUserCanAccessForAccount(user, BankIdAccountId(BankId(bankIdStr), accountId)) + .exists(_.allowed_actions.exists(_ == CAN_SEE_BANK_ACCOUNT_BALANCE)) + } (balances, _) <- code.api.util.newstyle.BankAccountBalanceNewStyle.getBankAccountBalances(accountId, Some(cc)) } yield JSONFactory510.createBankAccountBalancesJson(balances) } @@ -985,7 +992,8 @@ object Http4s510 { |""", EmptyBody, bankAccountBalancesJsonV510, - List($AuthenticatedUserIsRequired, UserHasMissingRoles, UnknownError), + // BankNotFound / BankAccountNotFound come from the middleware; ViewDoesNotPermitAccess from the balance view check + List($AuthenticatedUserIsRequired, $BankNotFound, $BankAccountNotFound, ViewDoesNotPermitAccess, UserHasMissingRoles, UnknownError), List(apiTagAccount, apiTagBalance), None, http4sPartialFunction = Some(getAllBankAccountBalances) diff --git a/obp-api/src/test/scala/code/api/v5_1_0/BankAccountBalanceTest.scala b/obp-api/src/test/scala/code/api/v5_1_0/BankAccountBalanceTest.scala index 2531a551ce..ba648f014e 100644 --- a/obp-api/src/test/scala/code/api/v5_1_0/BankAccountBalanceTest.scala +++ b/obp-api/src/test/scala/code/api/v5_1_0/BankAccountBalanceTest.scala @@ -159,6 +159,14 @@ class BankAccountBalanceTest extends V510ServerSetup with DefaultUsers { response.code should equal(401) } + // user2 has no view on user1's account, so it must not read that account's balances + scenario("403 Forbidden (no view that can see balances)", GetAll, VersionOfApi) { + val request = (v5_1_0_Request / "banks" / bankId / "accounts" / accountId / "balances").GET <@ user2 + val response = makeGetRequest(request) + response.code should equal(403) + response.body.extract[ErrorMessage].message should startWith(ErrorMessages.ViewDoesNotPermitAccess) + } + scenario("200 Success", GetAll, VersionOfApi) { lazy val bankId = testBankId1.value lazy val accountId = testAccountId1.value diff --git a/scripts/resource_doc_baseline/parity_allowlist.json b/scripts/resource_doc_baseline/parity_allowlist.json index 32db1509a8..5288e5f029 100644 --- a/scripts/resource_doc_baseline/parity_allowlist.json +++ b/scripts/resource_doc_baseline/parity_allowlist.json @@ -2283,6 +2283,14 @@ "lift_digest": "692ebc3c9d61794ffa3032f1cd759b586823239cccdf3a78c58ea9253bd3a159", "http4s_digest": "a8bdac82543f60b89328544bdd0027259566919925b24729a7c84f30a795ca56" }, + { + "version": "v5_1_0", + "endpoint": "getAllBankAccountBalances", + "field": "errorResponseBodies", + "reason": "Added BankNotFound, BankAccountNotFound (middleware) and ViewDoesNotPermitAccess: the caller now needs a view with can_see_bank_account_balance on the account", + "lift_digest": "cb07b8a8074c71a21a64f9030e412d57858383e4f503c2444c6c7000be5f4030", + "http4s_digest": "95338e12a64f58edfdea236bf3ff5c9e7a2ee23446b662b6134809ce8782db6d" + }, { "version": "v6_0_0", "endpoint": "getConsumer", From 00d1bb2044ee110dadb830ad622b9512d48795aa Mon Sep 17 00:00:00 2001 From: simonredfern Date: Mon, 19 Oct 2026 00:53:07 +0100 Subject: [PATCH 10/30] Fix: Create Bank refuses an existing bank id in v2.2.0 and v4.0.0 --- .../scala/code/api/v2_2_0/Http4s220.scala | 9 ++- .../scala/code/api/v4_0_0/Http4s400.scala | 8 +++ .../test/scala/code/api/v2_2_0/BankTest.scala | 69 +++++++++++++++++++ .../scala/code/api/v4_0_0/BankTests.scala | 17 +++++ .../parity_allowlist.json | 16 +++++ 5 files changed, 118 insertions(+), 1 deletion(-) create mode 100644 obp-api/src/test/scala/code/api/v2_2_0/BankTest.scala diff --git a/obp-api/src/main/scala/code/api/v2_2_0/Http4s220.scala b/obp-api/src/main/scala/code/api/v2_2_0/Http4s220.scala index ab86a714b6..fd1e2db293 100644 --- a/obp-api/src/main/scala/code/api/v2_2_0/Http4s220.scala +++ b/obp-api/src/main/scala/code/api/v2_2_0/Http4s220.scala @@ -470,6 +470,12 @@ object Http4s220 { NewStyle.function.hasEntitlementAndScope("", user.userId, consumer.id.get.toString, canCreateBank, Some(cc)), Some(cc), UserHasMissingRoles + canCreateBank) } + // Refuse an existing BANK_ID (as v5.0.0 / v6.0.0 do): createOrUpdateBank would overwrite that + // bank and the grants below would make the caller entitlement admin at a bank they did not create. + (banks, _) <- NewStyle.function.getBanks(Some(cc)) + _ <- code.util.Helper.booleanToFuture(bankIdAlreadyExists, cc = Some(cc)) { + !banks.exists(_.bankId.value == bank.id) + } (success, _) <- NewStyle.function.createOrUpdateBank( bank.id, bank.full_name, bank.short_name, bank.logo_url, bank.website_url, bank.swift_bic, bank.national_identifier, @@ -501,7 +507,8 @@ object Http4s220 { s"""Create a new bank (Authenticated access). |${userAuthenticationMessage(true)}""", bankJSONV220, bankJSONV220, - List(InvalidJsonFormat, AuthenticatedUserIsRequired, InsufficientAuthorisationToCreateBank, UnknownError), + // bankIdAlreadyExists: the BANK_ID in the body is already taken (400) + List(InvalidJsonFormat, AuthenticatedUserIsRequired, InsufficientAuthorisationToCreateBank, bankIdAlreadyExists, UnknownError), List(apiTagBank, apiTagOldStyle), Some(List(canCreateBank)), http4sPartialFunction = Some(createBank)) diff --git a/obp-api/src/main/scala/code/api/v4_0_0/Http4s400.scala b/obp-api/src/main/scala/code/api/v4_0_0/Http4s400.scala index 51670f2ef3..102af2afcb 100644 --- a/obp-api/src/main/scala/code/api/v4_0_0/Http4s400.scala +++ b/obp-api/src/main/scala/code/api/v4_0_0/Http4s400.scala @@ -485,6 +485,13 @@ object Http4s400 { _ <- code.util.Helper.booleanToFuture( failMsg = s"$InvalidJsonFormat BANK_ID can not contain `::::` characters", cc = Some(cc)) { !APIUtil.`checkIfContains::::`(bank.id) } + // Refuse an existing BANK_ID (as v5.0.0 / v6.0.0 do): createOrUpdateBank would overwrite that + // bank and the grants below would make the caller entitlement admin at a bank they did not create. + (banks, _) <- NewStyle.function.getBanks(Some(cc)) + _ <- code.util.Helper.booleanToFuture( + failMsg = bankIdAlreadyExists, cc = Some(cc)) { + !banks.exists(_.bankId.value == bank.id) + } (success, _) <- NewStyle.function.createOrUpdateBank( bank.id, bank.full_name, bank.short_name, bank.logo, bank.website, bank.bank_routings.find(_.scheme == "BIC").map(_.address).getOrElse(""), @@ -529,6 +536,7 @@ object Http4s400 { InvalidJsonFormat, $AuthenticatedUserIsRequired, InsufficientAuthorisationToCreateBank, + bankIdAlreadyExists, // the BANK_ID in the body is already taken (400) UnknownError ), List(apiTagBank), diff --git a/obp-api/src/test/scala/code/api/v2_2_0/BankTest.scala b/obp-api/src/test/scala/code/api/v2_2_0/BankTest.scala new file mode 100644 index 0000000000..d5fd06d119 --- /dev/null +++ b/obp-api/src/test/scala/code/api/v2_2_0/BankTest.scala @@ -0,0 +1,69 @@ +/** +Open Bank Project - API +Copyright (C) 2011-2026, TESOBE GmbH. + +This program is free software: you can redistribute it and/or modify +it under the terms of the GNU Affero General Public License as published by +the Free Software Foundation, either version 3 of the License, or +(at your option) any later version. + +This program is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU Affero General Public License for more details. + +You should have received a copy of the GNU Affero General Public License +along with this program. If not, see . + +Email: contact@tesobe.com +TESOBE GmbH. +Osloer Strasse 16/17 +Berlin 13359, Germany + +This product includes software developed at +TESOBE (http://www.tesobe.com/) + + */ +package code.api.v2_2_0 + +import org.json4s._ +import code.api.ResourceDocs1_4_0.SwaggerDefinitionsJSON.bankJSONV220 +import code.api.util.APIUtil.OAuth._ +import code.api.util.{ApiRole, ErrorMessages, NewStyle} +import code.api.v2_2_0.Http4s220.Implementations2_2_0 +import code.entitlement.Entitlement +import code.setup.DefaultUsers +import com.github.dwickern.macros.NameOf.nameOf +import com.openbankproject.commons.model.ErrorMessage +import com.openbankproject.commons.util.ApiVersion +import org.json4s.native.Serialization.write +import org.scalatest.Tag + +import scala.concurrent.Await +import scala.concurrent.duration._ + +class BankTest extends V220ServerSetup with DefaultUsers { + + object VersionOfApi extends Tag(ApiVersion.v2_2_0.toString) + object ApiEndpoint1 extends Tag(nameOf(Implementations2_2_0.createBank)) + + feature("Assuring that endpoint createBank works as expected - v2.2.0") { + + // An existing BANK_ID is refused, so CanCreateBank cannot overwrite another bank or gain its entitlement admin role + scenario("We try to create a bank with the BANK_ID of an existing bank - Authorized access", ApiEndpoint1, VersionOfApi) { + Given("A bank that already exists and a user with CanCreateBank") + val existingBankId = createBank("testBank0").bankId.value + Entitlement.entitlement.vend.addEntitlement("", resourceUser1.userId, ApiRole.CanCreateBank.toString) + When("We post a bank with that BANK_ID") + val request = (v2_2Request / "banks").POST <@ (user1) + val response = makePostRequest(request, write(bankJSONV220.copy(id = existingBankId))) + Then("We should get a 400 with bankIdAlreadyExists") + response.code should equal(400) + response.body.extract[ErrorMessage].message should equal(ErrorMessages.bankIdAlreadyExists) + And("The caller is not given CanCreateEntitlementAtOneBank at that bank") + Await.result( + NewStyle.function.getEntitlementsByUserId(resourceUser1.userId, None), 10.seconds + ).exists(e => e.roleName == ApiRole.CanCreateEntitlementAtOneBank.toString && e.bankId == existingBankId) should equal(false) + } + } +} diff --git a/obp-api/src/test/scala/code/api/v4_0_0/BankTests.scala b/obp-api/src/test/scala/code/api/v4_0_0/BankTests.scala index 40ef3a632e..99c940e604 100644 --- a/obp-api/src/test/scala/code/api/v4_0_0/BankTests.scala +++ b/obp-api/src/test/scala/code/api/v4_0_0/BankTests.scala @@ -112,6 +112,23 @@ class BankTests extends V400ServerSetup with DefaultUsers { defaultOutgoingAccount._1.accountId.value should equal(OUTGOING_SETTLEMENT_ACCOUNT_ID) defaultIncomingAccount._1.accountId.value should equal(INCOMING_SETTLEMENT_ACCOUNT_ID) } + + // An existing BANK_ID is refused, so CanCreateBank cannot overwrite another bank or gain its entitlement admin role + scenario("We try to create a bank with the BANK_ID of an existing bank - Authorized access", ApiEndpoint1, VersionOfApi) { + Given("A bank that already exists and a user with CanCreateBank") + val existingBankId = createBank("testBank0").bankId.value + Entitlement.entitlement.vend.addEntitlement("", resourceUser1.userId, ApiRole.CanCreateBank.toString) + When("We post a bank with that BANK_ID") + val request = (v4_0_0_Request / "banks").POST <@ (user1) + val response = makePostRequest(request, write(bankJson400.copy(id = existingBankId))) + Then("We should get a 400 with bankIdAlreadyExists") + response.code should equal(400) + response.body.extract[ErrorMessage].message should equal(ErrorMessages.bankIdAlreadyExists) + And("The caller is not given CanCreateEntitlementAtOneBank at that bank") + Await.result( + NewStyle.function.getEntitlementsByUserId(resourceUser1.userId, None), 10.seconds + ).exists(e => e.roleName == ApiRole.CanCreateEntitlementAtOneBank.toString && e.bankId == existingBankId) should equal(false) + } } diff --git a/scripts/resource_doc_baseline/parity_allowlist.json b/scripts/resource_doc_baseline/parity_allowlist.json index 5288e5f029..e49f7a217f 100644 --- a/scripts/resource_doc_baseline/parity_allowlist.json +++ b/scripts/resource_doc_baseline/parity_allowlist.json @@ -1939,6 +1939,22 @@ "lift_digest": "4cb0d642b4a86b86be87cad67a311c77f1d68aacafb957b2e4e83d23ae9695fb", "http4s_digest": "5914176f0c140e0e56de59f39988dd4bd8312a0c873aa816729a12d5037ae936" }, + { + "version": "v2_2_0", + "endpoint": "createBank", + "field": "errorResponseBodies", + "reason": "Added bankIdAlreadyExists: createBank now refuses an existing BANK_ID instead of overwriting that bank and granting the caller CanCreateEntitlementAtOneBank there", + "lift_digest": "d9be7cb75e5f529810a1ba5852fa29f17b2137bc77c4dc7cd0c794b22fbd16bd", + "http4s_digest": "d21fc395f1489dfb4bf5b8cea84e210a92b642752e5dbaaa88791e4e5e709022" + }, + { + "version": "v4_0_0", + "endpoint": "createBank", + "field": "errorResponseBodies", + "reason": "Added bankIdAlreadyExists: createBank now refuses an existing BANK_ID instead of overwriting that bank and granting the caller CanCreateEntitlementAtOneBank there", + "lift_digest": "52de8e9110317d5430e3f54a267aa69b391f6dcddce3b99c8d40a7fee9ef0bc6", + "http4s_digest": "5d830c45b454632969c46172008000d36ffeb4e87afc980126f4d9d6e7955486" + }, { "version": "v4_0_0", "endpoint": "createConsumer", From 94d5fd8483fd7b887eaef4423eadf3acb69afd79 Mon Sep 17 00:00:00 2001 From: simonredfern Date: Mon, 19 Oct 2026 00:59:10 +0100 Subject: [PATCH 11/30] Fix: Don't allow v4.0.0 to update an existing bank. --- .../src/main/scala/code/api/v4_0_0/Http4s400.scala | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/obp-api/src/main/scala/code/api/v4_0_0/Http4s400.scala b/obp-api/src/main/scala/code/api/v4_0_0/Http4s400.scala index 102af2afcb..b0578fe9c6 100644 --- a/obp-api/src/main/scala/code/api/v4_0_0/Http4s400.scala +++ b/obp-api/src/main/scala/code/api/v4_0_0/Http4s400.scala @@ -499,17 +499,24 @@ object Http4s400 { bank.bank_routings.filterNot(_.scheme == "BIC").headOption.map(_.scheme).getOrElse(""), bank.bank_routings.filterNot(_.scheme == "BIC").headOption.map(_.address).getOrElse(""), Some(cc)) - entitlements <- NewStyle.function.getEntitlementsByUserId(cc.userId, Some(cc)) + // Creator grants target the HUMAN (as v5.0.0 / v6.0.0 do): under a Consent the + // authenticated user is a per-consent shadow, and roles granted to it are stranded. + humanUserId = cc.onBehalfOfUserId + entitlements <- NewStyle.function.getEntitlementsByUserId(humanUserId, Some(cc)) entitlementsByBank = entitlements.filter(_.bankId == bank.id) _ <- entitlementsByBank.exists(_.roleName == CanCreateEntitlementAtOneBank.toString()) match { case true => Future.successful(()) + // grant to the human, recording the authenticated caller as the granter case false => Future(Entitlement.entitlement.vend.addEntitlement( - bank.id, cc.userId, CanCreateEntitlementAtOneBank.toString())) + bank.id, humanUserId, CanCreateEntitlementAtOneBank.toString(), + grantedByUserId = Some(cc.userId))) } _ <- entitlementsByBank.exists(_.roleName == CanReadDynamicResourceDocsAtOneBank.toString()) match { case true => Future.successful(()) + // grant to the human, recording the authenticated caller as the granter case false => Future(Entitlement.entitlement.vend.addEntitlement( - bank.id, cc.userId, CanReadDynamicResourceDocsAtOneBank.toString())) + bank.id, humanUserId, CanReadDynamicResourceDocsAtOneBank.toString(), + grantedByUserId = Some(cc.userId))) } } yield JSONFactory400.createBankJSON400(success) } From 6476ed8fa8912d753ffd8bab4e1a2b55ace9ef8e Mon Sep 17 00:00:00 2001 From: simonredfern Date: Mon, 19 Oct 2026 01:09:03 +0100 Subject: [PATCH 12/30] Fix: Add My Api Collection Endpoint By Id checks the owner --- .../main/scala/code/api/v4_0_0/Http4s400.scala | 12 ++++++++++-- .../api/v4_0_0/ApiCollectionEndpointTest.scala | 18 ++++++++++++++++++ .../parity_allowlist.json | 8 ++++++++ 3 files changed, 36 insertions(+), 2 deletions(-) diff --git a/obp-api/src/main/scala/code/api/v4_0_0/Http4s400.scala b/obp-api/src/main/scala/code/api/v4_0_0/Http4s400.scala index b0578fe9c6..8dcea4dee4 100644 --- a/obp-api/src/main/scala/code/api/v4_0_0/Http4s400.scala +++ b/obp-api/src/main/scala/code/api/v4_0_0/Http4s400.scala @@ -5303,7 +5303,8 @@ object Http4s400 { lazy val createMyApiCollectionEndpointById: Http4sRoute = Http4sRoute { case req @ POST -> `prefixPath` / "my" / "api-collection-ids" / apiCollectionIdStr / "api-collection-endpoints" => - EndpointHelpers.withUserAndBodyCreated[PostApiCollectionEndpointJson400, Any](req) { (_, postJson, cc) => + // take the logged-in user, so the collection can be checked to be theirs + EndpointHelpers.withUserAndBodyCreated[PostApiCollectionEndpointJson400, Any](req) { (user, postJson, cc) => for { _ <- code.util.Helper.booleanToFuture( s"$InvalidOperationId Current OPERATION_ID(${postJson.operation_id})", @@ -5311,6 +5312,12 @@ object Http4s400 { getAllResourceDocs.find(_.operationId == postJson.operation_id.trim).isDefined } (apiCollection, _) <- NewStyle.function.getApiCollectionById(apiCollectionIdStr, Some(cc)) + // the collection must belong to the caller (a /my/ path), else any user adds endpoints to anyone's collection by id + _ <- code.util.Helper.booleanToFuture( + s"$ApiCollectionNotFound Please specify a valid value for API_COLLECTION_ID. Current API_COLLECTION_ID($apiCollectionIdStr) ", + 404, cc = Some(cc)) { + apiCollection.userId == user.userId + } existing <- Future { code.apicollectionendpoint.MappedApiCollectionEndpointsProvider .getApiCollectionEndpointByApiCollectionIdAndOperationId( @@ -5505,7 +5512,8 @@ object Http4s400 { |""".stripMargin, postApiCollectionEndpointJson400, apiCollectionEndpointJson400, - List($AuthenticatedUserIsRequired, InvalidJsonFormat, UnknownError), + // ApiCollectionNotFound: the collection does not exist or is not the caller's + List($AuthenticatedUserIsRequired, InvalidJsonFormat, ApiCollectionNotFound, UnknownError), List(apiTagApiCollection), None, http4sPartialFunction = Some(createMyApiCollectionEndpointById) diff --git a/obp-api/src/test/scala/code/api/v4_0_0/ApiCollectionEndpointTest.scala b/obp-api/src/test/scala/code/api/v4_0_0/ApiCollectionEndpointTest.scala index 0c390caaf2..943cc32c74 100644 --- a/obp-api/src/test/scala/code/api/v4_0_0/ApiCollectionEndpointTest.scala +++ b/obp-api/src/test/scala/code/api/v4_0_0/ApiCollectionEndpointTest.scala @@ -29,6 +29,8 @@ import org.json4s._ import code.api.ResourceDocs1_4_0.SwaggerDefinitionsJSON import code.api.berlin.group.v1_3.Http4sBGv13Alias import code.api.util.APIUtil.OAuth._ +import code.api.util.ErrorMessages.ApiCollectionNotFound +import com.openbankproject.commons.model.ErrorMessage import code.api.v4_0_0.Http4s400.Implementations4_0_0 import com.github.dwickern.macros.NameOf.nameOf import com.openbankproject.commons.util.ApiVersion @@ -292,6 +294,22 @@ class ApiCollectionEndpointTest extends V400ServerSetup { apiCollectionsJsonGet400.api_collection_endpoints.length should be (expected) } } + + // another user must not add endpoints to user1's collection by its id + scenario(s"We try $ApiEndpoint6 on another user's collection", ApiEndpoint6, VersionOfApi) { + When("user1 creates an apiCollection") + val request = (v4_0_0_Request / "my" / "api-collections").POST <@ (user1) + val response = makePostRequest(request, write(SwaggerDefinitionsJSON.postApiCollectionJson400.copy(api_collection_name = "collection-of-user1"))) + response.code should equal(201) + val apiCollectionId = response.body.extract[ApiCollectionJson400].api_collection_id + + Then("user2 adds an endpoint to it by id") + val requestApiCollectionEndpoint = (v4_0_0_Request / "my" / "api-collection-ids" / apiCollectionId / "api-collection-endpoints").POST <@ (user2) + val responseApiCollectionEndpoint = makePostRequest(requestApiCollectionEndpoint, write(SwaggerDefinitionsJSON.postApiCollectionEndpointJson400)) + Then("We should get a 404") + responseApiCollectionEndpoint.code should equal(404) + responseApiCollectionEndpoint.body.extract[ErrorMessage].message should startWith(ApiCollectionNotFound) + } } } diff --git a/scripts/resource_doc_baseline/parity_allowlist.json b/scripts/resource_doc_baseline/parity_allowlist.json index e49f7a217f..95012ce146 100644 --- a/scripts/resource_doc_baseline/parity_allowlist.json +++ b/scripts/resource_doc_baseline/parity_allowlist.json @@ -1355,6 +1355,14 @@ "lift_digest": "f9dd12dddc733891adb4cdb66e8b35a3793f5d399699f695deb858f51d0d4c76", "http4s_digest": "b326f57bd423fd3c9ba3d10bdb27d38dd34137f9d9aaa2d9acdfd110b97618be" }, + { + "version": "v4_0_0", + "endpoint": "createMyApiCollectionEndpointById", + "field": "errorResponseBodies", + "reason": "added ApiCollectionNotFound: the collection must belong to the caller, else any user adds endpoints to another user's collection by id", + "lift_digest": "8b9e9294a47baff632975336cec3da4c29f3ca3b8398ec5875e9b5d1712f0140", + "http4s_digest": "f0f3b033370c9a7624829ee507012e09385025c6ac7b77e3dd4b37b6ac18d299" + }, { "version": "v4_0_0", "endpoint": "deleteMyApiCollection", From 7f117900629f4d3754e7d12e4f68762ee9d2ed14 Mon Sep 17 00:00:00 2001 From: simonredfern Date: Mon, 19 Oct 2026 01:13:33 +0100 Subject: [PATCH 13/30] Fix: Make sure Delete My Api Collection only deletes a collection owned by the logged-in user --- .../scala/code/api/v4_0_0/Http4s400.scala | 14 ++++++++--- .../code/api/v4_0_0/ApiCollectionTest.scala | 24 +++++++++++++++++-- .../parity_allowlist.json | 8 +++++++ 3 files changed, 41 insertions(+), 5 deletions(-) diff --git a/obp-api/src/main/scala/code/api/v4_0_0/Http4s400.scala b/obp-api/src/main/scala/code/api/v4_0_0/Http4s400.scala index 8dcea4dee4..0c95845544 100644 --- a/obp-api/src/main/scala/code/api/v4_0_0/Http4s400.scala +++ b/obp-api/src/main/scala/code/api/v4_0_0/Http4s400.scala @@ -7064,9 +7064,16 @@ object Http4s400 { lazy val deleteMyApiCollection: Http4sRoute = Http4sRoute { case req @ DELETE -> `prefixPath` / "my" / "api-collections" / apiCollectionId => - EndpointHelpers.withUserDelete(req) { (_, cc) => + // bind the user so the collection's owner can be checked below + EndpointHelpers.withUserDelete(req) { (user, cc) => for { - (_, _) <- NewStyle.function.getApiCollectionById(apiCollectionId, Some(cc)) + // keep the collection to compare its owner with the caller + (ac, _) <- NewStyle.function.getApiCollectionById(apiCollectionId, Some(cc)) + // only the owner deletes it, else any user deletes any user's collection; answered as not found so nothing is told about other users' collections + _ <- code.util.Helper.booleanToFuture( + s"$ApiCollectionNotFound Please specify a valid value for API_COLLECTION_ID. Current API_COLLECTION_ID($apiCollectionId) ", 404, Some(cc)) { + ac.userId == user.userId + } (deleted, _) <- NewStyle.function.deleteApiCollectionById(apiCollectionId, Some(cc)) } yield deleted } @@ -7352,7 +7359,8 @@ object Http4s400 { |""", EmptyBody, Full(true), - List($AuthenticatedUserIsRequired, UserNotFoundByUserId, UnknownError), + // ApiCollectionNotFound: an unknown id, or a collection of another user + List($AuthenticatedUserIsRequired, UserNotFoundByUserId, ApiCollectionNotFound, UnknownError), List(apiTagApiCollection), None, http4sPartialFunction = Some(deleteMyApiCollection) diff --git a/obp-api/src/test/scala/code/api/v4_0_0/ApiCollectionTest.scala b/obp-api/src/test/scala/code/api/v4_0_0/ApiCollectionTest.scala index 8cbbd34621..bdfdedafbd 100644 --- a/obp-api/src/test/scala/code/api/v4_0_0/ApiCollectionTest.scala +++ b/obp-api/src/test/scala/code/api/v4_0_0/ApiCollectionTest.scala @@ -29,7 +29,7 @@ import org.json4s._ import code.api.ResourceDocs1_4_0.SwaggerDefinitionsJSON import code.api.util.APIUtil.OAuth._ import code.api.util.ApiRole -import code.api.util.ErrorMessages.{ApiCollectionEndpointNotFound, UserHasMissingRoles, AuthenticatedUserIsRequired} +import code.api.util.ErrorMessages.{ApiCollectionEndpointNotFound, ApiCollectionNotFound, UserHasMissingRoles, AuthenticatedUserIsRequired} import code.api.v4_0_0.Http4s400.Implementations4_0_0 import code.entitlement.Entitlement import com.github.dwickern.macros.NameOf.nameOf @@ -172,7 +172,27 @@ class ApiCollectionTest extends V400ServerSetup { apiCollectionsJsonGetAfterDelete.api_collections.length should be (0) } - + + // a user must not delete another user's api collection: it is answered as not found and stays + scenario("We try to delete another user's apiCollection", ApiEndpoint4, VersionOfApi) { + When("user1 creates an apiCollection") + val request = (v4_0_0_Request / "my" / "api-collections").POST <@ (user1) + val response = makePostRequest(request, write(SwaggerDefinitionsJSON.postApiCollectionJson400)) + response.code should equal(201) + val apiCollectionJson400 = response.body.extract[ApiCollectionJson400] + + Then("user2 deletes it and gets a 404") + val requestDelete = (v4_0_0_Request / "my" / "api-collections" / apiCollectionJson400.api_collection_id).DELETE <@ (user2) + val responseDelete = makeDeleteRequest(requestDelete) + responseDelete.code should equal(404) + responseDelete.body.toString contains(s"$ApiCollectionNotFound") should be (true) + + Then("user1 still has the apiCollection") + val responseGet = makeGetRequest((v4_0_0_Request / "my" / "api-collections").GET <@ (user1)) + responseGet.code should equal(200) + responseGet.body.extract[ApiCollectionsJson400].api_collections should contain (apiCollectionJson400) + } + scenario("We create the apiCollection and get sharable api collection", ApiEndpoint5, ApiEndpoint6, VersionOfApi) { When("We make a request v4.0.0") diff --git a/scripts/resource_doc_baseline/parity_allowlist.json b/scripts/resource_doc_baseline/parity_allowlist.json index 95012ce146..e9e1d3f7e0 100644 --- a/scripts/resource_doc_baseline/parity_allowlist.json +++ b/scripts/resource_doc_baseline/parity_allowlist.json @@ -139,6 +139,14 @@ "lift_digest": "e46d3ae943664116b752137af856edb5f377c6d8012a39e3ef7f39f80a802cac", "http4s_digest": "a5655ec6126f28a76720a61f3e0a6eff066cff954c4003c3dc6def846451db76" }, + { + "version": "v4_0_0", + "endpoint": "deleteMyApiCollection", + "field": "errorResponseBodies", + "reason": "Added ApiCollectionNotFound: the endpoint now refuses, as not found, to delete an API collection owned by another user", + "lift_digest": "d70bf075f3595bea6dcb3581b21a21b49717cc52e5b327a7978e126132667fb4", + "http4s_digest": "fc6bd7923a89033d884757bbb99a57bedd121250f68876e3ffe24e32b6eda4ce" + }, { "version": "v5_1_0", "endpoint": "revokeMyConsent", From 3ad6ec82602653764ddebb5939322274f2bfe379 Mon Sep 17 00:00:00 2001 From: simonredfern Date: Mon, 19 Oct 2026 01:17:24 +0100 Subject: [PATCH 14/30] Fix: Delete My Api Collection Endpoint By Id only removes endpoints from the caller's own API collection --- .../scala/code/api/v4_0_0/Http4s400.scala | 11 ++++++-- .../v4_0_0/ApiCollectionEndpointTest.scala | 25 +++++++++++++++++++ .../parity_allowlist.json | 8 ++++++ 3 files changed, 42 insertions(+), 2 deletions(-) diff --git a/obp-api/src/main/scala/code/api/v4_0_0/Http4s400.scala b/obp-api/src/main/scala/code/api/v4_0_0/Http4s400.scala index 0c95845544..2694811b5c 100644 --- a/obp-api/src/main/scala/code/api/v4_0_0/Http4s400.scala +++ b/obp-api/src/main/scala/code/api/v4_0_0/Http4s400.scala @@ -7095,9 +7095,15 @@ object Http4s400 { lazy val deleteMyApiCollectionEndpointByOperationId: Http4sRoute = Http4sRoute { case req @ DELETE -> `prefixPath` / "my" / "api-collection-ids" / apiCollectionId / "api-collection-endpoints" / operationId => - EndpointHelpers.withUserDelete(req) { (_, cc) => + // bind the user so the collection's owner can be checked below + EndpointHelpers.withUserDelete(req) { (user, cc) => for { (ac, _) <- NewStyle.function.getApiCollectionById(apiCollectionId, Some(cc)) + // only the owner removes its endpoints, else any user edits any user's collection; answered as not found so nothing is told about other users' collections + _ <- code.util.Helper.booleanToFuture( + s"$ApiCollectionNotFound Please specify a valid value for API_COLLECTION_ID. Current API_COLLECTION_ID($apiCollectionId) ", 404, Some(cc)) { + ac.userId == user.userId + } (ace, _) <- NewStyle.function.getApiCollectionEndpointByApiCollectionIdAndOperationId( ac.apiCollectionId, operationId, Some(cc)) (deleted, _) <- NewStyle.function.deleteApiCollectionEndpointById( @@ -7403,7 +7409,8 @@ object Http4s400 { |""", EmptyBody, Full(true), - List($AuthenticatedUserIsRequired, UserNotFoundByUserId, UnknownError), + // ApiCollectionNotFound: the collection does not exist or is another user's + List($AuthenticatedUserIsRequired, UserNotFoundByUserId, ApiCollectionNotFound, UnknownError), List(apiTagApiCollection), None, http4sPartialFunction = Some(deleteMyApiCollectionEndpointByOperationId) diff --git a/obp-api/src/test/scala/code/api/v4_0_0/ApiCollectionEndpointTest.scala b/obp-api/src/test/scala/code/api/v4_0_0/ApiCollectionEndpointTest.scala index 943cc32c74..9c4fc450ca 100644 --- a/obp-api/src/test/scala/code/api/v4_0_0/ApiCollectionEndpointTest.scala +++ b/obp-api/src/test/scala/code/api/v4_0_0/ApiCollectionEndpointTest.scala @@ -54,6 +54,7 @@ class ApiCollectionEndpointTest extends V400ServerSetup { object ApiEndpoint5 extends Tag(nameOf(Implementations4_0_0.getApiCollectionEndpoints)) object ApiEndpoint6 extends Tag(nameOf(Implementations4_0_0.createMyApiCollectionEndpointById)) object ApiEndpoint7 extends Tag(nameOf(Implementations4_0_0.getMyApiCollectionEndpointsById)) + object ApiEndpoint8 extends Tag(nameOf(Implementations4_0_0.deleteMyApiCollectionEndpointByOperationId)) feature("Test the apiCollection endpoints") { scenario("We create the apiCollection Endpoint", ApiEndpoint1,ApiEndpoint2, ApiEndpoint3, ApiEndpoint4, VersionOfApi) { @@ -310,6 +311,30 @@ class ApiCollectionEndpointTest extends V400ServerSetup { responseApiCollectionEndpoint.code should equal(404) responseApiCollectionEndpoint.body.extract[ErrorMessage].message should startWith(ApiCollectionNotFound) } + + // another user must not remove endpoints from user1's collection by its id and the operation id + scenario(s"We try $ApiEndpoint8 on another user's collection", ApiEndpoint8, VersionOfApi) { + When("user1 creates an apiCollection with one endpoint") + val request = (v4_0_0_Request / "my" / "api-collections").POST <@ (user1) + val response = makePostRequest(request, write(SwaggerDefinitionsJSON.postApiCollectionJson400.copy(api_collection_name = "collection-of-user1-to-delete-from"))) + response.code should equal(201) + val apiCollectionId = response.body.extract[ApiCollectionJson400].api_collection_id + val requestAdd = (v4_0_0_Request / "my" / "api-collection-ids" / apiCollectionId / "api-collection-endpoints").POST <@ (user1) + val responseAdd = makePostRequest(requestAdd, write(SwaggerDefinitionsJSON.postApiCollectionEndpointJson400)) + responseAdd.code should equal(201) + val operationId = responseAdd.body.extract[ApiCollectionEndpointJson400].operation_id + + Then("user2 deletes that endpoint from it") + val requestDelete = (v4_0_0_Request / "my" / "api-collection-ids" / apiCollectionId / "api-collection-endpoints" / operationId).DELETE <@ (user2) + val responseDelete = makeDeleteRequest(requestDelete) + Then("We should get a 404") + responseDelete.code should equal(404) + responseDelete.body.extract[ErrorMessage].message should startWith(ApiCollectionNotFound) + + And("the endpoint is still in user1's collection") + val requestGet = (v4_0_0_Request / "my" / "api-collection-ids" / apiCollectionId / "api-collection-endpoints").GET <@ (user1) + makeGetRequest(requestGet).body.extract[ApiCollectionEndpointsJson400].api_collection_endpoints.length should be (1) + } } } diff --git a/scripts/resource_doc_baseline/parity_allowlist.json b/scripts/resource_doc_baseline/parity_allowlist.json index e9e1d3f7e0..2ceaf5e19d 100644 --- a/scripts/resource_doc_baseline/parity_allowlist.json +++ b/scripts/resource_doc_baseline/parity_allowlist.json @@ -147,6 +147,14 @@ "lift_digest": "d70bf075f3595bea6dcb3581b21a21b49717cc52e5b327a7978e126132667fb4", "http4s_digest": "fc6bd7923a89033d884757bbb99a57bedd121250f68876e3ffe24e32b6eda4ce" }, + { + "version": "v4_0_0", + "endpoint": "deleteMyApiCollectionEndpointByOperationId", + "field": "errorResponseBodies", + "reason": "Added ApiCollectionNotFound: the endpoint now refuses with 404 a collection that belongs to another user", + "lift_digest": "d70bf075f3595bea6dcb3581b21a21b49717cc52e5b327a7978e126132667fb4", + "http4s_digest": "fc6bd7923a89033d884757bbb99a57bedd121250f68876e3ffe24e32b6eda4ce" + }, { "version": "v5_1_0", "endpoint": "revokeMyConsent", From ea3f860f71b49fbf0d6b5df5f75bca652ce5c9e2 Mon Sep 17 00:00:00 2001 From: simonredfern Date: Mon, 19 Oct 2026 01:22:59 +0100 Subject: [PATCH 15/30] Fix: The six ATM field updates require canUpdateAtm or canUpdateAtmAtAnyBank, so only bank staff change ATM data --- .../scala/code/api/v4_0_0/Http4s400.scala | 36 +++++++++----- .../test/scala/code/api/v4_0_0/AtmsTest.scala | 33 ++++++++++++- .../parity_allowlist.json | 48 +++++++++++++++++++ 3 files changed, 104 insertions(+), 13 deletions(-) diff --git a/obp-api/src/main/scala/code/api/v4_0_0/Http4s400.scala b/obp-api/src/main/scala/code/api/v4_0_0/Http4s400.scala index 2694811b5c..f6a1d7043a 100644 --- a/obp-api/src/main/scala/code/api/v4_0_0/Http4s400.scala +++ b/obp-api/src/main/scala/code/api/v4_0_0/Http4s400.scala @@ -6278,9 +6278,11 @@ object Http4s400 { |""", supportedCurrenciesJson, atmSupportedCurrenciesJson, - List($AuthenticatedUserIsRequired, InvalidJsonFormat, UnknownError), + // a caller without the role is refused with 403 UserHasMissingRoles + List($AuthenticatedUserIsRequired, UserHasMissingRoles, InvalidJsonFormat, UnknownError), List(apiTagATM), - None, + // changing a bank's ATM data is bank management: require the role, as updateAtm does, else any logged-in user edits any bank's ATM + Some(List(canUpdateAtm, canUpdateAtmAtAnyBank)), http4sPartialFunction = Some(updateAtmSupportedCurrencies) ) @@ -6294,9 +6296,11 @@ object Http4s400 { |""", supportedLanguagesJson, atmSupportedLanguagesJson, - List($AuthenticatedUserIsRequired, InvalidJsonFormat, UnknownError), + // a caller without the role is refused with 403 UserHasMissingRoles + List($AuthenticatedUserIsRequired, UserHasMissingRoles, InvalidJsonFormat, UnknownError), List(apiTagATM), - None, + // changing a bank's ATM data is bank management: require the role, as updateAtm does, else any logged-in user edits any bank's ATM + Some(List(canUpdateAtm, canUpdateAtmAtAnyBank)), http4sPartialFunction = Some(updateAtmSupportedLanguages) ) @@ -6310,9 +6314,11 @@ object Http4s400 { |""", accessibilityFeaturesJson, atmAccessibilityFeaturesJson, - List($AuthenticatedUserIsRequired, InvalidJsonFormat, UnknownError), + // a caller without the role is refused with 403 UserHasMissingRoles + List($AuthenticatedUserIsRequired, UserHasMissingRoles, InvalidJsonFormat, UnknownError), List(apiTagATM), - None, + // changing a bank's ATM data is bank management: require the role, as updateAtm does, else any logged-in user edits any bank's ATM + Some(List(canUpdateAtm, canUpdateAtmAtAnyBank)), http4sPartialFunction = Some(updateAtmAccessibilityFeatures) ) @@ -6326,9 +6332,11 @@ object Http4s400 { |""", atmServicesJson, atmServicesResponseJson, - List($AuthenticatedUserIsRequired, InvalidJsonFormat, UnknownError), + // a caller without the role is refused with 403 UserHasMissingRoles + List($AuthenticatedUserIsRequired, UserHasMissingRoles, InvalidJsonFormat, UnknownError), List(apiTagATM), - None, + // changing a bank's ATM data is bank management: require the role, as updateAtm does, else any logged-in user edits any bank's ATM + Some(List(canUpdateAtm, canUpdateAtmAtAnyBank)), http4sPartialFunction = Some(updateAtmServices) ) @@ -6342,9 +6350,11 @@ object Http4s400 { |""", atmNotesJson, atmNotesResponseJson, - List($AuthenticatedUserIsRequired, InvalidJsonFormat, UnknownError), + // a caller without the role is refused with 403 UserHasMissingRoles + List($AuthenticatedUserIsRequired, UserHasMissingRoles, InvalidJsonFormat, UnknownError), List(apiTagATM), - None, + // changing a bank's ATM data is bank management: require the role, as updateAtm does, else any logged-in user edits any bank's ATM + Some(List(canUpdateAtm, canUpdateAtmAtAnyBank)), http4sPartialFunction = Some(updateAtmNotes) ) @@ -6358,9 +6368,11 @@ object Http4s400 { |""", atmLocationCategoriesJsonV400, atmLocationCategoriesResponseJsonV400, - List($AuthenticatedUserIsRequired, InvalidJsonFormat, UnknownError), + // a caller without the role is refused with 403 UserHasMissingRoles + List($AuthenticatedUserIsRequired, UserHasMissingRoles, InvalidJsonFormat, UnknownError), List(apiTagATM), - None, + // changing a bank's ATM data is bank management: require the role, as updateAtm does, else any logged-in user edits any bank's ATM + Some(List(canUpdateAtm, canUpdateAtmAtAnyBank)), http4sPartialFunction = Some(updateAtmLocationCategories) ) diff --git a/obp-api/src/test/scala/code/api/v4_0_0/AtmsTest.scala b/obp-api/src/test/scala/code/api/v4_0_0/AtmsTest.scala index 942839eaf0..a2e0e073be 100644 --- a/obp-api/src/test/scala/code/api/v4_0_0/AtmsTest.scala +++ b/obp-api/src/test/scala/code/api/v4_0_0/AtmsTest.scala @@ -31,7 +31,7 @@ import org.json4s._ import code.api.ResourceDocs1_4_0.SwaggerDefinitionsJSON import code.api.util.APIUtil.OAuth._ import code.api.util.ApiRole -import code.api.util.ApiRole.{canCreateAtm, canUpdateAtm} +import code.api.util.ApiRole.{canCreateAtm, canUpdateAtm, canUpdateAtmAtAnyBank} import code.api.util.ErrorMessages.{$AuthenticatedUserIsRequired, UserHasMissingRoles} import code.api.v4_0_0.Http4s400.Implementations4_0_0 import code.entitlement.Entitlement @@ -95,6 +95,25 @@ class AtmsTest extends V400ServerSetup { responseUpdateAtmNoRole.code should be (403) responseUpdateAtmNoRole.body.extract[ErrorMessage].message should equal (UserHasMissingRoles + canUpdateAtm) } + + // the six ATM field updates require canUpdateAtm or canUpdateAtmAtAnyBank, else any logged-in user changes any bank's ATM + scenario("Put ATM fields - missing roles", ApiEndpoint1, ApiEndpoint3, ApiEndpoint4, ApiEndpoint5, ApiEndpoint6, ApiEndpoint7, VersionOfApi) { + val bodies = List( + "supported-currencies" -> write(SwaggerDefinitionsJSON.supportedCurrenciesJson), + "supported-languages" -> write(SwaggerDefinitionsJSON.supportedLanguagesJson), + "accessibility-features" -> write(SwaggerDefinitionsJSON.accessibilityFeaturesJson), + "services" -> write(SwaggerDefinitionsJSON.atmServicesJson), + "notes" -> write(SwaggerDefinitionsJSON.atmNotesJson), + "location-categories" -> write(SwaggerDefinitionsJSON.atmLocationCategoriesJsonV400) + ) + bodies.foreach { case (field, body) => + When(s" Put $field - user1 has no ATM role") + val request = (v4_0_0_Request / "banks" / bankId.value / "atms" / "xxx" / field).PUT <@ (user1) + val response = makePutRequest(request, body) + response.code should be (403) + response.body.extract[ErrorMessage].message should equal (UserHasMissingRoles + List(canUpdateAtm, canUpdateAtmAtAnyBank).mkString(" or ")) + } + } } feature("Test Create/Update/Get -- successful cases") { @@ -157,6 +176,8 @@ class AtmsTest extends V400ServerSetup { responseCreateAtm.code should be (201) val atmId = responseCreateAtm.body.extract[AtmJsonV400].id.getOrElse("") + // updating an ATM field now requires the update role + Entitlement.entitlement.vend.addEntitlement(bankId.value, resourceUser1.userId, ApiRole.CanUpdateAtm.toString) val update = (v4_0_0_Request / "banks" /bankId.value / "atms" / atmId / "supported-currencies").PUT <@ (user1) val responseUpdate = makePutRequest(update, write(postSupportedCurrenciesJson)) @@ -178,6 +199,8 @@ class AtmsTest extends V400ServerSetup { responseCreateAtm.code should be (201) val atmId = responseCreateAtm.body.extract[AtmJsonV400].id.getOrElse("") + // updating an ATM field now requires the update role + Entitlement.entitlement.vend.addEntitlement(bankId.value, resourceUser1.userId, ApiRole.CanUpdateAtm.toString) val update = (v4_0_0_Request / "banks" /bankId.value / "atms" / atmId / "accessibility-features").PUT <@ (user1) val responseUpdate = makePutRequest(update, write(postAccessibilityFeaturesJson)) @@ -199,6 +222,8 @@ class AtmsTest extends V400ServerSetup { responseCreateAtm.code should be (201) val atmId = responseCreateAtm.body.extract[AtmJsonV400].id.getOrElse("") + // updating an ATM field now requires the update role + Entitlement.entitlement.vend.addEntitlement(bankId.value, resourceUser1.userId, ApiRole.CanUpdateAtm.toString) val update = (v4_0_0_Request / "banks" /bankId.value / "atms" / atmId / "supported-languages").PUT <@ (user1) val responseUpdate = makePutRequest(update, write(postSupportedLanguagesJson)) @@ -220,6 +245,8 @@ class AtmsTest extends V400ServerSetup { responseCreateAtm.code should be (201) val atmId = responseCreateAtm.body.extract[AtmJsonV400].id.getOrElse("") + // updating an ATM field now requires the update role + Entitlement.entitlement.vend.addEntitlement(bankId.value, resourceUser1.userId, ApiRole.CanUpdateAtm.toString) val update = (v4_0_0_Request / "banks" /bankId.value / "atms" / atmId / "services").PUT <@ (user1) val responseUpdate = makePutRequest(update, write(postAtmServicesJson)) @@ -241,6 +268,8 @@ class AtmsTest extends V400ServerSetup { responseCreateAtm.code should be (201) val atmId = responseCreateAtm.body.extract[AtmJsonV400].id.getOrElse("") + // updating an ATM field now requires the update role + Entitlement.entitlement.vend.addEntitlement(bankId.value, resourceUser1.userId, ApiRole.CanUpdateAtm.toString) val update = (v4_0_0_Request / "banks" /bankId.value / "atms" / atmId / "notes").PUT <@ (user1) val responseUpdate = makePutRequest(update, write(postAtmNotesJson)) @@ -262,6 +291,8 @@ class AtmsTest extends V400ServerSetup { responseCreateAtm.code should be (201) val atmId = responseCreateAtm.body.extract[AtmJsonV400].id.getOrElse("") + // updating an ATM field now requires the update role + Entitlement.entitlement.vend.addEntitlement(bankId.value, resourceUser1.userId, ApiRole.CanUpdateAtm.toString) val update = (v4_0_0_Request / "banks" /bankId.value / "atms" / atmId / "location-categories").PUT <@ (user1) val responseUpdate = makePutRequest(update, write(postAtmLocationCategoriesJson)) diff --git a/scripts/resource_doc_baseline/parity_allowlist.json b/scripts/resource_doc_baseline/parity_allowlist.json index 2ceaf5e19d..444933a675 100644 --- a/scripts/resource_doc_baseline/parity_allowlist.json +++ b/scripts/resource_doc_baseline/parity_allowlist.json @@ -155,6 +155,54 @@ "lift_digest": "d70bf075f3595bea6dcb3581b21a21b49717cc52e5b327a7978e126132667fb4", "http4s_digest": "fc6bd7923a89033d884757bbb99a57bedd121250f68876e3ffe24e32b6eda4ce" }, + { + "version": "v4_0_0", + "endpoint": "updateAtmAccessibilityFeatures", + "field": "errorResponseBodies", + "reason": "Added UserHasMissingRoles: the endpoint now requires canUpdateAtm or canUpdateAtmAtAnyBank, so any logged-in user can no longer change any bank's ATM", + "lift_digest": "8b9e9294a47baff632975336cec3da4c29f3ca3b8398ec5875e9b5d1712f0140", + "http4s_digest": "160f3a99ccf321a6af0129ef59ea81b833b7ff8f636a6557c8fa22a69ef726fd" + }, + { + "version": "v4_0_0", + "endpoint": "updateAtmLocationCategories", + "field": "errorResponseBodies", + "reason": "Added UserHasMissingRoles: the endpoint now requires canUpdateAtm or canUpdateAtmAtAnyBank, so any logged-in user can no longer change any bank's ATM", + "lift_digest": "8b9e9294a47baff632975336cec3da4c29f3ca3b8398ec5875e9b5d1712f0140", + "http4s_digest": "160f3a99ccf321a6af0129ef59ea81b833b7ff8f636a6557c8fa22a69ef726fd" + }, + { + "version": "v4_0_0", + "endpoint": "updateAtmNotes", + "field": "errorResponseBodies", + "reason": "Added UserHasMissingRoles: the endpoint now requires canUpdateAtm or canUpdateAtmAtAnyBank, so any logged-in user can no longer change any bank's ATM", + "lift_digest": "8b9e9294a47baff632975336cec3da4c29f3ca3b8398ec5875e9b5d1712f0140", + "http4s_digest": "160f3a99ccf321a6af0129ef59ea81b833b7ff8f636a6557c8fa22a69ef726fd" + }, + { + "version": "v4_0_0", + "endpoint": "updateAtmServices", + "field": "errorResponseBodies", + "reason": "Added UserHasMissingRoles: the endpoint now requires canUpdateAtm or canUpdateAtmAtAnyBank, so any logged-in user can no longer change any bank's ATM", + "lift_digest": "8b9e9294a47baff632975336cec3da4c29f3ca3b8398ec5875e9b5d1712f0140", + "http4s_digest": "160f3a99ccf321a6af0129ef59ea81b833b7ff8f636a6557c8fa22a69ef726fd" + }, + { + "version": "v4_0_0", + "endpoint": "updateAtmSupportedCurrencies", + "field": "errorResponseBodies", + "reason": "Added UserHasMissingRoles: the endpoint now requires canUpdateAtm or canUpdateAtmAtAnyBank, so any logged-in user can no longer change any bank's ATM", + "lift_digest": "8b9e9294a47baff632975336cec3da4c29f3ca3b8398ec5875e9b5d1712f0140", + "http4s_digest": "160f3a99ccf321a6af0129ef59ea81b833b7ff8f636a6557c8fa22a69ef726fd" + }, + { + "version": "v4_0_0", + "endpoint": "updateAtmSupportedLanguages", + "field": "errorResponseBodies", + "reason": "Added UserHasMissingRoles: the endpoint now requires canUpdateAtm or canUpdateAtmAtAnyBank, so any logged-in user can no longer change any bank's ATM", + "lift_digest": "8b9e9294a47baff632975336cec3da4c29f3ca3b8398ec5875e9b5d1712f0140", + "http4s_digest": "160f3a99ccf321a6af0129ef59ea81b833b7ff8f636a6557c8fa22a69ef726fd" + }, { "version": "v5_1_0", "endpoint": "revokeMyConsent", From c68bff934cae92adb7c11b199b1362fdda04f7e7 Mon Sep 17 00:00:00 2001 From: simonredfern Date: Mon, 19 Oct 2026 01:28:16 +0100 Subject: [PATCH 16/30] Fix: Update My Personal User Attribute only changes one of the caller's own personal attributes --- .../scala/code/api/v4_0_0/Http4s400.scala | 7 ++++-- .../code/api/v4_0_0/UserAttributesTest.scala | 25 ++++++++++++++++++- .../parity_allowlist.json | 8 ++++++ 3 files changed, 37 insertions(+), 3 deletions(-) diff --git a/obp-api/src/main/scala/code/api/v4_0_0/Http4s400.scala b/obp-api/src/main/scala/code/api/v4_0_0/Http4s400.scala index f6a1d7043a..08f7fdfb5f 100644 --- a/obp-api/src/main/scala/code/api/v4_0_0/Http4s400.scala +++ b/obp-api/src/main/scala/code/api/v4_0_0/Http4s400.scala @@ -4873,7 +4873,9 @@ object Http4s400 { EndpointHelpers.withUserAndBody[UserAttributeJsonV400, Any](req) { (user, postedData, cc) => for { (attributes, _) <- NewStyle.function.getPersonalUserAttributes(user.userId, Some(cc)) - _ <- NewStyle.function.tryons(UserAttributeNotFound, 400, Some(cc)) { + // the attribute must be one of the caller's personal ones, else any user takes over any user attribute + // (tryons only caught exceptions and let a false through, so this check never failed) + _ <- code.util.Helper.booleanToFuture(UserAttributeNotFound, 404, Some(cc)) { attributes.exists(_.userAttributeId == userAttributeId) } attrType <- NewStyle.function.tryons( @@ -5151,7 +5153,8 @@ object Http4s400 { |""", userAttributeJsonV400, userAttributeResponseJson, - List($AuthenticatedUserIsRequired, InvalidJsonFormat, UnknownError), + // UserAttributeNotFound (404): the id is not one of the caller's personal attributes + List($AuthenticatedUserIsRequired, InvalidJsonFormat, UserAttributeNotFound, UnknownError), List(apiTagUser), Some(List()), http4sPartialFunction = Some(updateMyPersonalUserAttribute) diff --git a/obp-api/src/test/scala/code/api/v4_0_0/UserAttributesTest.scala b/obp-api/src/test/scala/code/api/v4_0_0/UserAttributesTest.scala index e22bcfa3f8..a11b6a7e58 100644 --- a/obp-api/src/test/scala/code/api/v4_0_0/UserAttributesTest.scala +++ b/obp-api/src/test/scala/code/api/v4_0_0/UserAttributesTest.scala @@ -152,5 +152,28 @@ class UserAttributesTest extends V400ServerSetup { putResponse400.body.extract[UserAttributeResponseJsonV400].name equals ("ROLE_2") } } - + // Another user's attribute id must be refused as not found, not updated and reassigned to the caller + feature(s"test $ApiEndpoint3 version $VersionOfApi - another user's attribute") { + scenario("We will update an attribute of another user", ApiEndpoint1, ApiEndpoint3, VersionOfApi) { + When("user1 creates a personal attribute") + val request400 = (v4_0_0_Request / "my" / "user" / "attributes").POST <@ (user1) + val response400 = makePostRequest(request400, write(postUserAttributeJsonV400)) + response400.code should equal(201) + val userAttributeId = response400.body.extract[UserAttributeResponseJsonV400].user_attribute_id + + When("user2 tries to update it") + val putRequest400 = (v4_0_0_Request / "my" / "user" / "attributes" / userAttributeId).PUT <@ (user2) + val putResponse400 = makePutRequest(putRequest400, write(putUserAttributeJsonV400)) + Then("We should get a 404") + putResponse400.code should equal(404) + putResponse400.body.extract[ErrorMessage].message should equal(UserAttributeNotFound) + + Then("user1 still has the attribute, unchanged") + val getRequest400 = (v4_0_0_Request / "my" / "user" / "attributes").GET <@ (user1) + val getResponse400 = makeGetRequest(getRequest400) + getResponse400.body.extract[UserAttributesResponseJson].user_attributes + .exists(a => a.user_attribute_id == userAttributeId && a.name == batteryLevel) should be (true) + } + } + } diff --git a/scripts/resource_doc_baseline/parity_allowlist.json b/scripts/resource_doc_baseline/parity_allowlist.json index 444933a675..d3fbf1e653 100644 --- a/scripts/resource_doc_baseline/parity_allowlist.json +++ b/scripts/resource_doc_baseline/parity_allowlist.json @@ -203,6 +203,14 @@ "lift_digest": "8b9e9294a47baff632975336cec3da4c29f3ca3b8398ec5875e9b5d1712f0140", "http4s_digest": "160f3a99ccf321a6af0129ef59ea81b833b7ff8f636a6557c8fa22a69ef726fd" }, + { + "version": "v4_0_0", + "endpoint": "updateMyPersonalUserAttribute", + "field": "errorResponseBodies", + "reason": "Added UserAttributeNotFound (404): the attribute id must be one of the caller's personal attributes, else a user could take over another user's attribute", + "lift_digest": "8b9e9294a47baff632975336cec3da4c29f3ca3b8398ec5875e9b5d1712f0140", + "http4s_digest": "e6807a06ff3025c039b0402f9b51e6a08c093f831943f023172632f7289fdb4f" + }, { "version": "v5_1_0", "endpoint": "revokeMyConsent", From 02a683302ab2c8d342dc409fdb7aa652742e3d4f Mon Sep 17 00:00:00 2001 From: simonredfern Date: Mon, 19 Oct 2026 08:33:39 +0100 Subject: [PATCH 17/30] Fix: Make sure that update Consent Status only reads and changes a consent that belongs to the logged-in user (return 404 if not) --- .../scala/code/api/v4_0_0/Http4s400.scala | 12 +++++++-- .../scala/code/api/v4_0_0/ConsentTests.scala | 25 +++++++++++++++++-- .../parity_allowlist.json | 8 ++++++ 3 files changed, 41 insertions(+), 4 deletions(-) diff --git a/obp-api/src/main/scala/code/api/v4_0_0/Http4s400.scala b/obp-api/src/main/scala/code/api/v4_0_0/Http4s400.scala index 08f7fdfb5f..98a092d061 100644 --- a/obp-api/src/main/scala/code/api/v4_0_0/Http4s400.scala +++ b/obp-api/src/main/scala/code/api/v4_0_0/Http4s400.scala @@ -5339,11 +5339,18 @@ object Http4s400 { lazy val updateConsentStatus: Http4sRoute = Http4sRoute { case req @ PUT -> `prefixPath` / "banks" / _ / "consents" / consentId => - EndpointHelpers.withUserAndBankAndBody[PutConsentStatusJsonV400, Any](req) { (_, _, consentJson, cc) => + // take the logged-in user, so the consent can be checked to be theirs + EndpointHelpers.withUserAndBankAndBody[PutConsentStatusJsonV400, Any](req) { (user, _, consentJson, cc) => for { consent <- Future( code.consent.Consents.consentProvider.vend.getConsentByConsentId(consentId) ) map { box => connectorEmptyResponse(box, Some(cc)) } + // the consent must belong to the caller, else any user reads any consent's JWT and changes its status + _ <- code.util.Helper.booleanToFuture( + s"$ConsentNotFound Current CONSENT_ID($consentId) ", + 404, cc = Some(cc)) { + consent.userId == user.userId + } status = code.consent.ConsentStatus.withName(consentJson.status) updated <- APIUtil.getPropsAsBoolValue("consents.sca.enabled", true) match { case true => @@ -5546,8 +5553,9 @@ object Http4s400 { "eyJhbGciOiJIUzI1NiJ9.eyJlbnRpdGxlbWVudHMiOltdLCJjcmVhdGVkQnlVc2VySWQiOiJhYjY1MzlhOS1iMTA1LTQ0ODktYTg4My0wYWQ4ZDZjNjE2NTciLCJzdWIiOiIyMWUxYzhjYy1mOTE4LTRlYWMtYjhlMy01ZTVlZWM2YjNiNGIiLCJhdWQiOiJlanpuazUwNWQxMzJyeW9tbmhieDFxbXRvaHVyYnNiYjBraWphanNrIiwibmJmIjoxNTUzNTU0ODk5LCJpc3MiOiJodHRwczpcL1wvd3d3Lm9wZW5iYW5rcHJvamVjdC5jb20iLCJleHAiOjE1NTM1NTg0OTksImlhdCI6MTU1MzU1NDg5OSwianRpIjoiMDlmODhkNWYtZWNlNi00Mzk4LThlOTktNjYxMWZhMWNkYmQ1Iiwidmlld3MiOlt7ImFjY291bnRfaWQiOiJtYXJrb19wcml2aXRlXzAxIiwiYmFua19pZCI6ImdoLjI5LnVrLngiLCJ2aWV3X2lkIjoib3duZXIifSx7ImFjY291bnRfaWQiOiJtYXJrb19wcml2aXRlXzAyIiwiYmFua19pZCI6ImdoLjI5LnVrLngiLCJ2aWV3X2lkIjoib3duZXIifV19.8cc7cBEf2NyQvJoukBCmDLT7LXYcuzTcSYLqSpbxLp4", status = "AUTHORISED" ), + // ConsentNotFound: the consent is not the caller's List($AuthenticatedUserIsRequired, $BankNotFound, InvalidJsonFormat, - InvalidConnectorResponse, UnknownError), + InvalidConnectorResponse, ConsentNotFound, UnknownError), apiTagConsent :: apiTagPSD2AIS :: Nil, None, http4sPartialFunction = Some(updateConsentStatus) diff --git a/obp-api/src/test/scala/code/api/v4_0_0/ConsentTests.scala b/obp-api/src/test/scala/code/api/v4_0_0/ConsentTests.scala index b44fdccf2c..67c4949c13 100644 --- a/obp-api/src/test/scala/code/api/v4_0_0/ConsentTests.scala +++ b/obp-api/src/test/scala/code/api/v4_0_0/ConsentTests.scala @@ -30,10 +30,13 @@ package code.api.v4_0_0 import code.api.util.APIUtil.OAuth._ import code.api.util.ErrorMessages import code.api.v4_0_0.Http4s400.Implementations4_0_0 +import code.consent.Consents import code.setup.DefaultUsers import com.github.dwickern.macros.NameOf.nameOf import com.openbankproject.commons.model.ErrorMessage import com.openbankproject.commons.util.ApiVersion +import org.json4s.JNothing +import org.json4s.native.Serialization.write import org.scalatest.Tag class ConsentTests extends V400ServerSetup with DefaultUsers { @@ -55,6 +58,7 @@ class ConsentTests extends V400ServerSetup with DefaultUsers { */ object VersionOfApi extends Tag(ApiVersion.v4_0_0.toString) object ApiEndpoint1 extends Tag(nameOf(Implementations4_0_0.getConsents)) + object ApiEndpoint2 extends Tag(nameOf(Implementations4_0_0.updateConsentStatus)) feature("Assuring that endpoint createBank works as expected - v4.0.0") { @@ -76,7 +80,24 @@ class ConsentTests extends V400ServerSetup with DefaultUsers { And("We should get a message: " + ErrorMessages.BankNotFound) responseGet.code should equal(404) responseGet.body.extract[ErrorMessage].message should startWith(ErrorMessages.BankNotFound) - } - + } + + } + + feature(s"Assuring that endpoint $ApiEndpoint2 only serves the caller's own consent - v4.0.0") { + + scenario(s"We try to update the status of another user's consent - $ApiEndpoint2", ApiEndpoint2, VersionOfApi) { + Given("A consent owned by user1") + val consent = Consents.consentProvider.vend.createObpConsent(resourceUser1, "12345678", None).openOrThrowException("consent") + When("user2 updates its status") + val request = (v4_0_0_Request / "banks" / testBankId1.value / "consents" / consent.consentId).PUT <@ (user2) + val response = makePutRequest(request, write(PutConsentStatusJsonV400(status = "REJECTED"))) + Then("We should get a 404, as if the consent did not exist") + response.code should equal(404) + response.body.extract[ErrorMessage].message should startWith(ErrorMessages.ConsentNotFound) + And("The consent's JWT is not returned") + (response.body \ "jwt") should equal(JNothing) + } + } } \ No newline at end of file diff --git a/scripts/resource_doc_baseline/parity_allowlist.json b/scripts/resource_doc_baseline/parity_allowlist.json index d3fbf1e653..50afe052c3 100644 --- a/scripts/resource_doc_baseline/parity_allowlist.json +++ b/scripts/resource_doc_baseline/parity_allowlist.json @@ -203,6 +203,14 @@ "lift_digest": "8b9e9294a47baff632975336cec3da4c29f3ca3b8398ec5875e9b5d1712f0140", "http4s_digest": "160f3a99ccf321a6af0129ef59ea81b833b7ff8f636a6557c8fa22a69ef726fd" }, + { + "version": "v4_0_0", + "endpoint": "updateConsentStatus", + "field": "errorResponseBodies", + "reason": "Added ConsentNotFound (404): the consent must belong to the caller, else any user reads any consent's JWT and changes its status", + "lift_digest": "cf8ea6cd85f19cc3b87dbeeeb4b6f41d2fd835c55e8bcfaec7e3459304a42ca4", + "http4s_digest": "3bb23da885584fa6b1b72ac9914a533340b6a74dd1e57227fecb794c7369f547" + }, { "version": "v4_0_0", "endpoint": "updateMyPersonalUserAttribute", From 51bfd5bd8bfc1d9d26d3363c8af7f790e4a150a7 Mon Sep 17 00:00:00 2001 From: simonredfern Date: Mon, 19 Oct 2026 08:44:05 +0100 Subject: [PATCH 18/30] Fix: Make sure Delete Chat Message only deletes a message of the room in the path, and of the bank in the path --- .../scala/code/api/v6_0_0/Http4s600.scala | 13 +- .../api/v6_0_0/ChatMessageDeleteTest.scala | 112 ++++++++++++++++++ 2 files changed, 121 insertions(+), 4 deletions(-) create mode 100644 obp-api/src/test/scala/code/api/v6_0_0/ChatMessageDeleteTest.scala diff --git a/obp-api/src/main/scala/code/api/v6_0_0/Http4s600.scala b/obp-api/src/main/scala/code/api/v6_0_0/Http4s600.scala index b20c00b000..54f3e82d98 100644 --- a/obp-api/src/main/scala/code/api/v6_0_0/Http4s600.scala +++ b/obp-api/src/main/scala/code/api/v6_0_0/Http4s600.scala @@ -3991,15 +3991,19 @@ object Http4s600 { } lazy val deleteBankChatMessage: Http4sRoute = Http4sRoute { - case req @ DELETE -> `prefixPath` / "banks" / _ / "chat-rooms" / chatRoomId / "messages" / chatMessageId => + // bankIdStr is read so the room can be checked against the bank in the path + case req @ DELETE -> `prefixPath` / "banks" / bankIdStr / "chat-rooms" / chatRoomId / "messages" / chatMessageId => EndpointHelpers.executeDelete(req) { cc => val u = cc.user.openOrThrowException("User not found in CallContext") for { roomBox <- Future(code.chat.ChatRoomTrait.chatRoomProvider.vend.getChatRoom(chatRoomId)) - _ <- Future(unboxFullOrFail(roomBox, Some(cc), ChatRoomNotFound, 404)) + room <- Future(unboxFullOrFail(roomBox, Some(cc), ChatRoomNotFound, 404)) + // the room must belong to the bank in the path, else it is treated as not found + _ <- Helper.booleanToFuture(ChatRoomNotFound, 404, cc = Some(cc)) { room.bankId == bankIdStr } partBox <- Future(code.chat.ChatPermissions.isParticipant(chatRoomId, u.userId)) _ <- Future(unboxFullOrFail(partBox, Some(cc), NotChatRoomParticipant, 403)) - msgBox <- Future(code.chat.ChatMessageTrait.chatMessageProvider.vend.getMessage(chatMessageId)) + // the message must be in the room in the path, else a participant of one room deletes messages of any room + msgBox <- Future(code.chat.ChatMessageTrait.chatMessageProvider.vend.getMessage(chatMessageId).filter(_.chatRoomId == chatRoomId)) msg <- Future(unboxFullOrFail(msgBox, Some(cc), ChatMessageNotFound, 404)) _ <- if (msg.senderUserId == u.userId) Future.successful(()) else Future(code.chat.ChatPermissions.checkParticipantPermission( @@ -4023,7 +4027,8 @@ object Http4s600 { _ <- Future(unboxFullOrFail(roomBox, Some(cc), ChatRoomNotFound, 404)) partBox <- Future(code.chat.ChatPermissions.isParticipant(chatRoomId, u.userId)) _ <- Future(unboxFullOrFail(partBox, Some(cc), NotChatRoomParticipant, 403)) - msgBox <- Future(code.chat.ChatMessageTrait.chatMessageProvider.vend.getMessage(chatMessageId)) + // the message must be in the room in the path, else a participant of one room deletes messages of any room + msgBox <- Future(code.chat.ChatMessageTrait.chatMessageProvider.vend.getMessage(chatMessageId).filter(_.chatRoomId == chatRoomId)) msg <- Future(unboxFullOrFail(msgBox, Some(cc), ChatMessageNotFound, 404)) _ <- if (msg.senderUserId == u.userId) Future.successful(()) else Future(code.chat.ChatPermissions.checkParticipantPermission( diff --git a/obp-api/src/test/scala/code/api/v6_0_0/ChatMessageDeleteTest.scala b/obp-api/src/test/scala/code/api/v6_0_0/ChatMessageDeleteTest.scala new file mode 100644 index 0000000000..78e223d714 --- /dev/null +++ b/obp-api/src/test/scala/code/api/v6_0_0/ChatMessageDeleteTest.scala @@ -0,0 +1,112 @@ +/** +Open Bank Project - API +Copyright (C) 2011-2026, TESOBE GmbH. + +This program is free software: you can redistribute it and/or modify +it under the terms of the GNU Affero General Public License as published by +the Free Software Foundation, either version 3 of the License, or +(at your option) any later version. + +This program is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU Affero General Public License for more details. + +You should have received a copy of the GNU Affero General Public License +along with this program. If not, see . + +Email: contact@tesobe.com +TESOBE GmbH. +Osloer Strasse 16/17 +Berlin 13359, Germany + +This product includes software developed at +TESOBE (http://www.tesobe.com/) + + */ + +package code.api.v6_0_0 + +import code.api.util.APIUtil.OAuth._ +import code.api.util.ErrorMessages.{ChatMessageNotFound, ChatRoomNotFound} +import code.chat.{ChatMessageTrait, ChatPermissions, ChatRoomTrait, ParticipantTrait} +import com.openbankproject.commons.model.ErrorMessage +import com.openbankproject.commons.util.ApiVersion +import org.scalatest.Tag + +/** + * A message can only be deleted through the room (and, for bank rooms, the bank) it belongs to. + */ +class ChatMessageDeleteTest extends V600ServerSetup { + + object VersionOfApi extends Tag(ApiVersion.v6_0_0.toString) + object ApiEndpointDeleteBank extends Tag("deleteBankChatMessage") + object ApiEndpointDeleteSystem extends Tag("deleteSystemChatMessage") + + // a room at bankId in which user1 may delete any message + private def createRoomWithDeleter(bankId: String): String = { + val room = ChatRoomTrait.chatRoomProvider.vend.createChatRoom( + bankId, s"room-${java.util.UUID.randomUUID()}", "test room", resourceUser2.userId).openOrThrowException("room") + ParticipantTrait.participantProvider.vend.addParticipant( + room.chatRoomId, resourceUser1.userId, "", List(ChatPermissions.CAN_DELETE_MESSAGE), "").openOrThrowException("participant") + room.chatRoomId + } + + // a message by user2 in a room user1 is not in + private def createMessageInOtherRoom(bankId: String): String = { + val otherRoom = ChatRoomTrait.chatRoomProvider.vend.createChatRoom( + bankId, s"other-${java.util.UUID.randomUUID()}", "other room", resourceUser2.userId).openOrThrowException("room") + ChatMessageTrait.chatMessageProvider.vend.createMessage( + otherRoom.chatRoomId, resourceUser2.userId, "", "secret", "text", Nil, "", "").openOrThrowException("message").chatMessageId + } + + private def isDeleted(chatMessageId: String): Boolean = + ChatMessageTrait.chatMessageProvider.vend.getMessage(chatMessageId).map(_.isDeleted).openOr(false) + + feature(s"Delete Bank Chat Message - DELETE /obp/v6.0.0/banks/BANK_ID/chat-rooms/CHAT_ROOM_ID/messages/CHAT_MESSAGE_ID - $VersionOfApi") { + + scenario("A message of another room is refused with 404 and not deleted", ApiEndpointDeleteBank, VersionOfApi) { + val roomId = createRoomWithDeleter(testBankId1.value) + val messageId = createMessageInOtherRoom(testBankId1.value) + val request = (v6_0_0_Request / "banks" / testBankId1.value / "chat-rooms" / roomId / "messages" / messageId).DELETE <@ (user1) + val response = makeDeleteRequest(request) + response.code should equal(404) + response.body.extract[ErrorMessage].message should equal(ChatMessageNotFound) + isDeleted(messageId) should equal(false) + } + + scenario("A room of another bank is refused with 404", ApiEndpointDeleteBank, VersionOfApi) { + val roomId = createRoomWithDeleter(testBankId1.value) + val messageId = ChatMessageTrait.chatMessageProvider.vend.createMessage( + roomId, resourceUser2.userId, "", "hello", "text", Nil, "", "").openOrThrowException("message").chatMessageId + val request = (v6_0_0_Request / "banks" / testBankId2.value / "chat-rooms" / roomId / "messages" / messageId).DELETE <@ (user1) + val response = makeDeleteRequest(request) + response.code should equal(404) + response.body.extract[ErrorMessage].message should equal(ChatRoomNotFound) + isDeleted(messageId) should equal(false) + } + + scenario("A message of the room in the path is deleted with 204", ApiEndpointDeleteBank, VersionOfApi) { + val roomId = createRoomWithDeleter(testBankId1.value) + val messageId = ChatMessageTrait.chatMessageProvider.vend.createMessage( + roomId, resourceUser2.userId, "", "hello", "text", Nil, "", "").openOrThrowException("message").chatMessageId + val request = (v6_0_0_Request / "banks" / testBankId1.value / "chat-rooms" / roomId / "messages" / messageId).DELETE <@ (user1) + val response = makeDeleteRequest(request) + response.code should equal(204) + isDeleted(messageId) should equal(true) + } + } + + feature(s"Delete System Chat Message - DELETE /obp/v6.0.0/chat-rooms/CHAT_ROOM_ID/messages/CHAT_MESSAGE_ID - $VersionOfApi") { + + scenario("A message of another room is refused with 404 and not deleted", ApiEndpointDeleteSystem, VersionOfApi) { + val roomId = createRoomWithDeleter("") + val messageId = createMessageInOtherRoom("") + val request = (v6_0_0_Request / "chat-rooms" / roomId / "messages" / messageId).DELETE <@ (user1) + val response = makeDeleteRequest(request) + response.code should equal(404) + response.body.extract[ErrorMessage].message should equal(ChatMessageNotFound) + isDeleted(messageId) should equal(false) + } + } +} From 9a7f2018f5254ba8076a95ebabea5f0a60573fbc Mon Sep 17 00:00:00 2001 From: simonredfern Date: Mon, 19 Oct 2026 08:59:35 +0100 Subject: [PATCH 19/30] Fix: Make sure that revoke access to all Views on Account revokes every view the user has on the account, and revokes nothing when the user is an account holder with owner view access or the only user with owner view access (OBP-20110 OwnerViewAccessMustRemain, 403); it used to delete only one of the user's views and still answer 204. --- .../scala/code/api/util/ErrorMessages.scala | 1 + .../scala/code/api/v1_2_1/Http4s121.scala | 4 +- .../scala/code/api/v4_0_0/Http4s400.scala | 2 + .../main/scala/code/views/MapperViews.scala | 24 +++++++-- .../scala/code/api/v1_2_1/API1_2_1Test.scala | 49 ++++++++++--------- .../parity_allowlist.json | 24 +++++++++ 6 files changed, 76 insertions(+), 28 deletions(-) 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 b8aef81b5a..80eee321d2 100644 --- a/obp-api/src/main/scala/code/api/util/ErrorMessages.scala +++ b/obp-api/src/main/scala/code/api/util/ErrorMessages.scala @@ -363,6 +363,7 @@ object ErrorMessages { val UserAlreadyValidated = "OBP-20107: User email is already validated." val InvalidRedirectUrl = "OBP-20108: Invalid redirect URL. Each redirect URL must use https, use http only for localhost, 127.0.0.1 or [::1], or use an app scheme in reverse-domain form such as com.example.app, and must not contain a wildcard, user information or a fragment. Rejected entry: " val RedirectUrlHostNotAllowed = "OBP-20109: Redirect URL host not allowed. This instance only accepts redirect URLs that point to the hosts listed for it. Rejected entry: " + val OwnerViewAccessMustRemain = "OBP-20110: Owner view access must remain. The user is an account holder of this account, or the only user with access to its owner view, so they keep their owner view access. No access has been revoked." // OAuth 2 val ApplicationNotIdentified = "OBP-20200: The application cannot be identified. " diff --git a/obp-api/src/main/scala/code/api/v1_2_1/Http4s121.scala b/obp-api/src/main/scala/code/api/v1_2_1/Http4s121.scala index c43addcb0f..8509b96453 100644 --- a/obp-api/src/main/scala/code/api/v1_2_1/Http4s121.scala +++ b/obp-api/src/main/scala/code/api/v1_2_1/Http4s121.scala @@ -943,13 +943,15 @@ object Http4s121 { "/banks/BANK_ID/accounts/ACCOUNT_ID/permissions/PROVIDER/PROVIDER_ID/views", "Revoke access to all Views on Account", s""""Revokes access to all Views on an Account for a certain User. + | + |Nothing is revoked if the User is an account holder of the Account and has access to its owner view, or if the User is the only one with access to the owner view: the request is refused with $OwnerViewAccessMustRemain | |$generalRevokeAccessToViewText | |${userAuthenticationMessage(true)} and the user needs to have access to the owner view.""", EmptyBody, EmptyBody, - List(AuthenticatedUserIsRequired, BankAccountNotFound, UnknownError, "user does not have access to owner view on account"), + List(AuthenticatedUserIsRequired, BankAccountNotFound, OwnerViewAccessMustRemain, UnknownError, "user does not have access to owner view on account"), List(apiTagView, apiTagAccount, apiTagUser, apiTagOwnerRequired), http4sPartialFunction = Some(removePermissionForUserForBankAccountForAllViews) ) diff --git a/obp-api/src/main/scala/code/api/v4_0_0/Http4s400.scala b/obp-api/src/main/scala/code/api/v4_0_0/Http4s400.scala index 98a092d061..3bc1078bdd 100644 --- a/obp-api/src/main/scala/code/api/v4_0_0/Http4s400.scala +++ b/obp-api/src/main/scala/code/api/v4_0_0/Http4s400.scala @@ -5540,6 +5540,8 @@ object Http4s400 { | |This endpoint is used to update the Status of Consent. | + |The consent must belong to the logged-in user; another user's consent returns 404 $ConsentNotFound + | |Each Consent has one of the following states: ${ConsentStatus.values.toList.sorted .mkString(", ")}. | diff --git a/obp-api/src/main/scala/code/views/MapperViews.scala b/obp-api/src/main/scala/code/views/MapperViews.scala index 8d3f79dd72..106e27b317 100644 --- a/obp-api/src/main/scala/code/views/MapperViews.scala +++ b/obp-api/src/main/scala/code/views/MapperViews.scala @@ -425,16 +425,30 @@ object MapperViews extends Views with MdcLoggable { /** - * remove all the accountAccess for one user and linked account. - * we already has the guard `canRevokeAccessToAllViews` on the top level. + * This removes every account access the user has to one account. + * + * The caller's right to revoke is checked at the top level (`canRevokeAccessToAllViews`); that check + * says nothing about the user losing the access. Here the same rule as revoking a single view applies + * (canRevokeOwnerAccess): an account holder keeps the owner view, and the account must keep at least one + * user with owner view access. If any of the user's accesses is protected by that rule, nothing is revoked, + * so the call never leaves the user with only part of their access removed. */ def revokeAllAccountAccess(bankId : BankId, accountId: AccountId, user : User) : Box[Boolean] = { - AccountAccess.find( + val accountAccesses = AccountAccess.findAll( By(AccountAccess.bank_id, bankId.value), By(AccountAccess.account_id, accountId.value), By(AccountAccess.user_fk, user.userPrimaryKey.value) - ).foreach(_.delete_!) - Full(true) + ) + val everyAccessCanBeRevoked = accountAccesses.forall(accountAccess => + ViewDefinition.findSystemView(accountAccess.view_id.get) + .forall(systemView => canRevokeOwnerAccess(bankId, accountId, systemView, user)) + ) + if (everyAccessCanBeRevoked) { + accountAccesses.foreach(_.delete_!) + Full(true) + } else { + Failure(OwnerViewAccessMustRemain) + } } def revokeAccountAccessByUser(bankId : BankId, accountId: AccountId, user : User, callContext: Option[CallContext]) : Box[Boolean] = { diff --git a/obp-api/src/test/scala/code/api/v1_2_1/API1_2_1Test.scala b/obp-api/src/test/scala/code/api/v1_2_1/API1_2_1Test.scala index 12e51ef796..3a1ee12a27 100644 --- a/obp-api/src/test/scala/code/api/v1_2_1/API1_2_1Test.scala +++ b/obp-api/src/test/scala/code/api/v1_2_1/API1_2_1Test.scala @@ -35,7 +35,9 @@ import code.api.util.APIUtil.isValidSystemViewId import code.api.util.ErrorMessages._ import code.bankconnectors.Connector import code.setup.{APIResponse, DefaultUsers, PrivateUser2AccountsAndSetUpWithTestData, ServerSetupWithTestData} +import code.accountholders.AccountHolders import code.views.Views +import code.views.system.AccountAccess // ErrorMessage is excluded: this file is in package code.api.v1_2_1, which defines its // own ErrorMessage, and that one shadows the wildcard-imported commons class. Both are // case class ErrorMessage(code: Int, message: String), so resolution never changed the @@ -2136,13 +2138,13 @@ class API1_2_1Test extends ServerSetupWithTestData with DefaultUsers with Privat } feature("Revoke a user access to all the views on a bank account"){ scenario("we will revoke the access of a user to all the views on an bank account", API1_2_1, DeletePermissions){ - Given("We will use an access token") + Given("A user with access to several views on the account, the owner view among them") val bankId = randomBank val bankAccount : AccountJSON = randomPrivateAccount(bankId) val userId = resourceUser2.idGivenByProvider - val viewId = randomCustomViewPermalink(bankId, bankAccount) - val viewsIdsToGrant = viewId :: Nil - grantUserAccessToViews(bankId, bankAccount.id, userId, viewsIdsToGrant, user1) + val viewsIdsToGrant = randomCustomViewPermalink(bankId, bankAccount) :: SYSTEM_AUDITOR_VIEW_ID :: SYSTEM_OWNER_VIEW_ID :: Nil + grantUserAccessToViews(bankId, bankAccount.id, userId, viewsIdsToGrant, user1).code should equal (201) + getUserAccountPermission(bankId, bankAccount.id, userId, user1).body.extract[ViewsJSONV121].views.length should be >= 3 When("the request is sent") val reply = revokeUserAccessToAllViews(bankId, bankAccount.id, userId, user1) Then("we should get a 204 no content code") @@ -2180,28 +2182,27 @@ class API1_2_1Test extends ServerSetupWithTestData with DefaultUsers with Privat scenario("we cannot revoke the access to the owner view via a revoke all views call if there " + "would then be no one with access to it", API1_2_1, DeletePermissions){ - Given("We will use an access token") + Given("The only user with access to the owner view of an account") val bankId = randomBank val bankAccount : AccountJSON = randomPrivateAccount(bankId) - val viewId = ViewId(SYSTEM_OWNER_VIEW_ID) - val view = Views.views.vend.systemView(viewId).openOrThrowException(attemptedToOpenAnEmptyBox) val userId = resourceUser1.idGivenByProvider - - Views.views.vend.getOwners(view).toList.length should equal(1) - Views.views.vend.getOwners(view).toList(0).idGivenByProvider should equal(userId) + val ownerViewUsersBefore = AccountAccess.findAllByBankIdAccountIdViewId(BankId(bankId), AccountId(bankAccount.id), ViewId(SYSTEM_OWNER_VIEW_ID)).flatMap(_.user_fk.obj.toList).map(_.userId).distinct + ownerViewUsersBefore should equal (List(resourceUser1.userId)) + val viewsBefore = getUserAccountPermission(bankId, bankAccount.id, userId, user1).body.extract[ViewsJSONV121].views.length When("the request is sent") val reply = revokeUserAccessToAllViews(bankId, bankAccount.id, userId, user1) - Then("we should get a 204 code") - reply.code should equal (204) + Then("we should get a 403 code") + reply.code should equal (403) + reply.body.extract[ErrorMessage].message should include (OwnerViewAccessMustRemain) - And("The user should not have had his access revoked") - Views.views.vend.getOwners(view).toList.length should equal(1) - Views.views.vend.getOwners(view).toList(0).idGivenByProvider should equal(userId) + And("None of the user's access to the account should have been revoked") + AccountAccess.findByBankIdAccountIdViewIdUserPrimaryKey(BankId(bankId), AccountId(bankAccount.id), ViewId(SYSTEM_OWNER_VIEW_ID), resourceUser1.userPrimaryKey).isDefined should equal (true) + getUserAccountPermission(bankId, bankAccount.id, userId, user1).body.extract[ViewsJSONV121].views.length should equal (viewsBefore) } - scenario("we can revoke the access of a user to owner view on a bank account via a revoke all views call" + - " if that user is an account holder of that account", API1_2_1, DeletePermissions){ + scenario("we cannot revoke the access of an account holder to the owner view on a bank account via a revoke all views call", + API1_2_1, DeletePermissions){ Given("A user is the account holder of an account (and has access to the owner view)") val bankId = randomBank val bankAccount : AccountJSON = randomPrivateAccount(bankId) @@ -2214,12 +2215,16 @@ class API1_2_1Test extends ServerSetupWithTestData with DefaultUsers with Privat When("We try to revoke this user's access to all views") val reply = revokeUserAccessToAllViews(bankId, bankAccount.id, resourceUser3.idGivenByProvider, user1) - Then("we should get a 204 code") - reply.code should equal (204) + Then("we should get a 403 code") + reply.code should equal (403) + reply.body.extract[ErrorMessage].message should include (OwnerViewAccessMustRemain) - And("The user should not have had his access revoked") - val view = Views.views.vend.systemView(ViewId(SYSTEM_OWNER_VIEW_ID)).openOrThrowException(attemptedToOpenAnEmptyBox) - Views.views.vend.getOwners(view).toList should not contain (resourceUser3) + And("The user should still have access to the owner view of this account") + AccountAccess.findByBankIdAccountIdViewIdUserPrimaryKey(BankId(bankId), AccountId(bankAccount.id), ViewId(ownerViewId), resourceUser3.userPrimaryKey).isDefined should equal (true) + + //clean up: the later tests expect user3 to have no access to the test accounts + AccountHolders.accountHolders.vend.deleteAccountHolder(resourceUser3, BankIdAccountId(BankId(bankId), AccountId(bankAccount.id))) + revokeUserAccessToAllViews(bankId, bankAccount.id, resourceUser3.idGivenByProvider, user1).code should equal (204) } } diff --git a/scripts/resource_doc_baseline/parity_allowlist.json b/scripts/resource_doc_baseline/parity_allowlist.json index 50afe052c3..8b7c58b089 100644 --- a/scripts/resource_doc_baseline/parity_allowlist.json +++ b/scripts/resource_doc_baseline/parity_allowlist.json @@ -203,6 +203,22 @@ "lift_digest": "8b9e9294a47baff632975336cec3da4c29f3ca3b8398ec5875e9b5d1712f0140", "http4s_digest": "160f3a99ccf321a6af0129ef59ea81b833b7ff8f636a6557c8fa22a69ef726fd" }, + { + "version": "v1_2_1", + "endpoint": "removePermissionForUserForBankAccountForAllViews", + "field": "description", + "reason": "Added that nothing is revoked when the user is an account holder with owner view access, or the only user with owner view access (OwnerViewAccessMustRemain)", + "lift_digest": "f31da0343c89bc37150a06bc3d8a39ed4e8732a18df8b21e36fdda2bd79517c3", + "http4s_digest": "42437a8a9503f3b2b7a0baaa7103c9e17b1a0380f89c60128c8c408df2067946" + }, + { + "version": "v1_2_1", + "endpoint": "removePermissionForUserForBankAccountForAllViews", + "field": "errorResponseBodies", + "reason": "Added OwnerViewAccessMustRemain (403): revoking all views no longer removes an account holder's or the last owner's owner view access", + "lift_digest": "b72cefb915c6af0190e4543aed820722db264a43a54247247d9a1fdb25321703", + "http4s_digest": "c7c4a40bf4a76ab083977ab386b6699136746f155c5749910f0b0edd2d77b35c" + }, { "version": "v4_0_0", "endpoint": "updateConsentStatus", @@ -211,6 +227,14 @@ "lift_digest": "cf8ea6cd85f19cc3b87dbeeeb4b6f41d2fd835c55e8bcfaec7e3459304a42ca4", "http4s_digest": "3bb23da885584fa6b1b72ac9914a533340b6a74dd1e57227fecb794c7369f547" }, + { + "version": "v4_0_0", + "endpoint": "updateConsentStatus", + "field": "description", + "reason": "Added that the consent must belong to the logged-in user, otherwise 404 ConsentNotFound", + "lift_digest": "72dc13245db8680613c4bfc56859b1b7ca4c10d35fb53f65b241c184ebf13827", + "http4s_digest": "2ab09de7128ff70488baf26b2fda2ab9f52df3565bf17644fedbc31b36e823ab" + }, { "version": "v4_0_0", "endpoint": "updateMyPersonalUserAttribute", From d70ae5013c766ae98882fd94190158b776f29578 Mon Sep 17 00:00:00 2001 From: simonredfern Date: Mon, 19 Oct 2026 09:47:41 +0100 Subject: [PATCH 20/30] Fix: Make sure create and Update Card Attribute need a card Role at the bank in the path --- .../scala/code/api/v3_1_0/Http4s310.scala | 12 +++++--- .../code/api/v3_1_0/CardAttributeTest.scala | 28 ++++++++++++++++++- .../parity_allowlist.json | 16 +++++++++++ 3 files changed, 51 insertions(+), 5 deletions(-) diff --git a/obp-api/src/main/scala/code/api/v3_1_0/Http4s310.scala b/obp-api/src/main/scala/code/api/v3_1_0/Http4s310.scala index e2619766a6..63fd267be0 100644 --- a/obp-api/src/main/scala/code/api/v3_1_0/Http4s310.scala +++ b/obp-api/src/main/scala/code/api/v3_1_0/Http4s310.scala @@ -3757,9 +3757,11 @@ object Http4s310 { cardAttributeNameExample.value, CardAttributeType.DOUBLE, cardAttributeValueExample.value), - List(AuthenticatedUserIsRequired, InvalidJsonFormat, UnknownError), + // UserHasMissingRoles: the middleware answers 403 without the role below + List(AuthenticatedUserIsRequired, UserHasMissingRoles, InvalidJsonFormat, UnknownError), List(apiTagCard, apiTagCardAttribute, apiTagAttribute), - None, + // the role at BANK_ID is required, else any logged-in user writes attributes on any bank's cards + Some(List(canCreateCardsForBank)), http4sPartialFunction = Some(createCardAttribute) ) @@ -3810,9 +3812,11 @@ object Http4s310 { cardAttributeNameExample.value, CardAttributeType.DOUBLE, cardAttributeValueExample.value), - List(AuthenticatedUserIsRequired, InvalidJsonFormat, UnknownError), + // UserHasMissingRoles: the middleware answers 403 without the role below + List(AuthenticatedUserIsRequired, UserHasMissingRoles, InvalidJsonFormat, UnknownError), List(apiTagCard, apiTagCardAttribute, apiTagAttribute), - None, + // the role at BANK_ID is required, else any logged-in user overwrites attributes of any bank's cards + Some(List(canUpdateCardsForBank)), http4sPartialFunction = Some(updateCardAttribute) ) diff --git a/obp-api/src/test/scala/code/api/v3_1_0/CardAttributeTest.scala b/obp-api/src/test/scala/code/api/v3_1_0/CardAttributeTest.scala index 6cc0bc36ff..a43c657035 100644 --- a/obp-api/src/test/scala/code/api/v3_1_0/CardAttributeTest.scala +++ b/obp-api/src/test/scala/code/api/v3_1_0/CardAttributeTest.scala @@ -34,6 +34,9 @@ import code.api.ResourceDocs1_4_0.SwaggerDefinitionsJSON.createPhysicalCardJsonV import code.api.util.APIUtil.OAuth._ import code.api.util.ApiRole.CanCreateCustomer import code.api.util.ApiRole +// for the 403 scenarios: the error body and its message +import code.api.util.ErrorMessages.UserHasMissingRoles +import com.openbankproject.commons.model.ErrorMessage import code.api.v3_1_0.Http4s310.Implementations3_1_0 import code.entitlement.Entitlement import code.setup.DefaultUsers @@ -127,7 +130,30 @@ class CardAttributeTest extends V310ServerSetup with DefaultUsers { responseForCreateCardAttribute.body.toString contains (properCreateCardAttributeJson.name) should be (true) responseForCreateCardAttribute.body.toString contains (properCreateCardAttributeJson.value) should be (true) responseForCreateCardAttribute.body.toString contains (properCreateCardAttributeJson.`type`) should be (true) - + + } + } + + // a logged-in user with no card role at the bank must not write or overwrite that bank's card attributes + feature("Card attributes need a card role at the bank") { + val cardAttributeJson = CardAttributeJson("TAX_NUMBER", "STRING", "123") + + scenario("Create Card Attribute without CanCreateCardsForBank", ApiEndpointDeleteCardForBank, VersionOfApi) { + When("user2, who has no role at the bank, posts a card attribute") + val request = (v3_1_0_Request / "management" / "banks" / testBankId1.value / "cards" / "any-card-id" / "attribute").POST <@ (user2) + val response = makePostRequest(request, write(cardAttributeJson)) + Then("We should get a 403 naming the missing role") + response.code should equal(403) + response.body.extract[ErrorMessage].message should equal(UserHasMissingRoles + ApiRole.canCreateCardsForBank) + } + + scenario("Update Card Attribute without CanUpdateCardsForBank", VersionOfApi) { + When("user2, who has no role at the bank, puts a card attribute") + val request = (v3_1_0_Request / "management" / "banks" / testBankId1.value / "cards" / "any-card-id" / "attributes" / "any-attribute-id").PUT <@ (user2) + val response = makePutRequest(request, write(cardAttributeJson)) + Then("We should get a 403 naming the missing role") + response.code should equal(403) + response.body.extract[ErrorMessage].message should equal(UserHasMissingRoles + ApiRole.canUpdateCardsForBank) } } diff --git a/scripts/resource_doc_baseline/parity_allowlist.json b/scripts/resource_doc_baseline/parity_allowlist.json index 8b7c58b089..d444af705a 100644 --- a/scripts/resource_doc_baseline/parity_allowlist.json +++ b/scripts/resource_doc_baseline/parity_allowlist.json @@ -723,6 +723,22 @@ "lift_digest": "1c0ca8143e3e026ebd7129941384db521bac7e280dc0bcf11399e89a1575a6e5", "http4s_digest": "996f7a532a9fd29d5a20500f888ca982522856316442612d4ce4087702ae1c3b" }, + { + "version": "v3_1_0", + "endpoint": "createCardAttribute", + "field": "errorResponseBodies", + "reason": "Added UserHasMissingRoles: canCreateCardsForBank is now required at BANK_ID so only bank staff can write card attributes", + "lift_digest": "ab6b9a504c9dafbcc134976d9f0b976cbe222c9054b9c811c5570adfb79ae6ce", + "http4s_digest": "647479a642a49ee3a80a4bfc28ab2ae3413796d26a3eecbc48411d6e05620ad5" + }, + { + "version": "v3_1_0", + "endpoint": "updateCardAttribute", + "field": "errorResponseBodies", + "reason": "Added UserHasMissingRoles: canUpdateCardsForBank is now required at BANK_ID so only bank staff can overwrite card attributes", + "lift_digest": "ab6b9a504c9dafbcc134976d9f0b976cbe222c9054b9c811c5570adfb79ae6ce", + "http4s_digest": "647479a642a49ee3a80a4bfc28ab2ae3413796d26a3eecbc48411d6e05620ad5" + }, { "version": "v3_1_0", "endpoint": "deleteSystemView", From de2e7b554d4bab3d4537a937bc7345f4e1ee323d Mon Sep 17 00:00:00 2001 From: simonredfern Date: Mon, 19 Oct 2026 09:51:34 +0100 Subject: [PATCH 21/30] fix: any view, the owner view included, can now be revoked from any user, also from an account holder or the only user with owner access (the canRevokeOwnerAccess rule that refused this is removed, and with it a revoked consent no longer keeps its owner access to an account), but a user cannot revoke their own access: the v1.2.1, v4.0.0 and v5.1.0 revoke endpoints refuse that with OBP-20111 CannotRevokeOwnAccountAccess (403). --- docs/operations/CONSENT_STALE_ACCESS.md | 40 ++++-------- .../scala/code/api/util/ConsentUtil.scala | 10 +-- .../scala/code/api/util/ErrorMessages.scala | 3 +- .../scala/code/api/v1_2_1/Http4s121.scala | 14 +++- .../scala/code/api/v4_0_0/Http4s400.scala | 7 +- .../scala/code/api/v5_1_0/Http4s510.scala | 7 +- .../main/scala/code/views/MapperViews.scala | 65 +++---------------- .../scala/code/api/v1_2_1/API1_2_1Test.scala | 41 ++++++++---- .../code/api/v4_0_0/AccountAccessTest.scala | 19 +++++- .../code/api/v5_1_0/AccountAccessTest.scala | 17 +++++ .../parity_allowlist.json | 56 ++++++++++++++-- 11 files changed, 167 insertions(+), 112 deletions(-) diff --git a/docs/operations/CONSENT_STALE_ACCESS.md b/docs/operations/CONSENT_STALE_ACCESS.md index 09041b27f0..0cf0a923c7 100644 --- a/docs/operations/CONSENT_STALE_ACCESS.md +++ b/docs/operations/CONSENT_STALE_ACCESS.md @@ -43,30 +43,15 @@ stale row. Treat the WARN as live over-exposure, not as a cosmetic inconsistency ## Why the revoke was refused -`revokeAccess` ([`MapperViews.scala:270`](../../obp-api/src/main/scala/code/views/MapperViews.scala#L270)) -delegates to `canRevokeOwnerAccess` -([`MapperViews.scala:405`](../../obp-api/src/main/scala/code/views/MapperViews.scala#L405)), which -refuses in exactly two cases, and **only for the `owner` view**: +Until 2026-10-19 `MapperViews` had a rule, `canRevokeOwnerAccess`, that refused to revoke the `owner` +view from an account holder, or from the only user holding `owner` on the account. That rule produced +the `Failure(access cannot be revoked)` in the example above. It has been removed: every revoke path, +the consent reconciliation included, now removes any view, `owner` included. -1. the user is an account holder on that account (`MapperAccountHolders`), or -2. that `AccountAccess` row is the **only** `owner` row on the account — - `findAllByBankIdAccountIdViewId(...).length > 1` is false - -Any other view id returns `true` unconditionally, so a WARN naming a view other than `owner` means -something outside these two rules failed — read the `Failure(...)` text at the end of the line -rather than following this runbook. - -### Case 2 is a signal about the account, not just the consent - -A shadow user is never an account holder, so in practice you are looking at case 2. And case 2 says -something stronger than "this consent is stuck": - -> **Nobody else holds `owner` on that account — not even the PSU.** - -A healthy account has the PSU's own `owner` row, which makes the count 2 and lets the revoke -succeed. If a consent's shadow user is the *sole* `owner` holder, the account has lost its real -owner access. **Investigate that before deleting anything**, or you will clear the symptom and leave -an account nobody owns. +On a build with that change, this WARN means something else failed. Read the `Failure(...)` text at +the end of the line (for example `CannotFindAccountAccess`) rather than following the rest of this +runbook. On an older build, the rule above is the likely cause, and the diagnosis and resolution below +apply. ## Diagnosis @@ -80,8 +65,9 @@ JOIN resourceuser ru ON ru.id = aa.user_fk WHERE ru.userid_ = '' AND aa.bank_id = '' AND aa.account_id = '' AND aa.view_id = 'owner'; --- 2. Who else holds owner on this account? An empty result besides row 1 is case 2, --- and is the finding that matters. +-- 2. Who else holds owner on this account? An empty result besides row 1 means the shadow user +-- was the only owner, which the old rule refused to revoke, and means the account has lost its +-- real owner access: investigate that before deleting anything. SELECT aa.id, ru.userid_, ru.createdbyconsentid, aa.consumer_id FROM accountaccess aa JOIN resourceuser ru ON ru.id = aa.user_fk @@ -144,8 +130,8 @@ for revoked consent 84d5a583-…. The access outlives the consent: Failure(acces `revokeConsentAccountAccess` ([`ConsentUtil.scala:920`](../../obp-api/src/main/scala/code/api/util/ConsentUtil.scala#L920)) goes -through `revokeAccessToViewForUserAndConsumer`, which applies the same `canRevokeOwnerAccess` rule, -so it can be refused for exactly the reasons above. +through `revokeAccessToViewForUserAndConsumer`, which applied the same `canRevokeOwnerAccess` rule on +builds before 2026-10-19, so it could be refused for exactly the reasons above. **Treat this as higher priority than the `grantAccessToViews` line.** In that case a live consent is over-serving, and revoking the consent would still clean up. Here the consent is *already gone* and 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 391487ad6b..0b196fa554 100644 --- a/obp-api/src/main/scala/code/api/util/ConsentUtil.scala +++ b/obp-api/src/main/scala/code/api/util/ConsentUtil.scala @@ -535,10 +535,6 @@ object Consent extends MdcLoggable { // no way back short of deleting rows by hand. The narrower reading is that the consent still // holds everything it declares, plus one view it should have lost; refusing everything is a // worse answer to that than serving it and saying so loudly. - // - // canRevokeOwnerAccess is the realistic trigger (MapperViews): it refuses to drop an `owner` - // row when no other principal holds one on that account, which an OBP-native consent can reach - // because createConsentJWT takes its views from whatever the PSU already holds, `owner` included. for { staleAccess <- held.filterNot(wantedSet.contains) } yield { @@ -1031,9 +1027,9 @@ object Consent extends MdcLoggable { if (count > 0) { logger.info(s"revokeConsentAccountAccess: dropped $count account access rows for consent ${consent.consentId}") } - // revokeAccessToViewForUserAndConsumer can refuse -- canRevokeOwnerAccess will not drop the - // last `owner` row on an account -- so counting the attempts rather than the successes - // reported rows as gone while they were still there. On a revoked consent that is the worst + // revokeAccessToViewForUserAndConsumer can fail (for example when the row is not found), so + // counting the attempts rather than the successes would report rows as gone while they were + // still there. On a revoked consent that is the worst // version of this to get wrong: the access outlives the consent entirely, and nothing else // ever comes back for it. for ((access, outcome) <- stuck) { 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 80eee321d2..fba094b280 100644 --- a/obp-api/src/main/scala/code/api/util/ErrorMessages.scala +++ b/obp-api/src/main/scala/code/api/util/ErrorMessages.scala @@ -363,7 +363,8 @@ object ErrorMessages { val UserAlreadyValidated = "OBP-20107: User email is already validated." val InvalidRedirectUrl = "OBP-20108: Invalid redirect URL. Each redirect URL must use https, use http only for localhost, 127.0.0.1 or [::1], or use an app scheme in reverse-domain form such as com.example.app, and must not contain a wildcard, user information or a fragment. Rejected entry: " val RedirectUrlHostNotAllowed = "OBP-20109: Redirect URL host not allowed. This instance only accepts redirect URLs that point to the hosts listed for it. Rejected entry: " - val OwnerViewAccessMustRemain = "OBP-20110: Owner view access must remain. The user is an account holder of this account, or the only user with access to its owner view, so they keep their owner view access. No access has been revoked." + // OBP-20110 (OwnerViewAccessMustRemain) is retired: account holders and the last owner can now lose their owner view access. + val CannotRevokeOwnAccountAccess = "OBP-20111: Cannot revoke your own account access. A user cannot revoke their own access to the views of an account; another user with permission to revoke access must do it." // OAuth 2 val ApplicationNotIdentified = "OBP-20200: The application cannot be identified. " diff --git a/obp-api/src/main/scala/code/api/v1_2_1/Http4s121.scala b/obp-api/src/main/scala/code/api/v1_2_1/Http4s121.scala index 8509b96453..304a5456e4 100644 --- a/obp-api/src/main/scala/code/api/v1_2_1/Http4s121.scala +++ b/obp-api/src/main/scala/code/api/v1_2_1/Http4s121.scala @@ -900,6 +900,9 @@ object Http4s121 { for { (_, callContext) <- NewStyle.function.getBank(BankId(bankId), Some(cc)) (account, callContext) <- NewStyle.function.getBankAccount(BankId(bankId), AccountId(accountId), callContext) + _ <- Helper.booleanToFuture(CannotRevokeOwnAccountAccess, 403, cc = callContext) { + !(user.provider == provider && user.idGivenByProvider == providerId) + } _ <- ViewNewStyle.revokeAccessToView(account, user, BankIdAccountIdViewId(BankId(bankId), AccountId(accountId), ViewId(viewId)), provider, providerId, callContext) } yield () } @@ -912,13 +915,15 @@ object Http4s121 { "/banks/BANK_ID/accounts/ACCOUNT_ID/permissions/PROVIDER/PROVIDER_ID/views/VIEW_ID", "Revoke access to one View", s"""Revokes access to a View on an Account for a certain User. + | + |Any View can be revoked, the owner view included, also from an account holder. A User cannot revoke their own access: that request is refused with $CannotRevokeOwnAccountAccess | |$generalRevokeAccessToViewText | |${userAuthenticationMessage(true)} and the user needs to have access to the owner view.""", EmptyBody, EmptyBody, - List(AuthenticatedUserIsRequired, BankAccountNotFound, "could not save the privilege", "user does not have access to owner view on account", UnknownError), + List(AuthenticatedUserIsRequired, BankAccountNotFound, CannotRevokeOwnAccountAccess, "could not save the privilege", "user does not have access to owner view on account", UnknownError), List(apiTagView, apiTagAccount, apiTagUser, apiTagEntitlement, apiTagOwnerRequired), http4sPartialFunction = Some(removePermissionForUserForBankAccountForOneView) ) @@ -931,6 +936,9 @@ object Http4s121 { for { (_, callContext) <- NewStyle.function.getBank(BankId(bankId), Some(cc)) (account, callContext) <- NewStyle.function.getBankAccount(BankId(bankId), AccountId(accountId), callContext) + _ <- Helper.booleanToFuture(CannotRevokeOwnAccountAccess, 403, cc = callContext) { + !(user.provider == provider && user.idGivenByProvider == providerId) + } _ <- NewStyle.function.revokeAllAccountAccess(account, user, provider, providerId, callContext) } yield () } @@ -944,14 +952,14 @@ object Http4s121 { "Revoke access to all Views on Account", s""""Revokes access to all Views on an Account for a certain User. | - |Nothing is revoked if the User is an account holder of the Account and has access to its owner view, or if the User is the only one with access to the owner view: the request is refused with $OwnerViewAccessMustRemain + |Every View is revoked, the owner view included, also from an account holder. A User cannot revoke their own access: that request is refused with $CannotRevokeOwnAccountAccess | |$generalRevokeAccessToViewText | |${userAuthenticationMessage(true)} and the user needs to have access to the owner view.""", EmptyBody, EmptyBody, - List(AuthenticatedUserIsRequired, BankAccountNotFound, OwnerViewAccessMustRemain, UnknownError, "user does not have access to owner view on account"), + List(AuthenticatedUserIsRequired, BankAccountNotFound, CannotRevokeOwnAccountAccess, UnknownError, "user does not have access to owner view on account"), List(apiTagView, apiTagAccount, apiTagUser, apiTagOwnerRequired), http4sPartialFunction = Some(removePermissionForUserForBankAccountForAllViews) ) diff --git a/obp-api/src/main/scala/code/api/v4_0_0/Http4s400.scala b/obp-api/src/main/scala/code/api/v4_0_0/Http4s400.scala index 3bc1078bdd..ba0ddfd2fd 100644 --- a/obp-api/src/main/scala/code/api/v4_0_0/Http4s400.scala +++ b/obp-api/src/main/scala/code/api/v4_0_0/Http4s400.scala @@ -5217,6 +5217,9 @@ object Http4s400 { cc = Some(cc)) { APIUtil.canRevokeAccessToView(bankId, accountId, viewId, loggedInUser, Some(cc)) } + _ <- code.util.Helper.booleanToFuture(CannotRevokeOwnAccountAccess, 403, cc = Some(cc)) { + postJson.user_id != loggedInUser.userId + } (targetUser, _) <- NewStyle.function.findByUserId(postJson.user_id, Some(cc)) view <- if (postJson.view.is_system) ViewNewStyle.systemView(viewId, Some(cc)) @@ -5434,6 +5437,8 @@ object Http4s400 { "/banks/BANK_ID/accounts/ACCOUNT_ID/account-access/revoke", "Revoke User access to View", s"""Revoke the User identified by USER_ID access to the view identified by VIEW_ID. + | + |Any view can be revoked, the owner view included, also from an account holder. A User cannot revoke their own access: that request is refused with $CannotRevokeOwnAccountAccess | |${userAuthenticationMessage( true @@ -5442,7 +5447,7 @@ object Http4s400 { |""", postAccountAccessJsonV400, revokedJsonV400, List($AuthenticatedUserIsRequired, - UserLacksPermissionCanRevokeAccessToViewForTargetAccount, + UserLacksPermissionCanRevokeAccessToViewForTargetAccount, CannotRevokeOwnAccountAccess, InvalidJsonFormat, UserNotFoundById, SystemViewNotFound, ViewNotFound, CannotRevokeAccountAccess, CannotFindAccountAccess, UnknownError), List(apiTagAccountAccess, apiTagView, apiTagAccount, apiTagUser, apiTagOwnerRequired), None, 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 c733e25e04..f95d818c86 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 @@ -3418,6 +3418,9 @@ object Http4s510 { _ <- Helper.booleanToFuture(msg, 403, cc = Some(cc)) { APIUtil.canRevokeAccessToView(com.openbankproject.commons.model.BankIdAccountIdViewId(bankId, accountId, viewId), targetViewId, user, Some(cc)) } + _ <- Helper.booleanToFuture(CannotRevokeOwnAccountAccess, 403, cc = Some(cc)) { + postJson.user_id != user.userId + } (targetUser, _) <- NewStyle.function.findByUserId(postJson.user_id, Some(cc)) view <- if (isValidSystemViewId(targetViewId.value)) ViewNewStyle.systemView(targetViewId, Some(cc)) else ViewNewStyle.customView(targetViewId, BankIdAccountId(bankId, accountId), Some(cc)) @@ -3435,6 +3438,8 @@ object Http4s510 { "Revoke User access to View", s"""Revoke the User identified by USER_ID access to the view identified. | + |Any view can be revoked, the owner view included, also from an account holder. A User cannot revoke their own access: that request is refused with $CannotRevokeOwnAccountAccess + | |${userAuthenticationMessage(true)}. | |""", @@ -3442,7 +3447,7 @@ object Http4s510 { revokedJsonV400, List($AuthenticatedUserIsRequired, $BankNotFound, $BankAccountNotFound, $UserNoPermissionAccessView, UserLacksPermissionCanRevokeAccessToCustomViewForTargetAccount, - UserLacksPermissionCanRevokeAccessToSystemViewForTargetAccount, + UserLacksPermissionCanRevokeAccessToSystemViewForTargetAccount, CannotRevokeOwnAccountAccess, InvalidJsonFormat, UserNotFoundById, SystemViewNotFound, ViewNotFound, CannotRevokeAccountAccess, CannotFindAccountAccess, UnknownError), List(apiTagAccountAccess, apiTagView, apiTagAccount, apiTagUser, apiTagOwnerRequired), diff --git a/obp-api/src/main/scala/code/views/MapperViews.scala b/obp-api/src/main/scala/code/views/MapperViews.scala index 106e27b317..6e3ecdc5cf 100644 --- a/obp-api/src/main/scala/code/views/MapperViews.scala +++ b/obp-api/src/main/scala/code/views/MapperViews.scala @@ -27,7 +27,6 @@ TESOBE (http://www.tesobe.com/) package code.views -import code.accountholders.MapperAccountHolders import code.api.APIFailure import code.api.Constant._ import code.api.util.APIUtil._ @@ -284,15 +283,13 @@ object MapperViews extends Views with MdcLoggable { val isRevokedSystemViewAccess = for { - systemViewDefinition <- ViewDefinition.findSystemView(bankIdAccountIdViewId.viewId.value) + _ <- ViewDefinition.findSystemView(bankIdAccountIdViewId.viewId.value) accountAccess <- AccountAccess.findByBankIdAccountIdViewIdUserPrimaryKey( bankIdAccountIdViewId.bankId, bankIdAccountIdViewId.accountId, bankIdAccountIdViewId.viewId, user.userPrimaryKey ) ?~! CannotFindAccountAccess - // Check if we are allowed to remove the View from the User - _ <- canRevokeOwnerAccessAsBox(bankIdAccountIdViewId.bankId, bankIdAccountIdViewId.accountId,systemViewDefinition, user) } yield { accountAccess.delete_! } @@ -304,15 +301,13 @@ object MapperViews extends Views with MdcLoggable { def revokeAccessToSystemView(bankId: BankId, accountId: AccountId, view : View, user : User) : Box[Boolean] = { val res = for { - systemViewDefinition <- ViewDefinition.find(By(ViewDefinition.id_, view.id)) + _ <- ViewDefinition.find(By(ViewDefinition.id_, view.id)) accountAccess <- AccountAccess.findByBankIdAccountIdViewIdUserPrimaryKey( bankId, accountId, view.viewId, user.userPrimaryKey ) ?~! CannotFindAccountAccess - // Check if we are allowed to remove the View from the User - _ <- canRevokeOwnerAccessAsBox(bankId: BankId, accountId: AccountId, systemViewDefinition, user) } yield { accountAccess.delete_! } @@ -379,9 +374,8 @@ object MapperViews extends Views with MdcLoggable { val isRevokedSystemViewAccess = for { - systemViewDefinition <- ViewDefinition.findSystemView(bankIdAccountIdViewId.viewId.value) + _ <- ViewDefinition.findSystemView(bankIdAccountIdViewId.viewId.value) accountAccess <- accountAccessRow - _ <- canRevokeOwnerAccessAsBox(bankIdAccountIdViewId.bankId, bankIdAccountIdViewId.accountId, systemViewDefinition, user) } yield { accountAccess.delete_! } @@ -396,59 +390,20 @@ object MapperViews extends Views with MdcLoggable { ).map(row => BankIdAccountIdViewId(BankId(row.bank_id.get), AccountId(row.account_id.get), ViewId(row.view_id.get))) } - //returns Full if deletable, Failure if not - def canRevokeOwnerAccessAsBox(bankId: BankId, accountId: AccountId, viewDefinition : ViewDefinition, user : User) : Box[Unit] = { - if(canRevokeOwnerAccess(bankId: BankId, accountId: AccountId, viewDefinition, user)) Full(()) - else Failure("access cannot be revoked") - } - - - def canRevokeOwnerAccess(bankId: BankId, accountId: AccountId, viewDefinition: ViewDefinition, user : User) : Boolean = { - if(viewDefinition.viewId == ViewId(SYSTEM_OWNER_VIEW_ID)) { - //if the user is an account holder, we can't revoke access to the owner view - val accountHolders = MapperAccountHolders.getAccountHolders(viewDefinition.bankId, viewDefinition.accountId) - if(accountHolders.map(h => h.userPrimaryKey).contains(user.userPrimaryKey)) { - false - } else { - // if it's the owner view, we can only revoke access if there would then still be someone else - // with access - AccountAccess.findAllByBankIdAccountIdViewId( - bankId: BankId, - accountId: AccountId, - viewDefinition.viewId - ).length > 1 - } - } else { - true - } - } - - /** - * This removes every account access the user has to one account. + * This removes every account access the user has to one account, whichever views they are, the owner + * view included, and whether or not the user is an account holder. * - * The caller's right to revoke is checked at the top level (`canRevokeAccessToAllViews`); that check - * says nothing about the user losing the access. Here the same rule as revoking a single view applies - * (canRevokeOwnerAccess): an account holder keeps the owner view, and the account must keep at least one - * user with owner view access. If any of the user's accesses is protected by that rule, nothing is revoked, - * so the call never leaves the user with only part of their access removed. + * The caller's right to revoke is checked by the caller (`canRevokeAccessToAllViews`), and the endpoints + * refuse a request in which the caller names themselves (`CannotRevokeOwnAccountAccess`). */ def revokeAllAccountAccess(bankId : BankId, accountId: AccountId, user : User) : Box[Boolean] = { - val accountAccesses = AccountAccess.findAll( + AccountAccess.findAll( By(AccountAccess.bank_id, bankId.value), By(AccountAccess.account_id, accountId.value), By(AccountAccess.user_fk, user.userPrimaryKey.value) - ) - val everyAccessCanBeRevoked = accountAccesses.forall(accountAccess => - ViewDefinition.findSystemView(accountAccess.view_id.get) - .forall(systemView => canRevokeOwnerAccess(bankId, accountId, systemView, user)) - ) - if (everyAccessCanBeRevoked) { - accountAccesses.foreach(_.delete_!) - Full(true) - } else { - Failure(OwnerViewAccessMustRemain) - } + ).foreach(_.delete_!) + Full(true) } def revokeAccountAccessByUser(bankId : BankId, accountId: AccountId, user : User, callContext: Option[CallContext]) : Box[Boolean] = { diff --git a/obp-api/src/test/scala/code/api/v1_2_1/API1_2_1Test.scala b/obp-api/src/test/scala/code/api/v1_2_1/API1_2_1Test.scala index 3a1ee12a27..ae876edbb3 100644 --- a/obp-api/src/test/scala/code/api/v1_2_1/API1_2_1Test.scala +++ b/obp-api/src/test/scala/code/api/v1_2_1/API1_2_1Test.scala @@ -2077,6 +2077,26 @@ class API1_2_1Test extends ServerSetupWithTestData with DefaultUsers with Privat viewsAfter should equal(viewsBefore -1) } + scenario("a user cannot revoke their own access to a view on a bank account, even if another user has access to it", API1_2_1, DeletePermission){ + Given("The owner view of an account is held by the caller and by another user") + val bankId = randomBank + val bankAccount : AccountJSON = randomPrivateAccount(bankId) + grantUserAccessToView(bankId, bankAccount.id, resourceUser2.idGivenByProvider, SYSTEM_OWNER_VIEW_ID, user1).code should equal (201) + + When("The caller tries to revoke their own access to the owner view") + val reply = revokeUserAccessToView(bankId, bankAccount.id, resourceUser1.idGivenByProvider, SYSTEM_OWNER_VIEW_ID, user1) + + Then("we should get a 403 code") + reply.code should equal (403) + reply.body.extract[ErrorMessage].message should include (CannotRevokeOwnAccountAccess) + + And("The caller should still have access to the owner view") + AccountAccess.findByBankIdAccountIdViewIdUserPrimaryKey(BankId(bankId), AccountId(bankAccount.id), ViewId(SYSTEM_OWNER_VIEW_ID), resourceUser1.userPrimaryKey).isDefined should equal (true) + + //clean up: the later tests expect user2 to have no access to the test accounts + revokeUserAccessToAllViews(bankId, bankAccount.id, resourceUser2.idGivenByProvider, user1).code should equal (204) + } + scenario("we cannot revoke the access to a user that does not exist", API1_2_1, DeletePermission){ Given("We will use an access token with a random user Id") val bankId = randomBank @@ -2180,9 +2200,8 @@ class API1_2_1Test extends ServerSetupWithTestData with DefaultUsers with Privat viewsAfter should equal(viewsBefore) } - scenario("we cannot revoke the access to the owner view via a revoke all views call if there " + - "would then be no one with access to it", API1_2_1, DeletePermissions){ - Given("The only user with access to the owner view of an account") + scenario("a user cannot revoke their own access to all the views on a bank account", API1_2_1, DeletePermissions){ + Given("A user with access to the owner view of an account") val bankId = randomBank val bankAccount : AccountJSON = randomPrivateAccount(bankId) val userId = resourceUser1.idGivenByProvider @@ -2194,14 +2213,14 @@ class API1_2_1Test extends ServerSetupWithTestData with DefaultUsers with Privat val reply = revokeUserAccessToAllViews(bankId, bankAccount.id, userId, user1) Then("we should get a 403 code") reply.code should equal (403) - reply.body.extract[ErrorMessage].message should include (OwnerViewAccessMustRemain) + reply.body.extract[ErrorMessage].message should include (CannotRevokeOwnAccountAccess) And("None of the user's access to the account should have been revoked") AccountAccess.findByBankIdAccountIdViewIdUserPrimaryKey(BankId(bankId), AccountId(bankAccount.id), ViewId(SYSTEM_OWNER_VIEW_ID), resourceUser1.userPrimaryKey).isDefined should equal (true) getUserAccountPermission(bankId, bankAccount.id, userId, user1).body.extract[ViewsJSONV121].views.length should equal (viewsBefore) } - scenario("we cannot revoke the access of an account holder to the owner view on a bank account via a revoke all views call", + scenario("we can revoke the access of an account holder to the owner view on a bank account via a revoke all views call", API1_2_1, DeletePermissions){ Given("A user is the account holder of an account (and has access to the owner view)") val bankId = randomBank @@ -2215,16 +2234,14 @@ class API1_2_1Test extends ServerSetupWithTestData with DefaultUsers with Privat When("We try to revoke this user's access to all views") val reply = revokeUserAccessToAllViews(bankId, bankAccount.id, resourceUser3.idGivenByProvider, user1) - Then("we should get a 403 code") - reply.code should equal (403) - reply.body.extract[ErrorMessage].message should include (OwnerViewAccessMustRemain) + Then("we should get a 204 code") + reply.code should equal (204) - And("The user should still have access to the owner view of this account") - AccountAccess.findByBankIdAccountIdViewIdUserPrimaryKey(BankId(bankId), AccountId(bankAccount.id), ViewId(ownerViewId), resourceUser3.userPrimaryKey).isDefined should equal (true) + And("The user should no longer have access to the owner view of this account") + AccountAccess.findByBankIdAccountIdViewIdUserPrimaryKey(BankId(bankId), AccountId(bankAccount.id), ViewId(ownerViewId), resourceUser3.userPrimaryKey).isDefined should equal (false) - //clean up: the later tests expect user3 to have no access to the test accounts + //clean up AccountHolders.accountHolders.vend.deleteAccountHolder(resourceUser3, BankIdAccountId(BankId(bankId), AccountId(bankAccount.id))) - revokeUserAccessToAllViews(bankId, bankAccount.id, resourceUser3.idGivenByProvider, user1).code should equal (204) } } diff --git a/obp-api/src/test/scala/code/api/v4_0_0/AccountAccessTest.scala b/obp-api/src/test/scala/code/api/v4_0_0/AccountAccessTest.scala index d27b1d2774..80546593f4 100644 --- a/obp-api/src/test/scala/code/api/v4_0_0/AccountAccessTest.scala +++ b/obp-api/src/test/scala/code/api/v4_0_0/AccountAccessTest.scala @@ -34,7 +34,7 @@ import code.api.ResourceDocs1_4_0.SwaggerDefinitionsJSON.createViewJsonV300 import code.api.util.APIUtil.OAuth._ import code.api.util.ApiRole import com.openbankproject.commons.util.ApiVersion -import code.api.util.ErrorMessages.AuthenticatedUserIsRequired +import code.api.util.ErrorMessages.{AuthenticatedUserIsRequired, CannotRevokeOwnAccountAccess} import code.api.v3_0_0.ViewJsonV300 import code.api.v3_1_0.CreateAccountResponseJsonV310 import code.api.v4_0_0.Http4s400.Implementations4_0_0 @@ -103,6 +103,23 @@ class AccountAccessTest extends V400ServerSetup { } feature(s"test $ApiEndpoint1 and $ApiEndpoint2 and $ApiEndpoint3 version $VersionOfApi - Authorized access") { + scenario("A user cannot revoke their own access", VersionOfApi, ApiEndpoint2) { + val addedEntitlement: Box[Entitlement] = Entitlement.entitlement.vend.addEntitlement(bankId, resourceUser1.userId, ApiRole.CanCreateAccount.toString) + val account = try { + createAnAccount(bankId, user1) + } finally { + Entitlement.entitlement.vend.deleteEntitlement(addedEntitlement) + } + + When("The caller asks to revoke their own access to the owner view") + val postJson = PostAccountAccessJsonV400(resourceUser1.userId, PostViewJsonV400(code.api.Constant.SYSTEM_OWNER_VIEW_ID, true)) + val requestRevoke = (v4_0_0_Request / "banks" / bankId / "accounts" / account.account_id / "account-access" / "revoke").POST <@ (user1) + val responseRevoke = makePostRequest(requestRevoke, write(postJson)) + Then("We should get a 403") + responseRevoke.code should equal(403) + responseRevoke.body.extract[ErrorMessage].message should include(CannotRevokeOwnAccountAccess) + } + scenario("We will call the endpoint with user credentials", VersionOfApi, ApiEndpoint1, ApiEndpoint2) { val addedEntitlement: Box[Entitlement] = Entitlement.entitlement.vend.addEntitlement(bankId, resourceUser1.userId, ApiRole.CanCreateAccount.toString) diff --git a/obp-api/src/test/scala/code/api/v5_1_0/AccountAccessTest.scala b/obp-api/src/test/scala/code/api/v5_1_0/AccountAccessTest.scala index 1a2e88717f..0830c4e329 100644 --- a/obp-api/src/test/scala/code/api/v5_1_0/AccountAccessTest.scala +++ b/obp-api/src/test/scala/code/api/v5_1_0/AccountAccessTest.scala @@ -265,6 +265,23 @@ class AccountAccessTest extends V510ServerSetup { response.body.extract[RevokedJsonV400].revoked should be (true) } + scenario("A user cannot revoke their own access", VersionOfApi, ApiEndpoint2) { + val addedEntitlement: Box[Entitlement] = Entitlement.entitlement.vend.addEntitlement(bankId, resourceUser1.userId, ApiRole.CanCreateAccount.toString) + val account = try { + createAnAccount(bankId, user1) + } finally { + Entitlement.entitlement.vend.deleteEntitlement(addedEntitlement) + } + + When("The caller asks to revoke their own access to the owner view") + val postJson = PostAccountAccessJsonV510(resourceUser1.userId, SYSTEM_OWNER_VIEW_ID) + val requestRevoke = (v5_1_0_Request / "banks" / bankId / "accounts" / account.account_id /"views"/ ownerView / "account-access" / "revoke").POST <@ (user1) + val response = makePostRequest(requestRevoke, write(postJson)) + Then("We should get a 403") + response.code should equal(403) + response.body.extract[ErrorMessage].message should include(CannotRevokeOwnAccountAccess) + } + scenario("We will call the endpoint with user credentials and custom view permission", VersionOfApi, ApiEndpoint1) { val addedEntitlement: Box[Entitlement] = Entitlement.entitlement.vend.addEntitlement(bankId, resourceUser1.userId, ApiRole.CanCreateAccount.toString) val account = try { diff --git a/scripts/resource_doc_baseline/parity_allowlist.json b/scripts/resource_doc_baseline/parity_allowlist.json index d444af705a..de53e9e068 100644 --- a/scripts/resource_doc_baseline/parity_allowlist.json +++ b/scripts/resource_doc_baseline/parity_allowlist.json @@ -207,17 +207,17 @@ "version": "v1_2_1", "endpoint": "removePermissionForUserForBankAccountForAllViews", "field": "description", - "reason": "Added that nothing is revoked when the user is an account holder with owner view access, or the only user with owner view access (OwnerViewAccessMustRemain)", + "reason": "Added that every view is revoked, the owner view included, also from an account holder, and that a user cannot revoke their own access (CannotRevokeOwnAccountAccess)", "lift_digest": "f31da0343c89bc37150a06bc3d8a39ed4e8732a18df8b21e36fdda2bd79517c3", - "http4s_digest": "42437a8a9503f3b2b7a0baaa7103c9e17b1a0380f89c60128c8c408df2067946" + "http4s_digest": "5979c1675784d7512516a10805d8e57669ff941ccf8959ed73bd0ccf6d9e911a" }, { "version": "v1_2_1", "endpoint": "removePermissionForUserForBankAccountForAllViews", "field": "errorResponseBodies", - "reason": "Added OwnerViewAccessMustRemain (403): revoking all views no longer removes an account holder's or the last owner's owner view access", + "reason": "Added CannotRevokeOwnAccountAccess (403): a user cannot revoke their own access to all views", "lift_digest": "b72cefb915c6af0190e4543aed820722db264a43a54247247d9a1fdb25321703", - "http4s_digest": "c7c4a40bf4a76ab083977ab386b6699136746f155c5749910f0b0edd2d77b35c" + "http4s_digest": "1504d43b14defdedbab99f5784e36dbc7d3c6e1eca29fddc41d81f43d3ae0bcd" }, { "version": "v4_0_0", @@ -2450,6 +2450,54 @@ "reason": "States that the paying account is the card's account, that the User needs the owner view with permission to add transaction requests (or CanCreateAnyTransactionRequest), and that the payee must be a counterparty of that account.", "lift_digest": "b670d8181d5d2d563db3a015bb357107c827ad273e2097c4215032c07e39502b", "http4s_digest": "b60ff8942421ae14a3245bcc8625bac7409abef79994972c629d56dafc582840" + }, + { + "version": "v1_2_1", + "endpoint": "removePermissionForUserForBankAccountForOneView", + "field": "description", + "reason": "Added that any view can be revoked, the owner view included, also from an account holder, and that a user cannot revoke their own access (CannotRevokeOwnAccountAccess)", + "lift_digest": "b19742add024159ef6d85db9e167ff2049110aa0e11e449fa05838bd2570f359", + "http4s_digest": "361d3ccf4787cc7c1715a9907c12970266ca09606c73cfde411772c8cc2d7094" + }, + { + "version": "v1_2_1", + "endpoint": "removePermissionForUserForBankAccountForOneView", + "field": "errorResponseBodies", + "reason": "Added CannotRevokeOwnAccountAccess (403): a user cannot revoke their own access to a view", + "lift_digest": "56b707566120b88065477b5618fdc5886dc75f45b2e4b1053dd0bdbda41e2f8f", + "http4s_digest": "bbfff69a556f8601736d5ac6e63d20d0cd31b761b64b443e23082f81001bfcfb" + }, + { + "version": "v4_0_0", + "endpoint": "revokeUserAccessToView", + "field": "description", + "reason": "Added that any view can be revoked, the owner view included, also from an account holder, and that a user cannot revoke their own access (CannotRevokeOwnAccountAccess)", + "lift_digest": "07f97f416e1380baf140efbc77647a2ef36937952c7238b79b03f184d241f3d9", + "http4s_digest": "554e40e9d4feffb8ea320c5d202bb00ba3b8d8c2c4daee055d07a1ad61c6a2a8" + }, + { + "version": "v4_0_0", + "endpoint": "revokeUserAccessToView", + "field": "errorResponseBodies", + "reason": "Added CannotRevokeOwnAccountAccess (403): a user cannot revoke their own access to a view", + "lift_digest": "53e6dd5b338b2443d8084e1f3112e6f8ec63ef81faa21f9bcad879997dabacd9", + "http4s_digest": "81f765f4125feedda61e1d75adb8ae6929c71df660303e2eac0b64d6eec1de89" + }, + { + "version": "v5_1_0", + "endpoint": "revokeUserAccessToViewById", + "field": "description", + "reason": "Added that any view can be revoked, the owner view included, also from an account holder, and that a user cannot revoke their own access (CannotRevokeOwnAccountAccess)", + "lift_digest": "5b65d5501056e25545dc7000a0b84e1d93de3a7a69fcd88e06afabeb03da0575", + "http4s_digest": "817a759ab7bde7501e16460d69e2b066fc124a682df74d38ead6adaf759ddc94" + }, + { + "version": "v5_1_0", + "endpoint": "revokeUserAccessToViewById", + "field": "errorResponseBodies", + "reason": "Added CannotRevokeOwnAccountAccess (403): a user cannot revoke their own access to a view", + "lift_digest": "46a5a1e4ea4bcb0ccae8f5338b321f9684a6cc591b6cdf13f1a98a2582d4eb27", + "http4s_digest": "110c6fd1269109a3c11cce5d9b922de6c421d0335558791f9bcf521d400d7a57" } ] } From c0b70c454b273966b24b363485fd00ba78e73e64 Mon Sep 17 00:00:00 2001 From: simonredfern Date: Mon, 19 Oct 2026 10:38:19 +0100 Subject: [PATCH 22/30] Fix: Make sure that Update My Api Collection only changes a collection owned by the logged-in user --- .../scala/code/api/v5_1_0/Http4s510.scala | 13 +++++++-- .../code/api/v5_1_0/ApiCollectionTest.scala | 27 ++++++++++++++++++- .../parity_allowlist.json | 8 ++++++ 3 files changed, 45 insertions(+), 3 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 f95d818c86..f30b73f74d 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 @@ -1966,7 +1966,15 @@ object Http4s510 { putJson <- NewStyle.function.tryons(s"$InvalidJsonFormat The Json body should be the ${classOf[code.api.v4_0_0.PostApiCollectionJson400].getSimpleName}", 400, Some(cc)) { com.openbankproject.commons.util.JsonAliases.parse(cc.httpBody.getOrElse("")).extract[code.api.v4_0_0.PostApiCollectionJson400] } - (_, _) <- NewStyle.function.getApiCollectionById(apiCollectionId, Some(cc)) + // the caller, to compare with the collection's owner below + user <- Future.successful(cc.user.openOrThrowException(AuthenticatedUserIsRequired)) + // keep the collection to compare its owner with the caller + (existing, _) <- NewStyle.function.getApiCollectionById(apiCollectionId, Some(cc)) + // only the owner updates it, else any user renames or shares any user's collection; answered as not found so nothing is told about other users' collections + _ <- code.util.Helper.booleanToFuture( + s"$ApiCollectionNotFound Please specify a valid value for API_COLLECTION_ID. Current API_COLLECTION_ID($apiCollectionId) ", 404, Some(cc)) { + existing.userId == user.userId + } (apiCollection, _) <- NewStyle.function.updateApiCollection( apiCollectionId, putJson.api_collection_name, putJson.is_sharable, putJson.description.getOrElse(""), Some(cc)) } yield JSONFactory400.createApiCollectionJsonV400(apiCollection) @@ -1984,7 +1992,8 @@ object Http4s510 { |""".stripMargin, postApiCollectionJson400, apiCollectionJson400, - List($AuthenticatedUserIsRequired, InvalidJsonFormat, UserNotFoundByUserId, UnknownError), + // ApiCollectionNotFound: no such collection, or it belongs to another user + List($AuthenticatedUserIsRequired, InvalidJsonFormat, UserNotFoundByUserId, ApiCollectionNotFound, UnknownError), List(apiTagApiCollection), None, http4sPartialFunction = Some(updateMyApiCollection) diff --git a/obp-api/src/test/scala/code/api/v5_1_0/ApiCollectionTest.scala b/obp-api/src/test/scala/code/api/v5_1_0/ApiCollectionTest.scala index d7c46e49ec..72098e5317 100644 --- a/obp-api/src/test/scala/code/api/v5_1_0/ApiCollectionTest.scala +++ b/obp-api/src/test/scala/code/api/v5_1_0/ApiCollectionTest.scala @@ -29,7 +29,7 @@ import org.json4s._ import code.api.ResourceDocs1_4_0.SwaggerDefinitionsJSON import code.api.util.APIUtil.OAuth._ import code.api.util.ApiRole -import code.api.util.ErrorMessages.{UserHasMissingRoles, AuthenticatedUserIsRequired} +import code.api.util.ErrorMessages.{ApiCollectionNotFound, UserHasMissingRoles, AuthenticatedUserIsRequired} import code.api.v4_0_0.Http4s400.Implementations4_0_0 import code.api.v4_0_0.{ApiCollectionJson400, ApiCollectionsJson400} import code.api.v5_1_0.Http4s510.Implementations5_1_0 @@ -160,4 +160,29 @@ class ApiCollectionTest extends V510ServerSetup { } } + feature(s"test $ApiEndpoint3 version $VersionOfApi - another user's collection") { + scenario("We will try to update the API Collection of another user", ApiEndpoint3, VersionOfApi) { + // user1 owns the collection + val requestPost510 = (v5_1_0_Request / "my" / "api-collections").POST <@ (user1) + val postApiCollectionJson = SwaggerDefinitionsJSON.postApiCollectionJson400.copy(api_collection_name = "owned-by-user1") + val responsePost510 = makePostRequest(requestPost510, write(postApiCollectionJson)) + responsePost510.code should equal(201) + val apiCollectionId = responsePost510.body.extract[ApiCollectionJson400].api_collection_id + + // user2 must not rename or share it: answered as not found + When(s"user2 makes a request $ApiEndpoint3 on user1's collection") + val requestPut510 = (v5_1_0_Request / "my" / "api-collections" / apiCollectionId).PUT <@ (user2) + val putApiCollectionJson = postApiCollectionJson.copy(api_collection_name = "renamed-by-user2", is_sharable = true) + val responsePut510 = makePutRequest(requestPut510, write(putApiCollectionJson)) + Then("We should get a 404") + responsePut510.code should equal(404) + responsePut510.body.extract[ErrorMessage].message should startWith(ApiCollectionNotFound) + + // the collection is unchanged + val responseGet = makeGetRequest((v5_1_0_Request / "my" / "api-collections" / apiCollectionId).GET <@ (user1)) + responseGet.code should equal(200) + responseGet.body.extract[ApiCollectionJson400].api_collection_name should be ("owned-by-user1") + } + } + } diff --git a/scripts/resource_doc_baseline/parity_allowlist.json b/scripts/resource_doc_baseline/parity_allowlist.json index de53e9e068..c28586b113 100644 --- a/scripts/resource_doc_baseline/parity_allowlist.json +++ b/scripts/resource_doc_baseline/parity_allowlist.json @@ -2498,6 +2498,14 @@ "reason": "Added CannotRevokeOwnAccountAccess (403): a user cannot revoke their own access to a view", "lift_digest": "46a5a1e4ea4bcb0ccae8f5338b321f9684a6cc591b6cdf13f1a98a2582d4eb27", "http4s_digest": "110c6fd1269109a3c11cce5d9b922de6c421d0335558791f9bcf521d400d7a57" + }, + { + "version": "v5_1_0", + "endpoint": "updateMyApiCollection", + "field": "errorResponseBodies", + "reason": "Added ApiCollectionNotFound: 404 when the collection does not exist or belongs to another user, so only its owner can update it", + "lift_digest": "ea8061d2189d61d4f84552f70fa4482c88aa1fb6d5f7692b7c3c6eae0b24f4e6", + "http4s_digest": "181a990959c91843d2617852d9247a24ae916364b5b3afa58fab87ddd6692130" } ] } From 8b6da332609289dbecafd1ad69d7ced2f2ceb40a Mon Sep 17 00:00:00 2001 From: simonredfern Date: Mon, 19 Oct 2026 10:47:59 +0100 Subject: [PATCH 23/30] Fix: Make sure that the data warehouse search no longer writes query bodies or returned warehouse rows to the INFO log --- obp-api/src/main/scala/code/search/search.scala | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/obp-api/src/main/scala/code/search/search.scala b/obp-api/src/main/scala/code/search/search.scala index f8798fa4cc..efc0832aa4 100644 --- a/obp-api/src/main/scala/code/search/search.scala +++ b/obp-api/src/main/scala/code/search/search.scala @@ -78,7 +78,8 @@ class elasticsearch extends MdcLoggable { val httpHost = "http://" + esHost + ":" + esPortHTTP val esUrl = s"${httpHost}${uri.replaceAll("\"", "")}" logger.info(s"searchProxyV300 says esUrl is: $esUrl") - logger.info(s"searchProxyV300 says body is: $body") + // the query body can hold search terms about people, so it is only logged at DEBUG + logger.debug(s"searchProxyV300 says body is: $body") val response = getAPIResponse(esUrl, body) if (statsOnly) Full(privacyCheckStatistics(response.body)) else Full(response.body) @@ -91,10 +92,11 @@ class elasticsearch extends MdcLoggable { val httpHost = "http://" + esHost + ":" + esPortHTTP val esUrl = s"${httpHost}${uri.replaceAll("\"", "")}" logger.info(s"searchProxyAsyncV300 says esUrl is: $esUrl") - logger.info(s"searchProxyAsyncV300 says body is: $body") + // the query body can hold search terms about people, so it is only logged at DEBUG + logger.debug(s"searchProxyAsyncV300 says body is: $body") val response = getAPIResponseAsync(esUrl, body) - logger.info(s"searchProxyAsyncV300 says response follows:") - response foreach { msg => logger.info(msg.body) } + // log only the status code: the response body holds warehouse rows that must not reach the logs + response foreach { msg => logger.info(s"searchProxyAsyncV300 says response code is: ${msg.code}") } response } From 5b140c3ac4c8a06fd214b7904cffb4fda44e7482 Mon Sep 17 00:00:00 2001 From: simonredfern Date: Mon, 19 Oct 2026 10:53:53 +0100 Subject: [PATCH 24/30] Fix: Make sure that Update Product Attribute v3.1.0 only changes an attribute of the product in the path, at the bank in the path --- .../scala/code/api/util/ErrorMessages.scala | 2 + .../scala/code/api/v3_1_0/Http4s310.scala | 11 ++++- .../api/v3_1_0/ProductAttributeTest.scala | 41 ++++++++++++++++++- .../parity_allowlist.json | 8 ++++ 4 files changed, 59 insertions(+), 3 deletions(-) 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 fba094b280..e2bb032945 100644 --- a/obp-api/src/main/scala/code/api/util/ErrorMessages.scala +++ b/obp-api/src/main/scala/code/api/util/ErrorMessages.scala @@ -505,6 +505,8 @@ object ErrorMessages { val DeleteWebhookError = "OBP-30152: Could not delete the Webhook." // an account attribute that does not exist, or does not belong to the BANK_ID / ACCOUNT_ID / PRODUCT_CODE in the path val AccountAttributeNotFound = "OBP-30153: Account Attribute not found. Please specify a valid value for ACCOUNT_ATTRIBUTE_ID for this BANK_ID, ACCOUNT_ID and PRODUCT_CODE." + // a product attribute that does not exist, or does not belong to the BANK_ID / PRODUCT_CODE in the path + val ProductAttributeNotFound = "OBP-30154: Product Attribute not found. Please specify a valid value for PRODUCT_ATTRIBUTE_ID for this BANK_ID and PRODUCT_CODE." val CreateWebhookError = "OBP-30047: Cannot create Webhook" val GetWebhooksError = "OBP-30048: Cannot get Webhooks" diff --git a/obp-api/src/main/scala/code/api/v3_1_0/Http4s310.scala b/obp-api/src/main/scala/code/api/v3_1_0/Http4s310.scala index 63fd267be0..431faf9741 100644 --- a/obp-api/src/main/scala/code/api/v3_1_0/Http4s310.scala +++ b/obp-api/src/main/scala/code/api/v3_1_0/Http4s310.scala @@ -2740,7 +2740,13 @@ object Http4s310 { s"$InvalidJsonFormat The `Type` field can only accept the following field: " + AttributeTypeDocs.typesWithExamples, 400, Some(cc)) { ProductAttributeType.withName(postedData.`type`) } - (_, _) <- NewStyle.function.getProductAttributeById(productAttributeIdStr, Some(cc)) + // keep the existing attribute so it can be checked against the path + (existingAttribute, _) <- NewStyle.function.getProductAttributeById(productAttributeIdStr, Some(cc)) + // the attribute must belong to the bank and product in the path, else a role at one bank overwrites and moves any bank's attribute + _ <- code.util.Helper.booleanToFuture(ProductAttributeNotFound, 404, Some(cc)) { + existingAttribute.bankId.value == bankIdStr && + existingAttribute.productCode.value == productCodeStr + } (productAttribute, _) <- NewStyle.function.createOrUpdateProductAttribute( BankId(bankIdStr), ProductCode(productCodeStr), Some(productAttributeIdStr), postedData.name, productAttributeType, postedData.value, None, Some(cc)) @@ -2766,7 +2772,8 @@ object Http4s310 { |""", productAttributeJson, productAttributeResponseJson, - List(UserHasMissingRoles, UnknownError), + // ProductAttributeNotFound: the attribute is not at the bank and product in the path + List(UserHasMissingRoles, ProductAttributeNotFound, UnknownError), List(apiTagProduct, apiTagProductAttribute, apiTagAttribute), Some(List(canUpdateProductAttribute)), http4sPartialFunction = Some(updateProductAttribute) diff --git a/obp-api/src/test/scala/code/api/v3_1_0/ProductAttributeTest.scala b/obp-api/src/test/scala/code/api/v3_1_0/ProductAttributeTest.scala index 051cbc761a..196cd7f1ef 100644 --- a/obp-api/src/test/scala/code/api/v3_1_0/ProductAttributeTest.scala +++ b/obp-api/src/test/scala/code/api/v3_1_0/ProductAttributeTest.scala @@ -34,6 +34,11 @@ import com.openbankproject.commons.util.ApiVersion import code.api.util.ErrorMessages._ import code.api.v3_1_0.Http4s310.Implementations3_1_0 import code.entitlement.Entitlement +// to create test attributes at another bank or product directly through the provider +import code.productattribute.ProductAttributeX +import com.openbankproject.commons.model.{BankId, ProductCode} +import scala.concurrent.Await +import scala.concurrent.duration.Duration import com.github.dwickern.macros.NameOf.nameOf import com.openbankproject.commons.model.enums.ProductAttributeType import org.json4s.native.Serialization.write @@ -207,8 +212,42 @@ class ProductAttributeTest extends V310ServerSetup { errorMessage contains (UserHasMissingRoles) should be (true) errorMessage contains (createProductEntitlementsRequiredText.toString()) should be (true) } + // a role at one bank must not overwrite, or move, a product attribute of another bank or another product + scenario("We will call the Update endpoint with an attribute of another bank or another product", ApiEndpoint3, VersionOfApi) { + // an attribute owned by another bank, created directly so no role at that bank is needed + val otherBankAttribute = Await.result( + ProductAttributeX.productAttributeProvider.vend.createOrUpdateProductAttribute( + BankId("other-bank-for-product-attribute"), ProductCode("PRODUCT_CODE"), None, + "other bank attribute", ProductAttributeType.STRING, "12345678", None), + Duration.Inf).openOrThrowException("could not create the test attribute") + // an attribute of another product at the bank in the path + val otherProductAttribute = Await.result( + ProductAttributeX.productAttributeProvider.vend.createOrUpdateProductAttribute( + BankId(testBankId), ProductCode("OTHER_PRODUCT_CODE"), None, + "other product attribute", ProductAttributeType.STRING, "12345678", None), + Duration.Inf).openOrThrowException("could not create the test attribute") + Entitlement.entitlement.vend.addEntitlement(testBankId, resourceUser1.userId, CanUpdateProductAttribute.toString) + When("We update the other bank's attribute through the bank in the path") + val requestOtherBank = (v3_1_0_Request / "banks" / testBankId / "products" / "PRODUCT_CODE" / "attributes" / otherBankAttribute.productAttributeId).PUT <@(user1) + val responseOtherBank = makePutRequest(requestOtherBank, write(putProductAttributeJson)) + Then("We should get a 404") + responseOtherBank.code should equal(404) + responseOtherBank.body.extract[ErrorMessage].message should equal (ProductAttributeNotFound) + And("The attribute stays at its own bank, unchanged") + val stored = Await.result( + ProductAttributeX.productAttributeProvider.vend.getProductAttributeById(otherBankAttribute.productAttributeId), + Duration.Inf).openOrThrowException("the test attribute is gone") + stored.bankId.value should equal ("other-bank-for-product-attribute") + stored.name should equal ("other bank attribute") + When("We update an attribute of another product through the product in the path") + val requestOtherProduct = (v3_1_0_Request / "banks" / testBankId / "products" / "PRODUCT_CODE" / "attributes" / otherProductAttribute.productAttributeId).PUT <@(user1) + val responseOtherProduct = makePutRequest(requestOtherProduct, write(putProductAttributeJson)) + Then("We should get a 404") + responseOtherProduct.code should equal(404) + responseOtherProduct.body.extract[ErrorMessage].message should equal (ProductAttributeNotFound) + } } - + feature("Delete Product Attribute v3.1.0") { scenario("We will call the Delete endpoint without a user credentials", ApiEndpoint4, VersionOfApi) { When("We make a request v3.1.0") diff --git a/scripts/resource_doc_baseline/parity_allowlist.json b/scripts/resource_doc_baseline/parity_allowlist.json index c28586b113..5a2a3161c7 100644 --- a/scripts/resource_doc_baseline/parity_allowlist.json +++ b/scripts/resource_doc_baseline/parity_allowlist.json @@ -2363,6 +2363,14 @@ "lift_digest": "ab6b9a504c9dafbcc134976d9f0b976cbe222c9054b9c811c5570adfb79ae6ce", "http4s_digest": "7c341afe281a974e145362105b5ad7be060660743ac6e3b064fdaa02e586c97b" }, + { + "version": "v3_1_0", + "endpoint": "updateProductAttribute", + "field": "errorResponseBodies", + "reason": "Added ProductAttributeNotFound (404): the attribute must belong to the BANK_ID and PRODUCT_CODE in the path, else a role at one bank could overwrite and move another bank's product attribute", + "lift_digest": "f0e3fc093d8749570c7520dabdb672bbb7a6f84d2f03fa9c138762e8c976981a", + "http4s_digest": "cbff97326a2c683036eca4d241b6bcc82b9a45d7cee49516184d63fc450c50b7" + }, { "version": "v5_0_0", "endpoint": "createUserAuthContextUpdateRequest", From 92bbd30ae2da95e1600da844ba0f03342f7395ea Mon Sep 17 00:00:00 2001 From: simonredfern Date: Mon, 19 Oct 2026 11:06:13 +0100 Subject: [PATCH 25/30] Fix: Make sure that Delete My Api Collection Endpoint By Id only removes an endpoint of the caller's own collection, and only if the endpoint is in the collection in the path --- .../scala/code/api/v4_0_0/Http4s400.scala | 21 ++++++- .../v4_0_0/ApiCollectionEndpointTest.scala | 57 ++++++++++++++++++- .../parity_allowlist.json | 8 +++ 3 files changed, 82 insertions(+), 4 deletions(-) diff --git a/obp-api/src/main/scala/code/api/v4_0_0/Http4s400.scala b/obp-api/src/main/scala/code/api/v4_0_0/Http4s400.scala index ba0ddfd2fd..493f5b7987 100644 --- a/obp-api/src/main/scala/code/api/v4_0_0/Http4s400.scala +++ b/obp-api/src/main/scala/code/api/v4_0_0/Http4s400.scala @@ -7143,9 +7143,23 @@ object Http4s400 { } lazy val deleteMyApiCollectionEndpointById: Http4sRoute = Http4sRoute { - case req @ DELETE -> `prefixPath` / "my" / "api-collection-ids" / _ / "api-collection-endpoint-ids" / apiCollectionEndpointId => - EndpointHelpers.withUserDelete(req) { (_, cc) => + // bind the collection id so the endpoint can be checked to belong to it below + case req @ DELETE -> `prefixPath` / "my" / "api-collection-ids" / apiCollectionId / "api-collection-endpoint-ids" / apiCollectionEndpointId => + // bind the user so the collection's owner can be checked below + EndpointHelpers.withUserDelete(req) { (user, cc) => for { + (ac, _) <- NewStyle.function.getApiCollectionById(apiCollectionId, Some(cc)) + // only the owner removes its endpoints, else any user edits any user's collection; answered as not found so nothing is told about other users' collections + _ <- code.util.Helper.booleanToFuture( + s"$ApiCollectionNotFound Please specify a valid value for API_COLLECTION_ID. Current API_COLLECTION_ID($apiCollectionId) ", 404, Some(cc)) { + ac.userId == user.userId + } + (ace, _) <- NewStyle.function.getApiCollectionEndpointById(apiCollectionEndpointId, Some(cc)) + // the endpoint must be in the collection in the path, else the owner of any collection deletes endpoints of another user's collection + _ <- code.util.Helper.booleanToFuture( + s"$ApiCollectionEndpointNotFound Please specify a valid value for API_COLLECTION_ENDPOINT_ID. Current API_COLLECTION_ENDPOINT_ID($apiCollectionEndpointId) ", 404, Some(cc)) { + ace.apiCollectionId == ac.apiCollectionId + } (deleted, _) <- NewStyle.function.deleteApiCollectionEndpointById(apiCollectionEndpointId, Some(cc)) } yield deleted } @@ -7461,7 +7475,8 @@ object Http4s400 { |""", EmptyBody, Full(true), - List($AuthenticatedUserIsRequired, UserNotFoundByUserId, UnknownError), + // ApiCollectionNotFound: the collection does not exist or is another user's; ApiCollectionEndpointNotFound: the endpoint does not exist or is not in that collection + List($AuthenticatedUserIsRequired, UserNotFoundByUserId, ApiCollectionNotFound, ApiCollectionEndpointNotFound, UnknownError), List(apiTagApiCollection), None, http4sPartialFunction = Some(deleteMyApiCollectionEndpointById) diff --git a/obp-api/src/test/scala/code/api/v4_0_0/ApiCollectionEndpointTest.scala b/obp-api/src/test/scala/code/api/v4_0_0/ApiCollectionEndpointTest.scala index 9c4fc450ca..bdc59762c2 100644 --- a/obp-api/src/test/scala/code/api/v4_0_0/ApiCollectionEndpointTest.scala +++ b/obp-api/src/test/scala/code/api/v4_0_0/ApiCollectionEndpointTest.scala @@ -29,7 +29,7 @@ import org.json4s._ import code.api.ResourceDocs1_4_0.SwaggerDefinitionsJSON import code.api.berlin.group.v1_3.Http4sBGv13Alias import code.api.util.APIUtil.OAuth._ -import code.api.util.ErrorMessages.ApiCollectionNotFound +import code.api.util.ErrorMessages.{ApiCollectionEndpointNotFound, ApiCollectionNotFound} import com.openbankproject.commons.model.ErrorMessage import code.api.v4_0_0.Http4s400.Implementations4_0_0 import com.github.dwickern.macros.NameOf.nameOf @@ -55,6 +55,7 @@ class ApiCollectionEndpointTest extends V400ServerSetup { object ApiEndpoint6 extends Tag(nameOf(Implementations4_0_0.createMyApiCollectionEndpointById)) object ApiEndpoint7 extends Tag(nameOf(Implementations4_0_0.getMyApiCollectionEndpointsById)) object ApiEndpoint8 extends Tag(nameOf(Implementations4_0_0.deleteMyApiCollectionEndpointByOperationId)) + object ApiEndpoint9 extends Tag(nameOf(Implementations4_0_0.deleteMyApiCollectionEndpointById)) feature("Test the apiCollection endpoints") { scenario("We create the apiCollection Endpoint", ApiEndpoint1,ApiEndpoint2, ApiEndpoint3, ApiEndpoint4, VersionOfApi) { @@ -335,6 +336,60 @@ class ApiCollectionEndpointTest extends V400ServerSetup { val requestGet = (v4_0_0_Request / "my" / "api-collection-ids" / apiCollectionId / "api-collection-endpoints").GET <@ (user1) makeGetRequest(requestGet).body.extract[ApiCollectionEndpointsJson400].api_collection_endpoints.length should be (1) } + + // another user must not remove endpoints from user1's collection by the collection endpoint id + scenario(s"We try $ApiEndpoint9 on another user's collection", ApiEndpoint9, VersionOfApi) { + When("user1 creates an apiCollection with one endpoint") + val request = (v4_0_0_Request / "my" / "api-collections").POST <@ (user1) + val response = makePostRequest(request, write(SwaggerDefinitionsJSON.postApiCollectionJson400.copy(api_collection_name = "collection-of-user1-to-delete-from-by-id"))) + response.code should equal(201) + val apiCollectionId = response.body.extract[ApiCollectionJson400].api_collection_id + val requestAdd = (v4_0_0_Request / "my" / "api-collection-ids" / apiCollectionId / "api-collection-endpoints").POST <@ (user1) + val responseAdd = makePostRequest(requestAdd, write(SwaggerDefinitionsJSON.postApiCollectionEndpointJson400)) + responseAdd.code should equal(201) + val apiCollectionEndpointId = responseAdd.body.extract[ApiCollectionEndpointJson400].api_collection_endpoint_id + + Then("user2 deletes that endpoint from it") + val requestDelete = (v4_0_0_Request / "my" / "api-collection-ids" / apiCollectionId / "api-collection-endpoint-ids" / apiCollectionEndpointId).DELETE <@ (user2) + val responseDelete = makeDeleteRequest(requestDelete) + Then("We should get a 404") + responseDelete.code should equal(404) + responseDelete.body.extract[ErrorMessage].message should startWith(ApiCollectionNotFound) + + And("the endpoint is still in user1's collection") + val requestGet = (v4_0_0_Request / "my" / "api-collection-ids" / apiCollectionId / "api-collection-endpoints").GET <@ (user1) + makeGetRequest(requestGet).body.extract[ApiCollectionEndpointsJson400].api_collection_endpoints.length should be (1) + } + + // a user must not remove an endpoint of user1's collection by naming their own collection in the path + scenario(s"We try $ApiEndpoint9 with an endpoint of another collection", ApiEndpoint9, VersionOfApi) { + When("user1 creates an apiCollection with one endpoint") + val request = (v4_0_0_Request / "my" / "api-collections").POST <@ (user1) + val response = makePostRequest(request, write(SwaggerDefinitionsJSON.postApiCollectionJson400.copy(api_collection_name = "collection-of-user1-endpoint-elsewhere"))) + response.code should equal(201) + val apiCollectionId = response.body.extract[ApiCollectionJson400].api_collection_id + val requestAdd = (v4_0_0_Request / "my" / "api-collection-ids" / apiCollectionId / "api-collection-endpoints").POST <@ (user1) + val responseAdd = makePostRequest(requestAdd, write(SwaggerDefinitionsJSON.postApiCollectionEndpointJson400)) + responseAdd.code should equal(201) + val apiCollectionEndpointId = responseAdd.body.extract[ApiCollectionEndpointJson400].api_collection_endpoint_id + + And("user2 creates a collection of their own") + val request2 = (v4_0_0_Request / "my" / "api-collections").POST <@ (user2) + val response2 = makePostRequest(request2, write(SwaggerDefinitionsJSON.postApiCollectionJson400.copy(api_collection_name = "collection-of-user2"))) + response2.code should equal(201) + val otherApiCollectionId = response2.body.extract[ApiCollectionJson400].api_collection_id + + Then("user2 deletes user1's endpoint through their own collection") + val requestDelete = (v4_0_0_Request / "my" / "api-collection-ids" / otherApiCollectionId / "api-collection-endpoint-ids" / apiCollectionEndpointId).DELETE <@ (user2) + val responseDelete = makeDeleteRequest(requestDelete) + Then("We should get a 404") + responseDelete.code should equal(404) + responseDelete.body.extract[ErrorMessage].message should startWith(ApiCollectionEndpointNotFound) + + And("the endpoint is still in user1's collection") + val requestGet = (v4_0_0_Request / "my" / "api-collection-ids" / apiCollectionId / "api-collection-endpoints").GET <@ (user1) + makeGetRequest(requestGet).body.extract[ApiCollectionEndpointsJson400].api_collection_endpoints.length should be (1) + } } } diff --git a/scripts/resource_doc_baseline/parity_allowlist.json b/scripts/resource_doc_baseline/parity_allowlist.json index 5a2a3161c7..d32be43082 100644 --- a/scripts/resource_doc_baseline/parity_allowlist.json +++ b/scripts/resource_doc_baseline/parity_allowlist.json @@ -155,6 +155,14 @@ "lift_digest": "d70bf075f3595bea6dcb3581b21a21b49717cc52e5b327a7978e126132667fb4", "http4s_digest": "fc6bd7923a89033d884757bbb99a57bedd121250f68876e3ffe24e32b6eda4ce" }, + { + "version": "v4_0_0", + "endpoint": "deleteMyApiCollectionEndpointById", + "field": "errorResponseBodies", + "reason": "Added ApiCollectionNotFound and ApiCollectionEndpointNotFound: the collection must be the caller's and the endpoint must be in it, else any user deletes endpoints from another user's collection", + "lift_digest": "d70bf075f3595bea6dcb3581b21a21b49717cc52e5b327a7978e126132667fb4", + "http4s_digest": "b5eb1f8977a5966b9b435c6a52b30ca7a7eed51efaaaa6bab5b63b213c03f931" + }, { "version": "v4_0_0", "endpoint": "updateAtmAccessibilityFeatures", From 48b4f8eff88ebd5fab7a5c96d1e752feffac60fb Mon Sep 17 00:00:00 2001 From: simonredfern Date: Mon, 19 Oct 2026 11:19:48 +0100 Subject: [PATCH 26/30] Fix: Make sure that Create Fx writes the FX rate at the bank in the path, where the Role is checked, not at the bank_id in the body --- .../scala/code/api/v2_2_0/Http4s220.scala | 3 ++- .../code/api/v2_2_0/ExchangeRateTest.scala | 21 +++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/obp-api/src/main/scala/code/api/v2_2_0/Http4s220.scala b/obp-api/src/main/scala/code/api/v2_2_0/Http4s220.scala index fd1e2db293..6c8d85756f 100644 --- a/obp-api/src/main/scala/code/api/v2_2_0/Http4s220.scala +++ b/obp-api/src/main/scala/code/api/v2_2_0/Http4s220.scala @@ -640,7 +640,8 @@ object Http4s220 { _ <- NewStyle.function.isValidCurrencyISOCode(body.from_currency_code, Some(cc)) _ <- NewStyle.function.isValidCurrencyISOCode(body.to_currency_code, Some(cc)) (fxRate, _) <- NewStyle.function.createOrUpdateFXRate( - bankId = body.bank_id, + // the rate is written at the bank in the path, where the Role was checked, not at the body's bank_id, else CanCreateFxRate at one bank writes any bank's rates + bankId = bank.bankId.value, fromCurrencyCode = body.from_currency_code, toCurrencyCode = body.to_currency_code, conversionValue = body.conversion_value, diff --git a/obp-api/src/test/scala/code/api/v2_2_0/ExchangeRateTest.scala b/obp-api/src/test/scala/code/api/v2_2_0/ExchangeRateTest.scala index e1482eca92..348e7ff980 100644 --- a/obp-api/src/test/scala/code/api/v2_2_0/ExchangeRateTest.scala +++ b/obp-api/src/test/scala/code/api/v2_2_0/ExchangeRateTest.scala @@ -117,5 +117,26 @@ class ExchangeRateTest extends V220ServerSetup with DefaultUsers { responseGet.code should equal(200) (responseGet.body \ "conversion_value").extract[Double] should equal(1.5) } + + // CanCreateFxRate at the path bank must not write another bank's rate through the body's bank_id + scenario("Create Fx writes the rate at the bank in the path, not at the body's bank_id", VersionOfApi, ApiEndpoint2) { + val pathBank = testBankId1 + val otherBank = testBankId2 + Entitlement.entitlement.vend.addEntitlement(pathBank.value, resourceUser1.userId, ApiRole.canCreateFxRate.toString()) + + When("We create an FX rate at the path bank, naming another bank in the body") + val body = + s"""{"bank_id":"${otherBank.value}","from_currency_code":"GBP","to_currency_code":"JPY", + |"conversion_value":190.0,"inverse_conversion_value":0.005263157894736842,"effective_date":"2026-10-06T00:00:00Z"}""".stripMargin + val responsePut = makePutRequest((v2_2Request / "banks" / pathBank.value / "fx").PUT <@ (user1), body) + Then("We should get a 201 for the path bank, and no rate at the other bank") + responsePut.code should equal(201) + (responsePut.body \ "bank_id").extract[String] should equal(pathBank.value) + code.fx.MappedFXRate.find( + net.liftweb.mapper.By(code.fx.MappedFXRate.mBankId, otherBank.value), + net.liftweb.mapper.By(code.fx.MappedFXRate.mFromCurrencyCode, "GBP"), + net.liftweb.mapper.By(code.fx.MappedFXRate.mToCurrencyCode, "JPY") + ).isDefined should equal(false) + } } } From 94c715caa3c6f86d06bb12148af571a7293cda98 Mon Sep 17 00:00:00 2001 From: simonredfern Date: Mon, 19 Oct 2026 11:34:08 +0100 Subject: [PATCH 27/30] Fix: Make sure that Create Customer Address only adds an address to a customer at the bank in the path --- .../scala/code/api/v3_1_0/Http4s310.scala | 13 ++++++++++-- .../code/api/v3_1_0/CustomerAddressTest.scala | 20 +++++++++++++++++++ .../parity_allowlist.json | 8 ++++++++ 3 files changed, 39 insertions(+), 2 deletions(-) diff --git a/obp-api/src/main/scala/code/api/v3_1_0/Http4s310.scala b/obp-api/src/main/scala/code/api/v3_1_0/Http4s310.scala index 431faf9741..ca23c6e669 100644 --- a/obp-api/src/main/scala/code/api/v3_1_0/Http4s310.scala +++ b/obp-api/src/main/scala/code/api/v3_1_0/Http4s310.scala @@ -2293,7 +2293,14 @@ object Http4s310 { EndpointHelpers.withUserAndBankAndBodyCreated[PostCustomerAddressJsonV310, Any](req) { (user, bank, postedData, cc) => for { _ <- NewStyle.function.hasEntitlement(bank.bankId.value, user.userId, canCreateCustomerAddress, Some(cc)) - (_, _) <- NewStyle.function.getCustomerByCustomerId(customerIdStr, Some(cc)) + // Keep the customer: its bank is checked next + (customer, _) <- NewStyle.function.getCustomerByCustomerId(customerIdStr, Some(cc)) + // The customer must be at the bank in the path, where the Role is held, else the Role at one bank + // would let the caller add addresses to customers at any bank + // 404, as for a customer that does not exist, so nothing is told about other banks' customers + _ <- code.util.Helper.booleanToFuture(CustomerNotFoundByCustomerId, 404, Some(cc)) { + customer.bankId == bank.bankId.value + } (address, _) <- NewStyle.function.createCustomerAddress( customerIdStr, postedData.line_1, postedData.line_2, postedData.line_3, @@ -2320,7 +2327,9 @@ object Http4s310 { |""", postCustomerAddressJsonV310, customerAddressJsonV310, - List(AuthenticatedUserIsRequired, UserHasMissingRoles, InvalidJsonFormat, UnknownError), + // CustomerNotFoundByCustomerId: the customer does not exist, or is not at BANK_ID (the guard above) + List(AuthenticatedUserIsRequired, UserHasMissingRoles, InvalidJsonFormat, CustomerNotFoundByCustomerId, + UnknownError), List(apiTagCustomer), Some(List(canCreateCustomerAddress)), http4sPartialFunction = Some(createCustomerAddress) diff --git a/obp-api/src/test/scala/code/api/v3_1_0/CustomerAddressTest.scala b/obp-api/src/test/scala/code/api/v3_1_0/CustomerAddressTest.scala index fd973e8579..ff726ba69d 100644 --- a/obp-api/src/test/scala/code/api/v3_1_0/CustomerAddressTest.scala +++ b/obp-api/src/test/scala/code/api/v3_1_0/CustomerAddressTest.scala @@ -233,5 +233,25 @@ class CustomerAddressTest extends V310ServerSetup { } } + // The Create endpoint adds an address only to a customer at the bank in the path + feature("Create Customer Address v3.1.0 stays within the bank in the path") { + scenario("A customer at another bank than the one in the path is not found", ApiEndpoint1, VersionOfApi) { + val bankA = testBankId1.value + val bankB = testBankId2.value + Entitlement.entitlement.vend.addEntitlement(bankA, resourceUser1.userId, CanCreateCustomerAddress.toString) + Entitlement.entitlement.vend.addEntitlement(bankB, resourceUser1.userId, CanCreateCustomer.toString) + val customer = makePostRequest((v3_1_0_Request / "banks" / bankB / "customers").POST <@(user1), write(postCustomerJson)) + customer.code should equal(201) + val customerAtB = customer.body.extract[CustomerJsonV310].customer_id + When("We add an address to bank B's customer through bank A's path") + val request310 = (v3_1_0_Request / "banks" / bankA / "customers" / customerAtB / "address").POST <@(user1) + val response310 = makePostRequest(request310, write(postCustomerAddressJson)) + Then("We should get a 404") + response310.code should equal(404) + And("error should be " + CustomerNotFoundByCustomerId) + response310.body.extract[ErrorMessage].message should startWith (CustomerNotFoundByCustomerId) + } + } + } diff --git a/scripts/resource_doc_baseline/parity_allowlist.json b/scripts/resource_doc_baseline/parity_allowlist.json index d32be43082..fd51afc5d1 100644 --- a/scripts/resource_doc_baseline/parity_allowlist.json +++ b/scripts/resource_doc_baseline/parity_allowlist.json @@ -2363,6 +2363,14 @@ "lift_digest": "647479a642a49ee3a80a4bfc28ab2ae3413796d26a3eecbc48411d6e05620ad5", "http4s_digest": "3a6402f32d558c47caf71d2eda8cf531d4e6a5228039bb567f09d57d66441cfa" }, + { + "version": "v3_1_0", + "endpoint": "createCustomerAddress", + "field": "errorResponseBodies", + "reason": "Added CustomerNotFoundByCustomerId: the customer must be at BANK_ID, else a Role at one bank would add addresses to customers at any bank", + "lift_digest": "647479a642a49ee3a80a4bfc28ab2ae3413796d26a3eecbc48411d6e05620ad5", + "http4s_digest": "3e88e7991ab67e6003f7b61db2d636c2d235017e02312ef328559958dc8dd1c3" + }, { "version": "v3_1_0", "endpoint": "updateAccountAttribute", From bfee1b0cbc812eb43a47cc1dc75df154e6ad2448 Mon Sep 17 00:00:00 2001 From: simonredfern Date: Mon, 19 Oct 2026 11:58:15 +0100 Subject: [PATCH 28/30] Fix: Make sure that Delete Product Attribute only deletes an attribute of the product in the path, at the bank in the path --- .../scala/code/api/v3_1_0/Http4s310.scala | 12 ++++++- .../api/v3_1_0/ProductAttributeTest.scala | 36 +++++++++++++++++++ .../parity_allowlist.json | 8 +++++ 3 files changed, 55 insertions(+), 1 deletion(-) diff --git a/obp-api/src/main/scala/code/api/v3_1_0/Http4s310.scala b/obp-api/src/main/scala/code/api/v3_1_0/Http4s310.scala index ca23c6e669..59370f431f 100644 --- a/obp-api/src/main/scala/code/api/v3_1_0/Http4s310.scala +++ b/obp-api/src/main/scala/code/api/v3_1_0/Http4s310.scala @@ -2014,11 +2014,19 @@ object Http4s310 { // Note: this DELETE returns 204 (matches original v3.1.0 behavior). val deleteProductAttribute: Http4sRoute = Http4sRoute { - case req @ DELETE -> `prefixPath` / "banks" / bankIdStr / "products" / _ / "attributes" / productAttributeIdStr => + // productCodeStr is read so the attribute can be checked against the product in the path + case req @ DELETE -> `prefixPath` / "banks" / bankIdStr / "products" / productCodeStr / "attributes" / productAttributeIdStr => EndpointHelpers.withUserDelete(req) { (user, cc) => for { _ <- NewStyle.function.hasEntitlement(bankIdStr, user.userId, canDeleteProductAttribute, Some(cc)) (_, _) <- NewStyle.function.getBank(BankId(bankIdStr), Some(cc)) + // load the attribute so it can be checked against the path + (existingAttribute, _) <- NewStyle.function.getProductAttributeById(productAttributeIdStr, Some(cc)) + // the attribute must belong to the bank and product in the path, else a role at one bank deletes any bank's attribute + _ <- code.util.Helper.booleanToFuture(ProductAttributeNotFound, 404, Some(cc)) { + existingAttribute.bankId.value == bankIdStr && + existingAttribute.productCode.value == productCodeStr + } (deleted, _) <- NewStyle.function.deleteProductAttribute(productAttributeIdStr, Some(cc)) } yield deleted } @@ -2044,6 +2052,8 @@ object Http4s310 { List( UserHasMissingRoles, BankNotFound, + // the attribute is not at the bank and product in the path + ProductAttributeNotFound, UnknownError ), List(apiTagProduct, apiTagProductAttribute, apiTagAttribute), diff --git a/obp-api/src/test/scala/code/api/v3_1_0/ProductAttributeTest.scala b/obp-api/src/test/scala/code/api/v3_1_0/ProductAttributeTest.scala index 196cd7f1ef..075fc143b6 100644 --- a/obp-api/src/test/scala/code/api/v3_1_0/ProductAttributeTest.scala +++ b/obp-api/src/test/scala/code/api/v3_1_0/ProductAttributeTest.scala @@ -270,6 +270,42 @@ class ProductAttributeTest extends V310ServerSetup { errorMessage contains (UserHasMissingRoles) should be (true) errorMessage contains (canDeleteProductAttribute.toString()) should be (true) } + // a role at one bank must not delete a product attribute of another bank or another product + scenario("We will call the Delete endpoint with an attribute of another bank or another product", ApiEndpoint4, VersionOfApi) { + // an attribute owned by another bank, created directly so no role at that bank is needed + val otherBankAttribute = Await.result( + ProductAttributeX.productAttributeProvider.vend.createOrUpdateProductAttribute( + BankId("other-bank-for-product-attribute-delete"), ProductCode("PRODUCT_CODE"), None, + "other bank attribute", ProductAttributeType.STRING, "12345678", None), + Duration.Inf).openOrThrowException("could not create the test attribute") + // an attribute of another product at the bank in the path + val otherProductAttribute = Await.result( + ProductAttributeX.productAttributeProvider.vend.createOrUpdateProductAttribute( + BankId(testBankId), ProductCode("OTHER_PRODUCT_CODE"), None, + "other product attribute", ProductAttributeType.STRING, "12345678", None), + Duration.Inf).openOrThrowException("could not create the test attribute") + Entitlement.entitlement.vend.addEntitlement(testBankId, resourceUser1.userId, CanDeleteProductAttribute.toString) + When("We delete the other bank's attribute through the bank in the path") + val requestOtherBank = (v3_1_0_Request / "banks" / testBankId / "products" / "PRODUCT_CODE" / "attributes" / otherBankAttribute.productAttributeId).DELETE <@(user1) + val responseOtherBank = makeDeleteRequest(requestOtherBank) + Then("We should get a 404") + responseOtherBank.code should equal(404) + responseOtherBank.body.extract[ErrorMessage].message should equal (ProductAttributeNotFound) + And("The attribute is still there") + Await.result( + ProductAttributeX.productAttributeProvider.vend.getProductAttributeById(otherBankAttribute.productAttributeId), + Duration.Inf).isDefined should be (true) + When("We delete an attribute of another product through the product in the path") + val requestOtherProduct = (v3_1_0_Request / "banks" / testBankId / "products" / "PRODUCT_CODE" / "attributes" / otherProductAttribute.productAttributeId).DELETE <@(user1) + val responseOtherProduct = makeDeleteRequest(requestOtherProduct) + Then("We should get a 404") + responseOtherProduct.code should equal(404) + responseOtherProduct.body.extract[ErrorMessage].message should equal (ProductAttributeNotFound) + And("The attribute is still there") + Await.result( + ProductAttributeX.productAttributeProvider.vend.getProductAttributeById(otherProductAttribute.productAttributeId), + Duration.Inf).isDefined should be (true) + } } } diff --git a/scripts/resource_doc_baseline/parity_allowlist.json b/scripts/resource_doc_baseline/parity_allowlist.json index fd51afc5d1..1e97ece815 100644 --- a/scripts/resource_doc_baseline/parity_allowlist.json +++ b/scripts/resource_doc_baseline/parity_allowlist.json @@ -2387,6 +2387,14 @@ "lift_digest": "f0e3fc093d8749570c7520dabdb672bbb7a6f84d2f03fa9c138762e8c976981a", "http4s_digest": "cbff97326a2c683036eca4d241b6bcc82b9a45d7cee49516184d63fc450c50b7" }, + { + "version": "v3_1_0", + "endpoint": "deleteProductAttribute", + "field": "errorResponseBodies", + "reason": "Added ProductAttributeNotFound (404): the attribute to delete must belong to the bank and product in the path, else a role at one bank deleted any bank's attribute", + "lift_digest": "e411ab351074a0055e9e38d197a6c0e27be6eddcc2158d1026e23c6735b9fbd9", + "http4s_digest": "f32444f850ad7c50c97d8f9fa4e9aac93757d7b16e708393903ee64f4e0e7a76" + }, { "version": "v5_0_0", "endpoint": "createUserAuthContextUpdateRequest", From 6a3f9be8399e5ea9721e6058fe526b6d825a4565 Mon Sep 17 00:00:00 2001 From: simonredfern Date: Mon, 19 Oct 2026 12:08:08 +0100 Subject: [PATCH 29/30] Fix: Make sure that Create Tax Residence only adds a tax residence to a customer at the bank in the path --- .../scala/code/api/v3_1_0/Http4s310.scala | 13 ++++++++++-- .../code/api/v3_1_0/TaxResidenceTest.scala | 20 +++++++++++++++++++ .../parity_allowlist.json | 8 ++++++++ 3 files changed, 39 insertions(+), 2 deletions(-) diff --git a/obp-api/src/main/scala/code/api/v3_1_0/Http4s310.scala b/obp-api/src/main/scala/code/api/v3_1_0/Http4s310.scala index 59370f431f..4b11781c54 100644 --- a/obp-api/src/main/scala/code/api/v3_1_0/Http4s310.scala +++ b/obp-api/src/main/scala/code/api/v3_1_0/Http4s310.scala @@ -2269,7 +2269,14 @@ object Http4s310 { EndpointHelpers.withUserAndBankAndBodyCreated[PostTaxResidenceJsonV310, Any](req) { (user, bank, postedData, cc) => for { _ <- NewStyle.function.hasEntitlement(bank.bankId.value, user.userId, canCreateTaxResidence, Some(cc)) - (_, _) <- NewStyle.function.getCustomerByCustomerId(customerIdStr, Some(cc)) + // Keep the customer: its bank is checked next + (customer, _) <- NewStyle.function.getCustomerByCustomerId(customerIdStr, Some(cc)) + // The customer must be at the bank in the path, where the Role is held, else the Role at one bank + // would let the caller add tax residences to customers at any bank + // 404, as for a customer that does not exist, so nothing is told about other banks' customers + _ <- code.util.Helper.booleanToFuture(CustomerNotFoundByCustomerId, 404, Some(cc)) { + customer.bankId == bank.bankId.value + } (taxResidence, _) <- NewStyle.function.createTaxResidence( customerIdStr, postedData.domain, postedData.tax_number, Some(cc)) } yield JSONFactory310.createTaxResidence(taxResidence) @@ -2290,7 +2297,9 @@ object Http4s310 { |""", postTaxResidenceJsonV310, taxResidenceV310, - List(AuthenticatedUserIsRequired, UserHasMissingRoles, InvalidJsonFormat, UnknownError), + // CustomerNotFoundByCustomerId: the customer does not exist, or is not at BANK_ID (the guard above) + List(AuthenticatedUserIsRequired, UserHasMissingRoles, InvalidJsonFormat, CustomerNotFoundByCustomerId, + UnknownError), List(apiTagCustomer, apiTagKyc), Some(List(canCreateTaxResidence)), http4sPartialFunction = Some(createTaxResidence) diff --git a/obp-api/src/test/scala/code/api/v3_1_0/TaxResidenceTest.scala b/obp-api/src/test/scala/code/api/v3_1_0/TaxResidenceTest.scala index 460dfc7306..a73731f587 100644 --- a/obp-api/src/test/scala/code/api/v3_1_0/TaxResidenceTest.scala +++ b/obp-api/src/test/scala/code/api/v3_1_0/TaxResidenceTest.scala @@ -153,6 +153,26 @@ class TaxResidenceTest extends V310ServerSetup { } } + // The Create endpoint adds a tax residence only to a customer at the bank in the path + feature("Create Tax Residence v3.1.0 stays within the bank in the path") { + scenario("A customer at another bank than the one in the path is not found", ApiEndpoint1, VersionOfApi) { + val bankA = testBankId1.value + val bankB = testBankId2.value + Entitlement.entitlement.vend.addEntitlement(bankA, resourceUser1.userId, CanCreateTaxResidence.toString) + Entitlement.entitlement.vend.addEntitlement(bankB, resourceUser1.userId, CanCreateCustomer.toString) + val customer = makePostRequest((v3_1_0_Request / "banks" / bankB / "customers").POST <@(user1), write(postCustomerJson)) + customer.code should equal(201) + val customerAtB = customer.body.extract[CustomerJsonV310].customer_id + When("We add a tax residence to bank B's customer through bank A's path") + val request310 = (v3_1_0_Request / "banks" / bankA / "customers" / customerAtB / "tax-residence").POST <@(user1) + val response310 = makePostRequest(request310, write(postTaxResidenceJson)) + Then("We should get a 404") + response310.code should equal(404) + And("error should be " + CustomerNotFoundByCustomerId) + response310.body.extract[ErrorMessage].message should startWith (CustomerNotFoundByCustomerId) + } + } + feature("Get the Tax Residence of the Customer specified by CUSTOMER_ID v3.1.0 - Authorized access") { scenario("We will call the endpoint without the proper Role " + canGetTaxResidence, ApiEndpoint2, VersionOfApi) { diff --git a/scripts/resource_doc_baseline/parity_allowlist.json b/scripts/resource_doc_baseline/parity_allowlist.json index 1e97ece815..51aac81103 100644 --- a/scripts/resource_doc_baseline/parity_allowlist.json +++ b/scripts/resource_doc_baseline/parity_allowlist.json @@ -2371,6 +2371,14 @@ "lift_digest": "647479a642a49ee3a80a4bfc28ab2ae3413796d26a3eecbc48411d6e05620ad5", "http4s_digest": "3e88e7991ab67e6003f7b61db2d636c2d235017e02312ef328559958dc8dd1c3" }, + { + "version": "v3_1_0", + "endpoint": "createTaxResidence", + "field": "errorResponseBodies", + "reason": "Added CustomerNotFoundByCustomerId: the customer in the path must be at BANK_ID, else 404, so the Role at one bank cannot add tax residences to customers at another bank", + "lift_digest": "647479a642a49ee3a80a4bfc28ab2ae3413796d26a3eecbc48411d6e05620ad5", + "http4s_digest": "3e88e7991ab67e6003f7b61db2d636c2d235017e02312ef328559958dc8dd1c3" + }, { "version": "v3_1_0", "endpoint": "updateAccountAttribute", From d4ce3d138bd98f0f15d905c0bb7300c532f2c116 Mon Sep 17 00:00:00 2001 From: simonredfern Date: Mon, 19 Oct 2026 13:33:15 +0100 Subject: [PATCH 30/30] Fix: Make sure that Delete Tax Residence only deletes a tax residence of the customer in the path, at the bank in the path --- .../scala/code/api/v3_1_0/Http4s310.scala | 18 +++++++- .../code/api/v3_1_0/TaxResidenceTest.scala | 42 +++++++++++++++++++ .../parity_allowlist.json | 8 ++++ 3 files changed, 66 insertions(+), 2 deletions(-) diff --git a/obp-api/src/main/scala/code/api/v3_1_0/Http4s310.scala b/obp-api/src/main/scala/code/api/v3_1_0/Http4s310.scala index 4b11781c54..c048078829 100644 --- a/obp-api/src/main/scala/code/api/v3_1_0/Http4s310.scala +++ b/obp-api/src/main/scala/code/api/v3_1_0/Http4s310.scala @@ -1951,7 +1951,19 @@ object Http4s310 { EndpointHelpers.withUserAndBank(req) { (user, bank, cc) => for { _ <- NewStyle.function.hasEntitlement(bank.bankId.value, user.userId, canDeleteTaxResidence, Some(cc)) - (_, _) <- NewStyle.function.getCustomerByCustomerId(customerIdStr, Some(cc)) + // Keep the customer: its bank is checked next + (customer, _) <- NewStyle.function.getCustomerByCustomerId(customerIdStr, Some(cc)) + // The customer must be at the bank in the path, where the Role is held, else the Role at one bank + // would let the caller delete tax residences of customers at any bank + // 404, as for a customer that does not exist, so nothing is told about other banks' customers + _ <- code.util.Helper.booleanToFuture(CustomerNotFoundByCustomerId, 404, Some(cc)) { + customer.bankId == bank.bankId.value + } + // The tax residence must be one of this customer's, else any tax residence id could be deleted + (taxResidences, _) <- NewStyle.function.getTaxResidences(customerIdStr, Some(cc)) + _ <- code.util.Helper.booleanToFuture(TaxResidenceNotFound, 404, Some(cc)) { + taxResidences.exists(_.taxResidenceId == taxResidenceIdStr) + } (deleted, _) <- NewStyle.function.deleteTaxResidence(taxResidenceIdStr, Some(cc)) } yield Full(deleted) } @@ -1971,7 +1983,9 @@ object Http4s310 { |""", EmptyBody, EmptyBody, - List(AuthenticatedUserIsRequired, UserHasMissingRoles, UnknownError), + // CustomerNotFoundByCustomerId and TaxResidenceNotFound (404): the customer must be at the bank in the path, + // and the tax residence must be the customer's + List(AuthenticatedUserIsRequired, UserHasMissingRoles, CustomerNotFoundByCustomerId, TaxResidenceNotFound, UnknownError), List(apiTagCustomer, apiTagKyc), Some(List(canDeleteTaxResidence)), http4sPartialFunction = Some(deleteTaxResidence) diff --git a/obp-api/src/test/scala/code/api/v3_1_0/TaxResidenceTest.scala b/obp-api/src/test/scala/code/api/v3_1_0/TaxResidenceTest.scala index a73731f587..3c3ab1e520 100644 --- a/obp-api/src/test/scala/code/api/v3_1_0/TaxResidenceTest.scala +++ b/obp-api/src/test/scala/code/api/v3_1_0/TaxResidenceTest.scala @@ -223,5 +223,47 @@ class TaxResidenceTest extends V310ServerSetup { } } + // The Delete endpoint deletes only a tax residence of the customer in the path, at the bank in the path + feature("Delete Tax Residence v3.1.0 stays within the bank and customer in the path") { + // Creates a customer at the bank with a tax residence, and returns (customer id, tax residence id) + def customerWithTaxResidence(bank: String): (String, String) = { + Entitlement.entitlement.vend.addEntitlement(bank, resourceUser1.userId, CanCreateCustomer.toString) + Entitlement.entitlement.vend.addEntitlement(bank, resourceUser1.userId, CanCreateTaxResidence.toString) + val customer = makePostRequest((v3_1_0_Request / "banks" / bank / "customers").POST <@(user1), write(postCustomerJson)) + customer.code should equal(201) + val customerId = customer.body.extract[CustomerJsonV310].customer_id + val taxResidence = makePostRequest((v3_1_0_Request / "banks" / bank / "customers" / customerId / "tax-residence").POST <@(user1), write(postTaxResidenceJson)) + taxResidence.code should equal(201) + (customerId, taxResidence.body.extract[TaxResidenceV310].tax_residence_id) + } + scenario("A customer at another bank than the one in the path is not found", ApiEndpoint3, VersionOfApi) { + val bankA = testBankId1.value + val bankB = testBankId2.value + val (customerAtB, taxResidenceAtB) = customerWithTaxResidence(bankB) + Entitlement.entitlement.vend.addEntitlement(bankA, resourceUser1.userId, CanDeleteTaxResidence.toString) + When("We delete bank B's customer's tax residence through bank A's path") + val request310 = (v3_1_0_Request / "banks" / bankA / "customers" / customerAtB / "tax_residencies" / taxResidenceAtB).DELETE <@(user1) + val response310 = makeDeleteRequest(request310) + Then("We should get a 404") + response310.code should equal(404) + And("error should be " + CustomerNotFoundByCustomerId) + response310.body.extract[ErrorMessage].message should startWith (CustomerNotFoundByCustomerId) + } + scenario("A tax residence of another customer than the one in the path is not found", ApiEndpoint3, VersionOfApi) { + val bankA = testBankId1.value + val bankB = testBankId2.value + val (customerAtA, _) = customerWithTaxResidence(bankA) + val (_, taxResidenceAtB) = customerWithTaxResidence(bankB) + Entitlement.entitlement.vend.addEntitlement(bankA, resourceUser1.userId, CanDeleteTaxResidence.toString) + When("We delete bank B's tax residence through bank A's customer") + val request310 = (v3_1_0_Request / "banks" / bankA / "customers" / customerAtA / "tax_residencies" / taxResidenceAtB).DELETE <@(user1) + val response310 = makeDeleteRequest(request310) + Then("We should get a 404") + response310.code should equal(404) + And("error should be " + TaxResidenceNotFound) + response310.body.extract[ErrorMessage].message should startWith (TaxResidenceNotFound) + } + } + } diff --git a/scripts/resource_doc_baseline/parity_allowlist.json b/scripts/resource_doc_baseline/parity_allowlist.json index 51aac81103..2476f76c2e 100644 --- a/scripts/resource_doc_baseline/parity_allowlist.json +++ b/scripts/resource_doc_baseline/parity_allowlist.json @@ -2403,6 +2403,14 @@ "lift_digest": "e411ab351074a0055e9e38d197a6c0e27be6eddcc2158d1026e23c6735b9fbd9", "http4s_digest": "f32444f850ad7c50c97d8f9fa4e9aac93757d7b16e708393903ee64f4e0e7a76" }, + { + "version": "v3_1_0", + "endpoint": "deleteTaxResidence", + "field": "errorResponseBodies", + "reason": "Added CustomerNotFoundByCustomerId and TaxResidenceNotFound (404): the customer must be at the bank in the path and the tax residence must be the customer's, else a Role at one bank deletes any bank's tax residences", + "lift_digest": "f06cd9a2114625e9b6a37b8e57fa91e7bdb6586b9899fcd9265713aed3e873a4", + "http4s_digest": "82d851ecfb266d07ca6bcd51e2de6fe66f14941401e4dabdf3a0d2617607b6f1" + }, { "version": "v5_0_0", "endpoint": "createUserAuthContextUpdateRequest",