fix(server/passwordreset): prevent user enumeration through brute force (#5633)

packages/frontend-2/lib/auth/composables/passwordReset.ts

@@ -20,7 +20,7 @@ export function usePasswordReset() {
       triggerNotification({
         type: ToastNotificationType.Info,
         title: 'Password reset email sent',
-        description: `We've sent the password reset instructions to ${email}`
+        description: `If the email address '${email}' is associated with a registered user, we have sent password reset instructions to that address.`
       })
     } catch (e) {
       triggerNotification({

packages/server/modules/auth/tests/helpers/registration.ts

@@ -213,7 +213,7 @@ export const localAuthRestApi = (params: { express: Express }) => {
 
     const user = await authCheck({ token })
     expect(user).to.be.ok
-    expect(user.email).to.equal(params.email)
+    expect(user.email.toLowerCase()).to.equal(params.email.toLowerCase())
 
     return user
   }

packages/server/modules/core/utils/ratelimiter.ts

@@ -167,6 +167,16 @@ export const LIMITS = <const>{
       duration: 1 * TIME.minute
     }
   },
+  'POST /auth/pwdreset/request': {
+    regularOptions: {
+      limitCount: getIntFromEnv('RATELIMIT_GET_AUTH', '4'),
+      duration: 10 * TIME.minute
+    },
+    burstOptions: {
+      limitCount: getIntFromEnv('RATELIMIT_BURST_GET_AUTH', '10'),
+      duration: 30 * TIME.minute
+    }
+  },
   '/auth/local/login': {
     regularOptions: {
       limitCount: getIntFromEnv('RATELIMIT_GET_AUTH', '4'),

packages/server/modules/pwdreset/rest/index.ts

@@ -22,12 +22,14 @@ import { BadRequestError } from '@/modules/shared/errors'
 import { withOperationLogging } from '@/observability/domain/businessLogging'
 import { ensureError } from '@speckle/shared'
 import type { Express } from 'express'
+import { UserNotFoundError } from '@/modules/core/errors/user'
 
 export default function (app: Express) {
   const getUserByEmail = getUserByEmailFactory({ db })
 
   // sends a password recovery email.
   app.post('/auth/pwdreset/request', async (req, res) => {
+    const responseMessage = 'Password reset email sent.'
     try {
       const email = req.body.email
       const logger = req.log.child({ email })
@@ -46,10 +48,16 @@ export default function (app: Express) {
         operationDescription: `Requesting password recovery`
       })
 
-      return res.status(200).send('Password reset email sent.')
+      return res.status(200).send(responseMessage)
     } catch (e: unknown) {
-      req.log.info({ err: e }, 'Error while requesting password recovery.')
-      res.status(400).send(ensureError(e).message)
+      const err = ensureError(e, 'Unknown error while requesting password recovery')
+      req.log.info({ err }, 'Error while requesting password recovery.')
+      if (err instanceof UserNotFoundError) {
+        // always 200 and use same response message to avoid user enumeration
+        res.status(200).send(responseMessage)
+      } else {
+        res.status(400).send(err.message)
+      }
     }
   })
 

packages/server/modules/pwdreset/services/request.ts

@@ -1,5 +1,6 @@
 import type { GetServerInfo } from '@/modules/core/domain/server/operations'
 import type { GetUserByEmail } from '@/modules/core/domain/users/operations'
+import { UserNotFoundError } from '@/modules/core/errors/user'
 import { getPasswordResetFinalizationRoute } from '@/modules/core/helpers/routeHelper'
 import type { EmailTemplateParams } from '@/modules/emails/domain/operations'
 import type { renderEmail } from '@/modules/emails/services/emailRendering'
@@ -31,9 +32,7 @@ const initializeNewTokenFactory =
     ])
 
     if (!user) {
-      throw new InvalidPasswordRecoveryRequestError(
-        'No user with that e-mail address found'
-      )
+      throw new UserNotFoundError('No user with that e-mail address found')
     }
 
     if (tokenAlreadyExists) {

packages/server/modules/pwdreset/tests/pwdrest.spec.ts

@@ -7,6 +7,8 @@ import { localAuthRestApi } from '@/modules/auth/tests/helpers/registration'
 import { expectToThrow } from '@/test/assertionHelper'
 import { expect } from 'chai'
 import { createTestUser } from '@/test/authHelper'
+import { createRandomEmail } from '@/modules/core/helpers/testHelpers'
+import cryptoRandomString from 'crypto-random-string'
 
 const ResetTokens = () => knex('pwdreset_tokens')
 
@@ -19,9 +21,9 @@ describe('Password reset requests @passwordresets', () => {
 
   it('Should carefully send a password request email', async () => {
     const userA = await createTestUser({
-      name: 'd1',
-      email: 'd@speckle.systems',
-      password: 'wowwow8charsplease',
+      name: cryptoRandomString({ length: 10 }),
+      email: createRandomEmail(),
+      password: cryptoRandomString({ length: 8 }),
       id: ''
     })
 
@@ -31,8 +33,8 @@ describe('Password reset requests @passwordresets', () => {
     // non-existent user
     await request(app)
       .post('/auth/pwdreset/request')
-      .send({ email: 'doesnot@exist.here' })
-      .expect(400)
+      .send({ email: createRandomEmail() }) // does not exist
+      .expect(200) // always 200 to prevent user enumeration
 
     // good request
     await request(app)
@@ -43,20 +45,20 @@ describe('Password reset requests @passwordresets', () => {
     // already has expiration token, fall back
     await request(app)
       .post('/auth/pwdreset/request')
-      .send({ email: 'd@speckle.systems' })
+      .send({ email: userA.email })
       .expect(400)
   })
 
   it('Should reset passwords', async () => {
     const userB = await createTestUser({
-      name: 'd2',
-      email: 'd2@speckle.systems',
-      password: 'w0ww0w8charsplease',
+      name: cryptoRandomString({ length: 10 }),
+      email: createRandomEmail(),
+      password: cryptoRandomString({ length: 8 }),
       id: ''
     })
 
     const authRestApi = localAuthRestApi({ express: app })
-    const newPassword = '12345678'
+    const newPassword = cryptoRandomString({ length: 8 })
 
     // trigger request
     await request(app)
@@ -88,7 +90,7 @@ describe('Password reset requests @passwordresets', () => {
     // token used up, should fail
     await request(app)
       .post('/auth/pwdreset/finalize')
-      .send({ tokenId: token.id, password: 'abc12345678' })
+      .send({ tokenId: token.id, password: cryptoRandomString({ length: 8 }) })
       .expect(400)
 
     // should be able to log in with new pw