Merge branch 'oauth-handler-implementation' into scope-challenge-http

Author: omgitsads

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("base-path"),
 				ExportTranslations:   viper.GetBool("export-translations"),
 				EnableCommandLogging: viper.GetBool("enable-command-logging"),
 				LogFilePath:          viper.GetString("log-file"),
@@ -136,9 +137,10 @@ func init() {
 	rootCmd.PersistentFlags().Bool("insiders", false, "Enable insiders features")
 	rootCmd.PersistentFlags().Duration("repo-access-cache-ttl", 5*time.Minute, "Override the repo access cache TTL (e.g. 1m, 0s to disable)")
 
-	// Add port flag to http command
-	httpCmd.PersistentFlags().Int("port", 8082, "HTTP server port")
-	httpCmd.PersistentFlags().String("base-url", "", "Base URL where this server is publicly accessible (for OAuth resource metadata)")
+	// HTTP command flags
+	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("base-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,8 +156,11 @@ func init() {
 	_ = viper.BindPFlag("lockdown-mode", rootCmd.PersistentFlags().Lookup("lockdown-mode"))
 	_ = viper.BindPFlag("insiders", rootCmd.PersistentFlags().Lookup("insiders"))
 	_ = viper.BindPFlag("repo-access-cache-ttl", rootCmd.PersistentFlags().Lookup("repo-access-cache-ttl"))
+
+	// HTTP command flags
 	_ = viper.BindPFlag("port", httpCmd.PersistentFlags().Lookup("port"))
 	_ = viper.BindPFlag("base-url", httpCmd.PersistentFlags().Lookup("base-url"))
+	_ = viper.BindPFlag("base-path", httpCmd.PersistentFlags().Lookup("base-path"))
 
 	// Add subcommands
 	rootCmd.AddCommand(stdioCmd)

pkg/http/headers/headers.go

@@ -26,10 +26,6 @@ const (
 	// ForwardedProtoHeader is a standard HTTP Header for preserving the original protocol when proxying.
 	ForwardedProtoHeader = "X-Forwarded-Proto"
 
-	// OriginalPathHeader is set to preserve the original request path
-	// before the /mcp prefix was stripped during proxying.
-	OriginalPathHeader = "X-GitHub-Original-Path"
-
 	// RequestHmacHeader is used to authenticate requests to the Raw API.
 	RequestHmacHeader = "Request-Hmac"
 

pkg/http/middleware/scope_challenge.go

@@ -150,7 +150,7 @@ func WithScopeChallenge(oauthCfg *oauth.Config) func(http.Handler) http.Handler
 			// Build the resource metadata URL using the shared utility
 			// GetEffectiveResourcePath returns the original path (e.g., /mcp or /mcp/x/all)
 			// which is used to construct the well-known OAuth protected resource URL
-			resourcePath := oauth.GetEffectiveResourcePath(r)
+			resourcePath := oauth.ResolveResourcePath(r, oauthCfg)
 			resourceMetadataURL := oauth.BuildResourceMetadataURL(r, oauthCfg, resourcePath)
 
 			// Build recommended scopes: existing scopes + required scopes

pkg/http/middleware/token.go

@@ -37,7 +37,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

@@ -5,7 +5,6 @@ package oauth
 import (
 	"fmt"
 	"net/http"
-	"net/url"
 	"strings"
 
 	"github.com/github/github-mcp-server/pkg/http/headers"
@@ -51,17 +50,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
@@ -97,127 +91,152 @@ 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) {
+		resourcePath := resolveResourcePath(
+			strings.TrimPrefix(r.URL.Path, OAuthProtectedResourcePrefix),
+			h.cfg.ResourcePath,
+		)
+		resourceURL := h.buildResourceURL(r, resourcePath)
+
+		metadata := &oauthex.ProtectedResourceMetadata{
+			Resource:               resourceURL,
+			AuthorizationServers:   []string{h.cfg.AuthorizationServer},
+			ResourceName:           "GitHub MCP Server",
+			ScopesSupported:        SupportedScopes,
+			BearerMethodsSupported: []string{"header"},
+		}
 
-	return &oauthex.ProtectedResourceMetadata{
-		Resource:               resourceURL,
-		AuthorizationServers:   []string{h.cfg.AuthorizationServer},
-		ResourceName:           "GitHub MCP Server",
-		ScopesSupported:        SupportedScopes,
-		BearerMethodsSupported: []string{"header"},
-	}
+		auth.ProtectedResourceMetadataHandler(metadata).ServeHTTP(w, r)
+	})
 }
 
 // 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
 }
 
-// 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.
-// If the header is not present, it falls back to
-// restoring the /mcp prefix.
-func GetEffectiveResourcePath(r *http.Request) string {
-	// Check for the original path header from GitHub (preferred method)
-	if originalPath := r.Header.Get(headers.OriginalPathHeader); originalPath != "" {
-		return originalPath
+// 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
+}
 
-	// Fallback: GitHub strips /mcp prefix, so we need to restore it for the external URL
-	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
 }