From 75dfdb3dcd8263471a2876867bea5d6b676b33c3 Mon Sep 17 00:00:00 2001 From: Jeffrey Smith Date: Fri, 3 Apr 2026 19:52:47 +0000 Subject: [PATCH] Feat v0.9.9 surface access via roles Co-Authored-By: Claude Opus 4.6 (1M context) --- CHANGELOG.md | 30 +++++ ROADMAP.md | 10 +- server/handlers/package_validate.go | 2 + server/handlers/team_roles_test.go | 87 +++++++++++++ server/pages/pages.go | 18 ++- server/pages/pages_handler_test.go | 193 ++++++++++++++++++++++++++++ server/store/interfaces.go | 3 + server/store/postgres/team.go | 11 ++ server/store/sqlite/team.go | 11 ++ 9 files changed, 359 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ebae952..4bad08b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,36 @@ All notable changes to Armature are documented here. +## v0.9.9 — Surface Access via Roles + +Surfaces can now gate access by team role. A manifest declaring +`"access": "role:approver"` restricts the surface to users who hold the +`approver` role in any team — consistent with how `group:NAME` works. + +**New surface access level: `role:ROLENAME`** + +- Added to both `evaluateAccess()` and `validAccessLevels()` validation +- User must hold the specified role in at least one team (any-team + semantics — no team context needed in the URL) +- System admins bypass role checks, consistent with `RequireRole` + middleware +- Unauthenticated users get redirected to login +- Graceful fallback: if Teams store is nil, access is denied (fail closed) + +**New store method: `HasRoleInAnyTeam(ctx, userID, role)`** + +- Checks both primary role (`team_members`) and additional roles + (`team_user_roles`) across all teams in a single query +- Implemented for both SQLite and PostgreSQL + +**Refactor:** `evaluateAccess` promoted from package-level function to +`Engine` method to enable store access for role lookups. + +**Tests:** 10 new tests — 5 handler integration tests (granted, denied, +unauthenticated, admin bypass, nil store), 3 store tests +(primary role, additional role, no match), 2 validation tests +(valid role, empty role name) + ## v0.9.8 — Conditional Routing → SDK Primitive Promotes the workflow branch-rule engine to a generic Starlark SDK diff --git a/ROADMAP.md b/ROADMAP.md index ae97d3c..e4326eb 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -145,11 +145,13 @@ neq, gt, lt, gte, lte, in, contains), first-match-wins, returns target string or None. Always available (pure computation, no permission). 8 new tests. -**v0.9.9 — Surface Access via Roles** +**v0.9.9 — Surface Access via Roles** *(completed)* -Wire team roles (v0.9.3) into surface access declarations: -`access: role:approver`. Kernel middleware checks role membership. -Completes the workflow→package access story. +`role:ROLENAME` surface access level. User must hold the role in any +team (any-team semantics, no URL context needed). `evaluateAccess` +promoted to Engine method for store access. `HasRoleInAnyTeam` store +method queries both primary and additional roles. Admin bypass, fail- +closed on nil store. 10 new tests. --- diff --git a/server/handlers/package_validate.go b/server/handlers/package_validate.go index 30559b8..1007df3 100644 --- a/server/handlers/package_validate.go +++ b/server/handlers/package_validate.go @@ -259,6 +259,8 @@ func validAccessLevels(access string) bool { return true case strings.HasPrefix(access, "group:"): return strings.TrimPrefix(access, "group:") != "" + case strings.HasPrefix(access, "role:"): + return strings.TrimPrefix(access, "role:") != "" default: return false } diff --git a/server/handlers/team_roles_test.go b/server/handlers/team_roles_test.go index 666cfad..798ec8f 100644 --- a/server/handlers/team_roles_test.go +++ b/server/handlers/team_roles_test.go @@ -262,6 +262,93 @@ func TestRequireRole_Denied(t *testing.T) { } } +// ── Store: HasRoleInAnyTeam ────────────────── + +func TestHasRoleInAnyTeam_PrimaryRole(t *testing.T) { + database.RequireTestDB(t) + stores := testStores(t) + ctx := context.Background() + + _, userID, _ := seedTeamAndMember(t, stores) + + // "member" is the primary role assigned during seedTeamAndMember + has, err := stores.Teams.HasRoleInAnyTeam(ctx, userID, "member") + if err != nil { + t.Fatalf("HasRoleInAnyTeam: %v", err) + } + if !has { + t.Error("expected true for primary role 'member'") + } +} + +func TestHasRoleInAnyTeam_AdditionalRole(t *testing.T) { + database.RequireTestDB(t) + stores := testStores(t) + ctx := context.Background() + + teamID, userID, _ := seedTeamAndMember(t, stores) + stores.Teams.AddUserRole(ctx, teamID, userID, "reviewer", userID) + + has, err := stores.Teams.HasRoleInAnyTeam(ctx, userID, "reviewer") + if err != nil { + t.Fatalf("HasRoleInAnyTeam: %v", err) + } + if !has { + t.Error("expected true for additional role 'reviewer'") + } +} + +func TestHasRoleInAnyTeam_NoMatch(t *testing.T) { + database.RequireTestDB(t) + stores := testStores(t) + ctx := context.Background() + + _, userID, _ := seedTeamAndMember(t, stores) + + has, err := stores.Teams.HasRoleInAnyTeam(ctx, userID, "nonexistent") + if err != nil { + t.Fatalf("HasRoleInAnyTeam: %v", err) + } + if has { + t.Error("expected false for non-existent role") + } +} + +// ── Manifest: role access validation ──────── + +func TestValidateManifest_SurfaceRoleAccess(t *testing.T) { + m := map[string]any{ + "id": "role-pkg", + "title": "Role Gated", + "type": "surface", + "surfaces": []any{ + map[string]any{"path": "/", "access": "role:approver"}, + }, + } + info, err := ValidateManifest(m) + if err != nil { + t.Fatalf("unexpected error for role:approver access: %v", err) + } + if !info.HasSurfaces { + t.Error("expected HasSurfaces to be true") + } +} + +func TestValidateManifest_SurfaceRoleAccessEmpty(t *testing.T) { + m := map[string]any{ + "id": "role-pkg", + "title": "Role Gated", + "type": "surface", + "surfaces": []any{ + map[string]any{"path": "/", "access": "role:"}, + }, + } + _, err := ValidateManifest(m) + if err == nil { + t.Fatal("expected error for empty role name 'role:'") + } +} + // ── helpers ────────────────────────────────── func seedRoleUser(t *testing.T, username, email string) string { diff --git a/server/pages/pages.go b/server/pages/pages.go index 6504983..1a29b15 100644 --- a/server/pages/pages.go +++ b/server/pages/pages.go @@ -522,7 +522,7 @@ func (e *Engine) RenderExtensionSurface() gin.HandlerFunc { } // ── Access check ───────────────────────────────────────── - if !evaluateAccess(c, surfaceAccess) { + if !e.evaluateAccess(c, surfaceAccess) { // Redirect unauthenticated users to login; deny others with 403 if c.GetString("user_id") == "" { c.Redirect(http.StatusTemporaryRedirect, e.cfg.BasePath+"/login") @@ -651,7 +651,7 @@ func aggregateAccess(surfaces []any) string { } // evaluateAccess checks whether the current request meets an access requirement. -func evaluateAccess(c *gin.Context, access string) bool { +func (e *Engine) evaluateAccess(c *gin.Context, access string) bool { switch { case access == "public": return true @@ -672,6 +672,20 @@ func evaluateAccess(c *gin.Context, access string) bool { } } return false + case strings.HasPrefix(access, "role:"): + role := strings.TrimPrefix(access, "role:") + userID := c.GetString("user_id") + if userID == "" { + return false + } + if c.GetBool("is_admin") { + return true + } + if e.stores.Teams == nil { + return false + } + has, err := e.stores.Teams.HasRoleInAnyTeam(c.Request.Context(), userID, role) + return err == nil && has default: return false } diff --git a/server/pages/pages_handler_test.go b/server/pages/pages_handler_test.go index 10720dc..fd903c3 100644 --- a/server/pages/pages_handler_test.go +++ b/server/pages/pages_handler_test.go @@ -11,9 +11,69 @@ import ( "github.com/gin-gonic/gin" "armature/config" + "armature/models" "armature/store" ) +// ── mock TeamStore ─────────────────────────────────────────── + +// mockTeamStore implements store.TeamStore with a simple role lookup map. +// Only HasRoleInAnyTeam is functional; everything else is a no-op stub. +type mockTeamStore struct { + // userRoles maps userID → set of roles they hold in any team + userRoles map[string]map[string]bool +} + +func newMockTeamStore() *mockTeamStore { + return &mockTeamStore{userRoles: make(map[string]map[string]bool)} +} + +func (m *mockTeamStore) addRole(userID, role string) { + if m.userRoles[userID] == nil { + m.userRoles[userID] = make(map[string]bool) + } + m.userRoles[userID][role] = true +} + +func (m *mockTeamStore) HasRoleInAnyTeam(_ context.Context, userID, role string) (bool, error) { + if roles, ok := m.userRoles[userID]; ok { + return roles[role], nil + } + return false, nil +} + +// Stubs — not exercised by surface access tests. +func (m *mockTeamStore) Create(context.Context, *models.Team) error { return nil } +func (m *mockTeamStore) GetByID(context.Context, string) (*models.Team, error) { return nil, nil } +func (m *mockTeamStore) Update(context.Context, string, map[string]interface{}) error { return nil } +func (m *mockTeamStore) Delete(context.Context, string) error { return nil } +func (m *mockTeamStore) List(context.Context) ([]models.Team, error) { return nil, nil } +func (m *mockTeamStore) ListForUser(context.Context, string) ([]models.Team, error) { return nil, nil } +func (m *mockTeamStore) AddMember(context.Context, string, string, string) error { return nil } +func (m *mockTeamStore) RemoveMember(context.Context, string, string) error { return nil } +func (m *mockTeamStore) UpdateMemberRole(context.Context, string, string, string) error { return nil } +func (m *mockTeamStore) ListMembers(context.Context, string) ([]models.TeamMember, error) { return nil, nil } +func (m *mockTeamStore) GetMember(context.Context, string, string) (*models.TeamMember, error) { return nil, nil } +func (m *mockTeamStore) GetUserTeamIDs(context.Context, string) ([]string, error) { return nil, nil } +func (m *mockTeamStore) IsTeamAdmin(context.Context, string, string) (bool, error) { return false, nil } +func (m *mockTeamStore) IsMember(context.Context, string, string) (bool, error) { return false, nil } +func (m *mockTeamStore) Exists(context.Context, string) (bool, error) { return false, nil } +func (m *mockTeamStore) UpdateMemberRoleByID(context.Context, string, string, string) (int64, error) { return 0, nil } +func (m *mockTeamStore) DeleteMemberByID(context.Context, string, string) (int64, error) { return 0, nil } +func (m *mockTeamStore) ListTeamAuditActions(context.Context, string) ([]string, error) { return nil, nil } +func (m *mockTeamStore) GetFirstTeamIDForUser(context.Context, string) (string, error) { return "", nil } +func (m *mockTeamStore) AddMemberReturningID(context.Context, string, string, string) (string, error) { return "", nil } +func (m *mockTeamStore) MergeSettings(context.Context, string, string) error { return nil } +func (m *mockTeamStore) AddUserRole(context.Context, string, string, string, string) error { return nil } +func (m *mockTeamStore) RemoveUserRole(context.Context, string, string, string) error { return nil } +func (m *mockTeamStore) ListUserRoles(context.Context, string, string) ([]string, error) { return nil, nil } +func (m *mockTeamStore) GetMemberRoles(context.Context, string, string) ([]string, error) { return nil, nil } +func (m *mockTeamStore) HasRole(context.Context, string, string, string) (bool, error) { return false, nil } +func (m *mockTeamStore) RemoveAllUserRoles(context.Context, string, string) error { return nil } +func (m *mockTeamStore) AddRoleToCatalog(context.Context, string, string, string) error { return nil } +func (m *mockTeamStore) ListRoleCatalog(context.Context, string) ([]store.TeamRoleCatalogEntry, error) { return nil, nil } +func (m *mockTeamStore) RemoveRoleCatalogBySource(context.Context, string, string) error { return nil } + // ── mock PackageStore ──────────────────────────────────────── // mockPackageStore implements store.PackageStore with in-memory data. @@ -293,3 +353,136 @@ func TestHandler_EarlyAuthShortCircuit(t *testing.T) { t.Errorf("expected redirect to /login, got %s", loc) } } + +// ── v0.9.9 — role-based surface access ────────────────────── + +// testRouterWithTeams builds a router whose Engine has a mock TeamStore. +// The optAuth middleware also checks X-Admin header to set is_admin. +func testRouterWithTeams(t *testing.T, teams *mockTeamStore, pkgs ...*store.PackageRegistration) *gin.Engine { + t.Helper() + gin.SetMode(gin.TestMode) + engine := &Engine{ + cfg: &config.Config{BasePath: ""}, + stores: store.Stores{Packages: newMockPackageStore(pkgs...), Teams: teams}, + loaders: make(map[string]DataLoaderFunc), + } + engine.parseTemplates() + engine.registerCoreSurfaces() + + r := gin.New() + optAuth := func(c *gin.Context) { + if uid := c.GetHeader("X-User-ID"); uid != "" { + c.Set("user_id", uid) + c.Set("role", "user") + } + if c.GetHeader("X-Admin") == "true" { + c.Set("is_admin", true) + } + c.Next() + } + apiAuth := func(c *gin.Context) { c.Next() } + apiHandler := func(c *gin.Context) { c.JSON(200, gin.H{"ok": true}) } + engine.RegisterExtensionRoutes(r.Group(""), optAuth, apiAuth, apiHandler) + return r +} + +func TestHandler_RoleAccess_Granted(t *testing.T) { + teams := newMockTeamStore() + teams.addRole("user-1", "approver") + + pkg := &store.PackageRegistration{ + ID: "role-pkg", Title: "Role Pkg", Type: "surface", Source: "extension", + Enabled: true, Status: "active", + Manifest: map[string]any{ + "surfaces": []any{ + map[string]any{"path": "/", "access": "role:approver", "title": "Approvals"}, + }, + }, + } + r := testRouterWithTeams(t, teams, pkg) + w := doGet(r, "/s/role-pkg", "X-User-ID", "user-1") + if w.Code != http.StatusOK { + t.Errorf("expected 200 for user with role, got %d", w.Code) + } +} + +func TestHandler_RoleAccess_Denied(t *testing.T) { + teams := newMockTeamStore() + // user-2 has no roles + + pkg := &store.PackageRegistration{ + ID: "role-pkg", Title: "Role Pkg", Type: "surface", Source: "extension", + Enabled: true, Status: "active", + Manifest: map[string]any{ + "surfaces": []any{ + map[string]any{"path": "/", "access": "role:approver", "title": "Approvals"}, + }, + }, + } + r := testRouterWithTeams(t, teams, pkg) + w := doGet(r, "/s/role-pkg", "X-User-ID", "user-2") + if w.Code != http.StatusForbidden { + t.Errorf("expected 403 for user without role, got %d", w.Code) + } +} + +func TestHandler_RoleAccess_Unauthenticated(t *testing.T) { + teams := newMockTeamStore() + + pkg := &store.PackageRegistration{ + ID: "role-pkg", Title: "Role Pkg", Type: "surface", Source: "extension", + Enabled: true, Status: "active", + Manifest: map[string]any{ + "surfaces": []any{ + map[string]any{"path": "/", "access": "role:approver", "title": "Approvals"}, + }, + }, + } + r := testRouterWithTeams(t, teams, pkg) + w := doGet(r, "/s/role-pkg") // no auth + if w.Code != http.StatusTemporaryRedirect { + t.Errorf("expected 307 redirect, got %d", w.Code) + } + loc := w.Header().Get("Location") + if !strings.Contains(loc, "/login") { + t.Errorf("expected redirect to /login, got %s", loc) + } +} + +func TestHandler_RoleAccess_AdminBypass(t *testing.T) { + teams := newMockTeamStore() + // admin-user has no "approver" role but is_admin=true + + pkg := &store.PackageRegistration{ + ID: "role-pkg", Title: "Role Pkg", Type: "surface", Source: "extension", + Enabled: true, Status: "active", + Manifest: map[string]any{ + "surfaces": []any{ + map[string]any{"path": "/", "access": "role:approver", "title": "Approvals"}, + }, + }, + } + r := testRouterWithTeams(t, teams, pkg) + w := doGet(r, "/s/role-pkg", "X-User-ID", "admin-user", "X-Admin", "true") + if w.Code != http.StatusOK { + t.Errorf("expected 200 for admin bypass, got %d", w.Code) + } +} + +func TestHandler_RoleAccess_NilTeamStore(t *testing.T) { + // Engine with no team store — role access should deny gracefully + pkg := &store.PackageRegistration{ + ID: "role-pkg", Title: "Role Pkg", Type: "surface", Source: "extension", + Enabled: true, Status: "active", + Manifest: map[string]any{ + "surfaces": []any{ + map[string]any{"path": "/", "access": "role:approver", "title": "Approvals"}, + }, + }, + } + r := testRouter(t, pkg) // testRouter uses nil Teams store + w := doGet(r, "/s/role-pkg", "X-User-ID", "user-1") + if w.Code != http.StatusForbidden { + t.Errorf("expected 403 when Teams store is nil, got %d", w.Code) + } +} diff --git a/server/store/interfaces.go b/server/store/interfaces.go index 675f578..97ed0a7 100644 --- a/server/store/interfaces.go +++ b/server/store/interfaces.go @@ -206,6 +206,9 @@ type TeamStore interface { // HasRole checks whether a user holds a specific role (primary or additional). HasRole(ctx context.Context, teamID, userID, role string) (bool, error) + // HasRoleInAnyTeam checks whether a user holds a specific role in any team. + HasRoleInAnyTeam(ctx context.Context, userID, role string) (bool, error) + // RemoveAllUserRoles deletes all additional roles for a member (cleanup on removal). RemoveAllUserRoles(ctx context.Context, teamID, userID string) error diff --git a/server/store/postgres/team.go b/server/store/postgres/team.go index 67ae8ac..334c456 100644 --- a/server/store/postgres/team.go +++ b/server/store/postgres/team.go @@ -388,6 +388,17 @@ func (s *TeamStore) HasRole(ctx context.Context, teamID, userID, role string) (b return exists, err } +func (s *TeamStore) HasRoleInAnyTeam(ctx context.Context, userID, role string) (bool, error) { + var exists bool + err := DB.QueryRowContext(ctx, ` + SELECT EXISTS( + SELECT 1 FROM team_members WHERE user_id = $1 AND role = $2 + UNION ALL + SELECT 1 FROM team_user_roles WHERE user_id = $1 AND role = $2 + )`, userID, role).Scan(&exists) + return exists, err +} + func (s *TeamStore) RemoveAllUserRoles(ctx context.Context, teamID, userID string) error { _, err := DB.ExecContext(ctx, `DELETE FROM team_user_roles WHERE team_id = $1 AND user_id = $2`, diff --git a/server/store/sqlite/team.go b/server/store/sqlite/team.go index 3a807e2..2fb2d3a 100644 --- a/server/store/sqlite/team.go +++ b/server/store/sqlite/team.go @@ -395,6 +395,17 @@ func (s *TeamStore) HasRole(ctx context.Context, teamID, userID, role string) (b return count > 0, err } +func (s *TeamStore) HasRoleInAnyTeam(ctx context.Context, userID, role string) (bool, error) { + var count int + err := DB.QueryRowContext(ctx, ` + SELECT COUNT(*) FROM ( + SELECT 1 FROM team_members WHERE user_id = ? AND role = ? + UNION ALL + SELECT 1 FROM team_user_roles WHERE user_id = ? AND role = ? + ) LIMIT 1`, userID, role, userID, role).Scan(&count) + return count > 0, err +} + func (s *TeamStore) RemoveAllUserRoles(ctx context.Context, teamID, userID string) error { _, err := DB.ExecContext(ctx, `DELETE FROM team_user_roles WHERE team_id = ? AND user_id = ?`,