From 77bb8237fddd2da215a1f751b3ad8ba1cf0c6c20 Mon Sep 17 00:00:00 2001 From: thomcost Date: Sat, 7 Jun 2025 07:42:34 +0000 Subject: [PATCH 1/2] Add multi-user HTTP mode: per-request GitHub token, docs, and tests --- README.md | 44 +++++++++++++++++++ cmd/github-mcp-server/main.go | 30 +++++++++++++ e2e/multiuser_e2e_test.go | 47 +++++++++++++++++++++ internal/ghmcp/multiuser_test.go | 72 ++++++++++++++++++++++++++++++++ internal/ghmcp/server.go | 68 +++++++++++++++++++++++++++++- 5 files changed, 260 insertions(+), 1 deletion(-) create mode 100644 e2e/multiuser_e2e_test.go create mode 100644 internal/ghmcp/multiuser_test.go diff --git a/README.md b/README.md index a17bf435..1ba86266 100644 --- a/README.md +++ b/README.md @@ -675,3 +675,47 @@ The exported Go API of this module should currently be considered unstable, and ## License This project is licensed under the terms of the MIT open source license. Please refer to [MIT](./LICENSE) for the full terms. + +## Multi-User HTTP Mode (Experimental) + +The GitHub MCP Server supports a multi-user HTTP mode for enterprise and cloud scenarios. In this mode, the server does **not** require a global GitHub token at startup. Instead, each HTTP request must include a GitHub token in the `Authorization` header: + +- The token is **never** passed as a tool parameter or exposed to the agent/model. +- The server extracts the token from the `Authorization` header for each request and creates a new GitHub client per request. +- This enables secure, scalable, and multi-tenant deployments. + +### Usage + +Start the server in multi-user mode on a configurable port (default: 8080): + +```bash +./github-mcp-server multi-user --port 8080 +``` + +#### Example HTTP Request + +```http +POST /v1/mcp HTTP/1.1 +Host: localhost:8080 +Authorization: Bearer +Content-Type: application/json + +{ ...MCP request body... } +``` + +- The `Authorization` header is **required** for every request. +- The server will return 401 Unauthorized if the header is missing. + +### Security Note +- The agent and model never see the token value. +- This is the recommended and secure approach for HTTP APIs. + +### Use Cases +- Multi-tenant SaaS +- Shared enterprise deployments +- Web integrations where each user authenticates with their own GitHub token + +### Backward Compatibility +- Single-user `stdio` and HTTP modes are still supported and unchanged. + +--- diff --git a/cmd/github-mcp-server/main.go b/cmd/github-mcp-server/main.go index fb716f78..9bf0aae4 100644 --- a/cmd/github-mcp-server/main.go +++ b/cmd/github-mcp-server/main.go @@ -58,6 +58,33 @@ var ( return ghmcp.RunStdioServer(stdioServerConfig) }, } + + multiUserCmd = &cobra.Command{ + Use: "multi-user", + Short: "Start multi-user HTTP server (experimental)", + Long: `Start a streamable HTTP server that supports per-request GitHub authentication tokens for multi-user scenarios.`, + RunE: func(cmd *cobra.Command, _ []string) error { + port := viper.GetInt("port") + if port == 0 { + port = 8080 // default + } + + var enabledToolsets []string + if err := viper.UnmarshalKey("toolsets", &enabledToolsets); err != nil { + return fmt.Errorf("failed to unmarshal toolsets: %w", err) + } + + multiUserConfig := ghmcp.MultiUserHTTPServerConfig{ + Version: version, + Host: viper.GetString("host"), + EnabledToolsets: enabledToolsets, + DynamicToolsets: viper.GetBool("dynamic_toolsets"), + ReadOnly: viper.GetBool("read-only"), + Port: port, + } + return ghmcp.RunMultiUserHTTPServer(multiUserConfig) + }, + } ) func init() { @@ -73,6 +100,7 @@ func init() { rootCmd.PersistentFlags().Bool("enable-command-logging", false, "When enabled, the server will log all command requests and responses to the log file") rootCmd.PersistentFlags().Bool("export-translations", false, "Save translations to a JSON file") rootCmd.PersistentFlags().String("gh-host", "", "Specify the GitHub hostname (for GitHub Enterprise etc.)") + rootCmd.PersistentFlags().Int("port", 8080, "Port to bind the HTTP server to (multi-user mode)") // Bind flag to viper _ = viper.BindPFlag("toolsets", rootCmd.PersistentFlags().Lookup("toolsets")) @@ -82,9 +110,11 @@ func init() { _ = viper.BindPFlag("enable-command-logging", rootCmd.PersistentFlags().Lookup("enable-command-logging")) _ = viper.BindPFlag("export-translations", rootCmd.PersistentFlags().Lookup("export-translations")) _ = viper.BindPFlag("host", rootCmd.PersistentFlags().Lookup("gh-host")) + _ = viper.BindPFlag("port", rootCmd.PersistentFlags().Lookup("port")) // Add subcommands rootCmd.AddCommand(stdioCmd) + rootCmd.AddCommand(multiUserCmd) } func initConfig() { diff --git a/e2e/multiuser_e2e_test.go b/e2e/multiuser_e2e_test.go new file mode 100644 index 00000000..f40a6b84 --- /dev/null +++ b/e2e/multiuser_e2e_test.go @@ -0,0 +1,47 @@ +//go:build e2e + +package e2e_test + +import ( + "bytes" + "encoding/json" + "net/http" + "os/exec" + "testing" + "time" +) + +func TestMultiUserHTTPServer_Integration(t *testing.T) { + // Start the server in multi-user mode on a random port (e.g. 18080) + cmd := exec.Command("../cmd/github-mcp-server/github-mcp-server", "multi-user", "--port", "18080") + cmd.Stdout = nil + cmd.Stderr = nil + if err := cmd.Start(); err != nil { + t.Fatalf("failed to start server: %v", err) + } + defer cmd.Process.Kill() + // Wait for server to start + time.Sleep(2 * time.Second) + + // Make a request without Authorization header + resp, err := http.Post("http://localhost:18080/v1/mcp", "application/json", bytes.NewBufferString(`{"test":"noauth"}`)) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if resp.StatusCode != http.StatusUnauthorized { + t.Errorf("expected 401 Unauthorized, got %d", resp.StatusCode) + } + + // Make a request with Authorization header + body, _ := json.Marshal(map[string]string{"test": "authed"}) + req, _ := http.NewRequest("POST", "http://localhost:18080/v1/mcp", bytes.NewBuffer(body)) + req.Header.Set("Authorization", "Bearer testtoken123") + req.Header.Set("Content-Type", "application/json") + resp2, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if resp2.StatusCode == http.StatusUnauthorized { + t.Errorf("expected not 401, got 401 (token should be accepted if server is running and token is valid)") + } +} diff --git a/internal/ghmcp/multiuser_test.go b/internal/ghmcp/multiuser_test.go new file mode 100644 index 00000000..804b9e7c --- /dev/null +++ b/internal/ghmcp/multiuser_test.go @@ -0,0 +1,72 @@ +package ghmcp + +import ( + "bytes" + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "testing" +) + +type dummyRequest struct { + Test string `json:"test"` +} + +func TestMultiUserHTTPServer_TokenRequired(t *testing.T) { + h := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + token := extractTokenFromRequest(r) + if token == "" { + w.WriteHeader(http.StatusUnauthorized) + _, _ = w.Write([]byte(`{"error":"missing GitHub token in Authorization header"}`)) + return + } + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`{"ok":true}`)) + }) + + ts := httptest.NewServer(h) + defer ts.Close() + + // No Authorization header + resp, err := http.Post(ts.URL, "application/json", bytes.NewBufferString(`{"test":"noauth"}`)) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if resp.StatusCode != http.StatusUnauthorized { + t.Errorf("expected 401 Unauthorized, got %d", resp.StatusCode) + } +} + +func TestMultiUserHTTPServer_ValidToken(t *testing.T) { + h := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + token := extractTokenFromRequest(r) + if token == "" { + w.WriteHeader(http.StatusUnauthorized) + _, _ = w.Write([]byte(`{"error":"missing GitHub token in Authorization header"}`)) + return + } + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`{"ok":true,"token":"` + token + `"}`)) + }) + + ts := httptest.NewServer(h) + defer ts.Close() + + // With Authorization header + body, _ := json.Marshal(dummyRequest{Test: "authed"}) + req, _ := http.NewRequest("POST", ts.URL, bytes.NewBuffer(body)) + req.Header.Set("Authorization", "Bearer testtoken123") + req.Header.Set("Content-Type", "application/json") + resp, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if resp.StatusCode != http.StatusOK { + t.Errorf("expected 200 OK, got %d", resp.StatusCode) + } + data, _ := io.ReadAll(resp.Body) + if !bytes.Contains(data, []byte("testtoken123")) { + t.Errorf("expected token in response, got %s", string(data)) + } +} diff --git a/internal/ghmcp/server.go b/internal/ghmcp/server.go index 593411ae..9f856d67 100644 --- a/internal/ghmcp/server.go +++ b/internal/ghmcp/server.go @@ -95,7 +95,7 @@ func NewMCPServer(cfg MCPServerConfig) (*server.MCPServer, error) { enabledToolsets := cfg.EnabledToolsets if cfg.DynamicToolsets { - // filter "all" from the enabled toolsets + // filter "all" from the enabled tool sets enabledToolsets = make([]string, 0, len(cfg.EnabledToolsets)) for _, toolset := range cfg.EnabledToolsets { if toolset != "all" { @@ -374,3 +374,69 @@ func (t *bearerAuthTransport) RoundTrip(req *http.Request) (*http.Response, erro req.Header.Set("Authorization", "Bearer "+t.token) return t.transport.RoundTrip(req) } + +// MultiUserHTTPServerConfig holds config for the multi-user HTTP server +// (no global token, per-request tokens) +type MultiUserHTTPServerConfig struct { + Version string + Host string + EnabledToolsets []string + DynamicToolsets bool + ReadOnly bool + Port int +} + +// RunMultiUserHTTPServer starts a streamable HTTP server that supports per-request GitHub tokens +func RunMultiUserHTTPServer(cfg MultiUserHTTPServerConfig) error { + ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM) + defer stop() + + t, _ := translations.TranslationHelper() + + handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + token := extractTokenFromRequest(r) + if token == "" { + w.WriteHeader(http.StatusUnauthorized) + _, _ = w.Write([]byte(`{"error":"missing GitHub token in Authorization header"}`)) + return + } + ghServer, err := NewMCPServer(MCPServerConfig{ + Version: cfg.Version, + Host: cfg.Host, + Token: token, + EnabledToolsets: cfg.EnabledToolsets, + DynamicToolsets: cfg.DynamicToolsets, + ReadOnly: cfg.ReadOnly, + Translator: t, + }) + if err != nil { + w.WriteHeader(http.StatusInternalServerError) + _, _ = w.Write([]byte(`{"error":"failed to create MCP server"}`)) + return + } + // Use the MCP server's HTTP handler for this request + mcpHTTP := server.NewStreamableHTTPServer(ghServer) + mcpHTTP.ServeHTTP(w, r) + }) + + addr := fmt.Sprintf(":%d", cfg.Port) + fmt.Fprintf(os.Stderr, "GitHub MCP Server running in multi-user HTTP mode on %s\n", addr) + server := &http.Server{Addr: addr, Handler: handler} + go func() { + <-ctx.Done() + _ = server.Shutdown(context.Background()) + }() + return server.ListenAndServe() +} + +// extractTokenFromRequest extracts the GitHub token from the Authorization header +func extractTokenFromRequest(r *http.Request) string { + h := r.Header.Get("Authorization") + if h == "" { + return "" + } + if strings.HasPrefix(h, "Bearer ") { + return strings.TrimPrefix(h, "Bearer ") + } + return h +} From 4b15b5328ae25df2b19f351598f99b624d7f3b58 Mon Sep 17 00:00:00 2001 From: Thomas Costello Date: Sat, 7 Jun 2025 00:58:06 -0700 Subject: [PATCH 2/2] Optimize multi-user HTTP mode performance and security - Replace per-request MCP server creation with token-aware client factories - Add proper HTTP timeouts and graceful shutdown - Enhance token validation with strict Bearer format checking - Add comprehensive tests for token extraction and context injection - Improve error handling and response formatting - Update documentation to reflect performance optimizations --- README.md | 3 +- internal/ghmcp/multiuser_test.go | 94 +++++++++++++++++ internal/ghmcp/server.go | 168 ++++++++++++++++++++++++------- 3 files changed, 230 insertions(+), 35 deletions(-) diff --git a/README.md b/README.md index 1ba86266..1d93fa50 100644 --- a/README.md +++ b/README.md @@ -681,7 +681,8 @@ This project is licensed under the terms of the MIT open source license. Please The GitHub MCP Server supports a multi-user HTTP mode for enterprise and cloud scenarios. In this mode, the server does **not** require a global GitHub token at startup. Instead, each HTTP request must include a GitHub token in the `Authorization` header: - The token is **never** passed as a tool parameter or exposed to the agent/model. -- The server extracts the token from the `Authorization` header for each request and creates a new GitHub client per request. +- The server extracts the token from the `Authorization` header for each request and creates GitHub clients per request using token-aware client factories. +- Optimized for performance: single MCP server instance with per-request authentication. - This enables secure, scalable, and multi-tenant deployments. ### Usage diff --git a/internal/ghmcp/multiuser_test.go b/internal/ghmcp/multiuser_test.go index 804b9e7c..d0fd3a8f 100644 --- a/internal/ghmcp/multiuser_test.go +++ b/internal/ghmcp/multiuser_test.go @@ -6,6 +6,7 @@ import ( "io" "net/http" "net/http/httptest" + "strings" "testing" ) @@ -70,3 +71,96 @@ func TestMultiUserHTTPServer_ValidToken(t *testing.T) { t.Errorf("expected token in response, got %s", string(data)) } } + +func TestExtractTokenFromRequest_StrictValidation(t *testing.T) { + tests := []struct { + name string + header string + expected string + }{ + {"valid bearer token", "Bearer ghp_1234567890abcdef", "ghp_1234567890abcdef"}, + {"valid long token", "Bearer github_pat_11AAAAAAA0AAAAAAAAAAAAAAAAA", "github_pat_11AAAAAAA0AAAAAAAAAAAAAAAAA"}, + {"missing header", "", ""}, + {"malformed - no Bearer", "ghp_1234567890abcdef", ""}, + {"malformed - wrong case", "bearer ghp_1234567890abcdef", ""}, + {"too short token", "Bearer abc", ""}, + {"empty token", "Bearer ", ""}, + {"only Bearer", "Bearer", ""}, + {"spaces in token", "Bearer ghp_123 456", "ghp_123 456"}, // This should work + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + req := &http.Request{ + Header: http.Header{}, + } + if tt.header != "" { + req.Header.Set("Authorization", tt.header) + } + + result := extractTokenFromRequest(req) + if result != tt.expected { + t.Errorf("expected %q, got %q", tt.expected, result) + } + }) + } +} + +func TestMultiUserHandler_ContextInjection(t *testing.T) { + // Mock MCP server that verifies token is in context + mockMCP := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + token, ok := r.Context().Value("github_token").(string) + if !ok { + t.Error("token not found in context") + w.WriteHeader(http.StatusInternalServerError) + return + } + if token != "test_token_123456" { + t.Errorf("wrong token in context: %s", token) + w.WriteHeader(http.StatusInternalServerError) + return + } + w.WriteHeader(http.StatusOK) + w.Write([]byte(`{"success":true}`)) + }) + + handler := &multiUserHandler{mcpServer: mockMCP} + + req := httptest.NewRequest("POST", "/", strings.NewReader("{}")) + req.Header.Set("Authorization", "Bearer test_token_123456") + + w := httptest.NewRecorder() + handler.ServeHTTP(w, req) + + if w.Code != http.StatusOK { + t.Errorf("expected 200, got %d. Body: %s", w.Code, w.Body.String()) + } +} + +func TestMultiUserHandler_MissingToken(t *testing.T) { + // Mock MCP server (should not be called) + mockMCP := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + t.Error("MCP server should not be called when token is missing") + }) + + handler := &multiUserHandler{mcpServer: mockMCP} + + req := httptest.NewRequest("POST", "/", strings.NewReader("{}")) + // No Authorization header + + w := httptest.NewRecorder() + handler.ServeHTTP(w, req) + + if w.Code != http.StatusUnauthorized { + t.Errorf("expected 401, got %d", w.Code) + } + + contentType := w.Header().Get("Content-Type") + if contentType != "application/json" { + t.Errorf("expected Content-Type: application/json, got %s", contentType) + } + + if !strings.Contains(w.Body.String(), "missing GitHub token") { + t.Errorf("expected error message about missing token, got: %s", w.Body.String()) + } +} diff --git a/internal/ghmcp/server.go b/internal/ghmcp/server.go index 9f856d67..6d16ce3b 100644 --- a/internal/ghmcp/server.go +++ b/internal/ghmcp/server.go @@ -11,6 +11,7 @@ import ( "os/signal" "strings" "syscall" + "time" "github.com/github/github-mcp-server/pkg/github" mcplog "github.com/github/github-mcp-server/pkg/log" @@ -393,50 +394,149 @@ func RunMultiUserHTTPServer(cfg MultiUserHTTPServerConfig) error { t, _ := translations.TranslationHelper() - handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - token := extractTokenFromRequest(r) - if token == "" { - w.WriteHeader(http.StatusUnauthorized) - _, _ = w.Write([]byte(`{"error":"missing GitHub token in Authorization header"}`)) - return + // Parse API host once for reuse + apiHost, err := parseAPIHost(cfg.Host) + if err != nil { + return fmt.Errorf("failed to parse API host: %w", err) + } + + // Create token-aware client factories that extract tokens from request context + getClient := func(ctx context.Context) (*gogithub.Client, error) { + token, ok := ctx.Value("github_token").(string) + if !ok || token == "" { + return nil, fmt.Errorf("no GitHub token found in request context") } - ghServer, err := NewMCPServer(MCPServerConfig{ - Version: cfg.Version, - Host: cfg.Host, - Token: token, - EnabledToolsets: cfg.EnabledToolsets, - DynamicToolsets: cfg.DynamicToolsets, - ReadOnly: cfg.ReadOnly, - Translator: t, - }) - if err != nil { - w.WriteHeader(http.StatusInternalServerError) - _, _ = w.Write([]byte(`{"error":"failed to create MCP server"}`)) - return + + client := gogithub.NewClient(nil).WithAuthToken(token) + client.UserAgent = fmt.Sprintf("github-mcp-server/%s", cfg.Version) + client.BaseURL = apiHost.baseRESTURL + client.UploadURL = apiHost.uploadURL + return client, nil + } + + getGQLClient := func(ctx context.Context) (*githubv4.Client, error) { + token, ok := ctx.Value("github_token").(string) + if !ok || token == "" { + return nil, fmt.Errorf("no GitHub token found in request context") } - // Use the MCP server's HTTP handler for this request - mcpHTTP := server.NewStreamableHTTPServer(ghServer) - mcpHTTP.ServeHTTP(w, r) - }) - addr := fmt.Sprintf(":%d", cfg.Port) - fmt.Fprintf(os.Stderr, "GitHub MCP Server running in multi-user HTTP mode on %s\n", addr) - server := &http.Server{Addr: addr, Handler: handler} + httpClient := &http.Client{ + Transport: &bearerAuthTransport{ + transport: http.DefaultTransport, + token: token, + }, + } + return githubv4.NewEnterpriseClient(apiHost.graphqlURL.String(), httpClient), nil + } + + // Create MCP server once with token-aware client factories + ghServer := github.NewServer(cfg.Version) + + enabledToolsets := cfg.EnabledToolsets + if cfg.DynamicToolsets { + // filter "all" from the enabled toolsets + enabledToolsets = make([]string, 0, len(cfg.EnabledToolsets)) + for _, toolset := range cfg.EnabledToolsets { + if toolset != "all" { + enabledToolsets = append(enabledToolsets, toolset) + } + } + } + + // Create default toolsets with token-aware client factories + tsg := github.DefaultToolsetGroup(cfg.ReadOnly, getClient, getGQLClient, t) + err = tsg.EnableToolsets(enabledToolsets) + if err != nil { + return fmt.Errorf("failed to enable toolsets: %w", err) + } + + contextToolset := github.InitContextToolset(getClient, t) + github.RegisterResources(ghServer, getClient, t) + + // Register the tools with the server + tsg.RegisterTools(ghServer) + contextToolset.RegisterTools(ghServer) + + if cfg.DynamicToolsets { + dynamic := github.InitDynamicToolset(ghServer, tsg, t) + dynamic.RegisterTools(ghServer) + } + + // Create streamable HTTP server once + mcpHTTPServer := server.NewStreamableHTTPServer(ghServer) + + // Create HTTP handler that injects tokens into context + handler := &multiUserHandler{ + mcpServer: mcpHTTPServer, + } + + // Setup HTTP server with proper timeouts + httpServer := &http.Server{ + Addr: fmt.Sprintf(":%d", cfg.Port), + Handler: handler, + ReadTimeout: 30 * time.Second, + WriteTimeout: 30 * time.Second, + IdleTimeout: 120 * time.Second, + } + + fmt.Fprintf(os.Stderr, "GitHub MCP Server running in multi-user HTTP mode on :%d\n", cfg.Port) + + // Start server in goroutine + errChan := make(chan error, 1) go func() { - <-ctx.Done() - _ = server.Shutdown(context.Background()) + if err := httpServer.ListenAndServe(); err != nil && err != http.ErrServerClosed { + errChan <- err + } }() - return server.ListenAndServe() + + // Wait for shutdown signal or error + select { + case <-ctx.Done(): + fmt.Fprintf(os.Stderr, "Shutting down server...\n") + shutdownCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + return httpServer.Shutdown(shutdownCtx) + case err := <-errChan: + return fmt.Errorf("server error: %w", err) + } +} + +// multiUserHandler handles per-request token extraction and injection +type multiUserHandler struct { + mcpServer http.Handler +} + +func (h *multiUserHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { + token := extractTokenFromRequest(r) + if token == "" { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusUnauthorized) + _, _ = w.Write([]byte(`{"error":"missing GitHub token in Authorization header"}`)) + return + } + + // Inject token into request context + ctx := context.WithValue(r.Context(), "github_token", token) + h.mcpServer.ServeHTTP(w, r.WithContext(ctx)) } // extractTokenFromRequest extracts the GitHub token from the Authorization header func extractTokenFromRequest(r *http.Request) string { - h := r.Header.Get("Authorization") - if h == "" { + authHeader := r.Header.Get("Authorization") + if authHeader == "" { return "" } - if strings.HasPrefix(h, "Bearer ") { - return strings.TrimPrefix(h, "Bearer ") + + // Only accept proper Bearer token format + if !strings.HasPrefix(authHeader, "Bearer ") { + return "" + } + + token := strings.TrimPrefix(authHeader, "Bearer ") + // Basic validation - non-empty and reasonable length + if len(token) < 10 { + return "" } - return h + + return token }