refactor(policy): add Filter.RolePermissions, use it in membership listing#1623
Conversation
…sting 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) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Removed a stale-state explainer that belongs in the PR description, not the code. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Coverage Report for CI Build 26087403040Coverage decreased (-0.04%) to 42.472%Details
Uncovered Changes
Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
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) <noreply@anthropic.com>
E2E test —
|
| # | Scenario | Expected | Got | Status |
|---|---|---|---|---|
| 1 | Org listing — Org Owner policy returns the org | [OrgA] |
[OrgA] |
PASS |
| 2 | Org listing — Org Viewer returns the org (no role gate) | [OrgA] |
[OrgA] |
PASS |
| 3 | Org listing — billing-only role returns the org (no role gate) | [OrgA] |
[OrgA] |
PASS |
| 4 | Org listing — multi-org user returns both orgs | [OrgA, OrgB] |
[OrgA, OrgB] |
PASS |
| 5 | Org listing — no policies → empty | [] |
[] |
PASS |
| 6 | Org listing — service user with org policy returns the org | [OrgA] |
[OrgA] |
PASS |
| 7 | Group listing — group member returns G1 | [G1] |
[G1] |
PASS |
| 8 | Group listing — non-member → empty | [] |
[] |
PASS |
| 9 | Project (direct) — ProjViewer role returns P1 | [P1] |
[P1] |
PASS |
| 10 | Project (direct) — role without visibility perms gated out → empty | [] |
[] |
PASS |
| 11 | Project (group) — G1 with ProjViewer on P3 → P3 | [P3] |
[P3] |
PASS |
| 12 | Project (group) — G2 with weak role on P4 → empty | [] |
[] |
PASS |
| 13 | Project (inherit) — Org Owner sees all 5 OrgA projects | [P1, P2, P3, P4, P5] |
same | PASS |
| 14 | Project (inherit) — Org Viewer fails gate → empty | [] |
[] |
PASS |
| 15 | Project (inherit) — billing-only fails gate → empty | [] |
[] |
PASS |
| 16 | Project (inherit) — empty-permissions role excluded by JOIN | [] |
[] |
PASS |
| 17 | Project (inherit) — multi-org Owner: 5 OrgA + 1 OrgB | [P1, P2, P3, P4, P5, P6] |
same | PASS |
| 18 | Project — OrgID filter narrows multi-org Owner to OrgA | [P1, P2, P3, P4, P5] |
same | PASS |
| 19 | Project — NonInherited=true + Owner-only → empty |
[] |
[] |
PASS |
| 20 | Project — NonInherited=true keeps direct policy |
[P1] |
[P1] |
PASS |
| 21 | Project — dedup: Owner + direct P1 → [P1..P5] once |
[P1, P2, P3, P4, P5] |
same | PASS |
| 22 | Project — no policies → empty | [] |
[] |
PASS |
| 23 | PAT — specific-project PAT clamps user's wider visibility → [P2] |
[P2] |
[P2] |
PASS |
| 24 | PAT — all-projects PAT matches user's full visibility → [P1..P5] |
[P1, P2, P3, P4, P5] |
same | PASS |
| 25 | PAT — zero-visibility PAT → empty | [] |
[] |
PASS |
Coverage
| Dimension | Tested |
|---|---|
| Resource types | app/organization, app/project, app/group |
| Principal types | app/user, app/serviceuser, app/pat |
| Project branches | direct, group-expanded, org-inheritance, combined |
| Role gate states | passes, fails (no overlap), fails (empty perms) |
| Filter modifiers | OrgID, NonInherited, default |
| PAT semantics | none, specific-project, all-projects, zero-scope |
e4fca7a
into
feat/membership-list-resources-by-principal
Summary
Pushes the "policies whose role grants any of these permissions" filter into the policy layer. Membership stops doing the local list-then-filter dance for project listing.
Stacked on #1618 — review the diff against that branch. Will rebase onto
mainonce #1618 merges.Change
policy.Filtergains:The repo implements it by joining the roles table and using Postgres
jsonb_exists_anyto check overlap withrole.permissions. Generated SQL (prepared mode):Membership call sites
Before:
After:
Same for
listGroupExpandedProjectIDsandlistOrgInheritedProjectIDs. ThefilterByRolePermissionshelper is removed (no remaining callers).What doesn't change
listOrgsForPrincipal) — doesn't setRolePermissions, behavior identical.listGroupsForPrincipal) — same.schema.go(ProjectDirectVisibilityPerms,OrganizationProjectInheritPerms) — unchanged; callers just pass them viapolicy.Filternow.TestService_ListResourcesByPrincipalcases still pass; mock expectations updated for the new filter shape.Files changed
core/policy/filter.go—RolePermissions []stringfieldinternal/store/postgres/policy_repository.go— JOIN +jsonb_exists_anybranch inapplyListFilter, built withgoqu.Funccore/membership/service.go— three helpers useRolePermissions;filterByRolePermissionsremovedcore/membership/service_test.go— mock expectations updated; obsolete role fixtures removedTest plan
make buildmake lint(0 issues)make test -raceon touched packages green (core/membership,core/policy,internal/store/postgres).agents/backend/sql-safety.mdfollowed: no?in single-quoted strings, nogoqu.L, identifiers viagoqu.I/goqu.T, prepared-mode-clean (all values bound as$N)458a3e94...→ Org Owner policy on PixxelSpace → 175 inherited projects)🤖 Generated with Claude Code