Commit 93b9bfd9 authored by Roger Rüttimann's avatar Roger Rüttimann Committed by Phil Hughes

Allow whitelisting for "external collaborator by default" setting

parent 3113fb84
......@@ -195,6 +195,9 @@ gem 're2', '~> 1.1.1'
gem 'version_sorter', '~> 2.1.0'
# Export Ruby Regex to Javascript
gem 'js_regex', '~> 2.2.1'
# User agent parsing
gem 'device_detector'
......
......@@ -428,6 +428,8 @@ GEM
multipart-post
oauth (~> 0.5, >= 0.5.0)
jquery-atwho-rails (1.3.2)
js_regex (2.2.1)
regexp_parser (>= 0.4.11, <= 0.5.0)
json (1.8.6)
json-jwt (1.9.4)
activesupport
......@@ -726,6 +728,7 @@ GEM
redis-store (>= 1.2, < 2)
redis-store (1.4.1)
redis (>= 2.2, < 5)
regexp_parser (0.5.0)
representable (3.0.4)
declarative (< 0.1.0)
declarative-option (< 0.2.0)
......@@ -1074,6 +1077,7 @@ DEPENDENCIES
influxdb (~> 0.2)
jira-ruby (~> 1.4)
jquery-atwho-rails (~> 1.3.2)
js_regex (~> 2.2.1)
json-schema (~> 2.8.0)
jwt (~> 1.5.6)
kaminari (~> 1.0)
......
......@@ -431,6 +431,8 @@ GEM
multipart-post
oauth (~> 0.5, >= 0.5.0)
jquery-atwho-rails (1.3.2)
js_regex (2.2.1)
regexp_parser (>= 0.4.11, <= 0.5.0)
json (1.8.6)
json-jwt (1.9.4)
activesupport
......@@ -735,6 +737,7 @@ GEM
redis-store (>= 1.2, < 2)
redis-store (1.4.1)
redis (>= 2.2, < 5)
regexp_parser (0.5.0)
representable (3.0.4)
declarative (< 0.1.0)
declarative-option (< 0.2.0)
......@@ -1084,6 +1087,7 @@ DEPENDENCIES
influxdb (~> 0.2)
jira-ruby (~> 1.4)
jquery-atwho-rails (~> 1.3.2)
js_regex (~> 2.2.1)
json-schema (~> 2.8.0)
jwt (~> 1.5.6)
kaminari (~> 1.0)
......
import { __ } from '~/locale';
export const PLACEHOLDER_USER_EXTERNAL_DEFAULT_TRUE = __('Regex pattern');
export const PLACEHOLDER_USER_EXTERNAL_DEFAULT_FALSE = __('To define internal users, first enable new users set to external');
function setUserInternalRegexPlaceholder(checkbox) {
const userInternalRegex = document.getElementById('application_setting_user_default_internal_regex');
if (checkbox && userInternalRegex) {
if (checkbox.checked) {
userInternalRegex.readOnly = false;
userInternalRegex.placeholder = PLACEHOLDER_USER_EXTERNAL_DEFAULT_TRUE;
} else {
userInternalRegex.readOnly = true;
userInternalRegex.placeholder = PLACEHOLDER_USER_EXTERNAL_DEFAULT_FALSE;
}
}
}
export default function initUserInternalRegexPlaceholder() {
const checkbox = document.getElementById('application_setting_user_default_external');
setUserInternalRegexPlaceholder(checkbox);
checkbox.addEventListener('change', () => {
setUserInternalRegexPlaceholder(checkbox);
});
}
import initAdmin from './admin';
import initUserInternalRegexPlaceholder from './application_settings/account_and_limits';
document.addEventListener('DOMContentLoaded', initAdmin);
document.addEventListener('DOMContentLoaded', () => {
initAdmin();
initUserInternalRegexPlaceholder();
});
import $ from 'jquery';
export default class UserInternalRegexHandler {
constructor() {
this.regexPattern = $('[data-user-internal-regex-pattern]').data('user-internal-regex-pattern');
if (this.regexPattern && this.regexPattern !== '') {
this.regexOptions = $('[data-user-internal-regex-options]').data('user-internal-regex-options');
this.external = $('#user_external');
this.warningMessage = $('#warning_external_automatically_set');
this.addListenerToEmailField();
this.addListenerToUserExternalCheckbox();
}
}
addListenerToEmailField() {
$('#user_email').on('input', (event) => {
this.setExternalCheckbox(event.currentTarget.value);
});
}
addListenerToUserExternalCheckbox() {
this.external.on('click', () => {
this.warningMessage.addClass('hidden');
});
}
isEmailInternal(email) {
const regex = new RegExp(this.regexPattern, this.regexOptions);
return regex.test(email);
}
setExternalCheckbox(email) {
const isChecked = this.external.prop('checked');
if (this.isEmailInternal(email)) {
if (isChecked) {
this.external.prop('checked', false);
this.warningMessage.removeClass('hidden');
}
} else if (!isChecked) {
this.external.prop('checked', true);
this.warningMessage.addClass('hidden');
}
}
}
document.addEventListener('DOMContentLoaded', () => {
// eslint-disable-next-line
new UserInternalRegexHandler();
});
......@@ -255,6 +255,7 @@ module ApplicationSettingsHelper
:instance_statistics_visibility_private,
:user_default_external,
:user_show_add_ssh_key_message,
:user_default_internal_regex,
:user_oauth_applications,
:version_check_enabled,
:web_ide_clientside_preview_enabled
......
......@@ -23,6 +23,17 @@ module UsersHelper
profile_tabs.include?(tab)
end
def user_internal_regex_data
settings = Gitlab::CurrentSettings.current_application_settings
pattern, options = if settings.user_default_internal_regex_enabled?
regex = settings.user_default_internal_regex_instance
JsRegex.new(regex).to_h.slice(:source, :options).values
end
{ user_internal_regex_pattern: pattern, user_internal_regex_options: options }
end
def current_user_menu_items
@current_user_menu_items ||= get_current_user_menu_items
end
......
......@@ -192,6 +192,8 @@ class ApplicationSetting < ActiveRecord::Base
numericality: { less_than_or_equal_to: :gitaly_timeout_default },
if: :gitaly_timeout_default
validates :user_default_internal_regex, js_regex: true, allow_nil: true
SUPPORTED_KEY_TYPES.each do |type|
validates :"#{type}_key_restriction", presence: true, key_restriction: { type: type }
end
......@@ -299,6 +301,7 @@ class ApplicationSetting < ActiveRecord::Base
usage_ping_enabled: Settings.gitlab['usage_ping_enabled'],
instance_statistics_visibility_private: false,
user_default_external: false,
user_default_internal_regex: nil,
user_show_add_ssh_key_message: true
}
end
......@@ -435,6 +438,14 @@ class ApplicationSetting < ActiveRecord::Base
password_authentication_enabled_for_web? || password_authentication_enabled_for_git?
end
def user_default_internal_regex_enabled?
user_default_external? && user_default_internal_regex.present?
end
def user_default_internal_regex_instance
Regexp.new(user_default_internal_regex, Regexp::IGNORECASE)
end
delegate :terms, to: :latest_terms, allow_nil: true
def latest_terms
@latest_terms ||= Term.latest
......
......@@ -2,6 +2,10 @@
module Users
class BuildService < BaseService
delegate :user_default_internal_regex_enabled?,
:user_default_internal_regex_instance,
to: :'Gitlab::CurrentSettings.current_application_settings'
def initialize(current_user, params = {})
@current_user = current_user
@params = params.dup
......@@ -89,6 +93,10 @@ module Users
if params[:reset_password]
user_params.merge!(force_random_password: true, password_expires_at: nil)
end
if user_default_internal_regex_enabled? && !user_params.key?(:external)
user_params[:external] = user_external?
end
else
allowed_signup_params = signup_params
allowed_signup_params << :skip_confirmation if skip_authorization
......@@ -105,5 +113,9 @@ module Users
def skip_user_confirmation_email_from_setting
!Gitlab::CurrentSettings.send_user_confirmation_email
end
def user_external?
user_default_internal_regex_instance.match(params[:email]).nil?
end
end
end
class JsRegexValidator < ActiveModel::EachValidator
def validate_each(record, attribute, value)
return true if value.blank?
parsed_regex = JsRegex.new(Regexp.new(value, Regexp::IGNORECASE))
if parsed_regex.source.empty?
record.errors.add(attribute, "Regex Pattern #{value} can not be expressed in Javascript")
else
parsed_regex.warnings.each { |warning| record.errors.add(attribute, warning) }
end
rescue RegexpError => regex_error
record.errors.add(attribute, regex_error.to_s)
end
end
......@@ -29,6 +29,13 @@
= f.check_box :user_default_external, class: 'form-check-input'
= f.label :user_default_external, class: 'form-check-label' do
Newly registered users will by default be external
.prepend-top-10
= _('Internal users')
= f.text_field :user_default_internal_regex, placeholder: _('Regex pattern'), class: 'form-control prepend-top-5'
.help-block
= _('Specify an e-mail address regex pattern to identify default internal users.')
= link_to _('More information'), help_page_path('user/permissions', anchor: 'external-users-permissions'),
target: '_blank'
.form-group
= f.label :user_show_add_ssh_key_message, 'Prompt users to upload SSH keys', class: 'label-bold'
.form-check
......
......@@ -34,8 +34,12 @@
.form-group.row
.col-sm-2.text-right
= f.label :external, class: 'col-form-label'
.hidden{ data: user_internal_regex_data }
.col-sm-10
= f.check_box :external do
External
%p.light
External users cannot see internal or private projects unless access is explicitly granted. Also, external users cannot create projects or groups.
%row.hidden#warning_external_automatically_set.hidden
.badge.badge-warning.text-white
= _('Automatically marked as default internal user')
---
title: Add an option to whitelist users based on email address as internal when the "New user set to external" setting is enabled.
merge_request: 17711
author: Roger Rüttimann
type: added
class AddUserInternalRegexToApplicationSetting < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
def up
add_column :application_settings, :user_default_internal_regex, :string, null: true
end
def down
remove_column :application_settings, :user_default_internal_regex
end
end
......@@ -164,6 +164,7 @@ ActiveRecord::Schema.define(version: 20180826111825) do
t.boolean "authorized_keys_enabled", default: true, null: false
t.string "auto_devops_domain"
t.boolean "pages_domain_verification_enabled", default: true, null: false
t.string "user_default_internal_regex"
t.boolean "allow_local_requests_from_hooks_and_services", default: false, null: false
t.boolean "enforce_terms", default: false
t.boolean "mirror_available", default: true, null: false
......
......@@ -197,7 +197,7 @@ They will, like usual users, receive a role in the project or group with all
the abilities that are mentioned in the table above. They cannot however create
groups or projects, and they have the same access as logged out users in all
other cases.
An administrator can flag a user as external [through the API](../api/users.md)
or by checking the checkbox on the admin panel. As an administrator, navigate
to **Admin > Users** to create a new user or edit an existing one. There, you
......@@ -206,6 +206,21 @@ will find the option to flag the user as external.
By default new users are not set as external users. This behavior can be changed
by an administrator under **Admin > Application Settings**.
### Default internal users
The "Internal users" field allows specifying an e-mail address regex pattern to identify default internal users.
New users whose email address matches the regex pattern will be set to internal by default rather than an external collaborator.
The regex pattern format is Ruby, but it needs to be convertible to JavaScript, and the ignore case flag will be set, e.g. "/regex pattern/i".
Here are some examples:
- Use `\.internal@domain\.com` to mark email addresses containing ".internal@domain.com" internal.
- Use `^(?:(?!\.ext@domain\.com).)*$\r?` to mark users with email addresses NOT including .ext@domain.com internal.
Please be aware that this regex could lead to a DOS attack, [see](https://en.wikipedia.org/wiki/ReDoS?) ReDos on Wikipedia.
## Auditor users **[PREMIUM ONLY]**
>[Introduced][ee-998] in [GitLab Premium][eep] 8.17.
......
......@@ -727,6 +727,9 @@ msgstr ""
msgid "AutoDevOps|enable Auto DevOps"
msgstr ""
msgid "Automatically marked as default internal user"
msgstr ""
msgid "Available"
msgstr ""
......@@ -3187,6 +3190,9 @@ msgstr ""
msgid "Internal - The project can be accessed by any logged in user."
msgstr ""
msgid "Internal users"
msgstr ""
msgid "Interval Pattern"
msgstr ""
......@@ -4670,6 +4676,9 @@ msgid_plural "Refreshing in %d seconds to show the updated status..."
msgstr[0] ""
msgstr[1] ""
msgid "Regex pattern"
msgstr ""
msgid "Register / Sign In"
msgstr ""
......@@ -5277,6 +5286,9 @@ msgstr ""
msgid "Specific Runners"
msgstr ""
msgid "Specify an e-mail address regex pattern to identify default internal users."
msgstr ""
msgid "Specify the following URL during the Runner setup:"
msgstr ""
......@@ -5934,6 +5946,9 @@ msgstr ""
msgid "To add an SSH key you need to %{generate_link_start}generate one%{link_end} or use an %{existing_link_start}existing key%{link_end}."
msgstr ""
msgid "To define internal users, first enable new users set to external"
msgstr ""
msgid "To get started you enter your FogBugz URL and login information below. In the next steps, you'll be able to map users and select the projects you want to import."
msgstr ""
......
......@@ -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/'
......
......@@ -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
......
......@@ -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 }
......
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
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
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();
});
});
});
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();
});
});
});
......@@ -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
......@@ -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)