From 9268206dafb0ee8cae5f57e6348f1f93b8b8bfba Mon Sep 17 00:00:00 2001 From: olegphenomenon Date: Tue, 28 Dec 2021 12:26:48 +0200 Subject: [PATCH 1/3] added ability to validate invalid mx email by a and aaaa records --- .../actions/a_and_aaaa_email_validation.rb | 33 +++++++++++++++++++ app/interactions/actions/contact_create.rb | 6 ++-- app/interactions/actions/contact_update.rb | 6 ++-- app/interactions/actions/email_check.rb | 21 +++++++----- .../actions/simple_mail_validator.rb | 26 ++++++++++++++- test/interactions/email_check_test.rb | 8 ++--- 6 files changed, 80 insertions(+), 20 deletions(-) create mode 100644 app/interactions/actions/a_and_aaaa_email_validation.rb diff --git a/app/interactions/actions/a_and_aaaa_email_validation.rb b/app/interactions/actions/a_and_aaaa_email_validation.rb new file mode 100644 index 000000000..742bbaf21 --- /dev/null +++ b/app/interactions/actions/a_and_aaaa_email_validation.rb @@ -0,0 +1,33 @@ +module Actions + module AAndAaaaEmailValidation + extend self + + def call(email:, value:) + check_for_records_value(email: email, value: value) + end + + private + + def check_for_records_value(email:, value:) + email = Mail::Address.new(email).domain + result = nil + dns_servers = ENV['dnssec_resolver_ips'].to_s.split(',').map(&:strip) + + Resolv::DNS.open({ nameserver: dns_servers }) do |dns| + dns.timeouts = ENV['a_and_aaaa_validation_timeout'].to_i || 1 + ress = nil + + case value + when 'A' + ress = dns.getresources email, Resolv::DNS::Resource::IN::A + when 'AAAA' + ress = dns.getresources email, Resolv::DNS::Resource::IN::AAAA + end + + result = ress.map { |r| r.address } + end + + result + end + end +end diff --git a/app/interactions/actions/contact_create.rb b/app/interactions/actions/contact_create.rb index ad5612322..6fcdf8d1f 100644 --- a/app/interactions/actions/contact_create.rb +++ b/app/interactions/actions/contact_create.rb @@ -20,10 +20,10 @@ module Actions return if Rails.env.test? [:regex, :mx].each do |m| - r = Actions::SimpleMailValidator.run(email: contact.email, level: m) + result = Actions::SimpleMailValidator.run(email: contact.email, level: m) - unless r.success - contact.add_epp_error('2005', nil, r.errors, I18n.t(:parameter_value_syntax_error)) + unless result + contact.add_epp_error('2005', nil, "email didn't pass validation", I18n.t(:parameter_value_syntax_error)) @error = true return end diff --git a/app/interactions/actions/contact_update.rb b/app/interactions/actions/contact_update.rb index a18afed56..151376d3f 100644 --- a/app/interactions/actions/contact_update.rb +++ b/app/interactions/actions/contact_update.rb @@ -24,10 +24,10 @@ module Actions return if Rails.env.test? [:regex, :mx].each do |m| - r = Actions::SimpleMailValidator.run(email: @new_attributes[:email], level: m) + result = Actions::SimpleMailValidator.run(email: @new_attributes[:email], level: m) - unless r.success - contact.add_epp_error('2005', nil, r.errors, I18n.t(:parameter_value_syntax_error)) + unless result + contact.add_epp_error('2005', nil, "email didn't pass validation", I18n.t(:parameter_value_syntax_error)) @error = true return end diff --git a/app/interactions/actions/email_check.rb b/app/interactions/actions/email_check.rb index 5f4616bab..397e403f7 100644 --- a/app/interactions/actions/email_check.rb +++ b/app/interactions/actions/email_check.rb @@ -51,18 +51,17 @@ module Actions def save_result(result) if !result.success && @check_level == "mx" - email_domain = Mail::Address.new(@email).domain + result_validation = Actions::AAndAaaaEmailValidation.call(email: @email, value: 'A') + output_a_and_aaaa_validation_results(email: @email, + result: result_validation, + type: 'A') unless Rails.env.test? - result_validation = check_for_records_value(domain: email_domain, value: 'A') - logger.info "Validated A record for #{email_domain}. Validation result - #{result_validation}" - p "Validated A record for #{email_domain}. Validation result - #{result_validation}" - - result_validation = check_for_records_value(domain: email_domain, value: 'AAAA') if result_validation.empty? - logger.info "Validated AAAA record for #{email_domain}. Validation result - #{result_validation}" if result_validation.empty? - p "Validated AAAA record for #{email_domain}. Validation result - #{result_validation}" if result_validation.empty? + result_validation = Actions::AAndAaaaEmailValidation.call(email: @email, value: 'AAAA') if result_validation.empty? + output_a_and_aaaa_validation_results(email: @email, + result: result_validation, + type: 'AAAA') unless Rails.env.test? result_validation.present? ? result.success = true : result.success = false - validation_eventable.validation_events.create(validation_event_attrs(result)) else validation_eventable.validation_events.create(validation_event_attrs(result)) @@ -72,6 +71,10 @@ module Actions true end + def output_a_and_aaaa_validation_results(email:, result:, type: ) + logger.info "Validated #{type} record for #{email}. Validation result - #{result}" + end + def check_for_records_value(domain:, value:) result = nil dns_servers = ENV['dnssec_resolver_ips'].to_s.split(',').map(&:strip) diff --git a/app/interactions/actions/simple_mail_validator.rb b/app/interactions/actions/simple_mail_validator.rb index b96c3db46..815422f65 100644 --- a/app/interactions/actions/simple_mail_validator.rb +++ b/app/interactions/actions/simple_mail_validator.rb @@ -3,7 +3,31 @@ module Actions extend self def run(email:, level:) - Truemail.validate(email, with: level).result + result = Truemail.validate(email, with: level).result.success + result = validate_for_a_and_aaaa_records(email) if !result && level == :mx + result + end + + def validate_for_a_and_aaaa_records(email) + result_validation = Actions::AAndAaaaEmailValidation.call(email: email, value: 'A') + output_a_and_aaaa_validation_results(email: email, + result: result_validation, + type: 'A') unless Rails.env.test? + + result_validation = Actions::AAndAaaaEmailValidation.call(email: email, value: 'AAAA') if result_validation.empty? + output_a_and_aaaa_validation_results(email: email, + result: result_validation, + type: 'AAAA') unless Rails.env.test? + + result_validation.present? ? true : false + end + + def output_a_and_aaaa_validation_results(email:, result:, type: ) + logger.info "Validated #{type} record for #{email}. Validation result - #{result}" + end + + def logger + @logger ||= Rails.logger end end end diff --git a/test/interactions/email_check_test.rb b/test/interactions/email_check_test.rb index aa5fd8dc4..b3c458a6e 100644 --- a/test/interactions/email_check_test.rb +++ b/test/interactions/email_check_test.rb @@ -16,7 +16,7 @@ class DoRequestTest < ActiveSupport::TestCase ) Spy.on_instance_method(Actions::EmailCheck, :check_email).and_return(trumail_results) - Spy.on_instance_method(Actions::EmailCheck, :check_for_records_value).and_return([true]) + Spy.on_instance_method(Actions::AAndAaaaEmailValidation, :call).and_return([true]) action = Actions::EmailCheck.new(email: @contact.email, validation_eventable: @contact, @@ -35,7 +35,7 @@ class DoRequestTest < ActiveSupport::TestCase ) Spy.on_instance_method(Actions::EmailCheck, :check_email).and_return(trumail_results) - Spy.on_instance_method(Actions::EmailCheck, :check_for_records_value).and_return([]) + Spy.on_instance_method(Actions::AAndAaaaEmailValidation, :call).and_return([]) action = Actions::EmailCheck.new(email: @contact.email, validation_eventable: @contact, @@ -54,7 +54,7 @@ class DoRequestTest < ActiveSupport::TestCase ) Spy.on_instance_method(Actions::EmailCheck, :check_email).and_return(trumail_results) - Spy.on_instance_method(Actions::EmailCheck, :check_for_records_value).and_return([]) + Spy.on_instance_method(Actions::AAndAaaaEmailValidation, :call).and_return([]) action = Actions::EmailCheck.new(email: @contact.email, validation_eventable: @contact, @@ -82,7 +82,7 @@ class DoRequestTest < ActiveSupport::TestCase ) Spy.on_instance_method(Actions::EmailCheck, :check_email).and_return(trumail_results) - Spy.on_instance_method(Actions::EmailCheck, :check_for_records_value).and_return([true]) + Spy.on_instance_method(Actions::AAndAaaaEmailValidation, :call).and_return([true]) action = Actions::EmailCheck.new(email: @contact.email, validation_eventable: @contact, From b8be4ec928b8761817d9d08e6814cbb9ea2645da Mon Sep 17 00:00:00 2001 From: olegphenomenon Date: Tue, 28 Dec 2021 12:29:29 +0200 Subject: [PATCH 2/3] update test name --- test/interactions/email_check_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/interactions/email_check_test.rb b/test/interactions/email_check_test.rb index b3c458a6e..4e77a5bee 100644 --- a/test/interactions/email_check_test.rb +++ b/test/interactions/email_check_test.rb @@ -1,6 +1,6 @@ require 'test_helper' -class DoRequestTest < ActiveSupport::TestCase +class EmailCheckTest < ActiveSupport::TestCase setup do WebMock.disable_net_connect! From 7cb220e7046660404bc9bc015d6e0a06689423f7 Mon Sep 17 00:00:00 2001 From: olegphenomenon Date: Tue, 28 Dec 2021 12:40:18 +0200 Subject: [PATCH 3/3] made refactoring base codeclimate --- .../actions/a_and_aaaa_email_validation.rb | 15 ++++++++------- app/interactions/actions/contact_create.rb | 10 +++++----- app/interactions/actions/contact_update.rb | 9 ++++----- app/interactions/actions/email_check.rb | 8 +++++--- app/interactions/actions/simple_mail_validator.rb | 14 ++++++++++---- 5 files changed, 32 insertions(+), 24 deletions(-) diff --git a/app/interactions/actions/a_and_aaaa_email_validation.rb b/app/interactions/actions/a_and_aaaa_email_validation.rb index 742bbaf21..f0ec569f4 100644 --- a/app/interactions/actions/a_and_aaaa_email_validation.rb +++ b/app/interactions/actions/a_and_aaaa_email_validation.rb @@ -9,25 +9,26 @@ module Actions private def check_for_records_value(email:, value:) - email = Mail::Address.new(email).domain - result = nil + email_domain = Mail::Address.new(email).domain dns_servers = ENV['dnssec_resolver_ips'].to_s.split(',').map(&:strip) + resolve_a_and_aaaa_records(dns_servers: dns_servers, email_domain: email_domain, value: value) + end + + def resolve_a_and_aaaa_records(dns_servers:, email_domain:, value:) Resolv::DNS.open({ nameserver: dns_servers }) do |dns| dns.timeouts = ENV['a_and_aaaa_validation_timeout'].to_i || 1 ress = nil case value when 'A' - ress = dns.getresources email, Resolv::DNS::Resource::IN::A + ress = dns.getresources email_domain, Resolv::DNS::Resource::IN::A when 'AAAA' - ress = dns.getresources email, Resolv::DNS::Resource::IN::AAAA + ress = dns.getresources email_domain, Resolv::DNS::Resource::IN::AAAA end - result = ress.map { |r| r.address } + ress.map(&:address) end - - result end end end diff --git a/app/interactions/actions/contact_create.rb b/app/interactions/actions/contact_create.rb index 6fcdf8d1f..d31af2e38 100644 --- a/app/interactions/actions/contact_create.rb +++ b/app/interactions/actions/contact_create.rb @@ -22,11 +22,11 @@ module Actions [:regex, :mx].each do |m| result = Actions::SimpleMailValidator.run(email: contact.email, level: m) - unless result - contact.add_epp_error('2005', nil, "email didn't pass validation", I18n.t(:parameter_value_syntax_error)) - @error = true - return - end + next if result + + contact.add_epp_error('2005', nil, "email didn't pass validation", I18n.t(:parameter_value_syntax_error)) + @error = true + return end true diff --git a/app/interactions/actions/contact_update.rb b/app/interactions/actions/contact_update.rb index 151376d3f..0cf76d116 100644 --- a/app/interactions/actions/contact_update.rb +++ b/app/interactions/actions/contact_update.rb @@ -25,12 +25,11 @@ module Actions [:regex, :mx].each do |m| result = Actions::SimpleMailValidator.run(email: @new_attributes[:email], level: m) + next if result - unless result - contact.add_epp_error('2005', nil, "email didn't pass validation", I18n.t(:parameter_value_syntax_error)) - @error = true - return - end + contact.add_epp_error('2005', nil, "email didn't pass validation", I18n.t(:parameter_value_syntax_error)) + @error = true + return end true diff --git a/app/interactions/actions/email_check.rb b/app/interactions/actions/email_check.rb index 397e403f7..b358db08d 100644 --- a/app/interactions/actions/email_check.rb +++ b/app/interactions/actions/email_check.rb @@ -54,12 +54,12 @@ module Actions result_validation = Actions::AAndAaaaEmailValidation.call(email: @email, value: 'A') output_a_and_aaaa_validation_results(email: @email, result: result_validation, - type: 'A') unless Rails.env.test? + type: 'A') result_validation = Actions::AAndAaaaEmailValidation.call(email: @email, value: 'AAAA') if result_validation.empty? output_a_and_aaaa_validation_results(email: @email, result: result_validation, - type: 'AAAA') unless Rails.env.test? + type: 'AAAA') result_validation.present? ? result.success = true : result.success = false validation_eventable.validation_events.create(validation_event_attrs(result)) @@ -71,7 +71,9 @@ module Actions true end - def output_a_and_aaaa_validation_results(email:, result:, type: ) + def output_a_and_aaaa_validation_results(email:, result:, type:) + return if Rails.env.test? + logger.info "Validated #{type} record for #{email}. Validation result - #{result}" end diff --git a/app/interactions/actions/simple_mail_validator.rb b/app/interactions/actions/simple_mail_validator.rb index 815422f65..7d945e875 100644 --- a/app/interactions/actions/simple_mail_validator.rb +++ b/app/interactions/actions/simple_mail_validator.rb @@ -3,26 +3,32 @@ module Actions extend self def run(email:, level:) - result = Truemail.validate(email, with: level).result.success + result = truemail_validate(email: email, level: level) result = validate_for_a_and_aaaa_records(email) if !result && level == :mx result end + def truemail_validate(email:, level:) + Truemail.validate(email, with: level).result.success + end + def validate_for_a_and_aaaa_records(email) result_validation = Actions::AAndAaaaEmailValidation.call(email: email, value: 'A') output_a_and_aaaa_validation_results(email: email, result: result_validation, - type: 'A') unless Rails.env.test? + type: 'A') result_validation = Actions::AAndAaaaEmailValidation.call(email: email, value: 'AAAA') if result_validation.empty? output_a_and_aaaa_validation_results(email: email, result: result_validation, - type: 'AAAA') unless Rails.env.test? + type: 'AAAA') result_validation.present? ? true : false end - def output_a_and_aaaa_validation_results(email:, result:, type: ) + def output_a_and_aaaa_validation_results(email:, result:, type:) + return if Rails.env.test? + logger.info "Validated #{type} record for #{email}. Validation result - #{result}" end