feat(oauth): wire stdio OAuth 2.1 login into the server (2/4)#2710
feat(oauth): wire stdio OAuth 2.1 login into the server (2/4)#2710SamMorrowDrums wants to merge 2 commits into
Conversation
Connect the internal/oauth core library to the stdio MCP server so users can authenticate with an OAuth App or GitHub App client ID instead of a static personal access token. - BearerAuthTransport gains a TokenProvider that is consulted per request, letting the lazily-acquired, auto-refreshing OAuth token take effect without rebuilding the client. - createGitHubClients uses BearerAuthTransport (and skips go-github's WithAuthToken, which would pin a static token) when a TokenProvider is set. - RunStdioServer starts without a token and installs receiving middleware that runs the authorization flow on the first tool call, surfacing the auth URL or device code via elicitation (or a tool result as a fallback). - Tool filtering uses the requested OAuth scopes; the default supported set hides nothing, while a narrower --oauth-scopes both narrows the grant and filters tools accordingly. - A sessionPrompter adapts the MCP server session to oauth.Prompter, keeping the authorization URL off the model's context. - New stdio flags: --oauth-client-id/-client-secret/-scopes/-callback-port. This is stdio-only and deliberately does not touch MCP-HTTP auth. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR (2/4 in the OAuth stack) wires the new internal/oauth library into the stdio MCP server so authentication can be performed lazily via OAuth (with refresh) instead of requiring a static PAT up-front.
Changes:
- Add per-request token resolution via
BearerAuthTransport.TokenProviderand propagate it through stdio GitHub client creation. - Add stdio OAuth wiring (OAuth manager + first-tool-call receiving middleware + scope-based tool filtering).
- Add stdio CLI flags/env support for OAuth client ID/secret/scopes/callback port and add focused tests for the new behavior.
Show a summary per file
| File | Description |
|---|---|
| pkg/http/transport/bearer.go | Adds TokenProvider support for per-request bearer token resolution. |
| pkg/http/transport/bearer_test.go | Adds unit tests for static token vs provider precedence, per-request refresh, and header passthrough. |
| pkg/github/server.go | Extends MCPServerConfig with TokenProvider for dynamic auth. |
| internal/ghmcp/server.go | Wires token provider into REST/GraphQL clients; adds stdio OAuth manager config + middleware + OAuth scope filtering. |
| internal/ghmcp/oauth.go | Implements MCP-session→OAuth prompter adapter and the receiving middleware that triggers auth on first tools/call. |
| internal/ghmcp/oauth_test.go | Adds tests for session prompter capability handling and OAuth middleware/client wiring. |
| cmd/github-mcp-server/main.go | Adds stdio CLI flags and constructs OAuth manager/scopes when no PAT is provided. |
Copilot's findings
- Files reviewed: 7/7 changed files
- Comments generated: 3
| token := t.Token | ||
| if t.TokenProvider != nil { | ||
| token = t.TokenProvider() | ||
| } | ||
| req.Header.Set(headers.AuthorizationHeader, "Bearer "+token) |
There was a problem hiding this comment.
Good catch on the cleanliness point. Fixed in 9ab8046: the transport now omits the Authorization header entirely when the token is empty (pre-authorization), so we send a properly unauthenticated request rather than an empty Bearer value. Tests updated to expect no header. (Note: the prior tests did pass — Header.Get trims the trailing space — but omitting the header is clearer and more correct.)
| func (p *sessionPrompter) PromptURL(ctx context.Context, prompt oauth.Prompt) error { | ||
| res, err := p.session.Elicit(ctx, &mcp.ElicitParams{ | ||
| Mode: "url", | ||
| Message: prompt.Message, | ||
| URL: prompt.URL, | ||
| ElicitationID: rand.Text(), | ||
| }) |
There was a problem hiding this comment.
crypto/rand.Text() is a standard-library function added in Go 1.24 (this module targets Go 1.25 — see go.mod), so it compiles and is exercised by the green CI build across macOS/Linux/Windows. It returns a cryptographically random RFC 4648 base32 string, which is exactly what we want for a unique ElicitationID. Leaving as-is.
| @@ -255,22 +283,34 @@ func RunStdioServer(cfg StdioServerConfig) error { | |||
| logger := slog.New(slogHandler) | |||
| logger.Info("starting server", "version", cfg.Version, "host", cfg.Host, "readOnly", cfg.ReadOnly, "lockdownEnabled", cfg.LockdownMode) | |||
|
|
|||
There was a problem hiding this comment.
Agreed — enforced in 9ab8046. RunStdioServer now returns an error up front if both a static Token and an OAuthManager are set, instead of silently using OAuth for auth and the static token for scope filtering. Added TestRunStdioServerRejectsTokenAndOAuth to cover it. (The CLI already only sets one or the other, but RunStdioServer is exported and used as a library, so the guard is worth having.)
…en/oauth - BearerAuthTransport omits the Authorization header entirely when the token is empty (pre-authorization) rather than sending an empty "Bearer " value. - RunStdioServer rejects the ambiguous combination of a static Token and an OAuthManager up front, enforcing the documented mutual exclusivity. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| var gotAuth string | ||
| server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| gotAuth = r.Header.Get(headers.AuthorizationHeader) | ||
| w.WriteHeader(http.StatusOK) | ||
| })) | ||
| defer server.Close() | ||
|
|
||
| rt := &BearerAuthTransport{ | ||
| Transport: http.DefaultTransport, | ||
| Token: tc.token, | ||
| TokenProvider: tc.tokenProvider, | ||
| } | ||
|
|
||
| req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, server.URL, nil) | ||
| require.NoError(t, err) | ||
|
|
||
| resp, err := rt.RoundTrip(req) | ||
| require.NoError(t, err) | ||
| defer resp.Body.Close() | ||
|
|
||
| assert.Equal(t, tc.wantAuth, gotAuth) |
| var gotAuth string | ||
| server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| gotAuth = r.Header.Get(headers.AuthorizationHeader) | ||
| w.WriteHeader(http.StatusOK) | ||
| })) | ||
| defer server.Close() | ||
|
|
||
| current := "" | ||
| rt := &BearerAuthTransport{ | ||
| Transport: http.DefaultTransport, | ||
| TokenProvider: func() string { return current }, | ||
| } | ||
|
|
||
| do := func() { | ||
| req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, server.URL, nil) | ||
| require.NoError(t, err) | ||
| resp, err := rt.RoundTrip(req) | ||
| require.NoError(t, err) | ||
| defer resp.Body.Close() | ||
| } |
| var gotFeatures string | ||
| server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| gotFeatures = r.Header.Get(headers.GraphQLFeaturesHeader) | ||
| w.WriteHeader(http.StatusOK) | ||
| })) | ||
| defer server.Close() | ||
|
|
||
| rt := &BearerAuthTransport{ | ||
| Transport: http.DefaultTransport, | ||
| Token: "token", | ||
| } | ||
|
|
||
| ctx := ghcontext.WithGraphQLFeatures(context.Background(), "feature1", "feature2") | ||
| req, err := http.NewRequestWithContext(ctx, http.MethodPost, server.URL, nil) | ||
| require.NoError(t, err) | ||
|
|
||
| resp, err := rt.RoundTrip(req) | ||
| require.NoError(t, err) | ||
| defer resp.Body.Close() | ||
|
|
||
| assert.Equal(t, "feature1, feature2", gotFeatures) |
| return func(ctx context.Context, method string, request mcp.Request) (mcp.Result, error) { | ||
| if method != "tools/call" || mgr.HasToken() { | ||
| return next(ctx, method, request) | ||
| } | ||
|
|
||
| callReq, ok := request.(*mcp.CallToolRequest) | ||
| if !ok { | ||
| return next(ctx, method, request) | ||
| } | ||
|
|
||
| outcome, err := mgr.Authenticate(ctx, &sessionPrompter{session: callReq.Session}) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("github authorization failed: %w", err) | ||
| } | ||
| if outcome != nil && outcome.UserAction != nil { | ||
| logger.Info("surfacing github authorization instructions to user") | ||
| return &mcp.CallToolResult{ | ||
| Content: []mcp.Content{&mcp.TextContent{Text: outcome.UserAction.Message}}, | ||
| }, nil | ||
| } | ||
| return next(ctx, method, request) | ||
| } |
| // to log in via the browser-based OAuth flow on first use. Works for both | ||
| // OAuth Apps and GitHub Apps. | ||
| stdioCmd.Flags().String("oauth-client-id", "", "OAuth App or GitHub App client ID, enabling interactive OAuth login when no token is set") | ||
| stdioCmd.Flags().String("oauth-client-secret", "", "OAuth client secret, if the app requires one (it is a public, non-confidential credential for distributed clients)") |
| var gotAuth string | ||
| server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| gotAuth = r.Header.Get(headers.AuthorizationHeader) | ||
| w.WriteHeader(http.StatusOK) | ||
| })) | ||
| defer server.Close() | ||
|
|
||
| current := "" | ||
| apiHost, err := utils.NewAPIHost(server.URL) | ||
| require.NoError(t, err) | ||
|
|
||
| clients, err := createGitHubClients(github.MCPServerConfig{ | ||
| Version: "test", | ||
| TokenProvider: func() string { return current }, | ||
| }, apiHost) | ||
| require.NoError(t, err) | ||
|
|
||
| do := func() { | ||
| resp, err := clients.rest.Client().Get(server.URL) | ||
| require.NoError(t, err) | ||
| defer resp.Body.Close() | ||
| } |
Part 2 of 4 — stdio wiring
Stacked on #2704 (PR 1/4,
internal/oauthcore library). Review and merge #2704 first; this PR's diff is only meaningful on top of it.What this does
Connects the
internal/oauthcore library to the stdio MCP server so users can authenticate with an OAuth App or GitHub App client ID instead of a static personal access token. The token is acquired lazily on the first tool call and auto-refreshes for the rest of the session.This is stdio-only and deliberately does not touch MCP-HTTP auth.
Changes
BearerAuthTransport.TokenProvider— consulted per request, taking precedence over the staticToken. Lets the lazily-acquired, refreshing OAuth token take effect without rebuilding the client.createGitHubClients— when aTokenProvideris set, authenticates viaBearerAuthTransportand skips go-github'sWithAuthToken(which would install its own round tripper pinning a static token).RunStdioServer— starts without a token when anOAuthManageris configured, and installs receiving middleware that runs the authorization flow on the firsttools/call, before any handler tries to call GitHub with an empty token.sessionPrompter— adapts the MCP server session tooauth.Prompter, surfacing the auth URL / device code via elicitation (URL → form), keeping the authorization URL out of the model's context. Falls back to a tool-result message when elicitation is unavailable.--oauth-scopesboth narrows the grant and filters tools accordingly.--oauth-client-id,--oauth-client-secret,--oauth-scopes,--oauth-callback-port(env:GITHUB_OAUTH_*).Works for both OAuth Apps and GitHub Apps (same endpoints/params; the refreshing
TokenSourceabsorbs GitHub App user-token expiry).Testing
pkg/http/transport/bearer_test.go— static token, provider precedence, per-request resolution (refresh), GraphQL-Features passthrough, no original-request mutation.internal/ghmcp/oauth_test.go—sessionPrompterexercised against a real in-memory MCP session (capability matrix + accept/decline/cancel × url/form); middleware tested at each branch via a deterministic fake authenticator;createGitHubClientstoken-provider wiring proven against an httptest server (current token used per request, no stale pin).The middleware depends on a small
oauthAuthenticatorinterface (2 real impls:*oauth.Manager+ test fake) so it can be exercised without standing up live GitHub flows — a genuine seam, not faux-DRY.Stack
internal/oauthcore libraryReplaces #1836.