From 40f7fb4cadc075047dca388dc42ca3854b3d139e Mon Sep 17 00:00:00 2001 From: Ryo Kusnadi Date: Wed, 8 Oct 2025 20:38:31 +0700 Subject: [PATCH 1/8] Rebase Using Main Branch, Move RequirePKCE To Auth Meta --- internal/testing/fake_auth_server.go | 18 +++++++++++++- oauthex/auth_meta.go | 16 ++++++++++++ oauthex/auth_meta_test.go | 37 ++++++++++++++++++++++++++++ 3 files changed, 70 insertions(+), 1 deletion(-) diff --git a/internal/testing/fake_auth_server.go b/internal/testing/fake_auth_server.go index 79fafe4e2..bd12e31c7 100644 --- a/internal/testing/fake_auth_server.go +++ b/internal/testing/fake_auth_server.go @@ -9,6 +9,7 @@ import ( "encoding/base64" "encoding/json" "fmt" + "net" "net/http" "time" @@ -44,7 +45,22 @@ func NewFakeAuthMux() *http.ServeMux { } func (s *state) handleMetadata(w http.ResponseWriter, r *http.Request) { - issuer := "https://localhost:" + r.URL.Port() + // Derive the port from the request Host; r.URL.Port() is empty on server side. + hostPort := r.Host + port := "" + + if hp := hostPort; hp != "" { + if _, p, err := net.SplitHostPort(hp); err == nil { + port = p + } + } + + if port == "" { + // Fallback attempt; may still be empty depending on server/request. + port = r.URL.Port() + } + + issuer := "https://localhost:" + port metadata := map[string]any{ "issuer": issuer, "authorization_endpoint": issuer + "/authorize", diff --git a/oauthex/auth_meta.go b/oauthex/auth_meta.go index 73b8fb101..2adf6aa63 100644 --- a/oauthex/auth_meta.go +++ b/oauthex/auth_meta.go @@ -145,3 +145,19 @@ func GetAuthServerMeta(ctx context.Context, issuerURL string, c *http.Client) (* } return nil, fmt.Errorf("failed to get auth server metadata from %q: %w", issuerURL, errors.Join(errs...)) } + +// RequirePKCE checks that the authorization server for issuerURL supports PKCE, +// by verifying that CodeChallengeMethodsSupported is non-empty. +// It returns an error if metadata cannot be fetched or PKCE is not advertised. +func RequirePKCE(ctx context.Context, issuerURL string, c *http.Client) error { + asm, err := GetAuthServerMeta(ctx, issuerURL, c) + if err != nil { + return err + } + + if len(asm.CodeChallengeMethodsSupported) == 0 { + return fmt.Errorf("authorization server at %s does not implement PKCE", issuerURL) + } + + return nil +} diff --git a/oauthex/auth_meta_test.go b/oauthex/auth_meta_test.go index bee8f900d..3945bd5cb 100644 --- a/oauthex/auth_meta_test.go +++ b/oauthex/auth_meta_test.go @@ -7,10 +7,16 @@ package oauthex import ( + "context" "encoding/json" + "net/http" + "net/http/httptest" + "net/url" "os" "path/filepath" "testing" + + itesting "github.com/modelcontextprotocol/go-sdk/internal/testing" ) func TestAuthMetaParse(t *testing.T) { @@ -28,3 +34,34 @@ func TestAuthMetaParse(t *testing.T) { t.Errorf("got %q, want %q", g, w) } } + +func TestRequirePKCE(t *testing.T) { + ctx := context.Background() + + // Start a fake OAuth 2.1 auth server that advertises PKCE (S256). + orig := itesting.NewFakeAuthMux() + wrapper := http.NewServeMux() + wrapper.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { + orig.ServeHTTP(w, r) + }) + ts := httptest.NewTLSServer(wrapper) + defer ts.Close() + + // Validate that the server supports PKCE per MCP auth requirements. + // The fake server sets issuer to https://localhost:, so compute that issuer. + u, _ := url.Parse(ts.URL) + issuer := "https://localhost:" + u.Port() + + // The fake server presents a cert for example.com; set ServerName accordingly. + httpClient := ts.Client() + if tr, ok := httpClient.Transport.(*http.Transport); ok { + clone := tr.Clone() + clone.TLSClientConfig.ServerName = "example.com" + httpClient.Transport = clone + } + + if err := RequirePKCE(ctx, issuer, httpClient); err != nil { + t.Fatal(err) + } + +} From a58b8c98bda815ff7917d19d5c6ce07cfb4cd243 Mon Sep 17 00:00:00 2001 From: Ryo Kusnadi Date: Fri, 10 Oct 2025 09:10:26 +0700 Subject: [PATCH 2/8] Move Require PCKE Check To GetAuthServerMeta --- oauthex/auth_meta.go | 21 +++++---------------- oauthex/auth_meta_test.go | 4 ++-- 2 files changed, 7 insertions(+), 18 deletions(-) diff --git a/oauthex/auth_meta.go b/oauthex/auth_meta.go index 2adf6aa63..3723318f2 100644 --- a/oauthex/auth_meta.go +++ b/oauthex/auth_meta.go @@ -139,25 +139,14 @@ func GetAuthServerMeta(ctx context.Context, issuerURL string, c *http.Client) (* // Security violation; don't keep trying. return nil, fmt.Errorf("metadata issuer %q does not match issuer URL %q", asm.Issuer, issuerURL) } + + if len(asm.CodeChallengeMethodsSupported) == 0 { + return nil, fmt.Errorf("authorization server at %s does not implement PKCE", issuerURL) + } + return asm, nil } errs = append(errs, err) } return nil, fmt.Errorf("failed to get auth server metadata from %q: %w", issuerURL, errors.Join(errs...)) } - -// RequirePKCE checks that the authorization server for issuerURL supports PKCE, -// by verifying that CodeChallengeMethodsSupported is non-empty. -// It returns an error if metadata cannot be fetched or PKCE is not advertised. -func RequirePKCE(ctx context.Context, issuerURL string, c *http.Client) error { - asm, err := GetAuthServerMeta(ctx, issuerURL, c) - if err != nil { - return err - } - - if len(asm.CodeChallengeMethodsSupported) == 0 { - return fmt.Errorf("authorization server at %s does not implement PKCE", issuerURL) - } - - return nil -} diff --git a/oauthex/auth_meta_test.go b/oauthex/auth_meta_test.go index 3945bd5cb..fbfeaed77 100644 --- a/oauthex/auth_meta_test.go +++ b/oauthex/auth_meta_test.go @@ -35,7 +35,7 @@ func TestAuthMetaParse(t *testing.T) { } } -func TestRequirePKCE(t *testing.T) { +func TestGetAuthServerMetaRequirePKCE(t *testing.T) { ctx := context.Background() // Start a fake OAuth 2.1 auth server that advertises PKCE (S256). @@ -60,7 +60,7 @@ func TestRequirePKCE(t *testing.T) { httpClient.Transport = clone } - if err := RequirePKCE(ctx, issuer, httpClient); err != nil { + if _, err := GetAuthServerMeta(ctx, issuer, httpClient); err != nil { t.Fatal(err) } From af3d71673493e8c55c9140726f3fc3252bf71190 Mon Sep 17 00:00:00 2001 From: Ryo Kusnadi Date: Fri, 10 Oct 2025 09:19:18 +0700 Subject: [PATCH 3/8] Readd Notes --- oauthex/auth_meta.go | 3 +++ oauthex/dcr.go | 3 +++ oauthex/oauth2.go | 2 +- oauthex/oauthex.go | 2 +- 4 files changed, 8 insertions(+), 2 deletions(-) diff --git a/oauthex/auth_meta.go b/oauthex/auth_meta.go index 4b643c328..3723318f2 100644 --- a/oauthex/auth_meta.go +++ b/oauthex/auth_meta.go @@ -2,6 +2,9 @@ // Use of this source code is governed by an MIT-style // license that can be found in the LICENSE file. +// This file implements Authorization Server Metadata. +// See https://www.rfc-editor.org/rfc/rfc8414.html. + //go:build mcp_go_client_oauth package oauthex diff --git a/oauthex/dcr.go b/oauthex/dcr.go index 575ee673d..75ce29616 100644 --- a/oauthex/dcr.go +++ b/oauthex/dcr.go @@ -2,6 +2,9 @@ // Use of this source code is governed by an MIT-style // license that can be found in the LICENSE file. +// This file implements Authorization Server Metadata. +// See https://www.rfc-editor.org/rfc/rfc8414.html. + //go:build mcp_go_client_oauth package oauthex diff --git a/oauthex/oauth2.go b/oauthex/oauth2.go index 0032e2455..92279acd3 100644 --- a/oauthex/oauth2.go +++ b/oauthex/oauth2.go @@ -2,7 +2,7 @@ // Use of this source code is governed by an MIT-style // license that can be found in the LICENSE file. -//go:build mcp_go_client_oauth +// Package oauthex implements extensions to OAuth2. package oauthex diff --git a/oauthex/oauthex.go b/oauthex/oauthex.go index 6a45e0001..8ffc632f9 100644 --- a/oauthex/oauthex.go +++ b/oauthex/oauthex.go @@ -2,7 +2,7 @@ // Use of this source code is governed by an MIT-style // license that can be found in the LICENSE file. -//go:build mcp_go_client_oauth +// Package oauthex implements extensions to OAuth2. package oauthex From 11b753ae9c75a8dacf239c4614d4fe7f76354fb1 Mon Sep 17 00:00:00 2001 From: Ryo Kusnadi Date: Fri, 10 Oct 2025 09:20:36 +0700 Subject: [PATCH 4/8] Readd Notes --- oauthex/oauth2.go | 2 ++ oauthex/oauthex.go | 1 - 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/oauthex/oauth2.go b/oauthex/oauth2.go index 92279acd3..f876be030 100644 --- a/oauthex/oauth2.go +++ b/oauthex/oauth2.go @@ -4,6 +4,8 @@ // Package oauthex implements extensions to OAuth2. +//go:build mcp_go_client_oauth + package oauthex import ( diff --git a/oauthex/oauthex.go b/oauthex/oauthex.go index 8ffc632f9..34ed55b59 100644 --- a/oauthex/oauthex.go +++ b/oauthex/oauthex.go @@ -3,7 +3,6 @@ // license that can be found in the LICENSE file. // Package oauthex implements extensions to OAuth2. - package oauthex // ProtectedResourceMetadata is the metadata for an OAuth 2.0 protected resource, From 76a7d1da3f5853af446f9b726969cbb1f85fa80c Mon Sep 17 00:00:00 2001 From: Ryo Kusnadi Date: Fri, 10 Oct 2025 23:13:05 +0700 Subject: [PATCH 5/8] Modify GetAuthServerMeta For Validate Not Implement PKCE --- oauthex/auth_meta_test.go | 38 +++++++++++++++++++++++++++++++------- 1 file changed, 31 insertions(+), 7 deletions(-) diff --git a/oauthex/auth_meta_test.go b/oauthex/auth_meta_test.go index d9418221a..b0feb6e1a 100644 --- a/oauthex/auth_meta_test.go +++ b/oauthex/auth_meta_test.go @@ -14,6 +14,7 @@ import ( "net/url" "os" "path/filepath" + "strings" "testing" ) @@ -33,18 +34,34 @@ func TestAuthMetaParse(t *testing.T) { } } -func TestGetAuthServerMetaRequirePKCE(t *testing.T) { +func TestGetAuthServerMetaNotImplementPKCE(t *testing.T) { ctx := context.Background() - // Start a fake OAuth 2.1 auth server that advertises PKCE (S256). + // Start a fake OAuth 2.1 auth server that does NOT advertise PKCE support. wrapper := http.NewServeMux() - wrapper.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { - NewFakeMCPServerMux().ServeHTTP(w, r) + wrapper.HandleFunc("/.well-known/oauth-authorization-server", func(w http.ResponseWriter, r *http.Request) { + // Create metadata without PKCE support + u, _ := url.Parse("https://" + r.Host) + issuer := "https://localhost:" + u.Port() + metadata := AuthServerMeta{ + Issuer: issuer, + AuthorizationEndpoint: issuer + "/authorize", + TokenEndpoint: issuer + "/token", + RegistrationEndpoint: issuer + "/register", + JWKSURI: issuer + "/.well-known/jwks.json", + ScopesSupported: []string{"openid", "profile", "email"}, + ResponseTypesSupported: []string{"code"}, + GrantTypesSupported: []string{"authorization_code"}, + TokenEndpointAuthMethodsSupported: []string{"none"}, + // CodeChallengeMethodsSupported is intentionally omitted/empty to test PKCE requirement + } + + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode(metadata) }) ts := httptest.NewTLSServer(wrapper) defer ts.Close() - // Validate that the server supports PKCE per MCP auth requirements. // The fake server sets issuer to https://localhost:, so compute that issuer. u, _ := url.Parse(ts.URL) issuer := "https://localhost:" + u.Port() @@ -57,8 +74,15 @@ func TestGetAuthServerMetaRequirePKCE(t *testing.T) { httpClient.Transport = clone } - if _, err := GetAuthServerMeta(ctx, issuer, httpClient); err != nil { - t.Fatal(err) + // This should fail because the server doesn't support PKCE + _, err := GetAuthServerMeta(ctx, issuer, httpClient) + if err == nil { + t.Fatal("expected error when auth server doesn't support PKCE, but got none") } + // Verify the error message mentions PKCE + expectedError := "does not implement PKCE" + if !strings.Contains(err.Error(), expectedError) { + t.Errorf("expected error to contain %q, but got: %v", expectedError, err) + } } From e931ec5d4d31c04fe79d45ecff8c81035bc220ea Mon Sep 17 00:00:00 2001 From: Ryo Kusnadi Date: Fri, 10 Oct 2025 23:15:44 +0700 Subject: [PATCH 6/8] Modify GetAuthServerMeta For Validate Not Implement PKCE --- oauthex/auth_meta_test.go | 125 +++++++++++++++++++++++++------------- 1 file changed, 82 insertions(+), 43 deletions(-) diff --git a/oauthex/auth_meta_test.go b/oauthex/auth_meta_test.go index b0feb6e1a..5f0f216e1 100644 --- a/oauthex/auth_meta_test.go +++ b/oauthex/auth_meta_test.go @@ -34,55 +34,94 @@ func TestAuthMetaParse(t *testing.T) { } } -func TestGetAuthServerMetaNotImplementPKCE(t *testing.T) { +func TestGetAuthServerMetaPKCESupport(t *testing.T) { ctx := context.Background() - // Start a fake OAuth 2.1 auth server that does NOT advertise PKCE support. - wrapper := http.NewServeMux() - wrapper.HandleFunc("/.well-known/oauth-authorization-server", func(w http.ResponseWriter, r *http.Request) { - // Create metadata without PKCE support - u, _ := url.Parse("https://" + r.Host) - issuer := "https://localhost:" + u.Port() - metadata := AuthServerMeta{ - Issuer: issuer, - AuthorizationEndpoint: issuer + "/authorize", - TokenEndpoint: issuer + "/token", - RegistrationEndpoint: issuer + "/register", - JWKSURI: issuer + "/.well-known/jwks.json", - ScopesSupported: []string{"openid", "profile", "email"}, - ResponseTypesSupported: []string{"code"}, - GrantTypesSupported: []string{"authorization_code"}, - TokenEndpointAuthMethodsSupported: []string{"none"}, - // CodeChallengeMethodsSupported is intentionally omitted/empty to test PKCE requirement - } + tests := []struct { + name string + hasPKCESupport bool + expectError bool + expectedError string + }{ + { + name: "server_with_pkce_support", + hasPKCESupport: true, + expectError: false, + }, + { + name: "server_without_pkce_support", + hasPKCESupport: false, + expectError: true, + expectedError: "does not implement PKCE", + }, + } - w.Header().Set("Content-Type", "application/json") - json.NewEncoder(w).Encode(metadata) - }) - ts := httptest.NewTLSServer(wrapper) - defer ts.Close() + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Start a fake OAuth 2.1 auth server + wrapper := http.NewServeMux() + wrapper.HandleFunc("/.well-known/oauth-authorization-server", func(w http.ResponseWriter, r *http.Request) { + // Create metadata + u, _ := url.Parse("https://" + r.Host) + issuer := "https://localhost:" + u.Port() + metadata := AuthServerMeta{ + Issuer: issuer, + AuthorizationEndpoint: issuer + "/authorize", + TokenEndpoint: issuer + "/token", + RegistrationEndpoint: issuer + "/register", + JWKSURI: issuer + "/.well-known/jwks.json", + ScopesSupported: []string{"openid", "profile", "email"}, + ResponseTypesSupported: []string{"code"}, + GrantTypesSupported: []string{"authorization_code"}, + TokenEndpointAuthMethodsSupported: []string{"none"}, + } - // The fake server sets issuer to https://localhost:, so compute that issuer. - u, _ := url.Parse(ts.URL) - issuer := "https://localhost:" + u.Port() + // Add PKCE support based on test case + if tt.hasPKCESupport { + metadata.CodeChallengeMethodsSupported = []string{"S256"} + } + // If hasPKCESupport is false, CodeChallengeMethodsSupported remains empty - // The fake server presents a cert for example.com; set ServerName accordingly. - httpClient := ts.Client() - if tr, ok := httpClient.Transport.(*http.Transport); ok { - clone := tr.Clone() - clone.TLSClientConfig.ServerName = "example.com" - httpClient.Transport = clone - } + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode(metadata) + }) + ts := httptest.NewTLSServer(wrapper) + defer ts.Close() - // This should fail because the server doesn't support PKCE - _, err := GetAuthServerMeta(ctx, issuer, httpClient) - if err == nil { - t.Fatal("expected error when auth server doesn't support PKCE, but got none") - } + // The fake server sets issuer to https://localhost:, so compute that issuer. + u, _ := url.Parse(ts.URL) + issuer := "https://localhost:" + u.Port() + + // The fake server presents a cert for example.com; set ServerName accordingly. + httpClient := ts.Client() + if tr, ok := httpClient.Transport.(*http.Transport); ok { + clone := tr.Clone() + clone.TLSClientConfig.ServerName = "example.com" + httpClient.Transport = clone + } + + // Test the GetAuthServerMeta function + meta, err := GetAuthServerMeta(ctx, issuer, httpClient) - // Verify the error message mentions PKCE - expectedError := "does not implement PKCE" - if !strings.Contains(err.Error(), expectedError) { - t.Errorf("expected error to contain %q, but got: %v", expectedError, err) + if tt.expectError { + if err == nil { + t.Fatal("expected error but got none") + } + if !strings.Contains(err.Error(), tt.expectedError) { + t.Errorf("expected error to contain %q, but got: %v", tt.expectedError, err) + } + } else { + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if meta == nil { + t.Fatal("expected metadata but got nil") + } + // Verify PKCE support is present + if len(meta.CodeChallengeMethodsSupported) == 0 { + t.Error("expected PKCE support but CodeChallengeMethodsSupported is empty") + } + } + }) } } From 9f75046ff8086dfb037d415e4e6f2d65b92fbc93 Mon Sep 17 00:00:00 2001 From: Ryo Kusnadi Date: Fri, 10 Oct 2025 23:16:34 +0700 Subject: [PATCH 7/8] Remove not used comment for TestGetAuthServerMetaPKCESupport --- oauthex/auth_meta_test.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/oauthex/auth_meta_test.go b/oauthex/auth_meta_test.go index 5f0f216e1..cf4064ed6 100644 --- a/oauthex/auth_meta_test.go +++ b/oauthex/auth_meta_test.go @@ -36,7 +36,6 @@ func TestAuthMetaParse(t *testing.T) { func TestGetAuthServerMetaPKCESupport(t *testing.T) { ctx := context.Background() - tests := []struct { name string hasPKCESupport bool @@ -61,7 +60,6 @@ func TestGetAuthServerMetaPKCESupport(t *testing.T) { // Start a fake OAuth 2.1 auth server wrapper := http.NewServeMux() wrapper.HandleFunc("/.well-known/oauth-authorization-server", func(w http.ResponseWriter, r *http.Request) { - // Create metadata u, _ := url.Parse("https://" + r.Host) issuer := "https://localhost:" + u.Port() metadata := AuthServerMeta{ @@ -100,9 +98,7 @@ func TestGetAuthServerMetaPKCESupport(t *testing.T) { httpClient.Transport = clone } - // Test the GetAuthServerMeta function meta, err := GetAuthServerMeta(ctx, issuer, httpClient) - if tt.expectError { if err == nil { t.Fatal("expected error but got none") From 8e36ba39e93eb00e738bccd5f5be028a14f4db10 Mon Sep 17 00:00:00 2001 From: Ryo Kusnadi Date: Tue, 14 Oct 2025 20:39:07 +0700 Subject: [PATCH 8/8] Update Unit Test For TestGetAuthServerMetaPKCESupport --- oauthex/auth_meta_test.go | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/oauthex/auth_meta_test.go b/oauthex/auth_meta_test.go index cf4064ed6..1e6088249 100644 --- a/oauthex/auth_meta_test.go +++ b/oauthex/auth_meta_test.go @@ -39,19 +39,16 @@ func TestGetAuthServerMetaPKCESupport(t *testing.T) { tests := []struct { name string hasPKCESupport bool - expectError bool - expectedError string + wantError string }{ { name: "server_with_pkce_support", hasPKCESupport: true, - expectError: false, }, { name: "server_without_pkce_support", hasPKCESupport: false, - expectError: true, - expectedError: "does not implement PKCE", + wantError: "does not implement PKCE", }, } @@ -99,23 +96,23 @@ func TestGetAuthServerMetaPKCESupport(t *testing.T) { } meta, err := GetAuthServerMeta(ctx, issuer, httpClient) - if tt.expectError { + if tt.wantError != "" { if err == nil { - t.Fatal("expected error but got none") + t.Fatal("wanted error but got none") } - if !strings.Contains(err.Error(), tt.expectedError) { - t.Errorf("expected error to contain %q, but got: %v", tt.expectedError, err) + if !strings.Contains(err.Error(), tt.wantError) { + t.Errorf("wanted error to contain %q, but got: %v", tt.wantError, err) } } else { if err != nil { - t.Fatalf("unexpected error: %v", err) + t.Fatalf("unwanted error: %v", err) } if meta == nil { - t.Fatal("expected metadata but got nil") + t.Fatal("wanted metadata but got nil") } // Verify PKCE support is present if len(meta.CodeChallengeMethodsSupported) == 0 { - t.Error("expected PKCE support but CodeChallengeMethodsSupported is empty") + t.Error("wanted PKCE support but CodeChallengeMethodsSupported is empty") } } })