Issue #2867703 by Berdir: Change #action in UserLoginBlock to a #lazy_builder...

Issue #2867703 by Berdir: Change #action in UserLoginBlock to a #lazy_builder to avoid the route cache context

core/modules/user/src/Plugin/Block/UserLoginBlock.php

@@ -6,7 +6,6 @@
 use Drupal\Core\Plugin\ContainerFactoryPluginInterface;
 use Drupal\Core\Routing\RedirectDestinationTrait;
 use Drupal\Core\Routing\RouteMatchInterface;
-use Drupal\Core\Routing\UrlGeneratorTrait;
 use Drupal\Core\Url;
 use Drupal\Core\Session\AccountInterface;
 use Drupal\Core\Block\BlockBase;
@@ -23,7 +22,6 @@
  */
 class UserLoginBlock extends BlockBase implements ContainerFactoryPluginInterface {
 
-  use UrlGeneratorTrait;
   use RedirectDestinationTrait;
 
   /**
@@ -94,7 +92,24 @@ public function build() {
     unset($form['pass']['#attributes']['aria-describedby']);
     $form['name']['#size'] = 15;
     $form['pass']['#size'] = 15;
-    $form['#action'] = $this->url('<current>', [], ['query' => $this->getDestinationArray(), 'external' => FALSE]);
+
+    // Instead of setting an actual action URL, we set the placeholder, which
+    // will be replaced at the very last moment. This ensures forms with
+    // dynamically generated action URLs don't have poor cacheability.
+    // Use the proper API to generate the placeholder, when we have one. See
+    // https://www.drupal.org/node/2562341. The placholder uses a fixed string
+    // that is
+    // Crypt::hashBase64('\Drupal\user\Plugin\Block\UserLoginBlock::build');
+    // This is based on the implementation in
+    // \Drupal\Core\Form\FormBuilder::prepareForm(), but the user login block
+    // requires different behavior for the destination query argument.
+    $placeholder = 'form_action_p_4r8ITd22yaUvXM6SzwrSe9rnQWe48hz9k1Sxto3pBvE';
+
+    $form['#attached']['placeholders'][$placeholder] = [
+      '#lazy_builder' => ['\Drupal\user\Plugin\Block\UserLoginBlock::renderPlaceholderFormAction', []],
+    ];
+    $form['#action'] = $placeholder;
+
     // Build action links.
     $items = [];
     if (\Drupal::config('user.settings')->get('register') != USER_REGISTER_ADMINISTRATORS_ONLY) {
@@ -128,4 +143,20 @@ public function build() {
     ];
   }
 
+  /**
+   * #lazy_builder callback; renders a form action URL including destination.
+   *
+   * @return array
+   *   A renderable array representing the form action.
+   *
+   * @see \Drupal\Core\Form\FormBuilder::renderPlaceholderFormAction()
+   */
+  public static function renderPlaceholderFormAction() {
+    return [
+      '#type' => 'markup',
+      '#markup' => Url::fromRoute('<current>', [], ['query' => \Drupal::destination()->getAsArray(), 'external' => FALSE])->toString(),
+      '#cache' => ['contexts' => ['url.path', 'url.query_args']],
+    ];
+  }
+
 }

core/modules/user/src/Tests/UserBlocksTest.php

@@ -2,6 +2,7 @@
 
 namespace Drupal\user\Tests;
 
+use Drupal\dynamic_page_cache\EventSubscriber\DynamicPageCacheSubscriber;
 use Drupal\simpletest\WebTestBase;
 
 /**
@@ -77,9 +78,29 @@ public function testUserLoginBlock() {
 
     // Now, log out and repeat with a non-403 page.
     $this->drupalLogout();
-    $this->drupalPostForm('filter/tips', $edit, t('Log in'));
+    $this->drupalGet('filter/tips');
+    $this->assertEqual('MISS', $this->drupalGetHeader(DynamicPageCacheSubscriber::HEADER));
+    $this->drupalPostForm(NULL, $edit, t('Log in'));
+    $this->assertNoText(t('User login'), 'Logged in.');
+    $this->assertPattern('!<title.*?' . t('Compose tips') . '.*?</title>!', 'Still on the same page after login for allowed page');
+
+    // Log out again and repeat with a non-403 page including query arguments.
+    $this->drupalLogout();
+    $this->drupalGet('filter/tips', ['query' => ['foo' => 'bar']]);
+    $this->assertEqual('HIT', $this->drupalGetHeader(DynamicPageCacheSubscriber::HEADER));
+    $this->drupalPostForm(NULL, $edit, t('Log in'));
+    $this->assertNoText(t('User login'), 'Logged in.');
+    $this->assertPattern('!<title.*?' . t('Compose tips') . '.*?</title>!', 'Still on the same page after login for allowed page');
+    $this->assertTrue(strpos($this->getUrl(), '/filter/tips?foo=bar') !== FALSE, 'Correct query arguments are displayed after login');
+
+    // Repeat with different query arguments.
+    $this->drupalLogout();
+    $this->drupalGet('filter/tips', ['query' => ['foo' => 'baz']]);
+    $this->assertEqual('HIT', $this->drupalGetHeader(DynamicPageCacheSubscriber::HEADER));
+    $this->drupalPostForm(NULL, $edit, t('Log in'));
     $this->assertNoText(t('User login'), 'Logged in.');
     $this->assertPattern('!<title.*?' . t('Compose tips') . '.*?</title>!', 'Still on the same page after login for allowed page');
+    $this->assertTrue(strpos($this->getUrl(), '/filter/tips?foo=baz') !== FALSE, 'Correct query arguments are displayed after login');
 
     // Check that the user login block is not vulnerable to information
     // disclosure to third party sites.