From 4a6742692a0c71d8b4de8c4f8c395e6a29ef2cf2 Mon Sep 17 00:00:00 2001 From: Maciej Szlosarczyk Date: Wed, 15 Aug 2018 12:47:27 +0300 Subject: [PATCH 1/6] Fix issues raising from upgrade to Ruby 2.4 --- .ruby-version | 2 +- Dockerfile | 2 +- lib/auth_token/auth_token_creator.rb | 5 ++++- lib/auth_token/auth_token_decryptor.rb | 5 ++++- test/lib/auth_token/auth_token_creator_test.rb | 2 +- 5 files changed, 11 insertions(+), 5 deletions(-) diff --git a/.ruby-version b/.ruby-version index 00355e29d..35cee72dc 100644 --- a/.ruby-version +++ b/.ruby-version @@ -1 +1 @@ -2.3.7 +2.4.3 diff --git a/Dockerfile b/Dockerfile index bd0cbc07b..b5871bfed 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,4 +1,4 @@ -FROM internetee/ruby:2.3 +FROM internetee/ruby:2.4 MAINTAINER maciej.szlosarczyk@internet.ee RUN mkdir -p /opt/webapps/app/tmp/pids diff --git a/lib/auth_token/auth_token_creator.rb b/lib/auth_token/auth_token_creator.rb index 9fff8e5cd..741cc3e8a 100644 --- a/lib/auth_token/auth_token_creator.rb +++ b/lib/auth_token/auth_token_creator.rb @@ -26,7 +26,10 @@ class AuthTokenCreator def encrypted_token encryptor = OpenSSL::Cipher::AES.new(256, :CBC) encryptor.encrypt - encryptor.key = key + + # OpenSSL used to automatically shrink oversized keys, it does not do that any longer. + # See: https://github.com/ruby/openssl/issues/116 + encryptor.key = key[0..31] encrypted_bytes = encryptor.update(hashable) + encryptor.final Base64.urlsafe_encode64(encrypted_bytes) end diff --git a/lib/auth_token/auth_token_decryptor.rb b/lib/auth_token/auth_token_decryptor.rb index be6bd99cd..acd67be99 100644 --- a/lib/auth_token/auth_token_decryptor.rb +++ b/lib/auth_token/auth_token_decryptor.rb @@ -16,7 +16,10 @@ class AuthTokenDecryptor def decrypt_token decipher = OpenSSL::Cipher::AES.new(256, :CBC) decipher.decrypt - decipher.key = key + + # OpenSSL used to automatically shrink oversized keys, it does not do that any longer. + # See: https://github.com/ruby/openssl/issues/116 + decipher.key = key[0..31] base64_decoded = Base64.urlsafe_decode64(token.to_s) plain = decipher.update(base64_decoded) + decipher.final diff --git a/test/lib/auth_token/auth_token_creator_test.rb b/test/lib/auth_token/auth_token_creator_test.rb index 9d4cdd2c6..0465de9f4 100644 --- a/test/lib/auth_token/auth_token_creator_test.rb +++ b/test/lib/auth_token/auth_token_creator_test.rb @@ -8,7 +8,7 @@ class AuthTokenCreatorTest < ActiveSupport::TestCase @user = users(:registrant) time = Time.zone.parse('2010-07-05 00:30:00 +0000') - @random_bytes = SecureRandom.random_bytes(64) + @random_bytes = SecureRandom.random_bytes(32) @token_creator = AuthTokenCreator.new(@user, @random_bytes, time) end From db08ad712516c445c22a11531e3594f40c0945ce Mon Sep 17 00:00:00 2001 From: Maciej Szlosarczyk Date: Fri, 17 Aug 2018 10:21:12 +0300 Subject: [PATCH 2/6] Use Ruby 2.4.4 instead of 2.4.3 --- .ruby-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.ruby-version b/.ruby-version index 35cee72dc..79a614418 100644 --- a/.ruby-version +++ b/.ruby-version @@ -1 +1 @@ -2.4.3 +2.4.4 From 82dbd3e8b844d1ca973bb1462fc0e04848d9b759 Mon Sep 17 00:00:00 2001 From: Maciej Szlosarczyk Date: Fri, 17 Aug 2018 10:36:52 +0300 Subject: [PATCH 3/6] Use match? instead of `match` for regex where MatchData is unused --- app/api/repp/domain_v1.rb | 2 +- app/controllers/epp_controller.rb | 2 +- app/models/certificate.rb | 4 ++-- app/models/concerns/versions.rb | 4 ++-- app/models/nameserver.rb | 4 ++-- app/validators/contact/ident/reg_no_validator.rb | 4 +++- app/validators/domain_name_validator.rb | 2 +- 7 files changed, 12 insertions(+), 10 deletions(-) diff --git a/app/api/repp/domain_v1.rb b/app/api/repp/domain_v1.rb index 5e6c286ca..cf45bfc6f 100644 --- a/app/api/repp/domain_v1.rb +++ b/app/api/repp/domain_v1.rb @@ -29,7 +29,7 @@ module Repp # example: curl -u registrar1:password localhost:3000/repp/v1/domains/1/transfer_info -H "Auth-Code: authinfopw1" get '/:id/transfer_info', requirements: { id: /.*/ } do ident = params[:id] - domain = ident =~ /\A[0-9]+\z/ ? Domain.find_by(id: ident) : Domain.find_by_idn(ident) + domain = ident.match?(/\A[0-9]+\z/) ? Domain.find_by(id: ident) : Domain.find_by_idn(ident) error! I18n.t('errors.messages.epp_domain_not_found'), 404 unless domain error! I18n.t('errors.messages.epp_authorization_error'), 401 unless domain.transfer_code.eql? request.headers['Auth-Code'] diff --git a/app/controllers/epp_controller.rb b/app/controllers/epp_controller.rb index 93da33ead..551f065a7 100644 --- a/app/controllers/epp_controller.rb +++ b/app/controllers/epp_controller.rb @@ -145,7 +145,7 @@ class EppController < ApplicationController # VALIDATION def latin_only return true if params['frame'].blank? - return true if params['frame'].match(/\A[\p{Latin}\p{Z}\p{P}\p{S}\p{Cc}\p{Cf}\w_\'\+\-\.\(\)\/]*\Z/i) + return true if params['frame'].match?(/\A[\p{Latin}\p{Z}\p{P}\p{S}\p{Cc}\p{Cf}\w_\'\+\-\.\(\)\/]*\Z/i) epp_errors << { msg: 'Parameter value policy error. Allowed only Latin characters.', diff --git a/app/models/certificate.rb b/app/models/certificate.rb index 50976a73e..cb28f629b 100644 --- a/app/models/certificate.rb +++ b/app/models/certificate.rb @@ -87,14 +87,14 @@ class Certificate < ActiveRecord::Base -extensions usr_cert -notext -md sha256 \ -in #{csr_file.path} -out #{crt_file.path} -key '#{ENV['ca_key_password']}' -batch") - if err.match(/Data Base Updated/) + if err.match?(/Data Base Updated/) crt_file.rewind self.crt = crt_file.read self.md5 = OpenSSL::Digest::MD5.new(parsed_crt.to_der).to_s save! else logger.error('FAILED TO CREATE CLIENT CERTIFICATE') - if err.match(/TXT_DB error number 2/) + if err.match?(/TXT_DB error number 2/) errors.add(:base, I18n.t('failed_to_create_crt_csr_already_signed')) logger.error('CSR ALREADY SIGNED') else diff --git a/app/models/concerns/versions.rb b/app/models/concerns/versions.rb index 5e2bad90c..5a0ad5705 100644 --- a/app/models/concerns/versions.rb +++ b/app/models/concerns/versions.rb @@ -34,9 +34,9 @@ module Versions end def user_from_id_role_username(str) - user = ApiUser.find_by(id: $1) if str =~ /^(\d+)-(ApiUser:|api-)/ + user = ApiUser.find_by(id: $1) if str.match(/^(\d+)-(ApiUser:|api-)/) unless user.present? - user = AdminUser.find_by(id: $1) if str =~ /^(\d+)-AdminUser:/ + user = AdminUser.find_by(id: $1) if str.match(/^(\d+)-AdminUser:/) unless user.present? # on import we copied Registrar name, which may eql code registrar = Registrar.find_by(name: str) diff --git a/app/models/nameserver.rb b/app/models/nameserver.rb index d9a5f2a75..85792f5da 100644 --- a/app/models/nameserver.rb +++ b/app/models/nameserver.rb @@ -105,13 +105,13 @@ class Nameserver < ActiveRecord::Base def validate_ipv4_format ipv4.to_a.each do |ip| - errors.add(:ipv4, :invalid) unless ip =~ IPV4_REGEXP + errors.add(:ipv4, :invalid) unless ip.match?(IPV4_REGEXP) end end def validate_ipv6_format ipv6.to_a.each do |ip| - errors.add(:ipv6, :invalid) unless ip =~ IPV6_REGEXP + errors.add(:ipv6, :invalid) unless ip.match?(IPV6_REGEXP) end end end diff --git a/app/validators/contact/ident/reg_no_validator.rb b/app/validators/contact/ident/reg_no_validator.rb index 611d13301..9d829a50b 100644 --- a/app/validators/contact/ident/reg_no_validator.rb +++ b/app/validators/contact/ident/reg_no_validator.rb @@ -10,7 +10,9 @@ class Contact::Ident::RegNoValidator < ActiveModel::EachValidator return unless format - record.errors.add(attribute, :invalid_reg_no, country: record.country) unless value =~ format + unless value.match?(format) + record.errors.add(attribute, :invalid_reg_no, country: record.country) + end end private diff --git a/app/validators/domain_name_validator.rb b/app/validators/domain_name_validator.rb index 22fe0cb34..0d5638b37 100644 --- a/app/validators/domain_name_validator.rb +++ b/app/validators/domain_name_validator.rb @@ -22,7 +22,7 @@ class DomainNameValidator < ActiveModel::EachValidator # it's punycode if value[2] == '-' && value[3] == '-' regexp = /\Axn--[a-zA-Z0-9-]{0,59}\.#{general_domains}\z/ - return false unless value =~ regexp + return false unless value.match?(regexp) value = SimpleIDN.to_unicode(value).mb_chars.downcase.strip end From cf16aa40395a276483795021f4f8812b3399d69a Mon Sep 17 00:00:00 2001 From: Maciej Szlosarczyk Date: Fri, 17 Aug 2018 11:03:01 +0300 Subject: [PATCH 4/6] Fix rubocop issues that are new to Ruby 2.4 --- app/controllers/epp_controller.rb | 4 +++- app/models/concerns/versions.rb | 8 ++++++-- app/models/nameserver.rb | 2 +- app/validators/contact/ident/reg_no_validator.rb | 5 ++--- 4 files changed, 12 insertions(+), 7 deletions(-) diff --git a/app/controllers/epp_controller.rb b/app/controllers/epp_controller.rb index 551f065a7..496526d71 100644 --- a/app/controllers/epp_controller.rb +++ b/app/controllers/epp_controller.rb @@ -145,7 +145,9 @@ class EppController < ApplicationController # VALIDATION def latin_only return true if params['frame'].blank? - return true if params['frame'].match?(/\A[\p{Latin}\p{Z}\p{P}\p{S}\p{Cc}\p{Cf}\w_\'\+\-\.\(\)\/]*\Z/i) + if params['frame'].match?(/\A[\p{Latin}\p{Z}\p{P}\p{S}\p{Cc}\p{Cf}\w_\'\+\-\.\(\)\/]*\Z/i) + return true + end epp_errors << { msg: 'Parameter value policy error. Allowed only Latin characters.', diff --git a/app/models/concerns/versions.rb b/app/models/concerns/versions.rb index 5a0ad5705..91926272c 100644 --- a/app/models/concerns/versions.rb +++ b/app/models/concerns/versions.rb @@ -34,9 +34,13 @@ module Versions end def user_from_id_role_username(str) - user = ApiUser.find_by(id: $1) if str.match(/^(\d+)-(ApiUser:|api-)/) + if str.match(/^(\d+)-(ApiUser:|api-)/) + user = ApiUser.find_by(id: $1) + end unless user.present? - user = AdminUser.find_by(id: $1) if str.match(/^(\d+)-AdminUser:/) + user = if str.match(/^(\d+)-AdminUser:/) + AdminUser.find_by(id: $1) + end unless user.present? # on import we copied Registrar name, which may eql code registrar = Registrar.find_by(name: str) diff --git a/app/models/nameserver.rb b/app/models/nameserver.rb index 85792f5da..d9fdde406 100644 --- a/app/models/nameserver.rb +++ b/app/models/nameserver.rb @@ -100,7 +100,7 @@ class Nameserver < ActiveRecord::Base def check_puny_symbols regexp = /(\A|\.)..--/ - errors.add(:hostname, :invalid) if hostname =~ regexp + errors.add(:hostname, :invalid) if hostname.match?(regexp) end def validate_ipv4_format diff --git a/app/validators/contact/ident/reg_no_validator.rb b/app/validators/contact/ident/reg_no_validator.rb index 9d829a50b..138aab56a 100644 --- a/app/validators/contact/ident/reg_no_validator.rb +++ b/app/validators/contact/ident/reg_no_validator.rb @@ -10,9 +10,8 @@ class Contact::Ident::RegNoValidator < ActiveModel::EachValidator return unless format - unless value.match?(format) - record.errors.add(attribute, :invalid_reg_no, country: record.country) - end + return if value.match?(format) + record.errors.add(attribute, :invalid_reg_no, country: record.country) end private From 98f55fd48a7dbbfc22d16352104e815bd6da802e Mon Sep 17 00:00:00 2001 From: Maciej Szlosarczyk Date: Fri, 17 Aug 2018 11:19:49 +0300 Subject: [PATCH 5/6] Simplify a method to remove Rubocop violation --- app/models/concerns/versions.rb | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/app/models/concerns/versions.rb b/app/models/concerns/versions.rb index 91926272c..7075303e9 100644 --- a/app/models/concerns/versions.rb +++ b/app/models/concerns/versions.rb @@ -34,20 +34,14 @@ module Versions end def user_from_id_role_username(str) - if str.match(/^(\d+)-(ApiUser:|api-)/) - user = ApiUser.find_by(id: $1) - end - unless user.present? - user = if str.match(/^(\d+)-AdminUser:/) - AdminUser.find_by(id: $1) - end - unless user.present? - # on import we copied Registrar name, which may eql code - registrar = Registrar.find_by(name: str) - # assume each registrar has only one user - user = registrar.api_users.first if registrar - end + registrar = Registrar.find_by(name: str) + user = registrar.api_users.first if registrar + str_match = str.match(/^(\d+)-(ApiUser:|api-|AdminUser:)/) + + if str_match + user ||= User.find_by(id: str_match[1]) end + user end From b57adca62480ce0f18ad600c186831dfd0e2594d Mon Sep 17 00:00:00 2001 From: Maciej Szlosarczyk Date: Fri, 17 Aug 2018 11:24:00 +0300 Subject: [PATCH 6/6] Use `if` modifier instead of an `if` block --- app/models/concerns/versions.rb | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/app/models/concerns/versions.rb b/app/models/concerns/versions.rb index 7075303e9..53a984179 100644 --- a/app/models/concerns/versions.rb +++ b/app/models/concerns/versions.rb @@ -36,11 +36,9 @@ module Versions def user_from_id_role_username(str) registrar = Registrar.find_by(name: str) user = registrar.api_users.first if registrar - str_match = str.match(/^(\d+)-(ApiUser:|api-|AdminUser:)/) - if str_match - user ||= User.find_by(id: str_match[1]) - end + str_match = str.match(/^(\d+)-(ApiUser:|api-|AdminUser:)/) + user ||= User.find_by(id: str_match[1]) if str_match user end