From 25fd981d7fa01fb4174288d80be00e87d745eba5 Mon Sep 17 00:00:00 2001 From: Jakub Zych Date: Wed, 28 Oct 2026 15:01:04 +0100 Subject: [PATCH] feat(32.1-02): expand OAuth MCP scopes from the published catalog - Publish the compiled member-token catalog as pact.MemberTokenScopeCatalog - Imply declared names only when the MCP subject has a ClientID - Reject a bad Implies and the reserved names backend and offline_access --- classes/api_token_scopes.go | 51 +++++++-- classes/mcp_guard.go | 55 ++++++++-- go.mod | 2 +- mcp_implies_test.go | 213 ++++++++++++++++++++++++++++++++++++ plugin.go | 8 +- 5 files changed, 305 insertions(+), 24 deletions(-) create mode 100644 mcp_implies_test.go diff --git a/classes/api_token_scopes.go b/classes/api_token_scopes.go index d8da97f..b09bb4d 100644 --- a/classes/api_token_scopes.go +++ b/classes/api_token_scopes.go @@ -15,23 +15,29 @@ import ( // a built-in (read, write, ai) or one a plugin declared through // pact.HasFrontendTokenScopes. Label and Description are phrase keys // (containing "::") or literal text; PluginID is the declaring plugin, empty -// for a built-in. +// for a built-in. Implies is empty on a built-in. On a declared scope it is +// empty, read, or write: an OAuth MCP grant of that stock scope also +// satisfies the declared name. The name is not stored on the grant. type APITokenScope struct { Name string Label string Description string PluginID string BuiltIn bool + Implies string } // APITokenScopeCatalog is the compiled list of member token scopes of one // application: the built-ins first, then the declared scopes in activation -// order. Boot publishes it; APITokenScopesFor reads it. +// order. Boot publishes it, and the same value as pact.MemberTokenScopeCatalog. +// APITokenScopesFor reads the concrete catalog. type APITokenScopeCatalog struct { scopes []APITokenScope byName map[string]int } +var _ pact.MemberTokenScopeCatalog = (*APITokenScopeCatalog)(nil) + // Scopes returns a copy of the catalog, built-ins first. func (c *APITokenScopeCatalog) Scopes() []APITokenScope { if c == nil { @@ -49,6 +55,23 @@ func (c *APITokenScopeCatalog) Allowed(name string) bool { return ok } +// MemberTokenScopes returns the compiled list as pact scopes, built-ins +// first, then declared scopes. Implied names are not a separate entry: they +// are the Implies field of the declared scope. +func (c *APITokenScopeCatalog) MemberTokenScopes() []pact.FrontendTokenScope { + scopes := c.Scopes() + out := make([]pact.FrontendTokenScope, 0, len(scopes)) + for _, scope := range scopes { + out = append(out, pact.FrontendTokenScope{ + Name: scope.Name, + Label: scope.Label, + Description: scope.Description, + Implies: scope.Implies, + }) + } + return out +} + // builtInAPITokenScopeKeys are the label and description phrase keys of // the built-in scopes, in AllowedAPITokenScopes order. var builtInAPITokenScopeKeys = map[string][2]string{ @@ -78,12 +101,14 @@ func newAPITokenScopeCatalog(scopes []APITokenScope) *APITokenScopeCatalog { // CompileAPITokenScopes builds the member token scope catalog from the // built-ins and every plugin that implements pact.HasFrontendTokenScopes, in // the order given (party.Activated). A declared name is trimmed and must -// pass pact.ValidTokenScopeName, must not be a built-in (read, write, ai) -// and must not repeat a name another plugin (or the same one) declared; its -// label and description must not be empty. Any violation is an error, which -// fails boot. Phrase keys are checked against the translator by Boot. Every -// name of a catalog that compiles is added to the process-wide set the -// legacy MintAPIToken accepts. +// pass pact.ValidTokenScopeName, must not be a built-in (read, write, ai), +// must not be the reserved name backend or offline_access, and must not +// repeat a name another plugin (or the same one) declared; its label and +// description must not be empty, and Implies must be empty, read, or write. +// Built-ins keep Implies empty. Any violation is an error, which fails boot. +// Phrase keys are checked against the translator by Boot. Every name of a +// catalog that compiles is added to the process-wide set the legacy +// MintAPIToken accepts. func CompileAPITokenScopes(plugins []party.Plugin) (*APITokenScopeCatalog, error) { scopes := builtInAPITokenScopes() owner := map[string]string{} @@ -111,14 +136,22 @@ func CompileAPITokenScopes(plugins []party.Plugin) (*APITokenScopeCatalog, error } return nil, fmt.Errorf("golem15.user: duplicate member token scope %q declared by %s and %s", name, other, id) } + if name == "backend" || name == "offline_access" { + return nil, fmt.Errorf("golem15.user: plugin %s declared the reserved member token scope %q", id, name) + } if strings.TrimSpace(scope.Label) == "" { return nil, fmt.Errorf("golem15.user: plugin %s member token scope %s has an empty label", id, name) } if strings.TrimSpace(scope.Description) == "" { return nil, fmt.Errorf("golem15.user: plugin %s member token scope %s has an empty description", id, name) } + if scope.Implies != "" && scope.Implies != "read" && scope.Implies != "write" { + return nil, fmt.Errorf("golem15.user: plugin %s member token scope %s has implies %q", id, name, scope.Implies) + } owner[name] = id - scopes = append(scopes, APITokenScope{Name: name, Label: scope.Label, Description: scope.Description, PluginID: id}) + scopes = append(scopes, APITokenScope{ + Name: name, Label: scope.Label, Description: scope.Description, PluginID: id, Implies: scope.Implies, + }) declared = append(declared, name) } } diff --git a/classes/mcp_guard.go b/classes/mcp_guard.go index 441166d..e23fb0e 100644 --- a/classes/mcp_guard.go +++ b/classes/mcp_guard.go @@ -84,12 +84,16 @@ func (g *MCPUserGuard) MCPRequirements(middleware []string) (pact.MCPRequirement return req, nil } -// MCPAllows requires every scope of req in the subject's scopes, then every -// permission code of req and of the tool, one by one (the route runs one -// user.permission middleware per code), granted to the user's groups. +// MCPAllows requires every scope of req in the subject's effective scopes, +// then every permission code of req and of the tool, one by one (the route +// runs one user.permission middleware per code), granted to the user's +// groups. Effective scopes are the grant itself for a personal token, and +// for an OAuth subject also the declared names the catalog implies. The +// expanded list is not written back onto the subject. func (g *MCPUserGuard) MCPAllows(ctx context.Context, subject pact.MCPSubject, req pact.MCPRequirements, permissions []string) (bool, error) { + scopes := g.mcpEffectiveScopes(subject) for _, scope := range req.Scopes { - if !slices.Contains(subject.Scopes, scope) { + if !slices.Contains(scopes, scope) { return false, nil } } @@ -114,13 +118,16 @@ func (g *MCPUserGuard) MCPAllows(ctx context.Context, subject pact.MCPSubject, r // MCPDispatch builds the principal (the user with the group grants, as the // api_token guard loads it) and an in-memory token credential carrying the -// session's scopes, so user.scope sees what the session was granted. For a -// personal-token subject (ClientID empty) the credential's ID is the -// subject's TokenID, the user_api_tokens row the session was opened with, so -// a handler can record which token made a change; an OAuth subject's TokenID -// is the OAuth server's own token row, not one of this plugin's, and its -// credential keeps ID 0. The credential is never saved. The marker is -// honoured by the api_token guard only. +// subject's effective scopes, so user.scope sees what the session may call. +// For an OAuth subject that includes declared names the catalog marks as +// implied by a stock scope the grant holds; a personal token's scopes are +// the grant unchanged. For a personal-token subject (ClientID empty) the +// credential's ID is the subject's TokenID, the user_api_tokens row the +// session was opened with, so a handler can record which token made a +// change; an OAuth subject's TokenID is the OAuth server's own token row, +// not one of this plugin's, and its credential keeps ID 0. The credential +// is never saved, and implied names are not written onto a token row or +// onto subject.Scopes. The marker is honoured by the api_token guard only. func (g *MCPUserGuard) MCPDispatch(ctx context.Context, subject pact.MCPSubject, _ string) (pact.MCPDispatch, error) { db, err := g.db() if err != nil { @@ -135,7 +142,7 @@ func (g *MCPUserGuard) MCPDispatch(ctx context.Context, subject pact.MCPSubject, } credential := &models.APIToken{ UserID: subject.UserID, - Scopes: lagoon.Jsonable[[]string]{Data: slices.Clone(subject.Scopes), Valid: true}, + Scopes: lagoon.Jsonable[[]string]{Data: g.mcpEffectiveScopes(subject), Valid: true}, } if subject.ClientID == "" { credential.ID = subject.TokenID @@ -143,6 +150,30 @@ func (g *MCPUserGuard) MCPDispatch(ctx context.Context, subject pact.MCPSubject, return pact.MCPDispatch{Principal: principal, Credential: credential, Guards: []string{"api_token"}}, nil } +// mcpEffectiveScopes is the scope list MCPAllows and MCPDispatch share. +// A personal token (ClientID empty) is returned unchanged: the same slice, +// with no declared name added. An OAuth subject gets a clone of its grant +// plus each catalog name whose Implies is read or write, when the clone +// holds that stock scope and not the name. write does not satisfy Implies +// read. Nothing the catalog does not mark is appended. The clone is not +// written back onto subject.Scopes, a token row, or a fingerprint. +func (g *MCPUserGuard) mcpEffectiveScopes(subject pact.MCPSubject) []string { + if subject.ClientID == "" { + return subject.Scopes + } + scopes := slices.Clone(subject.Scopes) + for _, entry := range APITokenScopesFor(g.app()).MemberTokenScopes() { + if entry.Implies != "read" && entry.Implies != "write" { + continue + } + if entry.Name == "" || slices.Contains(scopes, entry.Name) || !slices.Contains(scopes, entry.Implies) { + continue + } + scopes = append(scopes, entry.Name) + } + return scopes +} + // MCPResolvePersonalToken resolves a personal API token of this plugin into // an MCP subject with the token's own scopes. A token without the configured // prefix answers false; one with it that is unknown, revoked, expired or diff --git a/go.mod b/go.mod index b6a57f6..7dda658 100644 --- a/go.mod +++ b/go.mod @@ -180,4 +180,4 @@ require ( gorm.io/driver/postgres v1.6.3 // indirect ) -replace git.golem15.com/golem15/summercms => ../../../../summercms.go +replace git.golem15.com/golem15/summercms => ../summercms.go diff --git a/mcp_implies_test.go b/mcp_implies_test.go new file mode 100644 index 0000000..9f92d18 --- /dev/null +++ b/mcp_implies_test.go @@ -0,0 +1,213 @@ +package user + +import ( + "context" + "database/sql" + "database/sql/driver" + "io" + "slices" + "strings" + "testing" + "time" + + "git.golem15.com/golem15/sm-user-plugin/classes" + "git.golem15.com/golem15/sm-user-plugin/models" + "git.golem15.com/golem15/summercms/modules/backpack" + "git.golem15.com/golem15/summercms/modules/pact" + pgdriver "gorm.io/driver/postgres" + "gorm.io/gorm" +) + +// impliesServersRead is the one declared scope the tracer exercises: an +// OAuth grant of read satisfies it, a grant of write does not. +func impliesServersRead() []pact.FrontendTokenScope { + return []pact.FrontendTokenScope{{ + Name: "servers:read", Label: "Read servers", Description: "List servers.", Implies: "read", + }} +} + +// impliesApp boots the member-token catalog with scopes declared and +// publishes a database handle the guard can load a principal from. The +// handle answers one user and no group grants, in memory, so the scope +// check can reach a credential without Postgres. Nothing is written. +func impliesApp(t *testing.T, scopes []pact.FrontendTokenScope) (*backpack.App, *classes.MCPUserGuard) { + t.Helper() + t.Cleanup(classes.ResetDeclaredAPITokenScopes) + app := scopeBootApp(t, scopeDeclarer{"acme.scopes", scopes}) + if err := (&Plugin{}).bootAPITokenScopes(app); err != nil { + t.Fatal(err) + } + sqlDB, err := sql.Open("user_implies", "") + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = sqlDB.Close() }) + db, err := gorm.Open(pgdriver.New(pgdriver.Config{Conn: sqlDB}), &gorm.Config{DisableAutomaticPing: true}) + if err != nil { + t.Fatal(err) + } + if err := app.Publish(db); err != nil { + t.Fatal(err) + } + return app, &classes.MCPUserGuard{App: app} +} + +// TestMCPImpliesOAuth pins the OAuth half of D-01a: a grant of read satisfies +// a declared scope whose Implies is read, in MCPAllows and on the credential +// MCPDispatch builds, and that credential keeps ID 0. The grant slice itself +// is not rewritten, and the catalog is published as pact.MemberTokenScopeCatalog. +func TestMCPImpliesOAuth(t *testing.T) { + app, guard := impliesApp(t, impliesServersRead()) + catalog, ok := app.Lookup[pact.MemberTokenScopeCatalog]() + if !ok || catalog == nil { + t.Fatal("pact.MemberTokenScopeCatalog is not published") + } + if _, ok := app.Lookup[*classes.APITokenScopeCatalog](); !ok { + t.Fatal("the concrete catalog publish was removed") + } + var sawRead, sawDeclared bool + for _, scope := range catalog.MemberTokenScopes() { + switch scope.Name { + case "read", "write", "ai": + if scope.Implies != "" { + t.Fatalf("built-in %s Implies = %q", scope.Name, scope.Implies) + } + if scope.Name == "read" { + sawRead = true + } + case "servers:read": + sawDeclared = scope.Implies == "read" + } + } + if !sawRead || !sawDeclared { + t.Fatalf("catalog = %+v", catalog.MemberTokenScopes()) + } + + ctx := context.Background() + subject := pact.MCPSubject{Guard: "user", UserID: 7, ClientID: "oauth-client", Scopes: []string{"read"}} + orig := subject.Scopes + ok, err := guard.MCPAllows(ctx, subject, pact.MCPRequirements{Scopes: []string{"servers:read"}}, nil) + if err != nil || !ok { + t.Fatalf("MCPAllows(servers:read) = %v, %v", ok, err) + } + dispatch, err := guard.MCPDispatch(ctx, subject, "GET") + if err != nil { + t.Fatal(err) + } + credential, isToken := dispatch.Credential.(*models.APIToken) + if !isToken || credential.ID != 0 || credential.OAuthClientID != nil || !credential.HasScope("servers:read") || !credential.HasScope("read") { + t.Fatalf("credential = %+v", dispatch.Credential) + } + if len(subject.Scopes) != 1 || &subject.Scopes[0] != &orig[0] || subject.Scopes[0] != "read" { + t.Fatalf("subject.Scopes = %v, want the original grant slice", subject.Scopes) + } + if slices.Contains(subject.Scopes, "servers:read") { + t.Fatal("implied name was written onto subject.Scopes") + } +} + +func init() { + sql.Register("user_implies", impliesDriver{}) +} + +// impliesDriver is an in-memory database/sql driver for the Implies tests. +// A users query returns one row. A group-grant query returns none. +type impliesDriver struct{} + +func (impliesDriver) Open(string) (driver.Conn, error) { return impliesConn{}, nil } + +type impliesConn struct{} + +func (impliesConn) Prepare(string) (driver.Stmt, error) { + return nil, driver.ErrSkip +} +func (impliesConn) Close() error { return nil } +func (impliesConn) Begin() (driver.Tx, error) { return nil, driver.ErrSkip } + +func (impliesConn) QueryContext(_ context.Context, query string, _ []driver.NamedValue) (driver.Rows, error) { + return impliesRowsFor(query), nil +} + +func (impliesConn) ExecContext(context.Context, string, []driver.NamedValue) (driver.Result, error) { + return impliesResult(0), nil +} + +func impliesRowsFor(query string) driver.Rows { + upper := strings.ToUpper(query) + if strings.Contains(upper, "FROM \"USERS\"") || strings.Contains(upper, "FROM USERS") { + cols := impliesSelectCols(query) + vals := make([]driver.Value, len(cols)) + for i, col := range cols { + vals[i] = impliesColValue(col) + } + return &impliesRows{cols: cols, vals: [][]driver.Value{vals}} + } + return &impliesRows{cols: []string{"permissions"}} +} + +func impliesSelectCols(query string) []string { + upper := strings.ToUpper(query) + start := strings.Index(upper, "SELECT") + from := strings.Index(upper, " FROM ") + if start < 0 || from < 0 || from <= start+6 { + return []string{"id"} + } + raw := strings.Split(query[start+6:from], ",") + cols := make([]string, 0, len(raw)) + for _, part := range raw { + name := strings.TrimSpace(part) + name = strings.Trim(name, `"`) + if i := strings.LastIndex(name, "."); i >= 0 { + name = name[i+1:] + } + name = strings.Trim(name, `"`) + name = strings.TrimSpace(name) + if name == "" || name == "*" { + continue + } + cols = append(cols, name) + } + if len(cols) == 0 { + return []string{"id"} + } + return cols +} + +func impliesColValue(col string) driver.Value { + switch col { + case "id": + return int64(7) + case "email": + return "ada@example.test" + case "password": + return "x" + case "must_change_password", "is_activated", "has_self_set_password", "marketing_consent", "is_onboarded": + return false + case "created_at", "updated_at": + return time.Time{} + default: + return nil + } +} + +type impliesRows struct { + cols []string + vals [][]driver.Value + i int +} + +func (r *impliesRows) Columns() []string { return r.cols } +func (r *impliesRows) Close() error { return nil } +func (r *impliesRows) Next(dest []driver.Value) error { + if r.i >= len(r.vals) { + return io.EOF + } + copy(dest, r.vals[r.i]) + r.i++ + return nil +} + +type impliesResult int64 + +func (r impliesResult) LastInsertId() (int64, error) { return int64(r), nil } +func (r impliesResult) RowsAffected() (int64, error) { return int64(r), nil } diff --git a/plugin.go b/plugin.go index 27af92c..4d22c51 100644 --- a/plugin.go +++ b/plugin.go @@ -153,7 +153,8 @@ func (p *Plugin) bootDeviceRevocation(app *backpack.App) { // bootAPITokenScopes compiles the member token scope catalog from the // activated plugins (pact.HasFrontendTokenScopes), checks that every phrase -// key it names exists, and publishes it. A bad declaration fails boot. +// key it names exists, and publishes it as *classes.APITokenScopeCatalog and +// as pact.MemberTokenScopeCatalog. A bad declaration fails boot. func (p *Plugin) bootAPITokenScopes(app *backpack.App) error { activated, _ := app.Lookup[party.Activated]() catalog, err := classes.CompileAPITokenScopes(activated) @@ -173,7 +174,10 @@ func (p *Plugin) bootAPITokenScopes(app *backpack.App) error { } } } - return app.Publish(catalog) + if err := app.Publish(catalog); err != nil { + return err + } + return app.Publish[pact.MemberTokenScopeCatalog](catalog) } // bootMCPGuard registers the "user" MCP guard provider in the application's