Merge pull request #610 from johnnyshields/new-idp-binding-params

New IDP sso/slo binding params which deprecate :embed_sign

Author: pitbulk

README.md

@@ -2,8 +2,6 @@
 [![Build Status](https://github.com/onelogin/ruby-saml/actions/workflows/test.yml/badge.svg?query=branch%3Amaster)](https://github.com/onelogin/ruby-saml/actions/workflows/test.yml?query=branch%3Amaster)
 [![Coverage Status](https://coveralls.io/repos/onelogin/ruby-saml/badge.svg?branch=master)](https://coveralls.io/r/onelogin/ruby-saml?branch=master)
 
-## Breaking Changes
-
 Ruby SAML minor and tiny versions may introduce breaking changes. Please read
 [UPGRADING.md](UPGRADING.md) for guidance on upgrading to new Ruby SAML versions.
 
@@ -208,8 +206,10 @@ def saml_settings
   settings.assertion_consumer_service_url = "http://#{request.host}/saml/consume"
   settings.sp_entity_id                   = "http://#{request.host}/saml/metadata"
   settings.idp_entity_id                  = "https://app.onelogin.com/saml/metadata/#{OneLoginAppId}"
-  settings.idp_sso_service_url             = "https://app.onelogin.com/trust/saml2/http-post/sso/#{OneLoginAppId}"
-  settings.idp_slo_service_url             = "https://app.onelogin.com/trust/saml2/http-redirect/slo/#{OneLoginAppId}"
+  settings.idp_sso_service_url            = "https://app.onelogin.com/trust/saml2/http-post/sso/#{OneLoginAppId}"
+  settings.idp_sso_service_binding        = "urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST" # or :post, :redirect
+  settings.idp_slo_service_url            = "https://app.onelogin.com/trust/saml2/http-redirect/slo/#{OneLoginAppId}"
+  settings.idp_slo_service_binding        = "urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect" # or :post, :redirect
   settings.idp_cert_fingerprint           = OneLoginAppCertFingerPrint
   settings.idp_cert_fingerprint_algorithm = "http://www.w3.org/2000/09/xmldsig#sha1"
   settings.name_identifier_format         = "urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress"
@@ -222,9 +222,9 @@ def saml_settings
     "urn:oasis:names:tc:SAML:2.0:ac:classes:Password"
   ]
 
-  # Optional bindings (defaults to Redirect for logout POST for acs)
-  settings.single_logout_service_binding      = "urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect"
-  settings.assertion_consumer_service_binding = "urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST"
+  # Optional bindings (defaults to Redirect for logout POST for ACS)
+  settings.single_logout_service_binding      = "urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect" # or :post, :redirect
+  settings.assertion_consumer_service_binding = "urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST" # or :post, :redirect
 
   settings
 end
@@ -546,14 +546,12 @@ The settings related to sign are stored in the `security` attribute of the setti
   settings.security[:digest_method]    = XMLSecurity::Document::SHA1
   settings.security[:signature_method] = XMLSecurity::Document::RSA_SHA1
 
-  # Embeded signature or HTTP GET parameter signature
-  # Note that metadata signature is always embedded regardless of this value.
-  settings.security[:embed_sign] = false
   settings.security[:check_idp_cert_expiration] = false   # Enable or not IdP x509 cert expiration check
   settings.security[:check_sp_cert_expiration] = false   # Enable or not SP x509 cert expiration check
 ```
 
-Notice that the RelayState parameter is used when creating the Signature on the HTTP-Redirect Binding.
+Signatures will be handled for both `HTTP-Redirect` and `HTTP-Redirect` Bindings.
+Note that the RelayState parameter is used when creating the Signature on the `HTTP-Redirect` Binding.
 Remember to provide it to the Signature builder if you are sending a `GET RelayState` parameter or the
 signature validation process will fail at the Identity Provider.
 
@@ -604,7 +602,7 @@ def sp_logout_request
     delete_session
   else
 
-    logout_request = OneLogin::RubySaml::Logoutrequest.new()
+    logout_request = OneLogin::RubySaml::Logoutrequest.new
     logger.info "New SP SLO for userid '#{session[:userid]}' transactionid '#{logout_request.uuid}'"
 
     if settings.name_identifier_value.nil?
@@ -620,7 +618,7 @@ def sp_logout_request
     session[:transaction_id] = logout_request.uuid
     session[:logged_out_user] = logged_user
 
-    relayState =  url_for controller: 'saml', action: 'index'
+    relayState = url_for(controller: 'saml', action: 'index')
     redirect_to(logout_request.create(settings, :RelayState => relayState))
   end
 end

lib/onelogin/ruby-saml/authrequest.rb

@@ -73,7 +73,7 @@ def create_params(settings, params={})
         base64_request = encode(request)
         request_params = {"SAMLRequest" => base64_request}
 
-        if settings.security[:authn_requests_signed] && !settings.security[:embed_sign] && settings.private_key
+        if settings.idp_sso_service_binding == Utils::BINDINGS[:redirect] && settings.security[:authn_requests_signed] && settings.private_key
           params['SigAlg']    = settings.security[:signature_method]
           url_string = OneLogin::RubySaml::Utils.build_query(
             :type => 'SAMLRequest',
@@ -179,8 +179,7 @@ def create_xml_document(settings)
       end
 
       def sign_document(document, settings)
-        # embed signature
-        if settings.security[:authn_requests_signed] && settings.private_key && settings.certificate && settings.security[:embed_sign]
+        if settings.idp_sso_service_binding == Utils::BINDINGS[:post] && settings.security[:authn_requests_signed] && settings.private_key && settings.certificate
           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/idp_metadata_parser.rb

@@ -225,7 +225,9 @@ def to_hash(options = {})
             :idp_entity_id => @entity_id,
             :name_identifier_format => idp_name_id_format,
             :idp_sso_service_url => single_signon_service_url(options),
+            :idp_sso_service_binding => single_signon_service_binding(options[:sso_binding]),
             :idp_slo_service_url => single_logout_service_url(options),
+            :idp_slo_service_binding => single_logout_service_binding(options[:slo_binding]),
             :idp_slo_response_service_url => single_logout_response_service_url(options),
             :idp_attribute_names => attribute_names,
             :idp_cert => nil,
@@ -275,8 +277,8 @@ def single_signon_service_binding(binding_priority = nil)
           if binding_priority
             values = nodes.map(&:value)
             binding_priority.detect{ |binding| values.include? binding }
-          else
-            nodes.first.value if nodes.any?
+          elsif nodes.any?
+            nodes.first.value
           end
         end
 
@@ -307,8 +309,8 @@ def single_logout_service_binding(binding_priority = nil)
           if binding_priority
             values = nodes.map(&:value)
             binding_priority.detect{ |binding| values.include? binding }
-          else
-            nodes.first.value if nodes.any?
+          elsif nodes.any?
+            nodes.first.value
           end
         end
 

lib/onelogin/ruby-saml/logoutrequest.rb

@@ -70,8 +70,8 @@ def create_params(settings, params={})
         base64_request = encode(request)
         request_params = {"SAMLRequest" => base64_request}
 
-        if settings.security[:logout_requests_signed] && !settings.security[:embed_sign] && settings.private_key
-          params['SigAlg']    = settings.security[:signature_method]
+        if settings.idp_slo_service_binding == Utils::BINDINGS[:redirect] && settings.security[:logout_requests_signed] && settings.private_key
+          params['SigAlg'] = settings.security[:signature_method]
           url_string = OneLogin::RubySaml::Utils.build_query(
             :type => 'SAMLRequest',
             :data => base64_request,
@@ -138,7 +138,7 @@ def create_xml_document(settings)
 
       def sign_document(document, settings)
         # embed signature
-        if settings.security[:logout_requests_signed] && settings.private_key && settings.certificate && settings.security[:embed_sign]
+        if settings.idp_slo_service_binding == Utils::BINDINGS[:post] && settings.security[:logout_requests_signed] && settings.private_key && settings.certificate
           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/saml_message.rb

@@ -16,8 +16,8 @@ module RubySaml
     class SamlMessage
       include REXML
 
-      ASSERTION = "urn:oasis:names:tc:SAML:2.0:assertion"
-      PROTOCOL  = "urn:oasis:names:tc:SAML:2.0:protocol"
+      ASSERTION = "urn:oasis:names:tc:SAML:2.0:assertion".freeze
+      PROTOCOL  = "urn:oasis:names:tc:SAML:2.0:protocol".freeze
 
       BASE64_FORMAT = %r(\A([A-Za-z0-9+/]{4})*([A-Za-z0-9+/]{2}==|[A-Za-z0-9+/]{3}=)?\Z)
       @@mutex = Mutex.new

lib/onelogin/ruby-saml/settings.rb

@@ -31,9 +31,8 @@ def initialize(overrides = {}, keep_security_attributes = false)
 
       # IdP Data
       attr_accessor :idp_entity_id
-
-      attr_accessor :idp_sso_service_url
-      attr_accessor :idp_slo_service_url
+      attr_writer   :idp_sso_service_url
+      attr_writer   :idp_slo_service_url
       attr_accessor :idp_slo_response_service_url
       attr_accessor :idp_cert
       attr_accessor :idp_cert_fingerprint
@@ -43,8 +42,10 @@ def initialize(overrides = {}, keep_security_attributes = false)
       attr_accessor :idp_name_qualifier
       attr_accessor :valid_until
       # SP Data
+      attr_writer   :sp_entity_id
       attr_accessor :assertion_consumer_service_url
-      attr_accessor :assertion_consumer_service_binding
+      attr_reader   :assertion_consumer_service_binding
+      attr_writer   :single_logout_service_url
       attr_accessor :sp_name_qualifier
       attr_accessor :name_identifier_format
       attr_accessor :name_identifier_value
@@ -54,7 +55,7 @@ def initialize(overrides = {}, keep_security_attributes = false)
       attr_accessor :compress_response
       attr_accessor :double_quote_xml_attribute_values
       attr_accessor :passive
-      attr_accessor :protocol_binding
+      attr_reader   :protocol_binding
       attr_accessor :attributes_index
       attr_accessor :force_authn
       attr_accessor :certificate
@@ -67,104 +68,99 @@ def initialize(overrides = {}, keep_security_attributes = false)
       # Work-flow
       attr_accessor :security
       attr_accessor :soft
-      # Compability
+      # Deprecated
       attr_accessor :assertion_consumer_logout_service_url
-      attr_accessor :assertion_consumer_logout_service_binding
+      attr_reader   :assertion_consumer_logout_service_binding
       attr_accessor :issuer
       attr_accessor :idp_sso_target_url
       attr_accessor :idp_slo_target_url
 
       # @return [String] IdP Single Sign On Service URL
       #
       def idp_sso_service_url
-        val = nil
-        if @idp_sso_service_url.nil?
-          if @idp_sso_target_url
-            val = @idp_sso_target_url
-          end
-        else
-          val = @idp_sso_service_url
-        end
-        val
+        @idp_sso_service_url || @idp_sso_target_url
       end
 
       # @return [String] IdP Single Logout Service URL
       #
       def idp_slo_service_url
-        val = nil
-        if @idp_slo_service_url.nil?
-          if @idp_slo_target_url
-            val = @idp_slo_target_url
-          end
-        else
-          val = @idp_slo_service_url
-        end
-        val
+        @idp_slo_service_url || @idp_slo_target_url
+      end
+
+      # @return [String] IdP Single Sign On Service Binding
+      #
+      def idp_sso_service_binding
+        @idp_sso_service_binding || idp_binding_from_embed_sign
+      end
+
+      # Setter for IdP Single Sign On Service Binding
+      # @param value [String, Symbol].
+      #
+      def idp_sso_service_binding=(value)
+        @idp_sso_service_binding = get_binding(value)
+      end
+
+      # @return [String] IdP Single Logout Service Binding
+      #
+      def idp_slo_service_binding
+        @idp_slo_service_binding || idp_binding_from_embed_sign
+      end
+
+      # Setter for IdP Single Logout Service Binding
+      # @param value [String, Symbol].
+      #
+      def idp_slo_service_binding=(value)
+        @idp_slo_service_binding = get_binding(value)
       end
 
       # @return [String] SP Entity ID
       #
       def sp_entity_id
-        val = nil
-        if @sp_entity_id.nil?
-          if @issuer
-            val = @issuer
-          end
-        else
-          val = @sp_entity_id
-        end
-        val
+        @sp_entity_id || @issuer
       end
 
-      # Setter for SP Entity ID.
-      # @param val [String].
+      # Setter for SP Protocol Binding
+      # @param value [String, Symbol].
       #
-      def sp_entity_id=(val)
-        @sp_entity_id = val
+      def protocol_binding=(value)
+        @protocol_binding = get_binding(value)
       end
 
-      # @return [String] Single Logout Service URL.
+      # Setter for SP Assertion Consumer Service Binding
+      # @param value [String, Symbol].
       #
-      def single_logout_service_url
-        val = nil
-        if @single_logout_service_url.nil?
-          if @assertion_consumer_logout_service_url
-            val = @assertion_consumer_logout_service_url
-          end
-        else
-          val = @single_logout_service_url
-        end
-        val
+      def assertion_consumer_service_binding=(value)
+        @assertion_consumer_service_binding = get_binding(value)
       end
 
-      # Setter for the Single Logout Service URL.
-      # @param url [String].
+      # @return [String] Single Logout Service URL.
       #
-      def single_logout_service_url=(url)
-        @single_logout_service_url = url
+      def single_logout_service_url
+        @single_logout_service_url || @assertion_consumer_logout_service_url
       end
 
       # @return [String] Single Logout Service Binding.
       #
       def single_logout_service_binding
-        val = nil
-        if @single_logout_service_binding.nil?
-          if @assertion_consumer_logout_service_binding
-            val = @assertion_consumer_logout_service_binding
-          end
-        else
-          val = @single_logout_service_binding
-        end
-        val
+        @single_logout_service_binding || @assertion_consumer_logout_service_binding
       end
 
       # Setter for Single Logout Service Binding.
       #
       # (Currently we only support "urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect")
-      # @param url [String]
+      # @param value [String, Symbol]
       #
-      def single_logout_service_binding=(url)
-        @single_logout_service_binding = url
+      def single_logout_service_binding=(value)
+        @single_logout_service_binding = get_binding(value)
+      end
+
+      # @deprecated Setter for legacy Single Logout Service Binding parameter.
+      #
+      # (Currently we only support "urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect")
+      # @param value [String, Symbol]
+      #
+      def assertion_consumer_logout_service_binding=(value)
+        @assertion_consumer_logout_service_binding = get_binding(value)
       end
 
       # Calculates the fingerprint of the IdP x509 certificate.
@@ -252,9 +248,19 @@ def get_sp_key
 
       private
 
+      def idp_binding_from_embed_sign
+        security[:embed_sign] ? Utils::BINDINGS[:post] : Utils::BINDINGS[:redirect]
+      end
+
+      def get_binding(value)
+        return unless value
+
+        Utils::BINDINGS[value.to_sym] || value
+      end
+
       DEFAULTS = {
-        :assertion_consumer_service_binding        => "urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST".freeze,
-        :single_logout_service_binding             => "urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect".freeze,
+        :assertion_consumer_service_binding        => Utils::BINDINGS[:post],
+        :single_logout_service_binding             => Utils::BINDINGS[:redirect],
         :idp_cert_fingerprint_algorithm            => XMLSecurity::Document::SHA1,
         :compress_request                          => true,
         :compress_response                         => true,
@@ -268,7 +274,7 @@ def get_sp_key
           :want_assertions_encrypted  => false,
           :want_name_id               => false,
           :metadata_signed            => false,
-          :embed_sign                 => false,
+          :embed_sign                 => false, # Deprecated
           :digest_method              => XMLSecurity::Document::SHA1,
           :signature_method           => XMLSecurity::Document::RSA_SHA1,
           :check_idp_cert_expiration  => false,

lib/onelogin/ruby-saml/slo_logoutresponse.rb

@@ -79,7 +79,7 @@ def create_params(settings, request_id = nil, logout_message = nil, params = {},
         base64_response = encode(response)
         response_params = {"SAMLResponse" => base64_response}
 
-        if settings.security[:logout_responses_signed] && !settings.security[:embed_sign] && settings.private_key
+        if settings.idp_slo_service_binding == Utils::BINDINGS[:redirect] && settings.security[:logout_responses_signed] && settings.private_key
           params['SigAlg']    = settings.security[:signature_method]
           url_string = OneLogin::RubySaml::Utils.build_query(
             :type => 'SAMLResponse',
@@ -150,7 +150,7 @@ def create_xml_document(settings, request_id = nil, logout_message = nil, status
 
       def sign_document(document, settings)
         # embed signature
-        if settings.security[:logout_responses_signed] && settings.private_key && settings.certificate && settings.security[:embed_sign]
+        if settings.idp_slo_service_binding == Utils::BINDINGS[:post] && settings.private_key && settings.certificate
           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])