Added profile fetch and edit. Updated Navbar, routes, and guards to reflect auth state. #215
Conversation
Implemented JWT authentication in backend, added profile fetch and edit routes, and created corresponding frontend components for protected profile viewing and editing. Updated Navbar and routing to support authentication state and user profile management.
✅ Deploy Preview for github-spy ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
WalkthroughAdds JWT auth to backend and frontend: token issuance on login, token validation middleware, protected profile GET/PUT endpoints; frontend adds an Axios API client that injects Bearer tokens, protected routes, Navbar auth UI, Profile and EditProfile pages, and login flow storing tokens in localStorage. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Frontend
participant Backend
User->>Frontend: Submit login (email,password)
Frontend->>Backend: POST /login
Backend-->>Frontend: 200 { token, user }
Frontend->>Frontend: store token & user in localStorage
Frontend->>Frontend: dispatch authChange
Frontend-->>User: redirect to /
sequenceDiagram
participant User
participant Frontend
participant Backend
User->>Frontend: Navigate to /profile
Frontend->>Frontend: check isAuthenticated (localStorage / auth state)
alt authenticated
Frontend->>Backend: GET /profile (Authorization: Bearer <token>)
Backend->>Backend: requireAuth validates token, sets req.userId
Backend-->>Frontend: 200 { user }
Frontend-->>User: render Profile
else not authenticated
Frontend-->>User: Navigate to /login
end
sequenceDiagram
participant User
participant Frontend
participant Backend
User->>Frontend: Submit profile edit form
Frontend->>Backend: PUT /profile (Authorization: Bearer <token>, payload)
Backend->>Backend: requireAuth, validate password/current, apply updates
Backend-->>Frontend: 200 { updated user }
Frontend->>Frontend: update localStorage user, show success
Frontend-->>User: navigate to /profile
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
nishanthkj
changed the title
nishanthkj
changed the title
![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
♻️ Duplicate comments (1)
src/pages/Editprofile/EditProfile.tsx (1)
26-33: Theme logic is duplicated.This is the same theme initialization code as in Profile.tsx. Consider extracting to a shared hook as suggested in the Profile.tsx review.
🧹 Nitpick comments (9)
backend/routes/auth.js (1)
30-30: Rename SESSION_SECRET to JWT_SECRET for clarity.Using
process.env.SESSION_SECRETfor JWT signing is misleading. The environment variable name should reflect its purpose.Consider renaming the environment variable:
-const token = jwt.sign({ id: user.id }, process.env.SESSION_SECRET, { expiresIn: "1d" }); +const token = jwt.sign({ id: user.id }, process.env.JWT_SECRET, { expiresIn: "1d" });Update your
.envfile and documentation accordingly to useJWT_SECRETinstead ofSESSION_SECRET.lib/api.ts (1)
8-23: Consider adding response interceptor for 401 handling.Currently, 401 responses are handled individually in components. A global response interceptor could automatically clear auth state and redirect to login.
Add response interceptor after the request interceptor:
// Response interceptor to handle 401 globally api.interceptors.response.use( (response) => response, (error) => { if (error.response?.status === 401) { localStorage.removeItem("token"); localStorage.removeItem("user"); window.dispatchEvent(new Event("authChange")); window.location.href = "/login"; } return Promise.reject(error); } );src/pages/Profile/Profile.tsx (1)
16-25: Consider extracting theme logic into a custom hook.The same theme initialization logic is duplicated in both Profile.tsx and EditProfile.tsx. Extracting it would improve maintainability.
Create a custom hook
useThemein a shared location:// hooks/useTheme.ts import { useEffect, useState } from "react"; export const useTheme = () => { const [isDark, setIsDark] = useState(false); useEffect(() => { const savedTheme = localStorage.getItem("theme"); const dark = savedTheme === "dark"; setIsDark(dark); const root = document.documentElement; if (dark) root.classList.add("dark"); else root.classList.remove("dark"); }, []); return isDark; };Then use it in both components:
const isDark = useTheme();src/components/Navbar.tsx (6)
14-36: Outside-click and auth sync: add pointerdown + Escape + user sync
- Use
pointerdownfor more reliable outside-close (touch + quicker).- Handle
Escapeto close menus (a11y).- Update
useronstorage("user" key) and on customauthChange.useEffect(() => { - const onClick = (e: MouseEvent) => { + const onPointerDown = (e: PointerEvent) => { if (menuRef.current && !menuRef.current.contains(e.target as Node)) setMenuOpen(false); }; const onStorage = (e: StorageEvent) => { - if (e.key === "token") setIsAuthed(!!e.newValue); + if (e.key === "token") setIsAuthed(!!e.newValue); + if (e.key === "user") setUser(readUserFromStorage()); }; const onAuthChange = () => { setIsAuthed(!!localStorage.getItem("token")); + setUser(readUserFromStorage()); }; window.addEventListener("storage", onStorage); window.addEventListener("authChange", onAuthChange); - window.addEventListener("click", onClick); + window.addEventListener("pointerdown", onPointerDown); + const onKeyDown = (e: KeyboardEvent) => { + if (e.key === "Escape") { + setMenuOpen(false); + setIsOpen(false); + } + }; + window.addEventListener("keydown", onKeyDown); return () => { window.removeEventListener("storage", onStorage); window.removeEventListener("authChange", onAuthChange); - window.removeEventListener("click", onClick); + window.removeEventListener("pointerdown", onPointerDown); + window.removeEventListener("keydown", onKeyDown); }; }, []);
94-99: A11y: label button, wire menu with aria-controls/labelledby, mark items as menuitemImproves screen-reader UX and keyboard navigation semantics.
- <button + <button + id="user-menu-button" onClick={() => setMenuOpen((v) => !v)} className="flex items-center gap-2 px-2 py-1 rounded hover:bg-gray-100 dark:hover:bg-gray-800 transition" aria-haspopup="menu" aria-expanded={menuOpen} + aria-controls="user-menu" + aria-label="User menu" >- <div - role="menu" + <div + role="menu" + id="user-menu" + aria-labelledby="user-menu-button" className="absolute right-0 mt-2 w-56 rounded-lg border border-gray-200 dark:border-gray-700 bg-white dark:bg-gray-800 shadow-lg p-2" >- <Link + <Link + role="menuitem" to="/profile" className="block px-3 py-2 text-sm rounded hover:bg-gray-100 dark:hover:bg-gray-700" onClick={() => setMenuOpen(false)} > View Profile </Link> - <Link + <Link + role="menuitem" to="/profile/edit" className="block px-3 py-2 text-sm rounded hover:bg-gray-100 dark:hover:bg-gray-700" onClick={() => setMenuOpen(false)} > Edit Profile </Link> - <button + <button + role="menuitem" onClick={() => { setMenuOpen(false); handleLogout(); }} className="block w-full text-left px-3 py-2 text-sm rounded hover:bg-gray-100 dark:hover:bg-gray-700" > Logout </button>Also applies to: 129-133, 147-170
48-50: Auth state: prefer AuthContext over direct localStorage readsSince the PR adds an Auth context, derive
isAuthedanduserfrom it to avoid divergent state and bespoke event wiring in components.If AuthContext already exposes
isAuthed/user, consider:const { isAuthed, user, logout } = useAuth(); // exampleElse, we can refactor Navbar after AuthContext is finalized. Confirm what the context currently exposes.
1-6: Minor import nitYou import
useRefseparately; consider importing it with the other React hooks for consistency, but feel free to keep as-is per your lint rules.
48-50: Security note: JWT in localStorage increases XSS riskLonger-term, consider HttpOnly, Secure, SameSite cookies for access/refresh tokens to reduce exposure. If staying with localStorage, ensure strict CSP, no inline scripts, and rigorous input sanitization elsewhere.
247-264: Mobile logout should also close menu immediately (minor UX)Navigation will close via route-change, but proactively closing feels snappier.
- <button - onClick={() => { - handleLogout(); - }} + <button + onClick={() => { + setIsOpen(false); + handleLogout(); + }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
public/profile.svgis excluded by!**/*.svg
📒 Files selected for processing (9)
backend/package.json(1 hunks)backend/routes/auth.js(3 hunks)lib/api.ts(1 hunks)src/Routes/Router.tsx(3 hunks)src/components/Navbar.tsx(3 hunks)src/components/ProtectedRoute.tsx(1 hunks)src/pages/Editprofile/EditProfile.tsx(1 hunks)src/pages/Login/Login.tsx(4 hunks)src/pages/Profile/Profile.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
src/Routes/Router.tsx (1)
src/pages/Editprofile/EditProfile.tsx (1)
EditProfile(13-218)
backend/routes/auth.js (2)
backend/server.js (1)
passport(4-4)backend/config/passportConfig.js (4)
passport(1-1)user(10-10)user(40-40)User(3-3)
src/pages/Editprofile/EditProfile.tsx (1)
backend/routes/auth.js (3)
user(29-29)user(70-70)user(89-93)
lib/api.ts (1)
backend/routes/auth.js (2)
token(30-30)token(57-57)
src/pages/Profile/Profile.tsx (1)
backend/routes/auth.js (4)
User(3-3)user(29-29)user(70-70)user(89-93)
🔇 Additional comments (15)
backend/routes/auth.js (2)
51-65: LGTM with a note on token validation.The middleware correctly validates JWT tokens and handles missing/invalid tokens with appropriate 401 responses. The decoded user ID is properly attached to the request for downstream use.
68-76: LGTM - Profile fetch endpoint is correctly secured.The endpoint properly uses the requireAuth middleware, excludes the password field from the response, and handles user-not-found scenarios.
src/components/ProtectedRoute.tsx (1)
1-18: LGTM - Clean route protection implementation.The component correctly guards routes by redirecting unauthenticated users to the login page. The use of
replaceprop prevents users from navigating back to protected routes via browser history.src/Routes/Router.tsx (2)
16-27: LGTM - Proper auth state synchronization.The implementation correctly initializes auth state from localStorage and synchronizes it via custom events and storage events. Event listener cleanup in the useEffect return prevents memory leaks.
39-54: LGTM - Protected routes properly configured.The profile routes are correctly wrapped with ProtectedRoute component, ensuring only authenticated users can access them.
lib/api.ts (1)
8-23: LGTM - Standard JWT interceptor pattern.The Axios instance correctly attaches the Bearer token from localStorage to all requests. This centralizes auth header management across the app.
src/pages/Login/Login.tsx (2)
38-50: LGTM - Proper JWT storage and auth state sync.The login flow correctly stores the token and user data in localStorage and dispatches the authChange event to synchronize state across the app. Redirect to root path is appropriate.
51-57: LGTM - Enhanced error handling.The error handling properly distinguishes between Axios errors with server responses and other error types, providing appropriate user feedback.
src/pages/Profile/Profile.tsx (1)
28-40: LGTM - Proper profile data fetching.The profile fetch correctly uses the centralized API client, handles errors, and manages loading state.
src/pages/Editprofile/EditProfile.tsx (3)
35-53: LGTM - Proper profile data initialization.The profile fetch correctly handles loading state and populates the form with current user data, clearing password fields appropriately.
55-62: LGTM - Comprehensive client-side validation.The validation logic correctly enforces username length, email format, password requirement, and conditional new password length when password change is enabled.
86-86: Fix API endpoint path inconsistency.The edit profile submit uses
/profile, but the profile fetch uses/api/auth/profile. This inconsistency could cause issues if the backend doesn't handle both paths.Update the endpoint to match the backend route:
-const { data } = await api.put("/profile", payload); +const { data } = await api.put("/api/auth/profile", payload);Likely an incorrect or invalid review comment.
backend/package.json (1)
21-21: No vulnerabilities found in jsonwebtoken ^9.0.2Version 9.0.2 is the latest available version and all known security advisories are patched in this version.
src/components/Navbar.tsx (2)
38-41: Good: mobile menu auto-closes on route changeClosing via
useEffectonlocation.pathnameis correct and avoids orphaned open menus after navigation.
215-246: Routes /about and /contact are defined and won't cause 404s; however, mobile menu is missing /track (present in desktop)Routes are properly defined in
src/Routes/Router.tsx(lines 31, 34-36). The actual inconsistency: desktop menu has/track, but mobile menu lacks it. Consider adding/trackto mobile menu for parity.Likely an incorrect or invalid review comment.
| router.post("/login", passport.authenticate("local", { session: false }), (req, res) => { | ||
| try { | ||
| const user = req.user; | ||
| const token = jwt.sign({ id: user.id }, process.env.SESSION_SECRET, { expiresIn: "1d" }); | ||
| res.status(200).json({ message: "Login successful", token, user }); | ||
| } catch (error) { | ||
| res.status(500).json({ message: "Login failed", error: error.message }); | ||
| } | ||
| } | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
Verify Passport authentication configuration.
The login route uses passport.authenticate("local", { session: false }) which requires proper Passport local strategy configuration. Ensure the strategy is correctly set up to validate credentials before JWT issuance.
🏁 Script executed:
#!/bin/bash
# Verify Passport local strategy configuration
cat backend/config/passportConfig.jsLength of output: 1289
---
🏁 Script executed:
```shell
#!/bin/bash
# Check if User model has comparePassword method
rg -n "comparePassword" backend/models/User.js -A 3
Length of output: 152
🏁 Script executed:
#!/bin/bash
# Verify passportConfig is imported in auth.js
head -20 backend/routes/auth.js | grep -E "import|require"Length of output: 150
---
🏁 Script executed:
```shell
#!/bin/bash
# Check entire auth.js for passportConfig import
rg -n "passportConfig|passport.use" backend/routes/auth.js
Length of output: 0
🏁 Script executed:
#!/bin/bash
# Show full auth.js file to understand structure
cat backend/routes/auth.jsLength of output: 3399
Import passportConfig.js in auth.js to register the LocalStrategy.
The Passport local strategy is properly configured in backend/config/passportConfig.js with bcrypt password validation, but it's never imported in backend/routes/auth.js. When the login route calls passport.authenticate("local"), the strategy won't be registered, causing authentication to fail.
Add this import at the top of backend/routes/auth.js:
require("../config/passportConfig");🤖 Prompt for AI Agents
In backend/routes/auth.js around lines 27-36, the Passport LocalStrategy from
backend/config/passportConfig.js is not imported so
passport.authenticate("local") has no strategy registered; fix by adding a
top-of-file require/import that loads ../config/passportConfig (so the
LocalStrategy and bcrypt validation are registered before any
passport.authenticate calls), then keep the login route as-is.
| window.addEventListener("storage", onStorage); | ||
| window.addEventListener("authChange", onAuthChange); | ||
| window.addEventListener("click", onClick); |
There was a problem hiding this comment.
🧩 Analysis chain
Ensure 'authChange' is actually dispatched on login/profile save
Navbar listens to 'authChange', but if other code never dispatches it, same-tab updates won’t propagate.
🏁 Script executed:
#!/bin/bash
# Find any dispatches of authChange in the repo
rg -nP "dispatchEvent\\(new\\s+(?:Custom)?Event\\(['\"]authChange['\"]\\)\\)" -C2 || trueLength of output: 318
Let me search more broadly for all references to "authChange" and look for profile operations:
Let me search for profile save operations and see how authChange is used in Navbar:
Now let me search for profile save/update operations in the codebase:
Now let me search in the actual codebase for profile operations and whether authChange is dispatched in other scenarios:
Based on the initial shell script result showing authChange is dispatched in Login.tsx at line 44 after login, and my web search confirming that custom events work properly within the same tab, let me generate the final rewritten review comment:
Verify whether profile/settings updates also dispatch the 'authChange' event
The codebase currently dispatches the authChange custom event after successful login (Login.tsx:44). However, if users can update their authentication-related information (profile, email, password, etc.) through other forms or API calls, those operations should also dispatch authChange to keep the Navbar in sync. Without this, profile changes won't trigger Navbar updates within the same tab.
Ensure all auth state mutations (login, logout, profile updates) trigger the authChange event.
🏁 Script executed:
#!/bin/bash
# Search for profile-related operations and API calls
rg -nP "profile|update.*user|save.*user" --type=tsx --type=ts -i -C2 | head -80Length of output: 27
🏁 Script executed:
#!/bin/bash
# Search for all mentions of authChange in the codebase
rg -n "authChange" --type=tsx --type=ts -C3Length of output: 27
Let me retry with corrected syntax:
🏁 Script executed:
#!/bin/bash
# Search for all authChange references
rg -n "authChange" -C3Length of output: 1760
🏁 Script executed:
#!/bin/bash
# Search for profile/user update operations
rg -n "profile|update.*user|save" -i -C2Length of output: 21492
Dispatch 'authChange' event when profile is updated (EditProfile.tsx:86)
The codebase dispatches authChange on login (Login.tsx:44), but the profile update endpoint (EditProfile.tsx:86) does not dispatch this event after a successful update. This means the Navbar and Router won't reflect profile changes within the same tab until a page refresh.
Add window.dispatchEvent(new Event("authChange")); after the successful profile update response in EditProfile.tsx, similar to the login flow.
🤖 Prompt for AI Agents
In src/components/EditProfile.tsx around line 86, the profile update flow does
not dispatch the 'authChange' event after a successful update, so in-tab
components like Navbar/Router don't pick up the new profile; add
window.dispatchEvent(new Event("authChange")) immediately after the successful
update response handling (the same place Login.tsx dispatches it) so the rest of
the app can react to the changed profile.
| {/* Avatar Icon */} | ||
| <img | ||
| src={ | ||
| JSON.parse(localStorage.getItem("user") || "{}") | ||
| ?.avatarUrl || "/profile.svg" | ||
| } | ||
| alt="avatar" | ||
| className="h-8 w-8 rounded-full object-cover border border-gray-300 dark:border-gray-600" | ||
| /> | ||
|
|
||
| {/* Username beside icon */} | ||
| {/* <span className="text-sm font-medium"> | ||
| {JSON.parse(localStorage.getItem("user") || "{}")?.username || | ||
| "Me"} | ||
| </span> */} | ||
|
|
||
| {/* Chevron icon */} | ||
| <svg | ||
| className={`h-4 w-4 transition-transform ${ | ||
| menuOpen ? "rotate-180" : "" | ||
| }`} | ||
| viewBox="0 0 20 20" | ||
| fill="currentColor" | ||
| > | ||
| <path d="M5.25 7.5L10 12.25L14.75 7.5H5.25Z" /> | ||
| </svg> | ||
| </button> | ||
|
|
||
| {/* Dropdown Menu */} | ||
| {menuOpen && ( | ||
| <div | ||
| role="menu" | ||
| className="absolute right-0 mt-2 w-56 rounded-lg border border-gray-200 dark:border-gray-700 bg-white dark:bg-gray-800 shadow-lg p-2" | ||
| > | ||
| {/* User Info */} | ||
| <div className="px-3 py-2 border-b border-gray-100 dark:border-gray-700"> | ||
| <p className="text-sm font-semibold text-gray-900 dark:text-gray-100"> | ||
| {JSON.parse(localStorage.getItem("user") || "{}") | ||
| ?.username || "User"} | ||
| </p> | ||
| <p className="text-xs text-gray-500 dark:text-gray-400 truncate"> | ||
| {JSON.parse(localStorage.getItem("user") || "{}") | ||
| ?.email || "email@example.com"} | ||
| </p> | ||
| </div> |
There was a problem hiding this comment.
Guard against invalid localStorage.user and stop re-parsing per render
Repeated JSON.parse(localStorage.getItem("user") || "{}") in render can throw if the value is malformed and will re-parse on every render. Cache once in state with a safe parser and update via storage/authChange. Replace inline parses with a user object.
Apply within this range (use user var instead of parsing):
- <img
- src={
- JSON.parse(localStorage.getItem("user") || "{}")
- ?.avatarUrl || "/profile.svg"
- }
+ <img
+ src={user?.avatarUrl || "/profile.svg"}
alt="avatar"
className="h-8 w-8 rounded-full object-cover border border-gray-300 dark:border-gray-600"
/>
...
- <p className="text-sm font-semibold text-gray-900 dark:text-gray-100">
- {JSON.parse(localStorage.getItem("user") || "{}")
- ?.username || "User"}
- </p>
+ <p className="text-sm font-semibold text-gray-900 dark:text-gray-100">
+ {user?.username || "User"}
+ </p>
...
- <p className="text-xs text-gray-500 dark:text-gray-400 truncate">
- {JSON.parse(localStorage.getItem("user") || "{}")
- ?.email || "email@example.com"}
- </p>
+ <p className="text-xs text-gray-500 dark:text-gray-400 truncate">
+ {user?.email || "email@example.com"}
+ </p>Add outside this range (top-level helpers/state):
type User = { username?: string; email?: string; avatarUrl?: string };
function readUserFromStorage(): User | null {
const raw = localStorage.getItem("user");
if (!raw) return null;
try {
const u = JSON.parse(raw);
return typeof u === "object" && u ? u : null;
} catch {
return null;
}
}
// state
const [user, setUser] = useState<User | null>(readUserFromStorage());And update listeners to keep user fresh (see event-listener diff below).
🤖 Prompt for AI Agents
In src/components/Navbar.tsx around lines 100 to 144, the component repeatedly
calls JSON.parse(localStorage.getItem("user") || "{}") in render which re-parses
every render and can throw on malformed data; replace those inline parses with a
single user state variable and a safe parser: add a top-level helper
readUserFromStorage() and initialize state like const [user, setUser] =
useState(readUserFromStorage()), use user?.avatarUrl / user?.username /
user?.email inside the JSX instead of parsing, and add storage and auth-change
listeners to update setUser when localStorage.user changes (also ensure the
parser returns null on error and you fallback to defaults like "User" or
"/profile.svg" in the JSX).
| <button | ||
| onClick={() => (window.location.href = "/profile/edit")} | ||
| className="mt-6 w-full py-2 bg-black text-white dark:bg-white dark:text-black rounded-lg hover:opacity-90 transition" | ||
| > | ||
| Edit Profile | ||
| </button> |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Use React Router navigation instead of window.location.href.
Using window.location.href causes a full page reload, bypassing React Router's client-side navigation benefits.
Import and use the navigate hook:
+import { useNavigate } from "react-router-dom";
+
const Profile = () => {
+ const navigate = useNavigate();
const [user, setUser] = useState<User | null>(null);
// ...
return (
<div className="flex justify-center items-center min-h-screen bg-gray-50 dark:bg-inherit px-4">
<div className="max-w-md w-full bg-white dark:bg-gray-800 rounded-2xl shadow-md p-6 text-center transition-colors">
{/* ... */}
<button
- onClick={() => (window.location.href = "/profile/edit")}
+ onClick={() => navigate("/profile/edit")}
className="mt-6 w-full py-2 bg-black text-white dark:bg-white dark:text-black rounded-lg hover:opacity-90 transition"
>
Edit Profile
</button>
</div>
</div>
);
};Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/pages/Profile/Profile.tsx around lines 75-80, the button uses
window.location.href which triggers a full page reload; replace this with React
Router navigation by importing useNavigate from react-router-dom at the top,
calling const navigate = useNavigate() inside the component, and change the
button onClick to call navigate('/profile/edit') (or navigate('edit') if using
nested routes) so navigation is client-side and preserves app state.
There was a problem hiding this comment.
This will work, and I am using raw JavaScript here
There was a problem hiding this comment.
@nishanthkj, understood! I'll leave the navigation approach as you've implemented it. 👍
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/routes/auth.js (1)
39-48: Logout route is inconsistent with JWT authentication.The logout route calls
req.logout(), which is designed for session-based authentication. Since the app now uses stateless JWT authentication (login issues tokens, not sessions), this endpoint is non-functional. JWT invalidation must happen client-side by removing the token from storage.Either remove this endpoint or update it to acknowledge the client-side nature of JWT logout:
-// Logout route -router.get("/logout", (req, res) => { - req.logout((err) => { - if (err) - return res.status(500).json({ message: 'Logout failed', error: err.message }); - else - res.status(200).json({ message: 'Logged out successfully' }); - }); -}); +// Logout route (JWT auth is stateless - client removes token) +router.post("/logout", (req, res) => { + // With JWT, logout is handled client-side by removing the token + // This endpoint exists for consistency but doesn't need server-side logic + res.status(200).json({ message: 'Logged out successfully' }); +});Note: True JWT invalidation requires a token blacklist or short expiration times. Consider implementing a blacklist if immediate token revocation is required.
♻️ Duplicate comments (3)
src/components/Navbar.tsx (1)
104-148: Repeated JSON.parse calls are still present (unresolved from previous review)Lines 107-108, 141-142, and 145-146 repeatedly call
JSON.parse(localStorage.getItem("user") || "{}")on every render, which:
- Re-parses the same data multiple times per render (performance issue)
- Can throw on malformed JSON, crashing the component
- Was flagged in a previous review comment with a complete solution
The previous review comment on lines 104-148 provided a complete solution with a
readUserFromStorage()helper and user state. Please implement that solution to:
- Add a safe JSON parser that returns null on error
- Store parsed user in state:
const [user, setUser] = useState<User | null>(readUserFromStorage())- Replace all inline
JSON.parse(...)calls withuser?.avatarUrl,user?.username,user?.email- Update event listeners to refresh user state when storage changes
This will also fix the
setUser(null)error on line 46.backend/routes/auth.js (2)
27-36: Critical: passportConfig.js is still not imported.This is the same critical issue flagged in the previous review. The
passport.authenticate("local")call on line 27 requires the LocalStrategy to be registered, butbackend/config/passportConfig.jsis never imported in this file. Authentication will fail at runtime.Add this import at the top of the file:
const express = require("express"); const passport = require("passport"); +require("../config/passportConfig"); const User = require("../models/User"); const router = express.Router(); const jwt = require("jsonwebtoken");
78-107: Add input validation for profile updates.The endpoint still lacks input validation as flagged in the previous review. Without validation, malformed or malicious input could cause data integrity issues or application errors.
Add validation before processing updates:
router.put("/profile", requireAuth, async (req, res) => { try { const { username, email, bio, avatar, newPassword } = req.body; + + // Validate input + if (username !== undefined) { + if (typeof username !== "string" || username.trim().length < 3 || username.trim().length > 30) { + return res.status(400).json({ message: "Username must be 3-30 characters" }); + } + } + if (email !== undefined) { + if (typeof email !== "string") { + return res.status(400).json({ message: "Invalid email" }); + } + const emailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/; + if (!emailRegex.test(email.trim())) { + return res.status(400).json({ message: "Invalid email format" }); + } + } + if (bio !== undefined) { + if (typeof bio !== "string" || bio.length > 500) { + return res.status(400).json({ message: "Bio must be 500 characters or less" }); + } + } + if (avatar !== undefined && avatar.trim().length > 0) { + const urlRegex = /^https?:\/\/.+/; + if (!urlRegex.test(avatar)) { + return res.status(400).json({ message: "Avatar must be a valid URL" }); + } + } + if (newPassword !== undefined && newPassword.trim().length > 0) { + if (newPassword.length < 6) { + return res.status(400).json({ message: "Password must be at least 6 characters" }); + } + } const user = await User.findById(req.userId); // ... rest of logic
🧹 Nitpick comments (1)
src/components/Navbar.tsx (1)
115-118: Consider removing commented codeThis commented username display can be removed unless you plan to re-enable it soon. Commented code can be retrieved from git history if needed later.
- {/* Username beside icon */} - {/* <span className="text-sm font-medium"> - {JSON.parse(localStorage.getItem("user") || "{}")?.username || - "Me"} - </span> */} -
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
backend/routes/auth.js(3 hunks)src/components/Navbar.tsx(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
backend/routes/auth.js (1)
backend/config/passportConfig.js (4)
passport(1-1)user(10-10)user(40-40)User(3-3)
🔇 Additional comments (5)
src/components/Navbar.tsx (3)
1-13: LGTM! Imports and setup are appropriate.The added hooks (useNavigate, useLocation, useRef) and state (menuOpen, menuRef) are correctly imported and initialized for the auth dropdown and navigation features.
38-40: LGTM! Mobile menu closes on navigation.This effect correctly closes the mobile menu when the user navigates, ensuring good UX.
251-277: LGTM! Mobile auth UI is consistent with desktop.The mobile menu correctly shows Profile and Logout for authenticated users, and Login for unauthenticated users, maintaining consistency with the desktop navigation.
backend/routes/auth.js (2)
50-65: LGTM! requireAuth middleware is well-implemented.The middleware correctly validates JWT tokens, extracts the user ID, and handles error cases with appropriate 401 responses.
67-76: LGTM! Profile fetch endpoint is correctly implemented.The endpoint is properly protected, excludes sensitive data, and has appropriate error handling.
| // Handle password update if newPassword is provided | ||
| if (newPassword !== undefined && newPassword.trim().length > 0) { | ||
| // Password will be hashed by the User model's pre-save hook | ||
| updates.password = newPassword; | ||
| } |
There was a problem hiding this comment.
Critical: Password update bypasses hashing via findByIdAndUpdate.
While password update logic was added per the previous review, it uses findByIdAndUpdate on line 95, which bypasses mongoose pre-save hooks. The password hashing (typically done in a pre-save hook) will not execute, causing passwords to be stored in plaintext.
Apply this diff to properly hash passwords:
router.put("/profile", requireAuth, async (req, res) => {
try {
- const updates = {};
const { username, email, bio, avatar, newPassword } = req.body;
- if (username !== undefined) updates.username = username;
- if (email !== undefined) updates.email = email;
- if (bio !== undefined) updates.bio = bio;
- if (avatar !== undefined) updates.avatar = avatar;
-
- // Handle password update if newPassword is provided
- if (newPassword !== undefined && newPassword.trim().length > 0) {
- // Password will be hashed by the User model's pre-save hook
- updates.password = newPassword;
- }
-
- const user = await User.findByIdAndUpdate(req.userId, updates, {
- new: true,
- runValidators: true,
- select: "-password",
- });
+ const user = await User.findById(req.userId);
+ if (!user) return res.status(404).json({ message: "User not found" });
+
+ // Apply updates
+ if (username !== undefined) user.username = username;
+ if (email !== undefined) user.email = email;
+ if (bio !== undefined) user.bio = bio;
+ if (avatar !== undefined) user.avatar = avatar;
+
+ // Handle password update if newPassword is provided
+ if (newPassword !== undefined && newPassword.trim().length > 0) {
+ user.password = newPassword; // Will be hashed by pre-save hook
+ }
+
+ await user.save(); // Triggers pre-save hooks and validators
+
+ // Remove password from response
+ const userResponse = user.toObject();
+ delete userResponse.password;
- if (!user) return res.status(404).json({ message: "User not found" });
-
- res.status(200).json({ message: "Profile updated successfully", user });
+ res.status(200).json({ message: "Profile updated successfully", user: userResponse });
} catch (err) {
res.status(500).json({ message: "Error updating profile", error: err.message });
}
});🤖 Prompt for AI Agents
In backend/routes/auth.js around lines 89 to 93, the current password-update
sets updates.password and later uses findByIdAndUpdate which bypasses Mongoose
pre-save hooks so the password won’t be hashed; instead, either (a) load the
user with User.findById(id), set user.password = newPassword and call await
user.save() so the pre-save hook runs, or (b) if you prefer to keep a single
update call, manually hash newPassword with the same hashing utility (e.g.,
bcrypt.hash with the same salt rounds) before assigning updates.password; ensure
you await the hashing/save and propagate errors accordingly.
| const onAuthChange = () => { | ||
| setIsAuthed(!!localStorage.getItem("token")); | ||
| }; |
There was a problem hiding this comment.
authChange listener should also refresh user data from localStorage
The listener updates isAuthed but doesn't re-read the user object. When a profile update dispatches authChange, the Navbar will still display stale username/email/avatar until a full page refresh.
After adding a user state variable (see other comment), update this listener:
const onAuthChange = () => {
setIsAuthed(!!localStorage.getItem("token"));
+ setUser(readUserFromStorage());
};Similarly, update the storage listener if you want cross-tab user updates:
const onStorage = (e: StorageEvent) => {
- if (e.key === "token") setIsAuthed(!!e.newValue);
+ if (e.key === "token") {
+ setIsAuthed(!!e.newValue);
+ }
+ if (e.key === "user") {
+ setUser(readUserFromStorage());
+ }
};Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/components/Navbar.tsx around lines 24 to 26, the onAuthChange handler
only refreshes isAuthed and leaves the in-memory user stale; update it to also
re-read and set the user state from localStorage (parse the stored user JSON or
null) so the Navbar reflects profile updates immediately; likewise update any
window "storage" event listener to also re-read and set both isAuthed and user
when the token/user keys change to enable cross-tab user updates.
| const handleLogout = () => { | ||
| localStorage.removeItem("token"); | ||
| localStorage.removeItem("user"); | ||
| setIsAuthed(false); | ||
| setUser(null); | ||
| // notify same-tab listeners | ||
| window.dispatchEvent(new Event("authChange")); | ||
| navigate("/login"); | ||
| }; |
There was a problem hiding this comment.
Critical: setUser is not defined
Line 46 calls setUser(null) but no user state is declared in this component. Only isAuthed state exists (line 52). This will throw a runtime error when logout is triggered.
You need to add the user state as recommended in the unresolved review comment on lines 104-148. After adding const [user, setUser] = useState<User | null>(readUserFromStorage()); at the top level, this call will work correctly.
🤖 Prompt for AI Agents
In src/components/Navbar.tsx around lines 42 to 50, handleLogout calls
setUser(null) but the component never declares a user state; add the user state
at the top level as suggested in the unresolved review (lines 104-148) by
declaring const [user, setUser] = useState<User | null>(readUserFromStorage());
so setUser is defined and initialized from storage, then keep the existing
handleLogout logic which clears storage, sets isAuthed to false, sets user to
null, dispatches the authChange event, and navigates to /login.
There was a problem hiding this comment.
@nishanthkj, I'll provide the code to add the user state. Add this at the top of the file before the Navbar component:
// Define User type
interface User {
username: string;
email: string;
avatarUrl?: string;
}
// Helper function to read user from localStorage
function readUserFromStorage(): User | null {
try {
const userStr = localStorage.getItem("user");
return userStr ? JSON.parse(userStr) : null;
} catch {
return null;
}
}Then add the user state declaration after line 14 (after const menuRef = useRef<HTMLDivElement>(null);):
const [user, setUser] = useState<User | null>(readUserFromStorage());
const [isAuthed, setIsAuthed] = useState<boolean>(!!localStorage.getItem("token"));Also update the onAuthChange function inside the first useEffect (around line 24) to sync the user state:
const onAuthChange = () => {
setIsAuthed(!!localStorage.getItem("token"));
setUser(readUserFromStorage());
};This will:
- Fix the
setUser(null)error in handleLogout - Keep user data in sync across the component
- Provide type safety for user data
- Allow you to replace
JSON.parse(localStorage.getItem("user") || "{}")with theuserstate variable throughout the component
| > | ||
| Login | ||
| </Link> | ||
| {/* replace your auth block with this */} |
There was a problem hiding this comment.
Remove leftover development comment
This comment should be removed before merging to production.
- {/* replace your auth block with this */}
{isAuthed ? (📝 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.
| {/* replace your auth block with this */} | |
| {isAuthed ? ( |
🤖 Prompt for AI Agents
In src/components/Navbar.tsx at line 95, remove the leftover development comment
"/* replace your auth block with this */" from the JSX; delete the comment line
entirely so the production build contains no dev-only annotations and ensure
surrounding JSX remains syntactically correct after removal.
There was a problem hiding this comment.
@nishanthkj Thank you for confirming! I'll mark this as resolved once you remove that comment. 👍
1 participant
Related Issue
Description
Backend
Added
requireAuthmiddleware to validate JWT and populatereq.userId.Added New API
POST, PUT /api/auth/profile→ To Get Profile and edit ProfileAdded New Routes
/profile/profile/editRoutes:
POST /api/auth/login→ returns JWT.GET /api/auth/profile→ returns authenticated user (sans password).PUT /api/auth/profile→ updates profile fields; supports password change when provided.Standardized error responses and status codes.
Frontend
Auth context + token handling.
Protected routes and redirect to login when unauthenticated.
Profile page:
Navbar reflects auth state and shows profile entry and logout.
Routing updated for guarded Profile route.
How Has This Been Tested?
Unit
Integration
GET /api/auth/profilewith token → 200 and user.GET /api/auth/profilewithout token → 401.PUT /api/auth/profileupdates fields; password change only when new password provided.Manual
Screenshots (if applicable)
Type of Change
Breaking Changes
Security
Note
Summary by CodeRabbit