From: Ben Klang Date: Sat, 2 Jan 2010 01:59:09 +0000 (-0500) Subject: Enable deleting extensions in the LDAP driver X-Git-Url: https://git.internetallee.de/?a=commitdiff_plain;h=064d1bb522ce440a8b3354926a32ef8ac8baa3b2;p=horde.git Enable deleting extensions in the LDAP driver Also make fixes to the extensions processing page. --- diff --git a/shout/extensions.php b/shout/extensions.php index 261f467be..adeb6d7e8 100644 --- a/shout/extensions.php +++ b/shout/extensions.php @@ -40,20 +40,25 @@ case 'edit': } catch (Exception $e) { $notification->push($e); } - } else { - // Create a new add/edit form - $extension = Horde_Util::getFormData('extension'); - $extensions = $shout_extensions->getExtensions($context); - $vars = new Horde_Variables($extensions[$extension]); - if ($action == 'edit') { - $vars->set('oldextension', $extension); - } - $vars->set('action', $action); - $Form = new ExtensionDetailsForm($vars); - $Form->open($RENDERER, $vars, Horde::applicationUrl('extensions.php'), 'post'); - // Make sure we get the right template below. - $action = 'edit'; + + break; + } elseif ($Form->isSubmitted()) { + $notification->push(_("Problem processing the form. Please check below and try again."), 'horde.warning'); + } + + // Create a new add/edit form + $extension = Horde_Util::getFormData('extension'); + $extensions = $shout_extensions->getExtensions($context); + $vars = new Horde_Variables($extensions[$extension]); + if ($action == 'edit') { + $vars->set('oldextension', $extension); } + $vars->set('action', $action); + $Form = new ExtensionDetailsForm($vars); + $Form->open($RENDERER, $vars, Horde::applicationUrl('extensions.php'), 'post'); + // Make sure we get the right template below. + $action = 'edit'; + break; case 'delete': @@ -70,16 +75,19 @@ case 'delete': try { $Form->execute(); $notification->push(_("Extension Deleted.")); + $action = 'list'; } catch (Exception $e) { $notification->push($e); - $action = 'list'; } - } else { - $vars = Horde_Variables::getDefaultVariables(array()); - $Form = new ExtensionDeleteForm($vars); - $Form->open($RENDERER, $vars, Horde::applicationUrl('extensions.php'), 'post'); + } elseif ($Form->isSubmitted()) { + $notification->push(_("Problem processing the form. Please check below and try again."), 'horde.warning'); } + $vars = Horde_Variables::getDefaultVariables(array()); + $vars->set('context', $context); + $Form = new ExtensionDeleteForm($vars); + $Form->open($RENDERER, $vars, Horde::applicationUrl('extensions.php'), 'post'); + break; case 'list': diff --git a/shout/lib/Driver.php b/shout/lib/Driver.php index b343bb729..61bb56a26 100644 --- a/shout/lib/Driver.php +++ b/shout/lib/Driver.php @@ -127,11 +127,27 @@ class Shout_Driver { */ public function saveExtension($context, $extension, $details) { + if (empty($context) || empty($extension)) { + throw new Shout_Exception(_("Invalid extension.")); + } + if (!Shout::checkRights("shout:contexts:$context:extensions", PERMS_EDIT, 1)) { throw new Shout_Exception(_("Permission denied to save extensions in this context.")); } } + public function deleteExtension($context, $extension) + { + if (empty($context) || empty($extension)) { + throw new Shout_Exception(_("Invalid extension.")); + } + + if (!Shout::checkRights("shout:contexts:$context:users", + PERMS_DELETE, 1)) { + throw new Shout_Exception(_("Permission denied to delete users in this context.")); + } + } + /** * Attempts to return a concrete Shout_Driver instance based on * $driver. diff --git a/shout/lib/Driver/Ldap.php b/shout/lib/Driver/Ldap.php index b65475903..57763be6d 100644 --- a/shout/lib/Driver/Ldap.php +++ b/shout/lib/Driver/Ldap.php @@ -496,6 +496,7 @@ class Shout_Driver_Ldap extends Shout_Driver */ public function saveExtension($context, $extension, $details) { + // Check permissions parent::saveExtension($context, $extension, $details); // FIXME: Fix and uncomment the below @@ -533,33 +534,24 @@ class Shout_Driver_Ldap extends Shout_Driver // This is a change to an existing extension // First, identify the DN to modify // FIXME: Quote these strings - $filter = '(&(AstVoicemailMailbox=%s)(AstContext=%s))'; - $filter = sprintf($filter, $details['oldextension'], $context); - $attributes = array('dn'); - - $res = ldap_search($this->_LDAP, $this->_params['basedn'], - $filter, $attributes); - if ($res === false) { - $msg = sprintf('LDAP Error (%s): %s', ldap_errno($this->_LDAP), - ldap_error($this->_LDAP)); - Horde::logMessage($msg, __FILE__, __LINE__, PEAR_LOG_ERR); - throw new Shout_Exception(_("Error while searching the directory. Details have been logged for the administrator.")); - } + $olddn = $this->_getExtensionDn($context, $extension); // If the extension has changed we need to perform an object rename if ($extension != $details['oldextension']) { - $res = ldap_rename($this->_LDAP, $res['dn'], $rdn, + $res = ldap_rename($this->_LDAP, $olddn, $rdn, $this->_params['basedn'], true); if ($res === false) { - $msg = sprintf('LDAP Error (%s): %s', ldap_errno($this->_LDAP), + $msg = sprintf('LDAP Error (%s): %s', ldap_errno($this->_LDAP), ldap_error($this->_LDAP)); - Horde::logMessage($msg, __FILE__, __LINE__, PEAR_LOG_ERR); - throw new Shout_Exception(_("Error while modifying the directory. Details have been logged for the administrator.")); + Horde::logMessage($msg, __FILE__, __LINE__, PEAR_LOG_ERR); + throw new Shout_Exception(_("Error while modifying the directory. Details have been logged for the administrator.")); } } // Now apply the changes + // Avoid changing the objectClass, just in case + unset($entry['objectClass']); $res = ldap_modify($this->_LDAP, $dn, $entry); if ($res === false) { $msg = sprintf('LDAP Error (%s): %s', ldap_errno($this->_LDAP), @@ -581,44 +573,79 @@ class Shout_Driver_Ldap extends Shout_Driver return true; } + // Catch-all. We should not get here. throw new Shout_Exception(_("Unspecified error.")); } /** - * Deletes a user from the LDAP tree + * Deletes an extension from the LDAP tree * * @param string $context Context to delete the user from * @param string $extension Extension of the user to be deleted * * @return boolean True on success, PEAR::Error object on error */ - public function deleteUser($context, $extension) + public function deleteExtension($context, $extension) + { + // Check permissions + parent::deleteExtension($context, $extension); + + $dn = $this->_getExtensionDn($context, $extension); + + $res = @ldap_delete($this->_LDAP, $dn); + if ($res === false) { + $msg = sprintf('LDAP Error (%s): %s', ldap_errno($this->_LDAP), + ldap_error($this->_LDAP)); + Horde::logMessage($msg, __FILE__, __LINE__, PEAR_LOG_ERR); + throw new Horde_Exception(_("Error while deleting from the directory. Details have been logged for the administrator.")); + } + + return true; + } + + /** + * + * @param $context + * @param $extension + */ + protected function _getExtensionDn($context, $extension) { - $ldapKey = &$this->_ldapKey; - $appKey = &$this->_appKey; + // FIXME: Sanitize filter string against LDAP injection + $filter = '(&(AstVoicemailMailbox=%s)(AstContext=%s))'; + $filter = sprintf($filter, $extension, $context); + $attributes = array('dn'); - if (!Shout::checkRights("shout:contexts:$context:users", - PERMS_DELETE, 1)) { - return PEAR::raiseError("No permission to delete users in this " . - "context."); + $res = ldap_search($this->_LDAP, $this->_params['basedn'], + $filter, $attributes); + if ($res === false) { + $msg = sprintf('LDAP Error (%s): %s', ldap_errno($this->_LDAP), + ldap_error($this->_LDAP)); + Horde::logMessage($msg, __FILE__, __LINE__, PEAR_LOG_ERR); + throw new Shout_Exception(_("Error while searching the directory. Details have been logged for the administrator.")); } - $validusers = $this->getUsers($context); - if (!isset($validusers[$extension])) { - return PEAR::raiseError("That extension does not exist."); + if (ldap_count_entries($this->_LDAP, $res) < 1) { + throw new Shout_Exception(_("No such extension found.")); } - $dn = "$ldapKey=".$validusers[$extension][$appKey]; - $dn .= ',' . SHOUT_USERS_BRANCH . ',' . $this->_params['basedn']; + $res = ldap_first_entry($this->_LDAP, $res); + if ($res === false) { + $msg = sprintf('LDAP Error (%s): %s', ldap_errno($this->_LDAP), + ldap_error($this->_LDAP)); + Horde::logMessage($msg, __FILE__, __LINE__, PEAR_LOG_ERR); + throw new Shout_Exception(_("Error while searching the directory. Details have been logged for the administrator.")); + } - $res = @ldap_delete($this->_LDAP, $dn); - if (!$res) { - return PEAR::raiseError("Unable to delete $extension from " . - "$context: " . ldap_error($this->_LDAP)); + $dn = ldap_get_dn($this->_LDAP, $res); + if ($dn === false) { + $msg = sprintf('LDAP Error (%s): %s', ldap_errno($this->_LDAP), + ldap_error($this->_LDAP)); + Horde::logMessage($msg, __FILE__, __LINE__, PEAR_LOG_ERR); + throw new Shout_Exception(_("Internal LDAP error. Details have been logged for the administrator.")); } - return true; - } + return $dn; + } /* Needed because uksort can't take a classed function as its callback arg */ protected function _sortexten($e1, $e2) diff --git a/shout/lib/Forms/ExtensionForm.php b/shout/lib/Forms/ExtensionForm.php index db2fd962f..05aacb48e 100644 --- a/shout/lib/Forms/ExtensionForm.php +++ b/shout/lib/Forms/ExtensionForm.php @@ -76,7 +76,12 @@ class ExtensionDeleteForm extends Horde_Form { function __construct(&$vars) { - parent::__construct($vars, _("Delete Extension %s - Context: $context")); + $extension = $vars->get('extension'); + $context = $vars->get('context'); + + $title = _("Delete Extension %s - Context: %s"); + $title = sprintf($title, $extension, $context); + parent::__construct($vars, $title); $this->addHidden('', 'context', 'text', true); $this->addHidden('', 'extension', 'int', true); @@ -87,7 +92,7 @@ class ExtensionDeleteForm extends Horde_Form function execute() { global $shout_extensions; - $context = $this->_vars->get('extension'); + $context = $this->_vars->get('context'); $extension = $this->_vars->get('extension'); $shout_extensions->deleteExtension($context, $extension); }