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

10 files changed, 228 insertions, 33 deletions

diff --git a/app/controllers/admin/application_settings_controller.rb b/app/controllers/admin/application_settings_controller.rb
index 23ba83aba0e..3e27320ee5c 100644
--- a/app/controllers/admin/application_settings_controller.rb
+++ b/app/controllers/admin/application_settings_controller.rb
@@ -64,6 +64,7 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController
     params[:application_setting][:disabled_oauth_sign_in_sources] =
       AuthHelper.button_based_providers.map(&:to_s) -
       Array(enabled_oauth_sign_in_sources)
+    params.delete(:domain_blacklist_raw) if params[:domain_blacklist_file]
 
     params.require(:application_setting).permit(
       :default_projects_limit,
@@ -112,6 +113,9 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController
       :container_registry_token_expire_delay,
       :repository_storage,
       :enabled_git_access_protocol,
+      :domain_blacklist_enabled,
+      :domain_blacklist_raw,
+      :domain_blacklist_file,
       restricted_visibility_levels: [],
       import_sources: [],
       disabled_oauth_sign_in_sources: []
diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb
index c6f77cc055f..84b1b54eeae 100644
--- a/app/models/application_setting.rb
+++ b/app/models/application_setting.rb
@@ -9,7 +9,9 @@ class ApplicationSetting < ActiveRecord::Base
   serialize :import_sources
   serialize :disabled_oauth_sign_in_sources, Array
   serialize :restricted_signup_domains, Array
-  attr_accessor :restricted_signup_domains_raw
+  serialize :domain_blacklist, Array
+
+  attr_accessor :restricted_signup_domains_raw, :domain_blacklist_raw
 
   validates :session_expire_delay,
             presence: true,
@@ -62,6 +64,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: true,
+            if: :domain_blacklist_enabled?
+
   validates_each :restricted_visibility_levels do |record, attr, value|
     unless value.nil?
       value.each do |level|
@@ -154,18 +160,35 @@ class ApplicationSetting < ActiveRecord::Base
     self.restricted_signup_domains.join("\n") unless self.restricted_signup_domains.nil?
   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
+  def domain_blacklist_raw
+    self.domain_blacklist.join("\n") unless self.domain_blacklist.nil?
+  end
+
+  def splitter
+    /\s*[,;]\s*     # comma or semicolon, optionally surrounded by whitespace
       |               # or
       \s              # any whitespace character
       |               # or
       [\r\n]          # any number of newline characters
-      /x)
+    /x
+  end
+
+  def restricted_signup_domains_raw=(values)
+    self.restricted_signup_domains = []
+    self.restricted_signup_domains = values.split(splitter)
     self.restricted_signup_domains.reject! { |d| d.empty? }
   end
 
+  def domain_blacklist_raw=(values)
+    self.domain_blacklist = []
+    self.domain_blacklist = values.split(splitter)
+    self.domain_blacklist.reject! { |d| d.empty? }
+  end
+
+  def domain_blacklist_file=(file)
+    self.domain_blacklist_raw = file.read
+  end
+
   def runners_registration_token
     ensure_runners_registration_token!
   end
diff --git a/app/models/user.rb b/app/models/user.rb
index 3d0a033785c..b0c5d84fc40 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,27 +760,41 @@ class User < ActiveRecord::Base
     Project.where(id: events)
   end
 
-  def restricted_signup_domains
-    email_domains = current_application_settings.restricted_signup_domains
+  def match_domain(email_domains)
+    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
+  end
+
+  def signup_domain_valid?
+    valid = true
 
-    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
+    if current_application_settings.domain_blacklist_enabled?
+      blocked_domains = current_application_settings.domain_blacklist
+      if match_domain(blocked_domains)
+        self.errors.add :email, 'is not from an allowed domain.'
+        valid = false
       end
+    end
 
-      unless match_found
+    allowed_domains = current_application_settings.restricted_signup_domains
+    unless allowed_domains.blank?
+      if match_domain(allowed_domains)
+        self.errors.clear
+        valid = true
+      else
         self.errors.add :email,
                         'is not whitelisted. ' +
                         'Email domains valid for registration are: ' +
-                        email_domains.join(', ')
-        return false
+                        allowed_domains.join(', ')
+        valid = false
       end
     end
 
-    true
+    return valid
   end
 
   def can_be_removed?
diff --git a/app/views/admin/application_settings/_form.html.haml b/app/views/admin/application_settings/_form.html.haml
index 538d8176ce7..9443fe5e1d3 100644
--- a/app/views/admin/application_settings/_form.html.haml
+++ b/app/views/admin/application_settings/_form.html.haml
@@ -109,7 +109,7 @@
             Newly registered users will by default be external
 
   %fieldset
-    %legend Sign-in Restrictions
+    %legend Sign-up Restrictions
     .form-group
       .col-sm-offset-2.col-sm-10
         .checkbox
@@ -123,6 +123,49 @@
             = f.check_box :send_user_confirmation_email
             Send confirmation email on sign-up
     .form-group
+      = f.label :restricted_signup_domains, 'Restricted domains for sign-ups', class: 'control-label col-sm-2'
+      .col-sm-10
+        = f.text_area :restricted_signup_domains_raw, placeholder: 'domain.com', class: 'form-control'
+        .help-block ONLY users with e-mail addresses that match these domain(s) will be able to sign-up. Wildcards allowed. Use separate lines for multiple entries. Ex: domain.com, *.domain.com
+    .form-group
+      = f.label :domain_blacklist_enabled, 'Domain Blacklist', class: 'control-label col-sm-2'
+      .col-sm-10
+        .checkbox
+          = f.label :domain_blacklist_enabled do
+            = f.check_box :domain_blacklist_enabled
+            Enable domain blacklist for sign ups
+    .form-group
+      .col-sm-offset-2.col-sm-10
+        .radio
+          = label_tag :blacklist_type_file do
+            = radio_button_tag :blacklist_type, :file, @application_setting.domain_blacklist.blank?
+            .option-title
+              Upload blacklist file
+        .radio
+          = label_tag :blacklist_type_raw do
+            = radio_button_tag :blacklist_type, :raw, @application_setting.domain_blacklist.present?
+            .option-title
+              Enter blacklist manually
+    .form-group.blacklist-file
+      = f.label :domain_blacklist_file, 'Blacklist file', class: 'control-label col-sm-2'
+      .col-sm-10
+        = f.file_field :domain_blacklist_file, class: 'form-control', accept: '.txt,.conf'
+        .help-block Users with e-mail addresses that match these domain(s) will NOT be able to sign-up. Wildcards allowed. Use separate lines or commas for multiple entries.
+    .form-group.blacklist-raw
+      = f.label :domain_blacklist, 'Blacklisted domains', class: 'control-label col-sm-2'
+      .col-sm-10
+        = f.text_area :domain_blacklist_raw, placeholder: 'domain.com', class: 'form-control', rows: 10
+        .help-block 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
+
+    .form-group
+      = f.label :after_sign_up_text, class: 'control-label col-sm-2'
+      .col-sm-10
+        = f.text_area :after_sign_up_text, class: 'form-control', rows: 4
+        .help-block Markdown enabled
+
+  %fieldset
+    %legend Sign-in Restrictions
+    .form-group
       .col-sm-offset-2.col-sm-10
         .checkbox
           = f.label :signin_enabled do
@@ -148,11 +191,6 @@
         = f.number_field :two_factor_grace_period, min: 0, class: 'form-control', placeholder: '0'
         .help-block Amount of time (in hours) that users are allowed to skip forced configuration of two-factor authentication
     .form-group
-      = f.label :restricted_signup_domains, 'Restricted domains for sign-ups', class: 'control-label col-sm-2'
-      .col-sm-10
-        = f.text_area :restricted_signup_domains_raw, placeholder: 'domain.com', class: 'form-control'
-        .help-block Only users with e-mail addresses that match these domain(s) will be able to sign-up. Wildcards allowed. Use separate lines for multiple entries. Ex: domain.com, *.domain.com
-    .form-group
       = f.label :home_page_url, 'Home page URL', class: 'control-label col-sm-2'
       .col-sm-10
         = f.text_field :home_page_url, class: 'form-control', placeholder: 'http://company.example.com', :'aria-describedby' => 'home_help_block'
@@ -168,11 +206,6 @@
         = f.text_area :sign_in_text, class: 'form-control', rows: 4
         .help-block Markdown enabled
     .form-group
-      = f.label :after_sign_up_text, class: 'control-label col-sm-2'
-      .col-sm-10
-        = f.text_area :after_sign_up_text, class: 'form-control', rows: 4
-        .help-block Markdown enabled
-    .form-group
       = f.label :help_page_text, class: 'control-label col-sm-2'
       .col-sm-10
         = f.text_area :help_page_text, class: 'form-control', rows: 4
@@ -353,3 +386,21 @@
 
   .form-actions
     = f.submit 'Save', class: 'btn btn-save'
+
+:javascript
+  function showBlacklistType() {
+    if ($("input[name='blacklist_type']:checked").val() == "file")
+    {
+      $(".blacklist-file").show();
+      $(".blacklist-raw").hide();
+    }
+    else
+    {
+      $(".blacklist-file").hide();
+      $(".blacklist-raw").show();
+    }
+  }
+
+  $("input[name='blacklist_type']").click(showBlacklistType);
+
+  showBlacklistType();
\ No newline at end of file
diff --git a/db/migrate/20160713205315_add_domain_blacklist_to_application_settings.rb b/db/migrate/20160713205315_add_domain_blacklist_to_application_settings.rb
new file mode 100644
index 00000000000..ecdd1bd7e5e
--- /dev/null
+++ b/db/migrate/20160713205315_add_domain_blacklist_to_application_settings.rb
@@ -0,0 +1,22 @@
+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class AddDomainBlacklistToApplicationSettings < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  # When using the methods "add_concurrent_index" or "add_column_with_default"
+  # you must disable the use of transactions as these methods can not run in an
+  # existing transaction. When using "add_concurrent_index" make sure that this
+  # method is the _only_ method called in the migration, any other changes
+  # should go in a separate migration. This ensures that upon failure _only_ the
+  # index creation fails and can be retried or reverted easily.
+  #
+  # To disable transactions uncomment the following line and remove these
+  # comments:
+  # disable_ddl_transaction!
+
+  def change
+    add_column :application_settings, :domain_blacklist_enabled, :boolean, default: false
+    add_column :application_settings, :domain_blacklist, :text
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 8882377f9f4..25d94f283c9 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -88,6 +88,8 @@ ActiveRecord::Schema.define(version: 20160716115710) do
     t.text     "after_sign_up_text"
     t.string   "repository_storage",                    default: "default"
     t.string   "enabled_git_access_protocol"
+    t.boolean  "domain_blacklist_enabled",              default: false
+    t.text     "domain_blacklist"
   end
 
   create_table "audit_events", force: :cascade do |t|
diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index 3c79a00eb8c..4cd388658be 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -413,6 +413,8 @@ module API
       expose :default_snippet_visibility
       expose :default_group_visibility
       expose :restricted_signup_domains
+      expose :domain_blacklist_enabled
+      expose :domain_blacklist
       expose :user_oauth_applications
       expose :after_sign_out_path
       expose :container_registry_token_expire_delay
diff --git a/spec/fixtures/blacklist.txt b/spec/fixtures/blacklist.txt
new file mode 100644
index 00000000000..baeb11eda9a
--- /dev/null
+++ b/spec/fixtures/blacklist.txt
@@ -0,0 +1,3 @@
+example.com
+test.com
+foo.bar
\ No newline at end of file
diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb
index 2ea1320267c..582d9a8d8cd 100644
--- a/spec/models/application_setting_spec.rb
+++ b/spec/models/application_setting_spec.rb
@@ -73,4 +73,31 @@ describe ApplicationSetting, models: true do
       expect(setting.restricted_signup_domains).to eq(['example.com', '*.example.com'])
     end
   end
+
+  context 'blacklisted signup domains' do
+    it 'set single domain' do
+      setting.domain_blacklist_raw = 'example.com'
+      expect(setting.domain_blacklist).to eq(['example.com'])
+    end
+
+    it 'set multiple domains with spaces' do
+      setting.domain_blacklist_raw = 'example.com *.example.com'
+      expect(setting.domain_blacklist).to eq(['example.com', '*.example.com'])
+    end
+
+    it 'set multiple domains with newlines and a space' do
+      setting.domain_blacklist_raw = "example.com\n *.example.com"
+      expect(setting.domain_blacklist).to eq(['example.com', '*.example.com'])
+    end
+
+    it 'set multiple domains with commas' do
+      setting.domain_blacklist_raw = "example.com, *.example.com"
+      expect(setting.domain_blacklist).to eq(['example.com', '*.example.com'])
+    end
+
+    it 'set multiple domain with file' do
+      setting.domain_blacklist_file = File.open(Rails.root.join('spec/fixtures/', 'blacklist.txt'))
+      expect(setting.domain_blacklist).to eq(%w(example.com test.com foo.bar))
+    end
+  end
 end
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index fc74488ac0e..79f77d116a7 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -89,7 +89,7 @@ describe User, models: true do
     end
 
     describe 'email' do
-      context 'when no signup domains listed' do
+      context 'when no signup domains white listed' do
         before do
           allow_any_instance_of(ApplicationSetting).to receive(:restricted_signup_domains).and_return([])
         end
@@ -100,7 +100,7 @@ describe User, models: true do
         end
       end
 
-      context 'when a signup domain is listed and subdomains are allowed' do
+      context 'when a signup domain is white listed and subdomains are allowed' do
         before do
           allow_any_instance_of(ApplicationSetting).to receive(:restricted_signup_domains).and_return(['example.com', '*.example.com'])
         end
@@ -121,7 +121,7 @@ describe User, models: true do
         end
       end
 
-      context 'when a signup domain is listed and subdomains are not allowed' do
+      context 'when a signup domain is white listed and subdomains are not allowed' do
         before do
           allow_any_instance_of(ApplicationSetting).to receive(:restricted_signup_domains).and_return(['example.com'])
         end
@@ -142,6 +142,53 @@ describe User, models: true do
         end
       end
 
+      context 'domain blacklist' do
+        before do
+          allow_any_instance_of(ApplicationSetting).to receive(:domain_blacklist_enabled?).and_return(true)
+          allow_any_instance_of(ApplicationSetting).to receive(:domain_blacklist).and_return(['example.com'])
+        end
+
+        context 'when a signup domain is black listed' do
+          it 'accepts info@test.com' do
+            user = build(:user, email: 'info@test.com')
+            expect(user).to be_valid
+          end
+
+          it 'rejects info@example.com' do
+            user = build(:user, email: 'info@example.com')
+            expect(user).not_to be_valid
+          end
+        end
+
+        context 'when a signup domain is black listed but a wildcard subdomain is allowed' do
+          before do
+            allow_any_instance_of(ApplicationSetting).to receive(:domain_blacklist).and_return(['test.example.com'])
+            allow_any_instance_of(ApplicationSetting).to receive(:restricted_signup_domains).and_return(['*.example.com'])
+          end
+
+          it 'should give priority to whitelist and allow info@test.example.com' do
+            user = build(:user, email: 'info@test.example.com')
+            expect(user).to be_valid
+          end
+        end
+
+        context 'with both lists containing a domain' do
+          before do
+            allow_any_instance_of(ApplicationSetting).to receive(:restricted_signup_domains).and_return(['test.com'])
+          end
+
+          it 'accepts info@test.com' do
+            user = build(:user, email: 'info@test.com')
+            expect(user).to be_valid
+          end
+
+          it 'rejects info@example.com' do
+            user = build(:user, email: 'info@example.com')
+            expect(user).not_to be_valid
+          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")