Problem/Motivation
The ANY
access mode makes it impossible to reuse access checkers in other contexts. The problem is that an access checker has no means to say I don't care, other checkers should decide in a way which works in ANY as well as in ALL mode. If you return allowed()
then you allow too much in ANY mode and if you return neutral()
than that evaluates to forbidden()
in ALL mode.
According to @dawehner in IRC:
dawehner: znerol: historically #2157541: Views sets access to ANY on routes - could result in information disclosure was one usecase why it got introduced
But that use case has been found to be a misconception.
Proposed resolution
Drop support for _access_mode
routes and always assume ALL.
Remaining tasks
User interface changes
API changes
Beta phase evaluation
Issue category | Bug, because the design of ANY is broken | Issue priority | Major, because it solves potential security problems |
---|---|
Disruption | Modules using the ANY access mode would have to write some custom access checkers, but ALL is certainly the common case anyway. |
Comment | File | Size | Author |
---|---|---|---|
#12 | interdiff.txt | 463 bytes | dawehner |
#12 | 2431281-12.patch | 21.38 KB | dawehner |
#8 | interdiff.txt | 1.66 KB | dawehner |
#8 | 2431281-8.patch | 21.4 KB | dawehner |
#2 | 2431281-2.patch | 20.41 KB | dawehner |
Comments
Comment #1
tstoeckler_access_mode
has always bothered me when I used it and when I have taught others Drupal 8 but I could never really pin-point why. After reading the issue summary, I now know why! :-) Thanks.I totally agree. So long as we stick to the
allowed()
/neutral()
/forbidden()
trifecta - which is not going to change at this point -_access_mode
is just conceptually wrong.Comment #2
dawehnerJust an experiment. I don't say that there aren't usecases for ANY, BUT
Comment #3
Fabianx CreditAttribution: Fabianx commented+1 to that! I don't know the code good enough to be able to sensible RTBC it, though.
Comment #4
catchYes I've only seen this cause security issues and not actually help anything.
Comment #5
Wim Leers+1
Comment #6
dawehnerBumping to major, as it is security hardening.
Added a beta evaluation.
Comment #7
tim.plunkettWhy keep the constants? I would have expected this patch to remove them.
Comment #8
dawehnerGood point tim!
Comment #9
klausiYep, this caused the security issue in #2420559: REST permissions are not working as expected., +1 on the removal.
Comment #10
Crell CreditAttribution: Crell commented*sniff* I'll miss you, _access_mode.
Comment #11
klausinow we have "requirements:" twice here?
Comment #12
dawehnerIt's not the first time when I think that the YAML parser is not strict enough.
Comment #13
Wim LeersComment #14
alexpottThis issue addresses a major bug and is allowed per https://www.drupal.org/core/beta-changes. Committed 9c2b19c and pushed to 8.0.x. Thanks!
Comment #16
jibranWe have a change notice which needs update or deletion
Comment #17
dawehnerUnpublished https://www.drupal.org/node/2107991 updated https://www.drupal.org/node/1851490