From eb4f1eb5f55fe3630c9191db1b9da2dc92437391 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Sat, 2 May 2015 06:53:32 -0700 Subject: [PATCH] Add application setting to restrict user signups to e-mail domains This feature was requested long ago: http://feedback.gitlab.com/forums/176466-general/suggestions/4118466-ability-to-register-only-from-ceratain-domains This MR is based off !253 but changed to use application settings and use wildcard strings to give more flexibility in pattern matching. Regexps seemed overkill and easy to get wrong. Only restrict e-mail addresses upon creation --- CHANGELOG | 1 + .../admin/application_settings_controller.rb | 3 +- app/models/application_setting.rb | 24 +++++++++- app/models/user.rb | 24 ++++++++++ .../application_settings/_form.html.haml | 5 ++ config/initializers/1_settings.rb | 1 + ..._signup_domains_to_application_settings.rb | 5 ++ db/schema.rb | 3 +- spec/models/application_setting_spec.rb | 24 ++++++++++ spec/models/user_spec.rb | 47 +++++++++++++++++++ 10 files changed, 133 insertions(+), 4 deletions(-) create mode 100644 db/migrate/20150502064022_add_restricted_signup_domains_to_application_settings.rb diff --git a/CHANGELOG b/CHANGELOG index 2c5c9c76cc..dc3a670922 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,6 +1,7 @@ Please view this file on the master branch, on stable branches it's out of date. v 7.11.0 (unreleased) + - Add application setting to restrict user signups to e-mail domains (Stan Hu) - Don't allow a merge request to be merged when its title starts with "WIP". - Add a page title to every page. - Get Gitorious importer to work again. diff --git a/app/controllers/admin/application_settings_controller.rb b/app/controllers/admin/application_settings_controller.rb index 8f6a766635..3975e30835 100644 --- a/app/controllers/admin/application_settings_controller.rb +++ b/app/controllers/admin/application_settings_controller.rb @@ -41,7 +41,8 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController :max_attachment_size, :default_project_visibility, :default_snippet_visibility, - restricted_visibility_levels: [] + :restricted_signup_domains_raw, + restricted_visibility_levels: [], ) end end diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index 9406fb9193..f2cebde970 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -18,11 +18,13 @@ # restricted_visibility_levels :text # max_attachment_size :integer default(10) # default_project_visibility :integer -# default_snippet_visibility :integer +# restricted_signup_domains :text # class ApplicationSetting < ActiveRecord::Base serialize :restricted_visibility_levels + serialize :restricted_signup_domains, Array + attr_accessor :restricted_signup_domains_raw validates :home_page_url, allow_blank: true, @@ -55,11 +57,29 @@ class ApplicationSetting < ActiveRecord::Base restricted_visibility_levels: Settings.gitlab['restricted_visibility_levels'], max_attachment_size: Settings.gitlab['max_attachment_size'], default_project_visibility: Settings.gitlab.default_projects_features['visibility_level'], - default_snippet_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'] ) end def home_page_url_column_exist 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? + 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? } + end + end diff --git a/app/models/user.rb b/app/models/user.rb index d6b93afe73..f22fdc2843 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -142,6 +142,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 :sanitize_attrs before_validation :set_notification_email, if: ->(user) { user.email_changed? } before_validation :set_public_email, if: ->(user) { user.public_email_changed? } @@ -611,4 +612,27 @@ class User < ActiveRecord::Base select(:project_id). uniq.map(&:project_id) 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 end diff --git a/app/views/admin/application_settings/_form.html.haml b/app/views/admin/application_settings/_form.html.haml index 87e7c9634e..f6eb00ea0b 100644 --- a/app/views/admin/application_settings/_form.html.haml +++ b/app/views/admin/application_settings/_form.html.haml @@ -72,6 +72,11 @@ = f.label :max_attachment_size, 'Maximum attachment size (MB)', class: 'control-label col-sm-2' .col-sm-10 = f.number_field :max_attachment_size, class: 'form-control' + .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 Ex: domain.com, *.domain.com. Wildcards allowed. Use separate lines for multiple entries. .form-actions = f.submit 'Save', class: 'btn btn-primary' diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index 0abd34fc3e..e5ac66a232 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -132,6 +132,7 @@ Settings.gitlab.default_projects_features['wiki'] = true if Settings.g Settings.gitlab.default_projects_features['snippets'] = false if Settings.gitlab.default_projects_features['snippets'].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.absolute_path(Settings.gitlab['repository_downloads_path'] || 'tmp/repositories', Rails.root) +Settings.gitlab['restricted_signup_domains'] ||= [] # # Gravatar diff --git a/db/migrate/20150502064022_add_restricted_signup_domains_to_application_settings.rb b/db/migrate/20150502064022_add_restricted_signup_domains_to_application_settings.rb new file mode 100644 index 0000000000..184e265361 --- /dev/null +++ b/db/migrate/20150502064022_add_restricted_signup_domains_to_application_settings.rb @@ -0,0 +1,5 @@ +class AddRestrictedSignupDomainsToApplicationSettings < ActiveRecord::Migration + def change + add_column :application_settings, :restricted_signup_domains, :text + end +end diff --git a/db/schema.rb b/db/schema.rb index 02882186fc..f0cccb8b74 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20150429002313) do +ActiveRecord::Schema.define(version: 20150502064022) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -31,6 +31,7 @@ ActiveRecord::Schema.define(version: 20150429002313) 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" end create_table "broadcast_messages", force: true do |t| diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index b4f0b2c201..876502d03c 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -21,4 +21,28 @@ require 'spec_helper' describe ApplicationSetting, models: true do it { expect(ApplicationSetting.create_from_defaults).to be_valid } + + context 'restricted signup domains' do + let(:setting) { ApplicationSetting.create_from_defaults } + + it 'set single domain' do + setting.restricted_signup_domains_raw = 'example.com' + expect(setting.restricted_signup_domains).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']) + 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']) + 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']) + end + end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 24384e8bf2..441aa79313 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -54,6 +54,8 @@ require 'spec_helper' describe User do + include Gitlab::CurrentSettings + describe "Associations" do it { is_expected.to have_one(:namespace) } it { is_expected.to have_many(:snippets).class_name('Snippet').dependent(:destroy) } @@ -112,6 +114,51 @@ describe User do user = build(:user, email: "lol!'+=?><#$%^&*()@gmail.com") expect(user).to be_invalid end + + context 'when no signup domains listed' do + before { allow(current_application_settings).to receive(:restricted_signup_domains).and_return([]) } + it 'accepts any email' do + user = build(:user, email: "info@example.com") + expect(user).to be_valid + end + end + + context 'when a signup domain is listed and subdomains are allowed' do + before { allow(current_application_settings).to receive(:restricted_signup_domains).and_return(['example.com', '*.example.com']) } + it 'accepts info@example.com' do + user = build(:user, email: "info@example.com") + expect(user).to be_valid + end + + it 'accepts info@test.example.com' do + user = build(:user, email: "info@test.example.com") + expect(user).to be_valid + end + + it 'rejects example@test.com' do + user = build(:user, email: "example@test.com") + expect(user).to be_invalid + end + end + + context 'when a signup domain is listed and subdomains are not allowed' do + before { allow(current_application_settings).to receive(:restricted_signup_domains).and_return(['example.com']) } + + it 'accepts info@example.com' do + user = build(:user, email: "info@example.com") + expect(user).to be_valid + end + + it 'rejects info@test.example.com' do + user = build(:user, email: "info@test.example.com") + expect(user).to be_invalid + end + + it 'rejects example@test.com' do + user = build(:user, email: "example@test.com") + expect(user).to be_invalid + end + end end end