fix(auth): add withCredentials and fix CORS to persist session cookies#464
fix(auth): add withCredentials and fix CORS to persist session cookies#464anshul23102 wants to merge 1 commit into
Conversation
✅ Deploy Preview for github-spy ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughLogin and Signup pages update their axios POST requests to include ChangesFrontend Credential Configuration
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
🎉 Thank you @anshul23102 for your contribution. Please make sure your PR follows https://github.com/GitMetricsLab/github_tracker/blob/main/CONTRIBUTING.md#-pull-request-guidelines
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/server.js (1)
26-30:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd
express-sessioncookiesameSite/secureoptions for credentialed cross-origin requests.
backend/server.jsenables CORS withcredentials: true, and the frontend sendsaxiosrequests with{ withCredentials: true }, but theexpress-sessionconfig lackscookie.sameSite/cookie.secure. For cross-origin deployments, browsers will reject/drop the session cookie unlesssameSite: 'none'andsecure: trueare set.File: backend/server.js
Lines: 26-30app.use(session({ secret: process.env.SESSION_SECRET, resave: false, saveUninitialized: false, }));🍪 Proposed fix for cross-origin cookie support
app.use(session({ secret: process.env.SESSION_SECRET, resave: false, saveUninitialized: false, + cookie: { + secure: process.env.NODE_ENV === 'production', // HTTPS only in production + sameSite: process.env.NODE_ENV === 'production' ? 'none' : 'lax', + httpOnly: true, + }, }));🤖 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/server.js` around lines 26 - 30, The session cookie config in the app.use(session(...)) call is missing cookie options required for cross-origin credentialed requests; update the session middleware (the app.use(session) invocation) to include a cookie object with sameSite: 'none' and secure: true, and ensure the app trusts proxies when running behind an HTTPS terminator by setting app.set('trust proxy', 1) or equivalent so secure cookies are sent; optionally guard secure by NODE_ENV if you need a different behavior in development.
🤖 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.
Outside diff comments:
In `@backend/server.js`:
- Around line 26-30: The session cookie config in the app.use(session(...)) call
is missing cookie options required for cross-origin credentialed requests;
update the session middleware (the app.use(session) invocation) to include a
cookie object with sameSite: 'none' and secure: true, and ensure the app trusts
proxies when running behind an HTTPS terminator by setting app.set('trust
proxy', 1) or equivalent so secure cookies are sent; optionally guard secure by
NODE_ENV if you need a different behavior in development.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b2edab9d-7137-4ead-a7e8-37874a13d922
📒 Files selected for processing (3)
backend/server.jssrc/pages/Login/Login.tsxsrc/pages/Signup/Signup.tsx
|
Hi @GitMetricsLab team, just checking in on this one. The fix addresses the session cookie issue where the browser was silently dropping |
Login.tsx and Signup.tsx were sending axios POST requests without
{ withCredentials: true }, so the browser silently discarded the
Set-Cookie header on every cross-origin login/signup response. No
session cookie was ever stored, making every subsequent request
appear unauthenticated.
Changes:
- src/pages/Login/Login.tsx: pass { withCredentials: true } as the
third argument to axios.post for /api/auth/login
- src/pages/Signup/Signup.tsx: same fix for /api/auth/signup; also
remove the stale "Include cookies for session" comment that noted
the intent but was never fulfilled
- backend/server.js: replace cors('*') with a credentials-aware
config (origin: FRONTEND_URL, credentials: true); a wildcard origin
is rejected by browsers when credentials are present, so a specific
origin is required for Set-Cookie to be honoured
Fixes GitMetricsLab#414
anshul23102
force-pushed
the
fix/414-withcredentials-session-cookie
branch
from
fbf9cd8 to
d55fbcf
Compare
1 participant
Summary
Fixes #414
The root cause of the broken session is a missing
{ withCredentials: true }in both axios calls, compounded by a wildcard CORS config that the browser rejects whenever credentials are involved. Three files are changed.Changes
src/pages/Login/Login.tsxAdded
{ withCredentials: true }as the third argument toaxios.postfor/api/auth/login. Without this flag the browser silently discards theSet-Cookieheader on every cross-origin response, so no session cookie is ever stored.src/pages/Signup/Signup.tsxSame fix for
/api/auth/signup. Also removed the stale inline comment// Include cookies for sessionthat documented the intended behaviour but was never implemented.backend/server.jsReplaced
cors('*')with a credentials-aware configuration:The Fetch specification forbids
Access-Control-Allow-Origin: *whenAccess-Control-Allow-Credentials: trueis set. Browsers reject the response silently, which meanswithCredentials: trueon the client side has no effect without this server-side change. A specific origin is required;FRONTEND_URLis read from the backend.envfile, with a localhost fallback for local development.Test plan
Cookieheader is present on subsequent requestsFRONTEND_URLinbackend/.envto the production URL and confirm CORS header matches in prodSummary by CodeRabbit