Modify AttributeAlter to allow removal of values

[GoogleCodeExporter]

I want to remove some attributes from the IdPs assertion before it is sent to 
the SP. I know I can do this with the core:AttributeLimit module, but it uses a 
whitelist. I want to use a blacklist.

In order to do this, I patched core:AttributeAlter, so that it can not only 
change attributes, but also remove this. I did this by making the module not 
only accept '%replace' in the configuration, but also '%remove'. I have 
attached the patch.

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

Attachments:

• AttributeAlter.php.diff

[GoogleCodeExporter]

Original comment by jaim...@gmail.com on 1 Feb 2014 at 5:01

• Changed title: Modify AttributeAlter to allow removal of values
• Changed state: Started
• Added labels: Type-Enhancement

[GoogleCodeExporter]

Hi!

This is now implemented in r3350. I've made the changes based on your patch, 
but without applying it directly, since there was some other bugs in the filter.

Please, check that it works for you (it should) and reopen the issue if you 
find anything wrong.

Thanks for the patch!

Original comment by jaim...@gmail.com on 2 Feb 2014 at 6:57

• Changed state: Fixed

[GoogleCodeExporter]

Thank you! The modified filter works fine in our setup.

Original comment by yorndej...@gmail.com on 2 Feb 2014 at 7:49

[GoogleCodeExporter]

I found an error, the "ensure the subject exists" check. In our workflow, it is 
possible that the attribute does not exist at all, because there are no values. 
The filter should not throw an exception in this case.

Maybe it would be an option to not execute this check if the filter is in 
remove mode ($this->remove is true)? The check makes sense when changing a 
value but not so much when removing a value.

I cannot re-open this issue myself.

Original comment by yorndej...@gmail.com on 3 Feb 2014 at 10:21

[GoogleCodeExporter]

Hi!

You are right, it should stop gracefully instead. Besides, the previous 
behaviour was as you describe. Regarding the %remove flag, I don't think it 
both things are related. What you describe should be the norm, instead of 
depend on the configuration.

Fixed in r3352 ;-)

Original comment by jaim...@gmail.com on 3 Feb 2014 at 2:12

[GoogleCodeExporter]

When the %remove filter removes all values, an empty saml:Attribute remains in 
the assertion. This is a problem in ruby-saml, which (wrongly) assumes that all 
attributes have exactly one value.

Original comment by yorndej...@gmail.com on 14 Feb 2014 at 3:46

[GoogleCodeExporter]

That's strange, I would swear that was the behaviour. In fact the documentation 
explicitly says so: "If no other values exist, the attribute will be removed 
completely." Must have removed it while testing different ways to code this...

Anyway, to me both behaviours are legit. SAML attributes with no attribute 
values are legitimate and can mean different things, but at the same time it 
sounds logical that the attribute gets removed if no values are left. Both are 
fine to me as long as the code behaves coherently with the documentation.

Check r3366.

Original comment by jaim...@gmail.com on 17 Feb 2014 at 5:10