Adapt PR. Toolkit will only support 1 AttributeConsumingService. Adding AttributeConsumingService with String style

Author: pitbulk

README.md

@@ -227,17 +227,17 @@ This is the settings.json file:
         // attributeConsumingService. nameFormat, attributeValue and
         // friendlyName can be ommited
         "attributeConsumingService": {
-            "ServiceName": "SP test",
-            "serviceDescription": "Test Service",
-            "requestedAttributes": [
-                {
-                    "name": "",
-                    "isRequired": false,
-                    "nameFormat": "",
-                    "friendlyName": "",
-                    "attributeValue": ""
-                }
-            ]
+                "ServiceName": "SP test",
+                "serviceDescription": "Test Service",
+                "requestedAttributes": [
+                    {
+                        "name": "",
+                        "isRequired": false,
+                        "nameFormat": "",
+                        "friendlyName": "",
+                        "attributeValue": ""
+                    }
+                ]
         },
         // Specifies info about where and how the <Logout Response> message MUST be
         // returned to the requester, in this case our SP. 

src/onelogin/saml2/authn_request.py

@@ -88,8 +88,6 @@ def __init__(self, settings, force_authn=False, is_passive=False):
 
         attr_consuming_service_str = ''
         if 'attributeConsumingService' in sp_data and sp_data['attributeConsumingService']:
-            # TODO: Do we have to account for the case when we have multiple attributeconsumers?
-            # like will the index be > 1?
             attr_consuming_service_str = 'AttributeConsumingServiceIndex="1"'
 
         request = """<samlp:AuthnRequest

src/onelogin/saml2/metadata.py

@@ -12,7 +12,6 @@
 from time import gmtime, strftime
 from datetime import datetime
 from defusedxml.minidom import parseString
-from lxml import etree
 
 from onelogin.saml2.constants import OneLogin_Saml2_Constants
 from onelogin.saml2.utils import OneLogin_Saml2_Utils
@@ -28,63 +27,6 @@ class OneLogin_Saml2_Metadata(object):
     TIME_VALID = 172800   # 2 days
     TIME_CACHED = 604800  # 1 week
 
-    @staticmethod
-    def add_attribute_consuming_service(root, attr_consuming_service):
-        """Helper function to add the AttributeConsumingService nodes"""
-        attrib_index = 1
-        spso_node = root.find('{%s}SPSSODescriptor' % OneLogin_Saml2_Constants.NS_MD)
-
-        # iterate through all the consuming services listed
-        for acs in attr_consuming_service:
-            acs_node = etree.SubElement(spso_node, "{%s}AttributeConsumingService" % OneLogin_Saml2_Constants.NS_MD)
-            acs_node.set('index', str(attrib_index))
-            try:
-                acs_node.set('isDefault', str(acs['isDefault']).lower())
-            except KeyError:
-                pass
-
-            svc_name = etree.SubElement(acs_node, "{%s}ServiceName" % OneLogin_Saml2_Constants.NS_MD)
-            svc_name.set('{%s}lang' % OneLogin_Saml2_Constants.XML, 'en')
-            svc_name.text = acs['serviceName']
-            try:
-                svc_description = etree.SubElement(acs_node, "{%s}ServiceDescription" % OneLogin_Saml2_Constants.NS_MD)
-                svc_description.set('{%s}lang' % OneLogin_Saml2_Constants.XML, 'en')
-                svc_description.text = acs['serviceDescription']
-            except KeyError:
-                # serviceDescription is optional
-                pass
-
-            # iterate through all the requested attributes of each service
-            for req_attribs in acs['requestedAttributes']:
-
-                requested_attribute = etree.SubElement(acs_node, "{%s}RequestedAttribute" % OneLogin_Saml2_Constants.NS_MD)
-                # construct the permissible attrib values, if present
-                try:
-                    for attrib_val in req_attribs['attributeValue']:
-                        val = etree.SubElement(requested_attribute, "{%s}AttributeValue" % OneLogin_Saml2_Constants.NS_SAML)
-                        val.text = attrib_val
-                except KeyError:
-                    # it's fine, attributeValue is an optional element
-                    pass
-
-                requested_attribute.set('Name', req_attribs['name'])
-                try:
-                    requested_attribute.set('NameFormat', req_attribs['nameFormat'])
-                except KeyError:
-                    pass
-
-                try:
-                    requested_attribute.set('FriendlyName', req_attribs['friendlyName'])
-                except KeyError:
-                    pass
-
-                try:
-                    requested_attribute.set('isRequired', str(req_attribs['isRequired']).lower())
-                except KeyError:
-                    pass
-
-            attrib_index += 1
-
     @staticmethod
     def builder(sp, authnsign=False, wsign=False, valid_until=None, cache_duration=None, contacts=None, organization=None):
         """
@@ -134,10 +76,54 @@ def builder(sp, authnsign=False, wsign=False, valid_until=None, cache_duration=N
         if organization is None:
             organization = {}
 
-        try:
-            attr_consuming_service = sp['attributeConsumingService']
-        except KeyError:
-            attr_consuming_service = []
+        str_attribute_consuming_service = ''
+
+        if 'attributeConsumingService' in sp and len(sp['attributeConsumingService']):
+            attr_cs_desc_str = ''
+            if "serviceDescription" in sp['attributeConsumingService']:
+                attr_cs_desc_str = """            <md:ServiceDescription xml:lang="en">%s</md:ServiceDescription>
+""" % sp['attributeConsumingService']['serviceDescription']
+
+            requested_attribute_data = []
+            for req_attribs in sp['attributeConsumingService']['requestedAttributes']:
+                req_attr_nameformat_str = req_attr_friendlyname_str = req_attr_isrequired_str = ''
+                req_attr_aux_str = ' \>'
+
+                if 'nameFormat' in req_attribs.keys() and req_attribs['nameFormat']:
+                    req_attr_nameformat_str = " NameFormat=\"%s\"" % req_attribs['nameFormat']
+                if 'friendlyName' in req_attribs.keys() and req_attribs['friendlyName']:
+                    req_attr_nameformat_str = " FriendlyName=\"%s\"" % req_attribs['friendlyName']
+                if 'isRequired' in req_attribs.keys() and req_attribs['isRequired']:
+                    req_attr_isrequired_str = " isRequired=\"%s\"" % req_attribs['isRequired']
+                if 'attributeValue' in req_attribs.keys() and req_attribs['attributeValue']:
+                    req_attr_aux_str = """ >
+            <saml:AttributeValue xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion>%(attributeValue)</saml:AttributeValue>
+</md:RequestedAttribute>""" % \
+                        {
+                            'attributeValue': req_attribs['attributeValue']
+                        }
+
+                requested_attribute = """            <md:RequestedAttribute Name="%(req_attr_name)s"%(req_attr_nameformat_str)s%(req_attr_isrequired_str)s%(req_attr_aux_str)s""" % \
+                    {
+                        'req_attr_name': req_attribs['name'],
+                        'req_attr_nameformat_str': req_attr_nameformat_str,
+                        'req_attr_friendlyname_str': req_attr_friendlyname_str,
+                        'req_attr_isrequired_str': req_attr_isrequired_str,
+                        'req_attr_aux_str': req_attr_aux_str
+                    }
+
+                requested_attribute_data.append(requested_attribute)
+
+            str_attribute_consuming_service = """        <md:AttributeConsumingService index="1">
+            <md:ServiceName xml:lang="en">%(service_name)s</md:ServiceName>
+%(attr_cs_desc)s%(requested_attribute_str)s
+        </md:AttributeConsumingService>
+""" % \
+                {
+                    'service_name': sp['attributeConsumingService']['serviceName'],
+                    'attr_cs_desc': attr_cs_desc_str,
+                    'requested_attribute_str': '\n'.join(requested_attribute_data)
+                }
 
         sls = ''
         if 'singleLogoutService' in sp and 'url' in sp['singleLogoutService']:
@@ -163,7 +149,7 @@ def builder(sp, authnsign=False, wsign=False, valid_until=None, cache_duration=N
             org_data = '\n'.join(organization_names) + '\n' + '\n'.join(organization_displaynames) + '\n' + '\n'.join(organization_urls)
             str_organization = """    <md:Organization>
 %(org)s
-    </md:Organization>""" % {'org': org_data}
+    </md:Organization>\n""" % {'org': org_data}
 
         str_contacts = ''
         if len(contacts) > 0:
@@ -179,10 +165,10 @@ def builder(sp, authnsign=False, wsign=False, valid_until=None, cache_duration=N
                         'email': info['emailAddress'],
                     }
                 contacts_info.append(contact)
-            str_contacts = '\n'.join(contacts_info)
+            str_contacts = '\n'.join(contacts_info) + '\n'
 
         metadata = """<?xml version="1.0"?>
-<md:EntityDescriptor xmlns:md="urn:oasis:names:tc:SAML:2.0:metadata" %(saml_namespace)s
+<md:EntityDescriptor xmlns:md="urn:oasis:names:tc:SAML:2.0:metadata"
                      %(valid)s
                      %(cache)s
                      entityID="%(entity_id)s">
@@ -191,10 +177,8 @@ def builder(sp, authnsign=False, wsign=False, valid_until=None, cache_duration=N
         <md:AssertionConsumerService Binding="%(binding)s"
                                      Location="%(location)s"
                                      index="1" />
-    </md:SPSSODescriptor>
-%(organization)s
-%(contacts)s
-</md:EntityDescriptor>""" % \
+%(attribute_consuming_service)s    </md:SPSSODescriptor>
+%(organization)s%(contacts)s</md:EntityDescriptor>""" % \
             {
                 'valid': ('validUntil="%s"' % valid_until_str) if valid_until_str else '',
                 'cache': ('cacheDuration="%s"' % cache_duration_str) if cache_duration_str else '',
@@ -207,16 +191,9 @@ def builder(sp, authnsign=False, wsign=False, valid_until=None, cache_duration=N
                 'sls': sls,
                 'organization': str_organization,
                 'contacts': str_contacts,
-                'saml_namespace': 'xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion"' if attr_consuming_service else ''
+                'attribute_consuming_service': str_attribute_consuming_service
             }
-
-        # i'm not sure why the above xml was build by hand. Building via lxml is way easier,
-        # especially for conditional attributes etc..
-        # So as a work around, i'm creating a xml dom, insert the attibute_consumer_service
-        # nodes into it and then return the serialized xml
-        root = etree.fromstring(metadata)
-        OneLogin_Saml2_Metadata.add_attribute_consuming_service(root, attr_consuming_service)
-        return etree.tostring(root, pretty_print=True)
+        return metadata
 
     @staticmethod
     def sign_metadata(metadata, key, cert, sign_algorithm=OneLogin_Saml2_Constants.RSA_SHA1):

src/onelogin/saml2/settings.py

@@ -257,9 +257,8 @@ def __add_default_values(self):
         if 'binding' not in self.__sp['assertionConsumerService'].keys():
             self.__sp['assertionConsumerService']['binding'] = OneLogin_Saml2_Constants.BINDING_HTTP_POST
 
-        # attributeConsumingService is optional
-        if 'attributeConsumingService' not in self.__sp:
-            self.__sp['attributeConsumingService'] = []
+        if 'attributeConsumingService' not in self.__sp.keys():
+            self.__sp['attributeConsumingService'] = {}
 
         if 'singleLogoutService' not in self.__sp.keys():
             self.__sp['singleLogoutService'] = {}
@@ -441,48 +440,29 @@ def check_sp_settings(self, settings):
                     errors.append('sp_acs_url_invalid')
 
                 if 'attributeConsumingService' in sp and len(sp['attributeConsumingService']):
-                    # so we have a attributeConsumingService element...
-
-                    # serviceName and requestedAttrib are required
-                    attribute_consuming_service = sp['attributeConsumingService']
-                    for attrib in attribute_consuming_service:
-                        if 'serviceName' not in attrib:
-                            errors.append('sp_attributeConsumingService_serviceName_not_found')
-                        if 'requestedAttributes' not in attrib:
-                            errors.append('sp_attributeConsumingService_requestedAttributes_not_found')
-
-                        # verify that tags are of the correct types
-                        try:
-                            if type(attrib['isDefault']) != bool:
-                                errors.append('sp_attributeConsumingService_isDefault_type_invalid')
-                        except KeyError:
-                            # isDefault attribute is optional
-                            pass
-
-                        if 'serviceName' in attrib and not isinstance(attrib['serviceName'], basestring):
-                            errors.append('sp_attributeConsumingService_serviceName_type_invalid')
-
-                        try:
-                            if not isinstance(attrib['serviceDescription'], basestring):
-                                errors.append('sp_attributeConsumingService_serviceDescription_type_invalid')
-                        except KeyError:
-                            # serviceDescription attribute is optional
-                            pass
-
-                        if 'requestedAttributes' in attrib:
-                            if type(attrib['requestedAttributes']) != list:
-                                errors.append('sp_attributeConsumingService_requestedAttributes_type_invalid')
-
-                            for req_attrib in attrib['requestedAttributes']:
-                                if 'name' not in req_attrib:
-                                    errors.append('sp_attributeConsumingService_requestedAttributes_name_not_found')
-                                if 'name' in req_attrib and not req_attrib['name'].strip():
-                                    # name cannot be empty
-                                    errors.append('sp_attributeConsumingService_requestedAttributes_name_invalid')
-                                if 'attributeValue' in req_attrib and type(req_attrib['attributeValue']) != list:
-                                    errors.append('sp_attributeConsumingService_requestedAttributes_attributeValue_type_invalid')
-                                if 'isRequired' in req_attrib and type(req_attrib['isRequired']) != bool:
-                                    errors.append('sp_attributeConsumingService_requestedAttributes_isRequired_type_invalid')
+                    attributeConsumingService = sp['attributeConsumingService']
+                    if 'serviceName' not in attributeConsumingService:
+                        errors.append('sp_attributeConsumingService_serviceName_not_found')
+                    elif not isinstance(attributeConsumingService['serviceName'], basestring):
+                        errors.append('sp_attributeConsumingService_serviceName_type_invalid')
+
+                    if 'requestedAttributes' not in attributeConsumingService:
+                        errors.append('sp_attributeConsumingService_requestedAttributes_not_found')
+                    elif not isinstance(attributeConsumingService['requestedAttributes'], list):
+                        errors.append('sp_attributeConsumingService_serviceName_type_invalid')
+                    else:
+                        for req_attrib in attributeConsumingService['requestedAttributes']:
+                            if 'name' not in req_attrib:
+                                errors.append('sp_attributeConsumingService_requestedAttributes_name_not_found')
+                            if 'name' in req_attrib and not req_attrib['name'].strip():
+                                errors.append('sp_attributeConsumingService_requestedAttributes_name_invalid')
+                            if 'attributeValue' in req_attrib and type(req_attrib['attributeValue']) != list:
+                                errors.append('sp_attributeConsumingService_requestedAttributes_attributeValue_type_invalid')
+                            if 'isRequired' in req_attrib and type(req_attrib['isRequired']) != bool:
+                                errors.append('sp_attributeConsumingService_requestedAttributes_isRequired_type_invalid')
+
+                    if "serviceDescription" in attributeConsumingService and not isinstance(attributeConsumingService['serviceDescription'], basestring):
+                        errors.append('sp_attributeConsumingService_serviceDescription_type_invalid')
 
                 if 'singleLogoutService' in sp and \
                     'url' in sp['singleLogoutService'] and \