From 1dd787327aab2b2d54432a05174c5d534f6d2b6f Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Fri, 28 Aug 2015 13:21:57 +1200 Subject: [PATCH] API Integrate with secure assets module --- .travis.yml | 6 +- _config/secureassets.yml | 16 +++ code/extensions/SecureEditableFileField.php | 144 ++++++++++++++++++++ composer.json | 3 +- docs/en/installation.md | 38 +++++- tests/SecureEditableFileFieldTest.php | 83 +++++++++++ 6 files changed, 283 insertions(+), 7 deletions(-) create mode 100644 _config/secureassets.yml create mode 100644 code/extensions/SecureEditableFileField.php create mode 100644 tests/SecureEditableFileFieldTest.php diff --git a/.travis.yml b/.travis.yml index e4d08e0..81ba02a 100644 --- a/.travis.yml +++ b/.travis.yml @@ -29,10 +29,14 @@ matrix: env: DB=PGSQL CORE_RELEASE=3.1 - php: 5.6 env: DB=PGSQL CORE_RELEASE=3.1 + include: + - php: 5.4 + env: DB=MYSQL CORE_RELEASE=3.1 SECUREASSETS=1 before_script: - git clone git://github.com/silverstripe-labs/silverstripe-travis-support.git ~/travis-support - - php ~/travis-support/travis_setup.php --source `pwd` --target ~/builds/ss + - "if [ \"$SECUREASSETS\" = \"\" ]; then php ~/travis-support/travis_setup.php --source `pwd` --target ~/builds/ss; fi" + - "if [ \"$SECUREASSETS\" = \"1\" ]; then php ~/travis-support/travis_setup.php --source `pwd` --target ~/builds/ss --require silverstripe/secureassets; fi" - cd ~/builds/ss script: diff --git a/_config/secureassets.yml b/_config/secureassets.yml new file mode 100644 index 0000000..2be1e1e --- /dev/null +++ b/_config/secureassets.yml @@ -0,0 +1,16 @@ +--- +Name: userformssecurity +Only: + ModuleExists: secureassets +--- +EditableFileField: + extensions: + - SecureEditableFileField + +--- +Name: userformsnosecurity +Except: + ModuleExists: secureassets +--- +EditableFileField: + hidden: true \ No newline at end of file diff --git a/code/extensions/SecureEditableFileField.php b/code/extensions/SecureEditableFileField.php new file mode 100644 index 0000000..e8bae94 --- /dev/null +++ b/code/extensions/SecureEditableFileField.php @@ -0,0 +1,144 @@ +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')->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/composer.json b/composer.json index e608167..15363c2 100644 --- a/composer.json +++ b/composer.json @@ -15,7 +15,8 @@ "silverstripe/segment-field": "^1.0" }, "suggest": { - "colymba/gridfield-bulk-editing-tools": "Allows for bulk management of form submissions" + "colymba/gridfield-bulk-editing-tools": "Allows for bulk management of form submissions", + "silverstripe/secureassets": "Enables files uploaded via userforms to be secured from public access" }, "extra": { "branch-alias": { diff --git a/docs/en/installation.md b/docs/en/installation.md index d283410..05c9975 100644 --- a/docs/en/installation.md +++ b/docs/en/installation.md @@ -21,8 +21,8 @@ You should see a new PageType in the CMS 'User Defined Form'. This has a new 'Fo ## File Uploads and Security -The module allows adding a "File Upload Field" to a form, -which enables users of this form to upload files to the website's assets +The module optionally allows adding a "File Upload Field" to a form. +The field enables users of this form to upload files to the website's assets so they can be viewed later by CMS authors. Small files are also attached to the (optional) email notifications to any configured recipients. @@ -35,15 +35,43 @@ configuration setting. The allowed upload size is determined by PHP configuration for this website (the smaller value of `upload_max_filesize` or `post_max_size`). +The field is disabled by default since implementors need to determine how files are secured. Since uploaded files are kept in `assets/` folder of the webroot, there is no built-in permission control around who can view them. It is unlikely that website users guess the URLs to uploaded files unless they are specifically exposed through custom code. -Nevertheless, you should think carefully about the use case for file uploads. +You should think carefully about the use case for file uploads. Unauthorised viewing of files might be desired, e.g. submissions for public competitions. In other cases, submissions could be expected to contain private data. -Please consider securing these files, e.g. through the [secureassets](http://addons.silverstripe.org/add-ons/silverstripe/secureassets) module. + +In order to enable this field it is advisable to install the +[secureassets](http://addons.silverstripe.org/add-ons/silverstripe/secureassets) module. + +This can be done using the below composer command: + +``` +composer require silverstripe/secureassets ~1.0.4 +``` + +This step will have the following effects: + + * The "File Upload Field" will be enabled through the CMS interface + * By default any new file upload fields will be assigned to a default folder, `SecureUploads`, + which can only be accessed by admins. + * Any existing file upload fields, if they are not assigned to a folder, will now upload + to this folder. + * Any existing file upload fields which are assigned folders will have the security settings + for those folders updated so that only admins can access them. + +This functionality can be configured in the following ways: + + * Assigning another group to the `SecureUploads` folder will allow users from that group + (rather than admins only) to access those files. + * The folder name can be configured using the `EditableFileField.secure_folder_name` config option. + * Security functionality can be disabled (although this is not advisable) by setting + `EditableFileField.disable_security` to true. + ### Custom email templates @@ -54,4 +82,4 @@ UserDefinedForm: email_template_directory: your/template/path/ ```` -Any SilverStripe templates placed in your `email_template_directory` directory will be available for use with submission emails. +Any SilverStripe templates placed in your `email_template_directory` directory will be available for use with submission emails. \ No newline at end of file diff --git a/tests/SecureEditableFileFieldTest.php b/tests/SecureEditableFileFieldTest.php new file mode 100644 index 0000000..f3ca713 --- /dev/null +++ b/tests/SecureEditableFileFieldTest.php @@ -0,0 +1,83 @@ +skipTest = true; + $this->markTestSkipped(get_class() . ' skipped unless running with securefiles'); + } + Config::inst()->update('EditableFileField', 'secure_folder_name', 'SecureEditableFileFieldTest/SecureUploads'); + $this->clearPath(); + } + + public 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::inst()->update('EditableFileField', '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::inst()->update('EditableFileField', 'disable_security', false); + singleton('EditableFileField')->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()); + } +} \ No newline at end of file