diff --git a/app/controllers/simplified-account/settings/api-keys/create/create-api-key.controller.js b/app/controllers/simplified-account/settings/api-keys/create/create-api-key.controller.js index 2fd91254c..ee0d1999e 100644 --- a/app/controllers/simplified-account/settings/api-keys/create/create-api-key.controller.js +++ b/app/controllers/simplified-account/settings/api-keys/create/create-api-key.controller.js @@ -2,6 +2,8 @@ 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', { @@ -9,8 +11,31 @@ async function get (req, res) { }) } +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, diff --git a/app/controllers/simplified-account/settings/api-keys/create/create-api-key.controller.test.js b/app/controllers/simplified-account/settings/api-keys/create/create-api-key.controller.test.js index 4b8b7e968..126c8d65b 100644 --- a/app/controllers/simplified-account/settings/api-keys/create/create-api-key.controller.test.js +++ b/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') + }) + }) }) }) }) diff --git a/app/utils/simplified-account/settings/service-settings.js b/app/utils/simplified-account/settings/service-settings.js index 8ad3b1afd..f70108b9e 100644 --- a/app/utils/simplified-account/settings/service-settings.js +++ b/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 diff --git a/app/views/simplified-account/settings/api-keys/api-key-name.njk b/app/views/simplified-account/settings/api-keys/api-key-name.njk index fd39fc0fc..8b236456d 100644 --- a/app/views/simplified-account/settings/api-keys/api-key-name.njk +++ b/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: { diff --git a/test/cypress/integration/simplified-account/service-settings/api-keys/api-keys.cy.js b/test/cypress/integration/simplified-account/service-settings/api-keys/api-keys.cy.js index 12955b5d7..ddb2e5552 100644 --- a/test/cypress/integration/simplified-account/service-settings/api-keys/api-keys.cy.js +++ b/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')