replace custom protected resource metadata handler with our own

Author: mattdholloway

pkg/http/oauth/oauth.go

@@ -3,17 +3,16 @@
 package oauth
 
 import (
-	"bytes"
-	_ "embed"
 	"fmt"
-	"html"
 	"net/http"
 	"net/url"
 	"strings"
-	"text/template"
+
+	"github.com/modelcontextprotocol/go-sdk/auth"
 
 	"github.com/github/github-mcp-server/pkg/http/headers"
 	"github.com/go-chi/chi/v5"
+	"github.com/modelcontextprotocol/go-sdk/oauthex"
 )
 
 const (
@@ -24,9 +23,6 @@ const (
 	DefaultAuthorizationServer = "https://github.com/login/oauth"
 )
 
-//go:embed protected_resource.json.tmpl
-var protectedResourceTemplate []byte
-
 // SupportedScopes lists all OAuth scopes that may be required by MCP tools.
 var SupportedScopes = []string{
 	"repo",
@@ -66,8 +62,7 @@ type ProtectedResourceData struct {
 
 // AuthHandler handles OAuth-related HTTP endpoints.
 type AuthHandler struct {
-	cfg                       *Config
-	protectedResourceTemplate *template.Template
+	cfg *Config
 }
 
 // NewAuthHandler creates a new OAuth auth handler.
@@ -81,21 +76,16 @@ func NewAuthHandler(cfg *Config) (*AuthHandler, error) {
 		cfg.AuthorizationServer = DefaultAuthorizationServer
 	}
 
-	tmpl, err := template.New("protected-resource").Parse(string(protectedResourceTemplate))
-	if err != nil {
-		return nil, fmt.Errorf("failed to parse protected resource template: %w", err)
-	}
-
 	return &AuthHandler{
-		cfg:                       cfg,
-		protectedResourceTemplate: tmpl,
+		cfg: cfg,
 	}, nil
 }
 
 // routePatterns defines the route patterns for OAuth protected resource metadata.
 var routePatterns = []string{
 	"",          // Root: /.well-known/oauth-protected-resource
 	"/readonly", // Read-only mode
+	"/insiders", // Insiders mode
 	"/x/{toolset}",
 	"/x/{toolset}/readonly",
 }
@@ -105,12 +95,30 @@ func (h *AuthHandler) RegisterRoutes(r chi.Router) {
 	for _, pattern := range routePatterns {
 		for _, route := range h.routesForPattern(pattern) {
 			path := OAuthProtectedResourcePrefix + route
-			r.Get(path, h.handleProtectedResource)
-			r.Options(path, h.handleProtectedResource) // CORS support
+
+			// Build metadata for this specific resource path
+			metadata := h.buildMetadata(route)
+			r.Handle(path, auth.ProtectedResourceMetadataHandler(metadata))
 		}
 	}
 }
 
+func (h *AuthHandler) buildMetadata(resourcePath string) *oauthex.ProtectedResourceMetadata {
+	baseURL := strings.TrimSuffix(h.cfg.BaseURL, "/")
+	resourceURL := baseURL
+	if resourcePath != "" && resourcePath != "/" {
+		resourceURL = baseURL + resourcePath
+	}
+
+	return &oauthex.ProtectedResourceMetadata{
+		Resource:               resourceURL,
+		AuthorizationServers:   []string{h.cfg.AuthorizationServer},
+		ResourceName:           "GitHub MCP Server",
+		ScopesSupported:        SupportedScopes,
+		BearerMethodsSupported: []string{"header"},
+	}
+}
+
 // routesForPattern generates route variants for a given pattern.
 // GitHub strips the /mcp prefix before forwarding, so we register both variants:
 // - With /mcp prefix: for direct access or when GitHub doesn't strip
@@ -124,37 +132,6 @@ func (h *AuthHandler) routesForPattern(pattern string) []string {
 	}
 }
 
-// handleProtectedResource handles requests for OAuth protected resource metadata.
-func (h *AuthHandler) handleProtectedResource(w http.ResponseWriter, r *http.Request) {
-	// Extract the resource path from the URL
-	resourcePath := strings.TrimPrefix(r.URL.Path, OAuthProtectedResourcePrefix)
-	if resourcePath == "" || resourcePath == "/" {
-		resourcePath = "/"
-	} else {
-		resourcePath = strings.TrimPrefix(resourcePath, "/")
-	}
-
-	data, err := h.GetProtectedResourceData(r, html.EscapeString(resourcePath))
-	if err != nil {
-		http.Error(w, err.Error(), http.StatusBadRequest)
-		return
-	}
-
-	var buf bytes.Buffer
-	if err := h.protectedResourceTemplate.Execute(&buf, data); err != nil {
-		http.Error(w, "Internal server error", http.StatusInternalServerError)
-		return
-	}
-
-	// Set CORS headers
-	w.Header().Set("Access-Control-Allow-Origin", "*")
-	w.Header().Set("Access-Control-Allow-Methods", "GET, OPTIONS")
-	w.Header().Set("Access-Control-Allow-Headers", "Authorization, Content-Type")
-	w.Header().Set("Content-Type", "application/json")
-	w.WriteHeader(http.StatusOK)
-	_, _ = w.Write(buf.Bytes())
-}
-
 // GetEffectiveResourcePath returns the resource path for OAuth protected resource URLs.
 // It checks for the X-GitHub-Original-Path header set by GitHub, which contains
 // the exact path the client requested before the /mcp prefix was stripped.

pkg/http/oauth/oauth_test.go

@@ -62,7 +62,6 @@ func TestNewAuthHandler(t *testing.T) {
 			require.NotNil(t, handler)
 
 			assert.Equal(t, tc.expectedAuthServer, handler.cfg.AuthorizationServer)
-			assert.NotNil(t, handler.protectedResourceTemplate)
 		})
 	}
 }
@@ -444,8 +443,10 @@ func TestHandleProtectedResource(t *testing.T) {
 		validateResponse   func(t *testing.T, body map[string]any)
 	}{
 		{
-			name:               "GET request returns protected resource metadata",
-			cfg:                &Config{},
+			name: "GET request returns protected resource metadata",
+			cfg: &Config{
+				BaseURL: "https://api.example.com",
+			},
 			path:               OAuthProtectedResourcePrefix,
 			host:               "api.example.com",
 			method:             http.MethodGet,
@@ -454,7 +455,7 @@ func TestHandleProtectedResource(t *testing.T) {
 			validateResponse: func(t *testing.T, body map[string]any) {
 				t.Helper()
 				assert.Equal(t, "GitHub MCP Server", body["resource_name"])
-				assert.Contains(t, body["resource"], "api.example.com")
+				assert.Equal(t, "https://api.example.com", body["resource"])
 
 				authServers, ok := body["authorization_servers"].([]any)
 				require.True(t, ok)
@@ -463,40 +464,47 @@ func TestHandleProtectedResource(t *testing.T) {
 			},
 		},
 		{
-			name:               "OPTIONS request for CORS",
-			cfg:                &Config{},
+			name: "OPTIONS request for CORS preflight",
+			cfg: &Config{
+				BaseURL: "https://api.example.com",
+			},
 			path:               OAuthProtectedResourcePrefix,
 			host:               "api.example.com",
 			method:             http.MethodOptions,
-			expectedStatusCode: http.StatusOK,
+			expectedStatusCode: http.StatusNoContent,
 		},
 		{
-			name:               "path with /mcp suffix",
-			cfg:                &Config{},
+			name: "path with /mcp suffix",
+			cfg: &Config{
+				BaseURL: "https://api.example.com",
+			},
 			path:               OAuthProtectedResourcePrefix + "/mcp",
 			host:               "api.example.com",
 			method:             http.MethodGet,
 			expectedStatusCode: http.StatusOK,
 			validateResponse: func(t *testing.T, body map[string]any) {
 				t.Helper()
-				assert.Contains(t, body["resource"], "/mcp")
+				assert.Equal(t, "https://api.example.com/mcp", body["resource"])
 			},
 		},
 		{
-			name:               "path with /readonly suffix",
-			cfg:                &Config{},
+			name: "path with /readonly suffix",
+			cfg: &Config{
+				BaseURL: "https://api.example.com",
+			},
 			path:               OAuthProtectedResourcePrefix + "/readonly",
 			host:               "api.example.com",
 			method:             http.MethodGet,
 			expectedStatusCode: http.StatusOK,
 			validateResponse: func(t *testing.T, body map[string]any) {
 				t.Helper()
-				assert.Contains(t, body["resource"], "/readonly")
+				assert.Equal(t, "https://api.example.com/readonly", body["resource"])
 			},
 		},
 		{
 			name: "custom authorization server in response",
 			cfg: &Config{
+				BaseURL:             "https://api.example.com",
 				AuthorizationServer: "https://custom.auth.example.com/oauth",
 			},
 			path:               OAuthProtectedResourcePrefix,
@@ -559,7 +567,9 @@ func TestHandleProtectedResource(t *testing.T) {
 func TestRegisterRoutes(t *testing.T) {
 	t.Parallel()
 
-	handler, err := NewAuthHandler(&Config{})
+	handler, err := NewAuthHandler(&Config{
+		BaseURL: "https://api.example.com",
+	})
 	require.NoError(t, err)
 
 	router := chi.NewRouter()
@@ -588,12 +598,12 @@ func TestRegisterRoutes(t *testing.T) {
 			router.ServeHTTP(rec, req)
 			assert.Equal(t, http.StatusOK, rec.Code, "GET %s should return 200", route)
 
-			// Test OPTIONS (CORS)
+			// Test OPTIONS (CORS preflight)
 			req = httptest.NewRequest(http.MethodOptions, route, nil)
 			req.Host = "api.example.com"
 			rec = httptest.NewRecorder()
 			router.ServeHTTP(rec, req)
-			assert.Equal(t, http.StatusOK, rec.Code, "OPTIONS %s should return 200", route)
+			assert.Equal(t, http.StatusNoContent, rec.Code, "OPTIONS %s should return 204", route)
 		})
 	}
 }
@@ -623,7 +633,9 @@ func TestSupportedScopes(t *testing.T) {
 func TestProtectedResourceResponseFormat(t *testing.T) {
 	t.Parallel()
 
-	handler, err := NewAuthHandler(&Config{})
+	handler, err := NewAuthHandler(&Config{
+		BaseURL: "https://api.example.com",
+	})
 	require.NoError(t, err)
 
 	router := chi.NewRouter()