Commit c90871f

mo <mo.khan@gmail.com>
2018-02-16 18:43:01
record validation error when decryption fails.
1 parent 9febf08
lib/saml/kit/locales/en.yml
@@ -3,6 +3,7 @@ en:
   saml/kit:
     errors:
       Assertion:
+        cannot_decrypt: "cannot be decrypted."
         expired: "must not be expired."
         must_match_issuer: "must match entityId."
         must_contain_single_assertion: "must contain single Assertion."
lib/saml/kit/assertion.rb
@@ -8,9 +8,10 @@ module Saml
       include ActiveModel::Validations
       include Translatable
 
-      validate :must_match_issuer
-      validate :must_be_active_session
-      validate :must_have_valid_signature
+      validate :must_be_decryptable
+      validate :must_match_issuer, if: :decryptable?
+      validate :must_be_active_session, if: :decryptable?
+      validate :must_have_valid_signature, if: :decryptable?
       attr_reader :name
       attr_accessor :occurred_at
 
@@ -84,6 +85,11 @@ module Saml
         @xml_hash.fetch('EncryptedAssertion', nil).present?
       end
 
+      def decryptable?
+        return true unless encrypted?
+        !@cannot_decrypt
+      end
+
       def present?
         assertion.present?
       end
@@ -110,6 +116,9 @@ module Saml
 
         encrypted_assertion = @node.at_xpath('./xmlenc:EncryptedData', Saml::Kit::Document::NAMESPACES)
         @node = decryptor.decrypt_node(encrypted_assertion)
+      rescue Xml::Kit::DecryptionError => error
+        @cannot_decrypt = true
+        Saml::Kit.logger.error(error)
       end
 
       def parse_date(value)
@@ -138,6 +147,10 @@ module Saml
         end
       end
 
+      def must_be_decryptable
+        errors.add(:base, error_message(:cannot_decrypt)) unless decryptable?
+      end
+
       def at_xpath(xpath)
         @node.at_xpath(xpath, Saml::Kit::Document::NAMESPACES)
       end
lib/saml/kit/response.rb
@@ -32,6 +32,7 @@ module Saml
       def must_be_valid_assertion
         assertion.valid?
         assertion.errors.each do |attribute, error|
+          attribute = :assertion if :base == attribute
           self.errors[attribute] << error
         end
       end
spec/saml/response_spec.rb
@@ -117,7 +117,7 @@ RSpec.describe Saml::Kit::Response do
       subject = described_class.build(user, request)
       travel_to Saml::Kit.configuration.session_timeout.from_now + 5.seconds
       expect(subject).to_not be_valid
-      expect(subject.errors[:base]).to be_present
+      expect(subject.errors[:assertion]).to match_array(["must not be expired."])
     end
 
     it 'is invalid before the valid session window' do
@@ -127,7 +127,7 @@ RSpec.describe Saml::Kit::Response do
       subject = described_class.build(user, request)
       travel_to (Saml::Kit.configuration.clock_drift + 1.second).before(Time.now)
       expect(subject).to be_invalid
-      expect(subject.errors[:base]).to be_present
+      expect(subject.errors[:assertion]).to match_array(["must not be expired."])
     end
 
     it 'is invalid when the audience does not match the expected issuer' do
@@ -244,6 +244,16 @@ RSpec.describe Saml::Kit::Response do
       expect(subject).to_not be_valid
       expect(subject.errors[:digest_value]).to be_present
     end
+
+    it 'is invalid when we do not have a private key to decrypt the assertion' do
+      xml = described_class.build_xml(user, request) do |x|
+        x.encrypt_with(::Xml::Kit::KeyPair.generate(use: :encryption))
+      end
+
+      subject = described_class.new(xml)
+      expect(subject).to be_invalid
+      expect(subject.errors[:assertion]).to match_array(["cannot be decrypted."])
+    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.9", "<= 1.0.0"
+  spec.add_dependency "xml-kit", ">= 0.1.10", "<= 1.0.0"
   spec.add_development_dependency "bundler", "~> 1.15"
   spec.add_development_dependency "ffaker", "~> 2.7"
   spec.add_development_dependency "rake", "~> 10.0"