Support from Acquia helps fund testing for Drupal Acquia logo

Comments

InternetDevels’s picture

Here is the patch.

InternetDevels’s picture

Status: Active » Needs review

Status: Needs review » Needs work

The last submitted patch, drupal-shortcut_php_replace_user_access.patch, failed testing.

andypost’s picture

+++ b/core/modules/shortcut/shortcut.admin.inc
@@ -27,7 +27,7 @@
+  $user = Drupal::request()->attributes->get('_account');

+++ b/core/modules/shortcut/shortcut.module
@@ -11,7 +11,7 @@
+  $user = Drupal::request()->attributes->get('_account');

@@ -170,12 +170,13 @@ function shortcut_admin_paths() {
+  $account = Drupal::request()->attributes->get('_account');

@@ -194,14 +195,14 @@ function shortcut_set_edit_access($shortcut_set = NULL) {
+  $user = Drupal::request()->attributes->get('_account');;

Use Drupal::currentUser() service

rhm5000’s picture

Status: Needs work » Needs review
FileSize
1.74 KB
4.09 KB
andypost’s picture

Looks RTBC except one nitpick

+++ b/core/modules/shortcut/shortcut.admin.inc
@@ -27,7 +27,8 @@
 function shortcut_set_switch($form, &$form_state, $account = NULL) {
-  global $user;
+  $user = Drupal::currentUser();
+  ¶<------

just a trailing white-space

rhm5000’s picture

andypost’s picture

Status: Needs review » Reviewed & tested by the community

Thanx for quick re-roll, patch is right! Usage in shortcut_set_switch_access() is right, explains bellow

+++ b/core/modules/shortcut/shortcut.module
@@ -195,14 +196,14 @@ function shortcut_set_edit_access($shortcut_set = NULL) {
 function shortcut_set_switch_access($account = NULL) {
-  global $user;
+  $user = Drupal::currentUser();
...
-  if (user_access('administer shortcuts')) {
+  if ($user->hasPermission('administer shortcuts')) {
...
-  if (!user_access('switch shortcut sets')) {
+  if (!$user->hasPermission('switch shortcut sets')) {

That access check should happen against current user, the passed account here's for other purpose

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.x. Thanks!

tim.plunkett’s picture

Manual testing is good. This issue just "updated" a broken access checker, which was being fixed in #1978952: Convert shortcut_set_add to a Controller

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