Contrib.social

Node access control handler only respects 'administer nodes' on revision operatons

Open on Drupal.org →
Created on 20 September 2023, almost 2 years ago

Problem/Motivation

The administer nodes permission is pretty much useless and the "real" administration permission is bypass node access. This is pretty confusing.

The logic in \Drupal\node\NodeAccessControlHandler::checkAccess checks the administer nodes permission here:

    // Revision operations.
    if ($revision_permission_operation) {
....
      elseif ($account->hasPermission('administer nodes')) {
        return AccessResult::allowed()->cachePerPermissions();
      }

It also does not invoke the parent method, which has:

    if ($operation == 'delete' && $entity->isNew()) {
      return AccessResult::forbidden()->addCacheableDependency($entity);
    }
    if ($admin_permission = $this->entityType->getAdminPermission()) {
      return AccessResult::allowedIfHasPermission($account, $admin_permission);
    }

That means all logic is delegated to hook_entity_access and node_node_access performs per-bundle access checks.

Steps to reproduce

Assign administer nodes to a role and try to actually manage content.

Proposed resolution

Add admin_permission annotation to Nodes with administer nodes. Have \Drupal\node\NodeAccessControlHandler::checkAccess call its parent so that the admin permission is respected.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

🐛 Bug report
Status

Active

Version

11.0 🔥

Component
Node system  →

Last updated about 12 hours ago

No maintainer
Created by

🇺🇸United States mglaman WI, USA

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

  • Issue created by @mglaman
  • Comment almost 2 years ago →
    🇺🇸United States mglaman WI, USA

    The same is true for \Drupal\node\NodeAccessControlHandler::checkCreateAccess

    Parent:

      protected function checkCreateAccess(AccountInterface $account, array $context, $entity_bundle = NULL) {
    if ($admin_permission = $this->entityType->getAdminPermission()) {
      return AccessResult::allowedIfHasPermission($account, $admin_permission);
    }
    else {
      // No opinion.
      return AccessResult::neutral();
    }
  }

    Node:

      protected function checkCreateAccess(AccountInterface $account, array $context, $entity_bundle = NULL) {
    return AccessResult::allowedIf($account->hasPermission('create ' . $entity_bundle . ' content'))->cachePerPermissions();
  }

    It should call the parent as well.

  • Status changed to Postponed: needs info 4 months ago
  • Comment 4 months ago →
    🇦🇺Australia acbramley

    I don't think we could do this in a way that would preserve BC.

    Instead we can improve the description of the administer nodes permission in 🐛 Improve 'administer content' permission description Needs work

    I agree it's really not ideal, if we were to add an admin_permission I think it'd need to be a new permission.

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024