[AmanGIT07]

Summary

Removing a user from an organization now also revokes the PATs they created for that organization. Closes #1970.

Changes

• New userpat.Service.OnOrgMemberRemoved revokes every PAT a user holds in an organization. Each PAT is marked deleted and its policies are removed. Its pat.revoked audit record carries reason: member_removed in the target metadata.
• It reads the user's PATs 1000 at a time and reads every page before revoking any.
• revokeOrgPAT is renamed revokePAT and takes the audit target metadata. Organization delete passes none, so its records do not change.
• RemoveOrganizationMember calls it for users, after the member's own policies are removed. It also calls it when the user is no longer a member, then returns not-a-member. The membership UserPATService interface and its mock change with it.
• User delete does not call it. It removes the user from each organization through RemoveOrganizationMemberOnDelete, then revokes all of the user's PATs once, as before.

Technical Details

• Expired PATs are revoked too. They keep their policies and can be regenerated, and they do not count toward the per-organization limit.
• 1000 is far above the per-organization limit, so one read covers almost every user.
• The audit record's actor is whoever removed the member.
• Each PAT goes through the same revoke step as organization delete. A PAT its owner deletes at the same moment is skipped, with no error and no second audit record.
• If a revoke fails, the request returns an error after the member is already removed. Removing the user again returns not-a-member and revokes the remaining PATs.
• Removing a user who is no longer a member also clears PATs left by removals before this change.
• After the removal the user has no get on the organization, so they cannot create a new PAT there. Only a PAT created at the same moment as the removal can be missed, and removing the user again revokes it.
• If the PAT service is not wired, removal logs an error and goes on, like the resource service check above it.

Test Plan

• go test ./core/userpat/ passes. New TestService_OnOrgMemberRemoved: each PAT is revoked with the reason in its audit record; with more than 1000 PATs it reads every page before revoking any; it stops at the first PAT it cannot revoke. Organization delete records carry no reason.
• go test ./core/membership/ passes. Removing a user revokes their PATs. Removing a user who is no longer a member still revokes their PATs and returns not-a-member, and a failed revoke there returns the error. A non-user principal and user delete never reach the PAT step. A missing PAT service still removes the member.
• e2e TestPATOfRemovedOrgMemberIsRevoked: an organization owner (not a superuser) removes a member who has an org-scoped PAT and a project-scoped PAT. For both tokens: the token is rejected, hidden from its owner, holds no policy, and is gone from the organization's token list. Each pat.revoked record names the owner and reason: member_removed. After the member is added back with the same access, neither token works or has access in SpiceDB. Without the PAT step, every check fails for both tokens.
• go test ./test/e2e/... passes.
• make test passes.
• golangci-lint run reports no issues.

[vercel]

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

Project	Deployment	Actions	Updated
frontier	Ready	Preview	Nov 11, 2026 3:04pm UTC

[coderabbitai]

Summary

Summary by CodeRabbit

• Bug Fixes
  • Removing a user from an organization now revokes their personal access tokens, including tokens with organization or project access, so they can no longer use them to access resources.
  • Revoked tokens stay revoked if the user is later added back to the organization; the user must create new tokens to regain token-based access.

Removing a user from an organization now triggers revocation of that user’s organization-scoped PATs and their policies. Revocation records include member_removed metadata. Tests cover service behavior, membership removal, and the end-to-end flow.

Changes: Organization member PAT cleanup

Layer / File(s)	Summary
PAT revocation contract and implementation core/membership/service.go, core/userpat/service.go, core/userpat/service_test.go	UserPATService adds OnOrgMemberRemoved. The PAT service lists the user’s organization PATs in pages of 1,000 and revokes them with member_removed metadata. DeleteAllByOrg uses the shared revocation path. Unit tests cover pagination, failures, and audit metadata.
Membership removal integration core/membership/org.go, core/membership/mocks/user_pat_service.go, core/membership/org_test.go	Member removal invokes PAT cleanup for user principals. If the PAT service is unavailable, removal logs the condition and continues. A revocation error is returned. Tests cover these cases and skip revocation for PAT principals.
End-to-end removal regression test/e2e/regression/pat_test.go	The regression test verifies that member removal revokes organization- and project-scoped PATs, removes their policies and organization-list entries, and records audit events. Re-adding the member does not restore either token.

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

Change: Bug fix · Severity of issue fixed: Medium

Possibly related PRs

• raystack/frontier#2031: Changes user deletion to use RemoveOrganizationMemberOnDelete, a separate path for membership removal during user deletion.
• raystack/frontier#2027: Adds PAT cleanup to user deletion while removing the user from organizations.

Suggested reviewers: rohilsurana

Merge Risk: Low · up to ddfe0

Removing an organization member now revokes that user's organization PATs. If policy cleanup fails partway through, a retry may not remove the leftover policies. The window is narrow and the cleanup can happen separately, so this is mergeable with owner awareness.

Pre-merge checks: 2

Check name	Status	Explanation
Linked Issues check		Issue #1970 requires removal of a user’s organization PATs and their policies when membership ends. RemoveOrganizationMember calls OnOrgMemberRemoved after member-policy removal. The service reads…
Out of Scope Changes check		The membership interface and mock support the new callback. The audit metadata parameter preserves organization-delete behavior. Unit and e2e tests verify the required revocation and regression behavi…

Full details: Linked Issues check

Explanation

Issue #1970 requires removal of a user’s organization PATs and their policies when membership ends. RemoveOrganizationMember calls OnOrgMemberRemoved after member-policy removal. The service reads all PAT pages before revoking them, marks each PAT deleted, removes its policies, and records reason: member_removed. The tests cover pagination, failures, non-user principals, and re-add behavior.

Full details: Out of Scope Changes check

Explanation

The membership interface and mock support the new callback. The audit metadata parameter preserves organization-delete behavior. Unit and e2e tests verify the required revocation and regression behavior. No unrelated change is evident.

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

• X
• Mastodon
• Reddit
• LinkedIn

[coderabbitai]

Actionable comments posted: 1

Review info

Run configuration

• Source mode: Auto
• Repository: raystack/frontier
• Branch: main
• File: .coderabbit.yaml
• Commit: f38ac11
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: 47f00a98-b160-4c3e-ac23-50d375215d08

Commits

Reviewing files that changed from the base of the PR and between f38ac11 and 7266892.

Files selected for processing: 7

• core/membership/mocks/user_pat_service.go
• core/membership/org.go
• core/membership/org_test.go
• core/membership/service.go
• core/userpat/service.go
• core/userpat/service_test.go
• test/e2e/regression/pat_test.go

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.

[coveralls]

Coverage Report for CI Build 43044648206

Warning

Build has drifted: This PR's base is out of sync with its target branch, so coverage data may include unrelated changes.
Quick fix: rebase this PR. Learn more →

Coverage increased (+0.04%) to 56.097%

Details

• Coverage increased (+0.04%) from the base build.
• Patch coverage: 48 of 48 lines across 2 files are fully covered (100%).
• No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.

---

Coverage Stats

Relevant Lines:	42232
Covered Lines:	23691
Line Coverage:	56.1%
Coverage Strength:	21.71 hits per line

---

💛 - Coveralls

[rohilsurana]

Looks good. The fix matches #1970. The PAT revoke reuses the same step as org delete. The e2e test covers the re-add case, which was the real risk. One thing I would fix before merge: a failed revoke cannot be retried today. Details inline.

Two questions that are outside this PR:

• DeletePolicy can remove a user's last org policy. That also ends their membership, but their PATs stay. Should that path revoke PATs too, or should PAT auth check that the owner is still a member?
• A PAT created at the same moment as the removal can survive. You already call this out. Is a follow up ticket worth it, or is the window small enough to leave?

[rohilsurana]

I agree with CodeRabbit here. If the revoke fails, the member is already gone. A retry then stops at ErrNotMember on line 247, so the PATs never get revoked. If the user is added back later, those PATs work again, which is the bug this PR fixes. Could we run revokeMemberPATs when the error is ErrNotMember too, and still return ErrNotMember to the caller? It is a no-op when there are no PATs. A unit test for the retry would be good as well.

[AmanGIT07]

Done in c0aec7b. RemoveOrganizationMember now runs the PAT step on ErrNotMember too and still returns ErrNotMember. Removing the user again finishes a revoke that failed, and it also clears PATs left by removals before this change. Two unit cases cover it: the retry, and a retry whose revoke fails.

[rohilsurana]

Small one: this adds a new sentence to a code comment. We are trying not to add new code comments. Could the "leaves PATs to the user delete" note move to the PR description instead?

[AmanGIT07]

Keeping this line. The code here can't show it: user delete revokes all of the user's PATs itself, in core/deleter (DeleteAllByUser). Without the line, the missing PAT step next to RemoveOrganizationMember reads like a bug.

[rohilsurana]

Optional: a second PAT scoped to a project in the same org would show that project policies are removed too.

[AmanGIT07]

Added in ddfe0f4. The member now holds a second PAT scoped to a project in the org, and every check runs for both. Without the PAT step, every check fails for both PATs.

[AmanGIT07]

On the two questions in the review:

1. DeletePolicy ending membership: agreed, the PATs stay there too. It's outside this PR and will be handled separately.
2. A PAT created at the same moment as the removal: removing the user again now revokes it (c0aec7b), and the window is small, so I'd leave it.

[coderabbitai]

Actionable comments posted: 1

Nitpick comments: 1

test/e2e/regression/pat_test.go: 1

918-919: Trivial · Maintainability & Code Quality · Quick win

Add PAT preservation controls to this regression.

Both PATs belong to the removed member in the same organization. This test also passes if cleanup incorrectly revokes every PAT in the organization or every PAT owned by the member.

Add another member’s PAT in this organization and this member’s PAT in another organization. After removal, assert that both control PATs still authenticate and retain their policies.

Based on learnings, filtered-deletion tests should include similar-but-distinct records and assert that they are preserved.

Source: Learnings

Review info

Run configuration

• Source mode: Auto
• Repository: raystack/frontier
• Branch: main
• File: .coderabbit.yaml
• Commit: f38ac11
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: e00ff19d-4946-404f-9ca5-b1549e05f38e

Commits

Reviewing files that changed from the base of the PR and between 7266892 and ddfe0f4.

Files selected for processing: 3

• core/membership/org.go
• core/membership/org_test.go
• test/e2e/regression/pat_test.go

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.

[coderabbitai]

Minor · Data Integrity & Integration · Quick win

Supported by static analysis

Script executed:

# Locate PAT repository implementations and inspect deletion filters.
fd -i 'pat.*\.go$' . |
while IFS= read -r file; do
  rg -n -C 6 'func .*List\(|deleted_at|DeletedAt|IncludeDeleted|include_deleted' "$file"
done

# Inspect the policy-cleanup entry point.
rg -n -C 12 --glob '*.go' 'func .*RemoveAllPATPolicies\(' .

Repository: raystack/frontier

Length of output: 19075

---

Script executed:

git diff f38ac119be14d4fda11f874ecfad9b2c4d9b65e8 ddfe0f438081e83b4bad4a9a5d735919af7da49c -- core/membership/org.go core/userpat/service.go
rg -n -F -e 'listAllByUserInOrg' -e 'OnOrgMemberRemoved' -e 'RemoveAllPATPolicies' -e 'ListAllByOrg' -- core/userpat core/membership internal | grep -v '/mocks/'
rg -n -F 'type UserPATRepository struct' -- .
cat -n core/membership/org.go | sed -n '175,260p'
cat -n core/userpat/service.go | sed -n '1,180p'
cat -n core/userpat/service.go | sed -n '200,420p'

Repository: raystack/frontier

Length of output: 32815

---

Script executed:

cat -n internal/store/postgres/userpat_repository.go | sed -n '25,45p;123,216p;414,460p;483,515p'
cat -n core/membership/service.go | sed -n '130,205p'
cat -n core/membership/org.go | sed -n '399,418p'
rg -n -F -e 'NewUserPATRepository(' -e 'userpat.NewService(' -e 'func (s *Service) removePoliciesByFilter' -- internal core
rg -n -F 'func (s *Service) Delete(' -- core/policy

Repository: raystack/frontier

Length of output: 26172

---

Script executed:

rg -n -F -e 'NewUserPATRepository' -e 'userpat.NewService' -e 'SetUserPATService' --glob '!**/*test.go' --glob '!**/mocks/**' .

Repository: raystack/frontier

Length of output: 795

---

Allow retries to finish PAT policy cleanup.

If policy cleanup fails after revokePAT soft-deletes a PAT, the next member-removal attempt skips that PAT because UserPATRepository.List excludes deleted rows. Its policies can remain until a separate cleanup occurs.

Make pending cleanup reachable on retry without changing normal PAT listings.