Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions oauthex/auth_meta.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"errors"
"fmt"
"net/http"
"slices"

"github.com/modelcontextprotocol/go-sdk/internal/authutil"
)
Expand Down Expand Up @@ -131,8 +132,9 @@ type AuthServerMeta struct {
// - The metadataURL must use HTTPS or be a local address.
// - The Issuer field is checked against metadataURL.Issuer.
//
// It also verifies that the authorization server supports PKCE and that the URLs
// in the metadata don't use dangerous schemes.
// It also verifies that the authorization server supports PKCE with the S256
// code challenge method, which is the one this SDK's clients use, and that the
// URLs in the metadata don't use dangerous schemes.
//
// It returns an error if the request fails with a non-4xx status code or the fetched
// metadata doesn't pass security validations.
Expand Down Expand Up @@ -161,6 +163,9 @@ func GetAuthServerMeta(ctx context.Context, metadataURL, issuer string, c *http.
if len(asm.CodeChallengeMethodsSupported) == 0 {
return nil, fmt.Errorf("authorization server at %s does not implement PKCE", issuer)
}
if !slices.Contains(asm.CodeChallengeMethodsSupported, "S256") {
return nil, fmt.Errorf("authorization server at %s does not support the S256 PKCE method", issuer)
}

// Validate endpoint URLs to prevent XSS attacks (see #526).
if err := validateAuthServerMetaURLs(asm); err != nil {
Expand Down
17 changes: 17 additions & 0 deletions oauthex/auth_meta_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ func TestGetAuthServerMetaPKCESupport(t *testing.T) {
tests := []struct {
name string
hasPKCESupport bool
pkceMethods []string // overrides the default ["S256"] when set
wantError string
issuerWithTrailingSlash bool
}{
Expand All @@ -49,6 +50,19 @@ func TestGetAuthServerMetaPKCESupport(t *testing.T) {
hasPKCESupport: false,
wantError: "does not implement PKCE",
},
{
// The client always sends an S256 code challenge, so a server
// that offers only plain cannot complete the flow.
name: "server_with_only_plain_pkce",
hasPKCESupport: true,
pkceMethods: []string{"plain"},
wantError: "does not support the S256 PKCE method",
},
{
name: "server_with_plain_and_s256_pkce",
hasPKCESupport: true,
pkceMethods: []string{"plain", "S256"},
},
{
// ProtectedResourceMetadata may contain AuthorizationServers with a trailing slash (see Issue #953)
name: "issuer_with_trailing_slash",
Expand Down Expand Up @@ -79,6 +93,9 @@ func TestGetAuthServerMetaPKCESupport(t *testing.T) {
// Add PKCE support based on test case
if tt.hasPKCESupport {
metadata.CodeChallengeMethodsSupported = []string{"S256"}
if tt.pkceMethods != nil {
metadata.CodeChallengeMethodsSupported = tt.pkceMethods
}
}
// If hasPKCESupport is false, CodeChallengeMethodsSupported remains empty

Expand Down
Loading