From 6836174a6543ff55baf3ef6d6e6cf50b836518b5 Mon Sep 17 00:00:00 2001 From: Robbie Averill Date: Wed, 20 Sep 2017 11:12:50 +1200 Subject: [PATCH] API Remove deprecated migrateSettings() and secure assets integration code --- _config/secureassets.yml | 15 -- code/Extension/SecureEditableFileField.php | 160 ------------------ code/Model/EditableFormField.php | 22 --- .../EditableFormField/EditableCheckbox.php | 11 -- .../EditableFormField/EditableFileField.php | 12 -- docs/en/upgrading.md | 17 ++ .../SecureEditableFileFieldTest.php | 97 ----------- 7 files changed, 17 insertions(+), 317 deletions(-) delete mode 100644 _config/secureassets.yml delete mode 100644 code/Extension/SecureEditableFileField.php delete mode 100644 tests/Model/EditableFormField/SecureEditableFileFieldTest.php diff --git a/_config/secureassets.yml b/_config/secureassets.yml deleted file mode 100644 index 67b090d..0000000 --- a/_config/secureassets.yml +++ /dev/null @@ -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 diff --git a/code/Extension/SecureEditableFileField.php b/code/Extension/SecureEditableFileField.php deleted file mode 100644 index c17ba75..0000000 --- a/code/Extension/SecureEditableFileField.php +++ /dev/null @@ -1,160 +0,0 @@ -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; - } - } -} diff --git a/code/Model/EditableFormField.php b/code/Model/EditableFormField.php index 41fe13e..633c22b 100755 --- a/code/Model/EditableFormField.php +++ b/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 * diff --git a/code/Model/EditableFormField/EditableCheckbox.php b/code/Model/EditableFormField/EditableCheckbox.php index a6c7afd..a58960f 100755 --- a/code/Model/EditableFormField/EditableCheckbox.php +++ b/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; diff --git a/code/Model/EditableFormField/EditableFileField.php b/code/Model/EditableFormField/EditableFileField.php index 43d6850..d5e9553 100755 --- a/code/Model/EditableFormField/EditableFileField.php +++ b/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 */ diff --git a/docs/en/upgrading.md b/docs/en/upgrading.md index fce68bc..1818b5a 100644 --- a/docs/en/upgrading.md +++ b/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. diff --git a/tests/Model/EditableFormField/SecureEditableFileFieldTest.php b/tests/Model/EditableFormField/SecureEditableFileFieldTest.php deleted file mode 100644 index a3c3ee5..0000000 --- a/tests/Model/EditableFormField/SecureEditableFileFieldTest.php +++ /dev/null @@ -1,97 +0,0 @@ -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()); - } -}