AttributeValueBase: include xs namespace definition in set_type

[jgehrcke]

The following response generated by pysaml2 does not pass schema validation (against saml-schema-protocol-2.0.xsd):

<?xml version='1.0' encoding='UTF-8'?>
<ns0:Response xmlns:ns0="urn:oasis:names:tc:SAML:2.0:protocol" xmlns:ns1="urn:oasis:names:tc:SAML:2.0:assertion" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" Destination="http://127.0.0.1:8101/foo/bar/auth/saml/providers/testprovider-pysaml2/acs-callback" ID="id-t0N4PVt5M85l8Q8mL" InResponseTo="FOOBAR_2d4bd2cf5bc9fe4adddf5093ff3f5efce0a294ab" IssueInstant="2016-05-11T12:07:01Z" Version="2.0"><ns1:Issuer Format="urn:oasis:names:tc:SAML:2.0:nameid-format:entity">https://127.0.0.1:8088/idp.xml</ns1:Issuer><ns0:Status><ns0:StatusCode Value="urn:oasis:names:tc:SAML:2.0:status:Success" /></ns0:Status><ns1:Assertion ID="id-U2zvMe8m9pu70etTa" IssueInstant="2016-05-11T12:07:01Z" Version="2.0"><ns1:Issuer Format="urn:oasis:names:tc:SAML:2.0:nameid-format:entity">https://127.0.0.1:8088/idp.xml</ns1:Issuer><ns1:Subject><ns1:NameID Format="urn:oasis:names:tc:SAML:2.0:nameid-format:transient" NameQualifier="https://127.0.0.1:8088/idp.xml" SPNameQualifier="http://127.0.0.1:8101/foo/bar/auth/saml/providers/testprovider-pysaml2/sp-metadata">f2df8e56ce61f5d6f0a8b9ff85a60db97b78917e4c4d8c692811d9fad0f5125a</ns1:NameID><ns1:SubjectConfirmation Method="urn:oasis:names:tc:SAML:2.0:cm:bearer"><ns1:SubjectConfirmationData InResponseTo="FOOBAR_2d4bd2cf5bc9fe4adddf5093ff3f5efce0a294ab" NotOnOrAfter="2016-05-11T12:22:01Z" Recipient="http://127.0.0.1:8101/foo/bar/auth/saml/providers/testprovider-pysaml2/acs-callback" /></ns1:SubjectConfirmation></ns1:Subject><ns1:Conditions NotBefore="2016-05-11T12:07:01Z" NotOnOrAfter="2016-05-11T12:22:01Z"><ns1:AudienceRestriction><ns1:Audience>http://127.0.0.1:8101/foo/bar/auth/saml/providers/testprovider-pysaml2/sp-metadata</ns1:Audience></ns1:AudienceRestriction></ns1:Conditions><ns1:AuthnStatement AuthnInstant="2016-05-11T12:07:01Z" SessionIndex="id-NrA0bJJCqJnRZpz8I"><ns1:AuthnContext><ns1:AuthnContextClassRef>urn:oasis:names:tc:SAML:2.0:ac:classes:Password</ns1:AuthnContextClassRef><ns1:AuthenticatingAuthority>https://127.0.0.1:8088</ns1:AuthenticatingAuthority></ns1:AuthnContext></ns1:AuthnStatement><ns1:AttributeStatement><ns1:Attribute FriendlyName="givenName" Name="urn:oid:2.5.4.42" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri"><ns1:AttributeValue xsi:type="xs:string">Roland</ns1:AttributeValue></ns1:Attribute><ns1:Attribute FriendlyName="mail" Name="urn:oid:0.9.2342.19200300.100.1.3" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri"><ns1:AttributeValue xsi:type="xs:string">[email protected]</ns1:AttributeValue></ns1:Attribute><ns1:Attribute FriendlyName="eduPersonTargetedID" Name="urn:oid:1.3.6.1.4.1.5923.1.1.1.10" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri"><ns1:AttributeValue xsi:type="xs:string">one!for!all</ns1:AttributeValue></ns1:Attribute></ns1:AttributeStatement></ns1:Assertion></ns0:Response>

Errors:

Element '{urn:oasis:names:tc:SAML:2.0:assertion}AttributeValue', attribute '{http://www.w3.org/2001/XMLSchema-instance}type': The QName value 'xs:string' has no corresponding namespace declaration in scope.
Element '{urn:oasis:names:tc:SAML:2.0:assertion}AttributeValue', attribute '{http://www.w3.org/2001/XMLSchema-instance}type': The QName value 'xs:string' has no corresponding namespace declaration in scope.
Element '{urn:oasis:names:tc:SAML:2.0:assertion}AttributeValue', attribute '{http://www.w3.org/2001/XMLSchema-instance}type': The QName value 'xs:string' has no corresponding namespace declaration in scope.

http://docs.oasis-open.org/security/saml/v2.0/saml-core-2.0-os.pdf says that using xsi:type is optional:

If the data content of an element is of an XML Schema simple type (such as
xs:integer or xs:string), the datatype MAY be declared explicitly by means of an xsi:type declaration
in the element

Now that we use it, we must also declare the xs namespace. Indeed, in test data used by pysaml2 we see that declaration (as an attribute in the AttributeValue tag):

    <saml2:Attribute FriendlyName="uid" Name="urn:oid:0.9.2342.19200300.100.1.1" 
        NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri">
        <saml2:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema" 
            xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" 
            xsi:type="xs:string">demouser</saml2:AttributeValue>
    </saml2:Attribute>

Ref: https://github.com/rohe/pysaml2/blob/master/tests/attribute_statement_data.py#L81

In the SimpleSamlPHP auth response we see that the namespace is declared at the document root:

<samlp:Response xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol"
    xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion" xmlns:xs="http://www.w3.org/2001/XMLSchema"

Ref: https://github.com/rohe/pysaml2/blob/master/tests/simplesamlphp_authnresponse.xml#L3

My intial goal was to also get the declaration added to the document root if required (not just always). However, my deepdive into pysaml2's XML foo was not deep enough.

Still, I could come up with a (hopefully clean and nice) solution to conditionally add the xs namespace declaration to AttributeValue tags in an AttributeStatement. That's also how it's still documented for simplesamlphp: https://simplesamlphp.org/docs/1.11/simplesamlphp-metadata-extensions-attributes.

I've grepped the code base and it seems that set_type() is consistently used -- so it's the central entry point for xs:-based types into the document. That's why it seems to be a proper place to conditionally add the namespace declaration.

The patch works, in the sense that the resulting documents contain the declaration and pass the validation that failed previously.

This issue has actually been reported before:

• http://stackoverflow.com/questions/31975364/namespace-issue-with-pysaml2-xml-canonicalization
• Way to specify inclusive namespace prefix for XML canonicalization? #249

This PR was mentioned as related: #326 -- I am not sure if that works, and the changes are probably way larger than required.

Related:

• What is "translation missing: en.error_saml_invalid_ticket"?? chrodriguez/redmine_omniauth_saml#4 (comment)
• https://www.w3.org/TR/2002/REC-xml-exc-c14n-20020718
• https://www.w3.org/TR/2001/REC-xmlschema-1-20010502/
• https://exchange-specifications.wikispaces.com/share/view/70472736

[jgehrcke]

Two things I noticed was:

• XS_NAMESPACE was so far only defined, but not used:

$ grep -R "XS_NAMESPACE" .
./saml.py:XS_NAMESPACE = 'http://www.w3.org/2001/XMLSchema'

• There is this suspicious comment in s_utils.py (and no other significant occurrance of xmlns:xs in the entire codebase):

# xmlns:xs="http://www.w3.org/2001/XMLSchema"
# xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"

Looks like this was thought about, but then postponed.

@rohe would appreciate your feedback, thanks!

[jgehrcke]

I was a little surprised to see all CI tests pass. We should probably add a test that would have failed before.

[jaustinpage]

These changes look good to me. 👍

[jaustinpage]

👍

[jgehrcke]

Closing, after #365 has been merged. Thanks for pushing and discussing @jaustinpage @ashimaathri @rohe

[c00kiemon5ter]

Yes, a failing test would have been nice, so that future changes do not break this edge case again.