From f1c96d91459b5d3f9255cafe70a3f3fa565f7a4c Mon Sep 17 00:00:00 2001 From: Antariksh Date: Thu, 19 Nov 2020 15:37:45 +0800 Subject: [PATCH 1/7] ref: use WithForm in types --- src/app/modules/form/admin-form/admin-form.controller.ts | 3 +-- src/app/modules/spcp/spcp.controller.ts | 7 +------ 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/src/app/modules/form/admin-form/admin-form.controller.ts b/src/app/modules/form/admin-form/admin-form.controller.ts index c8051a9258..ff330329e5 100644 --- a/src/app/modules/form/admin-form/admin-form.controller.ts +++ b/src/app/modules/form/admin-form/admin-form.controller.ts @@ -2,7 +2,7 @@ import { RequestHandler } from 'express' import { ParamsDictionary } from 'express-serve-static-core' import { createLoggerWithLabel } from '../../../../config/logger' -import { AuthType, IPopulatedForm } from '../../../../types' +import { AuthType, WithForm } from '../../../../types' import { createReqMeta } from '../../../utils/request' import * as SubmissionService from '../../submission/submission.service' import * as UserService from '../../user/user.service' @@ -18,7 +18,6 @@ import { assertHasReadPermissions, mapRouteError } from './admin-form.utils' const logger = createLoggerWithLabel(module) -type WithForm = T & { form: IPopulatedForm } /** * Handler for GET /adminform endpoint. * @security session diff --git a/src/app/modules/spcp/spcp.controller.ts b/src/app/modules/spcp/spcp.controller.ts index 77e3e820c9..4e301c850b 100644 --- a/src/app/modules/spcp/spcp.controller.ts +++ b/src/app/modules/spcp/spcp.controller.ts @@ -4,7 +4,7 @@ import { StatusCodes } from 'http-status-codes' import config from '../../../config/config' import { createLoggerWithLabel } from '../../../config/logger' -import { AuthType, IPopulatedForm } from '../../../types' +import { AuthType, WithForm } from '../../../types' import { createReqMeta } from '../../utils/request' import * as FormService from '../form/form.service' @@ -14,11 +14,6 @@ import { extractJwt, mapRouteError } from './spcp.util' const logger = createLoggerWithLabel(module) -// TODO (#42): remove these types when migrating away from middleware pattern -type WithForm = T & { - form: IPopulatedForm -} - /** * Generates redirect URL to Official SingPass/CorpPass log in page * @param req - Express request object From 9675c71b915ee03220b82c5815d4668c9841a723 Mon Sep 17 00:00:00 2001 From: Antariksh Date: Thu, 19 Nov 2020 15:39:24 +0800 Subject: [PATCH 2/7] ref: move SpcpLocals to express.locals --- .../modules/form/admin-form/admin-form.service.ts | 14 ++++++-------- src/types/express.locals.ts | 8 ++++++++ 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/src/app/modules/form/admin-form/admin-form.service.ts b/src/app/modules/form/admin-form/admin-form.service.ts index 03655d6a25..d8c9b9e502 100644 --- a/src/app/modules/form/admin-form/admin-form.service.ts +++ b/src/app/modules/form/admin-form/admin-form.service.ts @@ -8,7 +8,12 @@ import { MAX_UPLOAD_FILE_SIZE, VALID_UPLOAD_FILE_TYPES, } from '../../../../shared/constants' -import { AuthType, DashboardFormView, MyInfoAttribute } from '../../../../types' +import { + AuthType, + DashboardFormView, + MyInfoAttribute, + SpcpLocals, +} from '../../../../types' import getFormModel from '../../../models/form.server.model' import { DatabaseError } from '../../core/core.errors' import { MissingUserError } from '../../user/user.errors' @@ -167,13 +172,6 @@ export const createPresignedPostForLogos = ( return createPresignedPost(AwsConfig.logoS3Bucket, uploadParams) } -type SpcpLocals = - | { - uinFin: string - hashedFields: Set - } - | { uinFin: string; userInfo: string } - | { [key: string]: never } // empty object export const getMockSpcpLocals = ( authType: AuthType, myInfoAttrs: MyInfoAttribute[], diff --git a/src/types/express.locals.ts b/src/types/express.locals.ts index 819dfb1fd9..80741821dc 100644 --- a/src/types/express.locals.ts +++ b/src/types/express.locals.ts @@ -19,3 +19,11 @@ export type ResWithUinFin = T & { export type ResWithHashedFields = T & { locals: { hashedFields?: Set } } + +export type SpcpLocals = + | { + uinFin: string + hashedFields: Set + } + | { uinFin: string; userInfo: string } + | { [key: string]: never } // empty object From dc10e8c0eb174c6cb32270b679158f71c705f497 Mon Sep 17 00:00:00 2001 From: Antariksh Date: Thu, 19 Nov 2020 16:15:00 +0800 Subject: [PATCH 3/7] ref: add utils for appending responses --- src/app/modules/spcp/spcp.util.ts | 39 ++++++++++++++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-) diff --git a/src/app/modules/spcp/spcp.util.ts b/src/app/modules/spcp/spcp.util.ts index c893612a83..010eb085d5 100644 --- a/src/app/modules/spcp/spcp.util.ts +++ b/src/app/modules/spcp/spcp.util.ts @@ -3,8 +3,9 @@ import crypto from 'crypto' import { StatusCodes } from 'http-status-codes' import { createLoggerWithLabel } from '../../../config/logger' -import { AuthType, MapRouteError } from '../../../types' +import { AuthType, BasicField, MapRouteError } from '../../../types' import { MissingFeatureError } from '../core/core.errors' +import { ProcessedSingleAnswerResponse } from '../submission/submission.types' import { CreateRedirectUrlError, @@ -114,6 +115,42 @@ export const extractJwt = ( } } +export const createSingpassParsedResponses = ( + uinFin: string, +): ProcessedSingleAnswerResponse[] => { + return [ + { + _id: '', + question: 'SingPass Validated NRIC', + fieldType: 'authenticationSp' as BasicField, + isVisible: true, + answer: uinFin, + }, + ] +} + +export const createCorppassParsedResponses = ( + uinFin: string, + userInfo: string, +): ProcessedSingleAnswerResponse[] => { + return [ + { + _id: '', + question: 'CorpPass Validated UEN', + fieldType: 'authenticationCp' as BasicField, + isVisible: true, + answer: uinFin, + }, + { + _id: '', + question: 'CorpPass Validated UID', + fieldType: 'authenticationCp' as BasicField, + isVisible: true, + answer: userInfo, + }, + ] +} + export const mapRouteError: MapRouteError = (error) => { switch (error.constructor) { case MissingFeatureError: From 7608e9c31ca951cb0d8b35170f000a8b9ce177d6 Mon Sep 17 00:00:00 2001 From: Antariksh Date: Thu, 19 Nov 2020 16:20:05 +0800 Subject: [PATCH 4/7] ref: implement new appendVerifiedSPCPResponses --- src/app/modules/spcp/spcp.controller.ts | 33 ++++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/src/app/modules/spcp/spcp.controller.ts b/src/app/modules/spcp/spcp.controller.ts index 4e301c850b..f8a08fa705 100644 --- a/src/app/modules/spcp/spcp.controller.ts +++ b/src/app/modules/spcp/spcp.controller.ts @@ -7,10 +7,16 @@ import { createLoggerWithLabel } from '../../../config/logger' import { AuthType, WithForm } from '../../../types' import { createReqMeta } from '../../utils/request' import * as FormService from '../form/form.service' +import { ProcessedFieldResponse } from '../submission/submission.types' import { SpcpFactory } from './spcp.factory' import { JwtName, LoginPageValidationResult } from './spcp.types' -import { extractJwt, mapRouteError } from './spcp.util' +import { + createCorppassParsedResponses, + createSingpassParsedResponses, + extractJwt, + mapRouteError, +} from './spcp.util' const logger = createLoggerWithLabel(module) @@ -240,3 +246,28 @@ export const handleLogin: ( return res.redirect(destination) }) } + +/** + * Append additional verified responses(s) for SP and CP responses so that they show up in email response + * @param req - Express request object + * @param res - Express response object + */ +export const appendVerifiedSPCPResponses: RequestHandler< + ParamsDictionary, + unknown, + { parsedResponses: ProcessedFieldResponse[] } +> = (req, res, next) => { + const { form } = req as WithForm + const { uinFin, userInfo } = res.locals + switch (form.authType) { + case AuthType.SP: + req.body.parsedResponses.push(...createSingpassParsedResponses(uinFin)) + break + case AuthType.CP: + req.body.parsedResponses.push( + ...createCorppassParsedResponses(uinFin, userInfo), + ) + break + } + return next() +} From 2caf39d57cb789972500a25275577106bd43da07 Mon Sep 17 00:00:00 2001 From: Antariksh Date: Thu, 19 Nov 2020 16:27:30 +0800 Subject: [PATCH 5/7] ref: use new method --- src/app/routes/admin-forms.server.routes.js | 5 ++--- src/app/routes/public-forms.server.routes.js | 3 +-- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/app/routes/admin-forms.server.routes.js b/src/app/routes/admin-forms.server.routes.js index 36b35e28ad..2163d85396 100644 --- a/src/app/routes/admin-forms.server.routes.js +++ b/src/app/routes/admin-forms.server.routes.js @@ -13,12 +13,11 @@ let submissions = require('../../app/controllers/submissions.server.controller') const emailSubmissions = require('../../app/controllers/email-submissions.server.controller') let encryptSubmissions = require('../../app/controllers/encrypt-submissions.server.controller') let PERMISSIONS = require('../utils/permission-levels').default -const spcpFactory = require('../factories/spcp.factory') const webhookVerifiedContentFactory = require('../factories/webhook-verified-content.factory') const AdminFormController = require('../modules/form/admin-form/admin-form.controller') const { withUserAuthentication } = require('../modules/auth/auth.middlewares') const EncryptSubmissionController = require('../modules/submission/encrypt-submission/encrypt-submission.controller') - +const SpcpController = require('../modules/spcp/spcp.controller') const YYYY_MM_DD_REGEX = /([12]\d{3}-(0[1-9]|1[0-2])-(0[1-9]|[12]\d|3[01]))/ const emailValOpts = { @@ -342,7 +341,7 @@ module.exports = function (app) { emailSubmissions.validateEmailSubmission, AdminFormController.passThroughSpcp, submissions.injectAutoReplyInfo, - spcpFactory.appendVerifiedSPCPResponses, + SpcpController.appendVerifiedSPCPResponses, emailSubmissions.prepareEmailSubmission, adminForms.passThroughSaveMetadataToDb, emailSubmissions.sendAdminEmail, diff --git a/src/app/routes/public-forms.server.routes.js b/src/app/routes/public-forms.server.routes.js index 91eacd0493..18f892ab73 100644 --- a/src/app/routes/public-forms.server.routes.js +++ b/src/app/routes/public-forms.server.routes.js @@ -10,7 +10,6 @@ const encryptSubmissions = require('../../app/controllers/encrypt-submissions.se const emailSubmissions = require('../../app/controllers/email-submissions.server.controller') const myInfoController = require('../../app/controllers/myinfo.server.controller') const { celebrate, Joi, Segments } = require('celebrate') -const spcpFactory = require('../factories/spcp.factory') const webhookVerifiedContentFactory = require('../factories/webhook-verified-content.factory') const { CaptchaFactory } = require('../factories/captcha.factory') const { limitRate } = require('../utils/limit-rate') @@ -192,7 +191,7 @@ module.exports = function (app) { emailSubmissions.validateEmailSubmission, myInfoController.verifyMyInfoVals, submissions.injectAutoReplyInfo, - spcpFactory.appendVerifiedSPCPResponses, + SpcpController.appendVerifiedSPCPResponses, emailSubmissions.prepareEmailSubmission, emailSubmissions.saveMetadataToDb, emailSubmissions.sendAdminEmail, From a9d4a55c556a785d327ac585b11d8894a5c3ba47 Mon Sep 17 00:00:00 2001 From: Antariksh Date: Thu, 19 Nov 2020 16:29:23 +0800 Subject: [PATCH 6/7] ref: remove unused code --- src/app/controllers/spcp.server.controller.js | 37 ------------------- src/app/factories/spcp.factory.js | 16 -------- 2 files changed, 53 deletions(-) delete mode 100644 src/app/factories/spcp.factory.js diff --git a/src/app/controllers/spcp.server.controller.js b/src/app/controllers/spcp.server.controller.js index 34b973da50..313857ccf9 100644 --- a/src/app/controllers/spcp.server.controller.js +++ b/src/app/controllers/spcp.server.controller.js @@ -49,40 +49,3 @@ exports.encryptedVerifiedFields = (signingSecretKey) => { } } } - -/** - * Append additional verified responses(s) for SP and CP responses so that they show up in email response - * @param {Object} req - Express request object - * @param {Object} res - Express response object - */ -exports.appendVerifiedSPCPResponses = function (req, res, next) { - const { form } = req - const { uinFin, userInfo } = res.locals - switch (form.authType) { - case 'SP': - req.body.parsedResponses.push({ - question: 'SingPass Validated NRIC', - fieldType: 'authenticationSp', - isVisible: true, - answer: uinFin, - }) - break - case 'CP': - // Add UEN to responses - req.body.parsedResponses.push({ - question: 'CorpPass Validated UEN', - fieldType: 'authenticationCp', - isVisible: true, - answer: uinFin, - }) - // Add UID to responses - req.body.parsedResponses.push({ - question: 'CorpPass Validated UID', - fieldType: 'authenticationCp', - isVisible: true, - answer: userInfo, - }) - break - } - return next() -} diff --git a/src/app/factories/spcp.factory.js b/src/app/factories/spcp.factory.js deleted file mode 100644 index e31116030e..0000000000 --- a/src/app/factories/spcp.factory.js +++ /dev/null @@ -1,16 +0,0 @@ -const spcp = require('../controllers/spcp.server.controller') -const featureManager = require('../../config/feature-manager').default - -const spcpFactory = ({ isEnabled, props }) => { - if (isEnabled && props) { - return { - appendVerifiedSPCPResponses: spcp.appendVerifiedSPCPResponses, - } - } else { - return { - appendVerifiedSPCPResponses: (req, res, next) => next(), - } - } -} - -module.exports = spcpFactory(featureManager.get('spcp-myinfo')) From fc7349898f28bbaec8c6256e83b1bfee0678a8e9 Mon Sep 17 00:00:00 2001 From: Antariksh Date: Thu, 19 Nov 2020 16:30:40 +0800 Subject: [PATCH 7/7] test: fix existing tests --- .../email-submissions.server.controller.spec.js | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/tests/unit/backend/controllers/email-submissions.server.controller.spec.js b/tests/unit/backend/controllers/email-submissions.server.controller.spec.js index 1b5316cd94..17f8f844c1 100644 --- a/tests/unit/backend/controllers/email-submissions.server.controller.spec.js +++ b/tests/unit/backend/controllers/email-submissions.server.controller.spec.js @@ -48,13 +48,9 @@ describe('Email Submissions Controller', () => { }, }, ) - const spcpController = spec( - 'dist/backend/app/controllers/spcp.server.controller', - { - mongoose: Object.assign(mongoose, { '@noCallThru': true }), - '../../config/ndi-config': {}, - }, - ) + const spcpController = spec('dist/backend/app/modules/spcp/spcp.controller', { + mongoose: Object.assign(mongoose, { '@noCallThru': true }), + }) beforeAll(async () => await dbHandler.connect()) beforeEach(() => {