misc: addressed PR comments

backend/src/db/schemas/users.ts

@@ -23,14 +23,11 @@ export const UsersSchema = z.object({
   isGhost: z.boolean().default(false),
   username: z.string(),
   isEmailVerified: z.boolean().default(false).nullable().optional(),
-  consecutiveFailedMfaAttempts: z.number(),
+  consecutiveFailedMfaAttempts: z.number().optional(),
-  isLocked: z.boolean(),
+  isLocked: z.boolean().optional(),
   temporaryLockDateEnd: z.date().nullable().optional()
 });
 
 export type TUsers = z.infer<typeof UsersSchema>;
-export type TUsersInsert = Omit<
+export type TUsersInsert = Omit<z.input<typeof UsersSchema>, TImmutableDBKeys>;
-  z.input<typeof UsersSchema>,
-  TImmutableDBKeys | "isLocked" | "consecutiveFailedMfaAttempts"
->;
 export type TUsersUpdate = Partial<Omit<z.input<typeof UsersSchema>, TImmutableDBKeys>>;

backend/src/server/routes/v1/user-router.ts

@@ -32,7 +32,7 @@ export const registerUserRouter = async (server: FastifyZodProvider) => {
 
   server.route({
     method: "GET",
-    url: "/:userId/unlock-verify",
+    url: "/:userId/unlock",
     config: {
       rateLimit: authRateLimit
     },

backend/src/services/auth/auth-fns.ts

@@ -50,7 +50,7 @@ export const enforceUserLockStatus = (isLocked: boolean, temporaryLockDateEnd?:
     throw new UnauthorizedError({
       name: "User Locked",
       message:
-        "User is locked due to multiple failed login attempts. An email has been sent to you in order to unlock your account."
+        "User is locked due to multiple failed login attempts. An email has been sent to you in order to unlock your account. You can also reset your password to unlock."
     });
   }
 

backend/src/services/auth/auth-login-service.ts

@@ -4,7 +4,7 @@ import { TUsers, UserDeviceSchema } from "@app/db/schemas";
 import { isAuthMethodSaml } from "@app/ee/services/permission/permission-fns";
 import { getConfig } from "@app/lib/config/env";
 import { generateSrpServerKey, srpCheckClientProof } from "@app/lib/crypto";
-import { BadRequestError, UnauthorizedError } from "@app/lib/errors";
+import { BadRequestError, DatabaseError, UnauthorizedError } from "@app/lib/errors";
 import { getServerCfg } from "@app/services/super-admin/super-admin-service";
 
 import { TTokenDALFactory } from "../auth-token/auth-token-dal";
@@ -13,7 +13,6 @@ import { TokenType } from "../auth-token/auth-token-types";
 import { TOrgDALFactory } from "../org/org-dal";
 import { SmtpTemplates, TSmtpService } from "../smtp/smtp-service";
 import { TUserDALFactory } from "../user/user-dal";
-import { processFailedMfaAttempt } from "../user/user-fns";
 import { enforceUserLockStatus, validateProviderAuthToken } from "./auth-fns";
 import {
   TLoginClientProofDTO,
@@ -214,7 +213,7 @@ export const authLoginServiceFactory = ({
     // send multi factor auth token if they it enabled
     if (userEnc.isMfaEnabled && userEnc.email) {
       const user = await userDAL.findById(userEnc.userId);
-      enforceUserLockStatus(user.isLocked, user.temporaryLockDateEnd);
+      enforceUserLockStatus(Boolean(user.isLocked), user.temporaryLockDateEnd);
 
       const mfaToken = jwt.sign(
         {
@@ -304,13 +303,61 @@ export const authLoginServiceFactory = ({
   const resendMfaToken = async (userId: string) => {
     const user = await userDAL.findById(userId);
     if (!user || !user.email) return;
-    enforceUserLockStatus(user.isLocked, user.temporaryLockDateEnd);
+    enforceUserLockStatus(Boolean(user.isLocked), user.temporaryLockDateEnd);
     await sendUserMfaCode({
       userId: user.id,
       email: user.email
     });
   };
 
+  const processFailedMfaAttempt = async (userId: string) => {
+    try {
+      const updatedUser = await userDAL.transaction(async (tx) => {
+        const PROGRESSIVE_DELAY_INTERVAL = 3;
+        const user = await userDAL.incrementFailedMfaAttempt(userId, tx);
+
+        if (!user) {
+          throw new Error("User not found");
+        }
+
+        const progressiveDelaysInMins = [5, 30, 60];
+
+        // lock user when failed attempt exceeds threshold
+        if (
+          user.consecutiveFailedMfaAttempts &&
+          user.consecutiveFailedMfaAttempts >= PROGRESSIVE_DELAY_INTERVAL * (progressiveDelaysInMins.length + 1)
+        ) {
+          return userDAL.updateById(
+            userId,
+            {
+              isLocked: true,
+              temporaryLockDateEnd: null
+            },
+            tx
+          );
+        }
+
+        // delay user only when failed MFA attempts is a multiple of configured delay interval
+        if (user.consecutiveFailedMfaAttempts && user.consecutiveFailedMfaAttempts % PROGRESSIVE_DELAY_INTERVAL === 0) {
+          const delayIndex = user.consecutiveFailedMfaAttempts / PROGRESSIVE_DELAY_INTERVAL - 1;
+          return userDAL.updateById(
+            userId,
+            {
+              temporaryLockDateEnd: new Date(new Date().getTime() + progressiveDelaysInMins[delayIndex] * 60 * 1000)
+            },
+            tx
+          );
+        }
+
+        return user;
+      });
+
+      return updatedUser;
+    } catch (error) {
+      throw new DatabaseError({ error, name: "Process failed MFA Attempt" });
+    }
+  };
+
   /*
    * Multi factor authentication verification of code
    * Third step of login in which user completes with mfa
@@ -318,7 +365,7 @@ export const authLoginServiceFactory = ({
   const verifyMfaToken = async ({ userId, mfaToken, mfaJwtToken, ip, userAgent, orgId }: TVerifyMfaTokenDTO) => {
     const appCfg = getConfig();
     const user = await userDAL.findById(userId);
-    enforceUserLockStatus(user.isLocked, user.temporaryLockDateEnd);
+    enforceUserLockStatus(Boolean(user.isLocked), user.temporaryLockDateEnd);
 
     try {
       await tokenService.validateTokenForUser({
@@ -327,7 +374,7 @@ export const authLoginServiceFactory = ({
         code: mfaToken
       });
     } catch (err) {
-      const updatedUser = await processFailedMfaAttempt(userId, userDAL);
+      const updatedUser = await processFailedMfaAttempt(userId);
       if (updatedUser.isLocked) {
         if (updatedUser.email) {
           const unlockToken = await tokenService.createTokenForUser({
@@ -341,7 +388,7 @@ export const authLoginServiceFactory = ({
             recipients: [updatedUser.email],
             substitutions: {
               token: unlockToken,
-              callback_url: `${appCfg.SITE_URL}/api/v1/user/${updatedUser.id}/unlock-verify`
+              callback_url: `${appCfg.SITE_URL}/api/v1/user/${updatedUser.id}/unlock`
             }
           });
         }

backend/src/services/auth/auth-password-service.ts

@@ -138,6 +138,11 @@ export const authPaswordServiceFactory = ({
       code
     });
 
+    await userDAL.updateById(user.id, {
+      isLocked: false,
+      temporaryLockDateEnd: null
+    });
+
     const token = jwt.sign(
       {
         authTokenType: AuthTokenType.SIGNUP_TOKEN,

backend/src/services/user/user-dal.ts

@@ -143,6 +143,19 @@ export const userDALFactory = (db: TDbClient) => {
     }
   };
 
+  const incrementFailedMfaAttempt = async (userId: string, tx?: Knex) => {
+    try {
+      const [user] = await (tx || db)(TableName.Users)
+        .where("id", userId)
+        .increment("consecutiveFailedMfaAttempts", 1)
+        .returning("*");
+
+      return user;
+    } catch (error) {
+      throw new DatabaseError({ error, name: "Increment Failed MFA Attempt" });
+    }
+  };
+
   return {
     ...userOrm,
     findUserByUsername,
@@ -155,6 +168,7 @@ export const userDALFactory = (db: TDbClient) => {
     upsertUserEncryptionKey,
     createUserEncryption,
     findOneUserAction,
-    createUserAction
+    createUserAction,
+    incrementFailedMfaAttempt
   };
 };

backend/src/services/user/user-fns.ts

@@ -1,7 +1,5 @@
 import slugify from "@sindresorhus/slugify";
 
-import { TableName } from "@app/db/schemas";
-import { DatabaseError } from "@app/lib/errors";
 import { alphaNumericNanoId } from "@app/lib/nanoid";
 import { TUserDALFactory } from "@app/services/user/user-dal";
 
@@ -21,54 +19,3 @@ export const normalizeUsername = async (username: string, userDAL: Pick<TUserDAL
     }
   }
 };
-
-export const processFailedMfaAttempt = async (userId: string, userDAL: Pick<TUserDALFactory, "transaction">) => {
-  try {
-    const updatedUser = await userDAL.transaction(async (tx) => {
-      const PROGRESSIVE_DELAY_INTERVAL = 3;
-      const [user] = await tx(TableName.Users)
-        .where("id", userId)
-        .increment("consecutiveFailedMfaAttempts", 1)
-        .returning("*");
-
-      if (!user) {
-        throw new Error("User not found");
-      }
-
-      const progressiveDelaysInMins = [5, 30, 60];
-
-      // lock user when failed attempt exceeds threshold
-      if (user.consecutiveFailedMfaAttempts >= PROGRESSIVE_DELAY_INTERVAL * (progressiveDelaysInMins.length + 1)) {
-        return (
-          await tx(TableName.Users)
-            .where("id", userId)
-            .update({
-              isLocked: true,
-              temporaryLockDateEnd: null
-            })
-            .returning("*")
-        )[0];
-      }
-
-      // delay user only when failed MFA attempts is a multiple of configured delay interval
-      if (user.consecutiveFailedMfaAttempts % PROGRESSIVE_DELAY_INTERVAL === 0) {
-        const delayIndex = user.consecutiveFailedMfaAttempts / PROGRESSIVE_DELAY_INTERVAL - 1;
-
-        return (
-          await tx(TableName.Users)
-            .where("id", userId)
-            .update({
-              temporaryLockDateEnd: new Date(new Date().getTime() + progressiveDelaysInMins[delayIndex] * 60 * 1000)
-            })
-            .returning("*")
-        )[0];
-      }
-
-      return user;
-    });
-
-    return updatedUser;
-  } catch (error) {
-    throw new DatabaseError({ error, name: "Process failed MFA Attempt" });
-  }
-};