Skip to content

Commit

Permalink
Update rubocop rules and fix lint violations
Browse files Browse the repository at this point in the history
  • Loading branch information
ahukkanen committed Feb 13, 2024
1 parent 1f8ffe3 commit 51fcb29
Show file tree
Hide file tree
Showing 9 changed files with 54 additions and 51 deletions.
13 changes: 7 additions & 6 deletions .rubocop.yml
Original file line number Diff line number Diff line change
@@ -1,19 +1,24 @@
AllCops:
TargetRubyVersion: 2.6
NewCops: enable

Layout/AccessModifierIndentation:
EnforcedStyle: outdent

Layout/AlignHash:
Layout/HashAlignment:
Enabled: false

Layout/DotPosition:
EnforcedStyle: trailing

Layout/LineLength:
AllowURI: true
Enabled: false

Layout/SpaceInsideHashLiteralBraces:
EnforcedStyle: no_space

Lint/HandleExceptions:
Lint/SuppressedException:
Enabled: false

Metrics/BlockLength:
Expand All @@ -25,10 +30,6 @@ Metrics/BlockNesting:
Metrics/ClassLength:
Enabled: false

Metrics/LineLength:
AllowURI: true
Enabled: false

Metrics/MethodLength:
CountComments: false
Max: 15
Expand Down
8 changes: 4 additions & 4 deletions .simplecov
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
# frozen_string_literal: true

SimpleCov.start do
root ENV["ENGINE_ROOT"]
root ENV.fetch('ENGINE_ROOT', nil)

add_filter "lib/omniauth-suomifi/version.rb"
add_filter "/spec"
add_filter 'lib/omniauth-suomifi/version.rb'
add_filter '/spec'
end

SimpleCov.command_name ENV["COMMAND_NAME"] || File.basename(Dir.pwd)
SimpleCov.command_name ENV['COMMAND_NAME'] || File.basename(Dir.pwd)

SimpleCov.merge_timeout 1800

Expand Down
6 changes: 4 additions & 2 deletions lib/omniauth-suomifi/ruby_saml_extensions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
# @param symmetric_key [String] The symetric key used to encrypt the text
# @param algorithm [String] The encrypted algorithm
# @return [String] The deciphered text
# rubocop:disable Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity, Metrics/MethodLength
def self.retrieve_plaintext(cipher_text, symmetric_key, algorithm)
case algorithm
when 'http://www.w3.org/2001/04/xmlenc#tripledes-cbc' then cipher = OpenSSL::Cipher.new('DES-EDE3-CBC').decrypt
Expand All @@ -43,7 +44,7 @@ def self.retrieve_plaintext(cipher_text, symmetric_key, algorithm)

if cipher
iv_len = cipher.iv_len
data = cipher_text[iv_len..-1]
data = cipher_text[iv_len..]
cipher.padding = 0
cipher.key = symmetric_key
cipher.iv = cipher_text[0..iv_len - 1]
Expand All @@ -58,7 +59,7 @@ def self.retrieve_plaintext(cipher_text, symmetric_key, algorithm)
auth_cipher.key = symmetric_key
auth_cipher.iv = cipher_text[0..iv_len - 1]
auth_cipher.auth_data = ''
auth_cipher.auth_tag = cipher_text[text_len - tag_len..-1]
auth_cipher.auth_tag = cipher_text[text_len - tag_len..]
assertion_plaintext = auth_cipher.update(data)
assertion_plaintext << auth_cipher.final
elsif rsa
Expand All @@ -69,4 +70,5 @@ def self.retrieve_plaintext(cipher_text, symmetric_key, algorithm)
cipher_text
end
end
# rubocop:enable Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity, Metrics/MethodLength
end
4 changes: 2 additions & 2 deletions lib/omniauth-suomifi/test/certificate_generator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,14 @@ def certificate
cert = OpenSSL::X509::Certificate.new
cert.subject = cert.issuer = OpenSSL::X509::Name.parse(subject)
cert.not_before = Time.now
cert.not_after = Time.now + 365 * 24 * 60 * 60
cert.not_after = Time.now + (365 * 24 * 60 * 60)
cert.public_key = public_key
cert.serial = 0x0
cert.version = 2

inject_certificate_extensions(cert)

cert.sign(private_key, OpenSSL::Digest::SHA1.new)
cert.sign(private_key, OpenSSL::Digest.new('SHA1'))

cert
end
Expand Down
4 changes: 2 additions & 2 deletions lib/omniauth-suomifi/test/xml_encryptor.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ def encrypt(raw_xml)
end

def self.encrypted_xml(raw_xml_file, cert, sign_cert, sign_key)
raw_xml = IO.read(raw_xml_file)
raw_xml = File.read(raw_xml_file)
encrypted_xml_from_string(raw_xml, cert, sign_cert, sign_key)
end

Expand All @@ -53,7 +53,7 @@ def encryption_template
template_path = Utility.template_filepath(
'encrypted_data_template.xml'
)
template_io = IO.read(template_path)
template_io = File.read(template_path)

Nokogiri::XML::Document.parse(template_io).root
end
Expand Down
15 changes: 9 additions & 6 deletions lib/omniauth/strategies/suomifi.rb
Original file line number Diff line number Diff line change
Expand Up @@ -458,22 +458,21 @@ class Suomifi < SAML
eidas_id = find_attribute_by(
['http://eidas.europa.eu/attributes/naturalperson/PersonIdentifier']
)
hash_salt = begin
hash_salt =
if options.uid_salt
options.uid_salt
elsif defined?(::Rails) && ::Rails.application
::Rails.application.secrets.secret_key_base
else
''
end
end

if !electronic_id.nil?
'FINUID:' + electronic_id
"FINUID:#{electronic_id}"
elsif !national_id.nil?
'FIHETU:' + Digest::MD5.hexdigest("FI:#{national_id}:#{hash_salt}")
"FIHETU:#{Digest::MD5.hexdigest("FI:#{national_id}:#{hash_salt}")}"
elsif !eidas_id.nil?
'EIDASPID:' + Digest::MD5.hexdigest("EIDAS:#{eidas_id}:#{hash_salt}")
"EIDASPID:#{Digest::MD5.hexdigest("EIDAS:#{eidas_id}:#{hash_salt}")}"
else
@name_id
end
Expand All @@ -491,6 +490,7 @@ class Suomifi < SAML
attr_accessor :options
attr_reader :suomifi_thread

# rubocop:disable Metrics/MethodLength
def initialize(app, *args, &block)
super

Expand Down Expand Up @@ -520,6 +520,7 @@ def initialize(app, *args, &block)
)
end
end
# rubocop:enable Metrics/MethodLength

# Override the request phase to be able to pass the locale parameter to
# the redirect URL. Note that this needs to be the last parameter to
Expand Down Expand Up @@ -590,7 +591,7 @@ def handle_logout_request(raw_request, settings)
# currently open at the website.
logout_request = OneLogin::RubySaml::SloLogoutrequest.new(
raw_request,
{ settings: settings, get_params: @request.params }
{settings: settings, get_params: @request.params}
)
raise OmniAuth::Strategies::SAML::ValidationError.new('SAML failed to process LogoutRequest') unless logout_request.is_valid?

Expand Down Expand Up @@ -665,6 +666,7 @@ def idp_metadata_url
end
end

# rubocop:disable Metrics/MethodLength
def suomifi_options
idp_metadata_parser = OneLogin::RubySaml::IdpMetadataParser.new

Expand Down Expand Up @@ -705,6 +707,7 @@ def suomifi_options

settings
end
# rubocop:enable Metrics/MethodLength

# This will return true if the VTJ search (population information system,
# väestötietojärjestelmä) was successful and information about the person
Expand Down
3 changes: 2 additions & 1 deletion omniauth-suomifi.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,10 @@ require 'omniauth-suomifi/version'
Gem::Specification.new do |spec|
spec.name = 'omniauth-suomifi'
spec.version = OmniAuth::Suomifi::VERSION
spec.required_ruby_version = ">= 2.5"
spec.required_ruby_version = '>= 2.6'
spec.authors = ['Antti Hukkanen']
spec.email = ['[email protected]']
spec.metadata['rubygems_mfa_required'] = 'true'

spec.summary = 'Provides a Suomi.fi strategy for OmniAuth.'
spec.description = 'Suomi.fi e-Identification service integration for OmniAuth.'
Expand Down
50 changes: 23 additions & 27 deletions spec/omniauth/strategies/suomifi_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,7 @@
let(:sp_entity_id) { 'https://www.service.fi/auth/suomifi/metadata' }
let(:scope_of_data) { :medium_extensive }
let(:strategy) { [OmniAuth::Strategies::Suomifi, saml_options] }
let(:thread) { double(
join: nil,
alive?: false
)}
let(:thread) { double(join: nil, alive?: false) }

before do
# Stub the metadata to return the locally stored metadata for easier
Expand Down Expand Up @@ -143,8 +140,8 @@
attr_reader :temp_dir

before do
File.open(certificate_file, 'w') { |f| f.write(certificate.to_pem) }
File.open(private_key_file, 'w') { |f| f.write(private_key.to_pem) }
File.write(certificate_file, certificate.to_pem)
File.write(private_key_file, private_key.to_pem)
end

it 'should read the certificate and private key from the files' do
Expand Down Expand Up @@ -343,9 +340,16 @@
end

raw_xml_file = support_filepath("#{xml}.xml")
xml_signed = begin
if !custom_saml_attributes.empty?
xml_io = IO.read(raw_xml_file)
xml_signed =
if custom_saml_attributes.empty?
OmniAuth::Suomifi::Test::Utility.encrypted_signed_xml(
raw_xml_file,
certificate: certificate,
sign_certificate: sign_certificate,
sign_private_key: sign_private_key
)
else
xml_io = File.read(raw_xml_file)
doc = Nokogiri::XML::Document.parse(xml_io)
statements_node = doc.root.at_xpath(
'//saml2:Assertion//saml2:AttributeStatement',
Expand Down Expand Up @@ -387,15 +391,7 @@
sign_certificate: sign_certificate,
sign_private_key: sign_private_key
)
else
OmniAuth::Suomifi::Test::Utility.encrypted_signed_xml(
raw_xml_file,
certificate: certificate,
sign_certificate: sign_certificate,
sign_private_key: sign_private_key
)
end
end

saml_response = Base64.encode64(xml_signed)

Expand All @@ -406,7 +402,7 @@
end

after :each do
Object.send(:remove_const, :Rails) if defined?(::Rails)
Object.send(:remove_const, :Rails) if defined?(Rails)
end

it 'should set the info hash correctly' do
Expand Down Expand Up @@ -513,7 +509,7 @@
# The HETU is already set in the sample XML
it 'should set the uid to hashed HETU ID' do
expect(auth_hash['uid']).to eq(
'FIHETU:' + Digest::MD5.hexdigest("FI:210281-9988:#{uid_salt}")
"FIHETU:#{Digest::MD5.hexdigest("FI:210281-9988:#{uid_salt}")}"
)
end

Expand All @@ -523,7 +519,7 @@

it 'should set the uid to hashed eIDAS PID' do
expect(auth_hash['uid']).to eq(
'FIHETU:' + Digest::MD5.hexdigest("FI:210281-9988:#{rails_salt}")
"FIHETU:#{Digest::MD5.hexdigest("FI:210281-9988:#{rails_salt}")}"
)
end
end
Expand All @@ -533,7 +529,7 @@

it 'should set the uid to hashed eIDAS PID' do
expect(auth_hash['uid']).to eq(
'FIHETU:' + Digest::MD5.hexdigest('FI:210281-9988:')
"FIHETU:#{Digest::MD5.hexdigest('FI:210281-9988:')}"
)
end
end
Expand All @@ -555,7 +551,7 @@

it 'should set the uid to hashed eIDAS PID' do
expect(auth_hash['uid']).to eq(
'EIDASPID:' + Digest::MD5.hexdigest("EIDAS:28493196Z:#{uid_salt}")
"EIDASPID:#{Digest::MD5.hexdigest("EIDAS:28493196Z:#{uid_salt}")}"
)
end

Expand All @@ -565,7 +561,7 @@

it 'should set the uid to hashed eIDAS PID' do
expect(auth_hash['uid']).to eq(
'EIDASPID:' + Digest::MD5.hexdigest("EIDAS:28493196Z:#{rails_salt}")
"EIDASPID:#{Digest::MD5.hexdigest("EIDAS:28493196Z:#{rails_salt}")}"
)
end
end
Expand All @@ -575,7 +571,7 @@

it 'should set the uid to hashed eIDAS PID' do
expect(auth_hash['uid']).to eq(
'EIDASPID:' + Digest::MD5.hexdigest('EIDAS:28493196Z:')
"EIDASPID:#{Digest::MD5.hexdigest('EIDAS:28493196Z:')}"
)
end
end
Expand Down Expand Up @@ -734,9 +730,9 @@
it 'should call the application' do
expect(last_response.body).to eq(app_response)
expect(logout_request).to be_a(OneLogin::RubySaml::SloLogoutrequest)
expect(logout_response.scheme).to eq("https")
expect(logout_response.host).to eq("testi.apro.tunnistus.fi")
expect(logout_response.path).to eq("/idp/profile/SAML2/Redirect/SLO")
expect(logout_response.scheme).to eq('https')
expect(logout_response.host).to eq('testi.apro.tunnistus.fi')
expect(logout_response.path).to eq('/idp/profile/SAML2/Redirect/SLO')
expect(actual_relay_state).to eq('/')
end
end
Expand Down
2 changes: 1 addition & 1 deletion spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ def support_filepath(filename)
end

def support_file_io(filename)
IO.read(support_filepath(filename))
File.read(support_filepath(filename))
end

def base64_file(filename)
Expand Down

0 comments on commit 51fcb29

Please sign in to comment.