Skip to content

Commit

Permalink
PP-12395: Change api key name (#4399)
Browse files Browse the repository at this point in the history
* PP-12395: Change api key name
  • Loading branch information
oswaldquek authored Jan 2, 2025
1 parent 054a76a commit ebdea77
Show file tree
Hide file tree
Showing 15 changed files with 245 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,19 @@ 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: formatSimplifiedAccountPathsFor(paths.simplifiedAccount.settings.apiKeys.create, req.service.externalId, req.account.type),
activeKeys: activeKeys.map(activeKey => {
return {
...activeKey,
changeNameLink: formatSimplifiedAccountPathsFor(paths.simplifiedAccount.settings.apiKeys.changeName,
req.service.externalId, req.account.type, activeKey.tokenLink)
}
}),
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')
module.exports.changeName = require('./change-name/change-name.controller')
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ const ACCOUNT_TYPE = 'live'
const SERVICE_ID = 'service-id-123abc'

const mockResponse = sinon.spy()
const apiKeys = [{ description: 'my token', createdBy: 'system generated', issuedDate: '12 Dec 2024' }]
const apiKeys = [{ description: 'my token', createdBy: 'system generated', issuedDate: '12 Dec 2024', tokenLink: '123-345' }]
const apiKeysService = {
getActiveKeys: sinon.stub().resolves(apiKeys)
}
Expand Down Expand Up @@ -39,7 +39,13 @@ describe('Controller: settings/api-keys', () => {

it('should pass context data to the response method', () => {
expect(mockResponse.args[0][3]).to.have.property('accountType').to.equal('live')
expect(mockResponse.args[0][3]).to.have.property('activeKeys').to.deep.equal(apiKeys)
expect(mockResponse.args[0][3]).to.have.property('activeKeys').to.deep.equal(
apiKeys.map(apiKey => {
return {
...apiKey,
changeNameLink: `/simplified/service/${SERVICE_ID}/account/${ACCOUNT_TYPE}/settings/api-keys/change-name/${apiKeys[0].tokenLink}`
}
}))
})
})
})
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
const paths = require('@root/paths')
const { validationResult } = require('express-validator')
const formatSimplifiedAccountPathsFor = require('@utils/simplified-account/format/format-simplified-account-paths-for')
const formatValidationErrors = require('@utils/simplified-account/format/format-validation-errors')
const { response } = require('@utils/response')
const { changeApiKeyName } = require('@services/api-keys.service')
const DESCRIPTION_VALIDATION = require('@controllers/simplified-account/settings/api-keys/validations')

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) {
await Promise.all(DESCRIPTION_VALIDATION.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 tokenLink = req.params.tokenLink
const description = req.body.description
await changeApiKeyName(tokenLink, description)
res.redirect(formatSimplifiedAccountPathsFor(paths.simplifiedAccount.settings.apiKeys.index, req.service.externalId, req.account.type))
}

module.exports = { get, post }
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
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 ACCOUNT_TYPE = 'live'
const SERVICE_ID = 'service-id-123abc'
const mockResponse = sinon.spy()
const apiKeysService = {
changeApiKeyName: sinon.stub().resolves()
}
const {
req,
res,
nextRequest,
call
} = new ControllerTestBuilder('@controllers/simplified-account/settings/api-keys/change-name/change-name.controller')
.withServiceExternalId(SERVICE_ID)
.withAccountType(ACCOUNT_TYPE)
.withStubs({
'@utils/response': { response: mockResponse },
'@services/api-keys.service': apiKeysService
})
.build()

describe('Controller: settings/api-keys/change-name', () => {
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, 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) })
})
})

describe('post', () => {
describe('a valid description', () => {
before(() => {
nextRequest({
body: {
description: 'a test api key'
},
params: {
tokenLink: '123-456-abc'
}
})
call('post')
})

it('should submit values to the api keys service', () => {
expect(apiKeysService.changeApiKeyName).to.have.been.calledWith('123-456-abc', 'a test api key')
})

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('an invalid description', () => {
before(() => {
nextRequest({
body: {
description: ''
},
params: {
tokenLink: '123-456-abc'
}
})
call('post')
})

it('should not call apiKeysService.changeApiKeyName', () => {
sinon.assert.notCalled(apiKeysService.changeApiKeyName)
})

it('should pass req, res, template path and context to the response method', () => {
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('Name must not be empty')
expect(mockResponse.args[0][3].errors.formErrors.description).to.equal('Name must not be empty')
expect(mockResponse.args[0][3].backLink).to.equal(
formatSimplifiedAccountPathsFor(paths.simplifiedAccount.settings.apiKeys.index, SERVICE_ID, ACCOUNT_TYPE))
})
})
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -2,26 +2,18 @@ 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 { validationResult } = require('express-validator')
const formatValidationErrors = require('@utils/simplified-account/format/format-validation-errors')
const DESCRIPTION_VALIDATION = require('@controllers/simplified-account/settings/api-keys/validations')

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) {
await Promise.all(descriptionValidation.map(validation => validation.run(req)))
await Promise.all(DESCRIPTION_VALIDATION.map(validation => validation.run(req)))
const errors = validationResult(req)

if (!errors.isEmpty()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ const {
})
.build()

describe('Controller: settings/create-api-key', () => {
describe('Controller: settings/api-keys/create', () => {
describe('get', () => {
before(() => {
call('get')
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
const { body } = require('express-validator')
const DESCRIPTION_VALIDATION = [
body('description')
.trim()
.notEmpty()
.withMessage('Name must not be empty')
.isLength({ max: 50 })
.withMessage('Name must be 50 characters or fewer')
]

module.exports = DESCRIPTION_VALIDATION
7 changes: 7 additions & 0 deletions app/models/Token.class.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,17 @@ class Token {
return this
}

withTokenLink (tokenLink) {
this.tokenLink = tokenLink
return this
}

toJson () {
return {
...this.description && { description: this.description },
...this.createdBy && { created_by: this.createdBy },
...this.issuedDate && { issued_date: this.issuedDate },
...this.tokenLink && { token_link: this.tokenLink },
...this.lastUsed && { last_used: this.lastUsed }
}
}
Expand All @@ -37,6 +43,7 @@ class Token {
.withCreatedBy(data?.created_by)
.withIssuedDate(data?.issued_date)
.withLastUsed(data?.last_used)
.withTokenLink(data?.token_link)
}
}

Expand Down
3 changes: 2 additions & 1 deletion app/paths.js
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,8 @@ module.exports = {
},
apiKeys: {
index: '/settings/api-keys',
create: '/settings/api-keys/create'
create: '/settings/api-keys/create',
changeName: '/settings/api-keys/change-name/:tokenLink'
},
webhooks: {
index: '/settings/webhooks'
Expand Down
10 changes: 10 additions & 0 deletions app/services/api-keys.service.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,17 @@ const getActiveKeys = async (gatewayAccountId) => {
return publicAuthData.tokens.map(tokenData => Token.fromJson(tokenData))
}

/**
* @param {string} tokenLink
* @param {string} name The new name/description
* @return {Promise<void>}
*/
const changeApiKeyName = async (tokenLink, name) => {
await publicAuthClient.updateToken({ payload: { token_link: tokenLink, description: name } })
}

module.exports = {
changeApiKeyName,
createApiKey,
getActiveKeys,
TOKEN_SOURCE
Expand Down
2 changes: 2 additions & 0 deletions app/simplified-account-routes.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,8 @@ simplifiedAccount.get(paths.simplifiedAccount.settings.cardTypes.index, permissi
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)
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)

// stripe details
const stripeDetailsPath = paths.simplifiedAccount.settings.stripeDetails
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,6 @@
maxlength: "50"
},
classes: "govuk-input--width-40",
label: {
text: "Add a description for the key"
},
hint: {
text: "For example, “John Smith’s API key”"
}
Expand Down
2 changes: 1 addition & 1 deletion app/views/simplified-account/settings/api-keys/index.njk
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
actions: {
items: [
{
href: '#',
href: key.changeNameLink,
text: 'Change name'
},
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ const ROLES = require('@test/fixtures/roles.fixtures')
const gatewayAccountStubs = require('@test/cypress/stubs/gateway-account-stubs')
const apiKeysStubs = require('@test/cypress/stubs/api-keys-stubs')
const { Token } = require('@models/Token.class')
const formatSimplifiedAccountPathsFor = require('@utils/simplified-account/format/format-simplified-account-paths-for')
const paths = require('@root/paths')

const USER_EXTERNAL_ID = 'user-123-abc'
const SERVICE_EXTERNAL_ID = 'service-456-def'
Expand Down Expand Up @@ -56,13 +58,17 @@ describe('Settings - API keys', () => {

describe('when there are active API keys', () => {
const apiKeys = [
new Token().withCreatedBy('system generated').withDescription('description').withIssuedDate('12 Dec 2024'),
new Token().withCreatedBy('algae bra').withDescription('mathematical clothes').withIssuedDate('10 Dec 2024').withLastUsed('10 Dec 2024')
new Token().withCreatedBy('system generated').withDescription('description')
.withIssuedDate('12 Dec 2024').withTokenLink('token-link-1'),
new Token().withCreatedBy('algae bra').withDescription('mathematical clothes')
.withIssuedDate('10 Dec 2024').withLastUsed('10 Dec 2024').withTokenLink('token-link-2')
]

beforeEach(() => {
setupStubs('admin', apiKeys)
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', 'Test API keys')
cy.get('.service-settings-pane')
Expand Down Expand Up @@ -91,7 +97,8 @@ describe('Settings - API keys', () => {
.within(() => {
cy.get('a')
.should('contain.text', 'Change name')
.and('have.attr', 'href', '#')
.and('have.attr', 'href', formatSimplifiedAccountPathsFor(paths.simplifiedAccount.settings.apiKeys.changeName,
SERVICE_EXTERNAL_ID, ACCOUNT_TYPE, token.tokenLink))
})

cy.get('.govuk-summary-card__action').eq(1)
Expand Down Expand Up @@ -160,6 +167,43 @@ describe('Settings - API keys', () => {
})
})
})

describe('re-name an api key', () => {
const NEW_API_KEY_NAME = 'api key description' // pragma: allowlist secret
const TOKEN_LINK = 'token-link-1'

const apiKeys = [
new Token().withCreatedBy('algae bra').withDescription('mathematical clothes')
.withIssuedDate('10 Dec 2024').withLastUsed('10 Dec 2024').withTokenLink(TOKEN_LINK)
]

beforeEach(() => {
setupStubs('admin', apiKeys)
cy.task('setupStubs', [
apiKeysStubs.changeApiKeyName(TOKEN_LINK, NEW_API_KEY_NAME)
])
cy.visit(`/simplified/service/${SERVICE_EXTERNAL_ID}/account/${ACCOUNT_TYPE}/settings/api-keys`)
})

it('show the API key name page', () => {
cy.get('.govuk-summary-card').within(() => {
cy.contains('h2', 'mathematical clothes').should('exist')
cy.contains('a', 'Change name').click()
})
cy.contains('h1', 'API key name').should('exist')
})

it('should re-name the api key successfully', () => {
cy.get('.govuk-summary-card').within(() => {
cy.contains('h2', 'mathematical clothes').should('exist')
cy.contains('a', 'Change name').click()
})
cy.get('input[id="description"]').type(NEW_API_KEY_NAME)
cy.contains('button', 'Continue').click()
cy.url().should('include', `/simplified/service/${SERVICE_EXTERNAL_ID}/account/${ACCOUNT_TYPE}/settings/api-keys`)
cy.contains('h1', 'Test API keys').should('exist')
})
})
})

describe('for a non-admin user', () => {
Expand Down
Loading

0 comments on commit ebdea77

Please sign in to comment.