Merge branch 'nicolasdular/email-restriction-regex' into 'master'

Add restrictions for email addresses

See merge request gitlab-org/gitlab!25122

app/helpers/application_settings_helper.rb

@@ -315,7 +315,9 @@ module ApplicationSettingsHelper
       :push_event_hooks_limit,
       :push_event_activities_limit,
       :custom_http_clone_url_root,
-      :snippet_size_limit
+      :snippet_size_limit,
+      :email_restrictions_enabled,
+      :email_restrictions
     ]
   end
 

app/models/application_setting.rb

@@ -243,6 +243,8 @@ class ApplicationSetting < ApplicationRecord
 
   validates :snippet_size_limit, numericality: { only_integer: true, greater_than: 0 }
 
+  validate :email_restrictions_regex_valid?
+
   SUPPORTED_KEY_TYPES.each do |type|
     validates :"#{type}_key_restriction", presence: true, key_restriction: { type: type }
   end
@@ -381,6 +383,14 @@ class ApplicationSetting < ApplicationRecord
   def recaptcha_or_login_protection_enabled
     recaptcha_enabled || login_recaptcha_protection_enabled
   end
+
+  def email_restrictions_regex_valid?
+    return if email_restrictions.blank?
+
+    Gitlab::UntrustedRegexp.new(email_restrictions)
+  rescue RegexpError
+    errors.add(:email_restrictions, _('is not a valid regular expression'))
+  end
 end
 
 ApplicationSetting.prepend_if_ee('EE::ApplicationSetting')

app/models/application_setting_implementation.rb

@@ -62,6 +62,8 @@ module ApplicationSettingImplementation
         eks_account_id: nil,
         eks_access_key_id: nil,
         eks_secret_access_key: nil,
+        email_restrictions_enabled: false,
+        email_restrictions: nil,
         first_day_of_week: 0,
         gitaly_timeout_default: 55,
         gitaly_timeout_fast: 10,

app/models/user.rb

@@ -189,6 +189,7 @@ class User < ApplicationRecord
   validate :owns_public_email, if: :public_email_changed?
   validate :owns_commit_email, if: :commit_email_changed?
   validate :signup_domain_valid?, on: :create, if: ->(user) { !user.created_by_id }
+  validate :check_email_restrictions, on: :create, if: ->(user) { !user.created_by_id }
 
   validates :theme_id, allow_nil: true, inclusion: { in: Gitlab::Themes.valid_ids,
     message: _("%{placeholder} is not a valid theme") % { placeholder: '%{value}' } }
@@ -1751,6 +1752,18 @@ class User < ApplicationRecord
     end
   end
 
+  def check_email_restrictions
+    return unless Feature.enabled?(:email_restrictions)
+    return unless Gitlab::CurrentSettings.email_restrictions_enabled?
+
+    restrictions = Gitlab::CurrentSettings.email_restrictions
+    return if restrictions.blank?
+
+    if Gitlab::UntrustedRegexp.new(restrictions).match?(email)
+      errors.add(:email, _('is not allowed for sign-up'))
+    end
+  end
+
   def self.unique_internal(scope, username, email_pattern, &block)
     scope.first || create_unique_internal(scope, username, email_pattern, &block)
   end

app/views/admin/application_settings/_signup.html.haml

@@ -49,6 +49,20 @@
       = f.label :domain_blacklist, 'Blacklisted domains for sign-ups', class: 'label-bold'
       = f.text_area :domain_blacklist_raw, placeholder: 'domain.com', class: 'form-control', rows: 8
       .form-text.text-muted Users with e-mail addresses that match these domain(s) will NOT be able to sign-up. Wildcards allowed. Use separate lines for multiple entries. Ex: domain.com, *.domain.com
+    - if Feature.enabled?(:email_restrictions)
+      .form-group
+        = f.label :email_restrictions_enabled, _('Email restrictions'), class: 'label-bold'
+        .form-check
+          = f.check_box :email_restrictions_enabled, class: 'form-check-input'
+          = f.label :email_restrictions_enabled, class: 'form-check-label' do
+            = _('Enable email restrictions for sign ups')
+      .form-group
+        = f.label :email_restrictions, _('Email restrictions for sign-ups'), class: 'label-bold'
+        = f.text_area :email_restrictions, class: 'form-control', rows: 4
+        .form-text.text-muted
+        - supported_syntax_link_url = 'https://github.com/google/re2/wiki/Syntax'
+        - supported_syntax_link_start = '<a href="%{url}" target="_blank" rel="noopener noreferrer">'.html_safe % { url: supported_syntax_link_url }
+        = _('Restricts sign-ups for email addresses that match the given regex. See the %{supported_syntax_link_start}supported syntax%{supported_syntax_link_end} for more information.').html_safe % { supported_syntax_link_start: supported_syntax_link_start, supported_syntax_link_end: '</a>'.html_safe }
 
     .form-group
       = f.label :after_sign_up_text, class: 'label-bold'

changelogs/unreleased/nicolasdular-email-restriction-regex.yml

+---
+title: Add restrictions for signup email addresses
+merge_request: 25122
+author:
+type: added

db/migrate/20200213093702_add_email_restrictions_to_application_settings.rb

+# frozen_string_literal: true
+
+class AddEmailRestrictionsToApplicationSettings < ActiveRecord::Migration[6.0]
+  DOWNTIME = false
+
+  def up
+    add_column(:application_settings, :email_restrictions_enabled, :boolean, default: false, null: false)
+    add_column(:application_settings, :email_restrictions, :text, null: true)
+  end
+
+  def down
+    remove_column(:application_settings, :email_restrictions_enabled)
+    remove_column(:application_settings, :email_restrictions)
+  end
+end

db/schema.rb

@@ -349,6 +349,8 @@ ActiveRecord::Schema.define(version: 2020_02_13_220211) do
     t.boolean "disable_overriding_approvers_per_merge_request", default: false, null: false
     t.boolean "prevent_merge_requests_author_approval", default: false, null: false
     t.boolean "prevent_merge_requests_committers_approval", default: false, null: false
+    t.boolean "email_restrictions_enabled", default: false, null: false
+    t.text "email_restrictions"
     t.index ["custom_project_templates_group_id"], name: "index_application_settings_on_custom_project_templates_group_id"
     t.index ["file_template_project_id"], name: "index_application_settings_on_file_template_project_id"
     t.index ["instance_administration_project_id"], name: "index_applicationsettings_on_instance_administration_project_id"

locale/gitlab.pot

@@ -7071,6 +7071,12 @@ msgstr ""
 msgid "Email patch"
 msgstr ""
 
+msgid "Email restrictions"
+msgstr ""
+
+msgid "Email restrictions for sign-ups"
+msgstr ""
+
 msgid "Email the pipelines status to a list of recipients."
 msgstr ""
 
@@ -7179,6 +7185,9 @@ msgstr ""
 msgid "Enable classification control using an external service"
 msgstr ""
 
+msgid "Enable email restrictions for sign ups"
+msgstr ""
+
 msgid "Enable error tracking"
 msgstr ""
 
@@ -16377,6 +16386,9 @@ msgstr ""
 msgid "Restrict membership by email"
 msgstr ""
 
+msgid "Restricts sign-ups for email addresses that match the given regex. See the %{supported_syntax_link_start}supported syntax%{supported_syntax_link_end} for more information."
+msgstr ""
+
 msgid "Resume"
 msgstr ""
 
@@ -23097,6 +23109,12 @@ msgstr ""
 msgid "is not a valid X509 certificate."
 msgstr ""
 
+msgid "is not a valid regular expression"
+msgstr ""
+
+msgid "is not allowed for sign-up"
+msgstr ""
+
 msgid "is not an email you own"
 msgstr ""
 

spec/models/application_setting_spec.rb

@@ -633,5 +633,56 @@ describe ApplicationSetting do
     end
   end
 
+  describe 'email_restrictions' do
+    context 'when email restrictions are enabled' do
+      before do
+        subject.email_restrictions_enabled = true
+      end
+
+      it 'allows empty email restrictions' do
+        subject.email_restrictions = ''
+
+        expect(subject).to be_valid
+      end
+
+      it 'accepts valid email restrictions regex' do
+        subject.email_restrictions = '\+'
+
+        expect(subject).to be_valid
+      end
+
+      it 'does not accept invalid email restrictions regex' do
+        subject.email_restrictions = '+'
+
+        expect(subject).not_to be_valid
+      end
+
+      it 'sets an error when regex is not valid' do
+        subject.email_restrictions = '+'
+
+        expect(subject).not_to be_valid
+        expect(subject.errors.messages[:email_restrictions].first).to eq(_('is not a valid regular expression'))
+      end
+    end
+
+    context 'when email restrictions are disabled' do
+      before do
+        subject.email_restrictions_enabled = false
+      end
+
+      it 'allows empty email restrictions' do
+        subject.email_restrictions = ''
+
+        expect(subject).to be_valid
+      end
+
+      it 'invalid regex is not valid' do
+        subject.email_restrictions = '+'
+
+        expect(subject).not_to be_valid
+      end
+    end
+  end
+
   it_behaves_like 'application settings examples'
 end

spec/models/user_spec.rb

@@ -430,6 +430,73 @@ describe User, :do_not_mock_admin_mode do
         end
       end
 
+      context 'email restrictions' do
+        context 'when email restriction is disabled' do
+          before do
+            stub_application_setting(email_restrictions_enabled: false)
+            stub_application_setting(email_restrictions: '\+')
+          end
+
+          it 'does accept email address' do
+            user = build(:user, email: 'info+1@test.com')
+
+            expect(user).to be_valid
+          end
+        end
+
+        context 'when email restrictions is enabled' do
+          before do
+            stub_application_setting(email_restrictions_enabled: true)
+            stub_application_setting(email_restrictions: '([\+]|\b(\w*gitlab.com\w*)\b)')
+          end
+
+          it 'does not accept email address with + characters' do
+            user = build(:user, email: 'info+1@test.com')
+
+            expect(user).not_to be_valid
+          end
+
+          it 'does not accept email with a gitlab domain' do
+            user = build(:user, email: 'info@gitlab.com')
+
+            expect(user).not_to be_valid
+          end
+
+          it 'adds an error message when email is not accepted' do
+            user = build(:user, email: 'info@gitlab.com')
+
+            expect(user).not_to be_valid
+            expect(user.errors.messages[:email].first).to eq(_('is not allowed for sign-up'))
+          end
+
+          it 'does accept a valid email address' do
+            user = build(:user, email: 'info@test.com')
+
+            expect(user).to be_valid
+          end
+
+          context 'when feature flag is turned off' do
+            before do
+              stub_feature_flags(email_restrictions: false)
+            end
+
+            it 'does accept the email address' do
+              user = build(:user, email: 'info+1@test.com')
+
+              expect(user).to be_valid
+            end
+          end
+
+          context 'when created_by_id is set' do
+            it 'does accept the email address' do
+              user = build(:user, email: 'info+1@test.com', created_by_id: 1)
+
+              expect(user).to be_valid
+            end
+          end
+        end
+      end
+
       context 'owns_notification_email' do
         it 'accepts temp_oauth_email emails' do
           user = build(:user, email: "temp-email-for-oauth@example.com")