From cf9c35aa0aa43c00b4b983fe0c22d6b0d4cc2ab4 Mon Sep 17 00:00:00 2001 From: agent-exec-transport Date: Mon, 19 Oct 2026 15:40:32 +0000 Subject: [PATCH] agent-exec transport 875f5aca7406765a0c4913ffa0fbf707/3f6d66d1ffaa --- internal/app/role_provider_test.go | 31 +++- internal/authz/catalog/catalog.go | 5 + internal/authz/catalog/catalog_test.go | 7 + internal/authz/service.go | 26 +-- internal/server/role_authorization_test.go | 186 ++++++++++++++++++++ internal/server/roles.go | 4 +- internal/server/roles_test.go | 109 +++++++++--- internal/store/role_policy.go | 73 ++++++-- internal/store/role_policy_test.go | 192 +++++++++++++++++++++ 9 files changed, 566 insertions(+), 67 deletions(-) diff --git a/internal/app/role_provider_test.go b/internal/app/role_provider_test.go index 08494f0..459039f 100644 --- a/internal/app/role_provider_test.go +++ b/internal/app/role_provider_test.go @@ -27,7 +27,7 @@ func TestRoleProviderReturnsVersionedNonSecretPolicy(t *testing.T) { t.Fatal(err) } receipt, err := st.ReplaceRolePermissions("admin", role.RoleID, role.RoleVersion, - []store.RolePermissionChange{{ActionID: "document.read", Allowed: true, CapAllowed: true}}, "provider-policy") + []store.RolePermissionChange{{ActionID: "project.create", Allowed: true, CapAllowed: true}}, "provider-policy") if err != nil { t.Fatal(err) } @@ -52,14 +52,18 @@ func TestRoleProviderReturnsVersionedNonSecretPolicy(t *testing.T) { !snapshot.RoleEnabled || snapshot.RoleVersion != receipt.RoleVersion || snapshot.AuthorityEpoch < receipt.AuthorityEpoch { t.Fatalf("snapshot=%+v", snapshot) } - permission := snapshot.Permissions["document.read"] - if !permission.Allowed || !permission.CapAllowed { - t.Fatalf("permission=%+v", permission) + permission := snapshot.Permissions["project.create"] + if len(snapshot.Permissions) != 1 || !permission.Allowed || !permission.CapAllowed { + t.Fatalf("permissions=%+v", snapshot.Permissions) } - decision, err := provider.CheckRoleAction(context.Background(), alice.ID, "document.read") + decision, err := provider.CheckRoleAction(context.Background(), alice.ID, "project.create") if err != nil || !decision.Allowed || !decision.CapAllowed || decision.RoleID != role.RoleID { t.Fatalf("decision=%+v err=%v", decision, err) } + legacy, err := provider.CheckRoleAction(context.Background(), alice.ID, "document.read") + if err != nil || legacy.Allowed || legacy.Reason != "action-unconfigured" { + t.Fatalf("non-project role action=%+v err=%v", legacy, err) + } systemProvider, ok := any(h).(extension.SystemActionProvider) if !ok { t.Fatal("dispatch host missing SystemActionProvider") @@ -69,7 +73,7 @@ func TestRoleProviderReturnsVersionedNonSecretPolicy(t *testing.T) { t.Fatalf("system decision=%+v err=%v", systemDecision, err) } updated, err := st.ReplaceRolePermissions("admin", role.RoleID, snapshot.RoleVersion, - []store.RolePermissionChange{{ActionID: "document.read", Allowed: false, CapAllowed: true}}, "provider-policy-update") + []store.RolePermissionChange{{ActionID: "project.create", Allowed: false, CapAllowed: true}}, "provider-policy-update") if err != nil { t.Fatal(err) } @@ -77,6 +81,13 @@ func TestRoleProviderReturnsVersionedNonSecretPolicy(t *testing.T) { if err != nil || refreshed.AuthorityEpoch != updated.AuthorityEpoch || refreshed.RoleVersion != updated.RoleVersion { t.Fatalf("refreshed=%+v receipt=%+v err=%v", refreshed, updated, err) } + if denied := refreshed.Permissions["project.create"]; len(refreshed.Permissions) != 1 || denied.Allowed || denied.CapAllowed { + t.Fatalf("refreshed permissions=%+v", refreshed.Permissions) + } + revoked, err := provider.CheckRoleAction(context.Background(), alice.ID, "project.create") + if err != nil || revoked.Allowed || revoked.CapAllowed { + t.Fatalf("revoked decision=%+v err=%v", revoked, err) + } } func TestRoleProviderRejectsUntrustedScopedHost(t *testing.T) { @@ -117,8 +128,12 @@ func TestRolePolicyAdminUsesBoundActorAndStableOwnerCompatibility(t *testing.T) if err != nil { t.Fatal(err) } - role, err = admin.SetRolePermission(context.Background(), role.RoleID, "document.read", true, true, "legacy-action") - if err != nil || !role.Permissions["document.read"].Allowed { + if _, err := admin.SetRolePermission(context.Background(), role.RoleID, "document.read", true, true, "legacy-action"); !errors.Is(err, store.ErrInvalidRolePolicy) { + t.Fatalf("non-project role action accepted: %v", err) + } + role, err = admin.SetRolePermission(context.Background(), role.RoleID, "project.create", true, true, "project-access") + access := role.Permissions["project.create"] + if err != nil || len(role.Permissions) != 1 || !access.Allowed || !access.CapAllowed { t.Fatalf("role=%+v err=%v", role, err) } binding, err := admin.AssignRoleByOwner(context.Background(), "alice", role.RoleID, "legacy-assign") diff --git a/internal/authz/catalog/catalog.go b/internal/authz/catalog/catalog.go index 29c7271..6dba7ed 100644 --- a/internal/authz/catalog/catalog.go +++ b/internal/authz/catalog/catalog.go @@ -9,6 +9,11 @@ const ( ScopeProject Scope = "project" ) +// ProjectAccessAction is the only action a position role carries (R2). Every +// other catalog action comes from system grants or project membership; role +// rows naming them are retained for audit but never authorize anything. +const ProjectAccessAction = "project.create" + type Descriptor struct { ActionID string `json:"actionId"` Label string `json:"label"` diff --git a/internal/authz/catalog/catalog_test.go b/internal/authz/catalog/catalog_test.go index 66565d6..64a10b2 100644 --- a/internal/authz/catalog/catalog_test.go +++ b/internal/authz/catalog/catalog_test.go @@ -16,6 +16,13 @@ func TestCatalogRejectsUnknownAndProtectsManagementActions(t *testing.T) { } } +func TestCatalogProjectAccessIsTheOnlyRoleAction(t *testing.T) { + access, ok := DescriptorByID(ProjectAccessAction) + if !ok || access.ActionID != "project.create" || access.Scope != ScopeSystem || access.Protected { + t.Fatalf("project access descriptor=%+v ok=%v", access, ok) + } +} + func TestCatalogReturnsIndependentCopies(t *testing.T) { first := Catalog() if len(first) == 0 { diff --git a/internal/authz/service.go b/internal/authz/service.go index 71f053a..76e162d 100644 --- a/internal/authz/service.go +++ b/internal/authz/service.go @@ -47,6 +47,21 @@ func (s *Service) AuthorizeSystem(actor, actionID string) Decision { decision.Reason = "account-unavailable" return decision } + // Project access comes only from the actor's role (R2): neither is_admin + // nor a system grant confers it. + if actionID == catalog.ProjectAccessAction { + roleAllowed, _, err := s.store.RolePermissionAllowed(actor, actionID) + if err != nil { + decision.Reason = "authority-unavailable" + return decision + } + if roleAllowed { + decision.Allowed, decision.Source = true, "role" + return decision + } + decision.Reason = "not-granted" + return decision + } // is_admin remains the narrow bootstrap/root recovery source. Ordinary // delegated management uses explicit system_grants. if account.IsAdmin { @@ -62,17 +77,6 @@ func (s *Service) AuthorizeSystem(actor, actionID string) Decision { decision.Allowed, decision.Source = true, "system-grant" return decision } - if actionID == "project.create" { - roleAllowed, capAllowed, err := s.store.RolePermissionAllowed(actor, actionID) - if err != nil { - decision.Reason = "authority-unavailable" - return decision - } - if roleAllowed && capAllowed { - decision.Allowed, decision.Source = true, "role" - return decision - } - } decision.Reason = "not-granted" return decision } diff --git a/internal/server/role_authorization_test.go b/internal/server/role_authorization_test.go index e855a5b..de097f3 100644 --- a/internal/server/role_authorization_test.go +++ b/internal/server/role_authorization_test.go @@ -7,6 +7,8 @@ import ( "strconv" "strings" "testing" + + "agent-system-backend/internal/store" ) func bearerRequest(h http.Handler, method, path, body, key string) *httptest.ResponseRecorder { @@ -150,3 +152,187 @@ func TestDelegatedUserActionsReachTransactionalStoreChecks(t *testing.T) { t.Fatalf("delegated disable: %d %s", disabled.Code, disabled.Body.String()) } } + +// grantSystemActionsFixture creates owner with exactly the given explicit +// system grants (issued by the super administrator) and returns its API key. +func grantSystemActionsFixture(t *testing.T, h http.Handler, st *store.Store, superAdmin, owner string, actions ...string) string { + t.Helper() + if err := st.CreateAccount(owner, owner, "", false); err != nil { + t.Fatal(err) + } + account, err := st.GetAccountByOwner(owner) + if err != nil { + t.Fatal(err) + } + epoch, err := st.AuthorityEpoch() + if err != nil { + t.Fatal(err) + } + body, _ := json.Marshal(map[string]any{"expectedAuthorityEpoch": epoch, "idempotencyKey": "grant-" + owner, "actions": actions}) + if rec := do(h, "PUT", "/api/admin/users/"+account.ID+"/system-grants", string(body), superAdmin); rec.Code != http.StatusOK { + t.Fatalf("grant %s: %d %s", owner, rec.Code, rec.Body.String()) + } + key, err := st.CreateAPIKey(owner, owner+"-test") + if err != nil { + t.Fatal(err) + } + return key +} + +func createAssignedRoleFixture(t *testing.T, h http.Handler, st *store.Store, superAdmin, owner, roleName string, projectAccess bool) string { + t.Helper() + roleRec := do(h, "POST", "/api/admin/roles", `{"name":"`+roleName+`"}`, superAdmin) + if roleRec.Code != http.StatusOK { + t.Fatalf("create role: %d %s", roleRec.Code, roleRec.Body.String()) + } + role := decodeRoleHTTP(t, roleRec) + if projectAccess { + policy := do(h, "PUT", "/api/admin/roles/"+role.RoleID+"/permissions", + `{"expectedRoleVersion":1,"idempotencyKey":"access-`+role.RoleID+`","changes":[{"actionId":"project.create","allowed":true,"capAllowed":true}]}`, superAdmin) + if policy.Code != http.StatusOK { + t.Fatalf("set project access: %d %s", policy.Code, policy.Body.String()) + } + } + account, err := st.GetAccountByOwner(owner) + if err != nil { + t.Fatal(err) + } + assign := do(h, "PUT", "/api/admin/users/"+account.ID+"/role", + `{"roleId":"`+role.RoleID+`","expectedStateVersion":`+strconv.FormatInt(account.StateVersion, 10)+`,"idempotencyKey":"assign-`+owner+`"}`, superAdmin) + if assign.Code != http.StatusOK { + t.Fatalf("assign %s: %d %s", owner, assign.Code, assign.Body.String()) + } + return role.RoleID +} + +func systemDecisionFixture(t *testing.T, rec *httptest.ResponseRecorder) (bool, string, string) { + t.Helper() + if rec.Code != http.StatusOK { + t.Fatalf("explain: %d %s", rec.Code, rec.Body.String()) + } + var decision struct { + Allowed bool `json:"allowed"` + Source string `json:"source"` + Reason string `json:"reason"` + } + if err := json.Unmarshal(rec.Body.Bytes(), &decision); err != nil { + t.Fatal(err) + } + return decision.Allowed, decision.Source, decision.Reason +} + +func TestProjectAccessIgnoresAdministratorBypass(t *testing.T) { + srv, st := newAuthTestServer(t) + h := srv.Handler() + superAdmin := bootSuperAdmin(t, h, st) + ops := bootOrdinaryAdmin(t, h, st, "ops") + + for name, session := range map[string]string{"super": superAdmin, "ordinary": ops} { + if rec := do(h, "POST", "/api/projects", `{"name":"管理员项目-`+name+`"}`, session); rec.Code != http.StatusForbidden { + t.Fatalf("%s admin created project without project access: %d %s", name, rec.Code, rec.Body.String()) + } + if allowed, _, reason := systemDecisionFixture(t, do(h, "GET", "/api/authz/explain?action=project.create", "", session)); allowed || reason != "not-granted" { + t.Fatalf("%s admin project.create allowed=%v reason=%s", name, allowed, reason) + } + if allowed, source, _ := systemDecisionFixture(t, do(h, "GET", "/api/authz/explain?action=system.permissions.manage", "", session)); !allowed || source != "bootstrap-admin" { + t.Fatalf("%s admin protected action allowed=%v source=%s", name, allowed, source) + } + me := do(h, "GET", "/api/me", "", session) + if me.Code != http.StatusOK || strings.Contains(me.Body.String(), `"project.create"`) || !strings.Contains(me.Body.String(), `"system.permissions.manage"`) { + t.Fatalf("%s admin system actions: %d %s", name, me.Code, me.Body.String()) + } + } + + if err := st.CreateAccount("engineer", "Engineer", "", false); err != nil { + t.Fatal(err) + } + engineerKey, err := st.CreateAPIKey("engineer", "engineer-test") + if err != nil { + t.Fatal(err) + } + createAssignedRoleFixture(t, h, st, superAdmin, "engineer", "工程师", true) + if rec := bearerRequest(h, "POST", "/api/projects", `{"name":"工程师项目"}`, engineerKey); rec.Code != http.StatusOK { + t.Fatalf("ordinary user with project access: %d %s", rec.Code, rec.Body.String()) + } + if allowed, source, _ := systemDecisionFixture(t, bearerRequest(h, "GET", "/api/authz/explain?action=project.create", "", engineerKey)); !allowed || source != "role" { + t.Fatalf("engineer project.create allowed=%v source=%s", allowed, source) + } + + createAssignedRoleFixture(t, h, st, superAdmin, "admin", "管理岗位", true) + if rec := do(h, "POST", "/api/projects", `{"name":"管理员授权项目"}`, superAdmin); rec.Code != http.StatusOK { + t.Fatalf("admin with project-access role: %d %s", rec.Code, rec.Body.String()) + } +} + +func TestProjectAccessIgnoresSystemGrantWithoutRole(t *testing.T) { + srv, st := newAuthTestServer(t) + h := srv.Handler() + superAdmin := bootSuperAdmin(t, h, st) + key := grantSystemActionsFixture(t, h, st, superAdmin, "granted", "project.create") + if rec := bearerRequest(h, "POST", "/api/projects", `{"name":"系统授权项目"}`, key); rec.Code != http.StatusForbidden { + t.Fatalf("system grant conferred project access: %d %s", rec.Code, rec.Body.String()) + } +} + +func TestLegacyRoleRowsDoNotAuthorizeSystemActionsOrAccess(t *testing.T) { + srv, st := newAuthTestServer(t) + h := srv.Handler() + superAdmin := bootSuperAdmin(t, h, st) + if err := st.CreateAccount("engineer", "Engineer", "", false); err != nil { + t.Fatal(err) + } + key, err := st.CreateAPIKey("engineer", "engineer-test") + if err != nil { + t.Fatal(err) + } + roleID := createAssignedRoleFixture(t, h, st, superAdmin, "engineer", "旧模型职位", false) + admin, err := st.GetAccountByOwner("admin") + if err != nil { + t.Fatal(err) + } + legacy := func(actionID string, allowed, capAllowed bool) { + t.Helper() + if _, err := st.DB().Exec(`INSERT INTO role_permissions(role_id,action_id,allowed,cap_allowed,permission_version,updated_by,updated_at) + VALUES(?,?,?,?,1,?,'2026-01-01T00:00:00Z') ON CONFLICT(role_id,action_id) DO UPDATE SET allowed=excluded.allowed,cap_allowed=excluded.cap_allowed`, + roleID, actionID, allowed, capAllowed, admin.ID); err != nil { + t.Fatal(err) + } + } + for _, actionID := range []string{"system.settings.read", "system.settings.write", "system.users.view", "system.users.create", "system.roles.view", "document.read"} { + legacy(actionID, true, true) + } + legacy("project.create", false, true) + + if rec := bearerRequest(h, "GET", "/api/admin/users", "", key); rec.Code != http.StatusForbidden { + t.Fatalf("legacy role row granted users.view: %d %s", rec.Code, rec.Body.String()) + } + if rec := bearerRequest(h, "GET", "/api/admin/roles", "", key); rec.Code != http.StatusForbidden { + t.Fatalf("legacy role row granted roles.view: %d %s", rec.Code, rec.Body.String()) + } + for _, actionID := range []string{"system.settings.read", "system.users.view", "project.create"} { + if allowed, _, _ := systemDecisionFixture(t, bearerRequest(h, "GET", "/api/authz/explain?action="+actionID, "", key)); allowed { + t.Fatalf("legacy role row authorized %s", actionID) + } + } + if rec := bearerRequest(h, "GET", "/api/me", "", key); rec.Code != http.StatusOK || !strings.Contains(rec.Body.String(), `"systemActions":[]`) { + t.Fatalf("legacy role system actions: %d %s", rec.Code, rec.Body.String()) + } + if rec := bearerRequest(h, "POST", "/api/projects", `{"name":"仅上限项目"}`, key); rec.Code != http.StatusForbidden { + t.Fatalf("cap-only legacy row granted project access: %d %s", rec.Code, rec.Body.String()) + } + roleRec := do(h, "GET", "/api/admin/roles/"+roleID, "", superAdmin) + requireRoleProjectAccess(t, roleRec, 1, false) + list := do(h, "GET", "/api/admin/roles", "", superAdmin) + for _, body := range []string{roleRec.Body.String(), list.Body.String()} { + for _, actionID := range []string{"system.settings", "system.users", "system.roles", "document.read"} { + if strings.Contains(body, actionID) { + t.Fatalf("role API exposed legacy %s: %s", actionID, body) + } + } + } + + legacy("project.create", true, true) + if rec := bearerRequest(h, "POST", "/api/projects", `{"name":"旧行允许项目"}`, key); rec.Code != http.StatusOK { + t.Fatalf("legacy true,true row denied project access: %d %s", rec.Code, rec.Body.String()) + } +} diff --git a/internal/server/roles.go b/internal/server/roles.go index 6387a5c..e65dddd 100644 --- a/internal/server/roles.go +++ b/internal/server/roles.go @@ -106,8 +106,10 @@ func (s *Server) patchRole(w http.ResponseWriter, r *http.Request) { writeJSONOr(w, view, err) } +// replaceRolePermissions saves a role's project-access boolean. It is part of +// role management (R13); system grants stay behind system.permissions.manage. func (s *Server) replaceRolePermissions(w http.ResponseWriter, r *http.Request) { - actor, ok := s.requireSystemAction(w, r, "system.permissions.manage") + actor, ok := s.requireSystemAction(w, r, "system.roles.manage") if !ok { return } diff --git a/internal/server/roles_test.go b/internal/server/roles_test.go index 03c029e..05dcd5c 100644 --- a/internal/server/roles_test.go +++ b/internal/server/roles_test.go @@ -89,14 +89,86 @@ func TestRoleHTTPCreateSearchPatchPermissionAndAssignment(t *testing.T) { } } +type roleProjectAccessHTTPView struct { + RoleVersion int64 `json:"roleVersion"` + ProjectAccess bool `json:"projectAccess"` + Permissions []struct { + ActionID string `json:"actionId"` + Allowed bool `json:"allowed"` + CapAllowed bool `json:"capAllowed"` + } `json:"permissions"` +} + +func decodeRoleProjectAccess(t *testing.T, rec *httptest.ResponseRecorder) roleProjectAccessHTTPView { + t.Helper() + var view roleProjectAccessHTTPView + if err := json.Unmarshal(rec.Body.Bytes(), &view); err != nil { + t.Fatalf("decode role access: %v body=%s", err, rec.Body.String()) + } + return view +} + +// requireRoleProjectAccess asserts the role endpoint exposes exactly the +// normalized project-access boolean and nothing else. +func requireRoleProjectAccess(t *testing.T, rec *httptest.ResponseRecorder, version int64, want bool) { + t.Helper() + if rec.Code != http.StatusOK { + t.Fatalf("read role: %d %s", rec.Code, rec.Body.String()) + } + view := decodeRoleProjectAccess(t, rec) + if view.RoleVersion != version || view.ProjectAccess != want || len(view.Permissions) != 1 { + t.Fatalf("role access=%+v want version=%d access=%v", view, version, want) + } + permission := view.Permissions[0] + if permission.ActionID != "project.create" || permission.Allowed != want || permission.CapAllowed != want { + t.Fatalf("role permission=%+v want=%v", permission, want) + } +} + func TestRoleHTTPUsesSpecificSystemActions(t *testing.T) { srv, st := newAuthTestServer(t) h := srv.Handler() admin := bootSuperAdmin(t, h, st) - if err := st.CreateAccount("role-manager", "Role Manager", "", false); err != nil { - t.Fatal(err) + managerKey := grantSystemActionsFixture(t, h, st, admin, "role-manager", "system.roles.view", "system.roles.manage") + viewerKey := grantSystemActionsFixture(t, h, st, admin, "role-viewer", "system.roles.view") + + rec := bearerRequest(h, "POST", "/api/admin/roles", `{"name":"测试职位"}`, managerKey) + if rec.Code != http.StatusOK { + t.Fatalf("role manager create: %d %s", rec.Code, rec.Body.String()) } - manager, err := st.GetAccountByOwner("role-manager") + role := decodeRoleHTTP(t, rec) + if created := decodeRoleProjectAccess(t, rec); created.ProjectAccess || len(created.Permissions) != 0 { + t.Fatalf("new role has project access: %+v", created) + } + rec = bearerRequest(h, "PUT", "/api/admin/roles/"+role.RoleID+"/permissions", + `{"expectedRoleVersion":1,"idempotencyKey":"manager-policy","changes":[{"actionId":"project.create","allowed":true,"capAllowed":true}]}`, managerKey) + if rec.Code != http.StatusOK { + t.Fatalf("role manager saved project access: %d %s", rec.Code, rec.Body.String()) + } + requireRoleProjectAccess(t, bearerRequest(h, "GET", "/api/admin/roles/"+role.RoleID, "", managerKey), 2, true) + + rec = bearerRequest(h, "PUT", "/api/admin/roles/"+role.RoleID+"/permissions", + `{"expectedRoleVersion":2,"idempotencyKey":"manager-system-row","changes":[{"actionId":"project.create","allowed":false,"capAllowed":false},{"actionId":"system.settings.write","allowed":true,"capAllowed":true}]}`, managerKey) + if rec.Code != http.StatusBadRequest { + t.Fatalf("role batch with system row: %d %s", rec.Code, rec.Body.String()) + } + requireRoleProjectAccess(t, bearerRequest(h, "GET", "/api/admin/roles/"+role.RoleID, "", managerKey), 2, true) + + requireRoleProjectAccess(t, bearerRequest(h, "GET", "/api/admin/roles/"+role.RoleID, "", viewerKey), 2, true) + if rec := bearerRequest(h, "GET", "/api/admin/roles", "", viewerKey); rec.Code != http.StatusOK || !strings.Contains(rec.Body.String(), `"projectAccess":true`) { + t.Fatalf("role viewer list: %d %s", rec.Code, rec.Body.String()) + } + rec = bearerRequest(h, "PUT", "/api/admin/roles/"+role.RoleID+"/permissions", + `{"expectedRoleVersion":2,"idempotencyKey":"viewer-policy","changes":[]}`, viewerKey) + if rec.Code != http.StatusForbidden { + t.Fatalf("role viewer changed project access: %d %s", rec.Code, rec.Body.String()) + } + if rec := bearerRequest(h, "POST", "/api/admin/roles", `{"name":"查看者职位"}`, viewerKey); rec.Code != http.StatusForbidden { + t.Fatalf("role viewer created role: %d %s", rec.Code, rec.Body.String()) + } + requireRoleProjectAccess(t, bearerRequest(h, "GET", "/api/admin/roles/"+role.RoleID, "", managerKey), 2, true) + + viewer, err := st.GetAccountByOwner("role-viewer") if err != nil { t.Fatal(err) } @@ -104,31 +176,12 @@ func TestRoleHTTPUsesSpecificSystemActions(t *testing.T) { if err != nil { t.Fatal(err) } - grant := do(h, "PUT", "/api/admin/users/"+manager.ID+"/system-grants", - `{"expectedAuthorityEpoch":`+strconv.FormatInt(epoch, 10)+`,"idempotencyKey":"grant-role-manager","actions":["system.roles.view","system.roles.manage"]}`, admin) - if grant.Code != http.StatusOK { - t.Fatalf("grant: %d %s", grant.Code, grant.Body.String()) - } - key, err := st.CreateAPIKey("role-manager", "role-manager-test") - if err != nil { - t.Fatal(err) - } - req := httptest.NewRequest("POST", "/api/admin/roles", strings.NewReader(`{"name":"测试职位"}`)) - req.Header.Set("Content-Type", "application/json") - req.Header.Set("Authorization", "Bearer "+key) - rec := httptest.NewRecorder() - h.ServeHTTP(rec, req) - if rec.Code != http.StatusOK { - t.Fatalf("role manager create: %d %s", rec.Code, rec.Body.String()) - } - role := decodeRoleHTTP(t, rec) - req = httptest.NewRequest("PUT", "/api/admin/roles/"+role.RoleID+"/permissions", - strings.NewReader(`{"expectedRoleVersion":1,"idempotencyKey":"forbidden-policy","changes":[{"actionId":"project.create","allowed":true,"capAllowed":true}]}`)) - req.Header.Set("Content-Type", "application/json") - req.Header.Set("Authorization", "Bearer "+key) - rec = httptest.NewRecorder() - h.ServeHTTP(rec, req) + rec = bearerRequest(h, "PUT", "/api/admin/users/"+viewer.ID+"/system-grants", + `{"expectedAuthorityEpoch":`+strconv.FormatInt(epoch, 10)+`,"idempotencyKey":"manager-grants","actions":["project.create"]}`, managerKey) if rec.Code != http.StatusForbidden { - t.Fatalf("role manager changed permissions: %d %s", rec.Code, rec.Body.String()) + t.Fatalf("role manager changed system grants: %d %s", rec.Code, rec.Body.String()) + } + if actions, _, err := st.SystemGrantsForUser(viewer.ID); err != nil || len(actions) != 1 || actions[0] != "system.roles.view" { + t.Fatalf("viewer grants=%v err=%v", actions, err) } } diff --git a/internal/store/role_policy.go b/internal/store/role_policy.go index 475a92f..a30792a 100644 --- a/internal/store/role_policy.go +++ b/internal/store/role_policy.go @@ -42,10 +42,23 @@ type RolePermissionRecord struct { PermissionVersion int64 `json:"permissionVersion"` } +// RoleAdminView exposes a role's single project-access capability both as +// ProjectAccess and, for existing consumers, as at most one project.create +// permission whose allowed and capAllowed carry the same effective value. type RoleAdminView struct { RoleRecord - BindingCount int `json:"bindingCount"` - Permissions []RolePermissionRecord `json:"permissions"` + BindingCount int `json:"bindingCount"` + ProjectAccess bool `json:"projectAccess"` + Permissions []RolePermissionRecord `json:"permissions"` +} + +func projectAccessOf(permissions []RolePermissionRecord) bool { + for _, permission := range permissions { + if permission.ActionID == catalog.ProjectAccessAction { + return permission.Allowed && permission.CapAllowed + } + } + return false } type PolicyReceipt struct { @@ -368,18 +381,21 @@ func (s *Store) ReplaceRolePermissions(actor, roleID string, expected int64, cha seen := map[string]bool{} for _, change := range changes { descriptor, ok := catalog.DescriptorByID(change.ActionID) - if !ok || descriptor.Protected || seen[change.ActionID] || (change.Allowed && !change.CapAllowed) { + if !ok || descriptor.Protected || change.ActionID != catalog.ProjectAccessAction || + seen[change.ActionID] || (change.Allowed && !change.CapAllowed) { return ErrInvalidRolePolicy } seen[change.ActionID] = true } - if _, err := tx.Exec(s.q(`DELETE FROM role_permissions WHERE role_id=?`), roleID); err != nil { + // Rows for other actions are inert legacy audit data and stay untouched. + if _, err := tx.Exec(s.q(`DELETE FROM role_permissions WHERE role_id=? AND action_id=?`), roleID, catalog.ProjectAccessAction); err != nil { return err } now := nowISO() for _, change := range changes { + access := change.Allowed && change.CapAllowed if _, err := tx.Exec(s.q(`INSERT INTO role_permissions(role_id,action_id,allowed,cap_allowed,permission_version,updated_by,updated_at) - VALUES(?,?,?,?,?,?,?)`), roleID, change.ActionID, change.Allowed, change.CapAllowed, 1, actorAccount.ID, now); err != nil { + VALUES(?,?,?,?,?,?,?)`), roleID, change.ActionID, access, access, 1, actorAccount.ID, now); err != nil { return err } } @@ -507,24 +523,28 @@ func (s *Store) RoleAdmin(roleID string) (RoleAdminView, error) { return RoleAdminView{}, err } view.Permissions, err = s.RolePermissions(roleID) + view.ProjectAccess = projectAccessOf(view.Permissions) return view, err } +// RolePermissions returns the role's normalized capability: at most one +// project.create record whose allowed and capAllowed both equal the stored +// allowed&&capAllowed. Legacy rows for any other action are filtered out. func (s *Store) RolePermissions(roleID string) ([]RolePermissionRecord, error) { - rows, err := s.db.Query(s.q(`SELECT action_id,allowed,cap_allowed,permission_version FROM role_permissions WHERE role_id=? ORDER BY action_id`), roleID) + var permission RolePermissionRecord + var allowed, capAllowed bool + err := s.db.QueryRow(s.q(`SELECT allowed,cap_allowed,permission_version FROM role_permissions WHERE role_id=? AND action_id=?`), + roleID, catalog.ProjectAccessAction).Scan(&allowed, &capAllowed, &permission.PermissionVersion) + if errors.Is(err, sql.ErrNoRows) { + return []RolePermissionRecord{}, nil + } if err != nil { return nil, err } - defer rows.Close() - out := []RolePermissionRecord{} - for rows.Next() { - var permission RolePermissionRecord - if err := rows.Scan(&permission.ActionID, &permission.Allowed, &permission.CapAllowed, &permission.PermissionVersion); err != nil { - return nil, err - } - out = append(out, permission) - } - return out, rows.Err() + permission.ActionID = catalog.ProjectAccessAction + permission.Allowed = allowed && capAllowed + permission.CapAllowed = permission.Allowed + return []RolePermissionRecord{permission}, nil } func (s *Store) ListRoles(query, mode, status, actionID, cursor string, limit int) ([]RoleAdminView, string, error) { @@ -561,8 +581,11 @@ func (s *Store) ListRoles(query, mode, status, actionID, cursor string, limit in if _, ok := catalog.DescriptorByID(actionID); !ok { return nil, "", ErrInvalidRolePolicy } - where = append(where, "EXISTS(SELECT 1 FROM role_permissions p WHERE p.role_id=roles.role_id AND p.action_id=?)") - args = append(args, actionID) + if actionID != catalog.ProjectAccessAction { + return []RoleAdminView{}, "", nil + } + where = append(where, "EXISTS(SELECT 1 FROM role_permissions p WHERE p.role_id=roles.role_id AND p.action_id=? AND p.allowed=? AND p.cap_allowed=?)") + args = append(args, actionID, true, true) } args = append(args, limit+1) rows, err := s.db.Query(s.q(`SELECT role_id,name,description,enabled,role_version,created_by,updated_by,created_at,updated_at, @@ -598,6 +621,7 @@ func (s *Store) ListRoles(query, mode, status, actionID, cursor string, limit in return nil, "", err } out[i].Permissions = permissions + out[i].ProjectAccess = projectAccessOf(permissions) } return out, next, nil } @@ -660,7 +684,14 @@ func (s *Store) requireSystemActionTx(tx *sql.Tx, actor Account, actionID string return err } +// RolePermissionAllowed keeps its historical (allowed, capAllowed) shape but +// both results now carry the same normalized project-access value: the stored +// allowed&&capAllowed of the actor's active, enabled role. Any action other +// than project.create is never role-authorized. func (s *Store) RolePermissionAllowed(owner, actionID string) (bool, bool, error) { + if actionID != catalog.ProjectAccessAction { + return false, false, nil + } var allowed, capAllowed bool err := s.db.QueryRow(s.q(`SELECT p.allowed,p.cap_allowed FROM users u JOIN user_role_bindings b ON b.user_id=u.id AND b.active=TRUE @@ -670,7 +701,11 @@ func (s *Store) RolePermissionAllowed(owner, actionID string) (bool, bool, error if errors.Is(err, sql.ErrNoRows) { return false, false, nil } - return allowed, capAllowed, err + if err != nil { + return false, false, err + } + access := allowed && capAllowed + return access, access, nil } // roleGrantsAccountAdministrationTx reports whether a role allows any diff --git a/internal/store/role_policy_test.go b/internal/store/role_policy_test.go index 2ec958e..4fbc2f1 100644 --- a/internal/store/role_policy_test.go +++ b/internal/store/role_policy_test.go @@ -260,3 +260,195 @@ func TestImportRoleAuthorityPreservesStableIDsAndBindingsAtomically(t *testing.T t.Fatalf("non-empty authority accepted a second import: %v", err) } } + +func insertLegacyRolePermission(t *testing.T, st *Store, roleID, actionID string, allowed, capAllowed bool) { + t.Helper() + admin, err := st.GetAccountByOwner("admin") + if err != nil { + t.Fatal(err) + } + if _, err := st.db.Exec(st.q(`INSERT INTO role_permissions(role_id,action_id,allowed,cap_allowed,permission_version,updated_by,updated_at) + VALUES(?,?,?,?,?,?,?) ON CONFLICT(role_id,action_id) DO UPDATE SET allowed=excluded.allowed,cap_allowed=excluded.cap_allowed`), + roleID, actionID, allowed, capAllowed, 1, admin.ID, nowISO()); err != nil { + t.Fatal(err) + } +} + +func TestRolePermissionsNormalizeLegacyRowsToProjectAccess(t *testing.T) { + st := openTestStore(t) + _, alice, _ := rolePolicyAccounts(t, st) + role, err := st.CreateRole("admin", RoleCreate{Name: "旧模型职位"}) + if err != nil { + t.Fatal(err) + } + if _, err := st.AssignUserRole("admin", alice.ID, role.RoleID, alice.StateVersion, "legacy-assign"); err != nil { + t.Fatal(err) + } + for _, actionID := range []string{"system.settings.read", "system.settings.write", "system.users.view", "system.users.create", "document.read"} { + insertLegacyRolePermission(t, st, role.RoleID, actionID, true, true) + } + for _, tc := range []struct { + name string + allowed, capAllowed bool + want bool + }{ + {"true-true-allows", true, true, true}, + {"false-true-denies", false, true, false}, + {"false-false-denies", false, false, false}, + {"illegal-true-false-denies", true, false, false}, + } { + t.Run(tc.name, func(t *testing.T) { + insertLegacyRolePermission(t, st, role.RoleID, "project.create", tc.allowed, tc.capAllowed) + permissions, err := st.RolePermissions(role.RoleID) + if err != nil || len(permissions) != 1 { + t.Fatalf("permissions=%+v err=%v", permissions, err) + } + got := permissions[0] + if got.ActionID != "project.create" || got.Allowed != tc.want || got.CapAllowed != tc.want { + t.Fatalf("normalized=%+v want=%v", got, tc.want) + } + view, err := st.RoleAdmin(role.RoleID) + if err != nil || view.ProjectAccess != tc.want || len(view.Permissions) != 1 || view.Permissions[0].ActionID != "project.create" { + t.Fatalf("role admin=%+v err=%v", view, err) + } + roles, _, err := st.ListRoles("", "fuzzy", "all", "", "", 20) + if err != nil || len(roles) != 1 || roles[0].ProjectAccess != tc.want || len(roles[0].Permissions) != 1 { + t.Fatalf("list roles=%+v err=%v", roles, err) + } + allowed, capAllowed, err := st.RolePermissionAllowed("alice", "project.create") + if err != nil || allowed != tc.want || capAllowed != tc.want { + t.Fatalf("role allowed=%v cap=%v err=%v want=%v", allowed, capAllowed, err, tc.want) + } + withAccess, _, err := st.ListRoles("", "fuzzy", "all", "project.create", "", 20) + if err != nil || (len(withAccess) == 1) != tc.want { + t.Fatalf("project access filter=%+v err=%v want=%v", withAccess, err, tc.want) + } + }) + } + for _, actionID := range []string{"system.settings.read", "system.users.view", "document.read"} { + allowed, capAllowed, err := st.RolePermissionAllowed("alice", actionID) + if err != nil || allowed || capAllowed { + t.Fatalf("legacy role row authorized %s: allowed=%v cap=%v err=%v", actionID, allowed, capAllowed, err) + } + filtered, _, err := st.ListRoles("", "fuzzy", "all", actionID, "", 20) + if err != nil || len(filtered) != 0 { + t.Fatalf("legacy action filter %s=%+v err=%v", actionID, filtered, err) + } + } + var legacy int + if err := st.db.QueryRow(st.q(`SELECT COUNT(*) FROM role_permissions WHERE role_id=? AND action_id<>?`), role.RoleID, "project.create").Scan(&legacy); err != nil || legacy != 5 { + t.Fatalf("legacy audit rows=%d err=%v", legacy, err) + } +} + +func TestRolePolicyProjectAccessFailsClosedForDisabledRoleOrAccount(t *testing.T) { + st := openTestStore(t) + _, alice, _ := rolePolicyAccounts(t, st) + role, err := st.CreateRole("admin", RoleCreate{Name: "项目职位"}) + if err != nil { + t.Fatal(err) + } + receipt, err := st.ReplaceRolePermissions("admin", role.RoleID, role.RoleVersion, + []RolePermissionChange{{ActionID: "project.create", Allowed: true, CapAllowed: true}}, "access-on") + if err != nil { + t.Fatal(err) + } + if _, err := st.AssignUserRole("admin", alice.ID, role.RoleID, alice.StateVersion, "access-assign"); err != nil { + t.Fatal(err) + } + if allowed, _, err := st.RolePermissionAllowed("alice", "project.create"); err != nil || !allowed { + t.Fatalf("enabled role access=%v err=%v", allowed, err) + } + if allowed, _, err := st.RolePermissionAllowed("alice", "project.unknown"); err != nil || allowed { + t.Fatalf("unknown action access=%v err=%v", allowed, err) + } + disabled := false + if _, err := st.PatchRole("admin", role.RoleID, receipt.RoleVersion, RolePatch{Enabled: &disabled}); err != nil { + t.Fatal(err) + } + if allowed, _, err := st.RolePermissionAllowed("alice", "project.create"); err != nil || allowed { + t.Fatalf("disabled role access=%v err=%v", allowed, err) + } + enabled := true + current, _ := st.GetRole(role.RoleID) + if _, err := st.PatchRole("admin", role.RoleID, current.RoleVersion, RolePatch{Enabled: &enabled}); err != nil { + t.Fatal(err) + } + alice, _ = st.GetAccount(alice.ID) + if _, err := st.SetAccountEnabled("admin", alice.ID, alice.StateVersion, false); err != nil { + t.Fatal(err) + } + if allowed, _, err := st.RolePermissionAllowed("alice", "project.create"); err != nil || allowed { + t.Fatalf("disabled account access=%v err=%v", allowed, err) + } +} + +func TestReplaceRolePermissionsAcceptsOnlyProjectAccessBoolean(t *testing.T) { + st := openTestStore(t) + rolePolicyAccounts(t, st) + role, err := st.CreateRole("admin", RoleCreate{Name: "项目职位"}) + if err != nil { + t.Fatal(err) + } + insertLegacyRolePermission(t, st, role.RoleID, "system.users.view", true, true) + stored := func() (bool, bool, int) { + t.Helper() + var allowed, capAllowed bool + var rows int + if err := st.db.QueryRow(st.q(`SELECT COUNT(*) FROM role_permissions WHERE role_id=? AND action_id=?`), role.RoleID, "project.create").Scan(&rows); err != nil { + t.Fatal(err) + } + if rows == 1 { + if err := st.db.QueryRow(st.q(`SELECT allowed,cap_allowed FROM role_permissions WHERE role_id=? AND action_id=?`), role.RoleID, "project.create").Scan(&allowed, &capAllowed); err != nil { + t.Fatal(err) + } + } + return allowed, capAllowed, rows + } + for i, bad := range [][]RolePermissionChange{ + {{ActionID: "project.create", Allowed: true, CapAllowed: true}, {ActionID: "system.settings.read", Allowed: true, CapAllowed: true}}, + {{ActionID: "project.create", Allowed: true, CapAllowed: true}, {ActionID: "system.users.create", Allowed: false, CapAllowed: false}}, + {{ActionID: "document.read", Allowed: true, CapAllowed: true}}, + {{ActionID: "project.create", Allowed: true, CapAllowed: true}, {ActionID: "project.create", Allowed: true, CapAllowed: true}}, + {{ActionID: "project.create", Allowed: true, CapAllowed: false}}, + } { + if _, err := st.ReplaceRolePermissions("admin", role.RoleID, role.RoleVersion, bad, "bad-"+string(rune('a'+i))); !errors.Is(err, ErrInvalidRolePolicy) { + t.Fatalf("batch %d accepted: %v", i, err) + } + if _, _, rows := stored(); rows != 0 { + t.Fatalf("batch %d partially wrote project access", i) + } + if current, _ := st.GetRole(role.RoleID); current.RoleVersion != role.RoleVersion { + t.Fatalf("batch %d bumped role version: %+v", i, current) + } + } + version := role.RoleVersion + for i, tc := range []struct { + change RolePermissionChange + want bool + }{ + {RolePermissionChange{ActionID: "project.create", Allowed: true, CapAllowed: true}, true}, + {RolePermissionChange{ActionID: "project.create", Allowed: false, CapAllowed: true}, false}, + {RolePermissionChange{ActionID: "project.create", Allowed: true, CapAllowed: true}, true}, + } { + receipt, err := st.ReplaceRolePermissions("admin", role.RoleID, version, []RolePermissionChange{tc.change}, "good-"+string(rune('a'+i))) + if err != nil { + t.Fatal(err) + } + version = receipt.RoleVersion + allowed, capAllowed, rows := stored() + if rows != 1 || allowed != tc.want || capAllowed != tc.want { + t.Fatalf("step %d stored allowed=%v cap=%v rows=%d want=%v", i, allowed, capAllowed, rows, tc.want) + } + } + if _, err := st.ReplaceRolePermissions("admin", role.RoleID, version, nil, "clear"); err != nil { + t.Fatal(err) + } + if _, _, rows := stored(); rows != 0 { + t.Fatalf("empty batch kept project access rows=%d", rows) + } + var legacy int + if err := st.db.QueryRow(st.q(`SELECT COUNT(*) FROM role_permissions WHERE role_id=? AND action_id=?`), role.RoleID, "system.users.view").Scan(&legacy); err != nil || legacy != 1 { + t.Fatalf("legacy audit row removed: %d err=%v", legacy, err) + } +}