An issuer identifier written with a terminating slash, like `https://auth.example.com/`, makes authorization server metadata discovery miss every endpoint. `GetAuthServerMetadata` probes three URLs that no conformant server serves, finds nothing, and the caller falls back to the guessed 2025-03-26 endpoints, which are themselves malformed because they are built by appending to the same slash-terminated issuer. `authorizationServerMetadataURLs` in `auth/shared.go` decides whether the issuer has a path component with `baseURL.Path == ""` (line 47), and a trailing-slash issuer has `Path == "/"`, so it takes the path-insertion branch. That branch inserts `strings.TrimLeft(originalPath, "/")` (lines 59 and 62), which strips the leading slash but keeps the trailing one. For `https://auth.example.com/` the probes come out as `/.well-known/oauth-authorization-server/`, `/.well-known/openid-configuration/` and `//.well-known/openid-configuration`. For `https://auth.example.com/tenant1/` the two path-insertion probes come out with a trailing slash. Only the path-appending form was already right, because it uses `strings.Trim`. RFC 8414 section 3.1 settles it: "If the issuer identifier value contains a path component, any terminating "/" MUST be removed before inserting "/.well-known/" and the well-known URI suffix between the host component and the path component." So `https://auth.example.com/` has no path component and gets the two root forms, which is the list the MCP authorization spec gives for issuers without a path component. The change trims slashes off both ends of the issuer path once, uses that to pick between the root and path-insertion branches, and reuses it for all three URLs. An issuer without a terminating slash produces exactly the same list as before. `TestAuthorizationServerMetadataURLs` pins the URL list for all four issuer shapes. `TestGetAuthServerMetadataTrailingSlashIssuer` drives the whole lookup against the existing fake authorization server, serving metadata at the root well-known path. On unmodified main the first fails on both trailing-slash cases and the second fails with `GetAuthServerMetadata() error = Get "/.well-known/openid-configuration": redirect into loopback address "127.0.0.1"`, because `//.well-known/openid-configuration` makes `net/http.ServeMux` issue a canonicalizing redirect that the discovery client's `CheckRedirect` refuses. Both pass with the change. Verification on go1.26.4: `gofmt -l .` clean, `go vet ./...` clean, `staticcheck ./...` clean at v0.6.1 (the version the Test workflow pins), `go test ./auth/...` ok, `go test ./...` ok. I did not file an issue first. The fix is self-contained, confined to one unexported helper, and every issuer shape other than the slash-terminated ones is byte-identical to before. Co-authored-by: Guglielmo Colombo <guglielmoc@google.com>
169 lines
4.7 KiB
Go
169 lines
4.7 KiB
Go
// Copyright 2026 The Go MCP SDK Authors. All rights reserved.
|
|
// Use of this source code is governed by an MIT-style
|
|
// license that can be found in the LICENSE file.
|
|
|
|
package auth
|
|
|
|
import (
|
|
"net/http"
|
|
"slices"
|
|
"testing"
|
|
|
|
"github.com/modelcontextprotocol/go-sdk/internal/oauthtest"
|
|
)
|
|
|
|
func TestAuthorizationServerMetadataURLs(t *testing.T) {
|
|
tests := []struct {
|
|
issuerURL string
|
|
want []string
|
|
}{
|
|
{
|
|
issuerURL: "https://auth.example.com",
|
|
want: []string{
|
|
"https://auth.example.com/.well-known/oauth-authorization-server",
|
|
"https://auth.example.com/.well-known/openid-configuration",
|
|
},
|
|
},
|
|
{
|
|
issuerURL: "https://auth.example.com/",
|
|
want: []string{
|
|
"https://auth.example.com/.well-known/oauth-authorization-server",
|
|
"https://auth.example.com/.well-known/openid-configuration",
|
|
},
|
|
},
|
|
{
|
|
issuerURL: "https://auth.example.com/tenant1",
|
|
want: []string{
|
|
"https://auth.example.com/.well-known/oauth-authorization-server/tenant1",
|
|
"https://auth.example.com/.well-known/openid-configuration/tenant1",
|
|
"https://auth.example.com/tenant1/.well-known/openid-configuration",
|
|
},
|
|
},
|
|
{
|
|
issuerURL: "https://auth.example.com/tenant1/",
|
|
want: []string{
|
|
"https://auth.example.com/.well-known/oauth-authorization-server/tenant1",
|
|
"https://auth.example.com/.well-known/openid-configuration/tenant1",
|
|
"https://auth.example.com/tenant1/.well-known/openid-configuration",
|
|
},
|
|
},
|
|
}
|
|
|
|
for _, tt := range tests {
|
|
t.Run(tt.issuerURL, func(t *testing.T) {
|
|
if got := authorizationServerMetadataURLs(tt.issuerURL); !slices.Equal(got, tt.want) {
|
|
t.Errorf("authorizationServerMetadataURLs(%q) =\n%v\nwant\n%v", tt.issuerURL, got, tt.want)
|
|
}
|
|
})
|
|
}
|
|
}
|
|
|
|
// TestGetAuthServerMetadataTrailingSlashIssuer checks that an issuer identifier
|
|
// written with a terminating slash still finds metadata served at the root
|
|
// well-known path.
|
|
func TestGetAuthServerMetadataTrailingSlashIssuer(t *testing.T) {
|
|
s := oauthtest.NewFakeAuthorizationServer(oauthtest.Config{
|
|
MetadataEndpointConfig: &oauthtest.MetadataEndpointConfig{
|
|
ServeOAuthInsertedEndpoint: true,
|
|
},
|
|
})
|
|
s.Start(t)
|
|
|
|
got, err := GetAuthServerMetadata(t.Context(), s.URL()+"/", http.DefaultClient)
|
|
if err != nil {
|
|
t.Fatalf("GetAuthServerMetadata() error = %v, want nil", err)
|
|
}
|
|
if got == nil {
|
|
t.Fatal("GetAuthServerMetadata() got nil, want metadata")
|
|
}
|
|
if got.Issuer != s.URL() {
|
|
t.Errorf("GetAuthServerMetadata() issuer = %q, want %q", got.Issuer, s.URL())
|
|
}
|
|
}
|
|
|
|
func TestGetAuthServerMetadata(t *testing.T) {
|
|
tests := []struct {
|
|
name string
|
|
issuerPath string
|
|
endpointConfig *oauthtest.MetadataEndpointConfig
|
|
wantNil bool
|
|
}{
|
|
{
|
|
name: "OAuthEndpoint_Root",
|
|
issuerPath: "",
|
|
endpointConfig: &oauthtest.MetadataEndpointConfig{
|
|
ServeOAuthInsertedEndpoint: true,
|
|
},
|
|
},
|
|
{
|
|
name: "OpenIDEndpoint_Root",
|
|
issuerPath: "",
|
|
endpointConfig: &oauthtest.MetadataEndpointConfig{
|
|
ServeOpenIDInsertedEndpoint: true,
|
|
},
|
|
},
|
|
{
|
|
name: "OAuthEndpoint_Path",
|
|
issuerPath: "/oauth",
|
|
endpointConfig: &oauthtest.MetadataEndpointConfig{
|
|
ServeOAuthInsertedEndpoint: true,
|
|
},
|
|
},
|
|
{
|
|
name: "OpenIDEndpoint_Path",
|
|
issuerPath: "/openid",
|
|
endpointConfig: &oauthtest.MetadataEndpointConfig{
|
|
ServeOpenIDInsertedEndpoint: true,
|
|
},
|
|
},
|
|
{
|
|
name: "OpenIDAppendedEndpoint_Path",
|
|
issuerPath: "/openid",
|
|
endpointConfig: &oauthtest.MetadataEndpointConfig{
|
|
ServeOpenIDAppendedEndpoint: true,
|
|
},
|
|
},
|
|
{
|
|
name: "NoMetadata",
|
|
issuerPath: "",
|
|
endpointConfig: &oauthtest.MetadataEndpointConfig{
|
|
// All metadata endpoints disabled.
|
|
ServeOAuthInsertedEndpoint: false,
|
|
ServeOpenIDInsertedEndpoint: false,
|
|
},
|
|
wantNil: true,
|
|
},
|
|
}
|
|
|
|
for _, tt := range tests {
|
|
t.Run(tt.name, func(t *testing.T) {
|
|
s := oauthtest.NewFakeAuthorizationServer(oauthtest.Config{
|
|
IssuerPath: tt.issuerPath,
|
|
MetadataEndpointConfig: tt.endpointConfig,
|
|
})
|
|
s.Start(t)
|
|
issuerURL := s.URL() + tt.issuerPath
|
|
|
|
got, err := GetAuthServerMetadata(t.Context(), issuerURL, http.DefaultClient)
|
|
if tt.wantNil {
|
|
// When no metadata is found, GetAuthServerMetadata returns (nil, nil).
|
|
if err != nil {
|
|
t.Fatalf("GetAuthServerMetadata() unexpected error = %v, want nil", err)
|
|
}
|
|
if got != nil {
|
|
t.Fatal("GetAuthServerMetadata() expected nil for no metadata, got metadata")
|
|
}
|
|
return
|
|
}
|
|
if err != nil {
|
|
t.Fatalf("GetAuthServerMetadata() error = %v, want nil", err)
|
|
}
|
|
if got == nil {
|
|
t.Fatal("GetAuthServerMetadata() got nil, want metadata")
|
|
}
|
|
if got.Issuer != issuerURL {
|
|
t.Errorf("GetAuthServerMetadata() issuer = %q, want %q", got.Issuer, issuerURL)
|
|
}
|
|
})
|
|
}
|
|
}
|