Merge pull request #231 from onelogin/refactor_http_redirect_sign

Refactor HTTP-Redirect Sign method, Move test data to right folder
SAML-Toolkits · May 7, 2015 · 46e1f5a · 46e1f5a
2 parents 5a25635 + 31a1d77
commit 46e1f5a
Show file tree

Hide file tree

Showing 9 changed files with 48 additions and 18 deletions.
diff --git a/lib/onelogin/ruby-saml/authrequest.rb b/lib/onelogin/ruby-saml/authrequest.rb
@@ -64,11 +64,14 @@ def create_params(settings, params={})
 
         if settings.security[:authn_requests_signed] && !settings.security[:embed_sign] && settings.private_key
           params['SigAlg']    = settings.security[:signature_method]
-          url_string          = "SAMLRequest=#{CGI.escape(base64_request)}"
-          url_string         << "&RelayState=#{CGI.escape(relay_state)}" if relay_state
-          url_string         << "&SigAlg=#{CGI.escape(params['SigAlg'])}"
-          private_key         = settings.get_sp_key
-          signature           = private_key.sign(XMLSecurity::BaseDocument.new.algorithm(settings.security[:signature_method]).new, url_string)
+          url_string = OneLogin::RubySaml::Utils.build_query(
+            :type => 'SAMLRequest',
+            :data => base64_request,
+            :relay_state => relay_state,
+            :sig_alg => params['SigAlg']
+          )
+          sign_algorithm = XMLSecurity::BaseDocument.new.algorithm(settings.security[:signature_method])
+          signature = settings.get_sp_key.sign(sign_algorithm.new, url_string)
           params['Signature'] = encode(signature)
         end
 

diff --git a/lib/onelogin/ruby-saml/logoutrequest.rb b/lib/onelogin/ruby-saml/logoutrequest.rb
@@ -62,11 +62,14 @@ def create_params(settings, params={})
 
         if settings.security[:logout_requests_signed] && !settings.security[:embed_sign] && settings.private_key
           params['SigAlg']    = settings.security[:signature_method]
-          url_string          = "SAMLRequest=#{CGI.escape(base64_request)}"
-          url_string         << "&RelayState=#{CGI.escape(relay_state)}" if relay_state
-          url_string         << "&SigAlg=#{CGI.escape(params['SigAlg'])}"
-          private_key         = settings.get_sp_key
-          signature           = private_key.sign(XMLSecurity::BaseDocument.new.algorithm(settings.security[:signature_method]).new, url_string)
+          url_string = OneLogin::RubySaml::Utils.build_query(
+            :type => 'SAMLRequest',
+            :data => base64_request,
+            :relay_state => relay_state,
+            :sig_alg => params['SigAlg']
+          )
+          sign_algorithm = XMLSecurity::BaseDocument.new.algorithm(settings.security[:signature_method])
+          signature = settings.get_sp_key.sign(sign_algorithm.new, url_string)
           params['Signature'] = encode(signature)
         end
 

diff --git a/lib/onelogin/ruby-saml/slo_logoutresponse.rb b/lib/onelogin/ruby-saml/slo_logoutresponse.rb
@@ -67,11 +67,14 @@ def create_params(settings, request_id = nil, logout_message = nil, params = {})
 
         if settings.security[:logout_responses_signed] && !settings.security[:embed_sign] && settings.private_key
           params['SigAlg']    = settings.security[:signature_method]
-          url_string          = "SAMLResponse=#{CGI.escape(base64_response)}"
-          url_string         << "&RelayState=#{CGI.escape(relay_state)}" if relay_state
-          url_string         << "&SigAlg=#{CGI.escape(params['SigAlg'])}"
-          private_key         = settings.get_sp_key
-          signature           = private_key.sign(XMLSecurity::BaseDocument.new.algorithm(settings.security[:signature_method]).new, url_string)
+          url_string = OneLogin::RubySaml::Utils.build_query(
+            :type => 'SAMLResponse',
+            :data => base64_response,
+            :relay_state => relay_state,
+            :sig_alg => params['SigAlg']
+          )
+          sign_algorithm = XMLSecurity::BaseDocument.new.algorithm(settings.security[:signature_method])
+          signature = settings.get_sp_key.sign(sign_algorithm.new, url_string)
           params['Signature'] = encode(signature)
         end
 

diff --git a/lib/onelogin/ruby-saml/utils.rb b/lib/onelogin/ruby-saml/utils.rb
@@ -38,6 +38,23 @@ def self.format_private_key(key)
         key_label = rsa_key ? "RSA PRIVATE KEY" : "PRIVATE KEY"
         "-----BEGIN #{key_label}-----\n#{key}\n-----END #{key_label}-----"
       end
+
+      # Build the Query String signature that will be used in the HTTP-Redirect binding
+      # to generate the Signature
+      # @param params [Hash] Parameters to build the Query String
+      # @option params [String] :type 'SAMLRequest' or 'SAMLResponse'
+      # @option params [String] :data Base64 encoded SAMLRequest or SAMLResponse
+      # @option params [String] :relay_state The RelayState parameter
+      # @option params [String] :sig_alg The SigAlg parameter
+      # @return [String] The Query String
+      #
+      def self.build_query(params)
+        type, data, relay_state, sig_alg = [:type, :data, :relay_state, :sig_alg].map { |k| params[k]}
+
+        url_string = "#{type}=#{CGI.escape(data)}"
+        url_string << "&RelayState=#{CGI.escape(relay_state)}" if relay_state
+        url_string << "&SigAlg=#{CGI.escape(sig_alg)}"
+        end
     end
   end
 end
diff --git a/test/responses/slo_request.xml → test/logout_requests/slo_request.xml b/test/responses/slo_request.xml → test/logout_requests/slo_request.xml
diff --git a/test/responses/logoutresponse_fixtures.rb → ...gout_responses/logoutresponse_fixtures.rb b/test/responses/logoutresponse_fixtures.rb → ...gout_responses/logoutresponse_fixtures.rb
diff --git a/test/logoutresponse_test.rb b/test/logoutresponse_test.rb
@@ -1,7 +1,7 @@
 require File.expand_path(File.join(File.dirname(__FILE__), "test_helper"))
 
 require 'onelogin/ruby-saml/logoutresponse'
-require 'responses/logoutresponse_fixtures'
+require 'logout_responses/logoutresponse_fixtures'
 
 class RubySamlTest < Minitest::Test
 

diff --git a/test/slo_logoutrequest_test.rb b/test/slo_logoutrequest_test.rb
@@ -1,5 +1,5 @@
 require File.expand_path(File.join(File.dirname(__FILE__), "test_helper"))
-require 'responses/logoutresponse_fixtures'
+require 'logout_responses/logoutresponse_fixtures'
 
 require 'onelogin/ruby-saml/slo_logoutrequest'
 

diff --git a/test/test_helper.rb b/test/test_helper.rb
@@ -24,6 +24,10 @@ def read_response(response)
     File.read(File.join(File.dirname(__FILE__), "responses", response))
   end
 
+  def read_logout_request(request)
+    File.read(File.join(File.dirname(__FILE__), "logout_requests", request))
+  end
+
   def read_certificate(certificate)
     File.read(File.join(File.dirname(__FILE__), "certificates", certificate))
   end
@@ -89,7 +93,7 @@ def idp_metadata
 
   def logout_request_document
     unless @logout_request_document
-      xml = read_response("slo_request.xml")
+      xml = read_logout_request("slo_request.xml")
       deflated = Zlib::Deflate.deflate(xml, 9)[2..-5]
       @logout_request_document = Base64.encode64(deflated)
     end