Skip to content

Per-server OAuth routing for authorize, revoke, and mcp-add (MCPT-484)#465

Merged
cutecatfann merged 1 commit intomainfrom
mcpt_484
Apr 2, 2026
Merged

Per-server OAuth routing for authorize, revoke, and mcp-add (MCPT-484)#465
cutecatfann merged 1 commit intomainfrom
mcpt_484

Conversation

@cutecatfann
Copy link
Copy Markdown
Contributor

What I did

  • Route docker mcp oauth authorize, docker mcp oauth revoke, and the mcp-add elicitation flow per-server using DetermineMode(ctx, isCommunity), replacing the global IsCEMode() check at these call sites
  • Add authorizeCommunityMode, full Gateway OAuth flow (localhost callback, PKCE, DCR via docker pass) for community servers in Desktop mode
  • Add revokeCommunityMode, deletes OAuth token and DCR client from docker pass
  • Refactor getRemoteOAuthServerStatus in the gateway to branch on ShouldUseGatewayOAuth: community/CE servers register DCR and start providers via Gateway; Desktop catalog servers use the existing Desktop API path
  • Extract dcr.DiscoverAndRegister() as a storage-agnostic function so both community mode (docker pass) and CE mode (credential helper) can share the discovery + DCR logic without coupling to a specific storage backend

(not mandatory) A picture of a cute animal, if possible in relation to what you did
image

@cutecatfann cutecatfann self-assigned this Apr 2, 2026
@cutecatfann cutecatfann requested a review from a team as a code owner April 2, 2026 15:56
Comment on lines +20 to +29
isCommunity, err := lookupIsCommunity(ctx, app)
if err != nil {
// Server not in catalog -- fall back to legacy global routing
// so existing servers without catalog entries still work.
if pkgoauth.IsCEMode() {
return authorizeCEMode(ctx, app, scopes)
}
return authorizeDesktopMode(ctx, app, scopes)
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QQ, on the isCommunity check -> is this true for all servers that are not in the Docker catalog?

IE: how would this behave for a custom catalog built with servers from the DD catalog?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lookupIsCommunity loads all catalogs (Docker + configured/custom) via catalog.GetWithOptions(ctx, true, nil), then checks server.IsCommunity() which looks for the "community" tag in Metadata.Tags. That tag is only set by catalog_next/create.go when importing from the community registry.

So servers from the DD catalog in a custom catalog would not have the "community" tag -- they route to ModeDesktop (unchanged behavior).

The error path (server not in any catalog) falls back to the legacy global IsCEMode() check for backward compat with servers configured outside of catalogs.

}

// authorizeCommunityMode handles OAuth for community servers in Desktop mode.
// Uses the Gateway OAuth flow (localhost callback, PKCE) with docker pass storage.
Copy link
Copy Markdown
Contributor

@austin5456 austin5456 Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had understood that CE mode ( and therefore the community servers ) would use docker-credential-helpers for storage, are we migrating it to pass as well?
I think pass might require DD

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No -- CE mode is unchanged and still uses docker-credential-helpers via authorizeCEMode / NewReadWriteCredentialHelper().

authorizeCommunityMode (docker pass) is only reached when DetermineMode returns ModeCommunity, which requires: Desktop mode + community server + McpGatewayOAuth flag ON. CE mode always returns ModeCE from DetermineMode and routes to authorizeCEMode.

Copy link
Copy Markdown
Contributor

@austin5456 austin5456 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just a couple questions

@cutecatfann cutecatfann merged commit 88f82d1 into main Apr 2, 2026
8 checks passed
@cutecatfann cutecatfann deleted the mcpt_484 branch April 2, 2026 19:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants