fix(client): persist cookies so the MFA session survives calls

admin_client.go

@@ -151,12 +151,12 @@ func (c *AuthorizerAdminClient) execute(spec adminMethodSpec, out interface{}) e
 		}
 		if spec.restResponse != nil {
 			msg := spec.restResponse()
-			if err := doREST(c.AuthorizerURL, spec.restMethod, spec.restPath, spec.restBody, c.ExtraHeaders, map[string]string{adminSecretHeader: c.AdminSecret}, msg); err != nil {
+			if err := doREST(nil, c.AuthorizerURL, spec.restMethod, spec.restPath, spec.restBody, c.ExtraHeaders, map[string]string{adminSecretHeader: c.AdminSecret}, msg); err != nil {
 				return err
 			}
 			return unwrapProto(msg, spec.responseUnwrap, out)
 		}
-		return doREST(c.AuthorizerURL, spec.restMethod, spec.restPath, spec.restBody, c.ExtraHeaders, map[string]string{adminSecretHeader: c.AdminSecret}, out)
+		return doREST(nil, c.AuthorizerURL, spec.restMethod, spec.restPath, spec.restBody, c.ExtraHeaders, map[string]string{adminSecretHeader: c.AdminSecret}, out)
 
 	case ProtocolGRPC:
 		if spec.grpcCall == nil {

client.go

@@ -4,7 +4,13 @@ package authorizer
 
 import (
 	"fmt"
+	"net/http"
+	"net/http/cookiejar"
+	"net/url"
 	"strings"
+	"time"
+
+	"google.golang.org/grpc"
 )
 
 // AuthorizerClient defines the attributes required to initiate authorizer client
@@ -19,6 +25,48 @@ type AuthorizerClient struct {
 	// GRPCEndpoint overrides the host:port dialed when Protocol is grpc. When
 	// empty it is derived from AuthorizerURL using the gRPC default port.
 	GRPCEndpoint string
+
+	// httpClient is shared across every call on this client and carries a
+	// cookie jar. The jar is REQUIRED, not an optimisation: the MFA offer flow
+	// identifies the pending user by a session cookie the server sets on
+	// signup/login, and SkipMfaSetup / VerifyOtp only resolve it if that cookie
+	// is sent back. Building a fresh http.Client per call, as this SDK used to,
+	// dropped the cookie and made those calls fail with "invalid session" —
+	// i.e. the whole MFA surface was unreachable from Go.
+	httpClient *http.Client
+}
+
+// newHTTPClient builds the shared cookie-aware client. cookiejar.New with a nil
+// options value never returns an error, but the error is handled rather than
+// ignored so a future options change cannot silently produce a jar-less client.
+func newHTTPClient() *http.Client {
+	jar, err := cookiejar.New(nil)
+	if err != nil {
+		return &http.Client{Timeout: 30 * time.Second}
+	}
+	return &http.Client{Timeout: 30 * time.Second, Jar: jar}
+}
+
+// HTTPClient returns the shared cookie-aware http client, initialising it on
+// first use so a zero-value AuthorizerClient (or one built by an older
+// constructor path) still carries a jar.
+func (c *AuthorizerClient) HTTPClient() *http.Client {
+	if c.httpClient == nil {
+		c.httpClient = newHTTPClient()
+	}
+	return c.httpClient
+}
+
+// dialGRPC opens a gRPC connection whose calls share this client's cookie jar,
+// so a session established over gRPC (or over HTTP) survives the next call.
+func (c *AuthorizerClient) dialGRPC() (*grpc.ClientConn, error) {
+	jar := c.HTTPClient().Jar
+	u, err := url.Parse(c.AuthorizerURL)
+	if jar == nil || err != nil || u.Host == "" {
+		return grpcDial(c.AuthorizerURL, c.GRPCEndpoint)
+	}
+	return grpcDial(c.AuthorizerURL, c.GRPCEndpoint,
+		grpc.WithChainUnaryInterceptor(cookieInterceptor(jar, u)))
 }
 
 // ClientOption customizes an AuthorizerClient at construction time.
@@ -76,6 +124,7 @@ func NewAuthorizerClient(clientID, authorizerURL, redirectURL string, extraHeade
 		ClientID:      clientID,
 		ExtraHeaders:  headers,
 		Protocol:      ProtocolGraphQL,
+		httpClient:    newHTTPClient(),
 	}
 	for _, opt := range opts {
 		opt(c)

cookie_jar_test.go

@@ -0,0 +1,162 @@
+package authorizer
+
+import (
+	"context"
+	"net/http"
+	"net/http/cookiejar"
+	"net/http/httptest"
+	"net/url"
+	"strings"
+	"testing"
+
+	"google.golang.org/grpc"
+	"google.golang.org/grpc/metadata"
+)
+
+// TestClientPersistsCookiesAcrossCalls is the regression test for the MFA
+// offer flow.
+//
+// Since server 2.4.0, MFA is on by default: signup/login withhold the access
+// token and return "Proceed to mfa setup", identifying the pending user by a
+// session cookie. SkipMfaSetup and VerifyOtp resolve that user ONLY if the
+// cookie is sent back.
+//
+// This SDK previously built a fresh http.Client per call, so the cookie was
+// dropped between them and every one of those calls failed with "invalid
+// session" — the entire MFA surface was unreachable from Go, while the methods
+// existed and looked correct.
+func TestClientPersistsCookiesAcrossCalls(t *testing.T) {
+	var secondRequestCookie string
+
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if c, err := r.Cookie("mfa_session"); err == nil {
+			secondRequestCookie = c.Value
+		}
+		http.SetCookie(w, &http.Cookie{Name: "mfa_session", Value: "session-abc", Path: "/"})
+		w.Header().Set("Content-Type", "application/json")
+		_, _ = w.Write([]byte(`{"data":{"ok":true}}`))
+	}))
+	defer srv.Close()
+
+	c, err := NewAuthorizerClient("test-client", srv.URL, "", nil)
+	if err != nil {
+		t.Fatalf("new client: %v", err)
+	}
+
+	// First call receives the cookie.
+	if _, err := c.ExecuteGraphQL(&GraphQLRequest{Query: "{ __typename }"}, nil); err != nil {
+		t.Fatalf("first call: %v", err)
+	}
+	// Second call must send it back.
+	if _, err := c.ExecuteGraphQL(&GraphQLRequest{Query: "{ __typename }"}, nil); err != nil {
+		t.Fatalf("second call: %v", err)
+	}
+
+	if secondRequestCookie != "session-abc" {
+		t.Fatalf("cookie not replayed on the second call (got %q) — "+
+			"SkipMfaSetup/VerifyOtp will fail with \"invalid session\"", secondRequestCookie)
+	}
+}
+
+// TestRESTTransportPersistsCookies pins the same contract for the REST
+// transport, which kept using http.DefaultClient — jar-less — after the jar was
+// added, so MFA over rest failed with "invalid session" while graphql worked.
+func TestRESTTransportPersistsCookies(t *testing.T) {
+	var secondRequestCookie string
+
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if c, err := r.Cookie("mfa_session"); err == nil {
+			secondRequestCookie = c.Value
+		}
+		http.SetCookie(w, &http.Cookie{Name: "mfa_session", Value: "session-abc", Path: "/"})
+		w.Header().Set("Content-Type", "application/json")
+		_, _ = w.Write([]byte(`{}`))
+	}))
+	defer srv.Close()
+
+	c, err := NewAuthorizerClient("test-client", srv.URL, "", nil, WithProtocol(ProtocolREST))
+	if err != nil {
+		t.Fatalf("new client: %v", err)
+	}
+	for i := 0; i < 2; i++ {
+		if err := c.executeREST(http.MethodPost, "/v1/signup", map[string]string{}, nil, nil); err != nil {
+			t.Fatalf("rest call %d: %v", i, err)
+		}
+	}
+
+	if secondRequestCookie != "session-abc" {
+		t.Fatalf("cookie not replayed on the second REST call (got %q)", secondRequestCookie)
+	}
+}
+
+// TestGRPCCookieInterceptor pins the gRPC half of the same contract: the
+// server hands cookies out as `set-cookie` header metadata and expects them
+// back as a `cookie` entry, so without this the MFA flow is unreachable over
+// gRPC. The invoker is stubbed — the contract under test is metadata handling,
+// not the wire.
+func TestGRPCCookieInterceptor(t *testing.T) {
+	u, _ := url.Parse("http://authorizer.test")
+	jar, err := cookiejar.New(nil)
+	if err != nil {
+		t.Fatalf("jar: %v", err)
+	}
+	intercept := cookieInterceptor(jar, u)
+
+	// First call: the server sets a cookie, the client sends none.
+	var sentFirst []string
+	err = intercept(context.Background(), "/Signup", nil, nil, nil,
+		func(ctx context.Context, method string, req, reply interface{}, cc *grpc.ClientConn, opts ...grpc.CallOption) error {
+			md, _ := metadata.FromOutgoingContext(ctx)
+			sentFirst = md.Get("cookie")
+			setHeader(opts, metadata.Pairs("set-cookie", "mfa_session=session-abc; Path=/"))
+			return nil
+		})
+	if err != nil {
+		t.Fatalf("first call: %v", err)
+	}
+	if len(sentFirst) != 0 {
+		t.Errorf("expected no cookie metadata on the first call, got %v", sentFirst)
+	}
+
+	// Second call: the stored cookie must be replayed.
+	var sentSecond []string
+	err = intercept(context.Background(), "/SkipMfaSetup", nil, nil, nil,
+		func(ctx context.Context, method string, req, reply interface{}, cc *grpc.ClientConn, opts ...grpc.CallOption) error {
+			md, _ := metadata.FromOutgoingContext(ctx)
+			sentSecond = md.Get("cookie")
+			return nil
+		})
+	if err != nil {
+		t.Fatalf("second call: %v", err)
+	}
+	if len(sentSecond) != 1 || !strings.Contains(sentSecond[0], "mfa_session=session-abc") {
+		t.Fatalf("cookie not replayed over grpc (got %v) — SkipMfaSetup will fail with \"invalid session\"", sentSecond)
+	}
+}
+
+// setHeader writes response header metadata the way a real grpc call does, by
+// filling the grpc.Header(&md) call option the interceptor appended.
+func setHeader(opts []grpc.CallOption, md metadata.MD) {
+	for _, o := range opts {
+		if h, ok := o.(grpc.HeaderCallOption); ok {
+			*h.HeaderAddr = md
+		}
+	}
+}
+
+// TestHTTPClientAlwaysHasAJar pins that no construction path yields a
+// jar-less client, including a zero-value struct built without the constructor.
+func TestHTTPClientAlwaysHasAJar(t *testing.T) {
+	c, err := NewAuthorizerClient("test-client", "http://localhost:8080", "", nil)
+	if err != nil {
+		t.Fatalf("new client: %v", err)
+	}
+	if c.HTTPClient().Jar == nil {
+		t.Fatal("constructor produced a client with no cookie jar")
+	}
+
+	var zero AuthorizerClient
+	if zero.HTTPClient().Jar == nil {
+		t.Fatal("zero-value client must lazily gain a cookie jar")
+	}
+}

execute.go

@@ -91,7 +91,7 @@ func (c *AuthorizerClient) execute(spec methodSpec, headers map[string]string, o
 		if spec.grpcCall == nil {
 			return unsupportedProtocol(spec.name, c.Protocol, spec.supported())
 		}
-		conn, err := grpcDial(c.AuthorizerURL, c.GRPCEndpoint)
+		conn, err := c.dialGRPC()
 		if err != nil {
 			return err
 		}

get_token.go

@@ -8,7 +8,6 @@ import (
 	"net/http"
 	"net/url"
 	"strings"
-	"time"
 )
 
 // GetTokenRequest defines attributes for token request. Only the set (non-nil)
@@ -116,7 +115,7 @@ func (c *AuthorizerClient) GetToken(req *GetTokenRequest) (*TokenResponse, error
 		}
 	}
 
-	client := http.Client{Timeout: 30 * time.Second}
+	client := c.HTTPClient()
 	res, err := client.Do(httpReq)
 	if err != nil {
 		return nil, err

graphql.go

@@ -7,7 +7,6 @@ import (
 	"io"
 	"net/http"
 	"net/url"
-	"time"
 )
 
 // GraphQLRequest is object used to make graphql queries
@@ -32,7 +31,7 @@ func (c *AuthorizerClient) ExecuteGraphQL(req *GraphQLRequest, headers map[strin
 		return nil, err
 	}
 
-	client := http.Client{Timeout: 30 * time.Second}
+	client := c.HTTPClient()
 	httpReq, err := http.NewRequest(http.MethodPost, c.AuthorizerURL+"/graphql", bytes.NewReader(jsonReq))
 	if err != nil {
 		return nil, err

protocol.go

@@ -4,12 +4,14 @@ import (
 	"context"
 	"crypto/tls"
 	"fmt"
+	"net/http"
 	"net/url"
 	"strings"
 
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/credentials"
 	"google.golang.org/grpc/credentials/insecure"
+	"google.golang.org/grpc/metadata"
 )
 
 // Protocol selects the wire transport a client uses to talk to authorizer.
@@ -40,7 +42,7 @@ const defaultGRPCPort = "9091"
 // not the HTTP URL's port. An https:// URL (or an explicit :443 host) uses TLS;
 // everything else dials insecurely, matching the typical self-hosted
 // http://host:8080 deployment.
-func grpcDial(authorizerURL, grpcEndpoint string) (*grpc.ClientConn, error) {
+func grpcDial(authorizerURL, grpcEndpoint string, opts ...grpc.DialOption) (*grpc.ClientConn, error) {
 	u, err := url.Parse(authorizerURL)
 	if err != nil {
 		return nil, fmt.Errorf("invalid authorizerURL %q: %w", authorizerURL, err)
@@ -67,7 +69,36 @@ func grpcDial(authorizerURL, grpcEndpoint string) (*grpc.ClientConn, error) {
 		host += ":443"
 	}
 
-	return grpc.NewClient(host, grpc.WithTransportCredentials(creds))
+	return grpc.NewClient(host, append([]grpc.DialOption{grpc.WithTransportCredentials(creds)}, opts...)...)
+}
+
+// cookieInterceptor carries the session cookie across gRPC calls. gRPC has no
+// cookie concept, so the server sends its cookies as `set-cookie` header
+// metadata and reads them back from a `cookie` metadata entry. Without this the
+// MFA offer flow is unreachable over gRPC for the same reason it was over
+// HTTP before the jar existed: signup/login hand out an MFA session the next
+// call never replays, and SkipMfaSetup/VerifyOtp answer "invalid session".
+// Cookies are stored in the client's shared jar, so a session started over one
+// protocol is usable from another.
+func cookieInterceptor(jar http.CookieJar, u *url.URL) grpc.UnaryClientInterceptor {
+	return func(ctx context.Context, method string, req, reply interface{}, cc *grpc.ClientConn, invoker grpc.UnaryInvoker, opts ...grpc.CallOption) error {
+		if cookies := jar.Cookies(u); len(cookies) > 0 {
+			pairs := make([]string, 0, len(cookies))
+			for _, c := range cookies {
+				pairs = append(pairs, c.Name+"="+c.Value)
+			}
+			ctx = metadata.AppendToOutgoingContext(ctx, "cookie", strings.Join(pairs, "; "))
+		}
+
+		var header metadata.MD
+		err := invoker(ctx, method, req, reply, cc, append(opts, grpc.Header(&header))...)
+		// Store cookies even when the call failed: a rejected MFA attempt can
+		// still rotate the session.
+		if set := header.Get("set-cookie"); len(set) > 0 {
+			jar.SetCookies(u, (&http.Response{Header: http.Header{"Set-Cookie": set}}).Cookies())
+		}
+		return err
+	}
 }
 
 // stripPort removes a trailing :port from host, leaving the bare host.

revoke_token.go

@@ -29,7 +29,7 @@ func (c *AuthorizerClient) RevokeToken(req *RevokeTokenInput) (*Response, error)
 		return nil, err
 	}
 
-	client := http.Client{}
+	client := c.HTTPClient()
 	httpReq, err := http.NewRequest(http.MethodPost, c.AuthorizerURL+"/oauth/revoke", bytes.NewReader(jsonReq))
 	if err != nil {
 		return nil, err