Add testing for fetching attribute values #1449
Conversation
SwapnilShahGit
force-pushed
the
add_attribute_testing
branch
from
aa9f134 to
0fcbc72
Compare
|
Thanks for the PR. Unfortunately the SAML treatment of attribute values is a bit more complicated. The base part of the spec is section 2.7.3.1.1 in the SAML core spec, which includes how null and empty values are handled. The TLDR (although I do recommend reading the spec):
The SAML Attribute Profiles (section 8, profile spec) contains even more rules, but as the main focus right now is the Web SSO Profile (and possibly Single Logout) I think we can ignore the profile spec for now. |
SwapnilShahGit
force-pushed
the
add_attribute_testing
branch
from
0fcbc72 to
760c155
Compare
SwapnilShahGit
force-pushed
the
add_attribute_testing
branch
from
3ce93af to
bf3c360
Compare
| { | ||
| //TODO: Test case + error handling for non text content. | ||
| IgnoreChildren(); | ||
|
|
||
| var nullNodeValue = CurrentNode?.Attributes?.GetNamedItem("xsi:nil")?.Value; |
There was a problem hiding this comment.
The support for null is specific to attribute values. The Saml Core Spec section 2.7.3.1.1 states that:
If a SAML attribute includes an empty value, such as the empty string, the corresponding
element MUST be empty (generally this is serialized as ).
This overrides the requirement in Section 1.3.1 that string values in SAML content contain at least one
non-whitespace character.
If a SAML attribute includes a "null" value, the corresponding element MUST be
empty and MUST contain the reserved xsi:nil XML attribute with a value of "true" or "1".
Checking 1.3.1 it states that string values must have at least one non-whitespace character.
I think that the correct validation to do in this method is to check that there is indeed at least one character, possibly with an "bool allowEmpty" param. Then the check for xsi:nil can be moved to ReadElements(XmlTraverser source, SamlAttribute attribute).
And btw, the retrieval of the attribute should use the namespace. The prefix xsi is just a common thing to do, it doesn't have to be exactly that prefix.
source.CurrentNode!.Attributes!["nil", XmlSchema.InstanceNamespace]?.Value
This PR adds test cases for reading attributes in a SAML document. Alongside, there is a null check added in the code such that an attribute will not be populated if no value is specified.