Skip to content

Commit

Permalink
Merge pull request #2490 from alphagov/PP-7583-url-structure-api-keys
Browse files Browse the repository at this point in the history
PP-7583 URL Restructure - API Keys
  • Loading branch information
iqbalgds authored Jan 15, 2021
2 parents 7baf574 + 528e1b7 commit 984aaec
Show file tree
Hide file tree
Showing 16 changed files with 132 additions and 86 deletions.
2 changes: 1 addition & 1 deletion app/controllers/api-keys/post-create.controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ const { response } = require('../../utils/response.js')
const publicAuthClient = require('../../services/clients/public-auth.client')
const { isADirectDebitAccount } = require('../../services/clients/direct-debit-connector.client.js')

module.exports = async function createAPIKey (req, res, next) {
module.exports = async function createApiKey (req, res, next) {
const accountId = req.account.gateway_account_id
const correlationId = req.correlationId
const description = req.body.description
Expand Down
9 changes: 6 additions & 3 deletions app/controllers/api-keys/post-revoke.controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,11 @@
const paths = require('../../paths')
const publicAuthClient = require('../../services/clients/public-auth.client')
const logger = require('../../utils/logger')(__filename)
const formatAccountPathsFor = require('../../utils/format-account-paths-for')

module.exports = async function revokeApiKey (req, res) {
const apiKeysPath = formatAccountPathsFor(paths.account.apiKeys.index, req.account.external_id)

module.exports = async (req, res) => {
const accountId = req.account.gateway_account_id
const payload = {
token_link: req.body.token_link
Expand All @@ -16,10 +19,10 @@ module.exports = async (req, res) => {
})

req.flash('generic', 'The API key was successfully revoked')
return res.redirect(paths.apiKeys.index)
return res.redirect(apiKeysPath)
} catch (error) {
logger.error('Error revoking API key', { error: error.message })
req.flash('genericError', 'Something went wrong. Please try again or contact support.')
return res.redirect(paths.apiKeys.index)
return res.redirect(apiKeysPath)
}
}
29 changes: 16 additions & 13 deletions app/controllers/api-keys/post-update.controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,11 @@
const paths = require('../../paths')
const publicAuthClient = require('../../services/clients/public-auth.client')
const logger = require('../../utils/logger')(__filename)
const formatAccountPathsFor = require('../../utils/format-account-paths-for')

module.exports = async function updateApiKey (req, res) {
const apiKeysPath = formatAccountPathsFor(paths.account.apiKeys.index, req.account.external_id)

module.exports = (req, res) => {
// this does not need to be explicitly tied down to account_id
// right now because the UUID space is big enough that no-one
// will be able to discover other peoples' tokens to change them
Expand All @@ -13,17 +16,17 @@ module.exports = (req, res) => {
description: req.body.description
}

publicAuthClient.updateToken({
payload: payload,
correlationId: req.correlationId
})
.then(() => {
req.flash('generic', 'The API key description was successfully updated')
res.redirect(paths.apiKeys.index)
})
.catch(error => {
logger.error('Error updating API key description', { error })
req.flash('genericError', 'Something went wrong. Please try again or contact support.')
res.redirect(paths.apiKeys.index)
try {
await publicAuthClient.updateToken({
payload: payload,
correlationId: req.correlationId
})

req.flash('generic', 'The API key description was successfully updated')
res.redirect(apiKeysPath)
} catch (error) {
logger.error('Error updating API key description', { error })
req.flash('genericError', 'Something went wrong. Please try again or contact support.')
res.redirect(apiKeysPath)
}
}
14 changes: 7 additions & 7 deletions app/paths.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,13 @@ module.exports = {
toggleMotoMaskCardNumberAndSecurityCode: {
cardNumber: '/moto-hide-card-number',
securityCode: '/moto-hide-security-code'
},
apiKeys: {
index: '/api-keys',
revoked: '/api-keys/revoked',
create: '/api-keys/create',
revoke: '/api-keys/revoke',
update: '/api-keys/update'
}
},
transactions: {
Expand Down Expand Up @@ -91,13 +98,6 @@ module.exports = {
dashboard: {
index: '/'
},
apiKeys: {
index: '/api-keys',
revoked: '/api-keys/revoked',
create: '/api-keys/create',
revoke: '/api-keys/revoke',
update: '/api-keys/update'
},
serviceSwitcher: {
index: '/my-services',
switch: '/my-services/switch',
Expand Down
15 changes: 8 additions & 7 deletions app/routes.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,12 +87,13 @@ const stripeSetupDashboardRedirectController = require('./controllers/stripe-set
// Assignments
const {
healthcheck, registerUser, user, dashboard, selfCreateService, transactions, credentials,
apiKeys, serviceSwitcher, teamMembers, staticPaths, inviteValidation, editServiceName, merchantDetails,
serviceSwitcher, teamMembers, staticPaths, inviteValidation, editServiceName, merchantDetails,
notificationCredentials, prototyping, paymentLinks,
requestToGoLive, policyPages, stripeSetup, stripe,
settings, yourPsp, allServiceTransactions, payouts
} = paths
const {
apiKeys,
digitalWallet,
emailNotifications,
paymentTypes,
Expand Down Expand Up @@ -236,12 +237,12 @@ module.exports.bind = function (app) {
app.post(merchantDetails.edit, permission('merchant-details:update'), merchantDetailsController.postEdit)

// API KEYS
app.get(apiKeys.index, permission('tokens-active:read'), getAccount, apiKeysController.getIndex)
app.get(apiKeys.revoked, permission('tokens-revoked:read'), getAccount, apiKeysController.getRevoked)
app.get(apiKeys.create, permission('tokens:create'), getAccount, apiKeysController.getCreate)
app.post(apiKeys.create, permission('tokens:create'), getAccount, apiKeysController.postCreate)
app.post(apiKeys.revoke, permission('tokens:delete'), getAccount, apiKeysController.postRevoke)
app.post(apiKeys.update, permission('tokens:update'), getAccount, apiKeysController.postUpdate)
account.get(apiKeys.index, permission('tokens-active:read'), apiKeysController.getIndex)
account.get(apiKeys.revoked, permission('tokens-revoked:read'), apiKeysController.getRevoked)
account.get(apiKeys.create, permission('tokens:create'), apiKeysController.getCreate)
account.post(apiKeys.create, permission('tokens:create'), apiKeysController.postCreate)
account.post(apiKeys.revoke, permission('tokens:delete'), apiKeysController.postRevoke)
account.post(apiKeys.update, permission('tokens:update'), apiKeysController.postUpdate)

account.get(paymentTypes.index, permission('payment-types:read'), paymentTypesController.getIndex)
account.post(paymentTypes.index, permission('payment-types:update'), paymentTypesController.postIndex)
Expand Down
10 changes: 6 additions & 4 deletions app/utils/nav-builder.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,11 @@ const serviceNavigationItems = (currentPath, permissions, type) => {
navigationItems.push({
id: 'navigation-menu-settings',
name: 'Settings',
url: type === 'card' ? paths.settings.index : paths.apiKeys.index,
url: paths.settings.index,
current: currentPath !== '/' ? pathLookup(currentPath, [
...mainSettingsPaths,
...yourPspPaths,
paths.apiKeys,
paths.account.apiKeys,
paths.account.paymentTypes
]) : false,
permissions: _.some([
Expand All @@ -69,6 +69,8 @@ const serviceNavigationItems = (currentPath, permissions, type) => {
}

const adminNavigationItems = (currentPath, permissions, type, paymentProvider, account = {}) => {
const apiKeysPath = formatAccountPathsFor(paths.account.apiKeys.index, account.external_id)

return [
{
id: 'navigation-menu-settings-home',
Expand All @@ -80,8 +82,8 @@ const adminNavigationItems = (currentPath, permissions, type, paymentProvider, a
{
id: 'navigation-menu-api-keys',
name: 'API keys',
url: paths.apiKeys.index,
current: pathLookup(currentPath, paths.apiKeys.index),
url: apiKeysPath,
current: pathLookup(currentPath, paths.account.apiKeys.index),
permissions: permissions.tokens_update
},
{
Expand Down
4 changes: 2 additions & 2 deletions app/views/api-keys/_key.njk
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
{% endif %}

{% if permissions.tokens_delete %}
<form class="target-to-show" id="delete-{{token.token_link}}" method="POST" action="{{routes.apiKeys.revoke}}">
<form class="target-to-show" id="delete-{{token.token_link}}" method="POST" action="{{formatAccountPathsFor(routes.account.apiKeys.revoke, currentGatewayAccount.external_id)}}">
<input id="csrf" name="csrfToken" type="hidden" value="{{csrf}}"/>
<input type="hidden" name="token_link" value="{{token.token_link}}"/>
<div class="govuk-error-summary govuk-!-margin-top-6" role="group" aria-labelledby="error-summary" tabindex="-1">
Expand All @@ -64,7 +64,7 @@ govuk-link--no-visited-state target-to-show--cancel" href="#main">Cancel</a>
</form>
{% endif %}
{% if permissions.tokens_update %}
<form class="target-to-show govuk-!-margin-top-6" id="update-{{token.token_link}}" method="POST" action="{{routes.apiKeys.update}}">
<form class="target-to-show govuk-!-margin-top-6" id="update-{{token.token_link}}" method="POST" action="{{formatAccountPathsFor(routes.account.apiKeys.update, currentGatewayAccount.external_id)}}">
<input id="csrf" name="csrfToken" type="hidden" value="{{csrf}}"/>
<input type="hidden" name="token_link" value="{{token.token_link}}"/>
{{
Expand Down
8 changes: 4 additions & 4 deletions app/views/api-keys/_keys.njk
Original file line number Diff line number Diff line change
Expand Up @@ -11,25 +11,25 @@
</h2>

{% if active %}
{% set apiKeyLinkRoute = routes.apiKeys.revoked %}
{% set apiKeyLinkRoute = routes.account.apiKeys.revoked %}
{% set apiKeyLinkText = 'revoked' %}
{% set apiKeyLinkId = 'revoked-keys-link' %}
{% else %}
{% set apiKeyLinkRoute = routes.apiKeys.index %}
{% set apiKeyLinkRoute = routes.account.apiKeys.index %}
{% set apiKeyLinkText = 'active' %}
{% set apiKeyLinkId = 'active-keys-link' %}
{% endif %}

<div class="govuk-body">
<a class="govuk-link" href="{{ apiKeyLinkRoute }}" id="{{apiKeyLinkId}}">View {{apiKeyLinkText}} keys</a>
<a class="govuk-link" href="{{ formatAccountPathsFor(apiKeyLinkRoute, currentGatewayAccount.external_id) }}" id="{{apiKeyLinkId}}">View {{apiKeyLinkText}} keys</a>
</div>

{% if permissions.tokens_create %}
{% if active %}
{{
govukButton({
text: "Create a new API key",
href: routes.apiKeys.create,
href: formatAccountPathsFor(routes.account.apiKeys.create, currentGatewayAccount.external_id),
classes: "generate-key",
attributes: {
id: "create-api-key"
Expand Down
6 changes: 3 additions & 3 deletions app/views/api-keys/create.njk
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ Create an API key - {{currentService.name}} {{currentGatewayAccount.full_type}}
<div class="govuk-grid-column-two-thirds">
{% if not token %}
<h1 class="govuk-heading-l page-title">Create an API key</h1>
<form class="form" method="post" action="{{routes.apiKeys.create}}">
<form class="form" method="post" action="{{formatAccountPathsFor(routes.account.apiKeys.create, currentGatewayAccount.external_id)}}">
<input id="csrf" name="csrfToken" type="hidden" value="{{csrf}}" />

{{
Expand Down Expand Up @@ -44,7 +44,7 @@ Create an API key - {{currentService.name}} {{currentGatewayAccount.full_type}}
}}
</form>
<p class="govuk-body">
<a class="govuk-link govuk-link--no-visited-state" href="{{routes.apiKeys.index}}">Cancel</a>
<a class="govuk-link govuk-link--no-visited-state" href="{{formatAccountPathsFor(routes.account.apiKeys.index, currentGatewayAccount.external_id)}}">Cancel</a>
</p>
{% else %}
<h1 class="govuk-heading-l page-title">New API key</h1>
Expand Down Expand Up @@ -74,7 +74,7 @@ Create an API key - {{currentService.name}} {{currentGatewayAccount.full_type}}
<div class="copy-this-api-key-notification govuk-visually-hidden" aria-live="assertive"></div>

<p class="govuk-body">
<a class="govuk-link govuk-link--no-visited-state" href="{{routes.apiKeys.index}}" id="finish-link">
<a class="govuk-link govuk-link--no-visited-state" href="{{formatAccountPathsFor(routes.account.apiKeys.index, currentGatewayAccount.external_id)}}" id="finish-link">
Back to API keys
</a>
</p>
Expand Down
2 changes: 1 addition & 1 deletion app/views/api-keys/update.njk
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ Create an API key - {{currentService.name}} {{currentGatewayAccount.full_type}}
<button type="submit" id="save-description-{{token.token_link}}" href="#{{token.token_link}}" class="button">
Save changes<span class="visuallyhidden"> to {{token.description}}</span>
</button>
<a id="cancel-{{token.token_link}}" href="{{routes.apiKeys.index}}" class="button-link">
<a id="cancel-{{token.token_link}}" href="{{formatAccountPathsFor(routes.account.apiKeys.index, currentGatewayAccount.external_id)}}" class="button-link">
Cancel<span class="visuallyhidden"> edits to {{token.description}}</span>
</a>
</div>
Expand Down
25 changes: 17 additions & 8 deletions test/ui/navigation.ui.test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
const { render } = require('../test-helpers/html-assertions')
const { serviceNavigationItems, adminNavigationItems } = require('../../app/utils/nav-builder')
const formatAccountPathsFor = require('../../app/utils/format-account-paths-for')

describe('navigation menu', function () {
it('should render only Home link when user does have any of the required permissions to show the navigation links', function () {
Expand All @@ -15,7 +16,8 @@ describe('navigation menu', function () {
hideServiceNav: false,
serviceNavigationItems: serviceNavigationItems('/', testPermissions, 'card'),
links: [],
linksToDisplay: []
linksToDisplay: [],
formatAccountPathsFor: formatAccountPathsFor
}

const body = render('dashboard/index', templateData)
Expand Down Expand Up @@ -72,7 +74,8 @@ describe('navigation menu', function () {
const templateData = {
permissions: testPermissions,
showSettingsNav: true,
adminNavigationItems: adminNavigationItems('/api-keys', testPermissions, 'card', 'sandbox')
adminNavigationItems: adminNavigationItems('/api-keys', testPermissions, 'card', 'sandbox'),
formatAccountPathsFor: formatAccountPathsFor
}

const body = render('api-keys/index', templateData)
Expand All @@ -93,7 +96,8 @@ describe('navigation menu', function () {
const templateData = {
permissions: testPermissions,
showSettingsNav: true,
adminNavigationItems: adminNavigationItems('/api-keys', testPermissions, 'card', 'worldpay')
adminNavigationItems: adminNavigationItems('/api-keys', testPermissions, 'card', 'worldpay'),
formatAccountPathsFor: formatAccountPathsFor
}

const body = render('api-keys/index', templateData)
Expand All @@ -114,7 +118,8 @@ describe('navigation menu', function () {
const templateData = {
permissions: testPermissions,
showSettingsNav: true,
adminNavigationItems: adminNavigationItems('/api-keys', testPermissions, 'card', 'stripe')
adminNavigationItems: adminNavigationItems('/api-keys', testPermissions, 'card', 'stripe'),
formatAccountPathsFor: formatAccountPathsFor
}

const body = render('api-keys/index', templateData)
Expand All @@ -135,7 +140,8 @@ describe('navigation menu', function () {
const templateData = {
permissions: testPermissions,
showSettingsNav: true,
adminNavigationItems: adminNavigationItems('/api-keys', testPermissions, 'card', 'sandbox')
adminNavigationItems: adminNavigationItems('/api-keys', testPermissions, 'card', 'sandbox'),
formatAccountPathsFor: formatAccountPathsFor
}

const body = render('api-keys/index', templateData)
Expand All @@ -155,7 +161,8 @@ describe('navigation menu', function () {
const templateData = {
permissions: testPermissions,
showSettingsNav: true,
adminNavigationItems: adminNavigationItems('/api-keys', testPermissions, 'card', 'sandbox')
adminNavigationItems: adminNavigationItems('/api-keys', testPermissions, 'card', 'sandbox'),
formatAccountPathsFor: formatAccountPathsFor
}

const body = render('api-keys/index', templateData)
Expand All @@ -178,7 +185,8 @@ describe('navigation menu', function () {
const templateData = {
permissions: testPermissions,
showSettingsNav: true,
adminNavigationItems: adminNavigationItems('/api-keys', testPermissions, 'direct debit')
adminNavigationItems: adminNavigationItems('/api-keys', testPermissions, 'direct debit'),
formatAccountPathsFor: formatAccountPathsFor
}

const body = render('api-keys/index', templateData)
Expand Down Expand Up @@ -206,7 +214,8 @@ describe('navigation menu', function () {
const templateData = {
permissions: testPermissions,
showSettingsNav: true,
adminNavigationItems: adminNavigationItems('/api-keys', testPermissions, 'card', 'worldpay')
adminNavigationItems: adminNavigationItems('/api-keys', testPermissions, 'card', 'worldpay'),
formatAccountPathsFor: formatAccountPathsFor
}

const body = render('api-keys/index', templateData)
Expand Down
Loading

0 comments on commit 984aaec

Please sign in to comment.