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"

CommentFileSizeAuthor
#46 ldapcas.tgz2.43 KBbfroehle
#36 cas-0.2.patch9.02 KBabulte
#35 cas.patch9.35 KBabulte
#34 cas.zip382.44 KBrealityloop
#30 ldap_integration.zip50.81 KBrealityloop
#24 cas_module.txt42.52 KBmetzlerd
#21 casldap.patch7.79 KBgoudal
#17 spenserj.patch2.62 KBAnonymous (not verified)
spenserj.patch3.05 KBAnonymous (not verified)
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

realityloop’s picture

Status: Needs review » Needs work

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.

realityloop’s picture

Status: Needs work » Reviewed & tested by the community

Nevermind: Double login was caused by "Initial login landing page:" setting

Psy-dupe’s picture

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

Psy-dupe’s picture

Status: Reviewed & tested by the community » Needs work

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,

realityloop’s picture

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 authname

But this means duplicate records could potentially be created, and is probably a bad way to do it..?

metzlerd’s picture

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.

realityloop’s picture

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

metzlerd’s picture

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.

realityloop’s picture

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..

metzlerd’s picture

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

realityloop’s picture

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.

Anonymous’s picture

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

metzlerd’s picture

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

Anonymous’s picture

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

Anonymous’s picture

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

metzlerd’s picture

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.

Anonymous’s picture

FileSize
2.62 KB

Okay, here is another patch against the current dev release then.

realityloop’s picture

#17 gives warning: Invalid argument supplied for foreach() in /sites/all/modules/cas/cas.module on line 332.

Using:
LDAP integration 6.x-1.0-beta2 - 2009-Oct-28
CAS 6.x-2.x-dev - 2009-Nov-11

goudal’s picture

Hello,
I have dug a bit in that problem and came to a working solution for me, but not as clean as I would like.
The dirty problem is the following :
the very first lines of the

function ldapgroups_user_login(&$account) {
$authmap = user_get_authmaps($account->name);
if (!isset($authmap['ldapauth'])) {
// This user is not authenticated via lapauth.
return;
}

Show that if the user is not registred as authenticating via ldap, the function returns. So we can't deal with all the ldapauth groups setting.
My dirty solution has been to copy the function in cas.module and than remove the test.

The second problem is that the patch provided by spenserj does make use of the ldap memberof attributes directly, so it avoid complelty the settings done by ldapgroups.

The third problem was to get the list of groups added by ldap in the user object, that needed one more line of patch in the ldap_group_user_login copy.

Than I have changed a bit the code, so that it adds the groups fond by the ldap_group_user_copy clone.

I can provide the patched cas.module, but I can somebody gives me the correct instruction to build the patch ?

Thanks in advance.

realityloop’s picture

Patch creation instructions can be found here: http://drupal.org/patch/create

goudal’s picture

FileSize
7.79 KB

Ok, here is an attempt for the patch... I hope it works against the 6-dev version.

metzlerd’s picture

This patch reverts some code cleanup that was done in head, and also has some stuff about not randomizing passwords, which is a security risk for some folks. It should be rerolled against head at minimum, and it should only contain code changes related to this issue.

Could someone please start thinking about implementing all this LDAP functionality in a separate module. I'm happy to help make sure we have the right hooks in the CAS module. But more and more LDAP_groups code is creeping into the cas module, and it just doesn't belong here. Not everyone who uses cas uses LDAP groups. Why aren't patches being submitted against LDAP groups to make that code work in a way that doesn't require us to reimplement LDAP groups functions in the CAS module?

I really want to see a user_login hook implementation of the ldap_groups functionality here. I've been calling for action on this for a while now, and don't feel like I'm being heard here. I continue to be resistant to committing code I can't test into the cas module.

How are we going to get a way forward here?

Dave

realityloop’s picture

Not to mention that #21 causes WSOD's

metzlerd’s picture

FileSize
42.52 KB

Thanks, realityloop for taking on the separation of this code. I'm attaching a version of the CAS module with all the LDAP code commented out so that you can work on this.

abulte’s picture

Any news on that ? We're having a hard time integrating CAS and ldap_integration too... Thanks !

abulte’s picture

Me and my company are interested in giving a shot at implementing all this LDAP functionality in a separate module. Has any work toward that goal already been done? Can someone provide some guidelines we can follow ?

Thanks in advance.

realityloop’s picture

I've done a small amount, but would not be at all offended if you have more time.. :)

I can upload what I've done in a few days, it's mostly settings screens.

dgeo’s picture

I'd be interested too for:
- getting actual code (we also have an emergency to get it working)
- helping dev. as I can, testing, and so on...

My thoughts about that:
- ldap* SHOULD not not appear in cas module (as metzlerd said in #10)
- ldapdata and ldapgroup SHOULD be able to work out of the box without ldapauth authentication. (don't know how, but maybe some configuration like "get LDAP informations for users identified with XXX module", where XXX would be 'cas' in our case) - hook_user seems to be done for that don't you think ?
- while this is not the case, I'm ready for testing/coding any patch or module to get it work correctly with ldapdata AND ldapgroup

abulte’s picture

We started work on this feature.

Our approach is :
- take the LDAP code away from CAS module (it seems we all agree on that) and expose hooks in CAS module where needed
- create a new cas_ldap module which would depend on cas and ldap_integration
- basically this module would : call the hooks defined in CAS and call the functions in ldap_integration to do the LDAP connecting and fetching stuff

We ran into a first problem today : ldap_integration module can not do anything with a user (for example, create it in Drupal DB) if you don't provide it with the user password. While it seems fair when you come from a login box (login & pass provided by the user), it's not efficient when you're in a CAS environnement : password check is done by CAS and the app is not supposed to have it. So we need to modify some LDAP functions to work without a password (SSO mode).
The first step will be to this "dirty" : duplicate ldap_integration code we need in cas_ldap. As a matter of fact, it seems difficult to get this functionality mainstream in ldap_integration.

realityloop’s picture

FileSize
50.81 KB

Here is the code at where I got to, far from complete but may help.. I did contact the ldap integration developer and he was open to talking, so I'm sure you can get help adding what is needed at that end.

The biggest problem I see at the moment is that to get it to work you need to make ALTER the authmap table so that there can be a cas and ldap entry per user, this isn't ideal as authmap should only have a single entry per user based on the default table setup, I was going to create a new authmap type casldap and seek to get both modules aware of it.

realityloop’s picture

From a discussion with ldap auth developer just now:

Miglius
ok, so basically what you need is to make ldapgroups work without the ldapauth. ldapauth sets $_SESSION['ldap_login']['dn'] and
$_SESSION['ldap_login']['pass'] on the user authentication and those credentials are used to pull the groups data from the LDAP. If you would set those variables in your module, maybe the ldapgroups just start working?

Realityloop
will ldapauth work without it's entry in authmap table?

Miglius
not sure, never tried that. one would need to test it
I doubt if ldapgroups cares about authmap entry
it just ldapauth which does

Realityloop
to have ldap groups work ldapauth needs to be enabled tho
doesn't it?

Miglius
yes, but it has a mixed mode:
Mixed mode. The LDAP authentication is performed only if Drupal authentication fails
so maybe your authentication would go before the ldapauth
also I could add a checkbox in the ldapauth module which would be something like:
Enable ldap authentication
so you could have ldapauth module installed but disabled
a lot of options there

Realityloop
that would probably be very handy..
I'll see how things progress and get back to you if we think it will help..

Miglius
deal

metzlerd’s picture

I don't like the idea of mucking with the authmap. It breaks all the user accounts for all prior installations and there's no real benefit. I'm still really unclear as to why this cannot be refactored in a way that makes it so you don't need ldap_auth. The code that you've posted here is the entire ldap project? What am I supposed to do with this?

realityloop’s picture

metzlerd, the code was attached for dgeo and abulte, it's not at a stage to incorporate yet, otherwise I'd have submitted as a patch.

Opps, I uploaded the wrong module *slaps forehead*

realityloop’s picture

FileSize
382.44 KB

The right archive attached this time!

abulte’s picture

Status: Needs work » Needs review
FileSize
9.35 KB

We now have a working solution.

It is composed of :
- a patch to the CAS module (attached) that removes all LDAP related code and define a hook userdata
- a new ldap_cas module that implements userdata hook and calls ldap_auth functions

You can find the last version of the module here (I'm applying for a CVS account) :
http://drupal.org/node/835412#comment-3127544

abulte’s picture

FileSize
9.02 KB

A new version of the patch (bug corrected)

metzlerd’s picture

First, Thanks for seeing this through.

I'll review this in more detail, but I thought I'd give you my first pass immediately.

I'm concerned about the removal of the user login hook from the external auth case. There are other modules that rely on this for detecting the right spot where a user was authenticated. (e.g. Login destination and persistent login) . What was the rationale for removing that?

You appear to have disabled the ability for cas users to be created automatically for the authmap case. That will break a lot of sites out there. (including mine). A primary reason for setting cas up this way is to allow users to change their login name. There are reasons beyond LDAP that are used for this. At minimum we'll need to implement a userdata hook in the cas module for saving. Could we discuss the approach here?

Could you outline what the the new userdata hook is supposed to do? Should be commented in the code for other module developers.

realityloop’s picture

I'd have thought it would also make sense to call the other module cas_ldap and include as part of CAS, seeing as it has no use on it's own?

metzlerd’s picture

Either way works for me. That decision can be made after the CVS account is processed/approved. I'd love to see a comaintainer for cas, and I'd be willing to give commit rights to another to get this project done right.

amallogia’s picture

Hi everyone,

First thanks for your feedback

As abulte said we wrote this module for internal needs and didn't take care of breaking functionalities (we haven't so much time and needed to have something working)

"I'm concerned about the removal of the user login hook from the external auth case. There are other modules that rely on this for detecting the right spot where a user was authenticated. (e.g. Login destination and persistent login) . What was the rationale for removing that?"
My first decision was to implement a specific user login hook in cas_ldap doing the user's authentication/registration process I don't remember why but it didn't work (maybe because authentication was done by user module before cas_ldap).
That's why I decided to implement a specific hook dedicated to authentication/registration process

"You appear to have disabled the ability for cas users to be created automatically for the authmap case."
There is a conflict between CAS and LDAP users (Primary key violation on user_set_authmap) so I decided to remove all the user's registration process from the CAS module but as you said we lose the cas automatic registration.

"Could you outline what the the new userdata hook is supposed to do? Should be commented in the code for other module developers."
the userdata hook is dedicated to authentication/registration.
You have to pass it basic information returning from CAS & drupal database (login/password) and then doing the Drupal registration (user doesn't exist in Drupal) or the Drupal authentication (user already exists in Drupal) process.
Then it "returns" the global $user variable which is available for the cas module.

We agree with you, we should discuss the best approach to redesign this module as a cas module :
- the userdata hook was a temporary solution which works well for us, but you right we should step back to user login hook and see what happened exactely.
- the ldap_cas module needs column authmap.module to be set at 'ldapauth' in order to recognize a ldap user. A cas entry in authmap leads to 'primary key violation' when new user coming from ldap (just have a look to user_set_authmap api).

metzlerd’s picture

Title: Modifications to LDAP Integration and LDAP Groups functionality » How about this for an alternative approach.

What about this for an option:

For setting the authmap realm:
We add a new parameter (or change to an array/object) to the cas_auth_tranform hook that cas already provides in hook_auth_transform. This would allow the cas_ldap to change the value of both the username and the authmap key that cas uses for the authmap realm. If we used an array, we could have the modules alter $cas_role information as well via the same hook.

Actually the more I think about this, we should merge the auth_filter hook and the auth_transform hook. There's really no need to separate them. Modules could alter information in them and then transform them. The allow_access variable could be just another thing that gets transformed?

Maybe we just call the new hook hook_cas_login?

This should dovetail nicely with the need to add attribute support for the cas module. What do you think? Workable?

For altering user_information:

I think the easiest way is we just use information from the cas_login hook, and let the cas module decide what and whether to save the user data. If there was $cas[role] data provided information, email addresses, user->names, then these would be updated. If the login hook uses profile data, then profile data gets saved?

What do you think about these approaches? If we can agree then I'll set up a branch to start working on these changes.

verta’s picture

(subscribing)

metzlerd’s picture

Title: How about this for an alternative approach. » Modifications to LDAP Integration and LDAP Groups functionality
Status: Needs review » Needs work

Haven't heard back form this but am taking the jump. Going to alter head so that the LDAP code is removed. We'll refactor in the 6.x so that CAS attribute support and LDAP integration are handled in the same sane way.

Note that the patched prior to this are not going into head because they break existing cas functionality.

realityloop’s picture

@metzlerd I've sent you an email about tying to catch up for a Skype meeting to discuss this, with more PHP experience and time I'm willing to tackle it again.

douradinhos’s picture

Cant sync the ldap mail attribute to new comers from CAS.

LDAP module works fine without CAS.

This thread is messed up, what patch's do I need to fix this?

bfroehle’s picture

FileSize
2.43 KB

I've attached a helper module I've been using for CAS + LDAP Data. Not the fanciest, but it got the job done for me. It didn't require any source changes to cas.module.

kolafson’s picture

#46 works very well, and uploaded on Christmas day no less ! Many thanks.

bfroehle’s picture

If you are trying to get LDAP attribute suport for 6.x-3.x or 7.x-1.x, there's a preliminary CAS helper development module has been written which offers basic LDAP abilities.

bfroehle’s picture

Title: Modifications to LDAP Integration and LDAP Groups functionality » Provide LDAP attributes/groups integration
Category: bug » feature
Status: Needs work » Active

I'm resetting this back to active, since there isn't really any current code here that is up for discussion.

bfroehle’s picture

I just marked #1075360: Create a CAS / LDAP integration module as a duplicate of this issue. (There are some nice screenshots in #1075360-1: Create a CAS / LDAP integration module if you want to see what CAS + LDAP integration might look like).

bfroehle’s picture

Project: CAS » CAS Attributes
Version: master » 7.x-1.x-dev
Assigned: » Unassigned

Moving this to the NEW CAS Attributes module.

ikeigenwijs’s picture

subscribe

bfroehle’s picture

Status: Active » Fixed

Please try out this module (CAS Attributes), which should be functional for both 6.x-3.x and 7.x-1.x.

(There is no LDAP Group support yet. If you have the ability to work on contributing that, please open a new issue).

Status: Fixed » Closed (fixed)

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