summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorxjm2017-08-16 12:10:35 -0500
committerxjm2017-08-16 12:41:01 -0500
commit15919734ffd7edbd003c9cbc5fd2a0c4b40ed08b (patch)
treed889d0d787a8db55bab2b052cef02a26a9a58377
parent418124b547d6fc33eefe4ad587f1d057eb5c63cc (diff)
SA-CORE-2017-004 by arshadcn, amateescu, Wim Leers, Berdir, Lendude, dawehner, klausi, pwolanin, xjm, mpdonadio, mlhess, larowlan, milesw
(cherry picked from commit 65cef2e9f5c37c7b0d1b70627779bd47d060e650)
-rw-r--r--core/lib/Drupal/Core/Entity/EntityAccessControlHandler.php16
-rw-r--r--core/modules/comment/src/Entity/Comment.php23
-rw-r--r--core/modules/comment/tests/src/Unit/Entity/CommentLockTest.php4
-rw-r--r--core/modules/rest/tests/src/Functional/EntityResource/Comment/CommentResourceTestBase.php33
-rw-r--r--core/modules/system/tests/modules/entity_test/entity_test.module10
-rw-r--r--core/modules/system/tests/modules/entity_test/src/Entity/EntityTestNoUuid.php29
-rw-r--r--core/modules/views/src/Controller/ViewAjaxController.php2
-rw-r--r--core/modules/views/src/Tests/ViewAjaxTest.php12
-rw-r--r--core/modules/views/tests/src/Unit/Controller/ViewAjaxControllerTest.php74
-rw-r--r--core/tests/Drupal/KernelTests/Core/Entity/EntityAccessControlHandlerTest.php70
10 files changed, 238 insertions, 35 deletions
diff --git a/core/lib/Drupal/Core/Entity/EntityAccessControlHandler.php b/core/lib/Drupal/Core/Entity/EntityAccessControlHandler.php
index 3b16d1c..ac36411 100644
--- a/core/lib/Drupal/Core/Entity/EntityAccessControlHandler.php
+++ b/core/lib/Drupal/Core/Entity/EntityAccessControlHandler.php
@@ -66,7 +66,19 @@ class EntityAccessControlHandler extends EntityHandlerBase implements EntityAcce
$operation = 'view';
}
- if (($return = $this->getCache($entity->uuid(), $operation, $langcode, $account)) !== NULL) {
+ // If an entity does not have a UUID, either from not being set or from not
+ // having them, use the 'entity type:ID' pattern as the cache $cid.
+ $cid = $entity->uuid() ?: $entity->getEntityTypeId() . ':' . $entity->id();
+
+ // If the entity is revisionable, then append the revision ID to allow
+ // individual revisions to have specific access control and be cached
+ // separately.
+ if ($entity instanceof RevisionableInterface) {
+ /** @var $entity \Drupal\Core\Entity\RevisionableInterface */
+ $cid .= ':' . $entity->getRevisionId();
+ }
+
+ if (($return = $this->getCache($cid, $operation, $langcode, $account)) !== NULL) {
// Cache hit, no work necessary.
return $return_as_object ? $return : $return->isAllowed();
}
@@ -92,7 +104,7 @@ class EntityAccessControlHandler extends EntityHandlerBase implements EntityAcce
if (!$return->isForbidden()) {
$return = $return->orIf($this->checkAccess($entity, $operation, $account));
}
- $result = $this->setCache($return, $entity->uuid(), $operation, $langcode, $account);
+ $result = $this->setCache($return, $cid, $operation, $langcode, $account);
return $return_as_object ? $result : $result->isAllowed();
}
diff --git a/core/modules/comment/src/Entity/Comment.php b/core/modules/comment/src/Entity/Comment.php
index 821b30d..c57e247 100644
--- a/core/modules/comment/src/Entity/Comment.php
+++ b/core/modules/comment/src/Entity/Comment.php
@@ -82,14 +82,6 @@ class Comment extends ContentEntityBase implements CommentInterface {
public function preSave(EntityStorageInterface $storage) {
parent::preSave($storage);
- if (is_null($this->get('status')->value)) {
- if (\Drupal::currentUser()->hasPermission('skip comment approval')) {
- $this->setPublished();
- }
- else {
- $this->setUnpublished();
- }
- }
if ($this->isNew()) {
// Add the comment to database. This next section builds the thread field.
// @see \Drupal\comment\CommentViewBuilder::buildComponents()
@@ -238,6 +230,9 @@ class Comment extends ContentEntityBase implements CommentInterface {
$fields['langcode']->setDescription(t('The comment language code.'));
+ // Set the default value callback for the status field.
+ $fields['status']->setDefaultValueCallback('Drupal\comment\Entity\Comment::getDefaultStatus');
+
$fields['pid'] = BaseFieldDefinition::create('entity_reference')
->setLabel(t('Parent ID'))
->setDescription(t('The parent comment ID if this is a reply to a comment.'))
@@ -559,4 +554,16 @@ class Comment extends ContentEntityBase implements CommentInterface {
return $this->bundle();
}
+ /**
+ * Default value callback for 'status' base field definition.
+ *
+ * @see ::baseFieldDefinitions()
+ *
+ * @return bool
+ * TRUE if the comment should be published, FALSE otherwise.
+ */
+ public static function getDefaultStatus() {
+ return \Drupal::currentUser()->hasPermission('skip comment approval') ? CommentInterface::PUBLISHED : CommentInterface::NOT_PUBLISHED;
+ }
+
}
diff --git a/core/modules/comment/tests/src/Unit/Entity/CommentLockTest.php b/core/modules/comment/tests/src/Unit/Entity/CommentLockTest.php
index e724dc6..9dbe6d3 100644
--- a/core/modules/comment/tests/src/Unit/Entity/CommentLockTest.php
+++ b/core/modules/comment/tests/src/Unit/Entity/CommentLockTest.php
@@ -81,10 +81,6 @@ class CommentLockTest extends UnitTestCase {
$comment->expects($this->any())
->method('getEntityType')
->will($this->returnValue($entity_type));
- $comment->expects($this->at(1))
- ->method('get')
- ->with('status')
- ->will($this->returnValue((object) ['value' => NULL]));
$storage = $this->getMock('Drupal\comment\CommentStorageInterface');
// preSave() should acquire the lock. (This is what's really being tested.)
diff --git a/core/modules/rest/tests/src/Functional/EntityResource/Comment/CommentResourceTestBase.php b/core/modules/rest/tests/src/Functional/EntityResource/Comment/CommentResourceTestBase.php
index ea705de..5b51014 100644
--- a/core/modules/rest/tests/src/Functional/EntityResource/Comment/CommentResourceTestBase.php
+++ b/core/modules/rest/tests/src/Functional/EntityResource/Comment/CommentResourceTestBase.php
@@ -324,4 +324,37 @@ abstract class CommentResourceTestBase extends EntityResourceTestBase {
}
}
+ /**
+ * Tests POSTing a comment with and without 'skip comment approval'
+ */
+ public function testPostSkipCommentApproval() {
+ $this->initAuthentication();
+ $this->provisionEntityResource();
+ $this->setUpAuthorization('POST');
+
+ // Create request.
+ $request_options = [];
+ $request_options[RequestOptions::HEADERS]['Accept'] = static::$mimeType;
+ $request_options[RequestOptions::HEADERS]['Content-Type'] = static::$mimeType;
+ $request_options = array_merge_recursive($request_options, $this->getAuthenticationRequestOptions('POST'));
+ $request_options[RequestOptions::BODY] = $this->serializer->encode($this->getNormalizedPostEntity(), static::$format);
+
+ $url = $this->getEntityResourcePostUrl()->setOption('query', ['_format' => static::$format]);
+
+ // Status should be FALSE when posting as anonymous.
+ $response = $this->request('POST', $url, $request_options);
+ $unserialized = $this->serializer->deserialize((string) $response->getBody(), get_class($this->entity), static::$format);
+ $this->assertResourceResponse(201, FALSE, $response);
+ $this->assertFalse($unserialized->getStatus());
+
+ // Grant anonymous permission to skip comment approval.
+ $this->grantPermissionsToTestedRole(['skip comment approval']);
+
+ // Status should be TRUE when posting as anonymous and skip comment approval.
+ $response = $this->request('POST', $url, $request_options);
+ $unserialized = $this->serializer->deserialize((string) $response->getBody(), get_class($this->entity), static::$format);
+ $this->assertResourceResponse(201, FALSE, $response);
+ $this->assertTrue($unserialized->getStatus());
+ }
+
}
diff --git a/core/modules/system/tests/modules/entity_test/entity_test.module b/core/modules/system/tests/modules/entity_test/entity_test.module
index 4d35275..bcf55f4 100644
--- a/core/modules/system/tests/modules/entity_test/entity_test.module
+++ b/core/modules/system/tests/modules/entity_test/entity_test.module
@@ -737,6 +737,16 @@ function entity_test_entity_access(EntityInterface $entity, $operation, AccountI
return AccessResult::allowed();
}
+ // Create specific labels to allow or deny access based on certain test
+ // conditions.
+ // @see \Drupal\KernelTests\Core\Entity\EntityAccessControlHandlerTest
+ if ($entity->label() == 'Accessible') {
+ return AccessResult::allowed();
+ }
+ if ($entity->label() == 'Inaccessible') {
+ return AccessResult::forbidden();
+ }
+
// Uncacheable because the access result depends on a State key-value pair and
// might therefore change at any time.
$condition = \Drupal::state()->get("entity_test_entity_access.{$operation}." . $entity->id(), FALSE);
diff --git a/core/modules/system/tests/modules/entity_test/src/Entity/EntityTestNoUuid.php b/core/modules/system/tests/modules/entity_test/src/Entity/EntityTestNoUuid.php
new file mode 100644
index 0000000..8d0ddac
--- /dev/null
+++ b/core/modules/system/tests/modules/entity_test/src/Entity/EntityTestNoUuid.php
@@ -0,0 +1,29 @@
+<?php
+
+namespace Drupal\entity_test\Entity;
+
+/**
+ * Test entity class with revisions but without UUIDs.
+ *
+ * @ContentEntityType(
+ * id = "entity_test_no_uuid",
+ * label = @Translation("Test entity without UUID"),
+ * handlers = {
+ * "access" = "Drupal\entity_test\EntityTestAccessControlHandler",
+ * },
+ * base_table = "entity_test_no_uuid",
+ * revision_table = "entity_test_no_uuid_revision",
+ * admin_permission = "administer entity_test content",
+ * persistent_cache = FALSE,
+ * entity_keys = {
+ * "id" = "id",
+ * "revision" = "vid",
+ * "bundle" = "type",
+ * "label" = "name",
+ * "langcode" = "langcode",
+ * },
+ * )
+ */
+class EntityTestNoUuid extends EntityTest {
+
+}
diff --git a/core/modules/views/src/Controller/ViewAjaxController.php b/core/modules/views/src/Controller/ViewAjaxController.php
index c40cf85..d204766 100644
--- a/core/modules/views/src/Controller/ViewAjaxController.php
+++ b/core/modules/views/src/Controller/ViewAjaxController.php
@@ -142,7 +142,7 @@ class ViewAjaxController implements ContainerInjectionInterface {
throw new NotFoundHttpException();
}
$view = $this->executableFactory->get($entity);
- if ($view && $view->access($display_id)) {
+ if ($view && $view->access($display_id) && $view->setDisplay($display_id) && $view->display_handler->getOption('use_ajax')) {
$response->setView($view);
// Fix the current path for paging.
if (!empty($path)) {
diff --git a/core/modules/views/src/Tests/ViewAjaxTest.php b/core/modules/views/src/Tests/ViewAjaxTest.php
index b8b2730..5a966a8 100644
--- a/core/modules/views/src/Tests/ViewAjaxTest.php
+++ b/core/modules/views/src/Tests/ViewAjaxTest.php
@@ -17,7 +17,7 @@ class ViewAjaxTest extends ViewTestBase {
*
* @var array
*/
- public static $testViews = ['test_ajax_view'];
+ public static $testViews = ['test_ajax_view', 'test_view'];
protected function setUp() {
parent::setUp();
@@ -61,4 +61,14 @@ class ViewAjaxTest extends ViewTestBase {
$this->assertEqual(count($result), 2, 'Ensure that two items are rendered in the HTML.');
}
+ /**
+ * Ensures that non-ajax view cannot be accessed via an ajax HTTP request.
+ */
+ public function testNonAjaxViewViaAjax() {
+ $this->drupalPost('views/ajax', '', ['view_name' => 'test_ajax_view', 'view_display_id' => 'default'], ['query' => [MainContentViewSubscriber::WRAPPER_FORMAT => 'drupal_ajax']]);
+ $this->assertResponse(200);
+ $this->drupalPost('views/ajax', '', ['view_name' => 'test_view', 'view_display_id' => 'default'], ['query' => [MainContentViewSubscriber::WRAPPER_FORMAT => 'drupal_ajax']]);
+ $this->assertResponse(403);
+ }
+
}
diff --git a/core/modules/views/tests/src/Unit/Controller/ViewAjaxControllerTest.php b/core/modules/views/tests/src/Unit/Controller/ViewAjaxControllerTest.php
index f766975..a02f098 100644
--- a/core/modules/views/tests/src/Unit/Controller/ViewAjaxControllerTest.php
+++ b/core/modules/views/tests/src/Unit/Controller/ViewAjaxControllerTest.php
@@ -18,6 +18,9 @@ use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
*/
class ViewAjaxControllerTest extends UnitTestCase {
+ const USE_AJAX = TRUE;
+ const USE_NO_AJAX = FALSE;
+
/**
* The mocked view entity storage.
*
@@ -186,23 +189,6 @@ class ViewAjaxControllerTest extends UnitTestCase {
list($view, $executable) = $this->setupValidMocks();
- $display_handler = $this->getMockBuilder('Drupal\views\Plugin\views\display\DisplayPluginBase')
- ->disableOriginalConstructor()
- ->getMock();
- // Ensure that the pager element is not set.
- $display_handler->expects($this->never())
- ->method('setOption');
-
- $display_collection = $this->getMockBuilder('Drupal\views\DisplayPluginCollection')
- ->disableOriginalConstructor()
- ->getMock();
- $display_collection->expects($this->any())
- ->method('get')
- ->with('page_1')
- ->will($this->returnValue($display_handler));
-
- $executable->displayHandlers = $display_collection;
-
$this->redirectDestination->expects($this->atLeastOnce())
->method('set')
->with('/test-page?type=article');
@@ -216,6 +202,24 @@ class ViewAjaxControllerTest extends UnitTestCase {
}
/**
+ * Tests a valid view without ajax enabled.
+ */
+ public function testAjaxViewWithoutAjax() {
+ $request = new Request();
+ $request->request->set('view_name', 'test_view');
+ $request->request->set('view_display_id', 'page_1');
+ $request->request->set('view_path', '/test-page');
+ $request->request->set('_wrapper_format', 'ajax');
+ $request->request->set('ajax_page_state', 'drupal.settings[]');
+ $request->request->set('type', 'article');
+
+ $this->setupValidMocks(static::USE_NO_AJAX);
+
+ $this->setExpectedException(AccessDeniedHttpException::class);
+ $this->viewAjaxController->ajaxView($request);
+ }
+
+ /**
* Tests a valid view with arguments.
*/
public function testAjaxViewWithArguments() {
@@ -297,8 +301,15 @@ class ViewAjaxControllerTest extends UnitTestCase {
/**
* Sets up a bunch of valid mocks like the view entity and executable.
+ *
+ * @param bool $use_ajax
+ * Whether the 'use_ajax' option is set on the view display. Defaults to
+ * using ajax (TRUE).
+ *
+ * @return array
+ * A pair of view storage entity and executable.
*/
- protected function setupValidMocks() {
+ protected function setupValidMocks($use_ajax = self::USE_AJAX) {
$view = $this->getMockBuilder('Drupal\views\Entity\View')
->disableOriginalConstructor()
->getMock();
@@ -314,7 +325,10 @@ class ViewAjaxControllerTest extends UnitTestCase {
$executable->expects($this->once())
->method('access')
->will($this->returnValue(TRUE));
- $executable->expects($this->once())
+ $executable->expects($this->any())
+ ->method('setDisplay')
+ ->willReturn(TRUE);
+ $executable->expects($this->atMost(1))
->method('preview')
->will($this->returnValue(['#markup' => 'View result']));
@@ -323,6 +337,28 @@ class ViewAjaxControllerTest extends UnitTestCase {
->with($view)
->will($this->returnValue($executable));
+ $display_handler = $this->getMockBuilder('Drupal\views\Plugin\views\display\DisplayPluginBase')
+ ->disableOriginalConstructor()
+ ->getMock();
+ // Ensure that the pager element is not set.
+ $display_handler->expects($this->never())
+ ->method('setOption');
+ $display_handler->expects($this->any())
+ ->method('getOption')
+ ->with('use_ajax')
+ ->willReturn($use_ajax);
+
+ $display_collection = $this->getMockBuilder('Drupal\views\DisplayPluginCollection')
+ ->disableOriginalConstructor()
+ ->getMock();
+ $display_collection->expects($this->any())
+ ->method('get')
+ ->with('page_1')
+ ->will($this->returnValue($display_handler));
+
+ $executable->display_handler = $display_handler;
+ $executable->displayHandlers = $display_collection;
+
return [$view, $executable];
}
diff --git a/core/tests/Drupal/KernelTests/Core/Entity/EntityAccessControlHandlerTest.php b/core/tests/Drupal/KernelTests/Core/Entity/EntityAccessControlHandlerTest.php
index d7644b2..b3af27f 100644
--- a/core/tests/Drupal/KernelTests/Core/Entity/EntityAccessControlHandlerTest.php
+++ b/core/tests/Drupal/KernelTests/Core/Entity/EntityAccessControlHandlerTest.php
@@ -9,7 +9,9 @@ use Drupal\Core\Entity\EntityAccessControlHandler;
use Drupal\Core\Session\AnonymousUserSession;
use Drupal\entity_test\Entity\EntityTest;
use Drupal\entity_test\Entity\EntityTestDefaultAccess;
+use Drupal\entity_test\Entity\EntityTestNoUuid;
use Drupal\entity_test\Entity\EntityTestLabel;
+use Drupal\entity_test\Entity\EntityTestRev;
use Drupal\language\Entity\ConfigurableLanguage;
use Drupal\user\Entity\User;
@@ -21,6 +23,16 @@ use Drupal\user\Entity\User;
class EntityAccessControlHandlerTest extends EntityLanguageTestBase {
/**
+ * {@inheritdoc}
+ */
+ public function setUp() {
+ parent::setUp();
+
+ $this->installEntitySchema('entity_test_no_uuid');
+ $this->installEntitySchema('entity_test_rev');
+ }
+
+ /**
* Asserts entity access correctly grants or denies access.
*/
public function assertEntityAccess($ops, AccessibleInterface $object, AccountInterface $account = NULL) {
@@ -200,6 +212,64 @@ class EntityAccessControlHandlerTest extends EntityLanguageTestBase {
}
/**
+ * Ensures the static access cache works correctly in the absence of an UUID.
+ *
+ * @see entity_test_entity_access()
+ */
+ public function testEntityWithoutUuidAccessCache() {
+ $account = $this->createUser();
+
+ $entity1 = EntityTestNoUuid::create([
+ 'name' => 'Accessible',
+ ]);
+ $entity1->save();
+
+ $entity2 = EntityTestNoUuid::create([
+ 'name' => 'Inaccessible',
+ ]);
+ $entity2->save();
+
+ $this->assertTrue($entity1->access('delete', $account), 'Entity 1 can be deleted.');
+ $this->assertFalse($entity2->access('delete', $account), 'Entity 2 CANNOT be deleted.');
+
+ $entity1
+ ->setName('Inaccessible')
+ ->setNewRevision();
+ $entity1->save();
+
+ $this->assertFalse($entity1->access('delete', $account), 'Entity 1 revision 2 CANNOT be deleted.');
+ }
+
+ /**
+ * Ensures the static access cache works correctly with a UUID and revisions.
+ *
+ * @see entity_test_entity_access()
+ */
+ public function testEntityWithUuidAccessCache() {
+ $account = $this->createUser();
+
+ $entity1 = EntityTestRev::create([
+ 'name' => 'Accessible',
+ ]);
+ $entity1->save();
+
+ $entity2 = EntityTestRev::create([
+ 'name' => 'Inaccessible',
+ ]);
+ $entity2->save();
+
+ $this->assertTrue($entity1->access('delete', $account), 'Entity 1 can be deleted.');
+ $this->assertFalse($entity2->access('delete', $account), 'Entity 2 CANNOT be deleted.');
+
+ $entity1
+ ->setName('Inaccessible')
+ ->setNewRevision();
+ $entity1->save();
+
+ $this->assertFalse($entity1->access('delete', $account), 'Entity 1 revision 2 CANNOT be deleted.');
+ }
+
+ /**
* Tests hook invocations.
*/
public function testHooks() {