Unverified Commit 5b224a44 authored by larowlan's avatar larowlan

Issue #2896726 by Sam152, timmillwood, Wim Leers, amateescu, jibran, larowlan:...

Issue #2896726 by Sam152, timmillwood, Wim Leers, amateescu, jibran, larowlan: Expand the entity access model for workflow states and transitions
parent 41b994cc
......@@ -2,56 +2,48 @@
namespace Drupal\workflows;
use Drupal\Core\Access\AccessResult;
use Drupal\Core\Entity\EntityInterface;
use Drupal\Core\Routing\Access\AccessInterface;
use Drupal\Core\Routing\RouteMatchInterface;
use Drupal\Core\Session\AccountInterface;
use Symfony\Component\Routing\Route;
/**
* Provides a access checker for deleting a workflow state.
*
* @internal
* Marked as internal until it's validated this should form part of the public
* API in https://www.drupal.org/node/2897148.
* Marked as internal for use by the workflows module only.
*
* @deprecated
* Using the _workflow_state_delete_access check is deprecated in Drupal 8.6.0
* and will be removed before Drupal 9.0.0, you can use _workflow_access in
* route definitions instead.
* @code
* # The old approach:
* requirements:
* _workflow_state_delete_access: 'true'
* # The new approach:
* requirements:
* _workflow_access: 'delete-state'
* @endcode
* As an internal API the ability to use _workflow_state_delete_access may
* also be removed in a minor release.
*
* @see \Drupal\workflows\WorkflowStateTransitionOperationsAccessCheck
* @see https://www.drupal.org/node/2929327
*/
class WorkflowDeleteAccessCheck implements AccessInterface {
class WorkflowDeleteAccessCheck extends WorkflowStateTransitionOperationsAccessCheck {
/**
* {@inheritdoc}
*/
public function access(RouteMatchInterface $route_match, AccountInterface $account) {
@trigger_error('Using the _workflow_state_delete_access check is deprecated in Drupal 8.6.0 and will be removed before Drupal 9.0.0, use _workflow_access instead. As an internal API _workflow_state_delete_access may also be removed in a minor release.', E_USER_DEPRECATED);
return parent::access($route_match, $account);
}
/**
* Checks access to deleting a workflow state for a particular route.
*
* The value of '_workflow_state_delete_access' is ignored. The route must
* have the parameters 'workflow' and 'workflow_state'. For example:
* @code
* pattern: '/foo/{workflow}/bar/{workflow_state}/delete'
* requirements:
* _workflow_state_delete_access: 'true'
* @endcode
* @see \Drupal\Core\ParamConverter\EntityConverter
*
* @param \Symfony\Component\Routing\Route $route
* The route to check against.
* @param \Drupal\Core\Routing\RouteMatchInterface $route_match
* The parametrized route
* @param \Drupal\Core\Session\AccountInterface $account
* The currently logged in account.
*
* @return \Drupal\Core\Access\AccessResultInterface
* The access result.
* {@inheritdoc}
*/
public function access(Route $route, RouteMatchInterface $route_match, AccountInterface $account) {
// If there is valid entity of the given entity type, check its access.
$parameters = $route_match->getParameters();
if ($parameters->has('workflow') && $parameters->has('workflow_state')) {
$entity = $parameters->get('workflow');
if ($entity instanceof EntityInterface) {
return $entity->access('delete-state:' . $parameters->get('workflow_state'), $account, TRUE);
}
}
// No opinion, so other access checks should decide if access should be
// allowed or not.
return AccessResult::neutral();
protected function getOperation(RouteMatchInterface $route_match) {
return 'delete-state';
}
}
<?php
namespace Drupal\workflows;
use Drupal\Core\Access\AccessResult;
use Drupal\Core\Routing\Access\AccessInterface;
use Drupal\Core\Routing\RouteMatchInterface;
use Drupal\Core\Session\AccountInterface;
/**
* Provides an access check for state and transition operations.
*/
class WorkflowStateTransitionOperationsAccessCheck implements AccessInterface {
/**
* Checks access for operations of workflow states and transitions.
*
* The value of '_workflow_access' is used to check to kind of access that
* should be applied to a route in the context of a workflow and a state or
* transition. States and transitions can individually have access control
* applied to them for 'add', 'update' and 'delete'. By default workflows will
* use the admin permission 'administer workflows' for all of these
* operations, except for delete-state which checks there is at least one
* state, a state does not have data and it's not a required state.
*
* For the update and delete operations, a workflow and a state or transition
* is required in the route for the access check to be applied. For the "add"
* operation, only a workflow is required. The '_workflow_access' requirement
* translates into access checks on the workflow entity type in the formats:
* - @code"$operation-state:$state_id"@endcode
* - @code"$operation-transition:$transition_id"@endcode
*
* For example the following route definition with the path
* "/test-workflow/foo-state/delete" the 'delete-state:foo-state' operation
* will be checked:
* @code
* pattern: '/{workflow}/{workflow_state}/delete'
* requirements:
* _workflow_access: 'delete-state'
* @endcode
*
* @param \Drupal\Core\Routing\RouteMatchInterface $route_match
* The parametrized route
* @param \Drupal\Core\Session\AccountInterface $account
* The currently logged in account.
*
* @return \Drupal\Core\Access\AccessResultInterface
* An access result.
*
* @throws \Exception
* Throws an exception when a route is defined with an invalid operation.
*/
public function access(RouteMatchInterface $route_match, AccountInterface $account) {
$workflow_operation = $this->getOperation($route_match);
if (!preg_match('/^(?<operation>add|update|delete)-(?<type>state|transition)$/', $workflow_operation, $matches)) {
throw new \Exception("Invalid _workflow_access operation '$workflow_operation' specified for route '{$route_match->getRouteName()}'.");
}
$parameters = $route_match->getParameters();
$workflow = $parameters->get('workflow');
if ($workflow && $matches['operation'] === 'add') {
return $workflow->access($workflow_operation, $account, TRUE);
}
if ($workflow && $type = $parameters->get(sprintf('workflow_%s', $matches['type']))) {
return $workflow->access(sprintf('%s:%s', $workflow_operation, $type), $account, TRUE);
}
return AccessResult::neutral();
}
/**
* Get the operation that will be used for the access check
*
* @param \Drupal\Core\Routing\RouteMatchInterface $route_match
* The parametrized route
*
* @return string
* The access operation.
*/
protected function getOperation(RouteMatchInterface $route_match) {
return $route_match->getRouteObject()->getRequirement('_workflow_access');
}
}
workflow.type_settings.workflow_type_test:
workflow_type_test.ignore_schema:
type: mapping
label: 'Workflow test type settings'
label: 'Ignore schema workflow type'
mapping:
states:
type: sequence
......@@ -11,18 +11,17 @@ workflow.type_settings.workflow_type_test:
sequence:
type: ignore
workflow.type_settings.workflow_type_required_state_test:
type: mapping
workflow.type_settings.workflow_type_test:
type: workflow_type_test.ignore_schema
label: 'Workflow test type settings'
mapping:
states:
type: sequence
sequence:
type: ignore
transitions:
type: sequence
sequence:
type: ignore
workflow.type_settings.workflow_type_required_state_test:
type: workflow_type_test.ignore_schema
label: 'Workflow test type required state'
workflow.type_settings.workflow_custom_access_type:
type: workflow_type_test.ignore_schema
label: 'Workflow custom access type'
# @todo, inline this straight into "workflow.type_settings.workflow_type_complex_test"
# after https://www.drupal.org/node/2871746 is resolved.
......
<?php
namespace Drupal\workflow_type_test\Plugin\WorkflowType;
use Drupal\Core\Access\AccessResult;
use Drupal\Core\Session\AccountInterface;
use Drupal\workflows\Plugin\WorkflowTypeBase;
use Drupal\workflows\WorkflowInterface;
/**
* A test workflow with custom state/transition access rules applied.
*
* @WorkflowType(
* id = "workflow_custom_access_type",
* label = @Translation("Workflow Custom Access Type Test"),
* )
*/
class WorkflowCustomAccessType extends WorkflowTypeBase {
/**
* {@inheritdoc}
*/
public function defaultConfiguration() {
return [
'states' => [
'cannot_update' => [
'label' => 'Cannot Update State',
'weight' => 0,
],
'cannot_delete' => [
'label' => 'Cannot Delete State',
'weight' => 0,
],
],
'transitions' => [
'cannot_update' => [
'label' => 'Cannot Update Transition',
'to' => 'cannot_update',
'weight' => 0,
'from' => [
'cannot_update',
],
],
'cannot_delete' => [
'label' => 'Cannot Delete Transition',
'to' => 'cannot_delete',
'weight' => 1,
'from' => [
'cannot_delete',
],
],
],
];
}
/**
* Implements hook_ENTITY_TYPE_access().
*
* @see workflow_type_test_workflow_access
*/
public static function workflowAccess(WorkflowInterface $entity, $operation, AccountInterface $account) {
$forbidden_operations = \Drupal::state()->get('workflow_type_test_forbidden_operations', []);
return in_array($operation, $forbidden_operations, TRUE)
? AccessResult::forbidden()
: AccessResult::neutral();
}
}
......@@ -5,6 +5,11 @@
* Module file for workflow_type_test.
*/
use Drupal\Core\Access\AccessResult;
use Drupal\Core\Session\AccountInterface;
use Drupal\workflow_type_test\Plugin\WorkflowType\WorkflowCustomAccessType;
use Drupal\workflows\WorkflowInterface;
/**
* Implements hook_workflow_type_info_alter().
*/
......@@ -26,3 +31,13 @@ function workflow_type_test_set_definitions($definitions) {
\Drupal::state()->set('workflow_type_test.plugin_definitions', $definitions);
\Drupal::service('plugin.manager.workflows.type')->clearCachedDefinitions();
}
/**
* Implements hook_ENTITY_TYPE_access() for the Workflow entity type.
*/
function workflow_type_test_workflow_access(WorkflowInterface $entity, $operation, AccountInterface $account) {
if ($entity->getTypePlugin()->getPluginId() === 'workflow_custom_access_type') {
return WorkflowCustomAccessType::workflowAccess($entity, $operation, $account);
}
return AccessResult::neutral();
}
<?php
namespace Drupal\Tests\workflows\Functional;
use Drupal\Tests\BrowserTestBase;
use Drupal\workflows\Entity\Workflow;
/**
* Test custom provided workflow access for state/transition operations.
*
* @group workflows
*/
class WorkflowCustomStateTransitionAccessTest extends BrowserTestBase {
/**
* {@inheritdoc}
*/
public static $modules = [
'workflows',
'workflow_type_test',
];
/**
* A test admin user.
*
* @var \Drupal\Core\Session\AccountInterface
*/
protected $adminUser;
/**
* A test workflow.
*
* @var \Drupal\workflows\WorkflowInterface
*/
protected $testWorkflow;
/**
* {@inheritdoc}
*/
protected function setUp() {
parent::setUp();
$this->adminUser = $this->createUser(['administer workflows']);
$this->testWorkflow = Workflow::create([
'label' => 'Test workflow',
'id' => 'test_type',
'type' => 'workflow_custom_access_type',
]);
$this->testWorkflow->save();
}
/**
* Test the custom state/transition operation access rules.
*/
public function testCustomWorkflowAccessOperations() {
$this->drupalLogin($this->adminUser);
$forbidden_paths = [
'admin/config/workflow/workflows/manage/test_type/state/cannot_delete/delete',
'admin/config/workflow/workflows/manage/test_type/state/cannot_update',
'admin/config/workflow/workflows/manage/test_type/transition/cannot_update',
'admin/config/workflow/workflows/manage/test_type/transition/cannot_delete/delete',
'admin/config/workflow/workflows/manage/test_type/add_state',
'admin/config/workflow/workflows/manage/test_type/add_transition',
];
// Until the list of forbidden operations have been set, the admin user
// should be able to access all the forbidden paths.
foreach ($forbidden_paths as $forbidden_path) {
$this->drupalGet($forbidden_path);
$this->assertSession()->statusCodeEquals(200);
}
// Update the forbidden operations which deny access to the actions
// represented by the above paths.
$this->container->get('state')->set('workflow_type_test_forbidden_operations', [
'update-state:cannot_update',
'delete-state:cannot_delete',
'update-transition:cannot_update',
'delete-transition:cannot_delete',
'add-state',
'add-transition',
]);
foreach ($forbidden_paths as $forbidden_path) {
$this->drupalGet($forbidden_path);
$this->assertSession()->statusCodeEquals(403);
}
}
}
......@@ -212,6 +212,66 @@ public function checkAccessProvider() {
->setReason("The 'administer workflows' permission is required."),
['foo' => TRUE, 'bar' => FALSE],
],
'Update state for user, uses admin permission by default' => [
'user',
'update-state:foo',
AccessResult::neutral()
->addCacheContexts(['user.permissions'])
->setReason("The 'administer workflows' permission is required."),
],
'Update state for admin, uses admin permission by default' => [
'adminUser',
'update-state:foo',
AccessResult::allowed()->addCacheContexts(['user.permissions']),
],
'Add state for user, uses admin permission by default' => [
'user',
'add-state',
AccessResult::neutral()
->addCacheContexts(['user.permissions'])
->setReason("The 'administer workflows' permission is required."),
],
'Add state for admin, uses admin permission by default' => [
'adminUser',
'add-state',
AccessResult::allowed()->addCacheContexts(['user.permissions']),
],
'Add transition for user, uses admin permission by default' => [
'user',
'add-transition',
AccessResult::neutral()
->addCacheContexts(['user.permissions'])
->setReason("The 'administer workflows' permission is required."),
],
'Add transition for admin, uses admin permission by default' => [
'adminUser',
'add-transition',
AccessResult::allowed()->addCacheContexts(['user.permissions']),
],
'Edit transition for user, uses admin permission by default' => [
'user',
'edit-transition:foo',
AccessResult::neutral()
->addCacheContexts(['user.permissions'])
->setReason("The 'administer workflows' permission is required."),
],
'Edit transition for admin, uses admin permission by default' => [
'adminUser',
'edit-transition:foo',
AccessResult::allowed()->addCacheContexts(['user.permissions']),
],
'Delete transition for user, uses admin permission by default' => [
'user',
'delete-transition:foo',
AccessResult::neutral()
->addCacheContexts(['user.permissions'])
->setReason("The 'administer workflows' permission is required."),
],
'Delete transition for admin, uses admin permission by default' => [
'adminUser',
'delete-transition:foo',
AccessResult::allowed()->addCacheContexts(['user.permissions']),
],
];
}
......
<?php
namespace Drupal\Tests\workflows\Unit;
use Drupal\Core\Access\AccessResult;
use Drupal\Core\Routing\RouteMatch;
use Drupal\Core\Session\AccountInterface;
use Drupal\Tests\UnitTestCase;
use Drupal\workflows\WorkflowDeleteAccessCheck;
use Drupal\workflows\WorkflowStateTransitionOperationsAccessCheck;
use Drupal\workflows\WorkflowInterface;
use Prophecy\Argument;
use Symfony\Component\Routing\Route;
/**
* @coversDefaultClass \Drupal\workflows\WorkflowStateTransitionOperationsAccessCheck
* @group workflows
*/
class WorkflowStateTransitionOperationsAccessCheckTest extends UnitTestCase {
/**
* Test the access method correctly proxies to the entity access system.
*
* @covers ::access
* @dataProvider accessTestCases
*/
public function testAccess($route_requirement, $resulting_entity_access_check, $route_parameters = []) {
$workflow_entity_access_result = AccessResult::allowed();
$workflow = $this->prophesize(WorkflowInterface::class);
$workflow->access($resulting_entity_access_check, Argument::type(AccountInterface::class), TRUE)
->shouldBeCalled()
->willReturn($workflow_entity_access_result);
$route = new Route(NULL, [
'workflow' => NULL,
'workflow_transition' => NULL,
'workflow_state' => NULL,
], [
'_workflow_access' => $route_requirement,
]);
$route_match_params = ['workflow' => $workflow->reveal()] + $route_parameters;
$route_match = new RouteMatch(NULL, $route, $route_match_params);
$access_check = new WorkflowStateTransitionOperationsAccessCheck();
$account = $this->prophesize(AccountInterface::class);
$this->assertEquals($workflow_entity_access_result, $access_check->access($route_match, $account->reveal()));
}
/**
* Test cases for ::testAccess.
*/
public function accessTestCases() {
return [
'Transition add' => [
'add-transition',
'add-transition',
],
'Transition update' => [
'update-transition',
'update-transition:foo-transition',
[
'workflow_transition' => 'foo-transition',
],
],
'Transition delete' => [
'delete-transition',
'delete-transition:foo-transition',
[
'workflow_transition' => 'foo-transition',
],
],
'State add' => [
'add-state',
'add-state',
],
'State update' => [
'update-state',
'update-state:bar-state',
[
'workflow_state' => 'bar-state',
],
],
'State delete' => [
'delete-state',
'delete-state:bar-state',
[
'workflow_state' => 'bar-state',
],
],
];
}
/**
* @covers ::access
*/
public function testMissingRouteParams() {
$workflow = $this->prophesize(WorkflowInterface::class);
$workflow->access()->shouldNotBeCalled();
$route = new Route(NULL, [
'workflow' => NULL,
'workflow_state' => NULL,
], [
'_workflow_access' => 'update-state',
]);
$access_check = new WorkflowStateTransitionOperationsAccessCheck();
$account = $this->prophesize(AccountInterface::class);
$missing_both = new RouteMatch(NULL, $route, []);
$this->assertEquals(AccessResult::neutral(), $access_check->access($missing_both, $account->reveal()));
$missing_state = new RouteMatch(NULL, $route, [
'workflow' => $workflow->reveal(),
]);
$this->assertEquals(AccessResult::neutral(), $access_check->access($missing_state, $account->reveal()));
$missing_workflow = new RouteMatch(NULL, $route, [
'workflow_state' => 'foo',
]);
$this->assertEquals(AccessResult::neutral(), $access_check->access($missing_workflow, $account->reveal()));
}
/**
* @covers ::access
* @dataProvider invalidOperationNameTestCases
*/
public function testInvalidOperationName($operation_name) {
$this->setExpectedException(\Exception::class, "Invalid _workflow_access operation '$operation_name' specified for route 'Foo Route'.");
$route = new Route(NULL, [], [
'_workflow_access' => $operation_name,
]);
$access_check = new WorkflowStateTransitionOperationsAccessCheck();
$account = $this->prophesize(AccountInterface::class);
$access_check->access(new RouteMatch('Foo Route', $route, []), $account->reveal());
}
/**
* Test cases for ::testInvalidOperationName.
*/
public function invalidOperationNameTestCases() {
return [
['invalid-op'],
['foo-add-transition'],
['add-transition-bar'],
];
}
/**
* @covers \Drupal\workflows\WorkflowDeleteAccessCheck::access
* @expectedDeprecation Using the _workflow_state_delete_access check is deprecated in Drupal 8.6.0 and will be removed before Drupal 9.0.0, use _workflow_access instead. As an internal API _workflow_state_delete_access may also be removed in a minor release.
* @group legacy
*/
public function testLegacyWorkflowStateDeleteAccessCheck() {
$workflow_entity_access_result = AccessResult::allowed();
// When using the legacy access check, passing a route with a state called
// 'foo-state' will result in an entity access check of
// 'delete-state:foo-state'.
$workflow = $this->prophesize(WorkflowInterface::class);
$workflow->access('delete-state:foo-state', Argument::type(AccountInterface::class), TRUE)
->shouldBeCalled()
->willReturn($workflow_entity_access_result);
$route = new Route(NULL, [
'workflow' => NULL,
'workflow_state' => NULL,
], ['_workflow_state_delete_access' => 'true']);
$route_match = new RouteMatch(NULL, $route, [
'workflow' => $workflow->reveal(),
'workflow_state' => 'foo-state',
]);
$access_check = new WorkflowDeleteAccessCheck();
$this->assertEquals($workflow_entity_access_result, $access_check->access($route_match, $this->prophesize(AccountInterface::class)->reveal()));
}
}
......@@ -4,7 +4,7 @@ entity.workflow.add_state_form:
_entity_form: 'workflow.add-state'
_title: 'Add state'
requirements:
_entity_access: 'workflow.edit'
_workflow_access: 'add-state'
entity.workflow.edit_state_form:
path: '/admin/config/workflow/workflows/manage/{workflow}/state/{workflow_state}'
......@@ -12,7 +12,7 @@ entity.workflow.edit_state_form:
_entity_form: 'workflow.edit-state'
_title: 'Edit state'
requirements:
_entity_access: 'workflow.edit'
_workflow_access: 'update-state'
entity.workflow.delete_state_form:
path: '/admin/config/workflow/workflows/manage/{workflow}/state/{workflow_state}/delete'
......@@ -20,7 +20,7 @@ entity.workflow.delete_state_form:
_form: '\Drupal\workflows\Form\WorkflowStateDeleteForm'
_title: 'Delete state'
requirements:
_workflow_state_delete_access: 'true'
_workflow_access: 'delete-state'
entity.workflow.add_transition_form:
path: '/admin/config/workflow/workflows/manage/{workflow}/add_transition'
......@@ -28,7 +28,7 @@ entity.workflow.add_transition_form:
_entity_form: 'workflow.add-transition'
_title: 'Add transition'
requirements:
_entity_access: 'workflow.edit'
_workflow_access: 'add-transition'
entity.workflow.edit_transition_form:
path: '/admin/config/workflow/workflows/manage/{workflow}/transition/{workflow_transition}'
......@@ -36,7 +36,7 @@ entity.workflow.edit_transition_form:
_entity_form: 'workflow.edit-transition'
_title: 'Edit transition'
requirements:
_entity_access: 'workflow.edit'
_workflow_access: 'update-transition'
entity.workflow.delete_transition_form:
path: '/admin/config/workflow/workflows/manage/{workflow}/transition/{workflow_transition}/delete'
......@@ -44,4 +44,4 @@ entity.workflow.delete_transition_form: