Conversation
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…hub/github-mcp-server into oauth-handler-implementation
There was a problem hiding this comment.
Pull request overview
This PR implements OAuth scope challenge functionality for the HTTP server, adding token type detection, scope validation, and tool filtering based on token scopes. The implementation includes new middleware for parsing MCP requests and validating OAuth scopes, along with supporting infrastructure for token type identification and scope management.
Changes:
- Adds OAuth scope challenge middleware that validates token scopes and returns WWW-Authenticate challenges for insufficient permissions
- Implements token type detection for PATs, fine-grained PATs, OAuth tokens, GitHub App tokens, and IDE tokens
- Adds MCP request parsing middleware to extract method and tool information early in the request lifecycle
- Introduces tool scope mapping infrastructure to track which tools require which OAuth scopes
- Adds CLI flag
--scope-challengeto enable the scope challenge feature
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/utils/token.go | New token parsing utility with type detection for various GitHub token formats |
| pkg/utils/api.go | Adds default API host resolver factory function |
| pkg/scopes/map.go | Tool scope mapping infrastructure for tracking tool scope requirements |
| pkg/scopes/map_test.go | Tests for tool scope mapping functionality |
| pkg/scopes/fetcher.go | Updates scope fetcher to use APIHostResolver interface instead of string |
| pkg/scopes/fetcher_test.go | Updates tests to use testAPIHostResolver |
| pkg/http/middleware/scope_challenge.go | New middleware for OAuth scope validation and challenge responses |
| pkg/http/middleware/mcp_parse.go | New middleware for early MCP JSON-RPC request parsing |
| pkg/http/middleware/token.go | Refactors token extraction to use centralized parsing utility |
| pkg/http/server.go | Adds ScopeChallenge config field and initializes scope fetcher |
| pkg/http/handler.go | Integrates scope challenge and fetcher into handler lifecycle |
| pkg/http/handler_test.go | Updates tests with scope fetcher mocks |
| pkg/context/token.go | Changes token context from string to TokenInfo struct with type information |
| pkg/context/mcp_info.go | New context type for storing parsed MCP method information |
| pkg/github/server.go | Uses MCP method info from context to optimize inventory filtering |
| pkg/github/dependencies.go | Updates to extract token from new TokenInfo structure |
| internal/ghmcp/server.go | Updates scope fetcher instantiation to use APIHostResolver |
| cmd/github-mcp-server/main.go | Adds --scope-challenge CLI flag |
Comments suppressed due to low confidence (3)
pkg/http/server.go:143
- Variable name typo: "severOptions" should be "serverOptions" to match the corrected variable name.
handler := NewHTTPMcpHandler(ctx, &cfg, deps, t, logger, apiHost, append(severOptions, WithFeatureChecker(featureChecker), WithOAuthConfig(oauthCfg))...)
pkg/http/server.go:139
- Variable name typo: "severOptions" should be "serverOptions" to match the corrected variable name.
severOptions = append(severOptions, WithScopeFetcher(scopeFetcher))
pkg/http/middleware/scope_challenge.go:178
- The new
scope_challenge.gomiddleware lacks test coverage. This middleware handles critical OAuth scope validation and authorization logic, including:
- Determining when to send scope challenges
- Fetching token scopes from GitHub API
- Checking if tools have required scopes
- Building WWW-Authenticate headers
Tests should verify the middleware behavior for different token types, scope combinations, error conditions, and ensure the fallback parsing logic works correctly.
package middleware
import (
"bytes"
"encoding/json"
"fmt"
"io"
"net/http"
"strings"
ghcontext "github.com/github/github-mcp-server/pkg/context"
"github.com/github/github-mcp-server/pkg/http/oauth"
"github.com/github/github-mcp-server/pkg/scopes"
"github.com/github/github-mcp-server/pkg/utils"
)
// WithScopeChallenge creates a new middleware that determines if an OAuth request contains sufficient scopes to
// complete the request and returns a scope challenge if not.
func WithScopeChallenge(oauthCfg *oauth.Config, scopeFetcher scopes.FetcherInterface) func(http.Handler) http.Handler {
return func(next http.Handler) http.Handler {
fn := func(w http.ResponseWriter, r *http.Request) {
ctx := r.Context()
// Skip health check endpoints
if r.URL.Path == "/_ping" {
next.ServeHTTP(w, r)
return
}
// Get user from context
tokenInfo, ok := ghcontext.GetTokenInfo(ctx)
if !ok {
next.ServeHTTP(w, r)
return
}
// Only check OAuth tokens - scope challenge allows OAuth apps to request additional scopes
if tokenInfo.TokenType != utils.TokenTypeOAuthAccessToken {
next.ServeHTTP(w, r)
return
}
// Try to use pre-parsed MCP method info first (performance optimization)
// This avoids re-parsing the JSON body if WithMCPParse middleware ran earlier
var toolName string
if methodInfo, ok := ghcontext.MCPMethod(ctx); ok && methodInfo != nil {
// Only check tools/call requests
if methodInfo.Method != "tools/call" {
next.ServeHTTP(w, r)
return
}
toolName = methodInfo.ItemName
} else {
// Fallback: parse the request body directly
body, err := io.ReadAll(r.Body)
if err != nil {
next.ServeHTTP(w, r)
return
}
r.Body = io.NopCloser(bytes.NewReader(body))
var mcpRequest struct {
JSONRPC string `json:"jsonrpc"`
Method string `json:"method"`
Params struct {
Name string `json:"name,omitempty"`
Arguments map[string]any `json:"arguments,omitempty"`
} `json:"params"`
}
err = json.Unmarshal(body, &mcpRequest)
if err != nil {
next.ServeHTTP(w, r)
return
}
// Only check tools/call requests
if mcpRequest.Method != "tools/call" {
next.ServeHTTP(w, r)
return
}
toolName = mcpRequest.Params.Name
}
toolScopeInfo, err := scopes.GetToolScopeInfo(toolName)
if err != nil {
next.ServeHTTP(w, r)
return
}
// If tool not found in scope map, allow the request
if toolScopeInfo == nil {
next.ServeHTTP(w, r)
return
}
// Get OAuth scopes from GitHub API
activeScopes, err := scopeFetcher.FetchTokenScopes(ctx, tokenInfo.Token)
if err != nil {
next.ServeHTTP(w, r)
return
}
// Store active scopes in context for downstream use
ghcontext.SetTokenScopes(ctx, activeScopes)
// Check if user has the required scopes
if toolScopeInfo.HasAcceptedScope(activeScopes...) {
next.ServeHTTP(w, r)
return
}
// User lacks required scopes - get the scopes they need
requiredScopes := toolScopeInfo.GetRequiredScopesSlice()
// 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.ResolveResourcePath(r, oauthCfg)
resourceMetadataURL := oauth.BuildResourceMetadataURL(r, oauthCfg, resourcePath)
// Build recommended scopes: existing scopes + required scopes
recommendedScopes := make([]string, 0, len(activeScopes)+len(requiredScopes))
recommendedScopes = append(recommendedScopes, activeScopes...)
recommendedScopes = append(recommendedScopes, requiredScopes...)
// Build the WWW-Authenticate header value
wwwAuthenticateHeader := fmt.Sprintf(`Bearer error="insufficient_scope", scope=%q, resource_metadata=%q, error_description=%q`,
strings.Join(recommendedScopes, " "),
resourceMetadataURL,
"Additional scopes required: "+strings.Join(requiredScopes, ", "),
)
// Send scope challenge response with the superset of existing and required scopes
w.Header().Set("WWW-Authenticate", wwwAuthenticateHeader)
http.Error(w, "Forbidden: insufficient scopes", http.StatusForbidden)
}
return http.HandlerFunc(fn)
}
}
SamMorrowDrums
left a comment
There was a problem hiding this comment.
I think this is looking good, didn't test it again, but I think it's a great and pretty substantial bit of work so I'm very happy to see it land!
| WithFeatureChecker(featureChecker) | ||
|
|
||
| b = InventoryFiltersForRequest(r, b) | ||
| b = PATScopeFilter(b, r, scopeFetcher) |
There was a problem hiding this comment.
Sanity check, this is still done in stdio server too right?
There was a problem hiding this comment.
Yeah, this runs slightly differently in STDIO, since you get the scopes at server startup: https://github.com/github/github-mcp-server/blob/scope-challenge-http/internal/ghmcp/server.go#L142-L144
e0db9b3 to
cb957d0
Compare
Summary
Adds Scope Challenge middleware for OAuth tokens, and Scope tool filtering for PATs
Why
We want to bring over scope challenges as part of porting over our Streamable HTTP support from
mcp.githubcopilot.com/mcp.What changed
MCP impact
Security / limits
This is mostly ported from the existing implementation in our Remote server.
Tool renaming
deprecated_tool_aliases.goNote: if you're renaming tools, you must add the tool aliases. For more information on how to do so, please refer to the official docs.
Lint & tests
./script/lint./script/testDocs