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

This commit is contained in:
Guo Xiang Tan 2020-06-03 10:13:25 +08:00
parent 1b5a505930
commit 062db10c52
No known key found for this signature in database
GPG Key ID: FBD110179AAC1F20
7 changed files with 24 additions and 11 deletions

View File

@ -18,7 +18,7 @@ class Invite < ActiveRecord::Base
has_many :topic_invites has_many :topic_invites
has_many :topics, through: :topic_invites, source: :topic has_many :topics, through: :topic_invites, source: :topic
validates_presence_of :invited_by_id 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 before_create do
self.invite_key ||= SecureRandom.hex self.invite_key ||= SecureRandom.hex

View File

@ -11,8 +11,7 @@ class UserEmail < ActiveRecord::Base
before_validation :strip_downcase_email before_validation :strip_downcase_email
validates :email, presence: true validates :email, presence: true
validates :email, email: true, format: { with: EmailValidator.email_regex }, validates :email, email: true, if: :validate_email?
if: :validate_email?
validates :primary, uniqueness: { scope: [:user_id] }, if: [:user_id, :primary] validates :primary, uniqueness: { scope: [:user_id] }, if: [:user_id, :primary]
validate :user_id_not_changed, if: :primary validate :user_id_not_changed, if: :primary

View File

@ -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_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." must_not_end_with_confusing_suffix: "must not end with a confusing suffix like .json or .png etc."
email: email:
invalid: "is invalid."
not_allowed: "is not allowed from that email provider. Please use another email address." not_allowed: "is not allowed from that email provider. Please use another email address."
blocked: "is not allowed." blocked: "is not allowed."
revoked: "Won't be sending emails to '%{email}' until %{date}." revoked: "Won't be sending emails to '%{email}' until %{date}."

View File

@ -3,6 +3,10 @@
class EmailValidator < ActiveModel::EachValidator class EmailValidator < ActiveModel::EachValidator
def validate_each(record, attribute, value) 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) unless EmailValidator.allowed?(value)
record.errors.add(attribute, I18n.t(:'user.email.not_allowed')) record.errors.add(attribute, I18n.t(:'user.email.not_allowed'))
end end

View File

@ -29,9 +29,9 @@ describe Invite do
fab!(:coding_horror) { Fabricate(:coding_horror) } fab!(:coding_horror) { Fabricate(:coding_horror) }
it "should not allow an invite with unformatted email address" do it "should not allow an invite with unformatted email address" do
expect { invite = Fabricate.build(:invite, email: "John Doe <john.doe@example.com>")
Fabricate(:invite, email: "John Doe <john.doe@example.com>") expect(invite.valid?).to eq(false)
}.to raise_error(ActiveRecord::RecordInvalid) expect(invite.errors.details[:email].first[:error]).to eq(I18n.t("user.email.invalid"))
end end
it "should not allow an invite with blacklisted email" do it "should not allow an invite with blacklisted email" do
@ -44,6 +44,11 @@ describe Invite do
expect(invite).to be_valid expect(invite).to be_valid
end 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 end
context '#create' do context '#create' do

View File

@ -3,32 +3,36 @@
require 'rails_helper' require 'rails_helper'
describe UserEmail do describe UserEmail do
fab!(:user) { Fabricate(:user) }
context "validation" do context "validation" do
it "allows only one primary email" do it "allows only one primary email" do
user = Fabricate(:user)
expect { expect {
Fabricate(:secondary_email, user: user, primary: true) Fabricate(:secondary_email, user: user, primary: true)
}.to raise_error(ActiveRecord::RecordInvalid) }.to raise_error(ActiveRecord::RecordInvalid)
end end
it "allows multiple secondary emails" do it "allows multiple secondary emails" do
user = Fabricate(:user)
Fabricate(:secondary_email, user: user, primary: false) Fabricate(:secondary_email, user: user, primary: false)
Fabricate(:secondary_email, user: user, primary: false) Fabricate(:secondary_email, user: user, primary: false)
expect(user.user_emails.count).to eq 3 expect(user.user_emails.count).to eq 3
end 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 end
context "indexes" do context "indexes" do
it "allows only one primary email" do it "allows only one primary email" do
user = Fabricate(:user)
expect { expect {
Fabricate.build(:secondary_email, user: user, primary: true).save(validate: false) Fabricate.build(:secondary_email, user: user, primary: true).save(validate: false)
}.to raise_error(ActiveRecord::RecordNotUnique) }.to raise_error(ActiveRecord::RecordNotUnique)
end end
it "allows multiple secondary emails" do 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)
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 expect(user.user_emails.count).to eq 3

View File

@ -282,7 +282,7 @@ describe UsersEmailController do
it 'raises an error without an invalid email' do it 'raises an error without an invalid email' do
put "/u/#{user.username}/preferences/email.json", params: { email: "sam@not-email.com'" } put "/u/#{user.username}/preferences/email.json", params: { email: "sam@not-email.com'" }
expect(response.status).to eq(422) expect(response.status).to eq(422)
expect(response.body).to include("email is invalid") expect(response.body).to include("Email is invalid")
end end
it "raises an error if you can't edit the user's email" do it "raises an error if you can't edit the user's email" do