AssignmentMatcher implements CacheableDependencyInterface so a consumer can merge what a match rests on. It reports the user context plus the delegation and tenant metadata, but not the acting account own cache tag, and it cannot, because getCacheTags() takes no account. So five consumers add the account tag by hand, each with a near-identical three-line comment explaining that candidacy reads roles and the user context keys rather than invalidates: WorkItemManager::checkActionAccess(), OperationAccessCheck, InstanceReadAccessCheck, InboxController::checkReassignAccess() and AssignmentCacheTrait.

One rule stated five times, and a sixth consumer that forgets it fails silently. That sixth consumer already exists.

The webform up-front refusal declares less than its verdict reads

OrchestraInteractionHandler::refuseSubmitThatCanDoNothing() disables the webform submit and warns the visitor, and declares that decision cacheability by hand as two lines: the token list tag and the user context. The verdict it declares for is OperationResumer::checkCompleteAccess(), which reads four things: the token state, the resolved tenant, standing cover that lapses on the clock, and the account roles. Three of the four are undeclared.

So a form page cached while a stand-in could not yet complete the step keeps showing that the step is not theirs to complete after the delegation starts, and keeps offering the submit after it lapses or the role is revoked. It is not a bypass, because preSave() re-checks, but the visitor is told the wrong thing and the delegation case silently blocks the person brought in to cover. The resumers carry no cacheability on the stated grounds that the answer is never merged into a render array, which stopped being true when #3621674: Refuse a form up front when submitting it can do nothing rendered it into a form.

Two tenant checks made uncacheable instead of declaring the tenant

InstanceTenantAccessCheck and IncidentTenantAccessCheck both end with setCacheMaxAge(0) where TenantContext is a declared cacheable dependency. The tree already declares it that way in TenantCacheContext, in the matcher itself and in the webform handler. Their two branches also declare different contexts: the administrator branch is cachePerPermissions() and the tenant branch has none, and the max-age zero is what currently hides that, so removing one without the other would leave a granted administer orchestra not flipping the verdict. A missed site of the lane #3621292: Cacheability and cost on paths that pay for nothing closed.

What replaces the max-age is the tenant context declared as a dependency, and that is only safe because of one thing: a realm derived from the request partitions the verdict by whatever request property it was derived from. orchestra_domain reads the host and reports url.site; orchestra_server_api reads the authenticated consumer and reports user; and TenantContext merges every registered resolver cacheability whether or not that resolver was the one that answered, so it over-partitions rather than under. Remove any link in that chain and a confinement becomes a cache hit for the wrong realm.

The fix is one seam

Give the matcher an account-aware cacheability method carrying the whole set, have checkCompleteAccess() carry it, and merge that one thing at every site instead of restating a subset of it.

Tests

A kernel test that the matcher account-aware cacheability names the account tag and the delegation bound; a kernel test that the webform refusal render array carries the tenant, the cover bound and the account tag; and a kernel test that every verdict the two tenant checks give is cacheable and declares the permission context, on both branches and for the run, the token and the incident alike, with a second test that the confinement itself still refuses another realm so the cacheability was not bought by allowing everything.

And one that follows the realm cacheability along the chain above, because none of it was asserted anywhere: the tests with no resolver installed are the single-realm case, which is the one arrangement where the max-age never mattered and its removal cannot show. A resolver reporting the context the shipped domain one reports is registered, and the context is followed from it, through the tenant context, to every verdict.

AI-Generated: Yes (Claude Code performed the audit that found this, drafted this summary, and wrote the fix and its tests on the merge request. I reviewed the findings against the source, and ran the tests, before posting.)

Issue fork orchestra-3621882

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

mably created an issue. See original summary.

mably’s picture

Status: Active » Needs review
mably’s picture

Issue summary: View changes

  • mably committed bc6135d1 on 1.x
    task: #3621882 Declare what a per-viewer verdict rests on once, at the...
mably’s picture

Status: Needs review » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.