From c37c6ce50fa9af9d0479756550f7b4a87fa16f8f Mon Sep 17 00:00:00 2001 From: Jakub Zych Date: Fri, 23 Oct 2026 20:51:57 +0200 Subject: [PATCH] fix(19.7): RC-WR-01 a backend grant acts only as the administrator it was consented for pact.MCPGrant gains BackendUserID, the backend account the consent page showed. pier gives an OAuth identity a backend subject (on resolution and after a link event) only while the user's link names that account; a link to anyone else, or no recorded account, gives none, so a link changed after the consent never re-targets the grant. --- docs/services/mcp.md | 2 +- modules/pact/README.md | 2 +- modules/pact/mcp.go | 9 +++++- modules/pier/README.md | 2 +- modules/pier/events.go | 20 ++------------ modules/pier/fanout_test.go | 19 +++++++++++++ modules/pier/helpers_test.go | 21 ++++++++++++-- modules/pier/identity.go | 49 ++++++++++++++++++++++----------- modules/pier/tools_list_test.go | 31 ++++++++++++++++++--- 9 files changed, 112 insertions(+), 43 deletions(-) diff --git a/docs/services/mcp.md b/docs/services/mcp.md index affd4e87..3fe2b37a 100644 --- a/docs/services/mcp.md +++ b/docs/services/mcp.md @@ -126,7 +126,7 @@ What pier does with it at boot: ## Identity and sessions -Every request is authorized afresh from its bearer; a session never replaces the bearer. An OAuth access token of the application's authorization server (`oas_at_…`) is verified through the `pact.MCPTokenVerifier` the OAuth server plugin publishes: the grant's user is the frontend subject, the grant's scopes are the ceiling. With the `backend` scope and a link the published `pact.MCPLinkedBackendResolver` confirms, the linked administrator is the backend subject, resolved on every request (cached for at most five seconds on one instance and dropped at once by a link event), so unlinking takes effect on the next call with no token surgery. The backend principal keeps the administrator's real permissions, superuser included, under the grant's ceiling. +Every request is authorized afresh from its bearer; a session never replaces the bearer. An OAuth access token of the application's authorization server (`oas_at_…`) is verified through the `pact.MCPTokenVerifier` the OAuth server plugin publishes: the grant's user is the frontend subject, the grant's scopes are the ceiling. With the `backend` scope and a link the published `pact.MCPLinkedBackendResolver` confirms, the linked administrator is the backend subject, resolved on every request (cached for at most five seconds on one instance and dropped at once by a link event), so unlinking takes effect on the next call with no token surgery. The verifier also reports which administrator the user consented to (`pact.MCPGrant.BackendUserID`); while the link names anyone else, or when the grant records none, the grant has no backend subject, so a link that changes after the consent never re-targets it. The backend principal keeps the administrator's real permissions, superuser included, under the grant's ceiling. The `read` and `write` scopes are a ceiling: a grant with only `read` sees tools whose read-only hint is true, `write` unlocks the rest, and permissions always filter on top. The `ai` and `offline_access` scopes play no part in it. diff --git a/modules/pact/README.md b/modules/pact/README.md index 261d32cd..bd42139d 100644 --- a/modules/pact/README.md +++ b/modules/pact/README.md @@ -160,7 +160,7 @@ func (p *Plugin) Schedule() []pact.ScheduledCommand { | `pact.MCPGuards` | The registry of guard providers published on the application; `pact.NewMCPGuards` creates it, `Register` refuses an empty or duplicate name with a `pact:` error naming the guard, `Lookup` finds one and `Names` lists them sorted. Safe for concurrent use. | | `pact.MCPSubject` | The identity an MCP session acts as under one guard: guard name, user id, granted scopes, OAuth client id and personal token id. | | `pact.MCPDispatch` | A guard provider's answer for one call: the principal (as `any`), an optional credential and the bouncer guard names that honour the dispatch marker. | -| `pact.MCPTokenVerifier` | Published by the OAuth server plugin: verifies an access token at the MCP endpoint into a `pact.MCPGrant` (user, client, token, scopes, expiry) and names its resource. A refused token is `pact.ErrMCPTokenInvalid`. | +| `pact.MCPTokenVerifier` | Published by the OAuth server plugin: verifies an access token at the MCP endpoint into a `pact.MCPGrant` (user, client, token, scopes, expiry and, for a `backend` grant, `BackendUserID`, the backend account the consent showed) and names its resource. A refused token is `pact.ErrMCPTokenInvalid`. | | `pact.MCPPersonalTokenResolver` | Published by the one plugin whose personal tokens may open an MCP session (one resolver per application): resolves a raw token into a `pact.MCPSubject` whose `TokenID` identifies the token row (it keys the session binding; a subject without one is refused); false when the prefix is not its own, which the endpoint answers as unauthorized. | | `pact.MCPLinkedBackendResolver` | Published by the plugin that owns the frontend-to-backend account link: the `pact.LinkedBackendUser` (id, login, display name) of a frontend user, false when unlinked. | | `pact.MCPSubjectChanged` | Event fired on the festival bus when a subject's permissions, link, grants or existence change (`Kind` is `pact.MCPSubjectPermissions`, `pact.MCPSubjectLink`, `pact.MCPSubjectRevoked` or `pact.MCPSubjectDeleted`), so the MCP endpoint re-lists tools or ends sessions. `TokenID` and `ClientID` narrow a revoked event: a token id ends only the sessions that personal token opened (subject with that `TokenID` and no `ClientID`), a client id only that OAuth client's sessions, neither every session of the subject; other kinds ignore them. | diff --git a/modules/pact/mcp.go b/modules/pact/mcp.go index 0b4bdd78..52288545 100644 --- a/modules/pact/mcp.go +++ b/modules/pact/mcp.go @@ -225,13 +225,20 @@ type MCPTokenVerifier interface { } // MCPGrant is a verified OAuth access token: the frontend user, the client, -// the token row, the granted scopes and the expiry. +// the token row, the granted scopes, the expiry and, for a grant carrying +// the backend scope, the backend account the user consented to. type MCPGrant struct { UserID uint ClientID string TokenID uint Scopes []string ExpiresAt time.Time + // BackendUserID is the backend account the consent page showed when + // the user granted the backend scope. The MCP endpoint gives the grant + // a backend subject only while the user's current link names this + // account, so a grant never acts as an administrator its consent did + // not show; zero (none recorded) gives no backend subject. + BackendUserID uint } // ErrMCPTokenInvalid is what an MCPTokenVerifier answers for a token it does diff --git a/modules/pier/README.md b/modules/pier/README.md index 48dcd485..19cf57bb 100644 --- a/modules/pier/README.md +++ b/modules/pier/README.md @@ -10,7 +10,7 @@ Opt-in MCP endpoint that lists the tools and prompts plugins declare per caller pier mounts a streamable HTTP MCP endpoint (POST, GET and DELETE at `mcp.path`, default `/mcp`) inside the application binary, on the official Go SDK. A plugin takes part by implementing `pact.HasMCP`: each `pact.MCPTool` names one of the plugin's own routes by method and path, an input struct whose tags say which fields fill the path, the query string and the body, and the guard the tool runs as. At boot pier compiles every declaration against the route table (an unknown route, a duplicate or invalid name, a route that is not token-guarded, a non-object input, a `path:` field the schema does not require, an embedded struct on a `GET` or `DELETE` tool all fail boot naming the plugin and the tool), reflects the input schema from the struct and prefixes the exposed name with the plugin's short code (`acme.blog` declaring `create_post` exposes `blog_create_post`). -Every request to the endpoint is authorized afresh from its bearer: an OAuth access token through the `pact.MCPTokenVerifier` the OAuth server plugin publishes, or, behind `mcp.allow_personal_tokens`, a personal API token (an admin `g15_` token through cabana, a frontend token through the published `pact.MCPPersonalTokenResolver`). The resulting `pier.Identity` names a frontend subject and, when the grant carries the `backend` scope and the published `pact.MCPLinkedBackendResolver` confirms a link, a backend subject. `tools/list` is computed for that identity: a tool needs a subject under its guard, a grant without `write` sees only read-only tools, and the guard provider's own check (the same logic its route middleware enforces) decides the rest. A `tools/call` runs the bound route's compiled middleware chain in-process with the `bouncer.Dispatch` marker carrying the principal the guard provider resolved; the inner request never carries an `Authorization` or `Cookie` header. A 2xx answer becomes text plus structured content, anything else a tool error carrying the route's own body. +Every request to the endpoint is authorized afresh from its bearer: an OAuth access token through the `pact.MCPTokenVerifier` the OAuth server plugin publishes, or, behind `mcp.allow_personal_tokens`, a personal API token (an admin `g15_` token through cabana, a frontend token through the published `pact.MCPPersonalTokenResolver`). The resulting `pier.Identity` names a frontend subject and, when the grant carries the `backend` scope and the published `pact.MCPLinkedBackendResolver` confirms a link, a backend subject, but only while that link names the administrator the grant was consented for (`pact.MCPGrant.BackendUserID`): a link changed to anyone else, or a grant that records no consented administrator, gets no backend subject. `tools/list` is computed for that identity: a tool needs a subject under its guard, a grant without `write` sees only read-only tools, and the guard provider's own check (the same logic its route middleware enforces) decides the rest. A `tools/call` runs the bound route's compiled middleware chain in-process with the `bouncer.Dispatch` marker carrying the principal the guard provider resolved; the inner request never carries an `Authorization` or `Cookie` header. A 2xx answer becomes text plus structured content, anything else a tool error carrying the route's own body. Sessions (`Mcp-Session-Id`) are bound to the identity that opened them (client and user, or the personal token), live in a store with a sliding idle TTL, and are served by a per-session SDK server on whichever instance receives the request, rehydrated from the stored session state when it is not local. The memory store serves one instance; the Redis store shares sessions across instances and fans subject events out to every instance, so a permission or link change pushes `notifications/tools/list_changed` to the sessions it concerns and a revocation ends them everywhere. Clients of protocol 2026-07-28 and later, which have no sessions, are served statelessly from the same path by the SDK's stateless handler over per-identity servers, sharing identity, dispatch and filtering with the stateful path. diff --git a/modules/pier/events.go b/modules/pier/events.go index 03ac1a77..c0199b5b 100644 --- a/modules/pier/events.go +++ b/modules/pier/events.go @@ -283,27 +283,13 @@ func (b *identityBox) set(id Identity) { } // refreshBackend re-resolves an OAuth identity's backend subject through -// the link resolver (after a link event), adding or dropping it. +// the link resolver (after a link event), adding or dropping it; only the +// administrator the grant was consented for is ever added. func (r *identityResolver) refreshBackend(ctx context.Context, id *Identity) { if id.TokenKind != TokenKindOAuth || id.Frontend == nil { return } - id.Backend = nil - for _, s := range id.Scopes { - if s != "backend" { - continue - } - if link, ok := r.linkedBackend(ctx, id.Frontend.UserID); ok { - id.Backend = &pact.MCPSubject{ - Guard: BackendGuard, - UserID: link.ID, - Scopes: id.Scopes, - ClientID: id.Frontend.ClientID, - TokenID: id.Frontend.TokenID, - } - } - return - } + id.Backend = r.consentedBackendSubject(ctx, id.Frontend.UserID, id.Frontend.ClientID, id.Frontend.TokenID, id.Scopes, id.consentedBackend) } // forgetAllLinks drops the whole link cache. diff --git a/modules/pier/fanout_test.go b/modules/pier/fanout_test.go index 678cc549..d0c35ea2 100644 --- a/modules/pier/fanout_test.go +++ b/modules/pier/fanout_test.go @@ -82,6 +82,25 @@ func TestFanOut(t *testing.T) { t.Fatalf("after linking = %v", got) } + // Relinked to another administrator: the grant was consented for + // admin 42, so the event drops its backend subject instead of + // re-targeting it (RC-WR-01); linking 42 again restores it. + links.set(10, 43) + fireSubjectChanged(t, a.app, "acme", 10, pact.MCPSubjectLink) + waitChanged(t, changed) + if got := listNames(t, ctx, cs); slices.Contains(got, "office_posts") { + t.Fatalf("after relinking to another administrator = %v", got) + } + if id, err := b.pier().Resolve(ctx, "oas_at_admin"); err != nil || id.Backend != nil { + t.Fatalf("backend subject linked to another administrator = %+v, %v", id.Backend, err) + } + links.set(10, 42) + fireSubjectChanged(t, a.app, "acme", 10, pact.MCPSubjectLink) + waitChanged(t, changed) + if got := listNames(t, ctx, cs); !slices.Contains(got, "office_posts") { + t.Fatalf("after linking the consented administrator again = %v", got) + } + // Unlinked without an event: the instance keeps its cached answer for // up to LinkCacheTTL; the link event drops it at once. links.remove(10) diff --git a/modules/pier/helpers_test.go b/modules/pier/helpers_test.go index 7cb5be44..8e19186a 100644 --- a/modules/pier/helpers_test.go +++ b/modules/pier/helpers_test.go @@ -534,9 +534,26 @@ func (g methodGuard) MCPAllowsMethod(_ context.Context, subject pact.MCPSubject, return g.decide(subject, req, perms, method), nil } -// grant builds one verifier answer valid for an hour. +// consentedAdmin is the backend account every backend grant of grant was +// consented for; the tests that link a user link it to this administrator. +const consentedAdmin = 42 + +// grant builds one verifier answer valid for an hour. A grant carrying the +// backend scope was consented for consentedAdmin. func grant(user uint, client string, token uint, scopes ...string) pact.MCPGrant { - return pact.MCPGrant{UserID: user, ClientID: client, TokenID: token, Scopes: scopes, ExpiresAt: time.Now().Add(time.Hour)} + g := pact.MCPGrant{UserID: user, ClientID: client, TokenID: token, Scopes: scopes, ExpiresAt: time.Now().Add(time.Hour)} + if slices.Contains(scopes, "backend") { + g.BackendUserID = consentedAdmin + } + return g +} + +// boundGrant is grant consented for the backend account admin (zero: none +// recorded). +func boundGrant(user uint, client string, token, admin uint, scopes ...string) pact.MCPGrant { + g := grant(user, client, token, scopes...) + g.BackendUserID = admin + return g } // fakeVerifier answers the grants it holds; revoke drops one. Its default diff --git a/modules/pier/identity.go b/modules/pier/identity.go index 03518dec..29bcd394 100644 --- a/modules/pier/identity.go +++ b/modules/pier/identity.go @@ -57,6 +57,10 @@ type Identity struct { // credential is the admin personal token row stored next to the // backend principal on a dispatched call. credential any + // consentedBackend is the backend account an OAuth grant was consented + // for (pact.MCPGrant.BackendUserID); the backend subject exists only + // while the user's link names it. + consentedBackend uint } // subjectFor returns the subject acting under guard, nil when the identity @@ -208,11 +212,12 @@ func (r *identityResolver) resolveOAuth(ctx context.Context, raw string) (Identi } scopes := append([]string(nil), grant.Scopes...) id := Identity{ - Scopes: scopes, - Binding: "oauth:" + grant.ClientID + ":" + strconv.FormatUint(uint64(grant.UserID), 10), - ExpiresAt: grant.ExpiresAt, - TokenKind: TokenKindOAuth, - ceiling: true, + Scopes: scopes, + Binding: "oauth:" + grant.ClientID + ":" + strconv.FormatUint(uint64(grant.UserID), 10), + ExpiresAt: grant.ExpiresAt, + TokenKind: TokenKindOAuth, + ceiling: true, + consentedBackend: grant.BackendUserID, } if r.frontendGuard != "" { id.Frontend = &pact.MCPSubject{ @@ -223,20 +228,32 @@ func (r *identityResolver) resolveOAuth(ctx context.Context, raw string) (Identi TokenID: grant.TokenID, } } - if slices.Contains(scopes, "backend") { - if link, ok := r.linkedBackend(ctx, grant.UserID); ok { - id.Backend = &pact.MCPSubject{ - Guard: BackendGuard, - UserID: link.ID, - Scopes: scopes, - ClientID: grant.ClientID, - TokenID: grant.TokenID, - } - } - } + id.Backend = r.consentedBackendSubject(ctx, grant.UserID, grant.ClientID, grant.TokenID, scopes, grant.BackendUserID) return id, nil } +// consentedBackendSubject is the backend subject of an OAuth grant of +// userID: the linked administrator, when the grant carries the backend +// scope and the link names consented, the account the grant was consented +// for. A link to anyone else, no link or no consented account gives none, +// so a link changed after the consent never re-targets the grant. +func (r *identityResolver) consentedBackendSubject(ctx context.Context, userID uint, clientID string, tokenID uint, scopes []string, consented uint) *pact.MCPSubject { + if consented == 0 || !slices.Contains(scopes, "backend") { + return nil + } + link, ok := r.linkedBackend(ctx, userID) + if !ok || link.ID != consented { + return nil + } + return &pact.MCPSubject{ + Guard: BackendGuard, + UserID: link.ID, + Scopes: scopes, + ClientID: clientID, + TokenID: tokenID, + } +} + func (r *identityResolver) resolveBackendToken(ctx context.Context, raw string) (Identity, error) { principal, row, scopes, err := r.backendTokens(ctx, r.app, raw) if err != nil || principal == nil || row == nil { diff --git a/modules/pier/tools_list_test.go b/modules/pier/tools_list_test.go index c8a6f00e..41430c2f 100644 --- a/modules/pier/tools_list_test.go +++ b/modules/pier/tools_list_test.go @@ -19,8 +19,8 @@ import ( // listFixture is the two-guard application of TestToolsList: the frontend // guard "acme" (user 8 lacks acme.echo) with a write and a read tool, the // "backend" guard with one read tool, the verifier's grants for users 7, 8, -// 9 (read only), 10 (backend, linked to admin 42) and 11 (backend, no -// link), and the hit counters of the write route and the backend route. +// 9 (read only), 10 (backend, linked to admin 42, plus grants consented for +// admin 43 and for none) and 11 (backend, no link), and the hit counters of the write route and the backend route. type listFixture struct { in *instance echoHits *atomic.Int64 @@ -39,6 +39,10 @@ func newListFixture(t *testing.T) listFixture { verifier := newFakeVerifier(map[string]pact.MCPGrant{ "oas_at_admin": grant(10, "c4", 4, "read", "write", "backend"), "oas_at_nolink": grant(11, "c5", 5, "read", "write", "backend"), + // User 10's grants consented for another administrator and for + // none (RC-WR-01). + "oas_at_rebound": boundGrant(10, "c8", 8, 43, "read", "write", "backend"), + "oas_at_unbound": boundGrant(10, "c9", 9, 0, "read", "write", "backend"), }) principalOf := func(w http.ResponseWriter, r *http.Request) { user, _ := bouncer.User(r.Context()) @@ -74,8 +78,10 @@ func newListFixture(t *testing.T) listFixture { // TestToolsList (SC-3, D-20, D-23): tools/list is computed per principal; // a read-only grant sees only read-only tools; a tool whose guard has no // subject for the caller is absent; calling a tool outside the caller's -// list is a JSON-RPC error that never reaches the route; an identity with -// both a frontend and a backend subject sees the union. +// list is a JSON-RPC error that never reaches the route; a backend grant +// whose consented administrator is not the linked one gets no backend +// subject (RC-WR-01); an identity with both a frontend and a backend +// subject sees the union. func TestToolsList(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) defer cancel() @@ -113,6 +119,23 @@ func TestToolsList(t *testing.T) { } }) + t.Run("a backend grant acts only as the administrator it was consented for", func(t *testing.T) { + before := f.officeHits.Load() + for _, token := range []string{"oas_at_rebound", "oas_at_unbound"} { + cs := sdkClient(t, ctx, f.in.url(), token, clientOpts{}) + if got := listNames(t, ctx, cs); !slices.Equal(got, []string{"acme_echo", "acme_show"}) { + t.Fatalf("%s (user 10, linked to admin 42) lists %v, want the frontend tools only", token, got) + } + res, err := cs.CallTool(ctx, &mcp.CallToolParams{Name: "office_posts"}) + if err == nil || !strings.Contains(err.Error(), "not available to this caller") { + t.Fatalf("%s calling office_posts: %+v, %v", token, res, err) + } + } + if f.officeHits.Load() != before { + t.Fatalf("a grant consented for another administrator reached the backend route") + } + }) + t.Run("unlisted call never reaches the route", func(t *testing.T) { before, office := f.echoHits.Load(), f.officeHits.Load() for token, tool := range map[string]string{"oas_at_other": "acme_echo", "oas_at_read": "acme_echo", "oas_at_test": "office_posts"} {