Merge pull request #113 from novafloss/108-get-assertion-when-empty-signature-ref-uri

Get/validate assertion when empty Response signature ref URI

Author: pitbulk

src/onelogin/saml2/response.py

@@ -417,8 +417,11 @@ def __query_assertion(self, xpath_expr):
             # Check if the message is signed
             signed_message_query = '/samlp:Response' + signature_expr
             message_reference_nodes = self.__query(signed_message_query)
+            # we can have reference node but URI can be empty
+            message_id = None
             if message_reference_nodes:
                 message_id = message_reference_nodes[0].get('URI')
+            if message_id:
                 final_query = "/samlp:Response[@ID='%s']/" % message_id[1:]
             else:
                 final_query = "/samlp:Response"

src/onelogin/saml2/utils.py

@@ -1051,17 +1051,15 @@ def validate_node_sign(signature_node, elem, cert=None, fingerprint=None, finger
                     if fingerprint == x509_fingerprint_value:
                         cert = OneLogin_Saml2_Utils.format_cert(x509_cert_value)
 
+                # Check if Reference URI is empty
+                reference_elem = OneLogin_Saml2_Utils.query(signature_node, '//ds:Reference')
+                if len(reference_elem) > 0:
+                    if reference_elem[0].get('URI') == '':
+                        reference_elem[0].set('URI', '#%s' % signature_node.getparent().get('ID'))
+
             if cert is None or cert == '':
                 return False
 
-            # Check if Reference URI is empty
-            reference_elem = OneLogin_Saml2_Utils.query(signature_node, '//ds:Reference')
-            if len(reference_elem) > 0:
-                if reference_elem[0].get('URI') == '':
-                    reference_elem[0].set('URI', '#%s' % signature_node.getparent().get('ID'))
-
-            dsig_ctx = xmlsec.DSigCtx()
-
             file_cert = OneLogin_Saml2_Utils.write_temp_file(cert)
 
             if validatecert:

tests/data/responses/valid_response_with_unsigned_assertion.xml.base64

@@ -0,0 +1 @@
+PD94bWwgdmVyc2lvbj0iMS4wIj8+CjxzYW1scDpSZXNwb25zZSB4bWxuczpzYW1scD0idXJuOm9hc2lzOm5hbWVzOnRjOlNBTUw6Mi4wOnByb3RvY29sIiB4bWxuczpzYW1sPSJ1cm46b2FzaXM6bmFtZXM6dGM6U0FNTDoyLjA6YXNzZXJ0aW9uIiBJRD0icGZ4MDVmM2NlMTAtMTYxNS1mM2VhLWE5ODgtNjBlMzgwYjMyOTlmIiBWZXJzaW9uPSIyLjAiIElzc3VlSW5zdGFudD0iMjAxNC0wMi0xOVQwMTozNzowMVoiIERlc3RpbmF0aW9uPSJodHRwczovL3BpdGJ1bGsubm8taXAub3JnL25ld29uZWxvZ2luL2RlbW8xL2luZGV4LnBocD9hY3MiIEluUmVzcG9uc2VUbz0iT05FTE9HSU5fNWZlOWQ2ZTQ5OWIyZjA5MTMyMDZhYWIzZjcxOTE3MjkwNDliYjgwNyI+CiAgPHNhbWw6SXNzdWVyPmh0dHBzOi8vcGl0YnVsay5uby1pcC5vcmcvc2ltcGxlc2FtbC9zYW1sMi9pZHAvbWV0YWRhdGEucGhwPC9zYW1sOklzc3Vlcj4KICA8ZHM6U2lnbmF0dXJlIHhtbG5zOmRzPSJodHRwOi8vd3d3LnczLm9yZy8yMDAwLzA5L3htbGRzaWcjIj4KICAgIDxkczpTaWduZWRJbmZvPgogICAgICA8ZHM6Q2Fub25pY2FsaXphdGlvbk1ldGhvZCBBbGdvcml0aG09Imh0dHA6Ly93d3cudzMub3JnLzIwMDEvMTAveG1sLWV4Yy1jMTRuIyIvPgogICAgICA8ZHM6U2lnbmF0dXJlTWV0aG9kIEFsZ29yaXRobT0iaHR0cDovL3d3dy53My5vcmcvMjAwMC8wOS94bWxkc2lnI3JzYS1zaGExIi8+CiAgICAgIDxkczpSZWZlcmVuY2UgVVJJPSIiPgogICAgICAgIDxkczpUcmFuc2Zvcm1zPgogICAgICAgICAgPGRzOlRyYW5zZm9ybSBBbGdvcml0aG09Imh0dHA6Ly93d3cudzMub3JnLzIwMDAvMDkveG1sZHNpZyNlbnZlbG9wZWQtc2lnbmF0dXJlIi8+CiAgICAgICAgICA8ZHM6VHJhbnNmb3JtIEFsZ29yaXRobT0iaHR0cDovL3d3dy53My5vcmcvMjAwMS8xMC94bWwtZXhjLWMxNG4jIi8+CiAgICAgICAgPC9kczpUcmFuc2Zvcm1zPgogICAgICAgIDxkczpEaWdlc3RNZXRob2QgQWxnb3JpdGhtPSJodHRwOi8vd3d3LnczLm9yZy8yMDAwLzA5L3htbGRzaWcjc2hhMSIvPgogICAgICAgIDxkczpEaWdlc3RWYWx1ZT51VFlSOWFBcHBLdWp3YkY3azRUOFczLzBaZDA9PC9kczpEaWdlc3RWYWx1ZT4KICAgICAgPC9kczpSZWZlcmVuY2U+CiAgICA8L2RzOlNpZ25lZEluZm8+CiAgICA8ZHM6U2lnbmF0dXJlVmFsdWU+MnYxSDBSaVNJUEtYdEdUcTh3L3hpZHRrci9SWXZUVU90MXdCRmZlaXRhT0wyQkt1Z0JobEFoQyt2U2FVeFoweQpOUHpIdW1odlMzRHZkV2xmUjNzZlNra2VJeDZtYTd3blB2eko3MHZna09QRnF1UFRRQmdJdTVzMnMwUXJoZGgrCmYrU1dEWkpEbTNQeGpPbkhEQ3FrRE1Sc1VhejhtYzV3M0tWQnozQlhRaEU9PC9kczpTaWduYXR1cmVWYWx1ZT4KICA8L2RzOlNpZ25hdHVyZT4KICA8c2FtbHA6U3RhdHVzPgogICAgPHNhbWxwOlN0YXR1c0NvZGUgVmFsdWU9InVybjpvYXNpczpuYW1lczp0YzpTQU1MOjIuMDpzdGF0dXM6U3VjY2VzcyIvPgogIDwvc2FtbHA6U3RhdHVzPgogIDxzYW1sOkFzc2VydGlvbiB4bWxuczp4c2k9Imh0dHA6Ly93d3cudzMub3JnLzIwMDEvWE1MU2NoZW1hLWluc3RhbmNlIiB4bWxuczp4cz0iaHR0cDovL3d3dy53My5vcmcvMjAwMS9YTUxTY2hlbWEiIElEPSJwZnhiNGVjOWM4YS00OGViLWZkYTItN2Y3NC1mYTFhMTA1YTk5ZmUiIFZlcnNpb249IjIuMCIgSXNzdWVJbnN0YW50PSIyMDE0LTAyLTE5VDAxOjM3OjAxWiI+CiAgICA8c2FtbDpJc3N1ZXI+aHR0cHM6Ly9waXRidWxrLm5vLWlwLm9yZy9zaW1wbGVzYW1sL3NhbWwyL2lkcC9tZXRhZGF0YS5waHA8L3NhbWw6SXNzdWVyPgogICAgPHNhbWw6U3ViamVjdD4KICAgICAgPHNhbWw6TmFtZUlEIFNQTmFtZVF1YWxpZmllcj0iaHR0cHM6Ly9waXRidWxrLm5vLWlwLm9yZy9uZXdvbmVsb2dpbi9kZW1vMS9tZXRhZGF0YS5waHAiIEZvcm1hdD0idXJuOm9hc2lzOm5hbWVzOnRjOlNBTUw6MS4xOm5hbWVpZC1mb3JtYXQ6ZW1haWxBZGRyZXNzIj40OTI4ODI2MTVhY2YzMWM4MDk2YjYyNzI0NWQ3NmFlNTMwMzZjMDkwPC9zYW1sOk5hbWVJRD4KICAgICAgPHNhbWw6U3ViamVjdENvbmZpcm1hdGlvbiBNZXRob2Q9InVybjpvYXNpczpuYW1lczp0YzpTQU1MOjIuMDpjbTpiZWFyZXIiPgogICAgICAgIDxzYW1sOlN1YmplY3RDb25maXJtYXRpb25EYXRhIE5vdE9uT3JBZnRlcj0iMjAyMy0wOC0yM1QwNjo1NzowMVoiIFJlY2lwaWVudD0iaHR0cHM6Ly9waXRidWxrLm5vLWlwLm9yZy9uZXdvbmVsb2dpbi9kZW1vMS9pbmRleC5waHA/YWNzIiBJblJlc3BvbnNlVG89Ik9ORUxPR0lOXzVmZTlkNmU0OTliMmYwOTEzMjA2YWFiM2Y3MTkxNzI5MDQ5YmI4MDciLz4KICAgICAgPC9zYW1sOlN1YmplY3RDb25maXJtYXRpb24+CiAgICA8L3NhbWw6U3ViamVjdD4KICAgIDxzYW1sOkNvbmRpdGlvbnMgTm90QmVmb3JlPSIyMDE0LTAyLTE5VDAxOjM2OjMxWiIgTm90T25PckFmdGVyPSIyMDIzLTA4LTIzVDA2OjU3OjAxWiI+CiAgICAgIDxzYW1sOkF1ZGllbmNlUmVzdHJpY3Rpb24+CiAgICAgICAgPHNhbWw6QXVkaWVuY2U+aHR0cHM6Ly9waXRidWxrLm5vLWlwLm9yZy9uZXdvbmVsb2dpbi9kZW1vMS9tZXRhZGF0YS5waHA8L3NhbWw6QXVkaWVuY2U+CiAgICAgIDwvc2FtbDpBdWRpZW5jZVJlc3RyaWN0aW9uPgogICAgPC9zYW1sOkNvbmRpdGlvbnM+CiAgICA8c2FtbDpBdXRoblN0YXRlbWVudCBBdXRobkluc3RhbnQ9IjIwMTQtMDItMTlUMDE6Mzc6MDFaIiBTZXNzaW9uTm90T25PckFmdGVyPSIyMDE0LTAyLTE5VDA5OjM3OjAxWiIgU2Vzc2lvbkluZGV4PSJfNjI3M2Q3N2I4Y2RlMGMzMzNlYzc5ZDIyYTlmYTAwMDNiOWZlMmQ3NWNiIj4KICAgICAgPHNhbWw6QXV0aG5Db250ZXh0PgogICAgICAgIDxzYW1sOkF1dGhuQ29udGV4dENsYXNzUmVmPnVybjpvYXNpczpuYW1lczp0YzpTQU1MOjIuMDphYzpjbGFzc2VzOlBhc3N3b3JkPC9zYW1sOkF1dGhuQ29udGV4dENsYXNzUmVmPgogICAgICA8L3NhbWw6QXV0aG5Db250ZXh0PgogICAgPC9zYW1sOkF1dGhuU3RhdGVtZW50PgogICAgPHNhbWw6QXR0cmlidXRlU3RhdGVtZW50PgogICAgICA8c2FtbDpBdHRyaWJ1dGUgTmFtZT0idWlkIiBOYW1lRm9ybWF0PSJ1cm46b2FzaXM6bmFtZXM6dGM6U0FNTDoyLjA6YXR0cm5hbWUtZm9ybWF0OmJhc2ljIj4KICAgICAgICA8c2FtbDpBdHRyaWJ1dGVWYWx1ZSB4c2k6dHlwZT0ieHM6c3RyaW5nIj5zbWFydGluPC9zYW1sOkF0dHJpYnV0ZVZhbHVlPgogICAgICA8L3NhbWw6QXR0cmlidXRlPgogICAgICA8c2FtbDpBdHRyaWJ1dGUgTmFtZT0ibWFpbCIgTmFtZUZvcm1hdD0idXJuOm9hc2lzOm5hbWVzOnRjOlNBTUw6Mi4wOmF0dHJuYW1lLWZvcm1hdDpiYXNpYyI+CiAgICAgICAgPHNhbWw6QXR0cmlidXRlVmFsdWUgeHNpOnR5cGU9InhzOnN0cmluZyI+c21hcnRpbkB5YWNvLmVzPC9zYW1sOkF0dHJpYnV0ZVZhbHVlPgogICAgICA8L3NhbWw6QXR0cmlidXRlPgogICAgICA8c2FtbDpBdHRyaWJ1dGUgTmFtZT0iY24iIE5hbWVGb3JtYXQ9InVybjpvYXNpczpuYW1lczp0YzpTQU1MOjIuMDphdHRybmFtZS1mb3JtYXQ6YmFzaWMiPgogICAgICAgIDxzYW1sOkF0dHJpYnV0ZVZhbHVlIHhzaTp0eXBlPSJ4czpzdHJpbmciPlNpeHRvMzwvc2FtbDpBdHRyaWJ1dGVWYWx1ZT4KICAgICAgPC9zYW1sOkF0dHJpYnV0ZT4KICAgICAgPHNhbWw6QXR0cmlidXRlIE5hbWU9InNuIiBOYW1lRm9ybWF0PSJ1cm46b2FzaXM6bmFtZXM6dGM6U0FNTDoyLjA6YXR0cm5hbWUtZm9ybWF0OmJhc2ljIj4KICAgICAgICA8c2FtbDpBdHRyaWJ1dGVWYWx1ZSB4c2k6dHlwZT0ieHM6c3RyaW5nIj5NYXJ0aW4yPC9zYW1sOkF0dHJpYnV0ZVZhbHVlPgogICAgICA8L3NhbWw6QXR0cmlidXRlPgogICAgICA8c2FtbDpBdHRyaWJ1dGUgTmFtZT0iZWR1UGVyc29uQWZmaWxpYXRpb24iIE5hbWVGb3JtYXQ9InVybjpvYXNpczpuYW1lczp0YzpTQU1MOjIuMDphdHRybmFtZS1mb3JtYXQ6YmFzaWMiPgogICAgICAgIDxzYW1sOkF0dHJpYnV0ZVZhbHVlIHhzaTp0eXBlPSJ4czpzdHJpbmciPnVzZXI8L3NhbWw6QXR0cmlidXRlVmFsdWU+CiAgICAgICAgPHNhbWw6QXR0cmlidXRlVmFsdWUgeHNpOnR5cGU9InhzOnN0cmluZyI+YWRtaW48L3NhbWw6QXR0cmlidXRlVmFsdWU+CiAgICAgIDwvc2FtbDpBdHRyaWJ1dGU+CiAgICA8L3NhbWw6QXR0cmlidXRlU3RhdGVtZW50PgogIDwvc2FtbDpBc3NlcnRpb24+Cjwvc2FtbHA6UmVzcG9uc2U+Cg==

tests/src/OneLogin/saml2_tests/response_test.py

@@ -207,6 +207,26 @@ def testQueryAssertions(self):
         response_7 = OneLogin_Saml2_Response(settings, xml_7)
         self.assertEqual(['http://idp.example.com/'], response_7.get_issuers())
 
+    def testQueryAssertionsWithEmptyRefenceURI(self):
+        """
+        Tests the __query_assertion if //Signature/Reference/@URI is empty.
+        """
+        settings = OneLogin_Saml2_Settings(self.loadSettingsJSON())
+
+        # test with signed assertion still work
+        xml = self.file_contents(join(self.data_path, 'responses', 'valid_response.xml.base64'))
+        response = OneLogin_Saml2_Response(settings, xml)
+        self.assertEqual('492882615acf31c8096b627245d76ae53036c090', response.get_nameid())
+
+        # test with unsigned assertion still work
+        xml = self.file_contents(join(self.data_path, 'responses', 'valid_response_with_unsigned_assertion.xml.base64'))
+        response = OneLogin_Saml2_Response(settings, xml)
+        self.assertEqual('492882615acf31c8096b627245d76ae53036c090', response.get_nameid())
+
+        xml = self.file_contents(join(self.data_path, 'responses', 'response_without_reference_uri.xml.base64'))
+        response = OneLogin_Saml2_Response(settings, xml)
+        self.assertEqual('saml@user.com', response.get_nameid())
+
     def testGetIssuers(self):
         """
         Tests the get_issuers method of the OneLogin_Saml2_Response
@@ -1198,6 +1218,12 @@ def testIsValidSignWithEmptyReferenceURI(self):
         response = OneLogin_Saml2_Response(settings, xml)
         self.assertTrue(response.is_valid(self.get_request_data()))
 
+    def testIsValidSignWithEmptyReferenceURIAndIdPCert(self):
+        settings = OneLogin_Saml2_Settings(self.loadSettingsJSON())
+        xml = self.file_contents(join(self.data_path, 'responses', 'valid_response_with_unsigned_assertion.xml.base64'))
+        response = OneLogin_Saml2_Response(settings, xml)
+        self.assertTrue(response.is_valid(self.get_request_data()))
+
     def testIsValidWithoutInResponseTo(self):
         """
         If assertion contains InResponseTo but not the Response tag, we should