implement better resource path handling for OAuth server

Author: mattdholloway

cmd/github-mcp-server/main.go

@@ -102,6 +102,7 @@ var (
 				Host:                 viper.GetString("host"),
 				Port:                 viper.GetInt("port"),
 				BaseURL:              viper.GetString("base-url"),
+				ResourcePath:         viper.GetString("resource-path"),
 				ExportTranslations:   viper.GetBool("export-translations"),
 				EnableCommandLogging: viper.GetBool("enable-command-logging"),
 				LogFilePath:          viper.GetString("log-file"),
@@ -137,6 +138,7 @@ func init() {
 	rootCmd.PersistentFlags().Duration("repo-access-cache-ttl", 5*time.Minute, "Override the repo access cache TTL (e.g. 1m, 0s to disable)")
 	rootCmd.PersistentFlags().Int("port", 8082, "HTTP server port")
 	rootCmd.PersistentFlags().String("base-url", "", "Base URL where this server is publicly accessible (for OAuth resource metadata)")
+	rootCmd.PersistentFlags().String("resource-path", "", "Externally visible base path for the HTTP server (for OAuth resource metadata)")
 
 	// Bind flag to viper
 	_ = viper.BindPFlag("toolsets", rootCmd.PersistentFlags().Lookup("toolsets"))
@@ -154,6 +156,7 @@ func init() {
 	_ = viper.BindPFlag("repo-access-cache-ttl", rootCmd.PersistentFlags().Lookup("repo-access-cache-ttl"))
 	_ = viper.BindPFlag("port", rootCmd.PersistentFlags().Lookup("port"))
 	_ = viper.BindPFlag("base-url", rootCmd.PersistentFlags().Lookup("base-url"))
+	_ = viper.BindPFlag("resource-path", rootCmd.PersistentFlags().Lookup("resource-path"))
 
 	// Add subcommands
 	rootCmd.AddCommand(stdioCmd)

pkg/http/handler.go

@@ -93,13 +93,16 @@ func NewHTTPMcpHandler(
 // RegisterRoutes registers the routes for the MCP server
 // URL-based values take precedence over header-based values
 func (h *Handler) RegisterRoutes(r chi.Router) {
-	r.Use(middleware.WithRequestConfig)
+	mcpRouter := chi.NewRouter()
+	mcpRouter.Use(middleware.WithRequestConfig)
 
-	r.Mount("/", h)
+	mcpRouter.Mount("/", h)
 	// Mount readonly and toolset routes
-	r.With(withToolset).Mount("/x/{toolset}", h)
-	r.With(withReadonly, withToolset).Mount("/x/{toolset}/readonly", h)
-	r.With(withReadonly).Mount("/readonly", h)
+	mcpRouter.With(withToolset).Mount("/x/{toolset}", h)
+	mcpRouter.With(withReadonly, withToolset).Mount("/x/{toolset}/readonly", h)
+	mcpRouter.With(withReadonly).Mount("/readonly", h)
+
+	r.Mount("/", mcpRouter)
 }
 
 // withReadonly is middleware that sets readonly mode in the request context

pkg/http/middleware/token.go

@@ -67,7 +67,8 @@ func ExtractUserToken(oauthCfg *oauth.Config) func(next http.Handler) http.Handl
 // sendAuthChallenge sends a 401 Unauthorized response with WWW-Authenticate header
 // containing the OAuth protected resource metadata URL as per RFC 6750 and MCP spec.
 func sendAuthChallenge(w http.ResponseWriter, r *http.Request, oauthCfg *oauth.Config) {
-	resourceMetadataURL := oauth.BuildResourceMetadataURL(r, oauthCfg, "mcp")
+	resourcePath := oauth.ResolveResourcePath(r, oauthCfg)
+	resourceMetadataURL := oauth.BuildResourceMetadataURL(r, oauthCfg, resourcePath)
 	w.Header().Set("WWW-Authenticate", fmt.Sprintf(`Bearer resource_metadata=%q`, resourceMetadataURL))
 	http.Error(w, "Unauthorized", http.StatusUnauthorized)
 }

pkg/http/oauth/oauth.go

@@ -3,14 +3,13 @@
 package oauth
 
 import (
+	"encoding/json"
 	"fmt"
 	"net/http"
-	"net/url"
 	"strings"
 
 	"github.com/github/github-mcp-server/pkg/http/headers"
 	"github.com/go-chi/chi/v5"
-	"github.com/modelcontextprotocol/go-sdk/auth"
 	"github.com/modelcontextprotocol/go-sdk/oauthex"
 )
 
@@ -48,17 +47,12 @@ type Config struct {
 	// Defaults to GitHub's OAuth server if not specified.
 	AuthorizationServer string
 
-	// ResourcePath is the resource path suffix (e.g., "/mcp").
-	// If empty, defaults to "/"
+	// ResourcePath is the externally visible base path for the MCP server (e.g., "/mcp").
+	// This is used to restore the original path when a proxy strips a base path before forwarding.
+	// If empty, requests are treated as already using the external path.
 	ResourcePath string
 }
 
-// ProtectedResourceData contains the data needed to build an OAuth protected resource response.
-type ProtectedResourceData struct {
-	ResourceURL         string
-	AuthorizationServer string
-}
-
 // AuthHandler handles OAuth-related HTTP endpoints.
 type AuthHandler struct {
 	cfg *Config
@@ -94,119 +88,165 @@ func (h *AuthHandler) RegisterRoutes(r chi.Router) {
 	for _, pattern := range routePatterns {
 		for _, route := range h.routesForPattern(pattern) {
 			path := OAuthProtectedResourcePrefix + route
-
-			// Build metadata for this specific resource path
-			metadata := h.buildMetadata(route)
-			r.Handle(path, auth.ProtectedResourceMetadataHandler(metadata))
+			r.Handle(path, h.metadataHandler())
 		}
 	}
 }
 
-func (h *AuthHandler) buildMetadata(resourcePath string) *oauthex.ProtectedResourceMetadata {
-	baseURL := strings.TrimSuffix(h.cfg.BaseURL, "/")
-	resourceURL := baseURL
-	if resourcePath != "" && resourcePath != "/" {
-		resourceURL = baseURL + resourcePath
-	}
+func (h *AuthHandler) metadataHandler() http.Handler {
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		// CORS headers for browser-based clients
+		w.Header().Set("Access-Control-Allow-Origin", "*")
+		w.Header().Set("Access-Control-Allow-Methods", "GET, OPTIONS")
+		w.Header().Set("Access-Control-Allow-Headers", "Content-Type, Authorization")
 
-	return &oauthex.ProtectedResourceMetadata{
-		Resource:               resourceURL,
-		AuthorizationServers:   []string{h.cfg.AuthorizationServer},
-		ResourceName:           "GitHub MCP Server",
-		ScopesSupported:        SupportedScopes,
-		BearerMethodsSupported: []string{"header"},
-	}
+		if r.Method == http.MethodOptions {
+			w.WriteHeader(http.StatusNoContent)
+			return
+		}
+		if r.Method != http.MethodGet {
+			w.WriteHeader(http.StatusMethodNotAllowed)
+			return
+		}
+
+		resourcePath := resolveResourcePath(
+			strings.TrimPrefix(r.URL.Path, OAuthProtectedResourcePrefix),
+			h.cfg.ResourcePath,
+		)
+		resourceURL := h.buildResourceURL(r, resourcePath)
+
+		w.Header().Set("Content-Type", "application/json")
+		_ = json.NewEncoder(w).Encode(&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
 // - Without /mcp prefix: for when GitHub has stripped the prefix
 func (h *AuthHandler) routesForPattern(pattern string) []string {
-	return []string{
-		pattern,
-		"/mcp" + pattern,
-		pattern + "/",
-		"/mcp" + pattern + "/",
+	basePaths := []string{""}
+	if basePath := normalizeBasePath(h.cfg.ResourcePath); basePath != "" {
+		basePaths = append(basePaths, basePath)
+	} else {
+		basePaths = append(basePaths, "/mcp")
 	}
+
+	routes := make([]string, 0, len(basePaths)*2)
+	for _, basePath := range basePaths {
+		routes = append(routes, joinRoute(basePath, pattern))
+		routes = append(routes, joinRoute(basePath, pattern)+"/")
+	}
+
+	return routes
+}
+
+// resolveResourcePath returns the externally visible resource path,
+// restoring the configured base path when proxies strip it before forwarding.
+func resolveResourcePath(path, basePath string) string {
+	if path == "" {
+		path = "/"
+	}
+	base := normalizeBasePath(basePath)
+	if base == "" {
+		return path
+	}
+	if path == "/" {
+		return base
+	}
+	if path == base || strings.HasPrefix(path, base+"/") {
+		return path
+	}
+	return base + path
 }
 
-// GetEffectiveResourcePath returns the resource path for OAuth protected resource URLs.
-// Since proxies may strip the /mcp prefix before forwarding requests, this function
-// restores the prefix for the external-facing URL.
-func GetEffectiveResourcePath(r *http.Request) string {
-	if r.URL.Path == "/" {
-		return "/mcp"
+// ResolveResourcePath returns the externally visible resource path for a request.
+// Exported for use by middleware.
+func ResolveResourcePath(r *http.Request, cfg *Config) string {
+	basePath := ""
+	if cfg != nil {
+		basePath = cfg.ResourcePath
 	}
-	return "/mcp" + r.URL.Path
+	return resolveResourcePath(r.URL.Path, basePath)
 }
 
-// GetProtectedResourceData builds the OAuth protected resource data for a request.
-func (h *AuthHandler) GetProtectedResourceData(r *http.Request, resourcePath string) (*ProtectedResourceData, error) {
+// buildResourceURL constructs the full resource URL for OAuth metadata.
+func (h *AuthHandler) buildResourceURL(r *http.Request, resourcePath string) string {
 	host, scheme := GetEffectiveHostAndScheme(r, h.cfg)
-
-	// Build the base URL
 	baseURL := fmt.Sprintf("%s://%s", scheme, host)
 	if h.cfg.BaseURL != "" {
 		baseURL = strings.TrimSuffix(h.cfg.BaseURL, "/")
 	}
-
-	// Build the resource URL using url.JoinPath for proper path handling
-	var resourceURL string
-	var err error
-	if resourcePath == "/" {
-		resourceURL = baseURL + "/"
-	} else {
-		resourceURL, err = url.JoinPath(baseURL, resourcePath)
-		if err != nil {
-			return nil, fmt.Errorf("failed to build resource URL: %w", err)
-		}
+	if resourcePath == "" {
+		resourcePath = "/"
 	}
-
-	return &ProtectedResourceData{
-		ResourceURL:         resourceURL,
-		AuthorizationServer: h.cfg.AuthorizationServer,
-	}, nil
+	if !strings.HasPrefix(resourcePath, "/") {
+		resourcePath = "/" + resourcePath
+	}
+	return baseURL + resourcePath
 }
 
 // GetEffectiveHostAndScheme returns the effective host and scheme for a request.
-// It checks X-Forwarded-Host and X-Forwarded-Proto headers first (set by proxies),
-// then falls back to the request's Host and TLS state.
-func GetEffectiveHostAndScheme(r *http.Request, cfg *Config) (host, scheme string) { //nolint:revive // parameters are required by http.oauth.BuildResourceMetadataURL signature
-	// Check for forwarded headers first (typically set by reverse proxies)
-	if forwardedHost := r.Header.Get(headers.ForwardedHostHeader); forwardedHost != "" {
-		host = forwardedHost
+func GetEffectiveHostAndScheme(r *http.Request, cfg *Config) (host, scheme string) { //nolint:revive
+	if fh := r.Header.Get(headers.ForwardedHostHeader); fh != "" {
+		host = fh
 	} else {
 		host = r.Host
 	}
-
-	// Determine scheme
-	switch {
-	case r.Header.Get(headers.ForwardedProtoHeader) != "":
-		scheme = strings.ToLower(r.Header.Get(headers.ForwardedProtoHeader))
-	case r.TLS != nil:
-		scheme = "https"
-	default:
-		// Default to HTTPS in production scenarios
-		scheme = "https"
+	if host == "" {
+		host = "localhost"
 	}
-
-	return host, scheme
+	if fp := r.Header.Get(headers.ForwardedProtoHeader); fp != "" {
+		scheme = strings.ToLower(fp)
+	} else {
+		scheme = "https" // Default to HTTPS
+	}
+	return
 }
 
 // BuildResourceMetadataURL constructs the full URL to the OAuth protected resource metadata endpoint.
 func BuildResourceMetadataURL(r *http.Request, cfg *Config, resourcePath string) string {
 	host, scheme := GetEffectiveHostAndScheme(r, cfg)
-
+	suffix := ""
+	if resourcePath != "" && resourcePath != "/" {
+		if !strings.HasPrefix(resourcePath, "/") {
+			suffix = "/" + resourcePath
+		} else {
+			suffix = resourcePath
+		}
+	}
 	if cfg != nil && cfg.BaseURL != "" {
-		baseURL := strings.TrimSuffix(cfg.BaseURL, "/")
-		return baseURL + OAuthProtectedResourcePrefix + "/" + strings.TrimPrefix(resourcePath, "/")
+		return strings.TrimSuffix(cfg.BaseURL, "/") + OAuthProtectedResourcePrefix + suffix
 	}
+	return fmt.Sprintf("%s://%s%s%s", scheme, host, OAuthProtectedResourcePrefix, suffix)
+}
 
-	path := OAuthProtectedResourcePrefix
-	if resourcePath != "" && resourcePath != "/" {
-		path = path + "/" + strings.TrimPrefix(resourcePath, "/")
+func normalizeBasePath(path string) string {
+	trimmed := strings.TrimSpace(path)
+	if trimmed == "" || trimmed == "/" {
+		return ""
 	}
+	if !strings.HasPrefix(trimmed, "/") {
+		trimmed = "/" + trimmed
+	}
+	return strings.TrimSuffix(trimmed, "/")
+}
 
-	return fmt.Sprintf("%s://%s%s", scheme, host, path)
+func joinRoute(basePath, pattern string) string {
+	if basePath == "" {
+		return pattern
+	}
+	if pattern == "" {
+		return basePath
+	}
+	if strings.HasSuffix(basePath, "/") {
+		return strings.TrimSuffix(basePath, "/") + pattern
+	}
+	return basePath + pattern
 }