From c6b1d4aa6bb80e0506a1fc1009e9489006a88b53 Mon Sep 17 00:00:00 2001 From: Ingo Schommer Date: Thu, 6 Dec 2012 16:25:45 +0100 Subject: [PATCH] API Storing alternative DB name in cookie rather than session Session is not initialized by the time we need to use the setting in DB::connect(). Cookie values get initialized automatically for each request. Tightened name format validation to ensure it can only be used for temporary databases, rather than switching the browser session to a different production database. Encrypting token for secure cookie usage. Added dev/generatesecuretoken to generate this token. Not storing in YML config directly because of web access issues. --- dev/DevelopmentAdmin.php | 40 +++++++++++++----- dev/TestRunner.php | 35 ++++++++-------- docs/en/changelogs/3.0.4.md | 12 ++++++ model/DB.php | 82 ++++++++++++++++++++++++++++++++++--- security/Security.php | 8 ++++ tests/model/DBTest.php | 36 ++++++++++++++++ 6 files changed, 181 insertions(+), 32 deletions(-) create mode 100644 docs/en/changelogs/3.0.4.md create mode 100644 tests/model/DBTest.php diff --git a/dev/DevelopmentAdmin.php b/dev/DevelopmentAdmin.php index 07d0874e7..786d7a2f1 100644 --- a/dev/DevelopmentAdmin.php +++ b/dev/DevelopmentAdmin.php @@ -18,15 +18,16 @@ class DevelopmentAdmin extends Controller { ); static $allowed_actions = array( - 'index', - 'tests', - 'jstests', - 'tasks', - 'viewmodel', - 'build', - 'reset', - 'viewcode' - ); + 'index', + 'tests', + 'jstests', + 'tasks', + 'viewmodel', + 'build', + 'reset', + 'viewcode', + 'generatesecuretoken', + ); public function init() { parent::init(); @@ -56,7 +57,7 @@ class DevelopmentAdmin extends Controller { $matched = false; if(isset($_FILE_TO_URL_MAPPING[$testPath])) { $matched = true; - break; + break; } $testPath = dirname($testPath); } @@ -172,6 +173,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 44faddd50..d186ae87e 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/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/security/Security.php b/security/Security.php index 5b3daf07c..f8da0e029 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