From e0ec30dcc843fa10955746154f442a86199379e0 Mon Sep 17 00:00:00 2001
From: Ingo Schommer <ingo@silverstripe.com>
Date: Thu, 2 Apr 2009 16:34:27 +0000
Subject: [PATCH] ENHANCEMENT Allowing usage of global settings via asterisk
 (*) in RequestHandler->$allowed_actions

git-svn-id: svn://svn.silverstripe.com/silverstripe/open/modules/sapphire/trunk@74009 467b73ca-7a2a-4603-9d3b-597d59a354a9
---
 core/control/RequestHandler.php | 31 +++++++++++++++++++------------
 tests/ControllerTest.php        | 31 +++++++++++++++++++++++++++++--
 2 files changed, 48 insertions(+), 14 deletions(-)

diff --git a/core/control/RequestHandler.php b/core/control/RequestHandler.php
index f4e25e4ab..4996223bd 100644
--- a/core/control/RequestHandler.php
+++ b/core/control/RequestHandler.php
@@ -166,24 +166,31 @@ class RequestHandler extends ViewableData {
 		if($action == 'index') return true;
 		
 		if($allowedActions)  {
+			// convert all keys and values to lowercase for easier comparison (only if not set as boolean)
 			foreach($allowedActions as $key => $value) {
-				$newAllowedActions[strtolower($key)] = strtolower($value);
+				$newAllowedActions[strtolower($key)] = (is_bool($value)) ? $value : strtolower($value);
 			}
-			
 			$allowedActions = $newAllowedActions;
 			
-			if(isset($allowedActions[$action])) {
-				$test = $allowedActions[$action];
-				
-				if($test === true) {
-					return true;
-				} elseif(substr($test, 0, 2) == '->') {
-					return $this->{substr($test, 2)}();
-				} elseif(Permission::check($test)) {
+			// check for specific action rules first, and fall back to global rules defined by asterisk
+			foreach(array($action,'*') as $actionOrAll) {
+				// check if specific action is set
+				if(isset($allowedActions[$actionOrAll])) {
+					$test = $allowedActions[$actionOrAll];
+					if($test === true) {
+						// Case 1: TRUE should always allow access
+						return true;
+					} elseif(substr($test, 0, 2) == '->') {
+						// Case 2: Determined by custom method with "->" prefix
+						return $this->{substr($test, 2)}();
+					} elseif(Permission::check($test)) {
+						// Case 3: Value is a permission code to check the current member against
+						return true;
+					}
+				} elseif((($key = array_search($actionOrAll, $allowedActions)) !== false) && is_numeric($key)) {
+					// Case 4: Allow numeric array notation (search for array value as action instead of key)
 					return true;
 				}
-			} elseif((($key = array_search($action, $allowedActions)) !== false) && is_numeric($key)) {
-				return true;
 			}
 		}
 		
diff --git a/tests/ControllerTest.php b/tests/ControllerTest.php
index 10f6e644e..ecb4b57d4 100644
--- a/tests/ControllerTest.php
+++ b/tests/ControllerTest.php
@@ -36,9 +36,20 @@ class ControllerTest extends SapphireTest {
 		$response = Director::test("ControllerTest_SecuredController/adminonly");
 		$this->assertEquals(403, $response->getStatusCode());
 		
-		// test that a controller without a specified allowed_actions allows actions through
 		$response = Director::test('ControllerTest_UnsecuredController/stringaction');
-		$this->assertEquals(200, $response->getStatusCode());
+		$this->assertEquals(200, $response->getStatusCode(), 
+			"test that a controller without a specified allowed_actions allows actions through"
+		);
+		
+		$response = Director::test("ControllerTest_FullSecuredController/adminonly");
+		$this->assertEquals(403, $response->getStatusCode(),
+			"Actions can be globally disallowed by using asterisk (*) instead of a method name"
+		);
+		
+		$response = Director::test("ControllerTest_FullSecuredController/unsecuredaction");
+		$this->assertEquals(200, $response->getStatusCode(),
+			"Actions can be overridden to be allowed if globally disallowed by using asterisk (*)"
+		);
 	}
 	
 	/**
@@ -111,4 +122,20 @@ class ControllerTest_SecuredController extends Controller {
 	}
 }
 
+class ControllerTest_FullSecuredController extends Controller {
+	
+	static $allowed_actions = array(
+		"*" => "ADMIN",
+		'unsecuredaction' => true,
+	);
+	
+	function adminonly() {
+		return "You must be an admin!";
+	}
+	
+	function unsecuredaction() {
+		return "Allowed for everybody";
+	}
+}
+
 class ControllerTest_UnsecuredController extends ControllerTest_SecuredController {}
\ No newline at end of file