From 5cba28ae20142218b84ce0733e2fb7e6da019146 Mon Sep 17 00:00:00 2001 From: Kar Rui Lau Date: Mon, 7 Sep 2020 22:36:49 +0800 Subject: [PATCH] refactor: migrate /auth endpoint handling to Typescript, Domain Driven Design (#215) * refactor: introduces nested routing in TS * doc: add better documentation for functions * chore: add eslint for all "*.spec.ts" files * test(jestExpress): enhance mockResponse creation - mockResponse now allows injection of other response-related keys used to setup all the middlewares that helps in testing * test(AuthRoutes): add /checkuser integration tests * fix: update tsconfig.build.json to exclude spec files and public * test: add CookieStore helper class to store cookies for auth'd routes * Introduces ApplicationError as an abstract class for extension - this prevents developers from using ApplicationError directly and are forced to extend from the class (in Typescript). --- .eslintrc | 3 +- package-lock.json | 25 + package.json | 1 + .../authentication.server.controller.js | 400 ------------- src/app/models/token.server.model.ts | 45 +- src/app/models/user.server.model.ts | 28 +- .../auth/__tests__/auth.controller.spec.ts | 285 ++++++++++ .../auth/__tests__/auth.middlewares.spec.ts | 71 +++ .../auth/__tests__/auth.routes.spec.ts | 530 ++++++++++++++++++ .../auth/__tests__/auth.service.spec.ts | 166 ++++++ src/app/modules/auth/auth.controller.ts | 202 +++++++ src/app/modules/auth/auth.errors.ts | 17 + src/app/modules/auth/auth.middlewares.ts | 58 ++ src/app/modules/auth/auth.routes.ts | 97 ++++ src/app/modules/auth/auth.service.ts | 105 ++++ src/app/modules/auth/auth.types.ts | 13 + src/app/modules/bounce/bounce.controller.ts | 5 +- src/app/modules/core/core.errors.ts | 2 +- src/app/modules/core/core.types.ts | 3 + src/app/modules/user/user.controller.ts | 5 +- src/app/modules/user/user.service.ts | 46 +- .../routes/authentication.server.routes.js | 119 ---- src/app/routes/index.js | 1 - src/loaders/express/index.ts | 3 + src/types/token.ts | 18 +- src/types/user.ts | 8 +- tests/integration/helpers/express-setup.ts | 63 +++ .../authentication.server.controller.spec.js | 298 ---------- tests/unit/backend/helpers/jest-express.ts | 31 +- tsconfig.build.json | 2 +- tsconfig.json | 1 - 31 files changed, 1790 insertions(+), 861 deletions(-) create mode 100644 src/app/modules/auth/__tests__/auth.controller.spec.ts create mode 100644 src/app/modules/auth/__tests__/auth.middlewares.spec.ts create mode 100644 src/app/modules/auth/__tests__/auth.routes.spec.ts create mode 100644 src/app/modules/auth/__tests__/auth.service.spec.ts create mode 100644 src/app/modules/auth/auth.controller.ts create mode 100644 src/app/modules/auth/auth.errors.ts create mode 100644 src/app/modules/auth/auth.middlewares.ts create mode 100644 src/app/modules/auth/auth.routes.ts create mode 100644 src/app/modules/auth/auth.service.ts create mode 100644 src/app/modules/auth/auth.types.ts create mode 100644 src/app/modules/core/core.types.ts delete mode 100755 src/app/routes/authentication.server.routes.js create mode 100644 tests/integration/helpers/express-setup.ts diff --git a/.eslintrc b/.eslintrc index 8cf461b78e..97378f5557 100644 --- a/.eslintrc +++ b/.eslintrc @@ -52,7 +52,8 @@ "import/newline-after-import": "error", "import/no-duplicates": "error" } - } + }, + { "files": ["*.spec.ts"], "extends": ["plugin:jest/recommended"] } ], "rules": { "no-unused-vars": ["error", { "argsIgnorePattern": "^_" }], diff --git a/package-lock.json b/package-lock.json index 9a8ca59300..67290cc072 100644 --- a/package-lock.json +++ b/package-lock.json @@ -4366,6 +4366,12 @@ "@types/express": "*" } }, + "@types/cookiejar": { + "version": "2.1.1", + "resolved": "https://registry.npmjs.org/@types/cookiejar/-/cookiejar-2.1.1.tgz", + "integrity": "sha512-aRnpPa7ysx3aNW60hTiCtLHlQaIFsXFCgQlpakNgDNVFzbtusSY8PwjAQgRWfSk0ekNoBjO51eQRB6upA9uuyw==", + "dev": true + }, "@types/cross-spawn": { "version": "6.0.2", "resolved": "https://registry.npmjs.org/@types/cross-spawn/-/cross-spawn-6.0.2.tgz", @@ -4816,6 +4822,25 @@ "integrity": "sha512-7NQmHra/JILCd1QqpSzl8+mJRc8ZHz3uDm8YV1Ks9IhK0epEiTw8aIErbvH9PI+6XbqhyIQy3462nEsn7UVzjQ==", "dev": true }, + "@types/superagent": { + "version": "4.1.10", + "resolved": "https://registry.npmjs.org/@types/superagent/-/superagent-4.1.10.tgz", + "integrity": "sha512-xAgkb2CMWUMCyVc/3+7iQfOEBE75NvuZeezvmixbUw3nmENf2tCnQkW5yQLTYqvXUQ+R6EXxdqKKbal2zM5V/g==", + "dev": true, + "requires": { + "@types/cookiejar": "*", + "@types/node": "*" + } + }, + "@types/supertest": { + "version": "2.0.10", + "resolved": "https://registry.npmjs.org/@types/supertest/-/supertest-2.0.10.tgz", + "integrity": "sha512-Xt8TbEyZTnD5Xulw95GLMOkmjGICrOQyJ2jqgkSjAUR3mm7pAIzSR0NFBaMcwlzVvlpCjNwbATcWWwjNiZiFrQ==", + "dev": true, + "requires": { + "@types/superagent": "*" + } + }, "@types/tmp": { "version": "0.2.0", "resolved": "https://registry.npmjs.org/@types/tmp/-/tmp-0.2.0.tgz", diff --git a/package.json b/package.json index 5582b8fc6d..85cff22f9e 100644 --- a/package.json +++ b/package.json @@ -186,6 +186,7 @@ "@types/nodemailer-direct-transport": "^1.0.31", "@types/promise-retry": "^1.1.3", "@types/puppeteer-core": "^2.0.0", + "@types/supertest": "^2.0.10", "@types/triple-beam": "^1.3.2", "@types/uid-generator": "^2.0.2", "@types/uuid": "^8.0.0", diff --git a/src/app/controllers/authentication.server.controller.js b/src/app/controllers/authentication.server.controller.js index 42f62e24cc..5787ddc46c 100755 --- a/src/app/controllers/authentication.server.controller.js +++ b/src/app/controllers/authentication.server.controller.js @@ -3,27 +3,9 @@ /** * Module dependencies. */ -const mongoose = require('mongoose') -const getUserModel = require('../models/user.server.model').default -const getTokenModel = require('../models/token.server.model').default -const getAgencyModel = require('../models/agency.server.model').default - -const User = getUserModel(mongoose) -const Token = getTokenModel(mongoose) -const Agency = getAgencyModel(mongoose) -const bcrypt = require('bcrypt') -const validator = require('validator') const { StatusCodes } = require('http-status-codes') -const config = require('../../config/config') -const { LINKS } = require('../../shared/constants') const PERMISSIONS = require('../utils/permission-levels').default -const { getRequestIp } = require('../utils/request') -const logger = require('../../config/logger').createLoggerWithLabel(module) -const { generateOtp } = require('../utils/otp') -const MailService = require('../services/mail.service').default - -const MAX_OTP_ATTEMPTS = 10 /** * Middleware that authenticates admin-user @@ -41,50 +23,6 @@ exports.authenticateUser = function (req, res, next) { } } -/** - * Checks if domain is from a whitelisted agency - * @param {Object} req - Express request object - * @param {Object} res - Express response object - */ -exports.validateDomain = function (req, res, next) { - let email = req.body.email - let emailDomain = String(validator.isEmail(email) && email.split('@').pop()) - Agency.findOne({ emailDomain }, function (err, agency) { - // Database issues - if (err) { - return res - .status(StatusCodes.INTERNAL_SERVER_ERROR) - .send( - `Unable to validate email domain. If this issue persists, please submit a Support Form (${LINKS.supportFormLink}).`, - ) - } - // Agency not found - if (!agency) { - logger.error({ - message: 'Agency not found', - meta: { - action: 'validateDomain', - email, - emailDomain, - ip: getRequestIp(req), - }, - error: err, - }) - return res - .status(StatusCodes.UNAUTHORIZED) - .send( - 'This is not a whitelisted public service email domain. Please log in with your official government or government-linked email address.', - ) - } - // Agency whitelisted - res.locals = { - agency, - email, - } - return next() - }) -} - /** * Returns a middleware function that ensures that only users with the requiredPermission will pass. * @param {String} requiredPermission - Either 'write' or 'delete', indicating what level of authorization @@ -139,341 +77,3 @@ exports.verifyPermission = (requiredPermission) => }) } } - -/** - * Create OTP using bcrypt and save to DB - * @param {Object} req - Express request object - * @param {Object} res - Express response object - * @param {Object} next - Express next middleware function - */ -exports.createOtp = function (req, res, next) { - // 1. Create 6 digit OTP using crypto - // 2. Hash OTP using bcrypt. OTP expires in 15 mins - // 3. Save OTP to DB - - let email = res.locals.email - let otp = generateOtp() - bcrypt.hash(otp, 10, function (bcryptErr, hashedOtp) { - if (bcryptErr) { - return res - .status(StatusCodes.INTERNAL_SERVER_ERROR) - .send( - 'Error generating OTP. Please try again later and if the problem persists, contact us.', - ) - } - - let record = { - email: email, - hashedOtp: hashedOtp, - numOtpAttempts: 0, - expireAt: new Date(Date.now() + config.otpLifeSpan), - } - - Token.findOneAndUpdate( - { email: email }, - { - $set: record, - }, - { w: 1, upsert: true, new: true }, - function (updateErr, updatedRecord) { - if (updateErr) { - logger.error({ - message: 'Token update error', - meta: { - action: 'createOtp', - ip: getRequestIp(req), - url: req.url, - headers: req.headers, - }, - error: updateErr, - }) - return res - .status(StatusCodes.INTERNAL_SERVER_ERROR) - .send( - 'Error saving OTP. Please try again later and if the problem persists, contact us.', - ) - } - - res.locals.otp = otp - res.locals.expireAt = updatedRecord.expireAt - return next() - }, - ) - }) -} - -/** - * Sends OTP using mail transport - * @param {Object} req - Express request object - * @param {Object} res - Express response object - */ -exports.sendOtp = async function (req, res) { - // 1. Configure email with OTP to be sent to user email - // 2. Return success statement to front end - - let otp = res.locals.otp - let recipient = res.locals.email - - try { - await MailService.sendLoginOtp({ - recipient, - otp, - ipAddress: getRequestIp(req), - }) - logger.info({ - message: 'Login OTP sent', - meta: { - action: 'sendOtp', - ip: getRequestIp(req), - email: recipient, - }, - }) - return res.status(StatusCodes.OK).send(`OTP sent to ${recipient}!`) - } catch (err) { - logger.error({ - message: 'Mail otp error', - meta: { - action: 'sendOtp', - ip: getRequestIp(req), - url: req.url, - headers: req.headers, - }, - error: err, - }) - return res - .status(StatusCodes.INTERNAL_SERVER_ERROR) - .send( - 'Error sending OTP. Please try again later and if the problem persists, contact us.', - ) - } -} - -/** - * Verify OTP using bcrypt - * @param {Object} req - Express request object - * @param {Object} res - Express response object - * @param {Object} next - Express next middleware function - */ -exports.verifyOtp = function (req, res, next) { - // 1. Increment the number of times this particular OTP has been attempted. - // * Upsert is false as we do not want OTP to be added to DB if not already present - // * We read and write to numOtpAttempts in findOneAndUpdate command to prevent concurrency issues - // 2. If number of attempts is more than max allowed, block user - // 3. Compare OTP given to hashed OTP in db - // 4. If OTP is correct, remove OTP from token table and call next() - let otp = req.body.otp - let email = res.locals.email - - let upsertData = { - $inc: { numOtpAttempts: 1 }, - } - - Token.findOneAndUpdate( - { email: email }, - upsertData, - { w: 1, upsert: false, new: true }, - function (updateErr, updatedRecord) { - if (updateErr) { - logger.error({ - message: 'Error updating Token in database', - meta: { - action: 'verifyOtp', - email, - ip: getRequestIp(req), - }, - error: updateErr, - }) - return res - .status(StatusCodes.INTERNAL_SERVER_ERROR) - .send( - `Unable to login at this time. Please submit a Support Form (${LINKS.supportFormLink}).`, - ) - } - if (!updatedRecord) { - logger.info({ - message: 'Expired OTP', - meta: { - action: 'verifyOtp', - ip: getRequestIp(req), - email, - }, - }) - return res - .status(StatusCodes.UNPROCESSABLE_ENTITY) - .send('OTP has expired. Click Resend to receive a new OTP.') - } - if (updatedRecord.numOtpAttempts > MAX_OTP_ATTEMPTS) { - logger.info({ - message: 'Exceeded max OTP attempts', - meta: { - action: 'verifyOtp', - ip: getRequestIp(req), - email, - }, - }) - return res - .status(StatusCodes.UNPROCESSABLE_ENTITY) - .send( - 'You have hit the max number of attempts for this OTP. Click Resend to receive a new OTP.', - ) - } - bcrypt.compare(otp, updatedRecord.hashedOtp, function ( - bcryptErr, - isCorrect, - ) { - if (bcryptErr) { - logger.error({ - message: 'Malformed OTP', - meta: { - action: 'verifyOtp', - ip: getRequestIp(req), - }, - error: bcryptErr, - }) - return res - .status(StatusCodes.INTERNAL_SERVER_ERROR) - .send( - 'Malformed OTP. Please try again later and if the problem persists, contact us.', - ) - } - if (isCorrect) { - Token.findOneAndRemove({ email: email }, function (removeErr) { - if (removeErr) { - logger.error({ - message: 'Error removing Token in database', - meta: { - action: 'verifyOtp', - email, - ip: getRequestIp(req), - }, - error: removeErr, - }) - return res - .status(StatusCodes.INTERNAL_SERVER_ERROR) - .send( - 'Failed to validate OTP. Please try again later and if the problem persists, contact us.', - ) - } else { - return next() - } - }) - } else { - logger.info({ - message: 'Invalid OTP', - meta: { - action: 'verifyOtp', - email, - ip: getRequestIp(req), - }, - }) - return res - .status(StatusCodes.UNAUTHORIZED) - .send('OTP is invalid. Please try again.') - } - }) - }, - ) -} - -/** - * Creates/updates user object and returns obj to client - * @param {Object} req - Express request object - * @param {Object} res - Express response object - */ -exports.signIn = function (req, res) { - // 1. Save user information to DB. Set agency id for new users - // 2. Start session by adding user and agency information to session object. Return user object to front end - - let email = res.locals.email - let agency = res.locals.agency - - let record = { - email: email, - agency: agency._id, - } - let upsertData = { - $set: record, - $setOnInsert: { - created: new Date(), - }, - } - User.findOneAndUpdate( - { email: email }, - upsertData, - { - w: 1, - upsert: true, - new: true, - runValidators: true, // Update validators are off by default - need to specify the runValidators option. - setDefaultsOnInsert: true, // Otherwise, defaults will not be set on update and findOneAndUpdate - }, - function (updateErr, user) { - if (updateErr || !user) { - return res - .status(StatusCodes.INTERNAL_SERVER_ERROR) - .send( - `User signin failed. Please try again later and if the problem persists, submit our Support Form (${LINKS.supportFormLink}).`, - ) - } - let userObj = { - agency: agency, - email: user.email, - contact: user.contact, - _id: user._id, - betaFlags: user.betaFlags, - } - - // Add user info to session - req.session.user = userObj - logger.info({ - message: 'Successful login', - meta: { - action: 'signIn', - email, - ip: getRequestIp(req), - }, - }) - return res.status(StatusCodes.OK).send(userObj) - }, - ) -} - -/** - * Sign out from session - * @param {Object} req - Express request object - * @param {Object} res - Express response object - */ -exports.signOut = function (req, res) { - req.session.destroy(function (err) { - if (err) { - return res - .status(StatusCodes.INTERNAL_SERVER_ERROR) - .send('Sign out failed') - } else { - res.clearCookie('connect.sid') - return res.status(StatusCodes.OK).send('Sign out successful') - } - }) -} - -/** - * Verifies if a request was sent by a user that has the beta flag enabled for a betaType - * @param {String} betaType - beta flag in User schema - * @param {Object} req - Express request object - * @param {Object} req.session - session from express-session - * @param {Object} [req.session.user] - user object with properties - * @param {Object} res - Express response object - * @param {Object} next - Express next object - */ - -exports.doesUserBeta = (betaType) => (req, res, next) => { - const { session } = req - const { user } = session - if (user && user.betaFlags && user.betaFlags[betaType]) { - return next() - } else { - return res.status(StatusCodes.FORBIDDEN).send({ - message: `User is not authorized to access beta feature: ${betaType}`, - }) - } -} diff --git a/src/app/models/token.server.model.ts b/src/app/models/token.server.model.ts index 7b2da1b432..f4d2538956 100644 --- a/src/app/models/token.server.model.ts +++ b/src/app/models/token.server.model.ts @@ -1,18 +1,21 @@ -import { Model, Mongoose, Schema } from 'mongoose' +import { Mongoose, Schema } from 'mongoose' -import { ITokenSchema } from '../../types' +import { IToken, ITokenModel, ITokenSchema } from '../../types' export const TOKEN_SCHEMA_ID = 'Token' const TokenSchema = new Schema({ email: { type: String, + required: true, }, hashedOtp: { type: String, + required: true, }, expireAt: { type: Date, + required: true, }, numOtpAttempts: { type: Number, @@ -25,6 +28,40 @@ const TokenSchema = new Schema({ }) TokenSchema.index({ expireAt: 1 }, { expireAfterSeconds: 0 }) +// Statics +/** + * Upserts given OTP into Token collection. + */ +TokenSchema.statics.upsertOtp = async function ( + this: ITokenModel, + upsertParams: Omit, +) { + return this.findOneAndUpdate( + { email: upsertParams.email }, + { + $set: { ...upsertParams, numOtpAttempts: 0 }, + $inc: { numOtpSent: 1 }, + }, + { upsert: true, new: true, setDefaultsOnInsert: true, runValidators: true }, + ) +} + +/** + * Increments the number of login attempts for the document associated with the + * given email. + * @param email the email to retrieve the related Token document + */ +TokenSchema.statics.incrementAttemptsByEmail = async function ( + this: ITokenModel, + email: string, +) { + return this.findOneAndUpdate( + { email: email }, + { $inc: { numOtpAttempts: 1 } }, + { new: true }, + ) +} + /** * Token Schema * @param db - Active DB Connection @@ -32,9 +69,9 @@ TokenSchema.index({ expireAt: 1 }, { expireAfterSeconds: 0 }) */ const getTokenModel = (db: Mongoose) => { try { - return db.model(TOKEN_SCHEMA_ID) as Model + return db.model(TOKEN_SCHEMA_ID) as ITokenModel } catch { - return db.model(TOKEN_SCHEMA_ID, TokenSchema) + return db.model(TOKEN_SCHEMA_ID, TokenSchema) } } diff --git a/src/app/models/user.server.model.ts b/src/app/models/user.server.model.ts index c9097279c8..047e6634eb 100644 --- a/src/app/models/user.server.model.ts +++ b/src/app/models/user.server.model.ts @@ -1,15 +1,13 @@ import { parsePhoneNumberFromString } from 'libphonenumber-js/mobile' -import { Model, Mongoose, Schema } from 'mongoose' +import { Mongoose, Schema } from 'mongoose' import validator from 'validator' -import { IUserSchema } from '../../types' +import { IUser, IUserModel, IUserSchema } from '../../types' import getAgencyModel, { AGENCY_SCHEMA_ID } from './agency.server.model' export const USER_SCHEMA_ID = 'User' -export type IUserModel = Model - const compileUserModel = (db: Mongoose) => { const Agency = getAgencyModel(db) @@ -79,7 +77,27 @@ const compileUserModel = (db: Mongoose) => { } }) - return db.model(USER_SCHEMA_ID, UserSchema) + // Statics + /** + * Upserts given user details into User collection. + */ + UserSchema.statics.upsertUser = async function ( + this: IUserModel, + upsertParams: Pick, + ) { + return this.findOneAndUpdate( + { email: upsertParams.email }, + { $set: upsertParams }, + { + upsert: true, + new: true, + runValidators: true, + setDefaultsOnInsert: true, + }, + ) + } + + return db.model(USER_SCHEMA_ID, UserSchema) } /** diff --git a/src/app/modules/auth/__tests__/auth.controller.spec.ts b/src/app/modules/auth/__tests__/auth.controller.spec.ts new file mode 100644 index 0000000000..21ba412ef6 --- /dev/null +++ b/src/app/modules/auth/__tests__/auth.controller.spec.ts @@ -0,0 +1,285 @@ +import expressHandler from 'tests/unit/backend/helpers/jest-express' +import { mocked } from 'ts-jest/utils' + +import MailService from 'src/app/services/mail.service' +import { IAgencySchema, IUserSchema } from 'src/types' + +import * as UserService from '../../user/user.service' +import * as AuthController from '../auth.controller' +import { InvalidOtpError } from '../auth.errors' +import * as AuthService from '../auth.service' + +const VALID_EMAIL = 'test@example.com' + +// Mock services invoked by AuthController +jest.mock('../auth.service') +jest.mock('../../user/user.service') +jest.mock('src/app/services/mail.service') +const MockAuthService = mocked(AuthService) +const MockMailService = mocked(MailService) +const MockUserService = mocked(UserService) + +describe('auth.controller', () => { + afterEach(() => { + jest.clearAllMocks() + }) + + describe('handleCheckUser', () => { + it('should return 200', async () => { + // Arrange + const mockRes = expressHandler.mockResponse() + + // Act + await AuthController.handleCheckUser( + expressHandler.mockRequest(), + mockRes, + jest.fn(), + ) + + // Assert + expect(mockRes.sendStatus).toBeCalledWith(200) + }) + }) + + describe('handleLoginSendOtp', () => { + const MOCK_OTP = '123456' + const MOCK_REQ = expressHandler.mockRequest({ + body: { email: VALID_EMAIL }, + }) + + it('should return 200 when login OTP is generated and sent to recipient successfully', async () => { + // Arrange + const mockRes = expressHandler.mockResponse() + // Mock AuthService and MailService to return without errors + MockAuthService.createLoginOtp.mockResolvedValueOnce(MOCK_OTP) + MockMailService.sendLoginOtp.mockResolvedValueOnce(true) + + // Act + await AuthController.handleLoginSendOtp(MOCK_REQ, mockRes, jest.fn()) + + // Assert + expect(mockRes.status).toBeCalledWith(200) + expect(mockRes.send).toBeCalledWith(`OTP sent to ${VALID_EMAIL}!`) + // Services should have been invoked. + expect(MockAuthService.createLoginOtp).toHaveBeenCalledTimes(1) + expect(MockMailService.sendLoginOtp).toHaveBeenCalledTimes(1) + }) + + it('should return 500 when there is an error generating login OTP', async () => { + // Arrange + const mockRes = expressHandler.mockResponse() + // Mock createLoginOtp failure + MockAuthService.createLoginOtp.mockRejectedValueOnce( + new Error('otp creation error'), + ) + + // Act + await AuthController.handleLoginSendOtp(MOCK_REQ, mockRes, jest.fn()) + + // Assert + expect(mockRes.status).toBeCalledWith(500) + expect(mockRes.send).toBeCalledWith( + 'Failed to send login OTP. Please try again later and if the problem persists, contact us.', + ) + // Sending login OTP should not have been called. + expect(MockAuthService.createLoginOtp).toHaveBeenCalledTimes(1) + expect(MockMailService.sendLoginOtp).not.toHaveBeenCalled() + }) + + it('should return 500 when there is an error sending login OTP', async () => { + // Arrange + const mockRes = expressHandler.mockResponse() + // Mock createLoginOtp success but sendLoginOtp failure. + MockAuthService.createLoginOtp.mockResolvedValueOnce(MOCK_OTP) + MockMailService.sendLoginOtp.mockRejectedValueOnce( + new Error('send error'), + ) + + // Act + await AuthController.handleLoginSendOtp(MOCK_REQ, mockRes, jest.fn()) + + // Assert + expect(mockRes.status).toBeCalledWith(500) + expect(mockRes.send).toBeCalledWith( + 'Error sending OTP. Please try again later and if the problem persists, contact us.', + ) + // Services should have been invoked. + expect(MockAuthService.createLoginOtp).toHaveBeenCalledTimes(1) + expect(MockMailService.sendLoginOtp).toHaveBeenCalledTimes(1) + }) + }) + + describe('handleLoginVerifyOtp', () => { + const MOCK_OTP = '123456' + const MOCK_REQ = expressHandler.mockRequest({ + body: { email: VALID_EMAIL, otp: MOCK_OTP }, + }) + const MOCK_AGENCY = { id: 'mock agency id' } as IAgencySchema + + it('should return 200 with the user when verification succeeds', async () => { + // Arrange + // Mock bare minimum mongo documents. + const mockUser = { + toObject: () => ({ id: 'imagine this is a user document from the db' }), + } as IUserSchema + // Add agency into locals due to precondition. + const mockRes = expressHandler.mockResponse({ + locals: { agency: MOCK_AGENCY }, + }) + + // Mock all service success. + MockAuthService.verifyLoginOtp.mockResolvedValueOnce(true) + MockUserService.retrieveUser.mockResolvedValueOnce(mockUser) + + // Act + await AuthController.handleLoginVerifyOtp(MOCK_REQ, mockRes, jest.fn()) + + // Assert + expect(mockRes.status).toBeCalledWith(200) + expect(mockRes.send).toBeCalledWith({ + ...mockUser.toObject(), + agency: MOCK_AGENCY, + }) + }) + + it('should return 422 when verifying login OTP throws an InvalidOtpError', async () => { + // Arrange + // Add agency into locals due to precondition. + const mockRes = expressHandler.mockResponse({ + locals: { agency: MOCK_AGENCY }, + }) + const expectedInvalidOtpError = new InvalidOtpError() + // Mock error from verifyLoginOtp. + MockAuthService.verifyLoginOtp.mockRejectedValueOnce( + expectedInvalidOtpError, + ) + + // Act + await AuthController.handleLoginVerifyOtp(MOCK_REQ, mockRes, jest.fn()) + + // Assert + expect(mockRes.status).toBeCalledWith(422) + expect(mockRes.send).toBeCalledWith(expectedInvalidOtpError.message) + // Check that the correct services have been called or not called. + expect(MockAuthService.verifyLoginOtp).toHaveBeenCalledTimes(1) + expect(MockUserService.retrieveUser).not.toHaveBeenCalled() + }) + + it('should return 500 when verifying login OTP throws a non-InvalidOtpError', async () => { + // Arrange + // Add agency into locals due to precondition. + const mockRes = expressHandler.mockResponse({ + locals: { agency: MOCK_AGENCY }, + }) + // Mock generic error from verifyLoginOtp. + MockAuthService.verifyLoginOtp.mockRejectedValueOnce( + new Error('generic error'), + ) + + // Act + await AuthController.handleLoginVerifyOtp(MOCK_REQ, mockRes, jest.fn()) + + // Assert + expect(mockRes.status).toBeCalledWith(500) + expect(mockRes.send).toBeCalledWith( + 'Failed to validate OTP. Please try again later and if the problem persists, contact us.', + ) + // Check that the correct services have been called or not called. + expect(MockAuthService.verifyLoginOtp).toHaveBeenCalledTimes(1) + expect(MockUserService.retrieveUser).not.toHaveBeenCalled() + }) + + it('should return 500 when an error is thrown while upserting user', async () => { + // Arrange + // Add agency into locals due to precondition. + const mockRes = expressHandler.mockResponse({ + locals: { agency: MOCK_AGENCY }, + }) + MockAuthService.verifyLoginOtp.mockResolvedValueOnce(true) + MockUserService.retrieveUser.mockRejectedValueOnce( + new Error('upsert error'), + ) + + // Act + await AuthController.handleLoginVerifyOtp(MOCK_REQ, mockRes, jest.fn()) + + // Assert + expect(mockRes.status).toBeCalledWith(500) + expect(mockRes.send).toBeCalledWith( + // Use stringContaining here due to dynamic text and out of test scope. + expect.stringContaining( + 'User signin failed. Please try again later and if the problem persists', + ), + ) + // Check that the correct services have been called or not called. + expect(MockAuthService.verifyLoginOtp).toHaveBeenCalledTimes(1) + expect(MockUserService.retrieveUser).toHaveBeenCalledTimes(1) + }) + }) + + describe('handleSignout', () => { + it('should return 200 when session is successfully destroyed', async () => { + // Arrange + const mockDestroy = jest.fn().mockImplementation((fn) => fn(false)) + const mockClearCookie = jest.fn() + const mockReq = expressHandler.mockRequest({ + session: { + destroy: mockDestroy, + }, + }) + const mockRes = expressHandler.mockResponse({ + clearCookie: mockClearCookie, + }) + + // Act + await AuthController.handleSignout(mockReq, mockRes, jest.fn()) + + // Assert + expect(mockRes.status).toBeCalledWith(200) + expect(mockRes.send).toBeCalledWith('Sign out successful') + expect(mockClearCookie).toBeCalledTimes(1) + expect(mockDestroy).toBeCalledTimes(1) + }) + + it('should return 400 when session does not exist in request', async () => { + // Arrange + const mockReqWithoutSession = expressHandler.mockRequest() + const mockRes = expressHandler.mockResponse() + + // Act + await AuthController.handleSignout( + mockReqWithoutSession, + mockRes, + jest.fn(), + ) + + // Assert + expect(mockRes.sendStatus).toBeCalledWith(400) + }) + + it('should return 500 when error is thrown when destroying session', async () => { + // Arrange + const mockDestroyWithErr = jest + .fn() + .mockImplementation((fn) => fn(new Error('some error'))) + const mockClearCookie = jest.fn() + const mockReq = expressHandler.mockRequest({ + session: { + destroy: mockDestroyWithErr, + }, + }) + const mockRes = expressHandler.mockResponse({ + clearCookie: mockClearCookie, + }) + + // Act + await AuthController.handleSignout(mockReq, mockRes, jest.fn()) + + // Assert + expect(mockRes.status).toBeCalledWith(500) + expect(mockRes.send).toBeCalledWith('Sign out failed') + expect(mockDestroyWithErr).toBeCalledTimes(1) + expect(mockClearCookie).not.toBeCalled() + }) + }) +}) diff --git a/src/app/modules/auth/__tests__/auth.middlewares.spec.ts b/src/app/modules/auth/__tests__/auth.middlewares.spec.ts new file mode 100644 index 0000000000..4c3bb824ba --- /dev/null +++ b/src/app/modules/auth/__tests__/auth.middlewares.spec.ts @@ -0,0 +1,71 @@ +import expressHandler from 'tests/unit/backend/helpers/jest-express' +import { mocked } from 'ts-jest/utils' + +import { IAgencySchema } from 'src/types' + +import { InvalidDomainError } from '../auth.errors' +import * as AuthMiddleware from '../auth.middlewares' +import * as AuthService from '../auth.service' + +jest.mock('../auth.service') +const MockAuthService = mocked(AuthService) + +describe('auth.middleware', () => { + describe('validateDomain', () => { + const MOCK_REQ = expressHandler.mockRequest({ + body: { email: 'test@example.com' }, + }) + + it('should continue without error when domain is valid', async () => { + // Arrange + const mockRes = expressHandler.mockResponse() + const mockNext = jest.fn() + MockAuthService.getAgencyWithEmail.mockResolvedValueOnce( + {} as IAgencySchema, + ) + + // Act + await AuthMiddleware.validateDomain(MOCK_REQ, mockRes, mockNext) + + // Assert + expect(mockNext).toBeCalled() + }) + + it('should return 500 when retrieving agency throws non ApplicationError', async () => { + // Arrange + const mockRes = expressHandler.mockResponse() + const mockNext = jest.fn() + MockAuthService.getAgencyWithEmail.mockRejectedValueOnce( + new Error('some error'), + ) + + // Act + await AuthMiddleware.validateDomain(MOCK_REQ, mockRes, mockNext) + + // Assert + expect(mockRes.status).toBeCalledWith(500) + expect(mockRes.send).toBeCalledWith( + expect.stringContaining( + 'Unable to validate email domain. If this issue persists, please submit a Support Form', + ), + ) + expect(mockNext).not.toBeCalled() + }) + + it('should return with ApplicationError status and message when retrieving agency throws ApplicationError', async () => { + // Arrange + const expectedError = new InvalidDomainError() + const mockRes = expressHandler.mockResponse() + const mockNext = jest.fn() + MockAuthService.getAgencyWithEmail.mockRejectedValueOnce(expectedError) + + // Act + await AuthMiddleware.validateDomain(MOCK_REQ, mockRes, mockNext) + + // Assert + expect(mockRes.status).toBeCalledWith(expectedError.status) + expect(mockRes.send).toBeCalledWith(expectedError.message) + expect(mockNext).not.toBeCalled() + }) + }) +}) diff --git a/src/app/modules/auth/__tests__/auth.routes.spec.ts b/src/app/modules/auth/__tests__/auth.routes.spec.ts new file mode 100644 index 0000000000..b6606de02e --- /dev/null +++ b/src/app/modules/auth/__tests__/auth.routes.spec.ts @@ -0,0 +1,530 @@ +import { pick } from 'lodash' +import supertest from 'supertest' +import { CookieStore, setupApp } from 'tests/integration/helpers/express-setup' +import dbHandler from 'tests/unit/backend/helpers/jest-db' +import validator from 'validator' + +import MailService from 'src/app/services/mail.service' +import * as OtpUtils from 'src/app/utils/otp' +import { IAgencySchema } from 'src/types' + +import * as UserService from '../../user/user.service' +import { AuthRouter } from '../auth.routes' +import * as AuthService from '../auth.service' + +const app = setupApp('/auth', AuthRouter) +const cookieStore = new CookieStore() +const request = supertest(app) + +describe('auth.routes', () => { + beforeAll(async () => await dbHandler.connect()) + afterEach(async () => { + await dbHandler.clearDatabase() + jest.restoreAllMocks() + cookieStore.clear() + }) + afterAll(async () => await dbHandler.closeDatabase()) + + describe('POST /auth/checkuser', () => { + it('should return 400 when body.email is not provided as a param', async () => { + // Act + const response = await request.post('/auth/checkuser') + + // Assert + expect(response.status).toEqual(400) + expect(response.text).toEqual('"email" is required') + }) + + it('should return 400 when body.email is invalid', async () => { + // Arrange + const invalidEmail = 'not an email' + + // Act + const response = await request + .post('/auth/checkuser') + .send({ email: invalidEmail }) + + // Assert + expect(response.status).toEqual(400) + expect(response.text).toEqual('Please enter a valid email') + }) + + it('should return 401 when domain of body.email does not exist in Agency collection', async () => { + // Arrange + const validEmailWithInvalidDomain = 'test@example.com' + + // Act + const response = await request + .post('/auth/checkuser') + .send({ email: validEmailWithInvalidDomain }) + + // Assert + expect(response.status).toEqual(401) + expect(response.text).toEqual( + 'This is not a whitelisted public service email domain. Please log in with your official government or government-linked email address.', + ) + }) + + it('should return 200 when domain of body.email exists in Agency collection', async () => { + // Arrange + // Insert agency + const validDomain = 'example.com' + const validEmail = `test@${validDomain}` + await dbHandler.insertDefaultAgency({ mailDomain: validDomain }) + + // Act + const response = await request + .post('/auth/checkuser') + .send({ email: validEmail }) + + // Assert + expect(response.status).toEqual(200) + expect(response.text).toEqual('OK') + }) + + it('should return 500 when validating domain throws an unknown error', async () => { + // Arrange + // Insert agency + const validDomain = 'example.com' + const validEmail = `test@${validDomain}` + await dbHandler.insertDefaultAgency({ mailDomain: validDomain }) + + const getAgencySpy = jest + .spyOn(AuthService, 'getAgencyWithEmail') + .mockRejectedValueOnce(new Error('some error occured')) + + // Act + const response = await request + .post('/auth/checkuser') + .send({ email: validEmail }) + + // Assert + expect(getAgencySpy).toBeCalled() + expect(response.status).toEqual(500) + expect(response.text).toEqual( + expect.stringContaining('Unable to validate email domain.'), + ) + }) + }) + + describe('POST /auth/sendotp', () => { + const VALID_DOMAIN = 'example.com' + const VALID_EMAIL = `test@${VALID_DOMAIN}` + const INVALID_DOMAIN = 'example.org' + + beforeEach(async () => + dbHandler.insertDefaultAgency({ mailDomain: VALID_DOMAIN }), + ) + + it('should return 400 when body.email is not provided as a param', async () => { + // Act + const response = await request.post('/auth/sendotp') + + // Assert + expect(response.status).toEqual(400) + expect(response.text).toEqual('"email" is required') + }) + + it('should return 400 when body.email is invalid', async () => { + // Arrange + const invalidEmail = 'not an email' + + // Act + const response = await request + .post('/auth/sendotp') + .send({ email: invalidEmail }) + + // Assert + expect(response.status).toEqual(400) + expect(response.text).toEqual('Please enter a valid email') + }) + + it('should return 401 when domain of body.email does not exist in Agency collection', async () => { + // Arrange + const validEmailWithInvalidDomain = `test@${INVALID_DOMAIN}` + expect(validator.isEmail(validEmailWithInvalidDomain)).toEqual(true) + + // Act + const response = await request + .post('/auth/sendotp') + .send({ email: validEmailWithInvalidDomain }) + + // Assert + expect(response.status).toEqual(401) + expect(response.text).toEqual( + 'This is not a whitelisted public service email domain. Please log in with your official government or government-linked email address.', + ) + }) + + it('should return 500 when error occurs whilst creating OTP', async () => { + // Arrange + const createLoginOtpSpy = jest + .spyOn(AuthService, 'createLoginOtp') + .mockRejectedValueOnce(new Error('some error')) + + // Act + const response = await request + .post('/auth/sendotp') + .send({ email: VALID_EMAIL }) + + // Assert + expect(createLoginOtpSpy).toHaveBeenCalled() + expect(response.status).toEqual(500) + expect(response.text).toEqual( + 'Failed to send login OTP. Please try again later and if the problem persists, contact us.', + ) + }) + + it('should return 500 when error occurs whilst sending login OTP', async () => { + // Arrange + const sendLoginOtpSpy = jest + .spyOn(MailService, 'sendLoginOtp') + .mockRejectedValueOnce(new Error('some error')) + + // Act + const response = await request + .post('/auth/sendotp') + .send({ email: VALID_EMAIL }) + + // Assert + expect(sendLoginOtpSpy).toHaveBeenCalled() + expect(response.status).toEqual(500) + expect(response.text).toEqual( + 'Error sending OTP. Please try again later and if the problem persists, contact us.', + ) + }) + + it('should return 500 when validating domain throws an unknown error', async () => { + // Arrange + const getAgencySpy = jest + .spyOn(AuthService, 'getAgencyWithEmail') + .mockRejectedValueOnce(new Error('some error occured')) + + // Act + const response = await request + .post('/auth/sendotp') + .send({ email: VALID_EMAIL }) + + // Assert + expect(getAgencySpy).toBeCalled() + expect(response.status).toEqual(500) + expect(response.text).toEqual( + expect.stringContaining('Unable to validate email domain.'), + ) + }) + + it('should return 200 when otp is sent successfully', async () => { + // Arrange + const sendLoginOtpSpy = jest + .spyOn(MailService, 'sendLoginOtp') + .mockResolvedValueOnce(true) + + // Act + const response = await request + .post('/auth/sendotp') + .send({ email: VALID_EMAIL }) + + // Assert + expect(sendLoginOtpSpy).toHaveBeenCalled() + expect(response.status).toEqual(200) + expect(response.text).toEqual(`OTP sent to ${VALID_EMAIL}!`) + }) + }) + + describe('POST /auth/verifyotp', () => { + const MOCK_VALID_OTP = '123456' + const VALID_DOMAIN = 'example.com' + const VALID_EMAIL = `test@${VALID_DOMAIN}` + const INVALID_DOMAIN = 'example.org' + + let defaultAgency: IAgencySchema + + beforeEach(async () => { + defaultAgency = await dbHandler.insertDefaultAgency({ + mailDomain: VALID_DOMAIN, + }) + jest.spyOn(OtpUtils, 'generateOtp').mockReturnValue(MOCK_VALID_OTP) + }) + + it('should return 400 when body.email is not provided as a param', async () => { + // Act + const response = await request.post('/auth/verifyotp').send({ + otp: MOCK_VALID_OTP, + }) + + // Assert + expect(response.status).toEqual(400) + expect(response.text).toEqual('"email" is required') + }) + + it('should return 400 when body.otp is not provided as a param', async () => { + // Act + const response = await request.post('/auth/verifyotp').send({ + email: VALID_EMAIL, + }) + + // Assert + expect(response.status).toEqual(400) + expect(response.text).toEqual('"otp" is required') + }) + + it('should return 400 when body.email is invalid', async () => { + // Arrange + const invalidEmail = 'not an email' + + // Act + const response = await request + .post('/auth/verifyotp') + .send({ email: invalidEmail, otp: MOCK_VALID_OTP }) + + // Assert + expect(response.status).toEqual(400) + expect(response.text).toEqual('Please enter a valid email') + }) + + it('should return 400 when body.otp is less than 6 digits', async () => { + // Act + const response = await request.post('/auth/verifyotp').send({ + email: VALID_EMAIL, + otp: '12345', + }) + + // Assert + expect(response.status).toEqual(400) + expect(response.text).toEqual('Please enter a valid otp') + }) + + it('should return 400 when body.otp is 6 characters but does not consist purely of digits', async () => { + // Act + const response = await request.post('/auth/verifyotp').send({ + email: VALID_EMAIL, + otp: '123abc', + }) + + // Assert + expect(response.status).toEqual(400) + expect(response.text).toEqual('Please enter a valid otp') + }) + + it('should return 401 when domain of body.email does not exist in Agency collection', async () => { + // Arrange + const validEmailWithInvalidDomain = `test@${INVALID_DOMAIN}` + expect(validator.isEmail(validEmailWithInvalidDomain)).toEqual(true) + + // Act + const response = await request + .post('/auth/verifyotp') + .send({ email: validEmailWithInvalidDomain, otp: MOCK_VALID_OTP }) + + // Assert + expect(response.status).toEqual(401) + expect(response.text).toEqual( + 'This is not a whitelisted public service email domain. Please log in with your official government or government-linked email address.', + ) + }) + + it('should return 500 when validating domain throws an unknown error', async () => { + // Arrange + const getAgencySpy = jest + .spyOn(AuthService, 'getAgencyWithEmail') + .mockRejectedValueOnce(new Error('some error occured')) + + // Act + const response = await request + .post('/auth/verifyotp') + .send({ email: VALID_EMAIL, otp: MOCK_VALID_OTP }) + + // Assert + expect(getAgencySpy).toBeCalled() + expect(response.status).toEqual(500) + expect(response.text).toEqual( + expect.stringContaining('Unable to validate email domain.'), + ) + }) + + it('should return 422 when hash does not exist for body.otp', async () => { + // Act + const response = await request + .post('/auth/verifyotp') + .send({ email: VALID_EMAIL, otp: MOCK_VALID_OTP }) + + // Assert + expect(response.status).toEqual(422) + expect(response.text).toEqual( + expect.stringContaining( + 'OTP has expired. Please request for a new OTP.', + ), + ) + }) + + it('should return 422 when body.otp is invalid', async () => { + // Arrange + const invalidOtp = '654321' + // Request for OTP so the hash exists. + await requestForOtp(VALID_EMAIL) + + // Act + const response = await request + .post('/auth/verifyotp') + .send({ email: VALID_EMAIL, otp: invalidOtp }) + + // Assert + expect(response.status).toEqual(422) + expect(response.text).toEqual('OTP is invalid. Please try again.') + }) + + it('should should return 422 when invalid body.otp has been attempted too many times', async () => { + // Arrange + const invalidOtp = '654321' + // Request for OTP so the hash exists. + await requestForOtp(VALID_EMAIL) + + // Act + // Attempt invalid OTP for MAX_OTP_ATTEMPTS. + const verifyPromises = [] + for (let i = 0; i < AuthService.MAX_OTP_ATTEMPTS; i++) { + verifyPromises.push( + request + .post('/auth/verifyotp') + .send({ email: VALID_EMAIL, otp: invalidOtp }), + ) + } + const results = (await Promise.all(verifyPromises)).map((resolve) => + pick(resolve, ['status', 'text']), + ) + // Should be all invalid OTP responses. + expect(results).toEqual( + Array(AuthService.MAX_OTP_ATTEMPTS).fill({ + status: 422, + text: 'OTP is invalid. Please try again.', + }), + ) + + // Act again, this time with a valid OTP. + const response = await request + .post('/auth/verifyotp') + .send({ email: VALID_EMAIL, otp: MOCK_VALID_OTP }) + + // Assert + // Should still reject with max OTP attempts error. + expect(response.status).toEqual(422) + expect(response.text).toEqual( + 'You have hit the max number of attempts. Please request for a new OTP.', + ) + }) + + it('should return 200 with user object when body.otp is a valid OTP', async () => { + // Arrange + // Request for OTP so the hash exists. + await requestForOtp(VALID_EMAIL) + + // Act + const response = await request + .post('/auth/verifyotp') + .send({ email: VALID_EMAIL, otp: MOCK_VALID_OTP }) + cookieStore.handleCookie(response) + + // Assert + expect(response.status).toEqual(200) + // Body should be an user object. + expect(response.body).toMatchObject({ + // Required since that's how the data is sent out from the application. + agency: JSON.parse(JSON.stringify(defaultAgency.toObject())), + _id: expect.any(String), + created: expect.any(String), + email: VALID_EMAIL, + }) + // Should have session cookie returned. + expect(cookieStore.get()).toEqual(expect.stringContaining('connect.sid')) + }) + + it('should return 500 when upserting user document fails', async () => { + // Arrange + // Request for OTP so the hash exists. + await requestForOtp(VALID_EMAIL) + + // Mock error thrown when creating user + const upsertSpy = jest + .spyOn(UserService, 'retrieveUser') + .mockRejectedValueOnce(new Error('some error')) + + // Act + const response = await request + .post('/auth/verifyotp') + .send({ email: VALID_EMAIL, otp: MOCK_VALID_OTP }) + + // Assert + // Should have reached this spy. + expect(upsertSpy).toBeCalled() + expect(response.status).toEqual(500) + expect(response.text).toEqual( + expect.stringContaining('User signin failed. Please try again later'), + ) + }) + }) + + describe('GET /auth/signout', () => { + const MOCK_VALID_OTP = '123456' + const VALID_DOMAIN = 'example.com' + const VALID_EMAIL = `test@${VALID_DOMAIN}` + + beforeEach(async () => { + await dbHandler.insertDefaultAgency({ + mailDomain: VALID_DOMAIN, + }) + jest.spyOn(OtpUtils, 'generateOtp').mockReturnValue(MOCK_VALID_OTP) + }) + + it('should return 200 and clear cookies when signout is successful', async () => { + // Act + // Sign in user + await signInUser(VALID_EMAIL, MOCK_VALID_OTP) + + // Arrange + const response = await request + .get('/auth/signout') + .set('Cookie', cookieStore.get()) + + // Assert + expect(response.status).toEqual(200) + expect(response.text).toEqual('Sign out successful') + // connect.sid should now be empty. + expect(response.header['set-cookie'][0]).toEqual( + expect.stringContaining('connect.sid=;'), + ) + }) + + it('should return 200 even when user has not signed in before', async () => { + // Arrange + // Should have no cookies. + expect(cookieStore.get()).toEqual('') + + // Act + const response = await request.get('/auth/signout') + + // Assert + expect(response.status).toEqual(200) + expect(response.text).toEqual('Sign out successful') + }) + }) +}) + +// Helper functions +const requestForOtp = async (email: string) => { + // Set that so no real mail is sent. + jest.spyOn(MailService, 'sendLoginOtp').mockResolvedValue(true) + + const response = await request.post('/auth/sendotp').send({ email }) + expect(response.text).toEqual(`OTP sent to ${email}!`) +} + +const signInUser = async (email: string, otp: string) => { + await requestForOtp(email) + const response = await request.post('/auth/verifyotp').send({ email, otp }) + cookieStore.handleCookie(response) + + // Assert + // Should have session cookie returned. + expect(cookieStore.get()).toEqual(expect.stringContaining('connect.sid')) + return response.body +} diff --git a/src/app/modules/auth/__tests__/auth.service.spec.ts b/src/app/modules/auth/__tests__/auth.service.spec.ts new file mode 100644 index 0000000000..540e0ed17d --- /dev/null +++ b/src/app/modules/auth/__tests__/auth.service.spec.ts @@ -0,0 +1,166 @@ +import mongoose from 'mongoose' +import dbHandler from 'tests/unit/backend/helpers/jest-db' +import { ImportMock } from 'ts-mock-imports' + +import getTokenModel from 'src/app/models/token.server.model' +import * as OtpUtils from 'src/app/utils/otp' +import { IAgencySchema } from 'src/types' + +import { InvalidDomainError, InvalidOtpError } from '../auth.errors' +import * as AuthService from '../auth.service' + +const TokenModel = getTokenModel(mongoose) + +const VALID_EMAIL_DOMAIN = 'test.gov.sg' +const VALID_EMAIL = `valid@${VALID_EMAIL_DOMAIN}` +const MOCK_OTP = '123456' + +// All calls to generateOtp will return MOCK_OTP. +ImportMock.mockFunction(OtpUtils, 'generateOtp', MOCK_OTP) + +describe('auth.service', () => { + let defaultAgency: IAgencySchema + + beforeAll(async () => { + await dbHandler.connect() + defaultAgency = await dbHandler.insertDefaultAgency({ + mailDomain: VALID_EMAIL_DOMAIN, + }) + }) + + // Only need to clear Token collection, and ignore other collections. + beforeEach( + async () => + await dbHandler.clearCollection(TokenModel.collection.collectionName), + ) + + afterAll(async () => await dbHandler.closeDatabase()) + + describe('getAgencyWithEmail', () => { + it('should retrieve agency successfully when email is valid and domain is in Agency collection', async () => { + // Act + const actual = await AuthService.getAgencyWithEmail(VALID_EMAIL) + + // Assert + expect(actual.toObject()).toEqual(defaultAgency.toObject()) + }) + + it('should throw InvalidDomainError when email is invalid', async () => { + // Arrange + const notAnEmail = 'not an email' + + // Act + const actualPromise = AuthService.getAgencyWithEmail(notAnEmail) + + // Assert + await expect(actualPromise).rejects.toThrowError(InvalidDomainError) + }) + + it('should throw InvalidDomainError when valid email domain is not in Agency collection', async () => { + // Arrange + const invalidEmail = 'invalid@example.com' + + // Act + const actualPromise = AuthService.getAgencyWithEmail(invalidEmail) + + // Assert + await expect(actualPromise).rejects.toThrowError(InvalidDomainError) + }) + }) + + describe('createLoginOtp', () => { + it('should create login otp successfully when email is valid', async () => { + // Arrange + // Should have no documents prior to this. + await expect(TokenModel.countDocuments()).resolves.toEqual(0) + + // Act + const actualOtp = await AuthService.createLoginOtp(VALID_EMAIL) + + // Assert + expect(actualOtp).toEqual(MOCK_OTP) + // Should have new token document inserted. + await expect(TokenModel.countDocuments()).resolves.toEqual(1) + }) + + it('should throw InvalidDomainError when email is invalid', async () => { + // Arrange + const notAnEmail = 'not an email' + + // Act + const actualPromise = AuthService.createLoginOtp(notAnEmail) + + // Assert + await expect(actualPromise).rejects.toThrowError(InvalidDomainError) + }) + }) + + describe('verifyLoginOtp', () => { + it('should successfully return true and delete Token document when OTP hash matches', async () => { + // Arrange + // Add a Token document to verify against. + await AuthService.createLoginOtp(VALID_EMAIL) + await expect(TokenModel.countDocuments()).resolves.toEqual(1) + + // Act + const actual = await AuthService.verifyLoginOtp(MOCK_OTP, VALID_EMAIL) + + // Assert + // Resolves successfully. + expect(actual).toEqual(true) + // Token document should be removed. + await expect(TokenModel.countDocuments()).resolves.toEqual(0) + }) + + it('should throw InvalidOtpError when Token document cannot be retrieved', async () => { + // Arrange + // No OTP requested; should have no documents prior to acting. + await expect(TokenModel.countDocuments()).resolves.toEqual(0) + + // Act + const verifyPromise = AuthService.verifyLoginOtp(MOCK_OTP, VALID_EMAIL) + + // Assert + const expectedError = new InvalidOtpError( + 'OTP has expired. Please request for a new OTP.', + ) + await expect(verifyPromise).rejects.toThrowError(expectedError) + }) + + it('should throw InvalidOtpError when verification has been attempted too many times', async () => { + // Arrange + // Add a Token document to verify against. + await AuthService.createLoginOtp(VALID_EMAIL) + // Update Token to already have MAX_OTP_ATTEMPTS. + await TokenModel.findOneAndUpdate( + { email: VALID_EMAIL }, + { $inc: { numOtpAttempts: AuthService.MAX_OTP_ATTEMPTS } }, + ) + + // Act + const verifyPromise = AuthService.verifyLoginOtp(MOCK_OTP, VALID_EMAIL) + + // Assert + const expectedError = new InvalidOtpError( + 'You have hit the max number of attempts. Please request for a new OTP.', + ) + await expect(verifyPromise).rejects.toThrowError(expectedError) + }) + + it('should throw InvalidOtpError when the OTP hash does not match', async () => { + // Arrange + // Add a Token document to verify against. + await AuthService.createLoginOtp(VALID_EMAIL) + const invalidOtp = '654321' + + // Act + const verifyPromise = AuthService.verifyLoginOtp(invalidOtp, VALID_EMAIL) + + // Assert + const expectedError = new InvalidOtpError( + 'OTP is invalid. Please try again.', + ) + await expect(verifyPromise).rejects.toThrowError(expectedError) + }) + }) +}) diff --git a/src/app/modules/auth/auth.controller.ts b/src/app/modules/auth/auth.controller.ts new file mode 100644 index 0000000000..6bee99769a --- /dev/null +++ b/src/app/modules/auth/auth.controller.ts @@ -0,0 +1,202 @@ +import to from 'await-to-js' +import { Request, RequestHandler } from 'express' +import { ParamsDictionary } from 'express-serve-static-core' +import { StatusCodes } from 'http-status-codes' +import { isEmpty } from 'lodash' + +import { createLoggerWithLabel } from '../../../config/logger' +import { LINKS } from '../../../shared/constants' +import MailService from '../../services/mail.service' +import { getRequestIp } from '../../utils/request' +import { ApplicationError } from '../core/core.errors' +import * as UserService from '../user/user.service' + +import * as AuthService from './auth.service' +import { ResponseAfter, SessionUser } from './auth.types' + +const logger = createLoggerWithLabel(module) + +/** + * Precondition: AuthMiddlewares.validateDomain must precede this handler. + * @returns 200 regardless, assumed to have passed domain validation. + */ +export const handleCheckUser: RequestHandler = async ( + _req: Request, + res: ResponseAfter['validateDomain'], +) => { + return res.sendStatus(StatusCodes.OK) +} + +/** + * Precondition: AuthMiddlewares.validateDomain must precede this handler. + */ +export const handleLoginSendOtp: RequestHandler = async ( + req: Request, + res: ResponseAfter['validateDomain'], +) => { + // Joi validation ensures existence. + const { email } = req.body + const requestIp = getRequestIp(req) + const logMeta = { + action: 'handleSendLoginOtp', + email, + ip: requestIp, + } + + // Create OTP. + const [otpErr, otp] = await to(AuthService.createLoginOtp(email)) + + if (otpErr) { + logger.error({ + message: 'Error generating OTP', + meta: logMeta, + error: otpErr, + }) + return res + .status(StatusCodes.INTERNAL_SERVER_ERROR) + .send( + 'Failed to send login OTP. Please try again later and if the problem persists, contact us.', + ) + } + + // Send OTP. + const [sendErr] = await to( + MailService.sendLoginOtp({ + recipient: email, + otp, + ipAddress: requestIp, + }), + ) + if (sendErr) { + logger.error({ + message: 'Error mailing OTP', + meta: logMeta, + error: sendErr, + }) + + return res + .status(StatusCodes.INTERNAL_SERVER_ERROR) + .send( + 'Error sending OTP. Please try again later and if the problem persists, contact us.', + ) + } + + // Successfully sent login otp. + logger.info({ + message: 'Login OTP sent successfully', + meta: logMeta, + }) + + return res.status(StatusCodes.OK).send(`OTP sent to ${email}!`) +} + +/** + * Precondition: AuthMiddlewares.validateDomain must precede this handler. + */ +export const handleLoginVerifyOtp: RequestHandler = async ( + req: Request< + ParamsDictionary, + string | SessionUser, + { email: string; otp: string } + >, + res: ResponseAfter['validateDomain'], +) => { + // Joi validation ensures existence. + const { email, otp } = req.body + // validateDomain middleware will populate agency. + const { agency } = res.locals + + const logMeta = { + action: 'handleLoginVerifyOtp', + email, + ip: getRequestIp(req), + } + + const [verifyErr] = await to(AuthService.verifyLoginOtp(otp, email)) + + if (verifyErr) { + logger.warn({ + message: + verifyErr instanceof ApplicationError + ? 'Login OTP is invalid' + : 'Error occurred when trying to validate login OTP', + meta: logMeta, + error: verifyErr, + }) + + if (verifyErr instanceof ApplicationError) { + return res.status(verifyErr.status).send(verifyErr.message) + } + + // Unknown error, return generic error response. + return res + .status(StatusCodes.INTERNAL_SERVER_ERROR) + .send( + 'Failed to validate OTP. Please try again later and if the problem persists, contact us.', + ) + } + + // OTP is valid, proceed to login user. + try { + const user = await UserService.retrieveUser(email, agency) + // Create user object to return to frontend. + const userObj = { ...user.toObject(), agency } + + if (!req.session) { + throw new Error('req.session not found') + } + + // TODO(#212): Should store only userId in session. + // Add user info to session. + req.session.user = userObj as SessionUser + logger.info({ + message: `Successfully logged in user ${user.email}`, + meta: logMeta, + }) + + return res.status(StatusCodes.OK).send(userObj) + } catch (err) { + logger.error({ + message: 'Error logging in user', + meta: logMeta, + error: err, + }) + + return res + .status(StatusCodes.INTERNAL_SERVER_ERROR) + .send( + `User signin failed. Please try again later and if the problem persists, submit our Support Form (${LINKS.supportFormLink}).`, + ) + } +} + +export const handleSignout: RequestHandler = async (req, res) => { + if (isEmpty(req.session)) { + logger.error({ + message: 'Attempted to sign out without a session', + meta: { + action: 'handleSignout', + }, + }) + return res.sendStatus(StatusCodes.BAD_REQUEST) + } + + req.session.destroy((error) => { + if (error) { + logger.error({ + message: 'Failed to destroy session', + meta: { + action: 'handleSignout', + }, + error, + }) + return res + .status(StatusCodes.INTERNAL_SERVER_ERROR) + .send('Sign out failed') + } + + // No error. + res.clearCookie('connect.sid') + return res.status(StatusCodes.OK).send('Sign out successful') + }) +} diff --git a/src/app/modules/auth/auth.errors.ts b/src/app/modules/auth/auth.errors.ts new file mode 100644 index 0000000000..3f6ae71de2 --- /dev/null +++ b/src/app/modules/auth/auth.errors.ts @@ -0,0 +1,17 @@ +import { StatusCodes } from 'http-status-codes' + +import { ApplicationError } from '../core/core.errors' + +export class InvalidDomainError extends ApplicationError { + constructor( + message = 'This is not a whitelisted public service email domain. Please log in with your official government or government-linked email address.', + ) { + super(message, StatusCodes.UNAUTHORIZED) + } +} + +export class InvalidOtpError extends ApplicationError { + constructor(message = 'OTP has expired. Please request for a new OTP.') { + super(message, StatusCodes.UNPROCESSABLE_ENTITY) + } +} diff --git a/src/app/modules/auth/auth.middlewares.ts b/src/app/modules/auth/auth.middlewares.ts new file mode 100644 index 0000000000..7a19f652e8 --- /dev/null +++ b/src/app/modules/auth/auth.middlewares.ts @@ -0,0 +1,58 @@ +import to from 'await-to-js' +import { RequestHandler } from 'express' +import { ParamsDictionary } from 'express-serve-static-core' +import { StatusCodes } from 'http-status-codes' + +import { createLoggerWithLabel } from '../../../config/logger' +import { LINKS } from '../../../shared/constants' +import { getRequestIp } from '../../utils/request' +import { ApplicationError } from '../core/core.errors' + +import * as AuthService from './auth.service' + +const logger = createLoggerWithLabel(module) + +/** + * Middleware to check if domain of email in the body is from a whitelisted + * agency. + * @returns 500 when there was an error validating email + * @returns 401 when email domain is invalid + * @returns sets retrieved agency in `res.locals.agency` and calls next when domain is valid + */ +export const validateDomain: RequestHandler< + ParamsDictionary, + string, + { email: string } +> = async (req, res, next) => { + // Joi validation ensures existence. + const { email } = req.body + + const [validationError, agency] = await to( + AuthService.getAgencyWithEmail(email), + ) + + if (validationError) { + logger.error({ + message: 'Domain validation error', + meta: { + action: 'validateDomain', + ip: getRequestIp(req), + email, + }, + error: validationError, + }) + if (validationError instanceof ApplicationError) { + return res.status(validationError.status).send(validationError.message) + } + return res + .status(StatusCodes.INTERNAL_SERVER_ERROR) + .send( + `Unable to validate email domain. If this issue persists, please submit a Support Form at (${LINKS.supportFormLink}).`, + ) + } + + // Pass down agency to next handler. + res.locals.agency = agency + + return next() +} diff --git a/src/app/modules/auth/auth.routes.ts b/src/app/modules/auth/auth.routes.ts new file mode 100644 index 0000000000..8584040a04 --- /dev/null +++ b/src/app/modules/auth/auth.routes.ts @@ -0,0 +1,97 @@ +import { celebrate, Joi, Segments } from 'celebrate' +import { Router } from 'express' + +import * as AuthController from './auth.controller' +import * as AuthMiddlewares from './auth.middlewares' + +export const AuthRouter = Router() + +/** + * Check if email domain is a valid agency + * @route POST /auth/checkuser + * @group admin + * @param body.email the user's email to validate domain for + * @return 200 when email domain is valid + * @return 401 when email domain is invalid + */ +AuthRouter.post( + '/checkuser', + celebrate({ + [Segments.BODY]: Joi.object().keys({ + email: Joi.string() + .required() + .email() + .message('Please enter a valid email'), + }), + }), + AuthMiddlewares.validateDomain, + AuthController.handleCheckUser, +) + +/** + * Send a one-time password (OTP) to the specified email address + * as part of the login procedure. + * @route POST /auth/sendotp + * @group admin + * @param body.email the user's email to validate domain for + * @produces application/json + * @consumes application/json + * @return 200 when OTP has been been successfully sent + * @return 401 when email domain is invalid + * @return 500 when FormSG was unable to generate the OTP, or create/send the email that delivers the OTP to the user's email address + */ +AuthRouter.post( + '/sendotp', + celebrate({ + [Segments.BODY]: Joi.object().keys({ + email: Joi.string() + .required() + .email() + .message('Please enter a valid email'), + }), + }), + AuthMiddlewares.validateDomain, + AuthController.handleLoginSendOtp, +) + +/** + * Verify the one-time password (OTP) for the specified email address + * as part of the login procedure. + * @route POST /auth/verifyotp + * @group admin + * @param body.email the user's email + * @param body.otp the otp to verify + * @headers 200.set-cookie contains the session cookie upon login + * @returns 200 when user has successfully logged in, with session cookie set + * @returns 401 when the email domain is invalid + * @returns 422 when the OTP is invalid + * @returns 500 when error occurred whilst verifying the OTP + */ +AuthRouter.post( + '/verifyotp', + celebrate({ + [Segments.BODY]: Joi.object().keys({ + email: Joi.string() + .required() + .email() + .message('Please enter a valid email'), + otp: Joi.string() + .required() + .regex(/^\d{6}$/) + .message('Please enter a valid otp'), + }), + }), + AuthMiddlewares.validateDomain, + AuthController.handleLoginVerifyOtp, +) + +/** + * Sign the user out of the session by clearing the relevant session cookie + * @route GET /auth/signout + * @group admin + * @headers 200.clear-cookie clears cookie upon signout + * @returns 200 when user has signed out successfully + * @returns 400 when the request does not contain a session + * @returns 500 when the session fails to be destroyed + */ +AuthRouter.get('/signout', AuthController.handleSignout) diff --git a/src/app/modules/auth/auth.service.ts b/src/app/modules/auth/auth.service.ts new file mode 100644 index 0000000000..ac6d34f4b6 --- /dev/null +++ b/src/app/modules/auth/auth.service.ts @@ -0,0 +1,105 @@ +import bcrypt from 'bcrypt' +import mongoose from 'mongoose' +import validator from 'validator' + +import config from '../../../config/config' +import getAgencyModel from '../../models/agency.server.model' +import getTokenModel from '../../models/token.server.model' +import { generateOtp } from '../../utils/otp' + +import { InvalidDomainError, InvalidOtpError } from './auth.errors' + +const TokenModel = getTokenModel(mongoose) +const AgencyModel = getAgencyModel(mongoose) + +const DEFAULT_SALT_ROUNDS = 10 +export const MAX_OTP_ATTEMPTS = 10 + +/** + * Validates the domain of the given email. A domain is valid if it exists in + * the Agency collection in the database. + * @param email the email to validate the domain for + * @returns the agency document with the domain of the email only if it is valid. + * @throws error if database query fails or if agency cannot be found. + */ +export const getAgencyWithEmail = async (email: string) => { + // Extra guard even if Joi validation has already checked. + if (!validator.isEmail(email)) { + throw new InvalidDomainError() + } + + const emailDomain = email.split('@').pop() + const agency = await AgencyModel.findOne({ emailDomain }) + if (!agency) { + throw new InvalidDomainError() + } + + return agency +} + +/** + * Creates a login OTP and saves its hash into the Token collection. + * @param email the email to link the generated otp to + * @returns the generated OTP if saving into DB is successful + * @throws {InvalidDomainError} the given email is invalid + * @throws {Error} if any error occur whilst creating the OTP or insertion of OTP into the database. + */ +export const createLoginOtp = async (email: string) => { + if (!validator.isEmail(email)) { + throw new InvalidDomainError() + } + + const otp = generateOtp() + const hashedOtp = await bcrypt.hash(otp, DEFAULT_SALT_ROUNDS) + + await TokenModel.upsertOtp({ + email, + hashedOtp, + expireAt: new Date(Date.now() + config.otpLifeSpan), + }) + + return otp +} + +/** + * Compares the given otp with their hashed counterparts in the database to be + * retrieved with the given email. + * + * If the hash matches, the saved document in the database is removed and + * returned. Else, the document is kept in the database and an error is thrown. + * @param otpToVerify the OTP to verify with the hashed counterpart + * @param email the email used to retrieve the document from the database + * @returns true on success + * @throws {InvalidOtpError} if the OTP is invalid or expired. + * @throws {Error} if any errors occur whilst retrieving from database or comparing hashes. + */ +export const verifyLoginOtp = async (otpToVerify: string, email: string) => { + const updatedDocument = await TokenModel.incrementAttemptsByEmail(email) + + // Does not exist, return expired error message. + if (!updatedDocument) { + throw new InvalidOtpError('OTP has expired. Please request for a new OTP.') + } + + // Too many attempts. + if (updatedDocument.numOtpAttempts! > MAX_OTP_ATTEMPTS) { + throw new InvalidOtpError( + 'You have hit the max number of attempts. Please request for a new OTP.', + ) + } + + // Compare otp with saved hash. + const isOtpMatch = await bcrypt.compare( + otpToVerify, + updatedDocument.hashedOtp, + ) + + if (!isOtpMatch) { + throw new InvalidOtpError('OTP is invalid. Please try again.') + } + + // Hashed OTP matches, remove from collection. + await TokenModel.findOneAndRemove({ email }) + // Finally return true (as success). + return true +} diff --git a/src/app/modules/auth/auth.types.ts b/src/app/modules/auth/auth.types.ts new file mode 100644 index 0000000000..861c0e7a98 --- /dev/null +++ b/src/app/modules/auth/auth.types.ts @@ -0,0 +1,13 @@ +import { IAgencySchema, IPopulatedUser } from 'src/types' + +import { ResponseWithLocals } from '../core/core.types' + +/** + * Meta typing for the shape of the Express.Response object after various + * middlewares for /auth. + */ +export type ResponseAfter = { + validateDomain: ResponseWithLocals<{ agency?: IAgencySchema }> +} + +export type SessionUser = IPopulatedUser diff --git a/src/app/modules/bounce/bounce.controller.ts b/src/app/modules/bounce/bounce.controller.ts index 561287ad48..b8ba7fad70 100644 --- a/src/app/modules/bounce/bounce.controller.ts +++ b/src/app/modules/bounce/bounce.controller.ts @@ -1,4 +1,5 @@ -import { Request, RequestHandler, Response } from 'express' +import { RequestHandler } from 'express' +import { ParamsDictionary } from 'express-serve-static-core' import { StatusCodes } from 'http-status-codes' import { createLoggerWithLabel } from '../../../config/logger' @@ -14,7 +15,7 @@ const logger = createLoggerWithLabel(module) * @param res - Express response object */ export const handleSns: RequestHandler< - Record, + ParamsDictionary, never, ISnsNotification > = async (req, res) => { diff --git a/src/app/modules/core/core.errors.ts b/src/app/modules/core/core.errors.ts index 9f43d02939..5c66fdfacb 100644 --- a/src/app/modules/core/core.errors.ts +++ b/src/app/modules/core/core.errors.ts @@ -2,7 +2,7 @@ * A custom base error class that encapsulates the name, message, status code, * and logging meta string (if any) for the error. */ -export class ApplicationError extends Error { +export abstract class ApplicationError extends Error { /** * Http status code for the error to be returned in the response. */ diff --git a/src/app/modules/core/core.types.ts b/src/app/modules/core/core.types.ts new file mode 100644 index 0000000000..9c1011c90c --- /dev/null +++ b/src/app/modules/core/core.types.ts @@ -0,0 +1,3 @@ +import { Response } from 'express' + +export type ResponseWithLocals = Omit & { locals: T } diff --git a/src/app/modules/user/user.controller.ts b/src/app/modules/user/user.controller.ts index 88975c1269..8a941ad0c3 100644 --- a/src/app/modules/user/user.controller.ts +++ b/src/app/modules/user/user.controller.ts @@ -1,5 +1,6 @@ import to from 'await-to-js' import { RequestHandler } from 'express' +import { ParamsDictionary } from 'express-serve-static-core' import { StatusCodes } from 'http-status-codes' import { createLoggerWithLabel } from '../../../config/logger' @@ -23,7 +24,7 @@ const logger = createLoggerWithLabel(module) * @returns 400 on OTP creation or SMS send failure */ export const handleContactSendOtp: RequestHandler< - Record, + ParamsDictionary, string, { contact: string; userId: string } > = async (req, res) => { @@ -57,7 +58,7 @@ export const handleContactSendOtp: RequestHandler< * @returns 500 when OTP is malformed or for unknown errors */ export const handleContactVerifyOtp: RequestHandler< - Record, + ParamsDictionary, string | IPopulatedUser, { userId: string diff --git a/src/app/modules/user/user.service.ts b/src/app/modules/user/user.service.ts index 955a935017..7bad460194 100644 --- a/src/app/modules/user/user.service.ts +++ b/src/app/modules/user/user.service.ts @@ -1,18 +1,20 @@ import to from 'await-to-js' import bcrypt from 'bcrypt' import mongoose from 'mongoose' +import validator from 'validator' import getAdminVerificationModel from '../../../app/models/admin_verification.server.model' import { AGENCY_SCHEMA_ID } from '../../../app/models/agency.server.model' import getUserModel from '../../../app/models/user.server.model' import { generateOtp } from '../../../app/utils/otp' import config from '../../../config/config' -import { IPopulatedUser, IUserSchema } from '../../../types' +import { IAgency, IPopulatedUser, IUserSchema } from '../../../types' +import { InvalidDomainError } from '../auth/auth.errors' import { InvalidOtpError, MalformedOtpError } from './user.errors' -const AdminVerification = getAdminVerificationModel(mongoose) -const User = getUserModel(mongoose) +const AdminVerificationModel = getAdminVerificationModel(mongoose) +const UserModel = getUserModel(mongoose) const DEFAULT_SALT_ROUNDS = 10 export const MAX_OTP_ATTEMPTS = 10 @@ -29,7 +31,7 @@ export const createContactOtp = async ( contact: string, ): Promise => { // Verify existence of userId - const admin = await User.findById(userId) + const admin = await UserModel.findById(userId) if (!admin) { throw new Error('User id is invalid') } @@ -38,7 +40,7 @@ export const createContactOtp = async ( const hashedOtp = await bcrypt.hash(otp, DEFAULT_SALT_ROUNDS) const hashedContact = await bcrypt.hash(contact, DEFAULT_SALT_ROUNDS) - await AdminVerification.upsertOtp({ + await AdminVerificationModel.upsertOtp({ admin: userId, expireAt: new Date(Date.now() + config.otpLifeSpan), hashedContact, @@ -65,7 +67,7 @@ export const verifyContactOtp = async ( contactToVerify: string, userId: IUserSchema['_id'], ) => { - const updatedDocument = await AdminVerification.incrementAttemptsByAdminId( + const updatedDocument = await AdminVerificationModel.incrementAttemptsByAdminId( userId, ) @@ -114,7 +116,7 @@ export const verifyContactOtp = async ( } // Hashed OTP matches, remove from collection. - await AdminVerification.findOneAndRemove({ admin: userId }) + await AdminVerificationModel.findOneAndRemove({ admin: userId }) // Finally return true (as success). return true } @@ -133,7 +135,7 @@ export const updateUserContact = async ( ): Promise => { // Retrieve user from database. // Update user's contact details. - const admin = await User.findById(userId).populate({ + const admin = await UserModel.findById(userId).populate({ path: 'agency', model: AGENCY_SCHEMA_ID, }) @@ -148,8 +150,34 @@ export const updateUserContact = async ( export const getPopulatedUserById = async ( userId: IUserSchema['_id'], ): Promise => { - return User.findById(userId).populate({ + return UserModel.findById(userId).populate({ path: 'agency', model: AGENCY_SCHEMA_ID, }) } + +/** + * Retrieves the user with the given email. If the user does not yet exist, a + * new user document is created and returned. + * @param email the email of the user to retrieve + * @param agency the agency document to associate with the user + * @returns the upserted user document + * @throws {InvalidDomainError} on invalid email + * @throws {Error} on upsert failure + */ +export const retrieveUser = async (email: string, agency: IAgency) => { + if (!validator.isEmail(email)) { + throw new InvalidDomainError() + } + + const admin = await UserModel.upsertUser({ + email, + agency: agency._id, + }) + + if (!admin) { + throw new Error('Failed to upsert user') + } + + return admin +} diff --git a/src/app/routes/authentication.server.routes.js b/src/app/routes/authentication.server.routes.js deleted file mode 100755 index e8884701b0..0000000000 --- a/src/app/routes/authentication.server.routes.js +++ /dev/null @@ -1,119 +0,0 @@ -'use strict' - -/** - * Module dependencies. - */ -const { StatusCodes } = require('http-status-codes') -const { celebrate, Joi } = require('celebrate') - -let auth = require('../../app/controllers/authentication.server.controller') -const emailValOpts = { - minDomainSegments: 2, // Number of segments required for the domain - tlds: true, // TLD (top level domain) validation - multiple: false, // Disallow multiple emails -} - -module.exports = function (app) { - /** - * @typedef Email - * @property {string} email.required - the user's email - */ - - /** - * Check if email domain is a valid agency - * @route POST /auth/checkuser - * @group admin - form administration - * @param {Email.model} email.body.required - the user's email - * @produces application/json - * @consumes application/json - * @returns {Boolean} 200 - indicates if user has logged in before - * @returns {string} 400 - a message indicating either a bad email address - */ - app.route('/auth/checkuser').post( - celebrate({ - body: Joi.object().keys({ - email: Joi.string() - .required() - .email(emailValOpts) - .error(() => 'Please enter a valid email'), - }), - }), - auth.validateDomain, - (_, res) => res.sendStatus(StatusCodes.OK), - ) - - /** - * Send a one-time password (OTP) to the specified email address - * as part of login procedure - * @route POST /auth/sendotp - * @group admin - form administration - * @param {Email.model} email.body.required - the user's email - * @produces application/json - * @consumes application/json - * @returns {string} 200 - OTP has been been successfully sent - * @returns {string} 400 - a message indicating either a bad email address, or that - * the agency indicated in the email address has not been onboarded to FormSG - * @returns {string} 500 - FormSG was unable to generate the OTP, or create or send - * the email that delivers the OTP to the user's email address - */ - app.route('/auth/sendotp').post( - celebrate({ - body: Joi.object().keys({ - email: Joi.string() - .required() - .email(emailValOpts) - .error(() => 'Please enter a valid email'), - }), - }), - auth.validateDomain, - auth.createOtp, - auth.sendOtp, - ) - - /** - * @typedef EmailAndOtp - * @property {string} email.required - the user's email - * @property {string} otp.required - the OTP provided by the user - */ - - /** - * Verify the one-time password (OTP) for the specified email address - * as part of login procedure - * @route POST /auth/verifyotp - * @group admin - form administration - * @param {EmailAndOtp.model} email.body.required - the user's email and otp - * @produces application/json - * @consumes application/json - * @returns {string} 200 - user has successfully logged in, with session cookie set - * @returns {string} 400 - the OTP is invalid or has expired, or the email is invalid - * @returns {string} 500 - FormSG was unable to verify the OTP - * @headers {string} 200.set-cookie - contains the session cookie upon login - */ - app.route('/auth/verifyotp').post( - celebrate({ - body: Joi.object().keys({ - email: Joi.string() - .required() - .email(emailValOpts) - .error(() => 'Please enter a valid email'), - otp: Joi.string() - .required() - .regex(/^\d{6}$/) - .error(() => 'Please enter a valid otp'), - }), - }), - auth.validateDomain, - auth.verifyOtp, - auth.signIn, - ) - - /** - * Sign the user out of the session by clearing the relevant session cookie - * @route GET /auth/signout - * @group admin - form administration - * @produces application/json - * @returns {string} 200 - user has signed out - * @returns {string} 400 - the signout failed for one reason or another - */ - app.route('/auth/signout').get(auth.signOut) -} diff --git a/src/app/routes/index.js b/src/app/routes/index.js index 3dacdafc3a..dde5caee35 100644 --- a/src/app/routes/index.js +++ b/src/app/routes/index.js @@ -1,7 +1,6 @@ module.exports = [ require('./admin-console.server.routes.js'), require('./admin-forms.server.routes.js'), - require('./authentication.server.routes.js'), require('./core.server.routes.js'), require('./frontend.server.routes.js'), require('./public-forms.server.routes.js'), diff --git a/src/loaders/express/index.ts b/src/loaders/express/index.ts index 0d87af2eaa..38b517e1c3 100644 --- a/src/loaders/express/index.ts +++ b/src/loaders/express/index.ts @@ -7,6 +7,7 @@ import { Connection } from 'mongoose' import path from 'path' import url from 'url' +import { AuthRouter } from '../../app/modules/auth/auth.routes' import { BounceRouter } from '../../app/modules/bounce/bounce.routes' import UserRouter from '../../app/modules/user/user.routes' import { VfnRouter } from '../../app/modules/verification/verification.routes' @@ -123,6 +124,8 @@ const loadExpressApp = async (connection: Connection) => { apiRoutes.forEach(function (routeFunction) { routeFunction(app) }) + + app.use('/auth', AuthRouter) app.use('/user', UserRouter) app.use('/emailnotifications', BounceRouter) app.use('/transaction', VfnRouter) diff --git a/src/types/token.ts b/src/types/token.ts index 446515ff38..f808a833d7 100644 --- a/src/types/token.ts +++ b/src/types/token.ts @@ -1,12 +1,22 @@ -import { Document } from 'mongoose' +import { Document, Model } from 'mongoose' export interface IToken { email: string hashedOtp: string expireAt: Date - numOtpAttempts: number - numOtpSent: number + numOtpAttempts?: number + numOtpSent?: number + _id?: Document['_id'] +} + +export interface ITokenSchema extends IToken, Document { _id: Document['_id'] } -export interface ITokenSchema extends IToken, Document {} +export interface ITokenModel extends Model { + upsertOtp: ( + params: Omit, + ) => Promise + + incrementAttemptsByEmail: (email: IToken['email']) => Promise +} diff --git a/src/types/user.ts b/src/types/user.ts index 961ea7404b..01ab753ab1 100644 --- a/src/types/user.ts +++ b/src/types/user.ts @@ -1,4 +1,4 @@ -import { Document } from 'mongoose' +import { Document, Model } from 'mongoose' import { IAgencySchema } from './agency' @@ -19,6 +19,12 @@ export interface IUserSchema extends IUser, Document { _id: Document['_id'] } +export interface IUserModel extends Model { + upsertUser: ( + upsertParams: Pick, + ) => Promise +} + export interface IPopulatedUser extends IUserSchema { agency: IAgencySchema } diff --git a/tests/integration/helpers/express-setup.ts b/tests/integration/helpers/express-setup.ts new file mode 100644 index 0000000000..994c51853f --- /dev/null +++ b/tests/integration/helpers/express-setup.ts @@ -0,0 +1,63 @@ +import compression from 'compression' +import express, { Router } from 'express' +import helmet from 'helmet' +import mongoose from 'mongoose' +import { Response } from 'supertest' + +import errorHandlerMiddlewares from 'src/loaders/express/error-handler' +import helmetMiddlewares from 'src/loaders/express/helmet' +import loggingMiddleware from 'src/loaders/express/logging' +import parserMiddlewares from 'src/loaders/express/parser' +import sessionMiddlewares from 'src/loaders/express/session' + +export const setupApp = ( + route: string, + router: Router, + options: { showLogs?: boolean } = {}, +) => { + const app = express() + + app.use(compression()) + app.use(parserMiddlewares()) + app.use(helmetMiddlewares()) + app.use(helmet.noCache()) + + app.use(sessionMiddlewares(mongoose.connection)) + + if (options.showLogs) { + app.use(loggingMiddleware()) + } + + app.use(route, router) + + app.use(errorHandlerMiddlewares()) + + return app +} + +/** + * Helper class to store cookies for routes that require authorization. + * @example + * request + .post('/some/route/that/requires/auth') + .set('cookie', cookieStore.get()); + */ +export class CookieStore { + #currentCookie: string = '' + + handleCookie(res: Response) { + this.set(res.header['set-cookie'][0]) + } + + set(cookie: string) { + this.#currentCookie = cookie + } + + get() { + return this.#currentCookie + } + + clear() { + this.#currentCookie = '' + } +} diff --git a/tests/unit/backend/controllers/authentication.server.controller.spec.js b/tests/unit/backend/controllers/authentication.server.controller.spec.js index 3b980fc620..5935f8466c 100644 --- a/tests/unit/backend/controllers/authentication.server.controller.spec.js +++ b/tests/unit/backend/controllers/authentication.server.controller.spec.js @@ -6,9 +6,6 @@ let roles = require('../helpers/roles') let permissionLevels = require('../../../../dist/backend/app/utils/permission-levels') .default -const User = dbHandler.makeModel('user.server.model', 'User') -const Token = dbHandler.makeModel('token.server.model', 'Token') - describe('Authentication Controller', () => { const TEST_OTP = '123456' const bcrypt = jasmine.createSpyObj('bcrypt', ['hash']) @@ -31,7 +28,6 @@ describe('Authentication Controller', () => { let req let res let testForm - let testAgency beforeAll(async () => await dbHandler.connect()) afterEach(async () => await dbHandler.clearDatabase()) @@ -61,7 +57,6 @@ describe('Authentication Controller', () => { }) // Insert test form before each test - testAgency = collections.agency testForm = collections.form }) @@ -83,47 +78,6 @@ describe('Authentication Controller', () => { }) }) - describe('validateDomain', () => { - const expectErrorCode = (email, code, done) => { - req.body = { email } - res.status.and.callFake(() => { - expect(res.status).toHaveBeenCalledWith(code) - done() - return res - }) - Controller.validateDomain(req, res) - } - - let invalidEmails = [ - 'falal@@al.gov.sg', - 'falalalsov.sg', - 'a@tech.gov.sg@,b@gmail.com', - 'a@tech.gov.sg,b@gmail.com', - 'b@gmail.com@,a@tech.gov.sg', - ] - - for (let email of invalidEmails) { - it(`should return 401 when email is ${email}`, (done) => { - expectErrorCode(email, 401, done) - }) - } - it('should return 401 if email domain is not found', (done) => { - expectErrorCode('falal@al.gov.sg', 401, done) - }) - it('should pass on to the next middleware if email domain is valid', (done) => { - const email = 'falal@test.gov.sg' - req.body = { email } - let next = jasmine.createSpy() - Controller.validateDomain(req, res, next) - next.and.callFake(() => { - expect(res.locals.email).toEqual(email) - expect(res.locals.agency._id).toEqual(testAgency._id) - expect(next).toHaveBeenCalled() - done() - }) - }) - }) - describe('hasFormAdminAuthorization', () => { it('should authorize if session user is admin', () => { let next = jasmine.createSpy() @@ -158,256 +112,4 @@ describe('Authentication Controller', () => { Controller.verifyPermission(permissionLevels.WRITE)(req, res, () => {}) }) }) - - describe('createOtp', () => { - it('should store token in db and pass on to next middleware if agency exists', (done) => { - const email = 'test@test.gov.sg' - res.locals = { email } - let next = jasmine.createSpy() - let fakeHash = '2131290312983213' - bcrypt.hash.and.callFake((otp, numRounds, callback) => { - callback(null, fakeHash) - expect(bcrypt.hash).toHaveBeenCalled() - }) - let expectedRecord = { - email, - hashedOtp: fakeHash, - numOtpAttempts: 0, - expireAt: Date.now() + 15 * 60 * 1000, - } - next.and.callFake(() => { - Token.findOne({ email }, (err, token) => { - expect(err).not.toBeTruthy() - expect(token.email).toEqual(expectedRecord.email) - expect(token.hashedOtp).toEqual(expectedRecord.hashedOtp) - expect(token.numOtpAttempts).toEqual(expectedRecord.numOtpAttempts) - // Precise to the last 4 digits - expect(token.expireAt.getTime()).toBeCloseTo( - expectedRecord.expireAt, - -4, - ) - expect(res.locals.otp).toEqual(TEST_OTP) - expect(res.locals.expireAt.getTime()).toEqual( - token.expireAt.getTime(), - ) - done() - }) - }) - Controller.createOtp(req, res, next) - }) - - it('should reset numOtpAttempts if token exists for email', (done) => { - res.locals = { email: req.session.user.email } - - let testToken = new Token({ - email: req.session.user.email, - hashedOtp: '12938710928312038109', - expireAt: Date.now(), - numOtpAttempts: 3, - }) - - bcrypt.hash.and.callFake((otp, numRounds, callback) => { - callback(null, '129830128310283018') - expect(bcrypt.hash).toHaveBeenCalled() - }) - - let next = jasmine.createSpy().and.callFake(() => { - Token.findOne({ email: req.session.user.email }, (err, token) => { - if (err || !token) { - done(err || new Error('Token not found')) - } else { - expect(token.numOtpAttempts).toEqual(0) - done() - } - }) - }) - testToken.save().then(() => { - Controller.createOtp(req, res, next) - }) - }) - }) - - describe('sendOtp', () => { - it('should send email and return a 200', (done) => { - res.app = { - locals: { title: 'AppTitle' }, - } - res.locals = { - otp: TEST_OTP, - expireAt: Date.now(), - email: req.session.user.email, - } - res.render = jasmine.createSpy().and.callFake((arg1, arg2, callback) => { - callback(null, '') - }) - res.status.and.callFake(() => { - done() - return res - }) - // Mock the callback with no errors - mockSendNodeMail.and.callFake((_mail, cb) => { - cb() - }) - Controller.sendOtp(req, res) - }) - }) - - describe('verifyOtp', () => { - // Legit hash for "123456" - let testHash = - '$2b$10$wPte/.r1R9xa6Xm0WMKdC.JGeH2ajHHziDH1txi7hKMzgqr8CabcC' - it('should return 422 if OTP has expired/ does not exist', (done) => { - res.locals.email = req.session.user.email - req.body.otp = TEST_OTP - res.status.and.callFake(() => { - expect(res.status).toHaveBeenCalledWith( - StatusCodes.UNPROCESSABLE_ENTITY, - ) - done() - return res - }) - Controller.verifyOtp(req, res, () => {}) - }) - - it('should return 422 if max attempts reached', (done) => { - res.locals.email = req.session.user.email - req.body.otp = TEST_OTP - let testToken = new Token({ - email: req.session.user.email, - hashedOtp: testHash, - expireAt: 0, - numOtpAttempts: 10, - }) - res.status.and.callFake(() => { - expect(res.status).toHaveBeenCalledWith( - StatusCodes.UNPROCESSABLE_ENTITY, - ) - done() - return res - }) - testToken.save().then(() => { - Controller.verifyOtp(req, res, () => {}) - }) - }) - - it('should return 401 if otp is wrong', (done) => { - res.locals.email = req.session.user.email - req.body.otp = '000000' - let testToken = new Token({ - email: req.session.user.email, - hashedOtp: testHash, - expireAt: 0, - numOtpAttempts: 0, - }) - res.status.and.callFake(() => { - expect(res.status).toHaveBeenCalledWith(StatusCodes.UNAUTHORIZED) - done() - return res - }) - testToken.save().then(() => { - Controller.verifyOtp(req, res, () => {}) - }) - }) - - it('should return 200 and token removed from db if otp is correct', (done) => { - res.locals.email = req.session.user.email - req.body.otp = TEST_OTP - let testToken = new Token({ - email: req.session.user.email, - hashedOtp: testHash, - expireAt: 0, - numOtpAttempts: 0, - }) - let next = jasmine.createSpy().and.callFake(() => { - expect(next).toHaveBeenCalled() - Token.findOne({ email: req.body.email }, (err, tokenFound) => { - expect(err).not.toBeTruthy() - expect(tokenFound).toBeNull() - done() - }) - }) - testToken.save().then(() => { - Controller.verifyOtp(req, res, next) - }) - }) - }) - - describe('signIn', () => { - it('should insert user if new valid email', (done) => { - const email = 'newuser@test.gov.sg' - res.locals = { email, agency: testAgency } - res.status.and.callFake(() => { - return res - }) - res.send.and.callFake((args) => { - expect(args.email).toEqual(email) - expect(args.agency.toObject()).toEqual(testAgency.toObject()) - expect(req.session.user).toEqual(args) - User.findOne({ email }, (err, user) => { - if (err || !user) { - done(err || new Error('User not found')) - } else { - let userObj = user.toObject() - expect(userObj.agency).toEqual(testAgency._id) - expect(userObj.created.getTime()).toBeCloseTo(Date.now(), -4) - done() - } - }) - }) - Controller.signIn(req, res) - }) - }) - - describe('signOut', () => { - it('should clear cookie connect.sid on sign out', (done) => { - req.session.destroy = jasmine.createSpy().and.callFake((cb) => { - expect(req.session.destroy).toHaveBeenCalled() - // Mock callback with no errors - cb() - expect(res.clearCookie).toHaveBeenCalledWith('connect.sid') - done() - }) - res.clearCookie = jasmine.createSpy() - res.status.and.callFake(() => { - return res - }) - Controller.signOut(req, res) - }) - }) - - describe('doesUserBeta', () => { - const mockBetaFlag = 'fakeBetaFlag' - it('should return 200 when user is beta for a given betaType', (done) => { - req.session.user.betaFlags = { - [mockBetaFlag]: true, - } - let next = jasmine.createSpy().and.callFake(() => { - expect(next).toHaveBeenCalled() - done() - }) - Controller.doesUserBeta(mockBetaFlag)(req, res, next) - }) - - it('should return 403 when user is not beta for a given betaType', (done) => { - req.session.user.betaFlags = { - [mockBetaFlag]: false, - } - res.status.and.callFake(() => { - expect(res.status).toHaveBeenCalledWith(StatusCodes.FORBIDDEN) - done() - return res - }) - Controller.doesUserBeta(mockBetaFlag)(req, res, () => {}) - }) - - it('should return 403 when user is undefined', (done) => { - req.session.user = undefined - res.status.and.callFake(() => { - expect(res.status).toHaveBeenCalledWith(StatusCodes.FORBIDDEN) - done() - return res - }) - Controller.doesUserBeta(mockBetaFlag)(req, res, () => {}) - }) - }) }) diff --git a/tests/unit/backend/helpers/jest-express.ts b/tests/unit/backend/helpers/jest-express.ts index 96fcc1847b..6b04951830 100644 --- a/tests/unit/backend/helpers/jest-express.ts +++ b/tests/unit/backend/helpers/jest-express.ts @@ -5,24 +5,31 @@ const mockRequest = ({ params, session, }: { - body: Record + body?: Record params?: Record session?: any -}) => { +} = {}) => { return { - body, - params, - session, + body: body ?? {}, + params: params ?? {}, + session: session ?? {}, + get(name: string) { + if (name === 'cf-connecting-ip') return 'MOCK_IP' + return null + }, } as Request } -const mockResponse = () => { - const mockRes = {} as Response - mockRes.status = jest.fn().mockReturnThis() - mockRes.send = jest.fn().mockReturnThis() - mockRes.sendStatus = jest.fn().mockReturnThis() - mockRes.json = jest.fn() - return mockRes +const mockResponse = (extraArgs: Partial> = {}) => { + const mockRes = { + locals: {}, + status: jest.fn().mockReturnThis(), + send: jest.fn().mockReturnThis(), + sendStatus: jest.fn().mockReturnThis(), + json: jest.fn(), + ...extraArgs, + } + return mockRes as Response } const expressHandler = { diff --git a/tsconfig.build.json b/tsconfig.build.json index 3976260911..1a236a6031 100644 --- a/tsconfig.build.json +++ b/tsconfig.build.json @@ -1,5 +1,5 @@ // Main tsconfig for building, prevents tests from being compiled. { "extends": "./tsconfig.json", - "exclude": ["tests/"] + "exclude": ["tests/", "**/*.spec.ts", "src/public/"] } diff --git a/tsconfig.json b/tsconfig.json index 2c6eb4cf34..c63b3ba484 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -69,5 +69,4 @@ "forceConsistentCasingInFileNames": true /* Disallow inconsistently-cased references to the same file. */ }, "include": ["src", "tests"], - "exclude": ["src/public"] }