From 37705791f1bd6d9aae0e1449a520efe5125bf7e1 Mon Sep 17 00:00:00 2001 From: ekes Date: Tue, 26 Aug 2025 14:33:42 +0200 Subject: [PATCH 1/4] Entity Query access control to decide if users can reference content. https://github.com/localgovdrupal/localgov_services/issues/216#issuecomment-3223678278 Adds tests. I think they're failing, because I think they do get the content in the list. --- .../src/EntityChildRelationshipUi.php | 3 --- .../ServicesSelection.php | 13 ++---------- ...ntityReferenceServicesAutocompleteTest.php | 20 +++++++++++++++++- .../LandingPageChildrenTest.php | 21 +++++++++++++++++++ 4 files changed, 42 insertions(+), 15 deletions(-) diff --git a/modules/localgov_services_navigation/src/EntityChildRelationshipUi.php b/modules/localgov_services_navigation/src/EntityChildRelationshipUi.php index 5b46253..9520770 100644 --- a/modules/localgov_services_navigation/src/EntityChildRelationshipUi.php +++ b/modules/localgov_services_navigation/src/EntityChildRelationshipUi.php @@ -153,9 +153,6 @@ protected function childrenField(NodeInterface $node) { $children_query->condition('localgov_services_parent', $node->id()); // Exclude status which automatically get addded to page seperately. $children_query->condition('type', 'localgov_services_status', '<>'); - if (!$this->currentUser->hasPermission('bypass node access') && !$this->moduleHandler->hasImplementations('node_grants')) { - $children_query->condition('status', NodeInterface::PUBLISHED); - } $children = $children_query->accessCheck(TRUE)->execute(); $unreferenced_children = array_diff($children, self::referencedChildren($node)); diff --git a/modules/localgov_services_navigation/src/Plugin/EntityReferenceSelection/ServicesSelection.php b/modules/localgov_services_navigation/src/Plugin/EntityReferenceSelection/ServicesSelection.php index 2a7f073..3e0aa65 100644 --- a/modules/localgov_services_navigation/src/Plugin/EntityReferenceSelection/ServicesSelection.php +++ b/modules/localgov_services_navigation/src/Plugin/EntityReferenceSelection/ServicesSelection.php @@ -167,8 +167,7 @@ public function buildConfigurationForm(array $form, FormStateInterface $form_sta */ public function getReferenceableEntities($match = NULL, $match_operator = 'CONTAINS', $limit = 0) { $entities = []; - $query = $this->buildEntityQuery($match, $match_operator) - ->accessCheck(TRUE); + $query = $this->buildEntityQuery($match, $match_operator); if ($limit > 0) { $query->range(0, $limit); } @@ -219,6 +218,7 @@ protected function buildEntityQuery($match = NULL, $match_operator = 'CONTAINS') $configuration = $this->getConfiguration(); $entity_type = $this->entityTypeManager->getDefinition('node'); $query = $this->entityTypeManager->getStorage('node')->getQuery(); + $query->accessCheck(TRUE); $query->condition($entity_type->getKey('bundle'), $configuration['target_bundles'], 'IN'); if (isset($match)) { @@ -232,15 +232,6 @@ protected function buildEntityQuery($match = NULL, $match_operator = 'CONTAINS') } $query->sort('localgov_services_parent.entity:node.title', 'ASC'); $query->sort('title', 'ASC'); - $query->addTag('node_access'); - // Adding the 'node_access' tag is sadly insufficient for nodes: core - // requires us to also know about the concept of 'published' and - // 'unpublished'. We need to do that as long as there are no access control - // modules in use on the site. As long as one access control module is - // there, it is supposed to handle this check. - if (!$this->currentUser->hasPermission('bypass node access') && !$this->moduleHandler->hasImplementations('node_grants')) { - $query->condition('status', NodeInterface::PUBLISHED); - } $query->addTag('entity_reference'); $query->addMetaData('entity_reference_selection_handler', $this); return $query; diff --git a/modules/localgov_services_navigation/tests/src/FunctionalJavascript/EntityReferenceServicesAutocompleteTest.php b/modules/localgov_services_navigation/tests/src/FunctionalJavascript/EntityReferenceServicesAutocompleteTest.php index 4bcac1e..23f4d14 100644 --- a/modules/localgov_services_navigation/tests/src/FunctionalJavascript/EntityReferenceServicesAutocompleteTest.php +++ b/modules/localgov_services_navigation/tests/src/FunctionalJavascript/EntityReferenceServicesAutocompleteTest.php @@ -19,6 +19,13 @@ class EntityReferenceServicesAutocompleteTest extends WebDriverTestBase { use EntityReferenceFieldCreationTrait; use NodeCreationTrait; + /** + * A user with access to all content. + * + * @var \Drupal\user\UserInterface + */ + protected $admin; + /** * A user with mininum permissions for test. * @@ -53,6 +60,10 @@ class EntityReferenceServicesAutocompleteTest extends WebDriverTestBase { protected function setUp(): void { parent::setUp(); + $this->admin = $this->drupalCreateUser([ + 'bypass node access', + ]); + // Create a Content type and two test nodes. $this->createContentType(['type' => 'page']); $this->createNode([ @@ -66,6 +77,13 @@ protected function setUp(): void { 'localgov_services_parent' => ['target_id' => 1], 'status' => NodeInterface::PUBLISHED, ]); + $this->createNode([ + 'title' => 'Unpublished Page Sub', + 'type' => 'localgov_services_sublanding', + 'localgov_services_parent' => ['target_id' => 1], + 'status' => NodeInterface::NOT_PUBLISHED, + 'uid' => $this->admin->id(), + ]); $this->user = $this->drupalCreateUser([ 'access content', @@ -114,7 +132,7 @@ public function testServicesEntityReferenceAutocompleteWidget() { // Return the landing page, not another sublanding page. $assert_session->pageTextContains('Landing Page 1'); $assert_session->pageTextContains('Landing Page 1 » Page Sub 1'); - + $assert_session->pageTextNotContains('Landing Page 1 » Unpublished Page Sub'); } /** diff --git a/modules/localgov_services_navigation/tests/src/FunctionalJavascript/LandingPageChildrenTest.php b/modules/localgov_services_navigation/tests/src/FunctionalJavascript/LandingPageChildrenTest.php index 6795352..80d4f77 100644 --- a/modules/localgov_services_navigation/tests/src/FunctionalJavascript/LandingPageChildrenTest.php +++ b/modules/localgov_services_navigation/tests/src/FunctionalJavascript/LandingPageChildrenTest.php @@ -22,6 +22,13 @@ class LandingPageChildrenTest extends WebDriverTestBase { */ protected $defaultTheme = 'claro'; + /** + * A user to access all content. + * + * @var \Drupal\user\UserInterface + */ + protected $admin; + /** * A user to edit landing pages. * @@ -56,6 +63,9 @@ protected function setUp(): void { 'label' => $this->randomMachineName(), ]); $field_instance->save(); + $this->admin = $this->drupalCreateUser([ + 'bypass node access', + ]); $this->user = $this->drupalCreateUser([ 'access content', 'create page content', @@ -108,6 +118,15 @@ public function testServiceLandingPageLink() { ]); $child[3]->save(); + $child[3] = $this->createNode([ + 'title' => 'unpublished page', + 'type' => 'page', + 'localgov_services_parent' => ['target_id' => $landing->id()], + 'status' => NodeInterface::NOT_PUBLISHED, + 'uid' => $this->admin->id(), + ]); + $child[3]->save(); + $this->drupalGet($landing->toUrl('edit-form')->toString()); $page = $this->getSession()->getPage(); $assert_session = $this->assertSession(); @@ -149,6 +168,8 @@ public function testServiceLandingPageLink() { $assert_session->fieldValueEquals('edit-localgov-common-tasks-1-uri', '/foo'); $assert_session->fieldValueEquals('edit-localgov-common-tasks-1-title', '\'; #child_2\n'); + // The unpublished node cannot be seen. + $assert_session->elementNotExists('css', '#localgov-child-drag-' . $child[4]->id()); } /** From 284a3499dc4c35490638a088e0206176016b4d3f Mon Sep 17 00:00:00 2001 From: ekes Date: Tue, 26 Aug 2025 14:56:39 +0200 Subject: [PATCH 2/4] Unused use statement. --- .../src/Plugin/EntityReferenceSelection/ServicesSelection.php | 1 - 1 file changed, 1 deletion(-) diff --git a/modules/localgov_services_navigation/src/Plugin/EntityReferenceSelection/ServicesSelection.php b/modules/localgov_services_navigation/src/Plugin/EntityReferenceSelection/ServicesSelection.php index 3e0aa65..4f9d333 100644 --- a/modules/localgov_services_navigation/src/Plugin/EntityReferenceSelection/ServicesSelection.php +++ b/modules/localgov_services_navigation/src/Plugin/EntityReferenceSelection/ServicesSelection.php @@ -12,7 +12,6 @@ use Drupal\Core\Form\FormStateInterface; use Drupal\Core\Plugin\ContainerFactoryPluginInterface; use Drupal\Core\Session\AccountInterface; -use Drupal\node\NodeInterface; use Symfony\Component\DependencyInjection\ContainerInterface; /** From ef6e1db45ab856602aeebdb47918c5c6c57fbfb0 Mon Sep 17 00:00:00 2001 From: Andy Broomfield Date: Wed, 27 Aug 2025 20:39:58 +0100 Subject: [PATCH 3/4] Restore the previous node_access check with addition of view any unpublished permission In the entity reference service selector. Restore previous node_access checking, and restrict to published nodes if the checks do not pass. Additionally, check if the user has permission view any unpublished content and allow access to unpublished nodes if that is the case. --- .../EntityReferenceSelection/ServicesSelection.php | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/modules/localgov_services_navigation/src/Plugin/EntityReferenceSelection/ServicesSelection.php b/modules/localgov_services_navigation/src/Plugin/EntityReferenceSelection/ServicesSelection.php index 4f9d333..1325584 100644 --- a/modules/localgov_services_navigation/src/Plugin/EntityReferenceSelection/ServicesSelection.php +++ b/modules/localgov_services_navigation/src/Plugin/EntityReferenceSelection/ServicesSelection.php @@ -12,6 +12,7 @@ use Drupal\Core\Form\FormStateInterface; use Drupal\Core\Plugin\ContainerFactoryPluginInterface; use Drupal\Core\Session\AccountInterface; +use Drupal\node\NodeInterface; use Symfony\Component\DependencyInjection\ContainerInterface; /** @@ -233,6 +234,18 @@ protected function buildEntityQuery($match = NULL, $match_operator = 'CONTAINS') $query->sort('title', 'ASC'); $query->addTag('entity_reference'); $query->addMetaData('entity_reference_selection_handler', $this); + + // Adding the 'node_access' tag is sadly insufficient for nodes: core + // requires us to also know about the concept of 'published' and + // 'unpublished'. We need to do that as long as there are no access control + // modules in use on the site. As long as one access control module is + // there, it is supposed to handle this check. + // Additionally, if the user has the permission + // 'view any unpublished content' from Content moderation, + // don't restrict access. + if (!$this->currentUser->hasPermission('bypass node access') && !$this->currentUser->hasPermission('view any unpublished content') && !$this->moduleHandler->hasImplementations('node_grants')) { + $query->condition('status', NodeInterface::PUBLISHED); + } return $query; } From 7023a20e4a63a815bc996f04e30c7cac11423b90 Mon Sep 17 00:00:00 2001 From: Andy Broomfield Date: Thu, 28 Aug 2025 10:11:51 +0100 Subject: [PATCH 4/4] Apply view any unpublished content permission fix to EntityChildRelationshipsUi Apply same limit to the widget used to find referenced child pages. --- .../src/EntityChildRelationshipUi.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/modules/localgov_services_navigation/src/EntityChildRelationshipUi.php b/modules/localgov_services_navigation/src/EntityChildRelationshipUi.php index 9520770..0a45bd8 100644 --- a/modules/localgov_services_navigation/src/EntityChildRelationshipUi.php +++ b/modules/localgov_services_navigation/src/EntityChildRelationshipUi.php @@ -153,6 +153,12 @@ protected function childrenField(NodeInterface $node) { $children_query->condition('localgov_services_parent', $node->id()); // Exclude status which automatically get addded to page seperately. $children_query->condition('type', 'localgov_services_status', '<>'); + // Only allow access to published nodes if the user does not have the + // bypass node access, view any unpublished permission, or there are + // implementation of hook_node_grants which will add the node_access table. + if (!$this->currentUser->hasPermission('bypass node access') && !$this->currentUser->hasPermission('view any unpublished content') && !$this->moduleHandler->hasImplementations('node_grants')) { + $children_query->condition('status', NodeInterface::PUBLISHED); + } $children = $children_query->accessCheck(TRUE)->execute(); $unreferenced_children = array_diff($children, self::referencedChildren($node));