Update: Update Backend Auth Module for Signup Flow (Model, Controller, Validator, Types) #39
Conversation
youneedgreg
commented
Dec 23, 2025
youneedgreg
commented
- Added signup schema fields [firstName], [birthPlace], [location], optional [portfolioUrl], [bio], and role enum enforcement with timestamps and unique email in src/api/v1/user/user.model.ts; aligned typings with role union and timestamps plus Request user typing update in src/api/v1/user/user.type.ts and types/express.d.ts.
- Centralized role/password/bio config and messages (e.g., AllowedUserRoles, PasswordConfig) in src/api/v1/user/user.constant.ts and expanded Zod signup validation for required fields, password strength, URL and bio length checks in src/api/v1/user/user.validator.ts.
- Hardened auth logic: duplicate-email guard with 409 responses and 400s for validation errors in src/api/v1/user/user.controller.ts; bcrypt-based hashing with legacy argon2 fallback, sanitized user responses (hide password/api keys), and reusable sanitization in src/api/v1/user/user.utils.ts; createUser defaults for optional fields in src/api/v1/user/user.service.ts.
- Exposed the required POST /api/v1/auth/signup (and matching login alias) while keeping existing user routes in src/api/v1/user/user.routes.ts; adjusted signup tests to the new payload/endpoint and duplicate-email status in src/api/v1/user/test/user.test.ts.
- Added bcrypt runtime and types in [package.json:30-50]
- Closes Update Backend Auth Module for Signup Flow (Model, Controller, Validator, Types) #35
…, Validator, Types)
youneedgreg
changed the title
Summary of ChangesHello @youneedgreg, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a comprehensive overhaul of the backend authentication module, primarily focusing on the user signup and login processes. The changes aim to enhance data integrity, security, and API usability by expanding user profile fields, strengthening password management, implementing detailed input validation, and providing clearer error responses. The update also centralizes configuration for user-related settings and introduces new, more intuitive authentication endpoints. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request significantly enhances the user signup flow by adding new user fields, strengthening validation with Zod, and improving security by migrating to bcrypt for password hashing. The introduction of a sanitizeUser utility is a great step towards centralizing logic and preventing data leaks.
My review focuses on ensuring the new changes integrate correctly with existing code. I've identified a critical issue where a change to the findById utility breaks other parts of the application. I've also pointed out some opportunities for code cleanup and clarification on a potential behavior change in the API response. Overall, these are solid improvements, and addressing the feedback will make the implementation more robust.
| public static async findById(userId: string): Promise<IUser | null> { | ||
| const user = await User.findById(userId) | ||
| return user | ||
| return this.sanitizeUser(user) as IUser | null |
There was a problem hiding this comment.
Changing findById to return a sanitized user breaks existing functionality. For example, the getApiKey controller method calls findById and then accesses user.apikey. However, the new sanitizeUser function you've introduced explicitly deletes the apikey property. This will cause the getApiKey method to always fail by incorrectly throwing an API_KEY_NOT_FOUND error.
Additionally, the type assertion as IUser | null is incorrect because sanitizeUser returns a Partial<IUser>, hiding the issue from the TypeScript compiler.
A safer approach is to keep findById as a utility for fetching the complete user document and call sanitizeUser explicitly where needed (e.g., in the profile controller after fetching the user).
| return this.sanitizeUser(user) as IUser | null | |
| return user |
| if (!userObj) { | ||
| throw new Error(UserConstant.USER_NOT_FOUND) | ||
| } |
There was a problem hiding this comment.
This if (!userObj) check is unreachable. The sanitizeUser function returns null only when its input user is null. However, there's a check on line 47 that throws an error if user is null. Therefore, user will always be a valid user object when sanitizeUser is called on line 65, and userObj will never be null. You can safely remove this redundant check to improve code clarity.
| 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> | ||
| } |
There was a problem hiding this comment.
The new sanitizeUser function is a great refactor to centralize sanitization logic. However, I noticed a change in behavior compared to the previous implementation. The old sanitization logic (e.g., in findUserByEmail) explicitly removed createdAt and updatedAt fields from the user object. The new sanitizeUser function does not.
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.
abhishek-nexgen-dev
added
good first issue
…pdate Update: Update Backend Auth Module for Signup Flow (Model, Controller, Validator, Types)
2 participants
