fix(client): persist cookies so the MFA session survives calls - #27
Open
lakhansamani wants to merge 2 commits into
Open
fix(client): persist cookies so the MFA session survives calls#27lakhansamani wants to merge 2 commits into
lakhansamani wants to merge 2 commits into
Conversation
Each call built its own http.Client with no jar, so the session cookie the server sets on signup/login was dropped before the next request. Since server 2.4.0 MFA is on by default: signup/login withhold the access token, return "Proceed to mfa setup", and identify the pending user by that cookie. SkipMfaSetup and VerifyOtp resolve it only if the cookie comes back, so both failed with "invalid session" — the whole MFA surface was unreachable from Go while the methods existed and read as correct. Share one cookie-aware client on AuthorizerClient, exposed via HTTPClient() so a zero-value struct lazily gains a jar too.
The cookie jar landed on the GraphQL transport only. REST kept using http.DefaultClient (jar-less) and gRPC had no cookie handling at all, so the MFA session set by signup/login was dropped on both and SkipMfaSetup answered "invalid session" — the MFA surface stayed unreachable over two of three protocols. REST now uses the client's shared http.Client. gRPC gets a unary interceptor that replays jar cookies as `cookie` metadata and stores the server's `set-cookie` header metadata back into the jar, which is the cookie bridge the server's grpc transport already expects. test: resolve the default MFA offer in the integration suite Since server 2.4.0 MFA is on by default: signup/login withhold the access token and offer enrollment. The suite assumed the old behaviour and had 16 failures across graphql/rest/grpc. Shared signUp/login helpers decline the offer via SkipMfaSetup and return the withheld token; a response that is not an MFA offer passes through untouched, so a genuinely missing token still fails.
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.
Problem
Every call built its own
http.Clientwith no cookie jar, so the session cookie set by the server on signup/login was dropped before the next request.Since server 2.4.0, MFA is on by default: signup/login withhold the access token, return
"Proceed to mfa setup", and identify the pending user by that cookie.SkipMfaSetupandVerifyOtpresolve it only if the cookie is replayed.So both failed with
invalid session— the entire MFA surface was unreachable from Go, while the methods existed and read as correct. Found by runningexamples/with-goagainst a default 2.4.0 server.Fix
One shared cookie-aware
http.ClientonAuthorizerClient, used by the GraphQL, token and revoke paths. Exposed viaHTTPClient(), which lazily initialises so a zero-value struct or an older construction path still gets a jar.Tests
TestClientPersistsCookiesAcrossCalls— httptest server sets a cookie on call 1 and asserts it is replayed on call 2. Verified to fail with the jar removed, reporting the exactinvalid sessionsymptom.TestHTTPClientAlwaysHasAJar— no construction path yields a jar-less client.Verification
go build·go vet·go test ./...all pass. End-to-end against a live 2.4.0 server, signup → MFA offer → skip → token → profile → admin now completes.