getsentry / getsentry/sentry

CSRF token not rotated after session cycling during OAuth/2FA login

Open
#106,996 1 comment 0 reactions 0 assignees View on GitHub
Auth
Dominant language
Python
Stars
44.8k
Forks
4.9k
Avg merge
21h 10m
Merged PRs (30d)
635

Description

## Problem

Users experience CSRF validation errors during OAuth login flows, sometimes requiring multiple attempts to successfully authenticate. The pattern observed:

1. Login with credentials → 2FA prompt → CSRF error (auto-rotate offered)
2. Redirect to IdP (e.g., Okta) → 2FA again → CSRF error (manual rotate required)
3. Third attempt succeeds

## Root Cause Analysis

When `session.cycle_key()` is called to prevent session fixation attacks, the CSRF token is not rotated to match the new session. This causes subsequent POST requests to fail CSRF validation because the token was generated for the old session ID.

### Affected Code Paths

| File | Line | Issue |
|------|------|-------|
| `src/sentry/web/frontend/oauth_authorize.py` | ~337 | `cycle_key()` called without `rotate_token()` |
| `src/sentry/web/frontend/oauth_device.py` | ~357 | `cycle_key()` called without `rotate_token()` |
| `src/sentry/web/frontend/twofactor.py` | ~65-77 | `auth.login()` cycles session internally, no `rotate_token()` follows |

### Flow Explanation

1. User submits login credentials
2. `_logged_out_post()` in `oauth_authorize.py` calls `session.cycle_key()` for session fixation prevention
3. CSRF token is now stale (generated for old session ID)
4. User completes 2FA
5. `perform_signin()` in `twofactor.py` calls `auth.login()` which cycles session again internally (Django's `login()` calls `cycle_key()`)
6. CSRF token still not rotated
7. Next POST request fails CSRF validation
8. User must manually rotate token via UI action
9. Process may repeat until token finally syncs

## Proposed Fix

Add `rotate_token(request)` immediately after every `session.cycle_key()` call, and after `auth.login()` calls that implicitly cycle the session.

Example for `oauth_authorize.py`:
```python
from django.middleware.csrf import rotate_token

if request.user.is_authenticated:
request.session.cycle_key()
rotate_token(request) # Keep CSRF token in sync with new session
request.session["oa2"]["uid"] = request.user.id
request.session.modified = True
```

This pattern is already used correctly in `auth_v2/endpoints/csrf.py` for explicit token rotation.

## Security Notes

- Session fixation prevention (`cycle_key()`) remains intact
- `rotate_token()` should be called AFTER `cycle_key()` so the new token associates with the new session
- No changes to CSRF validation logic required

Contributor guide

Open the contributing guide

Research direction

Start with the session changes in src/sentry/web/frontend/oauth_authorize.py, src/sentry/web/frontend/oauth_device.py, and src/sentry/web/frontend/twofactor.py, then compare the explicit rotation pattern in auth_v2/endpoints/csrf.py. Verify that each affected session cycle, including the one performed by auth.login(), is followed by CSRF rotation, and confirm that the OAuth and 2FA POST flows no longer produce CSRF validation errors.

Written by the indexing model from the issue text.

Assessment

Tech stack
django, python
Domain
authentication, security
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.