Fix #437. Creating AuthRequests/LogoutRequests/LogoutResponses with nil RelayState sends empty RelayState URL param

Author: pitbulk

lib/onelogin/ruby-saml/authrequest.rb

@@ -51,6 +51,11 @@ def create_params(settings, params={})
         # conflicts so this line will solve them.
         relay_state = params[:RelayState] || params['RelayState']
 
+        if relay_state.nil?
+          params.delete(:RelayState)
+          params.delete('RelayState')
+        end
+
         request_doc = create_authentication_xml_doc(settings)
         request_doc.context[:attribute_quote] = :quote if settings.double_quote_xml_attribute_values
 
@@ -158,7 +163,7 @@ def create_xml_document(settings)
 
       def sign_document(document, settings)
         # embed signature
-        if settings.security[:authn_requests_signed] && settings.private_key && settings.certificate && settings.security[:embed_sign] 
+        if settings.security[:authn_requests_signed] && settings.private_key && settings.certificate && settings.security[:embed_sign]
           private_key = settings.get_sp_key
           cert = settings.get_sp_cert
           document.sign_document(private_key, cert, settings.security[:signature_method], settings.security[:digest_method])

lib/onelogin/ruby-saml/logoutrequest.rb

@@ -47,6 +47,11 @@ def create_params(settings, params={})
         # conflicts so this line will solve them.
         relay_state = params[:RelayState] || params['RelayState']
 
+        if relay_state.nil?
+          params.delete(:RelayState)
+          params.delete('RelayState')
+        end
+
         request_doc = create_logout_request_xml_doc(settings)
         request_doc.context[:attribute_quote] = :quote if settings.double_quote_xml_attribute_values
 

lib/onelogin/ruby-saml/slo_logoutresponse.rb

@@ -53,6 +53,11 @@ def create_params(settings, request_id = nil, logout_message = nil, params = {})
         # conflicts so this line will solve them.
         relay_state = params[:RelayState] || params['RelayState']
 
+        if relay_state.nil?
+          params.delete(:RelayState)
+          params.delete('RelayState')
+        end
+
         response_doc = create_logout_response_xml_doc(settings, request_id, logout_message)
         response_doc.context[:attribute_quote] = :quote if settings.double_quote_xml_attribute_values
 
@@ -108,7 +113,7 @@ def create_logout_response_xml_doc(settings, request_id = nil, logout_message =
           issuer = root.add_element "saml:Issuer"
           issuer.text = settings.issuer
         end
-        
+
         # add success message
         status = root.add_element 'samlp:Status'
 

test/logoutrequest_test.rb

@@ -28,6 +28,20 @@ class RequestTest < Minitest::Test
       assert_match /&foo=bar$/, unauth_url
     end
 
+    it "RelayState cases" do
+      unauth_url = OneLogin::RubySaml::Logoutrequest.new.create(settings, { :RelayState => nil })
+      assert !unauth_url.include?('RelayState')
+
+      unauth_url = OneLogin::RubySaml::Logoutrequest.new.create(settings, { :RelayState => "http://example.com" })
+      assert unauth_url.include?('&RelayState=http%3A%2F%2Fexample.com')
+
+      unauth_url = OneLogin::RubySaml::Logoutrequest.new.create(settings, { 'RelayState' => nil })
+      assert !unauth_url.include?('RelayState')
+
+      unauth_url = OneLogin::RubySaml::Logoutrequest.new.create(settings, { 'RelayState' => "http://example.com" })
+      assert unauth_url.include?('&RelayState=http%3A%2F%2Fexample.com')
+    end
+
     it "set sessionindex" do
       settings.idp_slo_target_url = "http://example.com"
       sessionidx = OneLogin::RubySaml::Utils.uuid

test/request_test.rb

@@ -129,6 +129,20 @@ class RequestTest < Minitest::Test
       assert_match /&hello=$/, auth_url
     end
 
+    it "RelayState cases" do
+      auth_url = OneLogin::RubySaml::Authrequest.new.create(settings, { :RelayState => nil })
+      assert !auth_url.include?('RelayState')
+
+      auth_url = OneLogin::RubySaml::Authrequest.new.create(settings, { :RelayState => "http://example.com" })
+      assert auth_url.include?('&RelayState=http%3A%2F%2Fexample.com')
+
+      auth_url = OneLogin::RubySaml::Authrequest.new.create(settings, { 'RelayState' => nil })
+      assert !auth_url.include?('RelayState')
+
+      auth_url = OneLogin::RubySaml::Authrequest.new.create(settings, { 'RelayState' => "http://example.com" })
+      assert auth_url.include?('&RelayState=http%3A%2F%2Fexample.com')
+    end
+
     describe "when the target url is not set" do
       before do
         settings.idp_sso_target_url = nil
@@ -235,7 +249,7 @@ class RequestTest < Minitest::Test
         settings.certificate = ruby_saml_cert_text
         settings.private_key = ruby_saml_key_text
       end
-      
+
       it "create a signature parameter with RSA_SHA1 and validate it" do
         settings.security[:signature_method] = XMLSecurity::Document::RSA_SHA1
 
@@ -268,7 +282,7 @@ class RequestTest < Minitest::Test
 
         signature_algorithm = XMLSecurity::BaseDocument.new.algorithm(params['SigAlg'])
         assert_equal signature_algorithm, OpenSSL::Digest::SHA256
-        assert cert.public_key.verify(signature_algorithm.new, Base64.decode64(params['Signature']), query_string)        
+        assert cert.public_key.verify(signature_algorithm.new, Base64.decode64(params['Signature']), query_string)
       end
     end
 

test/slo_logoutresponse_test.rb

@@ -37,6 +37,20 @@ class SloLogoutresponseTest < Minitest::Test
       assert_match /&RelayState=http%3A%2F%2Fidp.example.com$/, unauth_url
     end
 
+    it "RelayState cases" do
+      unauth_url = OneLogin::RubySaml::SloLogoutresponse.new.create(settings, logout_request.id, nil, { :RelayState => nil })
+      assert !unauth_url.include?('RelayState')
+
+      unauth_url = OneLogin::RubySaml::SloLogoutresponse.new.create(settings, logout_request.id, nil, { :RelayState => "http://example.com" })
+      assert unauth_url.include?('&RelayState=http%3A%2F%2Fexample.com')
+
+      unauth_url = OneLogin::RubySaml::SloLogoutresponse.new.create(settings, logout_request.id, nil, { 'RelayState' => nil })
+      assert !unauth_url.include?('RelayState')
+
+      unauth_url = OneLogin::RubySaml::SloLogoutresponse.new.create(settings, logout_request.id, nil, { 'RelayState' => "http://example.com" })
+      assert unauth_url.include?('&RelayState=http%3A%2F%2Fexample.com')
+    end
+
     it "set InResponseTo to the ID from the logout request" do
       unauth_url = OneLogin::RubySaml::SloLogoutresponse.new.create(settings, logout_request.id)
 

test/test_helper.rb

@@ -117,15 +117,15 @@ def response_document_encrypted_nameid
   end
 
   def signed_message_encrypted_unsigned_assertion
-    @signed_message_encrypted_unsigned_assertion ||= File.read(File.join(File.dirname(__FILE__), 'responses', 'signed_message_encrypted_unsigned_assertion.xml.base64'))    
+    @signed_message_encrypted_unsigned_assertion ||= File.read(File.join(File.dirname(__FILE__), 'responses', 'signed_message_encrypted_unsigned_assertion.xml.base64'))
   end
 
   def signed_message_encrypted_signed_assertion
-    @signed_message_encrypted_signed_assertion ||= File.read(File.join(File.dirname(__FILE__), 'responses', 'signed_message_encrypted_signed_assertion.xml.base64'))    
+    @signed_message_encrypted_signed_assertion ||= File.read(File.join(File.dirname(__FILE__), 'responses', 'signed_message_encrypted_signed_assertion.xml.base64'))
   end
 
   def unsigned_message_encrypted_signed_assertion
-    @unsigned_message_encrypted_signed_assertion ||= File.read(File.join(File.dirname(__FILE__), 'responses', 'unsigned_message_encrypted_signed_assertion.xml.base64'))    
+    @unsigned_message_encrypted_signed_assertion ||= File.read(File.join(File.dirname(__FILE__), 'responses', 'unsigned_message_encrypted_signed_assertion.xml.base64'))
   end
 
   def unsigned_message_encrypted_unsigned_assertion
@@ -145,7 +145,7 @@ def signature_fingerprint_1
   end
 
   # certificate used on response_with_undefined_recipient
-  def signature_1  
+  def signature_1
     @signature1 ||= read_certificate("certificate1")
   end