diff --git a/.eslintrc b/.eslintrc index f69d51124f..87b14ecf6e 100644 --- a/.eslintrc +++ b/.eslintrc @@ -66,9 +66,10 @@ { "files": ["*.spec.ts"], "extends": ["plugin:jest/recommended"] }, { "files": ["*.ts", "*.js"], - "excludedFiles": ["**/*.spec.ts", "**/.spec.js"], + "excludedFiles": ["**/*.spec.ts", "**/.spec.js", "**/__tests__/**/*.ts"], "rules": { - "typesafe/no-await-without-trycatch": "warn" + "typesafe/no-await-without-trycatch": "warn", + "@typescript-eslint/no-non-null-assertion": "error" } } ], diff --git a/src/app/modules/examples/examples.service.ts b/src/app/modules/examples/examples.service.ts index b4d4ef62c3..3ca8f7ba31 100644 --- a/src/app/modules/examples/examples.service.ts +++ b/src/app/modules/examples/examples.service.ts @@ -157,10 +157,6 @@ const execExamplesQuery = ( ): ResultAsync => { return ResultAsync.fromPromise( queryBuilder - // TODO(#42): Missing type in native typescript, waiting on upstream fixes. - // Tracking at https://github.com/Automattic/mongoose/issues/9714. - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore .append( selectAndProjectCardInfo(/* limit= */ PAGE_SIZE, /* offset= */ offset), ) diff --git a/src/app/modules/form/public-form/__tests__/public-form.middlewares.spec.ts b/src/app/modules/form/public-form/__tests__/public-form.middlewares.spec.ts deleted file mode 100644 index 2d2d9fa9df..0000000000 --- a/src/app/modules/form/public-form/__tests__/public-form.middlewares.spec.ts +++ /dev/null @@ -1,218 +0,0 @@ -import { ObjectId } from 'bson-ext' -import { Request } from 'express' -import { StatusCodes } from 'http-status-codes' -import { merge, times } from 'lodash' -import mongoose from 'mongoose' - -import expressHandler from 'tests/unit/backend/helpers/jest-express' - -import dbHandler from '../../../../../../tests/unit/backend/helpers/jest-db' -import { - IEncryptedSubmissionSchema, - ResponseMode, - Status, - SubmissionType, - WithForm, -} from '../../../../../types' -import getFormModel from '../../../../models/form.server.model' -import getSubmissionModel from '../../../../models/submission.server.model' -import { - checkFormSubmissionLimitAndDeactivate, - isFormPublicCheck, -} from '../public-form.middlewares' - -const FormModel = getFormModel(mongoose) -const SubmissionModel = getSubmissionModel(mongoose) - -const MOCK_ADMIN_OBJ_ID = new ObjectId() - -const MOCK_FORM_PARAMS = { - title: 'Test Form', - admin: String(MOCK_ADMIN_OBJ_ID), -} -const MOCK_ENCRYPTED_FORM_PARAMS = { - ...MOCK_FORM_PARAMS, - publicKey: 'mockPublicKey', - responseMode: ResponseMode.Encrypt, -} - -describe('public-form.middlewares', () => { - describe('checkFormSubmissionLimitAndDeactivate', () => { - beforeAll(async () => await dbHandler.connect()) - beforeEach(async () => { - await dbHandler.insertFormCollectionReqs({ - userId: MOCK_ADMIN_OBJ_ID, - }) - }) - afterEach(async () => await dbHandler.clearDatabase()) - afterAll(async () => await dbHandler.closeDatabase()) - - it('should let requests through when form has no submission limit', async () => { - const mockReq = Object.assign(expressHandler.mockRequest(), { - form: { submissionLimit: null }, - }) as WithForm - const mockRes = expressHandler.mockResponse() - const mockNext = jest.fn() - - await checkFormSubmissionLimitAndDeactivate(mockReq, mockRes, mockNext) - - expect(mockNext).toBeCalled() - expect(mockRes.sendStatus).not.toBeCalled() - expect(mockRes.status).not.toBeCalled() - expect(mockRes.json).not.toBeCalled() - }) - - it('should let requests through when form has not reached submission limit', async () => { - const formParams = merge({}, MOCK_ENCRYPTED_FORM_PARAMS, { - status: Status.Public, - submissionLimit: 10, - }) - const validForm = new FormModel(formParams) - const form = await validForm.save() - - const submissionPromises = times(5, () => - SubmissionModel.create({ - form: form._id, - myInfoFields: [], - submissionType: SubmissionType.Encrypt, - encryptedContent: 'mockEncryptedContent', - version: 1, - created: new Date('2020-01-01'), - }), - ) - await Promise.all(submissionPromises) - - const title = 'My private form' - const inactiveMessage = 'The form is not available.' - - const mockReq = Object.assign(expressHandler.mockRequest(), { - form: { - _id: form._id.toHexString(), - submissionLimit: 10, - title, - inactiveMessage, - }, - }) as WithForm - const mockRes = expressHandler.mockResponse() - const mockNext = jest.fn() - - await checkFormSubmissionLimitAndDeactivate(mockReq, mockRes, mockNext) - - expect(mockNext).toBeCalled() - expect(mockRes.sendStatus).not.toBeCalled() - expect(mockRes.status).not.toBeCalled() - expect(mockRes.json).not.toBeCalled() - }) - - it('should not let requests through and deactivate form when form has reached submission limit', async () => { - const formParams = merge({}, MOCK_ENCRYPTED_FORM_PARAMS, { - status: Status.Public, - submissionLimit: 5, - }) - const validForm = new FormModel(formParams) - const form = await validForm.save() - - const submissionPromises = times(5, () => - SubmissionModel.create({ - form: form._id, - myInfoFields: [], - submissionType: SubmissionType.Encrypt, - encryptedContent: 'mockEncryptedContent', - version: 1, - created: new Date('2020-01-01'), - }), - ) - await Promise.all(submissionPromises) - - const title = 'My private form' - const inactiveMessage = 'The form is not available.' - - const mockReq = Object.assign(expressHandler.mockRequest(), { - form: { - _id: form._id.toHexString(), - submissionLimit: 5, - title, - inactiveMessage, - }, - }) as WithForm - const mockRes = expressHandler.mockResponse() - const mockNext = jest.fn() - - await checkFormSubmissionLimitAndDeactivate(mockReq, mockRes, mockNext) - - expect(mockNext).not.toBeCalled() - expect(mockRes.json).toBeCalledWith({ - message: inactiveMessage, - isPageFound: true, - formTitle: title, - }) - expect(mockRes.status).toBeCalledWith(StatusCodes.NOT_FOUND) - - const updated = await FormModel.findById(form._id) - expect(updated!.status).toBe('PRIVATE') - }) - }) - - describe('isFormPublicCheck', () => { - it('should call next middleware function if form is public', () => { - const mockReq = Object.assign(expressHandler.mockRequest(), { - form: { status: Status.Public }, - }) as WithForm - const mockRes = expressHandler.mockResponse() - const mockNext = jest.fn() - - isFormPublicCheck(mockReq, mockRes, mockNext) - - expect(mockNext).toBeCalled() - expect(mockRes.sendStatus).not.toBeCalled() - expect(mockRes.status).not.toBeCalled() - expect(mockRes.json).not.toBeCalled() - }) - - it('should return HTTP 404 Not Found if form is private', () => { - const title = 'My private form' - const inactiveMessage = 'The form is not available.' - const form = { - status: Status.Private, - title, - inactiveMessage, - } - - const mockReq = Object.assign(expressHandler.mockRequest(), { - form, - }) as WithForm - const mockRes = expressHandler.mockResponse() - const mockNext = jest.fn() - - isFormPublicCheck(mockReq, mockRes, mockNext) - - expect(mockNext).not.toBeCalled() - expect(mockRes.sendStatus).not.toBeCalled() - expect(mockRes.status).toBeCalledWith(StatusCodes.NOT_FOUND) - expect(mockRes.json).toBeCalledWith({ - message: inactiveMessage, - isPageFound: true, - formTitle: title, - }) - }) - - it('should return HTTP 410 Gone if form is archived', () => { - const form = { - status: Status.Archived, - } - - const mockReq = Object.assign(expressHandler.mockRequest(), { - form, - }) as WithForm - const mockRes = expressHandler.mockResponse() - const mockNext = jest.fn() - - isFormPublicCheck(mockReq, mockRes, mockNext) - - expect(mockNext).not.toBeCalled() - expect(mockRes.sendStatus).toBeCalledWith(StatusCodes.GONE) - expect(mockRes.status).not.toBeCalled() - expect(mockRes.json).not.toBeCalledWith() - }) - }) -}) diff --git a/src/app/modules/form/public-form/public-form.middlewares.ts b/src/app/modules/form/public-form/public-form.middlewares.ts deleted file mode 100644 index a47ee97a98..0000000000 --- a/src/app/modules/form/public-form/public-form.middlewares.ts +++ /dev/null @@ -1,54 +0,0 @@ -import { RequestHandler } from 'express' -import { StatusCodes } from 'http-status-codes' - -import { WithForm } from '../../../../types' -import { FormDeletedError } from '../form.errors' -import { - checkFormSubmissionLimitAndDeactivateForm, - isFormPublic, -} from '../form.service' - -/** - * Express middleware function that checks if a form has exceeded its submission limits before allowing - * downstream middleware to handle the request. Otherwise, it returns a HTTP 404. - */ -export const checkFormSubmissionLimitAndDeactivate: RequestHandler = async ( - req, - res, - next, -) => { - const { form } = req as WithForm - const formResult = await checkFormSubmissionLimitAndDeactivateForm(form) - if (formResult.isErr()) { - return res.status(StatusCodes.NOT_FOUND).json({ - message: form.inactiveMessage, - isPageFound: true, // Flag to prevent default 404 subtext ("please check link") from showing - formTitle: form.title, - }) - } - - return next() -} - -/** - * Express middleware function that checks if a form attached to the Express request handler is public. - * before allowing downstream middleware to handle the request. Otherwise, it returns a HTTP 404 if the - * form is private, or HTTP 410 if the form has been archived. - * @param req - Express request object - * @param res - Express response object - * @param next - Express next middleware function - */ -export const isFormPublicCheck: RequestHandler = (req, res, next) => { - const { form } = req as WithForm - return isFormPublic(form) - .map(() => next()) - .mapErr((error) => { - return error instanceof FormDeletedError - ? res.sendStatus(StatusCodes.GONE) - : res.status(StatusCodes.NOT_FOUND).json({ - message: form.inactiveMessage, - isPageFound: true, // Flag to prevent default 404 subtext ("please check link") from showing - formTitle: form.title, - }) - }) -} diff --git a/src/app/modules/myinfo/myinfo.util.ts b/src/app/modules/myinfo/myinfo.util.ts index cf154b7274..4f5ef51a33 100644 --- a/src/app/modules/myinfo/myinfo.util.ts +++ b/src/app/modules/myinfo/myinfo.util.ts @@ -83,16 +83,6 @@ const hasMyInfoAnswer = ( return !!field.isVisible && !!field.myInfo?.attr } -const filterFieldsWithHashes = ( - responses: ProcessedFieldResponse[], - hashes: IHashes, -): VisibleMyInfoResponse[] => { - // Filter twice to get types to cooperate - return responses - .filter(hasMyInfoAnswer) - .filter((response) => !!hashes[response.myInfo.attr]) -} - const transformAnswer = (field: VisibleMyInfoResponse): string => { const answer = field.answer return field.fieldType === BasicField.Date @@ -117,14 +107,15 @@ export const compareHashedValues = ( responses: ProcessedFieldResponse[], hashes: IHashes, ): MyInfoComparePromises => { - // Filter responses to only those fields with a corresponding hash - const fieldsWithHashes = filterFieldsWithHashes(responses, hashes) // Map MyInfoAttribute to response const myInfoResponsesMap: MyInfoComparePromises = new Map() - fieldsWithHashes.forEach((field) => { - const attr = field.myInfo.attr - // Already checked that hashes contains this attr - myInfoResponsesMap.set(field._id, compareSingleHash(hashes[attr]!, field)) + responses.forEach((field) => { + if (hasMyInfoAnswer(field)) { + const hash = hashes[field.myInfo.attr] + if (hash) { + myInfoResponsesMap.set(field._id, compareSingleHash(hash, field)) + } + } }) return myInfoResponsesMap } diff --git a/src/app/modules/spcp/__tests__/spcp.factory.spec.ts b/src/app/modules/spcp/__tests__/spcp.factory.spec.ts index 2cab38f5c4..7d8903e1dc 100644 --- a/src/app/modules/spcp/__tests__/spcp.factory.spec.ts +++ b/src/app/modules/spcp/__tests__/spcp.factory.spec.ts @@ -27,9 +27,11 @@ describe('spcp.factory', () => { ) const fetchLoginPageResult = await SpcpFactory.fetchLoginPage('') const validateLoginPageResult = SpcpFactory.validateLoginPage('') - const extractJwtPayloadResult = await SpcpFactory.extractJwtPayload( + const extractSingpassJwtPayloadResult = await SpcpFactory.extractSingpassJwtPayload( + '', + ) + const extractCorppassJwtPayloadResult = await SpcpFactory.extractCorppassJwtPayload( '', - AuthType.SP, ) const parseOOBParamsResult = SpcpFactory.parseOOBParams('', '', AuthType.SP) const getSpcpAttributesResult = await SpcpFactory.getSpcpAttributes( @@ -51,7 +53,8 @@ describe('spcp.factory', () => { expect(createRedirectUrlResult._unsafeUnwrapErr()).toEqual(error) expect(fetchLoginPageResult._unsafeUnwrapErr()).toEqual(error) expect(validateLoginPageResult._unsafeUnwrapErr()).toEqual(error) - expect(extractJwtPayloadResult._unsafeUnwrapErr()).toEqual(error) + expect(extractSingpassJwtPayloadResult._unsafeUnwrapErr()).toEqual(error) + expect(extractCorppassJwtPayloadResult._unsafeUnwrapErr()).toEqual(error) expect(parseOOBParamsResult._unsafeUnwrapErr()).toEqual(error) expect(getSpcpAttributesResult._unsafeUnwrapErr()).toEqual(error) expect(createJWTResult._unsafeUnwrapErr()).toEqual(error) @@ -74,9 +77,11 @@ describe('spcp.factory', () => { ) const fetchLoginPageResult = await SpcpFactory.fetchLoginPage('') const validateLoginPageResult = SpcpFactory.validateLoginPage('') - const extractJwtPayloadResult = await SpcpFactory.extractJwtPayload( + const extractSingpassJwtPayloadResult = await SpcpFactory.extractSingpassJwtPayload( + '', + ) + const extractCorppassJwtPayloadResult = await SpcpFactory.extractCorppassJwtPayload( '', - AuthType.SP, ) const parseOOBParamsResult = SpcpFactory.parseOOBParams('', '', AuthType.SP) const getSpcpAttributesResult = await SpcpFactory.getSpcpAttributes( @@ -98,7 +103,8 @@ describe('spcp.factory', () => { expect(createRedirectUrlResult._unsafeUnwrapErr()).toEqual(error) expect(fetchLoginPageResult._unsafeUnwrapErr()).toEqual(error) expect(validateLoginPageResult._unsafeUnwrapErr()).toEqual(error) - expect(extractJwtPayloadResult._unsafeUnwrapErr()).toEqual(error) + expect(extractSingpassJwtPayloadResult._unsafeUnwrapErr()).toEqual(error) + expect(extractCorppassJwtPayloadResult._unsafeUnwrapErr()).toEqual(error) expect(parseOOBParamsResult._unsafeUnwrapErr()).toEqual(error) expect(getSpcpAttributesResult._unsafeUnwrapErr()).toEqual(error) expect(createJWTResult._unsafeUnwrapErr()).toEqual(error) diff --git a/src/app/modules/spcp/__tests__/spcp.service.spec.ts b/src/app/modules/spcp/__tests__/spcp.service.spec.ts index 43746bbcf4..dc20f58a5f 100644 --- a/src/app/modules/spcp/__tests__/spcp.service.spec.ts +++ b/src/app/modules/spcp/__tests__/spcp.service.spec.ts @@ -215,7 +215,7 @@ describe('spcp.service', () => { }) }) - describe('extractJwtPayload', () => { + describe('extractSingpassJwtPayload', () => { it('should return the correct payload for Singpass when JWT is valid', async () => { const spcpService = new SpcpService(MOCK_PARAMS) // Assumes that SP auth client was instantiated first @@ -223,7 +223,7 @@ describe('spcp.service', () => { mockClient.verifyJWT.mockImplementationOnce((jwt, cb) => cb(null, MOCK_SP_JWT_PAYLOAD), ) - const result = await spcpService.extractJwtPayload(MOCK_JWT, AuthType.SP) + const result = await spcpService.extractSingpassJwtPayload(MOCK_JWT) expect(result._unsafeUnwrap()).toEqual(MOCK_SP_JWT_PAYLOAD) }) @@ -234,7 +234,7 @@ describe('spcp.service', () => { mockClient.verifyJWT.mockImplementationOnce((_jwt, cb) => cb(new Error(), null), ) - const result = await spcpService.extractJwtPayload(MOCK_JWT, AuthType.SP) + const result = await spcpService.extractSingpassJwtPayload(MOCK_JWT) expect(result._unsafeUnwrapErr()).toEqual(new VerifyJwtError()) }) @@ -247,12 +247,14 @@ describe('spcp.service', () => { const expected = new InvalidJwtError() // Act - const result = await spcpService.extractJwtPayload(MOCK_JWT, AuthType.SP) + const result = await spcpService.extractSingpassJwtPayload(MOCK_JWT) // Assert expect(result._unsafeUnwrapErr()).toEqual(expected) }) + }) + describe('extractCorppassJwtPayload', () => { it('should return the correct payload for Corppass when JWT is valid', async () => { const spcpService = new SpcpService(MOCK_PARAMS) // Assumes that SP auth client was instantiated first @@ -260,7 +262,7 @@ describe('spcp.service', () => { mockClient.verifyJWT.mockImplementationOnce((jwt, cb) => cb(null, MOCK_CP_JWT_PAYLOAD), ) - const result = await spcpService.extractJwtPayload(MOCK_JWT, AuthType.CP) + const result = await spcpService.extractCorppassJwtPayload(MOCK_JWT) expect(result._unsafeUnwrap()).toEqual(MOCK_CP_JWT_PAYLOAD) }) @@ -271,7 +273,7 @@ describe('spcp.service', () => { mockClient.verifyJWT.mockImplementationOnce((_jwt, cb) => cb(new Error(), null), ) - const result = await spcpService.extractJwtPayload(MOCK_JWT, AuthType.CP) + const result = await spcpService.extractCorppassJwtPayload(MOCK_JWT) expect(result._unsafeUnwrapErr()).toEqual(new VerifyJwtError()) }) @@ -284,7 +286,7 @@ describe('spcp.service', () => { const expected = new InvalidJwtError() // Act - const result = await spcpService.extractJwtPayload(MOCK_JWT, AuthType.CP) + const result = await spcpService.extractCorppassJwtPayload(MOCK_JWT) // Assert expect(result._unsafeUnwrapErr()).toEqual(expected) diff --git a/src/app/modules/spcp/spcp.controller.ts b/src/app/modules/spcp/spcp.controller.ts index 42f7f57b3f..77f7c6c082 100644 --- a/src/app/modules/spcp/spcp.controller.ts +++ b/src/app/modules/spcp/spcp.controller.ts @@ -4,7 +4,7 @@ import { StatusCodes } from 'http-status-codes' import config from '../../../config/config' import { createLoggerWithLabel } from '../../../config/logger' -import { AuthType, WithForm } from '../../../types' +import { AuthType } from '../../../types' import { createReqMeta } from '../../utils/request' import { BillingFactory } from '../billing/billing.factory' import * as FormService from '../form/form.service' @@ -90,42 +90,6 @@ export const handleValidate: RequestHandler< }) } -/** - * Adds session to returned JSON if form-filler is SPCP Authenticated - * @param req - Express request object - * @param res - Express response object - * @param next - Express next middleware function - */ -export const addSpcpSessionInfo: RequestHandler = async ( - req, - res, - next, -) => { - const { authType } = (req as WithForm).form - if (authType !== AuthType.SP && authType !== AuthType.CP) return next() - - const jwtResult = SpcpFactory.extractJwt(req.cookies, authType) - // No action needed if JWT is missing, just means user is not logged in - if (jwtResult.isErr()) return next() - - return SpcpFactory.extractJwtPayload(jwtResult.value, authType) - .map(({ userName }) => { - res.locals.spcpSession = { userName } - return next() - }) - .mapErr((error) => { - logger.error({ - message: 'Failed to verify JWT with auth client', - meta: { - action: 'addSpcpSessionInfo', - ...createReqMeta(req), - }, - error, - }) - return next() - }) -} - /** * Higher-order function which returns an Express handler to handle Singpass * and Corppass login requests. diff --git a/src/app/modules/spcp/spcp.factory.ts b/src/app/modules/spcp/spcp.factory.ts index 69cc2c031c..5df9aaf191 100644 --- a/src/app/modules/spcp/spcp.factory.ts +++ b/src/app/modules/spcp/spcp.factory.ts @@ -13,7 +13,8 @@ interface ISpcpFactory { fetchLoginPage: SpcpService['fetchLoginPage'] validateLoginPage: SpcpService['validateLoginPage'] extractJwt: SpcpService['extractJwt'] - extractJwtPayload: SpcpService['extractJwtPayload'] + extractSingpassJwtPayload: SpcpService['extractSingpassJwtPayload'] + extractCorppassJwtPayload: SpcpService['extractCorppassJwtPayload'] parseOOBParams: SpcpService['parseOOBParams'] getSpcpAttributes: SpcpService['getSpcpAttributes'] createJWT: SpcpService['createJWT'] @@ -32,7 +33,8 @@ export const createSpcpFactory = ({ createRedirectUrl: () => err(error), fetchLoginPage: () => errAsync(error), validateLoginPage: () => err(error), - extractJwtPayload: () => errAsync(error), + extractSingpassJwtPayload: () => errAsync(error), + extractCorppassJwtPayload: () => errAsync(error), extractJwt: () => err(error), parseOOBParams: () => err(error), getSpcpAttributes: () => errAsync(error), diff --git a/src/app/modules/spcp/spcp.service.ts b/src/app/modules/spcp/spcp.service.ts index 74911cdbda..81e353c2a0 100644 --- a/src/app/modules/spcp/spcp.service.ts +++ b/src/app/modules/spcp/spcp.service.ts @@ -22,11 +22,13 @@ import { } from './spcp.errors' import { CorppassAttributes, + CorppassJwtPayload, JwtName, JwtPayload, LoginPageValidationResult, ParsedSpcpParams, SingpassAttributes, + SingpassJwtPayload, SpcpCookies, SpcpDomainSettings, } from './spcp.types' @@ -34,7 +36,8 @@ import { extractFormId, getAttributesPromise, getSubstringBetween, - isJwtPayload, + isCorppassJwtPayload, + isSingpassJwtPayload, isValidAuthenticationQuery, verifyJwtPromise, } from './spcp.util' @@ -205,19 +208,16 @@ export class SpcpService { } /** - * Verifies a JWT and extracts its payload. + * Verifies a Singpass JWT and extracts its payload. * @param jwt The contents of the JWT cookie - * @param authType 'SP' or 'CP' */ - extractJwtPayload( + extractSingpassJwtPayload( jwt: string, - authType: AuthType.SP | AuthType.CP, - ): ResultAsync { + ): ResultAsync { const logMeta = { - action: 'extractJwtPayload', - authType, + action: 'extractSingpassJwtPayload', } - const authClient = this.getAuthClient(authType) + const authClient = this.getAuthClient(AuthType.SP) return ResultAsync.fromPromise( verifyJwtPromise(authClient, jwt), (error) => { @@ -229,7 +229,47 @@ export class SpcpService { return new VerifyJwtError() }, ).andThen((payload) => { - if (isJwtPayload(payload, authType)) { + if (isSingpassJwtPayload(payload)) { + return okAsync(payload) + } + const payloadIsDefined = !!payload + const payloadKeys = + typeof payload === 'object' && !!payload && Object.keys(payload) + logger.error({ + message: 'JWT has incorrect shape', + meta: { + ...logMeta, + payloadIsDefined, + payloadKeys, + }, + }) + return errAsync(new InvalidJwtError()) + }) + } + + /** + * Verifies a Corppass JWT and extracts its payload. + * @param jwt The contents of the JWT cookie + */ + extractCorppassJwtPayload( + jwt: string, + ): ResultAsync { + const logMeta = { + action: 'extractCorppassJwtPayload', + } + const authClient = this.getAuthClient(AuthType.CP) + return ResultAsync.fromPromise( + verifyJwtPromise(authClient, jwt), + (error) => { + logger.error({ + message: 'Failed to verify JWT with auth client', + meta: logMeta, + error, + }) + return new VerifyJwtError() + }, + ).andThen((payload) => { + if (isCorppassJwtPayload(payload)) { return okAsync(payload) } const payloadIsDefined = !!payload @@ -408,8 +448,13 @@ export class SpcpService { JwtPayload, VerifyJwtError | InvalidJwtError | MissingJwtError > { - return this.extractJwt(cookies, authType).asyncAndThen((jwtResult) => - this.extractJwtPayload(jwtResult, authType), - ) + return this.extractJwt(cookies, authType).asyncAndThen((jwtResult) => { + switch (authType) { + case AuthType.SP: + return this.extractSingpassJwtPayload(jwtResult) + case AuthType.CP: + return this.extractCorppassJwtPayload(jwtResult) + } + }) } } diff --git a/src/app/modules/spcp/spcp.types.ts b/src/app/modules/spcp/spcp.types.ts index 2eb486174e..a13976278a 100644 --- a/src/app/modules/spcp/spcp.types.ts +++ b/src/app/modules/spcp/spcp.types.ts @@ -9,12 +9,19 @@ export type LoginPageValidationResult = export type SpcpCookies = Partial> -export type JwtPayload = { +export type SingpassJwtPayload = { userName: string - userInfo?: string rememberMe: boolean } +export type CorppassJwtPayload = { + userName: string + userInfo: string + rememberMe: boolean +} + +export type JwtPayload = SingpassJwtPayload | CorppassJwtPayload + export interface SingpassAttributes { UserName?: string } diff --git a/src/app/modules/spcp/spcp.util.ts b/src/app/modules/spcp/spcp.util.ts index fc7bf12568..9c12577194 100644 --- a/src/app/modules/spcp/spcp.util.ts +++ b/src/app/modules/spcp/spcp.util.ts @@ -3,12 +3,8 @@ import crypto from 'crypto' import { StatusCodes } from 'http-status-codes' import { createLoggerWithLabel } from '../../../config/logger' -import { - AuthType, - BasicField, - MapRouteError, - SPCPFieldTitle, -} from '../../../types' +import { BasicField, MapRouteError, SPCPFieldTitle } from '../../../types' +import { hasProp } from '../../utils/has-prop' import { MissingFeatureError } from '../core/core.errors' import { ProcessedSingleAnswerResponse } from '../submission/submission.types' @@ -20,7 +16,7 @@ import { MissingJwtError, VerifyJwtError, } from './spcp.errors' -import { JwtPayload } from './spcp.types' +import { CorppassJwtPayload, SingpassJwtPayload } from './spcp.types' const logger = createLoggerWithLabel(module) const DESTINATION_REGEX = /^\/([\w]+)\/?/ @@ -138,27 +134,35 @@ export const verifyJwtPromise = ( } /** - * Typeguard for JWT payload. + * Typeguard for SingPass JWT payload. * @param payload Payload decrypted from JWT */ -export const isJwtPayload = ( +export const isSingpassJwtPayload = ( payload: unknown, - authType: AuthType.SP | AuthType.CP, -): payload is JwtPayload => { - if (authType === AuthType.SP) { - return ( - !!payload && - typeof payload === 'object' && - typeof (payload as Record).userName === 'string' - ) - } else { - return ( - !!payload && - typeof payload === 'object' && - typeof (payload as Record).userName === 'string' && - typeof (payload as Record).userInfo === 'string' - ) - } +): payload is SingpassJwtPayload => { + return ( + typeof payload === 'object' && + !!payload && + hasProp(payload, 'userName') && + typeof payload.userName === 'string' + ) +} + +/** + * Typeguard for Corppass JWT payload. + * @param payload Payload decrypted from JWT + */ +export const isCorppassJwtPayload = ( + payload: unknown, +): payload is CorppassJwtPayload => { + return ( + typeof payload === 'object' && + !!payload && + hasProp(payload, 'userName') && + typeof payload.userName === 'string' && + hasProp(payload, 'userInfo') && + typeof payload.userInfo === 'string' + ) } /** diff --git a/src/app/modules/submission/email-submission/email-submission.controller.ts b/src/app/modules/submission/email-submission/email-submission.controller.ts index 248da53408..bc751f0c92 100644 --- a/src/app/modules/submission/email-submission/email-submission.controller.ts +++ b/src/app/modules/submission/email-submission/email-submission.controller.ts @@ -126,64 +126,92 @@ export const handleEmailSubmission: RequestHandler< // Handle SingPass, CorpPass and MyInfo authentication and validation const { authType } = form - if (authType === AuthType.SP || authType === AuthType.CP) { - // Verify NRIC and/or UEN - const jwtPayloadResult = await SpcpFactory.extractJwt( - req.cookies, - authType, - ).asyncAndThen((jwt) => SpcpFactory.extractJwtPayload(jwt, authType)) - if (jwtPayloadResult.isErr()) { - logger.error({ - message: 'Failed to verify JWT with auth client', - meta: logMeta, - error: jwtPayloadResult.error, - }) - const { errorMessage, statusCode } = mapRouteError(jwtPayloadResult.error) - return res - .status(statusCode) - .json({ message: errorMessage, spcpSubmissionFailure: true }) - } - const { userName: uinFin, userInfo } = jwtPayloadResult.value - - // Append SingPass/CorpPass info to responses - if (authType === AuthType.SP) { - parsedResponses.push(...createSingpassParsedResponses(uinFin)) - } else if (authType === AuthType.CP) { - // TODO (#317): remove usage of non-null assertion with better typing of JWT payload - parsedResponses.push(...createCorppassParsedResponses(uinFin, userInfo!)) + switch (authType) { + case AuthType.SP: { + // Verify NRIC + const jwtPayloadResult = await SpcpFactory.extractJwt( + req.cookies, + authType, + ).asyncAndThen((jwt) => SpcpFactory.extractSingpassJwtPayload(jwt)) + if (jwtPayloadResult.isErr()) { + logger.error({ + message: 'Failed to verify Singpass JWT with auth client', + meta: logMeta, + error: jwtPayloadResult.error, + }) + const { errorMessage, statusCode } = mapRouteError( + jwtPayloadResult.error, + ) + return res + .status(statusCode) + .json({ message: errorMessage, spcpSubmissionFailure: true }) + } + parsedResponses.push( + ...createSingpassParsedResponses(jwtPayloadResult.value.userName), + ) + break } - } else if (authType === AuthType.MyInfo) { - const uinFinResult = MyInfoUtil.extractMyInfoCookie(req.cookies) - .andThen(MyInfoUtil.extractAccessTokenFromCookie) - .andThen((accessToken) => MyInfoFactory.extractUinFin(accessToken)) - if (uinFinResult.isErr()) { - const { errorMessage, statusCode } = mapRouteError(uinFinResult.error) - return res - .status(statusCode) - .json({ message: errorMessage, spcpSubmissionFailure: true }) + case AuthType.CP: { + // Verify NRIC and UEN + const jwtPayloadResult = await SpcpFactory.extractJwt( + req.cookies, + authType, + ).asyncAndThen((jwt) => SpcpFactory.extractCorppassJwtPayload(jwt)) + if (jwtPayloadResult.isErr()) { + logger.error({ + message: 'Failed to verify Corppass JWT with auth client', + meta: logMeta, + error: jwtPayloadResult.error, + }) + const { errorMessage, statusCode } = mapRouteError( + jwtPayloadResult.error, + ) + return res + .status(statusCode) + .json({ message: errorMessage, spcpSubmissionFailure: true }) + } + parsedResponses.push( + ...createCorppassParsedResponses( + jwtPayloadResult.value.userName, + jwtPayloadResult.value.userInfo, + ), + ) + break } - const uinFin = uinFinResult.value - const verifyMyInfoResult = await MyInfoFactory.fetchMyInfoHashes( - uinFin, - formId, - ).andThen((hashes) => - MyInfoFactory.checkMyInfoHashes(parsedResponses, hashes), - ) - if (verifyMyInfoResult.isErr()) { - logger.error({ - message: 'Error verifying MyInfo hashes', - meta: logMeta, - error: verifyMyInfoResult.error, - }) - const { errorMessage, statusCode } = mapRouteError( - verifyMyInfoResult.error, + case AuthType.MyInfo: { + const uinFinResult = MyInfoUtil.extractMyInfoCookie(req.cookies) + .andThen(MyInfoUtil.extractAccessTokenFromCookie) + .andThen((accessToken) => MyInfoFactory.extractUinFin(accessToken)) + if (uinFinResult.isErr()) { + const { errorMessage, statusCode } = mapRouteError(uinFinResult.error) + return res + .status(statusCode) + .json({ message: errorMessage, spcpSubmissionFailure: true }) + } + const uinFin = uinFinResult.value + const verifyMyInfoResult = await MyInfoFactory.fetchMyInfoHashes( + uinFin, + formId, + ).andThen((hashes) => + MyInfoFactory.checkMyInfoHashes(parsedResponses, hashes), ) - return res - .status(statusCode) - .json({ message: errorMessage, spcpSubmissionFailure: true }) + if (verifyMyInfoResult.isErr()) { + logger.error({ + message: 'Error verifying MyInfo hashes', + meta: logMeta, + error: verifyMyInfoResult.error, + }) + const { errorMessage, statusCode } = mapRouteError( + verifyMyInfoResult.error, + ) + return res + .status(statusCode) + .json({ message: errorMessage, spcpSubmissionFailure: true }) + } + hashedFields = verifyMyInfoResult.value + parsedResponses.push(...createSingpassParsedResponses(uinFin)) + break } - hashedFields = verifyMyInfoResult.value - parsedResponses.push(...createSingpassParsedResponses(uinFin)) } // Create data for response email as well as email confirmation diff --git a/src/app/modules/submission/encrypt-submission/encrypt-submission.controller.ts b/src/app/modules/submission/encrypt-submission/encrypt-submission.controller.ts index c640efbbb0..75fe9dc5e1 100644 --- a/src/app/modules/submission/encrypt-submission/encrypt-submission.controller.ts +++ b/src/app/modules/submission/encrypt-submission/encrypt-submission.controller.ts @@ -9,8 +9,6 @@ import { createLoggerWithLabel } from '../../../../config/logger' import { AuthType, EncryptedSubmissionDto, - ResWithHashedFields, - ResWithUinFin, SubmissionMetadataList, } from '../../../../types' import { EncryptSubmissionDto, ErrorDto } from '../../../../types/api' @@ -19,12 +17,13 @@ import { CaptchaFactory } from '../../../services/captcha/captcha.factory' import { checkIsEncryptedEncoding } from '../../../utils/encryption' import { createReqMeta, getRequestIp } from '../../../utils/request' import { getFormAfterPermissionChecks } from '../../auth/auth.service' -import { MissingFeatureError } from '../../core/core.errors' +import { + MalformedParametersError, + MissingFeatureError, +} from '../../core/core.errors' import { PermissionLevel } from '../../form/admin-form/admin-form.types' import * as FormService from '../../form/form.service' import { isFormEncryptMode } from '../../form/form.utils' -import { MyInfoFactory } from '../../myinfo/myinfo.factory' -import { mapVerifyMyInfoError } from '../../myinfo/myinfo.util' import { SpcpFactory } from '../../spcp/spcp.factory' import { getPopulatedUserById } from '../../user/user.service' import { VerifiedContentFactory } from '../../verified-content/verified-content.factory' @@ -168,62 +167,65 @@ export const handleEncryptedSubmission: RequestHandler = async (req, res) => { // Checks if user is SPCP-authenticated before allowing submission const { authType } = form - if (authType === AuthType.SP || authType === AuthType.CP) { - const spcpResult = await SpcpFactory.extractJwt( - req.cookies, - authType, - ).asyncAndThen((jwt) => SpcpFactory.extractJwtPayload(jwt, authType)) - if (spcpResult.isErr()) { - const { statusCode, errorMessage } = mapRouteError(spcpResult.error) + switch (authType) { + case AuthType.MyInfo: { logger.error({ - message: 'Failed to verify JWT with auth client', + message: + 'Storage mode form is not allowed to have MyInfo authorisation', meta: logMeta, - error: spcpResult.error, - }) - return res.status(statusCode).json({ - message: errorMessage, - spcpSubmissionFailure: true, }) + const { errorMessage, statusCode } = mapRouteError( + new MalformedParametersError( + 'Storage mode form is not allowed to have MyInfo authType', + ), + ) + return res.status(statusCode).json({ message: errorMessage }) } - res.locals.uinFin = spcpResult.value.userName - res.locals.userInfo = spcpResult.value.userInfo - } - - // validating that submitted MyInfo field values match the values - // originally retrieved from MyInfo. - // TODO(frankchn): Roll into a single service call as part of internal refactoring - const uinFin = (res as ResWithUinFin).locals.uinFin - const requestedAttributes = form.getUniqueMyInfoAttrs() - if (authType === AuthType.SP && requestedAttributes.length > 0) { - if (!uinFin) { - return res.status(StatusCodes.UNAUTHORIZED).send({ - message: 'Please log in to SingPass and try again.', - spcpSubmissionFailure: true, - }) + case AuthType.SP: { + const jwtPayloadResult = await SpcpFactory.extractJwt( + req.cookies, + authType, + ).asyncAndThen((jwt) => SpcpFactory.extractSingpassJwtPayload(jwt)) + if (jwtPayloadResult.isErr()) { + const { statusCode, errorMessage } = mapRouteError( + jwtPayloadResult.error, + ) + logger.error({ + message: 'Failed to verify Singpass JWT with auth client', + meta: logMeta, + error: jwtPayloadResult.error, + }) + return res.status(statusCode).json({ + message: errorMessage, + spcpSubmissionFailure: true, + }) + } + res.locals.uinFin = jwtPayloadResult.value.userName + break } - const myinfoResult = await MyInfoFactory.fetchMyInfoHashes( - uinFin, - formId, - ).andThen((hashes) => - MyInfoFactory.checkMyInfoHashes(processedResponses, hashes), - ) - if (myinfoResult.isErr()) { - logger.error({ - message: 'Error verifying MyInfo hashes', - meta: logMeta, - error: myinfoResult.error, - }) - const { statusCode, errorMessage } = mapVerifyMyInfoError( - myinfoResult.error, - ) - return res.status(statusCode).send({ - message: errorMessage, - spcpSubmissionFailure: true, - }) + case AuthType.CP: { + const jwtPayloadResult = await SpcpFactory.extractJwt( + req.cookies, + authType, + ).asyncAndThen((jwt) => SpcpFactory.extractCorppassJwtPayload(jwt)) + if (jwtPayloadResult.isErr()) { + const { statusCode, errorMessage } = mapRouteError( + jwtPayloadResult.error, + ) + logger.error({ + message: 'Failed to verify Corppass JWT with auth client', + meta: logMeta, + error: jwtPayloadResult.error, + }) + return res.status(statusCode).json({ + message: errorMessage, + spcpSubmissionFailure: true, + }) + } + res.locals.uinFin = jwtPayloadResult.value.userName + res.locals.userInfo = jwtPayloadResult.value.userInfo + break } - // eslint-disable-next-line @typescript-eslint/no-extra-semi - ;(res as ResWithHashedFields).locals.hashedFields = - myinfoResult.value } // Encrypt Verified SPCP Fields diff --git a/src/app/utils/encryption.ts b/src/app/utils/encryption.ts index 080d469fab..a8f22b1723 100644 --- a/src/app/utils/encryption.ts +++ b/src/app/utils/encryption.ts @@ -6,11 +6,6 @@ import { InvalidEncodingError } from '../modules/submission/submission.errors' export const checkIsEncryptedEncoding = ( encryptedStr: string, ): Result => { - // TODO (#42): Remove this type check once whole backend is in TypeScript. - if (typeof encryptedStr !== 'string') { - return err(new InvalidEncodingError('encryptedStr is not of type `string`')) - } - const [submissionPublicKey, nonceEncrypted] = encryptedStr.split(';') if (!nonceEncrypted) { diff --git a/src/app/utils/limit-rate.ts b/src/app/utils/limit-rate.ts index 073b45a863..9a97358882 100644 --- a/src/app/utils/limit-rate.ts +++ b/src/app/utils/limit-rate.ts @@ -12,9 +12,8 @@ import { createReqMeta } from './request' const logger = createLoggerWithLabel(module) /** - * Returns a middleware which logs a message if the rate of requests + * Returns a middleware which logs a message and returns 429 if the rate of requests * to an API endpoint exceeds a given rate. - * TODO (private #49): update this documentation. * @param options Custom options to be passed to RateLimit * @return Rate-limiting middleware */ diff --git a/src/config/config.ts b/src/config/config.ts index e20ad2654e..79f63c4581 100644 --- a/src/config/config.ts +++ b/src/config/config.ts @@ -84,7 +84,7 @@ const awsConfig: AwsConfig = { s3, } -let dbUri +let dbUri: string | undefined if (isDev) { if (basicVars.core.nodeEnv === Environment.Dev && prodOnlyVars.dbHost) { dbUri = prodOnlyVars.dbHost @@ -98,8 +98,7 @@ if (isDev) { } const dbConfig: DbConfig = { - // TODO (#317): remove usage of non-null assertion - uri: dbUri!, + uri: dbUri ?? '', options: { user: '', pass: '', diff --git a/src/config/logger.ts b/src/config/logger.ts index b3a793e822..91fa4c319b 100644 --- a/src/config/logger.ts +++ b/src/config/logger.ts @@ -1,5 +1,4 @@ import hasAnsi from 'has-ansi' -import { isEmpty } from 'lodash' import omit from 'lodash/omit' import logform from 'logform' import path from 'path' @@ -187,55 +186,27 @@ const getModuleLabel = (callingModule: NodeModule) => { /** * Overrides the given winston logger with a new signature, so as to enforce a * log format. - * TODO(#42): Remove try catch blocks when application is 100% TypeScript. * @param logger the logger to override */ const createCustomLogger = (logger: Logger) => { return { info: (params: Omit) => { - try { - const { message, meta } = params - // Not the expected shape, throw to catch block. - if (!message || isEmpty(meta)) { - //eslint-disable-next-line - throw new Error('Wrong shape') - } - return logger.info(message, { meta }) - } catch { - return logger.info(params) - } + const { message, meta } = params + return logger.info(message, { meta }) }, warn: (params: CustomLoggerParams) => { - try { - const { message, meta, error } = params - // Not the expected shape, throw to catch block. - if (!message || isEmpty(meta)) { - //eslint-disable-next-line - throw new Error('Wrong shape') - } - if (error) { - return logger.warn(message, { meta }, error) - } - return logger.warn(message, { meta }) - } catch { - return logger.warn(params) + const { message, meta, error } = params + if (error) { + return logger.warn(message, { meta }, error) } + return logger.warn(message, { meta }) }, error: (params: CustomLoggerParams) => { - try { - const { message, meta, error } = params - // Not the expected shape, throw to catch block. - if (!message || isEmpty(meta)) { - //eslint-disable-next-line - throw new Error('Wrong shape') - } - if (error) { - return logger.error(message, { meta }, error) - } - return logger.error(message, { meta }) - } catch { - return logger.error(params) + const { message, meta, error } = params + if (error) { + return logger.error(message, { meta }, error) } + return logger.error(message, { meta }) }, } } diff --git a/src/loaders/mongoose.ts b/src/loaders/mongoose.ts index 6c135111af..0ac7dbdde4 100644 --- a/src/loaders/mongoose.ts +++ b/src/loaders/mongoose.ts @@ -43,8 +43,7 @@ export default async (): Promise => { // Actually connect to the database function connect() { - // TODO (#317): remove usage of non-null assertion - return mongoose.connect(config.db.uri!, config.db.options) + return mongoose.connect(config.db.uri, config.db.options) } // Only required for initial connection errors, reconnect on error. diff --git a/src/shared/util/logic.ts b/src/shared/util/logic.ts index 2685a1501b..96b182111a 100644 --- a/src/shared/util/logic.ts +++ b/src/shared/util/logic.ts @@ -128,13 +128,15 @@ export const getLogicUnitPreventingSubmit = ( form: IFormDocument, visibleFieldIds?: FieldIdSet, ): IPreventSubmitLogicSchema | undefined => { - if (!visibleFieldIds) { - visibleFieldIds = getVisibleFieldIds(submission, form) - } + const definedVisibleFieldIds = + visibleFieldIds ?? getVisibleFieldIds(submission, form) const preventSubmitConditions = getPreventSubmitConditions(form) return preventSubmitConditions.find((logicUnit) => - // TODO (#317): remove usage of non-null assertion - isLogicUnitSatisfied(submission, logicUnit.conditions, visibleFieldIds!), + isLogicUnitSatisfied( + submission, + logicUnit.conditions, + definedVisibleFieldIds, + ), ) } diff --git a/src/types/express.locals.ts b/src/types/email_mode_data.ts similarity index 59% rename from src/types/express.locals.ts rename to src/types/email_mode_data.ts index cdd69b5b92..9f3d951c9f 100644 --- a/src/types/express.locals.ts +++ b/src/types/email_mode_data.ts @@ -1,30 +1,4 @@ -// TODO (#42): remove these types when migrating away from middleware pattern -import { LeanDocument } from 'mongoose' - -import { ProcessedFieldResponse } from '../app/modules/submission/submission.types' - import { BasicField } from './field' -import { IPopulatedForm } from './form' - -export type WithForm = T & { - form: IPopulatedForm -} - -export type WithJsonForm = T & { - form: LeanDocument -} - -export type WithParsedResponses = T & { - parsedResponses: ProcessedFieldResponse[] -} - -export type ResWithUinFin = T & { - uinFin?: string -} - -export type ResWithHashedFields = T & { - locals: { hashedFields?: Set } -} export type EmailRespondentConfirmationField = { question: string diff --git a/src/types/index.ts b/src/types/index.ts index 6874ade633..4cf095bc06 100644 --- a/src/types/index.ts +++ b/src/types/index.ts @@ -18,4 +18,4 @@ export * from './admin_verification' export * from './config' export * from './spcp' export * from './routing' -export * from './express.locals' +export * from './email_mode_data'