diff --git a/.travis.yml b/.travis.yml index 1f49fe992..214f1ca08 100644 --- a/.travis.yml +++ b/.travis.yml @@ -6,6 +6,7 @@ php: env: - TESTDB=MYSQL - TESTDB=PGSQL + - TESTDB=SQLITE matrix: exclude: @@ -13,6 +14,15 @@ matrix: env: TESTDB=PGSQL - php: 5.4 env: TESTDB=SQLITE + include: + - php: 5.4 + env: + - PHPCS=1 + allow_failures: + - env: TESTDB=PGSQL + - php: 5.4 + env: + - PHPCS=1 before_script: - pear install pear/PHP_CodeSniffer @@ -21,17 +31,13 @@ before_script: - cd ~/builds/ss script: - - phpunit -c phpunit.xml.dist - - phpcs --encoding=utf-8 --tab-width=4 --standard=framework/tests/phpcs/ruleset.xml -np framework - - phpcs --encoding=utf-8 --standard=framework/tests/phpcs/tabs.xml -np framework + - sh -c "if [ '$PHPCS' != '1' ]; then phpunit -c phpunit.xml.dist; else phpcs --encoding=utf-8 --tab-width=4 --standard=framework/tests/phpcs/ruleset.xml -np framework && phpcs --encoding=utf-8 --standard=framework/tests/phpcs/tabs.xml -np framework; fi" branches: except: - 2.1 - 2.2 - 2.3 - - 2.4 - - post-2.4 - translation-staging notifications: diff --git a/dev/DevelopmentAdmin.php b/dev/DevelopmentAdmin.php index 8dd45e743..ac76a27fa 100644 --- a/dev/DevelopmentAdmin.php +++ b/dev/DevelopmentAdmin.php @@ -25,7 +25,8 @@ class DevelopmentAdmin extends Controller { 'viewmodel', 'build', 'reset', - 'viewcode' + 'viewcode', + 'generatesecuretoken', ); public function init() { @@ -171,6 +172,25 @@ class DevelopmentAdmin extends Controller { } } + /** + * Generate a secure token which can be used as a crypto key. + * Returns the token and suggests PHP configuration to set it. + */ + public function generatesecuretoken() { + $generator = Injector::inst()->create('RandomGenerator'); + $token = $generator->randomToken('sha1'); + + echo <<update('Security', 'token', '$token'); + + +TXT; + } + public function reset() { $link = BASE_URL.'/dev/tests/startsession'; diff --git a/dev/TestRunner.php b/dev/TestRunner.php index d4e17f332..f09fb9714 100644 --- a/dev/TestRunner.php +++ b/dev/TestRunner.php @@ -349,6 +349,9 @@ class TestRunner extends Controller { * See {@link setdb()} for an alternative approach which just sets a database * name, and is used for more advanced use cases like interacting with test databases * directly during functional tests. + * + * Requires PHP's mycrypt extension in order to set the database name + * as an encrypted cookie. */ public function startsession() { if(!Director::isLive()) { @@ -420,7 +423,7 @@ HTML; } /** - * Set an alternative database name in the current browser session. + * Set an alternative database name in the current browser session as a cookie. * Useful for functional testing libraries like behat to create a "clean slate". * Does not actually create the database, that's usually handled * by {@link SapphireTest::create_temp_db()}. @@ -432,33 +435,33 @@ HTML; * * See {@link startsession()} for a different approach which actually creates * the DB and loads a fixture file instead. + * + * Requires PHP's mycrypt extension in order to set the database name + * as an encrypted cookie. */ public function setdb() { if(Director::isLive()) { - return $this->permissionFailure("dev/tests/setdb can only be used on dev and test sites"); + return $this->httpError(403, "dev/tests/setdb can only be used on dev and test sites"); } - if(!isset($_GET['database'])) { - return $this->permissionFailure("dev/tests/setdb must be used with a 'database' parameter"); + return $this->httpError(400, "dev/tests/setdb must be used with a 'database' parameter"); } - $database_name = $_GET['database']; + $name = $_GET['database']; $prefix = defined('SS_DATABASE_PREFIX') ? SS_DATABASE_PREFIX : 'ss_'; $pattern = strtolower(sprintf('#^%stmpdb\d{7}#', $prefix)); - if(!preg_match($pattern, $database_name)) { - return $this->permissionFailure("Invalid database name format"); + if($name && !preg_match($pattern, $name)) { + return $this->httpError(400, "Invalid database name format"); } - DB::set_alternative_database_name($database_name); + DB::set_alternative_database_name($name); - return "

Set database session to '$database_name'. Time to start testing; where would you like to start?

- "; + if($name) { + return "

Set database session to '$name'.

"; + } else { + return "

Unset database session.

"; + } + } public function emptydb() { diff --git a/docs/en/changelogs/3.0.4.md b/docs/en/changelogs/3.0.4.md new file mode 100644 index 000000000..fd9f55e1f --- /dev/null +++ b/docs/en/changelogs/3.0.4.md @@ -0,0 +1,12 @@ +# 3.0.4 + +## Overview + + * Changed `dev/tests/setdb` and `dev/tests/startsession` from session to cookie storage. + +## Upgrading + + * If you are using `dev/tests/setdb` and `dev/tests/startsession`, + you'll need to configure a secure token in order to encrypt the cookie value: + Simply run `sake dev/generatesecuretoken` and add the resulting code to your `mysite/_config.php`. + Note that this functionality now requires the PHP `mcrypt` extension. \ No newline at end of file diff --git a/docs/en/topics/index.md b/docs/en/topics/index.md index 49b5447ae..b70885bff 100644 --- a/docs/en/topics/index.md +++ b/docs/en/topics/index.md @@ -14,7 +14,6 @@ It is where most documentation should live, and is the natural "second step" aft * [Emails](email): Configuring and sending emails * [Environment management](environment-management): Sharing configuration details (e.g. database login, passwords) with multiple websites via a `_ss_environment.php` file * [Error Handling](error-handling): Error messages and filesystem logs - * [Extending the CMS](extending-the-cms): Introduction to changing the default CMS editor * [Files and Images](files): File and Image management in the database and how to manipulate images * [Form Validation](form-validation): Built-in validation on form fields, and how to extend it * [Forms](forms): Create your own form, add fields and create your own form template using the existing `Form` class @@ -29,4 +28,4 @@ It is where most documentation should live, and is the natural "second step" aft * [Testing](testing): Functional and Unit Testing with PHPUnit and SilverStripe's testing framework * [Developing Themes](theme-development): Package templates, images and CSS to a reusable theme * [Widgets](widgets): Small feature blocks which can be placed on a page by the CMS editor, also outlines how to create and add widgets - * [Versioning](versioning): Extension for SiteTree and other classes to store old versions and provide "staging" \ No newline at end of file + * [Versioning](versioning): Extension for SiteTree and other classes to store old versions and provide "staging" diff --git a/javascript/HtmlEditorField.js b/javascript/HtmlEditorField.js index 5f7724eb5..dc295ba4e 100644 --- a/javascript/HtmlEditorField.js +++ b/javascript/HtmlEditorField.js @@ -883,7 +883,7 @@ ss.editorWrappers['default'] = ss.editorWrappers.tinyMCE; //get the uploaded file ID when this event triggers, signaling the upload has compeleted successfully editFieldIDs.push($(this).data('id')); }); - var uploadedFiles = $('.ss-uploadfield-files').children('.ss-uploadfield-item'); + var uploadedFiles = form.find('.ss-uploadfield-files').children('.ss-uploadfield-item'); uploadedFiles.each(function(){ var uploadedID = $(this).data('fileid'); if ($.inArray(uploadedID, editFieldIDs) == -1) { diff --git a/model/DB.php b/model/DB.php index 70ef907ac..d9c1a3dac 100644 --- a/model/DB.php +++ b/model/DB.php @@ -60,19 +60,89 @@ class DB { } /** - * Set an alternative database to use for this browser session. - * This is useful when using testing systems other than SapphireTest; for example, Windmill. + * Set an alternative database in a browser cookie, + * with the cookie lifetime set to the browser session. + * This is useful for integration testing on temporary databases. + * + * There is a strict naming convention for temporary databases to avoid abuse: + * (default: 'ss_') + tmpdb + <7 digits> + * As an additional security measure, temporary databases will + * be ignored in "live" mode. + * + * Note that the database will be set on the next request. * Set it to null to revert to the main database. */ - public static function set_alternative_database_name($dbname) { - Session::set("alternativeDatabaseName", $dbname); + public static function set_alternative_database_name($name = null) { + if($name) { + if(!self::valid_alternative_database_name($name)) { + throw new InvalidArgumentException(sprintf( + 'Invalid alternative database name: "%s"', + $name + )); + } + + $key = Config::inst()->get('Security', 'token'); + if(!$key) { + throw new LogicException('"Security.token" not found, run "sake dev/generatesecuretoken"'); + } + if(!function_exists('mcrypt_encrypt')) { + throw new LogicException('DB::set_alternative_database_name() requires the mcrypt PHP extension'); + } + + $key = md5($key); // Ensure key is correct length for chosen cypher + $ivSize = mcrypt_get_iv_size(MCRYPT_RIJNDAEL_256, MCRYPT_MODE_CFB); + $iv = mcrypt_create_iv($ivSize); + $encrypted = mcrypt_encrypt( + MCRYPT_RIJNDAEL_256, $key, $name, MCRYPT_MODE_CFB, $iv + ); + + // Set to browser session lifetime, and restricted to HTTP access only + Cookie::set("alternativeDatabaseName", base64_encode($encrypted), 0, null, null, false, true); + Cookie::set("alternativeDatabaseNameIv", base64_encode($iv), 0, null, null, false, true); + } else { + Cookie::set("alternativeDatabaseName", null, 0, null, null, false, true); + Cookie::set("alternativeDatabaseNameIv", null, 0, null, null, false, true); + } } /** * Get the name of the database in use */ public static function get_alternative_database_name() { - return Session::get("alternativeDatabaseName"); + $name = Cookie::get("alternativeDatabaseName"); + $iv = Cookie::get("alternativeDatabaseNameIv"); + + if($name) { + $key = Config::inst()->get('Security', 'token'); + if(!$key) { + throw new LogicException('"Security.token" not found, run "sake dev/generatesecuretoken"'); + } + if(!function_exists('mcrypt_encrypt')) { + throw new LogicException('DB::set_alternative_database_name() requires the mcrypt PHP extension'); + } + $key = md5($key); // Ensure key is correct length for chosen cypher + $decrypted = mcrypt_decrypt( + MCRYPT_RIJNDAEL_256, $key, base64_decode($name), MCRYPT_MODE_CFB, base64_decode($iv) + ); + return (self::valid_alternative_database_name($decrypted)) ? $decrypted : false; + } else { + return false; + } + } + + /** + * Determines if the name is valid, as a security + * measure against setting arbitrary databases. + * + * @param String $name + * @return Boolean + */ + public static function valid_alternative_database_name($name) { + if(Director::isLive()) return false; + + $prefix = defined('SS_DATABASE_PREFIX') ? SS_DATABASE_PREFIX : 'ss_'; + $pattern = strtolower(sprintf('/^%stmpdb\d{7}$/', $prefix)); + return (bool)preg_match($pattern, $name); } /** @@ -84,7 +154,7 @@ class DB { */ public static function connect($databaseConfig) { // This is used by TestRunner::startsession() to test up a test session using an alt - if($name = Session::get('alternativeDatabaseName')) { + if($name = self::get_alternative_database_name()) { $databaseConfig['database'] = $name; } diff --git a/model/DataQuery.php b/model/DataQuery.php index bd1e968f3..16d3a1276 100644 --- a/model/DataQuery.php +++ b/model/DataQuery.php @@ -193,7 +193,7 @@ class DataQuery { } if ($joinTable) { - $query->addLeftJoin($tableClass, "\"$tableClass\".\"ID\" = \"$baseClass\".\"ID\"") ; + $query->addLeftJoin($tableClass, "\"$tableClass\".\"ID\" = \"$baseClass\".\"ID\"", $tableClass, 10) ; } } diff --git a/model/SQLQuery.php b/model/SQLQuery.php index 7981c61d4..6d823ae74 100644 --- a/model/SQLQuery.php +++ b/model/SQLQuery.php @@ -276,18 +276,28 @@ class SQLQuery { * Add a LEFT JOIN criteria to the FROM clause. * * @param string $table Unquoted table name - * @param string $onPredicate The "ON" SQL fragment in a "LEFT JOIN ... AS ... ON ..." statement, - * Needs to be valid (quoted) SQL. + * @param string $onPredicate The "ON" SQL fragment in a "LEFT JOIN ... AS ... ON ..." statement, Needs to be valid + * (quoted) SQL. * @param string $tableAlias Optional alias which makes it easier to identify and replace joins later on + * @param int $order A numerical index to control the order that joins are added to the query; lower order values + * will cause the query to appear first. The default is 20, and joins created automatically by the + * ORM have a value of 10. * @return SQLQuery */ - public function addLeftJoin($table, $onPredicate, $tableAlias = null) { - if(!$tableAlias) $tableAlias = $table; - $this->from[$tableAlias] = array('type' => 'LEFT', 'table' => $table, 'filter' => array($onPredicate)); + public function addLeftJoin($table, $onPredicate, $tableAlias = '', $order = 20) { + if(!$tableAlias) { + $tableAlias = $table; + } + $this->from[$tableAlias] = array( + 'type' => 'LEFT', + 'table' => $table, + 'filter' => array($onPredicate), + 'order' => $order + ); return $this; } - public function leftjoin($table, $onPredicate, $tableAlias = null) { + public function leftjoin($table, $onPredicate, $tableAlias = null, $order = 20) { Deprecation::notice('3.0', 'Please use addLeftJoin() instead!'); $this->addLeftJoin($table, $onPredicate, $tableAlias); } @@ -296,20 +306,28 @@ class SQLQuery { * Add an INNER JOIN criteria to the FROM clause. * * @param string $table Unquoted table name - * @param string $onPredicate The "ON" SQL fragment in an "INNER JOIN ... AS ... ON ..." statement. - * Needs to be valid (quoted) SQL. + * @param string $onPredicate The "ON" SQL fragment in an "INNER JOIN ... AS ... ON ..." statement. Needs to be + * valid (quoted) SQL. * @param string $tableAlias Optional alias which makes it easier to identify and replace joins later on + * @param int $order A numerical index to control the order that joins are added to the query; lower order values + * will cause the query to appear first. The default is 20, and joins created automatically by the + * ORM have a value of 10. * @return SQLQuery */ - public function addInnerJoin($table, $onPredicate, $tableAlias = null) { + public function addInnerJoin($table, $onPredicate, $tableAlias = null, $order = 20) { if(!$tableAlias) $tableAlias = $table; - $this->from[$tableAlias] = array('type' => 'INNER', 'table' => $table, 'filter' => array($onPredicate)); + $this->from[$tableAlias] = array( + 'type' => 'INNER', + 'table' => $table, + 'filter' => array($onPredicate), + 'order' => $order + ); return $this; } - public function innerjoin($table, $onPredicate, $tableAlias = null) { + public function innerjoin($table, $onPredicate, $tableAlias = null, $order = 20) { Deprecation::notice('3.0', 'Please use addInnerJoin() instead!'); - return $this->addInnerJoin($table, $onPredicate, $tableAlias); + return $this->addInnerJoin($table, $onPredicate, $tableAlias, $order); } /** @@ -869,12 +887,13 @@ class SQLQuery { // TODO: Don't require this internal-state manipulate-and-preserve - let sqlQueryToString() handle the new // syntax $origFrom = $this->from; - + // Sort the joins + $this->from = $this->getOrderedJoins($this->from); // Build from clauses foreach($this->from as $alias => $join) { // $join can be something like this array structure // array('type' => 'inner', 'table' => 'SiteTree', 'filter' => array("SiteTree.ID = 1", - // "Status = 'approved'")) + // "Status = 'approved'", 'order' => 20)) if(is_array($join)) { if(is_string($join['filter'])) $filter = $join['filter']; else if(sizeof($join['filter']) == 1) $filter = $join['filter'][0]; @@ -895,10 +914,9 @@ class SQLQuery { $this->from = $origFrom; // The query was most likely just created and then exectued. - if($sql === 'SELECT *') { + if(trim($sql) === 'SELECT * FROM') { return ''; } - return $sql; } @@ -1079,6 +1097,81 @@ class SQLQuery { $query->setLimit(1, $index); return $query; } + + /** + * Ensure that framework "auto-generated" table JOINs are first in the finalised SQL query. + * This prevents issues where developer-initiated JOINs attempt to JOIN using relations that haven't actually + * yet been scaffolded by the framework. Demonstrated by PostGres in errors like: + *"...ERROR: missing FROM-clause..." + * + * @param $from array - in the format of $this->select + * @return array - and reorderded list of selects + */ + protected function getOrderedJoins($from) { + // shift the first FROM table out from so we only deal with the JOINs + $baseFrom = array_shift($from); + $this->mergesort($from, function($firstJoin, $secondJoin) { + if($firstJoin['order'] == $secondJoin['order']) { + return 0; + } + return ($firstJoin['order'] < $secondJoin['order']) ? -1 : 1; + }); + + // Put the first FROM table back into the results + array_unshift($from, $baseFrom); + return $from; + } + /** + * Since uasort don't preserve the order of an array if the comparison is equal + * we have to resort to a merge sort. It's quick and stable: O(n*log(n)). + * + * @see http://stackoverflow.com/q/4353739/139301 + * + * @param array &$array - the array to sort + * @param callable $cmpFunction - the function to use for comparison + */ + protected function mergesort(&$array, $cmpFunction = 'strcmp') { + // Arrays of size < 2 require no action. + if (count($array) < 2) { + return; + } + // Split the array in half + $halfway = count($array) / 2; + $array1 = array_slice($array, 0, $halfway); + $array2 = array_slice($array, $halfway); + // Recurse to sort the two halves + $this->mergesort($array1, $cmpFunction); + $this->mergesort($array2, $cmpFunction); + // If all of $array1 is <= all of $array2, just append them. + if(call_user_func($cmpFunction, end($array1), reset($array2)) < 1) { + $array = array_merge($array1, $array2); + return; + } + // Merge the two sorted arrays into a single sorted array + $array = array(); + $val1 = reset($array1); + $val2 = reset($array2); + do { + if (call_user_func($cmpFunction, $val1, $val2) < 1) { + $array[key($array1)] = $val1; + $val1 = next($array1); + } else { + $array[key($array2)] = $val2; + $val2 = next($array2); + } + } while($val1 && $val2); + + // Merge the remainder + while($val1) { + $array[key($array1)] = $val1; + $val1 = next($array1); + } + while($val2) { + $array[key($array2)] = $val2; + $val2 = next($array2); + } + return; + } } diff --git a/security/Security.php b/security/Security.php index 773164b98..383248ae9 100644 --- a/security/Security.php +++ b/security/Security.php @@ -82,6 +82,14 @@ class Security extends Controller { * @var array|string */ protected static $default_message_set = ''; + + /** + * Random secure token, can be used as a crypto key internally. + * Generate one through 'sake dev/generatesecuretoken'. + * + * @var String + */ + public static $token; /** * Get location of word list file diff --git a/tests/model/DBTest.php b/tests/model/DBTest.php new file mode 100644 index 000000000..136d74905 --- /dev/null +++ b/tests/model/DBTest.php @@ -0,0 +1,36 @@ +origEnvType = Director::get_environment_type(); + Director::set_environment_type('dev'); + + parent::setUp(); + } + + function tearDown() { + Director::set_environment_type($this->origEnvType); + + parent::tearDown(); + } + + function testValidAlternativeDatabaseName() { + $this->assertTrue(DB::valid_alternative_database_name('ss_tmpdb1234567')); + $this->assertFalse(DB::valid_alternative_database_name('ss_tmpdb12345678')); + $this->assertFalse(DB::valid_alternative_database_name('tmpdb1234567')); + $this->assertFalse(DB::valid_alternative_database_name('random')); + $this->assertFalse(DB::valid_alternative_database_name('')); + + $origEnvType = Director::get_environment_type(); + Director::set_environment_type('live'); + $this->assertFalse(DB::valid_alternative_database_name('ss_tmpdb1234567')); + Director::set_environment_type($origEnvType); + } + +} \ No newline at end of file diff --git a/tests/model/DataQueryTest.php b/tests/model/DataQueryTest.php index d3d731110..95911f343 100644 --- a/tests/model/DataQueryTest.php +++ b/tests/model/DataQueryTest.php @@ -1,6 +1,13 @@ innerJoin('DataQueryTest_D', '"DataQueryTest_D"."RelationID" = "DataQueryTest_B"."ID"'); + $dataQuery->execute(); + } + public function testDisjunctiveGroup() { $dq = new DataQuery('DataQueryTest_A'); @@ -126,6 +139,7 @@ class DataQueryTest_B extends DataQueryTest_A { } class DataQueryTest_C extends DataObject implements TestOnly { + public static $has_one = array( 'TestA' => 'DataQueryTest_A', 'TestB' => 'DataQueryTest_B', @@ -141,3 +155,10 @@ class DataQueryTest_C extends DataObject implements TestOnly { 'ManyTestBs' => 'DataQueryTest_B', ); } + +class DataQueryTest_D extends DataObject implements TestOnly { + + public static $has_one = array( + 'Relation' => 'DataQueryTest_B', + ); +} diff --git a/tests/model/SQLQueryTest.php b/tests/model/SQLQueryTest.php index 8e2b25984..f59c2d519 100755 --- a/tests/model/SQLQueryTest.php +++ b/tests/model/SQLQueryTest.php @@ -292,6 +292,7 @@ class SQLQueryTest extends SapphireTest { $query->sql() ); } + public function testSetWhereAny() { $query = new SQLQuery(); @@ -376,4 +377,17 @@ class SQLQueryTest_DO extends DataObject implements TestOnly { ); } +class SQLQueryTestBase extends DataObject implements TestOnly { + static $db = array( + "Title" => "Varchar", + ); +} +class SQLQueryTestChild extends SQLQueryTestBase { + static $db = array( + "Name" => "Varchar", + ); + + static $has_one = array( + ); +} diff --git a/tests/travis/before_script b/tests/travis/before_script index d537d174b..adcdc0465 100755 --- a/tests/travis/before_script +++ b/tests/travis/before_script @@ -4,6 +4,14 @@ BUILD_DIR=$1 +# Environment info +echo "# Environment info" +echo " - `php --version`" +echo " - `mysql --version`" +echo " - `pg_config --version`" +echo " - SQLite3 `sqlite3 -version`" +echo "" + # Fetch all dependencies # TODO Replace with different composer.json variations