Views sets access to ANY on routes - could result in information disclosure

Let's document the usecase: Views wants to provide a permission "access all views". You need to have an logical ANY between them, though this is problematic in case you have more than 2 other access checks.

To be clear: without this line, \Drupal\views\ViewsAccessCheck will not work.
But that took me a couple minutes to figure out *why* we needed it, and how it works.

Might this be better? It is less obfuscated, and the 'ANY' doesn't raise red flags the same way when we're explicitly setting another requirement right before it.

I tested this manually, this should work...

Advice: I'm not sure what I am doing, so please review carefully.

Tim patch added a requirement, and the failing test was asserting on the number of requirements.
I increased the expected count (reordered assertEqual arguments), and added more explicit checks for those.

+1 on adding test coverage here but yeah we also need some sort of proper integration test in case we know what we want to do.

5: views-access-any-2157541-5.patch queued for re-testing.

One alternative approach could be to maybe ran another subscriber on the routing layer which runs somehow before
the actual access checking and allows to skip the other one.

Just made a small research, symfony for example has several so called voters, http://symfony.com/doc/current/cookbook/security/voters.html#the-voter-i...

Note: Once #1986640: Support AND/OR conjunctions for permission checks is in we can probably solve this issue properly.

I don't believe @dawehner in #12 though.

Try to reroll the last patch.

#14 still applies.

RTBC - looks great to me and tests make sense.

Thank you for working on this issue, but are we aware of the fact that we haven't solved the critical bug yet?

Originally and with the patch the problem stays the same:

• You start with a route having the following configuration:
  

  route_name:
    options:
      _access_mode: 'ALL'
    requirements:
      _permission: 'example permission'
      _additional_thingy: 'blub'
  

• Views comes in and modifies the route definition in the following way (even with this patch):
  

  route_name:
    options:
      _access_mode: 'ANY' // note this line
    requirements:
      _permission: 'example permission'
      _additional_thingy: 'blub'
      _views_access_all_views: 'TRUE'
  

• As you see we now allow access if just one of the three things return TRUE and there we have our comparable small security problem.

@Fabianx
One thing security related code should always have is a total integration test, so you are kind of sure that things don't break

In this case: CNW and we need an issue summary update as that is totally not clear from the summary.

Updated the IS

I really wonder whether we need this feature in the first place.

• Most views will be permission based, ... so you can configure it, if needed
• For UID=1, which most people use during development (let's be honest here), it doesn't matter for permissions at all.
  For all other ones, like based upon roles, you can also configure it somehow.

So the question is whether we can lieve with "regression" ...

@catch was fine with killing it, so:
Let's kill it!

I used this feature like once ever. I'm okay with removing it. But does this resolve the underlying "ANY" problem?

Will need a CR if we remove the feature.

I think we'd need to kill

    $route->setOption('_access_mode', AccessManagerInterface::ACCESS_MODE_ANY);

from PathPluginBase, right?

Absolutely!

Here's a draft change record: https://www.drupal.org/node/2381665

Much much cleaner, love it.

RTBC from my side, Change record looks great.

Looks good to me too.

Nice diffstat :)

This issue addresses a critical bug and is allowed per https://www.drupal.org/core/beta-changes. Committed ed01f40 and pushed to 8.0.x. Thanks!