From 661da6969d47d25ceb3c0b63684c079a2eb805c8 Mon Sep 17 00:00:00 2001 From: Peleg <106535297+peleg-development@users.noreply.github.com> Date: Tue, 9 Jun 2026 21:27:11 +0300 Subject: [PATCH] fix: address critical security vulnerabilities - Remove role from registration request body (privilege escalation) - Add authentication to user listing routes - Escape regex special characters in query filters (ReDoS) - Enable helmet middleware for security headers - Restrict user role field to enum values - Remove console.log leaking request body with passwords - Remove role from login response --- controllers/authController.js | 7 +------ controllers/productController.js | 11 +++-------- controllers/userController.js | 18 +++--------------- models/User.js | 4 ++-- routes/userRoutes.js | 7 ++++--- server.js | 1 + 6 files changed, 14 insertions(+), 34 deletions(-) diff --git a/controllers/authController.js b/controllers/authController.js index f81585d..014a61e 100644 --- a/controllers/authController.js +++ b/controllers/authController.js @@ -2,10 +2,8 @@ import User from "../models/User.js"; import jwt from "jsonwebtoken"; export const registerUser = async (req, res) => { - console.log(">>>>Register user function callled",req.body); - try { - const { username, email, password, role, age } = req.body; + const { username, email, password, age } = req.body; if (!username || !email || !password) { return res @@ -18,12 +16,10 @@ export const registerUser = async (req, res) => { return res.status(400).json({ message: "User already exists" }); } - // Include optional fields role and age const user = await User.create({ username, email, password, - role: role || "user", age: age || null, }); @@ -70,7 +66,6 @@ export const loginUser = async (req, res) => { username: user.username, email, token, - role: user.role, }); } catch (error) { console.log(error); diff --git a/controllers/productController.js b/controllers/productController.js index 71ed0d7..0315335 100644 --- a/controllers/productController.js +++ b/controllers/productController.js @@ -12,7 +12,6 @@ export const addProduct = async (req, res, next) => { publisher, isbn, stock, - // createdBy: req.user._id }); res.status(201).json(product); } catch (err) { @@ -24,16 +23,13 @@ export const addProduct = async (req, res, next) => { export const getProducts = async (req, res, next) => { try { const query = {}; - // console.log(">>>>>req.query", req.query.category); - // Category filter (case-insensitive) if (req.query.category) { query.category = { - $regex: new RegExp(`^${req.query.category.trim()}$`, "i"), + $regex: new RegExp(`^${req.query.category.trim().replace(/[.*+?^${}()|[\]\]/g, '\$&')}$`, "i"), }; } - // Price range filter if (req.query.minPrice || req.query.maxPrice) { query.price = {}; if (req.query.minPrice) query.price.$gte = Number(req.query.minPrice); @@ -75,7 +71,6 @@ export const updateProduct = async (req, res, next) => { // Delete product export const deleteProduct = async (req, res, next) => { - console.log(">>>>>delet product function called") try { const product = await Product.findById(req.params.id); if (!product) { @@ -83,9 +78,9 @@ export const deleteProduct = async (req, res, next) => { return next(new Error('Product not found')); } - await product.deleteOne(); // ✅ updated from remove() + await product.deleteOne(); res.json({ message: 'Product deleted' }); } catch (err) { next(err); } -}; \ No newline at end of file +}; diff --git a/controllers/userController.js b/controllers/userController.js index 67068e7..07cb415 100644 --- a/controllers/userController.js +++ b/controllers/userController.js @@ -3,23 +3,17 @@ import User from "../models/User.js"; export const getallUsers = async (req, res, next) => { - // console.log(">>>>req.headers",req.headers) - console.log(">>>>>>function called") try { const query = {}; - // Age filter (convert to Number) if (req.query.age) { query.age = Number(req.query.age); } - // Role filter (case-insensitive & trim spaces) if (req.query.role) { - query.role = { $regex: new RegExp(`^${req.query.role.trim()}$`, 'i') }; + query.role = { $regex: new RegExp(`^${req.query.role.trim().replace(/[.*+?^${}()|[\]\]/g, '\$&')}$`, 'i') }; } - console.log("Final Query:", query); // Debugging - const users = await User.find(query).select('-password'); res.json(users); } catch (err) { @@ -29,26 +23,20 @@ export const getallUsers = async (req, res, next) => { export const getUserByIdPublic = async (req, res, next) => { try { - console.log(req) - console.log(req.params) const userId = req.params.id.trim(); - // 1️⃣ Validate ID format if (!mongoose.Types.ObjectId.isValid(userId)) { return res.status(400).json({ message: 'Invalid user ID format' }); } - // 2️⃣ Find user, exclude password const user = await User.findById(userId).select('-password'); - // 3️⃣ Handle not found if (!user) { return res.status(404).json({ message: `User with ID ${userId} not found` }); } - // 4️⃣ Return user res.status(200).json(user); } catch (err) { - next(err); // Pass errors to your global error handler + next(err); } -}; \ No newline at end of file +}; diff --git a/models/User.js b/models/User.js index 04804bd..8e35e27 100644 --- a/models/User.js +++ b/models/User.js @@ -6,8 +6,8 @@ const userSchema = new mongoose.Schema( username: { type: String, required: true }, email: { type: String, required: true, unique: true }, password: { type: String, required: true }, - role: { type: String, default: "user" }, - age: { type: Number }, // Added age + role: { type: String, default: "user", enum: ["user", "admin"] }, + age: { type: Number }, }, { timestamps: true } ); diff --git a/routes/userRoutes.js b/routes/userRoutes.js index 201800b..3ce5dd6 100644 --- a/routes/userRoutes.js +++ b/routes/userRoutes.js @@ -1,11 +1,12 @@ import { Router } from "express"; import { getallUsers, getUserByIdPublic } from "../controllers/userController.js"; +import { verifyUser } from "../middleware/authMiddleware.js"; const userRoutes = Router({ mergeParams: true }); -userRoutes.get("/",getallUsers) +userRoutes.get("/", verifyUser, getallUsers) -userRoutes.get("/:id", getUserByIdPublic); +userRoutes.get("/:id", verifyUser, getUserByIdPublic); -export default userRoutes; \ No newline at end of file +export default userRoutes; diff --git a/server.js b/server.js index 5306740..7754019 100644 --- a/server.js +++ b/server.js @@ -17,6 +17,7 @@ const app = express(); const PORT = process.env.PORT || 3000; +app.use(helmet()); app.use(express.json({ limit: "30mb", extented: true })); app.use(express.urlencoded({ limit: "30mb", extented: true }));