Allow whitelisting for "external collaborator by default" setting

10 files changed, 378 insertions, 0 deletions

diff --git a/spec/features/admin/admin_settings_spec.rb b/spec/features/admin/admin_settings_spec.rb
index af1c153dec8..a3229fe1741 100644
--- a/spec/features/admin/admin_settings_spec.rb
+++ b/spec/features/admin/admin_settings_spec.rb
@@ -78,6 +78,18 @@ describe 'Admin updates settings' do
     expect(page).to have_content "Application settings saved successfully"
   end
 
+  it 'Change New users set to external', :js do
+    user_internal_regex = find('#application_setting_user_default_internal_regex', visible: :all)
+
+    expect(user_internal_regex).to be_readonly
+    expect(user_internal_regex['placeholder']).to eq 'To define internal users, first enable new users set to external'
+
+    check 'application_setting_user_default_external'
+
+    expect(user_internal_regex).not_to be_readonly
+    expect(user_internal_regex['placeholder']).to eq 'Regex pattern'
+  end
+
   it 'Change Sign-in restrictions' do
     page.within('.as-signin') do
       fill_in 'Home page URL', with: 'https://about.gitlab.com/'
diff --git a/spec/features/admin/admin_users_spec.rb b/spec/features/admin/admin_users_spec.rb
index b2eaeb1c487..d32f33ca1e2 100644
--- a/spec/features/admin/admin_users_spec.rb
+++ b/spec/features/admin/admin_users_spec.rb
@@ -125,6 +125,52 @@ describe "Admin::Users" do
         expect(page).to have_content('Username can contain only letters, digits')
       end
     end
+
+    context 'with new users set to external enabled' do
+      context 'with regex to match internal user email address set', :js do
+        before do
+          stub_application_setting(user_default_external: true)
+          stub_application_setting(user_default_internal_regex: '.internal@')
+
+          visit new_admin_user_path
+        end
+
+        def expects_external_to_be_checked
+          expect(find('#user_external')).to be_checked
+        end
+
+        def expects_external_to_be_unchecked
+          expect(find('#user_external')).not_to be_checked
+        end
+
+        def expects_warning_to_be_hidden
+          expect(find('#warning_external_automatically_set', visible: :all)[:class]).to include 'hidden'
+        end
+
+        def expects_warning_to_be_shown
+          expect(find('#warning_external_automatically_set')[:class]).not_to include 'hidden'
+        end
+
+        it 'automatically unchecks external for matching email' do
+          expects_external_to_be_checked
+          expects_warning_to_be_hidden
+
+          fill_in 'user_email', with: 'test.internal@domain.ch'
+
+          expects_external_to_be_unchecked
+          expects_warning_to_be_shown
+
+          fill_in 'user_email', with: 'test@domain.ch'
+
+          expects_external_to_be_checked
+          expects_warning_to_be_hidden
+
+          uncheck 'user_external'
+
+          expects_warning_to_be_hidden
+        end
+      end
+    end
   end
 
   describe "GET /admin/users/:id" do
diff --git a/spec/helpers/users_helper_spec.rb b/spec/helpers/users_helper_spec.rb
index b079802cb81..34d9115a1f6 100644
--- a/spec/helpers/users_helper_spec.rb
+++ b/spec/helpers/users_helper_spec.rb
@@ -42,6 +42,30 @@ describe UsersHelper do
     end
   end
 
+  describe '#user_internal_regex_data' do
+    using RSpec::Parameterized::TableSyntax
+
+    where(:user_default_external, :user_default_internal_regex, :result) do
+      false | nil                | { user_internal_regex_pattern: nil, user_internal_regex_options: nil }
+      false | ''                 | { user_internal_regex_pattern: nil, user_internal_regex_options: nil }
+      false | 'mockRegexPattern' | { user_internal_regex_pattern: nil, user_internal_regex_options: nil }
+      true  | nil                | { user_internal_regex_pattern: nil, user_internal_regex_options: nil }
+      true  | ''                 | { user_internal_regex_pattern: nil, user_internal_regex_options: nil }
+      true  | 'mockRegexPattern' | { user_internal_regex_pattern: 'mockRegexPattern', user_internal_regex_options: 'gi' }
+    end
+
+    with_them do
+      before do
+        stub_application_setting(user_default_external: user_default_external)
+        stub_application_setting(user_default_internal_regex: user_default_internal_regex)
+      end
+
+      subject { helper.user_internal_regex_data }
+
+      it { is_expected.to eq(result) }
+    end
+  end
+
   describe '#current_user_menu_items' do
     subject(:items) { helper.current_user_menu_items }
 
diff --git a/spec/javascripts/fixtures/admin_users.rb b/spec/javascripts/fixtures/admin_users.rb
new file mode 100644
index 00000000000..9989ac4fff2
--- /dev/null
+++ b/spec/javascripts/fixtures/admin_users.rb
@@ -0,0 +1,29 @@
+require 'spec_helper'
+
+describe Admin::UsersController, '(JavaScript fixtures)', type: :controller do
+  include StubENV
+  include JavaScriptFixturesHelpers
+
+  let(:admin) { create(:admin) }
+
+  before do
+    stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false')
+    sign_in(admin)
+  end
+
+  render_views
+
+  before(:all) do
+    clean_frontend_fixtures('admin/users')
+  end
+
+  it 'admin/users/new_with_internal_user_regex.html.raw' do |example|
+    stub_application_setting(user_default_external: true)
+    stub_application_setting(user_default_internal_regex: '^(?:(?!\.ext@).)*$\r?')
+
+    get :new
+
+    expect(response).to be_success
+    store_frontend_fixture(response, example.description)
+  end
+end
diff --git a/spec/javascripts/fixtures/application_settings.rb b/spec/javascripts/fixtures/application_settings.rb
new file mode 100644
index 00000000000..a9d3043f73d
--- /dev/null
+++ b/spec/javascripts/fixtures/application_settings.rb
@@ -0,0 +1,34 @@
+require 'spec_helper'
+
+describe Admin::ApplicationSettingsController, '(JavaScript fixtures)', type: :controller do
+  include StubENV
+  include JavaScriptFixturesHelpers
+
+  let(:admin) { create(:admin) }
+  let(:namespace) { create(:namespace, name: 'frontend-fixtures' )}
+  let(:project) { create(:project_empty_repo, namespace: namespace, path: 'application-settings') }
+
+  before do
+    stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false')
+    sign_in(admin)
+  end
+
+  render_views
+
+  before(:all) do
+    clean_frontend_fixtures('application_settings/')
+  end
+
+  after do
+    remove_repository(project)
+  end
+
+  it 'application_settings/accounts_and_limit.html.raw' do |example|
+    stub_application_setting(user_default_external: false)
+
+    get :show
+
+    expect(response).to be_success
+    store_frontend_fixture(response, example.description)
+  end
+end
diff --git a/spec/javascripts/pages/admin/application_settings/account_and_limits_spec.js b/spec/javascripts/pages/admin/application_settings/account_and_limits_spec.js
new file mode 100644
index 00000000000..4dbfd8f0eaa
--- /dev/null
+++ b/spec/javascripts/pages/admin/application_settings/account_and_limits_spec.js
@@ -0,0 +1,33 @@
+import $ from 'jquery';
+import initUserInternalRegexPlaceholder, { PLACEHOLDER_USER_EXTERNAL_DEFAULT_FALSE,
+  PLACEHOLDER_USER_EXTERNAL_DEFAULT_TRUE } from '~/pages/admin/application_settings/account_and_limits';
+
+describe('AccountAndLimits', () => {
+  const FIXTURE = 'application_settings/accounts_and_limit.html.raw';
+  let $userDefaultExternal;
+  let $userInternalRegex;
+  preloadFixtures(FIXTURE);
+
+  beforeEach(() => {
+    loadFixtures(FIXTURE);
+    initUserInternalRegexPlaceholder();
+    $userDefaultExternal = $('#application_setting_user_default_external');
+    $userInternalRegex = document.querySelector('#application_setting_user_default_internal_regex');
+  });
+
+  describe('Changing of userInternalRegex when userDefaultExternal', () => {
+    it('is unchecked', () => {
+      expect($userDefaultExternal.prop('checked')).toBeFalsy();
+      expect($userInternalRegex.placeholder).toEqual(PLACEHOLDER_USER_EXTERNAL_DEFAULT_FALSE);
+      expect($userInternalRegex.readOnly).toBeTruthy();
+    });
+
+    it('is checked', (done) => {
+      if (!$userDefaultExternal.prop('checked')) $userDefaultExternal.click();
+      expect($userDefaultExternal.prop('checked')).toBeTruthy();
+      expect($userInternalRegex.placeholder).toEqual(PLACEHOLDER_USER_EXTERNAL_DEFAULT_TRUE);
+      expect($userInternalRegex.readOnly).toBeFalsy();
+      done();
+    });
+  });
+});
diff --git a/spec/javascripts/pages/admin/users/new/index_spec.js b/spec/javascripts/pages/admin/users/new/index_spec.js
new file mode 100644
index 00000000000..2bac3951c3a
--- /dev/null
+++ b/spec/javascripts/pages/admin/users/new/index_spec.js
@@ -0,0 +1,43 @@
+import $ from 'jquery';
+import UserInternalRegexHandler from '~/pages/admin/users/new/index';
+
+describe('UserInternalRegexHandler', () => {
+  const FIXTURE = 'admin/users/new_with_internal_user_regex.html.raw';
+  let $userExternal;
+  let $userEmail;
+  let $warningMessage;
+
+  preloadFixtures(FIXTURE);
+
+  beforeEach(() => {
+    loadFixtures(FIXTURE);
+    // eslint-disable-next-line no-new
+    new UserInternalRegexHandler();
+    $userExternal = $('#user_external');
+    $userEmail = $('#user_email');
+    $warningMessage = $('#warning_external_automatically_set');
+    if (!$userExternal.prop('checked')) $userExternal.prop('checked', 'checked');
+  });
+
+  describe('Behaviour of userExternal checkbox when', () => {
+    it('matches email as internal', (done) => {
+      expect($warningMessage.hasClass('hidden')).toBeTruthy();
+
+      $userEmail.val('test@').trigger('input');
+
+      expect($userExternal.prop('checked')).toBeFalsy();
+      expect($warningMessage.hasClass('hidden')).toBeFalsy();
+      done();
+    });
+
+    it('matches email as external', (done) => {
+      expect($warningMessage.hasClass('hidden')).toBeTruthy();
+
+      $userEmail.val('test.ext@').trigger('input');
+
+      expect($userExternal.prop('checked')).toBeTruthy();
+      expect($warningMessage.hasClass('hidden')).toBeTruthy();
+      done();
+    });
+  });
+});
diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb
index 02f74e2ea54..483cc546423 100644
--- a/spec/models/application_setting_spec.rb
+++ b/spec/models/application_setting_spec.rb
@@ -538,4 +538,28 @@ describe ApplicationSetting do
       expect(setting.allow_signup?).to be_falsey
     end
   end
+
+  describe '#user_default_internal_regex_enabled?' do
+    using RSpec::Parameterized::TableSyntax
+
+    where(:user_default_external, :user_default_internal_regex, :result) do
+      false | nil                        | false
+      false | ''                         | false
+      false | '^(?:(?!\.ext@).)*$\r?\n?' | false
+      true  | ''                         | false
+      true  | nil                        | false
+      true  | '^(?:(?!\.ext@).)*$\r?\n?' | true
+    end
+
+    with_them do
+      before do
+        setting.update(user_default_external: user_default_external)
+        setting.update(user_default_internal_regex: user_default_internal_regex)
+      end
+
+      subject { setting.user_default_internal_regex_enabled? }
+
+      it { is_expected.to eq(result) }
+    end
+  end
 end
diff --git a/spec/services/users/build_service_spec.rb b/spec/services/users/build_service_spec.rb
index 677d4a622e1..b987fe45138 100644
--- a/spec/services/users/build_service_spec.rb
+++ b/spec/services/users/build_service_spec.rb
@@ -13,6 +13,59 @@ describe Users::BuildService do
       it 'returns a valid user' do
         expect(service.execute).to be_valid
       end
+
+      context 'with "user_default_external" application setting' do
+        using RSpec::Parameterized::TableSyntax
+
+        where(:user_default_external, :external, :email, :user_default_internal_regex, :result) do
+          true  | nil   | 'fl@example.com'        | nil                     | true
+          true  | true  | 'fl@example.com'        | nil                     | true
+          true  | false | 'fl@example.com'        | nil                     | false
+
+          true  | nil   | 'fl@example.com'        | ''                      | true
+          true  | true  | 'fl@example.com'        | ''                      | true
+          true  | false | 'fl@example.com'        | ''                      | false
+
+          true  | nil   | 'fl@example.com'        | '^(?:(?!\.ext@).)*$\r?' | false
+          true  | true  | 'fl@example.com'        | '^(?:(?!\.ext@).)*$\r?' | true
+          true  | false | 'fl@example.com'        | '^(?:(?!\.ext@).)*$\r?' | false
+
+          true  | nil   | 'tester.ext@domain.com' | '^(?:(?!\.ext@).)*$\r?' | true
+          true  | true  | 'tester.ext@domain.com' | '^(?:(?!\.ext@).)*$\r?' | true
+          true  | false | 'tester.ext@domain.com' | '^(?:(?!\.ext@).)*$\r?' | false
+
+          false | nil   | 'fl@example.com'        | nil                     | false
+          false | true  | 'fl@example.com'        | nil                     | true
+          false | false | 'fl@example.com'        | nil                     | false
+
+          false | nil   | 'fl@example.com'        | ''                      | false
+          false | true  | 'fl@example.com'        | ''                      | true
+          false | false | 'fl@example.com'        | ''                      | false
+
+          false | nil   | 'fl@example.com'        | '^(?:(?!\.ext@).)*$\r?' | false
+          false | true  | 'fl@example.com'        | '^(?:(?!\.ext@).)*$\r?' | true
+          false | false | 'fl@example.com'        | '^(?:(?!\.ext@).)*$\r?' | false
+
+          false | nil   | 'tester.ext@domain.com' | '^(?:(?!\.ext@).)*$\r?' | false
+          false | true  | 'tester.ext@domain.com' | '^(?:(?!\.ext@).)*$\r?' | true
+          false | false | 'tester.ext@domain.com' | '^(?:(?!\.ext@).)*$\r?' | false
+        end
+
+        with_them do
+          before do
+            stub_application_setting(user_default_external: user_default_external)
+            stub_application_setting(user_default_internal_regex: user_default_internal_regex)
+
+            params.merge!({ external: external, email: email }.compact)
+          end
+
+          subject(:user) { service.execute }
+
+          it 'correctly sets user.external' do
+            expect(user.external).to eq(result)
+          end
+        end
+      end
     end
 
     context 'with non admin user' do
@@ -50,6 +103,59 @@ describe Users::BuildService do
           expect(service.execute).to be_confirmed
         end
       end
+
+      context 'with "user_default_external" application setting' do
+        using RSpec::Parameterized::TableSyntax
+
+        where(:user_default_external, :external, :email, :user_default_internal_regex, :result) do
+          true  | nil   | 'fl@example.com'        | nil                     | true
+          true  | true  | 'fl@example.com'        | nil                     | true
+          true  | false | 'fl@example.com'        | nil                     | true
+
+          true  | nil   | 'fl@example.com'        | ''                      | true
+          true  | true  | 'fl@example.com'        | ''                      | true
+          true  | false | 'fl@example.com'        | ''                      | true
+
+          true  | nil   | 'fl@example.com'        | '^(?:(?!\.ext@).)*$\r?' | true
+          true  | true  | 'fl@example.com'        | '^(?:(?!\.ext@).)*$\r?' | true
+          true  | false | 'fl@example.com'        | '^(?:(?!\.ext@).)*$\r?' | true
+
+          true  | nil   | 'tester.ext@domain.com' | '^(?:(?!\.ext@).)*$\r?' | true
+          true  | true  | 'tester.ext@domain.com' | '^(?:(?!\.ext@).)*$\r?' | true
+          true  | false | 'tester.ext@domain.com' | '^(?:(?!\.ext@).)*$\r?' | true
+
+          false | nil   | 'fl@example.com'        | nil                     | false
+          false | true  | 'fl@example.com'        | nil                     | false
+          false | false | 'fl@example.com'        | nil                     | false
+
+          false | nil   | 'fl@example.com'        | ''                      | false
+          false | true  | 'fl@example.com'        | ''                      | false
+          false | false | 'fl@example.com'        | ''                      | false
+
+          false | nil   | 'fl@example.com'        | '^(?:(?!\.ext@).)*$\r?' | false
+          false | true  | 'fl@example.com'        | '^(?:(?!\.ext@).)*$\r?' | false
+          false | false | 'fl@example.com'        | '^(?:(?!\.ext@).)*$\r?' | false
+
+          false | nil   | 'tester.ext@domain.com' | '^(?:(?!\.ext@).)*$\r?' | false
+          false | true  | 'tester.ext@domain.com' | '^(?:(?!\.ext@).)*$\r?' | false
+          false | false | 'tester.ext@domain.com' | '^(?:(?!\.ext@).)*$\r?' | false
+        end
+
+        with_them do
+          before do
+            stub_application_setting(user_default_external: user_default_external)
+            stub_application_setting(user_default_internal_regex: user_default_internal_regex)
+
+            params.merge!({ external: external, email: email }.compact)
+          end
+
+          subject(:user) { service.execute }
+
+          it 'sets the value of Gitlab::CurrentSettings.user_default_external' do
+            expect(user.external).to eq(result)
+          end
+        end
+      end
     end
   end
 end
diff --git a/spec/validators/js_regex_validator_spec.rb b/spec/validators/js_regex_validator_spec.rb
new file mode 100644
index 00000000000..aeb55cdc0e5
--- /dev/null
+++ b/spec/validators/js_regex_validator_spec.rb
@@ -0,0 +1,27 @@
+require 'spec_helper'
+
+describe JsRegexValidator do
+  describe '#validates_each' do
+    using RSpec::Parameterized::TableSyntax
+
+    let(:validator) { described_class.new(attributes: [:user_default_internal_regex]) }
+    let(:application_setting) { build(:application_setting, user_default_external: true) }
+
+    where(:user_default_internal_regex, :result) do
+      nil            | []
+      ''             | []
+      '(?#comment)'  | ['Regex Pattern (?#comment) can not be expressed in Javascript']
+      '(?(a)b|c)'    | ['invalid conditional pattern: /(?(a)b|c)/i']
+      '[a-z&&[^uo]]' | ["Dropped unsupported set intersection '[a-z&&[^uo]]' at index 0",
+                        "Dropped unsupported nested negative set data '[^uo]' at index 6"]
+    end
+
+    with_them do
+      it 'generates correct errors' do
+        validator.validate_each(application_setting, :user_default_internal_regex, user_default_internal_regex)
+
+        expect(application_setting.errors[:user_default_internal_regex]).to eq result
+      end
+    end
+  end
+end