Issue #2435075 by dawehner: Implement admin role as a flag on the role storage, simplify permissions page, remove user_modules_installed

core/modules/filter/src/FilterFormatFormBase.php

@@ -90,12 +90,6 @@ abstract class FilterFormatFormBase extends EntityForm {
       // If editing an existing text format, pre-select its current permissions.
       $form['roles']['#default_value'] = array_keys(filter_get_roles_by_format($format));
     }
-    elseif ($admin_role = $this->config('user.settings')->get('admin_role')) {
-      // If adding a new text format and the site has an administrative role,
-      // pre-select that role so as to grant administrators access to the new
-      // text format permission by default.
-      $form['roles']['#default_value'] = array($admin_role);
-    }
 
     // Create filter plugin instances for all available filters, including both
     // enabled/configured ones as well as new and not yet unconfigured ones.

core/modules/shortcut/shortcut.api.php

@@ -32,7 +32,9 @@
  */
 function hook_shortcut_default_set($account) {
   // Use a special set of default shortcuts for administrators only.
-  if (in_array(\Drupal::config('user.settings')->get('admin_role'), $account->getRoles())) {
+  $roles = \Drupal::entityManager()->getStorage('user_role')->loadByProperties(['is_admin' => TRUE]);
+  $user_admin_roles = array_intersect(array_keys($roles), $account->getRoles());
+  if ($user_admin_roles) {
     return 'admin-shortcuts';
   }
 }

core/modules/user/config/install/user.role.anonymous.yml

@@ -3,4 +3,5 @@ label: 'Anonymous user'
 weight: 0
 langcode: en
 status: true
+is_admin: false
 dependencies: {  }

core/modules/user/config/install/user.role.authenticated.yml

@@ -3,4 +3,5 @@ label: 'Authenticated user'
 weight: 1
 langcode: en
 status: true
+is_admin: false
 dependencies: {  }

core/modules/user/config/install/user.settings.yml

@@ -1,4 +1,3 @@
-admin_role: ''
 anonymous: Anonymous
 verify_mail: true
 notify:

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

@@ -4,9 +4,6 @@ user.settings:
   type: mapping
   label: 'User settings'
   mapping:
-    admin_role:
-      type: string
-      label: 'Administrator role'
     anonymous:
       type: label
       label: 'Name'
@@ -128,6 +125,9 @@ user.role.*:
     weight:
       type: integer
       label: 'User role weight'
+    is_admin:
+      type: boolean
+      label: 'User is admin'
     permissions:
       type: sequence
       label: 'Permissions'

core/modules/user/src/AccountSettingsForm.php

@@ -26,6 +26,13 @@ class AccountSettingsForm extends ConfigFormBase {
    */
   protected $moduleHandler;
 
+  /**
+   * The role storage used when changing the admin role.
+   *
+   * @var \Drupal\user\RoleStorageInterface
+   */
+  protected $roleStorage;
+
   /**
    * Constructs a \Drupal\user\AccountSettingsForm object.
    *
@@ -33,10 +40,13 @@ class AccountSettingsForm extends ConfigFormBase {
    *   The factory for configuration objects.
    * @param \Drupal\Core\Extension\ModuleHandler $module_handler
    *   The module handler.
+   * @param \Drupal\user\RoleStorageInterface $role_storage
+   *   The role storage.
    */
-  public function __construct(ConfigFactoryInterface $config_factory, ModuleHandler $module_handler) {
+  public function __construct(ConfigFactoryInterface $config_factory, ModuleHandler $module_handler, RoleStorageInterface $role_storage) {
     parent::__construct($config_factory);
     $this->moduleHandler = $module_handler;
+    $this->roleStorage = $role_storage;
   }
 
   /**
@@ -45,7 +55,8 @@ class AccountSettingsForm extends ConfigFormBase {
   public static function create(ContainerInterface $container) {
     return new static(
       $container->get('config.factory'),
-      $container->get('module_handler')
+      $container->get('module_handler'),
+      $container->get('entity.manager')->getStorage('user_role')
     );
   }
 
@@ -102,13 +113,22 @@ class AccountSettingsForm extends ConfigFormBase {
     // administrator role.
     $roles = user_role_names(TRUE);
     unset($roles[DRUPAL_AUTHENTICATED_RID]);
+
+    $admin_roles = $this->roleStorage->getQuery()
+      ->condition('is_admin', TRUE)
+      ->execute();
+    $default_value = reset($admin_roles);
+
     $form['admin_role']['user_admin_role'] = array(
       '#type' => 'select',
       '#title' => $this->t('Administrator role'),
       '#empty_value' => '',
-      '#default_value' => $config->get('admin_role'),
+      '#default_value' => $default_value,
       '#options' => $roles,
       '#description' => $this->t('This role will be automatically assigned new permissions whenever a module is enabled. Changing this setting will not affect existing permissions.'),
+      // Don't allow to select a single admin role in case multiple roles got
+      // marked as admin role already.
+      '#access' => count($admin_roles) <= 1,
     );
 
     // @todo Remove this check once language settings are generalized.
@@ -427,7 +447,6 @@ class AccountSettingsForm extends ConfigFormBase {
 
     $this->config('user.settings')
       ->set('anonymous', $form_state->getValue('anonymous'))
-      ->set('admin_role', $form_state->getValue('user_admin_role'))
       ->set('register', $form_state->getValue('user_register'))
       ->set('password_strength', $form_state->getValue('user_password_strength'))
       ->set('verify_mail', $form_state->getValue('user_email_verification'))
@@ -459,6 +478,22 @@ class AccountSettingsForm extends ConfigFormBase {
       ->set('mail_notification', $form_state->getValue('mail_notification_address'))
       ->save();
 
+    // Change the admin role.
+    if ($form_state->hasValue('user_admin_role')) {
+      $admin_roles = $this->roleStorage->getQuery()
+        ->condition('is_admin', TRUE)
+        ->execute();
+
+      foreach ($admin_roles as $rid) {
+        $this->roleStorage->load($rid)->setIsAdmin(FALSE)->save();
+      }
+
+      $new_admin_role = $form_state->getValue('user_admin_role');
+      if ($new_admin_role) {
+        $this->roleStorage->load($new_admin_role)->setIsAdmin(TRUE)->save();
+      }
+    }
+
     // Clear field definition cache for signatures.
     \Drupal::entityManager()->clearCachedFieldDefinitions();
   }

core/modules/user/src/Entity/Role.php

@@ -73,10 +73,20 @@ class Role extends ConfigEntityBase implements RoleInterface {
    */
   protected $permissions = array();
 
+  /**
+   * An indicator whether the role has all permissions.
+   *
+   * @var bool
+   */
+  protected $is_admin;
+
   /**
    * {@inheritdoc}
    */
   public function getPermissions() {
+    if ($this->isAdmin()) {
+      return [];
+    }
     return $this->permissions;
   }
 
@@ -99,6 +109,9 @@ class Role extends ConfigEntityBase implements RoleInterface {
    * {@inheritdoc}
    */
   public function hasPermission($permission) {
+    if ($this->isAdmin()) {
+      return TRUE;
+    }
     return in_array($permission, $this->permissions);
   }
 
@@ -106,6 +119,9 @@ class Role extends ConfigEntityBase implements RoleInterface {
    * {@inheritdoc}
    */
   public function grantPermission($permission) {
+    if ($this->isAdmin()) {
+      return $this;
+    }
     if (!$this->hasPermission($permission)) {
       $this->permissions[] = $permission;
     }
@@ -116,10 +132,28 @@ class Role extends ConfigEntityBase implements RoleInterface {
    * {@inheritdoc}
    */
   public function revokePermission($permission) {
+    if ($this->isAdmin()) {
+      return $this;
+    }
     $this->permissions = array_diff($this->permissions, array($permission));
     return $this;
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  public function isAdmin() {
+    return (bool) $this->is_admin;
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function setIsAdmin($is_admin) {
+    $this->is_admin = $is_admin;
+    return $this;
+  }
+
   /**
    * {@inheritdoc}
    */

core/modules/user/src/Form/UserPermissionsForm.php

@@ -79,11 +79,13 @@ class UserPermissionsForm extends FormBase {
   public function buildForm(array $form, FormStateInterface $form_state) {
     $role_names = array();
     $role_permissions = array();
+    $admin_roles = array();
     foreach ($this->getRoles() as $role_name => $role) {
       // Retrieve role names for columns.
       $role_names[$role_name] = String::checkPlain($role->label());
       // Fetch permissions for the roles.
       $role_permissions[$role_name] = $role->getPermissions();
+      $admin_roles[$role_name] = $role->isAdmin();
     }
 
     // Store $role_names for use when saving the data.
@@ -164,6 +166,11 @@ class UserPermissionsForm extends FormBase {
             '#attributes' => array('class' => array('rid-' . $rid)),
             '#parents' => array($rid, $perm),
           );
+          // Show a column of disabled but checked checkboxes.
+          if ($admin_roles[$rid]) {
+            $form['permissions'][$perm][$rid]['#disabled'] = TRUE;
+            $form['permissions'][$perm][$rid]['#default_value'] = TRUE;
+          }
         }
       }
     }
@@ -181,7 +188,7 @@ class UserPermissionsForm extends FormBase {
    */
   function submitForm(array &$form, FormStateInterface $form_state) {
     foreach ($form_state->getValue('role_names') as $role_name => $name) {
-      user_role_change_permissions($role_name, $form_state->getValue($role_name));
+      user_role_change_permissions($role_name, (array) $form_state->getValue($role_name));
     }
 
     drupal_set_message($this->t('The changes have been saved.'));

core/modules/user/src/RoleInterface.php

@@ -55,6 +55,24 @@ interface RoleInterface extends ConfigEntityInterface {
    */
   public function revokePermission($permission);
 
+  /**
+   * Indicates that a role has all available permissions.
+   *
+   * @return bool
+   *   TRUE if the role has all permissions.
+   */
+  public function isAdmin();
+
+  /**
+   * Sets the role to be an admin role.
+   *
+   * @param bool $is_admin
+   *   TRUE, if the role should be an admin role.
+   *
+   * return $this
+   */
+  public function setIsAdmin($is_admin);
+
   /**
    * Returns the weight.
    *

core/modules/user/src/RoleStorage.php

@@ -20,7 +20,8 @@ class RoleStorage extends ConfigEntityStorage implements RoleStorageInterface {
   public function isPermissionInRoles($permission, array $rids) {
     $has_permission = FALSE;
     foreach ($this->loadMultiple($rids) as $role) {
-      if ($role->hasPermission($permission)) {
+      /** @var \Drupal\user\RoleInterface $role */
+      if ($role->isAdmin() || $role->hasPermission($permission)) {
         $has_permission = TRUE;
         break;
       }

core/modules/user/src/Tests/UserPermissionsTest.php

@@ -8,6 +8,7 @@
 namespace Drupal\user\Tests;
 
 use Drupal\simpletest\WebTestBase;
+use Drupal\user\Entity\Role;
 use Drupal\user\RoleStorage;
 
 /**
@@ -49,6 +50,12 @@ class UserPermissionsTest extends WebTestBase {
   function testUserPermissionChanges() {
     $permissions_hash_generator = $this->container->get('user.permissions_hash');
 
+    $storage = $this->container->get('entity.manager')->getStorage('user_role');
+
+    // Create an additional role and mark it as admin role.
+    Role::create(['is_admin' => TRUE, 'id' => 'administrator', 'label' => 'Administrator'])->save();
+    $storage->resetCache();
+
     $this->drupalLogin($this->adminUser);
     $rid = $this->rid;
     $account = $this->adminUser;
@@ -61,7 +68,6 @@ class UserPermissionsTest extends WebTestBase {
     $edit[$rid . '[administer users]'] = TRUE;
     $this->drupalPostForm('admin/people/permissions', $edit, t('Save permissions'));
     $this->assertText(t('The changes have been saved.'), 'Successful save message displayed.');
-    $storage = $this->container->get('entity.manager')->getStorage('user_role');
     $storage->resetCache();
     $this->assertTrue($account->hasPermission('administer users'), 'User now has "administer users" permission.');
     $current_permissions_hash = $permissions_hash_generator->generate($account);
@@ -80,6 +86,12 @@ class UserPermissionsTest extends WebTestBase {
     $current_permissions_hash = $permissions_hash_generator->generate($account);
     $this->assertIdentical($current_permissions_hash, $permissions_hash_generator->generate($this->loggedInUser));
     $this->assertNotEqual($previous_permissions_hash, $current_permissions_hash, 'Permissions hash has changed.');
+
+    // Ensure that the admin role doesn't have any checkboxes.
+    $this->drupalGet('admin/people/permissions');
+    foreach (array_keys($this->container->get('user.permissions')->getPermissions()) as $permission) {
+      $this->assertNoFieldByName('administrator[' . $permission . ']');
+    }
   }
 
   /**
@@ -92,16 +104,37 @@ class UserPermissionsTest extends WebTestBase {
     // Verify that the administration role is none by default.
     $this->assertOptionSelected('edit-user-admin-role', '', 'Administration role defaults to none.');
 
+    $this->assertFalse(Role::load($this->rid)->isAdmin());
+
     // Set the user's role to be the administrator role.
     $edit = array();
     $edit['user_admin_role'] = $this->rid;
     $this->drupalPostForm('admin/config/people/accounts', $edit, t('Save configuration'));
 
+    \Drupal::entityManager()->getStorage('user_role')->resetCache();
+    $this->assertTrue(Role::load($this->rid)->isAdmin());
+
     // Enable aggregator module and ensure the 'administer news feeds'
     // permission is assigned by default.
     \Drupal::service('module_installer')->install(array('aggregator'));
 
     $this->assertTrue($this->adminUser->hasPermission('administer news feeds'), 'The permission was automatically assigned to the administrator role');
+
+    // Ensure that selecting '- None -' removes the admin role.
+    $edit = array();
+    $edit['user_admin_role'] = '';
+    $this->drupalPostForm('admin/config/people/accounts', $edit, t('Save configuration'));
+
+    \Drupal::entityManager()->getStorage('user_role')->resetCache();
+    \Drupal::configFactory()->reset();
+    $this->assertFalse(Role::load($this->rid)->isAdmin());
+
+    // Manually create two admin roles, in that case the single select should be
+    // hidden.
+    Role::create(['id' => 'admin_role_0', 'is_admin' => TRUE, 'label' => 'Admin role 0'])->save();
+    Role::create(['id' => 'admin_role_1', 'is_admin' => TRUE, 'label' => 'Admin role 1'])->save();
+    $this->drupalGet('admin/config/people/accounts');
+    $this->assertNoFieldByName('user_admin_role');
   }
 
   /**

core/modules/user/user.module

@@ -1329,25 +1329,6 @@ function user_form_process_password_confirm($element) {
   return $element;
 }
 
-/**
- * Implements hook_modules_installed().
- */
-function user_modules_installed($modules) {
-  // Assign all available permissions to the administrator role.
-  $rid = \Drupal::config('user.settings')->get('admin_role');
-  if ($rid) {
-    // Some permissions call the url generator, so ensure that the routes are
-    // up to date.
-    \Drupal::service('router.builder_indicator')->setRebuildNeeded();
-    /** @var \Drupal\user\PermissionHandlerInterface $permission_handler */
-    $permission_handler = \Drupal::service('user.permissions');
-    $permissions = $permission_handler->getPermissions();
-    if (!empty($permissions)) {
-      user_role_grant_permissions($rid, array_keys($permissions));
-    }
-  }
-}
-
 /**
  * Implements hook_modules_uninstalled().
  */

core/profiles/standard/config/install/user.role.administrator.yml

@@ -2,3 +2,4 @@ id: administrator
 label: Administrator
 weight: 2
 langcode: en
+is_admin: true

core/profiles/standard/standard.install

@@ -32,11 +32,6 @@ function standard_install() {
   user_role_grant_permissions(DRUPAL_ANONYMOUS_RID, array('access comments'));
   user_role_grant_permissions(DRUPAL_AUTHENTICATED_RID, array('access comments', 'post comments', 'skip comment approval'));
 
-  // Enable all permissions for the administrator role.
-  user_role_grant_permissions('administrator', array_keys(\Drupal::service('user.permissions')->getPermissions()));
-  // Set this as the administrator role.
-  $user_settings->set('admin_role', 'administrator')->save();
-
   // Assign user 1 the "administrator" role.
   $user = User::load(1);
   $user->roles[] = 'administrator';