implemented backend logic for user and order cotrollers #5
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
WalkthroughAdds a new orders API with controllers and routes mounted at /api/orders; implements order creation, retrieval, status updates, and access control. Introduces refresh-token rotation and related routes; enforces registration email domain. Frontend adds Profile page/route, updates API base and refresh logic, styling/theme configs, and downgrades several dependencies. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client (Buyer)
participant API as Backend / Orders
participant Listings as Listings DB/Service
participant DB as Orders DB
Client->>API: POST /api/orders (items, address)
API->>Listings: Fetch listings by item IDs
Listings-->>API: Listing details (seller IDs, prices)
API->>API: Validate single seller & buyer != seller
API->>DB: Create order (buyer, seller, items, totals)
DB-->>API: Order created
API-->>Client: 201 Created (order)
sequenceDiagram
participant Client as Client (Any)
participant API as Backend / Auth
participant DB as Users DB
participant Cookies as Client Cookies
Client->>API: Request (401 response)
API-->>Client: 401 Unauthorized
Client->>API: POST /users/refresh-token (cookie or body)
API->>DB: Verify refresh token, rotation checks
DB-->>API: New tokens (access, refresh)
API-->>Cookies: Set new refresh token cookie
API-->>Client: 200 OK (accessToken)
Client->>API: Retry original request with new accessToken
API-->>Client: Original request result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/src/controllers/users.controller.js (1)
65-75: Normalize email before the domain check to avoid false rejections.
Uppercase or whitespace in valid emails will currently fail theendsWithcheck.🛠️ Suggested fix
- // Enforce IIT Bhilai domain - if (!email.endsWith("@iitbhilai.ac.in")) { + const normalizedEmail = email.trim().toLowerCase(); + if (!normalizedEmail.endsWith("@iitbhilai.ac.in")) { throw new ApiError(400, "Only `@iitbhilai.ac.in` emails are allowed to register."); } ... - email: email.toLowerCase(), + email: normalizedEmail,
🤖 Fix all issues with AI agents
In `@backend/src/controllers/order.controller.js`:
- Line 10: Remove acceptance of client-supplied paymentStatus by deleting
paymentStatus from the destructuring of req.body (remove "paymentStatus" from
const { items, address, paymentStatus } = req.body) and instead set
paymentStatus server-side when creating an order (e.g., default to 'pending' or
derive from your payment provider) inside the create order handler; also audit
the other occurrence around the update flow (the second mention at lines ~52)
and ensure any updates to paymentStatus are validated against a strict allowlist
or only changed by server/payment-webhook logic, not directly from req.body.
- Around line 123-136: The handler uses req.body.status (and assigns
order.deliveryStatus) without validation, allowing invalid or undefined values;
add a whitelist check before persisting: define allowed statuses (e.g.,
"delivered", "in-progress"), verify req.body.status is one of them, and if not
throw an ApiError(400, "Invalid delivery status") (or similar); only set
order.deliveryStatus = status after this validation and proceed to save the
Order returned by Order.findById.
- Around line 19-43: Derive the seller from each Listing fetched instead of
trusting client payload: after fetching the first listing via Listing.findById
(used in the existing loop where you build orderItems and totalAmount), set
sellerId = listing.owner._id.toString() and for every subsequent listing assert
listing.owner._id.toString() === sellerId (throw an ApiError 400 if mismatched
to enforce single-seller orders). Also validate item.quantity is a positive
integer before using it (if not, throw ApiError 400); use quantity = Math.max(1,
parsedQuantity) only after validation and multiply price by this validated
quantity when updating totalAmount and pushing to orderItems. Ensure you never
read sellerId from the client payload.
In `@frontend/package.json`:
- Around line 13-38: Update the vulnerable axios dependency in
frontend/package.json by raising the "axios" entry from "^1.6.2" to a secure
version (recommend "^1.12.0" or at least "^1.8.2"); after changing the "axios"
version, run your package manager to update the lockfile (npm install / yarn
install / pnpm install) and verify the app builds and tests pass (note Vite 5
requires Node 18+ if applicable). Target the "axios" dependency line in
package.json to make this change and ensure package-lock.json / yarn.lock is
committed so the upgrade takes effect.
In `@frontend/src/components/ui/Input.jsx`:
- Around line 37-43: In the Input.jsx class list the error state only adds
'border-red-500 focus:ring-red-500/20' so the always-present
'focus:border-indigo-500' still wins on focus; update the error conditional (the
expression that uses the error variable in the class list) to also include
'focus:border-red-500' so when error is true the focus border becomes red (i.e.,
change the error && entry in the class array used by the Input component to
include focus:border-red-500).
In `@frontend/src/index.css`:
- Around line 1-3: Update Biome's linter config so Tailwind at-rules are
allowed: add an entry under linter.rules.suspicious.noUnknownAtRules to include
an ignore list for "tailwind", "apply", and "layer" (so `@tailwind`, `@apply`,
`@layer` are accepted), or if you cannot change biome.json, add inline Biome
ignore comments around the Tailwind directives in frontend/src/index.css to
suppress the noUnknownAtRules warnings; target the rule name noUnknownAtRules
when making the change.
In `@frontend/src/pages/auth/Register.jsx`:
- Around line 48-63: Remove the console.log statements that print sensitive form
data (values and userData) in the registration flow (the handler that calls
register and navigate) — specifically remove or replace the logs "Handle
Register Started", "Sending user data:", and "Register API Response:" and any
logging of values/userData; if you need debugging output keep it behind a
dev-only flag (e.g., process.env.NODE_ENV === 'development') and redact
sensitive fields like password/confirmPassword before logging, while leaving
non-sensitive logs such as success/failure messages and the call sites using
register(...) and navigate("/login") intact.
In `@frontend/src/utils/validation.js`:
- Around line 7-13: The email validator function (email) should normalize the
input before running the regex: trim leading/trailing whitespace and convert to
lowercase, e.g., produce a normalized variable from the incoming email and use
that for the regex.test and any subsequent checks; update the regex/test to
operate on the normalized value so uppercase letters or stray spaces no longer
cause valid `@iitbhilai.ac.in` addresses to be rejected.
🧹 Nitpick comments (2)
frontend/src/services/api.js (1)
6-6: Ensure non-local environments override the localhost default.
If the env var isn’t set in production, requests will fail (or be blocked as mixed content). Consider documenting this or enforcing it in deployment.frontend/src/pages/Profile.jsx (1)
160-177: Associate the hostel label with its select for a11y.Add
htmlForon the label and a matchingidon the select to improve screen reader navigation.♿ Suggested tweak
- <label className="text-sm font-medium text-gray-700">Hostel</label> + <label htmlFor="profile-hostel" className="text-sm font-medium text-gray-700">Hostel</label> <div className="relative"> @@ - <select + <select + id="profile-hostel" name="hostel"
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@frontend/src/services/api.js`:
- Around line 40-41: The comment and code disagree: the code only calls
Cookies.remove('accessToken') but the comment claims "clear everything and
redirect to login"; either update the comment to accurately state that only the
access token cookie is removed, or implement the full behavior by clearing
session state (e.g., reset user store / auth context) and triggering a redirect
to the login page; locate the token removal in frontend/src/services/api.js
(Cookies.remove('accessToken')) and either change the comment text to "remove
access token cookie" or add calls to your user/state management (e.g.,
logout/resetUser) and a navigation action (e.g., window.location or router push
to login) so the implementation matches the intent.
- Around line 34-38: The refresh-token call uses the same axios instance `api`,
causing its response interceptor to catch 401s from the refresh endpoint and
potentially loop; change the refresh request to use a raw axios call (e.g.,
`axios.post`) or a dedicated instance without the 401 interceptor instead of
`api.post('/users/refresh-token')`, and ensure `originalRequest._retry = true`
remains set on the original request (and not relied on for the refresh call) so
a failed refresh returns the error instead of triggering another refresh
attempt; update `Cookies.set('accessToken', accessToken)` and
`originalRequest.headers['Authorization'] = \`Bearer ${accessToken}\`` after a
successful raw refresh and then retry via `api(originalRequest)`.
- Line 35: Add a new POST route '/users/refresh-token' in
backend/src/routes/users.routes.js and implement a controller (e.g.,
refreshTokenController or handleRefreshToken) that reads the refresh token from
cookies or the request body, validates it (use existing token utilities like
verifyRefreshToken or your auth service), generates a new access token (use the
existing generateAccessToken / signJwt helper), and returns the token wrapped in
an ApiResponse JSON object shaped as { data: { accessToken: '...' } } so the
frontend can destructure response.data.data.accessToken; wire the controller
into the router export and reuse existing auth helpers and ApiResponse class to
match project patterns.
🧹 Nitpick comments (2)
frontend/src/services/api.js (2)
31-38: Race condition with concurrent 401 responses.If multiple requests fail with 401 simultaneously, each will independently attempt to refresh the token. This can cause multiple refresh calls and potential token invalidation issues.
A common pattern is to queue pending requests while a refresh is in progress:
♻️ Proposed fix using a refresh promise queue
+let isRefreshing = false; +let failedQueue = []; + +const processQueue = (error, token = null) => { + failedQueue.forEach(prom => { + if (error) { + prom.reject(error); + } else { + prom.resolve(token); + } + }); + failedQueue = []; +}; + api.interceptors.response.use( (response) => response, async (error) => { const originalRequest = error.config; - // 401 Unauth + not a retry -> Attempt token refresh - if (error.response?.status === 401 && !originalRequest._retry) { - originalRequest._retry = true; - try { - const response = await api.post('/users/refresh-token'); // Corrected path - const { accessToken } = response.data.data; - Cookies.set('accessToken', accessToken); - originalRequest.headers['Authorization'] = `Bearer ${accessToken}`; - return api(originalRequest); - } catch (refreshError) { - // If refresh fails, clear everything and redirect to login - Cookies.remove('accessToken'); - return Promise.reject(refreshError); + + if (error.response?.status === 401 && !originalRequest._retry) { + if (isRefreshing) { + return new Promise((resolve, reject) => { + failedQueue.push({ resolve, reject }); + }).then(token => { + originalRequest.headers['Authorization'] = `Bearer ${token}`; + return api(originalRequest); + }); } + + originalRequest._retry = true; + isRefreshing = true; + + return new Promise((resolve, reject) => { + axios.post(`${API_BASE_URL}/users/refresh-token`, {}, { withCredentials: true }) + .then(response => { + const { accessToken } = response.data.data; + Cookies.set('accessToken', accessToken); + originalRequest.headers['Authorization'] = `Bearer ${accessToken}`; + processQueue(null, accessToken); + resolve(api(originalRequest)); + }) + .catch(refreshError => { + processQueue(refreshError, null); + Cookies.remove('accessToken'); + reject(refreshError); + }) + .finally(() => { + isRefreshing = false; + }); + }); } return Promise.reject(error); } );
36-36: Consider adding secure cookie options.The
accessTokencookie is set without specifyingsecure,sameSite, orexpiresoptions. For production security, consider adding these options to prevent CSRF attacks and ensure the cookie is only sent over HTTPS.🔒 Proposed fix to add cookie security options
- Cookies.set('accessToken', accessToken); + Cookies.set('accessToken', accessToken, { + secure: import.meta.env.PROD, + sameSite: 'Strict', + });
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@backend/src/controllers/users.controller.js`:
- Around line 443-477: In refreshAccessToken, after generating the
newRefreshToken (function generateAccessAndRefreshTokens) remove the used
incomingRefreshToken from user.refreshTokens (e.g., filter out entries where
t.token === incomingRefreshToken), then push or unshift the new refresh token
entry into user.refreshTokens and persist the change (await user.save()) so the
old token is invalidated on rotation; ensure this update happens before sending
cookies and response and preserve any existing token-limit logic (e.g., trimming
to 3 tokens) if present.
🧹 Nitpick comments (3)
backend/src/controllers/users.controller.js (2)
56-68: Fail fast: validate email domain before database lookup.The domain check should precede the existing user query to avoid unnecessary database calls when the email is invalid.
♻️ Suggested reorder
if (!name || !username || !email || !password) { throw new ApiError(400, "Name, username, email and password are required"); } + // Enforce IIT Bhilai domain (fail fast) + if (!email.endsWith("@iitbhilai.ac.in")) { + throw new ApiError(400, "Only `@iitbhilai.ac.in` emails are allowed to register."); + } + // Check if user already exists const existedUser = await User.findOne({ $or: [{ username }, { email }], }); if (existedUser) { throw new ApiError(409, "User with email or username already exists"); } - // Enforce IIT Bhilai domain - if (!email.endsWith("@iitbhilai.ac.in")) { - throw new ApiError(400, "Only `@iitbhilai.ac.in` emails are allowed to register."); - }
441-443: Consider consistent export style.
refreshAccessTokenuses inlineexport constwhile other functions use the consolidatedexport {}block at the bottom. This works but is inconsistent. Consider moving the export to the bottom block for uniformity.frontend/src/pages/auth/Login.jsx (1)
158-166: Thevariant="default"is redundant.Since
variantdefaults to'default'in the Button component, this prop can be omitted. Themt-6margin addition is fine for visual spacing.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@backend/src/controllers/users.controller.js`:
- Around line 476-478: The catch block currently always throws throw new
ApiError(401, ...) which masks non-auth errors; update the catch to re-throw
existing ApiError instances directly (if (error instanceof ApiError) throw
error) and only wrap unexpected errors into a new ApiError (e.g., new
ApiError(500, error?.message || "Internal server error")) instead of converting
every error into a 401; keep references to ApiError and the existing throw new
ApiError(401, ...) site in users.controller.js when making this change.
- Around line 462-466: The token rotation currently only mutates the in-memory
user.refreshTokens (filtering by incomingRefreshToken) but never persists it, so
the stale token remains in DB; persist the removal before calling
generateAccessAndRefreshTokens(user._id) by saving or updating the user record
(e.g., call user.save() or User.updateOne/_findByIdAndUpdate to remove the
token) so the subsequent generateAccessAndRefreshTokens fetches the updated user
without the old refresh token.
- Around line 65-68: Move the IIT Bhilai domain validation to run before any
user-existence lookup in the registration flow (e.g., inside the registerUser
controller) to prevent user enumeration attacks; also make the domain check
case-insensitive by normalizing the email (trim and toLowerCase) before
verifying email.endsWith("@iitbhilai.ac.in"), and throw the same ApiError(400,
...) when the domain is invalid, so the controller (registerUser) returns the
domain validation error prior to calling any userExists or findUserByEmail
logic.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@backend/src/controllers/users.controller.js`:
- Around line 195-205: In the forgotPassword handler, stop including the plain
resetToken in the HTTP response; after generating resetToken and storing its
hash on the user (user.resetPasswordToken / user.resetPasswordExpires in
forgotPassword), send the raw resetToken to the user's email (e.g. call your
mailer like sendResetEmail(resetToken, user.email)) or only expose it in dev
with a guarded flag, then return a generic ApiResponse (e.g. "If an account with
that email exists, a reset link has been sent") without the token in the JSON
payload.
- Around line 82-108: In loginUser (and similarly in refreshAccessToken) stop
returning the httpOnly refreshToken in the JSON payload: keep setting
refreshToken as an httpOnly cookie but remove refreshToken from the ApiResponse
body (only return user and, if needed, accessToken), so update the ApiResponse
construction in loginUser (and the response in refreshAccessToken) to omit the
refreshToken field while still sending the cookie via res.cookie.
- Around line 248-251: The updateUserProfile handler currently passes req.body
directly into User.findByIdAndUpdate, allowing clients to change sensitive
fields; instead build a whitelist of allowed profile fields (e.g., name, email,
avatar, bio, etc.), copy only those keys from req.body into an updatePayload,
validate/normalize expected types if needed, then call
User.findByIdAndUpdate(req.user._id, { $set: updatePayload }, { new: true
}).select("-password") so roles, isVerified, refreshTokens and other sensitive
fields cannot be modified via updateUserProfile or ApiResponse.
In `@frontend/src/services/api.js`:
- Around line 36-44: Remove the client-side refresh-token precheck that uses
Cookies.get('refreshToken')/hasRefreshToken in the response interceptor so
httpOnly cookies aren't assumed missing; delete the entire if (!hasRefreshToken)
branch and its early localStorage.removeItem('user') +
redirect/Promise.reject(error) logic inside the interceptor, leaving the code to
call the refresh endpoint (POST /users/refresh-token via the axios instance
configured with withCredentials) and rely on the existing catch block to handle
cleanup and redirect when the refresh actually fails.
🧹 Nitpick comments (1)
backend/src/controllers/users.controller.js (1)
43-76: Normalize email/username once for all checks.Use normalized values for domain validation, existence query, and persistence to avoid case-mismatch and duplicate-key errors.
♻️ Suggested normalization refactor
- const { name, username, email, password, phone, whatsapp, hostelLocation } = req.body; + const { name, username, email, password, phone, whatsapp, hostelLocation } = req.body; + const normalizedEmail = email?.trim().toLowerCase(); + const normalizedUsername = username?.trim().toLowerCase(); - if (!email.toLowerCase().endsWith("@iitbhilai.ac.in")) { + if (!normalizedEmail?.endsWith("@iitbhilai.ac.in")) { throw new ApiError(400, "Registration is restricted to official `@iitbhilai.ac.in` emails."); } - const existedUser = await User.findOne({ $or: [{ username }, { email }] }); + const existedUser = await User.findOne({ + $or: [{ username: normalizedUsername }, { email: normalizedEmail }], + }); - username: username.toLowerCase(), - email: email.toLowerCase(), + username: normalizedUsername, + email: normalizedEmail,
Comment on lines
82
to
+108
| const loginUser = asyncHandler(async (req, res) => { | ||
| const { email, username, password } = req.body; | ||
|
|
||
| if (!((username || email) && password)) { | ||
| throw new ApiError(400, "Username or email is required"); | ||
| throw new ApiError(400, "Username/Email and password are required"); | ||
| } | ||
|
|
||
| const user = await User.findOne({ | ||
| $or: [{ username }, { email }], | ||
| }).select("+password"); | ||
|
|
||
| if (!user) { | ||
| throw new ApiError(404, "User does not exist"); | ||
| if (!user || !(await user.comparePassword(password))) { | ||
| throw new ApiError(401, "Invalid user credentials. Please check your password."); | ||
| } | ||
|
|
||
| const isPasswordValid = await user.comparePassword(password); | ||
| const { accessToken, refreshToken } = await generateAccessAndRefreshTokens(user._id); | ||
|
|
||
| if (!isPasswordValid) { | ||
| throw new ApiError(401, "Invalid user credentials"); | ||
| } | ||
| const loggedInUser = await User.findById(user._id).select("-password -refreshTokens -verificationToken -resetPasswordToken"); | ||
|
|
||
| const { accessToken, refreshToken } = await generateAccessAndRefreshTokens( | ||
| user._id | ||
| ); | ||
|
|
||
| const loggedInUser = await User.findById(user._id).select( | ||
| "-password -refreshTokens -verificationToken -resetPasswordToken" | ||
| ); | ||
|
|
||
| const options = { | ||
| httpOnly: true, | ||
| secure: true, | ||
| }; | ||
| const options = { httpOnly: true, secure: true }; | ||
|
|
||
| return res | ||
| .status(200) | ||
| .cookie("accessToken", accessToken, options) | ||
| .cookie("refreshToken", refreshToken, options) | ||
| .json( | ||
| new ApiResponse( | ||
| 200, | ||
| { | ||
| user: loggedInUser, | ||
| accessToken, | ||
| refreshToken, | ||
| }, | ||
| "User logged in successfully" | ||
| ) | ||
| ); | ||
| .json(new ApiResponse(200, { user: loggedInUser, accessToken, refreshToken }, "Login successful")); | ||
| }); |
There was a problem hiding this comment.
Avoid returning refresh tokens in JSON.
Since refreshToken is set as an httpOnly cookie, returning it in the response body exposes it to JS/XSS. Consider removing it from the payload; the same applies to Line 143 in refreshAccessToken.
🔒 Suggested response change
- .json(new ApiResponse(200, { user: loggedInUser, accessToken, refreshToken }, "Login successful"));
+ .json(new ApiResponse(200, { user: loggedInUser, accessToken }, "Login successful"));🤖 Prompt for AI Agents
In `@backend/src/controllers/users.controller.js` around lines 82 - 108, In
loginUser (and similarly in refreshAccessToken) stop returning the httpOnly
refreshToken in the JSON payload: keep setting refreshToken as an httpOnly
cookie but remove refreshToken from the ApiResponse body (only return user and,
if needed, accessToken), so update the ApiResponse construction in loginUser
(and the response in refreshAccessToken) to omit the refreshToken field while
still sending the cookie via res.cookie.
Comment on lines
195
to
205
| const forgotPassword = asyncHandler(async (req, res) => { | ||
| const { email } = req.body; | ||
|
|
||
| if (!email) { | ||
| throw new ApiError(400, "Email is required"); | ||
| } | ||
|
|
||
| const user = await User.findOne({ email }); | ||
|
|
||
| if (!user) { | ||
| throw new ApiError(404, "User with this email does not exist"); | ||
| } | ||
| const user = await User.findOne({ email: req.body.email }); | ||
| if (!user) throw new ApiError(404, "User not found"); | ||
|
|
||
| const resetToken = crypto.randomBytes(32).toString("hex"); | ||
| user.resetPasswordToken = crypto | ||
| .createHash("sha256") | ||
| .update(resetToken) | ||
| .digest("hex"); | ||
| user.resetPasswordExpires = new Date(Date.now() + 10 * 60 * 1000); | ||
|
|
||
| user.resetPasswordToken = crypto.createHash("sha256").update(resetToken).digest("hex"); | ||
| user.resetPasswordExpires = Date.now() + 600000; // 10 mins | ||
| await user.save({ validateBeforeSave: false }); | ||
|
|
||
| // TODO: Send email with reset token | ||
| // For now, we'll just return the token (remove this in production) | ||
| return res | ||
| .status(200) | ||
| .json( | ||
| new ApiResponse(200, { resetToken }, "Password reset token sent to email") | ||
| ); | ||
| return res.status(200).json(new ApiResponse(200, { resetToken }, "Reset link generated")); | ||
| }); |
There was a problem hiding this comment.
Don’t return reset tokens in the response.
Returning resetToken lets anyone who knows an email reset the account without mailbox access. Send it via email (or gate it in dev only).
🛡️ Safer response payload
- return res.status(200).json(new ApiResponse(200, { resetToken }, "Reset link generated"));
+ return res.status(200).json(new ApiResponse(200, {}, "Reset link generated"));🤖 Prompt for AI Agents
In `@backend/src/controllers/users.controller.js` around lines 195 - 205, In the
forgotPassword handler, stop including the plain resetToken in the HTTP
response; after generating resetToken and storing its hash on the user
(user.resetPasswordToken / user.resetPasswordExpires in forgotPassword), send
the raw resetToken to the user's email (e.g. call your mailer like
sendResetEmail(resetToken, user.email)) or only expose it in dev with a guarded
flag, then return a generic ApiResponse (e.g. "If an account with that email
exists, a reset link has been sent") without the token in the JSON payload.
Comment on lines
248
to
251
| const updateUserProfile = asyncHandler(async (req, res) => { | ||
| const { name, phone, whatsapp, profileImage, preferences, hostelLocation } = | ||
| req.body; | ||
|
|
||
| const user = await User.findByIdAndUpdate( | ||
| req.user._id, | ||
| { | ||
| $set: { | ||
| name, | ||
| phone, | ||
| whatsapp, | ||
| profileImage, | ||
| preferences, | ||
| hostelLocation, | ||
| }, | ||
| }, | ||
| { new: true } | ||
| ).select("-password -refreshTokens -verificationToken -resetPasswordToken"); | ||
|
|
||
| return res | ||
| .status(200) | ||
| .json(new ApiResponse(200, user, "Account details updated successfully")); | ||
| const user = await User.findByIdAndUpdate(req.user._id, { $set: req.body }, { new: true }).select("-password"); | ||
| return res.status(200).json(new ApiResponse(200, user, "Profile updated")); | ||
| }); |
There was a problem hiding this comment.
Whitelist profile fields before update.
Passing req.body directly allows clients to mutate sensitive fields (e.g., roles, isVerified, refreshTokens). Prefer an allowlist.
🔐 Suggested allowlist update
- const user = await User.findByIdAndUpdate(req.user._id, { $set: req.body }, { new: true }).select("-password");
+ const allowedUpdates = (({
+ name,
+ phone,
+ whatsapp,
+ hostelLocation,
+ profileImage,
+ }) => ({ name, phone, whatsapp, hostelLocation, profileImage }))(req.body);
+
+ const user = await User.findByIdAndUpdate(
+ req.user._id,
+ { $set: allowedUpdates },
+ { new: true }
+ ).select("-password");🤖 Prompt for AI Agents
In `@backend/src/controllers/users.controller.js` around lines 248 - 251, The
updateUserProfile handler currently passes req.body directly into
User.findByIdAndUpdate, allowing clients to change sensitive fields; instead
build a whitelist of allowed profile fields (e.g., name, email, avatar, bio,
etc.), copy only those keys from req.body into an updatePayload,
validate/normalize expected types if needed, then call
User.findByIdAndUpdate(req.user._id, { $set: updatePayload }, { new: true
}).select("-password") so roles, isVerified, refreshTokens and other sensitive
fields cannot be modified via updateUserProfile or ApiResponse.
Comment on lines
+36
to
+44
| // Check if a refresh token actually exists before trying to call the API | ||
| const hasRefreshToken = Cookies.get('refreshToken'); | ||
|
|
||
| if (!hasRefreshToken) { | ||
| // If no refresh token exists, don't even try. Just clear and go to login. | ||
| localStorage.removeItem('user'); | ||
| if (window.location.pathname !== '/login') window.location.href = '/login'; | ||
| return Promise.reject(error); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's understand the structure and find the api.js file
find . -name "api.js" -type f | head -20Repository: OpenLake/Campus-Marketplace
Length of output: 101
🏁 Script executed:
# Look at the full context of the api.js file around lines 36-44
cat -n frontend/src/services/api.js | head -60Repository: OpenLake/Campus-Marketplace
Length of output: 2479
🏁 Script executed:
# Search for refreshToken handling in the codebase
rg "refreshToken" --type js -A 3 -B 3Repository: OpenLake/Campus-Marketplace
Length of output: 10773
🏁 Script executed:
# Look for backend configuration or any httpOnly cookie settings
find . -name "*.js" -o -name "*.ts" -o -name "*.json" | xargs grep -l "httpOnly" 2>/dev/null | head -20Repository: OpenLake/Campus-Marketplace
Length of output: 116
Remove the refresh-token precheck—it blocks legitimate refresh attempts.
httpOnly cookies cannot be read by JavaScript. The Cookies.get('refreshToken') check on line 37 always returns undefined, causing all 401 errors to redirect to login even when a valid refresh token exists. Since the axios instance is configured with withCredentials: true, the browser automatically includes the httpOnly cookie in the POST request to /users/refresh-token, where the backend validates it server-side. Remove the precheck and let the refresh endpoint handle validation; if it fails, the catch block (lines 52–58) already handles cleanup and redirect.
Suggested fix
- // Check if a refresh token actually exists before trying to call the API
- const hasRefreshToken = Cookies.get('refreshToken');
-
- if (!hasRefreshToken) {
- // If no refresh token exists, don't even try. Just clear and go to login.
- localStorage.removeItem('user');
- if (window.location.pathname !== '/login') window.location.href = '/login';
- return Promise.reject(error);
- }
originalRequest._retry = true;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Check if a refresh token actually exists before trying to call the API | |
| const hasRefreshToken = Cookies.get('refreshToken'); | |
| if (!hasRefreshToken) { | |
| // If no refresh token exists, don't even try. Just clear and go to login. | |
| localStorage.removeItem('user'); | |
| if (window.location.pathname !== '/login') window.location.href = '/login'; | |
| return Promise.reject(error); | |
| } | |
| originalRequest._retry = true; |
🤖 Prompt for AI Agents
In `@frontend/src/services/api.js` around lines 36 - 44, Remove the client-side
refresh-token precheck that uses Cookies.get('refreshToken')/hasRefreshToken in
the response interceptor so httpOnly cookies aren't assumed missing; delete the
entire if (!hasRefreshToken) branch and its early
localStorage.removeItem('user') + redirect/Promise.reject(error) logic inside
the interceptor, leaving the code to call the refresh endpoint (POST
/users/refresh-token via the axios instance configured with withCredentials) and
rely on the existing catch block to handle cleanup and redirect when the refresh
actually fails.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Bug Fixes / Behavior
Style
Chores
✏️ Tip: You can customize this high-level summary in your review settings.