-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Add OAuth 2.1 authentication for stdio mode with MCP URL elicitation and performance optimizations #1836
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Add OAuth 2.1 authentication for stdio mode with MCP URL elicitation and performance optimizations #1836
Changes from 12 commits
361bcb8
13f9216
7813b50
f87313f
da260e1
fc957af
7b5c1fe
3278eee
76defa8
5e50dfd
88c632b
39b60d6
70c4553
2bcd1ee
e08df2e
06d8b47
6ea3eec
0a3fc25
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,14 +1,18 @@ | ||
| package main | ||
|
|
||
| import ( | ||
| "errors" | ||
| "context" | ||
| "fmt" | ||
| "os" | ||
| "sort" | ||
| "strings" | ||
| "time" | ||
|
|
||
| "github.com/github/github-mcp-server/internal/ghmcp" | ||
| "github.com/github/github-mcp-server/internal/oauth" | ||
| "github.com/github/github-mcp-server/pkg/github" | ||
| "github.com/github/github-mcp-server/pkg/inventory" | ||
| "github.com/github/github-mcp-server/pkg/translations" | ||
| "github.com/spf13/cobra" | ||
| "github.com/spf13/pflag" | ||
| "github.com/spf13/viper" | ||
|
|
@@ -33,8 +37,33 @@ var ( | |
| Long: `Start a server that communicates via standard input/output streams using JSON-RPC messages.`, | ||
| RunE: func(_ *cobra.Command, _ []string) error { | ||
| token := viper.GetString("personal_access_token") | ||
| var oauthMgr *oauth.Manager | ||
|
|
||
| // If no token provided, setup OAuth manager if configured | ||
| if token == "" { | ||
| return errors.New("GITHUB_PERSONAL_ACCESS_TOKEN not set") | ||
| oauthClientID := viper.GetString("oauth_client_id") | ||
| if oauthClientID != "" { | ||
| // Create OAuth manager for lazy authentication | ||
| oauthCfg := oauth.GetGitHubOAuthConfig( | ||
| oauthClientID, | ||
| viper.GetString("oauth_client_secret"), | ||
| getOAuthScopes(), | ||
| viper.GetString("host"), | ||
| viper.GetInt("oauth_callback_port"), | ||
| ) | ||
| oauthMgr = oauth.NewManager(oauthCfg) | ||
| fmt.Fprintf(os.Stderr, "OAuth configured - will prompt for authentication when needed\n") | ||
| } else { | ||
| fmt.Fprintf(os.Stderr, "Warning: No authentication configured\n") | ||
| fmt.Fprintf(os.Stderr, " - Set GITHUB_PERSONAL_ACCESS_TOKEN, or\n") | ||
| fmt.Fprintf(os.Stderr, " - Configure OAuth with --oauth-client-id\n") | ||
| fmt.Fprintf(os.Stderr, "Tools will prompt for authentication when called\n") | ||
| } | ||
| } | ||
|
|
||
| // Extract token from OAuth manager if available | ||
| if oauthMgr != nil && token == "" { | ||
| token = oauthMgr.GetAccessToken() | ||
| } | ||
|
|
||
| // If you're wondering why we're not using viper.GetStringSlice("toolsets"), | ||
|
|
@@ -73,6 +102,7 @@ var ( | |
| Version: version, | ||
| Host: viper.GetString("host"), | ||
| Token: token, | ||
| OAuthManager: oauthMgr, | ||
| EnabledToolsets: enabledToolsets, | ||
| EnabledTools: enabledTools, | ||
| EnabledFeatures: enabledFeatures, | ||
|
|
@@ -112,6 +142,12 @@ func init() { | |
| rootCmd.PersistentFlags().Bool("insider-mode", false, "Enable insider features") | ||
| rootCmd.PersistentFlags().Duration("repo-access-cache-ttl", 5*time.Minute, "Override the repo access cache TTL (e.g. 1m, 0s to disable)") | ||
|
|
||
| // OAuth flags (stdio mode only) | ||
| rootCmd.PersistentFlags().String("oauth-client-id", "", "GitHub OAuth app client ID (enables interactive OAuth flow if token not set)") | ||
| rootCmd.PersistentFlags().String("oauth-client-secret", "", "GitHub OAuth app client secret (recommended)") | ||
| rootCmd.PersistentFlags().StringSlice("oauth-scopes", nil, "OAuth scopes to request (comma-separated)") | ||
| rootCmd.PersistentFlags().Int("oauth-callback-port", 0, "Fixed port for OAuth callback (0 for random, required for Docker with -p flag)") | ||
|
|
||
| // Bind flag to viper | ||
| _ = viper.BindPFlag("toolsets", rootCmd.PersistentFlags().Lookup("toolsets")) | ||
| _ = viper.BindPFlag("tools", rootCmd.PersistentFlags().Lookup("tools")) | ||
|
|
@@ -126,6 +162,10 @@ func init() { | |
| _ = viper.BindPFlag("lockdown-mode", rootCmd.PersistentFlags().Lookup("lockdown-mode")) | ||
| _ = viper.BindPFlag("insider-mode", rootCmd.PersistentFlags().Lookup("insider-mode")) | ||
| _ = viper.BindPFlag("repo-access-cache-ttl", rootCmd.PersistentFlags().Lookup("repo-access-cache-ttl")) | ||
| _ = viper.BindPFlag("oauth_client_id", rootCmd.PersistentFlags().Lookup("oauth-client-id")) | ||
| _ = viper.BindPFlag("oauth_client_secret", rootCmd.PersistentFlags().Lookup("oauth-client-secret")) | ||
| _ = viper.BindPFlag("oauth_scopes", rootCmd.PersistentFlags().Lookup("oauth-scopes")) | ||
| _ = viper.BindPFlag("oauth_callback_port", rootCmd.PersistentFlags().Lookup("oauth-callback-port")) | ||
|
|
||
| // Add subcommands | ||
| rootCmd.AddCommand(stdioCmd) | ||
|
|
@@ -154,3 +194,113 @@ func wordSepNormalizeFunc(_ *pflag.FlagSet, name string) pflag.NormalizedName { | |
| } | ||
| return pflag.NormalizedName(name) | ||
| } | ||
|
|
||
| // getOAuthScopes returns the OAuth scopes to request based on enabled tools | ||
| // Uses custom scopes if explicitly provided, otherwise computes required scopes | ||
| // from the tools that will be enabled based on user configuration | ||
| func getOAuthScopes() []string { | ||
| // Allow explicit override via --oauth-scopes flag | ||
| var scopes []string | ||
| if viper.IsSet("oauth_scopes") { | ||
| if err := viper.UnmarshalKey("oauth_scopes", &scopes); err == nil && len(scopes) > 0 { | ||
| return scopes | ||
| } | ||
| } | ||
|
|
||
| // Compute required scopes based on enabled tools | ||
| // This ensures we only request scopes for tools the user will actually use | ||
| var enabledToolsets []string | ||
| if viper.IsSet("toolsets") { | ||
| if err := viper.UnmarshalKey("toolsets", &enabledToolsets); err != nil { | ||
| // If unmarshaling fails, fall back to defaults | ||
| enabledToolsets = nil | ||
| } | ||
| } | ||
|
|
||
| var enabledTools []string | ||
| if viper.IsSet("tools") { | ||
| if err := viper.UnmarshalKey("tools", &enabledTools); err != nil { | ||
| enabledTools = nil | ||
| } | ||
| } | ||
|
|
||
| var enabledFeatures []string | ||
| if viper.IsSet("features") { | ||
| if err := viper.UnmarshalKey("features", &enabledFeatures); err != nil { | ||
| enabledFeatures = nil | ||
| } | ||
| } | ||
|
|
||
| // Build inventory with the same configuration that will be used at runtime | ||
| // This allows us to determine which tools will actually be available | ||
| t, _ := translations.TranslationHelper() | ||
| inventoryBuilder := github.NewInventory(t). | ||
| WithReadOnly(viper.GetBool("read-only")). | ||
| WithToolsets(enabledToolsets). | ||
| WithTools(enabledTools). | ||
| WithFeatureChecker(createFeatureChecker(enabledFeatures)) | ||
|
|
||
| inventory, err := inventoryBuilder.Build() | ||
| if err != nil { | ||
| // If inventory build fails, fall back to default scopes | ||
| return getDefaultOAuthScopes() | ||
| } | ||
|
|
||
| // Collect all required scopes from available tools | ||
| requiredScopes := collectRequiredScopes(inventory) | ||
| if len(requiredScopes) == 0 { | ||
| // If no tools require scopes, use defaults | ||
| return getDefaultOAuthScopes() | ||
| } | ||
|
|
||
| return requiredScopes | ||
| } | ||
|
|
||
| // getDefaultOAuthScopes returns the default scopes for GitHub MCP Server | ||
| // Based on the protected resource metadata at https://api.githubcopilot.com/.well-known/oauth-protected-resource/mcp | ||
| func getDefaultOAuthScopes() []string { | ||
| return []string{ | ||
| "repo", | ||
| "user", | ||
| "gist", | ||
| "notifications", | ||
| "read:org", | ||
| "project", | ||
| } | ||
|
Comment on lines
+258
to
+265
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @copilot, this is actually a bit weak. We have both typed scopes, and ability to discern what is required based on tools the user will enable via provided configuration, so we should not hard code but instead possibly determine this between filtering tools and registering them with new server, so the flow is seamless, and minimum required scopes based on selected scopes are automatically requested.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Implemented dynamic scope computation in commit
For example:
This seamlessly integrates with the existing tool filtering system and automatically adapts when tool selection changes. |
||
| } | ||
|
|
||
| // collectRequiredScopes collects all unique required scopes from available tools | ||
| // Returns a sorted, deduplicated list of OAuth scopes needed for the enabled tools | ||
| func collectRequiredScopes(inv *inventory.Inventory) []string { | ||
| scopeSet := make(map[string]bool) | ||
|
|
||
| // Get available tools (respects filters like read-only, toolsets, etc.) | ||
| for _, tool := range inv.AvailableTools(context.Background()) { | ||
| for _, scope := range tool.RequiredScopes { | ||
| if scope != "" { | ||
| scopeSet[scope] = true | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Convert to sorted slice for deterministic output | ||
| scopes := make([]string, 0, len(scopeSet)) | ||
| for scope := range scopeSet { | ||
| scopes = append(scopes, scope) | ||
| } | ||
| sort.Strings(scopes) | ||
|
|
||
| return scopes | ||
| } | ||
|
|
||
| // createFeatureChecker creates a feature flag checker from enabled features list | ||
| func createFeatureChecker(enabledFeatures []string) inventory.FeatureFlagChecker { | ||
| // Build a set for O(1) lookup | ||
| featureSet := make(map[string]bool, len(enabledFeatures)) | ||
| for _, f := range enabledFeatures { | ||
| featureSet[f] = true | ||
| } | ||
| return func(_ context.Context, flagName string) (bool, error) { | ||
| return featureSet[flagName], nil | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Out of curiosity is there a way to avoid the double inventory building by leveraging the inventory built when setting up the server? Considering this now happens after server setup @copilot
Also can we use the PAT scope filter middleware to filter tools by user provided scopes if we have them? The other way around is less important, because we have the scopes generated from the tools themselves, but anyway, that's just gloss. It's a quality of life improvement as stdio cannot challenge scopes and dead tools are a waste of context and can never succeed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Implemented both optimizations in commit
70c4553:Eliminated double inventory building: Modified
getOAuthScopes()to return both scopes and the built inventory. The inventory is now reused inNewMCPServer()instead of building it again, saving ~100ms on startup.Added OAuth scope filtering: When OAuth is configured, the OAuth scopes are now used for tool filtering (similar to PAT scope filtering). This hides tools requiring scopes not available in the OAuth token, improving QoL by removing "dead tools" that can never succeed.
The implementation keeps 90%+ of the logic in the auth module as intended, with minimal changes to server initialization code.