From 3fd021ceacd9208becc14895ad04da53e034d0b7 Mon Sep 17 00:00:00 2001 From: Mathieu Jobin Date: Tue, 3 Sep 2024 22:36:24 +0900 Subject: [PATCH 01/21] Set minimum test coverage (#207) * Set minimum test coverage to a very high value for testing * Update minimum coverage to actual current value --- spec/spec_helper.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index ffc9f9a8..d90955b7 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,5 +1,6 @@ # encoding: utf-8 require 'simplecov' +SimpleCov.minimum_coverage 96.45 SimpleCov.start do add_filter "/spec/" end From 94b00bcda10fbb790bcc05b96ab7b6563967fdf6 Mon Sep 17 00:00:00 2001 From: zogoo Date: Tue, 17 Sep 2024 23:20:12 +0200 Subject: [PATCH 02/21] [fix] Flaky test (#220) * Decode AuthnRequest params to XML format before pass to mock * Drop test case check --------- Co-authored-by: zogoo --- saml_idp.gemspec | 1 + spec/lib/saml_idp/controller_spec.rb | 2 +- spec/spec_helper.rb | 1 - spec/support/saml_request_macros.rb | 15 +++++++++++++-- 4 files changed, 15 insertions(+), 4 deletions(-) diff --git a/saml_idp.gemspec b/saml_idp.gemspec index 4c46a70d..ca7c836b 100644 --- a/saml_idp.gemspec +++ b/saml_idp.gemspec @@ -55,6 +55,7 @@ Gem::Specification.new do |s| s.add_development_dependency('capybara', '>= 2.16') s.add_development_dependency('rails', '>= 5.2') s.add_development_dependency('rake') + s.add_development_dependency('debug') s.add_development_dependency('rspec', '>= 3.7.0') s.add_development_dependency('ruby-saml', '>= 1.7.2') s.add_development_dependency('simplecov') diff --git a/spec/lib/saml_idp/controller_spec.rb b/spec/lib/saml_idp/controller_spec.rb index 883e0dba..1401b14c 100644 --- a/spec/lib/saml_idp/controller_spec.rb +++ b/spec/lib/saml_idp/controller_spec.rb @@ -33,7 +33,7 @@ def params end it 'should call xml signature validation method' do - signed_doc = SamlIdp::XMLSecurity::SignedDocument.new(params[:SAMLRequest]) + signed_doc = SamlIdp::XMLSecurity::SignedDocument.new(decode_saml_request(params[:SAMLRequest])) allow(signed_doc).to receive(:validate).and_return(true) allow(SamlIdp::XMLSecurity::SignedDocument).to receive(:new).and_return(signed_doc) validate_saml_request diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index d90955b7..ffc9f9a8 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,6 +1,5 @@ # encoding: utf-8 require 'simplecov' -SimpleCov.minimum_coverage 96.45 SimpleCov.start do add_filter "/spec/" end diff --git a/spec/support/saml_request_macros.rb b/spec/support/saml_request_macros.rb index d587cf68..d5629c83 100644 --- a/spec/support/saml_request_macros.rb +++ b/spec/support/saml_request_macros.rb @@ -3,8 +3,8 @@ module SamlRequestMacros def make_saml_request(requested_saml_acs_url = "https://foo.example.com/saml/consume", enable_secure_options = false) auth_request = OneLogin::RubySaml::Authrequest.new - auth_url = auth_request.create(saml_settings(requested_saml_acs_url, enable_secure_options)) - CGI.unescape(auth_url.split("=").last) + auth_url = auth_request.create_params(saml_settings(requested_saml_acs_url, enable_secure_options)) + auth_url['SAMLRequest'] end def make_saml_logout_request(requested_saml_logout_url = 'https://foo.example.com/saml/logout') @@ -90,6 +90,17 @@ def idp_configure(saml_acs_url = "https://foo.example.com/saml/consume", enable_ end end + def decode_saml_request(saml_request) + decoded_request = Base64.decode64(saml_request) + begin + # Try to decompress, since SAMLRequest might be compressed + Zlib::Inflate.new(-Zlib::MAX_WBITS).inflate(decoded_request) + rescue Zlib::DataError + # If it's not compressed, just return the decoded request + decoded_request + end + end + def print_pretty_xml(xml_string) doc = REXML::Document.new xml_string outbuf = "" From b4880085a74845f8254af6eaffe500121898bb2c Mon Sep 17 00:00:00 2001 From: Atish Maske <16266389+atish23@users.noreply.github.com> Date: Wed, 18 Sep 2024 03:00:14 +0530 Subject: [PATCH 03/21] Update saml_idp.gemspec (#215) Co-authored-by: Jon Phenow --- saml_idp.gemspec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/saml_idp.gemspec b/saml_idp.gemspec index ca7c836b..1404fbb7 100644 --- a/saml_idp.gemspec +++ b/saml_idp.gemspec @@ -10,7 +10,7 @@ Gem::Specification.new do |s| s.authors = ['Jon Phenow'] s.email = 'jon.phenow@sportngin.com' s.homepage = 'https://github.com/saml-idp/saml_idp' - s.summary = 'SAML Indentity Provider for Ruby' + s.summary = 'SAML Identity Provider for Ruby' s.description = 'SAML IdP (Identity Provider) Library for Ruby' s.date = Time.now.utc.strftime('%Y-%m-%d') s.files = Dir['lib/**/*', 'LICENSE', 'README.md', 'Gemfile', 'saml_idp.gemspec'] From 2003c94423e6fd4dc3e754332a15dfb5c6d80f67 Mon Sep 17 00:00:00 2001 From: Taketo Takashima Date: Thu, 19 Sep 2024 00:49:55 +0900 Subject: [PATCH 04/21] Add dependency of ostruct gem, to fix warning of ostruct was loaded (#217) Co-authored-by: Jon Phenow --- saml_idp.gemspec | 1 + 1 file changed, 1 insertion(+) diff --git a/saml_idp.gemspec b/saml_idp.gemspec index 1404fbb7..2e0d4f98 100644 --- a/saml_idp.gemspec +++ b/saml_idp.gemspec @@ -46,6 +46,7 @@ Gem::Specification.new do |s| s.add_dependency('activesupport', '>= 5.2') s.add_dependency('builder', '>= 3.0') s.add_dependency('nokogiri', '>= 1.6.2') + s.add_dependency('ostruct') s.add_dependency('rexml') s.add_dependency('xmlenc', '>= 0.7.1') From 62ca53762ed14cf67cb188dd8a7d25f37741464c Mon Sep 17 00:00:00 2001 From: Aleksandr Obukhov <125898833+aleksandr-obukhov@users.noreply.github.com> Date: Mon, 23 Sep 2024 20:03:50 +0200 Subject: [PATCH 05/21] Check if Rails is fully initialized (#216) --- lib/saml_idp.rb | 2 +- lib/saml_idp/configurator.rb | 2 +- spec/lib/saml_idp/configurator_spec.rb | 29 ++++++++++++++++++++++++++ 3 files changed, 31 insertions(+), 2 deletions(-) diff --git a/lib/saml_idp.rb b/lib/saml_idp.rb index 1e8532f2..d654fe89 100644 --- a/lib/saml_idp.rb +++ b/lib/saml_idp.rb @@ -9,7 +9,7 @@ module SamlIdp require 'saml_idp/metadata_builder' require 'saml_idp/version' require 'saml_idp/fingerprint' - require 'saml_idp/engine' if defined?(::Rails) + require 'saml_idp/engine' if defined?(::Rails::Engine) def self.config @config ||= SamlIdp::Configurator.new diff --git a/lib/saml_idp/configurator.rb b/lib/saml_idp/configurator.rb index 4998869a..e645f912 100644 --- a/lib/saml_idp/configurator.rb +++ b/lib/saml_idp/configurator.rb @@ -35,7 +35,7 @@ def initialize self.service_provider.persisted_metadata_getter = ->(id, service_provider) { } self.session_expiry = 0 self.attributes = {} - self.logger = defined?(::Rails) ? Rails.logger : ->(msg) { puts msg } + self.logger = (defined?(::Rails) && Rails.respond_to?(:logger)) ? Rails.logger : ->(msg) { puts msg } end # formats diff --git a/spec/lib/saml_idp/configurator_spec.rb b/spec/lib/saml_idp/configurator_spec.rb index 5148a289..33141117 100644 --- a/spec/lib/saml_idp/configurator_spec.rb +++ b/spec/lib/saml_idp/configurator_spec.rb @@ -47,5 +47,34 @@ module SamlIdp it 'has a valid session_expiry' do expect(subject.session_expiry).to eq(0) end + + context "logger initialization" do + context 'when Rails has been properly initialized' do + it 'sets logger to Rails.logger' do + rails_logger = double("Rails.logger") + stub_const("Rails", double(logger: rails_logger)) + + expect(subject.logger).to eq(Rails.logger) + end + end + + context 'when Rails is not fully initialized' do + it 'sets logger to a lambda' do + stub_const("Rails", Class.new) + + expect(subject.logger).to be_a(Proc) + expect { subject.logger.call("test") }.to output("test\n").to_stdout + end + end + + context 'when Rails is not defined' do + it 'sets logger to a lambda' do + hide_const("Rails") + + expect(subject.logger).to be_a(Proc) + expect { subject.logger.call("test") }.to output("test\n").to_stdout + end + end + end end end From 101925b2693373dc98f41ac4874c3e3ea6542ff4 Mon Sep 17 00:00:00 2001 From: zogoo Date: Thu, 24 Oct 2024 23:05:36 +0200 Subject: [PATCH 06/21] Add new versions for CI (#223) * Add new versions for CI * Add ruby 3.1 for dev env --------- Co-authored-by: zogoo --- .github/workflows/ci.yml | 26 +++++++++++++++++++------- .ruby-version | 2 +- gemfiles/rails_7.2.gemfile | 8 ++++++++ saml_idp.gemspec | 3 +-- 4 files changed, 29 insertions(+), 10 deletions(-) create mode 100644 gemfiles/rails_7.2.gemfile diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4c882730..e80550ae 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -11,26 +11,38 @@ jobs: fail-fast: false matrix: ruby: ['2.5', '2.6', '2.7', '3.0', '3.1', '3.2', '3.3'] - gemfile: [rails_5.2.gemfile, rails_6.1.gemfile, rails_7.0.gemfile, rails_7.1.gemfile, rails_dev.gemfile] + gemfile: [rails_5.2.gemfile, rails_6.1.gemfile, rails_7.0.gemfile, rails_7.1.gemfile, rails_7.2.gemfile, rails_dev.gemfile] exclude: + # Ruby 3.2 is min version for Rails 8 + - ruby: '2.5' + gemfile: rails_dev.gemfile + - ruby: '2.6' + gemfile: rails_dev.gemfile + - ruby: '2.7' + gemfile: rails_dev.gemfile + - ruby: '3.0' + gemfile: rails_dev.gemfile + - ruby: '3.1' + gemfile: rails_dev.gemfile - ruby: '2.5' gemfile: rails_7.0.gemfile - ruby: '2.5' gemfile: rails_7.1.gemfile - - ruby: '2.5' - gemfile: rails_dev.gemfile - ruby: '2.6' gemfile: rails_7.0.gemfile - ruby: '2.6' gemfile: rails_7.1.gemfile + # Ruby 3.1 is min version for Rails 7.2 + - ruby: '2.5' + gemfile: rails_7.2.gemfile - ruby: '2.6' - gemfile: rails_dev.gemfile + gemfile: rails_7.2.gemfile - ruby: '2.7' - gemfile: rails_dev.gemfile + gemfile: rails_7.2.gemfile - ruby: '3.0' - gemfile: rails_5.2.gemfile + gemfile: rails_7.2.gemfile - ruby: '3.0' - gemfile: rails_dev.gemfile + gemfile: rails_5.2.gemfile - ruby: '3.1' gemfile: rails_5.2.gemfile - ruby: '3.2' diff --git a/.ruby-version b/.ruby-version index ef538c28..fa7adc7a 100644 --- a/.ruby-version +++ b/.ruby-version @@ -1 +1 @@ -3.1.2 +3.3.5 diff --git a/gemfiles/rails_7.2.gemfile b/gemfiles/rails_7.2.gemfile new file mode 100644 index 00000000..6f6033ca --- /dev/null +++ b/gemfiles/rails_7.2.gemfile @@ -0,0 +1,8 @@ +# This file was generated by Appraisal + +source "https://rubygems.org" + +gem "rails", "~> 7.2.0" +gem 'activeresource', '~> 6.1', '>= 6.1.3' + +gemspec path: "../" diff --git a/saml_idp.gemspec b/saml_idp.gemspec index 2e0d4f98..49044ff4 100644 --- a/saml_idp.gemspec +++ b/saml_idp.gemspec @@ -50,9 +50,8 @@ Gem::Specification.new do |s| s.add_dependency('rexml') s.add_dependency('xmlenc', '>= 0.7.1') - s.add_development_dependency('activeresource', '>= 5.1') + s.add_development_dependency('activeresource', '~> 6.1') s.add_development_dependency('appraisal') - s.add_development_dependency('byebug') s.add_development_dependency('capybara', '>= 2.16') s.add_development_dependency('rails', '>= 5.2') s.add_development_dependency('rake') From 12416c438515aa250590cfa2f083ef55cb25f514 Mon Sep 17 00:00:00 2001 From: zogoo Date: Thu, 24 Oct 2024 23:25:29 +0200 Subject: [PATCH 07/21] [feat] External Signature param for Redirect Binding (#203) * Squash commits for saml_idp gem * Add explanation for external attributes of decode_request method --------- Co-authored-by: zogoo --- README.md | 10 ++- lib/saml_idp.rb | 4 +- lib/saml_idp/controller.rb | 12 ++- lib/saml_idp/request.rb | 51 +++++++++++-- lib/saml_idp/xml_security.rb | 33 +++++---- spec/lib/saml_idp/controller_spec.rb | 15 ++++ spec/lib/saml_idp/incoming_metadata_spec.rb | 1 - spec/lib/saml_idp/metadata_builder_spec.rb | 2 +- spec/lib/saml_idp/request_spec.rb | 74 +++++++++++++------ .../app/views/saml_idp/idp/new.html.erb | 3 + spec/support/saml_request_macros.rb | 15 +++- spec/xml_security_spec.rb | 18 +++-- 12 files changed, 177 insertions(+), 61 deletions(-) diff --git a/README.md b/README.md index 7d70b75c..d8c48dd7 100644 --- a/README.md +++ b/README.md @@ -24,9 +24,13 @@ Add this to your Gemfile: Include `SamlIdp::Controller` and see the examples that use rails. It should be straightforward for you. -Basically you call `decode_request(params[:SAMLRequest])` on an incoming request and then use the value -`saml_acs_url` to determine the source for which you need to authenticate a user. How you authenticate -a user is entirely up to you. +Basically, you call `decode_request(params[:SAMLRequest])` on an incoming request and then use the value +`saml_acs_url` to determine the source for which you need to authenticate a user. +If the signature (`Signature`) and signing algorithm (`SigAlg`) are provided as external parameters in the request, +you can pass those parameters as `decode_request(params[:SAMLRequest], params[:Signature], params[:SigAlg], params[:RelayState])`. +Then, you can verify the request signature with the `valid?` method. + +How you authenticate a user is entirely up to you. Once a user has successfully authenticated on your system send the Service Provider a SAMLResponse by posting to `saml_acs_url` the parameter `SAMLResponse` with the return value from a call to diff --git a/lib/saml_idp.rb b/lib/saml_idp.rb index d654fe89..543111a3 100644 --- a/lib/saml_idp.rb +++ b/lib/saml_idp.rb @@ -70,9 +70,9 @@ def signed? !!xpath("//ds:Signature", ds: signature_namespace).first end - def valid_signature?(fingerprint) + def valid_signature?(certificate, fingerprint) signed? && - signed_document.validate(fingerprint, :soft) + signed_document.validate(certificate, fingerprint, :soft) end def signed_document diff --git a/lib/saml_idp/controller.rb b/lib/saml_idp/controller.rb index e2bf25e7..d5bd20c9 100644 --- a/lib/saml_idp/controller.rb +++ b/lib/saml_idp/controller.rb @@ -33,15 +33,21 @@ def acs_url end def validate_saml_request(raw_saml_request = params[:SAMLRequest]) - decode_request(raw_saml_request) + decode_request(raw_saml_request, params[:Signature], params[:SigAlg], params[:RelayState]) return true if valid_saml_request? head :forbidden if defined?(::Rails) false end - def decode_request(raw_saml_request) - @saml_request = Request.from_deflated_request(raw_saml_request) + def decode_request(raw_saml_request, signature, sig_algorithm, relay_state) + @saml_request = Request.from_deflated_request( + raw_saml_request, + saml_request: raw_saml_request, + signature: signature, + sig_algorithm: sig_algorithm, + relay_state: relay_state + ) end def authn_context_classref diff --git a/lib/saml_idp/request.rb b/lib/saml_idp/request.rb index 4b8b891f..eb651145 100644 --- a/lib/saml_idp/request.rb +++ b/lib/saml_idp/request.rb @@ -3,7 +3,7 @@ require 'logger' module SamlIdp class Request - def self.from_deflated_request(raw) + def self.from_deflated_request(raw, external_attributes = {}) if raw decoded = Base64.decode64(raw) zstream = Zlib::Inflate.new(-Zlib::MAX_WBITS) @@ -18,18 +18,22 @@ def self.from_deflated_request(raw) else inflated = "" end - new(inflated) + new(inflated, external_attributes) end - attr_accessor :raw_xml + attr_accessor :raw_xml, :saml_request, :signature, :sig_algorithm, :relay_state delegate :config, to: :SamlIdp private :config delegate :xpath, to: :document private :xpath - def initialize(raw_xml = "") + def initialize(raw_xml = "", external_attributes = {}) self.raw_xml = raw_xml + self.saml_request = external_attributes[:saml_request] + self.relay_state = external_attributes[:relay_state] + self.sig_algorithm = external_attributes[:sig_algorithm] + self.signature = external_attributes[:signature] end def logout_request? @@ -85,7 +89,7 @@ def log(msg) end end - def valid? + def valid?(external_attributes = {}) unless service_provider? log "Unable to find service provider for issuer #{issuer}" return false @@ -96,8 +100,15 @@ def valid? return false end - unless valid_signature? - log "Signature is invalid in #{raw_xml}" + # XML embedded signature + if signature.nil? && !valid_signature? + log "Requested document signature is invalid in #{raw_xml}" + return false + end + + # URI query signature + if signature.present? && !valid_external_signature? + log "Requested URI signature is invalid in #{raw_xml}" return false end @@ -120,12 +131,29 @@ def valid_signature? # Validate signature when metadata specify AuthnRequest should be signed metadata = service_provider.current_metadata if logout_request? || authn_request? && metadata.respond_to?(:sign_authn_request?) && metadata.sign_authn_request? - document.valid_signature?(service_provider.fingerprint) + document.valid_signature?(service_provider.cert, service_provider.fingerprint) else true end end + def valid_external_signature? + cert = OpenSSL::X509::Certificate.new(service_provider.cert) + + sha_version = sig_algorithm =~ /sha(.*?)$/i && $1.to_i + raw_signature = Base64.decode64(signature) + + signature_algorithm = case sha_version + when 256 then OpenSSL::Digest::SHA256 + when 384 then OpenSSL::Digest::SHA384 + when 512 then OpenSSL::Digest::SHA512 + else + OpenSSL::Digest::SHA1 + end + + cert.public_key.verify(signature_algorithm.new, raw_signature, query_request_string) + end + def service_provider? service_provider && service_provider.valid? end @@ -148,6 +176,13 @@ def session_index @_session_index ||= xpath("//samlp:SessionIndex", samlp: samlp).first.try(:content) end + def query_request_string + url_string = "SAMLRequest=#{CGI.escape(saml_request)}" + url_string << "&RelayState=#{CGI.escape(relay_state)}" if relay_state + url_string << "&SigAlg=#{CGI.escape(sig_algorithm)}" + end + private :query_request_string + def response_host uri = URI(response_url) if uri diff --git a/lib/saml_idp/xml_security.rb b/lib/saml_idp/xml_security.rb index 640c0348..0a223ad2 100644 --- a/lib/saml_idp/xml_security.rb +++ b/lib/saml_idp/xml_security.rb @@ -43,24 +43,29 @@ def initialize(response) extract_signed_element_id end - def validate(idp_cert_fingerprint, soft = true) + def validate(idp_base64_cert, idp_cert_fingerprint, soft = true) # get cert from response cert_element = REXML::XPath.first(self, "//ds:X509Certificate", { "ds"=>DSIG }) - raise ValidationError.new("Certificate element missing in response (ds:X509Certificate)") unless cert_element - base64_cert = cert_element.text - cert_text = Base64.decode64(base64_cert) - cert = OpenSSL::X509::Certificate.new(cert_text) - - # check cert matches registered idp cert - fingerprint = fingerprint_cert(cert) - sha1_fingerprint = fingerprint_cert_sha1(cert) - plain_idp_cert_fingerprint = idp_cert_fingerprint.gsub(/[^a-zA-Z0-9]/,"").downcase - - if fingerprint != plain_idp_cert_fingerprint && sha1_fingerprint != plain_idp_cert_fingerprint - return soft ? false : (raise ValidationError.new("Fingerprint mismatch")) + if cert_element + idp_base64_cert = cert_element.text + cert_text = Base64.decode64(idp_base64_cert) + cert = OpenSSL::X509::Certificate.new(cert_text) + + # check cert matches registered idp cert + fingerprint = fingerprint_cert(cert) + sha1_fingerprint = fingerprint_cert_sha1(cert) + plain_idp_cert_fingerprint = idp_cert_fingerprint.gsub(/[^a-zA-Z0-9]/,"").downcase + + if fingerprint != plain_idp_cert_fingerprint && sha1_fingerprint != plain_idp_cert_fingerprint + return soft ? false : (raise ValidationError.new("Fingerprint mismatch")) + end + end + + if idp_base64_cert.nil? || idp_base64_cert.empty? + raise ValidationError.new("Certificate validation is required, but it doesn't exist.") end - validate_doc(base64_cert, soft) + validate_doc(idp_base64_cert, soft) end def fingerprint_cert(cert) diff --git a/spec/lib/saml_idp/controller_spec.rb b/spec/lib/saml_idp/controller_spec.rb index 1401b14c..735cdaae 100644 --- a/spec/lib/saml_idp/controller_spec.rb +++ b/spec/lib/saml_idp/controller_spec.rb @@ -124,4 +124,19 @@ def params end end end + + context "Single Logout Request" do + before do + idp_configure("https://foo.example.com/saml/consume", true) + slo_request = make_saml_sp_slo_request + params[:SAMLRequest] = slo_request['SAMLRequest'] + params[:RelayState] = slo_request['RelayState'] + params[:SigAlg] = slo_request['SigAlg'] + params[:Signature] = slo_request['Signature'] + end + + it 'should successfully validate signature' do + expect(validate_saml_request).to eq(true) + end + end end diff --git a/spec/lib/saml_idp/incoming_metadata_spec.rb b/spec/lib/saml_idp/incoming_metadata_spec.rb index 7d483e0b..a966e17d 100644 --- a/spec/lib/saml_idp/incoming_metadata_spec.rb +++ b/spec/lib/saml_idp/incoming_metadata_spec.rb @@ -33,7 +33,6 @@ module SamlIdp it 'should properly set sign_assertions to false' do metadata = SamlIdp::IncomingMetadata.new(metadata_1) expect(metadata.sign_assertions).to eq(false) - expect(metadata.sign_authn_request).to eq(false) end it 'should properly set entity_id as https://test-saml.com/saml' do diff --git a/spec/lib/saml_idp/metadata_builder_spec.rb b/spec/lib/saml_idp/metadata_builder_spec.rb index c8e14765..453a8d81 100644 --- a/spec/lib/saml_idp/metadata_builder_spec.rb +++ b/spec/lib/saml_idp/metadata_builder_spec.rb @@ -6,7 +6,7 @@ module SamlIdp end it "signs valid xml" do - expect(Saml::XML::Document.parse(subject.signed).valid_signature?(Default::FINGERPRINT)).to be_truthy + expect(Saml::XML::Document.parse(subject.signed).valid_signature?("", Default::FINGERPRINT)).to be_truthy end it "includes logout element" do diff --git a/spec/lib/saml_idp/request_spec.rb b/spec/lib/saml_idp/request_spec.rb index 47711539..794fc361 100644 --- a/spec/lib/saml_idp/request_spec.rb +++ b/spec/lib/saml_idp/request_spec.rb @@ -122,36 +122,68 @@ def info(msg); end end describe "logout request" do - let(:raw_logout_request) { "http://example.comsome_name_idabc123index" } + context 'when POST binding' do + let(:raw_logout_request) { "http://example.comsome_name_idabc123index" } - subject { described_class.new raw_logout_request } + subject { described_class.new raw_logout_request } - it "has a valid request_id" do - expect(subject.request_id).to eq('_some_response_id') - end + it "has a valid request_id" do + expect(subject.request_id).to eq('_some_response_id') + end - it "should be flagged as a logout_request" do - expect(subject.logout_request?).to eq(true) - end + it "should be flagged as a logout_request" do + expect(subject.logout_request?).to eq(true) + end - it "should have a valid name_id" do - expect(subject.name_id).to eq('some_name_id') - end + it "should have a valid name_id" do + expect(subject.name_id).to eq('some_name_id') + end - it "should have a session index" do - expect(subject.session_index).to eq('abc123index') - end + it "should have a session index" do + expect(subject.session_index).to eq('abc123index') + end - it "should have a valid issuer" do - expect(subject.issuer).to eq('http://example.com') - end + it "should have a valid issuer" do + expect(subject.issuer).to eq('http://example.com') + end - it "fetches internal request" do - expect(subject.request['ID']).to eq(subject.request_id) + it "fetches internal request" do + expect(subject.request['ID']).to eq(subject.request_id) + end + + it "should return logout_url for response_url" do + expect(subject.response_url).to eq(subject.logout_url) + end end - it "should return logout_url for response_url" do - expect(subject.response_url).to eq(subject.logout_url) + context 'when signature provided as external param' do + let!(:uri_query) { make_saml_sp_slo_request } + let(:raw_saml_request) { uri_query['SAMLRequest'] } + let(:relay_state) { uri_query['RelayState'] } + let(:siging_algorithm) { uri_query['SigAlg'] } + let(:signature) { uri_query['Signature'] } + + subject do + described_class.from_deflated_request( + raw_saml_request, + saml_request: raw_saml_request, + relay_state: relay_state, + sig_algorithm: siging_algorithm, + signature: signature + ) + end + + it "should validate the request" do + allow(ServiceProvider).to receive(:new).and_return( + ServiceProvider.new( + issuer: "http://example.com/issuer", + cert: sp_x509_cert, + response_hosts: ["example.com"], + assertion_consumer_logout_service_url: "http://example.com/logout" + ) + ) + expect(subject.valid?).to be true + end end end end diff --git a/spec/rails_app/app/views/saml_idp/idp/new.html.erb b/spec/rails_app/app/views/saml_idp/idp/new.html.erb index c71d85ab..01c86199 100644 --- a/spec/rails_app/app/views/saml_idp/idp/new.html.erb +++ b/spec/rails_app/app/views/saml_idp/idp/new.html.erb @@ -4,6 +4,9 @@ <%= form_tag do %> <%= hidden_field_tag("SAMLRequest", params[:SAMLRequest]) %> <%= hidden_field_tag("RelayState", params[:RelayState]) %> + <%= hidden_field_tag("SigAlg", params[:SigAlg]) %> + <%= hidden_field_tag("Signature", params[:Signature]) %> +

<%= label_tag :email %> <%= email_field_tag :email, params[:email], :autocapitalize => "off", :autocorrect => "off", :autofocus => "autofocus", :spellcheck => "false", :size => 30, :class => "email_pwd txt" %> diff --git a/spec/support/saml_request_macros.rb b/spec/support/saml_request_macros.rb index d5629c83..4f3049ad 100644 --- a/spec/support/saml_request_macros.rb +++ b/spec/support/saml_request_macros.rb @@ -18,6 +18,17 @@ def make_saml_logout_request(requested_saml_logout_url = 'https://foo.example.co Base64.strict_encode64(request_builder.signed) end + def make_saml_sp_slo_request(param_type: true, embed_sign: false) + logout_request = OneLogin::RubySaml::Logoutrequest.new + saml_sp_setting = saml_settings("https://foo.example.com/saml/consume") + add_securty_options(saml_sp_setting, embed_sign: embed_sign) + if param_type + logout_request.create_params(saml_sp_setting, 'RelayState' => 'https://foo.example.com/home') + else + logout_request.create(saml_sp_setting, 'RelayState' => 'https://foo.example.com/home') + end + end + def generate_sp_metadata(saml_acs_url = "https://foo.example.com/saml/consume", enable_secure_options = false) sp_metadata = OneLogin::RubySaml::Metadata.new sp_metadata.generate(saml_settings(saml_acs_url, enable_secure_options), true) @@ -28,6 +39,7 @@ def saml_settings(saml_acs_url = "https://foo.example.com/saml/consume", enable_ settings.assertion_consumer_service_url = saml_acs_url settings.issuer = "http://example.com/issuer" settings.idp_sso_target_url = "http://idp.com/saml/idp" + settings.idp_slo_target_url = "http://idp.com/saml/slo" settings.assertion_consumer_logout_service_url = 'https://foo.example.com/saml/logout' settings.idp_cert_fingerprint = SamlIdp::Default::FINGERPRINT settings.name_identifier_format = SamlIdp::Default::NAME_ID_FORMAT @@ -84,7 +96,8 @@ def idp_configure(saml_acs_url = "https://foo.example.com/saml/consume", enable_ response_hosts: [URI(saml_acs_url).host], acs_url: saml_acs_url, cert: sp_x509_cert, - fingerprint: SamlIdp::Fingerprint.certificate_digest(sp_x509_cert) + fingerprint: SamlIdp::Fingerprint.certificate_digest(sp_x509_cert), + assertion_consumer_logout_service_url: 'https://foo.example.com/saml/logout' } } end diff --git a/spec/xml_security_spec.rb b/spec/xml_security_spec.rb index 7b7bf3c4..276dfcef 100644 --- a/spec/xml_security_spec.rb +++ b/spec/xml_security_spec.rb @@ -19,7 +19,7 @@ module SamlIdp end it "it raise Fingerprint mismatch" do - expect { document.validate("no:fi:ng:er:pr:in:t", false) }.to( + expect { document.validate("", "no:fi:ng:er:pr:in:t", false) }.to( raise_error(SamlIdp::XMLSecurity::SignedDocument::ValidationError, "Fingerprint mismatch") ) end @@ -45,10 +45,10 @@ module SamlIdp response = Base64.decode64(response_document) response.sub!(/.*<\/ds:X509Certificate>/, "") document = XMLSecurity::SignedDocument.new(response) - expect { document.validate("a fingerprint", false) }.to( + expect { document.validate("", "a fingerprint", false) }.to( raise_error( SamlIdp::XMLSecurity::SignedDocument::ValidationError, - "Certificate element missing in response (ds:X509Certificate)" + "Certificate validation is required, but it doesn't exist." ) ) end @@ -57,22 +57,26 @@ module SamlIdp describe "Algorithms" do it "validate using SHA1" do document = XMLSecurity::SignedDocument.new(fixture(:adfs_response_sha1, false)) - expect(document.validate("F1:3C:6B:80:90:5A:03:0E:6C:91:3E:5D:15:FA:DD:B0:16:45:48:72")).to be_truthy + base64cert = document.elements["//ds:X509Certificate"].text + expect(document.validate(base64cert, "F1:3C:6B:80:90:5A:03:0E:6C:91:3E:5D:15:FA:DD:B0:16:45:48:72")).to be_truthy end it "validate using SHA256" do document = XMLSecurity::SignedDocument.new(fixture(:adfs_response_sha256, false)) - expect(document.validate("28:74:9B:E8:1F:E8:10:9C:A8:7C:A9:C3:E3:C5:01:6C:92:1C:B4:BA")).to be_truthy + base64cert = document.elements["//ds:X509Certificate"].text + expect(document.validate(base64cert, "28:74:9B:E8:1F:E8:10:9C:A8:7C:A9:C3:E3:C5:01:6C:92:1C:B4:BA")).to be_truthy end it "validate using SHA384" do document = XMLSecurity::SignedDocument.new(fixture(:adfs_response_sha384, false)) - expect(document.validate("F1:3C:6B:80:90:5A:03:0E:6C:91:3E:5D:15:FA:DD:B0:16:45:48:72")).to be_truthy + base64cert = document.elements["//ds:X509Certificate"].text + expect(document.validate(base64cert, "F1:3C:6B:80:90:5A:03:0E:6C:91:3E:5D:15:FA:DD:B0:16:45:48:72")).to be_truthy end it "validate using SHA512" do document = XMLSecurity::SignedDocument.new(fixture(:adfs_response_sha512, false)) - expect(document.validate("F1:3C:6B:80:90:5A:03:0E:6C:91:3E:5D:15:FA:DD:B0:16:45:48:72")).to be_truthy + base64cert = document.elements["//ds:X509Certificate"].text + expect(document.validate(base64cert, "F1:3C:6B:80:90:5A:03:0E:6C:91:3E:5D:15:FA:DD:B0:16:45:48:72")).to be_truthy end end From 4b7e4c88e183ad4502c5ae433a7e6570e60d218f Mon Sep 17 00:00:00 2001 From: zogoo Date: Tue, 29 Oct 2024 14:58:01 +0100 Subject: [PATCH 08/21] [improvements] Return error to application and support SLO request validation (#224) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 * [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 * wip add error collector * Fix type and rewrite request with proper validation test cases * Try with proper way to update helper method (#19) * Set minimum test coverage (#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 Co-authored-by: zogoo * Lead error render decision to gem user * Validate the certificate's existence before verifying the signature. * [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 * Support lowercase percent-encoded sequences for URL encoding (#20) Co-authored-by: zogoo * Remove duplications * Pre-conditions need to be defined in before section * Le's not test logger in here * Let's not break anything for now * Rename correctly --------- Co-authored-by: zogoo Co-authored-by: Mathieu Jobin --- lib/saml_idp/incoming_metadata.rb | 9 + lib/saml_idp/request.rb | 43 ++- lib/saml_idp/service_provider.rb | 1 + saml_idp.gemspec | 2 +- spec/lib/saml_idp/configurator_spec.rb | 17 +- spec/lib/saml_idp/controller_spec.rb | 25 +- spec/lib/saml_idp/incoming_metadata_spec.rb | 75 +++++ spec/lib/saml_idp/request_spec.rb | 304 ++++++++++---------- spec/support/saml_request_macros.rb | 56 ++-- spec/support/security_helpers.rb | 4 +- 10 files changed, 344 insertions(+), 192 deletions(-) diff --git a/lib/saml_idp/incoming_metadata.rb b/lib/saml_idp/incoming_metadata.rb index fe0f5c61..250a5fea 100644 --- a/lib/saml_idp/incoming_metadata.rb +++ b/lib/saml_idp/incoming_metadata.rb @@ -63,6 +63,15 @@ def contact_person end hashable :contact_person + def unspecified_certificate + xpath( + "//md:SPSSODescriptor/md:KeyDescriptor[not(@use)]/ds:KeyInfo/ds:X509Data/ds:X509Certificate", + ds: signature_namespace, + md: metadata_namespace + ).first.try(:content).to_s + end + hashable :unspecified_certificate + def signing_certificate xpath( "//md:SPSSODescriptor/md:KeyDescriptor[@use='signing']/ds:KeyInfo/ds:X509Data/ds:X509Certificate", diff --git a/lib/saml_idp/request.rb b/lib/saml_idp/request.rb index eb651145..bd84c5d8 100644 --- a/lib/saml_idp/request.rb +++ b/lib/saml_idp/request.rb @@ -3,6 +3,8 @@ require 'logger' module SamlIdp class Request + attr_accessor :errors + def self.from_deflated_request(raw, external_attributes = {}) if raw decoded = Base64.decode64(raw) @@ -34,6 +36,7 @@ def initialize(raw_xml = "", external_attributes = {}) self.relay_state = external_attributes[:relay_state] self.sig_algorithm = external_attributes[:sig_algorithm] self.signature = external_attributes[:signature] + self.errors = [] end def logout_request? @@ -89,37 +92,53 @@ def log(msg) end end + def collect_errors(error_type) + errors.push(error_type) + end + def valid?(external_attributes = {}) unless service_provider? log "Unable to find service provider for issuer #{issuer}" + collect_errors(:sp_not_found) return false end unless (authn_request? ^ logout_request?) log "One and only one of authnrequest and logout request is required. authnrequest: #{authn_request?} logout_request: #{logout_request?} " + collect_errors(:unaccepted_request) + return false + end + + if (logout_request? || validate_auth_request_signature?) && (service_provider.cert.to_s.empty? || !!service_provider.fingerprint.to_s.empty?) + log "Verifying request signature is required. But certificate and fingerprint was empty." + collect_errors(:empty_certificate) return false end # XML embedded signature if signature.nil? && !valid_signature? log "Requested document signature is invalid in #{raw_xml}" + collect_errors(:invalid_embedded_signature) return false end # URI query signature if signature.present? && !valid_external_signature? log "Requested URI signature is invalid in #{raw_xml}" + collect_errors(:invalid_external_signature) return false end if response_url.nil? log "Unable to find response url for #{issuer}: #{raw_xml}" + collect_errors(:empty_response_url) return false end if !service_provider.acceptable_response_hosts.include?(response_host) log "#{service_provider.acceptable_response_hosts} compare to #{response_host}" log "No acceptable AssertionConsumerServiceURL, either configure them via config.service_provider.response_hosts or match to your metadata_url host" + collect_errors(:not_allowed_host) return false end @@ -128,9 +147,7 @@ def valid?(external_attributes = {}) def valid_signature? # Force signatures for logout requests because there is no other protection against a cross-site DoS. - # Validate signature when metadata specify AuthnRequest should be signed - metadata = service_provider.current_metadata - if logout_request? || authn_request? && metadata.respond_to?(:sign_authn_request?) && metadata.sign_authn_request? + if logout_request? || authn_request? && validate_auth_request_signature? document.valid_signature?(service_provider.cert, service_provider.fingerprint) else true @@ -138,6 +155,8 @@ def valid_signature? end def valid_external_signature? + return true if authn_request? && !validate_auth_request_signature? + cert = OpenSSL::X509::Certificate.new(service_provider.cert) sha_version = sig_algorithm =~ /sha(.*?)$/i && $1.to_i @@ -151,7 +170,14 @@ def valid_external_signature? OpenSSL::Digest::SHA1 end - cert.public_key.verify(signature_algorithm.new, raw_signature, query_request_string) + result = cert.public_key.verify(signature_algorithm.new, raw_signature, query_request_string) + # Match all percent-encoded sequences (e.g., %20, %2B) and convert them to lowercase + # Upper case is recommended for consistency but some services such as MS Entra Id not follows it + # https://datatracker.ietf.org/doc/html/rfc3986#section-2.1 + result || cert.public_key.verify(signature_algorithm.new, raw_signature, query_request_string.gsub(/%[A-F0-9]{2}/) { |match| match.downcase }) + rescue OpenSSL::X509::CertificateError => e + log e.message + collect_errors(:cert_format_error) end def service_provider? @@ -232,5 +258,14 @@ def service_provider_finder config.service_provider.finder end private :service_provider_finder + + def validate_auth_request_signature? + # Validate signature when metadata specify AuthnRequest should be signed + metadata = service_provider.current_metadata + sign_authn_request = metadata.respond_to?(:sign_authn_request?) && metadata.sign_authn_request? + sign_authn_request = service_provider.sign_authn_request unless service_provider.sign_authn_request.nil? + sign_authn_request + end + private :validate_auth_request_signature? end end diff --git a/lib/saml_idp/service_provider.rb b/lib/saml_idp/service_provider.rb index eed24bfc..bedc1c33 100644 --- a/lib/saml_idp/service_provider.rb +++ b/lib/saml_idp/service_provider.rb @@ -11,6 +11,7 @@ class ServiceProvider attribute :fingerprint attribute :metadata_url attribute :validate_signature + attribute :sign_authn_request attribute :acs_url attribute :assertion_consumer_logout_service_url attribute :response_hosts diff --git a/saml_idp.gemspec b/saml_idp.gemspec index 49044ff4..1a332250 100644 --- a/saml_idp.gemspec +++ b/saml_idp.gemspec @@ -54,8 +54,8 @@ Gem::Specification.new do |s| s.add_development_dependency('appraisal') s.add_development_dependency('capybara', '>= 2.16') s.add_development_dependency('rails', '>= 5.2') - s.add_development_dependency('rake') s.add_development_dependency('debug') + s.add_development_dependency('rake') s.add_development_dependency('rspec', '>= 3.7.0') s.add_development_dependency('ruby-saml', '>= 1.7.2') s.add_development_dependency('simplecov') diff --git a/spec/lib/saml_idp/configurator_spec.rb b/spec/lib/saml_idp/configurator_spec.rb index 33141117..59c399a8 100644 --- a/spec/lib/saml_idp/configurator_spec.rb +++ b/spec/lib/saml_idp/configurator_spec.rb @@ -50,18 +50,21 @@ module SamlIdp context "logger initialization" do context 'when Rails has been properly initialized' do - it 'sets logger to Rails.logger' do - rails_logger = double("Rails.logger") - stub_const("Rails", double(logger: rails_logger)) + before do + stub_const("Rails", double(logger: double("Rails.logger"))) + end + it 'sets logger to Rails.logger' do expect(subject.logger).to eq(Rails.logger) end end context 'when Rails is not fully initialized' do - it 'sets logger to a lambda' do - stub_const("Rails", Class.new) + before do + stub_const("Rails", Class.new) + end + it 'sets logger to a lambda' do expect(subject.logger).to be_a(Proc) expect { subject.logger.call("test") }.to output("test\n").to_stdout end @@ -75,6 +78,10 @@ module SamlIdp expect { subject.logger.call("test") }.to output("test\n").to_stdout end end + + after do + hide_const("Rails") + end end end end diff --git a/spec/lib/saml_idp/controller_spec.rb b/spec/lib/saml_idp/controller_spec.rb index 735cdaae..94e189e9 100644 --- a/spec/lib/saml_idp/controller_spec.rb +++ b/spec/lib/saml_idp/controller_spec.rb @@ -81,16 +81,6 @@ def params expect(response.is_valid?).to be_truthy end - it "should create a SAML Logout Response" do - params[:SAMLRequest] = make_saml_logout_request - expect(validate_saml_request).to eq(true) - expect(saml_request.logout_request?).to eq true - saml_response = encode_response(principal) - response = OneLogin::RubySaml::Logoutresponse.new(saml_response, saml_settings) - expect(response.validate).to eq(true) - expect(response.issuer).to eq("http://example.com") - end - it "should by default create a SAML Response with a signed assertion" do saml_response = encode_response(principal) response = OneLogin::RubySaml::Response.new(saml_response) @@ -128,7 +118,7 @@ def params context "Single Logout Request" do before do idp_configure("https://foo.example.com/saml/consume", true) - slo_request = make_saml_sp_slo_request + slo_request = make_saml_sp_slo_request(security_options: { embed_sign: false }) params[:SAMLRequest] = slo_request['SAMLRequest'] params[:RelayState] = slo_request['RelayState'] params[:SigAlg] = slo_request['SigAlg'] @@ -138,5 +128,18 @@ def params it 'should successfully validate signature' do expect(validate_saml_request).to eq(true) end + + context "solicited Response" do + let(:principal) { double email_address: "foo@example.com" } + + it "should create a SAML Logout Response" do + expect(validate_saml_request).to eq(true) + expect(saml_request.logout_request?).to eq true + saml_response = encode_response(principal) + response = OneLogin::RubySaml::Logoutresponse.new(saml_response, saml_settings) + expect(response.validate).to eq(true) + expect(response.issuer).to eq("http://idp.com/saml/idp") + end + end end end diff --git a/spec/lib/saml_idp/incoming_metadata_spec.rb b/spec/lib/saml_idp/incoming_metadata_spec.rb index a966e17d..fb00ec53 100644 --- a/spec/lib/saml_idp/incoming_metadata_spec.rb +++ b/spec/lib/saml_idp/incoming_metadata_spec.rb @@ -1,4 +1,5 @@ require 'spec_helper' + module SamlIdp metadata_1 = <<-eos @@ -29,6 +30,48 @@ module SamlIdp eos + metadata_5 = <<-eos + + + + + + MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAnht3GR... + + + + + + eos + + metadata_6 = <<-eos + + + + + + MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAmw6vGr... + + + + + + eos + + metadata_7 = <<-eos + + + + + + MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA1dX3Gr... + + + + + + eos + describe IncomingMetadata do it 'should properly set sign_assertions to false' do metadata = SamlIdp::IncomingMetadata.new(metadata_1) @@ -55,5 +98,37 @@ module SamlIdp metadata = SamlIdp::IncomingMetadata.new(metadata_4) expect(metadata.sign_authn_request).to eq(false) end + + it 'should properly set unspecified_certificate when present' do + metadata = SamlIdp::IncomingMetadata.new(metadata_5) + expect(metadata.unspecified_certificate).to eq('MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAnht3GR...') + end + + it 'should return empty unspecified_certificate when not present' do + metadata = SamlIdp::IncomingMetadata.new(metadata_1) + expect(metadata.unspecified_certificate).to eq('') + end + + it 'should properly set signing_certificate when present but not unspecified_certificate' do + metadata = SamlIdp::IncomingMetadata.new(metadata_6) + expect(metadata.signing_certificate).to eq('MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAmw6vGr...') + expect(metadata.unspecified_certificate).to eq('') + end + + it 'should return empty signing_certificate when not present' do + metadata = SamlIdp::IncomingMetadata.new(metadata_1) + expect(metadata.signing_certificate).to eq('') + end + + it 'should properly set encryption_certificate when present but not unspecified_certificate' do + metadata = SamlIdp::IncomingMetadata.new(metadata_7) + expect(metadata.encryption_certificate).to eq('MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA1dX3Gr...') + expect(metadata.unspecified_certificate).to eq('') + end + + it 'should return empty encryption_certificate when not present' do + metadata = SamlIdp::IncomingMetadata.new(metadata_1) + expect(metadata.encryption_certificate).to eq('') + end end end diff --git a/spec/lib/saml_idp/request_spec.rb b/spec/lib/saml_idp/request_spec.rb index 794fc361..151e391a 100644 --- a/spec/lib/saml_idp/request_spec.rb +++ b/spec/lib/saml_idp/request_spec.rb @@ -1,189 +1,195 @@ require 'spec_helper' -module SamlIdp - describe Request do - let(:issuer) { 'localhost:3000' } - let(:raw_authn_request) do - "#{issuer}urn:oasis:names:tc:SAML:2.0:ac:classes:Password" - end - describe "deflated request" do - let(:deflated_request) { Base64.encode64(Zlib::Deflate.deflate(raw_authn_request, 9)[2..-5]) } +RSpec.describe SamlIdp::Request, type: :model do + let(:valid_saml_request) { make_saml_request("https://foo.example.com/saml/consume", true) } + let(:valid_logout_request) { make_saml_sp_slo_request(security_options: { embed_sign: true })['SAMLRequest'] } + let(:invalid_saml_request) { "invalid_saml_request" } + let(:external_attributes) { { saml_request: valid_saml_request, relay_state: "state" } } - subject { described_class.from_deflated_request deflated_request } + describe ".from_deflated_request" do + context "when request is valid and deflated" do + it "inflates and decodes the request" do + request = SamlIdp::Request.from_deflated_request(valid_saml_request) - it "inflates" do - expect(subject.request_id).to eq("_af43d1a0-e111-0130-661a-3c0754403fdb") + expect { Saml::XML::Document.parse(request.raw_xml) }.not_to raise_error end + end - it "handles invalid SAML" do - req = described_class.from_deflated_request "bang!" - expect(req.valid?).to eq(false) + context "when request is invalid" do + it "returns an empty inflated string" do + request = SamlIdp::Request.from_deflated_request(nil) + expect(request.raw_xml).to eq("") end end + end - describe "authn request" do - subject { described_class.new raw_authn_request } + describe "#logout_request?" do + it "returns true for a valid logout request" do + request = SamlIdp::Request.from_deflated_request(valid_logout_request) + expect(request.logout_request?).to be true + end - it "has a valid request_id" do - expect(subject.request_id).to eq("_af43d1a0-e111-0130-661a-3c0754403fdb") - end + it "returns false for a non-logout request" do + request = SamlIdp::Request.from_deflated_request(valid_saml_request) + expect(request.logout_request?).to be false + end + end - it "has a valid acs_url" do - expect(subject.acs_url).to eq("http://localhost:3000/saml/consume") - end + describe "#authn_request?" do + it "returns true for a valid authn request" do + request = SamlIdp::Request.from_deflated_request(valid_saml_request) + expect(request.authn_request?).to be true + end - it "has a valid service_provider" do - expect(subject.service_provider).to be_a ServiceProvider - end + it "returns false for a non-authn request" do + request = SamlIdp::Request.from_deflated_request(valid_logout_request) + expect(request.authn_request?).to be false + end + end - it "has a valid service_provider" do - expect(subject.service_provider).to be_truthy - end + describe "#valid?" do + let(:sp_issuer) { "test_issuer" } + let(:valid_service_provider) do + instance_double( + "SamlIdp::ServiceProvider", + valid?: true, + acs_url: 'https://foo.example.com/saml/consume', + current_metadata: instance_double("Metadata", sign_authn_request?: true), + assertion_consumer_logout_service_url: 'https://foo.example.com/saml/logout', + sign_authn_request: true, + acceptable_response_hosts: ["foo.example.com"], + cert: sp_x509_cert, + fingerprint: SamlIdp::Fingerprint.certificate_digest(sp_x509_cert, :sha256), + ) + end + + before do + allow_any_instance_of(SamlIdp::Request).to receive(:service_provider).and_return(valid_service_provider) + allow_any_instance_of(SamlIdp::Request).to receive(:issuer).and_return(sp_issuer) + end - it "has a valid issuer" do - expect(subject.issuer).to eq("localhost:3000") + context "when the request is valid" do + it "returns true for a valid authn request" do + request = SamlIdp::Request.from_deflated_request(valid_saml_request) + expect(request.errors).to be_empty + expect(request.valid?).to be true end - it "has a valid valid_signature" do - expect(subject.valid_signature?).to be_truthy + it "returns true for a valid logout request" do + request = SamlIdp::Request.from_deflated_request(valid_logout_request) + expect(request.errors).to be_empty + expect(request.valid?).to be true end + end - it "should return acs_url for response_url" do - expect(subject.response_url).to eq(subject.acs_url) + context 'when signature provided as external param' do + let!(:uri_query) { make_saml_sp_slo_request(security_options: { embed_sign: false }) } + let(:raw_saml_request) { uri_query['SAMLRequest'] } + let(:relay_state) { uri_query['RelayState'] } + let(:siging_algorithm) { uri_query['SigAlg'] } + let(:signature) { uri_query['Signature'] } + + subject do + described_class.from_deflated_request( + raw_saml_request, + saml_request: raw_saml_request, + relay_state: relay_state, + sig_algorithm: siging_algorithm, + signature: signature + ) end - it "is a authn request" do - expect(subject.authn_request?).to eq(true) + it "should validate the request" do + expect(subject.valid_external_signature?).to be true + expect(subject.errors).to be_empty end - it "fetches internal request" do - expect(subject.request['ID']).to eq(subject.request_id) + it "should collect errors when the signature is invalid" do + allow(subject).to receive(:valid_external_signature?).and_return(false) + expect(subject.valid?).to eq(false) + expect(subject.errors).to include(:invalid_external_signature) end + end - it 'has a valid authn context' do - expect(subject.requested_authn_context).to eq('urn:oasis:names:tc:SAML:2.0:ac:classes:Password') - end + context "when the service provider is invalid" do + it "returns false and logs an error" do + allow_any_instance_of(SamlIdp::Request).to receive(:service_provider?).and_return(false) + request = SamlIdp::Request.from_deflated_request(valid_saml_request) - context 'the issuer is empty' do - let(:issuer) { nil } - let(:logger) { ->(msg) { puts msg } } - - before do - allow(SamlIdp.config).to receive(:logger).and_return(logger) - end - - it 'is invalid' do - expect(subject.issuer).to_not eq('') - expect(subject.issuer).to be_nil - expect(subject.valid?).to eq(false) - end - - context 'a Ruby Logger is configured' do - let(:logger) { Logger.new($stdout) } - - before do - allow(logger).to receive(:info) - end - - it 'logs an error message' do - expect(subject.valid?).to be false - expect(logger).to have_received(:info).with('Unable to find service provider for issuer ') - end - end - - context 'a Logger-like logger is configured' do - let(:logger) do - Class.new { - def info(msg); end - }.new - end - - before do - allow(logger).to receive(:info) - end - - it 'logs an error message' do - expect(subject.valid?).to be false - expect(logger).to have_received(:info).with('Unable to find service provider for issuer ') - end - end - - context 'a logger lambda is configured' do - let(:logger) { double } - - before { allow(logger).to receive(:call) } - - it 'logs an error message' do - expect(subject.valid?).to be false - expect(logger).to have_received(:call).with('Unable to find service provider for issuer ') - end - end + expect(request.valid?).to be false + expect(request.errors).to include(:sp_not_found) end end - describe "logout request" do - context 'when POST binding' do - let(:raw_logout_request) { "http://example.comsome_name_idabc123index" } - - subject { described_class.new raw_logout_request } + context "when empty certificate for authn request validation" do + let(:valid_service_provider) do + instance_double( + "SamlIdp::ServiceProvider", + valid?: true, + acs_url: 'https://foo.example.com/saml/consume', + current_metadata: instance_double("Metadata", sign_authn_request?: true), + assertion_consumer_logout_service_url: 'https://foo.example.com/saml/logout', + sign_authn_request: true, + acceptable_response_hosts: ["foo.example.com"], + cert: nil, + fingerprint: nil, + ) + end + it "returns false and logs an error" do + request = SamlIdp::Request.from_deflated_request(valid_saml_request) - it "has a valid request_id" do - expect(subject.request_id).to eq('_some_response_id') - end + expect(request.valid?).to be false + expect(request.errors).to include(:empty_certificate) + end + end - it "should be flagged as a logout_request" do - expect(subject.logout_request?).to eq(true) - end + context "when empty certificate for logout validation" do + let(:valid_service_provider) do + instance_double( + "SamlIdp::ServiceProvider", + valid?: true, + acs_url: 'https://foo.example.com/saml/consume', + current_metadata: instance_double("Metadata", sign_authn_request?: true), + assertion_consumer_logout_service_url: 'https://foo.example.com/saml/logout', + sign_authn_request: true, + acceptable_response_hosts: ["foo.example.com"], + cert: nil, + fingerprint: nil, + ) + end - it "should have a valid name_id" do - expect(subject.name_id).to eq('some_name_id') - end + before do + allow_any_instance_of(SamlIdp::Request).to receive(:authn_request?).and_return(false) + allow_any_instance_of(SamlIdp::Request).to receive(:logout_request?).and_return(true) + end - it "should have a session index" do - expect(subject.session_index).to eq('abc123index') - end + it "returns false and logs an error" do + request = SamlIdp::Request.from_deflated_request(valid_saml_request) - it "should have a valid issuer" do - expect(subject.issuer).to eq('http://example.com') - end + expect(request.valid?).to be false + expect(request.errors).to include(:empty_certificate) + end + end - it "fetches internal request" do - expect(subject.request['ID']).to eq(subject.request_id) - end + context "when both authn and logout requests are present" do + it "returns false and logs an error" do + allow_any_instance_of(SamlIdp::Request).to receive(:authn_request?).and_return(true) + allow_any_instance_of(SamlIdp::Request).to receive(:logout_request?).and_return(true) + request = SamlIdp::Request.from_deflated_request(valid_saml_request) - it "should return logout_url for response_url" do - expect(subject.response_url).to eq(subject.logout_url) - end + expect(request.valid?).to be false + expect(request.errors).to include(:unaccepted_request) end + end + + context "when the signature is invalid" do + it "returns false and logs an error" do + allow_any_instance_of(SamlIdp::Request).to receive(:valid_signature?).and_return(false) + allow_any_instance_of(SamlIdp::Request).to receive(:log) + request = SamlIdp::Request.from_deflated_request(valid_saml_request) - context 'when signature provided as external param' do - let!(:uri_query) { make_saml_sp_slo_request } - let(:raw_saml_request) { uri_query['SAMLRequest'] } - let(:relay_state) { uri_query['RelayState'] } - let(:siging_algorithm) { uri_query['SigAlg'] } - let(:signature) { uri_query['Signature'] } - - subject do - described_class.from_deflated_request( - raw_saml_request, - saml_request: raw_saml_request, - relay_state: relay_state, - sig_algorithm: siging_algorithm, - signature: signature - ) - end - - it "should validate the request" do - allow(ServiceProvider).to receive(:new).and_return( - ServiceProvider.new( - issuer: "http://example.com/issuer", - cert: sp_x509_cert, - response_hosts: ["example.com"], - assertion_consumer_logout_service_url: "http://example.com/logout" - ) - ) - expect(subject.valid?).to be true - end + expect(request.valid?).to be false + expect(request.errors).to include(:invalid_embedded_signature) end end end diff --git a/spec/support/saml_request_macros.rb b/spec/support/saml_request_macros.rb index 4f3049ad..d4abf3d6 100644 --- a/spec/support/saml_request_macros.rb +++ b/spec/support/saml_request_macros.rb @@ -18,10 +18,9 @@ def make_saml_logout_request(requested_saml_logout_url = 'https://foo.example.co Base64.strict_encode64(request_builder.signed) end - def make_saml_sp_slo_request(param_type: true, embed_sign: false) + def make_saml_sp_slo_request(param_type: true, security_options: {}) logout_request = OneLogin::RubySaml::Logoutrequest.new - saml_sp_setting = saml_settings("https://foo.example.com/saml/consume") - add_securty_options(saml_sp_setting, embed_sign: embed_sign) + saml_sp_setting = saml_settings("https://foo.example.com/saml/consume", true, security_options: security_options) if param_type logout_request.create_params(saml_sp_setting, 'RelayState' => 'https://foo.example.com/home') else @@ -34,7 +33,7 @@ def generate_sp_metadata(saml_acs_url = "https://foo.example.com/saml/consume", sp_metadata.generate(saml_settings(saml_acs_url, enable_secure_options), true) end - def saml_settings(saml_acs_url = "https://foo.example.com/saml/consume", enable_secure_options = false) + def saml_settings(saml_acs_url = "https://foo.example.com/saml/consume", enable_secure_options = false, security_options: {}) settings = OneLogin::RubySaml::Settings.new settings.assertion_consumer_service_url = saml_acs_url settings.issuer = "http://example.com/issuer" @@ -43,28 +42,22 @@ def saml_settings(saml_acs_url = "https://foo.example.com/saml/consume", enable_ settings.assertion_consumer_logout_service_url = 'https://foo.example.com/saml/logout' settings.idp_cert_fingerprint = SamlIdp::Default::FINGERPRINT settings.name_identifier_format = SamlIdp::Default::NAME_ID_FORMAT - add_securty_options(settings) if enable_secure_options + add_securty_options(settings, default_sp_security_options.merge!(security_options)) if enable_secure_options settings end - def add_securty_options(settings, authn_requests_signed: true, - embed_sign: true, - logout_requests_signed: true, - logout_responses_signed: true, - digest_method: XMLSecurity::Document::SHA256, - signature_method: XMLSecurity::Document::RSA_SHA256, - assertions_signed: true) + def add_securty_options(settings, options = default_sp_security_options) # Security section settings.idp_cert = SamlIdp::Default::X509_CERTIFICATE # Signed embedded singature - settings.security[:authn_requests_signed] = authn_requests_signed - settings.security[:embed_sign] = embed_sign - settings.security[:logout_requests_signed] = logout_requests_signed - settings.security[:logout_responses_signed] = logout_responses_signed - settings.security[:metadata_signed] = digest_method - settings.security[:digest_method] = digest_method - settings.security[:signature_method] = signature_method - settings.security[:want_assertions_signed] = assertions_signed + settings.security[:authn_requests_signed] = options[:authn_requests_signed] + settings.security[:embed_sign] = options[:embed_sign] + settings.security[:logout_requests_signed] = options[:logout_requests_signed] + settings.security[:logout_responses_signed] = options[:logout_responses_signed] + settings.security[:metadata_signed] = options[:digest_method] + settings.security[:digest_method] = options[:digest_method] + settings.security[:signature_method] = options[:signature_method] + settings.security[:want_assertions_signed] = options[:assertions_signed] settings.private_key = sp_pv_key settings.certificate = sp_x509_cert end @@ -120,4 +113,27 @@ def print_pretty_xml(xml_string) doc.write(outbuf, 1) puts outbuf end + + def decode_saml_request(saml_request) + decoded_request = Base64.decode64(saml_request) + begin + # Try to decompress, since SAMLRequest might be compressed + Zlib::Inflate.new(-Zlib::MAX_WBITS).inflate(decoded_request) + rescue Zlib::DataError + # If it's not compressed, just return the decoded request + decoded_request + end + end + + def default_sp_security_options + { + authn_requests_signed: true, + embed_sign: true, + logout_requests_signed: true, + logout_responses_signed: true, + digest_method: XMLSecurity::Document::SHA256, + signature_method: XMLSecurity::Document::RSA_SHA256, + assertions_signed: true + } + end end diff --git a/spec/support/security_helpers.rb b/spec/support/security_helpers.rb index 2deb38ae..f6952ce7 100644 --- a/spec/support/security_helpers.rb +++ b/spec/support/security_helpers.rb @@ -51,8 +51,8 @@ def signature_fingerprint_1 @signature_fingerprint1 ||= "C5:19:85:D9:47:F1:BE:57:08:20:25:05:08:46:EB:27:F6:CA:B7:83" end - def signature_1 - @signature1 ||= File.read(File.join(File.dirname(__FILE__), 'certificates', 'certificate1')) + def certificate_1 + @certificate_1 ||= File.read(File.join(File.dirname(__FILE__), 'certificates', 'certificate1')) end def r1_signature_2 From 6f832aff456bfd14718c65905d8b658f0e42bc8f Mon Sep 17 00:00:00 2001 From: Chris Born Date: Fri, 1 Nov 2024 09:23:06 -0700 Subject: [PATCH 09/21] Alternative to support multiple x509 Certificates via procs (#211) * Adds support for multiple multiple x509 certificates, secret keys, and passwords by providing procs in the idp configuration. * Call the proc in the tests * Add documentation in the form of a comment in the README.md * fix extra space * remove additional change * Fix metadata x509 certificate --- README.md | 5 +++++ lib/saml_idp/configurator.rb | 4 ++-- lib/saml_idp/metadata_builder.rb | 3 ++- lib/saml_idp/signature_builder.rb | 3 ++- lib/saml_idp/signed_info_builder.rb | 4 ++-- spec/lib/saml_idp/configurator_spec.rb | 4 ++-- 6 files changed, 15 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index d8c48dd7..50f99ebe 100644 --- a/README.md +++ b/README.md @@ -78,6 +78,11 @@ 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/configurator.rb b/lib/saml_idp/configurator.rb index e645f912..99a662a0 100644 --- 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/metadata_builder.rb b/lib/saml_idp/metadata_builder.rb index 9a4afb92..92258e38 100644 --- a/lib/saml_idp/metadata_builder.rb +++ b/lib/saml_idp/metadata_builder.rb @@ -152,7 +152,8 @@ def raw_algorithm private :raw_algorithm def x509_certificate - SamlIdp.config.x509_certificate + certificate = SamlIdp.config.x509_certificate.is_a?(Proc) ? SamlIdp.config.x509_certificate.call : SamlIdp.config.x509_certificate + certificate .to_s .gsub(/-----BEGIN CERTIFICATE-----/,"") .gsub(/-----END CERTIFICATE-----/,"") diff --git a/lib/saml_idp/signature_builder.rb b/lib/saml_idp/signature_builder.rb index 83183f23..0406715c 100644 --- a/lib/saml_idp/signature_builder.rb +++ b/lib/saml_idp/signature_builder.rb @@ -21,7 +21,8 @@ def raw end def x509_certificate - SamlIdp.config.x509_certificate + certificate = SamlIdp.config.x509_certificate.is_a?(Proc) ? SamlIdp.config.x509_certificate.call : SamlIdp.config.x509_certificate + certificate .to_s .gsub(/-----BEGIN CERTIFICATE-----/,"") .gsub(/-----END CERTIFICATE-----/,"") diff --git a/lib/saml_idp/signed_info_builder.rb b/lib/saml_idp/signed_info_builder.rb index 81380666..4b0ff224 100644 --- a/lib/saml_idp/signed_info_builder.rb +++ b/lib/saml_idp/signed_info_builder.rb @@ -65,12 +65,12 @@ def clean_algorithm_name private :clean_algorithm_name def secret_key - SamlIdp.config.secret_key + SamlIdp.config.secret_key.is_a?(Proc) ? SamlIdp.config.secret_key.call : SamlIdp.config.secret_key end private :secret_key def password - SamlIdp.config.password + SamlIdp.config.password.is_a?(Proc) ? SamlIdp.config.password.call : SamlIdp.config.password end private :password diff --git a/spec/lib/saml_idp/configurator_spec.rb b/spec/lib/saml_idp/configurator_spec.rb index 59c399a8..80da3b14 100644 --- a/spec/lib/saml_idp/configurator_spec.rb +++ b/spec/lib/saml_idp/configurator_spec.rb @@ -20,11 +20,11 @@ module SamlIdp it { should respond_to :logger } it "has a valid x509_certificate" do - expect(subject.x509_certificate).to eq(Default::X509_CERTIFICATE) + expect(subject.x509_certificate.call).to eq(Default::X509_CERTIFICATE) end it "has a valid secret_key" do - expect(subject.secret_key).to eq(Default::SECRET_KEY) + expect(subject.secret_key.call).to eq(Default::SECRET_KEY) end it "has a valid algorithm" do From fb44202f1074cb8defc66a670dcc2f0228aa9087 Mon Sep 17 00:00:00 2001 From: zogoo Date: Tue, 5 Nov 2024 00:09:19 +0100 Subject: [PATCH 10/21] [fix] Accept reference id for SAML response (#226) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 * [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 * Try with proper way to update helper method (#19) * Set minimum test coverage (#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 Co-authored-by: zogoo * [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 * Support lowercase percent-encoded sequences for URL encoding (#20) Co-authored-by: zogoo * Pass ref id as Session Index * Official Rails 8 is not released yet to RubyGem until that let's stick official older version * [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 * [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 * Fixes for ORIGIN gem --------- Co-authored-by: zogoo Co-authored-by: Mathieu Jobin --- lib/saml_idp/saml_response.rb | 4 ++-- spec/lib/saml_idp/saml_response_spec.rb | 19 +++++++++++++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/lib/saml_idp/saml_response.rb b/lib/saml_idp/saml_response.rb index fdc327a5..85cf48b1 100644 --- a/lib/saml_idp/saml_response.rb +++ b/lib/saml_idp/saml_response.rb @@ -98,7 +98,7 @@ def response_builder def assertion_builder @assertion_builder ||= - AssertionBuilder.new SecureRandom.uuid, + AssertionBuilder.new(reference_id || SecureRandom.uuid, issuer_uri, principal, audience_uri, @@ -110,7 +110,7 @@ def assertion_builder encryption_opts, session_expiry, name_id_formats_opts, - asserted_attributes_opts + asserted_attributes_opts) end private :assertion_builder end diff --git a/spec/lib/saml_idp/saml_response_spec.rb b/spec/lib/saml_idp/saml_response_spec.rb index a9e82151..b79f8a2b 100644 --- a/spec/lib/saml_idp/saml_response_spec.rb +++ b/spec/lib/saml_idp/saml_response_spec.rb @@ -192,6 +192,25 @@ module SamlIdp expect(saml_resp.is_valid?).to eq(true) end + it "will pass reference_id as SessionIndex" do + expect { subject.build }.not_to raise_error + signed_encoded_xml = subject.build + resp_settings = saml_settings(saml_acs_url) + resp_settings.private_key = Default::SECRET_KEY + resp_settings.issuer = audience_uri + saml_resp = OneLogin::RubySaml::Response.new(signed_encoded_xml, settings: resp_settings) + + expect( + Nokogiri::XML(saml_resp.response).at_xpath( + "//saml:AuthnStatement/@SessionIndex", + { + "samlp" => "urn:oasis:names:tc:SAML:2.0:protocol", + "saml" => "urn:oasis:names:tc:SAML:2.0:assertion" + } + ).value + ).to eq("_#{reference_id}") + end + it "sets session expiration" do saml_resp = OneLogin::RubySaml::Response.new(subject.build) expect(saml_resp.session_expires_at).to eq Time.local(1990, "jan", 2).iso8601 From 78c18684fb232228c35e14667383ecaf55e603fe Mon Sep 17 00:00:00 2001 From: zogoo Date: Sun, 17 Nov 2024 22:12:40 +0100 Subject: [PATCH 11/21] Switchable assertion signature flag (#228) Co-authored-by: zogoo --- lib/saml_idp/controller.rb | 2 +- spec/lib/saml_idp/controller_spec.rb | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/lib/saml_idp/controller.rb b/lib/saml_idp/controller.rb index d5bd20c9..26ebbf29 100644 --- a/lib/saml_idp/controller.rb +++ b/lib/saml_idp/controller.rb @@ -69,7 +69,7 @@ def encode_authn_response(principal, opts = {}) signed_message_opts = opts[:signed_message] || false name_id_formats_opts = opts[:name_id_formats] || nil asserted_attributes_opts = opts[:attributes] || nil - signed_assertion_opts = opts[:signed_assertion] || true + signed_assertion_opts = opts[:signed_assertion].nil? ? true : opts[:signed_assertion] compress_opts = opts[:compress] || false SamlResponse.new( diff --git a/spec/lib/saml_idp/controller_spec.rb b/spec/lib/saml_idp/controller_spec.rb index 94e189e9..e33d1592 100644 --- a/spec/lib/saml_idp/controller_spec.rb +++ b/spec/lib/saml_idp/controller_spec.rb @@ -66,6 +66,16 @@ def params end end + context '#encode_authn_response' do + it 'uses default values when opts are not provided' do + saml_response = encode_authn_response(principal, { audience_uri: 'http://example.com/issuer', issuer_uri: 'http://example.com', acs_url: 'https://foo.example.com/saml/consume', signed_assertion: false }) + + response = OneLogin::RubySaml::Response.new(saml_response) + response.settings = saml_settings + expect(response.document.to_s).to_not include("") + end + end + context "solicited Response" do before(:each) do params[:SAMLRequest] = make_saml_request From 9780e65feb1cac2e72df0c06334c1e50ad74d65c Mon Sep 17 00:00:00 2001 From: zogoo Date: Tue, 21 Jan 2025 18:29:37 +0100 Subject: [PATCH 12/21] Use concurrent ruby fixed version for test (#230) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 * [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 * Try with proper way to update helper method (#19) * Set minimum test coverage (#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 Co-authored-by: zogoo * [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 * Support lowercase percent-encoded sequences for URL encoding (#20) Co-authored-by: zogoo * [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 * [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 * Support rails 8 for dev env (#23) Co-authored-by: zogoo * Assertion flag should able switchable by application (#24) Co-authored-by: zogoo * concurrent-ruby v1.3.5 has removed the dependency on logger --------- Co-authored-by: zogoo Co-authored-by: Mathieu Jobin --- gemfiles/rails_5.2.gemfile | 1 + gemfiles/rails_6.1.gemfile | 1 + gemfiles/rails_7.0.gemfile | 1 + lib/saml_idp/controller.rb | 5 +---- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/gemfiles/rails_5.2.gemfile b/gemfiles/rails_5.2.gemfile index 92f56759..0d08dd87 100644 --- a/gemfiles/rails_5.2.gemfile +++ b/gemfiles/rails_5.2.gemfile @@ -4,5 +4,6 @@ source "https://rubygems.org" gem "rails", "~> 5.2.4" gem "activeresource", "~> 5.1.0" +gem 'concurrent-ruby', '1.3.4' gemspec path: "../" diff --git a/gemfiles/rails_6.1.gemfile b/gemfiles/rails_6.1.gemfile index 34b89b00..c22c2925 100644 --- a/gemfiles/rails_6.1.gemfile +++ b/gemfiles/rails_6.1.gemfile @@ -4,5 +4,6 @@ source "https://rubygems.org" gem "rails", "~> 6.1.0" gem "activeresource", "~> 5.1.0" +gem 'concurrent-ruby', '1.3.4' gemspec path: "../" diff --git a/gemfiles/rails_7.0.gemfile b/gemfiles/rails_7.0.gemfile index f759cc55..1e30a8f2 100644 --- a/gemfiles/rails_7.0.gemfile +++ b/gemfiles/rails_7.0.gemfile @@ -4,5 +4,6 @@ source "https://rubygems.org" gem "rails", "~> 7.0.0" gem "activeresource", "~> 6.0.0" +gem 'concurrent-ruby', '1.3.4' gemspec path: "../" diff --git a/lib/saml_idp/controller.rb b/lib/saml_idp/controller.rb index 26ebbf29..a8e689f4 100644 --- a/lib/saml_idp/controller.rb +++ b/lib/saml_idp/controller.rb @@ -34,10 +34,7 @@ def acs_url def validate_saml_request(raw_saml_request = params[:SAMLRequest]) decode_request(raw_saml_request, params[:Signature], params[:SigAlg], params[:RelayState]) - return true if valid_saml_request? - - head :forbidden if defined?(::Rails) - false + valid_saml_request? end def decode_request(raw_saml_request, signature, sig_algorithm, relay_state) From 75b1cd97b7df079638f59dfe3c74619b56b50eea Mon Sep 17 00:00:00 2001 From: Taketo Takashima Date: Mon, 18 Aug 2025 04:50:58 +0900 Subject: [PATCH 13/21] Add Ruby 3.4 and Rails 8.0/7.2 to CI matrix (#236) --- .github/workflows/ci.yml | 30 +++++++++++++++++++++++------- Appraisals | 16 ++++++++++++++++ gemfiles/rails_6.1.gemfile | 4 +++- gemfiles/rails_7.0.gemfile | 4 +++- gemfiles/rails_7.2.gemfile | 2 +- gemfiles/rails_8.0.gemfile | 8 ++++++++ 6 files changed, 54 insertions(+), 10 deletions(-) create mode 100644 gemfiles/rails_8.0.gemfile diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e80550ae..690de92e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -10,7 +10,7 @@ jobs: strategy: fail-fast: false matrix: - ruby: ['2.5', '2.6', '2.7', '3.0', '3.1', '3.2', '3.3'] + ruby: ['2.5', '2.6', '2.7', '3.0', '3.1', '3.2', '3.3', '3.4'] gemfile: [rails_5.2.gemfile, rails_6.1.gemfile, rails_7.0.gemfile, rails_7.1.gemfile, rails_7.2.gemfile, rails_dev.gemfile] exclude: # Ruby 3.2 is min version for Rails 8 @@ -24,14 +24,17 @@ jobs: gemfile: rails_dev.gemfile - ruby: '3.1' gemfile: rails_dev.gemfile + # Ruby 3.2 is min version for Rails 8.0 - ruby: '2.5' - gemfile: rails_7.0.gemfile - - ruby: '2.5' - gemfile: rails_7.1.gemfile - - ruby: '2.6' - gemfile: rails_7.0.gemfile + gemfile: rails_8.0.gemfile - ruby: '2.6' - gemfile: rails_7.1.gemfile + gemfile: rails_8.0.gemfile + - ruby: '2.7' + gemfile: rails_8.0.gemfile + - ruby: '3.0' + gemfile: rails_8.0.gemfile + - ruby: '3.1' + gemfile: rails_8.0.gemfile # Ruby 3.1 is min version for Rails 7.2 - ruby: '2.5' gemfile: rails_7.2.gemfile @@ -41,6 +44,17 @@ jobs: gemfile: rails_7.2.gemfile - ruby: '3.0' gemfile: rails_7.2.gemfile + # Ruby 2.7 is min version for Rails 7.1 + - ruby: '2.5' + gemfile: rails_7.1.gemfile + - ruby: '2.6' + gemfile: rails_7.1.gemfile + # Ruby 2.7 is min version for Rails 7.0 + - ruby: '2.5' + gemfile: rails_7.0.gemfile + - ruby: '2.6' + gemfile: rails_7.0.gemfile + # Ruby 2.7 is max version for Rails 5.2 - ruby: '3.0' gemfile: rails_5.2.gemfile - ruby: '3.1' @@ -49,6 +63,8 @@ jobs: gemfile: rails_5.2.gemfile - ruby: '3.3' gemfile: rails_5.2.gemfile + - ruby: '3.4' + gemfile: rails_5.2.gemfile runs-on: ubuntu-latest env: BUNDLE_GEMFILE: gemfiles/${{ matrix.gemfile }} diff --git a/Appraisals b/Appraisals index 4893e47c..60709a28 100644 --- a/Appraisals +++ b/Appraisals @@ -6,11 +6,17 @@ end appraise 'rails-6.1' do gem 'rails', '~> 6.1.0' gem 'activeresource', '~> 5.1.0' + gem 'bigdecimal' + gem 'mutex_m' + gem 'concurrent-ruby', '1.3.4' end appraise 'rails-7.0' do gem 'rails', '~> 7.0.0' gem 'activeresource', '~> 6.0.0' + gem 'bigdecimal' + gem 'mutex_m' + gem 'concurrent-ruby', '1.3.4' end appraise 'rails-7.1' do @@ -18,6 +24,16 @@ appraise 'rails-7.1' do gem 'activeresource', '~> 6.0.0' end +appraise 'rails-7.2' do + gem 'rails', '~> 7.2.0' + gem 'activeresource', '~> 6.0.0' +end + +appraise 'rails-8.0' do + gem 'rails', '~> 8.0.0' + gem 'activeresource', '~> 6.0.0' +end + appraise 'rails-dev' do gem 'rails', :github => 'rails/rails', :branch => 'main' gem 'activeresource', :github => 'rails/activeresource', :branch => 'main' diff --git a/gemfiles/rails_6.1.gemfile b/gemfiles/rails_6.1.gemfile index c22c2925..4df3cadb 100644 --- a/gemfiles/rails_6.1.gemfile +++ b/gemfiles/rails_6.1.gemfile @@ -4,6 +4,8 @@ source "https://rubygems.org" gem "rails", "~> 6.1.0" gem "activeresource", "~> 5.1.0" -gem 'concurrent-ruby', '1.3.4' +gem "bigdecimal" +gem "mutex_m" +gem "concurrent-ruby", "1.3.4" gemspec path: "../" diff --git a/gemfiles/rails_7.0.gemfile b/gemfiles/rails_7.0.gemfile index 1e30a8f2..e8641e3a 100644 --- a/gemfiles/rails_7.0.gemfile +++ b/gemfiles/rails_7.0.gemfile @@ -4,6 +4,8 @@ source "https://rubygems.org" gem "rails", "~> 7.0.0" gem "activeresource", "~> 6.0.0" -gem 'concurrent-ruby', '1.3.4' +gem "bigdecimal" +gem "mutex_m" +gem "concurrent-ruby", "1.3.4" gemspec path: "../" diff --git a/gemfiles/rails_7.2.gemfile b/gemfiles/rails_7.2.gemfile index 6f6033ca..969d680b 100644 --- a/gemfiles/rails_7.2.gemfile +++ b/gemfiles/rails_7.2.gemfile @@ -3,6 +3,6 @@ source "https://rubygems.org" gem "rails", "~> 7.2.0" -gem 'activeresource', '~> 6.1', '>= 6.1.3' +gem "activeresource", "~> 6.0.0" gemspec path: "../" diff --git a/gemfiles/rails_8.0.gemfile b/gemfiles/rails_8.0.gemfile new file mode 100644 index 00000000..a3f12296 --- /dev/null +++ b/gemfiles/rails_8.0.gemfile @@ -0,0 +1,8 @@ +# This file was generated by Appraisal + +source "https://rubygems.org" + +gem "rails", "~> 8.0.0" +gem "activeresource", "~> 6.0.0" + +gemspec path: "../" From e7af4e29fbf939f4278cadd1f085df439616fdd3 Mon Sep 17 00:00:00 2001 From: Jon Phenow Date: Thu, 9 Oct 2025 13:11:03 -0500 Subject: [PATCH 14/21] v1 (#239) --- lib/saml_idp/version.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/saml_idp/version.rb b/lib/saml_idp/version.rb index d2edc837..f259259f 100644 --- a/lib/saml_idp/version.rb +++ b/lib/saml_idp/version.rb @@ -1,4 +1,4 @@ # encoding: utf-8 module SamlIdp - VERSION = '0.16.0' + VERSION = '1.0.0' end From abce86e8509c5333e897f9fc0b467fc8105e2df7 Mon Sep 17 00:00:00 2001 From: Taketo Takashima Date: Tue, 14 Oct 2025 06:58:20 +0900 Subject: [PATCH 15/21] CI: Follow up to add Rails 8.0 to CI matrix (#240) --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 690de92e..09c5c060 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -11,7 +11,7 @@ jobs: fail-fast: false matrix: ruby: ['2.5', '2.6', '2.7', '3.0', '3.1', '3.2', '3.3', '3.4'] - gemfile: [rails_5.2.gemfile, rails_6.1.gemfile, rails_7.0.gemfile, rails_7.1.gemfile, rails_7.2.gemfile, rails_dev.gemfile] + gemfile: [rails_5.2.gemfile, rails_6.1.gemfile, rails_7.0.gemfile, rails_7.1.gemfile, rails_7.2.gemfile, rails_8.0.gemfile, rails_dev.gemfile] exclude: # Ruby 3.2 is min version for Rails 8 - ruby: '2.5' From 407602d989c622ae9b92b37a59f0868b95d08697 Mon Sep 17 00:00:00 2001 From: zogoo Date: Mon, 3 Nov 2025 12:56:03 +0100 Subject: [PATCH 16/21] [fix] Configure a certificate and private key for each response (#227) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 * [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 * Try with proper way to update helper method (#19) * Set minimum test coverage (#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 Co-authored-by: zogoo * [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 * Support lowercase percent-encoded sequences for URL encoding (#20) Co-authored-by: zogoo * [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 * [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 * Support rails 8 for dev env (#23) Co-authored-by: zogoo * 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 * Assertion flag should able switchable by application (#24) Co-authored-by: zogoo * concurrent-ruby v1.3.5 has removed the dependency on logger (#27) Co-authored-by: zogoo * MetadataBuilder uses custom configurator (#25) Co-authored-by: Andrea Lorenzetti <64900248+andnoz@users.noreply.github.com> --------- Co-authored-by: zogoo Co-authored-by: Mathieu Jobin Co-authored-by: Massimo Zappino <99500013+mzappino-noz@users.noreply.github.com> Co-authored-by: Andrea Lorenzetti <64900248+andnoz@users.noreply.github.com> --- README.md | 5 - lib/saml_idp/algorithmable.rb | 5 +- lib/saml_idp/assertion_builder.rb | 66 +++--- lib/saml_idp/configurator.rb | 4 +- lib/saml_idp/controller.rb | 53 +++-- lib/saml_idp/logout_builder.rb | 16 +- lib/saml_idp/logout_request_builder.rb | 21 +- lib/saml_idp/logout_response_builder.rb | 21 +- lib/saml_idp/metadata_builder.rb | 12 +- lib/saml_idp/response_builder.rb | 27 ++- lib/saml_idp/saml_response.rb | 96 ++++---- lib/saml_idp/signable.rb | 35 +-- lib/saml_idp/signature_builder.rb | 29 +-- lib/saml_idp/signed_info_builder.rb | 25 +-- spec/lib/saml_idp/assertion_builder_spec.rb | 210 ++++++++++-------- spec/lib/saml_idp/configurator_spec.rb | 4 +- .../saml_idp/logout_request_builder_spec.rb | 12 +- .../saml_idp/logout_response_builder_spec.rb | 12 +- spec/lib/saml_idp/metadata_builder_spec.rb | 31 +++ spec/lib/saml_idp/response_builder_spec.rb | 15 +- spec/lib/saml_idp/saml_response_spec.rb | 75 ++++--- spec/lib/saml_idp/signable_spec.rb | 15 +- spec/lib/saml_idp/signature_builder_spec.rb | 3 +- spec/lib/saml_idp/signed_info_builder_spec.rb | 7 +- spec/spec_helper.rb | 1 + .../certificates/sp_encrypted_pv_key.pem | 30 +++ spec/support/certificates/sp_public_cert.pem | 16 ++ spec/support/security_helpers.rb | 8 + 28 files changed, 541 insertions(+), 313 deletions(-) create mode 100644 spec/support/certificates/sp_encrypted_pv_key.pem create mode 100644 spec/support/certificates/sp_public_cert.pem diff --git a/README.md b/README.md index 50f99ebe..d8c48dd7 100644 --- 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 index 354dbb92..182d6114 100644 --- 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 index 8e0170fa..482ab16a 100644 --- 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 index 99a662a0..e645f912 100644 --- 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 index a8e689f4..283aff38 100644 --- 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 index 7d049dd6..230e898c 100644 --- 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 index b75f9315..87f76c89 100644 --- 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 index fd14116b..ee9370cb 100644 --- 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 index 92258e38..246297ed 100644 --- a/lib/saml_idp/metadata_builder.rb +++ b/lib/saml_idp/metadata_builder.rb @@ -152,7 +152,7 @@ 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-----/,"") @@ -160,6 +160,16 @@ def x509_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 diff --git a/lib/saml_idp/response_builder.rb b/lib/saml_idp/response_builder.rb index 0686cd9b..fc278b98 100644 --- a/lib/saml_idp/response_builder.rb +++ b/lib/saml_idp/response_builder.rb @@ -11,16 +11,32 @@ class ResponseBuilder attr_accessor :saml_request_id attr_accessor :assertion_and_signature attr_accessor :raw_algorithm + attr_accessor :public_cert + attr_accessor :private_key + attr_accessor :pv_key_password alias_method :reference_id, :response_id - def initialize(response_id, issuer_uri, saml_acs_url, saml_request_id, assertion_and_signature, raw_algorithm) + def initialize( + response_id:, + issuer_uri:, + saml_acs_url:, + saml_request_id:, + assertion_and_signature:, + raw_algorithm:, + public_cert:, + private_key:, + pv_key_password: + ) self.response_id = response_id self.issuer_uri = issuer_uri self.saml_acs_url = saml_acs_url self.saml_request_id = saml_request_id self.assertion_and_signature = assertion_and_signature self.raw_algorithm = raw_algorithm + self.public_cert = public_cert + self.private_key = private_key + self.pv_key_password = pv_key_password end def encoded(signed_message: false, compress: false) @@ -31,15 +47,15 @@ def raw build end + private + def encode_raw_message(compress) Base64.strict_encode64(compress ? deflate(raw) : raw) end - private :encode_raw_message def encode_signed_message(compress) Base64.strict_encode64(compress ? deflate(signed) : signed) end - private :encode_signed_message def build resp_options = {} @@ -61,22 +77,17 @@ def build response << assertion_and_signature end end - private :build def response_id_string "_#{response_id}" end - alias_method :reference_id, :response_id - private :response_id_string def now_iso Time.now.utc.iso8601 end - private :now_iso def deflate(inflated) Zlib::Deflate.deflate(inflated, 9)[2..-5] end - private :deflate end end diff --git a/lib/saml_idp/saml_response.rb b/lib/saml_idp/saml_response.rb index 85cf48b1..ecda5cfe 100644 --- a/lib/saml_idp/saml_response.rb +++ b/lib/saml_idp/saml_response.rb @@ -12,8 +12,9 @@ class SamlResponse attr_accessor :saml_request_id attr_accessor :saml_acs_url attr_accessor :algorithm - attr_accessor :secret_key - attr_accessor :x509_certificate + attr_accessor :private_key + attr_accessor :pv_key_password + attr_accessor :public_cert attr_accessor :authn_context_classref attr_accessor :expiry attr_accessor :encryption_opts @@ -25,23 +26,26 @@ class SamlResponse attr_accessor :compression_opts def initialize( - reference_id, - response_id, - issuer_uri, - principal, - audience_uri, - saml_request_id, - saml_acs_url, - algorithm, - authn_context_classref, - expiry = 60 * 60, - encryption_opts = nil, - session_expiry = 0, - name_id_formats_opts = nil, - asserted_attributes_opts = nil, - signed_message_opts = false, - signed_assertion_opts = true, - compression_opts = false + reference_id:, + response_id:, + issuer_uri:, + principal:, + audience_uri:, + saml_request_id:, + saml_acs_url:, + algorithm:, + authn_context_classref:, + public_cert:, + private_key:, + pv_key_password:, + expiry: 60 * 60, + encryption_opts: nil, + session_expiry: 0, + name_id_formats_opts: nil, + asserted_attributes_opts: nil, + signed_message_opts: false, + signed_assertion_opts: true, + compression_opts: false ) self.reference_id = reference_id @@ -52,8 +56,9 @@ def initialize( self.saml_request_id = saml_request_id self.saml_acs_url = saml_acs_url self.algorithm = algorithm - self.secret_key = secret_key - self.x509_certificate = x509_certificate + self.private_key = private_key + self.pv_key_password = pv_key_password + self.public_cert = public_cert self.authn_context_classref = authn_context_classref self.expiry = expiry self.encryption_opts = encryption_opts @@ -80,7 +85,8 @@ def signed_assertion assertion_builder.raw end end - private :signed_assertion + + private def encoded_message if signed_message_opts @@ -89,29 +95,41 @@ def encoded_message response_builder.encoded(signed_message: false, compress: compression_opts) end end - private :encoded_message def response_builder - ResponseBuilder.new(response_id, issuer_uri, saml_acs_url, saml_request_id, signed_assertion, algorithm) + ResponseBuilder.new( + response_id: response_id, + issuer_uri: issuer_uri, + saml_acs_url: saml_acs_url, + saml_request_id: saml_request_id, + assertion_and_signature: signed_assertion, + raw_algorithm: algorithm, + public_cert: public_cert, + private_key: private_key, + pv_key_password: pv_key_password, + ) end - private :response_builder def assertion_builder @assertion_builder ||= - AssertionBuilder.new(reference_id || SecureRandom.uuid, - issuer_uri, - principal, - audience_uri, - saml_request_id, - saml_acs_url, - algorithm, - authn_context_classref, - expiry, - encryption_opts, - session_expiry, - name_id_formats_opts, - asserted_attributes_opts) + AssertionBuilder.new( + reference_id: (reference_id || SecureRandom.uuid), + issuer_uri: issuer_uri, + principal: principal, + audience_uri: audience_uri, + saml_request_id: saml_request_id, + saml_acs_url: saml_acs_url, + raw_algorithm: algorithm, + authn_context_classref: 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 + ) end - private :assertion_builder end end diff --git a/lib/saml_idp/signable.rb b/lib/saml_idp/signable.rb index 0a330a18..14c923a8 100644 --- a/lib/saml_idp/signable.rb +++ b/lib/saml_idp/signable.rb @@ -5,6 +5,7 @@ # * algorithm require 'saml_idp/signed_info_builder' require 'saml_idp/signature_builder' + module SamlIdp module Signable def self.included(base) @@ -24,6 +25,8 @@ def sign(el) el << signature if sign? end + private + def generated_reference_id if reference_id fin = yield reference_id if block_given? @@ -34,12 +37,10 @@ def generated_reference_id end block_given? ? fin : ref end - private :generated_reference_id def reference_id_generator SamlIdp.config.reference_id_generator end - private :reference_id_generator def with_signature original = @sign @@ -48,7 +49,6 @@ def with_signature @sign = original end end - private :with_signature def without_signature original = @sign @@ -57,49 +57,52 @@ def without_signature @sign = original end end - private :without_signature def sign? !!@sign end - private :sign? def signature - SignatureBuilder.new(signed_info_builder).raw + SignatureBuilder.new(signed_info_builder, get_public_cert).raw end - private :signature def signed_info_builder - SignedInfoBuilder.new(get_reference_id, get_digest, get_algorithm) + SignedInfoBuilder.new(get_reference_id, get_digest, get_algorithm, get_private_key, pv_key_password) end - private :signed_info_builder def get_reference_id send(self.class.reference_id_method) end - private :get_reference_id def get_digest without_signature do send(self.class.digest_method) end end - private :get_digest def get_algorithm send(self.class.algorithm_method) end - private :get_algorithm def get_raw send(self.class.raw_method) end - private :get_raw + + def get_public_cert + send(self.class.public_cert_method) + end + + def get_private_key + send(self.class.private_key_method) + end + + def pv_key_password + send(self.class.pv_key_password_method) + end def noko_raw Nokogiri::XML::Document.parse(get_raw) end - private :noko_raw def digest # Make it check for inclusive at some point (https://github.com/onelogin/ruby-saml/blob/master/lib/xml_security.rb#L159) @@ -111,7 +114,6 @@ def digest hash = digest_algorithm.digest(canon_hashed_element) Base64.strict_encode64(hash).gsub(/\n/, '') end - private :digest module ClassMethods def self.module_method(name, default = nil) @@ -125,6 +127,9 @@ def self.module_method(name, default = nil) module_method :digest module_method :algorithm module_method :reference_id + module_method :public_cert + module_method :private_key + module_method :pv_key_password end end end diff --git a/lib/saml_idp/signature_builder.rb b/lib/saml_idp/signature_builder.rb index 0406715c..fe6e4c39 100644 --- a/lib/saml_idp/signature_builder.rb +++ b/lib/saml_idp/signature_builder.rb @@ -1,10 +1,12 @@ require 'builder' + module SamlIdp class SignatureBuilder - attr_accessor :signed_info_builder + attr_accessor :signed_info_builder, :x509_certificate - def initialize(signed_info_builder) + def initialize(signed_info_builder, x509_certificate) self.signed_info_builder = signed_info_builder + self.x509_certificate = x509_certificate end def raw @@ -14,30 +16,31 @@ def raw signature.tag! "ds:SignatureValue", signature_value signature.KeyInfo xmlns: "http://www.w3.org/2000/09/xmldsig#" do |key_info| key_info.tag! "ds:X509Data" do |x509| - x509.tag! "ds:X509Certificate", x509_certificate + x509.tag! "ds:X509Certificate", der_certificate end end end end - def x509_certificate - certificate = SamlIdp.config.x509_certificate.is_a?(Proc) ? SamlIdp.config.x509_certificate.call : SamlIdp.config.x509_certificate - certificate - .to_s - .gsub(/-----BEGIN CERTIFICATE-----/,"") - .gsub(/-----END CERTIFICATE-----/,"") - .gsub(/\n/, "") + private + + def der_certificate + certificate_formatter(x509_certificate) end - private :x509_certificate def signed_info signed_info_builder.raw end - private :signed_info def signature_value signed_info_builder.signed end - private :signature_value + + def certificate_formatter(pem_certificate) + pem_certificate + .gsub(/-----BEGIN CERTIFICATE-----/,"") + .gsub(/-----END CERTIFICATE-----/,"") + .gsub(/\n/, "") + end end end diff --git a/lib/saml_idp/signed_info_builder.rb b/lib/saml_idp/signed_info_builder.rb index 4b0ff224..a858197e 100644 --- a/lib/saml_idp/signed_info_builder.rb +++ b/lib/saml_idp/signed_info_builder.rb @@ -22,11 +22,15 @@ class SignedInfoBuilder attr_accessor :reference_id attr_accessor :digest_value attr_accessor :raw_algorithm + attr_accessor :private_key + attr_accessor :pv_key_password - def initialize(reference_id, digest_value, raw_algorithm) + def initialize(reference_id, digest_value, raw_algorithm, private_key, pv_key_password) self.reference_id = reference_id self.digest_value = digest_value self.raw_algorithm = raw_algorithm + self.private_key = private_key + self.pv_key_password = pv_key_password end def raw @@ -49,40 +53,27 @@ def signed encoded.gsub(/\n/, "") end + private + def digest_method DIGEST_METHODS.fetch(clean_algorithm_name, DIGEST_METHODS["sha1"]) end - private :digest_method def signature_method SIGNATURE_METHODS.fetch(clean_algorithm_name, SIGNATURE_METHODS["sha1"]) end - private :signature_method def clean_algorithm_name algorithm_name.to_s.downcase end - private :clean_algorithm_name - - def secret_key - SamlIdp.config.secret_key.is_a?(Proc) ? SamlIdp.config.secret_key.call : SamlIdp.config.secret_key - end - private :secret_key - - def password - SamlIdp.config.password.is_a?(Proc) ? SamlIdp.config.password.call : SamlIdp.config.password - end - private :password def encoded - key = OpenSSL::PKey::RSA.new(secret_key, password) + key = OpenSSL::PKey::RSA.new(private_key, pv_key_password) Base64.strict_encode64(key.sign(algorithm.new, raw)) end - private :encoded def reference_string "#_#{reference_id}" end - private :reference_string end end diff --git a/spec/lib/saml_idp/assertion_builder_spec.rb b/spec/lib/saml_idp/assertion_builder_spec.rb index 9e267464..8592cd3f 100644 --- a/spec/lib/saml_idp/assertion_builder_spec.rb +++ b/spec/lib/saml_idp/assertion_builder_spec.rb @@ -1,4 +1,6 @@ require 'spec_helper' +require 'debug' + module SamlIdp describe AssertionBuilder do let(:reference_id) { "abc" } @@ -22,16 +24,21 @@ module SamlIdp let(:session_expiry) { nil } let(:name_id_formats_opt) { nil } let(:asserted_attributes_opt) { nil } + let(:principal) { Struct.new(:email, :asserted_attributes).new('foo@example.com', { emailAddress: { getter: :email } })} + subject { described_class.new( - reference_id, - issuer_uri, - name_id, - audience_uri, - saml_request_id, - saml_acs_url, - algorithm, - authn_context_classref, - expiry + reference_id: reference_id, + issuer_uri: issuer_uri, + principal: name_id, + audience_uri: audience_uri, + saml_request_id: saml_request_id, + saml_acs_url: saml_acs_url, + raw_algorithm: algorithm, + authn_context_classref: authn_context_classref, + public_cert: sp_encrypted_pv_key[:sp_public_cert], + private_key: sp_encrypted_pv_key[:sp_encrypted_pv_key], + pv_key_password: sp_encrypted_pv_key[:pv_key_password], + expiry: expiry ) } context "No Request ID" do @@ -70,18 +77,19 @@ module SamlIdp describe "with principal.asserted_attributes" do it "delegates attributes to principal" do - Principal = Struct.new(:email, :asserted_attributes) - principal = Principal.new('foo@example.com', { emailAddress: { getter: :email } }) builder = described_class.new( - reference_id, - issuer_uri, - principal, - audience_uri, - saml_request_id, - saml_acs_url, - algorithm, - authn_context_classref, - expiry + reference_id: reference_id, + issuer_uri: issuer_uri, + principal: principal, + audience_uri: audience_uri, + saml_request_id: saml_request_id, + saml_acs_url: saml_acs_url, + raw_algorithm: algorithm, + authn_context_classref: authn_context_classref, + public_cert: sp_encrypted_pv_key[:sp_public_cert], + private_key: sp_encrypted_pv_key[:sp_encrypted_pv_key], + pv_key_password: sp_encrypted_pv_key[:pv_key_password], + expiry: expiry ) Timecop.travel(Time.zone.local(2010, 6, 1, 13, 0, 0)) do expect(builder.raw).to eq("http://sportngin.comfoo@example.comhttp://example.comurn:oasis:names:tc:SAML:2.0:ac:classes:Passwordfoo@example.com") @@ -91,16 +99,19 @@ module SamlIdp it "builds encrypted XML" do builder = described_class.new( - reference_id, - issuer_uri, - name_id, - audience_uri, - saml_request_id, - saml_acs_url, - algorithm, - authn_context_classref, - expiry, - encryption_opts + reference_id: reference_id, + issuer_uri: issuer_uri, + principal: name_id, + audience_uri: audience_uri, + saml_request_id: saml_request_id, + saml_acs_url: saml_acs_url, + raw_algorithm: algorithm, + authn_context_classref: authn_context_classref, + public_cert: sp_encrypted_pv_key[:sp_public_cert], + private_key: sp_encrypted_pv_key[:sp_encrypted_pv_key], + pv_key_password: sp_encrypted_pv_key[:pv_key_password], + expiry: expiry, + encryption_opts: encryption_opts ) encrypted_xml = builder.encrypt expect(encrypted_xml).to_not match(audience_uri) @@ -118,19 +129,22 @@ module SamlIdp UserWithUniqueId = Struct.new(:unique_identifier, :email, :asserted_attributes) principal = UserWithUniqueId.new('unique_identifier_123456', 'foo@example.com', { emailAddress: { getter: :email } }) builder = described_class.new( - reference_id, - issuer_uri, - principal, - audience_uri, - saml_request_id, - saml_acs_url, - algorithm, - authn_context_classref, - expiry, - encryption_opts, - session_expiry, - name_id_formats_opt, - asserted_attributes_opt + reference_id: reference_id, + issuer_uri: issuer_uri, + principal: principal, + audience_uri: audience_uri, + saml_request_id: saml_request_id, + saml_acs_url: saml_acs_url, + raw_algorithm: algorithm, + authn_context_classref: authn_context_classref, + public_cert: sp_encrypted_pv_key[:sp_public_cert], + private_key: sp_encrypted_pv_key[:sp_encrypted_pv_key], + pv_key_password: sp_encrypted_pv_key[:pv_key_password], + expiry: expiry, + encryption_opts: encryption_opts, + session_expiry: session_expiry, + name_id_formats_opts: name_id_formats_opt, + asserted_attributes_opts: asserted_attributes_opt ) Timecop.travel(Time.zone.local(2010, 6, 1, 13, 0, 0)) do expect(builder.raw).to eq("http://sportngin.comunique_identifier_123456http://example.comurn:oasis:names:tc:SAML:2.0:ac:classes:Passwordfoo@example.com") @@ -156,19 +170,22 @@ module SamlIdp UserWithName = Struct.new(:email, :first_name, :last_name) principal = UserWithName.new('foo@example.com', 'George', 'Washington') builder = described_class.new( - reference_id, - issuer_uri, - principal, - audience_uri, - saml_request_id, - saml_acs_url, - algorithm, - authn_context_classref, - expiry, - encryption_opts, - session_expiry, - name_id_formats_opt, - asserted_attributes_opt + reference_id: reference_id, + issuer_uri: issuer_uri, + principal: principal, + audience_uri: audience_uri, + saml_request_id: saml_request_id, + saml_acs_url: saml_acs_url, + raw_algorithm: algorithm, + authn_context_classref: authn_context_classref, + public_cert: sp_encrypted_pv_key[:sp_public_cert], + private_key: sp_encrypted_pv_key[:sp_encrypted_pv_key], + pv_key_password: sp_encrypted_pv_key[:pv_key_password], + expiry: expiry, + encryption_opts: encryption_opts, + session_expiry: session_expiry, + name_id_formats_opts: name_id_formats_opt, + asserted_attributes_opts: asserted_attributes_opt ) Timecop.travel(Time.zone.local(2010, 6, 1, 13, 0, 0)) do expect(builder.raw).to eq("http://sportngin.comfoo@example.comhttp://example.comurn:oasis:names:tc:SAML:2.0:ac:classes:PasswordGeorgeWashington") @@ -185,16 +202,19 @@ module SamlIdp it "sets default session_expiry from config" do builder = described_class.new( - reference_id, - issuer_uri, - name_id, - audience_uri, - saml_request_id, - saml_acs_url, - algorithm, - authn_context_classref, - expiry, - encryption_opts + reference_id: reference_id, + issuer_uri: issuer_uri, + principal: name_id, + audience_uri: audience_uri, + saml_request_id: saml_request_id, + saml_acs_url: saml_acs_url, + raw_algorithm: algorithm, + authn_context_classref: authn_context_classref, + public_cert: sp_encrypted_pv_key[:sp_public_cert], + private_key: sp_encrypted_pv_key[:sp_encrypted_pv_key], + pv_key_password: sp_encrypted_pv_key[:pv_key_password], + expiry: expiry, + encryption_opts: encryption_opts ) expect(builder.session_expiry).to eq(8) end @@ -212,19 +232,22 @@ module SamlIdp UserWithUniqueId = Struct.new(:unique_identifier, :email, :asserted_attributes) principal = UserWithUniqueId.new('unique_identifier_123456', 'foo@example.com', { emailAddress: { getter: :email } }) builder = described_class.new( - reference_id, - issuer_uri, - principal, - audience_uri, - saml_request_id, - saml_acs_url, - algorithm, - authn_context_classref, - expiry, - encryption_opts, - session_expiry, - name_id_formats_opt, - asserted_attributes_opt + reference_id: reference_id, + issuer_uri: issuer_uri, + principal: principal, + audience_uri: audience_uri, + saml_request_id: saml_request_id, + saml_acs_url: saml_acs_url, + raw_algorithm: algorithm, + authn_context_classref: authn_context_classref, + public_cert: sp_encrypted_pv_key[:sp_public_cert], + private_key: sp_encrypted_pv_key[:sp_encrypted_pv_key], + pv_key_password: sp_encrypted_pv_key[:pv_key_password], + expiry: expiry, + encryption_opts: encryption_opts, + session_expiry: session_expiry, + name_id_formats_opts: name_id_formats_opt, + asserted_attributes_opts: asserted_attributes_opt ) Timecop.travel(Time.zone.local(2010, 6, 1, 13, 0, 0)) do expect(builder.raw).to eq("http://sportngin.comunique_identifier_123456http://example.comurn:oasis:names:tc:SAML:2.0:ac:classes:Passwordfoo@example.com") @@ -250,19 +273,22 @@ module SamlIdp UserWithName = Struct.new(:email, :first_name, :last_name) principal = UserWithName.new('foo@example.com', 'George', 'Washington') builder = described_class.new( - reference_id, - issuer_uri, - principal, - audience_uri, - saml_request_id, - saml_acs_url, - algorithm, - authn_context_classref, - expiry, - encryption_opts, - session_expiry, - name_id_formats_opt, - asserted_attributes_opt + reference_id: reference_id, + issuer_uri: issuer_uri, + principal: principal, + audience_uri: audience_uri, + saml_request_id: saml_request_id, + saml_acs_url: saml_acs_url, + raw_algorithm: algorithm, + authn_context_classref: authn_context_classref, + public_cert: sp_encrypted_pv_key[:sp_public_cert], + private_key: sp_encrypted_pv_key[:sp_encrypted_pv_key], + pv_key_password: sp_encrypted_pv_key[:pv_key_password], + expiry: expiry, + encryption_opts: encryption_opts, + session_expiry: session_expiry, + name_id_formats_opts: name_id_formats_opt, + asserted_attributes_opts: asserted_attributes_opt ) Timecop.travel(Time.zone.local(2010, 6, 1, 13, 0, 0)) do expect(builder.raw).to eq("http://sportngin.comfoo@example.comhttp://example.comurn:oasis:names:tc:SAML:2.0:ac:classes:PasswordGeorgeWashington") diff --git a/spec/lib/saml_idp/configurator_spec.rb b/spec/lib/saml_idp/configurator_spec.rb index 80da3b14..59c399a8 100644 --- a/spec/lib/saml_idp/configurator_spec.rb +++ b/spec/lib/saml_idp/configurator_spec.rb @@ -20,11 +20,11 @@ module SamlIdp it { should respond_to :logger } it "has a valid x509_certificate" do - expect(subject.x509_certificate.call).to eq(Default::X509_CERTIFICATE) + expect(subject.x509_certificate).to eq(Default::X509_CERTIFICATE) end it "has a valid secret_key" do - expect(subject.secret_key.call).to eq(Default::SECRET_KEY) + expect(subject.secret_key).to eq(Default::SECRET_KEY) end it "has a valid algorithm" do diff --git a/spec/lib/saml_idp/logout_request_builder_spec.rb b/spec/lib/saml_idp/logout_request_builder_spec.rb index 73f0a626..26890159 100644 --- a/spec/lib/saml_idp/logout_request_builder_spec.rb +++ b/spec/lib/saml_idp/logout_request_builder_spec.rb @@ -19,11 +19,13 @@ module SamlIdp subject do described_class.new( - response_id, - issuer_uri, - saml_slo_url, - name_id, - algorithm + response_id: response_id, + issuer_uri: issuer_uri, + saml_slo_url: saml_slo_url, + name_id: name_id, + algorithm: algorithm, + public_cert: SamlIdp::Default::X509_CERTIFICATE, + private_key: SamlIdp::Default::SECRET_KEY ) end diff --git a/spec/lib/saml_idp/logout_response_builder_spec.rb b/spec/lib/saml_idp/logout_response_builder_spec.rb index a4ca7bfd..44f730dd 100644 --- a/spec/lib/saml_idp/logout_response_builder_spec.rb +++ b/spec/lib/saml_idp/logout_response_builder_spec.rb @@ -19,11 +19,13 @@ module SamlIdp subject do described_class.new( - response_id, - issuer_uri, - saml_slo_url, - request_id, - algorithm + response_id: response_id, + issuer_uri: issuer_uri, + saml_slo_url: saml_slo_url, + saml_request_id: request_id, + algorithm: algorithm, + public_cert: SamlIdp::Default::X509_CERTIFICATE, + private_key: SamlIdp::Default::SECRET_KEY ) end diff --git a/spec/lib/saml_idp/metadata_builder_spec.rb b/spec/lib/saml_idp/metadata_builder_spec.rb index 453a8d81..2b84c295 100644 --- a/spec/lib/saml_idp/metadata_builder_spec.rb +++ b/spec/lib/saml_idp/metadata_builder_spec.rb @@ -71,5 +71,36 @@ module SamlIdp subject.configurator.single_logout_service_redirect_location = 'https://example.com/saml/logout' expect(subject.fresh).to match('') end + + context 'with custom configurator' do + let(:certificate) {'a certificate'} + let(:configurator) do SamlIdp::Configurator.new.tap do |c| + c.secret_key = 'a private key' + c.x509_certificate = certificate + end + end + subject { described_class.new(configurator) } + + describe '.private_key' do + it 'returns the given private_key' do + expect(subject.private_key).to eq(configurator.secret_key) + end + end + + describe '.x509_certificate' do + context 'with a given certificate string' do + it 'returns the given certificate' do + expect(subject.x509_certificate).to eq('a certificate') + end + end + + context 'with a given certificate proc' do + let(:certificate) {Proc.new { "a certificate from proc"}} + it 'returns the given certificate' do + expect(subject.x509_certificate).to eq('a certificate from proc') + end + end + end + end end end diff --git a/spec/lib/saml_idp/response_builder_spec.rb b/spec/lib/saml_idp/response_builder_spec.rb index 9105eede..ce5224e8 100644 --- a/spec/lib/saml_idp/response_builder_spec.rb +++ b/spec/lib/saml_idp/response_builder_spec.rb @@ -8,12 +8,15 @@ module SamlIdp let(:assertion_and_signature) { "http://sportngin.comstuffjon.phenow@sportngin.comhttp://example.comjon.phenow@sportngin.comurn:federation:authentication:windows" } let(:algorithm) { :sha256 } subject { described_class.new( - response_id, - issuer_uri, - saml_acs_url, - saml_request_id, - assertion_and_signature, - algorithm + response_id: response_id, + issuer_uri: issuer_uri, + saml_acs_url: saml_acs_url, + saml_request_id: saml_request_id, + assertion_and_signature: assertion_and_signature, + raw_algorithm: algorithm, + public_cert: sp_encrypted_pv_key[:sp_public_cert], + private_key: sp_encrypted_pv_key[:sp_encrypted_pv_key], + pv_key_password: sp_encrypted_pv_key[:pv_key_password], ) } before do diff --git a/spec/lib/saml_idp/saml_response_spec.rb b/spec/lib/saml_idp/saml_response_spec.rb index b79f8a2b..f702054c 100644 --- a/spec/lib/saml_idp/saml_response_spec.rb +++ b/spec/lib/saml_idp/saml_response_spec.rb @@ -28,43 +28,50 @@ module SamlIdp let(:unsigned_response_opts) { false } let(:signed_assertion_opts) { true } let(:compress_opts) { false } - let(:subject_encrypted) { described_class.new(reference_id, - response_id, - issuer_uri, - name_id, - audience_uri, - saml_request_id, - saml_acs_url, - algorithm, - authn_context_classref, - expiry, - encryption_opts, - session_expiry, - nil, - nil, - unsigned_response_opts, - signed_assertion_opts, - compress_opts + let(:subject_encrypted) { described_class.new( + reference_id: reference_id, + response_id: response_id, + issuer_uri: issuer_uri, + principal: name_id, + audience_uri: audience_uri, + saml_request_id: saml_request_id, + saml_acs_url: saml_acs_url, + algorithm: algorithm, + authn_context_classref: authn_context_classref, + public_cert: x509_certificate, + private_key: secret_key, + pv_key_password: nil, + expiry: expiry, + encryption_opts: encryption_opts, + session_expiry: session_expiry, + name_id_formats_opts: nil, + asserted_attributes_opts: nil, + signed_message_opts: unsigned_response_opts, + signed_assertion_opts: signed_assertion_opts, + compression_opts: compress_opts ) } - subject { described_class.new(reference_id, - response_id, - issuer_uri, - name_id, - audience_uri, - saml_request_id, - saml_acs_url, - algorithm, - authn_context_classref, - expiry, - nil, - session_expiry, - nil, - nil, - signed_response_opts, - signed_assertion_opts, - compress_opts + subject { described_class.new(reference_id: reference_id, + response_id: response_id, + issuer_uri: issuer_uri, + principal: name_id, + audience_uri: audience_uri, + saml_request_id: saml_request_id, + saml_acs_url: saml_acs_url, + algorithm: algorithm, + authn_context_classref: authn_context_classref, + public_cert: x509_certificate, + private_key: secret_key, + pv_key_password: nil, + expiry: expiry, + encryption_opts: nil, + session_expiry: session_expiry, + name_id_formats_opts: nil, + asserted_attributes_opts: nil, + signed_message_opts: signed_response_opts, + signed_assertion_opts: signed_assertion_opts, + compression_opts: compress_opts ) } diff --git a/spec/lib/saml_idp/signable_spec.rb b/spec/lib/saml_idp/signable_spec.rb index cd9a90ae..0d033780 100644 --- a/spec/lib/saml_idp/signable_spec.rb +++ b/spec/lib/saml_idp/signable_spec.rb @@ -1,7 +1,9 @@ # encoding: utf-8 require 'spec_helper' + class MockSignable include SamlIdp::Signable + include SecurityHelpers def raw builder = Builder::XmlMarkup.new @@ -21,6 +23,18 @@ def digest def algorithm OpenSSL::Digest::SHA1 end + + def private_key + sp_encrypted_pv_key[:sp_encrypted_pv_key] + end + + def pv_key_password + sp_encrypted_pv_key[:pv_key_password] + end + + def public_cert + sp_encrypted_pv_key[:sp_public_cert] + end end module SamlIdp @@ -72,6 +86,5 @@ module SamlIdp it "has a valid signed" do expect(subject.signed).to match all_regex end - end end diff --git a/spec/lib/saml_idp/signature_builder_spec.rb b/spec/lib/saml_idp/signature_builder_spec.rb index ff28575a..8fe612b2 100644 --- a/spec/lib/saml_idp/signature_builder_spec.rb +++ b/spec/lib/saml_idp/signature_builder_spec.rb @@ -5,7 +5,8 @@ module SamlIdp let(:raw_info) { "em8csGAWynywpe8S4nN64o56/4DosXi2XWMY6RJ6YfA=" } let(:signed_info) { double raw: raw_info, signed: signed } subject { described_class.new( - signed_info + signed_info, + SamlIdp::Default::X509_CERTIFICATE ) } before do diff --git a/spec/lib/saml_idp/signed_info_builder_spec.rb b/spec/lib/saml_idp/signed_info_builder_spec.rb index 4685d39a..0ac39966 100644 --- a/spec/lib/saml_idp/signed_info_builder_spec.rb +++ b/spec/lib/saml_idp/signed_info_builder_spec.rb @@ -1,4 +1,5 @@ require 'spec_helper' + module SamlIdp describe SignedInfoBuilder do let(:reference_id) { "abc" } @@ -7,7 +8,9 @@ module SamlIdp subject { described_class.new( reference_id, digest, - algorithm + algorithm, + sp_encrypted_pv_key[:sp_encrypted_pv_key], + sp_encrypted_pv_key[:pv_key_password] ) } before do @@ -19,7 +22,7 @@ module SamlIdp end it "builds a legit digest of the XML file" do - expect(subject.signed).to eq("hKLeWLRgatHcV6N5Fc8aKveqNp6Y/J4m2WSYp0awGFtsCTa/2nab32wI3du+3kuuIy59EDKeUhHVxEfyhoHUo6xTZuO2N7XcTpSonuZ/CB3WjozC2Q/9elss3z1rOC3154v5pW4puirLPRoG+Pwi8SmptxNRHczr6NvmfYmmGfo=") + expect(subject.signed).to eq("YP2e6cTEfRj1vI1/gaaSApLAMxPBQzyuBvbvulbS99x17LCLDSKvqA6MyU4WLavmVba5qiF88e97f0XKLsse7gEGOfnF/6jaRV3fePXk6+LFaeYUHZ11u7PkZ1/ucO459ASsuPN/9P9xCY2t+jtVKvIrcSZQbomymfsWGt9P/oY83elKU712aAwqcfvINsa1N+RefZRwdAW4ATBwwcDjE3VTR6mKOyGMsPJ4HQcPrNiEmuwd1QaPH0H1LLzxtewGQGmIL2UqNE/QMe/kKiSTFZ0loBKuSEc9WBw5XuH31QxbzpLJjqM/C1qy4aykPqDUuJtQ4csx78GgfFS4uiqowg==") end end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index ffc9f9a8..61819b77 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -17,6 +17,7 @@ require 'ruby-saml' require 'saml_idp' require 'timecop' +require 'debug' Dir[File.dirname(__FILE__) + "/support/**/*.rb"].each {|f| require f} diff --git a/spec/support/certificates/sp_encrypted_pv_key.pem b/spec/support/certificates/sp_encrypted_pv_key.pem new file mode 100644 index 00000000..2fecdc96 --- /dev/null +++ b/spec/support/certificates/sp_encrypted_pv_key.pem @@ -0,0 +1,30 @@ +-----BEGIN RSA PRIVATE KEY----- +Proc-Type: 4,ENCRYPTED +DEK-Info: AES-128-CBC,89BFDC1035C5C7A2277D87B8F0763A70 + +9Tpy80djhnb8S7Xq7O94TkH6ec38gO54JWbMbC3xTtVnR8lqvX6kIeF5gLjbySUI +Q30F+1bZ1/AiT1PeJ9Mg+TUEFDxiNrRw+liS3F6mhM5XyGe96FVOGEKp+WbCEznV +ZgLzzMXXwcAzKblsVNAiBAozqyfdhyGg/BhTcPXpAXt3wxy4VfbwHs1CW5+Ar+iN +qlYXDFyg+UPh0EbKQc7FUB6NKcgZ76yuaWQlkUfAbBjx4f7zpHSwYO29Gv2Hq/E4 +REhS4GXuJDTZLSDW3n6TcOszyudVYNSNFEjDWtc88/l3gQW5ZtJVnLarNMOfQF48 +MFFQBT/ksT/9GbCIKOonr12D+XlGgHJpNJrHQ+Hzee3sU8U4tn2/4sM2ZMrjOiMh +giuJq1OiM95ohK3XcdcE5apbMtCTzBjcwumG6MaI/okQURHuHvMvdhivhp5CC1Ix +dRx9cnCmXidEdhsgKBq5vBvcqt4VJqqkaGEA0YBAge//LZbzc1fp8Ghzjdev78Sb +SeW/QnzGTPW4FX/UMLrZPuwNpx9fJ1pJdI8UOOWBxIBkvl+1dM9WOVP+07b80RO1 +rJm+QKZVEbrHZKX8Gq3+DyvI2a+FUWv0WTDxp4fcPMAGy+Kr8Xratxh/fQhvksbV +ZK6/m/8f97zfxsUU5tM0B+WwIv6/KQm3wwloeL226OgmBX8x+WKthFe+MozchVLI +UNvXCw5NeGmIbLWlcoZuzsvHHcfNJ/iDaBLl6itJxd1sNiuamTHfpTQlxoDCwaQO +OqRRMfq8pjEgHYXHMm0us3sQ235yN5W32K2N79YlTQOQUA9QYHLlpmMvVRtOSRQY +GNhIPNWqLWP7+CAtfXVA0o97p5Wd0ySWwOcv3iJuAxywBg0a6s6iYV2lkRHHkJY2 +dvDH39hbaqRakPUKFOPkCHdHvN+cM7trvdxpzURj67u9IKLPMwK64FJ3LEerqSgh +FIcwTcN8jQBLdcsi9XHNPRJEaa+4Sf7KpeUYe79yZlFNDKU6yG8DMpt2uJ6p17C7 +R2L11AdsPC0MAgxaj2yNV1/CSP3wc6Y36AMSnRJtMsUX7BjZT6TSUjOj21zPfu9R +h2aJYe1224EpT6bb31Sgfky2TcIOXAY60g8slRLniCDzmexB6u/EUvF7A4fYXFRK +pgcbWVAfYr1/GqXs9srLLAeYRbFcg6aaTD63uTe0rf/Vse5h7c9tmMZ6/Wk/rkwQ +oguzl8rXvXd2mpJdhvT/NDI2AI6jzIDcCb/F21qlyfEk8K5Rlykm8BDoRNx6+wW/ +SuUZfnXJmywovvpRSsEaGTYo/3eERZVdIZqV5nLHVJtcTRzvh3QuCm2JC8S1HNHE +PboWwv1BHZOR9/llB/voip+DJuIGkBVwbN9mDUC+uf7L/y6Xj8kOvI7g3zaJR/ac +t2YjhgL3sxORZp8lIsYlwZYHT39jGkYP4Pi1HoVWH1sKyn+P55nF8f9BrQCmhyFU +t1K+p2AwzuWxYw6YThQYy2SdiGSaZ9axGrwlu1kdh9Vq2YDctXX+yR1Bs3DytK/6 +PzdLSUAJt4HoLCND+lXG8Ii79WWTCqX9UrYWTJN4PhApCnN+/RYft2Bjd8ZGl0E2 +-----END RSA PRIVATE KEY----- diff --git a/spec/support/certificates/sp_public_cert.pem b/spec/support/certificates/sp_public_cert.pem new file mode 100644 index 00000000..02bea35a --- /dev/null +++ b/spec/support/certificates/sp_public_cert.pem @@ -0,0 +1,16 @@ +-----BEGIN CERTIFICATE----- +MIICjTCCAXWgAwIBAgIVAOTwdQ89937l4RyuaJ7Ni1K2YifEMA0GCSqGSIb3DQEB +CwUAMAAwHhcNMjQxMTAxMjIyMzQ4WhcNMzQxMTAxMjIyMzQ4WjAAMIIBIjANBgkq +hkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA1Ddn+u21iDGHHcLLsThAgdAXLnvL8Pqi +aXKu+MoMu2LYu/pHPk6ZuCmecdM6Tsqdc69S1fCJGu45zSbSuQNzZnW+iLItzBYj +WWv5cgQdOTkT6WVUBsA7NU9sKKmCqsU/3DOCXbZ8OYPxnftZGRIZdydsENOG49f6 +195VJ7h3FybXUnqpaw+bO+pepl4UNl6Y6nIKv4eZnAXJ8P7sJX2iHiu5zl2y/eht +rrL9faNdiSdb2wERkcIZKDrnoGlTvxkFd7fr7sIHIIJvxCBvBpBrgSNfQrHtqjsG +STQ4IK1f5Vtwy8jVkuK+SBEzA+Ft6IlWDkTW3oTnFgVjwwOv0NQdNwIDAQABMA0G +CSqGSIb3DQEBCwUAA4IBAQC2nUkoFXCBVvmkJ9iwXnmJwTGPuDFlZh0zVAuDnVCi +DAcb5z+96GV1dPucFGP/0dtSVazHw3bpw1b9sm1844ne11XjU6PFGEkVAbAyA72Z +Vi0t+ksEVlXP044XjSvZLuWnETJU31w8MjUcoi4BUhdlPtgqYd5jWMSKTncpZCmp +zggCKpz6tSpJcBl7kBAudcxP3kCrWNku5V1OW/4Z4A27ISJLE+Gzi2Q+O/lKHydr +ebXtjQpuuCuMnNwWaLRnxQaQWmYdbVvCuXIweoC6pSPffwjbMpU4ZTpHn1UoPqUQ +9JfW50G2IUKQjq9ssjZAKcGsQIRAIwzwWLvNtl62OeoK +-----END CERTIFICATE----- diff --git a/spec/support/security_helpers.rb b/spec/support/security_helpers.rb index f6952ce7..93b57391 100644 --- a/spec/support/security_helpers.rb +++ b/spec/support/security_helpers.rb @@ -68,4 +68,12 @@ def sp_pv_key def sp_x509_cert @sp_x509_cert ||= File.read(File.join(File.dirname(__FILE__), 'certificates', 'sp_x509_cert.crt')) end + + def sp_encrypted_pv_key + @sp_encrypted_pv_key ||= { + sp_encrypted_pv_key: File.read(File.join(File.dirname(__FILE__), 'certificates', 'sp_encrypted_pv_key.pem')), + sp_public_cert: File.read(File.join(File.dirname(__FILE__), 'certificates', 'sp_public_cert.pem')), + pv_key_password: 'dccf0b7614b554fbe16b0f31' + } + end end From ed7e2d23d74979a59cbac71e7a40a8fe38abdf82 Mon Sep 17 00:00:00 2001 From: Taketo Takashima Date: Mon, 2 Feb 2026 19:32:38 +0900 Subject: [PATCH 17/21] CI: Add Ruby 4.0 and Rails 8.1 to CI Matrix (#243) Fix a load error for the benchmark gem on Rails 6.1 with Ruby 4.0 Remove Ruby 3.2 from CI Matrix following rails-dev (Rails 8.2) support drop Bump actions/checkout from 4 to 6 --- .github/workflows/ci.yml | 23 +++++++++++++++++++---- Appraisals | 11 ++++++----- gemfiles/rails_6.1.gemfile | 3 ++- gemfiles/rails_7.0.gemfile | 1 - gemfiles/rails_7.1.gemfile | 1 - gemfiles/rails_7.2.gemfile | 1 - gemfiles/rails_8.0.gemfile | 1 - gemfiles/rails_8.1.gemfile | 7 +++++++ 8 files changed, 34 insertions(+), 14 deletions(-) create mode 100644 gemfiles/rails_8.1.gemfile diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 09c5c060..d2683073 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -10,10 +10,10 @@ jobs: strategy: fail-fast: false matrix: - ruby: ['2.5', '2.6', '2.7', '3.0', '3.1', '3.2', '3.3', '3.4'] - gemfile: [rails_5.2.gemfile, rails_6.1.gemfile, rails_7.0.gemfile, rails_7.1.gemfile, rails_7.2.gemfile, rails_8.0.gemfile, rails_dev.gemfile] + ruby: ['2.5', '2.6', '2.7', '3.0', '3.1', '3.2', '3.3', '3.4', '4.0'] + gemfile: [rails_5.2.gemfile, rails_6.1.gemfile, rails_7.0.gemfile, rails_7.1.gemfile, rails_7.2.gemfile, rails_8.0.gemfile, rails_8.1.gemfile, rails_dev.gemfile] exclude: - # Ruby 3.2 is min version for Rails 8 + # Ruby 3.3 is min version for Rails 8.2 - ruby: '2.5' gemfile: rails_dev.gemfile - ruby: '2.6' @@ -24,6 +24,19 @@ jobs: gemfile: rails_dev.gemfile - ruby: '3.1' gemfile: rails_dev.gemfile + - ruby: '3.2' + gemfile: rails_dev.gemfile + # Ruby 3.2 is min version for Rails 8.1 + - ruby: '2.5' + gemfile: rails_8.1.gemfile + - ruby: '2.6' + gemfile: rails_8.1.gemfile + - ruby: '2.7' + gemfile: rails_8.1.gemfile + - ruby: '3.0' + gemfile: rails_8.1.gemfile + - ruby: '3.1' + gemfile: rails_8.1.gemfile # Ruby 3.2 is min version for Rails 8.0 - ruby: '2.5' gemfile: rails_8.0.gemfile @@ -65,11 +78,13 @@ jobs: gemfile: rails_5.2.gemfile - ruby: '3.4' gemfile: rails_5.2.gemfile + - ruby: '4.0' + gemfile: rails_5.2.gemfile runs-on: ubuntu-latest env: BUNDLE_GEMFILE: gemfiles/${{ matrix.gemfile }} steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v6 - uses: ruby/setup-ruby@v1 with: ruby-version: ${{ matrix.ruby }} diff --git a/Appraisals b/Appraisals index 60709a28..7d6715b3 100644 --- a/Appraisals +++ b/Appraisals @@ -5,15 +5,15 @@ end appraise 'rails-6.1' do gem 'rails', '~> 6.1.0' - gem 'activeresource', '~> 5.1.0' + gem 'activeresource', '>= 5.1.0' gem 'bigdecimal' gem 'mutex_m' + gem 'benchmark' gem 'concurrent-ruby', '1.3.4' end appraise 'rails-7.0' do gem 'rails', '~> 7.0.0' - gem 'activeresource', '~> 6.0.0' gem 'bigdecimal' gem 'mutex_m' gem 'concurrent-ruby', '1.3.4' @@ -21,17 +21,18 @@ end appraise 'rails-7.1' do gem 'rails', '~> 7.1.0' - gem 'activeresource', '~> 6.0.0' end appraise 'rails-7.2' do gem 'rails', '~> 7.2.0' - gem 'activeresource', '~> 6.0.0' end appraise 'rails-8.0' do gem 'rails', '~> 8.0.0' - gem 'activeresource', '~> 6.0.0' +end + +appraise 'rails-8.1' do + gem 'rails', '~> 8.1.0' end appraise 'rails-dev' do diff --git a/gemfiles/rails_6.1.gemfile b/gemfiles/rails_6.1.gemfile index 4df3cadb..51d00079 100644 --- a/gemfiles/rails_6.1.gemfile +++ b/gemfiles/rails_6.1.gemfile @@ -3,9 +3,10 @@ source "https://rubygems.org" gem "rails", "~> 6.1.0" -gem "activeresource", "~> 5.1.0" +gem "activeresource", ">= 5.1.0" gem "bigdecimal" gem "mutex_m" +gem "benchmark" gem "concurrent-ruby", "1.3.4" gemspec path: "../" diff --git a/gemfiles/rails_7.0.gemfile b/gemfiles/rails_7.0.gemfile index e8641e3a..f4152aa2 100644 --- a/gemfiles/rails_7.0.gemfile +++ b/gemfiles/rails_7.0.gemfile @@ -3,7 +3,6 @@ source "https://rubygems.org" gem "rails", "~> 7.0.0" -gem "activeresource", "~> 6.0.0" gem "bigdecimal" gem "mutex_m" gem "concurrent-ruby", "1.3.4" diff --git a/gemfiles/rails_7.1.gemfile b/gemfiles/rails_7.1.gemfile index f1e307d8..35a0ba34 100644 --- a/gemfiles/rails_7.1.gemfile +++ b/gemfiles/rails_7.1.gemfile @@ -3,6 +3,5 @@ source "https://rubygems.org" gem "rails", "~> 7.1.0" -gem "activeresource", "~> 6.0.0" gemspec path: "../" diff --git a/gemfiles/rails_7.2.gemfile b/gemfiles/rails_7.2.gemfile index 969d680b..92cc2b2d 100644 --- a/gemfiles/rails_7.2.gemfile +++ b/gemfiles/rails_7.2.gemfile @@ -3,6 +3,5 @@ source "https://rubygems.org" gem "rails", "~> 7.2.0" -gem "activeresource", "~> 6.0.0" gemspec path: "../" diff --git a/gemfiles/rails_8.0.gemfile b/gemfiles/rails_8.0.gemfile index a3f12296..3b3765bd 100644 --- a/gemfiles/rails_8.0.gemfile +++ b/gemfiles/rails_8.0.gemfile @@ -3,6 +3,5 @@ source "https://rubygems.org" gem "rails", "~> 8.0.0" -gem "activeresource", "~> 6.0.0" gemspec path: "../" diff --git a/gemfiles/rails_8.1.gemfile b/gemfiles/rails_8.1.gemfile new file mode 100644 index 00000000..cedb65f8 --- /dev/null +++ b/gemfiles/rails_8.1.gemfile @@ -0,0 +1,7 @@ +# This file was generated by Appraisal + +source "https://rubygems.org" + +gem "rails", "~> 8.1.0" + +gemspec path: "../" From 4dde1a3cb9b740c48d5c17154df3e58e3b43de5b Mon Sep 17 00:00:00 2001 From: tyage Date: Mon, 2 Feb 2026 19:38:03 +0900 Subject: [PATCH 18/21] Fix CSRF token leakage in SAML POST binding template (#241) Co-authored-by: zogoo --- spec/rails_app/app/views/saml_idp/idp/saml_post.html.erb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/rails_app/app/views/saml_idp/idp/saml_post.html.erb b/spec/rails_app/app/views/saml_idp/idp/saml_post.html.erb index f79890c8..5c0c3d0d 100644 --- a/spec/rails_app/app/views/saml_idp/idp/saml_post.html.erb +++ b/spec/rails_app/app/views/saml_idp/idp/saml_post.html.erb @@ -5,10 +5,10 @@ - <%= form_tag(saml_acs_url) do %> +

<%= hidden_field_tag("SAMLResponse", @saml_response) %> <%= hidden_field_tag("RelayState", params[:RelayState]) %> <%= submit_tag "Submit" %> - <% end %> +
\ No newline at end of file From 8bdcfe2120006933b405c5e0fba5c29c29c984d9 Mon Sep 17 00:00:00 2001 From: Chris Born Date: Mon, 2 Feb 2026 10:04:43 -0800 Subject: [PATCH 19/21] Basic patch of our assertion extension feature for upstream introduction - Introduced `assertion_extension` attribute in `AssertionBuilder`, `SamlResponse`, and `Controller`. - Updated initialization methods to accept `assertion_extension` as an option. - Enhanced the `build` methods to utilize `assertion_extension` for customizing subject confirmation data and authentication context. - Ensured backward compatibility by maintaining existing functionality when no extension is provided. --- lib/saml_idp/assertion_builder.rb | 16 +++++-- lib/saml_idp/assertion_extension.rb | 32 +++++++++++++ lib/saml_idp/controller.rb | 4 +- lib/saml_idp/saml_response.rb | 8 +++- spec/lib/saml_idp/assertion_extension_spec.rb | 47 +++++++++++++++++++ 5 files changed, 101 insertions(+), 6 deletions(-) create mode 100644 lib/saml_idp/assertion_extension.rb create mode 100644 spec/lib/saml_idp/assertion_extension_spec.rb diff --git a/lib/saml_idp/assertion_builder.rb b/lib/saml_idp/assertion_builder.rb index 482ab16a..a713ed88 100644 --- a/lib/saml_idp/assertion_builder.rb +++ b/lib/saml_idp/assertion_builder.rb @@ -1,6 +1,7 @@ require 'builder' require 'saml_idp/algorithmable' require 'saml_idp/signable' +require 'saml_idp/assertion_extension' module SamlIdp class AssertionBuilder include Algorithmable @@ -21,6 +22,7 @@ class AssertionBuilder attr_accessor :public_cert attr_accessor :private_key attr_accessor :pv_key_password + attr_accessor :assertion_extension delegate :config, to: :SamlIdp @@ -40,7 +42,8 @@ def initialize( encryption_opts: nil, session_expiry: nil, name_id_formats_opts: nil, - asserted_attributes_opts: nil + asserted_attributes_opts: nil, + assertion_extension: nil ) self.reference_id = reference_id self.issuer_uri = issuer_uri @@ -58,6 +61,7 @@ def initialize( self.public_cert = public_cert self.private_key = private_key self.pv_key_password = pv_key_password + self.assertion_extension = assertion_extension end def encrypt(opts = {}) @@ -83,8 +87,11 @@ def fresh confirmation_hash[:InResponseTo] = saml_request_id unless saml_request_id.nil? confirmation_hash[:NotOnOrAfter] = not_on_or_after_subject confirmation_hash[:Recipient] = saml_acs_url - - confirmation.SubjectConfirmationData "", confirmation_hash + if assertion_extension.present? && assertion_extension.extension_point == AssertionExtension::SUBJECT_CONFIRMATION_DATA_EXTENSION_POINT + assertion_extension.build confirmation + else + confirmation.SubjectConfirmationData "", confirmation_hash + end end end assertion.Conditions NotBefore: not_before, NotOnOrAfter: not_on_or_after_condition do |conditions| @@ -102,6 +109,9 @@ def fresh assertion.AuthnStatement authn_statement_props do |statement| statement.AuthnContext do |context| context.AuthnContextClassRef authn_context_classref + if assertion_extension.present? && assertion_extension.extension_point == AssertionExtension::AUTHN_CONTEXT_DECL_EXTENSION_POINT + assertion_extension.build context + end end end if asserted_attributes diff --git a/lib/saml_idp/assertion_extension.rb b/lib/saml_idp/assertion_extension.rb new file mode 100644 index 00000000..b058e1b9 --- /dev/null +++ b/lib/saml_idp/assertion_extension.rb @@ -0,0 +1,32 @@ +require "builder" + +module SamlIdp + class AssertionExtension + SUBJECT_CONFIRMATION_DATA_EXTENSION_POINT = "SubjectConfirmationData" + AUTHN_CONTEXT_DECL_EXTENSION_POINT = "AuthnContextDecl" + + attr_accessor :extension_point + + def initialize(extension_point) + self.extension_point = extension_point + end + + # This is an abstract base class, an example extension may look like this: + # + # context.AuthnContextDecl do |builder| + # builder.AuthenticationContextDeclaration xmlns: "urn:my_org:saml:2.0:Device" do |context| + # context.Extension do |ext| + # ext.Device xmlns: "urn:my_org:saml:2.0:ZeroTrust", ID: "f659c992-2b3d-4e2d-a155-6d32161e6754" do |device| + # device.Trust do |trust| + # trust.Data Name: "Managed", Value: true + # trust.Data Name: "Compliant", Value: true + # end + # end + # end + # end + # end + def build(context) + raise "#{self.class} must implement build method" + end + end +end diff --git a/lib/saml_idp/controller.rb b/lib/saml_idp/controller.rb index 283aff38..8bf10729 100644 --- a/lib/saml_idp/controller.rb +++ b/lib/saml_idp/controller.rb @@ -71,6 +71,7 @@ def encode_authn_response(principal, opts = {}) asserted_attributes_opts = opts[:attributes] || nil signed_assertion_opts = opts[:signed_assertion].nil? ? true : opts[:signed_assertion] compress_opts = opts[:compress] || false + assertion_extension = opts[:assertion_extension] || nil SamlResponse.new( reference_id: reference_id, @@ -92,7 +93,8 @@ def encode_authn_response(principal, opts = {}) asserted_attributes_opts: asserted_attributes_opts, signed_message_opts: signed_message_opts, signed_assertion_opts: signed_assertion_opts, - compression_opts: compress_opts + compression_opts: compress_opts, + assertion_extension: assertion_extension ).build end diff --git a/lib/saml_idp/saml_response.rb b/lib/saml_idp/saml_response.rb index ecda5cfe..b8495bc9 100644 --- a/lib/saml_idp/saml_response.rb +++ b/lib/saml_idp/saml_response.rb @@ -24,6 +24,7 @@ class SamlResponse attr_accessor :signed_message_opts attr_accessor :signed_assertion_opts attr_accessor :compression_opts + attr_accessor :assertion_extension def initialize( reference_id:, @@ -45,7 +46,8 @@ def initialize( asserted_attributes_opts: nil, signed_message_opts: false, signed_assertion_opts: true, - compression_opts: false + compression_opts: false, + assertion_extension: nil ) self.reference_id = reference_id @@ -70,6 +72,7 @@ def initialize( self.name_id_formats_opts = name_id_formats_opts self.asserted_attributes_opts = asserted_attributes_opts self.compression_opts = compression_opts + self.assertion_extension = assertion_extension end def build @@ -128,7 +131,8 @@ def assertion_builder encryption_opts: encryption_opts, session_expiry: session_expiry, name_id_formats_opts: name_id_formats_opts, - asserted_attributes_opts: asserted_attributes_opts + asserted_attributes_opts: asserted_attributes_opts, + assertion_extension: assertion_extension ) end end diff --git a/spec/lib/saml_idp/assertion_extension_spec.rb b/spec/lib/saml_idp/assertion_extension_spec.rb new file mode 100644 index 00000000..04a29254 --- /dev/null +++ b/spec/lib/saml_idp/assertion_extension_spec.rb @@ -0,0 +1,47 @@ +require 'spec_helper' + +module SamlIdp + describe AssertionExtension do + describe "extension point constants" do + it "defines SUBJECT_CONFIRMATION_DATA_EXTENSION_POINT" do + expect(described_class::SUBJECT_CONFIRMATION_DATA_EXTENSION_POINT).to eq("SubjectConfirmationData") + end + + it "defines AUTHN_CONTEXT_DECL_EXTENSION_POINT" do + expect(described_class::AUTHN_CONTEXT_DECL_EXTENSION_POINT).to eq("AuthnContextDecl") + end + end + + describe "#initialize" do + it "sets extension_point" do + extension = described_class.new(described_class::AUTHN_CONTEXT_DECL_EXTENSION_POINT) + expect(extension.extension_point).to eq("AuthnContextDecl") + end + end + + describe "#build" do + it "raises when not overridden" do + extension = described_class.new(described_class::AUTHN_CONTEXT_DECL_EXTENSION_POINT) + context = double("builder context") + expect { extension.build(context) }.to raise_error(/#{described_class} must implement build method/) + end + end + + describe "subclass implementing build" do + let(:extension_class) do + Class.new(described_class) do + def build(context) + context.CustomElement "custom_value" + end + end + end + + it "invokes build with the builder context" do + extension = extension_class.new(described_class::AUTHN_CONTEXT_DECL_EXTENSION_POINT) + context = double("builder context") + expect(context).to receive(:CustomElement).with("custom_value") + extension.build(context) + end + end + end +end From a09cc251b5834619e527b06efd43b6a954dd467d Mon Sep 17 00:00:00 2001 From: Chris Born Date: Mon, 2 Feb 2026 10:10:25 -0800 Subject: [PATCH 20/21] Add documentation for AssertionExtension class - Enhanced comments in the AssertionExtension class to clarify its purpose and usage. - Provided details on the expected implementation of subclasses and alignment with SAML 2.0 specifications. - Added references to external documentation for further specification analysis. --- lib/saml_idp/assertion_extension.rb | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/lib/saml_idp/assertion_extension.rb b/lib/saml_idp/assertion_extension.rb index b058e1b9..0f4a70df 100644 --- a/lib/saml_idp/assertion_extension.rb +++ b/lib/saml_idp/assertion_extension.rb @@ -1,6 +1,10 @@ require "builder" module SamlIdp + # Base class for extending the SAML 2.0 assertion at defined extension points. + # Aligns with OASIS SAML 2.0 Core: SubjectConfirmationData (2.4.1.2) allows arbitrary + # elements/attributes; AuthnContext (2.7.2.2) allows AuthnContextDecl by value. + # See docs/assertion_extension_saml_spec_analysis.md for spec analysis. class AssertionExtension SUBJECT_CONFIRMATION_DATA_EXTENSION_POINT = "SubjectConfirmationData" AUTHN_CONTEXT_DECL_EXTENSION_POINT = "AuthnContextDecl" @@ -11,7 +15,13 @@ def initialize(extension_point) self.extension_point = extension_point end - # This is an abstract base class, an example extension may look like this: + # Subclasses must implement build(context). The context is a Builder block for + # either SubjectConfirmation (for SubjectConfirmationData) or AuthnContext (for AuthnContextDecl). + # + # For SUBJECT_CONFIRMATION_DATA: emit SubjectConfirmationData with standard attributes + # (NotOnOrAfter, Recipient, InResponseTo when using bearer) and add custom elements inside it. + # + # Example (AuthnContextDecl extension): # # context.AuthnContextDecl do |builder| # builder.AuthenticationContextDeclaration xmlns: "urn:my_org:saml:2.0:Device" do |context| From 4e13a916e8b5ff10814e277cb87305861de3eabe Mon Sep 17 00:00:00 2001 From: pelted Date: Thu, 23 Apr 2026 10:05:27 -0700 Subject: [PATCH 21/21] Make SubjectConfirmationData extension additive instead of replacing The SubjectConfirmationData extension point previously replaced the entire element, requiring the extension to reimplement standard attributes (Recipient, InResponseTo, NotOnOrAfter). This was inconsistent with the AuthnContextDecl extension point which is additive. Now both extension points behave the same way: standard SAML elements are always emitted, and the extension adds custom content inside them. The SubjectConfirmationData element is always rendered with its standard attributes, and the extension receives the builder to add child elements. --- lib/saml_idp/assertion_builder.rb | 8 ++++---- lib/saml_idp/assertion_extension.rb | 15 +++++++++++---- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/lib/saml_idp/assertion_builder.rb b/lib/saml_idp/assertion_builder.rb index a713ed88..862e374f 100644 --- a/lib/saml_idp/assertion_builder.rb +++ b/lib/saml_idp/assertion_builder.rb @@ -87,10 +87,10 @@ def fresh confirmation_hash[:InResponseTo] = saml_request_id unless saml_request_id.nil? confirmation_hash[:NotOnOrAfter] = not_on_or_after_subject confirmation_hash[:Recipient] = saml_acs_url - if assertion_extension.present? && assertion_extension.extension_point == AssertionExtension::SUBJECT_CONFIRMATION_DATA_EXTENSION_POINT - assertion_extension.build confirmation - else - confirmation.SubjectConfirmationData "", confirmation_hash + confirmation.SubjectConfirmationData confirmation_hash do |confirmation_data| + if assertion_extension.present? && assertion_extension.extension_point == AssertionExtension::SUBJECT_CONFIRMATION_DATA_EXTENSION_POINT + assertion_extension.build confirmation_data + end end end end diff --git a/lib/saml_idp/assertion_extension.rb b/lib/saml_idp/assertion_extension.rb index 0f4a70df..8797f853 100644 --- a/lib/saml_idp/assertion_extension.rb +++ b/lib/saml_idp/assertion_extension.rb @@ -15,11 +15,18 @@ def initialize(extension_point) self.extension_point = extension_point end - # Subclasses must implement build(context). The context is a Builder block for - # either SubjectConfirmation (for SubjectConfirmationData) or AuthnContext (for AuthnContextDecl). + # Subclasses must implement build(context). Both extension points are additive: + # standard SAML elements are always emitted, and the extension adds content + # inside them. # - # For SUBJECT_CONFIRMATION_DATA: emit SubjectConfirmationData with standard attributes - # (NotOnOrAfter, Recipient, InResponseTo when using bearer) and add custom elements inside it. + # For SUBJECT_CONFIRMATION_DATA: The standard SubjectConfirmationData element + # with NotOnOrAfter, Recipient, and InResponseTo attributes is always emitted. + # The extension receives the SubjectConfirmationData builder to add custom + # child elements inside it. + # + # For AUTHN_CONTEXT_DECL: The standard AuthnContextClassRef is always emitted. + # The extension receives the AuthnContext builder to add AuthnContextDecl or + # other child elements alongside it. # # Example (AuthnContextDecl extension): #