From 3ddd0e5187c3afd79d441374f681629605d02199 Mon Sep 17 00:00:00 2001 From: Kristiaan Van den Eynde Date: Thu, 25 May 2017 11:00:36 +0200 Subject: [PATCH] Issue #540008 by kristiaanvandeneynde, michaelfavia, ianthomas_uk, zaporylie, greggles: Remove the special behavior of uid #1 --- core/core.services.yml | 2 +- .../Core/Cache/Context/UserRolesCacheContext.php | 7 -- core/lib/Drupal/Core/Session/AccountInterface.php | 5 + .../Core/Session/PermissionsHashGenerator.php | 25 +++- core/lib/Drupal/Core/Session/UserSession.php | 5 - .../Drupal/Core/Test/FunctionalTestSetupTrait.php | 2 + .../CommentDefaultFormatterCacheTagsTest.php | 36 +++--- .../src/Kernel/Views/CommentAdminViewTest.php | 57 ++++----- .../Kernel/Views/CommentViewsKernelTestBase.php | 2 + .../tests/src/Kernel/FileManagedUnitTestBase.php | 2 +- .../tests/src/Kernel/FilterDefaultConfigTest.php | 3 + .../tests/src/Kernel/TextFormatElementFormTest.php | 7 +- .../EntityResource/Role/RoleResourceTestBase.php | 2 +- .../tests/src/Functional/Module/UninstallTest.php | 4 +- .../Kernel/DateFormatAccessControlHandlerTest.php | 34 +----- .../src/Kernel/MenuAccessControlHandlerTest.php | 34 +----- .../config/install/user.role.administrator.yml | 8 ++ core/modules/user/src/AccountSettingsForm.php | 44 ------- core/modules/user/src/Entity/User.php | 14 +-- core/modules/user/src/RoleAccessControlHandler.php | 7 +- core/modules/user/src/RoleInterface.php | 5 + .../modules/user/src/Tests/UserPermissionsTest.php | 47 -------- .../Update/UserUpdateUserOneAdminTest.php | 106 ++++++++++++++++ .../user/tests/src/Kernel/UserEntityTest.php | 14 ++- .../user/tests/src/Kernel/UserInstallTest.php | 5 + .../src/Kernel/Views/UserViewsFieldAccessTest.php | 19 --- core/modules/user/user.install | 38 ++++++ core/modules/user/user.post_update.php | 29 +++++ .../Kernel/Handler/FieldFieldAccessTestBase.php | 2 +- .../tests/src/Kernel/Handler/FieldFieldTest.php | 6 +- .../src/Kernel/Handler/FieldRenderedEntityTest.php | 12 +- .../tests/src/Kernel/Plugin/RowRenderCacheTest.php | 3 +- .../config/install/user.role.administrator.yml | 8 -- core/profiles/standard/standard.install | 6 - .../Element/EntityAutocompleteElementFormTest.php | 7 ++ .../KernelTests/Core/Render/RenderCacheTest.php | 44 +++---- .../Core/Session/PermissionsHashGeneratorTest.php | 133 +++++++++++++++++---- 37 files changed, 450 insertions(+), 334 deletions(-) create mode 100644 core/modules/user/config/install/user.role.administrator.yml create mode 100644 core/modules/user/tests/src/Functional/Update/UserUpdateUserOneAdminTest.php delete mode 100644 core/profiles/standard/config/install/user.role.administrator.yml diff --git a/core/core.services.yml b/core/core.services.yml index 21a3e59..217db4c 100644 --- a/core/core.services.yml +++ b/core/core.services.yml @@ -1477,7 +1477,7 @@ services: arguments: ['@current_user', '@session_handler.write_safe'] user_permissions_hash_generator: class: Drupal\Core\Session\PermissionsHashGenerator - arguments: ['@private_key', '@cache.bootstrap', '@cache.static'] + arguments: ['@private_key', '@cache.bootstrap', '@cache.static', '@entity_type.manager'] current_user: class: Drupal\Core\Session\AccountProxy session_configuration: diff --git a/core/lib/Drupal/Core/Cache/Context/UserRolesCacheContext.php b/core/lib/Drupal/Core/Cache/Context/UserRolesCacheContext.php index 0cd0fbe..0a7f909 100644 --- a/core/lib/Drupal/Core/Cache/Context/UserRolesCacheContext.php +++ b/core/lib/Drupal/Core/Cache/Context/UserRolesCacheContext.php @@ -27,13 +27,6 @@ public static function getLabel() { * {@inheritdoc} */ public function getContext($role = NULL) { - // User 1 does not actually have any special behavior for roles; this is - // added as additional security and backwards compatibility protection for - // SA-CORE-2015-002. - // @todo Remove in Drupal 9.0.0. - if ($this->user->id() == 1) { - return 'is-super-user'; - } if ($role === NULL) { return implode(',', $this->user->getRoles()); } diff --git a/core/lib/Drupal/Core/Session/AccountInterface.php b/core/lib/Drupal/Core/Session/AccountInterface.php index 1cf82dc..f3567f8 100644 --- a/core/lib/Drupal/Core/Session/AccountInterface.php +++ b/core/lib/Drupal/Core/Session/AccountInterface.php @@ -23,6 +23,11 @@ const AUTHENTICATED_ROLE = 'authenticated'; /** + * Role ID for the administrator users. + */ + const ADMINISTRATOR_ROLE = 'administrator'; + + /** * Returns the user ID or 0 for anonymous. * * @return int diff --git a/core/lib/Drupal/Core/Session/PermissionsHashGenerator.php b/core/lib/Drupal/Core/Session/PermissionsHashGenerator.php index e172537..60e40ab 100644 --- a/core/lib/Drupal/Core/Session/PermissionsHashGenerator.php +++ b/core/lib/Drupal/Core/Session/PermissionsHashGenerator.php @@ -5,6 +5,7 @@ use Drupal\Core\PrivateKey; use Drupal\Core\Cache\Cache; use Drupal\Core\Cache\CacheBackendInterface; +use Drupal\Core\Entity\EntityTypeManagerInterface; use Drupal\Core\Site\Settings; /** @@ -34,6 +35,13 @@ class PermissionsHashGenerator implements PermissionsHashGeneratorInterface { protected $static; /** + * The entity type manager. + * + * @var \Drupal\Core\Entity\EntityTypeManagerInterface + */ + protected $entityTypeManager; + + /** * Constructs a PermissionsHashGenerator object. * * @param \Drupal\Core\PrivateKey $private_key @@ -42,11 +50,14 @@ class PermissionsHashGenerator implements PermissionsHashGeneratorInterface { * The cache backend interface to use for the persistent cache. * @param \Drupal\Core\Cache\CacheBackendInterface $static * The cache backend interface to use for the static cache. + * @param \Drupal\Core\Entity\EntityTypeManagerInterface $entity_type_manager + * The entity type manager used to retrieve a storage handler from. */ - public function __construct(PrivateKey $private_key, CacheBackendInterface $cache, CacheBackendInterface $static) { + public function __construct(PrivateKey $private_key, CacheBackendInterface $cache, CacheBackendInterface $static, EntityTypeManagerInterface $entity_type_manager) { $this->privateKey = $private_key; $this->cache = $cache; $this->static = $static; + $this->entityTypeManager = $entity_type_manager; } /** @@ -55,10 +66,14 @@ public function __construct(PrivateKey $private_key, CacheBackendInterface $cach * Cached by role, invalidated whenever permissions change. */ public function generate(AccountInterface $account) { - // User 1 is the super user, and can always access all permissions. Use a - // different, unique identifier for the hash. - if ($account->id() == 1) { - return $this->hash('is-super-user'); + // Admin roles have all permissions implicitly assigned. Use a different, + // unique identifier for the hash. + $storage = $this->entityTypeManager->getStorage('user_role'); + foreach ($storage->loadMultiple($account->getRoles()) as $role) { + /** @var \Drupal\user\RoleInterface $role */ + if ($role->isAdmin()) { + return $this->hash('is-admin'); + } } $sorted_roles = $account->getRoles(); diff --git a/core/lib/Drupal/Core/Session/UserSession.php b/core/lib/Drupal/Core/Session/UserSession.php index f426803..d07dfe2 100644 --- a/core/lib/Drupal/Core/Session/UserSession.php +++ b/core/lib/Drupal/Core/Session/UserSession.php @@ -103,11 +103,6 @@ public function getRoles($exclude_locked_roles = FALSE) { * {@inheritdoc} */ public function hasPermission($permission) { - // User #1 has all privileges. - if ((int) $this->id() === 1) { - return TRUE; - } - return $this->getRoleStorage()->isPermissionInRoles($permission, $this->getRoles()); } diff --git a/core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php b/core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php index f9f97ec..e1458ea 100644 --- a/core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php +++ b/core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php @@ -10,6 +10,7 @@ use Drupal\Core\DrupalKernel; use Drupal\Core\Extension\MissingDependencyException; use Drupal\Core\Serialization\Yaml; +use Drupal\Core\Session\AccountInterface; use Drupal\Core\Session\UserSession; use Drupal\Core\Site\Settings; use Drupal\Core\StreamWrapper\StreamWrapperInterface; @@ -370,6 +371,7 @@ protected function initUserSession() { 'pass_raw' => $password, 'passRaw' => $password, 'timezone' => date_default_timezone_get(), + 'roles' => [AccountInterface::AUTHENTICATED_ROLE, AccountInterface::ADMINISTRATOR_ROLE] ]); // The child site derives its session name from the database prefix when diff --git a/core/modules/comment/tests/src/Kernel/CommentDefaultFormatterCacheTagsTest.php b/core/modules/comment/tests/src/Kernel/CommentDefaultFormatterCacheTagsTest.php index bdea71d..bed4521 100644 --- a/core/modules/comment/tests/src/Kernel/CommentDefaultFormatterCacheTagsTest.php +++ b/core/modules/comment/tests/src/Kernel/CommentDefaultFormatterCacheTagsTest.php @@ -34,8 +34,10 @@ class CommentDefaultFormatterCacheTagsTest extends EntityKernelTestBase { protected function setUp() { parent::setUp(); - $session = new Session(); + $this->container->get('module_handler')->loadInclude('comment', 'install'); + comment_install(); + $session = new Session(); $request = Request::create('/'); $request->setSession($session); @@ -48,7 +50,7 @@ protected function setUp() { // user does not have access to the 'administer comments' permission, to // ensure only published comments are visible to the end user. $current_user = $this->container->get('current_user'); - $current_user->setAccount($this->createUser([], ['access comments'])); + $current_user->setAccount($this->createUser([], ['access comments', 'post comments'])); // Install tables and config needed to render comments. $this->installSchema('comment', ['comment_entity_statistics']); @@ -66,6 +68,14 @@ protected function setUp() { * Tests the bubbling of cache tags. */ public function testCacheTags() { + $entity_type_manager = $this->container->get('entity_type.manager'); + + /** @var \Drupal\Core\Entity\EntityViewBuilderInterface $view_builder */ + $view_builder = $entity_type_manager->getViewBuilder('entity_test'); + + /** @var \Drupal\Core\Entity\ContentEntityStorageInterface $storage */ + $storage = $entity_type_manager->getStorage('entity_test'); + /** @var \Drupal\Core\Render\RendererInterface $renderer */ $renderer = $this->container->get('renderer'); @@ -74,18 +84,16 @@ public function testCacheTags() { $commented_entity->save(); // Verify cache tags on the rendered entity before it has comments. - $build = \Drupal::entityManager() - ->getViewBuilder('entity_test') - ->view($commented_entity); + $build = $view_builder->view($commented_entity); $renderer->renderRoot($build); $expected_cache_tags = [ - 'entity_test_view', - 'entity_test:' . $commented_entity->id(), 'config:core.entity_form_display.comment.comment.default', 'config:field.field.comment.comment.comment_body', 'config:field.field.entity_test.entity_test.comment', 'config:field.storage.comment.comment_body', 'config:user.settings', + 'entity_test_view', + 'entity_test:' . $commented_entity->id(), ]; sort($expected_cache_tags); $this->assertEqual($build['#cache']['tags'], $expected_cache_tags); @@ -109,17 +117,11 @@ public function testCacheTags() { ]); $comment->save(); - // Load commented entity so comment_count gets computed. - // @todo Remove the $reset = TRUE parameter after - // https://www.drupal.org/node/597236 lands. It's a temporary work-around. - $storage = $this->container->get('entity_type.manager')->getStorage('entity_test'); - $storage->resetCache([$commented_entity->id()]); + // Load commented entity again so we get the updated comment_count. $commented_entity = $storage->load($commented_entity->id()); // Verify cache tags on the rendered entity when it has comments. - $build = \Drupal::entityManager() - ->getViewBuilder('entity_test') - ->view($commented_entity); + $build = $view_builder->view($commented_entity); $renderer->renderRoot($build); $expected_cache_tags = [ 'entity_test_view', @@ -142,9 +144,7 @@ public function testCacheTags() { // builder elements bubble up outside of the entity and we can check that // it got the correct cache max age. $build = ['#type' => 'container']; - $build['entity'] = \Drupal::entityManager() - ->getViewBuilder('entity_test') - ->view($commented_entity); + $build['entity'] = $view_builder->view($commented_entity); $renderer->renderRoot($build); // The entity itself was cached but the top-level element is max-age 0 due diff --git a/core/modules/comment/tests/src/Kernel/Views/CommentAdminViewTest.php b/core/modules/comment/tests/src/Kernel/Views/CommentAdminViewTest.php index 86e8949..aa876f5 100644 --- a/core/modules/comment/tests/src/Kernel/Views/CommentAdminViewTest.php +++ b/core/modules/comment/tests/src/Kernel/Views/CommentAdminViewTest.php @@ -94,19 +94,22 @@ protected function setUp($import_test_views = TRUE) { $entity->save(); // Create some comments. - $comment = Comment::create([ + $comment_data = [ 'subject' => 'My comment title', 'uid' => $this->adminUser->id(), 'entity_type' => 'entity_test', 'comment_type' => 'comment', 'status' => 1, 'entity_id' => $entity->id(), - ]); + ]; + $comment = Comment::create($comment_data); $comment->save(); - + $comment_data['subject'] = 'My comment ur title'; + $comment_translation = $comment->addTranslation('ur', $comment_data); + $comment_translation->save(); $this->comments[] = $comment; - $comment_anonymous = Comment::create([ + $comment_anonymous_data = [ 'subject' => 'Anonymous comment title', 'uid' => 0, 'name' => 'barry', @@ -117,8 +120,12 @@ protected function setUp($import_test_views = TRUE) { 'created' => 123456, 'status' => 1, 'entity_id' => $entity->id(), - ]); + ]; + $comment_anonymous = Comment::create($comment_anonymous_data); $comment_anonymous->save(); + $comment_anonymous_data['subject'] = 'Anonymous comment ur title'; + $comment_anonymous_translation = $comment_anonymous->addTranslation('ur', $comment_anonymous_data); + $comment_anonymous_translation->save(); $this->comments[] = $comment_anonymous; } @@ -131,6 +138,9 @@ public function testFilters() { foreach ($this->comments as $comment) { $comment->setUnpublished(); $comment->save(); + $comment_translation = $comment->getTranslation('ur'); + $comment_translation->setUnpublished(); + $comment_translation->save(); } $this->doTestFilters('page_unapproved'); } @@ -162,7 +172,7 @@ protected function doTestFilters($display_id) { $this->assertField('langcode'); $elements = $this->cssSelect('input[type="checkbox"]'); - $this->assertEquals(2, count($elements), 'There are two comments on the page.'); + $this->assertEquals(4, count($elements), 'There are four comments on the page.'); $this->assertText($comment->label()); $this->assertText($comment_anonymous->label()); $executable->destroy(); @@ -174,7 +184,7 @@ protected function doTestFilters($display_id) { $this->verbose($this->getRawContent()); $elements = $this->cssSelect('input[type="checkbox"]'); - $this->assertEquals(1, count($elements), 'Only anonymous comment is visible.'); + $this->assertEquals(2, count($elements), 'Only anonymous comments are visible.'); $this->assertNoText($comment->label()); $this->assertText($comment_anonymous->label()); $executable->destroy(); @@ -185,7 +195,7 @@ protected function doTestFilters($display_id) { $this->verbose($this->getRawContent()); $elements = $this->cssSelect('input[type="checkbox"]'); - $this->assertEquals(1, count($elements), 'Only admin comment is visible.'); + $this->assertEquals(2, count($elements), 'Only admin comments are visible.'); $this->assertText($comment->label()); $this->assertNoText($comment_anonymous->label()); $executable->destroy(); @@ -197,7 +207,7 @@ protected function doTestFilters($display_id) { $this->verbose($this->getRawContent()); $elements = $this->cssSelect('input[type="checkbox"]'); - $this->assertEquals(1, count($elements), 'Only anonymous comment is visible.'); + $this->assertEquals(2, count($elements), 'Only anonymous comments are visible.'); $this->assertNoText($comment->label()); $this->assertText($comment_anonymous->label()); $executable->destroy(); @@ -209,7 +219,7 @@ protected function doTestFilters($display_id) { $this->verbose($this->getRawContent()); $elements = $this->cssSelect('input[type="checkbox"]'); - $this->assertEquals(1, count($elements), 'Only admin comment is visible.'); + $this->assertEquals(2, count($elements), 'Only admin comments are visible.'); $this->assertText($comment->label()); $this->assertNoText($comment_anonymous->label()); $executable->destroy(); @@ -227,29 +237,6 @@ protected function doTestFilters($display_id) { $executable->destroy(); // Tests comment translation filter. - if (!$comment->hasTranslation('ur')) { - // If we don't have the translation then create one. - $comment_translation = $comment->addTranslation('ur', ['subject' => 'ur title']); - $comment_translation->save(); - } - else { - // If we have the translation then unpublish it. - $comment_translation = $comment->getTranslation('ur'); - $comment_translation->setUnpublished(); - $comment_translation->save(); - } - if (!$comment_anonymous->hasTranslation('ur')) { - // If we don't have the translation then create one. - $comment_anonymous_translation = $comment_anonymous->addTranslation('ur', ['subject' => 'ur Anonymous title']); - $comment_anonymous_translation->save(); - } - else { - // If we have the translation then unpublish it. - $comment_anonymous_translation = $comment_anonymous->getTranslation('ur'); - $comment_anonymous_translation->setUnpublished(); - $comment_anonymous_translation->save(); - } - $executable->setExposedInput(['langcode' => 'ur']); $build = $executable->preview($display_id); $this->setRawContent($renderer->renderRoot($build)); @@ -259,8 +246,8 @@ protected function doTestFilters($display_id) { $this->assertEquals(2, count($elements), 'Both comments are visible.'); $this->assertNoText($comment->label()); $this->assertNoText($comment_anonymous->label()); - $this->assertText($comment_translation->label()); - $this->assertText($comment_anonymous_translation->label()); + $this->assertText($comment->getTranslation('ur')->label()); + $this->assertText($comment_anonymous->getTranslation('ur')->label()); $executable->destroy(); } diff --git a/core/modules/comment/tests/src/Kernel/Views/CommentViewsKernelTestBase.php b/core/modules/comment/tests/src/Kernel/Views/CommentViewsKernelTestBase.php index 5167503..3e98768 100644 --- a/core/modules/comment/tests/src/Kernel/Views/CommentViewsKernelTestBase.php +++ b/core/modules/comment/tests/src/Kernel/Views/CommentViewsKernelTestBase.php @@ -63,6 +63,8 @@ protected function setUp($import_test_views = TRUE) { $admin_role = Role::create(['id' => 'admin']); $admin_role->grantPermission('administer comments'); + $admin_role->grantPermission('access comments'); + $admin_role->grantPermission('post comments'); $admin_role->save(); /* @var \Drupal\user\RoleInterface $anonymous_role */ diff --git a/core/modules/file/tests/src/Kernel/FileManagedUnitTestBase.php b/core/modules/file/tests/src/Kernel/FileManagedUnitTestBase.php index 7c43e9b..f2c8871 100644 --- a/core/modules/file/tests/src/Kernel/FileManagedUnitTestBase.php +++ b/core/modules/file/tests/src/Kernel/FileManagedUnitTestBase.php @@ -32,7 +32,7 @@ protected function setUp() { // Make sure that a user with uid 1 exists, self::createFile() relies on // it. - $user = User::create(['uid' => 1, 'name' => $this->randomMachineName()]); + $user = User::create(['uid' => 1, 'status' => 1, 'name' => $this->randomMachineName()]); $user->enforceIsNew(); $user->save(); \Drupal::currentUser()->setAccount($user); diff --git a/core/modules/filter/tests/src/Kernel/FilterDefaultConfigTest.php b/core/modules/filter/tests/src/Kernel/FilterDefaultConfigTest.php index e16bdda..896873f 100644 --- a/core/modules/filter/tests/src/Kernel/FilterDefaultConfigTest.php +++ b/core/modules/filter/tests/src/Kernel/FilterDefaultConfigTest.php @@ -47,6 +47,7 @@ public function testInstallation() { $this->assertEqual(array_keys(filter_get_roles_by_format($format)), [ RoleInterface::ANONYMOUS_ID, RoleInterface::AUTHENTICATED_ID, + RoleInterface::ADMINISTRATOR_ID, ]); // Verify enabled filters. @@ -76,6 +77,7 @@ public function testUpdateRoles() { $this->assertEqual(array_keys(filter_get_roles_by_format($format)), [ RoleInterface::ANONYMOUS_ID, RoleInterface::AUTHENTICATED_ID, + RoleInterface::ADMINISTRATOR_ID, ]); // Attempt to change roles. @@ -89,6 +91,7 @@ public function testUpdateRoles() { $this->assertEqual(array_keys(filter_get_roles_by_format($format)), [ RoleInterface::ANONYMOUS_ID, RoleInterface::AUTHENTICATED_ID, + RoleInterface::ADMINISTRATOR_ID, ]); } diff --git a/core/modules/filter/tests/src/Kernel/TextFormatElementFormTest.php b/core/modules/filter/tests/src/Kernel/TextFormatElementFormTest.php index 8fca62e..a0ea403 100644 --- a/core/modules/filter/tests/src/Kernel/TextFormatElementFormTest.php +++ b/core/modules/filter/tests/src/Kernel/TextFormatElementFormTest.php @@ -44,15 +44,16 @@ protected function setUp() { $manager = \Drupal::service('plugin.manager.element_info'); $manager->clearCachedDefinitions(); $manager->getDefinitions(); - /* @var \Drupal\filter\FilterFormatInterface $filter_test_format */ - $filter_test_format = FilterFormat::load('filter_test'); /* @var \Drupal\user\RoleInterface $role */ $role = Role::create([ 'id' => 'admin', 'label' => 'admin', ]); - $role->grantPermission($filter_test_format->getPermissionName()); + foreach (FilterFormat::loadMultiple(['full_html', 'filtered_html', 'filter_test']) as $format) { + /* @var \Drupal\filter\FilterFormatInterface $format */ + $role->grantPermission($format->getPermissionName()); + } $role->save(); $this->testUser = User::create([ 'name' => 'foobar', diff --git a/core/modules/rest/tests/src/Functional/EntityResource/Role/RoleResourceTestBase.php b/core/modules/rest/tests/src/Functional/EntityResource/Role/RoleResourceTestBase.php index ee719c4..041934d 100644 --- a/core/modules/rest/tests/src/Functional/EntityResource/Role/RoleResourceTestBase.php +++ b/core/modules/rest/tests/src/Functional/EntityResource/Role/RoleResourceTestBase.php @@ -48,7 +48,7 @@ protected function createEntity() { protected function getExpectedNormalizedEntity() { return [ 'uuid' => $this->entity->uuid(), - 'weight' => 2, + 'weight' => 3, 'langcode' => 'en', 'status' => TRUE, 'dependencies' => [], diff --git a/core/modules/system/tests/src/Functional/Module/UninstallTest.php b/core/modules/system/tests/src/Functional/Module/UninstallTest.php index 2b17b8f..f491a7b 100644 --- a/core/modules/system/tests/src/Functional/Module/UninstallTest.php +++ b/core/modules/system/tests/src/Functional/Module/UninstallTest.php @@ -5,6 +5,7 @@ use Drupal\Core\Cache\Cache; use Drupal\Component\Utility\SafeMarkup; use Drupal\Core\Entity\EntityMalformedException; +use Drupal\Core\Session\AccountInterface; use Drupal\node\Entity\Node; use Drupal\node\Entity\NodeType; use Drupal\Tests\BrowserTestBase; @@ -32,7 +33,8 @@ public function testUserPermsUninstalled() { $this->container->get('module_installer')->uninstall(['module_test']); // Are the perms defined by module_test removed? - $this->assertFalse(user_roles(FALSE, 'module_test perm'), 'Permissions were all removed.'); + $roles = array_diff_key(user_roles(FALSE, 'module_test perm'), [AccountInterface::ADMINISTRATOR_ROLE => 1]); + $this->assertFalse($roles, 'Permissions were all removed.'); } /** diff --git a/core/modules/system/tests/src/Kernel/DateFormatAccessControlHandlerTest.php b/core/modules/system/tests/src/Kernel/DateFormatAccessControlHandlerTest.php index 01b09dc..758132e 100644 --- a/core/modules/system/tests/src/Kernel/DateFormatAccessControlHandlerTest.php +++ b/core/modules/system/tests/src/Kernel/DateFormatAccessControlHandlerTest.php @@ -53,18 +53,10 @@ protected function setUp() { * @dataProvider testAccessProvider */ public function testAccess($which_user, $which_entity, $view_label_access_result, $view_access_result, $update_access_result, $delete_access_result, $create_access_result) { - // We must always create user 1, so that a "normal" user has a ID >1. - $root_user = $this->drupalCreateUser(); - - if ($which_user === 'user1') { - $user = $root_user; - } - else { - $permissions = ($which_user === 'admin') - ? ['administer site configuration'] - : []; - $user = $this->drupalCreateUser($permissions); - } + $permissions = ($which_user === 'admin') + ? ['administer site configuration'] + : []; + $user = $this->drupalCreateUser($permissions); $entity_values = ($which_entity === 'unlocked') ? ['locked' => FALSE] @@ -125,24 +117,6 @@ public function testAccessProvider() { AccessResult::forbidden()->addCacheTags(['rendered'])->setReason("The DateFormat config entity is locked."), AccessResult::allowed()->addCacheContexts(['user.permissions']), ], - 'user1 + unlocked' => [ - 'user1', - 'unlocked', - AccessResult::allowed(), - AccessResult::allowed()->addCacheContexts(['user.permissions']), - AccessResult::allowed()->addCacheContexts(['user.permissions'])->addCacheTags(['rendered']), - AccessResult::allowed()->addCacheContexts(['user.permissions'])->addCacheTags(['rendered']), - AccessResult::allowed()->addCacheContexts(['user.permissions']), - ], - 'user1 + locked' => [ - 'user1', - 'locked', - AccessResult::allowed(), - AccessResult::allowed()->addCacheContexts(['user.permissions']), - AccessResult::forbidden()->addCacheTags(['rendered'])->setReason("The DateFormat config entity is locked."), - AccessResult::forbidden()->addCacheTags(['rendered'])->setReason("The DateFormat config entity is locked."), - AccessResult::allowed()->addCacheContexts(['user.permissions']), - ], ]; } diff --git a/core/modules/system/tests/src/Kernel/MenuAccessControlHandlerTest.php b/core/modules/system/tests/src/Kernel/MenuAccessControlHandlerTest.php index 6d702c7..1801098 100644 --- a/core/modules/system/tests/src/Kernel/MenuAccessControlHandlerTest.php +++ b/core/modules/system/tests/src/Kernel/MenuAccessControlHandlerTest.php @@ -53,18 +53,10 @@ protected function setUp() { * @dataProvider testAccessProvider */ public function testAccess($which_user, $which_entity, $view_label_access_result, $view_access_result, $update_access_result, $delete_access_result, $create_access_result) { - // We must always create user 1, so that a "normal" user has a ID >1. - $root_user = $this->drupalCreateUser(); - - if ($which_user === 'user1') { - $user = $root_user; - } - else { - $permissions = ($which_user === 'admin') - ? ['administer menu'] - : []; - $user = $this->drupalCreateUser($permissions); - } + $permissions = ($which_user === 'admin') + ? ['administer menu'] + : []; + $user = $this->drupalCreateUser($permissions); $entity_values = ($which_entity === 'unlocked') ? ['locked' => FALSE] @@ -125,24 +117,6 @@ public function testAccessProvider() { AccessResult::forbidden()->addCacheTags(['config:system.menu.llama'])->setReason("The Menu config entity is locked."), AccessResult::allowed()->addCacheContexts(['user.permissions']), ], - 'user1 + unlocked' => [ - 'user1', - 'unlocked', - AccessResult::allowed(), - AccessResult::allowed()->addCacheContexts(['user.permissions']), - AccessResult::allowed()->addCacheContexts(['user.permissions']), - AccessResult::allowed()->addCacheContexts(['user.permissions'])->addCacheTags(['config:system.menu.llama']), - AccessResult::allowed()->addCacheContexts(['user.permissions']), - ], - 'user1 + locked' => [ - 'user1', - 'locked', - AccessResult::allowed(), - AccessResult::allowed()->addCacheContexts(['user.permissions']), - AccessResult::allowed()->addCacheContexts(['user.permissions']), - AccessResult::forbidden()->addCacheTags(['config:system.menu.llama'])->setReason("The Menu config entity is locked."), - AccessResult::allowed()->addCacheContexts(['user.permissions']), - ], ]; } diff --git a/core/modules/user/config/install/user.role.administrator.yml b/core/modules/user/config/install/user.role.administrator.yml new file mode 100644 index 0000000..e5453b7 --- /dev/null +++ b/core/modules/user/config/install/user.role.administrator.yml @@ -0,0 +1,8 @@ +langcode: en +status: true +dependencies: { } +id: administrator +label: Administrator +weight: 2 +is_admin: true +permissions: { } diff --git a/core/modules/user/src/AccountSettingsForm.php b/core/modules/user/src/AccountSettingsForm.php index bd80dd7..b12360b 100644 --- a/core/modules/user/src/AccountSettingsForm.php +++ b/core/modules/user/src/AccountSettingsForm.php @@ -100,34 +100,6 @@ public function buildForm(array $form, FormStateInterface $form_state) { '#required' => TRUE, ]; - // Administrative role option. - $form['admin_role'] = [ - '#type' => 'details', - '#title' => $this->t('Administrator role'), - '#open' => TRUE, - ]; - // Do not allow users to set the anonymous or authenticated user roles as the - // administrator role. - $roles = user_role_names(TRUE); - unset($roles[RoleInterface::AUTHENTICATED_ID]); - - $admin_roles = $this->roleStorage->getQuery() - ->condition('is_admin', TRUE) - ->execute(); - $default_value = reset($admin_roles); - - $form['admin_role']['user_admin_role'] = [ - '#type' => 'select', - '#title' => $this->t('Administrator role'), - '#empty_value' => '', - '#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. if ($this->moduleHandler->moduleExists('content_translation')) { $form['language'] = [ @@ -460,22 +432,6 @@ public function submitForm(array &$form, FormStateInterface $form_state) { $this->config('system.site') ->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(); - } - } } } diff --git a/core/modules/user/src/Entity/User.php b/core/modules/user/src/Entity/User.php index f254ccf..2eff477 100644 --- a/core/modules/user/src/Entity/User.php +++ b/core/modules/user/src/Entity/User.php @@ -82,11 +82,14 @@ public function preSave(EntityStorageInterface $storage) { parent::preSave($storage); // Make sure that the authenticated/anonymous roles are not persisted. - foreach ($this->get('roles') as $index => $item) { - if (in_array($item->target_id, [RoleInterface::ANONYMOUS_ID, RoleInterface::AUTHENTICATED_ID])) { - $this->get('roles')->offsetUnset($index); + $internal = [RoleInterface::ANONYMOUS_ID, RoleInterface::AUTHENTICATED_ID]; + $role_refs = $this->get('roles')->getValue(); + foreach ($role_refs as $index => $role_ref) { + if (in_array($role_ref['target_id'], $internal)) { + unset($role_refs[$index]); } } + $this->get('roles')->setValue($role_refs); // Store account cancellation information. foreach (['user_cancel_method', 'user_cancel_notify'] as $key) { @@ -194,11 +197,6 @@ public function removeRole($rid) { * {@inheritdoc} */ public function hasPermission($permission) { - // User #1 has all privileges. - if ((int) $this->id() === 1) { - return TRUE; - } - return $this->getRoleStorage()->isPermissionInRoles($permission, $this->getRoles()); } diff --git a/core/modules/user/src/RoleAccessControlHandler.php b/core/modules/user/src/RoleAccessControlHandler.php index e979f2d..f784a31 100644 --- a/core/modules/user/src/RoleAccessControlHandler.php +++ b/core/modules/user/src/RoleAccessControlHandler.php @@ -20,7 +20,12 @@ class RoleAccessControlHandler extends EntityAccessControlHandler { protected function checkAccess(EntityInterface $entity, $operation, AccountInterface $account) { switch ($operation) { case 'delete': - if ($entity->id() == RoleInterface::ANONYMOUS_ID || $entity->id() == RoleInterface::AUTHENTICATED_ID) { + $internal_roles = [ + RoleInterface::ANONYMOUS_ID, + RoleInterface::AUTHENTICATED_ID, + RoleInterface::ADMINISTRATOR_ID, + ]; + if (in_array($entity->id(), $internal_roles)) { return AccessResult::forbidden(); } diff --git a/core/modules/user/src/RoleInterface.php b/core/modules/user/src/RoleInterface.php index 4633637..15b388e 100644 --- a/core/modules/user/src/RoleInterface.php +++ b/core/modules/user/src/RoleInterface.php @@ -23,6 +23,11 @@ const AUTHENTICATED_ID = AccountInterface::AUTHENTICATED_ROLE; /** + * Role ID for the admin role; should match what's in the "role" table. + */ + const ADMINISTRATOR_ID = AccountInterface::ADMINISTRATOR_ROLE; + + /** * Returns a list of permissions assigned to the role. * * @return array diff --git a/core/modules/user/src/Tests/UserPermissionsTest.php b/core/modules/user/src/Tests/UserPermissionsTest.php index 4f9e260..2a54432 100644 --- a/core/modules/user/src/Tests/UserPermissionsTest.php +++ b/core/modules/user/src/Tests/UserPermissionsTest.php @@ -47,10 +47,6 @@ public function testUserPermissionChanges() { $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; @@ -90,49 +86,6 @@ public function testUserPermissionChanges() { } /** - * Test assigning of permissions for the administrator role. - */ - public function testAdministratorRole() { - $this->drupalLogin($this->adminUser); - $this->drupalGet('admin/config/people/accounts'); - - // 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 = []; - $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(['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 = []; - $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'); - } - - /** * Verify proper permission changes by user_role_change_permissions(). */ public function testUserRoleChangePermissions() { diff --git a/core/modules/user/tests/src/Functional/Update/UserUpdateUserOneAdminTest.php b/core/modules/user/tests/src/Functional/Update/UserUpdateUserOneAdminTest.php new file mode 100644 index 0000000..dd2171d --- /dev/null +++ b/core/modules/user/tests/src/Functional/Update/UserUpdateUserOneAdminTest.php @@ -0,0 +1,106 @@ +databaseDumpFiles = [ + __DIR__ . '/../../../../../system/tests/fixtures/update/drupal-8.bare.standard.php.gz', + ]; + } + + /** + * Tests that user 1 has the administrator role after the update. + */ + 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->userOneHasAdminRole()); + $user1 = User::load(1); + $user1->removeRole('administrator'); + $user1->save(); + $this->assertFalse($this->userOneHasAdminRole()); + + // 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->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('administrator'); + $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($this->getLegacyAdminRoleId()); + $this->assertEqual(['access content'], $legacy_admin_role->getPermissions(), 'Legacy admin role has the right permissions.'); + } + + /** + * Checks whether user 1 has the administrator role. + * + * @return bool + * Whether user 1 has the administrator role. + */ + protected function userOneHasAdminRole() { + $user1 = User::load(1); + return in_array('administrator', $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($this->getLegacyAdminRoleId(), $user1->getRoles(), TRUE); + } + + /** + * Retrieves the legacy administrator role ID. + * + * By default this is administrator_legacy but in the rare event of a naming + * collision, people may choose an ID themselves by setting it to the + * user_update_8101_legacy_admin_role setting in settings.php. + * + * @return string + * The legacy administrator role ID. + */ + protected function getLegacyAdminRoleId() { + return Settings::get('user_update_8101_legacy_admin_role', 'administrator_legacy'); + } + +} diff --git a/core/modules/user/tests/src/Kernel/UserEntityTest.php b/core/modules/user/tests/src/Kernel/UserEntityTest.php index 73fe869..42f053b 100644 --- a/core/modules/user/tests/src/Kernel/UserEntityTest.php +++ b/core/modules/user/tests/src/Kernel/UserEntityTest.php @@ -42,10 +42,16 @@ public function testUserMethods() { $role_storage->create(['id' => 'test_role_two'])->save(); $role_storage->create(['id' => 'test_role_three'])->save(); - $values = [ - 'uid' => 1, - 'roles' => ['test_role_one'], - ]; + // Test that user 1 is no different than any regular authenticated user. + $user = User::create(['uid' => 1]); + $this->assertEqual([RoleInterface::AUTHENTICATED_ID], $user->getRoles()); + + // Test that a regular authenticated user gets the authenticated role. + $user = User::create(['uid' => 2]); + $this->assertEqual([RoleInterface::AUTHENTICATED_ID], $user->getRoles()); + + // Test role manipulation for an authenticated user. + $values = ['uid' => 3, 'roles' => ['test_role_one']]; $user = User::create($values); $this->assertTrue($user->hasRole('test_role_one')); diff --git a/core/modules/user/tests/src/Kernel/UserInstallTest.php b/core/modules/user/tests/src/Kernel/UserInstallTest.php index f61811b..2228607 100644 --- a/core/modules/user/tests/src/Kernel/UserInstallTest.php +++ b/core/modules/user/tests/src/Kernel/UserInstallTest.php @@ -2,6 +2,7 @@ namespace Drupal\Tests\user\Kernel; +use Drupal\Core\Session\AccountInterface; use Drupal\KernelTests\KernelTestBase; /** @@ -49,6 +50,10 @@ public function testUserInstall() { $this->assertEqual($admin->status, 1); // Test that the anonymous user is blocked. $this->assertEqual($anon->status, 0); + + // Verify that UID 1 gets the administrator role. + $rids = db_query('SELECT roles_target_id FROM {user__roles} WHERE entity_id = 1')->fetchCol(); + $this->assertEquals([AccountInterface::ADMINISTRATOR_ROLE], $rids, 'Admin user has administrator role'); } } diff --git a/core/modules/user/tests/src/Kernel/Views/UserViewsFieldAccessTest.php b/core/modules/user/tests/src/Kernel/Views/UserViewsFieldAccessTest.php index f14779e..c17c3dc 100644 --- a/core/modules/user/tests/src/Kernel/Views/UserViewsFieldAccessTest.php +++ b/core/modules/user/tests/src/Kernel/Views/UserViewsFieldAccessTest.php @@ -2,7 +2,6 @@ namespace Drupal\Tests\user\Kernel\Views; -use Drupal\language\Entity\ConfigurableLanguage; use Drupal\user\Entity\User; use Drupal\Tests\views\Kernel\Handler\FieldFieldAccessTestBase; @@ -28,22 +27,9 @@ protected function setUp($import_test_views = TRUE) { } public function testUserFields() { - ConfigurableLanguage::create([ - 'id' => 'es', - 'name' => 'Spanish', - ])->save(); - ConfigurableLanguage::create([ - 'id' => 'fr', - 'name' => 'French', - ])->save(); - $user = User::create([ 'name' => 'test user', - 'mail' => 'druplicon@drop.org', 'status' => 1, - 'preferred_langcode' => 'es', - 'preferred_admin_langcode' => 'fr', - 'timezone' => 'ut1', 'created' => 123456, ]); @@ -54,12 +40,7 @@ public function testUserFields() { $this->assertFieldAccess('user', 'uid', $user->id()); $this->assertFieldAccess('user', 'uuid', $user->uuid()); $this->assertFieldAccess('user', 'langcode', $user->language()->getName()); - $this->assertFieldAccess('user', 'preferred_langcode', 'Spanish'); - $this->assertFieldAccess('user', 'preferred_admin_langcode', 'French'); $this->assertFieldAccess('user', 'name', 'test user'); - // $this->assertFieldAccess('user', 'mail', 'druplicon@drop.org'); - $this->assertFieldAccess('user', 'timezone', 'ut1'); - $this->assertFieldAccess('user', 'status', 'On'); // $this->assertFieldAccess('user', 'created', \Drupal::service('date.formatter')->format(123456)); // $this->assertFieldAccess('user', 'changed', \Drupal::service('date.formatter')->format(REQUEST_TIME)); } diff --git a/core/modules/user/user.install b/core/modules/user/user.install index 0af797a..014fe1c 100644 --- a/core/modules/user/user.install +++ b/core/modules/user/user.install @@ -5,6 +5,11 @@ * Install, update and uninstall functions for the user module. */ +use Drupal\Core\Config\ExtensionInstallStorage; +use Drupal\Core\Config\InstallStorage; +use Drupal\Core\Session\AccountInterface; +use Drupal\Core\Site\Settings; + /** * Implements hook_schema(). */ @@ -82,6 +87,7 @@ function user_install() { 'name' => 'placeholder-for-uid-1', 'mail' => 'placeholder-for-uid-1', 'status' => TRUE, + 'roles' => [AccountInterface::ADMINISTRATOR_ROLE], ]) ->save(); } @@ -98,3 +104,35 @@ function user_update_8100() { $config->set('status_blocked', $mail)->save(TRUE); } } + +/** + * Ensure the presence of the administrator role. + */ +function user_update_8101() { + $config_factory = \Drupal::configFactory(); + $config_name = 'user.role.administrator'; + $role = $config_factory->getEditable($config_name); + + // 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. + $legacy_admin_role = Settings::get('user_update_8101_legacy_admin_role', 'administrator_legacy'); + if (!$role->isNew() && !$role->get('is_admin')) { + $legacy_role = $config_factory->getEditable('user.role.' . $legacy_admin_role); + $legacy_role->setData($role->getRawData()); + $legacy_role->set('id', $legacy_admin_role); + $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 + ); + $role->setData($yaml_storage->read($config_name)); + $role->save(TRUE); + } +} diff --git a/core/modules/user/user.post_update.php b/core/modules/user/user.post_update.php index 3f19b0c..1aa3f84 100644 --- a/core/modules/user/user.post_update.php +++ b/core/modules/user/user.post_update.php @@ -5,7 +5,9 @@ * Post update functions for User module. */ +use Drupal\Core\Site\Settings; use Drupal\user\Entity\Role; +use Drupal\user\Entity\User; /** * Enforce order of role permissions. @@ -20,3 +22,30 @@ function user_post_update_enforce_order_of_permissions() { }; array_map($entity_save, Role::loadMultiple()); } + +/** + * Grant user 1 the administrator role and potentially update legacy admins. + */ +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. + $legacy_admin_role = Settings::get('user_update_8101_legacy_admin_role', 'administrator_legacy'); + if ($role = Role::load($legacy_admin_role)) { + $uids = Drupal::entityQuery('user') + ->condition('roles', 'administrator') + ->execute(); + + if (!empty($uids)) { + /** @var \Drupal\user\UserInterface $account */ + foreach (User::loadMultiple($uids) as $account) { + $account->removeRole('administrator'); + $account->addRole($legacy_admin_role); + $account->save(); + } + } + } + + $account = User::load(1); + $account->addRole('administrator'); + $account->save(); +} diff --git a/core/modules/views/tests/src/Kernel/Handler/FieldFieldAccessTestBase.php b/core/modules/views/tests/src/Kernel/Handler/FieldFieldAccessTestBase.php index 5ff77bc..d5617b5 100644 --- a/core/modules/views/tests/src/Kernel/Handler/FieldFieldAccessTestBase.php +++ b/core/modules/views/tests/src/Kernel/Handler/FieldFieldAccessTestBase.php @@ -42,7 +42,7 @@ protected function setUp($import_test_views = TRUE) { $role_with_access = Role::create([ 'id' => 'with_access', - 'permissions' => ['view test entity field'], + 'permissions' => ['view test entity field', 'access content'], ]); $role_with_access->save(); $role_without_access = Role::create([ diff --git a/core/modules/views/tests/src/Kernel/Handler/FieldFieldTest.php b/core/modules/views/tests/src/Kernel/Handler/FieldFieldTest.php index 1d304f5..8cc940b 100644 --- a/core/modules/views/tests/src/Kernel/Handler/FieldFieldTest.php +++ b/core/modules/views/tests/src/Kernel/Handler/FieldFieldTest.php @@ -7,9 +7,10 @@ use Drupal\entity_test\Entity\EntityTestRev; use Drupal\field\Entity\FieldConfig; use Drupal\field\Entity\FieldStorageConfig; +use Drupal\Tests\views\Kernel\ViewsKernelTestBase; use Drupal\user\Entity\User; +use Drupal\user\RoleInterface; use Drupal\views\Plugin\views\field\EntityField; -use Drupal\Tests\views\Kernel\ViewsKernelTestBase; use Drupal\views\Tests\ViewTestData; use Drupal\views\Views; @@ -66,6 +67,7 @@ protected function setUp($import_test_views = TRUE) { // First setup the needed entity types before installing the views. parent::setUp(FALSE); + $this->installConfig('user'); $this->installEntitySchema('user'); $this->installEntitySchema('entity_test'); $this->installEntitySchema('entity_test_rev'); @@ -73,7 +75,7 @@ protected function setUp($import_test_views = TRUE) { ViewTestData::createTestViews(get_class($this), ['views_test_config']); // Bypass any field access. - $this->adminUser = User::create(['name' => $this->randomString()]); + $this->adminUser = User::create(['name' => $this->randomString(), 'roles' => [RoleInterface::ADMINISTRATOR_ID]]); $this->adminUser->save(); $this->container->get('current_user')->setAccount($this->adminUser); diff --git a/core/modules/views/tests/src/Kernel/Handler/FieldRenderedEntityTest.php b/core/modules/views/tests/src/Kernel/Handler/FieldRenderedEntityTest.php index 02419f4..c70941e 100644 --- a/core/modules/views/tests/src/Kernel/Handler/FieldRenderedEntityTest.php +++ b/core/modules/views/tests/src/Kernel/Handler/FieldRenderedEntityTest.php @@ -6,6 +6,7 @@ use Drupal\entity_test\Entity\EntityTest; use Drupal\field\Entity\FieldConfig; use Drupal\field\Entity\FieldStorageConfig; +use Drupal\user\Entity\Role; use Drupal\user\Entity\User; use Drupal\views\Entity\View; use Drupal\Tests\views\Kernel\ViewsKernelTestBase; @@ -87,10 +88,17 @@ protected function setUpFixtures() { ])->save(); } + Role::create([ + 'id' => 'test_role', + 'label' => 'Can view test entities', + 'permissions' => ['view test entity'], + ])->save(); $this->user = User::create([ 'name' => 'test user', + 'roles' => ['test_role'], ]); $this->user->save(); + \Drupal::currentUser()->setAccount($this->user); parent::setUpFixtures(); } @@ -99,8 +107,6 @@ protected function setUpFixtures() { * Tests the default rendered entity output. */ public function testRenderedEntityWithoutField() { - \Drupal::currentUser()->setAccount($this->user); - EntityViewDisplay::load('entity_test.entity_test.foobar') ->removeComponent('test_field') ->save(); @@ -167,8 +173,6 @@ protected function assertConfigDependencies(View $storage) { * Tests the rendered entity output with the test field configured to show. */ public function testRenderedEntityWithField() { - \Drupal::currentUser()->setAccount($this->user); - // Show the test_field on the entity_test.entity_test.foobar view display. EntityViewDisplay::load('entity_test.entity_test.foobar')->setComponent('test_field', ['type' => 'string', 'label' => 'above'])->save(); diff --git a/core/modules/views/tests/src/Kernel/Plugin/RowRenderCacheTest.php b/core/modules/views/tests/src/Kernel/Plugin/RowRenderCacheTest.php index 223963e..6be14a4 100644 --- a/core/modules/views/tests/src/Kernel/Plugin/RowRenderCacheTest.php +++ b/core/modules/views/tests/src/Kernel/Plugin/RowRenderCacheTest.php @@ -188,11 +188,10 @@ protected function doTestRenderedOutput(AccountInterface $account, $check_cache if ($check_cache) { $keys = $cache_plugin->getRowCacheKeys($view->result[$index]); - $user_context = !$account->hasPermission('edit any test content') ? 'user' : 'user.permissions'; $cache = [ '#cache' => [ 'keys' => $keys, - 'contexts' => ['languages:language_interface', 'theme', $user_context], + 'contexts' => ['languages:language_interface', 'theme', 'user.permissions'], ], ]; $element = $render_cache->get($cache); diff --git a/core/profiles/standard/config/install/user.role.administrator.yml b/core/profiles/standard/config/install/user.role.administrator.yml deleted file mode 100644 index e5453b7..0000000 --- a/core/profiles/standard/config/install/user.role.administrator.yml +++ /dev/null @@ -1,8 +0,0 @@ -langcode: en -status: true -dependencies: { } -id: administrator -label: Administrator -weight: 2 -is_admin: true -permissions: { } diff --git a/core/profiles/standard/standard.install b/core/profiles/standard/standard.install index 2f567ab..f0be314 100644 --- a/core/profiles/standard/standard.install +++ b/core/profiles/standard/standard.install @@ -5,7 +5,6 @@ * Install, update and uninstall functions for the standard installation profile. */ -use Drupal\user\Entity\User; use Drupal\user\RoleInterface; use Drupal\shortcut\Entity\Shortcut; @@ -28,11 +27,6 @@ function standard_install() { user_role_grant_permissions(RoleInterface::ANONYMOUS_ID, ['access comments']); user_role_grant_permissions(RoleInterface::AUTHENTICATED_ID, ['access comments', 'post comments', 'skip comment approval']); - // Assign user 1 the "administrator" role. - $user = User::load(1); - $user->roles[] = 'administrator'; - $user->save(); - // We install some menu links, so we have to rebuild the router, to ensure the // menu links are valid. \Drupal::service('router.builder')->rebuildIfNeeded(); diff --git a/core/tests/Drupal/KernelTests/Core/Entity/Element/EntityAutocompleteElementFormTest.php b/core/tests/Drupal/KernelTests/Core/Entity/Element/EntityAutocompleteElementFormTest.php index c18910e..1b69f72 100644 --- a/core/tests/Drupal/KernelTests/Core/Entity/Element/EntityAutocompleteElementFormTest.php +++ b/core/tests/Drupal/KernelTests/Core/Entity/Element/EntityAutocompleteElementFormTest.php @@ -10,6 +10,7 @@ use Drupal\entity_test\Entity\EntityTest; use Drupal\entity_test\Entity\EntityTestStringId; use Drupal\KernelTests\Core\Entity\EntityKernelTestBase; +use Drupal\user\Entity\Role; use Drupal\user\Entity\User; /** @@ -50,9 +51,15 @@ protected function setUp() { $this->installEntitySchema('entity_test_string_id'); \Drupal::service('router.builder')->rebuild(); + Role::create([ + 'id' => 'test_role', + 'label' => 'Can view test entities', + 'permissions' => ['view test entity'], + ])->save(); $this->testUser = User::create([ 'name' => 'foobar1', 'mail' => 'foobar1@example.com', + 'roles' => ['test_role'], ]); $this->testUser->save(); \Drupal::service('current_user')->setAccount($this->testUser); diff --git a/core/tests/Drupal/KernelTests/Core/Render/RenderCacheTest.php b/core/tests/Drupal/KernelTests/Core/Render/RenderCacheTest.php index 91061ca..a8715db 100644 --- a/core/tests/Drupal/KernelTests/Core/Render/RenderCacheTest.php +++ b/core/tests/Drupal/KernelTests/Core/Render/RenderCacheTest.php @@ -32,63 +32,52 @@ protected function setUp() { } /** - * Tests that user 1 has a different permission context with the same roles. + * Check the render cache for the user.permissions context. */ - public function testUser1PermissionContext() { - $this->doTestUser1WithContexts(['user.permissions']); + public function testPermissionContext() { + $this->doTestWithContexts(['user.permissions']); } /** - * Tests that user 1 has a different roles context with the same roles. + * Check the render cache for the user.roles context. */ - public function testUser1RolesContext() { - $this->doTestUser1WithContexts(['user.roles']); + public function testRolesContext() { + $this->doTestWithContexts(['user.roles']); } /** - * Ensures that user 1 has a unique render cache for the given context. + * Checks the functionality of the render cache for the given context. * * @param string[] $contexts * List of cache contexts to use. */ - protected function doTestUser1WithContexts($contexts) { - // Test that user 1 does not share the cache with other users who have the - // same roles, even when using a role-based cache context. - $user1 = $this->createUser(); - $this->assertEqual($user1->id(), 1); + protected function doTestWithContexts($contexts) { + // Set up two authenticated users and an admin user so we can test the + // output of the render cache for them. $first_authenticated_user = $this->createUser(); $second_authenticated_user = $this->createUser(); $admin_user = $this->createUser([], NULL, TRUE); - $this->assertEqual($user1->getRoles(), $first_authenticated_user->getRoles(), 'User 1 has the same roles as an authenticated user.'); - // Impersonate user 1 and render content that only user 1 should have - // permission to see. - \Drupal::service('account_switcher')->switchTo($user1); + // Set up a test element we will reuse below. $test_element = [ '#cache' => [ 'keys' => ['test'], 'contexts' => $contexts, ], ]; + + // Render content that only authenticated users should see. + \Drupal::service('account_switcher')->switchTo($first_authenticated_user); $element = $test_element; - $element['#markup'] = 'content for user 1'; + $element['#markup'] = 'content for authenticated users'; $output = \Drupal::service('renderer')->renderRoot($element); - $this->assertEqual($output, 'content for user 1'); + $this->assertEqual($output, 'content for authenticated users'); // Verify the cache is working by rendering the same element but with // different markup passed in; the result should be the same. $element = $test_element; $element['#markup'] = 'should not be used'; $output = \Drupal::service('renderer')->renderRoot($element); - $this->assertEqual($output, 'content for user 1'); - \Drupal::service('account_switcher')->switchBack(); - - // Verify that the first authenticated user does not see the same content - // as user 1. - \Drupal::service('account_switcher')->switchTo($first_authenticated_user); - $element = $test_element; - $element['#markup'] = 'content for authenticated users'; - $output = \Drupal::service('renderer')->renderRoot($element); $this->assertEqual($output, 'content for authenticated users'); \Drupal::service('account_switcher')->switchBack(); @@ -108,7 +97,6 @@ protected function doTestUser1WithContexts($contexts) { $element['#markup'] = 'content for admin user'; $output = \Drupal::service('renderer')->renderRoot($element); $this->assertEqual($output, 'content for admin user'); - \Drupal::service('account_switcher')->switchBack(); } } diff --git a/core/tests/Drupal/Tests/Core/Session/PermissionsHashGeneratorTest.php b/core/tests/Drupal/Tests/Core/Session/PermissionsHashGeneratorTest.php index aa980f8..1b71318 100644 --- a/core/tests/Drupal/Tests/Core/Session/PermissionsHashGeneratorTest.php +++ b/core/tests/Drupal/Tests/Core/Session/PermissionsHashGeneratorTest.php @@ -14,13 +14,20 @@ class PermissionsHashGeneratorTest extends UnitTestCase { /** - * The mocked super user account. + * The mocked admin account. * * @var \Drupal\user\UserInterface|\PHPUnit_Framework_MockObject_MockObject */ protected $account1; /** + * An "updated" admin account. + * + * @var \Drupal\user\UserInterface|\PHPUnit_Framework_MockObject_MockObject + */ + protected $account1Updated; + + /** * A mocked account. * * @var \Drupal\user\UserInterface|\PHPUnit_Framework_MockObject_MockObject @@ -63,6 +70,13 @@ class PermissionsHashGeneratorTest extends UnitTestCase { protected $staticCache; /** + * The mocked entity type manager. + * + * @var \Drupal\Core\Entity\EntityTypeManagerInterface|\PHPUnit_Framework_MockObject_MockObject + */ + protected $entityTypeManager; + + /** * The permission hash class being tested. * * @var \Drupal\Core\Session\PermissionsHashGeneratorInterface @@ -77,57 +91,107 @@ protected function setUp() { new Settings(['hash_salt' => 'test']); - // The mocked super user account, with the same roles as Account 2. + // Mock an admin role. + $admin_role = $this->getMockBuilder('Drupal\user\Entity\Role') + ->disableOriginalConstructor() + ->setMethods(['isAdmin']) + ->getMock(); + $admin_role->expects($this->any()) + ->method('isAdmin') + ->will($this->returnValue(TRUE)); + + // Mock a regular role. + $regular_role = $this->getMockBuilder('Drupal\user\Entity\Role') + ->disableOriginalConstructor() + ->setMethods(['isAdmin']) + ->getMock(); + $regular_role->expects($this->any()) + ->method('isAdmin') + ->will($this->returnValue(FALSE)); + + // Account 1: 'authenticated' and 'administrator' role. + $roles_1 = [ + 'authenticated' => $regular_role, + 'administrator' => $admin_role, + ]; $this->account1 = $this->getMockBuilder('Drupal\user\Entity\User') ->disableOriginalConstructor() ->setMethods(['getRoles', 'id']) ->getMock(); $this->account1->expects($this->any()) + ->method('getRoles') + ->will($this->returnValue(array_keys($roles_1))); + $this->account1->expects($this->any()) ->method('id') ->willReturn(1); - $this->account1->expects($this->never()) - ->method('getRoles'); - // Account 2: 'administrator' and 'authenticated' roles. - $roles_1 = ['administrator', 'authenticated']; + // Account 2: 'editor' and 'authenticated' roles. + $roles_2 = [ + 'editor' => $regular_role, + 'authenticated' => $regular_role, + ]; $this->account2 = $this->getMockBuilder('Drupal\user\Entity\User') ->disableOriginalConstructor() ->setMethods(['getRoles', 'id']) ->getMock(); $this->account2->expects($this->any()) ->method('getRoles') - ->will($this->returnValue($roles_1)); + ->will($this->returnValue(array_keys($roles_2))); $this->account2->expects($this->any()) ->method('id') ->willReturn(2); - // Account 3: 'authenticated' and 'administrator' roles (different order). - $roles_3 = ['authenticated', 'administrator']; + // Account 3: 'authenticated' and 'editor' roles (different order). + $roles_3 = [ + 'authenticated' => $regular_role, + 'editor' => $regular_role, + ]; $this->account3 = $this->getMockBuilder('Drupal\user\Entity\User') ->disableOriginalConstructor() ->setMethods(['getRoles', 'id']) ->getMock(); $this->account3->expects($this->any()) ->method('getRoles') - ->will($this->returnValue($roles_3)); + ->will($this->returnValue(array_keys($roles_3))); $this->account3->expects($this->any()) ->method('id') ->willReturn(3); - // Updated account 2: now also 'editor' role. - $roles_2_updated = ['editor', 'administrator', 'authenticated']; + // Updated account 1: now also 'publisher' role. + $roles_1_updated = [ + 'authenticated' => $regular_role, + 'administrator' => $admin_role, + 'publisher' => $regular_role, + ]; + $this->account1Updated = $this->getMockBuilder('Drupal\user\Entity\User') + ->disableOriginalConstructor() + ->setMethods(['getRoles', 'id']) + ->getMock(); + $this->account1Updated->expects($this->any()) + ->method('getRoles') + ->will($this->returnValue(array_keys($roles_1_updated))); + $this->account1Updated->expects($this->any()) + ->method('id') + ->willReturn(1); + + // Updated account 2: now also 'publisher' role. + $roles_2_updated = [ + 'editor' => $regular_role, + 'authenticated' => $regular_role, + 'publisher' => $regular_role, + ]; $this->account2Updated = $this->getMockBuilder('Drupal\user\Entity\User') ->disableOriginalConstructor() ->setMethods(['getRoles', 'id']) ->getMock(); $this->account2Updated->expects($this->any()) ->method('getRoles') - ->will($this->returnValue($roles_2_updated)); + ->will($this->returnValue(array_keys($roles_2_updated))); $this->account2Updated->expects($this->any()) ->method('id') ->willReturn(2); - // Mocked private key + cache services. + // Mocked private key, cache, static cache and entity type manager services. $random = Crypt::randomBytesBase64(55); $this->privateKey = $this->getMockBuilder('Drupal\Core\PrivateKey') ->disableOriginalConstructor() @@ -142,25 +206,48 @@ protected function setUp() { $this->staticCache = $this->getMockBuilder('Drupal\Core\Cache\CacheBackendInterface') ->disableOriginalConstructor() ->getMock(); + $this->entityTypeManager = $this->getMockBuilder('Drupal\Core\Entity\EntityTypeManagerInterface') + ->disableOriginalConstructor() + ->getMock(); - $this->permissionsHash = new PermissionsHashGenerator($this->privateKey, $this->cache, $this->staticCache); + // Mock the role storage and the return values we care about. + $role_storage = $this->getMockBuilder('Drupal\Core\Entity\EntityStorageInterface') + ->disableOriginalConstructor() + ->getMock(); + $role_storage->expects($this->any()) + ->method('loadMultiple') + ->willReturnMap([ + [$this->account1->getRoles(), $roles_1], + [$this->account2->getRoles(), $roles_2], + [$this->account3->getRoles(), $roles_3], + [$this->account1Updated->getRoles(), $roles_1_updated], + [$this->account2Updated->getRoles(), $roles_2_updated], + ]); + + // Set the role storage on the entity type manager. + $this->entityTypeManager->expects($this->any()) + ->method('getStorage') + ->with('user_role') + ->will($this->returnValue($role_storage)); + + $this->permissionsHash = new PermissionsHashGenerator($this->privateKey, $this->cache, $this->staticCache, $this->entityTypeManager); } /** * @covers ::generate */ public function testGenerate() { - // Ensure that the super user (user 1) always gets the same hash. - $super_user_hash = $this->permissionsHash->generate($this->account1); + // Ensure that admin accounts always gets the same hash. + $admin_hash = $this->permissionsHash->generate($this->account1); + $updated_admin_hash = $this->permissionsHash->generate($this->account1Updated); + $this->assertSame($admin_hash, $updated_admin_hash, 'Admin user with updated roles generates same permissions hash.'); // Ensure that two user accounts with the same roles generate the same hash. $hash_2 = $this->permissionsHash->generate($this->account2); $hash_3 = $this->permissionsHash->generate($this->account3); $this->assertSame($hash_2, $hash_3, 'Different users with the same roles generate the same permissions hash.'); - $this->assertNotSame($hash_2, $super_user_hash, 'User 1 has a different hash despite having the same roles'); - - // Compare with hash for user account 1 with an additional role. + // Compare with hash for user account 2 with an additional role. $updated_hash_2 = $this->permissionsHash->generate($this->account2Updated); $this->assertNotSame($hash_2, $updated_hash_2, 'Same user with updated roles generates different permissions hash.'); } @@ -170,7 +257,7 @@ public function testGenerate() { */ public function testGeneratePersistentCache() { // Set expectations for the mocked cache backend. - $expected_cid = 'user_permissions_hash:administrator,authenticated'; + $expected_cid = 'user_permissions_hash:authenticated,editor'; $mock_cache = new \stdClass(); $mock_cache->data = 'test_hash_here'; @@ -198,7 +285,7 @@ public function testGeneratePersistentCache() { */ public function testGenerateStaticCache() { // Set expectations for the mocked cache backend. - $expected_cid = 'user_permissions_hash:administrator,authenticated'; + $expected_cid = 'user_permissions_hash:authenticated,editor'; $mock_cache = new \stdClass(); $mock_cache->data = 'test_hash_here'; @@ -223,7 +310,7 @@ public function testGenerateStaticCache() { */ public function testGenerateNoCache() { // Set expectations for the mocked cache backend. - $expected_cid = 'user_permissions_hash:administrator,authenticated'; + $expected_cid = 'user_permissions_hash:authenticated,editor'; $this->staticCache->expects($this->once()) ->method('get') -- 2.8.1