Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: show error Toastr for intranet users on SPCP forms #1397

Merged
merged 12 commits into from
Mar 18, 2021
25 changes: 25 additions & 0 deletions src/app/controllers/forms.server.controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ const { StatusCodes } = require('http-status-codes')
const { createReqMeta } = require('../utils/request')
const logger = require('../../config/logger').createLoggerWithLabel(module)
const getFormModel = require('../models/form.server.model').default
const { IntranetFactory } = require('../services/intranet/intranet.factory')
const { getRequestIp } = require('../utils/request')
const { AuthType } = require('../../types')

const Form = getFormModel(mongoose)

Expand Down Expand Up @@ -75,10 +78,32 @@ exports.read = (requestType) =>
form = _.pick(form, formPublicFields)
}

const isIntranetResult = IntranetFactory.isIntranetIp(getRequestIp(req))
let isIntranetUser = false
if (isIntranetResult.isOk()) {
isIntranetUser = isIntranetResult.value
}

// SP, CP and MyInfo are not available on intranet
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: 'read',
formId: form._id,
},
})
}

return res.json({
form,
spcpSession,
myInfoError,
isIntranetUser,
})
}

Expand Down
50 changes: 50 additions & 0 deletions src/app/services/intranet/__tests__/intranet.factory.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import { mocked } from 'ts-jest/utils'

import { MissingFeatureError } from 'src/app/modules/core/core.errors'
import { FeatureNames } from 'src/config/feature-manager'

import { createIntranetFactory } from '../intranet.factory'
import { IntranetService } from '../intranet.service'

jest.mock('../intranet.service')
const MockIntranetService = mocked(IntranetService, true)

const MOCK_INTRANET_PROPS = {
intranetIpListPath: 'somePath',
}

describe('intranet.factory', () => {
afterEach(() => jest.clearAllMocks())

it('should call the IntranetService constructor when Intranet feature is enabled', () => {
createIntranetFactory({
isEnabled: true,
props: MOCK_INTRANET_PROPS,
})

expect(MockIntranetService).toHaveBeenCalledWith(MOCK_INTRANET_PROPS)
})

it('should return error functions when isEnabled is false', () => {
const intranetFactory = createIntranetFactory({
isEnabled: false,
props: MOCK_INTRANET_PROPS,
})

expect(MockIntranetService).not.toHaveBeenCalled()
expect(intranetFactory.isIntranetIp('')._unsafeUnwrapErr()).toEqual(
new MissingFeatureError(FeatureNames.Intranet),
)
})

it('should return error functions when props is undefined', () => {
const intranetFactory = createIntranetFactory({
isEnabled: true,
})

expect(MockIntranetService).not.toHaveBeenCalled()
expect(intranetFactory.isIntranetIp('')._unsafeUnwrapErr()).toEqual(
new MissingFeatureError(FeatureNames.Intranet),
)
})
})
45 changes: 45 additions & 0 deletions src/app/services/intranet/__tests__/intranet.service.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
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),
}))

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])

expect(result._unsafeUnwrap()).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)

expect(result._unsafeUnwrap()).toBe(false)
})
})
})
30 changes: 30 additions & 0 deletions src/app/services/intranet/intranet.factory.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import { err } from 'neverthrow'

import FeatureManager, {
FeatureNames,
RegisteredFeature,
} from '../../../config/feature-manager'
import { MissingFeatureError } from '../../modules/core/core.errors'

import { IntranetService } from './intranet.service'

interface IIntranetFactory {
isIntranetIp: IntranetService['isIntranetIp']
}

export const createIntranetFactory = ({
isEnabled,
props,
}: RegisteredFeature<FeatureNames.Intranet>): IIntranetFactory => {
if (isEnabled && props?.intranetIpListPath) {
return new IntranetService(props)
}

const error = new MissingFeatureError(FeatureNames.Intranet)
return {
isIntranetIp: () => err(error),
}
}

const intranetFeature = FeatureManager.get(FeatureNames.Intranet)
export const IntranetFactory = createIntranetFactory(intranetFeature)
23 changes: 23 additions & 0 deletions src/app/services/intranet/intranet.middleware.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { RequestHandler } from 'express-serve-static-core'

import { createLoggerWithLabel } from '../../../config/logger'
import { createReqMeta, getRequestIp } from '../../utils/request'

import { IntranetFactory } from './intranet.factory'

const logger = createLoggerWithLabel(module)

export const logIntranetUsage: RequestHandler = (req, _res, next) => {
const isIntranetResult = IntranetFactory.isIntranetIp(getRequestIp(req))
// Ignore case where result is err, as this means intranet feature is not enabled
if (isIntranetResult.isOk() && isIntranetResult.value) {
logger.info({
message: 'Request originated from SGProxy',
meta: {
action: 'logIntranetUsage',
...createReqMeta(req),
},
})
}
return next()
}
48 changes: 48 additions & 0 deletions src/app/services/intranet/intranet.service.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
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)

/**
* Handles intranet functionality based on a given list of intranet IPs.
*/
export class IntranetService {
/**
* List of IP addresses associated with intranet
*/
intranetIps: string[]

constructor(intranetConfig: IIntranet) {
// In future if crucial intranet-specific functionality is implemented,
// 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.
try {
this.intranetIps = fs
.readFileSync(intranetConfig.intranetIpListPath)
.toString()
.split('\n')
} catch {
logger.warn({
message: 'Could not read file containing intranet IPs',
meta: {
action: 'IntranetService',
},
})
this.intranetIps = []
}
}

/**
* Checks whether the given IP address is an intranet IP.
* @param ip IP address to check
* @returns Whether the IP address originated from the intranet
*/
isIntranetIp(ip: string): Result<boolean, ApplicationError> {
return ok(this.intranetIps.includes(ip))
}
}
3 changes: 2 additions & 1 deletion src/app/utils/request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ export const createReqMeta = (req: Request) => {
return {
ip: getRequestIp(req),
trace: getTrace(req), // trace using cloudflare cf-ray header, with x-request-id header as backup
url: req.url,
url: req.baseUrl + req.path,
urlWithQueryParams: req.originalUrl,
headers: req.headers,
}
}
2 changes: 2 additions & 0 deletions src/config/feature-manager/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import FeatureManager from './util/FeatureManager.class'
import aggregateStats from './aggregate-stats.config'
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'
Expand All @@ -21,5 +22,6 @@ featureManager.register(spcpMyInfo)
featureManager.register(webhookVerifiedContent)
featureManager.register(sms)
featureManager.register(verifiedFields)
featureManager.register(intranetFeature)

export default featureManager
14 changes: 14 additions & 0 deletions src/config/feature-manager/intranet.config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import { FeatureNames, RegisterableFeature } from './types'

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',
},
},
}
6 changes: 6 additions & 0 deletions src/config/feature-manager/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ export enum FeatureNames {
SpcpMyInfo = 'spcp-myinfo',
VerifiedFields = 'verified-fields',
WebhookVerifiedContent = 'webhook-verified-content',
Intranet = 'intranet',
}

export interface IAggregateStats {
Expand Down Expand Up @@ -84,6 +85,10 @@ export interface IWebhookVerifiedContent {
signingSecretKey: string
}

export interface IIntranet {
intranetIpListPath: string
}

export interface IFeatureManager {
[FeatureNames.AggregateStats]: IAggregateStats
[FeatureNames.Captcha]: ICaptcha
Expand All @@ -93,6 +98,7 @@ export interface IFeatureManager {
[FeatureNames.SpcpMyInfo]: ISpcpMyInfo
[FeatureNames.VerifiedFields]: IVerifiedFields
[FeatureNames.WebhookVerifiedContent]: IWebhookVerifiedContent
[FeatureNames.Intranet]: IIntranet
}

export interface RegisteredFeature<T extends FeatureNames> {
Expand Down
4 changes: 4 additions & 0 deletions src/loaders/express/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import { SubmissionRouter } from '../../app/modules/submission/submission.routes
import UserRouter from '../../app/modules/user/user.routes'
import { VfnRouter } from '../../app/modules/verification/verification.routes'
import apiRoutes from '../../app/routes'
import * as IntranetMiddleware from '../../app/services/intranet/intranet.middleware'
import config from '../../config/config'

import errorHandlerMiddlewares from './error-handler'
Expand Down Expand Up @@ -137,6 +138,9 @@ const loadExpressApp = async (connection: Connection) => {
// setup express-device
app.use(device.capture({ parseUserAgent: true }))

// Log intranet usage
app.use(IntranetMiddleware.logIntranetUsage)

// Mount all API endpoints
apiRoutes.forEach(function (routeFunction) {
routeFunction(app)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,18 @@ angular
'$window',
'$document',
'GTag',
'Toastr',
SubmitFormController,
])

function SubmitFormController(FormData, SpcpSession, $window, $document, GTag) {
function SubmitFormController(
FormData,
SpcpSession,
$window,
$document,
GTag,
Toastr,
) {
const vm = this

// The form attribute of the FormData object contains the form fields, logic etc
Expand All @@ -31,6 +39,14 @@ function SubmitFormController(FormData, SpcpSession, $window, $document, GTag) {
vm.myform.isTemplate = Boolean(FormData.isTemplate)
vm.myform.isPreview = Boolean(FormData.isPreview)
vm.myInfoError = Boolean(FormData.myInfoError)
if (
FormData.isIntranetUser &&
['SP', 'CP', 'MyInfo'].includes(vm.myform.authType)
) {
Toastr.permanentError(
mantariksh marked this conversation as resolved.
Show resolved Hide resolved
'SingPass/CorpPass login is not supported from WOG Intranet. Please use an Internet-enabled device to submit this form.',
)
}
vm.logoUrl = getFormLogo(vm.myform)

// Show banner content if available
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ describe('Form Controller', () => {
},
headers: {},
ip: '127.0.0.1',
get: () => this.ip,
}
})

Expand All @@ -61,6 +62,7 @@ describe('Form Controller', () => {
form: expectedForm,
spcpSession: res.locals.spcpSession,
myInfoError: res.locals.myInfoError,
isIntranetUser: false,
})
})
})
Expand Down Expand Up @@ -98,6 +100,7 @@ describe('Form Controller', () => {
form: expectedForm,
spcpSession: res.locals.spcpSession,
myInfoError: res.locals.myInfoError,
isIntranetUser: false,
})
})
})
Expand Down