Skip to content

Commit

Permalink
Deprecate the settings.compress_request and settings.compress_respons…
Browse files Browse the repository at this point in the history
…e parameters. Set their behavior automatically based on settings.idp_sso_service_binding and settings.idp_slo_service_binding respectively. HTTP-Redirect will always use compression, while HTTP-POST will not.
  • Loading branch information
johnnyshields committed Jul 9, 2024
1 parent a3d2045 commit 19a3e15
Show file tree
Hide file tree
Showing 14 changed files with 107 additions and 85 deletions.
34 changes: 13 additions & 21 deletions .rubocop_todo.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# This configuration was generated by
# `rubocop --auto-gen-config`
# on 2024-07-08 10:27:10 UTC using RuboCop version 1.64.1.
# on 2024-07-09 08:57:21 UTC using RuboCop version 1.64.1.
# The point is for the user to remove these configuration records
# one by one as the offenses are removed from the code base.
# Note that changes in the inspected code, or installation of new
Expand Down Expand Up @@ -59,13 +59,6 @@ Layout/EmptyLinesAroundModuleBody:
- 'lib/ruby_saml/utils.rb'
- 'lib/xml_security.rb'

# Offense count: 1
# Configuration parameters: EnforcedStyle.
# SupportedStyles: native, lf, crlf
Layout/EndOfLine:
Exclude:
- 'lib/ruby_saml.rb'

# Offense count: 3
# This cop supports safe autocorrection (--autocorrect).
# Configuration parameters: AllowForAlignment, AllowBeforeTrailingComments, ForceEqualSignAlignment.
Expand Down Expand Up @@ -156,14 +149,6 @@ Layout/SpaceInsideHashLiteralBraces:
- 'lib/ruby_saml/slo_logoutresponse.rb'
- 'lib/xml_security.rb'

# Offense count: 1
# This cop supports safe autocorrection (--autocorrect).
# Configuration parameters: EnforcedStyle.
# SupportedStyles: final_newline, final_blank_line
Layout/TrailingEmptyLines:
Exclude:
- 'lib/ruby_saml.rb'

# Offense count: 2
Lint/NoReturnInBeginEndBlocks:
Exclude:
Expand All @@ -185,12 +170,11 @@ Lint/UnreachableLoop:
Exclude:
- 'lib/ruby_saml/saml_message.rb'

# Offense count: 3
# Offense count: 2
# This cop supports unsafe autocorrection (--autocorrect-all).
# Configuration parameters: AutoCorrect.
Lint/UselessAssignment:
Exclude:
- 'lib/ruby_saml/logging.rb'
- 'lib/ruby_saml/slo_logoutrequest.rb'

# Offense count: 42
Expand Down Expand Up @@ -310,7 +294,7 @@ Performance/StringReplacement:
- 'lib/ruby_saml/utils.rb'
- 'lib/xml_security.rb'

# Offense count: 54
# Offense count: 52
# This cop supports safe autocorrection (--autocorrect).
# Configuration parameters: EnforcedStyle.
# SupportedStyles: separated, grouped
Expand Down Expand Up @@ -418,7 +402,15 @@ Style/IfUnlessModifier:
- 'lib/ruby_saml/utils.rb'
- 'lib/xml_security.rb'

# Offense count: 15
# Offense count: 1
# This cop supports unsafe autocorrection (--autocorrect-all).
# Configuration parameters: EnforcedStyle, Autocorrect.
# SupportedStyles: module_function, extend_self, forbidden
Style/ModuleFunction:
Exclude:
- 'lib/ruby_saml/logging.rb'

# Offense count: 16
# Configuration parameters: AllowedMethods.
# AllowedMethods: respond_to_missing?
Style/OptionalBooleanParameter:
Expand Down Expand Up @@ -510,7 +502,7 @@ Style/SymbolArray:
Exclude:
- 'lib/ruby_saml/settings.rb'

# Offense count: 94
# Offense count: 91
# This cop supports safe autocorrection (--autocorrect).
# Configuration parameters: AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, AllowedPatterns.
# URISchemes: http, https
Expand Down
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.
* [#695](https://github.com/SAML-Toolkits/ruby-saml/pull/695) Deprecate `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
12 changes: 12 additions & 0 deletions UPGRADING.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,18 @@ settings.security[:digest_method] = XMLSecurity::Document::SHA1
settings.security[:signature_method] = XMLSecurity::Document::RSA_SHA1
```

### Deprecation of Compression Settings

The `settings.compress_request` and `settings.compress_response` parameters have been deprecated
and are no longer functional. They will be removed in RubySaml 2.1.0. Please remove `compress_request`
and `compress_response` everywhere within your project code.

The SAML SP request/response message compression behavior is now controlled automatically by 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
29 changes: 15 additions & 14 deletions lib/ruby_saml/logging.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,32 +2,33 @@

require 'logger'

# Simplistic log class when we're running in Rails
module RubySaml
class Logging
module Logging
extend self

DEFAULT_LOGGER = ::Logger.new($stdout)

def self.logger
attr_writer :logger

def logger
@logger ||= begin
logger = Rails.logger if defined?(::Rails) && Rails.respond_to?(:logger)
logger ||= DEFAULT_LOGGER
logger || DEFAULT_LOGGER
end
end

class << self
attr_writer :logger
%i[error warn debug info].each do |level|
define_method(level) do |message|
logger.send(level, message) if enabled?
end
end

def self.debug(message)
return if ENV['ruby-saml/testing']

logger.debug(message)
def deprecate(message)
warn("[DEPRECATION] RubySaml: #{message}")
end

def self.info(message)
return if ENV['ruby-saml/testing']

logger.info(message)
def enabled?
!ENV['ruby-saml/testing']
end
end
end
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
16 changes: 12 additions & 4 deletions lib/ruby_saml/saml_message.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
require 'rexml/document'
require 'rexml/xpath'
require 'ruby_saml/error_handling'
require 'ruby_saml/logging'

# Only supports SAML 2.0
module RubySaml
Expand Down Expand Up @@ -104,11 +105,18 @@ 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 settings_or_compress [true|false|RubySaml::Settings|nil] Whether or not the SAML should be deflated.
# The usage of RubySaml::Settings here is deprecated.
# @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, settings_or_compress = false)
if settings_or_compress.is_a?(TrueClass)
saml = deflate(saml)
elsif settings_or_compress.respond_to?(:compress_request)
Logging.deprecate('Please change the second argument of `encode_raw_saml_message` to a boolean ' \
'indicating whether or not to use compression. Using a boolean will be required ' \
'in RubySaml 2.1.0.')
saml = deflate(saml) if settings_or_compress.compress_request
end

CGI.escape(encode(saml))
end
Expand Down
36 changes: 32 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 All @@ -301,8 +297,40 @@ def get_binding(value)
}.freeze
}.freeze

# @deprecated Will be removed in v2.1.0
def compress_request
compress_deprecation('compress_request', 'idp_sso_service_binding')
@compress_request
end

# @deprecated Will be removed in v2.1.0
def compress_request=(value)
compress_deprecation('compress_request', 'idp_sso_service_binding')
@compress_request = value
end

# @deprecated Will be removed in v2.1.0
def compress_response
compress_deprecation('compress_response', 'idp_slo_service_binding')
@compress_response
end

# @deprecated Will be removed in v2.1.0
def compress_response=(value)
compress_deprecation('compress_response', 'idp_slo_service_binding')
@compress_response = value
end

private

# @deprecated Will be removed in v2.1.0
def compress_deprecation(old_param, new_param)
Logging.deprecate "`RubySaml::Settings##{old_param}` is deprecated and no longer functional. " \
'It will be removed in RubySaml 2.1.0. ' \
"Its functionality is now handled by `RubySaml::Settings##{new_param}` instead: " \
'"HTTP-Redirect" will always be compressed, and "HTTP-POST" will always be uncompressed.'
end

# @return [Hash<Symbol, Array<Array<OpenSSL::X509::Certificate, OpenSSL::PKey::RSA>>>]
# Build the SP certificates and private keys from the settings. Returns all
# certificates and private keys, even if they are expired.
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
Loading

0 comments on commit 19a3e15

Please sign in to comment.