From 8e8e3d2ddd72471ba886346ecabfb5d98fd27d9b Mon Sep 17 00:00:00 2001 From: xjm Date: Tue, 14 Sep 2021 17:07:36 -0500 Subject: [PATCH] SA-CORE-2021-009 by illeace, Wim Leers, xjm, effulgentsia, larowlan, pandaski, vijaycs85, phenaproxima, mcdruid --- modules/quickedit/src/MetadataGenerator.php | 2 +- .../MockQuickEditEntityFieldAccessCheck.php | 15 +++- .../LayoutBuilderQuickEditTest.php | 28 +++++-- .../QuickEditImageTest.php | 74 ++++++++++++------- .../QuickEditIntegrationTest.php | 18 ----- .../src/Kernel/EditorIntegrationTest.php | 5 ++ .../src/Kernel/MetadataGeneratorTest.php | 15 ++++ 7 files changed, 106 insertions(+), 51 deletions(-) diff --git a/modules/quickedit/src/MetadataGenerator.php b/modules/quickedit/src/MetadataGenerator.php index f01256fcbbf..174726fe132 100644 --- a/modules/quickedit/src/MetadataGenerator.php +++ b/modules/quickedit/src/MetadataGenerator.php @@ -68,7 +68,7 @@ public function generateFieldMetadata(FieldItemListInterface $items, $view_mode) // Early-return if user does not have access. $access = $this->accessChecker->accessEditEntityField($entity, $field_name); - if (!$access) { + if (!$access->isAllowed()) { return ['access' => FALSE]; } diff --git a/modules/quickedit/tests/modules/src/MockQuickEditEntityFieldAccessCheck.php b/modules/quickedit/tests/modules/src/MockQuickEditEntityFieldAccessCheck.php index 2459c50f085..5e52e3c3045 100644 --- a/modules/quickedit/tests/modules/src/MockQuickEditEntityFieldAccessCheck.php +++ b/modules/quickedit/tests/modules/src/MockQuickEditEntityFieldAccessCheck.php @@ -2,6 +2,7 @@ namespace Drupal\quickedit_test; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Entity\EntityInterface; use Drupal\quickedit\Access\QuickEditEntityFieldAccessCheckInterface; @@ -14,7 +15,19 @@ class MockQuickEditEntityFieldAccessCheck implements QuickEditEntityFieldAccessC * {@inheritdoc} */ public function accessEditEntityField(EntityInterface $entity, $field_name) { - return TRUE; + switch (\Drupal::state()->get('quickedit_test_field_access')) { + case 'allowed': + return AccessResult::allowed(); + + case 'neutral': + return AccessResult::neutral(); + + case 'forbidden': + return AccessResult::forbidden(); + + default: + throw new \OutOfRangeException("The state for the 'quickedit_test_field_access' key must be either 'allowed', 'neutral' or 'forbidden'."); + } } } diff --git a/modules/quickedit/tests/src/FunctionalJavascript/LayoutBuilderQuickEditTest.php b/modules/quickedit/tests/src/FunctionalJavascript/LayoutBuilderQuickEditTest.php index 52fbb2fff39..3f498917136 100644 --- a/modules/quickedit/tests/src/FunctionalJavascript/LayoutBuilderQuickEditTest.php +++ b/modules/quickedit/tests/src/FunctionalJavascript/LayoutBuilderQuickEditTest.php @@ -112,6 +112,14 @@ public function testQuickEditIgnoresDuplicateFields() { $this->drupalLogin($this->contentAuthorUser); $this->usingLayoutBuilder = TRUE; + $this->assertQuickEditInit(['title']); + $this->drupalLogin($this->drupalCreateUser([ + 'access contextual links', + 'access in-place editing', + 'access content', + 'edit any article content', + 'administer nodes', + ])); $this->assertQuickEditInit(['title', 'uid', 'created']); } @@ -123,7 +131,7 @@ public function testQuickEditIgnoresDuplicateFields() { * * @dataProvider providerEnableDisableLayoutBuilder */ - public function testEnableDisableLayoutBuilder($use_revisions) { + public function testEnableDisableLayoutBuilder($use_revisions, $admin_permission = FALSE) { if (!$use_revisions) { $content_type = NodeType::load('article'); $content_type->setNewRevision(FALSE); @@ -131,10 +139,18 @@ public function testEnableDisableLayoutBuilder($use_revisions) { } $fields = [ 'title', - 'uid', - 'created', 'body', ]; + if ($admin_permission) { + $fields = array_merge($fields, ['uid', 'created']); + $this->drupalLogin($this->drupalCreateUser([ + 'access contextual links', + 'access in-place editing', + 'access content', + 'edit any article content', + 'administer nodes', + ])); + } // Test article with Layout Builder disabled. $this->assertQuickEditInit($fields); @@ -168,8 +184,10 @@ public function testEnableDisableLayoutBuilder($use_revisions) { */ public function providerEnableDisableLayoutBuilder() { return [ - 'use revisions' => [TRUE], - 'do not use revisions' => [FALSE], + 'use revisions, not admin' => [TRUE], + 'do not use revisions, not admin' => [FALSE], + 'use revisions, admin' => [TRUE, TRUE], + 'do not use revisions, admin' => [FALSE, TRUE], ]; } diff --git a/modules/quickedit/tests/src/FunctionalJavascript/QuickEditImageTest.php b/modules/quickedit/tests/src/FunctionalJavascript/QuickEditImageTest.php index f029848436b..dee43c54378 100644 --- a/modules/quickedit/tests/src/FunctionalJavascript/QuickEditImageTest.php +++ b/modules/quickedit/tests/src/FunctionalJavascript/QuickEditImageTest.php @@ -41,9 +41,19 @@ protected function setUp(): void { // Create the Article node type. $this->drupalCreateContentType(['type' => 'article', 'name' => 'Article']); + } + /** + * Tests that quick editor works correctly with images. + * + * @covers ::isCompatible + * @covers ::getAttachments + * + * @dataProvider providerTestImageInPlaceEditor + */ + public function testImageInPlaceEditor($admin_permission = FALSE) { // Log in as a content author who can use Quick Edit and edit Articles. - $this->contentAuthorUser = $this->drupalCreateUser([ + $permissions = [ 'access contextual links', 'access toolbar', 'access in-place editing', @@ -51,17 +61,12 @@ protected function setUp(): void { 'create article content', 'edit any article content', 'delete any article content', - ]); + ]; + if ($admin_permission) { + $permissions[] = 'administer nodes'; + } + $this->contentAuthorUser = $this->drupalCreateUser($permissions); $this->drupalLogin($this->contentAuthorUser); - } - - /** - * Tests that quick editor works correctly with images. - * - * @covers ::isCompatible - * @covers ::getAttachments - */ - public function testImageInPlaceEditor() { // Create a field with a basic filetype restriction. $field_name = strtolower($this->randomMachineName()); $field_settings = [ @@ -126,13 +131,25 @@ public function testImageInPlaceEditor() { $this->assertEntityInstanceStates([ 'node/1[0]' => 'closed', ]); + + $admin_inactive = []; + $admin_candidate = []; + if ($admin_permission) { + $admin_inactive = [ + 'node/1/uid/en/full' => 'inactive', + 'node/1/created/en/full' => 'inactive', + ]; + $admin_candidate = [ + 'node/1/uid/en/full' => 'candidate', + 'node/1/created/en/full' => 'candidate', + ]; + } + $this->assertEntityInstanceFieldStates('node', 1, 0, [ 'node/1/title/en/full' => 'inactive', - 'node/1/uid/en/full' => 'inactive', - 'node/1/created/en/full' => 'inactive', 'node/1/body/en/full' => 'inactive', 'node/1/' . $field_name . '/en/full' => 'inactive', - ]); + ] + $admin_inactive); // Start in-place editing of the article node. $this->startQuickEditViaToolbar('node', 1, 0); @@ -142,11 +159,9 @@ public function testImageInPlaceEditor() { $this->assertQuickEditEntityToolbar((string) $node->label(), NULL); $this->assertEntityInstanceFieldStates('node', 1, 0, [ 'node/1/title/en/full' => 'candidate', - 'node/1/uid/en/full' => 'candidate', - 'node/1/created/en/full' => 'candidate', 'node/1/body/en/full' => 'candidate', 'node/1/' . $field_name . '/en/full' => 'candidate', - ]); + ] + $admin_candidate); // Click the image field. $this->click($field_selector); @@ -154,21 +169,17 @@ public function testImageInPlaceEditor() { $this->assertSession()->elementExists('css', $field_selector . ' .quickedit-image-dropzone'); $this->assertEntityInstanceFieldStates('node', 1, 0, [ 'node/1/title/en/full' => 'candidate', - 'node/1/uid/en/full' => 'candidate', - 'node/1/created/en/full' => 'candidate', 'node/1/body/en/full' => 'candidate', 'node/1/' . $field_name . '/en/full' => 'active', - ]); + ] + $admin_candidate); // Type new 'alt' text. $this->typeInImageEditorAltTextInput('New text'); $this->assertEntityInstanceFieldStates('node', 1, 0, [ 'node/1/title/en/full' => 'candidate', - 'node/1/uid/en/full' => 'candidate', - 'node/1/created/en/full' => 'candidate', 'node/1/body/en/full' => 'candidate', 'node/1/' . $field_name . '/en/full' => 'changed', - ]); + ] + $admin_candidate); // Drag and drop an image. $this->dropImageOnImageEditor($valid_images[1]->uri); @@ -184,11 +195,9 @@ public function testImageInPlaceEditor() { ]); $this->assertEntityInstanceFieldStates('node', 1, 0, [ 'node/1/title/en/full' => 'candidate', - 'node/1/uid/en/full' => 'candidate', - 'node/1/created/en/full' => 'candidate', 'node/1/body/en/full' => 'candidate', 'node/1/' . $field_name . '/en/full' => 'saving', - ]); + ] + $admin_candidate); $this->assertEntityInstanceFieldMarkup([ 'node/1/' . $field_name . '/en/full' => '.quickedit-changed', ]); @@ -208,4 +217,17 @@ public function testImageInPlaceEditor() { $this->assertSession()->elementExists('css', $entity_selector . ' ' . $field_selector . ' ' . $new_image_selector); } + /** + * Data provider for ::testImageInPlaceEditor(). + * + * @return array + * Test cases. + */ + public function providerTestImageInPlaceEditor(): array { + return [ + 'with permission' => [TRUE], + 'without permission' => [FALSE], + ]; + } + } diff --git a/modules/quickedit/tests/src/FunctionalJavascript/QuickEditIntegrationTest.php b/modules/quickedit/tests/src/FunctionalJavascript/QuickEditIntegrationTest.php index 2328880a8d9..3669dfa9256 100644 --- a/modules/quickedit/tests/src/FunctionalJavascript/QuickEditIntegrationTest.php +++ b/modules/quickedit/tests/src/FunctionalJavascript/QuickEditIntegrationTest.php @@ -146,8 +146,6 @@ public function testArticleNode() { ]); $this->assertEntityInstanceFieldStates('node', 1, 0, [ 'node/1/title/en/full' => 'inactive', - 'node/1/uid/en/full' => 'inactive', - 'node/1/created/en/full' => 'inactive', 'node/1/body/en/full' => 'inactive', 'node/1/field_tags/en/full' => 'inactive', ]); @@ -160,8 +158,6 @@ public function testArticleNode() { $this->assertQuickEditEntityToolbar((string) $node->label(), NULL); $this->assertEntityInstanceFieldStates('node', 1, 0, [ 'node/1/title/en/full' => 'candidate', - 'node/1/uid/en/full' => 'candidate', - 'node/1/created/en/full' => 'candidate', 'node/1/body/en/full' => 'candidate', 'node/1/field_tags/en/full' => 'candidate', ]); @@ -174,8 +170,6 @@ public function testArticleNode() { $this->assertQuickEditEntityToolbar((string) $node->label(), 'Title'); $this->assertEntityInstanceFieldStates('node', 1, 0, [ 'node/1/title/en/full' => 'active', - 'node/1/uid/en/full' => 'candidate', - 'node/1/created/en/full' => 'candidate', 'node/1/body/en/full' => 'candidate', 'node/1/field_tags/en/full' => 'candidate', ]); @@ -188,8 +182,6 @@ public function testArticleNode() { $this->awaitEntityInstanceFieldState('node', 1, 0, 'title', 'en', 'changed'); $this->assertEntityInstanceFieldStates('node', 1, 0, [ 'node/1/title/en/full' => 'changed', - 'node/1/uid/en/full' => 'candidate', - 'node/1/created/en/full' => 'candidate', 'node/1/body/en/full' => 'candidate', 'node/1/field_tags/en/full' => 'candidate', ]); @@ -201,8 +193,6 @@ public function testArticleNode() { $this->assertQuickEditEntityToolbar((string) $node->label(), 'Body'); $this->assertEntityInstanceFieldStates('node', 1, 0, [ 'node/1/title/en/full' => 'saving', - 'node/1/uid/en/full' => 'candidate', - 'node/1/created/en/full' => 'candidate', 'node/1/body/en/full' => 'active', 'node/1/field_tags/en/full' => 'candidate', ]); @@ -223,8 +213,6 @@ public function testArticleNode() { $assert_session->waitForElement('css', '.quickedit-toolbar-field div[id*="tags"]'); $this->assertQuickEditEntityToolbar((string) $node->label(), 'Tags'); $this->assertEntityInstanceFieldStates('node', 1, 0, [ - 'node/1/uid/en/full' => 'candidate', - 'node/1/created/en/full' => 'candidate', 'node/1/body/en/full' => 'candidate', 'node/1/field_tags/en/full' => 'activating', 'node/1/title/en/full' => 'candidate', @@ -239,8 +227,6 @@ public function testArticleNode() { // Wait for the form to load. $this->assertJsCondition('document.querySelector(\'.quickedit-form-container > .quickedit-form[role="dialog"] > .placeholder\') === null'); $this->assertEntityInstanceFieldStates('node', 1, 0, [ - 'node/1/uid/en/full' => 'candidate', - 'node/1/created/en/full' => 'candidate', 'node/1/body/en/full' => 'candidate', 'node/1/field_tags/en/full' => 'active', 'node/1/title/en/full' => 'candidate', @@ -250,8 +236,6 @@ public function testArticleNode() { $this->typeInFormEditorTextInputField('field_tags[target_id]', 'foo, bar'); $this->awaitEntityInstanceFieldState('node', 1, 0, 'field_tags', 'en', 'changed'); $this->assertEntityInstanceFieldStates('node', 1, 0, [ - 'node/1/uid/en/full' => 'candidate', - 'node/1/created/en/full' => 'candidate', 'node/1/body/en/full' => 'candidate', 'node/1/field_tags/en/full' => 'changed', 'node/1/title/en/full' => 'candidate', @@ -264,8 +248,6 @@ public function testArticleNode() { 'node/1[0]' => 'committing', ]); $this->assertEntityInstanceFieldStates('node', 1, 0, [ - 'node/1/uid/en/full' => 'candidate', - 'node/1/created/en/full' => 'candidate', 'node/1/body/en/full' => 'candidate', 'node/1/field_tags/en/full' => 'saving', 'node/1/title/en/full' => 'candidate', diff --git a/modules/quickedit/tests/src/Kernel/EditorIntegrationTest.php b/modules/quickedit/tests/src/Kernel/EditorIntegrationTest.php index ed8d1a8389e..6852bafdb5e 100644 --- a/modules/quickedit/tests/src/Kernel/EditorIntegrationTest.php +++ b/modules/quickedit/tests/src/Kernel/EditorIntegrationTest.php @@ -179,6 +179,11 @@ public function testMetadata() { // Verify metadata. $items = $entity->get($this->fieldName); + \Drupal::state()->set('quickedit_test_field_access', 'forbidden'); + $this->assertSame(['access' => FALSE], $this->metadataGenerator->generateFieldMetadata($items, 'default')); + \Drupal::state()->set('quickedit_test_field_access', 'neutral'); + $this->assertSame(['access' => FALSE], $this->metadataGenerator->generateFieldMetadata($items, 'default')); + \Drupal::state()->set('quickedit_test_field_access', 'allowed'); $metadata = $this->metadataGenerator->generateFieldMetadata($items, 'default'); $expected = [ 'access' => TRUE, diff --git a/modules/quickedit/tests/src/Kernel/MetadataGeneratorTest.php b/modules/quickedit/tests/src/Kernel/MetadataGeneratorTest.php index 8aff88d7b94..880c9c1677a 100644 --- a/modules/quickedit/tests/src/Kernel/MetadataGeneratorTest.php +++ b/modules/quickedit/tests/src/Kernel/MetadataGeneratorTest.php @@ -97,6 +97,11 @@ public function testSimpleEntityType() { // Verify metadata for field 1. $items_1 = $entity->get($field_1_name); + \Drupal::state()->set('quickedit_test_field_access', 'forbidden'); + $this->assertSame(['access' => FALSE], $this->metadataGenerator->generateFieldMetadata($items_1, 'default')); + \Drupal::state()->set('quickedit_test_field_access', 'neutral'); + $this->assertSame(['access' => FALSE], $this->metadataGenerator->generateFieldMetadata($items_1, 'default')); + \Drupal::state()->set('quickedit_test_field_access', 'allowed'); $metadata_1 = $this->metadataGenerator->generateFieldMetadata($items_1, 'default'); $expected_1 = [ 'access' => TRUE, @@ -107,6 +112,11 @@ public function testSimpleEntityType() { // Verify metadata for field 2. $items_2 = $entity->get($field_2_name); + \Drupal::state()->set('quickedit_test_field_access', 'forbidden'); + $this->assertSame(['access' => FALSE], $this->metadataGenerator->generateFieldMetadata($items_2, 'default')); + \Drupal::state()->set('quickedit_test_field_access', 'neutral'); + $this->assertSame(['access' => FALSE], $this->metadataGenerator->generateFieldMetadata($items_2, 'default')); + \Drupal::state()->set('quickedit_test_field_access', 'allowed'); $metadata_2 = $this->metadataGenerator->generateFieldMetadata($items_2, 'default'); $expected_2 = [ 'access' => TRUE, @@ -163,6 +173,11 @@ public function testEditorWithCustomMetadata() { // Verify metadata. $items = $entity->get($field_name); + \Drupal::state()->set('quickedit_test_field_access', 'forbidden'); + $this->assertSame(['access' => FALSE], $this->metadataGenerator->generateFieldMetadata($items, 'default')); + \Drupal::state()->set('quickedit_test_field_access', 'neutral'); + $this->assertSame(['access' => FALSE], $this->metadataGenerator->generateFieldMetadata($items, 'default')); + \Drupal::state()->set('quickedit_test_field_access', 'allowed'); $metadata = $this->metadataGenerator->generateFieldMetadata($items, 'default'); $expected = [ 'access' => TRUE,