Skip to content

Commit

Permalink
Merge pull request #3551 from alphagov/PP-11244-prepare-for-changing-…
Browse files Browse the repository at this point in the history
…token-response

PP-11244 Prepare for changing find charge by token response
  • Loading branch information
stephencdaly committed Jul 4, 2023
2 parents 11ba365 + f246eb7 commit 1e7c971
Show file tree
Hide file tree
Showing 9 changed files with 48 additions and 30 deletions.
28 changes: 20 additions & 8 deletions app/controllers/secure.controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,21 +25,33 @@ const paths = require('../paths')
exports.new = async function (req, res) {
const chargeTokenId = req.params.chargeTokenId || req.body.chargeTokenId
const correlationId = req.headers[CORRELATION_HEADER] || ''
var chargeId
try {
const chargeData = await Charge(correlationId).findByToken(chargeTokenId, getLoggingFields(req))
chargeId = chargeData.charge.externalId
const tokenResponse = await Charge(correlationId).findByToken(chargeTokenId, getLoggingFields(req))
let chargeId, gatewayAccountId, gatewayAccountType

// Temporarily support both the new and old response schemas from connector
if (tokenResponse.charge.externalId) {
chargeId = tokenResponse.charge.externalId
gatewayAccountId = tokenResponse.charge.gatewayAccount.gateway_account_id
gatewayAccountType = tokenResponse.charge.gatewayAccount.type
} else {
chargeId = tokenResponse.charge.charge_id
gatewayAccountId = tokenResponse.charge.gateway_account.gateway_account_id
gatewayAccountType = tokenResponse.charge.gateway_account.type
}

const chargeStatus = tokenResponse.charge.status

setLoggingField(req, PAYMENT_EXTERNAL_ID, chargeId)
setLoggingField(req, GATEWAY_ACCOUNT_ID, chargeData.charge.gatewayAccount.gateway_account_id)
setLoggingField(req, GATEWAY_ACCOUNT_TYPE, chargeData.charge.gatewayAccount.type)
setLoggingField(req, GATEWAY_ACCOUNT_ID, gatewayAccountId)
setLoggingField(req, GATEWAY_ACCOUNT_TYPE, gatewayAccountType)

if (chargeData.used === true) {
if (tokenResponse.used === true) {
if (!getSessionVariable(req, createChargeIdSessionKey(chargeId))) {
throw new Error('UNAUTHORISED')
}
logger.info('Payment token being reused', getLoggingFields(req))
const stateName = chargeData.charge.status.toUpperCase().replace(/\s/g, '_')
const stateName = chargeStatus.toUpperCase().replace(/\s/g, '_')
responseRouter.response(req, res, stateName, {
chargeId: chargeId,
returnUrl: paths.generateRoute('card.return', { chargeId })
Expand All @@ -48,7 +60,7 @@ exports.new = async function (req, res) {
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() })
res.redirect(303, generateRoute(resolveActionName(chargeData.charge.status, 'get'), { chargeId }))
res.redirect(303, generateRoute(resolveActionName(chargeStatus, 'get'), { chargeId }))
}
} catch (err) {
if (err.message === 'UNAUTHORISED') {
Expand Down
36 changes: 25 additions & 11 deletions test/controllers/secure.controller.test.js
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
'use strict'

// NPM dependencies
const proxyquire = require('proxyquire')
const sinon = require('sinon')
const expect = require('chai').expect

// Local dependencies
const paths = require('../../app/paths.js')
const withAnalyticsError = require('../../app/utils/analytics').withAnalyticsError
const { validChargeDetails } = require('../fixtures/payment.fixtures')

// configure
require('../test-helpers/html-assertions.js')

const chargeId = 'ch_dh6kpbb4k82oiibbe4b9haujjk'

const mockCharge = (function () {
const mock = function (withSuccess, chargeObject) {
return function () {
Expand Down Expand Up @@ -134,13 +134,27 @@ describe('secure controller', function () {
})
})

describe('then mark as used successfully', function () {
it('should store the service name into the session and redirect', async function () {
await requireSecureController(mockCharge.withSuccess(chargeObject), mockToken.withSuccess(), responseRouter).new(request, response)
expect(response.redirect.calledWith(303, paths.generateRoute('card.new', { chargeId: chargeObject.charge.externalId }))).to.be.true // eslint-disable-line
expect(request.frontend_state).to.have.all.keys('ch_dh6kpbb4k82oiibbe4b9haujjk')
expect(request.frontend_state.ch_dh6kpbb4k82oiibbe4b9haujjk).to.eql({
csrfSecret: 'foo' // pragma: allowlist secret
describe('when the token is used successfully', function () {
describe('old connector token response', () => {
it('should store the service name into the session and redirect', async function () {
await requireSecureController(mockCharge.withSuccess(chargeObject), mockToken.withSuccess(), responseRouter).new(request, response)
expect(response.redirect.calledWith(303, paths.generateRoute('card.new', {chargeId: chargeObject.charge.externalId}))).to.be.true // eslint-disable-line
expect(request.frontend_state).to.have.all.keys(chargeId)
expect(request.frontend_state.ch_dh6kpbb4k82oiibbe4b9haujjk).to.eql({
csrfSecret: 'foo' // pragma: allowlist secret
})
})
})

describe('new connector token response', () => {
it('should store the service name into the session and redirect', async function () {
const newChargeObject = validChargeDetails({ chargeId })
await requireSecureController(mockCharge.withSuccess(newChargeObject), mockToken.withSuccess(), responseRouter).new(request, response)
expect(response.redirect.calledWith(303, paths.generateRoute('card.new', {chargeId: chargeObject.charge.externalId}))).to.be.true // eslint-disable-line
expect(request.frontend_state).to.have.all.keys(chargeId)
expect(request.frontend_state.ch_dh6kpbb4k82oiibbe4b9haujjk).to.eql({
csrfSecret: 'foo' // pragma: allowlist secret
})
})
})
})
Expand Down Expand Up @@ -253,7 +267,7 @@ describe('secure controller', function () {
returnUrl: '/return/dh6kpbb4k82oiibbe4b9haujjk'
}
expect(responseRouter.response.calledWith(requestWithFrontendStateCookie, response, 'AUTHORISATION_SUCCESS', opts)).to.be.true // eslint-disable-line
expect(requestWithFrontendStateCookie.frontend_state).to.have.all.keys('ch_dh6kpbb4k82oiibbe4b9haujjk')
expect(requestWithFrontendStateCookie.frontend_state).to.have.all.keys(chargeId)
expect(requestWithFrontendStateCookie.frontend_state.ch_dh6kpbb4k82oiibbe4b9haujjk).to.eql({
csrfSecret: 'foo' // pragma: allowlist secret
})
Expand Down
1 change: 0 additions & 1 deletion test/cypress/integration/card/awaiting-auth.cy.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ describe('Awaiting auth', () => {
cy.get('#address-postcode').type(validPayment.postcode)
cy.get('#email').type(validPayment.email)


const paymentDetails = {
cardNumber: validPayment.cardNumber,
expiryMonth: validPayment.expiryMonth,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ describe('Billing address collection', () => {
tokenId, chargeId, 'en', gatewayAccountId, serviceOpts, {}, {}, chargeOpts)

it('Should show billing address section and populate with default billing address', () => {

cy.task('setupStubs', createPaymentChargeStubs)
cy.visit(`/secure/${tokenId}`)

Expand Down Expand Up @@ -82,7 +81,6 @@ describe('Billing address collection', () => {
tokenId, chargeId, 'en', gatewayAccountId, serviceOpts, {}, {}, chargeOpts)

it('Should not show the billing address section and allow valid card submission', () => {

cy.task('setupStubs', createPaymentChargeStubs)
cy.visit(`/secure/${tokenId}`)

Expand All @@ -95,7 +93,6 @@ describe('Billing address collection', () => {
cy.get('#address-city').should('not.exist')
cy.get('#address-postcode').should('not.exist')


cy.task('clearStubs')

const checkCardDetailsStubs = cardPaymentStubs.checkCardDetailsStubs(chargeId)
Expand Down
1 change: 0 additions & 1 deletion test/cypress/integration/card/email-typo.cy.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,6 @@ describe('Standard card payment flow', () => {
cy.get('#cardholder-name').should(($td) => expect($td).to.contain(validPayment.name))
cy.get('#email').should(($td) => expect($td).to.contain(validPayment.email))


cy.task('clearStubs')
cy.task('setupStubs', submitPaymentCaptureStubs)

Expand Down
1 change: 0 additions & 1 deletion test/cypress/integration/card/payment.cy.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,6 @@ describe('Standard card payment flow', () => {
cy.get('#address-postcode').type(validPayment.postcode)
cy.get('#email').type(validPayment.email)


const lastFourCardDigits = validPayment.cardNumber.toString().slice(-4)

cy.task('clearStubs')
Expand Down
4 changes: 1 addition & 3 deletions test/fixtures/payment.fixtures.js
Original file line number Diff line number Diff line change
Expand Up @@ -258,9 +258,7 @@ const fixtures = {
tokenResponse: (opts = {}) => {
return {
used: opts.used,
charge: {
externalId: opts.chargeExternalId
}
charge: {}
}
},

Expand Down
2 changes: 1 addition & 1 deletion test/unit/charge.controller.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ describe('walletEnabled', () => {
{ walletToggle: true, gatewayAccountId: '1', payTestGatewayAccounts: ['5', '10', '15', '20'], expected: true },
{ walletToggle: false, gatewayAccountId: '1', payTestGatewayAccounts: ['5', '10', '15', '20'], expected: false },
{ walletToggle: false, gatewayAccountId: '15', payTestGatewayAccounts: ['5', '10', '15', '20'], expected: true },
{ walletToggle: false, gatewayAccountId: '15', payTestGatewayAccounts: [], expected: false },
{ walletToggle: false, gatewayAccountId: '15', payTestGatewayAccounts: [], expected: false }
]

testData.forEach(({ walletToggle, gatewayAccountId, payTestGatewayAccounts, expected }) => {
Expand Down
2 changes: 1 addition & 1 deletion test/unit/clients/connector-client-token.pact.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ describe('connector client - tokens', function () {
tokenId: TOKEN
})
expect(res.body.used).to.be.equal(false)
expect(res.body.charge.externalId).to.be.equal('chargeExternalId')
expect(res.body.charge).to.exist // eslint-disable-line
})
})
})

0 comments on commit 1e7c971

Please sign in to comment.