From 287c4ea5d60bc42ee81669d897231dcac7d7280b Mon Sep 17 00:00:00 2001 From: olegphenomenon Date: Thu, 6 Jan 2022 14:25:12 +0200 Subject: [PATCH] remove nameserver validation from update and create interactions, refactored tests, job and move validator to services scoup --- app/interactions/actions/domain_create.rb | 26 ---------- app/interactions/actions/domain_update.rb | 28 ---------- .../domains/nameserver_validator.rb | 51 ------------------- app/jobs/nameserver_record_validation_job.rb | 37 ++++++++++---- app/services/nameserver_validator.rb | 47 +++++++++++++++++ .../epp/domain/update/base_test.rb | 4 +- .../epp/domain/update/rem_dns_test.rb | 4 +- .../repp/v1/domains/contacts_test.rb | 1 - .../repp/v1/domains/create_test.rb | 1 - .../repp/v1/domains/dnssec_test.rb | 1 - .../repp/v1/domains/nameservers_test.rb | 2 - .../repp/v1/domains/update_test.rb | 1 - test/jobs/domain_update_confirm_job_test.rb | 2 - 13 files changed, 76 insertions(+), 129 deletions(-) delete mode 100644 app/interactions/domains/nameserver_validator.rb create mode 100644 app/services/nameserver_validator.rb diff --git a/app/interactions/actions/domain_create.rb b/app/interactions/actions/domain_create.rb index 646cf2e7e..8fd25df0f 100644 --- a/app/interactions/actions/domain_create.rb +++ b/app/interactions/actions/domain_create.rb @@ -14,7 +14,6 @@ module Actions assign_registrant assign_nameservers - check_for_valid_nameserver assign_domain_contacts domain.attach_default_contacts assign_expiry_time @@ -23,31 +22,6 @@ module Actions commit end - def check_for_valid_nameserver - nameservers_data = params[:nameservers_attributes] - - nameservers_data.each do |nameserver| - result = parse_nameserver_hash(nameserver) - - next unless result - end - end - - def parse_nameserver_hash(nameserver) - name = params[:name].strip.downcase - result = Domains::NameserverValidator.run(domain_name: name, nameserver_address: nameserver[:hostname]) - - return true if result[:result] - - if result[:reason] == 'exception' - reason = result[:error_info] - else - reason = result[:reason] - end - - domain.add_epp_error('2303', nil, reason, 'Problem with nameserver: ') - end - def check_contact_duplications if check_for_same_contacts(@admin_contacts, 'admin') && check_for_same_contacts(@tech_contacts, 'tech') diff --git a/app/interactions/actions/domain_update.rb b/app/interactions/actions/domain_update.rb index 2fdbd72c0..40b7876f6 100644 --- a/app/interactions/actions/domain_update.rb +++ b/app/interactions/actions/domain_update.rb @@ -14,7 +14,6 @@ module Actions assign_new_registrant if params[:registrant] assign_relational_modifications assign_requested_statuses - check_for_valid_nameserver ::Actions::BaseAction.maybe_attach_legal_doc(domain, params[:legal_document]) commit @@ -29,33 +28,6 @@ module Actions assign_tech_contact_changes end - def check_for_valid_nameserver - nameservers_data = params[:nameservers] - - nameservers_data.each do |nameserver| - result = parse_nameserver_hash(nameserver) - - next unless result - end - end - - def parse_nameserver_hash(nameserver) - return false unless nameserver[:action] == "add" - - name = params[:domain].strip.downcase - result = Domains::NameserverValidator.run(domain_name: name, nameserver_address: nameserver[:hostname]) - - return true if result[:result] - - if result[:reason] == 'exception' - reason = result[:error_info] - else - reason = result[:reason] - end - - domain.add_epp_error('2303', nil, reason, 'Problem with nameserver: ') - end - def check_for_same_contacts(contacts, contact_type) return unless contacts.uniq.count != contacts.count diff --git a/app/interactions/domains/nameserver_validator.rb b/app/interactions/domains/nameserver_validator.rb deleted file mode 100644 index f2efb90dd..000000000 --- a/app/interactions/domains/nameserver_validator.rb +++ /dev/null @@ -1,51 +0,0 @@ -module Domains - module NameserverValidator - include Dnsruby - - extend self - - def run(domain_name:, nameserver_address:) - validate(hostname: domain_name, nameserver_address: nameserver_address) - end - - private - - def validate(hostname: , nameserver_address:) - resolver = Resolver.new - resolver.nameserver = nameserver_address - result = resolver.query(hostname, Dnsruby::Types.SOA) - - return { result: false, reason: 'answer' } if result.answer.empty? - - decision = result.answer.all? do |a| - a.serial.present? - end - - return { result: false, reason: 'serial' } unless decision - - { result: true, reason: '' } - rescue Dnsruby::NXDomain => e - logger.info "#{e} - seems hostname don't found" - return { result: false, reason: 'not found' } - rescue StandardError => e - logger.info e - return { result: false, reason: 'exception', error_info: e } - end - - def setup_resolver(nameserver_address) - # resolver.do_validation=true - # resolver.query_timeout=1 - # resolver.single_resolvers[0].server='ns.tld.ee' - timeout = ENV['nameserver_validation_timeout'] || '1' - # dns_servers = ENV['dnssec_resolver_ips'].to_s.split(',').map(&:strip) - # Resolver.new({nameserver: dns_servers, timeout: timeout.to_i}) - resolver = Resolver.new - resolver.nameserver = nameserver_address - - end - - def logger - @logger ||= Rails.logger - end - end -end diff --git a/app/jobs/nameserver_record_validation_job.rb b/app/jobs/nameserver_record_validation_job.rb index 6cb1647ac..a3be73aaf 100644 --- a/app/jobs/nameserver_record_validation_job.rb +++ b/app/jobs/nameserver_record_validation_job.rb @@ -5,10 +5,32 @@ require 'resolv' class NameserverRecordValidationJob < ApplicationJob include Dnsruby - def perform(domain_name: nil, nameserver_address: nil) - if nameserver_address.nil? - Nameserver.all.map do |nameserver| - result = Domains::NameserverValidator.run(domain_name: nameserver.domain.name, nameserver_address: nameserver.hostname) + def perform(domain_name: nil) + if domain_name.nil? + domains = Domain.all.select { |domain| domain.nameservers.exists? }. + select { |domain| domain.created_at < Time.zone.now - 8.hours } + + domains.each do |domain| + domain.nameservers.each do |nameserver| + result = NameserverValidator.run(domain_name: domain.name, hostname: nameserver.hostname) + + if result[:result] + true + else + parse_result(result, nameserver) + false + end + end + end + else + domain = Domain.find_by(name: domain_name) + + return logger.info 'Domain not found' if domain.nil? + return logger.info 'It should take 8 hours after the domain was created' if domain.created_at > Time.zone.now - 8.hours + return logger.info 'Domain not has nameservers' if domain.nameservers.empty? + + domain.nameservers.each do |nameserver| + result = NameserverValidator.run(domain_name: domain.name, hostname: nameserver.hostname) if result[:result] true @@ -17,11 +39,6 @@ class NameserverRecordValidationJob < ApplicationJob false end end - else - result = Domains::NameserverValidator.run(domain_name: domain_name, nameserver_address: nameserver_address) - return parse_result(result, nameserver_address) unless result[:result] - - true end end @@ -33,7 +50,7 @@ class NameserverRecordValidationJob < ApplicationJob when 'answer' text = "No any answer come from **#{nameserver}**" when 'serial' - text = "Serial number for nameserver hostname **#{nameserver}** doesn't present" + text = "Serial number for nameserver hostname **#{nameserver}** doesn't present. Seems nameservers out the zone" when 'not found' text = "Seems nameserver hostname **#{nameserver}** doesn't exist" when 'exception' diff --git a/app/services/nameserver_validator.rb b/app/services/nameserver_validator.rb new file mode 100644 index 000000000..4fd3a7e55 --- /dev/null +++ b/app/services/nameserver_validator.rb @@ -0,0 +1,47 @@ +module NameserverValidator + include Dnsruby + + extend self + + def run(domain_name:, hostname:) + validate(domain_name: domain_name, hostname: hostname) + end + + private + + def validate(domain_name: , hostname:) + resolver = setup_resolver(hostname) + result = resolver.query(domain_name, 'SOA', 'IN') + + return { result: false, reason: 'answer' } if result.answer.empty? + + decision = result.answer.all? do |a| + a.serial.present? + end + + return { result: false, reason: 'serial' } unless decision + + logger.info "Serial number - #{result.answer[0].serial.to_s} of #{hostname} - domain name: #{domain_name}" + + { result: true, reason: '' } + rescue StandardError => e + logger.error e.message + logger.error "failed #{hostname} validation of #{domain_name} domain name" + return { result: false, reason: 'exception', error_info: e } + end + + def setup_resolver(hostname) + resolver = Dnsruby::Resolver.new + resolver.retry_times = 3 + resolver.recurse = 0 # Send out non-recursive queries + # disable caching otherwise SOA is cached from first nameserver queried + resolver.do_caching = false + resolver.nameserver = hostname + + resolver + end + + def logger + @logger ||= Rails.logger + end +end diff --git a/test/integration/epp/domain/update/base_test.rb b/test/integration/epp/domain/update/base_test.rb index f11007978..d021b496d 100644 --- a/test/integration/epp/domain/update/base_test.rb +++ b/test/integration/epp/domain/update/base_test.rb @@ -8,10 +8,8 @@ class EppDomainUpdateBaseTest < EppTestCase @domain = domains(:shop) @contact = contacts(:john) @original_registrant_change_verification = - Setting.request_confirmation_on_registrant_change_enabled + Setting.request_confirmation_on_registrant_change_enabled ActionMailer::Base.deliveries.clear - - Spy.on_instance_method(Actions::DomainUpdate, :check_for_valid_nameserver).and_return(true) end teardown do diff --git a/test/integration/epp/domain/update/rem_dns_test.rb b/test/integration/epp/domain/update/rem_dns_test.rb index c2a624e29..6e079b126 100644 --- a/test/integration/epp/domain/update/rem_dns_test.rb +++ b/test/integration/epp/domain/update/rem_dns_test.rb @@ -10,10 +10,8 @@ class EppDomainUpdateRemDnsTest < EppTestCase @dnskey = dnskeys(:one) @dnskey.update(domain: @domain) @original_registrant_change_verification = - Setting.request_confirmation_on_registrant_change_enabled + Setting.request_confirmation_on_registrant_change_enabled ActionMailer::Base.deliveries.clear - - Spy.on_instance_method(Actions::DomainUpdate, :check_for_valid_nameserver).and_return(true) end teardown do diff --git a/test/integration/repp/v1/domains/contacts_test.rb b/test/integration/repp/v1/domains/contacts_test.rb index 229511a99..b9b26a745 100644 --- a/test/integration/repp/v1/domains/contacts_test.rb +++ b/test/integration/repp/v1/domains/contacts_test.rb @@ -8,7 +8,6 @@ class ReppV1DomainsContactsTest < ActionDispatch::IntegrationTest token = "Basic #{token}" @auth_headers = { 'Authorization' => token } - Spy.on_instance_method(Actions::DomainUpdate, :check_for_valid_nameserver).and_return(true) end def test_shows_existing_domain_contacts diff --git a/test/integration/repp/v1/domains/create_test.rb b/test/integration/repp/v1/domains/create_test.rb index 1c04c46d9..7907e709e 100644 --- a/test/integration/repp/v1/domains/create_test.rb +++ b/test/integration/repp/v1/domains/create_test.rb @@ -8,7 +8,6 @@ class ReppV1DomainsCreateTest < ActionDispatch::IntegrationTest token = "Basic #{token}" @auth_headers = { 'Authorization' => token } - Spy.on_instance_method(Actions::DomainCreate, :check_for_valid_nameserver).and_return(true) end def test_creates_new_domain_successfully diff --git a/test/integration/repp/v1/domains/dnssec_test.rb b/test/integration/repp/v1/domains/dnssec_test.rb index 5363729e9..6835e2600 100644 --- a/test/integration/repp/v1/domains/dnssec_test.rb +++ b/test/integration/repp/v1/domains/dnssec_test.rb @@ -8,7 +8,6 @@ class ReppV1DomainsDnssecTest < ActionDispatch::IntegrationTest token = "Basic #{token}" @auth_headers = { 'Authorization' => token } - Spy.on_instance_method(Actions::DomainUpdate, :check_for_valid_nameserver).and_return(true) end def test_shows_dnssec_keys_associated_with_domain diff --git a/test/integration/repp/v1/domains/nameservers_test.rb b/test/integration/repp/v1/domains/nameservers_test.rb index dd8434215..780e889c1 100644 --- a/test/integration/repp/v1/domains/nameservers_test.rb +++ b/test/integration/repp/v1/domains/nameservers_test.rb @@ -8,8 +8,6 @@ class ReppV1DomainsNameserversTest < ActionDispatch::IntegrationTest token = "Basic #{token}" @auth_headers = { 'Authorization' => token } - - Spy.on_instance_method(Actions::DomainUpdate, :check_for_valid_nameserver).and_return(true) end def test_can_add_new_nameserver diff --git a/test/integration/repp/v1/domains/update_test.rb b/test/integration/repp/v1/domains/update_test.rb index b60bdd804..d924fe7a3 100644 --- a/test/integration/repp/v1/domains/update_test.rb +++ b/test/integration/repp/v1/domains/update_test.rb @@ -8,7 +8,6 @@ class ReppV1DomainsUpdateTest < ActionDispatch::IntegrationTest token = "Basic #{token}" @auth_headers = { 'Authorization' => token } - Spy.on_instance_method(Actions::DomainUpdate, :check_for_valid_nameserver).and_return(true) end def test_updates_transfer_code_for_domain diff --git a/test/jobs/domain_update_confirm_job_test.rb b/test/jobs/domain_update_confirm_job_test.rb index 4d7cc3a9c..158729ae3 100644 --- a/test/jobs/domain_update_confirm_job_test.rb +++ b/test/jobs/domain_update_confirm_job_test.rb @@ -14,8 +14,6 @@ class DomainUpdateConfirmJobTest < ActiveSupport::TestCase new_registrant_name: @new_registrant.name, new_registrant_email: @new_registrant.email, current_user_id: @user.id }) - - Spy.on_instance_method(Actions::DomainUpdate, :check_for_valid_nameserver).and_return(true) end def teardown