From 301b64574aed640aa336c92499eca03bae1f9859 Mon Sep 17 00:00:00 2001 From: Antariksh Date: Wed, 9 Jun 2021 23:14:58 +0800 Subject: [PATCH 1/5] fix(auth): lowercase auth emails through Joi --- src/app/modules/auth/auth.routes.ts | 3 +++ src/app/routes/api/v3/auth/auth.routes.ts | 3 +++ 2 files changed, 6 insertions(+) diff --git a/src/app/modules/auth/auth.routes.ts b/src/app/modules/auth/auth.routes.ts index bd1d343669..633635ef7c 100644 --- a/src/app/modules/auth/auth.routes.ts +++ b/src/app/modules/auth/auth.routes.ts @@ -23,6 +23,7 @@ AuthRouter.post( email: Joi.string() .required() .email() + .lowercase() .message('Please enter a valid email'), }), }), @@ -49,6 +50,7 @@ AuthRouter.post( email: Joi.string() .required() .email() + .lowercase() .message('Please enter a valid email'), }), }), @@ -75,6 +77,7 @@ AuthRouter.post( email: Joi.string() .required() .email() + .lowercase() .message('Please enter a valid email'), otp: Joi.string() .required() diff --git a/src/app/routes/api/v3/auth/auth.routes.ts b/src/app/routes/api/v3/auth/auth.routes.ts index bf144dc9fb..5bed5f7ce2 100644 --- a/src/app/routes/api/v3/auth/auth.routes.ts +++ b/src/app/routes/api/v3/auth/auth.routes.ts @@ -21,6 +21,7 @@ AuthRouter.post( email: Joi.string() .required() .email() + .lowercase() .message('Please enter a valid email'), }), }), @@ -47,6 +48,7 @@ AuthRouter.post( email: Joi.string() .required() .email() + .lowercase() .message('Please enter a valid email'), }), }), @@ -73,6 +75,7 @@ AuthRouter.post( email: Joi.string() .required() .email() + .lowercase() .message('Please enter a valid email'), otp: Joi.string() .required() From 3db7e3e45877f5295d805aef3396c8ab864f3584 Mon Sep 17 00:00:00 2001 From: Antariksh Date: Thu, 10 Jun 2021 10:07:07 +0800 Subject: [PATCH 2/5] test: add tests for email case-insensitivity --- .../auth/__tests__/auth.routes.spec.ts | 61 +++++++++++++++++++ .../api/v3/auth/__tests__/auth.routes.spec.ts | 61 +++++++++++++++++++ 2 files changed, 122 insertions(+) diff --git a/src/app/modules/auth/__tests__/auth.routes.spec.ts b/src/app/modules/auth/__tests__/auth.routes.spec.ts index 66d44b6430..e5d3749cd3 100644 --- a/src/app/modules/auth/__tests__/auth.routes.spec.ts +++ b/src/app/modules/auth/__tests__/auth.routes.spec.ts @@ -97,6 +97,23 @@ describe('auth.routes', () => { expect(response.text).toEqual('OK') }) + it('should return 200 when domain of body.email has a case-insensitive match 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/checkuser') + .send({ email: validEmail.toUpperCase() }) + + // 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 @@ -251,6 +268,23 @@ describe('auth.routes', () => { expect(response.status).toEqual(200) expect(response.body).toEqual(`OTP sent to ${VALID_EMAIL}`) }) + + it('should return 200 when otp is sent successfully and email is non-lowercase', async () => { + // Arrange + const sendLoginOtpSpy = jest + .spyOn(MailService, 'sendLoginOtp') + .mockReturnValueOnce(okAsync(true)) + + // Act + const response = await request + .post('/auth/sendotp') + .send({ email: VALID_EMAIL.toUpperCase() }) + + // Assert + expect(sendLoginOtpSpy).toHaveBeenCalled() + expect(response.status).toEqual(200) + expect(response.body).toEqual(`OTP sent to ${VALID_EMAIL}`) + }) }) describe('POST /auth/verifyotp', () => { @@ -476,6 +510,33 @@ describe('auth.routes', () => { expect(sessionCookie).toBeDefined() }) + it('should return 200 with user object when body.otp is a valid OTP and body.email is non-lowercase', 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.toUpperCase(), 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: jsonParseStringify(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. 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 index d69a1d18f7..0479032e4f 100644 --- a/src/app/routes/api/v3/auth/__tests__/auth.routes.spec.ts +++ b/src/app/routes/api/v3/auth/__tests__/auth.routes.spec.ts @@ -96,6 +96,23 @@ describe('auth.routes', () => { expect(response.text).toEqual('OK') }) + it('should return 200 when domain of body.email has a case-insensitive match 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/checkuser') + .send({ email: validEmail.toUpperCase() }) + + // 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 @@ -250,6 +267,23 @@ describe('auth.routes', () => { expect(response.status).toEqual(200) expect(response.body).toEqual(`OTP sent to ${VALID_EMAIL}`) }) + + it('should return 200 when otp is sent successfully and email is non-lowercase', async () => { + // Arrange + const sendLoginOtpSpy = jest + .spyOn(MailService, 'sendLoginOtp') + .mockReturnValueOnce(okAsync(true)) + + // Act + const response = await request + .post('/auth/sendotp') + .send({ email: VALID_EMAIL.toUpperCase() }) + + // Assert + expect(sendLoginOtpSpy).toHaveBeenCalled() + expect(response.status).toEqual(200) + expect(response.body).toEqual(`OTP sent to ${VALID_EMAIL}`) + }) }) describe('POST /auth/otp/verify', () => { @@ -475,6 +509,33 @@ describe('auth.routes', () => { expect(sessionCookie).toBeDefined() }) + it('should return 200 with user object when body.otp is a valid OTP and body.email is non-lowercase', 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.toUpperCase(), 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. From 8adbd104d8a175c1276b52d80f01fa7e4713df1d Mon Sep 17 00:00:00 2001 From: Antariksh Date: Thu, 10 Jun 2021 10:34:10 +0800 Subject: [PATCH 3/5] refactor: move lowercase after email validation message --- src/app/modules/auth/auth.routes.ts | 12 ++++++------ src/app/routes/api/v3/auth/auth.routes.ts | 12 ++++++------ 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/app/modules/auth/auth.routes.ts b/src/app/modules/auth/auth.routes.ts index 633635ef7c..69e669fb73 100644 --- a/src/app/modules/auth/auth.routes.ts +++ b/src/app/modules/auth/auth.routes.ts @@ -23,8 +23,8 @@ AuthRouter.post( email: Joi.string() .required() .email() - .lowercase() - .message('Please enter a valid email'), + .message('Please enter a valid email') + .lowercase(), }), }), AuthController.handleCheckUser, @@ -50,8 +50,8 @@ AuthRouter.post( email: Joi.string() .required() .email() - .lowercase() - .message('Please enter a valid email'), + .message('Please enter a valid email') + .lowercase(), }), }), AuthController.handleLoginSendOtp, @@ -77,8 +77,8 @@ AuthRouter.post( email: Joi.string() .required() .email() - .lowercase() - .message('Please enter a valid email'), + .message('Please enter a valid email') + .lowercase(), otp: Joi.string() .required() .regex(/^\d{6}$/) diff --git a/src/app/routes/api/v3/auth/auth.routes.ts b/src/app/routes/api/v3/auth/auth.routes.ts index 5bed5f7ce2..eedc25fff8 100644 --- a/src/app/routes/api/v3/auth/auth.routes.ts +++ b/src/app/routes/api/v3/auth/auth.routes.ts @@ -21,8 +21,8 @@ AuthRouter.post( email: Joi.string() .required() .email() - .lowercase() - .message('Please enter a valid email'), + .message('Please enter a valid email') + .lowercase(), }), }), AuthController.handleCheckUser, @@ -48,8 +48,8 @@ AuthRouter.post( email: Joi.string() .required() .email() - .lowercase() - .message('Please enter a valid email'), + .message('Please enter a valid email') + .lowercase(), }), }), AuthController.handleLoginSendOtp, @@ -75,8 +75,8 @@ AuthRouter.post( email: Joi.string() .required() .email() - .lowercase() - .message('Please enter a valid email'), + .message('Please enter a valid email') + .lowercase(), otp: Joi.string() .required() .regex(/^\d{6}$/) From c66054eb0ca1f9619f5ce136ab38320c51a478f2 Mon Sep 17 00:00:00 2001 From: Antariksh Date: Thu, 10 Jun 2021 10:34:50 +0800 Subject: [PATCH 4/5] test: fix POST endpoint in new tests --- src/app/routes/api/v3/auth/__tests__/auth.routes.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 index 0479032e4f..d6aed891d9 100644 --- a/src/app/routes/api/v3/auth/__tests__/auth.routes.spec.ts +++ b/src/app/routes/api/v3/auth/__tests__/auth.routes.spec.ts @@ -105,7 +105,7 @@ describe('auth.routes', () => { // Act const response = await request - .post('/auth/checkuser') + .post('/auth/email/validate') .send({ email: validEmail.toUpperCase() }) // Assert @@ -276,7 +276,7 @@ describe('auth.routes', () => { // Act const response = await request - .post('/auth/sendotp') + .post('/auth/otp/generate') .send({ email: VALID_EMAIL.toUpperCase() }) // Assert From 967a4a58ac6f3ed9f012bdee730d51a6b6c230af Mon Sep 17 00:00:00 2001 From: Antariksh Date: Thu, 10 Jun 2021 10:35:24 +0800 Subject: [PATCH 5/5] test: fix remaining tests --- .../__tests__/admin-forms.settings.routes.spec.ts | 10 +++++++--- tests/integration/helpers/express-auth.ts | 2 +- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/app/routes/api/v3/admin/forms/__tests__/admin-forms.settings.routes.spec.ts b/src/app/routes/api/v3/admin/forms/__tests__/admin-forms.settings.routes.spec.ts index b5b7f3546a..d100bb20f4 100644 --- a/src/app/routes/api/v3/admin/forms/__tests__/admin-forms.settings.routes.spec.ts +++ b/src/app/routes/api/v3/admin/forms/__tests__/admin-forms.settings.routes.spec.ts @@ -301,7 +301,9 @@ describe('admin-form.settings.routes', () => { }) const session = await createAuthedSession(fakeUser.email, request) const expectedResponse = jsonParseStringify({ - message: `User ${fakeUser.email} not authorized to perform write operation on Form ${form._id} with title: ${form.title}.`, + message: `User ${fakeUser.email.toLowerCase()} not authorized to perform write operation on Form ${ + form._id + } with title: ${form.title}.`, }) // Act @@ -431,12 +433,14 @@ describe('admin-form.settings.routes', () => { }, }) const fakeUser = await dbHandler.insertUser({ - mailName: 'fakeUser', + mailName: 'userWithoutReadPermissions', agencyId: new ObjectId(), }) const session = await createAuthedSession(fakeUser.email, request) const expectedResponse = jsonParseStringify({ - message: `User ${fakeUser.email} not authorized to perform read operation on Form ${form._id} with title: ${form.title}.`, + message: `User ${fakeUser.email.toLowerCase()} not authorized to perform read operation on Form ${ + form._id + } with title: ${form.title}.`, }) // Act diff --git a/tests/integration/helpers/express-auth.ts b/tests/integration/helpers/express-auth.ts index 9c65cc791f..0e6f94117f 100644 --- a/tests/integration/helpers/express-auth.ts +++ b/tests/integration/helpers/express-auth.ts @@ -35,7 +35,7 @@ export const createAuthedSession = async ( const sendOtpResponse = await request.post('/auth/sendotp').send({ email }) expect(sendOtpResponse.status).toEqual(200) - expect(sendOtpResponse.body).toEqual(`OTP sent to ${email}`) + expect(sendOtpResponse.body).toEqual(`OTP sent to ${email.toLowerCase()}`) // Act await request.post('/auth/verifyotp').send({ email, otp: MOCK_VALID_OTP })