diff --git a/core/modules/content_moderation/content_moderation.permissions.yml b/core/modules/content_moderation/content_moderation.permissions.yml index 18790fb52e61fd02f952fae785afda9dfda4d9d0..4b193d07213008ce6e8d646b6a3111c3bf44cc79 100644 --- a/core/modules/content_moderation/content_moderation.permissions.yml +++ b/core/modules/content_moderation/content_moderation.permissions.yml @@ -2,10 +2,6 @@ view any unpublished content: title: 'View any unpublished content' description: 'This permission is necessary for any users that may moderate content.' -view content moderation: - title: 'View content moderation' - description: 'View content moderation.' - view latest version: title: 'View the latest version' description: 'View the latest version of an entity. (Also requires "View any unpublished content" permission)' diff --git a/core/modules/content_moderation/src/Plugin/WorkflowType/ContentModeration.php b/core/modules/content_moderation/src/Plugin/WorkflowType/ContentModeration.php index ceaa565767554cd0f22f700e72a1ffd190c99967..1515cb46d84b7254f13fa1b022ae199081d1c895 100644 --- a/core/modules/content_moderation/src/Plugin/WorkflowType/ContentModeration.php +++ b/core/modules/content_moderation/src/Plugin/WorkflowType/ContentModeration.php @@ -3,13 +3,11 @@ namespace Drupal\content_moderation\Plugin\WorkflowType; use Drupal\content_moderation\ModerationInformationInterface; -use Drupal\Core\Access\AccessResult; use Drupal\Core\Entity\ContentEntityInterface; use Drupal\Core\Entity\EntityTypeBundleInfoInterface; use Drupal\Core\Entity\EntityTypeManagerInterface; use Drupal\Core\Entity\EntityPublishedInterface; use Drupal\Core\Plugin\ContainerFactoryPluginInterface; -use Drupal\Core\Session\AccountInterface; use Drupal\Core\StringTranslation\StringTranslationTrait; use Drupal\content_moderation\ContentModerationState; use Drupal\workflows\Plugin\WorkflowTypeBase; @@ -93,16 +91,6 @@ public static function create(ContainerInterface $container, array $configuratio ); } - /** - * {@inheritdoc} - */ - public function checkWorkflowAccess(WorkflowInterface $entity, $operation, AccountInterface $account) { - if ($operation === 'view') { - return AccessResult::allowedIfHasPermission($account, 'view content moderation'); - } - return parent::checkWorkflowAccess($entity, $operation, $account); - } - /** * {@inheritdoc} */ diff --git a/core/modules/content_moderation/tests/src/Functional/ModerationRevisionRevertTest.php b/core/modules/content_moderation/tests/src/Functional/ModerationRevisionRevertTest.php index c054d9d92ed522b569dbc656a945963cd1ac18fa..703c10ff7930b9d1ed43a36d7193d2a8ea09e3a5 100644 --- a/core/modules/content_moderation/tests/src/Functional/ModerationRevisionRevertTest.php +++ b/core/modules/content_moderation/tests/src/Functional/ModerationRevisionRevertTest.php @@ -43,7 +43,6 @@ public function setUp() { 'administer nodes', 'bypass node access', 'view all revisions', - 'view content moderation', 'use editorial transition create_new_draft', 'use editorial transition publish', ]); diff --git a/core/modules/content_moderation/tests/src/Functional/ModerationStateAccessTest.php b/core/modules/content_moderation/tests/src/Functional/ModerationStateAccessTest.php index 868049944fa57116e35962ea424ede3b4b7ac791..304dcb5c895af426b98c88ce0ccfac1185b94757 100644 --- a/core/modules/content_moderation/tests/src/Functional/ModerationStateAccessTest.php +++ b/core/modules/content_moderation/tests/src/Functional/ModerationStateAccessTest.php @@ -32,7 +32,6 @@ public function testViewShowsCorrectStates() { $permissions = [ 'access content', 'view all revisions', - 'view content moderation', ]; $editor1 = $this->drupalCreateUser($permissions); $this->drupalLogin($editor1); diff --git a/core/modules/workflows/src/Plugin/WorkflowTypeBase.php b/core/modules/workflows/src/Plugin/WorkflowTypeBase.php index 23481a46172e2ce318645195a15bb53151a70b84..9005836e77f4f17e962e08e89a203edd1bde4938 100644 --- a/core/modules/workflows/src/Plugin/WorkflowTypeBase.php +++ b/core/modules/workflows/src/Plugin/WorkflowTypeBase.php @@ -3,9 +3,7 @@ namespace Drupal\workflows\Plugin; use Drupal\Component\Plugin\PluginBase; -use Drupal\Core\Access\AccessResult; use Drupal\Core\Plugin\PluginWithFormsTrait; -use Drupal\Core\Session\AccountInterface; use Drupal\workflows\State; use Drupal\workflows\StateInterface; use Drupal\workflows\Transition; @@ -48,13 +46,6 @@ public function label() { return $definition['label']; } - /** - * {@inheritdoc} - */ - public function checkWorkflowAccess(WorkflowInterface $entity, $operation, AccountInterface $account) { - return AccessResult::neutral(); - } - /** * {@inheritdoc} */ diff --git a/core/modules/workflows/src/WorkflowAccessControlHandler.php b/core/modules/workflows/src/WorkflowAccessControlHandler.php index 71274c0e326d8f987af79b6a15678784a018f3f5..e34e647e0858bdd4eb9511b3baa06a752a189c3b 100644 --- a/core/modules/workflows/src/WorkflowAccessControlHandler.php +++ b/core/modules/workflows/src/WorkflowAccessControlHandler.php @@ -12,7 +12,7 @@ use Symfony\Component\DependencyInjection\ContainerInterface; /** - * Access controller for the Moderation State entity. + * Access controller for the Workflow entity. * * @see \Drupal\workflows\Entity\Workflow. * @@ -62,15 +62,13 @@ protected function checkAccess(EntityInterface $entity, $operation, AccountInter list(, $state_id) = explode(':', $operation, 2); // Deleting a state is editing a workflow, but also we should forbid // access if there is only one state. - $admin_access = AccessResult::allowedIf(count($entity->getTypePlugin()->getStates()) > 1) + return AccessResult::allowedIf(count($entity->getTypePlugin()->getStates()) > 1) ->andIf(parent::checkAccess($entity, 'edit', $account)) ->andIf(AccessResult::allowedIf(!in_array($state_id, $workflow_type->getRequiredStates(), TRUE))) ->addCacheableDependency($entity); } - else { - $admin_access = parent::checkAccess($entity, $operation, $account); - } - return $workflow_type->checkWorkflowAccess($entity, $operation, $account)->orIf($admin_access); + + return parent::checkAccess($entity, $operation, $account); } /** @@ -81,7 +79,9 @@ protected function checkCreateAccess(AccountInterface $account, array $context, $admin_access = parent::checkCreateAccess($account, $context, $entity_bundle); // Allow access if there is at least one workflow type. Since workflow types // are provided by modules this is cacheable until extensions change. - return $admin_access->andIf(AccessResult::allowedIf($workflow_types_count > 0))->addCacheTags(['config:core.extension']); + return $admin_access + ->andIf(AccessResult::allowedIf($workflow_types_count > 0)) + ->addCacheTags(['workflow_type_plugins']); } } diff --git a/core/modules/workflows/src/WorkflowTypeInterface.php b/core/modules/workflows/src/WorkflowTypeInterface.php index 032422e04b8180ac8afc48540eef424cd3ee8253..0d9107104a523c2296205bae975aedb6087e8b98 100644 --- a/core/modules/workflows/src/WorkflowTypeInterface.php +++ b/core/modules/workflows/src/WorkflowTypeInterface.php @@ -5,7 +5,6 @@ use Drupal\Component\Plugin\ConfigurablePluginInterface; use Drupal\Component\Plugin\DerivativeInspectionInterface; use Drupal\Core\Plugin\PluginWithFormsInterface; -use Drupal\Core\Session\AccountInterface; /** * An interface for Workflow type plugins. @@ -29,22 +28,6 @@ interface WorkflowTypeInterface extends PluginWithFormsInterface, DerivativeInsp */ public function label(); - /** - * Performs access checks. - * - * @param \Drupal\workflows\WorkflowInterface $entity - * The workflow entity for which to check access. - * @param string $operation - * The entity operation. Usually one of 'view', 'view label', 'update' or - * 'delete'. - * @param \Drupal\Core\Session\AccountInterface $account - * The user for which to check access. - * - * @return \Drupal\Core\Access\AccessResultInterface - * The access result. - */ - public function checkWorkflowAccess(WorkflowInterface $entity, $operation, AccountInterface $account); - /** * Determines if the workflow is being has data associated with it. * diff --git a/core/modules/workflows/src/WorkflowTypeManager.php b/core/modules/workflows/src/WorkflowTypeManager.php index b2eafa863bdd70878253ad6fc6d21f28b8f91e44..b1675380bf444c175be56cda03913dc32c5232f4 100644 --- a/core/modules/workflows/src/WorkflowTypeManager.php +++ b/core/modules/workflows/src/WorkflowTypeManager.php @@ -34,7 +34,7 @@ class WorkflowTypeManager extends DefaultPluginManager { public function __construct(\Traversable $namespaces, CacheBackendInterface $cache_backend, ModuleHandlerInterface $module_handler) { parent::__construct('Plugin/WorkflowType', $namespaces, $module_handler, WorkflowTypeInterface::class, WorkflowType::class); $this->alterInfo('workflow_type_info'); - $this->setCacheBackend($cache_backend, 'workflow_type_info'); + $this->setCacheBackend($cache_backend, 'workflow_type_info', ['workflow_type_plugins']); } } diff --git a/core/modules/workflows/tests/modules/workflow_type_test/workflow_type_test.module b/core/modules/workflows/tests/modules/workflow_type_test/workflow_type_test.module new file mode 100644 index 0000000000000000000000000000000000000000..b97f2c5e85f00a179a4cff470766f7d290ce6f5c --- /dev/null +++ b/core/modules/workflows/tests/modules/workflow_type_test/workflow_type_test.module @@ -0,0 +1,28 @@ +get('workflow_type_test.plugin_definitions') !== NULL) { + $definitions = $state->get('workflow_type_test.plugin_definitions'); + } +} + +/** + * Sets the type plugin definitions override and clear the cache. + * + * @param array $definitions + * Definitions to set. + */ +function workflow_type_test_set_definitions($definitions) { + \Drupal::state()->set('workflow_type_test.plugin_definitions', $definitions); + \Drupal::service('plugin.manager.workflows.type')->clearCachedDefinitions(); +} diff --git a/core/modules/workflows/tests/src/Kernel/WorkflowAccessControlHandlerTest.php b/core/modules/workflows/tests/src/Kernel/WorkflowAccessControlHandlerTest.php new file mode 100644 index 0000000000000000000000000000000000000000..cd4e7ceb14e05447d2560f968e06495bca424c91 --- /dev/null +++ b/core/modules/workflows/tests/src/Kernel/WorkflowAccessControlHandlerTest.php @@ -0,0 +1,218 @@ +installEntitySchema('workflow'); + $this->installEntitySchema('user'); + $this->installSchema('system', ['sequences']); + + $this->accessControlHandler = $this->container->get('entity_type.manager')->getAccessControlHandler('workflow'); + + // Create and discard user 1, which is special and bypasses all access + // checking. + $this->createUser([]); + $this->user = $this->createUser([]); + $this->adminUser = $this->createUser(['administer workflows']); + } + + /** + * @covers ::checkCreateAccess + */ + public function testCheckCreateAccess() { + // A user must have the correct permission to create a workflow. + $this->assertEquals( + AccessResult::neutral() + ->addCacheContexts(['user.permissions']) + ->setReason("The 'administer workflows' permission is required.") + ->addCacheTags(['workflow_type_plugins']), + $this->accessControlHandler->createAccess(NULL, $this->user, [], TRUE) + ); + $this->assertEquals( + AccessResult::allowed() + ->addCacheContexts(['user.permissions']) + ->addCacheTags(['workflow_type_plugins']), + $this->accessControlHandler->createAccess(NULL, $this->adminUser, [], TRUE) + ); + + // Remove all plugin types and ensure not even the admin user is allowed to + // create a workflow. + workflow_type_test_set_definitions([]); + $this->accessControlHandler->resetCache(); + $this->assertEquals( + AccessResult::neutral() + ->addCacheContexts(['user.permissions']) + ->addCacheTags(['workflow_type_plugins']), + $this->accessControlHandler->createAccess(NULL, $this->adminUser, [], TRUE) + ); + } + + /** + * @covers ::checkAccess + * @dataProvider checkAccessProvider + */ + public function testCheckAccess($user, $operation, $result, $states_to_create = []) { + $workflow = Workflow::create([ + 'type' => 'workflow_type_test', + 'id' => 'test_workflow', + ]); + $workflow->save(); + $workflow_type = $workflow->getTypePlugin(); + foreach ($states_to_create as $state_id => $is_required) { + $workflow_type->addState($state_id, $this->randomString()); + } + \Drupal::state()->set('workflow_type_test.required_states', array_filter($states_to_create)); + $this->assertEquals($result, $this->accessControlHandler->access($workflow, $operation, $this->{$user}, TRUE)); + } + + /** + * Data provider for ::testCheckAccess. + * + * @return array + */ + public function checkAccessProvider() { + $container = new ContainerBuilder(); + $cache_contexts_manager = $this->prophesize(CacheContextsManager::class); + $cache_contexts_manager->assertValidTokens()->willReturn(TRUE); + $cache_contexts_manager->reveal(); + $container->set('cache_contexts_manager', $cache_contexts_manager); + \Drupal::setContainer($container); + + return [ + 'Admin view' => [ + 'adminUser', + 'view', + AccessResult::allowed()->addCacheContexts(['user.permissions']), + ], + 'Admin update' => [ + 'adminUser', + 'update', + AccessResult::allowed()->addCacheContexts(['user.permissions']), + ], + 'Admin delete' => [ + 'adminUser', + 'delete', + AccessResult::allowed()->addCacheContexts(['user.permissions']), + ], + 'Admin delete only state' => [ + 'adminUser', + 'delete-state:foo', + AccessResult::neutral()->addCacheTags(['config:workflows.workflow.test_workflow']), + ['foo' => FALSE], + ], + 'Admin delete one of two states' => [ + 'adminUser', + 'delete-state:foo', + AccessResult::allowed() + ->addCacheTags(['config:workflows.workflow.test_workflow']) + ->addCacheContexts(['user.permissions']), + ['foo' => FALSE, 'bar' => FALSE], + ], + 'Admin delete required state when there are >1 states' => [ + 'adminUser', + 'delete-state:foo', + AccessResult::allowed() + ->addCacheTags(['config:workflows.workflow.test_workflow']) + ->addCacheContexts(['user.permissions']), + ['foo' => TRUE, 'bar' => FALSE], + ], + 'User view' => [ + 'user', + 'view', + AccessResult::neutral() + ->addCacheContexts(['user.permissions']) + ->setReason("The 'administer workflows' permission is required."), + ], + 'User update' => [ + 'user', + 'update', + AccessResult::neutral() + ->addCacheContexts(['user.permissions']) + ->setReason("The 'administer workflows' permission is required."), + ], + 'User delete' => [ + 'user', + 'delete', + AccessResult::neutral() + ->addCacheContexts(['user.permissions']) + ->setReason("The 'administer workflows' permission is required."), + ], + 'User delete only state' => [ + 'user', + 'delete-state:foo', + AccessResult::neutral()->addCacheTags(['config:workflows.workflow.test_workflow']), + ['foo' => FALSE], + ], + 'User delete one of two states' => [ + 'user', + 'delete-state:foo', + AccessResult::neutral() + ->addCacheTags(['config:workflows.workflow.test_workflow']) + ->addCacheContexts(['user.permissions']) + ->setReason("The 'administer workflows' permission is required."), + ['foo' => FALSE, 'bar' => FALSE], + ], + 'User delete required state when there are >1 states' => [ + 'user', + 'delete-state:foo', + AccessResult::neutral() + ->addCacheTags(['config:workflows.workflow.test_workflow']) + ->addCacheContexts(['user.permissions']) + ->setReason("The 'administer workflows' permission is required."), + ['foo' => TRUE, 'bar' => FALSE], + ], + ]; + } + +}