Skip to content

Commit 705a801

Browse files
authored
Add dynamic OAuth discovery for community MCP servers (#410)
## Summary - Community MCP servers from third-party registries (e.g., Kubit) require OAuth but have no `oauth.providers` metadata in their catalog entry, so the existing `IsRemoteOAuthServer()` gate prevents DCR entries from being created - Adds `RegisterProviderForDynamicDiscovery()` which probes remote servers using `DiscoverOAuthRequirements()` and creates pending DCR entries when OAuth is detected - Integrates the fallback into all three callsites: working set sync, `docker mcp server enable`, and gateway `mcpadd` - Expands the gateway OAuth condition to also match remote servers without `oauth.providers` ## Test plan - [x] `docker mcp catalog-next pull mcp/community-registry` - [x] `docker mcp profile server add default --server catalog://mcp/community-registry/com-notion-mcp` - [x] Verify DCR entry is created: `docker mcp oauth ls` - [x] Authorize the server: `docker mcp oauth authorize com-notion-mcp` - [x] Verify authorization: `docker mcp oauth ls` - [x] Revoke and re-authorize: `docker mcp oauth revoke com-notion-mcp` - [x] Verify existing servers with `oauth.providers` still work (no regression) - [x] Verify servers that don't require OAuth are not affected (probe returns no OAuth) Confirming `com-notion-mcp` from community registry successfully authorized via oauth: ``` ❯ docker mcp oauth ls ai-kubit-mcp-server | not authorized com-notion-mcp | authorized github | authorized miro-remote | not authorized ``` Confirming oauth flow for `com-notion-mcp` from community registry: ``` ❯ docker mcp oauth authorize com-notion-mcp Opening your browser for authentication. If it doesn't open automatically, please visit: ... ```
1 parent 5d74672 commit 705a801

File tree

14 files changed

+602
-38
lines changed

14 files changed

+602
-38
lines changed

cmd/docker-mcp/oauth/revoke.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ func Revoke(ctx context.Context, app string) error {
2626
func revokeDesktopMode(ctx context.Context, app string) error {
2727
client := desktop.NewAuthClient()
2828

29-
// Revoke tokens
29+
// Revoke tokens via Docker Desktop
3030
if err := client.DeleteOAuthApp(ctx, app); err != nil {
3131
return fmt.Errorf("failed to revoke OAuth access: %w", err)
3232
}

cmd/docker-mcp/server/enable.go

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ func update(ctx context.Context, docker docker.Client, dockerCli command.Cli, ad
6161
}
6262

6363
// DCR flag enabled AND type="remote" AND oauth present
64-
if mcpOAuthDcrEnabled && server.IsRemoteOAuthServer() {
64+
if mcpOAuthDcrEnabled && server.HasExplicitOAuthProviders() {
6565
// In CE mode, skip lazy setup - DCR happens during oauth authorize
6666
if pkgoauth.IsCEMode() {
6767
fmt.Printf("OAuth server %s enabled. Run 'docker mcp oauth authorize %s' to authenticate\n", serverName, serverName)
@@ -74,11 +74,20 @@ func update(ctx context.Context, docker docker.Client, dockerCli command.Cli, ad
7474
fmt.Printf("OAuth provider configured for %s - use 'docker mcp oauth authorize %s' to authenticate\n", serverName, serverName)
7575
}
7676
}
77-
} else if !mcpOAuthDcrEnabled && server.IsRemoteOAuthServer() {
77+
} else if !mcpOAuthDcrEnabled && server.HasExplicitOAuthProviders() {
7878
// Provide guidance when DCR is needed but disabled
7979
fmt.Printf("Server %s requires OAuth authentication but DCR is disabled.\n", serverName)
8080
fmt.Printf(" To enable automatic OAuth setup, run: docker mcp feature enable mcp-oauth-dcr\n")
8181
fmt.Printf(" Or set up OAuth manually using: docker mcp oauth authorize %s\n", serverName)
82+
} else if mcpOAuthDcrEnabled && server.Type == "remote" && !server.IsOAuthServer() && server.Remote.URL != "" {
83+
// Community server without oauth.providers — probe for OAuth
84+
if pkgoauth.IsCEMode() {
85+
fmt.Printf("Remote server %s enabled. Run 'docker mcp oauth authorize %s' if authentication is required\n", serverName, serverName)
86+
} else {
87+
if err := pkgoauth.RegisterProviderForDynamicDiscovery(ctx, serverName, server.Remote.URL); err != nil {
88+
fmt.Printf("Warning: Dynamic OAuth discovery failed for %s: %v\n", serverName, err)
89+
}
90+
}
8291
}
8392
} else {
8493
return fmt.Errorf("server %s not found in catalog", serverName)

pkg/catalog/types.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,10 @@ func (s *Server) IsOAuthServer() bool {
6363
return s.OAuth != nil && len(s.OAuth.Providers) > 0
6464
}
6565

66-
func (s *Server) IsRemoteOAuthServer() bool {
66+
// HasExplicitOAuthProviders returns true if this is a remote server with
67+
// explicit OAuth provider metadata in the catalog (e.g. oauth.providers YAML).
68+
// Community servers that discover OAuth dynamically will return false here.
69+
func (s *Server) HasExplicitOAuthProviders() bool {
6770
return s.Type == "remote" && s.IsOAuthServer()
6871
}
6972

pkg/gateway/clientpool.go

Lines changed: 16 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -179,24 +179,23 @@ func (cp *clientPool) InvalidateOAuthClients(provider string) {
179179

180180
var invalidatedKeys []clientKey
181181
for key, keptClient := range cp.keptClients {
182-
// Check if this client uses OAuth for the specified provider
183-
if keptClient.Config.Spec.OAuth != nil {
184-
// Match by server name (for DCR providers, server name matches provider)
185-
if keptClient.Config.Name == provider {
186-
log.Log(fmt.Sprintf("ClientPool: Closing OAuth connection for server: %s", keptClient.Config.Name))
187-
188-
// Close the connection
189-
client, err := keptClient.Getter.GetClient(context.TODO())
190-
if err == nil {
191-
client.Session().Close()
192-
log.Log(fmt.Sprintf("ClientPool: Successfully closed connection for %s", keptClient.Config.Name))
193-
} else {
194-
log.Log(fmt.Sprintf("ClientPool: Warning - failed to get client for %s during invalidation: %v", keptClient.Config.Name, err))
195-
}
196-
197-
// Mark for removal from kept clients
198-
invalidatedKeys = append(invalidatedKeys, key)
182+
// Check if this remote client matches the OAuth provider
183+
// Matches both catalog servers (explicit OAuth metadata) and community servers
184+
// (dynamic OAuth discovery via DCR without Spec.OAuth)
185+
if keptClient.Config.Name == provider && keptClient.Config.IsRemote() {
186+
log.Log(fmt.Sprintf("ClientPool: Closing OAuth connection for server: %s", keptClient.Config.Name))
187+
188+
// Close the connection
189+
client, err := keptClient.Getter.GetClient(context.TODO())
190+
if err == nil {
191+
client.Session().Close()
192+
log.Log(fmt.Sprintf("ClientPool: Successfully closed connection for %s", keptClient.Config.Name))
193+
} else {
194+
log.Log(fmt.Sprintf("ClientPool: Warning - failed to get client for %s during invalidation: %v", keptClient.Config.Name, err))
199195
}
196+
197+
// Mark for removal from kept clients
198+
invalidatedKeys = append(invalidatedKeys, key)
200199
}
201200
}
202201

pkg/gateway/clientpool_test.go

Lines changed: 192 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package gateway
22

33
import (
44
"context"
5+
"fmt"
56
"os"
67
"testing"
78
"time"
@@ -277,6 +278,197 @@ func parseConfig(t *testing.T, contentYAML string) map[string]any {
277278
return config
278279
}
279280

281+
func TestInvalidateOAuthClients_MatchesCommunityServer(t *testing.T) {
282+
// Community server: remote URL set, but no Spec.OAuth metadata.
283+
// This verifies Gap 3: InvalidateOAuthClients matches community servers
284+
// that use dynamic OAuth discovery without explicit OAuth config.
285+
cp := &clientPool{
286+
keptClients: make(map[clientKey]keptClient),
287+
}
288+
289+
getter := &clientGetter{}
290+
getter.once.Do(func() {}) // mark as executed
291+
getter.err = fmt.Errorf("mock: no real client")
292+
293+
key := clientKey{serverName: "com-notion-mcp"}
294+
cp.keptClients[key] = keptClient{
295+
Name: "com-notion-mcp",
296+
Getter: getter,
297+
Config: &catalog.ServerConfig{
298+
Name: "com-notion-mcp",
299+
Spec: catalog.Server{
300+
Type: "remote",
301+
Remote: catalog.Remote{
302+
URL: "https://mcp.notion.so/mcp",
303+
Transport: "streamable-http",
304+
},
305+
// No OAuth field - community server
306+
},
307+
},
308+
}
309+
310+
cp.InvalidateOAuthClients("com-notion-mcp")
311+
312+
assert.Empty(t, cp.keptClients, "community server should be invalidated by name")
313+
}
314+
315+
func TestInvalidateOAuthClients_MatchesCatalogServer(t *testing.T) {
316+
// Catalog server: remote URL set WITH Spec.OAuth metadata.
317+
// Verifies backward compatibility: catalog servers still get invalidated.
318+
cp := &clientPool{
319+
keptClients: make(map[clientKey]keptClient),
320+
}
321+
322+
getter := &clientGetter{}
323+
getter.once.Do(func() {})
324+
getter.err = fmt.Errorf("mock: no real client")
325+
326+
key := clientKey{serverName: "notion-remote"}
327+
cp.keptClients[key] = keptClient{
328+
Name: "notion-remote",
329+
Getter: getter,
330+
Config: &catalog.ServerConfig{
331+
Name: "notion-remote",
332+
Spec: catalog.Server{
333+
Type: "remote",
334+
Remote: catalog.Remote{
335+
URL: "https://mcp.notion.so/mcp",
336+
Transport: "streamable-http",
337+
},
338+
OAuth: &catalog.OAuth{
339+
Providers: []catalog.OAuthProvider{{Provider: "notion"}},
340+
},
341+
},
342+
},
343+
}
344+
345+
cp.InvalidateOAuthClients("notion-remote")
346+
347+
assert.Empty(t, cp.keptClients, "catalog server should be invalidated by name")
348+
}
349+
350+
func TestInvalidateOAuthClients_SkipsNonRemoteServer(t *testing.T) {
351+
// Docker container server: not remote, should NOT be invalidated.
352+
cp := &clientPool{
353+
keptClients: make(map[clientKey]keptClient),
354+
}
355+
356+
getter := &clientGetter{}
357+
getter.once.Do(func() {})
358+
getter.err = fmt.Errorf("mock: no real client")
359+
360+
key := clientKey{serverName: "my-container-server"}
361+
cp.keptClients[key] = keptClient{
362+
Name: "my-container-server",
363+
Getter: getter,
364+
Config: &catalog.ServerConfig{
365+
Name: "my-container-server",
366+
Spec: catalog.Server{
367+
Type: "server",
368+
Image: "mcp/my-server:latest",
369+
// Not remote - no URL
370+
},
371+
},
372+
}
373+
374+
cp.InvalidateOAuthClients("my-container-server")
375+
376+
assert.Len(t, cp.keptClients, 1, "non-remote server should NOT be invalidated")
377+
}
378+
379+
func TestInvalidateOAuthClients_SkipsMismatchedName(t *testing.T) {
380+
// Remote server with different name: should NOT be invalidated.
381+
cp := &clientPool{
382+
keptClients: make(map[clientKey]keptClient),
383+
}
384+
385+
getter := &clientGetter{}
386+
getter.once.Do(func() {})
387+
getter.err = fmt.Errorf("mock: no real client")
388+
389+
key := clientKey{serverName: "other-server"}
390+
cp.keptClients[key] = keptClient{
391+
Name: "other-server",
392+
Getter: getter,
393+
Config: &catalog.ServerConfig{
394+
Name: "other-server",
395+
Spec: catalog.Server{
396+
Type: "remote",
397+
Remote: catalog.Remote{
398+
URL: "https://other.example.com/mcp",
399+
},
400+
},
401+
},
402+
}
403+
404+
cp.InvalidateOAuthClients("com-notion-mcp")
405+
406+
assert.Len(t, cp.keptClients, 1, "server with different name should NOT be invalidated")
407+
}
408+
409+
func TestInvalidateOAuthClients_OnlyMatchingRemoved(t *testing.T) {
410+
// Multiple clients: only the matching remote server should be removed.
411+
cp := &clientPool{
412+
keptClients: make(map[clientKey]keptClient),
413+
}
414+
415+
makeGetter := func() *clientGetter {
416+
g := &clientGetter{}
417+
g.once.Do(func() {})
418+
g.err = fmt.Errorf("mock: no real client")
419+
return g
420+
}
421+
422+
// Community OAuth server (should be invalidated)
423+
cp.keptClients[clientKey{serverName: "com-notion-mcp"}] = keptClient{
424+
Name: "com-notion-mcp",
425+
Getter: makeGetter(),
426+
Config: &catalog.ServerConfig{
427+
Name: "com-notion-mcp",
428+
Spec: catalog.Server{
429+
Type: "remote",
430+
Remote: catalog.Remote{URL: "https://mcp.notion.so/mcp"},
431+
},
432+
},
433+
}
434+
435+
// Different remote server (should NOT be invalidated)
436+
cp.keptClients[clientKey{serverName: "github-remote"}] = keptClient{
437+
Name: "github-remote",
438+
Getter: makeGetter(),
439+
Config: &catalog.ServerConfig{
440+
Name: "github-remote",
441+
Spec: catalog.Server{
442+
Type: "remote",
443+
Remote: catalog.Remote{URL: "https://mcp.github.com/mcp"},
444+
},
445+
},
446+
}
447+
448+
// Docker container server (should NOT be invalidated)
449+
cp.keptClients[clientKey{serverName: "local-server"}] = keptClient{
450+
Name: "local-server",
451+
Getter: makeGetter(),
452+
Config: &catalog.ServerConfig{
453+
Name: "local-server",
454+
Spec: catalog.Server{
455+
Type: "server",
456+
Image: "mcp/local:latest",
457+
},
458+
},
459+
}
460+
461+
cp.InvalidateOAuthClients("com-notion-mcp")
462+
463+
assert.Len(t, cp.keptClients, 2, "only the matching remote server should be removed")
464+
_, hasNotion := cp.keptClients[clientKey{serverName: "com-notion-mcp"}]
465+
assert.False(t, hasNotion, "com-notion-mcp should have been removed")
466+
_, hasGithub := cp.keptClients[clientKey{serverName: "github-remote"}]
467+
assert.True(t, hasGithub, "github-remote should remain")
468+
_, hasLocal := cp.keptClients[clientKey{serverName: "local-server"}]
469+
assert.True(t, hasLocal, "local-server should remain")
470+
}
471+
280472
func TestStdioClientInitialization(t *testing.T) {
281473
// This is an integration test that requires Docker
282474
if testing.Short() {

pkg/gateway/mcpadd.go

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -286,10 +286,12 @@ func addServerHandler(g *Gateway, clientConfig *clientConfig) mcp.ToolHandler {
286286
}
287287
}
288288

289-
// Handle OAuth DCR only when the client supports elicitation (e.g. not stdio-based clients)
289+
// Handle OAuth DCR for any remote server — covers both catalog servers
290+
// (explicit OAuth metadata) and community servers (dynamic discovery).
291+
// getRemoteOAuthServerStatus handles the case where OAuth is not needed.
290292
if g.McpOAuthDcrEnabled &&
291293
serverConfig != nil &&
292-
serverConfig.Spec.IsRemoteOAuthServer() {
294+
serverConfig.IsRemote() {
293295

294296
init := req.Session.InitializeParams()
295297
if init != nil &&
@@ -444,7 +446,25 @@ func (g *Gateway) getRemoteOAuthServerStatus(ctx context.Context, serverName str
444446
if !providerExists {
445447
// Register DCR client with DD so user can authorize
446448
if err := oauth.RegisterProviderForLazySetup(ctx, serverName); err != nil {
447-
log.Logf("Warning: Failed to register OAuth provider for %s: %v", serverName, err)
449+
// Fallback: try dynamic discovery for community servers without oauth.providers
450+
if serverConfig, _, found := g.configuration.Find(serverName); found && serverConfig.Spec.Remote.URL != "" {
451+
if err := oauth.RegisterProviderForDynamicDiscovery(ctx, serverName, serverConfig.Spec.Remote.URL); err != nil {
452+
log.Logf("Warning: Failed to register OAuth provider for %s: %v", serverName, err)
453+
}
454+
} else {
455+
log.Logf("Warning: Failed to register OAuth provider for %s: %v", serverName, err)
456+
}
457+
}
458+
459+
// Verify DCR entry was created — dynamic discovery may have found no OAuth requirement.
460+
// Distinguish "not found" (server doesn't need OAuth) from transient API errors.
461+
authClient := desktop.NewAuthClient()
462+
if _, err := authClient.GetDCRClient(ctx, serverName); err != nil {
463+
if strings.Contains(err.Error(), "HTTP 404") {
464+
return true, "" // Server doesn't require OAuth
465+
}
466+
log.Logf("Warning: Failed to verify DCR entry for %s (may be transient): %v", serverName, err)
467+
return true, "" // Fail open — avoid blocking the add flow on transient errors
448468
}
449469

450470
// Start provider (CE mode only - Desktop mode doesn't need polling)

pkg/gateway/run.go

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -355,13 +355,25 @@ func (g *Gateway) Run(ctx context.Context) error {
355355
// Start OAuth provider for each OAuth server.
356356
// Each provider runs in its own goroutine with dynamic timing based on token expiry.
357357
log.Log("- Starting OAuth provider loops...")
358+
credHelper := oauth.NewOAuthCredentialHelper()
358359
for _, serverName := range configuration.ServerNames() {
359360
serverConfig, _, found := configuration.Find(serverName)
360-
if !found || serverConfig == nil || !serverConfig.Spec.IsRemoteOAuthServer() {
361+
if !found || serverConfig == nil {
361362
continue
362363
}
363364

364-
g.startProvider(ctx, serverName)
365+
if serverConfig.Spec.HasExplicitOAuthProviders() {
366+
g.startProvider(ctx, serverName)
367+
} else if serverConfig.IsRemote() {
368+
// Community servers: start provider if they have a stored OAuth token
369+
// from dynamic discovery (DCR without explicit OAuth metadata)
370+
if exists, err := credHelper.TokenExists(ctx, serverName); err != nil {
371+
log.Logf("Warning: Failed to check OAuth token for %s: %v", serverName, err)
372+
} else if exists {
373+
log.Logf("- Starting OAuth provider for community server: %s", serverName)
374+
g.startProvider(ctx, serverName)
375+
}
376+
}
365377
}
366378
}
367379

@@ -700,7 +712,9 @@ func (g *Gateway) routeEventToProvider(event oauth.Event) {
700712
g.clientPool.InvalidateOAuthClients(event.Provider)
701713

702714
case oauth.EventLogoutSuccess:
703-
// User logged out - stop provider if exists
715+
// Invalidate cached OAuth client connections (clear stale bearer tokens)
716+
g.clientPool.InvalidateOAuthClients(event.Provider)
717+
// Stop provider if exists
704718
if exists {
705719
log.Logf("- Stopping provider for %s after logout", event.Provider)
706720
g.stopProvider(event.Provider)

pkg/mcp/remote.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,17 @@ func (c *remoteMCPClient) Initialize(ctx context.Context, _ *mcp.InitializeParam
9393
} else if token != "" {
9494
headers["Authorization"] = "Bearer " + token
9595
}
96+
} else if c.config.Spec.Remote.URL != "" {
97+
// Community servers may have OAuth tokens via dynamic discovery (DCR)
98+
// without explicit OAuth metadata in the catalog. Try to get a stored token.
99+
credHelper := oauth.NewOAuthCredentialHelper()
100+
token, err := credHelper.GetOAuthToken(ctx, c.config.Name)
101+
if err == nil && token != "" {
102+
if verbose {
103+
log.Logf(" - Using dynamic OAuth token for: %s", c.config.Name)
104+
}
105+
headers["Authorization"] = "Bearer " + token
106+
}
96107
}
97108

98109
var mcpTransport mcp.Transport

0 commit comments

Comments
 (0)