From c3c0c33339c2029d08fdb2f09698e742c38e3cbc Mon Sep 17 00:00:00 2001 From: Nico Date: Wed, 18 Feb 2026 12:24:15 -0800 Subject: [PATCH] fix: harden auth inputs, throttling, and debug exposure --- backend/app.js | 6 ++- backend/controllers/auth.controller.js | 38 +++++++++++++--- backend/controllers/users.controller.js | 13 +++--- backend/middleware/auth.js | 8 +++- backend/middleware/rate-limit.js | 58 +++++++++++++++++++++++++ backend/routes/auth.routes.js | 21 +++++++-- backend/routes/users.routes.js | 24 +++++++--- backend/utils/cookies.js | 7 ++- 8 files changed, 147 insertions(+), 28 deletions(-) create mode 100644 backend/middleware/rate-limit.js diff --git a/backend/app.js b/backend/app.js index fa41196..a1c7fb9 100644 --- a/backend/app.js +++ b/backend/app.js @@ -9,8 +9,10 @@ const app = express(); app.use(requestIdMiddleware); app.use(express.json()); -// Serve static files from public directory -app.use('/test', express.static(path.join(__dirname, 'public'))); +// Expose manual API test pages in non-production environments only. +if (process.env.NODE_ENV !== "production") { + app.use("/test", express.static(path.join(__dirname, "public"))); +} const allowedOrigins = (process.env.ALLOWED_ORIGINS || "") .split(",") diff --git a/backend/controllers/auth.controller.js b/backend/controllers/auth.controller.js index 3b809a9..0226ef6 100644 --- a/backend/controllers/auth.controller.js +++ b/backend/controllers/auth.controller.js @@ -9,13 +9,26 @@ const { logError } = require("../utils/logger"); exports.register = async (req, res) => { let { username, password, name } = req.body; + + if ( + !username || + !password || + !name || + typeof username !== "string" || + typeof password !== "string" || + typeof name !== "string" + ) { + return sendError(res, 400, "Username, password, and name are required"); + } + username = username.toLowerCase(); - console.log(`Registration attempt for ${name} => username:${username}`); + if (password.length < 8) { + return sendError(res, 400, "Password must be at least 8 characters"); + } try { const hash = await bcrypt.hash(password, 10); const user = await User.createUser(username, hash, name); - console.log(`User registered: ${username}`); res.json({ message: "User registered", user }); } catch (err) { @@ -27,22 +40,35 @@ exports.register = async (req, res) => { exports.login = async (req, res) => { let { username, password } = req.body; + if ( + !username || + !password || + typeof username !== "string" || + typeof password !== "string" + ) { + return sendError(res, 400, "Username and password are required"); + } + username = username.toLowerCase(); const user = await User.findByUsername(username); if (!user) { - console.log(`Login attempt with unknown user: ${username}`); - return sendError(res, 401, "User not found"); + return sendError(res, 401, "Invalid credentials"); } const valid = await bcrypt.compare(password, user.password); if (!valid) { - console.log(`Invalid login attempt for user ${username}`); return sendError(res, 401, "Invalid credentials"); } + const jwtSecret = process.env.JWT_SECRET; + if (!jwtSecret) { + logError(req, "auth.login.jwtSecretMissing", new Error("JWT_SECRET is not configured")); + return sendError(res, 500, "Authentication is unavailable"); + } + const token = jwt.sign( { id: user.id, role: user.role }, - process.env.JWT_SECRET, + jwtSecret, { expiresIn: "1 year" } ); diff --git a/backend/controllers/users.controller.js b/backend/controllers/users.controller.js index e39452e..16a4093 100644 --- a/backend/controllers/users.controller.js +++ b/backend/controllers/users.controller.js @@ -3,10 +3,9 @@ const bcrypt = require("bcryptjs"); const { sendError } = require("../utils/http"); const { logError } = require("../utils/logger"); -exports.test = async (req, res) => { - console.log("User route is working"); - res.json({ message: "User route is working" }); -}; +exports.test = async (req, res) => { + res.json({ message: "User route is working" }); +}; exports.getAllUsers = async (req, res) => { const users = await User.getAllUsers(); @@ -15,10 +14,8 @@ exports.getAllUsers = async (req, res) => { exports.updateUserRole = async (req, res) => { - try { - const { id, role } = req.body; - - console.log(`Updating user ${id} to role ${role}`); + try { + const { id, role } = req.body; if (!Object.values(User.ROLES).includes(role)) return sendError(res, 400, "Invalid role"); diff --git a/backend/middleware/auth.js b/backend/middleware/auth.js index 9702cba..08e1ae6 100644 --- a/backend/middleware/auth.js +++ b/backend/middleware/auth.js @@ -10,8 +10,14 @@ async function auth(req, res, next) { const token = header.startsWith("Bearer ") ? header.slice(7).trim() : null; if (token) { + const jwtSecret = process.env.JWT_SECRET; + if (!jwtSecret) { + logError(req, "middleware.auth.jwtSecretMissing", new Error("JWT_SECRET is not configured")); + return sendError(res, 500, "Authentication is unavailable"); + } + try { - const decoded = jwt.verify(token, process.env.JWT_SECRET); + const decoded = jwt.verify(token, jwtSecret); req.user = decoded; // id + role return next(); } catch (err) { diff --git a/backend/middleware/rate-limit.js b/backend/middleware/rate-limit.js new file mode 100644 index 0000000..a90133c --- /dev/null +++ b/backend/middleware/rate-limit.js @@ -0,0 +1,58 @@ +const { sendError } = require("../utils/http"); + +const buckets = new Map(); + +function pruneExpired(now) { + for (const [key, value] of buckets.entries()) { + if (value.resetAt <= now) { + buckets.delete(key); + } + } +} + +function getClientIp(req) { + const forwardedFor = req.headers["x-forwarded-for"]; + if (typeof forwardedFor === "string" && forwardedFor.trim()) { + return forwardedFor.split(",")[0].trim(); + } + return req.ip || req.socket?.remoteAddress || "unknown"; +} + +function createRateLimit({ keyPrefix, windowMs, max, message }) { + return (req, res, next) => { + const now = Date.now(); + + if (buckets.size > 5000) { + pruneExpired(now); + } + + const key = `${keyPrefix}:${getClientIp(req)}`; + const existing = buckets.get(key); + const bucket = + !existing || existing.resetAt <= now + ? { count: 0, resetAt: now + windowMs } + : existing; + + bucket.count += 1; + buckets.set(key, bucket); + + if (bucket.count > max) { + const retryAfterSeconds = Math.max( + 1, + Math.ceil((bucket.resetAt - now) / 1000) + ); + res.setHeader("Retry-After", String(retryAfterSeconds)); + return sendError( + res, + 429, + message || "Too many requests. Please try again later." + ); + } + + return next(); + }; +} + +module.exports = { + createRateLimit, +}; diff --git a/backend/routes/auth.routes.js b/backend/routes/auth.routes.js index 5c87ebb..dcf2300 100644 --- a/backend/routes/auth.routes.js +++ b/backend/routes/auth.routes.js @@ -1,9 +1,24 @@ const router = require("express").Router(); const controller = require("../controllers/auth.controller"); const User = require("../models/user.model"); - -router.post("/register", controller.register); -router.post("/login", controller.login); +const { createRateLimit } = require("../middleware/rate-limit"); + +const loginRateLimit = createRateLimit({ + keyPrefix: "auth:login", + windowMs: 15 * 60 * 1000, + max: 25, + message: "Too many login attempts. Please try again later.", +}); + +const registerRateLimit = createRateLimit({ + keyPrefix: "auth:register", + windowMs: 15 * 60 * 1000, + max: 10, + message: "Too many registration attempts. Please try again later.", +}); + +router.post("/register", registerRateLimit, controller.register); +router.post("/login", loginRateLimit, controller.login); router.post("/logout", controller.logout); router.post("/", async (req, res) => { res.status(200).json({ diff --git a/backend/routes/users.routes.js b/backend/routes/users.routes.js index 2a8796b..e3d8d14 100644 --- a/backend/routes/users.routes.js +++ b/backend/routes/users.routes.js @@ -1,11 +1,21 @@ const router = require("express").Router(); -const auth = require("../middleware/auth"); -const requireRole = require("../middleware/rbac"); -const usersController = require("../controllers/users.controller"); -const { ROLES } = require("../models/user.model"); - -router.get("/exists", usersController.checkIfUserExists); -router.get("/test", usersController.test); +const auth = require("../middleware/auth"); +const requireRole = require("../middleware/rbac"); +const usersController = require("../controllers/users.controller"); +const { ROLES } = require("../models/user.model"); +const { createRateLimit } = require("../middleware/rate-limit"); + +const userExistsRateLimit = createRateLimit({ + keyPrefix: "users:exists", + windowMs: 15 * 60 * 1000, + max: 60, + message: "Too many availability checks. Please try again later.", +}); + +router.get("/exists", userExistsRateLimit, usersController.checkIfUserExists); +if (process.env.NODE_ENV !== "production") { + router.get("/test", usersController.test); +} // Current user profile routes (authenticated) router.get("/me", auth, usersController.getCurrentUser); diff --git a/backend/utils/cookies.js b/backend/utils/cookies.js index 7b0469a..359954f 100644 --- a/backend/utils/cookies.js +++ b/backend/utils/cookies.js @@ -9,7 +9,12 @@ function parseCookieHeader(cookieHeader) { const key = segment.slice(0, index).trim(); const value = segment.slice(index + 1).trim(); if (!key) continue; - cookies[key] = decodeURIComponent(value); + try { + cookies[key] = decodeURIComponent(value); + } catch (_) { + // Ignore malformed cookie values instead of throwing. + continue; + } } return cookies;