From d0b1986a02d0f35d812a04716a8032b528ecfac4 Mon Sep 17 00:00:00 2001 From: Adam Holt Date: Mon, 23 Feb 2026 17:30:31 +0100 Subject: [PATCH 1/8] Generate authorization server urls during API resolution. Then we can pass this into the OAuth implementation. --- pkg/http/oauth/oauth.go | 14 +++-- pkg/http/oauth/oauth_test.go | 107 +++++++++++++++++++++++++++++++---- pkg/http/server.go | 2 +- pkg/scopes/fetcher_test.go | 3 + pkg/utils/api.go | 68 ++++++++++++++++------ 5 files changed, 160 insertions(+), 34 deletions(-) diff --git a/pkg/http/oauth/oauth.go b/pkg/http/oauth/oauth.go index 5da253566..3b325f3c0 100644 --- a/pkg/http/oauth/oauth.go +++ b/pkg/http/oauth/oauth.go @@ -3,11 +3,13 @@ package oauth import ( + "context" "fmt" "net/http" "strings" "github.com/github/github-mcp-server/pkg/http/headers" + "github.com/github/github-mcp-server/pkg/utils" "github.com/go-chi/chi/v5" "github.com/modelcontextprotocol/go-sdk/auth" "github.com/modelcontextprotocol/go-sdk/oauthex" @@ -16,9 +18,6 @@ import ( 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" ) // SupportedScopes lists all OAuth scopes that may be required by MCP tools. @@ -59,14 +58,19 @@ type AuthHandler struct { } // NewAuthHandler creates a new OAuth auth handler. -func NewAuthHandler(cfg *Config) (*AuthHandler, error) { +func NewAuthHandler(ctx context.Context, cfg *Config, apiHost utils.APIHostResolver) (*AuthHandler, error) { if cfg == nil { cfg = &Config{} } // Default authorization server to GitHub if cfg.AuthorizationServer == "" { - cfg.AuthorizationServer = DefaultAuthorizationServer + url, err := apiHost.AuthorizationServerURL(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get authorization server URL from API host: %w", err) + } + + cfg.AuthorizationServer = url.String() } return &AuthHandler{ diff --git a/pkg/http/oauth/oauth_test.go b/pkg/http/oauth/oauth_test.go index 9133e8331..d3acc31bc 100644 --- a/pkg/http/oauth/oauth_test.go +++ b/pkg/http/oauth/oauth_test.go @@ -8,14 +8,22 @@ import ( "testing" "github.com/github/github-mcp-server/pkg/http/headers" + "github.com/github/github-mcp-server/pkg/utils" "github.com/go-chi/chi/v5" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) +var ( + defaultAuthorizationServer = "https://github.com/login/oauth" +) + func TestNewAuthHandler(t *testing.T) { t.Parallel() + dotcomHost, err := utils.NewAPIHost("https://api.github.com") + require.NoError(t, err) + tests := []struct { name string cfg *Config @@ -25,13 +33,13 @@ func TestNewAuthHandler(t *testing.T) { { name: "nil config uses defaults", cfg: nil, - expectedAuthServer: DefaultAuthorizationServer, + expectedAuthServer: defaultAuthorizationServer, expectedResourcePath: "", }, { name: "empty config uses defaults", cfg: &Config{}, - expectedAuthServer: DefaultAuthorizationServer, + expectedAuthServer: defaultAuthorizationServer, expectedResourcePath: "", }, { @@ -48,7 +56,7 @@ func TestNewAuthHandler(t *testing.T) { BaseURL: "https://example.com", ResourcePath: "/mcp", }, - expectedAuthServer: DefaultAuthorizationServer, + expectedAuthServer: defaultAuthorizationServer, expectedResourcePath: "/mcp", }, } @@ -57,11 +65,12 @@ func TestNewAuthHandler(t *testing.T) { t.Run(tc.name, func(t *testing.T) { t.Parallel() - handler, err := NewAuthHandler(tc.cfg) + handler, err := NewAuthHandler(t.Context(), tc.cfg, dotcomHost) require.NoError(t, err) require.NotNil(t, handler) assert.Equal(t, tc.expectedAuthServer, handler.cfg.AuthorizationServer) + assert.Equal(t, tc.expectedResourcePath, handler.cfg.ResourcePath) }) } } @@ -372,7 +381,7 @@ func TestHandleProtectedResource(t *testing.T) { authServers, ok := body["authorization_servers"].([]any) require.True(t, ok) require.Len(t, authServers, 1) - assert.Equal(t, DefaultAuthorizationServer, authServers[0]) + assert.Equal(t, defaultAuthorizationServer, authServers[0]) }, }, { @@ -451,7 +460,10 @@ func TestHandleProtectedResource(t *testing.T) { t.Run(tc.name, func(t *testing.T) { t.Parallel() - handler, err := NewAuthHandler(tc.cfg) + dotcomHost, err := utils.NewAPIHost("https://api.github.com") + require.NoError(t, err) + + handler, err := NewAuthHandler(t.Context(), tc.cfg, dotcomHost) require.NoError(t, err) router := chi.NewRouter() @@ -493,9 +505,12 @@ func TestHandleProtectedResource(t *testing.T) { func TestRegisterRoutes(t *testing.T) { t.Parallel() - handler, err := NewAuthHandler(&Config{ + dotcomHost, err := utils.NewAPIHost("https://api.github.com") + require.NoError(t, err) + + handler, err := NewAuthHandler(t.Context(), &Config{ BaseURL: "https://api.example.com", - }) + }, dotcomHost) require.NoError(t, err) router := chi.NewRouter() @@ -559,9 +574,12 @@ func TestSupportedScopes(t *testing.T) { func TestProtectedResourceResponseFormat(t *testing.T) { t.Parallel() - handler, err := NewAuthHandler(&Config{ + dotcomHost, err := utils.NewAPIHost("https://api.github.com") + require.NoError(t, err) + + handler, err := NewAuthHandler(t.Context(), &Config{ BaseURL: "https://api.example.com", - }) + }, dotcomHost) require.NoError(t, err) router := chi.NewRouter() @@ -598,7 +616,7 @@ func TestProtectedResourceResponseFormat(t *testing.T) { authServers, ok := response["authorization_servers"].([]any) require.True(t, ok) assert.Len(t, authServers, 1) - assert.Equal(t, DefaultAuthorizationServer, authServers[0]) + assert.Equal(t, defaultAuthorizationServer, authServers[0]) } func TestOAuthProtectedResourcePrefix(t *testing.T) { @@ -611,5 +629,70 @@ func TestOAuthProtectedResourcePrefix(t *testing.T) { func TestDefaultAuthorizationServer(t *testing.T) { t.Parallel() - assert.Equal(t, "https://github.com/login/oauth", DefaultAuthorizationServer) + assert.Equal(t, "https://github.com/login/oauth", defaultAuthorizationServer) +} + +func TestAPIHostResolver_AuthorizationServerURL(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + host string + expectedURL string + expectError bool + errorContains string + }{ + { + name: "valid host returns authorization server URL", + host: "http://api.github.com", + expectedURL: "https://github.com/login/oauth", + expectError: false, + }, + { + name: "invalid host returns error", + host: "://invalid-url", + expectedURL: "", + expectError: true, + errorContains: "could not parse host as URL", + }, + { + name: "host without scheme returns error", + host: "api.github.com", + expectedURL: "", + expectError: true, + errorContains: "host must have a scheme", + }, + { + name: "GHES host returns correct authorization server URL with subdomain isolation", + host: "https://api.ghe.example.com", + expectedURL: "https://ghe.example.com/login/oauth", + expectError: false, + }, + { + name: "GHES host returns correct authorization server URL without subdomain isolation", + host: "https://ghe-nosubdomain.example.com/api/v3", + expectedURL: "https://ghe-nosubdomain.example.com/login/oauth", + expectError: false, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + apiHost, err := utils.NewAPIHost(tc.host) + if tc.expectError { + require.Error(t, err) + if tc.errorContains != "" { + assert.Contains(t, err.Error(), tc.errorContains) + } + return + } + require.NoError(t, err) + + url, err := apiHost.AuthorizationServerURL(t.Context()) + require.NoError(t, err) + assert.Equal(t, tc.expectedURL, url.String()) + }) + } } diff --git a/pkg/http/server.go b/pkg/http/server.go index 7397e54a8..aa5f6040f 100644 --- a/pkg/http/server.go +++ b/pkg/http/server.go @@ -136,7 +136,7 @@ func RunHTTPServer(cfg ServerConfig) error { r := chi.NewRouter() handler := NewHTTPMcpHandler(ctx, &cfg, deps, t, logger, apiHost, append(serverOptions, WithFeatureChecker(featureChecker), WithOAuthConfig(oauthCfg))...) - oauthHandler, err := oauth.NewAuthHandler(oauthCfg) + oauthHandler, err := oauth.NewAuthHandler(ctx, oauthCfg, apiHost) if err != nil { return fmt.Errorf("failed to create OAuth handler: %w", err) } diff --git a/pkg/scopes/fetcher_test.go b/pkg/scopes/fetcher_test.go index 2d887d7a8..7ef910a56 100644 --- a/pkg/scopes/fetcher_test.go +++ b/pkg/scopes/fetcher_test.go @@ -28,6 +28,9 @@ func (t testAPIHostResolver) UploadURL(_ context.Context) (*url.URL, error) { func (t testAPIHostResolver) RawURL(_ context.Context) (*url.URL, error) { return nil, nil } +func (t testAPIHostResolver) AuthorizationServerURL(_ context.Context) (*url.URL, error) { + return nil, nil +} func TestParseScopeHeader(t *testing.T) { tests := []struct { diff --git a/pkg/utils/api.go b/pkg/utils/api.go index a523917de..5e173a908 100644 --- a/pkg/utils/api.go +++ b/pkg/utils/api.go @@ -14,13 +14,15 @@ type APIHostResolver interface { GraphqlURL(ctx context.Context) (*url.URL, error) UploadURL(ctx context.Context) (*url.URL, error) RawURL(ctx context.Context) (*url.URL, error) + AuthorizationServerURL(ctx context.Context) (*url.URL, error) } type APIHost struct { - restURL *url.URL - gqlURL *url.URL - uploadURL *url.URL - rawURL *url.URL + restURL *url.URL + gqlURL *url.URL + uploadURL *url.URL + rawURL *url.URL + authorizationServerURL *url.URL } var _ APIHostResolver = APIHost{} @@ -52,6 +54,10 @@ func (a APIHost) RawURL(_ context.Context) (*url.URL, error) { return a.rawURL, nil } +func (a APIHost) AuthorizationServerURL(_ context.Context) (*url.URL, error) { + return a.authorizationServerURL, nil +} + func newDotcomHost() (APIHost, error) { baseRestURL, err := url.Parse("https://api.github.com/") if err != nil { @@ -73,11 +79,18 @@ func newDotcomHost() (APIHost, error) { return APIHost{}, fmt.Errorf("failed to parse dotcom Raw URL: %w", err) } + // The authorization server for GitHub.com is at github.com/login/oauth, not api.github.com + authorizationServerURL, err := url.Parse("https://github.com/login/oauth") + if err != nil { + return APIHost{}, fmt.Errorf("failed to parse dotcom Authorization Server URL: %w", err) + } + return APIHost{ - restURL: baseRestURL, - gqlURL: gqlURL, - uploadURL: uploadURL, - rawURL: rawURL, + restURL: baseRestURL, + gqlURL: gqlURL, + uploadURL: uploadURL, + rawURL: rawURL, + authorizationServerURL: authorizationServerURL, }, nil } @@ -112,11 +125,19 @@ func newGHECHost(hostname string) (APIHost, error) { return APIHost{}, fmt.Errorf("failed to parse GHEC Raw URL: %w", err) } + // The authorization server for GHEC is still on the root domain, not the api subdomain + rootHost := strings.TrimPrefix(u.Hostname(), "api.") + authorizationServerURL, err := url.Parse(fmt.Sprintf("https://%s/login/oauth", rootHost)) + if err != nil { + return APIHost{}, fmt.Errorf("failed to parse GHEC Authorization Server URL: %w", err) + } + return APIHost{ - restURL: restURL, - gqlURL: gqlURL, - uploadURL: uploadURL, - rawURL: rawURL, + restURL: restURL, + gqlURL: gqlURL, + uploadURL: uploadURL, + rawURL: rawURL, + authorizationServerURL: authorizationServerURL, }, nil } @@ -164,11 +185,26 @@ func newGHESHost(hostname string) (APIHost, error) { return APIHost{}, fmt.Errorf("failed to parse GHES Raw URL: %w", err) } + // If subdomain isolation is enabled, the hostname will be api.hostname, but the authorization server is still on the root domain at hostname/login/oauth + // If subdomain isolation is not enabled, the hostname is still hostname and the authorization server is at hostname/login/oauth + var rootHost string + if hasSubdomainIsolation { + rootHost = strings.TrimPrefix(u.Hostname(), "api.") + } else { + rootHost = u.Hostname() + } + authorizationServerURL, err := url.Parse(fmt.Sprintf("%s://%s/login/oauth", u.Scheme, rootHost)) + + if err != nil { + return APIHost{}, fmt.Errorf("failed to parse GHES Authorization Server URL: %w", err) + } + return APIHost{ - restURL: restURL, - gqlURL: gqlURL, - uploadURL: uploadURL, - rawURL: rawURL, + restURL: restURL, + gqlURL: gqlURL, + uploadURL: uploadURL, + rawURL: rawURL, + authorizationServerURL: authorizationServerURL, }, nil } From 03b0a84959c86d7441eb60766bc95a6a4da37114 Mon Sep 17 00:00:00 2001 From: Adam Holt Date: Mon, 23 Feb 2026 17:42:22 +0100 Subject: [PATCH 2/8] cfg.Host should be the non-api prefixed host --- pkg/http/oauth/oauth_test.go | 8 ++++---- pkg/utils/api.go | 15 ++------------- 2 files changed, 6 insertions(+), 17 deletions(-) diff --git a/pkg/http/oauth/oauth_test.go b/pkg/http/oauth/oauth_test.go index d3acc31bc..a19de7a7a 100644 --- a/pkg/http/oauth/oauth_test.go +++ b/pkg/http/oauth/oauth_test.go @@ -644,7 +644,7 @@ func TestAPIHostResolver_AuthorizationServerURL(t *testing.T) { }{ { name: "valid host returns authorization server URL", - host: "http://api.github.com", + host: "http://github.com", expectedURL: "https://github.com/login/oauth", expectError: false, }, @@ -657,20 +657,20 @@ func TestAPIHostResolver_AuthorizationServerURL(t *testing.T) { }, { name: "host without scheme returns error", - host: "api.github.com", + host: "github.com", expectedURL: "", expectError: true, errorContains: "host must have a scheme", }, { name: "GHES host returns correct authorization server URL with subdomain isolation", - host: "https://api.ghe.example.com", + host: "https://ghe.example.com", expectedURL: "https://ghe.example.com/login/oauth", expectError: false, }, { name: "GHES host returns correct authorization server URL without subdomain isolation", - host: "https://ghe-nosubdomain.example.com/api/v3", + host: "https://ghe-nosubdomain.example.com", expectedURL: "https://ghe-nosubdomain.example.com/login/oauth", expectError: false, }, diff --git a/pkg/utils/api.go b/pkg/utils/api.go index 5e173a908..a22711b23 100644 --- a/pkg/utils/api.go +++ b/pkg/utils/api.go @@ -125,9 +125,7 @@ func newGHECHost(hostname string) (APIHost, error) { return APIHost{}, fmt.Errorf("failed to parse GHEC Raw URL: %w", err) } - // The authorization server for GHEC is still on the root domain, not the api subdomain - rootHost := strings.TrimPrefix(u.Hostname(), "api.") - authorizationServerURL, err := url.Parse(fmt.Sprintf("https://%s/login/oauth", rootHost)) + authorizationServerURL, err := url.Parse(fmt.Sprintf("https://%s/login/oauth", u.Hostname())) if err != nil { return APIHost{}, fmt.Errorf("failed to parse GHEC Authorization Server URL: %w", err) } @@ -185,16 +183,7 @@ func newGHESHost(hostname string) (APIHost, error) { return APIHost{}, fmt.Errorf("failed to parse GHES Raw URL: %w", err) } - // If subdomain isolation is enabled, the hostname will be api.hostname, but the authorization server is still on the root domain at hostname/login/oauth - // If subdomain isolation is not enabled, the hostname is still hostname and the authorization server is at hostname/login/oauth - var rootHost string - if hasSubdomainIsolation { - rootHost = strings.TrimPrefix(u.Hostname(), "api.") - } else { - rootHost = u.Hostname() - } - authorizationServerURL, err := url.Parse(fmt.Sprintf("%s://%s/login/oauth", u.Scheme, rootHost)) - + authorizationServerURL, err := url.Parse(fmt.Sprintf("%s://%s/login/oauth", u.Scheme, u.Hostname())) if err != nil { return APIHost{}, fmt.Errorf("failed to parse GHES Authorization Server URL: %w", err) } From b81964bc5be34d036e99105fbca519923a9c8416 Mon Sep 17 00:00:00 2001 From: Adam Holt Date: Mon, 23 Feb 2026 17:47:19 +0100 Subject: [PATCH 3/8] Add GHEC test --- pkg/http/oauth/oauth_test.go | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/pkg/http/oauth/oauth_test.go b/pkg/http/oauth/oauth_test.go index a19de7a7a..8e2691c43 100644 --- a/pkg/http/oauth/oauth_test.go +++ b/pkg/http/oauth/oauth_test.go @@ -663,15 +663,14 @@ func TestAPIHostResolver_AuthorizationServerURL(t *testing.T) { errorContains: "host must have a scheme", }, { - name: "GHES host returns correct authorization server URL with subdomain isolation", - host: "https://ghe.example.com", - expectedURL: "https://ghe.example.com/login/oauth", - expectError: false, + name: "GHEC host returns correct authorization server URL", + host: "https://test.ghe.com", + expectedURL: "https://test.ghe.com/login/oauth", }, { - name: "GHES host returns correct authorization server URL without subdomain isolation", - host: "https://ghe-nosubdomain.example.com", - expectedURL: "https://ghe-nosubdomain.example.com/login/oauth", + name: "GHES host returns correct authorization server URL", + host: "https://ghe.example.com", + expectedURL: "https://ghe.example.com/login/oauth", expectError: false, }, } From 4989da610ff348ced69fe340367f059d7fff21f1 Mon Sep 17 00:00:00 2001 From: Adam Holt Date: Mon, 23 Feb 2026 18:15:26 +0100 Subject: [PATCH 4/8] Resolve the URL at request time. If we do this at start time, we won't know which GHEC tenant we're on. --- pkg/http/oauth/oauth.go | 31 +++++++------ pkg/http/oauth/oauth_test.go | 90 ++++++++++++++++++++++-------------- 2 files changed, 74 insertions(+), 47 deletions(-) diff --git a/pkg/http/oauth/oauth.go b/pkg/http/oauth/oauth.go index 3b325f3c0..5900251cf 100644 --- a/pkg/http/oauth/oauth.go +++ b/pkg/http/oauth/oauth.go @@ -54,7 +54,8 @@ type Config struct { // AuthHandler handles OAuth-related HTTP endpoints. type AuthHandler struct { - cfg *Config + cfg *Config + apiHost utils.APIHostResolver } // NewAuthHandler creates a new OAuth auth handler. @@ -63,18 +64,9 @@ func NewAuthHandler(ctx context.Context, cfg *Config, apiHost utils.APIHostResol cfg = &Config{} } - // Default authorization server to GitHub - if cfg.AuthorizationServer == "" { - url, err := apiHost.AuthorizationServerURL(ctx) - if err != nil { - return nil, fmt.Errorf("failed to get authorization server URL from API host: %w", err) - } - - cfg.AuthorizationServer = url.String() - } - return &AuthHandler{ - cfg: cfg, + cfg: cfg, + apiHost: apiHost, }, nil } @@ -99,15 +91,28 @@ func (h *AuthHandler) RegisterRoutes(r chi.Router) { func (h *AuthHandler) metadataHandler() http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + ctx := r.Context() resourcePath := resolveResourcePath( strings.TrimPrefix(r.URL.Path, OAuthProtectedResourcePrefix), h.cfg.ResourcePath, ) resourceURL := h.buildResourceURL(r, resourcePath) + var authorizationServerURL string + if h.cfg.AuthorizationServer != "" { + authorizationServerURL = h.cfg.AuthorizationServer + } else { + authURL, err := h.apiHost.AuthorizationServerURL(ctx) + if err != nil { + http.Error(w, fmt.Sprintf("failed to resolve authorization server URL: %v", err), http.StatusInternalServerError) + return + } + authorizationServerURL = authURL.String() + } + metadata := &oauthex.ProtectedResourceMetadata{ Resource: resourceURL, - AuthorizationServers: []string{h.cfg.AuthorizationServer}, + AuthorizationServers: []string{authorizationServerURL}, ResourceName: "GitHub MCP Server", ScopesSupported: SupportedScopes, BearerMethodsSupported: []string{"header"}, diff --git a/pkg/http/oauth/oauth_test.go b/pkg/http/oauth/oauth_test.go index 8e2691c43..673824920 100644 --- a/pkg/http/oauth/oauth_test.go +++ b/pkg/http/oauth/oauth_test.go @@ -30,18 +30,6 @@ func TestNewAuthHandler(t *testing.T) { expectedAuthServer string expectedResourcePath string }{ - { - name: "nil config uses defaults", - cfg: nil, - expectedAuthServer: defaultAuthorizationServer, - expectedResourcePath: "", - }, - { - name: "empty config uses defaults", - cfg: &Config{}, - expectedAuthServer: defaultAuthorizationServer, - expectedResourcePath: "", - }, { name: "custom authorization server", cfg: &Config{ @@ -56,7 +44,7 @@ func TestNewAuthHandler(t *testing.T) { BaseURL: "https://example.com", ResourcePath: "/mcp", }, - expectedAuthServer: defaultAuthorizationServer, + expectedAuthServer: "", expectedResourcePath: "/mcp", }, } @@ -636,42 +624,44 @@ func TestAPIHostResolver_AuthorizationServerURL(t *testing.T) { t.Parallel() tests := []struct { - name string - host string - expectedURL string - expectError bool - errorContains string + name string + host string + expectedURL string + expectedError bool + expectedStatusCode int + errorContains string }{ { - name: "valid host returns authorization server URL", - host: "http://github.com", - expectedURL: "https://github.com/login/oauth", - expectError: false, + name: "valid host returns authorization server URL", + host: "http://github.com", + expectedURL: "https://github.com/login/oauth", + expectedStatusCode: http.StatusOK, }, { name: "invalid host returns error", host: "://invalid-url", expectedURL: "", - expectError: true, + expectedError: true, errorContains: "could not parse host as URL", }, { name: "host without scheme returns error", host: "github.com", expectedURL: "", - expectError: true, + expectedError: true, errorContains: "host must have a scheme", }, { - name: "GHEC host returns correct authorization server URL", - host: "https://test.ghe.com", - expectedURL: "https://test.ghe.com/login/oauth", + name: "GHEC host returns correct authorization server URL", + host: "https://test.ghe.com", + expectedURL: "https://test.ghe.com/login/oauth", + expectedStatusCode: http.StatusOK, }, { - name: "GHES host returns correct authorization server URL", - host: "https://ghe.example.com", - expectedURL: "https://ghe.example.com/login/oauth", - expectError: false, + name: "GHES host returns correct authorization server URL", + host: "https://ghe.example.com", + expectedURL: "https://ghe.example.com/login/oauth", + expectedStatusCode: http.StatusOK, }, } @@ -680,18 +670,50 @@ func TestAPIHostResolver_AuthorizationServerURL(t *testing.T) { t.Parallel() apiHost, err := utils.NewAPIHost(tc.host) - if tc.expectError { + if tc.expectedError { require.Error(t, err) if tc.errorContains != "" { assert.Contains(t, err.Error(), tc.errorContains) } return + } else { + require.NoError(t, err) } + + handler, err := NewAuthHandler(t.Context(), &Config{ + BaseURL: "https://api.example.com", + }, apiHost) require.NoError(t, err) - url, err := apiHost.AuthorizationServerURL(t.Context()) + router := chi.NewRouter() + handler.RegisterRoutes(router) + + req := httptest.NewRequest(http.MethodGet, OAuthProtectedResourcePrefix, nil) + req.Host = "api.example.com" + + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + + require.Equal(t, http.StatusOK, rec.Code) + + var response map[string]any + err = json.Unmarshal(rec.Body.Bytes(), &response) require.NoError(t, err) - assert.Equal(t, tc.expectedURL, url.String()) + + assert.Contains(t, response, "authorization_servers") + if tc.expectedStatusCode != http.StatusOK { + require.Equal(t, tc.expectedStatusCode, rec.Code) + if tc.errorContains != "" { + assert.Contains(t, rec.Body.String(), tc.errorContains) + } + return + } + require.NoError(t, err) + + responseAuthServers, ok := response["authorization_servers"].([]any) + require.True(t, ok) + require.Len(t, responseAuthServers, 1) + assert.Equal(t, tc.expectedURL, responseAuthServers[0]) }) } } From f5a4da1bbdbd5cfb9516a2ef20cf38abfc8fff8a Mon Sep 17 00:00:00 2001 From: Adam Holt Date: Mon, 23 Feb 2026 18:20:55 +0100 Subject: [PATCH 5/8] Linter fixes and ctx no longer needed --- pkg/http/oauth/oauth.go | 3 +-- pkg/http/oauth/oauth_test.go | 14 ++++++-------- pkg/http/server.go | 2 +- 3 files changed, 8 insertions(+), 11 deletions(-) diff --git a/pkg/http/oauth/oauth.go b/pkg/http/oauth/oauth.go index 5900251cf..807029cfd 100644 --- a/pkg/http/oauth/oauth.go +++ b/pkg/http/oauth/oauth.go @@ -3,7 +3,6 @@ package oauth import ( - "context" "fmt" "net/http" "strings" @@ -59,7 +58,7 @@ type AuthHandler struct { } // NewAuthHandler creates a new OAuth auth handler. -func NewAuthHandler(ctx context.Context, cfg *Config, apiHost utils.APIHostResolver) (*AuthHandler, error) { +func NewAuthHandler(cfg *Config, apiHost utils.APIHostResolver) (*AuthHandler, error) { if cfg == nil { cfg = &Config{} } diff --git a/pkg/http/oauth/oauth_test.go b/pkg/http/oauth/oauth_test.go index 673824920..d8da6916d 100644 --- a/pkg/http/oauth/oauth_test.go +++ b/pkg/http/oauth/oauth_test.go @@ -53,7 +53,7 @@ func TestNewAuthHandler(t *testing.T) { t.Run(tc.name, func(t *testing.T) { t.Parallel() - handler, err := NewAuthHandler(t.Context(), tc.cfg, dotcomHost) + handler, err := NewAuthHandler(tc.cfg, dotcomHost) require.NoError(t, err) require.NotNil(t, handler) @@ -451,7 +451,7 @@ func TestHandleProtectedResource(t *testing.T) { dotcomHost, err := utils.NewAPIHost("https://api.github.com") require.NoError(t, err) - handler, err := NewAuthHandler(t.Context(), tc.cfg, dotcomHost) + handler, err := NewAuthHandler(tc.cfg, dotcomHost) require.NoError(t, err) router := chi.NewRouter() @@ -496,7 +496,7 @@ func TestRegisterRoutes(t *testing.T) { dotcomHost, err := utils.NewAPIHost("https://api.github.com") require.NoError(t, err) - handler, err := NewAuthHandler(t.Context(), &Config{ + handler, err := NewAuthHandler(&Config{ BaseURL: "https://api.example.com", }, dotcomHost) require.NoError(t, err) @@ -565,7 +565,7 @@ func TestProtectedResourceResponseFormat(t *testing.T) { dotcomHost, err := utils.NewAPIHost("https://api.github.com") require.NoError(t, err) - handler, err := NewAuthHandler(t.Context(), &Config{ + handler, err := NewAuthHandler(&Config{ BaseURL: "https://api.example.com", }, dotcomHost) require.NoError(t, err) @@ -676,11 +676,10 @@ func TestAPIHostResolver_AuthorizationServerURL(t *testing.T) { assert.Contains(t, err.Error(), tc.errorContains) } return - } else { - require.NoError(t, err) } + require.NoError(t, err) - handler, err := NewAuthHandler(t.Context(), &Config{ + handler, err := NewAuthHandler(&Config{ BaseURL: "https://api.example.com", }, apiHost) require.NoError(t, err) @@ -708,7 +707,6 @@ func TestAPIHostResolver_AuthorizationServerURL(t *testing.T) { } return } - require.NoError(t, err) responseAuthServers, ok := response["authorization_servers"].([]any) require.True(t, ok) diff --git a/pkg/http/server.go b/pkg/http/server.go index aa5f6040f..872303940 100644 --- a/pkg/http/server.go +++ b/pkg/http/server.go @@ -136,7 +136,7 @@ func RunHTTPServer(cfg ServerConfig) error { r := chi.NewRouter() handler := NewHTTPMcpHandler(ctx, &cfg, deps, t, logger, apiHost, append(serverOptions, WithFeatureChecker(featureChecker), WithOAuthConfig(oauthCfg))...) - oauthHandler, err := oauth.NewAuthHandler(ctx, oauthCfg, apiHost) + oauthHandler, err := oauth.NewAuthHandler(oauthCfg, apiHost) if err != nil { return fmt.Errorf("failed to create OAuth handler: %w", err) } From cdcb674940cba3d7d82f820c98a75cff92ca79ed Mon Sep 17 00:00:00 2001 From: Adam Holt Date: Mon, 23 Feb 2026 18:23:24 +0100 Subject: [PATCH 6/8] Fix default scheme, add test for http GHES --- pkg/http/oauth/oauth_test.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/pkg/http/oauth/oauth_test.go b/pkg/http/oauth/oauth_test.go index d8da6916d..d5509457e 100644 --- a/pkg/http/oauth/oauth_test.go +++ b/pkg/http/oauth/oauth_test.go @@ -633,7 +633,7 @@ func TestAPIHostResolver_AuthorizationServerURL(t *testing.T) { }{ { name: "valid host returns authorization server URL", - host: "http://github.com", + host: "https://github.com", expectedURL: "https://github.com/login/oauth", expectedStatusCode: http.StatusOK, }, @@ -663,6 +663,12 @@ func TestAPIHostResolver_AuthorizationServerURL(t *testing.T) { expectedURL: "https://ghe.example.com/login/oauth", expectedStatusCode: http.StatusOK, }, + { + name: "GHES with http scheme returns the correct authorization server URL", + host: "http://ghe.example.com", + expectedURL: "http://ghe.example.com/login/oauth", + expectedStatusCode: http.StatusOK, + }, } for _, tc := range tests { From 5c06b129431b4fb6394d7371c2cca6d963825515 Mon Sep 17 00:00:00 2001 From: Adam Holt Date: Mon, 23 Feb 2026 18:24:40 +0100 Subject: [PATCH 7/8] Default to a api.github.com host --- pkg/http/oauth/oauth.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/pkg/http/oauth/oauth.go b/pkg/http/oauth/oauth.go index 807029cfd..3b4d41959 100644 --- a/pkg/http/oauth/oauth.go +++ b/pkg/http/oauth/oauth.go @@ -63,6 +63,14 @@ func NewAuthHandler(cfg *Config, apiHost utils.APIHostResolver) (*AuthHandler, e cfg = &Config{} } + if apiHost == nil { + var err error + apiHost, err = utils.NewAPIHost("https://api.github.com") + if err != nil { + return nil, fmt.Errorf("failed to create default API host: %w", err) + } + } + return &AuthHandler{ cfg: cfg, apiHost: apiHost, From e6a24f0f5ce3c562290ef3025cc92be3639922ac Mon Sep 17 00:00:00 2001 From: Adam Holt Date: Mon, 23 Feb 2026 18:32:55 +0100 Subject: [PATCH 8/8] Test that explicit AuthorizationServer settings take precedence. --- pkg/http/oauth/oauth_test.go | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/pkg/http/oauth/oauth_test.go b/pkg/http/oauth/oauth_test.go index d5509457e..6d76b579f 100644 --- a/pkg/http/oauth/oauth_test.go +++ b/pkg/http/oauth/oauth_test.go @@ -626,6 +626,7 @@ func TestAPIHostResolver_AuthorizationServerURL(t *testing.T) { tests := []struct { name string host string + oauthConfig *Config expectedURL string expectedError bool expectedStatusCode int @@ -669,6 +670,15 @@ func TestAPIHostResolver_AuthorizationServerURL(t *testing.T) { expectedURL: "http://ghe.example.com/login/oauth", expectedStatusCode: http.StatusOK, }, + { + name: "custom authorization server in config takes precedence", + host: "https://github.com", + oauthConfig: &Config{ + AuthorizationServer: "https://custom.auth.example.com/oauth", + }, + expectedURL: "https://custom.auth.example.com/oauth", + expectedStatusCode: http.StatusOK, + }, } for _, tc := range tests { @@ -685,9 +695,13 @@ func TestAPIHostResolver_AuthorizationServerURL(t *testing.T) { } require.NoError(t, err) - handler, err := NewAuthHandler(&Config{ - BaseURL: "https://api.example.com", - }, apiHost) + config := tc.oauthConfig + if config == nil { + config = &Config{} + } + config.BaseURL = tc.host + + handler, err := NewAuthHandler(config, apiHost) require.NoError(t, err) router := chi.NewRouter()