Repository navigation
Persist ESI refresh tokens across sessions, discover SSO endpoints from OAuth metadata - #60
Open
mogglemoss wants to merge 2 commits into
Open
mogglemoss wants to merge 2 commits into
mogglemoss wants to merge 2 commits into
Conversation
CCP recommends resolving the SSO authorization and token endpoints from the OAuth metadata document (/.well-known/oauth-authorization-server) rather than hardcoding the /v2/oauth/* URLs. They have a track record of retiring auth routes: the March 2026 spring cleaning removed /verify and the implicit flow, which broke every deployed Short Circuit build at once. Had those builds followed the metadata document, the switch to new routes would have been invisible. The discovery result is cached in-process for an hour, and any failure (network, bad JSON, missing keys) falls back to the existing hardcoded ENDPOINT_EVE_* constants, so login in the worst case behaves exactly as before this change. All three SSO round trips — the browser authorize URL, the code exchange, and the token refresh — now use the discovered endpoints. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The PKCE login keeps the refresh token in memory only, so every launch of Short Circuit requires a full browser round trip to EVE SSO even though CCP hands us a long-lived refresh token precisely so clients can resume sessions. For a tool people open at the start of every play session, that is the single most repeated piece of friction in the app. The refresh token is functionally a password — anyone holding it can act as the character within the granted scopes — so it goes in the OS keychain (macOS Keychain, Windows Credential Manager, Linux Secret Service) via the keyring library, never a plaintext file. The only thing written to disk is the last-used character_id, which is public information (killboards, ESI URLs), stored under appdirs user_data_dir without version scoping so the session survives app upgrades. If no keyring backend exists (headless Linux without Secret Service), we degrade to exactly the old behaviour: re-auth each session, nothing secret ever on disk in the clear. Startup now calls try_silent_login() in a background thread: it loads the persisted token, exchanges it for a fresh access token, decodes the character from the JWT, and fires login_response exactly like an interactive login, so the UI needs no new states. Two behaviours worth calling out: - CCP rotates the refresh token on every successful refresh and the old value is single-use, so the rotated token is persisted on every refresh, not just at login. Skipping this bricks the next silent login with invalid_grant. - Any refresh failure (rejected token, network error, undecodable JWT) logs out AND purges the persisted session. Purging on network blips is deliberately conservative: it trades an occasional extra re-auth for never looping on a dead token at startup and never showing a "connected" UI backed by an expired access token. The network-error handling also fixes a pre-existing crash: an offline refresh previously raised ConnectionError and silently killed the timer thread, leaving the UI claiming to be logged in. Explicit Logout also purges, so logging out now means staying logged out across restarts. Tests fake the keyring module and the char_id meta path — they never touch the real OS keychain, which can prompt interactively on macOS. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
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
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
Two quality-of-life/robustness improvements to the ESI auth flow, ported from the mogglemoss fork where they've been in use:
/.well-known/oauth-authorization-server) instead of being hardcoded, so the next time CCP moves an auth route, deployed builds keep working without a rebuild.Context
CCP's March 2026 spring cleaning (removal of
/verifyand the implicit flow) broke every deployed build at once — that's what motivated both changes. Discovery addresses the route-move class of breakage directly; the persistence work means users log in once, not once per session, which for a tool opened at the start of every play session is the single most repeated piece of friction.Approach
discover_sso_endpoints()caches the metadata document in-process for an hour and falls back to the existing hardcoded constants on any failure (network, bad JSON, missing keys) — worst case is exactly current behavior. All three SSO round trips (authorize URL, code exchange, token refresh) use it.keyringlibrary — never a plaintext file. The only thing on disk is the last-used character_id (public info — killboards, ESI URLs), stored under appdirs'user_data_dirwithout version scoping so sessions survive upgrades. No keyring backend available (headless Linux without Secret Service) degrades to exactly today's behavior.try_silent_login()on a background thread; it fireslogin_responseexactly like an interactive login, so the UI needed no new states. Explicit Logout purges the stored session, so logging out means staying logged out across restarts.Trade-offs
invalid_grant) if you'd prefer.ConnectionErrorthat killed the timer thread, leaving the UI claiming to be logged in with a token that silently expired.keyring. The Pipfile.lock diff is additive only (keyring + its deps); no existing pins changed. The import is guarded, so environments without it keep working.Testing
src/shortcircuit/model/esi/test_esi.py: metadata discovery (success/cache/force-refresh/fallbacks) and persistence (storage round trips, silent-login success with token rotation, dead-token purge, offline handling, logout purge, keyring-unavailable degradation).Risks
Low for discovery (hard fallback to current constants). For persistence, the riskiest surface is keychain behavior across the three OS backends; failures there degrade to re-auth rather than breaking login. Silent login runs on a background thread like the existing login/refresh callbacks, and emits through the same Qt signals.
🤖 Generated with Claude Code