Change e-mail domain blocks to block IPs dynamically (#17635)

* Change e-mail domain blocks to block IPs dynamically

* Update app/workers/scheduler/email_domain_block_refresh_scheduler.rb

Co-authored-by: Yamagishi Kazutoshi <ykzts@desire.sh>

* Update app/workers/scheduler/email_domain_block_refresh_scheduler.rb

Co-authored-by: Yamagishi Kazutoshi <ykzts@desire.sh>

Co-authored-by: Yamagishi Kazutoshi <ykzts@desire.sh>

4 files changed, 73 insertions, 54 deletions

diff --git a/spec/controllers/admin/email_domain_blocks_controller_spec.rb b/spec/controllers/admin/email_domain_blocks_controller_spec.rb
index 52db56f4e..cf194579d 100644
--- a/spec/controllers/admin/email_domain_blocks_controller_spec.rb
+++ b/spec/controllers/admin/email_domain_blocks_controller_spec.rb
@@ -17,43 +17,43 @@ RSpec.describe Admin::EmailDomainBlocksController, type: :controller do
       EmailDomainBlock.paginates_per default_per_page
     end
 
-    it 'renders email blacks' do
+    it 'returns http success' do
       2.times { Fabricate(:email_domain_block) }
-
       get :index, params: { page: 2 }
-
-      assigned = assigns(:email_domain_blocks)
-      expect(assigned.count).to eq 1
-      expect(assigned.klass).to be EmailDomainBlock
       expect(response).to have_http_status(200)
     end
   end
 
   describe 'GET #new' do
-    it 'assigns a new email black' do
+    it 'returns http success' do
       get :new
-
-      expect(assigns(:email_domain_block)).to be_instance_of(EmailDomainBlock)
       expect(response).to have_http_status(200)
     end
   end
 
   describe 'POST #create' do
-    it 'blocks the domain when succeeded to save' do
-      post :create, params: { email_domain_block: { domain: 'example.com' } }
-
-      expect(flash[:notice]).to eq I18n.t('admin.email_domain_blocks.created_msg')
-      expect(response).to redirect_to(admin_email_domain_blocks_path)
+    context 'when resolve button is pressed' do
+      before do
+        post :create, params: { email_domain_block: { domain: 'example.com' } }
+      end
+
+      it 'renders new template' do
+        expect(response).to render_template(:new)
+      end
     end
-  end
 
-  describe 'DELETE #destroy' do
-    it 'unblocks the domain' do
-      email_domain_block = Fabricate(:email_domain_block)
-      delete :destroy, params: { id: email_domain_block.id }
+    context 'when save button is pressed' do
+      before do
+        post :create, params: { email_domain_block: { domain: 'example.com' }, save: '' }
+      end
+
+      it 'blocks the domain' do
+        expect(EmailDomainBlock.find_by(domain: 'example.com')).to_not be_nil
+      end
 
-      expect(flash[:notice]).to eq I18n.t('admin.email_domain_blocks.destroyed_msg')
-      expect(response).to redirect_to(admin_email_domain_blocks_path)
+      it 'redirects to e-mail domain blocks' do
+        expect(response).to redirect_to(admin_email_domain_blocks_path)
+      end
     end
   end
 end
diff --git a/spec/models/email_domain_block_spec.rb b/spec/models/email_domain_block_spec.rb
index efd2853a9..567a32c32 100644
--- a/spec/models/email_domain_block_spec.rb
+++ b/spec/models/email_domain_block_spec.rb
@@ -9,14 +9,29 @@ RSpec.describe EmailDomainBlock, type: :model do
   end
 
   describe 'block?' do
-    it 'returns true if the domain is registed' do
-      Fabricate(:email_domain_block, domain: 'example.com')
-      expect(EmailDomainBlock.block?('nyarn@example.com')).to eq true
+    let(:input) { nil }
+
+    context 'given an e-mail address' do
+      let(:input) { 'nyarn@example.com' }
+
+      it 'returns true if the domain is blocked' do
+        Fabricate(:email_domain_block, domain: 'example.com')
+        expect(EmailDomainBlock.block?(input)).to be true
+      end
+
+      it 'returns false if the domain is not blocked' do
+        Fabricate(:email_domain_block, domain: 'other-example.com')
+        expect(EmailDomainBlock.block?(input)).to be false
+      end
     end
 
-    it 'returns true if the domain is not registed' do
-      Fabricate(:email_domain_block, domain: 'example.com')
-      expect(EmailDomainBlock.block?('nyarn@example.net')).to eq false
+    context 'given an array of domains' do
+      let(:input) { %w(foo.com mail.foo.com) }
+
+      it 'returns true if the domain is blocked' do
+        Fabricate(:email_domain_block, domain: 'mail.foo.com')
+        expect(EmailDomainBlock.block?(input)).to be true
+      end
     end
   end
 end
diff --git a/spec/validators/blacklisted_email_validator_spec.rb b/spec/validators/blacklisted_email_validator_spec.rb
index f7d5e01bc..351de0707 100644
--- a/spec/validators/blacklisted_email_validator_spec.rb
+++ b/spec/validators/blacklisted_email_validator_spec.rb
@@ -4,7 +4,7 @@ require 'rails_helper'
 
 RSpec.describe BlacklistedEmailValidator, type: :validator do
   describe '#validate' do
-    let(:user)   { double(email: 'info@mail.com', errors: errors) }
+    let(:user)   { double(email: 'info@mail.com', sign_up_ip: '1.2.3.4', errors: errors) }
     let(:errors) { double(add: nil) }
 
     before do
diff --git a/spec/validators/email_mx_validator_spec.rb b/spec/validators/email_mx_validator_spec.rb
index 550e91996..af0eb98f5 100644
--- a/spec/validators/email_mx_validator_spec.rb
+++ b/spec/validators/email_mx_validator_spec.rb
@@ -4,24 +4,28 @@ require 'rails_helper'
 
 describe EmailMxValidator do
   describe '#validate' do
-    let(:user) { double(email: 'foo@example.com', errors: double(add: nil)) }
-
-    it 'does not add errors if there are no DNS records for an e-mail domain that is explicitly allowed' do
-      old_whitelist = Rails.configuration.x.email_domains_whitelist
-      Rails.configuration.x.email_domains_whitelist = 'example.com'
-
-      resolver = double
-
-      allow(resolver).to receive(:getresources).with('example.com', Resolv::DNS::Resource::IN::MX).and_return([])
-      allow(resolver).to receive(:getresources).with('example.com', Resolv::DNS::Resource::IN::A).and_return([])
-      allow(resolver).to receive(:getresources).with('example.com', Resolv::DNS::Resource::IN::AAAA).and_return([])
-      allow(resolver).to receive(:timeouts=).and_return(nil)
-      allow(Resolv::DNS).to receive(:open).and_yield(resolver)
-
-      subject.validate(user)
-      expect(user.errors).to_not have_received(:add)
-
-      Rails.configuration.x.email_domains_whitelist = old_whitelist
+    let(:user) { double(email: 'foo@example.com', sign_up_ip: '1.2.3.4', errors: double(add: nil)) }
+
+    context 'for an e-mail domain that is explicitly allowed' do
+      around do |block|
+        tmp = Rails.configuration.x.email_domains_whitelist
+        Rails.configuration.x.email_domains_whitelist = 'example.com'
+        block.call
+        Rails.configuration.x.email_domains_whitelist = tmp
+      end
+
+      it 'does not add errors if there are no DNS records' do
+        resolver = double
+
+        allow(resolver).to receive(:getresources).with('example.com', Resolv::DNS::Resource::IN::MX).and_return([])
+        allow(resolver).to receive(:getresources).with('example.com', Resolv::DNS::Resource::IN::A).and_return([])
+        allow(resolver).to receive(:getresources).with('example.com', Resolv::DNS::Resource::IN::AAAA).and_return([])
+        allow(resolver).to receive(:timeouts=).and_return(nil)
+        allow(Resolv::DNS).to receive(:open).and_yield(resolver)
+
+        subject.validate(user)
+        expect(user.errors).to_not have_received(:add)
+      end
     end
 
     it 'adds an error if there are no DNS records for the e-mail domain' do
@@ -37,7 +41,7 @@ describe EmailMxValidator do
       expect(user.errors).to have_received(:add)
     end
 
-    it 'adds an error if a MX record exists but does not lead to an IP' do
+    it 'adds an error if a MX record does not lead to an IP' do
       resolver = double
 
       allow(resolver).to receive(:getresources).with('example.com', Resolv::DNS::Resource::IN::MX).and_return([double(exchange: 'mail.example.com')])
@@ -53,7 +57,7 @@ describe EmailMxValidator do
     end
 
     it 'adds an error if the A record is blacklisted' do
-      EmailDomainBlock.create!(domain: '1.2.3.4')
+      EmailDomainBlock.create!(domain: 'alternate-example.com', ips: ['1.2.3.4'])
       resolver = double
 
       allow(resolver).to receive(:getresources).with('example.com', Resolv::DNS::Resource::IN::MX).and_return([])
@@ -67,7 +71,7 @@ describe EmailMxValidator do
     end
 
     it 'adds an error if the AAAA record is blacklisted' do
-      EmailDomainBlock.create!(domain: 'fd00::1')
+      EmailDomainBlock.create!(domain: 'alternate-example.com', ips: ['fd00::1'])
       resolver = double
 
       allow(resolver).to receive(:getresources).with('example.com', Resolv::DNS::Resource::IN::MX).and_return([])
@@ -80,8 +84,8 @@ describe EmailMxValidator do
       expect(user.errors).to have_received(:add)
     end
 
-    it 'adds an error if the MX record is blacklisted' do
-      EmailDomainBlock.create!(domain: '2.3.4.5')
+    it 'adds an error if the A record of the MX record is blacklisted' do
+      EmailDomainBlock.create!(domain: 'mail.other-domain.com', ips: ['2.3.4.5'])
       resolver = double
 
       allow(resolver).to receive(:getresources).with('example.com', Resolv::DNS::Resource::IN::MX).and_return([double(exchange: 'mail.example.com')])
@@ -96,8 +100,8 @@ describe EmailMxValidator do
       expect(user.errors).to have_received(:add)
     end
 
-    it 'adds an error if the MX IPv6 record is blacklisted' do
-      EmailDomainBlock.create!(domain: 'fd00::2')
+    it 'adds an error if the AAAA record of the MX record is blacklisted' do
+      EmailDomainBlock.create!(domain: 'mail.other-domain.com', ips: ['fd00::2'])
       resolver = double
 
       allow(resolver).to receive(:getresources).with('example.com', Resolv::DNS::Resource::IN::MX).and_return([double(exchange: 'mail.example.com')])
@@ -112,7 +116,7 @@ describe EmailMxValidator do
       expect(user.errors).to have_received(:add)
     end
 
-    it 'adds an error if the MX hostname is blacklisted' do
+    it 'adds an error if the MX record is blacklisted' do
       EmailDomainBlock.create!(domain: 'mail.example.com')
       resolver = double