diff --git a/docs/mcp/configuration.md b/docs/mcp/configuration.md index 01f5d07..bfb3da7 100644 --- a/docs/mcp/configuration.md +++ b/docs/mcp/configuration.md @@ -68,7 +68,7 @@ MCPs). | `bearer` | `token_env` | In-cluster sidecars; CI machine-to-machine | | `static` | `token_env` | Same as bearer; named for clarity | | `platform` | top-level `platform` block | **Managed:** agent-principal (service) identity resolved by the platform | -| `user` | top-level `platform` block | **Managed:** delegated per-requesting-user identity, resolved by the platform | +| `user` | top-level `platform` block **or** explicit endpoints (standalone) | Delegated per-requesting-user identity — **managed** (platform-resolved) or **standalone** (Forge runs the OAuth itself, #332) | `token_env` is the name of an environment variable; the variable's value is read at runtime — never stored in `forge.yaml`. @@ -125,9 +125,69 @@ mcp: > B's. Connections are established lazily (first call) and per subject. > The platform materializes both the `platform` block and the per-identity -> server entries (same URL, split by `type`). For the **standalone** (no -> platform) equivalents, use `type: oauth` — 3-legged `forge mcp login` for a -> user, or `grant: client_credentials` for an agent-principal (above). +> server entries (same URL, split by `type`). For a standalone agent-principal, +> use `type: oauth` with `grant: client_credentials` (above); for standalone +> **per-user** delegation, use standalone `type: user` (below). + +#### Standalone delegated consent — `type: user` without a platform (#332) + +When there is **no** `platform` block, `type: user` runs the delegated OAuth +**in Forge itself**: a grantless call parks on the auth-required gate, the user +is shown a "Connect" login link, they consent in the browser, and Forge stores +their per-user access token in memory (no disk) so the call resumes. + +```yaml +server: + public_url: https://agent.example.com # or the AGENT_URL env var — see below + +mcp: + servers: + - name: atlassian + url: https://mcp.atlassian.com/mcp + auth: + type: user + # standalone requires explicit endpoints + client_id (no runtime discovery): + client_id: ${ATLASSIAN_CLIENT_ID} + authorize_url: https://auth.atlassian.com/authorize + token_url: https://auth.atlassian.com/oauth/token + scopes: [read:jira-work, write:jira-work] + required: false + tools: { allow: ["*"] } +``` + +- **`grant`** is `authorization_code` (the default) — `client_credentials` is + rejected for standalone `type: user`. +- **Explicit endpoints + `client_id` are required.** Standalone does not run + discovery/registration at runtime, so `authorize_url`, `token_url`, and + `client_id` must be set. (Add a `platform` block instead for managed + delegation, which resolves tokens without any of these.) +- **`server.public_url`** (falling back to the `AGENT_URL` env var) is the + agent's externally-reachable base URL. Forge builds `redirect_uri = + /mcp/oauth/callback`, so the URL must be reachable by the user's + browser after IdP consent. +- **Delivery.** The login link is published on the parked task's A2A + `auth-required` artifact (a UI/A2A client renders it). Forge-driven Slack + delivery of the same link is tracked separately (#343). +- **Endpoints registered** (only in standalone mode): `GET /mcp/oauth/start` + (sets a `forge_session` cookie, then redirects to the IdP) and + `GET /mcp/oauth/callback` (validates state + session, exchanges the code, + resumes the parked call). Both are auth-exempt (anonymous browser hops); their + authenticity rests on the single-use, expiring, session-bound state — see the + [auth-required gate](#delegated-consent--the-auth-required-gate-330). + +> **Trust model / limitation (standalone).** The consent link is a **bearer +> capability**: the browser that completes it is anonymous, so the session +> cookie only proves the *same browser* did `/start` and `/callback` across the +> IdP round-trip — it does **not** prove the completing user is the parked +> subject. If the link leaks, an attacker could authenticate at the IdP as +> *themselves* and have that token filed under the victim's subject (a +> confused-deputy / token-fixation vector). This is bounded by the single-use, +> short-TTL state and, above all, by **delivering the link only over an +> authenticated channel** to the requesting user (the A2A `auth-required` +> artifact in their own session; the Slack DM in #343). Treat the link as a +> secret. The tamper-proof (heavier) alternative — verifying the IdP `userinfo` +> identity against the parked subject at exchange time — is deferred; managed +> mode sidesteps it entirely (the platform owns the callback and token custody). #### Materialized tool schemas (#317) diff --git a/forge-cli/runtime/mcp_consent_callback.go b/forge-cli/runtime/mcp_consent_callback.go index e116168..5358ad2 100644 --- a/forge-cli/runtime/mcp_consent_callback.go +++ b/forge-cli/runtime/mcp_consent_callback.go @@ -1,7 +1,9 @@ package runtime import ( + "context" "net/http" + "strings" "sync" "time" @@ -26,10 +28,18 @@ import ( // stateBinding is what an issued OAuth `state` is bound to. The callback // must match all of it (modulo an empty session) to be honored. type stateBinding struct { - subject string - server string - session string // the initiating session; callback must match (cross-session guard) - deadline time.Time + subject string + server string + session string // the initiating session; callback must match (cross-session guard) + // verifier is the PKCE code_verifier minted when the authorize URL was + // built; the callback needs it to exchange the code (#332). Empty for + // managed flows (Forge never sees the code) or where PKCE isn't used. + verifier string + // authorizeURL is the IdP authorize URL this state was minted for. The + // GET /mcp/oauth/start endpoint redirects the browser here after setting + // the session cookie (#332). Empty when Forge doesn't front the redirect. + authorizeURL string + deadline time.Time } // stateBinder issues single-use, expiring OAuth `state` values bound to the @@ -56,7 +66,8 @@ func newStateBinder(ttl time.Duration) *stateBinder { } // Issue mints a cryptographically-random state bound to {subject, server, -// session} and returns it for embedding in the authorize URL. +// session} and returns it for embedding in the authorize URL. Used where the +// caller doesn't front the redirect (no verifier/authorizeURL captured). func (s *stateBinder) Issue(subject, server, session string) (string, error) { state, err := oauth.GenerateState() if err != nil { @@ -69,6 +80,38 @@ func (s *stateBinder) Issue(subject, server, session string) (string, error) { return state, nil } +// Bind stores a fully-formed binding under a caller-provided state. The +// standalone front-half (#332) generates the state itself (it must embed it in +// the authorize URL before binding), then records the session, PKCE verifier, +// and authorize URL here so GET /mcp/oauth/start can set the session cookie and +// redirect, and GET /mcp/oauth/callback can complete the exchange. +func (s *stateBinder) Bind(state, subject, server, session, verifier, authorizeURL string) { + s.mu.Lock() + defer s.mu.Unlock() + s.sweepLocked() + s.m[state] = stateBinding{ + subject: subject, server: server, session: session, + verifier: verifier, authorizeURL: authorizeURL, + deadline: s.now().Add(s.ttl), + } +} + +// Peek returns the binding for state WITHOUT consuming it (unlike Consume), +// for the GET /mcp/oauth/start redirect: the state is spent later, at the +// callback. Returns ok=false for unknown or expired states. +func (s *stateBinder) Peek(state string) (stateBinding, bool) { + if state == "" { + return stateBinding{}, false + } + s.mu.Lock() + defer s.mu.Unlock() + b, ok := s.m[state] + if !ok || s.now().After(b.deadline) { + return stateBinding{}, false + } + return b, true +} + // Consume looks up and REMOVES the binding for state. It returns ok=false // for an unknown state (forged), an already-used state (replay — the entry // is gone after the first Consume), or an expired one. Single-use + expiry @@ -100,12 +143,13 @@ func (s *stateBinder) sweepLocked() { } } -// CallbackCompleter exchanges an OAuth authorization code for a token and -// stores it for {subject, server}, so the resumed call finds a grant. The -// standalone interactive resolver provides one; when nil the loopback -// callback is NOT registered (managed mode hosts its own callback and never -// hands Forge a code). -type CallbackCompleter func(subject, server, code string) error +// CallbackCompleter exchanges an OAuth authorization code (with the PKCE +// verifier bound to the state) for a token and stores it for {subject, server}, +// so the resumed call finds a grant. The standalone resolver provides one; when +// nil the loopback callback is NOT registered (managed mode hosts its own +// callback and never hands Forge a code). ctx is the request context so the +// token exchange inherits a finite deadline. +type CallbackCompleter func(ctx context.Context, subject, server, code, verifier string) error // sessionFromRequest extracts the caller's session id for the cross-session // check. Prefers an explicit forge session header; falls back to a session @@ -123,9 +167,15 @@ func sessionFromRequest(r *http.Request) string { return "" } -// registerMCPCallbackEndpoint wires the standalone loopback callback. It is +// registerMCPCallbackEndpoint wires the standalone consent endpoints. They are // registered ONLY when a CallbackCompleter is set (standalone interactive -// mode); managed deployments never register it. +// mode); managed deployments never register them. +// +// - GET /mcp/oauth/start: the link Forge delivers. It sets the forge_session +// cookie (the producer the callback's mandatory session match requires) and +// redirects the browser to the IdP authorize URL. +// - GET /mcp/oauth/callback: the IdP redirect target. Validates state + +// session, exchanges the code, resumes the parked call. func (r *Runner) registerMCPCallbackEndpoint(srv *server.Server, auditLogger *coreruntime.AuditLogger) { if r.authGateEngine == nil || r.callbackCompleter == nil { return @@ -133,10 +183,55 @@ func (r *Runner) registerMCPCallbackEndpoint(srv *server.Server, auditLogger *co if r.stateBinder == nil { r.stateBinder = newStateBinder(defaultStateTTL) } + srv.RegisterHTTPHandler("GET /mcp/oauth/start", makeMCPStartHandler(r.stateBinder)) srv.RegisterHTTPHandler("GET /mcp/oauth/callback", makeMCPCallbackHandler(r.stateBinder, r.authGateEngine, r.callbackCompleter, sessionFromRequest, auditLogger)) } +// forgeSessionCookie is the cookie name the callback's cross-session guard +// reads (sessionFromRequest). The start endpoint is its only producer. +const forgeSessionCookie = "forge_session" + +// makeMCPStartHandler serves GET /mcp/oauth/start?state=. It is the +// session producer for the standalone consent flow (#332): the callback +// mandates a matching forge_session, and this is where the browser gets it. +// The step is necessary because the user's browser only ever touches Forge at +// the callback (after the IdP), so the cookie must be planted on an earlier +// same-origin visit — this redirect. +// +// The session value lives ONLY in the server-side binding and the cookie, never +// in a URL query, so it is never leaked to the IdP via Referer (unlike the +// state, which round-trips through the IdP by design). SameSite=Lax lets the +// cookie survive the top-level cross-site redirect back from the IdP. +// +// Scope of the guarantee (see docs/mcp/configuration.md — Trust model): the +// browser here is ANONYMOUS, and this plants the cookie for whoever holds the +// link. So the session binding proves browser CONTINUITY across the round-trip +// (a stolen state alone, replayed from a different browser, can't complete) — +// it does NOT prove the completing user is the parked subject. The link is a +// bearer capability; its real containment is authenticated-channel delivery + +// single-use short-TTL state. +func makeMCPStartHandler(binder *stateBinder) http.HandlerFunc { + return func(w http.ResponseWriter, req *http.Request) { + state := req.URL.Query().Get("state") + b, ok := binder.Peek(state) // Peek, not Consume — the state is spent at the callback. + if !ok || b.authorizeURL == "" || b.session == "" { + http.Error(w, "invalid or expired consent link", http.StatusBadRequest) + return + } + http.SetCookie(w, &http.Cookie{ + Name: forgeSessionCookie, + Value: b.session, + Path: "/mcp/oauth/", + HttpOnly: true, + Secure: req.TLS != nil || strings.EqualFold(req.Header.Get("X-Forwarded-Proto"), "https"), + SameSite: http.SameSiteLaxMode, + Expires: b.deadline, + }) + http.Redirect(w, req, b.authorizeURL, http.StatusFound) + } +} + // makeMCPCallbackHandler validates the state, enforces the session match, // exchanges the code for a token, and resumes the parked call. Extracted so // tests can drive every rejection path without a real IdP. @@ -162,17 +257,24 @@ func makeMCPCallbackHandler( http.Error(w, "invalid, expired, or already-used state", http.StatusBadRequest) return } - // Cross-session guard — MANDATORY. This endpoint is unauthenticated + // Session-continuity guard — MANDATORY. This endpoint is unauthenticated // (a browser redirect carries no bearer token) and is registered on // the network-exposed main server (cfg.Host, possibly 0.0.0.0), so // the state binding is its ENTIRE security. Single-use + expiry alone // would let anyone who obtains a state within its TTL complete the - // flow, so we additionally require the callback to land in the SAME - // session that started it. An empty bound session is a config/Issue- - // side bug (a network-exposed flow must bind a session) and is - // rejected fail-closed rather than silently downgrading to - // single-use+expiry. A future loopback-bound variant that wanted to - // relax this would bind the listener to localhost. + // flow from a DIFFERENT browser, so we additionally require the callback + // to land in the SAME browser that started it (the forge_session cookie + // planted at /start). An empty bound session is a config/Issue-side bug + // (a network-exposed flow must bind a session) and is rejected + // fail-closed rather than silently downgrading to single-use+expiry. + // + // What this does NOT guarantee (see docs/mcp/configuration.md — Trust + // model): in standalone mode the browser is anonymous, so this proves + // browser continuity across the IdP round-trip, NOT that the completing + // user is b.subject. Containment against a leaked link is + // authenticated-channel delivery + the short single-use TTL, not this + // cookie. A future loopback-bound variant would bind the listener to + // localhost; the tamper-proof alternative verifies IdP userinfo == subject. if b.session == "" || sessionOf(req) != b.session { http.Error(w, "state/session mismatch", http.StatusBadRequest) return @@ -180,8 +282,9 @@ func makeMCPCallbackHandler( // Exchange the code for a token and store it for {subject, server}. // Only AFTER this succeeds do we resume — resolving the gate with no // grant would just re-park the call (delegation follows - // authorization: never resume before the grant exists). - if err := complete(b.subject, b.server, code); err != nil { + // authorization: never resume before the grant exists). The PKCE + // verifier bound to the state proves this is the same flow we started. + if err := complete(req.Context(), b.subject, b.server, code, b.verifier); err != nil { http.Error(w, "authorization exchange failed", http.StatusBadGateway) return } diff --git a/forge-cli/runtime/mcp_consent_callback_test.go b/forge-cli/runtime/mcp_consent_callback_test.go index e9a85d3..be285c2 100644 --- a/forge-cli/runtime/mcp_consent_callback_test.go +++ b/forge-cli/runtime/mcp_consent_callback_test.go @@ -90,7 +90,7 @@ type fakeCompleter struct { err error } -func (f *fakeCompleter) complete(_, _, code string) error { +func (f *fakeCompleter) complete(_ context.Context, _, _, code, _ string) error { f.mu.Lock() defer f.mu.Unlock() f.calls++ diff --git a/forge-cli/runtime/mcp_standalone_consent.go b/forge-cli/runtime/mcp_standalone_consent.go new file mode 100644 index 0000000..b0c30b7 --- /dev/null +++ b/forge-cli/runtime/mcp_standalone_consent.go @@ -0,0 +1,228 @@ +package runtime + +import ( + "context" + "fmt" + "net/http" + "net/url" + "os" + "strings" + "time" + + "github.com/initializ/forge/forge-core/a2a" + "github.com/initializ/forge/forge-core/llm/oauth" + "github.com/initializ/forge/forge-core/mcp" + "github.com/initializ/forge/forge-core/types" +) + +// This file is the STANDALONE (no-platform) delegated-consent front-half +// (#332). It turns a grantless type: user + grant: authorization_code MCP call +// into a browser OAuth flow that Forge itself drives: +// +// park (ErrNoToken) → deliver a "Connect " link on the A2A +// auth-required artifact → GET /mcp/oauth/start sets the session cookie and +// redirects to the IdP → GET /mcp/oauth/callback exchanges the code and +// stores the token in the shared SubjectStore → the parked call resumes and +// the standalone resolver (server.go) finds the grant. +// +// Managed mode (a platform block is present) uses none of this — the platform +// delivers the prompt, hosts the callback, and brokers the token. + +// AuthorizeURLProvider supplies the consent link the deliverer presents to the +// user. Standalone builds it here (buildStandaloneConsentLink); a managed +// platform can supply its own pre-built URL via SetAuthorizeURLProvider (the +// seam #343's Slack delivery consumes so the same delivery code serves both +// modes). Returns the link to open, or an error the deliverer surfaces. +type AuthorizeURLProvider func(ctx context.Context, subject, server string) (string, error) + +// SetAuthorizeURLProvider overrides how the consent link is built. Standalone +// wires its own by default; a managed platform sets one that returns its +// own authorize URL (its client_id/state/redirect_uri) so Forge never +// constructs a managed URL from local config. Must be called before Run(). +func (r *Runner) SetAuthorizeURLProvider(fn AuthorizeURLProvider) { + r.authorizeURLProvider = fn +} + +// standaloneDelegatedServers returns the type: user MCP servers running in +// standalone mode (no platform block). Empty ⇒ nothing to wire. +func (r *Runner) standaloneDelegatedServers() []types.MCPServer { + p := r.cfg.Config.Platform + if p != nil && p.TokenEndpoint != "" { + return nil // managed — the platform owns delegation + } + var out []types.MCPServer + for _, s := range r.cfg.Config.MCP.Servers { + if s.Auth != nil && s.Auth.Type == "user" { + out = append(out, s) + } + } + return out +} + +// enableStandaloneConsent wires the standalone consent loop when the config has +// at least one standalone type: user server. It creates the shared +// SubjectStore (read by the resolver, written by the callback), and — unless an +// operator/platform already set them — installs the callback completer, the +// A2A-artifact deliverer, and the authorize-URL provider. Called before the MCP +// manager starts so the store is handed to it, and before the consent endpoints +// are registered. egressClient routes the token exchange through the allowlist. +func (r *Runner) enableStandaloneConsent(egressClient *http.Client) { + if len(r.standaloneDelegatedServers()) == 0 { + return + } + if r.standaloneSubjectStore == nil { + r.standaloneSubjectStore = mcp.NewInMemorySubjectTokenStore() + } + if r.stateBinder == nil { + r.stateBinder = newStateBinder(defaultStateTTL) + } + if r.authorizeURLProvider == nil { + r.authorizeURLProvider = r.buildStandaloneConsentLink + } + if r.callbackCompleter == nil { + r.callbackCompleter = r.makeStandaloneCompleter(egressClient) + } + if r.consentDeliverer == nil { + r.consentDeliverer = r.standaloneConsentDeliverer + } +} + +// publicBaseURL is the agent's externally-reachable base URL (no trailing +// slash), used to build the OAuth redirect_uri. Precedence: server.public_url, +// then the AGENT_URL env var. Empty ⇒ the consent flow can't build a callback +// the IdP can redirect to, and delivery fails with a clear error. +func (r *Runner) publicBaseURL() string { + base := r.cfg.Config.Server.PublicURL + if base == "" { + base = os.Getenv("AGENT_URL") + } + return strings.TrimRight(strings.TrimSpace(base), "/") +} + +// callbackRedirectURI is the redirect_uri baked into the authorize URL and +// replayed at the token exchange — they MUST match, so both derive from here. +func (r *Runner) callbackRedirectURI() (string, error) { + base := r.publicBaseURL() + if base == "" { + return "", fmt.Errorf("standalone consent needs the agent's public URL: set server.public_url in forge.yaml or the AGENT_URL env var") + } + return base + "/mcp/oauth/callback", nil +} + +// mcpServerSpec returns the configured spec for a server name. +func (r *Runner) mcpServerSpec(name string) (types.MCPServer, bool) { + for _, s := range r.cfg.Config.MCP.Servers { + if s.Name == name { + return s, true + } + } + return types.MCPServer{}, false +} + +// buildStandaloneConsentLink is the standalone AuthorizeURLProvider. It mints a +// fresh PKCE pair + state + session, builds the IdP authorize URL, records the +// binding, and returns the /mcp/oauth/start link the user clicks. The session +// lives only in the binding + cookie (never a URL), so it isn't leaked to the +// IdP via Referer. +func (r *Runner) buildStandaloneConsentLink(_ context.Context, subject, server string) (string, error) { + spec, ok := r.mcpServerSpec(server) + if !ok || spec.Auth == nil { + return "", fmt.Errorf("standalone consent: unknown or non-authed server %q", server) + } + redirectURI, err := r.callbackRedirectURI() + if err != nil { + return "", err + } + pkce, err := oauth.GeneratePKCE() + if err != nil { + return "", err + } + state, err := oauth.GenerateState() + if err != nil { + return "", err + } + session, err := oauth.GenerateState() + if err != nil { + return "", err + } + authorizeURL, err := mcp.BuildAuthorizeURL(spec.Auth.AuthorizeURL, spec.Auth.ClientID, redirectURI, state, pkce.Challenge, spec.Auth.Scopes) + if err != nil { + return "", err + } + r.stateBinder.Bind(state, subject, server, session, pkce.Verifier, authorizeURL) + return r.publicBaseURL() + "/mcp/oauth/start?state=" + url.QueryEscape(state), nil +} + +// makeStandaloneCompleter returns the CallbackCompleter that exchanges the code +// (with the state-bound PKCE verifier) for a token and caches it per-subject. +// Only after this succeeds does the callback resume the parked call. +func (r *Runner) makeStandaloneCompleter(egressClient *http.Client) CallbackCompleter { + client := egressClient + if client == nil { + client = http.DefaultClient + } + return func(ctx context.Context, subject, server, code, verifier string) error { + spec, ok := r.mcpServerSpec(server) + if !ok || spec.Auth == nil { + return fmt.Errorf("standalone consent: unknown or non-authed server %q", server) + } + redirectURI, err := r.callbackRedirectURI() + if err != nil { + return err + } + tok, err := oauth.ExchangeCodeCtx(ctx, client, spec.Auth.TokenURL, spec.Auth.ClientID, code, redirectURI, verifier) + if err != nil { + return err + } + if tok.AccessToken == "" { + return fmt.Errorf("standalone consent: token endpoint returned no access_token for server %q", server) + } + r.standaloneSubjectStore.Put(subject, tok.AccessToken, tokenTTL(tok)) + return nil + } +} + +// tokenTTL derives a cache lifetime from the token, defaulting conservatively +// when the IdP doesn't advertise one. +func tokenTTL(tok *oauth.Token) time.Duration { + if !tok.ExpiresAt.IsZero() { + if d := time.Until(tok.ExpiresAt); d > 0 { + return d + } + } + if tok.ExpiresIn > 0 { + return time.Duration(tok.ExpiresIn) * time.Second + } + return 5 * time.Minute +} + +// standaloneConsentDeliverer is the ConsentDeliverer that "delivers" the login +// link in-band: it builds the link and writes it onto the parked task's +// auth-required artifact so a UI/A2A client renders a clickable prompt. Channel +// (Slack) delivery is #343; this is the platform-free default. +func (r *Runner) standaloneConsentDeliverer(ctx context.Context, subject, server, taskID string, deadline time.Time) error { + link, err := r.authorizeURLProvider(ctx, subject, server) + if err != nil { + return err + } + if r.taskStore == nil { + return fmt.Errorf("standalone consent: no task store to publish the auth-required artifact") + } + r.SetStatus(taskID, a2a.TaskStatus{ + State: a2a.TaskStateAuthRequired, + Message: &a2a.Message{ + Role: a2a.MessageRoleAgent, + Parts: []a2a.Part{ + a2a.NewTextPart(fmt.Sprintf("Authorization required: connect %s (as %s). Open this link to continue: %s", server, subject, link)), + a2a.NewDataPart(map[string]any{ + "type": "mcp_auth_required", + "server": server, + "subject": subject, + "authorize_url": link, + "expires_at": deadline.UTC().Format(time.RFC3339), + }), + }, + }, + }) + return nil +} diff --git a/forge-cli/runtime/mcp_standalone_consent_test.go b/forge-cli/runtime/mcp_standalone_consent_test.go new file mode 100644 index 0000000..b41abe1 --- /dev/null +++ b/forge-cli/runtime/mcp_standalone_consent_test.go @@ -0,0 +1,192 @@ +package runtime + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "testing" + "time" + + "github.com/initializ/forge/forge-core/a2a" + "github.com/initializ/forge/forge-core/mcp" + "github.com/initializ/forge/forge-core/types" +) + +// standaloneRunner builds a Runner configured with one standalone type: user +// MCP server (no platform block) and a public URL, with the consent loop wired. +func standaloneRunner(t *testing.T, tokenURL string) *Runner { + t.Helper() + r := &Runner{ + logger: nopLogger{}, + cfg: RunnerConfig{ + Config: &types.ForgeConfig{ + Server: types.ServerConfig{PublicURL: "https://agent.example"}, + MCP: types.MCPConfig{Servers: []types.MCPServer{{ + Name: "atl", URL: "https://mcp.atlassian.example/mcp", + Auth: &types.MCPAuth{ + Type: "user", + ClientID: "forge-client", + AuthorizeURL: "https://idp.example/authorize", + TokenURL: tokenURL, + Scopes: []string{"read", "write"}, + }, + }}}, + }, + }, + } + r.enableStandaloneConsent(http.DefaultClient) + return r +} + +// TestStandaloneConsent_LinkAndStart covers the front-half: building the consent +// link records a binding (session + PKCE verifier + authorize URL), and +// GET /mcp/oauth/start plants the session cookie and redirects to the IdP. +func TestStandaloneConsent_LinkAndStart(t *testing.T) { + r := standaloneRunner(t, "https://idp.example/token") + + link, err := r.buildStandaloneConsentLink(context.Background(), "alice@corp.com", "atl") + if err != nil { + t.Fatalf("buildStandaloneConsentLink: %v", err) + } + if !strings.HasPrefix(link, "https://agent.example/mcp/oauth/start?state=") { + t.Fatalf("link = %q, want a /mcp/oauth/start?state= URL", link) + } + u, _ := url.Parse(link) + state := u.Query().Get("state") + + b, ok := r.stateBinder.Peek(state) + if !ok { + t.Fatal("no binding recorded for the issued state") + } + if b.subject != "alice@corp.com" || b.server != "atl" { + t.Errorf("binding subject/server = %q/%q", b.subject, b.server) + } + if b.session == "" || b.verifier == "" { + t.Error("binding must carry a non-empty session and PKCE verifier") + } + // The authorize URL must carry the redirect_uri, state, and PKCE challenge. + az, _ := url.Parse(b.authorizeURL) + if az.Query().Get("redirect_uri") != "https://agent.example/mcp/oauth/callback" { + t.Errorf("redirect_uri = %q", az.Query().Get("redirect_uri")) + } + if az.Query().Get("state") != state || az.Query().Get("code_challenge") == "" { + t.Error("authorize URL missing state or code_challenge") + } + + // /start → 302 to the IdP + Set-Cookie forge_session = the bound session. + rec := httptest.NewRecorder() + makeMCPStartHandler(r.stateBinder)(rec, httptest.NewRequest("GET", "/mcp/oauth/start?state="+url.QueryEscape(state), nil)) + if rec.Code != http.StatusFound { + t.Fatalf("/start → %d, want 302", rec.Code) + } + if loc := rec.Header().Get("Location"); loc != b.authorizeURL { + t.Errorf("/start Location = %q, want the authorize URL", loc) + } + var sessionCookie string + for _, c := range rec.Result().Cookies() { + if c.Name == forgeSessionCookie { + sessionCookie = c.Value + } + } + if sessionCookie != b.session { + t.Errorf("forge_session cookie = %q, want the bound session %q", sessionCookie, b.session) + } + + // A bogus state is rejected (no cookie, no redirect). + rec2 := httptest.NewRecorder() + makeMCPStartHandler(r.stateBinder)(rec2, httptest.NewRequest("GET", "/mcp/oauth/start?state=nope", nil)) + if rec2.Code != http.StatusBadRequest { + t.Errorf("/start with bad state → %d, want 400", rec2.Code) + } +} + +// TestStandaloneConsent_CompleterExchangesAndStores proves the callback +// completer exchanges the code (with the bound verifier) and caches the token +// per-subject, so the standalone resolver then finds a grant. +func TestStandaloneConsent_CompleterExchangesAndStores(t *testing.T) { + var gotForm url.Values + idp := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + _ = req.ParseForm() + gotForm = req.Form + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(map[string]any{ + "access_token": "access-alice", "token_type": "Bearer", "expires_in": 3600, + }) + })) + defer idp.Close() + + r := standaloneRunner(t, idp.URL) + completer := r.makeStandaloneCompleter(idp.Client()) + + if err := completer(context.Background(), "alice@corp.com", "atl", "authcode123", "verifier-xyz"); err != nil { + t.Fatalf("completer: %v", err) + } + // The exchange must present the code, verifier, and matching redirect_uri. + if gotForm.Get("code") != "authcode123" || gotForm.Get("code_verifier") != "verifier-xyz" { + t.Errorf("token request code/verifier = %q/%q", gotForm.Get("code"), gotForm.Get("code_verifier")) + } + if gotForm.Get("redirect_uri") != "https://agent.example/mcp/oauth/callback" { + t.Errorf("token request redirect_uri = %q", gotForm.Get("redirect_uri")) + } + // Token is now cached for the subject. + tok, ok := r.standaloneSubjectStore.Get("alice@corp.com") + if !ok || tok != "access-alice" { + t.Fatalf("subject store Get = %q,%v, want access-alice", tok, ok) + } + // And a different subject still has nothing. + if _, ok := r.standaloneSubjectStore.Get("eve@corp.com"); ok { + t.Error("unrelated subject must not have a grant") + } +} + +// TestStandaloneConsent_DelivererWritesArtifact proves the standalone +// ConsentDeliverer publishes the login link on the task's auth-required artifact. +func TestStandaloneConsent_DelivererWritesArtifact(t *testing.T) { + r := standaloneRunner(t, "https://idp.example/token") + r.taskStore = a2a.NewTaskStore() + r.taskStore.Put(&a2a.Task{ID: "task-1", Status: a2a.TaskStatus{State: a2a.TaskStateWorking}}) + + err := r.standaloneConsentDeliverer(context.Background(), "alice@corp.com", "atl", "task-1", time.Now().Add(time.Hour)) + if err != nil { + t.Fatalf("deliverer: %v", err) + } + + task := r.taskStore.Get("task-1") + if task.Status.State != a2a.TaskStateAuthRequired { + t.Fatalf("task state = %q, want auth-required", task.Status.State) + } + if task.Status.Message == nil || len(task.Status.Message.Parts) < 2 { + t.Fatal("auth-required message missing text + data parts") + } + text := task.Status.Message.Parts[0].Text + if !strings.Contains(text, "/mcp/oauth/start?state=") { + t.Errorf("artifact text lacks the consent link: %q", text) + } + data, _ := task.Status.Message.Parts[1].Data.(map[string]any) + if data["authorize_url"] == "" || data["server"] != "atl" { + t.Errorf("data part = %v, want authorize_url + server", data) + } +} + +// TestStandaloneConsent_NotWiredWhenManaged confirms a platform block disables +// the standalone wiring (managed mode owns delegation). +func TestStandaloneConsent_NotWiredWhenManaged(t *testing.T) { + r := &Runner{ + logger: nopLogger{}, + cfg: RunnerConfig{Config: &types.ForgeConfig{ + Platform: &types.PlatformConfig{TokenEndpoint: "https://platform.example/token"}, + MCP: types.MCPConfig{Servers: []types.MCPServer{{ + Name: "atl", Auth: &types.MCPAuth{Type: "user", Ref: "mcp.atl"}, + }}}, + }}, + } + r.enableStandaloneConsent(http.DefaultClient) + if r.standaloneSubjectStore != nil || r.consentDeliverer != nil || r.callbackCompleter != nil { + t.Error("managed mode must not wire the standalone consent loop") + } + // Sanity: the helper wouldn't create the in-memory store either. + _ = mcp.NewInMemorySubjectTokenStore +} diff --git a/forge-cli/runtime/runner.go b/forge-cli/runtime/runner.go index 01e4e11..8212492 100644 --- a/forge-cli/runtime/runner.go +++ b/forge-cli/runtime/runner.go @@ -37,6 +37,7 @@ import ( "github.com/initializ/forge/forge-core/llm" "github.com/initializ/forge/forge-core/llm/oauth" "github.com/initializ/forge/forge-core/llm/providers" + "github.com/initializ/forge/forge-core/mcp" "github.com/initializ/forge/forge-core/memory" "github.com/initializ/forge/forge-core/observability" coreruntime "github.com/initializ/forge/forge-core/runtime" @@ -156,31 +157,33 @@ After editing, trace the failing input through your new code. Read the functions // Runner orchestrates the local A2A development server. type Runner struct { - cfg RunnerConfig - logger coreruntime.Logger - cliExecTool *clitools.CLIExecuteTool - modelConfig *coreruntime.ModelConfig // resolved model config (for banner) - derivedCLIConfig *contract.DerivedCLIConfig // auto-derived from skill requirements - derivedBrowserConfig *contract.DerivedBrowserConfig // non-nil when a skill declares requires.capabilities: [browser] (#94) - browserManager *browser.Manager // lazy Chromium owner; nil unless browser tools registered - skillGuardrails *agentspec.SkillGuardrailRules // runtime-parsed skill guardrails (fallback when no build artifact) - schedBackend scheduler.Backend // schedule backend (nil until started); FileBackend in non-cluster deploys, KubernetesBackend (#162 part 2b) when running in-cluster with scheduler.backend=auto|kubernetes - startTime time.Time // server start time (for /health uptime) - scheduleNotifier ScheduleNotifier // optional: delivers cron results to channels - deferralNotifier DeferralNotifier // optional: delivers DEFER approval requests to channels (#310) - authToken string // resolved auth token (empty if --no-auth) - cancelRegistry *coreruntime.CancellationRegistry // per-Runner in-flight cancellation registry (issue #88 / FWS-4) - auditSigningKey *coreruntime.LoadedKey // loaded once at startup; nil when signing is off (#213). Served on JWKS endpoint. - compression *compress.Runtime // ctxzip compression runtime; nil when compression is disabled - intentEngine *intent.Engine // R3 (#208) intent-alignment engine; nil when disabled - stepUpEngine *stepup.Engine // R4b (#210) step-up authorization engine; nil when disabled - deferEngine *deferengine.Engine // R4c (#211) deferred-authorization engine; nil when disabled - authGateEngine *authgate.Engine // R10 (#330) MCP auth-required gate; nil until an MCP manager with a type=user server is wired - consentDeliverer ConsentDeliverer // optional: delivers MCP consent prompts to channels (#330); nil in standalone (no delivery yet) - callbackCompleter CallbackCompleter // optional: standalone loopback code→token exchange (#330); nil ⇒ no loopback callback (managed hosts its own) - stateBinder *stateBinder // standalone OAuth state binding (single-use/expiring/session-bound); lazily built with the callback endpoint - taskStore *a2a.TaskStore // shared task store, populated once srv is built; read by defer hook when it fires - platformCommandGuard *coreruntime.PlatformCommandGuard // #238 (ASI02) operator-authored command deny, applied to every tool call; empty when no layer declares denied_command_patterns + cfg RunnerConfig + logger coreruntime.Logger + cliExecTool *clitools.CLIExecuteTool + modelConfig *coreruntime.ModelConfig // resolved model config (for banner) + derivedCLIConfig *contract.DerivedCLIConfig // auto-derived from skill requirements + derivedBrowserConfig *contract.DerivedBrowserConfig // non-nil when a skill declares requires.capabilities: [browser] (#94) + browserManager *browser.Manager // lazy Chromium owner; nil unless browser tools registered + skillGuardrails *agentspec.SkillGuardrailRules // runtime-parsed skill guardrails (fallback when no build artifact) + schedBackend scheduler.Backend // schedule backend (nil until started); FileBackend in non-cluster deploys, KubernetesBackend (#162 part 2b) when running in-cluster with scheduler.backend=auto|kubernetes + startTime time.Time // server start time (for /health uptime) + scheduleNotifier ScheduleNotifier // optional: delivers cron results to channels + deferralNotifier DeferralNotifier // optional: delivers DEFER approval requests to channels (#310) + authToken string // resolved auth token (empty if --no-auth) + cancelRegistry *coreruntime.CancellationRegistry // per-Runner in-flight cancellation registry (issue #88 / FWS-4) + auditSigningKey *coreruntime.LoadedKey // loaded once at startup; nil when signing is off (#213). Served on JWKS endpoint. + compression *compress.Runtime // ctxzip compression runtime; nil when compression is disabled + intentEngine *intent.Engine // R3 (#208) intent-alignment engine; nil when disabled + stepUpEngine *stepup.Engine // R4b (#210) step-up authorization engine; nil when disabled + deferEngine *deferengine.Engine // R4c (#211) deferred-authorization engine; nil when disabled + authGateEngine *authgate.Engine // R10 (#330) MCP auth-required gate; nil until an MCP manager with a type=user server is wired + consentDeliverer ConsentDeliverer // optional: delivers MCP consent prompts to channels (#330); standalone auto-wires the A2A-artifact deliverer (#332) + callbackCompleter CallbackCompleter // optional: standalone loopback code→token exchange (#330); nil ⇒ no loopback callback (managed hosts its own) + stateBinder *stateBinder // standalone OAuth state binding (single-use/expiring/session-bound); lazily built with the callback endpoint + authorizeURLProvider AuthorizeURLProvider // supplies the consent link (#332 standalone builds it; managed platform supplies its own for #343) + standaloneSubjectStore mcp.SubjectTokenStore // #332 shared per-subject token cache: standalone resolver reads, callback writes; nil unless a standalone type:user server exists + taskStore *a2a.TaskStore // shared task store, populated once srv is built; read by defer hook when it fires + platformCommandGuard *coreruntime.PlatformCommandGuard // #238 (ASI02) operator-authored command deny, applied to every tool call; empty when no layer declares denied_command_patterns } // NewRunner creates a Runner from the given config. @@ -1042,6 +1045,14 @@ func (r *Runner) Run(ctx context.Context) error { } } + // Standalone delegated consent (#332): when a type: user MCP + // server runs without a platform block, Forge drives the per-user + // OAuth itself. Wire the shared SubjectStore + callback completer + + // A2A-artifact deliverer BEFORE the manager starts (it needs the + // store) and before the consent endpoints register. No-op unless a + // standalone type: user server is configured. + r.enableStandaloneConsent(egressClient) + // Start MCP servers (Phase 1: HTTP-only) and register their // discovered tools as namespaced "__" entries. // Required=true server failures here cause Run() to return diff --git a/forge-cli/runtime/runner_mcp.go b/forge-cli/runtime/runner_mcp.go index 3c25ef1..e40e47f 100644 --- a/forge-cli/runtime/runner_mcp.go +++ b/forge-cli/runtime/runner_mcp.go @@ -119,6 +119,10 @@ func (r *Runner) startMCPManager( Audit: auditLogger, OAuth: flow, Platform: r.cfg.Config.Platform, + // #332 — the standalone type: user resolver reads this shared store; + // the consent callback writes to it. nil unless enableStandaloneConsent + // created it (i.e. a standalone type: user server is configured). + SubjectStore: r.standaloneSubjectStore, }) if err != nil { return nil, err diff --git a/forge-core/auth/middleware.go b/forge-core/auth/middleware.go index 143376e..a88c4e4 100644 --- a/forge-core/auth/middleware.go +++ b/forge-core/auth/middleware.go @@ -33,22 +33,32 @@ func DefaultSkipPaths() map[string]bool { "GET /.well-known/agent.json": true, "GET /healthz": true, "GET /health": true, - // The standalone MCP OAuth callback (#330) is a browser redirect - // FROM the IdP — it carries no bearer token, so it must bypass auth - // or the middleware 401s it before the handler runs. Its authenticity - // is the single-use, session-bound OAuth `state` it validates, not a - // bearer token. Exempt unconditionally: it is registered only in - // standalone mode (a CallbackCompleter is set); in managed mode the - // path is unregistered and a request just 404s after the skip. NOTE - // the contrast with POST /mcp/consent, which is deliberately NOT - // exempt — the managed platform authenticates when it posts the - // resume signal. + // The standalone MCP OAuth endpoints (#330/#332) are browser hops + // that carry no bearer token, so they must bypass auth or the + // middleware 401s them before the handler runs: + // + // - GET /mcp/oauth/start: the "Connect" link the user's ANONYMOUS + // browser opens first; it plants the session cookie and redirects + // to the IdP. Its authenticity rests on the Peek'd session-bound + // state (it rejects any binding lacking authorizeURL+session), and + // it is not an open redirect — b.authorizeURL is server-built from + // config, reachable only via a Forge-minted state. + // - GET /mcp/oauth/callback: the redirect FROM the IdP; authenticity + // is the single-use, session-bound state it validates. + // + // Both are registered ONLY in standalone mode (a CallbackCompleter is + // set); in managed mode the paths are unregistered and a request just + // 404s after the skip. NOTE the contrast with POST /mcp/consent, which + // is deliberately NOT exempt — the managed platform authenticates when + // it posts the resume signal. + "GET /mcp/oauth/start": true, "GET /mcp/oauth/callback": true, "OPTIONS /": true, "OPTIONS /.well-known/agent-card.json": true, "OPTIONS /.well-known/agent.json": true, "OPTIONS /healthz": true, "OPTIONS /health": true, + "OPTIONS /mcp/oauth/start": true, "OPTIONS /mcp/oauth/callback": true, } } diff --git a/forge-core/auth/middleware_test.go b/forge-core/auth/middleware_test.go index 942b60c..0415f56 100644 --- a/forge-core/auth/middleware_test.go +++ b/forge-core/auth/middleware_test.go @@ -135,6 +135,17 @@ func TestMiddleware(t *testing.T) { path: "/mcp/oauth/callback", wantStatus: http.StatusOK, }, + { + // #332: the standalone consent "start" hop is opened by the user's + // ANONYMOUS browser (a link click, no bearer token) — it MUST bypass + // auth or the flow dies at its first hop. Its authenticity is the + // session-bound state it Peeks, not a token. + name: "GET /mcp/oauth/start is public (anonymous browser link click)", + opts: MiddlewareOptions{Chain: chain, SkipPaths: DefaultSkipPaths()}, + method: "GET", + path: "/mcp/oauth/start", + wantStatus: http.StatusOK, + }, { // #330: the consent RESUME signal is the opposite — the managed // platform authenticates when it posts it, so it must NOT be diff --git a/forge-core/mcp/manager.go b/forge-core/mcp/manager.go index dc0e31e..472097d 100644 --- a/forge-core/mcp/manager.go +++ b/forge-core/mcp/manager.go @@ -60,6 +60,13 @@ type ManagerDeps struct { // required by servers with auth.type=platform; otherwise may be nil. // MUST stay field-identical with ServerDeps (type conversion below). Platform *types.PlatformConfig + + // SubjectStore is the per-requesting-user access-token cache used by a + // STANDALONE type: user server (no platform block, grant + // authorization_code, #332): the resolver reads it and the standalone + // consent callback writes to it. nil ⇒ no standalone delegated servers + // in this config. MUST stay field-identical with ServerDeps. + SubjectStore SubjectTokenStore } // NewManager constructs a Manager. Fails fast if config validation diff --git a/forge-core/mcp/oauth_flow.go b/forge-core/mcp/oauth_flow.go index 7685ca2..b8e6cea 100644 --- a/forge-core/mcp/oauth_flow.go +++ b/forge-core/mcp/oauth_flow.go @@ -110,6 +110,10 @@ type OAuthServerConfig struct { // grantClientCredentials is the 2-legged agent-principal grant (#324). const grantClientCredentials = "client_credentials" +// grantAuthorizationCode is the 3-legged delegated grant — the default for +// interactive OAuth and for standalone delegated consent (#332). +const grantAuthorizationCode = "authorization_code" + // storeKey returns the credential-store key for an MCP server. // Prefixed "MCP_" so MCP tokens are namespaced separately from LLM // provider tokens in the same encrypted file. @@ -484,6 +488,14 @@ type refreshGroup struct { err error } +// BuildAuthorizeURL assembles an OAuth 2.1 + PKCE (S256) authorize URL. +// Exported for the standalone delegated-consent front-half (#332), which builds +// the URL outside this package; the interactive Login path uses the unexported +// form directly. +func BuildAuthorizeURL(authorizeURL, clientID, redirectURI, state, challenge string, scopes []string) (string, error) { + return buildAuthorizeURL(authorizeURL, clientID, redirectURI, state, challenge, scopes) +} + // buildAuthorizeURL assembles the authorize-endpoint URL. Returns // ErrProtocolError if AuthorizeURL is malformed. func buildAuthorizeURL(authorizeURL, clientID, redirectURI, state, challenge string, scopes []string) (string, error) { diff --git a/forge-core/mcp/server.go b/forge-core/mcp/server.go index c5ae214..74d6403 100644 --- a/forge-core/mcp/server.go +++ b/forge-core/mcp/server.go @@ -98,6 +98,11 @@ type ServerDeps struct { // Platform is the managed token-resolver wiring, required by servers // with auth.type=platform. nil → no platform servers in this config. Platform *types.PlatformConfig + + // SubjectStore is the per-requesting-user token cache read by a + // STANDALONE type: user resolver (#332). Field-identical with + // ManagerDeps (they convert via ServerDeps(deps)). + SubjectStore SubjectTokenStore } // knownMCPAuthTypes is the closed set of accepted Auth.Type values. @@ -186,11 +191,25 @@ func NewServer(spec types.MCPServer, deps ServerDeps) (*Server, error) { if spec.Required { return nil, fmt.Errorf("%w: server %q: auth.type=user cannot be required:true — delegated identity connects lazily after consent", ErrProtocolError, spec.Name) } - // #317: the delegated token resolves via the platform token - // endpoint (per-user, {server, subject}) — so the platform block - // is the contract, same as type=platform. - if deps.Platform == nil || deps.Platform.TokenEndpoint == "" { - return nil, fmt.Errorf("%w: server %q: auth.type=user requires the top-level platform block (delegated tokens resolve via the platform token endpoint)", ErrProtocolError, spec.Name) + // Two delegated modes, selected by the platform block: + // - MANAGED (#317): platform block present → the delegated token + // resolves via the platform token endpoint (per-user + // {server, subject}). + // - STANDALONE (#332): no platform block → Forge runs the + // per-user OAuth itself (grant authorization_code) and reads a + // local SubjectStore the consent callback populates. Endpoints + // + client_id must be explicit (no runtime discovery). + if deps.Platform != nil && deps.Platform.TokenEndpoint != "" { + break // managed — nothing more to validate here + } + if spec.Auth.Grant != "" && spec.Auth.Grant != grantAuthorizationCode { + return nil, fmt.Errorf("%w: server %q: standalone auth.type=user (no platform block) is authorization-code consent — grant must be authorization_code, got %q", ErrProtocolError, spec.Name, spec.Auth.Grant) + } + if spec.Auth.AuthorizeURL == "" || spec.Auth.TokenURL == "" || spec.Auth.ClientID == "" { + return nil, fmt.Errorf("%w: server %q: standalone auth.type=user requires explicit auth.authorize_url, auth.token_url, and auth.client_id (no runtime discovery), or add a platform block for managed delegation", ErrProtocolError, spec.Name) + } + if deps.SubjectStore == nil { + return nil, fmt.Errorf("%w: server %q: standalone auth.type=user requires a per-subject token store (ManagerDeps.SubjectStore) — the runtime wires it alongside the consent callback", ErrProtocolError, spec.Name) } } } @@ -709,11 +728,32 @@ func buildAuthFn(spec types.MCPServer, deps ServerDeps) AuthTokenFunc { }) return src.Token case "user": - // Delegated user identity (#317): resolve a per-REQUESTING-USER - // token from the platform token endpoint. Lazy by design — until a - // request carries an authenticated user, and until the platform has - // a grant for that user, this fails with ErrNoToken so the server - // never blocks startup. + // Delegated per-REQUESTING-USER identity. Lazy by design — until a + // request carries an authenticated user, and until a grant exists for + // that user, this fails with ErrNoToken (tripping the auth-required + // gate) so the server never blocks startup. + serverName := spec.Name + if deps.Platform == nil || deps.Platform.TokenEndpoint == "" { + // STANDALONE (#332): no platform. The token is minted by Forge's + // own consent loop and cached per-subject in SubjectStore; a miss + // is ErrNoToken so the gate parks the call until consent lands. + store := deps.SubjectStore + return func(ctx context.Context) (string, error) { + subject := delegatedSubject(ctx) + if subject == "" { + return "", fmt.Errorf("%w: server %q uses delegated user identity but no requesting user is in context — it connects lazily under a user's session, never at startup", ErrNoToken, serverName) + } + if store == nil { + return "", fmt.Errorf("%w: server %q: standalone delegated token store is not wired", ErrNoToken, serverName) + } + if tok, ok := store.Get(subject); ok { + return tok, nil + } + return "", fmt.Errorf("%w: no local grant for subject %q on server %q yet — awaiting standalone consent (#332)", ErrNoToken, subject, serverName) + } + } + // MANAGED (#317): resolve the per-user token from the platform token + // endpoint (per-user, {server, subject}). ref := spec.Auth.Ref if ref == "" { ref = spec.Name @@ -724,7 +764,6 @@ func buildAuthFn(spec types.MCPServer, deps ServerDeps) AuthTokenFunc { Ref: ref, HTTPClient: deps.HTTPClient, }) - serverName := spec.Name return func(ctx context.Context) (string, error) { subject := delegatedSubject(ctx) if subject == "" { diff --git a/forge-core/mcp/standalone_delegated_test.go b/forge-core/mcp/standalone_delegated_test.go new file mode 100644 index 0000000..e733a59 --- /dev/null +++ b/forge-core/mcp/standalone_delegated_test.go @@ -0,0 +1,117 @@ +package mcp + +import ( + "context" + "errors" + "net/http" + "testing" + "time" + + "github.com/initializ/forge/forge-core/auth" + "github.com/initializ/forge/forge-core/types" +) + +// standaloneUserSpec is a valid standalone (#332) delegated server: type=user, +// no platform block, explicit endpoints + client_id, authorization_code grant. +func standaloneUserSpec() types.MCPServer { + return types.MCPServer{ + Name: "atlassian", URL: "https://mcp.atlassian.example/mcp", + Auth: &types.MCPAuth{ + Type: "user", + ClientID: "forge-client", + AuthorizeURL: "https://idp.example/authorize", + TokenURL: "https://idp.example/token", + Scopes: []string{"read", "write"}, + }, + } +} + +// TestStandaloneUser_ResolverReadsSubjectStore proves the standalone resolver: +// no user → ErrNoToken; user with an empty store → ErrNoToken (trips the gate); +// user with a stored token → that token. +func TestStandaloneUser_ResolverReadsSubjectStore(t *testing.T) { + store := newMemSubjectTokenStore(0) + authFn := buildAuthFn(standaloneUserSpec(), ServerDeps{ + HTTPClient: http.DefaultClient, + SubjectStore: store, + }) + if authFn == nil { + t.Fatal("buildAuthFn returned nil for standalone type=user") + } + + // No requesting user → lazy ErrNoToken. + if _, err := authFn(context.Background()); !errors.Is(err, ErrNoToken) { + t.Fatalf("no user in ctx must fail with ErrNoToken, got %v", err) + } + + ctx := auth.WithIdentity(context.Background(), &auth.Identity{Email: "dave@corp.com"}) + + // User but no grant yet → ErrNoToken (the auth-required gate parks here). + if _, err := authFn(ctx); !errors.Is(err, ErrNoToken) { + t.Fatalf("no stored grant must fail with ErrNoToken, got %v", err) + } + + // Consent completes → the callback populates the store → resolver returns it. + store.Put("dave@corp.com", "access-dave", time.Hour) + tok, err := authFn(ctx) + if err != nil || tok != "access-dave" { + t.Fatalf("after Put: token=%q err=%v, want access-dave", tok, err) + } + + // Per-subject isolation: a different user still has no grant. + other := auth.WithIdentity(context.Background(), &auth.Identity{Email: "eve@corp.com"}) + if _, err := authFn(other); !errors.Is(err, ErrNoToken) { + t.Fatalf("unrelated subject must still fail ErrNoToken, got %v", err) + } +} + +// TestStandaloneUser_NewServerValidation pins NewServer's accept/reject rules +// for standalone type=user (no platform block). +func TestStandaloneUser_NewServerValidation(t *testing.T) { + store := newMemSubjectTokenStore(0) + baseDeps := ServerDeps{HTTPClient: http.DefaultClient, SubjectStore: store} + + t.Run("valid standalone is accepted", func(t *testing.T) { + if _, err := NewServer(standaloneUserSpec(), baseDeps); err != nil { + t.Fatalf("valid standalone type=user rejected: %v", err) + } + }) + + t.Run("missing explicit endpoints rejected", func(t *testing.T) { + spec := standaloneUserSpec() + spec.Auth.AuthorizeURL = "" // no discovery at runtime + if _, err := NewServer(spec, baseDeps); err == nil { + t.Fatal("standalone type=user without explicit endpoints must be rejected") + } + }) + + t.Run("client_credentials grant rejected", func(t *testing.T) { + spec := standaloneUserSpec() + spec.Auth.Grant = "client_credentials" + if _, err := NewServer(spec, baseDeps); err == nil { + t.Fatal("standalone type=user with grant client_credentials must be rejected") + } + }) + + t.Run("missing SubjectStore rejected", func(t *testing.T) { + if _, err := NewServer(standaloneUserSpec(), ServerDeps{HTTPClient: http.DefaultClient}); err == nil { + t.Fatal("standalone type=user without a SubjectStore must be rejected") + } + }) + + t.Run("required:true still rejected", func(t *testing.T) { + spec := standaloneUserSpec() + spec.Required = true + if _, err := NewServer(spec, baseDeps); err == nil { + t.Fatal("type=user + required:true must be rejected") + } + }) + + t.Run("explicit authorization_code grant accepted", func(t *testing.T) { + spec := standaloneUserSpec() + spec.Auth.Grant = "authorization_code" + if _, err := NewServer(spec, baseDeps); err != nil { + t.Fatalf("explicit authorization_code grant rejected: %v", err) + } + }) +} diff --git a/forge-core/mcp/subject_token_store.go b/forge-core/mcp/subject_token_store.go index 3bc8c6e..28b9b8c 100644 --- a/forge-core/mcp/subject_token_store.go +++ b/forge-core/mcp/subject_token_store.go @@ -30,6 +30,13 @@ type SubjectTokenStore interface { Evict(subject string) } +// NewInMemorySubjectTokenStore returns the default in-process +// SubjectTokenStore. Exported for the standalone consent wiring (#332) to +// create the shared store its resolver reads and its callback writes. +func NewInMemorySubjectTokenStore() SubjectTokenStore { + return newMemSubjectTokenStore(0) +} + // memSubjectTokenStore is the default in-process SubjectTokenStore: a // per-subject map with early-refresh skew and an opportunistic sweep so it // can't grow unbounded with one-shot users, without a background goroutine. diff --git a/forge-core/types/config.go b/forge-core/types/config.go index 8d7d432..07a6424 100644 --- a/forge-core/types/config.go +++ b/forge-core/types/config.go @@ -495,6 +495,16 @@ type TracingYAML struct { // See issue #110 / FWS-10. type ServerConfig struct { RateLimit RateLimitYAML `yaml:"rate_limit,omitempty"` + + // PublicURL is the agent's externally-reachable base URL (scheme + + // host, no trailing slash), e.g. "https://agent.example.com". It is + // used to build the OAuth redirect_uri for standalone delegated + // consent (#332): /mcp/oauth/callback must be reachable by + // the user's browser after IdP consent. When empty, the runtime falls + // back to the AGENT_URL env var. Only required for standalone + // type: user (authorization_code) MCP servers; managed deployments + // host their own callback and don't need it. + PublicURL string `yaml:"public_url,omitempty"` } // RateLimitYAML mirrors the runtime RateLimitConfig but lives in diff --git a/forge-core/validate/forge_config.go b/forge-core/validate/forge_config.go index 9e1ef41..73b0cb8 100644 --- a/forge-core/validate/forge_config.go +++ b/forge-core/validate/forge_config.go @@ -161,6 +161,32 @@ func ValidateForgeConfig(cfg *types.ForgeConfig) *ValidationResult { ValidateAuthConfig(cfg.Auth, r) ValidateMCPConfig(cfg.MCP, r) + validateStandaloneDelegatedConsent(cfg, r) return r } + +// validateStandaloneDelegatedConsent enforces the standalone (#332) rules for +// auth.type=user servers that have NO platform block: without a platform token +// endpoint, Forge runs the per-user OAuth itself, which needs explicit +// endpoints + client_id (no runtime discovery) and the authorization_code +// grant. Lives here (not in ValidateMCPConfig) because the standalone-vs-managed +// distinction depends on the top-level platform block. Mirrors NewServer. +func validateStandaloneDelegatedConsent(cfg *types.ForgeConfig, r *ValidationResult) { + managed := cfg.Platform != nil && cfg.Platform.TokenEndpoint != "" + if managed { + return + } + for i, s := range cfg.MCP.Servers { + if s.Auth == nil || s.Auth.Type != "user" { + continue + } + prefix := fmt.Sprintf("mcp.servers[%d] (%s)", i, s.Name) + if s.Auth.Grant != "" && s.Auth.Grant != "authorization_code" { + r.Errors = append(r.Errors, prefix+": standalone auth.type=user (no platform block) uses the authorization_code grant — remove grant or set it to authorization_code") + } + if s.Auth.AuthorizeURL == "" || s.Auth.TokenURL == "" || s.Auth.ClientID == "" { + r.Errors = append(r.Errors, prefix+": standalone auth.type=user requires explicit auth.authorize_url, auth.token_url, and auth.client_id (no runtime discovery) — or add a platform block for managed delegation") + } + } +} diff --git a/forge-core/validate/standalone_consent_test.go b/forge-core/validate/standalone_consent_test.go new file mode 100644 index 0000000..2a0abe5 --- /dev/null +++ b/forge-core/validate/standalone_consent_test.go @@ -0,0 +1,65 @@ +package validate + +import ( + "strings" + "testing" + + "github.com/initializ/forge/forge-core/types" +) + +func userServer(a *types.MCPAuth) types.ForgeConfig { + return types.ForgeConfig{ + MCP: types.MCPConfig{Servers: []types.MCPServer{{Name: "atl", Auth: a}}}, + } +} + +func TestValidateStandaloneDelegatedConsent(t *testing.T) { + fullStandalone := &types.MCPAuth{ + Type: "user", ClientID: "c", AuthorizeURL: "https://idp/az", TokenURL: "https://idp/tok", + } + + tests := []struct { + name string + cfg types.ForgeConfig + wantErr string // substring; "" means no error expected + }{ + { + name: "valid standalone", + cfg: userServer(fullStandalone), + wantErr: "", + }, + { + name: "missing endpoints", + cfg: userServer(&types.MCPAuth{Type: "user", ClientID: "c"}), + wantErr: "explicit auth.authorize_url", + }, + { + name: "client_credentials grant rejected", + cfg: userServer(&types.MCPAuth{Type: "user", ClientID: "c", AuthorizeURL: "https://idp/az", TokenURL: "https://idp/tok", Grant: "client_credentials"}), + wantErr: "authorization_code grant", + }, + { + name: "managed (platform set) skips standalone checks", + cfg: func() types.ForgeConfig { + c := userServer(&types.MCPAuth{Type: "user", Ref: "mcp.atl"}) // no endpoints, but managed + c.Platform = &types.PlatformConfig{TokenEndpoint: "https://platform/token"} + return c + }(), + wantErr: "", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + r := &ValidationResult{} + validateStandaloneDelegatedConsent(&tc.cfg, r) + joined := strings.Join(r.Errors, "\n") + if tc.wantErr == "" && len(r.Errors) != 0 { + t.Fatalf("expected no errors, got: %s", joined) + } + if tc.wantErr != "" && !strings.Contains(joined, tc.wantErr) { + t.Fatalf("expected error containing %q, got: %s", tc.wantErr, joined) + } + }) + } +}