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: Create API key functionality #4394

Merged
merged 2 commits into from
Dec 20, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
6 changes: 6 additions & 0 deletions app/assets/sass/components/api-keys.scss
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Original file line number Diff line number Diff line change
@@ -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')
Original file line number Diff line number Diff line change
@@ -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)
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if it's better or worse to allow for customising the back link text, e.g. if a backLinkText variable is provided in the context then it will override the text. I'll leave that up to you, just a thought

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's worth doing if I encounter more back links that don't have the text "Back". Going to leave this for now.

})
}

module.exports = { get, post }
Original file line number Diff line number Diff line change
@@ -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(
Copy link
Contributor

Choose a reason for hiding this comment

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

some nice stuff you can do with sinon-chai

Suggested change
expect(mockResponse.args[0][3]).to.have.property('backLink').to.equal(
expect(mockResponse).to.have.been.calledWith(sinon.match.any, sinon.match.any, sinon.match.any, {
backLink: '...'
})

but again just a suggestion, up to you which you prefer

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I might as well delete this test and amend the one above it:

    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) })
    })

formatSimplifiedAccountPathsFor(paths.simplifiedAccount.settings.apiKeys.index, SERVICE_ID, ACCOUNT_TYPE))
})
})

describe('post', () => {
before(() => {
nextRequest({
body: {
description: 'a test api key'
},
user: {
email: '[email protected]'
}
})
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',
'[email protected]',
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')
})
})
})
3 changes: 2 additions & 1 deletion app/paths.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
29 changes: 28 additions & 1 deletion app/services/api-keys.service.js
Original file line number Diff line number Diff line change
@@ -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
Copy link
Contributor

Choose a reason for hiding this comment

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

a tad pedantic but this returns a Promise<String> as it's async

*/
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
Expand All @@ -14,5 +39,7 @@ const getActiveKeys = async (gatewayAccountId) => {
}

module.exports = {
getActiveKeys
createApiKey,
getActiveKeys,
TOKEN_SOURCE
}
2 changes: 2 additions & 0 deletions app/simplified-account-routes.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion app/utils/simplified-account/settings/service-settings.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
34 changes: 34 additions & 0 deletions app/views/simplified-account/settings/api-keys/api-key-name.njk
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
{% extends "../settings-layout.njk" %}

{% block settingsPageTitle %}
API key name
{% endblock %}

{% block settingsContent %}

<h1 class="govuk-heading-l page-title">API key name</h1>

<form id="api-key-name" method="post" novalidate>
<input id="csrf" name="csrfToken" type="hidden" value="{{ csrf }}"/>

{{ 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"
}) }}
</form>
{% endblock %}
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
{% extends "../settings-layout.njk" %}

{% block settingsPageTitle %}
New API key
{% endblock %}

{% block settingsContent %}

{{ govukBackLink({
text: "Back to API keys",
href: backToApiKeysLink
}) }}

<h1 class="govuk-heading-l page-title">New API key</h1>

{% set html %}
<p class="govuk-body">
Copy your key to somewhere safe.</br></br>You won’t be able to see it again once you leave this page.
</p>
{% endset %}

{{ govukInsetText({
html: html
}) }}

<p class="govuk-body">
You must copy the whole key, including the <span class="key-prefix">api_test_</span> or <span class="key-prefix">api_live_</span> prefix.
</p>

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

<ul class="govuk-list govuk-list--bullet">
<li>You must store your API keys securely.</li>
<li>You must not share this key in publicly accessible documents or repositories.</li>
<li>You must not share it with anyone who should not be using the GOV.UK Pay API directly.</li>
</ul>

<p class="govuk-body">
More about <a class="govuk-link" href="https://docs.payments.service.gov.uk/security/#securing-your-developer-keys">
securing your API keys</a>.
</p>

<h2 class="govuk-heading-m">
{{ description }}
</h2>
<div>
<span id="apiKey" class="code copy-this-api-key govuk-!-margin-top-6">{{ apiKey }}</span>
</div>

{{ 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"
}
}) }}

<div class="copy-this-api-key-notification govuk-visually-hidden" aria-live="assertive"></div>

{% endblock %}
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down
Loading