diff options
author | Robert Speicher <robert@gitlab.com> | 2016-07-20 20:26:00 +0000 |
---|---|---|
committer | Robert Speicher <robert@gitlab.com> | 2016-07-20 20:26:00 +0000 |
commit | 22c8e21bf432a68f05bd81685d76acc0a3c9607f (patch) | |
tree | 3d113695853db1348f31d1fbc1fbe17590aecefa | |
parent | 5a77eb153669bfbac4ab1f05615d11965beb826d (diff) | |
parent | 1a057d2b4013feb151e77aa71dd0b5a83412d2d6 (diff) | |
download | gitlab-ce-22c8e21bf432a68f05bd81685d76acc0a3c9607f.tar.gz |
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
-rw-r--r-- | CHANGELOG | 1 | ||||
-rw-r--r-- | app/assets/javascripts/admin.js.coffee | 11 | ||||
-rw-r--r-- | app/controllers/admin/application_settings_controller.rb | 6 | ||||
-rw-r--r-- | app/models/application_setting.rb | 52 | ||||
-rw-r--r-- | app/models/user.rb | 61 | ||||
-rw-r--r-- | app/views/admin/application_settings/_form.html.haml | 57 | ||||
-rw-r--r-- | config/initializers/1_settings.rb | 2 | ||||
-rw-r--r-- | db/migrate/20160713205315_add_domain_blacklist_to_application_settings.rb | 22 | ||||
-rw-r--r-- | db/migrate/20160715230841_rename_application_settings_restricted_signup_domains.rb | 21 | ||||
-rw-r--r-- | db/schema.rb | 4 | ||||
-rw-r--r-- | doc/administration/access_restrictions.md | 22 | ||||
-rw-r--r-- | doc/administration/img/domain_blacklist.png | bin | 0 -> 178444 bytes | |||
-rw-r--r-- | doc/api/settings.md | 12 | ||||
-rw-r--r-- | doc/development/doc_styleguide.md | 2 | ||||
-rw-r--r-- | lib/api/entities.rb | 4 | ||||
-rw-r--r-- | lib/gitlab/current_settings.rb | 2 | ||||
-rw-r--r-- | spec/fixtures/domain_blacklist.txt | 3 | ||||
-rw-r--r-- | spec/models/application_setting_spec.rb | 53 | ||||
-rw-r--r-- | spec/models/user_spec.rb | 59 |
19 files changed, 318 insertions, 76 deletions
diff --git a/CHANGELOG b/CHANGELOG index d5e66840f23..dfee9cc20bb 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -29,6 +29,7 @@ v 8.10.0 (unreleased) - Escape file extension when parsing search results !5141 (winniehell) - Add "passing with warnings" to the merge request pipeline possible statuses, this happens when builds that allow failures have failed. !5004 - Apply the trusted_proxies config to the rack request object for use with rack_attack + - Added the ability to block sign ups using a domain blacklist !5259 - Upgrade to Rails 4.2.7. !5236 - Extend exposed environment variables for CI builds - Deprecate APIs "projects/:id/keys/...". Use "projects/:id/deploy_keys/..." instead diff --git a/app/assets/javascripts/admin.js.coffee b/app/assets/javascripts/admin.js.coffee index b2b8e1b7ffb..90c09619f8c 100644 --- a/app/assets/javascripts/admin.js.coffee +++ b/app/assets/javascripts/admin.js.coffee @@ -38,3 +38,14 @@ class @Admin $('li.group_member').bind 'ajax:success', -> Turbolinks.visit(location.href) + + 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() diff --git a/app/controllers/admin/application_settings_controller.rb b/app/controllers/admin/application_settings_controller.rb index 23ba83aba0e..9e1dc15de84 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, @@ -83,7 +84,10 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController :default_project_visibility, :default_snippet_visibility, :default_group_visibility, - :restricted_signup_domains_raw, + :domain_whitelist_raw, + :domain_blacklist_enabled, + :domain_blacklist_raw, + :domain_blacklist_file, :version_check_enabled, :admin_notification_email, :user_oauth_applications, 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 diff --git a/app/views/admin/application_settings/_form.html.haml b/app/views/admin/application_settings/_form.html.haml index 538d8176ce7..23b52d08df7 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 :domain_whitelist, 'Whitelisted domains for sign-ups', class: 'control-label col-sm-2' + .col-sm-10 + = f.text_area :domain_whitelist_raw, placeholder: 'domain.com', class: 'form-control', rows: 8 + .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 + .option-title + Upload blacklist file + .radio + = label_tag :blacklist_type_raw do + = radio_button_tag :blacklist_type, :raw, @application_setting.domain_blacklist.present? || @application_setting.domain_blacklist.blank? + .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 for sign-ups', class: 'control-label col-sm-2' + .col-sm-10 + = f.text_area :domain_blacklist_raw, placeholder: 'domain.com', class: 'form-control', rows: 8 + .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 @@ -352,4 +385,4 @@ .form-actions - = f.submit 'Save', class: 'btn btn-save' + = f.submit 'Save', class: 'btn btn-save'
\ No newline at end of file diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index 51d93e8cde0..693507e0bec 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -212,7 +212,7 @@ Settings.gitlab.default_projects_features['builds'] = true if Settin Settings.gitlab.default_projects_features['container_registry'] = true if Settings.gitlab.default_projects_features['container_registry'].nil? Settings.gitlab.default_projects_features['visibility_level'] = Settings.send(:verify_constant, Gitlab::VisibilityLevel, Settings.gitlab.default_projects_features['visibility_level'], Gitlab::VisibilityLevel::PRIVATE) Settings.gitlab['repository_downloads_path'] = File.join(Settings.shared['path'], 'cache/archive') if Settings.gitlab['repository_downloads_path'].nil? -Settings.gitlab['restricted_signup_domains'] ||= [] +Settings.gitlab['domain_whitelist'] ||= [] Settings.gitlab['import_sources'] ||= %w[github bitbucket gitlab gitorious google_code fogbugz git gitlab_project] Settings.gitlab['trusted_proxies'] ||= [] 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/migrate/20160715230841_rename_application_settings_restricted_signup_domains.rb b/db/migrate/20160715230841_rename_application_settings_restricted_signup_domains.rb new file mode 100644 index 00000000000..dd15704800a --- /dev/null +++ b/db/migrate/20160715230841_rename_application_settings_restricted_signup_domains.rb @@ -0,0 +1,21 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class RenameApplicationSettingsRestrictedSignupDomains < 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 + rename_column :application_settings, :restricted_signup_domains, :domain_whitelist + end +end diff --git a/db/schema.rb b/db/schema.rb index 72780fb8d03..c7876426424 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -49,7 +49,7 @@ ActiveRecord::Schema.define(version: 20160718153603) do t.integer "max_attachment_size", default: 10, null: false t.integer "default_project_visibility" t.integer "default_snippet_visibility" - t.text "restricted_signup_domains" + t.text "domain_whitelist" t.boolean "user_oauth_applications", default: true t.string "after_sign_out_path" t.integer "session_expire_delay", default: 10080, null: false @@ -88,6 +88,8 @@ ActiveRecord::Schema.define(version: 20160718153603) 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/doc/administration/access_restrictions.md b/doc/administration/access_restrictions.md index 51d7996effd..eb08cf139d4 100644 --- a/doc/administration/access_restrictions.md +++ b/doc/administration/access_restrictions.md @@ -1,6 +1,6 @@ # Access Restrictions -> **Note:** This feature is only available on versions 8.10 and above. +> **Note:** These features are only available on versions 8.10 and above. With GitLab's Access restrictions you can choose which Git access protocols you want your users to use to communicate with GitLab. This feature can be enabled @@ -35,4 +35,22 @@ not selected. > **Note:** Please keep in mind that disabling an access protocol does not actually block access to the server itself. The ports used for the protocol, be it SSH or HTTP, will still be accessible. What GitLab does is restrict access on the - application level.
\ No newline at end of file + application level. + +## Blacklist email domains + +With this feature enabled, you can block email addresses of a specific domain +from creating an account on your GitLab server. This is particularly useful to +prevent spam. Disposable email addresses are usually used by malicious users to +create dummy accounts and spam issues. + +This feature can be activated via the `Application Settings` in the Admin area, +and you have the option of entering the list manually, or uploading a file with +the list. + +The blacklist accepts wildcards, so you can use `*.test.com` to block every +`test.com` subdomain, or `*.io` to block all domains ending in `.io`. Domains +should be separated by a whitespace, semicolon, comma, or a new line. + +![Domain Blacklist](img/domain_blacklist.png) + diff --git a/doc/administration/img/domain_blacklist.png b/doc/administration/img/domain_blacklist.png Binary files differnew file mode 100644 index 00000000000..a7894e5f08d --- /dev/null +++ b/doc/administration/img/domain_blacklist.png diff --git a/doc/api/settings.md b/doc/api/settings.md index d9b68eaeadf..ea39b32561c 100644 --- a/doc/api/settings.md +++ b/doc/api/settings.md @@ -33,7 +33,9 @@ Example response: "session_expire_delay" : 10080, "home_page_url" : null, "default_snippet_visibility" : 0, - "restricted_signup_domains" : [], + "domain_whitelist" : [], + "domain_blacklist_enabled" : false, + "domain_blacklist" : [], "created_at" : "2016-01-04T15:44:55.176Z", "default_project_visibility" : 0, "gravatar_enabled" : true, @@ -63,7 +65,9 @@ PUT /application/settings | `session_expire_delay` | integer | no | Session duration in minutes. GitLab restart is required to apply changes | | `default_project_visibility` | integer | no | What visibility level new projects receive. Can take `0` _(Private)_, `1` _(Internal)_ and `2` _(Public)_ as a parameter. Default is `0`.| | `default_snippet_visibility` | integer | no | What visibility level new snippets receive. Can take `0` _(Private)_, `1` _(Internal)_ and `2` _(Public)_ as a parameter. Default is `0`.| -| `restricted_signup_domains` | array of strings | no | Force people to use only corporate emails for sign-up. Default is null, meaning there is no restriction. | +| `domain_whitelist` | array of strings | no | Force people to use only corporate emails for sign-up. Default is null, meaning there is no restriction. | +| `domain_blacklist_enabled` | boolean | no | Enable/disable the `domain_blacklist` | +| `domain_blacklist` | array of strings | yes (if `domain_whitelist_enabled` is `true` | People trying to sign-up with emails from this domain will not be allowed to do so. | | `user_oauth_applications` | boolean | no | Allow users to register any application to use GitLab as an OAuth provider | | `after_sign_out_path` | string | no | Where to redirect users after logout | | `container_registry_token_expire_delay` | integer | no | Container Registry token duration in minutes | @@ -93,7 +97,9 @@ Example response: "session_expire_delay": 10080, "default_project_visibility": 1, "default_snippet_visibility": 0, - "restricted_signup_domains": [], + "domain_whitelist": [], + "domain_blacklist_enabled" : false, + "domain_blacklist" : [], "user_oauth_applications": true, "after_sign_out_path": "", "container_registry_token_expire_delay": 5, diff --git a/doc/development/doc_styleguide.md b/doc/development/doc_styleguide.md index fac35ec964d..6ee7b3cfeeb 100644 --- a/doc/development/doc_styleguide.md +++ b/doc/development/doc_styleguide.md @@ -359,7 +359,7 @@ restrict the sign-up e-mail domains of a GitLab instance to `*.example.com` and `example.net`, you would do something like this: ```bash -curl -X PUT -H "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" -d "restricted_signup_domains[]=*.example.com" -d "restricted_signup_domains[]=example.net" https://gitlab.example.com/api/v3/application/settings +curl -X PUT -H "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" -d "domain_whitelist[]=*.example.com" -d "domain_whitelist[]=example.net" https://gitlab.example.com/api/v3/application/settings ``` [cURL]: http://curl.haxx.se/ "cURL website" diff --git a/lib/api/entities.rb b/lib/api/entities.rb index d7e74582459..fbf0d74663f 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -414,7 +414,9 @@ module API expose :default_project_visibility expose :default_snippet_visibility expose :default_group_visibility - expose :restricted_signup_domains + expose :domain_whitelist + 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/lib/gitlab/current_settings.rb b/lib/gitlab/current_settings.rb index ffc1814b29d..735331df66c 100644 --- a/lib/gitlab/current_settings.rb +++ b/lib/gitlab/current_settings.rb @@ -39,7 +39,7 @@ module Gitlab 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'], diff --git a/spec/fixtures/domain_blacklist.txt b/spec/fixtures/domain_blacklist.txt new file mode 100644 index 00000000000..baeb11eda9a --- /dev/null +++ b/spec/fixtures/domain_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..fb040ba82bc 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -54,23 +54,60 @@ describe ApplicationSetting, models: true do context 'restricted signup domains' do it 'set single domain' do - setting.restricted_signup_domains_raw = 'example.com' - expect(setting.restricted_signup_domains).to eq(['example.com']) + setting.domain_whitelist_raw = 'example.com' + expect(setting.domain_whitelist).to eq(['example.com']) end it 'set multiple domains with spaces' do - setting.restricted_signup_domains_raw = 'example.com *.example.com' - expect(setting.restricted_signup_domains).to eq(['example.com', '*.example.com']) + setting.domain_whitelist_raw = 'example.com *.example.com' + expect(setting.domain_whitelist).to eq(['example.com', '*.example.com']) end it 'set multiple domains with newlines and a space' do - setting.restricted_signup_domains_raw = "example.com\n *.example.com" - expect(setting.restricted_signup_domains).to eq(['example.com', '*.example.com']) + setting.domain_whitelist_raw = "example.com\n *.example.com" + expect(setting.domain_whitelist).to eq(['example.com', '*.example.com']) end it 'set multiple domains with commas' do - setting.restricted_signup_domains_raw = "example.com, *.example.com" - expect(setting.restricted_signup_domains).to eq(['example.com', '*.example.com']) + setting.domain_whitelist_raw = "example.com, *.example.com" + expect(setting.domain_whitelist).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 contain_exactly('example.com') + end + + it 'set multiple domains with spaces' do + setting.domain_blacklist_raw = 'example.com *.example.com' + expect(setting.domain_blacklist).to contain_exactly('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 contain_exactly('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 contain_exactly('example.com', '*.example.com') + end + + it 'set multiple domains with semicolon' do + setting.domain_blacklist_raw = "example.com; *.example.com" + expect(setting.domain_blacklist).to contain_exactly('example.com', '*.example.com') + end + + it 'set multiple domains with mixture of everything' do + setting.domain_blacklist_raw = "example.com; *.example.com\n test.com\sblock.com yes.com" + expect(setting.domain_blacklist).to contain_exactly('example.com', '*.example.com', 'test.com', 'block.com', 'yes.com') + end + + it 'set multiple domain with file' do + setting.domain_blacklist_file = File.open(Rails.root.join('spec/fixtures/', 'domain_blacklist.txt')) + expect(setting.domain_blacklist).to contain_exactly('example.com', 'test.com', 'foo.bar') end end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 3bf82cf2668..2a5a7fb2fc6 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -89,9 +89,9 @@ describe User, models: true do end describe 'email' do - context 'when no signup domains listed' do + context 'when no signup domains whitelisted' do before do - allow_any_instance_of(ApplicationSetting).to receive(:restricted_signup_domains).and_return([]) + allow_any_instance_of(ApplicationSetting).to receive(:domain_whitelist).and_return([]) end it 'accepts any email' do @@ -100,9 +100,9 @@ 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 whitelisted and subdomains are allowed' do before do - allow_any_instance_of(ApplicationSetting).to receive(:restricted_signup_domains).and_return(['example.com', '*.example.com']) + allow_any_instance_of(ApplicationSetting).to receive(:domain_whitelist).and_return(['example.com', '*.example.com']) end it 'accepts info@example.com' do @@ -121,9 +121,9 @@ 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 whitelisted and subdomains are not allowed' do before do - allow_any_instance_of(ApplicationSetting).to receive(:restricted_signup_domains).and_return(['example.com']) + allow_any_instance_of(ApplicationSetting).to receive(:domain_whitelist).and_return(['example.com']) end it 'accepts info@example.com' do @@ -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 blacklisted' 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 blacklisted 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(:domain_whitelist).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(:domain_whitelist).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") |