feat(auth): integrate production-ready session middleware with secure cookie configurations#679
Open
Aryan0819 wants to merge 3 commits into
Open
feat(auth): integrate production-ready session middleware with secure cookie configurations#679Aryan0819 wants to merge 3 commits into
Aryan0819 wants to merge 3 commits into
Conversation
Add session middleware with cookie settings
✅ Deploy Preview for github-spy ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
github-actions
Bot
added
gssoc:approved
level:intermediate
quality:clean
type:accessibility
labels
Contributor
|
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 (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughExpress-session in ChangesSession and Auth
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ 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}
Contributor
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 `@backend/server.js`:
- Around line 36-40: Update session cookie config and add CSRF middleware: set
cookie.httpOnly: true in the express-session config (alongside secure and
sameSite) and install/use csurf (or implement double-submit tokens) to protect
state-changing routes; mount csurf middleware (or token validation) for the auth
routes (functions/handlers handling POST /api/auth/signup and POST
/api/auth/login and the logout handler) and ensure the CSRF token is issued to
the client (e.g., via a secure sameSite cookie or a /api/csrf-token endpoint) so
the client can include it on requests; change the logout route from GET
/api/auth/logout to a non-GET method (e.g., POST) and enforce CSRF verification
in the logout handler as well.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Comment on lines
+36
to
+40
| cookie: { | ||
| maxAge: 24 * 60 * 60 * 1000, | ||
| secure: process.env.NODE_ENV === "production", | ||
| sameSite: process.env.NODE_ENV === "production" ? "none" : "lax" | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if CSRF middleware is used in the codebase
echo "=== Searching for CSRF middleware usage ==="
rg -n -i 'csurf|csrf' --type=js
echo ""
echo "=== Checking route handlers for Content-Type validation ==="
rg -n -C3 'app\.(post|put|patch|delete)' --type=js
echo ""
echo "=== Looking at auth routes for potential CSRF concerns ==="
fd -t f 'auth' --extension js --exec cat {}Repository: GitMetricsLab/github_tracker
Length of output: 2971
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== backend/server.js (lines around 1-120) ==="
nl -ba backend/server.js | sed -n '1,140p'
echo
echo "=== Full search for csurf/csrf usage across repo ==="
rg -n -i 'csurf|csrf' --hidden --glob '!*node_modules/*'
echo
echo "=== Search for CSRF token patterns / middleware ==="
rg -n -i 'csrftoken|csrfToken|x-csrf|xsrf' --hidden --glob '!*node_modules/*'
echo
echo "=== Search for JSON-only / content-type validation on mutation routes ==="
rg -n -i 'content-type|application\/json|req\.is\(.+json' --hidden --glob '!*node_modules/*' --type=jsRepository: GitMetricsLab/github_tracker
Length of output: 180
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== backend/server.js: session/cookie config area ==="
cat -n backend/server.js | sed -n '1,140p'
echo
echo "=== rg: express-session usage ==="
rg -n "express-session" backend/server.js
echo
echo "=== rg: cookie/sameSite/secure/maxAge in backend/server.js ==="
rg -n "cookie|sameSite|secure|maxAge" backend/server.js
echo
echo "=== repo-wide rg: CSRF middleware usage (csurf/csrf) ==="
rg -n -i 'csurf|csrf' --hidden --glob '!*node_modules/*'
echo
echo "=== repo-wide rg: csrfToken / csrftoken patterns ==="
rg -n -i 'csrftoken|csrfToken|x-csrf|xsrf' --hidden --glob '!*node_modules/*'
echo
echo "=== repo-wide rg: content-type enforcement for JSON (req.is / Content-Type checks) ==="
rg -n -i "content-type|application\/json|req\.is\([^)]*json|req\.get\(['\"]Content-Type['\"]\)" --hidden --glob '!*node_modules/*'
echo
echo "=== repo-wide rg: route handlers defined on router.(post|put|patch|delete) ==="
rg -n "router\.(post|put|patch|delete)\(" --hidden --glob '!*node_modules/*' --type=jsRepository: GitMetricsLab/github_tracker
Length of output: 2518
🏁 Script executed:
#!/bin/bash
set -u
shopt -s globstar
echo "=== backend/routes tree (js) ==="
find backend/routes -type f -maxdepth 3 2>/dev/null || true
echo
echo "=== backend/validators tree (js) ==="
find backend/validators -type f -maxdepth 3 2>/dev/null || true
echo
echo "=== repo-wide rg: CSRF middleware usage (csurf/csrf) ==="
rg -n -i 'csurf|csrf' --hidden --glob '!*node_modules/*' || true
echo
echo "=== repo-wide rg: csrfToken / csrftoken patterns ==="
rg -n -i 'csrftoken|csrfToken|x-csrf|xsrf' --hidden --glob '!*node_modules/*' || true
echo
echo "=== repo-wide rg: Content-Type / JSON-only enforcement ==="
rg -n -i "content-type|application\/json|req\.is\([^)]*json|req\.get\(['\"]Content-Type['\"]\)" --hidden --glob '!*node_modules/*' --type=js || true
echo
echo "=== repo-wide rg: mutation route definitions ==="
rg -n "router\.(post|put|patch|delete)\(" backend --hidden --glob '!*node_modules/*' --type=js || true
echo
echo "=== backend/server.js: confirm middleware order ==="
cat -n backend/server.js | sed -n '25,90p'Repository: GitMetricsLab/github_tracker
Length of output: 2038
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== backend/routes/auth.js ==="
cat -n backend/routes/auth.js
echo
echo "=== backend/validators/validationRequest.js ==="
cat -n backend/validators/validationRequest.js
echo
echo "=== backend/validators/authValidator.js ==="
cat -n backend/validators/authValidator.js
echo
echo "=== repo-wide: search for Content-Type enforcement patterns (simple) ==="
rg -n "Content-Type|application/json|req\.is\(|req\.get\(" backend --hidden --glob '!*node_modules/*' --type=js || true
echo
echo "=== repo-wide: search for csurf/csrf tokens (simple) ==="
rg -n -i "csurf|csrfToken|xsrf|csrftoken|x-csrf" backend --hidden --glob '!*node_modules/*' --type=js || trueRepository: GitMetricsLab/github_tracker
Length of output: 4345
Add CSRF protection for cross-site session cookies (sameSite: "none")
backend/server.js sets express-session to sameSite: "none" in production (with secure enabled), which sends the session cookie on cross-site requests. The repo has no CSRF middleware/tokens (csurf/csrfToken/csrftoken etc. all missing), and the only auth routes are state-changing without CSRF checks: POST /api/auth/signup, POST /api/auth/login, and GET /api/auth/logout. CORS origin restriction does not mitigate CSRF for cookie-based requests.
- Implement CSRF protection (e.g.,
csurfor double-submit tokens) for all state-changing endpoints (at least login/logout/signup) - Change
logoutfromGETto a non-GET method and protect it with CSRF - Set
cookie.httpOnly: trueexplicitly for defense-in-depth
🤖 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 36 - 40, Update session cookie config and add
CSRF middleware: set cookie.httpOnly: true in the express-session config
(alongside secure and sameSite) and install/use csurf (or implement
double-submit tokens) to protect state-changing routes; mount csurf middleware
(or token validation) for the auth routes (functions/handlers handling POST
/api/auth/signup and POST /api/auth/login and the logout handler) and ensure the
CSRF token is issued to the client (e.g., via a secure sameSite cookie or a
/api/csrf-token endpoint) so the client can include it on requests; change the
logout route from GET /api/auth/logout to a non-GET method (e.g., POST) and
enforce CSRF verification in the logout handler as well.
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.
Add session middleware with cookie settings
Related Issue
Description
🧱 The Architectural Context:
Cross-origin resource transactions rely on strict browser-level tracking rules. For session-based authentication to maintain continuity across distinct hosting domains (e.g., a frontend hosted on Netlify communicating with a backend hosted on Render), cookies must be explicitly configured with secure, cross-site transport policies.
❌ The Failure Mechanism:
The original session configuration initialized express-session with default fallback cookie settings. While this setup works flawlessly on localhost where the frontend and backend share a common root network origin, it breaks entirely in production. Modern web browsers block cross-origin state tracking by default, causing the client browser to immediately drop and discard incoming session cookies from the server.
💥 The Impact:
Users are completely unable to log in or maintain an active authentication state on the deployed production application. Although the backend processes validation endpoints successfully and returns validation headers, the browser drops the cookie payload on subsequent API requests, throwing infinite authorization errors.
✅ The Solution:
Updated the session middleware configuration inside service.js to include a dynamic, environment-aware cookie parameter object. By programmatically assessing process.env.NODE_ENV, the cookie scales up its policy permissions to secure: true (forcing HTTPS transmission) and sameSite: "none" (authorizing cross-site storage) explicitly when running in production.
How Has This Been Tested?
Local Interoperability Regression Testing: Verified that local development on localhost:5173 retains cookie assignment under standard lax rules without breaking developer flows.
Production Environmental Simulation: Validated through mock environment settings that the cookie payload structures alter correctly to enforce strict cross-origin permissions when the production flag is triggered.
Type of Change
Summary by CodeRabbit
Security
Style