Replace user_access() calls with $account->hasPermission() in all core modules except user

1. +++ b/core/modules/contact/contact.pages.inc
   @@ -24,7 +24,8 @@
   +  $user = Drupal::request()->attributes->get('_account');
   

   Use Drupal::currentUser() service https://drupal.org/node/2032447

2. +++ b/core/modules/contact/contact.pages.inc
   @@ -24,7 +24,8 @@
   +  if (!$user->hasPermission('administer contact forms')) {
   
   @@ -36,7 +37,7 @@ function contact_site_page(Category $category = NULL) {
   +      if ($user->hasPermission('administer contact forms')) {
   
   @@ -71,7 +72,7 @@ function contact_personal_page($recipient) {
      global $user;
   ...
   +  if (!$user->hasPermission('administer contact forms') && !$user->hasPermission('administer users')) {
   

   This will need re-roll after #1938390: Convert contact_site_page and contact_person_page to a new-style Controller

This is the only occurrence of user_access left in the contact module.

Yep, the one one left

Thanks for your work on this!

See #2048171-17: [meta] Replace user_access() calls with $account->hasPermission() wherever possible.. Expanding the scope of this issue; please update this patch to include fixes to all modules except user.module and then confirm that no instances remain in core/modules that are not in core/modules/user before RTBCing.

Oh, and please help make sure everyone who worked on the duplicates for other core modules gets credit for this patch! Put a commit message in the summary and final RTBC comment for core maintainers.

Hi Jess,

There are only 3 other issues left for this (including user module issue). 23 have already been committed.

• #2061977: Replace user_access() calls with $account->hasPermission() in all core modules except user
• #2062039: Replace user_access() calls with $account->hasPermission() in user module
• #2062043: Replace user_access() calls with $account->hasPermission() in core files

I'm not sure how much efficiency we'll get out of combining them?

Kim

@kim.pepper, many other issues were left, but the remaining ones were closed as duplicate, and only 3 were left to merge all the others.
for this issue, I think these should be merged:
#2061961: Replace user_access() calls with $account->hasPermission() in content translation module
#2062041: Replace user_access() calls with $account->hasPermission() in views module
#2062037: Replace user_access() calls with $account->hasPermission() in update module
#2061993: Replace user_access() calls with $account->hasPermission() in file module
#2061995: Replace user_access() calls with $account->hasPermission() in filter module
#2061975: Replace user_access() calls with $account->hasPermission() in comment module
#2061989: Replace user_access() calls with $account->hasPermission() in field_ui module
#2062007: Replace user_access() calls with $account->hasPermission() in node module
#2062029: Replace user_access() calls with $account->hasPermission() in toolbar module
#2062025: Replace user_access() calls with $account->hasPermission() in system module
so, there were a lot of issues; but a lot of tiny issues that each one had to go through review-rtbc-reroll-patch, review-rtbc-reroll-patch, ... over and over again. makes sense to do this all once.

OK. Here's a combined patch of all those listed above.

Compiled list of people involved from all duplicate issues #8:

InternetDevels, rhm50
naveenvalecha, InternetDevels
rhm50, InternetDevels, andypost
InternetDevels, mandar.harkare, sergeypavlenko
sidharthap, InternetDevels
andypost, InternetDevels, kim.pepper, naveenvalecha
InternetDevels, SIz, sidharthap
tkuldeep17, herom, sidharthap, InternetDevels
rhm50, InternetDevels
herom, InternetDevels

Issue #2061977 by kim.pepper, InternetDevels, rhm50, naveenvalecha, andypost, mandar.harkare, sergeypavlenko, sidharthap, herom, SIz, tkuldeep17: Replace user_access() calls with $account->hasPermission() in all core modules except user.

added to summary

+++ b/core/authorize.php
@@ -58,7 +58,7 @@ function authorize_access_denied_page() {
+  return settings()->get('allow_authorize_operations', TRUE) && Drupal::currentUser()->hasPermission('administer software updates');

+++ b/core/includes/bootstrap.inc
@@ -2860,13 +2860,13 @@ function drupal_classloader_register($name, $path) {
+ * function datetime_default_format_type() {

+++ b/core/includes/menu.inc
@@ -599,10 +599,10 @@ function _menu_check_access(&$item, $map) {
+    // As call_user_func_array is quite slow and user_access is a very common ¶

+++ b/core/lib/Drupal/Core/Extension/UpdateModuleHandler.php
@@ -46,7 +46,9 @@ public function getImplementations($hook) {
+      // Those are needed by

This changes should be fixed in #2062043: Replace user_access() calls with $account->hasPermission() in core files

Re-roll plus remove core includes as per #11

RTBC if green

not green...

and a couple of things I noticed, that should be fixed with the next patch:

1. +++ b/core/modules/views/views.api.php
   @@ -377,7 +377,9 @@ function hook_views_form_substitutions() {
   +  $account = Drupal::currentUser();
   +
   +  if ($view->name == 'my_special_view' && $account->hasPermission('my special permission') && $display_id == 'public_display') {
   
   @@ -444,7 +446,9 @@ function hook_views_pre_execute(ViewExecutable $view) {
   +  $account = Drupal::currentUser();
   +
   +  if (count($view->query->tables) > 2 && $account->hasPermission('administer views')) {
   
   +++ b/core/modules/views/views.module
   @@ -332,7 +332,8 @@ function views_page_alter(&$page) {
   +  $account = Drupal::currentUser();
   +  if (!$account || !$account->hasPermission('access contextual links')) {
   

   Drupal:: called without "\" prefix. And no need for local variable here, you can call \Drupal::currentUser()->hasPermission() directly.
   Also, !$account || is unnecessary, since that would always be false here.

2. +++ b/core/update.php
   @@ -70,7 +70,7 @@ function update_helpful_links() {
   -  if (user_access('access administration pages')) {
   +  if (Drupal::currentUser()->hasPermission('access administration pages')) {
   

   this also belongs to the core files issue.

fixing the tests, points in #15, and a few extra $account params.

oops.

18: 2061977-user-access-18.patch queued for re-testing.

reroll.

21: 2061977-user-access-21.patch queued for re-testing.

the fails don't seem relevant, and are different than previous test.
doing another retest, to see what comes up.

21: 2061977-user-access-21.patch queued for re-testing.

Reroll, plus replacing occurrences that have slipped in this patch. Interdiff shows the new occurrences I replaced after completing my reroll.

rerolled + a one line fix.

re-uploading the patch, since it didn't show up in the issue summary Files table.

I think it is fine to not do injection for now.

+++ b/core/modules/node/node.api.php
@@ -562,23 +562,23 @@ function hook_node_load($nodes) {
-    if ($op == 'create' && user_access('create ' . $type . ' content', $account)) {
+    if ($op == 'create' && $account->hasPermission('create ' . $type . ' content')) {

The new way for this example is actually much nicer code!

Committed and pushed to 8.x. Thanks!

Sorry, that last bit was to fix commit credit. I missed the nicely formatted commit message in the issue summary because I was trying to commit patches and talk on the phone at the same time. :P Thanks to herom for the alert!