feat: #7 S5 — scope-aware role assignment + revocation (AssignRole/RevokeRole scope tuple, escalation prevention)#337
Merged
Conversation
Wires the (scope_kind, scope_id) tuple through AssignRoleTo{User,UserGroup}
and RevokeRoleFrom{User,UserGroup} — the grant-authoring half of #7. The
proto fields, RoleGrantScopeKind enum, AssignRoleScope permission, and the
scope-aware projector writes already existed (S1/S2); the handlers ignored
them until now.
Assign validation (validateAssignGrantScope, shared by both handlers):
- paired-or-neither (scope_kind and scope_id set together or not at all);
- AssignRoleScope gate for any scoped grant;
- scope-group existence (device_group / user_group);
- per-role target_kind match — every permission in the role must accept
the scope kind (device_group scope => all perms TargetDevice). Derived
self-discoveringly from the permission registry (TargetKindFor), not a
hardcoded list, so a new permission can't silently slip through.
Escalation prevention (auth, gate-then-narrow over the actor's own
ScopedGrants from S2b):
- EnforceGrantScopeAuthority: a scope-limited admin may attach ONLY a
scope its own AssignRoleScope authority covers (global => any; scoped =>
equal-id match; sub-group 'narrower' containment is a future refinement);
- EnforceUnscopedGrantAuthority: a scope-limited admin may not mint an
unscoped (fleet-wide) grant.
Revoke targets a SPECIFIC (subject, role, scope) grant via new NULL-aware
existence checks (UserHasScopedRole / UserGroupHasScopedRole, hand-written
like S2's scope queries — sqlc can't see the DO-block columns). Revoking a
grant that exists only at a different scope is rejected (FailedPrecondition)
rather than silently no-op'd; nothing-there stays an idempotent no-op.
Test fidelity: AdminContext now carries every admin permission as an
unscoped grant, mirroring production where the flat permission list is
DERIVED from grants — so the scope-authority checks see an org admin as
globally unrestricted rather than as having no scope authority.
Tests: auth helper units (target-kind match, both escalation bounds);
handler integration (scoped grant end-to-end, AssignRoleScope gate,
target_kind mismatch, missing scope group, paired-or-neither, escalation
out-of-scope + unscoped denial, scope-targeted revoke) for both the user
and user-group paths.
Refs #7.
|
Warning Review limit reached
More reviews will be available in 15 minutes and 43 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR adds scope tuple normalization and validation, permission scopability checks, and scope-authority enforcement; it applies these to user and user-group role assign/revoke handlers, emits scope-aware events and stream IDs, and adds scoped existence SQL queries and tests. ChangesScoped Role Grants
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/api/role_handler.go (1)
454-457:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSession invalidation should be conditional on actual revocation.
The session version is bumped unconditionally, even when the grant doesn't exist (idempotent no-op case at lines 439-452). This is inconsistent with
AssignRoleToUser(lines 362-369), which only bumps whenassignedAnyis true. Unnecessary session bumps force users to re-login even when no state changed.🔒 Proposed fix to make session bump conditional
if hasScoped { streamID := req.Msg.UserId + ":" + req.Msg.RoleId if scopeKind != "" { streamID += ":" + scopeID } if err := appendEvent(ctx, h.store, h.logger, store.Event{ StreamType: "user_role", StreamID: streamID, EventType: string(eventtypes.UserRoleRevoked), Data: payloads.UserRoleRevoked{ UserID: req.Msg.UserId, RoleID: req.Msg.RoleId, ScopeKind: sk, ScopeID: si, }, ActorType: "user", ActorID: userCtx.ID, }, "failed to revoke role"); err != nil { return nil, err } + + // Bump user's session version to invalidate cached permissions + if err := h.bumpUserSessionVersion(ctx, req.Msg.UserId, userCtx.ID); err != nil { + return nil, apiErrorCtx(ctx, ErrInternal, connect.CodeInternal, "failed to invalidate user session after role revocation") + } } else { // The targeted grant doesn't exist. If the role IS assigned at a // different scope, the caller targeted the wrong grant — surface // that rather than silently no-op (`#7` S5). If nothing exists at // all, fall through as an idempotent no-op. hasAny, err := h.store.Repos().Role.UserHasRole(ctx, req.Msg.UserId, req.Msg.RoleId) if err != nil { return nil, apiErrorCtx(ctx, ErrInternal, connect.CodeInternal, "failed to check role assignment") } if hasAny { return nil, apiErrorCtx(ctx, ErrRoleNotFound, connect.CodeFailedPrecondition, "role is not assigned at the specified scope (it is assigned at a different scope)") } } - // Bump user's session version to invalidate cached permissions - if err := h.bumpUserSessionVersion(ctx, req.Msg.UserId, userCtx.ID); err != nil { - return nil, apiErrorCtx(ctx, ErrInternal, connect.CodeInternal, "failed to invalidate user session after role revocation") - } - return connect.NewResponse(&pm.RevokeRoleFromUserResponse{}), nil🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/api/role_handler.go` around lines 454 - 457, The session bump is happening unconditionally after attempting to revoke a role; change it to only bump when the revocation actually removed a grant (like AssignRoleToUser does). Locate the revoke flow in role_handler.go where bumpUserSessionVersion(ctx, req.Msg.UserId, userCtx.ID) is called and make it conditional on the flag that tracks whether any grant was revoked (e.g., revokedAny or similar variable set in the revocation loop). Only call h.bumpUserSessionVersion(...) when that flag is true to avoid forcing logouts on idempotent no-ops.
🧹 Nitpick comments (2)
internal/api/role_handler.go (2)
449-450: ⚡ Quick winError enum name doesn't match the failure mode.
ErrRoleNotFoundis used for a scope mismatch case, but the role itself exists—just not at the specified scope. This could confuse debugging and error handling code that expectsErrRoleNotFoundto mean the role entity doesn't exist. Consider using a more specific error enum likeErrScopeMismatchorErrGrantNotFoundAtScope.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/api/role_handler.go` around lines 449 - 450, The return uses ErrRoleNotFound for a scope-mismatch case; update this to a more accurate error enum (e.g., ErrScopeMismatch or ErrGrantNotFoundAtScope) so callers can distinguish "role exists but wrong scope" from "role missing." Replace ErrRoleNotFound in the apiErrorCtx call that returns connect.CodeFailedPrecondition (the return line invoking apiErrorCtx) with the new enum, and ensure the new error is defined and used consistently where scope-mismatch semantics are required.
321-359: ⚖️ Poor tradeoffNote: Scoped grant re-assigns bypass idempotency pre-check.
For unscoped grants,
UserHasRolepre-checks and skips redundant assigns (lines 326-337), soassignedAnystays false and the session isn't bumped. For scoped grants, no pre-check is performed; the event is always appended (relying on the projector'sON CONFLICT DO NOTHING), andassignedAnyis set to true (line 359), causing the session to be bumped even for redundant scoped assigns. This asymmetry means idempotent scoped assign retries will unnecessarily invalidate sessions, contrary to the goal stated at lines 363-364.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/api/role_handler.go` around lines 321 - 359, The code skips the idempotency pre-check only for scoped grants, causing redundant scoped re-assigns to set assignedAny and bump sessions; add a scoped pre-check like the unscoped branch: when scopeKind != "" call the appropriate DB check (e.g., reuse/implement q.UserHasRole with scope parameters or add a new query such as UserHasRoleWithScope) using req.Msg.UserId, roleID, scopeKind/scopeID (use scopePtrs(scopeKind, scopeID) values if needed) and if it already exists continue without appending the event; only appendEvent and set assignedAny = true when the scoped grant did not already exist (refer to scopePtrs, appendEvent, assignedAny, and UserHasRole/UserHasRoleWithScope).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/api/scope_grant.go`:
- Around line 14-24: scopeKindString currently returns "" for any unknown enum
which makes malformed/future scope_kind silently unscoped; change
scopeKindString(pm.RoleGrantScopeKind) to return (string, error) and map
ROLE_GRANT_SCOPE_KIND_UNSPECIFIED -> ("", nil) and the two known kinds to their
auth.ScopeKind strings, but return an error (InvalidArgument) for any other enum
value; update all callers (the grant-parsing logic around the existing handling
at lines ~46-60 that expect "" to mean unscoped) to handle the error path and
propagate/return InvalidArgument when scopeKindString returns an error.
- Around line 68-74: The current code calls scopeGroupExists(ctx, q, scopeKind,
scopeID) before auth.EnforceGrantScopeAuthority(ctx, scopeKind, scopeID), which
leaks existence information to scope-limited callers; swap the checks so you
call auth.EnforceGrantScopeAuthority first and return its error immediately if
any, then call scopeGroupExists; update the block around scopeGroupExists and
EnforceGrantScopeAuthority to enforce scope authority before checking group
existence (functions: auth.EnforceGrantScopeAuthority and scopeGroupExists)
while preserving the existing return values and error handling.
---
Outside diff comments:
In `@internal/api/role_handler.go`:
- Around line 454-457: The session bump is happening unconditionally after
attempting to revoke a role; change it to only bump when the revocation actually
removed a grant (like AssignRoleToUser does). Locate the revoke flow in
role_handler.go where bumpUserSessionVersion(ctx, req.Msg.UserId, userCtx.ID) is
called and make it conditional on the flag that tracks whether any grant was
revoked (e.g., revokedAny or similar variable set in the revocation loop). Only
call h.bumpUserSessionVersion(...) when that flag is true to avoid forcing
logouts on idempotent no-ops.
---
Nitpick comments:
In `@internal/api/role_handler.go`:
- Around line 449-450: The return uses ErrRoleNotFound for a scope-mismatch
case; update this to a more accurate error enum (e.g., ErrScopeMismatch or
ErrGrantNotFoundAtScope) so callers can distinguish "role exists but wrong
scope" from "role missing." Replace ErrRoleNotFound in the apiErrorCtx call that
returns connect.CodeFailedPrecondition (the return line invoking apiErrorCtx)
with the new enum, and ensure the new error is defined and used consistently
where scope-mismatch semantics are required.
- Around line 321-359: The code skips the idempotency pre-check only for scoped
grants, causing redundant scoped re-assigns to set assignedAny and bump
sessions; add a scoped pre-check like the unscoped branch: when scopeKind != ""
call the appropriate DB check (e.g., reuse/implement q.UserHasRole with scope
parameters or add a new query such as UserHasRoleWithScope) using
req.Msg.UserId, roleID, scopeKind/scopeID (use scopePtrs(scopeKind, scopeID)
values if needed) and if it already exists continue without appending the event;
only appendEvent and set assignedAny = true when the scoped grant did not
already exist (refer to scopePtrs, appendEvent, assignedAny, and
UserHasRole/UserHasRoleWithScope).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9bc553f6-cd5f-4c25-96c4-777debf58d79
⛔ Files ignored due to path filters (2)
internal/store/generated/roles.sql.gois excluded by!**/generated/**internal/store/generated/user_groups.sql.gois excluded by!**/generated/**
📒 Files selected for processing (11)
internal/api/role_handler.gointernal/api/role_scope_handler_test.gointernal/api/scope_grant.gointernal/api/user_group_handler.gointernal/api/user_group_scope_handler_test.gointernal/auth/permissions.gointernal/auth/scope.gointernal/auth/scope_test.gointernal/store/queries/roles.sqlinternal/store/queries/user_groups.sqlinternal/testutil/factories_user.go
…xistence (#7 S5) CodeRabbit review of #337: 1. (Major) scopeKindString mapped any out-of-range / future RoleGrantScopeKind enum to "", so a malformed scope_kind with an empty scope_id slipped through paired-or-neither and became an unscoped, fleet-wide grant. It now returns (string, ok); only the explicit UNSPECIFIED maps to "" and every unknown value is rejected as InvalidArgument. Applied to all three call sites — the assign validator and both revoke handlers. 2. (Minor) validateAssignGrantScope checked scope-group existence (NotFound) before scope authority (PermissionDenied), letting a scope-limited caller use group existence as an oracle for ids outside its authority. Authority is now enforced first, so an out-of-scope id is PermissionDenied whether or not the group exists. Regression tests pin both: an unknown enum yields InvalidArgument and creates no grant; a scope-limited caller targeting an id outside its authority gets PermissionDenied regardless of the group's existence. Refs #7.
Contributor
Author
|
@coderabbitai full review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
internal/api/role_handler.go (1)
452-454: 💤 Low valueSemantic mismatch:
ErrRoleNotFoundused for scope-mismatch error.The error sentinel
ErrRoleNotFoundis used when the role IS assigned but at a different scope. Consider introducing a dedicated sentinel (e.g.,ErrScopeMismatch) or reusing a more accurate one, since the role was found — just not at the targeted scope. The Connect code (CodeFailedPrecondition) and message are correct, so this is purely a semantic clarity concern for internal error tracking/metrics.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/api/role_handler.go` around lines 452 - 454, The current return uses the ErrRoleNotFound sentinel while the role exists but at a different scope; introduce a new sentinel ErrScopeMismatch (or pick an existing more appropriate sentinel) and replace ErrRoleNotFound with ErrScopeMismatch in the apiErrorCtx call in the role validation branch (the code that currently returns apiErrorCtx(ctx, ErrRoleNotFound, connect.CodeFailedPrecondition, "...")). Add the ErrScopeMismatch declaration alongside other error sentinels, update any usages or tests that expect ErrRoleNotFound for this path, and keep the existing connect.CodeFailedPrecondition and message unchanged.internal/api/user_group_scope_handler_test.go (1)
92-97: ⚡ Quick winConsider adding post-revoke assertion to verify grant removal.
The test verifies the revoke call succeeds, but doesn't assert that the grant is actually removed from the projection. Adding a verification would make the test more robust.
🧪 Suggested addition
// The exact scoped revoke succeeds. _, err = h.RevokeRoleFromUserGroup(ctx, connect.NewRequest(&pm.RevokeRoleFromUserGroupRequest{ GroupId: group, RoleId: role, ScopeKind: deviceGroupScope, ScopeId: dg, })) require.NoError(t, err) + + // Verify the grant is actually removed. + var count int + err = st.TestingPool().QueryRow(context.Background(), + "SELECT COUNT(*) FROM user_group_roles_projection WHERE group_id=$1 AND role_id=$2", + group, role).Scan(&count) + require.NoError(t, err) + assert.Equal(t, 0, count, "grant should be removed after revoke")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/api/user_group_scope_handler_test.go` around lines 92 - 97, After calling h.RevokeRoleFromUserGroup, add an assertion that the grant no longer exists in the projection by querying the projection/store for the specific grant (GroupId: group, RoleId: role, ScopeKind: deviceGroupScope, ScopeId: dg) and asserting it's absent; e.g. call the projection's read method (GetGrant or ListGrants equivalent) and use require.NoError for the query and require.Nil/require.False/require.Empty to confirm the grant is removed. Ensure the check targets the same identifiers used in the test (group, role, deviceGroupScope, dg) so the revocation is validated end-to-end.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/api/user_group_handler.go`:
- Around line 599-615: The RevokeRoleFromUserGroup handler currently returns
CodeNotFound when neither the scoped grant nor any assignment exists, which is
inconsistent with RevokeRoleFromUser's idempotent no-op; update
RevokeRoleFromUserGroup so that after calling q.UserGroupHasRole
(UserGroupHasRole) if hasAny is false it does not return apiErrorCtx with
ErrRoleNotFound/CodeNotFound but instead treats it as a successful no-op (same
behavior as RevokeRoleFromUser) and continues to the normal post-revoke flow
(e.g., bumping session), removing or replacing the ErrRoleNotFound CodeNotFound
branch to align semantics.
---
Nitpick comments:
In `@internal/api/role_handler.go`:
- Around line 452-454: The current return uses the ErrRoleNotFound sentinel
while the role exists but at a different scope; introduce a new sentinel
ErrScopeMismatch (or pick an existing more appropriate sentinel) and replace
ErrRoleNotFound with ErrScopeMismatch in the apiErrorCtx call in the role
validation branch (the code that currently returns apiErrorCtx(ctx,
ErrRoleNotFound, connect.CodeFailedPrecondition, "...")). Add the
ErrScopeMismatch declaration alongside other error sentinels, update any usages
or tests that expect ErrRoleNotFound for this path, and keep the existing
connect.CodeFailedPrecondition and message unchanged.
In `@internal/api/user_group_scope_handler_test.go`:
- Around line 92-97: After calling h.RevokeRoleFromUserGroup, add an assertion
that the grant no longer exists in the projection by querying the
projection/store for the specific grant (GroupId: group, RoleId: role,
ScopeKind: deviceGroupScope, ScopeId: dg) and asserting it's absent; e.g. call
the projection's read method (GetGrant or ListGrants equivalent) and use
require.NoError for the query and require.Nil/require.False/require.Empty to
confirm the grant is removed. Ensure the check targets the same identifiers used
in the test (group, role, deviceGroupScope, dg) so the revocation is validated
end-to-end.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8de4faf3-24d6-40b3-97f5-b915561dd565
⛔ Files ignored due to path filters (2)
internal/store/generated/roles.sql.gois excluded by!**/generated/**internal/store/generated/user_groups.sql.gois excluded by!**/generated/**
📒 Files selected for processing (11)
internal/api/role_handler.gointernal/api/role_scope_handler_test.gointernal/api/scope_grant.gointernal/api/user_group_handler.gointernal/api/user_group_scope_handler_test.gointernal/auth/permissions.gointernal/auth/scope.gointernal/auth/scope_test.gointernal/store/queries/roles.sqlinternal/store/queries/user_groups.sqlinternal/testutil/factories_user.go
…S5) CodeRabbit re-review of #337 flagged a pre-existing inconsistency that S5 made adjacent: revoking a role grant that doesn't exist behaved two ways — RevokeRoleFromUser fell through as a no-op (success) while RevokeRoleFromUserGroup returned NotFound. Aligned both to an idempotent no-op (success), matching the project's desired-state/ABSENT idempotency (assign uses ON CONFLICT DO NOTHING) and keeping revoke retry-safe. The "assigned at a different scope" case still returns FailedPrecondition — that ambiguity is deliberately surfaced. The no-op path now also skips the session-version bump (nothing changed), so a no-op revoke no longer needlessly invalidates sessions (the group variant would otherwise have re-logged-in every group member). Tests: TestRevokeRoleFromUserGroup_NotAssigned flipped from NotFound to success; added TestRevokeRoleFromUser_NotAssignedIsNoop for parity. Refs #7.
PaulDotterer
added a commit
that referenced
this pull request
Jun 10, 2026
The Integration Tests job starts a fresh Postgres testcontainer per test; with the #7 scope/TerminalAdmin slices the api + store integration packages now run near the 20m ceiling (#337 used 19m20s; #345 landed at ~20m). Bump to 30m as the interim unblock the issue recommends, ahead of the durable container-reuse fix (testcontainers Reuse / per-package TestMain + TRUNCATE). Refs #338.
This was referenced Jun 11, 2026
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
1 participant
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.
Fourth slice of #7 — the grant-authoring half:
AssignRoleTo{User,UserGroup}andRevokeRoleFrom{User,UserGroup}now consume the(scope_kind, scope_id)tuple. Server-only — the proto fields,RoleGrantScopeKindenum,AssignRoleScopepermission,PermissionTargetKind, and the scope-aware projector writes all already existed (S1/#333, S2/#334); the handlers ignored scope until now. Builds on S2b (#335)'sUserContext.ScopedGrants.Assign validation (
validateAssignGrantScope, shared by both handlers)scope_kind/scope_idset together or not at all;AssignRoleScopegate for any scoped grant;target_kindmatch — every permission in the role must accept the scope kind (device_group scope ⇒ all permsTargetDevice). Derived self-discoveringly from the permission registry (TargetKindFor), not a hardcoded list, so a new permission can't silently slip the check.Escalation prevention (auth, over the actor's own
ScopedGrants)EnforceGrantScopeAuthority— a scope-limited admin may attach only a scope its ownAssignRoleScopeauthority covers (global ⇒ any; scoped ⇒ equal-id match; sub-group "narrower" containment is a documented future refinement).EnforceUnscopedGrantAuthority— a scope-limited admin may not mint an unscoped, fleet-wide grant.In V1
AssignRoleScopeis org-tier (held only by unscoped admins), so the bounded path isn't reachable yet — this is defense-in-depth ahead of per-scope admins.Revoke
Targets a SPECIFIC
(subject, role, scope)grant via new NULL-aware existence checks (UserHasScopedRole/UserGroupHasScopedRole, hand-written like S2's scope queries — sqlc can't read migration 010'sDO-block columns, see #336). Revoking a grant that exists only at a different scope is rejected (FailedPrecondition) rather than silently no-op'd; nothing-there stays an idempotent no-op.Test fidelity fix
AdminContextnow carries every admin permission as an unscoped grant, mirroring production — where the flat permission list is derived from grants — so the scope-authority checks see an org admin as globally unrestricted rather than as having no scope authority. (Additive; existing tests unaffected.)Tests
AssignRoleScopegate, target_kind mismatch, missing scope group, paired-or-neither, escalation out-of-scope + unscoped denial, scope-targeted revoke + wrong-scope rejection.go vet ./...,gofmt,go build,internal/auth,internal/projectors, all targeted scope tests green. Fullinternal/api: 312 pass / 0 fail (local wall-clock exceeded the default timeout under heavy back-to-back container churn; CI's-timeout 20mfresh runner ran the identical-size suite in 562s at S2b). Local CodeRabbit: no findings.Follow-ups
The enforcement-consuming half — per-device/-user handlers calling
EnforceDeviceScope/EnforceUserScopewith aScopeResolverwired to the projections (S6), the per-scope reconciler, and the TTY-gated web scope picker (S9).Closes part of #7.
Summary by CodeRabbit
New Features
Tests