From 74847f3da3ca2bbd12a5274f13e4ca08760f9367 Mon Sep 17 00:00:00 2001 From: Oswald Quek Date: Fri, 3 Jan 2025 12:47:12 +0000 Subject: [PATCH] PP-12395: Revoke key functionality --- .../settings/api-keys/api-keys.controller.js | 3 + .../api-keys/api-keys.controller.test.js | 3 +- .../api-keys/revoke/revoke.controller.js | 22 ++++ .../api-keys/revoke/revoke.controller.test.js | 121 ++++++++++++++++++ app/paths.js | 3 +- app/services/api-keys.service.js | 23 ++++ app/services/clients/public-auth.client.js | 14 ++ app/simplified-account-routes.js | 2 + .../settings/api-keys/index.njk | 2 +- .../settings/api-keys/revoke.njk | 38 ++++++ .../service-settings/api-keys/api-keys.cy.js | 3 +- 11 files changed, 230 insertions(+), 4 deletions(-) create mode 100644 app/controllers/simplified-account/settings/api-keys/revoke/revoke.controller.js create mode 100644 app/controllers/simplified-account/settings/api-keys/revoke/revoke.controller.test.js create mode 100644 app/views/simplified-account/settings/api-keys/revoke.njk 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 433f950e7..e93b5bf00 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 @@ -11,6 +11,8 @@ async function get (req, res) { return { ...activeKey, changeNameLink: formatSimplifiedAccountPathsFor(paths.simplifiedAccount.settings.apiKeys.changeName, + req.service.externalId, req.account.type, activeKey.tokenLink), + revokeKeyLink: formatSimplifiedAccountPathsFor(paths.simplifiedAccount.settings.apiKeys.revoke, req.service.externalId, req.account.type, activeKey.tokenLink) } }), @@ -23,3 +25,4 @@ async function get (req, res) { module.exports = { get } module.exports.createApiKey = require('./create/create-api-key.controller') module.exports.changeName = require('./change-name/change-name.controller') +module.exports.revoke = require('./revoke/revoke.controller') diff --git a/app/controllers/simplified-account/settings/api-keys/api-keys.controller.test.js b/app/controllers/simplified-account/settings/api-keys/api-keys.controller.test.js index be8f379ca..8ac90f314 100644 --- a/app/controllers/simplified-account/settings/api-keys/api-keys.controller.test.js +++ b/app/controllers/simplified-account/settings/api-keys/api-keys.controller.test.js @@ -43,7 +43,8 @@ describe('Controller: settings/api-keys', () => { apiKeys.map(apiKey => { return { ...apiKey, - changeNameLink: `/simplified/service/${SERVICE_ID}/account/${ACCOUNT_TYPE}/settings/api-keys/change-name/${apiKeys[0].tokenLink}` + changeNameLink: `/simplified/service/${SERVICE_ID}/account/${ACCOUNT_TYPE}/settings/api-keys/change-name/${apiKeys[0].tokenLink}`, + revokeKeyLink: `/simplified/service/${SERVICE_ID}/account/${ACCOUNT_TYPE}/settings/api-keys/revoke/${apiKeys[0].tokenLink}` } })) }) diff --git a/app/controllers/simplified-account/settings/api-keys/revoke/revoke.controller.js b/app/controllers/simplified-account/settings/api-keys/revoke/revoke.controller.js new file mode 100644 index 000000000..b0d416c88 --- /dev/null +++ b/app/controllers/simplified-account/settings/api-keys/revoke/revoke.controller.js @@ -0,0 +1,22 @@ +const paths = require('@root/paths') +const formatSimplifiedAccountPathsFor = require('@utils/simplified-account/format/format-simplified-account-paths-for') +const { response } = require('@utils/response') +const { getKeyByTokenLink, revokeKey } = require('@services/api-keys.service') + +async function get (req, res) { + const tokenLink = req.params.tokenLink + const apiKey = await getKeyByTokenLink(req.account.id, tokenLink) + return response(req, res, 'simplified-account/settings/api-keys/revoke', { + description: apiKey.description, + backLink: formatSimplifiedAccountPathsFor(paths.simplifiedAccount.settings.apiKeys.index, req.service.externalId, req.account.type) + }) +} + +async function post (req, res) { + if (req.body.revokeApiKey === 'Yes') { // pragma: allowlist secret + await revokeKey(req.account.id, req.params.tokenLink) + } + res.redirect(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/revoke/revoke.controller.test.js b/app/controllers/simplified-account/settings/api-keys/revoke/revoke.controller.test.js new file mode 100644 index 000000000..a5fd55b42 --- /dev/null +++ b/app/controllers/simplified-account/settings/api-keys/revoke/revoke.controller.test.js @@ -0,0 +1,121 @@ +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 GatewayAccount = require('@models/GatewayAccount.class') + +const ACCOUNT_TYPE = 'live' +const SERVICE_ID = 'service-id-123abc' +const TOKEN_LINK = '550e8400-e29b-41d4-a716-446655440000' +const GATEWAY_ACCOUNT_ID = 1 +const mockResponse = sinon.spy() +const token = { + description: 'token description', + token_link: TOKEN_LINK +} +const apiKeysService = { + getKeyByTokenLink: sinon.stub().resolves(token), + revokeKey: sinon.stub().resolves() +} + +const { + req, + res, + nextRequest, + call +} = new ControllerTestBuilder('@controllers/simplified-account/settings/api-keys/revoke/revoke.controller') + .withServiceExternalId(SERVICE_ID) + .withAccount(new GatewayAccount({ + type: ACCOUNT_TYPE, + gateway_account_id: GATEWAY_ACCOUNT_ID + })) + .withStubs({ + '@utils/response': { response: mockResponse }, + '@services/api-keys.service': apiKeysService + }) + .build() + +describe('Controller: settings/api-keys/revoke', () => { + describe('get', () => { + before(() => { + nextRequest({ + params: { + tokenLink: TOKEN_LINK + } + }) + call('get') + }) + + it('should call apiKeysService.getKeyByTokenLink', () => { + expect(apiKeysService.getKeyByTokenLink).to.have.been.calledWith(GATEWAY_ACCOUNT_ID, TOKEN_LINK) + }) + + it('should call the response method', () => { + expect(mockResponse).to.have.been.calledOnce // eslint-disable-line + }) + + it('should pass req, res, template path and context to the response method', () => { + expect(mockResponse).to.have.been.calledWith( + { ...req, params: { tokenLink: TOKEN_LINK } }, + res, + 'simplified-account/settings/api-keys/revoke', + { + description: token.description, + backLink: formatSimplifiedAccountPathsFor(paths.simplifiedAccount.settings.apiKeys.index, SERVICE_ID, ACCOUNT_TYPE) + }) + }) + }) + + describe('post', () => { + describe('when No is selected', () => { + before(() => { + nextRequest({ + body: { + revokeApiKey: 'No' + }, + params: { + tokenLink: TOKEN_LINK + } + }) + call('post') + }) + + it('should not call apiKeysService.revokeKey', () => { + sinon.assert.notCalled(apiKeysService.revokeKey) + }) + + it('should redirect to the api keys index page', () => { + expect(res.redirect.calledOnce).to.be.true // eslint-disable-line + expect(res.redirect.args[0][0]).to.include( + formatSimplifiedAccountPathsFor(paths.simplifiedAccount.settings.apiKeys.index, SERVICE_ID, ACCOUNT_TYPE) + ) + }) + }) + + describe('when Yes is selected', () => { + before(() => { + nextRequest({ + body: { + revokeApiKey: 'Yes' // pragma: allowlist secret + }, + params: { + tokenLink: TOKEN_LINK + } + }) + call('post') + }) + + it('should call apiKeysService.revokeKey', () => { + expect(apiKeysService.revokeKey).to.have.been.calledWith(GATEWAY_ACCOUNT_ID, TOKEN_LINK) + }) + + it('should redirect to the api keys index page', () => { + expect(res.redirect.calledOnce).to.be.true // eslint-disable-line + expect(res.redirect.args[0][0]).to.include( + formatSimplifiedAccountPathsFor(paths.simplifiedAccount.settings.apiKeys.index, SERVICE_ID, ACCOUNT_TYPE) + ) + }) + }) + }) +}) diff --git a/app/paths.js b/app/paths.js index 63555849f..f9921dd82 100644 --- a/app/paths.js +++ b/app/paths.js @@ -230,7 +230,8 @@ module.exports = { apiKeys: { index: '/settings/api-keys', create: '/settings/api-keys/create', - changeName: '/settings/api-keys/change-name/:tokenLink' + changeName: '/settings/api-keys/change-name/:tokenLink', + revoke: '/settings/api-keys/revoke/:tokenLink' }, webhooks: { index: '/settings/webhooks' diff --git a/app/services/api-keys.service.js b/app/services/api-keys.service.js index 4e364a7fc..650b233f0 100644 --- a/app/services/api-keys.service.js +++ b/app/services/api-keys.service.js @@ -47,9 +47,32 @@ const changeApiKeyName = async (tokenLink, name) => { await publicAuthClient.updateToken({ payload: { token_link: tokenLink, description: name } }) } +/** + * @param {string} gatewayAccountId + * @param {string} tokenLink + * @return {Promise<*>} + */ +const getKeyByTokenLink = async (gatewayAccountId, tokenLink) => { + return await publicAuthClient.getKeyByTokenLink(gatewayAccountId, tokenLink) +} + +/** + * @param {string} gatewayAccountId + * @param {string} tokenLink + * @return {Promise<*>} + */ +const revokeKey = async (gatewayAccountId, tokenLink) => { + await publicAuthClient.deleteTokenForAccount({ + accountId: gatewayAccountId, + payload: { token_link: tokenLink } + }) +} + module.exports = { changeApiKeyName, createApiKey, getActiveKeys, + getKeyByTokenLink, + revokeKey, TOKEN_SOURCE } diff --git a/app/services/clients/public-auth.client.js b/app/services/clients/public-auth.client.js index 05d7ee13a..a70ddef71 100644 --- a/app/services/clients/public-auth.client.js +++ b/app/services/clients/public-auth.client.js @@ -118,8 +118,22 @@ async function revokeTokensForAccount (accountId) { await this.client.delete(url, 'revoke all tokens for gateway account') } +/** + * @param {string} accountId + * @param {string} tokenLink + * @return {Promise<*>} + */ +async function getKeyByTokenLink (accountId, tokenLink) { + this.client = new Client(SERVICE_NAME) + const url = `${process.env.PUBLIC_AUTH_URL}/${accountId}/${tokenLink}` + configureClient(this.client, url) + const response = await this.client.get(url, 'get token by token link and account id') + return response.data +} + module.exports = { getActiveTokensForAccount, + getKeyByTokenLink, getRevokedTokensForAccount, createTokenForAccount, updateToken, diff --git a/app/simplified-account-routes.js b/app/simplified-account-routes.js index 168907e53..43ed2e934 100644 --- a/app/simplified-account-routes.js +++ b/app/simplified-account-routes.js @@ -79,6 +79,8 @@ simplifiedAccount.get(paths.simplifiedAccount.settings.apiKeys.create, permissio simplifiedAccount.post(paths.simplifiedAccount.settings.apiKeys.create, permission('tokens:create'), serviceSettingsController.apiKeys.createApiKey.post) simplifiedAccount.get(paths.simplifiedAccount.settings.apiKeys.changeName, permission('tokens:update'), serviceSettingsController.apiKeys.changeName.get) simplifiedAccount.post(paths.simplifiedAccount.settings.apiKeys.changeName, permission('tokens:update'), serviceSettingsController.apiKeys.changeName.post) +simplifiedAccount.get(paths.simplifiedAccount.settings.apiKeys.revoke, permission('tokens:delete'), serviceSettingsController.apiKeys.revoke.get) +simplifiedAccount.post(paths.simplifiedAccount.settings.apiKeys.revoke, permission('tokens:delete'), serviceSettingsController.apiKeys.revoke.post) // stripe details const stripeDetailsPath = paths.simplifiedAccount.settings.stripeDetails diff --git a/app/views/simplified-account/settings/api-keys/index.njk b/app/views/simplified-account/settings/api-keys/index.njk index 5c902b810..8fe8fa6d2 100644 --- a/app/views/simplified-account/settings/api-keys/index.njk +++ b/app/views/simplified-account/settings/api-keys/index.njk @@ -53,7 +53,7 @@ text: 'Change name' }, { - href: '#', + href: key.revokeKeyLink, text: 'Revoke' } ] diff --git a/app/views/simplified-account/settings/api-keys/revoke.njk b/app/views/simplified-account/settings/api-keys/revoke.njk new file mode 100644 index 000000000..73caf16be --- /dev/null +++ b/app/views/simplified-account/settings/api-keys/revoke.njk @@ -0,0 +1,38 @@ +{% extends "../settings-layout.njk" %} + +{% block settingsPageTitle %} + API keys - revoke key +{% endblock %} + +{% block settingsContent %} +
+ + + {{ govukRadios({ + name: 'revokeApiKey', + fieldset: { + legend: { + text: 'Are you sure you want to revoke ' + description, + isPageHeading: true, + classes: 'govuk-fieldset__legend--l govuk-!-font-weight-bold' + } + }, + hint: { + text: 'The key will stop working immediately. Any integration you’ve created will no longer work.' + }, + items: [ + { + value: 'Yes', + text: 'Yes' + }, + { + value: 'No', + text: 'No' + } + ] + }) }} + {{ govukButton({ + text: 'Save changes' + }) }} +
+{% 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 f7a6237e2..c5d0af1cb 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 @@ -105,7 +105,8 @@ describe('Settings - API keys', () => { .within(() => { cy.get('a') .should('contain.text', 'Revoke') - .and('have.attr', 'href', '#') + .and('have.attr', 'href', formatSimplifiedAccountPathsFor(paths.simplifiedAccount.settings.apiKeys.revoke, + SERVICE_EXTERNAL_ID, ACCOUNT_TYPE, token.tokenLink)) }) cy.get('.govuk-summary-list__row').eq(0)