BUGFIX Allowing dev/build in "live" mode when Security::database_is_ready() returns FALSE (typically happens when an existing SilverStripe project is upgraded and database columns in Member/Permission/Group have been added) (fixes #4957)

MINOR Using Object::create() in DevelopmentAdmin to make objects mockable
ENHANCEMENT Added Security::$force_database_is_ready to mock database_is_ready() state
ENHANCEMENT Added permission check exception in TaskRunner and DatabaseAdmin if SapphireTest::is_running_test() returns TRUE (necessary for DevelopmentAdminTest) (from r107415)

git-svn-id: svn://svn.silverstripe.com/silverstripe/open/modules/sapphire/trunk@112588 467b73ca-7a2a-4603-9d3b-597d59a354a9

core/model/DatabaseAdmin.php

@@ -32,7 +32,9 @@ class DatabaseAdmin extends Controller {
 		$canAccess = (
 			Director::isDev() 
 			|| !Security::database_is_ready() 
-			|| Director::is_cli() 
+			// We need to ensure that DevelopmentAdminTest can simulate permission failures when running
+			// "dev/tests" from CLI. 
+			|| (Director::is_cli() && !SapphireTest::is_running_test())
 			|| Permission::check("ADMIN")
 		);
 		if(!$canAccess) {

dev/DebugView.php

@@ -11,7 +11,7 @@
  * @package sapphire
  * @subpackage dev
  */
-class DebugView {
+class DebugView extends Object {
 
 	protected static $error_types = array(
 		E_USER_ERROR => array(

dev/DevelopmentAdmin.php

@@ -20,17 +20,19 @@ class DevelopmentAdmin extends Controller {
 	function init() {
 		parent::init();
 		
+		// Special case for dev/build: Defer permission checks to DatabaseAdmin->init() (see #4957)
+		$requestedDevBuild = (stripos($this->request->getURL(), 'dev/build') === 0);
+		
 		// We allow access to this controller regardless of live-status or ADMIN permission only
 		// if on CLI.  Access to this controller is always allowed in "dev-mode", or of the user is ADMIN.
-		$canAccess = (Director::isDev() || Director::is_cli() || Permission::check("ADMIN"));
-		// Special case for dev/build: Allow unauthenticated building of database, emulate DatabaseAdmin->init()
-		// permission restrictions (see #4957)
-		// TODO Decouple sub-controllers like DatabaseAdmin instead of weak URL checking
-		$requestedDevBuild = (stripos($this->request->getURL(), 'dev/build') === 0 && !Security::database_is_ready());
-		
-		if(!$canAccess && !$requestedDevBuild) {
-			return Security::permissionFailure($this);
-		}
+		$canAccess = (
+			$requestedDevBuild 
+			|| Director::isDev() 
+			|| Director::is_cli() 
+			// Its important that we don't run this check if dev/build was requested
+			|| Permission::check("ADMIN")
+		);
+		if(!$canAccess) return Security::permissionFailure($this);
 		
 		// check for valid url mapping
 		// lacking this information can cause really nasty bugs,
@@ -79,7 +81,7 @@ class DevelopmentAdmin extends Controller {
 			// This action is sake-only right now.
 			unset($actions["modules/add"]);
 			
-			$renderer = new DebugView();
+			$renderer = Object::create('DebugView');
 			$renderer->writeHeader();
 			$renderer->writeInfo("Sapphire Development Tools", Director::absoluteBaseURL());
 			$base = Director::baseURL();
@@ -103,33 +105,33 @@ class DevelopmentAdmin extends Controller {
 	}
 	
 	function tests($request) {
-		return new TestRunner();
+		return Object::create('TestRunner');
 	}
 	
 	function jstests($request) {
-		return new JSTestRunner();
+		return Object::create('JSTestRunner');
 	}
 	
 	function tasks() {
-		return new TaskRunner();
+		return Object::create('TaskRunner');
 	}
 	
 	function viewmodel() {
-		return new ModelViewer();
+		return Object::create('ModelViewer');
 	}
 	
-	function build() {
+	function build($request) {
 		if(Director::is_cli()) {
-			$da = new DatabaseAdmin();
-			$da->build();
+			$da = Object::create('DatabaseAdmin');
+			return $da->handleRequest($request);
 		} else {
-			$renderer = new DebugView();
+			$renderer = Object::create('DebugView');
 			$renderer->writeHeader();
 			$renderer->writeInfo("Environment Builder", Director::absoluteBaseURL());
 			echo "<div style=\"margin: 0 2em\">";
 
-			$da = new DatabaseAdmin();
-			$da->build();
+			$da = Object::create('DatabaseAdmin');
+			return $da->handleRequest($request);
 
 			echo "</div>";
 			$renderer->writeFooter();
@@ -143,10 +145,10 @@ class DevelopmentAdmin extends Controller {
 	 *		'build/defaults' => 'buildDefaults',
 	 */
 	function buildDefaults() {
-		$da = new DatabaseAdmin();
+		$da = Object::create('DatabaseAdmin');
 
 		if (!Director::is_cli()) {
-			$renderer = new DebugView();
+			$renderer = Object::create('DebugView');
 			$renderer->writeHeader();
 			$renderer->writeInfo("Defaults Builder", Director::absoluteBaseURL());
 			echo "<div style=\"margin: 0 2em\">";
@@ -175,7 +177,7 @@ class DevelopmentAdmin extends Controller {
 	}
 	
 	function viewcode($request) {
-		return new CodeViewer();
+		return Object::create('CodeViewer');
 	}
 }
 ?>

dev/SapphireTest.php

@@ -39,6 +39,9 @@ class SapphireTest extends PHPUnit_Framework_TestCase {
 	
 	protected $mailer;
 	
+	/**
+	 * @var boolean
+	 */
 	protected static $is_running_test = false;
 	
 	/**
@@ -89,6 +92,15 @@ class SapphireTest extends PHPUnit_Framework_TestCase {
 	 */
 	private $extensionsToReapply = array(), $extensionsToRemove = array();
 	
+	/**
+	 * Determines if unit tests are currently run (via {@link TestRunner}).
+	 * This is used as a cheap replacement for fully mockable state
+	 * in certain contiditions (e.g. access checks).
+	 * Caution: When set to FALSE, certain controllers might bypass
+	 * access checks, so this is a very security sensitive setting.
+	 * 
+	 * @return boolean
+	 */
 	public static function is_running_test() {
 		return self::$is_running_test;
 	}

dev/TaskRunner.php

@@ -13,7 +13,13 @@ class TaskRunner extends Controller {
 	function init() {
 		parent::init();
 		
-		$canAccess = (Director::isDev() || Director::is_cli() || Permission::check("ADMIN"));
+		$canAccess = (
+			Director::isDev() 
+			// We need to ensure that DevelopmentAdminTest can simulate permission failures when running
+			// "dev/tasks" from CLI.
+			|| (Director::is_cli() && !SapphireTest::is_running_test())
+			|| Permission::check("ADMIN")
+		);
 		if(!$canAccess) return Security::permissionFailure($this);
 	}
 	

security/Security.php

@@ -98,6 +98,12 @@ class Security extends Controller {
 	 */
 	protected static $login_recording = false;
 	
+	/**
+	 * @var boolean If set to TRUE or FALSE, {@link database_is_ready()}
+	 * will always return FALSE. Used for unit testing.
+	 */
+	static $force_database_is_ready = null;
+	
 	/**
 	 * Set location of word list file
 	 * 
@@ -846,6 +852,9 @@ class Security extends Controller {
 	 * @return bool
 	 */
 	public static function database_is_ready() {
+		// Used for unit tests
+		if(self::$force_database_is_ready !== NULL) return self::$force_database_is_ready;
+		
 		$requiredTables = ClassInfo::dataClassesFor('Member');
 		$requiredTables[] = 'Group';
 		$requiredTables[] = 'Permission';