Merge pull request #661 from creative-commoners/pulls/5.0/secure-upload-info

API Remove deprecated migrateSettings() and secure assets integration code

_config/secureassets.yml

@@ -1,15 +0,0 @@
----
-Name: userformssecurity
-Only:
-  ModuleExists: secureassets
----
-SilverStripe\UserForms\Model\EditableFormField\EditableFileField:
-  extensions:
-    - SilverStripe\UserForms\Extension\SecureEditableFileField
----
-Name: userformsnosecurity
-Except:
-  ModuleExists: secureassets
----
-SilverStripe\UserForms\Model\EditableFormField\EditableFileField:
-  hidden: true

code/Extension/SecureEditableFileField.php

@@ -1,160 +0,0 @@
-<?php
-
-namespace SilverStripe\UserForms\Extension;
-
-use SilverStripe\Assets\Folder;
-use SilverStripe\ORM\DataExtension;
-use SilverStripe\ORM\DB;
-use SilverStripe\Security\Group;
-use SilverStripe\Security\Permission;
-use SilverStripe\UserForms\Model\EditableFormField\EditableFileField;
-
-/**
- * Provides additional file security for uploaded files when the securefiles module is installed
- *
- * {@see EditableFileField}
- */
-class SecureEditableFileField extends DataExtension
-{
-
-    /**
-     * Path to secure files location under assets
-     *
-     * @config
-     * @var type
-     */
-    private static $secure_folder_name = 'SecureUploads';
-
-    /**
-     * Disable file security if a user-defined mechanism is in place
-     *
-     * @config
-     * @var bool
-     */
-    private static $disable_security = false;
-
-    /*
-     * Check if file security is enabled
-     *
-     * @return bool
-     */
-    public function getIsSecurityEnabled()
-    {
-        // Skip if requested
-        if ($this->owner->config()->disable_security) {
-            return false;
-        }
-
-        // Check for necessary security module
-        if (!class_exists('SecureFileExtension')) {
-            trigger_error('SecureEditableFileField requires secureassets module', E_USER_WARNING);
-            return false;
-        }
-
-        return true;
-    }
-
-    public function requireDefaultRecords()
-    {
-        // Skip if disabled
-        if (!$this->getIsSecurityEnabled()) {
-            return;
-        }
-
-        // Update all instances of editablefilefield which do NOT have a secure folder assigned
-        foreach (EditableFileField::get() as $fileField) {
-            // Skip if secured
-            if ($fileField->getIsSecure()) {
-                continue;
-            }
-
-            // Force this field to secure itself on write
-            $fileField->write(false, false, true);
-            DB::alteration_message(
-                "Restricting editable file field \"{$fileField->Title}\" to secure folder",
-                "changed"
-            );
-        }
-    }
-
-    /**
-     * Secure this field before saving
-     */
-    public function onBeforeWrite()
-    {
-        $this->makeSecure();
-    }
-
-    /**
-     * Ensure this field is secured, but does not write changes to the database
-     */
-    public function makeSecure()
-    {
-        // Skip if disabled or already secure
-        if (!$this->getIsSecurityEnabled() || $this->owner->getIsSecure()) {
-            return;
-        }
-
-        // Ensure folder exists
-        $folder = $this->owner->Folder();
-        if (!$folder || !$folder->exists()) {
-            // Create new folder in default location
-            $folder = Folder::find_or_make($this->owner->config()->secure_folder_name);
-            $this->owner->FolderID = $folder->ID;
-        } elseif ($this->isFolderSecured($folder)) {
-            // If folder exists and is secure stop
-            return;
-        }
-
-        // Make secure
-        $folder->CanViewType = 'OnlyTheseUsers';
-        $folder->ViewerGroups()->add($this->findAdminGroup());
-        $folder->write();
-    }
-
-    /**
-     * Find target group to record
-     *
-     * @return Group
-     */
-    protected function findAdminGroup()
-    {
-        singleton(Group::class)->requireDefaultRecords();
-        return Permission::get_groups_by_permission('ADMIN')->First();
-    }
-
-    /**
-     * Determine if the field is secure
-     *
-     * @return bool
-     */
-    public function getIsSecure()
-    {
-        return $this->isFolderSecured($this->owner->Folder());
-    }
-
-    /**
-     * Check if a Folder object is secure
-     *
-     * @param Folder $folder
-     * @return boolean
-     */
-    protected function isFolderSecured($folder)
-    {
-        if (! ($folder instanceof Folder) || !$folder->exists()) {
-            return false;
-        }
-
-        switch ($folder->CanViewType) {
-            case 'OnlyTheseUsers':
-                return true;
-            case 'Inherit':
-                $parent = $folder->Parent();
-                return $parent && $parent->exists() && $this->isFolderSecured($parent);
-            case 'Anyone':
-            case 'LoggedInUsers':
-            default:
-                return false;
-        }
-    }
-}

code/Model/EditableFormField.php

@@ -794,28 +794,6 @@ class EditableFormField extends DataObject
         return DBField::create_field('Varchar', $errorMessage);
     }
 
-    /**
-     * Invoked by UserFormUpgradeService to migrate settings specific to this field from CustomSettings
-     * to the field proper
-     *
-     * @param array $data Unserialised data
-     */
-    public function migrateSettings($data)
-    {
-        // Map 'Show' / 'Hide' to boolean
-        if (isset($data['ShowOnLoad'])) {
-            $this->ShowOnLoad = $data['ShowOnLoad'] === '' || ($data['ShowOnLoad'] && $data['ShowOnLoad'] !== 'Hide');
-            unset($data['ShowOnLoad']);
-        }
-
-        // Migrate all other settings
-        foreach ($data as $key => $value) {
-            if ($this->hasField($key)) {
-                $this->setField($key, $value);
-            }
-        }
-    }
-
     /**
      * Get the formfield to use when editing this inline in gridfield
      *

code/Model/EditableFormField/EditableCheckbox.php

@@ -62,17 +62,6 @@ class EditableCheckbox extends EditableFormField
             : _t('SilverStripe\\UserForms\\Model\\EditableFormField.NO', 'No');
     }
 
-    public function migrateSettings($data)
-    {
-        // Migrate 'Default' setting to 'CheckedDefault'
-        if (isset($data['Default'])) {
-            $this->CheckedDefault = (bool)$data['Default'];
-            unset($data['Default']);
-        }
-
-        parent::migrateSettings($data);
-    }
-
     public function isCheckBoxField()
     {
         return true;

code/Model/EditableFormField/EditableFileField.php

@@ -147,18 +147,6 @@ class EditableFileField extends EditableFormField
         return SubmittedFileField::create();
     }
 
-
-    public function migrateSettings($data)
-    {
-        // Migrate 'Folder' setting to 'FolderID'
-        if (isset($data[Folder::class])) {
-            $this->FolderID = $data[Folder::class];
-            unset($data[Folder::class]);
-        }
-
-        parent::migrateSettings($data);
-    }
-
     /**
      * @return float
      */

docs/en/upgrading.md

@@ -12,6 +12,7 @@ The SilverStripe 4 compatible version of UserForms (5.x) introduces some breakin
   * `EditableFormField::getSetting()`
   * `EditableFormField::getFieldName()`
   * `EditableFormField::getSettingName()`
+  * `EditableFormField::migrateSettings()`
 
 The frontend JavaScript and SASS assets have been converted from Compass to
 [Webpack](https://github.com/silverstripe/webpack-config), and now live in the `client/` folder. Please
@@ -22,3 +23,19 @@ The frontend JavaScript and SASS assets have been converted from Compass to
 UserForms 5.0 includes a legacy class name remapping configuration file (legacy.yml) as well as `$table_name`
 configuration for every DataObject to ensure that existing data in your database will remain unchanged. This
 should help to ensure that your upgrade process from UserForms 4.x on SilverStripe 3 is as seamless as possible.
+
+## Secure assets for EditableFileField
+
+In SilverStripe 3 with UserForms you could install the SecureAssets module to allow the ability to set permissions
+on folders that files can be uploaded into via your custom forms.
+
+In SilverStripe 4 this functionality is built into the core assets module, so the SecureAssets module is no longer
+required, and the SecureEditableFileField extension has been removed.
+
+**Important:** While you shouldn't have to do anything in terms of folder permissions during this upgrade, we highly
+recommend that you sanity check any secure folders you have in your website to ensure that the permissions are still
+locked down before going live.
+
+Please be aware that if you had SecureAssets installed in SilverStripe 3 with UserForms your EditableFileField folder
+choice would be made secure by default, whereas in SilverStripe 4 this is up to the user to configure at their
+discretion.

tests/Model/EditableFormField/SecureEditableFileFieldTest.php

@@ -1,97 +0,0 @@
-<?php
-
-namespace SilverStripe\UserForms\Tests\Model\EditableFormField;
-
-use SilverStripe\Assets\Filesystem;
-use SilverStripe\Assets\Folder;
-use SilverStripe\Core\Config\Config;
-use SilverStripe\Dev\SapphireTest;
-use SilverStripe\UserForms\Model\EditableFormField\EditableFileField;
-
-/**
- * Tests integration of EditableFileField with the securefiles module
- *
- * @todo
- * @author dmooyman
- */
-class SecureEditableFileFieldTest extends SapphireTest
-{
-    protected $usesDatabase = true;
-
-    protected function setUp()
-    {
-        parent::setUp();
-
-        if (!class_exists('SecureFileExtension')) {
-            $this->skipTest = true;
-            $this->markTestSkipped(get_class() . ' skipped unless running with securefiles');
-        }
-        Config::modify()->set(EditableFileField::class, 'secure_folder_name', 'SecureEditableFileFieldTest/SecureUploads');
-        $this->clearPath();
-    }
-
-    protected function tearDown()
-    {
-        $this->clearPath();
-        parent::tearDown();
-    }
-
-    protected function clearPath()
-    {
-        if (file_exists(ASSETS_PATH . '/SecureEditableFileFieldTest')) {
-            Filesystem::removeFolder(ASSETS_PATH . '/SecureEditableFileFieldTest');
-        }
-    }
-
-    /**
-     * Test that newly created folders are secure
-     */
-    public function testCreateFolder()
-    {
-        $field = new EditableFileField();
-        $field->write();
-        $this->assertTrue($field->getIsSecure());
-        $this->assertTrue($field->Folder()->exists());
-        $this->assertEquals('assets/SecureEditableFileFieldTest/SecureUploads/', $field->Folder()->Filename);
-        $this->assertEquals('OnlyTheseUsers', $field->Folder()->CanViewType);
-        $this->assertEquals(1, $field->Folder()->ViewerGroups()->first()->Permissions()->filter('code', 'ADMIN')->count());
-    }
-
-    /**
-     * Test new folders that are created without security enabled
-     */
-    public function testCreateInsecure()
-    {
-        Config::modify()->set(EditableFileField::class, 'disable_security', true);
-
-        // Esure folder is created without a folder
-        $field = new EditableFileField();
-        $field->write();
-        $this->assertFalse($field->getIsSecure());
-        $this->assertFalse($field->Folder()->exists());
-
-        // Assigning a non-secure folder doesn't secure this
-        $folder = Folder::find_or_make('SecureEditableFileFieldTest/PublicFolder');
-        $field->FolderID = $folder->ID;
-        $field->write();
-
-        $this->assertFalse($field->getIsSecure());
-        $this->assertTrue($field->Folder()->exists());
-        $this->assertEquals('assets/SecureEditableFileFieldTest/PublicFolder/', $field->Folder()->Filename);
-        $this->assertEquals('Inherit', $field->Folder()->CanViewType);
-
-        // Enabling security and re-saving will force this field to be made secure (but not changed)
-        Config::modify()->set(EditableFileField::class, 'disable_security', false);
-        singleton(EditableFileField::class)->requireDefaultRecords();
-
-        // Reload record from DB
-        $field = EditableFileField::get()->byID($field->ID);
-
-        // Existing folder is now secured (retro-actively secures any old uploads)
-        $this->assertTrue($field->getIsSecure());
-        $this->assertTrue($field->Folder()->exists());
-        $this->assertEquals('assets/SecureEditableFileFieldTest/PublicFolder/', $field->Folder()->Filename);
-        $this->assertEquals('OnlyTheseUsers', $field->Folder()->CanViewType);
-        $this->assertEquals(1, $field->Folder()->ViewerGroups()->first()->Permissions()->filter('code', 'ADMIN')->count());
-    }
-}