Fix session cookies with proper CORS and credentialed auth requests#254
Open
ChaitanyaChute wants to merge 4 commits into
Open
Fix session cookies with proper CORS and credentialed auth requests#254ChaitanyaChute wants to merge 4 commits into
ChaitanyaChute wants to merge 4 commits into
Conversation
✅ Deploy Preview for github-spy ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
📝 WalkthroughWalkthroughThis PR fixes session cookie persistence by implementing credentialed authentication across the stack: the backend now uses an explicit CORS allowlist with credentials enabled, validates signup by both email and username with proper error handling, and the frontend's login and signup requests explicitly send credentials to maintain session state. ChangesSession Authentication with Credentials and Credentials
Sequence Diagram(s)sequenceDiagram
participant Client as Frontend Client
participant CORS as CORS Middleware
participant SignupRoute as POST /api/auth/signup
participant MongoDB as MongoDB
Client->>CORS: POST signup with credentials
activate CORS
CORS->>CORS: Check origin against allowlist
CORS->>CORS: Allow credentials: true
CORS->>SignupRoute: Forward request
deactivate CORS
activate SignupRoute
SignupRoute->>MongoDB: Query for email OR username
activate MongooDB
alt User found
MongooDB-->>SignupRoute: Existing user
SignupRoute-->>Client: 400 "User already exists"
else User not found
MongooDB-->>SignupRoute: No match
SignupRoute->>MongooDB: Insert new user
MongooDB-->>SignupRoute: Success
SignupRoute-->>Client: 201 Created + set-cookie
end
deactivate MongooDB
deactivate SignupRoute
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
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 |
ChaitanyaChute
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/pages/Signup/Signup.tsx (1)
40-56: ⚡ Quick winConsider removing the commented-out mock implementation.
The old mock code and TODO comments are no longer needed since the real backend integration is now working. Removing dead code improves maintainability.
🧹 Proposed cleanup
- - // // Simulate API call (replace with your actual backend integration) - // try { - // // Mock successful signup - // setMessage("Account created successfully! Redirecting to login..."); - - // // In your actual implementation, integrate with your backend here: - // // const response = await fetch(`${backendUrl}/api/auth/signup`, { - // // method: 'POST', - // // headers: { 'Content-Type': 'application/json' }, - // // body: JSON.stringify(formData) - // // }); - - // setTimeout(() => { - // // Navigate to login page in your actual implementation - // console.log("Redirecting to login page..."); - // }, 2000); - } catch (error) { setMessage("Something went wrong. Please try again."); }🤖 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 `@src/pages/Signup/Signup.tsx` around lines 40 - 56, Remove the dead commented-out mock signup flow in Signup.tsx: delete the block that simulates the API call and the setTimeout/console.log redirect (the commented lines referencing setMessage, formData, and the mock fetch), leaving only the real backend integration in the signup handler (e.g., the actual fetch/POST logic and subsequent setMessage/navigation). Ensure no leftover TODOs or commented code related to the old mock remain.
🤖 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.
Nitpick comments:
In `@src/pages/Signup/Signup.tsx`:
- Around line 40-56: Remove the dead commented-out mock signup flow in
Signup.tsx: delete the block that simulates the API call and the
setTimeout/console.log redirect (the commented lines referencing setMessage,
formData, and the mock fetch), leaving only the real backend integration in the
signup handler (e.g., the actual fetch/POST logic and subsequent
setMessage/navigation). Ensure no leftover TODOs or commented code related to
the old mock remain.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f369502f-c36b-422e-a280-f53d5dc44797
📒 Files selected for processing (6)
Dockerfile.prodbackend/routes/auth.jsbackend/server.jsspec/auth.routes.spec.cjssrc/pages/Login/Login.tsxsrc/pages/Signup/Signup.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related Issue
Description
Type of Change
Summary by CodeRabbit
Bug Fixes
Tests