Update all access checkers to use static::ALLOW/static::DENY/static::KILL

In a lot of places in that patch, where FALSE used to be returned, you're now returning static::DENY, but shouldn't that always be static::KILL?

No, because more than half the time, the FALSE was wrong.

Yep that patch looks good to me; That's totally right, You can't have a decent access system relying on TRUE/FALSE alone. In most cases why should only one access check be able to block access to a route before everyone else has had a chance?

After the tone of the comments in #1984528: Follow-up: Allow for more robust access checking, you're a good man Daniel.

#3: that makes sense :)

#6: I apologize if the tone was perceived as harsh. I definitely was very frustrated. But having re-read my comments on that issue, I am reading almost solely observations and questions. Little emotion.

Reroll to fix exceptions, I ran FloodTest locally and it is now fully green, so hopefully the other tests will be too.

The remaining test failures in Drupal\rest\test\CreateTest look legitimate.

There's no issue summary at all!

It's almost as easy to write one as it is to add the tag.

So looking at the patch suppose that only checkers implementing StaticAccessCheckInterface should be updated?
Please clarify this

+++ b/core/modules/rest/lib/Drupal/rest/Access/CSRFAccessCheck.php
@@ -55,12 +55,12 @@ public function access(Route $route, Request $request) {
     // As we do not perform any authorization here we always return NULL to
     // indicate that other access checkers should decide if the request is
     // legit.
-    return NULL;
+    return static::DENY;
   }

This looks like a pre-existing logic error. If you have 3 checkers on a route, set to ALL, and one of them is CSRF, then you can never ever access the route. This needs to return ALLOW or KILL, but not DENY.

Otherwise this looks good to me.

I think all you need is to use the right constant name. :-)

#24: 2063405-access-checkers.patch queued for re-testing.

#24: 2063405-access-checkers.patch queued for re-testing.

I grepped the code base for all occurrences of the string "AccessCheckInterface", opened the files if they were actual access check classes, and confirmed that they are using static::(ALLOW|DENY|KILL) everywhere, without exception.

Combined with the fact that tests pass: RTBC.

Committed c5ef1c9 and pushed to 8.x. Thanks!