Deprecate the settings.compress_request and settings.compress_respons…

…e parameters. Set their behavior automatically based on settings.idp_sso_service_binding and settings.idp_slo_service_binding respectively. HTTP-Redirect will always use compression, while HTTP-POST will not.

CHANGELOG.md

@@ -6,6 +6,7 @@
 * [#685](https://github.com/SAML-Toolkits/ruby-saml/pull/685) Change directly structure from `lib/onelogin/ruby-saml` to `lib/ruby_saml`.
 * [#685](https://github.com/SAML-Toolkits/ruby-saml/pull/685) Move schema files from `lib/onelogin/schemas` to `lib/ruby_saml/schemas`.
 * [#686](https://github.com/SAML-Toolkits/ruby-saml/pull/686) Use SHA-256 as the default hashing algorithm everywhere instead of SHA-1, including signatures, fingerprints, and digests.
+* [#695](https://github.com/SAML-Toolkits/ruby-saml/pull/695) Deprecate `settings.compress_request` and `settings.compess_response` parameters.
 
 ### 1.17.0
 * [#673](https://github.com/SAML-Toolkits/ruby-saml/pull/673) Add `Settings#sp_cert_multi` paramter to facilitate SP certificate and key rotation.

UPGRADING.md

@@ -35,6 +35,18 @@ settings.security[:digest_method] = XMLSecurity::Document::SHA1
 settings.security[:signature_method] = XMLSecurity::Document::RSA_SHA1
 ```
 
+### 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 `compress_response` everywhere within your project code.
+
+The SAML SP request/response message compression behavior is now controlled automatically by the
+`settings.idp_sso_service_binding` and `settings.idp_slo_service_binding` parameters respectively:
+`HTTP-Redirect` will always use compression, while `HTTP-POST` will not. For clarity, here
+"compression" is used to make redirect URLs which contain SAML messages be shorter. For POST messages,
+compression may be achieved by enabling `Content-Encoding: gzip` on your webserver.
+
 ## Updating 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

lib/ruby_saml/authrequest.rb

@@ -56,6 +56,7 @@ def create_params(settings, params={})
       # The method expects :RelayState but sometimes we get 'RelayState' instead.
       # Based on the HashWithIndifferentAccess value in Rails we could experience
       # conflicts so this line will solve them.
+      binding_redirect = settings.idp_sso_service_binding == Utils::BINDINGS[:redirect]
       relay_state = params[:RelayState] || params['RelayState']
 
       if relay_state.nil?
@@ -71,12 +72,12 @@ def create_params(settings, params={})
 
       Logging.debug "Created AuthnRequest: #{request}"
 
-      request = deflate(request) if settings.compress_request
+      request = deflate(request) if binding_redirect
       base64_request = encode(request)
       request_params = {"SAMLRequest" => base64_request}
       sp_signing_key = settings.get_sp_signing_key
 
-      if settings.idp_sso_service_binding == Utils::BINDINGS[:redirect] && settings.security[:authn_requests_signed] && sp_signing_key
+      if binding_redirect && settings.security[:authn_requests_signed] && sp_signing_key
         params['SigAlg'] = settings.security[:signature_method]
         url_string = RubySaml::Utils.build_query(
           type: 'SAMLRequest',

lib/ruby_saml/logging.rb

@@ -2,32 +2,33 @@
 
 require 'logger'
 
-# Simplistic log class when we're running in Rails
 module RubySaml
-  class Logging
+  module Logging
+    extend self
+
     DEFAULT_LOGGER = ::Logger.new($stdout)
 
-    def self.logger
+    attr_writer :logger
+
+    def logger
       @logger ||= begin
         logger = Rails.logger if defined?(::Rails) && Rails.respond_to?(:logger)
-        logger ||= DEFAULT_LOGGER
+        logger || DEFAULT_LOGGER
       end
     end
 
-    class << self
-      attr_writer :logger
+    %i[error warn debug info].each do |level|
+      define_method(level) do |message|
+        logger.send(level, message) if enabled?
+      end
     end
 
-    def self.debug(message)
-      return if ENV['ruby-saml/testing']
-
-      logger.debug(message)
+    def deprecate(message)
+      warn("[DEPRECATION] RubySaml: #{message}")
     end
 
-    def self.info(message)
-      return if ENV['ruby-saml/testing']
-
-      logger.info(message)
+    def enabled?
+      !ENV['ruby-saml/testing']
     end
   end
 end

lib/ruby_saml/logoutrequest.rb

@@ -53,6 +53,7 @@ def create_params(settings, params={})
       # The method expects :RelayState but sometimes we get 'RelayState' instead.
       # Based on the HashWithIndifferentAccess value in Rails we could experience
       # conflicts so this line will solve them.
+      binding_redirect = settings.idp_slo_service_binding == Utils::BINDINGS[:redirect]
       relay_state = params[:RelayState] || params['RelayState']
 
       if relay_state.nil?
@@ -68,12 +69,12 @@ def create_params(settings, params={})
 
       Logging.debug "Created SLO Logout Request: #{request}"
 
-      request = deflate(request) if settings.compress_request
+      request = deflate(request) if binding_redirect
       base64_request = encode(request)
       request_params = {"SAMLRequest" => base64_request}
       sp_signing_key = settings.get_sp_signing_key
 
-      if settings.idp_slo_service_binding == Utils::BINDINGS[:redirect] && settings.security[:logout_requests_signed] && sp_signing_key
+      if binding_redirect && settings.security[:logout_requests_signed] && sp_signing_key
         params['SigAlg'] = settings.security[:signature_method]
         url_string = RubySaml::Utils.build_query(
           type: 'SAMLRequest',

lib/ruby_saml/saml_message.rb

@@ -7,6 +7,7 @@
 require 'rexml/document'
 require 'rexml/xpath'
 require 'ruby_saml/error_handling'
+require 'ruby_saml/logging'
 
 # Only supports SAML 2.0
 module RubySaml
@@ -104,11 +105,18 @@ def decode_raw_saml(saml, settings = nil)
 
     # Deflate, base64 encode and url-encode a SAML Message (To be used in the HTTP-redirect binding)
     # @param saml [String] The plain SAML Message
-    # @param settings [RubySaml::Settings|nil] Toolkit settings
+    # @param settings_or_compress [true|false|RubySaml::Settings|nil] Whether or not the SAML should be deflated.
+    #   The usage of RubySaml::Settings here is deprecated.
     # @return [String] The deflated and encoded SAML Message (encoded if the compression is requested)
-    #
-    def encode_raw_saml(saml, settings)
-      saml = deflate(saml) if settings.compress_request
+    def encode_raw_saml(saml, settings_or_compress = false)
+      if settings_or_compress.is_a?(TrueClass)
+        saml = deflate(saml)
+      elsif settings_or_compress.respond_to?(:compress_request)
+        Logging.deprecate('Please change the second argument of `encode_raw_saml_message` to a boolean '\
+                          'indicating whether or not to use compression. Using a boolean will be required '\
+                          'in RubySaml 2.1.0.')
+        saml = deflate(saml) if settings_or_compress.compress_request
+      end
 
       CGI.escape(encode(saml))
     end

lib/ruby_saml/settings.rb

@@ -52,8 +52,6 @@ def initialize(overrides = {}, keep_security_attributes = false)
     attr_accessor :name_identifier_value
     attr_accessor :name_identifier_value_requested
     attr_accessor :sessionindex
-    attr_accessor :compress_request
-    attr_accessor :compress_response
     attr_accessor :double_quote_xml_attribute_values
     attr_accessor :message_max_bytesize
     attr_accessor :passive
@@ -278,8 +276,6 @@ def get_binding(value)
       assertion_consumer_service_binding: Utils::BINDINGS[:post],
       single_logout_service_binding: Utils::BINDINGS[:redirect],
       idp_cert_fingerprint_algorithm: XMLSecurity::Document::SHA256,
-      compress_request: true,
-      compress_response: true,
       message_max_bytesize: 250_000,
       soft: true,
       double_quote_xml_attribute_values: false,
@@ -301,8 +297,40 @@ def get_binding(value)
       }.freeze
     }.freeze
 
+    # @deprecated Will be removed in v2.1.0
+    def compress_request
+      compress_deprecation('compress_request', 'idp_sso_service_binding')
+      @compress_request
+    end
+
+    # @deprecated Will be removed in v2.1.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
+    def compress_response
+      compress_deprecation('compress_response', 'idp_slo_service_binding')
+      @compress_response
+    end
+
+    # @deprecated Will be removed in v2.1.0
+    def compress_response=(value)
+      compress_deprecation('compress_response', 'idp_slo_service_binding')
+      @compress_response = value
+    end
+
     private
 
+    # @deprecated Will be removed in v2.1.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. '\
+                        "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
+
     # @return [Hash<Symbol, Array<Array<OpenSSL::X509::Certificate, OpenSSL::PKey::RSA>>>]
     #   Build the SP certificates and private keys from the settings. Returns all
     #   certificates and private keys, even if they are expired.

lib/ruby_saml/slo_logoutresponse.rb

@@ -62,6 +62,7 @@ def create_params(settings, request_id = nil, logout_message = nil, params = {},
       # The method expects :RelayState but sometimes we get 'RelayState' instead.
       # Based on the HashWithIndifferentAccess value in Rails we could experience
       # conflicts so this line will solve them.
+      binding_redirect = settings.idp_slo_service_binding == Utils::BINDINGS[:redirect]
       relay_state = params[:RelayState] || params['RelayState']
 
       if relay_state.nil?
@@ -77,12 +78,12 @@ def create_params(settings, request_id = nil, logout_message = nil, params = {},
 
       Logging.debug "Created SLO Logout Response: #{response}"
 
-      response = deflate(response) if settings.compress_response
+      response = deflate(response) if binding_redirect
       base64_response = encode(response)
       response_params = {"SAMLResponse" => base64_response}
       sp_signing_key = settings.get_sp_signing_key
 
-      if settings.idp_slo_service_binding == Utils::BINDINGS[:redirect] && settings.security[:logout_responses_signed] && sp_signing_key
+      if binding_redirect && settings.security[:logout_responses_signed] && sp_signing_key
         params['SigAlg'] = settings.security[:signature_method]
         url_string = RubySaml::Utils.build_query(
           type: 'SAMLResponse',

test/logoutrequest_test.rb

@@ -152,21 +152,7 @@ class RequestTest < Minitest::Test
         assert_match %r[<ds:DigestMethod Algorithm='http://www.w3.org/2001/04/xmlenc#sha256'/>], inflated
       end
 
-      it "create a signed logout request" do
-        settings.compress_request = true
-
-        unauth_req = RubySaml::Logoutrequest.new
-        unauth_url = unauth_req.create(settings)
-
-        inflated = decode_saml_request_payload(unauth_url)
-        assert_match %r[<ds:SignatureValue>([a-zA-Z0-9/+=]+)</ds:SignatureValue>], inflated
-        assert_match %r[<ds:SignatureMethod Algorithm='http://www.w3.org/2001/04/xmldsig-more#rsa-sha256'/>], inflated
-        assert_match %r[<ds:DigestMethod Algorithm='http://www.w3.org/2001/04/xmlenc#sha256'/>], inflated
-      end
-
       it "create an uncompressed signed logout request" do
-        settings.compress_request = false
-
         params = RubySaml::Logoutrequest.new.create_params(settings)
         request_xml = Base64.decode64(params["SAMLRequest"])
 
@@ -176,7 +162,6 @@ class RequestTest < Minitest::Test
       end
 
       it "create a signed logout request with 256 digest and signature method" do
-        settings.compress_request = false
         settings.security[:signature_method] = XMLSecurity::Document::RSA_SHA256
         settings.security[:digest_method] = XMLSecurity::Document::SHA256
 
@@ -188,7 +173,6 @@ class RequestTest < Minitest::Test
       end
 
       it "create a signed logout request with 512 digest and signature method RSA_SHA384" do
-        settings.compress_request = false
         settings.security[:signature_method] = XMLSecurity::Document::RSA_SHA384
         settings.security[:digest_method] = XMLSecurity::Document::SHA512
 
@@ -201,7 +185,6 @@ class RequestTest < Minitest::Test
       end
 
       it "create a signed logout request using the first certificate and key" do
-        settings.compress_request = false
         settings.certificate = nil
         settings.private_key = nil
         settings.sp_cert_multi = {
@@ -220,7 +203,6 @@ class RequestTest < Minitest::Test
       end
 
       it "create a signed logout request using the first valid certificate and key when :check_sp_cert_expiration is true" do
-        settings.compress_request = false
         settings.certificate = nil
         settings.private_key = nil
         settings.security[:check_sp_cert_expiration] = true
@@ -328,7 +310,6 @@ class RequestTest < Minitest::Test
 
       it "create a signature parameter using the first certificate and key" do
         settings.security[:signature_method] = XMLSecurity::Document::RSA_SHA1
-        settings.compress_request = false
         settings.certificate = nil
         settings.private_key = nil
         settings.sp_cert_multi = {
@@ -366,19 +347,17 @@ class RequestTest < Minitest::Test
 
       before do
         # sign the logout request
+        settings.idp_slo_service_binding = RubySaml::Utils::BINDINGS[:post]
         settings.security[:logout_requests_signed] = true
         settings.security[:embed_sign] = true
         settings.certificate = ruby_saml_cert_text
         settings.private_key = ruby_saml_key_text
       end
 
       it "created a signed logout request" do
-        settings.compress_request = true
-
         unauth_req = RubySaml::Logoutrequest.new
-        unauth_url = unauth_req.create(settings)
+        inflated = unauth_req.create_logout_request_xml_doc(settings).to_s
 
-        inflated = decode_saml_request_payload(unauth_url)
         assert_match %r[<ds:SignatureValue>([a-zA-Z0-9/+=]+)</ds:SignatureValue>], inflated
         assert_match %r[<ds:SignatureMethod Algorithm='http://www.w3.org/2001/04/xmldsig-more#rsa-sha256'/>], inflated
         assert_match %r[<ds:DigestMethod Algorithm='http://www.w3.org/2001/04/xmlenc#sha256'/>], inflated

test/request_test.rb

@@ -42,7 +42,7 @@ class RequestTest < Minitest::Test
     end
 
     it "create the SAMLRequest URL parameter without deflating" do
-      settings.compress_request = false
+      settings.idp_sso_service_binding = RubySaml::Utils::BINDINGS[:post]
       auth_url = RubySaml::Authrequest.new.create(settings)
       assert_match(/^http:\/\/example\.com\?SAMLRequest=/, auth_url)
       payload  = CGI.unescape(auth_url.split("=").last)
@@ -242,7 +242,6 @@ class RequestTest < Minitest::Test
 
     describe "#create_params signing with HTTP-POST binding" do
       before do
-        settings.compress_request = false
         settings.idp_sso_service_url = "http://example.com?field=value"
         settings.idp_sso_service_binding = :post
         settings.security[:authn_requests_signed] = true
@@ -317,7 +316,6 @@ class RequestTest < Minitest::Test
       let(:cert) { OpenSSL::X509::Certificate.new(ruby_saml_cert_text) }
 
       before do
-        settings.compress_request = false
         settings.idp_sso_service_url = "http://example.com?field=value"
         settings.idp_sso_service_binding = :redirect
         settings.assertion_consumer_service_binding = "urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST-SimpleSign"
@@ -362,7 +360,6 @@ class RequestTest < Minitest::Test
 
       it "create a signature parameter using the first certificate and key" do
         settings.security[:signature_method] = XMLSecurity::Document::RSA_SHA1
-        settings.compress_request = false
         settings.certificate = nil
         settings.private_key = nil
         settings.sp_cert_multi = {
@@ -432,7 +429,6 @@ class RequestTest < Minitest::Test
 
     describe "DEPRECATED: #create_params signing with HTTP-POST binding via :embed_sign" do
       before do
-        settings.compress_request = false
         settings.idp_sso_service_url = "http://example.com?field=value"
         settings.security[:authn_requests_signed] = true
         settings.security[:embed_sign] = true
@@ -452,7 +448,6 @@ class RequestTest < Minitest::Test
       let(:cert) { OpenSSL::X509::Certificate.new(ruby_saml_cert_text) }
 
       before do
-        settings.compress_request = false
         settings.idp_sso_service_url = "http://example.com?field=value"
         settings.assertion_consumer_service_binding = "urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST-SimpleSign"
         settings.security[:authn_requests_signed] = true

test/saml_message_test.rb

@@ -15,6 +15,15 @@ class RubySamlTest < Minitest::Test
     end
 
     it "return encoded raw saml" do
+      encoded_raw = saml_message.send(:encode_raw_saml, logout_request_document, true)
+      assert logout_request_deflated_base64, encoded_raw
+
+      deflated = saml_message.send(:deflate, logout_request_deflated_base64)
+      encoded_raw = saml_message.send(:encode_raw_saml, deflated, false)
+      assert logout_request_deflated_base64, encoded_raw
+    end
+
+    it "DEPRECATED: return encoded raw saml using RubySaml::Settings as the second arg" do
       settings.compress_request = true
       encoded_raw = saml_message.send(:encode_raw_saml, logout_request_document, settings)
       assert logout_request_deflated_base64, encoded_raw

test/settings_test.rb

@@ -17,7 +17,7 @@ class SettingsTest < Minitest::Test
         :idp_attribute_names, :issuer, :assertion_consumer_service_url, :single_logout_service_url,
         :sp_name_qualifier, :name_identifier_format, :name_identifier_value, :name_identifier_value_requested,
         :sessionindex, :attributes_index, :passive, :force_authn,
-        :compress_request, :double_quote_xml_attribute_values, :message_max_bytesize,
+        :double_quote_xml_attribute_values, :message_max_bytesize,
         :security, :certificate, :private_key, :certificate_new, :sp_cert_multi,
         :authn_context, :authn_context_comparison, :authn_context_decl_ref,
         :assertion_consumer_logout_service_url