Sorry, you can't use ?isDev=1 until your - Member and Group tables database are available. Perhaps your database - connection is failing?
"; - $firstTimeCheckingGetVar = false; - } - } - } - - return $result; + return false; } /** * This function will return true if the site is in a test environment. * For information about environment types, see {@link Director::set_environment_type()}. - * - * @param $skipDatabase Skips database checks for current login permissions if set to TRUE, - * which is useful for checks happening before the database is functional. */ - public static function isTest($skipDatabase = false) { - // Use ?isTest=1 to get test access on the live server, or explicitly set your environment - if(!$skipDatabase && isset($_GET['isTest'])) { - if(Security::database_is_ready()) { - BasicAuth::requireLogin("SilverStripe developer access. Use your CMS login", "ADMIN"); - $_SESSION['isTest'] = $_GET['isTest']; - } else { - return true; - } - } + public static function isTest() { + // In case of isDev and isTest both being set, dev has higher priority + if(self::isDev()) return false; - if(self::isDev($skipDatabase)) { - return false; - } + // Check saved session + if($env = self::session_environment()) return $env === 'test'; - if(Config::inst()->get('Director', 'environment_type')) { - return Config::inst()->get('Director', 'environment_type') == 'test'; - } + // Check config + if(Config::inst()->get('Director', 'environment_type') === 'test') return true; // Check if we are running on one of the test servers $testServers = (array)Config::inst()->get('Director', 'test_servers'); @@ -1066,6 +1026,36 @@ class Director implements TemplateGlobalProvider { return false; } + + /** + * Check or update any temporary environment specified in the session + * + * @return string 'test', 'dev', or null + */ + protected static function session_environment() { + // Set session from querystring + if(isset($_GET['isDev'])) { + if(isset($_SESSION)) { + unset($_SESSION['isTest']); // In case we are changing from test mode + $_SESSION['isDev'] = $_GET['isDev']; + } + return 'dev'; + } elseif(isset($_GET['isTest'])) { + if(isset($_SESSION)) { + unset($_SESSION['isDev']); // In case we are changing from dev mode + $_SESSION['isTest'] = $_GET['isTest']; + } + return 'test'; + } + // Check session + if(isset($_SESSION['isDev']) && $_SESSION['isDev']) { + return 'dev'; + } elseif(isset($_SESSION['isTest']) && $_SESSION['isTest']) { + return 'test'; + } else { + return null; + } + } /** * @return array Returns an array of strings of the method names of methods on the call that should be exposed diff --git a/core/startup/ParameterConfirmationToken.php b/core/startup/ParameterConfirmationToken.php index 495bd503a..35eb88a70 100644 --- a/core/startup/ParameterConfirmationToken.php +++ b/core/startup/ParameterConfirmationToken.php @@ -54,14 +54,49 @@ class ParameterConfirmationToken { // If a token was provided, but isn't valid, ignore it if ($this->token && (!$this->checkToken($this->token))) $this->token = null; } + + /** + * Get the name of this token + * + * @return string + */ + public function getName() { + return $this->parameterName; + } + /** + * Is the parameter requested? + * + * @return bool + */ public function parameterProvided() { return $this->parameter !== null; } + /** + * Is the necessary token provided for this parameter? + * + * @return bool + */ public function tokenProvided() { return $this->token !== null; } + + /** + * Is this parameter requested without a valid token? + * + * @return bool True if the parameter is given without a valid token + */ + public function reloadRequired() { + return $this->parameterProvided() && !$this->tokenProvided(); + } + + /** + * Suppress the current parameter by unsetting it from $_GET + */ + public function suppress() { + unset($_GET[$this->parameterName]); + } public function params() { return array( @@ -139,4 +174,24 @@ You are being redirected. If you are not redirected soon, cl else header('location: '.$location, true, 302); die; } + + /** + * Given a list of token names, suppress all tokens that have not been validated, and + * return the non-validated token with the highest priority + * + * @param type $keys List of token keys in ascending priority (low to high) + * @return ParameterConfirmationToken The token container for the unvalidated $key given with the highest priority + */ + public static function prepare_tokens($keys) { + $target = null; + foreach($keys as $key) { + $token = new ParameterConfirmationToken($key); + // Validate this token + if($token->reloadRequired()) { + $token->suppress(); + $target = $token; + } + } + return $target; + } } diff --git a/docs/en/changelogs/3.1.6.md b/docs/en/changelogs/3.1.6.md new file mode 100644 index 000000000..54d68db30 --- /dev/null +++ b/docs/en/changelogs/3.1.6.md @@ -0,0 +1,9 @@ +# 3.1.6 + +## Upgrading + + * The use of the isDev or isTest query string parameter is now restricted to those logged in as admin, + and requires users to login via the front end form rather than using basic authentication. This + follows the same process as the use of the 'flush' query string parameter, and will redirect + requests containing this argument with a token in the querystring. + Director::isDev, Director::isTest, and Director::isLive no longer have any parameters. diff --git a/main.php b/main.php index 888376897..9f10da239 100644 --- a/main.php +++ b/main.php @@ -110,18 +110,13 @@ if (substr(strtolower($url), 0, strlen(BASE_URL)) == strtolower(BASE_URL)) $url require_once('core/startup/ErrorControlChain.php'); require_once('core/startup/ParameterConfirmationToken.php'); +// Prepare tokens and execute chain +$reloadToken = ParameterConfirmationToken::prepare_tokens(array('isTest', 'isDev', 'flush')); $chain = new ErrorControlChain(); -$token = new ParameterConfirmationToken('flush'); - $chain - ->then(function($chain) use ($token){ - // First, if $_GET['flush'] was set, but no valid token, suppress the flush - if (isset($_GET['flush']) && !$token->tokenProvided()) { - unset($_GET['flush']); - } - else { - $chain->setSuppression(false); - } + ->then(function($chain) use ($reloadToken) { + // If no redirection is necessary then we can disable error supression + if (!$reloadToken) $chain->setSuppression(false); // Load in core require_once('core/Core.php'); @@ -130,38 +125,30 @@ $chain require_once('model/DB.php'); global $databaseConfig; if ($databaseConfig) DB::connect($databaseConfig); - - // Then if a flush was requested, redirect to it - if ($token->parameterProvided() && !$token->tokenProvided()) { - // First, check if we're in dev mode, or the database doesn't have any security data - $canFlush = Director::isDev(true) || !Security::database_is_ready(); - - // Otherwise, we start up the session if needed, then check for admin - if (!$canFlush) { - if(!isset($_SESSION) && Session::request_contains_session_id()) { - Session::start(); - } - - if (Permission::check('ADMIN')) { - $canFlush = true; - } - else { - $loginPage = Director::absoluteURL(Config::inst()->get('Security', 'login_url')); - $loginPage .= "?BackURL=" . urlencode($_SERVER['REQUEST_URI']); - - header('location: '.$loginPage, true, 302); - die; - } - } - - // And if we can flush, reload with an authority token - if ($canFlush) $token->reloadWithToken(); + + // Check if a token is requesting a redirect + if (!$reloadToken) return; + + // Otherwise, we start up the session if needed + if(!isset($_SESSION) && Session::request_contains_session_id()) { + Session::start(); } + + // Next, check if we're in dev mode, or the database doesn't have any security data, or we are admin + if (Director::isDev() || !Security::database_is_ready() || Permission::check('ADMIN')) { + return $reloadToken->reloadWithToken(); + } + + // Fail and redirect the user to the login page + $loginPage = Director::absoluteURL(Config::inst()->get('Security', 'login_url')); + $loginPage .= "?BackURL=" . urlencode($_SERVER['REQUEST_URI']); + header('location: '.$loginPage, true, 302); + die; }) - // Finally if a flush was requested but there was an error while figuring out if it's allowed, do it anyway - ->thenIfErrored(function() use ($token){ - if ($token->parameterProvided() && !$token->tokenProvided()) { - $token->reloadWithToken(); + // Finally if a token was requested but there was an error while figuring out if it's allowed, do it anyway + ->thenIfErrored(function() use ($reloadToken){ + if ($reloadToken) { + $reloadToken->reloadWithToken(); } }) ->execute(); diff --git a/tests/control/DirectorTest.php b/tests/control/DirectorTest.php index acd522dbb..1e4d0e0c9 100644 --- a/tests/control/DirectorTest.php +++ b/tests/control/DirectorTest.php @@ -10,6 +10,10 @@ class DirectorTest extends SapphireTest { protected static $originalRequestURI; protected $originalProtocolHeaders = array(); + + protected $originalGet = array(); + + protected $originalSession = array(); public function setUp() { parent::setUp(); @@ -22,6 +26,9 @@ class DirectorTest extends SapphireTest { self::$originalRequestURI = $_SERVER['REQUEST_URI']; } + $this->originalGet = $_GET; + $this->originalSession = $_SESSION; + Config::inst()->update('Director', 'rules', array( 'DirectorTestRule/$Action/$ID/$OtherID' => 'DirectorTestRequest_Controller', 'en-nz/$Action/$ID/$OtherID' => array( @@ -47,6 +54,9 @@ class DirectorTest extends SapphireTest { // Remove base URL override (setting to false reverts to default behaviour) Config::inst()->update('Director', 'alternate_base_url', false); + $_GET = $this->originalGet; + $_SESSION = $this->originalSession; + // Reinstate the original REQUEST_URI after it was modified by some tests $_SERVER['REQUEST_URI'] = self::$originalRequestURI; @@ -221,6 +231,41 @@ class DirectorTest extends SapphireTest { $this->assertFalse(Director::is_site_url("//test.com?url=" . Director::absoluteBaseURL())); } + /** + * Tests isDev, isTest, isLive set from querystring + */ + public function testQueryIsEnvironment() { + // Reset + unset($_SESSION['isDev']); + unset($_SESSION['isLive']); + unset($_GET['isTest']); + unset($_GET['isDev']); + + // Test isDev=1 + $_GET['isDev'] = '1'; + $this->assertTrue(Director::isDev()); + $this->assertFalse(Director::isTest()); + $this->assertFalse(Director::isLive()); + + // Test persistence + unset($_GET['isDev']); + $this->assertTrue(Director::isDev()); + $this->assertFalse(Director::isTest()); + $this->assertFalse(Director::isLive()); + + // Test change to isTest + $_GET['isTest'] = '1'; + $this->assertFalse(Director::isDev()); + $this->assertTrue(Director::isTest()); + $this->assertFalse(Director::isLive()); + + // Test persistence + unset($_GET['isTest']); + $this->assertFalse(Director::isDev()); + $this->assertTrue(Director::isTest()); + $this->assertFalse(Director::isLive()); + } + public function testResetGlobalsAfterTestRequest() { $_GET = array('somekey' => 'getvalue'); $_POST = array('somekey' => 'postvalue'); diff --git a/tests/core/startup/ParameterConfirmationTokenTest.php b/tests/core/startup/ParameterConfirmationTokenTest.php index ad0795598..18de1f886 100644 --- a/tests/core/startup/ParameterConfirmationTokenTest.php +++ b/tests/core/startup/ParameterConfirmationTokenTest.php @@ -1,11 +1,24 @@ assertTrue($withoutToken->parameterProvided()); + $this->assertTrue($emptyParameter->parameterProvided()); // even if empty, it's still provided + $this->assertTrue($withToken->parameterProvided()); + $this->assertFalse($withoutParameter->parameterProvided()); + + // Check token + $this->assertFalse($withoutToken->tokenProvided()); + $this->assertFalse($emptyParameter->tokenProvided()); + $this->assertTrue($withToken->tokenProvided()); + $this->assertFalse($withoutParameter->tokenProvided()); + + // Check if reload is required + $this->assertTrue($withoutToken->reloadRequired()); + $this->assertTrue($emptyParameter->reloadRequired()); + $this->assertFalse($withToken->reloadRequired()); + $this->assertFalse($withoutParameter->reloadRequired()); + + // Check suppression + $this->assertTrue(isset($_GET['parameterconfirmationtokentest_notoken'])); + $withoutToken->suppress(); + $this->assertFalse(isset($_GET['parameterconfirmationtokentest_notoken'])); + } + + public function testPrepareTokens() { + // Test priority ordering + $token = ParameterConfirmationToken::prepare_tokens(array( + 'parameterconfirmationtokentest_notoken', + 'parameterconfirmationtokentest_empty', + 'parameterconfirmationtokentest_noparam' + )); + // Test no invalid tokens + $this->assertEquals('parameterconfirmationtokentest_empty', $token->getName()); + $token = ParameterConfirmationToken::prepare_tokens(array( + 'parameterconfirmationtokentest_noparam' + )); + $this->assertEmpty($token); + } /** * currentAbsoluteURL needs to handle base or url being missing, or any combination of slashes. @@ -25,7 +98,7 @@ class ParameterConfirmationTokenTest extends SapphireTest { * There should always be exactly one slash between each part in the result, and any trailing slash * should be preserved. */ - function testCurrentAbsoluteURLHandlesSlashes() { + public function testCurrentAbsoluteURLHandlesSlashes() { global $url; $token = new ParameterConfirmationTokenTest_Token('parameterconfirmationtokentest_parameter');