This is mainly directed at cgmonroe, but any insight might be helpful.
I was looking at the stats at https://drupal.org/project/usage/ldap_integration

Seems like a fair amount of use of dev version (to indicate) and a great amount of beta2s. There is a tendency for new implementers to use the beta rather than dev; for good reason. Because of all the patches since august, should we release a beta3? Do any of the patches require an update hook? I don't see any that do in the recent past, but haven't looked to closely.

I'm not using the 6 version, just trying to help out maintaining it. Perhaps a release candidate is more appropriate, but I think the mass sych/provisioning/paging issues would need to be fixed first.

There are also some no brainer patches needing review (https://drupal.org/project/issues/ldap_integration?text=&status=8&priori...) that maybe could be committed beforehand?

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

cgmonroe’s picture

Yes, getting to a new release state was what I have (slowly) been working towards since being drafted...lol

My loose plan has been to look through the pending patches list and start evaluating each for it's "riskiness". By this, I mean looking to each one to get a feel for: code complexity; changes to existing core logic; testing requirements; and the like. I've then been getting them applied to the current code base and running them through my test site to test the new / changed functionality.

Technically, this should only involve "bug fixes" in "beta mode"... but it's been beta so long that I'm looking at both features and bugs. But by being thoughtful about the code being changed and doing testing, I feel we can end up with a fairly well tested, stable dev that can be tagged as a release.

Timing for this depends on how many "code sprint" windows that work/life will allow and how much we want to get into the release. For example, deciding if the next beta is held up waiting for all the Sync module problems to be worked out (IMHO, this will take some time to get right) or not.

FWIW: I'm actually looking at the new features below before circling back to the bugs. The thought being that having them in the -dev first would give a little more time for "Murphy" to rear his head prior to a release.

Anyway, this is my first take on the new features with patches that I'm looking at now.

#62784: Mapping image-type attribute from LDAP - Been around/requested a lot
#1400154: Ldapdata module support for updating LDAP attributes from Content Profile Nodes - Blatant self interest... and useful to others :)
#692670: Integration with Features Module - Capturing setting in a feature module would make multi-site config easy / prevent config typos
#1031426: Return only specific attributes from ldap_read, instead of all attributes (optimization) - Seems like a good performance optimization

As to bugs, I haven't had time to get a good read on these other than:

Ldap Sync seems to have the greatest need here... but may take some semi major re-writing.

Allowing e-mail duplication needs to be fixed (since it's against Drupal standards)

Will probably concentrate on the newest reports... a lot of stuff marked as 6.x seem to only have errors reported in the 5.x version and no confirmation that the problem exists in 6.x.

Well that's my first take on this... will probably change based on comments, etc.

johnbarclay’s picture

Sounds like you have a great plan. As far as querying the needed attributes, In the 7.x-2.0 branch I'm creating a drupal alter hook " hook_ldap_attributes_needed_alter" that takes an $op and $server id as arguments. This allows other ldap etc modules to add on what attributes they need in specific contexts. The following is roughly how it will work in the ldap_user module which is a combination of ldap sync, ldap provision, and ldap profile from d6.

I'll keep my eye out for any code in the 7 branch that is useful for your 4 features.

/**
 * implements hook_ldap_attributes_needed_alter()
 */
function ldap_user_ldap_attributes_needed_alter(&$attributes, $op, $sid = NULL) {
  $ldap_user_conf = ldap_user_conf();
  // get server object
  // determine if puid property is set
  $attributes[] = 'dn';

  if ($sid) { // puid attributes are server specific
    $ldap_server = ldap_servers_get_servers($sid);

    switch ($op) {

      case 'create_user':
      case 'update_user':
        $attributes[] = $ldap_server->user_attr;
        $attributes[] = $ldap_server->mail_attr;
        ldap_servers_extract_attributes_from_token($attributes,  $ldap_server->mail_template);
        $attributes[] = $ldap_server->unique_persistent_attr;
      break;

    }
  }

  return $attributes;
}
eiriksm’s picture

With the same "blatant self interest, and useful to others" argument, I would really like to see this commited:

#1209556: Need to check for existing emails in ldapauth.module

Would be great to use official release instead of patched.

kevin.mcnamee@mailbox.org’s picture

Subscribing

cgmonroe’s picture

Assigned: Unassigned » cgmonroe

FYI - I'm going to start using this Issue to post status / info about commits related to getting this release out. Please do a new issue if there is a problem with a new feature or bug fix posted here. That said....

Support for Features (and more) was just committed to the dev version. It may be because of changes in Features/Strongarm but the changes to the globals (#692670: Integration with Features Module patch #8) did not need to be made. This has had extensive testing and since it does not impact the core code, I've added it. Please open a NEW issue if I missed anything.

New things included in this commit are:

- Features support via to new Feature components:

LDAP Integration: ldap_setting - Allows all ldapauth, ldapdata, ldapsync, and ldapgroups global setting to be individually selected when creating a feature. This requires the Strongarm module to be enabled.

LDAP Integration: ldap_servers - Allows you to select individual server entries to add to a feature. Will export server info for all four modules in this project.

NOTE: The current core code links users to the ldap servers are autoincrement sid field. So, there may be some issues if you do a feature revert that causes an existing server to be deleted and a new entry to be created. You can prevent this by making sure that existing server's have machine names that match the ones in the feature module(s). This will ensure a revert updates DB entries.

A future TODO: is to not use the sid but to use the machine_name. But that will require a lot of re-coding/re-testing.

- Machine name support

Added a machine_name column to the ldapauth table. The add / edit server forms now use jQuery behaviors to auto-generate these with an option to edit them.

- The Admin screens now have Import and per server Export options

If ctools is enabled, each server entry in the admin list will have an Export link that produces exportable pHp code. There will also be an import tab at the top to admin screen.

Only import of new servers is allows.

- Centralized CRUD functions added

The new code adds a set of ldapauth_server_* functions for managing ldapauth db records. There are currently used only in the new code but eventually all db calls will be routed thru these. This should eventually allow for "in code only" server definition to be used. E.g., true ctools/features exportables rather than faux-exportable.

SECURITY NOTE:

Please note that the new feature/export functions include any bind passwords used in the configuration (e.g. for lookup or data syncing). These should be treated with appropriate care to prevent security problems.

IMHO - The proper solution to protect these would be to change the code to store the passwords in the DB in an encrypted form based on a global settings key. Then a feature could be created without this key and "unlocked" by setting the key in the admin screens. This allows for two part distribution (e.g. send the code in one e-mail / the pass phrase in an other). Patches welcomed to do this...

cgmonroe’s picture

Title: Time for another 6 beta? » Beta 6.x-1.x Status

Changed title to reflect current usage of this issue.

The -dev version now has full support for the Content profile module. If you have content profile fields mapped to ldap attributes and use r/w syncs, any changes made in content profile fields will change the ldap attributes as well.

Some minor changes include such as:

The Content Profile header in the admin screen only shows if there are content profile nodes configured.
The field list will show fields from all content profile types as [TYPE]->[field]
Improved ldap->node syncing. The code now only writes info out once instead of for every attribute.

Also included a bug fix which will make the bindpw clear option work properly (problem from last update) and added the default port numbers to the port field description.

cgmonroe’s picture

The dev version now has fixes for the following issues:

#1209556: Need to check for existing emails in ldapauth.module

This introduces a new "What to use for testing for existing users" admin option to the ldapsync module.
It also has strict testing to ensure that both ldapauth and ldapsync do not allow the creation of users who's name and e-mail are not unique. I.e., if the name is in use - create denied or if the email is in use - create denied.

#1525752: If "Remove email field from form" is selected, user email addresses can not be synced by ldapdata or changed at all by user_save

The code now uses the user form validate op to prevent e-mail changes (for both the hide and readonly options). This allows code based user saves to change it...i.e. ldapdata / ldapsync.

cgmonroe’s picture

Two-fer today... the final fixes / QA for this commit went smoother than I expected...

#1031426: Return only specific attributes from ldap_read, instead of all attributes (optimization)

This has been implemented based on the D7 LDAP code.

Prior to LDAP searches / retrieveAttributes the ldapauth_attributes_needed function is called with an $op parameter and $sid to get the required attributes. This list is then used to limit the amount of data returned. The $op values are defined in the LDAPAUTH_SYNC_CONTEXT* constants.

The ldapauth_attributes_needed function call the hook_ldap_attributes_needed_alter methods in all modules that have special attribute needs.
---------

The code has also been changed to optimize server information caching by consolidating all the individual static caches into the ldapauth_server_load function. This cuts down the number of queries made to the ldapauth table.

Also finished integrating the machine_name column into the code. The LDAPInterface object now stores this and the various _init functions set it.

Finally, there is now a D7 style ldapauth.api.php "psuedo" code/documentation file.

TODO: For Beta release

Support for Persistent and Unique User ID attributes (PUID). a.k.a. #638798: Associate LDAP & Drupal user accounts via email

LDAPSync bugs

Still working thru bugs..

eiriksm’s picture

Don't mean to clutter up this meta-issue, but great job cgmonroe! I'll try to test these new changes out on the sites I use the module.

cgmonroe’s picture

FileSize
4.65 KB

FYI - Just committed a fairly major update and again requires running Update.php.

Here's the changeLog entries that apply:

- Moved from authmap to checking account settings in a lot of places
- Converted from include and requires to module_load_include
- Fixed ldapdata bug that caused non-LDAP admin accounts to "blank out" profile info if they clicked on "Edit"
- Added hooks for site specific modules to create PUIDs and influence LDAP user id to Drupal User id mapping - See ldapauth.api.php
- Added/Restructured code to use ldapauth_drupal_user_lookup (map ldap ID to Drupal id) and ldapauth_drupal_user_create (create new Drupal user logic).
- Refactored ldapauth so "core" functions (used by other modules) are in includes\ldap.core.inc file.
- (Related to many issues) Added support for Persistent and Unique IDs (PUID) - Requires update.php to be run

The bulk of this was to add support of Persistent and Unique IDs (PUID). (See attached README-PUID.txt document). Please be aware that this commit does not include LDAPSync support for PUIDs. All other modules work with this, but ldapsync is going to have some major work done on it. So I opted to do this major commit first, then start work on ldapsync changes.

But this also includes some code restructuring that added an ldap.core.inc file in the includes directory. This is a move towards having a set of module independent functions that are designed to be used across multiple modules.

No existing functions where moved or changed. Instead, the functions in this new ldap.core.inc file are a combination of the new CRUD functions, functions recently added, and code that has been carved out of the existing functions.

The two functions of most note are:

ldapauth_drupal_user_lookup() - This centralizes all the new LDAP user to Drupal user code needed to support the PUIDs.

ldapauth_drupal_user_create() - This centralizes all the checks required before a user is created. E.g., admin allows it; unique name/e-mail; and the like.

See the README-PUID.txt and ldapauth.api.php for info on a couple of new hooks.

This also include some other minor changes like fixing an ldapdata bug that would problems if the local admin user tried to edit a user's info...

I have spent a lot of time testing the various PUID conditions and many error conditions (e.g. matching uids on two servers, etc) and feel this should be pretty stable. One possible "side effect by design" is that this version may catch LDAP->Drupal user problems that earlier versions let slide. (E.g. non-unique e-mail or user names / LDAP user that match admin id / and some others). In most cases, this probably won't be a problem, but complex sites should be aware of this.

Next Steps for Beta

ldapsync rewrite / bug fixes

The ldapsync rewrite is basically to change the flow from:

Create a big list of ldap users then process to Update/Create new users / Delete orphans

to:

Create smaller list of ldap user info and Update/create new users while doing it. Then use the smaller list to delete orphans.

This will help optimize ldap server connections and reduce the memory needed to sync.

I'm contemplating how to let admins decide to split up searches using wildcards. E.g., I may just have a text field that admins can enter their alphanumeric letters they care about. E.g., abcdefghijklmnopqrstuvwxyz would split the searches into a wild card search starting with each letter. E.g. search1 filter = (uid=a*), search2 filter = (uid=b*). This would fix the 1000 user AD limit for most folks and allow for non-ascii character sets. Not too elegant but seems like it would work/not be too complicated.

cgmonroe’s picture

Just committed a bunch of changes that came out of re-structuring ldapsync to use core functions and fixing a bunch of ldapsync related issues. This set of changes does NOT require update.php to be run for once...lol

Here are the changelog.txt entries that were part of this:

  • by cgmonroe ldapsync now has a User Conflict resolution option to deal with existing drupal users who match LDAP users
  • by cgmonroe added a Sync Existing user only option to ldapsync
  • by cgmonroe ldapsync's optional search filter can now contain multiple filters and optionally have the default filter appended.
  • by cgmonroe Included ldapauth_users table removal in ldapauth uninstall function.
  • Issue #1550504 by superhenne,cgmonroe ldapauth_users table not created on update/installation
  • by cgmonroe Convert ldapgroups from $user to recommended $account for user handling.
  • Issue #805752 by cgmonroe Detecting Groups failing (when user has no groups and only allow from these groups defined).
  • by cgmonroe Restructured ldapgroups to support hook_ldap_user_deny_alter
  • Issues #897222 and #949560 by cgmonroe Added hook_ldap_user_deny_alter so ldapgroups (and other modules) could deny an ldap user access to server before account creation/login.
  • by cgmonroe Fixed features api so it worked with the new ldap.core structure
  • by cgmonroe Restructured ldapsyn to use new core functions and fix multiple issues
  • by cgmonroe Added caching to ldapauth_lookup_user_by_dn

Some highlights/details are:

LDAP Synchronization:

Should use a smaller memory footprint because it now processes the ldap users as they are found rather than creating a big array of all users, then processing them.

Uses the core functions to find and create users. This means that PUID handling, checks for duplicates name/email, and the like are handled with common code.

The sync search filter has been expanded from a single entry to multiple entries with an option to automatically add the ([user_name_attr]=*) filter to them. The description text has also been expanded to talk about the AD limits. These filters will be used for individual searches in each server / base dn area defined in the ldapauth.

The basic intent is that people who are hitting the search limit can create a set of subsearches that will say under the max limit. Worst case would be to have a search for each letter of your local alphabet. E.g. (uid=a*); (uid=b*);...(uid=z*). But you could do things like:

(&(objectClass=person)(homedirectory=\\\\server1*))

or similar to cut things up.

There are a bunch of new options such as only updating existing users and being able to define the User Conflict Resolution process

The summary message/log entry now contains more information about LDAP entries found, users denied, and the like.

API Additions

Added hook_ldap_user_deny_alter that allows modules to deny ldap users access BEFORE any account is created.

ldapgroups, ldapauth, and ldapsync have been restructured to use this. (Now users aren't created then denied).

The ldapauth.api.php documentation file now has some simple example code in most of the functions.

Bugs

The install process now handles Postgres DBs

A bug in the features API which caused function not found during update.php has been fixed.

Some user lookup queries have been made case insensitive

Next Steps to Beta Release

JUST ABOUT THERE!!!!!

There are a couple of simple bugs I remember seeing and may look at the ldapgroups issues to see if anything major is there.

Hopefully there will be a new Beta / 1.0 Release Candidate ready in a couple of weeks. (probably let these dev changes be used by the brave few for a while first).

cgmonroe’s picture

Title: Beta 6.x-1.x Status » 6.x-1.0 Release Candidate 1 Status

I'm changing the title again because IMHO, with the ldapgroups related changes just checked in, the bulk of bug and feature requests have been done. It's had some extensive testing on my servers, so, I think this is more of a v1.0 release candidate than beta. I'm sure once this is released there will be bugs found, but I want to start circling down to a 1.0 release that will probably last until D6 shuts down.

Here's the changelog for what was just committed:

  • Made sure role lookup querys were case insensitive
  • Added hook_ldap_user_groups_alter and hook_ldap_user_roles_alter to ldapgroups and ldapgroups.api.php doc.
  • Allowed multiple Drupal roles to be defined in the group to role mapping area.
  • Added a test user group mappings option to ldapgroups admin that displays groups found, access rights, and roles for a specific user.
  • Converted ldapgroups "account creation" to ldap access rules.
  • Improved ldapgroups admin screen layout and text

Starting from the bottom:

The ldapgroups admin screens now have a lot of extra text and better wording about functionality. Should make it easier for folks to understand/configure.

Access Rules

The "Groups that can create accounts" area is now LDAP Access rules. These are a simple way to allow lots of options in allowing and denying folks via groups. This change is backward compatible.

Here's the help text that can be seen on the admin screen

Rules can be define below that will limit who can access this server. These rules can take two forms.

First, it can just be a list of groups. In this case, the user must be a member of at least one of these groups to be allowed access.

The second form uses rules of the format: "action-type: group-name". Each rule group-name is compared to the user's groups. If the user is a member of the rule's group, the action is applied. The last matching rule determines the user's access rights. Note that all rule sets start with access denied.

The action types are:

ALLOW - Access granted if user is in the group and not denied by rule below it.
ALLOW-X - If the user is in the group, access is granted and rule processing ends
DENY - User is denied if they are in the group unless granted by a rule below this one.
DENY-X - User denied if in group and no further rules are processed.

In addition, there are two "PSEUDO" groups that can be used in rules:

ALL - Matches all authenticated LDAP users
EXISTING - Matches existing users who have been authenticated by LDAP in the past.

Here's an example ruleset to deny all Group1 users but allow existing users and (new) Group2 users to access the server.

DENY-X: cn=Group1,ou=Groups,dc=myorg
ALLOW-X: EXISTING
ALLOW: cn=Group2,ou=Groups,dc=myorg

Note that rule types and groups are case insensitive. However, group names must have the same spacing as returned by the server to match. E.g. if server return cn=X,ou=Groups... then a rule group name, cn=X, ou=Groups... will not match because of the space after the comma.

Test Group Setting Page

I got tired of logging on and off to test things and wrote a page that takes an ldap user and returns the user's groups, server access, and roles. Since this is a great testing/debugging tool, I made it admin proof and added it to the ldap groups admin pages.

LDAP Group to Drupal Role Filtering

There are now three options for the ldapgroups_mapping_filter setting: Automatic roles naming, Use mapping info, and Disabled. This will allow ldapgroups to be used for access control only.

The mapping rules can now have the format of [ldap group]|[drupal role], [drupal_role]... This allows for members of a single ldap group to be placed in multiple drupal roles.

LDAPgroups hooks

There are now two ldapgroups specific hooks: hook_ldap_user_groups_alter and hook_ldap_user_roles_alter. The first allows groups to be added / removed from the ones detected by the admin UI settings. The second allows the user roles to be altered prior to being processed. See the new ldapgroups.api.php file for details.

Misc

Drupal's admin screens treat roles as case-insensitive so all the look up queries are now case-insensitive.

A lot of checking has been added to prevent actions from happening if no group detection rules have been defined.

Next Steps to Release

None - This -dev version should be stable and I will not be making changes. When all the bugs marked "fixed" get closed by the system, I'll generate the 6.x-1.0-RC1 tags/release. WHOOOOOT!

cgmonroe’s picture

Just a quick update for folks following this.... All the issues marked fixed have been closed by the system for 2 weeks inactivity. However, I'm going to get the latest dev at least a full month without updates before creating the new 6.x-1.0-RC1 release. Seems like a good idea to let this age at least as long as a good homebrew beer should. :)

johnbarclay’s picture

You'll get much better testing if you release a beta3 in the short term. If you look at the bottom of the page at https://drupal.org/project/usage/ldap_integration, you'll see very few people are using -dev. Releaseing a beta3 will flush out more testers.

verta’s picture

Agree with #14. I am one of the beta2 users holding off. I can't really keep up with -dev releases.

omaster’s picture

Yes release a beta3 please.

cgmonroe’s picture

FYI - 6.x-1.0-beta3 now created.

The developer pride side of me says this is a RC1 release... but my fear of Murphy and his co-conspirator Finagle made me do this as a beta...lol. Hopefully it will escape their full attention this will soon be turned into V1.0.

cgmonroe’s picture

In order to help debug problems, I did some work on ldaphelp. The -dev version now has an updated version that fixes a couple of bugs (one of which has been around for a while apparently). These are:

#1664384: Fatal error on viewing a user profile
With pointed out the problem with ldaphelp trying to get configuration information from forms and not testing for value elements with objects or arrays. The ldaphelp code now excludes them.

An semi reported problem - LDAP Help used a function named ldaphelp_watchdog() which the watchdog module thought was a hook_watchdog() function. This can cause problems when watchdog tries to call it at inappropriate times. Function/menu now renamed to correct this.

Here's the full change list of fixes and improvements:

by cgmonroe Miscellaneous changes to keep Coder happy.
by cgmonroe Improved labels and text version output format / added some additional suggestions.
by cgmonroe Corrected some logic problems that prevented help suggestions from being displayed.
by cgmonroe Added check to verify that base dns exist on the server
by cgmonroe Fixed incorrect info that was output when server bind fails (e.g. anonymous binds not allowed).
by cgmonroe Updated LDAP Authentication settings info to show all beta3 settings.
by cgmonroe Added phpdocs to various functions to clairify what's generating what.
by cgmonroe LDAPHelp was inadvertently implimenting hook_watchdog as a page. Renamed page function to avoid conflict.
Issue #1664384 by cgmonroe ldaphelp errors fixed by testing for objects and arrays in function.

Note that the wizard part of ldap help has not be updated or tested with beta3. It should work... but there may be some spurious errors.

To help folks debug beta3 without going back to the -dev version, here is a zip file with the two files that contain these changes. Just replace them in the beta3 to get the new ldaphelp code.

cgmonroe’s picture

The -dev release now has code and an admin option to turn on extra logging during user authentication. The check box option is found in the LDAP authentication settings admin screen.

Checking this option will cause between 2 and 5 extra messages to be logged into the watchdog log for each login. These messages are intended to help administrators determine why a person is being denied as well as providing some "waypoints" messages to help in reporting problems. E.g. some sample message output depending on conditions are:,

  • authenticate: User, @user, is a local Drupal user - calling Drupal authentication
  • authenticate: User, @user: Drupal authentication failed, seeing if id/password works with an LDAP user.
  • authenticate: User, @user, was not found in LDAP or authentication failed.
  • authenticate: User, @user, mapped to LDAP DN, @dn.
  • groups_deny: User, @name, denied by group access rules.
  • authenticate: User, @user, denied by another module via hook_ldap_user_deny
  • authenticate: Attempting to creating local Drupal user, @user.
  • authenticate: User, @user, has been authenticated as an LDAP user.
cgmonroe’s picture

In order to help identify "transition problems" due to beta2 users not having up to date dn info (user moved/hasn't logged in since move and the like), I added a "User Info" tab to the -dev version of the ldaphelp module. See:

#1694726: Another user already exists in the system with the same login name

This tab will display a list of all the local users marked with ldap_authentified and show their server machine name and dn. It also does some status checks for PUID status (e.g., Not using PUIDS, PUID set, PUID not set).

The list can also be generated with a "verify" flag that will try to look the user up on LDAP using the local information or PUID (if applicable). This will show any problems with the account ldap_dn not existing, the PUID not found, or the DN associated with the PUID not matching the account dn.

Note that the user listing code currently does not do any "paging" of the users... so this may break or take a long time on sites with a large number of LDAP authenticated users.

kenorb’s picture

The 6.x most likely is not supported anymore. Feel free to re-open for 7.x

kenorb’s picture

Status: Active » Closed (outdated)