Merge pull request #302 from alperkokmen/destination-validation-for-saml-response

add Destination validation for ruby-saml/response

Author: pitbulk

lib/onelogin/ruby-saml/response.rb

@@ -254,6 +254,19 @@ def in_response_to
         end
       end
 
+      # @return [String|nil] Destination attribute from the SAML Response.
+      #
+      def destination
+        @destination ||= begin
+          node = REXML::XPath.first(
+            document,
+            "/p:Response",
+            { "p" => PROTOCOL }
+          )
+          node.nil? ? nil : node.attributes['Destination']
+        end
+      end
+
       # @return [Array] The Audience elements from the Contitions of the SAML Response.
       #
       def audiences
@@ -295,6 +308,7 @@ def validate
         validate_in_response_to &&
         validate_conditions &&
         validate_audience &&
+        validate_destination &&
         validate_issuer &&
         validate_session_expiration &&
         validate_subject_confirmation &&
@@ -461,6 +475,21 @@ def validate_audience
         true
       end
 
+      # Validates the Destination, (If the SAML Response is received where expected)
+      # If fails, the error is added to the errors array
+      # @return [Boolean] True if there is a Destination element that matches the Consumer Service URL, otherwise False
+      #
+      def validate_destination
+        return true if destination.nil? || destination.empty? || settings.assertion_consumer_service_url.nil? || settings.assertion_consumer_service_url.empty?
+
+        unless destination == settings.assertion_consumer_service_url
+          error_msg = "The response was received at #{destination} instead of #{settings.assertion_consumer_service_url}"
+          return append_error(error_msg)
+        end
+
+        true
+      end
+
       # Validates the Conditions. (If the response was initialized with the :skip_conditions option, this validation is skipped,
       # If the response was initialized with the :allowed_clock_drift option, the timing validations are relaxed by the allowed_clock_drift value)
       # @return [Boolean] True if satisfies the conditions, otherwise False if soft=True

test/response_test.rb

@@ -415,6 +415,21 @@ class RubySamlTest < Minitest::Test
       end
     end
 
+    describe "#validate_destination" do
+      it "return true when the destination of the SAML Response matches the assertion consumer service url" do
+        response.settings = settings
+        assert response.send(:validate_destination)
+        assert_empty response.errors
+      end
+
+      it "return false when the destination of the SAML Response does not match the assertion consumer service url" do
+        response.settings = settings
+        response.settings.assertion_consumer_service_url = 'invalid_acs'
+        assert !response.send(:validate_destination)
+        assert_includes response.errors, "The response was received at #{response.destination} instead of #{response.settings.assertion_consumer_service_url}"
+      end
+    end
+
     describe "#validate_issuer" do
       it "return true when the issuer of the Message/Assertion matches the IdP entityId" do
         response_valid_signed.settings = settings