PP-13360 move csrf middleware functions to pay-js-commons

app/middleware/csrf.js

@@ -1,52 +1,34 @@
-'use strict'
-
-// NPM dependencies
-const csrf = require('csrf')
-
-// Local dependencies
 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')
+const { configureCsrfMiddleware } = require('@govuk-pay/pay-js-commons/lib/utils/middleware/csrf.middleware')
 
-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()
-}
+const csrfMiddleware = configureCsrfMiddleware(logger, cookies.getSessionCookieName(), 'csrfSecret', 'csrfToken')
 
-exports.csrfTokenGeneration = (req, res, next) => {
-  const csrfSecret = cookies.getSessionCsrfSecret(req)
-  res.locals.csrf = csrf().create(csrfSecret)
-  next()
-}
+exports.setSecret = csrfMiddleware.setSecret
+
+exports.generateToken = csrfMiddleware.generateToken
+
+exports.checkToken = [checkToken, handleCsrfError]
 
-exports.csrfCheck = (req, res, next) => {
+function checkToken (req, res, next) {
   const chargeId = fetchAndValidateChargeId(req)
   if (!chargeId) {
     return responseRouter.response(req, res, 'UNAUTHORISED')
   }
+  csrfMiddleware.checkToken(req, res, next)
+}
 
-  const sessionCsrfSecret = cookies.getSessionCsrfSecret(req)
-  const csrfToken = req.body.csrfToken
-
-  if (!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, sessionCsrfSecret, req)) {
-    responseRouter.systemErrorResponse(req, res, 'CSRF is invalid')
-  } else {
-    next()
+function handleCsrfError (err, req, res, next) {
+  if (err && err.name === 'CsrfError') {
+    if (err.message.toLowerCase().includes('csrf secret was not found')) {
+      return responseRouter.response(req, res, 'UNAUTHORISED')
+    } else {
+      return responseRouter.systemErrorResponse(req, res, 'CSRF is invalid')
+    }
   }
+  next(err)
 }
 
 function fetchAndValidateChargeId (req) {
@@ -55,14 +37,3 @@ function fetchAndValidateChargeId (req) {
   }
   return false
 }
-
-function csrfValid (csrfToken, secret, req) {
-  if (!secret) {
-    return false
-  }
-  if (!['put', 'post'].includes(req.method.toLowerCase())) {
-    return true
-  } else {
-    return csrf().verify(secret, csrfToken)
-  }
-}

app/routes.js

@@ -14,7 +14,7 @@ const { log } = require('./controllers/client-side-logging.controller')
 const paths = require('./paths.js')
 
 // Express middleware
-const { csrfSetSecret, csrfCheck, csrfTokenGeneration } = require('./middleware/csrf.js')
+const { setSecret, generateToken, checkToken } = 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,9 +43,9 @@ exports.bind = function (app) {
   const card = paths.card
 
   const standardMiddlewareStack = [
-    csrfSetSecret,
-    csrfCheck,
-    csrfTokenGeneration,
+    setSecret,
+    checkToken,
+    generateToken,
     actionName,
     enforceSessionCookie,
     retrieveCharge,

app/utils/response-router.js

@@ -239,7 +239,7 @@ const actions = {
   }
 }
 
-exports.errorResponse = function errorReponse (req, res, reason, options = {}, error) {
+exports.errorResponse = function errorResponse (req, res, reason, options = {}, error) {
   const action = actions.ERROR
   logErrorPageShown(action.view, reason, getLoggingFields(req), error)
   options.viewName = 'ERROR'

package.json

@@ -72,15 +72,14 @@
   "dependencies": {
     "@aws-crypto/decrypt-node": "^1.0.3",
     "@aws-crypto/raw-rsa-keyring-node": "^1.1.0",
-    "@govuk-pay/pay-js-commons": "^6.0.18",
+    "@govuk-pay/pay-js-commons": "^6.0.22",
     "@govuk-pay/pay-js-metrics": "^1.0.6",
     "@sentry/node": "7.119.2",
     "cert-info": "^1.5.1",
     "change-case": "2.3.x",
     "client-sessions": "0.8.x",
     "compression": "1.7.x",
     "credit-card-type": "6.3.x",
-    "csrf": "3.1.x",
     "express": "4.21.2",
     "express-rate-limit": "^7.1.4",
     "gaap-analytics": "^3.1.0",

test/middleware/csrf.test.js

@@ -4,7 +4,7 @@ const _ = require('lodash')
 const expect = require('chai').expect
 const nock = require('nock')
 const helper = require('../test-helpers/test-helpers.js')
-const { csrfCheck, csrfTokenGeneration } = require('../../app/middleware/csrf.js')
+const { checkToken, generateToken } = require('../../app/middleware/csrf.js')
 
 describe('retrieve param test', function () {
   const response = {
@@ -32,9 +32,6 @@ describe('retrieve param test', function () {
   const noCharge = _.cloneDeep(validGetRequest)
   delete noCharge.frontend_state.ch_foo
 
-  const noSecret = _.cloneDeep(validGetRequest)
-  delete noSecret.frontend_state.csrfSecret
-
   const invalidPost = _.cloneDeep(validGetRequest)
   delete invalidPost.method
   const invalidPut = _.cloneDeep(invalidPost)
@@ -45,6 +42,9 @@ describe('retrieve param test', function () {
   const validPost = _.cloneDeep(invalidPost)
   validPost.body.csrfToken = helper.csrfToken()
 
+  const noSecret = _.cloneDeep(validPost)
+  delete noSecret.frontend_state.csrfSecret
+
   const validPut = _.cloneDeep(invalidPut)
   validPut.body.csrfToken = helper.csrfToken()
 
@@ -67,6 +67,18 @@ describe('retrieve param test', function () {
     expect(resp.locals.csrf).to.not.be.undefined // eslint-disable-line
   }
 
+  const callCheckToken = (scenario, expectedResponse, next) => {
+    const [checkTokenMiddleware, handleCsrfError] = checkToken
+    checkTokenMiddleware(scenario, expectedResponse, (err) => {
+      // simulate next(err)
+      if (err) {
+        handleCsrfError(err, scenario, expectedResponse, next)
+      } else {
+        next()
+      }
+    })
+  }
+
   beforeEach(function () {
     status = sinon.stub(response, 'status')
     render = sinon.stub(response, 'render')
@@ -81,38 +93,38 @@ describe('retrieve param test', function () {
 
   it('should append csrf token to response locals on get request', function () {
     const resp = _.cloneDeep(response)
-    csrfTokenGeneration(validGetRequest, resp, next)
+    generateToken(validGetRequest, resp, next)
     assertValidRequest(next, resp, status, render)
   })
 
   it('should error if no charge in session', function () {
     const resp = _.cloneDeep(response)
-    csrfCheck(noCharge, resp, next)
+    callCheckToken(noCharge, resp, next)
     assertUnauthorisedRequest(next, resp, status, render)
   })
 
   it('should error if no secret in session', function () {
     const resp = _.cloneDeep(response)
-    csrfCheck(noSecret, resp, next)
+    callCheckToken(noSecret, resp, next)
     assertUnauthorisedRequest(next, resp, status, render)
   })
 
   it('should error if no csrfToken in post request', function () {
     const resp = _.cloneDeep(response)
-    csrfCheck(invalidPost, resp, next)
+    callCheckToken(invalidPost, resp, next)
     assertErrorRequest(next, resp, status, render)
   })
 
   it('should be successful on post if valid put', function () {
     const resp = _.cloneDeep(response)
-    csrfCheck(validPut, resp, next)
-    csrfTokenGeneration(validGetRequest, resp, next)
+    callCheckToken(validPut, resp, next)
+    generateToken(validGetRequest, resp, next)
     assertValidRequest(next, resp, status, render)
   })
 
   it('should error if no csrfToken in put request', function () {
     const resp = _.cloneDeep(response)
-    csrfCheck(invalidPut, resp, next)
+    callCheckToken(invalidPut, resp, next)
     assertErrorRequest(next, resp, status, render)
   })
 })