Commit 79026da

mo <mo@mokhan.ca>
2017-11-07 18:14:33
update error messages.
1 parent 3058900
Changed files (3)
lib
saml
spec
lib/saml/kit/locales/en.yml
@@ -8,3 +8,10 @@ en:
       IDPSSODescriptor:
         invalid: "must contain identity provider metadata."
         invalid_signature: "invalid signature."
+      Response:
+        invalid: "must contain response."
+        unregistered: "must originate from registered service provider."
+        expired: "must not be expired."
+        invalid_version: "must be 2.0."
+        invalid_response_to: "must match request id."
+        must_match_issuer: "must match entityId."
lib/saml/kit/response.rb
@@ -13,7 +13,8 @@ module Saml
       validate :must_be_registered
       validate :must_match_xsd
       validate :must_be_valid_version
-      validate :must_be_successful
+      validates_inclusion_of :status_code, in: [Namespaces::SUCCESS]
+
       validate :must_match_request_id
       validate :must_be_active_session
       validate :must_match_issuer
@@ -130,7 +131,7 @@ module Saml
         return unless login_response?
         return if provider.present? && provider.matches?(fingerprint, use: "signing")
 
-        errors[:base] << error_message(:invalid)
+        errors[:base] << error_message(:unregistered)
       end
 
       def must_match_xsd
@@ -140,19 +141,14 @@ module Saml
       def must_be_valid_version
         return unless login_response?
         return if "2.0" == version
-        errors[:base] << error_message(:invalid)
-      end
-
-      def must_be_successful
-        return if Namespaces::SUCCESS == status_code
-        errors[:base] << error_message(:invalid)
+        errors[:version] << error_message(:invalid_version)
       end
 
       def must_match_request_id
         return if request_id.nil?
 
         if in_response_to != request_id
-          errors[:base] << error_message(:invalid)
+          errors[:in_response_to] << error_message(:invalid_response_to)
         end
       end
 
@@ -165,7 +161,7 @@ module Saml
         return unless login_response?
 
         unless audiences.include?(Saml::Kit.configuration.issuer)
-          errors[:base] << error_message(:expired)
+          errors[:audience] << error_message(:must_match_issuer)
         end
       end
 
spec/saml/response_spec.rb
@@ -108,7 +108,9 @@ RSpec.describe Saml::Kit::Response do
     end
 
     it 'is invalid when blank' do
-      expect(described_class.new("")).to be_invalid
+      subject = described_class.new("")
+      expect(subject).to be_invalid
+      expect(subject.errors[:content]).to be_present
     end
 
     it 'is invalid if the document has been tampered with' do
@@ -117,18 +119,22 @@ RSpec.describe Saml::Kit::Response do
       status_code = FFaker::Movie.title
       builder.status_code = status_code
       subject = described_class.new(builder.to_xml.gsub(status_code, "TAMPERED"))
-      expect(subject).to_not be_valid
+      expect(subject).to be_invalid
     end
 
     it 'is invalid when not a Response' do
       xml = Saml::Kit::IdentityProviderMetadata::Builder.new.to_xml
-      expect(described_class.new(xml)).to be_invalid
+      subject = described_class.new(xml)
+      expect(subject).to be_invalid
+      expect(subject.errors[:base]).to be_present
     end
 
     it 'is invalid when the fingerprint of the certificate does not match the registered fingerprint' do
       allow(registry).to receive(:metadata_for).and_return(metadata)
       allow(metadata).to receive(:matches?).and_return(false)
-      expect(described_class.new(builder.to_xml)).to be_invalid
+      subject = described_class.new(builder.to_xml)
+      expect(subject).to be_invalid
+      expect(subject.errors[:base]).to be_present
     end
 
     it 'validates the schema of the response' do
@@ -144,34 +150,44 @@ RSpec.describe Saml::Kit::Response do
           xml.NotAllowed "Huh?"
         end
       end
-      expect(described_class.new(signature.finalize(xml))).to be_invalid
+      subject = described_class.new(signature.finalize(xml))
+      expect(subject).to be_invalid
+      expect(subject.errors[:base]).to be_present
     end
 
     it 'validates the version' do
       allow(registry).to receive(:metadata_for).and_return(metadata)
       allow(metadata).to receive(:matches?).and_return(true)
       builder.version = "1.1"
-      expect(described_class.new(builder.to_xml)).to be_invalid
+      subject = described_class.new(builder.to_xml)
+      expect(subject).to be_invalid
+      expect(subject.errors[:version]).to be_present
     end
 
     it 'validates the id' do
       allow(registry).to receive(:metadata_for).and_return(metadata)
       allow(metadata).to receive(:matches?).and_return(true)
       builder.id = nil
-      expect(described_class.new(builder.to_xml)).to_not be_valid
+      subject = described_class.new(builder.to_xml)
+      expect(subject).to be_invalid
+      expect(subject.errors[:id]).to be_present
     end
 
     it 'validates the status code' do
       allow(registry).to receive(:metadata_for).and_return(metadata)
       allow(metadata).to receive(:matches?).and_return(true)
       builder.status_code = Saml::Kit::Namespaces::REQUESTER_ERROR
-      expect(described_class.new(builder.to_xml)).to_not be_valid
+      subject = described_class.new(builder.to_xml)
+      expect(subject).to be_invalid
+      expect(subject.errors[:status_code]).to be_present
     end
 
     it 'validates the InResponseTo' do
       allow(registry).to receive(:metadata_for).and_return(metadata)
       allow(metadata).to receive(:matches?).and_return(true)
-      expect(described_class.new(builder.to_xml, request_id: SecureRandom.uuid)).to_not be_valid
+      subject = described_class.new(builder.to_xml, request_id: SecureRandom.uuid)
+      expect(subject).to be_invalid
+      expect(subject.errors[:in_response_to]).to be_present
     end
 
     it 'is invalid after a valid session window' do
@@ -181,6 +197,7 @@ RSpec.describe Saml::Kit::Response do
       subject = described_class.new(builder.to_xml)
       travel_to Saml::Kit.configuration.session_timeout.from_now + 5.seconds
       expect(subject).to_not be_valid
+      expect(subject.errors[:base]).to be_present
     end
 
     it 'is invalid before the valid session window' do
@@ -189,7 +206,8 @@ RSpec.describe Saml::Kit::Response do
 
       subject = described_class.new(builder.to_xml)
       travel_to 5.seconds.ago
-      expect(subject).to_not be_valid
+      expect(subject).to be_invalid
+      expect(subject.errors[:base]).to be_present
     end
 
     it 'is invalid when the audience does not match the expected issuer' do
@@ -199,7 +217,9 @@ RSpec.describe Saml::Kit::Response do
       allow(Saml::Kit.configuration).to receive(:issuer).and_return(FFaker::Internet.http_url)
       allow(request).to receive(:issuer).and_return(FFaker::Internet.http_url)
       subject = described_class.new(builder.to_xml)
-      expect(subject).to_not be_valid
+
+      expect(subject).to be_invalid
+      expect(subject.errors[:audience]).to be_present
     end
   end
 end