Extend unmanaged OAuth flow to drive code exchange in-process

[trungutt]

Why

The current unmanaged OAuth flow (WithManagedOAuth(false), added in #830) emits an elicitation carrying only authorization-server metadata and expects the connected client to:

1. Generate PKCE + state
2. Run Dynamic Client Registration if needed
3. Open the browser
4. Run a callback server (or use a deeplink)
5. Exchange the code for a token at the auth server's token endpoint
6. Return the resulting {access_token, refresh_token, …} via ResumeElicitation

That contract works fine when the client is a Go process (the CLI mirror at pkg/runtime/remote_runtime.go:handleOAuthElicitation reuses the same helpers). It is a poor fit for embedders that want to be thin couriers — for example a host that registers a custom URI scheme and forwards the deeplink payload to docker-agent — because every such host ends up re-implementing PKCE / DCR / token exchange in the host language.

This branch extends the unmanaged flow with an opt-in sub-behavior where the runtime drives everything itself and the embedder just relays the deeplink payload back.

How the modes compare

Three OAuth flows are now available. The mode is picked per-toolset at flow-entry time:

flowchart TD
  start([handleOAuthFlow])
  managed{WithManagedOAuth true?}
  redir{mcp-oauth-redirect-uri set?}
  M[Managed flow]
  UClient[Unmanaged - client-driven]
  UDrive[Unmanaged - drive-flow NEW]

  start --> managed
  managed -- yes --> M
  managed -- no --> redir
  redir -- no --> UClient
  redir -- yes --> UDrive

Loading

Managed flow (unchanged)

The runtime owns the entire OAuth dance. A local callback server bound to 127.0.0.1:N receives the redirect.

sequenceDiagram
  participant agent as docker-agent
  participant client as MCP client
  participant browser as Browser
  participant authServer as Authorization Server

  agent->>agent: generate state and PKCE, run DCR if needed
  agent->>agent: start local callback server on 127.0.0.1
  agent->>client: elicitation asking to proceed with OAuth
  client-->>agent: accept
  agent->>browser: open authorize URL with localhost redirect_uri
  browser->>authServer: GET authorize
  authServer-->>browser: redirect to local callback with code and state
  browser->>agent: GET callback (hits local server)
  agent->>authServer: POST token with code, code_verifier, redirect_uri
  authServer-->>agent: access_token and refresh_token
  agent->>agent: store token in keychain

Loading

Unmanaged - client-driven (unchanged)

The runtime hands the client raw auth-server metadata and expects a finished token back.

sequenceDiagram
  participant agent as docker-agent
  participant client as MCP client
  participant browser as Browser
  participant authServer as Authorization Server

  agent->>client: elicitation with auth-server metadata
  client->>client: generate state and PKCE, run DCR, pick own redirect_uri
  client->>browser: open authorize URL
  browser->>authServer: GET authorize
  authServer-->>browser: redirect to client's redirect_uri
  browser->>client: deliver code and state
  client->>authServer: POST token with code, code_verifier, redirect_uri
  authServer-->>client: access_token and refresh_token
  client->>agent: ResumeElicitation with access_token and refresh_token
  agent->>agent: store token in keychain

Loading

Unmanaged - drive-flow (this PR)

The runtime owns PKCE, DCR, and token exchange and advertises an opaque redirect_uri. The client (or any process that received the deeplink) delivers { code, state } back. Two delivery paths are accepted; either or both work in a given deployment.

sequenceDiagram
  participant agent as docker-agent
  participant client as MCP client
  participant browser as Browser
  participant authServer as Authorization Server
  participant relay as Deeplink relay

  agent->>agent: generate state and PKCE, run DCR if needed
  agent->>agent: build authorize URL with configured redirect_uri
  agent->>agent: register state-to-waiter in registry
  agent->>client: elicitation with authorize_url and state
  client->>browser: open authorize_url
  browser->>authServer: GET authorize
  authServer-->>browser: redirect to configured redirect_uri with code and state

  alt Path A - out-of-band callback (new)
    browser->>relay: deeplink or URI-scheme handler
    relay->>agent: POST /api/mcp-oauth/callback with state and code
    agent->>agent: lookup state in registry, hand payload to waiter
  else Path B - elicitation reply
    browser->>client: deliver code and state
    client->>agent: ResumeElicitation with code and state
  end

  agent->>agent: verify state in constant time
  agent->>authServer: POST token with code, code_verifier, redirect_uri
  authServer-->>agent: access_token and refresh_token
  agent->>agent: store token plus client_id and secret for silent refresh

Loading

What changes

1. Runtime-driven OAuth flow (server mode)

A new server-level flag — --mcp-oauth-redirect-uri=<URL> (also MCPOAuthRedirectURI on RuntimeConfig and WithUnmanagedOAuthRedirectURI on the runtime). When set, handleUnmanagedOAuthFlow:

• Generates state + PKCE in-process
• Runs DCR if the auth server advertises a registration endpoint (or uses the per-toolset clientId/clientSecret from RemoteOAuthConfig)
• Builds the full authorize URL with the configured redirect_uri
• Emits an elicitation whose Meta includes:
  • docker-agent/authorize_url — URL the client should open in the browser
  • docker-agent/state — state value the client must echo back
  • plus the existing docker-agent/server_url, auth_server, auth_server_metadata, resource_metadata
• Accepts {code, state} as a ResumeElicitation payload, verifies state in constant time, exchanges the code at the token endpoint (same redirect_uri per RFC 6749 §4.1.3), and stores the resulting token in the keychain with client credentials stamped on for silent refresh

When the flag is not set, the elicitation shape stays identical to today and the runtime expects the client-driven {access_token, …} reply. CLI-mirror clients continue to work unchanged.

The new path also accepts the client-driven {access_token, …} reply, so a client that prefers to do the exchange itself is still free to.

A per-toolset RemoteOAuthConfig.CallbackRedirectURL overrides the runtime-wide flag for that toolset.

2. Out-of-band callback route — POST /api/mcp-oauth/callback

Adds a second, session-less way to deliver the deeplink payload to the runtime:

POST /api/mcp-oauth/callback?state=<opaque>
                            [&code=<opaque>]
                            [&error=<rfc6749 code>]
                            [&error_description=<text>]

The parameters mirror what an authorization server sends to a redirect URI per RFC 6749 §4.1.2. The body is empty; everything travels in the query string.

A process-wide pending-OAuth registry maps state → waiting unmanaged flow. handleUnmanagedOAuthFlow registers its waiter on entry (drive-flow only) and unregisters on exit. The new route looks up the waiter by state and hands it the payload through a single-shot buffered channel. The waiting goroutine then performs the token exchange and persists the token exactly as before.

The lookup-by-state IS the authentication: only states the runtime has just generated are present in the registry. Unknown or replayed states return 404. State values continue to come from GenerateState (≥128 bits, opaque, base64).

This route is strictly additive. The existing session-keyed POST /api/sessions/:id/elicitation path still works; embedders may use the elicitation path, the new callback path, or both.

3. Propagate user-initiated cancellation across the WithoutCancel boundary

clientConnector.Connect detaches the caller's ctx with context.WithoutCancel so that the MCP toolset's session can outlive any single request (otherwise the SSE/stdio connection tears down when the request that triggered toolset.Start returns, and a successful OAuth flow gets killed at request end).

The detachment also severs user-initiated cancellation, which the OAuth select inside Connect does need to observe — without that signal, embedders that abort the in-progress run while the user is in the OAuth dialog would leave the OAuth select blocked indefinitely, the per-session streaming lock held, and the next message returning 409 Conflict / ErrSessionBusy.

The connection-longevity invariant is preserved by stashing the caller's original ctx as a value on the detached one. The detached ctx remains the primary driver of the connection's lifetime; operations that want to observe user-initiated cancellation opt in by reading the parent back out and selecting on its Done channel.

Carrying ctx-as-value is unusual but appropriate here: the parent is metadata about how the detached ctx was constructed, not data the request is operating on. The helpers (withCancellableParent / cancellableParentFromContext) live next to the connector that creates the relationship so the rationale stays close to the code.

[docker-agent]

❌ PR Review Failed — The review agent encountered an error and could not complete the review. View logs.

[docker-agent]

Assessment: 🟡 NEEDS ATTENTION

This PR correctly implements the new drive-flow OAuth mode with PKCE, DCR, and token exchange in-process, and the pending-OAuth registry with its channel-based waiter is race-safe. Three medium-severity issues were identified in the newly added code that are worth addressing before merge.

[docker-agent]

[MEDIUM] State validation tautology on the deeplink callback path

When the out-of-band deeplink wins the select race, the code synthesises the payload that consumeUnmanagedElicitationReply expects by pinning state to the local expectedState:

content = map[string]any{"code": cb.Code, "state": expectedState}

consumeUnmanagedElicitationReply then extracts state from that map and does:

if subtle.ConstantTimeCompare([]byte(state), []byte(expectedState)) != 1 {
    return nil, errors.New("state mismatch in elicitation reply")
}

Because state == expectedState by construction, this comparison is always true — the ConstantTimeCompare guard is a no-op for the deeplink path. The actual state that arrived from the HTTP callback is never independently validated inside the flow.

The registry provides the primary authentication (only a POST carrying the exact state the runtime registered can deliver to the channel), so this is not directly exploitable as-is. However:

• The per-flow check gives a false sense of security — reviewers and future maintainers may think state is re-validated from the callback, when it is not.
• If PendingOAuthCallback ever grows a State field (e.g. for logging), the same pattern would silently skip validating it.
• Defence-in-depth is weakened: any future change that loosens the registry's state-key strictness would remove the only real protection.

Suggestion: Either remove the ConstantTimeCompare from the deeplink branch entirely (making the code honest), or add a State field to PendingOAuthCallback, populate it from the HTTP query param in mcpOAuthCallback, and validate it here instead of synthesising the expected value.

[docker-agent]

[MEDIUM] /api/mcp-oauth/callback is unauthenticated when no bearer token is configured

The endpoint is registered under the group that only has BearerTokenMiddleware when authToken != "":

if authToken != "" {
    e.Use(BearerTokenMiddleware(authToken))
}

When the server runs without --auth-token (a documented/valid configuration), any network-reachable client can POST to /api/mcp-oauth/callback?state=<s>&code=<c> with no credentials.

The pending-OAuth registry provides meaningful defence: only states the runtime has generated and is currently awaiting are accepted (unknown states → 404). However, if an attacker can observe a live state value (it appears in the elicitation meta sent to the client over the session stream, at debug-level logs, and in the authorize URL the client is asked to open), they can inject a malicious authorization code. The token exchange would then proceed with an attacker-controlled code, potentially yielding the attacker's own tokens bound to the victim's session.

Suggestions:

• This route is not a "deeplink receiver" in the same sense as the OAuth redirect URI on the authorization server — it is a programmatic API call made by the embedder. Requiring an auth token for it is reasonable even in deployments where other API calls are also authenticated.
• At minimum, add a comment explaining the threat model (what the registry guards against, what it does not) so operators know the risk of running without an auth token.
• Consider rate-limiting or adding an explicit warning in the docs that running without --auth-token weakens the callback endpoint's security posture.

[docker-agent]

[MEDIUM] Indefinite block in handleUnmanagedOAuthFlow if the elicitation goroutine hangs and the parent context never cancels

ctx is the WithoutCancel-detached context set up in clientConnector.Connect — its Done() channel never fires on its own. The select therefore has only three live cancellation signals:

Case	When it fires
ctx.Done()	Never (WithoutCancel)
userCancelCh	Only if the embedder actively cancels the parent ctx
elicCh	Only when requestElicitation returns

If the MCP client disconnects silently (TCP RST, idle timeout, process kill) and requestElicitation has no internal deadline, the goroutine and the entire select will block indefinitely. The per-session streaming lock at the SessionManager level is then held forever: all subsequent messages from that session return 409 Conflict / ErrSessionBusy until a server restart.

callbackCh (the deeplink path) also provides no relief — it will never fire if the client that was supposed to open the browser is already gone.

Suggestion: Add a context with a reasonable deadline around the elicitation wait, e.g.:

elicCtx, elicCancel := context.WithTimeout(ctx, 10*time.Minute)
defer elicCancel()

Or, if the elicitation itself must be open-ended, wrap the outer select with the same timeout so the streaming lock is released even if no reply arrives. The choice of timeout can be made configurable via RuntimeConfig.

[dgageot]

Docs reference outdated metadata keys: cagent/type, cagent/server_url, cagent/authorize_url, cagent/state, but the code emits docker-agent/.... Embedders following the docs would look up the wrong keys — please update the docs to match the implementation before merging.

[dgageot]

@trungutt Let's merge this one and iterate