From c71e658ccac85f111517e04b79d915c10867c7e3 Mon Sep 17 00:00:00 2001
From: Patricio Cano <suprnova32@gmail.com>
Date: Fri, 15 Jul 2016 18:30:38 -0500
Subject: [PATCH] Refactor and rename `restricted_signup_domains` to
`domain_whitelist` to better conform to its behavior and newly introduced
behavior.
---
.../admin/application_settings_controller.rb | 2 +-
app/models/application_setting.rb | 20 +++++++-------
app/models/user.rb | 2 +-
.../application_settings/_form.html.haml | 26 +++----------------
config/initializers/1_settings.rb | 2 +-
...tion_settings_restricted_signup_domains.rb | 21 +++++++++++++++
db/schema.rb | 2 +-
doc/api/settings.md | 6 ++---
doc/development/doc_styleguide.md | 2 +-
lib/api/entities.rb | 2 +-
lib/gitlab/current_settings.rb | 2 +-
spec/models/application_setting_spec.rb | 16 ++++++------
spec/models/user_spec.rb | 10 +++----
13 files changed, 58 insertions(+), 55 deletions(-)
create mode 100644 db/migrate/20160715230841_rename_application_settings_restricted_signup_domains.rb
diff --git a/app/controllers/admin/application_settings_controller.rb b/app/controllers/admin/application_settings_controller.rb
index 3e27320ee5c..c5b44ff8c44 100644
--- a/app/controllers/admin/application_settings_controller.rb
+++ b/app/controllers/admin/application_settings_controller.rb
@@ -84,7 +84,7 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController
:default_project_visibility,
:default_snippet_visibility,
:default_group_visibility,
- :restricted_signup_domains_raw,
+ :domain_whitelist_raw,
:version_check_enabled,
:admin_notification_email,
:user_oauth_applications,
diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb
index 03c2bc0b57d..d923b4d5235 100644
--- a/app/models/application_setting.rb
+++ b/app/models/application_setting.rb
@@ -14,10 +14,10 @@ class ApplicationSetting < ActiveRecord::Base
serialize :restricted_visibility_levels
serialize :import_sources
serialize :disabled_oauth_sign_in_sources, Array
- serialize :restricted_signup_domains, Array
+ serialize :domain_whitelist, Array
serialize :domain_blacklist, Array
- attr_accessor :restricted_signup_domains_raw, :domain_blacklist_raw
+ attr_accessor :domain_whitelist_raw, :domain_blacklist_raw
validates :session_expire_delay,
presence: true,
@@ -141,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'],
@@ -162,19 +162,19 @@ 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 restricted_signup_domains_raw=(values)
- self.restricted_signup_domains = []
- self.restricted_signup_domains = values.split(DOMAIN_LIST_SEPARATOR)
- self.restricted_signup_domains.reject! { |d| d.empty? }
- self.restricted_signup_domains
+ 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)
diff --git a/app/models/user.rb b/app/models/user.rb
index 0168008355b..6932e750fe0 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -871,7 +871,7 @@ class User < ActiveRecord::Base
end
end
- allowed_domains = current_application_settings.restricted_signup_domains
+ allowed_domains = current_application_settings.domain_whitelist
unless allowed_domains.blank?
if match_domain(allowed_domains, self.email)
valid = true
diff --git a/app/views/admin/application_settings/_form.html.haml b/app/views/admin/application_settings/_form.html.haml
index 9443fe5e1d3..35fea2d8fa9 100644
--- a/app/views/admin/application_settings/_form.html.haml
+++ b/app/views/admin/application_settings/_form.html.haml
@@ -123,9 +123,9 @@
= 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'
+ = f.label :domain_whitelist, 'Whitelisted 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'
+ = f.text_area :domain_whitelist_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'
@@ -152,7 +152,7 @@
= 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'
+ = 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: 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
@@ -385,22 +385,4 @@
.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
+ = 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/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 25d94f283c9..3d769ccac50 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -49,7 +49,7 @@ ActiveRecord::Schema.define(version: 20160716115710) 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
diff --git a/doc/api/settings.md b/doc/api/settings.md
index d9b68eaeadf..c925fa1861e 100644
--- a/doc/api/settings.md
+++ b/doc/api/settings.md
@@ -33,7 +33,7 @@ Example response:
"session_expire_delay" : 10080,
"home_page_url" : null,
"default_snippet_visibility" : 0,
- "restricted_signup_domains" : [],
+ "domain_whitelist" : [],
"created_at" : "2016-01-04T15:44:55.176Z",
"default_project_visibility" : 0,
"gravatar_enabled" : true,
@@ -63,7 +63,7 @@ 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. |
| `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 +93,7 @@ Example response:
"session_expire_delay": 10080,
"default_project_visibility": 1,
"default_snippet_visibility": 0,
- "restricted_signup_domains": [],
+ "domain_whitelist": [],
"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 4cd388658be..ec9a56afde8 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -412,7 +412,7 @@ 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
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/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb
index ebcd9b0f947..a780c04abde 100644
--- a/spec/models/application_setting_spec.rb
+++ b/spec/models/application_setting_spec.rb
@@ -54,23 +54,23 @@ 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
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 5f130234df1..41e531c684b 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -91,7 +91,7 @@ describe User, models: true do
describe 'email' 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
@@ -102,7 +102,7 @@ describe User, models: true 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
@@ -123,7 +123,7 @@ describe User, models: true 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
@@ -163,7 +163,7 @@ describe User, models: true do
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'])
+ 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
@@ -174,7 +174,7 @@ describe User, models: true do
context 'with both lists containing a domain' do
before do
- allow_any_instance_of(ApplicationSetting).to receive(:restricted_signup_domains).and_return(['test.com'])
+ allow_any_instance_of(ApplicationSetting).to receive(:domain_whitelist).and_return(['test.com'])
end
it 'accepts info@test.com' do
--
GitLab