From e346246b88bb621f975254c0fc1e8cad51ac8fa1 Mon Sep 17 00:00:00 2001 From: Lakhan Samani Date: Sat, 15 Aug 2026 12:56:52 +0530 Subject: [PATCH] feat(mcp): accept RFC 8693 delegated tokens at /mcp MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An agent could not ask Authorizer about its own delegated authority: check_permissions was unreachable with the very token that proves the delegation, so agent-delegation over MCP stayed a stdio-only story. ValidateMCPAccessToken falls back to a delegated check when the stateful one fails, gated on the `act` claim so an ordinary wrong-audience token is rejected once rather than paying a second validation on an internet-facing endpoint. The fallback goes through a new entry point, ValidateDelegatedAccessTokenForResource, rather than widening ValidateDelegatedAccessToken. That function has one caller — GetUserIDFromSessionOrAccessToken — which backs /graphql, /v1/* and gRPC, so relaxing its audience check in place would have made every MCP-bound delegated token a full first-party credential. The match stays exact in both directions: a token bound to the bare server URL is still refused at /mcp, and an MCP-bound one is still refused everywhere else. One token, one surface. --- internal/e2e/smoke_test.go | 108 ++++++- .../integration_tests/mcp_delegated_test.go | 288 ++++++++++++++++++ internal/mcp/exposed_methods_test.go | 48 +++ internal/token/delegated_access_token.go | 84 ++++- internal/token/mcp_access_token.go | 53 +++- internal/token/provider.go | 10 +- 6 files changed, 573 insertions(+), 18 deletions(-) create mode 100644 internal/integration_tests/mcp_delegated_test.go diff --git a/internal/e2e/smoke_test.go b/internal/e2e/smoke_test.go index 1f64bf05..f2b6d194 100644 --- a/internal/e2e/smoke_test.go +++ b/internal/e2e/smoke_test.go @@ -53,7 +53,10 @@ const ( // fgaModelDSL is the minimal OpenFGA model the scenario authorizes // against: a user can be a viewer of a document. - fgaModelDSL = "model\n schema 1.1\ntype user\ntype document\n relations\n define viewer: [user]" + // `agent` is declared so the RFC 8693 delegation intersection has a subject + // type to hold the agent half. Declaring the type IS the opt-in; the user + // assertions elsewhere in this file are unaffected by its presence. + fgaModelDSL = "model\n schema 1.1\ntype user\ntype agent\ntype document\n relations\n define viewer: [user, agent]" ) // TestReleaseSmoke is the release gate: one scenario across every public @@ -482,6 +485,109 @@ func TestReleaseSmoke(t *testing.T) { "a token minted for the client, not for /mcp, must be refused") }) + // --- Surface 4b: MCP over HTTP with an RFC 8693 delegated token ------- + // An agent acting for the user must reach the tools, and must be answered + // with ITS OWN authority (perms(agent) ∩ perms(user)) rather than the + // user's. Run against the real binary because the property spans the token + // endpoint, the audience check and the FGA subject expansion — three + // components that are individually tested and could still disagree once + // wired together. + t.Run("mcp delegated", func(t *testing.T) { + resource := baseURL + "/mcp" + + created := gql.mutate(t, `mutation { _create_client(params:{name:"smoke-agent", allowed_scopes:["openid"]}) { client { client_id } client_secret } }`) + agent := created["_create_client"].(map[string]any) + agentID := agent["client"].(map[string]any)["client_id"].(string) + agentSecret := agent["client_secret"].(string) + require.NotEmpty(t, agentID) + + postForm := func(form url.Values) map[string]any { + resp, err := http.Post(baseURL+"/oauth/token", + "application/x-www-form-urlencoded", strings.NewReader(form.Encode())) + require.NoError(t, err) + defer func() { _ = resp.Body.Close() }() + raw, _ := io.ReadAll(resp.Body) + require.Equal(t, http.StatusOK, resp.StatusCode, "token endpoint: %s", raw) + var out map[string]any + require.NoError(t, json.Unmarshal(raw, &out)) + return out + } + + ccForm := url.Values{} + ccForm.Set("grant_type", "client_credentials") + ccForm.Set("client_id", agentID) + ccForm.Set("client_secret", agentSecret) + actorToken := postForm(ccForm)["access_token"].(string) + + exchange := func(res string) string { + form := url.Values{} + form.Set("grant_type", "urn:ietf:params:oauth:grant-type:token-exchange") + form.Set("client_id", agentID) + form.Set("client_secret", agentSecret) + form.Set("subject_token", token) + form.Set("subject_token_type", "urn:ietf:params:oauth:token-type:access_token") + form.Set("actor_token", actorToken) + form.Set("actor_token_type", "urn:ietf:params:oauth:token-type:access_token") + form.Set("resource", res) + return postForm(form)["access_token"].(string) + } + + delegated := exchange(resource) + apiBound := exchange(baseURL) + + mcpCall := func(bearer, body string) *http.Response { + req, err := http.NewRequest(http.MethodPost, resource, strings.NewReader(body)) + require.NoError(t, err) + req.Header.Set("Content-Type", "application/json") + req.Header.Set("Accept", "application/json, text/event-stream") + req.Header.Set("Authorization", "Bearer "+bearer) + resp, err := http.DefaultClient.Do(req) + require.NoError(t, err) + return resp + } + + // The bijection: each token opens exactly the surface its audience names. + refused := mcpCall(apiBound, mcpInitializeRPC) + defer func() { _ = refused.Body.Close() }() + assert.Equal(t, http.StatusUnauthorized, refused.StatusCode, + "a delegated token bound to the bare server URL must not open /mcp") + + accepted := mcpCall(delegated, mcpInitializeRPC) + defer func() { _ = accepted.Body.Close() }() + require.Equal(t, http.StatusOK, accepted.StatusCode, "delegated token must reach /mcp") + + // The intersection, through the real tool. The user is a viewer of + // document:readme (seeded above); this agent holds no grant at all, so + // every answer must be false — including the one the user can see. + callResp := mcpCall(delegated, `{"jsonrpc":"2.0","id":7,"method":"tools/call","params":{"name":"check_permissions",`+ + `"arguments":{"checks":[{"relation":"viewer","object":"document:readme"}]}}}`) + defer func() { _ = callResp.Body.Close() }() + require.Equal(t, http.StatusOK, callResp.StatusCode) + + var toolOut struct { + Result struct { + Content []struct { + Text string `json:"text"` + } `json:"content"` + IsError bool `json:"isError"` + } `json:"result"` + } + require.NoError(t, json.NewDecoder(callResp.Body).Decode(&toolOut)) + require.False(t, toolOut.Result.IsError) + require.NotEmpty(t, toolOut.Result.Content) + + var perms struct { + Results []struct { + Allowed bool `json:"allowed"` + } `json:"results"` + } + require.NoError(t, json.Unmarshal([]byte(toolOut.Result.Content[0].Text), &perms)) + require.Len(t, perms.Results, 1) + assert.False(t, perms.Results[0].Allowed, + "CONFUSED DEPUTY: the agent holds no grant, so it must be denied even "+ + "though the delegating user is a viewer of document:readme") + }) + // --- Surface 5: MCP (stdio subprocess, deprecated) -------------------- // The MCP subcommand is a separate process sharing the sqlite store, so // stop the server first to avoid two writers on one sqlite file. diff --git a/internal/integration_tests/mcp_delegated_test.go b/internal/integration_tests/mcp_delegated_test.go new file mode 100644 index 00000000..6f3eb1ba --- /dev/null +++ b/internal/integration_tests/mcp_delegated_test.go @@ -0,0 +1,288 @@ +package integration_tests + +import ( + "encoding/json" + "fmt" + "net/http" + "testing" + + "github.com/gin-gonic/gin" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/authorizerdev/authorizer/internal/config" + "github.com/authorizerdev/authorizer/internal/graph/model" + "github.com/authorizerdev/authorizer/internal/parsers" +) + +// This file covers RFC 8693 delegated tokens at the MCP surface: an agent +// holding "agent X acting for user Y" authority calling Authorizer's own MCP +// tools to ask about that authority. +// +// Every test starts at an HTTP endpoint and mints through the REAL +// /oauth/token. That is deliberate and load-bearing: an earlier round of +// delegation tests called CreateDelegatedAccessToken directly and asserted on +// the validator in isolation, and both passed while the feature was entirely +// unreachable in production (see agent_intersection_e2e_test.go). Testing the +// pieces proves nothing about the system. + +// mcpTestSetup pins the canonical URL to the running test server and returns +// the MCP resource identifier plus a router serving BOTH /oauth/token (to mint) +// and /mcp (to spend). +// +// The URL pinning is not incidental. MCP requires --url, and with it set +// parsers.GetHost returns the canonical value for every request regardless of +// headers — so a token's `iss`, the delegated validator's issuer check and the +// audience comparison all resolve to one value. Pinning it to the real listener +// address (rather than a fictional host) is what lets tokens minted through the +// real endpoint validate here: the mint helpers stamp testAuthorizerHost(ts) as +// the issuer, and a mismatch would reject every token in this file for a reason +// unrelated to the rule under test. +func mcpTestSetup(t *testing.T, ts *testSetup, cfg *config.Config) (string, http.Handler, http.Handler) { + t.Helper() + + cfg.MCPEnabled = true + cfg.AuthorizerURL = testAuthorizerHost(ts) + parsers.SetTrustedURL(cfg.AuthorizerURL) + t.Cleanup(func() { parsers.SetTrustedURL("") }) + + tokenRouter := gin.New() + tokenRouter.POST("/oauth/token", ts.HttpProvider.TokenHandler()) + + // Built after AuthorizerURL is set: mcpRouter captures cfg.MCPResource() at + // wiring time, exactly as internal/server does. + return cfg.MCPResource(), tokenRouter, mcpRouter(t, ts, cfg) +} + +// mcpToolCall invokes an MCP tool over the real Streamable HTTP transport and +// returns the tool's text content together with its isError flag. +func mcpToolCall(t *testing.T, router http.Handler, bearer, tool string, args map[string]interface{}) (string, bool) { + t.Helper() + + argsJSON, err := json.Marshal(args) + require.NoError(t, err) + body := fmt.Sprintf( + `{"jsonrpc":"2.0","id":9,"method":"tools/call","params":{"name":%q,"arguments":%s}}`, + tool, argsJSON) + + w := mcpPost(t, router, bearer, body) + require.Equal(t, http.StatusOK, w.Code, "tools/call body: %s", w.Body.String()) + + var resp struct { + Result struct { + Content []struct { + Text string `json:"text"` + } `json:"content"` + IsError bool `json:"isError"` + } `json:"result"` + } + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &resp), "raw: %s", w.Body.String()) + require.NotEmpty(t, resp.Result.Content, "a tool result must carry content; raw: %s", w.Body.String()) + return resp.Result.Content[0].Text, resp.Result.IsError +} + +// TestMCPDelegatedTokenReachesMCPSurface is the reachability acceptance test. +// +// It is the one that would have caught the feature being dead: /oauth/token +// requires `resource` to be an absolute URI and stamps it verbatim as `aud`, so +// a token bound to "/mcp" is producible — and before this change it was +// accepted nowhere at all, refused at /mcp by the stateful session check and at +// /graphql by the audience check. +func TestMCPDelegatedTokenReachesMCPSurface(t *testing.T) { + cfg := getTestConfig() + ts := initTestSetup(t, cfg) + resource, tokenRouter, router := mcpTestSetup(t, ts, cfg) + + delegated, _, _ := mintDelegatedViaEndpoint(t, ts, tokenRouter, resource) + + t.Run("handshake succeeds", func(t *testing.T) { + w := mcpPost(t, router, delegated, initializeRPC) + require.Equal(t, http.StatusOK, w.Code, "body: %s", w.Body.String()) + }) + + t.Run("the tool surface is reachable", func(t *testing.T) { + w := mcpPost(t, router, delegated, `{"jsonrpc":"2.0","id":2,"method":"tools/list"}`) + require.Equal(t, http.StatusOK, w.Code, "body: %s", w.Body.String()) + + var resp struct { + Result struct { + Tools []struct { + Name string `json:"name"` + } `json:"tools"` + } `json:"result"` + } + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &resp)) + + names := make([]string, 0, len(resp.Result.Tools)) + for _, tool := range resp.Result.Tools { + names = append(names, tool.Name) + } + assert.Contains(t, names, "check_permissions", + "the permission tools are the entire reason an agent presents a delegated token here") + }) +} + +// TestMCPDelegatedTokenAudienceBijection pins the invariant the widening had to +// preserve: +// +// f(aud) = surface, and it is a BIJECTION. +// +// Every token is valid at exactly ONE surface — the one its audience names. +// Both directions are asserted because a mistake in either is a real +// vulnerability, and because the obvious implementation of this feature breaks +// the first one silently: +// +// - ValidateDelegatedAccessToken has a single caller, +// GetUserIDFromSessionOrAccessToken, which is the default rule behind +// /graphql, /v1/* and gRPC. Relaxing its audience check in place — rather +// than adding ValidateDelegatedAccessTokenForResource — would have made +// every MCP-bound delegated token a full first-party API credential. +// - Matching "hostname OR resource" instead of exactly one would break the +// bijection from the other side, letting a token minted for the first-party +// API authenticate at /mcp. +func TestMCPDelegatedTokenAudienceBijection(t *testing.T) { + cfg := getTestConfig() + ts := initTestSetup(t, cfg) + resource, tokenRouter, router := mcpTestSetup(t, ts, cfg) + firstParty := cfg.AuthorizerURL + + mcpBound, _, _ := mintDelegatedViaEndpoint(t, ts, tokenRouter, resource) + apiBound, _, _ := mintDelegatedViaEndpoint(t, ts, tokenRouter, firstParty) + + t.Run("mcp-bound token is ACCEPTED at /mcp", func(t *testing.T) { + w := mcpPost(t, router, mcpBound, initializeRPC) + assert.Equal(t, http.StatusOK, w.Code, "body: %s", w.Body.String()) + }) + + t.Run("mcp-bound token is REJECTED at the first-party API", func(t *testing.T) { + // The default rule, i.e. what /graphql, /v1/* and gRPC all use. + _, err := ts.TokenProvider.GetUserIDFromSessionOrAccessToken( + bearerGinContext(t, ts, mcpBound)) + require.Error(t, err, + "an MCP token may be handed to a semi-trusted agent; it must never "+ + "become a full GraphQL/REST/gRPC credential") + }) + + t.Run("api-bound token is REJECTED at /mcp", func(t *testing.T) { + w := mcpPost(t, router, apiBound, initializeRPC) + assert.Equal(t, http.StatusUnauthorized, w.Code, + "the audience match must be EXACT: a delegated token for the "+ + "first-party API must not also open the MCP surface") + assert.Contains(t, w.Header().Get("WWW-Authenticate"), `error="invalid_token"`) + }) + + t.Run("api-bound token is still ACCEPTED at the first-party API", func(t *testing.T) { + // Regression guard: the refactor that added the resource-parameterized + // entry point must not have narrowed the existing one. + data, err := ts.TokenProvider.GetUserIDFromSessionOrAccessToken( + bearerGinContext(t, ts, apiBound)) + require.NoError(t, err) + require.NotNil(t, data) + assert.NotEmpty(t, data.ActorID, "the delegation actor must survive validation") + }) + + t.Run("an ordinary login token is still rejected at /mcp", func(t *testing.T) { + // The delegated fallback is gated on the `act` claim. A non-delegated + // token must take the unchanged stateful rejection, not the new path. + w := mcpPost(t, router, testAccessToken(t, ts), initializeRPC) + assert.Equal(t, http.StatusUnauthorized, w.Code) + }) +} + +// TestMCPDelegatedIntersectionThroughMCP is the security test that actually +// matters for this feature. +// +// The whole point of letting an agent reach /mcp is that the answer it gets is +// its OWN authority — perms(agent) ∩ perms(user) — not the delegating user's. +// If the intersection were dropped on this transport, an agent would ask +// "can I read payroll?", be told yes because the USER can, and the delegation +// model would be decorative exactly where a model is in the loop. +// +// The equivalent GraphQL assertions live in TestAgentIntersectionThroughGraphQL. +// This is the same property observed through the MCP tool surface, because the +// two reach resolveFgaCaller by different routes: GraphQL falls back to the +// request token, while MCP populates authctx.Principal from the interceptor. +func TestMCPDelegatedIntersectionThroughMCP(t *testing.T) { + cfg := getTestConfig() + ts, _ := initFGATestSetup(t, cfg) + _, ctx := createContext(ts) + resource, tokenRouter, router := mcpTestSetup(t, ts, cfg) + + setAdminCookie(t, ts) + _, err := ts.GraphQLProvider.FgaWriteModel(ctx, &model.FgaWriteModelInput{Dsl: fgaAgentModel}) + require.NoError(t, err) + + delegated, agentID, userID := mintDelegatedViaEndpoint(t, ts, tokenRouter, resource) + + // The USER can view the document. The AGENT has no grant of its own. + setAdminCookie(t, ts) + _, err = ts.GraphQLProvider.FgaWriteTuples(ctx, &model.FgaWriteTuplesInput{ + Tuples: []*model.FgaTupleInput{ + {User: "user:" + userID, Relation: "viewer", Object: "document:secret"}, + }, + }) + require.NoError(t, err) + + checkSecret := func(t *testing.T, args map[string]interface{}) bool { + t.Helper() + text, isErr := mcpToolCall(t, router, delegated, "check_permissions", args) + require.False(t, isErr, "check_permissions returned an error result: %s", text) + + var out struct { + Results []struct { + Allowed bool `json:"allowed"` + } `json:"results"` + } + require.NoError(t, json.Unmarshal([]byte(text), &out), "raw: %s", text) + require.Len(t, out.Results, 1) + return out.Results[0].Allowed + } + + selfCheck := map[string]interface{}{ + "checks": []map[string]string{{"relation": "can_view", "object": "document:secret"}}, + } + + t.Run("agent WITHOUT its own grant is denied though the user has access", func(t *testing.T) { + assert.False(t, checkSecret(t, selfCheck), + "CONFUSED DEPUTY: the agent holds no grant, so it must be denied over MCP "+ + "even though the delegating user can view the document") + }) + + t.Run("explicit self user must not drop the agent half", func(t *testing.T) { + assert.False(t, checkSecret(t, map[string]interface{}{ + "checks": selfCheck["checks"], + "user": "user:" + userID, + }), "echoing back your own subject must not shed the agent constraint") + }) + + t.Run("naming another subject is refused outright", func(t *testing.T) { + text, isErr := mcpToolCall(t, router, delegated, "check_permissions", map[string]interface{}{ + "checks": selfCheck["checks"], + "user": "user:someone-else", + }) + assert.True(t, isErr, + "a delegated caller may never widen its subject; got: %s", text) + }) + + t.Run("granting the agent allows the intersection", func(t *testing.T) { + setAdminCookie(t, ts) + _, wErr := ts.GraphQLProvider.FgaWriteTuples(ctx, &model.FgaWriteTuplesInput{ + Tuples: []*model.FgaTupleInput{ + {User: "agent:" + agentID, Relation: "viewer", Object: "document:secret"}, + }, + }) + require.NoError(t, wErr) + assert.True(t, checkSecret(t, selfCheck), + "both halves granted must allow — otherwise the tool is useless, not merely safe") + }) + + t.Run("list_permissions intersects too", func(t *testing.T) { + text, isErr := mcpToolCall(t, router, delegated, "list_permissions", map[string]interface{}{ + "relation": "can_view", + "object_type": "document", + }) + require.False(t, isErr, "list_permissions returned an error result: %s", text) + assert.Contains(t, text, "document:secret", + "the agent now holds the grant, so enumeration must include it") + }) +} diff --git a/internal/mcp/exposed_methods_test.go b/internal/mcp/exposed_methods_test.go index 2d69dc3c..499260a4 100644 --- a/internal/mcp/exposed_methods_test.go +++ b/internal/mcp/exposed_methods_test.go @@ -3,6 +3,8 @@ package mcp import ( "testing" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "google.golang.org/protobuf/proto" "google.golang.org/protobuf/reflect/protoreflect" "google.golang.org/protobuf/reflect/protoregistry" @@ -88,6 +90,52 @@ func TestExposedMCPToolsCannotBypassTheMCPTokenRule(t *testing.T) { t.Logf("checked %d mcp_tool-exposed methods", checked) } +// TestExposedMCPToolSetIsPinned fixes the exposed tool set so that adding one is +// a deliberate act with a review attached, not a one-line proto edit. +// +// The specific hazard this guards, beyond "the surface grew": +// +// Since /mcp accepts RFC 8693 delegated tokens (ValidateMCPAccessToken), a tool +// call can arrive with NO `nonce` claim — delegated tokens are stateless and +// carry none. MCPTokenResolver propagates that empty nonce into +// authctx.Principal, which is harmless for every tool below because none of them +// reads it. It is NOT harmless in general: service.Logout and service.Session +// both call MemoryStoreProvider.DeleteUserSession(sessionKey, nonce), so +// exposing a session-mutating RPC as an MCP tool would hand that code an empty +// nonce from a delegated caller. +// +// So: a new entry here is fine, but check what the underlying RPC does with +// SessionOrAccessTokenData.Nonce before adding it. +func TestExposedMCPToolSetIsPinned(t *testing.T) { + want := map[string]bool{ + "Meta": true, + "Profile": true, + "CheckPermissions": true, + "ListPermissions": true, + } + + got := map[string]bool{} + protoregistry.GlobalFiles.RangeFiles(func(fd protoreflect.FileDescriptor) bool { + svcs := fd.Services() + for i := 0; i < svcs.Len(); i++ { + methods := svcs.Get(i).Methods() + for j := 0; j < methods.Len(); j++ { + m := methods.Get(j) + if tool := mcpToolFromMethod(m); tool != nil && tool.GetExposed() { + got[string(m.Name())] = true + } + } + } + return true + }) + + require.NotEmpty(t, got, "the proto registry was not linked in, so this test proved nothing") + assert.Equal(t, want, got, + "the MCP tool set changed. Adding a tool is a security decision: confirm the RPC "+ + "does not consume SessionOrAccessTokenData.Nonce (a delegated caller supplies "+ + "none) and does not mutate session state, then update `want` in this test.") +} + // methodIsPublic mirrors interceptors.isPublicMethod. Duplicated rather than // exported across packages because it is two lines and this test must read the // annotation exactly as the interceptor does; a shared helper that drifted from diff --git a/internal/token/delegated_access_token.go b/internal/token/delegated_access_token.go index 2ba78950..a4f05127 100644 --- a/internal/token/delegated_access_token.go +++ b/internal/token/delegated_access_token.go @@ -37,16 +37,27 @@ import ( // # What is still enforced // // - Signature and expiry, via ParseJWTToken. +// // - An `act` claim MUST be present. Without it the token is not delegated and // has no business on this path. +// // - `aud` MUST equal this server's own URL. A token minted with an RFC 8707 // resource indicator carries that resource as its `aud` and is usable ONLY // there — accepting it here would be audience confusion and would make the // resource binding decorative. An agent that wants to call Authorizer must // explicitly request Authorizer's URL as the resource. +// +// The MCP transport needs the SAME checks against a DIFFERENT audience +// ("/mcp"), and takes ValidateDelegatedAccessTokenForResource rather +// than a second audience accepted here. Read that function before changing +// this one: the two entry points existing separately is what keeps a token +// valid at exactly one surface. +// // - Issuer/claims via ValidateJWTClaims, and token_type must be an access token. +// // - The subject must not be revoked or deactivated — a database lookup, not a // session lookup, so revoking a user still stops their agents. +// // - The session the delegation was derived from must still exist. See // DelegationSessionID: this is what makes logout and password reset stop a // delegated token here. @@ -60,6 +71,58 @@ import ( // that session issued. The signature, the short TTL and the audience binding // carry the rest. func (p *provider) ValidateDelegatedAccessToken(gc *gin.Context, accessToken string) (map[string]interface{}, error) { + // The first-party audience: this server's own URL. Unchanged behaviour — + // every caller of this function reaches Authorizer's own API surface + // (/graphql, /v1/*, gRPC) via GetUserIDFromSessionOrAccessToken. + return p.validateDelegatedForAudience(gc, accessToken, parsers.GetHost(gc)) +} + +// ValidateDelegatedAccessTokenForResource is ValidateDelegatedAccessToken with +// the accepted audience supplied by the caller instead of derived from the +// request host. It exists for exactly one caller — ValidateMCPAccessToken — and +// the separation is the security boundary, not a convenience. +// +// # Why a second entry point rather than a looser audience rule +// +// ValidateDelegatedAccessToken has ONE caller, +// GetUserIDFromSessionOrAccessToken, which is the default rule behind /graphql, +// /v1/* and gRPC. Widening the audience check INSIDE it — accepting +// "/mcp" alongside "" — would therefore make an MCP-bound delegated +// token valid on every first-party surface as a side effect. That is precisely +// the audience confusion firstPartyAudienceOK exists to prevent, and it would +// be asymmetric: the stateful path rejects every resource-bound audience while +// the delegated path silently accepted one. +// +// # The invariant this preserves +// +// f(aud) = surface, and it is a BIJECTION. +// +// Every token is valid at exactly one surface: the one its audience names. The +// match here is therefore EXACT (sameAudience), never "hostname or resource" — +// accepting both would break the bijection from the other side, letting a +// delegated token minted for the first-party API authenticate at /mcp too. +// +// Fails closed on an empty resource for the same reason ValidateMCPAccessToken +// does: an empty expected audience must never degrade into "accept anything". +func (p *provider) ValidateDelegatedAccessTokenForResource(gc *gin.Context, accessToken string, resource string) (map[string]interface{}, error) { + if strings.TrimSpace(resource) == "" { + return map[string]interface{}{}, fmt.Errorf(`unauthorized: no resource configured`) + } + return p.validateDelegatedForAudience(gc, accessToken, resource) +} + +// validateDelegatedForAudience is the shared core. expectedAud is the ONLY thing +// that varies between the first-party and MCP entry points; every other check — +// the act claim, delegation-session liveness, subject liveness, issuer, subject +// and token type — is identical by construction, so a fix to any of them cannot +// land on one surface and miss the other. +// +// Note that expectedAud is deliberately NOT used as the issuer. The `iss` claim +// is always this server's bare URL regardless of which resource the token is +// bound to, so the issuer check below keeps using parsers.GetHost. Conflating +// the two would make an MCP token's issuer "/mcp", which no token this +// server mints ever carries. +func (p *provider) validateDelegatedForAudience(gc *gin.Context, accessToken string, expectedAud string) (map[string]interface{}, error) { res := make(map[string]interface{}) if accessToken == "" { return res, fmt.Errorf(`unauthorized`) @@ -89,24 +152,31 @@ func (p *provider) ValidateDelegatedAccessToken(gc *gin.Context, accessToken str hostname := parsers.GetHost(gc) // Audience isolation, RFC 8707. /oauth/token requires `resource` to be an - // ABSOLUTE URI and stamps it verbatim as `aud`, so the only way to name - // this server is to request this server's own URL as the resource. The - // audience must therefore equal that URL — not the opaque --client-id, - // which no resource indicator can ever be. + // ABSOLUTE URI and stamps it verbatim as `aud`, so the only way to reach a + // surface here is to have named that surface's resource at exchange time: + // this server's own URL for the first-party API, "/mcp" for the MCP + // transport. The audience must equal the caller-supplied expectedAud — not + // the opaque --client-id, which no resource indicator can ever be. + // + // EXACT, never "one of". Each entry point passes exactly one expectedAud, so + // a token minted for the first-party API does not authenticate at /mcp and + // an MCP-bound token does not authenticate at /graphql. Relaxing this to + // accept both audiences is the one change that would collapse the bijection + // described on ValidateDelegatedAccessTokenForResource. // // Getting this wrong in the strict direction is not a safe failure: it // makes the delegated path unreachable by any token this deployment can // mint, so the feature silently does nothing. Getting it wrong in the loose - // direction accepts a token minted for a downstream resource server, which + // direction accepts a token minted for a different resource server, which // is audience confusion. Both are tested end to end through /oauth/token. // // Compared after trimming a trailing slash so "https://auth.example.com" // and "https://auth.example.com/" are the same audience — otherwise the // caller's exact spelling of the resource decides whether auth works. aud, _ := res["aud"].(string) - if !sameAudience(aud, hostname) { + if !sameAudience(aud, expectedAud) { if u, uErr := url.Parse(aud); uErr == nil && u.IsAbs() { - p.dependencies.Log.Debug().Str("aud", aud).Str("expected", hostname). + p.dependencies.Log.Debug().Str("aud", aud).Str("expected", expectedAud). Msg("delegated token rejected: audience names a different resource server") } return res, fmt.Errorf(`unauthorized: token audience is not this server`) diff --git a/internal/token/mcp_access_token.go b/internal/token/mcp_access_token.go index bb9bbfdc..fe7f2e32 100644 --- a/internal/token/mcp_access_token.go +++ b/internal/token/mcp_access_token.go @@ -47,15 +47,40 @@ import ( // uses. MCP is NOT a weaker path: it differs from the first-party check in // exactly one rule, the audience, and there it is the stricter of the two. // -// # What is deliberately NOT accepted +// # RFC 8693 delegated tokens ARE accepted, as a fallback // -// RFC 8693 delegated tokens. They are stateless by design — no nonce, no session -// entry — so they fail the core's session lookup, and ValidateDelegatedAccessToken -// requires `aud` to equal the bare server URL rather than the /mcp resource. An -// agent therefore cannot yet reach /mcp with a delegated token. That is a scoping -// decision, not an oversight: the delegated path gives up the byte-for-byte -// comparison against a stored token, and widening it is a deliberate edit to that -// function (see its doc comment), not a side effect of adding a transport. +// A delegated token is stateless by design — no nonce, no session entry — so it +// always fails the stateful core above. It is then retried against +// ValidateDelegatedAccessTokenForResource, which enforces every other check plus +// an EXACT match on this same `resource`. +// +// This is what lets an agent holding "agent X acting for user Y" authority ask +// Authorizer about that authority through the MCP tools it was granted. Without +// it, check_permissions was unreachable with the very token that proves the +// delegation, and agent-delegation over MCP was a stdio-only story. +// +// Three properties make the fallback safe rather than a hole: +// +// - Ordered as a FALLBACK, not a branch. A first-party MCP token is validated +// exactly as before and never touches the weaker path. +// - Gated on the `act` claim. Only a token that actually carries an actor is +// retried, so an ordinary wrong-audience login token is rejected once +// instead of paying a second full validation (JWT parse, session lookup, +// subject-liveness DB read) on the hot rejection path of an +// internet-facing endpoint. The claims are read from the stateful attempt's +// own signature-verified parse, so this costs nothing extra; a wrong hint +// could only cause a rejection, never an acceptance, because the delegated +// validator re-parses and re-verifies independently. +// - The audience match stays EXACT. A delegated token bound to the bare +// server URL — the kind that works at /graphql today — is still refused +// here, and an MCP-bound one is still refused there. See the bijection note +// on ValidateDelegatedAccessTokenForResource. +// +// What the delegated path gives up relative to the stateful one is the +// byte-for-byte comparison against a stored copy of the token. The compensating +// controls are the signature, the 5-minute DelegatedAccessTokenTTL, the exact +// audience binding, and delegationSessionIsLive — so logout, password reset and +// admin revoke still take an agent's access down with the user's session. func (p *provider) ValidateMCPAccessToken(gc *gin.Context, accessToken string, resource string) (map[string]interface{}, error) { if resource == "" { // Fail closed. An empty expected audience would make the comparison @@ -63,7 +88,7 @@ func (p *provider) ValidateMCPAccessToken(gc *gin.Context, accessToken string, r // subtle for an auth path: say no explicitly. return map[string]interface{}{}, fmt.Errorf(`unauthorized: no mcp resource configured`) } - return p.validateStatefulAccessToken(gc, accessToken, func(aud string) error { + claims, err := p.validateStatefulAccessToken(gc, accessToken, func(aud string) error { if !sameAudience(aud, resource) { p.dependencies.Log.Debug().Str("aud", aud).Str("expected", resource). Msg("access token rejected at mcp: audience names a different resource") @@ -71,4 +96,14 @@ func (p *provider) ValidateMCPAccessToken(gc *gin.Context, accessToken string, r } return nil }) + if err == nil { + return claims, nil + } + // Not a delegated token: report the stateful failure as-is. ImmediateActor + // reads the claims validateStatefulAccessToken already parsed and + // signature-verified; an empty map (parse failure) yields "" and stops here. + if ImmediateActor(claims) == "" { + return claims, err + } + return p.ValidateDelegatedAccessTokenForResource(gc, accessToken, resource) } diff --git a/internal/token/provider.go b/internal/token/provider.go index 2fbf7ff8..2115687a 100644 --- a/internal/token/provider.go +++ b/internal/token/provider.go @@ -86,11 +86,19 @@ type Provider interface { // ValidateAccessToken by exactly one property (no session lookup) and // stricter by one (audience must be this server) — see its doc comment. ValidateDelegatedAccessToken(gc *gin.Context, accessToken string) (map[string]interface{}, error) + // ValidateDelegatedAccessTokenForResource is ValidateDelegatedAccessToken + // with the accepted audience supplied by the caller rather than derived from + // the request host. Exists so the MCP surface can accept delegated tokens + // bound to "/mcp" WITHOUT widening the first-party rule — see its doc + // comment for the bijection that separation preserves. + ValidateDelegatedAccessTokenForResource(gc *gin.Context, accessToken string, resource string) (map[string]interface{}, error) // ValidateMCPAccessToken validates an access token presented at the MCP // surface. Same stateful core as ValidateAccessToken, differing in exactly // one rule: `aud` must equal the caller-supplied canonical MCP resource URI // (RFC 8707 / MCP authorization), which is the audience ValidateAccessToken - // rejects. Every other check, subject liveness included, is shared. + // rejects. Every other check, subject liveness included, is shared. Falls + // back to ValidateDelegatedAccessTokenForResource for tokens carrying an + // RFC 8693 `act` claim. ValidateMCPAccessToken(gc *gin.Context, accessToken string, resource string) (map[string]interface{}, error) // ValidateAdminToken validates session token ValidateBrowserSession(gc *gin.Context, encryptedSession string) (*SessionData, error)