Merge pull request #2492 from alphagov/PP-7445-upgrade-previous-url-s…

…tructure

PP-7445 Update accounts URL utility

app/controllers/my-services/post-index.controller.js

@@ -9,13 +9,15 @@ const validAccountId = (accountId, user) => {
 }
 
 module.exports = (req, res) => {
-  let newAccountId = _.get(req, 'body.gatewayAccountId')
+  const gatewayAccountId = req.body && req.body.gatewayAccountId
+  const gatewayAccountExternalId = req.body && req.body.gatewayAccountExternalId
 
-  if (validAccountId(newAccountId, req.user)) {
-    req.gateway_account.currentGatewayAccountId = newAccountId
+  if (validAccountId(gatewayAccountId, req.user)) {
+    req.gateway_account.currentGatewayAccountId = gatewayAccountId
+    req.gateway_account.currentGatewayAccountExternalId = gatewayAccountExternalId
     res.redirect(302, paths.dashboard.index)
   } else {
-    logger.warn(`Attempted to switch to invalid account ${newAccountId}`)
+    logger.warn(`Attempted to switch to invalid account ${gatewayAccountId}`)
     res.redirect(302, paths.serviceSwitcher.index)
   }
 }

app/models/GatewayAccount.class.js

@@ -23,6 +23,7 @@ class GatewayAccount {
    **/
   constructor (gatewayAccountData) {
     this.id = gatewayAccountData.gateway_account_id
+    this.external_id = gatewayAccountData.external_id
     this.name = gatewayAccountData.service_name
     this.type = gatewayAccountData.type
     this.paymentProvider = gatewayAccountData.payment_provider
@@ -39,7 +40,7 @@ class GatewayAccount {
     // until we have external ids for card accounts, the external id is the internal one
     return {
       id: this.id,
-      external_id: this.id,
+      external_id: this.external_id,
       payment_provider: this.paymentProvider,
       service_name: this.name,
       type: this.type

app/routes.js

@@ -7,6 +7,7 @@ const logger = require('./utils/logger')(__filename)
 const response = require('./utils/response.js').response
 const generateRoute = require('./utils/generate-route')
 const paths = require('./paths.js')
+const accountUrls = require('./utils/gateway-account-urls')
 
 const userIsAuthorised = require('./middleware/user-is-authorised')
 const getServiceAndAccount = require('./middleware/get-service-and-gateway-account.middleware')
@@ -488,6 +489,18 @@ module.exports.bind = function (app) {
   app.use(paths.account.root, account)
 
   app.all('*', (req, res) => {
+    const currentSessionAccountExternalId = req.gateway_account && req.gateway_account.currentGatewayAccountExternalId
+    if (accountUrls.isLegacyAccountsUrl(req.url) && currentSessionAccountExternalId) {
+      const upgradedPath = accountUrls.getUpgradedAccountStructureUrl(req.url, currentSessionAccountExternalId)
+      logger.info('Accounts URL utility upgraded a request to a legacy account URL', {
+        url: req.originalUrl,
+        redirected_url: upgradedPath,
+        session_has_user: !!req.user,
+        is_internal_user: req.user && req.user.internalUser
+      })
+      res.redirect(upgradedPath)
+      return
+    }
     logger.info('Page not found', {
       url: req.originalUrl
     })

app/utils/flatten-nested-values.js

@@ -0,0 +1,12 @@
+'use strict'
+function flattenNestedValues (target) {
+  return Object.values(target).reduce((aggregate, value) => {
+    const valueIsNestedObject = typeof value === 'object' && value !== null
+    if (valueIsNestedObject) {
+      return [ ...aggregate, ...flattenNestedValues(value) ]
+    }
+    return [ ...aggregate, value ]
+  }, [])
+}
+
+module.exports = flattenNestedValues

app/utils/flatten-nested-values.test.js

@@ -0,0 +1,19 @@
+const { expect } = require('chai')
+const flattenNestedValues = require('./flatten-nested-values')
+describe('flatten nested values utility', () => {
+  it('correctly flattens nested values', () => {
+    const nested = {
+      one: {
+        two: {
+          index: 'path-1',
+          secondPage: 'path-2'
+        },
+        three: 'path-3'
+      },
+      four: 'path-4'
+    }
+    const flat = flattenNestedValues(nested)
+    expect(flat.length).to.equal(4)
+    expect(flat).to.have.members(['path-1', 'path-2', 'path-3', 'path-4'])
+  })
+})

app/utils/gateway-account-urls.js

@@ -0,0 +1,47 @@
+'use strict'
+// check if a missed URL (404) is a URL that has been upgraded during the
+// account URL structure change. When this utility is reporting few or no
+// upgrades it can be removed
+const urlJoin = require('url-join')
+const paths = require('../paths')
+const formattedPathFor = require('./replace-params-in-path')
+const flattenNestedValues = require('./flatten-nested-values')
+
+// only flatten paths once given the singleton module export patten, these
+// should never change after the server spins up
+const allAccountPaths = flattenNestedValues(paths.account)
+const templatedAccountPaths = allAccountPaths.filter((path) => path.includes(':'))
+
+const removeEmptyValues = (value) => !!value
+
+function isLegacyAccountsUrl (url) {
+  if (allAccountPaths.includes(url)) {
+    return true
+  } else {
+    // the path isn't directly in the list, check to see if it's a templated value
+    const numberOfUrlParts = url.split('/').filter(removeEmptyValues).length
+    return templatedAccountPaths.some((templatedPath) => {
+      const parts = templatedPath.split('/').filter(removeEmptyValues)
+      const matches = parts
+
+        // remove variable sections
+        .filter((part) => !part.startsWith(':'))
+
+        // ensure every part of the url structure is present in the url we're comparing against
+        .every((part) => url.includes(part))
+
+      // verify it matches and is not a subset (has less length)
+      return matches && parts.length === numberOfUrlParts
+    })
+  }
+}
+
+function getUpgradedAccountStructureUrl (url, gatewayAccountExternalId) {
+  const base = formattedPathFor(paths.account.root, gatewayAccountExternalId)
+  return urlJoin(base, url)
+}
+
+module.exports = {
+  isLegacyAccountsUrl,
+  getUpgradedAccountStructureUrl
+}

app/utils/gateway-account-urls.js.test.js

@@ -0,0 +1,16 @@
+const { expect } = require('chai')
+
+const accountsUrl = require('./gateway-account-urls')
+describe('account URL checker', () => {
+  it('correctly identifies an original account URL', () => {
+    const url = '/billing-address'
+    const result = accountsUrl.isLegacyAccountsUrl(url)
+    expect(result).to.be.true //eslint-disable-line
+  })
+
+  it('correctly upgrades a URL to the account structure', () => {
+    const url = '/create-payment-link/manage/some-product-external-id/add-reporting-column/some-metadata-key'
+    const gatewayAccountExternalId = 'some-account-external-id'
+    expect(accountsUrl.getUpgradedAccountStructureUrl(url, gatewayAccountExternalId)).to.equal('/account/some-account-external-id/create-payment-link/manage/some-product-external-id/add-reporting-column/some-metadata-key')
+  })
+})

app/views/services/_service-switch.njk

@@ -1,7 +1,8 @@
 <li class="service--{{ account.type }}">
   <form method="post" action="{{routes.serviceSwitcher.switch}}">
     <input name="csrfToken" type="hidden" value="{{csrf}}" />
-    <input name="gatewayAccountId" type="hidden" value="{{ account.external_id }}"/>
+    <input name="gatewayAccountId" type="hidden" value="{{ account.id }}"/>
+    <input name="gatewayAccountExternalId" type="hidden" value="{{ account.external_id }}"/>
     <button
       class="govuk-link pay-button--as-link service-switcher {{ account.type }} {{ account.payment_provider }}"
       type="submit"

test/integration/routes.it.test.js

@@ -0,0 +1,22 @@
+const request = require('supertest')
+
+const { getApp } = require('../../server')
+const session = require('../test-helpers/mock-session.js')
+const app = session.getAppWithLoggedInUser(getApp(), session.getUser())
+
+describe('URL upgrade utility', () => {
+  it('correctly upgrades URLs in the account specific paths', () => {
+    return request(app)
+      .get('/billing-address')
+      .expect(302)
+      .then((res) => {
+        res.header['location'].should.include('/account/external-id-set-by-create-app-with-session/billing-address') // eslint-disable-line
+      })
+  })
+
+  it('correctly 404s as expected for non account specific paths', () => {
+    return request(app)
+      .get('/unknown-address')
+      .expect(404)
+  })
+})

test/test-helpers/mock-session.js

@@ -18,7 +18,8 @@ const createAppWithSession = function (app, sessionData, gatewayAccountCookie, r
     req.session = req.session || sessionData || {}
     req.register_invite = registerInviteData || {}
     req.gateway_account = gatewayAccountCookie || {
-      currentGatewayAccountId: _.get(sessionData, 'passport.user.serviceRoles[0].service.gatewayAccountIds[0]')
+      currentGatewayAccountId: _.get(sessionData, 'passport.user.serviceRoles[0].service.gatewayAccountIds[0]'),
+      currentGatewayAccountExternalId: 'external-id-set-by-create-app-with-session'
     }
 
     next()

test/unit/controller/service-switch.controller.it.test.js

@@ -155,13 +155,15 @@ describe('service switch controller: switching', function () {
       session: session,
       gateway_account: gatewayAccount,
       body: {
-        gatewayAccountId: '6'
+        gatewayAccountId: '6',
+        gatewayAccountExternalId: 'some-external-id'
       }
     }
 
     const res = {
       redirect: function () {
         expect(gatewayAccount.currentGatewayAccountId).to.be.equal('6')
+        expect(gatewayAccount.currentGatewayAccountExternalId).to.be.equal('some-external-id')
         expect(arguments[0]).to.equal(302)
         expect(arguments[1]).to.equal('/')
       }