fix: secure session cookies and validate WebSocket origin#405
fix: secure session cookies and validate WebSocket origin#405its-me-abhishek merged 3 commits intomainfrom
Conversation
Security fixes: - Add Secure, HttpOnly, SameSite flags to session cookies - HttpOnly: prevents JavaScript access (XSS protection) - Secure: cookies sent only over HTTPS in production - SameSite=Lax: CSRF protection while allowing OAuth redirects - Validate WebSocket origin against allowed origins - Check Origin header against ALLOWED_ORIGIN env var - Allow localhost in development mode - Fallback to same-origin check if no env var configured - Log rejected connection attempts - Update example.backend.env with security settings - Add ENV=production for secure mode - Add ALLOWED_ORIGIN for WebSocket validation - Better documentation of all variables Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Thank you for opening this PR! Before a maintainer takes a look, it would be really helpful if you could walk through your changes using GitHub's review tools. Please take a moment to:
More information on how to conduct a self review: This helps make the review process smoother and gives us a clearer understanding of your thought process. Once you've added your self-review, we'll continue from our side. Thank you! |
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
| // If no ALLOWED_ORIGIN configured, check if origin matches the request host | ||
| // This provides same-origin protection as fallback | ||
| host := r.Host | ||
| if strings.Contains(origin, host) { |
There was a problem hiding this comment.
this might be easy to break.
xyz.abc.org
and
xyz.org would both have xyz as host
- Club empty origin check with development mode check as suggested - Replace strings.Contains with proper URL parsing for exact host match - In production, require Origin header (reject if missing) - Use url.Parse() for safe hostname extraction and comparison Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Addressed both review comments in commit f284ff9:
Also added: In production mode, missing Origin header is now rejected (logged as warning). Tested on VPS - login works, bad origins are rejected with 400. |
Summary
Security fixes for two high-severity vulnerabilities identified in the security audit.
1. Secure Session Cookies
Problem: Session cookies were created without security flags, making them vulnerable to:
Fix: Added secure cookie options in
backend/main.go:HttpOnly: true- Prevents JavaScript accessSecure: true(in production) - Cookies only sent over HTTPSSameSite: Lax- CSRF protection while allowing OAuth redirectsMaxAge: 7 days- Reasonable session lifetime2. WebSocket Origin Validation
Problem: WebSocket upgrader accepted connections from ANY origin:
This allowed any website to establish WebSocket connections, enabling cross-site WebSocket hijacking.
Fix: Added
checkWebSocketOrigin()function that:ALLOWED_ORIGINenvironment variableENV != "production")3. Updated Production Configuration
ENVvariable documentation (set to "production" for secure mode)ALLOWED_ORIGINvariable for WebSocket validationFiles Changed
backend/main.go- Added secure session cookie optionsbackend/controllers/websocket.go- Added origin validationproduction/example.backend.env- Added security configurationDeployment Notes
After merging, update the VPS backend
.envto include:Then rebuild and restart the backend container.
Test Plan
🤖 Generated with Claude Code