From c1f241be16aa88a278d0e51fc7baa78e39008cca Mon Sep 17 00:00:00 2001 From: Michael M Slusarz Date: Fri, 30 Jul 2010 14:29:55 -0600 Subject: [PATCH] Fix/cleanup app permissions checking --- framework/Block/lib/Horde/Block/Layout/Manager.php | 7 ++--- framework/Core/lib/Horde.php | 27 ------------------- framework/Perms/lib/Horde/Perms.php | 12 ++++++--- horde/lib/Application.php | 20 ++++++++++++++ horde/templates/portal/edit.inc | 5 ++-- imp/lib/Application.php | 31 +++++++++++----------- ingo/lib/Application.php | 26 +++++++++--------- kronolith/lib/Application.php | 18 ++++++------- mnemo/lib/Application.php | 22 ++++++++++++++- mnemo/lib/Mnemo.php | 12 ++++----- mnemo/memo.php | 26 +++++++++--------- nag/lib/Application.php | 18 ++++++------- 12 files changed, 122 insertions(+), 102 deletions(-) diff --git a/framework/Block/lib/Horde/Block/Layout/Manager.php b/framework/Block/lib/Horde/Block/Layout/Manager.php index d42bc9ff0..743aae3fe 100644 --- a/framework/Block/lib/Horde/Block/Layout/Manager.php +++ b/framework/Block/lib/Horde/Block/Layout/Manager.php @@ -235,12 +235,13 @@ class Horde_Block_Layout_Manager extends Horde_Block_Layout !$this->rowExists($row) || !$this->colExists($col)) { // Check permissions. - if (Horde::hasPermission('max_blocks') !== true && - Horde::hasPermission('max_blocks') <= $this->count()) { + $max_blocks = $GLOBALS['injector']->getInstance('Horde_Perms')->hasAppPermission('max_blocks'); + if (($max_blocks !== true) && + ($max_blocks <= $this->count())) { try { $message = Horde::callHook('perms_denied', array('horde:max_blocks')); } catch (Horde_Exception_HookNotSet $e) { - $message = @htmlspecialchars(sprintf(ngettext("You are not allowed to create more than %d block.", "You are not allowed to create more than %d blocks.", Horde::hasPermission('max_blocks')), Horde::hasPermission('max_blocks')), ENT_COMPAT, $GLOBALS['registry']->getCharset()); + $message = @htmlspecialchars(sprintf(ngettext("You are not allowed to create more than %d block.", "You are not allowed to create more than %d blocks.", $max_blocks), $max_blocks), ENT_COMPAT, $GLOBALS['registry']->getCharset()); } $GLOBALS['notification']->push($message, 'horde.error', array('content.raw')); break; diff --git a/framework/Core/lib/Horde.php b/framework/Core/lib/Horde.php index 19e71b431..f67df8c6e 100644 --- a/framework/Core/lib/Horde.php +++ b/framework/Core/lib/Horde.php @@ -1737,33 +1737,6 @@ HTML; } /** - * Returns the specified permission for the current user. - * - * @param string $permission A permission, currently only 'max_blocks'. - * - * @return mixed The value of the specified permission. - */ - static public function hasPermission($permission) - { - $perms = $GLOBALS['injector']->getInstance('Horde_Perms'); - - if (!$perms->exists('horde:' . $permission)) { - return true; - } - - $allowed = $perms->getPermissions('horde:' . $permission); - if (is_array($allowed)) { - switch ($permission) { - case 'max_blocks': - $allowed = max($allowed); - break; - } - } - - return $allowed; - } - - /** * Utility function to send redirect headers to browser, handling any * browser quirks. * diff --git a/framework/Perms/lib/Horde/Perms.php b/framework/Perms/lib/Horde/Perms.php index 427c528f9..886447332 100644 --- a/framework/Perms/lib/Horde/Perms.php +++ b/framework/Perms/lib/Horde/Perms.php @@ -450,11 +450,17 @@ class Horde_Perms : $GLOBALS['registry']->getApp(); if ($this->exists($app . ':' . $permission)) { - $args = array($this->getPermissions($app . ':' . $permission)); - if (isset($opts['opts'])) { - $args[] = $opts['opts']; + $perms = $this->getPermissions($app . ':' . $permission); + if ($perms === false) { + return false; } + $args = array( + $permission, + $perms, + isset($opts['opts']) ? $opts['opts'] : array() + ); + try { return $GLOBALS['registry']->callAppMethod($app, 'hasPermission', array('args' => $args)); } catch (Horde_Exception $e) {} diff --git a/horde/lib/Application.php b/horde/lib/Application.php index 2f9218f91..912bdd545 100644 --- a/horde/lib/Application.php +++ b/horde/lib/Application.php @@ -43,6 +43,26 @@ class Horde_Application extends Horde_Registry_Application } /** + * Returns the specified permission for the given app permission. + * + * @param string $permission The permission to check. + * @param mixed $allowed The allowed permissions. + * @param array $opts Additional options (NONE). + * + * @return mixed The value of the specified permission. + */ + public function hasPermission($permission, $allowed, $opts = array()) + { + switch ($permission) { + case 'max_blocks': + $allowed = max($allowed); + break; + } + + return $allowed; + } + + /** * Populate dynamically-generated preference values. * * @param Horde_Core_Prefs_Ui $ui The UI object. diff --git a/horde/templates/portal/edit.inc b/horde/templates/portal/edit.inc index 4cf52658b..811f8c64e 100644 --- a/horde/templates/portal/edit.inc +++ b/horde/templates/portal/edit.inc @@ -1,10 +1,11 @@ rows(); +$max_blocks = $GLOBALS['injector']->getInstance('Horde_Perms')->hasAppPermission('max_blocks'); list($current_row, $current_col) = $layout->getCurrentBlock(); $allow_add = !empty($conf['hooks']['permsdenied']) || - (Horde::hasPermission('max_blocks') === true) || - (Horde::hasPermission('max_blocks') > $layout->count()); + ($max_blocks === true) || + ($max_blocks > $layout->count()); $columns = 0; for ($row = 0; $row < $rows; ++$row): ?> diff --git a/imp/lib/Application.php b/imp/lib/Application.php index abaef477a..7c20488b3 100644 --- a/imp/lib/Application.php +++ b/imp/lib/Application.php @@ -178,27 +178,26 @@ class IMP_Application extends Horde_Registry_Application } /** - * Returns the specified permission for the current user. + * Returns the specified permission for the given app permission. * - * @param mixed $allowed The allowed permissions. - * @param array $opts Additinal options ('value'). + * @param string $permission The permission to check. + * @param mixed $allowed The allowed permissions. + * @param array $opts Additional options ('value'). * * @return mixed The value of the specified permission. */ - public function hasPermission($allowed, $opts = array()) + public function hasPermission($permission, $allowed, $opts = array()) { - if (is_array($allowed)) { - switch ($permission) { - case 'create_folders': - $allowed = (bool)count(array_filter($allowed)); - break; - - case 'max_folders': - case 'max_recipients': - case 'max_timelimit': - $allowed = max($allowed); - break; - } + switch ($permission) { + case 'create_folders': + $allowed = (bool)count(array_filter($allowed)); + break; + + case 'max_folders': + case 'max_recipients': + case 'max_timelimit': + $allowed = max($allowed); + break; } return (($permission == 'max_folders') && empty($opts['value'])) diff --git a/ingo/lib/Application.php b/ingo/lib/Application.php index c57316249..94042fc80 100644 --- a/ingo/lib/Application.php +++ b/ingo/lib/Application.php @@ -119,24 +119,24 @@ class Ingo_Application extends Horde_Registry_Application } /** - * Returns the specified permission for the current user. + * Returns the specified permission for the given app permission. * - * @param mixed $allowed The allowed permissions. + * @param string $permission The permission to check. + * @param mixed $allowed The allowed permissions. + * @param array $opts Additional options (NONE). * * @return mixed The value of the specified permission. */ - public function hasPermission($allowed) + public function hasPermission($permission, $allowed, $opts = array()) { - if (is_array($allowed)) { - switch ($permission) { - case 'allow_rules': - $allowed = (bool)count(array_filter($allowed)); - break; - - case 'max_rules': - $allowed = max($allowed); - break; - } + switch ($permission) { + case 'allow_rules': + $allowed = (bool)count(array_filter($allowed)); + break; + + case 'max_rules': + $allowed = max($allowed); + break; } return $allowed; diff --git a/kronolith/lib/Application.php b/kronolith/lib/Application.php index fce6980a8..9963db8cb 100644 --- a/kronolith/lib/Application.php +++ b/kronolith/lib/Application.php @@ -89,20 +89,20 @@ class Kronolith_Application extends Horde_Registry_Application } /** - * Returns the specified permission for the current user. + * Returns the specified permission for the given app permission. * - * @param mixed $allowed The allowed permissions. + * @param string $permission The permission to check. + * @param mixed $allowed The allowed permissions. + * @param array $opts Additional options (NONE). * * @return mixed The value of the specified permission. */ - public function hasPermission($allowed) + public function hasPermission($permission, $allowed, $opts = array()) { - if (is_array($allowed)) { - switch ($permission) { - case 'max_events': - $allowed = max($allowed); - break; - } + switch ($permission) { + case 'max_events': + $allowed = max($allowed); + break; } return $allowed; diff --git a/mnemo/lib/Application.php b/mnemo/lib/Application.php index a7a00fef5..a45754cb7 100644 --- a/mnemo/lib/Application.php +++ b/mnemo/lib/Application.php @@ -64,7 +64,27 @@ class Mnemo_Application extends Horde_Registry_Application $perms['title']['mnemo:max_notes'] = _("Maximum Number of Notes"); $perms['type']['mnemo:max_notes'] = 'int'; - return $perms; + return $perms; + } + + /** + * Returns the specified permission for the given app permission. + * + * @param string $permission The permission to check. + * @param mixed $allowed The allowed permissions. + * @param array $opts Additional options (NONE). + * + * @return mixed The value of the specified permission. + */ + public function hasPermission($permission, $allowed, $opts = array()) + { + switch ($permission) { + case 'max_notes': + $allowed = max($allowed); + break; + } + + return $allowed; } /** diff --git a/mnemo/lib/Mnemo.php b/mnemo/lib/Mnemo.php index 0ce11c92d..9900824f1 100644 --- a/mnemo/lib/Mnemo.php +++ b/mnemo/lib/Mnemo.php @@ -22,32 +22,32 @@ class Mnemo { * Sort by memo description. */ const SORT_DESC = 0; - + /** * Sort by memo category. */ const SORT_CATEGORY = 1; - + /** * Sort by notepad. */ const SORT_NOTEPAD = 2; - + /** * Sort in ascending order. */ const SORT_ASCEND = 0; - + /** * Sort in descending order. */ const SORT_DESCEND = 1; - + /** * No passphrase provided. */ const ERR_NO_PASSPHRASE = 100; - + /** * Decrypting failed */ diff --git a/mnemo/memo.php b/mnemo/memo.php index 767f986e8..df414632a 100644 --- a/mnemo/memo.php +++ b/mnemo/memo.php @@ -71,9 +71,9 @@ $cManager = new Horde_Prefs_CategoryManager(); switch ($actionID) { case 'add_memo': /* Check permissions. */ - if ($GLOBALS['injector']->getInstance('Horde_Perms')->hasAppPermission('max_notes') !== true && - $GLOBALS['injector']->getInstance('Horde_Perms')->hasAppPermission('max_notes') <= Mnemo::countMemos()) { - $message = @htmlspecialchars(sprintf(_("You are not allowed to create more than %d notes."), Mnemo::hasPermission('max_notes')), ENT_COMPAT, $GLOBALS['registry']->getCharset()); + if ($injector->getInstance('Horde_Perms')->hasAppPermission('max_notes') !== true && + $injector->getInstance('Horde_Perms')->hasAppPermission('max_notes') <= Mnemo::countMemos()) { + $message = @htmlspecialchars(sprintf(_("You are not allowed to create more than %d notes."), $injector->getInstance('Horde_Perms')->hasAppPermission('max_notes')), ENT_COMPAT, $registry->getCharset()); if (!empty($conf['hooks']['permsdenied'])) { $message = Horde::callHook('_perms_hook_denied', array('mnemo:max_notes'), 'horde', $message); } @@ -130,12 +130,12 @@ case 'save_memo': $memo_passphrase2 = Horde_Util::getFormData('memo_passphrase2'); try { - $share = $GLOBALS['mnemo_shares']->getShare($notepad_target); + $share = $mnemo_shares->getShare($notepad_target); } catch (Horde_Share_Exception $e) { throw new Mnemo_Exception($e); } - if (!$share->hasPermission($GLOBALS['registry']->getAuth(), Horde_Perms::EDIT)) { + if (!$share->hasPermission($registry->getAuth(), Horde_Perms::EDIT)) { $notification->push(sprintf(_("Access denied saving note to %s."), $share->get('name')), 'horde.error'); } elseif ($memo_passphrase != $memo_passphrase2) { $notification->push(_("The passwords don't match."), 'horde.error'); @@ -173,17 +173,17 @@ case 'save_memo': if ($memolist_original != $notepad_target) { /* Moving the note to another notepad. */ try { - $share = $GLOBALS['mnemo_shares']->getShare($memolist_original); + $share = $mnemo_shares->getShare($memolist_original); } catch (Horde_Share_Exception $e) { throw new Mnemo_Exception($e); } - if ($share->hasPermission($GLOBALS['registry']->getAuth(), Horde_Perms::DELETE)) { + if ($share->hasPermission($registry->getAuth(), Horde_Perms::DELETE)) { try { - $share = &$GLOBALS['mnemo_shares']->getShare($notepad_target); + $share = &$mnemo_shares->getShare($notepad_target); } catch (Horde_Share_Exception $e) { throw new Mnemo_Exception($e); } - if ($share->hasPermission($GLOBALS['registry']->getAuth(), Horde_Perms::EDIT)) { + if ($share->hasPermission($registry->getAuth(), Horde_Perms::EDIT)) { $result = $storage->move($memo_id, $notepad_target); $storage = &Mnemo_Driver::singleton($notepad_target); } else { @@ -201,8 +201,8 @@ case 'save_memo': $result = $storage->modify($memo_id, $memo_desc, $memo_body, $memo_category, $memo_passphrase); } else { /* Check permissions. */ - if ($GLOBALS['injector']->getInstance('Horde_Perms')->hasAppPermission('max_notes') !== true && - $GLOBALS['injector']->getInstance('Horde_Perms')->hasAppPermission('max_notes') <= Mnemo::countMemos()) { + if ($injector->getInstance('Horde_Perms')->hasAppPermission('max_notes') !== true && + $injector->getInstance('Horde_Perms')->hasAppPermission('max_notes') <= Mnemo::countMemos()) { header('Location: ' . Horde::applicationUrl('list.php', true)); exit; } @@ -233,11 +233,11 @@ case 'delete_memos': if (!is_null($memo_id) && Mnemo::getMemo($memolist_id, $memo_id)) { try { - $share = $GLOBALS['mnemo_shares']->getShare($memolist_id); + $share = $mnemo_shares->getShare($memolist_id); } catch (Horde_Share_Exception $e) { throw new Mnemo_Exception($e); } - if ($share->hasPermission($GLOBALS['registry']->getAuth(), Horde_Perms::DELETE)) { + if ($share->hasPermission($registry->getAuth(), Horde_Perms::DELETE)) { $storage = &Mnemo_Driver::singleton($memolist_id); $result = $storage->delete($memo_id); if ($result instanceof PEAR_Error) { diff --git a/nag/lib/Application.php b/nag/lib/Application.php index 38fb261d1..95ee7f84a 100644 --- a/nag/lib/Application.php +++ b/nag/lib/Application.php @@ -72,20 +72,20 @@ class Nag_Application extends Horde_Registry_Application } /** - * Returns the specified permission for the current user. + * Returns the specified permission for the given app permission. * - * @param mixed $allowed The allowed permissions. + * @param string $permission The permission to check. + * @param mixed $allowed The allowed permissions. + * @param array $opts Additional options (NONE). * * @return mixed The value of the specified permission. */ - public function hasPermission($allowed) + public function hasPermission($permission, $allowed, $opts = array()) { - if (is_array($allowed)) { - switch ($permission) { - case 'max_tasks': - $allowed = max($allowed); - break; - } + switch ($permission) { + case 'max_tasks': + $allowed = max($allowed); + break; } return $allowed; -- 2.11.0