diff --git a/README.md b/README.md index 23f3066d..2ba2d898 100644 --- a/README.md +++ b/README.md @@ -929,22 +929,24 @@ or underscore, and can only contain letters, digits, underscores, hyphens, and p Some IdPs may require to add SPs to add additional fields (Organization, ContactPerson, etc.) into the SP metadata. This can be done by extending the `RubySaml::Metadata` class and -overriding the `#add_extras` method using a Nokogiri XML builder as per the following example: +overriding the `#add_extras` method where the first arg is a +[Nokogiri::XML::Builder](https://nokogiri.org/rdoc/Nokogiri/XML/Builder.html) object as per +the following example: ```ruby class MyMetadata < RubySaml::Metadata private def add_extras(xml, _settings) - xml['md'].Organization do - xml['md'].OrganizationName('ACME Inc.', 'xml:lang' => 'en-US') - xml['md'].OrganizationDisplayName('ACME', 'xml:lang' => 'en-US') - xml['md'].OrganizationURL('https://www.acme.com', 'xml:lang' => 'en-US') + xml.Organization do + xml.OrganizationName('xml:lang' => 'en-US') { xml.text 'ACME Inc.' } + xml.OrganizationDisplayName('xml:lang' => 'en-US') { xml.text 'ACME' } + xml.OrganizationURL('xml:lang' => 'en-US') { xml.text 'https://www.acme.com' } end - xml['md'].ContactPerson('contactType' => 'technical') do - xml['md'].GivenName('ACME SAML Team') - xml['md'].EmailAddress('saml@acme.com') + xml.ContactPerson('contactType' => 'technical') do + xml.GivenName { xml.text 'ACME SAML Team' } + xml.EmailAddress { xml.text 'saml@acme.com' } end end end diff --git a/UPGRADING.md b/UPGRADING.md index 61cc544d..7d422559 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -1,6 +1,6 @@ # Ruby SAML Migration Guide -## Updating from 1.x to 2.0.0 +## Upgrading from 1.x to 2.0.0 **IMPORTANT: Please read this section carefully as it contains breaking changes!** @@ -34,7 +34,7 @@ Note that the project folder structure has also been updated accordingly. Notabl `lib/onelogin/schemas` is now `lib/ruby_saml/schemas`. For backward compatibility, the alias `OneLogin = Object` has been set, so `OneLogin::RubySaml::` will still work -as before. This alias will be removed in RubySaml version `2.1.0`. +as before. This alias will be removed in RubySaml version `3.0.0`. ### Deprecation and removal of "XMLSecurity" namespace @@ -74,24 +74,33 @@ settings.security[:signature_method] = RubySaml::XML::RSA_SHA1 ### Replacement of REXML with Nokogiri RubySaml `1.x` used a combination of REXML and Nokogiri for XML parsing and generation. -In `2.0.0`, REXML has been replaced with Nokogiri. This change should be transparent -to most users, however, see note about Custom Metadata Fields below. +In `2.0.0`, REXML has been replaced with Nokogiri. As a result, there are minor differences +in how XML is generated, ncluding SAML requests and SP Metadata: + +1. All XML namespace declarations will be on the root node of the XML. Previously, + some declarations such as `xmlns:ds` were done on child nodes. +2. The ordering of attributes on each node may be different. + +These differences should not affect how the XML is parsed by various XML parsing libraries. +However, if you are strictly asserting that the generated XML is an exact string in your tests, +such tests may need to be adjusted accordingly. ### Custom Metadata Fields now use Nokogiri XML Builder If you have added custom fields to your SP metadata generation by overriding -the `RubySaml::Metadata#add_extras` method, you will need to update your code to use -[Nokogiri::XML::Builder](https://nokogiri.org/rdoc/Nokogiri/XML/Builder.html) format -instead of REXML. Here is an example of the new format: +the `RubySaml::Metadata#add_extras` method, you will need to update your code +so that the first arg of the method is a +[Nokogiri::XML::Builder](https://nokogiri.org/rdoc/Nokogiri/XML/Builder.html) +object. Here is an example of the new format: ```ruby class MyMetadata < RubySaml::Metadata private - def add_extras(xml, _settings) - xml['md'].ContactPerson('contactType' => 'technical') do - xml['md'].GivenName('ACME SAML Team') - xml['md'].EmailAddress('saml@acme.com') + def add_extras(builder, _settings) + builder.ContactPerson('contactType' => 'technical') do + builder.GivenName { builder.text 'ACME SAML Team' } + builder.EmailAddress { builder.text 'saml@acme.com' } end end end @@ -101,7 +110,7 @@ end RubySaml now always uses double quotes for attribute values when generating XML. The `settings.double_quote_xml_attribute_values` parameter now always behaves as `true`, -and will be removed in RubySaml 2.1.0. +and will be removed in RubySaml 3.0.0. The reasons for this change are: - RubySaml will use Nokogiri instead of REXML to generate XML. Nokogiri does not support @@ -154,7 +163,7 @@ a different `sp_uuid_prefix` is passed-in on subsequent calls. ### Deprecation of compression settings The `settings.compress_request` and `settings.compress_response` parameters have been deprecated -and are no longer functional. They will be removed in RubySaml 2.1.0. Please remove `compress_request` +and are no longer functional. They will be removed in RubySaml 3.0.0. Please remove `compress_request` and `compress_response` everywhere within your project code. The SAML SP request/response message compression behavior is now controlled automatically by the @@ -166,13 +175,15 @@ compression may be achieved by enabling `Content-Encoding: gzip` on your webserv ### Deprecation of IdP certificate fingerprint settings The `settings.idp_cert_fingerprint` and `settings.idp_cert_fingerprint_algorithm` are deprecated -and will be removed in RubySaml 2.1.0. Please use `settings.idp_cert` or `settings.idp_cert_multi` instead. -The reasons for this deprecation are that (1) fingerprint cannot be used with HTTP-Redirect binding, -and (2) fingerprint is theoretically susceptible to collision attacks. +and will be removed in RubySaml 3.0.0. Please use `settings.idp_cert` or `settings.idp_cert_multi` instead. + +The reasons for this deprecation are: +- Fingerprint cannot be used with HTTP-Redirect binding +- Fingerprint is theoretically susceptible to collision attacks. ### Other settings deprecations -The following parameters in `RubySaml::Settings` are deprecated and will be removed in RubySaml 2.1.0: +The following parameters in `RubySaml::Settings` are deprecated and will be removed in RubySaml 3.0.0: - `#issuer` is deprecated and replaced 1:1 by `#sp_entity_id` - `#idp_sso_target_url` is deprecated and replaced 1:1 by `#idp_sso_service_url` @@ -212,7 +223,7 @@ and `#format_private_key` methods. Specifically: stripped out. - Case 7: If no valid certificates are found, the entire original string will be returned. -## Updating from 1.17.x to 1.18.0 +## Upgrading from 1.17.x to 1.18.0 Version `1.18.0` changes the way the toolkit validates SAML signatures. There is a new order how validation happens in the toolkit and also the toolkit by default will check malformed doc @@ -222,7 +233,7 @@ The SignedDocument class defined at xml_security.rb experienced several changes. We don't expect compatibilty issues if you use the main methods offered by ruby-saml, but if you use a fork or customized usage, is possible that you need to adapt your code. -## Updating from 1.12.x to 1.13.0 +## Upgrading from 1.12.x to 1.13.0 Version `1.13.0` adds `settings.idp_sso_service_binding` and `settings.idp_slo_service_binding`, and deprecates `settings.security[:embed_sign]`. If specified, new binding parameters will be used in place of `:embed_sign` diff --git a/lib/ruby_saml.rb b/lib/ruby_saml.rb index b481a2ba..cefe824b 100644 --- a/lib/ruby_saml.rb +++ b/lib/ruby_saml.rb @@ -20,5 +20,5 @@ require 'ruby_saml/utils' require 'ruby_saml/version' -# @deprecated This alias adds compatibility with v1.x and will be removed in v2.1.0 +# @deprecated This alias adds compatibility with v1.x and will be removed in v3.0.0 OneLogin = Object diff --git a/lib/ruby_saml/idp_metadata_parser.rb b/lib/ruby_saml/idp_metadata_parser.rb index 07b5641a..666155c3 100644 --- a/lib/ruby_saml/idp_metadata_parser.rb +++ b/lib/ruby_saml/idp_metadata_parser.rb @@ -161,7 +161,7 @@ def parse_to_array(idp_metadata, options = {}) end def parse_to_idp_metadata_array(idp_metadata, options = {}) - @document = Nokogiri::XML(idp_metadata) + @document = Nokogiri::XML(idp_metadata) # TODO: RubySaml::XML.safe_load_nokogiri @options = options idpsso_descriptors = self.class.get_idps(@document, options[:entity_id]) @@ -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 diff --git a/lib/ruby_saml/logoutresponse.rb b/lib/ruby_saml/logoutresponse.rb index b4529649..86537974 100644 --- a/lib/ruby_saml/logoutresponse.rb +++ b/lib/ruby_saml/logoutresponse.rb @@ -41,7 +41,7 @@ def initialize(response, settings = nil, options = {}) @options = options @response = RubySaml::XML::Decoder.decode_message(response, @settings&.message_max_bytesize) - @document = RubySaml::XML::SignedDocument.new(@response) + @document = RubySaml::XML.safe_load_nokogiri(@response) super() end @@ -60,47 +60,35 @@ def success? # @return [String|nil] Gets the InResponseTo attribute from the Logout Response if exists. # def in_response_to - @in_response_to ||= begin - node = REXML::XPath.first( - document, - "/p:LogoutResponse", - { "p" => RubySaml::XML::NS_PROTOCOL } - ) - node.nil? ? nil : node.attributes['InResponseTo'] - end + @in_response_to ||= document.at_xpath( + "/p:LogoutResponse", + { "p" => RubySaml::XML::NS_PROTOCOL } + )&.[]('InResponseTo') end # @return [String] Gets the Issuer from the Logout Response. # def issuer - @issuer ||= begin - node = REXML::XPath.first( - document, - "/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. # def status_code - @status_code ||= begin - node = REXML::XPath.first(document, "/p:LogoutResponse/p:Status/p:StatusCode", { "p" => RubySaml::XML::NS_PROTOCOL }) - node.nil? ? nil : node.attributes["Value"] - end + @status_code ||= document.at_xpath( + "/p:LogoutResponse/p:Status/p:StatusCode", + { "p" => RubySaml::XML::NS_PROTOCOL } + )&.[]('Value') end def status_message - @status_message ||= begin - node = REXML::XPath.first( - document, - "/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 diff --git a/lib/ruby_saml/memoizable.rb b/lib/ruby_saml/memoizable.rb new file mode 100644 index 00000000..99e4fde4 --- /dev/null +++ b/lib/ruby_saml/memoizable.rb @@ -0,0 +1,66 @@ +# frozen_string_literal: true + +module RubySaml + # Mixin for memoizing methods + module Memoizable + # Creates a memoized method + # + # @param method_name [Symbol] the name of the method to memoize + # @param original_method [Symbol, nil] the original method to memoize (defaults to method_name) + def self.included(base) + base.extend(ClassMethods) + end + + private + + # Memoizes the result of a block using the given name as the cache key + # + # @param cache_key [Symbol, String] the name to use as the cache key + # @yield the block whose result will be cached + # @return [Object] the cached result or the result of the block + def memoize(cache_key) + cache_key = "@#{cache_key.to_s.delete_prefix('@')}" + return instance_variable_get(cache_key) if instance_variable_defined?(cache_key) + + instance_variable_set(cache_key, yield) + end + + # Class methods for memoization + module ClassMethods + # Defines multiple memoized methods + # + # @param method_names [Array] the names of the methods to memoize + # @raise [ArgumentError] if any method has an arity greater than 0 + def memoize_method(*method_names) + method_names.each do |method_name| + method_obj = instance_method(method_name) + + # Check method arity + if method_obj.arity > 0 # rubocop:disable Style/IfUnlessModifier + raise ArgumentError.new("Cannot memoize method '#{method_name}' with arity > 0") + end + + # Store the original method + original_method_name = "#{method_name}_without_memoization" + alias_method original_method_name, method_name + private original_method_name + + # Define the memoized version + define_method(method_name) do |&block| + cache_key = "@memoized_#{method_name}" + memoize(cache_key) do + send(original_method_name, &block) + end + end + + # Preserve method visibility + if private_method_defined?(original_method_name) + private method_name + elsif protected_method_defined?(original_method_name) + protected method_name + end + end + end + end + end +end diff --git a/lib/ruby_saml/response.rb b/lib/ruby_saml/response.rb index 131679f1..10bd6931 100644 --- a/lib/ruby_saml/response.rb +++ b/lib/ruby_saml/response.rb @@ -13,7 +13,8 @@ class Response < SamlMessage # TODO: Migrate this to RubySaml::XML SAML_NAMESPACES = { 'p' => RubySaml::XML::NS_PROTOCOL, - 'a' => RubySaml::XML::NS_ASSERTION + 'a' => RubySaml::XML::NS_ASSERTION, + 'ds' => RubySaml::XML::DSIG }.freeze # TODO: This should not be an accessor @@ -59,7 +60,7 @@ def initialize(response, options = {}) end @response = RubySaml::XML::Decoder.decode_message(response, @settings&.message_max_bytesize) - @document = RubySaml::XML::SignedDocument.new(@response, @errors) + @document = RubySaml::XML.safe_load_nokogiri(@response) if assertion_encrypted? @decrypted_document = generate_decrypted_document @@ -79,11 +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 ||= if name_id_node.is_a?(REXML::Element) - Utils.element_text(name_id_node) - else - name_id_node&.content - end + @name_id ||= name_id_node&.text end alias_method :nameid, :name_id @@ -91,11 +88,7 @@ def name_id # @return [String] the NameID Format provided by the SAML response from the IdP. # def name_id_format - @name_id_format ||= if name_id_node.is_a?(REXML::Element) - name_id_node&.attribute('Format')&.value - else - name_id_node&.[]('Format') - end + @name_id_format ||= name_id_node&.[]('Format') end alias_method :nameid_format, :name_id_format @@ -103,21 +96,13 @@ def name_id_format # @return [String] the NameID SPNameQualifier provided by the SAML response from the IdP. # def name_id_spnamequalifier - @name_id_spnamequalifier ||= if name_id_node.is_a?(REXML::Element) - name_id_node&.attribute('SPNameQualifier')&.value - else - name_id_node&.[]('SPNameQualifier') - end + @name_id_spnamequalifier ||= name_id_node&.[]('SPNameQualifier') end # @return [String] the NameID NameQualifier provided by the SAML response from the IdP. # def name_id_namequalifier - @name_id_namequalifier ||= if name_id_node.is_a?(REXML::Element) - name_id_node&.attribute('NameQualifier')&.value - else - name_id_node&.[]('NameQualifier') - end + @name_id_namequalifier ||= name_id_node&.[]('NameQualifier') end # Gets the SessionIndex from the AuthnStatement. @@ -127,10 +112,7 @@ def name_id_namequalifier # @return [String] SessionIndex Value # def sessionindex - @sessionindex ||= begin - node = xpath_first_from_signed_assertion('/a:AuthnStatement') - node.nil? ? nil : node.attributes['SessionIndex'] - end + @sessionindex ||= xpath_first_from_signed_assertion('/a:AuthnStatement')&.[]('SessionIndex') end # Gets the Attributes from the AttributeStatement element. @@ -160,12 +142,7 @@ def attributes else node = attr_element end - - if node.is_a?(Nokogiri::XML::Element) - name, values = handle_nokogiri_attribute(node, attributes) - else - name, values = handle_rexml_attribute(node, attributes) - end + name, values = handle_nokogiri_attribute(node, attributes) attributes.add(name, values) end end @@ -174,33 +151,6 @@ def attributes end end - def handle_rexml_attribute(node, attributes) - name = node.attributes["Name"] - - if options[:check_duplicated_attributes] && attributes.include?(name) - raise ValidationError.new("Found an Attribute element with duplicated Name") - end - - values = node.elements.map do |e| - if e.elements.nil? || 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.attributes['xsi:nil']) ? nil : Utils.element_text(e) - 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 - REXML::XPath.match(e,'a:NameID', { "a" => RubySaml::XML::NS_ASSERTION }).map do |n| - base_path = n.attributes['NameQualifier'] ? "#{n.attributes['NameQualifier']}/" : '' - "#{base_path}#{Utils.element_text(n)}" - end - end - end.flatten - - [name, values] - end - def handle_nokogiri_attribute(node, attributes) name = node['Name'] @@ -209,17 +159,17 @@ def handle_nokogiri_attribute(node, attributes) end values = node.elements.map do |e| - if e.elements.nil? || e.elements.empty? + 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 @@ -235,8 +185,8 @@ def handle_nokogiri_attribute(node, attributes) # def session_expires_at @expires_at ||= begin - node = xpath_first_from_signed_assertion('/a:AuthnStatement') - node.nil? ? nil : parse_time(node, "SessionNotOnOrAfter") + node = xpath_first_from_signed_assertion('/a:AuthnStatement') + parse_time(node, "SessionNotOnOrAfter") if node end end @@ -246,10 +196,7 @@ def session_expires_at # @return [String] AuthnInstant value # def authn_instant - @authn_instant ||= begin - node = xpath_first_from_signed_assertion('/a:AuthnStatement') - node.nil? ? nil : node.attributes['AuthnInstant'] - end + @authn_instant ||= xpath_first_from_signed_assertion('/a:AuthnStatement')&.[]('AuthnInstant') end # Gets the AuthnContextClassRef from the AuthnStatement @@ -258,44 +205,42 @@ def authn_instant # @return [String] AuthnContextClassRef value # def authn_context_class_ref - @authn_context_class_ref ||= Utils.element_text(xpath_first_from_signed_assertion('/a:AuthnStatement/a:AuthnContext/a:AuthnContextClassRef')) + @authn_context_class_ref ||= xpath_first_from_signed_assertion('/a:AuthnStatement/a:AuthnContext/a:AuthnContextClassRef')&.text end # Checks if the Status has the "Success" code # @return [Boolean] True if the StatusCode is Sucess # def success? - status_code == "urn:oasis:names:tc:SAML:2.0:status:Success" + status_code == 'urn:oasis:names:tc:SAML:2.0:status:Success' end # @return [String] StatusCode value from a SAML Response. # def status_code @status_code ||= begin - nodes = REXML::XPath.match( - document, - "/p:Response/p:Status/p:StatusCode", - { "p" => RubySaml::XML::NS_PROTOCOL } - ) - if nodes.size == 1 - node = nodes[0] - code = node.attributes["Value"] if node&.attributes - - unless code == "urn:oasis:names:tc:SAML:2.0:status:Success" - nodes = REXML::XPath.match( - document, - "/p:Response/p:Status/p:StatusCode/p:StatusCode", - { "p" => RubySaml::XML::NS_PROTOCOL } - ) - statuses = nodes.map do |inner_node| - inner_node.attributes["Value"] - end + nodes = document.xpath( + "/p:Response/p:Status/p:StatusCode", + { "p" => RubySaml::XML::NS_PROTOCOL } + ) + if nodes.size == 1 + node = nodes[0] + code = node['Value'] if node - code = [code, statuses].flatten.join(" | ") - end + unless code == "urn:oasis:names:tc:SAML:2.0:status:Success" + nodes = document.xpath( + "/p:Response/p:Status/p:StatusCode/p:StatusCode", + { "p" => RubySaml::XML::NS_PROTOCOL } + ) + statuses = nodes.map do |inner_node| + inner_node['Value'] + end - code - end + code = [code, statuses].flatten.join(" | ") + end + + code + end end end @@ -303,19 +248,18 @@ def status_code # def status_message @status_message ||= begin - nodes = REXML::XPath.match( - document, - "/p:Response/p:Status/p:StatusMessage", - { "p" => RubySaml::XML::NS_PROTOCOL } - ) + nodes = document.xpath( + "/p:Response/p:Status/p:StatusMessage", + { "p" => RubySaml::XML::NS_PROTOCOL } + ) - Utils.element_text(nodes.first) if nodes.size == 1 + nodes.first&.text if nodes.size == 1 end end # Gets the Condition Element of the SAML Response if exists. # (returns the first node that matches the supplied xpath) - # @return [REXML::Element] Conditions Element if exists + # @return [Nokogiri::XML::Element] Conditions Element if exists # def conditions @conditions ||= xpath_first_from_signed_assertion('/a:Conditions') @@ -337,56 +281,45 @@ def not_on_or_after # Gets the Issuers (from Response and Assertion). # (returns the first node that matches the supplied xpath from the Response and from the Assertion) - # @return [Array] Array with the Issuers (REXML::Element) + # @return [Array] Array with the Issuers (Nokogiri::XML::Element) # def issuers @issuers ||= begin - issuer_response_nodes = REXML::XPath.match( - document, - "/p:Response/a:Issuer", - SAML_NAMESPACES - ) + issuer_response_nodes = document.xpath( + "/p:Response/a:Issuer", + SAML_NAMESPACES + ) - unless issuer_response_nodes.size == 1 - error_msg = "Issuer of the Response not found or multiple." - raise ValidationError.new(error_msg) - end + unless issuer_response_nodes.size == 1 + error_msg = "Issuer of the Response not found or multiple." + raise ValidationError.new(error_msg) + end - issuer_assertion_nodes = xpath_from_signed_assertion("/a:Issuer") - unless issuer_assertion_nodes.size == 1 - error_msg = "Issuer of the Assertion not found or multiple." - raise ValidationError.new(error_msg) - end + issuer_assertion_nodes = xpath_from_signed_assertion("/a:Issuer") + unless issuer_assertion_nodes.size == 1 + error_msg = "Issuer of the Assertion not found or multiple." + raise ValidationError.new(error_msg) + end - nodes = issuer_response_nodes + issuer_assertion_nodes - nodes.filter_map { |node| Utils.element_text(node) }.uniq + nodes = issuer_response_nodes + issuer_assertion_nodes + nodes.map(&:text).reject(&:empty?).uniq end end # @return [String|nil] The InResponseTo attribute from the SAML Response. - # def in_response_to - @in_response_to ||= begin - node = REXML::XPath.first( - document, - "/p:Response", - { "p" => RubySaml::XML::NS_PROTOCOL } - ) - node.nil? ? nil : node.attributes['InResponseTo'] - end + @in_response_to ||= document.at_xpath( + "/p:Response", + { "p" => RubySaml::XML::NS_PROTOCOL } + )&.[]('InResponseTo') end # @return [String|nil] Destination attribute from the SAML Response. - # def destination - @destination ||= begin - node = REXML::XPath.first( - document, - "/p:Response", - { "p" => RubySaml::XML::NS_PROTOCOL } - ) - node.nil? ? nil : node.attributes['Destination'] - end + @destination ||= document.at_xpath( + "/p:Response", + { "p" => RubySaml::XML::NS_PROTOCOL } + )&.[]('Destination') end # @return [Array] The Audience elements from the Contitions of the SAML Response. @@ -394,7 +327,7 @@ def destination def audiences @audiences ||= begin nodes = xpath_from_signed_assertion('/a:Conditions/a:AudienceRestriction/a:Audience') - nodes.map { |node| Utils.element_text(node) }.reject(&:empty?) + nodes.map(&:text).reject(&:empty?) end end @@ -408,9 +341,8 @@ def allowed_clock_drift # @return [Boolean] True if the SAML Response contains an EncryptedAssertion element # def assertion_encrypted? - !REXML::XPath.first( - document, - "(/p:Response/EncryptedAssertion/)|(/p:Response/a:EncryptedAssertion/)", + !document.at_xpath( + "/p:Response/EncryptedAssertion | /p:Response/a:EncryptedAssertion", SAML_NAMESPACES ).nil? end @@ -421,8 +353,8 @@ def response_id def assertion_id @assertion_id ||= begin - node = xpath_first_from_signed_assertion("") - node.nil? ? nil : node.attributes['ID'] + node = xpath_first_from_signed_assertion('') + node.nil? ? nil : node['ID'] end end @@ -537,13 +469,11 @@ def validate_version # def validate_num_assertion error_msg = "SAML Response must contain 1 assertion" - assertions = REXML::XPath.match( - document, + assertions = document.xpath( "//a:Assertion", { "a" => RubySaml::XML::NS_ASSERTION } ) - encrypted_assertions = REXML::XPath.match( - document, + encrypted_assertions = document.xpath( "//a:EncryptedAssertion", { "a" => RubySaml::XML::NS_ASSERTION } ) @@ -553,8 +483,7 @@ def validate_num_assertion end unless decrypted_document.nil? - assertions = REXML::XPath.match( - decrypted_document, + assertions = decrypted_document.xpath( "//a:Assertion", { "a" => RubySaml::XML::NS_ASSERTION } ) @@ -589,8 +518,7 @@ def validate_no_duplicated_attributes # an are a Response or an Assertion Element, otherwise False if soft=True # def validate_signed_elements - signature_nodes = REXML::XPath.match( - decrypted_document.nil? ? document : decrypted_document, + signature_nodes = (decrypted_document.nil? ? document : decrypted_document).xpath( "//ds:Signature", { "ds" => RubySaml::XML::DSIG } ) @@ -603,22 +531,22 @@ def validate_signed_elements return append_error("Invalid Signature Element '#{signed_element}'. SAML Response rejected") end - if signature_node.parent.attributes['ID'].nil? + if signature_node.parent['ID'].nil? return append_error("Signed Element must contain an ID. SAML Response rejected") end - id = signature_node.parent.attributes.get_attribute("ID").value + id = signature_node.parent['ID'] if verified_ids.include?(id) return append_error("Duplicated ID. SAML Response rejected") end verified_ids.push(id) # Check that reference URI matches the parent ID and no duplicate References or IDs - ref = REXML::XPath.first(signature_node, ".//ds:Reference", { "ds" => RubySaml::XML::DSIG }) + ref = signature_node.at_xpath(".//ds:Reference", { "ds" => RubySaml::XML::DSIG }) if ref - uri = ref.attributes.get_attribute("URI") - if uri && !uri.value.empty? - sei = uri.value[1..] + uri = ref['URI'] + if uri && !uri.empty? + sei = uri[1..] unless sei == id return append_error("Found an invalid Signed Element. SAML Response rejected") @@ -821,42 +749,6 @@ def validate_subject_confirmation valid_subject_confirmation = false subject_confirmation_nodes = xpath_from_signed_assertion('/a:Subject/a:SubjectConfirmation') - return validate_subject_confirmation_nokogiri(subject_confirmation_nodes) if subject_confirmation_nodes.first.is_a?(Nokogiri::XML::Element) - - now = Time.now.utc - subject_confirmation_nodes.each do |subject_confirmation| - if subject_confirmation.attributes.include? "Method" and subject_confirmation.attributes['Method'] != 'urn:oasis:names:tc:SAML:2.0:cm:bearer' - next - end - - confirmation_data_node = REXML::XPath.first( - subject_confirmation, - 'a:SubjectConfirmationData', - { "a" => RubySaml::XML::NS_ASSERTION } - ) - - next unless confirmation_data_node - - attrs = confirmation_data_node.attributes - next if (attrs.include? "InResponseTo" and attrs['InResponseTo'] != in_response_to) || - (attrs.include? "NotBefore" and now < (parse_time(confirmation_data_node, "NotBefore") - allowed_clock_drift)) || - (attrs.include? "NotOnOrAfter" and now >= (parse_time(confirmation_data_node, "NotOnOrAfter") + allowed_clock_drift)) || - (attrs.include? "Recipient" and !options[:skip_recipient_check] and settings and attrs['Recipient'] != settings.assertion_consumer_service_url) - - valid_subject_confirmation = true - break - end - - unless valid_subject_confirmation - error_msg = "A valid SubjectConfirmation was not found on this Response" - return append_error(error_msg) - end - - true - end - - def validate_subject_confirmation_nokogiri(subject_confirmation_nodes) - valid_subject_confirmation = false now = Time.now.utc subject_confirmation_nodes.each do |subject_confirmation| @@ -905,22 +797,20 @@ 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 - sig_elements = REXML::XPath.match( - document, + # 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 + + sig_elements = document.xpath( "/p:Response[@ID=$id]/ds:Signature", { "p" => RubySaml::XML::NS_PROTOCOL, "ds" => RubySaml::XML::DSIG }, - { 'id' => document.signed_element_id } + id: subject_id ) use_original = sig_elements.size == 1 || decrypted_document.nil? - doc = use_original ? document : decrypted_document - unless doc.processed - doc.cache_referenced_xml(@soft, check_malformed_doc: check_malformed_doc_enabled?) - end - - doc + use_original ? document : decrypted_document end # Validates the Signature @@ -929,31 +819,34 @@ def doc_to_validate # def validate_signature error_msg = "Invalid Signature on SAML Response" - doc = doc_to_validate - sig_elements = REXML::XPath.match( - document, - "/p:Response[@ID=$id]/ds:Signature", - { "p" => RubySaml::XML::NS_PROTOCOL, "ds" => RubySaml::XML::DSIG }, - { 'id' => document.signed_element_id } - ) + # TODO: document vs doc is super confusing + subject_id = RubySaml::XML::SignedDocumentValidator.subject_id(document) + sig_elements = [] + if subject_id + sig_elements = document.xpath( + "/p:Response[@ID=$id]/ds:Signature", + { "p" => RubySaml::XML::NS_PROTOCOL, "ds" => RubySaml::XML::DSIG }, + id: subject_id + ) + end # Check signature node inside assertion - if !sig_elements || sig_elements.empty? - sig_elements = REXML::XPath.match( - doc, + if sig_elements.empty? + subject_id2 = RubySaml::XML::SignedDocumentValidator.subject_id(doc) + sig_elements = doc.xpath( "/p:Response/a:Assertion[@ID=$id]/ds:Signature", - SAML_NAMESPACES.merge({ "ds" => RubySaml::XML::DSIG }), - { 'id' => doc.signed_element_id } + SAML_NAMESPACES, + id: subject_id2 ) end if sig_elements.size != 1 if sig_elements.empty? - append_error("Signed element id ##{doc.signed_element_id} is not found") + append_error("Signed element id ##{subject_id} is not found") else - append_error("Signed element id ##{doc.signed_element_id} is found more than once") + append_error("Signed element id ##{subject_id} is found more than once") end return append_error(error_msg) end @@ -969,7 +862,7 @@ def validate_signature fingerprint_alg: settings.idp_cert_fingerprint_algorithm } - if fingerprint && doc.validate_document(fingerprint, @soft, opts) + if fingerprint && RubySaml::XML::SignedDocumentValidator.validate_document(doc, fingerprint, @errors, soft: @soft, **opts).is_a?(TrueClass) # TODO: DANGEROUS if settings.security[:check_idp_cert_expiration] && RubySaml::Utils.is_cert_expired(idp_cert) return append_error("IdP x509 certificate expired") end @@ -980,7 +873,7 @@ def validate_signature valid = false expired = false idp_certs[:signing].each do |idp_cert| - valid = doc.validate_document_with_cert(idp_cert, true) + valid = RubySaml::XML::SignedDocumentValidator.validate_document_with_cert(doc, idp_cert, @errors, soft: @soft).is_a?(TrueClass) # TODO: DANGEROUS next unless valid if settings.security[:check_idp_cert_expiration] && RubySaml::Utils.is_cert_expired(idp_cert) @@ -1019,25 +912,30 @@ def name_id_node end def cached_signed_assertion - xml = doc_to_validate.referenced_xml - empty_doc = REXML::Document.new + empty_doc = Nokogiri::XML::Document.new + xml = doc_to_validate + return empty_doc if xml.nil? + + subject = RubySaml::XML::SignedDocumentValidator.subject_node(xml) return empty_doc if xml.nil? # when no signature/reference is found, return empty document - root = REXML::Document.new(xml).root - if root["ID"] != doc_to_validate.signed_element_id + subject_id = RubySaml::XML::SignedDocumentValidator.subject_id(xml) + return nil unless subject_id + + if subject['ID'] != subject_id return empty_doc end assertion = empty_doc - if root.name == "Response" - if REXML::XPath.first(root, "a:Assertion", {"a" => RubySaml::XML::NS_ASSERTION}) - assertion = REXML::XPath.first(root, "a:Assertion", {"a" => RubySaml::XML::NS_ASSERTION}) - elsif REXML::XPath.first(root, "a:EncryptedAssertion", {"a" => RubySaml::XML::NS_ASSERTION}) - assertion = RubySaml::XML::Decryptor.decrypt_assertion(REXML::XPath.first(root, "a:EncryptedAssertion", {"a" => RubySaml::XML::NS_ASSERTION}), settings&.get_sp_decryption_keys) + if subject.name == "Response" + if (result = subject.at_xpath("a:Assertion", {"a" => RubySaml::XML::NS_ASSERTION})) + assertion = result + elsif (result = subject.at_xpath("a:EncryptedAssertion", {"a" => RubySaml::XML::NS_ASSERTION})) + assertion = RubySaml::XML::Decryptor.decrypt_assertion(result, settings&.get_sp_decryption_keys) end - elsif root.name == "Assertion" - assertion = root + elsif subject.name == "Assertion" + assertion = subject end assertion @@ -1047,59 +945,38 @@ def signed_assertion @signed_assertion ||= cached_signed_assertion end - # Extracts the first appearance that matchs the subelt (pattern) + # Extracts the first appearance that matchs the subpath (pattern) # Search on any Assertion that is signed, or has a Response parent signed - # @param subelt [String] The XPath pattern - # @return [REXML::Element | nil] If any matches, return the Element - # - def xpath_first_from_signed_assertion(subelt = nil) - doc = signed_assertion - if doc.is_a?(REXML::Element) - REXML::XPath.first( - doc, - "./#{subelt}", - SAML_NAMESPACES - ) - else - return if !subelt || subelt.empty? - doc.at_xpath("./#{subelt}", SAML_NAMESPACES) - end + # @param subpath [String] The XPath pattern + # @return [Nokogiri::XML::Element | nil] If any matches, return the Element + def xpath_first_from_signed_assertion(subpath = nil) + return if !subpath || subpath.empty? + signed_assertion.at_xpath("./#{subpath}", SAML_NAMESPACES) end - # Extracts all the appearances that matchs the subelt (pattern) + # Extracts all the appearances that matchs the subpath (pattern) # Search on any Assertion that is signed, or has a Response parent signed - # @param subelt [String] The XPath pattern - # @return [Array of REXML::Element] Return all matches - # - def xpath_from_signed_assertion(subelt = nil) - doc = signed_assertion - if doc.is_a?(REXML::Element) - REXML::XPath.match( - doc, - "./#{subelt}", - SAML_NAMESPACES - ) - else - return if !subelt || subelt.empty? - doc.xpath("./#{subelt}", SAML_NAMESPACES) - end + # @param subpath [String] The XPath pattern + # @return [Array of Nokogiri::XML::Element] Return all matches + def xpath_from_signed_assertion(subpath = nil) + return if !subpath || subpath.empty? + signed_assertion.xpath("./#{subpath}", SAML_NAMESPACES) end # Generates the decrypted_document # @return [RubySaml::XML::SignedDocument] The SAML Response with the assertion decrypted # def generate_decrypted_document - noko = RubySaml::XML::Decryptor.decrypt_document(document.to_s, settings&.get_sp_decryption_keys) - RubySaml::XML::SignedDocument.new(noko.to_xml(save_with: Nokogiri::XML::Node::SaveOptions::AS_XML)) + RubySaml::XML::Decryptor.decrypt_document(document, settings&.get_sp_decryption_keys) end # Parse the attribute of a given node in Time format - # @param node [REXML:Element] The node + # @param node [Nokogiri::XML::Element] The node # @param attribute [String] The attribute name # @return [Time|nil] The parsed value # def parse_time(node, attribute) - return unless (value = node&.attributes&.[](attribute)) + return unless (value = node&.[](attribute)) Time.parse(value) end diff --git a/lib/ruby_saml/saml_message.rb b/lib/ruby_saml/saml_message.rb index 52c9a137..c9ead9e4 100644 --- a/lib/ruby_saml/saml_message.rb +++ b/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. diff --git a/lib/ruby_saml/settings.rb b/lib/ruby_saml/settings.rb index 63e5d1f9..c9351244 100644 --- a/lib/ruby_saml/settings.rb +++ b/lib/ruby_saml/settings.rb @@ -248,13 +248,13 @@ def get_binding(value) { double_quote_xml_attribute_values: true }.each do |old_param, new_value| - # @deprecated Will be removed in v2.1.0 + # @deprecated Will be removed in v3.0.0 define_method(old_param) do removed_deprecation(old_param, new_value) new_value end - # @deprecated Will be removed in v2.1.0 + # @deprecated Will be removed in v3.0.0 define_method(:"#{old_param}=") do |_| removed_deprecation(old_param, new_value) new_value @@ -268,13 +268,13 @@ def get_binding(value) assertion_consumer_logout_service_url: :single_logout_service_url, assertion_consumer_logout_service_binding: :single_logout_service_binding }.each do |old_param, new_param| - # @deprecated Will be removed in v2.1.0 + # @deprecated Will be removed in v3.0.0 define_method(old_param) do replaced_deprecation(old_param, new_param) send(new_param) end - # @deprecated Will be removed in v2.1.0 + # @deprecated Will be removed in v3.0.0 define_method(:"#{old_param}=") do |value| replaced_deprecation(old_param, new_param) send(:"#{new_param}=", value) @@ -318,49 +318,49 @@ def get_sp_digest_method end end - # @deprecated Will be removed in v2.1.0 + # @deprecated Will be removed in v3.0.0 def idp_cert_fingerprint=(value) idp_cert_fingerprint_deprecation @idp_cert_fingerprint = value end - # @deprecated Will be removed in v2.1.0 + # @deprecated Will be removed in v3.0.0 def idp_cert_fingerprint_algorithm=(value) idp_cert_fingerprint_deprecation @idp_cert_fingerprint_algorithm = value end - # @deprecated Will be removed in v2.1.0 + # @deprecated Will be removed in v3.0.0 def certificate_new certificate_new_deprecation @certificate_new end - # @deprecated Will be removed in v2.1.0 + # @deprecated Will be removed in v3.0.0 def certificate_new=(value) certificate_new_deprecation @certificate_new = value end - # @deprecated Will be removed in v2.1.0 + # @deprecated Will be removed in v3.0.0 def compress_request compress_deprecation('compress_request', 'idp_sso_service_binding') defined?(@compress_request) ? @compress_request : true end - # @deprecated Will be removed in v2.1.0 + # @deprecated Will be removed in v3.0.0 def compress_request=(value) compress_deprecation('compress_request', 'idp_sso_service_binding') @compress_request = value end - # @deprecated Will be removed in v2.1.0 + # @deprecated Will be removed in v3.0.0 def compress_response compress_deprecation('compress_response', 'idp_slo_service_binding') defined?(@compress_response) ? @compress_response : true end - # @deprecated Will be removed in v2.1.0 + # @deprecated Will be removed in v3.0.0 def compress_response=(value) compress_deprecation('compress_response', 'idp_slo_service_binding') @compress_response = value @@ -368,36 +368,36 @@ def compress_response=(value) private - # @deprecated Will be removed in v2.1.0 + # @deprecated Will be removed in v3.0.0 def removed_deprecation(old_param, new_value) - Logging.deprecate "`RubySaml::Settings##{old_param}` is deprecated and will be removed in RubySaml 2.1.0. " \ + Logging.deprecate "`RubySaml::Settings##{old_param}` is deprecated and will be removed in RubySaml 3.0.0. " \ "It no longer has any effect, and will behave as if always set to #{new_value.inspect}." end - # @deprecated Will be removed in v2.1.0 + # @deprecated Will be removed in v3.0.0 def replaced_deprecation(old_param, new_param) - Logging.deprecate "`RubySaml::Settings##{old_param}` is deprecated and will be removed in RubySaml 2.1.0. " \ + Logging.deprecate "`RubySaml::Settings##{old_param}` is deprecated and will be removed in RubySaml 3.0.0. " \ "Please set the same value to `RubySaml::Settings##{new_param}` instead." end - # @deprecated Will be removed in v2.1.0 + # @deprecated Will be removed in v3.0.0 def idp_cert_fingerprint_deprecation Logging.deprecate '`RubySaml::Settings#idp_cert_fingerprint` and `#idp_cert_fingerprint_algorithm` are ' \ - 'deprecated and will be removed in RubySaml v2.1.0. Please provide the full IdP certificate in ' \ + 'deprecated and will be removed in RubySaml v3.0.0. Please provide the full IdP certificate in ' \ '`RubySaml::Settings#idp_cert` instead.' end - # @deprecated Will be removed in v2.1.0 + # @deprecated Will be removed in v3.0.0 def certificate_new_deprecation - Logging.deprecate '`RubySaml::Settings#certificate_new` is deprecated and will be removed in RubySaml v2.1.0. ' \ + Logging.deprecate '`RubySaml::Settings#certificate_new` is deprecated and will be removed in RubySaml v3.0.0. ' \ 'Please set `RubySaml::Settings#sp_cert_multi` instead. ' \ 'Please refer to documentation as `sp_cert_multi` has a different value type.' end - # @deprecated Will be removed in v2.1.0 + # @deprecated Will be removed in v3.0.0 def compress_deprecation(old_param, new_param) Logging.deprecate "`RubySaml::Settings##{old_param}` is deprecated and no longer functional. " \ - 'It will be removed in RubySaml 2.1.0. ' \ + 'It will be removed in RubySaml 3.0.0. ' \ "Its functionality is now handled by `RubySaml::Settings##{new_param}` instead: " \ '"HTTP-Redirect" will always be compressed, and "HTTP-POST" will always be uncompressed.' end diff --git a/lib/ruby_saml/slo_logoutrequest.rb b/lib/ruby_saml/slo_logoutrequest.rb index f7968ac5..bcfac86d 100644 --- a/lib/ruby_saml/slo_logoutrequest.rb +++ b/lib/ruby_saml/slo_logoutrequest.rb @@ -40,7 +40,7 @@ def initialize(request, options = {}) end @request = RubySaml::XML::Decoder.decode_message(request, @settings&.message_max_bytesize) - @document = REXML::Document.new(@request) + @document = RubySaml::XML.safe_load_nokogiri(@request) super() end @@ -58,31 +58,23 @@ def is_valid?(collect_errors = false) # @return [String] The NameID of the Logout Request. def name_id - @name_id ||= if name_id_node.is_a?(REXML::Element) - Utils.element_text(name_id_node) - else - name_id_node&.content - end + @name_id ||= name_id_node&.text end alias_method :nameid, :name_id # @return [String] The NameID Format of the Logout Request. def name_id_format - @name_id_format ||= if name_id_node.is_a?(REXML::Element) - name_id_node&.attribute('Format')&.value - else - name_id_node&.[]('Format') - end + @name_id_format ||= name_id_node&.[]('Format') end alias_method :nameid_format, :name_id_format def name_id_node @name_id_node ||= begin - encrypted_node = REXML::XPath.first(document, "/p:LogoutRequest/a:EncryptedID", { "p" => RubySaml::XML::NS_PROTOCOL, "a" => RubySaml::XML::NS_ASSERTION }) + encrypted_node = document.at_xpath("/p:LogoutRequest/a:EncryptedID", { "p" => RubySaml::XML::NS_PROTOCOL, "a" => RubySaml::XML::NS_ASSERTION }) if encrypted_node RubySaml::XML::Decryptor.decrypt_nameid(encrypted_node, settings&.get_sp_decryption_keys) else - REXML::XPath.first(document, "/p:LogoutRequest/a:NameID", { "p" => RubySaml::XML::NS_PROTOCOL, "a" => RubySaml::XML::NS_ASSERTION }) + document.at_xpath("/p:LogoutRequest/a:NameID", { "p" => RubySaml::XML::NS_PROTOCOL, "a" => RubySaml::XML::NS_ASSERTION }) end end end @@ -96,22 +88,17 @@ def id # @return [String] Gets the Issuer from the Logout Request. # def issuer - @issuer ||= begin - node = REXML::XPath.first( - document, - "/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. # def not_on_or_after @not_on_or_after ||= begin - node = REXML::XPath.first( - document, + node = document.at_xpath( "/p:LogoutRequest", { "p" => RubySaml::XML::NS_PROTOCOL } ) @@ -125,13 +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 = REXML::XPath.match( - document, + document.xpath( "/p:LogoutRequest/p:SessionIndex", { "p" => RubySaml::XML::NS_PROTOCOL } - ) - - nodes.map { |node| Utils.element_text(node) } + ).map(&:text) end private diff --git a/lib/ruby_saml/utils.rb b/lib/ruby_saml/utils.rb index 4ccaaee3..45b2d3bc 100644 --- a/lib/ruby_saml/utils.rb +++ b/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) diff --git a/lib/ruby_saml/xml.rb b/lib/ruby_saml/xml.rb index 177c6c5b..5fd47bf3 100644 --- a/lib/ruby_saml/xml.rb +++ b/lib/ruby_saml/xml.rb @@ -1,6 +1,5 @@ # frozen_string_literal: true -require 'rexml/element' require 'openssl' require 'nokogiri' require 'digest/sha1' @@ -50,30 +49,54 @@ module XML NOKOGIRI_OPTIONS = Nokogiri::XML::ParseOptions::STRICT | Nokogiri::XML::ParseOptions::NONET + # TODO: safe_load_message (rename safe_load_nokogiri --> safe_load_xml) + # def safe_load_message(message, check_malformed_doc: true) + # message = Decoder.decode(message) + # begin + # safe_load_nokogiri(message, check_malformed_doc: check_malformed_doc) + # rescue RubySaml::Errors::XMLLoadError + # Nokogiri::XML::Document.new + # end + # end + # 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 self.safe_load_nokogiri(document, check_malformed_doc: true) + def safe_load_nokogiri(document, check_malformed_doc: true) doc_str = document.to_s - raise StandardError.new('Dangerous XML detected. No Doctype nodes allowed') if doc_str.include?(' e + error ||= e + # raise StandardError.new(e.message) end - rescue StandardError => e - raise StandardError.new(e.message) end - raise StandardError.new('Dangerous XML detected. No Doctype nodes allowed') if xml.internal_subset - - raise StandardError.new("There were XML errors when parsing: #{xml.errors}") if check_malformed_doc && !xml.errors.empty? + # 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? + end + return Nokogiri::XML::Document.new if error || !xml xml end + def copy_nokogiri(noko) + Nokogiri::XML(noko.to_xml(save_with: Nokogiri::XML::Node::SaveOptions::AS_XML)) do |config| + config.options = NOKOGIRI_OPTIONS + end + end + # Lookup XML canonicalization algorithm. # @api private def canon_algorithm(element, default: true) @@ -132,8 +155,6 @@ def hash_algorithm(element) def get_algorithm_attr(element) if element.is_a?(Nokogiri::XML::Element) element['Algorithm'] - elsif element.is_a?(REXML::Element) - element.attribute('Algorithm').value elsif element element end @@ -144,5 +165,6 @@ def get_algorithm_attr(element) require 'ruby_saml/xml/decoder' require 'ruby_saml/xml/decryptor' require 'ruby_saml/xml/document_signer' -require 'ruby_saml/xml/signed_document' +require 'ruby_saml/xml/signed_document_info' +require 'ruby_saml/xml/signed_document_validator' require 'ruby_saml/xml/deprecated' diff --git a/lib/ruby_saml/xml/decryptor.rb b/lib/ruby_saml/xml/decryptor.rb index 3f78af82..e481c8af 100644 --- a/lib/ruby_saml/xml/decryptor.rb +++ b/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( @@ -72,9 +65,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' %() else diff --git a/lib/ruby_saml/xml/deprecated.rb b/lib/ruby_saml/xml/deprecated.rb index a853f826..3e25ef00 100644 --- a/lib/ruby_saml/xml/deprecated.rb +++ b/lib/ruby_saml/xml/deprecated.rb @@ -7,7 +7,7 @@ REXML::Security.entity_expansion_limit = 0 module XMLSecurity - # @deprecated Will be removed in v2.1.0. + # @deprecated Will be removed in v3.0.0. # @api private class BaseDocument < REXML::Document # @deprecated Constants @@ -15,22 +15,22 @@ class BaseDocument < REXML::Document DSIG = RubySaml::XML::DSIG NOKOGIRI_OPTIONS = RubySaml::XML::NOKOGIRI_OPTIONS - # @deprecated Will be removed in v2.1.0. + # @deprecated Will be removed in v3.0.0. def canon_algorithm(algorithm) - RubySaml::Logging.deprecate 'XMLSecurity::BaseDocument#canon_algorithm is deprecated and will be removed in v2.1.0. ' \ + RubySaml::Logging.deprecate 'XMLSecurity::BaseDocument#canon_algorithm is deprecated and will be removed in v3.0.0. ' \ 'Use RubySaml::XML.canon_algorithm instead.' RubySaml::XML.canon_algorithm(algorithm) end - # @deprecated Will be removed in v2.1.0. + # @deprecated Will be removed in v3.0.0. def algorithm(algorithm) - RubySaml::Logging.deprecate 'XMLSecurity::BaseDocument#algorithm is deprecated and will be removed in v2.1.0. ' \ + RubySaml::Logging.deprecate 'XMLSecurity::BaseDocument#algorithm is deprecated and will be removed in v3.0.0. ' \ 'Use RubySaml::XML.hash_algorithm instead.' RubySaml::XML.hash_algorithm(algorithm) end end - # @deprecated Will be removed in v2.1.0. + # @deprecated Will be removed in v3.0.0. # @api private class Document < BaseDocument # @deprecated Constants @@ -54,14 +54,14 @@ class Document < BaseDocument SHA512 = RubySaml::XML::SHA512 ENVELOPED_SIG = RubySaml::XML::ENVELOPED_SIG - # @deprecated Will be removed in v2.1.0. + # @deprecated Will be removed in v3.0.0. def initialize(*args, **_kwargs) - RubySaml::Logging.deprecate 'XMLSecurity::Document is deprecated and will be removed in v2.1.0. ' \ + RubySaml::Logging.deprecate 'XMLSecurity::Document is deprecated and will be removed in v3.0.0. ' \ 'Use RubySaml::XML::DocumentSigner.sign_document instead.' super(args[0]) end - # @deprecated Will be removed in v2.1.0. + # @deprecated Will be removed in v3.0.0. def sign_document(*_args, **_kwargs) msg = 'XMLSecurity::Document#sign_document has been removed. ' \ 'Use RubySaml::XML::DocumentSigner.sign_document instead.' @@ -69,24 +69,24 @@ def sign_document(*_args, **_kwargs) end end - # @deprecated Will be removed in v2.1.0. + # @deprecated Will be removed in v3.0.0. # @api private class SignedDocument < BaseDocument - # @deprecated Will be removed in v2.1.0. + # @deprecated Will be removed in v3.0.0. def initialize(*args, **_kwargs) - RubySaml::Logging.deprecate 'XMLSecurity::SignedDocument is deprecated and will be removed in v2.1.0.' \ + RubySaml::Logging.deprecate 'XMLSecurity::SignedDocument is deprecated and will be removed in v3.0.0.' \ 'Use RubySaml::XML::SignedDocumentValidator.validate_document instead.' super(args[0]) end - # @deprecated Will be removed in v2.1.0. + # @deprecated Will be removed in v3.0.0. def validate_document(*_args, **_kwargs) msg = 'XMLSecurity::SignedDocument#validate_document has been removed. ' \ 'Use RubySaml::XML::SignedDocumentValidator.validate_document instead.' raise ::NoMethodError.new(msg) end - # @deprecated Will be removed in v2.1.0. + # @deprecated Will be removed in v3.0.0. def extract_inclusive_namespaces msg = 'XMLSecurity::SignedDocument#extract_inclusive_namespaces has been removed.' raise ::NoMethodError.new(msg) diff --git a/lib/ruby_saml/xml/document_signer.rb b/lib/ruby_saml/xml/document_signer.rb index 9e559d29..823b4d61 100644 --- a/lib/ruby_saml/xml/document_signer.rb +++ b/lib/ruby_saml/xml/document_signer.rb @@ -8,6 +8,9 @@ module DocumentSigner INC_PREFIX_LIST = '#default samlp saml ds xs xsi md' + # Returns a copy of the document with a signature added. + # + # @example The Signature is added following the Issuer node. # # # @@ -23,7 +26,6 @@ module DocumentSigner # # # - # Returns a copy of the document with a signature added. def sign_document(document, private_key, certificate, signature_method = RubySaml::XML::RSA_SHA256, digest_method = RubySaml::XML::SHA256) noko = RubySaml::XML.safe_load_nokogiri(document.to_s) @@ -32,14 +34,14 @@ def sign_document(document, private_key, certificate, signature_method = RubySam # Modifies an existing Nokogiri document to add a signature. def sign_document!(noko, private_key, certificate, signature_method = RubySaml::XML::RSA_SHA256, digest_method = RubySaml::XML::SHA256) - signature_element = build_signature_element(noko, private_key, certificate, signature_method, digest_method) - issuer_element = noko.at_xpath('//saml:Issuer', 'saml' => RubySaml::XML::NS_ASSERTION) - if issuer_element - issuer_element.after(signature_element) + signature_node = build_signature_node(noko, private_key, certificate, signature_method, digest_method) + + if (issuer_node = noko.at_xpath('//saml:Issuer', 'saml' => RubySaml::XML::NS_ASSERTION)) + issuer_node.after(signature_node) elsif noko.root.children.any? - noko.root.children.first.before(signature_element) + noko.root.children.first.before(signature_node) else - noko.root.add_child(signature_element) + noko.root.add_child(signature_node) end noko @@ -47,8 +49,8 @@ def sign_document!(noko, private_key, certificate, signature_method = RubySaml:: private - def build_signature_element(noko, private_key, certificate, signature_method, digest_method) - signature_element = Nokogiri::XML::Builder.new do |xml| + def build_signature_node(noko, private_key, certificate, signature_method, digest_method) + signature_node = Nokogiri::XML::Builder.new do |xml| xml['ds'].Signature('xmlns:ds' => RubySaml::XML::DSIG) do xml['ds'].SignedInfo do xml['ds'].CanonicalizationMethod(Algorithm: RubySaml::XML::C14N) @@ -77,11 +79,11 @@ def build_signature_element(noko, private_key, certificate, signature_method, di end.doc.root # Set the signature value - signed_info_element = signature_element.at_xpath('//ds:SignedInfo', 'ds' => RubySaml::XML::DSIG) - sig_value_element = signature_element.at_xpath('//ds:SignatureValue', 'ds' => RubySaml::XML::DSIG) - sig_value_element.content = signature_value(signed_info_element, private_key, signature_method) + signed_info_node = signature_node.at_xpath('//ds:SignedInfo', 'ds' => RubySaml::XML::DSIG) + signature_value_node = signature_node.at_xpath('//ds:SignatureValue', 'ds' => RubySaml::XML::DSIG) + signature_value_node.content = signature_value(signed_info_node, private_key, signature_method) - signature_element + signature_node end def digest_value(document, digest_method) @@ -93,11 +95,11 @@ def digest_value(document, digest_method) Base64.strict_encode64(hash_algorithm.digest(canon_doc)) end - def signature_value(signed_info_element, private_key, signature_method) + def signature_value(signed_info_node, private_key, signature_method) canon_algorithm = RubySaml::XML.canon_algorithm(RubySaml::XML::C14N) hash_algorithm = RubySaml::XML.hash_algorithm(signature_method).new - canon_string = signed_info_element.canonicalize(canon_algorithm) + canon_string = signed_info_node.canonicalize(canon_algorithm) Base64.strict_encode64(private_key.sign(hash_algorithm, canon_string)) end diff --git a/lib/ruby_saml/xml/signed_document.rb b/lib/ruby_saml/xml/signed_document.rb deleted file mode 100644 index 18a4026b..00000000 --- a/lib/ruby_saml/xml/signed_document.rb +++ /dev/null @@ -1,273 +0,0 @@ -# frozen_string_literal: true - -require 'rexml/document' -require 'rexml/security' -require 'rexml/xpath' -require 'ruby_saml/error_handling' -require 'ruby_saml/utils' - -REXML::Security.entity_expansion_limit = 0 - -module RubySaml - module XML - class SignedDocument < REXML::Document - include RubySaml::ErrorHandling - - attr_reader :processed, - :referenced_xml - - def initialize(response, errors = []) - super(response) - @errors = errors - reset_elements - end - - def reset_elements - @referenced_xml = nil - @cached_signed_info = nil - @signature = nil - @signature_hash_algorithm = nil - @ref = nil - @processed = false - end - - def signed_element_id - @signed_element_id ||= extract_signed_element_id - end - - # Validates the referenced_xml, which is the signed part of the document - def validate_document(idp_cert_fingerprint, soft = true, options = {}) - # get cert from response - cert_element = REXML::XPath.first( - self, - '//ds:X509Certificate', - { 'ds' => RubySaml::XML::DSIG } - ) - - if cert_element - base64_cert = RubySaml::Utils.element_text(cert_element) - cert_text = Base64.decode64(base64_cert) - begin - cert = OpenSSL::X509::Certificate.new(cert_text) - rescue OpenSSL::X509::CertificateError => _e - return append_error('Document Certificate Error', soft) - end - - if options[:fingerprint_alg] - fingerprint_alg = RubySaml::XML.hash_algorithm(options[:fingerprint_alg]).new - else - fingerprint_alg = OpenSSL::Digest.new('SHA256') - end - fingerprint = fingerprint_alg.hexdigest(cert.to_der) - - # check cert matches registered idp cert - if fingerprint != idp_cert_fingerprint.gsub(/[^a-zA-Z0-9]/, '').downcase - return append_error('Fingerprint mismatch', soft) - end - - base64_cert = Base64.strict_encode64(cert.to_der) - elsif options[:cert] - base64_cert = Base64.strict_encode64(options[:cert].to_pem) - elsif soft - return false - else - return append_error('Certificate element missing in response (ds:X509Certificate) and not cert provided at settings', soft) - end - - validate_signature(base64_cert, soft) - end - - def validate_document_with_cert(idp_cert, soft = true) - # get cert from response - cert_element = REXML::XPath.first( - self, - '//ds:X509Certificate', - { 'ds' => RubySaml::XML::DSIG } - ) - - if cert_element - base64_cert = RubySaml::Utils.element_text(cert_element) - cert_text = Base64.decode64(base64_cert) - begin - cert = OpenSSL::X509::Certificate.new(cert_text) - rescue OpenSSL::X509::CertificateError => _e - return append_error('Document Certificate Error', soft) - end - - # check saml response cert matches provided idp cert - if idp_cert.to_pem != cert.to_pem - return append_error('Certificate of the Signature element does not match provided certificate', soft) - end - end - - encoded_idp_cert = Base64.strict_encode64(idp_cert.to_pem) - validate_signature(encoded_idp_cert, true) - end - - def cache_referenced_xml(soft, check_malformed_doc: true) - reset_elements - @processed = true - - begin - noko = RubySaml::XML.safe_load_nokogiri(self, check_malformed_doc: check_malformed_doc) - rescue StandardError => e - @errors << e.message - return false if soft - - raise ValidationError.new("XML load failed: #{e.message}") - end - - # get signature node - sig_element = noko.at_xpath( - '//ds:Signature', - { 'ds' => RubySaml::XML::DSIG } - ) - return if sig_element.nil? - - # signature method - sig_alg_value = sig_element.at_xpath( - './ds:SignedInfo/ds:SignatureMethod', - { 'ds' => RubySaml::XML::DSIG } - ) - @signature_hash_algorithm = RubySaml::XML.hash_algorithm(sig_alg_value) - - # get signature - base64_signature = sig_element.at_xpath( - './ds:SignatureValue', - { 'ds' => RubySaml::XML::DSIG } - ) - return if base64_signature.nil? - - base64_signature_text = base64_signature.content - @signature = Base64.decode64(base64_signature_text) if base64_signature_text - - # canonicalization method - canon_method_node = sig_element.at_xpath( - './ds:SignedInfo/ds:CanonicalizationMethod', - { 'ds' => RubySaml::XML::DSIG } - ) - canon_algorithm = RubySaml::XML.canon_algorithm(canon_method_node) - - noko_sig_element = noko.at_xpath('//ds:Signature', 'ds' => RubySaml::XML::DSIG) - noko_signed_info_element = noko_sig_element.at_xpath('./ds:SignedInfo', 'ds' => RubySaml::XML::DSIG) - @cached_signed_info = noko_signed_info_element.canonicalize(canon_algorithm) - - # Now get the @referenced_xml to use? - rexml_signed_info = REXML::Document.new(@cached_signed_info.to_s).root - - noko_sig_element.remove - - # get inclusive namespaces - inclusive_namespaces = extract_inclusive_namespaces - - # check digests - @ref = REXML::XPath.first(rexml_signed_info, './ds:Reference', { 'ds' => DSIG }) - return if @ref.nil? - - reference_nodes = noko.xpath('//*[@ID=$id]', nil, { 'id' => extract_signed_element_id }) - - hashed_element = reference_nodes[0] - return if hashed_element.nil? - - canon_method_node = noko_signed_info_element.at_xpath( - './ds:CanonicalizationMethod', - { 'ds' => RubySaml::XML::DSIG } - ) - canon_algorithm = RubySaml::XML.canon_algorithm(canon_method_node) - canon_algorithm = process_transforms(@ref, canon_algorithm) - - @referenced_xml = hashed_element.canonicalize(canon_algorithm, inclusive_namespaces) - end - - def validate_signature(base64_cert, soft = true) - cache_referenced_xml(soft) unless @processed - - return append_error('No Signature Hash Algorithm Method found', soft) if @signature_hash_algorithm.nil? - return append_error('No Signature node found', soft) if @signature.nil? - return append_error('No canonized SignedInfo ', soft) if @cached_signed_info.nil? - return append_error('No Reference node found', soft) if @ref.nil? - return append_error('No referenced XML', soft) if @referenced_xml.nil? - - # get certificate object - cert_text = Base64.decode64(base64_cert) - cert = OpenSSL::X509::Certificate.new(cert_text) - - digest_method_node = REXML::XPath.first( - @ref, - './ds:DigestMethod', - { 'ds' => DSIG } - ) - digest_algorithm = RubySaml::XML.hash_algorithm(digest_method_node) - hash = digest_algorithm.digest(@referenced_xml) - encoded_digest_value = REXML::XPath.first( - @ref, - './ds:DigestValue', - { 'ds' => DSIG } - ) - encoded_digest_value_text = RubySaml::Utils.element_text(encoded_digest_value) - digest_value = encoded_digest_value_text.nil? ? nil : Base64.decode64(encoded_digest_value_text) - - # Compare the computed "hash" with the "signed" hash - unless hash && hash == digest_value - return append_error('Digest mismatch', soft) - end - - # verify signature - signature_verified = false - begin - signature_verified = cert.public_key.verify(@signature_hash_algorithm.new, @signature, @cached_signed_info) - rescue OpenSSL::PKey::PKeyError # rubocop:disable Lint/SuppressedException - end - return append_error('Key validation error', soft) unless signature_verified - - true - end - - private - - def process_transforms(ref, canon_algorithm) - transforms = REXML::XPath.match( - ref, - './ds:Transforms/ds:Transform', - { 'ds' => RubySaml::XML::DSIG } - ) - - transforms.each do |transform_element| - next unless transform_element.attributes&.[]('Algorithm') - - canon_algorithm = RubySaml::XML.canon_algorithm(transform_element, default: false) - end - - canon_algorithm - end - - def digests_match?(hash, digest_value) - hash == digest_value - end - - def extract_signed_element_id - reference_element = REXML::XPath.first( - self, - '//ds:Signature/ds:SignedInfo/ds:Reference', - { 'ds' => RubySaml::XML::DSIG } - ) - - return nil if reference_element.nil? - - sei = reference_element.attribute('URI').value[1..] - sei.nil? ? reference_element.parent.parent.parent.attribute('ID').value : sei - end - - def extract_inclusive_namespaces - element = REXML::XPath.first( - self, - '//ec:InclusiveNamespaces', - { 'ec' => RubySaml::XML::C14N } - ) - return unless element - - element.attributes.get_attribute('PrefixList').value.split - end - end - end -end diff --git a/lib/ruby_saml/xml/signed_document_info.rb b/lib/ruby_saml/xml/signed_document_info.rb new file mode 100644 index 00000000..081c9a6b --- /dev/null +++ b/lib/ruby_saml/xml/signed_document_info.rb @@ -0,0 +1,243 @@ +# frozen_string_literal: true + +module RubySaml + module XML + # Represents the information extracted from a signed document. + class SignedDocumentInfo + attr_reader :noko, + :check_malformed_doc + + # Represents the information extracted from a signed document. + # Intended to avoid signature wrapping attacks. + # + # @param noko [Nokogiri::XML] The XML document to validate + # @param check_malformed_doc [Boolean] Whether to check for malformed documents + def initialize(noko, check_malformed_doc: true) + noko = if noko.is_a?(Nokogiri::XML::Document) + RubySaml::XML.copy_nokogiri(noko) + else + RubySaml::XML.safe_load_nokogiri(noko, check_malformed_doc: check_malformed_doc) + end + @noko = noko + @check_malformed_doc = check_malformed_doc + end + + # Validates the subject_node, which is the signed part of the document + def validate_document(idp_cert_fingerprint, options = {}) + # Get certificate from document + if certificate_object + # Calculate fingerprint using specified algorithm + fingerprint = certificate_fingerprint(options[:fingerprint_alg] || 'SHA256') + + # Check cert matches registered idp cert fingerprint + raise RubySaml::ValidationError.new('Fingerprint mismatch') if fingerprint != idp_cert_fingerprint.gsub(/[^a-zA-Z0-9]/, '').downcase + + cert = certificate_object + elsif options[:cert] + cert = options[:cert] + else + raise RubySaml::ValidationError.new('Certificate element missing in response (ds:X509Certificate) and no cert provided in settings') + end + + validate_signature(cert) + end + + def validate_document_with_cert(idp_cert) + # Check saml response cert matches provided idp cert + raise RubySaml::ValidationError.new('Certificate of the Signature element does not match provided certificate') if certificate_object&.to_pem&.!=(idp_cert.to_pem) + + validate_signature(idp_cert) + end + + def validate_signature(cert) + # TODO: Remove this + # Get certificate object + cert = OpenSSL::X509::Certificate.new(Base64.decode64(cert)) if cert.is_a?(String) + + # Compare digest + calculated_digest = digest_algorithm.digest(canonicalized_subject) + raise RubySaml::ValidationError.new('Digest mismatch') unless calculated_digest == digest_value + + # Verify signature + signature_verified = false + begin + signature_verified = cert.public_key.verify(signature_hash_algorithm.new, + signature_value, + canonicalized_signed_info) + rescue OpenSSL::PKey::PKeyError # rubocop:disable Lint/SuppressedException + end + raise RubySaml::ValidationError.new('Key validation error') unless signature_verified + + true + end + + # Get the signature hash algorithm + # @return [OpenSSL::Digest] The signature hash algorithm + def signature_hash_algorithm + sig_alg_value = signed_info_node.at_xpath( + './ds:SignatureMethod', + { 'ds' => RubySaml::XML::DSIG } + ) + RubySaml::XML.hash_algorithm(sig_alg_value) + end + + # Get the decoded SignatureValue + # @return [String] The decoded signature value + def signature_value + base64_signature = signature_node.at_xpath( + './ds:SignatureValue', + { 'ds' => RubySaml::XML::DSIG } + )&.text&.strip + raise RubySaml::ValidationError.new('No Signature Value found') if base64_signature.nil? + + Base64.decode64(base64_signature) + end + + # Get the canonicalized SignedInfo element + # @return [String] The canonicalized SignedInfo element + def canonicalized_signed_info + @canonicalized_signed_info ||= signed_info_node.canonicalize(canon_algorithm_from_signed_info) + end + + # Get the Reference node + # @return [Nokogiri::XML::Element] The Reference node + def reference_node + signed_info_node.at_xpath('./ds:Reference', { 'ds' => RubySaml::XML::DSIG }) || + (raise RubySaml::ValidationError.new('No Reference node found')) + end + + # Get the ID of the signed element + # @return [String] The ID of the signed element + def subject_id + # TODO: The error here is problematic, perhaps it can be checked elsewhere + @subject_id ||= extract_subject_id || (raise RubySaml::ValidationError.new('No signed subject ID found')) + end + + # Get the subject node (the node being signed) + # @return [Nokogiri::XML::Element] The subject + def subject_node + noko.at_xpath('//*[@ID=$id]', nil, { 'id' => subject_id }) || + (raise RubySaml::ValidationError.new('No subject node found')) + end + + # Get the canonicalized subject node (the node being signed) + # @return [String] The canonicalized subject + def canonicalized_subject + remove_signature_node! + subject_node.canonicalize(canon_algorithm, inclusive_namespaces) + end + + # TODO: Destructive side-effect!! signature_node.remove + # should possibly deep copy the noko object initially + def remove_signature_node! + # memoize various elements + subject_id + inclusive_namespaces + canonicalized_signed_info + + signature_node.remove + end + + # Get the digest algorithm + # @return [OpenSSL::Digest] The digest algorithm + def digest_algorithm + digest_method_node = reference_node.at_xpath( + './ds:DigestMethod', + { 'ds' => RubySaml::XML::DSIG } + ) + RubySaml::XML.hash_algorithm(digest_method_node) + end + + # Get the decoded DigestValue + # @return [String] The decoded digest value + def digest_value + encoded_digest = reference_node.at_xpath( + './ds:DigestValue', + { 'ds' => RubySaml::XML::DSIG } + )&.text&.strip + raise RubySaml::ValidationError.new('No DigestValue found') if encoded_digest.nil? + + Base64.decode64(encoded_digest) + end + + def certificate_text + cert = noko.at_xpath( + '//ds:X509Certificate', + { 'ds' => RubySaml::XML::DSIG } + )&.text&.strip + Base64.decode64(cert) if cert && !cert.empty? + end + + # Get the certificate from the document + # @return [OpenSSL::X509::Certificate] The certificate + def certificate_object + return unless certificate_text + + OpenSSL::X509::Certificate.new(certificate_text) + rescue OpenSSL::X509::CertificateError => _e + # TODO: include underlying error + raise RubySaml::ValidationError.new('Document Certificate Error') + end + + # Calculate the fingerprint of the certificate + # @param algorithm [String, Symbol] The algorithm to use for fingerprinting + # @return [String] The fingerprint + def certificate_fingerprint(algorithm = 'SHA256') + cert = certificate_object + return nil unless cert + + fingerprint_alg = RubySaml::XML.hash_algorithm(algorithm).new + fingerprint_alg.hexdigest(cert.to_der).gsub(/[^a-zA-Z0-9]/, '').downcase + end + + # Extract inclusive namespaces from the document + # @return [Array, nil] The inclusive namespaces + def inclusive_namespaces + @inclusive_namespaces ||= noko.at_xpath( + '//ec:InclusiveNamespaces', + { 'ec' => RubySaml::XML::C14N } + )&.[]('PrefixList')&.split + end + + private + + def extract_subject_id + return unless reference_node + + reference_node['URI'][1..] || signature_node.parent['ID'] + end + + # Get the ds:Signature element from the document + # @return [Nokogiri::XML::Element] The Signature element + def signature_node + @signature_node ||= noko.at_xpath('//ds:Signature', { 'ds' => RubySaml::XML::DSIG }) || + (raise RubySaml::ValidationError.new('No Signature node found')) + end + + # Get the ds:SignedInfo element from the document + # @return [Nokogiri::XML::Element] The SignedInfo element + def signed_info_node + signature_node.at_xpath('./ds:SignedInfo', 'ds' => RubySaml::XML::DSIG) || + (raise RubySaml::ValidationError.new('No SignedInfo node found')) + end + + def canon_algorithm + canon_algorithm_from_transforms || canon_algorithm_from_signed_info + end + + def canon_algorithm_from_signed_info + canon_method_node = signed_info_node.at_xpath( + './ds:CanonicalizationMethod', + { 'ds' => RubySaml::XML::DSIG } + ) + RubySaml::XML.canon_algorithm(canon_method_node) + end + + def canon_algorithm_from_transforms + transforms = reference_node.xpath('./ds:Transforms/ds:Transform', { 'ds' => RubySaml::XML::DSIG }) + transform_element = transforms.reverse.detect { |el| el['Algorithm'] } + RubySaml::XML.canon_algorithm(transform_element, default: false) + end + end + end +end diff --git a/lib/ruby_saml/xml/signed_document_validator.rb b/lib/ruby_saml/xml/signed_document_validator.rb new file mode 100644 index 00000000..e30b1524 --- /dev/null +++ b/lib/ruby_saml/xml/signed_document_validator.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +require 'ruby_saml/error_handling' +require 'ruby_saml/utils' + +module RubySaml + module XML + # Wrapper for the SignedDocumentInfo class. + # TODO: This should be refactored and removed + module SignedDocumentValidator + extend self + + def with_error_handling(errors, soft) + yield + rescue RubySaml::ValidationError => e + errors << e.message + raise e unless soft + + errors # TODO: Return false?? + end + + # TODO: [ERRORS-REFACTOR] -- Rather than returning array of error, + # raise actual error classes + def validate_document(document, idp_cert_fingerprint, errors = [], soft: true, **options) + with_error_handling(errors, soft) do + SignedDocumentInfo.new(document).validate_document(idp_cert_fingerprint, options) + end + end + + def validate_document_with_cert(document, idp_cert, errors = [], soft: true) + with_error_handling(errors, soft) do + SignedDocumentInfo.new(document).validate_document_with_cert(idp_cert) + end + end + + def validate_signature(document, base64_cert, errors = [], soft: true) + with_error_handling(errors, soft) do + SignedDocumentInfo.new(document).validate_signature(base64_cert) + end + end + + # TODO: This is a workaround to avoid errors + def subject_id(document) + SignedDocumentInfo.new(document).subject_id + rescue RubySaml::ValidationError + # TODO: Consider removing the error in SignedDocumentInfo#subject_id + end + + def subject_node(document) + SignedDocumentInfo.new(document).subject_node + end + end + end +end diff --git a/lib/xml_security.rb b/lib/xml_security.rb index a913e87a..3864e8b2 100644 --- a/lib/xml_security.rb +++ b/lib/xml_security.rb @@ -2,8 +2,8 @@ require 'ruby_saml/logging' RubySaml::Logging.deprecate 'Using `require "xml_security"` is deprecated and will be removed ' \ - 'in RubySaml 2.1.0. Instead, please `require "ruby-saml"` and use ' \ + 'in RubySaml 3.0.0. Instead, please `require "ruby-saml"` and use ' \ 'the modules in RubySaml::XML instead.' -# @deprecated This file adds compatibility with v1.x and will be removed in v2.1.0 +# @deprecated This file adds compatibility with v1.x and will be removed in v3.0.0 require 'ruby_saml/xml' diff --git a/test/metadata_test.rb b/test/metadata_test.rb index 2f7c637f..9fb547d0 100644 --- a/test/metadata_test.rb +++ b/test/metadata_test.rb @@ -333,12 +333,11 @@ class MetadataTest < Minitest::Test @cert, @pkey = CertificateHelper.generate_pair(:rsa) settings.certificate, settings.private_key = [@cert, @pkey].map(&:to_pem) @fingerprint = OpenSSL::Digest.new('SHA256', @cert.to_der).to_s - signed_metadata = RubySaml::XML::SignedDocument.new(xml_text) assert_match(signature_value_matcher, xml_text) assert_match(signature_method_matcher(:rsa, :sha256), xml_text) assert_match(digest_method_matcher(:sha256), xml_text) - assert(signed_metadata.validate_document(@fingerprint, false)) + assert(RubySaml::XML::SignedDocumentValidator.validate_document(xml_text, @fingerprint, soft: false)) assert(validate_xml!(xml_text, "saml-schema-metadata-2.0.xsd")) end @@ -353,13 +352,11 @@ class MetadataTest < Minitest::Test end it "creates a signed metadata" do - signed_metadata = RubySaml::XML::SignedDocument.new(xml_text) - assert_match(signature_value_matcher, xml_text) assert_match(signature_method_matcher(sp_key_algo, sp_hash_algo), xml_text) assert_match(digest_method_matcher(sp_hash_algo), xml_text) - assert signed_metadata.validate_document(@fingerprint, false) + assert RubySaml::XML::SignedDocumentValidator.validate_document(xml_text, @fingerprint, soft: false) assert validate_xml!(xml_text, "saml-schema-metadata-2.0.xsd") end @@ -367,23 +364,21 @@ class MetadataTest < Minitest::Test it 'using mixed signature and digest methods (signature SHA256)' do # RSA is ignored here; only the hash sp_key_algo is used settings.security[:signature_method] = RubySaml::XML::RSA_SHA256 - signed_metadata = RubySaml::XML::SignedDocument.new(xml_text) assert_match(signature_value_matcher, xml_text) assert_match(signature_method_matcher(sp_key_algo, :sha256), xml_text) assert_match(digest_method_matcher(sp_hash_algo), xml_text) - assert(signed_metadata.validate_document(@fingerprint, false)) + assert(RubySaml::XML::SignedDocumentValidator.validate_document(xml_text, @fingerprint, soft: false)) assert(validate_xml!(xml_text, "saml-schema-metadata-2.0.xsd")) end it 'using mixed signature and digest methods (digest SHA256)' do settings.security[:digest_method] = RubySaml::XML::SHA256 - signed_metadata = RubySaml::XML::SignedDocument.new(xml_text) assert_match(signature_value_matcher, xml_text) assert_match(signature_method_matcher(sp_key_algo, sp_hash_algo), xml_text) assert_match(digest_method_matcher(:sha256), xml_text) - assert(signed_metadata.validate_document(@fingerprint, false)) + assert(RubySaml::XML::SignedDocumentValidator.validate_document(xml_text, @fingerprint, soft: false)) assert(validate_xml!(xml_text, "saml-schema-metadata-2.0.xsd")) end end @@ -411,7 +406,6 @@ def add_extras(xml, _settings) it "inserts signature as the first child of root element" do xml_text = subclass.new.generate(settings, false) - signed_metadata = RubySaml::XML::SignedDocument.new(xml_text) first_child = xml_doc.root.element_children.first assert_equal first_child.namespace.prefix, 'ds' @@ -419,7 +413,7 @@ def add_extras(xml, _settings) assert_match(signature_value_matcher, xml_text) assert_match(signature_method_matcher(sp_key_algo, sp_hash_algo), xml_text) assert_match(digest_method_matcher(sp_hash_algo), xml_text) - assert signed_metadata.validate_document(@fingerprint, false) + assert(RubySaml::XML::SignedDocumentValidator.validate_document(xml_text, @fingerprint, soft: false)) assert validate_xml!(xml_text, "saml-schema-metadata-2.0.xsd") end end diff --git a/test/response_test.rb b/test/response_test.rb index 2112348f..123d3931 100644 --- a/test/response_test.rb +++ b/test/response_test.rb @@ -74,7 +74,7 @@ def generate_audience_error(expected, actual) end it "be able to parse a document which contains ampersands" do - RubySaml::XML::SignedDocument.any_instance.stubs(:digests_match?).returns(true) + RubySaml::XML::SignedDocumentValidator.stubs(:digests_match?).returns(true) RubySaml::Response.any_instance.stubs(:validate_conditions).returns(true) ampersands_response = RubySaml::Response.new(ampersands_document) @@ -139,7 +139,7 @@ def generate_audience_error(expected, actual) it "raise when evil attack vector is present, soft = false " do @response.soft = false error_msg = "XML load failed: Dangerous XML detected. No Doctype nodes allowed" - assert_raises(OneLogin::RubySaml::ValidationError, error_msg) do + assert_raises(RubySaml::ValidationError, error_msg) do @response.send(:validate_structure) end end @@ -213,23 +213,23 @@ def generate_audience_error(expected, actual) end it "raise when no signature" do - settings.idp_cert_fingerprint = signature_fingerprint_1 - response_no_signed_elements.settings = settings - response_no_signed_elements.soft = false - error_msg = "Found an unexpected number of Signature Element. SAML Response rejected" - assert_raises(RubySaml::ValidationError, error_msg) do - response_no_signed_elements.is_valid? - end + settings.idp_cert_fingerprint = signature_fingerprint_1 + response_no_signed_elements.settings = settings + response_no_signed_elements.soft = false + error_msg = "Found an unexpected number of Signature Element. SAML Response rejected" + assert_raises(RubySaml::ValidationError, error_msg) do + response_no_signed_elements.is_valid? + end end it "raise when multiple signatures" do - settings.idp_cert_fingerprint = signature_fingerprint_1 - response_multiple_signed.settings = settings - response_multiple_signed.soft = false - error_msg = "Duplicated ID. SAML Response rejected" - assert_raises(RubySaml::ValidationError, error_msg) do - response_multiple_signed.is_valid? - end + settings.idp_cert_fingerprint = signature_fingerprint_1 + response_multiple_signed.settings = settings + response_multiple_signed.soft = false + error_msg = "Duplicated ID. SAML Response rejected" + assert_raises(RubySaml::ValidationError, error_msg) do + response_multiple_signed.is_valid? + end end it "validate SAML 2.0 XML structure" do @@ -658,11 +658,11 @@ def generate_audience_error(expected, actual) end describe "validate_formatted_x509_certificate" do - let(:response_with_formatted_x509certificate) { + let(:response_with_formatted_x509certificate) do RubySaml::Response.new(read_response("valid_response_with_formatted_x509certificate.xml.base64"), { :skip_conditions => true, :skip_subject_confirmation => true }) - } + end it "be able to parse the response wihout errors" do response_with_formatted_x509certificate.settings = settings @@ -910,7 +910,7 @@ def generate_audience_error(expected, actual) assert_includes response.errors, "Invalid Signature on SAML Response" end - it "return false when no X509Certificate and not cert provided at settings" do + it "return false when no X509Certificate and no cert provided in settings" do settings.idp_cert_fingerprint = ruby_saml_cert_fingerprint settings.idp_cert = nil response_valid_signed_without_x509certificate.settings = settings @@ -940,8 +940,8 @@ def generate_audience_error(expected, actual) settings.idp_cert_fingerprint = nil settings.idp_cert = ruby_saml_cert_text content = read_response('response_with_signed_message_and_assertion.xml') - content = content.sub(/.*<\/ds:X509Certificate>/, - "an-invalid-certificate") + content = content.sub(%r{.*}, + "an-invalid-certificate") response_invalid_x509certificate = RubySaml::Response.new(content) response_invalid_x509certificate.settings = settings assert !response_invalid_x509certificate.send(:validate_signature) @@ -1227,7 +1227,7 @@ def generate_audience_error(expected, actual) it "raise error when the assertion contains encrypted attributes but no private key to decrypt" do settings.private_key = nil response_encrypted_attrs.settings = settings - assert_raises(RubySaml::ValidationError, "An encrypted element was found, but the Settings does not contain any SP private keys to decrypt it") do + assert_raises(RubySaml::ValidationError, "An EncryptedAttribute found and no SP private key found on the settings to decrypt it") do response_encrypted_attrs.attributes end end @@ -1236,9 +1236,8 @@ def generate_audience_error(expected, actual) settings.certificate = ruby_saml_cert_text settings.private_key = ruby_saml_key_text response_encrypted_attrs.settings = settings - attributes = response_encrypted_attrs.attributes - assert_equal "test", attributes[:uid] - assert_equal "test@example.com", attributes[:mail] + assert_equal "test", response_encrypted_attrs.attributes[:uid] + assert_equal "test@example.com", response_encrypted_attrs.attributes[:mail] end end @@ -1381,7 +1380,10 @@ def generate_audience_error(expected, actual) $evalled = nil malicious_response_document = fixture('response_eval', false) malicious_response = RubySaml::Response.new(malicious_response_document) - malicious_response.send(:xpath_first_from_signed_assertion) + begin + malicious_response.send(:xpath_first_from_signed_assertion) + rescue RubySaml::ValidationError # TODO: This should be a more specific error + end assert_nil $evalled end end @@ -1450,6 +1452,7 @@ def generate_audience_error(expected, actual) assert_equal "test@onelogin.com", response_encrypted_nameid.nameid assert_equal "urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress", response_encrypted_nameid.name_id_format end + end describe 'try to initialize an encrypted response' do @@ -1568,16 +1571,15 @@ def generate_audience_error(expected, actual) it "is not possible to decrypt the assertion if no private key" do response = RubySaml::Response.new(signed_message_encrypted_unsigned_assertion, settings: settings) - encrypted_assertion_node = REXML::XPath.first( - response.document, - "(/p:Response/EncryptedAssertion)|(/p:Response/a:EncryptedAssertion)", + encrypted_assertion_node = response.document.at_xpath( + "/p:Response/EncryptedAssertion | /p:Response/a:EncryptedAssertion", { "p" => RubySaml::XML::NS_PROTOCOL, "a" => RubySaml::XML::NS_ASSERTION } ) response.settings.private_key = nil error_msg = "An EncryptedAssertion found and no SP private key found on the settings to decrypt it" assert_raises(RubySaml::ValidationError, error_msg) do - RubySaml::XML::Decryptor.decrypt_assertion(encrypted_assertion_node, settings.get_sp_decryption_keys) + RubySaml::XML::Decryptor.decrypt_assertion(response.document, encrypted_assertion_node) end end @@ -1592,16 +1594,15 @@ def generate_audience_error(expected, actual) it "is possible to decrypt the assertion if private key" do response = RubySaml::Response.new(signed_message_encrypted_unsigned_assertion, settings: settings) - encrypted_assertion_node = REXML::XPath.first( - response.document, - "(/p:Response/EncryptedAssertion)|(/p:Response/a:EncryptedAssertion)", + encrypted_assertion_node = response.document.at_xpath( + "/p:Response/EncryptedAssertion | /p:Response/a:EncryptedAssertion", { "p" => RubySaml::XML::NS_PROTOCOL, "a" => RubySaml::XML::NS_ASSERTION } ) decrypted = RubySaml::XML::Decryptor.decrypt_assertion(encrypted_assertion_node, settings.get_sp_decryption_keys) encrypted_assertion_node2 = decrypted.at_xpath( - "(/p:Response/EncryptedAssertion)|(/p:Response/a:EncryptedAssertion)", - { "p" => RubySaml::XML::NS_PROTOCOL, "a" => RubySaml::XML::NS_ASSERTION } + "/p:Response/EncryptedAssertion | /p:Response/a:EncryptedAssertion", + { "p" => "urn:oasis:names:tc:SAML:2.0:protocol", "a" => "urn:oasis:names:tc:SAML:2.0:assertion" } ) assert_nil encrypted_assertion_node2 assert decrypted.name, "Assertion" @@ -1617,9 +1618,8 @@ def generate_audience_error(expected, actual) } response = RubySaml::Response.new(signed_message_encrypted_unsigned_assertion, settings: settings) - encrypted_assertion_node = REXML::XPath.first( - response.document, - "(/p:Response/EncryptedAssertion)|(/p:Response/a:EncryptedAssertion)", + encrypted_assertion_node = response.document.at_xpath( + "/p:Response/EncryptedAssertion | /p:Response/a:EncryptedAssertion", { "p" => RubySaml::XML::NS_PROTOCOL, "a" => RubySaml::XML::NS_ASSERTION } ) decrypted = RubySaml::XML::Decryptor.decrypt_assertion(encrypted_assertion_node, settings.get_sp_decryption_keys) @@ -1632,16 +1632,15 @@ def generate_audience_error(expected, actual) resp = read_response('response_with_retrieval_method.xml') response = RubySaml::Response.new(resp, settings: settings) - encrypted_assertion_node = REXML::XPath.first( - response.document, - "(/p:Response/EncryptedAssertion)|(/p:Response/a:EncryptedAssertion)", + encrypted_assertion_node = response.document.at_xpath( + "/p:Response/EncryptedAssertion | /p:Response/a:EncryptedAssertion", { "p" => RubySaml::XML::NS_PROTOCOL, "a" => RubySaml::XML::NS_ASSERTION } ) decrypted = RubySaml::XML::Decryptor.decrypt_assertion(encrypted_assertion_node, settings.get_sp_decryption_keys) encrypted_assertion_node2 = decrypted.at_xpath( - "(/p:Response/EncryptedAssertion)|(/p:Response/a:EncryptedAssertion)", - { "p" => RubySaml::XML::NS_PROTOCOL, "a" => RubySaml::XML::NS_ASSERTION } + "/p:Response/EncryptedAssertion | /p:Response/a:EncryptedAssertion", + { "p" => "urn:oasis:names:tc:SAML:2.0:protocol", "a" => "urn:oasis:names:tc:SAML:2.0:assertion" } ) assert_nil encrypted_assertion_node2 @@ -1651,16 +1650,16 @@ def generate_audience_error(expected, actual) it "is possible to decrypt the assertion if private key but no saml namespace on the Assertion Element that is inside the EncryptedAssertion" do unsigned_message_encrypted_assertion_without_saml_namespace = read_response('unsigned_message_encrypted_assertion_without_saml_namespace.xml.base64') response = RubySaml::Response.new(unsigned_message_encrypted_assertion_without_saml_namespace, settings: settings) - encrypted_assertion_node = REXML::XPath.first( - response.document, - "(/p:Response/EncryptedAssertion)|(/p:Response/a:EncryptedAssertion)", + + encrypted_assertion_node = response.document.at_xpath( + "/p:Response/EncryptedAssertion | /p:Response/a:EncryptedAssertion", { "p" => RubySaml::XML::NS_PROTOCOL, "a" => RubySaml::XML::NS_ASSERTION } ) decrypted = RubySaml::XML::Decryptor.decrypt_assertion(encrypted_assertion_node, settings.get_sp_decryption_keys) encrypted_assertion_node2 = decrypted.at_xpath( - "(/p:Response/EncryptedAssertion)|(/p:Response/a:EncryptedAssertion)", - { "p" => RubySaml::XML::NS_PROTOCOL, "a" => RubySaml::XML::NS_ASSERTION } + "/p:Response/EncryptedAssertion | /p:Response/a:EncryptedAssertion", + { "p" => "urn:oasis:names:tc:SAML:2.0:protocol", "a" => "urn:oasis:names:tc:SAML:2.0:assertion" } ) assert_nil encrypted_assertion_node2 assert decrypted.name, "Assertion" @@ -1720,6 +1719,7 @@ def generate_audience_error(expected, actual) assert_equal "_ce3d2948b4cf20146dee0a0b3dd6f69b6cf86f62d7", response.nameid end end + end describe "#status_code" do @@ -1792,10 +1792,9 @@ def generate_audience_error(expected, actual) each_signature_algorithm do |idp_key_algo, idp_hash_algo| describe "#validate_signature" do let(:xml_signed) do - RubySaml::XML::DocumentSigner.sign_document(read_response('response_unsigned2.xml'), - @pkey, @cert, - signature_method(idp_key_algo, idp_hash_algo), - digest_method(idp_hash_algo)).to_s + doc = read_response('response_unsigned2.xml') + RubySaml::XML::DocumentSigner.sign_document(doc, @pkey, @cert, signature_method(idp_key_algo, idp_hash_algo), digest_method(idp_hash_algo)) + .to_s end before do diff --git a/test/slo_logoutrequest_test.rb b/test/slo_logoutrequest_test.rb index 85b00a9e..2a9c2c6d 100644 --- a/test/slo_logoutrequest_test.rb +++ b/test/slo_logoutrequest_test.rb @@ -136,7 +136,7 @@ class RubySamlTest < Minitest::Test it "extract the value of the NotOnOrAfter attribute" do time_value = '2014-07-17T01:01:48Z' assert_nil logout_request.not_on_or_after - logout_request.document.root.attributes['NotOnOrAfter'] = time_value + logout_request.document.root['NotOnOrAfter'] = time_value assert_equal Time.parse(time_value), logout_request.not_on_or_after end end @@ -183,7 +183,7 @@ class RubySamlTest < Minitest::Test assert_empty logout_request.errors Timecop.freeze Time.parse('2014-07-17T01:01:47Z') do - logout_request.document.root.attributes['NotOnOrAfter'] = '2014-07-17T01:01:48Z' + logout_request.document.root['NotOnOrAfter'] = '2014-07-17T01:01:48Z' assert logout_request.send(:validate_not_on_or_after) assert_empty logout_request.errors end @@ -191,7 +191,7 @@ class RubySamlTest < Minitest::Test it "return false when the logout request has an invalid NotOnOrAfter" do Timecop.freeze Time.parse('2014-07-17T01:01:49Z') do - logout_request.document.root.attributes['NotOnOrAfter'] = '2014-07-17T01:01:48Z' + logout_request.document.root['NotOnOrAfter'] = '2014-07-17T01:01:48Z' refute logout_request.send(:validate_not_on_or_after) assert_match(/Current time is on or after NotOnOrAfter/, logout_request.errors[0]) end @@ -199,7 +199,7 @@ class RubySamlTest < Minitest::Test it "raise when the logout request has an invalid NotOnOrAfter" do Timecop.freeze Time.parse('2014-07-17T01:01:49Z') do - logout_request.document.root.attributes['NotOnOrAfter'] = '2014-07-17T01:01:48Z' + logout_request.document.root['NotOnOrAfter'] = '2014-07-17T01:01:48Z' logout_request.soft = false assert_raises(RubySaml::ValidationError, "Current time is on or after NotOnOrAfter") do logout_request.send(:validate_not_on_or_after) @@ -212,7 +212,7 @@ class RubySamlTest < Minitest::Test java = jruby? || truffleruby? logout_request.soft = true - logout_request.document.root.attributes['NotOnOrAfter'] = '2011-06-14T18:31:01.516Z' + logout_request.document.root['NotOnOrAfter'] = '2011-06-14T18:31:01.516Z' # The NotBefore condition in the document is 2011-06-1418:31:01.516Z Timecop.freeze(Time.parse("2011-06-14T18:31:02Z")) do diff --git a/test/utils_test.rb b/test/utils_test.rb index b0b26681..02c403a1 100644 --- a/test/utils_test.rb +++ b/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 text').elements.first - assert_equal 'element text', RubySaml::Utils.element_text(element) + element = Nokogiri::XML('element text').root + assert_equal 'element text', element.text end it 'returns all segments of the element text' do - element = REXML::Document.new('element text').elements.first - assert_equal 'element text', RubySaml::Utils.element_text(element) + element = Nokogiri::XML('element text').root + assert_equal 'element text', element.text end it 'returns normalized element text' do - element = REXML::Document.new('element & text').elements.first - assert_equal 'element & text', RubySaml::Utils.element_text(element) + element = Nokogiri::XML('element & text').root + assert_equal 'element & text', element.text end it 'returns the CDATA element text' do - element = REXML::Document.new('').elements.first - assert_equal 'element & text', RubySaml::Utils.element_text(element) + element = Nokogiri::XML('').root + assert_equal 'element & text', element.text end it 'returns the element text with newlines and additional whitespace' do - element = REXML::Document.new(" element \n text ").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 \n text ").root + assert_equal " element \n text ", element.text end it 'returns empty string when element has no text' do - element = REXML::Document.new('').elements.first - assert_equal '', RubySaml::Utils.element_text(element) + element = Nokogiri::XML('').root + assert_equal '', element.text end end end diff --git a/test/xml/decryptor_test.rb b/test/xml/decryptor_test.rb index 5b82b487..c4e41f2d 100644 --- a/test/xml/decryptor_test.rb +++ b/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("") - 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 @@ -158,7 +154,7 @@ class XmlDecryptorTest < Minitest::Test assert_equal 'NameID', decrypted_nameid.name assert_equal RubySaml::XML::NS_ASSERTION, decrypted_nameid.namespace.href assert_equal 'urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress', decrypted_nameid['Format'] - assert_equal 'test@onelogin.com', decrypted_nameid.content + assert_equal 'test@onelogin.com', decrypted_nameid.text end it 'should raise an error when no decryption keys are provided' do diff --git a/test/xml_test.rb b/test/xml_test.rb index 298d4da9..8c947ad8 100644 --- a/test/xml_test.rb +++ b/test/xml_test.rb @@ -4,84 +4,91 @@ class XmlTest < Minitest::Test - describe "RubySaml::XML" do + describe RubySaml::XML::SignedDocumentValidator do let(:decoded_response) { Base64.decode64(response_document_without_recipient) } - let(:document) { RubySaml::XML::SignedDocument.new(decoded_response) } let(:settings) { RubySaml::Settings.new } - + let(:errors) { [] } before do - @base64cert = document.elements["//ds:X509Certificate"].text + # Get the X509Certificate from the document for testing + doc = Nokogiri::XML(decoded_response) + @base64cert = doc.at_xpath("//ds:X509Certificate", { "ds" => "http://www.w3.org/2000/09/xmldsig#" }).text end it "should run validate without throwing NS related exceptions" do - assert !document.validate_signature(@base64cert, true) + refute RubySaml::XML::SignedDocumentValidator.validate_signature(decoded_response, @base64cert).is_a?(TrueClass) end it "should run validate with throwing NS related exceptions" do assert_raises(RubySaml::ValidationError) do - document.validate_signature(@base64cert, false) + RubySaml::XML::SignedDocumentValidator.validate_signature(decoded_response, @base64cert, soft: false) end end it "not raise an error when softly validating the document multiple times" do - 2.times { assert_equal document.validate_signature(@base64cert, true), false } + 2.times do + refute RubySaml::XML::SignedDocumentValidator.validate_signature(decoded_response, @base64cert).is_a?(TrueClass) + end end it "not raise an error when softly validating the document and the X509Certificate is missing" do - decoded_response.sub!(/.*<\/ds:X509Certificate>/, "") - mod_document = RubySaml::XML::SignedDocument.new(decoded_response) - assert !mod_document.validate_document("a fingerprint", true) # The fingerprint isn't relevant to this test + modified_response = decoded_response.sub(%r{.*}, '') + # The fingerprint isn't relevant to this test + refute RubySaml::XML::SignedDocumentValidator.validate_document(modified_response, "a fingerprint", soft: true).is_a?(TrueClass) end it "should raise Fingerprint mismatch" do + errors = [] exception = assert_raises(RubySaml::ValidationError) do - document.validate_document("no:fi:ng:er:pr:in:t", false) + RubySaml::XML::SignedDocumentValidator.validate_document(decoded_response, "no:fi:ng:er:pr:in:t", errors, soft: false) end assert_equal("Fingerprint mismatch", exception.message) - assert_includes document.errors, "Fingerprint mismatch" + assert_includes errors, "Fingerprint mismatch" end it "should raise Digest mismatch" do + errors = [] exception = assert_raises(RubySaml::ValidationError) do - document.validate_signature(@base64cert, false) + RubySaml::XML::SignedDocumentValidator.validate_signature(decoded_response, @base64cert, errors, soft: false) end assert_equal("Digest mismatch", exception.message) - assert_includes document.errors, "Digest mismatch" + assert_includes errors, "Digest mismatch" end it "should raise Key validation error" do - decoded_response.sub!("pJQ7MS/ek4KRRWGmv/H43ReHYMs=", - "b9xsAXLsynugg3Wc1CI3kpWku+0=") - mod_document = RubySaml::XML::SignedDocument.new(decoded_response) - base64cert = mod_document.elements["//ds:X509Certificate"].text + modified_response = decoded_response.sub("pJQ7MS/ek4KRRWGmv/H43ReHYMs=", + "b9xsAXLsynugg3Wc1CI3kpWku+0=") + doc = Nokogiri::XML(modified_response) + base64cert = doc.at_xpath("//ds:X509Certificate", { "ds" => "http://www.w3.org/2000/09/xmldsig#" }).text + + errors = [] exception = assert_raises(RubySaml::ValidationError) do - mod_document.validate_signature(base64cert, false) + RubySaml::XML::SignedDocumentValidator.validate_signature(modified_response, base64cert, errors, soft: false) end assert_equal("Key validation error", exception.message) - assert_includes mod_document.errors, "Key validation error" + assert_includes errors, "Key validation error" end it "correctly obtain the digest method with alternate namespace declaration" do adfs_document = fixture(:adfs_response_xmlns, false) - base64cert = adfs_document[%r{(.*?)}, 1] - adfs_document = RubySaml::XML::SignedDocument.new(adfs_document) - assert adfs_document.validate_signature(base64cert, false) + base64cert = adfs_document.scan(%r{(.*)}).first.first + assert RubySaml::XML::SignedDocumentValidator.validate_signature(adfs_document, base64cert, soft: false) end it "raise validation error when the X509Certificate is missing and no cert provided" do - decoded_response.sub!(%r{.*}, '') - mod_document = RubySaml::XML::SignedDocument.new(decoded_response) + modified_response = decoded_response.sub(%r{.*}, '') exception = assert_raises(RubySaml::ValidationError) do - mod_document.validate_document("a fingerprint", false) # The fingerprint isn't relevant to this test + # The fingerprint isn't relevant to this test + RubySaml::XML::SignedDocumentValidator.validate_document(modified_response, "a fingerprint", soft: false) end - assert_equal("Certificate element missing in response (ds:X509Certificate) and not cert provided at settings", exception.message) + assert_equal("Certificate element missing in response (ds:X509Certificate) and no cert provided in settings", exception.message) end - it "invalidaties when the X509Certificate is missing and the cert is provided but mismatches" do - decoded_response.sub!(/.*<\/ds:X509Certificate>/, "") - mod_document = RubySaml::XML::SignedDocument.new(decoded_response) + it "invalidates when the X509Certificate is missing and the cert is provided but mismatches" do + modified_response = decoded_response.sub(%r{.*}, '') cert = OpenSSL::X509::Certificate.new(ruby_saml_cert) - assert !mod_document.validate_document("a fingerprint", true, :cert => cert) # The fingerprint isn't relevant to this test + + # The fingerprint isn't relevant to this test + refute RubySaml::XML::SignedDocumentValidator.validate_document(modified_response, "a fingerprint", soft: true, cert: cert).is_a?(TrueClass) end end @@ -133,115 +140,117 @@ class XmlTest < Minitest::Test end end - describe "Fingerprint Algorithms" do + describe 'Fingerprint Algorithms' do let(:response_fingerprint_test) { RubySaml::Response.new(fixture(:adfs_response_sha1, false)) } + let(:document) { response_fingerprint_test.document } - it "validate using SHA1" do - sha1_fingerprint = "F1:3C:6B:80:90:5A:03:0E:6C:91:3E:5D:15:FA:DD:B0:16:45:48:72" + it 'validate using SHA1' do + sha1_fingerprint = 'F1:3C:6B:80:90:5A:03:0E:6C:91:3E:5D:15:FA:DD:B0:16:45:48:72' sha1_fingerprint_downcase = sha1_fingerprint.tr(':', '').downcase - assert response_fingerprint_test.document.validate_document(sha1_fingerprint, true, fingerprint_alg: RubySaml::XML::SHA1) - assert response_fingerprint_test.document.validate_document(sha1_fingerprint_downcase, true, fingerprint_alg: RubySaml::XML::SHA1) + assert RubySaml::XML::SignedDocumentValidator.validate_document(document, sha1_fingerprint, fingerprint_alg: RubySaml::XML::SHA1) + # TODO: Do not return errors, instead raise error + refute RubySaml::XML::SignedDocumentValidator.validate_document(document, sha1_fingerprint, fingerprint_alg: RubySaml::XML::SHA256).is_a?(TrueClass) + assert RubySaml::XML::SignedDocumentValidator.validate_document(document, sha1_fingerprint_downcase, fingerprint_alg: RubySaml::XML::SHA1) end - it "validate using SHA256" do - sha256_fingerprint = "C4:C6:BD:41:EC:AD:57:97:CE:7B:7D:80:06:C3:E4:30:53:29:02:0B:DD:2D:47:02:9E:BD:85:AD:93:02:45:21" + it 'validate using SHA256' do + sha256_fingerprint = 'C4:C6:BD:41:EC:AD:57:97:CE:7B:7D:80:06:C3:E4:30:53:29:02:0B:DD:2D:47:02:9E:BD:85:AD:93:02:45:21' sha256_fingerprint_downcase = sha256_fingerprint.tr(':', '').downcase - assert response_fingerprint_test.document.validate_document(sha256_fingerprint) - assert response_fingerprint_test.document.validate_document(sha256_fingerprint, true, fingerprint_alg: RubySaml::XML::SHA256) + assert RubySaml::XML::SignedDocumentValidator.validate_document(document, sha256_fingerprint) + # TODO: Do not return errors, instead raise error + refute RubySaml::XML::SignedDocumentValidator.validate_document(document, sha256_fingerprint, fingerprint_alg: RubySaml::XML::SHA1).is_a?(TrueClass) + assert RubySaml::XML::SignedDocumentValidator.validate_document(document, sha256_fingerprint, fingerprint_alg: RubySaml::XML::SHA256) - assert response_fingerprint_test.document.validate_document(sha256_fingerprint_downcase) - assert response_fingerprint_test.document.validate_document(sha256_fingerprint_downcase, true, fingerprint_alg: RubySaml::XML::SHA256) + assert RubySaml::XML::SignedDocumentValidator.validate_document(document, sha256_fingerprint_downcase) + assert RubySaml::XML::SignedDocumentValidator.validate_document(document, sha256_fingerprint_downcase, fingerprint_alg: RubySaml::XML::SHA256) end - it "validate using SHA384" do - sha384_fingerprint = "98:FE:17:90:31:E7:68:18:8A:65:4D:DA:F5:76:E2:09:97:BE:8B:E3:7E:AA:8D:63:64:7C:0C:38:23:9A:AC:A2:EC:CE:48:A6:74:4D:E0:4C:50:80:40:B4:8D:55:14:14" + it 'validate using SHA384' do + sha384_fingerprint = '98:FE:17:90:31:E7:68:18:8A:65:4D:DA:F5:76:E2:09:97:BE:8B:E3:7E:AA:8D:63:64:7C:0C:38:23:9A:AC:A2:EC:CE:48:A6:74:4D:E0:4C:50:80:40:B4:8D:55:14:14' - assert !response_fingerprint_test.document.validate_document(sha384_fingerprint) - assert response_fingerprint_test.document.validate_document(sha384_fingerprint, true, :fingerprint_alg => RubySaml::XML::SHA384) + # TODO: Do not return errors, instead raise error + assert !RubySaml::XML::SignedDocumentValidator.validate_document(document, sha384_fingerprint).is_a?(TrueClass) + assert RubySaml::XML::SignedDocumentValidator.validate_document(document, sha384_fingerprint, fingerprint_alg: RubySaml::XML::SHA384) end - it "validate using SHA512" do - sha512_fingerprint = "5A:AE:BA:D0:BA:9D:1E:25:05:01:1E:1A:C9:E9:FF:DB:ED:FA:6E:F7:52:EB:45:49:BD:DB:06:D8:A3:7E:CC:63:3A:04:A2:DD:DF:EE:61:05:D9:58:95:2A:77:17:30:4B:EB:4A:9F:48:4A:44:1C:D0:9E:0B:1E:04:77:FD:A3:D2" + it 'validate using SHA512' do + sha512_fingerprint = '5A:AE:BA:D0:BA:9D:1E:25:05:01:1E:1A:C9:E9:FF:DB:ED:FA:6E:F7:52:EB:45:49:BD:DB:06:D8:A3:7E:CC:63:3A:04:A2:DD:DF:EE:61:05:D9:58:95:2A:77:17:30:4B:EB:4A:9F:48:4A:44:1C:D0:9E:0B:1E:04:77:FD:A3:D2' - assert !response_fingerprint_test.document.validate_document(sha512_fingerprint) - assert response_fingerprint_test.document.validate_document(sha512_fingerprint, true, fingerprint_alg: RubySaml::XML::SHA512) + # TODO: Do not return errors, instead raise error + assert !RubySaml::XML::SignedDocumentValidator.validate_document(document, sha512_fingerprint).is_a?(TrueClass) + assert RubySaml::XML::SignedDocumentValidator.validate_document(document, sha512_fingerprint, fingerprint_alg: RubySaml::XML::SHA512) end - end - describe "Signature Algorithms" do - it "validate using SHA1" do - document = RubySaml::XML::SignedDocument.new(fixture(:adfs_response_sha1, false)) - assert document.validate_document("C4:C6:BD:41:EC:AD:57:97:CE:7B:7D:80:06:C3:E4:30:53:29:02:0B:DD:2D:47:02:9E:BD:85:AD:93:02:45:21") + describe 'Signature Algorithms' do + it 'validate using SHA1' do + document = fixture(:adfs_response_sha1, false) + assert RubySaml::XML::SignedDocumentValidator.validate_document(document, 'C4:C6:BD:41:EC:AD:57:97:CE:7B:7D:80:06:C3:E4:30:53:29:02:0B:DD:2D:47:02:9E:BD:85:AD:93:02:45:21') end - it "validate using SHA256" do - document = RubySaml::XML::SignedDocument.new(fixture(:adfs_response_sha256, false)) - assert document.validate_document("3D:C5:BC:58:60:5D:19:64:94:E3:BA:C8:3D:49:01:D5:56:34:44:65:C2:85:0A:A8:65:A5:AC:76:7E:65:1F:F7") + it 'validate using SHA256' do + document = fixture(:adfs_response_sha256, false) + assert RubySaml::XML::SignedDocumentValidator.validate_document(document, '3D:C5:BC:58:60:5D:19:64:94:E3:BA:C8:3D:49:01:D5:56:34:44:65:C2:85:0A:A8:65:A5:AC:76:7E:65:1F:F7') end - it "validate using SHA384" do - document = RubySaml::XML::SignedDocument.new(fixture(:adfs_response_sha384, false)) - assert document.validate_document("C4:C6:BD:41:EC:AD:57:97:CE:7B:7D:80:06:C3:E4:30:53:29:02:0B:DD:2D:47:02:9E:BD:85:AD:93:02:45:21") + it 'validate using SHA384' do + document = fixture(:adfs_response_sha384, false) + assert RubySaml::XML::SignedDocumentValidator.validate_document(document, 'C4:C6:BD:41:EC:AD:57:97:CE:7B:7D:80:06:C3:E4:30:53:29:02:0B:DD:2D:47:02:9E:BD:85:AD:93:02:45:21') end - it "validate using SHA512" do - document = RubySaml::XML::SignedDocument.new(fixture(:adfs_response_sha512, false)) - assert document.validate_document("C4:C6:BD:41:EC:AD:57:97:CE:7B:7D:80:06:C3:E4:30:53:29:02:0B:DD:2D:47:02:9E:BD:85:AD:93:02:45:21") + it 'validate using SHA512' do + document = fixture(:adfs_response_sha512, false) + assert RubySaml::XML::SignedDocumentValidator.validate_document(document, 'C4:C6:BD:41:EC:AD:57:97:CE:7B:7D:80:06:C3:E4:30:53:29:02:0B:DD:2D:47:02:9E:BD:85:AD:93:02:45:21') end end - describe "RubySaml::XML::SignedDocument" do + describe 'RubySaml::XML::SignedDocument' do - describe "#extract_inclusive_namespaces" do - it "support explicit namespace resolution for exclusive canonicalization" do - response = fixture(:open_saml_response, false) - document = RubySaml::XML::SignedDocument.new(response) - inclusive_namespaces = document.send(:extract_inclusive_namespaces) + describe '#extract_inclusive_namespaces' do + it 'support explicit namespace resolution for exclusive canonicalization' do + document = fixture(:open_saml_response, false) + inclusive_namespaces = RubySaml::XML::SignedDocumentInfo.new(document).send(:inclusive_namespaces) assert_equal %w[ xs ], inclusive_namespaces end - it "support implicit namespace resolution for exclusive canonicalization" do - response = fixture(:no_signature_ns, false) - document = RubySaml::XML::SignedDocument.new(response) - inclusive_namespaces = document.send(:extract_inclusive_namespaces) + it 'support implicit namespace resolution for exclusive canonicalization' do + document = fixture(:no_signature_ns, false) + inclusive_namespaces = RubySaml::XML::SignedDocumentInfo.new(document).send(:inclusive_namespaces) assert_equal %w[ #default saml ds xs xsi ], inclusive_namespaces end it 'support inclusive canonicalization' do skip('test not yet implemented') - response = RubySaml::Response.new(fixture("tdnf_response.xml")) + response = RubySaml::Response.new(fixture('tdnf_response.xml')) response.stubs(:conditions).returns(nil) assert !response.is_valid? assert !response.is_valid? response.settings = settings assert !response.is_valid? - settings.idp_cert_fingerprint = "e6 38 9a 20 b7 4f 13 db 6a bc b1 42 6a e7 52 1d d6 56 d4 1b".upcase.gsub(" ", ":") + settings.idp_cert_fingerprint = 'e6 38 9a 20 b7 4f 13 db 6a bc b1 42 6a e7 52 1d d6 56 d4 1b'.upcase.gsub(' ', ':') assert response.is_valid? end - it "return nil when inclusive namespace element is missing" do - response = fixture(:no_signature_ns, false) - response.slice! %r{} - - document = RubySaml::XML::SignedDocument.new(response) - inclusive_namespaces = document.send(:extract_inclusive_namespaces) + it 'return nil when inclusive namespace element is missing' do + document = fixture(:no_signature_ns, false) + document.slice! %r{} + inclusive_namespaces = RubySaml::XML::SignedDocumentInfo.new(document).send(:inclusive_namespaces) assert inclusive_namespaces.nil? end end - describe "RubySaml::XML::DSIG" do + describe "RubySaml::XML" do let(:settings) do settings = RubySaml::Settings.new settings.idp_sso_service_url = "https://idp.example.com/sso" settings.protocol_binding = "urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST" settings.idp_slo_service_url = "https://idp.example.com/slo", - settings.sp_entity_id = "https://sp.example.com/saml2" + settings.sp_entity_id = "https://sp.example.com/saml2" settings.assertion_consumer_service_url = "https://sp.example.com/acs" settings.single_logout_service_url = "https://sp.example.com/sls" settings @@ -251,9 +260,8 @@ class XmlTest < Minitest::Test request_doc = RubySaml::Authrequest.new.create_authentication_xml_doc(settings) request_doc = RubySaml::XML::DocumentSigner.sign_document(request_doc, ruby_saml_key, ruby_saml_cert) - # verify our signature - signed_doc = RubySaml::XML::SignedDocument.new(request_doc.to_s) - assert signed_doc.validate_document(ruby_saml_cert_fingerprint, false) + # verify signature + assert RubySaml::XML::SignedDocumentValidator.validate_document(request_doc.to_s, ruby_saml_cert_fingerprint, soft: false) end it "signs an AuthNRequest with a certificate string" do @@ -261,8 +269,7 @@ class XmlTest < Minitest::Test request_doc = RubySaml::XML::DocumentSigner.sign_document(request_doc, ruby_saml_key, ruby_saml_cert_text) # verify signature - signed_doc = RubySaml::XML::SignedDocument.new(request_doc.to_s) - assert signed_doc.validate_document(ruby_saml_cert_fingerprint, false) + assert RubySaml::XML::SignedDocumentValidator.validate_document(request_doc.to_s, ruby_saml_cert_fingerprint, soft: false) end it "signs a LogoutRequest with a certificate object" do @@ -270,8 +277,7 @@ class XmlTest < Minitest::Test logout_request_doc = RubySaml::XML::DocumentSigner.sign_document(logout_request_doc, ruby_saml_key, ruby_saml_cert) # verify signature - signed_doc = RubySaml::XML::SignedDocument.new(logout_request_doc.to_s) - assert signed_doc.validate_document(ruby_saml_cert_fingerprint, false) + assert RubySaml::XML::SignedDocumentValidator.validate_document(logout_request_doc.to_s, ruby_saml_cert_fingerprint, soft: false) end it "signs a LogoutRequest with a certificate string" do @@ -279,8 +285,7 @@ class XmlTest < Minitest::Test logout_request_doc = RubySaml::XML::DocumentSigner.sign_document(logout_request_doc, ruby_saml_key, ruby_saml_cert_text) # verify signature - signed_doc = RubySaml::XML::SignedDocument.new(logout_request_doc.to_s) - assert signed_doc.validate_document(ruby_saml_cert_fingerprint, false) + assert RubySaml::XML::SignedDocumentValidator.validate_document(logout_request_doc.to_s, ruby_saml_cert_fingerprint, soft: false) end it "signs a LogoutResponse with a certificate object" do @@ -288,8 +293,7 @@ class XmlTest < Minitest::Test logout_response_doc = RubySaml::XML::DocumentSigner.sign_document(logout_response_doc, ruby_saml_key, ruby_saml_cert) # verify signature - signed_doc = RubySaml::XML::SignedDocument.new(logout_response_doc.to_s) - assert signed_doc.validate_document(ruby_saml_cert_fingerprint, false) + assert RubySaml::XML::SignedDocumentValidator.validate_document(logout_response_doc.to_s, ruby_saml_cert_fingerprint, soft: false) end it "signs a LogoutResponse with a certificate string" do @@ -297,8 +301,7 @@ class XmlTest < Minitest::Test logout_response_doc = RubySaml::XML::DocumentSigner.sign_document(logout_response_doc, ruby_saml_key, ruby_saml_cert_text) # verify signature - signed_doc = RubySaml::XML::SignedDocument.new(logout_response_doc.to_s) - assert signed_doc.validate_document(ruby_saml_cert_fingerprint, false) + assert RubySaml::XML::SignedDocumentValidator.validate_document(logout_response_doc.to_s, ruby_saml_cert_fingerprint, soft: false) end end @@ -343,46 +346,46 @@ class XmlTest < Minitest::Test end describe '#validate_document' do + let(:response) { RubySaml::Response.new(document_data) } + let(:document) { response.document } + describe 'with valid document' do describe 'when response has signed message and assertion' do let(:document_data) { read_response('response_with_signed_message_and_assertion.xml') } - let(:document) { RubySaml::Response.new(document_data).document } let(:fingerprint) { '6385109dd146a45d4382799491cb2707bd1ebda3738f27a0e4a4a8159c0fe6cd' } it 'is valid' do - assert document.validate_document(fingerprint, true), 'Document should be valid' + assert RubySaml::XML::SignedDocumentValidator.validate_document(document, fingerprint), 'Document should be valid' end end describe 'when response has signed assertion' do let(:document_data) { read_response('response_with_signed_assertion_3.xml') } - let(:document) { RubySaml::Response.new(document_data).document } let(:fingerprint) { '6385109dd146a45d4382799491cb2707bd1ebda3738f27a0e4a4a8159c0fe6cd' } it 'is valid' do - assert document.validate_document(fingerprint, true), 'Document should be valid' + assert RubySaml::XML::SignedDocumentValidator.validate_document(document, fingerprint), 'Document should be valid' end end end describe 'signature_wrapping_attack' do let(:document_data) { read_invalid_response("signature_wrapping_attack.xml.base64") } - let(:document) { RubySaml::Response.new(document_data).document } let(:fingerprint) { 'afe71c28ef740bc87425be13a2263d37971da1f9' } it 'is invalid' do - assert !document.validate_document(fingerprint, true), 'Document should be invalid' + # TODO: Raise errors instead of returning errors, remove the is_a?(TrueClass) check + refute RubySaml::XML::SignedDocumentValidator.validate_document(document, fingerprint).is_a?(TrueClass), 'Document should be invalid' end end describe 'signature wrapping attack - doubled SAML response body' do let(:document_data) { read_invalid_response("response_with_doubled_signed_assertion.xml") } - let(:document) { RubySaml::Response.new(document_data) } let(:fingerprint) { '6385109dd146a45d4382799491cb2707bd1ebda3738f27a0e4a4a8159c0fe6cd' } it 'is valid, but the unsigned information is ignored in favour of the signed information' do - assert document.document.validate_document(fingerprint, true), 'Document should be valid' - assert_equal 'someone@example.org', document.name_id, 'Document should expose only signed, valid details' + assert RubySaml::XML::SignedDocumentValidator.validate_document(document, fingerprint), 'Document should be valid' + assert_equal 'someone@example.org', response.name_id, 'Document should expose only signed, valid details' end end @@ -392,7 +395,7 @@ class XmlTest < Minitest::Test let(:fingerprint) { '6385109dd146a45d4382799491cb2707bd1ebda3738f27a0e4a4a8159c0fe6cd' } it 'is valid, but the unsigned information is ignored in favour of the signed information' do - assert document.document.validate_document(fingerprint, true), 'Document should be valid' + assert RubySaml::XML::SignedDocumentValidator.validate_document(document.document, fingerprint), 'Document should be valid' assert_equal 'someone@example.org', document.name_id, 'Document should expose only signed, valid details' end end @@ -400,18 +403,19 @@ class XmlTest < Minitest::Test describe '#validate_document_with_cert' do let(:document_data) { read_response('response_with_signed_message_and_assertion.xml') } - let(:document) { RubySaml::Response.new(document_data).document } + let(:response) { RubySaml::Response.new(document_data) } + let(:document) { response.document } let(:idp_cert) { OpenSSL::X509::Certificate.new(ruby_saml_cert_text) } let(:fingerprint) { '4b68c453c7d994aad9025c99d5efcf566287fe8d' } describe 'with invalid document ' do describe 'when certificate is invalid' do - let(:document) { RubySaml::Response.new(document_data).document } - it 'is invalid' do - wrong_document_data = document_data.sub(/.*<\/ds:X509Certificate>/, "invalid<\/ds:X509Certificate>") + wrong_document_data = document_data.sub(%r{.*}, + 'invalid') wrong_document = RubySaml::Response.new(wrong_document_data).document - refute wrong_document.validate_document_with_cert(idp_cert), 'Document should be invalid' + # TODO: Do not return errors, instead raise error + refute RubySaml::XML::SignedDocumentValidator.validate_document_with_cert(wrong_document, idp_cert).is_a?(TrueClass), 'Document should be invalid' end end end @@ -419,16 +423,16 @@ class XmlTest < Minitest::Test describe 'with valid document' do describe 'when response has cert' do it 'is valid' do - assert document.validate_document_with_cert(idp_cert), 'Document should be valid' + assert RubySaml::XML::SignedDocumentValidator.validate_document_with_cert(document, idp_cert), 'Document should be valid' end end describe 'when response has no cert but you have local cert' do - let(:document) { RubySaml::Response.new(response_document_valid_signed_without_x509certificate).document } + let(:response) { RubySaml::Response.new(response_document_valid_signed_without_x509certificate) } let(:idp_cert) { OpenSSL::X509::Certificate.new(ruby_saml_cert_text) } it 'is valid' do - assert document.validate_document_with_cert(idp_cert), 'Document should be valid' + assert RubySaml::XML::SignedDocumentValidator.validate_document_with_cert(document, idp_cert), 'Document should be valid' end end end @@ -437,20 +441,22 @@ class XmlTest < Minitest::Test let(:document_data) { response_document_valid_signed_without_x509certificate } it 'is valid' do - assert document.validate_document_with_cert(idp_cert), 'Document should be valid' + assert RubySaml::XML::SignedDocumentValidator.validate_document_with_cert(document, idp_cert), 'Document should be valid' end end describe 'when response cert is invalid' do let(:document_data) do contents = read_response('response_with_signed_message_and_assertion.xml') - contents.sub(/.*<\/ds:X509Certificate>/, + contents.sub(%r{.*}, "an-invalid-certificate") end it 'is not valid' do - assert !document.validate_document_with_cert(idp_cert), 'Document should be valid' - assert_equal(["Document Certificate Error"], document.errors) + refute RubySaml::XML::SignedDocumentValidator.validate_document_with_cert(document, idp_cert).is_a?(TrueClass), 'Document should be valid' + errors = [] + RubySaml::XML::SignedDocumentValidator.validate_document_with_cert(document, idp_cert, errors) + assert_equal(["Document Certificate Error"], errors) end end @@ -459,14 +465,15 @@ class XmlTest < Minitest::Test it 'is not valid' do exception = assert_raises(RubySaml::ValidationError) do - document.validate_document_with_cert(idp_cert, false) + RubySaml::XML::SignedDocumentValidator.validate_document_with_cert(document, idp_cert, soft: false) end assert_equal("Certificate of the Signature element does not match provided certificate", exception.message) end it 'is not valid (soft = true)' do - document.validate_document_with_cert(idp_cert) - assert_equal(["Certificate of the Signature element does not match provided certificate"], document.errors) + errors = [] + RubySaml::XML::SignedDocumentValidator.validate_document_with_cert(document, idp_cert, errors) + assert_equal(["Certificate of the Signature element does not match provided certificate"], errors) end end end