Skip to content

Commit 1f4a0ab

Browse files
committed
Add dynamic OAuth discovery for community MCP servers
Community servers from third-party registries lack oauth.providers metadata, so the existing IsRemoteOAuthServer() gate prevents DCR entries from being created. This adds a fallback path that probes remote servers for OAuth support using DiscoverOAuthRequirements() and creates pending DCR entries. - Add RegisterProviderForDynamicDiscovery() in pkg/oauth/dcr_registration.go - Add fallback branch in workingset, server enable, and gateway mcpadd - Expand gateway OAuth condition to match remote servers without oauth.providers
1 parent 7ecec65 commit 1f4a0ab

File tree

10 files changed

+498
-30
lines changed

10 files changed

+498
-30
lines changed

cmd/docker-mcp/oauth/revoke.go

Lines changed: 4 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import (
44
"context"
55
"fmt"
66

7-
"github.com/docker/mcp-gateway/pkg/catalog"
87
"github.com/docker/mcp-gateway/pkg/desktop"
98
pkgoauth "github.com/docker/mcp-gateway/pkg/oauth"
109
)
@@ -25,27 +24,14 @@ func Revoke(ctx context.Context, app string) error {
2524
func revokeDesktopMode(ctx context.Context, app string) error {
2625
client := desktop.NewAuthClient()
2726

28-
// Get catalog to check if this is a remote OAuth server
29-
catalogData, err := catalog.GetWithOptions(ctx, true, nil)
30-
if err != nil {
31-
return fmt.Errorf("failed to get catalog: %w", err)
32-
}
33-
34-
server, found := catalogData.Servers[app]
35-
isRemoteOAuth := found && server.IsRemoteOAuthServer()
36-
37-
// Revoke tokens
27+
// Revoke tokens via Docker Desktop.
28+
// The DCR client entry is intentionally kept so the server remains
29+
// visible in the OAuth UI for re-authorization. DCR cleanup happens
30+
// when the server is removed from all profiles (doCleanupOrphanedDCREntries).
3831
if err := client.DeleteOAuthApp(ctx, app); err != nil {
3932
return fmt.Errorf("failed to revoke OAuth access: %w", err)
4033
}
4134

42-
// For remote OAuth servers, also delete DCR client
43-
if isRemoteOAuth {
44-
if err := client.DeleteDCRClient(ctx, app); err != nil {
45-
return fmt.Errorf("failed to remove DCR client: %w", err)
46-
}
47-
}
48-
4935
fmt.Printf("OAuth access revoked for %s\n", app)
5036
return nil
5137
}

cmd/docker-mcp/server/enable.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,15 @@ func update(ctx context.Context, docker docker.Client, dockerCli command.Cli, ad
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/gateway/mcpadd.go

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -289,7 +289,8 @@ func addServerHandler(g *Gateway, clientConfig *clientConfig) mcp.ToolHandler {
289289
// Handle OAuth DCR only when the client supports elicitation (e.g. not stdio-based clients)
290290
if g.McpOAuthDcrEnabled &&
291291
serverConfig != nil &&
292-
serverConfig.Spec.IsRemoteOAuthServer() {
292+
(serverConfig.Spec.IsRemoteOAuthServer() ||
293+
(serverConfig.Spec.Type == "remote" && !serverConfig.Spec.IsOAuthServer() && serverConfig.Spec.Remote.URL != "")) {
293294

294295
init := req.Session.InitializeParams()
295296
if init != nil &&
@@ -444,7 +445,20 @@ func (g *Gateway) getRemoteOAuthServerStatus(ctx context.Context, serverName str
444445
if !providerExists {
445446
// Register DCR client with DD so user can authorize
446447
if err := oauth.RegisterProviderForLazySetup(ctx, serverName); err != nil {
447-
log.Logf("Warning: Failed to register OAuth provider for %s: %v", serverName, err)
448+
// Fallback: try dynamic discovery for community servers without oauth.providers
449+
if serverConfig, _, found := g.configuration.Find(serverName); found && serverConfig.Spec.Remote.URL != "" {
450+
if err := oauth.RegisterProviderForDynamicDiscovery(ctx, serverName, serverConfig.Spec.Remote.URL); err != nil {
451+
log.Logf("Warning: Failed to register OAuth provider for %s: %v", serverName, err)
452+
}
453+
} else {
454+
log.Logf("Warning: Failed to register OAuth provider for %s: %v", serverName, err)
455+
}
456+
}
457+
458+
// Verify DCR entry was created — dynamic discovery may have found no OAuth requirement
459+
authClient := desktop.NewAuthClient()
460+
if _, err := authClient.GetDCRClient(ctx, serverName); err != nil {
461+
return true, "" // Server doesn't require OAuth
448462
}
449463

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

pkg/gateway/run.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -697,7 +697,9 @@ func (g *Gateway) routeEventToProvider(event oauth.Event) {
697697
g.clientPool.InvalidateOAuthClients(event.Provider)
698698

699699
case oauth.EventLogoutSuccess:
700-
// User logged out - stop provider if exists
700+
// Invalidate cached OAuth client connections (clear stale bearer tokens)
701+
g.clientPool.InvalidateOAuthClients(event.Provider)
702+
// Stop provider if exists
701703
if exists {
702704
log.Logf("- Stopping provider for %s after logout", event.Provider)
703705
g.stopProvider(event.Provider)

pkg/oauth/dcr_registration.go

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,33 @@ package oauth
33
import (
44
"context"
55
"fmt"
6+
"time"
7+
8+
oauthhelpers "github.com/docker/mcp-gateway-oauth-helpers"
69

710
"github.com/docker/mcp-gateway/pkg/catalog"
811
"github.com/docker/mcp-gateway/pkg/desktop"
912
)
1013

14+
// dcrRegistrationClient is the subset of desktop.Tools used for DCR registration.
15+
// Extracted as an interface to enable testing.
16+
type dcrRegistrationClient interface {
17+
GetDCRClient(ctx context.Context, app string) (*desktop.DCRClient, error)
18+
RegisterDCRClientPending(ctx context.Context, app string, req desktop.RegisterDCRRequest) error
19+
}
20+
21+
// oauthProber abstracts OAuth discovery to enable testing.
22+
type oauthProber interface {
23+
DiscoverOAuthRequirements(ctx context.Context, serverURL string) (*oauthhelpers.Discovery, error)
24+
}
25+
26+
// defaultOAuthProber wraps the package-level function.
27+
type defaultOAuthProber struct{}
28+
29+
func (defaultOAuthProber) DiscoverOAuthRequirements(ctx context.Context, serverURL string) (*oauthhelpers.Discovery, error) {
30+
return oauthhelpers.DiscoverOAuthRequirements(ctx, serverURL)
31+
}
32+
1133
// RegisterProviderForLazySetup registers a DCR provider with Docker Desktop
1234
// This allows 'docker mcp oauth authorize' to work before full DCR is complete
1335
// Idempotent - safe to call multiple times for the same server
@@ -46,6 +68,35 @@ func RegisterProviderForLazySetup(ctx context.Context, serverName string) error
4668
return client.RegisterDCRClientPending(ctx, serverName, dcrRequest)
4769
}
4870

71+
// RegisterProviderForDynamicDiscovery probes a remote server for OAuth support
72+
// and creates a pending DCR entry if the server requires OAuth.
73+
// This is used for community servers that lack oauth.providers metadata in the catalog.
74+
// Idempotent - safe to call multiple times for the same server.
75+
func RegisterProviderForDynamicDiscovery(ctx context.Context, serverName, serverURL string) error {
76+
return registerProviderForDynamicDiscovery(ctx, serverName, serverURL, desktop.NewAuthClient(), defaultOAuthProber{})
77+
}
78+
79+
func registerProviderForDynamicDiscovery(ctx context.Context, serverName, serverURL string, client dcrRegistrationClient, prober oauthProber) error {
80+
// Idempotent check - already registered?
81+
_, err := client.GetDCRClient(ctx, serverName)
82+
if err == nil {
83+
return nil // Already registered
84+
}
85+
86+
// Probe the server with a timeout to discover OAuth requirements
87+
probeCtx, cancel := context.WithTimeout(ctx, 10*time.Second)
88+
defer cancel()
89+
discovery, err := prober.DiscoverOAuthRequirements(probeCtx, serverURL)
90+
if err != nil || !discovery.RequiresOAuth {
91+
return nil // Server doesn't need OAuth, not an error
92+
}
93+
94+
// Register with DD (pending DCR state) using server name as provider name
95+
return client.RegisterDCRClientPending(ctx, serverName, desktop.RegisterDCRRequest{
96+
ProviderName: serverName,
97+
})
98+
}
99+
49100
// RegisterProviderWithSnapshot registers a DCR provider using OAuth metadata from the server snapshot
50101
// This avoids querying the catalog since the snapshot already contains all necessary OAuth information
51102
// Idempotent - safe to call multiple times for the same server

pkg/oauth/dcr_registration_test.go

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
package oauth
2+
3+
import (
4+
"context"
5+
"errors"
6+
"testing"
7+
8+
"github.com/stretchr/testify/assert"
9+
"github.com/stretchr/testify/require"
10+
11+
oauthhelpers "github.com/docker/mcp-gateway-oauth-helpers"
12+
"github.com/docker/mcp-gateway/pkg/desktop"
13+
)
14+
15+
// mockDCRClient implements dcrRegistrationClient for testing.
16+
type mockDCRClient struct {
17+
clients map[string]*desktop.DCRClient
18+
registered map[string]desktop.RegisterDCRRequest
19+
}
20+
21+
func newMockDCRClient() *mockDCRClient {
22+
return &mockDCRClient{
23+
clients: make(map[string]*desktop.DCRClient),
24+
registered: make(map[string]desktop.RegisterDCRRequest),
25+
}
26+
}
27+
28+
func (m *mockDCRClient) GetDCRClient(_ context.Context, app string) (*desktop.DCRClient, error) {
29+
c, ok := m.clients[app]
30+
if !ok {
31+
return nil, errors.New("not found")
32+
}
33+
return c, nil
34+
}
35+
36+
func (m *mockDCRClient) RegisterDCRClientPending(_ context.Context, app string, req desktop.RegisterDCRRequest) error {
37+
m.registered[app] = req
38+
return nil
39+
}
40+
41+
// mockProber implements oauthProber for testing.
42+
type mockProber struct {
43+
discovery *oauthhelpers.Discovery
44+
err error
45+
}
46+
47+
func (m *mockProber) DiscoverOAuthRequirements(_ context.Context, _ string) (*oauthhelpers.Discovery, error) {
48+
return m.discovery, m.err
49+
}
50+
51+
func TestRegisterProviderForDynamicDiscovery_SkipsAlreadyRegistered(t *testing.T) {
52+
client := newMockDCRClient()
53+
client.clients["my-server"] = &desktop.DCRClient{State: "unregistered"}
54+
55+
prober := &mockProber{} // should not be called
56+
57+
err := registerProviderForDynamicDiscovery(t.Context(), "my-server", "https://example.com/mcp", client, prober)
58+
require.NoError(t, err)
59+
assert.Empty(t, client.registered, "should not register when already exists")
60+
}
61+
62+
func TestRegisterProviderForDynamicDiscovery_RegistersWhenOAuthRequired(t *testing.T) {
63+
client := newMockDCRClient()
64+
prober := &mockProber{
65+
discovery: &oauthhelpers.Discovery{RequiresOAuth: true},
66+
}
67+
68+
err := registerProviderForDynamicDiscovery(t.Context(), "ai-kubit-mcp-server", "https://mcp.kubit.ai/mcp", client, prober)
69+
require.NoError(t, err)
70+
71+
req, ok := client.registered["ai-kubit-mcp-server"]
72+
require.True(t, ok, "should have registered DCR client")
73+
assert.Equal(t, "ai-kubit-mcp-server", req.ProviderName, "provider name should match server name")
74+
}
75+
76+
func TestRegisterProviderForDynamicDiscovery_SkipsWhenNoOAuthRequired(t *testing.T) {
77+
client := newMockDCRClient()
78+
prober := &mockProber{
79+
discovery: &oauthhelpers.Discovery{RequiresOAuth: false},
80+
}
81+
82+
err := registerProviderForDynamicDiscovery(t.Context(), "my-server", "https://example.com/mcp", client, prober)
83+
require.NoError(t, err)
84+
assert.Empty(t, client.registered, "should not register when OAuth not required")
85+
}
86+
87+
func TestRegisterProviderForDynamicDiscovery_SkipsOnProbeError(t *testing.T) {
88+
client := newMockDCRClient()
89+
prober := &mockProber{
90+
err: errors.New("connection refused"),
91+
}
92+
93+
err := registerProviderForDynamicDiscovery(t.Context(), "my-server", "https://unreachable.example.com/mcp", client, prober)
94+
require.NoError(t, err)
95+
assert.Empty(t, client.registered, "should not register when probe fails")
96+
}

pkg/oauth/provider.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,10 +152,14 @@ func (p *Provider) Run(ctx context.Context) {
152152
// Trigger refresh if needed
153153
if shouldTriggerRefresh {
154154
if IsCEMode() {
155-
// CE mode: Refresh token directly
155+
// CE mode: Refresh token directly, then reload server connection
156156
go func() {
157157
if err := p.refreshTokenCE(); err != nil {
158158
log.Logf("! Token refresh failed for %s: %v", p.name, err)
159+
return
160+
}
161+
if err := p.reloadFn(ctx, p.name); err != nil {
162+
log.Logf("! Failed to reload %s after token refresh: %v", p.name, err)
159163
}
160164
}()
161165
} else {

0 commit comments

Comments
 (0)