From 4ed57d454b343c81dec1c3106e942fef85d8ee46 Mon Sep 17 00:00:00 2001 From: Alex Pott Date: Tue, 30 Apr 2024 08:17:36 +0100 Subject: [PATCH] Issue #3439907 by sukr_s, smustgrave, vensires, alexpott: Fix System tests that rely on UID1's super user behavior (cherry picked from commit 8aeebfb32ed2ba4ff99f624ee953d3b2f71e3fc4) --- .../Bootstrap/DrupalMessengerServiceTest.php | 11 +---- .../EntityReferenceFieldCreationTest.php | 14 ++---- .../File/FileSaveHtaccessLoggingTest.php | 11 +---- .../src/Functional/Menu/LocalTasksTest.php | 26 ++++++---- .../src/Functional/Module/ClassLoaderTest.php | 15 ++---- .../Module/GenericModuleTestBase.php | 10 +--- .../System/DateFormatsLockedTest.php | 10 +--- .../MaintenanceThemeUpdateRegistryTest.php | 11 +---- .../UpdateSystem/UpdateScriptTest.php | 19 +++---- .../DateFormatAccessControlHandlerTest.php | 48 +++--------------- .../EntityReferenceSelectionAccessTest.php | 11 ++--- .../Kernel/MenuAccessControlHandlerTest.php | 49 +++---------------- 12 files changed, 58 insertions(+), 177 deletions(-) diff --git a/core/modules/system/tests/src/Functional/Bootstrap/DrupalMessengerServiceTest.php b/core/modules/system/tests/src/Functional/Bootstrap/DrupalMessengerServiceTest.php index f6ef52cdce8..8fdc4500a32 100644 --- a/core/modules/system/tests/src/Functional/Bootstrap/DrupalMessengerServiceTest.php +++ b/core/modules/system/tests/src/Functional/Bootstrap/DrupalMessengerServiceTest.php @@ -22,14 +22,6 @@ class DrupalMessengerServiceTest extends BrowserTestBase { */ protected static $modules = ['system_test']; - /** - * {@inheritdoc} - * - * @todo Remove and fix test to not rely on super user. - * @see https://www.drupal.org/project/drupal/issues/3437620 - */ - protected bool $usesSuperUserAccessPolicy = TRUE; - /** * {@inheritdoc} */ @@ -63,7 +55,8 @@ class DrupalMessengerServiceTest extends BrowserTestBase { // Ensure messages survive a container rebuild. $assert = $this->assertSession(); - $this->drupalLogin($this->rootUser); + $this->drupalLogin($this->drupalCreateUser(['administer modules'])); + $edit = []; $edit["modules[help][enable]"] = TRUE; $this->drupalGet('admin/modules'); diff --git a/core/modules/system/tests/src/Functional/Entity/EntityReferenceFieldCreationTest.php b/core/modules/system/tests/src/Functional/Entity/EntityReferenceFieldCreationTest.php index bf7384543ad..c2fb6ccc96e 100644 --- a/core/modules/system/tests/src/Functional/Entity/EntityReferenceFieldCreationTest.php +++ b/core/modules/system/tests/src/Functional/Entity/EntityReferenceFieldCreationTest.php @@ -23,14 +23,6 @@ class EntityReferenceFieldCreationTest extends BrowserTestBase { */ protected static $modules = ['entity_test', 'node', 'field_ui']; - /** - * {@inheritdoc} - * - * @todo Remove and fix test to not rely on super user. - * @see https://www.drupal.org/project/drupal/issues/3437620 - */ - protected bool $usesSuperUserAccessPolicy = TRUE; - /** * {@inheritdoc} */ @@ -40,8 +32,12 @@ class EntityReferenceFieldCreationTest extends BrowserTestBase { * Tests that entity reference fields cannot target entity types without IDs. */ public function testAddReferenceFieldTargetingEntityTypeWithoutId() { - $this->drupalLogin($this->rootUser); + $node_type = $this->drupalCreateContentType()->id(); + $this->drupalLogin($this->drupalCreateUser([ + 'administer content types', + 'administer node fields', + ])); // Entity types without an ID key should not be presented as options when // creating an entity reference field in the UI. diff --git a/core/modules/system/tests/src/Functional/File/FileSaveHtaccessLoggingTest.php b/core/modules/system/tests/src/Functional/File/FileSaveHtaccessLoggingTest.php index 1067d8d0b07..41a105e9613 100644 --- a/core/modules/system/tests/src/Functional/File/FileSaveHtaccessLoggingTest.php +++ b/core/modules/system/tests/src/Functional/File/FileSaveHtaccessLoggingTest.php @@ -19,14 +19,6 @@ class FileSaveHtaccessLoggingTest extends BrowserTestBase { */ protected static $modules = ['dblog']; - /** - * {@inheritdoc} - * - * @todo Remove and fix test to not rely on super user. - * @see https://www.drupal.org/project/drupal/issues/3437620 - */ - protected bool $usesSuperUserAccessPolicy = TRUE; - /** * {@inheritdoc} */ @@ -45,7 +37,8 @@ class FileSaveHtaccessLoggingTest extends BrowserTestBase { /** @var \Drupal\Core\File\HtaccessWriterInterface $htaccess */ $htaccess = \Drupal::service('file.htaccess_writer'); $this->assertFalse($htaccess->write($private, TRUE)); - $this->drupalLogin($this->rootUser); + $this->drupalLogin($this->drupalCreateUser(['access site reports'])); + $this->drupalGet('admin/reports/dblog'); $this->clickLink("Security warning: Couldn't write .htaccess file."); diff --git a/core/modules/system/tests/src/Functional/Menu/LocalTasksTest.php b/core/modules/system/tests/src/Functional/Menu/LocalTasksTest.php index e82ac3b50f7..9cc6263f0ee 100644 --- a/core/modules/system/tests/src/Functional/Menu/LocalTasksTest.php +++ b/core/modules/system/tests/src/Functional/Menu/LocalTasksTest.php @@ -24,14 +24,6 @@ class LocalTasksTest extends BrowserTestBase { */ protected static $modules = ['block', 'menu_test', 'entity_test', 'node']; - /** - * {@inheritdoc} - * - * @todo Remove and fix test to not rely on super user. - * @see https://www.drupal.org/project/drupal/issues/3437620 - */ - protected bool $usesSuperUserAccessPolicy = TRUE; - /** * {@inheritdoc} */ @@ -268,11 +260,16 @@ class LocalTasksTest extends BrowserTestBase { * Tests that local tasks blocks cache is invalidated correctly. */ public function testLocalTaskBlockCache() { - $this->drupalLogin($this->rootUser); + $this->drupalLogin($this->drupalCreateUser([ + 'administer content types', + 'administer permissions', + ])); $this->drupalCreateContentType(['type' => 'page']); // Only the Edit task. The block avoids showing a single tab. $this->drupalGet('/admin/config/people/accounts'); + // @@todo Add assertion here to check the page was actually visited. + // https://www.drupal.org/project/drupal/issues/3443748 $this->assertNoLocalTasks(); // Only the Edit and Manage permission tabs. @@ -284,6 +281,17 @@ class LocalTasksTest extends BrowserTestBase { // Field UI adds the usual Manage fields etc tabs. \Drupal::service('module_installer')->install(['field_ui']); + + $this->drupalLogin($this->drupalCreateUser([ + 'administer content types', + 'administer permissions', + 'administer account settings', + 'administer display modes', + 'administer node display', + 'administer node fields', + 'administer node form display', + ])); + $this->drupalGet('/admin/structure/types/manage/page'); $this->assertLocalTasks([ ['entity.node_type.edit_form', ['node_type' => 'page']], diff --git a/core/modules/system/tests/src/Functional/Module/ClassLoaderTest.php b/core/modules/system/tests/src/Functional/Module/ClassLoaderTest.php index f8bc0645057..122bbc1598d 100644 --- a/core/modules/system/tests/src/Functional/Module/ClassLoaderTest.php +++ b/core/modules/system/tests/src/Functional/Module/ClassLoaderTest.php @@ -14,14 +14,6 @@ use Drupal\Tests\BrowserTestBase; */ class ClassLoaderTest extends BrowserTestBase { - /** - * {@inheritdoc} - * - * @todo Remove and fix test to not rely on super user. - * @see https://www.drupal.org/project/drupal/issues/3437620 - */ - protected bool $usesSuperUserAccessPolicy = TRUE; - /** * The expected result from calling the module-provided class' method. * @@ -97,7 +89,8 @@ class ClassLoaderTest extends BrowserTestBase { * Ensures the negative caches in the class loader don't result in crashes. */ public function testMultipleModules() { - $this->drupalLogin($this->rootUser); + $this->drupalLogin($this->drupalCreateUser(['administer modules'])); + $edit = [ "modules[module_install_class_loader_test1][enable]" => TRUE, "modules[module_install_class_loader_test2][enable]" => TRUE, @@ -113,7 +106,9 @@ class ClassLoaderTest extends BrowserTestBase { */ public function testAutoloadFromModuleFile() { $this->assertFalse(defined('MODULE_AUTOLOAD_TEST_CONSTANT')); - $this->drupalLogin($this->rootUser); + // Create use with required permissions. + $this->drupalLogin($this->drupalCreateUser(['administer modules'])); + $edit = [ "modules[module_autoload_test][enable]" => TRUE, ]; diff --git a/core/modules/system/tests/src/Functional/Module/GenericModuleTestBase.php b/core/modules/system/tests/src/Functional/Module/GenericModuleTestBase.php index f17f81fa0d8..7b3754bc34c 100644 --- a/core/modules/system/tests/src/Functional/Module/GenericModuleTestBase.php +++ b/core/modules/system/tests/src/Functional/Module/GenericModuleTestBase.php @@ -19,14 +19,6 @@ abstract class GenericModuleTestBase extends BrowserTestBase { 'help', ]; - /** - * {@inheritdoc} - * - * @todo Remove and fix test to not rely on super user. - * @see https://www.drupal.org/project/drupal/issues/3437620 - */ - protected bool $usesSuperUserAccessPolicy = TRUE; - /** * {@inheritdoc} */ @@ -52,7 +44,7 @@ abstract class GenericModuleTestBase extends BrowserTestBase { if (!empty($info['required']) && !empty($info['hidden'])) { $this->markTestSkipped('Nothing to assert for hidden, required modules.'); } - $this->drupalLogin($this->rootUser); + $this->drupalLogin($this->createUser(['access help pages'])); $this->assertHookHelp($module); if (empty($info['required'])) { diff --git a/core/modules/system/tests/src/Functional/System/DateFormatsLockedTest.php b/core/modules/system/tests/src/Functional/System/DateFormatsLockedTest.php index 52da4d4fede..f061534ba72 100644 --- a/core/modules/system/tests/src/Functional/System/DateFormatsLockedTest.php +++ b/core/modules/system/tests/src/Functional/System/DateFormatsLockedTest.php @@ -13,14 +13,6 @@ use Drupal\Tests\BrowserTestBase; */ class DateFormatsLockedTest extends BrowserTestBase { - /** - * {@inheritdoc} - * - * @todo Remove and fix test to not rely on super user. - * @see https://www.drupal.org/project/drupal/issues/3437620 - */ - protected bool $usesSuperUserAccessPolicy = TRUE; - /** * {@inheritdoc} */ @@ -30,7 +22,7 @@ class DateFormatsLockedTest extends BrowserTestBase { * Tests attempts at listing, editing, and deleting locked date formats. */ public function testDateLocking() { - $this->drupalLogin($this->rootUser); + $this->drupalLogin($this->drupalCreateUser(['administer site configuration'])); // Locked date formats are not linked on the listing page, locked date // formats are clearly marked as such; unlocked formats are not marked as diff --git a/core/modules/system/tests/src/Functional/Theme/MaintenanceThemeUpdateRegistryTest.php b/core/modules/system/tests/src/Functional/Theme/MaintenanceThemeUpdateRegistryTest.php index 706b7501ac9..3a15c1c9cba 100644 --- a/core/modules/system/tests/src/Functional/Theme/MaintenanceThemeUpdateRegistryTest.php +++ b/core/modules/system/tests/src/Functional/Theme/MaintenanceThemeUpdateRegistryTest.php @@ -16,14 +16,6 @@ use Drupal\Tests\RequirementsPageTrait; class MaintenanceThemeUpdateRegistryTest extends BrowserTestBase { use RequirementsPageTrait; - /** - * {@inheritdoc} - * - * @todo Remove and fix test to not rely on super user. - * @see https://www.drupal.org/project/drupal/issues/3437620 - */ - protected bool $usesSuperUserAccessPolicy = TRUE; - /** * {@inheritdoc} */ @@ -74,7 +66,8 @@ class MaintenanceThemeUpdateRegistryTest extends BrowserTestBase { * Tests that after installing the profile there are no outstanding updates. */ public function testMaintenanceThemeUpdateRegistration() { - $this->drupalLogin($this->rootUser); + $this->drupalLogin($this->drupalCreateUser(['administer software updates'])); + $this->drupalGet('update.php/selection'); $this->updateRequirementsProblem(); $this->drupalGet('update.php/selection'); diff --git a/core/modules/system/tests/src/Functional/UpdateSystem/UpdateScriptTest.php b/core/modules/system/tests/src/Functional/UpdateSystem/UpdateScriptTest.php index c8f2faa703f..d0a10206585 100644 --- a/core/modules/system/tests/src/Functional/UpdateSystem/UpdateScriptTest.php +++ b/core/modules/system/tests/src/Functional/UpdateSystem/UpdateScriptTest.php @@ -36,14 +36,6 @@ class UpdateScriptTest extends BrowserTestBase { 'test_another_module_required_by_theme', ]; - /** - * {@inheritdoc} - * - * @todo Remove and fix test to not rely on super user. - * @see https://www.drupal.org/project/drupal/issues/3437620 - */ - protected bool $usesSuperUserAccessPolicy = TRUE; - /** * {@inheritdoc} */ @@ -118,12 +110,17 @@ class UpdateScriptTest extends BrowserTestBase { $this->drupalGet('/update-script-test/database-updates-menu-item'); $this->assertSession()->linkExists('Run database updates'); - // Access the update page as user 1. - $this->drupalLogin($this->rootUser); + // Access the update page as administrator. + $this->drupalLogin($this->createUser([ + 'administer software updates', + 'access site in maintenance mode', + 'administer themes', + ])); $this->drupalGet($this->updateUrl, ['external' => TRUE]); $this->assertSession()->statusCodeEquals(200); - // Check that a link to the update page is accessible to user 1. + // Check that a link to the update page is accessible to users with proper + // permissions. $this->drupalGet('/update-script-test/database-updates-menu-item'); $this->assertSession()->linkExists('Run database updates'); } diff --git a/core/modules/system/tests/src/Kernel/DateFormatAccessControlHandlerTest.php b/core/modules/system/tests/src/Kernel/DateFormatAccessControlHandlerTest.php index 4ee11e85a46..2259e64b8e6 100644 --- a/core/modules/system/tests/src/Kernel/DateFormatAccessControlHandlerTest.php +++ b/core/modules/system/tests/src/Kernel/DateFormatAccessControlHandlerTest.php @@ -32,14 +32,6 @@ class DateFormatAccessControlHandlerTest extends KernelTestBase { 'user', ]; - /** - * {@inheritdoc} - * - * @todo Remove and fix test to not rely on super user. - * @see https://www.drupal.org/project/drupal/issues/3437620 - */ - protected bool $usesSuperUserAccessPolicy = TRUE; - /** * The date_format access control handler. * @@ -61,19 +53,9 @@ class DateFormatAccessControlHandlerTest extends KernelTestBase { * @covers ::checkCreateAccess * @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 an ID >1. - $root_user = $this->drupalCreateUser(); + public function testAccess($permissions, $which_entity, $view_label_access_result, $view_access_result, $update_access_result, $delete_access_result, $create_access_result) { - if ($which_user === 'user1') { - $user = $root_user; - } - else { - $permissions = ($which_user === 'admin') - ? ['administer site configuration'] - : []; - $user = $this->drupalCreateUser($permissions); - } + $user = $this->drupalCreateUser($permissions); $entity_values = ($which_entity === 'unlocked') ? ['locked' => FALSE] @@ -100,7 +82,7 @@ class DateFormatAccessControlHandlerTest extends KernelTestBase { return [ 'permissionless + unlocked' => [ - 'permissionless', + [], 'unlocked', AccessResult::allowed(), AccessResult::neutral()->addCacheContexts(['user.permissions'])->setReason("The 'administer site configuration' permission is required."), @@ -109,7 +91,7 @@ class DateFormatAccessControlHandlerTest extends KernelTestBase { AccessResult::neutral()->addCacheContexts(['user.permissions'])->setReason("The 'administer site configuration' permission is required."), ], 'permissionless + locked' => [ - 'permissionless', + [], 'locked', AccessResult::allowed(), AccessResult::neutral()->addCacheContexts(['user.permissions'])->setReason("The 'administer site configuration' permission is required."), @@ -118,7 +100,7 @@ class DateFormatAccessControlHandlerTest extends KernelTestBase { AccessResult::neutral()->addCacheContexts(['user.permissions'])->setReason("The 'administer site configuration' permission is required."), ], 'admin + unlocked' => [ - 'admin', + ['administer site configuration'], 'unlocked', AccessResult::allowed(), AccessResult::allowed()->addCacheContexts(['user.permissions']), @@ -127,25 +109,7 @@ class DateFormatAccessControlHandlerTest extends KernelTestBase { AccessResult::allowed()->addCacheContexts(['user.permissions']), ], 'admin + locked' => [ - 'admin', - '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']), - ], - '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', + ['administer site configuration'], 'locked', AccessResult::allowed(), AccessResult::allowed()->addCacheContexts(['user.permissions']), diff --git a/core/modules/system/tests/src/Kernel/Entity/EntityReferenceSelection/EntityReferenceSelectionAccessTest.php b/core/modules/system/tests/src/Kernel/Entity/EntityReferenceSelection/EntityReferenceSelectionAccessTest.php index 50e8f1a98b3..0e10ab05f78 100644 --- a/core/modules/system/tests/src/Kernel/Entity/EntityReferenceSelection/EntityReferenceSelectionAccessTest.php +++ b/core/modules/system/tests/src/Kernel/Entity/EntityReferenceSelection/EntityReferenceSelectionAccessTest.php @@ -50,14 +50,6 @@ class EntityReferenceSelectionAccessTest extends KernelTestBase { 'user', ]; - /** - * {@inheritdoc} - * - * @todo Remove and fix test to not rely on super user. - * @see https://www.drupal.org/project/drupal/issues/3437620 - */ - protected bool $usesSuperUserAccessPolicy = TRUE; - /** * {@inheritdoc} */ @@ -82,10 +74,13 @@ class EntityReferenceSelectionAccessTest extends KernelTestBase { 'name' => '', ]); $anonymous_user->save(); + + // Create role for administrator. $admin_user = User::create([ 'uid' => 1, 'name' => 'admin', 'status' => 1, + 'roles' => [$this->createRole(['administer users'])], ]); $admin_user->save(); } diff --git a/core/modules/system/tests/src/Kernel/MenuAccessControlHandlerTest.php b/core/modules/system/tests/src/Kernel/MenuAccessControlHandlerTest.php index 5eff6b76461..02e4b0db678 100644 --- a/core/modules/system/tests/src/Kernel/MenuAccessControlHandlerTest.php +++ b/core/modules/system/tests/src/Kernel/MenuAccessControlHandlerTest.php @@ -31,14 +31,6 @@ class MenuAccessControlHandlerTest extends KernelTestBase { 'user', ]; - /** - * {@inheritdoc} - * - * @todo Remove and fix test to not rely on super user. - * @see https://www.drupal.org/project/drupal/issues/3437620 - */ - protected bool $usesSuperUserAccessPolicy = TRUE; - /** * The menu access control handler. * @@ -60,19 +52,8 @@ class MenuAccessControlHandlerTest extends KernelTestBase { * @covers ::checkCreateAccess * @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 an ID >1. - $root_user = $this->drupalCreateUser(); - - if ($which_user === 'user1') { - $user = $root_user; - } - else { - $permissions = ($which_user === 'admin') - ? ['administer menu'] - : []; - $user = $this->drupalCreateUser($permissions); - } + public function testAccess($permissions, $which_entity, $view_label_access_result, $view_access_result, $update_access_result, $delete_access_result, $create_access_result) { + $user = $this->drupalCreateUser($permissions); $entity_values = ($which_entity === 'unlocked') ? ['locked' => FALSE] @@ -98,7 +79,7 @@ class MenuAccessControlHandlerTest extends KernelTestBase { return [ 'permissionless + unlocked' => [ - 'permissionless', + [], 'unlocked', AccessResult::allowed(), AccessResult::neutral()->addCacheContexts(['user.permissions'])->setReason("The 'administer menu' permission is required."), @@ -107,7 +88,7 @@ class MenuAccessControlHandlerTest extends KernelTestBase { AccessResult::neutral()->addCacheContexts(['user.permissions'])->setReason("The 'administer menu' permission is required."), ], 'permissionless + locked' => [ - 'permissionless', + [], 'locked', AccessResult::allowed(), AccessResult::neutral()->addCacheContexts(['user.permissions'])->setReason("The 'administer menu' permission is required."), @@ -116,7 +97,7 @@ class MenuAccessControlHandlerTest extends KernelTestBase { AccessResult::neutral()->addCacheContexts(['user.permissions'])->setReason("The 'administer menu' permission is required."), ], 'admin + unlocked' => [ - 'admin', + ['administer menu'], 'unlocked', AccessResult::allowed(), AccessResult::allowed()->addCacheContexts(['user.permissions']), @@ -125,25 +106,7 @@ class MenuAccessControlHandlerTest extends KernelTestBase { AccessResult::allowed()->addCacheContexts(['user.permissions']), ], 'admin + locked' => [ - 'admin', - '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']), - ], - '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', + ['administer menu'], 'locked', AccessResult::allowed(), AccessResult::allowed()->addCacheContexts(['user.permissions']),