diff --git a/package-lock.json b/package-lock.json index 6ac8a54707..f2b03dc5c7 100644 --- a/package-lock.json +++ b/package-lock.json @@ -23921,6 +23921,12 @@ "utf8-byte-length": "^1.0.1" } }, + "ts-essentials": { + "version": "7.0.1", + "resolved": "https://registry.npmjs.org/ts-essentials/-/ts-essentials-7.0.1.tgz", + "integrity": "sha512-8lwh3QJtIc1UWhkQtr9XuksXu3O0YQdEE5g79guDfhCaU1FWTDIEDZ1ZSx4HTHUmlJZ8L812j3BZQ4a0aOUkSA==", + "dev": true + }, "ts-jest": { "version": "26.5.4", "resolved": "https://registry.npmjs.org/ts-jest/-/ts-jest-26.5.4.tgz", diff --git a/package.json b/package.json index d60b176cdd..79ae4eb9c8 100644 --- a/package.json +++ b/package.json @@ -247,6 +247,7 @@ "supertest-session": "^4.1.0", "terser-webpack-plugin": "^1.2.3", "testcafe": "^1.14.0", + "ts-essentials": "^7.0.1", "ts-jest": "^26.5.4", "ts-loader": "^7.0.5", "ts-mock-imports": "^1.3.3", diff --git a/src/app/modules/form/__tests__/form.service.spec.ts b/src/app/modules/form/__tests__/form.service.spec.ts index f4668bf567..20be4ce147 100644 --- a/src/app/modules/form/__tests__/form.service.spec.ts +++ b/src/app/modules/form/__tests__/form.service.spec.ts @@ -242,10 +242,10 @@ describe('FormService', () => { ) // Assert - expect(actual._unsafeUnwrap()).toEqual(true) + expect(actual._unsafeUnwrap()).toEqual(form) }) - it('should let requests through when form has not reached submission limit', async () => { + it('should return the form when the submission limit is not reached', async () => { // Arrange const formParams = merge({}, MOCK_ENCRYPTED_FORM_PARAMS, { status: Status.Public, @@ -268,11 +268,11 @@ describe('FormService', () => { // Act const actual = await FormService.checkFormSubmissionLimitAndDeactivateForm( - form, + form as IPopulatedForm, ) // Assert - expect(actual._unsafeUnwrap()).toEqual(true) + expect(actual._unsafeUnwrap()).toEqual(validForm) }) it('should not let requests through and deactivate form when form has reached submission limit', async () => { diff --git a/src/app/modules/form/form.service.ts b/src/app/modules/form/form.service.ts index f4ddf91ed1..5ff5e62333 100644 --- a/src/app/modules/form/form.service.ts +++ b/src/app/modules/form/form.service.ts @@ -3,6 +3,7 @@ import { err, errAsync, ok, okAsync, Result, ResultAsync } from 'neverthrow' import { createLoggerWithLabel } from '../../../config/logger' import { + AuthType, IEmailFormModel, IEncryptedFormModel, IFormSchema, @@ -15,6 +16,7 @@ import getFormModel, { getEncryptedFormModel, } from '../../models/form.server.model' import getSubmissionModel from '../../models/submission.server.model' +import { IntranetFactory } from '../../services/intranet/intranet.factory' import { getMongoErrorMessage, transformMongoError, @@ -37,10 +39,42 @@ const EmailFormModel = getEmailFormModel(mongoose) const EncryptedFormModel = getEncryptedFormModel(mongoose) const SubmissionModel = getSubmissionModel(mongoose) -export const deactivateForm = async ( +/** + * Deactivates a given form by its id + * @param formId the id of the form to deactivate + * @returns ok(true) if the form has been deactivated successfully + * @returns err(PossibleDatabaseError) if an error occurred while trying to deactivate the form + * @returns err(FormNotFoundError) if there is no form with the given formId + */ +export const deactivateForm = ( formId: string, -): Promise => { - return FormModel.deactivateById(formId) +): ResultAsync => { + return ResultAsync.fromPromise(FormModel.deactivateById(formId), (error) => { + logger.error({ + message: 'Error deactivating form by id', + meta: { + action: 'deactivateForm', + form: formId, + }, + error, + }) + + return transformMongoError(error) + }).andThen((deactivatedForm) => { + if (!deactivatedForm) { + logger.error({ + message: + 'Attempted to deactivate form that cannot be found in the database', + meta: { + action: 'deactivateForm', + form: formId, + }, + }) + return errAsync(new FormNotFoundError()) + } + // Successfully deactivated. + return okAsync(deactivatedForm) + }) } /** @@ -118,9 +152,9 @@ export const retrieveFormById = ( * Method to ensure given form is available to the public. * @param form the form to check * @returns ok(true) if form is public + * @returns err(ApplicationError) if form has an invalid state * @returns err(FormDeletedError) if form has been deleted * @returns err(PrivateFormError) if form is private, the message will be the form inactive message - * @returns err(ApplicationError) if form has an invalid state */ export const isFormPublic = ( form: IPopulatedForm, @@ -128,7 +162,6 @@ export const isFormPublic = ( if (!form.status) { return err(new ApplicationError()) } - switch (form.status) { case Status.Public: return ok(true) @@ -142,20 +175,30 @@ export const isFormPublic = ( /** * Method to check whether a form has reached submission limits, and deactivate the form if necessary * @param form the form to check - * @returns ok(true) if submission is allowed because the form has not reached limits - * @returns ok(false) if submission is not allowed because the form has reached limits + * @returns ok(form) if submission is allowed because the form has not reached limits + * @returns err(PossibleDatabaseError) if an error occurred while querying the database for the specified form + * @returns err(FormNotFoundError) if the form has exceeded the submission limits but could not be found and deactivated + * @returns err(PrivateFormError) if the count of the form has been exceeded and the form has been deactivated */ export const checkFormSubmissionLimitAndDeactivateForm = ( form: IPopulatedForm, -): ResultAsync => { +): ResultAsync< + IPopulatedForm, + PossibleDatabaseError | PrivateFormError | FormNotFoundError +> => { const logMeta = { action: 'checkFormSubmissionLimitAndDeactivateForm', formId: form._id, } - if (form.submissionLimit === null) return okAsync(true) + const { submissionLimit } = form + const formId = String(form._id) + // Not using falsey check as submissionLimit === 0 can result in incorrectly + // returning form without any actions. + if (submissionLimit === null) return okAsync(form) + return ResultAsync.fromPromise( SubmissionModel.countDocuments({ - form: form._id, + form: formId, }).exec(), (error) => { logger.error({ @@ -165,21 +208,18 @@ export const checkFormSubmissionLimitAndDeactivateForm = ( }) return transformMongoError(error) }, - ).andThen((count) => { - if (count < form.submissionLimit) return okAsync(true) + ).andThen((currentCount) => { + // Limit has not been hit yet, passthrough. + if (currentCount < submissionLimit) return okAsync(form) + logger.info({ message: 'Form reached maximum submission count, deactivating.', meta: logMeta, }) - return ResultAsync.fromPromise(deactivateForm(form._id), (error) => { - logger.error({ - message: 'Error while deactivating form', - meta: logMeta, - error, - }) - return transformMongoError(error) - }).andThen(() => - // Always return err because submission limit was exceeded + + // Map success case back into error to display to client as form has been + // deactivated. + return deactivateForm(formId).andThen(() => errAsync( new PrivateFormError( 'Submission made after form submission limit was reached', @@ -200,3 +240,39 @@ export const getFormModelByResponseMode = ( return EncryptedFormModel } } + +/** + * Checks if a form is accessed from within intranet and sets the property accordingly + * @param ip The ip of the request + * @param publicFormView The form to check + * @returns ok(PublicFormView) if the form is accessed from the internet + * @returns err(ApplicationError) if an error occured while checking if the ip of the request is from the intranet + */ +export const checkIsIntranetFormAccess = ( + ip: string, + form: IPopulatedForm, +): boolean => { + return ( + IntranetFactory.isIntranetIp(ip) + .andThen((isIntranetUser) => { + // Warn if form is being accessed from within intranet + // and the form has authentication set + if ( + isIntranetUser && + [AuthType.SP, AuthType.CP, AuthType.MyInfo].includes(form.authType) + ) { + logger.warn({ + message: + 'Attempting to access SingPass, CorpPass or MyInfo form from intranet', + meta: { + action: 'checkIsIntranetFormAccess', + formId: form._id, + }, + }) + } + return ok(isIntranetUser) + }) + // This is required becausing the factory can throw missing feature error on initialization + .unwrapOr(false) + ) +} diff --git a/src/app/modules/form/public-form/__tests__/public-form.controller.spec.ts b/src/app/modules/form/public-form/__tests__/public-form.controller.spec.ts index cc3cf172fc..e18cce6cdd 100644 --- a/src/app/modules/form/public-form/__tests__/public-form.controller.spec.ts +++ b/src/app/modules/form/public-form/__tests__/public-form.controller.spec.ts @@ -1,3 +1,4 @@ +import { IPersonResponse } from '@opengovsg/myinfo-gov-client' import { ObjectId } from 'bson-ext' import { merge } from 'lodash' import mongoose from 'mongoose' @@ -6,11 +7,35 @@ import querystring from 'querystring' import { mocked } from 'ts-jest/utils' import getFormFeedbackModel from 'src/app/models/form_feedback.server.model' -import { DatabaseError } from 'src/app/modules/core/core.errors' -import { IPopulatedForm } from 'src/types' +import { + DatabaseError, + MissingFeatureError, +} from 'src/app/modules/core/core.errors' +import { MyInfoData } from 'src/app/modules/myinfo/myinfo.adapter' +import { + MyInfoAuthTypeError, + MyInfoCookieAccessError, + MyInfoMissingAccessTokenError, + MyInfoNoESrvcIdError, +} from 'src/app/modules/myinfo/myinfo.errors' +import { MyInfoCookieState } from 'src/app/modules/myinfo/myinfo.types' +import { JwtPayload } from 'src/app/modules/spcp/spcp.types' +import { FeatureNames } from 'src/config/feature-manager/types' +import { + AuthType, + IPopulatedForm, + IPopulatedUser, + MyInfoAttribute, + PublicForm, +} from 'src/types' import expressHandler from 'tests/unit/backend/helpers/jest-express' +import * as AuthService from '../../../auth/auth.service' +import { MyInfoCookieStateError } from '../../../myinfo/myinfo.errors' +import { MyInfoFactory } from '../../../myinfo/myinfo.factory' +import { MissingJwtError } from '../../../spcp/spcp.errors' +import { SpcpFactory } from '../../../spcp/spcp.factory' import { FormDeletedError, FormNotFoundError, @@ -21,10 +46,17 @@ import * as PublicFormController from '../public-form.controller' import * as PublicFormService from '../public-form.service' import { Metatags } from '../public-form.types' -jest.mock('../../form.service') jest.mock('../public-form.service') +jest.mock('../../form.service') +jest.mock('../../../auth/auth.service') +jest.mock('../../../spcp/spcp.factory') +jest.mock('../../../myinfo/myinfo.factory') + const MockFormService = mocked(FormService) const MockPublicFormService = mocked(PublicFormService) +const MockAuthService = mocked(AuthService) +const MockSpcpFactory = mocked(SpcpFactory, true) +const MockMyInfoFactory = mocked(MyInfoFactory, true) const FormFeedbackModel = getFormFeedbackModel(mongoose) @@ -185,7 +217,7 @@ describe('public-form.controller', () => { expect(mockRes.json).toHaveBeenCalledWith({ message: 'Gone' }) }) - it('should return 500 when databse errors occur', async () => { + it('should return 500 when database errors occur', async () => { // Arrange const mockRes = expressHandler.mockResponse() const mockErrorString = 'Form feedback could not be created' @@ -389,4 +421,718 @@ describe('public-form.controller', () => { expect(mockRes.redirect).toHaveBeenCalledWith(expectedRedirectPath) }) }) + + describe('handleGetPublicForm', () => { + const MOCK_FORM_ID = new ObjectId().toHexString() + const MOCK_USER_ID = new ObjectId().toHexString() + const MOCK_USER = { + _id: MOCK_USER_ID, + email: 'randomrandomtest@example.com', + } as IPopulatedUser + + const MOCK_SCRUBBED_FORM = ({ + _id: MOCK_FORM_ID, + title: 'mock title', + admin: { _id: MOCK_USER_ID }, + } as unknown) as PublicForm + + const BASE_FORM = { + admin: MOCK_USER, + _id: MOCK_FORM_ID, + title: MOCK_SCRUBBED_FORM.title, + getUniqueMyInfoAttrs: jest.fn().mockReturnValue([MyInfoAttribute.Name]), + getPublicView: jest.fn().mockReturnValue(MOCK_SCRUBBED_FORM), + } + + const MOCK_REQ = expressHandler.mockRequest({ + params: { + formId: MOCK_FORM_ID, + }, + }) + + const MOCK_MYINFO_COOKIE = { + accessToken: 'cookie', + usedCount: 0, + state: MyInfoCookieState.Success, + } + + let mockReqWithCookies: ReturnType + + beforeEach(() => { + mockReqWithCookies = expressHandler.mockRequest({ + params: { + formId: MOCK_FORM_ID, + }, + others: { cookies: { MyInfoCookie: MOCK_MYINFO_COOKIE } }, + }) + }) + + // Success + describe('valid form id', () => { + const MOCK_JWT_PAYLOAD: JwtPayload = { + userName: 'mock', + rememberMe: false, + } + + beforeAll(() => { + MockFormService.checkIsIntranetFormAccess.mockReturnValue(false) + }) + + it('should return 200 when there is no AuthType on the request', async () => { + // Arrange + const MOCK_NIL_AUTH_FORM = ({ + ...BASE_FORM, + authType: AuthType.NIL, + } as unknown) as IPopulatedForm + const mockRes = expressHandler.mockResponse() + + MockAuthService.getFormIfPublic.mockReturnValueOnce( + okAsync(MOCK_NIL_AUTH_FORM), + ) + MockFormService.checkFormSubmissionLimitAndDeactivateForm.mockReturnValueOnce( + okAsync(MOCK_NIL_AUTH_FORM), + ) + + // Act + await PublicFormController.handleGetPublicForm( + MOCK_REQ, + mockRes, + jest.fn(), + ) + + // Assert + expect(mockRes.json).toHaveBeenCalledWith({ + form: MOCK_NIL_AUTH_FORM.getPublicView(), + isIntranetUser: false, + }) + }) + + it('should return 200 when client authenticates using SP', async () => { + // Arrange + const MOCK_SPCP_SESSION = { userName: MOCK_JWT_PAYLOAD.userName } + const MOCK_SP_AUTH_FORM = ({ + ...BASE_FORM, + authType: AuthType.SP, + } as unknown) as IPopulatedForm + const mockRes = expressHandler.mockResponse() + + MockAuthService.getFormIfPublic.mockReturnValueOnce( + okAsync(MOCK_SP_AUTH_FORM), + ) + MockFormService.checkFormSubmissionLimitAndDeactivateForm.mockReturnValueOnce( + okAsync(MOCK_SP_AUTH_FORM), + ) + MockSpcpFactory.extractJwtPayloadFromRequest.mockReturnValueOnce( + okAsync(MOCK_SPCP_SESSION), + ) + + // Act + await PublicFormController.handleGetPublicForm( + MOCK_REQ, + mockRes, + jest.fn(), + ) + + // Assert + expect(mockRes.json).toHaveBeenCalledWith({ + form: MOCK_SP_AUTH_FORM.getPublicView(), + isIntranetUser: false, + spcpSession: MOCK_SPCP_SESSION, + }) + }) + + it('should return 200 when client authenticates using CP', async () => { + // Arrange + const MOCK_SPCP_SESSION = { userName: MOCK_JWT_PAYLOAD.userName } + const MOCK_CP_AUTH_FORM = ({ + ...BASE_FORM, + authType: AuthType.CP, + } as unknown) as IPopulatedForm + const mockRes = expressHandler.mockResponse() + + MockAuthService.getFormIfPublic.mockReturnValueOnce( + okAsync(MOCK_CP_AUTH_FORM), + ) + MockFormService.checkFormSubmissionLimitAndDeactivateForm.mockReturnValueOnce( + okAsync(MOCK_CP_AUTH_FORM), + ) + MockSpcpFactory.extractJwtPayloadFromRequest.mockReturnValueOnce( + okAsync(MOCK_SPCP_SESSION), + ) + // Act + await PublicFormController.handleGetPublicForm( + MOCK_REQ, + mockRes, + jest.fn(), + ) + + // Assert + expect(mockRes.json).toHaveBeenCalledWith({ + form: MOCK_CP_AUTH_FORM.getPublicView(), + isIntranetUser: false, + spcpSession: MOCK_SPCP_SESSION, + }) + }) + + it('should return 200 when client authenticates using MyInfo', async () => { + // Arrange + const MOCK_MYINFO_AUTH_FORM = ({ + ...BASE_FORM, + esrvcId: 'thing', + authType: AuthType.MyInfo, + toJSON: jest.fn().mockReturnValue(BASE_FORM), + } as unknown) as IPopulatedForm + const MOCK_MYINFO_DATA = new MyInfoData({ + uinFin: 'i am a fish', + } as IPersonResponse) + const MOCK_SPCP_SESSION = { userName: MOCK_MYINFO_DATA.getUinFin() } + const mockRes = expressHandler.mockResponse({ + clearCookie: jest.fn().mockReturnThis(), + cookie: jest.fn().mockReturnThis(), + }) + MockAuthService.getFormIfPublic.mockReturnValueOnce( + okAsync(MOCK_MYINFO_AUTH_FORM), + ) + MockFormService.checkFormSubmissionLimitAndDeactivateForm.mockReturnValueOnce( + okAsync(MOCK_MYINFO_AUTH_FORM), + ) + MockMyInfoFactory.getMyInfoDataForForm.mockReturnValueOnce( + okAsync(MOCK_MYINFO_DATA), + ) + MockMyInfoFactory.prefillAndSaveMyInfoFields.mockReturnValueOnce( + okAsync([]), + ) + + // Act + await PublicFormController.handleGetPublicForm( + mockReqWithCookies, + mockRes, + jest.fn(), + ) + + // Assert + expect(mockRes.clearCookie).not.toHaveBeenCalled() + expect(mockRes.cookie).toHaveBeenCalled() + expect(mockRes.json).toHaveBeenCalledWith({ + form: { ...MOCK_MYINFO_AUTH_FORM.getPublicView(), form_fields: [] }, + spcpSession: MOCK_SPCP_SESSION, + isIntranetUser: false, + }) + }) + }) + + // Errors + describe('errors in myInfo', () => { + const MOCK_MYINFO_FORM = ({ + ...BASE_FORM, + toJSON: jest.fn().mockReturnThis(), + authType: AuthType.MyInfo, + } as unknown) as IPopulatedForm + + // Setup because this gets invoked at the start of the controller to decide which branch to take + beforeAll(() => { + MockAuthService.getFormIfPublic.mockReturnValue( + okAsync(MOCK_MYINFO_FORM), + ) + + MockFormService.checkIsIntranetFormAccess.mockReturnValueOnce(false) + + MockFormService.checkFormSubmissionLimitAndDeactivateForm.mockReturnValue( + okAsync(MOCK_MYINFO_FORM), + ) + }) + + it('should return 200 but the response should have cookies cleared with myInfoError set to undefined when the request has no cookie', async () => { + // Arrange + // 1. Mock the response and calls + const mockRes = expressHandler.mockResponse({ + clearCookie: jest.fn().mockReturnThis(), + }) + + MockMyInfoFactory.getMyInfoDataForForm.mockReturnValueOnce( + errAsync(new MyInfoMissingAccessTokenError()), + ) + + // Act + await PublicFormController.handleGetPublicForm( + MOCK_REQ, + mockRes, + jest.fn(), + ) + + // Assert + expect(mockRes.clearCookie).toHaveBeenCalled() + expect(mockRes.json).toHaveBeenCalledWith({ + form: MOCK_MYINFO_FORM.getPublicView(), + isIntranetUser: false, + myInfoError: undefined, + }) + }) + + it('should return 200 but the response should have cookies cleared with myInfoError set to undefined when the cookie has been used before', async () => { + // Arrange + // 1. Mock the response and calls + const mockRes = expressHandler.mockResponse({ + clearCookie: jest.fn().mockReturnThis(), + }) + + MockMyInfoFactory.getMyInfoDataForForm.mockReturnValueOnce( + errAsync(new MyInfoCookieAccessError()), + ) + + // Act + await PublicFormController.handleGetPublicForm( + MOCK_REQ, + mockRes, + jest.fn(), + ) + + // Assert + expect(mockRes.clearCookie).toHaveBeenCalled() + expect(mockRes.json).toHaveBeenCalledWith({ + form: MOCK_MYINFO_FORM.getPublicView(), + isIntranetUser: false, + myInfoError: undefined, + }) + }) + + it('should return 200 but the response should have cookies cleared and myInfoError when the cookie cannot be validated', async () => { + // Arrange + // 1. Mock the response and calls + const mockRes = expressHandler.mockResponse({ + clearCookie: jest.fn().mockReturnThis(), + }) + + MockMyInfoFactory.getMyInfoDataForForm.mockReturnValueOnce( + errAsync(new MyInfoCookieStateError()), + ) + + // Act + await PublicFormController.handleGetPublicForm( + mockReqWithCookies, + mockRes, + jest.fn(), + ) + + // Assert + expect(mockRes.clearCookie).toHaveBeenCalled() + expect(mockRes.json).toHaveBeenCalledWith({ + form: MOCK_MYINFO_FORM.getPublicView(), + isIntranetUser: false, + myInfoError: true, + }) + }) + + it('should return 200 but the response should have cookies cleared and myInfoError if the form cannot be validated', async () => { + // Arrange + // 1. Mock the response and calls + const mockRes = expressHandler.mockResponse({ + clearCookie: jest.fn().mockReturnThis(), + }) + + MockMyInfoFactory.getMyInfoDataForForm.mockReturnValueOnce( + errAsync(new MyInfoAuthTypeError()), + ) + + // Act + await PublicFormController.handleGetPublicForm( + mockReqWithCookies, + mockRes, + jest.fn(), + ) + + // Assert + expect(mockRes.clearCookie).toHaveBeenCalled() + expect(mockRes.json).toHaveBeenCalledWith({ + form: MOCK_MYINFO_FORM.getPublicView(), + myInfoError: true, + isIntranetUser: false, + }) + }) + + it('should return 200 but the response should have cookies cleared and myInfoError when the form has no eservcId', async () => { + // Arrange + // 1. Mock the response and calls + const mockRes = expressHandler.mockResponse({ + clearCookie: jest.fn().mockReturnThis(), + }) + + MockMyInfoFactory.getMyInfoDataForForm.mockReturnValueOnce( + errAsync(new MyInfoNoESrvcIdError()), + ) + + // Act + await PublicFormController.handleGetPublicForm( + mockReqWithCookies, + mockRes, + jest.fn(), + ) + + // Assert + expect(mockRes.clearCookie).toHaveBeenCalled() + expect(mockRes.json).toHaveBeenCalledWith({ + form: MOCK_MYINFO_FORM.getPublicView(), + isIntranetUser: false, + myInfoError: true, + }) + }) + + it('should return 200 but the response should have cookies cleared and myInfoError when the form could not be filled', async () => { + // Arrange + // 1. Mock the response and calls + const mockRes = expressHandler.mockResponse({ + clearCookie: jest.fn().mockReturnThis(), + }) + + MockMyInfoFactory.getMyInfoDataForForm.mockReturnValueOnce( + errAsync( + new MissingFeatureError( + 'testing is the missing feature' as FeatureNames, + ), + ), + ) + + // Act + await PublicFormController.handleGetPublicForm( + mockReqWithCookies, + mockRes, + jest.fn(), + ) + + // Assert + expect(mockRes.clearCookie).toHaveBeenCalled() + expect(mockRes.json).toHaveBeenCalledWith({ + form: MOCK_MYINFO_FORM.getPublicView(), + isIntranetUser: false, + myInfoError: true, + }) + }) + + it('should return 200 but the response should have cookies cleared and myInfoError if a database error occurs while saving hashes', async () => { + // Arrange + // 1. Mock the response and calls + const MOCK_MYINFO_DATA = new MyInfoData({ + uinFin: 'i am a fish', + } as IPersonResponse) + const expected = new DatabaseError('fish error') + const mockRes = expressHandler.mockResponse({ + clearCookie: jest.fn().mockReturnThis(), + }) + MockMyInfoFactory.getMyInfoDataForForm.mockReturnValueOnce( + okAsync(MOCK_MYINFO_DATA), + ) + MockMyInfoFactory.prefillAndSaveMyInfoFields.mockReturnValueOnce( + errAsync(expected), + ) + + // Act + await PublicFormController.handleGetPublicForm( + mockReqWithCookies, + mockRes, + jest.fn(), + ) + + // Assert + expect(mockRes.clearCookie).toHaveBeenCalled() + expect(mockRes.json).toHaveBeenCalledWith({ + form: MOCK_MYINFO_FORM.getPublicView(), + isIntranetUser: false, + myInfoError: true, + }) + }) + }) + + describe('errors in spcp', () => { + const MOCK_SPCP_FORM = ({ + ...BASE_FORM, + authType: AuthType.SP, + } as unknown) as IPopulatedForm + it('should return 200 with the form but without a spcpSession when the JWT token could not be found', async () => { + // Arrange + // 1. Mock the response and calls + const mockRes = expressHandler.mockResponse() + + MockAuthService.getFormIfPublic.mockReturnValueOnce( + okAsync(MOCK_SPCP_FORM), + ) + MockFormService.checkFormSubmissionLimitAndDeactivateForm.mockReturnValueOnce( + okAsync(MOCK_SPCP_FORM), + ) + MockSpcpFactory.extractJwtPayloadFromRequest.mockReturnValueOnce( + errAsync(new MissingJwtError()), + ) + + // Act + // 2. GET the endpoint + await PublicFormController.handleGetPublicForm( + MOCK_REQ, + mockRes, + jest.fn(), + ) + + // Assert + // Status should be 200 + // json object should only have form property + expect(mockRes.json).toHaveBeenCalledWith({ + form: MOCK_SPCP_FORM.getPublicView(), + isIntranetUser: false, + }) + }) + }) + + describe('errors in form retrieval', () => { + const MOCK_ERROR_STRING = 'mockingbird' + + it('should return 500 when a database error occurs', async () => { + // Arrange + // 1. Mock the response + const mockRes = expressHandler.mockResponse() + + // 2. Mock the call to retrieve the form + MockAuthService.getFormIfPublic.mockReturnValueOnce( + errAsync(new DatabaseError(MOCK_ERROR_STRING)), + ) + + // Act + await PublicFormController.handleGetPublicForm( + MOCK_REQ, + mockRes, + jest.fn(), + ) + + // Assert + // 1. Check args of mocked services + expect(MockAuthService.getFormIfPublic).toHaveBeenCalledWith( + MOCK_FORM_ID, + ) + // 2. Check that error is correct + expect( + MockFormService.checkFormSubmissionLimitAndDeactivateForm, + ).not.toHaveBeenCalled() + expect(mockRes.status).toHaveBeenCalledWith(500) + expect(mockRes.json).toHaveBeenCalledWith({ + message: MOCK_ERROR_STRING, + }) + }) + + it('should return 404 when the form is not found', async () => { + // Arrange + // 1. Mock the response + const mockRes = expressHandler.mockResponse() + const MOCK_ERROR_STRING = 'Your form was eaten up by a monster' + + // 2. Mock the call to retrieve the form + MockAuthService.getFormIfPublic.mockReturnValueOnce( + errAsync(new FormNotFoundError(MOCK_ERROR_STRING)), + ) + + // Act + await PublicFormController.handleGetPublicForm( + MOCK_REQ, + mockRes, + jest.fn(), + ) + + // Assert + // 1. Check args of mocked services + expect(MockAuthService.getFormIfPublic).toHaveBeenCalledWith( + MOCK_FORM_ID, + ) + // 2. Check that error is correct + expect( + MockFormService.checkFormSubmissionLimitAndDeactivateForm, + ).not.toHaveBeenCalled() + expect(mockRes.status).toHaveBeenCalledWith(404) + expect(mockRes.json).toHaveBeenCalledWith({ + message: MOCK_ERROR_STRING, + }) + }) + + it('should return 404 when the form is private and not accessible by the public', async () => { + // Arrange + // 1. Mock the response + const mockRes = expressHandler.mockResponse() + + // 2. Mock the call to retrieve the form + MockAuthService.getFormIfPublic.mockReturnValueOnce( + errAsync(new PrivateFormError(MOCK_ERROR_STRING, 'private form')), + ) + + // Act + await PublicFormController.handleGetPublicForm( + MOCK_REQ, + mockRes, + jest.fn(), + ) + + // Assert + // 1. Check args of mocked services + expect(MockAuthService.getFormIfPublic).toHaveBeenCalledWith( + MOCK_FORM_ID, + ) + // 2. Check that error is correct + expect( + MockFormService.checkFormSubmissionLimitAndDeactivateForm, + ).not.toHaveBeenCalled() + expect(mockRes.status).toHaveBeenCalledWith(404) + expect(mockRes.json).toHaveBeenCalledWith({ + message: MOCK_ERROR_STRING, + }) + }) + }) + + describe('errors in form access', () => { + const MOCK_SPCP_SESSION = { + userName: 'mock', + } + + it('should return 200 with isIntranetUser set to false when a user accesses a form from outside intranet', async () => { + // Arrange + const MOCK_NIL_AUTH_FORM = ({ + ...BASE_FORM, + authType: AuthType.NIL, + } as unknown) as IPopulatedForm + const mockRes = expressHandler.mockResponse() + + MockAuthService.getFormIfPublic.mockReturnValueOnce( + okAsync(MOCK_NIL_AUTH_FORM), + ) + MockFormService.checkFormSubmissionLimitAndDeactivateForm.mockReturnValueOnce( + okAsync(MOCK_NIL_AUTH_FORM), + ) + MockFormService.checkIsIntranetFormAccess.mockReturnValueOnce(false) + + // Act + await PublicFormController.handleGetPublicForm( + MOCK_REQ, + mockRes, + jest.fn(), + ) + + // Assert + expect(mockRes.json).toHaveBeenCalledWith({ + form: MOCK_NIL_AUTH_FORM.getPublicView(), + isIntranetUser: false, + }) + }) + + it('should return 200 with isIntranetUser set to true when a intranet user accesses an AuthType.SP form', async () => { + // Arrange + const MOCK_SP_AUTH_FORM = ({ + ...BASE_FORM, + authType: AuthType.SP, + } as unknown) as IPopulatedForm + + const mockRes = expressHandler.mockResponse() + + MockSpcpFactory.extractJwtPayloadFromRequest.mockReturnValueOnce( + okAsync(MOCK_SPCP_SESSION), + ) + MockFormService.checkIsIntranetFormAccess.mockReturnValueOnce(true) + MockAuthService.getFormIfPublic.mockReturnValueOnce( + okAsync(MOCK_SP_AUTH_FORM), + ) + MockFormService.checkFormSubmissionLimitAndDeactivateForm.mockReturnValueOnce( + okAsync(MOCK_SP_AUTH_FORM), + ) + + // Act + await PublicFormController.handleGetPublicForm( + MOCK_REQ, + mockRes, + jest.fn(), + ) + + // Assert + expect(mockRes.json).toHaveBeenCalledWith({ + form: MOCK_SP_AUTH_FORM.getPublicView(), + spcpSession: MOCK_SPCP_SESSION, + isIntranetUser: true, + }) + }) + + it('should return 200 with isIntranetUser set to true when a intranet user accesses an AuthType.CP form', async () => { + // Arrange + const MOCK_CP_AUTH_FORM = ({ + ...BASE_FORM, + authType: AuthType.CP, + } as unknown) as IPopulatedForm + + const mockRes = expressHandler.mockResponse() + + MockFormService.checkIsIntranetFormAccess.mockReturnValueOnce(true) + MockSpcpFactory.extractJwtPayloadFromRequest.mockReturnValueOnce( + okAsync(MOCK_SPCP_SESSION), + ) + MockAuthService.getFormIfPublic.mockReturnValueOnce( + okAsync(MOCK_CP_AUTH_FORM), + ) + MockFormService.checkFormSubmissionLimitAndDeactivateForm.mockReturnValueOnce( + okAsync(MOCK_CP_AUTH_FORM), + ) + + // Act + await PublicFormController.handleGetPublicForm( + MOCK_REQ, + mockRes, + jest.fn(), + ) + + // Assert + expect(mockRes.json).toHaveBeenCalledWith({ + form: MOCK_CP_AUTH_FORM.getPublicView(), + spcpSession: MOCK_SPCP_SESSION, + isIntranetUser: true, + }) + }) + + it('should return 200 with isIntranetUser set to true when a intranet user accesses an AuthType.MyInfo form', async () => { + // Arrange + const MOCK_MYINFO_AUTH_FORM = ({ + ...BASE_FORM, + esrvcId: 'thing', + authType: AuthType.MyInfo, + toJSON: jest.fn().mockReturnValue(BASE_FORM), + } as unknown) as IPopulatedForm + const mockRes = expressHandler.mockResponse({ + clearCookie: jest.fn().mockReturnThis(), + cookie: jest.fn().mockReturnThis(), + }) + + const MOCK_MYINFO_DATA = new MyInfoData({ + uinFin: 'i am a fish', + } as IPersonResponse) + + MockAuthService.getFormIfPublic.mockReturnValueOnce( + okAsync(MOCK_MYINFO_AUTH_FORM), + ) + MockFormService.checkFormSubmissionLimitAndDeactivateForm.mockReturnValueOnce( + okAsync(MOCK_MYINFO_AUTH_FORM), + ) + MockFormService.checkIsIntranetFormAccess.mockReturnValueOnce(true) + MockMyInfoFactory.getMyInfoDataForForm.mockReturnValueOnce( + okAsync(MOCK_MYINFO_DATA), + ) + MockMyInfoFactory.prefillAndSaveMyInfoFields.mockReturnValueOnce( + okAsync([]), + ) + + // Act + await PublicFormController.handleGetPublicForm( + mockReqWithCookies, + mockRes, + jest.fn(), + ) + + // Assert + expect(mockRes.clearCookie).not.toHaveBeenCalled() + expect(mockRes.cookie).toHaveBeenCalled() + expect(mockRes.json).toHaveBeenCalledWith({ + form: { ...MOCK_MYINFO_AUTH_FORM.getPublicView(), form_fields: [] }, + spcpSession: { userName: MOCK_MYINFO_DATA.getUinFin() }, + isIntranetUser: true, + }) + }) + }) + }) }) diff --git a/src/app/modules/form/public-form/public-form.controller.ts b/src/app/modules/form/public-form/public-form.controller.ts index 474058a54c..90034261da 100644 --- a/src/app/modules/form/public-form/public-form.controller.ts +++ b/src/app/modules/form/public-form/public-form.controller.ts @@ -1,14 +1,31 @@ import { RequestHandler } from 'express' import { StatusCodes } from 'http-status-codes' import querystring from 'querystring' +import { UnreachableCaseError } from 'ts-essentials' import { createLoggerWithLabel } from '../../../../config/logger' -import { createReqMeta } from '../../../utils/request' +import { AuthType } from '../../../../types' +import { ErrorDto } from '../../../../types/api' +import { isMongoError } from '../../../utils/handle-mongo-error' +import { createReqMeta, getRequestIp } from '../../../utils/request' +import { getFormIfPublic } from '../../auth/auth.service' +import { + MYINFO_COOKIE_NAME, + MYINFO_COOKIE_OPTIONS, +} from '../../myinfo/myinfo.constants' +import { + MyInfoCookieAccessError, + MyInfoMissingAccessTokenError, +} from '../../myinfo/myinfo.errors' +import { MyInfoFactory } from '../../myinfo/myinfo.factory' +import { extractAndAssertMyInfoCookieValidity } from '../../myinfo/myinfo.util' +import { InvalidJwtError, VerifyJwtError } from '../../spcp/spcp.errors' +import { SpcpFactory } from '../../spcp/spcp.factory' import { PrivateFormError } from '../form.errors' import * as FormService from '../form.service' import * as PublicFormService from './public-form.service' -import { RedirectParams } from './public-form.types' +import { PublicFormViewDto, RedirectParams } from './public-form.types' import { mapRouteError } from './public-form.utils' const logger = createLoggerWithLabel(module) @@ -161,3 +178,144 @@ export const handleRedirect: RequestHandler< redirectPath, }) } + +/** + * Handler for GET /:formId/publicform endpoint + * @returns 200 if the form exists + * @returns 404 if form with formId does not exist or is private + * @returns 410 if form has been archived + * @returns 500 if database error occurs or if the type of error is unknown + */ +export const handleGetPublicForm: RequestHandler< + { formId: string }, + PublicFormViewDto | ErrorDto +> = async (req, res) => { + const { formId } = req.params + const logMeta = { + action: 'handleGetPublicForm', + ...createReqMeta(req), + formId, + } + + const formResult = await getFormIfPublic(formId).andThen((form) => + FormService.checkFormSubmissionLimitAndDeactivateForm(form), + ) + + // Early return if form is not public or any error occurred. + if (formResult.isErr()) { + const { error } = formResult + // NOTE: Only log on possible database errors. + // This is because the other kinds of errors are expected errors and are not truly exceptional + if (isMongoError(error)) { + logger.error({ + message: 'Error retrieving public form', + meta: logMeta, + error, + }) + } + const { errorMessage, statusCode } = mapRouteError(error) + return res.status(statusCode).json({ message: errorMessage }) + } + + const form = formResult.value + const publicForm = form.getPublicView() + const { authType } = form + const isIntranetUser = FormService.checkIsIntranetFormAccess( + getRequestIp(req), + form, + ) + + switch (authType) { + case AuthType.NIL: + return res.json({ form: publicForm, isIntranetUser }) + case AuthType.SP: + case AuthType.CP: + return SpcpFactory.extractJwtPayloadFromRequest(authType, req.cookies) + .map(({ userName }) => + res.json({ + form: publicForm, + isIntranetUser, + spcpSession: { userName }, + }), + ) + .mapErr((error) => { + // Report only relevant errors - verification failed for user here + if ( + error instanceof VerifyJwtError || + error instanceof InvalidJwtError + ) { + logger.error({ + message: 'Error getting public form', + meta: logMeta, + error, + }) + } + return res.json({ form: publicForm, isIntranetUser }) + }) + case AuthType.MyInfo: { + // Step 1. Fetch required data and fill the form based off data retrieved + return ( + MyInfoFactory.getMyInfoDataForForm(form, req.cookies) + .andThen((myInfoData) => { + return MyInfoFactory.prefillAndSaveMyInfoFields( + form._id, + myInfoData, + form.toJSON().form_fields, + ).map((prefilledFields) => ({ + prefilledFields, + spcpSession: { userName: myInfoData.getUinFin() }, + })) + }) + // Check if the user is signed in + .andThen(({ prefilledFields, spcpSession }) => { + return extractAndAssertMyInfoCookieValidity(req.cookies).map( + (myInfoCookie) => ({ + prefilledFields, + spcpSession, + myInfoCookie, + }), + ) + }) + .map(({ myInfoCookie, prefilledFields, spcpSession }) => { + const updatedMyInfoCookie = { + ...myInfoCookie, + usedCount: myInfoCookie.usedCount + 1, + } + // Set the updated cookie accordingly and return the form back to the user + return res + .cookie( + MYINFO_COOKIE_NAME, + updatedMyInfoCookie, + MYINFO_COOKIE_OPTIONS, + ) + .json({ + spcpSession, + form: { ...publicForm, form_fields: prefilledFields }, + isIntranetUser, + }) + }) + .mapErr((error) => { + // NOTE: If the user is not signed in or if the user refreshes the page while logged in, it is not an error. + // myInfoError is set to true only when the authentication provider rejects the user's attempt at auth + // or when there is a network or database error during the process of retrieval + const isMyInfoError = !( + error instanceof MyInfoCookieAccessError || + error instanceof MyInfoMissingAccessTokenError + ) + // No need for cookie if data could not be retrieved + // NOTE: If the user does not have any cookie, clearing the cookie still has the same result + return res + .clearCookie(MYINFO_COOKIE_NAME, MYINFO_COOKIE_OPTIONS) + .json({ + form: publicForm, + // Setting to undefined ensures that the frontend does not get myInfoError if it is false + myInfoError: isMyInfoError || undefined, + isIntranetUser, + }) + }) + ) + } + default: + return new UnreachableCaseError(authType) + } +} diff --git a/src/app/modules/form/public-form/public-form.routes.ts b/src/app/modules/form/public-form/public-form.routes.ts new file mode 100644 index 0000000000..0d4fd71cc0 --- /dev/null +++ b/src/app/modules/form/public-form/public-form.routes.ts @@ -0,0 +1,27 @@ +import { Router } from 'express' + +import * as PublicFormController from './public-form.controller' + +export const PublicFormRouter = Router() + +/** + * Returns the specified form to the user, along with any + * identity information obtained from SingPass/CorpPass, + * and MyInfo details, if any. + * + * WARNING: TemperatureSG batch jobs rely on this endpoint to + * retrieve the master list of personnel for daily reporting. + * Please strictly ensure backwards compatibility. + * + * @route GET /{formId}/publicform + * @group forms - endpoints to serve forms + * @param {string} formId.path.required - the form id + * @consumes application/json + * @produces application/json + * @returns {string} 404 - form is not made public + * @returns {PublicForm.model} 200 - the form, and other information + */ +PublicFormRouter.get( + '/:formId([a-fA-F0-9]{24})/publicform', + PublicFormController.handleGetPublicForm, +) diff --git a/src/app/modules/form/public-form/public-form.types.ts b/src/app/modules/form/public-form/public-form.types.ts index 543318873c..6f970f46f6 100644 --- a/src/app/modules/form/public-form/public-form.types.ts +++ b/src/app/modules/form/public-form/public-form.types.ts @@ -1,5 +1,10 @@ import { ParamsDictionary } from 'express-serve-static-core' +import { IFieldSchema, PublicForm } from 'src/types' + +import { SpcpSession } from '../../../../types/spcp' +import { IPossiblyPrefilledField } from '../../myinfo/myinfo.types' + export type Metatags = { title: string description?: string @@ -13,3 +18,16 @@ export type RedirectParams = ParamsDictionary & { // TODO(#144): Rename Id to formId after all routes have been updated. Id: string } + +// NOTE: This is needed because PublicForm inherits from IFormDocument (where form_fields has type of IFieldSchema). +// However, the form returned back to the client has form_field of two possible types +interface PossiblyPrefilledPublicForm extends Omit { + form_fields: IPossiblyPrefilledField[] | IFieldSchema[] +} + +export type PublicFormViewDto = { + form: PossiblyPrefilledPublicForm + spcpSession?: SpcpSession + isIntranetUser?: boolean + myInfoError?: true +} diff --git a/src/app/modules/myinfo/__tests__/myinfo.factory.spec.ts b/src/app/modules/myinfo/__tests__/myinfo.factory.spec.ts index 4812b81efc..932d5200ca 100644 --- a/src/app/modules/myinfo/__tests__/myinfo.factory.spec.ts +++ b/src/app/modules/myinfo/__tests__/myinfo.factory.spec.ts @@ -2,7 +2,7 @@ import { mocked } from 'ts-jest/utils' import config from 'src/config/config' import { ISpcpMyInfo } from 'src/config/feature-manager' -import { Environment } from 'src/types' +import { Environment, IPopulatedForm } from 'src/types' import { MyInfoData } from '../myinfo.adapter' import { createMyInfoFactory } from '../myinfo.factory' @@ -45,12 +45,12 @@ describe('myinfo.factory', () => { ) const parseMyInfoRelayStateResult = MyInfoFactory.parseMyInfoRelayState('') const extractUinFinResult = MyInfoFactory.extractUinFin('') - const fetchMyInfoPersonDataResult = await MyInfoFactory.fetchMyInfoPersonData( - '', - [], - '', + const getMyInfoDataForFormResult = await MyInfoFactory.getMyInfoDataForForm( + ({} as unknown) as IPopulatedForm, + {}, ) - const prefillMyInfoFieldsResult = MyInfoFactory.prefillMyInfoFields( + const prefillAndSaveMyInfoFieldsResult = await MyInfoFactory.prefillAndSaveMyInfoFields( + '', {} as MyInfoData, [], ) @@ -71,8 +71,8 @@ describe('myinfo.factory', () => { expect(parseMyInfoRelayStateResult._unsafeUnwrapErr()).toEqual(error) expect(createRedirectURLResult._unsafeUnwrapErr()).toEqual(error) expect(extractUinFinResult._unsafeUnwrapErr()).toEqual(error) - expect(fetchMyInfoPersonDataResult._unsafeUnwrapErr()).toEqual(error) - expect(prefillMyInfoFieldsResult._unsafeUnwrapErr()).toEqual(error) + expect(getMyInfoDataForFormResult._unsafeUnwrapErr()).toEqual(error) + expect(prefillAndSaveMyInfoFieldsResult._unsafeUnwrapErr()).toEqual(error) expect(saveMyInfoHashesResult._unsafeUnwrapErr()).toEqual(error) expect(fetchMyInfoHashesResult._unsafeUnwrapErr()).toEqual(error) expect(checkMyInfoHashesResult._unsafeUnwrapErr()).toEqual(error) @@ -94,12 +94,12 @@ describe('myinfo.factory', () => { ) const parseMyInfoRelayStateResult = MyInfoFactory.parseMyInfoRelayState('') const extractUinFinResult = MyInfoFactory.extractUinFin('') - const fetchMyInfoPersonDataResult = await MyInfoFactory.fetchMyInfoPersonData( - '', - [], - '', + const getMyInfoDataForFormResult = await MyInfoFactory.getMyInfoDataForForm( + ({} as unknown) as IPopulatedForm, + {}, ) - const prefillMyInfoFieldsResult = MyInfoFactory.prefillMyInfoFields( + const prefillAndSaveMyInfoFieldsResult = await MyInfoFactory.prefillAndSaveMyInfoFields( + '', {} as MyInfoData, [], ) @@ -120,8 +120,8 @@ describe('myinfo.factory', () => { expect(parseMyInfoRelayStateResult._unsafeUnwrapErr()).toEqual(error) expect(createRedirectURLResult._unsafeUnwrapErr()).toEqual(error) expect(extractUinFinResult._unsafeUnwrapErr()).toEqual(error) - expect(fetchMyInfoPersonDataResult._unsafeUnwrapErr()).toEqual(error) - expect(prefillMyInfoFieldsResult._unsafeUnwrapErr()).toEqual(error) + expect(getMyInfoDataForFormResult._unsafeUnwrapErr()).toEqual(error) + expect(prefillAndSaveMyInfoFieldsResult._unsafeUnwrapErr()).toEqual(error) expect(saveMyInfoHashesResult._unsafeUnwrapErr()).toEqual(error) expect(fetchMyInfoHashesResult._unsafeUnwrapErr()).toEqual(error) expect(checkMyInfoHashesResult._unsafeUnwrapErr()).toEqual(error) diff --git a/src/app/modules/myinfo/__tests__/myinfo.service.spec.ts b/src/app/modules/myinfo/__tests__/myinfo.service.spec.ts index b433e47025..418ec67bac 100644 --- a/src/app/modules/myinfo/__tests__/myinfo.service.spec.ts +++ b/src/app/modules/myinfo/__tests__/myinfo.service.spec.ts @@ -1,4 +1,5 @@ import bcrypt from 'bcrypt' +import { ObjectId } from 'bson-ext' import mongoose from 'mongoose' import { mocked } from 'ts-jest/utils' import { v4 as uuidv4 } from 'uuid' @@ -10,17 +11,20 @@ import { IFieldSchema, IHashes, IMyInfoHashSchema, + IPopulatedForm, MyInfoAttribute, } from 'src/types' import dbHandler from 'tests/unit/backend/helpers/jest-db' +import { DatabaseError } from '../../core/core.errors' import { MyInfoData } from '../myinfo.adapter' import { MYINFO_CONSENT_PAGE_PURPOSE } from '../myinfo.constants' import { MyInfoCircuitBreakerError, MyInfoFetchError, MyInfoInvalidAccessTokenError, + MyInfoMissingAccessTokenError, MyInfoParseRelayStateError, } from '../myinfo.errors' import { IPossiblyPrefilledField, MyInfoRelayState } from '../myinfo.types' @@ -35,11 +39,13 @@ import { MOCK_HASHED_FIELD_IDS, MOCK_HASHES, MOCK_MYINFO_DATA, + MOCK_MYINFO_FORM, MOCK_POPULATED_FORM_FIELDS, MOCK_REDIRECT_URL, MOCK_REQUESTED_ATTRS, MOCK_RESPONSES, MOCK_SERVICE_PARAMS, + MOCK_SUCCESSFUL_COOKIE, MOCK_UINFIN, } from './myinfo.test.constants' @@ -183,75 +189,14 @@ describe('MyInfoService', () => { }) }) - describe('fetchMyInfoPersonData', () => { - beforeEach(() => { - myInfoService = new MyInfoService(MOCK_SERVICE_PARAMS) - }) - - it('should call MyInfoGovClient.getPerson with the correct parameters', async () => { - const mockReturnedParams = { - uinFin: MOCK_UINFIN, - data: MOCK_MYINFO_DATA, - } - mockGetPerson.mockResolvedValueOnce(mockReturnedParams) - const result = await myInfoService.fetchMyInfoPersonData( - MOCK_ACCESS_TOKEN, - MOCK_REQUESTED_ATTRS, - MOCK_ESRVC_ID, - ) - - expect(mockGetPerson).toHaveBeenCalledWith( - MOCK_ACCESS_TOKEN, - MOCK_REQUESTED_ATTRS.concat('uinfin' as MyInfoAttribute), - MOCK_ESRVC_ID, - ) - expect(result._unsafeUnwrap()).toEqual(new MyInfoData(mockReturnedParams)) - }) - - it('should throw MyInfoFetchError when getPerson fails once', async () => { - mockGetPerson.mockRejectedValueOnce(new Error()) - const result = await myInfoService.fetchMyInfoPersonData( - MOCK_ACCESS_TOKEN, - MOCK_REQUESTED_ATTRS, - MOCK_ESRVC_ID, - ) - - expect(mockGetPerson).toHaveBeenCalledWith( - MOCK_ACCESS_TOKEN, - MOCK_REQUESTED_ATTRS.concat('uinfin' as MyInfoAttribute), - MOCK_ESRVC_ID, - ) - expect(result._unsafeUnwrapErr()).toEqual(new MyInfoFetchError()) - }) - - it('should throw MyInfoCircuitBreakerError when getPerson fails 5 times', async () => { - mockGetPerson.mockRejectedValue(new Error()) - for (let i = 0; i < 5; i++) { - await myInfoService.fetchMyInfoPersonData( - MOCK_ACCESS_TOKEN, - MOCK_REQUESTED_ATTRS, - MOCK_ESRVC_ID, - ) - } - const result = await myInfoService.fetchMyInfoPersonData( - MOCK_ACCESS_TOKEN, - MOCK_REQUESTED_ATTRS, - MOCK_ESRVC_ID, - ) - - // Last function call doesn't count as breaker is open, so expect 5 calls - expect(mockGetPerson).toHaveBeenCalledTimes(5) - expect(result._unsafeUnwrapErr()).toEqual(new MyInfoCircuitBreakerError()) - }) - }) - - describe('prefillMyInfoFields', () => { - it('should prefill fields correctly', () => { + describe('prefillAndSaveMyInfoFields', () => { + it('should prefill fields correctly', async () => { const mockData = new MyInfoData({ data: MOCK_MYINFO_DATA, uinFin: MOCK_UINFIN, }) - const result = myInfoService.prefillMyInfoFields( + const result = await myInfoService.prefillAndSaveMyInfoFields( + new ObjectId().toHexString(), mockData, MOCK_FORM_FIELDS as IFieldSchema[], ) @@ -313,7 +258,7 @@ describe('MyInfoService', () => { MOCK_POPULATED_FORM_FIELDS as IPossiblyPrefilledField[], ) expect(result._unsafeUnwrapErr()).toEqual( - new Error('Failed to save MyInfo hashes to database'), + new DatabaseError('Failed to save MyInfo hashes to database'), ) }) }) @@ -435,4 +380,107 @@ describe('MyInfoService', () => { ) }) }) + + describe('getMyInfoDataForForm', () => { + // NOTE: Mocks the underlying circuit breaker implementation to avoid network calls + beforeEach(() => { + myInfoService = new MyInfoService(MOCK_SERVICE_PARAMS) + }) + + it('should return myInfo data when the provided form and cookie is valid', async () => { + // Arrange + const mockReturnedParams = { + uinFin: MOCK_UINFIN, + data: MOCK_MYINFO_DATA, + } + + mockGetPerson.mockResolvedValueOnce(mockReturnedParams) + + // Act + const result = await myInfoService.getMyInfoDataForForm( + MOCK_MYINFO_FORM as IPopulatedForm, + { MyInfoCookie: MOCK_SUCCESSFUL_COOKIE }, + ) + + // Assert + expect(result._unsafeUnwrap()).toEqual(new MyInfoData(mockReturnedParams)) + }) + + it('should call MyInfoGovClient.getPerson with the correct parameters', async () => { + // Arrange + const mockReturnedParams = { + uinFin: MOCK_UINFIN, + data: MOCK_MYINFO_DATA, + } + mockGetPerson.mockResolvedValueOnce(mockReturnedParams) + + // Act + const result = await myInfoService.getMyInfoDataForForm( + MOCK_MYINFO_FORM as IPopulatedForm, + { MyInfoCookie: MOCK_SUCCESSFUL_COOKIE }, + ) + + // Assert + expect(mockGetPerson).toHaveBeenCalledWith( + MOCK_ACCESS_TOKEN, + MOCK_REQUESTED_ATTRS.concat('uinfin' as MyInfoAttribute), + MOCK_ESRVC_ID, + ) + expect(result._unsafeUnwrap()).toEqual(new MyInfoData(mockReturnedParams)) + }) + + it('should throw MyInfoFetchError when getPerson fails once', async () => { + // Arrange + mockGetPerson.mockRejectedValueOnce(new Error()) + + // Act + const result = await myInfoService.getMyInfoDataForForm( + MOCK_MYINFO_FORM as IPopulatedForm, + { MyInfoCookie: MOCK_SUCCESSFUL_COOKIE }, + ) + + // Assert + expect(mockGetPerson).toHaveBeenCalledWith( + MOCK_ACCESS_TOKEN, + MOCK_REQUESTED_ATTRS.concat('uinfin' as MyInfoAttribute), + MOCK_ESRVC_ID, + ) + expect(result._unsafeUnwrapErr()).toEqual(new MyInfoFetchError()) + }) + + it('should throw MyInfoCircuitBreakerError when getPerson fails 5 times', async () => { + // Arrange + mockGetPerson.mockRejectedValue(new Error()) + for (let i = 0; i < 5; i++) { + await myInfoService.getMyInfoDataForForm( + MOCK_MYINFO_FORM as IPopulatedForm, + { MyInfoCookie: MOCK_SUCCESSFUL_COOKIE }, + ) + } + + // Act + const result = await myInfoService.getMyInfoDataForForm( + MOCK_MYINFO_FORM as IPopulatedForm, + { MyInfoCookie: MOCK_SUCCESSFUL_COOKIE }, + ) + + // Assert + // Last function call doesn't count as breaker is open, so expect 5 calls + expect(mockGetPerson).toHaveBeenCalledTimes(5) + expect(result._unsafeUnwrapErr()).toEqual(new MyInfoCircuitBreakerError()) + }) + it('should not validate the form if the cookie does not exist', async () => { + // Arrange + const expected = new MyInfoMissingAccessTokenError() + + // Act + const result = await myInfoService.getMyInfoDataForForm( + MOCK_MYINFO_FORM as IPopulatedForm, + {}, + ) + + // Assert + expect(result._unsafeUnwrapErr()).toEqual(expected) + }) + }) }) diff --git a/src/app/modules/myinfo/__tests__/myinfo.test.constants.ts b/src/app/modules/myinfo/__tests__/myinfo.test.constants.ts index 3ac62d4c4b..033b77af50 100644 --- a/src/app/modules/myinfo/__tests__/myinfo.test.constants.ts +++ b/src/app/modules/myinfo/__tests__/myinfo.test.constants.ts @@ -5,12 +5,16 @@ import { MyInfoSource, } from '@opengovsg/myinfo-gov-client' import { ObjectId } from 'bson' -import { merge, zipWith } from 'lodash' +import { merge, omit, zipWith } from 'lodash' import { ISpcpMyInfo } from 'src/config/feature-manager' -import { Environment, IFormSchema, MyInfoAttribute } from 'src/types' +import { AuthType, Environment, IFormSchema, MyInfoAttribute } from 'src/types' -import { IMyInfoServiceConfig } from '../myinfo.types' +import { + IMyInfoServiceConfig, + MyInfoCookieState, + MyInfoSuccessfulCookiePayload, +} from '../myinfo.types' export const MOCK_MYINFO_DATA = { name: { @@ -144,6 +148,23 @@ export const MOCK_SERVICE_PARAMS: IMyInfoServiceConfig = { export const MOCK_MYINFO_FORM = ({ _id: MOCK_FORM_ID, esrvcId: MOCK_ESRVC_ID, - authType: 'MyInfo', + authType: AuthType.MyInfo, + admin: { + _id: new ObjectId().toHexString(), + agency: new ObjectId().toHexString(), + }, getUniqueMyInfoAttrs: () => MOCK_REQUESTED_ATTRS, + getPublicView: function () { + return omit(this, 'admin') + }, + toJSON: function () { + return this + }, + form_fields: [], } as unknown) as IFormSchema + +export const MOCK_SUCCESSFUL_COOKIE: MyInfoSuccessfulCookiePayload = { + accessToken: MOCK_ACCESS_TOKEN, + usedCount: 0, + state: MyInfoCookieState.Success, +} diff --git a/src/app/modules/myinfo/myinfo.errors.ts b/src/app/modules/myinfo/myinfo.errors.ts index 29f25c8c04..f149653498 100644 --- a/src/app/modules/myinfo/myinfo.errors.ts +++ b/src/app/modules/myinfo/myinfo.errors.ts @@ -103,3 +103,12 @@ export class MyInfoCookieStateError extends ApplicationError { super(message) } } + +/** + * MyInfo cookie has been used more than once + */ +export class MyInfoCookieAccessError extends ApplicationError { + constructor(message = 'MyInfo cookie has already been used') { + super(message) + } +} diff --git a/src/app/modules/myinfo/myinfo.factory.ts b/src/app/modules/myinfo/myinfo.factory.ts index 9327f04d5f..d7f51b04b6 100644 --- a/src/app/modules/myinfo/myinfo.factory.ts +++ b/src/app/modules/myinfo/myinfo.factory.ts @@ -10,19 +10,24 @@ import { IFieldSchema, IHashes, IMyInfoHashSchema, - MyInfoAttribute, + IPopulatedForm, } from '../../../types' import { DatabaseError, MissingFeatureError } from '../core/core.errors' import { ProcessedFieldResponse } from '../submission/submission.types' import { MyInfoData } from './myinfo.adapter' import { + MyInfoAuthTypeError, MyInfoCircuitBreakerError, + MyInfoCookieAccessError, + MyInfoCookieStateError, MyInfoFetchError, MyInfoHashDidNotMatchError, MyInfoHashingError, MyInfoInvalidAccessTokenError, + MyInfoMissingAccessTokenError, MyInfoMissingHashError, + MyInfoNoESrvcIdError, MyInfoParseRelayStateError, } from './myinfo.errors' import { MyInfoService } from './myinfo.service' @@ -39,24 +44,20 @@ interface IMyInfoFactory { retrieveAccessToken: ( authCode: string, ) => ResultAsync - fetchMyInfoPersonData: ( - accessToken: string, - requestedAttributes: MyInfoAttribute[], - singpassEserviceId: string, - ) => ResultAsync< - MyInfoData, - MyInfoCircuitBreakerError | MyInfoFetchError | MissingFeatureError - > parseMyInfoRelayState: ( relayState: string, ) => Result< MyInfoParsedRelayState, MyInfoParseRelayStateError | MissingFeatureError > - prefillMyInfoFields: ( + prefillAndSaveMyInfoFields: ( + formId: string, myInfoData: MyInfoData, currFormFields: LeanDocument, - ) => Result + ) => ResultAsync< + IPossiblyPrefilledField[], + MyInfoHashingError | DatabaseError + > saveMyInfoHashes: ( uinFin: string, formId: string, @@ -82,6 +83,21 @@ interface IMyInfoFactory { extractUinFin: ( accessToken: string, ) => Result + + getMyInfoDataForForm: ( + form: IPopulatedForm, + cookies: Record, + ) => ResultAsync< + MyInfoData, + | MyInfoMissingAccessTokenError + | MyInfoCookieStateError + | MyInfoNoESrvcIdError + | MyInfoAuthTypeError + | MyInfoCircuitBreakerError + | MyInfoFetchError + | MissingFeatureError + | MyInfoCookieAccessError + > } export const createMyInfoFactory = ({ @@ -92,14 +108,14 @@ export const createMyInfoFactory = ({ const error = new MissingFeatureError(FeatureNames.SpcpMyInfo) return { retrieveAccessToken: () => errAsync(error), - fetchMyInfoPersonData: () => errAsync(error), - prefillMyInfoFields: () => err(error), + prefillAndSaveMyInfoFields: () => errAsync(error), saveMyInfoHashes: () => errAsync(error), fetchMyInfoHashes: () => errAsync(error), checkMyInfoHashes: () => errAsync(error), createRedirectURL: () => err(error), parseMyInfoRelayState: () => err(error), extractUinFin: () => err(error), + getMyInfoDataForForm: () => errAsync(error), } } return new MyInfoService({ diff --git a/src/app/modules/myinfo/myinfo.middleware.ts b/src/app/modules/myinfo/myinfo.middleware.ts deleted file mode 100644 index 2741a4a211..0000000000 --- a/src/app/modules/myinfo/myinfo.middleware.ts +++ /dev/null @@ -1,101 +0,0 @@ -// TODO (#144): move these into their respective controllers when -// those controllers are being refactored. -// A services module should not contain a controller. -import { RequestHandler } from 'express' -import { ParamsDictionary } from 'express-serve-static-core' - -import { createLoggerWithLabel } from '../../../config/logger' -import { AuthType, WithForm, WithJsonForm } from '../../../types' -import { createReqMeta } from '../../utils/request' - -import { MYINFO_COOKIE_NAME, MYINFO_COOKIE_OPTIONS } from './myinfo.constants' -import { MyInfoFactory } from './myinfo.factory' -import { MyInfoCookiePayload, MyInfoCookieState } from './myinfo.types' -import { extractMyInfoCookie, validateMyInfoForm } from './myinfo.util' - -const logger = createLoggerWithLabel(module) - -/** - * Middleware for prefilling MyInfo values. - * @returns next, always. If any error occurs, res.locals.myInfoError is set to true. - */ -export const addMyInfo: RequestHandler = async ( - req, - res, - next, -) => { - // TODO (#42): add proper types here when migrating away from middleware pattern - const formDocument = (req as WithForm).form - const formJson = formDocument.toJSON() - const myInfoCookieResult = extractMyInfoCookie(req.cookies) - - // No action needed if no cookie is present, this just means user is not signed in - if (formDocument.authType !== AuthType.MyInfo || myInfoCookieResult.isErr()) - return next() - const myInfoCookie = myInfoCookieResult.value - - // Error occurred while retrieving access token - if (myInfoCookie.state !== MyInfoCookieState.Success) { - res.locals.myInfoError = true - // Important - clear the cookie so that user will not see error on refresh - res.clearCookie(MYINFO_COOKIE_NAME, MYINFO_COOKIE_OPTIONS) - return next() - } - - // Access token is already used - if (myInfoCookie.usedCount > 0) { - res.clearCookie(MYINFO_COOKIE_NAME, MYINFO_COOKIE_OPTIONS) - return next() - } - - const requestedAttributes = (req as WithForm< - typeof req - >).form.getUniqueMyInfoAttrs() - return validateMyInfoForm(formDocument) - .asyncAndThen((form) => - MyInfoFactory.fetchMyInfoPersonData( - myInfoCookie.accessToken, - requestedAttributes, - form.esrvcId, - ), - ) - .andThen((myInfoData) => { - // Increment count in cookie - const cookiePayload: MyInfoCookiePayload = { - ...myInfoCookie, - usedCount: myInfoCookie.usedCount + 1, - } - res.cookie(MYINFO_COOKIE_NAME, cookiePayload, MYINFO_COOKIE_OPTIONS) - return MyInfoFactory.prefillMyInfoFields( - myInfoData, - formJson.form_fields, - ).asyncAndThen((prefilledFields) => { - formJson.form_fields = prefilledFields - ;(req as WithJsonForm).form = formJson - res.locals.spcpSession = { userName: myInfoData.getUinFin() } - return MyInfoFactory.saveMyInfoHashes( - myInfoData.getUinFin(), - formDocument._id, - prefilledFields, - ) - }) - }) - .map(() => next()) - .mapErr((error) => { - // No need for cookie if data could not be retrieved - res.clearCookie(MYINFO_COOKIE_NAME, MYINFO_COOKIE_OPTIONS) - logger.error({ - message: error.message, - meta: { - action: 'addMyInfo', - ...createReqMeta(req), - formId: formDocument._id, - esrvcId: formDocument.esrvcId, - requestedAttributes, - }, - error, - }) - res.locals.myInfoError = true - return next() - }) -} diff --git a/src/app/modules/myinfo/myinfo.service.ts b/src/app/modules/myinfo/myinfo.service.ts index 92291c1b23..99e5062269 100644 --- a/src/app/modules/myinfo/myinfo.service.ts +++ b/src/app/modules/myinfo/myinfo.service.ts @@ -16,9 +16,10 @@ import { IFieldSchema, IHashes, IMyInfoHashSchema, + IPopulatedForm, MyInfoAttribute, } from '../../../types' -import { DatabaseError } from '../core/core.errors' +import { DatabaseError, MissingFeatureError } from '../core/core.errors' import { ProcessedFieldResponse } from '../submission/submission.types' import { internalAttrListToScopes, MyInfoData } from './myinfo.adapter' @@ -28,12 +29,17 @@ import { MYINFO_ROUTER_PREFIX, } from './myinfo.constants' import { + MyInfoAuthTypeError, MyInfoCircuitBreakerError, + MyInfoCookieAccessError, + MyInfoCookieStateError, MyInfoFetchError, MyInfoHashDidNotMatchError, MyInfoHashingError, MyInfoInvalidAccessTokenError, + MyInfoMissingAccessTokenError, MyInfoMissingHashError, + MyInfoNoESrvcIdError, MyInfoParseRelayStateError, } from './myinfo.errors' import { @@ -43,10 +49,13 @@ import { MyInfoParsedRelayState, } from './myinfo.types' import { + assertMyInfoCookieUnused, compareHashedValues, createRelayState, + extractAndAssertMyInfoCookieValidity, hashFieldValues, isMyInfoRelayState, + validateMyInfoForm, } from './myinfo.util' import getMyInfoHashModel from './myinfo_hash.model' @@ -86,6 +95,12 @@ export class MyInfoService { */ #spCookieMaxAge: number + #fetchMyInfoPersonData: ( + accessToken: string, + requestedAttributes: MyInfoAttribute[], + singpassEserviceId: string, + ) => ResultAsync + /** * * @param myInfoConfig Environment variables including myInfoClientMode and myInfoKeyPath @@ -118,6 +133,54 @@ export class MyInfoService { this.#myInfoGovClient.getPerson(accessToken, attributes, eSrvcId), BREAKER_PARAMS, ) + + /** + * Fetches MyInfo person detail with given params. + * This function has circuit breaking built into it, and will throw an error + * if any recent usages of this function returned an error. + * @param params The params required to retrieve the data. + * @param params.uinFin The uin/fin of the person's data to retrieve. + * @param params.requestedAttributes The requested attributes to fetch. + * @param params.singpassEserviceId The eservice id of the form requesting the data. + * @returns the person object retrieved. + * @throws an error on fetch failure or if circuit breaker is in the opened state. Use {@link CircuitBreaker#isOurError} to determine if a rejection was a result of the circuit breaker or the action. + */ + this.#fetchMyInfoPersonData = function ( + accessToken: string, + requestedAttributes: MyInfoAttribute[], + singpassEserviceId: string, + ): ResultAsync { + return ResultAsync.fromPromise( + this.#myInfoPersonBreaker + .fire( + accessToken, + internalAttrListToScopes(requestedAttributes), + singpassEserviceId, + ) + .then((response) => new MyInfoData(response)), + (error) => { + const logMeta = { + action: 'fetchMyInfoPersonData', + requestedAttributes, + } + if (CircuitBreaker.isOurError(error)) { + logger.error({ + message: 'Circuit breaker tripped', + meta: logMeta, + error, + }) + return new MyInfoCircuitBreakerError() + } else { + logger.error({ + message: 'Error retrieving data from MyInfo', + meta: logMeta, + error, + }) + return new MyInfoFetchError() + } + }, + ) + } } /** @@ -218,64 +281,21 @@ export class MyInfoService { ) } - /** - * Fetches MyInfo person detail with given params. - * This function has circuit breaking built into it, and will throw an error - * if any recent usages of this function returned an error. - * @param params The params required to retrieve the data. - * @param params.uinFin The uin/fin of the person's data to retrieve. - * @param params.requestedAttributes The requested attributes to fetch. - * @param params.singpassEserviceId The eservice id of the form requesting the data. - * @returns the person object retrieved. - * @throws an error on fetch failure or if circuit breaker is in the opened state. Use {@link CircuitBreaker#isOurError} to determine if a rejection was a result of the circuit breaker or the action. - */ - fetchMyInfoPersonData( - accessToken: string, - requestedAttributes: MyInfoAttribute[], - singpassEserviceId: string, - ): ResultAsync { - return ResultAsync.fromPromise( - this.#myInfoPersonBreaker - .fire( - accessToken, - internalAttrListToScopes(requestedAttributes), - singpassEserviceId, - ) - .then((response) => new MyInfoData(response)), - (error) => { - const logMeta = { - action: 'fetchMyInfoPersonData', - requestedAttributes, - } - if (CircuitBreaker.isOurError(error)) { - logger.error({ - message: 'Circuit breaker tripped', - meta: logMeta, - error, - }) - return new MyInfoCircuitBreakerError() - } else { - logger.error({ - message: 'Error retrieving data from MyInfo', - meta: logMeta, - error, - }) - return new MyInfoFetchError() - } - }, - ) - } - /** * Prefill given current form fields with given MyInfo data. + * Saves the has of the prefilled fields as well because the two operations are atomic and should not be separated * @param myInfoData * @param currFormFields * @returns currFormFields with the MyInfo fields prefilled with data from myInfoData */ - prefillMyInfoFields( + prefillAndSaveMyInfoFields( + formId: string, myInfoData: MyInfoData, currFormFields: LeanDocument, - ): Result { + ): ResultAsync< + IPossiblyPrefilledField[], + MyInfoHashingError | DatabaseError + > { const prefilledFields = currFormFields.map((field) => { if (!field.myInfo?.attr) return field @@ -290,7 +310,11 @@ export class MyInfoService { prefilledField.disabled = isReadOnly return prefilledField }) - return ok(prefilledFields) + return this.saveMyInfoHashes( + myInfoData.getUinFin(), + formId, + prefilledFields, + ).map(() => prefilledFields) } /** @@ -451,4 +475,46 @@ export class MyInfoService { }, )() } + + /** + * Gets myInfo data using the provided form and the cookies of the request + * @param form the form to validate + * @param cookies cookies of the request + * @returns ok(MyInfoData) if the form has been validated successfully + * @returns err(MyInfoMissingAccessTokenError) if no myInfoCookie was found on the request + * @returns err(MyInfoCookieStateError) if cookie was not successful + * @returns err(MyInfoCookieAccessError) if the cookie has already been used before + * @returns err(MyInfoNoESrvcIdError) if form has no eserviceId + * @returns err(MyInfoAuthTypeError) if the client was not authenticated using MyInfo + * @returns err(MyInfoCircuitBreakerError) if circuit breaker was active + * @returns err(MyInfoFetchError) if validated but the data could not be retrieved + * @returns err(MissingFeatureError) if using an outdated version that does not support myInfo + */ + getMyInfoDataForForm( + form: IPopulatedForm, + cookies: Record, + ): ResultAsync< + MyInfoData, + | MyInfoMissingAccessTokenError + | MyInfoCookieStateError + | MyInfoNoESrvcIdError + | MyInfoAuthTypeError + | MyInfoCircuitBreakerError + | MyInfoFetchError + | MissingFeatureError + | MyInfoCookieAccessError + > { + const requestedAttributes = form.getUniqueMyInfoAttrs() + return extractAndAssertMyInfoCookieValidity(cookies) + .andThen((myInfoCookie) => assertMyInfoCookieUnused(myInfoCookie)) + .asyncAndThen((cookiePayload) => + validateMyInfoForm(form).asyncAndThen((form) => + this.#fetchMyInfoPersonData( + cookiePayload.accessToken, + requestedAttributes, + form.esrvcId, + ), + ), + ) + } } diff --git a/src/app/modules/myinfo/myinfo.types.ts b/src/app/modules/myinfo/myinfo.types.ts index d5d91c8a9f..2be487cad8 100644 --- a/src/app/modules/myinfo/myinfo.types.ts +++ b/src/app/modules/myinfo/myinfo.types.ts @@ -44,15 +44,15 @@ export enum MyInfoCookieState { Error = 'error', } +export type MyInfoSuccessfulCookiePayload = { + accessToken: string + usedCount: number + state: MyInfoCookieState.Success +} + export type MyInfoCookiePayload = - | { - accessToken: string - usedCount: number - state: MyInfoCookieState.Success - } - | { - state: Exclude - } + | MyInfoSuccessfulCookiePayload + | { state: Exclude } /** * The stringified properties included in the state sent to MyInfo. diff --git a/src/app/modules/myinfo/myinfo.util.ts b/src/app/modules/myinfo/myinfo.util.ts index a77db98b0c..cf154b7274 100644 --- a/src/app/modules/myinfo/myinfo.util.ts +++ b/src/app/modules/myinfo/myinfo.util.ts @@ -29,6 +29,7 @@ import { ProcessedFieldResponse } from '../submission/submission.types' import { MYINFO_COOKIE_NAME } from './myinfo.constants' import { MyInfoAuthTypeError, + MyInfoCookieAccessError, MyInfoCookieStateError, MyInfoHashDidNotMatchError, MyInfoHashingError, @@ -44,6 +45,7 @@ import { MyInfoCookieState, MyInfoHashPromises, MyInfoRelayState, + MyInfoSuccessfulCookiePayload, VisibleMyInfoResponse, } from './myinfo.types' @@ -346,6 +348,53 @@ export const extractMyInfoCookie = ( return err(new MyInfoMissingAccessTokenError()) } +/** + * Asserts that myInfoCookie is in success state + * This function acts as a discriminator so that the type of the cookie is encoded in its type + * @param cookie the cookie to + * @returns ok(cookie) the successful myInfoCookie + * @returns err(cookie) the errored cookie + */ +export const assertMyInfoCookieSuccessState = ( + cookie: MyInfoCookiePayload, +): Result => + cookie.state === MyInfoCookieState.Success + ? ok(cookie) + : err(new MyInfoCookieStateError()) + +/** + * Extracts and asserts a successful myInfoCookie from a request's cookies + * @param cookies Cookies in a request + * @return ok(cookie) the successful myInfoCookie + * @return err(MyInfoMissingAccessTokenError) if myInfoCookie is not present on the request + * @return err(MyInfoCookieStateError) if the extracted myInfoCookie was in an error state + * @return err(MyInfoCookieAccessError) if the cookie has been accessed before + */ +export const extractAndAssertMyInfoCookieValidity = ( + cookies: Record, +): Result< + MyInfoSuccessfulCookiePayload, + | MyInfoCookieStateError + | MyInfoMissingAccessTokenError + | MyInfoCookieAccessError +> => + extractMyInfoCookie(cookies) + .andThen((cookiePayload) => assertMyInfoCookieSuccessState(cookiePayload)) + .andThen((cookiePayload) => assertMyInfoCookieUnused(cookiePayload)) + +/** + * Asserts that the myInfoCookie has not been used before + * @param myInfoCookie + * @returns ok(myInfoCookie) if the cookie has not been used before + * @returns err(MyInfoCookieAccessError) if the cookie has been used before + */ +export const assertMyInfoCookieUnused = ( + myInfoCookie: MyInfoSuccessfulCookiePayload, +): Result => + myInfoCookie.usedCount <= 0 + ? ok(myInfoCookie) + : err(new MyInfoCookieAccessError()) + /** * Extracts access token from a MyInfo cookie * @param cookie Cookie from which access token should be extracted diff --git a/src/app/modules/spcp/__tests__/spcp.service.spec.ts b/src/app/modules/spcp/__tests__/spcp.service.spec.ts index 3a96990ed0..43746bbcf4 100644 --- a/src/app/modules/spcp/__tests__/spcp.service.spec.ts +++ b/src/app/modules/spcp/__tests__/spcp.service.spec.ts @@ -13,6 +13,7 @@ import dbHandler from 'tests/unit/backend/helpers/jest-db' import { CreateRedirectUrlError, FetchLoginPageError, + InvalidJwtError, InvalidOOBParamsError, LoginPageValidationError, MissingAttributesError, @@ -226,7 +227,7 @@ describe('spcp.service', () => { expect(result._unsafeUnwrap()).toEqual(MOCK_SP_JWT_PAYLOAD) }) - it('should return VerifyJwtError when SingPass JWT is invalid', async () => { + it('should return VerifyJwtError when SingPass JWT could not be verified', async () => { const spcpService = new SpcpService(MOCK_PARAMS) // Assumes that SP auth client was instantiated first const mockClient = mocked(MockAuthClient.mock.instances[0], true) @@ -237,6 +238,21 @@ describe('spcp.service', () => { expect(result._unsafeUnwrapErr()).toEqual(new VerifyJwtError()) }) + it('should return InvalidJwtError when SP JWT has invalid shape', async () => { + // Arrange + const spcpService = new SpcpService(MOCK_PARAMS) + // Assumes that SP auth client was instantiated first + const mockClient = mocked(MockAuthClient.mock.instances[0], true) + mockClient.verifyJWT.mockImplementationOnce((jwt, cb) => cb(null, {})) + const expected = new InvalidJwtError() + + // Act + const result = await spcpService.extractJwtPayload(MOCK_JWT, AuthType.SP) + + // Assert + expect(result._unsafeUnwrapErr()).toEqual(expected) + }) + 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 @@ -248,7 +264,7 @@ describe('spcp.service', () => { expect(result._unsafeUnwrap()).toEqual(MOCK_CP_JWT_PAYLOAD) }) - it('should return VerifyJwtError when CorpPass JWT is invalid', async () => { + it('should return VerifyJwtError when CorpPass JWT could not be verified', async () => { const spcpService = new SpcpService(MOCK_PARAMS) // Assumes that SP auth client was instantiated first const mockClient = mocked(MockAuthClient.mock.instances[1], true) @@ -258,6 +274,21 @@ describe('spcp.service', () => { const result = await spcpService.extractJwtPayload(MOCK_JWT, AuthType.CP) expect(result._unsafeUnwrapErr()).toEqual(new VerifyJwtError()) }) + + it('should return InvalidJwtError when CorpPass JWT has invalid shape', async () => { + // Arrange + const spcpService = new SpcpService(MOCK_PARAMS) + // Assumes that SP auth client was instantiated first + const mockClient = mocked(MockAuthClient.mock.instances[1], true) + mockClient.verifyJWT.mockImplementationOnce((jwt, cb) => cb(null, {})) + const expected = new InvalidJwtError() + + // Act + const result = await spcpService.extractJwtPayload(MOCK_JWT, AuthType.CP) + + // Assert + expect(result._unsafeUnwrapErr()).toEqual(expected) + }) }) describe('parseOOBParams', () => { @@ -768,4 +799,149 @@ describe('spcp.service', () => { expect(result._unsafeUnwrapErr()).toEqual(new MissingJwtError()) }) }) + + describe('extractJwtPayloadFromRequest', () => { + it('should return a SP JWT payload when there is a valid JWT in the request', async () => { + // Arrange + const spcpService = new SpcpService(MOCK_PARAMS) + // Assumes that SP auth client was instantiated first + const mockClient = mocked(MockAuthClient.mock.instances[0], true) + mockClient.verifyJWT.mockImplementationOnce((jwt, cb) => + cb(null, MOCK_SP_JWT_PAYLOAD), + ) + + // Act + const result = await spcpService.extractJwtPayloadFromRequest( + AuthType.SP, + MOCK_COOKIES, + ) + + // Assert + expect(result._unsafeUnwrap()).toEqual(MOCK_SP_JWT_PAYLOAD) + }) + + it('should return a CP JWT payload when there is a valid JWT in the request', async () => { + // Arrange + const spcpService = new SpcpService(MOCK_PARAMS) + // Assumes that CP auth client was instantiated first + const mockClient = mocked(MockAuthClient.mock.instances[1], true) + mockClient.verifyJWT.mockImplementationOnce((jwt, cb) => + cb(null, MOCK_CP_JWT_PAYLOAD), + ) + + // Act + const result = await spcpService.extractJwtPayloadFromRequest( + AuthType.CP, + MOCK_COOKIES, + ) + + // Assert + expect(result._unsafeUnwrap()).toEqual(MOCK_CP_JWT_PAYLOAD) + }) + + it('should return MissingJwtError if there is no JWT when client authenticates using SP', async () => { + // Arrange + const spcpService = new SpcpService(MOCK_PARAMS) + const expected = new MissingJwtError() + + // Act + const result = await spcpService.extractJwtPayloadFromRequest( + AuthType.SP, + {}, + ) + + // Assert + expect(result._unsafeUnwrapErr()).toEqual(expected) + }) + + it('should return MissingJwtError when client authenticates using CP and there is no JWT', async () => { + // Arrange + const spcpService = new SpcpService(MOCK_PARAMS) + const expected = new MissingJwtError() + + // Act + const result = await spcpService.extractJwtPayloadFromRequest( + AuthType.CP, + {}, + ) + + // Assert + expect(result._unsafeUnwrapErr()).toEqual(expected) + }) + + it('should return InvalidJwtError when the client authenticates using SP and the JWT has wrong shape', async () => { + // Arrange + const spcpService = new SpcpService(MOCK_PARAMS) + // Assumes that SP auth client was instantiated first + const mockClient = mocked(MockAuthClient.mock.instances[0], true) + mockClient.verifyJWT.mockImplementationOnce((jwt, cb) => + cb(new Error(), null), + ) + const expected = new VerifyJwtError() + + // Act + const result = await spcpService.extractJwtPayloadFromRequest( + AuthType.SP, + MOCK_COOKIES, + ) + + // Assert + expect(result._unsafeUnwrapErr()).toEqual(expected) + }) + + it('should return VerifyJwtError when the client authenticates using CP and the JWT has wrong shape', async () => { + // Arrange + const spcpService = new SpcpService(MOCK_PARAMS) + // Assumes that SP auth client was instantiated first + const mockClient = mocked(MockAuthClient.mock.instances[1], true) + mockClient.verifyJWT.mockImplementationOnce((jwt, cb) => + cb(new Error(), null), + ) + const expected = new VerifyJwtError() + + // Act + const result = await spcpService.extractJwtPayloadFromRequest( + AuthType.CP, + MOCK_COOKIES, + ) + + // Assert + expect(result._unsafeUnwrapErr()).toEqual(expected) + }) + it('should return InvalidJwtError when the client authenticates using SP and the JWT has invalid shape', async () => { + // Arrange + const spcpService = new SpcpService(MOCK_PARAMS) + // Assumes that SP auth client was instantiated first + const mockClient = mocked(MockAuthClient.mock.instances[0], true) + mockClient.verifyJWT.mockImplementationOnce((jwt, cb) => cb(null, {})) + const expected = new InvalidJwtError() + + // Act + const result = await spcpService.extractJwtPayloadFromRequest( + AuthType.SP, + MOCK_COOKIES, + ) + + // Assert + expect(result._unsafeUnwrapErr()).toEqual(expected) + }) + + it('should return InvalidJwtError when the client authenticates using CP and the JWT has invalid shape', async () => { + // Arrange + const spcpService = new SpcpService(MOCK_PARAMS) + // Assumes that SP auth client was instantiated first + const mockClient = mocked(MockAuthClient.mock.instances[1], true) + mockClient.verifyJWT.mockImplementationOnce((jwt, cb) => cb(null, {})) + const expected = new InvalidJwtError() + + // Act + const result = await spcpService.extractJwtPayloadFromRequest( + AuthType.CP, + MOCK_COOKIES, + ) + + // Assert + expect(result._unsafeUnwrapErr()).toEqual(expected) + }) + }) }) diff --git a/src/app/modules/spcp/__tests__/spcp.test.constants.ts b/src/app/modules/spcp/__tests__/spcp.test.constants.ts index 6a059fc214..2a036f4a12 100644 --- a/src/app/modules/spcp/__tests__/spcp.test.constants.ts +++ b/src/app/modules/spcp/__tests__/spcp.test.constants.ts @@ -1,6 +1,7 @@ import { MyInfoMode } from '@opengovsg/myinfo-gov-client' import { ObjectId } from 'bson' import crypto from 'crypto' +import _ from 'lodash' import { ISpcpMyInfo } from 'src/config/feature-manager' import { ILoginSchema, IPopulatedForm } from 'src/types' @@ -114,6 +115,7 @@ export const MOCK_SP_FORM = ({ _id: new ObjectId().toHexString(), agency: new ObjectId().toHexString(), }, + getPublicView: () => _.omit(this, 'admin'), } as unknown) as IPopulatedForm export const MOCK_CP_FORM = ({ @@ -124,6 +126,17 @@ export const MOCK_CP_FORM = ({ _id: new ObjectId().toHexString(), agency: new ObjectId().toHexString(), }, + getPublicView: () => _.omit(this, 'admin'), +} as unknown) as IPopulatedForm + +export const MOCK_MYINFO_FORM = ({ + authType: 'MyInfo', + title: 'Mock MyInfo form', + _id: new ObjectId().toHexString(), + admin: { + _id: new ObjectId().toHexString(), + agency: new ObjectId().toHexString(), + }, } as unknown) as IPopulatedForm export const MOCK_LOGIN_DOC = { diff --git a/src/app/modules/spcp/spcp.factory.ts b/src/app/modules/spcp/spcp.factory.ts index d7db1f322d..69cc2c031c 100644 --- a/src/app/modules/spcp/spcp.factory.ts +++ b/src/app/modules/spcp/spcp.factory.ts @@ -19,6 +19,7 @@ interface ISpcpFactory { createJWT: SpcpService['createJWT'] createJWTPayload: SpcpService['createJWTPayload'] getCookieSettings: SpcpService['getCookieSettings'] + extractJwtPayloadFromRequest: SpcpService['extractJwtPayloadFromRequest'] } export const createSpcpFactory = ({ @@ -38,6 +39,7 @@ export const createSpcpFactory = ({ createJWT: () => err(error), createJWTPayload: () => err(error), getCookieSettings: () => ({}), + extractJwtPayloadFromRequest: () => errAsync(error), } } return new SpcpService(props) diff --git a/src/app/modules/spcp/spcp.service.ts b/src/app/modules/spcp/spcp.service.ts index aae6ae8b9e..74911cdbda 100644 --- a/src/app/modules/spcp/spcp.service.ts +++ b/src/app/modules/spcp/spcp.service.ts @@ -201,10 +201,7 @@ export class SpcpService { ): Result { const jwtName = authType === AuthType.SP ? JwtName.SP : JwtName.CP const cookie = cookies[jwtName] - if (!cookie) { - return err(new MissingJwtError()) - } - return ok(cookie) + return cookie ? ok(cookie) : err(new MissingJwtError()) } /** @@ -394,4 +391,25 @@ export class SpcpService { const spcpCookieDomain = this.#spcpProps.spcpCookieDomain return spcpCookieDomain ? { domain: spcpCookieDomain, path: '/' } : {} } + + /** + * Gets the spcp session info from the auth, cookies + * @param authType The authentication type of the user + * @param cookies The spcp cookies set by the redirect + * @return ok(jwtPayload) if successful + * @return err(MissingJwtError) if the specified cookie for the authType (spcp) does not exist + * @return err(VerifyJwtError) if the jwt exists but could not be authenticated + * @return err(InvalidJwtError) if the jwt exists but the payload is invalid + */ + extractJwtPayloadFromRequest( + authType: AuthType.SP | AuthType.CP, + cookies: SpcpCookies, + ): ResultAsync< + JwtPayload, + VerifyJwtError | InvalidJwtError | MissingJwtError + > { + return this.extractJwt(cookies, authType).asyncAndThen((jwtResult) => + this.extractJwtPayload(jwtResult, authType), + ) + } } diff --git a/src/app/routes/public-forms.server.routes.js b/src/app/routes/public-forms.server.routes.js index 2e4bf9cf81..b3ed749508 100644 --- a/src/app/routes/public-forms.server.routes.js +++ b/src/app/routes/public-forms.server.routes.js @@ -4,14 +4,11 @@ * Module dependencies. */ const forms = require('../../app/controllers/forms.server.controller') -const publicForms = require('../modules/form/public-form/public-form.middlewares') -const MyInfoMiddleware = require('../modules/myinfo/myinfo.middleware') const { celebrate, Joi, Segments } = require('celebrate') const { CaptchaFactory } = require('../services/captcha/captcha.factory') const { limitRate } = require('../utils/limit-rate') const { rateLimitConfig } = require('../../config/config') const PublicFormController = require('../modules/form/public-form/public-form.controller') -const SpcpController = require('../modules/spcp/spcp.controller') const EncryptSubmissionController = require('../modules/submission/encrypt-submission/encrypt-submission.controller') const EncryptSubmissionMiddleware = require('../modules/submission/encrypt-submission/encrypt-submission.middleware') @@ -127,14 +124,7 @@ module.exports = function (app) { */ app .route('/:formId([a-fA-F0-9]{24})/publicform') - .get( - forms.formById, - publicForms.isFormPublicCheck, - publicForms.checkFormSubmissionLimitAndDeactivate, - SpcpController.addSpcpSessionInfo, - MyInfoMiddleware.addMyInfo, - forms.read(forms.REQUEST_TYPE.PUBLIC), - ) + .get(PublicFormController.handleGetPublicForm) /** * On preview, submit a form response, and stores the encrypted contents. Optionally, an autoreply diff --git a/src/app/services/intranet/__tests__/intranet.service.spec.ts b/src/app/services/intranet/__tests__/intranet.service.spec.ts index 654256d133..ed4db49f4c 100644 --- a/src/app/services/intranet/__tests__/intranet.service.spec.ts +++ b/src/app/services/intranet/__tests__/intranet.service.spec.ts @@ -31,7 +31,7 @@ describe('IntranetService', () => { it('should return true when IP is in intranet IP list', () => { const result = intranetService.isIntranetIp(MOCK_IP_LIST[0]) - expect(result._unsafeUnwrap()).toBe(true) + expect(result).toBe(true) }) it('should return false when IP is not in intranet IP list', () => { @@ -39,7 +39,7 @@ describe('IntranetService', () => { const result = intranetService.isIntranetIp(ipNotInList) - expect(result._unsafeUnwrap()).toBe(false) + expect(result).toBe(false) }) }) }) diff --git a/src/app/services/intranet/intranet.factory.ts b/src/app/services/intranet/intranet.factory.ts index 6b15b317a6..32860b2eea 100644 --- a/src/app/services/intranet/intranet.factory.ts +++ b/src/app/services/intranet/intranet.factory.ts @@ -1,4 +1,4 @@ -import { err } from 'neverthrow' +import { err, ok, Result } from 'neverthrow' import FeatureManager, { FeatureNames, @@ -9,7 +9,7 @@ import { MissingFeatureError } from '../../modules/core/core.errors' import { IntranetService } from './intranet.service' interface IIntranetFactory { - isIntranetIp: IntranetService['isIntranetIp'] + isIntranetIp: (ip: string) => Result } export const createIntranetFactory = ({ @@ -17,7 +17,10 @@ export const createIntranetFactory = ({ props, }: RegisteredFeature): IIntranetFactory => { if (isEnabled && props?.intranetIpListPath) { - return new IntranetService(props) + const intranetService = new IntranetService(props) + return { + isIntranetIp: (ip: string) => ok(intranetService.isIntranetIp(ip)), + } } const error = new MissingFeatureError(FeatureNames.Intranet) diff --git a/src/app/services/intranet/intranet.service.ts b/src/app/services/intranet/intranet.service.ts index 56c1f00b97..91d957231b 100644 --- a/src/app/services/intranet/intranet.service.ts +++ b/src/app/services/intranet/intranet.service.ts @@ -1,9 +1,7 @@ import fs from 'fs' -import { ok, Result } from 'neverthrow' import { IIntranet } from '../../../config/feature-manager' import { createLoggerWithLabel } from '../../../config/logger' -import { ApplicationError } from '../../modules/core/core.errors' const logger = createLoggerWithLabel(module) @@ -42,7 +40,7 @@ export class IntranetService { * @param ip IP address to check * @returns Whether the IP address originated from the intranet */ - isIntranetIp(ip: string): Result { - return ok(this.intranetIps.includes(ip)) + isIntranetIp(ip: string): boolean { + return this.intranetIps.includes(ip) } } diff --git a/src/app/utils/handle-mongo-error.ts b/src/app/utils/handle-mongo-error.ts index cf3033402c..2ca25fcf43 100644 --- a/src/app/utils/handle-mongo-error.ts +++ b/src/app/utils/handle-mongo-error.ts @@ -6,6 +6,7 @@ import { DatabaseError, DatabasePayloadSizeError, DatabaseValidationError, + PossibleDatabaseError, } from '../modules/core/core.errors' /** @@ -66,13 +67,7 @@ export const getMongoErrorMessage = ( * @param error the error thrown by database operations * @returns errors that extend from ApplicationError class */ -export const transformMongoError = ( - error: unknown, -): - | DatabaseError - | DatabaseValidationError - | DatabaseConflictError - | DatabasePayloadSizeError => { +export const transformMongoError = (error: unknown): PossibleDatabaseError => { const errorMessage = getMongoErrorMessage(error) if (!(error instanceof Error)) { return new DatabaseError(errorMessage) @@ -99,3 +94,15 @@ export const transformMongoError = ( return new DatabaseError(errorMessage) } + +export const isMongoError = (error: Error): boolean => { + switch (error.constructor) { + case DatabaseConflictError: + case DatabaseError: + case DatabasePayloadSizeError: + case DatabaseValidationError: + return true + default: + return false + } +} diff --git a/src/types/form.ts b/src/types/form.ts index c2928fb977..e8964e443a 100644 --- a/src/types/form.ts +++ b/src/types/form.ts @@ -129,7 +129,7 @@ export interface IForm { status?: Status inactiveMessage?: string - submissionLimit?: number + submissionLimit?: number | null isListed?: boolean esrvcId?: string webhook?: Webhook @@ -208,7 +208,9 @@ export interface IFormDocument extends IFormSchema { authType: NonNullable status: NonNullable inactiveMessage: NonNullable - submissionLimit: NonNullable + // NOTE: Due to the way creating a form works, creating a form without specifying submissionLimit will throw an error. + // Hence, using Exclude here over NonNullable. + submissionLimit: Exclude isListed: NonNullable form_fields: NonNullable startPage: SetRequired, 'colorTheme'>