feat(oauth): add OpenRouter PKCE OAuth flow - #2355
Merged
Merged
Conversation
New ``gptme.oauth.openrouter`` module that implements OpenRouter's dynamic OAuth/PKCE flow: 1. Generate PKCE verifier + S256 challenge 2. Open browser to https://openrouter.ai/auth?... 3. Listen for callback on http://127.0.0.1:3000/callback 4. Exchange code at https://openrouter.ai/api/v1/auth/keys 5. Receive a durable ``sk-or-...`` API key OpenRouter's OAuth flow needs no client_id registration on our side and returns a permanent API key (not a JWT), so the result is well suited for both interactive setup and headless automation. This is foundation work for the redesigned ``/account setup openrouter`` command (#2313, follow-up to closed #2353). Shipped as a standalone module with no UI integration so the surface stays minimal and testable. Tests cover: - PKCE verifier/S256 challenge derivation - Port availability check - Full happy-path OAuth flow with mocked browser + token exchange - CSRF state mismatch rejection - Provider-side error surfacing - Token exchange HTTP failure - Malformed/missing-key response 10 tests, all passing.
Contributor
Greptile SummaryThis PR introduces
Confidence Score: 5/5Safe to merge — all issues flagged in the previous review round have been fully addressed in this revision. The module is new, self-contained, and introduces no regressions to existing auth paths. The handler factory isolates per-flow state, HTML injection is prevented with html.escape(), the TOCTOU bind race is caught with try/except OSError, and output goes through the logging hierarchy. The test suite exercises every error path. No remaining correctness or security issues were found. No files require special attention. Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant Authenticate as authenticate()
participant Browser
participant CallbackServer as HTTPServer 127.0.0.1:3000
participant OpenRouter
Authenticate->>Authenticate: generate_pkce() verifier + S256 challenge
Authenticate->>Authenticate: token_urlsafe(16) CSRF state
Authenticate->>Authenticate: _make_callback_handler per-flow closure
Authenticate->>CallbackServer: start serve_forever on daemon thread
Authenticate->>Browser: open_browser(auth_url with challenge + state)
Browser->>OpenRouter: "GET /auth?callback_url=...&code_challenge=...&state=..."
User->>OpenRouter: Sign in
OpenRouter->>CallbackServer: "GET /callback?code=...&state=..."
CallbackServer->>CallbackServer: Validate CSRF state
CallbackServer->>Authenticate: "state_holder authorization_code = code"
Authenticate->>Authenticate: poll loop exits (0.2s interval)
Authenticate->>Authenticate: server.shutdown() + server_close()
Authenticate->>OpenRouter: POST /api/v1/auth/keys with code and code_verifier
OpenRouter-->>Authenticate: key sk-or-...
Authenticate-->>User: return durable API key
Reviews (2): Last reviewed commit: "fix(oauth): address Greptile review find..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
- Shared class-level state → closure-based handler factory (_make_callback_handler) so concurrent authenticate() calls on different ports cannot corrupt each other's CSRF state - HTML injection → html.escape() on error_description before interpolating into the callback response page - TOCTOU → wrap HTTPServer() construction in try/except OSError so races between is_port_available() and bind() raise OAuthError with a clear message instead of a bare OSError - print() → logger.info/warning throughout; library code should not write to stdout directly
5 tasks done
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
1 participant
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.
Summary
Adds a
gptme.oauth.openroutermodule that runs OpenRouter's dynamic OAuth/PKCE flow:https://openrouter.ai/auth?...http://127.0.0.1:3000/callbackhttps://openrouter.ai/api/v1/auth/keyssk-or-...API keyOpenRouter's flow needs no client_id registration on our side and returns a permanent API key (not a JWT), so the result is suitable for both interactive setup and headless automation.
Why this exists
Foundation for the redesigned
/account setup openrouterflow that #2313 originally asked for. Closed PR #2353 went the "swap raw API keys" route, which Erik pushed back on ("kinda meh and pointless") — the more ambitious shape is OAuth-first onboarding so users go frompip install gptmeto a working agent without ever pasting a key.This PR ships only the OAuth module so the surface stays small and reviewable. The
/accountcommand wiring will land in a follow-up.Design notes
llm_openai_subscription.pyflow (which is JWT-based and OpenAI-specific). No risk of regressing OpenAI subscription auth.authenticate(open_browser=...)accepts an injectable browser hook so the full happy path is testable without a real browser.OAuthError— port-busy, callback errors, token-exchange failures, and malformed responses all raise the same exception type with a descriptive message.Test plan
poetry run pytest tests/test_oauth_openrouter.py -v— 10 passedpoetry run ruff check gptme/oauth/ tests/test_oauth_openrouter.py— All checks passedpoetry run ruff format gptme/oauth/ tests/test_oauth_openrouter.py— cleanpoetry run mypy gptme/oauth/openrouter.py— Success: no issues foundTests cover:
urlsafe_b64encode(sha256(verifier))is_port_available)authenticate()raisesOAuthErrorwhen port busyerror=...callbacks surface the description verbatimOAuthErrorsk-or-...key raisesOAuthErrorFollow-up
A separate PR will wire this module into a
/account setup openroutercommand and store the issued key in the gptme credential config. That PR depends on this one but stays out of scope here so review can focus on the OAuth flow itself.Refs: #2313 (original ask), #2353 (closed predecessor PR — different approach).