feature: add new authentication edge case coverage#513
Conversation
✅ Deploy Preview for github-spy ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Warning Review limit reached
Your plan includes 1 review of capacity. Refill in 55 minutes and 54 seconds. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more review capacity refills, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than trial, open-source, and free plans. In all cases, review capacity refills continuously over time. Please see our FAQ for further information. 📝 WalkthroughWalkthroughThis PR reformats the authentication test setup and expands test coverage with new edge case validations. The test infrastructure is cleaned up for readability, and comprehensive negative test cases are added for invalid signup and login inputs. ChangesAuth Route Test Coverage Expansion
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
spec/auth.routes.spec.cjs (1)
46-169:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winFix test passwords to satisfy
signupSchemavalidation (otherwise/auth/signupalways 400s).
spec/auth.routes.spec.cjsuses'password123','password456', and'wrongpassword', butbackend/validators/authValidator.jsrequires the password regex to include upper + lower + digit + a special character from@$!%*?&. Sincebackend/routes/auth.jsappliesvalidateRequest(signupSchema)before checking for existing users, these requests will fail early with 400 andmessage: 'Validation failed'(not201/User created successfullyor400/User already exists), breaking the signup/login/logout tests.Proposed fix (update passwords in this spec file)
.send({ username: 'testuser', email: 'test@example.com', - password: 'password123', + password: 'Password123!', });Use a matching valid password (e.g.,
Password123!) for every signup attempt inspec/auth.routes.spec.cjs.🤖 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 `@spec/auth.routes.spec.cjs` around lines 46 - 169, The signup tests in spec/auth.routes.spec.cjs use weak passwords ('password123', 'password456', 'wrongpassword') that fail validateRequest(signupSchema) (password regex requires upper, lower, digit, and special char), causing early 400s; update every password string used in tests (signup and login flows) to a valid value that matches signupSchema (e.g., "Password123!") so the routes in routes/auth.js proceed past validation and the tests exercise user-creation, duplicate-user checks, login (post('/auth/login')) and logout (get('/auth/logout')) as intended.
🧹 Nitpick comments (1)
spec/auth.routes.spec.cjs (1)
186-195: ⚡ Quick winUse precise status code assertions.
All new validation tests use
toBeGreaterThanOrEqual(400), which is imprecise. Based onbackend/validators/validationRequest.js, validation failures return exactly400with a structured error response.Consider asserting the exact status code and optionally verifying the error response structure:
📋 Proposed fix: use exact status code assertions
- expect(res.status).toBeGreaterThanOrEqual(400); + expect(res.status).toBe(400); + expect(res.body.success).toBe(false); + expect(res.body.message).toBe('Validation failed');Apply this pattern to all four new validation tests (lines 194, 205, 215, 223).
Also applies to: 197-206, 208-216, 218-224
🤖 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 `@spec/auth.routes.spec.cjs` around lines 186 - 195, Replace the imprecise status assertions in the signup validation tests so they assert the exact 400 response: change expect(res.status).toBeGreaterThanOrEqual(400) to expect(res.status).toBe(400) in the tests for missing email/username/password/etc. (the tests that call request(app).post('/auth/signup').send(...)); optionally add an assertion on the response body to match the validator's structured error (e.g., check presence of an errors array or message property) to ensure the validationRequest.js format is returned.
🤖 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 `@spec/auth.routes.spec.cjs`:
- Around line 173-224: Add the missing tests required by issue `#512`: create
tests verifying signup with missing username using the '/auth/signup' route and
login with missing email and missing password using '/auth/login' (distinct from
the empty-credentials test), add security tests that query the User model (e.g.,
User.findOne or equivalent used in your test suite) after signup to assert
stored passwords are hashed (not equal to the plaintext) and ensure responses
from '/auth/login' and '/auth/signup' never include password or passwordHash
fields, and add a session test calling '/auth/logout' when no session exists to
assert an appropriate no-session response; implement each as independent it(...)
cases in spec/auth.routes.spec.cjs with clear names like "should not sign up
with missing username", "should not login with missing email", "should not login
with missing password", "should store hashed passwords", "should not expose
password hash in login response", and "should handle logout with no active
session".
---
Outside diff comments:
In `@spec/auth.routes.spec.cjs`:
- Around line 46-169: The signup tests in spec/auth.routes.spec.cjs use weak
passwords ('password123', 'password456', 'wrongpassword') that fail
validateRequest(signupSchema) (password regex requires upper, lower, digit, and
special char), causing early 400s; update every password string used in tests
(signup and login flows) to a valid value that matches signupSchema (e.g.,
"Password123!") so the routes in routes/auth.js proceed past validation and the
tests exercise user-creation, duplicate-user checks, login (post('/auth/login'))
and logout (get('/auth/logout')) as intended.
---
Nitpick comments:
In `@spec/auth.routes.spec.cjs`:
- Around line 186-195: Replace the imprecise status assertions in the signup
validation tests so they assert the exact 400 response: change
expect(res.status).toBeGreaterThanOrEqual(400) to expect(res.status).toBe(400)
in the tests for missing email/username/password/etc. (the tests that call
request(app).post('/auth/signup').send(...)); optionally add an assertion on the
response body to match the validator's structured error (e.g., check presence of
an errors array or message property) to ensure the validationRequest.js format
is returned.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
|
@mehul-m-prajapati Check out this PR . |
|
🎉🎉 Thank you for your contribution! Your PR #513 has been merged! 🎉🎉 |
2 participants
🚀 Feature: Add new test coverage for auth route validation and security edge cases
Closes #512
GSSoC'26 Details
GSSoC Profile: https://gssoc.girlscript.org/profile/e47b2f4a-f3e9-4cb4-97b5-ddd8cb15a1e9
Description
This PR enhances the authentication route test suite by adding coverage for validation and edge-case scenarios that were previously untested.
The existing test suite already verifies the core authentication workflow, including:
This contribution extends coverage to ensure the authentication system handles invalid inputs and edge cases correctly.
Added Test Cases
Signup Validation
Login Validation
Session Handling
Benefits
Testing
auth.routes.spec.cjsExpected Outcome
A more comprehensive and reliable authentication test suite with stronger validation and edge-case coverage.
Type of Change
Summary by CodeRabbit