Merge branch 'feature/whitelist-new-users-as-internal' into 'master'

Allow whitelisting for "external collaborator by default" setting

See merge request gitlab-org/gitlab-ce!17711

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 }

spec/javascripts/fixtures/admin_users.rb

+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

spec/javascripts/fixtures/application_settings.rb

+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

spec/javascripts/pages/admin/application_settings/account_and_limits_spec.js

+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();
+    });
+  });
+});

spec/javascripts/pages/admin/users/new/index_spec.js

+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();
+    });
+  });
+});

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

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

spec/validators/js_regex_validator_spec.rb

+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