From 77addbef6ec235a238cf976c82cf4d6db87c5d2b Mon Sep 17 00:00:00 2001 From: Nathaniel Catchpole Date: Tue, 17 Dec 2013 12:29:55 +0000 Subject: [PATCH] Revert "Issue #1912602 by dawehner, dagmar, tim.plunkett, bdone: Changing view access from 'Permission' to 'Role' causes AJAX error message re getRoles()." This reverts commit 0f17b0ec75024f4e4f6d43c2dc01362b02739ab9. --- core/lib/Drupal/Core/Access/AccessManager.php | 12 ++-- .../lib/Drupal/simpletest/WebTestBase.php | 3 - .../Drupal/user/Plugin/views/access/Role.php | 6 +- .../user/Tests/Views/AccessRoleTest.php | 43 +++-------- .../user/Tests/Views/AccessRoleUITest.php | 71 ------------------- .../user/Tests/Views/AccessTestBase.php | 1 - .../views.view.test_access_role.yml | 15 +--- .../Plugin/views/display/PathPluginBase.php | 4 -- .../lib/Drupal/views/ViewsAccessCheck.php | 8 +-- .../Tests/Core/Access/AccessManagerTest.php | 31 -------- 10 files changed, 22 insertions(+), 172 deletions(-) delete mode 100644 core/modules/user/lib/Drupal/user/Tests/Views/AccessRoleUITest.php diff --git a/core/lib/Drupal/Core/Access/AccessManager.php b/core/lib/Drupal/Core/Access/AccessManager.php index f33d47f345..f5b3733df0 100644 --- a/core/lib/Drupal/Core/Access/AccessManager.php +++ b/core/lib/Drupal/Core/Access/AccessManager.php @@ -159,11 +159,13 @@ protected function applies(Route $route) { $checks[] = $service_id; } } - } - // Finally, see if any dynamic access checkers apply. - foreach ($this->dynamicRequirementMap as $service_id) { - if ($this->checks[$service_id]->applies($route)) { - $checks[] = $service_id; + // This means appliesTo() method was empty. Iterate through all checkers. + else { + foreach ($this->dynamicRequirementMap as $service_id) { + if ($this->checks[$service_id]->applies($route)) { + $checks[] = $service_id; + } + } } } diff --git a/core/modules/simpletest/lib/Drupal/simpletest/WebTestBase.php b/core/modules/simpletest/lib/Drupal/simpletest/WebTestBase.php index 7541fb26af..79d8b8f544 100644 --- a/core/modules/simpletest/lib/Drupal/simpletest/WebTestBase.php +++ b/core/modules/simpletest/lib/Drupal/simpletest/WebTestBase.php @@ -645,9 +645,6 @@ protected function drupalLogin(AccountInterface $account) { if ($pass) { $this->loggedInUser = $account; $this->container->set('current_user', $account); - // @todo Temporary workaround for not being able to use synchronized - // services in non dumped container. - $this->container->get('access_subscriber')->setCurrentUser($account); } } diff --git a/core/modules/user/lib/Drupal/user/Plugin/views/access/Role.php b/core/modules/user/lib/Drupal/user/Plugin/views/access/Role.php index 1c4fecc4a6..d8122706f6 100644 --- a/core/modules/user/lib/Drupal/user/Plugin/views/access/Role.php +++ b/core/modules/user/lib/Drupal/user/Plugin/views/access/Role.php @@ -42,9 +42,7 @@ public function access(AccountInterface $account) { * {@inheritdoc} */ public function alterRouteDefinition(Route $route) { - if ($this->options['role']) { - $route->setRequirement('_role', (string) implode(',', $this->options['role'])); - } + $route->setRequirement('_role_id', $this->options['role']); } public function summaryTitle() { @@ -76,7 +74,7 @@ public function buildOptionsForm(&$form, &$form_state) { '#type' => 'checkboxes', '#title' => t('Role'), '#default_value' => $this->options['role'], - '#options' => array_map('\Drupal\Component\Utility\String::checkPlain', user_role_names()), + '#options' => array_map('check_plain', $this->getRoles()), '#description' => t('Only the checked roles will be able to access this display. Note that users with "access all views" can see any view, regardless of role.'), ); } diff --git a/core/modules/user/lib/Drupal/user/Tests/Views/AccessRoleTest.php b/core/modules/user/lib/Drupal/user/Tests/Views/AccessRoleTest.php index 0f0e480c25..7218046381 100644 --- a/core/modules/user/lib/Drupal/user/Tests/Views/AccessRoleTest.php +++ b/core/modules/user/lib/Drupal/user/Tests/Views/AccessRoleTest.php @@ -8,9 +8,6 @@ namespace Drupal\user\Tests\Views; use Drupal\user\Plugin\views\access\Role; -use Drupal\views\Views; -use Drupal\views\ViewStorageInterface; -use Symfony\Component\HttpFoundation\Request; /** * Tests views role access plugin. @@ -38,43 +35,19 @@ public static function getInfo() { * Tests role access plugin. */ function testAccessRole() { - /** @var \Drupal\views\ViewStorageInterface $view */ - $view = \Drupal::entityManager()->getStorageController('view')->load('test_access_role'); - $display = &$view->getDisplay('default'); - $display['display_options']['access']['options']['role'] = array( + $view = views_get_view('test_access_role'); + $view->setDisplay(); + + $view->displayHandlers->get('default')->options['access']['options']['role'] = array( $this->normalRole => $this->normalRole, ); - $view->save(); - - $executable = Views::executableFactory()->get($view); - $executable->setDisplay('page_1'); - $access_plugin = $executable->display_handler->getPlugin('access'); + $access_plugin = $view->display_handler->getPlugin('access'); $this->assertTrue($access_plugin instanceof Role, 'Make sure the right class got instantiated.'); - // Test the access() method on the access plugin. - $this->assertTrue($executable->display_handler->access($this->adminUser), 'Admin-Account should be able to access the view everytime'); - $this->assertFalse($executable->display_handler->access($this->webUser)); - $this->assertTrue($executable->display_handler->access($this->normalUser)); - - // Test the actual access doing a request. - /** @var \Symfony\Component\HttpKernel\HttpKernelInterface $kernel */ - $kernel = $this->container->get('http_kernel'); - - $this->drupalLogin($this->adminUser); - $request = Request::create('/test-role'); - $response = $kernel->handle($request); - $this->assertEqual($response->getStatusCode(), 200); - - $this->drupalLogin($this->webUser); - $request = Request::create('/test-role'); - $response = $kernel->handle($request); - $this->assertEqual($response->getStatusCode(), 403); - - $this->drupalLogin($this->normalUser); - $request = Request::create('/test-role'); - $response = $kernel->handle($request); - $this->assertEqual($response->getStatusCode(), 200); + $this->assertTrue($view->display_handler->access($this->adminUser), 'Admin-Account should be able to access the view everytime'); + $this->assertFalse($view->display_handler->access($this->webUser)); + $this->assertTrue($view->display_handler->access($this->normalUser)); } } diff --git a/core/modules/user/lib/Drupal/user/Tests/Views/AccessRoleUITest.php b/core/modules/user/lib/Drupal/user/Tests/Views/AccessRoleUITest.php deleted file mode 100644 index 3ae1dc6552..0000000000 --- a/core/modules/user/lib/Drupal/user/Tests/Views/AccessRoleUITest.php +++ /dev/null @@ -1,71 +0,0 @@ - 'User: Access role (UI)', - 'description' => 'Tests views role access plugin UI.', - 'group' => 'Views module integration', - ); - } - - /** - * {@inheritdoc} - */ - protected function setUp() { - parent::setUp(); - - ViewTestData::createTestViews(get_class($this), array('user_test_views')); - } - - /** - * Tests the role access plugin UI. - */ - public function testAccessRoleUI() { - $entity_manager = $this->container->get('entity.manager'); - $entity_manager->getStorageController('user_role')->create(array('id' => 'custom_role', 'label' => 'Custom role'))->save(); - $access_url = "admin/structure/views/nojs/display/test_access_role/default/access_options"; - $this->drupalPostForm($access_url, array('access_options[role][custom_role]' => 1), t('Apply')); - $this->assertResponse(200); - - $this->drupalPostForm(NULL, array(), t('Save')); - $view = $entity_manager->getStorageController('view')->load('test_access_role'); - - $display = $view->getDisplay('default'); - $this->assertEqual($display['display_options']['access']['options']['role'], array('custom_role' => 'custom_role')); - } - -} diff --git a/core/modules/user/lib/Drupal/user/Tests/Views/AccessTestBase.php b/core/modules/user/lib/Drupal/user/Tests/Views/AccessTestBase.php index 5fac787d68..4ec709d9bd 100644 --- a/core/modules/user/lib/Drupal/user/Tests/Views/AccessTestBase.php +++ b/core/modules/user/lib/Drupal/user/Tests/Views/AccessTestBase.php @@ -60,7 +60,6 @@ protected function setUp() { $this->normalRole = $this->drupalCreateRole(array()); $this->normalUser = $this->drupalCreateUser(array('views_test_data test permission')); $this->normalUser->addRole($this->normalRole); - $this->normalUser->save(); // @todo when all the plugin information is cached make a reset function and // call it here. } diff --git a/core/modules/user/tests/modules/user_test_views/test_views/views.view.test_access_role.yml b/core/modules/user/tests/modules/user_test_views/test_views/views.view.test_access_role.yml index f971f75de3..870b4bbe2e 100644 --- a/core/modules/user/tests/modules/user_test_views/test_views/views.view.test_access_role.yml +++ b/core/modules/user/tests/modules/user_test_views/test_views/views.view.test_access_role.yml @@ -1,16 +1,10 @@ -base_table: views_test_data +base_table: node core: '8' description: '' status: '1' display: default: display_options: - fields: - id: - id: id - field: id - table: views_test_data - plugin_id: numeric access: type: role cache: @@ -27,13 +21,6 @@ display: display_title: Master id: default position: '0' - page_1: - display_options: - path: test-role - display_plugin: page - display_title: Page - id: page_1 - position: '1' label: '' id: test_access_role tag: '' diff --git a/core/modules/views/lib/Drupal/views/Plugin/views/display/PathPluginBase.php b/core/modules/views/lib/Drupal/views/Plugin/views/display/PathPluginBase.php index 10cdd2ce70..01790c270a 100644 --- a/core/modules/views/lib/Drupal/views/Plugin/views/display/PathPluginBase.php +++ b/core/modules/views/lib/Drupal/views/Plugin/views/display/PathPluginBase.php @@ -185,10 +185,6 @@ protected function getRoute($view_id, $display_id) { $access_plugin = Views::pluginManager('access')->createInstance('none'); } $access_plugin->alterRouteDefinition($route); - // @todo Figure out whether _access_mode ANY is the proper one. This is - // particular important for altering routes. - $route->setOption('_access_mode', 'ANY'); - return $route; } diff --git a/core/modules/views/lib/Drupal/views/ViewsAccessCheck.php b/core/modules/views/lib/Drupal/views/ViewsAccessCheck.php index 8a81fb70ef..483f5821a1 100644 --- a/core/modules/views/lib/Drupal/views/ViewsAccessCheck.php +++ b/core/modules/views/lib/Drupal/views/ViewsAccessCheck.php @@ -7,7 +7,7 @@ namespace Drupal\views; -use Drupal\Core\Access\AccessCheckInterface; +use Drupal\Core\Access\StaticAccessCheckInterface; use Drupal\Core\Session\AccountInterface; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\Routing\Route; @@ -17,13 +17,13 @@ * * @todo We could leverage the permission one as well? */ -class ViewsAccessCheck implements AccessCheckInterface { +class ViewsAccessCheck implements StaticAccessCheckInterface { /** * {@inheritdoc} */ - public function applies(Route $route) { - return $route->hasDefault('view_id'); + public function appliesTo() { + return array('views_id'); } /** diff --git a/core/tests/Drupal/Tests/Core/Access/AccessManagerTest.php b/core/tests/Drupal/Tests/Core/Access/AccessManagerTest.php index 689cdf9446..740191c4a3 100644 --- a/core/tests/Drupal/Tests/Core/Access/AccessManagerTest.php +++ b/core/tests/Drupal/Tests/Core/Access/AccessManagerTest.php @@ -146,37 +146,6 @@ public function testSetChecks() { $this->assertEquals($this->routeCollection->get('test_route_3')->getOption('_access_checks'), array('test_access_default')); } - /** - * Tests setChecks with a dynamic access checker. - */ - public function testSetChecksWithDynamicAccessChecker() { - // Setup the access manager. - $this->accessManager = new AccessManager($this->routeProvider, $this->urlGenerator, $this->paramConverter, $this->account); - $this->accessManager->setContainer($this->container); - - // Setup the dynamic access checker. - $access_check = $this->getMock('Drupal\Core\Access\AccessCheckInterface'); - $this->container->set('test_access', $access_check); - $this->accessManager->addCheckService('test_access'); - - $route = new Route('/test-path', array(), array('_foo' => '1', '_bar' => '1')); - $route2 = new Route('/test-path', array(), array('_foo' => '1', '_bar' => '2')); - $collection = new RouteCollection(); - $collection->add('test_route', $route); - $collection->add('test_route2', $route2); - - $access_check->expects($this->exactly(2)) - ->method('applies') - ->with($this->isInstanceOf('Symfony\Component\Routing\Route')) - ->will($this->returnCallback(function (Route $route) { - return $route->getRequirement('_bar') == 2; - })); - - $this->accessManager->setChecks($collection); - $this->assertEmpty($route->getOption('_access_checks')); - $this->assertEquals(array('test_access'), $route2->getOption('_access_checks')); - } - /** * Tests \Drupal\Core\Access\AccessManager::check(). */ -- GitLab