diff --git a/core/lib/Drupal/Core/Extension/ModuleInstaller.php b/core/lib/Drupal/Core/Extension/ModuleInstaller.php index 10fabd412a..5374bd3c75 100644 --- a/core/lib/Drupal/Core/Extension/ModuleInstaller.php +++ b/core/lib/Drupal/Core/Extension/ModuleInstaller.php @@ -194,6 +194,17 @@ public function install(array $module_list, $enable_dependencies = TRUE) { // Update the kernel to include it. $this->updateKernel($module_filenames); + // Replace the route provider service with a version that will rebuild + // if routes used during installation. This ensures that a module's + // routes are available during installation. This has to occur before + // any services that depend on the it are instantiated otherwise those + // services will have the old route provider injected. Note that, since + // the container is rebuilt by updating the kernel, the route provider + // service is the regular one even though we are in a loop and might + // have replaced it before. + \Drupal::getContainer()->set('router.route_provider.old', \Drupal::service('router.route_provider')); + \Drupal::getContainer()->set('router.route_provider', \Drupal::service('router.route_provider.lazy_builder')); + // Allow modules to react prior to the installation of a module. $this->moduleHandler->invokeAll('module_preinstall', [$module]); @@ -283,10 +294,6 @@ public function install(array $module_list, $enable_dependencies = TRUE) { // @see https://www.drupal.org/node/2208429 \Drupal::service('theme_handler')->refreshInfo(); - // In order to make uninstalling transactional if anything uses routes. - \Drupal::getContainer()->set('router.route_provider.old', \Drupal::service('router.route_provider')); - \Drupal::getContainer()->set('router.route_provider', \Drupal::service('router.route_provider.lazy_builder')); - // Allow the module to perform install tasks. $this->moduleHandler->invoke($module, 'install'); diff --git a/core/modules/content_moderation/src/Permissions.php b/core/modules/content_moderation/src/Permissions.php index a6d6ce505d..038d6d96ea 100644 --- a/core/modules/content_moderation/src/Permissions.php +++ b/core/modules/content_moderation/src/Permissions.php @@ -30,6 +30,9 @@ public function transitionPermissions() { '%workflow' => $workflow->label(), '%transition' => $transition->label(), ]), + 'dependencies' => [ + $workflow->getConfigDependencyKey() => [$workflow->getConfigDependencyName()], + ], ]; } } diff --git a/core/modules/content_translation/src/ContentTranslationPermissions.php b/core/modules/content_translation/src/ContentTranslationPermissions.php index dc7959bb6c..1d2fbce2c1 100644 --- a/core/modules/content_translation/src/ContentTranslationPermissions.php +++ b/core/modules/content_translation/src/ContentTranslationPermissions.php @@ -72,6 +72,9 @@ public function contentPermissions() { $permission["translate $bundle $entity_type_id"] = [ 'title' => $this->t('Translate %bundle_label @entity_label', $t_args), ]; + if (($bundle_entity_type = $entity_type->getBundleEntityType()) && $bundle_entity = $this->entityManager->getStorage($bundle_entity_type)->load($bundle)) { + $permission["translate $bundle $entity_type_id"]['dependencies'][$bundle_entity->getConfigDependencyKey()][] = $bundle_entity->getConfigDependencyName(); + } } } break; diff --git a/core/modules/field_ui/src/FieldUiPermissions.php b/core/modules/field_ui/src/FieldUiPermissions.php index be73db6658..b36124c8b9 100644 --- a/core/modules/field_ui/src/FieldUiPermissions.php +++ b/core/modules/field_ui/src/FieldUiPermissions.php @@ -48,17 +48,22 @@ public function fieldPermissions() { foreach ($this->entityManager->getDefinitions() as $entity_type_id => $entity_type) { if ($entity_type->get('field_ui_base_route')) { + // The permissions should depend on the module that provides the entity. + $dependencies = ['module' => [$entity_type->getProvider()]]; // Create a permission for each fieldable entity to manage // the fields and the display. $permissions['administer ' . $entity_type_id . ' fields'] = [ 'title' => $this->t('%entity_label: Administer fields', ['%entity_label' => $entity_type->getLabel()]), 'restrict access' => TRUE, + 'dependencies' => $dependencies, ]; $permissions['administer ' . $entity_type_id . ' form display'] = [ - 'title' => $this->t('%entity_label: Administer form display', ['%entity_label' => $entity_type->getLabel()]) + 'title' => $this->t('%entity_label: Administer form display', ['%entity_label' => $entity_type->getLabel()]), + 'dependencies' => $dependencies, ]; $permissions['administer ' . $entity_type_id . ' display'] = [ - 'title' => $this->t('%entity_label: Administer display', ['%entity_label' => $entity_type->getLabel()]) + 'title' => $this->t('%entity_label: Administer display', ['%entity_label' => $entity_type->getLabel()]), + 'dependencies' => $dependencies, ]; } } diff --git a/core/modules/filter/src/FilterPermissions.php b/core/modules/filter/src/FilterPermissions.php index fc22f3c4cc..181394dd81 100644 --- a/core/modules/filter/src/FilterPermissions.php +++ b/core/modules/filter/src/FilterPermissions.php @@ -59,6 +59,13 @@ public function permissions() { '#markup' => $this->t('Warning: This permission may have security implications depending on how the text format is configured.'), '#suffix' => '' ], + // This permission is generated in behalf of $format text format, thus + // we add this text format as a config dependency. + 'dependencies' => [ + $format->getConfigDependencyKey() => [ + $format->getConfigDependencyName(), + ], + ], ]; } } diff --git a/core/modules/node/src/NodePermissions.php b/core/modules/node/src/NodePermissions.php index 1996360ae1..bf4a246b43 100644 --- a/core/modules/node/src/NodePermissions.php +++ b/core/modules/node/src/NodePermissions.php @@ -23,9 +23,20 @@ class NodePermissions { */ public function nodeTypePermissions() { $perms = []; + /* @var \Drupal\node\Entity\NodeType $type */ // Generate node permissions for all node types. foreach (NodeType::loadMultiple() as $type) { - $perms += $this->buildPermissions($type); + $perms_to_add = array_map( + function ($perm) use ($type) { + // This permission is generated in behalf of a node type, therefore + // add the node type as a config dependency. + $perm['dependencies'] = [ + $type->getConfigDependencyKey() => [$type->getConfigDependencyName()], + ]; + return $perm; + }, + $this->buildPermissions($type)); + $perms += $perms_to_add; } return $perms; diff --git a/core/modules/rest/src/Plugin/rest/resource/EntityResource.php b/core/modules/rest/src/Plugin/rest/resource/EntityResource.php index 5d9849ded4..94baa54fd1 100644 --- a/core/modules/rest/src/Plugin/rest/resource/EntityResource.php +++ b/core/modules/rest/src/Plugin/rest/resource/EntityResource.php @@ -365,7 +365,10 @@ public function permissions() { // @see https://www.drupal.org/node/2664780 if ($this->configFactory->get('rest.settings')->get('bc_entity_resource_permissions')) { // The default Drupal 8.0.x and 8.1.x behavior. - return parent::permissions(); + return array_map(function ($permission_info) { + $permission_info['dependencies']['config'][] = 'rest.settings'; + return $permission_info; + }, parent::permissions()); } else { // The default Drupal 8.2.x behavior. diff --git a/core/modules/rest/src/RestPermissions.php b/core/modules/rest/src/RestPermissions.php index 473c1924ee..045e8c902f 100644 --- a/core/modules/rest/src/RestPermissions.php +++ b/core/modules/rest/src/RestPermissions.php @@ -57,7 +57,17 @@ public function permissions() { $resource_configs = $this->resourceConfigStorage->loadMultiple(); foreach ($resource_configs as $resource_config) { $plugin = $resource_config->getResourcePlugin(); - $permissions = array_merge($permissions, $plugin->permissions()); + // @todo work out what to do here for permission dependencies. + $permissions_to_add = array_map(function ($permission_info) use ($resource_config) { + $permission_info += ['dependencies' => []]; + $permission_info['dependencies'] += [ + $resource_config->getConfigDependencyKey() => [ + $resource_config->getConfigDependencyName(), + ], + ]; + return $permission_info; + }, $plugin->permissions()); + $permissions = array_merge($permissions, $permissions_to_add); } return $permissions; } diff --git a/core/modules/taxonomy/src/TaxonomyPermissions.php b/core/modules/taxonomy/src/TaxonomyPermissions.php index 1772a34f97..5321d2396a 100644 --- a/core/modules/taxonomy/src/TaxonomyPermissions.php +++ b/core/modules/taxonomy/src/TaxonomyPermissions.php @@ -50,7 +50,17 @@ public static function create(ContainerInterface $container) { public function permissions() { $permissions = []; foreach (Vocabulary::loadMultiple() as $vocabulary) { - $permissions += $this->buildPermissions($vocabulary); + $perms_to_add = array_map( + function ($perm) use ($vocabulary) { + // This permission is generated in behalf of a vocabulary, therefore + // add the vocabulary as a config dependency. + $perm['dependencies'] = [ + $vocabulary->getConfigDependencyKey() => [$vocabulary->getConfigDependencyName()], + ]; + return $perm; + }, + $this->buildPermissions($vocabulary)); + $permissions += $perms_to_add; } return $permissions; } diff --git a/core/modules/user/src/Entity/Role.php b/core/modules/user/src/Entity/Role.php index 0226f94b78..c288a761fe 100644 --- a/core/modules/user/src/Entity/Role.php +++ b/core/modules/user/src/Entity/Role.php @@ -186,4 +186,67 @@ public function preSave(EntityStorageInterface $storage) { } } + /** + * {@inheritdoc} + */ + public function calculateDependencies() { + parent::calculateDependencies(); + // Load all permissions. + $permissions = \Drupal::service('user.permissions')->getPermissions(); + foreach ($this->permissions as $permission) { + // @todo Remove this if() when https://www.drupal.org/node/2569741 is in. + if (isset($permissions[$permission])) { + // Depend on the module that is providing this permissions. + $this->addDependency('module', $permissions[$permission]['provider']); + // Depend on any other dependencies defined by permissions granted to + // this role. + if (!empty($permissions[$permission]['dependencies'])) { + foreach ($permissions[$permission]['dependencies'] as $type => $dependencies) { + foreach ($dependencies as $dependency) { + $this->addDependency($type, $dependency); + } + } + } + } + } + return $this->dependencies; + } + + /** + * {@inheritdoc} + */ + public function onDependencyRemoval(array $dependencies) { + $changed = parent::onDependencyRemoval($dependencies); + // Load all permissions. + $permissions = \Drupal::service('user.permissions')->getPermissions(); + + // Merge the permission provider into the module dependency list. + array_walk($permissions, function (&$permission) { + $permission['dependencies']['module'][] = $permission['provider']; + $permission['dependencies']['module'] = array_unique($permission['dependencies']['module']); + }); + + $removed_permissions = []; + foreach ($dependencies as $type => $items) { + foreach ($items as $key => $dependency) { + // Get the dependency name, based on dependency type. + $name = in_array($type, ['config', 'content']) ? $dependency->getConfigDependencyName() : $dependency; + foreach ($this->permissions as $permission) { + if (!empty($permissions[$permission]['dependencies'][$type])) { + if (in_array($name, $permissions[$permission]['dependencies'][$type])) { + $removed_permissions[] = $permission; + break; + } + } + } + } + } + if ($removed_permissions) { + $this->permissions = array_diff($this->permissions, $removed_permissions); + $changed = TRUE; + } + + return $changed; + } + } diff --git a/core/modules/user/src/PermissionHandler.php b/core/modules/user/src/PermissionHandler.php index 1f07b25d95..38559447bc 100644 --- a/core/modules/user/src/PermissionHandler.php +++ b/core/modules/user/src/PermissionHandler.php @@ -33,6 +33,11 @@ * # (optional) Boolean, when set to true a warning about site security will * # be displayed on the Permissions page. Defaults to false. * restrict access: false + * # (optional) Dependency array following the same structure as the return + * # config entities dependencies. + * dependencies: + * config: + * - node.type.article * * # An array of callables used to generate dynamic permissions. * permission_callbacks: @@ -41,6 +46,7 @@ * - Drupal\filter\FilterPermissions::permissions * @endcode * + * @see \Drupal\user\PermissionHandlerInterface::getPermissions() * @see filter.permissions.yml * @see \Drupal\filter\FilterPermissions * @see user_api @@ -130,10 +136,10 @@ public function moduleProvidesPermissions($module_name) { * Builds all permissions provided by .permissions.yml files. * * @return array[] - * Each return permission is an array with the following keys: - * - title: The title of the permission. - * - description: The description of the permission, defaults to NULL. - * - provider: The provider of the permission. + * An array with the same structure as + * PermissionHandlerInterface::getPermissions(). + * + * @see \Drupal\user\PermissionHandlerInterface::getPermissions() */ protected function buildPermissionsYaml() { $all_permissions = []; @@ -193,10 +199,10 @@ protected function buildPermissionsYaml() { * The permissions to be sorted. * * @return array[] - * Each return permission is an array with the following keys: - * - title: The title of the permission. - * - description: The description of the permission, defaults to NULL. - * - provider: The provider of the permission. + * An array with the same structure as + * PermissionHandlerInterface::getPermissions(). + * + * @see \Drupal\user\PermissionHandlerInterface::getPermissions() */ protected function sortPermissions(array $all_permissions = []) { // Get a list of all the modules providing permissions and sort by diff --git a/core/modules/user/src/PermissionHandlerInterface.php b/core/modules/user/src/PermissionHandlerInterface.php index 61526f339a..ec70d8e6fc 100644 --- a/core/modules/user/src/PermissionHandlerInterface.php +++ b/core/modules/user/src/PermissionHandlerInterface.php @@ -34,7 +34,20 @@ * permissions to have a clear, consistent security warning that is the * same across the site. Use the 'description' key instead to provide any * information that is specific to the permission you are defining. + * - dependencies: (optional) An array of dependency entities used when + * building this permission name, structured in the same way as the return + * of ConfigEntityInterface::calculateDependencies(). For example if this + * permission was generated as effect of the existence of node type + * 'article', then value of the dependency key is: + * @code + * 'dependencies' => ['config' => ['node.type.article']] + * @endcode + * The module providing this permission doesn't have to be added as + * dependency, because is automatically parsed, stored and retrieved from + * the 'provider' key. * - provider: (optional) The provider name of the permission. + * + * @see \Drupal\Core\Config\Entity\ConfigEntityInterface::calculateDependencies() */ public function getPermissions(); diff --git a/core/modules/user/tests/src/Kernel/UserRoleDeleteTest.php b/core/modules/user/tests/src/Kernel/UserRoleDeleteTest.php index cac470aaab..08f3888b6c 100644 --- a/core/modules/user/tests/src/Kernel/UserRoleDeleteTest.php +++ b/core/modules/user/tests/src/Kernel/UserRoleDeleteTest.php @@ -2,7 +2,11 @@ namespace Drupal\Tests\user\Kernel; +use Drupal\Component\Utility\Unicode; +use Drupal\filter\Entity\FilterFormat; use Drupal\KernelTests\KernelTestBase; +use Drupal\node\Entity\NodeType; +use Drupal\user\Entity\Role; use Drupal\user\Entity\User; /** @@ -70,5 +74,73 @@ public function testRoleDeleteUserRoleReferenceDelete() { $this->assertTrue($user->hasRole('test_role_two')); } + /** + * Tests the removal of user role dependencies. + */ + public function testDependenciesRemoval() { + $this->enableModules(['node', 'filter']); + $this->installSchema('system', ['router']); + $this->container->get('router.builder')->rebuild(); + + /** @var \Drupal\user\RoleInterface $role */ + $role = Role::create([ + 'id' => $rid = Unicode::strtolower($this->randomMachineName()), + 'label' => $this->randomString(), + ]); + $role->save(); + + /** @var \Drupal\node\NodeTypeInterface $node_type */ + $node_type = NodeType::create([ + 'type' => Unicode::strtolower($this->randomMachineName()), + 'name' => $this->randomString(), + ]); + $node_type->save(); + // Create a new text format to be used by role $role. + $format = FilterFormat::create([ + 'format' => Unicode::strtolower($this->randomMachineName()), + 'name' => $this->randomString(), + ]); + $format->save(); + + $permission_format = "use text format {$format->id()}"; + $permission_node_type = "edit any {$node_type->id()} content"; + + // Grant $role with permission to use $format and edit $node_type. + $role + ->grantPermission($permission_format) + ->grantPermission($permission_node_type) + ->save(); + + // The role $role has the permissions to use $format and edit $node_type. + $role = Role::load($rid); + $this->assertTrue($role->hasPermission($permission_format)); + $this->assertTrue($role->hasPermission($permission_node_type)); + + // The $role config entity exists before removing dependencies. + $this->assertNotNull($role = Role::load($rid)); + + // Remove the format. + $format->delete(); + + // The $role config entity exists after removing the config dependency. + if ($this->assertNotNull($role = Role::load($rid))) { + // The $format permission should have been revoked. + $this->assertFalse($role->hasPermission($permission_format)); + $this->assertTrue($role->hasPermission($permission_node_type)); + } + + // We have to manually trigger the removal of configuration belonging to the + // module because KernelTestBase::disableModules() is not aware of this. + $this->container->get('config.manager')->uninstall('module', 'node'); + // Disable the node module. + $this->disableModules(['node']); + + // The $role config entity exists after removing the module dependency. + if ($this->assertNotNull($role = Role::load($rid))) { + // The $node_type permission should have been revoked too. + $this->assertFalse($role->hasPermission($permission_format)); + $this->assertFalse($role->hasPermission($permission_node_type)); + } + } }