Skip to content

Permissions Decisions

Each entry is a real fork in the road — what we chose, what we passed on, and why. Implementation details that aren't a tradeoff are NOT here (read the codebase for those).


Role IDs: auto-incrementing integers, not ULIDs

  • Chose: Auto-increment integer primary key on roles (consistent with User, Team, Project, Issue, etc.).
  • Rejected: Keep the already-implemented ULIDs (globally unique, time-sortable).
  • Why: Every other model uses int IDs — ULIDs forced ugly URLs, frontend String() coercion, and gave no distributed-ID benefit in a single-tenant-per-DB architecture. Migrations were undeployed, so modify in place.
  • Source: KD-0315

Permission gating: replace isAdmin/requiresAdmin with can() everywhere

  • Chose: All 15 component checks, all requiresAdmin route metas, and all ownership hybrid checks migrated to can(resource, action).
  • Rejected: Keep isAdmin for admin-only features (simple, but defeats the permission system).
  • Why: Custom roles with the right permissions can now access features without being full admins. Half-migrating leaves two systems running in parallel.
  • Source: KD-0315

Resource enum slot 13: drop AppSettings, add Roles

  • Chose: Remove the synthetic AppSettings resource (catch-all that mapped to no real CRUD entity); add Roles at slot 13. Total stays at 15.
  • Rejected: Hide AppSettings in the form but keep the backend (leaves dead code); remove AppSettings and keep 14 resources (Roles still piggybacks on Users).
  • Why: Pre-production, so a clean swap with no migration headaches. RolePolicy now checks the Roles resource directly instead of overloading Users.
  • Source: KD-0315

Always-readable resources: 12 forced read, 4 toggleable

  • Chose: Force can_read=true on 12 project-scoped + tenant-wide resources. Keep toggleable read only on 4 sensitive ones: SystemPrompts, ProjectTokens, Roles, AppSettings.
  • Rejected: Force read on the core 3 only (Projects/Issues/Teams) — minimum to prevent sidebar breakage; guard navbar with can() and handle 403 gracefully.
  • Why: Project membership is the real read gate for project-scoped resources — per-resource read toggles are redundant and only break things (silent navbar failures). Sensitive resources keep the toggle because exposing AI instructions or API credentials must remain explicit.
  • Source: KD-0315

Read-when-update auto-grant

  • Chose: Auto-grant read whenever update or delete is set. Frontend ticks the checkbox on toggle; backend normalizes before save.
  • Rejected: Reject as 422 (force explicit read); frontend-only auto-check.
  • Why: A role with update-but-not-read is logically impossible — better to silently fix it than throw friction at users. Backend normalization closes the API-bypass hole.
  • Source: KD-0315

Project owner bypass: ALL project-scoped resources, not just settings

  • Chose: Project owner bypasses permission checks for all 12 project-scoped resources (Projects, Sprints, Epics, Lanes, Issues, etc.). Tenant-scoped (Users, Roles, Teams, AppSettings) NOT bypassed.
  • Rejected: Owner only bypasses for project settings (sprints/epics still gated by role); owner gets implicit admin role per project.
  • Why: "My project, my rules" is the simpler mental model — single early-return in CheckPermission. Restricting owners by role would let a restrictive role lock owners out of their own sprints.
  • Source: KD-0315 D17

Project settings access: normal Projects.Update, not owner-only

  • Chose: Project settings page uses standard Projects.Update permission. No special isProjectSettings gate.
  • Rejected: Wire the existing isProjectSettings flag to restrict to owner+admin (per original design doc).
  • Why: Restricting Projects.Update holders from editing project settings defeats the permission system's purpose. Owner bypass (above) covers owner access automatically. isProjectSettings parameter becomes dead code.
  • Source: KD-0315 D18

Ownership transfer: dedicated policy ability, not generic Projects.Update

  • Chose: New transferOwnership(User, Project): bool ability on ProjectPolicy, owner-or-admin only.
  • Rejected: Reuse the existing isProjectSettings flag for the transfer endpoint.
  • Why: Anyone with Projects.Update should NOT also be able to transfer ownership — that's too permissive. Naming isProjectSettings for ownership transfer is misleading. Dedicated ability keeps intent clear.
  • Source: KD-0315 D19

Ownership transfer scope: project members only

  • Chose: Both layers: frontend modal lists only current project members; backend validates new_owner_id exists in project_user for that project.
  • Rejected: Trust frontend filter; rely only on exists:users,id (allows transfer to non-members).
  • Why: Original implementation passed all tenant users with only generic existence validation. Defense-in-depth with the project-scoped Rule::exists.
  • Source: KD-0315 D20

Frontend owner bypass API: new canInProject(), don't change can() signature

  • Chose: New canInProject(resource, action, projectOwnerId, entityOwnerId?) exported alongside can().
  • Rejected: Add projectOwnerId to can() (changes every existing call site); useProjectPermission(project) composable.
  • Why: Additive — existing calls unchanged. usePermission is initialized once in auth state, not per-component, so a composable doesn't fit.
  • Source: KD-0315 D21

IDOR fix: custom validation rule, not Gate or controller-level authorize

  • Chose: New UserHasProjectAccess rule in FormRequests using ProjectBuilder::accessibleBy($user)->pluck('id') to batch-check all project IDs in one query.
  • Rejected: Gate::authorize('access', Project::find($id)) in the Action; $this->authorize() in the controller.
  • Why: Action-level Gate violates ADR-0011 (Actions shouldn't depend on HTTP context). Controller-level breaks the convention that controllers only delegate to Actions. Validation rule keeps the logic in the request layer with a clear 422 response.
  • Source: KD-0340 D1

NotificationPolicy: remove admin bypass entirely

  • Chose: Remove before() from NotificationPolicy.
  • Rejected: Keep + document the bypass as intentional; keep + add a test codifying it.
  • Why: Notifications are personal data — admins should not be able to mark other users' notifications as read via API. This is the only policy where the admin bypass pattern is semantically wrong.
  • Source: KD-0340 D6

Global search: scope exists rules to accessible projects

  • Chose: GlobalSearchIssuesRequest validates lane_id/sprint_id/epic_id against ProjectBuilder::accessibleBy($user)->pluck('id').
  • Rejected: Trust the existing unscoped Rule::exists (200 vs 422 response code reveals which IDs exist across all projects).
  • Why: Closes the cross-project enumeration leak. One extra query per search is acceptable.
  • Source: KD-0340 D7

Teams visibility: frontend filter only, NOT backend scopeQuery

  • Chose: Overview.vue adds a computed filtering allTeams by membership or admin. /api/teams contract preserved.
  • Rejected: v1's plan to add TeamPolicy::scopeQuery + CheckPermission::canManageTeams and scope the API; query-param ?scope=mine.
  • Why: Backend scoping silently breaks assignee dropdowns on multi-team projects (a Member on team A viewing a project assigned A+B+C would lose every assignee reachable only via B or C — including admins). The ticket is a UX complaint, not data leakage. Client-side filter solves the exact problem with zero risk.
  • Source: KD-0361 D1

User Management visibility: gate on Users.Update, don't restrict /api/users for Members

  • Chose: Sidebar link, route, and deleted-users tab move from can(Users, Read) to can(Users, Update). /api/users stays open.
  • Rejected: Revoke Member.Users.can_read (literal AC2 reading); scope /api/users to teammates+referenced-users (expensive, fragile).
  • Why: userStore.retrieveAll() is consumed by Navbar, comment authors, assignees, time-entry owners — restricting it would render names as "User #42" everywhere. CEO confirmed: admin names should still display in context. Members lose the browsable list but keep name resolution.
  • Source: KD-0361 D3

Users.Read removal NOT applied to Roles — Member loses Roles.Read

  • Chose: Migration sets Member.Roles.can_read = false. Sidebar gate auto-hides the Roles link.
  • Rejected: Treat Roles like Users (keep the API open, gate only the UI).
  • Why: /api/roles is consumed only by the Roles domain pages (no global store). Revoking can_read is safe and matches the intent of "geen toegang tot algemene projectinstellingen."
  • Source: KD-0361 D4

NO CheckPermission::canManageTeams + TeamPolicy::scopeQuery

  • Chose: Don't introduce union-permission helpers or a novel scopeQuery policy method.
  • Rejected: v1's canManageTeams + scopeQuery to support hypothetical "Team Manager" custom roles.
  • Why: YAGNI — no Team Manager role exists. Frontend filter (D1 above) solves the real problem with zero scaffolding. scopeQuery would be a net-new pattern with no codebase precedent. Revisit when a Team Manager role becomes real.
  • Source: KD-0361 D9

Issue Own-scope delete: frontend-only, no backend changes

  • Chose: Add show-own UI toggle to issue update + delete cells. Use existing PermissionScopeEnum::Own, IssuePolicy::delete(), CheckPermission::checkScope().
  • Rejected: Add new enum values, policy changes, scope checks (already exists).
  • Why: Backend has supported Own-scope for issue delete/update since the permission system was built. Audit before designing — this was a UI-only gap.
  • Source: KD-0386 D1

Bulk delete UX for Own-scope: hide entirely, don't filter selection

  • Chose: Bulk delete button stays hidden when scope=Own (existing behavior).
  • Rejected: Filter bulk selection to only the user's own issues.
  • Why: Issue explicitly excludes bulk deletion from scope. Granular bulk-delete is more complex with no real demand.
  • Source: KD-0386 D4

Team-scoped user visibility: new access_all_users boolean on Role

  • Chose: New access_all_users boolean column on roles, mirroring the existing access_all_projects. Composable — a custom role can have one, both, or neither.
  • Rejected: Three-state enum None|SharedProjects|All (no precedent — existing fields are booleans); implicit via Users.Update (couples "manage" with "see"); only is_admin bypasses (KD-0361 explicitly reopened the question).
  • Why: Identical shape to access_all_projects means same toggle UI, same DTO field, same FormRequest validation. Reviewer can compare one-to-one. Composability future-proofs custom Users-Manager roles.
  • Source: KD-0501 D1

User scoping: filter the response, don't trim the resource

  • Chose: Drop outside-scope users from /api/users. Resource shape stays UserPublicResourceData (admin) / UserResourceData (non-admin).
  • Rejected: Two-tier resource swap (UserSummaryResourceData for non-bypass actors); hybrid filter+trim with two shapes in one response.
  • Why: Trimming doesn't fix the enumeration complaint — non-bypass actors still get one row per tenant user. Two resource shapes in one response has no precedent. Filtering closes KD-0361's accepted residual leak (team_ids[]/project_ids[] enumeration).
  • Source: KD-0501 D2

Visibility scope: project-team transitive, derived from hasProjectAccess semantics

  • Chose: Visible users = members of any team on accessible projects ∪ direct members of accessible projects ∪ all admins ∪ all access_all_users users ∪ self.
  • Rejected: "My teams' members only" (breaks multi-team projects); "My teams + my direct memberships' direct members" (half-fix, still misses other teams on the project).
  • Why: Multi-team projects must keep working — exactly the regression KD-0361 D1 was paranoid about. Naturally honors access_all_projects without an explicit coupling rule.
  • Source: KD-0501 D3

Visibility logic: Eloquent scope on User model, not a Helpers class

  • Chose: User::scopeVisibleTo(Builder $query, User $actor): void (mirrors Issue::scopeAssignedOpenFor).
  • Rejected: App\Helpers\UserVisibilityScope class injecting CheckPermission; inline duplication in controller and policy.
  • Why: Deptrac forbids Helpers → Policies AND Policies → Helpers. The User-model scope is the only locus reachable from BOTH UserController::index AND UserPolicy::update/delete/invite. Direct codebase precedent exists.
  • Source: KD-0501 D4

Always include admins + access_all_users users in scoped responses

  • Chose: Append whereHas('roles', is_admin OR access_all_users) to the User scope so admins are always visible.
  • Rejected: Server-side union of "referenced user IDs" from issues/comments/time-entries (rejected as expensive in KD-0361 D3); inline author names in each Resource; accept "User #42" rendering.
  • Why: Without this, an admin who isn't on any of the actor's project teams disappears from /api/users but their comments/issues are still visible — breaking name resolution. Reverses KD-0361 D7's explicit guarantee. Cheap one extra whereHas clause.
  • Source: KD-0501 D5

/api/users/{user} action routes: gate via UserPolicy, not Route::bind

  • Chose: Extend UserPolicy::update/::delete/::invite to gate by visibility. Out-of-scope target → false → 403.
  • Rejected: Route::bind('user', ...) returning 404 for out-of-scope IDs; inline check in each controller method.
  • Why: Route::bind has zero codebase precedent and risks affecting other routes binding User. Inline controller checks mix auth-shaped logic into thin controllers (violates the project's policy/permission tier convention). Policy is the in-codebase shape with ->can('update', 'user') middleware already wired.
  • Source: KD-0501 D10

access_all_users privilege-escalation guard mirrors access_all_projects

  • Chose: RoleRequest validates access_all_users with Rule::when(!isAdmin, [Rule::in([false])]) — only admins can set it to true.
  • Rejected: Trust the field unconditionally.
  • Why: Without the guard, a Users-Manager role with Roles.Update could grant their own role bypass capability — privilege escalation. Existing access_all_projects already uses this exact pattern verbatim.
  • Source: KD-0501 D11

BillingPolicy::manage() routes through CheckPermission, not raw isAdmin()

  • Chose: Migrate the backend policy gate to CheckPermission for Billing:READ OR Billing:UPDATE (admins still pass via the bypass step), matching the route-meta OR-semantics.
  • Rejected: Leave manage() as $user->isAdmin() for a smaller diff.
  • Why: Keeping the policy admin-only makes the route-meta change cosmetic and lets backend and frontend gates diverge for any future non-admin billing role.
  • Source: KD-0640 D3

Permission-seed backfill: raw DB::table() migration, not an Action wrapper

  • Chose: Follow the five precedent permission-seed migrations — raw inline DB::table()->insert() in up(), partitioned by is_admin/is_system.
  • Rejected: Wrap the backfill in a BackfillBillingPermissionsAction to match the literal text of ADR-0011.
  • Why: ADR-0011 governs request-time application code, not one-shot migrations; introducing an Action here would own a net-new mini-pattern no existing seed migration uses.
  • Source: KD-0640 D7

Permission-denied message: one global AuthorizationException renderer, not a per-site exception migration

  • Chose: Register a single render() closure in bootstrap/app.php's withExceptions() that intercepts authorization denials for JSON requests and returns a structured 403 — every ->can() route denial and in-Action Gate::authorize gets the better message at once.
  • Rejected: The ticket's framed design — a generic PermissionDeniedException extends CustomException + a PermissionDenialReason enum + a Gate::denies-throw per Action, migrated one site at a time (with arch test, deptrac widening, MCP + FE changes).
  • Why: Every Kendo permission denial is uniform ("you lack a role permission; an admin manages roles") — there is no per-site nuance needing a bespoke message or machine-readable reason catalog, so the per-site scaffolding delivered what one framework-level override delivers everywhere in ~8 lines.
  • Source: KD-0739 D0/D1

Denial override swaps only the framework-default message; custom messages pass through

  • Chose: The closure rewrites the message only when it equals Laravel's default "This action is unauthorized."; any custom message (e.g. token-ownership "This token does not belong to you.") passes through unchanged, with code still attached.
  • Rejected: A blanket override that genericizes every AuthorizationException message.
  • Why: All Kendo policies return plain bool (no Response::deny('…') custom messages), so the default-string check cleanly partitions "policy/gate denial" from "manual custom-message throw" — a blanket rewrite would wrongly flatten the ownership message into the generic explainer.
  • Source: KD-0739 D2

403 discriminator: reuse the existing {code, message} shape, not a new reason_code key or reason enum

  • Chose: Body is {code: "PERMISSION_DENIED", message} — a single static code and a complete self-contained message the FE shows as-is.
  • Rejected: A reason_code key backed by a per-reason enum/catalog; a resolved_by field the FE concatenates into the message.
  • Why: It mirrors TenantNotVerifiedException's {code, message} — the established 403-discriminator convention the FE already reads via data.code — so inventing a third key or a reason catalog adds machinery for a denial type that has no per-reason nuance.
  • Source: KD-0739 D3

Forbid direct AuthorizationException throws via arch test, migrate offenders to a dedicated exception

  • Chose: A Level-1 Pest arch test bans new AuthorizationException(...) in app/; non-generic denials must use ->can()/Gate::authorize() (RBAC → standard explainer) or a dedicated CustomException subclass — the two token-ownership sites moved to a new TokenNotOwnedException.
  • Rejected: An opt-in marker on the global override to flag intentional non-generic throws.
  • Why: The override is implicit, so a future developer throwing AuthorizationException for a case that should NOT read as "ask an admin" gets no signal; ->can() route denials have no throw site to gate and are correct by design, so the only risky pattern is a deliberate direct throw — exactly what the arch test catches, shipping with zero allowlist.
  • Source: KD-0739 D7

Permission-change detection diffs resolved effective permissions, not the mutation or its inputs

  • Chose: Snapshot each affected user's resolved permission state before the mutation, recompute after, and notify only on a real difference.
  • Rejected: Notify everyone a role/membership mutation touches; diff the raw RolePermission/pivot rows.
  • Why: "No spurious notifications" is a hard acceptance criterion and only a resolved-output diff meets it — a role rename, or a permission re-granted that the user already held via another role, leaves the resolved matrix identical while both cheaper checks fire anyway.
  • Source: KD-0582 D1

Reuse an existing preference column for the new notification type instead of adding a dead one

  • Chose: Point the new NotificationTypeEnum case's exhaustive column() at the existing notify_project_membership column, with a comment marking it vestigial for this type.
  • Rejected: Add a notify_project_permissions column plus migration and factory update.
  • Why: column() is consumed only by email unsubscribe and this type sends no email — a new column would be dead on arrival, existing purely to satisfy an exhaustive match.
  • Source: KD-0582 D7

Deptrac forces the notifier to re-derive the permission matrix rather than call CheckPermission

  • Chose: A dedicated Action re-derives the union matrix straight from Role/RolePermission, mirroring the existing GetPermissionMatrixAction, with a Feature test asserting parity against CheckPermission.
  • Rejected: Depend on App\Policies\CheckPermission from the Action — the natural design.
  • Why: The Actions layer's deptrac allow-list excludes Policies, so this replication is structural rather than sloppy; the answer to the resulting drift risk is a parity test, not a layering exception.
  • Source: KD-0582 D8

Permission fingerprint is per-user, because the role union matrix is project-independent

  • Chose: One signature per user — {isAdmin, roleUnionMatrixHash, accessibleProjectIds, ownedProjectIds} — with each project's effective state derived cheaply from it.
  • Rejected: A fingerprint per (user, project) pair — the naive shape, given notifications are per project.
  • Why: CheckPermission unions role permissions with no project input, so per-project resolution collapses into a derivation — compute is O(affected users) instead of O(users × projects), even though notification fan-out stays per project.
  • Source: KD-0582 D9

Before-state is captured in the request and carried in the job payload

  • Chose: The triggering Action reads before-signatures prior to its own write and passes them into the queued job as primitives; the job recomputes "after" from live state and diffs against the carried "before".
  • Rejected: The existing notification-job pattern of re-reading current state inside handle().
  • Why: The "before" state no longer exists once the mutation commits, so the standard re-read pattern is simply unavailable here; the TOCTOU window it costs can only ever surface a real change, never a spurious one.
  • Source: KD-0582 D10

Trigger coverage and backend/frontend enum parity pinned by L1 arch tests, not code review

  • Chose: An arch test pinning every effective-permission-mutating Action to its job dispatch (mirroring CachedModelHashBumpTest), plus a frontend arch test asserting the enum collection covers the backend value set.
  • Rejected: Rely on code review to catch a forgotten dispatch or a missed enum value.
  • Why: A forgotten dispatch is invisible until a user reports a notification that never arrived, and the live NewError=8 frontend enum drift is standing proof that review does not catch parity omissions.
  • Source: KD-0582 D11

Multi-ability project tokens get a child table, not a bitmask on the existing column

  • Chose: One project_token_abilities row per (token, ability), mirroring the existing LaneGithubTrigger shape (model + enum cast + unique pair).
  • Rejected: OR-ing abilities into the existing int column; a JSON column.
  • Why: The bitmask is the smallest diff but fights Rule::enum and the enum cast, is unreadable in DB tooling, and has no precedent — and structured data in json() columns is already banned project-wide.
  • Source: KD-1044 D1

Drop the single-ability column and rename the wire field in one branch — no transition shim

  • Chose: Create + backfill + drop migrations in the same branch, plus a breaking ability: intabilities: int[] rename on the endpoint.
  • Rejected: Dual-writing the first ability for one release; a two-phase migration across releases; accepting both request shapes during a transition window.
  • Why: The SPA is the only caller of this session-authed endpoint and deploys in lockstep with the backend, so no reader survives the cutover — transition machinery would maintain a consistency invariant with no consumer. Accepted cost: a cached pre-deploy bundle gets a 422 during the deploy window.
  • Source: KD-1044 D2/D3

No edit endpoint for token abilities — mint a replacement instead

  • Chose: Tokens stay create/delete-only; changing abilities means creating a new token with the right ones and deleting the old.
  • Rejected: A PATCH that rotates the secret on ability change; moving enforcement to a DB-side ability check so abilities become editable.
  • Why: Passport bakes scopes into the JWT claims at mint time, so "edit" either silently breaks live integrations via secret rotation or rewires all three enforcement sites into a second source of truth — and no-mutation-path makes the "every token has ≥1 ability" invariant hold by construction.
  • Source: KD-1044 D4

No restrictions on which ability combinations a token may carry

  • Chose: Validate only required|array|min:1 + distinct + per-element enum — any non-empty subset is legal.
  • Rejected: Forbidding certain combinations, such as an all-three token.
  • Why: Mint permission was never per-ability, so anyone who can mint one ability can already mint all three as separate tokens today — a combination rule would restrict nothing while adding validation to maintain.
  • Source: KD-1044 D6