Skip to content
Snippets Groups Projects
Commit 480336a3 authored by Andreas Gohr's avatar Andreas Gohr
Browse files

no longer rely on actionOk when checking if actions are disabled

loadAction() and checkAction() are now public and could be used within
actionOK(). However some weird circular references prevent that. In
addition, actionOK is also used to check for things that aren't Actions
(yet) like 'rss' and 'top'.
parent ec701221
No related branches found
No related tags found
No related merge requests found
......@@ -87,6 +87,9 @@ class action_general extends DokuWikiTest {
* @param $name
*/
public function testBaseClassActionOkPermission($name) {
$this->assertTrue(true); // mark as not risky
if($name == 'Show') return; // disabling show does not work
$classname = 'dokuwiki\\Action\\' . $name;
/** @var \dokuwiki\Action\AbstractAction $class */
$class = new $classname();
......@@ -95,9 +98,10 @@ class action_general extends DokuWikiTest {
$conf['useacl'] = 1;
$conf['subscribers'] = 1;
$conf['disableactions'] = '';
$_SERVER['REMOTE_USER'] = 'someone';
try {
$class->checkPermissions();
\dokuwiki\ActionRouter::getInstance(true)->checkAction($class);
} catch(\Exception $e) {
$this->assertNotSame(ActionDisabledException::class, get_class($e));
}
......@@ -105,12 +109,10 @@ class action_general extends DokuWikiTest {
$conf['disableactions'] = $class->getActionName();
try {
$class->checkPermissions();
\dokuwiki\ActionRouter::getInstance(true)->checkAction($class);
} catch(\Exception $e) {
$this->assertSame(ActionDisabledException::class, get_class($e));
$this->assertSame(ActionDisabledException::class, get_class($e), $e);
}
$this->assertTrue(true); // mark as not risky
}
/**
......
......@@ -17,7 +17,9 @@ abstract class AbstractAclAction extends AbstractAction {
public function checkPermissions() {
parent::checkPermissions();
global $conf;
global $auth;
if(!$conf['useacl']) throw new ActionAclRequiredException();
if(!$auth) throw new ActionAclRequiredException();
}
}
......@@ -23,7 +23,7 @@ abstract class AbstractAction {
*
* @param string $actionname the name of this action (see getActioName() for caveats)
*/
public function __construct($actionname='') {
public function __construct($actionname = '') {
if($actionname !== '') {
$this->actionname = $actionname;
} else {
......@@ -50,7 +50,6 @@ abstract class AbstractAction {
* @return void
*/
public function checkPermissions() {
if(!actionOK($this->actionname)) throw new ActionDisabledException();
}
/**
......
......@@ -26,7 +26,6 @@ class Login extends AbstractAclAction {
// nothing to do
throw new ActionException();
}
// FIXME auth login capabilities
}
/** @inheritdoc */
......
......@@ -2,6 +2,7 @@
namespace dokuwiki\Action;
use dokuwiki\Action\Exception\ActionDisabledException;
use dokuwiki\Action\Exception\ActionException;
/**
......@@ -18,6 +19,15 @@ class Logout extends AbstractUserAction {
return AUTH_NONE;
}
/** @inheritdoc */
public function checkPermissions() {
parent::checkPermissions();
/** @var \DokuWiki_Auth_Plugin $auth */
global $auth;
if(!$auth->canDo('logout')) throw new ActionDisabledException();
}
/** @inheritdoc */
public function preProcess() {
global $ID;
......
......@@ -3,6 +3,7 @@
namespace dokuwiki\Action;
use dokuwiki\Action\Exception\ActionAbort;
use dokuwiki\Action\Exception\ActionDisabledException;
/**
* Class Profile
......@@ -18,6 +19,15 @@ class Profile extends AbstractUserAction {
return AUTH_NONE;
}
/** @inheritdoc */
public function checkPermissions() {
parent::checkPermissions();
/** @var \DokuWiki_Auth_Plugin $auth */
global $auth;
if(!$auth->canDo('Profile')) throw new ActionDisabledException();
}
/** @inheritdoc */
public function preProcess() {
global $lang;
......
......@@ -3,6 +3,7 @@
namespace dokuwiki\Action;
use dokuwiki\Action\Exception\ActionAbort;
use dokuwiki\Action\Exception\ActionDisabledException;
/**
* Class ProfileDelete
......@@ -18,6 +19,15 @@ class ProfileDelete extends AbstractUserAction {
return AUTH_NONE;
}
/** @inheritdoc */
public function checkPermissions() {
parent::checkPermissions();
/** @var \DokuWiki_Auth_Plugin $auth */
global $auth;
if(!$auth->canDo('delUser')) throw new ActionDisabledException();
}
/** @inheritdoc */
public function preProcess() {
global $lang;
......
......@@ -3,6 +3,7 @@
namespace dokuwiki\Action;
use dokuwiki\Action\Exception\ActionAbort;
use dokuwiki\Action\Exception\ActionDisabledException;
/**
* Class Register
......@@ -11,13 +12,24 @@ use dokuwiki\Action\Exception\ActionAbort;
*
* @package dokuwiki\Action
*/
class Register extends AbstractAction {
class Register extends AbstractAclAction {
/** @inheritdoc */
public function minimumPermission() {
return AUTH_NONE;
}
/** @inheritdoc */
public function checkPermissions() {
parent::checkPermissions();
/** @var \DokuWiki_Auth_Plugin $auth */
global $auth;
global $conf;
if(isset($conf['openregister']) && !$conf['openregister']) throw new ActionDisabledException();
if(!$auth->canDo('addUser')) throw new ActionDisabledException();
}
/** @inheritdoc */
public function preProcess() {
if(register()) { // FIXME could be moved from auth to here
......
......@@ -3,6 +3,7 @@
namespace dokuwiki\Action;
use dokuwiki\Action\Exception\ActionAbort;
use dokuwiki\Action\Exception\ActionDisabledException;
/**
* Class Resendpwd
......@@ -18,6 +19,17 @@ class Resendpwd extends AbstractAclAction {
return AUTH_NONE;
}
/** @inheritdoc */
public function checkPermissions() {
parent::checkPermissions();
/** @var \DokuWiki_Auth_Plugin $auth */
global $auth;
global $conf;
if(isset($conf['resendpasswd']) && !$conf['resendpasswd']) throw new ActionDisabledException(); //legacy option
if(!$auth->canDo('modPass')) throw new ActionDisabledException();
}
/** @inheritdoc */
public function preProcess() {
if($this->resendpwd()) {
......
......@@ -2,6 +2,8 @@
namespace dokuwiki\Action;
use dokuwiki\Action\Exception\ActionDisabledException;
/**
* Class Subscribe
*
......@@ -16,6 +18,14 @@ class Subscribe extends AbstractUserAction {
return AUTH_READ;
}
/** @inheritdoc */
public function checkPermissions() {
parent::checkPermissions();
global $conf;
if(isset($conf['subscribers']) && !$conf['subscribers']) throw new ActionDisabledException();
}
/** @inheritdoc */
public function preProcess() {
$act = $this->actionname;
......
......@@ -27,6 +27,9 @@ class ActionRouter {
/** maximum loop */
const MAX_TRANSITIONS = 5;
/** @var string[] the actions disabled in the configuration */
protected $disabled;
/**
* ActionRouter constructor. Singleton, thus protected!
*
......@@ -35,6 +38,12 @@ class ActionRouter {
*/
protected function __construct() {
global $ACT;
global $conf;
$this->disabled = explode(',', $conf['disableactions']);
$this->disabled = array_map('trim', $this->disabled);
$this->transitions = 0;
$ACT = act_clean($ACT);
$this->setupAction($ACT);
$ACT = $this->action->getActionName();
......@@ -67,8 +76,7 @@ class ActionRouter {
try {
$this->action = $this->loadAction($actionname);
$this->action->checkPermissions();
$this->ensureMinimumPermission($this->action->minimumPermission());
$this->checkAction($this->action);
$this->action->preProcess();
} catch(ActionException $e) {
......@@ -139,19 +147,6 @@ class ActionRouter {
$this->setupAction($to);
}
/**
* Check that the given minimum permissions are reached
*
* @param int $permneed
* @throws ActionException
*/
protected function ensureMinimumPermission($permneed) {
global $INFO;
if($INFO['perm'] < $permneed) {
throw new ActionException('denied');
}
}
/**
* Aborts all processing with a message
*
......@@ -183,7 +178,7 @@ class ActionRouter {
* @return AbstractAction
* @throws NoActionException
*/
protected function loadAction($actionname) {
public function loadAction($actionname) {
$actionname = strtolower($actionname); // FIXME is this needed here? should we run a cleanup somewhere else?
$parts = explode('_', $actionname);
while($parts) {
......@@ -198,6 +193,34 @@ class ActionRouter {
throw new NoActionException();
}
/**
* Execute all the checks to see if this action can be executed
*
* @param AbstractAction $action
* @throws ActionDisabledException
* @throws ActionException
*/
public function checkAction(AbstractAction $action) {
global $INFO;
global $ID;
if(in_array($action->getActionName(), $this->disabled)) {
throw new ActionDisabledException();
}
$action->checkPermissions();
if(isset($INFO)) {
$perm = $INFO['perm'];
} else {
$perm = auth_quickaclcheck($ID);
}
if($perm < $action->minimumPermission()) {
throw new ActionException('denied');
}
}
/**
* Returns the action handling the current request
*
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment