Comparing changes

v1.0.2 v1.0.3
7 commits 10 files changed

Commits

lib/saml/kit/builders/assertion.rb
@@ -6,10 +6,11 @@ module Saml
         include XmlTemplatable
         extend Forwardable
 
-        def_delegators :@response_builder, :embed_signature, :request, :issuer, :reference_id, :now, :configuration, :user, :version, :destination
+        def_delegators :@response_builder, :request, :issuer, :reference_id, :now, :configuration, :user, :version, :destination
 
-        def initialize(response_builder)
+        def initialize(response_builder, embed_signature)
           @response_builder = response_builder
+          self.embed_signature = embed_signature
         end
 
         def name_id_format
lib/saml/kit/builders/response.rb
@@ -35,12 +35,10 @@ module Saml
           nil
         end
 
-        private
-
         def assertion
           @assertion ||=
             begin
-              assertion = Saml::Kit::Builders::Assertion.new(self)
+              assertion = Saml::Kit::Builders::Assertion.new(self, embed_signature)
               if encrypt
                 Saml::Kit::Builders::EncryptedAssertion.new(self, assertion)
               else
@@ -49,6 +47,8 @@ module Saml
             end
         end
 
+        private
+
         def response_options
           {
             ID: id,
lib/saml/kit/metadata.rb
@@ -77,7 +77,9 @@ module Saml
       def certificates
         @certificates ||= document.find_all("/md:EntityDescriptor/md:#{name}/md:KeyDescriptor").map do |item|
           cert = item.at_xpath("./ds:KeyInfo/ds:X509Data/ds:X509Certificate", NAMESPACES).text
-          ::Xml::Kit::Certificate.new(cert, use: item.attribute('use').value.to_sym)
+          attribute = item.attribute('use')
+          use = attribute.nil? ? nil : item.attribute('use').value
+          ::Xml::Kit::Certificate.new(cert, use: use)
         end
       end
 
lib/saml/kit/response.rb
@@ -26,8 +26,6 @@ module Saml
           self.errors[attribute] << error
         end
       end
-
-      Builder = ActiveSupport::Deprecation::DeprecatedConstantProxy.new('Saml::Kit::Response::Builder', 'Saml::Kit::Builders::Response')
     end
   end
 end
lib/saml/kit/version.rb
@@ -1,5 +1,5 @@
 module Saml
   module Kit
-    VERSION = "1.0.2"
+    VERSION = "1.0.3"
   end
 end
lib/saml/kit/xml_templatable.rb
@@ -12,7 +12,8 @@ module Saml
       # Returns true if an embedded signature is requested and at least one signing certificate is available via the configuration.
       def sign?
         return configuration.sign? if embed_signature.nil?
-        embed_signature && configuration.sign?
+        (embed_signature && configuration.sign?) ||
+          (embed_signature && @signing_key_pair.present?)
       end
 
       def digest_method
spec/saml/authentication_request_spec.rb
@@ -143,14 +143,19 @@ RSpec.describe Saml::Kit::AuthenticationRequest do
         certificate
       end
       let(:private_key) { OpenSSL::PKey::RSA.new(2048) }
-      let(:configuration) do
-        Saml::Kit::Configuration.new do |config|
-          config.add_key_pair(expired_certificate, private_key, passphrase: nil, use: :signing)
-        end
+      let(:digest_algorithm) { OpenSSL::Digest::SHA256.new }
+
+      before :each do
+        expired_certificate.sign(private_key, digest_algorithm)
       end
 
       it 'is invalid' do
-        subject = described_class.new(raw_xml, configuration: configuration)
+        document = described_class.build do |x|
+          x.embed_signature = true
+          certificate = ::Xml::Kit::Certificate.new(expired_certificate)
+          x.sign_with(certificate.to_key_pair(private_key))
+        end
+        subject = described_class.new(document.to_xml)
         expect(subject).to be_invalid
         expect(subject.errors[:certificate]).to be_present
       end
spec/saml/metadata_spec.rb
@@ -47,4 +47,19 @@ RSpec.describe Saml::Kit::Metadata do
       expect(result.contact_person_company).to eql("mailto:hi@example.com")
     end
   end
+
+  describe "#certificates" do
+    it 'returns each certificate when missing a "use"' do
+      configuration = Saml::Kit::Configuration.new do |config|
+        config.generate_key_pair_for(use: :signing)
+      end
+      xml = Saml::Kit::Metadata.build_xml(configuration: configuration) do |x|
+        x.embed_signature = false
+        x.build_identity_provider
+      end
+      modified_xml = xml.gsub(/use/, 'misuse')
+      subject = described_class.from(modified_xml)
+      expect(subject.certificates.count).to eql(1)
+    end
+  end
 end
spec/saml/response_spec.rb
@@ -229,6 +229,20 @@ RSpec.describe Saml::Kit::Response do
       expect(subject).to_not be_valid
       expect(subject.errors[:assertion]).to be_present
     end
+
+    it 'is invalid when the assertion has a signature and has been tampered with' do
+      user = double(:user, name_id_for: SecureRandom.uuid, assertion_attributes_for: { token: SecureRandom.uuid })
+      request = Saml::Kit::AuthenticationRequest.build
+      document = described_class.build(user, request, configuration: configuration) do |x|
+        x.embed_signature = false
+        x.assertion.embed_signature = true
+      end
+
+      altered_xml = document.to_xml.gsub(/token/, 'heck')
+      subject = described_class.new(altered_xml)
+      expect(subject).to_not be_valid
+      expect(subject.errors[:digest_value]).to be_present
+    end
   end
 
   describe "#signed?" do
saml-kit.gemspec
@@ -24,7 +24,7 @@ Gem::Specification.new do |spec|
   spec.require_paths = ["lib"]
 
   spec.add_dependency "activemodel", ">= 4.2.0"
-  spec.add_dependency "xml-kit", "~> 0.1"
+  spec.add_dependency "xml-kit", ">= 0.1.5", "<= 1.0.0"
   spec.add_development_dependency "bundler", "~> 1.15"
   spec.add_development_dependency "ffaker", "~> 2.7"
   spec.add_development_dependency "rake", "~> 10.0"