Skip to content

Commit 4143536

Browse files
johnnyshieldsjohnnyshields
committed
More rubocop fixes
1 parent b373252 commit 4143536

File tree

11 files changed

+98
-221
lines changed

11 files changed

+98
-221
lines changed

‎.rubocop.yml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,3 +15,9 @@ AllCops:
1515
- '.git/**/*'
1616
- 'tmp/**/*'
1717
- 'vendor/**/*'
18+
19+
Style/NumericPredicate:
20+
EnforcedStyle: comparison
21+
22+
Style/RaiseArgs:
23+
EnforcedStyle: compact

‎.rubocop_todo.yml‎

Lines changed: 0 additions & 106 deletions
Original file line numberDiff line numberDiff line change
@@ -409,112 +409,6 @@ Style/NegatedIfElseCondition:
409409
Exclude:
410410
- 'lib/onelogin/ruby-saml/authrequest.rb'
411411

412-
# Offense count: 6
413-
# This cop supports safe autocorrection (--autocorrect).
414-
# Configuration parameters: EnforcedStyle, MinBodyLength.
415-
# SupportedStyles: skip_modifier_ifs, always
416-
Style/Next:
417-
Exclude:
418-
- 'lib/onelogin/ruby-saml/logoutresponse.rb'
419-
- 'lib/onelogin/ruby-saml/metadata.rb'
420-
- 'lib/onelogin/ruby-saml/response.rb'
421-
- 'lib/onelogin/ruby-saml/slo_logoutrequest.rb'
422-
- 'lib/xml_security.rb'
423-
424-
# Offense count: 7
425-
# This cop supports unsafe autocorrection (--autocorrect-all).
426-
# Configuration parameters: EnforcedStyle, AllowedMethods, AllowedPatterns.
427-
# SupportedStyles: predicate, comparison
428-
Style/NumericPredicate:
429-
Exclude:
430-
- 'spec/**/*'
431-
- 'lib/onelogin/ruby-saml/attribute_service.rb'
432-
- 'lib/onelogin/ruby-saml/response.rb'
433-
- 'lib/onelogin/ruby-saml/slo_logoutrequest.rb'
434-
435-
# Offense count: 15
436-
# Configuration parameters: AllowedMethods.
437-
# AllowedMethods: respond_to_missing?
438-
Style/OptionalBooleanParameter:
439-
Exclude:
440-
- 'lib/onelogin/ruby-saml/idp_metadata_parser.rb'
441-
- 'lib/onelogin/ruby-saml/logoutresponse.rb'
442-
- 'lib/onelogin/ruby-saml/metadata.rb'
443-
- 'lib/onelogin/ruby-saml/response.rb'
444-
- 'lib/onelogin/ruby-saml/saml_message.rb'
445-
- 'lib/onelogin/ruby-saml/settings.rb'
446-
- 'lib/onelogin/ruby-saml/slo_logoutrequest.rb'
447-
- 'lib/onelogin/ruby-saml/utils.rb'
448-
- 'lib/xml_security.rb'
449-
450-
# Offense count: 9
451-
# This cop supports unsafe autocorrection (--autocorrect-all).
452-
# Configuration parameters: EnforcedStyle.
453-
# SupportedStyles: short, verbose
454-
Style/PreferredHashMethods:
455-
Exclude:
456-
- 'lib/onelogin/ruby-saml/attributes.rb'
457-
- 'lib/onelogin/ruby-saml/logoutresponse.rb'
458-
- 'lib/onelogin/ruby-saml/response.rb'
459-
- 'lib/onelogin/ruby-saml/slo_logoutrequest.rb'
460-
461-
# Offense count: 21
462-
# This cop supports safe autocorrection (--autocorrect).
463-
# Configuration parameters: AllowedCompactTypes.
464-
# SupportedStyles: compact, exploded
465-
Style/RaiseArgs:
466-
EnforcedStyle: compact
467-
468-
# Offense count: 4
469-
# This cop supports safe autocorrection (--autocorrect).
470-
Style/RedundantRegexpEscape:
471-
Exclude:
472-
- 'lib/onelogin/ruby-saml/utils.rb'
473-
474-
# Offense count: 5
475-
# This cop supports safe autocorrection (--autocorrect).
476-
# Configuration parameters: AllowMultipleReturnValues.
477-
Style/RedundantReturn:
478-
Exclude:
479-
- 'lib/onelogin/ruby-saml/logoutresponse.rb'
480-
- 'lib/onelogin/ruby-saml/utils.rb'
481-
- 'lib/xml_security.rb'
482-
483-
# Offense count: 4
484-
# This cop supports safe autocorrection (--autocorrect).
485-
Style/RedundantSelf:
486-
Exclude:
487-
- 'lib/onelogin/ruby-saml/utils.rb'
488-
- 'lib/xml_security.rb'
489-
490-
# Offense count: 4
491-
# This cop supports safe autocorrection (--autocorrect).
492-
# Configuration parameters: EnforcedStyle, AllowInnerSlashes.
493-
# SupportedStyles: slashes, percent_r, mixed
494-
Style/RegexpLiteral:
495-
Exclude:
496-
- 'lib/onelogin/ruby-saml/response.rb'
497-
- 'lib/onelogin/ruby-saml/utils.rb'
498-
499-
# Offense count: 4
500-
# This cop supports unsafe autocorrection (--autocorrect-all).
501-
Style/SlicingWithRange:
502-
Exclude:
503-
- 'lib/onelogin/ruby-saml/response.rb'
504-
- 'lib/onelogin/ruby-saml/utils.rb'
505-
- 'lib/xml_security.rb'
506-
507-
# Offense count: 10
508-
# This cop supports safe autocorrection (--autocorrect).
509-
# Configuration parameters: AllowModifier.
510-
Style/SoleNestedConditional:
511-
Exclude:
512-
- 'lib/onelogin/ruby-saml/idp_metadata_parser.rb'
513-
- 'lib/onelogin/ruby-saml/logoutresponse.rb'
514-
- 'lib/onelogin/ruby-saml/response.rb'
515-
- 'lib/onelogin/ruby-saml/settings.rb'
516-
- 'lib/onelogin/ruby-saml/slo_logoutrequest.rb'
517-
518412
# Offense count: 7
519413
# This cop supports unsafe autocorrection (--autocorrect-all).
520414
# Configuration parameters: Mode.

‎lib/onelogin/ruby-saml/attributes.rb‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ def each(&block)
4848
# @param name [String] The attribute name to be checked
4949
#
5050
def include?(name)
51-
attributes.has_key?(canonize_name(name))
51+
attributes.key?(canonize_name(name))
5252
end
5353

5454
# Return first value for an attribute

‎lib/onelogin/ruby-saml/idp_metadata_parser.rb‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -125,10 +125,8 @@ def parse(idp_metadata, options = {})
125125

126126
unless parsed_metadata[:cache_duration].nil?
127127
cache_valid_until_timestamp = OneLogin::RubySaml::Utils.parse_duration(parsed_metadata[:cache_duration])
128-
unless cache_valid_until_timestamp.nil?
129-
if parsed_metadata[:valid_until].nil? || cache_valid_until_timestamp < Time.parse(parsed_metadata[:valid_until], Time.now.utc).to_i
130-
parsed_metadata[:valid_until] = Time.at(cache_valid_until_timestamp).utc.strftime("%Y-%m-%dT%H:%M:%SZ")
131-
end
128+
if !cache_valid_until_timestamp.nil? && (parsed_metadata[:valid_until].nil? || cache_valid_until_timestamp < Time.parse(parsed_metadata[:valid_until], Time.now.utc).to_i)
129+
parsed_metadata[:valid_until] = Time.at(cache_valid_until_timestamp).utc.strftime("%Y-%m-%dT%H:%M:%SZ")
132130
end
133131
end
134132
# Remove the cache_duration because on the settings

‎lib/onelogin/ruby-saml/logoutresponse.rb‎

Lines changed: 11 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ def response_id
5959
# @raise [ValidationError] if soft == false and validation fails
6060
#
6161
def success?
62-
return status_code == "urn:oasis:names:tc:SAML:2.0:status:Success"
62+
status_code == "urn:oasis:names:tc:SAML:2.0:status:Success"
6363
end
6464

6565
# @return [String|nil] Gets the InResponseTo attribute from the Logout Response if exists.
@@ -185,7 +185,7 @@ def valid_state?
185185
# @raise [ValidationError] if soft == false and validation fails
186186
#
187187
def valid_in_response_to?
188-
return true unless options.has_key? :matches_request_id
188+
return true unless options.key? :matches_request_id
189189
return true if options[:matches_request_id].nil?
190190
return true unless options[:matches_request_id] != in_response_to
191191

@@ -212,8 +212,8 @@ def valid_issuer?
212212
#
213213
def validate_signature
214214
return true if options.nil?
215-
return true unless options.has_key? :get_params
216-
return true unless options[:get_params].has_key? 'Signature'
215+
return true unless options.key? :get_params
216+
return true unless options[:get_params].key? 'Signature'
217217

218218
options[:raw_get_params] = OneLogin::RubySaml::Utils.prepare_raw_get_params(options[:raw_get_params], options[:get_params], settings.security[:lowercase_url_encoding])
219219

@@ -225,7 +225,7 @@ def validate_signature
225225
idp_certs = settings.get_idp_cert_multi
226226

227227
if idp_cert.nil? && (idp_certs.nil? || idp_certs[:signing].empty?)
228-
return options.has_key? :relax_signature_validation
228+
return options.key? :relax_signature_validation
229229
end
230230

231231
query_string = OneLogin::RubySaml::Utils.build_query_from_raw_parts(
@@ -243,10 +243,8 @@ def validate_signature
243243
signature: options[:get_params]['Signature'],
244244
query_string: query_string
245245
)
246-
if valid && settings.security[:check_idp_cert_expiration]
247-
if OneLogin::RubySaml::Utils.is_cert_expired(idp_cert)
248-
expired = true
249-
end
246+
if valid && settings.security[:check_idp_cert_expiration] && OneLogin::RubySaml::Utils.is_cert_expired(idp_cert)
247+
expired = true
250248
end
251249
else
252250
valid = false
@@ -257,14 +255,11 @@ def validate_signature
257255
signature: options[:get_params]['Signature'],
258256
query_string: query_string
259257
)
260-
if valid
261-
if settings.security[:check_idp_cert_expiration]
262-
if OneLogin::RubySaml::Utils.is_cert_expired(signing_idp_cert)
263-
expired = true
264-
end
265-
end
266-
break
258+
next unless valid
259+
if settings.security[:check_idp_cert_expiration] && OneLogin::RubySaml::Utils.is_cert_expired(signing_idp_cert)
260+
expired = true
267261
end
262+
break
268263
end
269264
end
270265

‎lib/onelogin/ruby-saml/metadata.rb‎

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -71,22 +71,22 @@ def add_sp_certificates(sp_sso, settings)
7171
cert_new = settings.get_sp_cert_new
7272

7373
[cert, cert_new].each do |sp_cert|
74-
if sp_cert
75-
cert_text = Base64.encode64(sp_cert.to_der).gsub("\n", '')
76-
kd = sp_sso.add_element "md:KeyDescriptor", { "use" => "signing" }
77-
ki = kd.add_element "ds:KeyInfo", {"xmlns:ds" => "http://www.w3.org/2000/09/xmldsig#"}
78-
xd = ki.add_element "ds:X509Data"
79-
xc = xd.add_element "ds:X509Certificate"
80-
xc.text = cert_text
81-
82-
if settings.security[:want_assertions_encrypted]
83-
kd2 = sp_sso.add_element "md:KeyDescriptor", { "use" => "encryption" }
84-
ki2 = kd2.add_element "ds:KeyInfo", {"xmlns:ds" => "http://www.w3.org/2000/09/xmldsig#"}
85-
xd2 = ki2.add_element "ds:X509Data"
86-
xc2 = xd2.add_element "ds:X509Certificate"
87-
xc2.text = cert_text
88-
end
89-
end
74+
next unless sp_cert
75+
76+
cert_text = Base64.encode64(sp_cert.to_der).gsub("\n", '')
77+
kd = sp_sso.add_element "md:KeyDescriptor", { "use" => "signing" }
78+
ki = kd.add_element "ds:KeyInfo", {"xmlns:ds" => "http://www.w3.org/2000/09/xmldsig#"}
79+
xd = ki.add_element "ds:X509Data"
80+
xc = xd.add_element "ds:X509Certificate"
81+
xc.text = cert_text
82+
83+
next unless settings.security[:want_assertions_encrypted]
84+
85+
kd2 = sp_sso.add_element "md:KeyDescriptor", { "use" => "encryption" }
86+
ki2 = kd2.add_element "ds:KeyInfo", {"xmlns:ds" => "http://www.w3.org/2000/09/xmldsig#"}
87+
xd2 = ki2.add_element "ds:X509Data"
88+
xc2 = xd2.add_element "ds:X509Certificate"
89+
xc2.text = cert_text
9090
end
9191

9292
sp_sso
@@ -131,11 +131,11 @@ def add_sp_service_elements(sp_sso, settings)
131131
"FriendlyName" => attribute[:friendly_name],
132132
"isRequired" => attribute[:is_required] || false
133133
}
134-
unless attribute[:attribute_value].nil?
135-
Array(attribute[:attribute_value]).each do |value|
136-
sp_attr_val = sp_req_attr.add_element "saml:AttributeValue"
137-
sp_attr_val.text = value.to_s
138-
end
134+
next if attribute[:attribute_value].nil?
135+
136+
Array(attribute[:attribute_value]).each do |value|
137+
sp_attr_val = sp_req_attr.add_element "saml:AttributeValue"
138+
sp_attr_val.text = value.to_s
139139
end
140140
end
141141
end

‎lib/onelogin/ruby-saml/response.rb‎

Lines changed: 15 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -430,10 +430,8 @@ def validate_structure
430430
return append_error(structure_error_msg)
431431
end
432432

433-
unless decrypted_document.nil?
434-
unless valid_saml?(decrypted_document, soft)
433+
if !decrypted_document.nil? && !valid_saml?(decrypted_document, soft)
435434
return append_error(structure_error_msg)
436-
end
437435
end
438436

439437
true
@@ -566,7 +564,7 @@ def validate_signed_elements
566564
if ref
567565
uri = ref.attributes.get_attribute("URI")
568566
if uri && !uri.value.empty?
569-
sei = uri.value[1..-1]
567+
sei = uri.value[1..]
570568

571569
unless sei == id
572570
return append_error("Found an invalid Signed Element. SAML Response rejected")
@@ -600,7 +598,7 @@ def validate_signed_elements
600598
# @raise [ValidationError] if soft == false and validation fails
601599
#
602600
def validate_in_response_to
603-
return true unless options.has_key? :matches_request_id
601+
return true unless options.key? :matches_request_id
604602
return true if options[:matches_request_id].nil?
605603
return true unless options[:matches_request_id] != in_response_to
606604

@@ -813,10 +811,8 @@ def validate_name_id
813811
return append_error("An empty NameID value found")
814812
end
815813

816-
unless settings.sp_entity_id.nil? || settings.sp_entity_id.empty? || name_id_spnamequalifier.nil? || name_id_spnamequalifier.empty?
817-
if name_id_spnamequalifier != settings.sp_entity_id
814+
if !(settings.sp_entity_id.nil? || settings.sp_entity_id.empty? || name_id_spnamequalifier.nil? || name_id_spnamequalifier.empty?) && (name_id_spnamequalifier != settings.sp_entity_id)
818815
return append_error("The SPNameQualifier value mistmatch the SP entityID value.")
819-
end
820816
end
821817
end
822818

@@ -872,11 +868,9 @@ def validate_signature
872868
opts[:cert] = idp_cert
873869

874870
if fingerprint && doc.validate_document(fingerprint, @soft, opts)
875-
if settings.security[:check_idp_cert_expiration]
876-
if OneLogin::RubySaml::Utils.is_cert_expired(idp_cert)
871+
if settings.security[:check_idp_cert_expiration] && OneLogin::RubySaml::Utils.is_cert_expired(idp_cert)
877872
error_msg = "IdP x509 certificate expired"
878873
return append_error(error_msg)
879-
end
880874
end
881875
else
882876
return append_error(error_msg)
@@ -886,17 +880,14 @@ def validate_signature
886880
expired = false
887881
idp_certs[:signing].each do |idp_cert|
888882
valid = doc.validate_document_with_cert(idp_cert, true)
889-
if valid
890-
if settings.security[:check_idp_cert_expiration]
891-
if OneLogin::RubySaml::Utils.is_cert_expired(idp_cert)
892-
expired = true
893-
end
894-
end
895-
896-
# At least one certificate is valid, restore the old accumulated errors
897-
@errors = old_errors
898-
break
883+
next unless valid
884+
if settings.security[:check_idp_cert_expiration] && OneLogin::RubySaml::Utils.is_cert_expired(idp_cert)
885+
expired = true
899886
end
887+
888+
# At least one certificate is valid, restore the old accumulated errors
889+
@errors = old_errors
890+
break
900891
end
901892
if expired
902893
error_msg = "IdP x509 certificate expired"
@@ -1005,23 +996,23 @@ def decrypt_assertion_from_document(document_copy)
1005996
# @return [REXML::Document] The decrypted EncryptedAssertion element
1006997
#
1007998
def decrypt_assertion(encrypted_assertion_node)
1008-
decrypt_element(encrypted_assertion_node, /(.*<\/(\w+:)?Assertion>)/m)
999+
decrypt_element(encrypted_assertion_node, %r{(.*</(\w+:)?Assertion>)}m)
10091000
end
10101001

10111002
# Decrypts an EncryptedID element
10121003
# @param encryptedid_node [REXML::Element] The EncryptedID element
10131004
# @return [REXML::Document] The decrypted EncrypedtID element
10141005
#
10151006
def decrypt_nameid(encryptedid_node)
1016-
decrypt_element(encryptedid_node, /(.*<\/(\w+:)?NameID>)/m)
1007+
decrypt_element(encryptedid_node, %r{(.*</(\w+:)?NameID>)}m)
10171008
end
10181009

10191010
# Decrypts an EncryptedID element
10201011
# @param encryptedid_node [REXML::Element] The EncryptedID element
10211012
# @return [REXML::Document] The decrypted EncrypedtID element
10221013
#
10231014
def decrypt_attribute(encryptedattribute_node)
1024-
decrypt_element(encryptedattribute_node, /(.*<\/(\w+:)?Attribute>)/m)
1015+
decrypt_element(encryptedattribute_node, %r{(.*</(\w+:)?Attribute>)}m)
10251016
end
10261017

10271018
# Decrypt an element

‎lib/onelogin/ruby-saml/settings.rb‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -218,10 +218,8 @@ def get_sp_cert
218218
formatted_cert = OneLogin::RubySaml::Utils.format_cert(certificate)
219219
cert = OpenSSL::X509::Certificate.new(formatted_cert)
220220

221-
if security[:check_sp_cert_expiration]
222-
if OneLogin::RubySaml::Utils.is_cert_expired(cert)
223-
raise OneLogin::RubySaml::ValidationError.new("The SP certificate expired.")
224-
end
221+
if security[:check_sp_cert_expiration] && OneLogin::RubySaml::Utils.is_cert_expired(cert)
222+
raise OneLogin::RubySaml::ValidationError.new("The SP certificate expired.")
225223
end
226224

227225
cert

0 commit comments

Comments
 (0)