From 5f304460d148d8b1e969fbb74381f24c5227aa3e Mon Sep 17 00:00:00 2001 From: guglielmoc Date: Wed, 6 May 2026 12:46:24 +0000 Subject: [PATCH 1/3] feat: add optional issuer validation to ClientCredentials to ensure binding to a specific authorization server --- auth/authorization_code.go | 3 ++ auth/authorization_code_test.go | 52 +++++++++++++++++++++++++ auth/extauth/client_credentials.go | 6 ++- auth/extauth/client_credentials_test.go | 23 +++++++++++ oauthex/client.go | 7 ++++ oauthex/client_test.go | 2 +- 6 files changed, 91 insertions(+), 2 deletions(-) diff --git a/auth/authorization_code.go b/auth/authorization_code.go index 758b88e3..e4828d85 100644 --- a/auth/authorization_code.go +++ b/auth/authorization_code.go @@ -456,6 +456,9 @@ func (h *AuthorizationCodeHandler) handleRegistration(ctx context.Context, asm * // 2. Attempt to use pre-registered client configuration. preCfg := h.config.PreregisteredClient if preCfg != nil { + if preCfg.Issuer != "" && preCfg.Issuer != asm.Issuer { + return nil, fmt.Errorf("authorization server issuer %q does not match pre-registered credentials issuer %q", asm.Issuer, preCfg.Issuer) + } authStyle := selectTokenAuthMethod(asm.TokenEndpointAuthMethodsSupported) clientSecret := "" if preCfg.ClientSecretAuth != nil { diff --git a/auth/authorization_code_test.go b/auth/authorization_code_test.go index 92b6faf8..7dc4c3df 100644 --- a/auth/authorization_code_test.go +++ b/auth/authorization_code_test.go @@ -517,6 +517,51 @@ func TestHandleRegistration(t *testing.T) { authStyle: oauth2.AuthStyleInParams, }, }, + { + name: "Preregistered_IssuerMatch", + serverConfig: &oauthtest.RegistrationConfig{ + PreregisteredClients: map[string]oauthtest.ClientInfo{ + "pre_client_id": { + Secret: "pre_client_secret", + }, + }, + }, + handlerConfig: &AuthorizationCodeHandlerConfig{ + PreregisteredClient: &oauthex.ClientCredentials{ + ClientID: "pre_client_id", + ClientSecretAuth: &oauthex.ClientSecretAuth{ + ClientSecret: "pre_client_secret", + }, + Issuer: "", // set dynamically in the test + }, + }, + want: &resolvedClientConfig{ + registrationType: registrationTypePreregistered, + clientID: "pre_client_id", + clientSecret: "pre_client_secret", + authStyle: oauth2.AuthStyleInParams, + }, + }, + { + name: "Preregistered_IssuerMismatch", + serverConfig: &oauthtest.RegistrationConfig{ + PreregisteredClients: map[string]oauthtest.ClientInfo{ + "pre_client_id": { + Secret: "pre_client_secret", + }, + }, + }, + handlerConfig: &AuthorizationCodeHandlerConfig{ + PreregisteredClient: &oauthex.ClientCredentials{ + ClientID: "pre_client_id", + ClientSecretAuth: &oauthex.ClientSecretAuth{ + ClientSecret: "pre_client_secret", + }, + Issuer: "https://other-issuer.example.com", + }, + }, + wantError: true, + }, { name: "NoneSupported", handlerConfig: &AuthorizationCodeHandlerConfig{ @@ -530,6 +575,10 @@ func TestHandleRegistration(t *testing.T) { t.Run(tt.name, func(t *testing.T) { s := oauthtest.NewFakeAuthorizationServer(oauthtest.Config{RegistrationConfig: tt.serverConfig}) s.Start(t) + // For the IssuerMatch test, set the Issuer to the actual server URL. + if tt.name == "Preregistered_IssuerMatch" { + tt.handlerConfig.PreregisteredClient.Issuer = s.URL() + } tt.handlerConfig.AuthorizationCodeFetcher = func(ctx context.Context, args *AuthorizationArgs) (*AuthorizationResult, error) { return nil, nil } @@ -549,6 +598,9 @@ func TestHandleRegistration(t *testing.T) { } return } + if tt.wantError { + t.Fatal("handleRegistration() expected error, got nil") + } if got.registrationType != tt.want.registrationType { t.Errorf("handleRegistration() registrationType = %v, want %v", got.registrationType, tt.want.registrationType) } diff --git a/auth/extauth/client_credentials.go b/auth/extauth/client_credentials.go index fefa29b9..2062b33c 100644 --- a/auth/extauth/client_credentials.go +++ b/auth/extauth/client_credentials.go @@ -121,6 +121,11 @@ func (h *ClientCredentialsHandler) Authorize(ctx context.Context, req *http.Requ } } + creds := h.config.Credentials + if creds.Issuer != "" && creds.Issuer != asm.Issuer { + return fmt.Errorf("authorization server issuer %q does not match pre-registered credentials issuer %q", asm.Issuer, creds.Issuer) + } + // Determine scopes: use PRM's scopes_supported if available. scopes := scopesFromChallenges(wwwChallenges) if len(scopes) == 0 && len(prm.ScopesSupported) > 0 { @@ -128,7 +133,6 @@ func (h *ClientCredentialsHandler) Authorize(ctx context.Context, req *http.Requ } // Step 3: Exchange client credentials for an access token. - creds := h.config.Credentials cfg := &clientcredentials.Config{ ClientID: creds.ClientID, ClientSecret: creds.ClientSecretAuth.ClientSecret, diff --git a/auth/extauth/client_credentials_test.go b/auth/extauth/client_credentials_test.go index 91ae7c04..598bef9f 100644 --- a/auth/extauth/client_credentials_test.go +++ b/auth/extauth/client_credentials_test.go @@ -219,6 +219,29 @@ func TestClientCredentialsHandler_Authorize(t *testing.T) { } }) + t.Run("issuer mismatch", func(t *testing.T) { + config := validClientCredentialsConfig() + config.Credentials.Issuer = "https://other-issuer.example.com" + handler, err := NewClientCredentialsHandler(config) + if err != nil { + t.Fatal(err) + } + + resp := &http.Response{ + StatusCode: http.StatusUnauthorized, + Header: http.Header{}, + Body: http.NoBody, + } + req := httptest.NewRequest("GET", resourceURL, nil) + err = handler.Authorize(t.Context(), req, resp) + if err == nil { + t.Fatal("expected Authorize to fail with issuer mismatch") + } + if !strings.Contains(err.Error(), "does not match") { + t.Errorf("error %q does not mention issuer mismatch", err.Error()) + } + }) + t.Run("PRM via resource_metadata in challenge", func(t *testing.T) { prmMux := http.NewServeMux() prmMux.Handle("/custom-prm", auth.ProtectedResourceMetadataHandler(&oauthex.ProtectedResourceMetadata{ diff --git a/oauthex/client.go b/oauthex/client.go index e8f99182..ca41cad6 100644 --- a/oauthex/client.go +++ b/oauthex/client.go @@ -18,6 +18,13 @@ type ClientCredentials struct { // This is the most common authentication method for confidential clients. // OPTIONAL. If not provided, the client is treated as a public client. ClientSecretAuth *ClientSecretAuth + + // Issuer is the issuer identifier of the authorization server these + // credentials are registered with. Pre-registered credentials are bound + // to a specific authorization server; when set, an error is returned if + // the discovered authorization server does not match, per SEP-2352. + // OPTIONAL. + Issuer string } // ClientSecretAuth holds client secret authentication credentials. diff --git a/oauthex/client_test.go b/oauthex/client_test.go index b78e9c8b..34d8188c 100644 --- a/oauthex/client_test.go +++ b/oauthex/client_test.go @@ -73,7 +73,7 @@ func TestClientCredentials_ValidateCoversAllAuthFields(t *testing.T) { var pointerFields int for i := range typ.NumField() { f := typ.Field(i) - if f.Name == "ClientID" { + if f.Name == "ClientID" || f.Name == "Issuer" { continue } if f.Type.Kind() != reflect.Ptr { From e57c85807ab600a2015dec39aad6f02181986e43 Mon Sep 17 00:00:00 2001 From: guglielmoc Date: Wed, 27 May 2026 10:10:45 +0000 Subject: [PATCH 2/3] feat: add authutil.IssuersEqual and update issuer validation to ignore trailing slashes --- auth/authorization_code.go | 2 +- auth/authorization_code_test.go | 30 +++++++++++++++++++++++++ auth/extauth/client_credentials.go | 2 +- auth/extauth/client_credentials_test.go | 22 ++++++++++++++++++ internal/authutil/util.go | 19 ++++++++++++++++ internal/authutil/util_test.go | 29 ++++++++++++++++++++++++ oauthex/client.go | 2 ++ 7 files changed, 104 insertions(+), 2 deletions(-) create mode 100644 internal/authutil/util.go create mode 100644 internal/authutil/util_test.go diff --git a/auth/authorization_code.go b/auth/authorization_code.go index da53034e..a3daeecb 100644 --- a/auth/authorization_code.go +++ b/auth/authorization_code.go @@ -507,7 +507,7 @@ func (h *AuthorizationCodeHandler) handleRegistration(ctx context.Context, asm * // 2. Attempt to use pre-registered client configuration. preCfg := h.config.PreregisteredClient if preCfg != nil { - if preCfg.Issuer != "" && preCfg.Issuer != asm.Issuer { + if preCfg.Issuer != "" && !authutil.IssuersEqual(preCfg.Issuer, asm.Issuer) { return nil, fmt.Errorf("authorization server issuer %q does not match pre-registered credentials issuer %q", asm.Issuer, preCfg.Issuer) } authStyle := selectTokenAuthMethod(asm.TokenEndpointAuthMethodsSupported) diff --git a/auth/authorization_code_test.go b/auth/authorization_code_test.go index 94064628..1255c7b7 100644 --- a/auth/authorization_code_test.go +++ b/auth/authorization_code_test.go @@ -692,6 +692,31 @@ func TestHandleRegistration(t *testing.T) { }, wantError: true, }, + { + name: "Preregistered_IssuerMatchTrailingSlash", + serverConfig: &oauthtest.RegistrationConfig{ + PreregisteredClients: map[string]oauthtest.ClientInfo{ + "pre_client_id": { + Secret: "pre_client_secret", + }, + }, + }, + handlerConfig: &AuthorizationCodeHandlerConfig{ + PreregisteredClient: &oauthex.ClientCredentials{ + ClientID: "pre_client_id", + ClientSecretAuth: &oauthex.ClientSecretAuth{ + ClientSecret: "pre_client_secret", + }, + Issuer: "", // set dynamically in the test (with trailing slash) + }, + }, + want: &resolvedClientConfig{ + registrationType: registrationTypePreregistered, + clientID: "pre_client_id", + clientSecret: "pre_client_secret", + authStyle: oauth2.AuthStyleInParams, + }, + }, { name: "NoneSupported", handlerConfig: &AuthorizationCodeHandlerConfig{ @@ -709,6 +734,11 @@ func TestHandleRegistration(t *testing.T) { if tt.name == "Preregistered_IssuerMatch" { tt.handlerConfig.PreregisteredClient.Issuer = s.URL() } + // For the trailing-slash variant, append a trailing slash so the + // configured issuer differs only in normalization from asm.Issuer. + if tt.name == "Preregistered_IssuerMatchTrailingSlash" { + tt.handlerConfig.PreregisteredClient.Issuer = s.URL() + "/" + } tt.handlerConfig.AuthorizationCodeFetcher = func(ctx context.Context, args *AuthorizationArgs) (*AuthorizationResult, error) { return nil, nil } diff --git a/auth/extauth/client_credentials.go b/auth/extauth/client_credentials.go index c4f0789f..c65d8b06 100644 --- a/auth/extauth/client_credentials.go +++ b/auth/extauth/client_credentials.go @@ -129,7 +129,7 @@ func (h *ClientCredentialsHandler) Authorize(ctx context.Context, req *http.Requ } creds := h.config.Credentials - if creds.Issuer != "" && creds.Issuer != asm.Issuer { + if creds.Issuer != "" && !authutil.IssuersEqual(creds.Issuer, asm.Issuer) { return fmt.Errorf("authorization server issuer %q does not match pre-registered credentials issuer %q", asm.Issuer, creds.Issuer) } diff --git a/auth/extauth/client_credentials_test.go b/auth/extauth/client_credentials_test.go index 0531cf62..b2973de7 100644 --- a/auth/extauth/client_credentials_test.go +++ b/auth/extauth/client_credentials_test.go @@ -244,6 +244,28 @@ func TestClientCredentialsHandler_Authorize(t *testing.T) { } }) + t.Run("issuer match ignoring trailing slash", func(t *testing.T) { + config := validClientCredentialsConfig() + // authServer.URL() has no trailing slash; configure with one to + // verify the comparison tolerates the difference (per RFC 8414 ยง3.3 + // normalization applied in oauthex.IssuersEqual). + config.Credentials.Issuer = authServer.URL() + "/" + handler, err := NewClientCredentialsHandler(config) + if err != nil { + t.Fatal(err) + } + + resp := &http.Response{ + StatusCode: http.StatusUnauthorized, + Header: http.Header{}, + Body: http.NoBody, + } + req := httptest.NewRequest("GET", resourceURL, nil) + if err := handler.Authorize(t.Context(), req, resp); err != nil { + t.Fatalf("Authorize() unexpected error = %v", err) + } + }) + t.Run("PRM via resource_metadata in challenge", func(t *testing.T) { prmMux := http.NewServeMux() prmMux.Handle("/custom-prm", auth.ProtectedResourceMetadataHandler(&oauthex.ProtectedResourceMetadata{ diff --git a/internal/authutil/util.go b/internal/authutil/util.go new file mode 100644 index 00000000..dc485c0e --- /dev/null +++ b/internal/authutil/util.go @@ -0,0 +1,19 @@ +// Copyright 2026 The Go MCP SDK Authors. All rights reserved. +// Use of this source code is governed by the license +// that can be found in the LICENSE file. + +package authutil + +import "strings" + +// IssuersEqual reports whether two OAuth 2.0 authorization server issuer +// identifiers refer to the same server. The comparison ignores a single +// trailing slash, matching the tolerance applied during RFC 8414 Section 3.3 +// metadata validation. +// +// This helper is not appropriate for comparing the RFC 9207 iss response +// parameter, which the MCP authorization spec requires to be compared without +// any normalization. +func IssuersEqual(a, b string) bool { + return strings.TrimRight(a, "/") == strings.TrimRight(b, "/") +} diff --git a/internal/authutil/util_test.go b/internal/authutil/util_test.go new file mode 100644 index 00000000..9d3ff7a4 --- /dev/null +++ b/internal/authutil/util_test.go @@ -0,0 +1,29 @@ +// Copyright 2026 The Go MCP SDK Authors. All rights reserved. +// Use of this source code is governed by the license +// that can be found in the LICENSE file. + +package authutil + +import "testing" + +func TestIssuersEqual(t *testing.T) { + tests := []struct { + a, b string + want bool + }{ + {"https://issuer.example.com", "https://issuer.example.com", true}, + {"https://issuer.example.com/", "https://issuer.example.com", true}, + {"https://issuer.example.com", "https://issuer.example.com/", true}, + {"https://issuer.example.com/", "https://issuer.example.com/", true}, + {"https://issuer.example.com/tenant", "https://issuer.example.com/tenant", true}, + {"https://issuer.example.com/tenant/", "https://issuer.example.com/tenant", true}, + {"https://issuer.example.com", "https://other.example.com", false}, + {"https://issuer.example.com/a", "https://issuer.example.com/b", false}, + {"", "", true}, + } + for _, tt := range tests { + if got := IssuersEqual(tt.a, tt.b); got != tt.want { + t.Errorf("IssuersEqual(%q, %q) = %v, want %v", tt.a, tt.b, got, tt.want) + } + } +} diff --git a/oauthex/client.go b/oauthex/client.go index ca41cad6..1b19eed5 100644 --- a/oauthex/client.go +++ b/oauthex/client.go @@ -23,6 +23,8 @@ type ClientCredentials struct { // credentials are registered with. Pre-registered credentials are bound // to a specific authorization server; when set, an error is returned if // the discovered authorization server does not match, per SEP-2352. + // The comparison ignores a single trailing slash, matching the + // tolerance applied during RFC 8414 Section 3.3 metadata validation. // OPTIONAL. Issuer string } From 5ffed1580ab9c1ebe2a252deea2304fc8dabff3b Mon Sep 17 00:00:00 2001 From: guglielmoc Date: Thu, 28 May 2026 14:22:46 +0000 Subject: [PATCH 3/3] refactor: standardize issuer comparison to use consistent trailing slash removal and update test infrastructure --- auth/authorization_code_test.go | 16 ++++++++-------- internal/authutil/util.go | 10 ++-------- oauthex/auth_meta.go | 6 +++--- 3 files changed, 13 insertions(+), 19 deletions(-) diff --git a/auth/authorization_code_test.go b/auth/authorization_code_test.go index 1255c7b7..c84b0032 100644 --- a/auth/authorization_code_test.go +++ b/auth/authorization_code_test.go @@ -609,6 +609,8 @@ func TestHandleRegistration(t *testing.T) { asm *oauthex.AuthServerMeta want *resolvedClientConfig wantError bool + issuerMatch bool + issuerSuffix string }{ { name: "ClientIDMetadataDocument", @@ -671,6 +673,7 @@ func TestHandleRegistration(t *testing.T) { clientSecret: "pre_client_secret", authStyle: oauth2.AuthStyleInParams, }, + issuerMatch: true, }, { name: "Preregistered_IssuerMismatch", @@ -716,6 +719,8 @@ func TestHandleRegistration(t *testing.T) { clientSecret: "pre_client_secret", authStyle: oauth2.AuthStyleInParams, }, + issuerMatch: true, + issuerSuffix: "/", }, { name: "NoneSupported", @@ -730,14 +735,9 @@ func TestHandleRegistration(t *testing.T) { t.Run(tt.name, func(t *testing.T) { s := oauthtest.NewFakeAuthorizationServer(oauthtest.Config{RegistrationConfig: tt.serverConfig}) s.Start(t) - // For the IssuerMatch test, set the Issuer to the actual server URL. - if tt.name == "Preregistered_IssuerMatch" { - tt.handlerConfig.PreregisteredClient.Issuer = s.URL() - } - // For the trailing-slash variant, append a trailing slash so the - // configured issuer differs only in normalization from asm.Issuer. - if tt.name == "Preregistered_IssuerMatchTrailingSlash" { - tt.handlerConfig.PreregisteredClient.Issuer = s.URL() + "/" + // Set the Issuer dynamically if requested by the test case. + if tt.issuerMatch { + tt.handlerConfig.PreregisteredClient.Issuer = s.URL() + tt.issuerSuffix } tt.handlerConfig.AuthorizationCodeFetcher = func(ctx context.Context, args *AuthorizationArgs) (*AuthorizationResult, error) { return nil, nil diff --git a/internal/authutil/util.go b/internal/authutil/util.go index dc485c0e..880d1bfa 100644 --- a/internal/authutil/util.go +++ b/internal/authutil/util.go @@ -7,13 +7,7 @@ package authutil import "strings" // IssuersEqual reports whether two OAuth 2.0 authorization server issuer -// identifiers refer to the same server. The comparison ignores a single -// trailing slash, matching the tolerance applied during RFC 8414 Section 3.3 -// metadata validation. -// -// This helper is not appropriate for comparing the RFC 9207 iss response -// parameter, which the MCP authorization spec requires to be compared without -// any normalization. +// identifiers refer to the same server comparing them without the final trailing slash. func IssuersEqual(a, b string) bool { - return strings.TrimRight(a, "/") == strings.TrimRight(b, "/") + return strings.TrimSuffix(a, "/") == strings.TrimSuffix(b, "/") } diff --git a/oauthex/auth_meta.go b/oauthex/auth_meta.go index 7ebf6e22..711b17a4 100644 --- a/oauthex/auth_meta.go +++ b/oauthex/auth_meta.go @@ -12,7 +12,8 @@ import ( "errors" "fmt" "net/http" - "strings" + + "github.com/modelcontextprotocol/go-sdk/internal/authutil" ) // AuthServerMeta represents the metadata for an OAuth 2.0 authorization server, @@ -153,8 +154,7 @@ func GetAuthServerMeta(ctx context.Context, metadataURL, issuer string, c *http. } return nil, fmt.Errorf("%v", err) // Do not expose error types. } - if strings.TrimRight(asm.Issuer, "/") != strings.TrimRight(issuer, "/") { - // Validate the Issuer field (see RFC 8414, section 3.3). + if !authutil.IssuersEqual(asm.Issuer, issuer) { return nil, fmt.Errorf("metadata issuer %q does not match issuer URL %q", asm.Issuer, issuer) }