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

Revert "Get/validate assertion when empty Response signature ref URI"

Author: pitbulk

src/onelogin/saml2/response.py

@@ -417,11 +417,8 @@ 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,15 +1051,17 @@ 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

@@ -207,26 +207,6 @@ 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
@@ -1218,12 +1198,6 @@ 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