[PR #68] [MERGED] fix: OAuth login 500s at callback — code_verifier never forwarded to token exchange #66

New issue

Closed

opened 2026-08-12 19:09:21 +02:00 by zaph0d · 0 comments

zaph0d commented

2026-08-12 19:09:21 +02:00

Owner

📋 Pull Request Information

Original PR: https://github.com/niels-emmer/myace/pull/68
Author: @niels-emmer
Created: 8/12/2026
Status: ✅ Merged
Merged: 8/12/2026
Merged by: @niels-emmer

Base: main ← Head: fix/oauth-pkce-code-verifier-not-sent

📝 Commits (1)

4bd911c fix: forward PKCE code_verifier from session to OAuth token exchange

📊 Changes

3 files changed (+119 additions, -1 deletions)

View changed files

📝 AGENTS.md (+14 -0)
📝 backend/app/api/auth.py (+10 -1)
➕ backend/tests/test_oauth_callback.py (+95 -0)

📄 Description

Summary

Fixes a second production bug in the OAuth login flow, found immediately after #67 (the redirect_uri scheme fix) unblocked reaching this step for the first time via a real GitHub login: the callback 500s with authlib...OAuthError: invalid_grant: A code_verifier was not included, but the authorization request included a code_challenge.
Root cause: login() generates a PKCE code_verifier/code_challenge itself and stashes the verifier in request.session["code_verifier"], passing code_challenge/code_challenge_method to authorize_redirect() as per-request kwargs (so one generic client works for OIDC/GitHub/Google). Authlib only auto-generates and replays a code_verifier through its own session-backed state when code_challenge_method is set on the client at registration time — since it isn't here, Authlib's internal state never contains a verifier, and auth_callback() was calling authorize_access_token(request) with nothing to fill that gap. The verifier login() stored was written but never read back.
Fix: auth_callback() now does request.session.pop("code_verifier", None) and passes it explicitly to authorize_access_token(request, code_verifier=code_verifier).
This affected all three providers identically (OIDC/GitHub/Google) — it just took a real end-to-end GitHub login to surface it, since it only breaks against a real provider validating a real code_challenge, not against any mocked test.

Test plan

Added backend/tests/test_oauth_callback.py with a fake OAuth client (no network calls) that records what authorize_redirect/authorize_access_token were called with, and asserts the verifier sent at callback time cryptographically produces (via S256) the challenge sent at login time — plus a single-use regression test.
Manually reverted the fix locally and confirmed both new tests fail with the exact original error path; restored the fix and confirmed they pass.
ruff check clean on the changed files.
Full pytest run against the local dev container hit 5 unrelated failures (test_seed_collections.py, test_provider_toggle.py, test_startup_warnings.py) — reproduced independent of this change (pre-existing Postgres-connection contention when the full suite runs back-to-back against this long-lived, already-poked-at local dev container instead of a fresh one); each of those passes individually. CI runs in a clean environment.

🤖 Generated with Claude Code

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/niels-emmer/myace/pull/68 **Author:** [@niels-emmer](https://github.com/niels-emmer) **Created:** 8/12/2026 **Status:** ✅ Merged **Merged:** 8/12/2026 **Merged by:** [@niels-emmer](https://github.com/niels-emmer) **Base:** `main` ← **Head:** `fix/oauth-pkce-code-verifier-not-sent` --- ### 📝 Commits (1) - [`4bd911c`](https://github.com/niels-emmer/myace/commit/4bd911c6dfba7279752ed58189d0101be896a1bd) fix: forward PKCE code_verifier from session to OAuth token exchange ### 📊 Changes **3 files changed** (+119 additions, -1 deletions) <details> <summary>View changed files</summary> 📝 `AGENTS.md` (+14 -0) 📝 `backend/app/api/auth.py` (+10 -1) ➕ `backend/tests/test_oauth_callback.py` (+95 -0) </details> ### 📄 Description ## Summary - Fixes a second production bug in the OAuth login flow, found immediately after #67 (the redirect_uri scheme fix) unblocked reaching this step for the first time via a real GitHub login: the callback 500s with `authlib...OAuthError: invalid_grant: A code_verifier was not included, but the authorization request included a code_challenge.` - Root cause: `login()` generates a PKCE `code_verifier`/`code_challenge` itself and stashes the verifier in `request.session["code_verifier"]`, passing `code_challenge`/`code_challenge_method` to `authorize_redirect()` as per-request kwargs (so one generic client works for OIDC/GitHub/Google). Authlib only auto-generates and replays a `code_verifier` through its own session-backed state when `code_challenge_method` is set on the *client at registration time* — since it isn't here, Authlib's internal state never contains a verifier, and `auth_callback()` was calling `authorize_access_token(request)` with nothing to fill that gap. The verifier `login()` stored was written but never read back. - Fix: `auth_callback()` now does `request.session.pop("code_verifier", None)` and passes it explicitly to `authorize_access_token(request, code_verifier=code_verifier)`. - This affected all three providers identically (OIDC/GitHub/Google) — it just took a real end-to-end GitHub login to surface it, since it only breaks against a real provider validating a real `code_challenge`, not against any mocked test. ## Test plan - [x] Added `backend/tests/test_oauth_callback.py` with a fake OAuth client (no network calls) that records what `authorize_redirect`/`authorize_access_token` were called with, and asserts the verifier sent at callback time cryptographically produces (via S256) the challenge sent at login time — plus a single-use regression test. - [x] Manually reverted the fix locally and confirmed both new tests fail with the exact original error path; restored the fix and confirmed they pass. - [x] `ruff check` clean on the changed files. - Full `pytest` run against the local dev container hit 5 unrelated failures (`test_seed_collections.py`, `test_provider_toggle.py`, `test_startup_warnings.py`) — reproduced independent of this change (pre-existing Postgres-connection contention when the full suite runs back-to-back against this long-lived, already-poked-at local dev container instead of a fresh one); each of those passes individually. CI runs in a clean environment. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

zaph0d

2026-08-12 19:09:21 +02:00