Problem/Motivation

Make it easy to check access, therefore \Drupal should make it easy to find the access manager.

Proposed resolution

Put it onto the \Drupal class

Remaining tasks

User interface changes

API changes

CommentFileSizeAuthor
#12 put_the_access_manager-2321809-12.patch1.12 KBJeroenT
#6 put_access_manager_onto_drupal-2321809-6.patch520 bytesundertext
#2 put_access_manager_onto_drupal-2321809-1.patch511 bytesundertext
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

undertext’s picture

undertext CreditAttribution: undertext commented
Assigned: Unassigned » undertext
undertext’s picture

undertext CreditAttribution: undertext commented
Status: Active » Needs review
FileSize
put_access_manager_onto_drupal-2321809-1.patch511 bytes
dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Needs review » Reviewed & tested by the community

Great!

m1r1k’s picture

m1r1k CreditAttribution: m1r1k commented
Issue tags: +#ams2014contest
herom’s picture

herom CreditAttribution: herom commented
Status: Reviewed & tested by the community » Needs work 
+++ b/core/lib/Drupal.php
@@ -657,4 +657,14 @@ public static function menuTree() {
+   * @return \Drupal\Core\Access\AccessManager

We should probably typehint on "\Drupal\Core\Access\AccessManagerInterface"

undertext’s picture

undertext CreditAttribution: undertext commented
Status: Needs work » Needs review
FileSize
put_access_manager_onto_drupal-2321809-6.patch520 bytes

Yeap. You are right.

herom’s picture

herom CreditAttribution: herom commented
Status: Needs review » Reviewed & tested by the community

That was it.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Needs review

This feels related to #2322809: Do not depend on event subscribers for security: Tighten routing security by access checking in matchRequest - if the access is checked by the router perhaps we don't need to do this?

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented

@alexpott
Well, access is already checked during every request, with a quite small tiny possible time, when access is not checked. The access manager though
is used in more places than just routing of the current request. For example when you generate things like a link, having the access manager available
would be also a big gain. Making access checking easy should certainly be done.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Needs review » Reviewed & tested by the community

Given my previous response ...

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Needs work

We're missing a test in \Drupal\Tests\Core\DrupalTest

JeroenT’s picture

JeroenT
Primary language Dutch
Location 🇧🇪
CreditAttribution: JeroenT commented
Status: Needs work » Needs review
Issue tags: -
FileSize
put_the_access_manager-2321809-12.patch1.12 KB

Added a test for the accessmanager method.

Patch attached.

JeroenT’s picture

JeroenT
Primary language Dutch
Location 🇧🇪
CreditAttribution: JeroenT commented
Issue tags: +DUGBE1409
JeroenT’s picture

JeroenT
Primary language Dutch
Location 🇧🇪
CreditAttribution: JeroenT commented
Assigned: undertext » Unassigned
dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Needs review » Reviewed & tested by the community

Good point, alex

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Fixed

Committed e3f8efb and pushed to 8.0.x. Thanks!

  • alexpott committed e3f8efb on 8.0.x 
    Issue #2321809 by undertext, JeroenT | dawehner: Put the access manager...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.