Skip to content

Commit

Permalink
refactor: migrate addSpcpSessionInfo to TypeScript (#664)
Browse files Browse the repository at this point in the history
  • Loading branch information
mantariksh committed Nov 19, 2020
1 parent 51702c6 commit 4665751
Show file tree
Hide file tree
Showing 13 changed files with 214 additions and 120 deletions.
42 changes: 0 additions & 42 deletions src/app/controllers/spcp.server.controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -228,48 +228,6 @@ exports.corpPassLogin = (ndiConfig) => {
})
}

/**
* Adds session to returned JSON if form-filler is SPCP Authenticated
* @param {Object} req - Express request object
* @param {Object} res - Express response object
* @param {Object} next - Express next middleware function
*/
exports.addSpcpSessionInfo = (authClients) => {
return (req, res, next) => {
const { authType } = req.form
let authClient = authClients[authType] ? authClients[authType] : undefined
let jwtName = jwtNames[authType]
let jwt = req.cookies[jwtName]
if (authType && authClient && jwt) {
// add session info if logged in
authClient.verifyJWT(jwt, (err, payload) => {
if (err) {
// Do not specify userName to call MyInfo endpoint with if jwt is
// invalid.
// Client will inform the form-filler to log in with SingPass again.
logger.error({
message: 'Failed to verify JWT with auth client',
meta: {
action: 'addSpcpSessionInfo',
...createReqMeta(req),
},
error: err,
})
} else {
const { userName } = payload
// For use in addMyInfo middleware
res.locals.spcpSession = {
userName: userName,
}
}
return next()
})
} else {
return next()
}
}
}

/**
* Encrypt and sign verified fields if exist
* @param {Object} req - Express request object
Expand Down
2 changes: 0 additions & 2 deletions src/app/factories/spcp.factory.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ const spcpFactory = ({ isEnabled, props }) => {
passThroughSpcp: admin.passThroughSpcp,
singPassLogin: spcp.singPassLogin(ndiConfig),
corpPassLogin: spcp.corpPassLogin(ndiConfig),
addSpcpSessionInfo: spcp.addSpcpSessionInfo(authClients),
isSpcpAuthenticated: spcp.isSpcpAuthenticated(authClients),
}
} else {
Expand All @@ -81,7 +80,6 @@ const spcpFactory = ({ isEnabled, props }) => {
res.status(StatusCodes.INTERNAL_SERVER_ERROR).json({ message: errMsg }),
corpPassLogin: (req, res) =>
res.status(StatusCodes.INTERNAL_SERVER_ERROR).json({ message: errMsg }),
addSpcpSessionInfo: (req, res, next) => next(),
isSpcpAuthenticated: (req, res, next) => next(),
}
}
Expand Down
10 changes: 10 additions & 0 deletions src/app/modules/spcp/__tests__/spcp.factory.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,14 @@ describe('spcp.factory', () => {
)
const fetchLoginPageResult = await SpcpFactory.fetchLoginPage('')
const validateLoginPageResult = SpcpFactory.validateLoginPage('')
const extractJwtPayloadResult = await SpcpFactory.extractJwtPayload(
'',
AuthType.SP,
)
expect(createRedirectUrlResult._unsafeUnwrapErr()).toEqual(error)
expect(fetchLoginPageResult._unsafeUnwrapErr()).toEqual(error)
expect(validateLoginPageResult._unsafeUnwrapErr()).toEqual(error)
expect(extractJwtPayloadResult._unsafeUnwrapErr()).toEqual(error)
})

it('should return error functions when props is undefined', async () => {
Expand All @@ -46,9 +51,14 @@ describe('spcp.factory', () => {
)
const fetchLoginPageResult = await SpcpFactory.fetchLoginPage('')
const validateLoginPageResult = SpcpFactory.validateLoginPage('')
const extractJwtPayloadResult = await SpcpFactory.extractJwtPayload(
'',
AuthType.SP,
)
expect(createRedirectUrlResult._unsafeUnwrapErr()).toEqual(error)
expect(fetchLoginPageResult._unsafeUnwrapErr()).toEqual(error)
expect(validateLoginPageResult._unsafeUnwrapErr()).toEqual(error)
expect(extractJwtPayloadResult._unsafeUnwrapErr()).toEqual(error)
})

it('should call the SpcpService constructor when isEnabled is true and props is truthy', () => {
Expand Down
44 changes: 44 additions & 0 deletions src/app/modules/spcp/__tests__/spcp.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,14 @@ import {
CreateRedirectUrlError,
FetchLoginPageError,
LoginPageValidationError,
VerifyJwtError,
} from '../spcp.errors'
import { SpcpService } from '../spcp.service'

import {
MOCK_ERROR_CODE,
MOCK_ESRVCID,
MOCK_JWT,
MOCK_LOGIN_HTML,
MOCK_REDIRECT_URL,
MOCK_SERVICE_PARAMS as MOCK_PARAMS,
Expand Down Expand Up @@ -184,4 +186,46 @@ describe('spcp.service', () => {
expect(result._unsafeUnwrapErr()).toEqual(new LoginPageValidationError())
})
})

describe('extractJwtPayload', () => {
it('should return the correct payload for Singpass when JWT is valid', async () => {
const spcpService = new SpcpService(MOCK_PARAMS)
// Assumes that SP auth client was instantiated first
const mockSpClient = mocked(MockAuthClient.mock.instances[0], true)
mockSpClient.verifyJWT.mockImplementationOnce((jwt, cb) => cb(null, jwt))
const result = await spcpService.extractJwtPayload(MOCK_JWT, AuthType.SP)
expect(result._unsafeUnwrap()).toEqual(MOCK_JWT)
})

it('should return VerifyJwtError when SingPass JWT is invalid', async () => {
const spcpService = new SpcpService(MOCK_PARAMS)
// Assumes that SP auth client was instantiated first
const mockSpClient = mocked(MockAuthClient.mock.instances[0], true)
mockSpClient.verifyJWT.mockImplementationOnce((_jwt, cb) =>
cb(new Error(), null),
)
const result = await spcpService.extractJwtPayload(MOCK_JWT, AuthType.SP)
expect(result._unsafeUnwrapErr()).toEqual(new VerifyJwtError())
})

it('should return the correct payload for Corppass when JWT is valid', async () => {
const spcpService = new SpcpService(MOCK_PARAMS)
// Assumes that SP auth client was instantiated first
const mockCpClient = mocked(MockAuthClient.mock.instances[1], true)
mockCpClient.verifyJWT.mockImplementationOnce((jwt, cb) => cb(null, jwt))
const result = await spcpService.extractJwtPayload(MOCK_JWT, AuthType.CP)
expect(result._unsafeUnwrap()).toEqual(MOCK_JWT)
})

it('should return VerifyJwtError when CorpPass JWT is invalid', async () => {
const spcpService = new SpcpService(MOCK_PARAMS)
// Assumes that SP auth client was instantiated first
const mockCpClient = mocked(MockAuthClient.mock.instances[1], true)
mockCpClient.verifyJWT.mockImplementationOnce((_jwt, cb) =>
cb(new Error(), null),
)
const result = await spcpService.extractJwtPayload(MOCK_JWT, AuthType.CP)
expect(result._unsafeUnwrapErr()).toEqual(new VerifyJwtError())
})
})
})
1 change: 1 addition & 0 deletions src/app/modules/spcp/__tests__/spcp.test.constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,3 +36,4 @@ export const MOCK_REDIRECT_URL = 'redirectUrl'
export const MOCK_LOGIN_HTML = 'html'
export const MOCK_ERROR_CODE = 'errorCode'
export const MOCK_TITLE = 'title'
export const MOCK_JWT = 'jwt'
44 changes: 42 additions & 2 deletions src/app/modules/spcp/spcp.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,20 @@ import { ParamsDictionary } from 'express-serve-static-core'
import { StatusCodes } from 'http-status-codes'

import { createLoggerWithLabel } from '../../../config/logger'
import { AuthType } from '../../../types'
import { AuthType, IPopulatedForm } from '../../../types'
import { createReqMeta } from '../../utils/request'

import { SpcpFactory } from './spcp.factory'
import { LoginPageValidationResult } from './spcp.types'
import { mapRouteError } from './spcp.util'
import { extractJwt, mapRouteError } from './spcp.util'

const logger = createLoggerWithLabel(module)

// TODO (#42): remove these types when migrating away from middleware pattern
type WithForm<T> = T & {
form: IPopulatedForm
}

/**
* Generates redirect URL to Official SingPass/CorpPass log in page
* @param req - Express request object
Expand Down Expand Up @@ -77,3 +82,38 @@ export const handleValidate: RequestHandler<
return res.status(statusCode).json({ message: errorMessage })
})
}

/**
* Adds session to returned JSON if form-filler is SPCP Authenticated
* @param req - Express request object
* @param res - Express response object
* @param next - Express next middleware function
*/
export const addSpcpSessionInfo: RequestHandler<ParamsDictionary> = async (
req,
res,
next,
) => {
const { authType } = (req as WithForm<typeof req>).form
if (!authType) return next()

const jwt = extractJwt(req.cookies, authType)
if (!jwt) return next()

return SpcpFactory.extractJwtPayload(jwt, authType)
.map(({ userName }) => {
res.locals.spcpSession = { userName }
return next()
})
.mapErr((error) => {
logger.error({
message: 'Failed to verify JWT with auth client',
meta: {
action: 'addSpcpSessionInfo',
...createReqMeta(req),
},
error,
})
return next()
})
}
9 changes: 9 additions & 0 deletions src/app/modules/spcp/spcp.errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,3 +34,12 @@ export class LoginPageValidationError extends ApplicationError {
super(message)
}
}

/**
* Invalid JWT.
*/
export class VerifyJwtError extends ApplicationError {
constructor(message = 'Invalid JWT') {
super(message)
}
}
8 changes: 7 additions & 1 deletion src/app/modules/spcp/spcp.factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,10 @@ import {
FetchLoginPageError,
InvalidAuthTypeError,
LoginPageValidationError,
VerifyJwtError,
} from './spcp.errors'
import { SpcpService } from './spcp.service'
import { LoginPageValidationResult } from './spcp.types'
import { JwtPayload, LoginPageValidationResult } from './spcp.types'

interface ISpcpFactory {
createRedirectUrl(
Expand All @@ -34,6 +35,10 @@ interface ISpcpFactory {
LoginPageValidationResult,
LoginPageValidationError | MissingFeatureError
>
extractJwtPayload(
jwt: string,
authType: AuthType,
): ResultAsync<JwtPayload, VerifyJwtError | InvalidAuthTypeError>
}

export const createSpcpFactory = ({
Expand All @@ -46,6 +51,7 @@ export const createSpcpFactory = ({
createRedirectUrl: () => err(error),
fetchLoginPage: () => errAsync(error),
validateLoginPage: () => err(error),
extractJwtPayload: () => errAsync(error),
}
}
return new SpcpService(props)
Expand Down
58 changes: 55 additions & 3 deletions src/app/modules/spcp/spcp.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import SPCPAuthClient from '@opengovsg/spcp-auth-client'
import axios from 'axios'
import fs from 'fs'
import { StatusCodes } from 'http-status-codes'
import { err, ok, Result, ResultAsync } from 'neverthrow'
import { err, errAsync, ok, Result, ResultAsync } from 'neverthrow'

import { ISpcpMyInfo } from '../../../config/feature-manager'
import { createLoggerWithLabel } from '../../../config/logger'
Expand All @@ -13,9 +13,10 @@ import {
FetchLoginPageError,
InvalidAuthTypeError,
LoginPageValidationError,
VerifyJwtError,
} from './spcp.errors'
import { LoginPageValidationResult } from './spcp.types'
import { getSubstringBetween } from './spcp.util'
import { JwtPayload, LoginPageValidationResult } from './spcp.types'
import { getSubstringBetween, verifyJwtPromise } from './spcp.util'

const logger = createLoggerWithLabel(module)
const LOGIN_PAGE_HEADERS =
Expand Down Expand Up @@ -48,6 +49,13 @@ export class SpcpService {
})
}

/**
* Create the URL to which the client should be redirected for Singpass/
* Corppass login.
* @param authType 'SP' or 'CP'
* @param target The target URL which will become the SPCP RelayState
* @param esrvcId SP/CP e-service ID
*/
createRedirectUrl(
authType: AuthType.SP | AuthType.CP,
target: string,
Expand Down Expand Up @@ -90,6 +98,10 @@ export class SpcpService {
}
}

/**
* Fetches the HTML of the given URL.
* @param redirectUrl URL from which to obtain the HTML
*/
fetchLoginPage(
redirectUrl: string,
): ResultAsync<string, FetchLoginPageError> {
Expand Down Expand Up @@ -118,6 +130,10 @@ export class SpcpService {
)
}

/**
* Validates that the login page does not have an error.
* @param loginHtml The HTML of the page to validate
*/
validateLoginPage(
loginHtml: string,
): Result<LoginPageValidationResult, LoginPageValidationError> {
Expand Down Expand Up @@ -152,4 +168,40 @@ export class SpcpService {
return ok({ isValid: false, errorCode })
}
}

/**
* Verifies a JWT and extracts its payload.
* @param jwt The contents of the JWT cookie
* @param authType 'SP' or 'CP'
*/
extractJwtPayload(
jwt: string,
authType: AuthType.SP | AuthType.CP,
): ResultAsync<JwtPayload, VerifyJwtError | InvalidAuthTypeError> {
let authClient: SPCPAuthClient
switch (authType) {
case AuthType.SP:
authClient = this.#singpassAuthClient
break
case AuthType.CP:
authClient = this.#corppassAuthClient
break
default:
return errAsync(new InvalidAuthTypeError(authType))
}
return ResultAsync.fromPromise(
verifyJwtPromise(authClient, jwt),
(error) => {
logger.error({
message: 'Failed to verify JWT with auth client',
meta: {
action: 'extractPayload',
authType,
},
error,
})
return new VerifyJwtError()
},
)
}
}
13 changes: 13 additions & 0 deletions src/app/modules/spcp/spcp.types.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,16 @@
export enum JwtName {
SP = 'jwtSp',
CP = 'jwtCp',
}

export type LoginPageValidationResult =
| { isValid: true }
| { isValid: false; errorCode: string | null }

export type SpcpCookies = Partial<Record<JwtName, string>>

export type JwtPayload = {
userName: string
userInfo?: string
rememberMe: boolean
}
Loading

0 comments on commit 4665751

Please sign in to comment.