From 63b8b5bc254a435675254df09683951a99bbcacd Mon Sep 17 00:00:00 2001 From: thomasridd Date: Wed, 27 Mar 2024 09:15:42 +0000 Subject: [PATCH 1/4] add service roles to get base-client page --- server/interfaces/baseClientApi/baseClient.ts | 2 +- .../baseClientApi/baseClientResponse.ts | 1 + server/mappers/baseClientApi/getBaseClient.ts | 6 +- server/testutils/factories/baseClient.ts | 2 +- server/views/pages/base-client.njk | 101 +++++------------- 5 files changed, 33 insertions(+), 79 deletions(-) diff --git a/server/interfaces/baseClientApi/baseClient.ts b/server/interfaces/baseClientApi/baseClient.ts index 77b493cd..579a6496 100644 --- a/server/interfaces/baseClientApi/baseClient.ts +++ b/server/interfaces/baseClientApi/baseClient.ts @@ -34,7 +34,7 @@ interface AuthorisationCodeDetails { interface ServiceDetails { serviceName: string description: string - authorisedRoles: string[] + serviceRoles: string[] url: string contact: string status: string diff --git a/server/interfaces/baseClientApi/baseClientResponse.ts b/server/interfaces/baseClientApi/baseClientResponse.ts index 26d2ed1c..4143a35a 100644 --- a/server/interfaces/baseClientApi/baseClientResponse.ts +++ b/server/interfaces/baseClientApi/baseClientResponse.ts @@ -40,6 +40,7 @@ export interface GetBaseClientResponse { secretKey: string deploymentInfo: string } + serviceAuthorities?: string[] } export interface ClientSecretsResponse { diff --git a/server/mappers/baseClientApi/getBaseClient.ts b/server/mappers/baseClientApi/getBaseClient.ts index 8edd7426..6faf1177 100644 --- a/server/mappers/baseClientApi/getBaseClient.ts +++ b/server/mappers/baseClientApi/getBaseClient.ts @@ -2,11 +2,11 @@ import { GetBaseClientResponse } from '../../interfaces/baseClientApi/baseClient import { BaseClient, DeploymentDetails } from '../../interfaces/baseClientApi/baseClient' import { ClientType } from '../../data/enums/clientTypes' import { HostingType } from '../../data/enums/hostingTypes' -import { snake } from '../../utils/utils' +import { snake, toBaseClientId } from '../../utils/utils' export default (response: GetBaseClientResponse): BaseClient => { return { - baseClientId: response.clientId, + baseClientId: toBaseClientId(response.clientId), accessTokenValidity: response.accessTokenValiditySeconds ? response.accessTokenValiditySeconds : 0, scopes: response.scopes ? response.scopes : [], grantType: snake(response.grantType), @@ -26,7 +26,7 @@ export default (response: GetBaseClientResponse): BaseClient => { service: { serviceName: '', description: '', - authorisedRoles: [], + serviceRoles: response.serviceAuthorities ? response.serviceAuthorities : [], url: '', contact: '', status: '', diff --git a/server/testutils/factories/baseClient.ts b/server/testutils/factories/baseClient.ts index 30b1632e..8d236793 100644 --- a/server/testutils/factories/baseClient.ts +++ b/server/testutils/factories/baseClient.ts @@ -27,7 +27,7 @@ export default Factory.define(() => ({ service: { serviceName: 'service name', description: 'service description', - authorisedRoles: ['ROLE_CLIENT_CREDENTIALS'], + serviceRoles: ['ROLE_CLIENT_CREDENTIALS'], url: 'https://localhost:3000', contact: 'service contact', status: 'ACTIVE', diff --git a/server/views/pages/base-client.njk b/server/views/pages/base-client.njk index e71ce2e8..06e5b49c 100644 --- a/server/views/pages/base-client.njk +++ b/server/views/pages/base-client.njk @@ -228,9 +228,35 @@ "data-qa": "base-client-authorization-code-table" } }) }} + + {{ govukTable({ + firstCellIsHeader: false, + head: [ + { + text: "Service details", + classes: "govuk-!-width-one-half" + },{ + text: "" + }], + rows: [ + [ + { + text: "Service roles", + classes: "govuk-!-width-one-half" + },{ + html: toLinesHtml(baseClient.service.serviceRoles) + } + ] + ], + attributes: { + "data-qa": "base-client-service-table" + } + }) }} {% endif %} + + {{ govukTable({ firstCellIsHeader: false, head: [ @@ -262,85 +288,12 @@ } }) }} - - {% if baseClient.grantType == "authorization_code" and presenter.enableServiceDetails %} -
-
-

Service details

-
-
- Change - service details -
-
- - {{ govukTable({ - firstCellIsHeader: false, - head: [ - { - text: "Service details", - classes: "govuk-!-width-one-half" - },{ - 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: "Authorised roles", - classes: "govuk-!-width-one-half" - },{ - html: toLinesHtml(baseClient.service.authorisedRoles) - } - ],[ - { - text: "URL", - classes: "govuk-!-width-one-half" - },{ - text: baseClient.service.url - } - ],[ - { - text: "Contact URL/email", - classes: "govuk-!-width-one-half" - },{ - text: baseClient.service.contact - } - ],[ - { - text: "Status", - classes: "govuk-!-width-one-half" - },{ - text: presenter.serviceEnabledLabel - } - ] - ], - attributes: { - "data-qa": "base-client-service-table" - } - }) }} - {% endif %} -

Deployment details

- Change - deployment details + Change deployment details
From eb2bdd3dd90d8ff837044abbf16e51e6a11d8b1d Mon Sep 17 00:00:00 2001 From: thomasridd Date: Wed, 27 Mar 2024 09:18:07 +0000 Subject: [PATCH 2/4] enable audit by default --- server/config.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/config.ts b/server/config.ts index 2f510c3c..c7d1fffd 100755 --- a/server/config.ts +++ b/server/config.ts @@ -56,7 +56,7 @@ export default { }, apis: { audit: { - enabled: get('AUDIT_ENABLED', 'false') === 'true', + enabled: get('AUDIT_ENABLED', 'true') === 'true', region: get('AUDIT_SQS_REGION', 'eu-west-2', requiredInProduction), queueUrl: get('AUDIT_SQS_QUEUE_URL', 'http://localhost:4566/000000000000/mainQueue', requiredInProduction), serviceName: get('AUDIT_SERVICE_NAME', 'authorization-ui', requiredInProduction), From aafca485c726d733b362e0ea72b5e14329e104ed Mon Sep 17 00:00:00 2001 From: thomasridd Date: Wed, 27 Mar 2024 11:23:19 +0000 Subject: [PATCH 3/4] remove redundant feature flags --- README.md | 6 ++---- feature.env | 3 +-- helm_deploy/values-dev.yaml | 2 -- helm_deploy/values-preprod.yaml | 2 -- helm_deploy/values-prod.yaml | 2 -- helm_deploy/values-stage.yaml | 2 -- server/config.ts | 2 -- server/controllers/baseClientController.test.ts | 4 ++-- server/controllers/baseClientController.ts | 5 +---- server/views/presenters/viewBaseClientPresenter.ts | 1 - 10 files changed, 6 insertions(+), 23 deletions(-) diff --git a/README.md b/README.md index 87c78e12..b163d252 100644 --- a/README.md +++ b/README.md @@ -69,11 +69,9 @@ Or run tests with the cypress UI: ## Environment variables -The following environment variables can be set to run the application: +The following environment variables can be set when running the application: -`ENABLE_AUTHORIZATION_CODE` - set to `true` to enable the authorization code grant type. Default is `false`. - -`ENABLE_SERVICE_DETAILS` - set to `true` to enable the service details section. Default is `false`. +`AUDIT_ENABLED` - Default is `true` - can be set to `false` to disable audit logging locally. Audit statements are sent to the console. ## Change log diff --git a/feature.env b/feature.env index 019757c7..73305473 100644 --- a/feature.env +++ b/feature.env @@ -10,5 +10,4 @@ API_CLIENT_ID=clientid API_CLIENT_SECRET=clientsecret SYSTEM_CLIENT_ID=clientid SYSTEM_CLIENT_SECRET=clientsecret -ENVIRONMENT_NAME=dev -ENABLE_AUTHORIZATION_CODE=true \ No newline at end of file +ENVIRONMENT_NAME=dev \ No newline at end of file diff --git a/helm_deploy/values-dev.yaml b/helm_deploy/values-dev.yaml index c180cebf..0093abb6 100644 --- a/helm_deploy/values-dev.yaml +++ b/helm_deploy/values-dev.yaml @@ -13,8 +13,6 @@ generic-service: HMPPS_AUTHORIZATION_SERVER_URL: "https://authorization-server-dev.hmpps.service.justice.gov.uk" MANAGE_USERS_API_URL: "https://manage-users-api-dev.hmpps.service.justice.gov.uk" TOKEN_VERIFICATION_API_URL: "https://token-verification-api-dev.prison.service.justice.gov.uk" - ENABLE_AUTHORIZATION_CODE: "true" - ENABLE_SERVICE_DETAILS: "false" ENVIRONMENT_NAME: DEV generic-prometheus-alerts: diff --git a/helm_deploy/values-preprod.yaml b/helm_deploy/values-preprod.yaml index c05cff33..04012222 100644 --- a/helm_deploy/values-preprod.yaml +++ b/helm_deploy/values-preprod.yaml @@ -12,8 +12,6 @@ generic-service: HMPPS_AUTHORIZATION_SERVER_URL: "https://authorization-preprod.hmpps.service.justice.gov.uk" MANAGE_USERS_API_URL: "https://manage-users-api-preprod.hmpps.service.justice.gov.uk" TOKEN_VERIFICATION_API_URL: "https://token-verification-api-preprod.prison.service.justice.gov.uk" - ENABLE_AUTHORIZATION_CODE: "true" - ENABLE_SERVICE_DETAILS: "false" ENVIRONMENT_NAME: PRE-PRODUCTION generic-prometheus-alerts: diff --git a/helm_deploy/values-prod.yaml b/helm_deploy/values-prod.yaml index f3c0c912..78355911 100644 --- a/helm_deploy/values-prod.yaml +++ b/helm_deploy/values-prod.yaml @@ -10,8 +10,6 @@ generic-service: HMPPS_AUTHORIZATION_SERVER_URL: "https://authorization.hmpps.service.justice.gov.uk" MANAGE_USERS_API_URL: "https://manage-users-api.hmpps.service.justice.gov.uk" TOKEN_VERIFICATION_API_URL: "https://token-verification-api.prison.service.justice.gov.uk" - ENABLE_AUTHORIZATION_CODE: "true" - ENABLE_SERVICE_DETAILS: "false" generic-prometheus-alerts: alertSeverity: digital-prison-service diff --git a/helm_deploy/values-stage.yaml b/helm_deploy/values-stage.yaml index 2d0287a7..b4fb4335 100644 --- a/helm_deploy/values-stage.yaml +++ b/helm_deploy/values-stage.yaml @@ -14,8 +14,6 @@ generic-service: HMPPS_AUTHORIZATION_SERVER_URL: "https://authorization-api-stage.hmpps.service.justice.gov.uk" MANAGE_USERS_API_URL: "https://manage-users-api-stage.hmpps.service.justice.gov.uk" TOKEN_VERIFICATION_API_URL: "https://token-verification-api-stage.prison.service.justice.gov.uk" - ENABLE_AUTHORIZATION_CODE: "true" - ENABLE_SERVICE_DETAILS: "false" ENVIRONMENT_NAME: STAGE generic-prometheus-alerts: diff --git a/server/config.ts b/server/config.ts index c7d1fffd..f310826b 100755 --- a/server/config.ts +++ b/server/config.ts @@ -48,8 +48,6 @@ export default { password: process.env.REDIS_PASSWORD, tls_enabled: get('REDIS_TLS_ENABLED', 'false'), }, - enableAuthorizationCode: get('ENABLE_AUTHORIZATION_CODE', 'true') === 'true', - enableServiceDetails: get('ENABLE_SERVICE_DETAILS', 'false') === 'true', session: { secret: get('SESSION_SECRET', 'app-insecure-default-session', requiredInProduction), expiryMinutes: Number(get('WEB_SESSION_TIMEOUT_IN_MINUTES', 120)), diff --git a/server/controllers/baseClientController.test.ts b/server/controllers/baseClientController.test.ts index dad1b7c6..4023b6d4 100644 --- a/server/controllers/baseClientController.test.ts +++ b/server/controllers/baseClientController.test.ts @@ -177,7 +177,7 @@ describe('BaseClientController', () => { await baseClientController.displayNewBaseClient()(request, response, next) // THEN the choose client type page is rendered - expect(response.render).toHaveBeenCalledWith('pages/new-base-client-grant.njk', expect.anything()) + expect(response.render).toHaveBeenCalledWith('pages/new-base-client-grant.njk') }) it('if grant is specified with client-credentials renders the details screen', async () => { @@ -218,7 +218,7 @@ describe('BaseClientController', () => { await baseClientController.displayNewBaseClient()(request, response, next) // THEN the choose client type page is rendered - expect(response.render).toHaveBeenCalledWith('pages/new-base-client-grant.njk', expect.anything()) + expect(response.render).toHaveBeenCalledWith('pages/new-base-client-grant.njk') }) it('if validation fails because no id specified renders the details screen with error message', async () => { diff --git a/server/controllers/baseClientController.ts b/server/controllers/baseClientController.ts index 47e53fe1..a776ffe9 100644 --- a/server/controllers/baseClientController.ts +++ b/server/controllers/baseClientController.ts @@ -13,9 +13,6 @@ import baseClientAudit, { BaseClientAuditFunction } from '../audit/baseClientAud import { BaseClientEvent } from '../audit/baseClientEvent' import { Client } from '../interfaces/baseClientApi/client' import { mapFilterToUrlQuery, mapListBaseClientRequest } from '../mappers/baseClientApi/listBaseClients' -import config from '../config' - -const { enableAuthorizationCode } = config export default class BaseClientController { constructor(private readonly baseClientService: BaseClientService) {} @@ -72,7 +69,7 @@ export default class BaseClientController { return async (req, res) => { const { grant } = req.query if (!(grant === kebab(GrantType.ClientCredentials) || grant === kebab(GrantType.AuthorizationCode))) { - res.render('pages/new-base-client-grant.njk', { enableAuthorizationCode }) + res.render('pages/new-base-client-grant.njk') return } res.render('pages/new-base-client-details.njk', { diff --git a/server/views/presenters/viewBaseClientPresenter.ts b/server/views/presenters/viewBaseClientPresenter.ts index c160a186..a7ba2ddb 100644 --- a/server/views/presenters/viewBaseClientPresenter.ts +++ b/server/views/presenters/viewBaseClientPresenter.ts @@ -21,6 +21,5 @@ export default (baseClient: BaseClient, clients: Client[]) => { ]), expiry: baseClient.config.expiryDate ? `Yes - days remaining ${daysRemaining(baseClient.config.expiryDate)}` : 'No', skipToAzureField: '', - enableServiceDetails: config.enableServiceDetails, } } From 7091ca18d2a204982322c0ebbd05a78b11bfb4d9 Mon Sep 17 00:00:00 2001 From: thomasridd Date: Wed, 27 Mar 2024 11:44:52 +0000 Subject: [PATCH 4/4] add integration test --- integration_tests/e2e/view-base-client.cy.ts | 4 ++++ integration_tests/pages/viewBaseClient.ts | 2 ++ server/data/localMockData/baseClientsResponseMock.ts | 1 + server/mappers/baseClientApi/listBaseClients.ts | 2 +- server/mappers/forms/mapCreateBaseClientForm.ts | 2 +- server/views/presenters/viewBaseClientPresenter.ts | 1 - 6 files changed, 9 insertions(+), 3 deletions(-) diff --git a/integration_tests/e2e/view-base-client.cy.ts b/integration_tests/e2e/view-base-client.cy.ts index d78788d3..7da90cf2 100644 --- a/integration_tests/e2e/view-base-client.cy.ts +++ b/integration_tests/e2e/view-base-client.cy.ts @@ -143,5 +143,9 @@ context('Base client page - authorization-code flow', () => { it('User can see config table', () => { baseClientsPage.baseClientConfigTable().should('be.visible') }) + + it('User can see service details panel', () => { + baseClientsPage.baseClientServiceDetailsTable().should('be.visible') + }) }) }) diff --git a/integration_tests/pages/viewBaseClient.ts b/integration_tests/pages/viewBaseClient.ts index 52f3cdd8..c7012cec 100644 --- a/integration_tests/pages/viewBaseClient.ts +++ b/integration_tests/pages/viewBaseClient.ts @@ -23,6 +23,8 @@ export default class ViewBaseClientPage extends Page { baseClientConfigTable = () => cy.get('[data-qa="base-client-config-table"]') + baseClientServiceDetailsTable = () => cy.get('[data-qa="base-client-service-table"]') + baseClientDeploymentContactTable = () => cy.get('[data-qa="base-client-deployment-contact-table"]') baseClientDeploymentPlatformTable = () => cy.get('[data-qa="base-client-deployment-platform-table"]') diff --git a/server/data/localMockData/baseClientsResponseMock.ts b/server/data/localMockData/baseClientsResponseMock.ts index 8c57c985..0ba25283 100644 --- a/server/data/localMockData/baseClientsResponseMock.ts +++ b/server/data/localMockData/baseClientsResponseMock.ts @@ -45,6 +45,7 @@ export const getBaseClientResponseMock: (grantType: GrantType) => GetBaseClientR jiraNumber: 'jiraNumber', validDays: 1, accessTokenValiditySeconds: 3600, + serviceAuthorities: ['ROLE_ONE', 'ROLE_TWO'], deployment: { clientType: 'service', team: 'deployment team', diff --git a/server/mappers/baseClientApi/listBaseClients.ts b/server/mappers/baseClientApi/listBaseClients.ts index d1d6c603..a4048730 100644 --- a/server/mappers/baseClientApi/listBaseClients.ts +++ b/server/mappers/baseClientApi/listBaseClients.ts @@ -75,7 +75,7 @@ export default (response: ListBaseClientsResponse): BaseClient[] => { service: { serviceName: client.teamName || '', description: '', - authorisedRoles: multiSeparatorSplit(client.roles, [' ', ',', '\n']), + serviceRoles: [], url: '', contact: '', status: '', diff --git a/server/mappers/forms/mapCreateBaseClientForm.ts b/server/mappers/forms/mapCreateBaseClientForm.ts index 5081009d..442b5f03 100644 --- a/server/mappers/forms/mapCreateBaseClientForm.ts +++ b/server/mappers/forms/mapCreateBaseClientForm.ts @@ -34,7 +34,7 @@ export default (request: Request): BaseClient => { service: { serviceName: '', description: '', - authorisedRoles: [], + serviceRoles: [], url: '', contact: '', status: '', diff --git a/server/views/presenters/viewBaseClientPresenter.ts b/server/views/presenters/viewBaseClientPresenter.ts index a7ba2ddb..9077601f 100644 --- a/server/views/presenters/viewBaseClientPresenter.ts +++ b/server/views/presenters/viewBaseClientPresenter.ts @@ -1,7 +1,6 @@ import { BaseClient } from '../../interfaces/baseClientApi/baseClient' import { Client } from '../../interfaces/baseClientApi/client' import { dateTimeFormat, daysRemaining } from '../../utils/utils' -import config from '../../config' export default (baseClient: BaseClient, clients: Client[]) => { return {