Patch for 'ldapauth' module with fixes/improvements

Post modules, themes or any other code you want to share with the community.

Moderator: Developers

rossigee
Posts: 11
Joined: Mon Jul 26, 2010 4:58 am

Patch for 'ldapauth' module with fixes/improvements

Postby rossigee » Tue Aug 03, 2010 9:00 am

Attached a patch that fixes some behavioural problems with the 'ldapauth' module.

Firstly, our LDAP server contains only scant information on each user. Currently, the only data we trust in it is the login, passwords and group memberships. Therefore, we have disabled the behaviour whereby on a successful login, the LDAP details are used to update the 'go_users' record. This was imperative, as we have set up all the 'go_users' correctly, and rely on them to be correct for our custom modules to work as expected. The old behaviour may still be desirable in the majority of cases, so I would recommend making this condition configurable if this patch is to be considered for upstream.

So, given that it no longer updates the 'go_users' table, the ldapauth class has to log the user in during the 'before_login' hook (with $GO_SECURITY->logged_in($user_id), if they have successfully authenticated against the LDAP server. This is really all we were hoping and expecting the ldapauth module to do. To complete this, it was necessary to update 'auth' to only authenticate against the 'go_users' table if none of the 'before_login' hooks had agreed to log the user in already.

And, as 'ldapauth' and 'imapauth' should be siblings (i.e. no direct dependency), I figured that it would be better for it ldapauth not to inherit imapauth, but rather to instantiate it in order to make use of it's 'get_domain_config' function *only if* the 'imapauth' module exists and is enabled.

Hope this helps. Please feel free to include upstream, as-is, in part or modified, as you see fit.

Regards,

--
Ross
Attachments
go-ldapauth-fixes.patch.gz
(1.54 KiB) Downloaded 396 times
rossigee
Posts: 11
Joined: Mon Jul 26, 2010 4:58 am

Re: Patch for 'ldapauth' module with fixes/improvements

Postby rossigee » Tue Aug 17, 2010 8:10 am

Another patch, this one fixes a minor regression against the latest release - fixing the 'ldap_connect' call in the case that the host specified is an LDAP URL, and no port number is specified (because it is implied in the URL).

--
Ross
Attachments
ldap_fix.patch.gz
Patch for broken 'ldap_connect' call
(364 Bytes) Downloaded 387 times
mschering
Site Admin
Site Admin
Posts: 8154
Joined: Tue Apr 20, 2004 1:06 pm
Location: The Netherlands - Den Bosch
Contact:

Re: Patch for 'ldapauth' module with fixes/improvements

Postby mschering » Thu Aug 19, 2010 7:16 am

Thanks, I've implemented it.
Best regards,

Merijn Schering
Intermesh
rossigee
Posts: 11
Joined: Mon Jul 26, 2010 4:58 am

Re: Patch for 'ldapauth' module with fixes/improvements

Postby rossigee » Fri Oct 01, 2010 7:29 am

Great! Thanks.

I've got a follow-up patch that takes care of the loose-ends from the first patch. In particular, we needed to connect to the LDAP connection using an LDAP URL (specifying host, port and tls options seperately too cumbersome, URL's being the de-facto method these days), and bind using a DN that is not part of the 'people' OU.

This time, I've prepared and tested it against a fresh GO trunk checkout to make integration easier. In summary...


Main Changes
------------

* Ability to connect to SSL-based LDAP servers (add URL-based config parameters)
* Backwards-compatible for instances still configured using old LDAP config parameters.
* Ability to bind to the LDAP server using any DN (replace user/pass config with 'binddn' and 'bindpw').
* Backwards-compatible with old config params.
* Refactor main functionality of 'before_login' hook into smaller functions, for simplicity/readability. Add comments.
* Simplify 'add', 'modify' and 'delete' methods on LDAP class. Duplicated code unnecessary, as caller should really have connected first anyway.
* Add a simple 'get_connection()' method, allowing calling modules (e.g. LDAP address book or directory management modules) to get an LDAP handle without duplicating code between them.
* Create 'ldap_create_groupoffice_account' configuration parameter, so GO accounts are only created from successful LDAP authentications if explicitly configured to be.
* Create 'ldap_disable_password_update' configuration parameter, to allow the user to disable the (often undesirable) behaviour of LDAP passwords getting copied into GroupOffice's database. IMHO, it should probably really be disabled by default and configurable explicitly (?).
* Create 'ldap_create_mailboxes_for_email_domain' to explicitly enable the attempt to create mailboxes in conjunction with the imapauth module.

Also attached, updated source for the following wiki page to cover new configuration parameters.

http://www.group-office.com/wiki/IMAP_o ... entication

Please accept these patches with our best regards.

--
Ross
Agent Design Ltd
Attachments
ldapauth.patch.gz
Patch to trunk for affected files
(5.68 KiB) Downloaded 436 times
wiki-update.txt
Update to wiki page
(6.4 KiB) Downloaded 463 times
mschering
Site Admin
Site Admin
Posts: 8154
Joined: Tue Apr 20, 2004 1:06 pm
Location: The Netherlands - Den Bosch
Contact:

Re: Patch for 'ldapauth' module with fixes/improvements

Postby mschering » Thu Oct 07, 2010 1:48 pm

I don't want to implement this in 3.5 because I'm afraid it might break some customers configuration. I'm going to create a 3.6 branch and beta release tomorrow. Let's apply it there and do some extensive testing.
Best regards,



Merijn Schering

Intermesh
rossigee
Posts: 11
Joined: Mon Jul 26, 2010 4:58 am

Re: Patch for 'ldapauth' module with fixes/improvements

Postby rossigee » Mon Oct 11, 2010 3:22 am

Updated patch which fixes a bug where 'ldap://localhost/' is wrongly used instead of the configured 'ldap_url'.
Attachments
ldapauth.patch.gz
(5.62 KiB) Downloaded 389 times
mschering
Site Admin
Site Admin
Posts: 8154
Joined: Tue Apr 20, 2004 1:06 pm
Location: The Netherlands - Den Bosch
Contact:

Re: Patch for 'ldapauth' module with fixes/improvements

Postby mschering » Fri Oct 15, 2010 7:33 am

Can you create a patch for the current trunk version?
Best regards,



Merijn Schering

Intermesh

Return to “Contributed development”

Who is online

Users browsing this forum: No registered users and 1 guest

cron