Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PP-12395: Change api key name #4399

Merged
merged 4 commits into from
Jan 2, 2025
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 => {
oswaldquek marked this conversation as resolved.
Show resolved Hide resolved
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 { response } = require('@utils/response')
const formatSimplifiedAccountPathsFor = require('@utils/simplified-account/format/format-simplified-account-paths-for')
const paths = require('@root/paths')
const { changeApiKeyName } = require('@services/api-keys.service')
const DESCRIPTION_VALIDATION = require('@controllers/simplified-account/settings/api-keys/validations')
const { validationResult } = require('express-validator')
const formatValidationErrors = require('@utils/simplified-account/format/format-validation-errors')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there is a convention to order these require statements to improve readability, with core modules first and destructured imports last.


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
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,9 +58,12 @@ 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-1')
]

beforeEach(() => {
setupStubs('admin', apiKeys)
cy.visit(`/simplified/service/${SERVICE_EXTERNAL_ID}/account/${ACCOUNT_TYPE}/settings/api-keys`)
Expand Down Expand Up @@ -91,7 +96,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
Loading