From 6ad8f61260da017475ea71aba63a133af14d8b0c Mon Sep 17 00:00:00 2001 From: aman Date: Tue, 19 May 2026 14:25:33 +0530 Subject: [PATCH 1/3] refactor(policy): add Filter.RolePermissions, use it in membership listing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pushes the "policies whose role grants any of these permissions" filter into the policy layer. The repo joins roles and uses jsonb_exists_any to match against role.permissions. Membership's three project-listing helpers drop the local list-then-filter dance and pass RolePermissions through to policy.List. The filterByRolePermissions helper is removed. Org and group listing don't use the field — they don't gate by role permissions, behavior unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) --- core/membership/service.go | 113 +++------- core/membership/service_test.go | 210 +++++++++---------- core/policy/filter.go | 2 + internal/store/postgres/policy_repository.go | 15 ++ 4 files changed, 148 insertions(+), 192 deletions(-) diff --git a/core/membership/service.go b/core/membership/service.go index d3e293bae..684c1f33a 100644 --- a/core/membership/service.go +++ b/core/membership/service.go @@ -1716,24 +1716,24 @@ func (s *Service) listProjectsForPrincipal(ctx context.Context, principalID, pri } // listDirectProjectIDs returns projects the principal has a direct policy on, -// kept only if the role grants at least one permission in -// schema.ProjectDirectVisibilityPerms. This is the project-listing analog of -// what SpiceDB does today for the "get" check on a project. +// kept only if the role grants any of the permissions that imply project +// visibility. This is the project-listing analog of what SpiceDB does today +// for the "get" check on a project. func (s *Service) listDirectProjectIDs(ctx context.Context, principalID, principalType string) ([]string, error) { policies, err := s.policyService.List(ctx, policy.Filter{ - PrincipalID: principalID, - PrincipalType: principalType, - ResourceType: schema.ProjectNamespace, + PrincipalID: principalID, + PrincipalType: principalType, + ResourceType: schema.ProjectNamespace, + RolePermissions: schema.ProjectDirectVisibilityPerms, }) if err != nil { return nil, fmt.Errorf("list direct project policies: %w", err) } - return s.filterByRolePermissions(ctx, policies, schema.ProjectDirectVisibilityPerms) + return policyResourceIDs(policies), nil } -// listGroupExpandedProjectIDs: principal → groups → project policies on those -// groups → gated by schema.ProjectDirectVisibilityPerms (same rule as direct -// project policies). +// listGroupExpandedProjectIDs walks: principal → groups → project policies on +// those groups → kept only if the role grants project visibility. func (s *Service) listGroupExpandedProjectIDs(ctx context.Context, principalID, principalType string) ([]string, error) { // Use the per-principal helper (not ListResourcesByPrincipal) so the PAT // pass doesn't trigger another PAT recursion on itself. @@ -1745,36 +1745,34 @@ func (s *Service) listGroupExpandedProjectIDs(ctx context.Context, principalID, return nil, nil } policies, err := s.policyService.List(ctx, policy.Filter{ - PrincipalType: schema.GroupPrincipal, - PrincipalIDs: groupIDs, - ResourceType: schema.ProjectNamespace, + PrincipalType: schema.GroupPrincipal, + PrincipalIDs: groupIDs, + ResourceType: schema.ProjectNamespace, + RolePermissions: schema.ProjectDirectVisibilityPerms, }) if err != nil { return nil, fmt.Errorf("list project policies for principal groups: %w", err) } - return s.filterByRolePermissions(ctx, policies, schema.ProjectDirectVisibilityPerms) + return policyResourceIDs(policies), nil } // listOrgInheritedProjectIDs finds projects a principal can see by virtue of // holding a strong-enough role on the project's org (e.g. Org Owner sees all // projects in their org; Org Viewer doesn't). Steps: -// - get the principal's policies on orgs -// - keep only the orgs whose role grants something in -// schema.OrganizationProjectInheritPerms +// - get the principal's policies on orgs, kept only if the role grants any +// permission that implies org→all-projects inheritance // - fetch all projects in those orgs in a single batched query func (s *Service) listOrgInheritedProjectIDs(ctx context.Context, principalID, principalType string) ([]string, error) { policies, err := s.policyService.List(ctx, policy.Filter{ - PrincipalID: principalID, - PrincipalType: principalType, - ResourceType: schema.OrganizationNamespace, + PrincipalID: principalID, + PrincipalType: principalType, + ResourceType: schema.OrganizationNamespace, + RolePermissions: schema.OrganizationProjectInheritPerms, }) if err != nil { return nil, fmt.Errorf("list org policies for inheritance: %w", err) } - inheritingOrgIDs, err := s.filterByRolePermissions(ctx, policies, schema.OrganizationProjectInheritPerms) - if err != nil { - return nil, err - } + inheritingOrgIDs := policyResourceIDs(policies) if len(inheritingOrgIDs) == 0 { return nil, nil } @@ -1789,6 +1787,15 @@ func (s *Service) listOrgInheritedProjectIDs(ctx context.Context, principalID, p return ids, nil } +// policyResourceIDs plucks the deduped resource IDs from a policy slice. +func policyResourceIDs(policies []policy.Policy) []string { + ids := make([]string, 0, len(policies)) + for _, pol := range policies { + ids = append(ids, pol.ResourceID) + } + return utils.Deduplicate(ids) +} + // narrowProjectsByOrg keeps only IDs whose org_id matches orgID (single query). func (s *Service) narrowProjectsByOrg(ctx context.Context, ids []string, orgID string) ([]string, error) { projects, err := s.projectService.List(ctx, project.Filter{ @@ -1804,61 +1811,3 @@ func (s *Service) narrowProjectsByOrg(ctx context.Context, ids []string, orgID s } return out, nil } - -// filterByRolePermissions returns resource IDs from policies whose role grants -// at least one of the given permissions. Roles are loaded once in a single -// batched call, not one lookup per policy. -// -// We don't look at how the policy was granted (direct vs. PAT) — the -// permission lists already account for both kinds today. If the schema ever -// makes the two paths grant different sets of permissions, this needs to -// branch on grant_relation. -func (s *Service) filterByRolePermissions(ctx context.Context, policies []policy.Policy, permissions []string) ([]string, error) { - if len(policies) == 0 || len(permissions) == 0 { - return nil, nil - } - - wanted := make(map[string]struct{}, len(permissions)) - for _, p := range permissions { - wanted[p] = struct{}{} - } - - roleIDSet := make(map[string]struct{}, len(policies)) - for _, pol := range policies { - if pol.RoleID == "" { - continue - } - roleIDSet[pol.RoleID] = struct{}{} - } - if len(roleIDSet) == 0 { - return nil, nil - } - roleIDs := make([]string, 0, len(roleIDSet)) - for id := range roleIDSet { - roleIDs = append(roleIDs, id) - } - - roles, err := s.roleService.List(ctx, role.Filter{IDs: roleIDs}) - if err != nil { - return nil, fmt.Errorf("list roles for permission filter: %w", err) - } - rolePermits := make(map[string]bool, len(roles)) - for _, r := range roles { - grants := false - for _, p := range r.Permissions { - if _, ok := wanted[p]; ok { - grants = true - break - } - } - rolePermits[r.ID] = grants - } - - out := make([]string, 0, len(policies)) - for _, pol := range policies { - if rolePermits[pol.RoleID] { - out = append(out, pol.ResourceID) - } - } - return utils.Deduplicate(out), nil -} diff --git a/core/membership/service_test.go b/core/membership/service_test.go index f93b2907c..267186e8e 100644 --- a/core/membership/service_test.go +++ b/core/membership/service_test.go @@ -1813,15 +1813,10 @@ func TestService_ListResourcesByPrincipal(t *testing.T) { roleProjectViewerID := uuid.New().String() roleProjectOwnerID := uuid.New().String() - // roles with permissions that mirror the canonical predefined roles. - orgViewerRole := role.Role{ID: roleOrgViewerID, Name: schema.RoleOrganizationViewer, Permissions: []string{"app_organization_get"}} - orgManagerRole := role.Role{ID: roleOrgManagerID, Name: schema.RoleOrganizationManager, Permissions: []string{ - "app_organization_update", "app_organization_get", "app_project_get", "app_project_update", - }} - orgOwnerRole := role.Role{ID: roleOrgOwnerID, Name: schema.RoleOrganizationOwner, Permissions: []string{"app_organization_administer"}} - orgCustomRole := role.Role{ID: roleOrgCustomID, Name: "custom_app_project_admin", Permissions: []string{"app_project_administer"}} - projectViewerRole := role.Role{ID: roleProjectViewerID, Name: schema.RoleProjectViewer, Permissions: []string{"app_project_get"}} - projectOwnerRole := role.Role{ID: roleProjectOwnerID, Name: schema.RoleProjectOwner, Permissions: []string{"app_project_administer"}} + // The full role.Role fixtures aren't needed anymore — the role-permission + // gate now happens at the policy filter (policy.Filter.RolePermissions), + // so the mocked policyService.List returns are already gated. We just + // need the role IDs to attach to mock policies. type mockSet struct { policy *mocks.PolicyService @@ -1909,22 +1904,20 @@ func TestService_ListResourcesByPrincipal(t *testing.T) { want: []string{groupA}, }, { - name: "project listing: direct policy gated by ProjectDirectVisibility, viewer role kept", + name: "project listing: direct policy with role granting project visibility is returned", principal: authenticate.Principal{ID: userID, Type: schema.UserPrincipal}, resourceType: schema.ProjectNamespace, filter: membership.ResourceFilter{NonInherited: true}, setup: func(m *mockSet) { - // direct project policies + // direct project policies — gated by RolePermissions at policy.Filter m.policy.EXPECT().List(ctx, policy.Filter{ - PrincipalID: userID, - PrincipalType: schema.UserPrincipal, - ResourceType: schema.ProjectNamespace, + PrincipalID: userID, + PrincipalType: schema.UserPrincipal, + ResourceType: schema.ProjectNamespace, + RolePermissions: schema.ProjectDirectVisibilityPerms, }).Return([]policy.Policy{ {ResourceID: project1, RoleID: roleProjectViewerID}, }, nil) - m.role.EXPECT().List(ctx, mock.MatchedBy(func(f role.Filter) bool { - return len(f.IDs) == 1 && f.IDs[0] == roleProjectViewerID - })).Return([]role.Role{projectViewerRole}, nil) // group expansion: principal has no groups m.policy.EXPECT().List(ctx, policy.Filter{ PrincipalID: userID, @@ -1941,9 +1934,10 @@ func TestService_ListResourcesByPrincipal(t *testing.T) { resourceType: schema.ProjectNamespace, setup: func(m *mockSet) { m.policy.EXPECT().List(ctx, policy.Filter{ - PrincipalID: userID, - PrincipalType: schema.UserPrincipal, - ResourceType: schema.ProjectNamespace, + PrincipalID: userID, + PrincipalType: schema.UserPrincipal, + ResourceType: schema.ProjectNamespace, + RolePermissions: schema.ProjectDirectVisibilityPerms, }).Return([]policy.Policy{}, nil) m.policy.EXPECT().List(ctx, policy.Filter{ PrincipalID: userID, @@ -1951,15 +1945,13 @@ func TestService_ListResourcesByPrincipal(t *testing.T) { ResourceType: schema.GroupNamespace, }).Return([]policy.Policy{}, nil) m.policy.EXPECT().List(ctx, policy.Filter{ - PrincipalID: userID, - PrincipalType: schema.UserPrincipal, - ResourceType: schema.OrganizationNamespace, + PrincipalID: userID, + PrincipalType: schema.UserPrincipal, + ResourceType: schema.OrganizationNamespace, + RolePermissions: schema.OrganizationProjectInheritPerms, }).Return([]policy.Policy{ {ResourceID: orgA, RoleID: roleOrgOwnerID}, }, nil) - m.role.EXPECT().List(ctx, mock.MatchedBy(func(f role.Filter) bool { - return len(f.IDs) == 1 && f.IDs[0] == roleOrgOwnerID - })).Return([]role.Role{orgOwnerRole}, nil) m.project.EXPECT().List(ctx, project.Filter{OrgIDs: []string{orgA}}).Return([]project.Project{ {ID: project1}, {ID: project2}, {ID: project3}, }, nil) @@ -1972,9 +1964,10 @@ func TestService_ListResourcesByPrincipal(t *testing.T) { resourceType: schema.ProjectNamespace, setup: func(m *mockSet) { m.policy.EXPECT().List(ctx, policy.Filter{ - PrincipalID: userID, - PrincipalType: schema.UserPrincipal, - ResourceType: schema.ProjectNamespace, + PrincipalID: userID, + PrincipalType: schema.UserPrincipal, + ResourceType: schema.ProjectNamespace, + RolePermissions: schema.ProjectDirectVisibilityPerms, }).Return([]policy.Policy{}, nil) m.policy.EXPECT().List(ctx, policy.Filter{ PrincipalID: userID, @@ -1982,13 +1975,13 @@ func TestService_ListResourcesByPrincipal(t *testing.T) { ResourceType: schema.GroupNamespace, }).Return([]policy.Policy{}, nil) m.policy.EXPECT().List(ctx, policy.Filter{ - PrincipalID: userID, - PrincipalType: schema.UserPrincipal, - ResourceType: schema.OrganizationNamespace, + PrincipalID: userID, + PrincipalType: schema.UserPrincipal, + ResourceType: schema.OrganizationNamespace, + RolePermissions: schema.OrganizationProjectInheritPerms, }).Return([]policy.Policy{ {ResourceID: orgA, RoleID: roleOrgManagerID}, }, nil) - m.role.EXPECT().List(ctx, mock.Anything).Return([]role.Role{orgManagerRole}, nil) m.project.EXPECT().List(ctx, project.Filter{OrgIDs: []string{orgA}}).Return([]project.Project{ {ID: project1}, {ID: project2}, }, nil) @@ -2001,24 +1994,25 @@ func TestService_ListResourcesByPrincipal(t *testing.T) { resourceType: schema.ProjectNamespace, setup: func(m *mockSet) { m.policy.EXPECT().List(ctx, policy.Filter{ - PrincipalID: userID, - PrincipalType: schema.UserPrincipal, - ResourceType: schema.ProjectNamespace, + PrincipalID: userID, + PrincipalType: schema.UserPrincipal, + ResourceType: schema.ProjectNamespace, + RolePermissions: schema.ProjectDirectVisibilityPerms, }).Return([]policy.Policy{}, nil) m.policy.EXPECT().List(ctx, policy.Filter{ PrincipalID: userID, PrincipalType: schema.UserPrincipal, ResourceType: schema.GroupNamespace, }).Return([]policy.Policy{}, nil) + // SQL filter excludes the viewer's policy (role doesn't grant + // any OrganizationProjectInheritPerms) — empty result, no + // follow-up projectService.List call. m.policy.EXPECT().List(ctx, policy.Filter{ - PrincipalID: userID, - PrincipalType: schema.UserPrincipal, - ResourceType: schema.OrganizationNamespace, - }).Return([]policy.Policy{ - {ResourceID: orgA, RoleID: roleOrgViewerID}, - }, nil) - m.role.EXPECT().List(ctx, mock.Anything).Return([]role.Role{orgViewerRole}, nil) - // no projectService.List call expected — inheritingOrgIDs is empty + PrincipalID: userID, + PrincipalType: schema.UserPrincipal, + ResourceType: schema.OrganizationNamespace, + RolePermissions: schema.OrganizationProjectInheritPerms, + }).Return([]policy.Policy{}, nil) }, want: []string{}, }, @@ -2028,9 +2022,10 @@ func TestService_ListResourcesByPrincipal(t *testing.T) { resourceType: schema.ProjectNamespace, setup: func(m *mockSet) { m.policy.EXPECT().List(ctx, policy.Filter{ - PrincipalID: userID, - PrincipalType: schema.UserPrincipal, - ResourceType: schema.ProjectNamespace, + PrincipalID: userID, + PrincipalType: schema.UserPrincipal, + ResourceType: schema.ProjectNamespace, + RolePermissions: schema.ProjectDirectVisibilityPerms, }).Return([]policy.Policy{}, nil) m.policy.EXPECT().List(ctx, policy.Filter{ PrincipalID: userID, @@ -2038,13 +2033,13 @@ func TestService_ListResourcesByPrincipal(t *testing.T) { ResourceType: schema.GroupNamespace, }).Return([]policy.Policy{}, nil) m.policy.EXPECT().List(ctx, policy.Filter{ - PrincipalID: userID, - PrincipalType: schema.UserPrincipal, - ResourceType: schema.OrganizationNamespace, + PrincipalID: userID, + PrincipalType: schema.UserPrincipal, + ResourceType: schema.OrganizationNamespace, + RolePermissions: schema.OrganizationProjectInheritPerms, }).Return([]policy.Policy{ {ResourceID: orgA, RoleID: roleOrgCustomID}, }, nil) - m.role.EXPECT().List(ctx, mock.Anything).Return([]role.Role{orgCustomRole}, nil) m.project.EXPECT().List(ctx, project.Filter{OrgIDs: []string{orgA}}).Return([]project.Project{ {ID: project1}, }, nil) @@ -2058,11 +2053,13 @@ func TestService_ListResourcesByPrincipal(t *testing.T) { filter: membership.ResourceFilter{NonInherited: true}, setup: func(m *mockSet) { m.policy.EXPECT().List(ctx, policy.Filter{ - PrincipalID: userID, - PrincipalType: schema.UserPrincipal, - ResourceType: schema.ProjectNamespace, + PrincipalID: userID, + PrincipalType: schema.UserPrincipal, + ResourceType: schema.ProjectNamespace, + RolePermissions: schema.ProjectDirectVisibilityPerms, }).Return([]policy.Policy{}, nil) - // recursion to list groups for the user + // recursion to list groups for the user (no RolePermissions — + // group listing isn't role-permission-gated) m.policy.EXPECT().List(ctx, policy.Filter{ PrincipalID: userID, PrincipalType: schema.UserPrincipal, @@ -2070,17 +2067,15 @@ func TestService_ListResourcesByPrincipal(t *testing.T) { }).Return([]policy.Policy{ {ResourceID: groupA, RoleID: uuid.New().String()}, }, nil) - // then project policies on those groups + // then project policies on those groups, also gated m.policy.EXPECT().List(ctx, policy.Filter{ - PrincipalType: schema.GroupPrincipal, - PrincipalIDs: []string{groupA}, - ResourceType: schema.ProjectNamespace, + PrincipalType: schema.GroupPrincipal, + PrincipalIDs: []string{groupA}, + ResourceType: schema.ProjectNamespace, + RolePermissions: schema.ProjectDirectVisibilityPerms, }).Return([]policy.Policy{ {ResourceID: project2, RoleID: roleProjectViewerID}, }, nil) - m.role.EXPECT().List(ctx, mock.MatchedBy(func(f role.Filter) bool { - return len(f.IDs) == 1 && f.IDs[0] == roleProjectViewerID - })).Return([]role.Role{projectViewerRole}, nil) }, want: []string{project2}, }, @@ -2091,16 +2086,14 @@ func TestService_ListResourcesByPrincipal(t *testing.T) { filter: membership.ResourceFilter{OrgID: orgA, NonInherited: true}, setup: func(m *mockSet) { m.policy.EXPECT().List(ctx, policy.Filter{ - PrincipalID: userID, - PrincipalType: schema.UserPrincipal, - ResourceType: schema.ProjectNamespace, + PrincipalID: userID, + PrincipalType: schema.UserPrincipal, + ResourceType: schema.ProjectNamespace, + RolePermissions: schema.ProjectDirectVisibilityPerms, }).Return([]policy.Policy{ {ResourceID: project1, RoleID: roleProjectViewerID}, {ResourceID: project2, RoleID: roleProjectViewerID}, }, nil) - m.role.EXPECT().List(ctx, mock.MatchedBy(func(f role.Filter) bool { - return len(f.IDs) == 1 - })).Return([]role.Role{projectViewerRole}, nil) m.policy.EXPECT().List(ctx, policy.Filter{ PrincipalID: userID, PrincipalType: schema.UserPrincipal, @@ -2141,13 +2134,13 @@ func TestService_ListResourcesByPrincipal(t *testing.T) { setup: func(m *mockSet) { // only the user-pass queries fire; no second list under the PAT principal type m.policy.EXPECT().List(ctx, policy.Filter{ - PrincipalID: userID, - PrincipalType: schema.UserPrincipal, - ResourceType: schema.ProjectNamespace, + PrincipalID: userID, + PrincipalType: schema.UserPrincipal, + ResourceType: schema.ProjectNamespace, + RolePermissions: schema.ProjectDirectVisibilityPerms, }).Return([]policy.Policy{ {ResourceID: project1, RoleID: roleProjectViewerID}, }, nil) - m.role.EXPECT().List(ctx, mock.Anything).Return([]role.Role{projectViewerRole}, nil) m.policy.EXPECT().List(ctx, policy.Filter{ PrincipalID: userID, PrincipalType: schema.UserPrincipal, @@ -2167,9 +2160,10 @@ func TestService_ListResourcesByPrincipal(t *testing.T) { setup: func(m *mockSet) { // user pass — user is org owner, expands via inheritance m.policy.EXPECT().List(ctx, policy.Filter{ - PrincipalID: userID, - PrincipalType: schema.UserPrincipal, - ResourceType: schema.ProjectNamespace, + PrincipalID: userID, + PrincipalType: schema.UserPrincipal, + ResourceType: schema.ProjectNamespace, + RolePermissions: schema.ProjectDirectVisibilityPerms, }).Return([]policy.Policy{}, nil) m.policy.EXPECT().List(ctx, policy.Filter{ PrincipalID: userID, @@ -2177,23 +2171,22 @@ func TestService_ListResourcesByPrincipal(t *testing.T) { ResourceType: schema.GroupNamespace, }).Return([]policy.Policy{}, nil) m.policy.EXPECT().List(ctx, policy.Filter{ - PrincipalID: userID, - PrincipalType: schema.UserPrincipal, - ResourceType: schema.OrganizationNamespace, + PrincipalID: userID, + PrincipalType: schema.UserPrincipal, + ResourceType: schema.OrganizationNamespace, + RolePermissions: schema.OrganizationProjectInheritPerms, }).Return([]policy.Policy{ {ResourceID: orgA, RoleID: roleOrgOwnerID}, }, nil) - m.role.EXPECT().List(ctx, mock.MatchedBy(func(f role.Filter) bool { - return len(f.IDs) == 1 && f.IDs[0] == roleOrgOwnerID - })).Return([]role.Role{orgOwnerRole}, nil) m.project.EXPECT().List(ctx, project.Filter{OrgIDs: []string{orgA}}).Return([]project.Project{ {ID: project1}, {ID: project2}, {ID: project3}, }, nil) // PAT pass — all-projects scope is one pat_granted policy on the org m.policy.EXPECT().List(ctx, policy.Filter{ - PrincipalID: patID, - PrincipalType: schema.PATPrincipal, - ResourceType: schema.ProjectNamespace, + PrincipalID: patID, + PrincipalType: schema.PATPrincipal, + ResourceType: schema.ProjectNamespace, + RolePermissions: schema.ProjectDirectVisibilityPerms, }).Return([]policy.Policy{}, nil) m.policy.EXPECT().List(ctx, policy.Filter{ PrincipalID: patID, @@ -2201,18 +2194,16 @@ func TestService_ListResourcesByPrincipal(t *testing.T) { ResourceType: schema.GroupNamespace, }).Return([]policy.Policy{}, nil) m.policy.EXPECT().List(ctx, policy.Filter{ - PrincipalID: patID, - PrincipalType: schema.PATPrincipal, - ResourceType: schema.OrganizationNamespace, + PrincipalID: patID, + PrincipalType: schema.PATPrincipal, + ResourceType: schema.OrganizationNamespace, + RolePermissions: schema.OrganizationProjectInheritPerms, }).Return([]policy.Policy{ // grant_relation here would be pat_granted in production; // listing doesn't filter on it, so the value doesn't matter // for behavior — only the role's permissions do. {ResourceID: orgA, RoleID: roleProjectOwnerID}, }, nil) - m.role.EXPECT().List(ctx, mock.MatchedBy(func(f role.Filter) bool { - return len(f.IDs) == 1 && f.IDs[0] == roleProjectOwnerID - })).Return([]role.Role{projectOwnerRole}, nil) m.project.EXPECT().List(ctx, project.Filter{OrgIDs: []string{orgA}}).Return([]project.Project{ {ID: project1}, {ID: project2}, {ID: project3}, }, nil) @@ -2229,11 +2220,13 @@ func TestService_ListResourcesByPrincipal(t *testing.T) { }, resourceType: schema.ProjectNamespace, setup: func(m *mockSet) { - // user pass + // user pass — viewer role on org doesn't pass the inheritance + // gate, so the org-inheritance query returns [] m.policy.EXPECT().List(ctx, policy.Filter{ - PrincipalID: userID, - PrincipalType: schema.UserPrincipal, - ResourceType: schema.ProjectNamespace, + PrincipalID: userID, + PrincipalType: schema.UserPrincipal, + ResourceType: schema.ProjectNamespace, + RolePermissions: schema.ProjectDirectVisibilityPerms, }).Return([]policy.Policy{ {ResourceID: project1, RoleID: roleProjectViewerID}, }, nil) @@ -2243,17 +2236,17 @@ func TestService_ListResourcesByPrincipal(t *testing.T) { ResourceType: schema.GroupNamespace, }).Return([]policy.Policy{}, nil) m.policy.EXPECT().List(ctx, policy.Filter{ - PrincipalID: userID, - PrincipalType: schema.UserPrincipal, - ResourceType: schema.OrganizationNamespace, - }).Return([]policy.Policy{ - {ResourceID: orgA, RoleID: roleOrgViewerID}, - }, nil) + PrincipalID: userID, + PrincipalType: schema.UserPrincipal, + ResourceType: schema.OrganizationNamespace, + RolePermissions: schema.OrganizationProjectInheritPerms, + }).Return([]policy.Policy{}, nil) // PAT pass m.policy.EXPECT().List(ctx, policy.Filter{ - PrincipalID: patID, - PrincipalType: schema.PATPrincipal, - ResourceType: schema.ProjectNamespace, + PrincipalID: patID, + PrincipalType: schema.PATPrincipal, + ResourceType: schema.ProjectNamespace, + RolePermissions: schema.ProjectDirectVisibilityPerms, }).Return([]policy.Policy{ {ResourceID: project2, RoleID: roleProjectViewerID}, }, nil) @@ -2263,14 +2256,11 @@ func TestService_ListResourcesByPrincipal(t *testing.T) { ResourceType: schema.GroupNamespace, }).Return([]policy.Policy{}, nil) m.policy.EXPECT().List(ctx, policy.Filter{ - PrincipalID: patID, - PrincipalType: schema.PATPrincipal, - ResourceType: schema.OrganizationNamespace, + PrincipalID: patID, + PrincipalType: schema.PATPrincipal, + ResourceType: schema.OrganizationNamespace, + RolePermissions: schema.OrganizationProjectInheritPerms, }).Return([]policy.Policy{}, nil) - // role lookups required: both passes hit filterByRolePermissions, - // without which the empty-intersection assertion would pass - // for the wrong reason. - m.role.EXPECT().List(ctx, mock.Anything).Return([]role.Role{projectViewerRole, orgViewerRole}, nil) }, // user sees [P1], PAT sees [P2], intersection = [] want: []string{}, diff --git a/core/policy/filter.go b/core/policy/filter.go index ee7743e13..ac51bbcc4 100644 --- a/core/policy/filter.go +++ b/core/policy/filter.go @@ -11,4 +11,6 @@ type Filter struct { PrincipalID string PrincipalIDs []string ResourceType string + + RolePermissions []string } diff --git a/internal/store/postgres/policy_repository.go b/internal/store/postgres/policy_repository.go index aba751860..e7295934d 100644 --- a/internal/store/postgres/policy_repository.go +++ b/internal/store/postgres/policy_repository.go @@ -11,6 +11,7 @@ import ( "github.com/doug-martin/goqu/v9" "github.com/jmoiron/sqlx" + "github.com/lib/pq" "github.com/raystack/frontier/core/namespace" "github.com/raystack/frontier/core/policy" "github.com/raystack/frontier/internal/bootstrap/schema" @@ -126,6 +127,20 @@ func applyListFilter(stmt *goqu.SelectDataset, flt policy.Filter) *goqu.SelectDa "role_id": flt.RoleIDs, }) } + if len(flt.RolePermissions) > 0 { + // Join the roles table to keep only policies whose role grants at + // least one of the listed permission names. + stmt = stmt. + Join( + goqu.T(TABLE_ROLES).As("r"), + goqu.On(goqu.I("r.id").Eq(goqu.I("p.role_id"))), + ). + Where(goqu.Func( + "jsonb_exists_any", + goqu.I("r.permissions"), + pq.Array(flt.RolePermissions), + )) + } return stmt } From 0c055ad0bdcf07618ea6b36207b6ccf20df452bb Mon Sep 17 00:00:00 2001 From: aman Date: Tue, 19 May 2026 14:30:32 +0530 Subject: [PATCH 2/3] docs(membership): drop "why fixtures are gone" comment from test file Removed a stale-state explainer that belongs in the PR description, not the code. Co-Authored-By: Claude Opus 4.7 (1M context) --- core/membership/service_test.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/core/membership/service_test.go b/core/membership/service_test.go index 267186e8e..2c96871ce 100644 --- a/core/membership/service_test.go +++ b/core/membership/service_test.go @@ -1813,11 +1813,6 @@ func TestService_ListResourcesByPrincipal(t *testing.T) { roleProjectViewerID := uuid.New().String() roleProjectOwnerID := uuid.New().String() - // The full role.Role fixtures aren't needed anymore — the role-permission - // gate now happens at the policy filter (policy.Filter.RolePermissions), - // so the mocked policyService.List returns are already gated. We just - // need the role IDs to attach to mock policies. - type mockSet struct { policy *mocks.PolicyService role *mocks.RoleService From 5f09d3948be3fe7d19a5a909a1c66a7323303056 Mon Sep 17 00:00:00 2001 From: aman Date: Tue, 19 May 2026 14:34:57 +0530 Subject: [PATCH 3/3] docs(membership): trim stale "today's X" / "analog of Y" comments Drops a few doc lines that referenced what the new code replaces rather than what it does: - listProjectsForPrincipal: "matching today's listNonInheritedProjectIDs" - listDirectProjectIDs: "analog of what SpiceDB does today" - TestService_ListResourcesByPrincipal: outdated header that also claimed direct/group aren't role-gated (now incorrect) Co-Authored-By: Claude Opus 4.7 (1M context) --- core/membership/service.go | 7 +++---- core/membership/service_test.go | 9 ++------- 2 files changed, 5 insertions(+), 11 deletions(-) diff --git a/core/membership/service.go b/core/membership/service.go index 684c1f33a..76a4c1ba6 100644 --- a/core/membership/service.go +++ b/core/membership/service.go @@ -1676,9 +1676,9 @@ func (s *Service) narrowGroupsByOrg(ctx context.Context, ids []string, orgID str // // 1. Direct project policies — gated by schema.ProjectDirectVisibilityPerms. // 2. Group-expanded projects — same gate as direct. Runs even with -// NonInherited=true, matching today's listNonInheritedProjectIDs. +// NonInherited=true; a user can be a project member via group. // 3. Org inheritance (skipped if NonInherited=true) — gated by -// schema.OrganizationProjectInheritPerms so only org roles that actually grant +// schema.OrganizationProjectInheritPerms so only org roles that grant // project visibility (Owner, Manager, etc.) expand. Batched via // project.Filter.OrgIDs to avoid N+1 across multi-org users. func (s *Service) listProjectsForPrincipal(ctx context.Context, principalID, principalType string, filter ResourceFilter) ([]string, error) { @@ -1717,8 +1717,7 @@ func (s *Service) listProjectsForPrincipal(ctx context.Context, principalID, pri // listDirectProjectIDs returns projects the principal has a direct policy on, // kept only if the role grants any of the permissions that imply project -// visibility. This is the project-listing analog of what SpiceDB does today -// for the "get" check on a project. +// visibility. func (s *Service) listDirectProjectIDs(ctx context.Context, principalID, principalType string) ([]string, error) { policies, err := s.policyService.List(ctx, policy.Filter{ PrincipalID: principalID, diff --git a/core/membership/service_test.go b/core/membership/service_test.go index 2c96871ce..36701b484 100644 --- a/core/membership/service_test.go +++ b/core/membership/service_test.go @@ -1787,13 +1787,8 @@ func TestService_OnGroupDeleted(t *testing.T) { }) } -// TestService_ListResourcesByPrincipal exercises the policy-driven listing -// path that replaces today's SpiceDB LookupResources-based ListByUser methods. -// Table-driven, mirroring TestService_ListPrincipalsByResource above. -// -// Org-inheritance gating is keyed off schema.OrganizationProjectInheritPerms (the -// hardcoded constant whose schema-parity is enforced by inheritance_test.go). -// Direct project / group-expanded policies are not role-permission-gated. +// TestService_ListResourcesByPrincipal covers each resource type, role-based +// visibility filtering, group expansion, OrgID narrowing, and PAT intersection. func TestService_ListResourcesByPrincipal(t *testing.T) { ctx := context.Background()