diff --git a/backend/package-lock.json b/backend/package-lock.json index 5228cf1..9d490ad 100644 --- a/backend/package-lock.json +++ b/backend/package-lock.json @@ -16,7 +16,8 @@ "dotenv": "^17.2.3", "express": "^5.1.0", "express-rate-limit": "^8.2.0", - "express-validator": "^7.2.1", + "express-validator": "^7.3.0", + "helmet": "^8.1.0", "jsonwebtoken": "^9.0.2", "mongoose": "^8.18.3", "socket.io": "^4.8.1" @@ -3290,13 +3291,13 @@ } }, "node_modules/express-validator": { - "version": "7.2.1", - "resolved": "https://registry.npmjs.org/express-validator/-/express-validator-7.2.1.tgz", - "integrity": "sha512-CjNE6aakfpuwGaHQZ3m8ltCG2Qvivd7RHtVMS/6nVxOM7xVGqr4bhflsm4+N5FP5zI7Zxp+Hae+9RE+o8e3ZOQ==", + "version": "7.3.0", + "resolved": "https://registry.npmjs.org/express-validator/-/express-validator-7.3.0.tgz", + "integrity": "sha512-ujK2BX5JUun5NR4JuBo83YSXoDDIpoGz3QxgHTzQcHFevkKnwV1in4K7YNuuXQ1W3a2ObXB/P4OTnTZpUyGWiw==", "license": "MIT", "dependencies": { "lodash": "^4.17.21", - "validator": "~13.12.0" + "validator": "~13.15.15" }, "engines": { "node": ">= 8.0.0" @@ -3785,6 +3786,15 @@ "node": ">= 0.4" } }, + "node_modules/helmet": { + "version": "8.1.0", + "resolved": "https://registry.npmjs.org/helmet/-/helmet-8.1.0.tgz", + "integrity": "sha512-jOiHyAZsmnr8LqoPGmCjYAaiuWwjAPLgY8ZX2XrmHawt99/u1y6RgrZMTeoPfpUbV96HOalYgz1qzkRbw54Pmg==", + "license": "MIT", + "engines": { + "node": ">=18.0.0" + } + }, "node_modules/html-escaper": { "version": "2.0.2", "resolved": "https://registry.npmjs.org/html-escaper/-/html-escaper-2.0.2.tgz", @@ -7036,9 +7046,9 @@ } }, "node_modules/validator": { - "version": "13.12.0", - "resolved": "https://registry.npmjs.org/validator/-/validator-13.12.0.tgz", - "integrity": "sha512-c1Q0mCiPlgdTVVVIJIrBuxNicYE+t/7oKeI9MWLj3fh/uq2Pxh/3eeWbVZ4OcGW1TUf53At0njHw5SMdA3tmMg==", + "version": "13.15.20", + "resolved": "https://registry.npmjs.org/validator/-/validator-13.15.20.tgz", + "integrity": "sha512-KxPOq3V2LmfQPP4eqf3Mq/zrT0Dqp2Vmx2Bn285LwVahLc+CsxOM0crBHczm8ijlcjZ0Q5Xd6LW3z3odTPnlrw==", "license": "MIT", "engines": { "node": ">= 0.10" diff --git a/backend/package.json b/backend/package.json index d00b608..eb5180a 100644 --- a/backend/package.json +++ b/backend/package.json @@ -24,7 +24,8 @@ "dotenv": "^17.2.3", "express": "^5.1.0", "express-rate-limit": "^8.2.0", - "express-validator": "^7.2.1", + "express-validator": "^7.3.0", + "helmet": "^8.1.0", "jsonwebtoken": "^9.0.2", "mongoose": "^8.18.3", "socket.io": "^4.8.1" diff --git a/backend/src/app.ts b/backend/src/app.ts index ce34cb7..abdef4d 100644 --- a/backend/src/app.ts +++ b/backend/src/app.ts @@ -1,6 +1,14 @@ import express from "express"; import { createServer } from "http"; import cors from "cors"; +import { + corsConfig, + helmetConfig, + generalLimiter, + speedLimiter, + securityHeaders, + requestSizeLimiter +} from "./middleware/security"; import healthRoutes from "./routes/health"; import authRoutes from "./routes/auth"; import projectRoutes from "./routes/project"; @@ -18,18 +26,19 @@ export function createApp() { // Initialize Socket.IO service const socketService = new SocketService(server); - // Middleware - app.use(cors({ - origin: [ - "http://localhost:5173", - "http://localhost:5174", - "http://127.0.0.1:5173", - "http://127.0.0.1:5174", - "https://collab-code-review-5gi4vw7jd-ram-prasads-projects-12031425.vercel.app" - ], - credentials: true - })); - app.use(express.json()); + // Security Middleware (order matters!) + app.use(helmetConfig); // Security headers + app.use(securityHeaders); // Additional custom security headers + app.use(corsConfig); // CORS policy + app.use(requestSizeLimiter); // Request size limiting + // Note: Rate limiting now applied per-route for better control + + // Body parsing middleware + app.use(express.json({ limit: '10mb' })); + app.use(express.urlencoded({ extended: true, limit: '10mb' })); + + // Trust proxy for accurate IP addresses (important for rate limiting) + app.set('trust proxy', 1); // Routes app.get("/", (req, res) => { @@ -38,11 +47,11 @@ export function createApp() { app.use("/health", healthRoutes); app.use("/api/auth", authRoutes); - app.use("/api/projects", projectRoutes); - app.use("/api/snippets", snippetRoutes); - app.use("/api/pull-requests", pullRequestRoutes); - app.use("/api/notifications", notificationRoutes); - app.use("/api/users", userRoutes); + app.use("/api/projects", generalLimiter, projectRoutes); + app.use("/api/snippets", generalLimiter, snippetRoutes); + app.use("/api/pull-requests", generalLimiter, pullRequestRoutes); + app.use("/api/notifications", generalLimiter, notificationRoutes); + app.use("/api/users", generalLimiter, userRoutes); app.use("/api/branch-protection", branchProtectionRoutes); return { app, server, socketService }; diff --git a/backend/src/middleware/security.ts b/backend/src/middleware/security.ts new file mode 100644 index 0000000..2212065 --- /dev/null +++ b/backend/src/middleware/security.ts @@ -0,0 +1,185 @@ +import helmet from 'helmet'; +import rateLimit from 'express-rate-limit'; +import cors from 'cors'; +import { Request, Response, NextFunction } from 'express'; + +// CORS Configuration +export const corsConfig = cors({ + origin: function (origin, callback) { + // Allow requests with no origin (mobile apps, etc.) + if (!origin) return callback(null, true); + + const allowedOrigins = [ + 'http://localhost:3000', + 'http://localhost:5173', + 'http://127.0.0.1:3000', + 'http://127.0.0.1:5173', + // Add your production domains here + process.env.FRONTEND_URL || 'http://localhost:5173' + ]; + + if (allowedOrigins.indexOf(origin) !== -1) { + callback(null, true); + } else { + callback(new Error('Not allowed by CORS')); + } + }, + credentials: true, + methods: ['GET', 'POST', 'PUT', 'PATCH', 'DELETE', 'OPTIONS'], + allowedHeaders: [ + 'Origin', + 'X-Requested-With', + 'Content-Type', + 'Accept', + 'Authorization', + 'Cache-Control', + 'X-HTTP-Method-Override' + ], + maxAge: 86400 // Cache preflight for 24 hours +}); + +// Helmet Configuration for Security Headers +export const helmetConfig = helmet({ + contentSecurityPolicy: { + directives: { + defaultSrc: ["'self'"], + styleSrc: ["'self'", "'unsafe-inline'", "https://fonts.googleapis.com"], + fontSrc: ["'self'", "https://fonts.gstatic.com"], + imgSrc: ["'self'", "data:", "https:"], + scriptSrc: ["'self'"], + connectSrc: ["'self'", "ws:", "wss:"], // Allow WebSocket connections + frameAncestors: ["'none'"], + objectSrc: ["'none'"], + upgradeInsecureRequests: [] + } + }, + crossOriginEmbedderPolicy: false, // Disable for WebSocket compatibility + hsts: { + maxAge: 31536000, + includeSubDomains: true, + preload: true + } +}); + +// Rate Limiting Configurations +export const createRateLimit = (windowMs: number, max: number, message: string) => + rateLimit({ + windowMs, + max, + message: { error: message, retryAfter: Math.ceil(windowMs / 1000) }, + standardHeaders: true, + legacyHeaders: false, + handler: (req: Request, res: Response) => { + res.status(429).json({ + error: message, + retryAfter: Math.ceil(windowMs / 1000), + limit: max, + windowMs + }); + } + }); + +// Tiered Rate Limiting +export const authLimiter = createRateLimit( + 15 * 60 * 1000, // 15 minutes + 10, // 10 attempts per window (increased from 5) + 'Too many authentication attempts, please try again later' +); + +export const generalLimiter = createRateLimit( + 15 * 60 * 1000, // 15 minutes + 500, // 500 requests per window (increased for better UX) + 'Too many requests, please slow down' +); + +export const strictLimiter = createRateLimit( + 15 * 60 * 1000, // 15 minutes + 50, // 50 requests per window (increased) + 'Rate limit exceeded for this operation' +); + +// Lenient limiter for health and status endpoints +export const healthLimiter = createRateLimit( + 5 * 60 * 1000, // 5 minutes + 1000, // 1000 requests per window + 'Health endpoint rate limit exceeded' +); + +// Custom Progressive Delay Middleware +interface DelayStore { + [key: string]: { hits: number; windowStart: number }; +} + +const delayStore: DelayStore = {}; + +export const speedLimiter = (req: Request, res: Response, next: NextFunction) => { + const ip = req.ip || 'unknown'; + const now = Date.now(); + const windowMs = 15 * 60 * 1000; // 15 minutes + const delayThreshold = 50; // Start delaying after 50 requests + const delayIncrement = 100; // Add 100ms per excess request + const maxDelay = 3000; // Maximum 3 seconds delay + + // Clean old entries + if (!delayStore[ip] || now - delayStore[ip].windowStart > windowMs) { + delayStore[ip] = { hits: 0, windowStart: now }; + } + + delayStore[ip].hits++; + const hits = delayStore[ip].hits; + + if (hits > delayThreshold) { + const excessHits = hits - delayThreshold; + const delay = Math.min(excessHits * delayIncrement, maxDelay); + + setTimeout(() => { + next(); + }, delay); + } else { + next(); + } +}; + +// IP Whitelist Middleware +export const ipWhitelist = (whitelist: string[] = []) => { + return (req: Request, res: Response, next: NextFunction) => { + if (process.env.NODE_ENV === 'development') { + return next(); // Skip IP checking in development + } + + const clientIp = req.ip || req.connection.remoteAddress || req.socket.remoteAddress; + + if (whitelist.length === 0 || whitelist.includes(clientIp as string)) { + next(); + } else { + res.status(403).json({ error: 'IP address not allowed' }); + } + }; +}; + +// Request Size Limiter +export const requestSizeLimiter = (req: Request, res: Response, next: NextFunction) => { + const contentLength = req.headers['content-length']; + const maxSize = 10 * 1024 * 1024; // 10MB limit + + if (contentLength && parseInt(contentLength) > maxSize) { + return res.status(413).json({ error: 'Request entity too large' }); + } + + next(); +}; + +// Security Headers Middleware +export const securityHeaders = (req: Request, res: Response, next: NextFunction) => { + // Remove powered by header + res.removeHeader('X-Powered-By'); + + // Add custom security headers + res.setHeader('X-Content-Type-Options', 'nosniff'); + res.setHeader('X-Frame-Options', 'DENY'); + res.setHeader('X-XSS-Protection', '1; mode=block'); + res.setHeader('Referrer-Policy', 'strict-origin-when-cross-origin'); + res.setHeader('Permissions-Policy', 'camera=(), microphone=(), geolocation=()'); + + next(); +}; \ No newline at end of file diff --git a/backend/src/middleware/validation.ts b/backend/src/middleware/validation.ts index fbe337e..519e765 100644 --- a/backend/src/middleware/validation.ts +++ b/backend/src/middleware/validation.ts @@ -1,19 +1,150 @@ +import { body, param, query, validationResult } from 'express-validator'; import { Request, Response, NextFunction } from 'express'; -import { validationResult } from 'express-validator'; +import mongoose from 'mongoose'; -export const handleValidationErrors = ( - req: Request, - res: Response, - next: NextFunction -): void => { +// Custom validation functions +const isValidObjectId = (value: string) => { + return mongoose.Types.ObjectId.isValid(value); +}; + +const isValidProjectId = (value: string) => { + return value === 'global' || value === 'default' || isValidObjectId(value); +}; + +// Validation result handler +export const handleValidationErrors = (req: Request, res: Response, next: NextFunction) => { const errors = validationResult(req); if (!errors.isEmpty()) { - res.status(400).json({ - success: false, - message: 'Validation failed', - errors: errors.array() + return res.status(400).json({ + error: 'Validation failed', + details: errors.array().map(err => ({ + field: err.type === 'field' ? (err as any).path : 'unknown', + message: err.msg, + value: err.type === 'field' ? (err as any).value : undefined + })) }); - return; } next(); -}; \ No newline at end of file +}; + +// Auth Validation Schemas +export const validateLogin = [ + body('email') + .isEmail() + .normalizeEmail() + .withMessage('Must be a valid email address'), + body('password') + .isLength({ min: 6, max: 128 }) + .withMessage('Password must be between 6 and 128 characters'), + handleValidationErrors +]; + +export const validateSignup = [ + body('username') + .isLength({ min: 3, max: 30 }) + .withMessage('Username must be between 3 and 30 characters') + .matches(/^[a-zA-Z0-9_-]+$/) + .withMessage('Username can only contain letters, numbers, underscores, and hyphens'), + body('email') + .isEmail() + .normalizeEmail() + .withMessage('Must be a valid email address'), + body('password') + .isLength({ min: 8, max: 128 }) + .withMessage('Password must be between 8 and 128 characters'), + handleValidationErrors +]; + +// Pull Request Validation Schemas +export const validateCreatePR = [ + body('title') + .isLength({ min: 1, max: 200 }) + .withMessage('Title must be between 1 and 200 characters') + .trim(), + body('description') + .optional() + .isLength({ max: 5000 }) + .withMessage('Description cannot exceed 5000 characters') + .trim(), + body('sourceBranch') + .matches(/^[a-zA-Z0-9/_-]+$/) + .withMessage('Source branch name contains invalid characters'), + body('targetBranch') + .matches(/^[a-zA-Z0-9/_-]+$/) + .withMessage('Target branch name contains invalid characters'), + body('repository') + .custom(isValidObjectId) + .withMessage('Invalid repository ID'), + handleValidationErrors +]; + +export const validatePRReview = [ + param('id') + .custom(isValidObjectId) + .withMessage('Invalid pull request ID'), + body('decision') + .isIn(['approved', 'rejected', 'comment']) + .withMessage('Decision must be approved, rejected, or comment'), + body('comment') + .optional() + .isLength({ max: 2000 }) + .withMessage('Comment cannot exceed 2000 characters') + .trim(), + handleValidationErrors +]; + +// Branch Protection Validation Schemas +export const validateBranchProtectionRules = [ + body('projectId') + .custom(isValidProjectId) + .withMessage('Invalid project ID'), + body('rules.requiredReviewers') + .optional() + .isInt({ min: 0, max: 10 }) + .withMessage('Required reviewers must be between 0 and 10'), + handleValidationErrors +]; + +// Merge Validation Schemas +export const validateMerge = [ + param('id') + .custom(isValidObjectId) + .withMessage('Invalid pull request ID'), + body('mergeMethod') + .optional() + .isIn(['merge', 'squash', 'rebase']) + .withMessage('Merge method must be merge, squash, or rebase'), + handleValidationErrors +]; + +export const validateForceMerge = [ + param('id') + .custom(isValidObjectId) + .withMessage('Invalid pull request ID'), + body('reason') + .isLength({ min: 10, max: 500 }) + .withMessage('Force merge reason must be between 10 and 500 characters') + .trim(), + body('mergeMethod') + .optional() + .isIn(['merge', 'squash', 'rebase']) + .withMessage('Merge method must be merge, squash, or rebase'), + handleValidationErrors +]; + +// Generic ID Validation +export const validateObjectId = (paramName: string = 'id') => [ + param(paramName) + .custom(isValidObjectId) + .withMessage(`Invalid ${paramName}`), + handleValidationErrors +]; + +// Query Parameter Validation +export const validateProjectQuery = [ + query('projectId') + .optional() + .custom(isValidProjectId) + .withMessage('Invalid project ID in query'), + handleValidationErrors +]; \ No newline at end of file diff --git a/backend/src/routes/auth.ts b/backend/src/routes/auth.ts index 7843557..5007062 100644 --- a/backend/src/routes/auth.ts +++ b/backend/src/routes/auth.ts @@ -1,58 +1,115 @@ -import { Router } from "express"; +import { Router, Response } from "express"; import User from "../models/User"; import bcrypt from "bcryptjs"; import jwt from "jsonwebtoken"; import authMiddleware, { AuthRequest } from "../middleware/auth"; +import { validateLogin, validateSignup } from "../middleware/validation"; +import { authLimiter } from "../middleware/security"; const router = Router(); // Register -router.post("/signup", async (req, res) => { +router.post("/signup", authLimiter, validateSignup, async (req: AuthRequest, res: Response) => { try { const { username, email, password } = req.body; // Check if user exists const existingUser = await User.findOne({ email }); if (existingUser) { - return res.status(400).json({ message: "User already exists" }); + return res.status(400).json({ + error: "User already exists", + message: "An account with this email already exists" + }); } - // Hash password - const hashedPassword = await bcrypt.hash(password, 10); + // Hash password with higher rounds for better security + const hashedPassword = await bcrypt.hash(password, 12); // Create user const newUser = new User({ username, email, password: hashedPassword }); await newUser.save(); - res.status(201).json({ message: "User registered successfully" }); + // Log successful registration (without sensitive data) + console.log(`New user registered: ${username} (${email})`); + + res.status(201).json({ + message: "User registered successfully", + user: { + id: newUser._id, + username: newUser.username, + email: newUser.email + } + }); } catch (err) { - res.status(500).json({ message: "Server error", error: err }); + console.error('Registration error:', err); + res.status(500).json({ + error: "Registration failed", + message: "Unable to create account. Please try again later." + }); } }); // Login -router.post("/login", async (req, res) => { +router.post("/login", authLimiter, validateLogin, async (req: AuthRequest, res: Response) => { try { const { email, password } = req.body; - // Find user - const user = await User.findOne({ email }); - if (!user) return res.status(400).json({ message: "Invalid credentials" }); + // Find user and include password for comparison + const user = await User.findOne({ email }).select('+password'); + if (!user) { + // Log failed login attempt (don't specify if email exists or not) + console.log(`Failed login attempt for email: ${email} from IP: ${req.ip}`); + return res.status(401).json({ + error: "Invalid credentials", + message: "Email or password is incorrect" + }); + } - // Compare password + // Compare password with timing-safe comparison const isMatch = await bcrypt.compare(password, user.password); - if (!isMatch) return res.status(400).json({ message: "Invalid credentials" }); + if (!isMatch) { + console.log(`Failed password for user: ${user.username} from IP: ${req.ip}`); + return res.status(401).json({ + error: "Invalid credentials", + message: "Email or password is incorrect" + }); + } - // Generate JWT + // Generate JWT with stronger settings const token = jwt.sign( - { id: user._id, email: user.email, username: user.username }, + { + id: user._id, + email: user.email, + username: user.username, + iat: Math.floor(Date.now() / 1000) + }, process.env.JWT_SECRET as string, - { expiresIn: "1h" } + { + expiresIn: "24h", + issuer: "collab-code-review", + audience: "collab-code-review-users" + } ); - res.json({ message: "Login successful", token }); + // Log successful login + console.log(`Successful login: ${user.username} from IP: ${req.ip}`); + + res.json({ + message: "Login successful", + token, + user: { + id: user._id, + username: user.username, + email: user.email + }, + expiresIn: "24h" + }); } catch (err) { - res.status(500).json({ message: "Server error", error: err }); + console.error('Login error:', err); + res.status(500).json({ + error: "Login failed", + message: "Unable to process login. Please try again later." + }); } }); diff --git a/backend/src/routes/branchProtection.ts b/backend/src/routes/branchProtection.ts index 03db6f9..7450e2a 100644 --- a/backend/src/routes/branchProtection.ts +++ b/backend/src/routes/branchProtection.ts @@ -1,4 +1,4 @@ -import express from 'express'; +import express, { Request, Response } from 'express'; import rateLimit from 'express-rate-limit'; import { getBranchProtectionStatus, @@ -7,6 +7,13 @@ import { isProtectedBranch } from '../middleware/branchProtection'; import authMiddleware from '../middleware/auth'; +import { + validateMerge, + validateForceMerge, + validateObjectId, + validateBranchProtectionRules, + validateProjectQuery +} from '../middleware/validation'; import PullRequestModel from '../models/PullRequest'; import BranchProtectionRule from '../models/BranchProtectionRule'; @@ -75,7 +82,7 @@ router.get('/pull-requests/:id/protection-status', readLimiter, authMiddleware, * POST /api/branch-protection/merge/:id * Merge a pull request (with branch protection validation) */ -router.post('/merge/:id', mergeLimiter, authMiddleware, validatePRRequirements, async (req, res) => { +router.post('/merge/:id', mergeLimiter, authMiddleware, validateMerge, validatePRRequirements, async (req: Request, res: Response) => { try { const { id } = req.params; const { mergeMethod = 'merge' } = req.body; // merge, squash, rebase diff --git a/backend/src/routes/health.ts b/backend/src/routes/health.ts index 9ac0ac8..19b81ee 100644 --- a/backend/src/routes/health.ts +++ b/backend/src/routes/health.ts @@ -1,9 +1,36 @@ -import { Router } from "express"; +import { Router, Request, Response } from "express"; +import { healthLimiter } from "../middleware/security"; const router = Router(); -router.get("/", (req, res) => { - res.json({ status: "ok", message: "Backend running smoothly" }); +router.get("/", healthLimiter, (req: Request, res: Response) => { + const securityHeaders = { + helmet: req.headers['x-content-type-options'] ? 'enabled' : 'disabled', + cors: req.headers.origin ? 'configured' : 'no-origin', + rateLimit: req.headers['x-ratelimit-limit'] ? 'active' : 'inactive' + }; + + res.json({ + status: "ok", + message: "Backend running smoothly", + timestamp: new Date().toISOString(), + security: securityHeaders, + environment: process.env.NODE_ENV || 'development' + }); +}); + +// Security test endpoint (shows rate limiting in action) +router.get("/security-test", healthLimiter, (req: Request, res: Response) => { + res.json({ + message: "Security middleware active", + ip: req.ip, + userAgent: req.headers['user-agent'], + rateLimitInfo: { + limit: req.headers['x-ratelimit-limit'], + remaining: req.headers['x-ratelimit-remaining'], + reset: req.headers['x-ratelimit-reset'] + } + }); }); export default router; diff --git a/backend/src/routes/pullRequest.ts b/backend/src/routes/pullRequest.ts index 84f03ec..8fd4d7a 100644 --- a/backend/src/routes/pullRequest.ts +++ b/backend/src/routes/pullRequest.ts @@ -1,5 +1,11 @@ import express, { Response } from 'express'; import authMiddleware, { AuthRequest } from '../middleware/auth'; +import { + validateCreatePR, + validatePRReview, + validateObjectId +} from '../middleware/validation'; +import { strictLimiter } from '../middleware/security'; import PullRequest from '../models/PullRequest'; import Project from '../models/Project'; import { NotificationService } from '../services/NotificationService'; @@ -7,7 +13,7 @@ import { NotificationService } from '../services/NotificationService'; const router = express.Router(); // Create a new pull request -router.post('/', authMiddleware, async (req: AuthRequest, res: Response): Promise => { +router.post('/', strictLimiter, authMiddleware, validateCreatePR, async (req: AuthRequest, res: Response): Promise => { try { const { title, @@ -276,7 +282,7 @@ router.post("/:id/comments", authMiddleware, async (req: AuthRequest, res: Respo }); // Submit review decision -router.post("/:id/review", authMiddleware, async (req: AuthRequest, res: Response): Promise => { +router.post("/:id/review", strictLimiter, authMiddleware, validatePRReview, async (req: AuthRequest, res: Response): Promise => { try { const { decision, comment } = req.body; const pullRequest = await PullRequest.findById(req.params.id);