Re: SquirrelMail LDAP address book feature needs implementing/tweaking

On Mon, Jun 30, 2008 at 11:22 AM, David Härdeman <[email protected]> wrote:
> On Wed, Jun 18, 2008 at 06:16:19PM -0700, Paul Lesniewski wrote:
>>
>> Hi David,
>>
>> I saw that your code was adopted for the SquirrelMail development
>> branch for LDAP address book lookups, adds, deletes, etc.  If you are
>> interested, the other address book backends now have the ability to
>> look up address book entries by fields other than the nickname/alias.
>> The LDAP backend needs to have this implemented.  See:
>>
>> http://squirrelmail.svn.sourceforge.net/squirrelmail/?rev=13186&view=rev
>
> The attached patch should hopefully do the right thing. I can unfortunately
> not test it right now since I don't have access to an LDAP enabled SQ
> installation at the moment.

I have a question about what $dn becomes in your patch as opposed to
what it was before.  Before your patch, there was a comma and
$this->basedn following the "cn=VALUE".  Your patch takes it out.  Is
this intentional or a bug?

Also, you changed the argument to ldap_search() so that more than one
entry is acceptable as a return.  I think this is a misunderstanding
of the needed functionality: what we need is the ability to look up
entries by fields other than nickname, primarily by email.  If there
are more than one, only the first one need be returned, as the usual
use is just a test to see if such an entry exists.  I think a "search"
would want all entries to be returned, but a "lookup" as we have here
need not return them all.

I am inclined to adopt your patch, modified as follows (uncertain
about if "," . $this->basedn is needed).  It is also attached.


Index: functions/abook_ldap_server.php
===================================================================
--- functions/abook_ldap_server.php     (revision 13187)
+++ functions/abook_ldap_server.php     (working copy)
@@ -12,7 +12,7 @@
  * StartTLS code by John Lane
  *   <starfry at users.sourceforge.net> (#1197703)
  * Code for remove, add, modify, lookup by David Hﾃ､rdeman
- *   <david at 2gen.com> (#1495763)
+ *   <david at hardeman.nu> (#1495763)
  *
  * This backend uses LDAP person (RFC2256), organizationalPerson (RFC2256)
  * and inetOrgPerson (RFC2798) objects and dn, description, sn, givenname,
@@ -424,7 +424,7 @@
             return false;
         }

-        $attributes = array('dn', 'description', 'sn', 'givenname',
'cn', 'mail');
+        $attributes = array('dn', 'description', 'sn', 'givenName',
'cn', 'mail');

         if ($singleentry) {
             // ldap_read - search for one single entry
@@ -672,6 +672,34 @@
         }
     }

+    /**
+     * Determine internal attribute name given one of
+     * the SquirrelMail SM_ABOOK_FIELD_* constants
+     *
+     * @param integer $attr The SM_ABOOK_FIELD_* contant to look up
+     *
+     * @return string The desired attribute name, or the string "ERROR"
+     *                if the $field is not understood (the caller
+     *                is responsible for handing errors)
+     *
+     */
+    function get_attr_name($attr) {
+        switch ($attr) {
+            case SM_ABOOK_FIELD_NICKNAME:
+                return 'cn';
+            case SM_ABOOK_FIELD_FIRSTNAME:
+                return 'givenName';
+            case SM_ABOOK_FIELD_LASTNAME:
+                return 'sn';
+            case SM_ABOOK_FIELD_EMAIL:
+                return 'mail';
+            case SM_ABOOK_FIELD_LABEL:
+                return 'description';
+            default:
+                return 'ERROR';
+        }
+    }
+
     /* ========================== Public ======================== */

     /**
@@ -743,15 +771,16 @@
      */
     function lookup($value, $field=SM_ABOOK_FIELD_NICKNAME) {

-//FIXME: implement lookup by other fields
-        if ($field != SM_ABOOK_FIELD_NICKNAME)
-            return $this->set_error('LDAP lookup of fields other than
nickname/alias not yet implemented');

-        /* Generate the dn and try to retrieve that single entry */
-        $cn = $this->quotevalue($value);
-        $dn = 'cn=' . $cn . ',' . $this->basedn;
+        $attr = get_attr_name($field);
+        if ($attr == 'ERROR') {
+            return $this->set_error(sprintf(_("Unknown field name:
%s"), $field));
+        }

-        /* Do the search */
+        // Generate the dn
+        $dn = $attr . '=' . $this->quotevalue($value) . ',' . $this->basedn;
+
+        // Do the search
         $result = $this->ldap_search($dn, true);
         if (!is_array($result) || count($result) < 1)
             return array();

-------------------------------------------------------------------------
Sponsored by: SourceForge.net Community Choice Awards: VOTE NOW!
Studies have shown that voting for your favorite open source project,
along with a healthy diet, reduces your potential for chronic lameness
and boredom. Vote Now at http://www.sourceforge.net/community/cca08

-----
squirrelmail-devel mailing list
Posting guidelines: http://squirrelmail.org/postingguidelines
List address: [email protected]
List archives: http://news.gmane.org/gmane.mail.squirrelmail.devel
List info (subscribe/unsubscribe/change options): https://lists.sourceforge.net/lists/listinfo/squirrelmail-devel

Index: functions/abook_ldap_server.php
===================================================================
--- functions/abook_ldap_server.php	(revision 13187)
+++ functions/abook_ldap_server.php	(working copy)
@@ -12,7 +12,7 @@
  * StartTLS code by John Lane
  *   <starfry at users.sourceforge.net> (#1197703)
  * Code for remove, add, modify, lookup by David Härdeman
- *   <david at 2gen.com> (#1495763)
+ *   <david at hardeman.nu> (#1495763)
  *
  * This backend uses LDAP person (RFC2256), organizationalPerson (RFC2256)
  * and inetOrgPerson (RFC2798) objects and dn, description, sn, givenname,
@@ -424,7 +424,7 @@
             return false;
         }
 
-        $attributes = array('dn', 'description', 'sn', 'givenname', 'cn', 'mail');
+        $attributes = array('dn', 'description', 'sn', 'givenName', 'cn', 'mail');
 
         if ($singleentry) {
             // ldap_read - search for one single entry
@@ -672,6 +672,34 @@
         }
     }
 
+    /**
+     * Determine internal attribute name given one of
+     * the SquirrelMail SM_ABOOK_FIELD_* constants
+     *
+     * @param integer $attr The SM_ABOOK_FIELD_* contant to look up
+     *
+     * @return string The desired attribute name, or the string "ERROR"
+     *                if the $field is not understood (the caller
+     *                is responsible for handing errors)
+     *
+     */
+    function get_attr_name($attr) {
+        switch ($attr) {
+            case SM_ABOOK_FIELD_NICKNAME:
+                return 'cn';
+            case SM_ABOOK_FIELD_FIRSTNAME:
+                return 'givenName';
+            case SM_ABOOK_FIELD_LASTNAME:
+                return 'sn';
+            case SM_ABOOK_FIELD_EMAIL:
+                return 'mail';
+            case SM_ABOOK_FIELD_LABEL:
+                return 'description';
+            default:
+                return 'ERROR';
+        }
+    }
+
     /* ========================== Public ======================== */
 
     /**
@@ -743,15 +771,16 @@
      */
     function lookup($value, $field=SM_ABOOK_FIELD_NICKNAME) {
 
-//FIXME: implement lookup by other fields
-        if ($field != SM_ABOOK_FIELD_NICKNAME)
-            return $this->set_error('LDAP lookup of fields other than nickname/alias not yet implemented');
 
-        /* Generate the dn and try to retrieve that single entry */
-        $cn = $this->quotevalue($value);
-        $dn = 'cn=' . $cn . ',' . $this->basedn;
+        $attr = get_attr_name($field);
+        if ($attr == 'ERROR') {
+            return $this->set_error(sprintf(_("Unknown field name: %s"), $field));
+        }
 
-        /* Do the search */
+        // Generate the dn
+        $dn = $attr . '=' . $this->quotevalue($value) . ',' . $this->basedn;
+
+        // Do the search
         $result = $this->ldap_search($dn, true);
         if (!is_array($result) || count($result) < 1)
             return array();