Skip to content
12 changes: 9 additions & 3 deletions lib/onelogin/ruby-saml/authrequest.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,13 @@ def request_id
end

# Creates the AuthNRequest string.
# @param settings [OneLogin::RubySaml::Settings|nil] Toolkit settings
# @param settings [OneLogin::RubySaml::Settings] Toolkit settings
# @param params [Hash] Some extra parameters to be added in the GET for example the RelayState
# @return [String] AuthNRequest string that includes the SAMLRequest
#
def create(settings, params = {})
raise ArgumentError, "Invalid settings, settings should not be nil!" if settings.nil?

params = create_params(settings, params)
params_prefix = (settings.idp_sso_service_url =~ /\?/) ? '&' : '?'
saml_request = CGI.escape(params.delete("SAMLRequest"))
Expand All @@ -46,11 +48,13 @@ def create(settings, params = {})
end

# Creates the Get parameters for the request.
# @param settings [OneLogin::RubySaml::Settings|nil] Toolkit settings
# @param settings [OneLogin::RubySaml::Settings] Toolkit settings
# @param params [Hash] Some extra parameters to be added in the GET for example the RelayState
# @return [Hash] Parameters
#
def create_params(settings, params={})
raise ArgumentError, "Invalid settings, settings should not be nil!" if settings.nil?

# The method expects :RelayState but sometimes we get 'RelayState' instead.
# Based on the HashWithIndifferentAccess value in Rails we could experience
# conflicts so this line will solve them.
Expand Down Expand Up @@ -95,10 +99,12 @@ def create_params(settings, params={})
end

# Creates the SAMLRequest String.
# @param settings [OneLogin::RubySaml::Settings|nil] Toolkit settings
# @param settings [OneLogin::RubySaml::Settings] Toolkit settings
# @return [String] The SAMLRequest String.
#
def create_authentication_xml_doc(settings)
raise ArgumentError, "Invalid settings, settings should not be nil!" if settings.nil?

document = create_xml_document(settings)
sign_document(document, settings)
end
Expand Down
12 changes: 9 additions & 3 deletions lib/onelogin/ruby-saml/logoutrequest.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,13 @@ def request_id
end

# Creates the Logout Request string.
# @param settings [OneLogin::RubySaml::Settings|nil] Toolkit settings
# @param settings [OneLogin::RubySaml::Settings] Toolkit settings
# @param params [Hash] Some extra parameters to be added in the GET for example the RelayState
# @return [String] Logout Request string that includes the SAMLRequest
#
def create(settings, params={})
raise ArgumentError, "Invalid settings, settings should not be nil!" if settings.nil?

params = create_params(settings, params)
params_prefix = (settings.idp_slo_service_url =~ /\?/) ? '&' : '?'
saml_request = CGI.escape(params.delete("SAMLRequest"))
Expand All @@ -43,11 +45,13 @@ def create(settings, params={})
end

# Creates the Get parameters for the logout request.
# @param settings [OneLogin::RubySaml::Settings|nil] Toolkit settings
# @param settings [OneLogin::RubySaml::Settings] Toolkit settings
# @param params [Hash] Some extra parameters to be added in the GET for example the RelayState
# @return [Hash] Parameters
#
def create_params(settings, params={})
raise ArgumentError, "Invalid settings, settings should not be nil!" if settings.nil?

# The method expects :RelayState but sometimes we get 'RelayState' instead.
# Based on the HashWithIndifferentAccess value in Rails we could experience
# conflicts so this line will solve them.
Expand Down Expand Up @@ -92,10 +96,12 @@ def create_params(settings, params={})
end

# Creates the SAMLRequest String.
# @param settings [OneLogin::RubySaml::Settings|nil] Toolkit settings
# @param settings [OneLogin::RubySaml::Settings] Toolkit settings
# @return [String] The SAMLRequest String.
#
def create_logout_request_xml_doc(settings)
raise ArgumentError, "Invalid settings, settings should not be nil!" if settings.nil?

document = create_xml_document(settings)
sign_document(document, settings)
end
Expand Down
4 changes: 3 additions & 1 deletion lib/onelogin/ruby-saml/metadata.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,16 @@ module RubySaml
class Metadata

# Return SP metadata based on the settings.
# @param settings [OneLogin::RubySaml::Settings|nil] Toolkit settings
# @param settings [OneLogin::RubySaml::Settings] Toolkit settings
# @param pretty_print [Boolean] Pretty print or not the response
# (No pretty print if you are going to validate the signature)
# @param valid_until [DateTime] Metadata's valid time
# @param cache_duration [Integer] Duration of the cache in seconds
# @return [String] XML Metadata of the Service Provider
#
def generate(settings, pretty_print=false, valid_until=nil, cache_duration=nil)
raise ArgumentError, "Invalid settings, settings should not be nil!" if settings.nil?

meta_doc = XMLSecurity::Document.new
add_xml_declaration(meta_doc)
root = add_root_element(meta_doc, settings, valid_until, cache_duration)
Expand Down
11 changes: 0 additions & 11 deletions lib/onelogin/ruby-saml/saml_message.rb
Original file line number Diff line number Diff line change
Expand Up @@ -100,17 +100,6 @@ def decode_raw_saml(saml, settings = nil)
saml
end

# Deflate, base64 encode and url-encode a SAML Message (To be used in the HTTP-redirect binding)
# @param saml [String] The plain SAML Message
# @param settings [OneLogin::RubySaml::Settings|nil] Toolkit settings
# @return [String] The deflated and encoded SAML Message (encoded if the compression is requested)
#
def encode_raw_saml(saml, settings)
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found this dead code. What do you think about me deleting this code because I thought it was related to these changes? As far as I checked, this code isn’t called from anywhere and is a private method.

saml = deflate(saml) if settings.compress_request

CGI.escape(encode(saml))
end

# Base 64 decode method
# @param string [String] The string message
# @return [String] The decoded string
Expand Down
12 changes: 9 additions & 3 deletions lib/onelogin/ruby-saml/slo_logoutresponse.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,16 @@ def response_id
end

# Creates the Logout Response string.
# @param settings [OneLogin::RubySaml::Settings|nil] Toolkit settings
# @param settings [OneLogin::RubySaml::Settings] Toolkit settings
# @param request_id [String] The ID of the LogoutRequest sent by this SP to the IdP. That ID will be placed as the InResponseTo in the logout response
# @param logout_message [String] The Message to be placed as StatusMessage in the logout response
# @param params [Hash] Some extra parameters to be added in the GET for example, the RelayState
# @param logout_status_code [String] The StatusCode to be placed as StatusMessage in the logout response
# @return [String] Logout Request string that includes the SAMLRequest
#
def create(settings, request_id = nil, logout_message = nil, params = {}, logout_status_code = nil)
raise ArgumentError, "Invalid settings, settings should not be nil!" if settings.nil?

params = create_params(settings, request_id, logout_message, params, logout_status_code)
params_prefix = (settings.idp_slo_service_url =~ /\?/) ? '&' : '?'
url = settings.idp_slo_response_service_url || settings.idp_slo_service_url
Expand All @@ -49,14 +51,16 @@ def create(settings, request_id = nil, logout_message = nil, params = {}, logout
end

# Creates the Get parameters for the logout response.
# @param settings [OneLogin::RubySaml::Settings|nil] Toolkit settings
# @param settings [OneLogin::RubySaml::Settings] Toolkit settings
# @param request_id [String] The ID of the LogoutRequest sent by this SP to the IdP. That ID will be placed as the InResponseTo in the logout response
# @param logout_message [String] The Message to be placed as StatusMessage in the logout response
# @param params [Hash] Some extra parameters to be added in the GET for example, the RelayState
# @param logout_status_code [String] The StatusCode to be placed as StatusMessage in the logout response
# @return [Hash] Parameters
#
def create_params(settings, request_id = nil, logout_message = nil, params = {}, logout_status_code = nil)
raise ArgumentError, "Invalid settings, settings should not be nil!" if settings.nil?

# The method expects :RelayState but sometimes we get 'RelayState' instead.
# Based on the HashWithIndifferentAccess value in Rails we could experience
# conflicts so this line will solve them.
Expand Down Expand Up @@ -101,13 +105,15 @@ def create_params(settings, request_id = nil, logout_message = nil, params = {},
end

# Creates the SAMLResponse String.
# @param settings [OneLogin::RubySaml::Settings|nil] Toolkit settings
# @param settings [OneLogin::RubySaml::Settings] Toolkit settings
# @param request_id [String] The ID of the LogoutRequest sent by this SP to the IdP. That ID will be placed as the InResponseTo in the logout response
# @param logout_message [String] The Message to be placed as StatusMessage in the logout response
# @param logout_status_code [String] The StatusCode to be placed as StatusMessage in the logout response
# @return [String] The SAMLResponse String.
#
def create_logout_response_xml_doc(settings, request_id = nil, logout_message = nil, logout_status_code = nil)
raise ArgumentError, "Invalid settings, settings should not be nil!" if settings.nil?

document = create_xml_document(settings, request_id, logout_message, logout_status_code)
sign_document(document, settings)
end
Expand Down
23 changes: 23 additions & 0 deletions test/logoutrequest_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,15 @@ class RequestTest < Minitest::Test
assert_match %r(#{name_identifier_value}</saml:NameID>), inflated
end

describe "when the settings is nil" do
it "raises an error with a descriptive message" do
err = assert_raises ArgumentError do
OneLogin::RubySaml::Logoutrequest.new.create(nil)
end
assert_match(/settings should not be nil/, err.message)
end
end

describe "when the target url doesn't contain a query string" do
it "create the SAMLRequest parameter correctly" do
unauth_url = OneLogin::RubySaml::Logoutrequest.new.create(settings)
Expand Down Expand Up @@ -109,6 +118,20 @@ class RequestTest < Minitest::Test
end
end

it "raises error when settings is nil on create_params" do
err = assert_raises ArgumentError do
OneLogin::RubySaml::Logoutrequest.new.create_params(nil)
end
assert_match(/settings should not be nil/, err.message)
end

it "raises error when settings is nil on create_logout_request_xml_doc" do
err = assert_raises ArgumentError do
OneLogin::RubySaml::Logoutrequest.new.create_logout_request_xml_doc(nil)
end
assert_match(/settings should not be nil/, err.message)
end

describe "signing with HTTP-POST binding" do
before do
settings.security[:logout_requests_signed] = true
Expand Down
7 changes: 7 additions & 0 deletions test/metadata_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,13 @@ class MetadataTest < Minitest::Test
assert_equal "PT604800S", REXML::XPath.first(doc_metadata, "//md:EntityDescriptor").attribute("cacheDuration").value
end

it "raises error when the settings is nil" do
err = assert_raises ArgumentError do
OneLogin::RubySaml::Metadata.new.generate(nil)
end
assert_match(/settings should not be nil/, err.message)
end

describe "WantAssertionsSigned" do
it "generates Service Provider Metadata with WantAssertionsSigned = false" do
settings.security[:want_assertions_signed] = false
Expand Down
25 changes: 25 additions & 0 deletions test/request_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,15 @@ class RequestTest < Minitest::Test
OneLogin::RubySaml::Utils::set_prefix("_")
end

describe "when the settings is nil" do
it "raises an error with a descriptive message" do
err = assert_raises ArgumentError do
OneLogin::RubySaml::Authrequest.new.create(nil)
end
assert_match(/settings should not be nil/, err.message)
end
end

describe "when the target url is not set" do
before do
settings.idp_sso_service_url = nil
Expand Down Expand Up @@ -240,6 +249,15 @@ class RequestTest < Minitest::Test
assert_match(/<saml:AuthnContextDeclRef>urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport<\/saml:AuthnContextDeclRef>/, auth_doc.to_s)
end

describe "#create_params signing with nil settings" do
it "raises ArgumentError" do
err = assert_raises ArgumentError do
OneLogin::RubySaml::Authrequest.new.create_params(nil)
end
assert_match(/settings should not be nil/, err.message)
end
end

describe "#create_params signing with HTTP-POST binding" do
before do
settings.compress_request = false
Expand Down Expand Up @@ -430,6 +448,13 @@ class RequestTest < Minitest::Test
assert auth_doc.to_s =~ /<saml:AuthnContextDeclRef>example\/decl\/ref<\/saml:AuthnContextDeclRef>/
end

it "raises error when settings is nil" do
err = assert_raises ArgumentError do
OneLogin::RubySaml::Authrequest.new.create_authentication_xml_doc(nil)
end
assert_match(/settings should not be nil/, err.message)
end

describe "DEPRECATED: #create_params signing with HTTP-POST binding via :embed_sign" do
before do
settings.compress_request = false
Expand Down
11 changes: 0 additions & 11 deletions test/saml_message_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,6 @@ class RubySamlTest < Minitest::Test
assert logout_request_document, decoded_raw
end

it "return encoded raw saml" do
settings.compress_request = true
encoded_raw = saml_message.send(:encode_raw_saml, logout_request_document, settings)
assert logout_request_deflated_base64, encoded_raw

settings.compress_request = false
deflated = saml_message.send(:deflate, logout_request_deflated_base64)
encoded_raw = saml_message.send(:encode_raw_saml, deflated, settings)
assert logout_request_deflated_base64, encoded_raw
end

it "return decoded string" do
decoded = saml_message.send(:decode, response_document)
assert response_document_xml, decoded
Expand Down
21 changes: 21 additions & 0 deletions test/slo_logoutresponse_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,13 @@ class SloLogoutresponseTest < Minitest::Test
assert_match(/Destination='http:\/\/unauth.com\/logout\/return'/, inflated)
end

it "raises error when the settings is nil" do
err = assert_raises ArgumentError do
OneLogin::RubySaml::SloLogoutresponse.new.create(nil)
end
assert_match(/settings should not be nil/, err.message)
end

describe "playgin with preix" do
it "creates request with ID prefixed with default '_'" do
request = OneLogin::RubySaml::SloLogoutresponse.new
Expand All @@ -98,6 +105,20 @@ class SloLogoutresponseTest < Minitest::Test
end
end

it "raises error when settings is nil on create_params" do
err = assert_raises ArgumentError do
OneLogin::RubySaml::SloLogoutresponse.new.create_params(nil)
end
assert_match(/settings should not be nil/, err.message)
end

it "raises error when settings is nil on create_logout_response_xml_doc" do
err = assert_raises ArgumentError do
OneLogin::RubySaml::SloLogoutresponse.new.create_logout_response_xml_doc(nil)
end
assert_match(/settings should not be nil/, err.message)
end

describe "signing with HTTP-POST binding" do
before do
settings.idp_sso_service_binding = :redirect
Expand Down