BUG using isDev or isTest query string no longer triggers basic auth

This commit is contained in:
Damian Mooyman 2014-07-01 19:07:13 +12:00
parent 6874e44179
commit d3c7e41419
6 changed files with 258 additions and 99 deletions

View File

@ -979,29 +979,21 @@ class Director implements TemplateGlobalProvider {
/*
* This function will return true if the site is in a live 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 isLive($skipDatabase = false) {
return !(Director::isDev($skipDatabase) || Director::isTest($skipDatabase));
public static function isLive() {
return !(Director::isDev() || Director::isTest());
}
/**
* This function will return true if the site is in a development 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 isDev($skipDatabase = false) {
// This variable is used to supress repetitions of the isDev security message below.
static $firstTimeCheckingGetVar = true;
$result = false;
if(isset($_SESSION['isDev']) && $_SESSION['isDev']) $result = true;
if(Config::inst()->get('Director', 'environment_type') == 'dev') $result = true;
public static function isDev() {
// Check session
if($env = self::session_environment()) return $env === 'dev';
// Check config
if(Config::inst()->get('Director', 'environment_type') === 'dev') return true;
// Check if we are running on one of the test servers
$devServers = (array)Config::inst()->get('Director', 'dev_servers');
@ -1009,54 +1001,22 @@ class Director implements TemplateGlobalProvider {
return true;
}
// Use ?isDev=1 to get development access on the live server
if(!$skipDatabase && !$result && isset($_GET['isDev'])) {
if(Security::database_is_ready()) {
if($firstTimeCheckingGetVar && !Permission::check('ADMIN')){
BasicAuth::requireLogin("SilverStripe developer access. Use your CMS login", "ADMIN");
}
$_SESSION['isDev'] = $_GET['isDev'];
$firstTimeCheckingGetVar = false;
$result = $_GET['isDev'];
} else {
if($firstTimeCheckingGetVar && DB::connection_attempted()) {
echo "<p style=\"padding: 3px; margin: 3px; background-color: orange;
color: white; font-weight: bold\">Sorry, you can't use ?isDev=1 until your
Member and Group tables database are available. Perhaps your database
connection is failing?</p>";
$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

View File

@ -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, <a href='$location'>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;
}
}

View File

@ -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.

View File

@ -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();

View File

@ -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');

View File

@ -1,11 +1,24 @@
<?php
class ParameterConfirmationTokenTest_Token extends ParameterConfirmationToken {
/**
* Dummy parameter token
*/
class ParameterConfirmationTokenTest_Token extends ParameterConfirmationToken implements TestOnly {
public function currentAbsoluteURL() {
return parent::currentAbsoluteURL();
}
}
/**
* A token that always validates a given token
*/
class ParameterConfirmationTokenTest_ValidToken extends ParameterConfirmationTokenTest_Token {
protected function checkToken($token) {
return true;
}
}
class ParameterConfirmationTokenTest extends SapphireTest {
@ -18,6 +31,66 @@ class ParameterConfirmationTokenTest extends SapphireTest {
return array($answer, $slash);
}
public function setUp() {
parent::setUp();
$_GET['parameterconfirmationtokentest_notoken'] = 'value';
$_GET['parameterconfirmationtokentest_empty'] = '';
$_GET['parameterconfirmationtokentest_withtoken'] = '1';
$_GET['parameterconfirmationtokentest_withtokentoken'] = 'dummy';
}
public function tearDown() {
foreach($_GET as $param) {
if(stripos($param, 'parameterconfirmationtokentest_') === 0) unset($_GET[$param]);
}
parent::tearDown();
}
public function testParameterDetectsParameters() {
$withoutToken = new ParameterConfirmationTokenTest_Token('parameterconfirmationtokentest_notoken');
$emptyParameter = new ParameterConfirmationTokenTest_Token('parameterconfirmationtokentest_empty');
$withToken = new ParameterConfirmationTokenTest_ValidToken('parameterconfirmationtokentest_withtoken');
$withoutParameter = new ParameterConfirmationTokenTest_Token('parameterconfirmationtokentest_noparam');
// Check parameter
$this->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');