feat(auth): add centralized request validation and input sanitization for authentication endpoints#61
Conversation
|
@om-dev007 is attempting to deploy a commit to the rishabhjtripathi2903-3434's projects Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughAdds Auth Request Validation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@Rishabhworkspace please review this pr |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with 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.
Inline comments:
In `@backend/src/middleware/validationMiddleware.ts`:
- Around line 8-28: The validation chains in validationMiddleware should stop
after the first failure so a single bad field does not emit multiple errors. Add
.bail() in each body('name'), body('email'), and body('password') chain after
the required check and before the type/length checks, and apply the same pattern
to the other validation block referenced by this review so the centralized error
payload stays one-error-per-field.
- Around line 43-46: The password validator in validationMiddleware only checks
for non-empty input, so short passwords still reach loginUser and the downstream
Prisma/bcrypt flow. Update the existing password rule used by the auth login
validation to enforce the same minimum length floor as registration, while
keeping the current empty check and message behavior. Use the body('password')
validator chain in validationMiddleware to add the length constraint so invalid
short passwords are rejected before controller/database work.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 036e1ffe-e8dc-499f-ae7e-c02f18198e8f
⛔ Files ignored due to path filters (1)
backend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
backend/package.jsonbackend/src/controllers/authController.tsbackend/src/middleware/validationMiddleware.tsbackend/src/routes/authRoutes.ts
💤 Files with no reviewable changes (1)
- backend/src/controllers/authController.ts
| body('name') | ||
| .trim() | ||
| .notEmpty() | ||
| .withMessage('Name is required') | ||
| .isLength({ min: 2, max: 50 }) | ||
| .withMessage('Name must be between 2 and 50 characters'), | ||
|
|
||
| body('email') | ||
| .trim() | ||
| .notEmpty() | ||
| .withMessage('Email is required') | ||
| .isEmail() | ||
| .withMessage('Please provide a valid email address') | ||
| .normalizeEmail(), | ||
|
|
||
| body('password') | ||
| .trim() | ||
| .notEmpty() | ||
| .withMessage('Password is required') | ||
| .isLength({ min: 8 }) | ||
| .withMessage('Password must be at least 8 characters long'), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Stop each validation chain after the first failure.
Without .bail(), whitespace-only name, email, or password values can produce multiple errors for the same field (required plus invalid/too short). That makes the centralized error payload less consistent than intended.
Suggested fix
export const registerValidation = [
body('name')
.trim()
.notEmpty()
.withMessage('Name is required')
+ .bail()
.isLength({ min: 2, max: 50 })
.withMessage('Name must be between 2 and 50 characters'),
body('email')
.trim()
.notEmpty()
.withMessage('Email is required')
+ .bail()
.isEmail()
.withMessage('Please provide a valid email address')
.normalizeEmail(),
body('password')
.trim()
.notEmpty()
.withMessage('Password is required')
+ .bail()
.isLength({ min: 8 })
.withMessage('Password must be at least 8 characters long'),
];
export const loginValidation = [
body('email')
.trim()
.notEmpty()
.withMessage('Email is required')
+ .bail()
.isEmail()
.withMessage('Please provide a valid email address')
.normalizeEmail(),
body('password')
.trim()
.notEmpty()
.withMessage('Password is required')
];Also applies to: 35-46
🤖 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/middleware/validationMiddleware.ts` around lines 8 - 28, The
validation chains in validationMiddleware should stop after the first failure so
a single bad field does not emit multiple errors. Add .bail() in each
body('name'), body('email'), and body('password') chain after the required check
and before the type/length checks, and apply the same pattern to the other
validation block referenced by this review so the centralized error payload
stays one-error-per-field.
| body('password') | ||
| .trim() | ||
| .notEmpty() | ||
| .withMessage('Password is required'), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Enforce the password length floor on login too.
Line 43-46 only reject empty passwords, so short values still reach loginUser and trigger the Prisma lookup / bcrypt compare path. That misses the stated auth-endpoint contract of keeping low-quality input out of the controller/database.
Suggested fix
export const loginValidation = [
body('email')
.trim()
.notEmpty()
.withMessage('Email is required')
.isEmail()
.withMessage('Please provide a valid email address')
.normalizeEmail(),
body('password')
.trim()
.notEmpty()
- .withMessage('Password is required'),
+ .withMessage('Password is required')
+ .bail()
+ .isLength({ min: 8 })
+ .withMessage('Password must be at least 8 characters long'),
];📝 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.
| body('password') | |
| .trim() | |
| .notEmpty() | |
| .withMessage('Password is required'), | |
| body('password') | |
| .trim() | |
| .notEmpty() | |
| .withMessage('Password is required') | |
| .bail() | |
| .isLength({ min: 8 }) | |
| .withMessage('Password must be at least 8 characters long'), |
🤖 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/middleware/validationMiddleware.ts` around lines 43 - 46, The
password validator in validationMiddleware only checks for non-empty input, so
short passwords still reach loginUser and the downstream Prisma/bcrypt flow.
Update the existing password rule used by the auth login validation to enforce
the same minimum length floor as registration, while keeping the current empty
check and message behavior. Use the body('password') validator chain in
validationMiddleware to add the length constraint so invalid short passwords are
rejected before controller/database work.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
2 participants
Description
This PR adds centralized request validation and input sanitization for authentication endpoints using express-validator.
Changes Made
Benefits
Closes #55
Summary by CodeRabbit
New Features
Bug Fixes