From 10bdc2b6d90ca0a0c2a4f3b9d03f2b22f5e5245f Mon Sep 17 00:00:00 2001 From: johnnyshields <27655+johnnyshields@users.noreply.github.com> Date: Sat, 11 Jan 2025 13:43:05 +0900 Subject: [PATCH 01/13] This PR replaces fixes the way SP request UUID generation is handled. - Replace the mutable RubySaml::Utils::UUID_PREFIX constant with `Settings.sp_uuid_prefix` - Make Authnrequest, etc. `uuid` attribute to be immutable. - Initialize Authnrequest, etc. `uuid` attribute when `create` is called, based on `Settings.sp_uuid_prefix`, not when instantiating the Ruby object. --- lib/ruby_saml/authrequest.rb | 16 +++------------- lib/ruby_saml/logoutrequest.rb | 16 +++------------- lib/ruby_saml/settings.rb | 1 + lib/ruby_saml/slo_logoutresponse.rb | 16 +++------------- lib/ruby_saml/utils.rb | 20 +++++++++++++++----- test/test_helper.rb | 2 +- 6 files changed, 26 insertions(+), 45 deletions(-) diff --git a/lib/ruby_saml/authrequest.rb b/lib/ruby_saml/authrequest.rb index 6aeccc10..ce13a27f 100644 --- a/lib/ruby_saml/authrequest.rb +++ b/lib/ruby_saml/authrequest.rb @@ -15,19 +15,8 @@ module RubySaml class Authrequest < SamlMessage # AuthNRequest ID - attr_accessor :uuid - - # Initializes the AuthNRequest. An Authrequest Object that is an extension of the SamlMessage class. - # Asigns an ID, a random uuid. - # - def initialize - @uuid = RubySaml::Utils.uuid - super() - end - - def request_id - @uuid - end + attr_reader :uuid + alias_method :request_id, :uuid # Creates the AuthNRequest string. # @param settings [RubySaml::Settings|nil] Toolkit settings @@ -35,6 +24,7 @@ def request_id # @return [String] AuthNRequest string that includes the SAMLRequest # def create(settings, params = {}) + @uuid = RubySaml::Utils.generate_uuid(settings.sp_uuid_prefix) params = create_params(settings, params) params_prefix = /\?/.match?(settings.idp_sso_service_url) ? '&' : '?' saml_request = CGI.escape(params.delete("SAMLRequest")) diff --git a/lib/ruby_saml/logoutrequest.rb b/lib/ruby_saml/logoutrequest.rb index 9ebf325c..4f10ff3e 100644 --- a/lib/ruby_saml/logoutrequest.rb +++ b/lib/ruby_saml/logoutrequest.rb @@ -13,19 +13,8 @@ module RubySaml class Logoutrequest < SamlMessage # Logout Request ID - attr_accessor :uuid - - # Initializes the Logout Request. A Logoutrequest Object that is an extension of the SamlMessage class. - # Asigns an ID, a random uuid. - # - def initialize - @uuid = RubySaml::Utils.uuid - super() - end - - def request_id - @uuid - end + attr_reader :uuid + alias_method :request_id, :uuid # Creates the Logout Request string. # @param settings [RubySaml::Settings|nil] Toolkit settings @@ -33,6 +22,7 @@ def request_id # @return [String] Logout Request string that includes the SAMLRequest # def create(settings, params={}) + @uuid = RubySaml::Utils.generate_uuid(settings.sp_uuid_prefix) params = create_params(settings, params) params_prefix = /\?/.match?(settings.idp_slo_service_url) ? '&' : '?' saml_request = CGI.escape(params.delete("SAMLRequest")) diff --git a/lib/ruby_saml/settings.rb b/lib/ruby_saml/settings.rb index 1d90e887..44af1028 100644 --- a/lib/ruby_saml/settings.rb +++ b/lib/ruby_saml/settings.rb @@ -48,6 +48,7 @@ def initialize(overrides = {}, keep_security_attributes = false) attr_reader :assertion_consumer_service_binding attr_accessor :single_logout_service_url attr_reader :single_logout_service_binding + attr_accessor :sp_uuid_prefix attr_accessor :sp_name_qualifier attr_accessor :name_identifier_format attr_accessor :name_identifier_value diff --git a/lib/ruby_saml/slo_logoutresponse.rb b/lib/ruby_saml/slo_logoutresponse.rb index 3cd5a215..d77faed8 100644 --- a/lib/ruby_saml/slo_logoutresponse.rb +++ b/lib/ruby_saml/slo_logoutresponse.rb @@ -14,19 +14,8 @@ module RubySaml class SloLogoutresponse < SamlMessage # Logout Response ID - attr_accessor :uuid - - # Initializes the Logout Response. A SloLogoutresponse Object that is an extension of the SamlMessage class. - # Asigns an ID, a random uuid. - # - def initialize - @uuid = RubySaml::Utils.uuid - super() - end - - def response_id - @uuid - end + attr_reader :uuid + alias_method :request_id, :uuid # Creates the Logout Response string. # @param settings [RubySaml::Settings|nil] Toolkit settings @@ -37,6 +26,7 @@ def response_id # @return [String] Logout Request string that includes the SAMLRequest # def create(settings, request_id = nil, logout_message = nil, params = {}, logout_status_code = nil) + @uuid = RubySaml::Utils.generate_uuid(settings.sp_uuid_prefix) params = create_params(settings, request_id, logout_message, params, logout_status_code) params_prefix = /\?/.match?(settings.idp_slo_service_url) ? '&' : '?' url = settings.idp_slo_response_service_url || settings.idp_slo_service_url diff --git a/lib/ruby_saml/utils.rb b/lib/ruby_saml/utils.rb index fa71174f..b1ddb969 100644 --- a/lib/ruby_saml/utils.rb +++ b/lib/ruby_saml/utils.rb @@ -30,7 +30,10 @@ module Utils (\d+)W # 8: Weeks ) $/x - UUID_PREFIX = +'_' + UUID_DEFAULT_PREFIX = '_' + + # @deprecated Use UUID_DEFAULT_PREFIX instead. + UUID_PREFIX = UUID_DEFAULT_PREFIX.dup # Checks if the x509 cert provided is expired. # @@ -380,13 +383,20 @@ def retrieve_plaintext(cipher_text, symmetric_key, algorithm) end end - def set_prefix(value) - UUID_PREFIX.replace value + def set_prefix(_value) + raise NoMethodError.new('RubySaml::Util.set_prefix has been removed. Please use RubySaml::Settings#uuid_prefix instead.') end - def uuid - "#{UUID_PREFIX}#{SecureRandom.uuid}" + # Generates a UUID with a prefix. + # + # @param prefix [String|false|nil] An explicit prefix override. + # Using nil will use the default prefix, and false will use no prefix. + # @return [String] The generated UUID. + def generate_uuid(prefix = nil) + prefix = prefix.is_a?(FalseClass) ? nil : prefix || UUID_DEFAULT_PREFIX + "#{prefix}#{SecureRandom.uuid}" end + alias_method :uuid, :generate_uuid # Given two strings, attempt to match them as URIs using Rails' parse method. If they can be parsed, # then the fully-qualified domain name and the host should performa a case-insensitive match, per the diff --git a/test/test_helper.rb b/test/test_helper.rb index 61581475..b1d7aed2 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -374,7 +374,7 @@ def ruby_saml_key_text # logoutresponse fixtures # def random_id - "_#{RubySaml::Utils.uuid}" + "_#{RubySaml::Utils.generate_uuid}" end # From 153497b4948d80fe6667fdb4d2f2559a7ba6426e Mon Sep 17 00:00:00 2001 From: johnnyshields <27655+johnnyshields@users.noreply.github.com> Date: Sat, 11 Jan 2025 13:51:16 +0900 Subject: [PATCH 02/13] Fix rubocop --- .rubocop.yml | 3 +++ README.md | 6 ++++++ lib/ruby_saml/utils.rb | 2 +- 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/.rubocop.yml b/.rubocop.yml index 7a7f2e7e..57ea3456 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -16,6 +16,9 @@ AllCops: - 'tmp/**/*' - 'vendor/**/*' +Style/Alias: + EnforcedStyle: prefer_alias_method + Style/ModuleFunction: EnforcedStyle: extend_self diff --git a/README.md b/README.md index 06c40e74..9ba593d2 100644 --- a/README.md +++ b/README.md @@ -963,6 +963,12 @@ end The `attribute_value` option additionally accepts an array of possible values. +## SP Request UUIDs + +By default, Ruby SAML will generate UUIDs for SP requests prefixed with the `_` character, +for example `"_ea8b5fdf-0a71-4bef-9f87-5406ee746f5b"`. To override this behavior, +you may set `settings.sp_uuid_prefix` to a string of your choice, or `false` to use no prefix. + ## Custom Metadata Fields Some IdPs may require to add SPs to add additional fields (Organization, ContactPerson, etc.) diff --git a/lib/ruby_saml/utils.rb b/lib/ruby_saml/utils.rb index b1ddb969..31395649 100644 --- a/lib/ruby_saml/utils.rb +++ b/lib/ruby_saml/utils.rb @@ -8,7 +8,7 @@ module RubySaml # SAML2 Auxiliary class # - module Utils + module Utils # rubocop:disable Metrics/ModuleLength extend self BINDINGS = { post: "urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST", From fd41c6979a128b5a0c39f9efe96aa8e7f0d5b8d4 Mon Sep 17 00:00:00 2001 From: johnnyshields <27655+johnnyshields@users.noreply.github.com> Date: Tue, 14 Jan 2025 07:38:12 +0900 Subject: [PATCH 03/13] Fix tests --- lib/ruby_saml/authrequest.rb | 2 +- lib/ruby_saml/logoutrequest.rb | 2 +- lib/ruby_saml/slo_logoutresponse.rb | 5 ++- test/authrequest_test.rb | 51 ++++++++++++++++++----------- test/logoutrequest_test.rb | 32 ++++++++++-------- test/slo_logoutresponse_test.rb | 43 ++++++++++++++---------- 6 files changed, 80 insertions(+), 55 deletions(-) diff --git a/lib/ruby_saml/authrequest.rb b/lib/ruby_saml/authrequest.rb index ce13a27f..84b81f65 100644 --- a/lib/ruby_saml/authrequest.rb +++ b/lib/ruby_saml/authrequest.rb @@ -15,7 +15,7 @@ module RubySaml class Authrequest < SamlMessage # AuthNRequest ID - attr_reader :uuid + attr_accessor :uuid alias_method :request_id, :uuid # Creates the AuthNRequest string. diff --git a/lib/ruby_saml/logoutrequest.rb b/lib/ruby_saml/logoutrequest.rb index 4f10ff3e..f89ab8d0 100644 --- a/lib/ruby_saml/logoutrequest.rb +++ b/lib/ruby_saml/logoutrequest.rb @@ -13,7 +13,7 @@ module RubySaml class Logoutrequest < SamlMessage # Logout Request ID - attr_reader :uuid + attr_accessor :uuid alias_method :request_id, :uuid # Creates the Logout Request string. diff --git a/lib/ruby_saml/slo_logoutresponse.rb b/lib/ruby_saml/slo_logoutresponse.rb index d77faed8..e8fc4680 100644 --- a/lib/ruby_saml/slo_logoutresponse.rb +++ b/lib/ruby_saml/slo_logoutresponse.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true require "ruby_saml/logging" - require "ruby_saml/saml_message" require "ruby_saml/utils" require "ruby_saml/setting_error" @@ -14,8 +13,8 @@ module RubySaml class SloLogoutresponse < SamlMessage # Logout Response ID - attr_reader :uuid - alias_method :request_id, :uuid + attr_accessor :uuid + alias_method :response_id, :uuid # Creates the Logout Response string. # @param settings [RubySaml::Settings|nil] Toolkit settings diff --git a/test/authrequest_test.rb b/test/authrequest_test.rb index 45468511..0a8e121c 100644 --- a/test/authrequest_test.rb +++ b/test/authrequest_test.rb @@ -161,17 +161,39 @@ class AuthrequestTest < Minitest::Test assert auth_url.include?('&RelayState=http%3A%2F%2Fexample.com') end - it "creates request with ID prefixed with default '_'" do - request = RubySaml::Authrequest.new + describe "uuid" do + it "uuid is initialized to nil" do + request = RubySaml::Authrequest.new - assert_match(/^_/, request.uuid) - end + assert_nil(request.uuid) + assert_equal request.request_id, request.uuid + end + + it "creates request with ID prefixed with default '_'" do + request = RubySaml::Authrequest.new + request.create(settings) + + assert_match(/^_/, request.uuid) + assert_equal request.request_id, request.uuid + end + + it "creates request with ID prefixed by Settings#sp_uuid_prefix" do + settings.sp_uuid_prefix = 'test' + request = RubySaml::Authrequest.new + request.create(settings) - it "creates request with ID is prefixed, when :id_prefix is passed" do - RubySaml::Utils::set_prefix("test") - request = RubySaml::Authrequest.new - assert_match(/^test/, request.uuid) - RubySaml::Utils::set_prefix("_") + assert_match(/^test/, request.uuid) + assert_equal request.request_id, request.uuid + end + + it "can mutate the uuid" do + request = RubySaml::Authrequest.new + request_id = request.request_id + assert_equal request_id, request.uuid + request.uuid = "new_uuid" + assert_equal "new_uuid", request.uuid + assert_equal request.request_id, request.uuid + end end describe "when the target url is not set" do @@ -272,17 +294,6 @@ class AuthrequestTest < Minitest::Test assert auth_doc.to_s =~ /example\/decl\/ref<\/saml:AuthnContextDeclRef>/ end - describe "#manipulate request_id" do - it "be able to modify the request id" do - authnrequest = RubySaml::Authrequest.new - request_id = authnrequest.request_id - assert_equal request_id, authnrequest.uuid - authnrequest.uuid = "new_uuid" - assert_equal authnrequest.request_id, authnrequest.uuid - assert_equal "new_uuid", authnrequest.request_id - end - end - each_signature_algorithm do |sp_key_algo, sp_hash_algo| describe "#create_params signing with HTTP-POST binding" do before do diff --git a/test/logoutrequest_test.rb b/test/logoutrequest_test.rb index 82828964..05e90767 100644 --- a/test/logoutrequest_test.rb +++ b/test/logoutrequest_test.rb @@ -94,29 +94,35 @@ class RequestTest < Minitest::Test end end - describe "playgin with preix" do + describe "uuid" do + it "uuid is initialized to nil" do + request = RubySaml::Logoutrequest.new + + assert_nil(request.uuid) + end + it "creates request with ID prefixed with default '_'" do request = RubySaml::Logoutrequest.new + request.create(settings) assert_match(/^_/, request.uuid) end - it "creates request with ID is prefixed, when :id_prefix is passed" do - RubySaml::Utils::set_prefix("test") + it "creates request with ID prefixed by Settings#sp_uuid_prefix" do + settings.sp_uuid_prefix = 'test' request = RubySaml::Logoutrequest.new + request.create(settings) + assert_match(/^test/, request.uuid) - RubySaml::Utils::set_prefix("_") end - end - describe "#manipulate request_id" do - it "be able to modify the request id" do - logoutrequest = RubySaml::Logoutrequest.new - request_id = logoutrequest.request_id - assert_equal request_id, logoutrequest.uuid - logoutrequest.uuid = "new_uuid" - assert_equal logoutrequest.request_id, logoutrequest.uuid - assert_equal "new_uuid", logoutrequest.request_id + it "can mutate the uuid" do + request = RubySaml::Logoutrequest.new + request_id = request.request_id + assert_equal request_id, request.uuid + request.uuid = "new_uuid" + assert_equal "new_uuid", request.uuid + assert_equal request.request_id, request.uuid end end diff --git a/test/slo_logoutresponse_test.rb b/test/slo_logoutresponse_test.rb index 1636c303..9f150b81 100644 --- a/test/slo_logoutresponse_test.rb +++ b/test/slo_logoutresponse_test.rb @@ -82,29 +82,38 @@ class SloLogoutresponseTest < Minitest::Test assert_match(/Destination='http:\/\/unauth.com\/logout\/return'/, inflated) end - describe "playgin with preix" do - it "creates request with ID prefixed with default '_'" do - request = RubySaml::SloLogoutresponse.new + describe "uuid" do + it "uuid is initialized to nil" do + response = RubySaml::SloLogoutresponse.new - assert_match(/^_/, request.uuid) + assert_nil(response.uuid) + assert_equal response.response_id, response.uuid end - it "creates request with ID is prefixed, when :id_prefix is passed" do - RubySaml::Utils::set_prefix("test") - request = RubySaml::SloLogoutresponse.new - assert_match(/^test/, request.uuid) - RubySaml::Utils::set_prefix("_") + it "creates response with ID prefixed with default '_'" do + response = RubySaml::SloLogoutresponse.new + response.create(settings) + + assert_match(/^_/, response.uuid) + assert_equal response.response_id, response.uuid + end + + it "creates response with ID prefixed by Settings#sp_uuid_prefix" do + settings.sp_uuid_prefix = 'test' + response = RubySaml::SloLogoutresponse.new + response.create(settings) + + assert_match(/^test/, response.uuid) + assert_equal response.response_id, response.uuid end - end - describe "#manipulate response_id" do it "be able to modify the response id" do - logoutresponse = RubySaml::SloLogoutresponse.new - response_id = logoutresponse.response_id - assert_equal response_id, logoutresponse.uuid - logoutresponse.uuid = "new_uuid" - assert_equal logoutresponse.response_id, logoutresponse.uuid - assert_equal "new_uuid", logoutresponse.response_id + response = RubySaml::SloLogoutresponse.new + response_id = response.response_id + assert_equal response_id, response.uuid + response.uuid = "new_uuid" + assert_equal "new_uuid", response.uuid + assert_equal response.response_id, response.uuid end end From 71880308e26695288179aeac4dcd4bae35f7ef4a Mon Sep 17 00:00:00 2001 From: johnnyshields <27655+johnnyshields@users.noreply.github.com> Date: Tue, 14 Jan 2025 07:54:45 +0900 Subject: [PATCH 04/13] Fix tests --- CHANGELOG.md | 3 ++- lib/ruby_saml/authrequest.rb | 7 ++++++- lib/ruby_saml/logoutrequest.rb | 7 ++++++- lib/ruby_saml/slo_logoutresponse.rb | 7 ++++++- lib/ruby_saml/utils.rb | 1 + 5 files changed, 21 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 79048f42..4a2b4e6c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,9 +13,10 @@ * [#709](https://github.com/SAML-Toolkits/ruby-saml/pull/709) Allow passing in `Net::HTTP` `:open_timeout`, `:read_timeout`, and `:max_retries` settings to `IdpMetadataParser#parse_remote`. * [#715](https://github.com/SAML-Toolkits/ruby-saml/pull/715) Fix typo in error when SPNameQualifier value does not match the SP entityID. * [#718](https://github.com/SAML-Toolkits/ruby-saml/pull/718) Add support to retrieve from SAMLResponse the AuthnInstant and AuthnContextClassRef values -* [#711](https://github.com/SAML-Toolkits/ruby-saml/pull/711) Standardize how RubySaml reads and formats certificate and private_key PEM values, including the `RubySaml::Util#format_cert` and `#format_private_key` methods. +* [#711](https://github.com/SAML-Toolkits/ruby-saml/pull/711) Standardize how RubySaml reads and formats certificate and private_key PEM values, including the `RubySaml::Utils#format_cert` and `#format_private_key` methods. * [#733](https://github.com/SAML-Toolkits/ruby-saml/pull/733) Allow `RubySaml::Utils.is_cert_expired` and `is_cert_active` to accept an optional time argument. * [#731](https://github.com/SAML-Toolkits/ruby-saml/pull/731) Add CI coverage for Ruby 3.4. Remove CI coverage for Ruby 1.x and 2.x. +* [#735](https://github.com/SAML-Toolkits/ruby-saml/pull/735) Add `Settings#sp_uuid_prefix` and deprecate `Utils#set_prefix`. ### 1.18.0 (???) * [#718](https://github.com/SAML-Toolkits/ruby-saml/pull/718) Add support to retrieve from SAMLResponse the AuthnInstant and AuthnContextClassRef values diff --git a/lib/ruby_saml/authrequest.rb b/lib/ruby_saml/authrequest.rb index 84b81f65..730f91e1 100644 --- a/lib/ruby_saml/authrequest.rb +++ b/lib/ruby_saml/authrequest.rb @@ -24,7 +24,7 @@ class Authrequest < SamlMessage # @return [String] AuthNRequest string that includes the SAMLRequest # def create(settings, params = {}) - @uuid = RubySaml::Utils.generate_uuid(settings.sp_uuid_prefix) + assign_uuid(settings) params = create_params(settings, params) params_prefix = /\?/.match?(settings.idp_sso_service_url) ? '&' : '?' saml_request = CGI.escape(params.delete("SAMLRequest")) @@ -97,6 +97,7 @@ def create_authentication_xml_doc(settings) def create_xml_document(settings) time = Time.now.utc.strftime("%Y-%m-%dT%H:%M:%SZ") + assign_uuid(settings) request_doc = RubySaml::XML::Document.new request_doc.uuid = uuid @@ -180,5 +181,9 @@ def sign_document(document, settings) document end + + def assign_uuid(settings) + @uuid ||= RubySaml::Utils.generate_uuid(settings.sp_uuid_prefix) + end end end diff --git a/lib/ruby_saml/logoutrequest.rb b/lib/ruby_saml/logoutrequest.rb index f89ab8d0..ab6f859d 100644 --- a/lib/ruby_saml/logoutrequest.rb +++ b/lib/ruby_saml/logoutrequest.rb @@ -22,7 +22,7 @@ class Logoutrequest < SamlMessage # @return [String] Logout Request string that includes the SAMLRequest # def create(settings, params={}) - @uuid = RubySaml::Utils.generate_uuid(settings.sp_uuid_prefix) + assign_uuid(settings) params = create_params(settings, params) params_prefix = /\?/.match?(settings.idp_slo_service_url) ? '&' : '?' saml_request = CGI.escape(params.delete("SAMLRequest")) @@ -95,6 +95,7 @@ def create_logout_request_xml_doc(settings) def create_xml_document(settings) time = Time.now.utc.strftime("%Y-%m-%dT%H:%M:%SZ") + assign_uuid(settings) request_doc = RubySaml::XML::Document.new request_doc.uuid = uuid @@ -139,5 +140,9 @@ def sign_document(document, settings) document end + + def assign_uuid(settings) + @uuid ||= RubySaml::Utils.generate_uuid(settings.sp_uuid_prefix) + end end end diff --git a/lib/ruby_saml/slo_logoutresponse.rb b/lib/ruby_saml/slo_logoutresponse.rb index e8fc4680..a8bb0527 100644 --- a/lib/ruby_saml/slo_logoutresponse.rb +++ b/lib/ruby_saml/slo_logoutresponse.rb @@ -25,7 +25,7 @@ class SloLogoutresponse < SamlMessage # @return [String] Logout Request string that includes the SAMLRequest # def create(settings, request_id = nil, logout_message = nil, params = {}, logout_status_code = nil) - @uuid = RubySaml::Utils.generate_uuid(settings.sp_uuid_prefix) + assign_uuid(settings) params = create_params(settings, request_id, logout_message, params, logout_status_code) params_prefix = /\?/.match?(settings.idp_slo_service_url) ? '&' : '?' url = settings.idp_slo_response_service_url || settings.idp_slo_service_url @@ -106,6 +106,7 @@ def create_logout_response_xml_doc(settings, request_id = nil, logout_message = def create_xml_document(settings, request_id = nil, logout_message = nil, status_code = nil) time = Time.now.utc.strftime('%Y-%m-%dT%H:%M:%SZ') + assign_uuid(settings) response_doc = RubySaml::XML::Document.new response_doc.uuid = uuid @@ -149,5 +150,9 @@ def sign_document(document, settings) document end + + def assign_uuid(settings) + @uuid ||= RubySaml::Utils.generate_uuid(settings.sp_uuid_prefix) + end end end diff --git a/lib/ruby_saml/utils.rb b/lib/ruby_saml/utils.rb index 854f5881..0fc55e90 100644 --- a/lib/ruby_saml/utils.rb +++ b/lib/ruby_saml/utils.rb @@ -33,6 +33,7 @@ module Utils # rubocop:disable Metrics/ModuleLength UUID_DEFAULT_PREFIX = '_' # @deprecated Use UUID_DEFAULT_PREFIX instead. + # This constant is intentionally unfrozen for backwards compatibility. UUID_PREFIX = UUID_DEFAULT_PREFIX.dup # Checks if the x509 cert provided is expired. From 3be51725f978b0040fc220ae5a944327ad667312 Mon Sep 17 00:00:00 2001 From: johnnyshields <27655+johnnyshields@users.noreply.github.com> Date: Tue, 14 Jan 2025 08:36:06 +0900 Subject: [PATCH 05/13] Add note to UPGRADING --- UPGRADING.md | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/UPGRADING.md b/UPGRADING.md index 72aa7ac6..e959d9ea 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -77,6 +77,26 @@ settings.idp_slo_service_binding = :redirect For clarity, the default value of both parameters is `:redirect` if they are not set. +### Addition of Settings sp_uuid_prefix and removal of Utils::UUID_PREFIX + +By default, the `uuid` (aliased to `request_id` / `response_id`) method in `RubySaml::Authrequest`, +`RubySaml::Logoutrequest`, and `RubySaml::Logoutresponse` uses the `_` character as a default prefix, +for example `_a1b3c5d7-9f1e-3d5c-7b1a-9f1e3d5c7b1a`. In RubySaml, versions prior to `2.0.0`, it was +possible to change this default prefix by either calling `RubySaml::Utils.set_prefix` or by mutating +the `RubySaml::Utils::UUID_PREFIX` constant (which was what `.set_prefix` did.) In RubySaml `2.0.0`, +this prefix is now set using `settings.sp_uuid_prefix`: + +```ruby +# Change the default prefix from `_` to `my_id_` +settings.sp_uuid_prefix = 'my_id_' +``` + +A side-effect of this change is that the `uuid` (aliased to `request_id` / `response_id`) method in +`RubySaml::Authrequest`, `RubySaml::Logoutrequest`, and `RubySaml::Logoutresponse` now is `nil` until +the `#create` method is called. Previously, it was generated automatically during object instantiation. +After calling `#create` for the first time the UUID will not change, even if a `Settings` object with +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 @@ -103,11 +123,10 @@ The following parameters in `RubySaml::Settings` are deprecated and will be remo ### Minor changes to Util#format_cert and #format_private_key - Version `2.0.0` standardizes how RubySaml reads and formats certificate and private key PEM strings. In general, version `2.0.0` is more permissive than `1.x`, and the changes are not anticipated to affect most users. Please note the change affects parameters -such `#idp_cert` and `#certificate`, as well as the `RubySaml::Util#format_cert` +such `#idp_cert` and `#certificate`, as well as the `RubySaml::Utils#format_cert` and `#format_private_key` methods. Specifically: From 6e027b7be6377e4871c9d2ff8a41f193b376d9e8 Mon Sep 17 00:00:00 2001 From: Johnny Shields <27655+johnnyshields@users.noreply.github.com> Date: Tue, 14 Jan 2025 09:14:17 +0900 Subject: [PATCH 06/13] Update utils.rb --- lib/ruby_saml/utils.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/ruby_saml/utils.rb b/lib/ruby_saml/utils.rb index 2489a5f8..3b9a5bc7 100644 --- a/lib/ruby_saml/utils.rb +++ b/lib/ruby_saml/utils.rb @@ -399,6 +399,7 @@ def generate_uuid(prefix = nil) prefix = prefix.is_a?(FalseClass) ? nil : prefix || UUID_DEFAULT_PREFIX "#{prefix}#{SecureRandom.uuid}" end + # @deprecated Use #generate_uuid alias_method :uuid, :generate_uuid # Given two strings, attempt to match them as URIs using Rails' parse method. If they can be parsed, From 3e3e2500d570a11f748ab5625b45f373eb134fe8 Mon Sep 17 00:00:00 2001 From: Johnny Shields <27655+johnnyshields@users.noreply.github.com> Date: Tue, 14 Jan 2025 09:48:50 +0900 Subject: [PATCH 07/13] Update UPGRADING.md --- UPGRADING.md | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/UPGRADING.md b/UPGRADING.md index e959d9ea..78036068 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -77,12 +77,12 @@ settings.idp_slo_service_binding = :redirect For clarity, the default value of both parameters is `:redirect` if they are not set. -### Addition of Settings sp_uuid_prefix and removal of Utils::UUID_PREFIX +### Change to message UUID prefix customization -By default, the `uuid` (aliased to `request_id` / `response_id`) method in `RubySaml::Authrequest`, -`RubySaml::Logoutrequest`, and `RubySaml::Logoutresponse` uses the `_` character as a default prefix, -for example `_a1b3c5d7-9f1e-3d5c-7b1a-9f1e3d5c7b1a`. In RubySaml, versions prior to `2.0.0`, it was -possible to change this default prefix by either calling `RubySaml::Utils.set_prefix` or by mutating +On SP-originated messages (`Authrequest`, `Logoutrequest`, `Logoutresponse`), RubySaml generates the +`uuid` (aliased to `request_id` / `response_id`) using the `_` character as a default prefix, +for example `_a1b3c5d7-9f1e-3d5c-7b1a-9f1e3d5c7b1a`. In RubySaml versions prior to `2.0.0`, it was +possible to change this default prefix by either `RubySaml::Utils.set_prefix` or by mutating the `RubySaml::Utils::UUID_PREFIX` constant (which was what `.set_prefix` did.) In RubySaml `2.0.0`, this prefix is now set using `settings.sp_uuid_prefix`: @@ -91,10 +91,9 @@ this prefix is now set using `settings.sp_uuid_prefix`: settings.sp_uuid_prefix = 'my_id_' ``` -A side-effect of this change is that the `uuid` (aliased to `request_id` / `response_id`) method in -`RubySaml::Authrequest`, `RubySaml::Logoutrequest`, and `RubySaml::Logoutresponse` now is `nil` until -the `#create` method is called. Previously, it was generated automatically during object instantiation. -After calling `#create` for the first time the UUID will not change, even if a `Settings` object with +A side-effect of this change is that the `uuid` of the `Authrequest`, `Logoutrequest`, and `Logoutresponse` +classes now is `nil` until the `#create` method is called (previously, it was generated during object instantiation.) +After calling `#create` for the first time the `uuid` will not change, even if a `Settings` object with a different `sp_uuid_prefix` is passed-in on subsequent calls. ### Deprecation of compression settings From b91bbc63c514ad7d23948953c3af497b0302a03b Mon Sep 17 00:00:00 2001 From: Johnny Shields <27655+johnnyshields@users.noreply.github.com> Date: Tue, 14 Jan 2025 09:52:16 +0900 Subject: [PATCH 08/13] Update UPGRADING.md --- UPGRADING.md | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/UPGRADING.md b/UPGRADING.md index 78036068..c5b1dbfd 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -89,10 +89,15 @@ this prefix is now set using `settings.sp_uuid_prefix`: ```ruby # Change the default prefix from `_` to `my_id_` settings.sp_uuid_prefix = 'my_id_' + +# Create the AuthNRequest message +request = RubySaml::Authrequest.new +request.create(settings) +request.uuid #=> "my_id_a1b3c5d7-9f1e-3d5c-7b1a-9f1e3d5c7b1a" ``` A side-effect of this change is that the `uuid` of the `Authrequest`, `Logoutrequest`, and `Logoutresponse` -classes now is `nil` until the `#create` method is called (previously, it was generated during object instantiation.) +classes now is `nil` until the `#create` method is called (previously, it was set in the constructor.) After calling `#create` for the first time the `uuid` will not change, even if a `Settings` object with a different `sp_uuid_prefix` is passed-in on subsequent calls. From b5a75487dc73a323c5a6b4e9dfdefbd722c3ffce Mon Sep 17 00:00:00 2001 From: johnnyshields <27655+johnnyshields@users.noreply.github.com> Date: Tue, 14 Jan 2025 09:56:14 +0900 Subject: [PATCH 09/13] Improve tests --- test/authrequest_test.rb | 17 ++++++++++++++--- test/logoutrequest_test.rb | 16 +++++++++++++++- test/slo_logoutresponse_test.rb | 19 +++++++++++++++---- 3 files changed, 44 insertions(+), 8 deletions(-) diff --git a/test/authrequest_test.rb b/test/authrequest_test.rb index 0a8e121c..cbbc7886 100644 --- a/test/authrequest_test.rb +++ b/test/authrequest_test.rb @@ -174,7 +174,18 @@ class AuthrequestTest < Minitest::Test request.create(settings) assert_match(/^_/, request.uuid) - assert_equal request.request_id, request.uuid + assert_equal request.uuid, request.request_id + end + + it "does not change even after repeated #create calls" do + request = RubySaml::Authrequest.new + request.create(settings) + + uuid = request.uuid + request.create(settings) + + assert_equal uuid, request.uuid + assert_equal request.uuid, request.request_id end it "creates request with ID prefixed by Settings#sp_uuid_prefix" do @@ -183,7 +194,7 @@ class AuthrequestTest < Minitest::Test request.create(settings) assert_match(/^test/, request.uuid) - assert_equal request.request_id, request.uuid + assert_equal request.uuid, request.request_id end it "can mutate the uuid" do @@ -192,7 +203,7 @@ class AuthrequestTest < Minitest::Test assert_equal request_id, request.uuid request.uuid = "new_uuid" assert_equal "new_uuid", request.uuid - assert_equal request.request_id, request.uuid + assert_equal request.uuid, request.request_id end end diff --git a/test/logoutrequest_test.rb b/test/logoutrequest_test.rb index 05e90767..dace51e7 100644 --- a/test/logoutrequest_test.rb +++ b/test/logoutrequest_test.rb @@ -99,6 +99,7 @@ class RequestTest < Minitest::Test request = RubySaml::Logoutrequest.new assert_nil(request.uuid) + assert_equal request.request_id, request.uuid end it "creates request with ID prefixed with default '_'" do @@ -106,6 +107,18 @@ class RequestTest < Minitest::Test request.create(settings) assert_match(/^_/, request.uuid) + assert_equal request.uuid, request.request_id + end + + it "does not change even after repeated #create calls" do + request = RubySaml::Logoutrequest.new + request.create(settings) + + uuid = request.uuid + request.create(settings) + + assert_equal uuid, request.uuid + assert_equal request.uuid, request.request_id end it "creates request with ID prefixed by Settings#sp_uuid_prefix" do @@ -114,6 +127,7 @@ class RequestTest < Minitest::Test request.create(settings) assert_match(/^test/, request.uuid) + assert_equal request.uuid, request.request_id end it "can mutate the uuid" do @@ -122,7 +136,7 @@ class RequestTest < Minitest::Test assert_equal request_id, request.uuid request.uuid = "new_uuid" assert_equal "new_uuid", request.uuid - assert_equal request.request_id, request.uuid + assert_equal request.uuid, request.request_id end end diff --git a/test/slo_logoutresponse_test.rb b/test/slo_logoutresponse_test.rb index 9f150b81..1706f38b 100644 --- a/test/slo_logoutresponse_test.rb +++ b/test/slo_logoutresponse_test.rb @@ -95,7 +95,18 @@ class SloLogoutresponseTest < Minitest::Test response.create(settings) assert_match(/^_/, response.uuid) - assert_equal response.response_id, response.uuid + assert_equal response.uuid, response.response_id + end + + it "does not change even after repeated #create calls" do + response = RubySaml::SloLogoutresponse.new + response.create(settings) + + uuid = response.uuid + response.create(settings) + + assert_equal uuid, response.uuid + assert_equal response.uuid, response.response_id end it "creates response with ID prefixed by Settings#sp_uuid_prefix" do @@ -104,16 +115,16 @@ class SloLogoutresponseTest < Minitest::Test response.create(settings) assert_match(/^test/, response.uuid) - assert_equal response.response_id, response.uuid + assert_equal response.uuid, response.response_id end - it "be able to modify the response id" do + it "can mutate the uuid" do response = RubySaml::SloLogoutresponse.new response_id = response.response_id assert_equal response_id, response.uuid response.uuid = "new_uuid" assert_equal "new_uuid", response.uuid - assert_equal response.response_id, response.uuid + assert_equal response.uuid, response.response_id end end From fbab99048f72aa8c1060b64c44d517dd678826f3 Mon Sep 17 00:00:00 2001 From: johnnyshields <27655+johnnyshields@users.noreply.github.com> Date: Tue, 14 Jan 2025 09:59:41 +0900 Subject: [PATCH 10/13] Fix rubocop --- .rubocop_todo.yml | 2 +- lib/ruby_saml/authrequest.rb | 2 +- lib/ruby_saml/logoutrequest.rb | 2 +- lib/ruby_saml/slo_logoutresponse.rb | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index a549e94b..6fbd8ee0 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -181,7 +181,7 @@ Lint/UselessAssignment: # Offense count: 42 # Configuration parameters: AllowedMethods, AllowedPatterns, CountRepeatedAttributes. Metrics/AbcSize: - Max: 100 + Max: 200 # Offense count: 1 # Configuration parameters: CountComments, CountAsOne, AllowedMethods, AllowedPatterns. diff --git a/lib/ruby_saml/authrequest.rb b/lib/ruby_saml/authrequest.rb index 730f91e1..43f2c712 100644 --- a/lib/ruby_saml/authrequest.rb +++ b/lib/ruby_saml/authrequest.rb @@ -183,7 +183,7 @@ def sign_document(document, settings) end def assign_uuid(settings) - @uuid ||= RubySaml::Utils.generate_uuid(settings.sp_uuid_prefix) + @uuid ||= RubySaml::Utils.generate_uuid(settings.sp_uuid_prefix) # rubocop:disable Naming/MemoizedInstanceVariableName end end end diff --git a/lib/ruby_saml/logoutrequest.rb b/lib/ruby_saml/logoutrequest.rb index ab6f859d..cb88f553 100644 --- a/lib/ruby_saml/logoutrequest.rb +++ b/lib/ruby_saml/logoutrequest.rb @@ -142,7 +142,7 @@ def sign_document(document, settings) end def assign_uuid(settings) - @uuid ||= RubySaml::Utils.generate_uuid(settings.sp_uuid_prefix) + @uuid ||= RubySaml::Utils.generate_uuid(settings.sp_uuid_prefix) # rubocop:disable Naming/MemoizedInstanceVariableName end end end diff --git a/lib/ruby_saml/slo_logoutresponse.rb b/lib/ruby_saml/slo_logoutresponse.rb index a8bb0527..fbd0ca0c 100644 --- a/lib/ruby_saml/slo_logoutresponse.rb +++ b/lib/ruby_saml/slo_logoutresponse.rb @@ -152,7 +152,7 @@ def sign_document(document, settings) end def assign_uuid(settings) - @uuid ||= RubySaml::Utils.generate_uuid(settings.sp_uuid_prefix) + @uuid ||= RubySaml::Utils.generate_uuid(settings.sp_uuid_prefix) # rubocop:disable Naming/MemoizedInstanceVariableName end end end From 3d0d76c1b1011f63353f9b87ae9c31a2820dc05e Mon Sep 17 00:00:00 2001 From: Johnny Shields <27655+johnnyshields@users.noreply.github.com> Date: Tue, 14 Jan 2025 10:03:55 +0900 Subject: [PATCH 11/13] Update utils.rb --- lib/ruby_saml/utils.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/ruby_saml/utils.rb b/lib/ruby_saml/utils.rb index 3b9a5bc7..e2505573 100644 --- a/lib/ruby_saml/utils.rb +++ b/lib/ruby_saml/utils.rb @@ -386,8 +386,9 @@ def retrieve_plaintext(cipher_text, symmetric_key, algorithm) end end + # @deprecated Use RubySaml::Settings#sp_uuid_prefix instead. def set_prefix(_value) - raise NoMethodError.new('RubySaml::Util.set_prefix has been removed. Please use RubySaml::Settings#uuid_prefix instead.') + raise NoMethodError.new('RubySaml::Util.set_prefix has been removed. Please use RubySaml::Settings#sp_uuid_prefix instead.') end # Generates a UUID with a prefix. From 0a324fd80a9a4e0108f8266946682f52151ad795 Mon Sep 17 00:00:00 2001 From: Johnny Shields <27655+johnnyshields@users.noreply.github.com> Date: Wed, 15 Jan 2025 16:25:39 +0900 Subject: [PATCH 12/13] Update README.md --- README.md | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index b1cbefae..26207d2a 100644 --- a/README.md +++ b/README.md @@ -963,12 +963,21 @@ end The `attribute_value` option additionally accepts an array of possible values. -## SP Request UUIDs +## SP-Originated Message IDs By default, Ruby SAML will generate UUIDs for SP requests prefixed with the `_` character, for example `"_ea8b5fdf-0a71-4bef-9f87-5406ee746f5b"`. To override this behavior, you may set `settings.sp_uuid_prefix` to a string of your choice, or `false` to use no prefix. +## SP-Originated Message IDs + +Ruby SAML automatically generates message IDs for SP-originated messages (AuthNRequest, etc.) +By default, this is a UUID prefixed by the `_` character, for example `"_ea8b5fdf-0a71-4bef-9f87-5406ee746f5b"`. +To override this behavior, you may set `settings.sp_uuid_prefix` to a string of your choice. +Note that the SAML specification requires that this type (`xsd:ID`) be an +[NCName](https://www.w3.org/TR/xmlschema-2/#NCName), meaning that it must start with a letter +or underscore, and can only contain letters, digits, underscores, hyphens, and periods. + ## Custom Metadata Fields Some IdPs may require to add SPs to add additional fields (Organization, ContactPerson, etc.) From e42990b22c69dea5cff2d65cbc6533236681c479 Mon Sep 17 00:00:00 2001 From: Sixto Martin Date: Wed, 15 Jan 2025 11:14:46 +0100 Subject: [PATCH 13/13] Update README.md, remove duplicated content --- README.md | 6 ------ 1 file changed, 6 deletions(-) diff --git a/README.md b/README.md index 26207d2a..5ed96767 100644 --- a/README.md +++ b/README.md @@ -965,12 +965,6 @@ The `attribute_value` option additionally accepts an array of possible values. ## SP-Originated Message IDs -By default, Ruby SAML will generate UUIDs for SP requests prefixed with the `_` character, -for example `"_ea8b5fdf-0a71-4bef-9f87-5406ee746f5b"`. To override this behavior, -you may set `settings.sp_uuid_prefix` to a string of your choice, or `false` to use no prefix. - -## SP-Originated Message IDs - Ruby SAML automatically generates message IDs for SP-originated messages (AuthNRequest, etc.) By default, this is a UUID prefixed by the `_` character, for example `"_ea8b5fdf-0a71-4bef-9f87-5406ee746f5b"`. To override this behavior, you may set `settings.sp_uuid_prefix` to a string of your choice.