More fixes, remove REXML

Author: johnnyshields

lib/ruby_saml/idp_metadata_parser.rb

@@ -348,14 +348,14 @@ def certificates
           unless signing_nodes.empty?
             certs['signing'] = []
             signing_nodes.each do |cert_node|
-              certs['signing'] << cert_node.content
+              certs['signing'] << cert_node.text
             end
           end
 
           unless encryption_nodes.empty?
             certs['encryption'] = []
             encryption_nodes.each do |cert_node|
-              certs['encryption'] << cert_node.content
+              certs['encryption'] << cert_node.text
             end
           end
           certs

lib/ruby_saml/logoutresponse.rb

@@ -69,13 +69,10 @@ def in_response_to
     # @return [String] Gets the Issuer from the Logout Response.
     #
     def issuer
-      @issuer ||= begin
-        node = document.at_xpath(
-          "/p:LogoutResponse/a:Issuer",
-          { "p" => RubySaml::XML::NS_PROTOCOL, "a" => RubySaml::XML::NS_ASSERTION }
-        )
-        Utils.element_text(node)
-      end
+      @issuer ||=document.at_xpath(
+        "/p:LogoutResponse/a:Issuer",
+        { "p" => RubySaml::XML::NS_PROTOCOL, "a" => RubySaml::XML::NS_ASSERTION }
+      )&.text
     end
 
     # @return [String] Gets the StatusCode from a Logout Response.
@@ -88,13 +85,10 @@ def status_code
     end
 
     def status_message
-      @status_message ||= begin
-        node = document.at_xpath(
-          "/p:LogoutResponse/p:Status/p:StatusMessage",
-          { "p" => RubySaml::XML::NS_PROTOCOL }
-        )
-        Utils.element_text(node)
-      end
+      @status_message ||= document.at_xpath(
+        "/p:LogoutResponse/p:Status/p:StatusMessage",
+        { "p" => RubySaml::XML::NS_PROTOCOL }
+      )&.text
     end
 
     # Aux function to validate the Logout Response

lib/ruby_saml/response.rb

@@ -80,7 +80,7 @@ def is_valid?(collect_errors = false)
     # @return [String] the NameID provided by the SAML response from the IdP.
     #
     def name_id
-      @name_id ||= name_id_node&.content
+      @name_id ||= name_id_node&.text
     end
 
     alias_method :nameid, :name_id
@@ -162,14 +162,14 @@ def handle_nokogiri_attribute(node, attributes)
         if e.elements.empty?
           # SAMLCore requires that nil AttributeValues MUST contain xsi:nil XML attribute set to "true" or "1"
           # otherwise the value is to be regarded as empty.
-          %w[true 1].include?(e['xsi:nil']) ? nil : e&.content
+          %w[true 1].include?(e['xsi:nil']) ? nil : e&.text
         else
           # Explicitly support saml2:NameID with saml2:NameQualifier if supplied in attributes
           # this is useful for allowing eduPersonTargetedId to be passed as an opaque identifier to use to
           # identify the subject in an SP rather than email or other less opaque attributes
           # NameQualifier, if present is prefixed with a "/" to the value
           e.xpath('a:NameID', { "a" => RubySaml::XML::NS_ASSERTION }).map do |n|
-            next unless (value = n&.content)
+            next unless (value = n&.text)
             base_path = n['NameQualifier'] ? "#{n['NameQualifier']}/" : ''
             "#{base_path}#{value}"
           end

lib/ruby_saml/saml_message.rb

@@ -29,20 +29,10 @@ def id(document)
     end
 
     def root_attribute(document, attribute)
-      if document.is_a?(Nokogiri::XML::Document)
-        node = document.at_xpath(
-          "/p:AuthnRequest | /p:Response | /p:LogoutResponse | /p:LogoutRequest",
-          { "p" => RubySaml::XML::NS_PROTOCOL }
-        )
-        node.nil? ? nil : node[attribute]
-      else
-        node = REXML::XPath.first(
-          document,
-          "/p:AuthnRequest | /p:Response | /p:LogoutResponse | /p:LogoutRequest",
-          { "p" => RubySaml::XML::NS_PROTOCOL }
-        )
-        node.nil? ? nil : node.attributes[attribute]
-      end
+      document.at_xpath(
+        "/p:AuthnRequest | /p:Response | /p:LogoutResponse | /p:LogoutRequest",
+        { "p" => RubySaml::XML::NS_PROTOCOL }
+      )&.[](attribute)
     end
 
     # Validates the SAML Message against the specified schema.

lib/ruby_saml/slo_logoutrequest.rb

@@ -58,7 +58,7 @@ def is_valid?(collect_errors = false)
 
     # @return [String] The NameID of the Logout Request.
     def name_id
-      @name_id ||= name_id_node&.content
+      @name_id ||= name_id_node&.text
     end
     alias_method :nameid, :name_id
 
@@ -88,13 +88,10 @@ def id
     # @return [String] Gets the Issuer from the Logout Request.
     #
     def issuer
-      @issuer ||= begin
-        node = document.at_xpath(
-          "/p:LogoutRequest/a:Issuer",
-          { "p" => RubySaml::XML::NS_PROTOCOL, "a" => RubySaml::XML::NS_ASSERTION }
-        )
-        Utils.element_text(node)
-      end
+      @issuer ||= document.at_xpath(
+        "/p:LogoutRequest/a:Issuer",
+        { "p" => RubySaml::XML::NS_PROTOCOL, "a" => RubySaml::XML::NS_ASSERTION }
+      )&.text
     end
 
     # @return [Time|nil] Gets the NotOnOrAfter Attribute value if exists.
@@ -115,12 +112,10 @@ def not_on_or_after
     # @return [Array] Gets the SessionIndex if exists (Supported multiple values). Empty Array if none found
     #
     def session_indexes
-      nodes = document.xpath(
+      document.xpath(
         "/p:LogoutRequest/p:SessionIndex",
         { "p" => RubySaml::XML::NS_PROTOCOL }
-      )
-
-      nodes.map { |node| Utils.element_text(node) }
+      ).map { |node| node.text }
     end
 
     private

lib/ruby_saml/utils.rb

@@ -295,17 +295,6 @@ def original_uri_match?(destination_url, settings_url)
       destination_url == settings_url
     end
 
-    # Given a REXML::Element instance, return the concatenation of all child text nodes. Assumes
-    # that there all children other than text nodes can be ignored (e.g. comments). If nil is
-    # passed, nil will be returned.
-    def element_text(element)
-      if element.is_a?(REXML::Element)
-        element.texts.map(&:value).join
-      else
-        element&.content
-      end
-    end
-
     # Given a private key PEM string, return an array of OpenSSL::PKey::PKey classes
     # that can be used to parse it, with the most likely match first.
     def private_key_classes(pem)

lib/ruby_saml/xml.rb

@@ -1,6 +1,5 @@
 # frozen_string_literal: true
 
-require 'rexml/element'
 require 'openssl'
 require 'nokogiri'
 require 'digest/sha1'
@@ -51,9 +50,9 @@ module XML
                        Nokogiri::XML::ParseOptions::NONET
 
     # Safely load the SAML Message XML.
-    # @param document [REXML::Document] The message to be loaded
+    # @param document [String | Nokogiri::XML::Document] The message to be loaded
     # @param check_malformed_doc [Boolean] check_malformed_doc Enable or Disable the check for malformed XML
-    # @return [Nokogiri::XML] The nokogiri document
+    # @return [Nokogiri::XML::Document] The nokogiri document
     # @raise [ValidationError] If there was a problem loading the SAML Message XML
     def safe_load_nokogiri(document, check_malformed_doc: true)
       doc_str = document.to_s
@@ -72,7 +71,7 @@ def safe_load_nokogiri(document, check_malformed_doc: true)
         end
       end
 
-      # TODO: This is messy, its shims how the old work REXML parser
+      # TODO: This is messy, its shims how the old REXML parser works
       if xml
         error ||= StandardError.new('Dangerous XML detected. No Doctype nodes allowed') if xml.internal_subset
         error ||= StandardError.new("There were XML errors when parsing: #{xml.errors}") if check_malformed_doc && !xml.errors.empty?

lib/ruby_saml/xml/decryptor.rb

@@ -72,9 +72,6 @@ def decrypt_attribute(encrypted_attribute_node, decryption_keys)
       def decrypt_node(encrypted_node, regexp, decryption_keys)
         validate_decryption_keys!(decryption_keys)
 
-        # TODO: Remove this
-        encrypted_node = Nokogiri::XML(encrypted_node.to_s).root if encrypted_node.is_a?(REXML::Element)
-
         node_header = if encrypted_node.name == 'EncryptedAttribute'
                         %(<node xmlns:saml="#{RubySaml::XML::NS_ASSERTION}" xmlns:xsi="#{RubySaml::XML::XSI}">)
                       else

lib/ruby_saml/xml/signed_document_info.rb

@@ -178,7 +178,7 @@ def certificate_text
         cert = noko.at_xpath(
           '//ds:X509Certificate',
           { 'ds' => RubySaml::XML::DSIG }
-        )&.content&.strip
+        )&.text&.strip
         Base64.decode64(cert) if cert && !cert.empty?
       end
 

test/utils_test.rb

@@ -325,39 +325,35 @@ def result(duration, reference = 0)
       end
     end
 
-    describe '.element_text' do
+    describe 'Nokogiri::XML::Node#text' do
       it 'returns the element text' do
-        element = REXML::Document.new('<element>element text</element>').elements.first
-        assert_equal 'element text', RubySaml::Utils.element_text(element)
+        element = Nokogiri::XML('<element>element text</element>').root
+        assert_equal 'element text', element.text
       end
 
       it 'returns all segments of the element text' do
-        element = REXML::Document.new('<element>element <!-- comment -->text</element>').elements.first
-        assert_equal 'element text', RubySaml::Utils.element_text(element)
+        element = Nokogiri::XML('<element>element <!-- comment -->text</element>').root
+        assert_equal 'element text', element.text
       end
 
       it 'returns normalized element text' do
-        element = REXML::Document.new('<element>element &amp; text</element>').elements.first
-        assert_equal 'element & text', RubySaml::Utils.element_text(element)
+        element = Nokogiri::XML('<element>element &amp; text</element>').root
+        assert_equal 'element & text', element.text
       end
 
       it 'returns the CDATA element text' do
-        element = REXML::Document.new('<element><![CDATA[element & text]]></element>').elements.first
-        assert_equal 'element & text', RubySaml::Utils.element_text(element)
+        element = Nokogiri::XML('<element><![CDATA[element & text]]></element>').root
+        assert_equal 'element & text', element.text
       end
 
       it 'returns the element text with newlines and additional whitespace' do
-        element = REXML::Document.new("<element>  element \n text  </element>").elements.first
-        assert_equal "  element \n text  ", RubySaml::Utils.element_text(element)
-      end
-
-      it 'returns nil when element is nil' do
-        assert_nil RubySaml::Utils.element_text(nil)
+        element = Nokogiri::XML("<element>  element \n text  </element>").root
+        assert_equal "  element \n text  ", element.text
       end
 
       it 'returns empty string when element has no text' do
-        element = REXML::Document.new('<element></element>').elements.first
-        assert_equal '', RubySaml::Utils.element_text(element)
+        element = Nokogiri::XML('<element></element>').root
+        assert_equal '', element.text
       end
     end
   end