From 9d17eab134002fa2eb9f50ac9d0129af4499838c Mon Sep 17 00:00:00 2001 From: Hongwei Date: Tue, 3 Nov 2026 13:28:30 +0100 Subject: [PATCH 1/2] feat: delete account holders in the account and bank cascades DeleteAccountCascade now removes the MapperAccountHolders rows of the deleted bank_id/account_id, and DeleteBankCascade removes them for every account of the bank through it. A holder row that outlived its account was read back by AuthUser.refreshUser (for example when the former holder created another account) and the five system views were granted on the deleted account again. --- .../scala/deletion/DeleteAccountCascade.scala | 10 +++ .../api/v4_0_0/DeleteAccountCascadeTest.scala | 66 +++++++++++++++++++ .../api/v4_0_0/DeleteBankCascadeTest.scala | 22 ++++++- 3 files changed, 97 insertions(+), 1 deletion(-) diff --git a/obp-api/src/main/scala/deletion/DeleteAccountCascade.scala b/obp-api/src/main/scala/deletion/DeleteAccountCascade.scala index 0a1f1eb81f..10fe721b52 100644 --- a/obp-api/src/main/scala/deletion/DeleteAccountCascade.scala +++ b/obp-api/src/main/scala/deletion/DeleteAccountCascade.scala @@ -28,6 +28,7 @@ TESOBE (http://www.tesobe.com/) package deletion import code.accountattribute.MappedAccountAttribute +import code.accountholders.MapperAccountHolders import code.api.APIFailureNewStyle import code.api.util.APIUtil.fullBoxOrException import code.api.util.ErrorMessages.CouldNotDeleteCascade @@ -63,6 +64,7 @@ object DeleteAccountCascade { deleteTransactions(bankId, accountId) :: deleteEntitlements(bankId, accountId) :: deleteAccountAccess(bankId, accountId) :: + deleteAccountHolders(bankId, accountId) :: deleteCustomViews(bankId, accountId) :: deleteAccountAttributes(bankId, accountId) :: deleteAccountWebhooks(bankId, accountId) :: @@ -145,6 +147,14 @@ object DeleteAccountCascade { By(AccountAccess.account_id, accountId.value) ) } + // A holder row that outlives its account makes the next refresh of the holder's accounts + // (AuthUser.refreshUser, run e.g. when the holder creates another account) grant the system views on it again. + private def deleteAccountHolders(bankId: BankId, accountId: AccountId): Boolean = { + MapperAccountHolders.bulkDelete_!!( + By(MapperAccountHolders.accountBankPermalink, bankId.value), + By(MapperAccountHolders.accountPermalink, accountId.value) + ) + } private def deleteAccountRoutings(bankId: BankId, accountId: AccountId): Boolean = { BankAccountRouting.bulkDelete_!!( By(BankAccountRouting.BankId, bankId.value), diff --git a/obp-api/src/test/scala/code/api/v4_0_0/DeleteAccountCascadeTest.scala b/obp-api/src/test/scala/code/api/v4_0_0/DeleteAccountCascadeTest.scala index f9b5b2f8bf..307a00627b 100644 --- a/obp-api/src/test/scala/code/api/v4_0_0/DeleteAccountCascadeTest.scala +++ b/obp-api/src/test/scala/code/api/v4_0_0/DeleteAccountCascadeTest.scala @@ -28,6 +28,7 @@ TESOBE (http://www.tesobe.com/) package code.api.v4_0_0 import org.json4s._ +import code.accountholders.MapperAccountHolders import code.api.Constant import code.api.ResourceDocs1_4_0.SwaggerDefinitionsJSON import code.api.ResourceDocs1_4_0.SwaggerDefinitionsJSON.createViewJsonV300 @@ -40,6 +41,7 @@ import code.api.v4_0_0.Http4s400.Implementations4_0_0 import code.cards.MappedPhysicalCard import code.entitlement.{Entitlement, MappedEntitlement} import code.model.dataAccess.MappedBankAccount +import code.views.system.AccountAccess import com.github.dwickern.macros.NameOf.nameOf import com.openbankproject.commons.model.{AccountId, AmountOfMoneyJsonV121, BankId, ErrorMessage} import com.openbankproject.commons.util.ApiVersion @@ -243,4 +245,68 @@ class DeleteAccountCascadeTest extends V400ServerSetup { } } + private def accountHolders(bankId: String, accountId: String): List[MapperAccountHolders] = + MapperAccountHolders.findAll( + By(MapperAccountHolders.accountBankPermalink, bankId), + By(MapperAccountHolders.accountPermalink, accountId) + ) + + private def accountAccess(bankId: String, accountId: String): List[AccountAccess] = + AccountAccess.findAll( + By(AccountAccess.bank_id, bankId), + By(AccountAccess.account_id, accountId) + ) + + feature(s"test $ApiEndpoint1 version $VersionOfApi - account holders") { + scenario("Deleting an account removes its holders, so a later refresh of the former holder grants no views on it", ApiEndpoint1, VersionOfApi) { + Given("An account created through the API, which makes the user its holder and grants the user its system views") + val bankId = createBank(APIUtil.generateUUID()).bankId.value + val deletedAccountId = createAccountViaEndpoint(bankId, addAccountJson, user1).account_id + accountHolders(bankId, deletedAccountId) should not be empty + accountAccess(bankId, deletedAccountId) should not be empty + + When("We delete the account with the cascading endpoint") + Entitlement.entitlement.vend.addEntitlement(bankId, resourceUser1.userId, ApiRole.canDeleteAccountCascade.toString) + val request = (v4_0_0_Request / "management" / "cascading" / "banks" / bankId / + "accounts" / deletedAccountId).DELETE <@ (user1) + makeDeleteRequest(request).code should equal(200) + + Then("The account's holders and account access are gone") + accountHolders(bankId, deletedAccountId) shouldBe empty + accountAccess(bankId, deletedAccountId) shouldBe empty + + When("The former holder reads the deleted account") + val getRequest = (v5_1_0_Request / "banks" / bankId / "accounts" / deletedAccountId / + Constant.SYSTEM_OWNER_VIEW_ID / "account").GET <@ (user1) + makeGetRequest(getRequest).code should equal(404) + + And("The former holder creates another account, which refreshes the views of every account the user holds") + createAccountViaEndpoint(bankId, addAccountJson, user1) + + Then("No account access is granted on the deleted account again") + accountAccess(bankId, deletedAccountId) shouldBe empty + } + + scenario("Deleting an account keeps the holders of an account with the same ACCOUNT_ID at another bank", ApiEndpoint1, VersionOfApi) { + Given("Two banks that each have an account with the same ACCOUNT_ID, held by the same user") + val bankId = BankId(randomString(10)) + val otherBankId = BankId(randomString(10)) + createBank(bankId.value) + createBank(otherBankId.value) + val accountId = AccountId(randomString(10)) + createAccountRelevantResource(Some(resourceUser1), bankId, accountId, "EUR") + createAccountRelevantResource(Some(resourceUser1), otherBankId, accountId, "EUR") + + When("We delete the account at the first bank with the cascading endpoint") + Entitlement.entitlement.vend.addEntitlement(bankId.value, resourceUser1.userId, ApiRole.canDeleteAccountCascade.toString) + val request = (v4_0_0_Request / "management" / "cascading" / "banks" / bankId.value / + "accounts" / accountId.value).DELETE <@ (user1) + makeDeleteRequest(request).code should equal(200) + + Then("The deleted account's holder is gone and the other bank's holder is kept") + accountHolders(bankId.value, accountId.value) shouldBe empty + accountHolders(otherBankId.value, accountId.value).size should equal(1) + } + } + } diff --git a/obp-api/src/test/scala/code/api/v4_0_0/DeleteBankCascadeTest.scala b/obp-api/src/test/scala/code/api/v4_0_0/DeleteBankCascadeTest.scala index 65b990002d..d89a503ac9 100644 --- a/obp-api/src/test/scala/code/api/v4_0_0/DeleteBankCascadeTest.scala +++ b/obp-api/src/test/scala/code/api/v4_0_0/DeleteBankCascadeTest.scala @@ -28,6 +28,7 @@ TESOBE (http://www.tesobe.com/) package code.api.v4_0_0 import org.json4s._ +import code.accountholders.MapperAccountHolders import code.api.Constant import java.util.concurrent.TimeUnit @@ -43,6 +44,7 @@ import code.entitlement.Entitlement import com.github.dwickern.macros.NameOf.nameOf import com.openbankproject.commons.model.{AmountOfMoneyJsonV121, ErrorMessage} import com.openbankproject.commons.util.ApiVersion +import net.liftweb.mapper.By import org.json4s.native.Serialization.write import org.scalatest.Tag @@ -177,7 +179,25 @@ class DeleteBankCascadeTest extends V400ServerSetup { transactionRequestsAfter shouldBe empty challengesAfter shouldBe empty } + scenario("We delete a bank whose accounts have holders", ApiEndpoint1, VersionOfApi) { + val bankId = createBank(APIUtil.generateUUID()).bankId.value + val accountId = createAccountViaEndpoint(bankId, addAccountJson, user1).account_id + def accountHolders = MapperAccountHolders.findAll( + By(MapperAccountHolders.accountBankPermalink, bankId), + By(MapperAccountHolders.accountPermalink, accountId) + ) + accountHolders should not be empty + + When("We grant the role") + Entitlement.entitlement.vend.addEntitlement(bankId, resourceUser1.userId, ApiRole.canDeleteBankCascade.toString) + And("We make a delete cascade request v4.0.0") + val deleteRequest400 = (v4_0_0_Request / "management" / "cascading" / "banks" / bankId ).DELETE <@(user1) + makeDeleteRequest(deleteRequest400).code should equal(200) + + Then("The holders of its accounts are deleted") + accountHolders shouldBe empty + } } - + } From 5affdc14c3eb52c0bbedf2bde1b54740ea68b798 Mon Sep 17 00:00:00 2001 From: Hongwei Date: Tue, 3 Nov 2026 14:36:05 +0100 Subject: [PATCH 2/2] test: give the new cascade holder steps their own descriptions --- .../test/scala/code/api/v4_0_0/DeleteAccountCascadeTest.scala | 2 +- .../src/test/scala/code/api/v4_0_0/DeleteBankCascadeTest.scala | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/obp-api/src/test/scala/code/api/v4_0_0/DeleteAccountCascadeTest.scala b/obp-api/src/test/scala/code/api/v4_0_0/DeleteAccountCascadeTest.scala index 307a00627b..8d8e41078b 100644 --- a/obp-api/src/test/scala/code/api/v4_0_0/DeleteAccountCascadeTest.scala +++ b/obp-api/src/test/scala/code/api/v4_0_0/DeleteAccountCascadeTest.scala @@ -297,7 +297,7 @@ class DeleteAccountCascadeTest extends V400ServerSetup { createAccountRelevantResource(Some(resourceUser1), bankId, accountId, "EUR") createAccountRelevantResource(Some(resourceUser1), otherBankId, accountId, "EUR") - When("We delete the account at the first bank with the cascading endpoint") + When("We delete the first bank's account, keeping the other bank's one") Entitlement.entitlement.vend.addEntitlement(bankId.value, resourceUser1.userId, ApiRole.canDeleteAccountCascade.toString) val request = (v4_0_0_Request / "management" / "cascading" / "banks" / bankId.value / "accounts" / accountId.value).DELETE <@ (user1) diff --git a/obp-api/src/test/scala/code/api/v4_0_0/DeleteBankCascadeTest.scala b/obp-api/src/test/scala/code/api/v4_0_0/DeleteBankCascadeTest.scala index d89a503ac9..d876cdd017 100644 --- a/obp-api/src/test/scala/code/api/v4_0_0/DeleteBankCascadeTest.scala +++ b/obp-api/src/test/scala/code/api/v4_0_0/DeleteBankCascadeTest.scala @@ -190,7 +190,7 @@ class DeleteBankCascadeTest extends V400ServerSetup { When("We grant the role") Entitlement.entitlement.vend.addEntitlement(bankId, resourceUser1.userId, ApiRole.canDeleteBankCascade.toString) - And("We make a delete cascade request v4.0.0") + And("We delete the bank with the cascading endpoint") val deleteRequest400 = (v4_0_0_Request / "management" / "cascading" / "banks" / bankId ).DELETE <@(user1) makeDeleteRequest(deleteRequest400).code should equal(200)