镜像站点 · 本页由第三方 GitHub 只读镜像提供,非 GitHub 官方站点,不接受任何登录或凭据输入。前往 github.com
Skip to content

mcp: Client.Connect falls back to initialize after a 401 on server/discover, running the authorization flow twice #1423

Open

Description

@clearly

Describe the bug

With an OAuthHandler on StreamableClientTransport, Client.Connect sends server/discover first. If that request is still unauthorized after Authorize ran, or Authorize itself fails, Connect treats the failure as "legacy server" and falls back to initialize (mcp/client.go:376-378: "fall back to the legacy initialize handshake on any non-modern error"). The initialize POST gets the same 401, so the transport calls OAuthHandler.Authorize a second time (mcp/streamable.go:2325).

With auth.AuthorizationCodeHandler, each Authorize is a full flow: PRM and AS discovery, registration, and a browser consent prompt. So one Connect shows the user two consent prompts. Before #1419 (merged today) it also created two DCR clients at the AS. #1419 now reuses the registration, but the second interactive authorization still happens on main. It happens whenever the first authorization doesn't produce a token the server accepts:

  • the user denies or cancels consent (Authorize returns an error), and is asked again immediately;
  • the AS issues a token the resource server rejects (we hit this with an AS that ignores RFC 8707 resource, so the token has no audience and a strict RS rejects it);
  • the account lacks access.

A 401/403 isn't an era signal. The 2026-07-28 Streamable HTTP binding says a client should inspect a 400 body before falling back. A legacy server that requires auth would 401 initialize just the same, so falling back can't help and only repeats the prompt.

To Reproduce

package main

import (
	"context"
	"errors"
	"fmt"
	"net/http"
	"net/http/httptest"
	"sync/atomic"

	"github.com/modelcontextprotocol/go-sdk/mcp"
	"golang.org/x/oauth2"
)

// countingHandler stands in for auth.AuthorizationCodeHandler; each Authorize
// call represents one full flow (discovery, registration, browser consent).
type countingHandler struct {
	authorizeCalls atomic.Int32
	fail           bool // simulate the user denying consent
}

func (h *countingHandler) TokenSource(context.Context) (oauth2.TokenSource, error) {
	return oauth2.StaticTokenSource(&oauth2.Token{AccessToken: "example-token", TokenType: "Bearer"}), nil
}

func (h *countingHandler) Authorize(ctx context.Context, req *http.Request, resp *http.Response) error {
	n := h.authorizeCalls.Add(1)
	fmt.Printf("  Authorize #%d for JSON-RPC method %q\n", n, req.Header.Get("Mcp-Method"))
	if h.fail {
		return errors.New("access_denied: user declined consent")
	}
	return nil
}

func run(name string, fail bool) {
	// Resource server that rejects every token.
	rs := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		w.Header().Set("WWW-Authenticate", `Bearer error="invalid_token", resource_metadata="https://rs.example/.well-known/oauth-protected-resource"`)
		w.WriteHeader(http.StatusUnauthorized)
	}))
	defer rs.Close()

	h := &countingHandler{fail: fail}
	c := mcp.NewClient(&mcp.Implementation{Name: "repro", Version: "0.0.1"}, nil)
	fmt.Println(name)
	_, err := c.Connect(context.Background(), &mcp.StreamableClientTransport{Endpoint: rs.URL, OAuthHandler: h}, nil)
	fmt.Printf("  Connect error: %v\n  Authorize calls: %d (want 1)\n", err, h.authorizeCalls.Load())
}

func main() {
	run("case 1: authorization succeeds but the token is still rejected", false)
	run("case 2: authorization fails (user denies consent)", true)
}

Actual behavior (v1.8.0 and main @ 3ebc6d1)

case 1: authorization succeeds but the token is still rejected
  Authorize #1 for JSON-RPC method "server/discover"
  Authorize #2 for JSON-RPC method ""
  Connect error: calling "initialize": sending "initialize": Unauthorized
  Authorize calls: 2 (want 1)
case 2: authorization fails (user denies consent)
  Authorize #1 for JSON-RPC method "server/discover"
  Authorize #2 for JSON-RPC method ""
  Connect error: calling "initialize": sending "initialize": rejected by transport: access_denied: user declined consent
  Authorize calls: 2 (want 1)

(The second call is the initialize POST, which carries no Mcp-Method header.)

Expected behavior

One Authorize call. Connect returns the server/discover authorization error, such as "Unauthorized" or the handler's access_denied, without trying initialize. The error is also more accurate: today it names initialize, a method the modern server never needed.

Environment

References

Suggested fix

In Client.Connect, don't fall back when the server/discover failure is an authorization failure. Return it instead. Concretely:

  • In streamableClientConn.Write, when the response is still 401/403 after Authorize (or Authorize returned an error), return an error that wraps an identifiable sentinel or type. That could be unexported, or a new exported ErrUnauthorized via a proposal.
  • In Connect, if errors.Is(err, thatSentinel), cs.Close() and return the error. Don't run the initialize fallback.

A legacy server behind auth still works. The first 401 runs Authorize, the retried server/discover reaches the legacy server with a valid token and gets its non-modern 400/404, and the fallback proceeds as today, so only one authorization happens. Tests: the two cases above should each assert exactly one Authorize call, plus a "legacy server behind auth" case that still connects through initialize.

Additional context

Our workaround is to run a single pre-flight authorization (401 → PRM → Authorize) before calling Connect. If it doesn't yield an accepted token, we don't call Connect at all.

Activity

  1. guglielmo-san commented on Oct 21, 2026

    @guglielmo-san
    Contributor

    @clearly thanks for the detailed report and reproducer!

    I agree we should never trigger OAuthHandler.Authorize twice during a single Client.Connect, but I think we need to be careful about skipping the initialize fallback altogether when the retried server/discover request returns 401 or 403.

    There are two distinct cases:

    1. OAuthHandler.Authorize itself returns an error :
      Currently in streamableClientConn.Write, c.failConnection(...) is only called if ctx.Err() is context.Canceled or context.DeadlineExceeded, and the error is wrapped with jsonrpc2.ErrRejected. That leaves the connection open, so Connect falls back to initialize and runs Authorize a second time. In this case we should not fall back to initialize.

    2. OAuthHandler.Authorize succeeds (a token is now in TokenSource), but the retried server/discover still returns 401 or 403:
      At this point the client has valid credentials in TokenSource. A server may reject server/discover with 403 (or 401) because server/discover is not in its method allowlist or has different per-method authorization rules, while those same credentials do have access to initialize.

      • If we abort Connect immediately after server/discover fails, we never even try initialize with the token we just acquired.
      • Instead, if we still allow the fallback to initialize, the first initialize request will automatically carry the token from TokenSource. If the user's credentials have access to initialize, it will succeed (200 OK) without calling Authorize again.
      • The second Authorize call only happens when initialize (with that token) also returns 401/403, because streamableClientConn does not know that Authorize already ran during server/discover.

    So rather than blocking the initialize fallback when the retried server/discover gets 401/403, I think the right fix is:

    • If Authorize itself returns an error during server/discover, do not fall back to initialize (return the error immediately).
    • If Authorize succeeds during server/discover and the retried server/discover still fails (including with 401/403), still fall back to initialize using the acquired token, but do not run Authorize a second time if initialize also fails with 401.

    What do you think about this approach?

  2. added a commit that references this issue on Oct 22, 2026
    5e069e9
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions