diff --git a/src/app/modules/auth/auth.controller.ts b/src/app/modules/auth/auth.controller.ts index 8eae149063..d649f9a95b 100644 --- a/src/app/modules/auth/auth.controller.ts +++ b/src/app/modules/auth/auth.controller.ts @@ -105,7 +105,7 @@ export const handleLoginSendOtp: RequestHandler< } /** - * Handler for POST /auth/verifyotp endpoint. + * Handler for POST /auth/verifyotp endpoint. * @returns 200 when user has successfully logged in, with session cookie set * @returns 401 when the email domain is invalid * @returns 422 when the OTP is invalid diff --git a/src/app/modules/form/form.errors.ts b/src/app/modules/form/form.errors.ts index 50dacb5589..38b9382d6e 100644 --- a/src/app/modules/form/form.errors.ts +++ b/src/app/modules/form/form.errors.ts @@ -1,3 +1,5 @@ +import { AuthType } from 'src/types' + import { ApplicationError } from '../core/core.errors' export class FormNotFoundError extends ApplicationError { @@ -60,3 +62,26 @@ export class LogicNotFoundError extends ApplicationError { super(message) } } + +/** + * Error to be returned when the form's auth type does not match what is required + */ +export class AuthTypeMismatchError extends ApplicationError { + constructor(attemptedAuthType: AuthType, formAuthType?: AuthType) { + super( + `Attempted authentication type ${attemptedAuthType} did not match form auth type ${formAuthType}`, + ) + } +} + +/** + * Error to be returned when the form has authentication enabled but is lacking an eServiceId + */ + +export class FormAuthNoEsrvcIdError extends ApplicationError { + constructor(formId: string) { + super( + `Attempted to validate form ${formId} whhich did not have an eServiceId`, + ) + } +} 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 2f164208af..66310c60ac 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 @@ -14,10 +14,8 @@ import { } 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' @@ -37,6 +35,8 @@ import { MyInfoFactory } from '../../../myinfo/myinfo.factory' import { MissingJwtError } from '../../../spcp/spcp.errors' import { SpcpFactory } from '../../../spcp/spcp.factory' import { + AuthTypeMismatchError, + FormAuthNoEsrvcIdError, FormDeletedError, FormNotFoundError, PrivateFormError, @@ -731,7 +731,7 @@ describe('public-form.controller', () => { }) MockMyInfoFactory.getMyInfoDataForForm.mockReturnValueOnce( - errAsync(new MyInfoAuthTypeError()), + errAsync(new AuthTypeMismatchError()), ) // Act @@ -758,7 +758,7 @@ describe('public-form.controller', () => { }) MockMyInfoFactory.getMyInfoDataForForm.mockReturnValueOnce( - errAsync(new MyInfoNoESrvcIdError()), + errAsync(new FormAuthNoEsrvcIdError()), ) // Act 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 5b3d4b762a..aa1542b1b3 100644 --- a/src/app/modules/form/public-form/public-form.controller.ts +++ b/src/app/modules/form/public-form/public-form.controller.ts @@ -1,11 +1,17 @@ import { celebrate, Joi, Segments } from 'celebrate' import { RequestHandler } from 'express' +import { Query } from 'express-serve-static-core' import { StatusCodes } from 'http-status-codes' +import { err } from 'neverthrow' import querystring from 'querystring' import { UnreachableCaseError } from 'ts-essentials' import { AuthType } from '../../../../types' -import { ErrorDto, PrivateFormErrorDto } from '../../../../types/api' +import { + ErrorDto, + PrivateFormErrorDto, + PublicFormAuthRedirectDto, +} from '../../../../types/api' import { createLoggerWithLabel } from '../../../config/logger' import { isMongoError } from '../../../utils/handle-mongo-error' import { createReqMeta, getRequestIp } from '../../../utils/request' @@ -19,15 +25,19 @@ import { MyInfoMissingAccessTokenError, } from '../../myinfo/myinfo.errors' import { MyInfoFactory } from '../../myinfo/myinfo.factory' -import { extractAndAssertMyInfoCookieValidity } from '../../myinfo/myinfo.util' +import { + extractAndAssertMyInfoCookieValidity, + validateMyInfoForm, +} from '../../myinfo/myinfo.util' import { InvalidJwtError, VerifyJwtError } from '../../spcp/spcp.errors' import { SpcpFactory } from '../../spcp/spcp.factory' -import { PrivateFormError } from '../form.errors' +import { getRedirectTarget, validateSpcpForm } from '../../spcp/spcp.util' +import { AuthTypeMismatchError, PrivateFormError } from '../form.errors' import * as FormService from '../form.service' import * as PublicFormService from './public-form.service' import { PublicFormViewDto, RedirectParams } from './public-form.types' -import { mapRouteError } from './public-form.utils' +import { mapFormAuthRedirectError, mapRouteError } from './public-form.utils' const logger = createLoggerWithLabel(module) @@ -350,3 +360,94 @@ export const handleGetPublicForm: RequestHandler< return new UnreachableCaseError(authType) } } + +/** + * NOTE: This is exported only for testing + * Generates redirect URL to Official SingPass/CorpPass log in page + * @param isPersistentLogin whether the client wants to have their login information stored + * @returns 200 with the redirect url when the user authenticates successfully + * @returns 400 when there is an error on the authType of the form + * @returns 400 when the eServiceId of the form does not exist + * @returns 404 when form with given ID does not exist + * @returns 500 when database error occurs + * @returns 500 when the redirect url could not be created + * @returns 500 when the redirect feature is not enabled + */ +export const _handleFormAuthRedirect: RequestHandler< + { formId: string }, + PublicFormAuthRedirectDto | ErrorDto, + unknown, + Query & { isPersistentLogin?: boolean } +> = (req, res) => { + const { formId } = req.params + const { isPersistentLogin } = req.query + const logMeta = { + action: 'handleFormAuthRedirect', + ...createReqMeta(req), + formId, + } + // NOTE: Using retrieveFullForm instead of retrieveForm to ensure authType always exists + return FormService.retrieveFullFormById(formId) + .andThen((form) => { + switch (form.authType) { + case AuthType.MyInfo: + return validateMyInfoForm(form).andThen((form) => + MyInfoFactory.createRedirectURL({ + formEsrvcId: form.esrvcId, + formId, + requestedAttributes: form.getUniqueMyInfoAttrs(), + }), + ) + case AuthType.SP: + case AuthType.CP: { + // NOTE: Persistent login is only set (and relevant) when the authType is SP. + // If authType is not SP, assume that it was set erroneously and default it to false + return validateSpcpForm(form).andThen((form) => { + const target = getRedirectTarget( + formId, + form.authType, + isPersistentLogin, + ) + return SpcpFactory.createRedirectUrl( + form.authType, + target, + form.esrvcId, + ) + }) + } + // NOTE: Only MyInfo and SPCP should have redirects as the point of a redirect is + // to provide auth for users from a third party + default: + return err( + new AuthTypeMismatchError(form.authType), + ) + } + }) + .map((redirectURL) => { + return res.status(StatusCodes.OK).json({ redirectURL }) + }) + .mapErr((error) => { + logger.error({ + message: 'Error while creating redirect URL', + meta: logMeta, + error, + }) + const { statusCode, errorMessage } = mapFormAuthRedirectError(error) + return res.status(statusCode).json({ message: errorMessage }) + }) +} + +/** + * Handler for /forms/:formId/auth/redirect + */ +export const handleFormAuthRedirect = [ + celebrate({ + [Segments.PARAMS]: Joi.object({ + formId: Joi.string().pattern(/^[a-fA-F0-9]{24}$/), + }), + [Segments.QUERY]: Joi.object({ + isPersistentLogin: Joi.boolean().optional(), + }), + }), + _handleFormAuthRedirect, +] as RequestHandler[] diff --git a/src/app/modules/form/public-form/public-form.utils.ts b/src/app/modules/form/public-form/public-form.utils.ts index bf7b73f574..e52d8ab9cc 100644 --- a/src/app/modules/form/public-form/public-form.utils.ts +++ b/src/app/modules/form/public-form/public-form.utils.ts @@ -1,8 +1,15 @@ import { getReasonPhrase, StatusCodes } from 'http-status-codes' +import { MapRouteError } from 'src/types' + import { createLoggerWithLabel } from '../../../config/logger' -import { ApplicationError, DatabaseError } from '../../core/core.errors' +import { + ApplicationError, + DatabaseError, + MissingFeatureError, +} from '../../core/core.errors' import { ErrorResponseData } from '../../core/core.types' +import { CreateRedirectUrlError } from '../../spcp/spcp.errors' import * as FormErrors from '../form.errors' const logger = createLoggerWithLabel(module) @@ -53,3 +60,53 @@ export const mapRouteError = ( } } } + +/** + * Handler to map ApplicationErrors from redirecting to SPCP login page to HTTP errors + * @param error The error to retrieve the status codes and error messages + * @param coreErrorMessage Any error message to return instead of the default core error message, if any + */ +export const mapFormAuthRedirectError: MapRouteError = ( + error, + coreErrorMessage = 'Sorry, something went wrong. Please try again.', +) => { + switch (error.constructor) { + case FormErrors.FormNotFoundError: + return { + statusCode: StatusCodes.NOT_FOUND, + errorMessage: + 'Could not find the form requested. Please refresh and try again.', + } + case FormErrors.FormAuthNoEsrvcIdError: + return { + statusCode: StatusCodes.BAD_REQUEST, + errorMessage: + 'This form does not have a valid eServiceId. Please refresh and try again.', + } + case FormErrors.AuthTypeMismatchError: + return { + statusCode: StatusCodes.BAD_REQUEST, + errorMessage: + 'Please ensure that the form has authentication enabled. Please refresh and try again.', + } + case DatabaseError: + case CreateRedirectUrlError: + case MissingFeatureError: + return { + statusCode: StatusCodes.INTERNAL_SERVER_ERROR, + errorMessage: coreErrorMessage, + } + default: + logger.error({ + message: 'Unknown route error observed', + meta: { + action: 'mapRouteError', + }, + error, + }) + return { + statusCode: StatusCodes.INTERNAL_SERVER_ERROR, + errorMessage: 'Sorry, something went wrong. Please try again.', + } + } +} diff --git a/src/app/modules/myinfo/__tests__/myinfo.routes.spec.ts b/src/app/modules/myinfo/__tests__/myinfo.routes.spec.ts index ed607a5f9e..94dcf6eba3 100644 --- a/src/app/modules/myinfo/__tests__/myinfo.routes.spec.ts +++ b/src/app/modules/myinfo/__tests__/myinfo.routes.spec.ts @@ -1,4 +1,4 @@ -import { MyInfoGovClient } from '@opengovsg/myinfo-gov-client' +import MyInfoClient, { IMyInfoConfig } from '@opengovsg/myinfo-gov-client' import SPCPAuthClient from '@opengovsg/spcp-auth-client' import axios from 'axios' import { ObjectId } from 'bson' @@ -30,7 +30,10 @@ jest.mock('@opengovsg/spcp-auth-client') const MockAuthClient = mocked(SPCPAuthClient, true) jest.mock('@opengovsg/myinfo-gov-client', () => ({ - MyInfoGovClient: jest.fn(), + MyInfoGovClient: jest.fn().mockReturnValue({ + createRedirectURL: jest.fn(), + getAccessToken: jest.fn(), + }), MyInfoMode: jest.requireActual('@opengovsg/myinfo-gov-client').MyInfoMode, MyInfoSource: jest.requireActual('@opengovsg/myinfo-gov-client').MyInfoSource, MyInfoAddressType: jest.requireActual('@opengovsg/myinfo-gov-client') @@ -38,15 +41,9 @@ jest.mock('@opengovsg/myinfo-gov-client', () => ({ MyInfoAttribute: jest.requireActual('@opengovsg/myinfo-gov-client') .MyInfoAttribute, })) -const MockMyInfoGovClient = mocked(MyInfoGovClient, true) -const mockCreateRedirectURL = jest.fn() -const mockGetAccessToken = jest.fn() -MockMyInfoGovClient.mockImplementation( - () => - (({ - createRedirectURL: mockCreateRedirectURL, - getAccessToken: mockGetAccessToken, - } as unknown) as MyInfoGovClient), +const MockMyInfoGovClient = mocked( + new MyInfoClient.MyInfoGovClient({} as IMyInfoConfig), + true, ) // Import last so that mocks are imported correctly @@ -68,7 +65,7 @@ describe('myinfo.routes', () => { let request: Session beforeAll(() => { - mockCreateRedirectURL.mockReturnValue(MOCK_REDIRECT_URL) + MockMyInfoGovClient.createRedirectURL.mockReturnValue(MOCK_REDIRECT_URL) }) beforeEach(async () => { @@ -218,7 +215,9 @@ describe('myinfo.routes', () => { beforeEach(async () => { request = session(myInfoApp) - mockGetAccessToken.mockResolvedValueOnce(MOCK_ACCESS_TOKEN) + MockMyInfoGovClient.getAccessToken.mockResolvedValueOnce( + MOCK_ACCESS_TOKEN, + ) await dbHandler.insertEmailForm({ formId: new ObjectId(MOCK_FORM_ID), formOptions: { @@ -346,8 +345,8 @@ describe('myinfo.routes', () => { it('should redirect to form with error cookie when access token cannot be retrieved', async () => { // Clear default mock implementation from beforeEach - mockGetAccessToken.mockReset() - mockGetAccessToken.mockRejectedValueOnce('rejected') + MockMyInfoGovClient.getAccessToken.mockReset() + MockMyInfoGovClient.getAccessToken.mockRejectedValueOnce('rejected') const response = await request .get(ROUTE) diff --git a/src/app/modules/myinfo/myinfo.errors.ts b/src/app/modules/myinfo/myinfo.errors.ts index f149653498..1f1cea6054 100644 --- a/src/app/modules/myinfo/myinfo.errors.ts +++ b/src/app/modules/myinfo/myinfo.errors.ts @@ -56,27 +56,6 @@ export class MyInfoParseRelayStateError extends ApplicationError { } } -/** - * Attempt to perform a MyInfo-related operation on a form without MyInfo - * authentication enabled. - */ -export class MyInfoAuthTypeError extends ApplicationError { - constructor( - message = 'MyInfo function called on form without MyInfo authentication type', - ) { - super(message) - } -} - -/** - * MyInfo form missing e-service ID. - */ -export class MyInfoNoESrvcIdError extends ApplicationError { - constructor(message = 'Form does not have e-service ID') { - super(message) - } -} - /** * Submission on MyInfo form missing access token. */ diff --git a/src/app/modules/myinfo/myinfo.factory.ts b/src/app/modules/myinfo/myinfo.factory.ts index 479e4113e9..d4e66be084 100644 --- a/src/app/modules/myinfo/myinfo.factory.ts +++ b/src/app/modules/myinfo/myinfo.factory.ts @@ -13,11 +13,14 @@ import FeatureManager, { RegisteredFeature, } from '../../config/feature-manager' import { DatabaseError, MissingFeatureError } from '../core/core.errors' +import { + AuthTypeMismatchError, + FormAuthNoEsrvcIdError, +} from '../form/form.errors' import { ProcessedFieldResponse } from '../submission/submission.types' import { MyInfoData } from './myinfo.adapter' import { - MyInfoAuthTypeError, MyInfoCircuitBreakerError, MyInfoCookieAccessError, MyInfoCookieStateError, @@ -27,7 +30,6 @@ import { MyInfoInvalidAccessTokenError, MyInfoMissingAccessTokenError, MyInfoMissingHashError, - MyInfoNoESrvcIdError, MyInfoParseRelayStateError, } from './myinfo.errors' import { MyInfoService } from './myinfo.service' @@ -91,8 +93,8 @@ interface IMyInfoFactory { MyInfoData, | MyInfoMissingAccessTokenError | MyInfoCookieStateError - | MyInfoNoESrvcIdError - | MyInfoAuthTypeError + | FormAuthNoEsrvcIdError + | AuthTypeMismatchError | MyInfoCircuitBreakerError | MyInfoFetchError | MissingFeatureError diff --git a/src/app/modules/myinfo/myinfo.routes.ts b/src/app/modules/myinfo/myinfo.routes.ts index e147206e70..3420c6b21d 100644 --- a/src/app/modules/myinfo/myinfo.routes.ts +++ b/src/app/modules/myinfo/myinfo.routes.ts @@ -12,6 +12,7 @@ export const MyInfoRouter = Router() /** * Serves requests to supply a redirect URL to log in to * MyInfo. + * @deprecated in favour of GET /api/v3/forms/:formId/auth/redirect */ MyInfoRouter.get('/redirect', handleRedirectURLRequest) diff --git a/src/app/modules/myinfo/myinfo.service.ts b/src/app/modules/myinfo/myinfo.service.ts index 020aeb4543..dae865966a 100644 --- a/src/app/modules/myinfo/myinfo.service.ts +++ b/src/app/modules/myinfo/myinfo.service.ts @@ -20,6 +20,10 @@ import { } from '../../../types' import { createLoggerWithLabel } from '../../config/logger' import { DatabaseError, MissingFeatureError } from '../core/core.errors' +import { + AuthTypeMismatchError, + FormAuthNoEsrvcIdError, +} from '../form/form.errors' import { ProcessedFieldResponse } from '../submission/submission.types' import { internalAttrListToScopes, MyInfoData } from './myinfo.adapter' @@ -29,7 +33,6 @@ import { MYINFO_ROUTER_PREFIX, } from './myinfo.constants' import { - MyInfoAuthTypeError, MyInfoCircuitBreakerError, MyInfoCookieAccessError, MyInfoCookieStateError, @@ -39,7 +42,6 @@ import { MyInfoInvalidAccessTokenError, MyInfoMissingAccessTokenError, MyInfoMissingHashError, - MyInfoNoESrvcIdError, MyInfoParseRelayStateError, } from './myinfo.errors' import { @@ -484,8 +486,8 @@ export class MyInfoService { * @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(FormAuthNoEsrvcIdError) if form has no eserviceId + * @returns err(AuthTypeMismatchError) 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 @@ -497,8 +499,8 @@ export class MyInfoService { MyInfoData, | MyInfoMissingAccessTokenError | MyInfoCookieStateError - | MyInfoNoESrvcIdError - | MyInfoAuthTypeError + | FormAuthNoEsrvcIdError + | AuthTypeMismatchError | MyInfoCircuitBreakerError | MyInfoFetchError | MissingFeatureError diff --git a/src/app/modules/myinfo/myinfo.types.ts b/src/app/modules/myinfo/myinfo.types.ts index d472230e07..d28b697f7c 100644 --- a/src/app/modules/myinfo/myinfo.types.ts +++ b/src/app/modules/myinfo/myinfo.types.ts @@ -69,7 +69,7 @@ export type MyInfoParsedRelayState = MyInfoRelayState & { cookieDuration: number } -export interface IMyInfoForm extends IFormSchema { +export type MyInfoForm = T & { authType: AuthType.MyInfo esrvcId: string } diff --git a/src/app/modules/myinfo/myinfo.util.ts b/src/app/modules/myinfo/myinfo.util.ts index 87629fa864..ecb97642a0 100644 --- a/src/app/modules/myinfo/myinfo.util.ts +++ b/src/app/modules/myinfo/myinfo.util.ts @@ -12,13 +12,16 @@ import { IFormSchema, IHashes, IMyInfo, - IPopulatedForm, MapRouteError, } from '../../../types' import { createLoggerWithLabel } from '../../config/logger' import { hasProp } from '../../utils/has-prop' import { DatabaseError, MissingFeatureError } from '../core/core.errors' -import { FormNotFoundError } from '../form/form.errors' +import { + AuthTypeMismatchError, + FormAuthNoEsrvcIdError, + FormNotFoundError, +} from '../form/form.errors' import { CreateRedirectUrlError, FetchLoginPageError, @@ -28,21 +31,19 @@ import { ProcessedFieldResponse } from '../submission/submission.types' import { MYINFO_COOKIE_NAME } from './myinfo.constants' import { - MyInfoAuthTypeError, MyInfoCookieAccessError, MyInfoCookieStateError, MyInfoHashDidNotMatchError, MyInfoHashingError, MyInfoMissingAccessTokenError, MyInfoMissingHashError, - MyInfoNoESrvcIdError, } from './myinfo.errors' import { - IMyInfoForm, IPossiblyPrefilledField, MyInfoComparePromises, MyInfoCookiePayload, MyInfoCookieState, + MyInfoForm, MyInfoHashPromises, MyInfoRelayState, MyInfoSuccessfulCookiePayload, @@ -176,7 +177,7 @@ export const mapRedirectURLError: MapRouteError = ( errorMessage: 'Could not find the form requested. Please refresh and try again.', } - case MyInfoAuthTypeError: + case AuthTypeMismatchError: return { statusCode: StatusCodes.BAD_REQUEST, errorMessage: @@ -219,13 +220,13 @@ export const mapEServiceIdCheckError: MapRouteError = ( errorMessage: 'Could not find the form requested. Please refresh and try again.', } - case MyInfoAuthTypeError: + case AuthTypeMismatchError: return { statusCode: StatusCodes.BAD_REQUEST, errorMessage: 'This form does not have MyInfo enabled. Please refresh and try again.', } - case MyInfoNoESrvcIdError: + case FormAuthNoEsrvcIdError: return { statusCode: StatusCodes.FORBIDDEN, errorMessage: @@ -285,16 +286,23 @@ export const createRelayState = (formId: string): string => * Validates that a form is a MyInfo form with an e-service ID * @param form Form to validate */ -export const validateMyInfoForm = ( - form: IFormSchema | IPopulatedForm, -): Result => { +export const validateMyInfoForm = ( + form: T, +): Result, FormAuthNoEsrvcIdError | AuthTypeMismatchError> => { if (!form.esrvcId) { - return err(new MyInfoNoESrvcIdError()) + return err(new FormAuthNoEsrvcIdError(form._id)) } - if (form.authType !== AuthType.MyInfo) { - return err(new MyInfoAuthTypeError()) + if (isMyInfoFormWithEsrvcId(form)) { + return ok(form) } - return ok(form as IMyInfoForm) + return err(new AuthTypeMismatchError(AuthType.MyInfo, form.authType)) +} + +// Typeguard to ensure that form has eserviceId and MyInfo authType +const isMyInfoFormWithEsrvcId = ( + form: F, +): form is MyInfoForm => { + return form.authType === AuthType.MyInfo && !!form.esrvcId } /** diff --git a/src/app/modules/spcp/spcp.errors.ts b/src/app/modules/spcp/spcp.errors.ts index 7a523de4db..f905e97c57 100644 --- a/src/app/modules/spcp/spcp.errors.ts +++ b/src/app/modules/spcp/spcp.errors.ts @@ -1,4 +1,3 @@ -import { AuthType } from '../../../types' import { ApplicationError } from '../../modules/core/core.errors' /** @@ -55,17 +54,6 @@ export class RetrieveAttributesError extends ApplicationError { } } -/** - * Form auth type did not match attempted auth method. - */ -export class AuthTypeMismatchError extends ApplicationError { - constructor(attemptedAuthType: AuthType, formAuthType?: AuthType) { - super( - `Attempted authentication type ${attemptedAuthType} did not match form auth type ${formAuthType}`, - ) - } -} - /** * Attributes given by SP/CP did not contain NRIC or entity ID/UID. */ diff --git a/src/app/modules/spcp/spcp.routes.ts b/src/app/modules/spcp/spcp.routes.ts index af475cbfbb..2dc929ebc1 100644 --- a/src/app/modules/spcp/spcp.routes.ts +++ b/src/app/modules/spcp/spcp.routes.ts @@ -14,6 +14,7 @@ export const SpcpRouter = Router() * Provide a URL that web agents are obliged to redirect users to. * Due to cross-origin restrictions in place by SingPass/CorpPass, * this endpoint cannot and should not issue a 302 redirect + * @deprecated in favour of GET /api/v3/forms/:formId/auth/redirect * @route GET /spcp/redirect * @group SPCP - SingPass/CorpPass logins for form-fillers * @param target.query.required - the destination URL after login diff --git a/src/app/modules/spcp/spcp.types.ts b/src/app/modules/spcp/spcp.types.ts index a13976278a..117676b227 100644 --- a/src/app/modules/spcp/spcp.types.ts +++ b/src/app/modules/spcp/spcp.types.ts @@ -1,3 +1,5 @@ +import { AuthType, IFormSchema } from '../../../types' + export enum JwtName { SP = 'jwtSp', CP = 'jwtCp', @@ -43,3 +45,8 @@ export interface ParsedSpcpParams { rememberMe: boolean cookieDuration: number } + +export type SpcpForm = T & { + authType: AuthType.SP | AuthType.CP + esrvcId: string +} diff --git a/src/app/modules/spcp/spcp.util.ts b/src/app/modules/spcp/spcp.util.ts index c802aded61..fde8713544 100644 --- a/src/app/modules/spcp/spcp.util.ts +++ b/src/app/modules/spcp/spcp.util.ts @@ -1,11 +1,22 @@ import SPCPAuthClient from '@opengovsg/spcp-auth-client' import crypto from 'crypto' import { StatusCodes } from 'http-status-codes' +import { err, ok, Result } from 'neverthrow' -import { BasicField, MapRouteError, SPCPFieldTitle } from '../../../types' +import { + AuthType, + BasicField, + IFormSchema, + MapRouteError, + SPCPFieldTitle, +} from '../../../types' import { createLoggerWithLabel } from '../../config/logger' import { hasProp } from '../../utils/has-prop' import { MissingFeatureError } from '../core/core.errors' +import { + AuthTypeMismatchError, + FormAuthNoEsrvcIdError, +} from '../form/form.errors' import { ProcessedSingleAnswerResponse } from '../submission/submission.types' import { @@ -16,7 +27,7 @@ import { MissingJwtError, VerifyJwtError, } from './spcp.errors' -import { CorppassJwtPayload, SingpassJwtPayload } from './spcp.types' +import { CorppassJwtPayload, SingpassJwtPayload, SpcpForm } from './spcp.types' const logger = createLoggerWithLabel(module) const DESTINATION_REGEX = /^\/([\w]+)\/?/ @@ -210,6 +221,32 @@ export const createCorppassParsedResponses = ( ] } +/** + * Validates that a form is a SPCP form with an e-service ID + * @param form Form to validate + */ +export const validateSpcpForm = ( + form: T, +): Result, FormAuthNoEsrvcIdError | AuthTypeMismatchError> => { + // This is an extra check to return the specific error encountered + if (!form.esrvcId) { + return err(new FormAuthNoEsrvcIdError(form.id)) + } + if (isSpcpForm(form)) { + return ok(form) + } + return err(new AuthTypeMismatchError(AuthType.CP, form.authType)) +} + +// Typeguard to ensure that form has eserviceId and correct authType +const isSpcpForm = (form: F): form is SpcpForm => { + return ( + !!form.authType && + [AuthType.SP, AuthType.CP].includes(form.authType) && + !!form.esrvcId + ) +} + /** * Maps errors to status codes and error messages to return to frontend. * @param error @@ -257,3 +294,16 @@ export const mapRouteError: MapRouteError = ( } } } + +// Generates the target to redirect to for the given form id +export const getRedirectTarget = ( + formId: string, + authType: AuthType.SP | AuthType.CP, + isPersistentLogin?: boolean, +): string => + `/${formId},${ + // Need to cast to boolean because undefined is allowed as a valid value + // We are not following corppass's official spec for + // the target parameter + authType === AuthType.SP ? !!isPersistentLogin : false + }` diff --git a/src/app/modules/submission/email-submission/__tests__/email-submission.routes.spec.ts b/src/app/modules/submission/email-submission/__tests__/email-submission.routes.spec.ts index b8ca2d45d9..39a36a3d67 100644 --- a/src/app/modules/submission/email-submission/__tests__/email-submission.routes.spec.ts +++ b/src/app/modules/submission/email-submission/__tests__/email-submission.routes.spec.ts @@ -1,4 +1,4 @@ -import { MyInfoGovClient } from '@opengovsg/myinfo-gov-client' +import MyInfoClient, { IMyInfoConfig } from '@opengovsg/myinfo-gov-client' import SPCPAuthClient from '@opengovsg/spcp-auth-client' import { omit } from 'lodash' import mongoose from 'mongoose' @@ -44,7 +44,9 @@ jest.mock('nodemailer', () => ({ })) jest.mock('@opengovsg/myinfo-gov-client', () => ({ - MyInfoGovClient: jest.fn(), + MyInfoGovClient: jest.fn().mockReturnValue({ + extractUinFin: jest.fn(), + }), MyInfoMode: jest.requireActual('@opengovsg/myinfo-gov-client').MyInfoMode, MyInfoSource: jest.requireActual('@opengovsg/myinfo-gov-client').MyInfoSource, MyInfoAddressType: jest.requireActual('@opengovsg/myinfo-gov-client') @@ -52,13 +54,10 @@ jest.mock('@opengovsg/myinfo-gov-client', () => ({ MyInfoAttribute: jest.requireActual('@opengovsg/myinfo-gov-client') .MyInfoAttribute, })) -const MockMyInfoGovClient = mocked(MyInfoGovClient, true) -const mockExtractUinFin = jest.fn() -MockMyInfoGovClient.mockImplementation( - () => - (({ - extractUinFin: mockExtractUinFin, - } as unknown) as MyInfoGovClient), + +const MockMyInfoGovClient = mocked( + new MyInfoClient.MyInfoGovClient({} as IMyInfoConfig), + true, ) const SUBMISSIONS_ENDPT_BASE = '/v2/submissions/email' @@ -616,7 +615,7 @@ describe('email-submission.routes', () => { describe('MyInfo', () => { it('should return 200 when submission is valid', async () => { - mockExtractUinFin.mockReturnValueOnce(MOCK_UINFIN) + MockMyInfoGovClient.extractUinFin.mockReturnValueOnce(MOCK_UINFIN) const { form } = await dbHandler.insertEmailForm({ formOptions: { esrvcId: 'mockEsrvcId', @@ -704,7 +703,7 @@ describe('email-submission.routes', () => { it('should return 401 when submission has invalid cookie', async () => { // Mock MyInfoGovClient to return error when decoding JWT - mockExtractUinFin.mockImplementationOnce(() => { + MockMyInfoGovClient.extractUinFin.mockImplementationOnce(() => { throw new Error() }) const { form } = await dbHandler.insertEmailForm({ diff --git a/src/app/routes/api/v3/forms/public-forms.auth.routes.ts b/src/app/routes/api/v3/forms/public-forms.auth.routes.ts new file mode 100644 index 0000000000..03f8bd52f2 --- /dev/null +++ b/src/app/routes/api/v3/forms/public-forms.auth.routes.ts @@ -0,0 +1,23 @@ +import { Router } from 'express' + +import * as PublicFormController from '../../../../modules/form/public-form/public-form.controller' + +export const PublicFormsAuthRouter = Router() + +/** + * Redirects the user to the specified authentication provider. + * After authenticating their identity, the user is redirected back to the form + * @route /:formId/auth/redirect + * @param isPersistentLogin if the user chooses to have their information saved to avoid future logins + * + * @returns 200 with the redirect url when the user authenticates successfully + * @returns 400 when there is an error on the authType of the form + * @returns 400 when the eServiceId of the form does not exist + * @returns 404 when form with given ID does not exist + * @returns 500 when database error occurs + * @returns 500 when the redirect url could not be created + * @returns 500 when the redirect feature is not enabled + */ +PublicFormsAuthRouter.route('/:formId([a-fA-F0-9]{24})/auth/redirect').get( + PublicFormController.handleFormAuthRedirect, +) diff --git a/src/app/routes/api/v3/forms/public-forms.form.routes.ts b/src/app/routes/api/v3/forms/public-forms.form.routes.ts index 515925ceec..e87cf9e443 100644 --- a/src/app/routes/api/v3/forms/public-forms.form.routes.ts +++ b/src/app/routes/api/v3/forms/public-forms.form.routes.ts @@ -18,7 +18,6 @@ export const PublicFormsFormRouter = Router() * @returns 410 when form is archived * @returns 500 when database error occurs */ -PublicFormsFormRouter.get( - '/:formId([a-fA-F0-9]{24})', +PublicFormsFormRouter.route('/:formId([a-fA-F0-9]{24})').get( PublicFormController.handleGetPublicForm, ) diff --git a/src/app/routes/api/v3/forms/public-forms.routes.ts b/src/app/routes/api/v3/forms/public-forms.routes.ts index 41129728c9..73745954e5 100644 --- a/src/app/routes/api/v3/forms/public-forms.routes.ts +++ b/src/app/routes/api/v3/forms/public-forms.routes.ts @@ -1,5 +1,6 @@ import { Router } from 'express' +import { PublicFormsAuthRouter } from './public-forms.auth.routes' import { PublicFormsFeedbackRouter } from './public-forms.feedback.routes' import { PublicFormsFormRouter } from './public-forms.form.routes' import { PublicFormsSubmissionsRouter } from './public-forms.submissions.routes' @@ -9,3 +10,4 @@ export const PublicFormsRouter = Router() PublicFormsRouter.use(PublicFormsSubmissionsRouter) PublicFormsRouter.use(PublicFormsFeedbackRouter) PublicFormsRouter.use(PublicFormsFormRouter) +PublicFormsRouter.use(PublicFormsAuthRouter) diff --git a/src/public/main.js b/src/public/main.js index 1bdfeacf96..dc69a9f1db 100644 --- a/src/public/main.js +++ b/src/public/main.js @@ -262,7 +262,6 @@ require('./modules/forms/services/form-factory.client.service.js') require('./modules/forms/services/form-api.client.factory.js') require('./modules/forms/services/form-error.client.factory.js') require('./modules/forms/services/spcp-session.client.factory.js') -require('./modules/forms/services/spcp-redirect.client.factory.js') require('./modules/forms/services/spcp-validate-esrvcid.client.factory.js') require('./modules/forms/services/submissions.client.factory.js') require('./modules/forms/services/toastr.client.factory.js') diff --git a/src/public/modules/forms/base/componentViews/start-page.html b/src/public/modules/forms/base/componentViews/start-page.html index 5753ce9f09..e9e756d762 100644 --- a/src/public/modules/forms/base/componentViews/start-page.html +++ b/src/public/modules/forms/base/componentViews/start-page.html @@ -50,7 +50,7 @@

{{ vm.formTitle }}