More cleanups

Author: johnnyshields

lib/ruby_saml/response.rb

@@ -797,8 +797,9 @@ def validate_name_id
     end
 
     def doc_to_validate
-      # If the response contains the signature, and the assertion was encrypted, validate the original SAML Response
-      # otherwise, review if the decrypted assertion contains a signature
+      # Validate the original SAML Response if the response contains the signature,
+      # and the assertion was encrypted. Otherwise, review if the decrypted assertion
+      # contains a signature.
       subject_id = RubySaml::XML::SignedDocumentValidator.subject_id(document)
       return decrypted_document unless subject_id
 
@@ -963,7 +964,6 @@ def xpath_from_signed_assertion(subpath = nil)
     # @return [RubySaml::XML::SignedDocument] The SAML Response with the assertion decrypted
     #
     def generate_decrypted_document
-      # TODO: try decrypt_document! instead
       RubySaml::XML::Decryptor.decrypt_document(document, settings&.get_sp_decryption_keys)
     end
 

lib/ruby_saml/xml/decryptor.rb

@@ -11,15 +11,8 @@ module Decryptor
       # @param decryption_keys [Array] Array of private keys for decryption
       # @return [Nokogiri::XML::Document] The SAML document with assertions decrypted
       def decrypt_document(document, decryption_keys)
-        document_copy = RubySaml::XML.safe_load_nokogiri(document.to_s)
-        decrypt_document!(document_copy, decryption_keys)
-      end
-
-      # Modifies a SAML document to decrypt its EncryptedAssertion element into an Assertion element.
-      # @param document [Nokogiri::XML::Document] The SAML document with the encrypted assertion
-      # @param decryption_keys [Array] Array of private keys for decryption
-      # @return [Nokogiri::XML::Document] The SAML document with the assertion decrypted
-      def decrypt_document!(document, decryption_keys)
+        # Copy the document
+        document = RubySaml::XML.safe_load_nokogiri(document.to_s)
         validate_decryption_keys!(decryption_keys)
 
         response_node = document.at_xpath(

test/response_test.rb

@@ -1787,9 +1787,7 @@ def generate_audience_error(expected, actual)
       end
     end
 
-    idp_key_algo = :rsa
-    idp_hash_algo = :sha256
-    # each_signature_algorithm do |idp_key_algo, idp_hash_algo|
+    each_signature_algorithm do |idp_key_algo, idp_hash_algo|
       describe "#validate_signature" do
         let(:xml_signed) do
           doc = read_response('response_unsigned2.xml')
@@ -1869,7 +1867,7 @@ def generate_audience_error(expected, actual)
             assert_includes response_sign_test.errors, 'Invalid Signature on SAML Response'
           end
         end
-      # end
+      end
     end
   end
 end

test/xml/decryptor_test.rb

@@ -42,26 +42,22 @@ class XmlDecryptorTest < Minitest::Test
 
         refute_nil decrypted_doc.at_xpath('/p:Response/a:Assertion', { 'p' => RubySaml::XML::NS_PROTOCOL, 'a' => RubySaml::XML::NS_ASSERTION })
       end
-    end
-
-    describe '#decrypt_document!' do
-      it 'should decrypt an encrypted assertion in a document' do
-        decrypted_doc = RubySaml::XML::Decryptor.decrypt_document!(noko_encrypted_assertion_doc, decryption_keys)
-
-        # The encrypted assertion should be removed
-        assert_nil decrypted_doc.at_xpath('/p:Response/EncryptedAssertion', { 'p' => RubySaml::XML::NS_PROTOCOL })
-
-        # An assertion should now be present
-        refute_nil decrypted_doc.at_xpath('/p:Response/a:Assertion', { 'p' => RubySaml::XML::NS_PROTOCOL, 'a' => RubySaml::XML::NS_ASSERTION })
-      end
 
       it 'should handle documents without an encrypted assertion' do
         doc_without_encrypted_assertion = Nokogiri::XML("<Response xmlns='urn:oasis:names:tc:SAML:2.0:protocol'><Assertion xmlns='urn:oasis:names:tc:SAML:2.0:assertion'></Assertion></Response>")
-        result = RubySaml::XML::Decryptor.decrypt_document!(doc_without_encrypted_assertion, decryption_keys)
+        result = RubySaml::XML::Decryptor.decrypt_document(doc_without_encrypted_assertion, decryption_keys)
 
         # Should return the document unmodified
         assert_equal doc_without_encrypted_assertion.to_s, result.to_s
       end
+
+      it 'does not modify the original document' do
+        original = document_encrypted_assertion.to_s
+        RubySaml::XML::Decryptor.decrypt_document(document_encrypted_assertion, decryption_keys)
+
+        # The original document should not be modified
+        assert_equal original, document_encrypted_assertion.to_s
+      end
     end
 
     describe '#decrypt_assertion' do