Skip to content

Commit f04b0ee

Browse files
author
Alper Kokmen
committed
add Destination validation for ruby-saml/response
this change adds validation for Destination attribute which is optionally included in the SAML response. - response method to return Destination attribute via REXML - response method to validate the destination against consumer service URL available in settings. - unit tests. note that @pitbulk originally implemented these changes in #197; however, this specific validation didn't make it to master branch (or >= 1.0.0). i am hoping this will resolve #301.
1 parent 45665ba commit f04b0ee

2 files changed

Lines changed: 44 additions & 0 deletions

File tree

lib/onelogin/ruby-saml/response.rb

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,19 @@ def in_response_to
254254
end
255255
end
256256

257+
# @return [String|nil] Destination attribute from the SAML Response.
258+
#
259+
def destination
260+
@destination ||= begin
261+
node = REXML::XPath.first(
262+
document,
263+
"/p:Response",
264+
{ "p" => PROTOCOL }
265+
)
266+
node.nil? ? nil : node.attributes['Destination']
267+
end
268+
end
269+
257270
# @return [Array] The Audience elements from the Contitions of the SAML Response.
258271
#
259272
def audiences
@@ -295,6 +308,7 @@ def validate
295308
validate_in_response_to &&
296309
validate_conditions &&
297310
validate_audience &&
311+
validate_destination &&
298312
validate_issuer &&
299313
validate_session_expiration &&
300314
validate_subject_confirmation &&
@@ -461,6 +475,21 @@ def validate_audience
461475
true
462476
end
463477

478+
# Validates the Destination, (If the SAML Response is received where expected)
479+
# If fails, the error is added to the errors array
480+
# @return [Boolean] True if there is a Destination element that matches the Consumer Service URL, otherwise False
481+
#
482+
def validate_destination
483+
return true if destination.nil? || destination.empty? || settings.assertion_consumer_service_url.nil? || settings.assertion_consumer_service_url.empty?
484+
485+
unless destination == settings.assertion_consumer_service_url
486+
error_msg = "The response was received at #{destination} instead of #{settings.assertion_consumer_service_url}"
487+
return append_error(error_msg)
488+
end
489+
490+
true
491+
end
492+
464493
# Validates the Conditions. (If the response was initialized with the :skip_conditions option, this validation is skipped,
465494
# If the response was initialized with the :allowed_clock_drift option, the timing validations are relaxed by the allowed_clock_drift value)
466495
# @return [Boolean] True if satisfies the conditions, otherwise False if soft=True

test/response_test.rb

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -415,6 +415,21 @@ class RubySamlTest < Minitest::Test
415415
end
416416
end
417417

418+
describe "#validate_destination" do
419+
it "return true when the destination of the SAML Response matches the assertion consumer service url" do
420+
response.settings = settings
421+
assert response.send(:validate_destination)
422+
assert_empty response.errors
423+
end
424+
425+
it "return false when the destination of the SAML Response does not match the assertion consumer service url" do
426+
response.settings = settings
427+
response.settings.assertion_consumer_service_url = 'invalid_acs'
428+
assert !response.send(:validate_destination)
429+
assert_includes response.errors, "The response was received at #{response.destination} instead of #{response.settings.assertion_consumer_service_url}"
430+
end
431+
end
432+
418433
describe "#validate_issuer" do
419434
it "return true when the issuer of the Message/Assertion matches the IdP entityId" do
420435
response_valid_signed.settings = settings

0 commit comments

Comments
 (0)