Issue #1872876 by chx, fubhy, dawehner, alexpott, tim.plunkett: Turn role...

Issue #1872876 by chx, fubhy, dawehner, alexpott, tim.plunkett: Turn role permission assignments into configuration.

core/includes/update.inc

@@ -1542,3 +1542,34 @@ function update_add_cache_columns($table) {
     ));
   }
 }
+
+/**
+ * Replace permissions during update.
+ *
+ * This function can replace one permission to several or even delete an old
+ * one.
+ *
+ * @param array $replace
+ *   An associative array. The keys are the old permissions the values are lists
+ *   of new permissions. If the list is an empty array, the old permission is
+ *   removed.
+ */
+function update_replace_permissions($replace) {
+  $prefix = 'user.role.';
+  $cut = strlen($prefix);
+  $role_names = Drupal::service('config.storage')->listAll($prefix);
+  foreach ($role_names as $role_name) {
+    $rid = substr($role_name, $cut);
+    $config = Drupal::config("user.role.$rid");
+    $permissions = $config->get('permissions') ?: array();
+    foreach ($replace as $old_permission => $new_permissions) {
+      if (($index = array_search($old_permission, $permissions)) !== FALSE) {
+        unset($permissions[$index]);
+        $permissions = array_unique(array_merge($permissions, $new_permissions));
+      }
+    }
+    $config
+      ->set('permissions', $permissions)
+      ->save();
+  }
+}

core/modules/block/lib/Drupal/block/Tests/BlockInterfaceTest.php

@@ -23,11 +23,6 @@ public static function getInfo() {
     );
   }
 
-  protected function setUp() {
-    parent::setUp();
-    $this->installSchema('user', 'role_permission');
-  }
-
   /**
    * Test configuration and subsequent form() and build() method calls.
    *

core/modules/comment/comment.module

@@ -1223,20 +1223,24 @@ function comment_node_update_index(EntityInterface $node, $langcode) {
   $index_comments = &drupal_static(__FUNCTION__);
 
   if ($index_comments === NULL) {
-    // Find and save roles that can 'access comments' or 'search content'.
-    $perms = array('access comments' => array(), 'search content' => array());
-    $result = db_query("SELECT rid, permission FROM {role_permission} WHERE permission IN ('access comments', 'search content')");
-    foreach ($result as $record) {
-      $perms[$record->permission][$record->rid] = $record->rid;
-    }
-
-    // Prevent indexing of comments if there are any roles that can search but
-    // not view comments.
+    // Do not index in the following three cases:
+    // 1. 'Authenticated user' can search content but can't access comments.
+    // 2. 'Anonymous user' can search content but can't access comments.
+    // 3. Any role can search content but can't access comments and access
+    // comments is not granted by the 'authenticated user' role. In this case
+    // all users might have both permissions from various roles but it is also
+    // possible to set up a user to have only search content and so a user
+    // edit could change the security situation so it is not safe to index the
+    // comments.
     $index_comments = TRUE;
-    foreach ($perms['search content'] as $rid) {
-      if (!isset($perms['access comments'][$rid]) && (($rid == DRUPAL_AUTHENTICATED_RID || $rid == DRUPAL_ANONYMOUS_RID) || !isset($perms['access comments'][DRUPAL_AUTHENTICATED_RID]))) {
-        $index_comments = FALSE;
-        break;
+    $roles = Drupal::entityManager()->getStorageController('user_role')->load();
+    $authenticated_can_access = $roles[DRUPAL_AUTHENTICATED_RID]->hasPermission('access comments');
+    foreach ($roles as $rid => $role) {
+      if ($role->hasPermission('search content') && !$role->hasPermission('access comments')) {
+        if ($rid == DRUPAL_AUTHENTICATED_RID || $rid == DRUPAL_ANONYMOUS_RID || !$authenticated_can_access) {
+          $index_comments = FALSE;
+          break;
+        }
       }
     }
   }

core/modules/field_ui/field_ui.install

@@ -9,39 +9,29 @@
  * Upgrade Field UI permissions.
  */
 function field_ui_update_8001() {
-
-  $permissions = array(
+  $replace = array(
     'administer comments' => array(
+      'administer comments',
       'administer comment fields',
       'administer comment display',
     ),
     'administer content types' => array(
+      'administer content types',
       'administer node fields',
       'administer node display',
     ),
     'administer users' => array(
+      'administer users',
       'administer user fields',
       'administer user display',
     ),
     'administer taxonomy' => array(
+      'administer taxonomy',
       'administer taxonomy_term fields',
       'administer taxonomy_term display',
     ),
   );
-
-  // We can not call user_permission_get_modules() as that will start
-  // invoking hooks which we can't during update hooks. Directly query
-  // for the permissions and insert them into the database.
-  foreach ($permissions as $old_permission => $new_permissions) {
-    $results = db_query("SELECT rid FROM {role_permission} WHERE permission = :permission", array(':permission' => $old_permission));
-    foreach ($results as $record) {
-      $query = db_insert('role_permission')->fields(array('rid', 'permission', 'module'));
-      foreach ($new_permissions as $new_permission) {
-        $query->values(array($record->rid, $new_permission, 'field_ui'));
-      }
-      $query->execute();
-    }
-  }
+  update_replace_permissions($replace);
 }
 
 /**

core/modules/filter/lib/Drupal/filter/Tests/FilterDefaultConfigTest.php

@@ -30,7 +30,7 @@ function setUp() {
     // filter_permission() calls into url() to output a link in the description.
     $this->installSchema('system', 'url_alias');
 
-    $this->installSchema('user', array('users_roles', 'role_permission'));
+    $this->installSchema('user', array('users_roles'));
 
     // Install filter_test module, which ships with custom default format.
     $this->installConfig(array('user', 'filter_test'));

core/modules/node/lib/Drupal/node/Tests/NodeAccessTest.php

@@ -26,9 +26,7 @@ public static function getInfo() {
   function setUp() {
     parent::setUp();
     // Clear permissions for authenticated users.
-    db_delete('role_permission')
-      ->condition('rid', DRUPAL_AUTHENTICATED_RID)
-      ->execute();
+    $this->container->get('config.factory')->get('user.role.' . DRUPAL_AUTHENTICATED_RID)->set('permissions', array())->save();
   }
 
   /**

core/modules/node/node.install

@@ -570,6 +570,15 @@ function _update_7000_node_get_types() {
  * @{
  */
 
+/**
+ * Implements hook_update_dependency().
+ */
+function node_update_dependency() {
+  $dependencies['node'][8013] = array(
+    'user' => 8002,
+  );
+}
+
 /**
  * Rename node type language variable names.
  *
@@ -796,15 +805,11 @@ function node_update_8012() {
  * Renames global revision permissions to use the word 'all'.
  */
 function node_update_8013() {
-  $actions = array('view', 'delete', 'revert');
-
-  foreach ($actions as $action) {
-    db_update('role_permission')
-      ->fields(array('permission' => $action . ' all revisions'))
-      ->condition('permission', $action . ' revisions')
-      ->condition('module', 'node')
-      ->execute();
-  }
+  update_replace_permissions(array(
+    'view revisions' => array('view all revisions'),
+    'revert revisions' => array('revert all revisions'),
+    'delete revisions' => array('delete all revisions'),
+  ));
 }
 
 /**

core/modules/node/node.views_execution.inc

@@ -30,17 +30,11 @@ function node_views_analyze(ViewExecutable $view) {
         // check for no access control
         $access = $display->getOption('access');
         if (empty($access['type']) || $access['type'] == 'none') {
-          $result = db_select('role_permission', 'p')
-            ->fields('p', array('rid', 'permission'))
-            ->condition('p.rid', array(DRUPAL_ANONYMOUS_RID, DRUPAL_AUTHENTICATED_RID), 'IN')
-            ->condition('p.permission', 'access content')
-            ->execute();
-
-          foreach ($result as $role) {
-            $role->safe = TRUE;
-            $roles[$role->rid] = $role;
-          }
-          if (!($roles[DRUPAL_ANONYMOUS_RID]->safe && $roles[DRUPAL_AUTHENTICATED_RID]->safe)) {
+          $anonymous_role = entity_load('user_role', DRUPAL_ANONYMOUS_RID);
+          $anonymous_has_access = $anonymous_role && $anonymous_role->hasPermission('access content');
+          $authenticated_role = entity_load('user_role', DRUPAL_AUTHENTICATED_RID);
+          $authenticated_has_access = $authenticated_role && $authenticated_role->hasPermission('access content');
+          if (!$anonymous_has_access || !$authenticated_has_access) {
             $ret[] = Analyzer::formatMessage(t('Some roles lack permission to access content, but display %display has no access control.', array('%display' => $display->display['display_title'])), 'warning');
           }
           $filters = $display->getOption('filters');

core/modules/simpletest/lib/Drupal/simpletest/WebTestBase.php

@@ -560,8 +560,7 @@ protected function drupalCreateRole(array $permissions, $rid = NULL, $name = NUL
       // Grant the specified permissions to the role, if any.
       if (!empty($permissions)) {
         user_role_grant_permissions($role->id(), $permissions);
-
-        $assigned_permissions = db_query('SELECT permission FROM {role_permission} WHERE rid = :rid', array(':rid' => $role->id()))->fetchCol();
+        $assigned_permissions = entity_load('user_role', $role->id())->permissions;
         $missing_permissions = array_diff($permissions, $assigned_permissions);
         if (!$missing_permissions) {
           $this->pass(t('Created permissions: @perms', array('@perms' => implode(', ', $permissions))), t('Role'));

core/modules/system/lib/Drupal/system/Tests/Entity/EntityAccessTest.php

@@ -29,7 +29,7 @@ public static function getInfo() {
 
   function setUp() {
     parent::setUp();
-    $this->installSchema('user', array('role_permission', 'users_roles'));
+    $this->installSchema('user', array('users_roles'));
     $this->installSchema('system', array('variable', 'url_alias'));
     $this->installSchema('language', 'language');
 

core/modules/system/lib/Drupal/system/Tests/Entity/EntityBCDecoratorTest.php

@@ -31,7 +31,7 @@ public static function getInfo() {
 
   public function setUp() {
     parent::setUp();
-    $this->installSchema('user', array('users_roles', 'users_data', 'role_permission'));
+    $this->installSchema('user', array('users_roles', 'users_data'));
     $this->installSchema('node', array('node', 'node_field_data', 'node_field_revision', 'node_type', 'node_access'));
     $this->installSchema('comment', array('comment', 'node_comment_statistics'));
   }

core/modules/system/lib/Drupal/system/Tests/Entity/EntityUnitTestBase.php

@@ -36,8 +36,7 @@ public function setUp() {
    *   (optional) The values used to create the entity.
    * @param array $permissions
    *   (optional) Array of permission names to assign to user. The
-   *   role_permission and users_roles tables must be installed before this can
-   *   be used.
+   *   users_roles tables must be installed before this can be used.
    *
    * @return \Drupal\user\Plugin\Core\Entity\User
    *   The created user entity.

core/modules/system/lib/Drupal/system/Tests/Module/UninstallTest.php

@@ -38,8 +38,7 @@ function testUserPermsUninstalled() {
     module_disable(array('module_test'));
     module_uninstall(array('module_test'));
 
-    // Are the perms defined by module_test removed from {role_permission}.
-    $count = db_query("SELECT COUNT(rid) FROM {role_permission} WHERE permission = :perm", array(':perm' => 'module_test perm'))->fetchField();
-    $this->assertEqual(0, $count, 'Permissions were all removed.');
+    // Are the perms defined by module_test removed?
+    $this->assertFalse(user_roles(FALSE, 'module_test perm'), 'Permissions were all removed.');
   }
 }

core/modules/system/lib/Drupal/system/Tests/Upgrade/FieldUIUpgradePathTest.php

@@ -65,11 +65,11 @@ function testFieldUIPermissions() {
 
     $role_permissions = user_role_permissions(array($this->normal_role_id, $this->admin_role_id));
     foreach ($permissions as $old_permission => $new_permissions) {
-      $this->assertFalse(isset($role_permissions[$this->normal_role_id][$old_permission]), format_string('%role_name does not have the old %permission permission', array('%role_name' => $this->normal_role_name, '%permission' => $old_permission)));
-      $this->assertTrue(isset($role_permissions[$this->admin_role_id][$old_permission]), format_string('%role_name still has the old %permission permission', array('%role_name' => $this->admin_role_name, '%permission' => $old_permission)));
+      $this->assertFalse(in_array($old_permission, $role_permissions[$this->normal_role_id]), format_string('%role_name does not have the old %permission permission', array('%role_name' => $this->normal_role_name, '%permission' => $old_permission)));
+      $this->assertTrue(in_array($old_permission, $role_permissions[$this->admin_role_id]), format_string('%role_name still has the old %permission permission', array('%role_name' => $this->admin_role_name, '%permission' => $old_permission)));
       foreach ($new_permissions as $new_permission) {
-        $this->assertFalse(isset($role_permissions[$this->normal_role_id][$new_permission]), format_string('%role_name does not have the new %permission permission', array('%role_name' => $this->normal_role_name, '%permission' => $new_permission)));
-        $this->assertTrue(isset($role_permissions[$this->admin_role_id][$new_permission]), format_string('%role_name has the new %permission permission', array('%role_name' => $this->admin_role_name, '%permission' => $new_permission)));
+        $this->assertFalse(in_array($new_permission, $role_permissions[$this->normal_role_id]), format_string('%role_name does not have the new %permission permission', array('%role_name' => $this->normal_role_name, '%permission' => $new_permission)));
+        $this->assertTrue(in_array($new_permission, $role_permissions[$this->admin_role_id]), format_string('%role_name has the new %permission permission', array('%role_name' => $this->admin_role_name, '%permission' => $new_permission)));
       }
     }
   }

core/modules/system/lib/Drupal/system/Tests/Upgrade/LanguageUpgradePathTest.php

@@ -191,6 +191,8 @@ public function testLanguageNoPluralsUpgrade() {
    * Tests upgrading translations permissions.
    */
   public function testLanguagePermissionsUpgrade() {
+    // Insert a permission into the Drupal 7 database before running the
+    // upgrade.
     db_insert('role_permission')->fields(array(
       'rid' => 2,
       'permission' => 'translate content',
@@ -198,12 +200,8 @@ public function testLanguagePermissionsUpgrade() {
     ))->execute();
 
     $this->assertTrue($this->performUpgrade(), 'The upgrade was completed successfully.');
-
-    // Check that translate content role doesn't exist on database.
-    $old_permission_exists = db_query('SELECT * FROM {role_permission} WHERE permission LIKE ?', array('translate content'))->fetchObject();
-    $this->assertFalse($old_permission_exists, 'No translate content role left on database.');
+    $this->assertFalse(user_roles(FALSE, 'translate content'), 'No translate content role left in config.');
     // Check that translate content has been renamed to translate all content.
-    $new_permission_exists = db_query('SELECT * FROM {role_permission} WHERE permission LIKE ?', array('translate all content'))->fetchObject();
-    $this->assertTrue($new_permission_exists, 'Rename role translate content to translate all content was completed successfully.');
+    $this->assertTrue(user_roles(FALSE, 'translate all content'), 'Rename role translate content to translate all content was completed successfully.');
   }
 }

core/modules/system/system.install

@@ -1520,13 +1520,9 @@ function system_update_8020() {
   $blocked_ips_exists = db_query_range('SELECT 1 FROM {blocked_ips}', 0, 1)->fetchField();
   if ($blocked_ips_exists) {
     // Rename the permission name.
-    db_update('role_permission')
-      ->fields(array(
-        'permission' => 'ban IP addresses',
-        'module' => 'ban',
-      ))
-      ->condition('permission', 'block IP addresses')
-      ->execute();
+    update_replace_permissions(array(
+      'block IP addresses' => array('ban IP addresses'),
+    ));
     // Rename {blocked_ips} table into {ban_ip}.
     db_rename_table('blocked_ips', 'ban_ip');
     // Remove all references to the removed action callback.

core/modules/text/lib/Drupal/text/Tests/TextSummaryTest.php

@@ -28,7 +28,6 @@ function setUp() {
     parent::setUp();
 
     $this->installSchema('system', 'url_alias');
-    $this->installSchema('user', 'role_permission');
     $this->installConfig(array('text'));
   }
 

core/modules/translation/translation.install

@@ -10,10 +10,9 @@
  * Rename the translate content permission.
  */
 function translation_update_8000() {
-  db_update('role_permission')
-    ->fields(array('permission' => 'translate all content'))
-    ->condition('permission', 'translate content')
-    ->execute();
+  update_replace_permissions(array(
+    'translate content' => array('translate all content'),
+  ));
 }
 
 /**

core/modules/user/config/schema/user.schema.yml

@@ -131,6 +131,12 @@ user.role.*:
     weight:
       type: integer
       label: 'User role weight'
+    permissions:
+      type: sequence
+      label: 'Permissions'
+      sequence:
+        - type: string
+          label: 'Permission'
     langcode:
       type: string
       label: 'Default language'

core/modules/user/config/views.view.user_admin_people.yml

@@ -742,7 +742,7 @@ display:
           plugin_id: user_roles
         permission:
           id: permission
-          table: role_permission
+          table: users_roles
           field: permission
           relationship: none
           group_type: group

core/modules/user/lib/Drupal/user/Plugin/Core/Entity/Role.php

@@ -67,6 +67,13 @@ class Role extends ConfigEntityBase implements RoleInterface {
    */
   public $weight;
 
+  /**
+   * The permissions belonging to this role.
+   *
+   * @var array
+   */
+  public $permissions = array();
+
   /**
    * {@inheritdoc}
    */
@@ -80,6 +87,38 @@ public function uri() {
     );
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  public function getPermissions() {
+    return $this->permissions;
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function hasPermission($permission) {
+    return in_array($permission, $this->permissions);
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function grantPermission($permission) {
+    if (!$this->hasPermission($permission)) {
+      $this->permissions[] = $permission;
+    }
+    return $this;
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function revokePermission($permission) {
+    $this->permissions = array_diff($this->permissions, array($permission));
+    return $this;
+  }
+
   /**
    * {@inheritdoc}
    */

core/modules/user/lib/Drupal/user/Plugin/views/field/Permissions.php

@@ -8,7 +8,8 @@
 namespace Drupal\user\Plugin\views\field;
 
 use Drupal\Component\Annotation\PluginID;
-use Drupal\Core\Database\Connection;
+use Drupal\Core\Entity\EntityManager;
+use Drupal\Core\Extension\ModuleHandlerInterface;
 use Drupal\views\Plugin\views\display\DisplayPluginBase;
 use Drupal\views\ViewExecutable;
 use Drupal\views\Plugin\views\field\PrerenderList;
@@ -24,11 +25,18 @@
 class Permissions extends PrerenderList {
 
   /**
-   * Database Service Object.
+   * The role storage controller.
    *
-   * @var \Drupal\Core\Database\Connection
+   * @var \Drupal\user\RoleStorageControllerInterface
    */
-  protected $database;
+  protected $roleStorageController;
+
+  /**
+   * The module handler.
+   *
+   * @var \Drupal\Core\Extension\ModuleHandlerInterface
+   */
+  protected $moduleHandler;
 
   /**
    * Constructs a Drupal\Component\Plugin\PluginBase object.
@@ -39,20 +47,23 @@ class Permissions extends PrerenderList {
    *   The plugin_id for the plugin instance.
    * @param array $plugin_definition
    *   The plugin implementation definition.
-   * @param \Drupal\Core\Database\Connection $database
-   *   Database Service Object.
+   * @param \Drupal\Core\Extension\ModuleHandlerInterface $module_handler
+   *   The module handler.
+   * @param \Drupal\Core\Entity\EntityManager $entity_manager
+   *   The entity manager
    */
-  public function __construct(array $configuration, $plugin_id, array $plugin_definition, Connection $database) {
+  public function __construct(array $configuration, $plugin_id, array $plugin_definition, ModuleHandlerInterface $module_handler, EntityManager $entity_manager) {
     parent::__construct($configuration, $plugin_id, $plugin_definition);
 
-    $this->database = $database;
+    $this->roleStorageController = $entity_manager->getStorageController('user_role');
+    $this->moduleHandler = $module_handler;
   }
 
   /**
    * {@inheritdoc}
    */
   public static function create(ContainerInterface $container, array $configuration, $plugin_id, array $plugin_definition) {
-    return new static($configuration, $plugin_id, $plugin_definition, $container->get('database'));
+    return new static($configuration, $plugin_id, $plugin_definition, $container->get('module_handler'), $container->get('plugin.manager.entity'));
   }
 
   /**
@@ -73,27 +84,32 @@ public function preRender(&$values) {
     $uids = array();
     $this->items = array();
 
+    $permission_names = \Drupal::moduleHandler()->invokeAll('permission');
+
+    $rids = array();
     foreach ($values as $result) {
-      $uids[] = $this->getValue($result);
-    }
+      $user_rids = $this->getEntity($result)->getRoles();
+      $uid = $this->getValue($result);
 
-    if ($uids) {
-      // Get a list of all the modules implementing a hook_permission() and sort by
-      // display name.
-      $module_info = system_get_info('module');
-      $modules = array();
-      foreach (module_implements('permission') as $module) {
-        $modules[$module] = $module_info[$module]['name'];
+      foreach ($user_rids as $rid) {
+        $rids[$rid][] = $uid;
       }
-      asort($modules);
-
-      $permissions = module_invoke_all('permission');
+    }
 
-      $result = $this->database->query('SELECT u.uid, u.rid, rp.permission FROM {role_permission} rp INNER JOIN {users_roles} u ON u.rid = rp.rid WHERE u.uid IN (:uids) AND rp.module IN (:modules) ORDER BY rp.permission',
-        array(':uids' => $uids, ':modules' => array_keys($modules)));
+    if ($rids) {
+      $roles = $this->roleStorageController->load(array_keys($rids));
+      foreach ($rids as $rid => $role_uids) {
+        foreach ($roles[$rid]->getPermissions() as $permission) {
+          foreach ($role_uids as $uid) {
+            $this->items[$uid][$permission]['permission'] = $permission_names[$permission]['title'];
+          }
+        }
+      }
 
-      foreach ($result as $perm) {
-        $this->items[$perm->uid][$perm->permission]['permission'] = $permissions[$perm->permission]['title'];
+      foreach ($uids as $uid) {
+        if (isset($this->items[$uid])) {
+          ksort($this->items[$uid]);
+        }
       }
     }
   }

core/modules/user/lib/Drupal/user/Plugin/views/filter/Permissions.php

@@ -8,7 +8,10 @@
 namespace Drupal\user\Plugin\views\filter;
 
 use Drupal\Component\Annotation\PluginID;
+use Drupal\Component\Utility\String;
+use Drupal\Core\Extension\ModuleHandlerInterface;
 use Drupal\views\Plugin\views\filter\ManyToOne;
+use Symfony\Component\DependencyInjection\ContainerInterface;
 
 /**
  * Filter handler for user roles.
@@ -19,25 +22,83 @@
  */
 class Permissions extends ManyToOne {
 
+  /**
+   * The module handler.
+   *
+   * @var \Drupal\Core\Extension\ModuleHandlerInterface
+   */
+  protected $moduleHandler;
+
+  /**
+   * Constructs a Permissions object.
+   *
+   * @param array $configuration
+   *   A configuration array containing information about the plugin instance.
+   * @param string $plugin_id
+   *   The plugin_id for the plugin instance.
+   * @param array $plugin_definition
+   *   The plugin implementation definition.
+   * @param \Drupal\Core\Extension\ModuleHandlerInterface $module_handler
+   *   The module handler.
+   */
+  public function __construct(array $configuration, $plugin_id, array $plugin_definition, ModuleHandlerInterface $module_handler) {