Allow SP config force signature validation

lib/saml_idp/request.rb

@@ -128,13 +128,16 @@ 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
-      require_signature = logout_request? || authn_request? && metadata.respond_to?(:sign_authn_request?) && metadata.sign_authn_request?
-      service_provider.valid_signature?(document, require_signature)
+      if logout_request? || authn_request? && validate_auth_request_signature?
+        document.valid_signature?(service_provider.cert, service_provider.fingerprint)
+      else
+        true
+      end
     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
@@ -229,5 +232,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

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
@@ -23,7 +24,7 @@ def valid?
 
     def valid_signature?(doc, require_signature = false)
       if require_signature || attributes[:validate_signature]
-        doc.valid_signature?(cert, fingerprint)
+        doc.valid_signature?(fingerprint)
       else
         true
       end

spec/lib/saml_idp/request_spec.rb

@@ -119,6 +119,74 @@ def info(msg); end
           end
         end
       end
+
+     context "validate_auth_request_signature?" do
+        let(:auth_request) { OneLogin::RubySaml::Authrequest.new }
+        let(:sp_setting) { saml_settings("https://foo.example.com/saml/consume", true) }
+        let(:raw_authn_request) { CGI.unescape(auth_request.create(sp_setting).split("=").last) }
+
+        subject { described_class.from_deflated_request raw_authn_request }
+
+        before do
+          idp_configure("http://localhost:3000/saml/consume", true)
+        end
+
+        context "when AuthnRequest signature validation is enabled" do
+          before do
+            SamlIdp.configure do |config|
+              config.service_provider.finder = lambda { |_issuer_or_entity_id|
+                {
+                  response_hosts: [URI("http://localhost:3000/saml/consume").host],
+                  acs_url: "http://localhost:3000/saml/consume",
+                  cert: sp_x509_cert,
+                  fingerprint: SamlIdp::Fingerprint.certificate_digest(sp_x509_cert),
+                  assertion_consumer_logout_service_url: 'https://foo.example.com/saml/logout',
+                  sign_authn_request: true
+                }
+              }
+            end
+          end
+
+          it "returns true" do
+            expect(subject.send(:validate_auth_request_signature?)).to be true
+          end
+
+          it "validates the signature" do
+            allow(subject).to receive(:signature).and_return(nil)
+            allow(subject).to receive(:valid_signature?).and_call_original
+
+            expect(subject.valid_signature?).to be true
+          end
+        end
+
+        context "when AuthnRequest signature validation is disabled" do
+          before do
+            SamlIdp.configure do |config|
+              config.service_provider.finder = lambda { |_issuer_or_entity_id|
+                {
+                  response_hosts: [URI("http://localhost:3000/saml/consume").host],
+                  acs_url: "http://localhost:3000/saml/consume",
+                  cert: sp_x509_cert,
+                  fingerprint: SamlIdp::Fingerprint.certificate_digest(sp_x509_cert),
+                  assertion_consumer_logout_service_url: 'https://foo.example.com/saml/logout',
+                  sign_authn_request: false
+                }
+              }
+            end
+          end
+
+          it "returns false" do
+            expect(subject.send(:validate_auth_request_signature?)).to be false
+          end
+
+          it "does not validate the signature and return true" do
+            allow(subject).to receive(:signature).and_return(nil)
+            allow(subject).to receive(:valid_signature?).and_call_original
+
+            expect(subject.valid_signature?).to be true
+          end
+        end
+      end
     end
 
     describe "logout request" do

spec/support/security_helpers.rb

@@ -51,7 +51,7 @@ 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
+  def certificate_1
     @signature1 ||= File.read(File.join(File.dirname(__FILE__), 'certificates', 'certificate1'))
   end