diff --git a/core/modules/user/src/Plugin/views/access/Permission.php b/core/modules/user/src/Plugin/views/access/Permission.php index ccc15b4..7e76e64 100644 --- a/core/modules/user/src/Plugin/views/access/Permission.php +++ b/core/modules/user/src/Plugin/views/access/Permission.php @@ -87,7 +87,7 @@ public static function create(ContainerInterface $container, array $configuratio * {@inheritdoc} */ public function access(AccountInterface $account) { - return AccessResult::allowedIfHasPermission($account, $this->options); + return AccessResult::allowedIfHasPermission($account, $this->options['perm']); } /** diff --git a/core/modules/views/src/Tests/Plugin/AccessTest.php b/core/modules/views/src/Tests/Plugin/AccessTest.php index 5ee6c98..71f6b34 100644 --- a/core/modules/views/src/Tests/Plugin/AccessTest.php +++ b/core/modules/views/src/Tests/Plugin/AccessTest.php @@ -7,6 +7,7 @@ namespace Drupal\views\Tests\Plugin; +use Drupal\Core\Access\AccessResultInterface; use Drupal\Core\Cache\Cache; use Drupal\views\Tests\ViewTestData; use Drupal\views\Views; @@ -71,8 +72,14 @@ function testAccessNone() { $view = Views::getView('test_access_none'); $view->setDisplay(); - $this->assertTrue($view->display_handler->access($this->webUser)); - $this->assertTrue($view->display_handler->access($this->normalUser)); + $access = $view->display_handler->access($this->webUser); + $this->assertTrue($access instanceof AccessResultInterface); + $this->assertTrue($access->isAllowed()); + + $access = $view->display_handler->access($this->normalUser); + $this->assertTrue($access); + $this->assertTrue($access instanceof AccessResultInterface); + $this->assertTrue($access->isAllowed()); } /** @@ -90,7 +97,9 @@ function testStaticAccessPlugin() { $access_plugin = $view->display_handler->getPlugin('access'); - $this->assertFalse($access_plugin->access($this->normalUser)); + $access = $access_plugin->access($this->normalUser); + $this->assertTrue($access instanceof AccessResultInterface); + $this->assertFalse($access->isAllowed()); $this->drupalGet('test_access_static'); $this->assertResponse(403); @@ -102,7 +111,9 @@ function testStaticAccessPlugin() { // termination event fires. Simulate that here. $this->container->get('router.builder')->rebuildIfNeeded(); - $this->assertTrue($access_plugin->access($this->normalUser)); + $access = $access_plugin->access($this->normalUser); + $this->assertTrue($access instanceof AccessResultInterface); + $this->assertTrue($access->isAllowed()); $this->drupalGet('test_access_static'); $this->assertResponse(200); diff --git a/core/modules/views/src/ViewExecutable.php b/core/modules/views/src/ViewExecutable.php index ae7ebe2..b915fd0 100644 --- a/core/modules/views/src/ViewExecutable.php +++ b/core/modules/views/src/ViewExecutable.php @@ -1646,6 +1646,10 @@ public function access($displays = NULL, AccountInterface $account = NULL, $retu $this->initDisplay(); } + if (!isset($displays)) { + $displays = [$this->current_display]; + } + if (!$account) { $account = $this->user; } @@ -1657,6 +1661,8 @@ public function access($displays = NULL, AccountInterface $account = NULL, $retu foreach ($displays as $display_id) { if ($this->displayHandlers->has($display_id)) { if (($display = $this->displayHandlers->get($display_id))) { + // @fixme This is certainly not the right thing to do. We want to not + // combine all of them but rather just match one of them. $access = $access->orIf($display->access($account)); } } diff --git a/core/modules/views/tests/src/Unit/ViewExecutableTest.php b/core/modules/views/tests/src/Unit/ViewExecutableTest.php index fea412b..65ca9ee 100644 --- a/core/modules/views/tests/src/Unit/ViewExecutableTest.php +++ b/core/modules/views/tests/src/Unit/ViewExecutableTest.php @@ -7,6 +7,8 @@ namespace Drupal\Tests\views\Unit; +use Drupal\Core\Access\AccessResult; +use Drupal\Core\Access\AccessResultInterface; use Drupal\Core\DependencyInjection\ContainerBuilder; use Drupal\Core\Url; use Drupal\Tests\UnitTestCase; @@ -429,6 +431,142 @@ public function testAttachDisplays() { } /** + * @covers ::access + */ + public function testAccessWithDisabledView() { + $this->view->expects($this->any()) + ->method('status') + ->willReturn(FALSE); + $access = $this->executable->access(NULL, NULL, TRUE); + + $this->assertInstanceOf(AccessResultInterface::CLASS, $access); + $this->assertTrue($access->isForbidden()); + } + + /** + * @covers ::access + */ + public function testAccessWithActiveDisplayAccessDenied() { + $this->view->expects($this->any()) + ->method('status') + ->willReturn(TRUE); + + $this->displayHandlers->expects($this->any()) + ->method('has') + ->with('page_1') + ->willReturn(TRUE); + + $this->displayHandlers->expects($this->any()) + ->method('get') + ->with('page_1') + ->willReturn($this->displayHandler); + + $this->executable->current_display = 'page_1'; + + $this->displayHandler->expects($this->atLeastOnce()) + ->method('access') + ->willReturn(AccessResult::forbidden()); + + $access = $this->executable->access(NULL, NULL, TRUE); + + $this->assertInstanceOf(AccessResultInterface::CLASS, $access); + $this->assertFalse($access->isAllowed()); + + $access = $this->executable->access(['page_1'], NULL, TRUE); + + $this->assertInstanceOf(AccessResultInterface::CLASS, $access); + $this->assertFalse($access->isAllowed()); + } + + /** + * @covers ::access + */ + public function testAccessWithActiveDisplayAccessAllowed() { + $this->view->expects($this->any()) + ->method('status') + ->willReturn(TRUE); + + $this->displayHandlers->expects($this->any()) + ->method('has') + ->with('page_1') + ->willReturn(TRUE); + + $this->displayHandlers->expects($this->any()) + ->method('get') + ->with('page_1') + ->willReturn($this->displayHandler); + + $this->executable->current_display = 'page_1'; + + $this->displayHandler->expects($this->atLeastOnce()) + ->method('access') + ->willReturn(AccessResult::allowed()); + + $access = $this->executable->access(NULL, NULL, TRUE); + + $this->assertInstanceOf(AccessResultInterface::CLASS, $access); + $this->assertTrue($access->isAllowed()); + + $access = $this->executable->access(['page_1'], NULL, TRUE); + + $this->assertInstanceOf(AccessResultInterface::CLASS, $access); + $this->assertTrue($access->isAllowed()); + } + + /** + * @covers ::access + * + * @dataProvider providerTestAccessWithMultipleDisplay + */ + public function testAccessWithMultipleDisplay($page_1_access, $page_2_access, $expected) { + $this->view->expects($this->any()) + ->method('status') + ->willReturn(TRUE); + + $this->executable->current_display = 'default'; + $this->displayHandlers->expects($this->any()) + ->method('has') + ->willReturnMap([ + ['page_1', TRUE], + ['page_2', TRUE], + ]) + ->willReturn(TRUE); + + $page_1 = clone $this->displayHandler; + $page_2 = clone $this->displayHandler; + $this->displayHandlers->expects($this->any()) + ->method('get') + ->willReturnMap([ + ['page_1', $page_1], + ['page_2', $page_2], + ]); + + $page_1->expects($this->atLeastOnce()) + ->method('access') + ->willReturn($page_1_access); + $page_2->expects($this->atLeastOnce()) + ->method('access') + ->willReturn($page_2_access); + + $access = $this->executable->access(['page_1', 'page_2'], NULL, TRUE); + + $this->assertInstanceOf(AccessResultInterface::CLASS, $access); + $this->assertEquals($expected, $access); + } + + public function providerTestAccessWithMultipleDisplay() { + $data = [ + [AccessResult::allowed(), AccessResult::allowed(), AccessResult::allowed()], + [AccessResult::neutral(), AccessResult::allowed(), AccessResult::allowed()], + [AccessResult::allowed(), AccessResult::neutral(), AccessResult::allowed()], + [AccessResult::neutral(), AccessResult::neutral(), AccessResult::neutral()], + [AccessResult::forbidden(), AccessResult::allowed(), AccessResult::allowed()], + ]; + + return $data; + } + + /** * Setups a view executable and default display. * * @return array