mcp: add optional issuer validator for pre-registered client validation (SEP-2352) (#946)

Fixes #978
This commit is contained in:
Guglielmo Colombo 2026-05-29 14:46:34 +02:00 • committed by GitHub
commit dfb45f1e11
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
9 changed files with 190 additions and 5 deletions

View file

@ -507,6 +507,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 != "" && !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)
clientSecret := ""
if preCfg.ClientSecretAuth != nil {

View file

@ -609,6 +609,8 @@ func TestHandleRegistration(t *testing.T) {
asm *oauthex.AuthServerMeta
want *resolvedClientConfig
wantError bool
issuerMatch bool
issuerSuffix string
}{
{
name: "ClientIDMetadataDocument",
@ -647,6 +649,79 @@ 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,
},
issuerMatch: true,
},
{
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: "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,
},
issuerMatch: true,
issuerSuffix: "/",
},
{
name: "NoneSupported",
handlerConfig: &AuthorizationCodeHandlerConfig{
@ -660,6 +735,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)
// 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
}
@ -679,6 +758,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)
}

View file

@ -128,6 +128,11 @@ func (h *ClientCredentialsHandler) Authorize(ctx context.Context, req *http.Requ
}
}
creds := h.config.Credentials
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)
}
// Determine requestedScopes: use PRM's scopes_supported if available.
requestedScopes := scopesFromChallenges(wwwChallenges)
if len(requestedScopes) == 0 && len(prm.ScopesSupported) > 0 {
@ -140,7 +145,6 @@ func (h *ClientCredentialsHandler) Authorize(ctx context.Context, req *http.Requ
requestedScopes = authutil.UnionScopes(h.grantedScopes[asm.Issuer], requestedScopes)
// Step 3: Exchange client credentials for an access token.
creds := h.config.Credentials
cfg := &clientcredentials.Config{
ClientID: creds.ClientID,
ClientSecret: creds.ClientSecretAuth.ClientSecret,

View file

@ -221,6 +221,51 @@ 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("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{

13
internal/authutil/util.go Normal file
View file

@ -0,0 +1,13 @@
// 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 comparing them without the final trailing slash.
func IssuersEqual(a, b string) bool {
return strings.TrimSuffix(a, "/") == strings.TrimSuffix(b, "/")
}

View file

@ -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)
}
}
}

View file

@ -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)
}

View file

@ -18,6 +18,15 @@ 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.
// The comparison ignores a single trailing slash, matching the
// tolerance applied during RFC 8414 Section 3.3 metadata validation.
// OPTIONAL.
Issuer string
}
// ClientSecretAuth holds client secret authentication credentials.

View file

@ -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 {