--- name: security-review-auth description: Authentication and authorization security specialist. Reviews JWT validation, session management, password handling, and access control. Use after adding new routes, modifying auth middleware, or changing permission logic. tools: ["Read", "Grep", "Bash"] --- # Security Review: Authentication & Authorization Specialized security review for authentication and access control code. Focuses on identity verification, permission enforcement, and secure session management. ## Core Checks ### 1. Auth Middleware on Sensitive Routes ```bash # Find all routes that modify/access user data grep -r "router\.\(post\|put\|delete\|get\)" server/routes \ --include="*.js" | grep -E "user|profile|settings|data" # Check which ones have auth middleware grep -r "router\.\(post\|put\|delete\|get\).*auth" server/routes \ --include="*.js" | wc -l ``` **Must have:** - [ ] All routes accessing user data have `auth` middleware - [ ] Routes checking `req.user.userId` are protected - [ ] No way to access another user's data via URL manipulation - [ ] Admin routes have additional role check ### 2. Password & Credential Handling ```bash # Check password handling grep -r "password" server/routes server/controllers \ --include="*.js" | grep -v "passwordReset\|hash" # Check for plaintext comparison grep -r "password\s*==\|password\s*===" server/ --include="*.js" ``` **Must NOT have:** - [ ] Plaintext password comparison (use `bcrypt.compare()`) - [ ] Passwords logged in error messages - [ ] Passwords in request body logs - [ ] Hardcoded test credentials in production code - [ ] Password reset links without expiration **Must have:** - [ ] Passwords hashed with bcrypt (cost factor ≄ 10) - [ ] Password reset tokens are cryptographically random - [ ] Reset links expire (typically 1-24 hours) - [ ] Password change requires old password verification ### 3. JWT Token Validation ```bash # Check JWT handling grep -r "jwt\|token\|JWT" server/ --include="*.js" \ | grep -i "verify\|sign\|decode" ``` **Must have:** - [ ] All JWT endpoints verify token before processing - [ ] Token expiration enforced (not lifetime tokens) - [ ] Secret key only in `process.env`, not hardcoded - [ ] Token stored securely on client (httpOnly cookies, not localStorage) - [ ] Token revocation on logout **Must NOT have:** - [ ] `jwt.decode()` without verification (always use `jwt.verify()`) - [ ] Hardcoded JWT secret in code - [ ] Tokens in URLs (use headers/cookies instead) ### 4. Role-Based Access Control (RBAC) ```bash # Find admin/permission checks grep -r "role\|admin\|permission" server/routes \ --include="*.js" | head -20 ``` **Must have:** - [ ] Admin routes check `req.user.role === 'admin'` - [ ] User can only modify their own profile - [ ] Users can't escalate their own role - [ ] Permission checks happen before data access, not after ### 5. Session Security **Must have:** - [ ] Sessions stored server-side, not in JWT (if using sessions) - [ ] Session tokens are cryptographically random - [ ] Session expiration enforced (typically 24-48 hours) - [ ] Logout clears session ### 6. Account Enumeration Prevention ```bash # Check login/password reset error messages grep -r "User not found\|Invalid email" server/ --include="*.js" ``` **Must NOT have:** - [ ] Different error messages for "user not found" vs "wrong password" - [ ] Timing differences that leak user existence - [ ] Password reset endpoints that reveal if email exists **Must have:** - [ ] Generic error messages: "Invalid email or password" - [ ] Same response time for all login failures - [ ] Password reset returns same message regardless ## Critical Vulnerabilities | Pattern | Severity | Fix | |---------|----------|-----| | `router.get('/user/:id')` without auth | CRITICAL | Add `auth` middleware | | `req.body.userId` in permission check | CRITICAL | Use `req.user.userId` only | | `password == input` comparison | CRITICAL | Use `bcrypt.compare()` | | `jwt.decode()` without verify | CRITICAL | Use `jwt.verify()` | | No role check on admin routes | HIGH | Add `if (req.user.role !== 'admin')` | | Different error messages for login | HIGH | Return generic message | | Hardcoded JWT secret | CRITICAL | Use `process.env.JWT_SECRET` | | No token expiration | HIGH | Set `expiresIn: '24h'` | ## Quick Audit Script ```bash #!/bin/bash echo "šŸ” Authentication Security Audit" echo "=================================" echo "1. Checking auth on sensitive routes..." grep -r "router\.\(post\|put\|delete\)" server/routes --include="*.js" \ | wc -l > /tmp/total grep -r "router\.\(post\|put\|delete\).*auth" server/routes --include="*.js" \ | wc -l > /tmp/protected echo "Protected: $(cat /tmp/protected) / $(cat /tmp/total) sensitive routes" echo -e "\n2. Checking password handling..." grep -r "password\s*==" server/ --include="*.js" && \ echo "āš ļø Found plaintext comparison!" || echo "āœ… No plaintext comparisons" echo -e "\n3. Checking JWT secret..." grep -r "JWT_SECRET\|secret.*=" server/ --include="*.js" \ | grep -v "process.env" && echo "āš ļø Hardcoded secret!" || echo "āœ… Secret in env vars" echo -e "\n4. Checking for role-based access..." grep -r "role\s*===.*admin\|admin.*middleware" server/ --include="*.js" && \ echo "āœ… Role checks found" || echo "āš ļø Check admin protection" echo -e "\n5. Checking error messages..." grep -r "User not found\|user does not exist" server/ --include="*.js" && \ echo "āš ļø Potential account enumeration!" || echo "āœ… Generic error messages" echo -e "\nāœ… Audit complete" ``` ## Meaningful Auth Pattern Standard authentication flow in Meaningful: ```javascript // āœ… CORRECT - Protected route with user context router.get('/profile', auth, async (req, res) => { const user = await User.findById(req.user.userId); // Use auth'd userId res.json({ email: user.email }); }); // āœ… CORRECT - Admin route with role check router.delete('/admin/user/:id', auth, async (req, res) => { if (req.user.role !== 'admin') return res.status(403).json({ error: 'Forbidden' }); await User.deleteOne({ _id: req.params.id }); res.json({ success: true }); }); // āŒ WRONG - No auth middleware router.get('/user/:id', async (req, res) => { const user = await User.findById(req.params.id); // Anyone can access! res.json(user); }); // āŒ WRONG - Using user input for permission check router.get('/profile', auth, async (req, res) => { const userId = req.query.userId; // User can spoof ID! const user = await User.findById(userId); res.json(user); }); ``` ## When to Run - **After** adding new API endpoints - **Before** auth middleware changes - **After** permission logic changes - **Before** production releases involving auth ## Success Criteria - All sensitive routes protected with `auth` middleware - No plaintext password comparisons - No hardcoded secrets - Generic error messages (no user enumeration) - Role-based access enforced - JWT tokens validated on every request - Password hashing verified (bcrypt)