Modifications to LDAP Integration and LDAP Groups functionality
spenserj - July 28, 2009 - 20:50
| Project: | CAS |
| Version: | HEAD |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | spenserj |
| Status: | needs work |
Description
The HEAD release currently has a bug where signing in through CAS (with LDAP as the backend) and then pulling the groups/roles from the LDAP connection fails. This patch should correct the issues, however it may need review to see if it breaks anything else regarding the roles (primarily right before saving the user in cas_login_check() as I commented out the roles line)
Issues that should be fixed with this patch are as follows (with LDAP Groups/Roles being primary):
- Including the LDAP Integration module was failing due to incorrect paths. Using drupal_get_path() instead of a hardcoded path
- Retrieving and initializing LDAP configuration was based on names, instead of the SID like the ldap_integration module requires
- cas_login_check() was not passing enough information to ldapgroups_user_login() resulting in it not retrieving the groups/roles properly. This was fixed by adding the required LDAP fields to the user variable before passing it along
This is my first submitted patch, so any suggestions on how to make the process a bit smoother (for submitting, not coding) would be wonderful! I created the patch file with the recommended "diff -up original.file modified.file > patch.file"
| Attachment | Size |
|---|---|
| spenserj.patch | 3.05 KB |
#1
I have tested this on 6.x-2.x-dev from 2009-Sep-09 and it works but I am getting a Duplicate entry error:
Error
user warning: Duplicate entry 'bgilbert' for key 2 query: INSERT INTO authmap (authname, uid, module) VALUES ('bgilbert', 19, 'cas') in /net/www/fsd-intranet/modules/user/user.module on line 1224.
Status
•Logged in via CAS as bgilbert.
•Logged in via CAS as bgilbert.
#2
Nevermind: Double login was caused by "Initial login landing page:" setting
#3
I had the same problem, and I corrected it by comment «authname_cas» because database has a unique constraint (I'm not sure it's the right thing to do)
«authname_cas» appear in line 267 and 251
I also did some modifications line 297-318 :
/*
** LDAPAuth interfacing - BEGIN
*/
if (variable_get('cas_useldap_groups', '')) {
if ($ldap_config_name = _get_ldap_config_name($user->name)) {
$_ldapauth_ldap=_ldapauth_init($ldap_config_name);
include_once(drupal_get_path('module', 'ldapgroups') . '/ldapgroups.module');
$data['ldap_authentified'] = TRUE;
$data['ldap_config'] = $ldap_config_name;
$ldapUser = _ldapauth_user_lookup($user->name);
$data['memberof'] = $ldapUser['memberof'];
$data['ldap_dn'] = $ldapUser['dn'];
$user = user_save($user, $data);
watchdog('cas', 'Logging of %user_dn', array('%user_dn' => $user->ldap_dn), WATCHDOG_NOTICE);
ldapgroups_user_login($user);
}
}
/*
** LDAPAuth interfacing - END
*/
It permits to have LDAPData interfacing as well, because it saves «ldap_dn» and previous version (with spenserj patch) doesn't (i think).
Best
#4
I've forgotten : I also correct the line 925
_ldapauth_init($ldap_config_name); to $ldapauth_ldap=_ldapauth_init($ldap_config_name);
because $ldapauth_ldap isn't initialised but used in next statement.
Best,
#5
have tried Psy's changes but still getting duplicate errors
Error
user warning: Duplicate entry 'jtyres' for key 2 query: INSERT INTO
authmap (authname, uid, module) VALUES ('jtyres', 30, 'cas') in
/net/www/fsd-intranet/modules/user/user.module on line 1224.
Update:
I can stop the error from occuring by executing the following against the authmap table:
ALTER TABLE authmap DROP INDEX authnameBut this means duplicate records could potentially be created, and is probably a bad way to do it..?
#6
You might try tweaking the "Is Drupal also the CAS user repository?" setting. If you set this to yes, then the auth_map table won't be used. Or alternatively check to make sure you're using the "If Drupal is not the user repository, should cas highjack users with the same name?". These will change how those user accounts get created or don't in the auth_map case.
#7
metzlerd:
I did not have "If Drupal is not the user repository, should cas highjack users with the same name?" checked
I did have "Should Drupal user accounts be automatically created?" checked
I think because I am also using ldap groups functionality I probably needed to have "If Drupal is not the user repository, should cas highjack users with the same name?" checked
#8
Yes, I think that's right, but you could alternatively"Is drupal also the CAS user repository". The only reason not to change that is if you want users to be able to change their username.
#9
automatic assigninment of groups via ldap groups module seems to be broken with the latest release of cas module
6.x-2.x-dev 2009-Nov-11
and ldap_integration
6.x-1.0-beta2 2009-Oct-28
even after trying to reapply these patches manually..
#10
I've been asking for someone who actually has this environment to step up an provide some maintenance and testing. I do not have an LDAP environment to test any of these patches. I'm seriously considering pulling the code for this out of the CAS module, since I can't seem to find people to maintain it.
saying "it appears to be broken" isn't much help in getting it fixed.
Dave
#11
Dave,
Please don't remove it, as it's a requirement for at least one of my sites..
I will provide whatever help I can after my honeymoon, I'll be back Dec 14th.
Even after reverting to previous version of the module this wasn't working and appears to have been an issue with ldap_integration, after removing and re-adding ldap_integrations server and groups settings it appears to work again
Thanks.
#12
Dave, I would agree, do not remove it, I will maintain it as much as I can. I have been AFK over the past month due to work and family issues, however I require this for one of my client's websites. I had stopped checking this for a little while beforehand as well, as there were no replies on it, but considering it had been working for people, I will attempt to maintain it as much as possible.
I had been running this with both hijack and "drupal is the repository" unchecked, and create the account checked.
I encountered the issue with auth-map the whole way along, however I can't remember if I ended up fixing it. I will try to find a solution for it when I start working on it again. Got a bunch of work to do in the mean time though, so it may be a little while.
Spenser
#13
Ok, I've been recruiting others offline as well. I won't remove the code. I've been told the current version in head/6.x dev is broken so I'm really concerned about the stability of this portion of the module. I will continue to advocate refactoring this so that it is done in a way that could be used equally well with LDAP or another account/group provider, as UPortal does. But att minimum I'd like to have consensus that the current dev snapshot works before rolling another release.
Help, help.... as they say.
Dave
#14
Well I just installed the most recent stable version of ldap_integration (6.x-1.0-beta2) and the dev release of CAS (6.x-2.x-dev (2009-Nov-10)) and the main portion of the LDAP code appears to be working properly for me. Users can sign in through CAS, and new accounts are being created, however they are not being granted the correct groups. I will try and re-merge my patch in, as that had fixed the issues for me, and I will give fixing the duplicate entry a shot too.
Refactoring would be a great idea though, I think that ldap_integration was just refactored quite a bit, as I checked a few files and it looked like they organized some code. Perhaps we should split the LDAP portion of the CAS module into a separate file? I will play around with a few things and see what I can do.
Spenser
#15
Okay, so it appears that I have groups working again (at least in my situation) for the current dev release. I think I may try to refactor the code before releasing it though, as it is all pretty messy, however if someone wants it, I can throw up a patch file real quick.
Other than being messy, it should be working in most (if not all) situations now.
Spenser
#16
Please do submit patches from current head or dev release. I'd love to see those reviewed by others who are willing to so that I can get some tested code committed so that I can roll a release. Feel free also to start another issue on the refactoring effort, so we can keep track of those speparately.
#17
Okay, here is another patch against the current dev release then.