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

Comments

undertext’s picture

undertext commented
Assigned: Unassigned » undertext
undertext’s picture

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

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

Great!

m1r1k’s picture

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

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 commented
Status: Needs work » Needs review
StatusFileSize
new put_access_manager_onto_drupal-2321809-6.patch520 bytes

Yeap. You are right.

herom’s picture

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

That was it.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
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
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
commented
Status: Needs review » Reviewed & tested by the community

Given my previous response ...

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
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 🇧🇪
commented
Status: Needs work » Needs review
Issue tags: -
StatusFileSize
new 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 🇧🇪
commented
Issue tags: +DUGBE1409
jeroent’s picture

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

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

Good point, alex

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
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.