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

Haar 1906 edit client details flow #31

Closed
wants to merge 15 commits into from
Closed
44 changes: 43 additions & 1 deletion server/controllers/baseClientController.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import listBaseClientsPresenter from '../views/presenters/listBaseClientsPresent
import createUserToken from '../testutils/createUserToken'
import viewBaseClientPresenter from '../views/presenters/viewBaseClientPresenter'
import nunjucksUtils from '../views/helpers/nunjucksUtils'
import editBaseClientPresenter from '../views/presenters/editBaseClientPresenter'

describe('BaseClientController', () => {
const token = createUserToken(['ADMIN'])
Expand Down Expand Up @@ -109,7 +110,7 @@ describe('BaseClientController', () => {
})

it('if grant is specified with authorization-code renders the details screen', async () => {
// GIVEN a request with grant="client-credentials" parameter
// GIVEN a request with grant="authorization-code" parameter
request = createMock<Request>({ query: { grant: 'authorization-code' } })

// WHEN the create base client page is requested
Expand Down Expand Up @@ -184,4 +185,45 @@ describe('BaseClientController', () => {
})
})
})

describe('update base client details', () => {
describe('journey', () => {
it('if grant is not specified as parameter renders the select grant screen', async () => {
// GIVEN a request to edit a base client
const baseClient = baseClientFactory.build()
baseClientService.getBaseClient.mockResolvedValue(baseClient)
request = createMock<Request>({ params: { baseClientId: baseClient.baseClientId } })

// WHEN the edit base client details page is requested
await baseClientController.displayEditBaseClient()(request, response, next)

// THEN the base client is retrieved from the base client service
expect(baseClientService.getBaseClient).toHaveBeenCalledWith(token, baseClient.baseClientId)

// AND the page is rendered
const presenter = editBaseClientPresenter(baseClient)
expect(response.render).toHaveBeenCalledWith('pages/edit-base-client-details.njk', {
baseClient,
presenter,
...nunjucksUtils,
})
})

it('if success redirects to view base client screen', async () => {
// GIVEN the service will return without an error
const baseClient = baseClientFactory.build()
request = createMock<Request>({
params: { baseClientId: baseClient.baseClientId },
body: { baseClientId: baseClient.baseClientId },
})
baseClientService.updateBaseClient.mockResolvedValue(new Response())

// WHEN it is posted
await baseClientController.updateBaseClientDetails()(request, response, next)

// THEN the user is redirected to the view base client page
expect(response.redirect).toHaveBeenCalledWith(`/base-clients/${baseClient.baseClientId}`)
})
})
})
})
36 changes: 35 additions & 1 deletion server/controllers/baseClientController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { BaseClientService } from '../services'
import listBaseClientsPresenter from '../views/presenters/listBaseClientsPresenter'
import viewBaseClientPresenter from '../views/presenters/viewBaseClientPresenter'
import nunjucksUtils from '../views/helpers/nunjucksUtils'
import { mapCreateBaseClientForm } from '../mappers'
import { mapCreateBaseClientForm, mapEditBaseClientDetailsForm } from '../mappers'
import { BaseClient } from '../interfaces/baseClientApi/baseClient'
import editBaseClientPresenter from '../views/presenters/editBaseClientPresenter'

Expand Down Expand Up @@ -94,4 +94,38 @@ export default class BaseClientController {
return ''
}
}

public displayEditBaseClient(): RequestHandler {
return async (req, res) => {
const userToken = res.locals.user.token
const { baseClientId } = req.params
const baseClient = await this.baseClientService.getBaseClient(userToken, baseClientId)

const presenter = editBaseClientPresenter(baseClient)
res.render('pages/edit-base-client-details.njk', {
baseClient,
presenter,
...nunjucksUtils,
})
}
}

public updateBaseClientDetails(): RequestHandler {
return async (req, res, next) => {
const userToken = res.locals.user.token
const { baseClientId } = req.params

// get current values
const baseClient = await this.baseClientService.getBaseClient(userToken, baseClientId)

// map form values to updated base client
const updatedClient = mapEditBaseClientDetailsForm(baseClient, req)

// update base client
await this.baseClientService.updateBaseClient(userToken, updatedClient)

// return to view base client page
res.redirect(`/base-clients/${baseClientId}`)
}
}
}
6 changes: 3 additions & 3 deletions server/mappers/baseClientApi/updateBaseClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ import { daysRemaining } from '../../utils/utils'

export default (baseClient: BaseClient): UpdateBaseClientRequest => {
return {
scopes: ['read', 'write'],
authorities: ['ROLE_CLIENT_CREDENTIALS'],
ips: [],
scopes: baseClient.scopes,
authorities: baseClient.clientCredentials.authorities,
ips: baseClient.config.allowedIPs,
jiraNumber: baseClient.audit,
databaseUserName: baseClient.clientCredentials.databaseUserName,
validDays: baseClient.config.expiryDate ? daysRemaining(baseClient.config.expiryDate) : null,
Expand Down
20 changes: 1 addition & 19 deletions server/mappers/forms/mapCreateBaseClientForm.ts
Original file line number Diff line number Diff line change
@@ -1,24 +1,6 @@
import type { Request } from 'express'
import { BaseClient } from '../../interfaces/baseClientApi/baseClient'
import { multiSeparatorSplit } from '../../utils/utils'

function getDayOfExpiry(daysRemaining: string) {
const daysRemainingInt = parseIntWithDefault(daysRemaining, 0)
const timeOfExpiry: Date = new Date(Date.now() + daysRemainingInt * 24 * 60 * 60 * 1000)
return timeOfExpiry.toISOString().split('T')[0]
}

function getAccessTokenValiditySeconds(accessTokenValidity: string, customAccessTokenValidity?: string) {
if (accessTokenValidity === 'custom' && customAccessTokenValidity) {
return parseIntWithDefault(customAccessTokenValidity, 0)
}
return parseIntWithDefault(accessTokenValidity, 0)
}

function parseIntWithDefault(value: string, defaultValue: number) {
const parsed = parseInt(value, 10)
return Number.isNaN(parsed) ? defaultValue : parsed
}
import { getAccessTokenValiditySeconds, getDayOfExpiry, multiSeparatorSplit } from '../../utils/utils'

export default (request: Request): BaseClient => {
// valid days is calculated from expiry date
Expand Down
127 changes: 127 additions & 0 deletions server/mappers/forms/mapEditBaseClientDetailsForm.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
import type { Request } from 'express'
import { createMock } from '@golevelup/ts-jest'
import { baseClientFactory } from '../../testutils/factories'
import { dateISOString, offsetNow } from '../../utils/utils'
import { mapEditBaseClientDetailsForm } from '../index'

const formRequest = (form: Record<string, unknown>) => {
return createMock<Request>({ body: form })
}

describe('mapEditBaseClientDetailsForm', () => {
describe('accessTokenValidity', () => {
it.each([
['1200', '1200', '', 1200],
['3600', '3600', '', 3600],
['43200', '43200', '', 43200],
['Null selection', null, '', 0],
['Custom with value', 'custom', '25', 25],
['Custom with string value', 'custom', 'blah', 0],
])(
'sets accessTokenValidity based on combination of dropdown selection %s and text box contents %s',
(_: string, selection: string, text: string, expected: number) => {
const baseClient = baseClientFactory.build({ accessTokenValidity: null })
const request = formRequest({ accessTokenValidity: selection, customAccessTokenValidity: text })

const updated = mapEditBaseClientDetailsForm(baseClient, request)

expect(updated.accessTokenValidity).toEqual(expected)
},
)
})

describe('expiry', () => {
it('defaults to null if expiry is null', () => {
const expiry: boolean = null
const expiryDays: string = null
const baseClient = baseClientFactory.build({ accessTokenValidity: null })
const request = formRequest({ expiry, expiryDays })

const presenter = mapEditBaseClientDetailsForm(baseClient, request)

expect(presenter.config.expiryDate).toBeNull()
})

it('defaults to already expired for non-numeric day count', () => {
const expiry: boolean = true
const expiryDays: string = 'blah'
const baseClient = baseClientFactory.build({ accessTokenValidity: null })
const request = formRequest({ expiry, expiryDays })

const presenter = mapEditBaseClientDetailsForm(baseClient, request)

const expected = dateISOString(offsetNow(0))
expect(presenter.config.expiryDate).toEqual(expected)
})

it('can be in the past (negative expiry days)', () => {
const expiry: boolean = true
const expiryDays: string = '-1'
const baseClient = baseClientFactory.build({ accessTokenValidity: null })
const request = formRequest({ expiry, expiryDays })

const presenter = mapEditBaseClientDetailsForm(baseClient, request)

const expected = dateISOString(offsetNow(-1))
expect(presenter.config.expiryDate).toEqual(expected)
})

it('can be in the future (positive expiry days)', () => {
const expiry: boolean = true
const expiryDays: string = '1'
const baseClient = baseClientFactory.build({ accessTokenValidity: null })
const request = formRequest({ expiry, expiryDays })

const presenter = mapEditBaseClientDetailsForm(baseClient, request)

const expected = dateISOString(offsetNow(1))
expect(presenter.config.expiryDate).toEqual(expected)
})
})

describe('updates details only', () => {
it('updates details only', () => {
// Given a base client with fields populated
const detailedBaseClient = baseClientFactory.build({
accessTokenValidity: 3600,
deployment: {
team: 'deployment team',
},
service: {
serviceName: 'service serviceName',
},
})

// and given an edit request with all fields populated
const request = formRequest({
baseClientId: detailedBaseClient.baseClientId,
clientType: 'request clientType',
approvedScopes: 'requestscope1,requestscope2',
audit: 'request audit',
grant: 'request grant',
authorities: 'requestauthority1\r\nrequestauthority2',
databaseUsername: 'request databaseUsername',
allowedIPs: 'requestallowedIP1\r\nrequestallowedIP2',
expiry: true,
expiryDays: '1',
})

// when the form is mapped
const update = mapEditBaseClientDetailsForm(detailedBaseClient, request)

// then the client details are updated
expect(update.baseClientId).toEqual(detailedBaseClient.baseClientId)
expect(update.clientType).toEqual('request clientType')
expect(update.scopes).toEqual(['requestscope1', 'requestscope2'])
expect(update.audit).toEqual('request audit')
expect(update.grantType).toEqual('request grant')
expect(update.clientCredentials.authorities).toEqual(['requestauthority1', 'requestauthority2'])
expect(update.clientCredentials.databaseUserName).toEqual('request databaseUsername')
expect(update.config.allowedIPs).toEqual(['requestallowedIP1', 'requestallowedIP2'])

// but deployment and service details are not updated
expect(update.deployment.team).toEqual(detailedBaseClient.deployment.team)
expect(update.service.serviceName).toEqual(detailedBaseClient.service.serviceName)
})
})
})
36 changes: 36 additions & 0 deletions server/mappers/forms/mapEditBaseClientDetailsForm.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import type { Request } from 'express'
import { BaseClient } from '../../interfaces/baseClientApi/baseClient'
import { getAccessTokenValiditySeconds, getDayOfExpiry, multiSeparatorSplit } from '../../utils/utils'

export default (baseClient: BaseClient, request: Request): BaseClient => {
const data = request.body

const { accessTokenValidity, customAccessTokenValidity } = data
const accessTokenValiditySeconds = getAccessTokenValiditySeconds(accessTokenValidity, customAccessTokenValidity)
const dayOfExpiry = data.expiry ? getDayOfExpiry(data.expiryDays) : null

return {
baseClientId: data.baseClientId,
clientType: data.clientType,
accessTokenValidity: accessTokenValiditySeconds,
scopes: multiSeparatorSplit(data.approvedScopes, [',', '\r\n', '\n']),
audit: data.audit,
count: baseClient.count,
grantType: data.grant,
clientCredentials: {
authorities: multiSeparatorSplit(data.authorities, [',', '\r\n', '\n']),
databaseUserName: data.databaseUsername,
},
authorisationCode: {
registeredRedirectURIs: [],
jwtFields: '',
azureAdLoginFlow: false,
},
service: baseClient.service,
deployment: baseClient.deployment,
config: {
allowedIPs: multiSeparatorSplit(data.allowedIPs, [',', '\r\n', '\n']),
expiryDate: dayOfExpiry,
},
}
}
2 changes: 2 additions & 0 deletions server/mappers/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import mapUpdateBaseClientRequest from './baseClientApi/updateBaseClient'
import mapUpdateBaseClientDeploymentRequest from './baseClientApi/updateBaseClientDeployment'
import mapListClientInstancesResponse from './baseClientApi/listClientInstances'
import mapCreateBaseClientForm from './forms/mapCreateBaseClientForm'
import mapEditBaseClientDetailsForm from './forms/mapEditBaseClientDetailsForm'

export {
mapGetBaseClientResponse,
Expand All @@ -18,4 +19,5 @@ export {
mapUpdateBaseClientDeploymentRequest,
mapListClientInstancesResponse,
mapCreateBaseClientForm,
mapEditBaseClientDetailsForm,
}
2 changes: 2 additions & 0 deletions server/routes/baseClientRouter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@ export default function baseClientRouter(services: Services): Router {

get('/', baseClientController.displayBaseClients())
get('/base-clients/new', baseClientController.displayNewBaseClient())
get('/base-clients/:baseClientId/edit', baseClientController.displayEditBaseClient())
get('/base-clients/:baseClientId', baseClientController.displayBaseClient())
post('/base-clients/new', baseClientController.createBaseClient())
post('/base-clients/:baseClientId/edit', baseClientController.updateBaseClientDetails())
return router
}
23 changes: 22 additions & 1 deletion server/utils/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,27 @@ export const offsetNow = (days: number) => {

export const offsetDate = (date: Date, days: number) => {
const newDate = new Date(date)
newDate.setDate(newDate.getDate() + days)
newDate.setDate(date.getDate() + days)
return newDate
}

export const parseIntWithDefault = (value: string, defaultValue: number) => {
const parsed = parseInt(value, 10)
return Number.isNaN(parsed) ? defaultValue : parsed
}

export const getDayOfExpiry = (daysLeft: string) => {
const daysRemainingInt = parseIntWithDefault(daysLeft, 0)
return offsetNow(daysRemainingInt).toISOString().split('T')[0]
}

export const dateISOString = (date: Date) => {
return date.toISOString().split('T')[0]
}

export const getAccessTokenValiditySeconds = (accessTokenValidity: string, customAccessTokenValidity?: string) => {
if (accessTokenValidity === 'custom' && customAccessTokenValidity) {
return parseIntWithDefault(customAccessTokenValidity, 0)
}
return parseIntWithDefault(accessTokenValidity, 0)
}
4 changes: 2 additions & 2 deletions server/views/pages/base-client.njk
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@
<h2 class="govuk-heading-l">Base client details</h2>
</div>
<div class="govuk-grid-column-one-third">
<a class="govuk-link" href="/baseClient/{{ baseClient.baseClientId }}/edit">Change
<a class="govuk-link" href="/base-clients/{{ baseClient.baseClientId }}/edit">Change
client details</a>
</div>
</div>
Expand Down Expand Up @@ -211,7 +211,7 @@
{
text: "Allowed IPs"
},{
text: toLinesHtml(baseClient.config.allowedIPs)
html: toLinesHtml(baseClient.config.allowedIPs)
}
]
]
Expand Down