feat(feature-manager): remove Intranet from feature manager

docker-compose.yml

@@ -39,6 +39,7 @@ services:
       - MYINFO_CLIENT_ID=mockClientId
       - MYINFO_CLIENT_SECRET=mockClientSecret
       - WEBHOOK_SQS_URL=http://localhost:4566/000000000000/local-webhooks-sqs-main
+      - INTRANET_IP_LIST_PATH
       - GA_TRACKING_ID
       - SENTRY_CONFIG_URL
       - TWILIO_ACCOUNT_SID

src/app/config/feature-manager/index.ts

@@ -1,7 +1,6 @@
 import FeatureManager from './util/FeatureManager.class'
 import captcha from './captcha.config'
 import googleAnalytics from './google-analytics.config'
-import { intranetFeature } from './intranet.config'
 import sentry from './sentry.config'
 import sms from './sms.config'
 import spcpMyInfo from './spcp-myinfo.config'
@@ -20,6 +19,5 @@ featureManager.register(spcpMyInfo)
 featureManager.register(webhookVerifiedContent)
 featureManager.register(sms)
 featureManager.register(verifiedFields)
-featureManager.register(intranetFeature)
 
 export default featureManager

src/app/config/feature-manager/intranet.config.ts

@@ -1,13 +1,18 @@
-import { FeatureNames, RegisterableFeature } from './types'
+import convict, { Schema } from 'convict'
 
-export const intranetFeature: RegisterableFeature<FeatureNames.Intranet> = {
-  name: FeatureNames.Intranet,
-  schema: {
-    intranetIpListPath: {
-      doc: 'Path to file containing list of intranet IP addresses, separated by newlines',
-      format: String,
-      default: null,
-      env: 'INTRANET_IP_LIST_PATH',
-    },
+export interface IIntranet {
+  intranetIpListPath: string
+}
+
+const intranetSchema: Schema<IIntranet> = {
+  intranetIpListPath: {
+    doc: 'Path to file containing list of intranet IP addresses, separated by newlines',
+    format: String,
+    default: '',
+    env: 'INTRANET_IP_LIST_PATH',
   },
 }
+
+export const intranetConfig = convict(intranetSchema)
+  .validate({ allowed: 'strict' })
+  .getProperties()

src/app/config/feature-manager/types.ts

@@ -9,7 +9,6 @@ export enum FeatureNames {
   SpcpMyInfo = 'spcp-myinfo',
   VerifiedFields = 'verified-fields',
   WebhookVerifiedContent = 'webhook-verified-content',
-  Intranet = 'intranet',
 }
 
 export interface ICaptcha {
@@ -77,10 +76,6 @@ export interface IWebhookVerifiedContent {
   webhookQueueUrl: string
 }
 
-export interface IIntranet {
-  intranetIpListPath: string
-}
-
 export interface IFeatureManager {
   [FeatureNames.Captcha]: ICaptcha
   [FeatureNames.GoogleAnalytics]: IGoogleAnalytics
@@ -89,7 +84,6 @@ export interface IFeatureManager {
   [FeatureNames.SpcpMyInfo]: ISpcpMyInfo
   [FeatureNames.VerifiedFields]: IVerifiedFields
   [FeatureNames.WebhookVerifiedContent]: IWebhookVerifiedContent
-  [FeatureNames.Intranet]: IIntranet
 }
 
 export interface RegisteredFeature<T extends FeatureNames> {

src/app/modules/form/form.service.ts

@@ -16,7 +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 { IntranetService } from '../../services/intranet/intranet.service'
 import {
   getMongoErrorMessage,
   transformMongoError,
@@ -285,27 +285,21 @@ 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)
-  )
+  const isIntranetUser = IntranetService.isIntranetIp(ip)
+  // 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 isIntranetUser
 }

src/app/services/intranet/__tests__/intranet.service.spec.ts

@@ -2,42 +2,23 @@ import fs from 'fs'
 
 import { IntranetService } from '../intranet.service'
 
-const MOCK_IP_LIST = ['1.2.3.4', '5.6.7.8']
-const MOCK_IP_LIST_FILE = Buffer.from(MOCK_IP_LIST.join('\n'))
-const MOCK_IP_LIST_PATH = '../some/path'
-
-jest.mock('fs', () => ({
-  ...(jest.requireActual('fs') as typeof fs),
-  readFileSync: jest.fn().mockImplementation(() => MOCK_IP_LIST_FILE),
-}))
+const MOCK_IP_LIST_PATH = 'tests/mock-intranet-ips.txt'
+const MOCK_IP_LIST = fs.readFileSync(MOCK_IP_LIST_PATH).toString().split('\n')
 
 describe('IntranetService', () => {
-  const intranetService = new IntranetService({
-    intranetIpListPath: MOCK_IP_LIST_PATH,
-  })
-
   afterEach(() => jest.clearAllMocks())
-  describe('constructor', () => {
-    it('should instantiate without errors', () => {
-      const intranetService = new IntranetService({
-        intranetIpListPath: MOCK_IP_LIST_PATH,
-      })
-
-      expect(intranetService).toBeTruthy()
-    })
-  })
 
   describe('isIntranetIp', () => {
     it('should return true when IP is in intranet IP list', () => {
-      const result = intranetService.isIntranetIp(MOCK_IP_LIST[0])
+      const result = IntranetService.isIntranetIp(MOCK_IP_LIST[0])
 
       expect(result).toBe(true)
     })
 
     it('should return false when IP is not in intranet IP list', () => {
       const ipNotInList = '10.20.30.40'
 
-      const result = intranetService.isIntranetIp(ipNotInList)
+      const result = IntranetService.isIntranetIp(ipNotInList)
 
       expect(result).toBe(false)
     })

src/app/services/intranet/intranet.middleware.ts

@@ -2,14 +2,14 @@ import { createLoggerWithLabel } from '../../config/logger'
 import { ControllerHandler } from '../../modules/core/core.types'
 import { createReqMeta, getRequestIp } from '../../utils/request'
 
-import { IntranetFactory } from './intranet.factory'
+import { IntranetService } from './intranet.service'
 
 const logger = createLoggerWithLabel(module)
 
 export const logIntranetUsage: ControllerHandler = (req, _res, next) => {
-  const isIntranetResult = IntranetFactory.isIntranetIp(getRequestIp(req))
+  const isIntranet = IntranetService.isIntranetIp(getRequestIp(req))
   // Ignore case where result is err, as this means intranet feature is not enabled
-  if (isIntranetResult.isOk() && isIntranetResult.value) {
+  if (isIntranet) {
     logger.info({
       message: 'Request originated from SGProxy',
       meta: {

src/app/services/intranet/intranet.service.ts

@@ -1,14 +1,17 @@
 import fs from 'fs'
 
-import { IIntranet } from '../../config/feature-manager'
+import {
+  IIntranet,
+  intranetConfig,
+} from '../../config/feature-manager/intranet.config'
 import { createLoggerWithLabel } from '../../config/logger'
 
 const logger = createLoggerWithLabel(module)
 
 /**
  * Handles intranet functionality based on a given list of intranet IPs.
  */
-export class IntranetService {
+class IntranetServiceClass {
   /**
    * List of IP addresses associated with intranet
    */
@@ -19,6 +22,10 @@ export class IntranetService {
     // e.g. intranet-only forms, then this try-catch should be removed so that
     // an error is thrown if the intranet IP list file does not exist.
     // For now, the functionality is not crucial, so we can default to an empty array.
+    if (!intranetConfig.intranetIpListPath) {
+      this.intranetIps = []
+      return
+    }
     try {
       this.intranetIps = fs
         .readFileSync(intranetConfig.intranetIpListPath)
@@ -44,3 +51,5 @@ export class IntranetService {
     return this.intranetIps.includes(ip)
   }
 }
+
+export const IntranetService = new IntranetServiceClass(intranetConfig)

tests/.test-env

@@ -71,4 +71,6 @@ AWS_SECRET_ACCESS_KEY=fakeSecretAccessKey
 
 AWS_ENDPOINT=http://localhost:4566
 
-APP_URL=http://localhost:5000
+APP_URL=http://localhost:5000
+
+INTRANET_IP_LIST_PATH=tests/mock-intranet-ips.txt

tests/mock-intranet-ips.txt

@@ -0,0 +1,2 @@
+1.2.3.4
+5.6.7.8