perekipchenko / simplesamlphp

Automatically exported from code.google.com/p/simplesamlphp
Other
0 stars 0 forks source link

ldap:AttributeAddFromLDAP only allows adding one attribute per filter call #616

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
The API of AttributeAddFromLDAP allows for only one attribute to be added per 
filter call. In our IdP, we want to add 20 attributes, so this would make a 
very large config file.

The source code of the filter mentions the possibility to have "[multiple] 
values" [1], but it seems like this was never implemented. The code doesn't 
seem to support it; even though it iterates over multiple "entries", the 
entries are the result of an LDAP search that can only return one attribute.

Because the API of AttributeAddFromLDAP seems to be locked to adding a single 
attribute, I wrote a new filter based on the old one that supports the adding 
of multiple attributes at once. I am not so sure about the name, though; I 
called it MultiAttributeAddFromLDAP, but there exist an authsource called 
"LDAPMulti" which refers to multiple LDAP servers, not multiple LDAP attributes.

MultiAttributeAddFromLDAP has the same API as AttributeFromLDAP, with the 
following differences:
- It does not take attribute.new or search.attribute as config parameters
- It requires the attributes config parameter, which is an array used for 
attribute mapping, with LDAP attribute name as key and SAML attribute name as 
value. The key may be an integer, which means that the same attribute name 
should be used in LDAP and SAML.

The filter is made to fail silently, as I use it in a setting where this filter 
failing is a normal operation, due to not every user logging in is available in 
the LDAP database.

[1] 
https://code.google.com/p/simplesamlphp/source/browse/trunk/modules/ldap/lib/Aut
h/Process/AttributeAddFromLDAP.php?r=3316#125

Original issue reported on code.google.com by yorndej...@gmail.com on 29 Jan 2014 at 11:08

Attachments:

GoogleCodeExporter commented 8 years ago

Original comment by jaim...@gmail.com on 19 Feb 2014 at 12:20

GoogleCodeExporter commented 8 years ago
Hi,

I don't think it's a good idea to create a new filter, when the difference 
between both is simply the amount of attributes you can get. I believe the best 
approach here is to fix the module, in a backwards-compatible way, as 
implemented in r3371. I've kept your syntax for the configuration, but inverted 
$key and $value. It works for me, but please check it out and tell me if you 
find anything not working.

Original comment by jaim...@gmail.com on 20 Feb 2014 at 8:41

GoogleCodeExporter commented 8 years ago
I'm having a problem with duplicate attribute values. I made a patch that does 
not add LDAP attribute values that are already present.

Original comment by yorndej...@gmail.com on 21 Feb 2014 at 12:29

Attachments:

GoogleCodeExporter commented 8 years ago
Hi again,

I've been discussing this with Olav. The thing is, though probably it doesn't 
make sense to have duplicate values for an attribute, the specs don't forbid 
it, and we might be missing some corner use-cases for them. There's also the 
question of what to do when using this filter as sort of an attribute 
authority. If we want this attribute source to have priority over the 
attributes gathered during the authentication step, we should have a different 
policy, then. In the end, we think there could be three different policies:

- add: to add a value blindly. Could derive in duplicates.
- merge: join "old" and "new" attributes, avoiding duplicates.
- replace the "old" values with the "new" ones.

I've implemented this and committed in r3375. As far as I can tell, it works 
fine. I've also updated the documentation accordingly.

Hope this fills your needs.

Original comment by jaim...@gmail.com on 24 Feb 2014 at 12:32