refactor(cookies): replace the cookie jar with the store gRPC already uses - #15
Merged
Merged
Conversation
… uses The SDK was emulating a user agent to solve a problem that needs two lines. http.cookiejar implements domain matching, public-suffix rules and a policy layer; an SDK talks to exactly ONE origin, fixed at construction, so every one of those rules is either inapplicable or a source of breakage. It supplied both: Secure cookies dropped on http://localhost, dotless-domain mismatch, and a set_ok-vs-set_cookie layering that made the first fix silently do nothing on Python 3.9/3.10. The replacement is not new — the gRPC transport has always carried cookies as a plain name->value dict, captured from `set-cookie` metadata and replayed as one `cookie` entry. HTTP now uses that same store, so the two transports share one implementation and one cookie store per client (a client that switches protocol keeps its session). What the jar was doing that still has to be done, now done directly: - Secure is still honoured (RFC 6265 4.1.2.5): a Secure cookie is not stored when the client's own origin is plain http and not loopback. Loopback is excepted because Secure Contexts makes it trustworthy — the same call Chrome, Firefox and Go's stdlib jar make, and RFC 6265bis 5.6 explicitly delegates to the user agent. - Max-Age<=0 still deletes, so logout/DeleteMfaSession clear the handle rather than leaving a dead one replayed forever. - Origin scoping is now structural rather than policy: every RequestSpec is built from config.authorizer_url, so there is no path that attaches the store to another host. Pinned by a test that fails if one appears. The cookie-injection hole the jar shim introduced cannot recur: with one origin there is no second host to accept a cookie from. Verified: ruff, mypy and pytest clean on 3.9-3.13, and a real MFA flow (signup withholds -> skip_mfa_setup redeems) plus the 56-test live integration suite against a running server.
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.
Follow-up to #11. That PR added a cookie jar; this removes the need for one.
The insight
The gRPC transport in this SDK has always carried cookies as a plain
dict[str, str]— captured fromset-cookiemetadata, replayed as onecookieentry. No jar, no policy. Meanwhile the HTTP transports usedhttp.cookiejar, which implements a user agent: domain matching, public-suffix rules, a policy layer.An SDK talks to exactly one origin, fixed at construction. Every one of those rules is either inapplicable or a source of breakage — and it supplied both:
Securecookies dropped overhttp://localhostlocalhostvslocalhost.local)set_ok-vs-set_cookielayering that made fix: persist MFA session cookie so skip_mfa_setup works #11's first fix silently do nothing on Python 3.9/3.10HTTP now uses the same store gRPC already had. One implementation, one cookie store per client — so a client that switches protocol keeps its session.
What the jar was doing that still gets done
Secureis still honoured (RFC 6265 §4.1.2.5): not stored when the client's own origin is plain http and not loopback. Loopback is excepted because Secure Contexts makes it trustworthy — the same call Chrome, Firefox and Go's stdlib jar make, and RFC 6265bis §5.6 explicitly delegates it to the user agent.Max-Age<=0still deletes, so logout /DeleteMfaSessionclear the handle instead of leaving a dead one replayed forever.RequestSpecis built fromconfig.authorizer_url, so no code path attaches the store to another host. Pinned by a test that fails if one appears.The cookie-injection hole #11's shim introduced cannot recur — with one origin there is no second host to accept a cookie from.
Verification
ruff,mypy,pytestclean on 3.9, 3.10, 3.11, 3.12, 3.13skip_mfa_setupredeems → token issuedServer-side contract documented in authorizerdev/docs#93.