Commit 304e490

mo <mo.khan@gmail.com>
2017-11-12 23:16:42
add validation for logout request.
1 parent 4fde95f
Changed files (3)
lib
spec
lib/saml/kit/logout_request.rb
@@ -1,33 +1,121 @@
 module Saml
   module Kit
     class LogoutRequest
+      PROTOCOL_XSD = File.expand_path("./xsd/saml-schema-protocol-2.0.xsd", File.dirname(__FILE__)).freeze
+      include XsdValidatable
+      include ActiveModel::Validations
+      validates_presence_of :content
+      validates_presence_of :single_logout_service, if: :logout_request?
+      validate :must_be_request
+      validate :must_have_valid_signature
+      validate :must_be_registered
+      validate :must_match_xsd
+
+      attr_reader :content, :name
+
       def initialize(xml)
-        @xml = xml
+        @content = xml
+        @name = "LogoutRequest"
         @xml_hash = Hash.from_xml(xml)
       end
 
       def issuer
-        @xml_hash['LogoutRequest']['Issuer']
+        to_h[name]['Issuer']
       end
 
       def issue_instant
-        @xml_hash['LogoutRequest']['IssueInstant']
+        to_h[name]['IssueInstant']
       end
 
       def version
-        @xml_hash['LogoutRequest']['Version']
+        to_h[name]['Version']
       end
 
       def destination
-        @xml_hash['LogoutRequest']['Destination']
+        to_h[name]['Destination']
       end
 
       def name_id
-        @xml_hash['LogoutRequest']['NameID']
+        to_h[name]['NameID']
+      end
+
+      def single_logout_service
+        return if provider.nil?
+        urls = provider.single_logout_services
+        return urls.first[:location] if urls.any?
+      end
+
+      def to_h
+        @xml_hash
       end
 
       def to_xml
-        @xml
+        @content
+      end
+
+      def trusted?
+        return false if provider.nil?
+        return false unless signed?
+        provider.matches?(fingerprint, use: :signing)
+      end
+
+      def provider
+        registry.metadata_for(issuer)
+      end
+
+      def certificate
+        return nil unless signed?
+        to_h[name]['Signature']['KeyInfo']['X509Data']['X509Certificate']
+      end
+
+      def fingerprint
+        return nil unless signed?
+        Fingerprint.new(certificate)
+      end
+
+      def signed?
+        to_h[name]['Signature'].present?
+      end
+
+      private
+
+      def registry
+        Saml::Kit.configuration.registry
+      end
+
+      def must_have_valid_signature
+        return if to_xml.blank?
+
+        xml = Saml::Kit::Xml.new(to_xml)
+        xml.valid?
+        xml.errors.each do |error|
+          errors[:base] << error
+        end
+      end
+
+      def must_be_request
+        return if to_h.nil?
+
+        errors[:base] << error_message(:invalid) unless logout_request?
+      end
+
+      def must_be_registered
+        return unless logout_request?
+        if provider.nil?
+          errors[:provider] << error_message(:unregistered)
+          return
+        end
+        return if trusted?
+        errors[:fingerprint] << error_message(:invalid_fingerprint)
+      end
+
+      def must_match_xsd
+        matches_xsd?(PROTOCOL_XSD)
+      end
+
+      def logout_request?
+        return false if to_xml.blank?
+        to_h[name].present?
       end
 
       class Builder
@@ -48,7 +136,7 @@ module Saml
           Signature.sign(id, sign: sign) do |xml, signature|
             xml.instruct!
             xml.LogoutRequest logout_request_options do
-              xml.Issuer issuer
+              xml.Issuer({ xmlns: Namespaces::ASSERTION }, issuer)
               signature.template(xml)
               xml.NameID name_id_options, user.name_id_for(name_id_format)
             end
@@ -67,12 +155,14 @@ module Saml
             Version: "2.0",
             IssueInstant: now.utc.iso8601,
             Destination: destination,
+            xmlns: Namespaces::PROTOCOL,
           }
         end
 
         def name_id_options
           {
             Format: name_id_format,
+            xmlns: Namespaces::ASSERTION,
           }
         end
       end
spec/saml/logout_request_spec.rb
@@ -29,6 +29,91 @@ RSpec.describe Saml::Kit::LogoutRequest do
     expect(subject.name_id).to eql(name_id)
   end
 
+  describe "#valid?" do
+    let(:registry) { instance_double(Saml::Kit::DefaultRegistry) }
+    let(:metadata) { instance_double(Saml::Kit::ServiceProviderMetadata) }
+
+    before :each do
+      allow(Saml::Kit.configuration).to receive(:registry).and_return(registry)
+      allow(registry).to receive(:metadata_for).and_return(metadata)
+      allow(metadata).to receive(:matches?).and_return(true)
+      allow(metadata).to receive(:single_logout_services).and_return([
+        { location: FFaker::Internet.http_url, binding: Saml::Kit::Namespaces::POST }
+      ])
+    end
+
+    it 'is valid when left untampered' do
+      expect(builder.build).to be_valid
+    end
+
+    it 'is invalid if the document has been tampered with' do
+      builder.issuer = FFaker::Internet.http_url
+      raw_xml = builder.to_xml.gsub(builder.issuer, 'corrupt')
+      subject = described_class.new(raw_xml)
+      expect(subject).to be_invalid
+    end
+
+    it 'is invalid when blank' do
+      subject = described_class.new('')
+      expect(subject).to be_invalid
+      expect(subject.errors[:content]).to be_present
+    end
+
+    it 'is invalid when not a LogoutRequest' do
+      xml = Saml::Kit::IdentityProviderMetadata::Builder.new.to_xml
+      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(metadata).to receive(:matches?).and_return(false)
+      subject = builder.build
+      expect(subject).to be_invalid
+      expect(subject.errors[:fingerprint]).to be_present
+    end
+
+    it 'is invalid when the provider is not known' do
+      allow(registry).to receive(:metadata_for).and_return(nil)
+      subject = builder.build
+      expect(subject).to be_invalid
+      expect(subject.errors[:provider]).to be_present
+    end
+
+    it 'is invalid when single logout service url is not provided' do
+      allow(metadata).to receive(:matches?).and_return(true)
+      allow(metadata).to receive(:single_logout_services).and_return([])
+
+      subject = builder.build
+      expect(subject).to be_invalid
+      expect(subject.errors[:single_logout_service]).to be_present
+    end
+
+    it 'is valid when a single lgout service url is available via the registry' do
+      builder.issuer = FFaker::Internet.http_url
+      allow(registry).to receive(:metadata_for).with(builder.issuer).and_return(metadata)
+      allow(metadata).to receive(:matches?).and_return(true)
+      allow(metadata).to receive(:single_logout_services).and_return([
+        { location: FFaker::Internet.http_url, binding: Saml::Kit::Namespaces::POST }
+      ])
+
+      expect(builder.build).to be_valid
+    end
+
+    it 'validates the schema of the request' do
+      id = SecureRandom.uuid
+      signature = Saml::Kit::Signature.new(id)
+      xml = ::Builder::XmlMarkup.new
+      xml.LogoutRequest ID: "_#{id}" do
+        signature.template(xml)
+        xml.Fake do
+          xml.NotAllowed "Huh?"
+        end
+      end
+      expect(described_class.new(signature.finalize(xml))).to be_invalid
+    end
+  end
+
   describe described_class::Builder do
     subject { described_class.new(user) }
     let(:user) { double(:user, name_id_for: name_id) }
@@ -51,7 +136,7 @@ RSpec.describe Saml::Kit::LogoutRequest do
 
       expect(xml_hash['LogoutRequest']['Issuer']).to eql(subject.issuer)
       expect(xml_hash['LogoutRequest']['NameID']).to eql(name_id)
-      expect(result).to have_xpath("//LogoutRequest//NameID[@Format=\"#{subject.name_id_format}\"]")
+      expect(result).to have_xpath("//samlp:LogoutRequest//saml:NameID[@Format=\"#{subject.name_id_format}\"]")
     end
 
     it 'includes a signature by default' do
spec/support/matchers/have_xml.rb
@@ -1,6 +1,11 @@
 RSpec::Matchers.define :have_xpath do |xpath|
+  SAML_NAMESPACES = {
+    'saml' => Saml::Kit::Namespaces::ASSERTION,
+    'samlp' => Saml::Kit::Namespaces::PROTOCOL,
+  }.freeze
+
   match do |actual|
-    xml_document(actual).xpath(xpath).any?
+    xml_document(actual).xpath(xpath, SAML_NAMESPACES).any?
   end
 
   failure_message do |actual|