-
Notifications
You must be signed in to change notification settings - Fork 45
Update: Update Backend Auth Module for Signup Flow (Model, Controller, Validator, Types) #39
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,10 +1,20 @@ | ||
| import { AllowedUserRoles } from './user.constant' | ||
|
|
||
| export type UserRole = (typeof AllowedUserRoles)[number] | ||
|
|
||
| export interface IUser { | ||
| _id?: string | undefined | ||
| name?: string | null | ||
| _id?: string | ||
| firstName: string | ||
| email: string | ||
| password?: string | ||
| role?: string | ||
| role?: UserRole | ||
| birthPlace: string | ||
| location: string | ||
| portfolioUrl?: string | null | ||
| bio?: string | null | ||
| apikey?: string | null | ||
| model?: string | null | ||
| modelApiKey?: string | null | ||
| createdAt?: Date | ||
| updatedAt?: Date | ||
| } |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -2,8 +2,9 @@ import jwt, { SignOptions } from 'jsonwebtoken' | |||||
| import { env } from '../../../constant/env.constant' | ||||||
| import { IUser } from './user.type' | ||||||
| import User from './user.model' | ||||||
| import bcrypt from 'bcrypt' | ||||||
| import * as argon2 from 'argon2' | ||||||
| import UserConstant from './user.constant' | ||||||
| import UserConstant, { PasswordConfig } from './user.constant' | ||||||
|
|
||||||
| export interface JwtPayload { | ||||||
| userId: string | ||||||
|
|
@@ -21,7 +22,7 @@ class UserUtils { | |||||
| } as SignOptions) | ||||||
| } | ||||||
|
|
||||||
| public static verifyToken(token: string): IUser | null { | ||||||
| public static verifyToken(token: string): Partial<IUser> | null { | ||||||
| try { | ||||||
| const decoded = jwt.verify(token, this.JWT_SECRET) | ||||||
|
|
||||||
|
|
@@ -61,11 +62,11 @@ class UserUtils { | |||||
| if (!PassMatch) { | ||||||
| throw new Error(UserConstant.INVALID_PASSWORD) | ||||||
| } | ||||||
| const userObj: IUser = user.toObject() | ||||||
| delete (userObj as { password?: string }).password | ||||||
| delete (userObj as { createdAt?: Date }).createdAt | ||||||
| delete (userObj as { updatedAt?: Date }).updatedAt | ||||||
| delete (userObj as { __v?: number }).__v | ||||||
| const userObj = this.sanitizeUser(user) | ||||||
|
|
||||||
| if (!userObj) { | ||||||
| throw new Error(UserConstant.USER_NOT_FOUND) | ||||||
| } | ||||||
|
|
||||||
| const token = this.generateToken({ | ||||||
| userId: String(user._id), | ||||||
|
|
@@ -84,22 +85,16 @@ class UserUtils { | |||||
|
|
||||||
| public static async findById(userId: string): Promise<IUser | null> { | ||||||
| const user = await User.findById(userId) | ||||||
| return user | ||||||
| return this.sanitizeUser(user) as IUser | null | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Changing Additionally, the type assertion A safer approach is to keep
Suggested change
|
||||||
| } | ||||||
|
|
||||||
| public static async passwordHash(password: string): Promise<string> { | ||||||
| return await argon2.hash(password) | ||||||
| return bcrypt.hash(password, PasswordConfig.saltRounds) | ||||||
| } | ||||||
|
|
||||||
| public static async findUserByEmail(email: string): Promise<Partial<IUser> | null> { | ||||||
| const user = await User.findOne({ email }) | ||||||
| if (!user) return null | ||||||
| const userObj = user.toObject() as IUser | ||||||
| delete (userObj as { password?: string }).password | ||||||
| delete (userObj as { createdAt?: Date }).createdAt | ||||||
| delete (userObj as { updatedAt?: Date }).updatedAt | ||||||
| delete (userObj as { __v?: number }).__v | ||||||
| return userObj | ||||||
| return this.sanitizeUser(user) | ||||||
| } | ||||||
|
|
||||||
| private static async passwordMatching({ | ||||||
|
|
@@ -109,7 +104,24 @@ class UserUtils { | |||||
| dbPass: string | ||||||
| userPass: string | ||||||
| }): Promise<boolean> { | ||||||
| return await argon2.verify(dbPass, userPass) | ||||||
| const isBcryptMatch = await bcrypt.compare(userPass, dbPass) | ||||||
| if (isBcryptMatch) return true | ||||||
|
|
||||||
| try { | ||||||
| return await argon2.verify(dbPass, userPass) | ||||||
| } catch { | ||||||
| return false | ||||||
| } | ||||||
| } | ||||||
| public static sanitizeUser(user: IUser | null): Partial<IUser> | null { | ||||||
| if (!user) return null | ||||||
| const userObj = typeof (user as any).toObject === 'function' ? (user as any).toObject() : { ...user } | ||||||
|
|
||||||
| delete (userObj as { password?: string }).password | ||||||
| delete (userObj as { __v?: number }).__v | ||||||
| delete (userObj as { apikey?: string | null }).apikey | ||||||
| delete (userObj as { modelApiKey?: string | null }).modelApiKey | ||||||
| return userObj as Partial<IUser> | ||||||
| } | ||||||
|
Comment on lines
+116
to
125
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The new Was it intentional to start exposing these timestamp fields in API responses? If not, you might want to add them to the list of deleted properties for consistency with the previous behavior. |
||||||
| public static maskApiKey(apiKey: string): string { | ||||||
| if (!apiKey || apiKey.length < 8) return '*' | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,33 +1,42 @@ | ||
| import { z } from 'zod' | ||
| import UserConstant from './user.constant' | ||
| import UserConstant, { AllowedUserRoles, BioConfig, PasswordConfig } from './user.constant' | ||
|
|
||
| export const userRegisterSchema = z.object({ | ||
| name: z.string().trim().min(1, UserConstant.NAME_REQUIRED), | ||
| const passwordSchema = z | ||
| .string() | ||
| .min(PasswordConfig.minLength, UserConstant.PASSWORD_MIN_LENGTH) | ||
| .max(PasswordConfig.maxLength, UserConstant.PASSWORD_MAX_LENGTH) | ||
| .regex(/[A-Z]/, UserConstant.PASSWORD_UPPERCASE_REQUIRED) | ||
| .regex(/[a-z]/, UserConstant.PASSWORD_LOWERCASE_REQUIRED) | ||
| .regex(/[0-9]/, UserConstant.PASSWORD_NUMBER_REQUIRED) | ||
| .regex(/[@$!%*?&]/, UserConstant.PASSWORD_SPECIAL_CHAR_REQUIRED) | ||
|
|
||
| role: z | ||
| .enum(['user', 'admin', 'creator'] as const) | ||
| .refine( | ||
| role => ['user', 'admin', 'creator'].includes(role), | ||
| { message: UserConstant.INVALID_ROLE } | ||
| ), | ||
|
|
||
| email: z.string().email(UserConstant.INVALID_CREDENTIALS).toLowerCase(), | ||
| const roleSchema = z.enum(AllowedUserRoles, { | ||
| errorMap: () => ({ message: UserConstant.INVALID_ROLE }), | ||
| }) | ||
|
|
||
| password: z | ||
| .string() | ||
| .min(8, UserConstant.PASSWORD_MIN_LENGTH) | ||
| .max(20, UserConstant.PASSWORD_MAX_LENGTH) | ||
| .regex(/[A-Z]/, UserConstant.PASSWORD_UPPERCASE_REQUIRED) | ||
| .regex(/[a-z]/, UserConstant.PASSWORD_LOWERCASE_REQUIRED) | ||
| .regex(/[0-9]/, UserConstant.PASSWORD_NUMBER_REQUIRED) | ||
| .regex(/[@$!%*?&]/, UserConstant.PASSWORD_SPECIAL_CHAR_REQUIRED), | ||
| const portfolioUrlSchema = z | ||
| .string() | ||
| .trim() | ||
| .url(UserConstant.INVALID_PORTFOLIO_URL) | ||
| .max(2048, UserConstant.INVALID_PORTFOLIO_URL) | ||
| .optional() | ||
|
|
||
| portfolioUrl: z.string().url(UserConstant.INVALID_URL).optional(), | ||
| export const userRegisterSchema = z | ||
| .object({ | ||
| firstName: z.string().trim().min(1, UserConstant.FIRST_NAME_REQUIRED), | ||
| role: roleSchema.default('user'), | ||
| email: z.string().email(UserConstant.INVALID_CREDENTIALS).toLowerCase(), | ||
| birthPlace: z.string().trim().min(1, UserConstant.BIRTH_PLACE_REQUIRED), | ||
| location: z.string().trim().min(1, UserConstant.LOCATION_REQUIRED), | ||
| password: passwordSchema, | ||
| portfolioUrl: portfolioUrlSchema, | ||
| bio: z.string().trim().max(BioConfig.maxLength, UserConstant.BIO_TOO_LONG).optional(), | ||
| }) | ||
| .strict() | ||
|
|
||
| bio: z.string().max(300, UserConstant.BIO_MAX_LENGTH).optional(), | ||
| }) | ||
|
|
||
| export const userLoginSchema = z.object({ | ||
| email: z.string().email(UserConstant.INVALID_CREDENTIALS).toLowerCase(), | ||
| password: z.string(), | ||
| }) | ||
| export const userLoginSchema = z | ||
| .object({ | ||
| email: z.string().email(UserConstant.INVALID_CREDENTIALS).toLowerCase(), | ||
| password: z.string(), | ||
| }) | ||
| .strict() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This
if (!userObj)check is unreachable. ThesanitizeUserfunction returnsnullonly when its inputuserisnull. However, there's a check on line 47 that throws an error ifuserisnull. Therefore,userwill always be a valid user object whensanitizeUseris called on line 65, anduserObjwill never benull. You can safely remove this redundant check to improve code clarity.