diff --git a/src/app/controllers/spcp.server.controller.js b/src/app/controllers/spcp.server.controller.js index d56145bd0e..a011a1c312 100644 --- a/src/app/controllers/spcp.server.controller.js +++ b/src/app/controllers/spcp.server.controller.js @@ -7,7 +7,6 @@ const { isEmpty } = require('lodash') const mongoose = require('mongoose') const crypto = require('crypto') const { StatusCodes } = require('http-status-codes') -const axios = require('axios') const { createReqMeta } = require('../utils/request') const logger = require('../../config/logger').createLoggerWithLabel(module) @@ -202,104 +201,6 @@ const handleOOBAuthenticationWith = (ndiConfig, authType, extractUser) => { } } -/** - * Generates redirect URL to Official SingPass/CorpPass log in page - * @param {Object} req - Express request object - * @param {Object} res - Express response object - * @param {function} next - Express next function - */ -exports.createSpcpRedirectURL = (authClients) => { - return (req, res, next) => { - const { target, authType, esrvcId } = req.query - let authClient = authClients[authType] ? authClients[authType] : undefined - if (target && authClient && esrvcId) { - req.redirectURL = authClient.createRedirectURL(target, esrvcId) - return next() - } else { - return res - .status(StatusCodes.BAD_REQUEST) - .json({ message: 'Redirect URL malformed' }) - } - } -} - -const getSubstringBetween = (text, markerStart, markerEnd) => { - const start = text.indexOf(markerStart) - if (start === -1) { - return null - } else { - const end = text.indexOf(markerEnd, start) - return end === -1 ? null : text.substring(start + markerStart.length, end) - } -} - -exports.validateESrvcId = (req, res) => { - let { redirectURL } = req - let validateUrl = redirectURL - - axios - .get(validateUrl, { - headers: { - Accept: - 'text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8,application/signed-exchange;v=b3', - }, - timeout: 10000, // 10 seconds - // Throw error if not status 200. - validateStatus: (status) => status === StatusCodes.OK, - }) - .then(({ data }) => { - // The successful login page should have the title 'SingPass Login' - // The error page should have the title 'SingPass - System Error Page' - const title = getSubstringBetween(data, '', '') - if (title === null) { - logger.error({ - message: 'Could not find title', - meta: { - action: 'validateESrvcId', - ...createReqMeta(req), - redirectUrl: redirectURL, - data, - }, - }) - return res.status(StatusCodes.BAD_GATEWAY).json({ - message: 'Singpass returned incomprehensible content', - }) - } - if (title.indexOf('Error') === -1) { - return res.status(StatusCodes.OK).json({ - isValid: true, - }) - } - - // The error page should have text like 'System Code: 138' - const errorCode = getSubstringBetween( - data, - 'System Code: ', - '', - ) - return res.status(StatusCodes.OK).json({ - isValid: false, - errorCode, - }) - }) - .catch((err) => { - const { statusCode } = err.response || {} - logger.error({ - message: 'Could not contact singpass to validate eservice id', - meta: { - action: 'validateESrvcId', - ...createReqMeta(req), - redirectUrl: redirectURL, - statusCode, - }, - error: err, - }) - return res.status(StatusCodes.SERVICE_UNAVAILABLE).json({ - message: 'Failed to contact Singpass', - }) - }) -} - /** * Assertion Consumer Endpoint - Authenticates form-filler with SingPass and creates session * @param {Object} req - Express request object diff --git a/src/app/factories/spcp.factory.js b/src/app/factories/spcp.factory.js index d6e148d68a..d33141fc9f 100644 --- a/src/app/factories/spcp.factory.js +++ b/src/app/factories/spcp.factory.js @@ -71,8 +71,6 @@ const spcpFactory = ({ isEnabled, props }) => { corpPassLogin: spcp.corpPassLogin(ndiConfig), addSpcpSessionInfo: spcp.addSpcpSessionInfo(authClients), isSpcpAuthenticated: spcp.isSpcpAuthenticated(authClients), - createSpcpRedirectURL: spcp.createSpcpRedirectURL(authClients), - validateESrvcId: spcp.validateESrvcId, } } else { const errMsg = 'SPCP/MyInfo feature is not enabled' @@ -85,9 +83,6 @@ const spcpFactory = ({ isEnabled, props }) => { res.status(StatusCodes.INTERNAL_SERVER_ERROR).json({ message: errMsg }), addSpcpSessionInfo: (req, res, next) => next(), isSpcpAuthenticated: (req, res, next) => next(), - createSpcpRedirectURL: (req, res, next) => next(), - validateESrvcId: (req, res) => - res.status(StatusCodes.INTERNAL_SERVER_ERROR).json({ message: errMsg }), } } } diff --git a/src/app/modules/spcp/__tests__/spcp.controller.spec.ts b/src/app/modules/spcp/__tests__/spcp.controller.spec.ts index 433a8646c6..d6b489a1b2 100644 --- a/src/app/modules/spcp/__tests__/spcp.controller.spec.ts +++ b/src/app/modules/spcp/__tests__/spcp.controller.spec.ts @@ -1,4 +1,4 @@ -import { err, ok } from 'neverthrow' +import { err, errAsync, ok, okAsync } from 'neverthrow' import { mocked } from 'ts-jest/utils' import { AuthType } from 'src/types' @@ -6,11 +6,17 @@ import { AuthType } from 'src/types' import expressHandler from 'tests/unit/backend/helpers/jest-express' import * as SpcpController from '../spcp.controller' -import { CreateRedirectUrlError } from '../spcp.errors' +import { + CreateRedirectUrlError, + FetchLoginPageError, + LoginPageValidationError, +} from '../spcp.errors' import { SpcpFactory } from '../spcp.factory' import { + MOCK_ERROR_CODE, MOCK_ESRVCID, + MOCK_LOGIN_HTML, MOCK_REDIRECT_URL, MOCK_TARGET, } from './spcp.test.constants' @@ -19,49 +25,186 @@ jest.mock('../spcp.factory') const MockSpcpFactory = mocked(SpcpFactory, true) const MOCK_RESPONSE = expressHandler.mockResponse() +const MOCK_REDIRECT_REQ = expressHandler.mockRequest({ + query: { + target: MOCK_TARGET, + authType: AuthType.SP, + esrvcId: MOCK_ESRVCID, + }, +}) describe('spcp.controller', () => { beforeEach(() => jest.clearAllMocks()) describe('handleRedirect', () => { it('should return the redirect URL correctly', () => { - const mockReq = expressHandler.mockRequest({ - query: { - target: MOCK_TARGET, - authType: AuthType.SP, - esrvcId: MOCK_ESRVCID, - }, - }) MockSpcpFactory.createRedirectUrl.mockReturnValueOnce( ok(MOCK_REDIRECT_URL), ) - SpcpController.handleRedirect(mockReq, MOCK_RESPONSE, jest.fn()) + SpcpController.handleRedirect(MOCK_REDIRECT_REQ, MOCK_RESPONSE, jest.fn()) - expect(MOCK_RESPONSE.status).toHaveBeenLastCalledWith(200) + expect(MockSpcpFactory.createRedirectUrl).toHaveBeenCalledWith( + AuthType.SP, + MOCK_TARGET, + MOCK_ESRVCID, + ) + expect(MOCK_RESPONSE.status).toHaveBeenCalledWith(200) expect(MOCK_RESPONSE.json).toHaveBeenCalledWith({ redirectURL: MOCK_REDIRECT_URL, }) }) it('should return 500 if auth client throws an error', () => { - const mockReq = expressHandler.mockRequest({ - query: { - target: MOCK_TARGET, - authType: AuthType.SP, - esrvcId: MOCK_ESRVCID, - }, - }) MockSpcpFactory.createRedirectUrl.mockReturnValueOnce( err(new CreateRedirectUrlError()), ) - SpcpController.handleRedirect(mockReq, MOCK_RESPONSE, jest.fn()) + SpcpController.handleRedirect(MOCK_REDIRECT_REQ, MOCK_RESPONSE, jest.fn()) - expect(MOCK_RESPONSE.status).toHaveBeenLastCalledWith(500) + expect(MockSpcpFactory.createRedirectUrl).toHaveBeenCalledWith( + AuthType.SP, + MOCK_TARGET, + MOCK_ESRVCID, + ) + expect(MOCK_RESPONSE.status).toHaveBeenCalledWith(500) expect(MOCK_RESPONSE.json).toHaveBeenCalledWith({ message: 'Sorry, something went wrong. Please try again.', }) }) }) + + describe('handleValidate', () => { + it('should return 200 with isValid true if validation passes', async () => { + MockSpcpFactory.createRedirectUrl.mockReturnValueOnce( + ok(MOCK_REDIRECT_URL), + ) + MockSpcpFactory.fetchLoginPage.mockReturnValueOnce( + okAsync(MOCK_LOGIN_HTML), + ) + MockSpcpFactory.validateLoginPage.mockReturnValueOnce( + ok({ isValid: true }), + ) + + await SpcpController.handleValidate( + MOCK_REDIRECT_REQ, + MOCK_RESPONSE, + jest.fn(), + ) + + expect(MockSpcpFactory.createRedirectUrl).toHaveBeenCalledWith( + AuthType.SP, + MOCK_TARGET, + MOCK_ESRVCID, + ) + expect(MockSpcpFactory.fetchLoginPage).toHaveBeenCalledWith( + MOCK_REDIRECT_URL, + ) + expect(MockSpcpFactory.validateLoginPage).toHaveBeenCalledWith( + MOCK_LOGIN_HTML, + ) + expect(MOCK_RESPONSE.status).toHaveBeenCalledWith(200) + expect(MOCK_RESPONSE.json).toHaveBeenCalledWith({ + isValid: true, + }) + }) + + it('should return 200 with isValid false if validation fails', async () => { + MockSpcpFactory.createRedirectUrl.mockReturnValueOnce( + ok(MOCK_REDIRECT_URL), + ) + MockSpcpFactory.fetchLoginPage.mockReturnValueOnce( + okAsync(MOCK_LOGIN_HTML), + ) + MockSpcpFactory.validateLoginPage.mockReturnValueOnce( + ok({ isValid: false, errorCode: MOCK_ERROR_CODE }), + ) + + await SpcpController.handleValidate( + MOCK_REDIRECT_REQ, + MOCK_RESPONSE, + jest.fn(), + ) + + expect(MockSpcpFactory.createRedirectUrl).toHaveBeenCalledWith( + AuthType.SP, + MOCK_TARGET, + MOCK_ESRVCID, + ) + expect(MockSpcpFactory.fetchLoginPage).toHaveBeenCalledWith( + MOCK_REDIRECT_URL, + ) + expect(MockSpcpFactory.validateLoginPage).toHaveBeenCalledWith( + MOCK_LOGIN_HTML, + ) + expect(MOCK_RESPONSE.status).toHaveBeenCalledWith(200) + expect(MOCK_RESPONSE.json).toHaveBeenCalledWith({ + isValid: false, + errorCode: MOCK_ERROR_CODE, + }) + }) + + it('should return 503 when FetchLoginPageError occurs', async () => { + MockSpcpFactory.createRedirectUrl.mockReturnValueOnce( + ok(MOCK_REDIRECT_URL), + ) + MockSpcpFactory.fetchLoginPage.mockReturnValueOnce( + errAsync(new FetchLoginPageError()), + ) + + await SpcpController.handleValidate( + MOCK_REDIRECT_REQ, + MOCK_RESPONSE, + jest.fn(), + ) + + expect(MockSpcpFactory.createRedirectUrl).toHaveBeenCalledWith( + AuthType.SP, + MOCK_TARGET, + MOCK_ESRVCID, + ) + expect(MockSpcpFactory.fetchLoginPage).toHaveBeenCalledWith( + MOCK_REDIRECT_URL, + ) + expect(MockSpcpFactory.validateLoginPage).not.toHaveBeenCalled() + expect(MOCK_RESPONSE.status).toHaveBeenCalledWith(503) + expect(MOCK_RESPONSE.json).toHaveBeenCalledWith({ + message: 'Failed to contact SingPass. Please try again.', + }) + }) + + it('should return 502 when LoginPageValidationError occurs', async () => { + MockSpcpFactory.createRedirectUrl.mockReturnValueOnce( + ok(MOCK_REDIRECT_URL), + ) + MockSpcpFactory.fetchLoginPage.mockReturnValueOnce( + okAsync(MOCK_LOGIN_HTML), + ) + MockSpcpFactory.validateLoginPage.mockReturnValueOnce( + err(new LoginPageValidationError()), + ) + + await SpcpController.handleValidate( + MOCK_REDIRECT_REQ, + MOCK_RESPONSE, + jest.fn(), + ) + + expect(MockSpcpFactory.createRedirectUrl).toHaveBeenCalledWith( + AuthType.SP, + MOCK_TARGET, + MOCK_ESRVCID, + ) + expect(MockSpcpFactory.fetchLoginPage).toHaveBeenCalledWith( + MOCK_REDIRECT_URL, + ) + expect(MockSpcpFactory.validateLoginPage).toHaveBeenCalledWith( + MOCK_LOGIN_HTML, + ) + expect(MOCK_RESPONSE.status).toHaveBeenCalledWith(502) + expect(MOCK_RESPONSE.json).toHaveBeenCalledWith({ + message: 'Error while contacting SingPass. Please try again.', + }) + }) + }) }) diff --git a/src/app/modules/spcp/__tests__/spcp.factory.spec.ts b/src/app/modules/spcp/__tests__/spcp.factory.spec.ts index 4ab7b02daa..a9c2563eaf 100644 --- a/src/app/modules/spcp/__tests__/spcp.factory.spec.ts +++ b/src/app/modules/spcp/__tests__/spcp.factory.spec.ts @@ -13,34 +13,42 @@ jest.mock('../spcp.service') const MockSpcpService = mocked(SpcpService, true) describe('spcp.factory', () => { - it('should return error functions when isEnabled is false', () => { - const MyInfoFactory = createSpcpFactory({ + it('should return error functions when isEnabled is false', async () => { + const SpcpFactory = createSpcpFactory({ isEnabled: false, props: {} as ISpcpMyInfo, }) const error = new MissingFeatureError(FeatureNames.SpcpMyInfo) - const createRedirectUrlResult = MyInfoFactory.createRedirectUrl( + const createRedirectUrlResult = SpcpFactory.createRedirectUrl( AuthType.SP, '', '', ) + const fetchLoginPageResult = await SpcpFactory.fetchLoginPage('') + const validateLoginPageResult = SpcpFactory.validateLoginPage('') expect(createRedirectUrlResult._unsafeUnwrapErr()).toEqual(error) + expect(fetchLoginPageResult._unsafeUnwrapErr()).toEqual(error) + expect(validateLoginPageResult._unsafeUnwrapErr()).toEqual(error) }) - it('should return error functions when props is undefined', () => { - const MyInfoFactory = createSpcpFactory({ + it('should return error functions when props is undefined', async () => { + const SpcpFactory = createSpcpFactory({ isEnabled: true, props: undefined, }) const error = new Error( 'spcp-myinfo is not activated, but a feature-specific function was called.', ) - const createRedirectUrlResult = MyInfoFactory.createRedirectUrl( + const createRedirectUrlResult = SpcpFactory.createRedirectUrl( AuthType.SP, '', '', ) + const fetchLoginPageResult = await SpcpFactory.fetchLoginPage('') + const validateLoginPageResult = SpcpFactory.validateLoginPage('') expect(createRedirectUrlResult._unsafeUnwrapErr()).toEqual(error) + expect(fetchLoginPageResult._unsafeUnwrapErr()).toEqual(error) + expect(validateLoginPageResult._unsafeUnwrapErr()).toEqual(error) }) it('should call the SpcpService constructor when isEnabled is true and props is truthy', () => { diff --git a/src/app/modules/spcp/__tests__/spcp.routes.spec.ts b/src/app/modules/spcp/__tests__/spcp.routes.spec.ts index 5d309b21bf..6fea875061 100644 --- a/src/app/modules/spcp/__tests__/spcp.routes.spec.ts +++ b/src/app/modules/spcp/__tests__/spcp.routes.spec.ts @@ -1,15 +1,23 @@ +import axios from 'axios' import session, { Session } from 'supertest-session' +import { mocked } from 'ts-jest/utils' import { setupApp } from 'tests/integration/helpers/express-setup' import { buildCelebrateError } from 'tests/unit/backend/helpers/celebrate' import { SpcpRouter } from '../spcp.routes' -import { MOCK_ESRVCID, MOCK_TARGET } from './spcp.test.constants' +import { + MOCK_ERROR_CODE, + MOCK_ESRVCID, + MOCK_TARGET, +} from './spcp.test.constants' +jest.mock('axios') +const MockAxios = mocked(axios, true) const spcpApp = setupApp('/spcp', SpcpRouter) -describe('GET /spcp/redirect', () => { +describe('spcp.routes', () => { let request: Session beforeEach(() => { @@ -17,76 +25,240 @@ describe('GET /spcp/redirect', () => { request = session(spcpApp) }) - it('should return 400 when authType is not provided as a query param', async () => { - const response = await request - .get('/spcp/redirect') - .query({ target: MOCK_TARGET, esrvcId: MOCK_ESRVCID }) + describe('GET /spcp/redirect', () => { + const ROUTE = '/spcp/redirect' - expect(response.status).toBe(400) - expect(response.body).toEqual( - buildCelebrateError({ query: { key: 'authType' } }), - ) - }) + it('should return 400 when authType is not provided as a query param', async () => { + const response = await request + .get(ROUTE) + .query({ target: MOCK_TARGET, esrvcId: MOCK_ESRVCID }) - it('should return 400 when authType is malformed', async () => { - const response = await request.get('/spcp/redirect').query({ - authType: 'malformed', - target: MOCK_TARGET, - esrvcId: MOCK_ESRVCID, - }) - - expect(response.status).toBe(400) - expect(response.body).toEqual( - buildCelebrateError({ - query: { - key: 'authType', - message: '"authType" must be one of [SP, CP]', - }, - }), - ) - }) + expect(response.status).toBe(400) + expect(response.body).toEqual( + buildCelebrateError({ query: { key: 'authType' } }), + ) + }) - it('should return 400 when target is not provided as a query param', async () => { - const response = await request - .get('/spcp/redirect') - .query({ authType: 'SP', esrvcId: MOCK_ESRVCID }) + it('should return 400 when authType is malformed', async () => { + const response = await request.get(ROUTE).query({ + authType: 'malformed', + target: MOCK_TARGET, + esrvcId: MOCK_ESRVCID, + }) - expect(response.status).toBe(400) - expect(response.body).toEqual( - buildCelebrateError({ query: { key: 'target' } }), - ) - }) + expect(response.status).toBe(400) + expect(response.body).toEqual( + buildCelebrateError({ + query: { + key: 'authType', + message: '"authType" must be one of [SP, CP]', + }, + }), + ) + }) - it('should return 400 when esrvcId is not provided as a query param', async () => { - const response = await request - .get('/spcp/redirect') - .query({ authType: 'CP', target: MOCK_TARGET }) + it('should return 400 when target is not provided as a query param', async () => { + const response = await request + .get(ROUTE) + .query({ authType: 'SP', esrvcId: MOCK_ESRVCID }) - expect(response.status).toBe(400) - expect(response.body).toEqual( - buildCelebrateError({ query: { key: 'esrvcId' } }), - ) - }) + expect(response.status).toBe(400) + expect(response.body).toEqual( + buildCelebrateError({ query: { key: 'target' } }), + ) + }) + + it('should return 400 when esrvcId is not provided as a query param', async () => { + const response = await request + .get(ROUTE) + .query({ authType: 'CP', target: MOCK_TARGET }) + + expect(response.status).toBe(400) + expect(response.body).toEqual( + buildCelebrateError({ query: { key: 'esrvcId' } }), + ) + }) - it('should return 200 with the redirect URL when Singpass request is valid', async () => { - const response = await request - .get('/spcp/redirect') - .query({ authType: 'SP', target: MOCK_TARGET, esrvcId: MOCK_ESRVCID }) + it('should return 200 with the redirect URL when Singpass request is valid', async () => { + const response = await request + .get(ROUTE) + .query({ authType: 'SP', target: MOCK_TARGET, esrvcId: MOCK_ESRVCID }) - expect(response.status).toBe(200) - expect(response.body).toEqual({ - redirectURL: expect.any(String), + expect(response.status).toBe(200) + expect(response.body).toEqual({ + redirectURL: expect.any(String), + }) + }) + + it('should return 200 with the redirect URL when Corppass request is valid', async () => { + const response = await request + .get('/spcp/redirect') + .query({ authType: 'CP', target: MOCK_TARGET, esrvcId: MOCK_ESRVCID }) + + expect(response.status).toBe(200) + expect(response.body).toEqual({ + redirectURL: expect.any(String), + }) }) }) - it('should return 200 with the redirect URL when Corppass request is valid', async () => { - const response = await request - .get('/spcp/redirect') - .query({ authType: 'CP', target: MOCK_TARGET, esrvcId: MOCK_ESRVCID }) + describe('GET /spcp/validate', () => { + const ROUTE = '/spcp/validate' + it('should return 400 when authType is not provided as a query param', async () => { + const response = await request + .get(ROUTE) + .query({ target: MOCK_TARGET, esrvcId: MOCK_ESRVCID }) + + expect(response.status).toBe(400) + expect(response.body).toEqual( + buildCelebrateError({ query: { key: 'authType' } }), + ) + }) + + it('should return 400 when authType is malformed', async () => { + const response = await request.get(ROUTE).query({ + authType: 'malformed', + target: MOCK_TARGET, + esrvcId: MOCK_ESRVCID, + }) + + expect(response.status).toBe(400) + expect(response.body).toEqual( + buildCelebrateError({ + query: { + key: 'authType', + message: '"authType" must be one of [SP, CP]', + }, + }), + ) + }) + + it('should return 400 when target is not provided as a query param', async () => { + const response = await request + .get(ROUTE) + .query({ authType: 'SP', esrvcId: MOCK_ESRVCID }) + + expect(response.status).toBe(400) + expect(response.body).toEqual( + buildCelebrateError({ query: { key: 'target' } }), + ) + }) + + it('should return 400 when esrvcId is not provided as a query param', async () => { + const response = await request + .get(ROUTE) + .query({ authType: 'CP', target: MOCK_TARGET }) + + expect(response.status).toBe(400) + expect(response.body).toEqual( + buildCelebrateError({ query: { key: 'esrvcId' } }), + ) + }) + + it('should return 200 with isValid true when Singpass request is valid', async () => { + MockAxios.get.mockResolvedValueOnce({ data: 'Title' }) + const response = await request + .get(ROUTE) + .query({ authType: 'SP', target: MOCK_TARGET, esrvcId: MOCK_ESRVCID }) + + expect(response.status).toBe(200) + expect(response.body).toEqual({ + isValid: true, + }) + }) + + it('should return 200 with isValid true when Corppass request is valid', async () => { + MockAxios.get.mockResolvedValueOnce({ data: 'Title' }) + const response = await request + .get(ROUTE) + .query({ authType: 'CP', target: MOCK_TARGET, esrvcId: MOCK_ESRVCID }) + + expect(response.status).toBe(200) + expect(response.body).toEqual({ + isValid: true, + }) + }) + + it('should return 200 with isValid false and errorCode when Singpass request is invalid', async () => { + MockAxios.get.mockResolvedValueOnce({ + data: `ErrorSystem Code: ${MOCK_ERROR_CODE}`, + }) + const response = await request + .get(ROUTE) + .query({ authType: 'SP', target: MOCK_TARGET, esrvcId: MOCK_ESRVCID }) + + expect(response.status).toBe(200) + expect(response.body).toEqual({ + isValid: false, + errorCode: MOCK_ERROR_CODE, + }) + }) + + it('should return 200 with isValid false and errorCode when Corppass request is invalid', async () => { + MockAxios.get.mockResolvedValueOnce({ + data: `ErrorSystem Code: ${MOCK_ERROR_CODE}`, + }) + const response = await request + .get(ROUTE) + .query({ authType: 'CP', target: MOCK_TARGET, esrvcId: MOCK_ESRVCID }) + + expect(response.status).toBe(200) + expect(response.body).toEqual({ + isValid: false, + errorCode: MOCK_ERROR_CODE, + }) + }) + + it('should return 503 with error message when Singpass server request errors', async () => { + MockAxios.get.mockRejectedValueOnce('') + const response = await request + .get(ROUTE) + .query({ authType: 'SP', target: MOCK_TARGET, esrvcId: MOCK_ESRVCID }) + + expect(response.status).toBe(503) + expect(response.body).toEqual({ + message: 'Failed to contact SingPass. Please try again.', + }) + }) + + it('should return 503 with error message when Corppass server request errors', async () => { + MockAxios.get.mockRejectedValueOnce('') + const response = await request + .get(ROUTE) + .query({ authType: 'CP', target: MOCK_TARGET, esrvcId: MOCK_ESRVCID }) + + expect(response.status).toBe(503) + expect(response.body).toEqual({ + message: 'Failed to contact SingPass. Please try again.', + }) + }) + + it('should return 502 with error message when Singpass server response cannot be parsed', async () => { + MockAxios.get.mockResolvedValueOnce({ + data: `mock`, + }) + const response = await request + .get(ROUTE) + .query({ authType: 'SP', target: MOCK_TARGET, esrvcId: MOCK_ESRVCID }) + + expect(response.status).toBe(502) + expect(response.body).toEqual({ + message: 'Error while contacting SingPass. Please try again.', + }) + }) + + it('should return 502 with error message when Corppass server response cannot be parsed', async () => { + MockAxios.get.mockResolvedValueOnce({ + data: `mock`, + }) + const response = await request + .get(ROUTE) + .query({ authType: 'CP', target: MOCK_TARGET, esrvcId: MOCK_ESRVCID }) - expect(response.status).toBe(200) - expect(response.body).toEqual({ - redirectURL: expect.any(String), + expect(response.status).toBe(502) + expect(response.body).toEqual({ + message: 'Error while contacting SingPass. Please try again.', + }) }) }) }) diff --git a/src/app/modules/spcp/__tests__/spcp.service.spec.ts b/src/app/modules/spcp/__tests__/spcp.service.spec.ts index 99ed1fdf7e..2b195dfc64 100644 --- a/src/app/modules/spcp/__tests__/spcp.service.spec.ts +++ b/src/app/modules/spcp/__tests__/spcp.service.spec.ts @@ -1,16 +1,24 @@ import SPCPAuthClient from '@opengovsg/spcp-auth-client' +import axios from 'axios' import { mocked } from 'ts-jest/utils' import { AuthType } from 'src/types' -import { CreateRedirectUrlError } from '../spcp.errors' +import { + CreateRedirectUrlError, + FetchLoginPageError, + LoginPageValidationError, +} from '../spcp.errors' import { SpcpService } from '../spcp.service' import { + MOCK_ERROR_CODE, MOCK_ESRVCID, + MOCK_LOGIN_HTML, MOCK_REDIRECT_URL, MOCK_SERVICE_PARAMS as MOCK_PARAMS, MOCK_TARGET, + MOCK_TITLE, } from './spcp.test.constants' jest.mock('@opengovsg/spcp-auth-client') @@ -18,6 +26,8 @@ const MockAuthClient = mocked(SPCPAuthClient, true) jest.mock('fs', () => ({ readFileSync: jest.fn().mockImplementation((v) => v), })) +jest.mock('axios') +const MockAxios = mocked(axios, true) describe('spcp.service', () => { beforeEach(() => jest.clearAllMocks()) @@ -106,4 +116,72 @@ describe('spcp.service', () => { ) }) }) + + describe('fetchLoginPage', () => { + it('should GET the correct URL and return the response when request succeeds', async () => { + const spcpService = new SpcpService(MOCK_PARAMS) + MockAxios.get.mockResolvedValueOnce({ + data: MOCK_LOGIN_HTML, + }) + + const result = await spcpService.fetchLoginPage(MOCK_REDIRECT_URL) + + expect(MockAxios.get).toHaveBeenCalledWith( + MOCK_REDIRECT_URL, + expect.objectContaining({ + headers: { + Accept: + 'text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8,application/signed-exchange;v=b3', + }, + timeout: 10000, + }), + ) + expect(result._unsafeUnwrap()).toBe(MOCK_LOGIN_HTML) + }) + + it('should return FetchLoginPageError when request fails', async () => { + const spcpService = new SpcpService(MOCK_PARAMS) + MockAxios.get.mockRejectedValueOnce('') + + const result = await spcpService.fetchLoginPage(MOCK_REDIRECT_URL) + + expect(MockAxios.get).toHaveBeenCalledWith( + MOCK_REDIRECT_URL, + expect.objectContaining({ + headers: { + Accept: + 'text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8,application/signed-exchange;v=b3', + }, + timeout: 10000, + }), + ) + expect(result._unsafeUnwrapErr()).toEqual(new FetchLoginPageError()) + }) + }) + + describe('validateLoginPage', () => { + it('should return null when there is a title and no error', () => { + const spcpService = new SpcpService(MOCK_PARAMS) + const mockHtml = `${MOCK_TITLE}` + const result = spcpService.validateLoginPage(mockHtml) + expect(result._unsafeUnwrap()).toEqual({ isValid: true }) + }) + + it('should return error code when there is error in title', () => { + const spcpService = new SpcpService(MOCK_PARAMS) + const mockHtml = `ErrorSystem Code: ${MOCK_ERROR_CODE}` + const result = spcpService.validateLoginPage(mockHtml) + expect(result._unsafeUnwrap()).toEqual({ + isValid: false, + errorCode: MOCK_ERROR_CODE, + }) + }) + + it('should return LoginPageValidationError when there is no title', () => { + const spcpService = new SpcpService(MOCK_PARAMS) + const mockHtml = 'mock' + const result = spcpService.validateLoginPage(mockHtml) + expect(result._unsafeUnwrapErr()).toEqual(new LoginPageValidationError()) + }) + }) }) diff --git a/src/app/modules/spcp/__tests__/spcp.test.constants.ts b/src/app/modules/spcp/__tests__/spcp.test.constants.ts index 25dfc0a25e..9949d45137 100644 --- a/src/app/modules/spcp/__tests__/spcp.test.constants.ts +++ b/src/app/modules/spcp/__tests__/spcp.test.constants.ts @@ -33,3 +33,6 @@ export const MOCK_SERVICE_PARAMS: ISpcpMyInfo = { export const MOCK_ESRVCID = 'eServiceId' export const MOCK_TARGET = new ObjectId().toHexString() export const MOCK_REDIRECT_URL = 'redirectUrl' +export const MOCK_LOGIN_HTML = 'html' +export const MOCK_ERROR_CODE = 'errorCode' +export const MOCK_TITLE = 'title' diff --git a/src/app/modules/spcp/spcp.controller.ts b/src/app/modules/spcp/spcp.controller.ts index 43616cba0e..b148597176 100644 --- a/src/app/modules/spcp/spcp.controller.ts +++ b/src/app/modules/spcp/spcp.controller.ts @@ -1,10 +1,13 @@ import { RequestHandler } from 'express' +import { ParamsDictionary } from 'express-serve-static-core' import { StatusCodes } from 'http-status-codes' import { createLoggerWithLabel } from '../../../config/logger' import { AuthType } from '../../../types' +import { createReqMeta } from '../../utils/request' import { SpcpFactory } from './spcp.factory' +import { LoginPageValidationResult } from './spcp.types' import { mapRouteError } from './spcp.util' const logger = createLoggerWithLabel(module) @@ -15,13 +18,13 @@ const logger = createLoggerWithLabel(module) * @param res - Express response object */ export const handleRedirect: RequestHandler< - unknown, - unknown, + ParamsDictionary, + { redirectURL: string } | { message: string }, unknown, { authType: AuthType; target: string; esrvcId: string } > = (req, res) => { const { target, authType, esrvcId } = req.query - SpcpFactory.createRedirectUrl(authType, target, esrvcId) + return SpcpFactory.createRedirectUrl(authType, target, esrvcId) .map((redirectURL) => { return res.status(StatusCodes.OK).json({ redirectURL }) }) @@ -30,6 +33,40 @@ export const handleRedirect: RequestHandler< message: 'Error while creating redirect URL', meta: { action: 'handleRedirect', + ...createReqMeta(req), + authType, + target, + esrvcId, + }, + error, + }) + const { statusCode, errorMessage } = mapRouteError(error) + return res.status(statusCode).json({ message: errorMessage }) + }) +} + +/** + * Validates the given e-service ID. + * @param req - Express request object + * @param res - Express response object + */ +export const handleValidate: RequestHandler< + ParamsDictionary, + LoginPageValidationResult | { message: string }, + unknown, + { authType: AuthType; target: string; esrvcId: string } +> = (req, res) => { + const { target, authType, esrvcId } = req.query + return SpcpFactory.createRedirectUrl(authType, target, esrvcId) + .asyncAndThen(SpcpFactory.fetchLoginPage) + .andThen(SpcpFactory.validateLoginPage) + .map((result) => res.status(StatusCodes.OK).json(result)) + .mapErr((error) => { + logger.error({ + message: 'Error while validating e-service ID', + meta: { + action: 'handleValidate', + ...createReqMeta(req), authType, target, esrvcId, diff --git a/src/app/modules/spcp/spcp.errors.ts b/src/app/modules/spcp/spcp.errors.ts index f6ccc8aff5..6cceb36852 100644 --- a/src/app/modules/spcp/spcp.errors.ts +++ b/src/app/modules/spcp/spcp.errors.ts @@ -16,3 +16,21 @@ export class InvalidAuthTypeError extends ApplicationError { super(`Invalid authType: ${authType}`) } } + +/** + * Error while fetching SP/CP login page. + */ +export class FetchLoginPageError extends ApplicationError { + constructor(message = 'Error while fetching SP/CP login page') { + super(message) + } +} + +/** + * Invalid SP/CP login page. + */ +export class LoginPageValidationError extends ApplicationError { + constructor(message = 'Invalid SP/CP login page') { + super(message) + } +} diff --git a/src/app/modules/spcp/spcp.factory.ts b/src/app/modules/spcp/spcp.factory.ts index 0bc78cd099..4de0f12dbb 100644 --- a/src/app/modules/spcp/spcp.factory.ts +++ b/src/app/modules/spcp/spcp.factory.ts @@ -1,4 +1,4 @@ -import { err, Result } from 'neverthrow' +import { err, errAsync, Result, ResultAsync } from 'neverthrow' import FeatureManager, { FeatureNames, @@ -7,8 +7,14 @@ import FeatureManager, { import { AuthType } from '../../../types' import { MissingFeatureError } from '../core/core.errors' -import { CreateRedirectUrlError, InvalidAuthTypeError } from './spcp.errors' +import { + CreateRedirectUrlError, + FetchLoginPageError, + InvalidAuthTypeError, + LoginPageValidationError, +} from './spcp.errors' import { SpcpService } from './spcp.service' +import { LoginPageValidationResult } from './spcp.types' interface ISpcpFactory { createRedirectUrl( @@ -19,6 +25,15 @@ interface ISpcpFactory { string, CreateRedirectUrlError | InvalidAuthTypeError | MissingFeatureError > + fetchLoginPage( + redirectUrl: string, + ): ResultAsync + validateLoginPage( + loginHtml: string, + ): Result< + LoginPageValidationResult, + LoginPageValidationError | MissingFeatureError + > } export const createSpcpFactory = ({ @@ -29,6 +44,8 @@ export const createSpcpFactory = ({ const error = new MissingFeatureError(FeatureNames.SpcpMyInfo) return { createRedirectUrl: () => err(error), + fetchLoginPage: () => errAsync(error), + validateLoginPage: () => err(error), } } return new SpcpService(props) diff --git a/src/app/modules/spcp/spcp.middlewares.ts b/src/app/modules/spcp/spcp.middlewares.ts new file mode 100644 index 0000000000..09fc8c3da1 --- /dev/null +++ b/src/app/modules/spcp/spcp.middlewares.ts @@ -0,0 +1,11 @@ +import { celebrate, Joi, Segments } from 'celebrate' + +import { AuthType } from '../../../types' + +export const redirectParamsMiddleware = celebrate({ + [Segments.QUERY]: Joi.object({ + target: Joi.string().required(), + authType: Joi.string().required().valid(AuthType.SP, AuthType.CP), + esrvcId: Joi.string().required(), + }), +}) diff --git a/src/app/modules/spcp/spcp.routes.ts b/src/app/modules/spcp/spcp.routes.ts index 5bad0b390a..eb1c60c54e 100644 --- a/src/app/modules/spcp/spcp.routes.ts +++ b/src/app/modules/spcp/spcp.routes.ts @@ -1,9 +1,7 @@ -import { celebrate, Joi, Segments } from 'celebrate' import { Router } from 'express' -import { AuthType } from '../../../types' - import * as SpcpController from './spcp.controller' +import { redirectParamsMiddleware } from './spcp.middlewares' // Shared routes for Singpass and Corppass export const SpcpRouter = Router() @@ -23,12 +21,26 @@ export const SpcpRouter = Router() */ SpcpRouter.get( '/redirect', - celebrate({ - [Segments.QUERY]: Joi.object({ - target: Joi.string().required(), - authType: Joi.string().required().valid(AuthType.SP, AuthType.CP), - esrvcId: Joi.string().required(), - }), - }), + redirectParamsMiddleware, SpcpController.handleRedirect, ) + +/** + * Gets the spcp redirect URL and parses the returned page to check for error codes + * @route GET /spcp/validate + * @group SPCP - SingPass/CorpPass logins for form-fillers + * @param {string} target.query.required - the destination URL after login + * @param {string} authType.query.required - `SP` for SingPass or `CP` for CorpPass + * @param {string} esrvcId.query.required - e-service id + * @produces application/json + * @returns {Object} 200 - {isValid: boolean, errorCode?: string} + * where isValid is true if eservice id was valid, and otherwise false with the errorCode set to + * the error code returned by SingPass/CorpPass + * @returns {string} 503 - error message if the SP/CP server could not be contacted to retrieve the login page + * @returns {string} 502 - error message if the SP/CP server returned content which could not be parsed + */ +SpcpRouter.get( + '/validate', + redirectParamsMiddleware, + SpcpController.handleValidate, +) diff --git a/src/app/modules/spcp/spcp.service.ts b/src/app/modules/spcp/spcp.service.ts index 698a57e724..1c2bd5dd35 100644 --- a/src/app/modules/spcp/spcp.service.ts +++ b/src/app/modules/spcp/spcp.service.ts @@ -1,14 +1,26 @@ import SPCPAuthClient from '@opengovsg/spcp-auth-client' +import axios from 'axios' import fs from 'fs' -import { err, ok, Result } from 'neverthrow' +import { StatusCodes } from 'http-status-codes' +import { err, ok, Result, ResultAsync } from 'neverthrow' import { ISpcpMyInfo } from '../../../config/feature-manager' import { createLoggerWithLabel } from '../../../config/logger' import { AuthType } from '../../../types' -import { CreateRedirectUrlError, InvalidAuthTypeError } from './spcp.errors' +import { + CreateRedirectUrlError, + FetchLoginPageError, + InvalidAuthTypeError, + LoginPageValidationError, +} from './spcp.errors' +import { LoginPageValidationResult } from './spcp.types' +import { getSubstringBetween } from './spcp.util' const logger = createLoggerWithLabel(module) +const LOGIN_PAGE_HEADERS = + 'text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8,application/signed-exchange;v=b3' +const LOGIN_PAGE_TIMEOUT = 10000 // 10 seconds export class SpcpService { #singpassAuthClient: SPCPAuthClient #corppassAuthClient: SPCPAuthClient @@ -77,4 +89,67 @@ export class SpcpService { return err(new CreateRedirectUrlError()) } } + + fetchLoginPage( + redirectUrl: string, + ): ResultAsync { + return ResultAsync.fromPromise( + axios + .get(redirectUrl, { + headers: { + Accept: LOGIN_PAGE_HEADERS, + }, + timeout: LOGIN_PAGE_TIMEOUT, + // Throw error if not status 200. + validateStatus: (status) => status === StatusCodes.OK, + }) + .then((response) => response.data), + (error) => { + logger.error({ + message: 'Error while fetching SP/CP login page', + meta: { + action: 'fetchLoginPage', + redirectUrl, + }, + error, + }) + return new FetchLoginPageError() + }, + ) + } + + validateLoginPage( + loginHtml: string, + ): Result { + // The successful login page should have the title 'SingPass Login' + // The error page should have the title 'SingPass - System Error Page' + const title = getSubstringBetween(loginHtml, '', '') + if (!title) { + logger.error({ + message: 'Could not find SP/CP login page title', + meta: { + action: 'validateLoginPage', + }, + }) + return err(new LoginPageValidationError()) + } + if (title.indexOf('Error') === -1) { + return ok({ isValid: true }) + } else { + // The error page should have text like 'System Code: 138' + const errorCode = getSubstringBetween( + loginHtml, + 'System Code: ', + '', + ) + logger.warn({ + message: 'Received error page from SP/CP', + meta: { + action: 'validateLoginPage', + errorCode, + }, + }) + return ok({ isValid: false, errorCode }) + } + } } diff --git a/src/app/modules/spcp/spcp.types.ts b/src/app/modules/spcp/spcp.types.ts new file mode 100644 index 0000000000..e3ae491f61 --- /dev/null +++ b/src/app/modules/spcp/spcp.types.ts @@ -0,0 +1,3 @@ +export type LoginPageValidationResult = + | { isValid: true } + | { isValid: false; errorCode: string | null } diff --git a/src/app/modules/spcp/spcp.util.ts b/src/app/modules/spcp/spcp.util.ts index 3efe9469a9..62c80aeb8b 100644 --- a/src/app/modules/spcp/spcp.util.ts +++ b/src/app/modules/spcp/spcp.util.ts @@ -4,10 +4,28 @@ import { createLoggerWithLabel } from '../../../config/logger' import { MapRouteError } from '../../../types' import { MissingFeatureError } from '../core/core.errors' -import { CreateRedirectUrlError } from './spcp.errors' +import { + CreateRedirectUrlError, + FetchLoginPageError, + LoginPageValidationError, +} from './spcp.errors' const logger = createLoggerWithLabel(module) +export const getSubstringBetween = ( + text: string, + markerStart: string, + markerEnd: string, +): string | null => { + const start = text.indexOf(markerStart) + if (start === -1) { + return null + } else { + const end = text.indexOf(markerEnd, start) + return end === -1 ? null : text.substring(start + markerStart.length, end) + } +} + export const mapRouteError: MapRouteError = (error) => { switch (error.constructor) { case MissingFeatureError: @@ -16,6 +34,16 @@ export const mapRouteError: MapRouteError = (error) => { statusCode: StatusCodes.INTERNAL_SERVER_ERROR, errorMessage: 'Sorry, something went wrong. Please try again.', } + case FetchLoginPageError: + return { + statusCode: StatusCodes.SERVICE_UNAVAILABLE, + errorMessage: 'Failed to contact SingPass. Please try again.', + } + case LoginPageValidationError: + return { + statusCode: StatusCodes.BAD_GATEWAY, + errorMessage: 'Error while contacting SingPass. Please try again.', + } default: logger.error({ message: 'Unknown route error observed', diff --git a/src/app/routes/spcp.server.routes.js b/src/app/routes/spcp.server.routes.js index 29f34fb7db..4fe04383ae 100644 --- a/src/app/routes/spcp.server.routes.js +++ b/src/app/routes/spcp.server.routes.js @@ -7,21 +7,6 @@ const spcpFactory = require('../factories/spcp.factory') */ module.exports = function (app) { - /** - * Gets the spcp redirect URL and parses the returned page to check for error codes - * @route GET /spcp/validate - * @group SPCP - SingPass/CorpPass logins for form-fillers - * @param {string} target.query.required - the destination URL after login - * @param {string} authType.query.required - `SP` for SingPass or `CP` for CorpPass - * @param {string} esrvcId.query.required - e-service id - * @produces application/json - * @returns {Object} 200 - {status: boolean} where boolean is true if eservice id was valid, false otherwise - * @returns {string} 400 - the redirect URL will be malformed due to missing parameters - */ - app - .route('/spcp/validate') - .get(spcpFactory.createSpcpRedirectURL, spcpFactory.validateESrvcId) - /** * Receive a SAML artifact and target destination from CorpPass, and * issue a 302 redirect on successful artifact verification diff --git a/tests/unit/backend/controllers/spcp.server.controller.spec.js b/tests/unit/backend/controllers/spcp.server.controller.spec.js index 20bbbaae98..157a2442ea 100644 --- a/tests/unit/backend/controllers/spcp.server.controller.spec.js +++ b/tests/unit/backend/controllers/spcp.server.controller.spec.js @@ -164,79 +164,6 @@ describe('SPCP Controller', () => { } } - describe('createSpcpRedirectURL', () => { - let expectedParam - let next - beforeEach(() => { - expectedParam = { - target: 'www.form.gov.sg/targetForm', - authType: 'NIL', - esrvcId: 'esrvcId', - } - req.query = _.cloneDeep(expectedParam) - next = jasmine.createSpy() - }) - - it('should return 400 if target not provided', () => { - req.query.target = '' - Controller.createSpcpRedirectURL(authClients)(req, res, next) - expect(res.status).toHaveBeenCalledWith(400) - expect(res.json).toHaveBeenCalledWith({ - message: 'Redirect URL malformed', - }) - }) - - it('should return 400 if authType not provided', () => { - req.query.authType = '' - Controller.createSpcpRedirectURL(authClients)(req, res, next) - expect(res.status).toHaveBeenCalledWith(400) - expect(res.json).toHaveBeenCalledWith({ - message: 'Redirect URL malformed', - }) - }) - it('should return 400 if esrvcId not provided', () => { - req.query.esrvcId = '' - Controller.createSpcpRedirectURL(authClients)(req, res, next) - expect(res.status).toHaveBeenCalledWith(400) - expect(res.json).toHaveBeenCalledWith({ - message: 'Redirect URL malformed', - }) - }) - it('should return 400 if authType is NIL', () => { - Controller.createSpcpRedirectURL(authClients)(req, res, next) - expect(res.status).toHaveBeenCalledWith(400) - expect(res.json).toHaveBeenCalledWith({ - message: 'Redirect URL malformed', - }) - }) - it('should return 200 and redirectUrl if authType is SP', () => { - req.query.authType = 'SP' - let expectedUrl = { - redirectURL: 'www.redirectUrlForSP.com', - } - singPassAuthClient.createRedirectURL.and.callFake((target, esrvcId) => { - expect(target).toEqual(expectedParam.target) - expect(esrvcId).toEqual(expectedParam.esrvcId) - return expectedUrl.redirectURL - }) - Controller.createSpcpRedirectURL(authClients)(req, res, next) - expect(next).toHaveBeenCalled() - }) - it('should return 200 and redirectUrl if authType is CP', () => { - req.query.authType = 'CP' - let expectedUrl = { - redirectURL: 'www.redirectUrlForCP.com', - } - corpPassAuthClient.createRedirectURL.and.callFake((target, esrvcId) => { - expect(target).toEqual(expectedParam.target) - expect(esrvcId).toEqual(expectedParam.esrvcId) - return expectedUrl.redirectURL - }) - Controller.createSpcpRedirectURL(authClients)(req, res, next) - expect(next).toHaveBeenCalled() - }) - }) - describe('isSpcpAuthenticated', () => { let next