diff --git a/auth/authorization_code.go b/auth/authorization_code.go index 1673a2e60..bb117caf6 100644 --- a/auth/authorization_code.go +++ b/auth/authorization_code.go @@ -17,6 +17,7 @@ import ( "sync" "github.com/modelcontextprotocol/go-sdk/internal/authutil" + "github.com/modelcontextprotocol/go-sdk/internal/mcpgodebug" "github.com/modelcontextprotocol/go-sdk/internal/util" "github.com/modelcontextprotocol/go-sdk/oauthex" "golang.org/x/oauth2" @@ -156,6 +157,13 @@ type AuthorizationCodeHandlerConfig struct { InitialTokenSource oauth2.TokenSource } +// noboundscopetodcr disables propagating discovered scopes into dynamic client +// registration metadata. When set to "1", the client will not automatically +// set the scope in DCR metadata from the requested scopes, restoring the +// previous behavior. See the documentation for the mcpgodebug package for +// instructions how to enable it. +var noboundscopetodcr = mcpgodebug.Value("noboundscopetodcr") + // AuthorizationCodeHandler is an implementation of [OAuthHandler] that uses // the authorization code flow to obtain access tokens. type AuthorizationCodeHandler struct { @@ -317,11 +325,6 @@ func (h *AuthorizationCodeHandler) Authorize(ctx context.Context, req *http.Requ } } - resolvedClientConfig, err := h.handleRegistration(ctx, asm) - if err != nil { - return err - } - requestedScopes := scopesFromChallenges(wwwChallenges) if len(requestedScopes) == 0 && len(prm.ScopesSupported) > 0 { requestedScopes = prm.ScopesSupported @@ -349,6 +352,21 @@ func (h *AuthorizationCodeHandler) Authorize(ctx context.Context, req *http.Requ h.mu.RUnlock() requestedScopes = authutil.UnionScopes(granted, requestedScopes) + // Propagate discovered scopes into the DCR metadata before registration, + // so the client is registered for the same scopes it will request. + // This prevents invalid_scope errors on strict authorization servers. + // Setting MCPGODEBUG=noboundscopetodcr=1 restores the previous behavior. + if noboundscopetodcr != "1" { + if dcrCfg := h.config.DynamicClientRegistrationConfig; dcrCfg != nil && dcrCfg.Metadata.Scope == "" && len(requestedScopes) > 0 { + dcrCfg.Metadata.Scope = strings.Join(requestedScopes, " ") + } + } + + resolvedClientConfig, err := h.handleRegistration(ctx, asm) + if err != nil { + return err + } + cfg := &oauth2.Config{ ClientID: resolvedClientConfig.clientID, ClientSecret: resolvedClientConfig.clientSecret, diff --git a/auth/authorization_code_test.go b/auth/authorization_code_test.go index c2f963f7a..ae4e87402 100644 --- a/auth/authorization_code_test.go +++ b/auth/authorization_code_test.go @@ -915,6 +915,179 @@ func TestDynamicRegistration(t *testing.T) { } } +func TestDCRScopePropagation(t *testing.T) { + s := oauthtest.NewFakeAuthorizationServer(oauthtest.Config{ + RegistrationConfig: &oauthtest.RegistrationConfig{ + DynamicClientRegistrationEnabled: true, + }, + ScopesSupported: []string{"read", "write"}, + }) + s.Start(t) + + resourceMux := http.NewServeMux() + resourceServer := httptest.NewServer(resourceMux) + t.Cleanup(resourceServer.Close) + resourceURL := resourceServer.URL + "/resource" + + resourceMux.Handle("/.well-known/oauth-protected-resource/resource", ProtectedResourceMetadataHandler(&oauthex.ProtectedResourceMetadata{ + Resource: resourceURL, + AuthorizationServers: []string{s.URL()}, + })) + + dcrMetadata := &oauthex.ClientRegistrationMetadata{ + RedirectURIs: []string{"http://localhost:12345/callback"}, + } + handler, err := NewAuthorizationCodeHandler(&AuthorizationCodeHandlerConfig{ + DynamicClientRegistrationConfig: &DynamicClientRegistrationConfig{ + Metadata: dcrMetadata, + }, + RedirectURL: "http://localhost:12345/callback", + AuthorizationCodeFetcher: func(ctx context.Context, args *AuthorizationArgs) (*AuthorizationResult, error) { + client := &http.Client{ + CheckRedirect: func(req *http.Request, via []*http.Request) error { + return http.ErrUseLastResponse + }, + } + resp, err := client.Get(args.URL) + if err != nil { + return nil, fmt.Errorf("failed to visit auth URL: %v", err) + } + defer resp.Body.Close() + location, err := resp.Location() + if err != nil { + return nil, fmt.Errorf("failed to get location header: %v", err) + } + return &AuthorizationResult{ + Code: location.Query().Get("code"), + State: location.Query().Get("state"), + Iss: location.Query().Get("iss"), + }, nil + }, + }) + if err != nil { + t.Fatalf("NewAuthorizationCodeHandler() error = %v", err) + } + + req := httptest.NewRequest(http.MethodGet, resourceURL, nil) + resp := &http.Response{ + StatusCode: http.StatusUnauthorized, + Header: make(http.Header), + Body: http.NoBody, + Request: req, + } + resp.Header.Set( + "WWW-Authenticate", + "Bearer scope=\"read write\", resource_metadata="+resourceServer.URL+"/.well-known/oauth-protected-resource/resource", + ) + + if err := handler.Authorize(context.Background(), req, resp); err != nil { + t.Fatalf("Authorize failed: %v", err) + } + + if got := dcrMetadata.Scope; got != "read write" { + t.Errorf("DCR metadata Scope = %q, want %q", got, "read write") + } + + tokenSource, err := handler.TokenSource(t.Context()) + if err != nil { + t.Fatalf("Failed to get token source: %v", err) + } + token, err := tokenSource.Token() + if err != nil { + t.Fatalf("Failed to get token: %v", err) + } + if token.AccessToken != "test_access_token" { + t.Errorf("Expected access token 'test_access_token', got '%s'", token.AccessToken) + } +} + +func TestDCRScopePropagation_PreservesExplicitScope(t *testing.T) { + s := oauthtest.NewFakeAuthorizationServer(oauthtest.Config{ + RegistrationConfig: &oauthtest.RegistrationConfig{ + DynamicClientRegistrationEnabled: true, + }, + ScopesSupported: []string{"read", "write"}, + }) + s.Start(t) + + resourceMux := http.NewServeMux() + resourceServer := httptest.NewServer(resourceMux) + t.Cleanup(resourceServer.Close) + resourceURL := resourceServer.URL + "/resource" + + resourceMux.Handle("/.well-known/oauth-protected-resource/resource", ProtectedResourceMetadataHandler(&oauthex.ProtectedResourceMetadata{ + Resource: resourceURL, + AuthorizationServers: []string{s.URL()}, + })) + + dcrMetadata := &oauthex.ClientRegistrationMetadata{ + RedirectURIs: []string{"http://localhost:12345/callback"}, + Scope: "explicit_scope", + } + handler, err := NewAuthorizationCodeHandler(&AuthorizationCodeHandlerConfig{ + DynamicClientRegistrationConfig: &DynamicClientRegistrationConfig{ + Metadata: dcrMetadata, + }, + RedirectURL: "http://localhost:12345/callback", + AuthorizationCodeFetcher: func(ctx context.Context, args *AuthorizationArgs) (*AuthorizationResult, error) { + client := &http.Client{ + CheckRedirect: func(req *http.Request, via []*http.Request) error { + return http.ErrUseLastResponse + }, + } + resp, err := client.Get(args.URL) + if err != nil { + return nil, fmt.Errorf("failed to visit auth URL: %v", err) + } + defer resp.Body.Close() + location, err := resp.Location() + if err != nil { + return nil, fmt.Errorf("failed to get location header: %v", err) + } + return &AuthorizationResult{ + Code: location.Query().Get("code"), + State: location.Query().Get("state"), + Iss: location.Query().Get("iss"), + }, nil + }, + }) + if err != nil { + t.Fatalf("NewAuthorizationCodeHandler() error = %v", err) + } + + req := httptest.NewRequest(http.MethodGet, resourceURL, nil) + resp := &http.Response{ + StatusCode: http.StatusUnauthorized, + Header: make(http.Header), + Body: http.NoBody, + Request: req, + } + resp.Header.Set( + "WWW-Authenticate", + "Bearer scope=\"read write\", resource_metadata="+resourceServer.URL+"/.well-known/oauth-protected-resource/resource", + ) + + if err := handler.Authorize(context.Background(), req, resp); err != nil { + t.Fatalf("Authorize failed: %v", err) + } + + if got := dcrMetadata.Scope; got != "explicit_scope" { + t.Errorf("DCR metadata Scope = %q, want %q (explicit scope should not be overridden)", got, "explicit_scope") + } + + tokenSource, err := handler.TokenSource(t.Context()) + if err != nil { + t.Fatalf("Failed to get token source: %v", err) + } + token, err := tokenSource.Token() + if err != nil { + t.Fatalf("Failed to get token: %v", err) + } + if token.AccessToken != "test_access_token" { + t.Errorf("Expected access token 'test_access_token', got '%s'", token.AccessToken) + } +} + func TestValidateIssuerResponse(t *testing.T) { const expectedIssuer = "https://auth.example.com" diff --git a/docs/mcpgodebug.md b/docs/mcpgodebug.md index d25b9b49e..1bcdc1e78 100644 --- a/docs/mcpgodebug.md +++ b/docs/mcpgodebug.md @@ -92,6 +92,12 @@ Options listed below were added and will be removed in the 1.9.0 version of the the request to the completion handler unconditionally. The default behavior was changed to reject malformed requests with `-32602` (Invalid Params). +- `noboundscopetodcr` added. If set to `1`, the authorization code handler will + not automatically propagate discovered scopes into dynamic client registration + metadata, restoring the previous behavior. The default behavior was changed to + automatically set the scope in DCR metadata from the requested scopes to + prevent `invalid_scope` errors on strict authorization servers. + ### 1.6.1 Options listed below were added and will be removed in the 1.8.0 version of the SDK. diff --git a/internal/docs/mcpgodebug.src.md b/internal/docs/mcpgodebug.src.md index 32f8af261..964915029 100644 --- a/internal/docs/mcpgodebug.src.md +++ b/internal/docs/mcpgodebug.src.md @@ -91,6 +91,12 @@ Options listed below were added and will be removed in the 1.9.0 version of the the request to the completion handler unconditionally. The default behavior was changed to reject malformed requests with `-32602` (Invalid Params). +- `noboundscopetodcr` added. If set to `1`, the authorization code handler will + not automatically propagate discovered scopes into dynamic client registration + metadata, restoring the previous behavior. The default behavior was changed to + automatically set the scope in DCR metadata from the requested scopes to + prevent `invalid_scope` errors on strict authorization servers. + ### 1.6.1 Options listed below were added and will be removed in the 1.8.0 version of the SDK. diff --git a/internal/oauthtest/fake_authorization_server.go b/internal/oauthtest/fake_authorization_server.go index e5134fb50..78b2ce529 100644 --- a/internal/oauthtest/fake_authorization_server.go +++ b/internal/oauthtest/fake_authorization_server.go @@ -24,8 +24,9 @@ import ( ) type ClientInfo struct { - Secret string - RedirectURIs []string + Secret string + RedirectURIs []string + RegisteredScope string } type MetadataEndpointConfig struct { @@ -245,8 +246,9 @@ func (s *FakeAuthorizationServer) handleRegister(w http.ResponseWriter, r *http. w.WriteHeader(http.StatusCreated) clientID := rand.Text() ci := ClientInfo{ - Secret: rand.Text(), - RedirectURIs: metadata.RedirectURIs, + Secret: rand.Text(), + RedirectURIs: metadata.RedirectURIs, + RegisteredScope: metadata.Scope, } s.clients[clientID] = ci metadata.TokenEndpointAuthMethod = "client_secret_basic" @@ -443,3 +445,9 @@ func (s *FakeAuthorizationServer) authenticateClient(r *http.Request) error { } return nil } + +// GetClient returns the ClientInfo for the given clientID, or false if not found. +func (s *FakeAuthorizationServer) GetClient(clientID string) (ClientInfo, bool) { + ci, ok := s.clients[clientID] + return ci, ok +}