From 7742669dfcab265d8e089b18fb7655b5fbc799f0 Mon Sep 17 00:00:00 2001 From: Peter Hudec Date: Thu, 23 May 2024 10:12:28 +0100 Subject: [PATCH 01/11] Use start NPM command instead of start:coverage in CI --- docker-compose.base.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docker-compose.base.yml b/docker-compose.base.yml index b5d40ce2f66..bacd55f9c57 100644 --- a/docker-compose.base.yml +++ b/docker-compose.base.yml @@ -23,6 +23,7 @@ services: # Required to test Data Hub roles in e2e tests, make sure this var # doesn't exists in your .env file as the override below won't work OAUTH2_DEV_TOKEN: ${OAUTH2_DEV_TOKEN:-ditStaffToken} - command: bash -c 'npm run build:for-test-coverage && npm run start:coverage' + # command: bash -c 'npm run build:for-test-coverage && npm run start:coverage' + command: bash -c 'npm run build:for-test-coverage && npm start' redis: image: redis:6.2.6 From ca44cc8b439aaadb34f69fb40dedc682663a02d5 Mon Sep 17 00:00:00 2001 From: Peter Hudec Date: Thu, 23 May 2024 10:15:03 +0100 Subject: [PATCH 02/11] Only run functional_tests in CI --- .circleci/config.yml | 60 ++++++++++++++++++++++---------------------- 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 010b7560b5a..dd6e699d82d 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -255,34 +255,34 @@ workflows: version: 2 datahub: jobs: - - lint: *ignore_ghpages - - unit_tests: *ignore_ghpages - - unit_client_tests: *ignore_ghpages + # - lint: *ignore_ghpages + # - unit_tests: *ignore_ghpages + # - unit_client_tests: *ignore_ghpages - functional_tests: *ignore_ghpages - - a11y_tests: *ignore_ghpages - - visual_tests: *ignore_ghpages - - visual_component_tests: *ignore_ghpages - - component_tests: *ignore_ghpages - - e2e_tests: - <<: *ignore_ghpages - name: e2e_tests_<< matrix.staff >> - matrix: - parameters: - staff: [da, lep] - - e2e_tests_dit: - <<: *ignore_ghpages - name: e2e_tests_dit - matrix: - parameters: - staff: [dit] - - release-storybook: - <<: *ignore_ghpages - filters: - branches: - only: - - main - - merge-and-publish-coverage: - <<: *ignore_ghpages - requires: - - functional_tests - - unit_tests + # - a11y_tests: *ignore_ghpages + # - visual_tests: *ignore_ghpages + # - visual_component_tests: *ignore_ghpages + # - component_tests: *ignore_ghpages + # - e2e_tests: + # <<: *ignore_ghpages + # name: e2e_tests_<< matrix.staff >> + # matrix: + # parameters: + # staff: [da, lep] + # - e2e_tests_dit: + # <<: *ignore_ghpages + # name: e2e_tests_dit + # matrix: + # parameters: + # staff: [dit] + # - release-storybook: + # <<: *ignore_ghpages + # filters: + # branches: + # only: + # - main + # - merge-and-publish-coverage: + # <<: *ignore_ghpages + # requires: + # - functional_tests + # - unit_tests From 6397dd1306a6adfc662238e0d20ac1cd054ded62 Mon Sep 17 00:00:00 2001 From: Peter Hudec Date: Thu, 23 May 2024 11:16:25 +0100 Subject: [PATCH 03/11] Use npm run develop --- docker-compose.base.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docker-compose.base.yml b/docker-compose.base.yml index bacd55f9c57..ab089edf58a 100644 --- a/docker-compose.base.yml +++ b/docker-compose.base.yml @@ -24,6 +24,6 @@ services: # doesn't exists in your .env file as the override below won't work OAUTH2_DEV_TOKEN: ${OAUTH2_DEV_TOKEN:-ditStaffToken} # command: bash -c 'npm run build:for-test-coverage && npm run start:coverage' - command: bash -c 'npm run build:for-test-coverage && npm start' + command: npm run develop redis: image: redis:6.2.6 From 367bbb7c6807842c8726d43c81a0782028fa5ef9 Mon Sep 17 00:00:00 2001 From: Peter Hudec Date: Thu, 23 May 2024 12:01:19 +0100 Subject: [PATCH 04/11] Unplug the headers middleware --- src/server.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/server.js b/src/server.js index 464435bb5d4..3b6995091d5 100644 --- a/src/server.js +++ b/src/server.js @@ -48,7 +48,7 @@ const exportWinsReview = require('./apps/__export-wins-review') const app = express() -app.use(headers) +// app.use(headers) if (global.__coverage__) { require('@cypress/code-coverage/middleware/express')(app) From b59e0b834392ef20a4c07131596cd8fd3ece4867 Mon Sep 17 00:00:00 2001 From: Peter Hudec Date: Thu, 23 May 2024 14:09:44 +0100 Subject: [PATCH 05/11] Removed flaky tests which were only testing response status 200 --- .../cypress/specs/contacts/activity-spec.js | 3 -- ...ntri-registration-status-attendees-spec.js | 21 ---------- .../cypress/specs/events/filter-spec.js | 31 --------------- .../cypress/specs/events/sort-spec.js | 38 ------------------- 4 files changed, 93 deletions(-) diff --git a/test/functional/cypress/specs/contacts/activity-spec.js b/test/functional/cypress/specs/contacts/activity-spec.js index 090709d1683..8380ed3ff57 100644 --- a/test/functional/cypress/specs/contacts/activity-spec.js +++ b/test/functional/cypress/specs/contacts/activity-spec.js @@ -67,9 +67,6 @@ describe('Contact activity', () => { }) it('should default to sort by newest', () => { - cy.wait('@newestRequest').then((request) => { - expect(request.response.statusCode).to.eql(200) - }) cy.get('[data-test=aventri-activity]').contains('EITA Test Event 2022') }) diff --git a/test/functional/cypress/specs/events/aventri-registration-status-attendees-spec.js b/test/functional/cypress/specs/events/aventri-registration-status-attendees-spec.js index 0a40045d9e4..c20506b8ac3 100644 --- a/test/functional/cypress/specs/events/aventri-registration-status-attendees-spec.js +++ b/test/functional/cypress/specs/events/aventri-registration-status-attendees-spec.js @@ -152,18 +152,9 @@ describe('Aventri status event registration attendees', () => { ) }) - it('should sort by "first name: A-Z" by default', () => { - cy.wait('@firstNameA-Z').then((request) => { - expect(request.response.statusCode).to.eql(200) - }) - }) - it('sorts by "first name: Z-A" when selected', () => { const element = '[data-test="sortby"] select' cy.get(element).select('first_name:desc') - cy.wait('@firstNameZ-A').then((request) => { - expect(request.response.statusCode).to.eql(200) - }) cy.get('[data-test="aventri-attendee"]') .eq(0) .should('contain', 'Diana Durrell') @@ -172,9 +163,6 @@ describe('Aventri status event registration attendees', () => { it('sorts by "last name: A-Z" when selected', () => { const element = '[data-test="sortby"] select' cy.get(element).select('last_name:asc') - cy.wait('@lastNameA-Z').then((request) => { - expect(request.response.statusCode).to.eql(200) - }) cy.get('[data-test="aventri-attendee"]') .eq(0) .should('contain', 'Alex Alderman') @@ -183,9 +171,6 @@ describe('Aventri status event registration attendees', () => { it('sorts by "last name: Z-A" when selected', () => { const element = '[data-test="sortby"] select' cy.get(element).select('last_name:desc') - cy.wait('@lastNameZ-A').then((request) => { - expect(request.response.statusCode).to.eql(200) - }) cy.get('[data-test="aventri-attendee"]') .eq(0) .should('contain', 'Diana Durrell') @@ -194,9 +179,6 @@ describe('Aventri status event registration attendees', () => { it('sorts by "company name: A-Z" when selected', () => { const element = '[data-test="sortby"] select' cy.get(element).select('company_name:asc') - cy.wait('@companyNameA-Z').then((request) => { - expect(request.response.statusCode).to.eql(200) - }) cy.get('[data-test="aventri-attendee"]') .eq(0) .should('contain', 'Alex Alderman') @@ -205,9 +187,6 @@ describe('Aventri status event registration attendees', () => { it('sorts by "company name: Z-A" when selected', () => { const element = '[data-test="sortby"] select' cy.get(element).select('company_name:desc') - cy.wait('@companyNameZ-A').then((request) => { - expect(request.response.statusCode).to.eql(200) - }) cy.get('[data-test="aventri-attendee"]') .eq(0) .should('contain', 'Diana Durrell') diff --git a/test/functional/cypress/specs/events/filter-spec.js b/test/functional/cypress/specs/events/filter-spec.js index 0180f470b68..104e62b6336 100644 --- a/test/functional/cypress/specs/events/filter-spec.js +++ b/test/functional/cypress/specs/events/filter-spec.js @@ -47,9 +47,6 @@ describe('events Collections Filter', () => { it('should pass the name to the controller', () => { cy.get(element).type(`${eventName}{enter}`) - cy.wait('@nameRequest').then((request) => { - expect(request.response.statusCode).to.eql(200) - }) // Should add input to URL query param cy.url().should('include', queryParamWithName) @@ -101,9 +98,6 @@ describe('events Collections Filter', () => { it('should pass the date to the controller', () => { cy.get(earliestStartElement).type(earliestStartDate) cy.get(latestStartElement).type(latestStartDate) - cy.wait('@dateRequest').then((request) => { - expect(request.response.statusCode).to.eql(200) - }) }) it('should add earliest start date to query param', () => { @@ -173,9 +167,6 @@ describe('events Collections Filter', () => { context('should filter from user input', () => { it('should pass the aventri Id to the controller', () => { cy.get(element).type(`${aventriId}{enter}`) - cy.wait('@aventriIdRequest').then((request) => { - expect(request.response.statusCode).to.eql(200) - }) }) it('should add an aventri ID from user input to query param', () => { @@ -236,11 +227,6 @@ describe('events Collections Filter', () => { }) context('should filter from user input and apply filter chips', () => { - it('should pass the country to the controller', () => { - cy.wait('@countryRequest').then((request) => { - expect(request.response.statusCode).to.eql(200) - }) - }) it('should pass the country from user input to query param', () => { cy.url().should('include', queryParamWithCountry) @@ -301,12 +287,6 @@ describe('events Collections Filter', () => { }) context('should filter from user input and apply filter chips', () => { - it('should pass the uk Region to the controller', () => { - cy.wait('@ukRegionRequest').then((request) => { - expect(request.response.statusCode).to.eql(200) - }) - }) - it('should pass the Uk region from user input to query param', () => { cy.url().should('include', queryParamWithUkRegion) }) @@ -352,11 +332,6 @@ describe('events Collections Filter', () => { }) context('should filter from user input and apply filter chips', () => { - it('should pass the organiser to the controller', () => { - cy.wait('@organiserRequest').then((request) => { - expect(request.response.statusCode).to.eql(200) - }) - }) it('should pass the organiser from user input to query param', () => { cy.url().should('include', queryParamWithAdvisor) @@ -399,12 +374,6 @@ describe('events Collections Filter', () => { testCheckBoxGroup({ element, value: eventType.id }) }) - it('should pass the event type to the controller', () => { - cy.wait('@eventTypeRequest').then((request) => { - expect(request.response.statusCode).to.eql(200) - }) - }) - it('should pass the event type from user input to query param', () => { cy.url().should('include', queryParamWithEventType) }) diff --git a/test/functional/cypress/specs/events/sort-spec.js b/test/functional/cypress/specs/events/sort-spec.js index 2d452224813..6f50eb4eaba 100644 --- a/test/functional/cypress/specs/events/sort-spec.js +++ b/test/functional/cypress/specs/events/sort-spec.js @@ -38,43 +38,5 @@ describe('Event Collections Sort', () => { ]) }) }) - - it('sorts by recently updated by default', () => { - cy.wait('@recentlyUpdatedRequest').then((request) => { - expect(request.response.statusCode).to.eql(200) - }) - }) - - it('sorts by "least recently updated" when selected', () => { - const element = '[data-test="sortby"] select' - cy.get(element).select('modified_on:asc') - cy.wait('@leastRecentlyUpdatedRequest').then((request) => { - expect(request.response.statusCode).to.eql(200) - }) - }) - - it('sorts by "name" when selected', () => { - const element = '[data-test="sortby"] select' - cy.get(element).select('name:asc') - cy.wait('@nameRequest').then((request) => { - expect(request.response.statusCode).to.eql(200) - }) - }) - - it('sorts by "earliest start date" when selected', () => { - const element = '[data-test="sortby"] select' - cy.get(element).select('start_date:asc') - cy.wait('@earliestStartDateRequest').then((request) => { - expect(request.response.statusCode).to.eql(200) - }) - }) - - it('sorts by "latest start date" when selected', () => { - const element = '[data-test="sortby"] select' - cy.get(element).select('start_date:desc') - cy.wait('@latestStartDateRequest').then((request) => { - expect(request.response.statusCode).to.eql(200) - }) - }) }) }) From 8fab0e51482c9c29c2f44bf551e3691cca0248a8 Mon Sep 17 00:00:00 2001 From: Peter Hudec Date: Thu, 23 May 2024 15:43:00 +0100 Subject: [PATCH 06/11] Fixed flaky test --- test/functional/cypress/specs/companies/overview-spec.js | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/test/functional/cypress/specs/companies/overview-spec.js b/test/functional/cypress/specs/companies/overview-spec.js index 831162f5b7b..32cfb883a0c 100644 --- a/test/functional/cypress/specs/companies/overview-spec.js +++ b/test/functional/cypress/specs/companies/overview-spec.js @@ -1,5 +1,4 @@ import { company } from '../../fixtures' -import { exportFaker } from '../../fakers/export' import { companyGlobalUltimateAllDetails, companyNoDetails, @@ -25,10 +24,6 @@ describe('Company overview page', () => { const allActivityUrlAllOverview = urls.companies.activity.index( fixtures.company.allOverviewDetails.id ) - const noActiveInvestments = exportFaker({ - count: 10, - results: [], - }) context( 'when viewing company overview the tab should display Overview', @@ -784,7 +779,8 @@ describe('Company overview page', () => { beforeEach(() => { cy.intercept('POST', '/api-proxy/v3/search/investment_project', { body: { - results: noActiveInvestments, + colunt: 0, + results: [], }, }).as('apiRequest') cy.visit( From 8503e921c7f3ff982b65c9e572f1aba8065e5516 Mon Sep 17 00:00:00 2001 From: Peter Hudec Date: Thu, 23 May 2024 16:01:10 +0100 Subject: [PATCH 07/11] Use hardcoded nonce --- src/middleware/headers.js | 15 +++++++++++++-- src/server.js | 2 +- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/src/middleware/headers.js b/src/middleware/headers.js index 5774f47ddc4..e975bcd015d 100644 --- a/src/middleware/headers.js +++ b/src/middleware/headers.js @@ -7,14 +7,25 @@ const STS_MAX_AGE = 180 * 24 * 60 * 60 const GOOGLE_TAG_MNGR = 'https://*.googletagmanager.com' const GOOGLE_ANALYTICS = 'https://*.google-analytics.com' +const nonBlockingNonceGenerator = () => { + let nonce = uuid() + return () => { + setTimeout(() => { + nonce = uuid() + }, 0) + return nonce + } +} + module.exports = function headers( req, res, next, // We allow nonce generator and mode to be pluggable for easy testing - { nonceGenerator = uuid, mode = config.env } = {} + { nonceGenerator = nonBlockingNonceGenerator, mode = config.env } = {} ) { - _.set(res, ['locals', 'cspNonce'], nonceGenerator()) + // _.set(res, ['locals', 'cspNonce'], nonceGenerator()) + _.set(res, ['locals', 'cspNonce'], 'foooooooo') // We have to enable unsafe-eval in tests because code instrumented // for coverage by the Istanbul library ends up with lots of evals diff --git a/src/server.js b/src/server.js index 3b6995091d5..464435bb5d4 100644 --- a/src/server.js +++ b/src/server.js @@ -48,7 +48,7 @@ const exportWinsReview = require('./apps/__export-wins-review') const app = express() -// app.use(headers) +app.use(headers) if (global.__coverage__) { require('@cypress/code-coverage/middleware/express')(app) From 350be2014857c280a8580dda5368ad504a45c195 Mon Sep 17 00:00:00 2001 From: Peter Hudec Date: Thu, 23 May 2024 16:31:00 +0100 Subject: [PATCH 08/11] Replaced _.set with vanilla JS --- src/middleware/headers.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/middleware/headers.js b/src/middleware/headers.js index e975bcd015d..eff40ce7e4b 100644 --- a/src/middleware/headers.js +++ b/src/middleware/headers.js @@ -25,7 +25,8 @@ module.exports = function headers( { nonceGenerator = nonBlockingNonceGenerator, mode = config.env } = {} ) { // _.set(res, ['locals', 'cspNonce'], nonceGenerator()) - _.set(res, ['locals', 'cspNonce'], 'foooooooo') + // _.set(res, ['locals', 'cspNonce'], 'foooooooo') + res.locals.cspNonce = 'foooooooooo' // We have to enable unsafe-eval in tests because code instrumented // for coverage by the Istanbul library ends up with lots of evals From 814cfee2d86193d1782fe9c57b2ed4f38edfdb71 Mon Sep 17 00:00:00 2001 From: Peter Hudec Date: Thu, 23 May 2024 16:58:45 +0100 Subject: [PATCH 09/11] Comment out the CSP header --- src/middleware/headers.js | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/src/middleware/headers.js b/src/middleware/headers.js index eff40ce7e4b..18c7c0def94 100644 --- a/src/middleware/headers.js +++ b/src/middleware/headers.js @@ -30,22 +30,22 @@ module.exports = function headers( // We have to enable unsafe-eval in tests because code instrumented // for coverage by the Istanbul library ends up with lots of evals - const testCoverage = mode === 'test' ? ` 'unsafe-eval'` : '' - - const selfAndNonce = `'self' 'nonce-${res.locals.cspNonce}'` - - res.set( - 'Content-Security-Policy', - [ - `default-src ${selfAndNonce}`, - // This prevents Data Hub to be loaded in iframes - `frame-ancestors 'none'`, - // Taken from https://developers.google.com/tag-platform/security/guides/csp#google_analytics_4_google_analytics - `script-src ${selfAndNonce} ${GOOGLE_TAG_MNGR}${testCoverage}`, - `img-src 'self' ${GOOGLE_ANALYTICS} ${GOOGLE_TAG_MNGR}`, - `connect-src 'self' ${GOOGLE_ANALYTICS} ${GOOGLE_TAG_MNGR} https://*.analytics.google.com`, - ].join(';') - ) + // const testCoverage = mode === 'test' ? ` 'unsafe-eval'` : '' + + // const selfAndNonce = `'self' 'nonce-${res.locals.cspNonce}'` + + // res.set( + // 'Content-Security-Policy', + // [ + // `default-src ${selfAndNonce}`, + // // This prevents Data Hub to be loaded in iframes + // `frame-ancestors 'none'`, + // // Taken from https://developers.google.com/tag-platform/security/guides/csp#google_analytics_4_google_analytics + // `script-src ${selfAndNonce} ${GOOGLE_TAG_MNGR}${testCoverage}`, + // `img-src 'self' ${GOOGLE_ANALYTICS} ${GOOGLE_TAG_MNGR}`, + // `connect-src 'self' ${GOOGLE_ANALYTICS} ${GOOGLE_TAG_MNGR} https://*.analytics.google.com`, + // ].join(';') + // ) // This is equivalent to `frame-ancestors 'none'` in the above CSP policy, // but keeping it here for older browsers From 3e03a79637d42a9c351de3a3d5274dbe41c99f7e Mon Sep 17 00:00:00 2001 From: Peter Hudec Date: Thu, 23 May 2024 17:28:21 +0100 Subject: [PATCH 10/11] Set all headers in a single step --- src/middleware/headers.js | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/src/middleware/headers.js b/src/middleware/headers.js index 18c7c0def94..e199a6a0e92 100644 --- a/src/middleware/headers.js +++ b/src/middleware/headers.js @@ -47,16 +47,18 @@ module.exports = function headers( // ].join(';') // ) - // This is equivalent to `frame-ancestors 'none'` in the above CSP policy, - // but keeping it here for older browsers - res.set('X-Frame-Options', 'DENY') - // Prevent Content Sniffing - res.set('X-Content-Type-Options', 'nosniff') - // Tell the browser to always require HTTPS - res.set('Strict-Transport-Security', `max-age=${STS_MAX_AGE}`) - // No caching as suggested in pen test report - res.set('Cache-Control', 'no-cache, no-store') - res.set('Pragma', 'no-cache') + res.set({ + // This is equivalent to `frame-ancestors 'none'` in the above CSP policy, + // but keeping it here for older browsers + 'X-Frame-Options': 'DENY', + // Prevent Content Sniffing + 'X-Content-Type-Options': 'nosniff', + // Tell the browser to always require HTTPS + 'Strict-Transport-Security': `max-age=${STS_MAX_AGE}`, + // No caching as suggested in pen test report + 'Cache-Control': 'no-cache, no-store', + Pragma: 'no-cache', + }) next() } From 633d4725a48835548b8e008187f5bd3f2549112e Mon Sep 17 00:00:00 2001 From: Peter Hudec Date: Fri, 24 May 2024 10:23:27 +0100 Subject: [PATCH 11/11] Try out a non blocking nonce generator --- src/middleware/headers.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/middleware/headers.js b/src/middleware/headers.js index e199a6a0e92..045a5443a80 100644 --- a/src/middleware/headers.js +++ b/src/middleware/headers.js @@ -22,11 +22,13 @@ module.exports = function headers( res, next, // We allow nonce generator and mode to be pluggable for easy testing - { nonceGenerator = nonBlockingNonceGenerator, mode = config.env } = {} + { nonceGenerator = nonBlockingNonceGenerator(), mode = config.env } = {} ) { - // _.set(res, ['locals', 'cspNonce'], nonceGenerator()) + const nonce = nonceGenerator() + console.log('NONCEEEEEEEEEEEEE', nonce) + _.set(res, ['locals', 'cspNonce'], nonce) // _.set(res, ['locals', 'cspNonce'], 'foooooooo') - res.locals.cspNonce = 'foooooooooo' + // res.locals.cspNonce = 'foooooooooo' // We have to enable unsafe-eval in tests because code instrumented // for coverage by the Istanbul library ends up with lots of evals