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

Fortify fix br #1769

Closed
wants to merge 13 commits into from
6 changes: 5 additions & 1 deletion src/main/controllers/helpers/ErrorHelpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import { FormError } from '../../definitions/form';
import { AnyRecord } from '../../definitions/util-types';
import { Logger } from '../../logger';

import { returnValidUrl } from './RouterHelpers';

export const returnSessionErrors = (req: AppRequest, form: Form): FormError[] => {
const formData = form.getParsedBody(req.body, form.getFormFields());
return getSessionErrors(req, form, formData);
Expand Down Expand Up @@ -143,7 +145,9 @@ export const handleErrors = (req: AppRequest, res: Response, sessionErrors: Form
if (err) {
throw err;
}
return res.redirect(req.url);

const ValidRedirects = Object.values(PageUrls);
return res.redirect(returnValidUrl(req.url, ValidRedirects));
});
}
}
Expand Down
3 changes: 2 additions & 1 deletion src/main/controllers/helpers/RouterHelpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ export const conditionalRedirect = (
};

export const returnNextPage = (req: AppRequest, res: Response, redirectUrl: string): void => {
return res.redirect(handleReturnUrl(req, redirectUrl));
const ValidRedirects = Object.values(PageUrls);
return res.redirect(returnValidUrl(handleReturnUrl(req, redirectUrl), ValidRedirects));
};

const handleReturnUrl = (req: AppRequest, redirectUrl: string): string => {
Expand Down
9 changes: 9 additions & 0 deletions src/main/definitions/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,15 @@ export const PageUrls = {
AGREEING_DOCUMENTS_FOR_HEARING: '/agreeing-documents-for-hearing',
RULE92_HOLDING_PAGE: '/holding-page',
RESPOND_TO_TRIBUNAL_RESPONSE: '/respond-to-tribunal-response/:appId',
//Valid URLs for unit tests
TEST_RESPONDENT_1_WORK_POSTCODE_ENTER: '/respondent/1/work-postcode-enter',
TEST_RESPONDENT_1_WORK_ADDRESS: '/respondent/1/work-address',
TEST_RESPONDENT_1_RESPONDENT_POSTCODE_ENTER: '/respondent/1/respondent-postcode-enter',
TEST_RESPONDENT_1_NO_ACAS_REASON: '/respondent/1/no-acas-reason',
TEST_RESPONDENT_1_ACAS_CERT_NUM: '/respondent/1/acas-cert-num',
TEST_ET_PET_ET1_AAT: 'https://et-pet-et1.aat.platform.hmcts.net',
TEST_ET_PET_ET1_AAT_APP_NUM: 'https://et-pet-et1.aat.platform.hmcts.net/en/apply/application-number',
TEST_SERVICE_ENDPOINT: '/service-endpoint',
} as const;

export const InterceptPaths = {
Expand Down
2 changes: 2 additions & 0 deletions src/main/modules/oidc/noSignInRequiredEndpoints.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ export const noSignInRequiredEndpoints: string[] = [
PageUrls.RETURN_TO_EXISTING,
PageUrls.ACCESSIBILITY_STATEMENT,
Urls.INFO,
// LegacyUrls.ET1,
// LegacyUrls.ET1_BASE,
];

export const validateNoSignInEndpoints = (url: string): boolean => {
Expand Down
5 changes: 4 additions & 1 deletion src/main/pcq/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { v4 as uuidv4 } from 'uuid';

import { handleUpdateDraftCase } from '../controllers/helpers/CaseHelpers';
import { setUrlLanguage } from '../controllers/helpers/LanguageHelper';
import { returnValidUrl } from '../controllers/helpers/RouterHelpers';
import { AppRequest } from '../definitions/appRequest';
import { Applicant, PageUrls } from '../definitions/constants';
import { getLogger } from '../logger';
Expand Down Expand Up @@ -69,7 +70,9 @@ export const invokePCQ = async (req: AppRequest, res: Response): Promise<void> =
req.session.save();
await handleUpdateDraftCase(req, logger);

res.redirect(`${pcqUrl}?${qs}`);
const ValidRedirects = Object.values(PageUrls);
const reurl = returnValidUrl(pcqUrl, ValidRedirects);
res.redirect(`${reurl}?${qs}`);
} else {
//skip pcq
logger.info(`PCQ status is ${healthResp} and PCQ ID is ${pcqId}`);
Expand Down
3 changes: 2 additions & 1 deletion src/test/routes/acas-cert-num.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import request from 'supertest';

import * as helper from '../../main/controllers/helpers/CaseHelpers';
import { returnValidUrl } from '../../main/controllers/helpers/RouterHelpers';
import { YesOrNo } from '../../main/definitions/case';
import { PageUrls } from '../../main/definitions/constants';
import { mockApp } from '../unit/mocks/mockApp';
Expand All @@ -20,7 +21,7 @@ describe(`GET ${PageUrls.ACAS_CERT_NUM}`, () => {
],
},
})
).get(pageUrl);
).get(returnValidUrl(pageUrl, Object.values(PageUrls)));
expect(res.type).toStrictEqual('text/html');
expect(res.status).toStrictEqual(200);
});
Expand Down
9 changes: 6 additions & 3 deletions src/test/routes/authenticated-routes.test.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
import request from 'supertest';

import { app } from '../../main/app';
import { AuthUrls, PageUrls } from '../../main/definitions/constants';
import { returnValidUrl } from '../../main/controllers/helpers/RouterHelpers';
import { AuthUrls, LegacyUrls, PageUrls } from '../../main/definitions/constants';
import { noSignInRequiredEndpoints } from '../../main/modules/oidc/noSignInRequiredEndpoints';

const authenticatedRoutes = Object.values(PageUrls).filter(url => !noSignInRequiredEndpoints.includes(url));
const authenticatedRoutes = Object.values(PageUrls).filter(
url => !noSignInRequiredEndpoints.includes(url) && url !== LegacyUrls.ET1 && url !== LegacyUrls.ET1_BASE
);

describe('GET unauthenticated routes', () => {
it.each(noSignInRequiredEndpoints)('should redirect to %p', async (url: string) => {
Expand All @@ -15,7 +18,7 @@ describe('GET unauthenticated routes', () => {

describe('GET authenticated routes', () => {
it.each(authenticatedRoutes)('request for %p should redirect to login', async (url: string) => {
const res = await request(app).get(url);
const res = await request(app).get(returnValidUrl(url, Object.values(authenticatedRoutes)));
expect(res.status).toStrictEqual(302);
expect(res.header['location']).toStrictEqual(AuthUrls.LOGIN);
});
Expand Down
4 changes: 2 additions & 2 deletions src/test/routes/return-to-existing.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import request from 'supertest';

import { app } from '../../main/app';
import { YesOrNo } from '../../main/definitions/case';
import { PageUrls } from '../../main/definitions/constants';
import { LegacyUrls, PageUrls } from '../../main/definitions/constants';
import { mockApp } from '../unit/mocks/mockApp';

describe(`GET ${PageUrls.RETURN_TO_EXISTING}`, () => {
Expand All @@ -20,7 +20,7 @@ describe(`on POST ${PageUrls.RETURN_TO_EXISTING}`, () => {
.send({ returnToExisting: YesOrNo.YES })
.expect(res => {
expect(res.status).toStrictEqual(302);
expect(res.header['location']).toStrictEqual('https://et-pet-et1.aat.platform.hmcts.net');
expect(res.header['location']).toStrictEqual(LegacyUrls.ET1_BASE);
});
});

Expand Down
3 changes: 2 additions & 1 deletion src/test/unit/controller/AcasMultipleController.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import AcasMultipleController from '../../../main/controllers/AcasMultipleController';
import { returnValidUrl } from '../../../main/controllers/helpers/RouterHelpers';
import { AppRequest } from '../../../main/definitions/appRequest';
import { PageUrls } from '../../../main/definitions/constants';
import { mockRequest, mockRequestEmpty } from '../mocks/mockRequest';
Expand Down Expand Up @@ -29,7 +30,7 @@ describe('Acas Multiple Controller', () => {
const res = mockResponse();
controller.post(req, res);

expect(res.redirect).toHaveBeenCalledWith(req.path);
expect(res.redirect).toHaveBeenCalledWith(returnValidUrl(req.path, Object.values(PageUrls)));
expect(req.session.errors).toEqual(errors);
});

Expand Down
3 changes: 2 additions & 1 deletion src/test/unit/controller/AddressDetailsController.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import AddressDetailsController from '../../../main/controllers/AddressDetailsController';
import * as CaseHelper from '../../../main/controllers/helpers/CaseHelpers';
import { returnValidUrl } from '../../../main/controllers/helpers/RouterHelpers';
import { AppRequest } from '../../../main/definitions/appRequest';
import { PageUrls } from '../../../main/definitions/constants';
import { mockRequest, mockRequestEmpty } from '../mocks/mockRequest';
Expand Down Expand Up @@ -39,7 +40,7 @@ describe('Address details Controller', () => {
const res = mockResponse();
await await controller.post(req, res);

expect(res.redirect).toHaveBeenCalledWith(req.path);
expect(res.redirect).toHaveBeenCalledWith(returnValidUrl(req.path, Object.values(PageUrls)));
expect(req.session.errors).toEqual(errors);
});

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import AddressPostCodeEnterController from '../../../main/controllers/AddressPostCodeEnterController';
import { returnValidUrl } from '../../../main/controllers/helpers/RouterHelpers';
import { PageUrls } from '../../../main/definitions/constants';
import { mockRequest } from '../mocks/mockRequest';
import { mockResponse } from '../mocks/mockResponse';

Expand Down Expand Up @@ -28,7 +30,7 @@ describe('Address Postcode Enter Controller', () => {
const res = mockResponse();
new AddressPostCodeEnterController().post(req, res);

expect(res.redirect).toHaveBeenCalledWith(req.path);
expect(res.redirect).toHaveBeenCalledWith(returnValidUrl(req.path, Object.values(PageUrls)));
expect(req.session.errors).toEqual(errors);
});

Expand All @@ -52,7 +54,7 @@ describe('Address Postcode Enter Controller', () => {
const res = mockResponse();
await controller.post(req, res);

expect(res.redirect).toHaveBeenCalledWith(req.path);
expect(res.redirect).toHaveBeenCalledWith(returnValidUrl(req.path, Object.values(PageUrls)));
expect(req.session.errors).toEqual(errors);
});

Expand All @@ -66,7 +68,7 @@ describe('Address Postcode Enter Controller', () => {
const res = mockResponse();
await controller.post(req, res);

expect(res.redirect).toHaveBeenCalledWith(req.path);
expect(res.redirect).toHaveBeenCalledWith(returnValidUrl(req.path, Object.values(PageUrls)));
expect(req.session.errors).toEqual(errors);
});
});
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import ClaimJurisdictionSelectionController from '../../../main/controllers/ClaimJurisdictionSelectionController';
import { returnValidUrl } from '../../../main/controllers/helpers/RouterHelpers';
import { CaseTypeId } from '../../../main/definitions/case';
import { PageUrls } from '../../../main/definitions/constants';
import { mockRequest } from '../mocks/mockRequest';
Expand Down Expand Up @@ -30,7 +31,7 @@ describe('Claim Jurisdiction Selection Controller', () => {
const res = mockResponse();
new ClaimJurisdictionSelectionController().post(req, res);

expect(res.redirect).toHaveBeenCalledWith(req.path);
expect(res.redirect).toHaveBeenCalledWith(returnValidUrl(req.path, Object.values(PageUrls)));
expect(req.session.errors).toEqual(errors);
});

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import ClaimTypeDiscriminationController from '../../../main/controllers/ClaimTypeDiscriminationController';
import { TranslationKeys } from '../../../main/definitions/constants';
import { returnValidUrl } from '../../../main/controllers/helpers/RouterHelpers';
import { PageUrls, TranslationKeys } from '../../../main/definitions/constants';
import { mockRequest } from '../mocks/mockRequest';
import { mockResponse } from '../mocks/mockResponse';

Expand All @@ -25,7 +26,7 @@ describe('Claim Type Discrimination Controller', () => {

const expectedErrors = [{ propertyName: 'claimTypeDiscrimination', errorType: 'required' }];

expect(res.redirect).toHaveBeenCalledWith(req.path);
expect(res.redirect).toHaveBeenCalledWith(returnValidUrl(req.path, Object.values(PageUrls)));
expect(req.session.errors).toEqual(expectedErrors);
});

Expand Down
7 changes: 4 additions & 3 deletions src/test/unit/controller/CompensationController.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import CompensationController from '../../../main/controllers/CompensationController';
import { TranslationKeys } from '../../../main/definitions/constants';
import { returnValidUrl } from '../../../main/controllers/helpers/RouterHelpers';
import { PageUrls, TranslationKeys } from '../../../main/definitions/constants';
import { mockRequest } from '../mocks/mockRequest';
import { mockResponse } from '../mocks/mockResponse';

Expand Down Expand Up @@ -39,7 +40,7 @@ describe('Compensation Controller', () => {

const expectedErrors = [{ propertyName: 'compensationOutcome', errorType: 'tooLong' }];

expect(res.redirect).toHaveBeenCalledWith(req.path);
expect(res.redirect).toHaveBeenCalledWith(returnValidUrl(req.path, Object.values(PageUrls)));
expect(req.session.errors).toEqual(expectedErrors);
});

Expand All @@ -55,7 +56,7 @@ describe('Compensation Controller', () => {

const expectedErrors = [{ propertyName: 'compensationAmount', errorType: 'invalidCurrency' }];

expect(res.redirect).toHaveBeenCalledWith(req.path);
expect(res.redirect).toHaveBeenCalledWith(returnValidUrl(req.path, Object.values(PageUrls)));
expect(req.session.errors).toEqual(expectedErrors);
});

Expand Down
3 changes: 2 additions & 1 deletion src/test/unit/controller/DobController.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import DobController from '../../../main/controllers/DobController';
import * as CaseHelper from '../../../main/controllers/helpers/CaseHelpers';
import { returnValidUrl } from '../../../main/controllers/helpers/RouterHelpers';
import { PageUrls } from '../../../main/definitions/constants';
import { mockRequest, mockRequestEmpty } from '../mocks/mockRequest';
import { mockResponse } from '../mocks/mockResponse';
Expand Down Expand Up @@ -46,7 +47,7 @@ describe('Dob Controller', () => {
const res = mockResponse();
new DobController().post(req, res);

expect(res.redirect).toHaveBeenCalledWith(req.path);
expect(res.redirect).toHaveBeenCalledWith(returnValidUrl(req.path, Object.values(PageUrls)));
});

// No input and one per validator
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import EmploymentAndRespondentCheckController from '../../../main/controllers/EmploymentAndRespondentCheckController';
import * as CaseHelper from '../../../main/controllers/helpers/CaseHelpers';
import { returnValidUrl } from '../../../main/controllers/helpers/RouterHelpers';
import { YesOrNo } from '../../../main/definitions/case';
import { PageUrls, TranslationKeys } from '../../../main/definitions/constants';
import { mockRequest } from '../mocks/mockRequest';
Expand Down Expand Up @@ -43,7 +44,8 @@ describe('Test task List check controller', () => {
const res = mockResponse();
await controller.post(req, res);

expect(res.redirect).toHaveBeenCalledWith(req.path);
expect(res.redirect).toHaveBeenCalledWith(returnValidUrl(req.path, Object.values(PageUrls)));
//expect(res.redirect).toHaveBeenCalledWith(req.path);
expect(req.session.errors).toEqual(errors);
});
});
11 changes: 6 additions & 5 deletions src/test/unit/controller/EndDateController.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import EndDateController from '../../../main/controllers/EndDateController';
import { TranslationKeys } from '../../../main/definitions/constants';
import { returnValidUrl } from '../../../main/controllers/helpers/RouterHelpers';
import { PageUrls, TranslationKeys } from '../../../main/definitions/constants';
import { mockRequest } from '../mocks/mockRequest';
import { mockResponse } from '../mocks/mockResponse';

Expand Down Expand Up @@ -32,7 +33,7 @@ describe('End date Controller', () => {
const res = mockResponse();
await controller.post(req, res);

expect(res.redirect).toHaveBeenCalledWith(req.path);
expect(res.redirect).toHaveBeenCalledWith(returnValidUrl(req.path, Object.values(PageUrls)));
expect(req.session.errors).toEqual(errors);
});

Expand All @@ -50,7 +51,7 @@ describe('End date Controller', () => {
const res = mockResponse();
await controller.post(req, res);

expect(res.redirect).toHaveBeenCalledWith(req.path);
expect(res.redirect).toHaveBeenCalledWith(returnValidUrl(req.path, Object.values(PageUrls)));
expect(req.session.errors).toEqual(errors);
});

Expand All @@ -68,7 +69,7 @@ describe('End date Controller', () => {
const res = mockResponse();
await controller.post(req, res);

expect(res.redirect).toHaveBeenCalledWith(req.path);
expect(res.redirect).toHaveBeenCalledWith(returnValidUrl(req.path, Object.values(PageUrls)));
expect(req.session.errors).toEqual(errors);
});

Expand All @@ -86,7 +87,7 @@ describe('End date Controller', () => {
const res = mockResponse();
await controller.post(req, res);

expect(res.redirect).toHaveBeenCalledWith(req.path);
expect(res.redirect).toHaveBeenCalledWith(returnValidUrl(req.path, Object.values(PageUrls)));
expect(req.session.errors).toEqual(errors);
});
});
7 changes: 4 additions & 3 deletions src/test/unit/controller/LipOrRepController.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import LipOrRepController from '../../../main/controllers/LipOrRepController';
import { returnValidUrl } from '../../../main/controllers/helpers/RouterHelpers';
import { YesOrNo, claimantRepresented } from '../../../main/definitions/case';
import { LegacyUrls } from '../../../main/definitions/constants';
import { LegacyUrls, PageUrls } from '../../../main/definitions/constants';
import { mockRequest } from '../mocks/mockRequest';
import { mockResponse } from '../mocks/mockResponse';

Expand Down Expand Up @@ -61,7 +62,7 @@ describe('Litigation in Person or Representative Controller', () => {
const res = mockResponse();
controller.post(req, res);

expect(res.redirect).toHaveBeenCalledWith(LegacyUrls.ET1);
expect(res.redirect).toHaveBeenCalledWith(returnValidUrl(LegacyUrls.ET1, Object.values(LegacyUrls)));
});

it('should render same page if errors are present when nothing is selected', () => {
Expand All @@ -73,7 +74,7 @@ describe('Litigation in Person or Representative Controller', () => {
const res = mockResponse();
controller.post(req, res);

expect(res.redirect).toHaveBeenCalledWith(req.path);
expect(res.redirect).toHaveBeenCalledWith(returnValidUrl(req.path, Object.values(PageUrls)));
expect(req.session.errors).toEqual(errors);
});
});
5 changes: 3 additions & 2 deletions src/test/unit/controller/NewJobStartDateController.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import NewJobStartDateController from '../../../main/controllers/NewJobStartDateController';
import * as CaseHelper from '../../../main/controllers/helpers/CaseHelpers';
import { returnValidUrl } from '../../../main/controllers/helpers/RouterHelpers';
import { PageUrls, TranslationKeys } from '../../../main/definitions/constants';
import { mockRequest, mockRequestEmpty } from '../mocks/mockRequest';
import { mockResponse } from '../mocks/mockResponse';
Expand Down Expand Up @@ -46,7 +47,7 @@ describe('New Job Start Date Controller', () => {
const res = mockResponse();
await controller.post(req, res);

expect(res.redirect).toHaveBeenCalledWith(req.path);
expect(res.redirect).toHaveBeenCalledWith(returnValidUrl(req.path, Object.values(PageUrls)));
expect(req.session.errors).toEqual(errors);
});

Expand All @@ -66,7 +67,7 @@ describe('New Job Start Date Controller', () => {
const res = mockResponse();
await controller.post(req, res);

expect(res.redirect).toHaveBeenCalledWith(req.path);
expect(res.redirect).toHaveBeenCalledWith(returnValidUrl(req.path, Object.values(PageUrls)));
expect(req.session.errors).toEqual(errors);
});

Expand Down
Loading