Skip to content

Commit ce05b87

Browse files
committed
Require an API host when creating scope fetchers
1 parent dad1e71 commit ce05b87

File tree

7 files changed

+28
-43
lines changed

7 files changed

+28
-43
lines changed

internal/ghmcp/server.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -366,9 +366,7 @@ func fetchTokenScopesForHost(ctx context.Context, token, host string) ([]string,
366366
return nil, fmt.Errorf("failed to parse API host: %w", err)
367367
}
368368

369-
fetcher := scopes.NewFetcher(scopes.FetcherOptions{
370-
APIHost: apiHost,
371-
})
369+
fetcher := scopes.NewFetcher(apiHost, scopes.FetcherOptions{})
372370

373371
return fetcher.FetchTokenScopes(ctx, token)
374372
}

pkg/http/handler.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -93,9 +93,7 @@ func NewHTTPMcpHandler(
9393

9494
scopeFetcher := opts.ScopeFetcher
9595
if scopeFetcher == nil {
96-
scopeFetcher = scopes.NewFetcher(scopes.FetcherOptions{
97-
APIHost: apiHost,
98-
})
96+
scopeFetcher = scopes.NewFetcher(apiHost, scopes.FetcherOptions{})
9997
}
10098

10199
inventoryFactory := opts.InventoryFactory
@@ -121,6 +119,7 @@ func (h *Handler) RegisterMiddleware(r chi.Router) {
121119
r.Use(
122120
middleware.ExtractUserToken(h.oauthCfg),
123121
middleware.WithRequestConfig,
122+
middleware.WithMCPParse(),
124123
)
125124

126125
if h.config.ScopeChallenge {

pkg/http/handler_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -276,7 +276,8 @@ func TestHTTPHandlerRoutes(t *testing.T) {
276276
// Create feature checker that reads from context (same as production)
277277
featureChecker := createHTTPFeatureChecker()
278278

279-
apiHost := utils.NewDefaultAPIHostResolver()
279+
apiHost, err := utils.NewAPIHost("https://api.github.com")
280+
require.NoError(t, err)
280281

281282
// Create inventory factory that captures the built inventory
282283
inventoryFactory := func(r *http.Request) (*inventory.Inventory, error) {

pkg/http/server.go

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -131,16 +131,14 @@ func RunHTTPServer(cfg ServerConfig) error {
131131
ResourcePath: cfg.ResourcePath,
132132
}
133133

134-
severOptions := []HandlerOption{}
134+
serverOptions := []HandlerOption{}
135135
if cfg.ScopeChallenge {
136-
scopeFetcher := scopes.NewFetcher(scopes.FetcherOptions{
137-
APIHost: apiHost,
138-
})
139-
severOptions = append(severOptions, WithScopeFetcher(scopeFetcher))
136+
scopeFetcher := scopes.NewFetcher(apiHost, scopes.FetcherOptions{})
137+
serverOptions = append(serverOptions, WithScopeFetcher(scopeFetcher))
140138
}
141139

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

pkg/scopes/fetcher.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -40,17 +40,12 @@ type Fetcher struct {
4040
}
4141

4242
// NewFetcher creates a new scope fetcher with the given options.
43-
func NewFetcher(opts FetcherOptions) *Fetcher {
43+
func NewFetcher(apiHost utils.APIHostResolver, opts FetcherOptions) *Fetcher {
4444
client := opts.HTTPClient
4545
if client == nil {
4646
client = &http.Client{Timeout: DefaultFetchTimeout}
4747
}
4848

49-
apiHost := opts.APIHost
50-
if apiHost == nil {
51-
apiHost = utils.NewDefaultAPIHostResolver()
52-
}
53-
5449
return &Fetcher{
5550
client: client,
5651
apiHost: apiHost,
@@ -126,11 +121,16 @@ func ParseScopeHeader(header string) []string {
126121
// FetchTokenScopes is a convenience function that creates a default fetcher
127122
// and fetches the token scopes.
128123
func FetchTokenScopes(ctx context.Context, token string) ([]string, error) {
129-
return NewFetcher(FetcherOptions{}).FetchTokenScopes(ctx, token)
124+
apiHost, err := utils.NewAPIHost("https://api.github.com/")
125+
if err != nil {
126+
return nil, fmt.Errorf("failed to create default API host: %w", err)
127+
}
128+
129+
return NewFetcher(apiHost, FetcherOptions{}).FetchTokenScopes(ctx, token)
130130
}
131131

132132
// FetchTokenScopesWithHost is a convenience function that creates a fetcher
133133
// for a specific API host and fetches the token scopes.
134134
func FetchTokenScopesWithHost(ctx context.Context, token string, apiHost utils.APIHostResolver) ([]string, error) {
135-
return NewFetcher(FetcherOptions{APIHost: apiHost}).FetchTokenScopes(ctx, token)
135+
return NewFetcher(apiHost, FetcherOptions{}).FetchTokenScopes(ctx, token)
136136
}

pkg/scopes/fetcher_test.go

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -164,10 +164,8 @@ func TestFetcher_FetchTokenScopes(t *testing.T) {
164164
t.Run(tt.name, func(t *testing.T) {
165165
server := httptest.NewServer(tt.handler)
166166
defer server.Close()
167-
168-
fetcher := NewFetcher(FetcherOptions{
169-
APIHost: testAPIHostResolver{baseURL: server.URL},
170-
})
167+
apiHost := testAPIHostResolver{baseURL: server.URL}
168+
fetcher := NewFetcher(apiHost, FetcherOptions{})
171169

172170
scopes, err := fetcher.FetchTokenScopes(context.Background(), "test-token")
173171

@@ -185,12 +183,13 @@ func TestFetcher_FetchTokenScopes(t *testing.T) {
185183
}
186184

187185
func TestFetcher_DefaultOptions(t *testing.T) {
188-
fetcher := NewFetcher(FetcherOptions{})
186+
apiHost := testAPIHostResolver{baseURL: "https://api.github.com"}
187+
fetcher := NewFetcher(apiHost, FetcherOptions{})
189188

190189
// Verify default API host is set
191190
apiURL, err := fetcher.apiHost.BaseRESTURL(context.Background())
192191
require.NoError(t, err)
193-
assert.Equal(t, "https://api.github.com/", apiURL.String())
192+
assert.Equal(t, "https://api.github.com", apiURL.String())
194193

195194
// Verify default HTTP client is set with timeout
196195
assert.NotNil(t, fetcher.client)
@@ -200,17 +199,17 @@ func TestFetcher_DefaultOptions(t *testing.T) {
200199
func TestFetcher_CustomHTTPClient(t *testing.T) {
201200
customClient := &http.Client{Timeout: 5 * time.Second}
202201

203-
fetcher := NewFetcher(FetcherOptions{
202+
apiHost := testAPIHostResolver{baseURL: "https://api.github.com"}
203+
fetcher := NewFetcher(apiHost, FetcherOptions{
204204
HTTPClient: customClient,
205205
})
206206

207207
assert.Equal(t, customClient, fetcher.client)
208208
}
209209

210210
func TestFetcher_CustomAPIHost(t *testing.T) {
211-
fetcher := NewFetcher(FetcherOptions{
212-
APIHost: testAPIHostResolver{baseURL: "https://api.github.enterprise.com"},
213-
})
211+
apiHost := testAPIHostResolver{baseURL: "https://api.github.enterprise.com"}
212+
fetcher := NewFetcher(apiHost, FetcherOptions{})
214213

215214
apiURL, err := fetcher.apiHost.BaseRESTURL(context.Background())
216215
require.NoError(t, err)
@@ -224,9 +223,8 @@ func TestFetcher_ContextCancellation(t *testing.T) {
224223
}))
225224
defer server.Close()
226225

227-
fetcher := NewFetcher(FetcherOptions{
228-
APIHost: testAPIHostResolver{baseURL: server.URL},
229-
})
226+
apiHost := testAPIHostResolver{baseURL: server.URL}
227+
fetcher := NewFetcher(apiHost, FetcherOptions{})
230228

231229
ctx, cancel := context.WithCancel(context.Background())
232230
cancel() // Cancel immediately

pkg/utils/api.go

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -25,15 +25,6 @@ type APIHost struct {
2525

2626
var _ APIHostResolver = APIHost{}
2727

28-
func NewDefaultAPIHostResolver() APIHostResolver {
29-
a, err := newDotcomHost()
30-
if err != nil {
31-
// This should never happen
32-
panic(fmt.Sprintf("failed to create default API host resolver: %v", err))
33-
}
34-
return a
35-
}
36-
3728
func NewAPIHost(s string) (APIHostResolver, error) {
3829
a, err := parseAPIHost(s)
3930

0 commit comments

Comments
 (0)