Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions backend/.env
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
MONGODB_URI=mongodb://localhost:27017/thinkboard
JWT_SECRET=my_super_secret_key_12345
PORT=5000
Comment on lines +1 to +3

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

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=5000

Example 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.

13 changes: 8 additions & 5 deletions backend/src/config/db.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
import mongoose from "mongoose";
import dotenv from "dotenv";


dotenv.config();

export const connectDB = async () => {
try {
await mongoose.connect(process.env.MONGO_URI);

console.log("MongoDB connected successfully");
await mongoose.connect(process.env.MONGODB_URI);
console.log("✅ MongoDB connected successfully");
} catch (error) {
console.error("Error connecting to MongoDB:", error);
process.exit(1);//exit with failure
process.exit(1);
}
};
};
116 changes: 88 additions & 28 deletions backend/src/controllers/userAuth.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,23 @@ import User from "../models/User.js";
import Validate from "../Utils/Validetor.js";
import bcrypt from "bcryptjs";
import jwt from "jsonwebtoken";
import redisClient from '../config/redis.js';

// ==================== REGISTER ====================
export const register = async (req, res) => {
try {
Validate(req.body);
const { name, email, password } = req.body;

// Check if user already exists
const existingUser = await User.findOne({ email });
if (existingUser) {
return res.status(400).json({
success: false,
error: "User already exists with this email",
});
}

const hashedPassword = await bcrypt.hash(password, 10);

const user = await User.create({
Expand All @@ -26,6 +36,8 @@ export const register = async (req, res) => {
res.cookie("token", token, {
maxAge: 60 * 60 * 1000,
httpOnly: true,
secure: process.env.NODE_ENV === "production",
sameSite: "lax",
path: "/",
});

Expand All @@ -39,7 +51,7 @@ export const register = async (req, res) => {
},
});
} catch (err) {
console.log("FULL ERROR:", err);
console.error("❌ Register error:", err);
res.status(400).json({
success: false,
error: err.message,
Expand All @@ -53,17 +65,28 @@ export const login = async (req, res) => {
const { email, password } = req.body;

if (!email || !password) {
throw new Error("Invalid Credentials");
return res.status(401).json({
success: false,
error: "Email and password are required",
});
}

const user = await User.findOne({ email });

if (!user) throw new Error("User not found");
if (!user) {
return res.status(401).json({
success: false,
error: "Invalid credentials",
});
}

const match = await bcrypt.compare(password, user.password);

if (!match) {
throw new Error("Invalid Credentials");
return res.status(401).json({
success: false,
error: "Invalid credentials",
});
}

const token = jwt.sign(
Expand All @@ -75,6 +98,8 @@ export const login = async (req, res) => {
res.cookie("token", token, {
maxAge: 60 * 60 * 1000,
httpOnly: true,
secure: process.env.NODE_ENV === "production",
sameSite: "lax",
path: "/",
});

Expand All @@ -88,19 +113,69 @@ export const login = async (req, res) => {
},
});
} catch (err) {
res.status(401).json({ error: err.message });
console.error("❌ Login error:", err);
res.status(401).json({
success: false,
error: err.message,
});
Comment on lines 115 to +120

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

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.

Suggested change
} 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.

}
};

// ==================== LOGOUT (Fast - Redis optional) ====================
export const logoutUser = async (req, res) => {
// ✅ First: Clear cookie immediately (fast)
res.clearCookie("token", {
httpOnly: true,
secure: process.env.NODE_ENV === "production",
sameSite: "lax",
path: "/",
});

// ✅ Second: Try Redis blacklist in background (don't await, let it run async)
const token = req.cookies?.token;
if (token) {
// Fire and forget - don't await, don't block response
const blacklistToken = async () => {
try {
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");
Comment on lines +140 to +145

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 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.js

Repository: 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.js

Repository: 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:

  1. Hash tokens when storing in Redis: use sha256(token) as the blacklist key
  2. Add a blacklist check in authenticateUser middleware before next() 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.

Suggested change
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.

}
}
} catch (error) {
// Silent fail - don't log every error
if (process.env.NODE_ENV === "development") {
console.warn("⚠️ Redis blacklist skipped");
}
}
};

// Execute without blocking response
blacklistToken();
}

// ✅ Send response immediately
res.status(200).json({
success: true,
message: "Logged out successfully",
});
};

// // ==================== GET CURRENT USER ====================
// ==================== GET CURRENT USER ====================
export const getCurrentUser = async (req, res) => {
try {
const userId = req.user._id;
const userId = req.user._id || req.user.id;

const user = await User.findById(userId).select("-password");

if (!user) {
return res.status(404).json({ error: "User not found" });
return res.status(404).json({
success: false,
error: "User not found",
});
}

res.json({
Expand All @@ -113,25 +188,10 @@ export const getCurrentUser = async (req, res) => {
},
});
} catch (error) {
console.error("Get current user error:", error);
res.status(500).json({ error: "Server error" });
}
};

// ==================== LOGOUT ====================
export const logout = async (req, res) => {
try {
res.clearCookie("token", {
httpOnly: true,
path: "/",
});
res.status(200).json({
success: true,
message: "Logged out successfully",
console.error("❌ Get current user error:", error);
res.status(500).json({
success: false,
error: "Server error",
});
} catch (error) {
console.error("Logout error:", error);
res.status(500).json({ success: false, error: "Server error during logout" });
}
};

};
10 changes: 4 additions & 6 deletions backend/src/routes/authRoutes.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
import express from "express";
const authRouter = express.Router();
import { register,login,getCurrentUser, logout } from "../controllers/userAuth.js";
import { register, login, getCurrentUser, logoutUser } from "../controllers/userAuth.js";
import { authenticateUser } from "../middleware/authMiddleware.js";


authRouter.post("/register", register);
authRouter.post("/login", login);
authRouter.get("/me", authenticateUser, getCurrentUser);
authRouter.post("/logout", logout);

authRouter.post("/logout", authenticateUser, logoutUser);
authRouter.get("/me", authenticateUser, getCurrentUser);

export default authRouter;
export default authRouter;
Loading