From 8b7aeaa761606b7095693dc13a917aea88115ae2 Mon Sep 17 00:00:00 2001 From: Ken Rickard Date: Wed, 10 Apr 2019 13:01:50 -0700 Subject: [PATCH 01/39] Adds protection module code for testiong. --- .../workbench_access_protect.info.yml | 7 +++++ .../workbench_access_protect.module | 27 +++++++++++++++++++ 2 files changed, 34 insertions(+) create mode 100644 workbench_access_protect/workbench_access_protect.info.yml create mode 100644 workbench_access_protect/workbench_access_protect.module diff --git a/workbench_access_protect/workbench_access_protect.info.yml b/workbench_access_protect/workbench_access_protect.info.yml new file mode 100644 index 00000000..863aacc4 --- /dev/null +++ b/workbench_access_protect/workbench_access_protect.info.yml @@ -0,0 +1,7 @@ +name: Workbench Access Protect +type: module +description: Prevents deletion +core: 8.x +package: Workbench +dependencies: + - workbench_access:workbench_access diff --git a/workbench_access_protect/workbench_access_protect.module b/workbench_access_protect/workbench_access_protect.module new file mode 100644 index 00000000..afd18e35 --- /dev/null +++ b/workbench_access_protect/workbench_access_protect.module @@ -0,0 +1,27 @@ +providesBundles($entity)) { + // check all bundles... + } + else { + // check this bundle + } + } + #$manager = \Drupal::service('plugin.manager.workbench_access.scheme'); +} From 605e51e5f6a46cba87d00bd203dae3963e4c6aae Mon Sep 17 00:00:00 2001 From: Jason Partyka <4501353+jasonpartyka@users.noreply.github.com> Date: Wed, 10 Apr 2019 15:22:18 -0700 Subject: [PATCH 02/39] Added more scaffold. --- .../src/Access/DeleteAccessCheckInterface.php | 51 ++++ .../src/Access/TaxonomyDeleteAccessCheck.php | 264 ++++++++++++++++++ .../workbench_access_protect.module | 22 +- .../workbench_access_protect.services.yml | 4 + 4 files changed, 337 insertions(+), 4 deletions(-) create mode 100644 workbench_access_protect/src/Access/DeleteAccessCheckInterface.php create mode 100644 workbench_access_protect/src/Access/TaxonomyDeleteAccessCheck.php create mode 100644 workbench_access_protect/workbench_access_protect.services.yml diff --git a/workbench_access_protect/src/Access/DeleteAccessCheckInterface.php b/workbench_access_protect/src/Access/DeleteAccessCheckInterface.php new file mode 100644 index 00000000..0346945d --- /dev/null +++ b/workbench_access_protect/src/Access/DeleteAccessCheckInterface.php @@ -0,0 +1,51 @@ +account = $account; + $this->route = $route; + $this->userSectionStorage = $userSectionStorage; + $this->fieldManager = $fieldManager; + $this->messenger = $messenger; + $this->entityTypeManager = $entityTypeManager; + + } + + /** + * This method is used to determine if it is OK to delete. + * + * The check is based on whether or not it is being actively used for access + * control, and if content is assigned to it. If either of these statements + * is true, then 'forbidden' will be returned to prevent the term + * from being deleted. + * + * @return \Drupal\Core\Access\AccessResultAllowed|\Drupal\Core\Access\AccessResultForbidden + * Returns 'forbidden' if the term is being used for access control. + * Returns 'allowed' if the term is not being used for access control. + * + * @throws \Drupal\Component\Plugin\Exception\InvalidPluginDefinitionException + * @throws \Drupal\Component\Plugin\Exception\PluginNotFoundException + */ + public function access() { + + if ($this->isDeleteAllowed()) { + return AccessResult::allowed(); + } + + return AccessResult::forbidden(); + } + + /** + * @{inheritdoc} + */ + public function isDeleteAllowed(EntityInterface $term) { + $retval = TRUE; + + if ($term instanceof TermInterface) { + /* + * Check to see if this user has stock permission to delete this term. + * If not, there are no checks to do and return control. + */ + $user_may_delete = $this->account->hasPermission('delete terms in ' . $term->bundle()); + if ($user_may_delete === FALSE) { + $retval = FALSE; + } + if ($user_may_delete) { + + $hasAccessControlMembers = $this->doesTermHaveMembers($term); + $assigned_content = $this->isAssignedToContent($term); + + /* + * If this term does not have users assigned to it for access + * control, and the term is not assigned to any pieces of content, + * it is OK to delete it. + */ + if ($hasAccessControlMembers || $assigned_content) { + + $override_allowed = $this->account->hasPermission('allow taxonomy term delete'); + + if ($assigned_content && !$override_allowed) { + $this->messages[] = t("The term %term is being used to tag content and may not be deleted.", + ['%term' => $term->getName()]); + $retval = FALSE; + } + elseif ($assigned_content) { + $this->messages[] = t("The term %term is being used to tag content.", + ['%term' => $term->getName()]); + $retval = TRUE; + } + + if ($hasAccessControlMembers && !$override_allowed) { + $this->messages[] = t("The term %term is being used for access control and may not be deleted.", + ['%term' => $term->getName()]); + $retval = FALSE; + } + } + } + } + + return $retval; + } + + public function hasBundles(EntityInterface $term) { + if ($term->bundle() === 'term') { + return FALSE; + } + return TRUE; + } + + public function getBundles() { + return \Drupal::service('entity_type.bundle.info')->getBundleInfo('taxonomy'); + } + + /** + * @{inheritdoc} + */ + public function checkBundle(EntityInterface $term) { + // TODO: Implement checkBundle() method. + } + + + /** + * Determines if this term has active members in it. + * + * @return bool + * TRUE if the term has members, FALSE otherwise. + * + * @throws \Drupal\Component\Plugin\Exception\InvalidPluginDefinitionException + * @throws \Drupal\Component\Plugin\Exception\PluginNotFoundException + */ + private function doesTermHaveMembers(TermInterface $term) { + + /** @var array $sections */ + $sections = $this->getActiveSections($term); + + if (count($sections) > 0) { + return TRUE; + } + + return FALSE; + } + + + /** + * Inspect the given taxonomy term. + * + * This will determine if there are any active users assigned to it. + * + * @param \Drupal\taxonomy\TermInterface $term + * The Taxonomy Term to inspect. + * + * @return array + * An array of the users assigned to this section. + * + * @throws \Drupal\Component\Plugin\Exception\InvalidPluginDefinitionException + * @throws \Drupal\Component\Plugin\Exception\PluginNotFoundException + */ + private function getActiveSections(TermInterface $term) { + /** @var \Drupal\workbench_access\UserSectionStorageInterface $sectionStorage */ + $sectionStorage = $this->userSectionStorage; + + $editors = array_reduce($this->entityTypeManager->getStorage('access_scheme')->loadMultiple(), + function (array $editors, AccessSchemeInterface $scheme) use ($sectionStorage, $term) { + $editors += $sectionStorage->getEditors($scheme, $term->id()); + return $editors; + }, []); + + return $editors; + } + + /** + * Determine if tagged content exists. + * + * This method will determine if any entities exist in the system that are + * tagged with the term. + * + * @param \Drupal\taxonomy\TermInterface $term + * The Taxonomy Term to inspect. + * + * @return bool + * TRUE if content is assigned to this term. + * FALSE if content is not assigned to this term. + * + * @throws \Drupal\Component\Plugin\Exception\InvalidPluginDefinitionException + * @throws \Drupal\Component\Plugin\Exception\PluginNotFoundException + */ + private function isAssignedToContent(TermInterface $term) { + + $map = $this->fieldManager->getFieldMap(); + + foreach ($map as $entity_type => $fields) { + foreach ($fields as $name => $field) { + if ($field['type'] == 'entity_reference') { + // Get the entity reference and determine if it's a taxonomy. + /** @var \Drupal\field\Entity\FieldStorageConfig $fieldConfig */ + $fieldConfig = FieldStorageConfig::loadByName($entity_type, $name); + if ($fieldConfig instanceof FieldStorageConfig && + $fieldConfig->getSettings()['target_type'] === 'taxonomy_term') { + $entities = \Drupal::entityTypeManager()->getStorage($entity_type)->loadByProperties([ + $name => $term->id(), + ]); + if (count($entities) > 0) { + return TRUE; + } + } + } + } + } + + return FALSE; + + } + +} diff --git a/workbench_access_protect/workbench_access_protect.module b/workbench_access_protect/workbench_access_protect.module index afd18e35..54f0ee66 100644 --- a/workbench_access_protect/workbench_access_protect.module +++ b/workbench_access_protect/workbench_access_protect.module @@ -6,7 +6,6 @@ */ use Drupal\Core\Entity\EntityInterface; -use Drupal\Core\Access\AccessResult; use Drupal\Core\Session\AccountInterface; /** @@ -15,13 +14,28 @@ use Drupal\Core\Session\AccountInterface; function workbench_access_protect_entity_access(EntityInterface $entity, $op, AccountInterface $account) { // Return net result of all enabled access schemes. If one scheme allows // access, then it is granted. + + + /** @var Drupal\workbench_access_protect\Access\TaxonomyDeleteAccessCheck $protect */ + // Need to get the check that is relevant to our acccess type + $protect = new Drupal\workbench_access_protect\Access\TaxonomyDeleteAccessCheck(); + if ($op == 'delete') { if ($protect->providesBundles($entity)) { - // check all bundles... + foreach ($protect->getBundles() as $bundle) { + if ($protect->checkBundle($bundle) === FALSE) { + return \Drupal\Core\Access\AccessResult::forbidden(); + } + } } else { - // check this bundle + // We're at the leaf entity and can check the specific entity. + if ($protect->isDeleteAllowed($entity) === FALSE) { + return \Drupal\Core\Access\AccessResult::forbidden(); + } + } } - #$manager = \Drupal::service('plugin.manager.workbench_access.scheme'); + + return \Drupal\Core\Access\AccessResult::neutral(); } diff --git a/workbench_access_protect/workbench_access_protect.services.yml b/workbench_access_protect/workbench_access_protect.services.yml new file mode 100644 index 00000000..c59b10fd --- /dev/null +++ b/workbench_access_protect/workbench_access_protect.services.yml @@ -0,0 +1,4 @@ +services: + workbench_access_protect.taxonomy: + class: Drupal\workbench_access_protect\Access\TaxonomyDeleteAccessCheck + arguments: ['@current_user', '@current_route_match', '@workbench_access.user_section_storage', '@entity_field.manager', '@messenger', '@entity_type.manager'] From 7d8b7e7df3f1c52feafdf2154bec34beaa94fbe5 Mon Sep 17 00:00:00 2001 From: Jason Partyka <4501353+jasonpartyka@users.noreply.github.com> Date: Thu, 11 Apr 2019 20:17:36 -0700 Subject: [PATCH 03/39] Generalized check to see if there is an entity being used, and added tests. --- tests/src/Traits/WorkbenchAccessTestTrait.php | 15 ++ ...eAccessCheck.php => DeleteAccessCheck.php} | 100 +++----- .../src/Access/DeleteAccessCheckInterface.php | 16 +- .../tests/Functional/TaxonomySchemeUITest.php | 224 ++++++++++++++++++ .../workbench_access_protect.info.yml | 2 +- .../workbench_access_protect.module | 17 +- .../workbench_access_protect.services.yml | 6 +- 7 files changed, 298 insertions(+), 82 deletions(-) rename workbench_access_protect/src/Access/{TaxonomyDeleteAccessCheck.php => DeleteAccessCheck.php} (64%) create mode 100644 workbench_access_protect/tests/Functional/TaxonomySchemeUITest.php diff --git a/tests/src/Traits/WorkbenchAccessTestTrait.php b/tests/src/Traits/WorkbenchAccessTestTrait.php index c57af669..fcc1079b 100644 --- a/tests/src/Traits/WorkbenchAccessTestTrait.php +++ b/tests/src/Traits/WorkbenchAccessTestTrait.php @@ -139,6 +139,21 @@ protected function createUserWithRole($rid) { return $user; } + /** + * Sets up a user the specific permissions and a unique role. + * + * @param array $additional_permissions + * Array of additional permissions beyond 'access administration pages' and + * 'assign workbench access'. + * + * @return \Drupal\user\Entity\User + * The user entity. + */ + protected function setUpUserUniqueRole($additional_permissions) { + $role = $this->createRole($additional_permissions); + return $this->createUserWithRole($role); + } + /** * Sets up a taxonomy scheme for a given node type. * diff --git a/workbench_access_protect/src/Access/TaxonomyDeleteAccessCheck.php b/workbench_access_protect/src/Access/DeleteAccessCheck.php similarity index 64% rename from workbench_access_protect/src/Access/TaxonomyDeleteAccessCheck.php rename to workbench_access_protect/src/Access/DeleteAccessCheck.php index 2160e261..a0413567 100644 --- a/workbench_access_protect/src/Access/TaxonomyDeleteAccessCheck.php +++ b/workbench_access_protect/src/Access/DeleteAccessCheck.php @@ -1,12 +1,10 @@ account = $account; $this->route = $route; $this->userSectionStorage = $userSectionStorage; $this->fieldManager = $fieldManager; @@ -101,65 +97,43 @@ public function access() { /** * @{inheritdoc} */ - public function isDeleteAllowed(EntityInterface $term) { - $retval = TRUE; + public function isDeleteAllowed(EntityInterface $entity) { - if ($term instanceof TermInterface) { - /* - * Check to see if this user has stock permission to delete this term. - * If not, there are no checks to do and return control. - */ - $user_may_delete = $this->account->hasPermission('delete terms in ' . $term->bundle()); - if ($user_may_delete === FALSE) { - $retval = FALSE; - } - if ($user_may_delete) { + $retval = TRUE; - $hasAccessControlMembers = $this->doesTermHaveMembers($term); - $assigned_content = $this->isAssignedToContent($term); + if ($entity instanceof EntityInterface) { - /* - * If this term does not have users assigned to it for access - * control, and the term is not assigned to any pieces of content, - * it is OK to delete it. - */ - if ($hasAccessControlMembers || $assigned_content) { + $hasAccessControlMembers = $this->doesTermHaveMembers($entity); + $assigned_content = $this->isAssignedToContent($entity); - $override_allowed = $this->account->hasPermission('allow taxonomy term delete'); + /* + * If this term does not have users assigned to it for access + * control, and the term is not assigned to any pieces of content, + * it is OK to delete it. + */ + if ($hasAccessControlMembers || $assigned_content) { - if ($assigned_content && !$override_allowed) { - $this->messages[] = t("The term %term is being used to tag content and may not be deleted.", - ['%term' => $term->getName()]); - $retval = FALSE; - } - elseif ($assigned_content) { - $this->messages[] = t("The term %term is being used to tag content.", - ['%term' => $term->getName()]); - $retval = TRUE; - } + if ($assigned_content) { + $retval = FALSE; + } - if ($hasAccessControlMembers && !$override_allowed) { - $this->messages[] = t("The term %term is being used for access control and may not be deleted.", - ['%term' => $term->getName()]); - $retval = FALSE; - } + if ($hasAccessControlMembers) { + $retval = FALSE; } } + } return $retval; } - public function hasBundles(EntityInterface $term) { - if ($term->bundle() === 'term') { - return FALSE; - } - return TRUE; + public function hasBundles(EntityInterface $entity) { + return \Drupal::service('entity_type.bundle.info')->getBundleInfo($entity->label()); } - public function getBundles() { - return \Drupal::service('entity_type.bundle.info')->getBundleInfo('taxonomy'); - } +// public function getBundles(EntityInterface $entity) { +// return \Drupal::service('entity_type.bundle.info')->getBundleInfo($entity->label()); +// } /** * @{inheritdoc} @@ -168,7 +142,6 @@ public function checkBundle(EntityInterface $term) { // TODO: Implement checkBundle() method. } - /** * Determines if this term has active members in it. * @@ -178,10 +151,10 @@ public function checkBundle(EntityInterface $term) { * @throws \Drupal\Component\Plugin\Exception\InvalidPluginDefinitionException * @throws \Drupal\Component\Plugin\Exception\PluginNotFoundException */ - private function doesTermHaveMembers(TermInterface $term) { + private function doesTermHaveMembers(EntityInterface $entity) { /** @var array $sections */ - $sections = $this->getActiveSections($term); + $sections = $this->getActiveSections($entity); if (count($sections) > 0) { return TRUE; @@ -196,7 +169,7 @@ private function doesTermHaveMembers(TermInterface $term) { * * This will determine if there are any active users assigned to it. * - * @param \Drupal\taxonomy\TermInterface $term + * @param \Drupal\Core\Entity\EntityInterface $entity * The Taxonomy Term to inspect. * * @return array @@ -205,13 +178,13 @@ private function doesTermHaveMembers(TermInterface $term) { * @throws \Drupal\Component\Plugin\Exception\InvalidPluginDefinitionException * @throws \Drupal\Component\Plugin\Exception\PluginNotFoundException */ - private function getActiveSections(TermInterface $term) { + private function getActiveSections(EntityInterface $entity) { /** @var \Drupal\workbench_access\UserSectionStorageInterface $sectionStorage */ $sectionStorage = $this->userSectionStorage; $editors = array_reduce($this->entityTypeManager->getStorage('access_scheme')->loadMultiple(), - function (array $editors, AccessSchemeInterface $scheme) use ($sectionStorage, $term) { - $editors += $sectionStorage->getEditors($scheme, $term->id()); + function (array $editors, AccessSchemeInterface $scheme) use ($sectionStorage, $entity) { + $editors += $sectionStorage->getEditors($scheme, $entity->id()); return $editors; }, []); @@ -224,8 +197,8 @@ function (array $editors, AccessSchemeInterface $scheme) use ($sectionStorage, $ * This method will determine if any entities exist in the system that are * tagged with the term. * - * @param \Drupal\taxonomy\TermInterface $term - * The Taxonomy Term to inspect. + * @param \Drupal\Core\Entity\EntityInterface $term + * The Entity to inspect. * * @return bool * TRUE if content is assigned to this term. @@ -234,7 +207,7 @@ function (array $editors, AccessSchemeInterface $scheme) use ($sectionStorage, $ * @throws \Drupal\Component\Plugin\Exception\InvalidPluginDefinitionException * @throws \Drupal\Component\Plugin\Exception\PluginNotFoundException */ - private function isAssignedToContent(TermInterface $term) { + private function isAssignedToContent(EntityInterface $entity) { $map = $this->fieldManager->getFieldMap(); @@ -244,10 +217,9 @@ private function isAssignedToContent(TermInterface $term) { // Get the entity reference and determine if it's a taxonomy. /** @var \Drupal\field\Entity\FieldStorageConfig $fieldConfig */ $fieldConfig = FieldStorageConfig::loadByName($entity_type, $name); - if ($fieldConfig instanceof FieldStorageConfig && - $fieldConfig->getSettings()['target_type'] === 'taxonomy_term') { + if ($fieldConfig instanceof FieldStorageConfig) { $entities = \Drupal::entityTypeManager()->getStorage($entity_type)->loadByProperties([ - $name => $term->id(), + $name => $entity->id(), ]); if (count($entities) > 0) { return TRUE; diff --git a/workbench_access_protect/src/Access/DeleteAccessCheckInterface.php b/workbench_access_protect/src/Access/DeleteAccessCheckInterface.php index 0346945d..973097ce 100644 --- a/workbench_access_protect/src/Access/DeleteAccessCheckInterface.php +++ b/workbench_access_protect/src/Access/DeleteAccessCheckInterface.php @@ -1,6 +1,6 @@ createContentType(['type' => 'page']); + $this->createContentType(['type' => 'article']); + $this->vocabulary = $this->setUpVocabulary(); + $this->setUpTaxonomyFieldForEntityType('node', 'page', $this->vocabulary->id()); + $this->setUpTaxonomyFieldForEntityType('taxonomy_term', $this->vocabulary->id(), $this->vocabulary->id(), 'recursive', 'Recursive Field'); + $vocab = Vocabulary::create(['vid' => 'selected', 'name' => 'Selected Vocabulary']); + $vocab->save(); + $this->setUpTaxonomyFieldForEntityType('taxonomy_term', $vocab->id(), $this->vocabulary->id(), 'non_recursive', 'Allowed Field'); + entity_test_create_bundle('access_controlled'); + entity_test_create_bundle('notaccess_controlled'); + $this->setUpTaxonomyFieldForEntityType('entity_test', 'access_controlled', $this->vocabulary->id()); + $this->admin = $this->setUpAdminUser([ + 'administer workbench access', + 'edit terms in workbench_access', + 'delete terms in workbench_access', + 'create terms in workbench_access', + ]); + $this->admin2 = $this->setUpUserUniqueRole([ + 'administer workbench access', + 'assign workbench access', + 'edit terms in workbench_access', + 'delete terms in workbench_access', + 'create terms in workbench_access', + 'access administration pages', + 'access taxonomy overview', + 'administer taxonomy', + ]); + $this->placeBlock('local_actions_block'); + + $this->setUpTestContent(); + + + + } + + // protected function tearDown() { + // parent::tearDown(); + // // $this->term->delete(); + // $this->testNode->delete(); + // } + + protected function setUpTestContent() { + // create a test term + $this->term = Term::create([ + 'name' => 'Test Term', + 'vid' => $this->vocabulary->id(), + ]); + + $this->term->save(); + + $this->emptyTerm = Term::create([ + 'name' => 'Empty Test Term', + 'vid' => $this->vocabulary->id(), + ]); + + $this->emptyTerm->save(); + + + // create some test nodes + $this->testNode = $this->createNode( + [ + 'title' => 'Node', + 'type' => 'page', + 'uid' => $this->admin->id(), + WorkbenchAccessManagerInterface::FIELD_NAME => $this->term->id(), + ] + ); + + $this->testNode->save(); + } + + /** + * Asset a non-administrator cannot delete terms that are actively used + * + * @throws \Behat\Mink\Exception\ExpectationException + */ + public function testAssertCannotDeleteTaxonomy() { + + // Switch user to the non-privileged account. + $this->drupalLogin($this->admin2); + + $path = '/taxonomy/term/' . $this->term->id() . '/edit'; + $this->drupalGet($path); + $this->assertSession()->linkNotExists("Delete"); + + $delete_path = '/taxonomy/term/' . $this->term->id() . '/delete'; + $this->drupalGet($delete_path); + $this->assertSession()->statusCodeEquals(403); + + // Test the overview page to make sure that a delete is not present. + $vocab_path = '/admin/structure/taxonomy/manage/' . $this->vocabulary->id() . '/overview'; + $this->drupalGet($vocab_path); + $delete_path = '/admin/structure/taxonomy/manage/' . $this->vocabulary->id() . '/delete'; + $this->assertSession()->linkByHrefNotExists($delete_path); + + // Switch user back to the privileged account. + $this->drupalLogout(); + } + + /** + * Assert that anonymous visitors are not told anything additional about + * this term, besides the existence of the URL as provided by Drupal. + * + * Just a sanity check to help check we didn't break anything. + * + * @throws \Behat\Mink\Exception\ExpectationException + */ + public function testAssertAnonymousGets403() { + $delete_path = '/taxonomy/term/' . $this->term->id() . '/delete'; + $this->drupalGet($delete_path); + $this->assertSession()->statusCodeEquals(403); + } + + public function testAssertUntaggedTermsMayBeDeleted() { + // Switch user to the non-privileged account. + + $this->drupalLogin($this->admin2); + + $path = '/taxonomy/term/' . $this->emptyTerm->id() . '/edit'; + $this->drupalGet($path); + $this->assertSession()->linkExists("Delete"); + + $delete_path = '/taxonomy/term/' . $this->emptyTerm->id() . '/delete'; + + $this->drupalGet($delete_path); + $this->assertSession()->statusCodeEquals(200); + + // Test the overview page to make sure that a delete is present. + $vocab_path = '/admin/structure/taxonomy/manage/' . $this->vocabulary->id(); + $delete_path = '/admin/structure/taxonomy/manage/' . $this->vocabulary->id() . '/delete'; + $this->drupalGet($vocab_path); + $this->assertSession()->linkByHrefExists($delete_path); + + // Switch user back to the privileged account. + $this->drupalLogout(); + $this->drupalLogin($this->admin); + } + + + +} diff --git a/workbench_access_protect/workbench_access_protect.info.yml b/workbench_access_protect/workbench_access_protect.info.yml index 863aacc4..7e5a2665 100644 --- a/workbench_access_protect/workbench_access_protect.info.yml +++ b/workbench_access_protect/workbench_access_protect.info.yml @@ -1,6 +1,6 @@ name: Workbench Access Protect type: module -description: Prevents deletion +description: Enable `Workbench Access Protect` to prevent entities from being core: 8.x package: Workbench dependencies: diff --git a/workbench_access_protect/workbench_access_protect.module b/workbench_access_protect/workbench_access_protect.module index 54f0ee66..7a94bbd0 100644 --- a/workbench_access_protect/workbench_access_protect.module +++ b/workbench_access_protect/workbench_access_protect.module @@ -7,6 +7,8 @@ use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Session\AccountInterface; +use Drupal\Core\Access\AccessResult; +use Drupal\workbench_access_protect\Access\Taxonomy; /** * Implements hook_entity_access(). @@ -15,27 +17,26 @@ function workbench_access_protect_entity_access(EntityInterface $entity, $op, Ac // Return net result of all enabled access schemes. If one scheme allows // access, then it is granted. - - /** @var Drupal\workbench_access_protect\Access\TaxonomyDeleteAccessCheck $protect */ - // Need to get the check that is relevant to our acccess type - $protect = new Drupal\workbench_access_protect\Access\TaxonomyDeleteAccessCheck(); + /** @var Drupal\workbench_access_protect\Access\DeleteAccessCheck $protect */ + $protect = \Drupal::service('workbench_access_protect.delete'); if ($op == 'delete') { - if ($protect->providesBundles($entity)) { + if ($protect->hasBundles($entity)) { foreach ($protect->getBundles() as $bundle) { if ($protect->checkBundle($bundle) === FALSE) { - return \Drupal\Core\Access\AccessResult::forbidden(); + return AccessResult::forbidden(); } } } else { // We're at the leaf entity and can check the specific entity. if ($protect->isDeleteAllowed($entity) === FALSE) { - return \Drupal\Core\Access\AccessResult::forbidden(); + return AccessResult::forbidden(); } } } - return \Drupal\Core\Access\AccessResult::neutral(); + return AccessResult::neutral(); + } diff --git a/workbench_access_protect/workbench_access_protect.services.yml b/workbench_access_protect/workbench_access_protect.services.yml index c59b10fd..be8f682a 100644 --- a/workbench_access_protect/workbench_access_protect.services.yml +++ b/workbench_access_protect/workbench_access_protect.services.yml @@ -1,4 +1,4 @@ services: - workbench_access_protect.taxonomy: - class: Drupal\workbench_access_protect\Access\TaxonomyDeleteAccessCheck - arguments: ['@current_user', '@current_route_match', '@workbench_access.user_section_storage', '@entity_field.manager', '@messenger', '@entity_type.manager'] + workbench_access_protect.delete : + class: Drupal\workbench_access_protect\Access\DeleteAccessCheck + arguments: ['@current_route_match', '@workbench_access.user_section_storage', '@entity_field.manager', '@messenger', '@entity_type.manager'] From 86f186906b6db215bc6e0749fd67180c82e391f0 Mon Sep 17 00:00:00 2001 From: Jason Partyka <4501353+jasonpartyka@users.noreply.github.com> Date: Thu, 11 Apr 2019 20:33:24 -0700 Subject: [PATCH 04/39] Added functional tests for workbench_access_protect. --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index 8421d841..3a825bfa 100644 --- a/.travis.yml +++ b/.travis.yml @@ -77,3 +77,4 @@ script: - php core/scripts/run-tests.sh --suppress-deprecations --verbose --color --concurrency 4 --types "PHPUnit-Functional" --php `which php` --url http://localhost:8080 "workbench_access" - php core/scripts/run-tests.sh --suppress-deprecations --verbose --color --concurrency 4 --types "PHPUnit-Kernel" --php `which php` --url http://localhost:8080 "workbench_access" - php core/scripts/run-tests.sh --suppress-deprecations --verbose --color --concurrency 4 --types "PHPUnit-Unit" --php `which php` --url http://localhost:8080 "workbench_access" + - php core/scripts/run-tests.sh --suppress-deprecations --verbose --color --concurrency 4 --types "PHPUnit-Functional" --php `which php` --url http://localhost:8080 "workbench_access_protect" From 99f3018ffb1bfee0ca0cf0132509c2042f004d28 Mon Sep 17 00:00:00 2001 From: Jason Partyka <4501353+jasonpartyka@users.noreply.github.com> Date: Thu, 11 Apr 2019 20:44:32 -0700 Subject: [PATCH 05/39] Changed test run command for workbench access protect. --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 3a825bfa..828647d2 100644 --- a/.travis.yml +++ b/.travis.yml @@ -77,4 +77,4 @@ script: - php core/scripts/run-tests.sh --suppress-deprecations --verbose --color --concurrency 4 --types "PHPUnit-Functional" --php `which php` --url http://localhost:8080 "workbench_access" - php core/scripts/run-tests.sh --suppress-deprecations --verbose --color --concurrency 4 --types "PHPUnit-Kernel" --php `which php` --url http://localhost:8080 "workbench_access" - php core/scripts/run-tests.sh --suppress-deprecations --verbose --color --concurrency 4 --types "PHPUnit-Unit" --php `which php` --url http://localhost:8080 "workbench_access" - - php core/scripts/run-tests.sh --suppress-deprecations --verbose --color --concurrency 4 --types "PHPUnit-Functional" --php `which php` --url http://localhost:8080 "workbench_access_protect" + - php core/scripts/run-tests.sh --suppress-deprecations --verbose --color --concurrency 4 --types --php `which php` --url http://localhost:8080 --class "Drupal\block\Tests\BlockTest" From 57b75e901e155d6d5471699936265300ef344667 Mon Sep 17 00:00:00 2001 From: Jason Partyka <4501353+jasonpartyka@users.noreply.github.com> Date: Thu, 11 Apr 2019 20:51:45 -0700 Subject: [PATCH 06/39] Changed test run command for workbench access protect. --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 828647d2..c510ffce 100644 --- a/.travis.yml +++ b/.travis.yml @@ -77,4 +77,4 @@ script: - php core/scripts/run-tests.sh --suppress-deprecations --verbose --color --concurrency 4 --types "PHPUnit-Functional" --php `which php` --url http://localhost:8080 "workbench_access" - php core/scripts/run-tests.sh --suppress-deprecations --verbose --color --concurrency 4 --types "PHPUnit-Kernel" --php `which php` --url http://localhost:8080 "workbench_access" - php core/scripts/run-tests.sh --suppress-deprecations --verbose --color --concurrency 4 --types "PHPUnit-Unit" --php `which php` --url http://localhost:8080 "workbench_access" - - php core/scripts/run-tests.sh --suppress-deprecations --verbose --color --concurrency 4 --types --php `which php` --url http://localhost:8080 --class "Drupal\block\Tests\BlockTest" + - php core/scripts/run-tests.sh --suppress-deprecations --verbose --color --concurrency 4 --php `which php` --url http://localhost:8080 --class "Drupal\Tests\workbench_access_protect\Functional\TaxonomySchemeUITest" From e0d8a7a0e783d6543998100eaf28ed83344a8105 Mon Sep 17 00:00:00 2001 From: Jason Partyka <4501353+jasonpartyka@users.noreply.github.com> Date: Thu, 11 Apr 2019 20:52:07 -0700 Subject: [PATCH 07/39] Changed test run command for workbench access protect. --- .../tests/Functional/TaxonomySchemeUITest.php | 1 - 1 file changed, 1 deletion(-) diff --git a/workbench_access_protect/tests/Functional/TaxonomySchemeUITest.php b/workbench_access_protect/tests/Functional/TaxonomySchemeUITest.php index 748fde0b..c52ec3cf 100644 --- a/workbench_access_protect/tests/Functional/TaxonomySchemeUITest.php +++ b/workbench_access_protect/tests/Functional/TaxonomySchemeUITest.php @@ -4,7 +4,6 @@ use Drupal\Tests\BrowserTestBase; use Drupal\taxonomy\Entity\Vocabulary; -use Drupal\workbench_access\Entity\AccessSchemeInterface; use Drupal\Tests\workbench_access\Traits\WorkbenchAccessTestTrait; use Drupal\taxonomy\Entity\Term; use Drupal\workbench_access\WorkbenchAccessManagerInterface; From e1c695f9f7c7f1f4bba5d20b7faa2d8626eceb57 Mon Sep 17 00:00:00 2001 From: Jason Partyka <4501353+jasonpartyka@users.noreply.github.com> Date: Thu, 11 Apr 2019 21:19:56 -0700 Subject: [PATCH 08/39] Changed test run command for workbench access protect. --- .travis.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index c510ffce..341fff01 100644 --- a/.travis.yml +++ b/.travis.yml @@ -64,7 +64,7 @@ before_script: - /usr/bin/env PHP_OPTIONS="-d sendmail_path=$(which true)" drush --yes --verbose site-install minimal --db-url=mysql://root:@127.0.0.1/wa # Install modules - travis_retry drush dl inline_entity_form - - drush --yes en simpletest workbench_access taxonomy + - drush --yes en simpletest workbench_access taxonomy workbench_access_protect - drush cr # Start a web server on port 8080 in the background. @@ -77,4 +77,4 @@ script: - php core/scripts/run-tests.sh --suppress-deprecations --verbose --color --concurrency 4 --types "PHPUnit-Functional" --php `which php` --url http://localhost:8080 "workbench_access" - php core/scripts/run-tests.sh --suppress-deprecations --verbose --color --concurrency 4 --types "PHPUnit-Kernel" --php `which php` --url http://localhost:8080 "workbench_access" - php core/scripts/run-tests.sh --suppress-deprecations --verbose --color --concurrency 4 --types "PHPUnit-Unit" --php `which php` --url http://localhost:8080 "workbench_access" - - php core/scripts/run-tests.sh --suppress-deprecations --verbose --color --concurrency 4 --php `which php` --url http://localhost:8080 --class "Drupal\Tests\workbench_access_protect\Functional\TaxonomySchemeUITest" + - vendor/bin/phpunit -c core modules/contrib/workbench_access/workbench_access_protect/tests/Functional/TaxonomySchemeUITest.php From 096b9bcbf51b2fbb5b0f955c91476d603eda8471 Mon Sep 17 00:00:00 2001 From: Jason Partyka <4501353+jasonpartyka@users.noreply.github.com> Date: Thu, 11 Apr 2019 21:28:59 -0700 Subject: [PATCH 09/39] Changed test run command for workbench access protect. --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 341fff01..8a995205 100644 --- a/.travis.yml +++ b/.travis.yml @@ -77,4 +77,4 @@ script: - php core/scripts/run-tests.sh --suppress-deprecations --verbose --color --concurrency 4 --types "PHPUnit-Functional" --php `which php` --url http://localhost:8080 "workbench_access" - php core/scripts/run-tests.sh --suppress-deprecations --verbose --color --concurrency 4 --types "PHPUnit-Kernel" --php `which php` --url http://localhost:8080 "workbench_access" - php core/scripts/run-tests.sh --suppress-deprecations --verbose --color --concurrency 4 --types "PHPUnit-Unit" --php `which php` --url http://localhost:8080 "workbench_access" - - vendor/bin/phpunit -c core modules/contrib/workbench_access/workbench_access_protect/tests/Functional/TaxonomySchemeUITest.php + - vendor/bin/phpunit -c core "modules/contrib/workbench_access/workbench_access_protect/tests/Functional/TaxonomySchemeUITest.php" From ceaa67d456b4ef320b4294308cb434b05bdfd1df Mon Sep 17 00:00:00 2001 From: Jason Partyka <4501353+jasonpartyka@users.noreply.github.com> Date: Sun, 14 Apr 2019 06:58:37 -0500 Subject: [PATCH 10/39] Changed test group & travis conifg. --- .travis.yml | 2 +- .../tests/Functional/TaxonomySchemeUITest.php | 8 +------- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/.travis.yml b/.travis.yml index 8a995205..d1c0f64f 100644 --- a/.travis.yml +++ b/.travis.yml @@ -77,4 +77,4 @@ script: - php core/scripts/run-tests.sh --suppress-deprecations --verbose --color --concurrency 4 --types "PHPUnit-Functional" --php `which php` --url http://localhost:8080 "workbench_access" - php core/scripts/run-tests.sh --suppress-deprecations --verbose --color --concurrency 4 --types "PHPUnit-Kernel" --php `which php` --url http://localhost:8080 "workbench_access" - php core/scripts/run-tests.sh --suppress-deprecations --verbose --color --concurrency 4 --types "PHPUnit-Unit" --php `which php` --url http://localhost:8080 "workbench_access" - - vendor/bin/phpunit -c core "modules/contrib/workbench_access/workbench_access_protect/tests/Functional/TaxonomySchemeUITest.php" + - php core/scripts/run-tests.sh --suppress-deprecations --verbose --color --concurrency 4 --types "PHPUnit-Unit" --php `which php` --url http://localhost:8080 "workbench_access_protect" diff --git a/workbench_access_protect/tests/Functional/TaxonomySchemeUITest.php b/workbench_access_protect/tests/Functional/TaxonomySchemeUITest.php index c52ec3cf..6233ed0b 100644 --- a/workbench_access_protect/tests/Functional/TaxonomySchemeUITest.php +++ b/workbench_access_protect/tests/Functional/TaxonomySchemeUITest.php @@ -11,7 +11,7 @@ /** * Defines a class for testing the UI to create and configure schemes. * - * @group workbench_access + * @group workbench_access_protect */ class TaxonomySchemeUITest extends BrowserTestBase { @@ -115,12 +115,6 @@ protected function setUp() { } - // protected function tearDown() { - // parent::tearDown(); - // // $this->term->delete(); - // $this->testNode->delete(); - // } - protected function setUpTestContent() { // create a test term $this->term = Term::create([ From 8f473f2bac2b723b9621cf6d82a3b997abf59d14 Mon Sep 17 00:00:00 2001 From: Jason Partyka <4501353+jasonpartyka@users.noreply.github.com> Date: Tue, 7 May 2019 14:31:04 -0500 Subject: [PATCH 11/39] Added check for bundle-wide usage. TODO: Limit the bundles we are checking just to the one used for access control. --- .../src/Access/DeleteAccessCheck.php | 72 +++-- .../src/Access/DeleteAccessCheckInterface.php | 24 +- .../workbench_access_protect.info.yml | 2 +- .../workbench_access_protect.module | 20 +- .../workbench_access_protect.services.yml | 2 +- .../src/Access/DeleteAccessCheck.php | 282 ++++++++++++++++++ .../src/Access/DeleteAccessCheckInterface.php | 61 ++++ .../tests/Functional/TaxonomySchemeUITest.php | 217 ++++++++++++++ .../workbench_access_protect.info.yml | 7 + .../workbench_access_protect.module | 42 +++ .../workbench_access_protect.services.yml | 4 + 11 files changed, 685 insertions(+), 48 deletions(-) create mode 100644 workbench_access_protect/workbench_access_protect/src/Access/DeleteAccessCheck.php create mode 100644 workbench_access_protect/workbench_access_protect/src/Access/DeleteAccessCheckInterface.php create mode 100644 workbench_access_protect/workbench_access_protect/tests/Functional/TaxonomySchemeUITest.php create mode 100644 workbench_access_protect/workbench_access_protect/workbench_access_protect.info.yml create mode 100644 workbench_access_protect/workbench_access_protect/workbench_access_protect.module create mode 100644 workbench_access_protect/workbench_access_protect/workbench_access_protect.services.yml diff --git a/workbench_access_protect/src/Access/DeleteAccessCheck.php b/workbench_access_protect/src/Access/DeleteAccessCheck.php index a0413567..013704fd 100644 --- a/workbench_access_protect/src/Access/DeleteAccessCheck.php +++ b/workbench_access_protect/src/Access/DeleteAccessCheck.php @@ -3,6 +3,7 @@ namespace Drupal\workbench_access_protect\Access; use Drupal\Core\Access\AccessResult; +use Drupal\Core\Entity\EntityTypeBundleInfoInterface; use Drupal\Core\Entity\EntityTypeManager; use Drupal\Core\Entity\EntityInterface; use Drupal\field\Entity\FieldStorageConfig; @@ -14,8 +15,6 @@ /** * Class TaxonomyDeleteAccessCheck. - * - * @package Drupal\workbench_access\Access */ class DeleteAccessCheck implements DeleteAccessCheckInterface { @@ -39,34 +38,28 @@ class DeleteAccessCheck implements DeleteAccessCheckInterface { */ private $fieldManager; - /** - * @var \Drupal\Core\Messenger\MessengerInterface + * @var \Drupal\Core\Entity\EntityTypeManager */ - private $messenger; + private $entityTypeManager; /** - * @var array - * A list of messages to be printed when access denied. + * @var \Drupal\Core\Entity\EntityTypeBundleInfoInterface */ - private $messages = []; + private $entityTypeBundleInfo; - /** - * @var \Drupal\Core\Entity\EntityTypeManager - */ - private $entityTypeManager; public function __construct(CurrentRouteMatch $route, UserSectionStorage $userSectionStorage, EntityFieldManager $fieldManager, - MessengerInterface $messenger, - EntityTypeManager $entityTypeManager) { + EntityTypeManager $entityTypeManager, + EntityTypeBundleInfoInterface $entityTypeBundleInfo) { $this->route = $route; $this->userSectionStorage = $userSectionStorage; $this->fieldManager = $fieldManager; - $this->messenger = $messenger; $this->entityTypeManager = $entityTypeManager; + $this->entityTypeBundleInfo = $entityTypeBundleInfo; } @@ -107,7 +100,7 @@ public function isDeleteAllowed(EntityInterface $entity) { $assigned_content = $this->isAssignedToContent($entity); /* - * If this term does not have users assigned to it for access + * If this entity does not have users assigned to it for access * control, and the term is not assigned to any pieces of content, * it is OK to delete it. */ @@ -127,24 +120,49 @@ public function isDeleteAllowed(EntityInterface $entity) { return $retval; } - public function hasBundles(EntityInterface $entity) { - return \Drupal::service('entity_type.bundle.info')->getBundleInfo($entity->label()); + public function getBundles(EntityInterface $entity) { + $bundle = $this->entityTypeManager->getDefinitions()[$entity->getEntityTypeId()]->get('bundle_of'); + $bundles = $this->entityTypeBundleInfo->getBundleInfo($bundle); + return array_combine(array_keys($bundles), array_keys($bundles)); + } -// public function getBundles(EntityInterface $entity) { -// return \Drupal::service('entity_type.bundle.info')->getBundleInfo($entity->label()); -// } + public function hasBundles(EntityInterface $entity) { + if ($this->entityTypeManager->getDefinitions()[$entity->getEntityTypeId()]->get('bundle_of') == null) { + return FALSE; + } + return TRUE; + } /** - * @{inheritdoc} + * {@inheritDoc} */ - public function checkBundle(EntityInterface $term) { - // TODO: Implement checkBundle() method. + public function isDeleteAllowedBundle($bundle, $entity){ + $bundle_of = $this->entityTypeManager->getDefinitions()[$entity->getEntityTypeId()]->get('bundle_of'); + $entity_id_key = $this->entityTypeManager->getDefinitions()[$entity->getEntityTypeId()]->get('entity_keys')['id']; + $entities = $this->entityTypeManager->getStorage($bundle_of)->loadByProperties( + [$entity_id_key => $bundle ] + ); + + /* + * Cycle through the entities of this bundle. As soon as one is discovered + * as being actively used for access control, we can deny delete. + */ + foreach ($entities as $bundle) { + if ($this->isDeleteAllowed($bundle) === FALSE) { + return FALSE; + } + } + + return TRUE; } /** * Determines if this term has active members in it. * + * @param \Drupal\Core\Entity\EntityInterface $entity + * The entity to inspect. + * * @return bool * TRUE if the term has members, FALSE otherwise. * @@ -170,7 +188,7 @@ private function doesTermHaveMembers(EntityInterface $entity) { * This will determine if there are any active users assigned to it. * * @param \Drupal\Core\Entity\EntityInterface $entity - * The Taxonomy Term to inspect. + * The entity to inspect. * * @return array * An array of the users assigned to this section. @@ -197,7 +215,7 @@ function (array $editors, AccessSchemeInterface $scheme) use ($sectionStorage, $ * This method will determine if any entities exist in the system that are * tagged with the term. * - * @param \Drupal\Core\Entity\EntityInterface $term + * @param \Drupal\Core\Entity\EntityInterface $entity * The Entity to inspect. * * @return bool @@ -218,7 +236,7 @@ private function isAssignedToContent(EntityInterface $entity) { /** @var \Drupal\field\Entity\FieldStorageConfig $fieldConfig */ $fieldConfig = FieldStorageConfig::loadByName($entity_type, $name); if ($fieldConfig instanceof FieldStorageConfig) { - $entities = \Drupal::entityTypeManager()->getStorage($entity_type)->loadByProperties([ + $entities = $this->entityTypeManager->getStorage($entity_type)->loadByProperties([ $name => $entity->id(), ]); if (count($entities) > 0) { diff --git a/workbench_access_protect/src/Access/DeleteAccessCheckInterface.php b/workbench_access_protect/src/Access/DeleteAccessCheckInterface.php index 973097ce..755f5da4 100644 --- a/workbench_access_protect/src/Access/DeleteAccessCheckInterface.php +++ b/workbench_access_protect/src/Access/DeleteAccessCheckInterface.php @@ -5,9 +5,7 @@ use Drupal\Core\Entity\EntityInterface; /** - * Class TaxonomyDeleteAccessCheck. - * - * @package Drupal\workbench_access\Access + * Class DeleteAccessCheck. */ interface DeleteAccessCheckInterface { @@ -39,17 +37,25 @@ public function isDeleteAllowed(EntityInterface $entity); public function hasBundles(EntityInterface $entity); /** - * Check a specific bundle to determine if it is in use or not. - * Use this to determine if this is the 'leaf' entity that actually - * needs to check to see if is deletec. + * @param \Drupal\Core\Entity\EntityInterface $entity * - * @param \Drupal\Core\Entity\EntityInterface $term + * @return mixed + */ + public function getBundles(EntityInterface $entity); + + /** + * Check an entire bundle to see if any of the bundle's + * entities are being actively used for access control. + * + * @param string $bundle + * The bundle ID + * @param string $entity + * The entity id * * @return bool * TRUE */ - public function checkBundle(EntityInterface $entity); - + public function isDeleteAllowedBundle($bundle, $entity); } diff --git a/workbench_access_protect/workbench_access_protect.info.yml b/workbench_access_protect/workbench_access_protect.info.yml index 7e5a2665..b9f6c1ba 100644 --- a/workbench_access_protect/workbench_access_protect.info.yml +++ b/workbench_access_protect/workbench_access_protect.info.yml @@ -1,6 +1,6 @@ name: Workbench Access Protect type: module -description: Enable `Workbench Access Protect` to prevent entities from being +description: Adds additional logic upon entities for delete operations. core: 8.x package: Workbench dependencies: diff --git a/workbench_access_protect/workbench_access_protect.module b/workbench_access_protect/workbench_access_protect.module index 7a94bbd0..f8cb6892 100644 --- a/workbench_access_protect/workbench_access_protect.module +++ b/workbench_access_protect/workbench_access_protect.module @@ -10,28 +10,28 @@ use Drupal\Core\Session\AccountInterface; use Drupal\Core\Access\AccessResult; use Drupal\workbench_access_protect\Access\Taxonomy; + /** * Implements hook_entity_access(). */ function workbench_access_protect_entity_access(EntityInterface $entity, $op, AccountInterface $account) { - // Return net result of all enabled access schemes. If one scheme allows - // access, then it is granted. - - /** @var Drupal\workbench_access_protect\Access\DeleteAccessCheck $protect */ - $protect = \Drupal::service('workbench_access_protect.delete'); if ($op == 'delete') { + + /** @var Drupal\workbench_access_protect\Access\DeleteAccessCheck $protect */ + $protect = \Drupal::service('workbench_access_protect.delete'); + if ($protect->hasBundles($entity)) { - foreach ($protect->getBundles() as $bundle) { - if ($protect->checkBundle($bundle) === FALSE) { - return AccessResult::forbidden(); - } + $bundle = $protect->getBundles($entity)[$entity->id()]; + // Check the entire bundle to see if any term is in use. + if ($protect->isDeleteAllowedBundle($bundle, $entity) === FALSE) { + return AccessResult::forbidden(); } } else { // We're at the leaf entity and can check the specific entity. if ($protect->isDeleteAllowed($entity) === FALSE) { - return AccessResult::forbidden(); + return AccessResult::forbidden(); } } diff --git a/workbench_access_protect/workbench_access_protect.services.yml b/workbench_access_protect/workbench_access_protect.services.yml index be8f682a..83dddd6d 100644 --- a/workbench_access_protect/workbench_access_protect.services.yml +++ b/workbench_access_protect/workbench_access_protect.services.yml @@ -1,4 +1,4 @@ services: workbench_access_protect.delete : class: Drupal\workbench_access_protect\Access\DeleteAccessCheck - arguments: ['@current_route_match', '@workbench_access.user_section_storage', '@entity_field.manager', '@messenger', '@entity_type.manager'] + arguments: ['@current_route_match', '@workbench_access.user_section_storage', '@entity_field.manager', '@entity_type.manager', '@entity_type.bundle.info'] diff --git a/workbench_access_protect/workbench_access_protect/src/Access/DeleteAccessCheck.php b/workbench_access_protect/workbench_access_protect/src/Access/DeleteAccessCheck.php new file mode 100644 index 00000000..ace21d3c --- /dev/null +++ b/workbench_access_protect/workbench_access_protect/src/Access/DeleteAccessCheck.php @@ -0,0 +1,282 @@ +route = $route; + $this->userSectionStorage = $userSectionStorage; + $this->fieldManager = $fieldManager; + $this->entityTypeManager = $entityTypeManager; + $this->entityTypeBundleInfo = $entityTypeBundleInfo; + + } + + /** + * This method is used to determine if it is OK to delete. + * + * The check is based on whether or not it is being actively used for access + * control, and if content is assigned to it. If either of these statements + * is true, then 'forbidden' will be returned to prevent the term + * from being deleted. + * + * @return \Drupal\Core\Access\AccessResultAllowed|\Drupal\Core\Access\AccessResultForbidden + * Returns 'forbidden' if the term is being used for access control. + * Returns 'allowed' if the term is not being used for access control. + * + * @throws \Drupal\Component\Plugin\Exception\InvalidPluginDefinitionException + * @throws \Drupal\Component\Plugin\Exception\PluginNotFoundException + */ + public function access() { + + if ($this->isDeleteAllowed()) { + return AccessResult::allowed(); + } + + return AccessResult::forbidden(); + } + + /** + * @{inheritdoc} + */ + public function isDeleteAllowed(EntityInterface $entity) { + + $retval = TRUE; + + if ($entity instanceof EntityInterface) { + + $hasAccessControlMembers = $this->doesTermHaveMembers($entity); + $assigned_content = $this->isAssignedToContent($entity); + + /* + * If this entity does not have users assigned to it for access + * control, and the term is not assigned to any pieces of content, + * it is OK to delete it. + */ + if ($hasAccessControlMembers || $assigned_content) { + + if ($assigned_content) { + $retval = FALSE; + } + + if ($hasAccessControlMembers) { + $retval = FALSE; + } + } + + } + + return $retval; + } + + public function getBundles(EntityInterface $entity) { + $bundle = $this->entityTypeManager->getDefinitions()[$entity->getEntityTypeId()]->get('bundle_of'); + $bundles = $this->entityTypeBundleInfo->getBundleInfo($bundle); + return array_combine(array_keys($bundles), array_keys($bundles)); + + } + + public function hasBundles(EntityInterface $entity) { + if ($this->entityTypeManager->getDefinitions()[$entity->getEntityTypeId()]->get('bundle_of') == null) { + return FALSE; + } + return TRUE; + } + + /** + * {@inheritDoc} + */ + public function isDeleteAllowedBundle($bundle, $entity){ + $bundle_of = $this->entityTypeManager->getDefinitions()[$entity->getEntityTypeId()]->get('bundle_of'); + $entity_id_key = $this->entityTypeManager->getDefinitions()[$entity->getEntityTypeId()]->get('entity_keys')['id']; + $entities = $this->entityTypeManager->getStorage($bundle_of)->loadByProperties( + [$entity_id_key => $bundle ] + ); + + /* + * Cycle through the entities of this bundle. As soon as one is discovered + * as being actively used for access control, we can deny delete. + */ + foreach ($entities as $bundle) { + if ($this->isDeleteAllowed($bundle) === FALSE) { + return FALSE; + } + } + + return TRUE; + } + + /** + * Determines if this term has active members in it. + * + * @param \Drupal\Core\Entity\EntityInterface $entity + * The entity to inspect. + * + * @return bool + * TRUE if the term has members, FALSE otherwise. + * + * @throws \Drupal\Component\Plugin\Exception\InvalidPluginDefinitionException + * @throws \Drupal\Component\Plugin\Exception\PluginNotFoundException + */ + private function doesTermHaveMembers(EntityInterface $entity) { + + /** @var array $sections */ + $sections = $this->getActiveSections($entity); + + if (count($sections) > 0) { + return TRUE; + } + + return FALSE; + } + + + /** + * Inspect the given taxonomy term. + * + * This will determine if there are any active users assigned to it. + * + * @param \Drupal\Core\Entity\EntityInterface $entity + * The entity to inspect. + * + * @return array + * An array of the users assigned to this section. + * + * @throws \Drupal\Component\Plugin\Exception\InvalidPluginDefinitionException + * @throws \Drupal\Component\Plugin\Exception\PluginNotFoundException + */ + private function getActiveSections(EntityInterface $entity) { + /** @var \Drupal\workbench_access\UserSectionStorageInterface $sectionStorage */ + $sectionStorage = $this->userSectionStorage; + + $editors = array_reduce($this->entityTypeManager->getStorage('access_scheme')->loadMultiple(), + function (array $editors, AccessSchemeInterface $scheme) use ($sectionStorage, $entity) { + $editors += $sectionStorage->getEditors($scheme, $entity->id()); + return $editors; + }, []); + + return $editors; + } + + /** + * Determine if tagged content exists. + * + * This method will determine if any entities exist in the system that are + * tagged with the term. + * + * @param \Drupal\Core\Entity\EntityInterface $entity + * The Entity to inspect. + * + * @return bool + * TRUE if content is assigned to this term. + * FALSE if content is not assigned to this term. + * + */ + private function isAssignedToContent(EntityInterface $entity) { + + + $reference_fields = $this->getAllReferenceFields($entity); + + foreach ($reference_fields as $name => $fieldConfig) { + // Get the entity reference and determine if it's a taxonomy. + if ($fieldConfig instanceof FieldStorageConfig) { + $entities = \Drupal::entityQuery($fieldConfig->get('entity_type')) + ->condition($fieldConfig->get('field_name'), $entity->id()) + ->range(0, 1) + ->execute(); + + if (count($entities) > 0) { + return TRUE; + } + } + } + + + return FALSE; + + } + + private function getAllReferenceFields(EntityInterface $entity) { + // First, we are going to try to retrieve a cached instance. + $found_fields = \Drupal::cache()->get('workbench_access_protect'); + + if ($found_fields === FALSE) { + $map = $this->fieldManager->getFieldMap(); + $found_fields = []; + foreach ($map as $entity_type => $fields) { + foreach ($fields as $name => $field) { + if ($field['type'] === 'entity_reference') { + // Get the entity reference and determine if it's a taxonomy. + /** @var \Drupal\field\Entity\FieldStorageConfig $fieldConfig */ + $fieldConfig = FieldStorageConfig::loadByName($entity_type, $name); + if ($fieldConfig !== NULL && + $fieldConfig->getSetting('target_type') === $entity->getEntityType()->id()) { + $found_fields[$name] = $fieldConfig; + } + } + } + } + } + + else { + $found_fields = $found_fields->data; + } + + \Drupal::cache() + ->set('workbench_access_protect', $found_fields, REQUEST_TIME + 60); + + return $found_fields; + } + +} diff --git a/workbench_access_protect/workbench_access_protect/src/Access/DeleteAccessCheckInterface.php b/workbench_access_protect/workbench_access_protect/src/Access/DeleteAccessCheckInterface.php new file mode 100644 index 00000000..755f5da4 --- /dev/null +++ b/workbench_access_protect/workbench_access_protect/src/Access/DeleteAccessCheckInterface.php @@ -0,0 +1,61 @@ +createContentType(['type' => 'page']); + $this->createContentType(['type' => 'article']); + $this->vocabulary = $this->setUpVocabulary(); + $this->setUpTaxonomyFieldForEntityType('node', 'page', $this->vocabulary->id()); + $this->setUpTaxonomyFieldForEntityType('taxonomy_term', $this->vocabulary->id(), $this->vocabulary->id(), 'recursive', 'Recursive Field'); + $vocab = Vocabulary::create(['vid' => 'selected', 'name' => 'Selected Vocabulary']); + $vocab->save(); + $this->setUpTaxonomyFieldForEntityType('taxonomy_term', $vocab->id(), $this->vocabulary->id(), 'non_recursive', 'Allowed Field'); + entity_test_create_bundle('access_controlled'); + entity_test_create_bundle('notaccess_controlled'); + $this->setUpTaxonomyFieldForEntityType('entity_test', 'access_controlled', $this->vocabulary->id()); + $this->admin = $this->setUpAdminUser([ + 'administer workbench access', + 'edit terms in workbench_access', + 'delete terms in workbench_access', + 'create terms in workbench_access', + ]); + $this->admin2 = $this->setUpUserUniqueRole([ + 'administer workbench access', + 'assign workbench access', + 'edit terms in workbench_access', + 'delete terms in workbench_access', + 'create terms in workbench_access', + 'access administration pages', + 'access taxonomy overview', + 'administer taxonomy', + ]); + $this->placeBlock('local_actions_block'); + + $this->setUpTestContent(); + + + + } + + protected function setUpTestContent() { + // create a test term + $this->term = Term::create([ + 'name' => 'Test Term', + 'vid' => $this->vocabulary->id(), + ]); + + $this->term->save(); + + $this->emptyTerm = Term::create([ + 'name' => 'Empty Test Term', + 'vid' => $this->vocabulary->id(), + ]); + + $this->emptyTerm->save(); + + + // create some test nodes + $this->testNode = $this->createNode( + [ + 'title' => 'Node', + 'type' => 'page', + 'uid' => $this->admin->id(), + WorkbenchAccessManagerInterface::FIELD_NAME => $this->term->id(), + ] + ); + + $this->testNode->save(); + } + + /** + * Asset a non-administrator cannot delete terms that are actively used + * + * @throws \Behat\Mink\Exception\ExpectationException + */ + public function testAssertCannotDeleteTaxonomy() { + + // Switch user to the non-privileged account. + $this->drupalLogin($this->admin2); + + $path = '/taxonomy/term/' . $this->term->id() . '/edit'; + $this->drupalGet($path); + $this->assertSession()->linkNotExists("Delete"); + + $delete_path = '/taxonomy/term/' . $this->term->id() . '/delete'; + $this->drupalGet($delete_path); + $this->assertSession()->statusCodeEquals(403); + + // Test the overview page to make sure that a delete is not present. + $vocab_path = '/admin/structure/taxonomy/manage/' . $this->vocabulary->id() . '/overview'; + $this->drupalGet($vocab_path); + $delete_path = '/admin/structure/taxonomy/manage/' . $this->vocabulary->id() . '/delete'; + $this->assertSession()->linkByHrefNotExists($delete_path); + + // Switch user back to the privileged account. + $this->drupalLogout(); + } + + /** + * Assert that anonymous visitors are not told anything additional about + * this term, besides the existence of the URL as provided by Drupal. + * + * Just a sanity check to help check we didn't break anything. + * + * @throws \Behat\Mink\Exception\ExpectationException + */ + public function testAssertAnonymousGets403() { + $delete_path = '/taxonomy/term/' . $this->term->id() . '/delete'; + $this->drupalGet($delete_path); + $this->assertSession()->statusCodeEquals(403); + } + + public function testAssertUntaggedTermsMayBeDeleted() { + // Switch user to the non-privileged account. + + $this->drupalLogin($this->admin2); + + $path = '/taxonomy/term/' . $this->emptyTerm->id() . '/edit'; + $this->drupalGet($path); + $this->assertSession()->linkExists("Delete"); + + $delete_path = '/taxonomy/term/' . $this->emptyTerm->id() . '/delete'; + + $this->drupalGet($delete_path); + $this->assertSession()->statusCodeEquals(200); + + // Test the overview page to make sure that a delete is present. + $vocab_path = '/admin/structure/taxonomy/manage/' . $this->vocabulary->id(); + $delete_path = '/admin/structure/taxonomy/manage/' . $this->vocabulary->id() . '/delete'; + $this->drupalGet($vocab_path); + $this->assertSession()->linkByHrefExists($delete_path); + + // Switch user back to the privileged account. + $this->drupalLogout(); + $this->drupalLogin($this->admin); + } + + + +} diff --git a/workbench_access_protect/workbench_access_protect/workbench_access_protect.info.yml b/workbench_access_protect/workbench_access_protect/workbench_access_protect.info.yml new file mode 100644 index 00000000..b9f6c1ba --- /dev/null +++ b/workbench_access_protect/workbench_access_protect/workbench_access_protect.info.yml @@ -0,0 +1,7 @@ +name: Workbench Access Protect +type: module +description: Adds additional logic upon entities for delete operations. +core: 8.x +package: Workbench +dependencies: + - workbench_access:workbench_access diff --git a/workbench_access_protect/workbench_access_protect/workbench_access_protect.module b/workbench_access_protect/workbench_access_protect/workbench_access_protect.module new file mode 100644 index 00000000..f8cb6892 --- /dev/null +++ b/workbench_access_protect/workbench_access_protect/workbench_access_protect.module @@ -0,0 +1,42 @@ +hasBundles($entity)) { + $bundle = $protect->getBundles($entity)[$entity->id()]; + // Check the entire bundle to see if any term is in use. + if ($protect->isDeleteAllowedBundle($bundle, $entity) === FALSE) { + return AccessResult::forbidden(); + } + } + else { + // We're at the leaf entity and can check the specific entity. + if ($protect->isDeleteAllowed($entity) === FALSE) { + return AccessResult::forbidden(); + } + + } + } + + return AccessResult::neutral(); + +} diff --git a/workbench_access_protect/workbench_access_protect/workbench_access_protect.services.yml b/workbench_access_protect/workbench_access_protect/workbench_access_protect.services.yml new file mode 100644 index 00000000..83dddd6d --- /dev/null +++ b/workbench_access_protect/workbench_access_protect/workbench_access_protect.services.yml @@ -0,0 +1,4 @@ +services: + workbench_access_protect.delete : + class: Drupal\workbench_access_protect\Access\DeleteAccessCheck + arguments: ['@current_route_match', '@workbench_access.user_section_storage', '@entity_field.manager', '@entity_type.manager', '@entity_type.bundle.info'] From c30d711888dc558c754e986135494e30ec87e9cf Mon Sep 17 00:00:00 2001 From: Jason Partyka <4501353+jasonpartyka@users.noreply.github.com> Date: Wed, 8 May 2019 08:36:55 -0500 Subject: [PATCH 12/39] Fixed accidental copy. --- .../src/Access/DeleteAccessCheck.php | 62 ++-- .../src/Access/DeleteAccessCheck.php | 282 ------------------ .../src/Access/DeleteAccessCheckInterface.php | 61 ---- .../tests/Functional/TaxonomySchemeUITest.php | 217 -------------- .../workbench_access_protect.info.yml | 7 - .../workbench_access_protect.module | 42 --- .../workbench_access_protect.services.yml | 4 - 7 files changed, 45 insertions(+), 630 deletions(-) delete mode 100644 workbench_access_protect/workbench_access_protect/src/Access/DeleteAccessCheck.php delete mode 100644 workbench_access_protect/workbench_access_protect/src/Access/DeleteAccessCheckInterface.php delete mode 100644 workbench_access_protect/workbench_access_protect/tests/Functional/TaxonomySchemeUITest.php delete mode 100644 workbench_access_protect/workbench_access_protect/workbench_access_protect.info.yml delete mode 100644 workbench_access_protect/workbench_access_protect/workbench_access_protect.module delete mode 100644 workbench_access_protect/workbench_access_protect/workbench_access_protect.services.yml diff --git a/workbench_access_protect/src/Access/DeleteAccessCheck.php b/workbench_access_protect/src/Access/DeleteAccessCheck.php index 013704fd..ace21d3c 100644 --- a/workbench_access_protect/src/Access/DeleteAccessCheck.php +++ b/workbench_access_protect/src/Access/DeleteAccessCheck.php @@ -222,33 +222,61 @@ function (array $editors, AccessSchemeInterface $scheme) use ($sectionStorage, $ * TRUE if content is assigned to this term. * FALSE if content is not assigned to this term. * - * @throws \Drupal\Component\Plugin\Exception\InvalidPluginDefinitionException - * @throws \Drupal\Component\Plugin\Exception\PluginNotFoundException */ private function isAssignedToContent(EntityInterface $entity) { - $map = $this->fieldManager->getFieldMap(); - - foreach ($map as $entity_type => $fields) { - foreach ($fields as $name => $field) { - if ($field['type'] == 'entity_reference') { - // Get the entity reference and determine if it's a taxonomy. - /** @var \Drupal\field\Entity\FieldStorageConfig $fieldConfig */ - $fieldConfig = FieldStorageConfig::loadByName($entity_type, $name); - if ($fieldConfig instanceof FieldStorageConfig) { - $entities = $this->entityTypeManager->getStorage($entity_type)->loadByProperties([ - $name => $entity->id(), - ]); - if (count($entities) > 0) { - return TRUE; + + $reference_fields = $this->getAllReferenceFields($entity); + + foreach ($reference_fields as $name => $fieldConfig) { + // Get the entity reference and determine if it's a taxonomy. + if ($fieldConfig instanceof FieldStorageConfig) { + $entities = \Drupal::entityQuery($fieldConfig->get('entity_type')) + ->condition($fieldConfig->get('field_name'), $entity->id()) + ->range(0, 1) + ->execute(); + + if (count($entities) > 0) { + return TRUE; + } + } + } + + + return FALSE; + + } + + private function getAllReferenceFields(EntityInterface $entity) { + // First, we are going to try to retrieve a cached instance. + $found_fields = \Drupal::cache()->get('workbench_access_protect'); + + if ($found_fields === FALSE) { + $map = $this->fieldManager->getFieldMap(); + $found_fields = []; + foreach ($map as $entity_type => $fields) { + foreach ($fields as $name => $field) { + if ($field['type'] === 'entity_reference') { + // Get the entity reference and determine if it's a taxonomy. + /** @var \Drupal\field\Entity\FieldStorageConfig $fieldConfig */ + $fieldConfig = FieldStorageConfig::loadByName($entity_type, $name); + if ($fieldConfig !== NULL && + $fieldConfig->getSetting('target_type') === $entity->getEntityType()->id()) { + $found_fields[$name] = $fieldConfig; + } } } } } + + else { + $found_fields = $found_fields->data; } - return FALSE; + \Drupal::cache() + ->set('workbench_access_protect', $found_fields, REQUEST_TIME + 60); + return $found_fields; } } diff --git a/workbench_access_protect/workbench_access_protect/src/Access/DeleteAccessCheck.php b/workbench_access_protect/workbench_access_protect/src/Access/DeleteAccessCheck.php deleted file mode 100644 index ace21d3c..00000000 --- a/workbench_access_protect/workbench_access_protect/src/Access/DeleteAccessCheck.php +++ /dev/null @@ -1,282 +0,0 @@ -route = $route; - $this->userSectionStorage = $userSectionStorage; - $this->fieldManager = $fieldManager; - $this->entityTypeManager = $entityTypeManager; - $this->entityTypeBundleInfo = $entityTypeBundleInfo; - - } - - /** - * This method is used to determine if it is OK to delete. - * - * The check is based on whether or not it is being actively used for access - * control, and if content is assigned to it. If either of these statements - * is true, then 'forbidden' will be returned to prevent the term - * from being deleted. - * - * @return \Drupal\Core\Access\AccessResultAllowed|\Drupal\Core\Access\AccessResultForbidden - * Returns 'forbidden' if the term is being used for access control. - * Returns 'allowed' if the term is not being used for access control. - * - * @throws \Drupal\Component\Plugin\Exception\InvalidPluginDefinitionException - * @throws \Drupal\Component\Plugin\Exception\PluginNotFoundException - */ - public function access() { - - if ($this->isDeleteAllowed()) { - return AccessResult::allowed(); - } - - return AccessResult::forbidden(); - } - - /** - * @{inheritdoc} - */ - public function isDeleteAllowed(EntityInterface $entity) { - - $retval = TRUE; - - if ($entity instanceof EntityInterface) { - - $hasAccessControlMembers = $this->doesTermHaveMembers($entity); - $assigned_content = $this->isAssignedToContent($entity); - - /* - * If this entity does not have users assigned to it for access - * control, and the term is not assigned to any pieces of content, - * it is OK to delete it. - */ - if ($hasAccessControlMembers || $assigned_content) { - - if ($assigned_content) { - $retval = FALSE; - } - - if ($hasAccessControlMembers) { - $retval = FALSE; - } - } - - } - - return $retval; - } - - public function getBundles(EntityInterface $entity) { - $bundle = $this->entityTypeManager->getDefinitions()[$entity->getEntityTypeId()]->get('bundle_of'); - $bundles = $this->entityTypeBundleInfo->getBundleInfo($bundle); - return array_combine(array_keys($bundles), array_keys($bundles)); - - } - - public function hasBundles(EntityInterface $entity) { - if ($this->entityTypeManager->getDefinitions()[$entity->getEntityTypeId()]->get('bundle_of') == null) { - return FALSE; - } - return TRUE; - } - - /** - * {@inheritDoc} - */ - public function isDeleteAllowedBundle($bundle, $entity){ - $bundle_of = $this->entityTypeManager->getDefinitions()[$entity->getEntityTypeId()]->get('bundle_of'); - $entity_id_key = $this->entityTypeManager->getDefinitions()[$entity->getEntityTypeId()]->get('entity_keys')['id']; - $entities = $this->entityTypeManager->getStorage($bundle_of)->loadByProperties( - [$entity_id_key => $bundle ] - ); - - /* - * Cycle through the entities of this bundle. As soon as one is discovered - * as being actively used for access control, we can deny delete. - */ - foreach ($entities as $bundle) { - if ($this->isDeleteAllowed($bundle) === FALSE) { - return FALSE; - } - } - - return TRUE; - } - - /** - * Determines if this term has active members in it. - * - * @param \Drupal\Core\Entity\EntityInterface $entity - * The entity to inspect. - * - * @return bool - * TRUE if the term has members, FALSE otherwise. - * - * @throws \Drupal\Component\Plugin\Exception\InvalidPluginDefinitionException - * @throws \Drupal\Component\Plugin\Exception\PluginNotFoundException - */ - private function doesTermHaveMembers(EntityInterface $entity) { - - /** @var array $sections */ - $sections = $this->getActiveSections($entity); - - if (count($sections) > 0) { - return TRUE; - } - - return FALSE; - } - - - /** - * Inspect the given taxonomy term. - * - * This will determine if there are any active users assigned to it. - * - * @param \Drupal\Core\Entity\EntityInterface $entity - * The entity to inspect. - * - * @return array - * An array of the users assigned to this section. - * - * @throws \Drupal\Component\Plugin\Exception\InvalidPluginDefinitionException - * @throws \Drupal\Component\Plugin\Exception\PluginNotFoundException - */ - private function getActiveSections(EntityInterface $entity) { - /** @var \Drupal\workbench_access\UserSectionStorageInterface $sectionStorage */ - $sectionStorage = $this->userSectionStorage; - - $editors = array_reduce($this->entityTypeManager->getStorage('access_scheme')->loadMultiple(), - function (array $editors, AccessSchemeInterface $scheme) use ($sectionStorage, $entity) { - $editors += $sectionStorage->getEditors($scheme, $entity->id()); - return $editors; - }, []); - - return $editors; - } - - /** - * Determine if tagged content exists. - * - * This method will determine if any entities exist in the system that are - * tagged with the term. - * - * @param \Drupal\Core\Entity\EntityInterface $entity - * The Entity to inspect. - * - * @return bool - * TRUE if content is assigned to this term. - * FALSE if content is not assigned to this term. - * - */ - private function isAssignedToContent(EntityInterface $entity) { - - - $reference_fields = $this->getAllReferenceFields($entity); - - foreach ($reference_fields as $name => $fieldConfig) { - // Get the entity reference and determine if it's a taxonomy. - if ($fieldConfig instanceof FieldStorageConfig) { - $entities = \Drupal::entityQuery($fieldConfig->get('entity_type')) - ->condition($fieldConfig->get('field_name'), $entity->id()) - ->range(0, 1) - ->execute(); - - if (count($entities) > 0) { - return TRUE; - } - } - } - - - return FALSE; - - } - - private function getAllReferenceFields(EntityInterface $entity) { - // First, we are going to try to retrieve a cached instance. - $found_fields = \Drupal::cache()->get('workbench_access_protect'); - - if ($found_fields === FALSE) { - $map = $this->fieldManager->getFieldMap(); - $found_fields = []; - foreach ($map as $entity_type => $fields) { - foreach ($fields as $name => $field) { - if ($field['type'] === 'entity_reference') { - // Get the entity reference and determine if it's a taxonomy. - /** @var \Drupal\field\Entity\FieldStorageConfig $fieldConfig */ - $fieldConfig = FieldStorageConfig::loadByName($entity_type, $name); - if ($fieldConfig !== NULL && - $fieldConfig->getSetting('target_type') === $entity->getEntityType()->id()) { - $found_fields[$name] = $fieldConfig; - } - } - } - } - } - - else { - $found_fields = $found_fields->data; - } - - \Drupal::cache() - ->set('workbench_access_protect', $found_fields, REQUEST_TIME + 60); - - return $found_fields; - } - -} diff --git a/workbench_access_protect/workbench_access_protect/src/Access/DeleteAccessCheckInterface.php b/workbench_access_protect/workbench_access_protect/src/Access/DeleteAccessCheckInterface.php deleted file mode 100644 index 755f5da4..00000000 --- a/workbench_access_protect/workbench_access_protect/src/Access/DeleteAccessCheckInterface.php +++ /dev/null @@ -1,61 +0,0 @@ -createContentType(['type' => 'page']); - $this->createContentType(['type' => 'article']); - $this->vocabulary = $this->setUpVocabulary(); - $this->setUpTaxonomyFieldForEntityType('node', 'page', $this->vocabulary->id()); - $this->setUpTaxonomyFieldForEntityType('taxonomy_term', $this->vocabulary->id(), $this->vocabulary->id(), 'recursive', 'Recursive Field'); - $vocab = Vocabulary::create(['vid' => 'selected', 'name' => 'Selected Vocabulary']); - $vocab->save(); - $this->setUpTaxonomyFieldForEntityType('taxonomy_term', $vocab->id(), $this->vocabulary->id(), 'non_recursive', 'Allowed Field'); - entity_test_create_bundle('access_controlled'); - entity_test_create_bundle('notaccess_controlled'); - $this->setUpTaxonomyFieldForEntityType('entity_test', 'access_controlled', $this->vocabulary->id()); - $this->admin = $this->setUpAdminUser([ - 'administer workbench access', - 'edit terms in workbench_access', - 'delete terms in workbench_access', - 'create terms in workbench_access', - ]); - $this->admin2 = $this->setUpUserUniqueRole([ - 'administer workbench access', - 'assign workbench access', - 'edit terms in workbench_access', - 'delete terms in workbench_access', - 'create terms in workbench_access', - 'access administration pages', - 'access taxonomy overview', - 'administer taxonomy', - ]); - $this->placeBlock('local_actions_block'); - - $this->setUpTestContent(); - - - - } - - protected function setUpTestContent() { - // create a test term - $this->term = Term::create([ - 'name' => 'Test Term', - 'vid' => $this->vocabulary->id(), - ]); - - $this->term->save(); - - $this->emptyTerm = Term::create([ - 'name' => 'Empty Test Term', - 'vid' => $this->vocabulary->id(), - ]); - - $this->emptyTerm->save(); - - - // create some test nodes - $this->testNode = $this->createNode( - [ - 'title' => 'Node', - 'type' => 'page', - 'uid' => $this->admin->id(), - WorkbenchAccessManagerInterface::FIELD_NAME => $this->term->id(), - ] - ); - - $this->testNode->save(); - } - - /** - * Asset a non-administrator cannot delete terms that are actively used - * - * @throws \Behat\Mink\Exception\ExpectationException - */ - public function testAssertCannotDeleteTaxonomy() { - - // Switch user to the non-privileged account. - $this->drupalLogin($this->admin2); - - $path = '/taxonomy/term/' . $this->term->id() . '/edit'; - $this->drupalGet($path); - $this->assertSession()->linkNotExists("Delete"); - - $delete_path = '/taxonomy/term/' . $this->term->id() . '/delete'; - $this->drupalGet($delete_path); - $this->assertSession()->statusCodeEquals(403); - - // Test the overview page to make sure that a delete is not present. - $vocab_path = '/admin/structure/taxonomy/manage/' . $this->vocabulary->id() . '/overview'; - $this->drupalGet($vocab_path); - $delete_path = '/admin/structure/taxonomy/manage/' . $this->vocabulary->id() . '/delete'; - $this->assertSession()->linkByHrefNotExists($delete_path); - - // Switch user back to the privileged account. - $this->drupalLogout(); - } - - /** - * Assert that anonymous visitors are not told anything additional about - * this term, besides the existence of the URL as provided by Drupal. - * - * Just a sanity check to help check we didn't break anything. - * - * @throws \Behat\Mink\Exception\ExpectationException - */ - public function testAssertAnonymousGets403() { - $delete_path = '/taxonomy/term/' . $this->term->id() . '/delete'; - $this->drupalGet($delete_path); - $this->assertSession()->statusCodeEquals(403); - } - - public function testAssertUntaggedTermsMayBeDeleted() { - // Switch user to the non-privileged account. - - $this->drupalLogin($this->admin2); - - $path = '/taxonomy/term/' . $this->emptyTerm->id() . '/edit'; - $this->drupalGet($path); - $this->assertSession()->linkExists("Delete"); - - $delete_path = '/taxonomy/term/' . $this->emptyTerm->id() . '/delete'; - - $this->drupalGet($delete_path); - $this->assertSession()->statusCodeEquals(200); - - // Test the overview page to make sure that a delete is present. - $vocab_path = '/admin/structure/taxonomy/manage/' . $this->vocabulary->id(); - $delete_path = '/admin/structure/taxonomy/manage/' . $this->vocabulary->id() . '/delete'; - $this->drupalGet($vocab_path); - $this->assertSession()->linkByHrefExists($delete_path); - - // Switch user back to the privileged account. - $this->drupalLogout(); - $this->drupalLogin($this->admin); - } - - - -} diff --git a/workbench_access_protect/workbench_access_protect/workbench_access_protect.info.yml b/workbench_access_protect/workbench_access_protect/workbench_access_protect.info.yml deleted file mode 100644 index b9f6c1ba..00000000 --- a/workbench_access_protect/workbench_access_protect/workbench_access_protect.info.yml +++ /dev/null @@ -1,7 +0,0 @@ -name: Workbench Access Protect -type: module -description: Adds additional logic upon entities for delete operations. -core: 8.x -package: Workbench -dependencies: - - workbench_access:workbench_access diff --git a/workbench_access_protect/workbench_access_protect/workbench_access_protect.module b/workbench_access_protect/workbench_access_protect/workbench_access_protect.module deleted file mode 100644 index f8cb6892..00000000 --- a/workbench_access_protect/workbench_access_protect/workbench_access_protect.module +++ /dev/null @@ -1,42 +0,0 @@ -hasBundles($entity)) { - $bundle = $protect->getBundles($entity)[$entity->id()]; - // Check the entire bundle to see if any term is in use. - if ($protect->isDeleteAllowedBundle($bundle, $entity) === FALSE) { - return AccessResult::forbidden(); - } - } - else { - // We're at the leaf entity and can check the specific entity. - if ($protect->isDeleteAllowed($entity) === FALSE) { - return AccessResult::forbidden(); - } - - } - } - - return AccessResult::neutral(); - -} diff --git a/workbench_access_protect/workbench_access_protect/workbench_access_protect.services.yml b/workbench_access_protect/workbench_access_protect/workbench_access_protect.services.yml deleted file mode 100644 index 83dddd6d..00000000 --- a/workbench_access_protect/workbench_access_protect/workbench_access_protect.services.yml +++ /dev/null @@ -1,4 +0,0 @@ -services: - workbench_access_protect.delete : - class: Drupal\workbench_access_protect\Access\DeleteAccessCheck - arguments: ['@current_route_match', '@workbench_access.user_section_storage', '@entity_field.manager', '@entity_type.manager', '@entity_type.bundle.info'] From 4534529dfa69ae3e7d9836a87db069247ea1fde4 Mon Sep 17 00:00:00 2001 From: Jason Partyka <4501353+jasonpartyka@users.noreply.github.com> Date: Fri, 17 May 2019 06:15:53 -0500 Subject: [PATCH 13/39] Added an 'ids' index to the configuration so that we can check the bundle an entity is part of for access control. --- src/Plugin/AccessControlHierarchy/Menu.php | 2 ++ .../AccessControlHierarchy/Taxonomy.php | 2 ++ .../src/Access/DeleteAccessCheck.php | 16 ++++++++++++-- .../src/Access/DeleteAccessCheckInterface.php | 15 +++++++++++++ .../workbench_access_protect.module | 22 ++++++++++--------- 5 files changed, 45 insertions(+), 12 deletions(-) diff --git a/src/Plugin/AccessControlHierarchy/Menu.php b/src/Plugin/AccessControlHierarchy/Menu.php index 0219f0c4..f555062a 100644 --- a/src/Plugin/AccessControlHierarchy/Menu.php +++ b/src/Plugin/AccessControlHierarchy/Menu.php @@ -257,6 +257,7 @@ public function applies($entity_type_id, $bundle) { */ public function submitConfigurationForm(array &$form, FormStateInterface $form_state) { $this->configuration['menus'] = array_values(array_filter($form_state->getValue('menus'))); + $this->configuration['ids'] = $this->configuration['menus']; $this->configuration['bundles'] = array_values(array_filter($form_state->getValue('bundles'))); } @@ -292,6 +293,7 @@ public function defaultConfiguration() { return [ 'menus' => [], 'bundles' => [], + 'ids' => [], ]; } diff --git a/src/Plugin/AccessControlHierarchy/Taxonomy.php b/src/Plugin/AccessControlHierarchy/Taxonomy.php index eaad70e6..d7c6ffcf 100644 --- a/src/Plugin/AccessControlHierarchy/Taxonomy.php +++ b/src/Plugin/AccessControlHierarchy/Taxonomy.php @@ -125,6 +125,7 @@ public function defaultConfiguration() { $defaults = [ 'fields' => [], 'vocabularies' => [], + 'ids' => [], ]; return $defaults + parent::defaultConfiguration(); } @@ -416,6 +417,7 @@ public function submitConfigurationForm(array &$form, FormStateInterface $form_s // Saving 'validate' can cause schema errors. unset($settings['validate']); $settings['vocabularies'] = array_values(array_filter($settings['vocabularies'])); + $settings['ids'] = $settings['vocabularies']; $settings['fields'] = array_values(array_map(function ($item) { list($entity_type, $bundle, $field_name) = explode(':', $item); return [ diff --git a/workbench_access_protect/src/Access/DeleteAccessCheck.php b/workbench_access_protect/src/Access/DeleteAccessCheck.php index ace21d3c..1132061b 100644 --- a/workbench_access_protect/src/Access/DeleteAccessCheck.php +++ b/workbench_access_protect/src/Access/DeleteAccessCheck.php @@ -225,7 +225,6 @@ function (array $editors, AccessSchemeInterface $scheme) use ($sectionStorage, $ */ private function isAssignedToContent(EntityInterface $entity) { - $reference_fields = $this->getAllReferenceFields($entity); foreach ($reference_fields as $name => $fieldConfig) { @@ -274,9 +273,22 @@ private function getAllReferenceFields(EntityInterface $entity) { } \Drupal::cache() - ->set('workbench_access_protect', $found_fields, REQUEST_TIME + 60); + ->set('workbench_access_protect', $found_fields, \Drupal::time()->getRequestTime() + 60); return $found_fields; } + public function isAccessControlled(EntityInterface $entity) { + $schemes = $this->entityTypeManager->getStorage('access_scheme')->loadMultiple(); + /** @var \Drupal\workbench_access\Entity\AccessScheme $scheme */ + foreach ($schemes as $scheme) { + foreach ($scheme->getAccessScheme()->getConfiguration() as $config) { + if (in_array($entity->bundle(), $config)) { + return TRUE; + } + } + } + return FALSE; + } + } diff --git a/workbench_access_protect/src/Access/DeleteAccessCheckInterface.php b/workbench_access_protect/src/Access/DeleteAccessCheckInterface.php index 755f5da4..c1ccff23 100644 --- a/workbench_access_protect/src/Access/DeleteAccessCheckInterface.php +++ b/workbench_access_protect/src/Access/DeleteAccessCheckInterface.php @@ -57,5 +57,20 @@ public function getBundles(EntityInterface $entity); */ public function isDeleteAllowedBundle($bundle, $entity); + /** + * Helper method to determine if the entity is used for access control. + * + * This method is used to determine if we should actually perform the usage + * check or skip. + * + * @param \Drupal\Core\Entity\EntityInterface $entity + * An entity object. + * + * @return bool + * TRUE if used, FALSE otherwise + */ + public function isAccessControlled(EntityInterface $entity); + + } diff --git a/workbench_access_protect/workbench_access_protect.module b/workbench_access_protect/workbench_access_protect.module index f8cb6892..791663fd 100644 --- a/workbench_access_protect/workbench_access_protect.module +++ b/workbench_access_protect/workbench_access_protect.module @@ -21,20 +21,22 @@ function workbench_access_protect_entity_access(EntityInterface $entity, $op, Ac /** @var Drupal\workbench_access_protect\Access\DeleteAccessCheck $protect */ $protect = \Drupal::service('workbench_access_protect.delete'); - if ($protect->hasBundles($entity)) { - $bundle = $protect->getBundles($entity)[$entity->id()]; - // Check the entire bundle to see if any term is in use. - if ($protect->isDeleteAllowedBundle($bundle, $entity) === FALSE) { - return AccessResult::forbidden(); + // First, let's check to see if this is an entity we are tracking. + if ($protect->hasBundles($entity)) { + $bundle = $protect->getBundles($entity)[$entity->id()]; + // Check the entire bundle to see if any term is in use. + if ($protect->isDeleteAllowedBundle($bundle, $entity) === FALSE) { + return AccessResult::forbidden(); + } } - } - else { - // We're at the leaf entity and can check the specific entity. - if ($protect->isDeleteAllowed($entity) === FALSE) { + else { + // We're at the leaf entity and can check the specific entity. + if ($protect->isAccessControlled($entity) && + $protect->isDeleteAllowed($entity) === FALSE) { return AccessResult::forbidden(); + } } - } } return AccessResult::neutral(); From 4259001ef18caa15873cd7229ad2cb6670620270 Mon Sep 17 00:00:00 2001 From: Jason Partyka <4501353+jasonpartyka@users.noreply.github.com> Date: Mon, 15 Jul 2019 13:39:17 -0500 Subject: [PATCH 14/39] Added IDs to schema. --- config/schema/workbench_access.schema.yml | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/config/schema/workbench_access.schema.yml b/config/schema/workbench_access.schema.yml index bb79dd65..a4281b53 100644 --- a/config/schema/workbench_access.schema.yml +++ b/config/schema/workbench_access.schema.yml @@ -65,6 +65,12 @@ workbench_access.scheme_settings.taxonomy: field: type: string label: 'Field' + ids: + type: sequence + label: "Vocabulary IDs" + sequence: + type: integer + label: "Vocabulary ID" workbench_access.scheme_settings.menu: type: workbench_access_scheme_settings @@ -81,3 +87,9 @@ workbench_access.scheme_settings.menu: sequence: type: string label: 'Bundle' + ids: + type: sequence + label: "Vocabulary IDs" + sequence: + type: integer + label: "Vocabulary ID" From c4ca783776d327f89fa877c95de3e4b97866f35e Mon Sep 17 00:00:00 2001 From: Jason Partyka <4501353+jasonpartyka@users.noreply.github.com> Date: Mon, 15 Jul 2019 14:10:15 -0500 Subject: [PATCH 15/39] Corrected copy and paste mistake. --- config/schema/workbench_access.schema.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/config/schema/workbench_access.schema.yml b/config/schema/workbench_access.schema.yml index a4281b53..539dfeba 100644 --- a/config/schema/workbench_access.schema.yml +++ b/config/schema/workbench_access.schema.yml @@ -89,7 +89,7 @@ workbench_access.scheme_settings.menu: label: 'Bundle' ids: type: sequence - label: "Vocabulary IDs" + label: "Menu IDs" sequence: type: integer - label: "Vocabulary ID" + label: "Menu ID" From 113217438c9debf0aa53874e949359400fcc3f33 Mon Sep 17 00:00:00 2001 From: Jason Partyka <4501353+jasonpartyka@users.noreply.github.com> Date: Mon, 15 Jul 2019 16:03:22 -0500 Subject: [PATCH 16/39] Changed type of test to Functional from Unit for workbench access protect. --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index d1c0f64f..df4061e2 100644 --- a/.travis.yml +++ b/.travis.yml @@ -77,4 +77,4 @@ script: - php core/scripts/run-tests.sh --suppress-deprecations --verbose --color --concurrency 4 --types "PHPUnit-Functional" --php `which php` --url http://localhost:8080 "workbench_access" - php core/scripts/run-tests.sh --suppress-deprecations --verbose --color --concurrency 4 --types "PHPUnit-Kernel" --php `which php` --url http://localhost:8080 "workbench_access" - php core/scripts/run-tests.sh --suppress-deprecations --verbose --color --concurrency 4 --types "PHPUnit-Unit" --php `which php` --url http://localhost:8080 "workbench_access" - - php core/scripts/run-tests.sh --suppress-deprecations --verbose --color --concurrency 4 --types "PHPUnit-Unit" --php `which php` --url http://localhost:8080 "workbench_access_protect" + - php core/scripts/run-tests.sh --suppress-deprecations --verbose --color --concurrency 4 --types "PHPUnit-Functional" --php `which php` --url http://localhost:8080 "workbench_access_protect" From 53fca84b79930a5dc7dfec37af05a4a276e50758 Mon Sep 17 00:00:00 2001 From: Jason Partyka <4501353+jasonpartyka@users.noreply.github.com> Date: Tue, 16 Jul 2019 21:02:31 -0500 Subject: [PATCH 17/39] Updating test. --- tests/src/Functional/UpdatePathTest.php | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/src/Functional/UpdatePathTest.php b/tests/src/Functional/UpdatePathTest.php index f98ad9e7..92a4746b 100644 --- a/tests/src/Functional/UpdatePathTest.php +++ b/tests/src/Functional/UpdatePathTest.php @@ -52,6 +52,7 @@ public function testUpdatePath() { ], ], 'vocabularies' => ['tags'], + 'ids' => [], ], $scheme->getAccessScheme()->getConfiguration()); // Test that user storage was updated. From bcc1f1023698885690ff91a86742aa0a52e9a5c9 Mon Sep 17 00:00:00 2001 From: Jason Partyka <4501353+jasonpartyka@users.noreply.github.com> Date: Tue, 16 Jul 2019 21:19:29 -0500 Subject: [PATCH 18/39] Moved workbench_access_protect test. --- .../tests/{ => src}/Functional/TaxonomySchemeUITest.php | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename workbench_access_protect/tests/{ => src}/Functional/TaxonomySchemeUITest.php (100%) diff --git a/workbench_access_protect/tests/Functional/TaxonomySchemeUITest.php b/workbench_access_protect/tests/src/Functional/TaxonomySchemeUITest.php similarity index 100% rename from workbench_access_protect/tests/Functional/TaxonomySchemeUITest.php rename to workbench_access_protect/tests/src/Functional/TaxonomySchemeUITest.php From 90ee9c58ea252c64720a9ae070bf6871b892261e Mon Sep 17 00:00:00 2001 From: Jason Partyka <4501353+jasonpartyka@users.noreply.github.com> Date: Wed, 17 Jul 2019 19:47:15 -0500 Subject: [PATCH 19/39] Renamed the ids field to parent_ids --- config/schema/workbench_access.schema.yml | 8 ++++---- src/Plugin/AccessControlHierarchy/Menu.php | 3 ++- src/Plugin/AccessControlHierarchy/Taxonomy.php | 3 ++- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/config/schema/workbench_access.schema.yml b/config/schema/workbench_access.schema.yml index 539dfeba..2dcc1d4f 100644 --- a/config/schema/workbench_access.schema.yml +++ b/config/schema/workbench_access.schema.yml @@ -65,12 +65,12 @@ workbench_access.scheme_settings.taxonomy: field: type: string label: 'Field' - ids: + parent_ids: type: sequence - label: "Vocabulary IDs" + label: "Vocabulary Parent IDs" sequence: type: integer - label: "Vocabulary ID" + label: "Parent ID" workbench_access.scheme_settings.menu: type: workbench_access_scheme_settings @@ -87,7 +87,7 @@ workbench_access.scheme_settings.menu: sequence: type: string label: 'Bundle' - ids: + parent_ids: type: sequence label: "Menu IDs" sequence: diff --git a/src/Plugin/AccessControlHierarchy/Menu.php b/src/Plugin/AccessControlHierarchy/Menu.php index f555062a..139fb7bb 100644 --- a/src/Plugin/AccessControlHierarchy/Menu.php +++ b/src/Plugin/AccessControlHierarchy/Menu.php @@ -257,7 +257,8 @@ public function applies($entity_type_id, $bundle) { */ public function submitConfigurationForm(array &$form, FormStateInterface $form_state) { $this->configuration['menus'] = array_values(array_filter($form_state->getValue('menus'))); - $this->configuration['ids'] = $this->configuration['menus']; + // Parent_ids is the machine name of the selected menus. + $this->configuration['parent_ids'] = $this->configuration['menus']; $this->configuration['bundles'] = array_values(array_filter($form_state->getValue('bundles'))); } diff --git a/src/Plugin/AccessControlHierarchy/Taxonomy.php b/src/Plugin/AccessControlHierarchy/Taxonomy.php index d7c6ffcf..05058be8 100644 --- a/src/Plugin/AccessControlHierarchy/Taxonomy.php +++ b/src/Plugin/AccessControlHierarchy/Taxonomy.php @@ -417,7 +417,8 @@ public function submitConfigurationForm(array &$form, FormStateInterface $form_s // Saving 'validate' can cause schema errors. unset($settings['validate']); $settings['vocabularies'] = array_values(array_filter($settings['vocabularies'])); - $settings['ids'] = $settings['vocabularies']; + // Parent_ids is the machine name of the selected vocabularies. + $settings['parent_ids'] = $settings['vocabularies']; $settings['fields'] = array_values(array_map(function ($item) { list($entity_type, $bundle, $field_name) = explode(':', $item); return [ From 2c61b0490c2a31eca3032e78543b6aa471d32693 Mon Sep 17 00:00:00 2001 From: Jason Partyka <4501353+jasonpartyka@users.noreply.github.com> Date: Wed, 17 Jul 2019 19:55:33 -0500 Subject: [PATCH 20/39] More renaming. --- src/Plugin/AccessControlHierarchy/Menu.php | 2 +- src/Plugin/AccessControlHierarchy/Taxonomy.php | 2 +- tests/src/Functional/UpdatePathTest.php | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Plugin/AccessControlHierarchy/Menu.php b/src/Plugin/AccessControlHierarchy/Menu.php index 139fb7bb..2514993e 100644 --- a/src/Plugin/AccessControlHierarchy/Menu.php +++ b/src/Plugin/AccessControlHierarchy/Menu.php @@ -294,7 +294,7 @@ public function defaultConfiguration() { return [ 'menus' => [], 'bundles' => [], - 'ids' => [], + 'parent_ids' => [], ]; } diff --git a/src/Plugin/AccessControlHierarchy/Taxonomy.php b/src/Plugin/AccessControlHierarchy/Taxonomy.php index 05058be8..673998f0 100644 --- a/src/Plugin/AccessControlHierarchy/Taxonomy.php +++ b/src/Plugin/AccessControlHierarchy/Taxonomy.php @@ -125,7 +125,7 @@ public function defaultConfiguration() { $defaults = [ 'fields' => [], 'vocabularies' => [], - 'ids' => [], + 'parent_ids' => [], ]; return $defaults + parent::defaultConfiguration(); } diff --git a/tests/src/Functional/UpdatePathTest.php b/tests/src/Functional/UpdatePathTest.php index 92a4746b..5e6f0cc8 100644 --- a/tests/src/Functional/UpdatePathTest.php +++ b/tests/src/Functional/UpdatePathTest.php @@ -52,7 +52,7 @@ public function testUpdatePath() { ], ], 'vocabularies' => ['tags'], - 'ids' => [], + 'parent_ids' => [], ], $scheme->getAccessScheme()->getConfiguration()); // Test that user storage was updated. From 56ee594c5b2001903ce62f062f98471bfdbc87c9 Mon Sep 17 00:00:00 2001 From: Jason Partyka <4501353+jasonpartyka@users.noreply.github.com> Date: Wed, 17 Jul 2019 20:31:41 -0500 Subject: [PATCH 21/39] Testing a change. --- .travis.yml | 6 +- .../src/Functional/TaxonomySchemeUITest.php | 2 +- .../tests/src/Unit/TaxonomySchemeUITest.php | 217 ++++++++++++++++++ 3 files changed, 221 insertions(+), 4 deletions(-) create mode 100644 workbench_access_protect/tests/src/Unit/TaxonomySchemeUITest.php diff --git a/.travis.yml b/.travis.yml index df4061e2..8e1db637 100644 --- a/.travis.yml +++ b/.travis.yml @@ -74,7 +74,7 @@ before_script: - until curl -s localhost:8080; do true; done > /dev/null script: - - php core/scripts/run-tests.sh --suppress-deprecations --verbose --color --concurrency 4 --types "PHPUnit-Functional" --php `which php` --url http://localhost:8080 "workbench_access" - - php core/scripts/run-tests.sh --suppress-deprecations --verbose --color --concurrency 4 --types "PHPUnit-Kernel" --php `which php` --url http://localhost:8080 "workbench_access" - - php core/scripts/run-tests.sh --suppress-deprecations --verbose --color --concurrency 4 --types "PHPUnit-Unit" --php `which php` --url http://localhost:8080 "workbench_access" +# - php core/scripts/run-tests.sh --suppress-deprecations --verbose --color --concurrency 4 --types "PHPUnit-Functional" --php `which php` --url http://localhost:8080 "workbench_access" +# - php core/scripts/run-tests.sh --suppress-deprecations --verbose --color --concurrency 4 --types "PHPUnit-Kernel" --php `which php` --url http://localhost:8080 "workbench_access" +# - php core/scripts/run-tests.sh --suppress-deprecations --verbose --color --concurrency 4 --types "PHPUnit-Unit" --php `which php` --url http://localhost:8080 "workbench_access" - php core/scripts/run-tests.sh --suppress-deprecations --verbose --color --concurrency 4 --types "PHPUnit-Functional" --php `which php` --url http://localhost:8080 "workbench_access_protect" diff --git a/workbench_access_protect/tests/src/Functional/TaxonomySchemeUITest.php b/workbench_access_protect/tests/src/Functional/TaxonomySchemeUITest.php index 6233ed0b..5adba632 100644 --- a/workbench_access_protect/tests/src/Functional/TaxonomySchemeUITest.php +++ b/workbench_access_protect/tests/src/Functional/TaxonomySchemeUITest.php @@ -157,7 +157,7 @@ public function testAssertCannotDeleteTaxonomy() { $path = '/taxonomy/term/' . $this->term->id() . '/edit'; $this->drupalGet($path); - $this->assertSession()->linkNotExists("Delete"); + $this->assertSession()->linkNotExistsExact("Delete"); $delete_path = '/taxonomy/term/' . $this->term->id() . '/delete'; $this->drupalGet($delete_path); diff --git a/workbench_access_protect/tests/src/Unit/TaxonomySchemeUITest.php b/workbench_access_protect/tests/src/Unit/TaxonomySchemeUITest.php new file mode 100644 index 00000000..6233ed0b --- /dev/null +++ b/workbench_access_protect/tests/src/Unit/TaxonomySchemeUITest.php @@ -0,0 +1,217 @@ +createContentType(['type' => 'page']); + $this->createContentType(['type' => 'article']); + $this->vocabulary = $this->setUpVocabulary(); + $this->setUpTaxonomyFieldForEntityType('node', 'page', $this->vocabulary->id()); + $this->setUpTaxonomyFieldForEntityType('taxonomy_term', $this->vocabulary->id(), $this->vocabulary->id(), 'recursive', 'Recursive Field'); + $vocab = Vocabulary::create(['vid' => 'selected', 'name' => 'Selected Vocabulary']); + $vocab->save(); + $this->setUpTaxonomyFieldForEntityType('taxonomy_term', $vocab->id(), $this->vocabulary->id(), 'non_recursive', 'Allowed Field'); + entity_test_create_bundle('access_controlled'); + entity_test_create_bundle('notaccess_controlled'); + $this->setUpTaxonomyFieldForEntityType('entity_test', 'access_controlled', $this->vocabulary->id()); + $this->admin = $this->setUpAdminUser([ + 'administer workbench access', + 'edit terms in workbench_access', + 'delete terms in workbench_access', + 'create terms in workbench_access', + ]); + $this->admin2 = $this->setUpUserUniqueRole([ + 'administer workbench access', + 'assign workbench access', + 'edit terms in workbench_access', + 'delete terms in workbench_access', + 'create terms in workbench_access', + 'access administration pages', + 'access taxonomy overview', + 'administer taxonomy', + ]); + $this->placeBlock('local_actions_block'); + + $this->setUpTestContent(); + + + + } + + protected function setUpTestContent() { + // create a test term + $this->term = Term::create([ + 'name' => 'Test Term', + 'vid' => $this->vocabulary->id(), + ]); + + $this->term->save(); + + $this->emptyTerm = Term::create([ + 'name' => 'Empty Test Term', + 'vid' => $this->vocabulary->id(), + ]); + + $this->emptyTerm->save(); + + + // create some test nodes + $this->testNode = $this->createNode( + [ + 'title' => 'Node', + 'type' => 'page', + 'uid' => $this->admin->id(), + WorkbenchAccessManagerInterface::FIELD_NAME => $this->term->id(), + ] + ); + + $this->testNode->save(); + } + + /** + * Asset a non-administrator cannot delete terms that are actively used + * + * @throws \Behat\Mink\Exception\ExpectationException + */ + public function testAssertCannotDeleteTaxonomy() { + + // Switch user to the non-privileged account. + $this->drupalLogin($this->admin2); + + $path = '/taxonomy/term/' . $this->term->id() . '/edit'; + $this->drupalGet($path); + $this->assertSession()->linkNotExists("Delete"); + + $delete_path = '/taxonomy/term/' . $this->term->id() . '/delete'; + $this->drupalGet($delete_path); + $this->assertSession()->statusCodeEquals(403); + + // Test the overview page to make sure that a delete is not present. + $vocab_path = '/admin/structure/taxonomy/manage/' . $this->vocabulary->id() . '/overview'; + $this->drupalGet($vocab_path); + $delete_path = '/admin/structure/taxonomy/manage/' . $this->vocabulary->id() . '/delete'; + $this->assertSession()->linkByHrefNotExists($delete_path); + + // Switch user back to the privileged account. + $this->drupalLogout(); + } + + /** + * Assert that anonymous visitors are not told anything additional about + * this term, besides the existence of the URL as provided by Drupal. + * + * Just a sanity check to help check we didn't break anything. + * + * @throws \Behat\Mink\Exception\ExpectationException + */ + public function testAssertAnonymousGets403() { + $delete_path = '/taxonomy/term/' . $this->term->id() . '/delete'; + $this->drupalGet($delete_path); + $this->assertSession()->statusCodeEquals(403); + } + + public function testAssertUntaggedTermsMayBeDeleted() { + // Switch user to the non-privileged account. + + $this->drupalLogin($this->admin2); + + $path = '/taxonomy/term/' . $this->emptyTerm->id() . '/edit'; + $this->drupalGet($path); + $this->assertSession()->linkExists("Delete"); + + $delete_path = '/taxonomy/term/' . $this->emptyTerm->id() . '/delete'; + + $this->drupalGet($delete_path); + $this->assertSession()->statusCodeEquals(200); + + // Test the overview page to make sure that a delete is present. + $vocab_path = '/admin/structure/taxonomy/manage/' . $this->vocabulary->id(); + $delete_path = '/admin/structure/taxonomy/manage/' . $this->vocabulary->id() . '/delete'; + $this->drupalGet($vocab_path); + $this->assertSession()->linkByHrefExists($delete_path); + + // Switch user back to the privileged account. + $this->drupalLogout(); + $this->drupalLogin($this->admin); + } + + + +} From be01f864ce159a185a1b838e313889a4ce16c3b8 Mon Sep 17 00:00:00 2001 From: Jason Partyka <4501353+jasonpartyka@users.noreply.github.com> Date: Wed, 17 Jul 2019 21:05:22 -0500 Subject: [PATCH 22/39] Forcing a test run. --- .../src/Functional/TaxonomySchemeUITest.php | 62 +++++++++---------- 1 file changed, 31 insertions(+), 31 deletions(-) diff --git a/workbench_access_protect/tests/src/Functional/TaxonomySchemeUITest.php b/workbench_access_protect/tests/src/Functional/TaxonomySchemeUITest.php index 5adba632..f22214dc 100644 --- a/workbench_access_protect/tests/src/Functional/TaxonomySchemeUITest.php +++ b/workbench_access_protect/tests/src/Functional/TaxonomySchemeUITest.php @@ -87,7 +87,7 @@ protected function setUp() { $this->setUpTaxonomyFieldForEntityType('taxonomy_term', $this->vocabulary->id(), $this->vocabulary->id(), 'recursive', 'Recursive Field'); $vocab = Vocabulary::create(['vid' => 'selected', 'name' => 'Selected Vocabulary']); $vocab->save(); - $this->setUpTaxonomyFieldForEntityType('taxonomy_term', $vocab->id(), $this->vocabulary->id(), 'non_recursive', 'Allowed Field'); + $this->setUpTaxonomyFieldForEntityType('taxonomy_term', $this->vocabulary->id(), $this->vocabulary->id(), 'non_recursive', 'Allowed Field'); entity_test_create_bundle('access_controlled'); entity_test_create_bundle('notaccess_controlled'); $this->setUpTaxonomyFieldForEntityType('entity_test', 'access_controlled', $this->vocabulary->id()); @@ -181,36 +181,36 @@ public function testAssertCannotDeleteTaxonomy() { * * @throws \Behat\Mink\Exception\ExpectationException */ - public function testAssertAnonymousGets403() { - $delete_path = '/taxonomy/term/' . $this->term->id() . '/delete'; - $this->drupalGet($delete_path); - $this->assertSession()->statusCodeEquals(403); - } - - public function testAssertUntaggedTermsMayBeDeleted() { - // Switch user to the non-privileged account. - - $this->drupalLogin($this->admin2); - - $path = '/taxonomy/term/' . $this->emptyTerm->id() . '/edit'; - $this->drupalGet($path); - $this->assertSession()->linkExists("Delete"); - - $delete_path = '/taxonomy/term/' . $this->emptyTerm->id() . '/delete'; - - $this->drupalGet($delete_path); - $this->assertSession()->statusCodeEquals(200); - - // Test the overview page to make sure that a delete is present. - $vocab_path = '/admin/structure/taxonomy/manage/' . $this->vocabulary->id(); - $delete_path = '/admin/structure/taxonomy/manage/' . $this->vocabulary->id() . '/delete'; - $this->drupalGet($vocab_path); - $this->assertSession()->linkByHrefExists($delete_path); - - // Switch user back to the privileged account. - $this->drupalLogout(); - $this->drupalLogin($this->admin); - } +// public function testAssertAnonymousGets403() { +// $delete_path = '/taxonomy/term/' . $this->term->id() . '/delete'; +// $this->drupalGet($delete_path); +// $this->assertSession()->statusCodeEquals(403); +// } +// +// public function testAssertUntaggedTermsMayBeDeleted() { +// // Switch user to the non-privileged account. +// +// $this->drupalLogin($this->admin2); +// +// $path = '/taxonomy/term/' . $this->emptyTerm->id() . '/edit'; +// $this->drupalGet($path); +// $this->assertSession()->linkExists("Delete"); +// +// $delete_path = '/taxonomy/term/' . $this->emptyTerm->id() . '/delete'; +// +// $this->drupalGet($delete_path); +// $this->assertSession()->statusCodeEquals(200); +// +// // Test the overview page to make sure that a delete is present. +// $vocab_path = '/admin/structure/taxonomy/manage/' . $this->vocabulary->id(); +// $delete_path = '/admin/structure/taxonomy/manage/' . $this->vocabulary->id() . '/delete'; +// $this->drupalGet($vocab_path); +// $this->assertSession()->linkByHrefExists($delete_path); +// +// // Switch user back to the privileged account. +// $this->drupalLogout(); +// $this->drupalLogin($this->admin); +// } From 3286023d44a864987ebf857ba3d3cbbf4ff49f7b Mon Sep 17 00:00:00 2001 From: jasonpartyka <4501353+jasonpartyka@users.noreply.github.com> Date: Tue, 10 Dec 2019 14:45:01 -0600 Subject: [PATCH 23/39] Update DeleteAccessCheck.php Per suggestion by @biz123 --- workbench_access_protect/src/Access/DeleteAccessCheck.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/workbench_access_protect/src/Access/DeleteAccessCheck.php b/workbench_access_protect/src/Access/DeleteAccessCheck.php index 1132061b..206f1777 100644 --- a/workbench_access_protect/src/Access/DeleteAccessCheck.php +++ b/workbench_access_protect/src/Access/DeleteAccessCheck.php @@ -104,7 +104,7 @@ public function isDeleteAllowed(EntityInterface $entity) { * control, and the term is not assigned to any pieces of content, * it is OK to delete it. */ - if ($hasAccessControlMembers || $assigned_content) { + if ($hasAccessControlMembers && $assigned_content) { if ($assigned_content) { $retval = FALSE; From 7fbcfa4268044c221d58fa960086332f6136e0e3 Mon Sep 17 00:00:00 2001 From: Ken Rickard Date: Wed, 11 Dec 2019 16:59:47 -0500 Subject: [PATCH 24/39] Removed bad unit test. --- .../tests/src/Unit/TaxonomySchemeUITest.php | 217 ------------------ 1 file changed, 217 deletions(-) delete mode 100644 workbench_access_protect/tests/src/Unit/TaxonomySchemeUITest.php diff --git a/workbench_access_protect/tests/src/Unit/TaxonomySchemeUITest.php b/workbench_access_protect/tests/src/Unit/TaxonomySchemeUITest.php deleted file mode 100644 index 6233ed0b..00000000 --- a/workbench_access_protect/tests/src/Unit/TaxonomySchemeUITest.php +++ /dev/null @@ -1,217 +0,0 @@ -createContentType(['type' => 'page']); - $this->createContentType(['type' => 'article']); - $this->vocabulary = $this->setUpVocabulary(); - $this->setUpTaxonomyFieldForEntityType('node', 'page', $this->vocabulary->id()); - $this->setUpTaxonomyFieldForEntityType('taxonomy_term', $this->vocabulary->id(), $this->vocabulary->id(), 'recursive', 'Recursive Field'); - $vocab = Vocabulary::create(['vid' => 'selected', 'name' => 'Selected Vocabulary']); - $vocab->save(); - $this->setUpTaxonomyFieldForEntityType('taxonomy_term', $vocab->id(), $this->vocabulary->id(), 'non_recursive', 'Allowed Field'); - entity_test_create_bundle('access_controlled'); - entity_test_create_bundle('notaccess_controlled'); - $this->setUpTaxonomyFieldForEntityType('entity_test', 'access_controlled', $this->vocabulary->id()); - $this->admin = $this->setUpAdminUser([ - 'administer workbench access', - 'edit terms in workbench_access', - 'delete terms in workbench_access', - 'create terms in workbench_access', - ]); - $this->admin2 = $this->setUpUserUniqueRole([ - 'administer workbench access', - 'assign workbench access', - 'edit terms in workbench_access', - 'delete terms in workbench_access', - 'create terms in workbench_access', - 'access administration pages', - 'access taxonomy overview', - 'administer taxonomy', - ]); - $this->placeBlock('local_actions_block'); - - $this->setUpTestContent(); - - - - } - - protected function setUpTestContent() { - // create a test term - $this->term = Term::create([ - 'name' => 'Test Term', - 'vid' => $this->vocabulary->id(), - ]); - - $this->term->save(); - - $this->emptyTerm = Term::create([ - 'name' => 'Empty Test Term', - 'vid' => $this->vocabulary->id(), - ]); - - $this->emptyTerm->save(); - - - // create some test nodes - $this->testNode = $this->createNode( - [ - 'title' => 'Node', - 'type' => 'page', - 'uid' => $this->admin->id(), - WorkbenchAccessManagerInterface::FIELD_NAME => $this->term->id(), - ] - ); - - $this->testNode->save(); - } - - /** - * Asset a non-administrator cannot delete terms that are actively used - * - * @throws \Behat\Mink\Exception\ExpectationException - */ - public function testAssertCannotDeleteTaxonomy() { - - // Switch user to the non-privileged account. - $this->drupalLogin($this->admin2); - - $path = '/taxonomy/term/' . $this->term->id() . '/edit'; - $this->drupalGet($path); - $this->assertSession()->linkNotExists("Delete"); - - $delete_path = '/taxonomy/term/' . $this->term->id() . '/delete'; - $this->drupalGet($delete_path); - $this->assertSession()->statusCodeEquals(403); - - // Test the overview page to make sure that a delete is not present. - $vocab_path = '/admin/structure/taxonomy/manage/' . $this->vocabulary->id() . '/overview'; - $this->drupalGet($vocab_path); - $delete_path = '/admin/structure/taxonomy/manage/' . $this->vocabulary->id() . '/delete'; - $this->assertSession()->linkByHrefNotExists($delete_path); - - // Switch user back to the privileged account. - $this->drupalLogout(); - } - - /** - * Assert that anonymous visitors are not told anything additional about - * this term, besides the existence of the URL as provided by Drupal. - * - * Just a sanity check to help check we didn't break anything. - * - * @throws \Behat\Mink\Exception\ExpectationException - */ - public function testAssertAnonymousGets403() { - $delete_path = '/taxonomy/term/' . $this->term->id() . '/delete'; - $this->drupalGet($delete_path); - $this->assertSession()->statusCodeEquals(403); - } - - public function testAssertUntaggedTermsMayBeDeleted() { - // Switch user to the non-privileged account. - - $this->drupalLogin($this->admin2); - - $path = '/taxonomy/term/' . $this->emptyTerm->id() . '/edit'; - $this->drupalGet($path); - $this->assertSession()->linkExists("Delete"); - - $delete_path = '/taxonomy/term/' . $this->emptyTerm->id() . '/delete'; - - $this->drupalGet($delete_path); - $this->assertSession()->statusCodeEquals(200); - - // Test the overview page to make sure that a delete is present. - $vocab_path = '/admin/structure/taxonomy/manage/' . $this->vocabulary->id(); - $delete_path = '/admin/structure/taxonomy/manage/' . $this->vocabulary->id() . '/delete'; - $this->drupalGet($vocab_path); - $this->assertSession()->linkByHrefExists($delete_path); - - // Switch user back to the privileged account. - $this->drupalLogout(); - $this->drupalLogin($this->admin); - } - - - -} From d0fa08c4fee70c254cd8f6516de2400766b5a832 Mon Sep 17 00:00:00 2001 From: Ken Rickard Date: Wed, 11 Dec 2019 17:00:13 -0500 Subject: [PATCH 25/39] Removes references to parent_id variable. --- config/schema/workbench_access.schema.yml | 12 ------ src/Plugin/AccessControlHierarchy/Menu.php | 3 -- .../AccessControlHierarchy/Taxonomy.php | 3 -- tests/src/Functional/UpdatePathTest.php | 1 - .../src/Functional/TaxonomySchemeUITest.php | 41 ------------------- 5 files changed, 60 deletions(-) diff --git a/config/schema/workbench_access.schema.yml b/config/schema/workbench_access.schema.yml index 2dcc1d4f..bb79dd65 100644 --- a/config/schema/workbench_access.schema.yml +++ b/config/schema/workbench_access.schema.yml @@ -65,12 +65,6 @@ workbench_access.scheme_settings.taxonomy: field: type: string label: 'Field' - parent_ids: - type: sequence - label: "Vocabulary Parent IDs" - sequence: - type: integer - label: "Parent ID" workbench_access.scheme_settings.menu: type: workbench_access_scheme_settings @@ -87,9 +81,3 @@ workbench_access.scheme_settings.menu: sequence: type: string label: 'Bundle' - parent_ids: - type: sequence - label: "Menu IDs" - sequence: - type: integer - label: "Menu ID" diff --git a/src/Plugin/AccessControlHierarchy/Menu.php b/src/Plugin/AccessControlHierarchy/Menu.php index 2514993e..0219f0c4 100644 --- a/src/Plugin/AccessControlHierarchy/Menu.php +++ b/src/Plugin/AccessControlHierarchy/Menu.php @@ -257,8 +257,6 @@ public function applies($entity_type_id, $bundle) { */ public function submitConfigurationForm(array &$form, FormStateInterface $form_state) { $this->configuration['menus'] = array_values(array_filter($form_state->getValue('menus'))); - // Parent_ids is the machine name of the selected menus. - $this->configuration['parent_ids'] = $this->configuration['menus']; $this->configuration['bundles'] = array_values(array_filter($form_state->getValue('bundles'))); } @@ -294,7 +292,6 @@ public function defaultConfiguration() { return [ 'menus' => [], 'bundles' => [], - 'parent_ids' => [], ]; } diff --git a/src/Plugin/AccessControlHierarchy/Taxonomy.php b/src/Plugin/AccessControlHierarchy/Taxonomy.php index 762ded3e..73e5edb5 100644 --- a/src/Plugin/AccessControlHierarchy/Taxonomy.php +++ b/src/Plugin/AccessControlHierarchy/Taxonomy.php @@ -125,7 +125,6 @@ public function defaultConfiguration() { $defaults = [ 'fields' => [], 'vocabularies' => [], - 'parent_ids' => [], ]; return $defaults + parent::defaultConfiguration(); } @@ -454,8 +453,6 @@ public function submitConfigurationForm(array &$form, FormStateInterface $form_s // Saving 'validate' can cause schema errors. unset($settings['validate']); $settings['vocabularies'] = array_values(array_filter($settings['vocabularies'])); - // Parent_ids is the machine name of the selected vocabularies. - $settings['parent_ids'] = $settings['vocabularies']; $settings['fields'] = array_values(array_map(function ($item) { list($entity_type, $bundle, $field_name) = explode(':', $item); return [ diff --git a/tests/src/Functional/UpdatePathTest.php b/tests/src/Functional/UpdatePathTest.php index 5d3a3415..8de8a5c5 100644 --- a/tests/src/Functional/UpdatePathTest.php +++ b/tests/src/Functional/UpdatePathTest.php @@ -61,7 +61,6 @@ public function testUpdatePath() { ], ], 'vocabularies' => ['tags'], - 'parent_ids' => [], ], $scheme->getAccessScheme()->getConfiguration()); // Test that user storage was updated. diff --git a/workbench_access_protect/tests/src/Functional/TaxonomySchemeUITest.php b/workbench_access_protect/tests/src/Functional/TaxonomySchemeUITest.php index f22214dc..2630dab5 100644 --- a/workbench_access_protect/tests/src/Functional/TaxonomySchemeUITest.php +++ b/workbench_access_protect/tests/src/Functional/TaxonomySchemeUITest.php @@ -173,45 +173,4 @@ public function testAssertCannotDeleteTaxonomy() { $this->drupalLogout(); } - /** - * Assert that anonymous visitors are not told anything additional about - * this term, besides the existence of the URL as provided by Drupal. - * - * Just a sanity check to help check we didn't break anything. - * - * @throws \Behat\Mink\Exception\ExpectationException - */ -// public function testAssertAnonymousGets403() { -// $delete_path = '/taxonomy/term/' . $this->term->id() . '/delete'; -// $this->drupalGet($delete_path); -// $this->assertSession()->statusCodeEquals(403); -// } -// -// public function testAssertUntaggedTermsMayBeDeleted() { -// // Switch user to the non-privileged account. -// -// $this->drupalLogin($this->admin2); -// -// $path = '/taxonomy/term/' . $this->emptyTerm->id() . '/edit'; -// $this->drupalGet($path); -// $this->assertSession()->linkExists("Delete"); -// -// $delete_path = '/taxonomy/term/' . $this->emptyTerm->id() . '/delete'; -// -// $this->drupalGet($delete_path); -// $this->assertSession()->statusCodeEquals(200); -// -// // Test the overview page to make sure that a delete is present. -// $vocab_path = '/admin/structure/taxonomy/manage/' . $this->vocabulary->id(); -// $delete_path = '/admin/structure/taxonomy/manage/' . $this->vocabulary->id() . '/delete'; -// $this->drupalGet($vocab_path); -// $this->assertSession()->linkByHrefExists($delete_path); -// -// // Switch user back to the privileged account. -// $this->drupalLogout(); -// $this->drupalLogin($this->admin); -// } - - - } From 03fcfa3a802a343b3357fa66a24a702a13616eaf Mon Sep 17 00:00:00 2001 From: Ken Rickard Date: Thu, 12 Dec 2019 15:49:38 -0500 Subject: [PATCH 26/39] Cleans up DeleteAccessCheck. --- .../src/Access/DeleteAccessCheck.php | 99 +++++++------------ 1 file changed, 34 insertions(+), 65 deletions(-) diff --git a/workbench_access_protect/src/Access/DeleteAccessCheck.php b/workbench_access_protect/src/Access/DeleteAccessCheck.php index 206f1777..3aec8157 100644 --- a/workbench_access_protect/src/Access/DeleteAccessCheck.php +++ b/workbench_access_protect/src/Access/DeleteAccessCheck.php @@ -14,7 +14,7 @@ use Drupal\workbench_access\Entity\AccessSchemeInterface; /** - * Class TaxonomyDeleteAccessCheck. + * Class DeleteAccessCheck. */ class DeleteAccessCheck implements DeleteAccessCheckInterface { @@ -91,35 +91,23 @@ public function access() { * @{inheritdoc} */ public function isDeleteAllowed(EntityInterface $entity) { + $return = TRUE; - $retval = TRUE; - - if ($entity instanceof EntityInterface) { - - $hasAccessControlMembers = $this->doesTermHaveMembers($entity); - $assigned_content = $this->isAssignedToContent($entity); - - /* - * If this entity does not have users assigned to it for access - * control, and the term is not assigned to any pieces of content, - * it is OK to delete it. - */ - if ($hasAccessControlMembers && $assigned_content) { - - if ($assigned_content) { - $retval = FALSE; - } - - if ($hasAccessControlMembers) { - $retval = FALSE; - } - } + $assigned_members = $this->doesTermHaveMembers($entity); + $assigned_content = $this->isAssignedToContent($entity); + // If this entity does not have users assigned to it for access control and + // is not assigned to any pieces of content, it is OK to delete it. + if ($assigned_members || $assigned_content) { + $return = FALSE; } - return $retval; + return $return; } + /** + * @{inheritdoc} + */ public function getBundles(EntityInterface $entity) { $bundle = $this->entityTypeManager->getDefinitions()[$entity->getEntityTypeId()]->get('bundle_of'); $bundles = $this->entityTypeBundleInfo->getBundleInfo($bundle); @@ -127,6 +115,9 @@ public function getBundles(EntityInterface $entity) { } + /** + * @{inheritdoc} + */ public function hasBundles(EntityInterface $entity) { if ($this->entityTypeManager->getDefinitions()[$entity->getEntityTypeId()]->get('bundle_of') == null) { return FALSE; @@ -165,27 +156,13 @@ public function isDeleteAllowedBundle($bundle, $entity){ * * @return bool * TRUE if the term has members, FALSE otherwise. - * - * @throws \Drupal\Component\Plugin\Exception\InvalidPluginDefinitionException - * @throws \Drupal\Component\Plugin\Exception\PluginNotFoundException */ private function doesTermHaveMembers(EntityInterface $entity) { - - /** @var array $sections */ - $sections = $this->getActiveSections($entity); - - if (count($sections) > 0) { - return TRUE; - } - - return FALSE; + return (bool) (count($this->getActiveSections($entity)) > 0); } - /** - * Inspect the given taxonomy term. - * - * This will determine if there are any active users assigned to it. + * Inspects the given entity for active users. * * @param \Drupal\Core\Entity\EntityInterface $entity * The entity to inspect. @@ -193,8 +170,6 @@ private function doesTermHaveMembers(EntityInterface $entity) { * @return array * An array of the users assigned to this section. * - * @throws \Drupal\Component\Plugin\Exception\InvalidPluginDefinitionException - * @throws \Drupal\Component\Plugin\Exception\PluginNotFoundException */ private function getActiveSections(EntityInterface $entity) { /** @var \Drupal\workbench_access\UserSectionStorageInterface $sectionStorage */ @@ -210,40 +185,31 @@ function (array $editors, AccessSchemeInterface $scheme) use ($sectionStorage, $ } /** - * Determine if tagged content exists. - * - * This method will determine if any entities exist in the system that are - * tagged with the term. + * Inspects the given entity for active content. * * @param \Drupal\Core\Entity\EntityInterface $entity - * The Entity to inspect. + * The entity to inspect. * * @return bool - * TRUE if content is assigned to this term. - * FALSE if content is not assigned to this term. + * TRUE if content is assigned to this entity. + * FALSE if content is not assigned to this entity. * */ private function isAssignedToContent(EntityInterface $entity) { - - $reference_fields = $this->getAllReferenceFields($entity); - - foreach ($reference_fields as $name => $fieldConfig) { - // Get the entity reference and determine if it's a taxonomy. - if ($fieldConfig instanceof FieldStorageConfig) { - $entities = \Drupal::entityQuery($fieldConfig->get('entity_type')) - ->condition($fieldConfig->get('field_name'), $entity->id()) - ->range(0, 1) - ->execute(); - - if (count($entities) > 0) { - return TRUE; - } + foreach ($this->getAllReferenceFields($entity) as $name => $fieldConfig) { + // Get the entity reference and determine if it's a taxonomy. + if ($fieldConfig instanceof FieldStorageConfig) { + $entities = \Drupal::entityQuery($fieldConfig->get('entity_type')) + ->condition($fieldConfig->get('field_name'), $entity->id()) + ->range(0, 1) + ->execute(); + if (count($entities) > 0) { + return TRUE; } + } } - return FALSE; - } private function getAllReferenceFields(EntityInterface $entity) { @@ -278,6 +244,9 @@ private function getAllReferenceFields(EntityInterface $entity) { return $found_fields; } + /** + * @{inheritdoc} + */ public function isAccessControlled(EntityInterface $entity) { $schemes = $this->entityTypeManager->getStorage('access_scheme')->loadMultiple(); /** @var \Drupal\workbench_access\Entity\AccessScheme $scheme */ From 35b56141520fcff8b8662e91059b04f0f5874e17 Mon Sep 17 00:00:00 2001 From: Ken Rickard Date: Fri, 13 Dec 2019 10:56:03 -0500 Subject: [PATCH 27/39] Cleans up some code style and naming issues. --- .../src/Access/DeleteAccessCheck.php | 13 ++++++----- .../src/Access/DeleteAccessCheckInterface.php | 22 +++++++++---------- 2 files changed, 17 insertions(+), 18 deletions(-) diff --git a/workbench_access_protect/src/Access/DeleteAccessCheck.php b/workbench_access_protect/src/Access/DeleteAccessCheck.php index 3aec8157..92b4527d 100644 --- a/workbench_access_protect/src/Access/DeleteAccessCheck.php +++ b/workbench_access_protect/src/Access/DeleteAccessCheck.php @@ -93,8 +93,8 @@ public function access() { public function isDeleteAllowed(EntityInterface $entity) { $return = TRUE; - $assigned_members = $this->doesTermHaveMembers($entity); - $assigned_content = $this->isAssignedToContent($entity); + $assigned_members = $this->hasMembers($entity); + $assigned_content = $this->hasContent($entity); // If this entity does not have users assigned to it for access control and // is not assigned to any pieces of content, it is OK to delete it. @@ -111,8 +111,8 @@ public function isDeleteAllowed(EntityInterface $entity) { public function getBundles(EntityInterface $entity) { $bundle = $this->entityTypeManager->getDefinitions()[$entity->getEntityTypeId()]->get('bundle_of'); $bundles = $this->entityTypeBundleInfo->getBundleInfo($bundle); - return array_combine(array_keys($bundles), array_keys($bundles)); + return array_combine(array_keys($bundles), array_keys($bundles)); } /** @@ -122,11 +122,12 @@ public function hasBundles(EntityInterface $entity) { if ($this->entityTypeManager->getDefinitions()[$entity->getEntityTypeId()]->get('bundle_of') == null) { return FALSE; } + return TRUE; } /** - * {@inheritDoc} + * {@inheritdoc} */ public function isDeleteAllowedBundle($bundle, $entity){ $bundle_of = $this->entityTypeManager->getDefinitions()[$entity->getEntityTypeId()]->get('bundle_of'); @@ -157,7 +158,7 @@ public function isDeleteAllowedBundle($bundle, $entity){ * @return bool * TRUE if the term has members, FALSE otherwise. */ - private function doesTermHaveMembers(EntityInterface $entity) { + private function hasMembers(EntityInterface $entity) { return (bool) (count($this->getActiveSections($entity)) > 0); } @@ -195,7 +196,7 @@ function (array $editors, AccessSchemeInterface $scheme) use ($sectionStorage, $ * FALSE if content is not assigned to this entity. * */ - private function isAssignedToContent(EntityInterface $entity) { + private function hasContent(EntityInterface $entity) { foreach ($this->getAllReferenceFields($entity) as $name => $fieldConfig) { // Get the entity reference and determine if it's a taxonomy. if ($fieldConfig instanceof FieldStorageConfig) { diff --git a/workbench_access_protect/src/Access/DeleteAccessCheckInterface.php b/workbench_access_protect/src/Access/DeleteAccessCheckInterface.php index c1ccff23..95b8677e 100644 --- a/workbench_access_protect/src/Access/DeleteAccessCheckInterface.php +++ b/workbench_access_protect/src/Access/DeleteAccessCheckInterface.php @@ -9,12 +9,11 @@ */ interface DeleteAccessCheckInterface { - /** - * Determine if this entity may be deleted. + * Determines if this entity may be deleted. * * @param EntityInterface $entity - * The term to check. + * The entity to check. * * @return bool * TRUE if it may be deleted, FALSE otherwise. @@ -25,27 +24,29 @@ interface DeleteAccessCheckInterface { public function isDeleteAllowed(EntityInterface $entity); /** - * Determine if this entity has bundles or not. + * Determines if this entity has bundles or not. * * Use this to determine if the entity has children bundles. * * @param EntityInterface $entity * * @return bool - * TRUE if the entity has bundles, FALSE otherwise + * TRUE if the entity has bundles, FALSE otherwise */ public function hasBundles(EntityInterface $entity); /** + * Gets the assigned bundles associated with an entity. + * * @param \Drupal\Core\Entity\EntityInterface $entity * - * @return mixed + * @return array + * An array where the keys and values are bundle ids. */ public function getBundles(EntityInterface $entity); /** - * Check an entire bundle to see if any of the bundle's - * entities are being actively used for access control. + * Checks if a bundle's entities are being actively used for access control. * * @param string $bundle * The bundle ID @@ -53,12 +54,11 @@ public function getBundles(EntityInterface $entity); * The entity id * * @return bool - * TRUE */ public function isDeleteAllowedBundle($bundle, $entity); /** - * Helper method to determine if the entity is used for access control. + * Determines if the entity is used for access control. * * This method is used to determine if we should actually perform the usage * check or skip. @@ -71,6 +71,4 @@ public function isDeleteAllowedBundle($bundle, $entity); */ public function isAccessControlled(EntityInterface $entity); - - } From 2de4c8fd9d8d28f6c0050733470622cc6da06208 Mon Sep 17 00:00:00 2001 From: Ken Rickard Date: Tue, 17 Dec 2019 14:12:28 -0500 Subject: [PATCH 28/39] Renames taxonomy protect test properly. --- .../{TaxonomySchemeUITest.php => TaxonomyProtectTest.php} | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) rename workbench_access_protect/tests/src/Functional/{TaxonomySchemeUITest.php => TaxonomyProtectTest.php} (97%) diff --git a/workbench_access_protect/tests/src/Functional/TaxonomySchemeUITest.php b/workbench_access_protect/tests/src/Functional/TaxonomyProtectTest.php similarity index 97% rename from workbench_access_protect/tests/src/Functional/TaxonomySchemeUITest.php rename to workbench_access_protect/tests/src/Functional/TaxonomyProtectTest.php index 2630dab5..ef4e64d7 100644 --- a/workbench_access_protect/tests/src/Functional/TaxonomySchemeUITest.php +++ b/workbench_access_protect/tests/src/Functional/TaxonomyProtectTest.php @@ -9,11 +9,11 @@ use Drupal\workbench_access\WorkbenchAccessManagerInterface; /** - * Defines a class for testing the UI to create and configure schemes. + * Tests protection of Taxonomy used for access control. * * @group workbench_access_protect */ -class TaxonomySchemeUITest extends BrowserTestBase { +class TaxonomyProtectTest extends BrowserTestBase { use WorkbenchAccessTestTrait; From b1a1e967d6142034682359c9a3793006b9ee3829 Mon Sep 17 00:00:00 2001 From: Ken Rickard Date: Tue, 17 Dec 2019 15:18:14 -0500 Subject: [PATCH 29/39] Passing tests. --- .../src/Functional/TaxonomyProtectTest.php | 141 +++++++++++------- 1 file changed, 87 insertions(+), 54 deletions(-) diff --git a/workbench_access_protect/tests/src/Functional/TaxonomyProtectTest.php b/workbench_access_protect/tests/src/Functional/TaxonomyProtectTest.php index ef4e64d7..ed45ec48 100644 --- a/workbench_access_protect/tests/src/Functional/TaxonomyProtectTest.php +++ b/workbench_access_protect/tests/src/Functional/TaxonomyProtectTest.php @@ -29,29 +29,42 @@ class TaxonomyProtectTest extends BrowserTestBase { * * @var \Drupal\user\UserInterface */ - protected $admin2; + protected $editor; /** - * Vocabulary. + * Vocabulary used for access control. * * @var \Drupal\taxonomy\VocabularyInterface */ protected $vocabulary; /** - * A Test term + * Vocabulary not used for access control. + * + * @var \Drupal\taxonomy\VocabularyInterface + */ + protected $emptyVocabulary; + + /** + * A test term * * @var \Drupal\taxonomy\Entity\Term; */ protected $term; /** - * A Test term that is never tagged in content. + * A test term that is never tagged in content. * * @var \Drupal\taxonomy\Entity\Term; */ protected $emptyTerm; + /** + * A test term that is never tagged in content. + * + * @var \Drupal\taxonomy\Entity\Term; + */ + protected $emptyVocabTerm; /** * A Test Node @@ -64,15 +77,13 @@ class TaxonomyProtectTest extends BrowserTestBase { * {@inheritdoc} */ public static $modules = [ - 'workbench_access', - 'workbench_access_protect', 'node', - 'taxonomy', 'options', - 'user', - 'block', - 'entity_test', 'system', + 'taxonomy', + 'user', + 'workbench_access', + 'workbench_access_protect', ]; /** @@ -80,24 +91,24 @@ class TaxonomyProtectTest extends BrowserTestBase { */ protected function setUp() { parent::setUp(); - $this->createContentType(['type' => 'page']); - $this->createContentType(['type' => 'article']); + // Set up a content type, taxonomy field, and taxonomy scheme. $this->vocabulary = $this->setUpVocabulary(); - $this->setUpTaxonomyFieldForEntityType('node', 'page', $this->vocabulary->id()); - $this->setUpTaxonomyFieldForEntityType('taxonomy_term', $this->vocabulary->id(), $this->vocabulary->id(), 'recursive', 'Recursive Field'); - $vocab = Vocabulary::create(['vid' => 'selected', 'name' => 'Selected Vocabulary']); - $vocab->save(); - $this->setUpTaxonomyFieldForEntityType('taxonomy_term', $this->vocabulary->id(), $this->vocabulary->id(), 'non_recursive', 'Allowed Field'); - entity_test_create_bundle('access_controlled'); - entity_test_create_bundle('notaccess_controlled'); - $this->setUpTaxonomyFieldForEntityType('entity_test', 'access_controlled', $this->vocabulary->id()); + $node_type = $this->createContentType(['type' => 'page']); + $field = $this->setUpTaxonomyFieldForEntityType('node', $node_type->id(), $this->vocabulary->id()); + $scheme = $this->setUpTaxonomyScheme($node_type, $this->vocabulary); + + // Set up an non-access control vocabulary as a control. + $this->emptyVocabulary = Vocabulary::create(['vid' => 'empty_vocabulary', 'name' => 'Empty Vocabulary']); + $this->emptyVocabulary->save(); + + // Create users. $this->admin = $this->setUpAdminUser([ 'administer workbench access', 'edit terms in workbench_access', 'delete terms in workbench_access', 'create terms in workbench_access', ]); - $this->admin2 = $this->setUpUserUniqueRole([ + $this->editor = $this->setUpUserUniqueRole([ 'administer workbench access', 'assign workbench access', 'edit terms in workbench_access', @@ -107,12 +118,6 @@ protected function setUp() { 'access taxonomy overview', 'administer taxonomy', ]); - $this->placeBlock('local_actions_block'); - - $this->setUpTestContent(); - - - } protected function setUpTestContent() { @@ -121,18 +126,20 @@ protected function setUpTestContent() { 'name' => 'Test Term', 'vid' => $this->vocabulary->id(), ]); - $this->term->save(); $this->emptyTerm = Term::create([ 'name' => 'Empty Test Term', 'vid' => $this->vocabulary->id(), ]); - $this->emptyTerm->save(); + $this->emptyVocabTerm = Term::create([ + 'name' => 'Empty Test Term', + 'vid' => $this->emptyVocabulary->id(), + ]); + $this->emptyVocabTerm->save(); - // create some test nodes $this->testNode = $this->createNode( [ 'title' => 'Node', @@ -141,36 +148,62 @@ protected function setUpTestContent() { WorkbenchAccessManagerInterface::FIELD_NAME => $this->term->id(), ] ); - $this->testNode->save(); } /** * Asset a non-administrator cannot delete terms that are actively used - * - * @throws \Behat\Mink\Exception\ExpectationException */ - public function testAssertCannotDeleteTaxonomy() { - - // Switch user to the non-privileged account. - $this->drupalLogin($this->admin2); - - $path = '/taxonomy/term/' . $this->term->id() . '/edit'; - $this->drupalGet($path); - $this->assertSession()->linkNotExistsExact("Delete"); - - $delete_path = '/taxonomy/term/' . $this->term->id() . '/delete'; - $this->drupalGet($delete_path); - $this->assertSession()->statusCodeEquals(403); - - // Test the overview page to make sure that a delete is not present. - $vocab_path = '/admin/structure/taxonomy/manage/' . $this->vocabulary->id() . '/overview'; - $this->drupalGet($vocab_path); - $delete_path = '/admin/structure/taxonomy/manage/' . $this->vocabulary->id() . '/delete'; - $this->assertSession()->linkByHrefNotExists($delete_path); + public function testCannotDeleteTaxonomyWithAssignedNode() { + $this->setUpTestContent(); + // Login to the non-privileged account. + $this->drupalLogin($this->editor); + + // Restricted term that has content. + $path = '/taxonomy/term/' . $this->term->id() . '/edit'; + $this->drupalGet($path); + $delete_path = '/taxonomy/term/' . $this->term->id() . '/delete'; + $this->assertSession()->linkByHrefNotExists($delete_path); + $this->drupalGet($delete_path); + $this->assertSession()->statusCodeEquals(403); + + // Unrestricted term that has no content. + $path = '/taxonomy/term/' . $this->emptyTerm->id() . '/edit'; + $this->drupalGet($path); + $delete_path = '/taxonomy/term/' . $this->emptyTerm->id() . '/delete'; + $this->assertSession()->linkByHrefExists($delete_path); + $this->drupalGet($delete_path); + $this->assertSession()->statusCodeEquals(200); + + $path = '/taxonomy/term/' . $this->emptyVocabTerm->id() . '/edit'; + $this->drupalGet($path); + $delete_path = '/taxonomy/term/' . $this->emptyVocabTerm->id() . '/delete'; + $this->assertSession()->linkByHrefExists($delete_path); + $delete_path = '/taxonomy/term/' . $this->emptyVocabTerm->id() . '/delete'; + $this->drupalGet($delete_path); + $this->assertSession()->statusCodeEquals(200); + + // Test for a delete link on the vocabularies. + $vocab_path = '/admin/structure/taxonomy/manage/' . $this->vocabulary->id() . '/delete'; + $this->drupalGet($vocab_path); + $this->assertSession()->statusCodeEquals(403); + $vocab_path = '/admin/structure/taxonomy/manage/' . $this->emptyVocabulary->id() . '/delete'; + $this->drupalGet($vocab_path); + $this->assertSession()->statusCodeEquals(200); + + // Test the overview page to make sure that the delete link is handled. + $vocab_path = '/admin/structure/taxonomy/manage/' . $this->vocabulary->id() . '/overview'; + $this->drupalGet($vocab_path); + $delete_path = '/taxonomy/term/' . $this->term->id() . '/delete'; + $this->assertSession()->linkByHrefNotExists($delete_path); + $delete_path = '/taxonomy/term/' . $this->emptyTerm->id() . '/delete'; + $this->assertSession()->linkByHrefExists($delete_path); + + $vocab_path = '/admin/structure/taxonomy/manage/' . $this->emptyVocabulary->id() . '/overview'; + $this->drupalGet($vocab_path); + $delete_path = '/taxonomy/term/' . $this->emptyVocabTerm->id() . '/delete'; + $this->assertSession()->linkByHrefExists($delete_path); - // Switch user back to the privileged account. - $this->drupalLogout(); - } + } } From 43a2cd9a55e9a3c79acf351dd2745f983ccff24e Mon Sep 17 00:00:00 2001 From: Ken Rickard Date: Tue, 17 Dec 2019 15:30:05 -0500 Subject: [PATCH 30/39] Extends tests to cover user protection. --- .../src/Functional/TaxonomyProtectTest.php | 85 +++++++++++++------ 1 file changed, 58 insertions(+), 27 deletions(-) diff --git a/workbench_access_protect/tests/src/Functional/TaxonomyProtectTest.php b/workbench_access_protect/tests/src/Functional/TaxonomyProtectTest.php index ed45ec48..848e2ee9 100644 --- a/workbench_access_protect/tests/src/Functional/TaxonomyProtectTest.php +++ b/workbench_access_protect/tests/src/Functional/TaxonomyProtectTest.php @@ -7,6 +7,7 @@ use Drupal\Tests\workbench_access\Traits\WorkbenchAccessTestTrait; use Drupal\taxonomy\Entity\Term; use Drupal\workbench_access\WorkbenchAccessManagerInterface; +use Drupal\workbench_access\Entity\AccessSchemeInterface; /** * Tests protection of Taxonomy used for access control. @@ -55,23 +56,23 @@ class TaxonomyProtectTest extends BrowserTestBase { /** * A test term that is never tagged in content. * - * @var \Drupal\taxonomy\Entity\Term; + * @var \Drupal\taxonomy\Entity\Term */ protected $emptyTerm; /** * A test term that is never tagged in content. * - * @var \Drupal\taxonomy\Entity\Term; + * @var \Drupal\taxonomy\Entity\Term */ protected $emptyVocabTerm; /** - * A Test Node + * The test access scheme. * - * @var \Drupal\node\Entity\Node; + * @var \Drupal\workbench_access\Entity\AccessSchemeInterface */ - protected $testNode; + protected $scheme; /** * {@inheritdoc} @@ -95,12 +96,31 @@ protected function setUp() { $this->vocabulary = $this->setUpVocabulary(); $node_type = $this->createContentType(['type' => 'page']); $field = $this->setUpTaxonomyFieldForEntityType('node', $node_type->id(), $this->vocabulary->id()); - $scheme = $this->setUpTaxonomyScheme($node_type, $this->vocabulary); + $this->scheme = $this->setUpTaxonomyScheme($node_type, $this->vocabulary); // Set up an non-access control vocabulary as a control. $this->emptyVocabulary = Vocabulary::create(['vid' => 'empty_vocabulary', 'name' => 'Empty Vocabulary']); $this->emptyVocabulary->save(); + // Create terms. + $this->term = Term::create([ + 'name' => 'Test Term', + 'vid' => $this->vocabulary->id(), + ]); + $this->term->save(); + + $this->emptyTerm = Term::create([ + 'name' => 'Empty Test Term', + 'vid' => $this->vocabulary->id(), + ]); + $this->emptyTerm->save(); + + $this->emptyVocabTerm = Term::create([ + 'name' => 'Empty Test Term', + 'vid' => $this->emptyVocabulary->id(), + ]); + $this->emptyVocabTerm->save(); + // Create users. $this->admin = $this->setUpAdminUser([ 'administer workbench access', @@ -120,26 +140,10 @@ protected function setUp() { ]); } + /** + * Creates content for the test. + */ protected function setUpTestContent() { - // create a test term - $this->term = Term::create([ - 'name' => 'Test Term', - 'vid' => $this->vocabulary->id(), - ]); - $this->term->save(); - - $this->emptyTerm = Term::create([ - 'name' => 'Empty Test Term', - 'vid' => $this->vocabulary->id(), - ]); - $this->emptyTerm->save(); - - $this->emptyVocabTerm = Term::create([ - 'name' => 'Empty Test Term', - 'vid' => $this->emptyVocabulary->id(), - ]); - $this->emptyVocabTerm->save(); - $this->testNode = $this->createNode( [ 'title' => 'Node', @@ -152,10 +156,38 @@ protected function setUpTestContent() { } /** - * Asset a non-administrator cannot delete terms that are actively used + * Assigns users for the test. + */ + protected function setUpTestUser() { + $user_storage = \Drupal::service('workbench_access.user_section_storage'); + $role_storage = \Drupal::service('workbench_access.role_section_storage'); + // Add the user to the base section. + $user_storage->addUser($this->scheme, $this->editor, [$this->term->id()]); + $expected = [$this->editor->id()]; + $existing_users = $user_storage->getEditors($this->scheme, $this->term->id()); + $this->assertEquals($expected, array_keys($existing_users)); + } + + /** + * Assert a non-administrator cannot delete terms used by nodes. */ public function testCannotDeleteTaxonomyWithAssignedNode() { $this->setUpTestContent(); + $this->assertTests(); + } + + /** + * Assert a non-administrator cannot delete terms used by users. + */ + public function testCannotDeleteTaxonomyWithAssignedUser() { + $this->setUpTestUser(); + $this->assertTests(); + } + + /** + * Runs our tests for both content and users. + */ + private function assertTests() { // Login to the non-privileged account. $this->drupalLogin($this->editor); @@ -203,7 +235,6 @@ public function testCannotDeleteTaxonomyWithAssignedNode() { $this->drupalGet($vocab_path); $delete_path = '/taxonomy/term/' . $this->emptyVocabTerm->id() . '/delete'; $this->assertSession()->linkByHrefExists($delete_path); - } } From 87436a8222e4acb9da54f6b9d08d46ca78f30585 Mon Sep 17 00:00:00 2001 From: Ken Rickard Date: Thu, 19 Dec 2019 13:21:24 -0500 Subject: [PATCH 31/39] Adds stubs for menu testing. Requires special handling in access checks. --- .../src/Access/DeleteAccessCheck.php | 5 +- .../tests/src/Functional/MenuProtectTest.php | 296 ++++++++++++++++++ 2 files changed, 298 insertions(+), 3 deletions(-) create mode 100644 workbench_access_protect/tests/src/Functional/MenuProtectTest.php diff --git a/workbench_access_protect/src/Access/DeleteAccessCheck.php b/workbench_access_protect/src/Access/DeleteAccessCheck.php index 92b4527d..22652fba 100644 --- a/workbench_access_protect/src/Access/DeleteAccessCheck.php +++ b/workbench_access_protect/src/Access/DeleteAccessCheck.php @@ -194,11 +194,10 @@ function (array $editors, AccessSchemeInterface $scheme) use ($sectionStorage, $ * @return bool * TRUE if content is assigned to this entity. * FALSE if content is not assigned to this entity. - * */ private function hasContent(EntityInterface $entity) { foreach ($this->getAllReferenceFields($entity) as $name => $fieldConfig) { - // Get the entity reference and determine if it's a taxonomy. + // Get the entity reference and determine if it is access controlled. if ($fieldConfig instanceof FieldStorageConfig) { $entities = \Drupal::entityQuery($fieldConfig->get('entity_type')) ->condition($fieldConfig->get('field_name'), $entity->id()) @@ -223,7 +222,7 @@ private function getAllReferenceFields(EntityInterface $entity) { foreach ($map as $entity_type => $fields) { foreach ($fields as $name => $field) { if ($field['type'] === 'entity_reference') { - // Get the entity reference and determine if it's a taxonomy. + // Get the entity reference and determine if it is access controlled. /** @var \Drupal\field\Entity\FieldStorageConfig $fieldConfig */ $fieldConfig = FieldStorageConfig::loadByName($entity_type, $name); if ($fieldConfig !== NULL && diff --git a/workbench_access_protect/tests/src/Functional/MenuProtectTest.php b/workbench_access_protect/tests/src/Functional/MenuProtectTest.php new file mode 100644 index 00000000..7f41f658 --- /dev/null +++ b/workbench_access_protect/tests/src/Functional/MenuProtectTest.php @@ -0,0 +1,296 @@ +createContentType(['type' => 'page']); + $this->scheme = $this->setUpMenuScheme(['page'], ['main']); + + // Set up a menu for access control. + $this->menu = Menu::create([ + 'id' => 'menu_test', + 'label' => 'Test menu', + 'description' => 'Description text', + ])->save(); + // Set up an non-access control menu as a control. + $this->emptyMenu = Menu::create([ + 'id' => 'empty_test', + 'label' => 'Empty menu', + 'description' => 'Description text', + ])->save(); + + // Allow nodes to be assigned to the menu. + $node_type->setThirdPartySetting('menu_ui', 'available_menus', ['menu_test', 'empty_test']); + $node_type->save(); + + // Set up some menu links for this test. + $this->link = MenuLinkContent::create([ + 'title' => 'Link 1', + 'link' => [['uri' => 'route:']], + 'menu_name' => 'menu_test', + ]); + $this->link->save(); + $this->emptyLink = MenuLinkContent::create([ + 'title' => 'Link 2', + 'link' => [['uri' => 'route:']], + 'menu_name' => 'menu_test', + ]); + $this->emptyLink->save(); + $this->emptyMenuLink = MenuLinkContent::create([ + 'title' => 'Link 3', + 'link' => [['uri' => 'route:']], + 'menu_name' => 'empty_test', + ]); + $this->emptyMenuLink->save(); + + // Create users. + $this->admin = $this->setUpAdminUser([ + 'administer workbench access', + 'administer menu', + 'link to any page' + ]); + $this->editor = $this->setUpUserUniqueRole([ + 'administer workbench access', + 'assign workbench access', + 'administer menu', + 'link to any page' + ]); + } + + /** + * Creates content for the test. + */ + protected function setUpTestContent() { + $testNode = $this->createNode( + [ + 'title' => 'Node', + 'type' => 'page', + 'uid' => $this->admin->id(), + ] + ); + _menu_ui_node_save($testNode, [ + 'title' => 'foo', + 'menu_name' => 'menu_test', + 'description' => 'view foo', + 'entity_id' => $this->link->id(), + 'parent' => NULL, + ]); + $testNode2 = $this->createNode( + [ + 'title' => 'Node 2', + 'type' => 'page', + 'uid' => $this->admin->id(), + ] + ); + _menu_ui_node_save($testNode2, [ + 'title' => 'bar', + 'menu_name' => 'menu_test', + 'description' => 'view bar', + 'entity_id' => $this->emptyLink->id(), + 'parent' => NULL, + ]); + $testNode3 = $this->createNode( + [ + 'title' => 'Node 3', + 'type' => 'page', + 'uid' => $this->admin->id(), + ] + ); + _menu_ui_node_save($testNode3, [ + 'title' => 'baz', + 'menu_name' => 'empty_test', + 'description' => 'view baz', + 'entity_id' => $this->emptyMenuLink->id(), + 'parent' => NULL, + ]); + } + + /** + * Assigns users for the test. + */ + protected function setUpTestUser() { + $user_storage = \Drupal::service('workbench_access.user_section_storage'); + // Add the user to the base section. + $user_storage->addUser($this->scheme, $this->editor, [$this->link->getPluginId()]); + $expected = [$this->editor->id()]; + $existing_users = $user_storage->getEditors($this->scheme, $this->link->getPluginId()); + $this->assertEquals($expected, array_keys($existing_users)); + } + + /** + * Assert a non-administrator cannot delete terms used by nodes. + */ + public function testCannotDeleteMenuWithAssignedNode() { + $this->setUpTestContent(); + $this->assertTests(); + } + + /** + * Assert a non-administrator cannot delete terms used by users. + */ + public function testCannotDeleteMenuWithAssignedUser() { + $this->setUpTestUser(); + $this->assertTests(); + } + + /** + * Runs our tests for both content and users. + */ + private function assertTests() { + // Login to the non-privileged account. + $this->drupalLogin($this->editor); + +// overview: /admin/structure/menu/manage/menu_test +// delete: /admin/structure/menu/manage/menu_test/delete +// overview: /admin/structure/menu/manage/empty_test +// delete: /admin/structure/menu/manage/empty_test/delete +// link: /admin/structure/menu/item/1/edit +// Link: /admin/structure/menu/item/1/delete + + + // Restricted link that has content. + $path = '/admin/structure/menu/item/' . $this->link->id() . '/edit'; + $this->drupalGet($path); + $delete_path = '/admin/structure/menu/item/' . $this->link->id() . '/delete'; + $this->assertSession()->linkByHrefNotExists($delete_path); + $this->drupalGet($delete_path); + $this->assertSession()->statusCodeEquals(403); + + // Unrestricted link that has no content. + $path = '/admin/structure/menu/item/' . $this->emptyLink->id() . '/edit'; + $this->drupalGet($path); + $delete_path = '/admin/structure/menu/item/' . $this->emptyLink->id() . '/delete'; + $this->assertSession()->linkByHrefExists($delete_path); + $this->drupalGet($delete_path); + $this->assertSession()->statusCodeEquals(200); + + // Unrestricted link in a menu that has no content. + $path = '/admin/structure/menu/item/' . $this->emptyMenuLink->id() . '/edit'; + $this->drupalGet($path); + $delete_path = '/admin/structure/menu/item/' . $this->emptyMenuLink->id() . '/delete'; + $this->assertSession()->linkByHrefExists($delete_path); + $this->drupalGet($delete_path); + $this->assertSession()->statusCodeEquals(200); + + // Test for a delete menu page. + $menu_path = '/admin/structure/menu/manage/' . $this->menu->id() . '/delete'; + $this->drupalGet($menu_path); + $this->assertSession()->statusCodeEquals(403); + $delete_path = '/admin/structure/menu/manage/' . $this->emptyMenu->id() . '/delete'; + $this->drupalGet($delete_path); + $this->assertSession()->statusCodeEquals(200); + + // Test the overview page to make sure that the delete link is handled. + $menu_path = '/admin/structure/menu/manage/' . $this->menu->id(); + $this->drupalGet($menu_path); + $delete_path = '/admin/structure/menu/manage/' . $this->menu->id() . '/delete'; + $this->assertSession()->linkByHrefNotExists($delete_path); + + // Test links on this page. + $delete_path = '/admin/structure/menu/item/' . $this->link->id() . '/delete'; + $this->assertSession()->linkByHrefNotExists($delete_path); + $delete_path = '/admin/structure/menu/item/' . $this->emptyLink->id() . '/delete'; + $this->assertSession()->linkByHrefExists($delete_path); + + $menu_path = '/admin/structure/menu/manage/' . $this->emptyMenu->id(); + $this->drupalGet($menu_path); + $delete_path = '/admin/structure/menu/manage/' . $this->emptyMenu->id() . '/delete'; + $this->assertSession()->linkByHrefExists($delete_path); + + // Test links on this page. + $delete_path = '/admin/structure/menu/item/' . $this->emptyMenuLink->id() . '/delete'; + $this->assertSession()->linkByHrefExists($delete_path); + } + +} From 4419c77bb73e565c1c97081553ae5be02aa0a7c7 Mon Sep 17 00:00:00 2001 From: Ken Rickard Date: Thu, 19 Dec 2019 17:09:46 -0500 Subject: [PATCH 32/39] Handles cases where deleting a parent deletes its children, which we do not allow. --- .../src/Access/DeleteAccessCheck.php | 25 +++++++++++++++---- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/workbench_access_protect/src/Access/DeleteAccessCheck.php b/workbench_access_protect/src/Access/DeleteAccessCheck.php index 22652fba..4492eb3b 100644 --- a/workbench_access_protect/src/Access/DeleteAccessCheck.php +++ b/workbench_access_protect/src/Access/DeleteAccessCheck.php @@ -199,11 +199,26 @@ private function hasContent(EntityInterface $entity) { foreach ($this->getAllReferenceFields($entity) as $name => $fieldConfig) { // Get the entity reference and determine if it is access controlled. if ($fieldConfig instanceof FieldStorageConfig) { - $entities = \Drupal::entityQuery($fieldConfig->get('entity_type')) - ->condition($fieldConfig->get('field_name'), $entity->id()) - ->range(0, 1) - ->execute(); - if (count($entities) > 0) { + // Base query. + $entities = \Drupal::entityQuery($fieldConfig->get('entity_type')); + // Check all children, if required. + // @TODO: allow this to be a configurable setting. + $storage = \Drupal::entityTypeManager()->getStorage($entity->getEntityTypeId()); + if (method_exists($storage, 'loadChildren')) { + $children = $storage->loadChildren($entity->id()); + if ($children) { + $ids = array_keys($children); + $ids[] = $entity->id(); + $entities->condition($fieldConfig->get('field_name'), $ids, 'IN'); + $children_checked = TRUE; + } + } + // Else just query this entity. + if (empty($children_checked)) { + $entities->condition($fieldConfig->get('field_name'), $entity->id()); + } + $result = $entities->range(0, 1)->execute(); + if (count($result) > 0) { return TRUE; } } From 27fdb22034302cca32f85b6130d77fc5229a337f Mon Sep 17 00:00:00 2001 From: Ken Rickard Date: Thu, 19 Dec 2019 17:11:53 -0500 Subject: [PATCH 33/39] Adds todo for new test. --- workbench_access_protect/src/Access/DeleteAccessCheck.php | 1 + 1 file changed, 1 insertion(+) diff --git a/workbench_access_protect/src/Access/DeleteAccessCheck.php b/workbench_access_protect/src/Access/DeleteAccessCheck.php index 4492eb3b..60ae5a5a 100644 --- a/workbench_access_protect/src/Access/DeleteAccessCheck.php +++ b/workbench_access_protect/src/Access/DeleteAccessCheck.php @@ -203,6 +203,7 @@ private function hasContent(EntityInterface $entity) { $entities = \Drupal::entityQuery($fieldConfig->get('entity_type')); // Check all children, if required. // @TODO: allow this to be a configurable setting. + // @TODO: test coverage. $storage = \Drupal::entityTypeManager()->getStorage($entity->getEntityTypeId()); if (method_exists($storage, 'loadChildren')) { $children = $storage->loadChildren($entity->id()); From 8c9bf60ba49838b4668c4a3292089c1b7e92a5ca Mon Sep 17 00:00:00 2001 From: Ken Rickard Date: Thu, 19 Dec 2019 17:12:28 -0500 Subject: [PATCH 34/39] Adds menu field note. --- workbench_access_protect/src/Access/DeleteAccessCheck.php | 1 + 1 file changed, 1 insertion(+) diff --git a/workbench_access_protect/src/Access/DeleteAccessCheck.php b/workbench_access_protect/src/Access/DeleteAccessCheck.php index 60ae5a5a..e887d0eb 100644 --- a/workbench_access_protect/src/Access/DeleteAccessCheck.php +++ b/workbench_access_protect/src/Access/DeleteAccessCheck.php @@ -223,6 +223,7 @@ private function hasContent(EntityInterface $entity) { return TRUE; } } + // @TODO: check for menu field handling. } return FALSE; From 297281f670a4d5a23f04c463d01fe60a9f14a524 Mon Sep 17 00:00:00 2001 From: agentrickard Date: Tue, 10 Mar 2020 15:06:52 -0400 Subject: [PATCH 35/39] Ensure that we do not run checks for access when they are not required. --- .../src/Access/DeleteAccessCheck.php | 13 +++- .../workbench_access_protect.module | 76 +++++++++++++++---- 2 files changed, 72 insertions(+), 17 deletions(-) diff --git a/workbench_access_protect/src/Access/DeleteAccessCheck.php b/workbench_access_protect/src/Access/DeleteAccessCheck.php index e887d0eb..67bb6d26 100644 --- a/workbench_access_protect/src/Access/DeleteAccessCheck.php +++ b/workbench_access_protect/src/Access/DeleteAccessCheck.php @@ -93,12 +93,17 @@ public function access() { public function isDeleteAllowed(EntityInterface $entity) { $return = TRUE; - $assigned_members = $this->hasMembers($entity); - $assigned_content = $this->hasContent($entity); - // If this entity does not have users assigned to it for access control and // is not assigned to any pieces of content, it is OK to delete it. - if ($assigned_members || $assigned_content) { + $assigned_members = $this->hasMembers($entity); + if ($assigned_members) { + return FALSE; + } + + // Breaking up the IF statement is a performance gain, since most sites + // have more nodes than users. + $assigned_content = $this->hasContent($entity); + if ($assigned_content) { $return = FALSE; } diff --git a/workbench_access_protect/workbench_access_protect.module b/workbench_access_protect/workbench_access_protect.module index 791663fd..dd937c3b 100644 --- a/workbench_access_protect/workbench_access_protect.module +++ b/workbench_access_protect/workbench_access_protect.module @@ -16,29 +16,79 @@ use Drupal\workbench_access_protect\Access\Taxonomy; */ function workbench_access_protect_entity_access(EntityInterface $entity, $op, AccountInterface $account) { - if ($op == 'delete') { + if ($op === 'delete') { + + // First, check that this entity type is used as an access scheme. + $check = FALSE; + $list = []; + $scheme_storage = \Drupal::entityTypeManager()->getStorage('access_scheme'); + if ($schemes = $scheme_storage->loadMultiple()) { + /** @var \Drupal\workbench_access\Entity\AccessSchemeInterface $scheme */ + foreach ($schemes as $id => $scheme) { + $scheme_type = $scheme->get('scheme'); + $list = workbench_access_protect_list($scheme_type); + foreach ($list as $type) { + if ($type === $entity->getEntityTypeId()) { + $check = TRUE; + } + } + } + } + if (!$check) { + return AccessResult::neutral(); + } /** @var Drupal\workbench_access_protect\Access\DeleteAccessCheck $protect */ $protect = \Drupal::service('workbench_access_protect.delete'); // First, let's check to see if this is an entity we are tracking. - if ($protect->hasBundles($entity)) { - $bundle = $protect->getBundles($entity)[$entity->id()]; - // Check the entire bundle to see if any term is in use. - if ($protect->isDeleteAllowedBundle($bundle, $entity) === FALSE) { - return AccessResult::forbidden(); - } + if ($protect->hasBundles($entity)) { + $bundle = $protect->getBundles($entity)[$entity->id()]; + // Check the entire bundle to see if any term is in use. + if ($protect->isDeleteAllowedBundle($bundle, $entity) === FALSE) { + return AccessResult::forbidden(); } - else { - // We're at the leaf entity and can check the specific entity. - if ($protect->isAccessControlled($entity) && - $protect->isDeleteAllowed($entity) === FALSE) { - return AccessResult::forbidden(); - } + } + else { + // We're at the leaf entity and can check the specific entity. + if ($protect->isAccessControlled($entity) && + $protect->isDeleteAllowed($entity) === FALSE) { + return AccessResult::forbidden(); } + } } return AccessResult::neutral(); } + +/** + * Lists the entity types protected by each access scheme. + * + * @param $scheme_type + * The scheme type to lookup, leave NULL to return all. + * + * @return array + * An array representing the entity types registered to a scheme, or an + * array of types keyed by scheme. + */ +function workbench_access_protect_list($scheme_type = NULL) { + $types = [ + 'menu' => [ + 'menu', + 'menu_link_content', + ], + 'taxonomy' => [ + 'taxonomy_term', + 'vocabulary', + ], + ]; + // Enable other modules to register. + \Drupal::moduleHandler()->alter('workbench_access_protect_list', $types); + + if ($scheme_type) { + return $types[$scheme_type]; + } + return $types; +} From 7487f5c1f508a7d83b6a59681419c63c78ca116d Mon Sep 17 00:00:00 2001 From: agentrickard Date: Tue, 10 Mar 2020 15:13:24 -0400 Subject: [PATCH 36/39] Adds api example file. --- .../workbench_access_protect.api.php | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 workbench_access_protect/workbench_access_protect.api.php diff --git a/workbench_access_protect/workbench_access_protect.api.php b/workbench_access_protect/workbench_access_protect.api.php new file mode 100644 index 00000000..55f0704f --- /dev/null +++ b/workbench_access_protect/workbench_access_protect.api.php @@ -0,0 +1,23 @@ + ['node_type']; +} From 48a40b5c0fc1a05f4551daabba31bade287a464e Mon Sep 17 00:00:00 2001 From: agentrickard Date: Tue, 10 Mar 2020 17:27:52 -0400 Subject: [PATCH 37/39] When checking content fields, ensure that we are pulling from workbench access configuration, which knows where to look. --- .../src/Access/DeleteAccessCheck.php | 28 ++++++++++--------- .../workbench_access_protect.module | 2 +- 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/workbench_access_protect/src/Access/DeleteAccessCheck.php b/workbench_access_protect/src/Access/DeleteAccessCheck.php index 67bb6d26..6a0c9a75 100644 --- a/workbench_access_protect/src/Access/DeleteAccessCheck.php +++ b/workbench_access_protect/src/Access/DeleteAccessCheck.php @@ -237,25 +237,27 @@ private function hasContent(EntityInterface $entity) { private function getAllReferenceFields(EntityInterface $entity) { // First, we are going to try to retrieve a cached instance. $found_fields = \Drupal::cache()->get('workbench_access_protect'); - if ($found_fields === FALSE) { - $map = $this->fieldManager->getFieldMap(); $found_fields = []; - foreach ($map as $entity_type => $fields) { - foreach ($fields as $name => $field) { - if ($field['type'] === 'entity_reference') { - // Get the entity reference and determine if it is access controlled. - /** @var \Drupal\field\Entity\FieldStorageConfig $fieldConfig */ - $fieldConfig = FieldStorageConfig::loadByName($entity_type, $name); - if ($fieldConfig !== NULL && - $fieldConfig->getSetting('target_type') === $entity->getEntityType()->id()) { - $found_fields[$name] = $fieldConfig; - } + $scheme_storage = \Drupal::entityTypeManager() + ->getStorage('access_scheme'); + // Grab the fields configured for each access scheme and then check + // that they are within our list of protected entity types. + foreach ($scheme_storage->loadMultiple() as $scheme) { + $access_scheme = $scheme->getAccessScheme(); + $scheme_type = $scheme->get('scheme'); + $protect_list = workbench_access_protect_list($scheme_type); + if (in_array($entity->getEntityTypeId(), $protect_list, TRUE)) { + $configuration = $access_scheme->getConfiguration(); + foreach ($configuration['fields'] as $field_info) { + $fieldConfig = FieldStorageConfig::loadByName($field_info['entity_type'], $field_info['field']); + if ($fieldConfig) { + $found_fields[$field_info['field']] = $fieldConfig; } } } } - + } else { $found_fields = $found_fields->data; } diff --git a/workbench_access_protect/workbench_access_protect.module b/workbench_access_protect/workbench_access_protect.module index dd937c3b..b54b8fc5 100644 --- a/workbench_access_protect/workbench_access_protect.module +++ b/workbench_access_protect/workbench_access_protect.module @@ -81,7 +81,7 @@ function workbench_access_protect_list($scheme_type = NULL) { ], 'taxonomy' => [ 'taxonomy_term', - 'vocabulary', + 'taxonomy_vocabulary', ], ]; // Enable other modules to register. From df4ba0eeb9bed334c9f6cf5651eb1b940f2fef5e Mon Sep 17 00:00:00 2001 From: agentrickard Date: Wed, 11 Mar 2020 14:46:18 -0400 Subject: [PATCH 38/39] Optimize lookups for access checks. --- .../src/Access/DeleteAccessCheck.php | 6 +--- .../workbench_access_protect.module | 31 ++++++++++++------- 2 files changed, 20 insertions(+), 17 deletions(-) diff --git a/workbench_access_protect/src/Access/DeleteAccessCheck.php b/workbench_access_protect/src/Access/DeleteAccessCheck.php index 6a0c9a75..cd425c14 100644 --- a/workbench_access_protect/src/Access/DeleteAccessCheck.php +++ b/workbench_access_protect/src/Access/DeleteAccessCheck.php @@ -124,11 +124,7 @@ public function getBundles(EntityInterface $entity) { * @{inheritdoc} */ public function hasBundles(EntityInterface $entity) { - if ($this->entityTypeManager->getDefinitions()[$entity->getEntityTypeId()]->get('bundle_of') == null) { - return FALSE; - } - - return TRUE; + return !is_null($this->entityTypeManager->getDefinitions()[$entity->getEntityTypeId()]->get('bundle_of')); } /** diff --git a/workbench_access_protect/workbench_access_protect.module b/workbench_access_protect/workbench_access_protect.module index b54b8fc5..bc7aed2d 100644 --- a/workbench_access_protect/workbench_access_protect.module +++ b/workbench_access_protect/workbench_access_protect.module @@ -15,24 +15,31 @@ use Drupal\workbench_access_protect\Access\Taxonomy; * Implements hook_entity_access(). */ function workbench_access_protect_entity_access(EntityInterface $entity, $op, AccountInterface $account) { + static $checked = []; if ($op === 'delete') { - // First, check that this entity type is used as an access scheme. $check = FALSE; - $list = []; - $scheme_storage = \Drupal::entityTypeManager()->getStorage('access_scheme'); - if ($schemes = $scheme_storage->loadMultiple()) { - /** @var \Drupal\workbench_access\Entity\AccessSchemeInterface $scheme */ - foreach ($schemes as $id => $scheme) { - $scheme_type = $scheme->get('scheme'); - $list = workbench_access_protect_list($scheme_type); - foreach ($list as $type) { - if ($type === $entity->getEntityTypeId()) { - $check = TRUE; + if (isset($checked[$entity->getEntityTypeId()])) { + $check = $checked[$entity->getEntityTypeId()]; + } + else { + // Determine if this entity type is access controlled and cache result. + $entity_type_id = $entity->getEntityTypeId(); + $scheme_storage = \Drupal::entityTypeManager()->getStorage('access_scheme'); + if ($schemes = $scheme_storage->loadMultiple()) { + /** @var \Drupal\workbench_access\Entity\AccessSchemeInterface $scheme */ + foreach ($schemes as $id => $scheme) { + $scheme_type = $scheme->get('scheme'); + $list = workbench_access_protect_list($scheme_type); + foreach ($list as $type) { + if ($type === $entity_type_id) { + $check = TRUE; + } } } } + $checked[$entity_type_id] = $check; } if (!$check) { return AccessResult::neutral(); @@ -41,7 +48,7 @@ function workbench_access_protect_entity_access(EntityInterface $entity, $op, Ac /** @var Drupal\workbench_access_protect\Access\DeleteAccessCheck $protect */ $protect = \Drupal::service('workbench_access_protect.delete'); - // First, let's check to see if this is an entity we are tracking. + // Now, let's check to see if this is an entity bundle we are tracking. if ($protect->hasBundles($entity)) { $bundle = $protect->getBundles($entity)[$entity->id()]; // Check the entire bundle to see if any term is in use. From 5378a8b26b5a30d9723df122746cff9b9e11cb8f Mon Sep 17 00:00:00 2001 From: agentrickard Date: Mon, 16 Mar 2020 12:16:36 -0400 Subject: [PATCH 39/39] Fixes code style issues. --- .../src/Access/DeleteAccessCheck.php | 48 +++++++++++++------ .../src/Access/DeleteAccessCheckInterface.php | 15 +++--- .../tests/src/Functional/MenuProtectTest.php | 26 +++++----- .../src/Functional/TaxonomyProtectTest.php | 11 ++--- .../workbench_access_protect.api.php | 15 ++++-- .../workbench_access_protect.module | 10 ++-- .../workbench_access_protect.services.yml | 1 + 7 files changed, 74 insertions(+), 52 deletions(-) diff --git a/workbench_access_protect/src/Access/DeleteAccessCheck.php b/workbench_access_protect/src/Access/DeleteAccessCheck.php index cd425c14..3d345c18 100644 --- a/workbench_access_protect/src/Access/DeleteAccessCheck.php +++ b/workbench_access_protect/src/Access/DeleteAccessCheck.php @@ -10,7 +10,6 @@ use Drupal\Core\Routing\CurrentRouteMatch; use Drupal\workbench_access\UserSectionStorage; use Drupal\Core\Entity\EntityFieldManager; -use Drupal\Core\Messenger\MessengerInterface; use Drupal\workbench_access\Entity\AccessSchemeInterface; /** @@ -19,36 +18,50 @@ class DeleteAccessCheck implements DeleteAccessCheckInterface { /** + * User account. + * * @var \Drupal\Core\Session\AccountInterface */ private $account; /** - * @var \Drupal\Core\Routing\CurrentRouteMatch; + * Default object for current_route_match service. + * + * @var \Drupal\Core\Routing\CurrentRouteMatch */ private $route; /** + * A class for storing and retrieving sections assigned to a user. + * * @var \Drupal\workbench_access\UserSectionStorage */ private $userSectionStorage; /** + * Manages the discovery of entity fields. + * * @var \Drupal\Core\Entity\EntityFieldManager */ private $fieldManager; /** + * Manages entity type plugin definitions. + * * @var \Drupal\Core\Entity\EntityTypeManager */ private $entityTypeManager; /** + * An interface for an entity type bundle info. + * * @var \Drupal\Core\Entity\EntityTypeBundleInfoInterface */ private $entityTypeBundleInfo; - + /** + * Constructs a new DeleteAccessCheck. + */ public function __construct(CurrentRouteMatch $route, UserSectionStorage $userSectionStorage, EntityFieldManager $fieldManager, @@ -88,7 +101,7 @@ public function access() { } /** - * @{inheritdoc} + * {@inheritdoc} */ public function isDeleteAllowed(EntityInterface $entity) { $return = TRUE; @@ -111,17 +124,16 @@ public function isDeleteAllowed(EntityInterface $entity) { } /** - * @{inheritdoc} + * {@inheritdoc} */ public function getBundles(EntityInterface $entity) { $bundle = $this->entityTypeManager->getDefinitions()[$entity->getEntityTypeId()]->get('bundle_of'); $bundles = $this->entityTypeBundleInfo->getBundleInfo($bundle); - return array_combine(array_keys($bundles), array_keys($bundles)); } /** - * @{inheritdoc} + * {@inheritdoc} */ public function hasBundles(EntityInterface $entity) { return !is_null($this->entityTypeManager->getDefinitions()[$entity->getEntityTypeId()]->get('bundle_of')); @@ -130,11 +142,11 @@ public function hasBundles(EntityInterface $entity) { /** * {@inheritdoc} */ - public function isDeleteAllowedBundle($bundle, $entity){ + public function isDeleteAllowedBundle($bundle, $entity) { $bundle_of = $this->entityTypeManager->getDefinitions()[$entity->getEntityTypeId()]->get('bundle_of'); $entity_id_key = $this->entityTypeManager->getDefinitions()[$entity->getEntityTypeId()]->get('entity_keys')['id']; $entities = $this->entityTypeManager->getStorage($bundle_of)->loadByProperties( - [$entity_id_key => $bundle ] + [$entity_id_key => $bundle] ); /* @@ -171,7 +183,6 @@ private function hasMembers(EntityInterface $entity) { * * @return array * An array of the users assigned to this section. - * */ private function getActiveSections(EntityInterface $entity) { /** @var \Drupal\workbench_access\UserSectionStorageInterface $sectionStorage */ @@ -179,9 +190,9 @@ private function getActiveSections(EntityInterface $entity) { $editors = array_reduce($this->entityTypeManager->getStorage('access_scheme')->loadMultiple(), function (array $editors, AccessSchemeInterface $scheme) use ($sectionStorage, $entity) { - $editors += $sectionStorage->getEditors($scheme, $entity->id()); - return $editors; - }, []); + $editors += $sectionStorage->getEditors($scheme, $entity->id()); + return $editors; + }, []); return $editors; } @@ -230,6 +241,15 @@ private function hasContent(EntityInterface $entity) { return FALSE; } + /** + * Get all entities with referencing fields targeting the ID. + * + * @param \Drupal\Core\Entity\EntityInterface $entity + * The entity to inspect. + * + * @return array + * An array of entities with referencing fields targeting the given ID. + */ private function getAllReferenceFields(EntityInterface $entity) { // First, we are going to try to retrieve a cached instance. $found_fields = \Drupal::cache()->get('workbench_access_protect'); @@ -265,7 +285,7 @@ private function getAllReferenceFields(EntityInterface $entity) { } /** - * @{inheritdoc} + * {@inheritdoc} */ public function isAccessControlled(EntityInterface $entity) { $schemes = $this->entityTypeManager->getStorage('access_scheme')->loadMultiple(); diff --git a/workbench_access_protect/src/Access/DeleteAccessCheckInterface.php b/workbench_access_protect/src/Access/DeleteAccessCheckInterface.php index 95b8677e..71da3218 100644 --- a/workbench_access_protect/src/Access/DeleteAccessCheckInterface.php +++ b/workbench_access_protect/src/Access/DeleteAccessCheckInterface.php @@ -12,7 +12,7 @@ interface DeleteAccessCheckInterface { /** * Determines if this entity may be deleted. * - * @param EntityInterface $entity + * @param \Drupal\Core\Entity\EntityInterface $entity * The entity to check. * * @return bool @@ -28,7 +28,8 @@ public function isDeleteAllowed(EntityInterface $entity); * * Use this to determine if the entity has children bundles. * - * @param EntityInterface $entity + * @param \Drupal\Core\Entity\EntityInterface $entity + * The term to check for bundles. * * @return bool * TRUE if the entity has bundles, FALSE otherwise @@ -39,6 +40,7 @@ public function hasBundles(EntityInterface $entity); * Gets the assigned bundles associated with an entity. * * @param \Drupal\Core\Entity\EntityInterface $entity + * The entity to get bundles on. * * @return array * An array where the keys and values are bundle ids. @@ -49,13 +51,14 @@ public function getBundles(EntityInterface $entity); * Checks if a bundle's entities are being actively used for access control. * * @param string $bundle - * The bundle ID + * The bundle ID. * @param string $entity - * The entity id + * The entity id. * * @return bool + * TRUE */ - public function isDeleteAllowedBundle($bundle, $entity); + public function isDeleteAllowedBundle($bundle, $entity); /** * Determines if the entity is used for access control. @@ -69,6 +72,6 @@ public function isDeleteAllowedBundle($bundle, $entity); * @return bool * TRUE if used, FALSE otherwise */ - public function isAccessControlled(EntityInterface $entity); + public function isAccessControlled(EntityInterface $entity); } diff --git a/workbench_access_protect/tests/src/Functional/MenuProtectTest.php b/workbench_access_protect/tests/src/Functional/MenuProtectTest.php index 7f41f658..d978a0f4 100644 --- a/workbench_access_protect/tests/src/Functional/MenuProtectTest.php +++ b/workbench_access_protect/tests/src/Functional/MenuProtectTest.php @@ -6,8 +6,6 @@ use Drupal\Tests\workbench_access\Traits\WorkbenchAccessTestTrait; use Drupal\menu_link_content\Entity\MenuLinkContent; use Drupal\system\Entity\Menu; -use Drupal\workbench_access\WorkbenchAccessManagerInterface; -use Drupal\workbench_access\Entity\AccessSchemeInterface; /** * Tests protection of Menu used for access control. @@ -138,13 +136,13 @@ protected function setUp() { $this->admin = $this->setUpAdminUser([ 'administer workbench access', 'administer menu', - 'link to any page' + 'link to any page', ]); $this->editor = $this->setUpUserUniqueRole([ 'administer workbench access', 'assign workbench access', 'administer menu', - 'link to any page' + 'link to any page', ]); } @@ -231,16 +229,14 @@ private function assertTests() { // Login to the non-privileged account. $this->drupalLogin($this->editor); -// overview: /admin/structure/menu/manage/menu_test -// delete: /admin/structure/menu/manage/menu_test/delete -// overview: /admin/structure/menu/manage/empty_test -// delete: /admin/structure/menu/manage/empty_test/delete -// link: /admin/structure/menu/item/1/edit -// Link: /admin/structure/menu/item/1/delete - - + // overview: /admin/structure/menu/manage/menu_test + // delete: /admin/structure/menu/manage/menu_test/delete + // overview: /admin/structure/menu/manage/empty_test + // delete: /admin/structure/menu/manage/empty_test/delete + // link: /admin/structure/menu/item/1/edit + // Link: /admin/structure/menu/item/1/delete // Restricted link that has content. - $path = '/admin/structure/menu/item/' . $this->link->id() . '/edit'; + $path = '/admin/structure/menu/item/' . $this->link->id() . '/edit'; $this->drupalGet($path); $delete_path = '/admin/structure/menu/item/' . $this->link->id() . '/delete'; $this->assertSession()->linkByHrefNotExists($delete_path); @@ -248,7 +244,7 @@ private function assertTests() { $this->assertSession()->statusCodeEquals(403); // Unrestricted link that has no content. - $path = '/admin/structure/menu/item/' . $this->emptyLink->id() . '/edit'; + $path = '/admin/structure/menu/item/' . $this->emptyLink->id() . '/edit'; $this->drupalGet($path); $delete_path = '/admin/structure/menu/item/' . $this->emptyLink->id() . '/delete'; $this->assertSession()->linkByHrefExists($delete_path); @@ -256,7 +252,7 @@ private function assertTests() { $this->assertSession()->statusCodeEquals(200); // Unrestricted link in a menu that has no content. - $path = '/admin/structure/menu/item/' . $this->emptyMenuLink->id() . '/edit'; + $path = '/admin/structure/menu/item/' . $this->emptyMenuLink->id() . '/edit'; $this->drupalGet($path); $delete_path = '/admin/structure/menu/item/' . $this->emptyMenuLink->id() . '/delete'; $this->assertSession()->linkByHrefExists($delete_path); diff --git a/workbench_access_protect/tests/src/Functional/TaxonomyProtectTest.php b/workbench_access_protect/tests/src/Functional/TaxonomyProtectTest.php index 848e2ee9..9f88d63f 100644 --- a/workbench_access_protect/tests/src/Functional/TaxonomyProtectTest.php +++ b/workbench_access_protect/tests/src/Functional/TaxonomyProtectTest.php @@ -7,7 +7,6 @@ use Drupal\Tests\workbench_access\Traits\WorkbenchAccessTestTrait; use Drupal\taxonomy\Entity\Term; use Drupal\workbench_access\WorkbenchAccessManagerInterface; -use Drupal\workbench_access\Entity\AccessSchemeInterface; /** * Tests protection of Taxonomy used for access control. @@ -47,9 +46,9 @@ class TaxonomyProtectTest extends BrowserTestBase { protected $emptyVocabulary; /** - * A test term + * A test term. * - * @var \Drupal\taxonomy\Entity\Term; + * @var \Drupal\taxonomy\Entity\Term */ protected $term; @@ -192,7 +191,7 @@ private function assertTests() { $this->drupalLogin($this->editor); // Restricted term that has content. - $path = '/taxonomy/term/' . $this->term->id() . '/edit'; + $path = '/taxonomy/term/' . $this->term->id() . '/edit'; $this->drupalGet($path); $delete_path = '/taxonomy/term/' . $this->term->id() . '/delete'; $this->assertSession()->linkByHrefNotExists($delete_path); @@ -200,14 +199,14 @@ private function assertTests() { $this->assertSession()->statusCodeEquals(403); // Unrestricted term that has no content. - $path = '/taxonomy/term/' . $this->emptyTerm->id() . '/edit'; + $path = '/taxonomy/term/' . $this->emptyTerm->id() . '/edit'; $this->drupalGet($path); $delete_path = '/taxonomy/term/' . $this->emptyTerm->id() . '/delete'; $this->assertSession()->linkByHrefExists($delete_path); $this->drupalGet($delete_path); $this->assertSession()->statusCodeEquals(200); - $path = '/taxonomy/term/' . $this->emptyVocabTerm->id() . '/edit'; + $path = '/taxonomy/term/' . $this->emptyVocabTerm->id() . '/edit'; $this->drupalGet($path); $delete_path = '/taxonomy/term/' . $this->emptyVocabTerm->id() . '/delete'; $this->assertSession()->linkByHrefExists($delete_path); diff --git a/workbench_access_protect/workbench_access_protect.api.php b/workbench_access_protect/workbench_access_protect.api.php index 55f0704f..8e9ab908 100644 --- a/workbench_access_protect/workbench_access_protect.api.php +++ b/workbench_access_protect/workbench_access_protect.api.php @@ -1,5 +1,10 @@ ['node_type']; diff --git a/workbench_access_protect/workbench_access_protect.module b/workbench_access_protect/workbench_access_protect.module index bc7aed2d..6df4ecc4 100644 --- a/workbench_access_protect/workbench_access_protect.module +++ b/workbench_access_protect/workbench_access_protect.module @@ -8,8 +8,6 @@ use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Session\AccountInterface; use Drupal\Core\Access\AccessResult; -use Drupal\workbench_access_protect\Access\Taxonomy; - /** * Implements hook_entity_access(). @@ -73,12 +71,12 @@ function workbench_access_protect_entity_access(EntityInterface $entity, $op, Ac /** * Lists the entity types protected by each access scheme. * - * @param $scheme_type - * The scheme type to lookup, leave NULL to return all. + * @param string $scheme_type + * The scheme type to lookup, leave NULL to return all. * * @return array - * An array representing the entity types registered to a scheme, or an - * array of types keyed by scheme. + * An array representing the entity types registered to a scheme, or an + * array of types keyed by scheme. */ function workbench_access_protect_list($scheme_type = NULL) { $types = [ diff --git a/workbench_access_protect/workbench_access_protect.services.yml b/workbench_access_protect/workbench_access_protect.services.yml index 83dddd6d..9eaabc1a 100644 --- a/workbench_access_protect/workbench_access_protect.services.yml +++ b/workbench_access_protect/workbench_access_protect.services.yml @@ -2,3 +2,4 @@ services: workbench_access_protect.delete : class: Drupal\workbench_access_protect\Access\DeleteAccessCheck arguments: ['@current_route_match', '@workbench_access.user_section_storage', '@entity_field.manager', '@entity_type.manager', '@entity_type.bundle.info'] +