Add tests to service_provider validate method

lib/saml_idp/service_provider.rb

@@ -21,10 +21,8 @@ def valid?
     end
 
     def valid_signature?(doc, require_signature = false, options = {})
-      Array(certs).any? do |cert|
-        if doc.valid_signature?(fingerprint_cert(cert), options.merge(cert: cert))
-          @matching_cert = cert
-        end
+      @matching_cert = Array(certs).find do |cert|
+        doc.valid_signature?(fingerprint_cert(cert), options.merge(cert: cert))
       end
 
       if require_signature || should_validate_signature?

spec/lib/saml_idp/service_provider_spec.rb

@@ -3,6 +3,8 @@ module SamlIdp
   describe ServiceProvider do
     subject { described_class.new attributes }
     let(:attributes) { {} }
+    let(:cert) { saml_settings.get_sp_cert }
+    let(:options) { {} }
 
     it { is_expected.to respond_to :metadata_url }
     it { is_expected.not_to be_valid }
@@ -17,5 +19,145 @@ module SamlIdp
 
       it { is_expected.to be_valid }
     end
+
+    describe "#valid_signature" do
+      let(:raw_xml) do
+        SamlIdp::Request.from_deflated_request(
+          make_saml_request
+        ).raw_xml
+      end
+
+      let(:doc) {  Saml::XML::Document.parse(raw_xml) }
+
+      describe 'the signature is not required' do
+        describe "the service provider has no certs" do
+          it "returns true" do
+            expect(subject.valid_signature?(doc)).to be true
+          end
+        end
+
+        describe "the service provider has certs" do
+          before { subject.certs = [cert] }
+          it "returns true" do
+            expect(subject.valid_signature?(doc)).to be true
+          end
+        end
+      end
+
+      describe "the signature is required" do
+        before { subject.validate_signature = true }
+
+        describe "a cert is not present in the document" do
+          let(:raw_xml) do
+            SamlIdp::Request.from_deflated_request(
+              signed_auth_request_options["SAMLRequest"]
+            ).raw_xml
+          end
+          let(:options) { {get_params: signed_auth_request_options}.with_indifferent_access }
+
+          describe "the service provider has no certs" do
+            it "returns false" do
+              expect(subject.valid_signature?(doc, true, options)).to be false
+            end
+          end
+
+          describe "the service provider has one or more certs" do
+            describe 'one valid cert' do
+              before { subject.certs = [cert] }
+
+              it "returns true" do
+                expect(subject.valid_signature?(doc, true, options)).to be true
+              end
+            end
+
+            describe "one invalid cert" do
+              before { subject.certs = [invalid_cert]}
+
+              it "returns false" do
+                expect(subject.valid_signature?(doc, true, options)).to be false
+              end
+            end
+
+            describe "multiple certs" do
+              before { subject.certs = [invalid_cert, cert] }
+
+              it "returns true" do
+                expect(subject.valid_signature?(doc, true, options)).to be true
+              end
+            end
+          end
+        end
+
+        describe "a cert is present in the document" do
+          let(:raw_xml) do
+            SamlIdp::Request.from_deflated_request(
+              signed_auth_request
+            ).raw_xml
+          end
+
+          describe "the service provider has no certs" do
+            it "returns false" do
+              expect(subject.valid_signature?(doc, true)).to be false
+            end
+
+            describe "the requirement is passed through the method" do
+              it "returns false" do
+                expect(subject.valid_signature?(doc, true)).to be false
+              end
+            end
+          end
+
+          describe "the service provider has one or more certs" do
+            describe 'one valid cert' do
+              before { subject.certs = [cert] }
+
+              it "returns true" do
+                expect(subject.valid_signature?(doc)).to be true
+              end
+            end
+
+            describe "one invalid cert" do
+              before { subject.certs = [invalid_cert]}
+
+              it "returns false" do
+                expect(subject.valid_signature?(doc, true)).to be false
+              end
+            end
+
+            describe "multiple certs" do
+              let(:other_cert) do
+                OpenSSL::X509::Certificate.new(cloudhsm_idp_x509_cert)
+              end
+
+              describe "the valid cert is registered in the idp" do
+                before { subject.certs = [other_cert, invalid_cert, cert] }
+
+                it "returns true" do
+                  expect(subject.valid_signature?(doc, true)).to be true
+                end
+
+                it "matches the right cert" do
+                  subject.valid_signature?(doc)
+                  expect(subject.matching_cert).to eq cert
+                end
+              end
+
+              describe "the valid cert is not registered in the idp" do
+                before { subject.certs = [other_cert, invalid_cert] }
+
+                it "returns false" do
+                  expect(subject.valid_signature?(doc, true)).to be false
+                end
+
+                it "matches the right cert" do
+                  subject.valid_signature?(doc)
+                  expect(subject.matching_cert).to be nil
+                end
+              end
+            end
+          end
+        end
+      end
+    end
   end
 end

spec/support/certificate_helpers.rb

@@ -37,4 +37,8 @@ def encrypted_secret_key
   def encrypted_secret_key_password
     'im a secret password.'
   end
+
+  def invalid_cert
+    OpenSSL::X509::Certificate.new(File.read('spec/support/certificates/too_short_cert.crt'))
+  end
 end

spec/support/certificates/too_short_cert.crt

@@ -0,0 +1,3 @@
+-----BEGIN CERTIFICATE-----
+MIICnTCCAgagAwIBAgIGAWVhrSugMA0GCSqGSIb3DQEBCwUAMIGHMR0wGwYDVQQDDBRzZWN1cmVwb3J0YWwub3BtLmdvdjEnMCUGA1UECgweT2ZmaWNlIG9mIFBlcnNvbm5lbCBNYW5hZ2VtZW50MQ4wDAYDVQQLDAVPQ0lTTzETMBEGA1UEBwwKV2FzaGluZ3RvbjELMAkGA1UECAwCREMxCzAJBgNVBAYTAlVTMB4XDTE4MDgyMjEyNDc0M1oXDTIzMDgyMjA0MDAwMFowgYcxHTAbBgNVBAMMFHNlY3VyZXBvcnRhbC5vcG0uZ292MScwJQYDVQQKDB5PZmZpY2Ugb2YgUGVyc29ubmVsIE1hbmFnZW1lbnQxDjAMBgNVBAsMBU9DSVNPMRMwEQYDVQQHDApXYXNoaW5ndG9uMQswCQYDVQQIDAJEQzELMAkGA1UEBhMCVVMwgZ8wDQYJKoZIhvcNAQEBBQADgY0AMIGJAoGBAIpUzkPbQdNQIRA3/64quioyOIetu4101bWg2NJFFu1Iy/xOAI7+hyGIbnvP1ZwlzB65+0f6yzZfrBy78uMIaPBQycDUXjrGAe9JS6eoJynSZiFvlUs5OOSQmoEZ1MfajH0EDhGV0drWTHFoPCdcckbdTd9TnyZdolVoC/3L93fbAgMBAAGjEjAQMA4GA1UdDwEB/wQEAwIC9DANBgkqhkiG9w0BAQsFAAOBgQBlqriiUBu0iiF+hMHqws5oHY+d6x66rR52XoBgnfIgKuwuQqWU57HCYXw8fho4ykefn+F7fdgAOOHNXjdTYVSlPihsU0pXqEA5NBDyCg5rzyvoLIz9jLDiIrmgZ8Tjog2wRm5aOV02fp4gcnjjBB+vNe8U+lmiJaBhY3AeiA7gSA==
+-----END CERTIFICATE-----

spec/support/saml_request_macros.rb

@@ -3,14 +3,27 @@
 module SamlRequestMacros
 
   def make_saml_request(requested_saml_acs_url = "https://foo.example.com/saml/consume")
-    auth_request = OneLogin::RubySaml::Authrequest.new
-    auth_url = auth_request.create(saml_settings(requested_saml_acs_url))
+    auth_url = url(saml_settings(requested_saml_acs_url))
     CGI.unescape(auth_url.split("=").last)
   end
 
-  def make_invalid_saml_request(requested_saml_acs_url = "https://foo.example.com/saml/consume")
+  def url(saml_settings)
     auth_request = OneLogin::RubySaml::Authrequest.new
-    auth_url = auth_request.create(invalid_saml_settings)
+    auth_request.create(saml_settings)
+  end
+
+  def signed_auth_request
+    CGI.unescape(url(signed_saml_settings).split("=").last)
+  end
+
+  def signed_auth_request_options
+    signed_auth_request_options ||=
+      uri = URI(url(signed_saml_settings(embed: false)))
+      Rack::Utils.parse_nested_query uri.query
+  end
+
+  def make_invalid_saml_request
+    auth_url = url(invalid_saml_settings)
     CGI.unescape(auth_url.split("=").last)
   end
 
@@ -32,9 +45,9 @@ def make_sp_logout_request(requested_saml_logout_url = 'https://foo.example.com/
     OneLogin::RubySaml::Logoutrequest.new.create(settings)
   end
 
-  def saml_settings(saml_acs_url = "https://foo.example.com/saml/consume")
+  def saml_settings(requested_saml_acs_url = "https://foo.example.com/saml/consume")
     settings = OneLogin::RubySaml::Settings.new
-    settings.assertion_consumer_service_url = saml_acs_url
+    settings.assertion_consumer_service_url = requested_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/idp-slo"
@@ -51,7 +64,19 @@ def saml_settings(saml_acs_url = "https://foo.example.com/saml/consume")
     settings
   end
 
-  def invalid_saml_settings(saml_acs_url = "https://foo.example.com/saml/consume")
+  def signed_saml_settings(embed: true)
+    settings = saml_settings("https://foo.example.com/saml/consume")
+    settings.security = {
+      embed_sign: embed,
+      authn_requests_signed: true,
+      want_assertions_signed: true,
+      digest_method: 'http://www.w3.org/2001/04/xmlenc#sha256',
+      signature_method: 'http://www.w3.org/2001/04/xmldsig-more#rsa-sha256'
+    }
+    settings
+  end
+
+  def invalid_saml_settings
     settings = saml_settings.dup
     settings.issuer = ''
     settings