Merge pull request #261 from doticatto/feature/fix-friendlyname

Allow duplicate named attributes in SAML2 assertions

Author: pitbulk

README.md

@@ -451,7 +451,11 @@ In addition to the required settings data (idp, sp), extra settings can be defin
         //    'http://www.w3.org/2001/04/xmlenc#sha256'
         //    'http://www.w3.org/2001/04/xmldsig-more#sha384'
         //    'http://www.w3.org/2001/04/xmlenc#sha512'
-        'digestAlgorithm': "http://www.w3.org/2001/04/xmlenc#sha256"
+        'digestAlgorithm': "http://www.w3.org/2001/04/xmlenc#sha256",
+            
+        // Specify if you want the SP to view assertions with duplicated Name or FriendlyName attributes to be valid
+        // Defaults to false if not specified
+        'allowRepeatAttributeName': false
     },
 
     // Contact information template, it is recommended to suply a

src/onelogin/saml2/idp_metadata_parser.py

@@ -23,7 +23,7 @@
 class OneLogin_Saml2_IdPMetadataParser(object):
     """
     A class that contain methods related to obtaining and parsing metadata from IdP
-    
+
     This class does not validate in any way the URL that is introduced,
     make sure to validate it properly before use it in a get_metadata method.
     """

src/onelogin/saml2/response.py

@@ -573,49 +573,25 @@ def get_attributes(self):
         Gets the Attributes from the AttributeStatement element.
         EncryptedAttributes are not supported
         """
-        attributes = {}
-        attribute_nodes = self.__query_assertion('/saml:AttributeStatement/saml:Attribute')
-        for attribute_node in attribute_nodes:
-            attr_name = attribute_node.get('Name')
-            if attr_name in attributes.keys():
-                raise OneLogin_Saml2_ValidationError(
-                    'Found an Attribute element with duplicated Name',
-                    OneLogin_Saml2_ValidationError.DUPLICATED_ATTRIBUTE_NAME_FOUND
-                )
-
-            values = []
-            for attr in attribute_node.iterchildren('{%s}AttributeValue' % OneLogin_Saml2_Constants.NSMAP['saml']):
-                attr_text = OneLogin_Saml2_XML.element_text(attr)
-                if attr_text:
-                    attr_text = attr_text.strip()
-                    if attr_text:
-                        values.append(attr_text)
-
-                # Parse any nested NameID children
-                for nameid in attr.iterchildren('{%s}NameID' % OneLogin_Saml2_Constants.NSMAP['saml']):
-                    values.append({
-                        'NameID': {
-                            'Format': nameid.get('Format'),
-                            'NameQualifier': nameid.get('NameQualifier'),
-                            'value': nameid.text
-                        }
-                    })
-            attributes[attr_name] = values
-        return attributes
+        return self._get_attributes('Name')
 
     def get_friendlyname_attributes(self):
         """
         Gets the Attributes from the AttributeStatement element indexed by FiendlyName.
         EncryptedAttributes are not supported
         """
+        return self._get_attributes('FriendlyName')
+
+    def _get_attributes(self, attr_name):
+        allow_duplicates = self.__settings.get_security_data().get('allowRepeatAttributeName', False)
         attributes = {}
         attribute_nodes = self.__query_assertion('/saml:AttributeStatement/saml:Attribute')
         for attribute_node in attribute_nodes:
-            attr_friendlyname = attribute_node.get('FriendlyName')
-            if attr_friendlyname:
-                if attr_friendlyname in attributes.keys():
+            attr_key = attribute_node.get(attr_name)
+            if attr_key:
+                if not allow_duplicates and attr_key in attributes:
                     raise OneLogin_Saml2_ValidationError(
-                        'Found an Attribute element with duplicated FriendlyName',
+                        'Found an Attribute element with duplicated ' + attr_name,
                         OneLogin_Saml2_ValidationError.DUPLICATED_ATTRIBUTE_NAME_FOUND
                     )
 
@@ -636,7 +612,10 @@ def get_friendlyname_attributes(self):
                                 'value': nameid.text
                             }
                         })
-                attributes[attr_friendlyname] = values
+                if attr_key in attributes:
+                    attributes[attr_key].extend(values)
+                else:
+                    attributes[attr_key] = values
         return attributes
 
     def validate_num_assertions(self):

src/onelogin/saml2/settings.py

@@ -315,6 +315,9 @@ def __add_default_values(self):
         # AttributeStatement required by default
         self.__security.setdefault('wantAttributeStatement', True)
 
+        # Disallow duplicate attribute names by default
+        self.__security.setdefault('allowRepeatAttributeName', False)
+
         self.__idp.setdefault('x509cert', '')
         self.__idp.setdefault('certFingerprint', '')
         self.__idp.setdefault('certFingerprintAlgorithm', 'sha1')

tests/data/responses/response1_with_duplicate_attributes.xml.base64

@@ -0,0 +1 @@
+PHNhbWxwOlJlc3BvbnNlIHhtbG5zOnNhbWw9InVybjpvYXNpczpuYW1lczp0YzpTQU1MOjIuMDphc3NlcnRpb24iIHhtbG5zOnNhbWxwPSJ1cm46b2FzaXM6bmFtZXM6dGM6U0FNTDoyLjA6cHJvdG9jb2wiIElEPSJHT1NBTUxSMTI5MDExNzQ1NzE3OTQiIFZlcnNpb249IjIuMCIgSXNzdWVJbnN0YW50PSIyMDEwLTExLTE4VDIxOjU3OjM3WiIgRGVzdGluYXRpb249IntyZWNpcGllbnR9Ij48c2FtbHA6U3RhdHVzPjxzYW1scDpTdGF0dXNDb2RlIFZhbHVlPSJ1cm46b2FzaXM6bmFtZXM6dGM6U0FNTDoyLjA6c3RhdHVzOlN1Y2Nlc3MiLz48L3NhbWxwOlN0YXR1cz48c2FtbDpBc3NlcnRpb24geG1sbnM6eHM9Imh0dHA6Ly93d3cudzMub3JnLzIwMDEvWE1MU2NoZW1hIiB4bWxuczp4c2k9Imh0dHA6Ly93d3cudzMub3JnLzIwMDEvWE1MU2NoZW1hLWluc3RhbmNlIiBWZXJzaW9uPSIyLjAiIElEPSJwZng4ZmZiMzk4My1jYmY2LTkyYTEtZjJjNC02MTlhZTFiZTFjODYiIElzc3VlSW5zdGFudD0iMjAxMC0xMS0xOFQyMTo1NzozN1oiPjxzYW1sOklzc3Vlcj5odHRwczovL2FwcC5vbmVsb2dpbi5jb20vc2FtbC9tZXRhZGF0YS8xMzU5MDwvc2FtbDpJc3N1ZXI+PGRzOlNpZ25hdHVyZSB4bWxuczpkcz0iaHR0cDovL3d3dy53My5vcmcvMjAwMC8wOS94bWxkc2lnIyI+PGRzOlNpZ25lZEluZm8+PGRzOkNhbm9uaWNhbGl6YXRpb25NZXRob2QgQWxnb3JpdGhtPSJodHRwOi8vd3d3LnczLm9yZy8yMDAxLzEwL3htbC1leGMtYzE0biMiLz48ZHM6U2lnbmF0dXJlTWV0aG9kIEFsZ29yaXRobT0iaHR0cDovL3d3dy53My5vcmcvMjAwMC8wOS94bWxkc2lnI3JzYS1zaGExIi8+PGRzOlJlZmVyZW5jZSBVUkk9IiNwZng4ZmZiMzk4My1jYmY2LTkyYTEtZjJjNC02MTlhZTFiZTFjODYiPjxkczpUcmFuc2Zvcm1zPjxkczpUcmFuc2Zvcm0gQWxnb3JpdGhtPSJodHRwOi8vd3d3LnczLm9yZy8yMDAwLzA5L3htbGRzaWcjZW52ZWxvcGVkLXNpZ25hdHVyZSIvPjxkczpUcmFuc2Zvcm0gQWxnb3JpdGhtPSJodHRwOi8vd3d3LnczLm9yZy8yMDAxLzEwL3htbC1leGMtYzE0biMiLz48L2RzOlRyYW5zZm9ybXM+PGRzOkRpZ2VzdE1ldGhvZCBBbGdvcml0aG09Imh0dHA6Ly93d3cudzMub3JnLzIwMDAvMDkveG1sZHNpZyNzaGExIi8+PGRzOkRpZ2VzdFZhbHVlPmhndVFiQ0hhbmliYkRDN3EzWnp4ekhjUG9uST08L2RzOkRpZ2VzdFZhbHVlPjwvZHM6UmVmZXJlbmNlPjwvZHM6U2lnbmVkSW5mbz48ZHM6U2lnbmF0dXJlVmFsdWU+R2FuY0Q5dlJvaDlNYlQwMDJEeTc5dDFtNkk2WWZoVUtQZmJsa21wMnVkb2xYdWp2NmUxTVd2c1ZteE56dHNJR2x4QWEwcUtEaVNNekNORFpzazNqc3lzVWwxbkFLbkFnMTg1amZYanN6aHNkbVIrTTkxZHhrNmtmY0xVb3NPb2xvdmFkV0xQV3FuN1AzajgvNXh6cDlMcFJBM2d2QjQxODJSU2lyV0NCWFBRPTwvZHM6U2lnbmF0dXJlVmFsdWU+PGRzOktleUluZm8+PGRzOlg1MDlEYXRhPjxkczpYNTA5Q2VydGlmaWNhdGU+TUlJQ2dUQ0NBZW9DQ1FDYk9scldEZFg3RlRBTkJna3Foa2lHOXcwQkFRVUZBRENCaERFTE1Ba0dBMVVFQmhNQ1RrOHhHREFXQmdOVkJBZ1REMEZ1WkhKbFlYTWdVMjlzWW1WeVp6RU1NQW9HQTFVRUJ4TURSbTl2TVJBd0RnWURWUVFLRXdkVlRrbE9SVlJVTVJnd0ZnWURWUVFERXc5bVpXbGtaUzVsY214aGJtY3VibTh4SVRBZkJna3Foa2lHOXcwQkNRRVdFbUZ1WkhKbFlYTkFkVzVwYm1WMGRDNXViekFlRncwd056QTJNVFV4TWpBeE16VmFGdzB3TnpBNE1UUXhNakF4TXpWYU1JR0VNUXN3Q1FZRFZRUUdFd0pPVHpFWU1CWUdBMVVFQ0JNUFFXNWtjbVZoY3lCVGIyeGlaWEpuTVF3d0NnWURWUVFIRXdOR2IyOHhFREFPQmdOVkJBb1RCMVZPU1U1RlZGUXhHREFXQmdOVkJBTVREMlpsYVdSbExtVnliR0Z1Wnk1dWJ6RWhNQjhHQ1NxR1NJYjNEUUVKQVJZU1lXNWtjbVZoYzBCMWJtbHVaWFIwTG01dk1JR2ZNQTBHQ1NxR1NJYjNEUUVCQVFVQUE0R05BRENCaVFLQmdRRGl2YmhSN1A1MTZ4L1MzQnFLeHVwUWUwTE9Ob2xpdXBpQk9lc0NPM1NIYkRybDMrcTlJYmZuZm1FMDRyTnVNY1BzSXhCMTYxVGREcEllc0xDbjdjOGFQSElTS090UGxBZVRaU25iOFFBdTdhUmpacTMrUGJyUDV1VzNUY2ZDR1B0S1R5dEhPZ2UvT2xKYm8wNzhkVmhYUTE0ZDFFRHdYSlcxclJYdVV0NEM4UUlEQVFBQk1BMEdDU3FHU0liM0RRRUJCUVVBQTRHQkFDRFZmcDg2SE9icVkrZThCVW9XUTkrVk1ReDFBU0RvaEJqd09zZzJXeWtVcVJYRitkTGZjVUg5ZFdSNjNDdFpJS0ZEYlN0Tm9tUG5RejduYksrb255Z3dCc3BWRWJuSHVVaWhacTNaVWRtdW1RcUN3NFV2cy8xVXZxM29yT28vV0pWaFR5dkxnRlZLMlFhclE0LzY3T1pmSGQ3UitQT0JYaG9waFNNdjFaT288L2RzOlg1MDlDZXJ0aWZpY2F0ZT48L2RzOlg1MDlEYXRhPjwvZHM6S2V5SW5mbz48L2RzOlNpZ25hdHVyZT48c2FtbDpTdWJqZWN0PjxzYW1sOk5hbWVJRCBGb3JtYXQ9InVybjpvYXNpczpuYW1lczp0YzpTQU1MOjEuMTpuYW1laWQtZm9ybWF0OmVtYWlsQWRkcmVzcyI+c3VwcG9ydEBvbmVsb2dpbi5jb208L3NhbWw6TmFtZUlEPjxzYW1sOlN1YmplY3RDb25maXJtYXRpb24gTWV0aG9kPSJ1cm46b2FzaXM6bmFtZXM6dGM6U0FNTDoyLjA6Y206YmVhcmVyIj48c2FtbDpTdWJqZWN0Q29uZmlybWF0aW9uRGF0YSBOb3RPbk9yQWZ0ZXI9IjIwMTAtMTEtMThUMjI6MDI6MzdaIiBSZWNpcGllbnQ9IntyZWNpcGllbnR9Ii8+PC9zYW1sOlN1YmplY3RDb25maXJtYXRpb24+PC9zYW1sOlN1YmplY3Q+PHNhbWw6Q29uZGl0aW9ucyBOb3RCZWZvcmU9IjIwMTAtMTEtMThUMjE6NTI6MzdaIiBOb3RPbk9yQWZ0ZXI9IjIwMTAtMTEtMThUMjI6MDI6MzdaIj48c2FtbDpBdWRpZW5jZVJlc3RyaWN0aW9uPjxzYW1sOkF1ZGllbmNlPnthdWRpZW5jZX08L3NhbWw6QXVkaWVuY2U+PC9zYW1sOkF1ZGllbmNlUmVzdHJpY3Rpb24+PC9zYW1sOkNvbmRpdGlvbnM+PHNhbWw6QXV0aG5TdGF0ZW1lbnQgQXV0aG5JbnN0YW50PSIyMDEwLTExLTE4VDIxOjU3OjM3WiIgU2Vzc2lvbk5vdE9uT3JBZnRlcj0iMjAxMC0xMS0xOVQyMTo1NzozN1oiIFNlc3Npb25JbmRleD0iXzUzMWMzMmQyODNiZGZmN2UwNGU0ODdiY2RiYzRkZDhkIj48c2FtbDpBdXRobkNvbnRleHQ+PHNhbWw6QXV0aG5Db250ZXh0Q2xhc3NSZWY+dXJuOm9hc2lzOm5hbWVzOnRjOlNBTUw6Mi4wOmFjOmNsYXNzZXM6UGFzc3dvcmQ8L3NhbWw6QXV0aG5Db250ZXh0Q2xhc3NSZWY+PC9zYW1sOkF1dGhuQ29udGV4dD48L3NhbWw6QXV0aG5TdGF0ZW1lbnQ+PHNhbWw6QXR0cmlidXRlU3RhdGVtZW50PjxzYW1sOkF0dHJpYnV0ZSBOYW1lPSJ1aWQiIEZyaWVuZGx5TmFtZT0idXNlcm5hbWUiPjxzYW1sOkF0dHJpYnV0ZVZhbHVlIHhtbG5zOnhzPSJodHRwOi8vd3d3LnczLm9yZy8yMDAxL1hNTFNjaGVtYSIgeG1sbnM6eHNpPSJodHRwOi8vd3d3LnczLm9yZy8yMDAxL1hNTFNjaGVtYS1pbnN0YW5jZSIgeHNpOnR5cGU9InhzOnN0cmluZyI+ZGVtbzwvc2FtbDpBdHRyaWJ1dGVWYWx1ZT48L3NhbWw6QXR0cmlidXRlPjxzYW1sOkF0dHJpYnV0ZSBOYW1lPSJmcmllbmRseTEiIEZyaWVuZGx5TmFtZT0iZnJpZW5kbHl0ZXN0Ij48c2FtbDpBdHRyaWJ1dGVWYWx1ZSB4bWxuczp4cz0iaHR0cDovL3d3dy53My5vcmcvMjAwMS9YTUxTY2hlbWEiIHhtbG5zOnhzaT0iaHR0cDovL3d3dy53My5vcmcvMjAwMS9YTUxTY2hlbWEtaW5zdGFuY2UiIHhzaTp0eXBlPSJ4czpzdHJpbmciPmZyaWVuZGx5MTwvc2FtbDpBdHRyaWJ1dGVWYWx1ZT48L3NhbWw6QXR0cmlidXRlPjxzYW1sOkF0dHJpYnV0ZSBOYW1lPSJmcmllbmRseTIiIEZyaWVuZGx5TmFtZT0iZnJpZW5kbHl0ZXN0Ij48c2FtbDpBdHRyaWJ1dGVWYWx1ZSB4bWxuczp4cz0iaHR0cDovL3d3dy53My5vcmcvMjAwMS9YTUxTY2hlbWEiIHhtbG5zOnhzaT0iaHR0cDovL3d3dy53My5vcmcvMjAwMS9YTUxTY2hlbWEtaW5zdGFuY2UiIHhzaTp0eXBlPSJ4czpzdHJpbmciPmZyaWVuZGx5Mjwvc2FtbDpBdHRyaWJ1dGVWYWx1ZT48L3NhbWw6QXR0cmlidXRlPjxzYW1sOkF0dHJpYnV0ZSBOYW1lPSJhbm90aGVyX3ZhbHVlIj48c2FtbDpBdHRyaWJ1dGVWYWx1ZSB4bWxuczp4cz0iaHR0cDovL3d3dy53My5vcmcvMjAwMS9YTUxTY2hlbWEiIHhtbG5zOnhzaT0iaHR0cDovL3d3dy53My5vcmcvMjAwMS9YTUxTY2hlbWEtaW5zdGFuY2UiIHhzaTp0eXBlPSJ4czpzdHJpbmciPnZhbHVlPC9zYW1sOkF0dHJpYnV0ZVZhbHVlPjwvc2FtbDpBdHRyaWJ1dGU+PHNhbWw6QXR0cmlidXRlIE5hbWU9ImR1cGxpY2F0ZV9uYW1lIj48c2FtbDpBdHRyaWJ1dGVWYWx1ZSB4bWxuczp4cz0iaHR0cDovL3d3dy53My5vcmcvMjAwMS9YTUxTY2hlbWEiIHhtbG5zOnhzaT0iaHR0cDovL3d3dy53My5vcmcvMjAwMS9YTUxTY2hlbWEtaW5zdGFuY2UiIHhzaTp0eXBlPSJ4czpzdHJpbmciPm5hbWUxPC9zYW1sOkF0dHJpYnV0ZVZhbHVlPjwvc2FtbDpBdHRyaWJ1dGU+PHNhbWw6QXR0cmlidXRlIE5hbWU9ImR1cGxpY2F0ZV9uYW1lIj48c2FtbDpBdHRyaWJ1dGVWYWx1ZSB4bWxuczp4cz0iaHR0cDovL3d3dy53My5vcmcvMjAwMS9YTUxTY2hlbWEiIHhtbG5zOnhzaT0iaHR0cDovL3d3dy53My5vcmcvMjAwMS9YTUxTY2hlbWEtaW5zdGFuY2UiIHhzaTp0eXBlPSJ4czpzdHJpbmciPm5hbWUyPC9zYW1sOkF0dHJpYnV0ZVZhbHVlPjwvc2FtbDpBdHRyaWJ1dGU+PC9zYW1sOkF0dHJpYnV0ZVN0YXRlbWVudD48L3NhbWw6QXNzZXJ0aW9uPjwvc2FtbHA6UmVzcG9uc2U+