Commit bfcd6445 authored by Magdalena Frankiewicz's avatar Magdalena Frankiewicz Committed by Imre Farkas

Revert "Revert "Merge branch 'restrict-user-autocompletes' into 'master'""

This reverts commit 8599c629.
parent f0c496e9
export const AJAX_USERS_SELECT_OPTIONS_MAP = {
projectId: 'projectId',
groupId: 'groupId',
showCurrentUser: 'currentUser',
authorId: 'authorId',
skipUsers: 'skipUsers',
};
export const AJAX_USERS_SELECT_PARAMS_MAP = {
project_id: 'projectId',
group_id: 'groupId',
skip_ldap: 'skipLdap',
todo_filter: 'todoFilter',
todo_state_filter: 'todoStateFilter',
current_user: 'showCurrentUser',
author_id: 'authorId',
skip_users: 'skipUsers',
};
......@@ -4,10 +4,15 @@
import $ from 'jquery';
import { escape, template, uniqBy } from 'lodash';
import axios from './lib/utils/axios_utils';
import { s__, __, sprintf } from './locale';
import ModalStore from './boards/stores/modal_store';
import { parseBoolean } from './lib/utils/common_utils';
import axios from '../lib/utils/axios_utils';
import { s__, __, sprintf } from '../locale';
import ModalStore from '../boards/stores/modal_store';
import { parseBoolean } from '../lib/utils/common_utils';
import {
AJAX_USERS_SELECT_OPTIONS_MAP,
AJAX_USERS_SELECT_PARAMS_MAP,
} from 'ee_else_ce/users_select/constants';
import { getAjaxUsersSelectOptions, getAjaxUsersSelectParams } from './utils';
// TODO: remove eventHub hack after code splitting refactor
window.emitSidebarEvent = window.emitSidebarEvent || $.noop;
......@@ -555,13 +560,8 @@ function UsersSelect(currentUser, els, options = {}) {
import(/* webpackChunkName: 'select2' */ 'select2/select2')
.then(() => {
$('.ajax-users-select').each((i, select) => {
const options = {};
const options = getAjaxUsersSelectOptions($(select), AJAX_USERS_SELECT_OPTIONS_MAP);
options.skipLdap = $(select).hasClass('skip_ldap');
options.projectId = $(select).data('projectId');
options.groupId = $(select).data('groupId');
options.showCurrentUser = $(select).data('currentUser');
options.authorId = $(select).data('authorId');
options.skipUsers = $(select).data('skipUsers');
const showNullUser = $(select).data('nullUser');
const showAnyUser = $(select).data('anyUser');
const showEmailUser = $(select).data('emailUser');
......@@ -702,14 +702,7 @@ UsersSelect.prototype.users = function(query, options, callback) {
const params = {
search: query,
active: true,
project_id: options.projectId || null,
group_id: options.groupId || null,
skip_ldap: options.skipLdap || null,
todo_filter: options.todoFilter || null,
todo_state_filter: options.todoStateFilter || null,
current_user: options.showCurrentUser || null,
author_id: options.authorId || null,
skip_users: options.skipUsers || null,
...getAjaxUsersSelectParams(options, AJAX_USERS_SELECT_PARAMS_MAP),
};
if (options.issuableType === 'merge_request') {
......
/**
* Get options from data attributes on passed `$select`.
* @param {jQuery} $select
* @param {Object} optionsMap e.g. { optionKeyName: 'dataAttributeName' }
*/
export const getAjaxUsersSelectOptions = ($select, optionsMap) => {
return Object.keys(optionsMap).reduce((accumulator, optionKey) => {
const dataKey = optionsMap[optionKey];
accumulator[optionKey] = $select.data(dataKey);
return accumulator;
}, {});
};
/**
* Get query parameters used for users request from passed `options` parameter
* @param {Object} options e.g. { currentUserId: 1, fooBar: 'baz' }
* @param {Object} paramsMap e.g. { user_id: 'currentUserId', foo_bar: 'fooBar' }
*/
export const getAjaxUsersSelectParams = (options, paramsMap) => {
return Object.keys(paramsMap).reduce((accumulator, paramKey) => {
const optionKey = paramsMap[paramKey];
accumulator[paramKey] = options[optionKey] || null;
return accumulator;
}, {});
};
import {
AJAX_USERS_SELECT_OPTIONS_MAP as CE_AJAX_USERS_SELECT_OPTIONS_MAP,
AJAX_USERS_SELECT_PARAMS_MAP as CE_AJAX_USERS_SELECT_PARAMS_MAP,
} from '~/users_select/constants';
export const AJAX_USERS_SELECT_OPTIONS_MAP = {
...CE_AJAX_USERS_SELECT_OPTIONS_MAP,
samlProviderId: 'samlProviderId',
};
export const AJAX_USERS_SELECT_PARAMS_MAP = {
...CE_AJAX_USERS_SELECT_PARAMS_MAP,
saml_provider_id: 'samlProviderId',
};
......@@ -5,7 +5,7 @@ module EE
module UsersFinder
extend ::Gitlab::Utils::Override
attr_reader :skip_ldap, :push_code_to_protected_branches, :push_code
attr_reader :skip_ldap, :push_code_to_protected_branches, :push_code, :saml_provider_id
override :initialize
def initialize(params:, current_user:, project:, group:)
......@@ -14,12 +14,12 @@ module EE
@skip_ldap = params[:skip_ldap]
@push_code_to_protected_branches = params[:push_code_to_protected_branches]
@push_code = params[:push_code]
@saml_provider_id = params[:saml_provider_id]
end
override :find_users
def find_users
users = super
users = super.limit_to_saml_provider(saml_provider_id)
skip_ldap == 'true' ? users.non_ldap : users
end
......
......@@ -2,6 +2,8 @@
module EE
module SelectsHelper
extend ::Gitlab::Utils::Override
def ldap_server_select_options
options_from_collection_for_select(
::Gitlab::Auth::Ldap::Config.available_servers,
......@@ -18,5 +20,17 @@ module EE
hidden_field_tag(id, value, class: css_class.join(' '))
end
override :users_select_data_attributes
def users_select_data_attributes(opts)
super.merge(saml_provider_id: opts[:saml_provider_id] || nil)
end
override :users_select_tag
def users_select_tag(id, opts = {})
root_group = @group&.root_ancestor || @project&.group&.root_ancestor
opts[:saml_provider_id] = root_group&.enforced_sso? && root_group.saml_provider&.id
super(id, opts)
end
end
end
......@@ -133,6 +133,16 @@ module EE
''
end
# Limits the users to those who have an identity that belongs to
# the given SAML Provider
def limit_to_saml_provider(saml_provider_id)
if saml_provider_id
joins(:identities).where(identities: { saml_provider_id: saml_provider_id })
else
all
end
end
end
def cannot_be_admin_and_auditor
......
---
title: If a parent group enforces SAML SSO, when adding a member to that group, its
subgroup or its project the autocomplete shows only users who have identity with
the SAML provider of the parent group
merge_request: 30607
author:
type: fixed
......@@ -6,17 +6,13 @@ describe 'Groups > Members > List members' do
let(:user2) { create(:user, name: 'Mary Jane') }
let(:group) { create(:group) }
before do
group.add_developer(user1)
sign_in(user1)
end
context 'with Group SAML identity linked for a user' do
let(:saml_provider) { create(:saml_provider) }
let(:group) { saml_provider.group }
before do
sign_in(user1)
group.add_developer(user1)
group.add_guest(user2)
user2.identities.create!(provider: :group_saml,
saml_provider: saml_provider,
......@@ -48,4 +44,38 @@ describe 'Groups > Members > List members' do
expect(page).to have_selector("#group_member_#{member.id} .badge-info", text: 'Managed Account')
end
end
context 'with SAML and enforced SSO' do
let(:saml_provider) { create(:saml_provider, group: group, enabled: true, enforced_sso: true) }
let(:user3) { create(:user, name: 'Amy with different SAML provider') }
let(:user4) { create(:user, name: 'Bob without SAML') }
let(:session) { { active_group_sso_sign_ins: { saml_provider.id => DateTime.now } } }
before do
stub_feature_flags(enforced_sso: true, group_saml: true)
stub_licensed_features(group_saml: true)
allow(Gitlab::Session).to receive(:current).and_return(session)
create(:identity, saml_provider: saml_provider, user: user1)
group.add_owner(user1)
sign_in(user1)
end
it 'returns only users with SAML in autocomplete', :js do
create(:identity, saml_provider: saml_provider, user: user2)
create(:identity, user: user3)
visit group_group_members_path(group)
wait_for_requests
find('.select2-container').click
expect(page).to have_content(user1.name)
expect(page).to have_content(user2.name)
expect(page).not_to have_content(user3.name)
expect(page).not_to have_content(user4.name)
end
end
end
# frozen_string_literal: true
require 'spec_helper'
describe Autocomplete::UsersFinder do
let(:current_user) { create(:user) }
let(:group) { create(:group) }
let(:saml_provider) { create(:saml_provider, group: group) }
it 'returns only users with that SAML provider when saml_provider_id is given' do
user1 = create(:user, username: 'samdoe')
user2 = create(:user, username: 'allymolly')
create(:identity, saml_provider: saml_provider, user: user1)
params = { saml_provider_id: saml_provider.id }
group.add_users([user1, user2], GroupMember::DEVELOPER)
users = described_class.new(params: params, current_user: current_user, project: nil, group: nil).execute.to_a
expect(users).to match_array([user1])
end
it 'returns the user that name matches the search' do
user1 = create(:user, username: 'samdoe')
user2 = create(:user, username: 'allymolly')
user3 = create(:user, username: 'Samamntha')
create(:identity, saml_provider: saml_provider, user: user1)
create(:identity, saml_provider: saml_provider, user: user2)
create(:identity, saml_provider: saml_provider, user: user3)
params = { saml_provider_id: saml_provider.id, search: 'sam' }
group.add_users([user1, user2, user3], GroupMember::DEVELOPER)
users = described_class.new(params: params, current_user: current_user, project: nil, group: nil).execute.to_a
expect(users).to match_array([user1, user3])
end
end
......@@ -527,6 +527,30 @@ describe User do
end
end
describe '.limit_to_saml_provider' do
let_it_be(:user1) { create(:user) }
let_it_be(:user2) { create(:user) }
it 'returns all users when SAML provider is nil' do
rel = described_class.limit_to_saml_provider(nil)
expect(rel).to include(user1, user2)
end
it 'returns only the users who have an identity that belongs to the given SAML provider' do
create(:user)
group = create(:group)
saml_provider = create(:saml_provider, group: group)
create(:identity, saml_provider: saml_provider, user: user1)
create(:identity, saml_provider: saml_provider, user: user2)
create(:identity, user: create(:user))
rel = described_class.limit_to_saml_provider(saml_provider.id)
expect(rel).to contain_exactly(user1, user2)
end
end
describe '#group_managed_account?' do
subject { user.group_managed_account? }
......
......@@ -3,6 +3,6 @@
FactoryBot.define do
factory :identity do
provider { 'ldapmain' }
extern_uid { 'my-ldap-id' }
sequence(:extern_uid) { |n| "my-ldap-id-#{n}" }
end
end
import $ from 'jquery';
import { getAjaxUsersSelectOptions, getAjaxUsersSelectParams } from '~/users_select/utils';
const options = {
fooBar: 'baz',
activeUserId: 1,
};
describe('getAjaxUsersSelectOptions', () => {
it('returns options built from select data attributes', () => {
const $select = $('<select />', { 'data-foo-bar': 'baz', 'data-user-id': 1 });
expect(
getAjaxUsersSelectOptions($select, { fooBar: 'fooBar', activeUserId: 'user-id' }),
).toEqual(options);
});
});
describe('getAjaxUsersSelectParams', () => {
it('returns query parameters built from provided options', () => {
expect(
getAjaxUsersSelectParams(options, {
foo_bar: 'fooBar',
active_user_id: 'activeUserId',
non_existent_key: 'nonExistentKey',
}),
).toEqual({
foo_bar: 'baz',
active_user_id: 1,
non_existent_key: null,
});
});
});
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment