Fix Social Auth Sign-Up Validation Failures#354
Conversation
|
Warning Review limit reached
Next review available in: 56 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughSocial OAuth user creation now retries Mongoose validation failures with validation disabled and translates creation failures into duplicate-user or signup errors. A Jest test verifies the fallback save and callback redirect. ChangesSocial authentication validation handling
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant SocialAuthCallback
participant CompiledUserModel
participant FrontendCallback
SocialAuthCallback->>CompiledUserModel: create(newUserPayload)
CompiledUserModel-->>SocialAuthCallback: ValidationError
SocialAuthCallback->>CompiledUserModel: save({validateBeforeSave: false})
CompiledUserModel-->>SocialAuthCallback: saved user
SocialAuthCallback->>FrontendCallback: redirect with userId
🚥 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 |
![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
🧹 Nitpick comments (2)
apps/public-api/src/__tests__/userAuth.social.test.js (1)
693-749: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a test for the fallback save failure path.
The test covers the happy path (save succeeds → redirect), but the error branch where
doc.savealso fails (controller lines 595-599) is untested. This is a critical path that returns error responses to the client. Consider adding a test wheremockSaverejects (e.g., with an E11000 duplicate key error) and asserting the appropriate error response.🤖 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 `@apps/public-api/src/__tests__/userAuth.social.test.js` around lines 693 - 749, Extend the test for handleSocialAuthCallback to cover the fallback save failure path by making mockSave reject with an E11000 duplicate-key error after MockModel.create throws ValidationError. Assert the controller returns the expected client error response for this failure and does not redirect, while preserving the existing setup and successful-save assertions in the current test.apps/public-api/src/controllers/userAuth.controller.js (1)
588-591: 🗄️ Data Integrity & Integration | 🔵 Trivial
validateBeforeSave: falsebypasses all validation — users are saved with missing required fields.Users persisted via this fallback will have
undefinedfor any custom required fields (e.g.,role,termsAccepted). Downstream code expecting these fields may fail at runtime. Consider setting default values for known custom required fields innewUserPayloadbefore the fallback save, or redirecting users to a profile-completion step after social auth to fill in missing fields.🤖 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 `@apps/public-api/src/controllers/userAuth.controller.js` around lines 588 - 591, Update the fallback save path in the user-auth controller around Model and newUserPayload so required custom user fields receive valid defaults before persistence, or route incomplete social-auth users through profile completion. Ensure fields such as role and termsAccepted are populated before doc.save, while preserving the existing fallback behavior for complete payloads.
🤖 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 `@apps/public-api/src/controllers/userAuth.controller.js`:
- Around line 595-599: Update the inner save-error handler around doc.save to
use saveErr rather than the outer err, distinguish validation failures from
other persistence errors, and avoid exposing raw Mongoose or MongoDB details.
Replace the raw Error/throw with the established AppError pattern, assigning the
appropriate client-safe message and status code for each error type.
---
Nitpick comments:
In `@apps/public-api/src/__tests__/userAuth.social.test.js`:
- Around line 693-749: Extend the test for handleSocialAuthCallback to cover the
fallback save failure path by making mockSave reject with an E11000
duplicate-key error after MockModel.create throws ValidationError. Assert the
controller returns the expected client error response for this failure and does
not redirect, while preserving the existing setup and successful-save assertions
in the current test.
In `@apps/public-api/src/controllers/userAuth.controller.js`:
- Around line 588-591: Update the fallback save path in the user-auth controller
around Model and newUserPayload so required custom user fields receive valid
defaults before persistence, or route incomplete social-auth users through
profile completion. Ensure fields such as role and termsAccepted are populated
before doc.save, while preserving the existing fallback behavior for complete
payloads.
🪄 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
Run ID: 215865d3-d950-4969-af67-85567cc83871
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (2)
apps/public-api/src/__tests__/userAuth.social.test.jsapps/public-api/src/controllers/userAuth.controller.js
|
Please fix coderabbits comment |
|
fix this before this Sunday |
Handle duplicate user error during social signup.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@apps/public-api/src/controllers/userAuth.controller.js`:
- Around line 606-608: Update the outer catch else branch in the relevant
user-auth controller flow to handle non-Mongoose-validation errors with the same
AppError mapping used by the inner catch block, rather than rethrowing raw err.
Preserve the existing validation-error handling while ensuring MongoDB errors
such as duplicate-key failures are converted into a safe client-facing AppError.
🪄 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
Run ID: 75a92eda-c125-4e3e-8203-d0597cac3eed
📒 Files selected for processing (1)
apps/public-api/src/controllers/userAuth.controller.js
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
LGTM |
|
LGTM @coderabbitai |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== PR diff: changed paths and focused patch =='
git diff --name-status HEAD^ HEAD
git diff -- apps/public-api/src/controllers/userAuth.controller.js \
apps/public-api/src/__tests__/userAuth.social.test.js
echo
echo '== Locate SDK/package directories and social-auth-related files =='
fd -HI -t d -E .git '(^|[-_])(react|sdk|python)([-_]|$)|sdk|react' . | head -200
fd -HI -t f -E .git . . | rg -i '(^|/)(react|sdk|python)(/|[-_.])|social.?auth|oauth|callback|userauth' | head -300
echo
echo '== Controller outline and social-auth/callback contract references =='
ast-grep outline apps/public-api/src/controllers/userAuth.controller.js --items all 2>/dev/null || true
rg -n -i -C 4 'findOrCreateSocialUser|social.?auth|oauth|callback|userId|frontend.*callback' \
apps/public-api/src/controllers/userAuth.controller.js \
-g '!**/node_modules/**' \
-g '!**/dist/**' \
-g '!**/build/**' || true
echo
echo '== Cross-repository/package references to affected public contract =='
rg -n -i -C 3 'social.?auth|oauth|auth/callback|callback.*userId|userId.*callback|google|github' \
-g '!**/node_modules/**' \
-g '!**/dist/**' \
-g '!**/build/**' \
. || trueLength of output: 190937 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== Exact PR changes =='
git diff --unified=80 HEAD^ HEAD -- apps/public-api/src/controllers/userAuth.controller.js \
apps/public-api/src/__tests__/userAuth.social.test.js
echo
echo '== Current social-auth controller flow (implementation and callback redirect) =='
rg -n -C 35 'async function findOrCreateSocialUser|const findOrCreateSocialUser|findOrCreateSocialUser|social.*callback|callback.*redirect|redirect\(' \
apps/public-api/src/controllers/userAuth.controller.js
echo
echo '== React SDK callback consumer =='
sed -n '45,105p' sdks/urbackend-react/src/context.tsx
echo
echo '== JavaScript SDK social-auth methods =='
sed -n '465,545p' sdks/urbackend-sdk/src/modules/auth.ts
echo
echo '== Python SDK social-auth methods =='
sed -n '375,445p' sdks/urbackend-python/src/urbackend/auth.py
echo
echo '== Verify no SDK files are changed by this PR =='
git diff --name-only HEAD^ HEAD -- sdks/ || trueLength of output: 42780
This PR only changes server-side user creation/error handling after OAuth authentication. The public OAuth routes and callback contract remain unchanged: No files under |
|
So i feel its good to merge? |
|
Tip For best results, initiate chat on the files or code changes.
The fix is server-side only, includes coverage for the validation fallback, preserves the OAuth callback contract, and does not require changes or releases for the React, JavaScript/TypeScript, or Python SDKs. The reported backend tests and frontend lint checks are also passing. 🐇 |
2 participants
Summary
FIXES #352
This PR fixes a bug where social authentication (Google/GitHub OAuth) failed with a 500 Internal Server Error when projects defined additional custom fields in the user schema with
required: true.Problem
During the OAuth callback flow (
handleSocialAuthCallback), a new user was created using:The payload only contained OAuth-related information such as:
If the application's user schema included any additional required fields (e.g.
phone,department,role, etc.), Mongoose threw aValidationError, causing the OAuth sign-up process to fail with a 500 error.Solution
This PR introduces a validation-bypass fallback inside:
Implementation flow:
If a Mongoose
ValidationErroroccurs because of missing custom required fields:This allows OAuth authentication to complete successfully while enabling applications to collect any missing required profile information later through an onboarding or profile-completion flow.
Changes Made
findOrCreateSocialUser.ValidationError.Testing
Backend
Frontend
Type of Change
Checklist
Summary by CodeRabbit
ValidationErrorduring user creation by retrying persistence with validation disabled.