diff --git a/.husky/pre-commit b/.husky/pre-commit index ce6b52cc..a0e89392 100755 --- a/.husky/pre-commit +++ b/.husky/pre-commit @@ -1,4 +1 @@ -#!/bin/sh -. "$(dirname "$0")/_/husky.sh" - NODE_ENV=dev && node_modules/.bin/lint-staged && npm run typecheck && npm test diff --git a/integration_tests/e2e/edit-base-client-deployment.cy.ts b/integration_tests/e2e/edit-base-client-deployment.cy.ts index 019c07bf..1a0b0236 100644 --- a/integration_tests/e2e/edit-base-client-deployment.cy.ts +++ b/integration_tests/e2e/edit-base-client-deployment.cy.ts @@ -16,7 +16,7 @@ context('Edit base client deployment: Auth', () => { cy.task('stubSignIn') cy.task('stubManageUser') cy.task('stubListBaseClients') - cy.task('stubGetBaseClient', { grantType: GrantType.ClientCredentials }) + cy.task('stubGetBaseClient', { grantType: GrantType.ClientCredentials, includeService: true }) cy.task('stubGetListClientInstancesList') }) @@ -41,7 +41,7 @@ context('Edit base client deployment details page', () => { cy.task('stubSignIn') cy.task('stubManageUser') cy.task('stubListBaseClients') - cy.task('stubGetBaseClient', { grantType: GrantType.ClientCredentials }) + cy.task('stubGetBaseClient', { grantType: GrantType.ClientCredentials, includeService: true }) cy.task('stubGetListClientInstancesList') editBaseClientDeploymentDetailsPage = visitEditBaseClientDeploymentDetailsPage() }) diff --git a/integration_tests/e2e/view-base-client.cy.ts b/integration_tests/e2e/view-base-client.cy.ts index 7da90cf2..ccc44588 100644 --- a/integration_tests/e2e/view-base-client.cy.ts +++ b/integration_tests/e2e/view-base-client.cy.ts @@ -17,7 +17,7 @@ context('Base client page - Auth', () => { beforeEach(() => { cy.task('reset') cy.task('stubSignIn') - cy.task('stubGetBaseClient', { grantType: GrantType.ClientCredentials }) + cy.task('stubGetBaseClient', { grantType: GrantType.ClientCredentials, includeService: false }) cy.task('stubManageUser') cy.task('stubGetListClientInstancesList') cy.task('stubAddClientInstance') @@ -42,7 +42,7 @@ context('Base client page - client credentials flow', () => { beforeEach(() => { cy.task('reset') cy.task('stubSignIn') - cy.task('stubGetBaseClient', { grantType: GrantType.ClientCredentials }) + cy.task('stubGetBaseClient', { grantType: GrantType.ClientCredentials, includeService: false }) cy.task('stubManageUser') cy.task('stubGetListClientInstancesList') cy.task('stubAddClientInstance') @@ -112,18 +112,17 @@ context('Base client page - client credentials flow', () => { context('Base client page - authorization-code flow', () => { let baseClientsPage: ViewBaseClientPage - beforeEach(() => { - cy.task('reset') - cy.task('stubSignIn') - cy.task('stubGetBaseClient', { grantType: GrantType.AuthorizationCode }) - cy.task('stubManageUser') - cy.task('stubGetListClientInstancesList') - cy.task('stubAddClientInstance') - - baseClientsPage = visitBaseClientPage() - }) + context('Base client details - response includes service details', () => { + beforeEach(() => { + cy.task('reset') + cy.task('stubSignIn') + cy.task('stubGetBaseClient', { grantType: GrantType.AuthorizationCode, includeService: true }) + cy.task('stubManageUser') + cy.task('stubGetListClientInstancesList') + cy.task('stubAddClientInstance') + baseClientsPage = visitBaseClientPage() + }) - context('Base client details', () => { it('User can see base client details table', () => { baseClientsPage.baseClientDetailsTable().should('be.visible') }) @@ -148,4 +147,40 @@ context('Base client page - authorization-code flow', () => { baseClientsPage.baseClientServiceDetailsTable().should('be.visible') }) }) + + context('Base client details - response has null service details', () => { + beforeEach(() => { + cy.task('reset') + cy.task('stubSignIn') + cy.task('stubGetBaseClient', { grantType: GrantType.AuthorizationCode, includeService: false }) + cy.task('stubManageUser') + cy.task('stubGetListClientInstancesList') + cy.task('stubAddClientInstance') + baseClientsPage = visitBaseClientPage() + }) + + it('User can see base client details table', () => { + baseClientsPage.baseClientDetailsTable().should('be.visible') + }) + + it('User can see audit trail table', () => { + baseClientsPage.baseClientAuditTable().should('be.visible') + }) + + it('User cannot see client credentials table', () => { + baseClientsPage.baseClientClientCredentialsTable().should('not.exist') + }) + + it('User can see authorization-code table', () => { + baseClientsPage.baseClientAuthorizationCodeTable().should('be.visible') + }) + + it('User can see config table', () => { + baseClientsPage.baseClientConfigTable().should('be.visible') + }) + + it('User cannot see service details panel', () => { + baseClientsPage.baseClientServiceDetailsTable().should('not.exist') + }) + }) }) diff --git a/integration_tests/mockApis/baseClientsApi.ts b/integration_tests/mockApis/baseClientsApi.ts index 99061c05..d19c1b54 100644 --- a/integration_tests/mockApis/baseClientsApi.ts +++ b/integration_tests/mockApis/baseClientsApi.ts @@ -24,7 +24,7 @@ export default { }) }, - stubGetBaseClient: (config: { grantType: GrantType }) => { + stubGetBaseClient: (config: { grantType: GrantType; includeService: boolean }) => { return stubFor({ request: { method: 'GET', @@ -35,7 +35,7 @@ export default { headers: { 'Content-Type': 'application/json;charset=UTF-8', }, - jsonBody: getBaseClientResponseMock(config.grantType), + jsonBody: getBaseClientResponseMock(config.grantType, config.includeService), }, }) }, diff --git a/server/audit/baseClientAudit.ts b/server/audit/baseClientAudit.ts index 6551454e..1d9d2510 100644 --- a/server/audit/baseClientAudit.ts +++ b/server/audit/baseClientAudit.ts @@ -16,14 +16,16 @@ export const sendBaseClientEvent = async ( if (!config.apis.audit.enabled) { logger.info(`${baseClientEvent} - ${baseClientId} - ${JSON.stringify(details)}`) } else { - await auditService.sendAuditMessage({ + const auditMessage = { action: baseClientEvent, who: username, subjectId: baseClientId, subjectType: BASE_CLIENT_SUBJECT_TYPE, correlationId, + service: config.productId, details: JSON.stringify(details), - }) + } + await auditService.sendAuditMessage(auditMessage) } } diff --git a/server/controllers/baseClientController.test.ts b/server/controllers/baseClientController.test.ts index 4023b6d4..32f7f9ae 100644 --- a/server/controllers/baseClientController.test.ts +++ b/server/controllers/baseClientController.test.ts @@ -123,6 +123,30 @@ describe('BaseClientController', () => { expect(baseClientService.getBaseClient).toHaveBeenCalledWith(token, baseClient.baseClientId) }) + it('renders the main view of a base client with null service', async () => { + // GIVEN a base client with null service + const baseClient = baseClientFactory.build() + baseClient.service = null + const clients = clientFactory.buildList(3) + baseClientService.getBaseClient.mockResolvedValue(baseClient) + baseClientService.listClientInstances.mockResolvedValue(clients) + + // WHEN the index page is requested + request = createMock({ params: { baseClientId: baseClient.baseClientId } }) + await baseClientController.displayBaseClient()(request, response, next) + + // THEN the view base client page is rendered + const presenter = viewBaseClientPresenter(baseClient, clients) + expect(response.render).toHaveBeenCalledWith('pages/base-client.njk', { + baseClient, + presenter, + ...nunjucksUtils, + }) + + // AND the base client is retrieved from the base client service + expect(baseClientService.getBaseClient).toHaveBeenCalledWith(token, baseClient.baseClientId) + }) + it('audits the view attempt', async () => { // GIVEN a base client const baseClient = baseClientFactory.build() @@ -606,7 +630,7 @@ describe('BaseClientController', () => { it.each([ ['renders one client instance', 1, true], ['renders multiple client instances', 3, false], - ])(`if %s renders the page with isLastClient %s`, async (_, clientCount, isLastClient) => { + ])(`if %s renders the page with isLastClient %s`, async (_, clientCount: number, isLastClient: boolean) => { // GIVEN a base client const baseClient = baseClientFactory.build() const clients = clientFactory.buildList(clientCount) @@ -663,7 +687,7 @@ describe('BaseClientController', () => { it.each([ ['one client instance exists', '/', 1], ['multiple client instances', '/base-clients/abcd', 3], - ])(`if delete successful and %s, redirects to %s`, async (_, redirectURL, clientCount) => { + ])(`if delete successful and %s, redirects to %s`, async (_, redirectURL: string, clientCount: number) => { // GIVEN a base client const baseClient = baseClientFactory.build({ baseClientId: 'abcd' }) const clients = clientFactory.buildList(clientCount) diff --git a/server/data/localMockData/baseClientsResponseMock.ts b/server/data/localMockData/baseClientsResponseMock.ts index 0ba25283..5d7c66b4 100644 --- a/server/data/localMockData/baseClientsResponseMock.ts +++ b/server/data/localMockData/baseClientsResponseMock.ts @@ -36,7 +36,10 @@ export const listBaseClientsResponseMock: ListBaseClientsResponse = { ], } -export const getBaseClientResponseMock: (grantType: GrantType) => GetBaseClientResponse = (grantType: GrantType) => { +export const getBaseClientResponseMock: (grantType: GrantType, includeService: boolean) => GetBaseClientResponse = ( + grantType: GrantType, + includeService: boolean = true, +) => { const response: GetBaseClientResponse = { grantType: 'CLIENT_CREDENTIALS', clientId: 'base_client_id_1', @@ -45,7 +48,6 @@ export const getBaseClientResponseMock: (grantType: GrantType) => GetBaseClientR jiraNumber: 'jiraNumber', validDays: 1, accessTokenValiditySeconds: 3600, - serviceAuthorities: ['ROLE_ONE', 'ROLE_TWO'], deployment: { clientType: 'service', team: 'deployment team', @@ -69,7 +71,7 @@ export const getBaseClientResponseMock: (grantType: GrantType) => GetBaseClientR } } - return { + const baseClient = { ...response, grantType: 'AUTHORIZATION_CODE', redirectUris: ['redirectUri1', 'redirectUri2'], @@ -77,6 +79,20 @@ export const getBaseClientResponseMock: (grantType: GrantType) => GetBaseClientR mfa: MfaType.None, mfaRememberMe: false, } + + return includeService + ? { + ...baseClient, + service: { + name: 'service name', + description: 'service description', + authorisedRoles: ['ROLE_ONE', 'ROLE_TWO'], + url: 'https://localhost:3000', + enabled: true, + contact: 'service contact', + }, + } + : baseClient } export const getListClientInstancesResponseMock: ListClientInstancesResponse = { diff --git a/server/interfaces/baseClientApi/baseClient.ts b/server/interfaces/baseClientApi/baseClient.ts index 579a6496..a371f50e 100644 --- a/server/interfaces/baseClientApi/baseClient.ts +++ b/server/interfaces/baseClientApi/baseClient.ts @@ -13,7 +13,7 @@ export interface BaseClient { expired: boolean clientCredentials: ClientCredentialsDetails authorisationCode: AuthorisationCodeDetails - service: ServiceDetails + service?: ServiceDetails deployment: DeploymentDetails config: ClientConfig } @@ -31,13 +31,13 @@ interface AuthorisationCodeDetails { mfa: MfaType } -interface ServiceDetails { +export interface ServiceDetails { serviceName: string description: string serviceRoles: string[] url: string contact: string - status: string + status: boolean } export interface DeploymentDetails { diff --git a/server/interfaces/baseClientApi/baseClientResponse.ts b/server/interfaces/baseClientApi/baseClientResponse.ts index 4143a35a..8368cefb 100644 --- a/server/interfaces/baseClientApi/baseClientResponse.ts +++ b/server/interfaces/baseClientApi/baseClientResponse.ts @@ -40,7 +40,14 @@ export interface GetBaseClientResponse { secretKey: string deploymentInfo: string } - serviceAuthorities?: string[] + service?: { + name: string + description: string + authorisedRoles: string[] + url: string + enabled: boolean + contact: string + } } export interface ClientSecretsResponse { diff --git a/server/mappers/baseClientApi/getBaseClient.ts b/server/mappers/baseClientApi/getBaseClient.ts index 6faf1177..85a4825a 100644 --- a/server/mappers/baseClientApi/getBaseClient.ts +++ b/server/mappers/baseClientApi/getBaseClient.ts @@ -1,5 +1,5 @@ import { GetBaseClientResponse } from '../../interfaces/baseClientApi/baseClientResponse' -import { BaseClient, DeploymentDetails } from '../../interfaces/baseClientApi/baseClient' +import { BaseClient, DeploymentDetails, ServiceDetails } from '../../interfaces/baseClientApi/baseClient' import { ClientType } from '../../data/enums/clientTypes' import { HostingType } from '../../data/enums/hostingTypes' import { snake, toBaseClientId } from '../../utils/utils' @@ -23,14 +23,7 @@ export default (response: GetBaseClientResponse): BaseClient => { mfaRememberMe: response.mfaRememberMe ? response.mfaRememberMe : false, mfa: response.mfa ? response.mfa : '', }, - service: { - serviceName: '', - description: '', - serviceRoles: response.serviceAuthorities ? response.serviceAuthorities : [], - url: '', - contact: '', - status: '', - }, + service: getService(response), deployment: getDeployment(response), config: { allowedIPs: response.ips ? response.ips : [], @@ -77,3 +70,18 @@ const getDeployment = (response: GetBaseClientResponse): DeploymentDetails => { return deployment } + +const getService = (response: GetBaseClientResponse): ServiceDetails => { + if (!response.service) { + return null + } + + return { + serviceName: response.service.name, + description: response.service.description, + serviceRoles: response.service.authorisedRoles ? response.service.authorisedRoles : [], + url: response.service.url, + contact: response.service.contact, + status: response.service.enabled, + } +} diff --git a/server/mappers/baseClientApi/listBaseClients.ts b/server/mappers/baseClientApi/listBaseClients.ts index a4048730..c9ea04e7 100644 --- a/server/mappers/baseClientApi/listBaseClients.ts +++ b/server/mappers/baseClientApi/listBaseClients.ts @@ -4,6 +4,7 @@ import { BaseClient, BaseClientListFilter } from '../../interfaces/baseClientApi import { toClientType } from '../../data/enums/clientTypes' import { toGrantType } from '../../data/enums/grantType' import { kebab, multiSeparatorSplit, snake, toBaseClientId } from '../../utils/utils' +import { MfaType } from '../../data/enums/mfaTypes' export const mapListBaseClientRequest = (request: Request): BaseClientListFilter => { const asJson = JSON.stringify(request.query) @@ -71,6 +72,8 @@ export default (response: ListBaseClientsResponse): BaseClient[] => { registeredRedirectURIs: [], jwtFields: '', azureAdLoginFlow: false, + mfaRememberMe: false, + mfa: MfaType.None, }, service: { serviceName: client.teamName || '', @@ -78,7 +81,7 @@ export default (response: ListBaseClientsResponse): BaseClient[] => { serviceRoles: [], url: '', contact: '', - status: '', + status: false, }, deployment: { clientType: snake(client.clientType), @@ -90,6 +93,8 @@ export default (response: ListBaseClientsResponse): BaseClient[] => { deployment: '', secretName: '', clientIdKey: '', + secretKey: '', + deploymentInfo: '', }, config: { allowedIPs: [], diff --git a/server/mappers/forms/mapCreateBaseClientForm.ts b/server/mappers/forms/mapCreateBaseClientForm.ts index 442b5f03..23c55b88 100644 --- a/server/mappers/forms/mapCreateBaseClientForm.ts +++ b/server/mappers/forms/mapCreateBaseClientForm.ts @@ -37,7 +37,7 @@ export default (request: Request): BaseClient => { serviceRoles: [], url: '', contact: '', - status: '', + status: false, }, deployment: { clientType: '', diff --git a/server/testutils/factories/baseClient.ts b/server/testutils/factories/baseClient.ts index 8d236793..2126acb8 100644 --- a/server/testutils/factories/baseClient.ts +++ b/server/testutils/factories/baseClient.ts @@ -4,48 +4,51 @@ import { BaseClient } from '../../interfaces/baseClientApi/baseClient' import { HostingType } from '../../data/enums/hostingTypes' import { MfaType } from '../../data/enums/mfaTypes' -export default Factory.define(() => ({ - baseClientId: faker.string.uuid(), - accessTokenValidity: 3600, - scopes: ['read', 'write'], - grantType: 'client_credentials', - audit: 'audit notes', - lastAccessed: '2021-01-01T00:00:00.000Z', - expired: false, - count: 1, - clientCredentials: { - authorities: ['ROLE_CLIENT_CREDENTIALS'], - databaseUserName: 'client_credentials', - }, - authorisationCode: { - registeredRedirectURIs: ['https://localhost:3000'], - jwtFields: 'jwt fields', - azureAdLoginFlow: false, - mfaRememberMe: false, - mfa: MfaType.None, - }, - service: { - serviceName: 'service name', - description: 'service description', - serviceRoles: ['ROLE_CLIENT_CREDENTIALS'], - url: 'https://localhost:3000', - contact: 'service contact', - status: 'ACTIVE', - }, - deployment: { - clientType: 'SERVICE', - team: `${faker.word.adjective()} ${faker.word.adverb()} ${faker.word.noun()}`, - teamContact: `${faker.person.firstName()} ${faker.person.lastName()}`, - teamSlack: `${faker.internet.url()}`, - hosting: HostingType.Cloud, - namespace: 'namespace', - deployment: '', - secretKey: 'secretKey', - secretName: 'secretName', - clientIdKey: 'clientIdKey', - deploymentInfo: 'deploymentInfo', - }, - config: { - allowedIPs: ['1.0.0.127'], - }, -})) +export default Factory.define( + (): BaseClient => + ({ + baseClientId: faker.string.uuid(), + accessTokenValidity: 3600, + scopes: ['read', 'write'], + grantType: 'client_credentials', + audit: 'audit notes', + lastAccessed: '2021-01-01T00:00:00.000Z', + expired: false, + count: 1, + clientCredentials: { + authorities: ['ROLE_CLIENT_CREDENTIALS'], + databaseUserName: 'client_credentials', + }, + authorisationCode: { + registeredRedirectURIs: ['https://localhost:3000'], + jwtFields: 'jwt fields', + azureAdLoginFlow: false, + mfaRememberMe: false, + mfa: MfaType.None, + }, + service: { + serviceName: 'service name', + description: 'service description', + serviceRoles: ['ROLE_CLIENT_CREDENTIALS'], + url: 'https://localhost:3000', + contact: 'service contact', + status: true, + }, + deployment: { + clientType: 'SERVICE', + team: `${faker.word.adjective()} ${faker.word.adverb()} ${faker.word.noun()}`, + teamContact: `${faker.person.firstName()} ${faker.person.lastName()}`, + teamSlack: `${faker.internet.url()}`, + hosting: HostingType.Cloud, + namespace: 'namespace', + deployment: '', + secretKey: 'secretKey', + secretName: 'secretName', + clientIdKey: 'clientIdKey', + deploymentInfo: 'deploymentInfo', + }, + config: { + allowedIPs: ['1.0.0.127'], + }, + }) as BaseClient, +) diff --git a/server/views/helpers/nunjucksUtils.ts b/server/views/helpers/nunjucksUtils.ts index a1067808..7fa5c4b4 100644 --- a/server/views/helpers/nunjucksUtils.ts +++ b/server/views/helpers/nunjucksUtils.ts @@ -39,6 +39,10 @@ const toLines = (str?: string[]): string | null => { return str.join('\n') } +const ternary = (condition: boolean, value: string, alternative: string): string => { + return condition ? value : alternative +} + export default { toLinesHtml, toLines, @@ -49,4 +53,5 @@ export default { kebab, dateFormat, dateTimeFormat, + ternary, } diff --git a/server/views/pages/base-client.njk b/server/views/pages/base-client.njk index 09ac6b73..a63b657c 100644 --- a/server/views/pages/base-client.njk +++ b/server/views/pages/base-client.njk @@ -232,34 +232,73 @@ } }) }} - {{ govukTable({ - firstCellIsHeader: false, - head: [ - { - text: "Service details", - classes: "govuk-!-width-one-half" - },{ - text: "" - }], - rows: [ - [ + {% if presenter.displayService %} + {{ govukTable({ + firstCellIsHeader: false, + head: [ { - text: "Service roles", + text: "Service details", classes: "govuk-!-width-one-half" },{ - html: toLinesHtml(baseClient.service.serviceRoles) + text: "" + }], + rows: [ + [ + { + text: "Name", + classes: "govuk-!-width-one-half" + },{ + text: baseClient.service.serviceName + } + ], + [ + { + text: "Description", + classes: "govuk-!-width-one-half" + },{ + text: baseClient.service.description + } + ], + [ + { + text: "Roles", + classes: "govuk-!-width-one-half" + },{ + html: toLinesHtml(baseClient.service.serviceRoles) + } + ], + [ + { + text: "URL", + classes: "govuk-!-width-one-half" + },{ + text: baseClient.service.url + } + ], + [ + { + text: "Contact us URL or email", + classes: "govuk-!-width-one-half" + },{ + text: baseClient.service.contact + } + ], + [ + { + text: "Status", + classes: "govuk-!-width-one-half" + },{ + text: ternary(baseClient.service.status, "Enabled", "Disabled") + } + ] + ], + attributes: { + "data-qa": "base-client-service-table" } - ] - ], - attributes: { - "data-qa": "base-client-service-table" - } - }) }} + }) }} + {% endif %} {% endif %} - - - {{ govukTable({ firstCellIsHeader: false, head: [ diff --git a/server/views/presenters/viewBaseClientPresenter.ts b/server/views/presenters/viewBaseClientPresenter.ts index b45eed8d..85085ae1 100644 --- a/server/views/presenters/viewBaseClientPresenter.ts +++ b/server/views/presenters/viewBaseClientPresenter.ts @@ -22,5 +22,6 @@ export default (baseClient: BaseClient, clients: Client[], isReadOnly: boolean = ]), expiry: baseClient.config.expiryDate ? `Yes - days remaining ${daysRemaining(baseClient.config.expiryDate)}` : 'No', skipToAzure: baseClient.authorisationCode.azureAdLoginFlow, + displayService: baseClient.service && baseClient.service.serviceName !== '', } }