fix: persist MFA session cookie so skip_mfa_setup works - #11
Open
lakhansamani wants to merge 1 commit into
Open
Conversation
Server >= 2.4.0 has MFA on by default: signup/login withhold the access token and open an MFA session identified ONLY by the mfa_session cookie. skip_mfa_setup (and the *_mfa_setup calls) could not redeem that token. Two independent cookie failures: * http/localhost: the server sets the cookie Secure (--app-cookie-secure defaults to true) and Domain=localhost. http.cookiejar refuses Secure cookies on http:// and rewrites a dotless host to "localhost.local", which never matches Domain=localhost, so the cookie was stored but never sent. Browsers send it (loopback is a trustworthy origin), and so does curl. Normalise loopback cookies in the jar rather than installing a CookiePolicy: httpx rebuilds the outgoing jar with the default policy on every request (BaseClient._merge_cookies). * grpc: the transport had no cookie handling at all. The server encodes cookies as set-cookie response metadata and reads them back from a cookie entry; capture and replay them per client. Live suite is MFA-aware now; it was asserting the pre-2.4.0 contract that signup returns a token (36 of 84 failing against 2.4.0).
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.
The bug
Since Authorizer 2.4.0 MFA is on by default.
signup/loginnow withhold the access token and answer:The token is redeemed by calling
skip_mfa_setup, and the MFA session it belongs to is identified only by themfa_sessioncookie. The SDK never sent that cookie back, soskip_mfa_setup(andtotp_mfa_setup/email_otp_mfa_setup/sms_otp_mfa_setup/lock_mfa/webauthn_registration_verify, which use the same session) always failed:User-visible impact: against a 2.4.0+ server, every Python SDK user is locked out after signup or login. There is no access token in the response and no way to obtain one — the SDK is unusable for its primary purpose.
The response parsing was fine (
AuthTokenalready carriesmessage,should_show_totp_screen, theshould_offer_*_mfa_setupflags), andhttpx.Clientdoes keep a cookie jar. The failure was in getting the cookie back out, in two independent places.1. http/localhost (graphql + rest)
The server sets the cookie
Secure(--app-cookie-securedefaults totrue) withDomain=localhost, even over plain http. Twohttp.cookiejarrules then drop it:Securecookies are never sent to anhttp://URL;eff_request_hostrewrites a dotless host tolocalhost.local, which never domain-matchesDomain=localhost.Browsers send the cookie in both cases — W3C secure contexts treat loopback as a trustworthy origin — and so does
curl. Only the Python SDK dropped it, which is why every local dev setup and every CI run against a local server broke.The fix normalises the stored cookie for loopback origins instead of installing a
CookiePolicy, because httpx rebuilds the outgoing jar with the default policy on every request (BaseClient._merge_cookiesdoesCookies(self.cookies)), silently discarding any custom policy. Non-loopback cookies are untouched: aSecurecookie from a real host is still never sent over plain http (covered by a test).2. gRPC — no cookie handling at all
_grpc_transportignored response cookies entirely. gRPC has no cookie concept, so the server serialises them asset-cookieresponse metadata and reads them back from acookiemetadata entry (internal/grpcsrv/transport/grpc_metadata.go). Neither half was implemented, so the whole cookie-bound surface was dead overprotocol="grpc". Cookies are now captured and replayed per client instance.Verification
Against a real server (2.4.0, sqlite, default
--app-cookie-secure),signup-> MFA offer ->skip_mfa_setup->get_profile:All four clients (sync/async, public/admin) fixed.
Test suite, same server,
graphql,rest,grpc:-m "not live")The 36 live failures were pre-existing (identical on
main): the suite asserted the pre-2.4.0 contract thatsignupreturns a token. It now completes the MFA offer viaskip_mfa_setup, which also exercises the fix end to end on every protocol.ruffandmypy --strictclean.Notes
tests/test_cookies.pyis the regression test; the localhost case fails without the fix (verified) and covers gRPC metadata round-tripping plus the "don't downgrade Secure for real hosts" guard.skip_mfa_setupmust be called on the same client instance that did the login/signup.