Add Logout Functionality to Clear Token and Redirect User#18
Add Logout Functionality to Clear Token and Redirect User#18zaibamachhaliya wants to merge 5 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces a full authentication + logout flow for ThinkBoard, including UI routes for login/signup and a backend JWT (HTTP-only cookie) auth API with optional Upstash Redis token blacklisting. It expands the codebase from “no-auth notes UI” toward a session-based, protected-app experience.
Changes:
- Added frontend auth routing,
AuthContext, protected routes, and navbar logout/login/signup actions. - Added backend user model + auth controllers/middleware and
/api/auth/*routes with cookie-based JWT. - Introduced Upstash Redis integration for rate limiting and token blacklist storage on logout.
Reviewed changes
Copilot reviewed 22 out of 27 changed files in this pull request and generated 22 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/tailwind.config.js | Minor formatting update for Tailwind/DaisyUI config. |
| frontend/src/pages/Signup.jsx | New signup page using AuthContext.register. |
| frontend/src/pages/Login.jsx | New login page using AuthContext.login. |
| frontend/src/pages/HomePage.jsx | Adds auth-aware redirects/loading and tweaks rate-limit rendering. |
| frontend/src/main.jsx | App bootstrap updated (routing now handled in App.jsx). |
| frontend/src/lib/utils.js | Minor formatting change. |
| frontend/src/lib/axios.js | Axios client updated for cookie auth + response interceptor. |
| frontend/src/index.css | Minor formatting/whitespace change. |
| frontend/src/context/AuthContext.jsx | New auth state provider with /auth/me, login/register/logout helpers. |
| frontend/src/components/ProtectedRoute.jsx | New route guard component for authenticated pages. |
| frontend/src/components/Navbar.jsx | Adds logout button (and login/signup links when logged out). |
| frontend/src/App.jsx | Reworks routing to include auth routes and route protection. |
| frontend/package.json | Dependency updates/additions (axios, react-router-dom, local file dep). |
| frontend/package-lock.json | Lockfile updates reflecting dependency changes. |
| backend/src/Utils/Validetor.js | New request validation helper for auth inputs. |
| backend/src/server.js | Adds cookie parsing, auth routes, health check, redis connection attempt, startup refactor. |
| backend/src/routes/notesRoutes.js | Minimal change (but remains the notes route entrypoint). |
| backend/src/routes/authRoutes.js | New auth router wiring register/login/logout/me endpoints. |
| backend/src/models/User.js | New Mongoose User model. |
| backend/src/middleware/rateLimiter.js | Updates rate limiting behavior (skip in non-production). |
| backend/src/middleware/authMiddleware.js | New JWT cookie authentication middleware. |
| backend/src/controllers/userAuth.js | New register/login/logout/current-user controllers with cookie JWT + blacklist on logout. |
| backend/src/controllers/notesController.js | Minor formatting change. |
| backend/src/config/upstash.js | Updates Upstash redis setup used by rate limiting. |
| backend/src/config/redis.js | New Upstash Redis client + connection check helper. |
| backend/package.json | Adds auth/redis-related deps (bcryptjs, jsonwebtoken, cookie-parser, validator, etc.). |
| backend/package-lock.json | Lockfile updates reflecting backend dependency changes. |
Files not reviewed (2)
- backend/package-lock.json: Language not supported
- frontend/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Hey @zaibamachhaliya, thank you so much for putting this together! Implementing a full JWT authentication and logout flow is a huge undertaking, and the structure you've laid out here looks fantastic. |
|
Before we can merge this in, there are a few merge conflicts to resolve, along with some critical bugs caught by the Copilot review that need addressing:
Unprotected Notes: Currently, the routes in backend/src/routes/notesRoutes.js don't use the new authenticateUser middleware, meaning anyone can still modify notes without being logged in. Hardcoded API URL: In frontend/src/lib/axios.js, the base URL is hardcoded to localhost:5001, which will break in production. Let's revert that to use the environment variable. Package.json Bug: There's an accidental "mern-thinkboard": "file:.." dependency in backend/package.json that will cause deployment builds to fail.
AuthContext Errors: In AuthContext.jsx, catching and returning false for login/register prevents the actual pages from showing the correct error messages (like 401 vs 404). Let the errors propagate so the UI can handle them. Logout Route: Remove the authenticateUser middleware from the /logout route in authRoutes.js. If a user's token is already expired, they still need to be able to hit this endpoint to clear their browser cookie. Once these are patched up and the conflicts are resolved, we'll be good to go. Let me know if you need help with any of these fixes! |
📝 WalkthroughWalkthroughThis PR implements complete user logout functionality: backend adds a Redis-backed JWT token blacklist and authenticated ChangesUser Logout Feature with Token Blacklisting and Security Hardening
Sequence DiagramsequenceDiagram
participant User as User
participant Navbar as Navbar Component
participant Backend as Backend /auth/logout
participant Redis as Redis Cache
User->>Navbar: Click Logout in Avatar Menu
Navbar->>Navbar: Close dropdown
Navbar->>Backend: POST /auth/logout (token cookie)
activate Backend
Backend->>Backend: Decode JWT, compute TTL
Backend->>Redis: SETEX blacklist:{token} TTL
activate Redis
Redis-->>Backend: OK (fire-and-forget)
deactivate Redis
Backend->>Backend: Clear token cookie
Backend->>Navbar: 200 Success
deactivate Backend
Navbar->>User: Navigate to /login
🎯 3 (Moderate) | ⏱️ ~28 minutes
Possibly Related PRs
Suggested Labels
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/context/AuthContext.jsx (1)
38-63:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon’t swallow auth API errors in
register/login.Returning
falsefrom catch blocks (Lines 48 and 62) drops backend error details (status/message), so pages can’t render correct feedback paths.Suggested fix
const register = async (userData) => { try { const response = await api.post("/auth/register", userData); if (response.data.success) { setUser(response.data.user); return true; } return false; } catch (error) { console.error("Register API error:", error); - return false; + throw error; } }; const login = async (userData) => { try { const response = await api.post("/auth/login", userData); if (response.data.success) { setUser(response.data.user); return true; } return false; } catch (error) { console.error("Login API error:", error); - return false; + throw error; } };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/context/AuthContext.jsx` around lines 38 - 63, The catch blocks in register and login currently swallow backend error details by returning false; update register and login to propagate error information instead: in the catch handler of both functions (register and login) capture the caught error, extract meaningful details (e.g., error.response?.data or error.message), log them via console.error, and rethrow the error or return a structured error object so calling components can render proper feedback while still calling setUser only on success.
🧹 Nitpick comments (1)
backend/src/controllers/userAuth.js (1)
103-107: ⚡ Quick winAvoid
jwt.decodefor blacklist TTL derivation without signature verification.Line 103 trusts unverified JWT payload. If
/logoutis made unauthenticated (recommended in route file), forged tokens can drive arbitrary blacklist writes/TTLs. Verify signature first (e.g.,jwt.verify(..., { ignoreExpiration: true })) before readingexp.Suggested hardening
- const decoded = jwt.decode(token); + const decoded = jwt.verify(token, process.env.JWT_SECRET, { + ignoreExpiration: true, + });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/controllers/userAuth.js` around lines 103 - 107, Replace the unsafe jwt.decode usage with signature-verified parsing: call jwt.verify(token, <jwtSecret>, { ignoreExpiration: true }) (catching verification errors) and only read decoded.exp if verification succeeds; then compute ttl and call redisClient.setex(`blacklist:${token}`, ttl, "blocked") as before. Ensure you retrieve the same JWT secret/config used elsewhere (e.g., same variable used by your auth middleware), handle verification exceptions (do not write to redis on failure), and preserve the existing TTL calculation using Math.floor(Date.now()/1000).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/src/controllers/userAuth.js`:
- Around line 99-129: The logout flow currently performs
redisClient.setex(jwt.decode(token)) inside the main try and only calls
res.clearCookie and the 200 response after that, so if redis blacklisting fails
the catch returns 500 and the cookie remains; update the controller so
res.clearCookie("token", { httpOnly: true, secure: false, sameSite: "lax", path:
"/" }) always runs regardless of redis outcome—e.g. extract the blacklist logic
into its own try/catch around redisClient.setex (referencing redisClient.setex
and jwt.decode) or move res.clearCookie and the success
res.status(200).json(...) into a finally-like path so failures in redis only log
the error (console.error) but still clear the cookie and return success to the
client.
In `@backend/src/routes/authRoutes.js`:
- Line 9: The logout route is currently protected by the authenticateUser
middleware (authRouter.post("/logout", authenticateUser, logoutUser)), which
prevents requests with expired/invalid tokens from reaching logoutUser to clear
cookies; remove the authenticateUser middleware so the route is registered as
authRouter.post("/logout", logoutUser) and remains callable for cleanup, while
keeping the /me route protected by authenticateUser; ensure logoutUser itself
still clears the auth cookie/session safely and adjust any tests that assumed
middleware enforcement.
In `@frontend/src/context/AuthContext.jsx`:
- Around line 66-74: The logout function currently always clears local state and
swallows server errors; change it so that logout awaits api.post("/auth/logout")
and only calls setUser(null) after a successful response, and in the catch block
do not clear the user but rethrow (or reject) the caught error so callers can
observe backend failures; update the implementation in the logout function that
uses api.post("/auth/logout") and setUser to propagate the error to the caller
instead of silently resolving.
---
Outside diff comments:
In `@frontend/src/context/AuthContext.jsx`:
- Around line 38-63: The catch blocks in register and login currently swallow
backend error details by returning false; update register and login to propagate
error information instead: in the catch handler of both functions (register and
login) capture the caught error, extract meaningful details (e.g.,
error.response?.data or error.message), log them via console.error, and rethrow
the error or return a structured error object so calling components can render
proper feedback while still calling setUser only on success.
---
Nitpick comments:
In `@backend/src/controllers/userAuth.js`:
- Around line 103-107: Replace the unsafe jwt.decode usage with
signature-verified parsing: call jwt.verify(token, <jwtSecret>, {
ignoreExpiration: true }) (catching verification errors) and only read
decoded.exp if verification succeeds; then compute ttl and call
redisClient.setex(`blacklist:${token}`, ttl, "blocked") as before. Ensure you
retrieve the same JWT secret/config used elsewhere (e.g., same variable used by
your auth middleware), handle verification exceptions (do not write to redis on
failure), and preserve the existing TTL calculation using
Math.floor(Date.now()/1000).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 079bd064-6496-4ead-ad6c-b9ffef402248
📒 Files selected for processing (4)
backend/src/controllers/userAuth.jsbackend/src/routes/authRoutes.jsfrontend/src/components/Navbar.jsxfrontend/src/context/AuthContext.jsx
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/src/components/Navbar.jsx`:
- Around line 34-40: The logout button in Navbar.jsx currently passes the logout
function directly to onClick, but since AuthContext.logout() does not handle
navigation, users will remain on the current page after logout. Create a new
async handler function that awaits the logout call and then redirects to either
/signup or /login using your routing solution (e.g., useNavigate from
react-router). Update the button's onClick prop to call this new handler instead
of the logout function directly.
- Line 3: The Navbar.jsx file contains merge artifacts with mismatched or
unclosed JSX tags at lines 43, 116, and 125 that prevent parsing, and references
several undeclared identifiers including useTheme hook, state variables
dropdownRef, isDropdownOpen, setIsDropdownOpen, and utility functions
getFirstLetter, getAvatarColor, and handleLogout. Fix the JSX syntax errors by
closing or balancing all tags at the affected lines, then import or define the
missing hook (useTheme from the appropriate theme library), add useState and
useRef hooks to declare the missing state and ref variables, and define or
import the utility functions getFirstLetter, getAvatarColor, and handleLogout.
Reorganize all imports at the top of the file in a logical order.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6038269f-ef77-4db7-bdf8-3b2d1f7fd835
📒 Files selected for processing (4)
backend/src/controllers/userAuth.jsbackend/src/routes/authRoutes.jsfrontend/src/components/Navbar.jsxfrontend/src/context/AuthContext.jsx
✅ Files skipped from review due to trivial changes (1)
- frontend/src/context/AuthContext.jsx
🚧 Files skipped from review as they are similar to previous changes (2)
- backend/src/routes/authRoutes.js
- backend/src/controllers/userAuth.js
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/src/components/Navbar.jsx`:
- Around line 34-40: The logout button in Navbar.jsx currently passes the logout
function directly to onClick, but since AuthContext.logout() does not handle
navigation, users will remain on the current page after logout. Create a new
async handler function that awaits the logout call and then redirects to either
/signup or /login using your routing solution (e.g., useNavigate from
react-router). Update the button's onClick prop to call this new handler instead
of the logout function directly.
- Line 3: The Navbar.jsx file contains merge artifacts with mismatched or
unclosed JSX tags at lines 43, 116, and 125 that prevent parsing, and references
several undeclared identifiers including useTheme hook, state variables
dropdownRef, isDropdownOpen, setIsDropdownOpen, and utility functions
getFirstLetter, getAvatarColor, and handleLogout. Fix the JSX syntax errors by
closing or balancing all tags at the affected lines, then import or define the
missing hook (useTheme from the appropriate theme library), add useState and
useRef hooks to declare the missing state and ref variables, and define or
import the utility functions getFirstLetter, getAvatarColor, and handleLogout.
Reorganize all imports at the top of the file in a logical order.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6038269f-ef77-4db7-bdf8-3b2d1f7fd835
📒 Files selected for processing (4)
backend/src/controllers/userAuth.jsbackend/src/routes/authRoutes.jsfrontend/src/components/Navbar.jsxfrontend/src/context/AuthContext.jsx
✅ Files skipped from review due to trivial changes (1)
- frontend/src/context/AuthContext.jsx
🚧 Files skipped from review as they are similar to previous changes (2)
- backend/src/routes/authRoutes.js
- backend/src/controllers/userAuth.js
🛑 Comments failed to post (2)
frontend/src/components/Navbar.jsx (2)
3-3:
⚠️ Potential issue | 🔴 Critical🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # Verify unresolved identifiers and parser status for Navbar.jsx rg -n '\b(useTheme|dropdownRef|isDropdownOpen|setIsDropdownOpen|getFirstLetter|getAvatarColor|handleLogout)\b' frontend/src/components/Navbar.jsx rg -n '\b(const|let|function)\s+(dropdownRef|isDropdownOpen|setIsDropdownOpen|getFirstLetter|getAvatarColor|handleLogout)\b' frontend/src/components/Navbar.jsx # If Biome is available in the repo toolchain, this should reproduce/clear the parse error. if command -v biome >/dev/null 2>&1; then biome check frontend/src/components/Navbar.jsx else echo "Run your project's Biome check command for frontend/src/components/Navbar.jsx" fiRepository: niharika-mente/ThinkBoard
Length of output: 3649
Fix merge-artifact JSX structure and undeclared identifiers (build blocker).
Lines 43, 116, and 125 contain JSX parse errors (mismatched/unclosed tags). Additionally, the file references undefined variables:
useTheme()(line 9),dropdownRef,isDropdownOpen,setIsDropdownOpen,getFirstLetter(),getAvatarColor(), andhandleLogout(lines 53–96)—none of which are declared in this file. Import organization also needs correction.Suggested stabilization patch
-import { PlusIcon, Sun, Moon, LogOut } from "lucide-react"; +import { LogOut, PlusIcon } from "lucide-react"; +import { useEffect, useRef, useState } from "react"; import { Link } from "react-router-dom"; import { useAuth } from "../context/AuthContext"; -import { useState, useRef, useEffect } from "react"; const Navbar = () => { - const { theme, toggleTheme } = useTheme(); const { user, logout } = useAuth(); + const [isDropdownOpen, setIsDropdownOpen] = useState(false); + const dropdownRef = useRef(null); + + const getFirstLetter = () => user?.name?.charAt(0) ?? "U"; + const getAvatarColor = () => "from-blue-500 to-purple-500"; + const handleLogout = () => logout();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/components/Navbar.jsx` at line 3, The Navbar.jsx file contains merge artifacts with mismatched or unclosed JSX tags at lines 43, 116, and 125 that prevent parsing, and references several undeclared identifiers including useTheme hook, state variables dropdownRef, isDropdownOpen, setIsDropdownOpen, and utility functions getFirstLetter, getAvatarColor, and handleLogout. Fix the JSX syntax errors by closing or balancing all tags at the affected lines, then import or define the missing hook (useTheme from the appropriate theme library), add useState and useRef hooks to declare the missing state and ref variables, and define or import the utility functions getFirstLetter, getAvatarColor, and handleLogout. Reorganize all imports at the top of the file in a logical order.Source: Linters/SAST tools
34-40:
⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd explicit redirect after successful logout to satisfy PR behavior.
Right now the button only calls
logout; per the implementedAuthContext.logout()contract, that clears user state but does not navigate. Add a local handler that awaits logout and redirects to/signup(or/login) on success.Targeted fix
-import { Link } from "react-router-dom"; +import { Link, useNavigate } from "react-router-dom"; ... const Navbar = () => { const { user, logout } = useAuth(); + const navigate = useNavigate(); + + const handleLogout = async () => { + const ok = await logout(); + if (ok) navigate("/signup"); + }; ... - <button - onClick={logout} + <button + onClick={handleLogout} className="inline-flex items-center gap-2 px-4 py-2 border border-gray-300 dark:border-gray-600 hover:bg-gray-100 dark:hover:bg-gray-850 text-gray-700 dark:text-gray-300 font-medium rounded-lg transition-colors" >🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/components/Navbar.jsx` around lines 34 - 40, The logout button in Navbar.jsx currently passes the logout function directly to onClick, but since AuthContext.logout() does not handle navigation, users will remain on the current page after logout. Create a new async handler function that awaits the logout call and then redirects to either /signup or /login using your routing solution (e.g., useNavigate from react-router). Update the button's onClick prop to call this new handler instead of the logout function directly.
|
Hi @maintainer PR is ready for review! All requested changes have been implemented. |
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
backend/src/controllers/userAuth.js (1)
125-164:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMake logout reachable without valid auth middleware.
backend/src/routes/authRoutes.jsstill wiresPOST /logoutthroughauthenticateUser, so expired/invalid JWT cookies return401before Line 127 can clear the browser cookie. Let the controller clear the cookie unconditionally, and only blacklist when the token verifies.🐛 Proposed route fix
-authRouter.post("/logout", authenticateUser, logoutUser); +authRouter.post("/logout", logoutUser);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/controllers/userAuth.js` around lines 125 - 164, The logoutUser endpoint is unreachable with expired or invalid tokens because the route is still protected by the authenticateUser middleware in authRoutes.js. Remove the authenticateUser middleware from the POST /logout route definition so the logoutUser controller can be invoked without requiring a valid JWT, allowing it to clear the cookie unconditionally. The controller logic already properly handles clearing the cookie first and then attempting optional token blacklisting only if the token can be verified, so no changes are needed to the logoutUser function itself.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/.env`:
- Around line 1-3: The `.env` file contains actual secrets and sensitive
configuration values (including the JWT_SECRET) that should never be committed
to version control. Remove all actual secret values from the committed `.env`
file and replace them with empty placeholders or example values. Create a
`.env.example` file with the same structure but containing only placeholder
values (empty strings or generic examples like those shown in the review
comment). Ensure the `.env` file is added to `.gitignore` so that local
configuration files are never accidentally committed in the future.
Additionally, coordinate with the team to rotate the exposed JWT_SECRET and any
other compromised credentials in your production environment.
In `@backend/src/controllers/userAuth.js`:
- Around line 115-120: The catch block in the login endpoint currently returns a
401 status code for all errors and exposes the actual error message to the
client. Refactor this to differentiate between expected authentication failures
and unexpected errors. Keep the explicit credential validation failures above
this catch block as 401 responses, but for unexpected errors (database errors,
runtime errors, etc.) caught in this catch block, return a 500 status code with
a generic error message instead of exposing err.message to the client. This
prevents leaking sensitive system information while maintaining proper HTTP
status codes for different failure scenarios.
- Around line 140-145: The token blacklist in Redis is not being enforced in the
authentication flow, allowing revoked tokens to remain valid on protected
routes. Additionally, storing raw JWT tokens as Redis keys exposes credentials
in logs. Hash tokens using sha256 when storing the blacklist entry (in the
redisClient.setex call around line 144, replace the raw token with a hashed
version as the key). In the authenticateUser middleware function (around lines
14-17), before calling next(), add a check that queries Redis using the same
hashed token key to verify the token is not blacklisted, and reject the request
if it is found in the blacklist.
---
Duplicate comments:
In `@backend/src/controllers/userAuth.js`:
- Around line 125-164: The logoutUser endpoint is unreachable with expired or
invalid tokens because the route is still protected by the authenticateUser
middleware in authRoutes.js. Remove the authenticateUser middleware from the
POST /logout route definition so the logoutUser controller can be invoked
without requiring a valid JWT, allowing it to clear the cookie unconditionally.
The controller logic already properly handles clearing the cookie first and then
attempting optional token blacklisting only if the token can be verified, so no
changes are needed to the logoutUser function itself.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 03319916-bd60-4687-afa4-f07e7d46ee30
📒 Files selected for processing (6)
backend/.envbackend/src/config/db.jsbackend/src/controllers/userAuth.jsbackend/src/routes/authRoutes.jsfrontend/src/components/Navbar.jsxfrontend/src/lib/axios.js
✅ Files skipped from review due to trivial changes (2)
- frontend/src/lib/axios.js
- backend/src/config/db.js
🚧 Files skipped from review as they are similar to previous changes (2)
- backend/src/routes/authRoutes.js
- frontend/src/components/Navbar.jsx
| MONGODB_URI=mongodb://localhost:27017/thinkboard | ||
| JWT_SECRET=my_super_secret_key_12345 | ||
| PORT=5000 No newline at end of file |
There was a problem hiding this comment.
Remove the committed .env secrets/config from the PR.
Line 2 exposes the JWT signing key used to sign and verify auth tokens; if this file is reused outside local dev, anyone with repo access can forge sessions. Keep only placeholders in a committed .env.example, add/keep .env ignored, and rotate this secret.
🛡️ Proposed cleanup
-MONGODB_URI=mongodb://localhost:27017/thinkboard
-JWT_SECRET=my_super_secret_key_12345
-PORT=5000Example committed template instead:
MONGODB_URI=
JWT_SECRET=
PORT=5000
UPSTASH_REDIS_REST_URL=
UPSTASH_REDIS_REST_TOKEN=🧰 Tools
🪛 dotenv-linter (4.0.0)
[warning] 2-2: [UnorderedKey] The JWT_SECRET key should go before the MONGODB_URI key
(UnorderedKey)
[warning] 3-3: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/.env` around lines 1 - 3, The `.env` file contains actual secrets and
sensitive configuration values (including the JWT_SECRET) that should never be
committed to version control. Remove all actual secret values from the committed
`.env` file and replace them with empty placeholders or example values. Create a
`.env.example` file with the same structure but containing only placeholder
values (empty strings or generic examples like those shown in the review
comment). Ensure the `.env` file is added to `.gitignore` so that local
configuration files are never accidentally committed in the future.
Additionally, coordinate with the team to rotate the exposed JWT_SECRET and any
other compromised credentials in your production environment.
| } catch (err) { | ||
| res.status(401).json({ error: err.message }); | ||
| console.error("❌ Login error:", err); | ||
| res.status(401).json({ | ||
| success: false, | ||
| error: err.message, | ||
| }); |
There was a problem hiding this comment.
Return a generic 500 for unexpected login failures.
This catch currently reports every database/runtime failure as 401 and sends err.message to the client. Keep the explicit credential failures above as 401, but make unexpected errors generic.
🛡️ Proposed fix
} catch (err) {
console.error("❌ Login error:", err);
- res.status(401).json({
+ res.status(500).json({
success: false,
- error: err.message,
+ error: "Server error",
});
}📝 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.
| } catch (err) { | |
| res.status(401).json({ error: err.message }); | |
| console.error("❌ Login error:", err); | |
| res.status(401).json({ | |
| success: false, | |
| error: err.message, | |
| }); | |
| } catch (err) { | |
| console.error("❌ Login error:", err); | |
| res.status(500).json({ | |
| success: false, | |
| error: "Server error", | |
| }); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/src/controllers/userAuth.js` around lines 115 - 120, The catch block
in the login endpoint currently returns a 401 status code for all errors and
exposes the actual error message to the client. Refactor this to differentiate
between expected authentication failures and unexpected errors. Keep the
explicit credential validation failures above this catch block as 401 responses,
but for unexpected errors (database errors, runtime errors, etc.) caught in this
catch block, return a 500 status code with a generic error message instead of
exposing err.message to the client. This prevents leaking sensitive system
information while maintaining proper HTTP status codes for different failure
scenarios.
| const decoded = jwt.decode(token); | ||
| if (decoded && decoded.exp) { | ||
| const ttl = decoded.exp - Math.floor(Date.now() / 1000); | ||
| if (ttl > 0 && redisClient && typeof redisClient.setex === 'function') { | ||
| await redisClient.setex(`blacklist:${token}`, ttl, "blocked"); | ||
| console.log("✅ Token blacklisted in Redis"); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Confirm blacklist writes and reads use the same non-raw token key.
# Expect: authMiddleware should contain a Redis get/exists check for the same hashed blacklist key before next().
rg -n -C3 'blacklist:|redisClient\.(get|exists|setex)|jwt\.(verify|decode)' \
backend/src/controllers/userAuth.js \
backend/src/middleware/authMiddleware.jsRepository: niharika-mente/ThinkBoard
Length of output: 1437
🏁 Script executed:
# Get full context of authMiddleware to see if blacklist is checked anywhere
cat -n backend/src/middleware/authMiddleware.jsRepository: niharika-mente/ThinkBoard
Length of output: 1073
🏁 Script executed:
# Search entire codebase for blacklist checks to see if it's enforced anywhere
rg -n "blacklist" backend/src/Repository: niharika-mente/ThinkBoard
Length of output: 639
Hash and enforce blacklist entries in the authentication middleware.
The blacklist is written to Redis but never enforced. Line 144 stores the raw JWT token as the Redis key, and the authenticateUser middleware (lines 14–17) verifies the JWT signature but skips any blacklist check before calling next(). This allows revoked tokens to remain valid on all protected routes.
Additionally, storing the raw token in Redis as a key exposes the bearer credential in Redis logs and monitoring tools.
Required changes:
- Hash tokens when storing in Redis: use
sha256(token)as the blacklist key - Add a blacklist check in
authenticateUsermiddleware beforenext()using the same hashed key
Proposed implementation
In userAuth.js:
+import crypto from "crypto";
+
+const getBlacklistKey = (token) =>
+ `blacklist:${crypto.createHash("sha256").update(token).digest("hex")}`;
+
...
const ttl = decoded.exp - Math.floor(Date.now() / 1000);
if (ttl > 0 && redisClient && typeof redisClient.setex === 'function') {
- await redisClient.setex(`blacklist:${token}`, ttl, "blocked");
+ await redisClient.setex(getBlacklistKey(token), ttl, "blocked");
console.log("✅ Token blacklisted in Redis");
}In authMiddleware.js, before next():
const decoded = jwt.verify(token, process.env.JWT_SECRET);
+ const blacklistKey = `blacklist:${crypto.createHash("sha256").update(token).digest("hex")}`;
+ const isBlacklisted = await redisClient.exists(blacklistKey);
+ if (isBlacklisted) {
+ return res.status(401).json({ success: false, error: "Token has been revoked." });
+ }
req.user = decoded;📝 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.
| const decoded = jwt.decode(token); | |
| if (decoded && decoded.exp) { | |
| const ttl = decoded.exp - Math.floor(Date.now() / 1000); | |
| if (ttl > 0 && redisClient && typeof redisClient.setex === 'function') { | |
| await redisClient.setex(`blacklist:${token}`, ttl, "blocked"); | |
| console.log("✅ Token blacklisted in Redis"); | |
| import crypto from "crypto"; | |
| const getBlacklistKey = (token) => | |
| `blacklist:${crypto.createHash("sha256").update(token).digest("hex")}`; | |
| // ... other code ... | |
| export const logoutUser = async (req, res) => { | |
| const token = req.cookies?.token; | |
| try { | |
| if (token) { | |
| const decoded = jwt.decode(token); | |
| if (decoded && decoded.exp) { | |
| const ttl = decoded.exp - Math.floor(Date.now() / 1000); | |
| if (ttl > 0 && redisClient && typeof redisClient.setex === 'function') { | |
| await redisClient.setex(getBlacklistKey(token), ttl, "blocked"); | |
| console.log("✅ Token blacklisted in Redis"); | |
| } | |
| } | |
| } | |
| } catch (error) { | |
| console.error("Logout blacklist error:", error); | |
| } | |
| res.clearCookie("token", { | |
| httpOnly: true, | |
| secure: false, | |
| sameSite: "lax", | |
| path: "/", | |
| }); | |
| return res.status(200).json({ | |
| success: true, | |
| message: "Logged out successfully", | |
| }); | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/src/controllers/userAuth.js` around lines 140 - 145, The token
blacklist in Redis is not being enforced in the authentication flow, allowing
revoked tokens to remain valid on protected routes. Additionally, storing raw
JWT tokens as Redis keys exposes credentials in logs. Hash tokens using sha256
when storing the blacklist entry (in the redisClient.setex call around line 144,
replace the raw token with a hashed version as the key). In the authenticateUser
middleware function (around lines 14-17), before calling next(), add a check
that queries Redis using the same hashed token key to verify the token is not
blacklisted, and reject the request if it is found in the blacklist.
|
Hey @zaibamachhaliya, thanks for the updates! The redirect on the frontend looks great, and the UI changes are really coming together. However, we cannot merge this just yet because of a critical security leak and a few lingering logic bugs caught by the review bots. Please address these blockers: Critical Security Blocker .env Committed: You accidentally committed the backend/.env file containing our actual JWT_SECRET and MONGODB_URI. Please remove this file from Git tracking (git rm --cached backend/.env), ensure .env is in your .gitignore, and create a .env.example with dummy values instead. Backend Logic Bugs Logout Route Middleware (authRoutes.js): The /logout route is still protected by authenticateUser. Please change this to authRouter.post("/logout", logoutUser);. If we require a valid token to log out, users with expired tokens will get a 401 and won't be able to clear their cookies. Useless Redis Blacklist (authMiddleware.js): You did a great job writing to the Upstash Redis blacklist on logout! However, you didn't add a check for it in authenticateUser. Right now, a blacklisted token will still be accepted. Please add a check in authMiddleware.js to reject the request if the token exists in Redis. (Also, it's best practice to hash the token before storing it in Redis so the raw credential isn't sitting in our database logs). Error Leaking (userAuth.js): In the login controller's catch block, unexpected server errors are returning a 401 with the raw err.message. Please change this to return a generic 500 status with a message like "Server error" so we don't leak database details to the client. Once these last backend/security hurdles are cleared, this will be ready for the main branch! Let me know if you need any help implementing the Redis check in the middleware. |
📌 Issue Reference
Fixes #(issue-number-jo-aapko assign hua tha)
🚀 Feature Description
Currently, once logged in, there is no way to log out. This PR adds complete logout functionality.
✅ Tasks Completed
POST /api/auth/logout)🛠️ Technical Implementation
Backend Changes
logoutUsercontroller inuserAuth.jsPOST /api/auth/logoutFrontend Changes
Navbar.jsxwith DaisyUI stylinglogoutfunction inAuthContext.jsx/signup📸 Before & After
Before: No logout button
After: Logout button next to New Note button
🧪 Testing Performed
📁 Files Changed
backend/src/controllers/userAuth.js
backend/src/routes/authRoutes.js
frontend/src/components/Navbar.jsx
frontend/src/context/AuthContext.jsx
closes #4
Summary by CodeRabbit