PP-12395: Validation when creating API key

app/controllers/simplified-account/settings/api-keys/create/create-api-key.controller.js

@@ -2,15 +2,40 @@ const { response } = require('@utils/response')
 const formatSimplifiedAccountPathsFor = require('@utils/simplified-account/format/format-simplified-account-paths-for')
 const paths = require('@root/paths')
 const { TOKEN_SOURCE, createApiKey } = require('@services/api-keys.service')
+const { body, validationResult } = require('express-validator')
+const formatValidationErrors = require('@utils/simplified-account/format/format-validation-errors')
 
 async function get (req, res) {
   return response(req, res, 'simplified-account/settings/api-keys/api-key-name', {
     backLink: formatSimplifiedAccountPathsFor(paths.simplifiedAccount.settings.apiKeys.index, req.service.externalId, req.account.type)
   })
 }
 
+const descriptionValidation = [
+  body('description')
+    .trim()
+    .notEmpty()
+    .withMessage('Name must not be empty')
+    .isLength({ max: 50 })
+    .withMessage('Name must be 50 characters or fewer')
+]
+
 async function post (req, res) {
-  const description = req.body.description // TODO validate length - deal with this in another PR
+  await Promise.all(descriptionValidation.map(validation => validation.run(req)))
+  const errors = validationResult(req)
+
+  if (!errors.isEmpty()) {
+    const formattedErrors = formatValidationErrors(errors)
+    return response(req, res, 'simplified-account/settings/api-keys/api-key-name', {
+      errors: {
+        summary: formattedErrors.errorSummary,
+        formErrors: formattedErrors.formErrors
+      },
+      backLink: formatSimplifiedAccountPathsFor(paths.simplifiedAccount.settings.apiKeys.index, req.service.externalId, req.account.type)
+    })
+  }
+
+  const description = req.body.description
   const newApiKey = await createApiKey(req.account, description, req.user.email, TOKEN_SOURCE.API)
   response(req, res, 'simplified-account/settings/api-keys/new-api-key-details', {
     description,

app/controllers/simplified-account/settings/api-keys/create/create-api-key.controller.test.js

@@ -45,37 +45,94 @@ describe('Controller: settings/create-api-key', () => {
   })
 
   describe('post', () => {
-    before(() => {
-      nextRequest({
-        body: {
-          description: 'a test api key'
-        },
-        user: {
-          email: 'potter@wand.com'
-        }
+    describe('a valid description', () => {
+      before(() => {
+        nextRequest({
+          body: {
+            description: 'a test api key'
+          },
+          user: {
+            email: 'potter@wand.com'
+          }
+        })
+        call('post')
       })
-      call('post')
-    })
 
-    it('should submit values to the api keys service', () => {
-      expect(apiKeysService.createApiKey).to.have.been.calledWith(
-        sinon.match.any,
-        'a test api key',
-        'potter@wand.com',
-        TOKEN_SOURCE.API
-      )
-    })
+      it('should submit values to the api keys service', () => {
+        expect(apiKeysService.createApiKey).to.have.been.calledWith(
+          sinon.match.any,
+          'a test api key',
+          'potter@wand.com',
+          TOKEN_SOURCE.API
+        )
+      })
 
-    it('should call the response method', () => {
-      expect(mockResponse).to.have.been.calledOnce // eslint-disable-line
+      it('should call the response method', () => {
+        expect(mockResponse).to.have.been.calledOnce // eslint-disable-line
+      })
+
+      it('should pass context data to the response method', () => {
+        expect(mockResponse.args[0][2]).to.equal('simplified-account/settings/api-keys/new-api-key-details')
+        expect(mockResponse.args[0][3]).to.have.property('backToApiKeysLink').to.equal(
+          formatSimplifiedAccountPathsFor(paths.simplifiedAccount.settings.apiKeys.index, SERVICE_ID, ACCOUNT_TYPE))
+        expect(mockResponse.args[0][3]).to.have.property('apiKey').to.equal(newApiKey)
+        expect(mockResponse.args[0][3]).to.have.property('description').to.equal('a test api key')
+      })
     })
 
-    it('should pass context data to the response method', () => {
-      expect(mockResponse.args[0][2]).to.equal('simplified-account/settings/api-keys/new-api-key-details')
-      expect(mockResponse.args[0][3]).to.have.property('backToApiKeysLink').to.equal(
-        formatSimplifiedAccountPathsFor(paths.simplifiedAccount.settings.apiKeys.index, SERVICE_ID, ACCOUNT_TYPE))
-      expect(mockResponse.args[0][3]).to.have.property('apiKey').to.equal(newApiKey)
-      expect(mockResponse.args[0][3]).to.have.property('description').to.equal('a test api key')
+    describe('an invalid description', () => {
+      function assertMockResponseArgs (errorMessage) {
+        expect(mockResponse.calledOnce).to.be.true // eslint-disable-line
+        expect(mockResponse.args[0][2]).to.equal('simplified-account/settings/api-keys/api-key-name')
+        expect(mockResponse.args[0][3].errors.summary[0].text).to.equal(errorMessage)
+        expect(mockResponse.args[0][3].errors.formErrors.description).to.equal(errorMessage)
+        expect(mockResponse.args[0][3].backLink).to.equal(
+          formatSimplifiedAccountPathsFor(paths.simplifiedAccount.settings.apiKeys.index, SERVICE_ID, ACCOUNT_TYPE))
+      }
+
+      describe('empty description', () => {
+        before(() => {
+          nextRequest({
+            body: {
+              description: ''
+            },
+            user: {
+              email: 'potter@wand.com'
+            }
+          })
+          call('post')
+        })
+
+        it('should not call apiKeysService.createApiKey', () => {
+          sinon.assert.notCalled(apiKeysService.createApiKey)
+        })
+
+        it('should pass req, res, template path and context to the response method', () => {
+          assertMockResponseArgs('Name must not be empty')
+        })
+      })
+
+      describe('description more than 50 chars', () => {
+        before(() => {
+          nextRequest({
+            body: {
+              description: 'more than fifty chars more than fifty chars more than fifty chars more than fifty chars'
+            },
+            user: {
+              email: 'potter@wand.com'
+            }
+          })
+          call('post')
+        })
+
+        it('should not call apiKeysService.createApiKey', () => {
+          sinon.assert.notCalled(apiKeysService.createApiKey)
+        })
+
+        it('should pass req, res, template path and context to the response method', () => {
+          assertMockResponseArgs('Name must be 50 characters or fewer')
+        })
+      })
     })
   })
 })

app/utils/simplified-account/settings/service-settings.js

@@ -70,7 +70,7 @@ module.exports = (account, service, currentUrl, permissions) => {
     .category('developers')
     .add({
       id: 'api-keys',
-      name: 'API keys',
+      name: account.type === 'test' ? 'Test API keys' : 'Live API keys',
       path: paths.simplifiedAccount.settings.apiKeys.index,
       permission: 'tokens_active_read',
       alwaysViewable: true

app/views/simplified-account/settings/api-keys/api-key-name.njk

@@ -16,7 +16,7 @@
       name: "description",
       type: "text",
       attributes: {
-        maxlength: "100"
+        maxlength: "50"
       },
       classes: "govuk-input--width-40",
       label: {

test/cypress/integration/simplified-account/service-settings/api-keys/api-keys.cy.js

@@ -36,7 +36,7 @@ describe('Settings - API keys', () => {
         cy.visit(`/simplified/service/${SERVICE_EXTERNAL_ID}/account/${ACCOUNT_TYPE}/settings/api-keys`)
       })
       it('should show appropriate buttons and text', () => {
-        cy.get('#api-keys').should('have.text', 'API keys')
+        cy.get('#api-keys').should('have.text', 'Test API keys')
         cy.get('.service-settings-pane')
           .find('a')
           .contains('Create a new API key')
@@ -62,7 +62,7 @@ describe('Settings - API keys', () => {
         cy.visit(`/simplified/service/${SERVICE_EXTERNAL_ID}/account/${ACCOUNT_TYPE}/settings/api-keys`)
       })
       it('should show appropriate buttons and text', () => {
-        cy.get('#api-keys').should('have.text', 'API keys')
+        cy.get('#api-keys').should('have.text', 'Test API keys')
         cy.get('.service-settings-pane')
           .find('a')
           .contains('Create a new API key')