FIX: `EmailValidator` needs to validate format of email.

FIX: EmailValidator needs to validate format of email.

diff --git a/app/models/invite.rb b/app/models/invite.rb
index 2618b37..5fde9a4 100644
--- a/app/models/invite.rb
+++ b/app/models/invite.rb
@@ -18,7 +18,7 @@ class Invite < ActiveRecord::Base
   has_many :topic_invites
   has_many :topics, through: :topic_invites, source: :topic
   validates_presence_of :invited_by_id
-  validates :email, email: true, format: { with: EmailValidator.email_regex }, allow_blank: true
+  validates :email, email: true, allow_blank: true
 
   before_create do
     self.invite_key ||= SecureRandom.hex
diff --git a/app/models/user_email.rb b/app/models/user_email.rb
index 7ce64cb..a3e76f2 100644
--- a/app/models/user_email.rb
+++ b/app/models/user_email.rb
@@ -11,8 +11,7 @@ class UserEmail < ActiveRecord::Base
   before_validation :strip_downcase_email
 
   validates :email, presence: true
-  validates :email, email: true, format: { with: EmailValidator.email_regex },
-                    if: :validate_email?
+  validates :email, email: true, if: :validate_email?
 
   validates :primary, uniqueness: { scope: [:user_id] }, if: [:user_id, :primary]
   validate :user_id_not_changed, if: :primary
diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml
index 7b564c4..da98bd5 100644
--- a/config/locales/server.en.yml
+++ b/config/locales/server.en.yml
@@ -2451,6 +2451,7 @@ en:
       must_not_contain_two_special_chars_in_seq: "must not contain a sequence of 2 or more special chars (.-_)"
       must_not_end_with_confusing_suffix: "must not end with a confusing suffix like .json or .png etc."
     email:
+      invalid: "is invalid."
       not_allowed: "is not allowed from that email provider. Please use another email address."
       blocked: "is not allowed."
       revoked: "Won't be sending emails to '%{email}' until %{date}."
diff --git a/lib/validators/email_validator.rb b/lib/validators/email_validator.rb
index 6436161..7090f25 100644
--- a/lib/validators/email_validator.rb
+++ b/lib/validators/email_validator.rb
@@ -3,6 +3,10 @@
 class EmailValidator < ActiveModel::EachValidator
 
   def validate_each(record, attribute, value)
+    unless value =~ EmailValidator.email_regex
+      record.errors.add(attribute, I18n.t(:'user.email.invalid'))
+    end
+
     unless EmailValidator.allowed?(value)
       record.errors.add(attribute, I18n.t(:'user.email.not_allowed'))
     end
diff --git a/spec/models/invite_spec.rb b/spec/models/invite_spec.rb
index fab7da2..f66a8f8 100644
--- a/spec/models/invite_spec.rb
+++ b/spec/models/invite_spec.rb
@@ -29,9 +29,9 @@ describe Invite do
     fab!(:coding_horror) { Fabricate(:coding_horror) }
 
     it "should not allow an invite with unformatted email address" do
-      expect {
-        Fabricate(:invite, email: "John Doe <john.doe@example.com>")
-      }.to raise_error(ActiveRecord::RecordInvalid)
+      invite = Fabricate.build(:invite, email: "John Doe <john.doe@example.com>")
+      expect(invite.valid?).to eq(false)
+      expect(invite.errors.details[:email].first[:error]).to eq(I18n.t("user.email.invalid"))
     end
 
     it "should not allow an invite with blacklisted email" do
@@ -44,6 +44,11 @@ describe Invite do
       expect(invite).to be_valid
     end
 
+    it "should not allow an invalid email address" do
+      invite = Fabricate.build(:invite, email: 'asjdso')
+      expect(invite.valid?).to eq(false)
+      expect(invite.errors.details[:email].first[:error]).to eq(I18n.t("user.email.invalid"))
+    end
   end
 
   context '#create' do
diff --git a/spec/models/user_email_spec.rb b/spec/models/user_email_spec.rb
index ee48170..ebd08de 100644
--- a/spec/models/user_email_spec.rb
+++ b/spec/models/user_email_spec.rb
@@ -3,32 +3,36 @@
 require 'rails_helper'
 
 describe UserEmail do
+  fab!(:user) { Fabricate(:user) }
+
   context "validation" do
     it "allows only one primary email" do
-      user = Fabricate(:user)
       expect {
         Fabricate(:secondary_email, user: user, primary: true)
       }.to raise_error(ActiveRecord::RecordInvalid)
     end
 
     it "allows multiple secondary emails" do
-      user = Fabricate(:user)
       Fabricate(:secondary_email, user: user, primary: false)
       Fabricate(:secondary_email, user: user, primary: false)
       expect(user.user_emails.count).to eq 3
     end
+
+    it "does not allow an invalid email" do
+      user_email = Fabricate.build(:user_email, user: user, email: "asjdaiosd")
+      expect(user_email.valid?).to eq(false)
+      expect(user_email.errors.details[:email].first[:error]).to eq(I18n.t("user.email.invalid"))
+    end
   end
 
   context "indexes" do
     it "allows only one primary email" do
-      user = Fabricate(:user)
       expect {
         Fabricate.build(:secondary_email, user: user, primary: true).save(validate: false)
       }.to raise_error(ActiveRecord::RecordNotUnique)
     end
 
     it "allows multiple secondary emails" do
-      user = Fabricate(:user)
       Fabricate.build(:secondary_email, user: user, primary: false).save(validate: false)
       Fabricate.build(:secondary_email, user: user, primary: false).save(validate: false)
       expect(user.user_emails.count).to eq 3
diff --git a/spec/requests/users_email_controller_spec.rb b/spec/requests/users_email_controller_spec.rb
index f7c3501..257a431 100644
--- a/spec/requests/users_email_controller_spec.rb
+++ b/spec/requests/users_email_controller_spec.rb
@@ -282,7 +282,7 @@ describe UsersEmailController do
       it 'raises an error without an invalid email' do
         put "/u/#{user.username}/preferences/email.json", params: { email: "sam@not-email.com'" }
         expect(response.status).to eq(422)
-        expect(response.body).to include("email is invalid")
+        expect(response.body).to include("Email is invalid")
       end
 
       it "raises an error if you can't edit the user's email" do

GitHub sha: 062db10c

2 Likes