Separate access and executable checks and make access completely optional

I'm very much in favour of this proposal. Event want to add to the global user switch issue that we found this to be so bad in most cases because most access control features become unusable in that context, and we've banned that feature from all our projects.

My only worry is that separating access from executable checks is not always that obvious. E.g. in the given example with loading an entity, what else should be access controlled if not the view permission on the given entity? And to accomplish that, the entity needs to be loaded, which inherently also checks that the entity even exists. What I'm saying here is that in this case there won't be any access control possible if this will be part of the executable check.

Nevertheless, making access control optional sounds tempting, and I could see a lot of admin ECAs benefitting from that. But that setting should probably not be global or on a per action plugin basis, it should be configurable per ECA model. That's the only way to make them shareable across multiple sites.

Yeah, the global user switch is a rabbit hole where it's hard to get out from later on.

And to accomplish that, the entity needs to be loaded, which inherently also checks that the entity even exists. What I'm saying here is that in this case there won't be any access control possible if this will be part of the executable check.

In this example, yes it obviously needs to load it. Maybe not the best example picked up. Still there may be an executable method that only checks whether it exists. If I don't care about the actual access check then, I could skip it. Loading the entity multiple times here could be optimized in case we see a potential performance issue.

But that setting should probably not be global or on a per action plugin basis, it should be configurable per ECA model.

Sounds good.

In the example of LoadEntity, the refactor may look something like this:

class LoadEntity extends ConfigurableActionBase ... {

  /**
   * {@inheritdoc}
   */
  public function executable(...$args): ExecutableResultInterface {
    if ($this->doLoadEntity($object)) {
      return ExecutableResult::executable();
    }
    return ExecutableResult::notExecutable('No entity available.');
  }

  /**
   * {@inheritdoc}
   */
  public function access($object, ?AccountInterface $account = NULL, $return_as_object = FALSE) {
    $reason = NULL;
    $entity = $this->doLoadEntity($object);
    $access_result = $entity->access('view', $account, TRUE);
    if (!$access_result->isAllowed()) {
      $reason = 'No permission to view the entity.';
    }
    if ($reason !== NULL && $access_result instanceof AccessResultReasonInterface) {
      $access_result->setReason($reason);
    }
    return $return_as_object ? $access_result : $access_result->isAllowed();
  }

}

// Within ECA Processor:

if ($action->executable(...$args)->isExecutable() && $action->access(...$args)->isAllowed()) {
  $action->execute(...$args);
}

.. which would streamline the access check to its purpose, since it doesn't need to check anymore whether the entity exists as this was already checked by executable().

Looks great. And in the processor, where we call $action->access(...$args)->isAllowed(), this can be disabled e.g. per ECA model.