Commit 24ffa3d

mo <mo.khan@gmail.com>
2017-11-01 22:04:49
promote validations to metadata.
1 parent 7c9fcec
lib/saml/kit/locales/en.yml
@@ -2,8 +2,9 @@
 en:
   saml/kit:
     errors:
-      identity_provider_metadata:
-        metadata:
-          invalid_idp: "must contain identity provider metadata."
-          blank: "metadata is required."
-          invalid_signature: "invalid signature."
+      SPSSODescriptor:
+        invalid: "must contain service provider metadata."
+        invalid_signature: "invalid signature."
+      IDPSSODescriptor:
+        invalid: "must contain identity provider metadata."
+        invalid_signature: "invalid signature."
lib/saml/kit/authentication_request.rb
@@ -60,7 +60,7 @@ module Saml
           options = {
             "xmlns:samlp" => Namespaces::PROTOCOL,
             "xmlns:saml" => Namespaces::ASSERTION,
-            ID: id,
+            ID: "_#{id}",
             Version: "2.0",
             IssueInstant: issued_at.strftime("%Y-%m-%dT%H:%M:%SZ"),
           }
lib/saml/kit/identity_provider_metadata.rb
@@ -1,14 +1,6 @@
 module Saml
   module Kit
     class IdentityProviderMetadata < Metadata
-      METADATA_XSD = File.expand_path("./xsd/saml-schema-metadata-2.0.xsd", File.dirname(__FILE__)).freeze
-
-      include ActiveModel::Validations
-      validates_presence_of :metadata
-      validate :must_contain_idp_descriptor
-      validate :must_match_xsd
-      validate :must_have_valid_signature
-
       def initialize(xml)
         super("IDPSSODescriptor", xml)
       end
@@ -32,41 +24,6 @@ module Saml
 
       private
 
-      def error_message(key)
-        I18n.translate(key, scope: 'saml/kit.errors.identity_provider_metadata')
-      end
-
-      def metadata
-        find_by('/md:EntityDescriptor/md:IDPSSODescriptor').present?
-      end
-
-      def must_contain_idp_descriptor
-        errors[:metadata] << error_message('metadata.invalid_idp') unless metadata
-      end
-
-      def must_match_xsd
-        Dir.chdir(File.dirname(METADATA_XSD)) do
-          xsd = Nokogiri::XML::Schema(IO.read(METADATA_XSD))
-          xsd.validate(document).each do |error|
-            errors[:metadata] << error.message
-          end
-        end
-      end
-
-      def must_have_valid_signature
-        return if to_xml.blank?
-        errors[:metadata] << error_message('metadata.invalid_signature') unless valid_signature?
-      end
-
-      def valid_signature?
-        xml = Saml::Kit::Xml.new(to_xml)
-        result = xml.valid?
-        xml.errors.each do |error|
-          errors[:metadata] << error
-        end
-        result
-      end
-
       class Builder
         attr_accessor :id, :organization_name, :organization_url, :contact_email, :entity_id, :single_sign_on_location, :single_logout_location, :attributes
 
@@ -112,7 +69,7 @@ module Saml
             'xmlns': Namespaces::METADATA,
             'xmlns:ds': Namespaces::SIGNATURE,
             'xmlns:saml': Namespaces::ASSERTION,
-            ID: id,
+            ID: "_#{id}",
             entityID: entity_id,
           }
         end
lib/saml/kit/metadata.rb
@@ -1,6 +1,9 @@
 module Saml
   module Kit
     class Metadata
+      include ActiveModel::Validations
+
+      METADATA_XSD = File.expand_path("./xsd/saml-schema-metadata-2.0.xsd", File.dirname(__FILE__)).freeze
       NAMESPACES = {
         "NameFormat": Namespaces::Formats::Attr::SPLAT,
         "ds": Namespaces::SIGNATURE,
@@ -8,6 +11,11 @@ module Saml
         "saml": Namespaces::ASSERTION,
       }.freeze
 
+      validates_presence_of :metadata
+      validate :must_contain_descriptor
+      validate :must_match_xsd
+      validate :must_have_valid_signature
+
       attr_reader :xml, :descriptor_name
 
       def initialize(descriptor_name, xml)
@@ -79,6 +87,46 @@ module Saml
       def pretty_fingerprint(fingerprint)
         fingerprint.upcase.scan(/../).join(":")
       end
+
+      def metadata
+        find_by("/md:EntityDescriptor/md:#{descriptor_name}").present?
+      end
+
+      def must_contain_descriptor
+        unless metadata
+          errors[:metadata] << error_message('invalid')
+        end
+      end
+
+      def must_match_xsd
+        Dir.chdir(File.dirname(METADATA_XSD)) do
+          xsd = Nokogiri::XML::Schema(IO.read(METADATA_XSD))
+          xsd.validate(document).each do |error|
+            errors[:metadata] << error.message
+          end
+        end
+      end
+
+      def must_have_valid_signature
+        return if to_xml.blank?
+
+        unless valid_signature?
+          errors[:metadata] << error_message('invalid_signature') 
+        end
+      end
+
+      def valid_signature?
+        xml = Saml::Kit::Xml.new(to_xml)
+        result = xml.valid?
+        xml.errors.each do |error|
+          errors[:metadata] << error
+        end
+        result
+      end
+
+      def error_message(key)
+        I18n.translate(key, scope: "saml/kit.errors.#{descriptor_name}")
+      end
     end
   end
 end
lib/saml/kit/response.rb
@@ -98,7 +98,7 @@ module Saml
 
         def response_options
           {
-            ID: id,
+            ID: "_#{id}",
             Version: "2.0",
             IssueInstant: now.iso8601,
             Destination: request.acs_url,
lib/saml/kit/service_provider_metadata.rb
@@ -77,7 +77,7 @@ module Saml
         def entity_descriptor_options
           {
             'xmlns:md': Namespaces::METADATA,
-            ID: id,
+            ID: "_#{id}",
             entityID: entity_id,
           }
         end
lib/saml/kit/signature.rb
@@ -29,7 +29,7 @@ module Saml
           xml.tag! "ds:SignedInfo", "xmlns:ds" => XMLDSIG do
             xml.tag! "ds:CanonicalizationMethod", Algorithm: "http://www.w3.org/2001/10/xml-exc-c14n#"
             xml.tag! "ds:SignatureMethod", Algorithm: SIGNATURE_METHODS[configuration.signature_method]
-            xml.tag! "ds:Reference", URI: "##{reference_id}" do
+            xml.tag! "ds:Reference", URI: "#_#{reference_id}" do
               xml.tag! "ds:Transforms" do
                 xml.tag! "ds:Transform", Algorithm: "http://www.w3.org/2000/09/xmldsig#enveloped-signature"
                 xml.tag! "ds:Transform", Algorithm: "http://www.w3.org/2001/10/xml-exc-c14n#"
spec/saml/authentication_request_spec.rb
@@ -16,7 +16,7 @@ RSpec.describe Saml::Kit::AuthenticationRequest do
   end
 
   it { expect(subject.issuer).to eql(issuer) }
-  it { expect(subject.id).to eql(id) }
+  it { expect(subject.id).to eql("_#{id}") }
   it { expect(subject.acs_url).to eql(acs_url) }
 
   describe "#valid?" do
spec/saml/identity_provider_metadata_spec.rb
@@ -175,16 +175,13 @@ RSpec.describe Saml::Kit::IdentityProviderMetadata do
 
     it 'valid when given valid identity provider metadata' do
       subject = described_class.new(identity_provider_metadata)
-      subject.validate do |error|
-        errors << error
-      end
-      expect(errors).to be_empty
+      expect(subject).to be_valid
     end
 
     it 'is invalid, when given service provider metadata' do
       subject = described_class.new(service_provider_metadata)
       expect(subject).to_not be_valid
-      expect(subject.errors[:metadata]).to include(I18n.translate("saml/kit.errors.identity_provider_metadata.metadata.invalid_idp"))
+      expect(subject.errors[:metadata]).to include(I18n.translate("saml/kit.errors.IDPSSODescriptor.invalid"))
     end
 
     it 'is invalid, when the metadata is nil' do
spec/saml/service_provider_metadata_spec.rb
@@ -1,8 +1,13 @@
 require 'spec_helper'
 
 RSpec.describe Saml::Kit::ServiceProviderMetadata do
+  let(:entity_id) { FFaker::Movie.title }
+  let(:acs_post_url) { "https://#{FFaker::Internet.domain_name}/post" }
+  let(:acs_redirect_url) { "https://#{FFaker::Internet.domain_name}/redirect" }
+  let(:logout_post_url) { "https://#{FFaker::Internet.domain_name}/post" }
+  let(:logout_redirect_url) { "https://#{FFaker::Internet.domain_name}/redirect" }
+
   describe described_class::Builder do
-    let(:entity_id) { FFaker::Movie.title }
     let(:acs_url) { "https://#{FFaker::Internet.domain_name}/acs" }
 
     <<-XML
@@ -42,11 +47,6 @@ RSpec.describe Saml::Kit::ServiceProviderMetadata do
   end
 
   describe described_class do
-    let(:entity_id) { FFaker::Movie.title }
-    let(:acs_post_url) { "https://#{FFaker::Internet.domain_name}/post" }
-    let(:acs_redirect_url) { "https://#{FFaker::Internet.domain_name}/redirect" }
-    let(:logout_post_url) { "https://#{FFaker::Internet.domain_name}/post" }
-    let(:logout_redirect_url) { "https://#{FFaker::Internet.domain_name}/redirect" }
     let(:builder) { described_class::Builder.new }
     subject do
       builder.entity_id = entity_id
@@ -92,4 +92,64 @@ RSpec.describe Saml::Kit::ServiceProviderMetadata do
       expect(subject.entity_id).to eql(entity_id)
     end
   end
+
+  describe "#validate" do
+    let(:errors) { [] }
+    let(:service_provider_metadata) do
+      builder = described_class::Builder.new
+      builder.entity_id = entity_id
+      builder.add_assertion_consumer_service(acs_post_url, binding: :post)
+      builder.add_assertion_consumer_service(acs_redirect_url, binding: :http_redirect)
+      builder.add_single_logout_service(logout_post_url, binding: :post)
+      builder.add_single_logout_service(logout_redirect_url, binding: :http_redirect)
+      builder.to_xml
+    end
+
+    let(:identity_provider_metadata) { IO.read("spec/fixtures/metadata/okta.xml") }
+
+    it 'valid when given valid service provider metadata' do
+      subject = described_class.new(service_provider_metadata)
+      expect(subject).to be_valid
+    end
+
+    it 'is invalid, when given identity provider metadata' do
+      subject = described_class.new(identity_provider_metadata)
+      expect(subject).to_not be_valid
+      expect(subject.errors[:metadata]).to include(I18n.translate("saml/kit.errors.SPSSODescriptor.invalid"))
+    end
+
+    it 'is invalid, when the metadata is nil' do
+      subject = described_class.new(nil)
+      expect(subject).to_not be_valid
+      expect(subject.errors[:metadata]).to include("can't be blank")
+    end
+
+    it 'is invalid, when the metadata does not validate against the xsd schema' do
+      xml = ::Builder::XmlMarkup.new
+      xml.instruct!
+      xml.EntityDescriptor 'xmlns': Saml::Kit::Namespaces::METADATA do
+        xml.SPSSODescriptor do
+          xml.Fake foo: :bar
+        end
+      end
+      subject = described_class.new(xml.target!)
+      expect(subject).to_not be_valid
+      expect(subject.errors[:metadata][0]).to include("1:0: ERROR: Element '{urn:oasis:names:tc:SAML:2.0:metadata}EntityDescriptor'")
+    end
+
+    context "signature validation" do
+      it 'is invalid, when the signature is invalid' do
+        new_url = 'https://myserver.com/hacked'
+        metadata_xml = service_provider_metadata.gsub(acs_post_url, new_url)
+        subject = described_class.new(metadata_xml)
+        expect(subject).to be_invalid
+        expect(subject.errors[:metadata]).to include("invalid signature.")
+      end
+
+      it 'is valid, when the content has not been tampered with' do
+        subject = described_class.new(service_provider_metadata)
+        expect(subject).to be_valid
+      end
+    end
+  end
 end
spec/saml/signature_spec.rb
@@ -36,7 +36,7 @@ RSpec.describe Saml::Kit::Signature do
     options = {
       "xmlns:samlp" => "urn:oasis:names:tc:SAML:2.0:protocol",
       "xmlns:saml" => "urn:oasis:names:tc:SAML:2.0:assertion",
-      ID: "#{reference_id}",
+      ID: "_#{reference_id}",
     }
     xml.tag!('samlp:AuthnRequest', options) do
       subject.template(xml)
@@ -50,7 +50,7 @@ RSpec.describe Saml::Kit::Signature do
     expect(signature['SignedInfo']['CanonicalizationMethod']['Algorithm']).to eql('http://www.w3.org/2001/10/xml-exc-c14n#')
     expect(signature['SignedInfo']['SignatureMethod']['Algorithm']).to eql("http://www.w3.org/2001/04/xmldsig-more#rsa-sha256")
 
-    expect(signature['SignedInfo']['Reference']['URI']).to eql("##{reference_id}")
+    expect(signature['SignedInfo']['Reference']['URI']).to eql("#_#{reference_id}")
     expect(signature['SignedInfo']['Reference']['Transforms']['Transform']).to match_array([
       { "Algorithm" => "http://www.w3.org/2000/09/xmldsig#enveloped-signature" },
       { "Algorithm" => "http://www.w3.org/2001/10/xml-exc-c14n#" }