feat(teams): per-member permission overrides - #19
Merged
Merged
Conversation
…nt scope and role position
…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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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_overridestable, resolving as: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
team_member_permission_overrides, PK(team_id, user_id), cascading offteam_membersso a removed member's overrides go with them.bit_orquery was collapsed into one shared fragment first, so the overlay is written once.has_any_team_permissionpreviously folded every team into a singlebit_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 withbool_or.GET /v1/teamsandGET /v1/teams/:id/membersservepermission_allow/permission_deny. Additive — older clients ignore them, and a newer client reads absent as 0.PUT /v1/teams/:team_id/members/:user_id/permissionswrites them, with validation (bits outsideALL_PERMISSIONS, negative masks, and a bit in both masks all 400) and amember.permissions_changedaudit row recording both masks before and after.MANAGE_MEMBERSgate: 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.CONNECT/VIEW_SECRETSto holding neither — theAnygate onget_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 toteam_members, andremove_membernever deleted those rows. Re-anchoring the joins toteam_memberscloses that.remove_membernow also deletes the member's role rows.Vault session visibility ignored permission bits' overlay.
visible_sessionscomputedVIEW_TERMINAL_SESSIONSdirectly offteam_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_rolecarries none of these four guardrails and can assign the builtinownerrole, including to yourself. Any member withMANAGE_MEMBERScan 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 thatoverride_guardrailsexists to reuse.Verification
386 tests pass.
cargo clippy --all-targets -- -D warningsis clean.Migration 042 is safe against a live database: new table only, no
ALTER, no backfill, and a briefSHARE ROW EXCLUSIVEonteam_membersthat validates instantly against an empty table. Rollback isDROP TABLE; the old binary simply ignores the table.The client half is a separate PR and depends on this being deployed first.