Fix ptr audit issues: #18 (#33)

* Fix #current_host_address behavior, implement reverse trace

.reek.yml

@@ -29,7 +29,7 @@ detectors:
       - Truemail::Validator#select_validation_type
       - Truemail::Validate::Mx#null_mx?
       - Truemail::Validate::Mx#a_record
-      - Truemail::Audit::Ptr#current_host_address
+      - Truemail::Audit::Base#verifier_domain
 
   ControlParameter:
     exclude:

Gemfile.lock

@@ -1,7 +1,7 @@
 PATH
   remote: .
   specs:
-    truemail (0.1.7)
+    truemail (0.1.8)
 
 GEM
   remote: https://rubygems.org/

lib/truemail/audit/base.rb

@@ -8,6 +8,10 @@ class Base < Truemail::Worker
       def add_warning(message)
         result.warnings[self.class.name.split('::').last.downcase.to_sym] = message
       end
+
+      def verifier_domain
+        Truemail.configuration.verifier_domain
+      end
     end
   end
 end

lib/truemail/audit/ptr.rb

@@ -6,23 +6,32 @@ class Ptr < Truemail::Audit::Base
       require 'ipaddr'
       require 'resolv'
 
-      NOT_FOUND = 'ptr record for current host address was not found'
-      NOT_REFERENCES = 'ptr record does not reference to current verifier domain'
+      GET_MY_IP_URL = 'https://api.ipify.org'
+      IPIFY_ERROR = 'impossible to detect current host address via third party service'
+      PTR_NOT_FOUND = 'ptr record for current host address was not found'
+      PTR_NOT_REFER = 'ptr record does not reference to current verifier domain'
+      VERIFIER_DOMAIN_NOT_REFER = ''
 
       def run
-        return if ptr_records.empty? && add_warning(Truemail::Audit::Ptr::NOT_FOUND)
-        return if ptr_references_to_verifier_domain?
-        add_warning(Truemail::Audit::Ptr::NOT_REFERENCES)
+        return if !current_host_address && add_warning(Truemail::Audit::Ptr::IPIFY_ERROR)
+        return if ptr_records.empty? && add_warning(Truemail::Audit::Ptr::PTR_NOT_FOUND)
+        return if ptr_not_refer_to_verifier_domain? && add_warning(Truemail::Audit::Ptr::PTR_NOT_REFER)
+        return if verifier_domain_refer_to_current_host_address?
+        add_warning(Truemail::Audit::Ptr::VERIFIER_DOMAIN_NOT_REFER)
       end
 
       private
 
+      def detect_ip_via_ipify
+        Net::HTTP.get(URI(Truemail::Audit::Ptr::GET_MY_IP_URL))
+      end
+
       def current_host_address
-        Resolv.getaddress(Socket.gethostname)
+        @current_host_address ||= Truemail::Wrapper.call { IPAddr.new(detect_ip_via_ipify) }
       end
 
       def current_host_reverse_lookup
-        IPAddr.new(current_host_address).reverse
+        current_host_address.reverse
       end
 
       def ptr_records
@@ -33,8 +42,16 @@ def ptr_records
         end || []
       end
 
-      def ptr_references_to_verifier_domain?
-        ptr_records.include?(Truemail.configuration.verifier_domain)
+      def ptr_not_refer_to_verifier_domain?
+        !ptr_records.include?(verifier_domain)
+      end
+
+      def a_record
+        Truemail::Wrapper.call { Resolv::DNS.new.getaddress(verifier_domain).to_s }
+      end
+
+      def verifier_domain_refer_to_current_host_address?
+        a_record == current_host_address.to_s
       end
     end
   end

lib/truemail/version.rb

@@ -1,5 +1,5 @@
 # frozen_string_literal: true
 
 module Truemail
-  VERSION = '0.1.7'
+  VERSION = '0.1.8'
 end

lib/truemail/wrapper.rb

@@ -14,7 +14,7 @@ def initialize
 
     def call(&block)
       Timeout.timeout(Truemail.configuration.connection_timeout, &block)
-    rescue Resolv::ResolvError
+    rescue Resolv::ResolvError, IPAddr::InvalidAddressError
       false
     rescue Timeout::Error
       retry unless (self.attempts -= 1).zero?

spec/support/matchers/match_to_ip_address.rb

@@ -0,0 +1,8 @@
+# frozen_string_literal: true
+
+# rubocop:disable Style/RescueModifier
+RSpec::Matchers.define(:match_to_ip_address) do
+  match do |ip_address|
+    IPAddr.new(ip_address) rescue false
+  end
+end

spec/truemail/audit/ptr_spec.rb

@@ -7,8 +7,11 @@
   before { Truemail.configure { |config| config.verifier_email = email } }
 
   describe 'defined constants' do
-    specify { expect(described_class).to be_const_defined(:NOT_FOUND) }
-    specify { expect(described_class).to be_const_defined(:NOT_REFERENCES) }
+    specify { expect(described_class).to be_const_defined(:GET_MY_IP_URL) }
+    specify { expect(described_class).to be_const_defined(:IPIFY_ERROR) }
+    specify { expect(described_class).to be_const_defined(:PTR_NOT_FOUND) }
+    specify { expect(described_class).to be_const_defined(:PTR_NOT_REFER) }
+    specify { expect(described_class).to be_const_defined(:VERIFIER_DOMAIN_NOT_REFER) }
   end
 
   describe '.check' do
@@ -28,38 +31,119 @@
 
     let(:ptr_auditor_instance) { described_class.new(result_instance) }
     let(:host_name) { Truemail.configuration.verifier_domain }
-    let(:other_host_name) { 'localhost' }
+    let(:host_address) { FFaker::Internet.ip_v4_address }
+    let(:other_host_address) { '127.0.0.1' }
 
-    before do
-      allow(Socket).to receive(:gethostname).and_return(other_host_name)
-      allow(Resolv).to receive(:getaddress).and_return(FFaker::Internet.ip_v4_address)
-    end
+    describe 'Success' do
+      context 'when ptr record exists, refereces to verifier domain and has reverse trace' do
+        before do
+          allow(ptr_auditor_instance).to receive(:detect_ip_via_ipify).and_return(host_address)
+          allow(ptr_auditor_instance).to receive(:ptr_records).and_return([host_name])
+          allow(ptr_auditor_instance).to receive(:a_record).and_return(host_address)
+        end
 
-    context 'when ptr record exists and refereces to verifier domain' do
-      it 'not changes warnings' do
-        expect(Resolv::DNS).to receive_message_chain(:new, :getresources, :map).and_return([host_name])
-        expect { ptr_auditor }.to not_change(result_instance, :warnings)
+        it 'not changes warnings' do
+          expect { ptr_auditor }.to not_change(result_instance, :warnings)
+        end
       end
     end
 
-    context 'when ptr record for current host address was not found' do
-      it 'addes not found warning' do
-        expect(Resolv::DNS).to receive_message_chain(:new, :getresources, :map).and_return([])
-        expect { ptr_auditor }.to change(result_instance, :warnings).from({}).to({ ptr: Truemail::Audit::Ptr::NOT_FOUND })
+    describe 'Fails' do
+      context 'when determining current host address' do
+        let(:expectation) do
+          expect { ptr_auditor }.to change(result_instance, :warnings).from({}).to({ ptr: Truemail::Audit::Ptr::IPIFY_ERROR })
+        end
+
+        context 'if error with third party service' do
+          it 'addes ipify warning' do
+            expect(ptr_auditor_instance).to receive(:current_host_address).and_return(false)
+            expectation
+          end
+        end
+
+        context 'if network error' do
+          it 'addes ipify warning' do
+            expect(ptr_auditor_instance).to receive(:detect_ip_via_ipify).and_raise(IPAddr::InvalidAddressError)
+            expectation
+          end
+        end
       end
-    end
 
-    context 'when ptr record checking crashes' do
-      it 'addes not found warning' do
-        expect(Resolv::DNS).to receive_message_chain(:new, :getresources, :map).and_raise(Resolv::ResolvError)
-        expect { ptr_auditor }.to change(result_instance, :warnings).from({}).to({ ptr: Truemail::Audit::Ptr::NOT_FOUND })
+      context 'when determining ptr records' do
+        let(:expectation) do
+          expect { ptr_auditor }.to change(result_instance, :warnings).from({}).to({ ptr: Truemail::Audit::Ptr::PTR_NOT_FOUND })
+        end
+
+        context 'if ptr record for current host address was not found' do
+          it 'addes not found warning' do
+            expect(ptr_auditor_instance).to receive(:current_host_address).and_return(true)
+            expect(ptr_auditor_instance).to receive(:ptr_records).and_return([])
+            expectation
+          end
+        end
+
+        context 'if ptr record checking crashes' do
+          it 'addes not found warning' do
+            expect(ptr_auditor_instance).to receive(:current_host_address).and_return(true)
+            expect(ptr_auditor_instance).to receive(:current_host_reverse_lookup).and_raise(Resolv::ResolvError)
+            expectation
+          end
+        end
+      end
+
+      context 'when determining ptr referer' do
+        context 'if ptr records do not refer to verifier domain' do
+          it 'addes not references warning' do
+            expect(ptr_auditor_instance).to receive(:current_host_address).and_return(true)
+            expect(ptr_auditor_instance).to receive(:ptr_records).and_return([host_name])
+            expect(ptr_auditor_instance).to receive(:ptr_not_refer_to_verifier_domain?).and_return(true)
+            expect { ptr_auditor }.to change(result_instance, :warnings).from({}).to({ ptr: Truemail::Audit::Ptr::PTR_NOT_REFER })
+          end
+        end
+      end
+
+      context 'when determining verifier domain referer' do
+        let(:expectation) do
+          expect { ptr_auditor }
+            .to change(result_instance, :warnings)
+            .from({}).to({ ptr: Truemail::Audit::Ptr::VERIFIER_DOMAIN_NOT_REFER })
+        end
+
+        before do
+          allow(ptr_auditor_instance).to receive(:detect_ip_via_ipify).and_return(host_address)
+          allow(ptr_auditor_instance).to receive(:ptr_records).and_return([host_name])
+          allow(ptr_auditor_instance).to receive(:ptr_not_refer_to_verifier_domain?).and_return(false)
+        end
+
+        context 'if verifier domain a record does not refer to ptr record' do
+          it 'addes not reverse trace warning' do
+            expect(ptr_auditor_instance).to receive(:a_record).and_return(other_host_address)
+            expectation
+          end
+        end
+
+        context 'if network error' do
+          it 'addes not reverse trace warning' do
+            expect(Resolv::DNS).to receive_message_chain(:new, :getaddress, :to_s).and_raise(Resolv::ResolvError)
+            expectation
+          end
+        end
       end
     end
+  end
+
+  describe 'ipify third party service integration tests' do
+    context 'when calles #detect_ip_via_ipify' do
+      subject(:ipify_response) { ptr_auditor_instance.send(:detect_ip_via_ipify) }
+
+      let(:ptr_auditor_instance) { described_class.new(result_instance) }
+
+      it 'returns current host ip address as a string' do
+        expect(ipify_response).to be_an_instance_of(String)
+      end
 
-    context 'when ptr record does not reference to current verifier domain' do
-      it 'addes not references warning' do
-        expect(Resolv::DNS).to receive_message_chain(:new, :getresources, :map).and_return([other_host_name])
-        expect { ptr_auditor }.to change(result_instance, :warnings).from({}).to({ ptr: Truemail::Audit::Ptr::NOT_REFERENCES })
+      it 'returned string matches to ip address pattern' do
+        expect(ipify_response).to match_to_ip_address
       end
     end
   end

spec/truemail/wrapper_spec.rb

@@ -35,6 +35,13 @@
         end
       end
 
+      context 'with IPAddr::InvalidAddressError exception' do
+        specify do
+          allow(mx_instance).to receive(method).and_raise(IPAddr::InvalidAddressError)
+          expect(resolver_execution_wrapper).to be(false)
+        end
+      end
+
       context 'with Timeout::Error exception' do
         specify do
           allow(mx_instance).to receive(method).and_raise(Timeout::Error)