Issue #2678822 by DamienMcKenna, David_Rothstein, stefan.r, Berdir: Drupal...

Issue #2678822 by DamienMcKenna, David_Rothstein, stefan.r, Berdir: Drupal 7.43 / 8.0.4 regression: When an anonymous user submits a form with an un-uploaded file that leads to a validation error, the file is lost on the next correct submission

core/modules/file/src/Element/ManagedFile.php

@@ -2,12 +2,14 @@
 
 namespace Drupal\file\Element;
 
-use Drupal\Component\Utility\NestedArray;
+use Drupal\Component\Utility\Crypt;
 use Drupal\Component\Utility\Html;
+use Drupal\Component\Utility\NestedArray;
 use Drupal\Core\Ajax\AjaxResponse;
 use Drupal\Core\Ajax\ReplaceCommand;
 use Drupal\Core\Form\FormStateInterface;
 use Drupal\Core\Render\Element\FormElement;
+use Drupal\Core\Site\Settings;
 use Drupal\Core\Url;
 use Drupal\file\Entity\File;
 use Symfony\Component\HttpFoundation\Request;
@@ -91,19 +93,32 @@ public static function valueCallback(&$element, $input, FormStateInterface $form
           $fids = [];
           foreach ($input['fids'] as $fid) {
             if ($file = File::load($fid)) {
+              $fids[] = $file->id();
               // Temporary files that belong to other users should never be
-              // allowed. Since file ownership can't be determined for anonymous
-              // users, they are not allowed to reuse temporary files at all.
-              if ($file->isTemporary() && (\Drupal::currentUser()->isAnonymous() || $file->getOwnerId() != \Drupal::currentUser()->id()))  {
-                $force_default = TRUE;
-                break;
-              }
-              // If all checks pass, allow the files to be changed.
-              else {
-                $fids[] = $file->id();
+              // allowed.
+              if ($file->isTemporary()) {
+                if ($file->getOwnerId() != \Drupal::currentUser()->id())  {
+                  $force_default = TRUE;
+                  break;
+                }
+                // Since file ownership can't be determined for anonymous users,
+                // they are not allowed to reuse temporary files at all. But
+                // they do need to be able to reuse their own files from earlier
+                // submissions of the same form, so to allow that, check for the
+                // token added by $this->processManagedFile().
+                elseif (\Drupal::currentUser()->isAnonymous()) {
+                  $token = NestedArray::getValue($form_state->getUserInput(), array_merge($element['#parents'], array('file_' . $file->id(), 'fid_token')));
+                  if ($token !== Crypt::hmacBase64('file-' . $file->id(), \Drupal::service('private_key')->get() . Settings::getHashSalt())) {
+                    $force_default = TRUE;
+                    break;
+                  }
+                }
               }
             }
           }
+          if ($force_default) {
+            $fids = [];
+          }
         }
       }
     }
@@ -309,6 +324,17 @@ public static function processManagedFile(&$element, FormStateInterface $form_st
         else {
           $element['file_' . $delta]['filename'] = $file_link + ['#weight' => -10];
         }
+        // Anonymous users who have uploaded a temporary file need a
+        // non-session-based token added so $this->valueCallback() can check
+        // that they have permission to use this file on subsequent submissions
+        // of the same form (for example, after an Ajax upload or form
+        // validation error).
+        if ($file->isTemporary() && \Drupal::currentUser()->isAnonymous()) {
+          $element['file_' . $delta]['fid_token'] = array(
+            '#type' => 'hidden',
+            '#value' => Crypt::hmacBase64('file-' . $delta, \Drupal::service('private_key')->get() . Settings::getHashSalt()),
+          );
+        }
       }
     }
 

core/modules/file/src/Tests/FileFieldAnonymousSubmissionTest.php

+<?php
+
+namespace Drupal\file\Tests;
+
+use Drupal\node\Entity\Node;
+use Drupal\user\RoleInterface;
+use Drupal\file\Entity\File;
+
+/**
+ * Confirm that file field submissions work correctly for anonymous visitors.
+ *
+ * @group file
+ */
+class FileFieldAnonymousSubmissionTest extends FileFieldTestBase {
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function setUp() {
+    parent::setUp();
+    // Set up permissions for anonymous attacker user.
+    user_role_change_permissions(RoleInterface::ANONYMOUS_ID, array(
+      'create article content' => TRUE,
+      'access content' => TRUE,
+    ));
+  }
+
+  /**
+   * Tests the basic node submission for an anonymous visitor.
+   */
+  public function testAnonymousNode() {
+    $bundle_label = 'Article';
+    $node_title = 'test page';
+
+    // Load the node form.
+    $this->drupalLogout();
+    $this->drupalGet('node/add/article');
+    $this->assertResponse(200, 'Loaded the article node form.');
+    $this->assertText(strip_tags(t('Create @name', array('@name' => $bundle_label))));
+
+    $edit = array(
+      'title[0][value]' => $node_title,
+      'body[0][value]' => 'Test article',
+    );
+    $this->drupalPostForm(NULL, $edit, 'Save');
+    $this->assertResponse(200);
+    $t_args = array('@type' => $bundle_label, '%title' => $node_title);
+    $this->assertText(strip_tags(t('@type %title has been created.', $t_args)), 'The node was created.');
+    $matches = array();
+    if (preg_match('@node/(\d+)$@', $this->getUrl(), $matches)) {
+      $nid = end($matches);
+      $this->assertNotEqual($nid, 0, 'The node ID was extracted from the URL.');
+      $node = Node::load($nid);
+      $this->assertNotEqual($node, NULL, 'The node was loaded successfully.');
+    }
+  }
+
+  /**
+   * Tests file submission for an anonymous visitor.
+   */
+  public function testAnonymousNodeWithFile() {
+    $bundle_label = 'Article';
+    $node_title = 'Test page';
+    $this->createFileField('field_image', 'node', 'article', array(), array('file_extensions' => 'txt png'));
+
+    // Load the node form.
+    $this->drupalLogout();
+    $this->drupalGet('node/add/article');
+    $this->assertResponse(200, 'Loaded the article node form.');
+    $this->assertText(strip_tags(t('Create @name', array('@name' => $bundle_label))));
+
+    // Generate an image file.
+    $image = $this->getTestFile('image');
+
+    // Submit the form.
+    $edit = array(
+      'title[0][value]' => $node_title,
+      'body[0][value]' => 'Test article',
+      'files[field_image_0]' => $this->container->get('file_system')->realpath($image->getFileUri()),
+    );
+    $this->drupalPostForm(NULL, $edit, 'Save');
+    $this->assertResponse(200);
+    $t_args = array('@type' => $bundle_label, '%title' => $node_title);
+    $this->assertText(strip_tags(t('@type %title has been created.', $t_args)), 'The node was created.');
+    $matches = array();
+    if (preg_match('@node/(\d+)$@', $this->getUrl(), $matches)) {
+      $nid = end($matches);
+      $this->assertNotEqual($nid, 0, 'The node ID was extracted from the URL.');
+      $node = Node::load($nid);
+      $this->assertNotEqual($node, NULL, 'The node was loaded successfully.');
+      $this->assertFileExists(File::load($node->field_image->target_id), 'The image was uploaded successfully.');
+    }
+  }
+
+  /**
+   * Tests file submission for an anonymous visitor with a missing node title.
+   */
+  public function testAnonymousNodeWithFileWithoutTitle() {
+    $this->drupalLogout();
+    $this->doTestNodeWithFileWithoutTitle();
+  }
+
+  /**
+   * Tests file submission for an authenticated user with a missing node title.
+   */
+  public function testAuthenticatedNodeWithFileWithoutTitle() {
+    $admin_user = $this->drupalCreateUser(array(
+      'bypass node access',
+      'access content overview',
+      'administer nodes',
+    ));
+    $this->drupalLogin($admin_user);
+    $this->doTestNodeWithFileWithoutTitle();
+  }
+
+  /**
+   * Helper method to test file submissions with missing node titles.
+   */
+  protected function doTestNodeWithFileWithoutTitle() {
+    $bundle_label = 'Article';
+    $node_title = 'Test page';
+    $this->createFileField('field_image', 'node', 'article', array(), array('file_extensions' => 'txt png'));
+
+    // Load the node form.
+    $this->drupalGet('node/add/article');
+    $this->assertResponse(200, 'Loaded the article node form.');
+    $this->assertText(strip_tags(t('Create @name', array('@name' => $bundle_label))));
+
+    // Generate an image file.
+    $image = $this->getTestFile('image');
+
+    // Submit the form but exclude the title field.
+    $edit = array(
+      'body[0][value]' => 'Test article',
+      'files[field_image_0]' => $this->container->get('file_system')->realpath($image->getFileUri()),
+    );
+    if (!$this->loggedInUser) {
+      $label = 'Save';
+    }
+    else {
+      $label = 'Save and publish';
+    }
+    $this->drupalPostForm(NULL, $edit, $label);
+    $this->assertResponse(200);
+    $t_args = array('@type' => $bundle_label, '%title' => $node_title);
+    $this->assertNoText(strip_tags(t('@type %title has been created.', $t_args)), 'The node was created.');
+    $this->assertText('Title field is required.');
+
+    // Submit the form again but this time with the missing title field. This
+    // should still work.
+    $edit = array(
+      'title[0][value]' => $node_title,
+    );
+    $this->drupalPostForm(NULL, $edit, $label);
+
+    // Confirm the final submission actually worked.
+    $t_args = array('@type' => $bundle_label, '%title' => $node_title);
+    $this->assertText(strip_tags(t('@type %title has been created.', $t_args)), 'The node was created.');
+    $matches = array();
+    if (preg_match('@node/(\d+)$@', $this->getUrl(), $matches)) {
+      $nid = end($matches);
+      $this->assertNotEqual($nid, 0, 'The node ID was extracted from the URL.');
+      $node = Node::load($nid);
+      $this->assertNotEqual($node, NULL, 'The node was loaded successfully.');
+      $this->assertFileExists(File::load($node->field_image->target_id), 'The image was uploaded successfully.');
+    }
+  }
+
+}