Skip to content

feat(mcp): standalone delegated consent — type:user without a platform (#332)#344

Merged
initializ-mk merged 2 commits into
mainfrom
feat/mcp-standalone-consent-332
Jul 19, 2026
Merged

feat(mcp): standalone delegated consent — type:user without a platform (#332)#344
initializ-mk merged 2 commits into
mainfrom
feat/mcp-standalone-consent-332

Conversation

@initializ-mk

Copy link
Copy Markdown
Contributor

Implements #332 — standalone (no-platform) delegated MCP consent. Closes the standalone half of the #317 arc; the Forge-driven Slack delivery layer stays in #343.

What it does

A type: user MCP server with grant: authorization_code and explicit endpoints now runs the per-user OAuth in Forge itself — no platform token endpoint required. End to end:

grantless type:user call → parks on the auth-required gate (#330)
  → deliver a "Connect <server>" link on the task's A2A auth-required artifact
  → GET /mcp/oauth/start  (sets forge_session cookie, 302 → IdP)
  → user consents → GET /mcp/oauth/callback (validate state+session, exchange code)
  → token cached per-subject in SubjectStore → parked call resumes → resolver finds the grant

Before this, type: user was construction-rejected without a platform block (server.go:192); standalone delegated identity didn't exist.

Design decisions (confirmed with maintainer)

  • Enable signal: type: user + no platform block + grant: authorization_code. Requires explicit authorize_url/token_url/client_id (no runtime discovery); client_credentials is rejected.
  • Callback host: new server.public_url field, falling back to the existing AGENT_URL env var → redirect_uri = <public_url>/mcp/oauth/callback.

Notable: the session producer (GET /mcp/oauth/start)

The #330 callback mandates a matching forge_session (fail-closed) but nothing produced it — the browser only ever touches Forge at the callback. /mcp/oauth/start is that producer: it plants the cookie (SameSite=Lax so it survives the IdP round-trip) and redirects to the IdP. The session lives only in the server-side binding + cookie — never a URL query — so it can't leak to the IdP via Referer (unlike the state, which round-trips by design). This is the one piece beyond the issue's "reuse these seams" list; it's what makes the existing session-bound callback usable on a network-exposed agent.

Layering

  • forge-core/mcpServerConfig.PublicURL; SubjectStore on Manager/ServerDeps; standalone branch in NewServer + buildAuthFn; exported BuildAuthorizeURL + NewInMemorySubjectTokenStore.
  • forge-core/validate — standalone accept/reject rules.
  • forge-cli/runtimestateBinder (verifier + authorizeURL, Bind/Peek); /mcp/oauth/start; CallbackCompleter(ctx,…,verifier); front-half builder + completer + A2A-artifact deliverer, auto-wired by enableStandaloneConsent; SetAuthorizeURLProvider seam for Forge-driven Slack delivery of MCP consent prompts (channel-adapter ConsentDeliverer) #343.

Tests

  • Standalone resolver: ErrNoToken on miss, token on hit, per-subject isolation; NewServer accept/reject matrix.
  • validate accept/reject (endpoints, grant, managed-skips).
  • stateBinder Bind/Peek; /start cookie + redirect + bad-state 400.
  • Completer exchanges (code + verifier + redirect_uri) → caches per subject.
  • Deliverer writes the auth-required artifact (link + structured data part).
  • Managed mode (platform block) leaves the standalone loop unwired.

All forge-core/{mcp,validate,types} and forge-cli/{runtime,cmd} suites green; lint 0 issues.

Follow-ups (not in this PR)

#332)

Standalone (no-platform) delegated identity: a `type: user` MCP server with
`grant: authorization_code` and explicit endpoints now runs the per-user OAuth
in Forge itself instead of requiring a platform token endpoint.

forge-core:
- ServerConfig.PublicURL — the agent's external base URL for redirect_uri.
- Manager/ServerDeps.SubjectStore — the per-subject token cache the standalone
  resolver reads and the consent callback writes.
- NewServer accepts standalone type:user (no platform block) when grant is
  authorization_code and authorize_url/token_url/client_id are explicit; the
  buildAuthFn user-case branches managed (platform) vs standalone (reads
  SubjectStore, ErrNoToken on miss → trips the auth-required gate).
- validate mirrors the standalone rules (endpoints + grant).
- Exported BuildAuthorizeURL and NewInMemorySubjectTokenStore for the runtime.

forge-cli:
- stateBinder carries the PKCE verifier + authorize URL; adds Bind + Peek.
- New GET /mcp/oauth/start — the session producer the callback's mandatory
  cross-session guard requires: it plants the forge_session cookie (SameSite=Lax,
  never in a URL so it can't leak to the IdP via Referer) and 302s to the IdP.
- CallbackCompleter now takes (ctx, subject, server, code, verifier) so the
  exchange uses the state-bound verifier and a request-scoped deadline.
- Standalone front-half: buildStandaloneConsentLink (PKCE+state+session+Issue),
  the code→token completer (ExchangeCodeCtx → SubjectStore.Put), and the
  A2A-artifact ConsentDeliverer that publishes the login link on the parked
  task. All auto-wired by enableStandaloneConsent when a standalone type:user
  server is configured; managed deployments are untouched.
- SetAuthorizeURLProvider seam so #343's Slack delivery can supply the link in
  both standalone (Forge builds) and managed (platform supplies) modes.

Tests: standalone resolver (ErrNoToken/hit/isolation) + NewServer validation;
validate accept/reject; stateBinder Bind/Peek; /start cookie+redirect; completer
exchange→store; deliverer artifact; managed-mode no-op. Docs: standalone
type:user section in docs/mcp/configuration.md.

@initializ-mk initializ-mk left a comment

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.

Review — standalone delegated consent (#332)

Request changes. One blocking bug that breaks the feature in exactly the deployment it targets, plus one design question worth an explicit decision. The OAuth mechanics themselves are correct and well-tested — PKCE bound to state and replayed at exchange, one callbackRedirectURI() feeding both the authorize URL and the token exchange, exchange routed through the egress allowlist, in-memory-only per-subject tokens keyed off the authenticated identity, and a single Platform.TokenEndpoint != "" switch used identically across the resolver, NewServer, validate, and standaloneDelegatedServers so config-validation, construction, and runtime wiring can't disagree.

🔴 [High / Blocking] GET /mcp/oauth/start is behind auth — the flow can't start in production

Inline on the /start registration. The new browser-facing endpoint is registered behind the auth middleware but was never added to auth.DefaultSkipPaths() (only the callback is exempt), so the user's anonymous browser is 401'd before the handler runs. Breaks the flow at its first hop in every auth-enforcing deployment — i.e. every real multi-user agent. CI is green because TestStandaloneConsent_LinkAndStart calls makeMCPStartHandler directly, bypassing the middleware — the same shape as the #331 callback-exemption bug and the #323 mcp login bypass.

🟡 [Medium / Design] What does the session binding actually buy in standalone mode?

Inline on the cookie set in makeMCPStartHandler. The consent browser is anonymous, and /start plants forge_session for anyone holding the link — so the cookie proves browser continuity, not that the completer is the parked subject. A leaked link lets an attacker complete consent with their own IdP account and have that token filed under the victim's subject (confused deputy). Bounded by single-use + short-TTL state and authenticated-channel delivery, so it's delivery-gated rather than open — but the naming reads stronger than the guarantee. Worth a decision + doc note.

🟢 Verified solid

PKCE / redirect_uri consistency / egress-routed exchange / in-memory per-subject isolation / the four-site standalone-vs-managed switch / the NewServer reject matrix (missing endpoints, client_credentials, missing SubjectStore, required:true). Thorough tests on everything that runs through a real code path.

if r.stateBinder == nil {
r.stateBinder = newStateBinder(defaultStateTTL)
}
srv.RegisterHTTPHandler("GET /mcp/oauth/start", makeMCPStartHandler(r.stateBinder))

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.

[High / Blocking] This endpoint is registered behind the auth middleware but isn't exempted — the standalone consent flow can't start in any auth-enforcing deployment.

Both /mcp/oauth/start (here) and /mcp/oauth/callback (next line) are wrapped by the auth middleware, but auth.DefaultSkipPaths() (forge-core/auth/middleware.go) exempts only the callback:

"GET /mcp/oauth/callback":     true,
"OPTIONS /mcp/oauth/callback": true,
// GET /mcp/oauth/start  ← missing

Matching is an exact lookup (skip[r.Method+" "+r.URL.Path]), and the runner uses DefaultSkipPaths() unmodified at all three sites (runner.go:3029/3071/3078). So when the user clicks the delivered "Connect" link, their anonymous browser (no bearer token) hits GET /mcp/oauth/startwriteAuthError(...)401 before the handler runs. The flow dies at its first hop in every deployment that enforces auth — which is the entire point of delegated per-user consent. (Works only under --no-auth.)

/start is safe to exempt for the same reason the callback is: its authenticity rests on the Peek'd session-bound state (it rejects any binding lacking authorizeURL+session), not a bearer token — and it is not an open redirect (b.authorizeURL is server-built from config, reachable only via a Forge-minted state).

Why CI didn't catch it: TestStandaloneConsent_LinkAndStart calls makeMCPStartHandler(...) directly, so it never exercises the middleware — green handler, broken wiring. Same shape as the #331 callback bug and the #323 mcp login bypass.

Fix: in DefaultSkipPaths() add

"GET /mcp/oauth/start":     true,
"OPTIONS /mcp/oauth/start": true, // symmetry; GET is the load-bearing one

and add a middleware-level test asserting /start is reachable unauthenticated and that POST /mcp/consent still requires auth (the both-directions test #331 added for the callback).

}
http.SetCookie(w, &http.Cookie{
Name: forgeSessionCookie,
Value: b.session,

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.

[Medium / Design question] In standalone mode this cookie proves browser continuity, not that the completer is the parked subject.

The consent browser is anonymous until it reaches /start, and /start plants forge_session = b.session for whoever presents the link. So the callback's session match only guarantees the same browser did /start and /callback across the IdP round-trip — it does not tie the flow to the authenticated identity that triggered the park.

Consequence: if the consent link leaks, an attacker can hit /start, authenticate at the IdP as themselves, and the callback files their token under the victim's b.subject (fixed server-side from the parked call). The victim's resumed call then runs against the attacker's account — a confused-deputy / token-fixation vector (attacker data flows into the victim's agent, or victim data is written to the attacker's account).

This is bounded: the state is single-use + short-TTL and the link is delivered on the authenticated A2A artifact (Slack DM in #343), so it's delivery-gated rather than open. Raising it because "session-bound cross-session guard" reads stronger than the standalone guarantee — the flow structurally can't bind the browser to the parked subject. Worth an explicit decision + a doc note (like #339 documented its spoofing limitation). The tamper-proof-but-heavy alternative is verifying the IdP userinfo identity against b.subject at exchange time.

…del (#344 review)

[High/Blocking] GET /mcp/oauth/start was registered behind the auth
middleware but never added to auth.DefaultSkipPaths(), so the user's
anonymous browser was 401'd at the flow's first hop in every auth-enforcing
deployment (CI missed it because the test drove makeMCPStartHandler directly).
Exempt GET + OPTIONS /mcp/oauth/start alongside the callback — its
authenticity is the Peek'd session-bound state, not a bearer token, and it is
not an open redirect (authorizeURL is server-built, reachable only via a
Forge-minted state). Added a middleware-level test asserting /start is public
(the both-directions pin already covers POST /mcp/consent still requiring auth).

[Medium/Design] Documented the standalone trust model: the anonymous consent
browser means the session cookie proves browser CONTINUITY across the IdP
round-trip, not that the completing user is the parked subject — a leaked link
is a confused-deputy / token-fixation vector, contained by authenticated-channel
delivery + single-use short-TTL state. Reframed the overclaiming "cross-session
guard" comments and added a Trust model note to docs/mcp/configuration.md;
noted the heavier IdP-userinfo-verification alternative.
@initializ-mk

Copy link
Copy Markdown
Contributor Author

Both addressed in 8adf5ce.

🔴 [High/Blocking] /mcp/oauth/start behind auth — fixed. Exact catch, thank you. Added GET/OPTIONS /mcp/oauth/start to auth.DefaultSkipPaths() (right beside the callback), with the same justification: authenticity is the Peek'd session-bound state, not a bearer token, and it's not an open redirect (b.authorizeURL is server-built from config, reachable only via a Forge-minted state). Added the middleware-level pin — GET /mcp/oauth/start is public — which exercises the real middleware (not makeMCPStartHandler directly); the existing POST /mcp/consent requires auth case keeps the both-directions guard.

🟡 [Medium/Design] Session binding overclaims — documented + reframed. Took the documented-limitation decision (matching the #339 precedent you cited), since the flow structurally can't bind the anonymous browser to the parked subject:

  • Added a Trust model / limitation note to docs/mcp/configuration.md: the consent link is a bearer capability; the cookie proves browser continuity across the round-trip, not subject identity; a leaked link is a confused-deputy / token-fixation vector; containment is authenticated-channel delivery (the A2A artifact in the user's own session; the Slack DM in Forge-driven Slack delivery of MCP consent prompts (channel-adapter ConsentDeliverer) #343) + single-use short-TTL state. "Treat the link as a secret."
  • Reframed the code comments (makeMCPStartHandler + the callback guard, renamed "cross-session" → "session-continuity") to state exactly what's guaranteed and what isn't.
  • Recorded the heavier alternative (verify IdP userinfo == b.subject at exchange) as deferred hardening; managed mode sidesteps it (platform owns the callback + token custody).

Local: forge-core/auth + forge-cli/runtime (incl. the standalone + callback suites) green, lint clean. Watching CI.

@initializ-mk initializ-mk left a comment

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.

Re-review — fix commit 8adf5ce

Both findings resolved; nothing new introduced (skip-paths + tests + comments/docs only).

🔴 [High/Blocking] /mcp/oauth/start behind auth — fixed, and pinned at the right layer

DefaultSkipPaths() now exempts GET and OPTIONS /mcp/oauth/start; all three runner sites call DefaultSkipPaths() so they pick it up automatically. The important part is where the new test sits — it drives the real middleware, not the handler in isolation:

name: "GET /mcp/oauth/start is public (anonymous browser link click)"
opts: MiddlewareOptions{Chain: chain, SkipPaths: DefaultSkipPaths()}  → 200

That's exactly the wired-path assertion whose absence let the bug ship green — and the opposite-direction case (POST /mcp/consent must NOT be exempt) is retained, so the both-directions invariant is now pinned for /start too. The regression that was invisible before is now the thing the test catches.

🟡 [Medium/Design] Session trust model — addressed correctly for a design finding

Keep the mechanism, document it precisely, defer the heavier hardening — and both the overclaim and the missing decision are fixed:

  • The callback comment is renamed "Cross-session guard" → "Session-continuity guard" and now states outright it proves browser continuity across the round-trip, not that the completer is b.subject.
  • A new "Trust model / limitation (standalone)" doc block names the link a bearer capability, spells out the confused-deputy / token-fixation vector, states the containment (treat the link as a secret + single-use short-TTL + authenticated-channel delivery), and records the deferred tamper-proof alternative (verify IdP userinfo == subject), noting managed mode sidesteps it entirely.

Test + Lint green at time of posting (Integration still running; change is test-only + a map entry, prior commit was green across all 10 checks). Approve — ready to merge once Integration lands.

@initializ-mk
initializ-mk merged commit be88da3 into main Jul 19, 2026
10 checks passed
initializ-mk added a commit that referenced this pull request Jul 19, 2026
When a type: user MCP call parks on the auth-required gate, Forge now presents
a "Connect <server>" login link to the requesting user over its existing Slack
connection, in both standalone and managed modes. Delivery is decoupled from
token custody — the same Slack code serves both.

forge-core:
- channels.ConsentDeliverer capability (DeliverConsent + SetConsentCanceler),
  ConsentPrompt{Subject,Server,AuthorizeURL,Deadline,Origin}, ChannelOrigin,
  ConsentCanceler — mirrors ApprovalDeliverer.
- PlatformConfig.AuthorizeEndpoint + mcp.FetchAuthorizeURL: the managed
  consent-URL contract — POST {server, subject} (Bearer agent identity +
  tenancy headers) → {authorize_url}. The platform builds the URL with its own
  client_id/redirect_uri/state and hosts the callback, so the code + refresh
  token never reach Forge.

forge-plugins/slack:
- consent.go: DeliverConsent DMs the subject (users.lookupByEmail →
  conversations.open → chat.postMessage) or replies in the Slack origin thread;
  Block Kit Connect URL button + optional Cancel; Cancel routes through the
  existing block-action dispatch to the wired canceler. Reuses the socket-mode
  connection + bot token (adds im:write scope).

forge-cli:
- Runner.AuthorizeURL resolves the link via a provider — standalone builds it
  (#332), managed fetches it from the platform, an embedder may inject its own
  (SetAuthorizeURLProvider takes precedence). enableManagedConsentProvider
  auto-wires the managed HTTP provider; the authorize_endpoint host is merged
  into the egress allowlist.
- cmd/run.go wires SetConsentDeliverer + SetConsentCanceler to the first active
  consent-capable adapter (mirrors the DEFER block); postMCPConsent helper for
  the Cancel path. No adapter → falls back to the A2A artifact / audit event.

Tests: Slack payload shape, DM-by-email + origin-thread delivery, cancel
routing; FetchAuthorizeURL contract; managed provider fetch + embedder
precedence. Docs: managed consent delivery + authorize_endpoint in
docs/mcp/configuration.md.

Stacked on #344 (uses the SetAuthorizeURLProvider seam).
initializ-mk added a commit that referenced this pull request Jul 19, 2026
When a type: user MCP call parks on the auth-required gate, Forge now presents
a "Connect <server>" login link to the requesting user over its existing Slack
connection, in both standalone and managed modes. Delivery is decoupled from
token custody — the same Slack code serves both.

forge-core:
- channels.ConsentDeliverer capability (DeliverConsent + SetConsentCanceler),
  ConsentPrompt{Subject,Server,AuthorizeURL,Deadline,Origin}, ChannelOrigin,
  ConsentCanceler — mirrors ApprovalDeliverer.
- PlatformConfig.AuthorizeEndpoint + mcp.FetchAuthorizeURL: the managed
  consent-URL contract — POST {server, subject} (Bearer agent identity +
  tenancy headers) → {authorize_url}. The platform builds the URL with its own
  client_id/redirect_uri/state and hosts the callback, so the code + refresh
  token never reach Forge.

forge-plugins/slack:
- consent.go: DeliverConsent DMs the subject (users.lookupByEmail →
  conversations.open → chat.postMessage) or replies in the Slack origin thread;
  Block Kit Connect URL button + optional Cancel; Cancel routes through the
  existing block-action dispatch to the wired canceler. Reuses the socket-mode
  connection + bot token (adds im:write scope).

forge-cli:
- Runner.AuthorizeURL resolves the link via a provider — standalone builds it
  (#332), managed fetches it from the platform, an embedder may inject its own
  (SetAuthorizeURLProvider takes precedence). enableManagedConsentProvider
  auto-wires the managed HTTP provider; the authorize_endpoint host is merged
  into the egress allowlist.
- cmd/run.go wires SetConsentDeliverer + SetConsentCanceler to the first active
  consent-capable adapter (mirrors the DEFER block); postMCPConsent helper for
  the Cancel path. No adapter → falls back to the A2A artifact / audit event.

Tests: Slack payload shape, DM-by-email + origin-thread delivery, cancel
routing; FetchAuthorizeURL contract; managed provider fetch + embedder
precedence. Docs: managed consent delivery + authorize_endpoint in
docs/mcp/configuration.md.

Stacked on #344 (uses the SetAuthorizeURLProvider seam).
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.

1 participant