Add uniqueness to block email domains (#5692)

4 files changed, 35 insertions, 6 deletions

diff --git a/app/models/email_domain_block.rb b/app/models/email_domain_block.rb
index 51410605b..2c348197c 100644
--- a/app/models/email_domain_block.rb
+++ b/app/models/email_domain_block.rb
@@ -4,14 +4,33 @@
 # Table name: email_domain_blocks
 #
 #  id         :bigint           not null, primary key
-#  domain     :string           not null
+#  domain     :string           default(""), not null
 #  created_at :datetime         not null
 #  updated_at :datetime         not null
 #
 
 class EmailDomainBlock < ApplicationRecord
+  before_validation :normalize_domain
+
+  validates :domain, presence: true, uniqueness: true
+
   def self.block?(email)
-    domain = email.gsub(/.+@([^.]+)/, '\1')
+    _, domain = email.split('@', 2)
+
+    return true if domain.nil?
+
+    begin
+      domain = TagManager.instance.normalize_domain(domain)
+    rescue Addressable::URI::InvalidURIError
+      return true
+    end
+
     where(domain: domain).exists?
   end
+
+  private
+
+  def normalize_domain
+    self.domain = TagManager.instance.normalize_domain(domain)
+  end
 end
diff --git a/db/migrate/20171114080328_add_index_domain_to_email_domain_blocks.rb b/db/migrate/20171114080328_add_index_domain_to_email_domain_blocks.rb
new file mode 100644
index 000000000..84a341510
--- /dev/null
+++ b/db/migrate/20171114080328_add_index_domain_to_email_domain_blocks.rb
@@ -0,0 +1,8 @@
+class AddIndexDomainToEmailDomainBlocks < ActiveRecord::Migration[5.1]
+  disable_ddl_transaction!
+
+  def change
+    add_index :email_domain_blocks, :domain, algorithm: :concurrently, unique: true
+    change_column_default :email_domain_blocks, :domain, from: nil, to: ''
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index f16b24fd6..bf319ce55 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20171109012327) do
+ActiveRecord::Schema.define(version: 20171114080328) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -126,9 +126,10 @@ ActiveRecord::Schema.define(version: 20171109012327) do
   end
 
   create_table "email_domain_blocks", force: :cascade do |t|
-    t.string "domain", null: false
+    t.string "domain", default: "", null: false
     t.datetime "created_at", null: false
     t.datetime "updated_at", null: false
+    t.index ["domain"], name: "index_email_domain_blocks_on_domain", unique: true
   end
 
   create_table "favourites", force: :cascade do |t|
diff --git a/spec/models/email_domain_block_spec.rb b/spec/models/email_domain_block_spec.rb
index 5f5d189d9..efd2853a9 100644
--- a/spec/models/email_domain_block_spec.rb
+++ b/spec/models/email_domain_block_spec.rb
@@ -13,9 +13,10 @@ RSpec.describe EmailDomainBlock, type: :model do
       Fabricate(:email_domain_block, domain: 'example.com')
       expect(EmailDomainBlock.block?('nyarn@example.com')).to eq true
     end
+
     it 'returns true if the domain is not registed' do
-      Fabricate(:email_domain_block, domain: 'domain')
-      expect(EmailDomainBlock.block?('example')).to eq false
+      Fabricate(:email_domain_block, domain: 'example.com')
+      expect(EmailDomainBlock.block?('nyarn@example.net')).to eq false
     end
   end
 end