fix(oauth): add state to OAuth callback flow to prevent Login-CSRF - #252
Merged
Conversation
ZIP/GitHub test-case links extracted from a task's HTML text were fetched through the same authenticated requests.Session used for the Stepik API, sending the Bearer token to any domain the task text happened to link to. External downloads (GitHub always, ZIP for non-stepik.org hosts) now go through core/stepik_client.py::external_download_get() - a fresh session with no Authorization header, gated by validate_external_url() against an explicit host allowlist (github.com, raw.githubusercontent.com, api.github.com, codeload.github.com) with loopback/private/link-local IP literals rejected outright. Genuine stepik.org ZIP links still use the authenticated session via is_stepik_url() since that's first-party, not a leak. _download_github_tests() no longer takes a session parameter at all. Closes #240 (security audit finding F-01, part of #146/#97). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JygXwmtDyBLzCaBa4opDcp
The loopback OAuth callback server accepted the first ?code=... it saw with no verification that the request actually originated from the authorize redirect it triggered. A page that lured the victim into hitting http://localhost:<port>/callback?code=<attacker's code> could bind the local app to the attacker's Stepik account. authorize_via_browser() now sends a cryptographically random state (secrets.token_urlsafe(32)) in the authorize URL. wait_for_auth_code() and _make_oauth_handler() take a required expected_state parameter and reject a missing/mismatched state with a clear RuntimeError before ever extracting a code. Closes #241 (security audit finding F-02, part of #146/#149/#97). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JygXwmtDyBLzCaBa4opDcp
# Conflicts: # CHANGELOG.md
ArtVsMark
marked this pull request as ready for review
July 10, 2026 07:40
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
2 participants
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.
Что и зачем
Локальный OAuth callback-сервер (
wait_for_auth_code/_make_oauth_handler) принимал первый пришедший?code=...без проверки, что колбэк действительно пришёл в ответ на именно этот authorize-редирект. Страница, заманившая жертву перейти наhttp://localhost:<port>/callback?code=<код атакующего>, могла привязать локальное приложение к аккаунту атакующего на Stepik (Login-CSRF, confirmed Medium-severity finding из security-аудита).Теперь:
authorize_via_browser()генерирует криптографически случайныйstate(secrets.token_urlsafe(32)) и добавляет его в authorize URL.wait_for_auth_code()/_make_oauth_handler()принимают обязательный параметрexpected_stateи отклоняют колбэк с отсутствующим/несовпадающимstate(RuntimeError, код не извлекается вовсе).Closes #241
Тип изменения
Чеклист
main, PR — вmainpytest tests/ -x -q— зелёные (996 passed)ruff check .иruff format --check .— чистоmypy src/stepik_grader --ignore-missing-imports— чисто### Fixed)Как проверял
Новые regression-тесты:
tests/test_oauth_flow.py::TestWaitForAuthCode/TestOAuthHandler— отдельные тесты на mismatch/missingstate, happy path обновлён под новую сигнатуру.tests/test_stepik_client_extra.py::TestAuthorizeViaBrowserState— authorize URL содержит тот жеstate, что уходит вwait_for_auth_code; state меняется между вызовами.Generated by Claude Code