Skip to content

Commit

Permalink
chore(EMS-3708): code scanning - Server-side URL redirect (#2882)
Browse files Browse the repository at this point in the history
* chore(EMS-3708): fix code scanning issue with unvalidated req.originalUrl

* chore(EMS-3708): code fixes

* chore(EMS-3707): code fixes

* chore(EMS-3707): test fix

* chore(EMS-3707): console.error fix

---------

Co-authored-by: Zain Kassam <[email protected]>
  • Loading branch information
Zainzzkk and Zain Kassam authored Aug 6, 2024
1 parent 74ed089 commit aa98cf7
Show file tree
Hide file tree
Showing 5 changed files with 152 additions and 7 deletions.
14 changes: 14 additions & 0 deletions src/ui/server/constants/regex.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,20 @@ export const REGEX = {
*/
NUMBER_HYPHEN_SPACE: /^[- \d]+$/,

/*
* VALID_REQUEST_ORIGINAL_URL:
* Regex that allows only:
* - numbers.
* - letters.
* - a hyphen.
* - a question mark.
* - a slash.
* - a dash.
* - &.
* Note: \d is exactly the same as [0-9]
*/
VALID_REQUEST_ORIGINAL_URL: /^[A-Za-z-=&?/\d]+$/,

/**
* SPACE_AND_HYPHEN:
* Regex that allows only:
Expand Down
32 changes: 27 additions & 5 deletions src/ui/server/controllers/redirects/index.test.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
import get from '.';
import generateRedirectUrl from '../../helpers/generate-redirect-url';
import { ROUTES } from '../../constants';
import { Request, Response } from '../../../types';
import { mockReq, mockRes } from '../../test-mocks';

const { PROBLEM_WITH_SERVICE } = ROUTES.INSURANCE;

describe('controllers/redirects', () => {
let req: Request;
let res: Response;
Expand All @@ -12,12 +15,31 @@ describe('controllers/redirects', () => {
res = mockRes();
});

it('should redirect to a new URL via generateRedirectUrl helper function', () => {
get(req, res);
describe('when the req.originalUrl is valid', () => {
beforeEach(() => {
req = mockReq();
});

it('should redirect to a new URL via generateRedirectUrl helper function', () => {
get(req, res);

const expected = generateRedirectUrl(req.originalUrl);

expect(res.redirect).toHaveBeenCalledTimes(1);
expect(res.redirect).toHaveBeenCalledWith(expected);
});
});

describe('when the req.original is invalid', () => {
beforeEach(() => {
req.originalUrl = '';
});

const expected = generateRedirectUrl(req.originalUrl);
it(`should redirect to a ${PROBLEM_WITH_SERVICE}`, () => {
get(req, res);

expect(res.redirect).toHaveBeenCalledTimes(1);
expect(res.redirect).toHaveBeenCalledWith(expected);
expect(res.redirect).toHaveBeenCalledTimes(1);
expect(res.redirect).toHaveBeenCalledWith(PROBLEM_WITH_SERVICE);
});
});
});
18 changes: 16 additions & 2 deletions src/ui/server/controllers/redirects/index.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,29 @@
import generateRedirectUrl from '../../helpers/generate-redirect-url';
import isValidReqOriginalUrl from '../../helpers/is-valid-req-original-url';
import { ROUTES } from '../../constants';
import { Request, Response } from '../../../types';

const { PROBLEM_WITH_SERVICE } = ROUTES.INSURANCE;

/**
* get
* Redirect an MVP_INSURANCE_ROOT URL to an INSURANCE_ROOT URL
* Redirect an MVP_INSURANCE_ROOT URL to an INSURANCE_ROOT URL if req.originalUrl is valid
* Redirect to PROBLEM_WITH_SERVICE if req.originalUrl is invalid
* @param {Express.Request} Express request
* @param {Express.Response} Express response
* @returns {Express.Response.redirect}
*/
const get = (req: Request, res: Response) => {
const newUrl = generateRedirectUrl(req.originalUrl);
const { originalUrl } = req;

const validUrl = isValidReqOriginalUrl(originalUrl);

if (!validUrl) {
console.error('Invalid original URL detected in "redirects" controller: %s', originalUrl);
return res.redirect(PROBLEM_WITH_SERVICE);
}

const newUrl = generateRedirectUrl(originalUrl);

return res.redirect(newUrl);
};
Expand Down
73 changes: 73 additions & 0 deletions src/ui/server/helpers/is-valid-req-original-url/index.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
import isValidReqOriginalUrl from '.';

describe('helpers/is-valid-req-original-url', () => {
describe('when provided url has invalid special characters', () => {
it('should return false', () => {
const mockUrl = 'abc!';

const result = isValidReqOriginalUrl(mockUrl);

expect(result).toEqual(false);
});
});

describe('when provided url is an empty string', () => {
it('should return false', () => {
const mockUrl = '';

const result = isValidReqOriginalUrl(mockUrl);

expect(result).toEqual(false);
});
});

describe('when provided url contains a space', () => {
it('should return false', () => {
const mockUrl = 'abc ';

const result = isValidReqOriginalUrl(mockUrl);

expect(result).toEqual(false);
});
});

describe('when provided url only contains letters', () => {
it('should return true', () => {
const mockUrl = 'abc';

const result = isValidReqOriginalUrl(mockUrl);

expect(result).toEqual(true);
});
});

describe('when provided url contains letters and numbers', () => {
it('should return true', () => {
const mockUrl = 'abc123';

const result = isValidReqOriginalUrl(mockUrl);

expect(result).toEqual(true);
});
});

describe('when provided url only contains numbers', () => {
it('should return true', () => {
const mockUrl = '123';

const result = isValidReqOriginalUrl(mockUrl);

expect(result).toEqual(true);
});
});

describe('when provided url contains letters, numbers, "/", "?", "&", "-" and "="', () => {
it('should return true', () => {
const mockUrl = '123abc/?&-=';

const result = isValidReqOriginalUrl(mockUrl);

expect(result).toEqual(true);
});
});
});
22 changes: 22 additions & 0 deletions src/ui/server/helpers/is-valid-req-original-url/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import Joi from 'joi';
import { REGEX } from '../../constants';

/**
* Checks if the provided original URL is valid.
* checks against NUMBER_ALPHA_HYPHEN_DASH_QUESTION_EQUALS_AND regex
* if error on validation, returns false
* otherwise, return true
* @param {string} originalUrl - The original URL to validate.
* @returns {Boolean}
*/
const isValidReqOriginalUrl = (originalUrl: string) => {
const joiString = Joi.string();

const schema = () => joiString.regex(REGEX.VALID_REQUEST_ORIGINAL_URL).required();

const validation = schema().validate(originalUrl);

return !validation.error;
};

export default isValidReqOriginalUrl;

0 comments on commit aa98cf7

Please sign in to comment.