Merge pull request #1367 from alphagov/PP-6079-dont-collect-billing-a…

…ddress-for-moto-payments

PP-6079 Don't collect billing address for moto payments

app/controllers/charge_controller.js

@@ -122,7 +122,7 @@ module.exports = {
     const cardModel = Card(req.chargeData.gateway_account.card_types, req.chargeData.gateway_account.block_prepaid_cards, req.headers[CORRELATION_HEADER])
     const chargeOptions = {
       email_collection_mode: charge.gatewayAccount.emailCollectionMode,
-      collect_billing_address: res.locals.service.collectBillingAddress
+      collect_billing_address: res.locals.collectBillingAddress
     }
     const validator = chargeValidator(i18n.__('fieldErrors'), logger, cardModel, chargeOptions, getLoggingFields(req))
 
@@ -185,7 +185,7 @@ module.exports = {
 
     const correlationId = req.headers[CORRELATION_HEADER] || ''
     const payload = normalise.apiPayload(req, card)
-    if (res.locals.service.collectBillingAddress === false) {
+    if (res.locals.collectBillingAddress === false) {
       delete payload.address
     }
     try {

app/middleware/resolve_service.js

@@ -25,6 +25,7 @@ module.exports = function resolveServiceMiddleware (req, res, next) {
   const cachedService = serviceCache.get(gatewayAccountId)
   if (cachedService) {
     res.locals.service = cachedService
+    res.locals.collectBillingAddress = res.locals.service.collectBillingAddress && !req.chargeData.moto
     next()
   } else {
     // @FIXME(sfount) tests shouldn't rely on middleware returning a value if
@@ -34,6 +35,7 @@ module.exports = function resolveServiceMiddleware (req, res, next) {
       .then(service => {
         serviceCache.put(gatewayAccountId, service, SERVICE_CACHE_MAX_AGE)
         res.locals.service = service
+        res.locals.collectBillingAddress = res.locals.service.collectBillingAddress && !req.chargeData.moto
         next()
       })
       .catch((err) => {

app/views/charge.njk

@@ -272,7 +272,7 @@
           </span>
           <img src="/images/amex-security-code.png" class="amex-cvc hidden" alt="{{ __p("cardDetails.amexcvcTip") }}"/>
         </div>
-        {% if service.collectBillingAddress %}
+        {% if collectBillingAddress %}
           <div class="govuk-form-group govuk-!-width-three-quarters govuk-!-padding-top-4 govuk-!-margin-top-8 pay-!-border-top {% if highlightErrorFields.addressCountry %} govuk-form-group--error{% endif %}" data-validation="addressCountry">
             <legend for="address-country" class="govuk-!-margin-bottom-6">
               <h2 class="govuk-heading-m govuk-!-margin-bottom-1">{{ __p("cardDetails.billingAddress") }}</h2>

app/views/includes/scripts.njk

@@ -30,7 +30,7 @@
   };
   window.Charge = {
     email_collection_mode: '{{ gatewayAccount.emailCollectionMode | safe }}',
-    collect_billing_address: {{ "true" if service.collectBillingAddress else "false" }},
+    collect_billing_address: {{ "true" if collectBillingAddress else "false" }},
     worldpay_3ds_flex_ddc_jwt: '{{ worldpay3dsFlexDdcJwt }}',
     worldpay_3ds_flex_ddc_url: '{{ worldpay3dsFlexDdcUrl }}'
   }
@@ -41,7 +41,7 @@
     if (mainWrap.classList.contains('charge-new')) {
       window.payScripts.helpers.setGlobalChargeId();
       showCardType().init();
-      {% if service.collectBillingAddress %}
+      {% if collectBillingAddress %}
         window.payScripts.helpers.initialiseAddressCountryAutocomplete();
       {% endif %}
       window.payScripts.inputConfirm.init();

test/charge_ui_tests.js

@@ -41,7 +41,7 @@ const generateConfirmViewTemplateData = (templateData = {}) => {
 describe('The charge view', function () {
   it('should render the amount', function () {
     const templateData = {
-      'amount': '50.00'
+      amount: '50.00'
     }
 
     const body = renderTemplate('charge', templateData)
@@ -51,7 +51,7 @@ describe('The charge view', function () {
   it('should have a submit form.', function () {
     const postAction = '/post_card_path'
     const templateData = {
-      'post_card_action': postAction
+      post_card_action: postAction
     }
 
     const body = renderTemplate('charge', templateData)
@@ -72,9 +72,7 @@ describe('The charge view', function () {
   it('should show all input fields.', function () {
     const body = renderTemplate('charge', {
       id: '1234',
-      service: {
-        collectBillingAddress: true
-      }
+      collectBillingAddress: true
     })
     body.should.containInputWithIdAndName('csrf', 'csrfToken', 'hidden')
     body.should.containInputWithIdAndName('card-no', 'cardNo', 'text').withAttribute('maxlength', '26').withLabel('card-no-lbl', 'Card number')
@@ -91,30 +89,8 @@ describe('The charge view', function () {
     body.should.not.containSelector('.custom-branding-image')
   })
 
-  it('should not show billing address for services not wanting to capture it', function () {
-    const body = renderTemplate('charge', {
-      id: '1234',
-      service: {
-        collectBillingAddress: false
-      }
-    })
-    body.should.containInputWithIdAndName('csrf', 'csrfToken', 'hidden')
-    body.should.containInputWithIdAndName('card-no', 'cardNo', 'text').withAttribute('maxlength', '26').withLabel('card-no-lbl', 'Card number')
-    body.should.containInputWithIdAndName('cvc', 'cvc', 'text').withLabel('cvc-lbl', 'Card security code')
-    body.should.containInputWithIdAndName('expiry-month', 'expiryMonth', 'text')
-    body.should.containInputWithIdAndName('expiry-year', 'expiryYear', 'text')
-    body.should.containInputWithIdAndName('cardholder-name', 'cardholderName', 'text').withAttribute('maxlength', '200').withLabel('cardholder-name-lbl', 'Name on card')
-    body.should.not.containSelector('#address-country')
-    body.should.not.containSelector('#address-line-1')
-    body.should.not.containSelector('#address-line-2')
-    body.should.not.containSelector('#address-city')
-    body.should.not.containSelector('#address-postcode')
-    body.should.containInputWithIdAndName('charge-id', 'chargeId', 'hidden').withAttribute('value', '1234')
-    body.should.not.containSelector('.custom-branding-image')
-  })
-
   it('should display custom branding', () => {
-    const templateData = lodash.merge('charge', { 'id': '1234' }, customBrandingData)
+    const templateData = lodash.merge('charge', { id: '1234' }, customBrandingData)
     const body = renderTemplate('charge', templateData)
     body.should.containSelector('.custom-branding-image')
 
@@ -133,9 +109,7 @@ describe('The charge view', function () {
       addressLine2: 'blah blah',
       addressCity: 'Leicester City',
       addressPostcode: 'CT16 1FB',
-      service: {
-        collectBillingAddress: true
-      }
+      collectBillingAddress: true
     }
     const body = renderTemplate('charge', responseData)
 
@@ -164,28 +138,6 @@ describe('The confirm view', function () {
     body.should.containSelector('#address').withText('1 street lane, avenue city, AB1 3DF')
   })
 
-  it('should not show billing address for services not wanting to capture it', function () {
-    const body = renderTemplate('confirm', generateConfirmViewTemplateData({
-      service: {
-        collectBillingAddress: false
-      },
-      charge: {
-        cardDetails: {
-          billingAddress: null
-        }
-      }
-    }))
-    const $ = cheerio.load(body)
-    $('#payment-description').html().should.contain('Payment Description & <xss attack> assessment')
-    body.should.containInputWithIdAndName('csrf', 'csrfToken', 'hidden')
-    body.should.containSelector('#card-number').withText('●●●●●●●●●●●●5100')
-    body.should.containSelector('#expiry-date').withText('11/99')
-    body.should.containSelector('#cardholder-name').withText('Francisco Blaya-Gonzalvez')
-    body.should.not.containSelector('#address').withText('1 street lane, avenue city, AB1 3DF')
-    body.should.containSelector('#payment-description').withText('Payment Description')
-    body.should.containSelector('#amount').withText('£10.00')
-  })
-
   it('should display custom branding', () => {
     const templateData = lodash.merge(successTemplateDataWithCollectBillingAddress, customBrandingData)
     const body = renderTemplate('confirm', templateData)
@@ -199,7 +151,7 @@ describe('The confirm view', function () {
   })
 
   it('should render a confirm button', function () {
-    const body = renderTemplate('confirm', { confirmPath: '/card_details/123/confirm', 'charge': { id: 1234, amount: 50 } })
+    const body = renderTemplate('confirm', { confirmPath: '/card_details/123/confirm', charge: { id: 1234, amount: 50 } })
     const $ = cheerio.load(body)
     body.should.containSelector('form#confirmation').withAttributes(
       {
@@ -213,7 +165,7 @@ describe('The confirm view', function () {
   it('should have a cancel form.', function () {
     const postAction = '/post_cancel_path'
     const templateData = {
-      'post_cancel_action': postAction
+      post_cancel_action: postAction
     }
 
     const body = renderTemplate('charge', templateData)

test/controllers/charge_controller_create_test.js

@@ -15,6 +15,23 @@ const chargeId = '42mdrsshtsk4chpeoifhlgf4lk'
 const card = paymentFixtures.validCardDetails()
 const chargeData = paymentFixtures.validChargeDetails({ emailCollectionMode: 'OFF' }).getPlain()
 
+const paymentDetailsWithoutAddress = {
+  chargeId: chargeId,
+  cardNo: '4242424242424242',
+  expiryMonth: '01',
+  expiryYear: '20',
+  cardholderName: 'Joe Bloggs',
+  cvc: '111'
+}
+
+const paymentDetails = {
+  ...paymentDetailsWithoutAddress,
+  addressCountry: 'GB',
+  addressLine1: '1 Horse Guards',
+  addressCity: 'London',
+  addressPostcode: 'E1 8QS'
+}
+
 const mockedChargeValidationBackend = function () {
   const validation = {
     hasError: false
@@ -38,7 +55,6 @@ describe('POST /card_details/{chargeId} endpoint', function () {
   let response
   let chargeAuthStub
   let mockedConnectorClient
-  let paymentDetails
 
   beforeEach(() => {
     chargeAuthStub = sinon.stub().resolves(
@@ -56,28 +72,13 @@ describe('POST /card_details/{chargeId} endpoint', function () {
     response = {
       redirect: sinon.spy(),
       locals: {
-        service: {
-          collectBillingAddress: true
-        }
-
+        collectBillingAddress: true
       }
     }
-    paymentDetails = {
-      'chargeId': chargeId,
-      'cardNo': '4242424242424242',
-      'expiryMonth': '01',
-      'expiryYear': '20',
-      'cardholderName': 'Joe Bloggs',
-      'cvc': '111',
-      'addressCountry': 'GB',
-      'addressLine1': '1 Horse Guards',
-      'addressCity': 'London',
-      'addressPostcode': 'E1 8QS'
-    }
   })
 
   it('should send worldpay_3ds_flex_ddc_result to connector when the request includes a worldpay3dsFlexDdcResult parameter', async function () {
-    paymentDetails['worldpay3dsFlexDdcResult'] = 'a-worldpay-3ds-flex-ddc-result'
+    paymentDetails.worldpay3dsFlexDdcResult = 'a-worldpay-3ds-flex-ddc-result'
     const request = {
       chargeData: chargeData,
       body: paymentDetails,
@@ -112,8 +113,8 @@ describe('POST /card_details/{chargeId} endpoint', function () {
 
     expect(chargeAuthStub.calledWith(sinon.match( // eslint-disable-line
       {
-        'chargeId': chargeId,
-        'payload': payload
+        chargeId: chargeId,
+        payload: payload
       }
     ))).to.be.true // eslint-disable-line
   })
@@ -148,12 +149,49 @@ describe('POST /card_details/{chargeId} endpoint', function () {
 
     delete payload.accept_header
     delete payload.user_agent_header
-    delete payload.worldpay_3ds_flex_ddc_result
 
     expect(chargeAuthStub.calledWith(sinon.match( // eslint-disable-line
       {
-        'chargeId': chargeId,
-        'payload': payload
+        chargeId: chargeId,
+        payload: payload
+      }
+    ))).to.be.true // eslint-disable-line
+  })
+
+  it('should not include billing address in authorisation request when billing address collection disabled', async function () {
+    response.locals.collectBillingAddress = false
+
+    const request = {
+      chargeData: chargeData,
+      body: paymentDetailsWithoutAddress,
+      chargeId: chargeId,
+      header: sinon.spy(),
+      headers: {
+        'x-request-id': 'unique-id',
+        'x-forwarded-for': '127.0.0.1'
+      }
+    }
+    await requireChargeController(mockedConnectorClient).create(request, response)
+
+    const payload = paymentFixtures.validAuthorisationRequest({
+      cardNumber: paymentDetailsWithoutAddress.cardNo,
+      cvc: paymentDetailsWithoutAddress.cvc,
+      cardBrand: card.brand,
+      expiryDate: `${paymentDetailsWithoutAddress.expiryMonth}/${paymentDetailsWithoutAddress.expiryYear}`,
+      cardholderName: paymentDetailsWithoutAddress.cardholderName,
+      cardType: card.type,
+      corporateCard: card.corporate,
+      prepaid: card.prepaid,
+      noBillingAddress: true
+    }).getPlain()
+
+    delete payload.accept_header
+    delete payload.user_agent_header
+
+    expect(chargeAuthStub.calledWith(sinon.match( // eslint-disable-line
+      {
+        chargeId: chargeId,
+        payload: payload
       }
     ))).to.be.true // eslint-disable-line
   })