-
Notifications
You must be signed in to change notification settings - Fork 63
Added profile fetch and edit. Updated Navbar, routes, and guards to reflect auth state. #215
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
base: main
Are you sure you want to change the base?
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 |
|---|---|---|
|
|
@@ -2,6 +2,7 @@ const express = require("express"); | |
| const passport = require("passport"); | ||
| const User = require("../models/User"); | ||
| const router = express.Router(); | ||
| const jwt = require("jsonwebtoken"); | ||
|
|
||
| // Signup route | ||
| router.post("/signup", async (req, res) => { | ||
|
|
@@ -23,9 +24,16 @@ router.post("/signup", async (req, res) => { | |
| }); | ||
|
|
||
| // Login route | ||
| router.post("/login", passport.authenticate('local'), (req, res) => { | ||
| res.status(200).json( { message: 'Login successful', user: req.user } ); | ||
| }); | ||
| 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 }); | ||
| } | ||
| } | ||
| ); | ||
|
|
||
| // Logout route | ||
| router.get("/logout", (req, res) => { | ||
|
|
@@ -39,4 +47,62 @@ router.get("/logout", (req, res) => { | |
| }); | ||
| }); | ||
|
|
||
| // ---------------- AUTH MIDDLEWARE ---------------- | ||
| function requireAuth(req, res, next) { | ||
| const authHeader = req.headers.authorization; | ||
| if (!authHeader || !authHeader.startsWith("Bearer ")) { | ||
| return res.status(401).json({ message: "Missing or invalid token" }); | ||
| } | ||
|
|
||
| const token = authHeader.split(" ")[1]; | ||
| try { | ||
| const decoded = jwt.verify(token, process.env.SESSION_SECRET); | ||
| req.userId = decoded.id; | ||
| next(); | ||
| } catch (err) { | ||
| return res.status(401).json({ message: "Invalid or expired token" }); | ||
| } | ||
| } | ||
|
|
||
| // ---------------- GET PROFILE ---------------- | ||
| router.get("/profile", requireAuth, async (req, res) => { | ||
| try { | ||
| const user = await User.findById(req.userId).select("-password"); | ||
| if (!user) return res.status(404).json({ message: "User not found" }); | ||
| res.status(200).json({ user }); | ||
| } catch (err) { | ||
| res.status(500).json({ message: "Error fetching profile", error: err.message }); | ||
| } | ||
| }); | ||
|
|
||
| // ---------------- EDIT PROFILE ---------------- | ||
| 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; | ||
| } | ||
|
Comment on lines
+89
to
+93
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. Critical: Password update bypasses hashing via findByIdAndUpdate. While password update logic was added per the previous review, it uses 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 |
||
|
|
||
| const user = await User.findByIdAndUpdate(req.userId, updates, { | ||
| new: true, | ||
| runValidators: true, | ||
| select: "-password", | ||
| }); | ||
|
|
||
| if (!user) return res.status(404).json({ message: "User not found" }); | ||
|
|
||
| res.status(200).json({ message: "Profile updated successfully", user }); | ||
| } catch (err) { | ||
| res.status(500).json({ message: "Error updating profile", error: err.message }); | ||
| } | ||
| }); | ||
| module.exports = router; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| /// <reference types="vite/client" /> | ||
| // src/services/api.ts | ||
| import axios from "axios"; | ||
|
|
||
| // Backend base URL from .env | ||
| const backendUrl = import.meta.env.VITE_BACKEND_URL; | ||
|
|
||
| const api = axios.create({ | ||
| baseURL: backendUrl, | ||
| headers: { "Content-Type": "application/json" }, | ||
| }); | ||
|
|
||
| // Interceptor to attach token from localStorage | ||
| api.interceptors.request.use( | ||
| (config) => { | ||
| const token = localStorage.getItem("token"); | ||
| if (token) { | ||
| config.headers.Authorization = `Bearer ${token}`; | ||
| } | ||
| return config; | ||
| }, | ||
| (error) => Promise.reject(error) | ||
| ); | ||
|
|
||
| export default api; |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,11 +1,57 @@ | ||||||
| import { Link } from "react-router-dom"; | ||||||
| import { useState, useContext } from "react"; | ||||||
| import { Link, useNavigate, useLocation } from "react-router-dom"; | ||||||
| import { useState, useContext, useEffect } from "react"; | ||||||
| import { ThemeContext } from "../context/ThemeContext"; | ||||||
| import { Moon, Sun } from 'lucide-react'; | ||||||
|
|
||||||
| import { Moon, Sun } from "lucide-react"; | ||||||
| import { useRef } from "react"; | ||||||
|
|
||||||
| const Navbar: React.FC = () => { | ||||||
| const navigate = useNavigate(); | ||||||
| const location = useLocation(); | ||||||
|
|
||||||
| const [menuOpen, setMenuOpen] = useState(false); | ||||||
| const menuRef = useRef<HTMLDivElement>(null); | ||||||
|
|
||||||
| useEffect(() => { | ||||||
| const onClick = (e: MouseEvent) => { | ||||||
| if (menuRef.current && !menuRef.current.contains(e.target as Node)) | ||||||
| setMenuOpen(false); | ||||||
| }; | ||||||
|
|
||||||
| const onStorage = (e: StorageEvent) => { | ||||||
| if (e.key === "token") setIsAuthed(!!e.newValue); | ||||||
| }; | ||||||
|
|
||||||
| const onAuthChange = () => { | ||||||
| setIsAuthed(!!localStorage.getItem("token")); | ||||||
| }; | ||||||
|
Comment on lines
+24
to
+26
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. authChange listener should also refresh user data from localStorage The listener updates After adding a 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());
+ }
};
🤖 Prompt for AI Agents |
||||||
|
|
||||||
| window.addEventListener("storage", onStorage); | ||||||
| window.addEventListener("authChange", onAuthChange); | ||||||
| window.addEventListener("click", onClick); | ||||||
|
Comment on lines
+28
to
+30
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. 🧩 Analysis chainEnsure 'authChange' is actually dispatched on login/profile save Navbar listens to 🏁 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 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 Verify whether profile/settings updates also dispatch the 'authChange' event The codebase currently dispatches the Ensure all auth state mutations (login, logout, profile updates) trigger the 🏁 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 Add 🤖 Prompt for AI Agents |
||||||
| return () => { | ||||||
| window.removeEventListener("storage", onStorage); | ||||||
| window.removeEventListener("authChange", onAuthChange); | ||||||
| window.removeEventListener("click", onClick); | ||||||
| }; | ||||||
| }, []); | ||||||
|
|
||||||
| useEffect(() => { | ||||||
| setIsOpen(false); // close mobile menu on navigation | ||||||
| }, [location.pathname]); | ||||||
|
|
||||||
| const handleLogout = () => { | ||||||
| localStorage.removeItem("token"); | ||||||
| localStorage.removeItem("user"); | ||||||
| setIsAuthed(false); | ||||||
| setUser(null); | ||||||
| // notify same-tab listeners | ||||||
| window.dispatchEvent(new Event("authChange")); | ||||||
| navigate("/login"); | ||||||
| }; | ||||||
|
Comment on lines
+42
to
+50
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. Critical: Line 46 calls You need to add the user state as recommended in the unresolved review comment on lines 104-148. After adding 🤖 Prompt for AI Agents
Contributor
Author
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. aDD IT
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. @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 [user, setUser] = useState<User | null>(readUserFromStorage());
const [isAuthed, setIsAuthed] = useState<boolean>(!!localStorage.getItem("token"));Also update the const onAuthChange = () => {
setIsAuthed(!!localStorage.getItem("token"));
setUser(readUserFromStorage());
};This will:
|
||||||
|
|
||||||
| const [isAuthed, setIsAuthed] = useState<boolean>( | ||||||
| !!localStorage.getItem("token") | ||||||
| ); | ||||||
| const [isOpen, setIsOpen] = useState<boolean>(false); | ||||||
| const themeContext = useContext(ThemeContext); | ||||||
|
|
||||||
|
|
@@ -46,12 +92,97 @@ const Navbar: React.FC = () => { | |||||
| > | ||||||
| Contributors | ||||||
| </Link> | ||||||
| <Link | ||||||
| to="/login" | ||||||
| className="text-lg font-medium hover:text-gray-300 transition-all px-2 py-1 border border-transparent hover:border-gray-400 rounded" | ||||||
| > | ||||||
| Login | ||||||
| </Link> | ||||||
| {/* replace your auth block with this */} | ||||||
|
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. Remove leftover development comment This comment should be removed before merging to production. - {/* replace your auth block with this */}
{isAuthed ? (📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
Contributor
Author
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. YES
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. @nishanthkj Thank you for confirming! I'll mark this as resolved once you remove that comment. 👍 |
||||||
| {isAuthed ? ( | ||||||
| <div className="relative" ref={menuRef}> | ||||||
| <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} | ||||||
| > | ||||||
| {/* 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> | ||||||
|
Comment on lines
+104
to
+148
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. Guard against invalid localStorage.user and stop re-parsing per render Repeated Apply within this range (use - <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 🤖 Prompt for AI Agents |
||||||
|
|
||||||
| {/* Menu Links */} | ||||||
| <Link | ||||||
| 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 | ||||||
| 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 | ||||||
| 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> | ||||||
| </div> | ||||||
| )} | ||||||
| </div> | ||||||
| ) : ( | ||||||
| <Link | ||||||
| to="/login" | ||||||
| className="text-lg font-medium hover:text-gray-300 transition-all px-2 py-1 border border-transparent hover:border-gray-400 rounded" | ||||||
| > | ||||||
| Login | ||||||
| </Link> | ||||||
| )} | ||||||
|
|
||||||
| <button | ||||||
| onClick={toggleTheme} | ||||||
| className="text-sm font-semibold px-3 py-1 rounded border border-gray-500 hover:text-gray-300 hover:border-gray-300 transition duration-200" | ||||||
|
|
@@ -117,13 +248,33 @@ const Navbar: React.FC = () => { | |||||
| > | ||||||
| Contributors | ||||||
| </Link> | ||||||
| <Link | ||||||
| to="/login" | ||||||
| className="block text-lg font-medium hover:text-gray-300 transition-all px-2 py-1 border border-transparent hover:border-gray-400 rounded" | ||||||
| onClick={() => setIsOpen(false)} | ||||||
| > | ||||||
| Login | ||||||
| </Link> | ||||||
| {isAuthed ? ( | ||||||
| <> | ||||||
| <Link | ||||||
| to="/profile" | ||||||
| className="block text-lg font-medium hover:text-gray-300 transition-all px-2 py-1 border border-transparent hover:border-gray-400 rounded" | ||||||
| onClick={() => setIsOpen(false)} | ||||||
| > | ||||||
| Profile | ||||||
| </Link> | ||||||
| <button | ||||||
| onClick={() => { | ||||||
| handleLogout(); | ||||||
| }} | ||||||
| className="block text-left text-lg font-medium px-2 py-1 border border-transparent hover:border-gray-400 rounded w-full" | ||||||
| > | ||||||
| Logout | ||||||
| </button> | ||||||
| </> | ||||||
| ) : ( | ||||||
| <Link | ||||||
| to="/login" | ||||||
| className="block text-lg font-medium hover:text-gray-300 transition-all px-2 py-1 border border-transparent hover:border-gray-400 rounded" | ||||||
| onClick={() => setIsOpen(false)} | ||||||
| > | ||||||
| Login | ||||||
| </Link> | ||||||
| )} | ||||||
| <button | ||||||
| onClick={() => { | ||||||
| toggleTheme(); | ||||||
|
|
||||||
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.
🧩 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:
Length of output: 1289
Length of output: 152
🏁 Script executed:
Length of output: 150
Length of output: 0
🏁 Script executed:
Length of output: 3399
Import passportConfig.js in auth.js to register the LocalStrategy.
The Passport local strategy is properly configured in
backend/config/passportConfig.jswith bcrypt password validation, but it's never imported inbackend/routes/auth.js. When the login route callspassport.authenticate("local"), the strategy won't be registered, causing authentication to fail.Add this import at the top of
backend/routes/auth.js:🤖 Prompt for AI Agents