From PHP's documentation for ldap_read(), regarding the use of the optional "attributes" parameter:

Using this parameter is much more efficient than the default action (which is to return all attributes and their associated values). The use of this parameter should therefore be considered good practice.

In LDAPInterface::retrieveAttributes(), ldap_read does not pass the "attributes" parameter, thus grabbing all attributes for the given object. This causes lots unnecessary traffic, when the only attributes needed are those specifically mapped in _ldapdata_reverse_mappings().

The following patch adds a new, optional "attributes" parameter to LDAPInterface::retrieveAttributes(). If empty, all the attributes will be returned. Otherwise, only the requested attributes will be returned.

The patch also updates the three instances in the ldapdata module to request only those attributes mapped in _ldapdata_reverse_mappings().

CommentFileSizeAuthor
ldap_integration.patch2.35 KBjoelstein
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

cgmonroe’s picture

cgmonroe CreditAttribution: cgmonroe commented
Status: Needs review » Fixed

This is now supported using D7 LDAP's approach.

Searches / Attribute retrievals now call ldapauth_attributes_needed with an op code and sid to get the required attributes for the specific ops. OPS are defined by LDAPAUTH_SYNC_CONTEXT* constants.

Submodules implement the hook_ldap_attributes_needed_alter to define attributes they need for specific operations.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

aCCa’s picture

aCCa CreditAttribution: aCCa commented

Hi,

how can I use your function retrieveAttributes from within the "PHP to filter users based on their LDAP data: " field ?

I would like to check an attribute before permit login in ldap authentication. Before having all attributes was easy, now in $ldap variable I have ony mail dn and uid...

aCCa’s picture

aCCa CreditAttribution: aCCa commented

I solved my problem using ldapgroups to filter out user access through attribute, that's a more reliable way for sure.

The problem still exist: From version 6.x-1.0-beta3 $ldap variable in "PHP to filter users based on their LDAP data" (ldapauth admin page) doesn't contain the full ldap attributes but only the attributes useful for login, so I think that filter are no longer needed. What you think ?

Thanks for a really good module.

cgmonroe’s picture

cgmonroe CreditAttribution: cgmonroe commented

If enough folks have problems, I'll look at adding a switch in the gui to turn off the optimization.

However, my preference is to phase out the PhP filters as they are a possible security problem. I know a few tools that let you extract ALL site information by simply pasting PhP into a filter box. Not to mention the easy stuff like just changing the admin password. Not a major thing since you have to be logged in with the correct rights.

FWIW - there is a way to do the filtering via the new hooks added to the api. You would need to create a module that implements the hook_ldap_attributes_needed_alter() to return the extra attributes. Then you can implement the hook_ldap_user_deny_alter() to test for your specific attribute and deny access based on this.

See the API doc file, ldapauth.api.php for more information on these hooks.

The intent of the new APIs was to try to make it so the main code doesn't need to support every possible condition by making it easier for sites to do custom modules that plug in to meet their needs..