diff --git a/doc/migrations/authorization-model.md b/doc/migrations/authorization-model.md index 13d7a3330..075941516 100644 --- a/doc/migrations/authorization-model.md +++ b/doc/migrations/authorization-model.md @@ -92,8 +92,8 @@ that leaf does not let them delegate the subtree. ## The legacy permission constant classes are gone The `Permissions` classes holding `verb:resource` strings — `AIPermissions`, `ConsoleLogsPermissions`, -`DashboardPermissions`, `ExternalAuthenticationPermissions`, `OpenTelemetryPermissions`, `SecretsPermissions` -and `StructuredLogsPermissions` — are removed rather than marked obsolete. Referencing one is now a compile +`DashboardPermissions`, `ExternalAuthenticationPermissions`, `OpenTelemetryPermissions`, `SecretsPermissions`, +`StructuredLogsPermissions` and `UserTasksPermissions` — are removed rather than marked obsolete. Referencing one is now a compile error, which is deliberate: every string they held is unparseable under the `{resource}:{verb}` grammar, so keeping them would leave code that still compiles, still reads as a permission check, and silently authorizes nothing. A compile error names the site and can be fixed against the mapping table below; an obsolete constant @@ -123,6 +123,39 @@ the policy away from one that creates users, which drops its roles just as surel needs nothing extra, so an administrator without the permission can still edit other fields on a connection whose default roles someone else configured, enable it, or validate it. +## User tasks join the structured vocabulary + +User Tasks was still declaring access through the legacy channel after the rest of the codebase had moved, so its +nine permissions are re-authored in this release: `read:user-tasks` and its siblings become verbs on a `user-tasks` +resource, and participant lookup becomes the sub-resource `user-tasks/participants`. + +**This is a breaking change for anyone granting the legacy strings**, which is everyone who granted User Tasks +anything — they were the only spelling that ever worked. Rewrite them from the table below. The strings do not +merely stop matching new-style grants; they were being compared as exact claim values, so nothing else had ever +matched them either. + +**Pattern grants now reach these endpoints for the first time.** Under the legacy declaration a claim had to equal +the required string character for character, so `*:view`, `user-tasks:*` and `user-tasks/*:view` all failed against +every User Tasks endpoint even though they read as though they covered it. A bare `*` worked, because it was +special-cased. If you worked around this by granting the exact legacy strings alongside a pattern, the pattern is +now doing the work and the legacy strings can go. + +**`manage:user-tasks` becomes `user-tasks:supervise`, not `user-tasks:manage`.** The permission never was an +aggregate — it grants tenant-wide oversight (read every task, assign, reschedule, cancel, see blocked tasks, retry +a failed resolution) and confers none of `claim`, `complete`, `assign`, `cancel` or `invite`. It is renamed for the +same reason `workflows/runtime:control` is not called `manage`: a name that reads like an aggregate invites being +granted as one. + +**Recorded consequence:** because participant lookup is a sub-resource, `user-tasks/*:view` now grants it along +with task read, where legacy `read:user-tasks` did not. Participant lookup returns a tenant-scoped directory of +users and groups, so a role that should read tasks without enumerating the directory must name `user-tasks:view` +rather than the subtree. + +If you implement `IUserTaskAccessPolicy` or construct `UserTaskActor` yourself: `UserTaskActor.HasPermission` now +takes a `Permission` (or a resource and verb) instead of a single string, and matches through `PermissionMatcher` +rather than by equality, so pattern grants reach your policy too. `UserTaskActor.Permissions` is compared ordinally +rather than case-insensitively, matching the rest of the model. + ## Third-party modules Modules outside this repository keep compiling. `ConfigurePermissions(params string[])` remains available but obsolete, and a permission that resolves to no registered descriptor registers an implicit one marked unverified, logs a warning, and appears as such in the catalog. The module keeps working and the gap stays visible. @@ -208,6 +241,15 @@ One caveat: the composite indexes only cover rows whose `TenantId` is non-null ( | `external-authentication:provider-trust:unsafe` | `external-authentication/provider-trust:override` | | `external-authentication:permissions:delegate` | `external-authentication/permission-grants:delegate` | | `external-authentication:permissions:delegate-unrestricted` | `external-authentication/permission-grants:delegate-unrestricted` | +| `read:user-tasks` | `user-tasks:view` | +| `claim:user-tasks` | `user-tasks:claim` | +| `complete:user-tasks` | `user-tasks:complete` | +| `assign:user-tasks` | `user-tasks:assign` | +| `update:user-tasks` | `user-tasks:update` | +| `cancel:user-tasks` | `user-tasks:cancel` | +| `invite:user-tasks` | `user-tasks:invite` | +| `manage:user-tasks` | `user-tasks:supervise` | +| `lookup:user-task-participants` | `user-tasks/participants:view` | | `read:dashboard` | `dashboard:view` | | `read:diagnostics:console-logs` | `diagnostics/console-logs:view` | | `read:diagnostics:structured-logs` | `diagnostics/structured-logs:view` | diff --git a/specs/013-rbac-authorization-model/contracts/permissions.md b/specs/013-rbac-authorization-model/contracts/permissions.md index 4bd167797..9e45bd7d0 100644 --- a/specs/013-rbac-authorization-model/contracts/permissions.md +++ b/specs/013-rbac-authorization-model/contracts/permissions.md @@ -116,6 +116,23 @@ BPMN interchange reuses `workflows/definitions`: analyze and export require `vie `policies/default-roles` is a sub-resource because the default-role list — the roles granted to a user auto-created for an unknown external identity — genuinely is a distinct thing being administered. +### User Tasks + +Added after the original census: User Tasks was still on the legacy channel when that census was taken, and was migrated separately in [#7993](https://github.com/elsa-workflows/elsa-core/issues/7993). + +| Resource | Verbs | +| --- | --- | +| `user-tasks` | view, update, claim★, complete★, assign★, cancel★, invite★, supervise★ | +| `user-tasks/participants` | view | + +`update` is core rather than a module verb because the only thing it modifies is a task's scheduling — priority and due date. There is no `create`: tasks come into existence from a workflow, never from the API, so the never-both rule is satisfied by `create` simply not existing. + +`claim` covers claiming and releasing, as `claim:user-tasks` did: releasing is undoing a claim, not a second thing to administer. `invite` likewise covers issuing, listing and revoking guest invitations, matching the module's own access policy, which treats all three as one supervisory act. + +`supervise` is the elevated tier — read every task in the tenant, assign, reschedule, cancel, see blocked tasks, and retry a failed resolution — and is deliberately not named `manage`, for the reason `workflows/runtime:control` is not: it implies none of the verbs beside it, and a name that reads like an aggregate invites being granted as one. + +`user-tasks/participants` is a sub-resource because it is a directory search backing the assignee picker, and a role that reads tasks does not necessarily enumerate the directory. **Recorded consequence:** the subtree grant `user-tasks/*:view` does confer both, so withholding the directory means naming `user-tasks:view` rather than the subtree. This is the same shape as outcome 3 below. + ### Diagnostics, dashboard, and operations | Resource | Verbs | @@ -222,6 +239,15 @@ The source for [`doc/migrations/authorization-model.md`](../../../doc/migrations | `external-authentication:provider-trust:unsafe` | `external-authentication/provider-trust:override` | | `external-authentication:permissions:delegate` | `external-authentication/permission-grants:delegate` | | `external-authentication:permissions:delegate-unrestricted` | `external-authentication/permission-grants:delegate-unrestricted` | +| `read:user-tasks` | `user-tasks:view` | +| `claim:user-tasks` | `user-tasks:claim` | +| `complete:user-tasks` | `user-tasks:complete` | +| `assign:user-tasks` | `user-tasks:assign` | +| `update:user-tasks` | `user-tasks:update` | +| `cancel:user-tasks` | `user-tasks:cancel` | +| `invite:user-tasks` | `user-tasks:invite` | +| `manage:user-tasks` | `user-tasks:supervise` | +| `lookup:user-task-participants` | `user-tasks/participants:view` | | `read:dashboard` | `dashboard:view` | | `read:diagnostics:console-logs` | `diagnostics/console-logs:view` | | `read:diagnostics:structured-logs` | `diagnostics/structured-logs:view` | diff --git a/specs/013-user-tasks/contracts/identity-contract.md b/specs/013-user-tasks/contracts/identity-contract.md index ae60a3f34..9c8eb6911 100644 --- a/specs/013-user-tasks/contracts/identity-contract.md +++ b/specs/013-user-tasks/contracts/identity-contract.md @@ -16,12 +16,12 @@ The built-in identity resolver maps namespaced `ClaimsPrincipal` claims. All int - `Live` (default): candidate group checks use current principal references/claims. An exact reference match remains authoritative even if the directory cannot resolve it. - `Snapshot`: groups are expanded at activation and stored as snapshot members. Failure to enumerate creates a blocking manager-only health issue. -- Explicit user exclusions deny assignment and protected access. Manager override is disabled by default; when enabled it requires `manage:user-tasks` and a reason. +- Explicit user exclusions deny assignment and protected access. Manager override is disabled by default; when enabled it requires `user-tasks:supervise` and a reason. - `Requester` is context only and grants no capability. ## Operation permissions -`read:user-tasks`, `claim:user-tasks`, `complete:user-tasks`, `assign:user-tasks`, `update:user-tasks`, `cancel:user-tasks`, `invite:user-tasks`, `manage:user-tasks`, and `lookup:user-task-participants` are independent permissions. Permission alone is insufficient: ordinary callers also need the corresponding task relationship. Managers are tenant-scoped. Guest sessions carry an allowlist of capabilities for one task. +`user-tasks:view`, `:claim`, `:complete`, `:assign`, `:update`, `:cancel`, `:invite`, `:supervise`, and `user-tasks/participants:view` are independent permissions; no verb implies another. Permission alone is insufficient: ordinary callers also need the corresponding task relationship. Managers are tenant-scoped. Guest sessions carry an allowlist of capabilities for one task. ## Disclosure diff --git a/specs/013-user-tasks/contracts/rest-api.md b/specs/013-user-tasks/contracts/rest-api.md index 91d2cf81a..5f13d96f9 100644 --- a/specs/013-user-tasks/contracts/rest-api.md +++ b/specs/013-user-tasks/contracts/rest-api.md @@ -16,7 +16,7 @@ Base path: `/user-tasks`. Authenticated endpoints use Elsa endpoint permission m } ``` -The descriptor is advisory: it decides what a client renders, never what the server allows. It is gated on `read:user-tasks`, so an actor without read access receives `403` and clients treat that as "hide the feature". +The descriptor is advisory: it decides what a client renders, never what the server allows. It is gated on `user-tasks:view`, so an actor without read access receives `403` and clients treat that as "hide the feature". `GET /user-tasks/{id}/capabilities` returns the per-task projection: `taskId`, `revision`, `allowedActions`, `canReadProtected`, and `canManage`. @@ -36,7 +36,7 @@ Items contain only the safe summary and the caller's capabilities: id, title, su `POST /user-tasks/{id}/reveal` with `{ fieldKey }` discloses one masked form field. The field must be marked `masked` and `canReveal` by its form provider, the caller must hold protected access, and the reveal is written to the audit trail. Masked values are never included in the ordinary detail response. -`GET /user-task-participants?search=&type=&cursor=&limit=` requires `lookup:user-task-participants`; absence of a directory returns an empty page, never an identity-module error. +`GET /user-task-participants?search=&type=&cursor=&limit=` requires `user-tasks/participants:view`; absence of a directory returns an empty page, never an identity-module error. ## Commands diff --git a/specs/013-user-tasks/contracts/studio-contract.md b/specs/013-user-tasks/contracts/studio-contract.md index 4ea58bf00..2a4c878bf 100644 --- a/specs/013-user-tasks/contracts/studio-contract.md +++ b/specs/013-user-tasks/contracts/studio-contract.md @@ -31,11 +31,11 @@ The default route opens the **Assigned to me** tab. Tabs are represented in the | Tab | Query value | Purpose | Required capability | | --- | --- | --- | --- | -| Assigned to me | `tab=assigned` | Tasks assigned to the current actor, including claimed tasks | `read:user-tasks` | -| Available | `tab=available` | Unassigned tasks for which the actor is an eligible candidate | `read:user-tasks` | -| History | `tab=history` | Terminal tasks completed by the actor and safe history for tasks they acted on | `read:user-tasks` | -| All | `tab=all` | Tenant-visible tasks for operations and support | `manage:user-tasks` | -| Needs Attention | `tab=needs-attention` | Unassigned, blocked-health, overdue, and stale-operation tasks | `manage:user-tasks` | +| Assigned to me | `tab=assigned` | Tasks assigned to the current actor, including claimed tasks | `user-tasks:view` | +| Available | `tab=available` | Unassigned tasks for which the actor is an eligible candidate | `user-tasks:view` | +| History | `tab=history` | Terminal tasks completed by the actor and safe history for tasks they acted on | `user-tasks:view` | +| All | `tab=all` | Tenant-visible tasks for operations and support | `user-tasks:supervise` | +| Needs Attention | `tab=needs-attention` | Unassigned, blocked-health, overdue, and stale-operation tasks | `user-tasks:supervise` | Tabs that are not allowed by the capability response are hidden. A deep link to a hidden tab displays the permitted default tab rather than an authorization error from the UI. The API remains authoritative. diff --git a/specs/013-user-tasks/quickstart.md b/specs/013-user-tasks/quickstart.md index 4ce8df66b..06631f129 100644 --- a/specs/013-user-tasks/quickstart.md +++ b/specs/013-user-tasks/quickstart.md @@ -9,7 +9,7 @@ This walkthrough creates a workflow-bound invoice approval, executes it, lists t - An Elsa Server with workflow management, workflow runtime, and the User Tasks module enabled. - A host identity provider that issues a bearer token with a stable subject and optional group claims. -- The `read:user-tasks`, `claim:user-tasks`, and `complete:user-tasks` permissions for the worker token. A manager token additionally needs `assign:user-tasks` and `manage:user-tasks`. +- The `user-tasks:view`, `user-tasks:claim`, and `user-tasks:complete` permissions for the worker token. A manager token additionally needs `user-tasks:assign` and `user-tasks:supervise`. - An installed form provider if the task uses `formReference`. The Core module defaults to an in-memory repository for development and tests. A durable host adds the User Tasks EF Core persistence package and its provider-specific shell feature, following the same package split used by `Elsa.Secrets`: @@ -236,7 +236,7 @@ curl --fail-with-body \ ## Inspect the manager queue and audit history -Managers with `manage:user-tasks` can query all task scopes and see the safe event timeline: +Managers with `user-tasks:supervise` can query all task scopes and see the safe event timeline: ```bash curl --fail-with-body \ diff --git a/src/modules/Elsa.UserTasks/Endpoints/UserTasksEndpoints.cs b/src/modules/Elsa.UserTasks/Endpoints/UserTasksEndpoints.cs index 2bb30f9ff..ae85bd66b 100644 --- a/src/modules/Elsa.UserTasks/Endpoints/UserTasksEndpoints.cs +++ b/src/modules/Elsa.UserTasks/Endpoints/UserTasksEndpoints.cs @@ -1,5 +1,6 @@ using System.Text.Json; using Elsa.Abstractions; +using Elsa.Authorization; using Elsa.Common; using Elsa.UserTasks.Contracts; using Elsa.UserTasks.Models; @@ -84,7 +85,7 @@ internal sealed class FeatureCapabilitiesEndpoint(IUserTaskIdentityResolver iden public override void Configure() { Get("/user-tasks/capabilities"); - ConfigurePermissions(UserTasksPermissions.Read); + RequirePermission(UserTasksResourcePermissions.UserTasks, CoreVerbs.View); } public override async Task HandleAsync(CancellationToken cancellationToken) @@ -97,23 +98,24 @@ internal sealed class FeatureCapabilitiesEndpoint(IUserTaskIdentityResolver iden } var settings = options.Value; - var isManager = actor.IsManager && actor.HasPermission(UserTasksPermissions.Manage); + bool Holds(string verb) => actor.HasPermission(UserTasksResourcePermissions.UserTasks, verb); + var isManager = actor.IsManager && Holds(UserTaskVerbs.Supervise); // The descriptor is advisory: it decides what the client renders, never what the server allows. await Send.OkAsync(new UserTaskFeatureCapabilities { Enabled = true, - CanList = actor.HasPermission(UserTasksPermissions.Read), - CanRead = actor.HasPermission(UserTasksPermissions.Read), + CanList = Holds(CoreVerbs.View), + CanRead = Holds(CoreVerbs.View), CanReadAll = isManager, - CanClaim = actor.HasPermission(UserTasksPermissions.Claim), - CanRelease = actor.HasPermission(UserTasksPermissions.Claim), - CanComplete = actor.HasPermission(UserTasksPermissions.Complete), - CanAssign = actor.HasPermission(UserTasksPermissions.Assign), - CanUpdate = actor.HasPermission(UserTasksPermissions.Update), - CanCancel = actor.HasPermission(UserTasksPermissions.Cancel), - CanCreateGuestLinks = actor.HasPermission(UserTasksPermissions.Invite), - CanViewProtected = actor.HasPermission(UserTasksPermissions.Read), - ParticipantPicker = actor.HasPermission(UserTasksPermissions.LookupParticipants) && directory is not EmptyUserTaskParticipantDirectory, + CanClaim = Holds(UserTaskVerbs.Claim), + CanRelease = Holds(UserTaskVerbs.Claim), + CanComplete = Holds(UserTaskVerbs.Complete), + CanAssign = Holds(UserTaskVerbs.Assign), + CanUpdate = Holds(CoreVerbs.Update), + CanCancel = Holds(UserTaskVerbs.Cancel), + CanCreateGuestLinks = Holds(UserTaskVerbs.Invite), + CanViewProtected = Holds(CoreVerbs.View), + ParticipantPicker = actor.HasPermission(UserTasksResourcePermissions.Participants, CoreVerbs.View) && directory is not EmptyUserTaskParticipantDirectory, Realtime = settings.RealtimeEnabled, PollingIntervalSeconds = settings.PollingIntervalSeconds }, cancellationToken); @@ -126,7 +128,7 @@ internal sealed class ListEndpoint(IUserTaskManager manager, IUserTaskIdentityRe public override void Configure() { Get("/user-tasks"); - ConfigurePermissions(UserTasksPermissions.Read); + RequirePermission(UserTasksResourcePermissions.UserTasks, CoreVerbs.View); } public override async Task HandleAsync(ListUserTasksRequest request, CancellationToken cancellationToken) @@ -178,7 +180,7 @@ internal sealed class GetEndpoint(IUserTaskManager manager, IUserTaskIdentityRes public override void Configure() { Get("/user-tasks/{taskId}"); - ConfigurePermissions(UserTasksPermissions.Read); + RequirePermission(UserTasksResourcePermissions.UserTasks, CoreVerbs.View); } public override async Task HandleAsync(CancellationToken cancellationToken) @@ -201,7 +203,7 @@ internal sealed class CapabilitiesEndpoint(IUserTaskManager manager, IUserTaskId public override void Configure() { Get("/user-tasks/{taskId}/capabilities"); - ConfigurePermissions(UserTasksPermissions.Read); + RequirePermission(UserTasksResourcePermissions.UserTasks, CoreVerbs.View); } public override async Task HandleAsync(CancellationToken cancellationToken) @@ -222,7 +224,7 @@ internal sealed class ListEventsEndpoint(IUserTaskManager manager, IUserTaskIden public override void Configure() { Get("/user-tasks/{taskId}/events"); - ConfigurePermissions(UserTasksPermissions.Read); + RequirePermission(UserTasksResourcePermissions.UserTasks, CoreVerbs.View); } public override async Task HandleAsync(CancellationToken cancellationToken) @@ -247,7 +249,7 @@ internal sealed class ClaimEndpoint(IUserTaskManager manager, IUserTaskIdentityR public override void Configure() { Post("/user-tasks/{taskId}/claim"); - ConfigurePermissions(UserTasksPermissions.Claim); + RequirePermission(UserTasksResourcePermissions.UserTasks, UserTaskVerbs.Claim); } public override async Task HandleAsync(UserTaskMutationApiRequest request, CancellationToken cancellationToken) @@ -269,7 +271,7 @@ internal sealed class ReleaseEndpoint(IUserTaskManager manager, IUserTaskIdentit public override void Configure() { Post("/user-tasks/{taskId}/release"); - ConfigurePermissions(UserTasksPermissions.Claim); + RequirePermission(UserTasksResourcePermissions.UserTasks, UserTaskVerbs.Claim); } public override async Task HandleAsync(UserTaskMutationApiRequest request, CancellationToken cancellationToken) @@ -291,7 +293,7 @@ internal sealed class AssignEndpoint(IUserTaskManager manager, IUserTaskIdentity public override void Configure() { Post("/user-tasks/{taskId}/assign"); - ConfigurePermissions(UserTasksPermissions.Assign); + RequirePermission(UserTasksResourcePermissions.UserTasks, UserTaskVerbs.Assign); } public override async Task HandleAsync(AssignUserTaskApiRequest request, CancellationToken cancellationToken) @@ -328,7 +330,7 @@ internal sealed class ScheduleEndpoint(IUserTaskManager manager, IUserTaskIdenti public override void Configure() { Patch("/user-tasks/{taskId}"); - ConfigurePermissions(UserTasksPermissions.Update); + RequirePermission(UserTasksResourcePermissions.UserTasks, CoreVerbs.Update); } public override async Task HandleAsync(ScheduleUserTaskApiRequest request, CancellationToken cancellationToken) @@ -351,7 +353,7 @@ internal sealed class CompleteEndpoint(IUserTaskManager manager, IUserTaskIdenti public override void Configure() { Post("/user-tasks/{taskId}/complete"); - ConfigurePermissions(UserTasksPermissions.Complete); + RequirePermission(UserTasksResourcePermissions.UserTasks, UserTaskVerbs.Complete); } public override async Task HandleAsync(CompleteUserTaskApiRequest request, CancellationToken cancellationToken) @@ -379,7 +381,7 @@ internal sealed class CancelEndpoint(IUserTaskManager manager, IUserTaskIdentity public override void Configure() { Post("/user-tasks/{taskId}/cancel"); - ConfigurePermissions(UserTasksPermissions.Cancel); + RequirePermission(UserTasksResourcePermissions.UserTasks, UserTaskVerbs.Cancel); } public override async Task HandleAsync(CancelUserTaskApiRequest request, CancellationToken cancellationToken) @@ -402,7 +404,7 @@ internal sealed class RetryResolutionEndpoint(IUserTaskManager manager, IUserTas public override void Configure() { Post("/user-tasks/{taskId}/retry-resolution"); - ConfigurePermissions(UserTasksPermissions.Manage); + RequirePermission(UserTasksResourcePermissions.UserTasks, UserTaskVerbs.Supervise); } public override async Task HandleAsync(UserTaskMutationApiRequest request, CancellationToken cancellationToken) @@ -424,7 +426,7 @@ internal sealed class RevealFieldEndpoint(IUserTaskManager manager, IUserTaskIde public override void Configure() { Post("/user-tasks/{taskId}/reveal"); - ConfigurePermissions(UserTasksPermissions.Read); + RequirePermission(UserTasksResourcePermissions.UserTasks, CoreVerbs.View); } public override async Task HandleAsync(RevealUserTaskFieldApiRequest request, CancellationToken cancellationToken) @@ -448,7 +450,7 @@ internal sealed class IssueInvitationEndpoint(IUserTaskInvitationService invitat public override void Configure() { Post("/user-tasks/{taskId}/invitations"); - ConfigurePermissions(UserTasksPermissions.Invite); + RequirePermission(UserTasksResourcePermissions.UserTasks, UserTaskVerbs.Invite); } public override async Task HandleAsync(IssueUserTaskInvitationApiRequest request, CancellationToken cancellationToken) @@ -479,7 +481,7 @@ internal sealed class ListInvitationsEndpoint(IUserTaskInvitationService invitat public override void Configure() { Get("/user-tasks/{taskId}/invitations"); - ConfigurePermissions(UserTasksPermissions.Invite); + RequirePermission(UserTasksResourcePermissions.UserTasks, UserTaskVerbs.Invite); } public override async Task HandleAsync(CancellationToken cancellationToken) @@ -501,7 +503,7 @@ internal sealed class RevokeInvitationEndpoint(IUserTaskInvitationService invita public override void Configure() { Delete("/user-tasks/{taskId}/invitations/{invitationId}"); - ConfigurePermissions(UserTasksPermissions.Invite); + RequirePermission(UserTasksResourcePermissions.UserTasks, UserTaskVerbs.Invite); } public override async Task HandleAsync(UserTaskMutationApiRequest request, CancellationToken cancellationToken) @@ -526,7 +528,7 @@ internal sealed class ListParticipantsEndpoint(IUserTaskParticipantDirectory dir public override void Configure() { Get("/user-task-participants"); - ConfigurePermissions(UserTasksPermissions.LookupParticipants); + RequirePermission(UserTasksResourcePermissions.Participants, CoreVerbs.View); } public override async Task HandleAsync(UserTaskParticipantLookupApiRequest request, CancellationToken cancellationToken) diff --git a/src/modules/Elsa.UserTasks/Models/UserTaskModels.cs b/src/modules/Elsa.UserTasks/Models/UserTaskModels.cs index bdf91369c..c995b4d0e 100644 --- a/src/modules/Elsa.UserTasks/Models/UserTaskModels.cs +++ b/src/modules/Elsa.UserTasks/Models/UserTaskModels.cs @@ -1,5 +1,6 @@ using System.Security.Claims; using System.Text.Json; +using Elsa.Authorization; using Elsa.Mediator.Contracts; namespace Elsa.UserTasks.Models; @@ -101,7 +102,7 @@ public sealed record UserTaskActor( { /// Indicates that the host has granted tenant-scoped manager access. public bool IsManager { get; init; } - public IReadOnlySet Permissions { get; init; } = new HashSet(StringComparer.OrdinalIgnoreCase); + public IReadOnlySet Permissions { get; init; } = new HashSet(StringComparer.Ordinal); /// /// Set when the caller authenticated with a guest invitation session. A guest is scoped to exactly @@ -114,7 +115,19 @@ public sealed record UserTaskActor( public bool IsGuest => GuestTaskId != null; - public bool HasPermission(string permission) => Permissions.Contains(permission) || Permissions.Contains("*"); + /// Whether a grant this actor holds satisfies . + /// + /// Matched through rather than by string equality, so a subtree or verb + /// wildcard reaches this check exactly as it reaches an endpoint's own gate. Comparing strings would + /// leave the two disagreeing: the endpoint would admit a caller holding user-tasks:* and the + /// policy would then deny them, which reads as a broken task rather than as a missing grant. A bare + /// * keeps working because it parses as *:*, not because it is special-cased here. + /// + public bool HasPermission(Permission required) => + Permissions.Any(x => Permission.TryParse(x, out var granted) && PermissionMatcher.Satisfies(granted, required)); + + /// + public bool HasPermission(string resource, string verb) => HasPermission(new Permission(resource, verb)); } public sealed record UserTaskAction( diff --git a/src/modules/Elsa.UserTasks/Permissions/UserTasksPermissions.cs b/src/modules/Elsa.UserTasks/Permissions/UserTasksPermissions.cs deleted file mode 100644 index bd1365197..000000000 --- a/src/modules/Elsa.UserTasks/Permissions/UserTasksPermissions.cs +++ /dev/null @@ -1,14 +0,0 @@ -namespace Elsa.UserTasks.Permissions; - -public static class UserTasksPermissions -{ - public const string Read = "read:user-tasks"; - public const string Claim = "claim:user-tasks"; - public const string Complete = "complete:user-tasks"; - public const string Assign = "assign:user-tasks"; - public const string Update = "update:user-tasks"; - public const string Cancel = "cancel:user-tasks"; - public const string Invite = "invite:user-tasks"; - public const string Manage = "manage:user-tasks"; - public const string LookupParticipants = "lookup:user-task-participants"; -} diff --git a/src/modules/Elsa.UserTasks/Permissions/UserTasksResourcePermissions.cs b/src/modules/Elsa.UserTasks/Permissions/UserTasksResourcePermissions.cs new file mode 100644 index 000000000..f21b6b4a0 --- /dev/null +++ b/src/modules/Elsa.UserTasks/Permissions/UserTasksResourcePermissions.cs @@ -0,0 +1,73 @@ +using Elsa.Authorization; +using Elsa.Permissions; +using JetBrains.Annotations; + +namespace Elsa.UserTasks.Permissions; + +/// +/// Stable resource names for User Tasks. Endpoints reference these constants rather than string +/// literals, and the descriptors below are declared alongside them so the two cannot drift. +/// +public static class UserTasksResourcePermissions +{ + /// Work on human tasks: read them, claim them, complete them, and supervise the whole tenant's queue. + public const string UserTasks = "user-tasks"; + + /// Search the users and groups a task may be assigned to. + public const string Participants = "user-tasks/participants"; +} + +/// +/// The non-core verbs User Tasks declares. They live beside the resources they apply to so a call site and +/// the catalog cannot drift apart, and so a policy check cannot spell one differently from the endpoint it +/// guards. +/// +public static class UserTaskVerbs +{ + /// Take a task from the candidate pool, or give it back. One verb, because releasing is undoing a claim. + public const string Claim = "claim"; + + /// Submit a completion action against a task, resuming the workflow that raised it. + public const string Complete = "complete"; + + /// Hand a task to a participant. + public const string Assign = "assign"; + + /// End a task without completing it. + public const string Cancel = "cancel"; + + /// Issue, list, and revoke guest invitations to a task. + public const string Invite = "invite"; + + /// + /// Act across the tenant's whole queue rather than only on tasks you take part in: read every task, + /// assign, reschedule, cancel, see blocked tasks, and retry a failed resolution. + /// + /// + /// Named supervise rather than manage for the reason workflows/runtime:control is + /// not called manage either: it is an elevated tier, not an aggregate of the verbs beside it, and + /// a name that reads like an aggregate invites exactly that misreading. Holding it confers none of + /// claim, complete, assign, cancel or invite — no verb implies another. + /// + public const string Supervise = "supervise"; +} + +/// Contributes the User Tasks resources to the permission catalog. +[UsedImplicitly] +public sealed class UserTasksResourcePermissionsDescriptorProvider : IPermissionDescriptorProvider +{ + /// + public IEnumerable GetDescriptors() => + [ + new(UserTasksResourcePermissions.UserTasks, + [CoreVerbs.View, CoreVerbs.Update, UserTaskVerbs.Claim, UserTaskVerbs.Complete, UserTaskVerbs.Assign, UserTaskVerbs.Cancel, UserTaskVerbs.Invite, UserTaskVerbs.Supervise], + "User tasks", + "Work on human tasks: read them, claim them, complete them, and supervise the whole tenant's queue.", + "User Tasks"), + new(UserTasksResourcePermissions.Participants, + [CoreVerbs.View], + "User task participants", + "Search the users and groups a task may be assigned to.", + "User Tasks"), + ]; +} diff --git a/src/modules/Elsa.UserTasks/Services/DefaultClaimsIdentityResolver.cs b/src/modules/Elsa.UserTasks/Services/DefaultClaimsIdentityResolver.cs index db9d9e566..228a6129d 100644 --- a/src/modules/Elsa.UserTasks/Services/DefaultClaimsIdentityResolver.cs +++ b/src/modules/Elsa.UserTasks/Services/DefaultClaimsIdentityResolver.cs @@ -2,12 +2,16 @@ using System.Security.Claims; using Elsa.UserTasks.Contracts; using Elsa.UserTasks.Models; using Elsa.UserTasks.Options; +using Elsa.UserTasks.Permissions; using Microsoft.Extensions.Options; namespace Elsa.UserTasks.Services; public sealed class DefaultClaimsIdentityResolver(IOptions options) : IUserTaskIdentityResolver { + /// An ASP.NET role, not a permission, despite reading like one. Kept for hosts that grant it. + private const string ManagerRole = "user-tasks:manager"; + private readonly UserTasksOptions _options = options.Value; public ValueTask ResolveAsync(ClaimsPrincipal principal, CancellationToken cancellationToken = default) @@ -29,17 +33,21 @@ public sealed class DefaultClaimsIdentityResolver(IOptions opt .Distinct(StringComparer.Ordinal) .Select(id => new ParticipantReference(tenantId, provider, UserTaskParticipantType.Group, id)) .ToArray(); + // Ordinal, matching the permission model everywhere else: folding case here would collapse two + // spellings into whichever arrived first, and the survivor might be the one that no longer matches. var permissions = _options.PermissionClaimTypes .SelectMany(type => principal.FindAll(type)) .Select(x => x.Value) .Where(value => !string.IsNullOrWhiteSpace(value)) - .ToHashSet(StringComparer.OrdinalIgnoreCase); + .ToHashSet(StringComparer.Ordinal); + var actor = new UserTaskActor(subject, groups, subject.DisplayName) { Permissions = permissions }; - return ValueTask.FromResult(new UserTaskActor(subject, groups, subject.DisplayName) + return ValueTask.FromResult(actor with { - IsManager = principal.IsInRole("user-tasks:manager") - || permissions.Contains("manage:user-tasks"), - Permissions = permissions + // Asked of the actor rather than of the raw strings, so a subtree or verb wildcard confers + // manager standing here exactly as it does at every other check. + IsManager = principal.IsInRole(ManagerRole) + || actor.HasPermission(UserTasksResourcePermissions.UserTasks, UserTaskVerbs.Supervise) }); } } diff --git a/src/modules/Elsa.UserTasks/Services/DefaultUserTaskAccessPolicy.cs b/src/modules/Elsa.UserTasks/Services/DefaultUserTaskAccessPolicy.cs index f1ce024ca..c04eca219 100644 --- a/src/modules/Elsa.UserTasks/Services/DefaultUserTaskAccessPolicy.cs +++ b/src/modules/Elsa.UserTasks/Services/DefaultUserTaskAccessPolicy.cs @@ -1,3 +1,4 @@ +using Elsa.Authorization; using Elsa.UserTasks.Contracts; using Elsa.UserTasks.Models; using Elsa.UserTasks.Permissions; @@ -20,7 +21,7 @@ public sealed class DefaultUserTaskAccessPolicy : IUserTaskAccessPolicy public Task CreateScopeAsync(UserTaskActor actor, UserTaskQueryScopeKind kind, CancellationToken cancellationToken = default) { // A guest session is issued for one task and carries no list capability at all. - if (actor.IsGuest || !actor.HasPermission(UserTasksPermissions.Read)) + if (actor.IsGuest || !actor.HasPermission(UserTasksResourcePermissions.UserTasks, CoreVerbs.View)) return Task.FromResult(null); var isManager = IsManager(actor); @@ -90,11 +91,11 @@ public sealed class DefaultUserTaskAccessPolicy : IUserTaskAccessPolicy } /// - /// A tenant-scoped manager must hold manage:user-tasks (or the wildcard grant). The actor flag on - /// its own is host-supplied metadata and is never sufficient. + /// A tenant-scoped manager must hold user-tasks:supervise (or a grant covering it). The actor flag + /// on its own is host-supplied metadata and is never sufficient. /// private static bool IsManager(UserTaskActor actor) => - !actor.IsGuest && actor.IsManager && actor.HasPermission(UserTasksPermissions.Manage); + !actor.IsGuest && actor.IsManager && actor.HasPermission(UserTasksResourcePermissions.UserTasks, UserTaskVerbs.Supervise); private static bool IsCandidate(UserTask task, UserTaskActor actor) { @@ -111,18 +112,24 @@ public sealed class DefaultUserTaskAccessPolicy : IUserTaskAccessPolicy private static bool IsExcluded(UserTask task, UserTaskActor actor) => task.ExcludedUsers.Any(x => x.Matches(actor.Subject)) && !(IsManager(actor) && task.AllowManagerExclusionOverride); - private static string RequiredPermission(UserTaskAccessOperation operation) => operation switch + /// + /// The permission an operation needs, which is the same one the endpoint offering it declares. Every + /// operation acts on the task itself, so only the verb varies. + /// + private static Permission RequiredPermission(UserTaskAccessOperation operation) => + new(UserTasksResourcePermissions.UserTasks, RequiredVerb(operation)); + + private static string RequiredVerb(UserTaskAccessOperation operation) => operation switch { - UserTaskAccessOperation.ReadSummary or UserTaskAccessOperation.ReadProtected => UserTasksPermissions.Read, - UserTaskAccessOperation.Claim or UserTaskAccessOperation.Release => UserTasksPermissions.Claim, - UserTaskAccessOperation.Complete => UserTasksPermissions.Complete, - UserTaskAccessOperation.Assign => UserTasksPermissions.Assign, - UserTaskAccessOperation.UpdateScheduling => UserTasksPermissions.Update, - UserTaskAccessOperation.Cancel => UserTasksPermissions.Cancel, - UserTaskAccessOperation.Manage => UserTasksPermissions.Manage, - UserTaskAccessOperation.IssueInvitation => UserTasksPermissions.Invite, - UserTaskAccessOperation.RetryResolution => UserTasksPermissions.Manage, - _ => UserTasksPermissions.Read + UserTaskAccessOperation.ReadSummary or UserTaskAccessOperation.ReadProtected => CoreVerbs.View, + UserTaskAccessOperation.Claim or UserTaskAccessOperation.Release => UserTaskVerbs.Claim, + UserTaskAccessOperation.Complete => UserTaskVerbs.Complete, + UserTaskAccessOperation.Assign => UserTaskVerbs.Assign, + UserTaskAccessOperation.UpdateScheduling => CoreVerbs.Update, + UserTaskAccessOperation.Cancel => UserTaskVerbs.Cancel, + UserTaskAccessOperation.Manage or UserTaskAccessOperation.RetryResolution => UserTaskVerbs.Supervise, + UserTaskAccessOperation.IssueInvitation => UserTaskVerbs.Invite, + _ => CoreVerbs.View }; } diff --git a/src/modules/Elsa.UserTasks/Services/UserTaskGuestSecurity.cs b/src/modules/Elsa.UserTasks/Services/UserTaskGuestSecurity.cs index 3ad42a02b..657cf7c20 100644 --- a/src/modules/Elsa.UserTasks/Services/UserTaskGuestSecurity.cs +++ b/src/modules/Elsa.UserTasks/Services/UserTaskGuestSecurity.cs @@ -2,10 +2,12 @@ using System.Collections.Concurrent; using System.Security.Cryptography; using System.Text; using System.Text.Json; +using Elsa.Authorization; using Elsa.Common; using Elsa.UserTasks.Contracts; using Elsa.UserTasks.Models; using Elsa.UserTasks.Options; +using Elsa.UserTasks.Permissions; using Microsoft.AspNetCore.DataProtection; using Microsoft.Extensions.Options; @@ -218,7 +220,10 @@ public sealed class UserTaskGuestActorResolver(IUserTaskGuestSessionIssuer sessi return new UserTaskActor(session.Subject, [], session.Subject.DisplayName) { IsManager = false, - Permissions = new HashSet([Permissions.UserTasksPermissions.Read, Permissions.UserTasksPermissions.Complete], StringComparer.OrdinalIgnoreCase), + Permissions = new HashSet([ + new Permission(UserTasksResourcePermissions.UserTasks, CoreVerbs.View).ToString(), + new Permission(UserTasksResourcePermissions.UserTasks, UserTaskVerbs.Complete).ToString() + ], StringComparer.Ordinal), GuestTaskId = session.TaskId, GuestAllowedActions = new HashSet(session.AllowedActions, StringComparer.OrdinalIgnoreCase) }; diff --git a/test/integration/Elsa.Hosts.SmokeTests/ClassicHostSmokeTests.cs b/test/integration/Elsa.Hosts.SmokeTests/ClassicHostSmokeTests.cs index ccb81c081..d5a9cc4a6 100644 --- a/test/integration/Elsa.Hosts.SmokeTests/ClassicHostSmokeTests.cs +++ b/test/integration/Elsa.Hosts.SmokeTests/ClassicHostSmokeTests.cs @@ -23,8 +23,8 @@ public class ShellHostSmokeTests(HostFixture host) : HostSmok /// /// /// The two route sets overlap but are not identical: each lists what its own host actually configures, - /// and External Authentication is enabled only here. Keeping them separate is the point -- a route that - /// disappears from one host and not the other is the divergence these tests are looking for. + /// and External Authentication and User Tasks are enabled only here. Keeping them separate is the point -- + /// a route that disappears from one host and not the other is the divergence these tests are looking for. /// protected override IReadOnlyCollection GatedRoutes => [ @@ -32,6 +32,7 @@ public class ShellHostSmokeTests(HostFixture host) : HostSmok "/elsa/api/workflow-instances", "/elsa/api/identity/permissions", "/elsa/api/external-authentication/connections", - "/elsa/api/external-authentication/descriptors/adapters" + "/elsa/api/external-authentication/descriptors/adapters", + "/elsa/api/user-tasks" ]; } diff --git a/test/unit/Elsa.UserTasks.Persistence.ConformanceTests/UserTaskFaultInjectionConformanceTests.cs b/test/unit/Elsa.UserTasks.Persistence.ConformanceTests/UserTaskFaultInjectionConformanceTests.cs index a3ec717e7..d73c79fe5 100644 --- a/test/unit/Elsa.UserTasks.Persistence.ConformanceTests/UserTaskFaultInjectionConformanceTests.cs +++ b/test/unit/Elsa.UserTasks.Persistence.ConformanceTests/UserTaskFaultInjectionConformanceTests.cs @@ -1,8 +1,10 @@ +using Elsa.Authorization; using Elsa.UserTasks.Contracts; using Elsa.UserTasks.Models; using Elsa.UserTasks.Persistence.ConformanceTests.Faults; using Elsa.UserTasks.Persistence.ConformanceTests.Infrastructure; using Elsa.UserTasks.Persistence.ConformanceTests.Providers; +using Elsa.UserTasks.Permissions; using Elsa.UserTasks.Services; using Elsa.Workflows; @@ -263,18 +265,20 @@ public abstract class UserTaskFaultInjectionConformanceTests(UserTaskStoreFixtur private DefaultUserTaskInvitationService CreateInvitationService(IUserTaskRepository repository, IUserTaskGuestSessionIssuer sessions) => new(repository, _policy, Fixture.Outbox, new DefaultUserTaskInvitationVerifier(), sessions, _sink, _identity, Clock, Fixture.Options); + /// The named verbs, as permissions on the user-tasks resource. + private static IReadOnlySet Grants(params string[] verbs) => + verbs.Select(verb => new Permission(UserTasksResourcePermissions.UserTasks, verb).ToString()).ToHashSet(StringComparer.Ordinal); + private UserTaskActor Actor(string id = "user-1") => new(Subject(id), []) { - Permissions = new HashSet(["read:user-tasks", "claim:user-tasks", "complete:user-tasks"], StringComparer.OrdinalIgnoreCase) + Permissions = Grants(CoreVerbs.View, UserTaskVerbs.Claim, UserTaskVerbs.Complete) }; private UserTaskActor ManagerActor() => Actor("manager-1") with { IsManager = true, - Permissions = new HashSet([ - "read:user-tasks", "claim:user-tasks", "complete:user-tasks", "assign:user-tasks", - "update:user-tasks", "cancel:user-tasks", "invite:user-tasks", "manage:user-tasks" - ], StringComparer.OrdinalIgnoreCase) + Permissions = Grants(CoreVerbs.View, UserTaskVerbs.Claim, UserTaskVerbs.Complete, UserTaskVerbs.Assign, + CoreVerbs.Update, UserTaskVerbs.Cancel, UserTaskVerbs.Invite, UserTaskVerbs.Supervise) }; private sealed class TestIdentityGenerator : IIdentityGenerator diff --git a/test/unit/Elsa.UserTasks.Persistence.EFCore.UnitTests/EFCoreUserTaskRepositoryTests.cs b/test/unit/Elsa.UserTasks.Persistence.EFCore.UnitTests/EFCoreUserTaskRepositoryTests.cs index 3e9765d83..1968f2dcb 100644 --- a/test/unit/Elsa.UserTasks.Persistence.EFCore.UnitTests/EFCoreUserTaskRepositoryTests.cs +++ b/test/unit/Elsa.UserTasks.Persistence.EFCore.UnitTests/EFCoreUserTaskRepositoryTests.cs @@ -1,8 +1,10 @@ +using Elsa.Authorization; using Elsa.Persistence.EFCore; using Elsa.Persistence.EFCore.Extensions; using Elsa.Common; using Elsa.UserTasks.Contracts; using Elsa.UserTasks.Options; +using Elsa.UserTasks.Permissions; using Elsa.UserTasks.Services; using Elsa.Workflows; using Microsoft.Extensions.Options; @@ -174,7 +176,10 @@ public sealed class EFCoreUserTaskRepositoryTests : IAsyncLifetime var subject = new ParticipantReference("tenant-1", "directory", UserTaskParticipantType.User, "u1"); var actor = new UserTaskActor(subject, []) { - Permissions = new HashSet(["read:user-tasks", "claim:user-tasks", "complete:user-tasks"], StringComparer.OrdinalIgnoreCase) + Permissions = new HashSet( + new[] { CoreVerbs.View, UserTaskVerbs.Claim, UserTaskVerbs.Complete } + .Select(verb => new Permission(UserTasksResourcePermissions.UserTasks, verb).ToString()), + StringComparer.Ordinal) }; var task = CreateTask(DateTimeOffset.UtcNow, subject); await repository.AddProjectionAsync(task); diff --git a/test/unit/Elsa.UserTasks.UnitTests/Authorization/ActorPermissionMatchingTests.cs b/test/unit/Elsa.UserTasks.UnitTests/Authorization/ActorPermissionMatchingTests.cs new file mode 100644 index 000000000..f3745d8e3 --- /dev/null +++ b/test/unit/Elsa.UserTasks.UnitTests/Authorization/ActorPermissionMatchingTests.cs @@ -0,0 +1,118 @@ +using System.Security.Claims; +using Elsa.Authorization; +using Elsa.UserTasks.Models; +using Elsa.UserTasks.Options; +using Elsa.UserTasks.Permissions; +using Elsa.UserTasks.Services; + +namespace Elsa.UserTasks.UnitTests.Authorization; + +/// +/// The policy layer matches grants the way the endpoint gate does. +/// +/// +/// These are the grants the module could not previously honour: it compared claim values for equality, so an +/// administrator who wrote user-tasks:* — a grant the model defines, the catalog implies, and every +/// other module accepts — was silently denied. Every case here failed before the migration, and a regression +/// would be invisible in behaviour that merely looks like a missing grant. +/// +public class ActorPermissionMatchingTests +{ + private readonly UserTaskTestFixture _fixture = new(); + + [Theory] + [InlineData("user-tasks:view")] + [InlineData("user-tasks:*")] + [InlineData("user-tasks/*:view")] + [InlineData("*:view")] + [InlineData("*")] + public async Task GrantCoveringViewOpensTheAssignedScope(string grant) + { + var actor = _fixture.Actor("user-1", grant); + + Assert.NotNull(await _fixture.Policy.CreateScopeAsync(actor, UserTaskQueryScopeKind.Assigned)); + } + + [Theory] + [InlineData("user-tasks:complete")] + [InlineData("workflows/*:view")] + [InlineData("user-tasks/participants:view")] + public async Task GrantNotCoveringViewDoesNot(string grant) + { + var actor = _fixture.Actor("user-1", grant); + + Assert.Null(await _fixture.Policy.CreateScopeAsync(actor, UserTaskQueryScopeKind.Assigned)); + } + + [Fact] + public async Task VerbWildcardAuthorizesAnOperationItNamesNoVerbFor() + { + var actor = _fixture.Actor("user-1", "user-tasks:*"); + var task = await _fixture.ProjectAsync(actor.Subject); + + Assert.True(await _fixture.Policy.AuthorizeAsync(task, actor, UserTaskAccessOperation.Claim)); + } + + [Fact] + public async Task SubtreeGrantConfersManagerStandingWhenTheHostSetsTheFlag() + { + // The tenant-wide scope needs view as well as supervise, and one subtree grant covers both. + var actor = _fixture.Actor("manager-1", "user-tasks/*:view", "user-tasks/*:supervise") with { IsManager = true }; + + Assert.NotNull(await _fixture.Policy.CreateScopeAsync(actor, UserTaskQueryScopeKind.All)); + } + + [Fact] + public async Task ClaimsResolverDerivesManagerStandingFromAWildcardGrant() + { + var resolver = new DefaultClaimsIdentityResolver(Microsoft.Extensions.Options.Options.Create(new UserTasksOptions())); + var principal = new ClaimsPrincipal(new ClaimsIdentity([ + new Claim("sub", "user-1"), + new Claim("permissions", "user-tasks:*") + ], "test")); + + var actor = await resolver.ResolveAsync(principal); + + Assert.True(actor!.IsManager); + Assert.True(actor.HasPermission(UserTasksResourcePermissions.UserTasks, UserTaskVerbs.Supervise)); + } + + [Fact] + public async Task ClaimsResolverWithholdsManagerStandingFromAPlainWorkerGrant() + { + var resolver = new DefaultClaimsIdentityResolver(Microsoft.Extensions.Options.Options.Create(new UserTasksOptions())); + var principal = new ClaimsPrincipal(new ClaimsIdentity([ + new Claim("sub", "user-1"), + new Claim("permissions", "user-tasks:view"), + new Claim("permissions", "user-tasks:claim") + ], "test")); + + var actor = await resolver.ResolveAsync(principal); + + Assert.False(actor!.IsManager); + } + + [Fact] + public void MalformedGrantsAreIgnoredRatherThanMatched() + { + // The legacy spelling is not a well-formed permission on the resource axis: it parses as resource + // 'read', verb 'user-tasks'. It must therefore authorize nothing here, which is what makes the + // migration guide's rewrite mandatory rather than advisory. + var actor = _fixture.Actor("user-1", "read:user-tasks", "not a permission"); + + Assert.False(actor.HasPermission(UserTasksResourcePermissions.UserTasks, CoreVerbs.View)); + } + + [Fact] + public async Task GuestSessionsCarryOnlyViewAndComplete() + { + var manager = _fixture.ManagerActor(); + var task = await _fixture.ProjectAsync(_fixture.Actor("user-1").Subject, UserTaskTestFixture.WithBearerInvitation()); + var (guest, _) = await _fixture.IssueGuestSessionAsync(task, manager); + + Assert.True(guest.HasPermission(UserTasksResourcePermissions.UserTasks, CoreVerbs.View)); + Assert.True(guest.HasPermission(UserTasksResourcePermissions.UserTasks, UserTaskVerbs.Complete)); + Assert.False(guest.HasPermission(UserTasksResourcePermissions.UserTasks, UserTaskVerbs.Claim)); + Assert.False(guest.HasPermission(UserTasksResourcePermissions.UserTasks, UserTaskVerbs.Supervise)); + } +} diff --git a/test/unit/Elsa.UserTasks.UnitTests/Authorization/EndpointCoverageTests.cs b/test/unit/Elsa.UserTasks.UnitTests/Authorization/EndpointCoverageTests.cs new file mode 100644 index 000000000..d418f3e42 --- /dev/null +++ b/test/unit/Elsa.UserTasks.UnitTests/Authorization/EndpointCoverageTests.cs @@ -0,0 +1,11 @@ +using Elsa.Testing.Shared.Authorization; +using Elsa.UserTasks.Services; + +namespace Elsa.UserTasks.UnitTests.Authorization; + +public class EndpointCoverageTests +{ + [Fact] + public void EveryUserTasksEndpointDeclaresItsAccess() => + EndpointCoverage.AssertEveryEndpointDeclaresAccess(typeof(DefaultUserTaskAccessPolicy).Assembly); +} diff --git a/test/unit/Elsa.UserTasks.UnitTests/Authorization/EndpointPermissionTests.cs b/test/unit/Elsa.UserTasks.UnitTests/Authorization/EndpointPermissionTests.cs new file mode 100644 index 000000000..8e081c574 --- /dev/null +++ b/test/unit/Elsa.UserTasks.UnitTests/Authorization/EndpointPermissionTests.cs @@ -0,0 +1,160 @@ +using System.Reflection; +using Elsa.Authorization; +using Elsa.Permissions; +using Elsa.UserTasks.Permissions; +using Elsa.UserTasks.Services; +using FastEndpoints; +using NSubstitute; + +namespace Elsa.UserTasks.UnitTests.Authorization; + +/// +/// Pins what each User Tasks endpoint requires, and checks that what they require is something the catalog +/// advertises. +/// +/// +/// The module reached production declaring access through the legacy string channel, which compared claims for +/// exact equality and appeared in no catalog. Nothing could see the gap: the coverage gate only asks whether an +/// endpoint declares something, and the migration guide's consistency check reads the guide, so a +/// permission absent from both the guide and the catalog was invisible from every direction. Asserting the +/// declarations against the descriptors closes that: an endpoint requiring a verb the module never advertises +/// cannot be granted through the role editor, and fails here rather than in a deployment. +/// +public class EndpointPermissionTests +{ + private static readonly Assembly Module = typeof(DefaultUserTaskAccessPolicy).Assembly; + + /// What each endpoint is expected to require. The rows are the migration guide's table, in code. + private static readonly (string Endpoint, string Resource, string Verb)[] Expected = + [ + ("FeatureCapabilitiesEndpoint", UserTasksResourcePermissions.UserTasks, CoreVerbs.View), + ("ListEndpoint", UserTasksResourcePermissions.UserTasks, CoreVerbs.View), + ("GetEndpoint", UserTasksResourcePermissions.UserTasks, CoreVerbs.View), + ("CapabilitiesEndpoint", UserTasksResourcePermissions.UserTasks, CoreVerbs.View), + ("ListEventsEndpoint", UserTasksResourcePermissions.UserTasks, CoreVerbs.View), + ("RevealFieldEndpoint", UserTasksResourcePermissions.UserTasks, CoreVerbs.View), + ("ClaimEndpoint", UserTasksResourcePermissions.UserTasks, UserTaskVerbs.Claim), + ("ReleaseEndpoint", UserTasksResourcePermissions.UserTasks, UserTaskVerbs.Claim), + ("AssignEndpoint", UserTasksResourcePermissions.UserTasks, UserTaskVerbs.Assign), + ("ScheduleEndpoint", UserTasksResourcePermissions.UserTasks, CoreVerbs.Update), + ("CompleteEndpoint", UserTasksResourcePermissions.UserTasks, UserTaskVerbs.Complete), + ("CancelEndpoint", UserTasksResourcePermissions.UserTasks, UserTaskVerbs.Cancel), + ("RetryResolutionEndpoint", UserTasksResourcePermissions.UserTasks, UserTaskVerbs.Supervise), + ("IssueInvitationEndpoint", UserTasksResourcePermissions.UserTasks, UserTaskVerbs.Invite), + ("ListInvitationsEndpoint", UserTasksResourcePermissions.UserTasks, UserTaskVerbs.Invite), + ("RevokeInvitationEndpoint", UserTasksResourcePermissions.UserTasks, UserTaskVerbs.Invite), + ("ListParticipantsEndpoint", UserTasksResourcePermissions.Participants, CoreVerbs.View) + ]; + + public static TheoryData Declarations + { + get + { + var data = new TheoryData(); + + foreach (var (endpoint, resource, verb) in Expected) + data.Add(endpoint, resource, verb); + + return data; + } + } + + [Theory] + [MemberData(nameof(Declarations))] + public void EndpointDeclaresItsExpectedPermission(string endpointName, string resource, string verb) + { + var declared = Declare(endpointName); + + Assert.Equal(new Permission(resource, verb), declared); + } + + [Theory] + [MemberData(nameof(Declarations))] + public void EveryDeclaredPermissionIsAdvertisedByTheCatalog(string endpointName, string resource, string verb) + { + var declared = Declare(endpointName); + var descriptor = new UserTasksResourcePermissionsDescriptorProvider().GetDescriptors() + .SingleOrDefault(x => x.Resource == declared.Resource); + + Assert.True(descriptor is not null, $"{endpointName} requires resource '{declared.Resource}', which the module contributes no descriptor for, so it cannot be granted through the role editor."); + Assert.True(descriptor!.Supports(declared.Verb), $"{endpointName} requires '{declared}', but '{declared.Resource}' advertises only [{string.Join(", ", descriptor.SupportedVerbs)}]."); + // The theory data is what the migration guide's rows are written against, so it has to agree too. + Assert.Equal(new Permission(resource, verb), declared); + } + + [Fact] + public void EveryEndpointInTheModuleIsCovered() + { + // Without this, deleting a row would silently stop testing an endpoint rather than fail. + var declaring = EndpointNames().OrderBy(x => x, StringComparer.Ordinal).ToArray(); + var asserted = Expected.Select(x => x.Endpoint).OrderBy(x => x, StringComparer.Ordinal).ToArray(); + + Assert.Equal(declaring, asserted); + } + + [Fact] + public void EveryAdvertisedVerbGuardsSomething() + { + // A verb nobody requires is one an administrator can grant to no effect. + var required = Expected.Select(x => new Permission(x.Resource, x.Verb)).ToHashSet(); + var unused = new UserTasksResourcePermissionsDescriptorProvider().GetDescriptors() + .SelectMany(x => x.SupportedVerbs.Select(verb => new Permission(x.Resource, verb))) + .Where(x => !required.Contains(x)) + .Select(x => x.ToString()) + .OrderBy(x => x, StringComparer.Ordinal) + .ToArray(); + + Assert.True(unused.Length == 0, $"The catalog advertises {unused.Length} permission(s) no endpoint requires: {string.Join(", ", unused)}."); + } + + /// The Elsa endpoints this module declares, by simple name. + private static IEnumerable EndpointNames() => + Elsa.Testing.Shared.Authorization.EndpointCoverage.FindEndpoints(Module).Select(x => x.Name); + + /// + /// Runs one endpoint's Configure() and returns what it recorded. The requirement is attached as an + /// inline policy, which cannot be read back off the definition, so the registry is the only way to observe + /// a declaration without booting a host. + /// + private static Permission Declare(string endpointName) + { + var endpointType = Module.GetTypes().Single(x => x.Name == endpointName); + var arguments = endpointType.GetConstructors(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic) + .Single() + .GetParameters() + .Select(x => Substitute.For([x.ParameterType], [])) + .ToArray(); + var endpoint = Activator.CreateInstance(endpointType, BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic, null, arguments, null)!; + var (requestType, responseType) = DtoTypes(endpointType); + + endpointType.GetProperty("Definition", BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic)! + .SetValue(endpoint, new EndpointDefinition(endpointType, requestType, responseType)); + endpointType.GetMethod("Configure")!.Invoke(endpoint, null); + + var permission = EndpointPermissionRegistry.Find(endpointType); + + Assert.True(permission.HasValue, $"{endpointName} declares no permission."); + return permission!.Value; + } + + private static (Type Request, Type Response) DtoTypes(Type endpointType) + { + for (var type = endpointType.BaseType; type is not null; type = type.BaseType) + { + if (!type.IsGenericType) + continue; + + var definition = type.GetGenericTypeDefinition(); + var arguments = type.GetGenericArguments(); + + if (definition == typeof(Elsa.Abstractions.ElsaEndpoint<,>)) + return (arguments[0], arguments[1]); + if (definition == typeof(Elsa.Abstractions.ElsaEndpointWithoutRequest<>)) + return (typeof(EmptyRequest), arguments[0]); + if (definition == typeof(Elsa.Abstractions.ElsaEndpoint<>)) + return (arguments[0], typeof(object)); + } + + throw new InvalidOperationException($"Unsupported endpoint type '{endpointType.FullName}'."); + } +} diff --git a/test/unit/Elsa.UserTasks.UnitTests/Elsa.UserTasks.UnitTests.csproj b/test/unit/Elsa.UserTasks.UnitTests/Elsa.UserTasks.UnitTests.csproj index 74d0da364..dd2d9522f 100644 --- a/test/unit/Elsa.UserTasks.UnitTests/Elsa.UserTasks.UnitTests.csproj +++ b/test/unit/Elsa.UserTasks.UnitTests/Elsa.UserTasks.UnitTests.csproj @@ -4,6 +4,7 @@ 0 + diff --git a/test/unit/Elsa.UserTasks.UnitTests/UserTaskInvitationTests.cs b/test/unit/Elsa.UserTasks.UnitTests/UserTaskInvitationTests.cs index f6f6c8b97..d7381fa96 100644 --- a/test/unit/Elsa.UserTasks.UnitTests/UserTaskInvitationTests.cs +++ b/test/unit/Elsa.UserTasks.UnitTests/UserTaskInvitationTests.cs @@ -1,7 +1,9 @@ using System.Text.Json; +using Elsa.Authorization; using Elsa.UserTasks.Contracts; using Elsa.UserTasks.Models; using Elsa.UserTasks.Options; +using Elsa.UserTasks.Permissions; using Elsa.UserTasks.Services; using Xunit; @@ -18,7 +20,7 @@ public class UserTaskInvitationTests private readonly UserTaskTestFixture _fixture = new(); private static Func WithBearerInvitation(params string[] actions) => - definition => definition with { Invitations = [new("bearer", actions.Length > 0 ? actions : ["Approve"], BearerOnly: true)] }; + UserTaskTestFixture.WithBearerInvitation(actions); [Fact] public async Task Issue_KeepsTheSecretOutOfTheApiResultAndOffTheAuditTrail() @@ -53,7 +55,7 @@ public class UserTaskInvitationTests [Fact] public async Task Issue_RequiresTheInvitePermissionAndManagerRelationship() { - var participant = _fixture.Actor("user-1", "read:user-tasks", "invite:user-tasks"); + var participant = _fixture.Actor("user-1", UserTaskTestFixture.Grant(CoreVerbs.View), UserTaskTestFixture.Grant(UserTaskVerbs.Invite)); var task = await _fixture.ProjectAsync(participant.Subject, WithBearerInvitation()); Assert.Null(await _fixture.Invitations.IssueAsync(Tenant, task.Id, new(task.Revision, "bearer", ["Approve"]), participant)); diff --git a/test/unit/Elsa.UserTasks.UnitTests/UserTaskTestFixture.cs b/test/unit/Elsa.UserTasks.UnitTests/UserTaskTestFixture.cs index e3a5f280b..7d2edc37e 100644 --- a/test/unit/Elsa.UserTasks.UnitTests/UserTaskTestFixture.cs +++ b/test/unit/Elsa.UserTasks.UnitTests/UserTaskTestFixture.cs @@ -1,7 +1,9 @@ +using Elsa.Authorization; using Elsa.Common; using Elsa.UserTasks.Contracts; using Elsa.UserTasks.Models; using Elsa.UserTasks.Options; +using Elsa.UserTasks.Permissions; using Elsa.UserTasks.Repositories; using Elsa.UserTasks.Services; using Elsa.Workflows; @@ -44,19 +46,24 @@ public sealed class UserTaskTestFixture GuestActors = new(GuestSessions); } + /// A permission on the user-tasks resource, so tests name a verb rather than a string. + public static string Grant(string verb) => new Permission(UserTasksResourcePermissions.UserTasks, verb).ToString(); + public UserTaskActor Actor(string id, params string[] permissions) => new(new(TenantId, "oidc", UserTaskParticipantType.User, id), []) { - Permissions = new HashSet(permissions.Length > 0 ? permissions : ["read:user-tasks", "claim:user-tasks", "complete:user-tasks"], StringComparer.OrdinalIgnoreCase) + Permissions = new HashSet( + permissions.Length > 0 ? permissions : [Grant(CoreVerbs.View), Grant(UserTaskVerbs.Claim), Grant(UserTaskVerbs.Complete)], + StringComparer.Ordinal) }; public UserTaskActor ManagerActor(string id = "manager-1") => Actor(id) with { IsManager = true, Permissions = new HashSet([ - "read:user-tasks", "claim:user-tasks", "complete:user-tasks", "assign:user-tasks", - "update:user-tasks", "cancel:user-tasks", "invite:user-tasks", "manage:user-tasks" - ], StringComparer.OrdinalIgnoreCase) + Grant(CoreVerbs.View), Grant(UserTaskVerbs.Claim), Grant(UserTaskVerbs.Complete), Grant(UserTaskVerbs.Assign), + Grant(CoreVerbs.Update), Grant(UserTaskVerbs.Cancel), Grant(UserTaskVerbs.Invite), Grant(UserTaskVerbs.Supervise) + ], StringComparer.Ordinal) }; public UserTaskMaterialization Materialization(ParticipantReference candidate, Func? configure = null) @@ -73,6 +80,10 @@ public sealed class UserTaskTestFixture "Approval workflow", 3, "correlation-1"); } + /// A definition carrying a bearer-verified guest invitation, which issuance requires. + public static Func WithBearerInvitation(params string[] actions) => + definition => definition with { Invitations = [new("bearer", actions.Length > 0 ? actions : ["Approve"], BearerOnly: true)] }; + /// Projects a task and returns it, so tests can start from a committed projection in one line. public async Task ProjectAsync(ParticipantReference candidate, Func? configure = null) => (await Manager.ProjectAsync(Materialization(candidate, configure))).Task; diff --git a/test/unit/Elsa.UserTasks.UnitTests/UserTaskTests.cs b/test/unit/Elsa.UserTasks.UnitTests/UserTaskTests.cs index 8b5bcaf1b..eb0f2bd37 100644 --- a/test/unit/Elsa.UserTasks.UnitTests/UserTaskTests.cs +++ b/test/unit/Elsa.UserTasks.UnitTests/UserTaskTests.cs @@ -1,7 +1,9 @@ using System.Security.Claims; using System.Text.Json; +using Elsa.Authorization; using Elsa.UserTasks.Models; using Elsa.UserTasks.Options; +using Elsa.UserTasks.Permissions; using Elsa.UserTasks.Repositories; using Elsa.UserTasks.Services; using Xunit; @@ -20,7 +22,7 @@ public class UserTaskTests new Claim("sub", "user-1"), new Claim("groups", "group,with;delimiters"), new Claim("groups", "finance"), - new Claim("permission", "read:user-tasks") + new Claim("permission", UserTaskTestFixture.Grant(CoreVerbs.View)) ], "test")); var actor = await resolver.ResolveAsync(principal); @@ -117,7 +119,7 @@ public class UserTaskTests [Fact] public async Task Policy_RequiresOperationPermissionInAdditionToCandidateRelationship() { - var actor = _fixture.Actor("user-1", "read:user-tasks"); + var actor = _fixture.Actor("user-1", UserTaskTestFixture.Grant(CoreVerbs.View)); var task = await _fixture.ProjectAsync(actor.Subject); var claim = await _fixture.Manager.ClaimAsync(UserTaskTestFixture.TenantId, task.Id, new(1, "claim-no-permission"), actor); @@ -129,11 +131,11 @@ public class UserTaskTests [Fact] public async Task Policy_ManagerFlagAloneDoesNotGrantManagementWithoutThePermission() { - var actor = _fixture.Actor("user-1", "read:user-tasks", "assign:user-tasks") with { IsManager = true }; + var actor = _fixture.Actor("user-1", UserTaskTestFixture.Grant(CoreVerbs.View), UserTaskTestFixture.Grant(UserTaskVerbs.Assign)) with { IsManager = true }; var candidate = _fixture.Actor("user-2"); var task = await _fixture.ProjectAsync(candidate.Subject); - // The host set IsManager but never granted manage:user-tasks, so management must still be refused. + // The host set IsManager but never granted user-tasks:supervise, so management must still be refused. Assert.False(await _fixture.Policy.AuthorizeAsync(task, actor, UserTaskAccessOperation.Assign)); Assert.Null(await _fixture.Policy.CreateScopeAsync(actor, UserTaskQueryScopeKind.All)); }