core/lib/Drupal/Core/Form/FormBuilder.php | 3 -- core/modules/contact/src/MessageForm.php | 3 ++ .../src/Tests/ContactAuthenticatedUserTest.php | 14 ++++++- .../Drupal/Tests/Core/Form/FormBuilderTest.php | 45 ++++++++++++++-------- 4 files changed, 44 insertions(+), 21 deletions(-) diff --git a/core/lib/Drupal/Core/Form/FormBuilder.php b/core/lib/Drupal/Core/Form/FormBuilder.php index 721ee92..3dac78b 100644 --- a/core/lib/Drupal/Core/Form/FormBuilder.php +++ b/core/lib/Drupal/Core/Form/FormBuilder.php @@ -734,9 +734,6 @@ public function prepareForm($form_id, &$form, FormStateInterface &$form_state) { // submitted form value appears literally, regardless of custom #tree // and #parents being set elsewhere. '#parents' => array('form_token'), - '#cache' => [ - 'max-age' => 0, - ], ); } } diff --git a/core/modules/contact/src/MessageForm.php b/core/modules/contact/src/MessageForm.php index 9cb5e54..c576a56 100644 --- a/core/modules/contact/src/MessageForm.php +++ b/core/modules/contact/src/MessageForm.php @@ -120,6 +120,7 @@ public function form(array $form, FormStateInterface $form_state) { '#title' => $this->t('Your email address'), '#required' => TRUE, ); + $form['#cache']['contexts'][] = 'user.roles:authenticated'; if ($user->isAnonymous()) { $form['#attached']['library'][] = 'core/drupal.form'; $form['#attributes']['data-user-info-from-browser'] = TRUE; @@ -131,11 +132,13 @@ public function form(array $form, FormStateInterface $form_state) { $form['name']['#value'] = $user->getUsername(); $form['name']['#required'] = FALSE; $form['name']['#plain_text'] = $user->getUsername(); + $form['name']['#cache']['contexts'][] = 'user'; $form['mail']['#type'] = 'item'; $form['mail']['#value'] = $user->getEmail(); $form['mail']['#required'] = FALSE; $form['mail']['#plain_text'] = $user->getEmail(); + $form['mail']['#cache']['contexts'][] = 'user'; } // The user contact form has a preset recipient. diff --git a/core/modules/contact/src/Tests/ContactAuthenticatedUserTest.php b/core/modules/contact/src/Tests/ContactAuthenticatedUserTest.php index 2adc4c2..923878e 100644 --- a/core/modules/contact/src/Tests/ContactAuthenticatedUserTest.php +++ b/core/modules/contact/src/Tests/ContactAuthenticatedUserTest.php @@ -21,19 +21,29 @@ class ContactAuthenticatedUserTest extends WebTestBase { * * @var array */ - public static $modules = array('contact'); + public static $modules = array('contact', 'contact_test'); /** * Tests that name and email fields are not present for authenticated users. */ function testContactSiteWideTextfieldsLoggedInTestCase() { - $this->drupalLogin($this->drupalCreateUser(array('access site-wide contact form'))); + $user1 = $this->drupalCreateUser(array('access site-wide contact form')); + $this->drupalLogin($user1); $this->drupalGet('contact'); + $this->assertResponse(200); + $this->assertCacheContext('user'); // Ensure that there is no textfield for name. $this->assertFalse($this->xpath('//input[@name=:name]', array(':name' => 'name'))); + $this->assertRaw($user1->getAccountName()); // Ensure that there is no textfield for email. $this->assertFalse($this->xpath('//input[@name=:name]', array(':name' => 'mail'))); + + // Log in as a different user and confirm that + $user2 = $this->drupalCreateUser(array('access site-wide contact form')); + $this->drupalLogin($user2); + $this->drupalGet('contact'); + $this->assertRaw($user2->getAccountName()); } } diff --git a/core/tests/Drupal/Tests/Core/Form/FormBuilderTest.php b/core/tests/Drupal/Tests/Core/Form/FormBuilderTest.php index e5f2963..f37a4c7 100644 --- a/core/tests/Drupal/Tests/Core/Form/FormBuilderTest.php +++ b/core/tests/Drupal/Tests/Core/Form/FormBuilderTest.php @@ -820,7 +820,7 @@ public function providerTestInvalidToken() { * * @dataProvider providerTestFormTokenCacheability */ - public function testFormTokenCacheability($token, $is_authenticated, $expected_form_cacheability, $expected_token_cacheability, $method) { + public function testFormTokenCacheability($token, $is_authenticated, $method) { $user = $this->prophesize(AccountProxyInterface::class); $user->isAuthenticated() ->willReturn($is_authenticated); @@ -845,19 +845,32 @@ public function testFormTokenCacheability($token, $is_authenticated, $expected_f $form_state = new FormState(); $built_form = $this->formBuilder->buildForm($form_arg, $form_state); - if (!isset($expected_form_cacheability) || ($method == 'get' && !is_string($token))) { + + // FormBuilder does not even consider to set a form token when: + // - #token = FALSE (opting out explicitly) + // - #method = GET and #token is not set to a string (GET forms don't get a + // form token by default, and this form did not explicitly opt in) + if ($token === FALSE || ($method == 'get' && !is_string($token))) { $this->assertFalse(isset($built_form['#cache'])); - } - else { - $this->assertTrue(isset($built_form['#cache'])); - $this->assertEquals($expected_form_cacheability, $built_form['#cache']); - } - if (!isset($expected_token_cacheability)) { $this->assertFalse(isset($built_form['form_token'])); } + // Otherwise, a form token is set, but only if the user is logged in. It is + // impossible (and unnecessary) to set a form token if the user is not + // logged in, because there is no session, and hence no CSRF token. else { - $this->assertTrue(isset($built_form['form_token'])); - $this->assertEquals($expected_token_cacheability, $built_form['form_token']['#cache']); + // For forms that are eligible for form tokens, a cache context must be + // set that indicates the form token only exists for logged in users. + $this->assertTrue(isset($built_form['#cache'])); + $this->assertEquals(['contexts' => ['user.roles:authenticated']], $built_form['#cache']); + // Finally, verify that a form token is generated when appropriate, with + // the expected cacheability metadata (or lack thereof). + if (!$is_authenticated) { + $this->assertFalse(isset($built_form['form_token'])); + } + else { + $this->assertTrue(isset($built_form['form_token'])); + $this->assertFalse(isset($built_form['form_token']['#cache'])); + } } } @@ -868,12 +881,12 @@ public function testFormTokenCacheability($token, $is_authenticated, $expected_f */ function providerTestFormTokenCacheability() { return [ - 'token:none,authenticated:true' => [NULL, TRUE, ['contexts' => ['user.roles:authenticated']], ['max-age' => 0], 'post'], - 'token:none,authenticated:false' => [NULL, FALSE, ['contexts' => ['user.roles:authenticated']], NULL, 'post'], - 'token:false,authenticated:false' => [FALSE, FALSE, NULL, NULL, 'post'], - 'token:false,authenticated:true' => [FALSE, TRUE, NULL, NULL, 'post'], - 'token:none,authenticated:false,method:get' => [NULL, FALSE, ['contexts' => ['user.roles:authenticated']], NULL, 'get'], - 'token:test_form_id,authenticated:false,method:get' => ['test_form_id', TRUE, ['contexts' => ['user.roles:authenticated']], ['max-age' => 0], 'get'], + 'token:none,authenticated:true' => [NULL, TRUE, 'post'], + 'token:none,authenticated:false' => [NULL, FALSE, 'post'], + 'token:false,authenticated:false' => [FALSE, FALSE, 'post'], + 'token:false,authenticated:true' => [FALSE, TRUE, 'post'], + 'token:none,authenticated:false,method:get' => [NULL, FALSE, 'get'], + 'token:test_form_id,authenticated:true,method:get' => ['test_form_id', TRUE, 'get'], ]; }