- Issue created by @ergonlogic
- Status changed to Postponed
about 1 year ago 12:32am 5 April 2024 - ๐จ๐ฆCanada ergonlogic Montrรฉal, Quรฉbec ๐จ๐ฆ
Here's a patch.
Marking as postponed until I have a chance to create an MR against 2.0.x
- Status changed to Needs work
3 months ago 9:11am 10 January 2025 - ๐บ๐ธUnited States freelock Seattle
We hit an issue where we had multiple events that use the "Determining Entity Access" plugin, and hit this issue. After reviewing, I think the solution proposed by @ergonlogic is incorrect -- I do think the result should get initialized with AccessResult::neutral(). However, if there is more than one of these events in your site, the last one wins because of this reset.
In our case two access checks applied to a particular action:
node:application, update
node:*, view,updateThe first was setting an AccessResult::allowed, the second AccessResult::neutral. After processing the ECA event, AccessResult::neutral was the final result, and access ended up being denied.
I think the correct solution is to make use of AccessResult::orIf(). In the event plugin, setAccessResult() method, I think it should see if it already has an access result set, and if it does, replace it with $this->accessResult->orIf($result).
This allows all the access rules to get checked, and reasons are available for all of the models.
The logic for the orIf() call is to return AccessResult::forbidden if either the existing or new accessresult is forbidden; return AccessResult::allowed if either the existing or new result is allowed, and return AccessResult::neutral if both are neutral.
The previous solution is still subject to the "last check wins" if multiple checks return something other than neutral.
- Merge request !461Issue #3438777 by ergonlogic, freelock: ECA Access: Do not set AccessResult in applies() โ (Open) created by freelock
- ๐ฉ๐ชGermany jurgenhaas Gottmadingen
In ECA 2.0, the initialisation with neutral has already been built in as part of ๐ Enable event plugins to determine appliance Fixed . It's now contained in
\Drupal\eca_access\Plugin\ECA\Event\AccessEvent::appliesForWildcard
. The problem with that is in fact, if multiple ECA events are subscribing to the same Drupal event, then each of them initializes the access result again.But the proposed MR resolves that issue correctly. Thanks @freelock for that suggestion. I've just left a couple of comments that are simple to apply.