fix(membership): clean custom-resource policies when removing an org member by whoAbhishekSah · Pull Request #1807 · raystack/frontier

whoAbhishekSah · 2026-07-27T05:46:33Z

Problem

When a member is removed from an organization, we delete their org, project, and group access. But we leave their access to custom resources inside that org's projects alone, in both the database and SpiceDB.

So a removed member keeps working access to resources in an org they are no longer part of. And if that user is deleted later, the leftover access row is never cleaned up, because by then they are not a member of any org.

Fix

The member removal now clears custom-resource access as well. It happens in one shared place, so every path that removes a member gets it — the API call, service user deletion, and user deletion.

The cleanup runs only after the last-owner check passes, so a rejected removal still changes nothing.

🤖 Generated with Claude Code

vercel · 2026-07-27T05:46:39Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
frontier	Ready	Preview, Comment	Jul 27, 2026 6:34am

coderabbitai · 2026-07-27T05:46:50Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 9d05ec14-96fd-42a8-912b-467e6825ebf4

📥 Commits

Reviewing files that changed from the base of the PR and between 061e811 and 0639730.

📒 Files selected for processing (7)

cmd/serve.go
core/deleter/mocks/resource_service.go
core/deleter/service.go
core/deleter/service_test.go
core/membership/mocks/resource_service.go
core/membership/service.go
core/membership/service_test.go

💤 Files with no reviewable changes (1)

core/deleter/mocks/resource_service.go

🚧 Files skipped from review as they are similar to previous changes (5)

core/membership/mocks/resource_service.go
cmd/serve.go
core/membership/service_test.go
core/membership/service.go
core/deleter/service.go

📝 Walkthrough

Summary by CodeRabbit

New Features
- Removing organization members now also cleans up their custom resource access across associated projects.
- Cleanup runs across multiple projects and aggregates failures while continuing the removal flow.
Bug Fixes
- Member removal no longer relies on separate project-level principal-access cleanup; cleanup is handled by the membership cascade.
- If cleanup fails for one user, removal continues for subsequent users and returns the first failure with partial results.
- Custom-resource cleanup remains optional and won’t block member removal when not available.

Walkthrough

Organization member-removal cascades now clean custom-resource policies through an injected resource service. Runtime wiring supplies that dependency, while the deleter no longer performs separate resource cleanup.

Changes

Membership resource-policy cleanup

Layer / File(s)	Summary
Membership cascade cleanup `core/membership/service.go`, `core/membership/service_test.go`, `core/membership/mocks/resource_service.go`	Membership removal collects organization project IDs, removes custom-resource access, joins cleanup errors, and covers configured, failed, guarded, and unwired scenarios.
Runtime dependency wiring `cmd/serve.go`	`buildAPIDependencies` injects `resourceService` into `membershipService`.
Deleter delegation update `core/deleter/service.go`, `core/deleter/service_test.go`, `core/deleter/mocks/resource_service.go`	`RemoveUsersFromOrg` delegates membership removal without direct resource cleanup, and the obsolete deleter mock method and expectations are removed.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

raystack/frontier#1800: Adds the resource principal-access cleanup path used by this membership cascade.
raystack/frontier#1696: Refactors organization user deletion to delegate to membership.ForceRemoveOrganizationMember.
raystack/frontier#1766: Modifies related resource-service dependency wiring in cmd/serve.go.

Suggested reviewers: rohilsurana

🚥 Pre-merge checks | ✅ 2

✅ Passed checks (2 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The PR implements `#1801` by removing custom-resource access during organization member removal via membership cascade cleanup.
Out of Scope Changes check	✅ Passed	The changes are scoped to org-member removal cleanup, service wiring, and related tests/mocks with no unrelated behavior added.

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

coveralls · 2026-07-27T05:52:47Z

Coverage Report for CI Build 30243145128

Coverage increased (+0.02%) to 46.909%

Details

Coverage increased (+0.02%) from the base build.
Patch coverage: 1 uncovered change across 1 file (20 of 21 lines covered, 95.24%).
No coverage regressions found.

Uncovered Changes

File	Changed	Covered	%
cmd/serve.go	1	0	0.0%
Total (2 files)	21	20	95.24%

Coverage Regressions

No coverage regressions found.

Coverage Stats


Relevant Lines:	39003
Covered Lines:	18296
Line Coverage:	46.91%
Coverage Strength:	14.36 hits per line

💛 - Coveralls

whoAbhishekSah · 2026-07-27T06:31:00Z

Tested end to end on a local server

Drove the real RPCs against a running Frontier with Postgres and SpiceDB, and checked all three places the state lives: the policies table, the SpiceDB tuples, and a live permission check.

Setup

Custom resource type widget/item, with permissions manage, get, delete.
Custom org role widget-admin holding app.organization.update + widget.item.manage + widget.item.get.
Org A with two projects: res1 in project 1, res2 in project 2. Two projects on purpose, so a fix that only cleaned one would show up.
Org B with one project holding res3. This is the control for an unrelated org.
Four users, logged in over mail OTP: alice (owner), bob, charlie, dave.

Grants before the removal:

Who	Org A membership	Custom-resource grants
bob	org viewer	`res1`, `res2` (Org A) and `res3` (Org B)
charlie	org viewer	`res1`
dave	org viewer	`res1`
alice	owner	`res1`

What we did

Called RemoveOrganizationMember(Org A, bob, app/user) as alice. Returned 200.

What we got

bob's grant	Before	After
Org A org-viewer policy	present	deleted
`res1` — project 1, Org A	present	deleted
`res2` — project 2, Org A	present	deleted
`res3` — Org B	present	kept

✅ Both of bob's custom-resource policies inside Org A are gone — the policy row and the granted tuple on the resource.
✅ res2 proves the cleanup covers every project in the org, not just the first one.
✅ res3 kept. The cleanup does not reach into another org.
✅ charlie's and dave's grants on res1 kept. It filters by person, not by resource.
✅ No orphan tuples. The bearer and role tuples on bob's rolebindings are gone too, so nothing dangles in SpiceDB.
✅ Rows are really deleted, not soft deleted — deleted_at is not set on anything, and the rows are absent. That covers the second half of RemoveOrganizationMember leaves the member's custom-resource policies behind #1801, where deleting the user later orphaned the policy row.
✅ CheckResourcePermission for widget.item.manage agrees: bob went from allow to deny on res1 and res2, and still gets allow on res3. charlie and dave unchanged.

Last-owner guard

Gave alice a grant on res1, then tried to remove her as the only owner of Org A.

✅ Rejected with failed_precondition: this is the last owner of the organization, and none of her four policies changed, including the one on the custom resource. So the new call really does sit after the guard, and a refused removal has no side effects.

Unit tests

✅ core/membership, core/deleter, core/resource all pass.

One thing found on the side, not from this change

Creating a policy on a custom resource whose namespace has no delete permission fails with a bare 500. The real cause only shows up in the server log:

relation/permission `delete` not found under definition `widget/item`

The authz check on CreatePolicy asks for delete on the target. Adding the permission fixes it. Worth a separate issue — the caller gets no hint about which permission is missing.

…member RemoveOrganizationMember deleted a member's org, project, and group policies but left their policies on custom resources in the org's projects untouched. The policy row and its SpiceDB rolebinding tuple stayed live, so an ex-member kept access to resources inside an org they had been removed from. Deleting that user later orphaned the policy row. The cascade now calls resource.RemovePrincipalAccess with the org's project IDs, after the last-owner guard so a rejected removal changes nothing. The deleter reaches the same cascade through ForceRemoveOrganizationMember, so its own call is no longer needed. Fixes #1801 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

vercel Bot deployed to Preview July 27, 2026 05:46 View deployment

whoAbhishekSah force-pushed the fix/org-member-removal-resource-policies branch from 061e811 to 0639730 Compare July 27, 2026 06:33

vercel Bot deployed to Preview July 27, 2026 06:34 View deployment

whoAbhishekSah requested a review from rohilsurana July 27, 2026 06:35

rohilsurana approved these changes Jul 28, 2026

View reviewed changes

whoAbhishekSah merged commit 1713cdd into main Jul 28, 2026
8 checks passed

whoAbhishekSah deleted the fix/org-member-removal-resource-policies branch July 28, 2026 05:41

coderabbitai Bot mentioned this pull request Jul 31, 2026

refactor(membership): deduplicate repeated patterns #1827

Merged

Navigation Menu

Uh oh!

fix(membership): clean custom-resource policies when removing an org member - #1807

fix(membership): clean custom-resource policies when removing an org member#1807
whoAbhishekSah merged 1 commit into
mainfrom
fix/org-member-removal-resource-policies

whoAbhishekSah commented Jul 27, 2026 •

edited

Loading

Uh oh!

vercel Bot commented Jul 27, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Jul 27, 2026 •

edited

Loading

Summary by CodeRabbit

Walkthrough

Changes

Uh oh!

coveralls commented Jul 27, 2026 •

edited

Loading

Uh oh!

whoAbhishekSah commented Jul 27, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

whoAbhishekSah commented Jul 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Fix

Uh oh!

vercel Bot commented Jul 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot commented Jul 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Walkthrough

Changes

Uh oh!

coveralls commented Jul 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Coverage Report for CI Build 30243145128

Coverage increased (+0.02%) to 46.909%

Details

Uncovered Changes

Coverage Regressions

Coverage Stats

💛 - Coveralls

Uh oh!

whoAbhishekSah commented Jul 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Tested end to end on a local server

Setup

What we did

What we got

Last-owner guard

Unit tests

One thing found on the side, not from this change

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

whoAbhishekSah commented Jul 27, 2026 •

edited

Loading

vercel Bot commented Jul 27, 2026 •

edited

Loading

coderabbitai Bot commented Jul 27, 2026 •

edited

Loading

coveralls commented Jul 27, 2026 •

edited

Loading

whoAbhishekSah commented Jul 27, 2026 •

edited

Loading