feat: delete consent challenges once they can no longer be used

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.
    *

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.

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)

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")

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. 

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,

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)
+    }
+  }
 }