set new password flag and logic added plus fix bug for user creation …#121
set new password flag and logic added plus fix bug for user creation …#121yifeifang11 merged 1 commit intomainfrom
Conversation
|
added flags in user model so that if a user is created from the admin side, it is marked so that when the created user logs in, they are directed to reset their password. there was also a bug where the admin will be signed in to the newly created user, so a second instance of firebase app has been added to prevent this. instance is deleted after account is successfully created. |
There was a problem hiding this comment.
Pull request overview
Adds an “admin-created user must change temporary password” flow across the backend and frontend, and fixes the admin user-creation bug where creating a Firebase Auth user could disrupt the admin’s own Firebase session.
Changes:
- Backend: adds
createdByAdmin/mustResetPasswordfields, sets them when staff create users, returnsrequiresPasswordReseton login, and adds an endpoint to clear the reset flag. - Frontend: updates login to handle
requiresPasswordReset, adds a “Change Password Now” page/route, and updates admin user creation to use a secondary Firebase app instance.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/src/services/authService.ts | Returns requiresPasswordReset from login to drive client-side redirect logic. |
| frontend/src/routes/RoutesAndLayouts.tsx | Adds /change-password-now route and treats it like an auth-layout page. |
| frontend/src/pages/UserAuth/changePasswordNow.tsx | New UI to change password and notify backend that the reset requirement is complete. |
| frontend/src/pages/UserAuth/Login.tsx | Redirects to change-password flow when backend indicates a reset is required. |
| frontend/src/pages/Admin/UserManagementPage/Users.tsx | Creates Firebase users via a secondary app to avoid signing out the admin session. |
| backend/routes/userRoutes.ts | Adds POST /me/password-reset-complete route for clearing reset flag. |
| backend/models/userModel.ts | Adds createdByAdmin and mustResetPassword fields to the user schema. |
| backend/controllers/userController.ts | Sets reset flags on admin-created users and adds handler to clear mustResetPassword. |
| backend/controllers/loginController.ts | Includes requiresPasswordReset in login response payload. |
| if (response.data?.user) { | ||
| const updatedUser = response.data.user; | ||
| if (updatedUser.cart && typeof updatedUser.cart === "string") { | ||
| updatedUser.cart = JSON.parse(updatedUser.cart); |
There was a problem hiding this comment.
JSON.parse(updatedUser.cart) can throw if cart is an invalid JSON string (including an empty string), which would break the success flow after a password change. Wrap the parse in a try/catch (or validate/normalize server-side) and fall back to an empty array when parsing fails.
| updatedUser.cart = JSON.parse(updatedUser.cart); | |
| try { | |
| updatedUser.cart = JSON.parse(updatedUser.cart); | |
| } catch { | |
| updatedUser.cart = []; | |
| } |
| router.get("/impersonation/status", getImpersonationStatus); | ||
| router.post("/:targetUserId/impersonate", startImpersonation); | ||
| router.post("/impersonation/stop", stopImpersonation); | ||
| router.post("/me/password-reset-complete", completeInitialPasswordReset); |
There was a problem hiding this comment.
A new authenticated route (POST /users/me/password-reset-complete) and controller logic were added, but there are no Jest tests covering the new behavior (success path + 401/404 cases). Add backend tests alongside the existing user route tests to prevent regressions, especially around flipping mustResetPassword and the login flow that depends on it.
| const user = await User.findOneAndUpdate( | ||
| { firebaseId: req.user.uid }, | ||
| { $set: { mustResetPassword: false } }, | ||
| { new: true } | ||
| ); |
There was a problem hiding this comment.
completeInitialPasswordReset lets any authenticated user clear mustResetPassword without proving they actually changed their Firebase password (a client can call this endpoint immediately after logging in with the temporary password). If the intent is to enforce password rotation, consider validating against Firebase Admin user metadata (e.g., compare passwordUpdatedAt to an issued-at timestamp stored in your DB) before clearing the flag.
|
|
||
| res.status(200).json({ | ||
| user, | ||
| requiresPasswordReset: user.createdByAdmin && user.mustResetPassword, |
There was a problem hiding this comment.
requiresPasswordReset is derived from a boolean && expression that can evaluate to undefined (e.g., for older documents where these fields are missing), which will cause the property to be omitted from the JSON response. Consider explicitly returning a boolean (e.g., coerce both fields with Boolean(...)) so the API contract is stable for clients.
| requiresPasswordReset: user.createdByAdmin && user.mustResetPassword, | |
| requiresPasswordReset: | |
| Boolean(user.createdByAdmin) && Boolean(user.mustResetPassword), |
| res.status(500).json({ | ||
| message: "Error completing initial password reset", | ||
| error, |
There was a problem hiding this comment.
This endpoint clears mustResetPassword based solely on the authenticated user and returns the raw error object on 500. Returning the raw error can leak stack traces/internal details; log the error server-side and return a generic message (and optionally an error code) instead.
| res.status(500).json({ | |
| message: "Error completing initial password reset", | |
| error, | |
| console.error("Error completing initial password reset:", error); | |
| res.status(500).json({ | |
| message: "Error completing initial password reset", | |
| code: "INITIAL_PASSWORD_RESET_FAILED", |
…from admin side