Merge branch 'email-domain-blacklist' into 'master'

Added the ability to block sign ups using a domain blacklist.

As part of this MR, I restructured the Application Settings form to separate **Sign up** related settings from **Sign in** related settings and make everything cleaner and easier to read.

Fixes #19749 

Related to #5573

See merge request !5259

2 files changed, 74 insertions, 39 deletions

diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb
index c6f77cc055f..8c19d9dc9c8 100644
--- a/app/models/application_setting.rb
+++ b/app/models/application_setting.rb
@@ -4,12 +4,20 @@ class ApplicationSetting < ActiveRecord::Base
   add_authentication_token_field :health_check_access_token
 
   CACHE_KEY = 'application_setting.last'
+  DOMAIN_LIST_SEPARATOR = %r{\s*[,;]\s*     # comma or semicolon, optionally surrounded by whitespace
+                            |               # or
+                            \s              # any whitespace character
+                            |               # or
+                            [\r\n]          # any number of newline characters
+                          }x
 
   serialize :restricted_visibility_levels
   serialize :import_sources
   serialize :disabled_oauth_sign_in_sources, Array
-  serialize :restricted_signup_domains, Array
-  attr_accessor :restricted_signup_domains_raw
+  serialize :domain_whitelist, Array
+  serialize :domain_blacklist, Array
+
+  attr_accessor :domain_whitelist_raw, :domain_blacklist_raw
 
   validates :session_expire_delay,
             presence: true,
@@ -62,6 +70,10 @@ class ApplicationSetting < ActiveRecord::Base
   validates :enabled_git_access_protocol,
             inclusion: { in: %w(ssh http), allow_blank: true, allow_nil: true }
 
+  validates :domain_blacklist,
+            presence: { message: 'Domain blacklist cannot be empty if Blacklist is enabled.' },
+            if: :domain_blacklist_enabled?
+
   validates_each :restricted_visibility_levels do |record, attr, value|
     unless value.nil?
       value.each do |level|
@@ -129,7 +141,7 @@ class ApplicationSetting < ActiveRecord::Base
       session_expire_delay: Settings.gitlab['session_expire_delay'],
       default_project_visibility: Settings.gitlab.default_projects_features['visibility_level'],
       default_snippet_visibility: Settings.gitlab.default_projects_features['visibility_level'],
-      restricted_signup_domains: Settings.gitlab['restricted_signup_domains'],
+      domain_whitelist: Settings.gitlab['domain_whitelist'],
       import_sources: %w[github bitbucket gitlab gitorious google_code fogbugz git gitlab_project],
       shared_runners_enabled: Settings.gitlab_ci['shared_runners_enabled'],
       max_artifacts_size: Settings.artifacts['max_size'],
@@ -150,20 +162,30 @@ class ApplicationSetting < ActiveRecord::Base
     ActiveRecord::Base.connection.column_exists?(:application_settings, :home_page_url)
   end
 
-  def restricted_signup_domains_raw
-    self.restricted_signup_domains.join("\n") unless self.restricted_signup_domains.nil?
+  def domain_whitelist_raw
+    self.domain_whitelist.join("\n") unless self.domain_whitelist.nil?
+  end
+
+  def domain_blacklist_raw
+    self.domain_blacklist.join("\n") unless self.domain_blacklist.nil?
+  end
+
+  def domain_whitelist_raw=(values)
+    self.domain_whitelist = []
+    self.domain_whitelist = values.split(DOMAIN_LIST_SEPARATOR)
+    self.domain_whitelist.reject! { |d| d.empty? }
+    self.domain_whitelist
+  end
+
+  def domain_blacklist_raw=(values)
+    self.domain_blacklist = []
+    self.domain_blacklist = values.split(DOMAIN_LIST_SEPARATOR)
+    self.domain_blacklist.reject! { |d| d.empty? }
+    self.domain_blacklist
   end
 
-  def restricted_signup_domains_raw=(values)
-    self.restricted_signup_domains = []
-    self.restricted_signup_domains = values.split(
-      /\s*[,;]\s*     # comma or semicolon, optionally surrounded by whitespace
-      |               # or
-      \s              # any whitespace character
-      |               # or
-      [\r\n]          # any number of newline characters
-      /x)
-    self.restricted_signup_domains.reject! { |d| d.empty? }
+  def domain_blacklist_file=(file)
+    self.domain_blacklist_raw = file.read
   end
 
   def runners_registration_token
diff --git a/app/models/user.rb b/app/models/user.rb
index 975e935fa20..7cbd97c0069 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -111,7 +111,7 @@ class User < ActiveRecord::Base
   validates :avatar, file_size: { maximum: 200.kilobytes.to_i }
 
   before_validation :generate_password, on: :create
-  before_validation :restricted_signup_domains, on: :create
+  before_validation :signup_domain_valid?, on: :create
   before_validation :sanitize_attrs
   before_validation :set_notification_email, if: ->(user) { user.email_changed? }
   before_validation :set_public_email, if: ->(user) { user.public_email_changed? }
@@ -760,29 +760,6 @@ class User < ActiveRecord::Base
     Project.where(id: events)
   end
 
-  def restricted_signup_domains
-    email_domains = current_application_settings.restricted_signup_domains
-
-    unless email_domains.blank?
-      match_found = email_domains.any? do |domain|
-        escaped = Regexp.escape(domain).gsub('\*', '.*?')
-        regexp = Regexp.new "^#{escaped}$", Regexp::IGNORECASE
-        email_domain = Mail::Address.new(self.email).domain
-        email_domain =~ regexp
-      end
-
-      unless match_found
-        self.errors.add :email,
-                        'is not whitelisted. ' +
-                        'Email domains valid for registration are: ' +
-                        email_domains.join(', ')
-        return false
-      end
-    end
-
-    true
-  end
-
   def can_be_removed?
     !solo_owned_groups.present?
   end
@@ -881,4 +858,40 @@ class User < ActiveRecord::Base
     self.can_create_group   = false
     self.projects_limit     = 0
   end
+
+  def signup_domain_valid?
+    valid = true
+    error = nil
+
+    if current_application_settings.domain_blacklist_enabled?
+      blocked_domains = current_application_settings.domain_blacklist
+      if domain_matches?(blocked_domains, self.email)
+        error = 'is not from an allowed domain.'
+        valid = false
+      end
+    end
+
+    allowed_domains = current_application_settings.domain_whitelist
+    unless allowed_domains.blank?
+      if domain_matches?(allowed_domains, self.email)
+        valid = true
+      else
+        error = "is not whitelisted. Email domains valid for registration are: #{allowed_domains.join(', ')}"
+        valid = false
+      end
+    end
+
+    self.errors.add(:email, error) unless valid
+
+    valid
+  end
+
+  def domain_matches?(email_domains, email)
+    signup_domain = Mail::Address.new(email).domain
+    email_domains.any? do |domain|
+      escaped = Regexp.escape(domain).gsub('\*', '.*?')
+      regexp = Regexp.new "^#{escaped}$", Regexp::IGNORECASE
+      signup_domain =~ regexp
+    end
+  end
 end