Why does Route access checking differ from Entity access checking?

andIf()

  |A N F
--+-----
A |A N F
N |N N F
F |F F F

orIf()

  |A N F
--+-----
A |A A F
N |A N F
F |F F F

The behavior you're describing is supported by orIf().

A normal truth table for "OR" would be

|T F
--+---
T |T T
F |T F

So you can think of it this way:
Allowed is TRUE
Neutral is FALSE

Forbidden overrides everything always, regardless of the OR

Neutral should always be neutral, meaning that the andIf() truth table is fundamentally wrong in core.

OR IF:

   |A N F
 --+-----
 A |A A F
 N |A N F
 F |F F F

AND IF:

   |A N F
 --+-----
 A |A N F
 N |N N F
 F |F F F

Should be:

OR IF:

   |A N F
 --+-----
 A |A A A
 N |A N F
 F |A F F

AND IF:

   |A N F
 --+-----
 A |A A F
 N |A N F
 F |F F F

Neutral in all cases must be ignored. The only case it should not if the end result case, if it's neutral, a policy should be set to either "grant on neutral" or "deny on neutral" but be set explicitely.

EDIT: Answering to:

So you can think of it this way:
Allowed is TRUE
Neutral is FALSE

Then neutral should not exist. In real life, it MUST exists, because access checkers CAN bail out if they wish, then it should remain neutral as appropriate.

To answer to @ivi.arocom original question: if you want to check access for entities, use the entity hook_*_access() because you will experience unexpected behaviours otherwise, route uses entity access, but other pieces of API do not use the route access checks, if both differs, you will experience a diverging behaviour in your site you will not debug easily.

EDIT: clearer answer: everything you will implement as a route access check will NOT be executed on entity access check (for example, for generating entity operation links) and this probably NOT what you want: the right place to do anything is entity access API. It's just a shame you cannot implement it any other way than using a hook.

This is the origin of these truth tables: https://boa.unimib.it/retrieve/handle/10281/46285/69194/ins250-web.pdf
See https://en.wikipedia.org/wiki/Many-valued_logic for an overview.

@tim.plunkett OK I see it makes more sense with that, but first, why wasn't it documented in the code ? And second question, I don't see the inderminate state being the same as "neutral" for the access check use case. And for what I see, I can understand this logical mathematics, but it's probably a huge DX fail, most developers will assert that neutral is neutral, not indeterminate (neutral is not the right semantics if you wished to implement the correct Kleene (strong) K3 and Priest logic P3 as I can read it.

And for what it worth, those are logics I am discovering right now, I always learnt only boolean logic at school. Fact is, most programmers are in the same case than me. In the end, access checks are supposed to be boolean, and say only true or false, there is no place for inderminate, either the user has access, either he has not, there is no heseinpermissions (sorry for the vulgarisation, no inderminate case exists in such use case) in route or entity access. I think using that using many valued logic for access checking is an error in the end, it will only confuse people.

Moreoever, Symfony considers neutral as neutral, it just drops those results, and say deny if none granted (depending on the access decision manager policy, of course, but default is that one grant is a global grant, it's configurable thought). Current Drupal core logic goes against that, and diverges once step more away from Symfony and other frameworks, it's really not helpful.

Originally it was ALLOW, DENY, KILL. and DENY was indeterminate.

#2340507: Make the new AccessResult API and implementation even better changed the implementation to what it is now.

#2343631: Improve AccessResultInterface andIf()/orIf() docs improved the docs, removing the reference to the "Many-valued logic" wiki entry, instead adding the truth table.

I'm not sure what would be better than what we have, but I agree it is confusing.

I read both discussion, all of that makes sense, there was a need for the KILL which is "Forbidden" in core, and DENY is "Neutral" in core, but there is no PASS which is "Abstain" in Symfony. This means that a security listener cannot just say "I'm not concerned, my decision has no impact". This is a serious flaw in the design.

Wording is so wrong, in all other frameworks, DENY means the Drupal NEUTRAL. It should have remain DENY for common mortal, and KILL instead of FORBIDDEN would have been much more explicit. And it lacks PASS/ABSTAIN.

And, there is KILL but no "Force allow" and sometime, that's what the developer want to do. Main problem is that sometime, you cannot force "allow" using a custom access service without actually writing a compiler pass that removes core ones, and that's not always possible: for example, we could voluntarily force menu access where there's an entity access, but still want to keep the entity access service in core for other routes: in this case, you cannot simply remove the core menu entity access service from the container.

Moreover, in entity.api.php we can read this example (and tons of other):

function hook_entity_access(\Drupal\Core\Entity\EntityInterface $entity, $operation, \Drupal\Core\Session\AccountInterface $account) {
  // No opinion.
  return AccessResult::neutral();
}

Which is misleading, because it's "no opinion" only because the entity access uses orIf(), but when using andIf() this would be called "deny" instead. It's not documented anywhere that entity access uses or and route access uses and.

block.api.php has an even worst example (which basically is not false either due to or usage):

function hook_block_access(\Drupal\block\Entity\Block $block, $operation, \Drupal\Core\Session\AccountInterface $account) {
  // Example code that would prevent displaying the 'Powered by Drupal' block in
  // a region different than the footer.
  if ($operation == 'view' && $block->getPluginId() == 'system_powered_by_block') {
    return AccessResult::forbiddenIf($block->getRegion() != 'footer')->addCacheableDependency($block);
  }

  // No opinion.
  return AccessResult::neutral();
}

I agree that the discrepancy between Route access checking (andIf) and Entity access checking (orIf) is confusing. I think the current situation is a historical artifact. Here's the timeline of what happened as I understand it (which I uncovered by looking at the git log of Drupal\Core\Access\AccessManager).

1. In #1896556: Routing AccessManager does not evaluate access checks correctly, the route access checking worked the same as entity access checking (the equivalent of what is now called orIf) with TRUE, FALSE, NULL being the then equivalents of what is now allowed, forbidded, neutral.
2. In #1984528: Follow-up: Allow for more robust access checking, NULL got renamed to DENY, and _access_mode was added to allow routes to specify whether to conjuct with ALL (and) logic or ANY (or) logic, with the default being ANY.
3. In #2063401: Replace the default _access_checks(access mode) with ALL instead of ANY, the default was changed to ALL.
4. In #2431281: Drop support for _access_mode routes and always assume ALL, support for ANY was dropped.

In a model where we wanted to support the choice of ALL or ANY, most permission checks were written to return either ALLOW (TRUE) or DENY (NULL), with KILL (FALSE) being a rarity. For example, PermissionAccessCheck returned DENY (NULL) when the user did not have permission, and so even now in HEAD, returns AccessResult::neutral() in that case.

However, in a model where we only need to support one conjunction mode, it would probably be clearer for permission checks that need to disallow access (e.g., PermissionAccessCheck) to return forbidden() rather than neutral() when the user lacks permission.

If we made that conversion for all core access checks and expected contrib access checks to also make that same change, then we could replace the andIf with orIf and still have the desired behavior. At which point, neutral() would be free to mean "no opinion", as its name implies.

I don't currently have any ideas on how to go about making such a change in a BC friendly way though. If we simply change the access manager from calling andIf to calling orIf, then existing contrib route access checkers that return neutral() when they mean "not allowed", as the core ones currently do, will start causing security (access bypass) vulnerabilities.

I don't currently have any ideas on how to go about making such a change in a BC friendly way though.

Now I do. How about this:

1. Add an interface with no methods. Name TBD, but e.g., Drupal\Core\Routing\Access\NeutralityCapableAccessInterface.
2. Route access checks that want to return AccessResult::neutral() (and have it mean what the word "neutral" means) SHOULD be changed to implement that interface (which, since there's no methods on the interface, just means adding it to the class declaration line).
3. Change AccessManager from using andIf() to using orIf(). However, for BC, when it encounters a checks that does not implement NeutralityCapableAccessInterface but returns AccessResult::neutral(), then:
   1. Trigger a deprecation notice.
   2. ->orIf() with AccessResult::forbidden() instead.

Thanks for the very detailed historic and comprehensive explanation.

From #2063401 I can read:

If a route needs several access checkers it currently uses an OR/ANY logic to concat them. Developers though expect it to be ALL/AND by default.

I think that this assertion is false, in most voter-based frameworks default behaviour is the opposite actually, this was a bad move.

Ok, and reading #2431281 I'm certain that this was an issue only because it lacks the "pass/abstain" behaviour, and all those inconsistencies were born from the fact that the system was inconsistent (or incomplete, depend on the point of view) to start with.

Now to answer your question:

I don't currently have any ideas on how to go about making such a change in a BC friendly way though. If we simply change the access manager from calling andIf to calling orIf, then existing contrib route access checkers that return neutral() when they mean "not allowed", as the core ones currently do, will start causing security (access bypass) vulnerabilities.

An easy way is just to introduce the real pass/abstain option, with that, the actual model with true/false/indeterminate based upon Kleene logic will work as it is. This would be a new feature, without any break in compatibility.

I insist on the fact that semantics are wrong, neutral should be documented as indeterminate and Kleene logic mentioned within the internal AccessResult class code, but exposed semantic must be changed to something in the security domain model from other frameworks, and since Drupal tends to get closer to Symfony, using Symfony's semantic would be good (i.e. introduce the new ABSTAIN state, and just rename NEUTRAL to DENY, then FORBIDDEN to KILL).

For this, all that needs to be done is actually:

• keep the existing methods, but marking them as being obsolete (using @deprecated in PHPdoc so that editors will highlight it for developers),
• make core use those new methods with new names (self-documentation),
• write proper documentation on AccessResultInterface interface and AccessResult class,
• provide trigger_error() deprecation warnings in those methods,
• using the ABSTAIN state, orIf() usage is not necessary anymore, just deprecate it, and introduce a simple and() method.

That's a solution I can see without thinking too much, I might be wrong of course, there's probably better. In fact I have no real proper solution, because what I really think is that exposing all those methods to business code is wrong in my opinion, I would prefer very much a much more basic voter system such as Symfony's one, which does a dawn good job.

But thinking about introducing NeutralityCapableAccessInterface feels wrong, everywhere in Drupal there's tons and tons of interfaces that have been introduced the same way, and believe me I was catching up with my Drupal 8 knowledge during the last month and those redundancies tend to be very, very confusing.

I like the idea of moving closer to Symfony's semantics. From the information in https://symfony.com/doc/current/components/security/authorization.html#c..., here's how I see the mapping:

Drupal's AccessResultAllowed = Symfony's VoterInterface::ACCESS_GRANTED.
Drupal's AccessResultNeutral = Symfony's VoterInterface::ACCESS_ABSTAIN.
Drupal's AccessResultForbidden = Symfony's VoterInterface::ACCESS_DENIED .

The current decision maker strategy for entity access is Symfony's unanimous strategy: only grant access if none of the voters has denied access.

The current decision maker strategy for route access doesn't map to one of Symfony's. If I were to name it, I'd call it strong_unanimous: only grant access if none of the voters has denied access or abstained.

I think a strong_unanimous strategy doesn't make sense, because it basically has the effect of preventing the semantics of "abstain".

I think it might be worth exploring actually bringing in Symfony's Security component and seeing if we can refactor Drupal's code to use VoterInterface and friends. If we do that, then I think we can start using the unanimous strategy for both entity and route access. However, for BC, within the routing system decision maker, we'd need to treat legacy AccessResultNeutral votes (that we can somehow distinguish from new-style VoterInterface::ACCESS_ABSTAIN votes) as the equivalent of VoterInterface::ACCESS_DENIED.

I agree with all of #16 - moving toward Symfony security component +9000. I use it in some custom Drupal 8 modules, and ties it with Drupal in the end using some hook magic, but I'm quite used to the voter system and I love it.

But that put aside, thing that'll disappear by doing that is cache metadata handling within permissions.

To go further, but that's out of this scope, using all of security component (decision manager, authorization checker, basic user interfaces, etc...) would be a good thing (I'd love to have the is_granted() method in twig templates for example) but that's another topic, and it would break way too much in core. The more Drupal uses symfony's code, the more it'll become consistent and the less code you would have to maintain at the same time.

Have marked #2861074: AccessManager assumes allowed and treats neutral as forbid as a duplicate of this.

It seems to me from reading through this issue, the consensus is D9 would benefit from using Symfony security component (which would resolve the problem). However, for D8 that isn't going to be possible. Two possible solutions:

1. As per #2991698-14: Why does Route access checking differ from Entity access checking? we could add a Drupal\Core\Routing\Access\NeutralityCapableAccessInterface or similar to allow access checks to determine if they are merged using an andIf vs orIf.
2. We could add a new router option which allows us to control which merging method is used for access checks on a particular route (defaulting to the existing andIf). This would allow developers to switch specific routes to behave as required.

Both seem to me to have their own merits/disadvantages and perhaps both would be worthwhile.

I've spent the last two hours investigating why my custom entity access check, altough succesfully returning Allowed access when expected, was still resolved as a 403.

This bug actually makes impossible to provide a custom entity access check. Because if the Core entity access check returns Neutral, then it is over, whatever happens after that will lead to a 403.

This is a bummer. The only solution is to entirely replace EntityAccessCheck service by a subclass and call the parent only when our custom implementation would return Neutral.

/**
 * Provides a custom access checker for entities.
 */
class EntityAccessCheck extends \Drupal\Core\Entity\EntityAccessCheck
{
  /**
   * @param Route $route
   * @param RouteMatchInterface $route_match
   * @param AccountInterface $account
   * @return bool|AccessResultInterface|AccessResultNeutral
   */
  public function access(Route $route, RouteMatchInterface $route_match, AccountInterface $account)
  {
   /** @var AccessResultInterface $result */
    $result = NULL;

    // Split the entity type and the operation.
    $requirement = $route->getRequirement('_entity_access');
    list($entity_type, $operation) = explode('.', $requirement);
    // If $entity_type parameter is a valid entity, call its own access check.
    $parameters = $route_match->getParameters();

    if ($parameters->has('entity')) {
      $entity = $parameters->get('entity');

      if ($entity instanceof EntityInterface) {
        $result = $entity->access($operation, $account, TRUE);
      }
    }

    if ($result === NULL) {
      $result = parent::access($route, $route_match, $account);
    }

    return $result;
  }
}

Another problem:

$entity->access('view');

Doesn't trigger hook_entity_access either.
https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Entity%21...

Edit: Ok my bad, I tested it with create, and therefore's another hook: hook_entity_create_access
https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Entity%21...

Indeed this is bad DX.... There should be big warning signs around this issue.

I really would love to help because I hit this issue right now, even if I have no idea how. But let's try to work something out together.

First of all, the history of all this goes way further back than Drupal 8. Way, way, way. Think Drupal 4 or something. The earliest api.drupal.org has is 4.6 but by no means did this originate with a release as new as 4.6 from 2005 ;)

Step one is https://api.drupal.org/api/drupal/modules%21node.module/function/node_ac...

  $access = module_invoke(node_get_module_name($node), 'access', $op, $node);
  if (!is_null($access)) {
    return $access;
  }

This made sense because the intent here is if the access checker exists then we return its results otherwise continue. At this time we still only had a single access checker but as you can see it already could return deny (FALSE), allow (TRUE) or abstain (NULL) even if noone designed the system to be so, the implementation made it so. Familiar?

The next step in mutation was in Drupal 7 https://api.drupal.org/api/drupal/modules%21node%21node.module/function/...

  // We grant access to the node if both of the following conditions are met:
  // - No modules say to deny access.
  // - At least one module says to grant access.
  // If no module specified either allow or deny, we fall back to the
  // node_access table.

A step in the right direction because instead of TRUE/FALSE new constants were introduced. Please note because this is terribly important: if every access checker just shrugged, access was denied.

Moving on, the other half comes from hook_block_list_alter where access checking was done by unsetting any blocks the hook didn't like. Simple? Yes, but it will have devastating consequences later because here if every access checker just shrugged, access was allowed!

Comes Drupal 8 and blocks and nodes are both entities and need to be shoehorned into the same access checker service. Hilarity ensues. Or not.

As for why Symfony security component was not used, check #1793520: Add access control mechanism for new router system:

Symfony Full Stack uses a separate Security component for controlling route access that is far too complex for us to integrate in Drupal 8. We're not going there right now.

One more call was made for it in #2062151-22: Create a current user service to ensure that current account is always available but it again was decided against. I think given the shackles of backwards compatibility getting there can't happen earlier than Drupal 10 and given the security implications it would be quite difficult to get there. Maybe something along the lines of AccessResult objects implementing Symfony VoterInterface...? Just a conversation starter, I do not know.

This issue mentions:

It's not documented anywhere that entity access uses or and route access uses and.

It is probably too hard to find but Daniel and I did add a warning exactly about that to hook_entity_access:

Be careful when writing generalized access checks shared between routing and entity checks: routing uses the andIf() operator. So returning an isNeutral() does not determine entity access at all but it always ends up denying access while routing.

Should we move, copy, link this warning elsewhere? Where? Maybe https://www.drupal.org/docs/8/api/routing-system/access-checking-on-rout... would be a good place? I do not know where to document it in core because AccessInterface has no methods and might or might not stay: #2266817: Deprecate empty AccessInterface and remove usages

I read all comments and related issues, but initial problem still not solved.

I agree with the fact that neutral should be neutral, like Symfony does. I understand choices explained in #7 have been made, but even more agreed with #8.

In any cases, if we decide to keep in #7 rules, we should at least provide a solution as explained in #14 (even if i don't think we need to implements all the NeutralityCapableAccessInterface part).

This will solve the main issues : access check in routes also depends of access checks order.

Neutral + Neutral = Neutral (so Forbidden) => OK
Neutral + Allowed = Neutral (so Forbidden) => KO
Allowed + Neutral = Allowed => OK
Allowed + Allowed = Allowed => OK

Currently issue by example:

1. A route implements an access_check with custom_access. Result of this custom access is Neutral (based on the fact neutral is default a forbidden, and for route check point of view, nothing permit to define we have permission to access, and nothing define we are not allowed).
2. Another module wants to add its own access check implementing the access_check tag on custom_access. This modules will never be able to make an accessResult::allowed because Route return accessResult::Neutral.

Making AccessResult::Neutral default in AccessManager::check() and replacing andIf() by orIf(), issues is fixed and other access checks should not be altered (tests should confirm this).

Otherwise, AccessManager for route cannot be altered, and the only solution is to override the route access check, which can be done by only one module (this remove flexibility).

In \Drupal\Core\Access\AccessManager::check()

    $result = AccessResult::neutral();
    if (!empty($checks)) {
      $arguments_resolver = $this->argumentsResolverFactory->getArgumentsResolver($route_match, $account, $request);
      $result = AccessResult::allowed();
      foreach ($checks as $service_id) {
        $result = $result->andIf($this->performCheck($service_id, $arguments_resolver));
      }
    }

Should be replaced by

    $result = AccessResult::neutral();
    if (!empty($checks)) {
      $arguments_resolver = $this->argumentsResolverFactory->getArgumentsResolver($route_match, $account, $request);
      foreach ($checks as $service_id) {
        $result = $result->orIf($this->performCheck($service_id, $arguments_resolver));
      }
    }

We should also keep in mind every changes in AccessManager should be reported on Drupal\webprofiler\Access\AccessManagerWrapper from devel module.

Attach a patch to see tests results.
Set this as a bug since we have no solutions to make access alterable with neutral (no opinion) result.

Thanks for the patch. Please read #13 as why this is not viable by itself (especially the last paragraph of it) and #14 as a way for forward from it. But note the issue after those comments tentatively argued for moving towards the Symfony security component although no concrete path exists.

I believe a clean break and temporary adding a "symfony_security: true" key-value to route requirements for the new kind of checked routes could work and then in Drupal 10 we could drop the current system.

I support #14, minimally-bc-breaking way forward. Postpone moving to Symfony's access model (adding Neutral -> Deny, add Abstain, andIf and orIf -> and) until there's a demonstrable benefit.

A deprecation in a widely-used feature costs maybe 10^3 or 10^4 developer hours for all the contrib and custom modules in existence. Each deprecation "cut" wounds those trying to stay with Drupal but facing non-productive* development costs. I'd prefer core committer save the deprecations for things that are really broken, not for things that aren't as pretty as they could be. My $0.02.

* This deprecation might take me 10 hours to change all my access results and thoroughly test. At the end, my module has no new features or benefits. Say a Drupal developer is worth $100/hour. That change cost me $1000 of actual money.

This disconnect and entity access in general is where I've found most security issues when reviewing custom code. Anything we can do to make this easier to understand for developers will be a win. But we do have to be super careful and make sure whatever changes we make don't make it harder to understand if you know how it currently works.

My concern with #14 is that during the transition period developers might get even more confused about how all this is supposed to work.

I wonder if there is an alternative to #14 where we deprecate AccessResult::neutral() and ask developers to replace it with either AccessResult::forbidden() in andIf() contexts or a new AccessResult::abstain() in orIf() contexts. Hopefully this conversion could be automated.

AccessResult::abstain() is simply ignored by both andIf() and orIf().

This might make for a less confusing transition as it leaves the semantics of neutral() unchanged and so all existing documentation, tutorials and examples are still valid as written.

I just ran into this myself. To summarize what we currently have in core and why we absolutely need to move to another system:

• Individual entity access checks: If anyone says YES, you're in unless someone says NO. Abstaining has no impact.
• Entity list access checks (node grants): If anyone says YES, you're in, even if someone says NO. Abstaining has no impact.
• Route access check: If everyone says YES, you're in. Abstaining denies access.

Where is the consistency? Trying to avoid code duplication by calling $entity->access() in a route access check is near impossible because of this.

In the meantime: What would be a good workaround to adding another listener to an 'access_check' tagged service? Right now, it doesn't do much because if the original listener returns neutral, your service is shit out of luck.

This is particularly frustrating because for most routes (e.g. a node) Drupal's entity access check will be the first one tried when looping through the $checks in AccessManager::check(). It has this code:

public function access(Route $route, RouteMatchInterface $route_match, AccountInterface $account) {
  // Split the entity type and the operation.
  $requirement = $route->getRequirement('_entity_access');
  [$entity_type, $operation] = explode('.', $requirement);
  // If $entity_type parameter is a valid entity, call its own access check.
  $parameters = $route_match->getParameters();
  if ($parameters->has($entity_type)) {
    $entity = $parameters->get($entity_type);
    if ($entity instanceof EntityInterface) {
      return $entity->access($operation, $account, TRUE);
    }
  }
  // No opinion, so other access checks should decide if access should be
  // allowed or not.
  return AccessResult::neutral();
}

The "no opinion" comment here is extremely misleading, because this neutral that gets returned actually prevents any check after it from granting access. So the only way of allowing it to be viewed is via the entity access permissions.

I have spotted another issue with this, relating to other modules. It seems that layout builder will allow the "Layout" operation to be accessed by anyone with the "View node" permission if layouts are enabled per node, for example. This is pretty bad and of course should be fixed in Layout Builder's access checks. I'm flagging it here just in case anyone sees the same or similar issues after this patch has been applied.

Ditto with Group and some of these operations. Applying the patch is not recommended without very carefully checking your module configuration.

Solving this is far beyond me, but I've struggled with these inconsistencies and would love to see a solution.

I made a patch for the solution proposed in #27.
I think it's a good solution for BC and is easy to implement for everyone who needs it.

Then we can add a deprecation notice and switch for the Symfony access check in a future Drupal release maybe ?

Edit: To be clearer, this patch adds a "_symfony_routing_access" requirement that devs can add to their routes (or any route using a RouteSubscriber) in order to use the "Symfony" access check style.

Use $access_result->isForbidden() explicitly since Access::Neutral() is also not !$access_result->isAllowed()

Patch #44 resolves the problem for me.

I just ran into this problem using Edit Media Modal - users who have permission to edit the media object have AccessResultNeutral after the access checks are done, which leads to core failing the check instead of allowing it.

The test failures imply that a lot of core's permission logic does not return Forbidden but instead return Neutral, which the old logic took to mean "no" when it should have been "yes".

With #44 the /user/logout route redirects to /user, but for some reason it then redirects to /user/0 instead of /user/login, and then gets a 403 error. This seems to cause most of the test failures.

This issue highlights two major DX problems:
1. A lack of documentation around access logic, how it should be used and how modules should interpret it.
2. Limitations in core's access tests, which leads people to thinking that certain operations should work, when they don't.

The problem I ran into with edit_media_modal is a perfect example of this - the user has the "edit [media type]" permission, the contrib logic uses that permission to determine access, but the access check fails when the user should have the appropriate access.

From #42 it would seem #27 was not clear: I suggested _symfony_routing_access to indicate a route is using symfony/Security instead of Drupal's system. I did not suggest this flag to change the behavior of Drupal's system.

I also believe both #42 and #44 needs some consider whether whether generic modules changing route access would open security holes with them.

For small changes I think #14 was the only feasible one. But, of course, what do I know? This is just my thought.

For large changes you would need to bring in symfony/security.

Just to summarize the current situation in other words, hopefully not getting it wrong:

Route Access is allowed, if ALL access checks return AccessResult::allowed(). It’s denied if ANY access check does NOT return AccessResult::allowed().

Entity Access is allowed, if ANY access check returns AccessResult:allowed() and NO access check returns AccessResult::forbidden(). It’s denied if NO access check returns AccessResult::allowed() or ANY access check returns AccessResult::forbidden().

Re #49 Sounds about right. I also summarized it as such in #33

The patch in #44 allows anonymous users to edit menus (/admin/structure/menu/manage/*), so beware of that!

This issue is still causing issues for me. Neutral implies that the access hook doesn't care (i.e. it could be allowed or denied), so to have it result in denied if there's 1 neutral is a major cause of confusing bugs.

Is there any solution other than to just return allowed in all cases where you don't actually care? It feels wrong to do, but if it should be forbidden, presumably another hook will return as such.

Its even more confusing when you haven't used neutral, but allowedIf(). These functions in Drupal core AccessResult all assume neutral means neutral when it does not. This means allowedIf() is pointless, it should just always be allowed.

Does anyone else feel like this needs higher priority?