PP-12546 remove csrf secret from charge objects

this change is part of a wider series that are being merged in sequence to prevent the need for a downtime deployment

- do not set charge level csrf secret
- do not validate tokens against charge level csrf secret
- update tests to reflect new behaviour

app/controllers/secure.controller.js

@@ -1,7 +1,5 @@
 'use strict'
 
-const csrf = require('csrf')
-
 const {
   PAYMENT_EXTERNAL_ID,
   GATEWAY_ACCOUNT_ID,
@@ -54,7 +52,6 @@ exports.new = async function (req, res) {
         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

@@ -31,19 +31,18 @@ exports.csrfCheck = (req, res, next) => {
     return responseRouter.response(req, res, 'UNAUTHORISED')
   }
 
-  const chargeSession = session.retrieve(req, chargeId) || {} // TODO: remove after PP-12546 has been merged
   const sessionCsrfSecret = cookies.getSessionCsrfSecret(req)
   const csrfToken = req.body.csrfToken
 
-  if (!chargeSession.csrfSecret && !sessionCsrfSecret) {
+  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, chargeSession.csrfSecret, req) && !csrfValid(csrfToken, sessionCsrfSecret, req)) {
+  } else if (!csrfValid(csrfToken, sessionCsrfSecret, req)) {
     responseRouter.systemErrorResponse(req, res, 'CSRF is invalid')
   } else {
     next()

test/middleware/csrf.test.js

@@ -4,7 +4,6 @@ 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')
 
 describe('retrieve param test', function () {
@@ -22,9 +21,9 @@ describe('retrieve param test', function () {
     body: {},
     route: { methods: { get: true } },
     frontend_state: {
-      csrfSecret: process.env.CSRF_USER_SECRET_TWO,
+      csrfSecret: process.env.CSRF_USER_SECRET,
       ch_foo: {
-        csrfSecret: process.env.CSRF_USER_SECRET
+       data: 'blah'
       }
     },
     get: () => null
@@ -34,7 +33,6 @@ describe('retrieve param test', function () {
   delete noCharge.frontend_state.ch_foo
 
   const noSecret = _.cloneDeep(validGetRequest)
-  delete noSecret.frontend_state.ch_foo.csrfSecret
   delete noSecret.frontend_state.csrfSecret
 
   const invalidPost = _.cloneDeep(validGetRequest)
@@ -47,16 +45,6 @@ describe('retrieve param test', function () {
   const validPost = _.cloneDeep(invalidPost)
   validPost.body.csrfToken = helper.csrfToken()
 
-  const validPostWithSessionSecretAndChargeSecretAndChargeSecretGeneratedToken = _.cloneDeep(invalidPost)
-  validPostWithSessionSecretAndChargeSecretAndChargeSecretGeneratedToken.body.csrfToken = helper.csrfToken(process.env.CSRF_USER_SECRET)
-
-  const validPostWithSessionSecretAndChargeSecretAndSessionSecretGeneratedToken = _.cloneDeep(invalidPost)
-  validPostWithSessionSecretAndChargeSecretAndSessionSecretGeneratedToken.body.csrfToken = helper.csrfToken(process.env.CSRF_USER_SECRET_TWO)
-
-  const validPostWithSessionSecretAndSessionSecretGeneratedToken = _.cloneDeep(invalidPost)
-  delete validPostWithSessionSecretAndSessionSecretGeneratedToken.frontend_state.ch_foo.csrfSecret
-  validPostWithSessionSecretAndSessionSecretGeneratedToken.body.csrfToken = helper.csrfToken(process.env.CSRF_USER_SECRET_TWO)
-
   const validPut = _.cloneDeep(invalidPut)
   validPut.body.csrfToken = helper.csrfToken()
 
@@ -122,24 +110,6 @@ describe('retrieve param test', function () {
     assertValidRequest(next, resp, status, render)
   })
 
-  it('should use charge secret for csrf check when session secret, charge secret and charge secret generated token are present', function () {
-    const resp = _.cloneDeep(response)
-    csrfCheck(validPostWithSessionSecretAndChargeSecretAndChargeSecretGeneratedToken, resp, next)
-    expect(next.called).to.be.true // eslint-disable-line
-  })
-
-  it('should use session secret for csrf check when session secret, charge secret and session secret generated token are present', function () {
-    const resp = _.cloneDeep(response)
-    csrfCheck(validPostWithSessionSecretAndChargeSecretAndSessionSecretGeneratedToken, resp, next)
-    expect(next.called).to.be.true // eslint-disable-line
-  })
-
-  it('should use session secret for csrf check when only session secret is set', function () {
-    const resp = _.cloneDeep(response)
-    csrfCheck(validPostWithSessionSecretAndSessionSecretGeneratedToken, resp, next)
-    expect(next.called).to.be.true // eslint-disable-line
-  })
-
   it('should error if no csrfToken in put request', function () {
     const resp = _.cloneDeep(response)
     csrfCheck(invalidPut, resp, next)

test/test-helpers/session.js

@@ -14,7 +14,6 @@ function createReturnUrlKey (chargeId) {
 
 function createSessionWithReturnUrl (chargeId, chargeSession, returnUrl) {
   chargeSession = chargeSession || { data: new ChargeState().toString() }
-  chargeSession.csrfSecret = process.env.CSRF_USER_SECRET
   const session = {}
   if (arguments.length > 0) {
     session.csrfSecret = process.env.CSRF_USER_SECRET

test/test.env

@@ -4,7 +4,6 @@ SECURE_COOKIE_OFF=false
 COOKIE_MAX_AGE=5400000
 SESSION_ENCRYPTION_KEY=naskjwefvwei72rjkwfmjwfi72rfkjwefmjwefiuwefjkbwfiu24fmjbwfk
 CSRF_USER_SECRET=123456789012345678
-CSRF_USER_SECRET_TWO=987654321012345678
 CARDID_HOST=http://cardid.pymnt.localdomain:65530
 WORLDPAY_APPLE_PAY_MERCHANT_ID=merchant.uk.gov.service.payments.test
 STRIPE_APPLE_PAY_MERCHANT_ID=merchant.uk.gov.service.payments.stripe.test