Skip to content

Commit

Permalink
Merge pull request #575 from mentimeter/fix_slo_logoutresponse
Browse files Browse the repository at this point in the history
Fix error in 1.12 that breaks SloLogoutresponse and LogoutRequest
  • Loading branch information
pitbulk authored Apr 8, 2021
2 parents 79fc1d7 + 9186660 commit 39bd342
Show file tree
Hide file tree
Showing 6 changed files with 16 additions and 18 deletions.
8 changes: 4 additions & 4 deletions lib/onelogin/ruby-saml/logoutrequest.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,14 @@ def request_id
#
def create(settings, params={})
params = create_params(settings, params)
params_prefix = (settings.idp_slo_target_url =~ /\?/) ? '&' : '?'
params_prefix = (settings.idp_slo_service_url =~ /\?/) ? '&' : '?'
saml_request = CGI.escape(params.delete("SAMLRequest"))
request_params = "#{params_prefix}SAMLRequest=#{saml_request}"
params.each_pair do |key, value|
request_params << "&#{key.to_s}=#{CGI.escape(value.to_s)}"
end
raise SettingError.new "Invalid settings, idp_slo_target_url is not set!" if settings.idp_slo_target_url.nil? or settings.idp_slo_target_url.empty?
@logout_url = settings.idp_slo_target_url + request_params
raise SettingError.new "Invalid settings, idp_slo_service_url is not set!" if settings.idp_slo_service_url.nil? or settings.idp_slo_service_url.empty?
@logout_url = settings.idp_slo_service_url + request_params
end

# Creates the Get parameters for the logout request.
Expand Down Expand Up @@ -109,7 +109,7 @@ def create_xml_document(settings)
root.attributes['ID'] = uuid
root.attributes['IssueInstant'] = time
root.attributes['Version'] = "2.0"
root.attributes['Destination'] = settings.idp_slo_target_url unless settings.idp_slo_target_url.nil? or settings.idp_slo_target_url.empty?
root.attributes['Destination'] = settings.idp_slo_service_url unless settings.idp_slo_service_url.nil? or settings.idp_slo_service_url.empty?

if settings.sp_entity_id
issuer = root.add_element "saml:Issuer"
Expand Down
9 changes: 5 additions & 4 deletions lib/onelogin/ruby-saml/slo_logoutresponse.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,15 +36,15 @@ def response_id
#
def create(settings, request_id = nil, logout_message = nil, params = {}, logout_status_code = nil)
params = create_params(settings, request_id, logout_message, params, logout_status_code)
params_prefix = (settings.idp_slo_target_url =~ /\?/) ? '&' : '?'
url = settings.idp_slo_response_service_url || settings.idp_slo_target_url
params_prefix = (settings.idp_slo_service_url =~ /\?/) ? '&' : '?'
url = settings.idp_slo_response_service_url || settings.idp_slo_service_url
saml_response = CGI.escape(params.delete("SAMLResponse"))
response_params = "#{params_prefix}SAMLResponse=#{saml_response}"
params.each_pair do |key, value|
response_params << "&#{key.to_s}=#{CGI.escape(value.to_s)}"
end

raise SettingError.new "Invalid settings, idp_slo_target_url is not set!" if url.nil? or url.empty?
raise SettingError.new "Invalid settings, idp_slo_service_url is not set!" if url.nil? or url.empty?
@logout_url = url + response_params
end

Expand Down Expand Up @@ -117,7 +117,8 @@ def create_xml_document(settings, request_id = nil, logout_message = nil, status
response_doc = XMLSecurity::Document.new
response_doc.uuid = uuid

destination = settings.idp_slo_response_service_url || settings.idp_slo_target_url
destination = settings.idp_slo_response_service_url || settings.idp_slo_service_url


root = response_doc.add_element 'samlp:LogoutResponse', { 'xmlns:samlp' => 'urn:oasis:names:tc:SAML:2.0:protocol', "xmlns:saml" => "urn:oasis:names:tc:SAML:2.0:assertion" }
root.attributes['ID'] = uuid
Expand Down
8 changes: 4 additions & 4 deletions test/logoutrequest_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ class RequestTest < Minitest::Test
let(:settings) { OneLogin::RubySaml::Settings.new }

before do
settings.idp_slo_target_url = "http://unauth.com/logout"
settings.idp_slo_service_url = "http://unauth.com/logout"
settings.name_identifier_value = "f00f00"
end

Expand Down Expand Up @@ -43,7 +43,7 @@ class RequestTest < Minitest::Test
end

it "set sessionindex" do
settings.idp_slo_target_url = "http://example.com"
settings.idp_slo_service_url = "http://example.com"
sessionidx = OneLogin::RubySaml::Utils.uuid
settings.sessionindex = sessionidx

Expand Down Expand Up @@ -75,7 +75,7 @@ class RequestTest < Minitest::Test

describe "when the target url contains a query string" do
it "create the SAMLRequest parameter correctly" do
settings.idp_slo_target_url = "http://example.com?field=value"
settings.idp_slo_service_url = "http://example.com?field=value"

unauth_url = OneLogin::RubySaml::Logoutrequest.new.create(settings)
assert_match /^http:\/\/example.com\?field=value&SAMLRequest/, unauth_url
Expand All @@ -84,7 +84,7 @@ class RequestTest < Minitest::Test

describe "consumation of logout may need to track the transaction" do
it "have access to the request uuid" do
settings.idp_slo_target_url = "http://example.com?field=value"
settings.idp_slo_service_url = "http://example.com?field=value"

unauth_req = OneLogin::RubySaml::Logoutrequest.new
unauth_url = unauth_req.create(settings)
Expand Down
4 changes: 2 additions & 2 deletions test/logoutresponse_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ class RubySamlTest < Minitest::Test

before do
settings.soft = true
settings.idp_slo_target_url = "http://example.com?field=value"
settings.idp_slo_service_url = "http://example.com?field=value"
settings.security[:logout_responses_signed] = true
settings.security[:embed_sign] = false
settings.certificate = ruby_saml_cert_text
Expand Down Expand Up @@ -373,7 +373,7 @@ class RubySamlTest < Minitest::Test

before do
settings.soft = true
settings.idp_slo_target_url = "http://example.com?field=value"
settings.idp_slo_service_url = "http://example.com?field=value"
settings.security[:signature_method] = XMLSecurity::Document::RSA_SHA1
settings.security[:logout_responses_signed] = true
settings.security[:embed_sign] = false
Expand Down
3 changes: 0 additions & 3 deletions test/slo_logoutrequest_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ class RubySamlTest < Minitest::Test

it "collect errors when collect_errors=true" do
settings.idp_entity_id = 'http://idp.example.com/invalid'
settings.idp_slo_target_url = "http://example.com?field=value"
settings.security[:logout_requests_signed] = true
settings.security[:embed_sign] = false
settings.certificate = ruby_saml_cert_text
Expand Down Expand Up @@ -247,7 +246,6 @@ class RubySamlTest < Minitest::Test

describe "#validate_signature" do
before do
settings.idp_slo_target_url = "http://example.com?field=value"
settings.security[:logout_requests_signed] = true
settings.security[:embed_sign] = false
settings.certificate = ruby_saml_cert_text
Expand Down Expand Up @@ -406,7 +404,6 @@ class RubySamlTest < Minitest::Test

describe "#validate_signature with multiple idp certs" do
before do
settings.idp_slo_target_url = "http://example.com?field=value"
settings.certificate = ruby_saml_cert_text
settings.private_key = ruby_saml_key_text
settings.idp_cert = nil
Expand Down
2 changes: 1 addition & 1 deletion test/slo_logoutresponse_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ class SloLogoutresponseTest < Minitest::Test

before do
settings.idp_entity_id = 'https://app.onelogin.com/saml/metadata/SOMEACCOUNT'
settings.idp_slo_target_url = "http://unauth.com/logout"
settings.idp_slo_service_url = "http://unauth.com/logout"
settings.name_identifier_value = "f00f00"
settings.compress_request = true
settings.certificate = ruby_saml_cert_text
Expand Down

0 comments on commit 39bd342

Please sign in to comment.