Remove user_access function calls on hook_permission functions

I'll work on a patch to solve this issue

I removed the calls to user_access on the file.module and the node.module files, sending patch for testing and discussion

for consistency, changed '@url' to '!url'
Please review now.

_

url() has been replaced with \Drupal:url() as per D8 standard.

As there is no routing info available for admin/content/files so that has not been updated in file.module.

status change

I looked at this patch and do not readily see any security implications despite the very scary title :)

Fixing the inconsistency in this direction is fixing a bug that doesn't exist ("uncacheability" of something that's never cached, and probably doesn't have much reason to ever want to cache?)... and in exchange it introduces an actual user-facing bug: more 403 errors for users following links in the admin interface.

Fixing it the other way removes the existing 403 errors, in exchange for some code complexity.

But it's rare for an admin to be able to see the permission page and not see these other pages, and also there are probably plenty of other similar 403 errors elsewhere in the admin interface (links in help text?), so just saying we're going to consistently show them the link in these situations regardless of whether they can access it is probably not so terrible.

Also an admin knowing that there is functionality they don't have permission to access is useful information.

Committed 3e34d84 and pushed to 8.x. Thanks!

As discussed in #1684930: Add description to "administer filters" permission this could perhaps be backported (or at least it could be discussed)? It wouldn't break any translations, since it would just remove some translatable strings.

I'm not sure Drupal 7 actually has an inconsistency currently though (everything I saw, at least on a standard install, does check the permissions). Also, it just occurred to me that to the extent that contrib modules are following core's pattern, we might not want to change it out from underneath them...

@David_Rothstein, I have removed user_access() from node_permission function in node.module but in file.module there is no hook_permission found.

Please review attached patch for D7.

Sorry forget to attach patch. Here is patch.

That looks right to me.. but I know there's (at least) one more place in the D7 codebase where user_access() is used in hook_permission(): the Filter module (since that's what kicked off this issue in the first place).

updated the patch with user_access() remove from hook_permission() in filter module.

Patch review:

• Applies cleanly against 7.x
• Created a 'test' user with only 'Administer permissions' perm.
• Created a 'test' text format with no roles having access granted to it.
• Same links are visible at permissions page both for uid 1 and 'test' user.
• Checked with user 1 that all pages linked there are accessible.
• Checked with 'test' user that 'Access denied' is returned when following the same links, as expected.

Patch is OK.

Notice: linked below there is a postponed issue awaiting to completion of this one.
#1684930: Add description to "administer filters" permission

Testbot fluke.

OK, it sounds like this is the direction people want to go in, so let's do this.

I enabled all core modules and looked at the Permissions page and didn't see any others that had links with a permission check on them, so this looks good.

Committed to 7.x - thanks!

Made this fix on commit, since the code comment was no longer correct:

--- a/modules/filter/filter.module
+++ b/modules/filter/filter.module
@@ -348,8 +348,6 @@ function filter_permission() {
   foreach (filter_formats() as $format) {
     $permission = filter_permission_name($format);
     if (!empty($permission)) {
-      // Only link to the text format configuration page if the user who is
-      // viewing this will have access to that page.
       $format_name_replacement = l($format->name, 'admin/config/content/formats/' . $format->format);
       $perms[$permission] = array(
         'title' => t("Use the !text_format text format", array('!text_format' => $format_name_replacement,)),