diff --git a/Gemfile b/Gemfile index e51b66ac2b..5e79a594df 100644 --- a/Gemfile +++ b/Gemfile @@ -17,7 +17,7 @@ gem 'figaro', '~> 1.2' # model related gem 'paper_trail', '~> 14.0' -gem 'pg', '1.5.7' +gem 'pg', '1.5.6' # 1.8 is for Rails < 5.0 gem 'ransack', '~> 4.0.0' gem 'truemail', '~> 3.0' # validates email by regexp, mail server existence and address existence @@ -43,7 +43,7 @@ gem 'data_migrate', '~> 9.0' gem 'dnsruby', '~> 1.61' gem 'isikukood' # for EE-id validation gem 'money-rails' -gem 'simpleidn', '0.2.3' # For punycode +gem 'simpleidn', '0.2.2' # For punycode gem 'whenever', '1.0.0', require: false # country listing @@ -78,7 +78,9 @@ gem 'rexml' gem 'wkhtmltopdf-binary', '~> 0.12.6.1' gem 'directo', github: 'internetee/directo', branch: 'master' - +gem 'net-ftp' +gem 'net-pop' +gem 'net-imap' gem 'strong_migrations' group :development, :test do diff --git a/Gemfile.lock b/Gemfile.lock index e4195ae935..1fcbeaf6cd 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -211,6 +211,7 @@ GEM activerecord (>= 5.a) database_cleaner-core (~> 2.0.0) database_cleaner-core (2.0.1) + date (3.3.4) devise (4.8.0) bcrypt (~> 3.0) orm_adapter (~> 0.1) @@ -293,7 +294,7 @@ GEM activerecord kaminari-core (= 1.2.1) kaminari-core (1.2.1) - libxml-ruby (3.2.1) + libxml-ruby (5.0.3) logger (1.4.3) loofah (2.21.4) crass (~> 1.0.2) @@ -322,6 +323,14 @@ GEM money (~> 6.13.2) railties (>= 3.0) msgpack (1.7.2) + net-ftp (0.3.7) + net-protocol + time + net-imap (0.4.14) + date + net-protocol + net-pop (0.1.2) + net-protocol net-protocol (0.1.3) timeout net-smtp (0.3.3) @@ -361,7 +370,7 @@ GEM activerecord (>= 6.0) request_store (~> 1.4) pdfkit (0.8.7.2) - pg (1.5.7) + pg (1.5.6) pg_query (2.1.2) google-protobuf (>= 3.17.1) pghero (3.1.0) @@ -469,7 +478,8 @@ GEM json (>= 1.8, < 3) simplecov-html (~> 0.10.0) simplecov-html (0.10.2) - simpleidn (0.2.3) + simpleidn (0.2.2) + unf (~> 0.1.4) sixarm_ruby_unaccent (1.2.0) socksify (1.7.1) sprockets (4.0.3) @@ -490,6 +500,8 @@ GEM temple (0.8.2) thor (1.2.2) tilt (2.0.11) + time (0.3.0) + date timeout (0.3.0) truemail (3.0.3) simpleidn (~> 0.2.1) @@ -572,6 +584,9 @@ DEPENDENCIES minitest (~> 5.17) minitest-stub_any_instance money-rails + net-ftp + net-imap + net-pop newrelic-infinite_tracing newrelic_rpm nokogiri (~> 1.16.0) @@ -579,7 +594,7 @@ DEPENDENCIES omniauth-tara! paper_trail (~> 14.0) pdfkit - pg (= 1.5.7) + pg (= 1.5.6) pg_query (>= 0.9.0) pghero pry (= 0.14.2) @@ -595,7 +610,7 @@ DEPENDENCIES selenium-webdriver sidekiq (~> 7.0) simplecov (= 0.17.1) - simpleidn (= 0.2.3) + simpleidn (= 0.2.2) spy strong_migrations truemail (~> 3.0) @@ -606,4 +621,4 @@ DEPENDENCIES wkhtmltopdf-binary (~> 0.12.6.1) BUNDLED WITH - 2.5.4 + 2.5.17 diff --git a/app/jobs/retry_greylisted_emails_job.rb b/app/jobs/retry_greylisted_emails_job.rb new file mode 100644 index 0000000000..ed2f715dfd --- /dev/null +++ b/app/jobs/retry_greylisted_emails_job.rb @@ -0,0 +1,82 @@ +class RetryGreylistedEmailsJob < ApplicationJob + queue_as :default + + MAX_RETRY_ATTEMPTS = 10 + INITIAL_RETRY_DELAY = 5.minutes + + def perform + unique_greylisted_contacts.each do |contact| + retry_count = 0 + success = false + + while retry_count < MAX_RETRY_ATTEMPTS && !success + success = retry_email_validation(contact, retry_count) + retry_count += 1 + sleep(calculate_delay(retry_count)) unless success + end + + if success + clear_and_save_successful_validation(contact) + else + mark_email_as_invalid(contact) + end + end + end + + private + + def unique_greylisted_contacts + ValidationEvent.greylisted_smtp_errors + .select(:validation_eventable_id, :validation_eventable_type) + .distinct + .map(&:validation_eventable) + end + + def retry_email_validation(contact, retry_count) + result = Truemail.validate(contact.email, with: :smtp).result + + contact.validation_events.create( + event_type: :email_validation, + success: result.success?, + event_data: { + check_level: 'smtp', + error: result.error, + retry_count: retry_count + } + ) + + result.success? + end + + def calculate_delay(retry_count) + INITIAL_RETRY_DELAY * (2 ** (retry_count - 1)) + end + + def clear_and_save_successful_validation(contact) + contact.validation_events.destroy_all + contact.validation_events.create( + event_type: :email_validation, + success: true, + event_data: { check_level: 'smtp' } + ) + end + + def mark_email_as_invalid(contact) + contact.validation_events.destroy_all + contact.validation_events.create( + event_type: :email_validation, + success: false, + event_data: { + check_level: 'smtp', + error: 'Max retry count exceeded' + } + ) + end + + # Prevents sleep in test environment + def sleep(seconds) + return if Rails.env.test? + + super + end +end \ No newline at end of file diff --git a/app/models/validation_event.rb b/app/models/validation_event.rb index 80327f7c2a..f5dcfe93f1 100644 --- a/app/models/validation_event.rb +++ b/app/models/validation_event.rb @@ -10,6 +10,8 @@ class ValidationEvent < ApplicationRecord VALID_CHECK_LEVELS = %w[regex mx smtp].freeze VALID_EVENTS_COUNT_THRESHOLD = 5 MX_CHECK = 3 + MAX_RETRY_COUNT = 10 + INITIAL_RETRY_DELAY = 5.minutes INVALID_EVENTS_COUNT_BY_LEVEL = { regex: 1, @@ -34,6 +36,13 @@ class ValidationEvent < ApplicationRecord scope :mx, -> { where('event_data @> ?', { 'check_level': 'mx' }.to_json) } scope :smtp, -> { where('event_data @> ?', { 'check_level': 'smtp' }.to_json) } scope :by_object, ->(object) { where(validation_eventable: object) } + scope :greylisted_smtp_errors, -> { + where(success: false) + .where("event_data->>'check_level' = ?", 'smtp') + .where("event_data->'smtp_debug'->0->'response'->'errors'->>'rcptto' LIKE ?", '%Greylisted for%') + .where('created_at > ?', 24.hours.ago) + } + def self.validated_ids_by(klass) old_records @@ -50,6 +59,10 @@ def successful? success end + def greylisted? + event_type == 'smtp' && event_data['error'].include?('Greylisted for') + end + def event_type @event_type ||= ValidationEvent::EventType.new(self[:event_type]) end diff --git a/test/jobs/retry_greylisted_emails_job_test.rb b/test/jobs/retry_greylisted_emails_job_test.rb new file mode 100644 index 0000000000..cc868f08fd --- /dev/null +++ b/test/jobs/retry_greylisted_emails_job_test.rb @@ -0,0 +1,141 @@ +require 'test_helper' + +class RetryGreylistedEmailsJobTest < ActiveJob::TestCase + def setup + @contact = contacts(:john) + @contact.validation_events.destroy_all + @greylisted_event = ValidationEvent.create( + validation_eventable: @contact, + event_type: :email_validation, + success: false, + event_data: { + email: @contact.email, + domain: @contact.email.split('@').last, + mail_servers: ["194.19.134.80", "185.138.56.208"], + errors: { smtp: "smtp error" }, + smtp_debug: [{ + host: "194.19.134.80", + email: @contact.email, + attempts: nil, + response: { + port_opened: true, + connection: true, + helo: true, + mailfrom: { status: "250", string: "250 2.1.0 Ok\n" }, + rcptto: false, + errors: { rcptto: "451 4.7.1 <#{@contact.email}>: Recipient address rejected: Greylisted for 1 minutes\n" } + }, + configuration: { + smtp_port: 25, + verifier_email: "no-reply@example.com", + verifier_domain: "example.com", + response_timeout: 1, + connection_timeout: 1 + } + }], + check_level: "smtp" + } + ) + @contact.reload + end + + test 'performs retry for greylisted emails and succeeds' do + mock_truemail_success do + assert_no_difference 'ValidationEvent.count' do + RetryGreylistedEmailsJob.perform_now + end + end + + @contact.reload + assert @contact.validation_events.last.success? + assert_nil @contact.validation_events.last.event_data.dig('smtp_debug', 0, 'response', 'errors', 'rcptto') + end + + def test_marks_email_as_invalid_after_max_retries + mock_truemail_failure(RetryGreylistedEmailsJob::MAX_RETRY_ATTEMPTS) do + assert_no_difference 'ValidationEvent.count' do + RetryGreylistedEmailsJob.perform_now + end + end + + @contact.reload + last_event = @contact.validation_events.last + refute last_event.success? + + error_message = last_event.event_data['errors']&.dig('smtp') || + last_event.event_data['error'] || + last_event.event_data.dig('smtp_debug', 0, 'response', 'errors', 'rcptto') + + assert_equal 'Max retry count exceeded', error_message + end + + test 'retries until email is not greylisted' do + mock_truemail_success_after_failures(3) do + assert_no_difference 'ValidationEvent.count' do + RetryGreylistedEmailsJob.perform_now + end + end + + @contact.reload + assert @contact.validation_events.last.success? + assert_nil @contact.validation_events.last.event_data.dig('smtp_debug', 0, 'response', 'errors', 'rcptto') + end + + private + + def mock_truemail_success + result = mock_truemail_result(true) + Truemail.stub :validate, result do + yield if block_given? + end +end + +def mock_truemail_failure(times = 1) + results = Array.new(times) { mock_truemail_result(false) } + Truemail.stub :validate, ->(*args) { results.shift || mock_truemail_result(false) } do + yield if block_given? + end +end + +def mock_truemail_success_after_failures(failure_count) + results = Array.new(failure_count) { mock_truemail_result(false) } + results << mock_truemail_result(true) + Truemail.stub :validate, ->(*args) { results.shift || mock_truemail_result(true) } do + yield if block_given? + end +end + +def mock_truemail_result(success) + OpenStruct.new( + result: OpenStruct.new( + success?: success, + email: @contact.email, + domain: @contact.email.split('@').last, + mail_servers: ["194.19.134.80", "185.138.56.208"], + errors: success ? {} : { smtp: "smtp error" }, + smtp_debug: [ + OpenStruct.new( + host: "194.19.134.80", + email: @contact.email, + attempts: nil, + response: OpenStruct.new( + port_opened: true, + connection: true, + helo: true, + mailfrom: OpenStruct.new(status: "250", string: "250 2.1.0 Ok\n"), + rcptto: success, + errors: success ? {} : { rcptto: "451 4.7.1 <#{@contact.email}>: Recipient address rejected: Greylisted for 1 minutes\n" } + ), + configuration: OpenStruct.new( + smtp_port: 25, + verifier_email: "no-reply@example.com", + verifier_domain: "example.com", + response_timeout: 1, + connection_timeout: 1 + ) + ) + ] + ) + ) +end +end \ No newline at end of file