[Tickets #551] NEW: Forwards LDAP driver
bugs at bugs.horde.org
bugs at bugs.horde.org
Tue Sep 7 17:03:56 PDT 2004
DO NOT REPLY TO THIS MESSAGE. THIS EMAIL ADDRESS IS NOT MONITORED.
Ticket URL: http://bugs.horde.org/ticket/?id=551
-----------------------------------------------------------------------
Ticket | 551
Created By | url-horde-tickets at freed.com
Summary | Forwards LDAP driver
Queue | Forwards
Version | RELENG_2
State | Unconfirmed
Priority | 2. Medium
Type | Bug
Owners |
-----------------------------------------------------------------------
url-horde-tickets at freed.com (2004-09-07 17:03) wrote:
I offer no warrantee on this code -- I'm not a native PHP speaker. But it
seems to work so far.
The patches below address several issues that I found in the LDAP driver for
Forwards. Each of the main issues is noted in the code with a comment
beginning // :ptf: (PTF is my initials).
First, a small patch to forwards/main.php. It seems to me that "forwarding
is enabled" should not be an error.
--- main.php.orig 2004-09-07 20:01:11.904287000 -0400
+++ main.php 2004-09-07 17:01:47.684079000 -0400
@@ -85,8 +85,9 @@
// 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'))) {
+ // :ptf: Why was this an ERROR?
Horde::raiseMessage( _("Forwarding is currently enabled."),
- HORDE_ERROR);
+ HORDE_SUCCESS);
}
include_once $registry->getTemplatePath('horde') .
'/javascript/open_help_win.js';
Now for the meat of it -- changes to forwards/lib/Driver/ldap.php. In
summary:
1) There was no function Forwards_Driver_ldap()
2) The local delivery flag was ignored. Added a config param (localEmail)
to support this function.
3) Errors such as "insufficient access" from the EnableForwarding()
function were not properly returned to the calling routine
4) isEnabledForwarding() needed to return Boolean values, not 'Y' and 'N'
5) Renamed _getForwards to _getFirstValue, since the name is more apt, and
changed filter from 'uid=*' to 'objectClass=*'
6) Added a utility function to remove the 'count' from arrays returned by
the ldap_get() functions.
Local Delivery still needs a bit of work. I added an attribute (localEmail)
to the config file to support this function. (The alternative -- building
an address out of the user and realm -- doesn't work well if the realm is
'default'.)
To properly handle local delivery, I need (1) to properly identify if the
local address is part of the forwarding string, and (2) if it is there,
strip it out when displaying the page and check off the "local delivery"
box. I should also handle the case where local delivery is the only
remaining address on the list.
--- ldap.php.orig 2004-09-07 16:13:51.668142000 -0400
+++ ldap.php 2004-09-07 19:44:26.845636000 -0400
@@ -22,6 +22,17 @@
*/
var $_ds;
+ // :ptf: The function Forwards_Driver_ldap was missing ... (?)
+ /**
+ * Constructs a new ldap Forwards_Driver object.
+ *
+ * @param array $params A hash containing connection
parameters.
+ */
+ function Forwards_Driver_ldap($params = array())
+ {
+ $this->_params = $params;
+ }
+
/**
* Check if the realm has a specific configuration. If not, try to
fall
* back on the default configuration. If still not a valid
configuration
@@ -41,6 +52,7 @@
}
// If still no host/port, then we have a misconfigured module.
+
if (empty($this->_params[$realm]['host']) ||
empty($this->_params[$realm]['port']) ) {
$this->_error = _("The module is not properly configured!");
@@ -55,7 +67,7 @@
* @param string $user The username to enable forwards for.
* @param string $realm The realm of the user.
* @param string $target The message to install.
- * @param string $keeplocal Never used.
+ * @param string $keeplocal Deliver message to box and forward it as
well.
*
* @return boolean Returns true on success, false on error.
*/
@@ -84,17 +96,64 @@
return false;
}
+
+ // :ptf: Handle "Local Delivery" flag
+ // Local delivery?
+ if ($keeplocal === 'on') {
+
+ // What is the local mailbox?
+ if ( $this->_params[$realm]['localEmail']) {
+ $attribs = array($this->_params[$realm]['localEmail']);
+ $local_target = $this->_getFirstValue($userdn, $attribs);
+ //
+ } else {
+ $res = PEAR::raiseError(_("Cannot determine local
mailbox"));
+ $this->err_str = "Cannot determine local mailbox";
+ $this->_disconnect();
+ return false;
+ }
+ // An alternative would be to guess what the local box
+ // might be (see below). But guessing badly is a bad thing.
+ // } elseif ($realm == 'default' ) {
+ // $local_target = $user;
+ // } else {
+ // $local_target = $user . '@' . $realm;
+ // }
+ //
+
+
+ // :ptf: LocalDelivery handling still has a two bugs
+ // BUG: I copied this code from the SQL driver -- but
+ // it ain't perfect:
+ // if the local target is foo at bar.com, and the
+ // forwards list contains bigfoo at bar.com, then
+ // it will get the wrong answer
+ //
+ // Assumes that Forwards attribute is comma-delimited list
+ if($local_target AND !strstr($message,$local_target)) {
+ $message .= ', ' . $local_target;
+ }
+
+ } else { // keeplocal is off
+ // BUG: We need to pull out the local target(s) if
+ // keeplocal is off
+ }
+
+
// Change the user's forwards.
$newDetails[$this->_params[$realm]['forwards']] = $message;
$res = ldap_mod_replace($this->_ds, $userdn, $newDetails);
+
+ // :ptf: Errors here were being ignored
if (!$res) {
$res = PEAR::raiseError(ldap_error($this->_ds));
+ $this->err_str = ldap_error($this->_ds);
+ $this->_disconnect();
+ return false;
+ } else {
+ $this->_disconnect();
+ return true;
}
-
- // Disconnect from the ldap server.
- $this->_disconnect();
-
- return true;
}
/**
@@ -133,7 +192,7 @@
// Delete the user's forwards.
$attribs = array($this->_params[$realm]['forwards']);
- $value = $this->_getForwards($userdn, $attribs);
+ $value = $this->_getFirstValue($userdn, $attribs);
if (!$value) {
// Nothing to delete, so treat as success.
return true;
@@ -158,9 +217,10 @@
*
* @param string $realm The realm of the user to check.
*
- * @return mixed Returns 'Y' if forwarding is enabled for the
- * user or 'N' if forwarding is currently
- * disabled.
+ * // :ptf: This function needs to return Boolean values, not strings
+ * @return boolean Returns true if forwarding is enabled for the
+ * user or false if forwarding is currently disabled or if
+ * an error occurs
*/
function isEnabledForwarding($user, $realm, $pass)
{
@@ -175,7 +235,7 @@
} else {
$userdn = $this->_lookupdn($user, $realm);
if (is_a($userdn, 'PEAR_Error')) {
- return $userdn;
+ return false;
}
}
@@ -184,18 +244,18 @@
if (is_a($res, 'PEAR_Error')) {
$this->_disconnect();
if ($res->getMessage() == _("Could not bind to ldap server"))
{
- return PEAR::raiseError(_("Incorrect Password"));
+ $res = PEAR::raiseError(_("Incorrect Password"));
}
- return $res;
+ return false;
}
$attribs = array($this->_params[$realm]['forwards']);
- if ($this->_getForwards($userdn, $attribs)) {
+ if ($this->_getFirstValue($userdn, $attribs)) {
$this->_disconnect();
- return 'Y';
+ return true;
} else {
$this->_disconnect();
- return 'N';
+ return false;
}
}
@@ -236,7 +296,7 @@
$attribs = array($this->_params[$realm]['forwards']);
- return $this->_getForwards($userdn, $attribs);
+ return $this->_getFirstValue($userdn, $attribs);
}
/**
@@ -275,9 +335,20 @@
return $userdn;
}
- function _getForwards($userdn, $attribs)
+ // :ptf: Renamed this function
+ /**
+ * Get the first available attribute value for a particular DSN
+ *
+ * @param $userdn The ldap basedn.
+ * @param $attribs A list whose first entry is the attribute we seek
+ *
+ * @return string The value of the attribute, or false if not found
+ *
+ */
+ function _getFirstValue($userdn, $attribs)
{
- $sr = ldap_search($this->_ds, $userdn, 'uid=*', $attribs);
+ // :ptf: Using objectClass, since we don't know if 'uid' exists
+ $sr = ldap_search($this->_ds, $userdn, 'objectClass=*', $attribs);
$entry = ldap_first_entry($this->_ds, $sr);
$res = ldap_get_attributes($this->_ds, $entry);
if ($res['count'] == 0) {
@@ -288,6 +359,32 @@
return $values[0];
}
+
+ // :ptf: Handy function to remove 'count' from ldap_get() results
+ /**
+ * Recursively remove the 'count' entry from an array
+ *
+ * Posted by mack at incise.org
+ * http://us4.php.net/manual/en/function.ldap-get-entries.php
+ *
+ * @param $arr The source array
+ *
+ * @return mixed The same array with no 'count' entries
+ *
+ */
+ function _rCountRemover($arr)
+ {
+ foreach($arr as $key=>$val) {
+ # (int)0 == "count", so we need to use ===
+ if($key === "count")
+ unset($arr[$key]);
+ elseif(is_array($val))
+ $arr[$key] = _rCountRemover($arr[$key]);
+ }
+ return $arr;
+ }
+
+
/**
* Do an ldap connect and bind as the guest user or as the optional
userdn.
*
More information about the bugs
mailing list