Commit 866a5fa0 authored by Magdalena Frankiewicz's avatar Magdalena Frankiewicz

Use the same email validation for User and Email

Otherwise, it is possible that a `User` primary email is successfully
saved, but it cannot be added to the emails table due to being invalid
for the `Email` model.

The `Email#email` validation is now more relaxed, and the same as the
`User#email` validation: it is the Devise email validation, that
only checks that there is a single @ sign separating local
part and domain. The previous stricter Email#email validation was
enforcing compliance with RFC3696, but many mail servers allow addresses
that are not fully compliant with the RFC. Therefore, the relaxed
validation is preferable, to avoid blocking working email addresses.

Changelog: fixed
parent 62403975
...@@ -6,8 +6,8 @@ class Email < ApplicationRecord ...@@ -6,8 +6,8 @@ class Email < ApplicationRecord
belongs_to :user, optional: false belongs_to :user, optional: false
validates :email, presence: true, uniqueness: true validates :email, presence: true, uniqueness: true, devise_email: true
validate :validate_email_format
validate :unique_email, if: ->(email) { email.email_changed? } validate :unique_email, if: ->(email) { email.email_changed? }
scope :confirmed, -> { where.not(confirmed_at: nil) } scope :confirmed, -> { where.not(confirmed_at: nil) }
...@@ -33,10 +33,6 @@ class Email < ApplicationRecord ...@@ -33,10 +33,6 @@ class Email < ApplicationRecord
self.errors.add(:email, 'has already been taken') if primary_email_of_another_user? self.errors.add(:email, 'has already been taken') if primary_email_of_another_user?
end end
def validate_email_format
self.errors.add(:email, I18n.t(:invalid, scope: 'valid_email.validations.email')) unless ValidateEmail.valid?(self.email)
end
# once email is confirmed, update the gpg signatures # once email is confirmed, update the gpg signatures
def update_invalid_gpg_signatures def update_invalid_gpg_signatures
user.update_invalid_gpg_signatures if confirmed? user.update_invalid_gpg_signatures if confirmed?
......
...@@ -16,7 +16,7 @@ RSpec.describe Gitlab::UsageDataNonSqlMetrics do ...@@ -16,7 +16,7 @@ RSpec.describe Gitlab::UsageDataNonSqlMetrics do
described_class.uncached_data described_class.uncached_data
end end
expect(recorder.count).to eq(63) expect(recorder.count).to eq(65)
end end
end end
end end
...@@ -49,7 +49,7 @@ RSpec.describe Profiles::EmailsController do ...@@ -49,7 +49,7 @@ RSpec.describe Profiles::EmailsController do
end end
context 'when email address is invalid' do context 'when email address is invalid' do
let(:email) { 'invalid.@example.com' } let(:email) { 'invalid@@example.com' }
it 'does not send an email confirmation' do it 'does not send an email confirmation' do
expect { subject }.not_to change { ActionMailer::Base.deliveries.size } expect { subject }.not_to change { ActionMailer::Base.deliveries.size }
......
...@@ -44,7 +44,7 @@ RSpec.describe 'Profile > Emails' do ...@@ -44,7 +44,7 @@ RSpec.describe 'Profile > Emails' do
end end
it 'does not add an invalid email' do it 'does not add an invalid email' do
fill_in('Email', with: 'test.@example.com') fill_in('Email', with: 'test@@example.com')
click_button('Add email address') click_button('Add email address')
email = user.emails.find_by(email: email) email = user.emails.find_by(email: email)
......
...@@ -10,7 +10,7 @@ RSpec.describe Email do ...@@ -10,7 +10,7 @@ RSpec.describe Email do
end end
describe 'validations' do describe 'validations' do
it_behaves_like 'an object with RFC3696 compliant email-formatted attributes', :email do it_behaves_like 'an object with email-formatted attributes', :email do
subject { build(:email) } subject { build(:email) }
end end
......
...@@ -436,7 +436,7 @@ RSpec.describe User do ...@@ -436,7 +436,7 @@ RSpec.describe User do
subject { build(:user) } subject { build(:user) }
end end
it_behaves_like 'an object with RFC3696 compliant email-formatted attributes', :public_email, :notification_email do it_behaves_like 'an object with email-formatted attributes', :public_email, :notification_email do
subject { create(:user).tap { |user| user.emails << build(:email, email: email_value, confirmed_at: Time.current) } } subject { create(:user).tap { |user| user.emails << build(:email, email: email_value, confirmed_at: Time.current) } }
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment