[sork] Patch for forwards

Richard.Heggs at nottinghamcity.gov.uk Richard.Heggs at nottinghamcity.gov.uk
Mon Dec 29 04:46:36 PST 2003


Hi List,

This is my first attempt at updating the ldap driver for the forwards
module.  It is diffed against HEAD as of this morning.

The changes are similar to the ones I made to the vacation module a few
weeks ago.  In summary:


lib/Driver/{forwards,qmail,mdaemon,sql}.php:
- isEnabledForwarding() now returns 'Y' if forwarding is enabled, otherwise
false.

main.php:
- Make user of the new return values for isEnabledForwarding(), to provide
the user with a helpful status message where possible.

lib/Driver/ldap.php:
- Added a 'version' parameter - PHP4 defaults to LDAP protocol 2, but newer
versions of OpenLDAP default to protocol 3, so it is necessary to set the
appropriate option befor calling ldap_bind().
- Use the error notification mechanism currently in HEAD.
- Tidied up the code that actually retrieves data from the LDAP server.
- Gracefully handle an attempt to double-free the LDAP forwarding
attribute.
- Rework isEnabledForwarding() to return 'Y' or 'N'.

Note that this patch does NOT make the LDAP driver handle 'Keep a copy in
your local mailbox?'.  That functionality is not currently present in LDAP,
and I'd like to update the existing functionality first, before adding new
goodies.

Please consider committing this to CVS.  Feedback would be useful, too :)


Richard


----------------------------------------------------------------
This message was sent using IMP, the Internet Messaging Program.

######################################################################
This e-mail (and any attachments) is confidential and may contain personal
views which are not the views of Nottingham City Council unless specifically
stated. If you have received it in error, please delete it from your system,
do not use, copy or disclose the information in any way nor act in reliance
on it and notify the sender immediately. Please note that Nottingham City
Council monitors e-mails sent or received. Further communication will
signify your consent to this.
######################################################################
-------------- next part --------------
Index: forwards/main.php
===================================================================
RCS file: /var/rsync-horde/forwards/main.php,v
retrieving revision 1.24
diff -u -r1.24 main.php
--- forwards/main.php	16 Sep 2003 23:04:15 -0000	1.24
+++ forwards/main.php	21 Dec 2003 15:04:55 -0000
@@ -74,11 +74,14 @@
     }
 }
 
-// If we can tell if the forwards are enabled, then say so. But if
-// this fails, it could be because it is disabled, or just because we
-// can't tell, so just be quiet about it.
-if ($driver->isEnabledForwarding($user, $realm, Auth::getCredential('password'))) {
+// The backend always returns 'Y' if forwarding is enabled, but may not be
+// able to say with certainty that forwarding is *disabled* - 'N' means
+// "there is definitely no forwarding address set", false means "I dunno"
+$pass = Auth::getCredential('password');
+if ($driver->isEnabledForwarding($user, $realm, $pass) == 'Y') {
     $notification->push(_("Forwarding is currently enabled."), 'horde.success');
+} elseif ($driver->isEnabledForwarding($user, $realm, $pass) == 'N') {
+    $notification->push(_("Forwarding is currently disabled."), 'horde.warning');
 }
 
 $title = _("Set or Remove E-Mail Forwards");
Index: forwards/lib/Driver/forwards.php
===================================================================
RCS file: /var/rsync-horde/forwards/lib/Driver/forwards.php,v
retrieving revision 1.23
diff -u -r1.23 forwards.php
--- forwards/lib/Driver/forwards.php	20 Sep 2003 20:40:44 -0000	1.23
+++ forwards/lib/Driver/forwards.php	29 Dec 2003 12:24:44 -0000
@@ -222,11 +222,16 @@
      *
      * @param string    $user       The username to check forwarding for.
      * @param string    $realm      The realm of the user.
-     * @return boolean  Returns true/false based on if forwarding is enabled.
+     * @return mixed		    'Y' if forwarding is enabled, or false.
      */
     function isEnabledForwarding($user, $realm, $password)
     {
-        return $this->currentTarget($user, $realm, $password) != false;
+        $yn = $this->currentTarget($user, $realm, $password);
+        if ($yn) {
+          return 'Y';
+        } else {
+          return false;
+        }
     }
 
 }
Index: forwards/lib/Driver/ldap.php
===================================================================
RCS file: /var/rsync-horde/forwards/lib/Driver/ldap.php,v
retrieving revision 1.1
diff -u -r1.1 ldap.php
--- forwards/lib/Driver/ldap.php	11 Jul 2003 12:28:21 -0000	1.1
+++ forwards/lib/Driver/ldap.php	29 Dec 2003 10:04:01 -0000
@@ -45,10 +45,15 @@
      * @return   boolean   True or False based on success of connect and bind.
      *
      */
-    function _connect($userdn = null, $password = null, $realm = 'default') {
+    function _connect($userdn = null, $password = null, $realm = 'default')
+    {
         $this->_ds = ldap_connect($this->_params[$realm]['host'], $this->_params[$realm]['port']);
         if (!$this->_ds) {
-           return PEAR::raiseError(_("Could not connect to ldap server"));
+            return PEAR::raiseError(_("Could not connect to ldap server"));
+        }
+        if (array_key_exists('version', $this->_params[$realm])) {
+             ldap_set_option($this->_ds, LDAP_OPT_PROTOCOL_VERSION,
+                                        $this->_params[$realm]['version']);
         }
 
         if (!is_null($userdn)) {
@@ -58,7 +63,7 @@
         }
 
         if (!$result) {
-          return PEAR::raiseError(_("Could not bind to ldap server"));
+            return PEAR::raiseError(_("Could not bind to ldap server"));
         }
 
         return true;
@@ -163,18 +168,15 @@
 
         // connect as the user
         $res = $this->_connect($userdn, $pass, $realm);
-        if (PEAR::isError($res)) {
-            $this->_disconnect();
-            if ($res->getMessage() == _("Could not bind to ldap server")) {
-                return PEAR::raiseError(_("Incorect Password"));
-            }
-            return $res;
+        if (is_a($res, 'PEAR_Error')) {
+            $this->err_str = $res->getMessage();
+            $this->err_str .= ' - ' .  _("Check your password");
+            return false;
         }
 
 	// change the user's forwards.
         $newDetails[$this->_params[$realm]['forwards']] = $message;
         $res = ldap_mod_replace($this->_ds, $userdn, $newDetails);
-        $value = $this->_get_forwards($userdn, $this->_params[$realm]['forwards']);
         if (!$res) {
             $res = PEAR::raiseError(ldap_error($this->_ds));
         }
@@ -185,51 +187,14 @@
         return true;
     }
 
-  function get_forwards($realm = 'default', $user, $pass) {
-        // Make sure the configuration file is correct
-        if (!$this->check_config($realm)) {
-            return false;
-        }
-
-        // get the user's dn
-        if ( array_key_exists('userdn', $this->_params[$realm])) {
-            $userdn = $this->_params[$realm]['userdn'];
-        } else {
-            $userdn = $this->_lookupdn($user, $realm);
-            if (PEAR::isError($userdn)) {
-                return $userdn;
-            }
-        }
-
-        // connect as the user
-        $res = $this->_connect($userdn, $pass, $realm);
-        if (PEAR::isError($res)) {
-            $this->_disconnect();
-            if ($res->getMessage() == _("Could not bind to ldap server")) {
-                return PEAR::raiseError(_("Incorect Password"));
-            }
-            return $res;
-        }
-
-	$vac = $this->_params[$realm]['forwards'];
-	$msg = $this->_get_forwards($userdn, $vac);
-
-        // disconnect from ldap server
-        $this->_disconnect();
-
-	return $msg;
-  }
-
-  function _get_forwards($userdn, $vac) {
-	$sr = ldap_search($this->_ds, $userdn, "$vac=*");
+  function _get_forwards($userdn, $attribs) {
+	$sr = ldap_search($this->_ds, $userdn, 'uid=*', $attribs);
 	$entry = ldap_first_entry($this->_ds, $sr);
-	if (!$entry) {
-	  return false;
-        }
-	$values = ldap_get_values($this->_ds, $entry, $vac);
-	if ($values["count"] == 0) {
-	   return false;
-        }
+        $res = ldap_get_attributes($this->_ds, $entry);
+        if ($res['count'] == 0) {
+	    return false;
+	}
+	$values = ldap_get_values($this->_ds, $entry, $attribs[0]);
 	return $values[0];
   }
 
@@ -251,7 +216,7 @@
         }
 
         // get the user's dn
-        if ( array_key_exists('userdn', $this->_params[$realm])) {
+        if (array_key_exists('userdn', $this->_params[$realm])) {
             $userdn = $this->_params[$realm]['userdn'];
         } else {
             $userdn = $this->_lookupdn($user, $realm);
@@ -262,17 +227,19 @@
 
         // connect as the user
         $res = $this->_connect($userdn, $pass, $realm);
-        if (PEAR::isError($res)) {
-            $this->_disconnect();
-            if ($res->getMessage() == _("Could not bind to ldap server")) {
-                return PEAR::raiseError(_("Incorect Password"));
-            }
-            return $res;
+        if (is_a($res, 'PEAR_Error')) {
+            $this->err_str = $res->getMessage();
+            $this->err_str .= ' - ' .  _("Check your password");
+            return false;
         }
 
 	// del the user's forwards.
-	$value = $this->_get_forwards($userdn,
-				      $this->_params[$realm]['forwards']);
+	$attribs = array($this->_params[$realm]['forwards']);
+	$value = $this->_get_forwards($userdn, $attribs);
+        if (!$value) {
+	    // Nothing do delete, so pretend we did anyway.
+	    return true;
+	}
 	$newDetails[$this->_params[$realm]['forwards']] = $value;
         $res = ldap_mod_del($this->_ds, $userdn, $newDetails);
         if (!$res) {
@@ -285,8 +252,6 @@
         return true;
     }
 
-}
-
 	/**
 	 * Retrieves status of mail redirection for a user
 	 *
@@ -294,17 +259,86 @@
 	 *
 	 * @param string	$realm	  The realm of the user to check.
 	 *
-	 * @return boolean	Returns true if forwarding is enabled for the user or false if
-	 *				  forwarding is currently disabled.
+	 * @return mixed	Returns 'Y' if forwarding is enabled for the
+	 *			user or 'N' if forwarding is currently
+	 *			disabled.
 	 */
-	function isEnabledForwarding($user, $realm, $pass)
-	{
-		if ($this->get_forwards($user, $realm, $pass)) {
-                   return true;
-                }
-                else {
-                   return false;
-                }
-	}
+    function isEnabledForwarding($user, $realm, $pass)
+    {
+        // Make sure the configuration file is correct
+        if (!$this->check_config($realm)) {
+            return false;
+        }
+    
+        // Get the user's dn.
+        if (array_key_exists('userdn', $this->_params[$realm])) {
+            $userdn = $this->_params[$realm]['userdn'];
+        } else {
+            $userdn = $this->_lookupdn($user, $realm);
+            if (is_a($userdn, 'PEAR_Error')) {
+                return $userdn;
+            }
+        }
 
+        // Connect as the user.
+        $res = $this->_connect($userdn, $pass, $realm);
+        if (is_a($res, 'PEAR_Error')) {
+            $this->_disconnect();
+            if ($res->getMessage() == _("Could not bind to ldap server")) {
+                return PEAR::raiseError(_("Incorrect Password"));
+            }
+            return $res;
+        }
+
+        $attribs = array($this->_params[$realm]['forwards']);
+        if ($this->_get_forwards($userdn, $attribs)) {
+            $this->_disconnect();
+            return 'Y';
+        } else {
+            $this->_disconnect();
+            return 'N';
+        }
+    }
+
+    /**
+     * Retrieves current target of mail redirection
+     *
+     * @param string    $user       The username of the user to get forward of.
+     * @param string    $realm      The realm of the user.
+     *
+     * @return mixed    A string of current forwarding mail address, or false.
+     */
+    function currentTarget($user, $realm = 'default', $pass)
+    {
+        // Make sure the configuration file is correct
+        if (!$this->check_config($realm)) {
+            return false;
+        }
+
+        // Get the user's dn.
+        if (array_key_exists('userdn', $this->_params[$realm])) {
+            $userdn = $this->_params[$realm]['userdn'];
+        } else {
+            $userdn = $this->_lookupdn($user, $realm);
+            if (is_a($userdn, 'PEAR_Error')) {
+                return $userdn;
+            }
+        }
+
+        // Connect as the user.
+        $res = $this->_connect($userdn, $pass, $realm);
+        if (is_a($res, 'PEAR_Error')) {
+            $this->_disconnect();
+            if ($res->getMessage() == _("Could not bind to ldap server")) {
+                return PEAR::raiseError(_("Incorrect Password"));
+            }
+            return $res;
+        }
+
+        $attribs = array($this->_params[$realm]['forwards']);
+
+        return $this->_get_forwards($userdn, $attribs);
+    }
+
+}
 ?>
Index: forwards/lib/Driver/mdaemon.php
===================================================================
RCS file: /var/rsync-horde/forwards/lib/Driver/mdaemon.php,v
retrieving revision 1.8
diff -u -r1.8 mdaemon.php
--- forwards/lib/Driver/mdaemon.php	27 Apr 2003 00:27:26 -0000	1.8
+++ forwards/lib/Driver/mdaemon.php	29 Dec 2003 12:25:36 -0000
@@ -167,8 +167,7 @@
      *
      * @param string    $realm      The realm of the user to check.
      *
-     * @return boolean    Returns true if forwarding is enabled for the user or false if
-     *                  forwarding is currently disabled.
+     * @return mixed    Returns 'y' if forwarding is enabled, or false.
      */
     function isEnabledForwarding($user, $realm = 'default', $password = "")
     {
@@ -181,7 +180,7 @@
 
         // check forwarding flag
         if (substr($current_details, 216, 1) == "Y") {
-            return true;
+            return 'Y';
         } else {
             return false;
         }
Index: forwards/lib/Driver/qmail.php
===================================================================
RCS file: /var/rsync-horde/forwards/lib/Driver/qmail.php,v
retrieving revision 1.7
diff -u -r1.7 qmail.php
--- forwards/lib/Driver/qmail.php	20 Sep 2003 20:40:44 -0000	1.7
+++ forwards/lib/Driver/qmail.php	29 Dec 2003 12:25:53 -0000
@@ -215,11 +215,16 @@
      *
      * @param string    $user       The username to check forwarding for.
      * @param string    $realm      The realm of the user.
-     * @return boolean  Returns true/false based on if forwarding is enabled.
+     * @return Mixed    Returns 'Y' if forwarding is enabled, or false.
      */
     function isEnabledForwarding($user, $realm, $password)
     {
-        return $this->currentTarget($user, $realm, $password) != false;
+        $yn = $this->currentTarget($user, $realm, $password);
+        if ($yn) {
+            return 'Y';
+        } else {
+            return false;
+        }
     }
 
 }
Index: forwards/lib/Driver/sql.php
===================================================================
RCS file: /var/rsync-horde/forwards/lib/Driver/sql.php,v
retrieving revision 1.10
diff -u -r1.10 sql.php
--- forwards/lib/Driver/sql.php	25 Aug 2003 18:28:42 -0000	1.10
+++ forwards/lib/Driver/sql.php	29 Dec 2003 12:26:19 -0000
@@ -193,9 +193,7 @@
      *
      * @param string        $realm    The realm of the user to check.
      *
-     * @return boolean      Returns true if forwarding is enabled for the
-     *                      user or false if forwarding is currently
-     *                      disabled for the user.
+     * @return mixed        Returns 'Y' if forwarding is enabled, or false.
      */
     function isEnabledForwarding($user, $realm, $password)
     {
@@ -209,7 +207,7 @@
         // check forwarding flag
         if ($current_details[$this->_params['forward']] === 'y' ||
              $current_details[$this->_params['forward']] === 'Y') {
-            return true;
+            return 'Y';
         } else {
             return false;
         }


More information about the sork mailing list