From c2a8d0c0a543746016b39dff6e2c9332d47958fb Mon Sep 17 00:00:00 2001 From: Oswald Quek Date: Wed, 18 Dec 2024 16:50:52 +0000 Subject: [PATCH 1/2] PP-12395: Create API key functionality Creating an API key consists of two pages: the "API key name" and "New API key" pages. Validation of the name/description and cypress test will be done in the next PRs. --- app/assets/sass/components/api-keys.scss | 6 ++ .../settings/api-keys/api-keys.controller.js | 5 +- .../create/create-api-key.controller.js | 22 +++++ .../create/create-api-key.controller.test.js | 85 +++++++++++++++++++ app/paths.js | 3 +- app/services/api-keys.service.js | 29 ++++++- app/simplified-account-routes.js | 2 + .../settings/service-settings.js | 3 +- .../settings/api-keys/api-key-name.njk | 34 ++++++++ .../settings/api-keys/new-api-key-details.njk | 69 +++++++++++++++ .../service-settings/api-keys/api-keys.cy.js | 14 +++ 11 files changed, 268 insertions(+), 4 deletions(-) create mode 100644 app/controllers/simplified-account/settings/api-keys/create/create-api-key.controller.js create mode 100644 app/controllers/simplified-account/settings/api-keys/create/create-api-key.controller.test.js create mode 100644 app/views/simplified-account/settings/api-keys/api-key-name.njk create mode 100644 app/views/simplified-account/settings/api-keys/new-api-key-details.njk diff --git a/app/assets/sass/components/api-keys.scss b/app/assets/sass/components/api-keys.scss index 4afc3ad9be..35f1913a3a 100644 --- a/app/assets/sass/components/api-keys.scss +++ b/app/assets/sass/components/api-keys.scss @@ -25,3 +25,9 @@ border-top: 1px solid $govuk-border-colour; padding: govuk-spacing(6) 0; } + +.key-prefix { + background-color: #f3f2f1; + color: #f47738; + padding: 5px; +} diff --git a/app/controllers/simplified-account/settings/api-keys/api-keys.controller.js b/app/controllers/simplified-account/settings/api-keys/api-keys.controller.js index 67f7b73f37..f09b55ed81 100644 --- a/app/controllers/simplified-account/settings/api-keys/api-keys.controller.js +++ b/app/controllers/simplified-account/settings/api-keys/api-keys.controller.js @@ -1,14 +1,17 @@ const { response } = require('@utils/response') const apiKeysService = require('@services/api-keys.service') +const formatSimplifiedAccountPathsFor = require('@utils/simplified-account/format/format-simplified-account-paths-for') +const paths = require('@root/paths') async function get (req, res) { const activeKeys = await apiKeysService.getActiveKeys(req.account.id) return response(req, res, 'simplified-account/settings/api-keys/index', { accountType: req.account.type, activeKeys, - createApiKeyLink: '#', + createApiKeyLink: formatSimplifiedAccountPathsFor(paths.simplifiedAccount.settings.apiKeys.create, req.service.externalId, req.account.type), showRevokedKeysLink: '#' }) } module.exports = { get } +module.exports.createApiKey = require('./create/create-api-key.controller') 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 new file mode 100644 index 0000000000..2fd91254c1 --- /dev/null +++ b/app/controllers/simplified-account/settings/api-keys/create/create-api-key.controller.js @@ -0,0 +1,22 @@ +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') + +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) + }) +} + +async function post (req, res) { + const description = req.body.description // TODO validate length - deal with this in another PR + 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, + apiKey: newApiKey, + backToApiKeysLink: formatSimplifiedAccountPathsFor(paths.simplifiedAccount.settings.apiKeys.index, req.service.externalId, req.account.type) + }) +} + +module.exports = { get, post } 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 new file mode 100644 index 0000000000..c9c3ef9651 --- /dev/null +++ b/app/controllers/simplified-account/settings/api-keys/create/create-api-key.controller.test.js @@ -0,0 +1,85 @@ +const ControllerTestBuilder = require('@test/test-helpers/simplified-account/controllers/ControllerTestBuilder.class') +const sinon = require('sinon') +const { expect } = require('chai') +const formatSimplifiedAccountPathsFor = require('@utils/simplified-account/format/format-simplified-account-paths-for') +const paths = require('@root/paths') +const { TOKEN_SOURCE } = require('@services/api-keys.service') + +const ACCOUNT_TYPE = 'live' +const SERVICE_ID = 'service-id-123abc' +const mockResponse = sinon.spy() +const newApiKey = 'api_test_123' // pragma: allowlist secret +const apiKeysService = { + createApiKey: sinon.stub().resolves(newApiKey), + TOKEN_SOURCE +} + +const { + req, + res, + nextRequest, + call +} = new ControllerTestBuilder('@controllers/simplified-account/settings/api-keys/create/create-api-key.controller') + .withServiceExternalId(SERVICE_ID) + .withAccountType(ACCOUNT_TYPE) + .withStubs({ + '@utils/response': { response: mockResponse }, + '@services/api-keys.service': apiKeysService + }) + .build() + +describe('Controller: settings/create-api-key', () => { + describe('get', () => { + before(() => { + call('get') + }) + + it('should call the response method', () => { + expect(mockResponse).to.have.been.calledOnce // eslint-disable-line + }) + + it('should pass req, res and template path to the response method', () => { + expect(mockResponse).to.have.been.calledWith(req, res, 'simplified-account/settings/api-keys/api-key-name') + }) + + it('should pass context data to the response method', () => { + expect(mockResponse.args[0][3]).to.have.property('backLink').to.equal( + formatSimplifiedAccountPathsFor(paths.simplifiedAccount.settings.apiKeys.index, SERVICE_ID, ACCOUNT_TYPE)) + }) + }) + + describe('post', () => { + before(() => { + nextRequest({ + body: { + description: 'a test api key' + }, + user: { + email: 'potter@wand.com' + } + }) + 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 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') + }) + }) +}) diff --git a/app/paths.js b/app/paths.js index a370acbc0d..366d2ae042 100644 --- a/app/paths.js +++ b/app/paths.js @@ -224,7 +224,8 @@ module.exports = { index: '/settings/card-types' }, apiKeys: { - index: '/settings/api-keys' + index: '/settings/api-keys', + create: '/settings/api-keys/create' }, webhooks: { index: '/settings/webhooks' diff --git a/app/services/api-keys.service.js b/app/services/api-keys.service.js index a7fc52917e..e7aa23918b 100644 --- a/app/services/api-keys.service.js +++ b/app/services/api-keys.service.js @@ -1,6 +1,31 @@ const publicAuthClient = require('@services/clients/public-auth.client') const { Token } = require('@models/Token.class') +const TOKEN_SOURCE = { + API: 'API', + PRODUCTS: 'PRODUCTS' +} + +/** + * @param {GatewayAccount} gatewayAccount + * @param {string} description + * @param {string} email - the user email + * @param {'API' | 'PRODUCTS'} tokenSource - The type of the token (must match one of TOKEN_TYPE values). + * @returns {string} the new api key + */ +const createApiKey = async (gatewayAccount, description, email, tokenSource) => { + const payload = { + description, + account_id: gatewayAccount.id, + created_by: email, + type: tokenSource, + token_type: 'CARD', + token_account_type: gatewayAccount.type + } + const response = await publicAuthClient.createTokenForAccount({ payload }) + return response.token +} + /** * Gets the list of active api keys for a gateway account * @param {string} gatewayAccountId @@ -14,5 +39,7 @@ const getActiveKeys = async (gatewayAccountId) => { } module.exports = { - getActiveKeys + createApiKey, + getActiveKeys, + TOKEN_SOURCE } diff --git a/app/simplified-account-routes.js b/app/simplified-account-routes.js index a6f92da681..6353578dc4 100644 --- a/app/simplified-account-routes.js +++ b/app/simplified-account-routes.js @@ -74,6 +74,8 @@ simplifiedAccount.get(paths.simplifiedAccount.settings.cardTypes.index, permissi // api keys simplifiedAccount.get(paths.simplifiedAccount.settings.apiKeys.index, permission('tokens-active:read'), serviceSettingsController.apiKeys.get) +simplifiedAccount.get(paths.simplifiedAccount.settings.apiKeys.create, permission('tokens:create'), serviceSettingsController.apiKeys.createApiKey.get) +simplifiedAccount.post(paths.simplifiedAccount.settings.apiKeys.create, permission('tokens:create'), serviceSettingsController.apiKeys.createApiKey.post) // stripe details const stripeDetailsPath = paths.simplifiedAccount.settings.stripeDetails diff --git a/app/utils/simplified-account/settings/service-settings.js b/app/utils/simplified-account/settings/service-settings.js index e7deb65595..8ad3b1afd8 100644 --- a/app/utils/simplified-account/settings/service-settings.js +++ b/app/utils/simplified-account/settings/service-settings.js @@ -72,7 +72,8 @@ module.exports = (account, service, currentUrl, permissions) => { id: 'api-keys', name: 'API keys', path: paths.simplifiedAccount.settings.apiKeys.index, - permission: 'tokens_active_read' + permission: 'tokens_active_read', + alwaysViewable: true }) .add({ id: 'webhooks', 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 new file mode 100644 index 0000000000..fd39fc0fce --- /dev/null +++ b/app/views/simplified-account/settings/api-keys/api-key-name.njk @@ -0,0 +1,34 @@ +{% extends "../settings-layout.njk" %} + +{% block settingsPageTitle %} + API key name +{% endblock %} + +{% block settingsContent %} + +

API key name

+ +
+ + + {{ govukInput({ + id: "description", + name: "description", + type: "text", + attributes: { + maxlength: "100" + }, + classes: "govuk-input--width-40", + label: { + text: "Add a description for the key" + }, + hint: { + text: "For example, “John Smith’s API key”" + } + }) }} + + {{ govukButton({ + text: "Continue" + }) }} +
+{% endblock %} diff --git a/app/views/simplified-account/settings/api-keys/new-api-key-details.njk b/app/views/simplified-account/settings/api-keys/new-api-key-details.njk new file mode 100644 index 0000000000..82e18217f8 --- /dev/null +++ b/app/views/simplified-account/settings/api-keys/new-api-key-details.njk @@ -0,0 +1,69 @@ +{% extends "../settings-layout.njk" %} + +{% block settingsPageTitle %} + New API key +{% endblock %} + +{% block settingsContent %} + + {{ govukBackLink({ + text: "Back to API keys", + href: backToApiKeysLink + }) }} + +

New API key

+ + {% set html %} +

+ Copy your key to somewhere safe.

You won’t be able to see it again once you leave this page. +

+ {% endset %} + + {{ govukInsetText({ + html: html + }) }} + +

+ You must copy the whole key, including the api_test_ or api_live_ prefix. +

+ + {{ govukWarningText( + { + text: 'API security', + iconFallbackText: 'Warning' + } + ) }} + + + +

+ More about + securing your API keys. +

+ +

+ {{ description }} +

+
+ {{ apiKey }} +
+ + {{ govukButton({ + text: "Copy API key to clipboard", + classes: "govuk-button--secondary govuk-!-margin-top-6", + attributes: { + id: "generate-button", + "data-copy-text": true, + "data-target": "copy-this-api-key", + "data-success": "API key has been copied", + "data-notification-target": "copy-this-api-key-notification" + } + }) }} + +
+ +{% endblock %} 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 52479dac80..12955b5d7a 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 @@ -85,6 +85,20 @@ describe('Settings - API keys', () => { .within(() => { cy.get('.govuk-summary-card__title').should('contain', token.description) + cy.get('.govuk-summary-card__action').eq(0) + .within(() => { + cy.get('a') + .should('contain.text', 'Change name') + .and('have.attr', 'href', '#') + }) + + cy.get('.govuk-summary-card__action').eq(1) + .within(() => { + cy.get('a') + .should('contain.text', 'Revoke') + .and('have.attr', 'href', '#') + }) + cy.get('.govuk-summary-list__row').eq(0) .within(() => { cy.get('.govuk-summary-list__key').should('contain', 'Created by') From 22f5f0d6f8d5bad446c56eaa934bbd9976a9eaeb Mon Sep 17 00:00:00 2001 From: Oswald Quek Date: Fri, 20 Dec 2024 15:16:49 +0000 Subject: [PATCH 2/2] [squash] fix review comments --- .../api-keys/create/create-api-key.controller.test.js | 10 +++------- app/services/api-keys.service.js | 2 +- 2 files changed, 4 insertions(+), 8 deletions(-) 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 c9c3ef9651..4b8b7e9682 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 @@ -38,13 +38,9 @@ describe('Controller: settings/create-api-key', () => { expect(mockResponse).to.have.been.calledOnce // eslint-disable-line }) - it('should pass req, res and template path to the response method', () => { - expect(mockResponse).to.have.been.calledWith(req, res, 'simplified-account/settings/api-keys/api-key-name') - }) - - it('should pass context data to the response method', () => { - expect(mockResponse.args[0][3]).to.have.property('backLink').to.equal( - formatSimplifiedAccountPathsFor(paths.simplifiedAccount.settings.apiKeys.index, SERVICE_ID, ACCOUNT_TYPE)) + it('should pass req, res, template path and context to the response method', () => { + expect(mockResponse).to.have.been.calledWith(req, res, 'simplified-account/settings/api-keys/api-key-name', + { backLink: formatSimplifiedAccountPathsFor(paths.simplifiedAccount.settings.apiKeys.index, SERVICE_ID, ACCOUNT_TYPE) }) }) }) diff --git a/app/services/api-keys.service.js b/app/services/api-keys.service.js index e7aa23918b..ebb396ab59 100644 --- a/app/services/api-keys.service.js +++ b/app/services/api-keys.service.js @@ -11,7 +11,7 @@ const TOKEN_SOURCE = { * @param {string} description * @param {string} email - the user email * @param {'API' | 'PRODUCTS'} tokenSource - The type of the token (must match one of TOKEN_TYPE values). - * @returns {string} the new api key + * @returns {Promise} the new api key */ const createApiKey = async (gatewayAccount, description, email, tokenSource) => { const payload = {