Skip to content

feat(teams): per-member permission overrides - #19

Merged
kipavy merged 19 commits into
mainfrom
feat/per-member-permission-overrides
Sep 12, 2026
Merged

kipavy merged 19 commits into
mainfrom
feat/per-member-permission-overrides

Conversation

@kipavy

@kipavy kipavy commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Adds a per-member permission overlay on top of role permissions, so granting one person a single extra capability — or taking one away — no longer requires minting a bespoke role for them.

Model

Each member gains two bitmasks in a new team_member_permission_overrides table, resolving as:

effective = (roleUnion | allow) & ~deny

Deny is applied last and always wins: a bit set in both masks resolves to denied, and an explicit deny cannot be undone by assigning another role later.

What is here

  • Migration 042team_member_permission_overrides, PK (team_id, user_id), cascading off team_members so a removed member's overrides go with them.
  • Both resolvers apply the overlay. The duplicated bit_or query was collapsed into one shared fragment first, so the overlay is written once.
  • Multi-team checks resolve per team. has_any_team_permission previously folded every team into a single bit_or; applying the overlay to that would have let a deny in team A erase a grant from team B. It now groups by team and reduces with bool_or.
  • GET /v1/teams and GET /v1/teams/:id/members serve permission_allow / permission_deny. Additive — older clients ignore them, and a newer client reads absent as 0.
  • PUT /v1/teams/:team_id/members/:user_id/permissions writes them, with validation (bits outside ALL_PERMISSIONS, negative masks, and a bit in both masks all 400) and a member.permissions_changed audit row recording both masks before and after.
  • Four guardrails on that endpoint, after the existing MANAGE_MEMBERS gate: no self-edit; no granting or clearing a bit you do not hold yourself; owner immunity; and the caller's strongest role position must outrank the target's.
  • Key rotation on revoked read access. Every member holds the team DEK wrapped to their key, so a deny stops future API reads but not access to already-synced ciphertext. When a change drops a member from holding one of CONNECT/VIEW_SECRETS to holding neither — the Any gate on get_my_vault_key — a rotation request is queued, atomically with the permission write.

Two fixes to existing behaviour

Removed members kept permissions through orphaned role rows. Both resolvers previously joined from team_member_roles, which has no FK to team_members, and remove_member never deleted those rows. Re-anchoring the joins to team_members closes that. remove_member now also deletes the member's role rows.

⚠️ Deploy note: anyone currently exercising permissions through orphaned rows loses them on deploy. Count them first:

SELECT count(*) FROM team_member_roles tmr
WHERE NOT EXISTS (SELECT 1 FROM team_members tm
                  WHERE tm.team_id = tmr.team_id AND tm.user_id = tmr.user_id);

⚠️ Behaviour change: a re-invited member now starts from the role their invitation grants, rather than silently regaining their previous roles. This is what closes the remove-and-re-invite path that otherwise laundered an owner-set deny.

Vault session visibility ignored permission bits' overlay. visible_sessions computed VIEW_TERMINAL_SESSIONS directly off team_roles.permissions. A deny did nothing — the member still saw every vault session's id, connection name, host and vault ids, while joining was blocked, so it looked like the deny worked. It now goes through the overlay.

Known limitation, not addressed here

assign_member_role carries none of these four guardrails and can assign the builtin owner role, including to yourself. Any member with MANAGE_MEMBERS can therefore take everything this endpoint withholds. That is pre-existing and unchanged by this PR, but it means the threat model above does not fully hold until role assignment is guarded too — worth its own change, now that override_guardrails exists to reuse.

Verification

386 tests pass. cargo clippy --all-targets -- -D warnings is clean.

Migration 042 is safe against a live database: new table only, no ALTER, no backfill, and a brief SHARE ROW EXCLUSIVE on team_members that validates instantly against an empty table. Rollback is DROP TABLE; the old binary simply ignores the table.

The client half is a separate PR and depends on this being deployed first.

…overlay

visible_sessions checked PERM_VIEW_TERMINAL_SESSIONS straight off
team_roles.permissions, so an override's deny did nothing and its allow
excluded roleless members outright. Replace the inner EXISTS with the
same (roleUnion | allow) & ~deny expression the resolvers use.
team_member_roles has no FK to team_members, so remove_member left role
rows behind; re-inviting restored full role permissions with any deny
override gone. Delete the pair's role rows in the same transaction.
…ust new denies

newly_denied only tracked the deny mask growing, so clearing an allow
that was a roleless member's only read-class access queued no rotation
at all. Compare effective access before/after instead, and narrow the
trigger to the bits get_my_vault_key's CONNECT_OR_VIEW_SECRETS gate
actually keys on (VIEW_SECRETS, CONNECT) so rotation isn't queued for
denies it can't remediate (e.g. COPY_SECRETS alone).

Updated the existing rotation test to give its target a role granting
VIEW_SECRETS, so the deny it exercises is a genuine loss under the new
comparison.
A target's authority can come entirely from an allow override, so the
old check — constraining only bits being granted — let any role-holder
with MANAGE_MEMBERS strip an allow-granted capability they never held
themselves. Require the actor to also hold every bit they are removing.
…rapper

It differed from the private effective_permissions only in visibility.
Make effective_permissions itself pub and update its one call site.
MIN() over zero rows is NULL, not an obvious fact from the match arm alone.
The trigger tested bits lost from the read-class mask, but get_my_vault_key
gates on Any([CONNECT, VIEW_SECRETS]). Denying VIEW_SECRETS while CONNECT
remained therefore queued a rotation the member could still defeat by
fetching the new epoch's key, nagging every key-holder client for nothing.
@kipavy
kipavy merged commit 3173dbe into main Sep 12, 2026
2 checks passed
@kipavy
kipavy deleted the feat/per-member-permission-overrides branch September 14, 2026 22:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant