I use ldap token attributes to assign value for user fields, such as username, mail, and a signature.
That works correctly and data are in my user profile.
In these fields, I put something like [cas:ldap:mail].

My question is what written in the role mapping part to assign role depends of ldap attribute and not cas attribute ?
I checked the roles to manage but no role is mapped when I use one of expression below :

[cas:ldap:edupersonaffiliation]
cas:ldap:edupersonaffiliation
ldap:edupersonaffiliation
edupersonaffiliation

What is the syntax to use for this case ?

Thanks,

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

lucuhb’s picture

Category: Support request » Bug report
FileSize
798 bytes

HI,

I think there is a bug in the module for roles mapping with ldap token : in the part "manage role" of the file cas_attributes.module, there is only a call to find attributes from cas, but not from ldap. If change the line

 $user_attributes = cas_phpcas_attributes($data['cas']);

to

    $user_cas_attributes = cas_phpcas_attributes($data['cas']);
    $user_ldap_attributes = cas_ldap_attributes($data['cas']);
    $user_attributes = array_merge($user_cas_attributes, $user_ldap_attributes);

and add the edupersonaffiliation (for exemple) in the cas attribute setting in attributes area of role mapping, roles are correctly assigned to the user.

Here is a patch for this problem. if it is OK for mainteners, is it possible to add this to the module ?

lucuhb’s picture

FileSize
714 bytes

Better if we test that the CAS LDAP Tokens module is enabled ! Sorry for this mistake.

bkosborne’s picture

Status: Active » Needs review

The last submitted patch, 1: cas_attributes.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 2: cas_attributes.patch, failed testing.

The last submitted patch, 1: cas_attributes.patch, failed testing.

The last submitted patch, 2: cas_attributes.patch, failed testing.

bkosborne’s picture

It's not clear if this was intentionally left out or not. The wording does seem to indicate that role mapping only works with CAS Attributes, since LDAP Attributes are not specified.

An issue I see w/ the patch is that merging CAS & LDAP attributes is mixing namespaces and there may be conflicts. As the patch is written, LDAP attributes would "win" and the CAS attribute with the same name would be ignored.

It's sort of an edge case since I don't think many users would use both CAS Attributes AND LDAP Attributes, but if someone were to use it like that I could see it being a very annoying bug to track down.

I suggest that there be a radio button for role mapping when the CAS LDAP module is enabled that asks the user if they want to map against CAS attributes or LDAP attributes, or both (with the risk of name collisions).

lucuhb’s picture

Yes, you are right, there would be a conflict between CAS attributes and LDAP attributes.

your proposal to choose between these two types of attributes can perhaps work around this problem.

bkosborne’s picture

Status: Needs work » Needs review
FileSize
2.34 KB

Here's a patch that asks the user to use CAS or LDAP attributes.

When I was writing, I was thinking it would have been better to have that mapping textarea support tokens (not sure why it doesn't). Seems like proper token support there would address these issues...

Status: Needs review » Needs work

The last submitted patch, 10: cas_ldap_role_mapping-2190967.patch, failed testing.

bkosborne’s picture

Version: 7.x-1.0-rc3 » 7.x-1.x-dev

Patch was made against dev

bkosborne’s picture

Tests are failing with "Fatal error: Class 'CasTestHelper' not found in /var/lib/drupaltestbot/sites/default/files/checkout/sites/default/modules/cas_attributes/cas_attributes.test on line 11]"

I'm new to Drupal testing, so I'm not sure on this one. The cas.test file has that class definition, and I thought the CAS module would have been installed and files within cas.info included automatically?

bkosborne’s picture

Missed a module_exists call

lucuhb’s picture

I just have tested your patch on 7.x-1.x branch. The problem is that only roles for the last attribute listed on "Attributes" area are assign on the person.

bkosborne’s picture

Can you double check that? That's not the behavior I'm seeing as I'm testing.

I have two attributes listed, separated with a newline. I also have two roles checked for "roles to manage". I have the radio button selected "LDAP" for "cas or LDAP attributes?".

When I log a user in via CAS, both managed roles are assigned to the user, because each of the two attributes matches up with the role name.

lucuhb’s picture

Of course, I can check another time. Having multivalued attributes in the ldap could be the cause of the problem?

For exemple, I have a person with these ldap attibutes values :

  • edupersonaffiliation => 2 values : faculty and member
  • affectation => 2 values : 904E4 and 904EB

When I checked "Ldap" for the "CAS or LDAP attributes? " radio button and write in the "attributes" area :

edupersonaffiliation
affectation

, only the roles 904E4 and 904EB are affected to the person.
If I put , in the "attributes" area :

affectation
edupersonaffiliation

, only roles faculty and member are affected to the person.

Of course these several roles are checked in the "Roles to manage" list.

bkosborne’s picture

I don't think this module supports that. Can you find documentation that it does? Looking at the existing code (that the patch doesn't modify) it's not performing a substring or regex search against the attribute value for the role name, it's always just checking if the entire attribute value is equal to the role name.

bkosborne’s picture

To be clear, you have the following roles on your site?

  1. faculty
  2. member
  3. 904E4
  4. 904EB
lucuhb’s picture

FileSize
2.66 KB

Yes, I have these roles on my site.

I found the problem : I have a "\r" before the "\n" , so when the explode("\n", $cas_attributes['roles']['mapping']); was called, the first attribute in the "$attributes_to_check" variable was "affectation\r" and not "affectation".

Here is another patch in which the split is made on "\n" and "\r".

And now all is OK :all the roles are affected on the person.

bkosborne’s picture

Status: Needs work » Reviewed & tested by the community

OK, that is unrelated to this patch, and should be a separate issue.

Also the preg_split should probably be preg_split("/\r\n|\n|\r/", $cas_attributes['roles']['mapping']);

The last submitted patch, 14: cas_ldap_role_mapping-2190967-14.patch, failed testing.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 20: cas_attributes_1.patch, failed testing.

snufkin’s picture

Status: Needs work » Needs review
FileSize
2.75 KB

The patch is not formatted correctly, here is a reroll, so the bot can start testing it..

Status: Needs review » Needs work

The last submitted patch, 24: 2190967-cas_attributes_from_ldap.patch, failed testing.

snufkin’s picture

Status: Needs work » Needs review
FileSize
3.62 KB

The role matching seems to be failing, I am not sure why. Changelog for this patch:

- I've separated the field and role mapping functions since they perform different tasks, this improves code clarity.
- Since the LDAP or CAS role mapping is a new feature, older modules will not have the variables set to use CAS, therefore this has to be the fallback option in the attribute retrieval phase, not a strict condition. The conditional now checks for LDAP uses those attributes and falls back to CAS otherwise.
- Fixed the $cas_attributes variable, I am not sure what it was doing there this code is not present in the dev code at this time, I assume this was meant to be the $mapping.

Status: Needs review » Needs work

The last submitted patch, 26: 2190967-cas_attributes_from_ldap.patch, failed testing.

snufkin’s picture

This fails according to the bot, but in reality the fails are fixed (meaning roles are mapped). All exceptions come from PHP notices because of phpCAS using deprecated settings:

curl_setopt(): CURLOPT_SSL_VERIFYHOST with value 1 is deprecated and will be removed as of libcurl 7.28.1. It is recommended to use value 2 instead

dflitner’s picture

#26 works for me, based on my limited testing so far. The patch applied with no errors that I saw and my LDAP role is now mapping successfully.

I could wish that it was case insensitive but that is entirely my own preference.

Thank you!

yalet’s picture

This fails according to the bot, but in reality the fails are fixed (meaning roles are mapped). All exceptions come from PHP notices because of phpCAS using deprecated settings:

curl_setopt(): CURLOPT_SSL_VERIFYHOST with value 1 is deprecated and will be removed as of libcurl 7.28.1. It is recommended to use value 2 instead

I will open a separate issue for this; I found the same behavior in the CAS module a while back. Requiring the tests to download phpCAS 1.3.2 instead of 1.3.1 fixed those issues. I will get that fixed, so that we can trust testbot output again, and then I will cycle back to review this patch.

Status: Needs work » Needs review

  • yalet committed c1e2fb9 on 7.x-1.x authored by snufkin
    Issue #2190967 by lucuhb, bkosborne, snufkin: Mapping roles from ldap...
yalet’s picture

Status: Needs review » Fixed

Committed. I also committed a change for the regex (based on bkosborne's suggestion) to match on all of \r, \n, and \r\n.

Status: Fixed » Closed (fixed)

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