Assertion AttributeStatements must be non-empty

The saml-core-2.0-os specification section 2.7.3 requires
the AttributeStatement element to be non-empty. Shibboleth verifies
this and rejects assertions that do not comply. We gather attributes
into a local dict first before adding them to the AttributeStatement
so the fix is easy. Test if the dict is empty, move the initialization
of the assertion AttributeStatement inside the test so it's
conditional on whether the dict has members.

https://fedorahosted.org/ipsilon/ticket/61

Signed-off-by: John Dennis <jdennis@redhat.com>
Reviewed-by: Nathan Kinder <nkinder@redhat.com>

diff --git a/ipsilon/providers/saml2/auth.py b/ipsilon/providers/saml2/auth.py
index ddebd8c..f5e8f0f 100644 (file)
--- a/ipsilon/providers/saml2/auth.py
+++ b/ipsilon/providers/saml2/auth.py
@@ -202,14 +202,6 @@ class AuthenticateRequest(ProviderPageBase):
             raise AuthenticationError("Unavailable Name ID type",
                                       lasso.SAML2_STATUS_CODE_AUTHN_FAILED)
 
             raise AuthenticationError("Unavailable Name ID type",
                                       lasso.SAML2_STATUS_CODE_AUTHN_FAILED)
 

-        if not login.assertion.attributeStatement:
-            attrstat = lasso.Saml2AttributeStatement()
-            login.assertion.attributeStatement = [attrstat]
-        else:
-            attrstat = login.assertion.attributeStatement[0]
-        if not attrstat.attribute:
-            attrstat.attribute = ()
-

         # Check attribute policy and perform mapping and filtering
         policy = Policy(self.cfg.default_attribute_mapping,
                         self.cfg.default_allowed_attributes)
         # Check attribute policy and perform mapping and filtering
         policy = Policy(self.cfg.default_attribute_mapping,
                         self.cfg.default_allowed_attributes)


@@ -222,6 +214,17 @@ class AuthenticateRequest(ProviderPageBase):
 
         self.debug("%s's attributes: %s" % (user.name, attributes))
 
 
         self.debug("%s's attributes: %s" % (user.name, attributes))
 

+        # The saml-core-2.0-os specification section 2.7.3 requires
+        # the AttributeStatement element to be non-empty.
+        if attributes:
+            if not login.assertion.attributeStatement:
+                attrstat = lasso.Saml2AttributeStatement()
+                login.assertion.attributeStatement = [attrstat]
+            else:
+                attrstat = login.assertion.attributeStatement[0]
+            if not attrstat.attribute:
+                attrstat.attribute = ()
+

         for key in attributes:
             # skip internal info
             if key[0] == '_':
         for key in attributes:
             # skip internal info
             if key[0] == '_':