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 261f467..adeb6d7 100644 (file)
--- 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 b343bb7..61bb56a 100644 (file)
--- 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 b654759..57763be 100644 (file)
--- 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 <type> $context
+     * @param <type> $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 db2fd96..05aacb4 100644 (file)
--- 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);
     }