Single Quotes in Attribute Names.
Description
Environment
Activity
Scott Cantor December 17, 2019 at 8:29 PM
Added a check for invalid IDs using the static method when installing transcoding rules, will add further deprecation support after more review.
Rod Widdowson September 7, 2019 at 3:11 PM
For the record, the deprecation warning is in 3.4.5 (and 4.0.0) but the open question about treatment of attributes as they come from dataconnectors still stands.
Rod Widdowson August 2, 2019 at 2:49 PM
Over to Scott to worry about...
Rod Widdowson June 27, 2019 at 10:44 AM
I'm about to get massively derailed so I've committed my work so far which is IdPAttribute and for Attribute Defintions. Note that both do a standard deprecation warning but the idpAttribute also does a per attribute log at DEBUG (and a stack a TRACE) since I imagine that debugging these if they are not from the attribute definition will be tricky.
Documentation and Transcoders are TBD.
Scott Cantor June 26, 2019 at 3:24 PM
My auto-name mapping, oops. Might need to think about that more then, in the context of what would technically be possible to encounter in SAML or other protocols. And/or implement some kind of escaping in the places I did that auto mapping.
This started as a side comment in but it (of course) gets grubbier than that.
The intial idea is to deprecate the use of single quotes in Attribute names. My first question whether that is enough and whether we need to consider double quotes , the percent sign and curly brackets.
I had initially thought that it would probably be safe to make this a constraint but I can certainly see that there may be attributes flowing from data connectors from badly created data stores (example: "User's name") so I think we need to warn in V4.
Regardless I was shocked when I went into the V3 source and discovered that the check wasn't in {{IdPAttribute}} (which is where there is a constraint in V4). This is because V3 is a different world and Attribute names only have any importance once they issue from AttributeResolvers. So the warning is on the attribute definition - which is the correct place. Attribute which occur with spaces outside this world are OK because they can never impact on the flows.
In V4 attributes can go through the complete flow and never see the attribute definition and so the (new) deprecation warning needs to be in the IdPAttribute (as well as attribute definition and potentially transcoders - see below). Further in IdPAttribute we need a debug level message with the attribute name to help diagnose these (since we are much further from the configuration at this point),
So I'll add the test for a single quotes. What does the team think about other special characters? Obviously we cannot disallow '.' and ':' but there are others.
Finally I should note that in V4 the constaint in IdpAttribute is
But in AbstractAttributeDefinition is is
(in other words tabs and other non printing characters).
It feels like, in this new world of definitional-less attributes we need to expand the check in IdPAttribute to match the definition, and also for helping in debugging add a similar test somewhere into the Transcoding stack.