Credential storage for community servers#461
Conversation
saucow
left a comment
There was a problem hiding this comment.
Summary: Per-server credential routing for community servers. Mode enum, three-way branching in the credential helper, docker pass storage, provider refresh wiring. State model is correct across all combinations — CE, Desktop catalog, Desktop community with flag on/off, ModeAuto backward compat. All startProvider/NewProvider callers updated. Docker pass writes via CLI, reads via Secrets Engine. IsCommunity() on Server type is a nice touch. Preflight docker pass check in run.go done once, not per-server.
Two minor suggestions:
1. Reconcile ShouldUseGatewayOAuth and DetermineMode
PR 459 added ShouldUseGatewayOAuth(ctx, isCommunity) bool. This PR adds DetermineMode(ctx, isCommunity) Mode. Same decision tree, different return types. DetermineMode is what the new code actually uses — ShouldUseGatewayOAuth just sits there. As Tasks #4 and #5 wire up the remaining call sites, having both creates ambiguity about which to reach for.
ShouldUseGatewayOAuth should become a wrapper:
func ShouldUseGatewayOAuth(ctx context.Context, isCommunity bool) bool {
return DetermineMode(ctx, isCommunity) != ModeDesktop
}2. Mode logs as integer
run.go logs mode=%d which outputs mode=3. A String() method on Mode would help:
func (m Mode) String() string {
switch m {
case ModeDesktop: return "Desktop"
case ModeCE: return "CE"
case ModeCommunity: return "Community"
default: return "Auto"
}
}🤖 Review assisted by Claude Code
What I did
For Jira MCPT-482: Credential storage for community servers (docker pass)
docker pass, while catalog servers continue using Secrets Engine and CE mode continues using the credential helperOAuthModeenum (Desktop,CE,Community,Auto) andDetermineOAuthMode(ctx,isCommunity) to replace the globalIsCEMode()binary at per-server decision pointsdocker passread/write operations for OAuth tokens (docker/mcp/oauth/{server}) and DCR clients (docker/mcp/oauth-dcr/{server}) following the existingcredstore.gopatternThe credential storage routing for a given server follows this decision tree:
Both the
docker-passSecrets Engine plugin and the desktop-mcp-oauth plugin match thedocker/mcp/oauth/{server}key path. For community servers, only docker-pass has an entry (Desktop's OAuth manager never wrote one), so the Secrets Engine resolves correctly.For catalog servers, the Desktop plugin responds first (more specific pattern match).