From 48a1f2e0da00994438f76e163adc636bda251fda Mon Sep 17 00:00:00 2001 From: Eugene Long Date: Mon, 5 Apr 2021 15:13:40 +0800 Subject: [PATCH 1/2] refactor(auth-api): duplicate auth endpoints to new /api/v3 router - duplicate auth endpoint functionality and update endpoints - update v3 router to use new endpoints - update frontend api calls to use new endpoints --- src/app/routes/api/v3/auth/auth.routes.ts | 95 +++++++++++++++++++ src/app/routes/api/v3/auth/index.ts | 1 + src/app/routes/api/v3/v3.routes.ts | 2 + .../users/services/auth.client.service.js | 8 +- 4 files changed, 102 insertions(+), 4 deletions(-) create mode 100644 src/app/routes/api/v3/auth/auth.routes.ts create mode 100644 src/app/routes/api/v3/auth/index.ts diff --git a/src/app/routes/api/v3/auth/auth.routes.ts b/src/app/routes/api/v3/auth/auth.routes.ts new file mode 100644 index 0000000000..b931d9d6fb --- /dev/null +++ b/src/app/routes/api/v3/auth/auth.routes.ts @@ -0,0 +1,95 @@ +import { celebrate, Joi, Segments } from 'celebrate' +import { Router } from 'express' + +import { rateLimitConfig } from '../../../../../config/config' +import * as AuthController from '../../../../modules/auth/auth.controller' +import { limitRate } from '../../../../utils/limit-rate' + +export const AuthRouter = Router() +/** + * Check if email domain is a valid agency + * @route POST /auth/email/validate + * @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( + '/email/validate', + celebrate({ + [Segments.BODY]: Joi.object().keys({ + email: Joi.string() + .required() + .email() + .message('Please enter a valid email'), + }), + }), + AuthController.handleCheckUser, +) + +/** + * Send a one-time password (OTP) to the specified email address + * as part of the login procedure. + * @route POST /auth/otp/generate + * @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( + '/otp/generate', + limitRate({ max: rateLimitConfig.sendAuthOtp }), + celebrate({ + [Segments.BODY]: Joi.object().keys({ + email: Joi.string() + .required() + .email() + .message('Please enter a valid email'), + }), + }), + AuthController.handleLoginSendOtp, +) + +/** + * Verify the one-time password (OTP) for the specified email address + * as part of the login procedure. + * @route POST /auth/otp/verify + * @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( + '/otp/verify', + 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'), + }), + }), + AuthController.handleLoginVerifyOtp, +) + +/** + * Sign the user out of the session by clearing the relevant session cookie + * @route GET /auth/logout + * @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('/logout', AuthController.handleSignout) diff --git a/src/app/routes/api/v3/auth/index.ts b/src/app/routes/api/v3/auth/index.ts new file mode 100644 index 0000000000..b035f15671 --- /dev/null +++ b/src/app/routes/api/v3/auth/index.ts @@ -0,0 +1 @@ +export { AuthRouter } from './auth.routes' diff --git a/src/app/routes/api/v3/v3.routes.ts b/src/app/routes/api/v3/v3.routes.ts index 8bd36bdd9e..e4a2061fe9 100644 --- a/src/app/routes/api/v3/v3.routes.ts +++ b/src/app/routes/api/v3/v3.routes.ts @@ -1,7 +1,9 @@ import { Router } from 'express' import { AdminRouter } from './admin' +import { AuthRouter } from './auth' export const V3Router = Router() V3Router.use('/admin', AdminRouter) +V3Router.use('/auth', AuthRouter) diff --git a/src/public/modules/users/services/auth.client.service.js b/src/public/modules/users/services/auth.client.service.js index 9be1957080..78e7703c46 100644 --- a/src/public/modules/users/services/auth.client.service.js +++ b/src/public/modules/users/services/auth.client.service.js @@ -69,7 +69,7 @@ function Auth($q, $http, $state, $window) { function checkUser(credentials) { let deferred = $q.defer() - $http.post('/auth/checkuser', credentials).then( + $http.post('/api/v3/auth/email/validate', credentials).then( function (response) { deferred.resolve(response.data) }, @@ -82,7 +82,7 @@ function Auth($q, $http, $state, $window) { function sendOtp(credentials) { let deferred = $q.defer() - $http.post('/auth/sendotp', credentials).then( + $http.post('/api/v3/auth/otp/generate', credentials).then( function (response) { deferred.resolve(response.data) }, @@ -95,7 +95,7 @@ function Auth($q, $http, $state, $window) { function verifyOtp(credentials) { let deferred = $q.defer() - $http.post('/auth/verifyotp', credentials).then( + $http.post('/api/v3/auth/otp/verify', credentials).then( function (response) { setUser(response.data) deferred.resolve() @@ -108,7 +108,7 @@ function Auth($q, $http, $state, $window) { } function signOut() { - $http.get('/auth/signout').then( + $http.get('/api/v3/auth/logout').then( function () { $window.localStorage.removeItem('user') // Clear contact banner on logout From c83d7a97a4d4bdcd7704b107b15f6422d4f3bee3 Mon Sep 17 00:00:00 2001 From: Eugene Long Date: Mon, 5 Apr 2021 15:55:59 +0800 Subject: [PATCH 2/2] test(auth-api): duplicate integration tests for new endpoint --- .../api/v3/auth/__tests__/auth.routes.spec.ts | 564 ++++++++++++++++++ 1 file changed, 564 insertions(+) create mode 100644 src/app/routes/api/v3/auth/__tests__/auth.routes.spec.ts diff --git a/src/app/routes/api/v3/auth/__tests__/auth.routes.spec.ts b/src/app/routes/api/v3/auth/__tests__/auth.routes.spec.ts new file mode 100644 index 0000000000..d69a1d18f7 --- /dev/null +++ b/src/app/routes/api/v3/auth/__tests__/auth.routes.spec.ts @@ -0,0 +1,564 @@ +import { pick } from 'lodash' +import { errAsync, okAsync } from 'neverthrow' +import supertest, { Session } from 'supertest-session' +import validator from 'validator' + +import MailService from 'src/app/services/mail/mail.service' +import { HashingError } from 'src/app/utils/hash' +import * as OtpUtils from 'src/app/utils/otp' +import { IAgencySchema } from 'src/types' + +import { setupApp } from 'tests/integration/helpers/express-setup' +import { buildCelebrateError } from 'tests/unit/backend/helpers/celebrate' +import dbHandler from 'tests/unit/backend/helpers/jest-db' + +import * as AuthService from '../../../../../modules/auth/auth.service' +import { DatabaseError } from '../../../../../modules/core/core.errors' +import * as UserService from '../../../../../modules/user/user.service' +import { MailSendError } from '../../../../../services/mail/mail.errors' +import { AuthRouter } from '../auth.routes' + +const app = setupApp('/auth', AuthRouter) + +describe('auth.routes', () => { + let request: Session + + beforeAll(async () => await dbHandler.connect()) + beforeEach(() => { + request = supertest(app) + }) + afterEach(async () => { + await dbHandler.clearDatabase() + jest.restoreAllMocks() + }) + afterAll(async () => await dbHandler.closeDatabase()) + + describe('POST /auth/email/validate', () => { + it('should return 400 when body.email is not provided as a param', async () => { + // Act + const response = await request.post('/auth/email/validate') + + // Assert + expect(response.status).toEqual(400) + expect(response.body).toEqual( + buildCelebrateError({ body: { key: 'email' } }), + ) + }) + + it('should return 400 when body.email is invalid', async () => { + // Arrange + const invalidEmail = 'not an email' + + // Act + const response = await request + .post('/auth/email/validate') + .send({ email: invalidEmail }) + + // Assert + expect(response.status).toEqual(400) + expect(response.body).toEqual( + buildCelebrateError({ + body: { key: 'email', message: '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/email/validate') + .send({ email: validEmailWithInvalidDomain }) + + // Assert + expect(response.status).toEqual(401) + expect(response.body).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.insertAgency({ mailDomain: validDomain }) + + // Act + const response = await request + .post('/auth/email/validate') + .send({ email: validEmail }) + + // Assert + expect(response.status).toEqual(200) + expect(response.text).toEqual('OK') + }) + + it('should return 500 when validating domain returns a database error', async () => { + // Arrange + // Insert agency + const validDomain = 'example.com' + const validEmail = `test@${validDomain}` + const mockErrorString = 'Unable to validate email domain.' + await dbHandler.insertAgency({ mailDomain: validDomain }) + + const getAgencySpy = jest + .spyOn(AuthService, 'validateEmailDomain') + .mockReturnValueOnce(errAsync(new DatabaseError(mockErrorString))) + + // Act + const response = await request + .post('/auth/email/validate') + .send({ email: validEmail }) + + // Assert + expect(getAgencySpy).toBeCalled() + expect(response.status).toEqual(500) + expect(response.body).toEqual(mockErrorString) + }) + }) + + describe('POST /auth/otp/generate', () => { + const VALID_DOMAIN = 'example.com' + const VALID_EMAIL = `test@${VALID_DOMAIN}` + const INVALID_DOMAIN = 'example.org' + + beforeEach(async () => dbHandler.insertAgency({ mailDomain: VALID_DOMAIN })) + + it('should return 400 when body.email is not provided as a param', async () => { + // Act + const response = await request.post('/auth/otp/generate') + + // Assert + expect(response.status).toEqual(400) + expect(response.body).toEqual( + buildCelebrateError({ body: { key: 'email' } }), + ) + }) + + it('should return 400 when body.email is invalid', async () => { + // Arrange + const invalidEmail = 'not an email' + + // Act + const response = await request + .post('/auth/otp/generate') + .send({ email: invalidEmail }) + + // Assert + expect(response.status).toEqual(400) + expect(response.body).toEqual( + buildCelebrateError({ + body: { key: 'email', message: '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/otp/generate') + .send({ email: validEmailWithInvalidDomain }) + + // Assert + expect(response.status).toEqual(401) + expect(response.body).toEqual({ + message: + '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') + .mockReturnValueOnce(errAsync(new HashingError())) + + // Act + const response = await request + .post('/auth/otp/generate') + .send({ email: VALID_EMAIL }) + + // Assert + expect(createLoginOtpSpy).toHaveBeenCalled() + expect(response.status).toEqual(500) + expect(response.body).toEqual({ + message: + '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') + .mockReturnValueOnce(errAsync(new MailSendError('some error'))) + + // Act + const response = await request + .post('/auth/otp/generate') + .send({ email: VALID_EMAIL }) + + // Assert + expect(sendLoginOtpSpy).toHaveBeenCalled() + expect(response.status).toEqual(500) + expect(response.body).toEqual({ + message: + 'Failed to send login OTP. Please try again later and if the problem persists, contact us.', + }) + }) + + it('should return 500 when validating domain returns a database error', async () => { + // Arrange + const getAgencySpy = jest + .spyOn(AuthService, 'validateEmailDomain') + .mockReturnValueOnce(errAsync(new DatabaseError())) + + // Act + const response = await request + .post('/auth/otp/generate') + .send({ email: VALID_EMAIL }) + + // Assert + expect(getAgencySpy).toBeCalled() + expect(response.status).toEqual(500) + expect(response.body).toEqual({ + message: + 'Failed to send login OTP. Please try again later and if the problem persists, contact us.', + }) + }) + + it('should return 200 when otp is sent successfully', async () => { + // Arrange + const sendLoginOtpSpy = jest + .spyOn(MailService, 'sendLoginOtp') + .mockReturnValueOnce(okAsync(true)) + + // Act + const response = await request + .post('/auth/otp/generate') + .send({ email: VALID_EMAIL }) + + // Assert + expect(sendLoginOtpSpy).toHaveBeenCalled() + expect(response.status).toEqual(200) + expect(response.body).toEqual(`OTP sent to ${VALID_EMAIL}`) + }) + }) + + describe('POST /auth/otp/verify', () => { + 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.insertAgency({ + 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/otp/verify').send({ + otp: MOCK_VALID_OTP, + }) + + // Assert + expect(response.status).toEqual(400) + expect(response.body).toEqual( + buildCelebrateError({ body: { key: 'email' } }), + ) + }) + + it('should return 400 when body.otp is not provided as a param', async () => { + // Act + const response = await request.post('/auth/otp/verify').send({ + email: VALID_EMAIL, + }) + + // Assert + expect(response.status).toEqual(400) + expect(response.body).toEqual( + buildCelebrateError({ body: { key: 'otp' } }), + ) + }) + + it('should return 400 when body.email is invalid', async () => { + // Arrange + const invalidEmail = 'not an email' + + // Act + const response = await request + .post('/auth/otp/verify') + .send({ email: invalidEmail, otp: MOCK_VALID_OTP }) + + // Assert + expect(response.status).toEqual(400) + expect(response.body).toEqual( + buildCelebrateError({ + body: { key: 'email', message: '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/otp/verify').send({ + email: VALID_EMAIL, + otp: '12345', + }) + + // Assert + expect(response.status).toEqual(400) + expect(response.body).toEqual( + buildCelebrateError({ + body: { key: 'otp', message: '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/otp/verify').send({ + email: VALID_EMAIL, + otp: '123abc', + }) + + // Assert + expect(response.status).toEqual(400) + expect(response.body).toEqual( + buildCelebrateError({ + body: { key: 'otp', message: '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/otp/verify') + .send({ email: validEmailWithInvalidDomain, otp: MOCK_VALID_OTP }) + + // Assert + expect(response.status).toEqual(401) + expect(response.body).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 returns a database error', async () => { + // Arrange + const getAgencySpy = jest + .spyOn(AuthService, 'validateEmailDomain') + .mockReturnValueOnce(errAsync(new DatabaseError())) + + // Act + const response = await request + .post('/auth/otp/verify') + .send({ email: VALID_EMAIL, otp: MOCK_VALID_OTP }) + + // Assert + expect(getAgencySpy).toBeCalled() + expect(response.status).toEqual(500) + expect(response.body).toEqual('Something went wrong. Please try again.') + }) + + it('should return 422 when hash does not exist for body.otp', async () => { + // Act + const response = await request + .post('/auth/otp/verify') + .send({ email: VALID_EMAIL, otp: MOCK_VALID_OTP }) + + // Assert + expect(response.status).toEqual(422) + expect(response.body).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/otp/verify') + .send({ email: VALID_EMAIL, otp: invalidOtp }) + + // Assert + expect(response.status).toEqual(422) + expect(response.body).toEqual('OTP is invalid. Please try again.') + }) + + it('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/otp/verify') + .send({ email: VALID_EMAIL, otp: invalidOtp }), + ) + } + const results = (await Promise.all(verifyPromises)).map((resolve) => + pick(resolve, ['status', 'body']), + ) + // Should be all invalid OTP responses. + expect(results).toEqual( + Array(AuthService.MAX_OTP_ATTEMPTS).fill({ + status: 422, + body: 'OTP is invalid. Please try again.', + }), + ) + + // Act again, this time with a valid OTP. + const response = await request + .post('/auth/otp/verify') + .send({ email: VALID_EMAIL, otp: MOCK_VALID_OTP }) + + // Assert + // Should still reject with max OTP attempts error. + expect(response.status).toEqual(422) + expect(response.body).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/otp/verify') + .send({ email: VALID_EMAIL, otp: MOCK_VALID_OTP }) + + // 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. + const sessionCookie = request.cookies.find( + (cookie) => cookie.name === 'connect.sid', + ) + expect(sessionCookie).toBeDefined() + }) + + it('should return 500 when upserting user document fails', async () => { + // Arrange + // Request for OTP so the hash exists. + await requestForOtp(VALID_EMAIL) + + // Mock error returned when creating user + const upsertSpy = jest + .spyOn(UserService, 'retrieveUser') + .mockReturnValueOnce(errAsync(new DatabaseError('some error'))) + + // Act + const response = await request + .post('/auth/otp/verify') + .send({ email: VALID_EMAIL, otp: MOCK_VALID_OTP }) + + // Assert + // Should have reached this spy. + expect(upsertSpy).toBeCalled() + expect(response.status).toEqual(500) + expect(response.body).toEqual( + expect.stringContaining('Failed to process OTP.'), + ) + }) + }) + + describe('GET /auth/logout', () => { + const MOCK_VALID_OTP = '123456' + const VALID_DOMAIN = 'example.com' + const VALID_EMAIL = `test@${VALID_DOMAIN}` + + beforeEach(async () => { + await dbHandler.insertAgency({ + 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/logout') + + // Assert + expect(response.status).toEqual(200) + expect(response.body).toEqual({ message: '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 () => { + // Note that no log in calls have been made with request yet. + // Act + const response = await request.get('/auth/logout') + + // Assert + expect(response.status).toEqual(200) + expect(response.body).toEqual({ message: 'Sign out successful' }) + }) + }) + + // Helper functions + const requestForOtp = async (email: string) => { + // Set that so no real mail is sent. + jest.spyOn(MailService, 'sendLoginOtp').mockReturnValue(okAsync(true)) + + const response = await request.post('/auth/otp/generate').send({ email }) + expect(response.body).toEqual(`OTP sent to ${email}`) + } + + const signInUser = async (email: string, otp: string) => { + await requestForOtp(email) + const response = await request.post('/auth/otp/verify').send({ email, otp }) + + // Assert + // Should have session cookie returned. + const sessionCookie = request.cookies.find( + (cookie) => cookie.name === 'connect.sid', + ) + expect(sessionCookie).toBeDefined() + return response.body + } +})