tonyair/silverstripe-framework
1
0
Fork 0
mirror of https://github.com/silverstripe/silverstripe-framework synced 2024-06-25 05:59:23 +02:00
Code Issues Projects Releases Wiki Activity

Merge pull request #2121 from sminnee/allowed-actions-templates

Browse Source 
FIX: Ensure that actions inferred from templates with the "_action" suff...
This commit is contained in:
Ingo Schommer 2013-06-19 02:16:37 -07:00
commit 5c3809a165
4 changed files with 97 additions and 54 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
control/Controller.php
View File

@ -265,10 +265,12 @@ class Controller extends RequestHandler implements TemplateGlobalProvider {
*/
public function getViewer($action) {
// Hard-coded templates
if($this->templates[$action]) {
if(isset($this->templates[$action]) && $this->templates[$action]) {
$templates = $this->templates[$action];
} else if($this->templates['index']) {
} else if(isset($this->templates['index']) && $this->templates['index']) {
$templates = $this->templates['index'];
} else if($this->template) {
$templates = $this->template;
} else {
@ -318,6 +320,23 @@ class Controller extends RequestHandler implements TemplateGlobalProvider {
return $returnURL;
}
/**
* Return the class that defines the given action, so that we know where to check allowed_actions.
* Overrides RequestHandler to also look at defined templates
*/
protected function definingClassForAction($action) {
$definingClass = parent::definingClassForAction($action);
if($definingClass) return $definingClass;
$class = get_class($this);
while($class != 'RequestHandler') {
$templateName = strtok($class, '_') . '_' . $action;
if(SSViewer::hasTemplate($templateName)) return $class;
$class = get_parent_class($class);
}
}
/**
* Returns TRUE if this controller has a template that is specifically designed to handle a specific action.

99
control/RequestHandler.php
View File

@ -371,6 +371,22 @@ class RequestHandler extends ViewableData {
return false;
}
/**
* Return the class that defines the given action, so that we know where to check allowed_actions.
*/
protected function definingClassForAction($actionOrigCasing) {
$action = strtolower($actionOrigCasing);
$definingClass = null;
$insts = array_merge(array($this), (array)$this->getExtensionInstances());
foreach($insts as $inst) {
if(!method_exists($inst, $action)) continue;
$r = new ReflectionClass(get_class($inst));
$m = $r->getMethod($actionOrigCasing);
return $m->getDeclaringClass()->getName();
}
}
/**
* Check that the given action is allowed to be called from a URL.
@ -382,58 +398,45 @@ class RequestHandler extends ViewableData {
$isAllowed = false;
$isDefined = false;
if($this->hasMethod($actionOrigCasing) || !$action || $action == 'index') {
// Get actions for this specific class (without inheritance)
$definingClass = null;
$insts = array_merge(array($this), (array)$this->getExtensionInstances());
foreach($insts as $inst) {
if(!method_exists($inst, $action)) continue;
$r = new ReflectionClass(get_class($inst));
$m = $r->getMethod($actionOrigCasing);
$definingClass = $m->getDeclaringClass()->getName();
}
// Get actions for this specific class (without inheritance)
$definingClass = $this->definingClassForAction($actionOrigCasing);
$allowedActions = $this->allowedActions($definingClass);
$allowedActions = $this->allowedActions($definingClass);
// check if specific action is set
if(isset($allowedActions[$action])) {
$isDefined = true;
$test = $allowedActions[$action];
if($test === true || $test === 1 || $test === '1') {
// TRUE should always allow access
$isAllowed = true;
} elseif(substr($test, 0, 2) == '->') {
// Determined by custom method with "->" prefix
list($method, $arguments) = Object::parse_class_spec(substr($test, 2));
$definingClassInst = Injector::inst()->get($definingClass);
$isAllowed = call_user_func_array(array($definingClassInst, $method), $arguments);
} else {
// Value is a permission code to check the current member against
$isAllowed = Permission::check($test);
}
} elseif(
is_array($allowedActions)
&& (($key = array_search($action, $allowedActions, true)) !== false)
&& is_numeric($key)
) {
// Allow numeric array notation (search for array value as action instead of key)
$isDefined = true;
$isAllowed = true;
} elseif(is_array($allowedActions) && !count($allowedActions)) {
// If defined as empty array, deny action
$isAllowed = false;
} elseif($allowedActions === null) {
// If undefined, allow action
// check if specific action is set
if(isset($allowedActions[$action])) {
$isDefined = true;
$test = $allowedActions[$action];
if($test === true || $test === 1 || $test === '1') {
// TRUE should always allow access
$isAllowed = true;
} elseif(substr($test, 0, 2) == '->') {
// Determined by custom method with "->" prefix
list($method, $arguments) = Object::parse_class_spec(substr($test, 2));
$isAllowed = call_user_func_array(array($this, $method), $arguments);
} else {
// Value is a permission code to check the current member against
$isAllowed = Permission::check($test);
}
} elseif(
is_array($allowedActions)
&& (($key = array_search($action, $allowedActions, true)) !== false)
&& is_numeric($key)
) {
// Allow numeric array notation (search for array value as action instead of key)
$isDefined = true;
$isAllowed = true;
} elseif(is_array($allowedActions) && !count($allowedActions)) {
// If defined as empty array, deny action
$isAllowed = false;
} elseif($allowedActions === null) {
// If undefined, allow action
$isAllowed = true;
}
// If we don't have a match in allowed_actions,
// whitelist the 'index' action as well as undefined actions.
if(!$isDefined && ($action == 'index' || empty($action))) {
$isAllowed = true;
}
} else {
// Doesn't have method, set to true so that a template can handle this action
// If we don't have a match in allowed_actions,
// whitelist the 'index' action as well as undefined actions.
if(!$isDefined && ($action == 'index' || empty($action))) {
$isAllowed = true;
}

21
tests/control/ControllerTest.php
View File

@ -100,10 +100,24 @@ class ControllerTest extends FunctionalTest {
$response = $this->get("ControllerTest_AccessSecuredController/method2");
$this->assertEquals(200, $response->getStatusCode(),
'Access grante on action originally defined with empty $allowed_actions on parent controller, ' .
'Access granted on action originally defined with empty $allowed_actions on parent controller, ' .
'because it has been redefined in the subclass'
);
$response = $this->get("ControllerTest_AccessSecuredController/templateaction");
$this->assertEquals(403, $response->getStatusCode(),
'Access denied on action with $allowed_actions on defining controller, ' .
'if action is not a method but rather a template discovered by naming convention'
);
$this->session()->inst_set('loggedInAs', $adminUser->ID);
$response = $this->get("ControllerTest_AccessSecuredController/templateaction");
$this->assertEquals(200, $response->getStatusCode(),
'Access granted for logged in admin on action with $allowed_actions on defining controller, ' .
'if action is not a method but rather a template discovered by naming convention'
);
$this->session()->inst_set('loggedInAs', null);
$response = $this->get("ControllerTest_AccessSecuredController/adminonly");
$this->assertEquals(403, $response->getStatusCode(),
'Access denied on action with $allowed_actions on defining controller, ' .
@ -349,8 +363,9 @@ class ControllerTest_Controller extends Controller implements TestOnly {
'methodaction',
'stringaction',
'redirectbacktest',
'templateaction'
);
public function methodaction() {
return array(
"Content" => "methodaction content"
@ -395,7 +410,7 @@ class ControllerTest_AccessSecuredController extends ControllerTest_AccessBaseCo
"method1", // denied because only defined in parent
"method2" => true, // granted because its redefined
"adminonly" => "ADMIN",
"protectedmethod" => true, // denied because its protected
'templateaction' => 'ADMIN'
);
public function method2() {}

8
view/SSViewer.php
View File

@ -717,8 +717,14 @@ class SSViewer {
public static function hasTemplate($templates) {
$manifest = SS_TemplateLoader::instance()->getManifest();
if(Config::inst()->get('SSViewer', 'theme_enabled')) {
$theme = Config::inst()->get('SSViewer', 'theme');
} else {
$theme = null;
}
foreach ((array) $templates as $template) {
if ($manifest->getTemplate($template)) return true;
if ($manifest->getCandidateTemplate($template, $theme)) return true;
}
return false;