Issue #2889855 by seanB, chr.fritsch, vijaycs85, phenaproxima, marcoscano, Wim Leers, govind.maloo, larowlan, pk188, Dinesh18, balazswmann, michael_wojcik, catch, effulgentsia, alexpott, Berdir, dawehner: Unpublished media entity can not be accessed by owner and update any media/delete any media access possibly cached by user

merge-requests/1119/head
effulgentsia 2019-03-26 09:18:11 -07:00
parent e4188e4dec
commit 9cd5ccca8e
6 changed files with 777 additions and 10 deletions

View File

@ -305,7 +305,7 @@ class MediaTest extends ResourceTestBase {
protected function getExpectedUnauthorizedAccessMessage($method) {
switch ($method) {
case 'GET';
return "The 'view media' permission is required and the media item must be published.";
return "The 'view media' permission is required when the media item is published.";
case 'POST':
return "The following permissions are required: 'administer media' OR 'create media' OR 'create camelids media'.";

View File

@ -35,3 +35,6 @@ access media overview:
permission_callbacks:
- \Drupal\media\MediaPermissions::mediaTypePermissions
view own unpublished media:
title: 'View own unpublished media'

View File

@ -24,11 +24,28 @@ class MediaAccessControlHandler extends EntityAccessControlHandler {
$is_owner = ($account->id() && $account->id() === $entity->getOwnerId());
switch ($operation) {
case 'view':
$access_result = AccessResult::allowedIf($account->hasPermission('view media') && $entity->isPublished())
->cachePerPermissions()
->addCacheableDependency($entity);
if (!$access_result->isAllowed()) {
$access_result->setReason("The 'view media' permission is required and the media item must be published.");
if ($entity->isPublished()) {
$access_result = AccessResult::allowedIf($account->hasPermission('view media'))
->cachePerPermissions()
->addCacheableDependency($entity);
if (!$access_result->isAllowed()) {
$access_result->setReason("The 'view media' permission is required when the media item is published.");
}
}
elseif ($account->hasPermission('view own unpublished media')) {
$access_result = AccessResult::allowedIf($is_owner)
->cachePerPermissions()
->cachePerUser()
->addCacheableDependency($entity);
if (!$access_result->isAllowed()) {
$access_result->setReason("The user must be the owner and the 'view own unpublished media' permission is required when the media item is unpublished.");
}
}
else {
$access_result = AccessResult::neutral()
->cachePerPermissions()
->addCacheableDependency($entity)
->setReason("The user must be the owner and the 'view own unpublished media' permission is required when the media item is unpublished.");
}
return $access_result;

View File

@ -2,6 +2,8 @@
namespace Drupal\Tests\media\Functional;
use Drupal\field\Entity\FieldConfig;
use Drupal\field\Entity\FieldStorageConfig;
use Drupal\media\Entity\Media;
use Drupal\Tests\system\Functional\Cache\AssertPageCacheContextsAndTagsTrait;
use Drupal\user\Entity\Role;
@ -38,9 +40,15 @@ class MediaAccessTest extends MediaFunctionalTestBase {
*/
public function testMediaAccess() {
$assert_session = $this->assertSession();
$media_type = $this->createMediaType('test');
\Drupal::configFactory()
->getEditable('media.settings')
->set('standalone_url', TRUE)
->save(TRUE);
$this->container->get('router.builder')->rebuild();
// Create media.
$media = Media::create([
'bundle' => $media_type->id(),
@ -60,7 +68,7 @@ class MediaAccessTest extends MediaFunctionalTestBase {
$assert_session->statusCodeEquals(200);
$this->drupalGet('media/' . $user_media->id());
$this->assertCacheContext('user.permissions');
$assert_session->statusCodeEquals(404);
$assert_session->statusCodeEquals(200);
$this->drupalGet('media/' . $user_media->id() . '/edit');
$this->assertCacheContext('user.permissions');
$assert_session->statusCodeEquals(200);
@ -86,6 +94,24 @@ class MediaAccessTest extends MediaFunctionalTestBase {
user_role_revoke_permissions($role->id(), $permissions);
$role = Role::load(RoleInterface::AUTHENTICATED_ID);
// Verify the author can not view the unpublished media item without
// 'view own unpublished media' permission.
$this->grantPermissions($role, ['view media']);
$this->drupalGet('media/' . $user_media->id());
$this->assertNoCacheContext('user');
$this->assertCacheContext('user.permissions');
$assert_session->statusCodeEquals(200);
$user_media->setUnpublished()->save();
$this->drupalGet('media/' . $user_media->id());
$this->assertCacheContext('user.permissions');
$assert_session->statusCodeEquals(403);
$access_result = $user_media->access('view', NULL, TRUE);
$this->assertSame("The user must be the owner and the 'view own unpublished media' permission is required when the media item is unpublished.", $access_result->getReason());
$this->grantPermissions($role, ['view own unpublished media']);
$this->drupalGet('media/' . $user_media->id());
$this->assertCacheContext('user');
$assert_session->statusCodeEquals(200);
// Test 'create media' permission.
$this->drupalGet('media/add/' . $media_type->id());
$this->assertCacheContext('user.permissions');
@ -198,11 +224,180 @@ class MediaAccessTest extends MediaFunctionalTestBase {
$this->assertCacheContext('user.permissions');
$assert_session->statusCodeEquals(403);
$access_result = $media->access('view', NULL, TRUE);
$this->assertSame("The 'view media' permission is required and the media item must be published.", $access_result->getReason());
$this->assertSame("The 'view media' permission is required when the media item is published.", $access_result->getReason());
$this->grantPermissions($role, ['view media']);
$this->drupalGet('media/' . $media->id());
$this->assertCacheContext('user.permissions');
$assert_session->statusCodeEquals(200);
}
/**
* Tests unpublished media access.
*/
public function testUnpublishedMediaUserAccess() {
\Drupal::configFactory()
->getEditable('media.settings')
->set('standalone_url', TRUE)
->save(TRUE);
$this->container->get('router.builder')->rebuild();
$assert_session = $this->assertSession();
$media_type = $this->createMediaType('test');
$permissions = [
'view media',
'view own unpublished media',
];
$user_one = $this->drupalCreateUser($permissions);
$user_two = $this->drupalCreateUser($permissions);
// Create media as user one.
$user_media = Media::create([
'bundle' => $media_type->id(),
'name' => 'Unnamed',
'uid' => $user_one->id(),
]);
$user_media->setUnpublished()->save();
// Make sure user two can't access unpublished media.
$this->drupalLogin($user_two);
$this->drupalGet('media/' . $user_media->id());
$assert_session->statusCodeEquals(403);
$this->assertCacheContext('user');
$this->drupalLogout();
// Make sure user one can access own unpublished media.
$this->drupalLogin($user_one);
$this->drupalGet('media/' . $user_media->id());
$assert_session->statusCodeEquals(200);
$this->assertCacheContext('user');
}
/**
* Tests media access of anonymous user.
*/
public function testMediaAnonymousUserAccess() {
\Drupal::configFactory()
->getEditable('media.settings')
->set('standalone_url', TRUE)
->save(TRUE);
$this->container->get('router.builder')->rebuild();
$assert_session = $this->assertSession();
$media_type = $this->createMediaType('test');
// Create media as anonymous user.
$user_media = Media::create([
'bundle' => $media_type->id(),
'name' => 'Unnamed',
'uid' => 0,
]);
$user_media->save();
$role = Role::load(RoleInterface::ANONYMOUS_ID);
$this->grantPermissions($role, ['view media', 'view own unpublished media']);
$this->drupalLogout();
// Make sure anonymous users can access published media.
$user_media->setPublished()->save();
$this->drupalGet('media/' . $user_media->id());
$assert_session->statusCodeEquals(200);
// Make sure anonymous users can not access unpublished media
// even though role has 'view own unpublished media' permission.
$user_media->setUnpublished()->save();
$this->drupalGet('media/' . $user_media->id());
$assert_session->statusCodeEquals(403);
$this->assertCacheContext('user');
}
/**
* Tests access for embedded medias.
*/
public function testReferencedRendering() {
\Drupal::configFactory()
->getEditable('media.settings')
->set('standalone_url', TRUE)
->save(TRUE);
$this->container->get('router.builder')->rebuild();
// Create a media type and a entity reference to itself.
$media_type = $this->createMediaType('test');
FieldStorageConfig::create([
'field_name' => 'field_reference',
'entity_type' => 'media',
'type' => 'entity_reference',
'settings' => [
'target_type' => 'media',
],
])->save();
FieldConfig::create([
'field_name' => 'field_reference',
'entity_type' => 'media',
'bundle' => $media_type->id(),
])->save();
$author = $this->drupalCreateUser([
'view media',
'view own unpublished media',
]);
$other_user = $this->drupalCreateUser([
'view media',
'view own unpublished media',
]);
$view_user = $this->drupalCreateUser(['view media']);
$child_title = 'Child media';
$media_child = Media::create([
'name' => $child_title,
'bundle' => $media_type->id(),
'uid' => $author->id(),
]);
$media_child->setUnpublished()->save();
$media_parent = Media::create([
'name' => 'Parent media',
'bundle' => $media_type->id(),
'field_reference' => $media_child->id(),
]);
$media_parent->save();
entity_get_display('media', $media_type->id(), 'full')
->set('content', [])
->setComponent('title', ['type' => 'string'])
->setComponent('field_reference', [
'type' => 'entity_reference_label',
])
->save();
$assert_session = $this->assertSession();
// The author of the child media items should have access to both the parent
// and child.
$this->drupalLogin($author);
$this->drupalGet($media_parent->toUrl());
$this->assertCacheContext('user');
$assert_session->pageTextContains($child_title);
// Other users with the 'view own unpublished media' permission should not
// be able to see the unpublished child media item. The 'user' cache context
// should be added in this case.
$this->drupalLogin($other_user);
$this->drupalGet($media_parent->toUrl());
$this->assertCacheContext('user');
$assert_session->pageTextNotContains($child_title);
// User with just the 'view media' permission should not be able to see the
// child media item. The 'user' cache context should not be added in this
// case.
$this->drupalLogin($view_user);
$this->drupalGet($media_parent->toUrl());
$this->assertNoCacheContext('user');
$assert_session->pageTextNotContains($child_title);
}
}

View File

@ -277,7 +277,7 @@ abstract class MediaResourceTestBase extends EntityResourceTestBase {
switch ($method) {
case 'GET';
return "The 'view media' permission is required and the media item must be published.";
return "The 'view media' permission is required when the media item is published.";
case 'POST':
return "The following permissions are required: 'administer media' OR 'create media' OR 'create camelids media'.";

View File

@ -0,0 +1,552 @@
<?php
namespace Drupal\Tests\media\Kernel;
use Drupal\Core\Access\AccessResult;
use Drupal\Core\Access\AccessResultInterface;
use Drupal\media\Entity\Media;
use Drupal\Tests\user\Traits\UserCreationTrait;
/**
* Tests the media access control handler.
*
* @group media
*
* @coversDefaultClass \Drupal\media\MediaAccessControlHandler
*/
class MediaAccessControlHandlerTest extends MediaKernelTestBase {
use UserCreationTrait;
/**
* Tests the media access control handler.
*
* @param string[] $permissions
* The permissions that the user should be given.
* @param array $entity_values
* Initial values from which to create the media entity.
* @param string $operation
* The operation, one of 'view', 'update' or 'delete'.
* @param \Drupal\Core\Access\AccessResultInterface $expected_result
* Expected result.
* @param string[] $expected_cache_contexts
* Expected cache contexts.
* @param string[] $expected_cache_tags
* Expected cache tags.
*
* @covers ::checkAccess
* @dataProvider providerAccess
*/
public function testAccess(array $permissions, array $entity_values, $operation, AccessResultInterface $expected_result, array $expected_cache_contexts, array $expected_cache_tags) {
// Set a fixed ID so the type specific permissions match.
$media_type = $this->createMediaType('test', [
'id' => 'test',
]);
$user = $this->createUser($permissions);
$entity_values += [
'status' => FALSE,
'uid' => $user->id(),
'bundle' => $media_type->id(),
];
$entity = Media::create($entity_values);
$entity->save();
/** @var \Drupal\Core\Entity\EntityAccessControlHandlerInterface $access_handler */
$access_handler = $this->container->get('entity_type.manager')->getAccessControlHandler('media');
$this->assertAccess($expected_result, $expected_cache_contexts, $expected_cache_tags, $access_handler->access($entity, $operation, $user, TRUE));
}
/**
* @param string[] $permissions
* User permissions.
* @param \Drupal\Core\Access\AccessResultInterface $expected_result
* Expected result.
* @param string[] $expected_cache_contexts
* Expected cache contexts.
* @param string[] $expected_cache_tags
* Expected cache tags.
*
* @covers ::checkCreateAccess
* @dataProvider providerCreateAccess
*/
public function testCreateAccess(array $permissions, AccessResultInterface $expected_result, array $expected_cache_contexts, array $expected_cache_tags) {
$user = $this->createUser($permissions);
/** @var \Drupal\Core\Entity\EntityAccessControlHandlerInterface $access_handler */
$access_handler = $this->container->get('entity_type.manager')->getAccessControlHandler('media');
$this->assertAccess($expected_result, $expected_cache_contexts, $expected_cache_tags, $access_handler->createAccess('test', $user, [], TRUE));
}
/**
* Asserts an access result.
*
* @param \Drupal\Core\Access\AccessResultInterface $expected_access_result
* The expected access result.
* @param string[] $expected_cache_contexts
* Expected contexts.
* @param string[] $expected_cache_tags
* Expected cache tags
* @param \Drupal\Core\Access\AccessResultInterface $actual
* The actual access result.
*/
protected function assertAccess(AccessResultInterface $expected_access_result, array $expected_cache_contexts, array $expected_cache_tags, AccessResultInterface $actual) {
$this->assertSame($expected_access_result->isAllowed(), $actual->isAllowed());
$this->assertSame($expected_access_result->isForbidden(), $actual->isForbidden());
$this->assertSame($expected_access_result->isNeutral(), $actual->isNeutral());
$actual_cache_contexts = $actual->getCacheContexts();
sort($expected_cache_contexts);
sort($actual_cache_contexts);
$this->assertSame($expected_cache_contexts, $actual_cache_contexts);
$actual_cache_tags = $actual->getCacheTags();
sort($expected_cache_tags);
sort($actual_cache_tags);
$this->assertSame($expected_cache_tags, $actual_cache_tags);
}
/**
* Data provider for testAccess().
*
* @return array
* The data sets to test.
*/
public function providerAccess() {
$test_data = [];
// Check published / unpublished media access for a user owning the media
// item without permissions.
$test_data['owner, no permissions / published / view'] = [
[],
['status' => TRUE],
'view',
AccessResult::neutral(),
['user.permissions'],
['media:1'],
];
$test_data['owner, no permissions / published / update'] = [
[],
['status' => TRUE],
'update',
AccessResult::neutral(),
['user.permissions'],
[],
];
$test_data['owner, no permissions / published / delete'] = [
[],
['status' => TRUE],
'delete',
AccessResult::neutral(),
['user.permissions'],
[],
];
$test_data['owner, no permissions / unpublished / view'] = [
[],
[],
'view',
AccessResult::neutral(),
['user.permissions'],
['media:1'],
];
$test_data['owner, no permissions / unpublished / update'] = [
[],
[],
'update',
AccessResult::neutral(),
['user.permissions'],
[],
];
$test_data['owner, no permissions / unpublished / delete'] = [
[],
[],
'delete',
AccessResult::neutral(),
['user.permissions'],
[],
];
// Check published / unpublished media access for a user not owning the
// media item without permissions.
$test_data['not owner, no permissions / published / view'] = [
[],
['uid' => 0, 'status' => TRUE],
'view',
AccessResult::neutral(),
['user.permissions'],
['media:1'],
];
$test_data['not owner, no permissions / published / update'] = [
[],
['uid' => 0, 'status' => TRUE],
'update',
AccessResult::neutral(),
['user.permissions'],
[],
];
$test_data['not owner, no permissions / published / delete'] = [
[],
['uid' => 0, 'status' => TRUE],
'delete',
AccessResult::neutral(),
['user.permissions'],
[],
];
$test_data['not owner, no permissions / unpublished / view'] = [
[],
['uid' => 0],
'view',
AccessResult::neutral(),
['user.permissions'],
['media:1'],
];
$test_data['not owner, no permissions / unpublished / update'] = [
[],
['uid' => 0],
'update',
AccessResult::neutral(),
['user.permissions'],
[],
];
$test_data['not owner, no permissions / unpublished / delete'] = [
[],
['uid' => 0],
'delete',
AccessResult::neutral(),
['user.permissions'],
[],
];
// Check published / unpublished media access for a user owning the media
// item with only the 'view media' permission.
$test_data['owner, can view media / published / view'] = [
['view media'],
['status' => TRUE],
'view',
AccessResult::allowed(),
['user.permissions'],
['media:1'],
];
$test_data['owner, can view media / published / update'] = [
['view media'],
['status' => TRUE],
'update',
AccessResult::neutral(),
['user.permissions'],
[],
];
$test_data['owner, can view media / published / delete'] = [
['view media'],
['status' => TRUE],
'delete',
AccessResult::neutral(),
['user.permissions'],
[],
];
$test_data['owner, can view media / unpublished / view'] = [
['view media'],
[],
'view',
AccessResult::neutral(),
['user.permissions'],
['media:1'],
];
$test_data['owner, can view media / unpublished / update'] = [
['view media'],
[],
'update',
AccessResult::neutral(),
['user.permissions'],
[],
];
$test_data['owner, can view media / unpublished / delete'] = [
['view media'],
[],
'delete',
AccessResult::neutral(),
['user.permissions'],
[],
];
// Check published / unpublished media access for a user not owning the
// media item with only the 'view media' permission.
$test_data['not owner, can view media / published / view'] = [
['view media'],
['uid' => 0, 'status' => TRUE],
'view',
AccessResult::allowed(),
['user.permissions'],
['media:1'],
];
$test_data['not owner, can view media / published / update'] = [
['view media'],
['uid' => 0, 'status' => TRUE],
'update',
AccessResult::neutral(),
['user.permissions'],
[],
];
$test_data['not owner, can view media / published / delete'] = [
['view media'],
['uid' => 0, 'status' => TRUE],
'delete',
AccessResult::neutral(),
['user.permissions'],
[],
];
$test_data['not owner, can view media / unpublished / view'] = [
['view media'],
['uid' => 0],
'view',
AccessResult::neutral(),
['user.permissions'],
['media:1'],
];
$test_data['not owner, can view media / unpublished / update'] = [
['view media'],
['uid' => 0],
'update',
AccessResult::neutral(),
['user.permissions'],
[],
];
$test_data['not owner, can view media / unpublished / delete'] = [
['view media'],
['uid' => 0],
'delete',
AccessResult::neutral(),
['user.permissions'],
[],
];
// Check published / unpublished media access for a user owning the media
// item with the 'view media' and 'view own unpublished' permission.
$test_data['owner, can view own unpublished media / published / view'] = [
['view media', 'view own unpublished media'],
['status' => TRUE],
'view',
AccessResult::allowed(),
['user.permissions'],
['media:1'],
];
$test_data['owner, can view own unpublished media / published / update'] = [
['view media', 'view own unpublished media'],
['status' => TRUE],
'update',
AccessResult::neutral(),
['user.permissions'],
[],
];
$test_data['owner, can view own unpublished media / published / delete'] = [
['view media', 'view own unpublished media'],
['status' => TRUE],
'delete',
AccessResult::neutral(),
['user.permissions'],
[],
];
$test_data['owner, can view own unpublished media / unpublished / view'] = [
['view media', 'view own unpublished media'],
[],
'view',
AccessResult::allowed(),
['user.permissions', 'user'],
['media:1'],
];
$test_data['owner, can view own unpublished media / unpublished / update'] = [
['view media', 'view own unpublished media'],
[],
'update',
AccessResult::neutral(),
['user.permissions'],
[],
];
$test_data['owner, can view own unpublished media / unpublished / delete'] = [
['view media', 'view own unpublished media'],
[],
'delete',
AccessResult::neutral(),
['user.permissions'],
[],
];
// Check published / unpublished media access for a user not owning the
// media item with the 'view media' and 'view own unpublished' permission.
$test_data['not owner, can view own unpublished media / published / view'] = [
['view media', 'view own unpublished media'],
['uid' => 0, 'status' => TRUE],
'view',
AccessResult::allowed(),
['user.permissions'],
['media:1'],
];
$test_data['not owner, can view own unpublished media / published / update'] = [
['view media', 'view own unpublished media'],
['uid' => 0, 'status' => TRUE],
'update',
AccessResult::neutral(),
['user.permissions'],
[],
];
$test_data['not owner, can view own unpublished media / published / delete'] = [
['view media', 'view own unpublished media'],
['uid' => 0, 'status' => TRUE],
'delete',
AccessResult::neutral(),
['user.permissions'],
[],
];
$test_data['not owner, can view own unpublished media / unpublished / view'] = [
['view media', 'view own unpublished media'],
['uid' => 0],
'view',
AccessResult::neutral(),
['user.permissions', 'user'],
['media:1'],
];
$test_data['not owner, can view own unpublished media / unpublished / update'] = [
['view media', 'view own unpublished media'],
['uid' => 0],
'update',
AccessResult::neutral(),
['user.permissions'],
[],
];
$test_data['not owner, can view own unpublished media / unpublished / delete'] = [
['view media', 'view own unpublished media'],
['uid' => 0],
'delete',
AccessResult::neutral(),
['user.permissions'],
[],
];
return $test_data;
}
/**
* Data provider for testCreateAccess().
*
* @return array
* The data sets to test.
*/
public function providerCreateAccess() {
$test_data = [];
// Check create access for a user without permissions.
$test_data['user, no permissions / create'] = [
[],
AccessResult::neutral()->setReason("The following permissions are required: 'administer media' OR 'create media'."),
['user.permissions'],
[],
];
// Check create access for a user with the 'view media' permission.
$test_data['user, can view media / create'] = [
[
'view media',
],
AccessResult::neutral("The following permissions are required: 'administer media' OR 'create media'."),
['user.permissions'],
[],
];
// Check create access for a user with the 'view media' and 'view own
// unpublished media' permission.
$test_data['user, can view own unpublished media / create'] = [
[
'view media',
'view own unpublished media',
],
AccessResult::neutral("The following permissions are required: 'administer media' OR 'create media'."),
['user.permissions'],
[],
];
// Check create access for a user with the 'view media', 'view own
// unpublished media', 'update any media' and 'delete any media' permission.
$test_data['user, can view own unpublished media and update or delete any media / create'] = [
[
'view media',
'view own unpublished media',
'update any media',
'delete any media',
],
AccessResult::neutral("The following permissions are required: 'administer media' OR 'create media'."),
['user.permissions'],
[],
];
// Check create access for a user with the 'view media', 'view own
// unpublished media', 'update media' and 'delete media' permission.
$test_data['user, can view own unpublished media and update or delete own media / create'] = [
[
'view media',
'view own unpublished media',
'update media',
'delete media',
],
AccessResult::neutral("The following permissions are required: 'administer media' OR 'create media'."),
['user.permissions'],
[],
];
// Check create access for a user with the 'view media', 'view own
// unpublished media', 'update any media', 'delete any media', 'update
// media' and 'delete media' permission.
$test_data['user, can view own unpublished media and update or delete all media / create'] = [
[
'view media',
'view own unpublished media',
'update any media',
'delete any media',
'update media',
'delete media',
],
AccessResult::neutral("The following permissions are required: 'administer media' OR 'create media'."),
['user.permissions'],
[],
];
// Check create access for a user with all media permissions except 'create
// media' or 'administer media'.
$test_data['user, can not create or administer media / create'] = [
[
'access media overview',
'view media',
'view own unpublished media',
'update any media',
'delete any media',
'update media',
'delete media',
],
AccessResult::neutral("The following permissions are required: 'administer media' OR 'create media'."),
['user.permissions'],
[],
];
// Check create access for a user with the 'create media' permission.
$test_data['user, can create media / create'] = [
[
'create media',
],
AccessResult::allowed(),
['user.permissions'],
[],
];
// Check create access for a user with the 'administer media' permission.
$test_data['user, can administer media / create'] = [
[
'administer media',
],
AccessResult::allowed(),
['user.permissions'],
[],
];
return $test_data;
}
}