Skip to content

feat: csrf protection - #997

Open
rohan-chaturvedi wants to merge 6 commits into
mainfrom
feat/csrf-protection
Open

feat: csrf protection#997
rohan-chaturvedi wants to merge 6 commits into
mainfrom
feat/csrf-protection

Conversation

@rohan-chaturvedi

Copy link
Copy Markdown
Member

🔍 Overview

The Console browser routes used session cookies without CSRF tokens. The routes /graphql/ and /logout/ were exempt and relied on SameSite cookies only. This PR adds token-based CSRF protection to all session-authenticated routes. The protection works across all supported hosting setups, from standalone Docker Compose to Kubernetes and load balancers.

💡 Proposed Changes

Backend

  • Enforce CSRF on /graphql/, /logout/, /auth/mfa/verify/, and /oauth/github/authorize.
  • Add GET /auth/csrf/. It returns the token in the response body, because the bundled nginx marks cookies HttpOnly. The response is never cacheable.
  • Add the TRUST_PROXY_SSL_HEADER environment variable (default true). When true, Django trusts X-Forwarded-Proto from the proxy for CSRF origin checks.
  • /auth/mfa/verify/ checks CSRF inside the view, because DRF views bypass the CSRF middleware.

Frontend

  • An Apollo link attaches the X-CSRFToken header to every GraphQL request.
  • The client fetches the token once at start and caches it. It never caches a failed fetch.
  • If a request fails CSRF (for example, a login in another tab rotates the secret), the client fetches a new token and retries once.
  • The TOTP verify form and logout send the header. The GitHub authorize form sends the token as a form field.

nginx

  • The bundled configuration now sends X-Forwarded-Proto to the backend.
  • A commented map shows how to pass the header through when TLS terminates upstream.

Out of scope by design

  • Bearer-token surfaces are not affected: the CLI, SDKs, Public API, SCIM, and secret syncs.
  • Login CSRF on the anonymous password endpoints is a known and accepted gap.

🖼️ Screenshots or Demo

No UI changes.

📝 Release Notes

  • The Console now enforces CSRF protection on session-authenticated routes.
  • Refresh open Console tabs after the upgrade. Old tabs get a one-time logout.
  • New environment variable: TRUST_PROXY_SSL_HEADER (default true).
  • If you manage your own copy of nginx/default.conf, download the new version. It adds the X-Forwarded-Proto header.
  • Make sure that ALLOWED_ORIGINS matches the origin in the browser address bar exactly.
  • API clients (CLI, SDKs, Kubernetes operator) are not affected.
  • Documentation: CSRF protection and reverse proxies

❓ Open Questions

None.

🧪 Testing

  • New: backend/tests/test_csrf.py (9 tests). They cover the token endpoint and the accept and reject paths for each protected route.
  • The full backend suite passes (1430 tests). The existing MFA tests are unaffected.
  • Type checks pass on the changed frontend files.

🎯 Reviewer Focus

  • backend/backend/urls.py and backend/backend/settings.py: the enforcement points and the proxy trust setting.
  • backend/api/views/auth_mfa.py: the in-view CSRF check.
  • frontend/apollo/client.ts: the token cache, the prefetch, and the retry on CSRF failure.

➕ Additional Context

  • A docs PR in phasehq/docs covers the configuration for each hosting environment.

✨ How to Test the Changes Locally

  1. Start the dev stack: docker compose -f dev-docker-compose.yml up -d
  2. Log in at https://localhost and save a secret. Mutations must succeed.
  3. Send a POST to /service/graphql/ without a token. The response must be a 403.
  4. Get a token from /service/auth/csrf/. Repeat the POST with the X-CSRFToken header. The CSRF error must be gone.
  5. Make sure that a request with a service token still works without a CSRF token.

💚 Did You...

  • Ensure linting passes (code style checks)?
  • Update dependencies and lockfiles (if required)
  • Update migrations (if required)
  • Regenerate graphql schema and types (if required)
  • Verify the app builds locally?
  • Manually test the changes on different browsers/devices?

@rohan-chaturvedi
rohan-chaturvedi marked this pull request as ready for review August 31, 2026 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants