Add dynamic OAuth discovery for community MCP servers#410
Add dynamic OAuth discovery for community MCP servers#410jchangx merged 5 commits intodocker:mainfrom
Conversation
jchangx
force-pushed
the
jchangx/community-oauth-flow-v039
branch
from
abc55ea to
cf09171
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: abc55eabac
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
jchangx
force-pushed
the
jchangx/community-oauth-flow-v039
branch
5 times, most recently
from
9c15228 to
4adff41
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4adff415d5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
jchangx
force-pushed
the
jchangx/community-oauth-flow-v039
branch
8 times, most recently
from
d7031f4 to
921ad1f
Compare
Community servers from third-party registries lack oauth.providers metadata, so the existing IsRemoteOAuthServer() gate prevents DCR entries from being created and OAuth tokens from being consumed. This adds support for the full dynamic OAuth lifecycle: discovery, token attachment, provider refresh, and connection invalidation. Discovery (DCR registration): - Add RegisterProviderForDynamicDiscovery() in pkg/oauth/dcr_registration.go - Add fallback branch in workingset, server enable, and gateway mcpadd Token consumption: - Attach stored OAuth tokens for community servers during Initialize (fallback when Spec.OAuth is nil but Remote.URL is set) - Start OAuth provider refresh loop for community servers with stored tokens - Fix InvalidateOAuthClients to match remote servers by name, not Spec.OAuth
Pass only the names that were actually removed from the profile to CleanupOrphanedDCREntries, instead of the full user-supplied list. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
jchangx
force-pushed
the
jchangx/community-oauth-flow-v039
branch
from
921ad1f to
4f4219c
Compare
- Guard against nil discovery response in registerProviderForDynamicDiscovery - Distinguish HTTP 404 from transient errors in DCR verification - Consolidate duplicate OAuth credential helper in remote.go - Simplify mcpadd OAuth condition to use serverConfig.IsRemote() - Remove extra blank line in server.go Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The consolidated check called GetOAuthToken for all remote servers, which logs errors for servers without stored tokens. Restore the two-branch structure so catalog servers (explicit OAuth metadata) log errors while community servers silently ignore missing tokens. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
saucow
left a comment
saucow
left a comment
There was a problem hiding this comment.
Overall looks good, mostly some suggestions to help with debugging and timeout values.
Not a blocker, but wondering if we should rename: IsRemoteOAuthServer -> something like: HasCatalogOAuthMetadata() or: HasExplicitOAuthProviders() since the criteria of remote OAuth servers is now being expanded.
- Remove redundant 10s probe timeout (discovery library uses 30s internally) - Log dynamic OAuth discovery failures instead of silently returning nil - Rename IsRemoteOAuthServer to HasExplicitOAuthProviders for clarity - Log TokenExists errors during community server provider startup Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2 participants
Summary
oauth.providersmetadata in their catalog entry, so the existingIsRemoteOAuthServer()gate prevents DCR entries from being createdRegisterProviderForDynamicDiscovery()which probes remote servers usingDiscoverOAuthRequirements()and creates pending DCR entries when OAuth is detecteddocker mcp server enable, and gatewaymcpaddoauth.providersTest plan
docker mcp catalog-next pull mcp/community-registrydocker mcp profile server add default --server catalog://mcp/community-registry/com-notion-mcpdocker mcp oauth lsdocker mcp oauth authorize com-notion-mcpdocker mcp oauth lsdocker mcp oauth revoke com-notion-mcpoauth.providersstill work (no regression)Confirming
com-notion-mcpfrom community registry successfully authorized via oauth:Confirming oauth flow for
com-notion-mcpfrom community registry: