Forgot Password Back-End TLC

packages/common/index.ts

@@ -3542,6 +3542,17 @@ export const ERROR_MESSAGES = {
       'Event date is in the past. No AutoCheckout will occur. Please unassign staff from event.',
     invalidRecurringEvent: 'Recurring events must have a start and end date',
   },
+  authController: {
+    userNotFoundWithEmail: 'No user exists with this email',
+    ssoAccountGoogle:
+      'This email is an SSO account and has no password to reset. Please login with "Continue with Google" instead.',
+    ssoAccountShibboleth: (organizationName: string) =>
+      `This email is an SSO account and has no password to reset. Please login with "Continue with ${organizationName}" instead.`,
+    incorrectAccountType:
+      'This account is the incorrect type and has no password to reset.',
+    invalidRecaptchaToken: 'Invalid recaptcha token',
+    emailNotVerified: 'Email not verified',
+  },
   organizationController: {
     notEnoughDiskSpace: 'Not enough disk space to upload file',
     userAlreadyInOrganization: 'User is already in organization',

packages/server/src/auth/auth.controller.ts

@@ -37,6 +37,8 @@ import { JwtAuthGuard } from 'guards/jwt-auth.guard';
 import * as bcrypt from 'bcrypt';
 import { getCookie } from '../common/helpers';
 import { CourseService } from 'course/course.service';
+import { ILike } from 'typeorm';
+import * as Sentry from '@sentry/nestjs';
 
 interface RequestUser {
   userId: string;
@@ -272,48 +274,99 @@ export class AuthController {
     });
   }
 
+  /* Should probably be renamed to forgotPassword() */
   @Post('/password/reset')
   async resetPassword(
     @Body() body: PasswordRequestResetBody,
     @Res() res: Response,
   ): Promise<Response<void>> {
     const { email, recaptchaToken, organizationId } = body;
 
-    if (!recaptchaToken) {
-      return res
-        .status(HttpStatus.BAD_REQUEST)
-        .send({ message: 'Invalid recaptcha token' });
-    }
-
     const response = await request.post(
       `https://www.google.com/recaptcha/api/siteverify?secret=${process.env.PRIVATE_RECAPTCHA_SITE_KEY}&response=${recaptchaToken}`,
     );
 
     if (!response.body.success) {
       return res.status(HttpStatus.BAD_REQUEST).send({
-        message: 'Recaptcha token invalid',
+        message: ERROR_MESSAGES.authController.invalidRecaptchaToken,
       });
     }
 
-    const user = await UserModel.findOne({
+    const users = await UserModel.find({
       where: {
-        email,
+        email: ILike(email), // Case insensitive search (since users can mistype emails)
         organizationUser: { organizationId },
-        accountType: AccountType.LEGACY,
       },
-      relations: ['organizationUser'],
+      relations: {
+        organizationUser: {
+          organization: true,
+        },
+      },
     });
+    let user: UserModel;
 
-    if (!user) {
+    if (!users || users.length === 0) {
+      return res
+        .status(HttpStatus.NOT_FOUND)
+        .send({ message: ERROR_MESSAGES.authController.userNotFoundWithEmail });
+    } else if (users.length === 1) {
+      // this is like 99.9% of users
+      user = users[0];
+    } else {
+      Sentry.captureMessage(
+        'Multiple users found with same email (can be case-sensitivity issue or multiple accounts with same type).',
+        {
+          level: 'error',
+          extra: {
+            users: users.map((user) => ({
+              id: user.id,
+              // email: user.email, // decided against logging email in sentry since then sentry would be collecting emails and UBC PIA probably won't like that
+            })),
+          },
+        },
+      );
+
+      const usersWithLegacyAccountType = users.filter(
+        (user) => user.accountType === AccountType.LEGACY,
+      );
+
+      // Find the legacy account and use it if it exists (Note that it shouldn't actually be possible to have multiple accounts with same email and different types)
+      if (usersWithLegacyAccountType.length === 1) {
+        user = usersWithLegacyAccountType[0];
+        // If there isn't one, use the first user.
+      } else if (usersWithLegacyAccountType.length === 0) {
+        user = users[0];
+        // If there's multiple legacy accounts, it's a case-sensitivity issue.
+      } else if (usersWithLegacyAccountType.length > 1) {
+        // this SHOULDN'T happen since emails should be unique. But, /register lacked case-insensitivity so some users have multiple accounts with same email (with different case).
+        return res.status(HttpStatus.BAD_REQUEST).send({
+          message:
+            'Multiple users found with this email (can be case-sensitivity issue). Please contact adam.fipke@ubc.ca',
+        });
+      }
+    }
+
+    // now handle logic for the user
+    if (user.accountType === AccountType.GOOGLE) {
+      return res
+        .status(HttpStatus.BAD_REQUEST)
+        .send({ message: ERROR_MESSAGES.authController.ssoAccountGoogle });
+    } else if (user.accountType === AccountType.SHIBBOLETH) {
+      return res.status(HttpStatus.BAD_REQUEST).send({
+        message: ERROR_MESSAGES.authController.ssoAccountShibboleth(
+          user.organizationUser.organization.name,
+        ),
+      });
+    } else if (user.accountType !== AccountType.LEGACY) {
       return res
         .status(HttpStatus.BAD_REQUEST)
-        .send({ message: 'User not found' });
+        .send({ message: ERROR_MESSAGES.authController.incorrectAccountType });
     }
 
     if (!user.emailVerified) {
       return res
         .status(HttpStatus.BAD_REQUEST)
-        .send({ message: 'Email not verified' });
+        .send({ message: ERROR_MESSAGES.authController.emailNotVerified });
     }
 
     const resetLink = await this.authService.createPasswordResetToken(user);

packages/server/test/auth.integration.ts

@@ -9,7 +9,7 @@ import {
   UserFactory,
 } from './util/factories';
 import { AuthService } from 'auth/auth.service';
-import { AccountType } from '@koh/common';
+import { AccountType, ERROR_MESSAGES } from '@koh/common';
 import { MailService } from 'mail/mail.service';
 import { OrganizationUserModel } from 'organization/organization-user.entity';
 import { UserModel } from 'profile/user.entity';
@@ -20,6 +20,7 @@ import {
 } from 'profile/user-token.entity';
 import { JwtAuthGuard } from 'guards/jwt-auth.guard';
 import { ExecutionContext, Injectable } from '@nestjs/common';
+import { OrganizationModel } from 'organization/organization.entity';
 
 const mockJWT = {
   signAsync: async (payload) => JSON.stringify(payload),
@@ -518,62 +519,94 @@ describe('Auth Integration', () => {
   });
 
   describe('POST password/reset', () => {
-    it('should return BAD REQUEST when Google returned false for recaptcha', () => {
-      return supertest()
-        .post('/auth/password/reset')
-        .send({
-          email: 'email.com',
-          recaptchaToken: 'invalid',
-          organizationId: 1,
-        })
-        .expect(400);
+    let user: UserModel;
+    let organization: OrganizationModel;
+    beforeEach(async () => {
+      user = await UserFactory.create({
+        email: 'email.com',
+        emailVerified: true,
+      });
+      organization = await OrganizationFactory.create();
+      await OrganizationUserModel.create({
+        organizationId: organization.id,
+        userId: user.id,
+      }).save();
+    });
+    it('should return BAD REQUEST when recaptcha token is missing', async () => {
+      const res = await supertest().post('/auth/password/reset').send({
+        email: user.email,
+        organizationId: organization.id,
+      });
+      expect(res.status).toBe(400);
+    });
+    it('should return BAD REQUEST when Google returned false for recaptcha', async () => {
+      const res = await supertest().post('/auth/password/reset').send({
+        email: user.email,
+        recaptchaToken: 'invalid',
+        organizationId: organization.id,
+      });
+      expect(res.status).toBe(400);
+      expect(res.body.message).toBe(
+        ERROR_MESSAGES.authController.invalidRecaptchaToken,
+      );
     });
+    it('should return NOT FOUND when user does not exist', async () => {
+      const res = await supertest().post('/auth/password/reset').send({
+        email: 'email.notfound@email.com',
+        recaptchaToken: 'token',
+        organizationId: organization.id,
+      });
 
-    it('should return BAD REQUEST when user does not exist', async () => {
-      const organization = await OrganizationFactory.create();
+      expect(res.status).toBe(404);
+      expect(res.body.message).toBe(
+        ERROR_MESSAGES.authController.userNotFoundWithEmail,
+      );
+    });
+    it('should return BAD REQUEST when email is not verified', async () => {
+      user.emailVerified = false;
+      await user.save();
 
       const res = await supertest().post('/auth/password/reset').send({
-        email: 'email.com',
+        email: user.email,
         recaptchaToken: 'token',
         organizationId: organization.id,
       });
 
       expect(res.status).toBe(400);
+      expect(res.body.message).toBe(
+        ERROR_MESSAGES.authController.emailNotVerified,
+      );
     });
+    it('should return BAD REQUEST when account is an SSO account (google)', async () => {
+      user.accountType = AccountType.GOOGLE;
+      await user.save();
 
-    it('should return BAD REQUEST when email is not verified', async () => {
-      const organization = await OrganizationFactory.create();
-      const user = await UserFactory.create({
-        email: 'email.com',
-        emailVerified: false,
+      const res = await supertest().post('/auth/password/reset').send({
+        email: user.email,
+        recaptchaToken: 'token',
+        organizationId: organization.id,
       });
 
-      await OrganizationUserModel.create({
-        organizationId: organization.id,
-        userId: user.id,
-      }).save();
+      expect(res.status).toBe(400);
+      expect(res.body.message).toBe(
+        ERROR_MESSAGES.authController.ssoAccountGoogle,
+      );
+    });
+    it('should return BAD REQUEST when account is an SSO account (shibboleth)', async () => {
+      user.accountType = AccountType.SHIBBOLETH;
+      await user.save();
 
       const res = await supertest().post('/auth/password/reset').send({
         email: user.email,
         recaptchaToken: 'token',
         organizationId: organization.id,
       });
-
       expect(res.status).toBe(400);
+      expect(res.body.message).toBe(
+        ERROR_MESSAGES.authController.ssoAccountShibboleth(organization.name),
+      );
     });
-
     it('should return ACCEPTED when email is sent', async () => {
-      const organization = await OrganizationFactory.create();
-      const user = await UserFactory.create({
-        email: 'email.com',
-        emailVerified: true,
-      });
-
-      await OrganizationUserModel.create({
-        organizationId: organization.id,
-        userId: user.id,
-      }).save();
-
       const res = await supertest().post('/auth/password/reset').send({
         email: user.email,
         recaptchaToken: 'token',
@@ -582,6 +615,14 @@ describe('Auth Integration', () => {
 
       expect(res.status).toBe(202);
     });
+    it('should return ACCEPTED when email is different case', async () => {
+      const res = await supertest().post('/auth/password/reset').send({
+        email: user.email.toUpperCase(),
+        recaptchaToken: 'token',
+        organizationId: organization.id,
+      });
+      expect(res.status).toBe(202);
+    });
   });
 
   describe('POST password/reset/:token', () => {