Permalink
Browse files

fix(actions): Misspelled action levels no longer treated as logged_in.

Instead we now log an error and set the level to “admin”

Fixes #8337
  • Loading branch information...
mrclay committed May 21, 2015
1 parent 8b0779a commit d936549a7cfb2afad5acf8a0dc407602bfd48a1f
Showing with 66 additions and 18 deletions.
  1. +66 −18 engine/classes/Elgg/ActionsService.php
@@ -16,6 +16,11 @@ class ActionsService {
/**
* Registered actions storage
+ *
+ * Each element has keys:
+ * "file" => filename
+ * "access" => access level
+ *
* @var array
*/
private $actions = array();
@@ -25,6 +30,11 @@ class ActionsService {
* @var string
*/
private $currentAction = null;
+
+ /**
+ * @var string[]
+ */
+ private static $access_levels = ['public', 'logged_in', 'admin'];
/**
* @see action
@@ -47,7 +57,7 @@ public function execute($action, $forwarder = "") {
if (!in_array($action, $exceptions)) {
// All actions require a token.
- action_gatekeeper($action);
+ $this->gatekeeper($action);
}
$forwarder = str_replace(_elgg_services()->config->getSiteUrl(), "", $forwarder);
@@ -56,26 +66,59 @@ public function execute($action, $forwarder = "") {
if (substr($forwarder, 0, 1) == "/") {
$forwarder = substr($forwarder, 1);
}
-
+
+ /**
+ * Complete the execution with a forward
+ *
+ * @param string $error_key Error message key
+ *
+ * @throws \SecurityException
+ */
+ $forward = function ($error_key = '') use ($action, $forwarder) {
+ if ($error_key) {
+ $msg = _elgg_services()->translator->translate($error_key, [$action]);
+ _elgg_services()->systemMessages->addErrorMessage($msg);
+ }
+
+ $forwarder = empty($forwarder) ? REFERER : $forwarder;
+ forward($forwarder);
+ };
+
if (!isset($this->actions[$action])) {
- register_error(_elgg_services()->translator->translate('actionundefined', array($action)));
- } elseif (!_elgg_services()->session->isAdminLoggedIn() && ($this->actions[$action]['access'] === 'admin')) {
- register_error(_elgg_services()->translator->translate('actionunauthorized'));
- } elseif (!_elgg_services()->session->isLoggedIn() && ($this->actions[$action]['access'] !== 'public')) {
- register_error(_elgg_services()->translator->translate('actionloggedout'));
- } else {
- // To quietly cancel the action file, return a falsey value in the "action" hook.
- if (_elgg_services()->hooks->trigger('action', $action, null, true)) {
- if (is_file($this->actions[$action]['file']) && is_readable($this->actions[$action]['file'])) {
- self::includeFile($this->actions[$action]['file']);
- } else {
- register_error(_elgg_services()->translator->translate('actionnotfound', array($action)));
+ $forward('actionundefined');
+ }
+
+ $user = _elgg_services()->session->getLoggedInUser();
+
+ // access checks
+ switch ($this->actions[$action]['access']) {
+ case 'public':
+ break;
+ case 'logged_in':
+ if (!$user) {
+ $forward('actionloggedout');
+ }
+ break;
+ default:
+ // admin or misspelling
+ if (!$user->isAdmin()) {
+ $forward('actionunauthorized');
}
- }
}
-
- $forwarder = empty($forwarder) ? REFERER : $forwarder;
- forward($forwarder);
+
+ // To quietly cancel the file, return a falsey value in the "action" hook.
+ if (!_elgg_services()->hooks->trigger('action', $action, null, true)) {
+ $forward();
+ }
+
+ $file = $this->actions[$action]['file'];
+
+ if (!is_file($file) || !is_readable($file)) {
+ $forward('actionnotfound');
+ }
+
+ self::includeFile($file);
+ $forward();
}
/**
@@ -106,6 +149,11 @@ public function register($action, $filename = "", $access = 'logged_in') {
$filename = $path . "actions/" . $action . ".php";
}
+
+ if (!in_array($access, self::$access_levels)) {
+ _elgg_services()->logger->error("Unrecognized value '$access' for \$access in " . __METHOD__);
+ $access = 'admin';
+ }
$this->actions[$action] = array(
'file' => $filename,

0 comments on commit d936549

Please sign in to comment.