Re: SquirrelMail LDAP address book feature needs implementing/tweaking
|
View:
New views
4 Messages
—
Rating Filter:
Alert me
|
 |
|
|
|
Re: SquirrelMail LDAP address book feature needs implementing/tweaking
by Paul Lesniewski
::
Rate this Message:
On Mon, Jun 30, 2008 at 11:22 AM, David Härdeman <david@...> 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. 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(); [ldap-fields-v2.patch] 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: squirrelmail-devel@... List archives: http://news.gmane.org/gmane.mail.squirrelmail.devel List info (subscribe/unsubscribe/change options): https://lists.sourceforge.net/lists/listinfo/squirrelmail-devel |
|
|
Re: SquirrelMail LDAP address book feature needs implementing/tweaking
by David Härdeman-2
::
Rate this Message:
On Wed, July 2, 2008 05:30, Paul Lesniewski wrote:
> 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? It was intentional but I was hesitant whether it should be done or not. Note that the patch also changed the second arg when lookup called ldap_search from "true" to false. BEFORE: basedn = 'cn=' . $SEARCH . ',' . $this->basedn; filter = objectClass=* scope = base AFTER: basedn = $this->basedn filter = $attr . '=' . $SEARCH scope = sub So the original method would do an exact match and the later method would do a subtree search for a match. The original method would be guaranteed to match an LDAP entry added using the SQ LDAP interface while the latter would match other entries as well (somewhere else in the hierarchy). It's probably better to leave that part as it was for now (as you did in your proposed patch which looks fine). -- David Härdeman ------------------------------------------------------------------------- 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: squirrelmail-devel@... List archives: http://news.gmane.org/gmane.mail.squirrelmail.devel List info (subscribe/unsubscribe/change options): https://lists.sourceforge.net/lists/listinfo/squirrelmail-devel |
|
|
Re: SquirrelMail LDAP address book feature needs implementing/tweaking
by Paul Lesniewski
::
Rate this Message:
On Wed, Jul 2, 2008 at 12:25 AM, David Härdeman <david@...> wrote:
> On Wed, July 2, 2008 05:30, Paul Lesniewski wrote: >> 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? > > It was intentional but I was hesitant whether it should be done or not. I don't use LDAP, so am not certain what the answer is here. I changed it back because it seemed to be unrelated to the needed change(?). > Note that the patch also changed the second arg when lookup called > ldap_search from "true" to false. This was addressed in my last email. I changed it back in my proposed patch because I think you misunderstood that we only need one result, not more than that. > BEFORE: > > basedn = 'cn=' . $SEARCH . ',' . $this->basedn; > filter = objectClass=* > scope = base > > AFTER: > > basedn = $this->basedn > filter = $attr . '=' . $SEARCH > scope = sub > > > So the original method would do an exact match and the later method would > do a subtree search for a match. The original method would be guaranteed > to match an LDAP entry added using the SQ LDAP interface while the latter > would match other entries as well (somewhere else in the hierarchy). I see. > It's probably better to leave that part as it was for now (as you did in > your proposed patch which looks fine). OK, thanks so very very very very very much for your help! Did I mention thank you? ------------------------------------------------------------------------- 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: squirrelmail-devel@... List archives: http://news.gmane.org/gmane.mail.squirrelmail.devel List info (subscribe/unsubscribe/change options): https://lists.sourceforge.net/lists/listinfo/squirrelmail-devel |
| Free Forum Powered by Nabble | Forum Help |