Skip to content

feat(oauth): wire stdio OAuth 2.1 login into the server (2/4)#2710

Draft
SamMorrowDrums wants to merge 2 commits into
sammorrowdrums/oauth-stdio-corefrom
sammorrowdrums/oauth-stdio-wiring
Draft

feat(oauth): wire stdio OAuth 2.1 login into the server (2/4)#2710
SamMorrowDrums wants to merge 2 commits into
sammorrowdrums/oauth-stdio-corefrom
sammorrowdrums/oauth-stdio-wiring

Conversation

@SamMorrowDrums

Copy link
Copy Markdown
Collaborator

Part 2 of 4 — stdio wiring

Stacked on #2704 (PR 1/4, internal/oauth core library). Review and merge #2704 first; this PR's diff is only meaningful on top of it.

What this does

Connects 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. 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 static Token. Lets the lazily-acquired, refreshing OAuth token take effect without rebuilding the client.
  • createGitHubClients — when a TokenProvider is set, authenticates via BearerAuthTransport and skips go-github's WithAuthToken (which would install its own round tripper pinning a static token).
  • RunStdioServer — starts without a token when an OAuthManager is configured, and installs receiving middleware that runs the authorization flow on the first tools/call, before any handler tries to call GitHub with an empty token.
  • sessionPrompter — adapts the MCP server session to oauth.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.
  • Scope-based tool filtering — uses the requested OAuth scopes. The default supported set hides nothing; a narrower --oauth-scopes both narrows the grant and filters tools accordingly.
  • New stdio flags--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 TokenSource absorbs 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.gosessionPrompter exercised against a real in-memory MCP session (capability matrix + accept/decline/cancel × url/form); middleware tested at each branch via a deterministic fake authenticator; createGitHubClients token-provider wiring proven against an httptest server (current token used per request, no stale pin).

The middleware depends on a small oauthAuthenticator interface (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

Replaces #1836.

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>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.TokenProvider and 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

Comment thread pkg/http/transport/bearer.go Outdated
Comment on lines +24 to +28
token := t.Token
if t.TokenProvider != nil {
token = t.TokenProvider()
}
req.Header.Set(headers.AuthorizationHeader, "Bearer "+token)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.)

Comment thread internal/ghmcp/oauth.go
Comment on lines +39 to +45
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(),
})

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread internal/ghmcp/server.go
@@ -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)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 7/7 changed files
  • Comments generated: 6

Comment on lines +52 to +72
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)
Comment on lines +84 to +103
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()
}
Comment on lines +120 to +140
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)
Comment thread internal/ghmcp/oauth.go
Comment on lines +102 to +123
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)")
Comment on lines +312 to +333
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()
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants