Skip to content

Conversation

@mattdholloway
Copy link
Contributor

@mattdholloway mattdholloway commented Jan 22, 2026

Depends on: https://github.com/github/github-mcp-server-remote/pull/628

Summary

This pull request adds comprehensive support for OAuth 2.0 Protected Resource Metadata (RFC 9728) to the GitHub MCP server in HTTP mode. The main improvements include introducing a new oauth package for serving OAuth resource metadata endpoints, updating server configuration to support a public base URL, and enhancing authentication middleware to comply with OAuth standards. The purpose of these changes is to migrate functionality from the Remote MCP Server to OSS as part of the upcoming HTTP handler changes.

Why

Closes: https://github.com/github/copilot-mcp-core/issues/1206

What changed

  • OAuth Protected Resource Metadata Support
  • Authentication Middleware Enhancements
  • Configuration and Header Support

MCP impact

  • No tool or API changes
  • Tool schema or behavior changed
  • New tool added

Prompts tested (tool changes only)

Security / limits

  • No security or limits impact
  • Auth / permissions considered
  • Data exposure, filtering, or token/size limits considered

Tool renaming

  • I am renaming tools as part of this PR (e.g. a part of a consolidation effort)
    • I have added the new tool aliases in deprecated_tool_aliases.go
  • I am not renaming tools as part of this PR

Note: 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

  • Linted locally with ./script/lint
  • Tested locally with ./script/test

Docs

  • Not needed
  • Updated (README / docs / examples)

@mattdholloway mattdholloway marked this pull request as ready for review January 26, 2026 15:54
@mattdholloway mattdholloway requested a review from a team as a code owner January 26, 2026 15:54
Copilot AI review requested due to automatic review settings January 26, 2026 15:54
Copy link
Contributor

Copilot AI left a comment

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 implements OAuth 2.0 Protected Resource Metadata (RFC 9728) support for the GitHub MCP Server's HTTP mode, migrating functionality from the Remote MCP Server to the open-source repository. The changes enable OAuth-compliant authentication challenges and resource discovery for MCP clients.

Changes:

  • Added new pkg/http/oauth package with OAuth resource metadata endpoint handlers and URL builders
  • Enhanced authentication middleware to send WWW-Authenticate challenges with resource metadata URLs
  • Added forwarding header support (X-Forwarded-Host, X-Forwarded-Proto, X-GitHub-Original-Path) for proxy environments
  • Introduced --base-url CLI flag for configuring the public server URL

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
pkg/http/oauth/oauth.go Core OAuth metadata implementation with route registration, metadata builders, and utility functions for determining effective host/scheme
pkg/http/oauth/oauth_test.go Comprehensive test suite covering OAuth handler initialization, URL building, route registration, and metadata format compliance
pkg/http/server.go Integrates OAuth handler registration into HTTP server initialization
pkg/http/handler.go Adds OAuth config as an optional parameter to HTTPMcpHandler
pkg/http/middleware/token.go Updates token extraction middleware to send OAuth-compliant auth challenges
pkg/http/headers/headers.go Defines new header constants for proxy forwarding and original path preservation
cmd/github-mcp-server/main.go Adds base-url CLI flag and passes it through to HTTP server config
Comments suppressed due to low confidence (3)

pkg/http/oauth/oauth.go:111

  • The buildMetadata function creates metadata at handler registration time with a static resource URL. This means the metadata will always use the BaseURL from the config and won't dynamically adapt to request headers like X-Forwarded-Host. This is a design decision that may be intentional for consistency, but it means that when BaseURL is not configured, the resource field will be empty. Consider whether the metadata should be built dynamically per-request or if BaseURL should be required when using OAuth metadata.
func (h *AuthHandler) buildMetadata(resourcePath string) *oauthex.ProtectedResourceMetadata {
	baseURL := strings.TrimSuffix(h.cfg.BaseURL, "/")
	resourceURL := baseURL
	if resourcePath != "" && resourcePath != "/" {
		resourceURL = baseURL + resourcePath
	}

pkg/http/oauth/oauth.go:24

  • The PR checklist indicates that linting was not run locally with script/lint. Per the coding guidelines, linting is mandatory before committing Go code changes. Please run script/lint to identify and fix any formatting or lint issues before merging.
// Package oauth provides OAuth 2.0 Protected Resource Metadata (RFC 9728) support
// for the GitHub MCP Server HTTP mode.
package oauth

import (
	"fmt"
	"net/http"
	"net/url"
	"strings"

	"github.com/modelcontextprotocol/go-sdk/auth"

	"github.com/github/github-mcp-server/pkg/http/headers"
	"github.com/go-chi/chi/v5"
	"github.com/modelcontextprotocol/go-sdk/oauthex"
)

const (
	// OAuthProtectedResourcePrefix is the well-known path prefix for OAuth protected resource metadata.
	OAuthProtectedResourcePrefix = "/.well-known/oauth-protected-resource"

	// DefaultAuthorizationServer is GitHub's OAuth authorization server.
	DefaultAuthorizationServer = "https://github.com/login/oauth"
)

pkg/http/oauth/oauth.go:54

  • The ResourcePath field in the Config struct is defined but never used. The GetEffectiveResourcePath function doesn't reference cfg.ResourcePath. Either remove this unused field or implement its intended functionality.
	// ResourcePath is the resource path suffix (e.g., "/mcp").
	// If empty, defaults to "/"
	ResourcePath string

mattdholloway and others added 2 commits January 26, 2026 16:35
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@mattdholloway mattdholloway self-assigned this Jan 26, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@mattdholloway mattdholloway force-pushed the oauth-handler-implementation branch from 3daa5c3 to 03a5082 Compare January 28, 2026 14:17
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.

Comment on lines 67 to 73
// sendAuthChallenge sends a 401 Unauthorized response with WWW-Authenticate header
// containing the OAuth protected resource metadata URL as per RFC 6750 and MCP spec.
func sendAuthChallenge(w http.ResponseWriter, r *http.Request, oauthCfg *oauth.Config) {
resourceMetadataURL := oauth.BuildResourceMetadataURL(r, oauthCfg, "mcp")
w.Header().Set("WWW-Authenticate", fmt.Sprintf(`Bearer resource_metadata=%q`, resourceMetadataURL))
http.Error(w, "Unauthorized", http.StatusUnauthorized)
}
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

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

The new function sendAuthChallenge and the modified ExtractUserToken middleware function lack test coverage. The oauth package has comprehensive tests, but the middleware changes that use this package are not tested. This is inconsistent with the codebase's testing practices, as the oauth package itself has 11 test functions covering various scenarios.

Consider adding tests for:

  1. The sendAuthChallenge function to verify the WWW-Authenticate header format
  2. The ExtractUserToken middleware with missing authorization headers to ensure proper 401 responses with OAuth metadata URLs
  3. Different oauth config scenarios (nil config, custom BaseURL, etc.)

Copilot uses AI. Check for mistakes.
Comment on lines 105 to 110
func (h *AuthHandler) buildMetadata(resourcePath string) *oauthex.ProtectedResourceMetadata {
baseURL := strings.TrimSuffix(h.cfg.BaseURL, "/")
resourceURL := baseURL
if resourcePath != "" && resourcePath != "/" {
resourceURL = baseURL + resourcePath
}
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

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

When BaseURL is not configured (empty string), the buildMetadata function will create OAuth metadata with an incomplete or invalid Resource field. For example, if BaseURL is empty and resourcePath is "/mcp", the resulting Resource will be just "/mcp" (a relative URL), which violates RFC 9728's requirement for an absolute URL.

Consider either:

  1. Validating that BaseURL is set when creating the AuthHandler
  2. Documenting that BaseURL is required for proper OAuth metadata
  3. Using a dynamic handler that builds the Resource URL from the incoming request's host (similar to how BuildResourceMetadataURL works)

Copilot uses AI. Check for mistakes.
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.

4 participants