Repository navigation
Key OAuth2 sessions by cookie, call providers asynchronously and deprecate the stream-socket mode - #8
Merged
Merged
Conversation
…ecate the stream-socket mode OAuth2 sessions were stored per client IP, so two players behind one NAT shared a login and ?user returned the previous visitor's profile. The browser is now identified by a random HttpOnly verifier_session cookie shared by /dwa and /ss14wa, so one browser's Discord and SS14 logins land under the same key (Civilizationbot's Civ14 verifier correlates them by that key), while unrelated browsers never collide. Idle sessions expire after 24 hours and a malformed cookie value gets a fresh id. The token exchange, profile and revocation calls used blocking cURL inside the event loop, which stalled the whole process (verifier, Discord gateway and HTTP server share one loop) for every provider round trip. The authenticator now takes an asynchronous HTTP client (ReactPHP Browser by default, 10 s timeout) and returns promises; a new AsyncEndpointInterface lets handleReact answer when the promise settles. The synchronous in-process adapter still serves login, logout and cached profile reads and answers 503 for anything that needs a provider call. A code can be exchanged once per login; the Discord revocation path is /oauth2/token/revoke as documented. init(null, true) (stream-socket mode) now emits a deprecation notice; the README explains its limits. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016WTP2S4qKEFT8a6a6yFEL4
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.
Requested by Val · project thread
Implements the three follow-up recommendations from the review (#4 to #7 fixed the bugs).
Problem
/dwa,/ss14wa) were stored per client IP. Two players behind one NAT shared a login, and?userreturned the previous visitor's profile.init(null, true)) reads 1 KiB per request and parses form bodies as headers; it only ever worked for the legacy header format.Change
SessionCookiemints a random 128-bitverifier_sessionid (HttpOnly, SameSite=Lax, 24 h idle expiry), shared by both endpoints so one browser's Discord and SS14 logins land under the same key. Sessions live in$sessions[<endpoint>][<id>], the shape Civilizationbot'sCiv14\Verifier::getIPFromDiscord()/getSS14FromIP()already walk (they only use the key opaquely). Idle entries are pruned; a malformed cookie value gets a fresh id.OAuth2Authenticatoris now one instance per endpoint taking an injected async HTTP client (ReactPHPBrowser, 10 s timeout, no redirects, by default) and returns promises. NewAsyncEndpointInterface;Server::handleReactreturns the promise for such endpoints and maps a rejection to 500. The synchronoushandle()/handleEndpoint()adapter still serves login, logout and cached profile reads and answers 503 for anything needing a provider call. A code is exchanged once per login (state is consumed). The Discord revocation URL is corrected to/oauth2/token/revoke(docs).init(null, true)emitsE_USER_DEPRECATED; README documents its limits.Compatibility with Civilizationbot
setDiscordOAuth2Endpoint/setSS14OAuth2Endpointsignatures andgetSessions()are unchanged; the verified-list endpoints are untouched. The only behavioural change the bot can observe is the session key (cookie id instead of IP), which its code treats as an opaque string. A test replays its correlation lookup. The Civ14 OAuth path is commented out in the bot today, so nothing live depends on it.Verification
New
tests/isolated/OAuth2SessionTest.phpuses a canned provider (no network): per-browser cookies and states, state check before any provider call, profile isolation between browsers, Discord/SS14 correlation by cookie, expiry and forged cookies, pending promises throughhandleReact, the 503 adapter path, and provider failure as 500. Not verified: a real Discord/SS14 login round trip.Generated by Claude Code