[feat] Thread safe configurator (#26)

* Signable logic with given certificate information * Update unit test with new test certificate * Assertion builder with certificate attribute * Response builder with ceritificate * Use directly provided cert and pv key * Remove config dependency from low layer logics * Use correct attribute name * Remove config dependency from low level logics * Remove config dependency from low level logics and fix test * Revert Proc approach * Switchable assertion signature flag (saml-idp#228) Co-authored-by: zogoo <[email protected]> * MetadataBuilder uses custom configurator (#25) Co-authored-by: Andrea Lorenzetti <[email protected]> * Use concurrent ruby fixed version for test (saml-idp#230) * Squash commits for saml_idp gem * [feat] Allow SP config force signature validation (#16) * Allow SP config force signature validation * Allow SP config force signature validation Tested with Slack with Authn request signature option --------- Co-authored-by: zogoo <[email protected]> * [feat] Don’t ignore certificates without usage (#17) I have tested with live SAML SP apps and it works fine * Unspecified certifciate from SP metadata --------- Co-authored-by: zogoo <[email protected]> * Try with proper way to update helper method (#19) * Set minimum test coverage (saml-idp#207) * Set minimum test coverage to a very high value for testing * Update minimum coverage to actual current value * Try with proper way to update helper method * Correctly decode and mock with correct REXML class * Drop the min coverage --------- Co-authored-by: Mathieu Jobin <[email protected]> Co-authored-by: zogoo <[email protected]> * [feat] Collect request validation errors (#18) * wip add error collector * Fix type and rewrite request with proper validation test cases * Lead error render decision to gem user * Validate the certificate's existence before verifying the signature. --------- Co-authored-by: zogoo <[email protected]> * Support lowercase percent-encoded sequences for URL encoding (#20) Co-authored-by: zogoo <[email protected]> * [fix] Gem CI updates for latest versions (#22) * Remove duplications * Pre-conditions need to be defined in before section * Le's not test logger in here --------- Co-authored-by: zogoo <[email protected]> * [fix] Allow IdP set reference ID for SAML response (#21) * Pass ref id as Session Index * Official Rails 8 is not released yet to RubyGem until that let's stick official older version --------- Co-authored-by: zogoo <[email protected]> * Support rails 8 for dev env (#23) Co-authored-by: zogoo <[email protected]> * Assertion flag should able switchable by application (#24) Co-authored-by: zogoo <[email protected]> * concurrent-ruby v1.3.5 has removed the dependency on logger --------- Co-authored-by: zogoo <[email protected]> Co-authored-by: Mathieu Jobin <[email protected]> --------- Co-authored-by: zogoo <[email protected]> Co-authored-by: Massimo Zappino <[email protected]> Co-authored-by: Andrea Lorenzetti <[email protected]> Co-authored-by: Mathieu Jobin <[email protected]>
Zogoo · Jan 22, 2025 · bda1269 · bda1269
1 parent e163c4d
commit bda1269
Show file tree

Hide file tree

Showing 28 changed files with 541 additions and 313 deletions.
diff --git a/README.md b/README.md
@@ -78,11 +78,6 @@ KEY DATA
 -----END RSA PRIVATE KEY-----
 CERT
 
-  # x509_certificate, secret_key, and password may also be set from within a proc, for example:
-  # config.x509_certificate = -> { File.read("cert.pem") }
-  # config.secret_key = -> { SecretKeyFinder.key_for(id: 1) }
-  # config.password = -> { "password" }
-
   # config.password = "secret_key_password"
   # config.algorithm = :sha256                                    # Default: sha1 only for development.
   # config.organization_name = "Your Organization"

diff --git a/lib/saml_idp/algorithmable.rb b/lib/saml_idp/algorithmable.rb
@@ -1,10 +1,9 @@
 module SamlIdp
   module Algorithmable
     def algorithm
-      algorithm_check = raw_algorithm || SamlIdp.config.algorithm
-      return algorithm_check if algorithm_check.respond_to?(:digest)
+      return raw_algorithm if raw_algorithm.respond_to?(:digest)
       begin
-        OpenSSL::Digest.const_get(algorithm_check.to_s.upcase)
+        OpenSSL::Digest.const_get(raw_algorithm.to_s.upcase)
       rescue NameError
         OpenSSL::Digest::SHA1
       end

diff --git a/lib/saml_idp/assertion_builder.rb b/lib/saml_idp/assertion_builder.rb
@@ -18,23 +18,29 @@ class AssertionBuilder
     attr_accessor :session_expiry
     attr_accessor :name_id_formats_opts
     attr_accessor :asserted_attributes_opts
+    attr_accessor :public_cert
+    attr_accessor :private_key
+    attr_accessor :pv_key_password
 
     delegate :config, to: :SamlIdp
 
     def initialize(
-        reference_id,
-        issuer_uri,
-        principal,
-        audience_uri,
-        saml_request_id,
-        saml_acs_url,
-        raw_algorithm,
-        authn_context_classref,
-        expiry=60*60,
-        encryption_opts=nil,
-        session_expiry=nil,
-        name_id_formats_opts = nil,
-        asserted_attributes_opts = nil
+        reference_id:,
+        issuer_uri:,
+        principal:,
+        audience_uri:,
+        saml_request_id:,
+        saml_acs_url:,
+        raw_algorithm:,
+        authn_context_classref:,
+        public_cert:,
+        private_key:,
+        pv_key_password:,
+        expiry: 60*60,
+        encryption_opts: nil,
+        session_expiry: nil,
+        name_id_formats_opts: nil,
+        asserted_attributes_opts: nil
     )
       self.reference_id = reference_id
       self.issuer_uri = issuer_uri
@@ -49,6 +55,17 @@ def initialize(
       self.session_expiry = session_expiry.nil? ? config.session_expiry : session_expiry
       self.name_id_formats_opts = name_id_formats_opts
       self.asserted_attributes_opts = asserted_attributes_opts
+      self.public_cert = public_cert
+      self.private_key = private_key
+      self.pv_key_password = pv_key_password
+    end
+
+    def encrypt(opts = {})
+      raise "Must set encryption_opts to encrypt" unless encryption_opts
+      raw_xml = opts[:sign] ? signed : raw
+      require 'saml_idp/encryptor'
+      encryptor = Encryptor.new encryption_opts
+      encryptor.encrypt(raw_xml)
     end
 
     def fresh
@@ -105,15 +122,8 @@ def fresh
         end
     end
     alias_method :raw, :fresh
-    private :fresh
 
-    def encrypt(opts = {})
-      raise "Must set encryption_opts to encrypt" unless encryption_opts
-      raw_xml = opts[:sign] ? signed : raw
-      require 'saml_idp/encryptor'
-      encryptor = Encryptor.new encryption_opts
-      encryptor.encrypt(raw_xml)
-    end
+    private
 
     def asserted_attributes
       if asserted_attributes_opts.present? && !asserted_attributes_opts.empty?
@@ -124,7 +134,6 @@ def asserted_attributes
         config.attributes
       end
     end
-    private :asserted_attributes
 
     def get_values_for(friendly_name, getter)
       result = nil
@@ -141,12 +150,10 @@ def get_values_for(friendly_name, getter)
       end
       Array(result)
     end
-    private :get_values_for
 
     def name_id
       name_id_getter.call principal
     end
-    private :name_id
 
     def name_id_getter
       getter = name_id_format[:getter]
@@ -156,56 +163,45 @@ def name_id_getter
         ->(principal) { principal.public_send getter.to_s }
       end
     end
-    private :name_id_getter
 
     def name_id_format
       @name_id_format ||= NameIdFormatter.new(name_id_formats).chosen
     end
-    private :name_id_format
 
     def name_id_formats
       @name_id_formats ||= (name_id_formats_opts || config.name_id.formats)
     end
-    private :name_id_formats
 
     def reference_string
       "_#{reference_id}"
     end
-    private :reference_string
 
     def now
       @now ||= Time.now.utc
     end
-    private :now
 
     def now_iso
       iso { now }
     end
-    private :now_iso
 
     def not_before
       iso { now - 5 }
     end
-    private :not_before
 
     def not_on_or_after_condition
       iso { now + expiry }
     end
-    private :not_on_or_after_condition
 
     def not_on_or_after_subject
       iso { now + 3 * 60 }
     end
-    private :not_on_or_after_subject
 
     def session_not_on_or_after
       iso { now + session_expiry }
     end
-    private :session_not_on_or_after
 
     def iso
       yield.iso8601
     end
-    private :iso
   end
 end
diff --git a/lib/saml_idp/configurator.rb b/lib/saml_idp/configurator.rb
@@ -25,8 +25,8 @@ class Configurator
     attr_accessor :logger
 
     def initialize
-      self.x509_certificate = -> { Default::X509_CERTIFICATE }
-      self.secret_key = -> { Default::SECRET_KEY }
+      self.x509_certificate = Default::X509_CERTIFICATE
+      self.secret_key = Default::SECRET_KEY
       self.algorithm = :sha1
       self.reference_id_generator = ->() { SecureRandom.uuid }
       self.service_provider = OpenStruct.new

diff --git a/lib/saml_idp/controller.rb b/lib/saml_idp/controller.rb
@@ -57,6 +57,9 @@ def encode_authn_response(principal, opts = {})
       audience_uri = opts[:audience_uri] || saml_request.issuer || saml_acs_url[/^(.*?\/\/.*?\/)/, 1]
       opt_issuer_uri = opts[:issuer_uri] || issuer_uri
       my_authn_context_classref = opts[:authn_context_classref] || authn_context_classref
+      public_cert = opts[:public_cert] || SamlIdp.config.x509_certificate
+      private_key = opts[:private_key] || SamlIdp.config.secret_key
+      pv_key_password = opts[:pv_key_password] || SamlIdp.config.password
       acs_url = opts[:acs_url] || saml_acs_url
       expiry = opts[:expiry] || 60*60
       session_expiry = opts[:session_expiry]
@@ -70,33 +73,39 @@ def encode_authn_response(principal, opts = {})
       compress_opts = opts[:compress] || false
 
       SamlResponse.new(
-        reference_id,
-        response_id,
-        opt_issuer_uri,
-        principal,
-        audience_uri,
-        saml_request_id,
-        acs_url,
-        (opts[:algorithm] || algorithm || default_algorithm),
-        my_authn_context_classref,
-        expiry,
-        encryption_opts,
-        session_expiry,
-        name_id_formats_opts,
-        asserted_attributes_opts,
-        signed_message_opts,
-        signed_assertion_opts,
-        compress_opts
+        reference_id: reference_id,
+        response_id: response_id,
+        issuer_uri: opt_issuer_uri,
+        principal: principal,
+        audience_uri: audience_uri,
+        saml_request_id: saml_request_id,
+        saml_acs_url: acs_url,
+        algorithm: (opts[:algorithm] || algorithm || default_algorithm),
+        authn_context_classref: my_authn_context_classref,
+        public_cert: public_cert,
+        private_key: private_key,
+        pv_key_password: pv_key_password,
+        expiry: expiry,
+        encryption_opts: encryption_opts,
+        session_expiry: session_expiry,
+        name_id_formats_opts: name_id_formats_opts,
+        asserted_attributes_opts: asserted_attributes_opts,
+        signed_message_opts: signed_message_opts,
+        signed_assertion_opts: signed_assertion_opts,
+        compression_opts: compress_opts
       ).build
     end
 
     def encode_logout_response(_principal, opts = {})
       SamlIdp::LogoutResponseBuilder.new(
-        get_saml_response_id,
-        (opts[:issuer_uri] || issuer_uri),
-        saml_logout_url,
-        saml_request_id,
-        (opts[:algorithm] || algorithm || default_algorithm)
+        response_id: get_saml_response_id,
+        issuer_uri: (opts[:issuer_uri] || issuer_uri),
+        saml_slo_url: saml_logout_url,
+        saml_request_id: saml_request_id,
+        algorithm: (opts[:algorithm] || algorithm || default_algorithm),
+        public_cert: opts[:public_cert] || SamlIdp.config.x509_certificate,
+        private_key: opts[:private_key] || SamlIdp.config.secret_key,
+        pv_key_password: opts[:pv_key_password] || SamlIdp.config.password
       ).signed
     end
 

diff --git a/lib/saml_idp/logout_builder.rb b/lib/saml_idp/logout_builder.rb
@@ -7,12 +7,26 @@ class LogoutBuilder
     attr_accessor :issuer_uri
     attr_accessor :saml_slo_url
     attr_accessor :algorithm
+    attr_accessor :public_cert
+    attr_accessor :private_key
+    attr_accessor :pv_key_password
 
-    def initialize(response_id, issuer_uri, saml_slo_url, algorithm)
+    def initialize(
+        response_id:,
+        issuer_uri:,
+        saml_slo_url:,
+        algorithm:,
+        public_cert:,
+        private_key:,
+        pv_key_password:
+    )
       self.response_id = response_id
       self.issuer_uri = issuer_uri
       self.saml_slo_url = saml_slo_url
       self.algorithm = algorithm
+      self.public_cert = public_cert
+      self.private_key = private_key
+      self.pv_key_password = pv_key_password
     end
 
     # this is an abstract base class.

diff --git a/lib/saml_idp/logout_request_builder.rb b/lib/saml_idp/logout_request_builder.rb
@@ -3,8 +3,25 @@ module SamlIdp
   class LogoutRequestBuilder < LogoutBuilder
     attr_accessor :name_id
 
-    def initialize(response_id, issuer_uri, saml_slo_url, name_id, algorithm)
-      super(response_id, issuer_uri, saml_slo_url, algorithm)
+    def initialize(
+      response_id:,
+      issuer_uri:,
+      saml_slo_url:,
+      name_id:,
+      algorithm:,
+      public_cert:,
+      private_key:,
+      pv_key_password: nil
+    )
+      super(
+        response_id: response_id,
+        issuer_uri: issuer_uri,
+        saml_slo_url: saml_slo_url,
+        algorithm: algorithm,
+        public_cert: public_cert,
+        private_key: private_key,
+        pv_key_password: pv_key_password
+      )
       self.name_id = name_id
     end
 

diff --git a/lib/saml_idp/logout_response_builder.rb b/lib/saml_idp/logout_response_builder.rb
@@ -3,8 +3,25 @@ module SamlIdp
   class LogoutResponseBuilder < LogoutBuilder
     attr_accessor :saml_request_id
 
-    def initialize(response_id, issuer_uri, saml_slo_url, saml_request_id, algorithm)
-      super(response_id, issuer_uri, saml_slo_url, algorithm)
+    def initialize(
+      response_id:,
+      issuer_uri:,
+      saml_slo_url:,
+      saml_request_id:,
+      algorithm:,
+      public_cert:,
+      private_key:,
+      pv_key_password: nil
+    )
+      super(
+        response_id: response_id,
+        issuer_uri: issuer_uri,
+        saml_slo_url: saml_slo_url,
+        algorithm: algorithm,
+        public_cert: public_cert,
+        private_key: private_key,
+        pv_key_password: pv_key_password
+      )
       self.saml_request_id = saml_request_id
     end
 

diff --git a/lib/saml_idp/metadata_builder.rb b/lib/saml_idp/metadata_builder.rb
@@ -152,14 +152,24 @@ def raw_algorithm
     private :raw_algorithm
 
     def x509_certificate
-      certificate = SamlIdp.config.x509_certificate.is_a?(Proc) ? SamlIdp.config.x509_certificate.call : SamlIdp.config.x509_certificate
+      certificate = configurator.x509_certificate.is_a?(Proc) ? configurator.x509_certificate.call : configurator.x509_certificate
       certificate
       .to_s
       .gsub(/-----BEGIN CERTIFICATE-----/,"")
       .gsub(/-----END CERTIFICATE-----/,"")
       .gsub(/\n/, "")
     end
 
+    alias_method :public_cert, :x509_certificate
+
+    def private_key
+      configurator.secret_key
+    end
+
+    def pv_key_password
+      nil
+    end
+
     %w[
       support_email
       organization_name