Comparing changes

@@ -21,7 +21,7 @@ module Saml
         private
 
         def signature_for(payload)
-          private_key = configuration.private_keys(use: :signing).sample
+          private_key = configuration.private_keys(use: :signing).last
           encode(private_key.sign(OpenSSL::Digest::SHA256.new, payload))
         end
 

@@ -24,7 +24,7 @@ module Saml
         def initialize(reference_id, configuration:)
           @configuration = configuration
           @reference_id = reference_id
-          @x509_certificate = configuration.certificates(use: :signing).sample.stripped
+          @x509_certificate = configuration.certificates(use: :signing).last.stripped
         end
 
         def signature_method

@@ -2,6 +2,8 @@
 en:
   saml/kit:
     errors:
+      Assertion:
+        expired: "must not be expired."
       AuthnRequest:
         invalid: "must contain AuthnRequest."
         invalid_fingerprint: "does not match."
@@ -17,7 +19,6 @@ en:
       LogoutResponse:
         unregistered: "is unregistered."
       Response:
-        expired: "must not be expired."
         invalid: "must contain Response."
         invalid_fingerprint: "does not match."
         invalid_response_to: "must match request id."

@@ -1,7 +1,15 @@
 module Saml
   module Kit
     class Assertion
+      include ActiveModel::Validations
+      include Translatable
+
+      validate :must_match_issuer
+      validate :must_be_active_session
+      attr_reader :name
+
       def initialize(xml_hash, configuration:)
+        @name = "Assertion"
         @xml_hash = xml_hash
         @configuration = configuration
       end
@@ -11,7 +19,20 @@ module Saml
       end
 
       def signed?
-        assertion.fetch('Signature', nil).present?
+        signature.present?
+      end
+
+      def signature
+        xml_hash = assertion.fetch('Signature', nil)
+        xml_hash ? Signature.new(xml_hash) : nil
+      end
+
+      def expired?
+        Time.current > expired_at
+      end
+
+      def active?
+        Time.current > started_at && !expired?
       end
 
       def attributes
@@ -35,10 +56,6 @@ module Saml
         parse_date(assertion.fetch('Conditions', {}).fetch('NotOnOrAfter', nil))
       end
 
-      def certificate
-        assertion.fetch('Signature', {}).fetch('KeyInfo', {}).fetch('X509Data', {}).fetch('X509Certificate', nil)
-      end
-
       def audiences
         Array(assertion['Conditions']['AudienceRestriction']['Audience'])
       rescue => error
@@ -68,6 +85,17 @@ module Saml
         Saml::Kit.logger.error(error)
         Time.at(0).to_datetime
       end
+
+      def must_match_issuer
+        unless audiences.include?(@configuration.issuer)
+          errors[:audience] << error_message(:must_match_issuer)
+        end
+      end
+
+      def must_be_active_session
+        return if active?
+        errors[:base] << error_message(:expired)
+      end
     end
   end
 end

@@ -8,6 +8,10 @@ module Saml
           builder(*args, &block).build
         end
 
+        def build_xml(*args, &block)
+          builder(*args, &block).to_xml
+        end
+
         def builder(*args)
           builder_class.new(*args).tap do |builder|
             yield builder if block_given?

@@ -19,11 +19,11 @@ module Saml
       end
 
       def encryption?
-        :encryption == use
+        for?(:encryption)
       end
 
       def signing?
-        :signing == use
+        for?(:signing)
       end
 
       def x509

@@ -2,8 +2,9 @@ module Saml
   module Kit
     class Document
       PROTOCOL_XSD = File.expand_path("./xsd/saml-schema-protocol-2.0.xsd", File.dirname(__FILE__)).freeze
-      include XsdValidatable
       include ActiveModel::Validations
+      include XsdValidatable
+      include Translatable
       include Trustable
       include Buildable
       validates_presence_of :content

@@ -1,10 +1,11 @@
 module Saml
   module Kit
     class Metadata
+      METADATA_XSD = File.expand_path("./xsd/saml-schema-metadata-2.0.xsd", File.dirname(__FILE__)).freeze
       include ActiveModel::Validations
       include XsdValidatable
+      include Translatable
       include Buildable
-      METADATA_XSD = File.expand_path("./xsd/saml-schema-metadata-2.0.xsd", File.dirname(__FILE__)).freeze
 
       validates_presence_of :metadata
       validate :must_contain_descriptor

@@ -4,50 +4,25 @@ module Saml
       include Respondable
       extend Forwardable
 
-      def_delegators :assertion, :name_id, :[], :attributes, :started_at, :expired_at, :audiences
+      def_delegators :assertion, :name_id, :[], :attributes
 
-      validate :must_be_active_session
-      validate :must_match_issuer
+      validate :must_be_valid_assertion
 
       def initialize(xml, request_id: nil, configuration: Saml::Kit.configuration)
         @request_id = request_id
         super(xml, name: "Response", configuration: configuration)
       end
 
-      def expired?
-        Time.current > expired_at
-      end
-
-      def active?
-        Time.current > started_at && !expired?
-      end
-
       def assertion
-        @assertion = Saml::Kit::Assertion.new(to_h, configuration: @configuration)
-      end
-
-      def signed?
-        super || assertion.signed?
-      end
-
-      def certificate
-        super || assertion.certificate
+        @assertion ||= Saml::Kit::Assertion.new(to_h, configuration: @configuration)
       end
 
       private
 
-      def must_be_active_session
-        return unless expected_type?
-        return unless success?
-        errors[:base] << error_message(:expired) unless active?
-      end
-
-      def must_match_issuer
-        return unless expected_type?
-        return unless success?
-
-        unless audiences.include?(configuration.issuer)
-          errors[:audience] << error_message(:must_match_issuer)
+      def must_be_valid_assertion
+        assertion.valid?
+        assertion.errors.each do |attribute, error|
+          self.errors[attribute] << error
         end
       end
 

@@ -1,22 +1,23 @@
 module Saml
   module Kit
     class Signature
-      attr_reader :signatures
-      attr_reader :xml
+      def initialize(xml_hash)
+        @xml_hash = xml_hash
+      end
 
-      def initialize(xml, signatures)
-        @signatures = signatures
-        @xml = xml
+      def certificate
+        value = to_h.fetch('KeyInfo', {}).fetch('X509Data', {}).fetch('X509Certificate', nil)
+        return if value.nil?
+        Saml::Kit::Certificate.new(value, use: :signing)
       end
 
-      def template(reference_id)
-        Template.new(signatures.build(reference_id)).to_xml(xml: xml)
+      def trusted?(metadata)
+        return false if metadata.nil?
+        metadata.matches?(certificate.fingerprint, use: :signing)
       end
 
-      def self.sign(xml: ::Builder::XmlMarkup.new, configuration: Saml::Kit.configuration)
-        signatures = Saml::Kit::Signatures.new(configuration: configuration)
-        yield xml, new(xml, signatures)
-        signatures.complete(xml.target!)
+      def to_h
+        @xml_hash
       end
     end
   end

@@ -14,9 +14,28 @@ module Saml
 
       def complete(raw_xml)
         return raw_xml unless configuration.sign?
-        private_key = configuration.private_keys(use: :signing).sample
+        private_key = configuration.private_keys(use: :signing).last
         Xmldsig::SignedDocument.new(raw_xml).sign(private_key)
       end
+
+      def self.sign(xml: ::Builder::XmlMarkup.new, configuration: Saml::Kit.configuration)
+        signatures = Saml::Kit::Signatures.new(configuration: configuration)
+        yield xml, XmlSignatureTemplate.new(xml, signatures)
+        signatures.complete(xml.target!)
+      end
+
+      class XmlSignatureTemplate
+        attr_reader :signatures, :xml
+
+        def initialize(xml, signatures)
+          @signatures = signatures
+          @xml = xml
+        end
+
+        def template(reference_id)
+          Template.new(signatures.build(reference_id)).to_xml(xml: xml)
+        end
+      end
     end
   end
 end

@@ -21,7 +21,7 @@ module Saml
       end
 
       def encryption_for(xml:)
-        if encrypt && encryption_certificate
+        if encrypt?
           temp = ::Builder::XmlMarkup.new
           yield temp
           xml_encryption = Saml::Kit::Builders::XmlEncryption.new(temp.target!, encryption_certificate.public_key)
@@ -31,6 +31,10 @@ module Saml
         end
       end
 
+      def encrypt?
+        encrypt && encryption_certificate
+      end
+
       def render(model, options)
         Saml::Kit::Template.new(model).to_xml(options)
       end

@@ -0,0 +1,9 @@
+module Saml
+  module Kit
+    module Translatable
+      def error_message(attribute, type: :invalid)
+        I18n.translate(attribute, scope: "saml/kit.errors.#{name}")
+      end
+    end
+  end
+end

@@ -9,24 +9,18 @@ module Saml
         validate :must_be_trusted, unless: :signature_manually_verified
       end
 
-      def certificate
-        return unless signed?
-        to_h.fetch(name, {}).fetch('Signature', {}).fetch('KeyInfo', {}).fetch('X509Data', {}).fetch('X509Certificate', nil)
-      end
-
-      def fingerprint
-        return if certificate.blank?
-        Fingerprint.new(certificate)
+      def signed?
+        signature.present?
       end
 
-      def signed?
-        to_h.fetch(name, {}).fetch('Signature', nil).present?
+      def signature
+        xml_hash = to_h.fetch(name, {}).fetch('Signature', nil)
+        xml_hash ? Signature.new(xml_hash) : nil
       end
 
       def trusted?
-        return false if provider.nil?
         return false unless signed?
-        provider.matches?(fingerprint, use: :signing)
+        signature.trusted?(provider)
       end
 
       def provider
@@ -59,6 +53,7 @@ module Saml
 
       def must_be_trusted
         return if trusted?
+        return if provider.present? && !signed?
         errors[:fingerprint] << error_message(:invalid_fingerprint)
       end
     end

@@ -1,5 +1,5 @@
 module Saml
   module Kit
-    VERSION = "0.2.4"
+    VERSION = "0.2.5"
   end
 end

@@ -4,7 +4,7 @@ module Saml
       attr_reader :private_key
 
       def initialize(configuration: Saml::Kit.configuration)
-        @private_key = configuration.private_keys(use: :encryption).sample
+        @private_key = configuration.private_keys(use: :encryption).last
       end
 
       def decrypt(data)

@@ -10,10 +10,6 @@ module Saml
           end
         end
       end
-
-      def error_message(key)
-        I18n.translate(key, scope: "saml/kit.errors.#{name}")
-      end
     end
   end
 end

@@ -24,6 +24,7 @@ require "saml/kit/xsd_validatable"
 require "saml/kit/respondable"
 require "saml/kit/requestable"
 require "saml/kit/trustable"
+require "saml/kit/translatable"
 require "saml/kit/document"
 
 require "saml/kit/assertion"

@@ -61,7 +61,7 @@ RSpec.describe Saml::Kit::Bindings::UrlBuilder do
           payload = "#{query_string_parameter}=#{query_params[query_string_parameter]}"
           payload << "&RelayState=#{query_params['RelayState']}"
           payload << "&SigAlg=#{query_params['SigAlg']}"
-          private_key = configuration.private_keys(use: :signing).sample
+          private_key = configuration.private_keys(use: :signing).last
           expected_signature = Base64.strict_encode64(private_key.sign(OpenSSL::Digest::SHA256.new, payload))
           expect(query_params['Signature']).to eql(expected_signature)
         end
@@ -73,7 +73,7 @@ RSpec.describe Saml::Kit::Bindings::UrlBuilder do
 
           payload = "#{query_string_parameter}=#{query_params[query_string_parameter]}"
           payload << "&SigAlg=#{query_params['SigAlg']}"
-          private_key = configuration.private_keys(use: :signing).sample
+          private_key = configuration.private_keys(use: :signing).last
           expected_signature = Base64.strict_encode64(private_key.sign(OpenSSL::Digest::SHA256.new, payload))
           expect(query_params['Signature']).to eql(expected_signature)
         end

@@ -8,14 +8,14 @@ RSpec.describe Saml::Kit::AuthenticationRequest do
   let(:destination) { FFaker::Internet.http_url }
   let(:name_id_format) { Saml::Kit::Namespaces::EMAIL_ADDRESS }
   let(:raw_xml) do
-    described_class.build(configuration: configuration) do |builder|
+    described_class.build_xml(configuration: configuration) do |builder|
       builder.id = id
       builder.now = Time.now.utc
       builder.issuer = issuer
       builder.assertion_consumer_service_url = assertion_consumer_service_url
       builder.name_id_format = name_id_format
       builder.destination = destination
-    end.to_xml
+    end
   end
   let(:configuration) do
     Saml::Kit::Configuration.new do |config|
@@ -36,7 +36,6 @@ RSpec.describe Saml::Kit::AuthenticationRequest do
     before :each do
       allow(configuration).to receive(:registry).and_return(registry)
       allow(registry).to receive(:metadata_for).and_return(metadata)
-      #allow(metadata).to receive(:matches?).and_return(true)
     end
 
     it 'is valid when left untampered' do
@@ -85,7 +84,7 @@ RSpec.describe Saml::Kit::AuthenticationRequest do
       id = Saml::Kit::Id.generate
       configuration = Saml::Kit::Configuration.new
       configuration.generate_key_pair_for(use: :signing)
-      signed_xml = Saml::Kit::Signature.sign(configuration: configuration) do |xml, signature|
+      signed_xml = Saml::Kit::Signatures.sign(configuration: configuration) do |xml, signature|
         xml.tag!('samlp:AuthnRequest', "xmlns:samlp" => Saml::Kit::Namespaces::PROTOCOL, AssertionConsumerServiceURL: assertion_consumer_service_url, ID: id) do
           signature.template(id)
           xml.Fake do
@@ -109,6 +108,34 @@ RSpec.describe Saml::Kit::AuthenticationRequest do
       subject.signature_verified!
       expect(subject).to be_valid
     end
+
+    it 'is valid when there is no signature, and the issuer is registered' do
+      now = Time.now.utc
+      raw_xml = <<-XML
+<samlp:AuthnRequest AssertionConsumerServiceURL='#{assertion_consumer_service_url}' ID='#{Saml::Kit::Id.generate}' IssueInstant='#{now.iso8601}' Version='2.0' xmlns:saml='#{Saml::Kit::Namespaces::ASSERTION}' xmlns:samlp='#{Saml::Kit::Namespaces::PROTOCOL}'>
+  <saml:Issuer>#{issuer}</saml:Issuer>
+  <samlp:NameIDPolicy AllowCreate='true' Format='#{Saml::Kit::Namespaces::PERSISTENT}'/>
+</samlp:AuthnRequest>
+      XML
+
+      allow(registry).to receive(:metadata_for).with(issuer).and_return(metadata)
+      subject = described_class.new(raw_xml, configuration: configuration)
+      expect(subject).to be_valid
+    end
+
+    it 'is invalid when there is no signature, and the issuer is not registered' do
+      now = Time.now.utc
+      raw_xml = <<-XML
+<samlp:AuthnRequest AssertionConsumerServiceURL='#{assertion_consumer_service_url}' ID='#{Saml::Kit::Id.generate}' IssueInstant='#{now.iso8601}' Version='2.0' xmlns:saml='#{Saml::Kit::Namespaces::ASSERTION}' xmlns:samlp='#{Saml::Kit::Namespaces::PROTOCOL}'>
+  <saml:Issuer>#{issuer}</saml:Issuer>
+  <samlp:NameIDPolicy AllowCreate='true' Format='#{Saml::Kit::Namespaces::PERSISTENT}'/>
+</samlp:AuthnRequest>
+      XML
+
+      allow(registry).to receive(:metadata_for).with(issuer).and_return(nil)
+      subject = described_class.new(raw_xml, configuration: configuration)
+      expect(subject).to be_invalid
+    end
   end
 
   describe "#assertion_consumer_service_url" do

@@ -117,7 +117,7 @@ RSpec.describe Saml::Kit::LogoutRequest do
       id = Saml::Kit::Id.generate
       configuration = Saml::Kit::Configuration.new
       configuration.generate_key_pair_for(use: :signing)
-      signed_xml = Saml::Kit::Signature.sign(configuration: configuration) do |xml, signature|
+      signed_xml = Saml::Kit::Signatures.sign(configuration: configuration) do |xml, signature|
         xml.LogoutRequest ID: id do
           signature.template(id)
           xml.Fake do

@@ -59,7 +59,7 @@ RSpec.describe Saml::Kit::Response do
       id = Saml::Kit::Id.generate
       configuration = Saml::Kit::Configuration.new
       configuration.generate_key_pair_for(use: :signing)
-      signed_xml = Saml::Kit::Signature.sign(configuration: configuration) do |xml, signature|
+      signed_xml = Saml::Kit::Signatures.sign(configuration: configuration) do |xml, signature|
         xml.tag! "samlp:Response", "xmlns:samlp" => Saml::Kit::Namespaces::PROTOCOL, ID: id do
           signature.template(id)
           xml.Fake do
@@ -196,7 +196,8 @@ RSpec.describe Saml::Kit::Response do
 </samlp:Response>
       XML
       subject = described_class.new(xml)
-      expect(subject).to be_signed
+      expect(subject).to_not be_signed
+      expect(subject.assertion).to be_signed
     end
 
     it 'returns true when the Response is signed' do
@@ -246,7 +247,12 @@ RSpec.describe Saml::Kit::Response do
     let(:now) { Time.now.utc }
     let(:id) { Saml::Kit::Id.generate }
     let(:url) { FFaker::Internet.uri("https") }
-    let(:certificate) { FFaker::Movie.title }
+    let(:certificate) do
+      Saml::Kit::Certificate.new(
+        Saml::Kit::SelfSignedCertificate.new("password").create[0],
+        use: :signing
+      )
+    end
 
     it 'returns the certificate when the Assertion is signed' do
       xml = <<-XML
@@ -269,7 +275,7 @@ RSpec.describe Saml::Kit::Response do
       <ds:SignatureValue></ds:SignatureValue>
       <KeyInfo xmlns="http://www.w3.org/2000/09/xmldsig#">
         <ds:X509Data>
-          <ds:X509Certificate>#{certificate}</ds:X509Certificate>
+          <ds:X509Certificate>#{certificate.stripped}</ds:X509Certificate>
         </ds:X509Data>
       </KeyInfo>
     </ds:Signature>
@@ -277,7 +283,9 @@ RSpec.describe Saml::Kit::Response do
 </samlp:Response>
       XML
       subject = described_class.new(xml)
-      expect(subject.certificate).to eql(certificate)
+      expect(subject.signature).to be_nil
+      expect(subject.assertion.signature).to be_present
+      expect(subject.assertion.signature.certificate.stripped).to eql(certificate.stripped)
     end
 
     it 'returns the certificate when the Response is signed' do
@@ -308,7 +316,7 @@ RSpec.describe Saml::Kit::Response do
 </samlp:Response>
       XML
       subject = described_class.new(xml)
-      expect(subject.certificate).to eql(certificate)
+      expect(subject.signature.certificate).to eql(certificate)
     end
 
     it 'returns nil when there is no signature' do
@@ -319,7 +327,7 @@ RSpec.describe Saml::Kit::Response do
 </samlp:Response>
       XML
       subject = described_class.new(xml)
-      expect(subject.certificate).to be_nil
+      expect(subject.signature).to be_nil
     end
   end
 

@@ -1,6 +1,6 @@
 require "spec_helper"
 
-RSpec.describe Saml::Kit::Signature do
+RSpec.describe Saml::Kit::Signatures do
   let(:configuration) do
     config = Saml::Kit::Configuration.new
     config.add_key_pair(certificate, private_key, password: password, use: :signing)