refactor: update getPublicForm endpoint to typescript (#1398)

* refactor(modules/spcp): shifts get spcp session out into service

* refactor(modules/form): adds utility methods and refactored checkFormSubmission

* refactor(modules/form): refactors GET public forms end point from js to ts

* refactor(utils): adds utility type for all possible db errors

* fix(types/form): fixed the type of submissionLimit to allow for null

* refactor(modules/form): updated deactiveForm so that it uses never throw and adds logging

* refactor(modules/form): updated form limit checking to account for submission limit being null

* refactor(modules/form): adds logging to getSpcpSession

* refactor(modules/form): refactored handleGetPublicForm for clarity and code cleanliness

* refactor(publicformctrl): refactored handler for GET public forms to be cleaner and easier to read

* refactor(spcpsvc): added logging to extract JWT; updated typings for getSpcpSession

* revert(formsvc): removes retrievePublicForm (will be done in another PR) to limit scope

* style(public-form/controller): updated object to use variables

* style(services/types): updated documentation for functions and removed unneeded logging

* refactor(public-form/controller): refactored spcp flow so it's clearer

* refactor(public-form/controller): wip for myInfo

* refactor(myinfo): extracts myInfoCookiePayload to 2 types and adds utility function to differentiate

* refactor(public-form/controller): refactored myinfo chunk so it's neater

* test(form/service): fixes failing tests due to checkFormSubmissionLimitAndDeactivateForm

* refactor(myinfo): removed unneeded cookie type and updated extractSuccessfulCookie to reflect this

* docs(public-form/controller): adds docs to confusing sections of handlGetPublicForm

* test(public-form/controller/test): add test for database error when GET /getPublicForm

* test(public-form/controller/tests): add more tests

* test(public-form/controller/test): adds test for success cases

* refactor(myinfo): extracts out chunks form public-form controller into myinfo service

* refactor(public-form/controller): updated controller to use MyInfoFactory method

* test(public-form/controller/test): updated tests to use mocks

* fix(public-form/controller): fixed succesful  200 when auth using myInfo returning a private view

* test(public-form/controller/test): adds remaining tests for myInfo

* refactor(spcp): combined controller calls to spcp services into a single spcp call

* refactor(myinfo): refactored multiple controller calls to myInfo into a single call

* refactor(public-form): updated typings and refactored getPublicForm to be cleaner

* style(public-form/controller/test): updated comments in tests to use when

* refactor(form/service): updated deactiveForm to return the form itself; updated tests

* style(myinfo): extractMyInfoData renamed to fetchMyInfoData

* fix(public-form/controller): fixed bug where wrong extract cookie method is being called

* refactor(spcp/myinfo): removed extra logging in spcp; updated names in myinfo

* test(spcp/service/test): adds service tests

* test(spcp/service/test): adds unit tests for createFormWithSpcpSession

* test(myinfo.service): adds tests for createFormWithMyInfo

* feat(form): adds new errors and utility methods

* refactor(public-form): refactor to account for intarnet

* refactor(public-form/controller): added compatability for checking intranet access

* test(public-form/controller): adds tests for checking intranet; fixes old tests due to this addition

* style(spcp/service/test): changed naming to camelCase for variables

* fix(form.service): fixed intranet ip checking

* refactor(public-form): shifts utility method out into public form service

* test(public-form/controller): updates tests

* refactor(public-forms/server/routes): swaps to new controller for express middleware

* fix(myinfo/util): added cookie access check

* fix(form/service): adds error recovery for missing feature error when checking intranet access

* refactor(public-form/controller): tightened logic for myinfo error

* build(package.json): added ts-essential for UnreachableCaseException

* refactor(forms): added new type for intranet form

* refactor(spcp/service): updated service methods

* refactor(intranet/factory): updated isIntranetIp and factory signature typings

* refactor(myinfo): updated typings and factory methods to remove responsibility from service

* refactor(public-form/controller): wip

* refactor(form/service): remove result wrapping as only error is MissingFeatureError

* refactor(myinfo): updated typings and comments for myInfo

* refactor(publicform): updated handleGetPublicForm method and removed unused types

* refactor(public-form/service): removed unnused method

* fix(public-form/controller): changed returned form to be a publicForm

* test(myinfo/service): updated tests for myinfo service

* test(public-form/controller): updated tests to fit iwth refactor

* chore(intranet): removed unused variables

* test(intranet/service): fixes failing tests due to refactor

* refactor(app/utils): removed duplicate datatype in handle mongo errors

* style(form/service): updated logger message

* test(spcp/service): removed tests for deleted method; updated test for getSpcpSession

* refactor(public-form/controller): extracts logger meta property into a variable

* style(form/service): updated action property of logger meta

* docs(public-form/controller): updated comments for getPublicForm on conditions for  myInfoError

* refactor(myinfo): deleted unused middleware; made fetchMyInfoPersonData private

* refactor(myinfo): changed fetchMyInfoPersonData to become a private field

* test(myinfo): fixed factory and service tests due to making myInfoPersonData private

* chore(webhook/service/test): fixed import

* chore(types/forms): removed unused type declaration

* style(form/service): chains calls together for clarity

* refactor(myinfo): refactored methods so that atomic operations are performed together

* test(myinfo): fixes tests for myinfo

* style(spcp): renamed service methods for clarity; removed unused error

* test(spcp): fixes spcp tests

* refactor(public-form): updated controller to account for myinfo/spcp refactoring

* test(public-form): updated tests

* fix(public-form/controller): fixed cp returning userInfo as ewll

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",

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 () => {

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<IFormSchema | null> => {
-  return FormModel.deactivateById(formId)
+): ResultAsync<IFormSchema, PossibleDatabaseError | FormNotFoundError> => {
+  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,17 +152,16 @@ 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,
 ): Result<true, FormDeletedError | PrivateFormError | ApplicationError> => {
   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<true, PrivateFormError | PossibleDatabaseError> => {
+): 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)
+  )
+}