Merge pull request #699 from johnnyshields/settings-params-deprecations

v2.0: Settings params deprecations
SAML-Toolkits · Jul 9, 2024 · 3229214 · 3229214
2 parents 8748250 + 77dd735
commit 3229214
Show file tree

Hide file tree

Showing 4 changed files with 67 additions and 52 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -9,6 +9,7 @@
 * [#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.
 * [#690](https://github.com/SAML-Toolkits/ruby-saml/pull/690) Remove deprecated `settings.security[:embed_sign]` parameter.
+* [#697](https://github.com/SAML-Toolkits/ruby-saml/pull/697) Add deprecation for various parameters in `RubySaml::Settings`.
 
 ### 1.17.0
 * [#687](https://github.com/SAML-Toolkits/ruby-saml/pull/687) Add CI coverage for Ruby 3.3 and Windows.

diff --git a/UPGRADING.md b/UPGRADING.md
@@ -80,6 +80,18 @@ The SAML SP request/response message compression behavior is now controlled auto
 "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.
 
+## Settings deprecations
+
+The following parameters in `RubySaml::Settings` are deprecated and will be removed in RubySaml 2.1.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`
+- `#idp_slo_target_url` is deprecated and replaced 1:1 by `#idp_slo_service_url`
+- `#assertion_consumer_logout_service_url` is deprecated and replaced 1:1 by `#single_logout_service_url`
+- `#assertion_consumer_logout_service_binding` is deprecated and replaced 1:1 by `#single_logout_service_binding`
+- `#certificate_new` is deprecated and replaced by `#sp_cert_multi`. Refer to documentation as `#sp_cert_multi`
+  has a different value type than `#certificate_new`.
+
 ## 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

diff --git a/lib/ruby_saml/settings.rb b/lib/ruby_saml/settings.rb
@@ -32,8 +32,8 @@ def initialize(overrides = {}, keep_security_attributes = false)
 
     # IdP Data
     attr_accessor :idp_entity_id
-    attr_writer   :idp_sso_service_url
-    attr_writer   :idp_slo_service_url
+    attr_accessor :idp_sso_service_url
+    attr_accessor :idp_slo_service_url
     attr_accessor :idp_slo_response_service_url
     attr_accessor :idp_cert
     attr_accessor :idp_cert_fingerprint
@@ -43,10 +43,11 @@ def initialize(overrides = {}, keep_security_attributes = false)
     attr_accessor :idp_name_qualifier
     attr_accessor :valid_until
     # SP Data
-    attr_writer   :sp_entity_id
+    attr_accessor :sp_entity_id
     attr_accessor :assertion_consumer_service_url
     attr_reader   :assertion_consumer_service_binding
-    attr_writer   :single_logout_service_url
+    attr_accessor :single_logout_service_url
+    attr_reader   :single_logout_service_binding
     attr_accessor :sp_name_qualifier
     attr_accessor :name_identifier_format
     attr_accessor :name_identifier_value
@@ -68,25 +69,6 @@ def initialize(overrides = {}, keep_security_attributes = false)
     # Work-flow
     attr_accessor :security
     attr_accessor :soft
-    # Deprecated
-    attr_accessor :certificate_new
-    attr_accessor :assertion_consumer_logout_service_url
-    attr_reader   :assertion_consumer_logout_service_binding
-    attr_accessor :issuer
-    attr_accessor :idp_sso_target_url
-    attr_accessor :idp_slo_target_url
-
-    # @return [String] IdP Single Sign On Service URL
-    #
-    def idp_sso_service_url
-      @idp_sso_service_url || @idp_sso_target_url
-    end
-
-    # @return [String] IdP Single Logout Service URL
-    #
-    def idp_slo_service_url
-      @idp_slo_service_url || @idp_slo_target_url
-    end
 
     # @return [String] IdP Single Sign On Service Binding
     #
@@ -114,12 +96,6 @@ def idp_slo_service_binding=(value)
       @idp_slo_service_binding = get_binding(value)
     end
 
-    # @return [String] SP Entity ID
-    #
-    def sp_entity_id
-      @sp_entity_id || @issuer
-    end
-
     # Setter for SP Protocol Binding
     # @param value [String, Symbol].
     #
@@ -134,18 +110,6 @@ def assertion_consumer_service_binding=(value)
       @assertion_consumer_service_binding = get_binding(value)
     end
 
-    # @return [String] Single Logout Service URL.
-    #
-    def single_logout_service_url
-      @single_logout_service_url || @assertion_consumer_logout_service_url
-    end
-
-    # @return [String] Single Logout Service Binding.
-    #
-    def single_logout_service_binding
-      @single_logout_service_binding || @assertion_consumer_logout_service_binding
-    end
-
     # Setter for Single Logout Service Binding.
     #
     # (Currently we only support "urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect")
@@ -155,15 +119,6 @@ def single_logout_service_binding=(value)
       @single_logout_service_binding = get_binding(value)
     end
 
-    # @deprecated Setter for legacy Single Logout Service Binding parameter.
-    #
-    # (Currently we only support "urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect")
-    # @param value [String, Symbol]
-    #
-    def assertion_consumer_logout_service_binding=(value)
-      @assertion_consumer_logout_service_binding = get_binding(value)
-    end
-
     # Calculates the fingerprint of the IdP x509 certificate.
     # @return [String] The fingerprint
     #
@@ -292,6 +247,38 @@ def get_binding(value)
       }.freeze
     }.freeze
 
+    {
+      issuer: :sp_entity_id,
+      idp_sso_target_url: :idp_sso_service_url,
+      idp_slo_target_url: :idp_slo_service_url,
+      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
+      define_method(old_param) do
+        replaced_deprecation(old_param, new_param)
+        send(new_param)
+      end
+
+      # @deprecated Will be removed in v2.1.0
+      define_method(:"#{old_param}=") do |value|
+        replaced_deprecation(old_param, new_param)
+        send(:"#{new_param}=", value)
+      end
+    end
+
+    # @deprecated Will be removed in v2.1.0
+    def certificate_new
+      certificate_new_deprecation
+      @certificate_new
+    end
+
+    # @deprecated Will be removed in v2.1.0
+    def certificate_new=(value)
+      certificate_new_deprecation
+      @certificate_new = value
+    end
+
     # @deprecated Will be removed in v2.1.0
     def compress_request
       compress_deprecation('compress_request', 'idp_sso_service_binding')
@@ -318,6 +305,19 @@ def compress_response=(value)
 
     private
 
+    # @deprecated Will be removed in v2.1.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. " \
+                        "Please set the same value to `RubySaml::Settings##{new_param}` instead."
+    end
+
+    # @deprecated Will be removed in v2.1.0
+    def certificate_new_deprecation
+      Logging.deprecate '`RubySaml::Settings#certificate_new` is deprecated and will be removed in RubySaml v2.1.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
     def compress_deprecation(old_param, new_param)
       Logging.deprecate "`RubySaml::Settings##{old_param}` is deprecated and no longer functional. " \

diff --git a/test/settings_test.rb b/test/settings_test.rb
@@ -35,8 +35,10 @@ class SettingsTest < Minitest::Test
 
     it "should provide getters and settings for binding parameters" do
       accessors = [
-        :protocol_binding, :assertion_consumer_service_binding,
-        :single_logout_service_binding, :assertion_consumer_logout_service_binding
+        :protocol_binding,
+        :assertion_consumer_service_binding,
+        :single_logout_service_binding,
+        :assertion_consumer_logout_service_binding
       ]
 
       accessors.each do |accessor|