ADOP-2488: Fix fortify open-redirect issues (#1583)

* test moving checks to be on req.query.lang not temp var * move the &&s * Spacing * use array.find to ensure redirect urls/query params are part of whitelist * add request url * fix issues * linting issues * mock secret * semicolon...
hmcts · Oct 1, 2024 · 62a1f31 · 62a1f31
1 parent 4f1c187
commit 62a1f31
Show file tree

Hide file tree

Showing 4 changed files with 9 additions and 5 deletions.
diff --git a/src/main/app/controller/GetController.ts b/src/main/app/controller/GetController.ts
@@ -116,7 +116,8 @@ export class GetController {
       if (callback) {
         callback();
       } else {
-        res.redirect(req.url);
+        const redirectUrl = Object.values(Urls).find(item => item === req.url);
+        res.redirect(redirectUrl ?? Urls.HOME_URL);
       }
     });
   }

diff --git a/src/main/modules/kba/index.ts b/src/main/modules/kba/index.ts
@@ -27,7 +27,7 @@ export class KbaMiddleware {
         let param = '';
         const supportedLang = ['en', 'cy'];
         if (langCode !== null && supportedLang.includes(langCode as string)) {
-          param = '?lang=' + langCode;
+          param = '?lang=' + supportedLang.find(item => item === langCode);
         }
         if (req.session.laPortalKba?.kbaCaseRef) {
           req.session.user = await getSystemUser();

diff --git a/src/main/modules/session/index.test.ts b/src/main/modules/session/index.test.ts
@@ -1,5 +1,6 @@
 jest.mock('config');
 const mockCreateClient = jest.fn(() => 'MOCK redis client');
+const mockSecret = jest.fn(() => 'mock-secret');
 
 jest.mock('redis', () => {
   return {
@@ -48,7 +49,7 @@ describe('session', () => {
   let mockApp;
 
   beforeEach(() => {
-    config.get = jest.fn().mockImplementationOnce(() => 'MOCK_SECRET');
+    config.get = jest.fn().mockImplementationOnce(() => mockSecret);
     mockApp = {
       locals: {
         developmentMode: false,
@@ -68,7 +69,7 @@ describe('session', () => {
       name: 'adoption-web-session',
       resave: false,
       saveUninitialized: false,
-      secret: 'MOCK_SECRET',
+      secret: mockSecret,
       cookie: {
         httpOnly: true,
         maxAge: 1260000,
@@ -84,7 +85,7 @@ describe('session', () => {
     beforeEach(() => {
       config.get = jest
         .fn()
-        .mockImplementationOnce(() => 'MOCK_SECRET')
+        .mockImplementationOnce(() => mockSecret)
         .mockImplementationOnce(() => 'true')
         .mockImplementationOnce(() => 'MOCK_REDIS_HOST')
         .mockImplementationOnce(() => 'MOCK_REDIS_KEY');

diff --git a/src/main/steps/urls.ts b/src/main/steps/urls.ts
@@ -253,3 +253,5 @@ export const LA_PORTAL_CONTACT_US: PageLink = `${LA_PORTAL}/contact-us`;
 export const LA_PORTAL_CHECK_YOUR_ANSWERS: PageLink = `${LA_PORTAL}/check-your-answers`;
 export const LA_PORTAL_STATEMENT_OF_TRUTH: PageLink = `${LA_PORTAL}/statement-of-truth`;
 export const LA_PORTAL_CONFIRMATION_PAGE: PageLink = `${LA_PORTAL}/confirmation`;
+
+export const TEST_REQUEST: PageLink = '/request'; // todo - remove this and update generic tests to use a valid URL