Commit b34963bc authored by Rémy Coutable's avatar Rémy Coutable
Browse files

Validate email addresses using Devise.email_regexp

Also:
- Get rid of legacy :strict_mode
- Get rid of custom :email validator
- Add some shared examples to spec emails validation
parent a52c5778
......@@ -19,6 +19,7 @@ v 8.5.0 (unreleased)
- Fix label links for a merge request pointing to issues list
- Don't vendor minified JS
- Increase project import timeout to 15 minutes
- Be more permissive with email address validation: it only has to contain a single '@'
- Display 404 error on group not found
- Track project import failure
- Support Two-factor Authentication for LDAP users
......
......@@ -71,8 +71,8 @@ class ApplicationSetting < ActiveRecord::Base
url: true
validates :admin_notification_email,
allow_blank: true,
email: true
format: { with: Devise.email_regexp },
allow_blank: true
validates :two_factor_grace_period,
numericality: { greater_than_or_equal_to: 0 }
......
......@@ -15,7 +15,7 @@ class Email < ActiveRecord::Base
belongs_to :user
validates :user_id, presence: true
validates :email, presence: true, email: { strict_mode: true }, uniqueness: true
validates :email, presence: true, uniqueness: true, format: { with: Devise.email_regexp }
validate :unique_email, if: ->(email) { email.email_changed? }
before_validation :cleanup_email
......
......@@ -38,8 +38,8 @@ class Member < ActiveRecord::Base
presence: {
if: :invite?
},
email: {
strict_mode: true,
format: {
with: Devise.email_regexp,
allow_nil: true
},
uniqueness: {
......
......@@ -146,11 +146,8 @@ class User < ActiveRecord::Base
# Validations
#
validates :name, presence: true
# Note that a 'uniqueness' and presence check is provided by devise :validatable for email. We do not need to
# duplicate that here as the validation framework will have duplicate errors in the event of a failure.
validates :email, presence: true, email: { strict_mode: true }
validates :notification_email, presence: true, email: { strict_mode: true }
validates :public_email, presence: true, email: { strict_mode: true }, allow_blank: true, uniqueness: true
validates :notification_email, presence: true, format: { with: Devise.email_regexp }
validates :public_email, presence: true, uniqueness: true, format: { with: Devise.email_regexp }, allow_blank: true
validates :bio, length: { maximum: 255 }, allow_blank: true
validates :projects_limit, presence: true, numericality: { greater_than_or_equal_to: 0 }
validates :username,
......
# EmailValidator
#
# Based on https://github.com/balexand/email_validator
#
# Extended to use only strict mode with following allowed characters:
# ' - apostrophe
#
# See http://www.remote.org/jochen/mail/info/chars.html
#
class EmailValidator < ActiveModel::EachValidator
PATTERN = /\A\s*([-a-z0-9+._']{1,64})@((?:[-a-z0-9]+\.)+[a-z]{2,})\s*\z/i.freeze
def validate_each(record, attribute, value)
unless value =~ PATTERN
record.errors.add(attribute, options[:message] || :invalid)
end
end
end
......@@ -74,6 +74,10 @@
.only_integer
.is_greater_than(0)
end
it_behaves_like 'an object with email-formated attributes', :admin_notification_email do
subject { setting }
end
end
context 'restricted signup domains' do
......
# == Schema Information
#
# Table name: emails
#
# id :integer not null, primary key
# user_id :integer not null
# email :string(255) not null
# created_at :datetime
# updated_at :datetime
#
require 'spec_helper'
describe Email, models: true do
describe 'validations' do
it_behaves_like 'an object with email-formated attributes', :email do
subject { build(:email) }
end
end
end
......@@ -31,6 +31,10 @@
it { is_expected.to validate_presence_of(:source) }
it { is_expected.to validate_inclusion_of(:access_level).in_array(Gitlab::Access.values) }
it_behaves_like 'an object with email-formated attributes', :invite_email do
subject { build(:project_member) }
end
context "when an invite email is provided" do
let(:member) { build(:project_member, invite_email: "user@example.com", user: nil) }
......@@ -159,7 +163,7 @@
describe "#generate_invite_token" do
let!(:member) { create(:project_member, invite_email: "user@example.com", user: nil) }
it "sets the invite token" do
expect { member.generate_invite_token }.to change { member.invite_token}
end
......
......@@ -119,37 +119,15 @@
it { is_expected.to validate_length_of(:bio).is_within(0..255) }
describe 'email' do
it 'accepts info@example.com' do
user = build(:user, email: 'info@example.com')
expect(user).to be_valid
end
it 'accepts info+test@example.com' do
user = build(:user, email: 'info+test@example.com')
expect(user).to be_valid
end
it "accepts o'reilly@example.com" do
user = build(:user, email: "o'reilly@example.com")
expect(user).to be_valid
end
it 'rejects test@test@example.com' do
user = build(:user, email: 'test@test@example.com')
expect(user).to be_invalid
end
it 'rejects mailto:test@example.com' do
user = build(:user, email: 'mailto:test@example.com')
expect(user).to be_invalid
end
it_behaves_like 'an object with email-formated attributes', :email do
subject { build(:user) }
end
it "rejects lol!'+=?><#$%^&*()@gmail.com" do
user = build(:user, email: "lol!'+=?><#$%^&*()@gmail.com")
expect(user).to be_invalid
end
it_behaves_like 'an object with email-formated attributes', :public_email, :notification_email do
subject { build(:user).tap { |user| user.emails << build(:email, email: email_value) } }
end
describe 'email' do
context 'when no signup domains listed' do
before { allow(current_application_settings).to receive(:restricted_signup_domains).and_return([]) }
it 'accepts any email' do
......
# Specifications for behavior common to all objects with an email attribute.
# Takes a list of email-format attributes and requires:
# - subject { "the object with a attribute= setter" }
# Note: You have access to `email_value` which is the email address value
# being currently tested).
shared_examples 'an object with email-formated attributes' do |*attributes|
attributes.each do |attribute|
describe "specifically its :#{attribute} attribute" do
%w[
info@example.com
info+test@example.com
o'reilly@example.com
mailto:test@example.com
lol!'+=?><#$%^&*()@gmail.com
].each do |valid_email|
context "with a value of '#{valid_email}'" do
let(:email_value) { valid_email }
it 'is valid' do
subject.send("#{attribute}=", valid_email)
expect(subject).to be_valid
end
end
end
%w[
foobar
test@test@example.com
].each do |invalid_email|
context "with a value of '#{invalid_email}'" do
let(:email_value) { invalid_email }
it 'is invalid' do
subject.send("#{attribute}=", invalid_email)
expect(subject).to be_invalid
end
end
end
end
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