Skip to content

Apply can directive action when the field resolves to a promise - #2786

Open
dxiiren wants to merge 1 commit into
nuwave:masterfrom
dxiiren:fix-can-directive-action-deferred
Open

Apply can directive action when the field resolves to a promise#2786
dxiiren wants to merge 1 commit into
nuwave:masterfrom
dxiiren:fix-can-directive-action-deferred

Conversation

@dxiiren

@dxiiren dxiiren commented Aug 2, 2026

Copy link
Copy Markdown

Resolves #2758

  • Added or updated tests
  • Documented user facing changes
  • Updated CHANGELOG.md (skip for docs-only changes)

Changes

BaseCanDirective decides what to do with an authorization failure based on the action
argument, but it only sees failures thrown synchronously:

try {
    $resolved = $this->authorizeRequest(...);
    if ($hasResolved) {
        return $resolved;
    }
} catch (\Throwable $throwable) {
    // EXCEPTION_NOT_AUTHORIZED / RETURN_VALUE handled here
}

CanResolvedDirective authorizes through Resolved::handle(), which chains onto a promise
when it gets one:

if ($resolved instanceof SyncPromise) {
    return $resolved->then($handle);
}

return $handle($resolved);

With batchload_relations enabled a relation resolves to a SyncPromise, so $handle
runs when that promise is fulfilled, which is after the try-catch above has been left. The
AuthorizationException escapes it, and action is ignored: RETURN_VALUE never returns
its value and EXCEPTION_NOT_AUTHORIZED never rewrites the message. Turning
batchload_relations off makes the same schema behave as documented, which is what makes
this surprising.

The catch body moves into handleAuthorizationFailure() and is also chained onto the
promise as a rejection handler, so the action applies whether the field resolves eagerly
or through a batch loader.

The added test covers a batch loaded @hasMany guarded by
@canResolved(action: RETURN_VALUE, returnValue: null). On current master the field
comes back null but an This action is unauthorized. error is reported alongside it,
which is exactly the ignored action. With this change the field is null and no error is
reported.

Verified on PHP 8.3.33 / Laravel 13.23.0 against MySQL and Redis, using the repository's
own Docker setup. All 90 tests across CanDirective, CanFindDirective, CanModelDirective,
CanQueryDirective, CanResolvedDirective and CanRootDirective pass.

Breaking changes

None for schemas that behaved correctly already. Fields that previously leaked an
authorization error despite action: RETURN_VALUE now return the configured value, which
is the documented behavior and the point of the fix.

BaseCanDirective handles an authorization failure according to the action
argument, but only catches what is thrown synchronously. A batch loaded
relation resolves to a SyncPromise, so CanResolvedDirective authorizes inside
its then() callback, which runs after the try-catch has been left. The failure
escaped, so RETURN_VALUE and EXCEPTION_NOT_AUTHORIZED were silently ignored
whenever batchload_relations was enabled.

Chain the same handling onto the promise rejection so the action applies
regardless of whether the field resolves eagerly or through a batch loader.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

@canResolved directive action ignored when batchload_relations enabled

1 participant