FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fix(http): preserve CORS across OAuth routes by SamMorrowDrums · Pull Request #3147 · github/github-mcp-server · GitHub

/ github-mcp-server Public

fix(http): preserve CORS across OAuth routes #3147

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.

Sign up for GitHub

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

Merged
SamMorrowDrums merged 2 commits into from
Aug 24, 2026
Conversation 2 Commits 2 Checks 19 Findings Files changed
Merged

fix(http): preserve CORS across OAuth routes #3147

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cd2534f
fix(http): preserve CORS across OAuth routes
SamMorrowDrums Aug 24, 2026
53e666d
fix(http): terminate unknown resource metadata routes
SamMorrowDrums Aug 24, 2026
File filter

Filter by extension

Filter by extension .go  (6) .md  (1) All 2 file types selected
Viewed files
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Unified
Split
Hide whitespace
Diff view
Unified
Split
Hide whitespace
13 changes: 13 additions & 0 deletions docs/streamable-http.md
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,19 @@ The OAuth protected resource metadata's `resource` attribute will be populated w

This allows OAuth clients to discover authentication requirements and endpoint information automatically.

The HTTP server is the OAuth protected resource, not the authorization server. It
therefore serves `/.well-known/oauth-protected-resource` but does not serve
`/.well-known/oauth-authorization-server` unless a separately deployed authorization
server is explicitly hosted on the same origin.

Clients discover authorization-server metadata from the issuer listed in
`authorization_servers`. For the default `https://github.com/login/oauth` issuer,
RFC 8414 path insertion produces
`https://github.com/.well-known/oauth-authorization-server/login/oauth`. Browser-based
clients require that authorization server and its discovery endpoints to support
their browser origin through CORS. If the selected authorization server does not,
configure `--authorization-server` to advertise a browser-compatible OAuth proxy.

### Behind a Trusted Proxy (advanced)

By default, the server ignores the `X-Forwarded-Host` and `X-Forwarded-Proto` headers when constructing OAuth resource metadata URLs, so an untrusted client cannot influence the URL advertised to MCP clients. For most deployments, setting `--base-url` to the externally visible URL is the right approach.
Expand Down
2 changes: 1 addition & 1 deletion pkg/http/middleware/cors.go
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ func SetCorsHeaders(h http.Handler) http.Handler {
w.Header().Set("Access-Control-Allow-Origin", "*")
w.Header().Set("Access-Control-Allow-Methods", "GET, POST, DELETE, OPTIONS")
w.Header().Set("Access-Control-Max-Age", "86400")
w.Header().Set("Access-Control-Expose-Headers", "Mcp-Session-Id, WWW-Authenticate")
w.Header().Add("Access-Control-Expose-Headers", "Mcp-Session-Id, WWW-Authenticate")
w.Header().Set("Access-Control-Allow-Headers", allowHeaders)

if r.Method == http.MethodOptions {
Expand Down
22 changes: 19 additions & 3 deletions pkg/http/middleware/cors_test.go
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -3,26 +3,35 @@ package middleware_test
import (
"net/http"
"net/http/httptest"
"strings"
"testing"

"github.com/github/github-mcp-server/pkg/http/middleware"
"github.com/stretchr/testify/assert"
)

func TestSetCorsHeaders(t *testing.T) {
innerCalled := false
inner := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
innerCalled = true
w.Header().Add("Access-Control-Expose-Headers", "X-Existing-Response")
w.WriteHeader(http.StatusOK)
})
handler := middleware.SetCorsHeaders(inner)

t.Run("OPTIONS preflight returns 200 with CORS headers", func(t *testing.T) {
innerCalled = false
req := httptest.NewRequest(http.MethodOptions, "/", nil)
req.Header.Set("Origin", "http://localhost:6274")
req.Header.Set("Origin", "https://confer.to")
req.Header.Set("Access-Control-Request-Method", http.MethodPost)
req.Header.Set("Access-Control-Request-Headers", "content-type")
rr := httptest.NewRecorder()
handler.ServeHTTP(rr, req)

assert.Equal(t, http.StatusOK, rr.Code)
assert.False(t, innerCalled)
assert.Equal(t, "*", rr.Header().Get("Access-Control-Allow-Origin"))
assert.Empty(t, rr.Header().Get("Access-Control-Allow-Credentials"))
assert.Contains(t, rr.Header().Get("Access-Control-Allow-Methods"), "POST")
assert.Contains(t, rr.Header().Get("Access-Control-Allow-Headers"), "Authorization")
assert.Contains(t, rr.Header().Get("Access-Control-Allow-Headers"), "Content-Type")
Expand All @@ -33,13 +42,20 @@ func TestSetCorsHeaders(t *testing.T) {
assert.Contains(t, rr.Header().Get("Access-Control-Expose-Headers"), "WWW-Authenticate")
})

t.Run("POST request includes CORS headers", func(t *testing.T) {
t.Run("POST request includes CORS headers without replacing existing exposed headers", func(t *testing.T) {
innerCalled = false
req := httptest.NewRequest(http.MethodPost, "/", nil)
req.Header.Set("Origin", "http://localhost:6274")
req.Header.Set("Origin", "https://confer.to")
rr := httptest.NewRecorder()
handler.ServeHTTP(rr, req)

assert.Equal(t, http.StatusOK, rr.Code)
assert.True(t, innerCalled)
assert.Equal(t, "*", rr.Header().Get("Access-Control-Allow-Origin"))
assert.Empty(t, rr.Header().Get("Access-Control-Allow-Credentials"))
exposedHeaders := strings.Join(rr.Header().Values("Access-Control-Expose-Headers"), ", ")
assert.Contains(t, exposedHeaders, "Mcp-Session-Id")
assert.Contains(t, exposedHeaders, "WWW-Authenticate")
assert.Contains(t, exposedHeaders, "X-Existing-Response")
})
}
10 changes: 7 additions & 3 deletions pkg/http/oauth/oauth.go
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -82,11 +82,14 @@ func NewAuthHandler(cfg *Config, apiHost utils.APIHostResolver) (*AuthHandler, e

// routePatterns defines the route patterns for OAuth protected resource metadata.
var routePatterns = []string{
"", // Root: /.well-known/oauth-protected-resource
"/readonly", // Read-only mode
"/insiders", // Insiders mode
"", // Root: /.well-known/oauth-protected-resource
"/readonly",
"/insiders",
"/readonly/insiders",
"/x/{toolset}",
"/x/{toolset}/readonly",
"/x/{toolset}/insiders",
"/x/{toolset}/readonly/insiders",
}

// RegisterRoutes registers the OAuth protected resource metadata routes.
Expand All @@ -97,6 +100,7 @@ func (h *AuthHandler) RegisterRoutes(r chi.Router) {
r.Handle(path, h.metadataHandler())
}
}
r.Handle(OAuthProtectedResourcePrefix+"/*", http.NotFoundHandler())
}

func (h *AuthHandler) metadataHandler() http.Handler {
Expand Down
61 changes: 33 additions & 28 deletions pkg/http/oauth/oauth_test.go
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -542,37 +542,42 @@ func TestRegisterRoutes(t *testing.T) {
router := chi.NewRouter()
handler.RegisterRoutes(router)

// List of expected routes that should be registered
expectedRoutes := []string{
OAuthProtectedResourcePrefix,
OAuthProtectedResourcePrefix + "/",
OAuthProtectedResourcePrefix + "/mcp",
OAuthProtectedResourcePrefix + "/mcp/",
OAuthProtectedResourcePrefix + "/readonly",
OAuthProtectedResourcePrefix + "/readonly/",
OAuthProtectedResourcePrefix + "/mcp/readonly",
OAuthProtectedResourcePrefix + "/mcp/readonly/",
OAuthProtectedResourcePrefix + "/x/repos",
OAuthProtectedResourcePrefix + "/mcp/x/repos",
resourcePaths := []string{
"",
"/readonly",
"/insiders",
"/readonly/insiders",
"/x/repos",
"/x/repos/readonly",
"/x/repos/insiders",
"/x/repos/readonly/insiders",
}

for _, route := range expectedRoutes {
t.Run("route:"+route, func(t *testing.T) {
// Test GET
req := httptest.NewRequest(http.MethodGet, route, nil)
req.Host = "api.example.com"
rec := httptest.NewRecorder()
router.ServeHTTP(rec, req)
assert.Equal(t, http.StatusOK, rec.Code, "GET %s should return 200", route)

// Test OPTIONS (CORS preflight)
req = httptest.NewRequest(http.MethodOptions, route, nil)
req.Host = "api.example.com"
rec = httptest.NewRecorder()
router.ServeHTTP(rec, req)
assert.Equal(t, http.StatusNoContent, rec.Code, "OPTIONS %s should return 204", route)
})
for _, basePath := range []string{"", "/mcp"} {
for _, resourcePath := range resourcePaths {
for _, trailingSlash := range []string{"", "/"} {
route := OAuthProtectedResourcePrefix + basePath + resourcePath + trailingSlash
t.Run("route:"+route, func(t *testing.T) {
req := httptest.NewRequest(http.MethodGet, route, nil)
req.Host = "api.example.com"
rec := httptest.NewRecorder()
router.ServeHTTP(rec, req)
assert.Equal(t, http.StatusOK, rec.Code, "GET %s should return 200", route)

req = httptest.NewRequest(http.MethodOptions, route, nil)
req.Host = "api.example.com"
rec = httptest.NewRecorder()
router.ServeHTTP(rec, req)
assert.Equal(t, http.StatusNoContent, rec.Code, "OPTIONS %s should return 204", route)
})
}
}
}

req := httptest.NewRequest(http.MethodGet, OAuthProtectedResourcePrefix+"/mcp/unknown", nil)
rec := httptest.NewRecorder()
router.ServeHTTP(rec, req)
assert.Equal(t, http.StatusNotFound, rec.Code)
}

func TestSupportedScopes(t *testing.T) {
Expand Down
32 changes: 17 additions & 15 deletions pkg/http/server.go
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -204,28 +204,22 @@ func RunHTTPServer(cfg ServerConfig) error {
WithScopeFetcher(scopeFetcher),
}

r := chi.NewRouter()
handler := NewHTTPMcpHandler(ctx, &cfg, deps, t, logger, apiHost, append(serverOptions, WithFeatureChecker(featureChecker), WithOAuthConfig(oauthCfg))...)
oauthHandler, err := oauth.NewAuthHandler(oauthCfg, apiHost)
if err != nil {
return fmt.Errorf("failed to create OAuth handler: %w", err)
}

r.Group(func(r chi.Router) {
r.Use(middleware.SetCorsHeaders)

// Register Middleware First, needs to be before route registration
handler.RegisterMiddleware(r)

// Register MCP server routes
handler.RegisterRoutes(r)
})
r := newHTTPRouter(
func(r chi.Router) {
// Register Middleware First, needs to be before route registration
handler.RegisterMiddleware(r)
// Register MCP server routes
handler.RegisterRoutes(r)
},
oauthHandler.RegisterRoutes,
)
logger.Info("MCP endpoints registered", "baseURL", cfg.BaseURL)

r.Group(func(r chi.Router) {
// Register OAuth protected resource metadata endpoints
oauthHandler.RegisterRoutes(r)
})
logger.Info("OAuth protected resource endpoints registered", "baseURL", cfg.BaseURL)

addr := resolveListenAddress(cfg.ListenHost, cfg.Port)
Expand Down Expand Up @@ -259,6 +253,14 @@ func RunHTTPServer(cfg ServerConfig) error {
return nil
}

func newHTTPRouter(registerMCPRoutes, registerOAuthRoutes func(chi.Router)) chi.Router {
r := chi.NewRouter()
r.Use(middleware.SetCorsHeaders)
r.Group(registerMCPRoutes)
r.Group(registerOAuthRoutes)
return r
}

func newOAuthConfig(cfg ServerConfig) *oauth.Config {
return &oauth.Config{
BaseURL: cfg.BaseURL,
Expand Down
Loading
Loading
Back | FazBrowse Home | New Git URL