diff --git a/core/lib/Drupal/Core/Entity/Entity/EntityFormDisplay.php b/core/lib/Drupal/Core/Entity/Entity/EntityFormDisplay.php index 8c591c9a4179d2063ee2048b55a617a03c24a241..c292c53ff6b2bf699b05dc9977be8fad5c81d62b 100644 --- a/core/lib/Drupal/Core/Entity/Entity/EntityFormDisplay.php +++ b/core/lib/Drupal/Core/Entity/Entity/EntityFormDisplay.php @@ -206,6 +206,11 @@ public function buildForm(FieldableEntityInterface $entity, array &$form, FormSt // Associate the cache tags for the form display. $this->renderer->addCacheableDependency($form, $this); + // The form might not have the correct cacheability metadata, so make it + // uncacheable by default. + // @todo Remove this in https://www.drupal.org/node/3395524. + $form['#cache']['max-age'] = 0; + // Add a process callback so we can assign weights and hide extra fields. $form['#process'][] = [$this, 'processForm']; } diff --git a/core/lib/Drupal/Core/Form/FormBuilder.php b/core/lib/Drupal/Core/Form/FormBuilder.php index f31346a378772fe3a64b4cd5e97e2844efa159e0..be0a22f456f8a9db363f5743bf3caf18572faded 100644 --- a/core/lib/Drupal/Core/Form/FormBuilder.php +++ b/core/lib/Drupal/Core/Form/FormBuilder.php @@ -770,10 +770,17 @@ public function prepareForm($form_id, &$form, FormStateInterface &$form_state) { ], ], ], - '#cache' => [ - 'max-age' => 0, - ], ]; + + // If a form hasn't explicitly opted in to caching by setting max-age at + // the top level, then make it uncacheable in case it doesn't have the + // correct cacheability metadata. + // @todo Remove this in the next major version, after the deprecation + // process from https://www.drupal.org/project/drupal/issues/3395157 + // has ended. + if (!isset($form['#cache']['max-age'])) { + $form['form_token']['#cache']['max-age'] = 0; + } } } diff --git a/core/modules/contact/src/Controller/ContactController.php b/core/modules/contact/src/Controller/ContactController.php index 709ae33245d444b88be3966fa5eff0a9b42b63cf..ac512b448581171e971380907cd94d01a782af0e 100644 --- a/core/modules/contact/src/Controller/ContactController.php +++ b/core/modules/contact/src/Controller/ContactController.php @@ -81,6 +81,12 @@ public function contactSitePage(?ContactFormInterface $contact_form = NULL) { $form['#title'] = $contact_form->label(); $form['#cache']['contexts'][] = 'user.permissions'; $this->renderer->addCacheableDependency($form, $config); + + // The form might not have the correct cacheability metadata, so make it + // uncacheable by default. + // @todo Remove this in https://www.drupal.org/node/3395506. + $form['#cache']['max-age'] = 0; + return $form; } diff --git a/core/modules/node/src/NodeForm.php b/core/modules/node/src/NodeForm.php index 31c97b3cb4c8ea889e4350fa2c261bca5c90a205..c3893136360e925a44d1e673c83d5c7eb45a199f 100644 --- a/core/modules/node/src/NodeForm.php +++ b/core/modules/node/src/NodeForm.php @@ -92,6 +92,10 @@ public function form(array $form, FormStateInterface $form_state) { // parent::form(). $store = $this->tempStoreFactory->get('node_preview'); + // Because of the temp store integration, this is not cacheable. + // @todo add the correct cache contexts in https://www.drupal.org/project/drupal/issues/3397987 + $form['#cache']['max-age'] = 0; + // Attempt to load from preview when the uuid is present unless we are // rebuilding the form. $request_uuid = \Drupal::request()->query->get('uuid'); diff --git a/core/tests/Drupal/Tests/Core/Form/FormBuilderTest.php b/core/tests/Drupal/Tests/Core/Form/FormBuilderTest.php index 3137681d18e664161ae4f12f88b906e1922b61a2..7b99fc5a2c12c1a4b1dd3a3c09b1b1060deb568f 100644 --- a/core/tests/Drupal/Tests/Core/Form/FormBuilderTest.php +++ b/core/tests/Drupal/Tests/Core/Form/FormBuilderTest.php @@ -7,6 +7,7 @@ use Drupal\Component\Utility\Html; use Drupal\Component\Utility\NestedArray; use Drupal\Core\Access\AccessResult; +use Drupal\Core\Cache\Cache; use Drupal\Core\Cache\Context\CacheContextsManager; use Drupal\Core\DependencyInjection\ContainerInjectionInterface; use Drupal\Core\Form\EnforcedResponseException; @@ -909,7 +910,7 @@ public static function providerTestInvalidToken() { * * @dataProvider providerTestFormTokenCacheability */ - public function testFormTokenCacheability($token, $is_authenticated, $expected_form_cacheability, $expected_token_cacheability, $method): void { + public function testFormTokenCacheability($token, $is_authenticated, $method, $opted_in_for_cache): void { $user = $this->prophesize(AccountProxyInterface::class); $user->isAuthenticated() ->willReturn($is_authenticated); @@ -924,6 +925,10 @@ public function testFormTokenCacheability($token, $is_authenticated, $expected_f $form['#token'] = $token; } + if ($opted_in_for_cache) { + $form['#cache']['max-age'] = Cache::PERMANENT; + } + $form_arg = $this->createMock('Drupal\Core\Form\FormInterface'); $form_arg->expects($this->once()) ->method('getFormId') @@ -934,19 +939,45 @@ 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 set a form token when: + // - #token is set to FALSE. + // - #method is set to 'GET' and #token is not a string. This means the GET + // form did not get a form token by default, and the form did not + // explicitly opt in. + if ($token === FALSE || ($method == 'get' && !is_string($token))) { $this->assertEquals($built_form['#cache'], ['tags' => ['CACHE_MISS_IF_UNCACHEABLE_HTTP_METHOD:form']]); - } - 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'])); + $expected_cacheability_metadata = [ + 'tags' => ['CACHE_MISS_IF_UNCACHEABLE_HTTP_METHOD:form'], + 'contexts' => ['user.roles:authenticated'], + ]; + if ($opted_in_for_cache) { + $expected_cacheability_metadata['max-age'] = Cache::PERMANENT; + } + $this->assertEquals($expected_cacheability_metadata, $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'])); + if ($opted_in_for_cache) { + $this->assertFalse(isset($built_form['form_token']['#cache'])); + } + else { + $this->assertEquals(['max-age' => 0], $built_form['form_token']['#cache']); + } + } } } @@ -957,12 +988,13 @@ public function testFormTokenCacheability($token, $is_authenticated, $expected_f */ public static function providerTestFormTokenCacheability() { return [ - 'token:none,authenticated:true' => [NULL, TRUE, ['contexts' => ['user.roles:authenticated'], 'tags' => ['CACHE_MISS_IF_UNCACHEABLE_HTTP_METHOD:form']], ['max-age' => 0], 'post'], - 'token:none,authenticated:false' => [NULL, FALSE, ['contexts' => ['user.roles:authenticated'], 'tags' => ['CACHE_MISS_IF_UNCACHEABLE_HTTP_METHOD:form']], 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'], 'tags' => ['CACHE_MISS_IF_UNCACHEABLE_HTTP_METHOD:form']], NULL, 'get'], - 'token:test_form_id,authenticated:false,method:get' => ['test_form_id', TRUE, ['contexts' => ['user.roles:authenticated'], 'tags' => ['CACHE_MISS_IF_UNCACHEABLE_HTTP_METHOD:form']], ['max-age' => 0], 'get'], + 'token:none,authenticated:true' => [NULL, TRUE, 'post', FALSE], + 'token:none,authenticated:true,opted_in_for_cache' => [NULL, TRUE, 'post', TRUE], + 'token:none,authenticated:false' => [NULL, FALSE, 'post', FALSE], + 'token:false,authenticated:false' => [FALSE, FALSE, 'post', FALSE], + 'token:false,authenticated:true' => [FALSE, TRUE, 'post', FALSE], + 'token:none,authenticated:false,method:get' => [NULL, FALSE, 'get', FALSE], + 'token:test_form_id,authenticated:false,method:get' => ['test_form_id', TRUE, 'get', FALSE], ]; }