Sipke Schoorstra 895d494c3f

fix(external-auth)!: wildcard-aware permission grant boundary, and startup smoke tests for both hosts (#7985 )

* fix(external-auth)!: match permission grant boundaries as patterns

The deployment allow/deny boundary and the delegation authorizer compared
permission strings with ordinal equality, so under the {resource}:{verb}
vocabulary they could not see wildcards. A deny list naming
'workflows/*:delete' did not deny 'workflows/definitions:delete', and a grant
of 'workflows/*:delete' outflanked a deny naming that leaf.

The bypass was reachable. ElsaRolePermissionGrantSource passes a role's
permissions to the boundary verbatim, survivors land in the issued token as
permission claims, and PermissionEvaluator does expand wildcards there. So an
ordinary role plus a deny list was enough, on every external sign-in, with no
privileged actor involved. Restoring the ordinal boundary under the new tests
fails seven of them.

Deny is now matched in both directions, allow one-directionally, both through
PermissionMatcher. A grant that is not a well-formed permission is dropped
with a warning rather than carried into a token it cannot authorize anything
in.

Five non-endpoint checks -- delegation, role-reference removal, unsafe
settings confirmation, the recovery override and the boundary itself -- also
still compared against the legacy ExternalAuthenticationPermissions
constants. Those carry two colons, so Permission.TryParse rejects them and no
principal can hold one, while the migration guide tells operators to replace
exactly those strings. All five now route through IPermissionEvaluator, and
the module registers AddElsaAuthorization itself instead of depending on host
ordering.

Non-core verbs move to ExternalAuthenticationVerbs, declared beside the
resources they apply to so a delegation check cannot spell one differently
from the endpoint it guards.

Refs #7982

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* style: apply IDE code cleanup to the diagnostics and identity modules

Redundant namespace qualifiers and usings removed, and primary-constructor
and record syntax applied, across Elsa.Diagnostics.ConsoleLogs,
Elsa.Diagnostics.StructuredLogs, Elsa.Expressions.JavaScript and
Elsa.Identity. Produced by a solution-wide IDE cleanup that ran alongside the
authorization work; separated from it so the permission changes can be
reviewed on their own.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(hosts): boot both hosts and assert their gated routes challenge

This repo runs two parallel feature systems, the classic Features/ path and
the CShells ShellFeatures/ path, and every module has to register in both.
Nothing exercised either. The unit and integration suites construct services
directly, so a module registered in one path and not the other, or a service
missing from one container, passes every test and fails only when a host
starts. Three bugs in #7980 were found by running these two hosts by hand,
two of them shell-versus-classic divergences.

Each host is booted through WebApplicationFactory, running its real Program
with full feature registration, and asked for a handful of routes it is
expected to serve behind a permission. A 404 means the module was never
registered, a 5xx means the endpoint was found but its dependencies could not
be constructed, and a 200 means no gate ran; only 401 passes. All routes are
reported together, so a feature system that stops registering a group of
modules reads as one failure rather than a queue of identical ones.

Removing AddExternalAuthenticationServices from the shell feature -- the
divergence this is built to catch -- fails the shell host on all five of its
routes while the classic host stays green.

The assertions go through HTTP rather than the container on purpose. The
hosts have different topologies: the classic host's root provider holds
everything and registers 125 routes, while CShells gives each shell its own
provider and mounts routes per shell, leaving 6 in the root. A container or
route-table assertion would have to encode that difference and would break
whenever CShells changed internally. Behaviour at the edge is host-agnostic,
and it is what actually has to match.

Each host gains a namespaced entry-point marker because both already declare
a Program in the global namespace, which a test project referencing both
cannot tell apart.

Coverage is off for this project: it references both hosts, so every module
either pulls in would enter its denominator without adding real coverage, and
coverlet cannot instrument a graph that size. TreatAsLocalProperty keeps CI's
/p:CollectCoverage=true from overriding that.

Refs #7982

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(external-auth)!: fail closed on an unparseable grant boundary

Two findings from review, both real.

The grant boundary parsed its allow and deny lists and silently dropped what
would not parse. An allow list of nothing but malformed entries therefore
reduced to an empty set, and an empty allow list means unrestricted -- so a
typo turned the boundary off entirely and let external grant sources put
permissions straight into issued tokens. The deny side had the mirror of it:
a malformed entry quietly stopped denying what it named.

A boundary that does not parse now admits nothing, and
ExternalAuthenticationOptionsValidator rejects the configuration at startup,
so the mistake reaches an operator rather than a token. Failing startup is
what makes the runtime behaviour safe to be strict about: it cannot be hit by
someone mid-edit, only by validation having been bypassed.

ConnectionEndpointSupport.HasPermission was a sixth ad-hoc permission check,
missed when the other five were converted. It compared claim values against
the legacy ExternalAuthenticationPermissions constants at four call sites --
policy management on create and update, session revocation, and unsafe
settings confirmation -- and those constants carry two colons, so nothing can
hold one once a deployment follows the migration guide. It now routes through
IPermissionEvaluator like the rest, resolved from the request with a fallback
to the shared evaluator, the same way EndpointSecurity does it.

Refs #7982

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* style(external-auth): filter permission patterns with Where

Addresses a review nit on ValidatePermissionPatterns. Behaviour is unchanged:
a null list still iterates nothing, only malformed entries are reported, and
the message text is identical.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(external-auth)!: apply the grant boundary to role permissions too

Token issuance concatenated the user's Elsa role permissions raw alongside
the boundary-filtered external grants. A permission the boundary had just
excluded during grant resolution therefore reappeared in the issued token
from the same roles, which made the deny list unenforceable for anything a
role carried and left ElsaRolePermissionGrantSource filtering nothing that
was not added back a moment later. The bypass did not even need that grant
source configured: role permissions reached the token regardless of which
sources a connection selected.

Both origins now pass the same boundary. Re-applying it at issuance also
picks up a boundary that changed since sign-in, since refreshing reissues.

This is a behaviour change for deployments that configured a boundary
expecting it to bound only claim-mapped permissions: an external login may
now carry fewer permissions than before. Deployments with no boundary
configured, the default, are unaffected -- every well-formed permission
passes. The migration guide describes both directions.

Refs #7982

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

2026-08-25 05:25:29 +02:00

14 KiB

Raw Blame History

Migrate to the Structured Authorization Model

Elsa's permission vocabulary changes shape. A permission is now {resource}:{verb} — a hierarchical resource path paired with a verb — replacing the flat verb:resource strings.

This is a breaking change for any deployment with hand-authored roles. Legacy permission strings stop authorizing. Nothing silently degrades: a startup validator reports every stored permission that no longer resolves, identified by the role that holds it.

What you have to do

Re-author each role's permissions using the table below, or through the catalog at GET /identity/permissions, which lists every registered resource and the verbs it accepts.

* keeps working. It parses to *:*, so the seeded administrator role continues to authorize everything and an instance cannot lock itself out while the rest is re-authored. Do this first, before touching anything else.

Three things that are not a simple rename

The migration expands

Some new resources are finer-grained than the permissions they replace, so one legacy string becomes several. A one-for-one substitution silently narrows the role.

Legacy	Expands to
`read:workflow-definitions`	`workflows/definitions:view` and `workflows/definitions/versions:view`
`delete:workflow-definitions`	`workflows/definitions:delete` and `workflows/definitions/versions:delete`
`publish:workflow-definitions`	`workflows/definitions:publish` and `workflows/definitions/versions:revert`
`external-authentication:links:manage`	`identity-links:view`, `:write` and `:delete`
`external-authentication:policies:manage`	`policies:view` and `:update`

`read:` and `exec:` become more powerful

Today they are literal claim values, not patterns: read:* authorizes only the twelve endpoints that happen to list it, out of roughly forty read endpoints. Their replacements, *:view and *:execute, work as the names always implied — across every resource, including ones added later.

Review any role holding them by hand. Do not rewrite them automatically.

The C#/Python expression permissions are removed

exec:csharp-expressions and exec:python-expressions are dropped rather than translated. They conflated an incoherent execution-side gate — a workflow runs under the server's authority, not the caller's, so the check never constrained what a script could do — with a meaningful authoring-side one.

This is a deliberate reduction in control. The host switch (CSharpOptions.AllowHostCodeExecution, PythonOptions.AllowHostCodeExecution) becomes the single control:

Where host code is disabled, nothing changes.
Where host code is enabled, any author who may write workflow definitions may use C# and Python, and the editor offers those expression types to every such author.

Deployments that enabled host code while trusting only some authors lose that granularity until #7975 lands. If that matters to you, disable host code until then.

Revocation

The default access-token lifetime drops from 1 hour to 15 minutes. This is the revocation bound: permission claims are issued at sign-in, and refreshing re-reads the user's roles, so removing a role takes effect at most one access-token lifetime later. Refresh already rotates both tokens, so no client change is required.

For a tighter bound, enable the optional permission stamp (Identity:PermissionStamp:IsEnabled). It is derived from the user's roles rather than stored, so it needs no schema change and no cross-node cache invalidation. CacheLifetime, default 30 seconds, is the effective bound when enabled.

External authentication grant boundaries

ExternalAuthentication:PermissionGrants:AllowedPermissions and DeniedPermissions bound which permissions an external identity provider connection may confer. Both lists are now matched as permission patterns rather than by exact string, so they read the way a role does.

Denied is matched in both directions. workflows/*:delete denies workflows/definitions:delete, and a connection granting workflows/*:delete is denied by a deny list naming only workflows/definitions:delete. Before this release both comparisons were exact, so either spelling slipped past the other and a deployment's deny list did not hold. If you carried a deny list across the upgrade, re-read it: it may now deny more than it used to, which is the intent.
Allowed is matched one way: an allow entry must cover the whole grant. workflows/*:delete admits workflows/definitions:delete, but an allow list naming only workflows/definitions:delete refuses a workflows/*:delete grant rather than admitting the part that overlaps.

The boundary now also applies to permissions the user's own Elsa roles carry, not only to those an external claim mapping confers. Previously token issuance concatenated role permissions raw, so a permission the boundary excluded during sign-in reappeared in the issued token from the same roles — which made the deny list unenforceable for anything a role happened to carry. If you configured a boundary expecting it to bound the whole token, it now does. If you configured one expecting it to bound only claim-mapped permissions, an external login may now carry fewer permissions than before; widen the list, or move the restriction into the roles themselves. Deployments with no boundary configured, which is the default, are unaffected.

A boundary that does not parse is now a startup failure rather than a silently ignored setting. An allow list whose entries are all malformed used to reduce to an empty list, which means unrestricted, so a typo turned the boundary off. Fix the entries the startup error names; the mapping table below gives the new spelling.

Rewrite both lists into the new {resource}:{verb} vocabulary using the full mapping. A value that is not a well-formed permission matches nothing, and a grant that is not well-formed is dropped at sign-in with a malformed_permission warning instead of being carried into a token.

The same matching now governs the delegation check: an actor may configure a mapping only for permissions their own grants cover, so holding workflows/*:delete lets them delegate workflows/definitions:delete, while holding just that leaf does not let them delegate the subtree.

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.

Per-tenant identity uniqueness

Included in the same release: User.Name, Role.Name, Application.Name and Application.ClientId move from globally unique indexes to composite indexes on (TenantId, Name). Two tenants could not previously hold a role of the same name. Apply the PerTenantIdentityUniqueness migration for your provider.

If you have duplicate names across tenants today, they were impossible to create, so no data conflict can arise. Going the other way — downgrading — will fail if duplicates exist by then.

Full mapping

Legacy permission	Replacement
`*`	`:`
`read:*`	`*:view`
`exec:*`	`*:execute`
`read:workflow-definitions`	`workflows/definitions:view` + `workflows/definitions/versions:view`
`write:workflow-definitions`	`workflows/definitions:write`
`delete:workflow-definitions`	`workflows/definitions:delete` + `workflows/definitions/versions:delete`
`exec:workflow-definitions`	`workflows/definitions:execute`
`publish:workflow-definitions`	`workflows/definitions:publish` + `workflows/definitions/versions:revert`
`retract:workflow-definitions`	`workflows/definitions:retract`
`actions:workflow-definitions:refresh`	`workflows/definitions:refresh`
`actions:workflow-definitions:reload`	`workflows/definitions:reload`
`read:workflow-definition-labels`	`workflows/definitions/labels:view`
`update:workflow-definition-labels`	`workflows/definitions/labels:update`
`read:workflow-instances`	`workflows/instances:view`
`write:workflow-instances`	`workflows/instances:write`
`delete:workflow-instances`	`workflows/instances:delete`
`cancel:workflow-instances`	`workflows/instances:cancel`
`read:activity-execution`	`workflows/activity-executions:view`
`read:workflow-runtime`	`workflows/runtime:view`
`ManageWorkflowRuntime`	`workflows/runtime:control`
`read:bookmark-queue:dead-letters`	`workflows/bookmark-queue/dead-letters:view`
`replay:bookmark-queue:dead-letters`	`workflows/bookmark-queue/dead-letters:replay`
`delete:bookmark-queue:dead-letters`	`workflows/bookmark-queue/dead-letters:delete`
`trigger:event`	`workflows/events:trigger`
`tasks:complete`	`workflows/tasks:complete`
`exec:tests`	`workflows/tests:execute`
`read:activity-descriptors`	`workflows/descriptors/activities:view`
`read:activity-descriptors-options`	`workflows/descriptors/activities:view`
`read:expression-descriptors`	`workflows/descriptors/expressions:view`
`read:storage-drivers`	`workflows/descriptors/storage-drivers:view`
`read:variable-descriptors`	`workflows/descriptors/variables:view`
`read:commit-strategies`	`workflows/descriptors/commit-strategies:view`
`read:incident-strategies`	`workflows/descriptors/incident-strategies:view`
`read:log-persistence-strategies`	`workflows/descriptors/log-persistence-strategies:view`
`read:output-converters`	`workflows/descriptors/output-converters:view`
`read:workflow-activation-strategies`	`workflows/descriptors/activation-strategies:view`
`read:javascript-type-definitions`	`workflows/scripting/javascript:view`
`exec:csharp-expressions`	removed — see #7975
`exec:python-expressions`	removed — see #7975
`read:user`	`identity/users:view`
`create:user`	`identity/users:create`
`update:user`	`identity/users:update`
`delete:user`	`identity/users:delete`
`read:role`	`identity/roles:view`
`create:role`	`identity/roles:create`
`update:role`	`identity/roles:update`
`delete:role`	`identity/roles:delete`
`create:application`	`identity/applications:create`
`read:secrets`	`secrets:view`
`write:secrets`	`secrets:write`
`delete:secrets`	`secrets:delete`
`test:secrets`	`secrets:test`
`use:secrets`	removed — unused
`import:secrets`	removed — unused
`export:secrets`	removed — unused
`external-authentication:connections:read`	`external-authentication/connections:view` + `external-authentication/descriptors:view`
`external-authentication:connections:create`	`external-authentication/connections:create`
`external-authentication:connections:update`	`external-authentication/connections:update`
`external-authentication:connections:archive`	`external-authentication/connections:archive`
`external-authentication:connections:test`	`external-authentication/connections:test`
`external-authentication:connections:preview`	`external-authentication/connections:preview`
`external-authentication:links:manage`	`external-authentication/identity-links:view` + `external-authentication/identity-links:write` + `external-authentication/identity-links:delete`
`external-authentication:sessions:read`	`external-authentication/sessions:view`
`external-authentication:sessions:revoke`	`external-authentication/sessions:revoke`
`external-authentication:policies:manage`	`external-authentication/policies:view` + `external-authentication/policies:update`
`external-authentication:roles:assign`	`external-authentication/policies/default-roles:update`
`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:dashboard`	`dashboard:view`
`read:diagnostics:console-logs`	`diagnostics/console-logs:view`
`read:diagnostics:structured-logs`	`diagnostics/structured-logs:view`
`read:diagnostics:opentelemetry`	`diagnostics/opentelemetry:view`
`ingest:diagnostics:opentelemetry`	removed — unused
`read:resilience`	`resilience/*:view`
`read:resilience:retries`	`resilience/retries:view`
`read:resilience:strategies`	`resilience/strategies:view`
`exec:resilience`	`resilience/*:execute`
`exec:resilience:simulate-response`	`resilience/simulation:execute`
`read:alterations`	`alterations:view`
`run:alterations`	`alterations:execute`
`read:labels`	`labels:view`
`create:labels`	`labels:create`
`update:labels`	`labels:update`
`delete:labels`	`labels:delete`
`read:tenants`	`tenants:view`
`write:tenants`	`tenants:write`
`delete:tenants`	`tenants:delete`
`execute:tenants:refresh`	`tenants:refresh`
`read:installed-features`	`system/features:view`
`actions:shells:reload`	`system/shells:reload`
`ai:chat`	`ai/chat:execute`
`ai:tools:view`	`ai/tools:view`
`ai:capabilities:view`	`ai/capabilities:view`
`ai:tools:manage`	removed — unused
`ai:proposals:view`	removed — unused
`ai:proposals:approve`	removed — unused
`ai:proposals:apply`	removed — unused

14 KiB Raw Blame History