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 0b196fa554..69e4e36505 100644 --- a/obp-api/src/main/scala/code/api/util/ConsentUtil.scala +++ b/obp-api/src/main/scala/code/api/util/ConsentUtil.scala @@ -1048,6 +1048,35 @@ object Consent extends MdcLoggable { consentJwtBox.foreach(releaseVrpMandateArtefacts(consent, _)) } + /** + * The consent statuses after which an SCA challenge raised for the consent can no longer be used: + * a REVOKED consent is re-authenticated with a fresh challenge, and REJECTED and EXPIRED are terminal. + */ + val consentChallengeEndingStatuses: Set[ConsentStatus] = + Set(ConsentStatus.REVOKED, ConsentStatus.REJECTED, ConsentStatus.EXPIRED) + + /** + * Delete the OBP_CONSENT_CHALLENGE rows of a consent once none of them can be used any more: when + * one has been answered correctly, or when the consent reaches one of consentChallengeEndingStatuses. + * + * Nothing reads these rows after the answer -- the attempt counter only matters while a challenge + * is still open -- so the row is not an audit trail, it is a hashed one-time password waiting to + * be swept. Berlin Group consent challenges are left alone (their authorisation endpoints keep + * listing them), and so are transaction request challenges. + * + * Never fails the caller: by the time this runs the answer or the status change has happened, and + * a row left behind is logged rather than turned into an error for something that succeeded. + */ + def removeConsentChallenges(consentId: String): Unit = + code.transactionChallenge.Challenges.ChallengeProvider.vend + .deleteChallengesByConsentId(consentId, com.openbankproject.commons.model.enums.ChallengeType.OBP_CONSENT_CHALLENGE.toString) match { + case Failure(msg, ex, _) => + logger.error( + s"removeConsentChallenges: the challenges of consent $consentId could not be deleted: $msg" + + ex.map(e => s" (${e.getClass.getSimpleName}: ${e.getMessage})").getOrElse("")) + case _ => + } + /** * Release the artefacts a VRP mandate created, when the consent that owns them is revoked. * 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 3bcff4c290..25ab62a618 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 @@ -4590,6 +4590,12 @@ object Http4s510 { _ <- Helper.booleanToFuture(s"$InvalidChallengeAnswer", 403, Some(cc)) { challenge.scaStatus.contains(StrongCustomerAuthenticationStatus.finalised) } + // The answered challenge is spent (validateChallenge lets one correct answer through, + // once), and any other challenge the PSU started on this consent is superseded by it, so + // they are deleted now rather than left in the table for good. Done here and not after + // the consent is AUTHORISED: if the steps below refuse the request, the PSU has to start + // a new challenge either way. + _ <- Future(Consent.removeConsentChallenges(consentId)) // Bind the consented permissions to the PSU-selected accounts, replacing the // (bank_id=null, account_id=null) dead views createUKConsentJWT wrote at consent // creation time. diff --git a/obp-api/src/main/scala/code/consent/MappedConsent.scala b/obp-api/src/main/scala/code/consent/MappedConsent.scala index d6a87b18f3..2ef02ecace 100644 --- a/obp-api/src/main/scala/code/consent/MappedConsent.scala +++ b/obp-api/src/main/scala/code/consent/MappedConsent.scala @@ -61,11 +61,14 @@ object MappedConsentProvider extends ConsentProvider with code.util.Helper.MdcLo MappedConsent.find(By(MappedConsent.mConsentId, consentId)) match { case Full(consent) => Consent.expireAllPreviousValidBerlinGroupConsents(consent, status) - tryo(consent + val updated = tryo(consent .mStatus(status.toString) .mLastActionDate(now) //maybe not right, but for the create we use the `now`, we need to update it later. .saveMe() ) + if (updated.isDefined && Consent.consentChallengeEndingStatuses.contains(status)) + Consent.removeConsentChallenges(consentId) + updated case Empty => Empty ?~! ErrorMessages.ConsentNotFound case Failure(msg, _, _) => @@ -423,6 +426,7 @@ object MappedConsentProvider extends ConsentProvider with code.util.Helper.MdcLo ex.map(e => s" (${e.getClass.getSimpleName}: ${e.getMessage})").getOrElse("")) case _ => } + Consent.removeConsentChallenges(consentId) MappedConsent.find(By(MappedConsent.mConsentId, consentId)) } else Failure(ErrorMessages.ConsentAlreadyRevoked) diff --git a/obp-api/src/main/scala/code/scheduler/ConsentScheduler.scala b/obp-api/src/main/scala/code/scheduler/ConsentScheduler.scala index 82e893139b..be2f84bc4f 100644 --- a/obp-api/src/main/scala/code/scheduler/ConsentScheduler.scala +++ b/obp-api/src/main/scala/code/scheduler/ConsentScheduler.scala @@ -212,7 +212,8 @@ object ConsentScheduler extends MdcLoggable { case Success(_) => logger.debug("|---> Task executed successfully") } } - private def expiredUKConsents(): Unit = { + // private[code] so a test can run one sweep without waiting for the schedule. + private[code] def expiredUKConsents(): Unit = { Try { logger.debug("|---> Checking for expired UK Open Banking consents...") @@ -239,6 +240,9 @@ object ConsentScheduler extends MdcLoggable { if (rows > 0) { // An expired consent must give its granted access back, exactly as a revoked one does. Consent.revokeConsentAccountAccess(consent) + // Only UK consents are authorised through OBP_CONSENT_CHALLENGEs, so this is the one + // expiry sweep that can leave such a challenge behind. + Consent.removeConsentChallenges(consent.consentId) logger.warn(message) } else logger.debug(s"|---> Skipped stale update for UK consent ${consent.id}: status already changed") diff --git a/obp-api/src/main/scala/code/transactionChallenge/ChallengeProvider.scala b/obp-api/src/main/scala/code/transactionChallenge/ChallengeProvider.scala index d88f24cdfb..b22d08684b 100644 --- a/obp-api/src/main/scala/code/transactionChallenge/ChallengeProvider.scala +++ b/obp-api/src/main/scala/code/transactionChallenge/ChallengeProvider.scala @@ -60,6 +60,13 @@ trait ChallengeProvider { def getChallengesByConsentId(consentId: String): Box[List[ChallengeTrait]] def getChallengesByBasketId(basketId: String): Box[List[ChallengeTrait]] + /** + * Deletes the challenges of the given type that were raised for this consent. + * The type is part of the match because a consent's challenges are not all alike: the Berlin Group + * authorisation endpoints keep listing theirs after they are answered. + */ + def deleteChallengesByConsentId(consentId: String, challengeType: String): Box[Boolean] + /** * There is another method: Connector.validateChallengeAnswer, it validates the challenge over CBS. * This method, will validate the answer in OBP side. diff --git a/obp-api/src/main/scala/code/transactionChallenge/MappedChallengeProvider.scala b/obp-api/src/main/scala/code/transactionChallenge/MappedChallengeProvider.scala index c01b6b20b5..9535265ae0 100644 --- a/obp-api/src/main/scala/code/transactionChallenge/MappedChallengeProvider.scala +++ b/obp-api/src/main/scala/code/transactionChallenge/MappedChallengeProvider.scala @@ -102,6 +102,12 @@ object MappedChallengeProvider extends ChallengeProvider { Full(MappedExpectedChallengeAnswer.findAll(By(MappedExpectedChallengeAnswer.ConsentId,consentId))) override def getChallengesByBasketId(basketId: String): Box[List[ChallengeTrait]] = Full(MappedExpectedChallengeAnswer.findAll(By(MappedExpectedChallengeAnswer.BasketId,basketId))) + + override def deleteChallengesByConsentId(consentId: String, challengeType: String): Box[Boolean] = + tryo(MappedExpectedChallengeAnswer.bulkDelete_!!( + By(MappedExpectedChallengeAnswer.ConsentId, consentId), + By(MappedExpectedChallengeAnswer.ChallengeType, challengeType) + )) override def validateChallenge( challengeId: String, diff --git a/obp-api/src/main/scala/deletion/DeleteAccountCascade.scala b/obp-api/src/main/scala/deletion/DeleteAccountCascade.scala index 0a1f1eb81f..efb9cb9264 100644 --- a/obp-api/src/main/scala/deletion/DeleteAccountCascade.scala +++ b/obp-api/src/main/scala/deletion/DeleteAccountCascade.scala @@ -28,11 +28,14 @@ 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.{ConsentJWT, CustomJsonFormats, JwtUtil} import code.api.util.ErrorMessages.CouldNotDeleteCascade import code.bankconnectors.Connector import code.cards.MappedPhysicalCard +import code.consent.Consents import code.counterpartyattribute.CounterpartyAttribute import code.counterpartylimit.CounterpartyLimit import code.entitlement.MappedEntitlement @@ -43,11 +46,14 @@ import code.transactionrequests.MappedTransactionRequest import code.views.system.{AccountAccess, ViewDefinition} import code.webhook.MappedAccountWebhook import com.openbankproject.commons.model.{AccountId, BankId} +import com.openbankproject.commons.util.JsonAliases import deletion.DeletionUtil.databaseAtomicTask import net.liftweb.common.{Box, Empty, Full} import net.liftweb.db.DB -import net.liftweb.mapper.{By, ByList} +import net.liftweb.mapper.{By, ByList, NotBy} import net.liftweb.util.DefaultConnectionIdentifier +import net.liftweb.util.Helpers.tryo +import org.json4s.Formats import scala.collection.immutable.List @@ -57,12 +63,14 @@ object DeleteAccountCascade { // The tasks run in list order: the challenges are found through the account's transaction // requests, so they must go before deleteTransactionRequests and deleteTransactions remove them. val doneTasks = - deleteTransactionRequestChallenges(bankId, accountId) :: + deleteConsentChallenges(bankId, accountId) :: + deleteTransactionRequestChallenges(bankId, accountId) :: deleteTransactionRequests(bankId, accountId) :: deleteDoubleEntryBookTransactions(bankId, accountId) :: deleteTransactions(bankId, accountId) :: deleteEntitlements(bankId, accountId) :: deleteAccountAccess(bankId, accountId) :: + deleteAccountHolders(bankId, accountId) :: deleteCustomViews(bankId, accountId) :: deleteAccountAttributes(bankId, accountId) :: deleteAccountWebhooks(bankId, accountId) :: @@ -145,6 +153,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), @@ -210,6 +226,26 @@ object DeleteAccountCascade { } } + // A consent names its accounts only in the views of its JWT, so the consents that still have + // challenges are read and those whose JWT names this account lose them. The consents are found + // through their challenges rather than through users: a Berlin Group consent awaiting its first + // challenge answer has no PSU bound to it yet. A consent bound to other accounts as well loses its + // challenges too: they record the authorisation of the consent as a whole. + private def deleteConsentChallenges(bankId: BankId, accountId: AccountId): Boolean = { + implicit val formats: Formats = CustomJsonFormats.formats + val consentIds = MappedExpectedChallengeAnswer.findAll(NotBy(MappedExpectedChallengeAnswer.ConsentId, "")) + .map(_.ConsentId.get).distinct + .flatMap(Consents.consentProvider.vend.getConsentByConsentId(_).toList) + .filter { consent => + JwtUtil.getSignedPayloadAsJson(consent.jsonWebToken) + .flatMap(payload => tryo(JsonAliases.parse(payload).extract[ConsentJWT])) + .exists(_.views.exists(view => view.bank_id == bankId.value && view.account_id == accountId.value)) + } + .map(_.consentId) + consentIds.isEmpty || MappedExpectedChallengeAnswer.bulkDelete_!!( + ByList(MappedExpectedChallengeAnswer.ConsentId, consentIds) + ) + } private def deleteTransactionRequestChallenges(bankId: BankId, accountId: AccountId): Boolean = { val transactionRequestIds = MappedTransactionRequest.findAll( By(MappedTransactionRequest.mFrom_BankId, bankId.value), diff --git a/obp-api/src/test/scala/code/api/UKOpenBanking/v4_0_1/UKOpenBankingV401AccountInfoTests.scala b/obp-api/src/test/scala/code/api/UKOpenBanking/v4_0_1/UKOpenBankingV401AccountInfoTests.scala index 96409f8b2e..313497facd 100644 --- a/obp-api/src/test/scala/code/api/UKOpenBanking/v4_0_1/UKOpenBankingV401AccountInfoTests.scala +++ b/obp-api/src/test/scala/code/api/UKOpenBanking/v4_0_1/UKOpenBankingV401AccountInfoTests.scala @@ -33,12 +33,15 @@ import code.api.util.ErrorMessages.{ConsentDoesNotMatchConsumer, ConsentDoesNotM import code.api.util.{CallContext, CertificateUtil, Consent} import code.consent.{ConsentStatus, Consents} import code.model.UserExtended -import code.transactionChallenge.Challenges +import code.scheduler.ConsentScheduler +import code.transactionChallenge.{Challenges, MappedChallengeProvider, MappedExpectedChallengeAnswer} import code.views.Views import com.openbankproject.commons.model.{BankIdAccountId, ErrorMessage, ViewId} import com.openbankproject.commons.model.enums.ChallengeType import com.openbankproject.commons.util.ApiVersion import net.liftweb.common.{Failure, Full} +import net.liftweb.mapper.By +import org.mindrot.jbcrypt.BCrypt import org.json4s._ import org.scalatest.Tag @@ -928,4 +931,128 @@ class UKOpenBankingV401AccountInfoTests extends UKOpenBankingV401ServerSetup { storedConsent(consentId).status should equal(ConsentStatus.AWAITINGAUTHORISATION.toString) } } + + // A consent's OBP_CONSENT_CHALLENGE rows are deleted once none of them can be used any more: after + // a correct answer, and when the consent becomes REVOKED, REJECTED or EXPIRED. A wrong answer keeps + // its row, because the attempt counter on it is what limits the next guesses. Rows of other types + // on the same consent stay: the Berlin Group authorisation endpoints list theirs after answering. + private def consentChallengesOf(consentId: String, challengeType: ChallengeType.Value) = + MappedExpectedChallengeAnswer.findAll( + By(MappedExpectedChallengeAnswer.ConsentId, consentId), + By(MappedExpectedChallengeAnswer.ChallengeType, challengeType.toString)) + private def seedConsentChallenge(consentId: String, challengeType: ChallengeType.Value): String = { + val challengeId = java.util.UUID.randomUUID.toString + val salt = BCrypt.gensalt() + MappedChallengeProvider.saveChallenge( + challengeId = challengeId, + transactionRequestId = "", + salt = salt, + expectedAnswer = BCrypt.hashpw("123", salt).substring(0, 44), + expectedUserId = resourceUser1.userId, + scaMethod = None, + scaStatus = None, + consentId = Some(consentId), + basketId = None, + authenticationMethodId = None, + challengeType = challengeType.toString + ).openOrThrowException(s"challenge for consent $consentId") + challengeId + } + + feature("UKOB v4.0.1 a consent's SCA challenges are deleted once they can no longer be used") { + scenario("a correct answer deletes the consent's challenges, other challenge types stay", UKOpenBankingV401AccountInfo) { + setPropsValues("suggested_default_sca_method" -> "DUMMY") + val consentId = createUKConsent(None, Some(new java.util.Date(System.currentTimeMillis() + 3600000L))) + + val challenge = makePostRequest(scaChallengeRequest(consentId) <@ (user1), "") + challenge.code should equal(200) + val challengeId = (challenge.body \ "challenge_id").extract[String] + // A second challenge the PSU started and never answered, and a Berlin Group one on the same id. + seedConsentChallenge(consentId, ChallengeType.OBP_CONSENT_CHALLENGE) + seedConsentChallenge(consentId, ChallengeType.BERLIN_GROUP_CONSENT_CHALLENGE) + consentChallengesOf(consentId, ChallengeType.OBP_CONSENT_CHALLENGE).size should equal(2) + + When("the PSU answers the challenge correctly") + val authorised = makePostRequest(authoriseRequest(consentId) <@ (user1), + s"""{"account_ids":["$acc"],"challenge_id":"$challengeId","answer":"123"}""") + authorised.code should equal(200) + storedConsent(consentId).status should equal(ConsentStatus.AUTHORISED.toString) + + Then("both OBP consent challenges are gone, and the Berlin Group one is still there") + consentChallengesOf(consentId, ChallengeType.OBP_CONSENT_CHALLENGE) should be(empty) + Challenges.ChallengeProvider.vend.getChallenge(challengeId).isDefined should equal(false) + consentChallengesOf(consentId, ChallengeType.BERLIN_GROUP_CONSENT_CHALLENGE).size should equal(1) + } + scenario("a wrong answer keeps the challenge and its attempt count", UKOpenBankingV401AccountInfo) { + setPropsValues("suggested_default_sca_method" -> "DUMMY") + val consentId = createUKConsent(None, Some(new java.util.Date(System.currentTimeMillis() + 3600000L))) + + val challenge = makePostRequest(scaChallengeRequest(consentId) <@ (user1), "") + challenge.code should equal(200) + val challengeId = (challenge.body \ "challenge_id").extract[String] + + val refused = makePostRequest(authoriseRequest(consentId) <@ (user1), + s"""{"account_ids":["$acc"],"challenge_id":"$challengeId","answer":"999"}""") + refused.code should not equal (200) + + val stored = Challenges.ChallengeProvider.vend.getChallenge(challengeId) + .openOrThrowException(s"challenge $challengeId") + stored.attemptCounter should equal(1) + stored.successful should equal(false) + storedConsent(consentId).status should equal(ConsentStatus.AWAITINGAUTHORISATION.toString) + } + scenario("a correct answer that is then refused still spends the challenge", UKOpenBankingV401AccountInfo) { + setPropsValues("suggested_default_sca_method" -> "DUMMY") + val consentId = createUKConsent(None, Some(new java.util.Date(System.currentTimeMillis() + 3600000L))) + + // user2 does not hold testAccountId1, so the authorisation is refused after the answer is checked. + val challenge = makePostRequest(scaChallengeRequest(consentId) <@ (user2), "") + challenge.code should equal(200) + val challengeId = (challenge.body \ "challenge_id").extract[String] + + val refused = makePostRequest(authoriseRequest(consentId) <@ (user2), + s"""{"account_ids":["$acc"],"challenge_id":"$challengeId","answer":"123"}""") + refused.code should equal(403) + + consentChallengesOf(consentId, ChallengeType.OBP_CONSENT_CHALLENGE) should be(empty) + storedConsent(consentId).status should equal(ConsentStatus.AWAITINGAUTHORISATION.toString) + } + scenario("REJECTED, REVOKED and EXPIRED delete the consent's challenges, AUTHORISED does not", UKOpenBankingV401AccountInfo) { + val expiry = Some(new java.util.Date(System.currentTimeMillis() + 3600000L)) + + val authorised = createUKConsent(None, expiry) + seedConsentChallenge(authorised, ChallengeType.OBP_CONSENT_CHALLENGE) + Consents.consentProvider.vend.updateConsentStatus(authorised, ConsentStatus.AUTHORISED) + consentChallengesOf(authorised, ChallengeType.OBP_CONSENT_CHALLENGE).size should equal(1) + + // REJECTED is what the management endpoint PUT /management/banks/BANK_ID/consents/CONSENT_ID sets. + val rejected = createUKConsent(None, expiry) + seedConsentChallenge(rejected, ChallengeType.OBP_CONSENT_CHALLENGE) + seedConsentChallenge(rejected, ChallengeType.BERLIN_GROUP_CONSENT_CHALLENGE) + Consents.consentProvider.vend.updateConsentStatus(rejected, ConsentStatus.REJECTED) + consentChallengesOf(rejected, ChallengeType.OBP_CONSENT_CHALLENGE) should be(empty) + consentChallengesOf(rejected, ChallengeType.BERLIN_GROUP_CONSENT_CHALLENGE).size should equal(1) + + // Every revoke endpoint, the UK DELETE included, goes through ConsentProvider.revoke. + val revoked = createUKConsent(None, expiry) + seedConsentChallenge(revoked, ChallengeType.OBP_CONSENT_CHALLENGE) + Consents.consentProvider.vend.revoke(revoked).isDefined should equal(true) + consentChallengesOf(revoked, ChallengeType.OBP_CONSENT_CHALLENGE) should be(empty) + + val expired = createUKConsent(None, expiry) + seedConsentChallenge(expired, ChallengeType.OBP_CONSENT_CHALLENGE) + Consents.consentProvider.vend.updateConsentStatus(expired, ConsentStatus.EXPIRED) + consentChallengesOf(expired, ChallengeType.OBP_CONSENT_CHALLENGE) should be(empty) + } + scenario("the UK expiry sweep deletes the challenges of the consents it expires", UKOpenBankingV401AccountInfo) { + val consentId = createUKConsent(Some(resourceUser1), Some(new java.util.Date(System.currentTimeMillis() - 60000L))) + Consents.consentProvider.vend.updateConsentStatus(consentId, ConsentStatus.AUTHORISED) + seedConsentChallenge(consentId, ChallengeType.OBP_CONSENT_CHALLENGE) + + ConsentScheduler.expiredUKConsents() + + storedConsent(consentId).status should equal(ConsentStatus.EXPIRED.toString) + consentChallengesOf(consentId, ChallengeType.OBP_CONSENT_CHALLENGE) should be(empty) + } + } } diff --git a/obp-api/src/test/scala/code/api/berlin/group/v1_3/BerlinGroupConsentFixtures.scala b/obp-api/src/test/scala/code/api/berlin/group/v1_3/BerlinGroupConsentFixtures.scala index 12973f60ca..5aa89419ab 100644 --- a/obp-api/src/test/scala/code/api/berlin/group/v1_3/BerlinGroupConsentFixtures.scala +++ b/obp-api/src/test/scala/code/api/berlin/group/v1_3/BerlinGroupConsentFixtures.scala @@ -66,14 +66,15 @@ trait BerlinGroupConsentFixtures extends BerlinGroupServerSetupV1_3 with Default def getNextMonthDate(): String = LocalDate.now().plusMonths(1).format(DateTimeFormatter.ISO_LOCAL_DATE) - /** One account, addressed by the first IBAN routing in the test data. */ - def bgConsentPostBody(): PostConsentJson = { - val acountRoutingIban = BankAccountRouting + /** One account, addressed by `iban`, or by the first IBAN routing in the test data. */ + def bgConsentPostBody(iban: Option[String] = None): PostConsentJson = { + val address = iban.getOrElse(BankAccountRouting .findAll(By(BankAccountRouting.AccountRoutingScheme, AccountRoutingScheme.IBAN.toString)).head + .accountRouting.address) PostConsentJson( access = ConsentAccessJson( accounts = Option(List(ConsentAccessAccountsJson( - iban = Some(acountRoutingIban.accountRouting.address), + iban = Some(address), bban = None, pan = None, maskedPan = None, @@ -131,8 +132,8 @@ trait BerlinGroupConsentFixtures extends BerlinGroupServerSetupV1_3 with Default * An unclaimed (PSU-less) consent lodged under testConsumer, built straight through the provider * -- the shape POST /consents leaves behind for a client_credentials caller. */ - def createUnclaimedBerlinGroupConsent(): ConsentTrait = { - val postJsonBody = bgConsentPostBody() + def createUnclaimedBerlinGroupConsent(iban: Option[String] = None): ConsentTrait = { + val postJsonBody = bgConsentPostBody(iban) val validUntilDate = BgSpecValidation.getDate(postJsonBody.validUntil) val createdConsent = Consents.consentProvider.vend.createBerlinGroupConsent( diff --git a/obp-api/src/test/scala/code/api/berlin/group/v1_3/DeleteAccountCascadeConsentChallengeTest.scala b/obp-api/src/test/scala/code/api/berlin/group/v1_3/DeleteAccountCascadeConsentChallengeTest.scala new file mode 100644 index 0000000000..3a5252bd41 --- /dev/null +++ b/obp-api/src/test/scala/code/api/berlin/group/v1_3/DeleteAccountCascadeConsentChallengeTest.scala @@ -0,0 +1,97 @@ +/** +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.berlin.group.v1_3 + +import code.api.berlin.group.v1_3.JSONFactory_BERLIN_GROUP_1_3.StartConsentAuthorisationJson +import code.api.util.APIUtil.OAuth._ +import code.api.util.ApiRole +import code.entitlement.Entitlement +import code.model.dataAccess.BankAccountRouting +import code.transactionChallenge.MappedExpectedChallengeAnswer +import com.openbankproject.commons.model.enums.AccountRoutingScheme +import net.liftweb.mapper.By +import org.scalatest.Tag + +/** + * A Berlin Group consent names its accounts when it is lodged, so once its PSU starts the + * authorisation the consent is bound to an account and has a challenge nobody has answered yet. + * Deleting that account with the cascading endpoint must take the challenge along; a challenge of a + * consent bound to another account stays. + */ +class DeleteAccountCascadeConsentChallengeTest extends BerlinGroupConsentFixtures { + + object DeleteAccountCascadeConsentChallenge extends Tag("DeleteAccountCascadeConsentChallenge") + + private def ibanOf(bankId: String, accountId: String): String = + BankAccountRouting.find( + By(BankAccountRouting.BankId, bankId), + By(BankAccountRouting.AccountId, accountId), + By(BankAccountRouting.AccountRoutingScheme, AccountRoutingScheme.IBAN.toString) + ).openOrThrowException(s"no IBAN for $bankId/$accountId").accountRouting.address + + private def challengesOf(consentId: String): Int = + MappedExpectedChallengeAnswer.findAll(By(MappedExpectedChallengeAnswer.ConsentId, consentId)).size + + // The consent is lodged for one account and resourceUser1 starts its authorisation, which binds + // the PSU and mints the challenge; the challenge is left unanswered. + private def consentAwaitingItsChallenge(bankId: String, accountId: String): String = { + val consentId = createUnclaimedBerlinGroupConsent(Some(ibanOf(bankId, accountId))).consentId + val started = makePostRequest( + (V1_3_BG / "consents" / consentId / "authorisations").POST <@ (user1), + """{"scaAuthenticationData":""}""") + started.code should equal(201) + started.body.extract[StartConsentAuthorisationJson].authorisationId should not be empty + consentId + } + + feature("Deleting an account by cascade deletes the challenges of the consents bound to it") { + scenario("A Berlin Group consent whose challenge is still unanswered", DeleteAccountCascadeConsentChallenge) { + setPropsValues("suggested_default_sca_method" -> "DUMMY") + Given("Two accounts resourceUser1 holds, each named by a consent awaiting its challenge") + val held = ibanAddressableAccountsHeldBy(resourceUser1).toList.sorted + held.size should be >= 2 + val (bankId, accountId) = held.head + val (otherBankId, otherAccountId) = held(1) + val consent = consentAwaitingItsChallenge(bankId, accountId) + val otherConsent = consentAwaitingItsChallenge(otherBankId, otherAccountId) + challengesOf(consent) should equal(1) + challengesOf(otherConsent) should equal(1) + + When("The first account is deleted with the cascading endpoint") + Entitlement.entitlement.vend.addEntitlement(bankId, resourceUser1.userId, ApiRole.canDeleteAccountCascade.toString) + val request = (baseRequest / "obp" / "v4.0.0" / "management" / "cascading" / "banks" / bankId / + "accounts" / accountId).DELETE <@ (user1) + makeDeleteRequest(request).code should equal(200) + + Then("The challenge of the consent bound to the deleted account is gone") + challengesOf(consent) should equal(0) + And("The challenge of the consent bound to the other account is kept") + challengesOf(otherConsent) should equal(1) + } + } +} 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..bd470951e0 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 @@ -62,6 +64,8 @@ class DeleteAccountCascadeTest extends V400ServerSetup { object VersionOfApi extends Tag(ApiVersion.v4_0_0.toString) object ApiEndpoint1 extends Tag(nameOf(Implementations4_0_0.deleteAccountCascade)) + private val deleteAtFirstBank = "We delete the account at the first bank with the cascading endpoint" + lazy val bankId = randomBankId lazy val bankAccount = randomPrivateAccountViaEndpoint(bankId) lazy val addAccountJson = SwaggerDefinitionsJSON.createAccountRequestJsonV310.copy(user_id = resourceUser1.userId, balance = AmountOfMoneyJsonV121("EUR","0")) @@ -195,7 +199,7 @@ class DeleteAccountCascadeTest extends V400ServerSetup { val card = addCard(bankId) val otherBankCard = addCard(otherBankId) - When("We delete the account at the first bank with the cascading endpoint") + When(deleteAtFirstBank) 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) @@ -229,7 +233,7 @@ class DeleteAccountCascadeTest extends V400ServerSetup { ) otherOwnerEntitlements should equal(1L) - When("We delete the account at the first bank with the cascading endpoint") + When(deleteAtFirstBank) 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) @@ -243,4 +247,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 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) + 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..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 @@ -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 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) + + Then("The holders of its accounts are deleted") + accountHolders shouldBe empty + } } - + }