-
Notifications
You must be signed in to change notification settings - Fork 44
Description
Problem
When users are added to organizations or groups, we create both a policy (role binding) and a direct SpiceDB relation. The direct relations are redundant and cause bugs - for example, an org creator demoted from owner to member retains owner permissions because their direct owner relation persists.
Current State vs Proposed
| Resource | Action | Role (via policy) | Direct Relation (current) | Direct Relation (proposed) |
|---|---|---|---|---|
| Organization | Add user as owner | Owner (app_organization_owner) |
organization#owner@user |
None |
| Organization | Add user as admin | Admin (app_organization_manager) |
organization#member@user |
None |
| Organization | Add user as member | Member (app_organization_viewer) |
organization#member@user |
None |
| Organization | Add serviceuser | (same as user) | organization#member@serviceuser |
None |
| Organization | Add group | - | organization#member@group#member |
Keep |
| Group | Add user as owner | Team Owner (app_group_owner) |
group#owner@user |
None |
| Group | Add user as member | Team Member (app_group_member) |
group#member@user |
Keep |
| Project | Add user as owner | Project Owner (app_project_owner) |
None | None |
| Project | Add user as manager | Project Manager (app_project_manager) |
None | None |
| Project | Add user as viewer | Project Viewer (app_project_viewer) |
None | None |
Project is the clean model - only policies, no direct relations. We want org and group to follow this pattern.
Why keep these relations?
| Relation | Reason |
|---|---|
organization#member@group#member |
Gives group members automatic org visibility |
group#member@user |
Needed for SpiceDB to resolve group-level policies |
Impact on Project Inherited Permissions
Projects inherit permissions from organizations via the org relation:
// Project permission flows through org
permission delete = org->project_delete + granted->app_project_administer + ...
// Org permission currently includes + owner
permission project_delete = ... + granted->app_organization_administer + owner
After removing + owner from org permissions:
The app_organization_owner role includes app_organization_administer permission, so the inheritance still works via the policy path:
project#delete
→ org#project_delete
→ granted->app_organization_administer ✅ (owner role has this)
| User State | Current Access | After Change |
|---|---|---|
| Has policy + relation | ✅ via both paths | ✅ via policy path |
| Has only policy | ✅ via policy | ✅ via policy |
| Has only relation (bug state) | ✅ via relation | ❌ loses access |
The third case (only relation, no policy) is the bug we're fixing. Users in this state shouldn't exist, and losing access is the correct behavior.
Benefits
| Benefit | Description |
|---|---|
| Fixes permission bug | Role changes via SetOrganizationMemberRole will actually work - no more stale permissions from direct relations |
| Code simplicity | Remove ~20 lines of relation management code per add member operation |
| SDK simplicity | SDK only deals with role-based operations (assign role, change role, remove role). Policies and relations are internal constructs - SDK consumers don't need to know about them |
| Consistency | Org and group will follow the same pattern as project (which already works correctly) |
| Single source of truth | Permissions only come from policies - easier to debug and audit |
| Cleaner user listing | Already uses policies, so no change needed there |
Potential Downsides
| Concern | Impact | Mitigation |
|---|---|---|
| Permission check latency | Direct relation check is O(1), role binding path requires graph traversal | SpiceDB caches these lookups |
| Migration complexity | Need to update schema before code, then clean up existing relations | Can be done in phases; existing relations become inert after schema change |
| Breaking change | External systems relying on direct SpiceDB relations will break | Document in release notes; these relations were internal implementation detail |
Performance Comparison
Current permission check (e.g., "can alice delete org acme?"):
Fast path: organization:acme#owner@user:alice → YES (direct lookup)
Slow path: organization:acme#granted → rolebinding → bearer/role check
Proposed permission check:
Only path: organization:acme#granted → rolebinding → bearer/role check
Schema Changes
Organization
// Before
relation member: app/user | app/group#member | app/serviceuser
relation owner: app/user | app/serviceuser
permission delete = ... + owner
permission get = ... + owner + member
// After
relation member: app/group#member
// owner removed
permission delete = ...
permission get = ... + member
Group
// Before
relation member: app/user
relation owner: app/user | app/serviceuser
permission delete = ... + owner
permission get = ... + member + owner
// After
relation member: app/user // keep for policy resolution
// owner removed
permission delete = ...
permission get = ... + member
Code Changes
core/organization/service.go-AddMember: removerelationService.Createcallcore/serviceuser/service.go- remove direct member relation on orgcore/group/service.go-addOwner: removerelationService.Createcall for owner relation
Future Improvement
See #1479 - Use computed permissions for group membership instead of direct relations. This would eliminate the need for group#member@user relations entirely by computing group membership from policies, making groups fully consistent with org and project.