From 16ba3ffc53112619b82a9505fc25134326c34754 Mon Sep 17 00:00:00 2001 From: Kristiaan Van den Eynde Date: Wed, 4 Oct 2017 13:29:20 +0200 Subject: [PATCH] interdiff --- .../Tests/Update/UserUpdateUserOneAdminTest.php | 47 +++++++++++++++++++--- core/modules/user/user.install | 21 ++++++---- core/modules/user/user.post_update.php | 21 +++++++++- 3 files changed, 75 insertions(+), 14 deletions(-) diff --git a/core/modules/user/src/Tests/Update/UserUpdateUserOneAdminTest.php b/core/modules/user/src/Tests/Update/UserUpdateUserOneAdminTest.php index 0e8a0bf..61b0839 100644 --- a/core/modules/user/src/Tests/Update/UserUpdateUserOneAdminTest.php +++ b/core/modules/user/src/Tests/Update/UserUpdateUserOneAdminTest.php @@ -3,6 +3,7 @@ namespace Drupal\user\Tests\Update; use Drupal\system\Tests\Update\UpdatePathTestBase; +use Drupal\user\Entity\Role; use Drupal\user\Entity\User; use Drupal\user\RoleInterface; @@ -29,16 +30,41 @@ public function testUserOneAdminRole() { // Standard install profile used to assign the administrator role to user 1. // Let's check for its presence, remove it and check whether the removal did // its job. - $this->assertTrue($this->UserOneHasAdministratorRole()); + $this->assertTrue($this->UserOneHasAdminRole()); $user1 = User::load(1); $user1->removeRole(RoleInterface::ADMINISTRATOR_ID); $user1->save(); - $this->assertFalse($this->UserOneHasAdministratorRole()); + $this->assertFalse($this->UserOneHasAdminRole()); - // The user_post_update_grant_user_1_admin_role() post update function + // The user_post_update_change_user_1_and_admin_role() post update function // should have added the administrator role to user 1. $this->runUpdates(); - $this->assertTrue($this->UserOneHasAdministratorRole()); + $this->assertTrue($this->UserOneHasAdminRole()); + } + + /** + * Tests that an existing non-admin 'administrator' role is renamed. + */ + public function testResolvesAdminRoleCollision() { + // Standard install profile used to assign the administrator role to user 1. + // Let's turn it into a non-admin role by assigning it a few permissions and + // unchecking the isAdmin() flag. It should then be migrated to a new role + // called administrator_legacy and users who had it should also receive the + // new role. + $admin_role = Role::load(RoleInterface::ADMINISTRATOR_ID); + $admin_role->setIsAdmin(FALSE); + $admin_role->grantPermission('access content'); + $admin_role->save(); + + // The user_post_update_change_user_1_and_admin_role() post update function + // should have added the administrator_legacy role to user 1. + $this->assertTrue($this->UserOneHasAdminRole()); + $this->runUpdates(); + $this->assertTrue($this->UserOneHasLegacyAdminRole(), 'User one received the legacy admin role.'); + + // The legacy admin role should have the permissions of the old admin role. + $legacy_admin_role = Role::load(RoleInterface::ADMINISTRATOR_ID . '_legacy'); + $this->assertEqual(['access content'], $legacy_admin_role->getPermissions(), 'Legacy admin role has the right permissions.'); } /** @@ -47,9 +73,20 @@ public function testUserOneAdminRole() { * @return bool * Whether user 1 has the administrator role. */ - protected function UserOneHasAdministratorRole() { + protected function UserOneHasAdminRole() { $user1 = User::load(1); return in_array(RoleInterface::ADMINISTRATOR_ID, $user1->getRoles(), TRUE); } + /** + * Checks whether user 1 has the legacy administrator role. + * + * @return bool + * Whether user 1 has the legacy administrator role. + */ + protected function UserOneHasLegacyAdminRole() { + $user1 = User::load(1); + return in_array(RoleInterface::ADMINISTRATOR_ID . '_legacy', $user1->getRoles(), TRUE); + } + } diff --git a/core/modules/user/user.install b/core/modules/user/user.install index bdab712..f420634 100644 --- a/core/modules/user/user.install +++ b/core/modules/user/user.install @@ -112,8 +112,20 @@ function user_update_8101() { $config_name = 'user.role.administrator'; $role = $config_factory->getEditable($config_name); - // Add the administrator role if it doesn't exist yet. - if ($role->isNew()) { + // We need to be careful not to grant people more access all of the sudden. + // Migrate the administrator role to a new administrator_legacy role. We will + // assign old administrators that role in the post update hook. + if (!$role->isNew() && !$role->get('is_admin')) { + $legacy_role = $config_factory->getEditable($config_name . '_legacy'); + $legacy_role->setData($role->getRawData()); + $legacy_role->set('id', 'administrator_legacy'); + $legacy_role->set('label', 'Administrator (legacy)'); + $legacy_role->save(TRUE); + } + + // Add the administrator role if it doesn't exist yet or if we migrated an + // existing administrator role to administrator_legacy. + if ($role->isNew() || isset($legacy_role)) { $yaml_storage = new ExtensionInstallStorage( \Drupal::service('config.storage'), InstallStorage::CONFIG_INSTALL_DIRECTORY @@ -121,9 +133,4 @@ function user_update_8101() { $role->setData($yaml_storage->read($config_name)); $role->save(TRUE); } - // Give the role admin privileges if it does not have them. - elseif (!$role->get('is_admin')) { - $role->set('is_admin', TRUE); - $role->save(TRUE); - } } diff --git a/core/modules/user/user.post_update.php b/core/modules/user/user.post_update.php index 96afce8..9851c9f 100644 --- a/core/modules/user/user.post_update.php +++ b/core/modules/user/user.post_update.php @@ -24,9 +24,26 @@ function user_post_update_enforce_order_of_permissions() { } /** - * Grant user 1 the administrator role. + * Grant user 1 the administrator role and potentially update legacy admins. */ -function user_post_update_grant_user_1_admin_role() { +function user_post_update_change_user_1_and_admin_role() { + // If we have a legacy administrator role, we need to update all users who had + // the original administrator role to now have the legacy role. + if ($role = Role::load(RoleInterface::ADMINISTRATOR_ID . '_legacy')) { + $uids = Drupal::entityQuery('user') + ->condition('roles', RoleInterface::ADMINISTRATOR_ID) + ->execute(); + + if (!empty($uids)) { + /** @var \Drupal\user\UserInterface $account */ + foreach (User::loadMultiple($uids) as $account) { + $account->removeRole(RoleInterface::ADMINISTRATOR_ID); + $account->addRole(RoleInterface::ADMINISTRATOR_ID . '_legacy'); + $account->save(); + } + } + } + $account = User::load(1); $account->addRole(RoleInterface::ADMINISTRATOR_ID); $account->save(); -- 2.8.1