diff --git a/auth/authorization_code.go b/auth/authorization_code.go index f349842f..1b6e097d 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" @@ -161,6 +162,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 { @@ -322,11 +330,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 @@ -354,6 +357,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 69a7ca60..8644722d 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 43a62ce8..f70b373c 100644 --- a/docs/mcpgodebug.md +++ b/docs/mcpgodebug.md @@ -119,6 +119,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 8d1385f2..fd4dc583 100644 --- a/internal/docs/mcpgodebug.src.md +++ b/internal/docs/mcpgodebug.src.md @@ -118,6 +118,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 4cd68264..1fa040c5 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 { @@ -251,8 +252,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" @@ -449,3 +451,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 +}