diff --git a/cmd/root/debug_oauth.go b/cmd/root/debug_oauth.go index 8118fbf1a9..047032d742 100644 --- a/cmd/root/debug_oauth.go +++ b/cmd/root/debug_oauth.go @@ -181,19 +181,19 @@ func newDebugOAuthLoginCmd() *cobra.Command { return err } - serverURL, err := findMCPRemoteURL(cfg, mcpName) + remote, err := findMCPRemote(cfg, mcpName) if err != nil { return err } w := cmd.OutOrStdout() - fmt.Fprintf(w, "Starting OAuth login for %s (%s)...\n", mcpName, serverURL) + fmt.Fprintf(w, "Starting OAuth login for %s (%s)...\n", mcpName, remote.URL) - if err := mcp.PerformOAuthLogin(ctx, serverURL); err != nil { + if err := mcp.PerformOAuthLogin(ctx, remote); err != nil { return fmt.Errorf("OAuth login failed: %w", err) } - fmt.Fprintf(w, "✅ OAuth login successful for %s\n", serverURL) + fmt.Fprintf(w, "✅ OAuth login successful for %s\n", remote.URL) return nil }, } @@ -203,20 +203,26 @@ func newDebugOAuthLoginCmd() *cobra.Command { return cmd } -// findMCPRemoteURL looks up the remote URL for the named MCP server in the config. -// It matches by name (top-level mcps key or toolset name), by URL substring, -// or returns the only remote MCP if there is exactly one. -func findMCPRemoteURL(cfg *latest.Config, name string) (string, error) { - // Collect all remote MCP URLs with their identifiers. +// findMCPRemote looks up the full remote configuration for the named MCP +// server in the config. It matches by exact name (top-level mcps key or +// toolset name) or by exact URL equality; a URL substring or prefix does +// not match. +// +// The returned latest.Remote is passed to PerformOAuthLogin verbatim: +// Remote.URL is used as-is for the OAuth probe, resource indicator, and +// token-store key, and Remote.OAuth carries any explicit client +// credentials/scopes configured for this MCP server. +func findMCPRemote(cfg *latest.Config, name string) (latest.Remote, error) { + // Collect all remote MCP entries with their identifiers. type mcpEntry struct { - label string - url string + label string + remote latest.Remote } var all []mcpEntry for k, m := range cfg.MCPs { if m.Remote.URL != "" { - all = append(all, mcpEntry{label: k, url: m.Remote.URL}) + all = append(all, mcpEntry{label: k, remote: m.Remote}) } } for _, agent := range cfg.Agents { @@ -226,7 +232,7 @@ func findMCPRemoteURL(cfg *latest.Config, name string) (string, error) { if label == "" { label = ts.Remote.URL } - all = append(all, mcpEntry{label: label, url: ts.Remote.URL}) + all = append(all, mcpEntry{label: label, remote: ts.Remote}) } } } @@ -234,14 +240,14 @@ func findMCPRemoteURL(cfg *latest.Config, name string) (string, error) { // Exact match by name/label. for _, e := range all { if e.label == name { - return e.url, nil + return e.remote, nil } } // Exact match by URL. for _, e := range all { - if e.url == name { - return e.url, nil + if e.remote.URL == name { + return e.remote, nil } } @@ -251,7 +257,7 @@ func findMCPRemoteURL(cfg *latest.Config, name string) (string, error) { labels = append(labels, e.label) } if len(labels) > 0 { - return "", fmt.Errorf("MCP %q not found; available: %v", name, labels) + return latest.Remote{}, fmt.Errorf("MCP %q not found; available: %v", name, labels) } - return "", fmt.Errorf("MCP %q not found; no remote MCPs found in config", name) + return latest.Remote{}, fmt.Errorf("MCP %q not found; no remote MCPs found in config", name) } diff --git a/cmd/root/debug_oauth_test.go b/cmd/root/debug_oauth_test.go new file mode 100644 index 0000000000..7a2a6dd1b8 --- /dev/null +++ b/cmd/root/debug_oauth_test.go @@ -0,0 +1,164 @@ +package root + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/docker/docker-agent/pkg/config/latest" +) + +// TestFindMCPRemote_MatchesTopLevelMCPByName covers the top-level `mcps:` +// map lookup path and proves the full latest.Remote (URL and OAuth config) +// is returned, not just the URL, so PerformOAuthLogin sees the same +// explicit client credentials/scopes the runtime would use. +func TestFindMCPRemote_MatchesTopLevelMCPByName(t *testing.T) { + t.Parallel() + + oauthConfig := &latest.RemoteOAuthConfig{ClientID: "configured-client", Scopes: []string{"scope-a"}} + cfg := &latest.Config{ + MCPs: map[string]latest.MCPToolset{ + "atlassian": {Toolset: latest.Toolset{ + Type: "mcp", + Remote: latest.Remote{URL: "https://mcp.atlassian.com/v1/mcp/authv2", OAuth: oauthConfig}, + }}, + }, + } + + remote, err := findMCPRemote(cfg, "atlassian") + require.NoError(t, err) + assert.Equal(t, "https://mcp.atlassian.com/v1/mcp/authv2", remote.URL) + require.NotNil(t, remote.OAuth) + assert.Equal(t, "configured-client", remote.OAuth.ClientID) + assert.Equal(t, []string{"scope-a"}, remote.OAuth.Scopes) +} + +// TestFindMCPRemote_MatchesAgentEmbeddedToolsetByName covers the +// agent.Toolsets lookup path (an MCP toolset declared inline on an agent +// rather than in the top-level `mcps:` map). +func TestFindMCPRemote_MatchesAgentEmbeddedToolsetByName(t *testing.T) { + t.Parallel() + + cfg := &latest.Config{ + Agents: latest.Agents{ + { + Toolsets: []latest.Toolset{ + {Type: "mcp", Name: "inline-mcp", Remote: latest.Remote{URL: "https://example.test/mcp"}}, + }, + }, + }, + } + + remote, err := findMCPRemote(cfg, "inline-mcp") + require.NoError(t, err) + assert.Equal(t, "https://example.test/mcp", remote.URL) +} + +// TestFindMCPRemote_MatchesByURL proves the exact-URL lookup path works +// when the caller passes a URL instead of a name. +func TestFindMCPRemote_MatchesByURL(t *testing.T) { + t.Parallel() + + cfg := &latest.Config{ + MCPs: map[string]latest.MCPToolset{ + "atlassian": {Toolset: latest.Toolset{ + Type: "mcp", + Remote: latest.Remote{URL: "https://mcp.atlassian.com/v1/mcp/authv2"}, + }}, + }, + } + + remote, err := findMCPRemote(cfg, "https://mcp.atlassian.com/v1/mcp/authv2") + require.NoError(t, err) + assert.Equal(t, "https://mcp.atlassian.com/v1/mcp/authv2", remote.URL) +} + +// TestFindMCPRemote_NameMatchWinsOverURLMatch proves name/label matching is +// tried before URL matching, so a name that also happens to be a URL used +// by a different entry does not cause ambiguity. +func TestFindMCPRemote_NameMatchWinsOverURLMatch(t *testing.T) { + t.Parallel() + + cfg := &latest.Config{ + MCPs: map[string]latest.MCPToolset{ + "first": {Toolset: latest.Toolset{Type: "mcp", Remote: latest.Remote{URL: "https://first.example.test/mcp"}}}, + "second": {Toolset: latest.Toolset{Type: "mcp", Remote: latest.Remote{URL: "first"}}}, + }, + } + + remote, err := findMCPRemote(cfg, "first") + require.NoError(t, err) + assert.Equal(t, "https://first.example.test/mcp", remote.URL, "the entry labeled \"first\" must win over the entry whose URL is literally \"first\"") +} + +// TestFindMCPRemote_URLPrefixDoesNotMatch proves matching is exact URL +// equality, not substring/prefix matching: a truncated form of a +// configured URL must not match and must return the not-found error. +func TestFindMCPRemote_URLPrefixDoesNotMatch(t *testing.T) { + t.Parallel() + + cfg := &latest.Config{ + MCPs: map[string]latest.MCPToolset{ + "atlassian": {Toolset: latest.Toolset{ + Type: "mcp", + Remote: latest.Remote{URL: "https://mcp.atlassian.com/v1/mcp/authv2"}, + }}, + }, + } + + _, err := findMCPRemote(cfg, "https://mcp.atlassian.com/v1/mcp") + require.Error(t, err) + assert.Contains(t, err.Error(), "https://mcp.atlassian.com/v1/mcp") + assert.Contains(t, err.Error(), "atlassian") +} + +// TestFindMCPRemote_NotFound_ListsAvailableNames covers the not-found error +// path when at least one remote MCP exists: the error must list the +// available names to help the user pick a valid one. +func TestFindMCPRemote_NotFound_ListsAvailableNames(t *testing.T) { + t.Parallel() + + cfg := &latest.Config{ + MCPs: map[string]latest.MCPToolset{ + "atlassian": {Toolset: latest.Toolset{Type: "mcp", Remote: latest.Remote{URL: "https://mcp.atlassian.com/mcp"}}}, + }, + } + + _, err := findMCPRemote(cfg, "does-not-exist") + require.Error(t, err) + assert.Contains(t, err.Error(), "does-not-exist") + assert.Contains(t, err.Error(), "atlassian") +} + +// TestFindMCPRemote_NotFound_NoRemoteMCPsInConfig covers the not-found error +// path when the config has no remote MCPs at all. +func TestFindMCPRemote_NotFound_NoRemoteMCPsInConfig(t *testing.T) { + t.Parallel() + + cfg := &latest.Config{} + + _, err := findMCPRemote(cfg, "anything") + require.Error(t, err) + assert.Contains(t, err.Error(), "no remote MCPs found in config") +} + +// TestFindMCPRemote_IgnoresNonMCPToolsetsAndEmptyRemotes proves stdio MCPs +// (Remote.URL empty) and non-mcp toolsets on agents are skipped rather than +// matched or surfaced as ambiguous candidates. +func TestFindMCPRemote_IgnoresNonMCPToolsetsAndEmptyRemotes(t *testing.T) { + t.Parallel() + + cfg := &latest.Config{ + MCPs: map[string]latest.MCPToolset{ + "stdio-mcp": {Toolset: latest.Toolset{Type: "mcp", Command: "some-binary"}}, + }, + Agents: latest.Agents{ + {Toolsets: []latest.Toolset{{Type: "shell", Name: "shell-tool"}}}, + }, + } + + _, err := findMCPRemote(cfg, "stdio-mcp") + require.Error(t, err) + assert.Contains(t, err.Error(), "no remote MCPs found in config") +} diff --git a/pkg/tools/builtin/mcpcatalog/mcpcatalog_test.go b/pkg/tools/builtin/mcpcatalog/mcpcatalog_test.go index 1d7239e7dd..c776fae95f 100644 --- a/pkg/tools/builtin/mcpcatalog/mcpcatalog_test.go +++ b/pkg/tools/builtin/mcpcatalog/mcpcatalog_test.go @@ -1756,7 +1756,10 @@ func writeJSONRPC(t *testing.T, w http.ResponseWriter, id json.RawMessage, resul // - the MCP endpoint challenges with 401 + WWW-Authenticate (or at // least surfaces a reachable origin), // - /.well-known/oauth-protected-resource is reachable (200 -// or 404 — either is fine, the WWW-Authenticate fallback covers 404), +// or 404 — either is fine: it is only the last of the ordered +// protected-resource metadata candidates pkg/tools/mcp/oauth_login.go +// walks, after the challenge's exact resource_metadata and the RFC +// 9728 §3.1 path-insertion URL), // - the authorization-server metadata advertises an HTTPS // `registration_endpoint` (Dynamic Client Registration is REQUIRED // by pkg/tools/mcp/oauth_login.go: without it docker-agent cannot diff --git a/pkg/tools/mcp/oauth.go b/pkg/tools/mcp/oauth.go index a16f22b7dc..4e3475cf45 100644 --- a/pkg/tools/mcp/oauth.go +++ b/pkg/tools/mcp/oauth.go @@ -268,6 +268,88 @@ func resourceMetadataFromWWWAuth(wwwAuth string) string { return params["resource"] } +// protectedResourceMetadataOptions configures fetchProtectedResourceMetadata. +type protectedResourceMetadataOptions struct { + // FallbackCandidateURLs are additional protected-resource metadata URLs + // tried, in order, only after the primary resourceURL 404s. Leave this + // empty for runtime callers, which must preserve the existing + // single-candidate, supplied-origin-default behavior exactly. Computing + // these candidates (RFC 9728 path-insertion, then origin-root) is the + // standalone CLI discovery flow's responsibility, not this helper's. + FallbackCandidateURLs []string + + // NotFoundIsHardError turns a 404 on the candidate it is checked against + // into a hard error (no fallback tried, metadata not defaulted) instead + // of the default "treat as empty metadata" outcome. Default-off (false) + // so every runtime call site keeps its existing 404-tolerant behavior + // unmodified. The standalone CLI sets this only when the sole candidate + // is the exact challenged resource_metadata URL: a 404 on that + // authoritative URL must stop discovery rather than silently fall + // through to a guessed authorization server. + NotFoundIsHardError bool +} + +// fetchProtectedResourceMetadata fetches and decodes RFC 9728 OAuth +// protected-resource metadata for a challenged MCP server, deduplicating +// logic shared by the runtime-managed and docker-agent-driven unmanaged +// OAuth flows. +// +// resourceURL is the primary candidate (normally the challenge's +// resource_metadata, or otherwise authServer's +// /.well-known/oauth-protected-resource). By default a 404 on a candidate +// is not an error: it is treated as an empty metadata document, matching +// current runtime behavior, and AuthorizationServers is defaulted to +// authServer below once every candidate has been tried. Callers that set +// opts.NotFoundIsHardError opt out of that tolerance: a 404 is then a hard +// error like any other non-404/non-200 response. +// +// Any decode failure or non-404/non-200 response (including another 2xx +// like 201 or 204) on any attempted candidate is a hard error: no further +// candidate is tried and the caller must not proceed to authorization-server +// discovery, DCR, or token work. +func fetchProtectedResourceMetadata(ctx context.Context, client *http.Client, resourceURL, authServer string, opts protectedResourceMetadataOptions) (protectedResourceMetadata, error) { + candidates := append([]string{resourceURL}, opts.FallbackCandidateURLs...) + + var metadata protectedResourceMetadata + for _, candidateURL := range candidates { + req, err := http.NewRequestWithContext(ctx, http.MethodGet, candidateURL, http.NoBody) + if err != nil { + return protectedResourceMetadata{}, err + } + resp, err := client.Do(req) + if err != nil { + return protectedResourceMetadata{}, err + } + + if resp.StatusCode == http.StatusNotFound { + resp.Body.Close() + if opts.NotFoundIsHardError { + return protectedResourceMetadata{}, errors.New("failed to fetch protected resource metadata") + } + continue + } + if resp.StatusCode != http.StatusOK { + _, _ = io.ReadAll(resp.Body) + resp.Body.Close() + return protectedResourceMetadata{}, errors.New("failed to fetch protected resource metadata") + } + + decodeErr := json.NewDecoder(resp.Body).Decode(&metadata) + resp.Body.Close() + if decodeErr != nil { + return protectedResourceMetadata{}, decodeErr + } + break + } + + if len(metadata.AuthorizationServers) == 0 { + slog.DebugContext(ctx, "No authorization servers in resource metadata, using auth server from WWW-Authenticate header") + metadata.AuthorizationServers = []string{authServer} + } + + return metadata, nil +} + // parseAuthParams parses the top-level auth-params across every challenge // in a WWW-Authenticate header value (RFC 7235 auth-param, as profiled by // RFC 6750 for the Bearer scheme) into a single lowercase-key map. It is @@ -1148,31 +1230,10 @@ func (t *oauthTransport) handleManagedOAuthFlow(ctx context.Context, authServer, resourceURL := cmp.Or(resourceMetadataFromWWWAuth(wwwAuth), authServer+"/.well-known/oauth-protected-resource") span.AddEvent("oauth.step", trace.WithAttributes(attribute.String("cagent.oauth.step", "fetch_protected_resource_metadata"))) - resourceReq, err := http.NewRequestWithContext(ctx, http.MethodGet, resourceURL, http.NoBody) + resourceMetadata, err := fetchProtectedResourceMetadata(ctx, t.oauthClient(), resourceURL, authServer, protectedResourceMetadataOptions{}) if err != nil { return err } - resp, err := t.oauthClient().Do(resourceReq) - if err != nil { - return err - } - defer resp.Body.Close() - - if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusNotFound { - _, _ = io.ReadAll(resp.Body) - return errors.New("failed to fetch protected resource metadata") - } - var resourceMetadata protectedResourceMetadata - if resp.StatusCode == http.StatusOK { - if err := json.NewDecoder(resp.Body).Decode(&resourceMetadata); err != nil { - return err - } - } - - if len(resourceMetadata.AuthorizationServers) == 0 { - slog.DebugContext(ctx, "No authorization servers in resource metadata, using auth server from WWW-Authenticate header") - resourceMetadata.AuthorizationServers = []string{authServer} - } oauth := &oauth{metadataClient: t.oauthClient()} span.AddEvent("oauth.step", trace.WithAttributes(attribute.String("cagent.oauth.step", "fetch_authorization_server_metadata"))) @@ -1438,31 +1499,10 @@ func (t *oauthTransport) handleUnmanagedOAuthFlow(ctx context.Context, authServe resourceURL := cmp.Or(resourceMetadataFromWWWAuth(wwwAuth), authServer+"/.well-known/oauth-protected-resource") span.AddEvent("oauth.step", trace.WithAttributes(attribute.String("cagent.oauth.step", "fetch_protected_resource_metadata"))) - resourceReq, err := http.NewRequestWithContext(ctx, http.MethodGet, resourceURL, http.NoBody) + resourceMetadata, err := fetchProtectedResourceMetadata(ctx, t.oauthClient(), resourceURL, authServer, protectedResourceMetadataOptions{}) if err != nil { return err } - resp, err := t.oauthClient().Do(resourceReq) - if err != nil { - return err - } - defer resp.Body.Close() - - if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusNotFound { - _, _ = io.ReadAll(resp.Body) - return errors.New("failed to fetch protected resource metadata") - } - var resourceMetadata protectedResourceMetadata - if resp.StatusCode == http.StatusOK { - if err := json.NewDecoder(resp.Body).Decode(&resourceMetadata); err != nil { - return err - } - } - - if len(resourceMetadata.AuthorizationServers) == 0 { - slog.DebugContext(ctx, "No authorization servers in resource metadata, using auth server from WWW-Authenticate header") - resourceMetadata.AuthorizationServers = []string{authServer} - } oauth := &oauth{metadataClient: t.oauthClient()} span.AddEvent("oauth.step", trace.WithAttributes(attribute.String("cagent.oauth.step", "fetch_authorization_server_metadata"))) diff --git a/pkg/tools/mcp/oauth_login.go b/pkg/tools/mcp/oauth_login.go index c543067116..9215322ab9 100644 --- a/pkg/tools/mcp/oauth_login.go +++ b/pkg/tools/mcp/oauth_login.go @@ -2,63 +2,94 @@ package mcp import ( "context" - "encoding/json" "errors" "fmt" + "io" "log/slog" "net/http" "net/url" + "strings" "time" "golang.org/x/oauth2" + "github.com/docker/docker-agent/pkg/config/latest" "github.com/docker/docker-agent/pkg/httpclient" + "github.com/docker/docker-agent/pkg/tools/mcp/oauthflow" ) -// PerformOAuthLogin performs a standalone OAuth flow for the given MCP server URL. -// It discovers the authorization server metadata, performs dynamic client registration, -// opens the browser for user authorization, and stores the resulting token in the keyring. -func PerformOAuthLogin(ctx context.Context, serverURL string) error { - tokenStore := NewKeyringTokenStore() +// oauthLoginHTTPClient builds the SSRF-safe *http.Client used for every +// request PerformOAuthLogin makes (probe, protected-resource and +// authorization-server metadata discovery, DCR, and token exchange). Tests +// replace it via setOAuthLoginHTTPClientForTesting to reach httptest +// servers, which bind to 127.0.0.1 — an address this client refuses to dial +// by design. +var oauthLoginHTTPClient = func() *http.Client { + return httpclient.NewSafeClient(5*time.Second, false) +} - o := &oauth{metadataClient: httpclient.NewSafeClient(5*time.Second, false)} +// setOAuthLoginHTTPClientForTesting replaces the client PerformOAuthLogin +// uses and returns a function restoring the previous one. Never call this +// outside tests. +func setOAuthLoginHTTPClientForTesting(c *http.Client) (restore func()) { + prev := oauthLoginHTTPClient + oauthLoginHTTPClient = func() *http.Client { return c } + return func() { oauthLoginHTTPClient = prev } +} + +// PerformOAuthLogin performs a standalone OAuth flow for the given remote +// MCP server. It probes the server unauthenticated, discovers protected- +// resource and authorization-server metadata, resolves an OAuth client +// (explicit config or Dynamic Client Registration), opens the browser for +// user authorization, and stores the resulting token in the keyring. +// +// remote.URL is used verbatim as the probe target, the RFC 8707 resource +// indicator for both /authorize and the token exchange, and the +// token-store key — even when protected-resource metadata reports a +// different resource — so this command and `debug oauth remove` operate on +// the exact same token the runtime would use for the same configured +// remote. +// +// Discovery mirrors the runtime OAuth flows in oauth.go as closely as a +// non-interactive CLI command can: when the unauthenticated probe's +// challenge names an exact resource_metadata URL (as opposed to a bare +// resource identifier, which is not treated as a metadata URL), that URL is +// authoritative and no other candidate is tried — not even on a 404, which +// is a hard error for that exact candidate. Otherwise the RFC 9728 §3.1 +// path-insertion URL is tried first and the origin-root well-known URL only +// after that 404s. Any decode failure or non-404/non-200 status on an +// attempted candidate is a hard error: no later candidate, DCR, browser, or +// token request follows. +// +// The callback/redirect mechanics below (NewCallbackServer, GetRedirectURI) +// are deliberately left unchanged: aligning them with the runtime's +// NewCallbackServerOnPort/ResolveRedirectURI (and honoring +// RemoteOAuthConfig.CallbackPort/CallbackRedirectURL here) is a separate, +// not-yet-made decision. +func PerformOAuthLogin(ctx context.Context, remote latest.Remote) error { + tokenStore := NewKeyringTokenStore() + client := oauthLoginHTTPClient() - // Derive the base origin (scheme + host) from the server URL. - // The well-known endpoints live at the origin, not under the SSE/path. + serverURL := remote.URL parsed, err := url.Parse(serverURL) if err != nil { return fmt.Errorf("invalid server URL: %w", err) } - baseURL := parsed.Scheme + "://" + parsed.Host + authOrigin := parsed.Scheme + "://" + parsed.Host - // Discover protected resource metadata. - resourceURL := baseURL + "/.well-known/oauth-protected-resource" - resourceReq, err := http.NewRequestWithContext(ctx, http.MethodGet, resourceURL, http.NoBody) + wwwAuth, err := probeUnauthenticated(ctx, client, serverURL) if err != nil { - return fmt.Errorf("failed to create resource metadata request: %w", err) + return err } - resp, err := o.metadataClient.Do(resourceReq) - if err != nil { - return fmt.Errorf("failed to fetch protected resource metadata: %w", err) - } - defer resp.Body.Close() - authServer := baseURL - resourceIndicator := serverURL - if resp.StatusCode == http.StatusOK { - var resourceMetadata protectedResourceMetadata - if decErr := json.NewDecoder(resp.Body).Decode(&resourceMetadata); decErr == nil { - if len(resourceMetadata.AuthorizationServers) > 0 { - authServer = resourceMetadata.AuthorizationServers[0] - } - if resourceMetadata.Resource != "" { - resourceIndicator = resourceMetadata.Resource - } - } + resourceURL, opts := protectedResourceMetadataCandidates(parsed, authOrigin, wwwAuth) + resourceMetadata, err := fetchProtectedResourceMetadata(ctx, client, resourceURL, authOrigin, opts) + if err != nil { + return fmt.Errorf("protected resource metadata discovery for %s failed: %w", sanitizeURLForLog(resourceURL), err) } - // Discover authorization server metadata. - authServerMetadata, err := o.getAuthorizationServerMetadata(ctx, authServer) + o := &oauth{metadataClient: client} + authServerMetadata, err := o.getAuthorizationServerMetadata(ctx, resourceMetadata.AuthorizationServers[0]) if err != nil { return fmt.Errorf("failed to fetch authorization server metadata: %w", err) } @@ -84,15 +115,12 @@ func PerformOAuthLogin(ctx context.Context, serverURL string) error { redirectURI := callbackServer.GetRedirectURI() - // Dynamic client registration. - var clientID, clientSecret string - if authServerMetadata.RegistrationEndpoint != "" { - clientID, clientSecret, err = RegisterClient(ctx, authServerMetadata, redirectURI, nil) - if err != nil { - return fmt.Errorf("dynamic client registration failed: %w", err) - } - } else { - return errors.New("authorization server does not support dynamic client registration") + clientID, clientSecret, scopes, err := resolveStandaloneClientCredentials( + ctx, client, remote.OAuth, authServerMetadata, redirectURI, + challengeScopesFromWWWAuth(wwwAuth), resourceMetadata.ScopesSupported, + ) + if err != nil { + return err } // Generate PKCE and state. @@ -102,6 +130,10 @@ func PerformOAuthLogin(ctx context.Context, serverURL string) error { } callbackServer.SetExpectedState(state) verifier := GeneratePKCEVerifier() + // remote.URL verbatim, never resourceMetadata.Resource: the resource + // indicator must match the token-store key exactly (see the + // PerformOAuthLogin doc comment above). + resourceIndicator := serverURL authURL := BuildAuthorizationURL( authServerMetadata.AuthorizationEndpoint, @@ -110,7 +142,7 @@ func PerformOAuthLogin(ctx context.Context, serverURL string) error { state, oauth2.S256ChallengeFromVerifier(verifier), resourceIndicator, - nil, + scopes, ) // Open the browser and wait for the callback. @@ -124,13 +156,15 @@ func PerformOAuthLogin(ctx context.Context, serverURL string) error { } // Exchange the code for a token. - token, err := ExchangeCodeForTokenWithResource(ctx, authServerMetadata.TokenEndpoint, code, verifier, clientID, clientSecret, redirectURI, resourceIndicator) + token, err := oauthflow.ExchangeCodeForTokenWithClient(ctx, client, authServerMetadata.TokenEndpoint, code, verifier, clientID, clientSecret, redirectURI, resourceIndicator) if err != nil { return fmt.Errorf("failed to exchange code for token: %w", err) } token.ClientID = clientID token.ClientSecret = clientSecret + token.AuthServer = resourceMetadata.AuthorizationServers[0] + token.RequestedScopes = scopes if err := tokenStore.StoreToken(serverURL, token); err != nil { return fmt.Errorf("failed to store token: %w", err) @@ -138,3 +172,101 @@ func PerformOAuthLogin(ctx context.Context, serverURL string) error { return nil } + +// probeUnauthenticated issues a single unauthenticated GET request to +// serverURL and returns the response's WWW-Authenticate header, if any. +// RFC 9728 §5.1 describes a Bearer challenge carrying resource_metadata for +// the server's exact protected-resource metadata URL as how a protected MCP +// server advertises OAuth support — it is not guaranteed: a server may +// respond without that challenge (wrong method, no auth required, or a +// transport that only challenges on a real MCP request). That case yields +// an empty string here, and discovery falls back to the RFC 9728 +// path-insertion/origin-root candidates instead of erroring outright. +func probeUnauthenticated(ctx context.Context, client *http.Client, serverURL string) (string, error) { + req, err := http.NewRequestWithContext(ctx, http.MethodGet, serverURL, http.NoBody) + if err != nil { + return "", fmt.Errorf("failed to create probe request: %w", err) + } + req.Header.Set("Accept", "application/json, text/event-stream") + + resp, err := client.Do(req) + if err != nil { + return "", fmt.Errorf("failed to probe %s: %w", sanitizeURLForLog(serverURL), err) + } + defer resp.Body.Close() + _, _ = io.Copy(io.Discard, resp.Body) + + return resp.Header.Get("WWW-Authenticate"), nil +} + +// protectedResourceMetadataCandidates decides the primary protected-resource +// metadata URL and any opt-in fallback candidates for fetchProtectedResourceMetadata. +// +// It consults only the challenge's resource_metadata auth-param, never the +// bare resource auth-param (which may name the protected resource itself, +// not a metadata document, and could point the discovery GET/JSON-decode at +// the MCP endpoint). When resource_metadata names an exact URL, that URL is +// authoritative: it is returned as the sole candidate, with no fallback and +// NotFoundIsHardError set, so a 404/error on it hard-stops discovery instead +// of being papered over by guessing another URL. Otherwise the RFC 9728 +// §3.1 path-insertion URL for serverURL is tried first, with the +// origin-root well-known URL as the one fallback candidate tried only if +// the path-insertion URL 404s. +func protectedResourceMetadataCandidates(serverURL *url.URL, authOrigin, wwwAuth string) (primary string, opts protectedResourceMetadataOptions) { + if challenged := parseAuthParams(wwwAuth)["resource_metadata"]; challenged != "" { + return challenged, protectedResourceMetadataOptions{NotFoundIsHardError: true} + } + + opts.FallbackCandidateURLs = []string{authOrigin + "/.well-known/oauth-protected-resource"} + return protectedResourceMetadataPathInsertionURL(serverURL), opts +} + +// protectedResourceMetadataPathInsertionURL returns the RFC 9728 §3.1 +// path-aware protected-resource metadata URL for resourceURL: the +// well-known suffix is inserted between origin and path, e.g. +// https://mcp.atlassian.com/v1/mcp/authv2 becomes +// https://mcp.atlassian.com/.well-known/oauth-protected-resource/v1/mcp/authv2. +func protectedResourceMetadataPathInsertionURL(resourceURL *url.URL) string { + origin := resourceURL.Scheme + "://" + resourceURL.Host + path := strings.TrimSuffix(resourceURL.Path, "/") + return origin + "/.well-known/oauth-protected-resource" + path +} + +// resolveStandaloneClientCredentials picks the OAuth client_id (and optional +// secret + scopes) for the standalone CLI login. It mirrors +// oauthTransport.resolveClientCredentials' explicit-credentials/DCR selector, +// but this is a non-interactive CLI command: unlike the runtime flows, a +// missing or failing Dynamic Client Registration is a hard error rather than +// falling back to an interactive credentials prompt. +// +// challengeScopes and prmScopesSupported feed selectDCRScopes exactly as +// they do at runtime: on a successful DCR the returned scopes are the exact +// scopes requested at registration and must be reused for /authorize and +// the stored token's RequestedScopes. +func resolveStandaloneClientCredentials( + ctx context.Context, + client *http.Client, + oauthConfig *latest.RemoteOAuthConfig, + authServerMetadata *AuthorizationServerMetadata, + redirectURI string, + challengeScopes, prmScopesSupported []string, +) (clientID, clientSecret string, scopes []string, err error) { + if oauthConfig != nil && oauthConfig.ClientID != "" { + slog.DebugContext(ctx, "Using explicit OAuth credentials from config") + return oauthConfig.ClientID, oauthConfig.ClientSecret, oauthConfig.Scopes, nil + } + + if authServerMetadata.RegistrationEndpoint == "" { + return "", "", nil, errors.New("authorization server does not support dynamic client registration; configure clientId (and clientSecret, if required) to use a pre-registered client") + } + + // Select before registering so the exact same scopes go into the DCR + // request and are returned for reuse at /authorize and on the stored + // token. + requestedScopes := selectDCRScopes(configuredScopes(oauthConfig), challengeScopes, prmScopesSupported) + clientID, clientSecret, err = oauthflow.RegisterClientWithClient(ctx, client, authServerMetadata, redirectURI, requestedScopes) + if err != nil { + return "", "", nil, fmt.Errorf("dynamic client registration failed: %w", err) + } + return clientID, clientSecret, requestedScopes, nil +} diff --git a/pkg/tools/mcp/oauth_login_test.go b/pkg/tools/mcp/oauth_login_test.go new file mode 100644 index 0000000000..ff30f0741a --- /dev/null +++ b/pkg/tools/mcp/oauth_login_test.go @@ -0,0 +1,1091 @@ +package mcp + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "net/url" + "os" + "strings" + "sync" + "sync/atomic" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/docker/docker-agent/pkg/config/latest" +) + +// TestProbeUnauthenticated characterizes probeUnauthenticated: it makes one +// GET request and reports the WWW-Authenticate header verbatim, or "" when +// the server answers without an OAuth challenge. +func TestProbeUnauthenticated(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + status int + wwwAuth string + wantHeader string + }{ + { + name: "401 with Bearer challenge is captured verbatim", + status: http.StatusUnauthorized, + wwwAuth: `Bearer resource_metadata="https://res.example.test/.well-known/oauth-protected-resource"`, + wantHeader: `Bearer resource_metadata="https://res.example.test/.well-known/oauth-protected-resource"`, + }, + { + name: "200 with no auth required yields an empty challenge", + status: http.StatusOK, + wantHeader: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + var gotMethod, gotAccept string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotMethod = r.Method + gotAccept = r.Header.Get("Accept") + if tt.wwwAuth != "" { + w.Header().Set("WWW-Authenticate", tt.wwwAuth) + } + w.WriteHeader(tt.status) + })) + defer srv.Close() + + got, err := probeUnauthenticated(t.Context(), srv.Client(), srv.URL) + require.NoError(t, err) + assert.Equal(t, tt.wantHeader, got) + assert.Equal(t, http.MethodGet, gotMethod, "probe must use GET") + assert.NotEmpty(t, gotAccept, "probe must advertise an Accept header") + }) + } +} + +// TestProtectedResourceMetadataCandidates covers the discovery selector's +// branches: an exact challenged resource_metadata URL is authoritative (no +// fallback, and a 404 on it is a hard error via NotFoundIsHardError); a +// challenge carrying only a bare resource auth-param is NOT treated as a +// metadata URL and falls back to the RFC 9728 §3.1 path-insertion URL +// first, with the origin-root well-known URL as the only opt-in fallback +// (NotFoundIsHardError unset, so a 404 there is tolerated as usual). +func TestProtectedResourceMetadataCandidates(t *testing.T) { + t.Parallel() + + serverURL, err := url.Parse("https://mcp.example.test/v1/mcp/authv2") + require.NoError(t, err) + const authOrigin = "https://mcp.example.test" + + tests := []struct { + name string + wwwAuth string + wantPrimary string + wantFallbackURL []string + wantNotFoundIsHardError bool + }{ + { + name: "exact challenged resource_metadata is authoritative with no fallback and hard-errors on 404", + wwwAuth: `Bearer resource_metadata="https://mcp.example.test/custom-prm"`, + wantPrimary: "https://mcp.example.test/custom-prm", + wantNotFoundIsHardError: true, + }, + { + name: "no resource_metadata falls back to path-insertion then origin-root", + wwwAuth: `Bearer error="insufficient_scope"`, + wantPrimary: "https://mcp.example.test/.well-known/oauth-protected-resource/v1/mcp/authv2", + wantFallbackURL: []string{"https://mcp.example.test/.well-known/oauth-protected-resource"}, + }, + { + name: "a bare resource auth-param is not a metadata URL: falls back to path-insertion then origin-root", + wwwAuth: `Bearer resource="https://mcp.example.test/v1/mcp/authv2"`, + wantPrimary: "https://mcp.example.test/.well-known/oauth-protected-resource/v1/mcp/authv2", + wantFallbackURL: []string{"https://mcp.example.test/.well-known/oauth-protected-resource"}, + }, + { + name: "empty challenge (no 401) also falls back to path-insertion then origin-root", + wwwAuth: "", + wantPrimary: "https://mcp.example.test/.well-known/oauth-protected-resource/v1/mcp/authv2", + wantFallbackURL: []string{"https://mcp.example.test/.well-known/oauth-protected-resource"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + primary, opts := protectedResourceMetadataCandidates(serverURL, authOrigin, tt.wwwAuth) + assert.Equal(t, tt.wantPrimary, primary) + assert.Equal(t, tt.wantFallbackURL, opts.FallbackCandidateURLs) + assert.Equal(t, tt.wantNotFoundIsHardError, opts.NotFoundIsHardError) + }) + } +} + +// TestProtectedResourceMetadataPathInsertionURL covers RFC 9728 §3.1 path +// insertion for both a path-bearing and a root resource URL. +func TestProtectedResourceMetadataPathInsertionURL(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + in string + want string + }{ + { + name: "path is inserted between origin and path", + in: "https://mcp.atlassian.com/v1/mcp/authv2", + want: "https://mcp.atlassian.com/.well-known/oauth-protected-resource/v1/mcp/authv2", + }, + { + name: "trailing slash is trimmed before insertion", + in: "https://mcp.example.test/mcp/", + want: "https://mcp.example.test/.well-known/oauth-protected-resource/mcp", + }, + { + name: "no path collapses to the plain well-known URL", + in: "https://mcp.example.test", + want: "https://mcp.example.test/.well-known/oauth-protected-resource", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + parsed, err := url.Parse(tt.in) + require.NoError(t, err) + assert.Equal(t, tt.want, protectedResourceMetadataPathInsertionURL(parsed)) + }) + } +} + +// TestResolveStandaloneClientCredentials characterizes the standalone CLI's +// client-credential selector against the full decision table: explicit +// config wins with its scopes carried exactly (nil when omitted); absent +// explicit credentials require DCR, using configured > challenge > PRM > +// omit scope selection; and a missing or failing DCR is a hard error with +// no interactive prompt (the selector never elicits). +func TestResolveStandaloneClientCredentials(t *testing.T) { + t.Parallel() + + t.Run("explicit client ID skips DCR and carries configured scopes exactly", func(t *testing.T) { + t.Parallel() + + var registerCalls atomic.Int32 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + registerCalls.Add(1) + w.WriteHeader(http.StatusInternalServerError) + })) + defer srv.Close() + + oauthConfig := &latest.RemoteOAuthConfig{ + ClientID: "explicit-client", + ClientSecret: "explicit-secret", + Scopes: []string{"explicit-scope"}, + } + authMeta := &AuthorizationServerMetadata{RegistrationEndpoint: srv.URL} + + clientID, clientSecret, scopes, err := resolveStandaloneClientCredentials( + t.Context(), srv.Client(), oauthConfig, authMeta, "http://127.0.0.1/callback", + []string{"challenge-scope"}, []string{"prm-scope"}, + ) + require.NoError(t, err) + assert.Equal(t, "explicit-client", clientID) + assert.Equal(t, "explicit-secret", clientSecret) + assert.Equal(t, []string{"explicit-scope"}, scopes) + assert.Equal(t, int32(0), registerCalls.Load(), "explicit credentials must never trigger DCR") + }) + + t.Run("explicit client ID with no configured scopes omits the scope parameter", func(t *testing.T) { + t.Parallel() + + oauthConfig := &latest.RemoteOAuthConfig{ClientID: "explicit-client"} + authMeta := &AuthorizationServerMetadata{RegistrationEndpoint: "http://unused.invalid"} + + clientID, clientSecret, scopes, err := resolveStandaloneClientCredentials( + t.Context(), http.DefaultClient, oauthConfig, authMeta, "http://127.0.0.1/callback", + []string{"challenge-scope"}, []string{"prm-scope"}, + ) + require.NoError(t, err) + assert.Equal(t, "explicit-client", clientID) + assert.Empty(t, clientSecret) + assert.Nil(t, scopes, "no configured scopes must not be backfilled from challenge/PRM when credentials are explicit") + }) + + t.Run("no explicit client ID and no registration endpoint hard-errors without prompting", func(t *testing.T) { + t.Parallel() + + authMeta := &AuthorizationServerMetadata{} + + _, _, _, err := resolveStandaloneClientCredentials( + t.Context(), http.DefaultClient, nil, authMeta, "http://127.0.0.1/callback", nil, nil, + ) + require.Error(t, err) + assert.Contains(t, err.Error(), "does not support dynamic client registration") + }) + + t.Run("DCR failure hard-errors without prompting", func(t *testing.T) { + t.Parallel() + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + })) + defer srv.Close() + + authMeta := &AuthorizationServerMetadata{RegistrationEndpoint: srv.URL} + + _, _, _, err := resolveStandaloneClientCredentials( + t.Context(), srv.Client(), nil, authMeta, "http://127.0.0.1/callback", nil, nil, + ) + require.Error(t, err) + assert.Contains(t, err.Error(), "dynamic client registration failed") + }) + + t.Run("successful DCR uses configured over challenge over PRM scopes and reuses them", func(t *testing.T) { + t.Parallel() + + var gotScope string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var body struct { + Scope string `json:"scope"` + } + _ = json.NewDecoder(r.Body).Decode(&body) + gotScope = body.Scope + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"client_id":"registered-id","client_secret":"registered-secret"}`)) + })) + defer srv.Close() + + authMeta := &AuthorizationServerMetadata{RegistrationEndpoint: srv.URL} + oauthConfig := &latest.RemoteOAuthConfig{Scopes: []string{"configured-scope"}} + + clientID, clientSecret, scopes, err := resolveStandaloneClientCredentials( + t.Context(), srv.Client(), oauthConfig, authMeta, "http://127.0.0.1/callback", + []string{"challenge-scope"}, []string{"prm-scope"}, + ) + require.NoError(t, err) + assert.Equal(t, "registered-id", clientID) + assert.Equal(t, "registered-secret", clientSecret) + assert.Equal(t, []string{"configured-scope"}, scopes) + assert.Equal(t, "configured-scope", gotScope, "the DCR request must carry the exact selected scope") + }) + + t.Run("successful DCR falls back to challenge then PRM scopes when unconfigured", func(t *testing.T) { + t.Parallel() + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"client_id":"registered-id"}`)) + })) + defer srv.Close() + authMeta := &AuthorizationServerMetadata{RegistrationEndpoint: srv.URL} + + _, _, scopes, err := resolveStandaloneClientCredentials( + t.Context(), srv.Client(), nil, authMeta, "http://127.0.0.1/callback", + nil, []string{"prm-scope"}, + ) + require.NoError(t, err) + assert.Equal(t, []string{"prm-scope"}, scopes, "PRM scopes_supported must be used when no configured or challenge scope is available") + }) +} + +// TestPerformOAuthLogin_ChallengeResourceMetadataAuthoritative_EndToEnd drives +// PerformOAuthLogin end to end when the unauthenticated probe's challenge +// names an exact resource_metadata URL: that URL is used verbatim (never +// recomputed via path-insertion/origin-root), and Remote.URL — including a +// query string, to prove it is never rewritten — is used verbatim as the +// probe target and, byte-for-byte, as the token-store key. +func TestPerformOAuthLogin_ChallengeResourceMetadataAuthoritative_EndToEnd(t *testing.T) { + resetDefaultStore(t) + store := NewInMemoryTokenStore() + SetDefaultTokenStoreFactory(func() OAuthTokenStore { return store }) + + captureFile := setupFakeBrowserOpener(t) + + const mcpPath = "/v1/mcp/authv2" + const mcpQuery = "tenant=42" + + var probeRequestURI string + var pathInsertionCalls, originRootCalls, registerCalls atomic.Int32 + mux := http.NewServeMux() + srv := httptest.NewServer(mux) + defer srv.Close() + + mux.HandleFunc(mcpPath, func(w http.ResponseWriter, r *http.Request) { + probeRequestURI = r.URL.RequestURI() + w.Header().Set("WWW-Authenticate", `Bearer resource_metadata="`+srv.URL+`/custom-prm"`) + w.WriteHeader(http.StatusUnauthorized) + }) + mux.HandleFunc("/custom-prm", func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(protectedResourceMetadata{ + Resource: srv.URL + mcpPath, + AuthorizationServers: []string{srv.URL}, + ScopesSupported: []string{"prm-scope"}, + }) + }) + mux.HandleFunc("/.well-known/oauth-protected-resource"+mcpPath, func(w http.ResponseWriter, _ *http.Request) { + pathInsertionCalls.Add(1) + w.WriteHeader(http.StatusNotFound) + }) + mux.HandleFunc("/.well-known/oauth-protected-resource", func(w http.ResponseWriter, _ *http.Request) { + originRootCalls.Add(1) + w.WriteHeader(http.StatusNotFound) + }) + mux.HandleFunc("/.well-known/oauth-authorization-server", func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(AuthorizationServerMetadata{ + Issuer: srv.URL, + AuthorizationEndpoint: srv.URL + "/authorize", + TokenEndpoint: srv.URL + "/token", + RegistrationEndpoint: srv.URL + "/register", + }) + }) + mux.HandleFunc("/register", func(w http.ResponseWriter, _ *http.Request) { + registerCalls.Add(1) + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"client_id":"registered-id","client_secret":"registered-secret"}`)) + }) + mux.HandleFunc("/token", func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"access_token":"exchanged-at","token_type":"Bearer","expires_in":3600}`)) + }) + + defer setOAuthLoginHTTPClientForTesting(srv.Client())() + + remote := latest.Remote{URL: srv.URL + mcpPath + "?" + mcpQuery} + + errCh := make(chan error, 1) + go func() { errCh <- PerformOAuthLogin(t.Context(), remote) }() + + authURL := requireCapturedAuthorizeURL(t, captureFile) + deliverFakeCallback(t, authURL, "code-challenge-exact-prm") + + require.NoError(t, <-errCh) + + assert.Equal(t, mcpPath+"?"+mcpQuery, probeRequestURI, "the probe must hit Remote.URL verbatim, including its query string") + assert.Equal(t, int32(0), pathInsertionCalls.Load(), "the exact challenged resource_metadata URL must not fall back to path-insertion") + assert.Equal(t, int32(0), originRootCalls.Load(), "the exact challenged resource_metadata URL must not fall back to origin-root") + assert.Equal(t, int32(1), registerCalls.Load()) + + tok, err := store.GetToken(remote.URL) + require.NoError(t, err, "the stored token must be retrievable by Remote.URL verbatim, byte-for-byte") + assert.Equal(t, "exchanged-at", tok.AccessToken) + assert.Equal(t, []string{"prm-scope"}, tok.RequestedScopes, "no configured/challenge scope: PRM scopes_supported must be selected and carried to the token") +} + +// TestPerformOAuthLogin_PathInsertionThenOriginRootFallback_EndToEnd drives +// PerformOAuthLogin end to end when the unauthenticated probe's challenge +// carries no resource_metadata: the RFC 9728 §3.1 path-insertion URL must be +// tried first, and the origin-root well-known URL only after that 404s. +func TestPerformOAuthLogin_PathInsertionThenOriginRootFallback_EndToEnd(t *testing.T) { + resetDefaultStore(t) + store := NewInMemoryTokenStore() + SetDefaultTokenStoreFactory(func() OAuthTokenStore { return store }) + + captureFile := setupFakeBrowserOpener(t) + + const mcpPath = "/v1/mcp/authv2" + + var orderMu sync.Mutex + var order []string + recordOrder := func(step string) { + orderMu.Lock() + defer orderMu.Unlock() + order = append(order, step) + } + mux := http.NewServeMux() + srv := httptest.NewServer(mux) + defer srv.Close() + + mux.HandleFunc(mcpPath, func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("WWW-Authenticate", `Bearer error="insufficient_scope"`) + w.WriteHeader(http.StatusUnauthorized) + }) + mux.HandleFunc("/.well-known/oauth-protected-resource"+mcpPath, func(w http.ResponseWriter, _ *http.Request) { + recordOrder("path-insertion") + w.WriteHeader(http.StatusNotFound) + }) + mux.HandleFunc("/.well-known/oauth-protected-resource", func(w http.ResponseWriter, _ *http.Request) { + recordOrder("origin-root") + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(protectedResourceMetadata{AuthorizationServers: []string{srv.URL}}) + }) + mux.HandleFunc("/.well-known/oauth-authorization-server", func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(AuthorizationServerMetadata{ + Issuer: srv.URL, + AuthorizationEndpoint: srv.URL + "/authorize", + TokenEndpoint: srv.URL + "/token", + RegistrationEndpoint: srv.URL + "/register", + }) + }) + mux.HandleFunc("/register", func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"client_id":"registered-id"}`)) + }) + mux.HandleFunc("/token", func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"access_token":"exchanged-at","token_type":"Bearer","expires_in":3600}`)) + }) + + defer setOAuthLoginHTTPClientForTesting(srv.Client())() + + remote := latest.Remote{URL: srv.URL + mcpPath} + + errCh := make(chan error, 1) + go func() { errCh <- PerformOAuthLogin(t.Context(), remote) }() + + authURL := requireCapturedAuthorizeURL(t, captureFile) + deliverFakeCallback(t, authURL, "code-path-insertion-fallback") + + require.NoError(t, <-errCh) + orderMu.Lock() + gotOrder := order + orderMu.Unlock() + assert.Equal(t, []string{"path-insertion", "origin-root"}, gotOrder, + "path-insertion must be tried before the origin-root fallback, and only after a 404") + + _, err := store.GetToken(remote.URL) + require.NoError(t, err) +} + +// TestPerformOAuthLogin_ProtectedResourceMetadataHardError_StopsFlow proves +// that a hard protected-resource-metadata error (non-404/non-200, or a +// decode failure) stops the flow immediately in both discovery branches: +// no authorization-server metadata, DCR, browser, or token request follows. +func TestPerformOAuthLogin_ProtectedResourceMetadataHardError_StopsFlow(t *testing.T) { + tests := []struct { + name string + wwwAuth string + }{ + {name: "exact challenged resource_metadata URL returns a hard error", wwwAuth: `Bearer resource_metadata="__PRM__"`}, + {name: "path-insertion candidate returns a hard error (no origin-root fallback)", wwwAuth: `Bearer error="insufficient_scope"`}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + resetDefaultStore(t) + store := NewInMemoryTokenStore() + SetDefaultTokenStoreFactory(func() OAuthTokenStore { return store }) + + captureFile := setupFakeBrowserOpener(t) + + const mcpPath = "/v1/mcp/authv2" + var authServerCalls, originRootCalls atomic.Int32 + mux := http.NewServeMux() + srv := httptest.NewServer(mux) + defer srv.Close() + + wwwAuth := strings.ReplaceAll(tt.wwwAuth, "__PRM__", srv.URL+"/prm-error") + mux.HandleFunc(mcpPath, func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("WWW-Authenticate", wwwAuth) + w.WriteHeader(http.StatusUnauthorized) + }) + // The exact-challenge branch's PRM candidate is /prm-error; the + // no-challenge branch's primary candidate is the path-insertion URL. + // Both are wired to the same hard-error handler; only the one the + // active sub-test's discovery branch actually reaches gets hit. + hardErrorHandler := func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + _, _ = w.Write([]byte("boom")) + } + mux.HandleFunc("/prm-error", hardErrorHandler) + mux.HandleFunc("/.well-known/oauth-protected-resource"+mcpPath, hardErrorHandler) + mux.HandleFunc("/.well-known/oauth-protected-resource", func(w http.ResponseWriter, _ *http.Request) { + originRootCalls.Add(1) + w.WriteHeader(http.StatusNotFound) + }) + mux.HandleFunc("/.well-known/oauth-authorization-server", func(w http.ResponseWriter, _ *http.Request) { + authServerCalls.Add(1) + w.WriteHeader(http.StatusNotFound) + }) + + defer setOAuthLoginHTTPClientForTesting(srv.Client())() + + remote := latest.Remote{URL: srv.URL + mcpPath} + err := PerformOAuthLogin(t.Context(), remote) + require.Error(t, err) + assert.Contains(t, err.Error(), "failed to fetch protected resource metadata") + + assert.Equal(t, int32(0), authServerCalls.Load(), "a hard PRM error must stop before authorization-server discovery") + assert.Equal(t, int32(0), originRootCalls.Load(), "a hard error on an earlier candidate must not try a later one") + _, err = os.ReadFile(captureFile) + require.Error(t, err, "no browser must be opened after a hard PRM error") + _, err = store.GetToken(remote.URL) + require.Error(t, err, "no token must be stored after a hard PRM error") + }) + } +} + +// TestPerformOAuthLogin_ExplicitClientCredentials_NoDCR proves that +// configuring Remote.OAuth.ClientID skips Dynamic Client Registration +// entirely and carries the configured scopes (nil when omitted) to +// /authorize and the stored token. +func TestPerformOAuthLogin_ExplicitClientCredentials_NoDCR(t *testing.T) { + resetDefaultStore(t) + store := NewInMemoryTokenStore() + SetDefaultTokenStoreFactory(func() OAuthTokenStore { return store }) + + captureFile := setupFakeBrowserOpener(t) + + const mcpPath = "/mcp" + var registerCalls atomic.Int32 + var gotAuthorizeScope string + mux := http.NewServeMux() + srv := httptest.NewServer(mux) + defer srv.Close() + + mux.HandleFunc(mcpPath, func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("WWW-Authenticate", `Bearer error="insufficient_scope"`) + w.WriteHeader(http.StatusUnauthorized) + }) + mux.HandleFunc("/.well-known/oauth-protected-resource"+mcpPath, func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNotFound) + }) + mux.HandleFunc("/.well-known/oauth-protected-resource", func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(protectedResourceMetadata{AuthorizationServers: []string{srv.URL}}) + }) + mux.HandleFunc("/.well-known/oauth-authorization-server", func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(AuthorizationServerMetadata{ + Issuer: srv.URL, + AuthorizationEndpoint: srv.URL + "/authorize", + TokenEndpoint: srv.URL + "/token", + RegistrationEndpoint: srv.URL + "/register", + }) + }) + mux.HandleFunc("/register", func(w http.ResponseWriter, _ *http.Request) { + registerCalls.Add(1) + w.WriteHeader(http.StatusInternalServerError) + }) + mux.HandleFunc("/token", func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"access_token":"exchanged-at","token_type":"Bearer","expires_in":3600}`)) + }) + + defer setOAuthLoginHTTPClientForTesting(srv.Client())() + + remote := latest.Remote{ + URL: srv.URL + mcpPath, + OAuth: &latest.RemoteOAuthConfig{ClientID: "pre-registered-client", ClientSecret: "pre-registered-secret"}, + } + + errCh := make(chan error, 1) + go func() { errCh <- PerformOAuthLogin(t.Context(), remote) }() + + authURL := requireCapturedAuthorizeURL(t, captureFile) + parsedAuthURL, err := url.Parse(authURL) + require.NoError(t, err) + gotAuthorizeScope = parsedAuthURL.Query().Get("scope") + assert.Equal(t, "pre-registered-client", parsedAuthURL.Query().Get("client_id")) + + deliverFakeCallback(t, authURL, "code-explicit-client-credentials") + require.NoError(t, <-errCh) + + assert.Equal(t, int32(0), registerCalls.Load(), "explicit credentials must never trigger DCR") + assert.Empty(t, gotAuthorizeScope, "no configured scopes: the scope parameter must be omitted, not backfilled from discovery") + + tok, err := store.GetToken(remote.URL) + require.NoError(t, err) + assert.Equal(t, "pre-registered-client", tok.ClientID) + assert.Nil(t, tok.RequestedScopes) +} + +// TestPerformOAuthLogin_NoDCR_NoExplicitCredentials_HardErrorsWithoutPrompting +// proves that when the authorization server does not support Dynamic Client +// Registration and no explicit clientId is configured, PerformOAuthLogin +// hard-errors (it never prompts, since it is a non-interactive command) and +// never opens the browser or stores a token. +func TestPerformOAuthLogin_NoDCR_NoExplicitCredentials_HardErrorsWithoutPrompting(t *testing.T) { + resetDefaultStore(t) + store := NewInMemoryTokenStore() + SetDefaultTokenStoreFactory(func() OAuthTokenStore { return store }) + + captureFile := setupFakeBrowserOpener(t) + + const mcpPath = "/mcp" + mux := http.NewServeMux() + srv := httptest.NewServer(mux) + defer srv.Close() + + mux.HandleFunc(mcpPath, func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("WWW-Authenticate", `Bearer error="insufficient_scope"`) + w.WriteHeader(http.StatusUnauthorized) + }) + mux.HandleFunc("/.well-known/oauth-protected-resource"+mcpPath, func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNotFound) + }) + mux.HandleFunc("/.well-known/oauth-protected-resource", func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(protectedResourceMetadata{AuthorizationServers: []string{srv.URL}}) + }) + mux.HandleFunc("/.well-known/oauth-authorization-server", func(w http.ResponseWriter, _ *http.Request) { + // No registration_endpoint: DCR is unsupported. + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(AuthorizationServerMetadata{ + Issuer: srv.URL, + AuthorizationEndpoint: srv.URL + "/authorize", + TokenEndpoint: srv.URL + "/token", + }) + }) + + defer setOAuthLoginHTTPClientForTesting(srv.Client())() + + remote := latest.Remote{URL: srv.URL + mcpPath} + err := PerformOAuthLogin(t.Context(), remote) + require.Error(t, err) + assert.Contains(t, err.Error(), "does not support dynamic client registration") + + _, err = os.ReadFile(captureFile) + require.Error(t, err, "no browser must be opened when DCR is unavailable and no explicit credentials are configured") + _, err = store.GetToken(remote.URL) + require.Error(t, err, "no token must be stored when DCR is unavailable and no explicit credentials are configured") +} + +// TestPerformOAuthLogin_DCRFails_HardErrorsWithoutPrompting proves that a +// failing Dynamic Client Registration call is a hard error, not a fallback +// to an interactive prompt: PerformOAuthLogin is non-interactive. +func TestPerformOAuthLogin_DCRFails_HardErrorsWithoutPrompting(t *testing.T) { + resetDefaultStore(t) + store := NewInMemoryTokenStore() + SetDefaultTokenStoreFactory(func() OAuthTokenStore { return store }) + + captureFile := setupFakeBrowserOpener(t) + + const mcpPath = "/mcp" + mux := http.NewServeMux() + srv := httptest.NewServer(mux) + defer srv.Close() + + mux.HandleFunc(mcpPath, func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("WWW-Authenticate", `Bearer error="insufficient_scope"`) + w.WriteHeader(http.StatusUnauthorized) + }) + mux.HandleFunc("/.well-known/oauth-protected-resource"+mcpPath, func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNotFound) + }) + mux.HandleFunc("/.well-known/oauth-protected-resource", func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(protectedResourceMetadata{AuthorizationServers: []string{srv.URL}}) + }) + mux.HandleFunc("/.well-known/oauth-authorization-server", func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(AuthorizationServerMetadata{ + Issuer: srv.URL, + AuthorizationEndpoint: srv.URL + "/authorize", + TokenEndpoint: srv.URL + "/token", + RegistrationEndpoint: srv.URL + "/register", + }) + }) + mux.HandleFunc("/register", func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + _, _ = w.Write([]byte("registration unavailable")) + }) + + defer setOAuthLoginHTTPClientForTesting(srv.Client())() + + remote := latest.Remote{URL: srv.URL + mcpPath} + err := PerformOAuthLogin(t.Context(), remote) + require.Error(t, err) + assert.Contains(t, err.Error(), "dynamic client registration failed") + + _, err = os.ReadFile(captureFile) + require.Error(t, err, "no browser must be opened after a DCR failure") + _, err = store.GetToken(remote.URL) + require.Error(t, err, "no token must be stored after a DCR failure") +} + +// TestPerformOAuthLogin_ChallengedResourceMetadata404_HardStopsFlow proves +// that a 404 on the exact challenged resource_metadata URL is a hard error +// (via fetchProtectedResourceMetadata's NotFoundIsHardError opt-in): it must +// not silently fall through to a guessed authorization server. No +// path-insertion/origin-root fallback, authorization-server metadata, DCR, +// browser, or token request follows, and the error string carries the +// underlying helper message exactly once, plus safe (query-free) URL +// context. +func TestPerformOAuthLogin_ChallengedResourceMetadata404_HardStopsFlow(t *testing.T) { + resetDefaultStore(t) + store := NewInMemoryTokenStore() + SetDefaultTokenStoreFactory(func() OAuthTokenStore { return store }) + + captureFile := setupFakeBrowserOpener(t) + + const mcpPath = "/v1/mcp/authv2" + var pathInsertionCalls, originRootCalls, authServerCalls atomic.Int32 + mux := http.NewServeMux() + srv := httptest.NewServer(mux) + defer srv.Close() + + mux.HandleFunc(mcpPath, func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("WWW-Authenticate", `Bearer resource_metadata="`+srv.URL+`/custom-prm"`) + w.WriteHeader(http.StatusUnauthorized) + }) + mux.HandleFunc("/custom-prm", func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNotFound) + }) + mux.HandleFunc("/.well-known/oauth-protected-resource"+mcpPath, func(w http.ResponseWriter, _ *http.Request) { + pathInsertionCalls.Add(1) + w.WriteHeader(http.StatusNotFound) + }) + mux.HandleFunc("/.well-known/oauth-protected-resource", func(w http.ResponseWriter, _ *http.Request) { + originRootCalls.Add(1) + w.WriteHeader(http.StatusNotFound) + }) + mux.HandleFunc("/.well-known/oauth-authorization-server", func(w http.ResponseWriter, _ *http.Request) { + authServerCalls.Add(1) + w.WriteHeader(http.StatusNotFound) + }) + + defer setOAuthLoginHTTPClientForTesting(srv.Client())() + + remote := latest.Remote{URL: srv.URL + mcpPath + "?secret=shhh"} + err := PerformOAuthLogin(t.Context(), remote) + require.Error(t, err) + assert.Equal(t, 1, strings.Count(err.Error(), "failed to fetch protected resource metadata"), + "the underlying helper message must appear exactly once, not doubled by the CLI wrap") + assert.NotContains(t, err.Error(), "secret=shhh", "the error must never leak the query string") + + assert.Equal(t, int32(0), pathInsertionCalls.Load(), "a 404 on the exact challenged URL must not fall back to path-insertion") + assert.Equal(t, int32(0), originRootCalls.Load(), "a 404 on the exact challenged URL must not fall back to origin-root") + assert.Equal(t, int32(0), authServerCalls.Load(), "a hard PRM error must stop before authorization-server discovery") + + _, err = os.ReadFile(captureFile) + require.Error(t, err, "no browser must be opened after a challenged-404 hard stop") + _, err = store.GetToken(remote.URL) + require.Error(t, err, "no token must be stored after a challenged-404 hard stop") +} + +// TestPerformOAuthLogin_ResourceOnlyChallenge_FallsBackToPathInsertion_EndToEnd +// proves that a challenge carrying only a bare resource auth-param (naming +// the MCP endpoint itself, with no resource_metadata) is never GET/decoded +// as protected-resource metadata: discovery instead proceeds via the RFC +// 9728 §3.1 path-insertion candidate, exactly like an unchallenged probe. +func TestPerformOAuthLogin_ResourceOnlyChallenge_FallsBackToPathInsertion_EndToEnd(t *testing.T) { + resetDefaultStore(t) + store := NewInMemoryTokenStore() + SetDefaultTokenStoreFactory(func() OAuthTokenStore { return store }) + + captureFile := setupFakeBrowserOpener(t) + + const mcpPath = "/v1/mcp/authv2" + var mcpEndpointHitsAfterProbe atomic.Int32 + mux := http.NewServeMux() + srv := httptest.NewServer(mux) + defer srv.Close() + + mux.HandleFunc(mcpPath, func(w http.ResponseWriter, _ *http.Request) { + mcpEndpointHitsAfterProbe.Add(1) + w.Header().Set("WWW-Authenticate", `Bearer resource="`+srv.URL+mcpPath+`"`) + w.WriteHeader(http.StatusUnauthorized) + }) + mux.HandleFunc("/.well-known/oauth-protected-resource"+mcpPath, func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(protectedResourceMetadata{AuthorizationServers: []string{srv.URL}}) + }) + mux.HandleFunc("/.well-known/oauth-protected-resource", func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNotFound) + }) + mux.HandleFunc("/.well-known/oauth-authorization-server", func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(AuthorizationServerMetadata{ + Issuer: srv.URL, + AuthorizationEndpoint: srv.URL + "/authorize", + TokenEndpoint: srv.URL + "/token", + RegistrationEndpoint: srv.URL + "/register", + }) + }) + mux.HandleFunc("/register", func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"client_id":"registered-id"}`)) + }) + mux.HandleFunc("/token", func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"access_token":"exchanged-at","token_type":"Bearer","expires_in":3600}`)) + }) + + defer setOAuthLoginHTTPClientForTesting(srv.Client())() + + remote := latest.Remote{URL: srv.URL + mcpPath} + + errCh := make(chan error, 1) + go func() { errCh <- PerformOAuthLogin(t.Context(), remote) }() + + authURL := requireCapturedAuthorizeURL(t, captureFile) + deliverFakeCallback(t, authURL, "code-resource-only-challenge") + + require.NoError(t, <-errCh) + assert.Equal(t, int32(1), mcpEndpointHitsAfterProbe.Load(), + "the MCP endpoint must be fetched exactly once (the probe itself), never again as protected-resource metadata") + + _, err := store.GetToken(remote.URL) + require.NoError(t, err) +} + +// TestPerformOAuthLogin_DCRSuccess_EndToEndScopeEquality is the standalone +// counterpart of TestHandleManagedOAuthFlow_DCRSuccess_EndToEndScopeEquality: +// it drives PerformOAuthLogin all the way to a stored token and proves the +// exact same selected scope set reaches every place that must agree on it — +// the DCR registration request body, the /authorize URL's scope parameter, +// and the stored token's RequestedScopes. +func TestPerformOAuthLogin_DCRSuccess_EndToEndScopeEquality(t *testing.T) { + resetDefaultStore(t) + store := NewInMemoryTokenStore() + SetDefaultTokenStoreFactory(func() OAuthTokenStore { return store }) + + captureFile := setupFakeBrowserOpener(t) + + const mcpPath = "/mcp" + var registerScope string + var haveRegisterScope bool + mux := http.NewServeMux() + srv := httptest.NewServer(mux) + defer srv.Close() + + mux.HandleFunc(mcpPath, func(w http.ResponseWriter, _ *http.Request) { + // No challenge scope: the config's scope must still win over PRM. + w.Header().Set("WWW-Authenticate", `Bearer error="insufficient_scope"`) + w.WriteHeader(http.StatusUnauthorized) + }) + mux.HandleFunc("/.well-known/oauth-protected-resource"+mcpPath, func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNotFound) + }) + mux.HandleFunc("/.well-known/oauth-protected-resource", func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(protectedResourceMetadata{ + AuthorizationServers: []string{srv.URL}, + ScopesSupported: []string{"prm-scope"}, + }) + }) + mux.HandleFunc("/.well-known/oauth-authorization-server", func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(AuthorizationServerMetadata{ + Issuer: srv.URL, + AuthorizationEndpoint: srv.URL + "/authorize", + TokenEndpoint: srv.URL + "/token", + RegistrationEndpoint: srv.URL + "/register", + }) + }) + mux.HandleFunc("/register", func(w http.ResponseWriter, r *http.Request) { + var body struct { + Scope string `json:"scope"` + } + _ = json.NewDecoder(r.Body).Decode(&body) + registerScope, haveRegisterScope = body.Scope, true + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"client_id":"registered-client-id","client_secret":"registered-secret"}`)) + }) + mux.HandleFunc("/token", func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"access_token":"exchanged-at","token_type":"Bearer","expires_in":3600}`)) + }) + + defer setOAuthLoginHTTPClientForTesting(srv.Client())() + + remote := latest.Remote{ + URL: srv.URL + mcpPath, + OAuth: &latest.RemoteOAuthConfig{Scopes: []string{"configured-scope"}}, + } + + errCh := make(chan error, 1) + go func() { errCh <- PerformOAuthLogin(t.Context(), remote) }() + + authURL := requireCapturedAuthorizeURL(t, captureFile) + deliverFakeCallback(t, authURL, "code-dcr-scope-equality") + + require.NoError(t, <-errCh) + + require.True(t, haveRegisterScope, "DCR request must carry a scope field") + assert.Equal(t, "configured-scope", registerScope, "DCR registration request must carry the selected scope") + + parsedAuthURL, err := url.Parse(authURL) + require.NoError(t, err) + assert.Equal(t, "configured-scope", parsedAuthURL.Query().Get("scope"), + "the /authorize URL must carry the exact same scope sent to DCR") + + tok, err := store.GetToken(remote.URL) + require.NoError(t, err) + assert.Equal(t, []string{"configured-scope"}, tok.RequestedScopes, + "the stored token's RequestedScopes must match the scope sent to DCR and /authorize") +} + +// TestPerformOAuthLogin_ChallengeScopePrecedenceOverDistinctPRMScopes_EndToEnd +// drives PerformOAuthLogin end to end with a challenge that carries a scope +// AND protected-resource metadata whose scopes_supported is a distinct, +// non-empty set. It proves the challenge scope wins on all three surfaces +// (DCR registration body, /authorize scope parameter, stored +// RequestedScopes) and that no PRM-only scope leaks into any of them. +func TestPerformOAuthLogin_ChallengeScopePrecedenceOverDistinctPRMScopes_EndToEnd(t *testing.T) { + resetDefaultStore(t) + store := NewInMemoryTokenStore() + SetDefaultTokenStoreFactory(func() OAuthTokenStore { return store }) + + captureFile := setupFakeBrowserOpener(t) + + const mcpPath = "/mcp" + var registerScope string + var haveRegisterScope bool + mux := http.NewServeMux() + srv := httptest.NewServer(mux) + defer srv.Close() + + mux.HandleFunc(mcpPath, func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("WWW-Authenticate", `Bearer scope="challenge-scope" resource_metadata="`+srv.URL+"/custom-prm"+`"`) + w.WriteHeader(http.StatusUnauthorized) + }) + mux.HandleFunc("/custom-prm", func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(protectedResourceMetadata{ + AuthorizationServers: []string{srv.URL}, + ScopesSupported: []string{"prm-only-scope"}, + }) + }) + mux.HandleFunc("/.well-known/oauth-authorization-server", func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(AuthorizationServerMetadata{ + Issuer: srv.URL, + AuthorizationEndpoint: srv.URL + "/authorize", + TokenEndpoint: srv.URL + "/token", + RegistrationEndpoint: srv.URL + "/register", + }) + }) + mux.HandleFunc("/register", func(w http.ResponseWriter, r *http.Request) { + var body struct { + Scope string `json:"scope"` + } + _ = json.NewDecoder(r.Body).Decode(&body) + registerScope, haveRegisterScope = body.Scope, true + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"client_id":"registered-client-id","client_secret":"registered-secret"}`)) + }) + mux.HandleFunc("/token", func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"access_token":"exchanged-at","token_type":"Bearer","expires_in":3600}`)) + }) + + defer setOAuthLoginHTTPClientForTesting(srv.Client())() + + remote := latest.Remote{URL: srv.URL + mcpPath} + + errCh := make(chan error, 1) + go func() { errCh <- PerformOAuthLogin(t.Context(), remote) }() + + authURL := requireCapturedAuthorizeURL(t, captureFile) + deliverFakeCallback(t, authURL, "code-challenge-over-prm") + + require.NoError(t, <-errCh) + + require.True(t, haveRegisterScope, "DCR request must carry a scope field") + assert.Equal(t, "challenge-scope", registerScope, "the challenge scope, not the distinct PRM scope, must reach DCR") + + parsedAuthURL, err := url.Parse(authURL) + require.NoError(t, err) + assert.Equal(t, "challenge-scope", parsedAuthURL.Query().Get("scope"), + "the /authorize URL must carry the challenge scope, not the distinct PRM scope") + + tok, err := store.GetToken(remote.URL) + require.NoError(t, err) + assert.Equal(t, []string{"challenge-scope"}, tok.RequestedScopes, + "the stored token's RequestedScopes must be the challenge scope, with no PRM-only scope leaking in") +} + +// TestPerformOAuthLogin_PRMResourceDiffersFromRemoteURL_ResourceIsRemoteURLVerbatim +// is the regression pin for R2: protected-resource metadata reports a +// resource deliberately DIFFERENT from the configured Remote.URL (which +// itself carries a query string), and the test proves Remote.URL — byte for +// byte, including the query — is what actually reaches every surface: the +// /authorize resource parameter, the token-exchange resource form value, and +// the token-store key. It also proves the PRM-reported resource leaks into +// none of the three; reintroducing `cmp.Or(resourceMetadata.Resource, +// serverURL)` would fail this test even though every other end-to-end +// fixture (whose PRM resource equals the MCP URL) would still pass. +func TestPerformOAuthLogin_PRMResourceDiffersFromRemoteURL_ResourceIsRemoteURLVerbatim(t *testing.T) { + resetDefaultStore(t) + store := NewInMemoryTokenStore() + SetDefaultTokenStoreFactory(func() OAuthTokenStore { return store }) + + captureFile := setupFakeBrowserOpener(t) + + const mcpPath = "/v1/mcp/authv2" + const mcpQuery = "tenant=42" + const prmResource = "https://prm-reports-a-different-resource.example.test/mcp" + + var tokenResource string + var haveTokenResource bool + mux := http.NewServeMux() + srv := httptest.NewServer(mux) + defer srv.Close() + + mux.HandleFunc(mcpPath, func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("WWW-Authenticate", `Bearer resource_metadata="`+srv.URL+`/custom-prm"`) + w.WriteHeader(http.StatusUnauthorized) + }) + mux.HandleFunc("/custom-prm", func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(protectedResourceMetadata{ + Resource: prmResource, + AuthorizationServers: []string{srv.URL}, + ScopesSupported: []string{"prm-scope"}, + }) + }) + mux.HandleFunc("/.well-known/oauth-authorization-server", func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(AuthorizationServerMetadata{ + Issuer: srv.URL, + AuthorizationEndpoint: srv.URL + "/authorize", + TokenEndpoint: srv.URL + "/token", + RegistrationEndpoint: srv.URL + "/register", + }) + }) + mux.HandleFunc("/register", func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"client_id":"registered-client-id"}`)) + }) + mux.HandleFunc("/token", func(w http.ResponseWriter, r *http.Request) { + if err := r.ParseForm(); err != nil { + t.Fatal(err) + } + tokenResource, haveTokenResource = r.FormValue("resource"), true + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"access_token":"exchanged-at","token_type":"Bearer","expires_in":3600}`)) + }) + + defer setOAuthLoginHTTPClientForTesting(srv.Client())() + + remote := latest.Remote{URL: srv.URL + mcpPath + "?" + mcpQuery} + + errCh := make(chan error, 1) + go func() { errCh <- PerformOAuthLogin(t.Context(), remote) }() + + authURL := requireCapturedAuthorizeURL(t, captureFile) + deliverFakeCallback(t, authURL, "code-prm-resource-differs") + + require.NoError(t, <-errCh) + + parsedAuthURL, err := url.Parse(authURL) + require.NoError(t, err) + gotAuthorizeResource := parsedAuthURL.Query().Get("resource") + assert.Equal(t, remote.URL, gotAuthorizeResource, + "the /authorize resource parameter must be Remote.URL verbatim, including its query string") + + require.True(t, haveTokenResource, "token exchange request must carry a resource field") + assert.Equal(t, remote.URL, tokenResource, + "the token-exchange resource form value must be Remote.URL verbatim, including its query string") + + tok, err := store.GetToken(remote.URL) + require.NoError(t, err, "the stored token must be retrievable by Remote.URL verbatim, byte-for-byte, including its query string") + assert.Equal(t, "exchanged-at", tok.AccessToken) + + _, err = store.GetToken(prmResource) + require.Error(t, err, "the PRM-reported resource must never become a token-store key") + + assert.NotEqual(t, prmResource, gotAuthorizeResource, + "the PRM-reported resource must not leak into the /authorize resource parameter") + assert.NotEqual(t, prmResource, tokenResource, + "the PRM-reported resource must not leak into the token-exchange resource form value") +} diff --git a/pkg/tools/mcp/oauthflow/types.go b/pkg/tools/mcp/oauthflow/types.go index 876a5e5531..a2bd68a1d1 100644 --- a/pkg/tools/mcp/oauthflow/types.go +++ b/pkg/tools/mcp/oauthflow/types.go @@ -15,8 +15,11 @@ type OAuthToken struct { ClientSecret string `json:"client_secret,omitempty"` AuthServer string `json:"auth_server,omitempty"` - // RequestedScopes records the scope list the config asked for when this - // token was obtained. Unlike Scope (which is whatever the authorization + // RequestedScopes records the scope list actually requested when this + // token was obtained: the configured scopes, or — on a successful + // dynamic client registration with no configured override — the + // challenge- or protected-resource-metadata-derived scopes selected by + // selectDCRScopes. Unlike Scope (which is whatever the authorization // server chose to return, sometimes empty, sometimes comma/space // separated), RequestedScopes reflects our intent and is used to detect // when the config has changed and a new OAuth flow is required. diff --git a/pkg/tools/mcp/protected_resource_metadata_test.go b/pkg/tools/mcp/protected_resource_metadata_test.go new file mode 100644 index 0000000000..eddd4b3f88 --- /dev/null +++ b/pkg/tools/mcp/protected_resource_metadata_test.go @@ -0,0 +1,402 @@ +package mcp + +import ( + "fmt" + "net/http" + "net/http/httptest" + "sync/atomic" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestFetchProtectedResourceMetadata characterizes fetchProtectedResourceMetadata, +// the helper shared by the managed and driven-unmanaged OAuth flows, against +// the RFC 9728 discovery outcome/action table: exact challenged metadata is +// authoritative, a 404 defaults AuthorizationServers to the supplied origin +// without touching decoded Resource/ScopesSupported, and any decode failure +// or non-404/non-200 response is a hard error with exactly one request made. +func TestFetchProtectedResourceMetadata(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + status int + body string + wantErr string + wantResult protectedResourceMetadata + }{ + { + name: "exact challenged metadata with 200 is decoded and returned unmodified", + status: http.StatusOK, + body: `{"resource":"https://res.example.test","authorization_servers":["https://as.example.test"],"scopes_supported":["read","write"]}`, + wantResult: protectedResourceMetadata{ + Resource: "https://res.example.test", + AuthorizationServers: []string{"https://as.example.test"}, + ScopesSupported: []string{"read", "write"}, + }, + }, + { + name: "404 is not an error and defaults AuthorizationServers to the supplied origin", + status: http.StatusNotFound, + wantResult: protectedResourceMetadata{ + AuthorizationServers: []string{"https://auth.example.test"}, + }, + }, + { + name: "non-404/non-200 is a hard error", + status: http.StatusInternalServerError, + body: "boom", + wantErr: "failed to fetch protected resource metadata", + }, + { + name: "a non-200 2xx status (e.g. 204) is a hard error, not treated as success", + status: http.StatusNoContent, + wantErr: "failed to fetch protected resource metadata", + }, + { + name: "200 with an undecodable body is a hard error", + status: http.StatusOK, + body: "{not valid json", + wantErr: "invalid character", + }, + { + name: "200 with empty authorization_servers defaults to the supplied origin while preserving resource and scopes", + status: http.StatusOK, + body: `{"resource":"https://res.example.test","scopes_supported":["read"]}`, + wantResult: protectedResourceMetadata{ + Resource: "https://res.example.test", + AuthorizationServers: []string{"https://auth.example.test"}, + ScopesSupported: []string{"read"}, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + var requestCount atomic.Int32 + mux := http.NewServeMux() + mux.HandleFunc("/.well-known/oauth-protected-resource", func(w http.ResponseWriter, _ *http.Request) { + requestCount.Add(1) + if tt.body != "" { + w.Header().Set("Content-Type", "application/json") + } + w.WriteHeader(tt.status) + if tt.body != "" { + _, _ = w.Write([]byte(tt.body)) + } + }) + srv := httptest.NewServer(mux) + defer srv.Close() + + resourceURL := srv.URL + "/.well-known/oauth-protected-resource" + result, err := fetchProtectedResourceMetadata(t.Context(), srv.Client(), resourceURL, "https://auth.example.test", protectedResourceMetadataOptions{}) + + if tt.wantErr != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantErr) + assert.Equal(t, int32(1), requestCount.Load(), "a hard error must not retry or try another candidate") + return + } + require.NoError(t, err) + assert.Equal(t, tt.wantResult, result) + assert.Equal(t, int32(1), requestCount.Load(), "fallback is disabled by default: exactly one request for the exact candidate") + }) + } +} + +// TestFetchProtectedResourceMetadata_RuntimeFallbackDisabled proves the +// zero-value protectedResourceMetadataOptions (every runtime call site's +// default) never tries a fallback candidate: even when the primary +// candidate 404s, the result matches the current runtime outcome (default +// AuthorizationServers to the supplied origin) with no second request. +func TestFetchProtectedResourceMetadata_RuntimeFallbackDisabled(t *testing.T) { + t.Parallel() + + var primaryCalls, fallbackCalls atomic.Int32 + mux := http.NewServeMux() + mux.HandleFunc("/.well-known/oauth-protected-resource", func(w http.ResponseWriter, _ *http.Request) { + primaryCalls.Add(1) + w.WriteHeader(http.StatusNotFound) + }) + mux.HandleFunc("/mcp/.well-known/oauth-protected-resource", func(w http.ResponseWriter, _ *http.Request) { + fallbackCalls.Add(1) + w.WriteHeader(http.StatusNotFound) + }) + srv := httptest.NewServer(mux) + defer srv.Close() + + // opts is the zero value: FallbackCandidateURLs is nil, which is what + // every runtime caller (managed and driven-unmanaged flows) passes. + result, err := fetchProtectedResourceMetadata( + t.Context(), srv.Client(), + srv.URL+"/.well-known/oauth-protected-resource", + srv.URL, + protectedResourceMetadataOptions{}, + ) + require.NoError(t, err) + assert.Equal(t, []string{srv.URL}, result.AuthorizationServers) + assert.Equal(t, int32(1), primaryCalls.Load(), "only the primary candidate is tried when fallback is disabled") + assert.Equal(t, int32(0), fallbackCalls.Load(), "no path-aware fallback candidate must be tried when disabled (the default)") +} + +// TestFetchProtectedResourceMetadata_FallbackCandidatesTriedInOrderAfter404 +// exercises the opt-in path: when FallbackCandidateURLs is non-empty, they +// are tried in order only after the primary candidate 404s, and the walk +// stops at the first candidate that isn't a 404. Every candidate request +// uses GET. Runtime callers never set this; it exists for the standalone +// CLI discovery flow to build on. +func TestFetchProtectedResourceMetadata_FallbackCandidatesTriedInOrderAfter404(t *testing.T) { + t.Parallel() + + var order []string + var methods []string + mux := http.NewServeMux() + mux.HandleFunc("/primary", func(w http.ResponseWriter, r *http.Request) { + order = append(order, "primary") + methods = append(methods, r.Method) + w.WriteHeader(http.StatusNotFound) + }) + mux.HandleFunc("/fallback1", func(w http.ResponseWriter, r *http.Request) { + order = append(order, "fallback1") + methods = append(methods, r.Method) + w.WriteHeader(http.StatusNotFound) + }) + mux.HandleFunc("/fallback2", func(w http.ResponseWriter, r *http.Request) { + order = append(order, "fallback2") + methods = append(methods, r.Method) + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"resource":"` + r.Host + `","authorization_servers":["https://as.example.test"]}`)) + }) + srv := httptest.NewServer(mux) + defer srv.Close() + + result, err := fetchProtectedResourceMetadata( + t.Context(), srv.Client(), + srv.URL+"/primary", + "https://auth.example.test", + protectedResourceMetadataOptions{ + FallbackCandidateURLs: []string{srv.URL + "/fallback1", srv.URL + "/fallback2"}, + }, + ) + require.NoError(t, err) + assert.Equal(t, []string{"primary", "fallback1", "fallback2"}, order, + "candidates must be tried strictly in order, exactly once each, stopping at the first non-404") + assert.Equal(t, []string{http.MethodGet, http.MethodGet, http.MethodGet}, methods, + "every candidate request, including fallbacks, must use GET") + assert.Equal(t, []string{"https://as.example.test"}, result.AuthorizationServers) +} + +// TestFetchProtectedResourceMetadata_FallbackHardStopsAndExhaustion covers +// the remaining opt-in fallback outcomes required by the discovery +// outcome/action table: a decode failure or a non-404/non-200 status +// (including a non-200 2xx like 204, which must not be mistaken for +// success) on any candidate reached after a prior 404 is a hard error that +// tries no further candidate, and exhausting every candidate with 404s +// falls back to the supplied origin. Every request, including ones that +// never fire, is accounted for. +func TestFetchProtectedResourceMetadata_FallbackHardStopsAndExhaustion(t *testing.T) { + t.Parallel() + + const authServer = "https://auth.example.test" + + tests := []struct { + name string + statuses []int // one entry per candidate that must be reached + bodies []string + wantErr string + wantServers []string + }{ + { + name: "404 then invalid JSON on the fallback hard-stops with no later candidate tried", + statuses: []int{http.StatusNotFound, http.StatusOK}, + bodies: []string{"", "{not valid json"}, + wantErr: "invalid character", + }, + { + name: "404 then non-404/non-200 on the fallback hard-stops with no later candidate tried", + statuses: []int{http.StatusNotFound, http.StatusInternalServerError}, + bodies: []string{"", "boom"}, + wantErr: "failed to fetch protected resource metadata", + }, + { + name: "404 then a non-200 2xx (204) on the fallback hard-stops with no later candidate tried", + statuses: []int{http.StatusNotFound, http.StatusNoContent}, + bodies: []string{"", ""}, + wantErr: "failed to fetch protected resource metadata", + }, + { + name: "exhausting every candidate with 404 defaults AuthorizationServers to the supplied origin", + statuses: []int{http.StatusNotFound, http.StatusNotFound, http.StatusNotFound}, + bodies: []string{"", "", ""}, + wantServers: []string{authServer}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + const numCandidateSlots = 3 + var callCounts [numCandidateSlots]atomic.Int32 + var methods [numCandidateSlots]atomic.Value + + mux := http.NewServeMux() + for i := range numCandidateSlots { + mux.HandleFunc(fmt.Sprintf("/c%d", i), func(w http.ResponseWriter, r *http.Request) { + callCounts[i].Add(1) + methods[i].Store(r.Method) + if i >= len(tt.statuses) { + w.WriteHeader(http.StatusNotFound) + return + } + if tt.bodies[i] != "" { + w.Header().Set("Content-Type", "application/json") + } + w.WriteHeader(tt.statuses[i]) + if tt.bodies[i] != "" { + _, _ = w.Write([]byte(tt.bodies[i])) + } + }) + } + srv := httptest.NewServer(mux) + defer srv.Close() + + fallbacks := make([]string, numCandidateSlots-1) + for i := 1; i < numCandidateSlots; i++ { + fallbacks[i-1] = fmt.Sprintf("%s/c%d", srv.URL, i) + } + + result, err := fetchProtectedResourceMetadata( + t.Context(), srv.Client(), + srv.URL+"/c0", + authServer, + protectedResourceMetadataOptions{FallbackCandidateURLs: fallbacks}, + ) + + wantReached := len(tt.statuses) + for i := range numCandidateSlots { + wantCalls := int32(0) + if i < wantReached { + wantCalls = 1 + } + assert.Equal(t, wantCalls, callCounts[i].Load(), "candidate /c%d call count", i) + if wantCalls == 1 { + assert.Equal(t, http.MethodGet, methods[i].Load(), "candidate /c%d must be requested with GET", i) + } + } + + if tt.wantErr != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantErr) + return + } + require.NoError(t, err) + assert.Equal(t, tt.wantServers, result.AuthorizationServers) + }) + } +} + +// TestFetchProtectedResourceMetadata_NotFoundIsHardError proves the opt-in +// NotFoundIsHardError flag turns a 404 on the primary candidate into a hard +// error with zero fallback and zero further requests, while the flag stays +// default-off (false, the zero value) for every existing call and +// therefore never changes runtime behavior. +func TestFetchProtectedResourceMetadata_NotFoundIsHardError(t *testing.T) { + t.Parallel() + + var primaryCalls, fallbackCalls atomic.Int32 + mux := http.NewServeMux() + mux.HandleFunc("/primary", func(w http.ResponseWriter, _ *http.Request) { + primaryCalls.Add(1) + w.WriteHeader(http.StatusNotFound) + }) + mux.HandleFunc("/fallback", func(w http.ResponseWriter, _ *http.Request) { + fallbackCalls.Add(1) + w.WriteHeader(http.StatusNotFound) + }) + srv := httptest.NewServer(mux) + defer srv.Close() + + _, err := fetchProtectedResourceMetadata( + t.Context(), srv.Client(), + srv.URL+"/primary", + srv.URL, + protectedResourceMetadataOptions{ + // A fallback candidate is set to prove it is never tried: the + // hard error on the primary candidate must stop the walk before + // it ever reaches a fallback. + FallbackCandidateURLs: []string{srv.URL + "/fallback"}, + NotFoundIsHardError: true, + }, + ) + require.Error(t, err) + assert.Contains(t, err.Error(), "failed to fetch protected resource metadata") + assert.Equal(t, int32(1), primaryCalls.Load()) + assert.Equal(t, int32(0), fallbackCalls.Load(), "NotFoundIsHardError must stop before any fallback candidate is tried") +} + +// TestHandleManagedOAuthFlow_ProtectedResourceMetadataHardErrorStopsFlow and +// its unmanaged counterpart below prove that a hard PRM error (non-404/ +// non-200, or a decode failure) returned by fetchProtectedResourceMetadata +// stops the OAuth flow immediately: no authorization-server discovery, DCR, +// browser, elicitation, or token request follows. +func TestHandleManagedOAuthFlow_ProtectedResourceMetadataHardErrorStopsFlow(t *testing.T) { + t.Parallel() + + var authServerCalls atomic.Int32 + mux := http.NewServeMux() + mux.HandleFunc("/.well-known/oauth-protected-resource", func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + }) + mux.HandleFunc("/.well-known/oauth-authorization-server", func(w http.ResponseWriter, _ *http.Request) { + authServerCalls.Add(1) + w.WriteHeader(http.StatusNotFound) + }) + srv := httptest.NewServer(mux) + defer srv.Close() + + capture := &elicitCaptured{} + transport := &oauthTransport{ + base: newTestTransport(t), + requestElicitation: capture.handler, + tokenStore: NewInMemoryTokenStore(), + baseURL: srv.URL, + managed: true, + oauthHTTPClient: oauthHTTPClientForAllowPrivateIPs(true), + } + + err := transport.handleManagedOAuthFlow(t.Context(), srv.URL, "") + require.Error(t, err) + assert.Contains(t, err.Error(), "failed to fetch protected resource metadata") + assert.Equal(t, int32(0), authServerCalls.Load(), "a hard PRM error must stop before authorization-server discovery") + assert.Nil(t, capture.req, "no elicitation must be sent after a hard PRM error") +} + +func TestHandleUnmanagedOAuthFlow_ProtectedResourceMetadataHardErrorStopsFlow(t *testing.T) { + t.Parallel() + + var authServerCalls atomic.Int32 + mux := http.NewServeMux() + mux.HandleFunc("/.well-known/oauth-protected-resource", func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + }) + mux.HandleFunc("/.well-known/oauth-authorization-server", func(w http.ResponseWriter, _ *http.Request) { + authServerCalls.Add(1) + w.WriteHeader(http.StatusNotFound) + }) + srv := httptest.NewServer(mux) + defer srv.Close() + + capture := &elicitCaptured{} + transport, _ := newUnmanagedTestTransport(t, srv.URL, "", capture) + + err := transport.handleUnmanagedOAuthFlow(t.Context(), srv.URL, "") + require.Error(t, err) + assert.Contains(t, err.Error(), "failed to fetch protected resource metadata") + assert.Equal(t, int32(0), authServerCalls.Load(), "a hard PRM error must stop before authorization-server discovery") + assert.Nil(t, capture.req, "no elicitation must be sent after a hard PRM error") +}