diff --git a/_config/asset.yml b/_config/asset.yml index 1b4249277..3c8541537 100644 --- a/_config/asset.yml +++ b/_config/asset.yml @@ -1,37 +1,66 @@ --- -Name: assetstore +Name: coreflysystem --- Injector: - # Public url plugin - FlysystemUrlPlugin: - class: 'SilverStripe\Filesystem\Flysystem\FlysystemUrlPlugin' # Define the default adapter for this filesystem - FlysystemDefaultAdapter: - class: 'SilverStripe\Filesystem\Flysystem\AssetAdapter' + FlysystemPublicAdapter: + class: 'SilverStripe\Filesystem\Flysystem\PublicAssetAdapter' + # Define the secondary adapter for protected assets + FlysystemProtectedAdapter: + class: 'SilverStripe\Filesystem\Flysystem\ProtectedAssetAdapter' # Define the default filesystem - FlysystemBackend: + FlysystemPublicBackend: class: 'League\Flysystem\Filesystem' constructor: - Adapter: '%$FlysystemDefaultAdapter' - calls: - PublicURLPlugin: [ addPlugin, [ %$FlysystemUrlPlugin ] ] + Adapter: '%$FlysystemPublicAdapter' + Config: + visibility: public + # Define the secondary filesystem for protected assets + FlysystemProtectedBackend: + class: 'League\Flysystem\Filesystem' + constructor: + Adapter: '%$FlysystemProtectedAdapter' + Config: + visibility: private +--- +Name: coreassets +After: + - '#coreflysystem' +--- +Injector: # Define our SS asset backend AssetStore: class: 'SilverStripe\Filesystem\Flysystem\FlysystemAssetStore' properties: - Filesystem: '%$FlysystemBackend' + PublicFilesystem: '%$FlysystemPublicBackend' + ProtectedFilesystem: '%$FlysystemProtectedBackend' + ProtectedFileController: + class: SilverStripe\Filesystem\Storage\ProtectedFileController + properties: + RouteHandler: '%$AssetStore' AssetNameGenerator: class: SilverStripe\Filesystem\Storage\DefaultAssetNameGenerator type: prototype - # Image mechanism - Image_Backend: GDBackend # Requirements config GeneratedAssetHandler: class: SilverStripe\Filesystem\Storage\FlysystemGeneratedAssetHandler properties: - Filesystem: '%$FlysystemBackend' + Filesystem: '%$FlysystemPublicBackend' Requirements_Minifier: class: SilverStripe\View\JSMinifier Requirements_Backend: properties: AssetHandler: '%$GeneratedAssetHandler' +--- +Name: coreassetroutes +After: + - '#coreassets' +--- +Director: + rules: + 'assets': 'ProtectedFileController' +--- +Name: imageconfig +--- +Injector: + Image_Backend: GDBackend diff --git a/core/Extension.php b/core/Extension.php index 57e1ca3b4..7a17bcddd 100644 --- a/core/Extension.php +++ b/core/Extension.php @@ -86,14 +86,15 @@ abstract class Extension { /** * Helper method to strip eval'ed arguments from a string - * thats passed to {@link DataObject::$extensions} or + * that's passed to {@link DataObject::$extensions} or * {@link Object::add_extension()}. * * @param string $extensionStr E.g. "Versioned('Stage','Live')" * @return string Extension classname, e.g. "Versioned" */ public static function get_classname_without_arguments($extensionStr) { - return (($p = strpos($extensionStr, '(')) !== false) ? substr($extensionStr, 0, $p) : $extensionStr; + $parts = explode('(', $extensionStr); + return $parts[0]; } diff --git a/core/Object.php b/core/Object.php index 6c9d17f84..ac29b5408 100755 --- a/core/Object.php +++ b/core/Object.php @@ -406,21 +406,20 @@ abstract class Object { //BC support if(func_num_args() > 1){ $class = $classOrExtension; - $requiredExtension = $requiredExtension; - } - else { + } else { $class = get_called_class(); $requiredExtension = $classOrExtension; } - $requiredExtension = strtolower($requiredExtension); - $extensions = Config::inst()->get($class, 'extensions'); - - if($extensions) foreach($extensions as $extension) { - $left = strtolower(Extension::get_classname_without_arguments($extension)); - $right = strtolower(Extension::get_classname_without_arguments($requiredExtension)); - if($left == $right) return true; - if (!$strict && is_subclass_of($left, $right)) return true; + $requiredExtension = Extension::get_classname_without_arguments($requiredExtension); + $extensions = self::get_extensions($class); + foreach($extensions as $extension) { + if(strcasecmp($extension, $requiredExtension) === 0) { + return true; + } + if (!$strict && is_subclass_of($extension, $requiredExtension)) { + return true; + } } return false; @@ -550,6 +549,12 @@ abstract class Object { */ public static function get_extensions($class, $includeArgumentString = false) { $extensions = Config::inst()->get($class, 'extensions'); + if(empty($extensions)) { + return array(); + } + + // Clean nullified named extensions + $extensions = array_filter(array_values($extensions)); if($includeArgumentString) { return $extensions; diff --git a/dev/CliTestReporter.php b/dev/CliTestReporter.php index c0a3486c0..677fa869b 100644 --- a/dev/CliTestReporter.php +++ b/dev/CliTestReporter.php @@ -12,35 +12,47 @@ class CliTestReporter extends SapphireTestReporter { */ public function writeResults() { $passCount = 0; - $failCount = 0; + $failCount = $this->currentSession['failures']; $testCount = 0; - $incompleteCount = 0; - $errorCount = 0; + $incompleteCount = $this->currentSession['incomplete']; + $errorCount = $this->currentSession['errors']; foreach($this->suiteResults['suites'] as $suite) { foreach($suite['tests'] as $test) { $testCount++; - if($test['status'] == 2) { - $incompleteCount++; - } elseif($test['status'] === 1) { - $passCount++; - } else { - $failCount++; + switch($test['status']) { + case TEST_INCOMPLETE: { + $incompleteCount++; + break; + } + case TEST_SUCCESS: { + $passCount++; + break; + } + case TEST_ERROR: { + $errorCount++; + break; + } + default: { + $failCount++; + break; + } } } } echo "\n\n"; - if ($failCount == 0 && $incompleteCount > 0) { + $breakages = $errorCount + $failCount; + if ($breakages == 0 && $incompleteCount > 0) { echo SS_Cli::text(" OK, BUT INCOMPLETE TESTS! ", "black", "yellow"); - } elseif ($failCount == 0) { + } elseif ($breakages == 0) { echo SS_Cli::text(" ALL TESTS PASS ", "black", "green"); } else { echo SS_Cli::text(" AT LEAST ONE FAILURE ", "black", "red"); } echo sprintf("\n\n%d tests run: %s, %s, and %s\n", $testCount, SS_Cli::text("$passCount passes"), - SS_Cli::text("$failCount failures"), SS_Cli::text("$incompleteCount incomplete")); + SS_Cli::text("$breakages failures"), SS_Cli::text("$incompleteCount incomplete")); echo "Maximum memory usage: " . number_format(memory_get_peak_usage()/(1024*1024), 1) . "M\n\n"; @@ -80,6 +92,19 @@ class CliTestReporter extends SapphireTestReporter { parent::endTest($test, $time); } + protected function addStatus($status, $message, $exception, $trace) { + if(!$this->currentTest && !$this->currentSuite) { + // Log non-test errors immediately + $statusResult = array( + 'status' => $status, + 'message' => $message, + 'exception' => $exception, + 'trace' => $trace + ); + $this->writeTest($statusResult); + } + parent::addStatus($status, $message, $exception, $trace); + } protected function writeTest($test) { if ($test['status'] != TEST_SUCCESS) { diff --git a/dev/SapphireTestReporter.php b/dev/SapphireTestReporter.php index b658b8f67..14bed0ce2 100644 --- a/dev/SapphireTestReporter.php +++ b/dev/SapphireTestReporter.php @@ -1,6 +1,10 @@ $this->hasTimer, // availability of PEAR Benchmark_Timer 'totalTests' => 0 // total number of tests run ); + + $this->currentSession = array( + 'errors' => 0, // number of tests with errors (including setup errors) + 'failures' => 0, // number of tests which failed + 'incomplete' => 0, // number of tests that were not completed correctly + 'error' => array(), // Any error encountered outside of suites + ); } /** @@ -110,54 +135,47 @@ class SapphireTestReporter implements PHPUnit_Framework_TestListener { * Sets up the container for result details of the current test suite when * each suite is first run * - * @access public - * @param obj PHPUnit2_Framework_TestSuite, the suite that is been run - * @return void + * @param PHPUnit_Framework_TestSuite $suite the suite that is been run */ - public function startTestSuite( PHPUnit_Framework_TestSuite $suite) { - if(strlen($suite->getName())) { - $this->endCurrentTestSuite(); + public function startTestSuite(PHPUnit_Framework_TestSuite $suite) { + $this->endCurrentTestSuite(); $this->currentSuite = array( 'suite' => $suite, // the test suite 'tests' => array(), // the tests in the suite 'errors' => 0, // number of tests with errors (including setup errors) 'failures' => 0, // number of tests which failed 'incomplete' => 0, // number of tests that were not completed correctly - 'error' => null); // Any error encountered during setup of the test suite - } + 'error' => null, // Any error encountered during setup of the test suite + ); } /** * Sets up the container for result details of the current test when each * test is first run * - * @access public - * @param obj PHPUnit_Framework_Test, the test that is being run - * @return void + * @param PHPUnit_Framework_Test $test the test that is being run */ public function startTest(PHPUnit_Framework_Test $test) { - $this->startTestTime = microtime(true); + $this->endCurrentTest(); - if($test instanceof PHPUnit_Framework_TestCase) { - $this->endCurrentTest(); - $this->currentTest = array( - // the name of the test (without the suite name) - 'name' => preg_replace('(\(.*\))', '', $test->toString()), - // execution time of the test - 'timeElapsed' => 0, - // status of the test execution - 'status' => TEST_SUCCESS, - // user message of test result - 'message' => '', - // original caught exception thrown by test upon failure/error - 'exception' => NULL, - // Stacktrace used for exception handling - 'trace' => NULL, - // a unique ID for this test (used for identification purposes in results) - 'uid' => md5(microtime()) - ); - if($this->hasTimer) $this->timer->start(); - } + $this->startTestTime = microtime(true); + $this->currentTest = array( + // the name of the test (without the suite name) + 'name' => $this->descriptiveTestName($test), + // execution time of the test + 'timeElapsed' => 0, + // status of the test execution + 'status' => TEST_SUCCESS, + // user message of test result + 'message' => '', + // original caught exception thrown by test upon failure/error + 'exception' => NULL, + // Stacktrace used for exception handling + 'trace' => NULL, + // a unique ID for this test (used for identification purposes in results) + 'uid' => md5(microtime()) + ); + if($this->hasTimer) $this->timer->start(); } /** @@ -170,7 +188,7 @@ class SapphireTestReporter implements PHPUnit_Framework_TestListener { */ protected function addStatus($status, $message, $exception, $trace) { // Build status body to be saved - $status = array( + $statusResult = array( 'status' => $status, 'message' => $message, 'exception' => $exception, @@ -179,9 +197,11 @@ class SapphireTestReporter implements PHPUnit_Framework_TestListener { // Log either to current test or suite record if($this->currentTest) { - $this->currentTest = array_merge($this->currentTest, $status); + $this->currentTest = array_merge($this->currentTest, $statusResult); + } elseif($this->currentSuite) { + $this->currentSuite['error'] = $statusResult; } else { - $this->currentSuite['error'] = $status; + $this->currentSession['error'][] = $statusResult; } } @@ -189,13 +209,16 @@ class SapphireTestReporter implements PHPUnit_Framework_TestListener { * Adds the failure detail to the current test and increases the failure * count for the current suite * - * @access public - * @param obj PHPUnit_Framework_Test, current test that is being run - * @param obj PHPUnit_Framework_AssertationFailedError, PHPUnit error - * @return void + * @param PHPUnit_Framework_Test $test current test that is being run + * @param PHPUnit_Framework_AssertionFailedError $e PHPUnit error + * @param int $time */ public function addFailure(PHPUnit_Framework_Test $test, PHPUnit_Framework_AssertionFailedError $e, $time) { - $this->currentSuite['failures']++; + if($this->currentSuite) { + $this->currentSuite['failures']++; + } else { + $this->currentSession['failures']++; + } $this->addStatus(TEST_FAILURE, $e->toString(), $this->getTestException($test, $e), $e->getTrace()); } @@ -203,13 +226,16 @@ class SapphireTestReporter implements PHPUnit_Framework_TestListener { * Adds the error detail to the current test and increases the error * count for the current suite * - * @access public - * @param obj PHPUnit_Framework_Test, current test that is being run - * @param obj PHPUnit_Framework_AssertationFailedError, PHPUnit error - * @return void + * @param PHPUnit_Framework_Test $test current test that is being run + * @param Exception $e PHPUnit error + * @param int $time */ public function addError(PHPUnit_Framework_Test $test, Exception $e, $time) { - $this->currentSuite['errors']++; + if($this->currentSuite) { + $this->currentSuite['errors']++; + } else { + $this->currentSession['errors']++; + } $this->addStatus(TEST_ERROR, $e->getMessage(), $this->getTestException($test, $e), $e->getTrace()); } @@ -217,21 +243,24 @@ class SapphireTestReporter implements PHPUnit_Framework_TestListener { * Adds the test incomplete detail to the current test and increases the incomplete * count for the current suite * - * @access public - * @param obj PHPUnit_Framework_Test, current test that is being run - * @param obj PHPUnit_Framework_AssertationFailedError, PHPUnit error - * @return void + * @param PHPUnit_Framework_Test $test current test that is being run + * @param Exception $e PHPUnit error */ public function addIncompleteTest(PHPUnit_Framework_Test $test, Exception $e, $time) { - $this->currentSuite['incomplete']++; - $this->addStatus(TEST_INCOMPLETE, $e->toString(), $this->getTestException($test, $e), $e->getTrace()); + if($this->currentSuite) { + $this->currentSuite['incomplete']++; + } else { + $this->currentSession['incomplete']++; + } + $this->addStatus(TEST_INCOMPLETE, $e->getMessage(), $this->getTestException($test, $e), $e->getTrace()); } /** * Not used * * @param PHPUnit_Framework_Test $test - * @param unknown_type $time + * @param Exception $e + * @param int $time */ public function addSkippedTest(PHPUnit_Framework_Test $test, Exception $e, $time) { // not implemented @@ -261,9 +290,8 @@ class SapphireTestReporter implements PHPUnit_Framework_TestListener { * Upon completion of a test, records the execution time (if available) and adds the test to * the tests performed in the current suite. * - * @access public - * @param obj PHPUnit_Framework_Test, current test that is being run - * @return void + * @param PHPUnit_Framework_Test $test Current test that is being run + * @param int $time */ public function endTest( PHPUnit_Framework_Test $test, $time) { $this->endCurrentTest(); @@ -290,9 +318,7 @@ class SapphireTestReporter implements PHPUnit_Framework_TestListener { /** * Upon completion of a test suite adds the suite to the suties performed * - * @access public - * @param obj PHPUnit_Framework_TestSuite, current suite that is being run - * @return void + * @param PHPUnit_Framework_TestSuite $suite current suite that is being run */ public function endTestSuite( PHPUnit_Framework_TestSuite $suite) { if(strlen($suite->getName())) { @@ -313,28 +339,32 @@ class SapphireTestReporter implements PHPUnit_Framework_TestListener { } /** - * Trys to get the original exception thrown by the test on failure/error + * Tries to get the original exception thrown by the test on failure/error * to enable us to give a bit more detail about the failure/error * - * @access private - * @param obj PHPUnit_Framework_Test, current test that is being run - * @param obj PHPUnit_Framework_AssertationFailedError, PHPUnit error + * @param PHPUnit_Framework_Test $test current test that is being run + * @param Exception $e PHPUnit error * @return array */ private function getTestException(PHPUnit_Framework_Test $test, Exception $e) { // get the name of the testFile from the test - $testName = preg_replace('/(.*)\((.*[^)])\)/', '\\2', $test->toString()); + $testName = $this->descriptiveTestName($test); $trace = $e->getTrace(); // loop through the exception trace to find the original exception for($i = 0; $i < count($trace); $i++) { if(array_key_exists('file', $trace[$i])) { - if(stristr($trace[$i]['file'], $testName.'.php') != false) return $trace[$i]; + if(stristr($trace[$i]['file'], $testName.'.php') != false) { + return $trace[$i]; + } } if(array_key_exists('file:protected', $trace[$i])) { - if(stristr($trace[$i]['file:protected'], $testName.'.php') != false) return $trace[$i]; + if(stristr($trace[$i]['file:protected'], $testName.'.php') != false) { + return $trace[$i]; + } } } + return array(); } /** @@ -364,6 +394,21 @@ class SapphireTestReporter implements PHPUnit_Framework_TestListener { // A suite may not end correctly if there was an error during setUp $this->endCurrentTestSuite(); + // Write session errors + if($this->currentSession['error']) { + $errorCount += $this->currentSession['errors']; + $failCount += $this->currentSession['failures']; + $incompleteCount += $this->currentSession['incomplete']; + foreach($this->currentSession['error'] as $error) { + $this->writeResultError( + 'Session', + $error['message'], + $error['trace'] + ); + } + } + + // Write suite errors foreach($this->suiteResults['suites'] as $suite) { // Report suite error. In the case of fatal non-success messages @@ -409,5 +454,23 @@ class SapphireTestReporter implements PHPUnit_Framework_TestListener { protected function testNameToPhrase($name) { return ucfirst(preg_replace("/([a-z])([A-Z])/", "$1 $2", $name)); } + + /** + * Get name for this test + * + * @param PHPUnit_Framework_Test $test + * @return string + */ + protected function descriptiveTestName(PHPUnit_Framework_Test $test) { + if ($test instanceof PHPUnit_Framework_TestCase) { + $name = $test->toString(); + } elseif(method_exists($test, 'getName')) { + $name = $test->getName(); + } else { + $name = get_class($test); + } + // the name of the test (without the suite name) + return preg_replace('(\(.*\))', '', $name); + } } diff --git a/docs/en/00_Getting_Started/01_Installation/How_To/Configure_Nginx.md b/docs/en/00_Getting_Started/01_Installation/How_To/Configure_Nginx.md index a8ae1cbd3..86d6e5247 100644 --- a/docs/en/00_Getting_Started/01_Installation/How_To/Configure_Nginx.md +++ b/docs/en/00_Getting_Started/01_Installation/How_To/Configure_Nginx.md @@ -37,8 +37,11 @@ But enough of the disclaimer, on to the actual configuration — typically in `n error_page 500 /assets/error-500.html; location ^~ /assets/ { + location ~ /\. { + deny all; + } sendfile on; - try_files $uri =404; + try_files $uri /framework/main.php?url=$uri&$query_string; } location ~ /framework/.*(main|rpc|tiny_mce_gzip)\.php$ { diff --git a/docs/en/02_Developer_Guides/09_Security/04_Secure_Coding.md b/docs/en/02_Developer_Guides/09_Security/04_Secure_Coding.md index ac36400b0..7c19759cc 100644 --- a/docs/en/02_Developer_Guides/09_Security/04_Secure_Coding.md +++ b/docs/en/02_Developer_Guides/09_Security/04_Secure_Coding.md @@ -430,29 +430,10 @@ standard PHP way. See [casting](/topics/datamodel#casting). ## Filesystem -### Don't allow script-execution in /assets +### Don't script-execution in /assets -As all uploaded files are stored by default on the /assets-directory, you should disallow script-execution for this -folder. This is just an additional security-measure to making sure you avoid directory-traversal, check for filesize and -disallow certain filetypes. - -Example configuration for Apache2: - - - - php_flag engine off - Options -ExecCGI -Includes -Indexes - - - - -If you are using shared hosting or in a situation where you cannot alter your Vhost definitions, you can use a .htaccess -file in the assets directory. This requires PHP to be loaded as an Apache module (not CGI or FastCGI). - -**/assets/.htaccess** - - php_flag engine off - Options -ExecCGI -Includes -Indexes +Please refer to the article on [file security](/developer_guides/files/file_security) +for instructions on how to secure the assets folder against malicious script execution. ### Don't allow access to YAML files diff --git a/docs/en/02_Developer_Guides/14_Files/03_File_Security.md b/docs/en/02_Developer_Guides/14_Files/03_File_Security.md new file mode 100644 index 000000000..f4687fb60 --- /dev/null +++ b/docs/en/02_Developer_Guides/14_Files/03_File_Security.md @@ -0,0 +1,375 @@ +summary: Manage access permission to assets + +# File Security + +## Security overview + +File security is an important concept, and is as essential as managing any other piece of data that exists +in your system. As pages and dataobjects can be either versioned, or restricted to view by authenticated +members, it is necessary at times to apply similar logic to any files which are attached to these objects +in the same way. + +Out of the box SilverStripe Framework comes with an asset storage mechanism with two stores, a public +store and a protected one. Most operations which act on assets work independently of this mechanism, +without having to consider whether any specific file is protected or public, but can normally be +instructed to favour private or protected stores in some cases. + +For instance, in order to write an asset to a protected location you can use the following additional +config option: + + + :::php + $store = singleton('AssetStore'); + $store->setFromString('My protected content', 'Documents/Mydocument.txt', null, null, array( + 'visibility' => AssetStore::VISIBILITY_PROTECTED + )); + + +## User access control + +Access for files is granted on a per-session basis, rather than on a per-member basis, via +whitelisting accessed assets. This means that access to any protected asset must be made prior to the user +actually attempting to download that asset. This is normally done in the PHP request that generated +the response containing the link to that file. + +An automated system will, in most cases, handle this whitelisting for you. Calls to getURL() +will automatically whitelist access to that file for the current user. Using this as a guide, you can easily +control access to embedded assets at a template level. + + :::ss + + +Users who are able to guess the value of $URL will not be able to access those urls without being +authorised by this code. + +In order to ensure protected assets are not leaked publicly, but are properly whitelisted for +authorised users, the following should be considered: + +* Caching mechanisms which prevent `$URL` being invoked for the user's request (such as `$URL` within a + partial cache block) will not whitelist those files automatically. You can manually whitelist a + file via PHP for the current user instead, by using the following code to grant access. + + + :::php + class Page_Controller extends ContentController { + public function init() { + parent::init(); + // Whitelist any protected files on this page for the current user + foreach($this->Files() as $file) { + if($file->canView()) { + $file->grantFile(); + } + } + } + } + +* If a user does not have access to a file, you can still generate the URL but suppress the default + permission whitelist by invoking the getter as a method, but pass in a falsey value as a parameter. + (or '0' in template as a workaround for all parameters being cast as string) + + + ::php + <% if not $canView %> + +
  • Access to $Title is denied
    • + <% else %> + + +* Alternatively, if a user has already been granted access, you can explicitly revoke their access using + the `revokeFile` method. + + + :::php + class Page_Controller extends ContentController { + public function init() { + parent::init(); + // Whitelist any protected files on this page for the current user + foreach($this->Files() as $file) { + if($file->canView()) { + $file->grantFile(); + } else { + // Will revoke any historical grants + $file->revokeFile(); + } + } + } + } + + +## Controlling asset visibility + +The asset API provides three main mechanisms for setting the visibility of an asset. Note that +these operations are applied on a per file basis, and unlike `revoke` or `grant` methods +these do not affect visibility for specific users. + +Visibility can be specified when files are created via one of the `AssetStore::VISIBILITY_PROTECTED` +or `AssetStore::VISIBILITY_PUBLIC` constants. It's advisable to ensure the visibility of any file +is declared as early as possible, so that potentially sensitive content never touches any +public facing area. + +E.g. + + + :::php + $object->MyFile->setFromLocalFile($tmpFile['Path'], $filename, null, null, array( + 'visibility' => AssetStore::VISIBILITY_PROTECTED + )); + + +You can also adjust the visibility of any existing file to either public or protected. + + + ::: + // This will make the file available only when a user calls `->grant()` + $object->SecretFile->protectFile(); + + // This file will be available to everyone with the URL + $object->PublicFile->publishFile(); + + +
    +One thing to note is that all variants of a single file will be treated as +a single entity for access control, so specific variants cannot be individually controlled. +
    + +## How file access is protected + +Public urls to files do not change, regardless of whether the file is protected or public. Similarly, +operations which modify files do not normally need to be told whether the file is protected or public +either. This provides a consistent method for interacting with files. + +In day to day operation, moving assets to or between either of these stores does not normally +alter these asset urls, as the routing mechanism will infer file access requirements dynamically. +This allows you to prepare predictable file urls on a draft site, which will not change once +the page is published, but will be protected in the mean time. + +For instance, consider two files `OldCompanyLogo.gif` in the public store, and `NewCompanyLogo.gif` +in the protected store, awaiting publishing. + +Internally your folder structure would look something like: + + + ::: + assets/ + .htaccess + .protected/ + .htaccess + a870de278b/ + NewCompanyLogo.gif + 33be1b95cb/ + OldCompanyLogo.gif + + +The urls for these two files, however, do not reflect the physical structure directly. + +* `http://www.mysite.com/assets/33be1b95cb/OldCompanyLogo.gif` will be served directly from the web server, + and will not invoke a php request. +* `http://www.mysite.com/assets/a870de278b/NewCompanyLogo.gif` will be routed via a 404 handler to PHP, + which will be passed to the `[api:ProtectedFileController]` controller, which will serve + up the content of the hidden file, conditional on a permission check. + +When the file `NewCompanyLogo.gif` is made public, the url will not change, but the file location +will be moved to `assets/a870de278b/NewCompanyLogo.gif`, and will be served directly via +the web server, bypassing the need for additional PHP requests. + + + :::php + $store = singleton('AssetStore'); + $store->publish('NewCompanyLogo.gif', 'a870de278b475cb75f5d9f451439b2d378e13af1'); + + +After this the filesystem will now look like below: + + + ::: + assets/ + .htaccess + .protected/ + .htaccess + 33be1b95cb/ + OldCompanyLogo.gif + a870de278b/ + NewCompanyLogo.gif + + +## Performance considerations + +In order to ensure that a site does not invoke any unnecessary PHP processes when serving up files, +it's important to ensure that your server is configured correctly. Serving public files via PHP +will add unnecessary load to your server, but conversely, attempting to serve protected files +directly may lead to necessary security checks being omitted. + +See the web server setting section below for more information on configuring your server properly + +### Performance: Static caching + +If you are deploying your site to a server configuration that makes use of static caching, it's essential +that you ensure any page or dataobject cached adequately publishes any linked assets. This is due to the +fact that static caching will bypass any PHP request, which would otherwise be necessary to whitelist +protected files for these users. + +This is especially important when dealing with draft content, as frontend caches should not attempt to +cache protected content being served to authenticated users. This can be achieved by configuring your cache +correctly to skip `Pragma: no-cache` headers and the `bypassStaticCache` cookie. + +## Configuring protected assets + +### Configuring: Protected folder location + +In the default SilverStripe configuration, protected assets are placed within the web root into the +`assets/.protected` folder, into which is also generated a `.htaccess` or `web.config` configured +to deny any and all direct web requests. + +In order to better ensure these files are protected, it's recommended to move this outside of the web +root altogether. + +For instance, given your web root is in the folder `/sites/mysite/www`, you can tell the asset store +to put protected files into `/sites/mysite/protected` with the below `_ss_environment.php` setting: + + + :::php + define('SS_PROTECTED_ASSETS_PATH', '/sites/mysite/protected'); + + +### Configuring: File types + +In addition to configuring file locations, it's also important to ensure that you have allowed the +appropriate file extensions for your instance. This can be done by setting the `File.allowed_extensions` +config. + + + :::yaml + File: + allowed_extensions: + - 7zip + - xzip + + +
    +Any file not included in this config, or in the default list of extensions, will be blocked from +any requests to the assets directory. Invalid files will be blocked regardless of whether they +exist or not, and will not invoke any PHP processes. +
    + +### Configuring: Protected file headers + +In certain situations, it's necessary to customise HTTP headers required either by +intermediary caching services, or by the client, or upstream caches. + +When a protected file is served it will also be transmitted with all headers defined by the +`SilverStripe\Filesystem\Flysystem\FlysystemAssetStore.file_response_headers` config. +You can customise this with the below config: + + + :::yaml + SilverStripe\Filesystem\Flysystem\FlysystemAssetStore: + file_response_headers: + Pragma: 'no-cache' + + +### Configuring: Archive behaviour + +By default, the default extension `AssetControlExtension` will control the disposal of assets +attached to objects when those objects are deleted. For example, unpublished versioned objects +will automatically have their attached assets moved to the protected store. The deletion of +draft or unversioned objects will have those assets permanantly deleted (along with all variants). + +In some cases, it may be preferable to have any deleted assets archived for versioned dataobjects, +rather than deleted. This uses more disk storage, but will allow the full recovery of archived +records and files. + +This can be applied to DataObjects on a case by case basis by setting the `archive_assets` +config to true on that class. Note that this feature only works with dataobjects with +the `Versioned` extension. + + + :::php + class MyVersiondObject extends DataObject { + /** Enable archiving */ + private static $archive_assets = true; + /** Versioned */ + private static $extensions = array('Versioned'); + } + + +The extension can also be globally disabled by removing it at the root level: + + + :::yaml + DataObject: + AssetControl: null + + +### Configuring: Web server settings + +If the default server configuration is not appropriate for your specific environment, then you can +further customise the .htaccess or web.config by editing one or more of the below: + +* `Assets_HTAccess.ss`: Template for public permissions on the Apache server. +* `Assets_WebConfig.ss`: Template for public permissions on the IIS server. +* `Protected_HTAccess.ss`: Template for the protected store on the Apache server (should deny all requests). +* `Protected_WebConfig.ss`: Template for the protected store on the IIS server (should deny all requests). + +Each of these files will be regenerated on ?flush, so it is important to ensure that these files are +overridden at the template level, not via manually generated configuration files. + +#### Configuring Web Server: Apache server + +In order to ensure that public files are served correctly, you should check that your ./assets +.htaccess bypasses PHP requests for files that do exist. The default template +(declared by `Assets_HTAccess.ss`) has the following section, which may be customised in your project: + + + # Non existant files passed to requesthandler + RewriteCond %{REQUEST_URI} ^(.*)$ + RewriteCond %{REQUEST_FILENAME} !-f + RewriteRule .* ../framework/main.php?url=%1 [QSA] + + +You will need to ensure that your core apache configuration has the necessary `AllowOverride` +settings to support the local .htaccess file. + +#### Configuring Web Server: Windows IIS 7.5+ + +Configuring via IIS requires the Rewrite extension to be installed and configured properly. +Any rules declared for the assets folder should be able to dynamically serve up existing files, +while ensuring non-existent files are processed via the Framework. + +The default rule for IIS is as below (only partial configuration displayed): + + + + + + + + + + + +You will need to make sure that the `allowOverride` property of your root web.config is not set +to false, to allow these to take effect. + +#### Configuring Web Server: Other server types + +If using a server configuration which must be configured outside of the web or asset root, you +will need to make sure you manually configure these rules. + +For instance, this will allow your nginx site to serve files directly, while ensuring +dynamic requests are processed via the Framework: + + + ::: + location ^~ /assets/ { + sendfile on; + try_files $uri /framework/main.php?url=$uri&$query_string; + } + diff --git a/docs/en/04_Changelogs/4.0.0.md b/docs/en/04_Changelogs/4.0.0.md index 2e7933127..22a005809 100644 --- a/docs/en/04_Changelogs/4.0.0.md +++ b/docs/en/04_Changelogs/4.0.0.md @@ -21,6 +21,8 @@ urls are no longer able to identify assets. * Extension point `HtmlEditorField::processImage` has been removed, and moved to `Image::regenerateImageHTML` * `Upload::load` now stores assets directly without saving into a `File` dataobject. + * Protected file storage is now a core Framework API. See [/developer_guides/files/file_security] for + more information. ## New API @@ -33,6 +35,9 @@ * `Requirements_Minifier` API can be used to declare any new mechanism for minifying combined required files. By default this api is provided by the `JSMinifier` class, but user code can substitute their own. * `AssetField` formfield to provide an `UploadField` style uploader for the new `DBFile` database field. + * `AssetControlExtension` is applied by default to all DataObjects, in order to support the management + of linked assets and file protection. + * `ProtectedFileController` class is used to serve up protected assets. ## Deprecated classes/methods @@ -461,3 +466,19 @@ E.g. extensions: - MyErrorPageExtension +### Upgrading asset web.config, .htaccess, or other server configuration + +Server configuration files for /assets are no longer static, and are regenerated via a set of +standard silverstripe templates on flush. These templates include: + + * `Assets_HTAccess.ss`: Template for public permissions on the Apache server. + * `Assets_WebConfig.ss`: Template for public permissions on the IIS server. + * `Protected_HTAccess.ss`: Template for the protected store on the Apache server (should deny all requests). + * `Protected_WebConfig.ss`: Template for the protected store on the IIS server (should deny all requests). + +You will need to make sure that these files are writable via the web server, and that any necessary +configuration customisation is done via overriding these templates. + +If upgrading from an existing installation, make sure to invoke ?flush=all at least once. + +See [/developer_guides/files/file_security] for more information. diff --git a/filesystem/AssetAdapterTest.php b/filesystem/AssetAdapterTest.php new file mode 100644 index 000000000..d83ec6360 --- /dev/null +++ b/filesystem/AssetAdapterTest.php @@ -0,0 +1,69 @@ +rootDir = ASSETS_PATH . '/AssetAdapterTest'; + Filesystem::makeFolder($this->rootDir); + $this->originalServer = $_SERVER; + } + + public function tearDown() { + if($this->rootDir) { + Filesystem::removeFolder($this->rootDir); + $this->rootDir = null; + } + if($this->originalServer) { + $_SERVER = $this->originalServer; + $this->originalServer = null; + } + parent::tearDown(); + } + + public function testPublicAdapter() { + $_SERVER['SERVER_SOFTWARE'] = 'Apache/2.2.22 (Win64) PHP/5.3.13'; + $adapter = new PublicAssetAdapter($this->rootDir); + $this->assertFileExists($this->rootDir . '/.htaccess'); + $this->assertFileNotExists($this->rootDir . '/web.config'); + + $htaccess = $adapter->read('.htaccess'); + $content = $htaccess['contents']; + // Allowed extensions set + $this->assertContains('RewriteCond %{REQUEST_URI} !.(?i:', $content); + foreach(File::config()->allowed_extensions as $extension) { + $this->assertRegExp('/\b'.preg_quote($extension).'\b/', $content); + } + + // Rewrite rules + $this->assertContains('RewriteRule .* ../framework/main.php?url=%1 [QSA]', $content); + $this->assertContains('RewriteRule error[^\\/]*.html$ - [L]', $content); + + // Test flush restores invalid content + \file_put_contents($this->rootDir . '/.htaccess', '# broken content'); + $adapter->flush(); + $htaccess2 = $adapter->read('.htaccess'); + $this->assertEquals($content, $htaccess2['contents']); + + // Test URL + $this->assertEquals('/assets/AssetAdapterTest/file.jpg', $adapter->getPublicUrl('file.jpg')); + } + + public function testProtectedAdapter() { + $_SERVER['SERVER_SOFTWARE'] = 'Apache/2.2.22 (Win64) PHP/5.3.13'; + $adapter = new ProtectedAssetAdapter($this->rootDir . '/.protected'); + $this->assertFileExists($this->rootDir . '/.protected/.htaccess'); + $this->assertFileNotExists($this->rootDir . '/.protected/web.config'); + + // Test url + $this->assertEquals('/assets/file.jpg', $adapter->getProtectedUrl('file.jpg')); + } +} \ No newline at end of file diff --git a/filesystem/AssetControlExtension.php b/filesystem/AssetControlExtension.php new file mode 100644 index 000000000..634987590 --- /dev/null +++ b/filesystem/AssetControlExtension.php @@ -0,0 +1,113 @@ +findAssets($this->owner); + + // When deleting from live, just secure assets + // Note that DataObject::delete() ignores sourceQueryParams + if($this->isVersioned() && \Versioned::current_stage() === \Versioned::get_live_stage()) { + $this->protectAll($assets); + return; + } + + // When deleting from stage then check if we should archive assets + $archive = $this->owner->config()->archive_assets; + if($archive && $this->isVersioned()) { + // Archived assets are kept protected + $this->protectAll($assets); + } else { + // Otherwise remove all assets + $this->deleteAll($assets); + } + } + + /** + * Return a list of all tuples attached to this dataobject + * Note: Variants are excluded + * + * @param DataObject $record to search + * @return array + */ + protected function findAssets(DataObject $record) { + // Search for dbfile instances + $files = array(); + foreach($record->db() as $field => $db) { + // Extract assets from this database field + list($dbClass) = explode('(', $db); + if(!is_a($dbClass, 'DBFile', true)) { + continue; + } + + // Omit variant and merge with set + $next = $record->dbObject($field)->getValue(); + unset($next['Variant']); + if ($next) { + $files[] = $next; + } + } + + // De-dupe + return array_map("unserialize", array_unique(array_map("serialize", $files))); + } + + /** + * Determine if versioning rules should be applied to this object + * + * @return bool + */ + protected function isVersioned() { + return $this->owner->has_extension('Versioned'); + } + + /** + * Delete all assets in the tuple list + * + * @param array $assets + */ + protected function deleteAll($assets) { + $store = $this->getAssetStore(); + foreach($assets as $asset) { + $store->delete($asset['Filename'], $asset['Hash']); + } + } + + /** + * Move all assets in the list to the protected store + * + * @param array $assets + */ + protected function protectAll($assets) { + $store = $this->getAssetStore(); + foreach($assets as $asset) { + $store->protect($asset['Filename'], $asset['Hash']); + } + } + + /** + * @return AssetStore + */ + protected function getAssetStore() { + return Injector::inst()->get('AssetStore'); + } + +} diff --git a/filesystem/File.php b/filesystem/File.php index 06f5e1337..dd69d3854 100644 --- a/filesystem/File.php +++ b/filesystem/File.php @@ -667,22 +667,24 @@ class File extends DataObject implements ShortcodeHandler, AssetContainer { * Gets the URL of this file * * @uses Director::baseURL() + * @param bool $grant Ensures that the url for any protected assets is granted for the current user. * @return string */ - public function getURL() { + public function getURL($grant = true) { if($this->File->exists()) { - return $this->File->getURL(); + return $this->File->getURL($grant); } } /** * Get URL, but without resampling. * + * @param bool $grant Ensures that the url for any protected assets is granted for the current user. * @return string */ - public function getSourceURL() { + public function getSourceURL($grant = true) { if($this->File->exists()) { - return $this->File->getSourceURL(); + return $this->File->getSourceURL($grant); } } @@ -971,8 +973,8 @@ class File extends DataObject implements ShortcodeHandler, AssetContainer { } } - public function setFromLocalFile($path, $filename = null, $hash = null, $variant = null, $conflictResolution = null) { - $result = $this->File->setFromLocalFile($path, $filename, $hash, $variant, $conflictResolution); + public function setFromLocalFile($path, $filename = null, $hash = null, $variant = null, $config = array()) { + $result = $this->File->setFromLocalFile($path, $filename, $hash, $variant, $config); // Update File record to name of the uploaded asset if($result) { @@ -981,8 +983,8 @@ class File extends DataObject implements ShortcodeHandler, AssetContainer { return $result; } - public function setFromStream($stream, $filename, $hash = null, $variant = null, $conflictResolution = null) { - $result = $this->File->setFromStream($stream, $filename, $hash, $variant, $conflictResolution); + public function setFromStream($stream, $filename, $hash = null, $variant = null, $config = array()) { + $result = $this->File->setFromStream($stream, $filename, $hash, $variant, $config); // Update File record to name of the uploaded asset if($result) { @@ -991,8 +993,8 @@ class File extends DataObject implements ShortcodeHandler, AssetContainer { return $result; } - public function setFromString($data, $filename, $hash = null, $variant = null, $conflictResolution = null) { - $result = $this->File->setFromString($data, $filename, $hash, $variant, $conflictResolution); + public function setFromString($data, $filename, $hash = null, $variant = null, $config = array()) { + $result = $this->File->setFromString($data, $filename, $hash, $variant, $config); // Update File record to name of the uploaded asset if($result) { @@ -1063,7 +1065,7 @@ class File extends DataObject implements ShortcodeHandler, AssetContainer { if(count($args) === 1 && is_array($args[0])) { $args = $args[0]; } - + $parts = array(); foreach($args as $arg) { $part = trim($arg, ' \\/'); @@ -1075,4 +1077,31 @@ class File extends DataObject implements ShortcodeHandler, AssetContainer { return implode('/', $parts); } + public function deleteFile() { + return $this->File->deleteFile(); + } + + public function getVisibility() { + return $this->File->getVisibility(); + } + + public function publishFile() { + $this->File->publishFile(); + } + + public function protectFile() { + $this->File->protectFile(); + } + + public function grantFile() { + $this->File->grantFile(); + } + + public function revokeFile() { + $this->File->revokeFile(); + } + + public function canViewFile() { + return $this->File->canViewFile(); + } } diff --git a/filesystem/FileMigrationHelper.php b/filesystem/FileMigrationHelper.php index 9f59c0714..681eb46ba 100644 --- a/filesystem/FileMigrationHelper.php +++ b/filesystem/FileMigrationHelper.php @@ -63,7 +63,10 @@ class FileMigrationHelper extends Object { // Copy local file into this filesystem $filename = $file->getFilename(); - $result = $file->setFromLocalFile($path, $filename, null, null, AssetStore::CONFLICT_OVERWRITE); + $result = $file->setFromLocalFile( + $path, $filename, null, null, + array('conflict' => AssetStore::CONFLICT_OVERWRITE) + ); // Move file if the APL changes filename value if($result['Filename'] !== $filename) { diff --git a/filesystem/Folder.php b/filesystem/Folder.php index 9251e55eb..6ee808942 100644 --- a/filesystem/Folder.php +++ b/filesystem/Folder.php @@ -239,9 +239,10 @@ class Folder extends File { /** * Folders do not have public URLs * - * @return null + * @param bool $grant + * @return null|string */ - public function getURL() { + public function getURL($grant = true) { return null; } diff --git a/filesystem/GDBackend.php b/filesystem/GDBackend.php index 9d37d9d3c..a580df013 100644 --- a/filesystem/GDBackend.php +++ b/filesystem/GDBackend.php @@ -548,14 +548,14 @@ class GDBackend extends Object implements Image_Backend, Flushable { return $output; } - public function writeToStore(AssetStore $assetStore, $filename, $hash = null, $variant = null, $conflictResolution = null) { + public function writeToStore(AssetStore $assetStore, $filename, $hash = null, $variant = null, $config = array()) { // Write to temporary file, taking care to maintain the extension $path = tempnam(sys_get_temp_dir(), 'gd'); if($extension = pathinfo($filename, PATHINFO_EXTENSION)) { $path .= "." . $extension; } $this->writeTo($path); - $result = $assetStore->setFromLocalFile($path, $filename, $hash, $variant, $conflictResolution); + $result = $assetStore->setFromLocalFile($path, $filename, $hash, $variant, $config); unlink($path); return $result; } diff --git a/filesystem/ImageManipulation.php b/filesystem/ImageManipulation.php index ee4f65b62..4614bc1c9 100644 --- a/filesystem/ImageManipulation.php +++ b/filesystem/ImageManipulation.php @@ -54,9 +54,10 @@ trait ImageManipulation { abstract public function getStream(); /** + * @param bool $grant Ensures that the url for any protected assets is granted for the current user. * @return string public url to the asset in this container */ - abstract public function getURL(); + abstract public function getURL($grant = true); /** * @return string The absolute URL to the asset in this container @@ -692,7 +693,10 @@ trait ImageManipulation { return null; } - return $backend->writeToStore($store, $filename, $hash, $variant, AssetStore::CONFLICT_USE_EXISTING); + return $backend->writeToStore( + $store, $filename, $hash, $variant, + array('conflict' => AssetStore::CONFLICT_USE_EXISTING) + ); } ); } diff --git a/filesystem/ImagickBackend.php b/filesystem/ImagickBackend.php index ba4e548b2..5f796ba93 100644 --- a/filesystem/ImagickBackend.php +++ b/filesystem/ImagickBackend.php @@ -49,14 +49,14 @@ class ImagickBackend extends Imagick implements Image_Backend { $this->setQuality(Config::inst()->get('ImagickBackend', 'default_quality')); } - public function writeToStore(AssetStore $assetStore, $filename, $hash = null, $variant = null, $conflictResolution = null) { + public function writeToStore(AssetStore $assetStore, $filename, $hash = null, $variant = null, $config = array()) { // Write to temporary file, taking care to maintain the extension $path = tempnam(sys_get_temp_dir(), 'imagemagick'); if($extension = pathinfo($filename, PATHINFO_EXTENSION)) { $path .= "." . $extension; } $this->writeimage($path); - $result = $assetStore->setFromLocalFile($path, $filename, $hash, $variant, $conflictResolution); + $result = $assetStore->setFromLocalFile($path, $filename, $hash, $variant, $config); unlink($path); return $result; } diff --git a/filesystem/Upload.php b/filesystem/Upload.php index 76e574c4b..1d6e4ab36 100644 --- a/filesystem/Upload.php +++ b/filesystem/Upload.php @@ -198,7 +198,8 @@ class Upload extends Controller { $conflictResolution = $this->replaceFile ? AssetStore::CONFLICT_OVERWRITE : AssetStore::CONFLICT_RENAME; - return $container->setFromLocalFile($tmpFile['tmp_name'], $filename, null, null, $conflictResolution); + $config = array('conflict' => $conflictResolution); + return $container->setFromLocalFile($tmpFile['tmp_name'], $filename, null, null, $config); } /** diff --git a/filesystem/flysystem/AssetAdapter.php b/filesystem/flysystem/AssetAdapter.php index ac9fadd31..03273e2f4 100644 --- a/filesystem/flysystem/AssetAdapter.php +++ b/filesystem/flysystem/AssetAdapter.php @@ -2,18 +2,24 @@ namespace SilverStripe\Filesystem\Flysystem; -use Controller; -use Director; use League\Flysystem\Adapter\Local; /** - * Adaptor for local filesystem based on assets directory + * Adapter for local filesystem based on assets directory * * @package framework * @subpackage filesystem */ class AssetAdapter extends Local { + /** + * Server specific configuration necessary to block http traffic to a local folder + * + * @config + * @var array Mapping of server configurations to configuration files necessary + */ + private static $server_configuration = array(); + /** * Config compatible permissions configuration * @@ -33,39 +39,101 @@ class AssetAdapter extends Local { public function __construct($root = null, $writeFlags = LOCK_EX, $linkHandling = self::DISALLOW_LINKS) { // Get root path - if (!$root) { - // Empty root will set the path to assets - $root = ASSETS_PATH; - } elseif(strpos($root, './') === 0) { - // Substitute leading ./ with BASE_PATH - $root = BASE_PATH . substr($root, 1); - } elseif(strpos($root, '../') === 0) { - // Substitute leading ./ with parent of BASE_PATH, in case storage is outside of the webroot. - $root = dirname(BASE_PATH) . substr($root, 2); - } + $root = $this->findRoot($root); // Override permissions with config $permissions = \Config::inst()->get(get_class($this), 'file_permissions'); - parent::__construct($root, $writeFlags, $linkHandling, $permissions); + + // Configure server + $this->configureServer(); } /** - * Provide downloadable url + * Determine the root folder absolute system path * - * @param string $path - * @return string|null + * @param string $root + * @return string */ - public function getPublicUrl($path) { - $rootPath = realpath(BASE_PATH); - $filesPath = realpath($this->pathPrefix); - - if(stripos($filesPath, $rootPath) === 0) { - $dir = substr($filesPath, strlen($rootPath)); - return Controller::join_links(Director::baseURL(), $dir, $path); + protected function findRoot($root) { + // Empty root will set the path to assets + if (!$root) { + throw new \InvalidArgumentException("Missing argument for root path"); } - // File outside of webroot can't be used - return null; + // Substitute leading ./ with BASE_PATH + if(strpos($root, './') === 0) { + return BASE_PATH . substr($root, 1); + } + + // Substitute leading ./ with parent of BASE_PATH, in case storage is outside of the webroot. + if(strpos($root, '../') === 0) { + return dirname(BASE_PATH) . substr($root, 2); + } + + return $root; } + + /** + * Force flush and regeneration of server files + */ + public function flush() { + $this->configureServer(true); + } + + /** + * Configure server files for this store + * + * @param bool $forceOverwrite Force regeneration even if files already exist + */ + protected function configureServer($forceOverwrite = false) { + // Get server type + $type = isset($_SERVER['SERVER_SOFTWARE']) ? $_SERVER['SERVER_SOFTWARE'] : '*'; + list($type) = explode('/', strtolower($type)); + + // Determine configurations to write + $rules = \Config::inst()->get(get_class($this), 'server_configuration', \Config::FIRST_SET); + if(empty($rules[$type])) { + return; + } + $configurations = $rules[$type]; + + // Apply each configuration + $config = new \League\Flysystem\Config(); + $config->set('visibility', 'private'); + foreach($configurations as $file => $template) { + if ($forceOverwrite || !$this->has($file)) { + // Evaluate file + $content = $this->renderTemplate($template); + $success = $this->write($file, $content, $config); + if(!$success) { + throw new \Exception("Error writing server configuration file \"{$file}\""); + } + } + } + } + + /** + * Render server configuration file from a template file + * + * @param string $template + * @return \HTMLText Rendered results + */ + protected function renderTemplate($template) { + // Build allowed extensions + $allowedExtensions = new \ArrayList(); + foreach(\File::config()->allowed_extensions as $extension) { + if($extension) { + $allowedExtensions->push(new \ArrayData(array( + 'Extension' => preg_quote($extension) + ))); + } + } + + $viewer = new \SSViewer(array($template)); + return (string)$viewer->process(new \ArrayData(array( + 'AllowedExtensions' => $allowedExtensions + ))); + } + } diff --git a/filesystem/flysystem/FlysystemAssetStore.php b/filesystem/flysystem/FlysystemAssetStore.php index f4b4c8212..d80792394 100644 --- a/filesystem/flysystem/FlysystemAssetStore.php +++ b/filesystem/flysystem/FlysystemAssetStore.php @@ -3,13 +3,20 @@ namespace SilverStripe\Filesystem\Flysystem; use Config; +use Generator; use Injector; +use Session; +use Flushable; use InvalidArgumentException; +use League\Flysystem\Directory; use League\Flysystem\Exception; use League\Flysystem\Filesystem; +use League\Flysystem\FilesystemInterface; use League\Flysystem\Util; use SilverStripe\Filesystem\Storage\AssetNameGenerator; use SilverStripe\Filesystem\Storage\AssetStore; +use SilverStripe\Filesystem\Storage\AssetStoreRouter; +use SS_HTTPResponse; /** * Asset store based on flysystem Filesystem as a backend @@ -17,29 +24,83 @@ use SilverStripe\Filesystem\Storage\AssetStore; * @package framework * @subpackage filesystem */ -class FlysystemAssetStore implements AssetStore { +class FlysystemAssetStore implements AssetStore, AssetStoreRouter, Flushable { + + /** + * Session key to use for user grants + */ + const GRANTS_SESSION = 'AssetStore_Grants'; /** * @var Filesystem */ - private $filesystem = null; + private $publicFilesystem = null; + + /** + * Filesystem to use for protected files + * + * @var Filesystem + */ + private $protectedFilesystem = null; /** * Enable to use legacy filename behaviour (omits hash) * + * Note that if using legacy filenames then duplicate files will not work. + * * @config * @var bool */ private static $legacy_filenames = false; + /** + * Flag if empty folders are allowed. + * If false, empty folders are cleared up when their contents are deleted. + * + * @config + * @var bool + */ + private static $keep_empty_dirs = false; + + /** + * Set HTTP error code for requests to secure denied assets. + * Note that this defaults to 404 to prevent information disclosure + * of secure files + * + * @config + * @var int + */ + private static $denied_response_code = 404; + + /** + * Set HTTP error code to use for missing secure assets + * + * @config + * @var int + */ + private static $missing_response_code = 404; + + /** + * Custom headers to add to all custom file responses + * + * @config + * @var array + */ + private static $file_response_headers = array( + 'Cache-Control' => 'private' + ); + /** * Assign new flysystem backend * * @param Filesystem $filesystem * @return $this */ - public function setFilesystem(Filesystem $filesystem) { - $this->filesystem = $filesystem; + public function setPublicFilesystem(Filesystem $filesystem) { + if(!$filesystem->getAdapter() instanceof PublicAdapter) { + throw new \InvalidArgumentException("Configured adapter must implement PublicAdapter"); + } + $this->publicFilesystem = $filesystem; return $this; } @@ -48,26 +109,115 @@ class FlysystemAssetStore implements AssetStore { * * @return Filesystem */ - public function getFilesystem() { - return $this->filesystem; + public function getPublicFilesystem() { + return $this->publicFilesystem; } + /** + * Assign filesystem to use for non-public files + * + * @param Filesystem $filesystem + * @return $this + */ + public function setProtectedFilesystem(Filesystem $filesystem) { + if(!$filesystem->getAdapter() instanceof ProtectedAdapter) { + throw new \InvalidArgumentException("Configured adapter must implement ProtectedAdapter"); + } + $this->protectedFilesystem = $filesystem; + return $this; + } + + /** + * Get filesystem to use for non-public files + * + * @return Filesystem + */ + public function getProtectedFilesystem() { + return $this->protectedFilesystem; + } + + /** + * Return the store that contains the given fileID + * + * @param string $fileID Internal file identifier + * @return Filesystem + */ + protected function getFilesystemFor($fileID) { + if($this->getPublicFilesystem()->has($fileID)) { + return $this->getPublicFilesystem(); + } + + if($this->getProtectedFilesystem()->has($fileID)) { + return $this->getProtectedFilesystem(); + } + + return null; + } + + public function getCapabilities() { + return array( + 'visibility' => array( + self::VISIBILITY_PUBLIC, + self::VISIBILITY_PROTECTED + ), + 'conflict' => array( + self::CONFLICT_EXCEPTION, + self::CONFLICT_OVERWRITE, + self::CONFLICT_RENAME, + self::CONFLICT_USE_EXISTING + ) + ); + } + + public function getVisibility($filename, $hash) { + $fileID = $this->getFileID($filename, $hash); + if($this->getPublicFilesystem()->has($fileID)) { + return self::VISIBILITY_PUBLIC; + } + + if($this->getProtectedFilesystem()->has($fileID)) { + return self::VISIBILITY_PROTECTED; + } + + return null; + } + + public function getAsStream($filename, $hash, $variant = null) { $fileID = $this->getFileID($filename, $hash, $variant); - return $this->getFilesystem()->readStream($fileID); + return $this + ->getFilesystemFor($fileID) + ->readStream($fileID); } public function getAsString($filename, $hash, $variant = null) { $fileID = $this->getFileID($filename, $hash, $variant); - return $this->getFilesystem()->read($fileID); + return $this + ->getFilesystemFor($fileID) + ->read($fileID); } - public function getAsURL($filename, $hash, $variant = null) { + public function getAsURL($filename, $hash, $variant = null, $grant = true) { + if($grant) { + $this->grant($filename, $hash); + } $fileID = $this->getFileID($filename, $hash, $variant); - return $this->getFilesystem()->getPublicUrl($fileID); + + // Check with filesystem this asset exists in + $public = $this->getPublicFilesystem(); + $protected = $this->getProtectedFilesystem(); + if($public->has($fileID) || !$protected->has($fileID)) { + /** @var PublicAdapter $publicAdapter */ + $publicAdapter = $public->getAdapter(); + return $publicAdapter->getPublicUrl($fileID); + } else { + /** @var ProtectedAdapter $protectedAdapter */ + $protectedAdapter = $protected->getAdapter(); + return $protectedAdapter->getProtectedUrl($fileID); + } } - public function setFromLocalFile($path, $filename = null, $hash = null, $variant = null, $conflictResolution = null) { + public function setFromLocalFile($path, $filename = null, $hash = null, $variant = null, $config = array()) { // Validate this file exists if(!file_exists($path)) { throw new InvalidArgumentException("$path does not exist"); @@ -79,8 +229,7 @@ class FlysystemAssetStore implements AssetStore { } // Callback for saving content - $filesystem = $this->getFilesystem(); - $callback = function($fileID) use ($filesystem, $path) { + $callback = function(Filesystem $filesystem, $fileID) use ($path) { // Read contents as string into flysystem $handle = fopen($path, 'r'); if($handle === false) { @@ -97,13 +246,12 @@ class FlysystemAssetStore implements AssetStore { } // Submit to conflict check - return $this->writeWithCallback($callback, $filename, $hash, $variant, $conflictResolution); + return $this->writeWithCallback($callback, $filename, $hash, $variant, $config); } - public function setFromString($data, $filename, $hash = null, $variant = null, $conflictResolution = null) { + public function setFromString($data, $filename, $hash = null, $variant = null, $config = array()) { // Callback for saving content - $filesystem = $this->getFilesystem(); - $callback = function($fileID) use ($filesystem, $data) { + $callback = function(Filesystem $filesystem, $fileID) use ($data) { return $filesystem->put($fileID, $data); }; @@ -113,21 +261,20 @@ class FlysystemAssetStore implements AssetStore { } // Submit to conflict check - return $this->writeWithCallback($callback, $filename, $hash, $variant, $conflictResolution); + return $this->writeWithCallback($callback, $filename, $hash, $variant, $config); } - public function setFromStream($stream, $filename, $hash = null, $variant = null, $conflictResolution = null) { + public function setFromStream($stream, $filename, $hash = null, $variant = null, $config = array()) { // If the stream isn't rewindable, write to a temporary filename if(!$this->isSeekableStream($stream)) { $path = $this->getStreamAsFile($stream); - $result = $this->setFromLocalFile($path, $filename, $hash, $variant, $conflictResolution); + $result = $this->setFromLocalFile($path, $filename, $hash, $variant, $config); unlink($path); return $result; } // Callback for saving content - $filesystem = $this->getFilesystem(); - $callback = function($fileID) use ($filesystem, $stream) { + $callback = function(Filesystem $filesystem, $fileID) use ($stream) { return $filesystem->putStream($fileID, $stream); }; @@ -137,7 +284,144 @@ class FlysystemAssetStore implements AssetStore { } // Submit to conflict check - return $this->writeWithCallback($callback, $filename, $hash, $variant, $conflictResolution); + return $this->writeWithCallback($callback, $filename, $hash, $variant, $config); + } + + public function delete($filename, $hash) { + $fileID = $this->getFileID($filename, $hash); + $protected = $this->deleteFromFilesystem($fileID, $this->getProtectedFilesystem()); + $public = $this->deleteFromFilesystem($fileID, $this->getPublicFilesystem()); + return $protected || $public; + } + + /** + * Delete the given file (and any variants) in the given {@see Filesystem} + * + * @param string $fileID + * @param Filesystem $filesystem + * @return bool True if a file was deleted + */ + protected function deleteFromFilesystem($fileID, Filesystem $filesystem) { + $deleted = false; + foreach($this->findVariants($fileID, $filesystem) as $nextID) { + $filesystem->delete($nextID); + $deleted = true; + } + + // Truncate empty dirs + $this->truncateDirectory(dirname($fileID), $filesystem); + + return $deleted; + } + + /** + * Clear directory if it's empty + * + * @param string $dirname Name of directory + * @param Filesystem $filesystem + */ + protected function truncateDirectory($dirname, Filesystem $filesystem) { + if ($dirname + && ! Config::inst()->get(get_class($this), 'keep_empty_dirs') + && ! $filesystem->listContents($dirname) + ) { + $filesystem->deleteDir($dirname); + } + } + + /** + * Returns an iterable {@see Generator} of all files / variants for the given $fileID in the given $filesystem + * This includes the empty (no) variant. + * + * @param string $fileID ID of original file to compare with. + * @param Filesystem $filesystem + * @return Generator + */ + protected function findVariants($fileID, Filesystem $filesystem) { + foreach($filesystem->listContents(dirname($fileID)) as $next) { + if($next['type'] !== 'file') { + continue; + } + $nextID = $next['path']; + // Compare given file to target, omitting variant + if($fileID === $this->removeVariant($nextID)) { + yield $nextID; + } + } + } + + public function publish($filename, $hash) { + $fileID = $this->getFileID($filename, $hash); + $protected = $this->getProtectedFilesystem(); + $public = $this->getPublicFilesystem(); + $this->moveBetweenFilesystems($fileID, $protected, $public); + } + + public function protect($filename, $hash) { + $fileID = $this->getFileID($filename, $hash); + $public = $this->getPublicFilesystem(); + $protected = $this->getProtectedFilesystem(); + $this->moveBetweenFilesystems($fileID, $public, $protected); + } + + /** + * Move a file (and its associative variants) between filesystems + * + * @param string $fileID + * @param Filesystem $from + * @param Filesystem $to + */ + protected function moveBetweenFilesystems($fileID, Filesystem $from, Filesystem $to) { + foreach($this->findVariants($fileID, $from) as $nextID) { + // Copy via stream + $stream = $from->readStream($nextID); + $to->putStream($nextID, $stream); + fclose($stream); + $from->delete($nextID); + } + + // Truncate empty dirs + $this->truncateDirectory(dirname($fileID), $from); + } + + public function grant($filename, $hash) { + $fileID = $this->getFileID($filename, $hash); + $granted = Session::get(self::GRANTS_SESSION) ?: array(); + $granted[$fileID] = true; + Session::set(self::GRANTS_SESSION, $granted); + } + + public function revoke($filename, $hash) { + $fileID = $this->getFileID($filename, $hash); + $granted = Session::get(self::GRANTS_SESSION) ?: array(); + unset($granted[$fileID]); + if($granted) { + Session::set(self::GRANTS_SESSION, $granted); + } else { + Session::clear(self::GRANTS_SESSION); + } + } + + public function canView($filename, $hash) { + $fileID = $this->getFileID($filename, $hash); + if($this->getProtectedFilesystem()->has($fileID)) { + return $this->isGranted($fileID); + } + return true; + } + + /** + * Determine if a grant exists for the given FileID + * + * @param string $fileID + * @return bool + */ + protected function isGranted($fileID) { + // Since permissions are applied to the non-variant only, + // map back to the original file before checking + $originalID = $this->removeVariant($fileID); + $granted = Session::get(self::GRANTS_SESSION) ?: array(); + return !empty($granted[$originalID]); } /** @@ -158,21 +442,22 @@ class FlysystemAssetStore implements AssetStore { * * @param resource $stream * @return string Filename of resulting stream content + * @throws Exception */ protected function getStreamAsFile($stream) { // Get temporary file and name $file = tempnam(sys_get_temp_dir(), 'ssflysystem'); $buffer = fopen($file, 'w'); - if (!$buffer) { - throw new Exception("Could not create temporary file"); - } + if (!$buffer) { + throw new Exception("Could not create temporary file"); + } // Transfer from given stream Util::rewindStream($stream); - stream_copy_to_stream($stream, $buffer); - if (! fclose($buffer)) { - throw new Exception("Could not write stream to temporary file"); - } + stream_copy_to_stream($stream, $buffer); + if (! fclose($buffer)) { + throw new Exception("Could not write stream to temporary file"); + } return $file; } @@ -195,14 +480,16 @@ class FlysystemAssetStore implements AssetStore { * @param string $filename Name for the resulting file * @param string $hash SHA1 of the original file content * @param string $variant Variant to write - * @param string $conflictResolution {@see AssetStore}. Will default to one chosen by the backend + * @param array $config Write options. {@see AssetStore} * @return array Tuple associative array (Filename, Hash, Variant) * @throws Exception */ - protected function writeWithCallback($callback, $filename, $hash, $variant = null, $conflictResolution = null) { + protected function writeWithCallback($callback, $filename, $hash, $variant = null, $config = array()) { // Set default conflict resolution - if(!$conflictResolution) { + if(empty($config['conflict'])) { $conflictResolution = $this->getDefaultConflictResolution($variant); + } else { + $conflictResolution = $config['conflict']; } // Validate parameters @@ -223,8 +510,22 @@ class FlysystemAssetStore implements AssetStore { // Check conflict resolution scheme $resolvedID = $this->resolveConflicts($conflictResolution, $fileID); if($resolvedID !== false) { + // Check if source file already exists on the filesystem + $mainID = $this->getFileID($filename, $hash); + $filesystem = $this->getFilesystemFor($mainID); + + // If writing a new file use the correct visibility + if(!$filesystem) { + // Default to public store unless requesting protected store + if(isset($config['visibility']) && $config['visibility'] === self::VISIBILITY_PROTECTED) { + $filesystem = $this->getProtectedFilesystem(); + } else { + $filesystem = $this->getPublicFilesystem(); + } + } + // Submit and validate result - $result = $callback($resolvedID); + $result = $callback($filesystem, $resolvedID); if(!$result) { throw new Exception("Could not save {$filename}"); } @@ -233,10 +534,10 @@ class FlysystemAssetStore implements AssetStore { $filename = $this->getOriginalFilename($resolvedID); } elseif(empty($variant)) { - // If defering to the existing file, return the sha of the existing file, + // If deferring to the existing file, return the sha of the existing file, // unless we are writing a variant (which has the same hash value as its original file) $stream = $this - ->getFilesystem() + ->getFilesystemFor($fileID) ->readStream($fileID); $hash = $this->getStreamSHA1($stream); } @@ -277,17 +578,26 @@ class FlysystemAssetStore implements AssetStore { public function getMetadata($filename, $hash, $variant = null) { $fileID = $this->getFileID($filename, $hash, $variant); - return $this->getFilesystem()->getMetadata($fileID); + $filesystem = $this->getFilesystemFor($fileID); + if($filesystem) { + return $filesystem->getMetadata($fileID); + } + return null; } public function getMimeType($filename, $hash, $variant = null) { $fileID = $this->getFileID($filename, $hash, $variant); - return $this->getFilesystem()->getMimetype($fileID); + $filesystem = $this->getFilesystemFor($fileID); + if($filesystem) { + return $filesystem->getMimetype($fileID); + } + return null; } public function exists($filename, $hash, $variant = null) { $fileID = $this->getFileID($filename, $hash, $variant); - return $this->getFilesystem()->has($fileID); + $filesystem = $this->getFilesystemFor($fileID); + return !empty($filesystem); } /** @@ -305,7 +615,7 @@ class FlysystemAssetStore implements AssetStore { } // Otherwise, check if this exists - $exists = $this->getFilesystem()->has($fileID); + $exists = $this->getFilesystemFor($fileID); if(!$exists) { return $fileID; } @@ -313,15 +623,14 @@ class FlysystemAssetStore implements AssetStore { // Flysystem defaults to use_existing switch($conflictResolution) { // Throw tantrum - case AssetStore::CONFLICT_EXCEPTION: { + case static::CONFLICT_EXCEPTION: { throw new \InvalidArgumentException("File already exists at path {$fileID}"); } // Rename - case AssetStore::CONFLICT_RENAME: { + case static::CONFLICT_RENAME: { foreach($this->fileGeneratorFor($fileID) as $candidate) { - // @todo better infinite loop breaking - if(!$this->getFilesystem()->has($candidate)) { + if(!$this->getFilesystemFor($candidate)) { return $candidate; } } @@ -330,7 +639,7 @@ class FlysystemAssetStore implements AssetStore { } // Use existing file - case AssetStore::CONFLICT_USE_EXISTING: + case static::CONFLICT_USE_EXISTING: default: { return false; } @@ -340,7 +649,7 @@ class FlysystemAssetStore implements AssetStore { /** * Get an asset renamer for the given filename. * - * @param string $fileID Adaptor specific identifier for this file/version + * @param string $fileID Adapter specific identifier for this file/version * @return AssetNameGenerator */ protected function fileGeneratorFor($fileID){ @@ -361,33 +670,42 @@ class FlysystemAssetStore implements AssetStore { } /** - * Given a FileID, map this back to the original filename, trimming variant + * Given a FileID, map this back to the original filename, trimming variant and hash * - * @param string $fileID Adaptor specific identifier for this file/version - * @param string $variant Out parameter for any found variant - * @return string + * @param string $fileID Adapter specific identifier for this file/version + * @return string Filename for this file, omitting hash and variant */ - protected function getOriginalFilename($fileID, &$variant = '') { + protected function getOriginalFilename($fileID) { // Remove variant - $original = $fileID; - $variant = ''; - if(preg_match('/^(?((?[^\\.]+)(?.*)$/', $fileID, $matches)) { - $original = $matches['before'].$matches['after']; - $variant = $matches['variant']; - } + $originalID = $this->removeVariant($fileID); // Remove hash (unless using legacy filenames, without hash) if($this->useLegacyFilenames()) { - return $original; + return $originalID; } else { return preg_replace( '/(?[a-zA-Z0-9]{10}\\/)(?[^\\/]+)$/', '$2', - $original + $originalID ); } } + /** + * Remove variant from a fileID + * + * @param string $fileID + * @return string FileID without variant + */ + protected function removeVariant($fileID) { + // Check variant + if (preg_match('/^(?((?[^\\.]+)(?.*)$/', $fileID, $matches)) { + return $matches['before'] . $matches['after']; + } + // There is no variant, so return original value + return $fileID; + } + /** * Map file tuple (hash, name, variant) to a filename to be used by flysystem * @@ -396,7 +714,7 @@ class FlysystemAssetStore implements AssetStore { * @param string $filename Name of file * @param string $hash Hash of original file * @param string $variant (if given) - * @return string Adaptor specific identifier for this file/version + * @return string Adapter specific identifier for this file/version */ protected function getFileID($filename, $hash, $variant = null) { // Since we use double underscore to delimit variants, eradicate them from filename @@ -411,7 +729,7 @@ class FlysystemAssetStore implements AssetStore { } // Unless in legacy mode, inject hash just prior to the filename - if(Config::inst()->get(__CLASS__, 'legacy_filenames')) { + if($this->useLegacyFilenames()) { $fileID = $name; } else { $fileID = substr($hash, 0, 10) . '/' . $name; @@ -436,4 +754,102 @@ class FlysystemAssetStore implements AssetStore { return $fileID; } + /** + * Ensure each adapter re-generates its own server configuration files + */ + public static function flush() { + // Ensure that this instance is constructed on flush, thus forcing + // bootstrapping of necessary .htaccess / web.config files + $instance = singleton('AssetStore'); + if ($instance instanceof FlysystemAssetStore) { + $publicAdapter = $instance->getPublicFilesystem()->getAdapter(); + if($publicAdapter instanceof AssetAdapter) { + $publicAdapter->flush(); + } + $protectedAdapter = $instance->getProtectedFilesystem()->getAdapter(); + if($protectedAdapter instanceof AssetAdapter) { + $protectedAdapter->flush(); + } + } + } + + public function getResponseFor($asset) { + // Check if file exists + $filesystem = $this->getFilesystemFor($asset); + if(!$filesystem) { + return $this->createMissingResponse(); + } + + // Block directory access + if($filesystem->get($asset) instanceof Directory) { + return $this->createDeniedResponse(); + } + + // Deny if file is protected and denied + if($filesystem === $this->getProtectedFilesystem() && !$this->isGranted($asset)) { + return $this->createDeniedResponse(); + } + + // Serve up file response + return $this->createResponseFor($filesystem, $asset); + } + + /** + * Generate an {@see SS_HTTPResponse} for the given file from the source filesystem + * @param FilesystemInterface $flysystem + * @param string $fileID + * @return SS_HTTPResponse + */ + protected function createResponseFor(FilesystemInterface $flysystem, $fileID) { + // Build response body + // @todo: gzip / buffer response? + $body = $flysystem->read($fileID); + $mime = $flysystem->getMimetype($fileID); + $response = new SS_HTTPResponse($body, 200); + + // Add headers + $response->addHeader('Content-Type', $mime); + $headers = Config::inst()->get(get_class($this), 'file_response_headers'); + foreach($headers as $header => $value) { + $response->addHeader($header, $value); + } + return $response; + } + + /** + * Generate a response for requests to a denied protected file + * + * @return SS_HTTPResponse + */ + protected function createDeniedResponse() { + $code = (int)Config::inst()->get(get_class($this), 'denied_response_code'); + return $this->createErrorResponse($code); + } + + /** + * Generate a response for missing file requests + * + * @return SS_HTTPResponse + */ + protected function createMissingResponse() { + $code = (int)Config::inst()->get(get_class($this), 'missing_response_code'); + return $this->createErrorResponse($code); + } + + /** + * Create a response with the given error code + * + * @param int $code + * @return SS_HTTPResponse + */ + protected function createErrorResponse($code) { + $response = new SS_HTTPResponse('', $code); + + // Show message in dev + if(!\Director::isLive()) { + $response->setBody($response->getStatusDescription()); + } + + return $response; + } } diff --git a/filesystem/flysystem/FlysystemGeneratedAssetHandler.php b/filesystem/flysystem/FlysystemGeneratedAssetHandler.php index 5606e072d..9baff7e62 100644 --- a/filesystem/flysystem/FlysystemGeneratedAssetHandler.php +++ b/filesystem/flysystem/FlysystemGeneratedAssetHandler.php @@ -46,6 +46,7 @@ class FlysystemGeneratedAssetHandler implements GeneratedAssetHandler { if($result) { return $this ->getFilesystem() + ->getAdapter() ->getPublicUrl($filename); } } diff --git a/filesystem/flysystem/FlysystemUrlPlugin.php b/filesystem/flysystem/FlysystemUrlPlugin.php deleted file mode 100644 index ddd6fd23e..000000000 --- a/filesystem/flysystem/FlysystemUrlPlugin.php +++ /dev/null @@ -1,73 +0,0 @@ -adapter = $filesystem->getAdapter(); - } - - public function getMethod() { - return 'getPublicUrl'; - } - - /** - * Generate public url - * - * @param string $path - * @return string The full url to the file - */ - public function handle($path) { - // Default adaptor - if($this->adapter instanceof AssetAdapter) { - return $this->adapter->getPublicUrl($path); - } - - // Check S3 adaptor - if (class_exists('League\Flysystem\AwsS3v2\AwsS3Adapter') - && $this->adapter instanceof AwsS3Adapter - ) { - return sprintf( - 'https://s3.amazonaws.com/%s/%s', - $this->adapter->getBucket(), - $path - ); - } - - // Local with host - if (class_exists('Oneup\FlysystemBundle\Adapter\LocalWithHost') - && $this->adapter instanceof LocalWithHost - ) { - return sprintf( - '%s/%s/%s', - $this->adapter->getBasePath(), - $this->adapter->getWebpath(), - $path - ); - } - - // no url available - return null; - } -} \ No newline at end of file diff --git a/filesystem/flysystem/ProtectedAdapter.php b/filesystem/flysystem/ProtectedAdapter.php new file mode 100644 index 000000000..5841baef7 --- /dev/null +++ b/filesystem/flysystem/ProtectedAdapter.php @@ -0,0 +1,19 @@ + array( + '.htaccess' => "Protected_HTAccess" + ), + 'microsoft-iis' => array( + 'web.config' => "Protected_WebConfig" + ) + ); + + protected function findRoot($root) { + // Use explicitly defined path + if($root) { + return parent::findRoot($root); + } + + // Use environment defined path + if(defined('SS_PROTECTED_ASSETS_PATH')) { + return SS_PROTECTED_ASSETS_PATH; + } + + // Default location is under assets + return ASSETS_PATH . '/' . \Config::inst()->get(static::class, 'secure_folder'); + } + + /** + * Provide secure downloadable + * + * @param string $path + * @return string|null + */ + public function getProtectedUrl($path) { + // Public URLs are handled via a request handler within /assets. + // If assets are stored locally, then asset paths of protected files should be equivalent. + return \Controller::join_links(\Director::baseURL(), ASSETS_DIR, $path); + } +} diff --git a/filesystem/flysystem/PublicAdapter.php b/filesystem/flysystem/PublicAdapter.php new file mode 100644 index 000000000..d58e991c6 --- /dev/null +++ b/filesystem/flysystem/PublicAdapter.php @@ -0,0 +1,19 @@ + array( + '.htaccess' => "Assets_HTAccess" + ), + 'microsoft-iis' => array( + 'web.config' => "Assets_WebConfig" + ) + ); + + protected function findRoot($root) { + if ($root) { + return parent::findRoot($root); + } + + // Empty root will set the path to assets + return ASSETS_PATH; + } + + /** + * Provide downloadable url + * + * @param string $path + * @return string|null + */ + public function getPublicUrl($path) { + $rootPath = realpath(BASE_PATH); + $filesPath = realpath($this->pathPrefix); + + if(stripos($filesPath, $rootPath) === 0) { + $dir = substr($filesPath, strlen($rootPath)); + return Controller::join_links(Director::baseURL(), $dir, $path); + } + + // File outside of webroot can't be used + return null; + } +} \ No newline at end of file diff --git a/filesystem/storage/AssetContainer.php b/filesystem/storage/AssetContainer.php index cea738203..c853536aa 100644 --- a/filesystem/storage/AssetContainer.php +++ b/filesystem/storage/AssetContainer.php @@ -2,6 +2,8 @@ namespace SilverStripe\Filesystem\Storage; +use SilverStripe\Filesystem\Storage\AssetStore; + /** * Represents a container for a specific asset. * @@ -13,7 +15,8 @@ namespace SilverStripe\Filesystem\Storage; * @package framework * @subpackage filesystem */ -interface AssetContainer { +interface AssetContainer +{ /** * Assign a set of data to the backend @@ -22,53 +25,58 @@ interface AssetContainer { * @param string $filename Name for the resulting file * @param string $hash Hash of original file, if storing a variant. * @param string $variant Name of variant, if storing a variant. - * @param string $conflictResolution {@see AssetStore}. Will default to one chosen by the backend + * @param array $config Write options. {@see AssetStore} * @return array Tuple associative array (Filename, Hash, Variant) Unless storing a variant, the hash * will be calculated from the given data. */ - public function setFromString($data, $filename, $hash = null, $variant = null, $conflictResolution = null); + public function setFromString($data, $filename, $hash = null, $variant = null, $config = array()); - /** + /** * Assign a local file to the backend. * * @param string $path Absolute filesystem path to file - * @param type $filename Optional path to ask the backend to name as. + * @param string $filename Optional path to ask the backend to name as. * Will default to the filename of the $path, excluding directories. * @param string $hash Hash of original file, if storing a variant. * @param string $variant Name of variant, if storing a variant. - * @param string $conflictResolution {@see AssetStore} + * @param array $config Write options. {@see AssetStore} * @return array Tuple associative array (Filename, Hash, Variant) Unless storing a variant, the hash * will be calculated from the local file content. */ - public function setFromLocalFile($path, $filename = null, $hash = null, $variant = null, $conflictResolution = null); + public function setFromLocalFile($path, $filename = null, $hash = null, $variant = null, $config = array()); - /** + /** * Assign a stream to the backend * * @param resource $stream Streamable resource * @param string $filename Name for the resulting file * @param string $hash Hash of original file, if storing a variant. * @param string $variant Name of variant, if storing a variant. - * @param string $conflictResolution {@see AssetStore} + * @param array $config Write options. {@see AssetStore} * @return array Tuple associative array (Filename, Hash, Variant) Unless storing a variant, the hash * will be calculated from the raw stream. */ - public function setFromStream($stream, $filename, $hash = null, $variant = null, $conflictResolution = null); + public function setFromStream($stream, $filename, $hash = null, $variant = null, $config = array()); - /** - * @return string Data from the file in this container - */ - public function getString(); + /** + * @return string Data from the file in this container + */ + public function getString(); - /** + /** * @return resource Data stream to the asset in this container */ - public function getStream(); + public function getStream(); - /** - * @return string public url to the asset in this container - */ - public function getURL(); + /** + * @param bool $grant Ensures that the url for any protected assets is granted for the current user. + * If set to true, and the file is currently in protected mode, the asset store will ensure the + * returned URL is accessible for the duration of the current session / user. + * This will have no effect if the file is in published mode. + * This will not grant access to users other than the owner of the current session. + * @return string public url to the asset in this container + */ + public function getURL($grant = true); /** * @return string The absolute URL to the asset in this container @@ -103,6 +111,14 @@ interface AssetContainer { */ public function getIsImage(); + /** + * Determine visibility of the given file + * + * @return string one of values defined by the constants VISIBILITY_PROTECTED or VISIBILITY_PUBLIC, or + * null if the file does not exist + */ + public function getVisibility(); + /** * Determine if this container has a valid value * @@ -130,4 +146,47 @@ interface AssetContainer { * @return string */ public function getVariant(); + + /** + * Delete a file (and all variants). + * {@see AssetStore::delete()} + * + * @return bool Flag if a file was deleted + */ + public function deleteFile(); + + /** + * Publicly expose the file (and all variants) identified by the given filename and hash + * {@see AssetStore::publish} + */ + public function publishFile(); + + /** + * Protect a file (and all variants) from public access, identified by the given filename and hash. + * {@see AssetStore::protect()} + */ + public function protectFile(); + + /** + * Ensures that access to the specified protected file is granted for the current user. + * If this file is currently in protected mode, the asset store will ensure the + * returned asset for the duration of the current session / user. + * This will have no effect if the file is in published mode. + * This will not grant access to users other than the owner of the current session. + * Does not require a member to be logged in. + */ + public function grantFile(); + + /** + * Revoke access to the given file for the current user. + * Note: This will have no effect if the given file is public + */ + public function revokeFile(); + + /** + * Check if the current user can view the given file. + * + * @return bool True if the file is verified and grants access to the current session / user. + */ + public function canViewFile(); } diff --git a/filesystem/storage/AssetStore.php b/filesystem/storage/AssetStore.php index 632300489..41981ec74 100644 --- a/filesystem/storage/AssetStore.php +++ b/filesystem/storage/AssetStore.php @@ -18,6 +18,10 @@ namespace SilverStripe\Filesystem\Storage; * of the mechanism used to generate this file, and is up to user code to perform the actual * generation. An empty variant identifies this file as the original file. * + * Write options have an additional $config parameter to provide additional options to the backend. + * This is an associative array. Standard array options include 'visibility' and 'conflict'. + * + * 'conflict' config option determines the conflict resolution mechanism. * When assets are stored in the backend, user code may request one of the following conflict resolution * mechanisms: * @@ -29,6 +33,12 @@ namespace SilverStripe\Filesystem\Storage; * existing file instead. * - CONFLICT_EXCEPTION - If there is an existing file with this tuple, throw an exception. * + * 'visibility' config option determines whether the file should be marked as publicly visible. + * This may be assigned to one of the below values: + * + * - VISIBILITY_PUBLIC: This file may be accessed by any public user. + * - VISIBILITY_PROTECTED: This file must be whitelisted for individual users before being made available to that user. + * * @package framework * @subpackage filesystem */ @@ -57,6 +67,26 @@ interface AssetStore { */ const CONFLICT_USE_EXISTING = 'existing'; + /** + * Protect this file + */ + const VISIBILITY_PROTECTED = 'protected'; + + /** + * Make this file public + */ + const VISIBILITY_PUBLIC = 'public'; + + /** + * Return list of feature capabilities of this backend as an array. + * Array keys will be the options supported by $config, and the + * values will be the list of accepted values for each option (or + * true if any value is allowed). + * + * @return array + */ + public function getCapabilities(); + /** * Assign a set of data to the backend * @@ -64,38 +94,38 @@ interface AssetStore { * @param string $filename Name for the resulting file * @param string $hash Hash of original file, if storing a variant. * @param string $variant Name of variant, if storing a variant. - * @param string $conflictResolution {@see AssetStore}. Will default to one chosen by the backend + * @param array $config Write options. {@see AssetStore} * @return array Tuple associative array (Filename, Hash, Variant) Unless storing a variant, the hash * will be calculated from the given data. */ - public function setFromString($data, $filename, $hash = null, $variant = null, $conflictResolution = null); + public function setFromString($data, $filename, $hash = null, $variant = null, $config = array()); - /** + /** * Assign a local file to the backend. * * @param string $path Absolute filesystem path to file - * @param type $filename Optional path to ask the backend to name as. + * @param string $filename Optional path to ask the backend to name as. * Will default to the filename of the $path, excluding directories. * @param string $hash Hash of original file, if storing a variant. * @param string $variant Name of variant, if storing a variant. - * @param string $conflictResolution {@see AssetStore} + * @param array $config Write options. {@see AssetStore} * @return array Tuple associative array (Filename, Hash, Variant) Unless storing a variant, the hash * will be calculated from the local file content. */ - public function setFromLocalFile($path, $filename = null, $hash = null, $variant = null, $conflictResolution = null); + public function setFromLocalFile($path, $filename = null, $hash = null, $variant = null, $config = array()); - /** + /** * Assign a stream to the backend * * @param resource $stream Streamable resource * @param string $filename Name for the resulting file * @param string $hash Hash of original file, if storing a variant. * @param string $variant Name of variant, if storing a variant. - * @param string $conflictResolution {@see AssetStore} + * @param array $config Write options. {@see AssetStore} * @return array Tuple associative array (Filename, Hash, Variant) Unless storing a variant, the hash * will be calculated from the raw stream. */ - public function setFromStream($stream, $filename, $hash = null, $variant = null, $conflictResolution = null); + public function setFromStream($stream, $filename, $hash = null, $variant = null, $config = array()); /** * Get contents of a given file @@ -126,9 +156,14 @@ interface AssetStore { * @param string $hash sha1 hash of the file content. * If a variant is requested, this is the hash of the file before it was modified. * @param string|null $variant Optional variant string for this file + * @param bool $grant Ensures that the url for any protected assets is granted for the current user. + * If set to true, and the file is currently in protected mode, the asset store will ensure the + * returned URL is accessible for the duration of the current session / user. + * This will have no effect if the file is in published mode. + * This will not grant access to users other than the owner of the current session. * @return string public url to this resource */ - public function getAsURL($filename, $hash, $variant = null); + public function getAsURL($filename, $hash, $variant = null, $grant = true); /** * Get metadata for this file, if available @@ -152,6 +187,16 @@ interface AssetStore { */ public function getMimeType($filename, $hash, $variant = null); + /** + * Determine visibility of the given file + * + * @param string $filename + * @param string $hash + * @return string one of values defined by the constants VISIBILITY_PROTECTED or VISIBILITY_PUBLIC, or + * null if the file does not exist + */ + public function getVisibility($filename, $hash); + /** * Determine if a file exists with the given tuple * @@ -162,4 +207,63 @@ interface AssetStore { * @return bool Flag as to whether the file exists */ public function exists($filename, $hash, $variant = null); + + /** + * Delete a file (and all variants) identified by the given filename and hash + * + * @param string $filename + * @param string $hash + * @return bool Flag if a file was deleted + */ + public function delete($filename, $hash); + + /** + * Publicly expose the file (and all variants) identified by the given filename and hash + * + * @param string $filename Filename (not including assets) + * @param string $hash sha1 hash of the file content. + */ + public function publish($filename, $hash); + + /** + * Protect a file (and all variants) from public access, identified by the given filename and hash. + * + * A protected file can be granted access to users on a per-session or per-user basis as response + * to any future invocations of {@see grant()} or {@see getAsURL()} with $grant = true + * + * @param string $filename Filename (not including assets) + * @param string $hash sha1 hash of the file content. + */ + public function protect($filename, $hash); + + /** + * Ensures that access to the specified protected file is granted for the current user. + * If this file is currently in protected mode, the asset store will ensure the + * returned asset for the duration of the current session / user. + * This will have no effect if the file is in published mode. + * This will not grant access to users other than the owner of the current session. + * Does not require a member to be logged in. + * + * @param string $filename + * @param string $hash + */ + public function grant($filename, $hash); + + /** + * Revoke access to the given file for the current user. + * Note: This will have no effect if the given file is public + * + * @param string $filename + * @param string $hash + */ + public function revoke($filename, $hash); + + /** + * Check if the current user can view the given file. + * + * @param string $filename + * @param string $hash + * @return bool True if the file is verified and grants access to the current session / user. + */ + public function canView($filename, $hash); } diff --git a/filesystem/storage/AssetStoreRouter.php b/filesystem/storage/AssetStoreRouter.php new file mode 100644 index 000000000..465d74e05 --- /dev/null +++ b/filesystem/storage/AssetStoreRouter.php @@ -0,0 +1,22 @@ +exists()) { - return basename($this->getSourceURL()); + if(!$this->exists()) { + return null; } + return basename($this->getSourceURL()); } /** @@ -155,9 +156,10 @@ class DBFile extends CompositeDBField implements AssetContainer, ShortcodeHandle * @return string */ public function getExtension() { - if($this->exists()) { - return pathinfo($this->Filename, PATHINFO_EXTENSION); + if(!$this->exists()) { + return null; } + return pathinfo($this->Filename, PATHINFO_EXTENSION); } /** @@ -174,11 +176,11 @@ class DBFile extends CompositeDBField implements AssetContainer, ShortcodeHandle return $this->getBasename(); } - public function setFromLocalFile($path, $filename = null, $hash = null, $variant = null, $conflictResolution = null) { + public function setFromLocalFile($path, $filename = null, $hash = null, $variant = null, $config = array()) { $this->assertFilenameValid($filename ?: $path); $result = $this ->getStore() - ->setFromLocalFile($path, $filename, $hash, $variant, $conflictResolution); + ->setFromLocalFile($path, $filename, $hash, $variant, $config); // Update from result if($result) { $this->setValue($result); @@ -186,11 +188,11 @@ class DBFile extends CompositeDBField implements AssetContainer, ShortcodeHandle return $result; } - public function setFromStream($stream, $filename, $hash = null, $variant = null, $conflictResolution = null) { + public function setFromStream($stream, $filename, $hash = null, $variant = null, $config = array()) { $this->assertFilenameValid($filename); $result = $this ->getStore() - ->setFromStream($stream, $filename, $hash, $variant, $conflictResolution); + ->setFromStream($stream, $filename, $hash, $variant, $config); // Update from result if($result) { $this->setValue($result); @@ -198,11 +200,11 @@ class DBFile extends CompositeDBField implements AssetContainer, ShortcodeHandle return $result; } - public function setFromString($data, $filename, $hash = null, $variant = null, $conflictResolution = null) { + public function setFromString($data, $filename, $hash = null, $variant = null, $config = array()) { $this->assertFilenameValid($filename); $result = $this ->getStore() - ->setFromString($data, $filename, $hash, $variant, $conflictResolution); + ->setFromString($data, $filename, $hash, $variant, $config); // Update from result if($result) { $this->setValue($result); @@ -228,11 +230,11 @@ class DBFile extends CompositeDBField implements AssetContainer, ShortcodeHandle ->getAsString($this->Filename, $this->Hash, $this->Variant); } - public function getURL() { + public function getURL($grant = true) { if(!$this->exists()) { return null; } - $url = $this->getSourceURL(); + $url = $this->getSourceURL($grant); $this->updateURL($url); $this->extend('updateURL', $url); return $url; @@ -242,18 +244,19 @@ class DBFile extends CompositeDBField implements AssetContainer, ShortcodeHandle * Get URL, but without resampling. * Note that this will return the url even if the file does not exist. * + * @param bool $grant Ensures that the url for any protected assets is granted for the current user. * @return string */ - public function getSourceURL() { + public function getSourceURL($grant = true) { return $this ->getStore() - ->getAsURL($this->Filename, $this->Hash, $this->Variant); + ->getAsURL($this->Filename, $this->Hash, $this->Variant, $grant); } /** * Get the absolute URL to this resource * - * @return type + * @return string */ public function getAbsoluteURL() { if(!$this->exists()) { @@ -281,13 +284,23 @@ class DBFile extends CompositeDBField implements AssetContainer, ShortcodeHandle } public function getValue() { - if($this->exists()) { - return array( - 'Filename' => $this->Filename, - 'Hash' => $this->Hash, - 'Variant' => $this->Variant - ); + if(!$this->exists()) { + return null; } + return array( + 'Filename' => $this->Filename, + 'Hash' => $this->Hash, + 'Variant' => $this->Variant + ); + } + + public function getVisibility() { + if(empty($this->Filename)) { + return null; + } + return $this + ->getStore() + ->getVisibility($this->Filename, $this->Hash); } public function exists() { @@ -329,6 +342,7 @@ class DBFile extends CompositeDBField implements AssetContainer, ShortcodeHandle if(isset($metadata['size'])) { return $metadata['size']; } + return 0; } /** @@ -470,4 +484,53 @@ class DBFile extends CompositeDBField implements AssetContainer, ShortcodeHandle } return false; } + + public function deleteFile() { + if(!$this->Filename) { + return false; + } + + return $this + ->getStore() + ->delete($this->Filename, $this->Hash); + } + + public function publishFile() { + if($this->Filename) { + $this + ->getStore() + ->publish($this->Filename, $this->Hash); + } + } + + public function protectFile() { + if($this->Filename) { + $this + ->getStore() + ->protect($this->Filename, $this->Hash); + } + } + + public function grantFile() { + if($this->Filename) { + $this + ->getStore() + ->grant($this->Filename, $this->Hash); + } + } + + public function revokeFile() { + if($this->Filename) { + $this + ->getStore() + ->revoke($this->Filename, $this->Hash); + } + } + + public function canViewFile() { + return $this->Filename + && $this + ->getStore() + ->canView($this->Filename, $this->Hash); + } } diff --git a/filesystem/storage/ProtectedFileController.php b/filesystem/storage/ProtectedFileController.php new file mode 100644 index 000000000..118d9aa8f --- /dev/null +++ b/filesystem/storage/ProtectedFileController.php @@ -0,0 +1,92 @@ +handler; + } + + /** + * @param AssetStoreRouter $handler + * @return $this + */ + public function setRouteHandler(AssetStoreRouter $handler) { + $this->handler = $handler; + return $this; + } + + private static $url_handlers = array( + '$Filename' => "handleFile" + ); + + private static $allowed_actions = array( + 'handleFile' + ); + + /** + * Provide a response for the given file request + * + * @param \SS_HTTPRequest $request + * @return \SS_HTTPResponse + */ + public function handleFile(\SS_HTTPRequest $request) { + $filename = $this->parseFilename($request); + + // Deny requests to private file + if(!$this->isValidFilename($filename)) { + return $this->httpError(400, "Invalid request"); + } + + // Pass through to backend + return $this->getRouteHandler()->getResponseFor($filename); + } + + /** + * Check if the given filename is safe to pass to the route handler. + * This should block direct requests to assets/.protected/ paths + * + * @param $filename + * @return bool True if the filename is allowed + */ + public function isValidFilename($filename) { + // Block hidden files + return !preg_match('#(^|[\\\\/])\\..*#', $filename); + } + + /** + * Get the file component from the request + * + * @param \SS_HTTPRequest $request + * @return string + */ + protected function parseFilename(\SS_HTTPRequest $request) { + $filename = ''; + $next = $request->param('Filename'); + while($next) { + $filename = $filename ? \File::join_paths($filename, $next) : $next; + $next = $request->shift(); + } + if($extension = $request->getExtension()) { + $filename = $filename . "." . $extension; + } + return $filename; + } +} diff --git a/model/DataObject.php b/model/DataObject.php index 35c2b2f46..64a3491e0 100644 --- a/model/DataObject.php +++ b/model/DataObject.php @@ -195,6 +195,16 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity 'Created' => 'SS_Datetime', ); + /** + * Core dataobject extensions + * + * @config + * @var array + */ + private static $extensions = array( + 'AssetControl' => '\\SilverStripe\\Filesystem\\AssetControlExtension' + ); + /** * Non-static relationship cache, indexed by component name. */ diff --git a/model/Image_Backend.php b/model/Image_Backend.php index 406b4599a..9fbc9806d 100644 --- a/model/Image_Backend.php +++ b/model/Image_Backend.php @@ -56,11 +56,11 @@ interface Image_Backend { * @param string $filename Name for the resulting file * @param string $hash Hash of original file, if storing a variant. * @param string $variant Name of variant, if storing a variant. - * @param string $conflictResolution {@see AssetStore}. Will default to one chosen by the backend + * @param array $config Write options. {@see AssetStore} * @return array Tuple associative array (Filename, Hash, Variant) Unless storing a variant, the hash * will be calculated from the given data. */ - public function writeToStore(AssetStore $assetStore, $filename, $hash = null, $variant = null, $conflictResolution = null); + public function writeToStore(AssetStore $assetStore, $filename, $hash = null, $variant = null, $config = array()); /** * Write the backend to a local path diff --git a/model/ValidationResult.php b/model/ValidationResult.php index 75cdee244..6b6dce155 100644 --- a/model/ValidationResult.php +++ b/model/ValidationResult.php @@ -30,9 +30,9 @@ class ValidationResult extends Object { /** * Record an error against this validation result, - * @param $message The validation error message - * @param $code An optional error code string, that can be accessed with {@link $this->codeList()}. - * @return ValidationResult this + * @param string $message The validation error message + * @param int $code An optional error code string, that can be accessed with {@link $this->codeList()}. + * @return $this */ public function error($message, $code = null) { $this->isValid = false; diff --git a/model/Versioned.php b/model/Versioned.php index e6fba50e2..63c46c4fb 100644 --- a/model/Versioned.php +++ b/model/Versioned.php @@ -805,10 +805,10 @@ class Versioned extends DataExtension implements TemplateGlobalProvider { /** * Move a database record from one stage to the other. * - * @param fromStage Place to copy from. Can be either a stage name or a version number. - * @param toStage Place to copy to. Must be a stage name. - * @param createNewVersion Set this to true to create a new version number. By default, the existing version - * number will be copied over. + * @param string $fromStage Place to copy from. Can be either a stage name or a version number. + * @param string $toStage Place to copy to. Must be a stage name. + * @param bool $createNewVersion Set this to true to create a new version number. + * By default, the existing version number will be copied over. */ public function publish($fromStage, $toStage, $createNewVersion = false) { $this->owner->extend('onBeforeVersionedPublish', $fromStage, $toStage, $createNewVersion); diff --git a/templates/filesystem/Assets_HTAccess.ss b/templates/filesystem/Assets_HTAccess.ss new file mode 100644 index 000000000..3e76eedfe --- /dev/null +++ b/templates/filesystem/Assets_HTAccess.ss @@ -0,0 +1,27 @@ +# +# Whitelist appropriate assets files. +# This file is automatically generated via File.allowed_extensions configuration +# See AssetAdapter::renderTemplate() for reference. +# + + + SetEnv HTTP_MOD_REWRITE On + RewriteEngine On + + # Disable PHP handler + RewriteCond %{REQUEST_URI} .(?i:php|phtml|php3|php4|php5|inc)$ + RewriteRule .* - [F] + + # Allow error pages + RewriteCond %{REQUEST_FILENAME} -f + RewriteRule error[^\\/]*\.html$ - [L] + + # Block invalid file extensions + RewriteCond %{REQUEST_URI} !\.(?i:<% loop $AllowedExtensions %>$Extension<% if not $Last %>|<% end_if %><% end_loop %>)$ + RewriteRule .* - [F] + + # Non existant files passed to requesthandler + RewriteCond %{REQUEST_URI} ^(.*)$ + RewriteCond %{REQUEST_FILENAME} !-f + RewriteRule .* ../framework/main.php?url=%1 [QSA] + diff --git a/templates/filesystem/Assets_WebConfig.ss b/templates/filesystem/Assets_WebConfig.ss new file mode 100644 index 000000000..18cc41ca8 --- /dev/null +++ b/templates/filesystem/Assets_WebConfig.ss @@ -0,0 +1,29 @@ + + + + + + + + <% loop $AllowedExtensions %> + + <% end_loop %> + + + + + + + + + + + + + + + + diff --git a/templates/filesystem/Protected_HTAccess.ss b/templates/filesystem/Protected_HTAccess.ss new file mode 100644 index 000000000..e324f8bb9 --- /dev/null +++ b/templates/filesystem/Protected_HTAccess.ss @@ -0,0 +1,2 @@ +Deny from all +RewriteRule .* - [F] diff --git a/templates/filesystem/Protected_WebConfig.ss b/templates/filesystem/Protected_WebConfig.ss new file mode 100644 index 000000000..bce81c05b --- /dev/null +++ b/templates/filesystem/Protected_WebConfig.ss @@ -0,0 +1,17 @@ + + + + + + + + + + + + + + + diff --git a/tests/filesystem/AssetControlExtensionTest.php b/tests/filesystem/AssetControlExtensionTest.php new file mode 100644 index 000000000..0744d54e9 --- /dev/null +++ b/tests/filesystem/AssetControlExtensionTest.php @@ -0,0 +1,154 @@ +Title = 'My object'; + $fish1 = realpath(__DIR__ .'/../model/testimages/test-image-high-quality.jpg'); + $object1->Header->setFromLocalFile($fish1, 'Header/MyObjectHeader.jpg'); + $object1->Download->setFromString('file content', 'Documents/File.txt'); + $object1->write(); + $object1->publish('Stage', 'Live'); + + $object2 = new AssetControlExtensionTest_Object(); + $object2->Title = 'Unversioned'; + $object2->Image->setFromLocalFile($fish1, 'Images/BeautifulFish.jpg'); + $object2->write(); + + $object3 = new AssetControlExtensionTest_ArchivedObject(); + $object3->Title = 'Archived'; + $object3->Header->setFromLocalFile($fish1, 'Archived/MyObjectHeader.jpg'); + $object3->write(); + $object3->publish('Stage', 'Live'); + } + + public function tearDown() { + AssetStoreTest_SpyStore::reset(); + parent::tearDown(); + } + + public function testFileDelete() { + /** @var AssetControlExtensionTest_VersionedObject $object1 */ + $object1 = AssetControlExtensionTest_VersionedObject::get() + ->filter('Title', 'My object') + ->first(); + /** @var AssetControlExtensionTest_Object $object2 */ + $object2 = AssetControlExtensionTest_Object::get() + ->filter('Title', 'Unversioned') + ->first(); + + /** @var AssetControlExtensionTest_ArchivedObject $object3 */ + $object3 = AssetControlExtensionTest_ArchivedObject::get() + ->filter('Title', 'Archived') + ->first(); + + $this->assertTrue($object1->Download->exists()); + $this->assertTrue($object1->Header->exists()); + $this->assertTrue($object2->Image->exists()); + $this->assertTrue($object3->Header->exists()); + $this->assertEquals(AssetStore::VISIBILITY_PUBLIC, $object1->Download->getVisibility()); + $this->assertEquals(AssetStore::VISIBILITY_PUBLIC, $object1->Header->getVisibility()); + $this->assertEquals(AssetStore::VISIBILITY_PUBLIC, $object2->Image->getVisibility()); + $this->assertEquals(AssetStore::VISIBILITY_PUBLIC, $object3->Header->getVisibility()); + + // Check live stage for versioned objects + $object1Live = Versioned::get_one_by_stage('AssetControlExtensionTest_VersionedObject', 'Live', + array('"ID"' => $object1->ID) + ); + $object3Live = Versioned::get_one_by_stage('AssetControlExtensionTest_ArchivedObject', 'Live', + array('"ID"' => $object3->ID) + ); + $this->assertTrue($object1Live->Download->exists()); + $this->assertTrue($object1Live->Header->exists()); + $this->assertTrue($object3Live->Header->exists()); + $this->assertEquals(AssetStore::VISIBILITY_PUBLIC, $object1Live->Download->getVisibility()); + $this->assertEquals(AssetStore::VISIBILITY_PUBLIC, $object1Live->Header->getVisibility()); + $this->assertEquals(AssetStore::VISIBILITY_PUBLIC, $object3Live->Header->getVisibility()); + + // Delete live records; Should cause versioned records to be protected + $object1Live->deleteFromStage('Live'); + $object3Live->deleteFromStage('Live'); + $this->assertTrue($object1->Download->exists()); + $this->assertTrue($object1->Header->exists()); + $this->assertTrue($object3->Header->exists()); + $this->assertTrue($object1Live->Download->exists()); + $this->assertTrue($object1Live->Header->exists()); + $this->assertTrue($object3Live->Header->exists()); + $this->assertEquals(AssetStore::VISIBILITY_PROTECTED, $object1->Download->getVisibility()); + $this->assertEquals(AssetStore::VISIBILITY_PROTECTED, $object1->Header->getVisibility()); + $this->assertEquals(AssetStore::VISIBILITY_PROTECTED, $object3->Header->getVisibility()); + + // Delete draft record; Should remove all records + // Archived assets only should remain + $object1->delete(); + $object2->delete(); + $object3->delete(); + $this->assertFalse($object1->Download->exists()); + $this->assertFalse($object1->Header->exists()); + $this->assertFalse($object2->Image->exists()); + $this->assertTrue($object3->Header->exists()); + $this->assertFalse($object1Live->Download->exists()); + $this->assertFalse($object1Live->Header->exists()); + $this->assertTrue($object3Live->Header->exists()); + $this->assertNull($object1->Download->getVisibility()); + $this->assertNull($object1->Header->getVisibility()); + $this->assertNull($object2->Image->getVisibility()); + $this->assertEquals(AssetStore::VISIBILITY_PROTECTED, $object3->Header->getVisibility()); + } +} + +/** + * Versioned object with attached assets + * + * @property string $Title + * @property DBFile $Header + * @property DBFile $Download + * @mixin Versioned + */ +class AssetControlExtensionTest_VersionedObject extends DataObject implements TestOnly { + private static $extensions = array( + 'Versioned' + ); + + private static $db = array( + 'Title' => 'Varchar(255)', + 'Header' => "DBFile('image/supported')", + 'Download' => 'DBFile' + ); +} + +/** + * A basic unversioned object + * + * @property string $Title + * @property DBFile $Image + */ +class AssetControlExtensionTest_Object extends DataObject implements TestOnly { + private static $db = array( + 'Title' => 'Varchar(255)', + 'Image' => "DBFile('image/supported')" + ); +} + +/** + * Versioned object that always archives its assets + */ +class AssetControlExtensionTest_ArchivedObject extends AssetControlExtensionTest_VersionedObject { + private static $archive_assets = true; +} diff --git a/tests/filesystem/AssetStoreTest.php b/tests/filesystem/AssetStoreTest.php index c000cf175..dfaa28a3d 100644 --- a/tests/filesystem/AssetStoreTest.php +++ b/tests/filesystem/AssetStoreTest.php @@ -1,10 +1,12 @@ assertEquals( - '/assets/DBFileTest/directory/a870de278b/lovely-fish.jpg', + '/assets/AssetStoreTest/directory/a870de278b/lovely-fish.jpg', $backend->getAsURL($fish1Tuple['Filename'], $fish1Tuple['Hash']) ); // Write a different file with same name. Should not detect duplicates since sha are different $fish2 = realpath(__DIR__ .'/../model/testimages/test-image-low-quality.jpg'); try { - $fish2Tuple = $backend->setFromLocalFile($fish2, 'directory/lovely-fish.jpg', null, null, AssetStore::CONFLICT_EXCEPTION); + $fish2Tuple = $backend->setFromLocalFile( + $fish2, + 'directory/lovely-fish.jpg', + null, + null, + array('conflict' => AssetStore::CONFLICT_EXCEPTION) + ); } catch(Exception $ex) { return $this->fail('Writing file with different sha to same location failed with exception'); } @@ -119,13 +127,19 @@ class AssetStoreTest extends SapphireTest { $fish2Tuple ); $this->assertEquals( - '/assets/DBFileTest/directory/33be1b95cb/lovely-fish.jpg', + '/assets/AssetStoreTest/directory/33be1b95cb/lovely-fish.jpg', $backend->getAsURL($fish2Tuple['Filename'], $fish2Tuple['Hash']) ); // Write original file back with rename $this->assertFileExists($fish1); - $fish3Tuple = $backend->setFromLocalFile($fish1, 'directory/lovely-fish.jpg', null, null, AssetStore::CONFLICT_RENAME); + $fish3Tuple = $backend->setFromLocalFile( + $fish1, + 'directory/lovely-fish.jpg', + null, + null, + array('conflict' => AssetStore::CONFLICT_RENAME) + ); $this->assertEquals( array( 'Hash' => 'a870de278b475cb75f5d9f451439b2d378e13af1', @@ -135,12 +149,18 @@ class AssetStoreTest extends SapphireTest { $fish3Tuple ); $this->assertEquals( - '/assets/DBFileTest/directory/a870de278b/lovely-fish-v2.jpg', + '/assets/AssetStoreTest/directory/a870de278b/lovely-fish-v2.jpg', $backend->getAsURL($fish3Tuple['Filename'], $fish3Tuple['Hash']) ); // Write another file should increment to -v3 - $fish4Tuple = $backend->setFromLocalFile($fish1, 'directory/lovely-fish-v2.jpg', null, null, AssetStore::CONFLICT_RENAME); + $fish4Tuple = $backend->setFromLocalFile( + $fish1, + 'directory/lovely-fish-v2.jpg', + null, + null, + array('conflict' => AssetStore::CONFLICT_RENAME) + ); $this->assertEquals( array( 'Hash' => 'a870de278b475cb75f5d9f451439b2d378e13af1', @@ -150,12 +170,18 @@ class AssetStoreTest extends SapphireTest { $fish4Tuple ); $this->assertEquals( - '/assets/DBFileTest/directory/a870de278b/lovely-fish-v3.jpg', + '/assets/AssetStoreTest/directory/a870de278b/lovely-fish-v3.jpg', $backend->getAsURL($fish4Tuple['Filename'], $fish4Tuple['Hash']) ); // Test conflict use existing file - $fish5Tuple = $backend->setFromLocalFile($fish1, 'directory/lovely-fish.jpg', null, null, AssetStore::CONFLICT_USE_EXISTING); + $fish5Tuple = $backend->setFromLocalFile( + $fish1, + 'directory/lovely-fish.jpg', + null, + null, + array('conflict' => AssetStore::CONFLICT_USE_EXISTING) + ); $this->assertEquals( array( 'Hash' => 'a870de278b475cb75f5d9f451439b2d378e13af1', @@ -165,12 +191,18 @@ class AssetStoreTest extends SapphireTest { $fish5Tuple ); $this->assertEquals( - '/assets/DBFileTest/directory/a870de278b/lovely-fish.jpg', + '/assets/AssetStoreTest/directory/a870de278b/lovely-fish.jpg', $backend->getAsURL($fish5Tuple['Filename'], $fish5Tuple['Hash']) ); // Test conflict use existing file - $fish6Tuple = $backend->setFromLocalFile($fish1, 'directory/lovely-fish.jpg', null, null, AssetStore::CONFLICT_OVERWRITE); + $fish6Tuple = $backend->setFromLocalFile( + $fish1, + 'directory/lovely-fish.jpg', + null, + null, + array('conflict' => AssetStore::CONFLICT_OVERWRITE) + ); $this->assertEquals( array( 'Hash' => 'a870de278b475cb75f5d9f451439b2d378e13af1', @@ -180,7 +212,7 @@ class AssetStoreTest extends SapphireTest { $fish6Tuple ); $this->assertEquals( - '/assets/DBFileTest/directory/a870de278b/lovely-fish.jpg', + '/assets/AssetStoreTest/directory/a870de278b/lovely-fish.jpg', $backend->getAsURL($fish6Tuple['Filename'], $fish6Tuple['Hash']) ); } @@ -192,44 +224,36 @@ class AssetStoreTest extends SapphireTest { $store = new AssetStoreTest_SpyStore(); $this->assertEquals( 'directory/lovely-fish.jpg', - $store->getOriginalFilename('directory/a870de278b/lovely-fish.jpg', $variant) + $store->getOriginalFilename('directory/a870de278b/lovely-fish.jpg') ); - $this->assertEmpty($variant); $this->assertEquals( 'directory/lovely-fish.jpg', - $store->getOriginalFilename('directory/a870de278b/lovely-fish__variant.jpg', $variant) + $store->getOriginalFilename('directory/a870de278b/lovely-fish__variant.jpg') ); - $this->assertEquals('variant', $variant); $this->assertEquals( 'directory/lovely_fish.jpg', - $store->getOriginalFilename('directory/a870de278b/lovely_fish__vari_ant.jpg', $variant) + $store->getOriginalFilename('directory/a870de278b/lovely_fish__vari_ant.jpg') ); - $this->assertEquals('vari_ant', $variant); $this->assertEquals( 'directory/lovely_fish.jpg', - $store->getOriginalFilename('directory/a870de278b/lovely_fish.jpg', $variant) + $store->getOriginalFilename('directory/a870de278b/lovely_fish.jpg') ); - $this->assertEmpty($variant); $this->assertEquals( 'lovely-fish.jpg', - $store->getOriginalFilename('a870de278b/lovely-fish.jpg', $variant) + $store->getOriginalFilename('a870de278b/lovely-fish.jpg') ); - $this->assertEmpty($variant); $this->assertEquals( 'lovely-fish.jpg', - $store->getOriginalFilename('a870de278b/lovely-fish__variant.jpg', $variant) + $store->getOriginalFilename('a870de278b/lovely-fish__variant.jpg') ); - $this->assertEquals('variant', $variant); $this->assertEquals( 'lovely_fish.jpg', - $store->getOriginalFilename('a870de278b/lovely_fish__vari__ant.jpg', $variant) + $store->getOriginalFilename('a870de278b/lovely_fish__vari__ant.jpg') ); - $this->assertEquals('vari__ant', $variant); $this->assertEquals( 'lovely_fish.jpg', - $store->getOriginalFilename('a870de278b/lovely_fish.jpg', $variant) + $store->getOriginalFilename('a870de278b/lovely_fish.jpg') ); - $this->assertEmpty($variant); } /** @@ -278,7 +302,6 @@ class AssetStoreTest extends SapphireTest { $this->assertEquals('file', $fishMeta['type']); $this->assertNotEmpty($fishMeta['timestamp']); - // text $puppies = 'puppies'; $puppiesTuple = $backend->setFromString($puppies, 'pets/my-puppy.txt'); @@ -313,7 +336,7 @@ class AssetStoreTest extends SapphireTest { $fish1Tuple ); $this->assertEquals( - '/assets/DBFileTest/directory/lovely-fish.jpg', + '/assets/AssetStoreTest/directory/lovely-fish.jpg', $backend->getAsURL($fish1Tuple['Filename'], $fish1Tuple['Hash']) ); @@ -321,14 +344,26 @@ class AssetStoreTest extends SapphireTest { // Since we are using legacy filenames, this should generate a new filename $fish2 = realpath(__DIR__ .'/../model/testimages/test-image-low-quality.jpg'); try { - $backend->setFromLocalFile($fish2, 'directory/lovely-fish.jpg', null, null, AssetStore::CONFLICT_EXCEPTION); + $backend->setFromLocalFile( + $fish2, + 'directory/lovely-fish.jpg', + null, + null, + array('conflict' => AssetStore::CONFLICT_EXCEPTION) + ); return $this->fail('Writing file with different sha to same location should throw exception'); } catch(Exception $ex) { // Success } // Re-attempt this file write with conflict_rename - $fish3Tuple = $backend->setFromLocalFile($fish2, 'directory/lovely-fish.jpg', null, null, AssetStore::CONFLICT_RENAME); + $fish3Tuple = $backend->setFromLocalFile( + $fish2, + 'directory/lovely-fish.jpg', + null, + null, + array('conflict' => AssetStore::CONFLICT_RENAME) + ); $this->assertEquals( array( 'Hash' => '33be1b95cba0358fe54e8b13532162d52f97421c', @@ -338,12 +373,18 @@ class AssetStoreTest extends SapphireTest { $fish3Tuple ); $this->assertEquals( - '/assets/DBFileTest/directory/lovely-fish-v2.jpg', + '/assets/AssetStoreTest/directory/lovely-fish-v2.jpg', $backend->getAsURL($fish3Tuple['Filename'], $fish3Tuple['Hash']) ); // Write back original file, but with CONFLICT_EXISTING. The file should not change - $fish4Tuple = $backend->setFromLocalFile($fish1, 'directory/lovely-fish-v2.jpg', null, null, AssetStore::CONFLICT_USE_EXISTING); + $fish4Tuple = $backend->setFromLocalFile( + $fish1, + 'directory/lovely-fish-v2.jpg', + null, + null, + array('conflict' => AssetStore::CONFLICT_USE_EXISTING) + ); $this->assertEquals( array( 'Hash' => '33be1b95cba0358fe54e8b13532162d52f97421c', @@ -353,12 +394,18 @@ class AssetStoreTest extends SapphireTest { $fish4Tuple ); $this->assertEquals( - '/assets/DBFileTest/directory/lovely-fish-v2.jpg', + '/assets/AssetStoreTest/directory/lovely-fish-v2.jpg', $backend->getAsURL($fish4Tuple['Filename'], $fish4Tuple['Hash']) ); // Write back original file with CONFLICT_OVERWRITE. The file sha should now be updated - $fish5Tuple = $backend->setFromLocalFile($fish1, 'directory/lovely-fish-v2.jpg', null, null, AssetStore::CONFLICT_OVERWRITE); + $fish5Tuple = $backend->setFromLocalFile( + $fish1, + 'directory/lovely-fish-v2.jpg', + null, + null, + array('conflict' => AssetStore::CONFLICT_OVERWRITE) + ); $this->assertEquals( array( 'Hash' => 'a870de278b475cb75f5d9f451439b2d378e13af1', @@ -368,7 +415,7 @@ class AssetStoreTest extends SapphireTest { $fish5Tuple ); $this->assertEquals( - '/assets/DBFileTest/directory/lovely-fish-v2.jpg', + '/assets/AssetStoreTest/directory/lovely-fish-v2.jpg', $backend->getAsURL($fish5Tuple['Filename'], $fish5Tuple['Hash']) ); } @@ -389,6 +436,74 @@ class AssetStoreTest extends SapphireTest { $this->assertEquals(AssetStore::CONFLICT_RENAME, $store->getDefaultConflictResolution(null)); $this->assertEquals(AssetStore::CONFLICT_OVERWRITE, $store->getDefaultConflictResolution('somevariant')); } + + /** + * Test protect / publish mechanisms + */ + public function testProtect() { + $backend = $this->getBackend(); + $fish = realpath(__DIR__ .'/../model/testimages/test-image-high-quality.jpg'); + $fishTuple = $backend->setFromLocalFile($fish, 'parent/lovely-fish.jpg'); + $fishVariantTuple = $backend->setFromLocalFile($fish, $fishTuple['Filename'], $fishTuple['Hash'], 'copy'); + + // Test public file storage + $this->assertFileExists(ASSETS_PATH . '/AssetStoreTest/parent/a870de278b/lovely-fish.jpg'); + $this->assertFileExists(ASSETS_PATH . '/AssetStoreTest/parent/a870de278b/lovely-fish__copy.jpg'); + $this->assertEquals( + AssetStore::VISIBILITY_PUBLIC, + $backend->getVisibility($fishTuple['Filename'], $fishTuple['Hash']) + ); + $this->assertEquals( + '/assets/AssetStoreTest/parent/a870de278b/lovely-fish.jpg', + $backend->getAsURL($fishTuple['Filename'], $fishTuple['Hash']) + ); + $this->assertEquals( + '/assets/AssetStoreTest/parent/a870de278b/lovely-fish__copy.jpg', + $backend->getAsURL($fishVariantTuple['Filename'], $fishVariantTuple['Hash'], $fishVariantTuple['Variant']) + ); + + // Test access rights to public files cannot be revoked + $backend->revoke($fishTuple['Filename'], $fishTuple['Hash']); // can't revoke public assets + $this->assertTrue($backend->canView($fishTuple['Filename'], $fishTuple['Hash'])); + + // Test protected file storage + $backend->protect($fishTuple['Filename'], $fishTuple['Hash']); + $this->assertFileNotExists(ASSETS_PATH . '/AssetStoreTest/parent/a870de278b/lovely-fish.jpg'); + $this->assertFileNotExists(ASSETS_PATH . '/AssetStoreTest/parent/a870de278b/lovely-fish__copy.jpg'); + $this->assertFileExists(ASSETS_PATH . '/AssetStoreTest/.protected/parent/a870de278b/lovely-fish.jpg'); + $this->assertFileExists(ASSETS_PATH . '/AssetStoreTest/.protected/parent/a870de278b/lovely-fish__copy.jpg'); + $this->assertEquals( + AssetStore::VISIBILITY_PROTECTED, + $backend->getVisibility($fishTuple['Filename'], $fishTuple['Hash']) + ); + + // Test access rights + $backend->revoke($fishTuple['Filename'], $fishTuple['Hash']); + $this->assertFalse($backend->canView($fishTuple['Filename'], $fishTuple['Hash'])); + $backend->grant($fishTuple['Filename'], $fishTuple['Hash']); + $this->assertTrue($backend->canView($fishTuple['Filename'], $fishTuple['Hash'])); + + // Protected urls should go through asset routing mechanism + $this->assertEquals( + '/assets/parent/a870de278b/lovely-fish.jpg', + $backend->getAsURL($fishTuple['Filename'], $fishTuple['Hash']) + ); + $this->assertEquals( + '/assets/parent/a870de278b/lovely-fish__copy.jpg', + $backend->getAsURL($fishVariantTuple['Filename'], $fishVariantTuple['Hash'], $fishVariantTuple['Variant']) + ); + + // Publish reverts visibility + $backend->publish($fishTuple['Filename'], $fishTuple['Hash']); + $this->assertFileExists(ASSETS_PATH . '/AssetStoreTest/parent/a870de278b/lovely-fish.jpg'); + $this->assertFileExists(ASSETS_PATH . '/AssetStoreTest/parent/a870de278b/lovely-fish__copy.jpg'); + $this->assertFileNotExists(ASSETS_PATH . '/AssetStoreTest/.protected/parent/a870de278b/lovely-fish.jpg'); + $this->assertFileNotExists(ASSETS_PATH . '/AssetStoreTest/.protected/parent/a870de278b/lovely-fish__copy.jpg'); + $this->assertEquals( + AssetStore::VISIBILITY_PUBLIC, + $backend->getVisibility($fishTuple['Filename'], $fishTuple['Hash']) + ); + } } /** @@ -396,6 +511,14 @@ class AssetStoreTest extends SapphireTest { */ class AssetStoreTest_SpyStore extends FlysystemAssetStore { + /** + * Enable disclosure of secure assets + * + * @config + * @var int + */ + private static $denied_response_code = 403; + /** * Set to true|false to override all isSeekableStream calls * @@ -417,16 +540,23 @@ class AssetStoreTest_SpyStore extends FlysystemAssetStore { */ public static function activate($basedir) { // Assign this as the new store - $adapter = new AssetAdapter(ASSETS_PATH . '/' . $basedir); - $filesystem = new Filesystem($adapter); - $filesystem->addPlugin(new FlysystemUrlPlugin()); + $publicAdapter = new PublicAssetAdapter(ASSETS_PATH . '/' . $basedir); + $publicFilesystem = new Filesystem($publicAdapter, [ + 'visibility' => AdapterInterface::VISIBILITY_PUBLIC + ]); + $protectedAdapter = new ProtectedAssetAdapter(ASSETS_PATH . '/' . $basedir . '/.protected'); + $protectedFilesystem = new Filesystem($protectedAdapter, [ + 'visibility' => AdapterInterface::VISIBILITY_PRIVATE + ]); + $backend = new AssetStoreTest_SpyStore(); - $backend->setFilesystem($filesystem); + $backend->setPublicFilesystem($publicFilesystem); + $backend->setProtectedFilesystem($protectedFilesystem); Injector::inst()->registerService($backend, 'AssetStore'); // Assign flysystem backend to generated asset handler at the same time $generated = new FlysystemGeneratedAssetHandler(); - $generated->setFilesystem($filesystem); + $generated->setFilesystem($publicFilesystem); Injector::inst()->registerService($generated, 'GeneratedAssetHandler'); Requirements::backend()->setAssetHandler($generated); @@ -439,7 +569,7 @@ class AssetStoreTest_SpyStore extends FlysystemAssetStore { self::$basedir = $basedir; // Ensure basedir exists - SS_Filesystem::makeFolder(self::base_path()); + \Filesystem::makeFolder(self::base_path()); } /** @@ -461,7 +591,7 @@ class AssetStoreTest_SpyStore extends FlysystemAssetStore { if(self::$basedir) { $path = self::base_path(); if(file_exists($path)) { - SS_Filesystem::removeFolder($path); + \Filesystem::removeFolder($path); } } self::$seekable_override = null; @@ -480,10 +610,18 @@ class AssetStoreTest_SpyStore extends FlysystemAssetStore { if($asset instanceof File) { $asset = $asset->File; } - if($asset instanceof DBFile) { - return BASE_PATH . $asset->getSourceURL(); + // Extract filesystem used to store this object + /** @var AssetStoreTest_SpyStore $assetStore */ + $assetStore = Injector::inst()->get('AssetStore'); + $fileID = $assetStore->getFileID($asset->Filename, $asset->Hash, $asset->Variant); + $filesystem = $assetStore->getProtectedFilesystem(); + if(!$filesystem->has($fileID)) { + $filesystem = $assetStore->getPublicFilesystem(); } - return BASE_PATH . $asset->getUrl(); + /** @var Local $adapter */ + $adapter = $filesystem->getAdapter(); + return $adapter->applyPathPrefix($fileID); + } public function cleanFilename($filename) { @@ -495,8 +633,12 @@ class AssetStoreTest_SpyStore extends FlysystemAssetStore { } - public function getOriginalFilename($fileID, &$variant = '') { - return parent::getOriginalFilename($fileID, $variant); + public function getOriginalFilename($fileID) { + return parent::getOriginalFilename($fileID); + } + + public function removeVariant($fileID) { + return parent::removeVariant($fileID); } public function getDefaultConflictResolution($variant) { diff --git a/tests/filesystem/FileTest.php b/tests/filesystem/FileTest.php index 5162c5b1d..01424d717 100644 --- a/tests/filesystem/FileTest.php +++ b/tests/filesystem/FileTest.php @@ -352,15 +352,15 @@ class FileTest extends SapphireTest { $this->assertEquals("93132.3 GB", File::format_size(100000000000000)); } - public function testDeleteDatabaseOnly() { + public function testDeleteFile() { $file = $this->objFromFixture('File', 'asdf'); $fileID = $file->ID; $filePath = AssetStoreTest_SpyStore::getLocalPath($file); - $file->delete(); - $this->assertFileExists($filePath); - $this->assertFalse(DataObject::get_by_id('File', $fileID)); + // File is deleted + $this->assertFileNotExists($filePath); + $this->assertEmpty(DataObject::get_by_id('File', $fileID)); } public function testRenameFolder() { diff --git a/tests/filesystem/ProtectedFileControllerTest.php b/tests/filesystem/ProtectedFileControllerTest.php new file mode 100644 index 000000000..a712c167c --- /dev/null +++ b/tests/filesystem/ProtectedFileControllerTest.php @@ -0,0 +1,201 @@ +exclude('ClassName', 'Folder') as $file) { + /** @var File $file */ + $path = AssetStoreTest_SpyStore::getLocalPath($file); + \Filesystem::makeFolder(dirname($path)); + $fh = fopen($path, "w+"); + fwrite($fh, str_repeat('x', 1000000)); + fclose($fh); + + // Create variant for each file + $this->getAssetStore()->setFromString( + str_repeat('y', 100), + $file->Filename, + $file->Hash, + 'variant' + ); + } + } + + /** + * @dataProvider getFilenames + */ + public function testIsValidFilename($name, $isValid) { + $controller = new ProtectedFileController(); + $this->assertEquals( + $isValid, + $controller->isValidFilename($name), + "Assert filename \"$name\" is " . $isValid ? "valid" : "invalid" + ); + } + + public function getFilenames() { + return array( + // Valid names + array('name.jpg', true), + array('parent/name.jpg', true), + array('parent/name', true), + array('parent\name.jpg', true), + array('parent\name', true), + array('name', true), + + // Invalid names + array('.invalid/name.jpg', false), + array('.invalid\name.jpg', false), + array('.htaccess', false), + array('test/.htaccess.jpg', false), + array('name/.jpg', false), + array('test\.htaccess.jpg', false), + array('name\.jpg', false) + ); + } + + /** + * Test that certain requests are denied + */ + public function testInvalidRequest() { + $result = $this->get('assets/.protected/file.jpg'); + $this->assertResponseEquals(400, null, $result); + } + + /** + * Test that invalid files generate 404 response + */ + public function testFileNotFound() { + $result = $this->get('assets/missing.jpg'); + $this->assertResponseEquals(404, null, $result); + } + + /** + * Check public access to assets is available at the appropriate time + */ + public function testAccessControl() { + $expectedContent = str_repeat('x', 1000000); + $variantContent = str_repeat('y', 100); + + $result = $this->get('assets/55b443b601/FileTest.txt'); + $this->assertResponseEquals(200, $expectedContent, $result); + $result = $this->get('assets/55b443b601/FileTest__variant.txt'); + $this->assertResponseEquals(200, $variantContent, $result); + + // Make this file protected + $this->getAssetStore()->protect( + 'FileTest.txt', + '55b443b60176235ef09801153cca4e6da7494a0c' + ); + + // Should now return explicitly denied errors + $result = $this->get('assets/55b443b601/FileTest.txt'); + $this->assertResponseEquals(403, null, $result); + $result = $this->get('assets/55b443b601/FileTest__variant.txt'); + $this->assertResponseEquals(403, null, $result); + + // Other assets remain available + $result = $this->get('assets/55b443b601/FileTest.pdf'); + $this->assertResponseEquals(200, $expectedContent, $result); + $result = $this->get('assets/55b443b601/FileTest__variant.pdf'); + $this->assertResponseEquals(200, $variantContent, $result); + + // granting access will allow access + $this->getAssetStore()->grant( + 'FileTest.txt', + '55b443b60176235ef09801153cca4e6da7494a0c' + ); + $result = $this->get('assets/55b443b601/FileTest.txt'); + $this->assertResponseEquals(200, $expectedContent, $result); + $result = $this->get('assets/55b443b601/FileTest__variant.txt'); + $this->assertResponseEquals(200, $variantContent, $result); + + // Revoking access will remove access again + $this->getAssetStore()->revoke( + 'FileTest.txt', + '55b443b60176235ef09801153cca4e6da7494a0c' + ); + $result = $this->get('assets/55b443b601/FileTest.txt'); + $this->assertResponseEquals(403, null, $result); + $result = $this->get('assets/55b443b601/FileTest__variant.txt'); + $this->assertResponseEquals(403, null, $result); + + // Moving file back to public store restores access + $this->getAssetStore()->publish( + 'FileTest.txt', + '55b443b60176235ef09801153cca4e6da7494a0c' + ); + $result = $this->get('assets/55b443b601/FileTest.txt'); + $this->assertResponseEquals(200, $expectedContent, $result); + $result = $this->get('assets/55b443b601/FileTest__variant.txt'); + $this->assertResponseEquals(200, $variantContent, $result); + + // Deleting the file will make the response 404 + $this->getAssetStore()->delete( + 'FileTest.txt', + '55b443b60176235ef09801153cca4e6da7494a0c' + ); + $result = $this->get('assets/55b443b601/FileTest.txt'); + $this->assertResponseEquals(404, null, $result); + $result = $this->get('assets/55b443b601/FileTest__variant.txt'); + $this->assertResponseEquals(404, null, $result); + } + + /** + * Test that access to folders is not permitted + */ + public function testFolders() { + $result = $this->get('assets/55b443b601'); + $this->assertResponseEquals(403, null, $result); + + $result = $this->get('assets/FileTest-subfolder'); + $this->assertResponseEquals(403, null, $result); + + $result = $this->get('assets'); + $this->assertResponseEquals(403, null, $result); + } + + /** + * @return AssetStore + */ + protected function getAssetStore() { + return singleton('AssetStore'); + } + + /** + * Assert that a response matches the given parameters + * + * @param int $code HTTP code + * @param string $body Body expected for 200 responses + * @param SS_HTTPResponse $response + */ + protected function assertResponseEquals($code, $body, SS_HTTPResponse $response) { + $this->assertEquals($code, $response->getStatusCode()); + if($code === 200) { + $this->assertFalse($response->isError()); + $this->assertEquals($body, $response->getBody()); + $this->assertEquals('text/plain', $response->getHeader('Content-Type')); + } else { + $this->assertTrue($response->isError()); + } + } + +} \ No newline at end of file diff --git a/tests/forms/DBFileTest.php b/tests/forms/DBFileTest.php index 2b53a7773..252968491 100644 --- a/tests/forms/DBFileTest.php +++ b/tests/forms/DBFileTest.php @@ -1,4 +1,5 @@ MyFile->setFromString('puppies', 'subdir/puppy-document.txt'); } + public function testPermission() { + $obj = new DBFileTest_Object(); + + // Test from image + $fish = realpath(__DIR__ .'/../model/testimages/test-image-high-quality.jpg'); + $this->assertFileExists($fish); + $obj->MyFile->setFromLocalFile($fish, 'private/awesome-fish.jpg', null, null, array( + 'visibility' => AssetStore::VISIBILITY_PROTECTED + )); + + // Test various file permissions work on DBFile + $this->assertFalse($obj->MyFile->canViewFile()); + $obj->MyFile->getURL(); + $this->assertTrue($obj->MyFile->canViewFile()); + $obj->MyFile->revokeFile(); + $this->assertFalse($obj->MyFile->canViewFile()); + $obj->MyFile->getURL(false); + $this->assertFalse($obj->MyFile->canViewFile()); + $obj->MyFile->grantFile(); + $this->assertTrue($obj->MyFile->canViewFile()); + } + } /** @@ -74,14 +97,18 @@ class DBFileTest_Object extends DataObject implements TestOnly { ); } - +/** + * @property DBFile $AnotherFile + */ class DBFileTest_Subclass extends DBFileTest_Object implements TestOnly { private static $db = array( "AnotherFile" => "DBFile" ); } - +/** + * @property DBFile $MyFile + */ class DBFileTest_ImageOnly extends DataObject implements TestOnly { private static $db = array( "MyFile" => "DBFile('image/supported')"