diff --git a/backend/src/admin-app.ts b/backend/src/admin-app.ts
index ff60879b3..84a1b69c8 100644
--- a/backend/src/admin-app.ts
+++ b/backend/src/admin-app.ts
@@ -3,7 +3,7 @@ import express, { NextFunction, Request, Response } from 'express';
import bodyParser from 'body-parser';
import promBundle from 'express-prom-bundle';
import { rateLimit } from 'express-rate-limit';
-import session from 'express-session';
+import session, { CookieOptions } from 'express-session';
import helmet from 'helmet';
import morgan from 'morgan';
import noCache from 'nocache';
@@ -24,15 +24,15 @@ import { logger } from './logger';
import prisma from './v1/prisma/prisma-client';
import adminAuthRouter from './v1/routes/admin-auth-routes';
import adminReportRoutes from './v1/routes/admin-report-routes';
-import adminUsersRoutes from './v1/routes/admin-users-routes';
import adminUserRouter from './v1/routes/admin-user-info-routes';
+import adminUsersRoutes from './v1/routes/admin-users-routes';
+import analyticRoutes from './v1/routes/analytic-routes';
+import announcementsRoutes from './v1/routes/announcement-routes';
import codeRouter from './v1/routes/code-routes';
import { router as configRouter } from './v1/routes/config-routes';
-import announcementsRoutes from './v1/routes/announcement-routes';
-import analyticRoutes from './v1/routes/analytic-routes';
-import resourcesRoutes from './v1/routes/resources-routes';
-import employerRoutes from './v1/routes/employer-routes';
import dashboardMetricsRouter from './v1/routes/dashboard';
+import employerRoutes from './v1/routes/employer-routes';
+import resourcesRoutes from './v1/routes/resources-routes';
import { adminAuth } from './v1/services/admin-auth-service';
import { utils } from './v1/services/utils-service';
@@ -77,10 +77,19 @@ adminApp.use(
}),
);
let proxy = true;
-const cookie = {
+const cookie: CookieOptions = {
secure: true,
httpOnly: true,
- maxAge: 1800000, //30 minutes in ms. this is same as session time. DO NOT MODIFY, IF MODIFIED, MAKE SURE SAME AS SESSION TIME OUT VALUE.
+ //30 minutes in ms. this is same as session time.
+ //DO NOT MODIFY, IF MODIFIED, MAKE SURE SAME AS SESSION TIME OUT VALUE.
+ maxAge: 1800000,
+ //sameSite: 'strict' would be preferable, but it complicates the
+ //authentication code flow of the login process. The callback
+ //url is accessed by the identify provider, which is a third party.
+ //With a strict policy the the browser won't pass the cookie to the
+ //callback endpoint. 'lax' is a middle ground that will provide
+ //some protection against CSRF attacks.
+ sameSite: 'lax',
};
if ('local' === config.get('environment')) {
cookie.secure = false;
diff --git a/backend/src/app.ts b/backend/src/app.ts
index 6c3f5e4dd..c4a4cb5c4 100644
--- a/backend/src/app.ts
+++ b/backend/src/app.ts
@@ -3,7 +3,7 @@ import express, { NextFunction, Request, Response } from 'express';
import bodyParser from 'body-parser';
import promBundle from 'express-prom-bundle';
import { rateLimit } from 'express-rate-limit';
-import session from 'express-session';
+import session, { CookieOptions } from 'express-session';
import helmet from 'helmet';
import morgan from 'morgan';
import noCache from 'nocache';
@@ -16,14 +16,14 @@ import fileSessionStore from 'session-file-store';
import { config } from './config';
import { logger } from './logger';
import prisma from './v1/prisma/prisma-client';
+import announcementRouter from './v1/routes/announcement-routes';
import codeRouter from './v1/routes/code-routes';
import { router as configRouter } from './v1/routes/config-routes';
import { fileUploadRouter } from './v1/routes/file-upload-routes';
import authRouter from './v1/routes/public-auth-routes';
import { reportRouter } from './v1/routes/report-routes';
-import userRouter from './v1/routes/user-info-routes';
-import announcementRouter from './v1/routes/announcement-routes';
import resourcesRoutes from './v1/routes/resources-routes';
+import userRouter from './v1/routes/user-info-routes';
import { publicAuth } from './v1/services/public-auth-service';
import { utils } from './v1/services/utils-service';
@@ -94,10 +94,18 @@ app.use(
}),
);
let proxy = true;
-const cookie = {
+const cookie: CookieOptions = {
secure: true,
httpOnly: true,
- maxAge: 1800000, //30 minutes in ms. this is same as session time. DO NOT MODIFY, IF MODIFIED, MAKE SURE SAME AS SESSION TIME OUT VALUE.
+ //maxAge: 30 minutes in ms. this is same as session time.
+ //DO NOT MODIFY, IF MODIFIED, MAKE SURE SAME AS SESSION TIME OUT VALUE.
+ maxAge: 1800000,
+ //sameSite: 'strict' would be preferable, but it complicates the
+ //authentication code flow of the login process (because the callback
+ //url is issued by the identify provider, which is a third party so the
+ //browser doesn't pass the cookie to the callback endpoint).
+ //'lax' is a middle ground that will
+ sameSite: 'lax',
};
if ('local' === config.get('environment')) {
cookie.secure = false;
diff --git a/backend/src/v1/routes/file-upload-routes.spec.ts b/backend/src/v1/routes/file-upload-routes.spec.ts
index b40a5cb1a..4767a11b5 100644
--- a/backend/src/v1/routes/file-upload-routes.spec.ts
+++ b/backend/src/v1/routes/file-upload-routes.spec.ts
@@ -1,9 +1,24 @@
+import bodyParser from 'body-parser';
import express, { Application } from 'express';
import request from 'supertest';
-import { fileUploadRouter } from './file-upload-routes';
import { SubmissionError } from '../services/file-upload-service';
+import { fileUploadRouter } from './file-upload-routes';
let app: Application;
+const maliciousObject = { $ne: '1' };
+const validSubmissionBody = {
+ companyName: 'ABC Company',
+ companyAddress: '123 Main St',
+ naicsCode: '11',
+ employeeCountRangeId: '123434565467678',
+ startDate: '2024-01-01',
+ endDate: '2024-12-31',
+ reportingYear: 2024,
+ dataConstraints: 'Data constraints here
',
+ comments: null,
+ rows: [],
+};
+
// Mock the module, but only override handleSubmission
const mockHandleSubmission = jest.fn().mockResolvedValue({});
jest.mock('../services/file-upload-service', () => {
@@ -31,25 +46,46 @@ describe('file-upload', () => {
beforeEach(() => {
jest.clearAllMocks();
app = express();
+ app.use(bodyParser.json());
app.use(fileUploadRouter);
});
- describe('/', () => {
- it('/ [POST] - should return 200', async () => {
- await request(app).post('/').expect(200);
- expect(mockStoreError).not.toHaveBeenCalled();
+ describe('POST /', () => {
+ describe('when the request body is valid', () => {
+ it('should return 200', async () => {
+ await request(app).post('/').send(validSubmissionBody).expect(200);
+ expect(mockStoreError).not.toHaveBeenCalled();
+ });
});
- it('/ [POST] - should return 400', async () => {
- mockHandleSubmission.mockResolvedValue(new SubmissionError('test'));
- await request(app).post('/').expect(400);
- expect(mockStoreError).toHaveBeenCalled();
+ describe('when the request body is empty', () => {
+ it('should return 400', async () => {
+ mockHandleSubmission.mockResolvedValue(new SubmissionError('test'));
+ await request(app).post('/').expect(400);
+ expect(mockStoreError).toHaveBeenCalled();
+ });
});
- it('/ [POST] - should return 500', async () => {
- mockHandleSubmission.mockImplementation(() => {
- throw Error('test');
+ [
+ 'companyName',
+ 'companyAddress',
+ 'naicsCode',
+ 'employeeCountRangeId',
+ 'startDate',
+ 'endDate',
+ 'reportingYear',
+ 'dataConstraints',
+ 'comments',
+ 'rows',
+ ].forEach((fieldName) => {
+ describe(`when ${fieldName} is invalid`, () => {
+ it('should return 400', async () => {
+ const invalidSubmissionBody = {
+ ...validSubmissionBody,
+ };
+ invalidSubmissionBody[fieldName] = maliciousObject;
+ await request(app).post('/').send(invalidSubmissionBody).expect(400);
+ expect(mockStoreError).toHaveBeenCalled();
+ });
});
- await request(app).post('/').expect(500);
- expect(mockStoreError).toHaveBeenCalled();
});
});
});
diff --git a/backend/src/v1/routes/file-upload-routes.ts b/backend/src/v1/routes/file-upload-routes.ts
index 1b8042898..7e257ed57 100644
--- a/backend/src/v1/routes/file-upload-routes.ts
+++ b/backend/src/v1/routes/file-upload-routes.ts
@@ -1,16 +1,29 @@
import express, { Request, Response } from 'express';
+import { body, validationResult } from 'express-validator';
import { logger as log } from '../../logger';
+import { errorService } from '../services/error-service';
import {
ISubmission,
SubmissionError,
fileUploadService,
} from '../services/file-upload-service';
import { utils } from '../services/utils-service';
-import { errorService } from '../services/error-service';
const fileUploadRouter = express.Router();
fileUploadRouter.post(
'/',
+ [
+ body('companyName').exists().isString(),
+ body('companyAddress').exists().isString(),
+ body('naicsCode').exists().isString(),
+ body('employeeCountRangeId').exists().isString(),
+ body('startDate').exists().isString(),
+ body('endDate').exists().isString(),
+ body('reportingYear').exists().isNumeric(),
+ body('dataConstraints').optional({ nullable: true }).isString(),
+ body('comments').optional({ nullable: true }).isString(),
+ body('rows').exists().isArray(),
+ ],
utils.asyncHandler(
async (req: Request, res: Response) => {
const session: any = req?.session;
@@ -22,6 +35,11 @@ fileUploadRouter.post(
const data: ISubmission = req.body;
let error = null;
try {
+ const preliminaryValidationErrors = validationResult(req);
+ if (!preliminaryValidationErrors.isEmpty()) {
+ error = new SubmissionError(preliminaryValidationErrors.array());
+ return res.status(400).json(error);
+ }
const result: any = await fileUploadService.handleSubmission(
userInfo,
data,
diff --git a/backend/src/v1/routes/public-auth-routes.spec.ts b/backend/src/v1/routes/public-auth-routes.spec.ts
index 43987c995..c795657d0 100644
--- a/backend/src/v1/routes/public-auth-routes.spec.ts
+++ b/backend/src/v1/routes/public-auth-routes.spec.ts
@@ -18,7 +18,7 @@ jest.mock('../services/public-auth-service', () => {
const actualPublicAuth = jest.requireActual(
'../services/public-auth-service',
);
- const mockedPublicAuth = jest.genMockFromModule(
+ const mockedPublicAuth = jest.createMockFromModule(
'../services/public-auth-service',
) as any;
@@ -61,16 +61,6 @@ jest.mock('passport', () => ({
jest.fn((req, res, next) => mockAuthenticate(req, res, next)),
}));
-const mockExists = jest.fn();
-const mockValidationResult = jest.fn();
-jest.mock('express-validator', () => ({
- ...jest.requireActual('express-validator'),
- body: (...args) => ({
- exists: () => jest.fn((req, res, next) => mockExists(req, res, next)),
- }),
- validationResult: jest.fn(() => mockValidationResult()),
-}));
-
const mockLogout = jest.fn();
const mockRequest = {
logout: jest.fn(mockLogout),
@@ -79,6 +69,7 @@ const mockRequest = {
},
user: {
idToken: 'ey....',
+ jwtFrontend: '567ghi',
},
};
@@ -264,7 +255,6 @@ describe('public-auth-routes', () => {
describe('/refresh', () => {
it('return 401 if token company details are missing', () => {
- mockExists.mockImplementation((req, res, next) => next());
app.use((req: any, res, next) => {
req.session = mockRequest.session;
req.user = mockRequest.user;
@@ -280,36 +270,31 @@ describe('public-auth-routes', () => {
expect(res.body.error).toBe(MISSING_COMPANY_DETAILS_ERROR);
});
});
- it('should handle validation errors', () => {
- mockExists.mockImplementation((req, res, next) => next());
- mockValidationResult.mockImplementation(() => {
- return {
- isEmpty: () => false,
- array: () => [1, 2],
- };
- });
+ it("if user isn't initialized, fail with 401", () => {
+ mockIsTokenExpired.mockReturnValue(false);
+ mockIsRenewable.mockReturnValue(true);
app.use((req: any, res, next) => {
req.session = { ...mockRequest.session, companyDetails: { id: 1 } };
req.user = mockRequest.user;
req.logout = mockRequest.logout;
+ req.body = { refreshToken: mockRequest.user.jwtFrontend };
next();
});
app.use('/auth', router);
return request(app)
.post('/auth/refresh')
- .expect(400)
+ .expect(401)
.expect((res) => {
- expect(res.body.errors).toEqual([1, 2]);
+ expect(res.body).toEqual({
+ error: 'Unauthorized',
+ error_description: 'Not logged in',
+ });
});
});
it('should return unauthorized when jwt or refresh token is not found', () => {
- mockExists.mockImplementation((req, res, next) => next());
- mockValidationResult.mockImplementation(() => {
- return {
- isEmpty: () => true,
- };
- });
+ mockIsTokenExpired.mockReturnValue(false);
+ mockIsRenewable.mockReturnValue(false);
app.use((req: any, res, next) => {
req.session = { ...mockRequest.session, companyDetails: { id: 1 } };
req.user = mockRequest.user;
@@ -318,29 +303,33 @@ describe('public-auth-routes', () => {
});
app.use('/auth', router);
- return request(app)
- .post('/auth/refresh')
- .expect(401)
- .expect((res) => {
- expect(res.body).toEqual({
- error: 'Unauthorized',
- error_description: 'Not logged in',
- });
- });
+ return request(app).post('/auth/refresh').expect(401);
});
- it('should renew the token if renewable', async () => {
+ it('should return 401 when the refreshToken body param contains a non-string', async () => {
const mockCorrelationId = 12;
- const mockFrontendToken = 'jwt_value';
- mockExists.mockImplementation((req, res, next) => next());
- mockValidationResult.mockImplementation(() => {
- return {
- isEmpty: () => true,
+ mockIsTokenExpired.mockReturnValue(true);
+ mockIsRenewable.mockReturnValue(true);
+
+ app.use((req: any, res, next) => {
+ req.session = {
+ ...mockRequest.session,
+ companyDetails: { id: 1 },
+ correlationID: mockCorrelationId,
};
+ req.user = { ...mockRequest.user, jwt: 'jwt', refreshToken: 'jwt' };
+ req.logout = mockRequest.logout;
+ req.body = { refreshToken: { $ne: '1' } };
+ next();
});
+ app.use('/auth', router);
+ await request(app).post('/auth/refresh').expect(401);
+ });
+ it('should renew the token if renewable', async () => {
+ const mockCorrelationId = 12;
+ const mockFrontendToken = 'jwt_value';
mockIsTokenExpired.mockReturnValue(true);
mockGenerateFrontendToken.mockReturnValue(mockFrontendToken);
mockIsRenewable.mockReturnValue(true);
- mockRenew.mockResolvedValue({ jwt: 1, refreshToken: 1 });
mockRenewBackendAndFrontendTokens.mockImplementation((req, res) =>
res.status(200).json({
jwtFrontend: mockFrontendToken,
@@ -356,6 +345,7 @@ describe('public-auth-routes', () => {
};
req.user = { ...mockRequest.user, jwt: 'jwt', refreshToken: 'jwt' };
req.logout = mockRequest.logout;
+ req.body = { refreshToken: mockRequest.user.jwtFrontend };
next();
});
app.use('/auth', router);
@@ -371,12 +361,6 @@ describe('public-auth-routes', () => {
});
it('should return unauthorized when it fails to renew tokens', () => {
const mockCorrelationId = 12;
- mockExists.mockImplementation((req, res, next) => next());
- mockValidationResult.mockImplementation(() => {
- return {
- isEmpty: () => true,
- };
- });
mockIsTokenExpired.mockReturnValue(true);
mockGenerateFrontendToken.mockReturnValue('jwt_value');
mockIsRenewable.mockReturnValue(true);
@@ -399,13 +383,7 @@ describe('public-auth-routes', () => {
return request(app).post('/auth/refresh').expect(401);
});
- it('should return unauthorized if token is not expired', () => {
- mockExists.mockImplementation((req, res, next) => next());
- mockValidationResult.mockImplementation(() => {
- return {
- isEmpty: () => true,
- };
- });
+ it('should return unauthorized if token is not renewable', () => {
mockIsTokenExpired.mockReturnValue(true);
mockIsRenewable.mockReturnValue(false);
@@ -417,21 +395,17 @@ describe('public-auth-routes', () => {
};
req.user = { ...mockRequest.user, jwt: 'jwt', refreshToken: 'jwt' };
req.logout = mockRequest.logout;
+ req.body = { refreshToken: mockRequest.user.jwtFrontend };
next();
});
app.use('/auth', router);
return request(app).post('/auth/refresh').expect(401);
});
- it('should return with current user tokens', () => {
- mockExists.mockImplementation((req, res, next) => next());
- mockValidationResult.mockImplementation(() => {
- return {
- isEmpty: () => true,
- };
- });
- mockIsTokenExpired.mockReturnValue(false);
+ it('should return 200 with current user token', () => {
+ mockIsTokenExpired.mockReturnValue(false);
+ mockIsRenewable.mockReturnValue(false);
app.use((req: any, res, next) => {
req.session = {
...mockRequest.session,
@@ -440,21 +414,21 @@ describe('public-auth-routes', () => {
};
req.user = {
...mockRequest.user,
- jwtFrontend: 'jwt',
jwt: 'jwt',
refreshToken: 'jwt',
};
req.logout = mockRequest.logout;
+ req.body = { refreshToken: mockRequest.user.jwtFrontend };
next();
});
app.use('/auth', router);
-
return request(app)
.post('/auth/refresh')
+ .set('Accept', 'application/json')
.expect(200)
.expect((res) => {
expect(res.body).toEqual({
- jwtFrontend: 'jwt',
+ jwtFrontend: mockRequest.user.jwtFrontend,
correlationID: 12,
});
});
@@ -463,7 +437,6 @@ describe('public-auth-routes', () => {
describe('/token', () => {
it('return 401 if token company details are missing', () => {
mockRefreshJWT.mockImplementation((req, res, next) => next());
- mockExists.mockImplementation((req, res, next) => next());
app.use((req: any, res, next) => {
req.session = mockRequest.session;
req.user = mockRequest.user;
@@ -481,7 +454,6 @@ describe('public-auth-routes', () => {
});
it('return 200 if jwtFrontend and refreshToken are available', () => {
mockRefreshJWT.mockImplementation((req, res, next) => next());
- mockExists.mockImplementation((req, res, next) => next());
app.use((req: any, res, next) => {
req.session = {
...mockRequest.session,
@@ -513,7 +485,6 @@ describe('public-auth-routes', () => {
});
it('return 401 unauthorized', () => {
mockRefreshJWT.mockImplementation((req, res, next) => next());
- mockExists.mockImplementation((req, res, next) => next());
app.use((req: any, res, next) => {
req.session = {
...mockRequest.session,
diff --git a/backend/src/v1/routes/public-auth-routes.ts b/backend/src/v1/routes/public-auth-routes.ts
index 75802160d..9c60cd06e 100644
--- a/backend/src/v1/routes/public-auth-routes.ts
+++ b/backend/src/v1/routes/public-auth-routes.ts
@@ -108,7 +108,16 @@ router.get(
//refreshes jwt on refresh if refreshToken is valid
router.post(
'/refresh',
- [body('refreshToken').exists()],
+ [
+ body('refreshToken').exists().isString(),
+ body('refreshToken').custom(async (refreshToken, { req }) => {
+ //the value of the 'refreshToken' attribute in the body must equal
+ //the value of the user's frontend token
+ if (!req?.user?.jwtFrontend || req?.user?.jwtFrontend != refreshToken) {
+ throw new Error('Invalid refresh token');
+ }
+ }),
+ ],
utils.asyncHandler(async (req: Request, res: Response) => {
const user: any = req.user;
const session: any = req.session;
@@ -129,9 +138,7 @@ router.post(
if (!errors.isEmpty()) {
log.error(JSON.stringify(errors));
- return res.status(400).json({
- errors: errors.array(),
- });
+ return res.status(401).json(UnauthorizedRsp);
}
if (!user?.refreshToken || !user?.jwt) {
log.error(MISSING_TOKENS_ERROR);