PP-12546 hold payments in state after completion (#3868)

frontend now holds up to 10 charge objects on the client to allow users
who navigate back to Pay from the service to see the status of their payment

a charge will be removed from the client if 10 charges exist and an 11th payment
is started

the logic for determining which charge to remove is documented in this ADR:
https://github.com/alphagov/pay-architecture/blob/main/adr/026-keep-payments-in-cookie-for-longer.md

- new ChargeState model
- new csrf secret management middleware
  - csrf secret is set at the root of the session object as part of a gradual migration of csrf behaviour
- remove used csrf token store from frontend state
- add new cookie functions for handling charge state
- remove unused session code from server

.secrets.baseline

@@ -372,7 +372,7 @@
         "filename": "test/unit/cookies.test.js",
         "hashed_secret": "ef5c25da3f68da43b33a9bc96de633756beb2d88",
         "is_verified": false,
-        "line_number": 35
+        "line_number": 36
       }
     ],
     "test/utils/normalise.test.js": [
@@ -385,5 +385,5 @@
       }
     ]
   },
-  "generated_at": "2024-07-29T10:56:11Z"
+  "generated_at": "2024-07-30T15:02:23Z"
 }

app/controllers/return.controller.js

@@ -6,15 +6,24 @@ const Charge = require('../models/charge')
 const responseRouter = require('../utils/response-router')
 const { CORRELATION_HEADER } = require('../../config/correlation-header')
 const { withAnalyticsError } = require('../utils/analytics')
-const cookies = require('../utils/cookies')
 const { createChargeIdSessionKey } = require('../utils/session')
+const cookies = require('../utils/cookies')
 
 exports.return = (req, res) => {
   const correlationId = req.headers[CORRELATION_HEADER] || ''
   const charge = Charge(correlationId)
-  // Remove the charge data from the cookie
-  const cookieKey = createChargeIdSessionKey(req.chargeId)
-  cookies.deleteSessionVariable(req, cookieKey)
+  const chargeKey = createChargeIdSessionKey(req.chargeId)
+  const chargeState = cookies.getSessionChargeState(req, chargeKey)
+  if (chargeState) {
+    chargeState.markTerminal()
+    cookies.setSessionChargeState(req, chargeKey, chargeState)
+  } else {
+    /* TODO: remove after PP-12546 has been merged
+    /  we no longer need to delete the session variable in the return controller as the cookie is cleaned up in the
+    /  secure controller, we're just cleaning up any existing payment journeys prior to the merge of PP-12546
+    */
+    cookies.deleteSessionVariable(req, chargeKey)
+  }
   if (charge.isCancellableCharge(req.chargeData.status)) {
     return charge.cancel(req.chargeId, getLoggingFields(req))
       .then(() => logger.warn('Return controller cancelled payment', getLoggingFields(req)))

app/controllers/secure.controller.js

@@ -1,7 +1,7 @@
 'use strict'
 
-// NPM dependencies
 const csrf = require('csrf')
+
 const {
   PAYMENT_EXTERNAL_ID,
   GATEWAY_ACCOUNT_ID,
@@ -16,11 +16,12 @@ const Token = require('../models/token')
 const Charge = require('../models/charge')
 const responseRouter = require('../utils/response-router')
 const { createChargeIdSessionKey } = require('../utils/session')
-const { setSessionVariable, getSessionVariable } = require('../utils/cookies')
+const cookies = require('../utils/cookies')
 const CORRELATION_HEADER = require('../../config/correlation-header').CORRELATION_HEADER
 const withAnalyticsError = require('../utils/analytics').withAnalyticsError
 const { resolveActionName } = require('../services/state.service')
 const paths = require('../paths')
+const { ChargeState } = require('../models/ChargeState')
 
 exports.new = async function (req, res) {
   const chargeTokenId = req.params.chargeTokenId || req.body.chargeTokenId
@@ -36,7 +37,7 @@ exports.new = async function (req, res) {
     setLoggingField(req, GATEWAY_ACCOUNT_TYPE, tokenResponse.charge.gateway_account.type)
 
     if (tokenResponse.used === true) {
-      if (!getSessionVariable(req, createChargeIdSessionKey(chargeId))) {
+      if (!cookies.getSessionVariable(req, createChargeIdSessionKey(chargeId))) {
         throw new Error('UNAUTHORISED')
       }
       logger.info('Payment token being reused', getLoggingFields(req))
@@ -48,7 +49,12 @@ exports.new = async function (req, res) {
     } else {
       logger.info('Payment token used for the first time', getLoggingFields(req))
       await Token.markTokenAsUsed(chargeTokenId, correlationId, getLoggingFields(req))
-      setSessionVariable(req, createChargeIdSessionKey(chargeId), { csrfSecret: csrf().secretSync() })
+      if (cookies.getChargesOnSession(req).length >= 10) {
+        const chargeToDelete = cookies.findSessionChargeToDelete(req)
+        cookies.deleteSessionVariable(req, chargeToDelete)
+      }
+      cookies.setSessionChargeState(req, createChargeIdSessionKey(chargeId), new ChargeState())
+      cookies.setSessionVariable(req, createChargeIdSessionKey(chargeId), { csrfSecret: csrf().secretSync() }) // TODO: remove after PP-12546 has been merged
       res.redirect(303, generateRoute(resolveActionName(chargeStatus, 'get'), { chargeId }))
     }
   } catch (err) {

app/middleware/csrf.js

@@ -8,11 +8,21 @@ const logger = require('../utils/logger')(__filename)
 const { getLoggingFields } = require('../utils/logging-fields-helper')
 const session = require('../utils/session')
 const responseRouter = require('../utils/response-router')
+const cookies = require('../utils/cookies')
+
+exports.csrfSetSecret = (req, _, next) => {
+  const csrfSecret = cookies.getSessionCsrfSecret(req)
+  if (!csrfSecret) {
+    logger.info('Setting CSRF secret for session')
+    cookies.setSessionVariable(req, 'csrfSecret', csrf().secretSync())
+  }
+  next()
+}
 
 exports.csrfTokenGeneration = (req, res, next) => {
   const chargeId = fetchAndValidateChargeId(req)
   const chargeSession = session.retrieve(req, chargeId)
-  res.locals.csrf = csrf().create(chargeSession.csrfSecret)
+  res.locals.csrf = csrf().create(chargeSession.csrfSecret) // TODO: generate tokens using the session secret
   next()
 }
 
@@ -23,22 +33,21 @@ exports.csrfCheck = (req, res, next) => {
     return responseRouter.response(req, res, 'UNAUTHORISED')
   }
 
-  const chargeSession = session.retrieve(req, chargeId) || {}
+  const chargeSession = session.retrieve(req, chargeId) || {} // TODO: remove after PP-12546 has been merged
+  const sessionCsrfSecret = cookies.getSessionCsrfSecret(req)
   const csrfToken = req.body.csrfToken
-  chargeSession.csrfTokens = chargeSession.csrfTokens || []
 
-  if (!chargeSession.csrfSecret) {
+  if (!chargeSession.csrfSecret && !sessionCsrfSecret) {
     responseRouter.response(req, res, 'UNAUTHORISED')
     logger.warn('CSRF secret is not defined', {
       ...getLoggingFields(req),
       referrer: req.get('Referrer'),
       url: req.originalUrl,
       method: req.method
     })
-  } else if (!csrfValid(csrfToken, chargeSession, req)) {
+  } else if (!csrfValid(csrfToken, chargeSession.csrfSecret, req) && !csrfValid(csrfToken, sessionCsrfSecret, req)) {
     responseRouter.systemErrorResponse(req, res, 'CSRF is invalid')
   } else {
-    chargeSession.csrfTokens.push(csrfToken)
     next()
   }
 }
@@ -50,13 +59,13 @@ function fetchAndValidateChargeId (req) {
   return false
 }
 
-function csrfValid (csrfToken, chargeSession, req) {
+function csrfValid (csrfToken, secret, req) {
+  if (!secret) {
+    return false
+  }
   if (!['put', 'post'].includes(req.method.toLowerCase())) {
     return true
-  } else if (chargeSession.csrfTokens.includes(csrfToken)) {
-    logger.warn('CSRF token was already used', getLoggingFields(req))
-    return false
   } else {
-    return csrf().verify(chargeSession.csrfSecret, csrfToken)
+    return csrf().verify(secret, csrfToken)
   }
 }

app/models/ChargeState.js

@@ -0,0 +1,52 @@
+const logger = require('../utils/logger')(__filename)
+
+class ChargeState {
+  /**
+   * @param {Number} [createdAt]
+   * @param {Number} [accessedAt]
+   * @param {boolean} [isTerminal]
+   */
+  constructor (createdAt, accessedAt, isTerminal) {
+    this.createdAt = createdAt || epochSecondsNow()
+    this.accessedAt = accessedAt || epochSecondsNow()
+    this.isTerminal = isTerminal || false
+  }
+
+  updateAccessedAt () {
+    this.accessedAt = epochSecondsNow()
+  }
+
+  markTerminal () {
+    this.updateAccessedAt()
+    this.isTerminal = true
+  }
+
+  toString () {
+    return `${this.createdAt},${this.accessedAt},${this.isTerminal ? 'T' : 'F'}`
+  }
+}
+
+const chargeStateFromString = (data) => {
+  try {
+    const dataParts = data.split(',')
+    if (dataParts.length === 3) {
+      return new ChargeState(Number(dataParts[0]), Number(dataParts[1]), dataParts[2] === 'T')
+    } else {
+      logger.error('argument is not a valid ChargeState')
+      return null
+    }
+  } catch (e) {
+    logger.warn(`Error de-serialising ChargeState from string: ${e.message}`)
+    return null
+  }
+}
+
+const epochSecondsNow = () => {
+  return Math.floor(Date.now() / 1000)
+}
+
+module.exports = {
+  ChargeState,
+  chargeStateFromString,
+  epochSecondsNow
+}

app/routes.js

@@ -14,7 +14,7 @@ const { log } = require('./controllers/client-side-logging.controller')
 const paths = require('./paths.js')
 
 // Express middleware
-const { csrfCheck, csrfTokenGeneration } = require('./middleware/csrf.js')
+const { csrfSetSecret, csrfCheck, csrfTokenGeneration } = require('./middleware/csrf.js')
 const actionName = require('./middleware/action-name.js')
 const stateEnforcer = require('./middleware/state-enforcer.js')
 const retrieveCharge = require('./middleware/retrieve-charge.js')
@@ -43,6 +43,7 @@ exports.bind = function (app) {
   const card = paths.card
 
   const standardMiddlewareStack = [
+    csrfSetSecret,
     csrfCheck,
     csrfTokenGeneration,
     actionName,

app/services/clients/connector.client.js

@@ -74,7 +74,7 @@ async function _putConnector (url, payload, description, loggingFields = {}, cal
   requestLogger.logRequestStart(context, loggingFields)
   try {
     configureClient(client, url)
-    const response = await client.put(url, payload, description )
+    const response = await client.put(url, payload, description)
     logger.info('PUT to %s ended - total time %dms', url, new Date() - startTime, loggingFields)
     incrementStatusCodeCounter(callingFunctionName, response.status)
     if (response.status > 499 && response.status < 600) {

app/utils/cookies.js

@@ -1,6 +1,7 @@
 'use strict'
 
 const clientSessions = require('client-sessions')
+const { chargeStateFromString, epochSecondsNow } = require('../models/ChargeState')
 const logger = require('../utils/logger')(__filename)
 
 // Constants
@@ -17,7 +18,11 @@ module.exports = {
   namedCookie,
   deleteSessionVariable,
   isSessionPresent,
-  getSessionVariableNames
+  getChargesOnSession,
+  getSessionCsrfSecret,
+  getSessionChargeState,
+  setSessionChargeState,
+  findSessionChargeToDelete
 }
 
 /**
@@ -133,6 +138,49 @@ function deleteSessionVariable (req, key) {
   }
 }
 
+/**
+ * @param {Request} req
+ * @returns {string}
+ */
+function findSessionChargeToDelete (req) {
+  const now = epochSecondsNow()
+  const yesterday = now - 86400
+  const ninetyMinutesAgo = now - 5400
+
+  const chargesSortedByAccessedDate = getChargesOnSession(req)
+    .map(chargeId => {
+      return {
+        chargeId,
+        state: getSessionChargeState(req, chargeId)
+      }
+    })
+    .sort(({ state: a }, { state: b }) => a.accessedAt - b.accessedAt)
+
+  logger.info(`User session contains ${chargesSortedByAccessedDate.length} charges`)
+
+  const leastRecentlyAccessedChargeCreatedMoreThanTwentyFourHoursAgo = chargesSortedByAccessedDate.find(charge => charge.state.createdAt < yesterday)
+
+  if (leastRecentlyAccessedChargeCreatedMoreThanTwentyFourHoursAgo) {
+    return leastRecentlyAccessedChargeCreatedMoreThanTwentyFourHoursAgo.chargeId
+  }
+
+  const leastRecentlyAccessedTerminalCharge = chargesSortedByAccessedDate.find(charge => charge.state.isTerminal)
+
+  if (leastRecentlyAccessedTerminalCharge) {
+    return leastRecentlyAccessedTerminalCharge.chargeId
+  }
+
+  const leastRecentlyAccessedChargeCreatedMoreThanNinetyMinutesAgo = chargesSortedByAccessedDate.find(charge => charge.state.createdAt < ninetyMinutesAgo)
+
+  if (leastRecentlyAccessedChargeCreatedMoreThanNinetyMinutesAgo) {
+    return leastRecentlyAccessedChargeCreatedMoreThanNinetyMinutesAgo.chargeId
+  }
+
+  logger.info('No preferred charges found for deletion on session, defaulting to least recently accessed charge')
+  const leastRecentlyAccessedCharge = chargesSortedByAccessedDate[0]
+  return leastRecentlyAccessedCharge.chargeId
+}
+
 /**
  * Sets session[key] = value for all valid sessions, based on existence of encryption key,
  * and the existence of relevant cookie on the request
@@ -151,6 +199,18 @@ function setSessionVariable (req, key, value) {
   }
 }
 
+/**
+ * @param {Request} req
+ * @param {string} key
+ * @param {ChargeState} chargeState
+ */
+function setSessionChargeState (req, key, chargeState) {
+  const chargeStateObject = {
+    data: chargeState.toString()
+  }
+  setSessionVariable(req, key, chargeStateObject)
+}
+
 /**
  * Gets value of key from session, based on existence of encryption key
  *
@@ -160,15 +220,38 @@ function setSessionVariable (req, key, value) {
  */
 function getSessionVariable (req, key) {
   const session = req[getSessionCookieName()]
-  return session && session[key]
+  return session && session?.[key]
+}
+
+/**
+ * @param {Request} req
+ * @param {string} key
+ * @returns {ChargeState}
+ */
+function getSessionChargeState (req, key) {
+  return chargeStateFromString(getSessionVariable(req, key).data)
+}
+
+/**
+ * @param {Request} req
+ * @returns {string}
+ */
+function getSessionCsrfSecret (req) {
+  return getSessionVariable(req, 'csrfSecret')
 }
 
 function isSessionPresent (req) {
   const session = req[getSessionCookieName()]
   return session && Object.getOwnPropertyNames(session).length > 0
 }
 
-function getSessionVariableNames (req) {
+/**
+ * @param {Request} req
+ * @returns {string[]}
+ */
+function getChargesOnSession (req) {
   const session = req[getSessionCookieName()]
-  return Object.keys(session)
+  return Object.keys(session).filter(key => {
+    return key.startsWith('ch_') && Object.keys(session[key]).includes('data')
+  })
 }