diff --git a/core/lib/Drupal/Core/Form/FormBuilder.php b/core/lib/Drupal/Core/Form/FormBuilder.php index 37bebe8..d966801 100644 --- a/core/lib/Drupal/Core/Form/FormBuilder.php +++ b/core/lib/Drupal/Core/Form/FormBuilder.php @@ -832,11 +832,9 @@ public function doBuildForm($form_id, &$element, FormStateInterface &$form_state $count = 0; if (isset($element['#access'])) { $access = $element['#access']; - if ($access instanceof AccessResultInterface || $access === FALSE) { - $child_access = $access; - } - else { - $child_access = NULL; + $inherited_access = NULL; + if (($access instanceof AccessResultInterface && !$access->isAllowed()) || $access === FALSE) { + $inherited_access = $access; } } foreach (Element::children($element) as $key) { @@ -853,8 +851,8 @@ public function doBuildForm($form_id, &$element, FormStateInterface &$form_state } // Children inherit #access from parent. - if (isset($child_access)) { - $element[$key]['#access'] = $child_access; + if (isset($inherited_access)) { + $element[$key]['#access'] = $inherited_access; } // Make child elements inherit their parent's #disabled and #allow_focus diff --git a/core/lib/Drupal/Core/Render/Renderer.php b/core/lib/Drupal/Core/Render/Renderer.php index 5d2f2e6..10ec8a5 100644 --- a/core/lib/Drupal/Core/Render/Renderer.php +++ b/core/lib/Drupal/Core/Render/Renderer.php @@ -190,7 +190,7 @@ protected function doRender(&$elements, $is_root_call = FALSE) { return ''; } } - elseif (!$elements['#access']) { + elseif ($elements['#access'] === FALSE) { return ''; } } diff --git a/core/tests/Drupal/Tests/Core/Form/FormBuilderTest.php b/core/tests/Drupal/Tests/Core/Form/FormBuilderTest.php index 6044570..05904ad 100644 --- a/core/tests/Drupal/Tests/Core/Form/FormBuilderTest.php +++ b/core/tests/Drupal/Tests/Core/Form/FormBuilderTest.php @@ -7,6 +7,9 @@ namespace Drupal\Tests\Core\Form; +use Drupal\Component\Utility\NestedArray; +use Drupal\Core\Access\AccessResult; +use Drupal\Core\Access\AccessResultForbidden; use Drupal\Core\DependencyInjection\ContainerInjectionInterface; use Drupal\Core\Form\EnforcedResponseException; use Drupal\Core\Form\FormBuilderInterface; @@ -449,6 +452,167 @@ public function testExceededFileSize() { $this->formBuilder->buildForm($form_arg, $form_state); } + /** + * @covers ::buildFOrm + * + * @dataProvider providerTestChildAccessInheritance + */ + public function testChildAccessInheritance($element, $access_checks) { + $form_arg = new TestFormWithPredefinedForm(); + $form_arg->setForm($element); + + $form_state = new FormState(); + + $form = $this->formBuilder->buildForm($form_arg, $form_state); + + + $actual_access_structure = []; + $expected_access_structure = []; + + // Ensure that the expected access checks are set. + foreach ($access_checks as $access_check) { + $parents = $access_check[0]; + $parents[] = '#access'; + + $actual_access = NestedArray::getValue($form, $parents); + $actual_access_structure[] = [$parents, $actual_access]; + $expected_access_structure[] = [$parents, $access_check[1]]; + } + + $this->assertEquals($expected_access_structure, $actual_access_structure); + } + + public function providerTestChildAccessInheritance() { + $data = []; + + $element = [ + 'child0' => [ + '#type' => 'checkbox', + ], + 'child1' => [ + '#type' => 'checkbox', + ], + 'child2' => [ + '#type' => 'fieldset', + 'child2.0' => [ + '#type' => 'checkbox', + ], + 'child2.1' => [ + '#type' => 'checkbox', + ], + 'child2.2' => [ + '#type' => 'checkbox', + ], + ], + ]; + + // Sets access FALSE on the root level, this should be inherited completely. + $clone = $element; + $clone['#access'] = FALSE; + + $expected_access = []; + $expected_access[] = [[], FALSE]; + $expected_access[] = [['child0'], FALSE]; + $expected_access[] = [['child1'], FALSE]; + $expected_access[] = [['child2'], FALSE]; + $expected_access[] = [['child2', 'child2.0'], FALSE]; + $expected_access[] = [['child2', 'child2.1'], FALSE]; + $expected_access[] = [['child2', 'child2.2'], FALSE]; + + $data['access-false-root'] = [$clone, $expected_access]; + + $clone = $element; + $access_result = AccessResult::forbidden()->addCacheContexts(['user']); + $clone['#access'] = $access_result; + + $expected_access = []; + $expected_access[] = [[], $access_result]; + $expected_access[] = [['child0'], $access_result]; + $expected_access[] = [['child1'], $access_result]; + $expected_access[] = [['child2'], $access_result]; + $expected_access[] = [['child2', 'child2.0'], $access_result]; + $expected_access[] = [['child2', 'child2.1'], $access_result]; + $expected_access[] = [['child2', 'child2.2'], $access_result]; + + $data['access-forbidden-root'] = [$clone, $expected_access]; + + // Allow access on the most outer level but set FALSE otherwise. + $clone = $element; + $clone['#access'] = TRUE; + $clone['child0']['#access'] = FALSE; + + $expected_access = []; + $expected_access[] = [[], TRUE]; + $expected_access[] = [['child0'], FALSE]; + $expected_access[] = [['child1'], NULL]; + $expected_access[] = [['child2'], NULL]; + $expected_access[] = [['child2', 'child2.0'], NULL]; + $expected_access[] = [['child2', 'child2.1'], NULL]; + $expected_access[] = [['child2', 'child2.2'], NULL]; + + $data['access-true-root'] = [$clone, $expected_access]; + + // Allow access on the most outer level but forbid otherwise. + $clone = $element; + $access_result_allowed = AccessResult::allowed() + ->addCacheContexts(['user']); + $clone['#access'] = $access_result_allowed; + $access_result_forbidden = AccessResult::forbidden() + ->addCacheContexts(['user']); + $clone['child0']['#access'] = $access_result_forbidden; + + $expected_access = []; + $expected_access[] = [[], $access_result_allowed]; + $expected_access[] = [['child0'], $access_result_forbidden]; + $expected_access[] = [['child1'], NULL]; + $expected_access[] = [['child2'], NULL]; + $expected_access[] = [['child2', 'child2.0'], NULL]; + $expected_access[] = [['child2', 'child2.1'], NULL]; + $expected_access[] = [['child2', 'child2.2'], NULL]; + + $data['access-allowed-root'] = [$clone, $expected_access]; + + // Allow access on the most outer level, deny access on a parent, and allow + // on a child. The denying should be inherited. + $clone = $element; + $clone['#access'] = TRUE; + $clone['child2']['#access'] = FALSE; + $clone['child2.0']['#access'] = TRUE; + $clone['child2.1']['#access'] = TRUE; + $clone['child2.2']['#access'] = TRUE; + + $expected_access = []; + $expected_access[] = [[], TRUE]; + $expected_access[] = [['child0'], NULL]; + $expected_access[] = [['child1'], NULL]; + $expected_access[] = [['child2'], FALSE]; + $expected_access[] = [['child2', 'child2.0'], FALSE]; + $expected_access[] = [['child2', 'child2.1'], FALSE]; + $expected_access[] = [['child2', 'child2.2'], FALSE]; + + $data['access-mixed-parents'] = [$clone, $expected_access]; + + $clone = $element; + $clone['#access'] = $access_result_allowed; + $clone['child2']['#access'] = $access_result_forbidden; + $clone['child2.0']['#access'] = $access_result_allowed; + $clone['child2.1']['#access'] = $access_result_allowed; + $clone['child2.2']['#access'] = $access_result_allowed; + + $expected_access = []; + $expected_access[] = [[], $access_result_allowed]; + $expected_access[] = [['child0'], NULL]; + $expected_access[] = [['child1'], NULL]; + $expected_access[] = [['child2'], $access_result_forbidden]; + $expected_access[] = [['child2', 'child2.0'], $access_result_forbidden]; + $expected_access[] = [['child2', 'child2.1'], $access_result_forbidden]; + $expected_access[] = [['child2', 'child2.2'], $access_result_forbidden]; + + $data['access-mixed-parents-object'] = [$clone, $expected_access]; + + return $data; + } + } class TestForm implements FormInterface { @@ -467,3 +631,21 @@ public static function create(ContainerInterface $container) { return new static(); } } + + +class TestFormWithPredefinedForm extends TestForm { + + /** + * @var array + */ + protected $form; + + public function setForm($form) { + $this->form = $form; + } + + public function buildForm(array $form, FormStateInterface $form_state) { + return $this->form; + } + +} diff --git a/core/tests/Drupal/Tests/Core/Render/RendererTest.php b/core/tests/Drupal/Tests/Core/Render/RendererTest.php index 3e3e3bd..af4c352 100644 --- a/core/tests/Drupal/Tests/Core/Render/RendererTest.php +++ b/core/tests/Drupal/Tests/Core/Render/RendererTest.php @@ -7,6 +7,8 @@ namespace Drupal\Tests\Core\Render; +use Drupal\Core\Access\AccessResult; +use Drupal\Core\Access\AccessResultInterface; use Drupal\Core\Cache\Cache; use Drupal\Core\Cache\CacheableDependencyInterface; use Drupal\Core\Render\Element; @@ -419,14 +421,44 @@ public function testRenderWithAccessPropertyAndCallback($access) { * @dataProvider providerBoolean */ public function testRenderWithAccessControllerResolved($access) { + + switch ($access) { + case AccessResult::allowed(): + $method = 'accessResultAllowed'; + break; + case AccessResult::forbidden(): + $method = 'accessResultForbidden'; + break; + case FALSE: + $method = 'accessFalse'; + break; + case TRUE: + $method = 'accessTrue'; + break; + } + $build = [ - '#access_callback' => 'Drupal\Tests\Core\Render\TestAccessClass::' . ($access ? 'accessTrue' : 'accessFalse'), + '#access_callback' => 'Drupal\Tests\Core\Render\TestAccessClass::' . $method, ]; $this->assertAccess($build, $access); } /** + * @covers ::render + * @covers ::doRender + */ + public function testRenderAccessCacheablityDependencyInheritance() { + $build = [ + '#access' => AccessResult::allowed()->addCacheContexts(['user']), + ]; + + $this->renderer->renderPlain($build); + + $this->assertEquals(['languages:language_interface', 'theme', 'user'], $build['#cache']['contexts']); + } + + /** * Tests that a first render returns the rendered output and a second doesn't. * * (Because of the #printed property.) @@ -454,7 +486,9 @@ public function testRenderTwice() { public function providerBoolean() { return [ [FALSE], - [TRUE] + [TRUE], + [AccessResult::forbidden()], + [AccessResult::allowed()], ]; } @@ -469,7 +503,7 @@ public function providerBoolean() { protected function assertAccess($build, $access) { $sensitive_content = $this->randomContextValue(); $build['#markup'] = $sensitive_content; - if ($access) { + if (($access instanceof AccessResultInterface && $access->isAllowed()) || $access === TRUE) { $this->assertSame($sensitive_content, $this->renderer->renderRoot($build)); } else { @@ -784,6 +818,14 @@ public static function accessFalse() { return FALSE; } + public static function accessResultAllowed() { + return AccessResult::allowed(); + } + + public static function accessResultForbidden() { + return AccessResult::forbidden(); + } + } class TestCallables {