Uh oh!
There was an error while loading. Please reload this page.
fix(auth): enable cross-site cookies and add CSRF protection - #17
Conversation
Address copilot bot review comments by implementing: - Enable SameSite=None cookies in production for cross-site auth (Vercel frontend with Railway backend) - Add CSRF validation using Origin/Referer header checking against ALLOWED_ORIGINS - Apply CSRF validation to register, login, and logout endpoints - Add trust proxy configuration to support deployments behind reverse proxies - Standardize on npm as package manager with packageManager field in package.json Addresses concerns about CSRF vulnerability when using SameSite=None cookies by implementing explicit origin validation for state-changing endpoints. https://claude.ai/code/session_01CEvBbvirScmX9qZbzt9NhV
…dress-pr-review-comments-HXXTV
There was a problem hiding this comment.
Pull request overview
This PR updates backend authentication to support cross-site cookie-based auth (e.g., Vercel ↔ Railway) by switching to SameSite=None in production and adding an Origin/Referer-based CSRF check on auth endpoints. It also adjusts Express proxy settings and pins the backend package manager.
Changes:
- Enable
SameSite=Nonecookies in production (andlaxin development) for auth token cookies. - Add Origin/Referer allowlist validation for CSRF protection on
/register,/login, and/logout. - Configure Express
trust proxyand addpackageManagermetadata to backendpackage.json.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| backend/src/server.ts | Enables trust proxy for reverse-proxy deployments; keeps CORS allowlist-based configuration. |
| backend/src/routes/auth.ts | Adds Origin/Referer validation and updates cookie sameSite behavior for cross-site auth. |
| backend/package.json | Adds packageManager field to standardize tooling metadata. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Get allowed origins from environment | ||
| const allowedOrigins = process.env.ALLOWED_ORIGINS | ||
| ? process.env.ALLOWED_ORIGINS.split(',').map(origin => origin.trim()).filter(origin => origin.length > 0) | ||
| : ['http://localhost:5173', 'http://localhost:3000']; |
There was a problem hiding this comment.
allowedOrigins is re-parsed here even though the server already builds the same list for CORS. Duplicating this parsing in multiple places risks the CORS allowlist and CSRF allowlist diverging over time; consider centralizing origin allowlist parsing in a shared module and importing it from both server and routes.
| function validateOrigin(req: express.Request, res: express.Response): boolean { | ||
| const origin = req.get('origin'); |
There was a problem hiding this comment.
validateOrigin takes res but doesn't use it. Dropping the unused parameter will make the function signature clearer and avoid implying that the response is modified during validation.
| // Check if origin header is present and in allowed list | ||
| if (origin) { | ||
| return allowedOrigins.some(allowed => origin === allowed || origin.startsWith(allowed)); | ||
| } | ||
| // Fallback to referer header if origin is not present | ||
| if (referer) { | ||
| return allowedOrigins.some(allowed => referer.startsWith(allowed)); | ||
| } |
There was a problem hiding this comment.
validateOrigin uses origin.startsWith(allowed) / referer.startsWith(allowed) for allowlisting. This is bypassable (e.g., https://good.com.evil.com starts with https://good.com, and differing ports can also match). Parse the header as a URL and compare the normalized .origin against an exact allowlist (or implement explicit, boundary-checked wildcard subdomain matching if needed).
| // CSRF protection: Validate origin | ||
| if (!validateOrigin(req, res)) { | ||
| return res.status(403).json({ message: 'CSRF validation failed' }); | ||
| } |
There was a problem hiding this comment.
CSRF validation is only applied to /register, /login, and /logout. With SameSite=None cookies in production, any cookie-authenticated state-changing endpoints (e.g., /api/notes POST/PUT/DELETE) remain CSRFable unless they also enforce an origin check or a CSRF token. Consider extracting this into middleware and applying it to all non-idempotent routes that accept cookie auth.
Address copilot bot review comments by implementing:
Addresses concerns about CSRF vulnerability when using SameSite=None cookies by implementing explicit origin validation for state-changing endpoints.
https://claude.ai/code/session_01CEvBbvirScmX9qZbzt9NhV