feat(core): native permission engine foundation (Phase 0) + console addCommand fix#3461
Conversation
There was a problem hiding this comment.
Pull request overview
Adds the Phase 0 foundation for a native, DB-backed permission engine (shared domain.action vocabulary across controllers + JSON-RPC + service layer) and fixes Symfony Console 7.4 lazy command registration so #[AsCommand] commands receive the Laravel container.
Changes:
- Introduces Core permission infrastructure (registry/discovery, repository, seeder, runtime service, attribute + enforcer) and a Tickets pilot permission catalog.
- Adds install-time schema + idempotent migration for
zp_roles,zp_permissions,zp_role_permissions, and bumpsdbVersionto 3.5.5. - Bridges Laravel Gate (
Gate::before) and wires centralized enforcement hooks in controller base classes + JSON-RPC dispatcher; fixes CLI command registration viaaddCommand()override.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| app/Domain/Tickets/Permissions/TicketsPermissions.php | Declares initial tickets.* permission vocabulary and default built-in role grants. |
| app/Domain/Install/Services/SchemaBuilder.php | Creates new permission engine tables on fresh installs. |
| app/Domain/Install/Repositories/Install.php | Adds migration 30505 to create tables + sync/seed permissions (idempotent). |
| app/Domain/Api/Controllers/Jsonrpc.php | Enforces #[RequiresPermission] before invoking JSON-RPC service methods. |
| app/Core/Domains/DomainService.php | Converts the unused CRUD interface into a marker interface. |
| app/Core/Domains/BaseService.php | Adds shared authorize()/can()/validate() helpers for service-layer adoption. |
| app/Core/Controller/HtmxController.php | Centralized permission enforcement hook before controller actions. |
| app/Core/Controller/Controller.php | Centralized permission enforcement hook before controller actions. |
| app/Core/Console/Application.php | Fixes Symfony 7.4 lazy #[AsCommand] loading by injecting Laravel container in addCommand(). |
| app/Core/Configuration/AppSettings.php | Bumps dbVersion to 3.5.5. |
| app/Core/Auth/RoleResolver.php | Adds shared effective-role resolution, including project-scoped resolution. |
| app/Core/Auth/Permissions/RequiresPermission.php | Adds attribute used to declare required permission for methods. |
| app/Core/Auth/Permissions/ProvidesPermissions.php | Defines contract for domain/plugin permission catalogs. |
| app/Core/Auth/Permissions/PermissionService.php | Implements runtime permission checks + cache, plus authorize helper. |
| app/Core/Auth/Permissions/PermissionSeeder.php | Syncs discovered vocabulary into DB and seeds built-in roles/grants. |
| app/Core/Auth/Permissions/PermissionRepository.php | Query-builder repository for roles/permissions/grants CRUD. |
| app/Core/Auth/Permissions/PermissionRegistry.php | Discovers domain/plugin permission providers and builds catalog. |
| app/Core/Auth/Permissions/PermissionEnforcer.php | Reads #[RequiresPermission] and enforces (audit vs block). |
| app/Core/Auth/Permissions/Permission.php | Immutable value object describing a capability and its defaults. |
| app/Core/Auth/AuthenticationServiceProvider.php | Registers permission services + Gate bridge to permission engine. |
| app/Command/SyncPermissionsCommand.php | Adds permissions:sync command (sync/prune/seed). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| $projectId = isset($arguments[0]) ? (int) $arguments[0] : null; | ||
|
|
||
| return $permissions->currentUserCan($ability, $projectId); |
| public function authorize(string $permissionKey, ?int $projectId = null, ?bool $forceGlobal = null): void | ||
| { | ||
| if (! $this->currentUserCan($permissionKey, $projectId, $forceGlobal)) { | ||
| throw new AuthorizationException("Not authorized: {$permissionKey}"); | ||
| } | ||
| } |
|
|
||
| return; | ||
| } | ||
|
|
||
| throw new AuthorizationException("Not authorized: {$attribute->permission}"); | ||
| } |
| protected function configure(): void | ||
| { | ||
| $this->addOption('seed', null, InputOption::VALUE_NONE, 'Also (re)grant the built-in roles their default permissions'); | ||
| $this->addOption('prune', null, InputOption::VALUE_NONE, 'Remove permissions that are no longer declared in code'); | ||
| } |
96feb71 to
76351ca
Compare
|
Reworked per review — thanks, these were all on point. Mapping each:
Verified end-to-end: a clean truncate→reseed via the new central matcher reproduces the documented role matrix exactly (28 grants). Pint + PHPStan green on all 28 files. |
76351ca to
39f7892
Compare
| #[\Override] | ||
| public function addCommand(callable|SymfonyCommand $command): ?SymfonyCommand |
| * - Web/HTMX: read centrally in {@see \Leantime\Core\Controller\Controller::callAction()} | ||
| * and {@see \Leantime\Core\Controller\HtmxController::callAction()} before the action runs. | ||
| * - JSON-RPC: read in {@see \Leantime\Domain\Api\Controllers\Jsonrpc::executeApiRequest()} | ||
| * on the resolved service method (RPC bypasses the controller gate, so this is what | ||
| * secures it). |
| * Shared by the three entry points so the declaration means the same everywhere: | ||
| * - {@see \Leantime\Core\Controller\Controller::callAction()} / HtmxController (web/HTMX), | ||
| * - {@see \Leantime\Domain\Api\Controllers\Jsonrpc::executeApiRequest()} (JSON-RPC). | ||
| * |
| @@ -993,14 +994,14 @@ public function getClientsFromProjectList(array $projects): array | |||
| * | |||
| * @api | |||
…ddCommand fix Establishes one `domain.action` permission vocabulary spoken by JSON-RPC, controllers, and the service layer, backed by a native DB-backed engine (no Spatie, no Eloquent). Additive and inert by default: enforcement is attribute-driven and audit/log-only until a method is annotated, so this PR changes no runtime authorization behavior on its own. Engine (app/Core/Auth, registered via a dedicated PermissionServiceProvider): - RoleResolver — effective-role resolution incl. project-scoped (mirrors Tickets::userIsAtLeastForProject), so authorization resolves against the entity's project, not the session project. - Permissions/PermissionService — currentUserCan()/authorize(); cached role->permission map; capability kept distinct from project data-access. - PermissionRepository (query builder, no ORM); PermissionRegistry (auto-discovers Domain/*/Permissions, like register.php). - All dependencies are constructor-injected — no app() helper in engine classes. The engine depends on a narrow ChecksProjectAccess contract (Core), implemented by the Projects service and bound in the provider. Vocabulary + role assignment (separated, Spatie-style): - A domain's permission class declares only its VERBS (TicketsPermissions: view/comment/upload/create/edit/delete) — no role information. - DefaultRolePermissions is the single central place the six built-in roles' default grants are defined (matched against the catalog by scope + verb, unioned up the hierarchy). After install, zp_role_permissions / the future admin UI own assignments; domains are never touched to re-assign. Enforcement (covers both routing paths, no base-class coupling): - #[RequiresPermission] read by a shared PermissionEnforcer, injected into Frontcontroller::executeAction (legacy convention path) and applied to all native Laravel routes via a CheckPermissions middleware wired in RouteLoader. JSON-RPC enforces it in Jsonrpc (enforcer injected via init()). Denial throws the existing AuthorizationException (403 web / RPC -32001). Service contract: - DomainService rewritten from the dead CRUD interface to a thin marker; new BaseService (authorize/can/validate helpers). BaseService gets its PermissionService via the provider's afterResolving hook — zero subclass constructor boilerplate, no app(). Templates: @can('domain.action'[, $projectId]) works via a Gate::before bridge; a can() helper covers legacy .tpl.php. Schema: zp_roles / zp_permissions / zp_role_permissions via SchemaBuilder + idempotent Install::update_sql_30505 + dbVersion 3.5.5. User->role stays on zp_user.role (no data migration); zp_roles.level preserves the legacy hierarchy. Console fix (separable bug): symfony/console 7.4 renamed add() to addCommand() and routes lazy #[AsCommand] loading through addCommand(), which laravel/framework 11.45 does not override (setLaravel() lives only in add()). Every #[AsCommand] command crashed with "make() on null". Overriding addCommand() in Core/Console/Application injects the container. Fixes cache:clearAll, db:migrate, permissions:sync, etc. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
39f7892 to
eccbf33
Compare
|
Thanks @copilot — addressed all of these in the latest push:
Verified locally: Pint + PHPStan clean, |
… engine Wires the native permission engine through two domains end-to-end, proving one `domain.action` vocabulary across JSON-RPC, controllers, services, and the UI. Stacked on the engine foundation (#3461). Enforcement is on by default (LEAN_PERMISSIONS_ENFORCE); set it false for audit/log-only. Comments (ownership + RPC): - CommentsPermissions verbs: view / create / moderate. `moderate` is a distinct verb so editing someone else's comment stays manager+ (the matrix would seed a comments.edit at editor+, which would over-grant). - Comments extends BaseService. addComment now authorizes comments.create (was access-only, so any assigned role could comment over RPC). canModifyComment is author OR comments.moderate — exact author-or-manager+ behavior preserved. Tickets (RPC + controller): - Tickets extends BaseService. @api methods migrated to in-method $this->authorize('tickets.*', $entityProjectId): patchTicket (hardened from a session-scoped role check to the ticket's project role), and addTicket / delete / deleteMilestone (which had NO role check — closes RPC holes; matches the documented "editor+ creates/deletes" rule). - Controllers DelTicket / DelMilestone / NewTicket / MoveTicket replace Auth::authOrRedirect with #[RequiresPermission] (tickets.delete/create/edit). Role assignment stays central (no role info in domains): DefaultRolePermissions gains explicit-key grants so commenters can create comments (the one case the verb convention doesn't fit). UI: representative templates migrated to @can / can() — the comment edit/delete buttons (`(author) || can('comments.moderate')`), the new-todo button (`can('tickets.create')`), and the kanban edit gate. Remaining template checks still work via the legacy $login helper (mechanical follow-up). Config + tests: LEAN_PERMISSIONS_ENFORCE (DefaultConfig + sample.env); DefaultRolePermissionsTest locks the built-in role→permission matrix (grant- equivalence guard); TicketsServiceTest patch-denial updated for the engine. Verified: full unit suite green (409), pint + phpstan clean, and a live truncate→seed reproduces the matrix (comments.create→commenter+, comments.moderate→manager+). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… engine Wires the native permission engine through two domains end-to-end, proving one `domain.action` vocabulary across JSON-RPC, controllers, services, and the UI. Stacked on the engine foundation (#3461). Enforcement is on by default (LEAN_PERMISSIONS_ENFORCE); set it false for audit/log-only. Comments (ownership + RPC): - CommentsPermissions verbs: view / create / moderate. `moderate` is a distinct verb so editing someone else's comment stays manager+ (the matrix would seed a comments.edit at editor+, which would over-grant). - Comments extends BaseService. addComment now authorizes comments.create (was access-only, so any assigned role could comment over RPC). canModifyComment is author OR comments.moderate — exact author-or-manager+ behavior preserved. Tickets (RPC + controller): - Tickets extends BaseService. @api methods migrated to in-method $this->authorize('tickets.*', $entityProjectId): patchTicket (hardened from a session-scoped role check to the ticket's project role), and addTicket / delete / deleteMilestone (which had NO role check — closes RPC holes; matches the documented "editor+ creates/deletes" rule). - Controllers DelTicket / DelMilestone / NewTicket / MoveTicket replace Auth::authOrRedirect with #[RequiresPermission] (tickets.delete/create/edit). Role assignment stays central (no role info in domains): DefaultRolePermissions gains explicit-key grants so commenters can create comments (the one case the verb convention doesn't fit). UI: representative templates migrated to @can / can() — the comment edit/delete buttons (`(author) || can('comments.moderate')`), the new-todo button (`can('tickets.create')`), and the kanban edit gate. Remaining template checks still work via the legacy $login helper (mechanical follow-up). Config + tests: LEAN_PERMISSIONS_ENFORCE (DefaultConfig + sample.env); DefaultRolePermissionsTest locks the built-in role→permission matrix (grant- equivalence guard); TicketsServiceTest patch-denial updated for the engine. Verified: full unit suite green (409), pint + phpstan clean, and a live truncate→seed reproduces the matrix (comments.create→commenter+, comments.moderate→manager+). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…api coverage (#3469) * feat(permissions): Phase 1 pilot — enforce Tickets + Comments via the engine Wires the native permission engine through two domains end-to-end, proving one `domain.action` vocabulary across JSON-RPC, controllers, services, and the UI. Stacked on the engine foundation (#3461). Enforcement is on by default (LEAN_PERMISSIONS_ENFORCE); set it false for audit/log-only. Comments (ownership + RPC): - CommentsPermissions verbs: view / create / moderate. `moderate` is a distinct verb so editing someone else's comment stays manager+ (the matrix would seed a comments.edit at editor+, which would over-grant). - Comments extends BaseService. addComment now authorizes comments.create (was access-only, so any assigned role could comment over RPC). canModifyComment is author OR comments.moderate — exact author-or-manager+ behavior preserved. Tickets (RPC + controller): - Tickets extends BaseService. @api methods migrated to in-method $this->authorize('tickets.*', $entityProjectId): patchTicket (hardened from a session-scoped role check to the ticket's project role), and addTicket / delete / deleteMilestone (which had NO role check — closes RPC holes; matches the documented "editor+ creates/deletes" rule). - Controllers DelTicket / DelMilestone / NewTicket / MoveTicket replace Auth::authOrRedirect with #[RequiresPermission] (tickets.delete/create/edit). Role assignment stays central (no role info in domains): DefaultRolePermissions gains explicit-key grants so commenters can create comments (the one case the verb convention doesn't fit). UI: representative templates migrated to @can / can() — the comment edit/delete buttons (`(author) || can('comments.moderate')`), the new-todo button (`can('tickets.create')`), and the kanban edit gate. Remaining template checks still work via the legacy $login helper (mechanical follow-up). Config + tests: LEAN_PERMISSIONS_ENFORCE (DefaultConfig + sample.env); DefaultRolePermissionsTest locks the built-in role→permission matrix (grant- equivalence guard); TicketsServiceTest patch-denial updated for the engine. Verified: full unit suite green (409), pint + phpstan clean, and a live truncate→seed reproduces the matrix (comments.create→commenter+, comments.moderate→manager+). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(permissions): full @api coverage on Tickets + Comments services Make the declarative #[RequiresPermission] attribute the standard on every @api service method (the JSON-RPC surface), closing the gap where only a few methods were gated. Enforcer/attribute: - RequiresPermission gains `global` (company-wide capability — check the global role, not a project) and `entityScoped` (project comes from an entity the method loads; the enforcer defers and the method self-authorizes in its body). - PermissionEnforcer honors both modes: entityScoped returns without a check; global calls currentUserCan(perm, null, forceGlobal: true). Tickets (Domain/Tickets/Services/Tickets): - Closed 9 ungated mutator RPC holes with in-method authorize(): quickAddTicket, quickAddMilestone, quickAddTicketFromKanban, createMilestoneFromDialog, upsertSubtask, addMilestoneComment, saveStatusLabels, markTicketReopen, moveTicket. Project resolved from the param/loaded entity, not the session. - Added entityScoped coverage attributes to the already-gated mutators (addTicket, updateTicket, patchTicket, quick/updateMilestone*, sortTickets, updateTicketStatusAndSorting, delete, deleteMilestone). - Attributed every @api read with tickets.view (projectIdParam where the project is a clean top-level argument, session-scoped otherwise). - De-@api'd 9 cron/helper methods that are not user actions (getNewMilestone, normalizeRoadmapParams, getMilestonesOverviewSearchCriteria, prepareTicketDates, pollFor{New,Updated}Account{Milestones,Todos}); fixed a duplicate @api tag. Comments: getComments/getCommentReactions -> comments.view; addComment + edit/deleteComment carry entityScoped markers (they self-authorize via the project / canModifyComment); toggleCommentReaction -> comments.create; pollComments de-@api'd (cron/poll helper). Tests: - New PermissionEnforcerTest locks the four resolution modes (entityScoped defers, global forces the company role, projectIdParam reads the named arg, default falls back to the session project) + the no-attribute no-op. - TicketsServiceTest: quickAddTicket denial proves a newly-gated leaf mutator throws via the engine. - JsonrpcTest repointed from the (now de-@api'd) pollComments fixture to the stable @api getComments read. Full unit suite green (415 tests, 990 assertions); pint + phpstan clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(permissions): keep poll* methods @api as gated reads (acceptance fix) The earlier pass bucketed the poll* methods in with genuine helpers and de-@api'd them. They are actually legitimate public API reads (mobile account polling) — both the unit JsonrpcTest and the acceptance ApiCest use leantime.rpc.Comments.pollComments as their canonical RPC fixture, so dropping it from the API surface returned -32601 and failed ApiCest::testJsonRpcEndpoint (+ StringId). Restore @api on the 5 poll methods and gate them as reads instead: - Comments::pollComments -> comments.view (projectIdParam: 'projectId') - Tickets::pollFor{New,Updated}Account{Milestones,Todos} -> tickets.view (projectIdParam: 'projectId') Only the 4 genuine internal helpers stay de-@api'd (getNewMilestone, normalizeRoadmapParams, getMilestonesOverviewSearchCriteria, prepareTicketDates). JsonrpcTest reverted to its original pollComments fixture (valid again). Unit suite green (415 tests, 990 assertions); pint + phpstan clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Summary
Phase 0 (foundation) of the service-layer / permission overhaul: a native, DB-backed permission engine (no Spatie, no Eloquent) establishing one
domain.actionvocabulary spoken by JSON-RPC, controllers, and the service layer, plus a thin service contract.Design
Permission vocabulary vs. assignment are separated (Spatie-style).
TicketsPermissions:view/comment/upload/create/edit/delete. No role information.Core/Auth/Permissions/DefaultRolePermissionsis the single central place the six built-in roles' default grants live (matched against the catalog by scope + verb, unioned up the hierarchy). After install,zp_role_permissions/ the future admin UI own assignments — domains are never touched to re-assign.Engine —
app/Core/Auth/, wired by a dedicatedPermissionServiceProvider.RoleResolver— effective-role resolution incl. project-scoped (mirrorsTickets::userIsAtLeastForProject), so authz resolves against the entity's project, not the session project.PermissionService—currentUserCan()/authorize(); cached role→permission map; capability kept distinct from project data-access.app()in engine classes. The engine depends on a narrowChecksProjectAccesscontract (Core), implemented byProjectsand bound in the provider.Enforcement covers both routing paths, with no base-class coupling.
#[RequiresPermission]is read by a sharedPermissionEnforcer, injected intoFrontcontroller::executeAction(legacy convention path) and applied to all native Laravel routes via aCheckPermissionsmiddleware wired inRouteLoader. JSON-RPC enforces it inJsonrpc(enforcer injected viainit()). Denial throws the existingAuthorizationException→ 403 web / RPC-32001.Service contract.
DomainService→ thin marker; newBaseService(authorize/can/validate).BaseServicereceivesPermissionServicevia the provider'safterResolvinghook — zero subclass constructor boilerplate, noapp().Templates.
@can('tickets.edit', $projectId)works via aGate::beforebridge; acan()helper covers legacy.tpl.php.Schema.
zp_roles/zp_permissions/zp_role_permissionsviaSchemaBuilder+ idempotentInstall::update_sql_30505+dbVersion 3.5.5. User→role stays onzp_user.role(no data migration);zp_roles.levelpreserves the legacy hierarchy.Console bugfix (separable — affects ALL
#[AsCommand]commands)symfony/console7.4 renamedadd()→addCommand()and routes lazy#[AsCommand]loading throughaddCommand();laravel/framework11.45 only overrides the deprecatedadd()(where the solesetLaravel()lives). Every lazily-loaded command (cache:clearAll,db:migrate,permissions:sync) crashed withmake() on null. Fixed by overridingaddCommand()inCore/Console/Application.Verification
db:migraterunsupdate_sql_30505;permissions:sync --seedworks.DefaultRolePermissionsproduces the documented matrix from scratch: 6 roles (levels 5→50), 6 permissions, 28 grants;tickets.view→ all roles,tickets.create/delete→ editor+.Notes for reviewers
30505(afterorigin/master's30504;30503was intentionally burned by the device-tokens work).BaseServiceadoption across existing services is a mechanical follow-up sweep (single-inheritance conflicts, e.g. Canvas variants, handled there); this PR establishes the base + DI.🤖 Generated with Claude Code