Commit 99734d6

mo <mo@mokhan.ca>
2017-11-22 00:10:16
change API for signature generation.
1 parent c4fa437
lib/saml/kit/authentication_request.rb
@@ -35,10 +35,10 @@ module Saml
         end
 
         def to_xml
-          Signature.sign(id, sign: sign) do |xml, signature|
+          Signature.sign(sign: sign) do |xml, signature|
             xml.tag!('samlp:AuthnRequest', request_options) do
               xml.tag!('saml:Issuer', issuer)
-              signature.template(xml)
+              signature.template(id)
               xml.tag!('samlp:NameIDPolicy', Format: name_id_format)
             end
           end
lib/saml/kit/identity_provider_metadata.rb
@@ -57,10 +57,10 @@ module Saml
         end
 
         def to_xml
-          Signature.sign(id, sign: sign) do |xml, signature|
+          Signature.sign(sign: sign) do |xml, signature|
             xml.instruct!
             xml.EntityDescriptor entity_descriptor_options do
-              signature.template(xml)
+              signature.template(id)
               xml.IDPSSODescriptor idp_sso_descriptor_options do
                 xml.KeyDescriptor use: "signing" do
                   xml.KeyInfo "xmlns": Namespaces::XMLDSIG do
lib/saml/kit/logout_request.rb
@@ -40,11 +40,11 @@ module Saml
         end
 
         def to_xml
-          Signature.sign(id, sign: sign) do |xml, signature|
+          Signature.sign(sign: sign) do |xml, signature|
             xml.instruct!
             xml.LogoutRequest logout_request_options do
               xml.Issuer({ xmlns: Namespaces::ASSERTION }, issuer)
-              signature.template(xml)
+              signature.template(id)
               xml.NameID name_id_options, user.name_id_for(name_id_format)
             end
           end
lib/saml/kit/logout_response.rb
@@ -30,10 +30,10 @@ module Saml
         end
 
         def to_xml
-          Signature.sign(id, sign: sign) do |xml, signature|
+          Signature.sign(sign: sign) do |xml, signature|
             xml.LogoutResponse logout_response_options do
               xml.Issuer(issuer, xmlns: Namespaces::ASSERTION)
-              signature.template(xml)
+              signature.template(id)
               xml.Status do
                 xml.StatusCode Value: status_code
               end
lib/saml/kit/response.rb
@@ -100,10 +100,10 @@ module Saml
         end
 
         def to_xml
-          Signature.sign(id, sign: sign) do |xml, signature|
+          Signature.sign(sign: sign) do |xml, signature|
             xml.Response response_options do
               xml.Issuer(issuer, xmlns: Namespaces::ASSERTION)
-              signature.template(xml)
+              signature.template(id)
               xml.Status do
                 xml.StatusCode Value: status_code
               end
lib/saml/kit/service_provider_metadata.rb
@@ -44,10 +44,10 @@ module Saml
         end
 
         def to_xml
-          Signature.sign(id, sign: sign) do |xml, signature|
+          Signature.sign(sign: sign) do |xml, signature|
             xml.instruct!
             xml.EntityDescriptor entity_descriptor_options do
-              signature.template(xml)
+              signature.template(id)
               xml.SPSSODescriptor descriptor_options do
                 if @configuration.signing_certificate_pem.present?
                   xml.KeyDescriptor use: "signing" do
lib/saml/kit/signature.rb
@@ -16,17 +16,19 @@ module Saml
         SHA512: "http://www.w3.org/2001/04/xmlenc#sha512",
       }.freeze
 
-      attr_reader :configuration, :reference_id, :sign
+      attr_reader :configuration, :sign, :xml
 
-      def initialize(reference_id, configuration: Saml::Kit.configuration, sign: true)
-        @reference_id = reference_id
+      def initialize(xml, configuration:, sign: true)
+        @xml = xml
         @configuration = configuration
         @sign = sign
+        @reference_ids = []
       end
 
-      def template(xml = ::Builder::XmlMarkup.new)
+      def template(reference_id)
         return unless sign
         return if reference_id.blank?
+        @reference_ids << reference_id
 
         xml.Signature "xmlns" => Namespaces::XMLDSIG do
           xml.SignedInfo do
@@ -50,19 +52,20 @@ module Saml
         end
       end
 
-      def finalize(xml)
-        if sign && reference_id.present?
-          document = Xmldsig::SignedDocument.new(xml.target!)
-          document.sign(private_key)
-        else
-          xml.target!
+      def finalize
+        return xml.target! unless sign
+
+        raw_xml = xml.target!
+        @reference_ids.each do |reference_id|
+          raw_xml = Xmldsig::SignedDocument.new(raw_xml).sign(private_key)
         end
+        raw_xml
       end
 
-      def self.sign(id, sign: true, xml: ::Builder::XmlMarkup.new)
-        signature = new(id, sign: sign)
+      def self.sign(sign: true, xml: ::Builder::XmlMarkup.new, configuration: Saml::Kit.configuration)
+        signature = new(xml, sign: sign, configuration: configuration)
         yield xml, signature
-        signature.finalize(xml)
+        signature.finalize
       end
 
       private
spec/saml/authentication_request_spec.rb
@@ -103,21 +103,16 @@ RSpec.describe Saml::Kit::AuthenticationRequest do
     end
 
     it 'validates the schema of the request' do
-      xml = ::Builder::XmlMarkup.new
       id = SecureRandom.uuid
-      options = {
-        "xmlns:samlp" => Saml::Kit::Namespaces::PROTOCOL,
-        AssertionConsumerServiceURL: acs_url,
-        ID: "_#{id}",
-      }
-      signature = Saml::Kit::Signature.new(id)
-      xml.tag!('samlp:AuthnRequest', options) do
-        signature.template(xml)
-        xml.Fake do
-          xml.NotAllowed "Huh?"
+      signed_xml = Saml::Kit::Signature.sign(sign: true) do |xml, signature|
+        xml.tag!('samlp:AuthnRequest', "xmlns:samlp" => Saml::Kit::Namespaces::PROTOCOL, AssertionConsumerServiceURL: acs_url, ID: "_#{id}") do
+          signature.template(id)
+          xml.Fake do
+            xml.NotAllowed "Huh?"
+          end
         end
       end
-      expect(described_class.new(signature.finalize(xml))).to be_invalid
+      expect(described_class.new(signed_xml)).to be_invalid
     end
 
     it 'validates a request without a signature' do
spec/saml/logout_request_spec.rb
@@ -102,15 +102,15 @@ RSpec.describe Saml::Kit::LogoutRequest do
 
     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?"
+      signed_xml = Saml::Kit::Signature.sign(sign: true) do |xml, signature|
+        xml.LogoutRequest ID: "_#{id}" do
+          signature.template(id)
+          xml.Fake do
+            xml.NotAllowed "Huh?"
+          end
         end
       end
-      expect(described_class.new(signature.finalize(xml))).to be_invalid
+      expect(described_class.new(signed_xml)).to be_invalid
     end
   end
 
spec/saml/response_spec.rb
@@ -142,17 +142,16 @@ RSpec.describe Saml::Kit::Response do
     it 'validates the schema of the response' do
       allow(registry).to receive(:metadata_for).and_return(metadata)
       allow(metadata).to receive(:matches?).and_return(true)
-      xml = ::Builder::XmlMarkup.new
       id = SecureRandom.uuid
-      options = { "xmlns:samlp" => Saml::Kit::Namespaces::PROTOCOL, ID: "_#{id}", }
-      signature = Saml::Kit::Signature.new(id)
-      xml.tag!("samlp:Response", options) do
-        signature.template(xml)
-        xml.Fake do
-          xml.NotAllowed "Huh?"
+      signed_xml = Saml::Kit::Signature.sign(sign: true) do |xml, signature|
+        xml.tag! "samlp:Response", "xmlns:samlp" => Saml::Kit::Namespaces::PROTOCOL, ID: "_#{id}" do
+          signature.template(id)
+          xml.Fake do
+            xml.NotAllowed "Huh?"
+          end
         end
       end
-      subject = described_class.new(signature.finalize(xml))
+      subject = described_class.new(signed_xml)
       expect(subject).to be_invalid
       expect(subject.errors[:base]).to be_present
     end
spec/saml/signature_spec.rb
@@ -1,7 +1,6 @@
 require "spec_helper"
 
 RSpec.describe Saml::Kit::Signature do
-  subject { described_class.new(reference_id, configuration: configuration) }
   let(:configuration) do
     config = Saml::Kit::Configuration.new
     config.signing_certificate_pem = certificate
@@ -32,17 +31,18 @@ RSpec.describe Saml::Kit::Signature do
   let(:password) { "password" }
 
   it 'generates a signature' do
-    xml = ::Builder::XmlMarkup.new
     options = {
       "xmlns:samlp" => "urn:oasis:names:tc:SAML:2.0:protocol",
       "xmlns:saml" => "urn:oasis:names:tc:SAML:2.0:assertion",
       ID: "_#{reference_id}",
     }
-    xml.tag!('samlp:AuthnRequest', options) do
-      subject.template(xml)
-      xml.tag!('saml:Issuer', "MyEntityID")
+    signed_xml = described_class.sign(sign: true, configuration: configuration) do |xml, signature|
+      xml.tag!('samlp:AuthnRequest', options) do
+        signature.template(reference_id)
+        xml.tag!('saml:Issuer', "MyEntityID")
+      end
     end
-    result = Hash.from_xml(subject.finalize(xml))
+    result = Hash.from_xml(signed_xml)
 
     signature = result["AuthnRequest"]["Signature"]
     expect(signature['xmlns']).to eql("http://www.w3.org/2000/09/xmldsig#")
@@ -63,13 +63,13 @@ RSpec.describe Saml::Kit::Signature do
   end
 
   it 'does not add a signature' do
-    subject = described_class.new(reference_id, sign: false, configuration: configuration)
-    xml = ::Builder::XmlMarkup.new
-    xml.AuthnRequest do
-      subject.template(xml)
-      xml.Issuer "MyEntityID"
+    signed_xml = described_class.sign(sign: false, configuration: configuration) do |xml, signature|
+      xml.AuthnRequest do
+        signature.template(reference_id)
+        xml.Issuer "MyEntityID"
+      end
     end
-    result = Hash.from_xml(subject.finalize(xml))
+    result = Hash.from_xml(signed_xml)
     expect(result['AuthnRequest']).to be_present
     expect(result["AuthnRequest"]["Signature"]).to be_nil
   end