Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

v2.0: Remove settings.compress_request and compess_response parameters #689

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
* [#685](https://github.com/SAML-Toolkits/ruby-saml/pull/685) Change directly structure from `lib/onelogin/ruby-saml` to `lib/ruby_saml`.
* [#685](https://github.com/SAML-Toolkits/ruby-saml/pull/685) Move schema files from `lib/onelogin/schemas` to `lib/ruby_saml/schemas`.
* [#686](https://github.com/SAML-Toolkits/ruby-saml/pull/686) Use SHA-256 as the default hashing algorithm everywhere instead of SHA-1, including signatures, fingerprints, and digests.
* [#689](https://github.com/SAML-Toolkits/ruby-saml/pull/689) Remove `settings.compress_request` and `settings.compess_response` parameters.

### 1.17.0
* [#673](https://github.com/SAML-Toolkits/ruby-saml/pull/673) Add `Settings#sp_cert_multi` paramter to facilitate SP certificate and key rotation.
Expand Down
9 changes: 9 additions & 0 deletions UPGRADING.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,15 @@ settings.security[:digest_method] = XMLSecurity::Document::SHA1
settings.security[:signature_method] = XMLSecurity::Document::RSA_SHA1
```

### Removal of Compression Settings

The `settings.compress_request` and `settings.compress_response` parameters have been removed.
Please remove them everywhere within your project code. These behaviors are now set automatically
according to the `settings.idp_sso_service_binding` and `settings.idp_slo_service_binding` parameters
respectively: `HTTP-Redirect` will always use compression, while `HTTP-POST` will not. For clarity,
here "compression" is used to make redirect URLs which contain SAML messages be shorter. For POST
messages, compression may be achieved by enabling `Content-Encoding: gzip` on your webserver.

## Updating from 1.12.x to 1.13.0

Version `1.13.0` adds `settings.idp_sso_service_binding` and `settings.idp_slo_service_binding`, and
Expand Down
5 changes: 3 additions & 2 deletions lib/ruby_saml/authrequest.rb
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ def create_params(settings, params={})
# 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.
binding_redirect = settings.idp_sso_service_binding == Utils::BINDINGS[:redirect]
relay_state = params[:RelayState] || params['RelayState']

if relay_state.nil?
Expand All @@ -71,12 +72,12 @@ def create_params(settings, params={})

Logging.debug "Created AuthnRequest: #{request}"

request = deflate(request) if settings.compress_request
request = deflate(request) if binding_redirect
base64_request = encode(request)
request_params = {"SAMLRequest" => base64_request}
sp_signing_key = settings.get_sp_signing_key

if settings.idp_sso_service_binding == Utils::BINDINGS[:redirect] && settings.security[:authn_requests_signed] && sp_signing_key
if binding_redirect && settings.security[:authn_requests_signed] && sp_signing_key
params['SigAlg'] = settings.security[:signature_method]
url_string = RubySaml::Utils.build_query(
type: 'SAMLRequest',
Expand Down
5 changes: 3 additions & 2 deletions lib/ruby_saml/logoutrequest.rb
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ def create_params(settings, params={})
# 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.
binding_redirect = settings.idp_slo_service_binding == Utils::BINDINGS[:redirect]
relay_state = params[:RelayState] || params['RelayState']

if relay_state.nil?
Expand All @@ -68,12 +69,12 @@ def create_params(settings, params={})

Logging.debug "Created SLO Logout Request: #{request}"

request = deflate(request) if settings.compress_request
request = deflate(request) if binding_redirect
base64_request = encode(request)
request_params = {"SAMLRequest" => base64_request}
sp_signing_key = settings.get_sp_signing_key

if settings.idp_slo_service_binding == Utils::BINDINGS[:redirect] && settings.security[:logout_requests_signed] && sp_signing_key
if binding_redirect && settings.security[:logout_requests_signed] && sp_signing_key
params['SigAlg'] = settings.security[:signature_method]
url_string = RubySaml::Utils.build_query(
type: 'SAMLRequest',
Expand Down
6 changes: 3 additions & 3 deletions lib/ruby_saml/saml_message.rb
Original file line number Diff line number Diff line change
Expand Up @@ -104,11 +104,11 @@ def decode_raw_saml(saml, settings = nil)

# 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 [RubySaml::Settings|nil] Toolkit settings
# @param compress [true|false] Whether or not the SAML should be deflated.
# @return [String] The deflated and encoded SAML Message (encoded if the compression is requested)
#
def encode_raw_saml(saml, settings)
saml = deflate(saml) if settings.compress_request
def encode_raw_saml(saml, compress = false)
saml = deflate(saml) if compress

CGI.escape(encode(saml))
end
Expand Down
4 changes: 0 additions & 4 deletions lib/ruby_saml/settings.rb
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,6 @@ def initialize(overrides = {}, keep_security_attributes = false)
attr_accessor :name_identifier_value
attr_accessor :name_identifier_value_requested
attr_accessor :sessionindex
attr_accessor :compress_request
attr_accessor :compress_response
attr_accessor :double_quote_xml_attribute_values
attr_accessor :message_max_bytesize
attr_accessor :passive
Expand Down Expand Up @@ -278,8 +276,6 @@ def get_binding(value)
assertion_consumer_service_binding: Utils::BINDINGS[:post],
single_logout_service_binding: Utils::BINDINGS[:redirect],
idp_cert_fingerprint_algorithm: XMLSecurity::Document::SHA256,
compress_request: true,
compress_response: true,
message_max_bytesize: 250_000,
soft: true,
double_quote_xml_attribute_values: false,
Expand Down
5 changes: 3 additions & 2 deletions lib/ruby_saml/slo_logoutresponse.rb
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ def create_params(settings, request_id = nil, logout_message = nil, params = {},
# 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.
binding_redirect = settings.idp_slo_service_binding == Utils::BINDINGS[:redirect]
relay_state = params[:RelayState] || params['RelayState']

if relay_state.nil?
Expand All @@ -77,12 +78,12 @@ def create_params(settings, request_id = nil, logout_message = nil, params = {},

Logging.debug "Created SLO Logout Response: #{response}"

response = deflate(response) if settings.compress_response
response = deflate(response) if binding_redirect
base64_response = encode(response)
response_params = {"SAMLResponse" => base64_response}
sp_signing_key = settings.get_sp_signing_key

if settings.idp_slo_service_binding == Utils::BINDINGS[:redirect] && settings.security[:logout_responses_signed] && sp_signing_key
if binding_redirect && settings.security[:logout_responses_signed] && sp_signing_key
params['SigAlg'] = settings.security[:signature_method]
url_string = RubySaml::Utils.build_query(
type: 'SAMLResponse',
Expand Down
25 changes: 2 additions & 23 deletions test/logoutrequest_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -152,21 +152,7 @@ class RequestTest < Minitest::Test
assert_match %r[<ds:DigestMethod Algorithm='http://www.w3.org/2001/04/xmlenc#sha256'/>], inflated
end

it "create a signed logout request" do
settings.compress_request = true

unauth_req = RubySaml::Logoutrequest.new
unauth_url = unauth_req.create(settings)

inflated = decode_saml_request_payload(unauth_url)
assert_match %r[<ds:SignatureValue>([a-zA-Z0-9/+=]+)</ds:SignatureValue>], inflated
assert_match %r[<ds:SignatureMethod Algorithm='http://www.w3.org/2001/04/xmldsig-more#rsa-sha256'/>], inflated
assert_match %r[<ds:DigestMethod Algorithm='http://www.w3.org/2001/04/xmlenc#sha256'/>], inflated
end

it "create an uncompressed signed logout request" do
settings.compress_request = false

params = RubySaml::Logoutrequest.new.create_params(settings)
request_xml = Base64.decode64(params["SAMLRequest"])

Expand All @@ -176,7 +162,6 @@ class RequestTest < Minitest::Test
end

it "create a signed logout request with 256 digest and signature method" do
settings.compress_request = false
settings.security[:signature_method] = XMLSecurity::Document::RSA_SHA256
settings.security[:digest_method] = XMLSecurity::Document::SHA256

Expand All @@ -188,7 +173,6 @@ class RequestTest < Minitest::Test
end

it "create a signed logout request with 512 digest and signature method RSA_SHA384" do
settings.compress_request = false
settings.security[:signature_method] = XMLSecurity::Document::RSA_SHA384
settings.security[:digest_method] = XMLSecurity::Document::SHA512

Expand All @@ -201,7 +185,6 @@ class RequestTest < Minitest::Test
end

it "create a signed logout request using the first certificate and key" do
settings.compress_request = false
settings.certificate = nil
settings.private_key = nil
settings.sp_cert_multi = {
Expand All @@ -220,7 +203,6 @@ class RequestTest < Minitest::Test
end

it "create a signed logout request using the first valid certificate and key when :check_sp_cert_expiration is true" do
settings.compress_request = false
settings.certificate = nil
settings.private_key = nil
settings.security[:check_sp_cert_expiration] = true
Expand Down Expand Up @@ -328,7 +310,6 @@ class RequestTest < Minitest::Test

it "create a signature parameter using the first certificate and key" do
settings.security[:signature_method] = XMLSecurity::Document::RSA_SHA1
settings.compress_request = false
settings.certificate = nil
settings.private_key = nil
settings.sp_cert_multi = {
Expand Down Expand Up @@ -366,19 +347,17 @@ class RequestTest < Minitest::Test

before do
# sign the logout request
settings.idp_slo_service_binding = RubySaml::Utils::BINDINGS[:post]
settings.security[:logout_requests_signed] = true
settings.security[:embed_sign] = true
settings.certificate = ruby_saml_cert_text
settings.private_key = ruby_saml_key_text
end

it "created a signed logout request" do
settings.compress_request = true

unauth_req = RubySaml::Logoutrequest.new
unauth_url = unauth_req.create(settings)
inflated = unauth_req.create_logout_request_xml_doc(settings).to_s

inflated = decode_saml_request_payload(unauth_url)
assert_match %r[<ds:SignatureValue>([a-zA-Z0-9/+=]+)</ds:SignatureValue>], inflated
assert_match %r[<ds:SignatureMethod Algorithm='http://www.w3.org/2001/04/xmldsig-more#rsa-sha256'/>], inflated
assert_match %r[<ds:DigestMethod Algorithm='http://www.w3.org/2001/04/xmlenc#sha256'/>], inflated
Expand Down
7 changes: 1 addition & 6 deletions test/request_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ class RequestTest < Minitest::Test
end

it "create the SAMLRequest URL parameter without deflating" do
settings.compress_request = false
settings.idp_sso_service_binding = RubySaml::Utils::BINDINGS[:post]
auth_url = RubySaml::Authrequest.new.create(settings)
assert_match(/^http:\/\/example\.com\?SAMLRequest=/, auth_url)
payload = CGI.unescape(auth_url.split("=").last)
Expand Down Expand Up @@ -242,7 +242,6 @@ class RequestTest < Minitest::Test

describe "#create_params signing with HTTP-POST binding" do
before do
settings.compress_request = false
settings.idp_sso_service_url = "http://example.com?field=value"
settings.idp_sso_service_binding = :post
settings.security[:authn_requests_signed] = true
Expand Down Expand Up @@ -317,7 +316,6 @@ class RequestTest < Minitest::Test
let(:cert) { OpenSSL::X509::Certificate.new(ruby_saml_cert_text) }

before do
settings.compress_request = false
settings.idp_sso_service_url = "http://example.com?field=value"
settings.idp_sso_service_binding = :redirect
settings.assertion_consumer_service_binding = "urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST-SimpleSign"
Expand Down Expand Up @@ -362,7 +360,6 @@ class RequestTest < Minitest::Test

it "create a signature parameter using the first certificate and key" do
settings.security[:signature_method] = XMLSecurity::Document::RSA_SHA1
settings.compress_request = false
settings.certificate = nil
settings.private_key = nil
settings.sp_cert_multi = {
Expand Down Expand Up @@ -432,7 +429,6 @@ class RequestTest < Minitest::Test

describe "DEPRECATED: #create_params signing with HTTP-POST binding via :embed_sign" do
before do
settings.compress_request = false
settings.idp_sso_service_url = "http://example.com?field=value"
settings.security[:authn_requests_signed] = true
settings.security[:embed_sign] = true
Expand All @@ -452,7 +448,6 @@ class RequestTest < Minitest::Test
let(:cert) { OpenSSL::X509::Certificate.new(ruby_saml_cert_text) }

before do
settings.compress_request = false
settings.idp_sso_service_url = "http://example.com?field=value"
settings.assertion_consumer_service_binding = "urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST-SimpleSign"
settings.security[:authn_requests_signed] = true
Expand Down
6 changes: 2 additions & 4 deletions test/saml_message_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,11 @@ class RubySamlTest < Minitest::Test
end

it "return encoded raw saml" do
settings.compress_request = true
encoded_raw = saml_message.send(:encode_raw_saml, logout_request_document, settings)
encoded_raw = saml_message.send(:encode_raw_saml, logout_request_document, true)
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)
encoded_raw = saml_message.send(:encode_raw_saml, deflated, false)
assert logout_request_deflated_base64, encoded_raw
end

Expand Down
2 changes: 1 addition & 1 deletion test/settings_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ class SettingsTest < Minitest::Test
:idp_attribute_names, :issuer, :assertion_consumer_service_url, :single_logout_service_url,
:sp_name_qualifier, :name_identifier_format, :name_identifier_value, :name_identifier_value_requested,
:sessionindex, :attributes_index, :passive, :force_authn,
:compress_request, :double_quote_xml_attribute_values, :message_max_bytesize,
:double_quote_xml_attribute_values, :message_max_bytesize,
:security, :certificate, :private_key, :certificate_new, :sp_cert_multi,
:authn_context, :authn_context_comparison, :authn_context_decl_ref,
:assertion_consumer_logout_service_url
Expand Down
6 changes: 0 additions & 6 deletions test/slo_logoutresponse_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ class SloLogoutresponseTest < Minitest::Test
settings.idp_entity_id = 'https://app.onelogin.com/saml/metadata/SOMEACCOUNT'
settings.idp_slo_service_url = "http://unauth.com/logout"
settings.name_identifier_value = "f00f00"
settings.compress_request = true
settings.certificate = ruby_saml_cert_text
settings.private_key = ruby_saml_key_text
logout_request.settings = settings
Expand Down Expand Up @@ -102,7 +101,6 @@ class SloLogoutresponseTest < Minitest::Test
before do
settings.idp_sso_service_binding = :redirect
settings.idp_slo_service_binding = :post
settings.compress_response = false
settings.security[:logout_responses_signed] = true
end

Expand Down Expand Up @@ -232,7 +230,6 @@ class SloLogoutresponseTest < Minitest::Test
before do
settings.idp_sso_service_binding = :post
settings.idp_slo_service_binding = :redirect
settings.compress_response = false
settings.security[:logout_responses_signed] = true
end

Expand Down Expand Up @@ -313,7 +310,6 @@ class SloLogoutresponseTest < Minitest::Test

it "create a signature parameter using the first certificate and key" do
settings.security[:signature_method] = XMLSecurity::Document::RSA_SHA1
settings.compress_request = false
settings.certificate = nil
settings.private_key = nil
settings.sp_cert_multi = {
Expand Down Expand Up @@ -349,7 +345,6 @@ class SloLogoutresponseTest < Minitest::Test

describe "DEPRECATED: signing with HTTP-POST binding via :embed_sign" do
before do
settings.compress_response = false
settings.security[:logout_responses_signed] = true
settings.security[:embed_sign] = true
end
Expand Down Expand Up @@ -384,7 +379,6 @@ class SloLogoutresponseTest < Minitest::Test
let(:cert) { OpenSSL::X509::Certificate.new(ruby_saml_cert_text) }

before do
settings.compress_response = false
settings.security[:logout_responses_signed] = true
settings.security[:embed_sign] = false
end
Expand Down
Loading