Issue #2558905 by dawehner, pwolanin, cilefen, plach, willzyx: Content...

Issue #2558905 by dawehner, pwolanin, cilefen, plach, willzyx: Content translation module - Information disclosure by insufficient access checking

core/modules/content_translation/content_translation.module

@@ -267,7 +267,8 @@ function content_translation_menu_links_discovered_alter(array &$links) {
  */
 function content_translation_translate_access(EntityInterface $entity) {
   $account = \Drupal::currentUser();
-  $condition = $entity instanceof ContentEntityInterface && !$entity->getUntranslated()->language()->isLocked() && \Drupal::languageManager()->isMultilingual() && $entity->isTranslatable() &&
+  $condition = $entity instanceof ContentEntityInterface && $entity->access('view') &&
+    !$entity->getUntranslated()->language()->isLocked() && \Drupal::languageManager()->isMultilingual() && $entity->isTranslatable() &&
     ($account->hasPermission('create content translations') || $account->hasPermission('update content translations') || $account->hasPermission('delete content translations'));
   return AccessResult::allowedIf($condition)->cachePerPermissions()->cacheUntilEntityChanges($entity);
 }

core/modules/content_translation/src/Routing/ContentTranslationRouteSubscriber.php

@@ -69,6 +69,7 @@ protected function alterRoutes(RouteCollection $collection) {
           'entity_type_id' => $entity_type_id,
         ),
         array(
+          '_entity_access' =>  $entity_type_id . '.view',
           '_access_content_translation_overview' => $entity_type_id,
         ),
         array(
@@ -94,6 +95,7 @@ protected function alterRoutes(RouteCollection $collection) {
 
         ),
         array(
+          '_entity_access' =>  $entity_type_id . '.view',
           '_access_content_translation_manage' => 'create',
         ),
         array(

core/modules/content_translation/src/Tests/ContentTestTranslationUITest.php

@@ -39,7 +39,7 @@ protected function setUp() {
    * Overrides \Drupal\content_translation\Tests\ContentTranslationUITestBase::getTranslatorPermission().
    */
   protected function getTranslatorPermissions() {
-    return array_merge(parent::getTranslatorPermissions(), array('administer entity_test content'));
+    return array_merge(parent::getTranslatorPermissions(), array('administer entity_test content', 'view test entity'));
   }
 
 }

core/modules/content_translation/src/Tests/ContentTranslationOperationsTest.php

@@ -9,6 +9,7 @@
 
 use Drupal\language\Entity\ConfigurableLanguage;
 use Drupal\node\Tests\NodeTestBase;
+use Drupal\user\Entity\Role;
 
 /**
  * Tests the content translation operations available in the content listing.
@@ -75,6 +76,63 @@ function testOperationTranslateLink() {
     $this->drupalLogin($this->baseUser2);
     $this->drupalGet('admin/content');
     $this->assertLinkByHref('node/' . $node->id() . '/translations');
+
+    // Ensure that an unintended misconfiguration of permissions does not open
+    // access to the translation form, see https://www.drupal.org/node/2558905.
+    $this->drupalLogout();
+    user_role_change_permissions(
+      Role::AUTHENTICATED_ID,
+      [
+        'create content translations' => TRUE,
+        'access content' => FALSE,
+      ]
+    );
+    $this->drupalLogin($this->baseUser1);
+    $this->drupalGet($node->urlInfo('drupal:content-translation-overview'));
+    $this->assertResponse(403);
+
+    // Ensure that the translation overview is also not accessible when the user
+    // has 'access content', but the node is not published.
+    user_role_change_permissions(
+      Role::AUTHENTICATED_ID,
+      [
+        'create content translations' => TRUE,
+        'access content' => TRUE,
+      ]
+    );
+    $node->setPublished(FALSE)->save();
+    $this->drupalGet($node->urlInfo('drupal:content-translation-overview'));
+    $this->assertResponse(403);
+  }
+
+  /**
+   * @see content_translation_translate_access()
+   */
+  public function testContentTranslationOverviewAccess() {
+    $access_control_handler = \Drupal::entityManager()->getAccessControlHandler('node');
+    $user = $this->createUser(['create content translations', 'access content']);
+    $this->drupalLogin($user);
+
+    $node = $this->drupalCreateNode(['status' => FALSE, 'type' => 'article']);
+    $this->assertFalse(content_translation_translate_access($node)->isAllowed());
+    $access_control_handler->resetCache();
+
+    $node->setPublished(TRUE);
+    $node->save();
+    $this->assertTrue(content_translation_translate_access($node)->isAllowed());
+    $access_control_handler->resetCache();
+
+    user_role_change_permissions(
+      Role::AUTHENTICATED_ID,
+      [
+        'access content' => FALSE,
+      ]
+    );
+
+    $user = $this->createUser(['create content translations']);
+    $this->drupalLogin($user);
+    $this->assertFalse(content_translation_translate_access($node)->isAllowed());
+    $access_control_handler->resetCache();
   }
 
 }

core/modules/content_translation/src/Tests/ContentTranslationWorkflowsTest.php

@@ -38,6 +38,16 @@ protected function setUp() {
     $this->setupEntity();
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  protected function getTranslatorPermissions() {
+    $permissions = parent::getTranslatorPermissions();
+    $permissions[] = 'view test entity';
+
+    return $permissions;
+  }
+
   /**
    * Overrides \Drupal\content_translation\Tests\ContentTranslationTestBase::getEditorPermissions().
    */
@@ -109,7 +119,7 @@ function testWorkflows() {
     $ops = array('create' => t('Add'), 'update' => t('Edit'), 'delete' => t('Delete'));
     $translations_url = $this->entity->urlInfo('drupal:content-translation-overview');
     foreach ($ops as $current_op => $item) {
-      $user = $this->drupalCreateUser(array($this->getTranslatePermission(), "$current_op content translations"));
+      $user = $this->drupalCreateUser(array($this->getTranslatePermission(), "$current_op content translations", 'view test entity'));
       $this->drupalLogin($user);
       $this->drupalGet($translations_url);
 

core/modules/content_translation/src/Tests/Views/TranslationLinkTest.php

@@ -54,6 +54,15 @@ protected function setUp() {
     ViewTestData::createTestViews(get_class($this), array('content_translation_test_views'));
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  protected function getTranslatorPermissions() {
+    $permissions = parent::getTranslatorPermissions();
+    $permissions[] = 'access user profiles';
+    return $permissions;
+  }
+
   /**
    * Tests the content translation overview link field handler.
    */

core/modules/menu_link_content/src/MenuLinkContentAccessControlHandler.php

@@ -54,8 +54,9 @@ public static function createInstance(ContainerInterface $container, EntityTypeI
   protected function checkAccess(EntityInterface $entity, $operation, $langcode, AccountInterface $account) {
     switch ($operation) {
       case 'view':
-        // There is no direct view.
-        return AccessResult::neutral();
+        // There is no direct viewing of a menu link, but still for purposes of
+        // content_translation we need a generic way to check access.
+        return AccessResult::allowedIfHasPermission($account, 'administer menu');
 
       case 'update':
         if (!$account->hasPermission('administer menu')) {

core/modules/rest/src/Tests/ResourceTest.php

@@ -6,6 +6,8 @@
  */
 
 namespace Drupal\rest\Tests;
+use Drupal\Core\Session\AccountInterface;
+use Drupal\user\Entity\Role;
 
 /**
  * Tests the structure of a REST resource.
@@ -38,6 +40,10 @@ protected function setUp() {
     // Create an entity programmatically.
     $this->entity = $this->entityCreate('entity_test');
     $this->entity->save();
+
+    Role::load(AccountInterface::ANONYMOUS_ROLE)
+      ->grantPermission('view test entity')
+      ->save();
   }
 
   /**

core/modules/system/src/Tests/Entity/EntityViewControllerTest.php

@@ -39,6 +39,7 @@ protected function setUp() {
       $this->entities[] = $entity_test;
     }
 
+    $this->drupalLogin($this->drupalCreateUser(['view test entity']));
   }
 
   /**

core/modules/system/tests/modules/entity_test/entity_test.routing.yml

@@ -4,7 +4,7 @@ entity.entity_test.canonical:
     _entity_view: 'entity_test.full'
     _title: 'Test full view mode'
   requirements:
-    _access: 'TRUE'
+    _entity_access: 'entity_test.view'
 
 entity.entity_test.render_options:
   path: '/entity_test_converter/{foo}'
@@ -15,7 +15,7 @@ entity.entity_test.render_options:
   defaults:
     _entity_view: 'entity_test.full'
   requirements:
-    _access: 'TRUE'
+    _entity_access: 'foo.view'
 
 entity.entity_test.render_no_view_mode:
   path: '/entity_test_no_view_mode/{entity_test}'

core/modules/views/src/Tests/Handler/FieldEntityLinkTest.php

@@ -10,6 +10,7 @@
 use Drupal\Core\Session\AccountInterface;
 use Drupal\entity_test\Entity\EntityTest;
 use Drupal\simpletest\UserCreationTrait;
+use Drupal\user\Entity\Role;
 use Drupal\views\Tests\ViewKernelTestBase;
 use Drupal\views\Views;
 
@@ -51,6 +52,7 @@ protected function setUpFixtures() {
 
     $this->installEntitySchema('user');
     $this->installEntitySchema('entity_test');
+    $this->installConfig(['user']);
 
     // Create some test entities.
     for ($i = 0; $i < 5; $i++) {
@@ -58,7 +60,11 @@ protected function setUpFixtures() {
     }
 
     // Create and admin user.
-    $this->adminUser = $this->createUser([], FALSE, TRUE);
+    $this->adminUser = $this->createUser(['view test entity'], FALSE, TRUE);
+
+    Role::load(AccountInterface::ANONYMOUS_ROLE)
+      ->grantPermission('view test entity')
+      ->save();
   }
 
   /**

core/tests/Drupal/Tests/Core/Entity/EntityAccessCheckTest.php

@@ -9,6 +9,9 @@
 
 use Drupal\Core\Cache\Context\CacheContextsManager;
 use Drupal\Core\DependencyInjection\Container;
+use Drupal\Core\Routing\RouteMatchInterface;
+use Drupal\Core\Session\AccountInterface;
+use Drupal\node\NodeInterface;
 use Symfony\Component\HttpFoundation\ParameterBag;
 use Symfony\Component\Routing\Route;
 use Drupal\Core\Access\AccessResult;
@@ -18,37 +21,67 @@
 /**
  * Unit test of entity access checking system.
  *
+ * @coversDefaultClass \Drupal\Core\Entity\EntityAccessCheck
+ *
  * @group Access
  * @group Entity
  */
 class EntityAccessCheckTest extends UnitTestCase {
 
   /**
-   * Tests the method for checking access to routes.
+   * {@inheritdoc}
    */
-  public function testAccess() {
+  protected function setUp() {
     $cache_contexts_manager = $this->prophesize(CacheContextsManager::class)->reveal();
     $container = new Container();
     $container->set('cache_contexts_manager', $cache_contexts_manager);
     \Drupal::setContainer($container);
+  }
+
+  /**
+   * Tests the method for checking access to routes.
+   */
+  public function testAccess() {
+    $route = new Route('/foo/{var_name}', [], ['_entity_access' => 'var_name.update'], ['parameters' => ['var_name' => ['type' => 'entity:node']]]);
+    /** @var \Drupal\Core\Session\AccountInterface $account */
+    $account = $this->prophesize(AccountInterface::class)->reveal();
+
+    /** @var \Drupal\node\NodeInterface|\Prophecy\Prophecy\ObjectProphecy $route_match */
+    $node = $this->prophesize(NodeInterface::class);
+    $node->access('update', $account, TRUE)->willReturn(AccessResult::allowed());
+    $node = $node->reveal();
+
+    /** @var \Drupal\Core\Routing\RouteMatchInterface|\Prophecy\Prophecy\ObjectProphecy $route_match */
+    $route_match = $this->prophesize(RouteMatchInterface::class);
+    $route_match->getRawParameters()->willReturn(new ParameterBag(['var_name' => 1]));
+    $route_match->getParameters()->willReturn(new ParameterBag(['var_name' => $node]));
+    $route_match = $route_match->reveal();
+
+    $access_check = new EntityAccessCheck();
+    $this->assertEquals(AccessResult::allowed(), $access_check->access($route, $route_match, $account));
+  }
+
+  /**
+   * @covers ::access
+   */
+  public function testAccessWithTypePlaceholder() {
+    $route = new Route('/foo/{entity_type}/{var_name}', [], ['_entity_access' => 'var_name.update'], ['parameters' => ['var_name' => ['type' => 'entity:{entity_type}']]]);
+    /** @var \Drupal\Core\Session\AccountInterface $account */
+    $account = $this->prophesize(AccountInterface::class)->reveal();
+
+    /** @var \Drupal\node\NodeInterface|\Prophecy\Prophecy\ObjectProphecy $node */
+    $node = $this->prophesize(NodeInterface::class);
+    $node->access('update', $account, TRUE)->willReturn(AccessResult::allowed());
+    $node = $node->reveal();
+
+    /** @var \Drupal\Core\Routing\RouteMatchInterface|\Prophecy\Prophecy\ObjectProphecy $route_match */
+    $route_match = $this->prophesize(RouteMatchInterface::class);
+    $route_match->getRawParameters()->willReturn(new ParameterBag(['entity_type' => 'node', 'var_name' => 1]));
+    $route_match->getParameters()->willReturn(new ParameterBag(['entity_type' => 'node', 'var_name' => $node]));
+    $route_match = $route_match->reveal();
 
-    $route = new Route('/foo', array(), array('_entity_access' => 'node.update'));
-    $upcasted_arguments = new ParameterBag();
-    $route_match = $this->getMock('Drupal\Core\Routing\RouteMatchInterface');
-    $route_match->expects($this->once())
-      ->method('getParameters')
-      ->will($this->returnValue($upcasted_arguments));
-    $node = $this->getMockBuilder('Drupal\node\Entity\Node')
-      ->disableOriginalConstructor()
-      ->getMock();
-    $node->expects($this->any())
-      ->method('access')
-      ->will($this->returnValue(AccessResult::allowed()->cachePerPermissions()));
     $access_check = new EntityAccessCheck();
-    $upcasted_arguments->set('node', $node);
-    $account = $this->getMock('Drupal\Core\Session\AccountInterface');
-    $access = $access_check->access($route, $route_match, $account);
-    $this->assertEquals(AccessResult::allowed()->cachePerPermissions(), $access);
+    $this->assertEquals(AccessResult::allowed(), $access_check->access($route, $route_match, $account));
   }
 
 }