Commit 6da2725d authored by Drew Blessing's avatar Drew Blessing

Ensure SAML Group Sync runs anytime SAML Group Links exist

SAML Group Sync should always be executed whenever SAML Group Links
exist. This ensures a user is removed from groups even if SAML
doesn't send any group names, or when group names do not match
any SAML Group Links.

Changelog: fixed
EE: true
parent 80f46041
......@@ -38,18 +38,30 @@ module Gitlab
def sync_groups?
return false unless user && group.saml_group_sync_available?
group_names_from_saml.any? && group_link_ids.any?
any_group_links_in_hierarchy?
end
# rubocop:disable CodeReuse/ActiveRecord
def group_link_ids
strong_memoize(:group_link_ids) do
next [] if group_names_from_saml.empty?
SamlGroupLink
.by_saml_group_name(group_names_from_saml)
.by_group_id(group.self_and_descendants.select(:id))
.by_group_id(group_ids_in_hierarchy)
.pluck(:id)
end
end
def any_group_links_in_hierarchy?
strong_memoize(:group_ids_with_any_links) do
SamlGroupLink.by_group_id(group_ids_in_hierarchy).exists?
end
end
def group_ids_in_hierarchy
group.self_and_descendants.select(:id)
end
# rubocop:enable CodeReuse/ActiveRecord
def group_names_from_saml
......
......@@ -64,8 +64,8 @@ RSpec.describe Gitlab::Auth::GroupSaml::MembershipUpdater do
allow(group).to receive(:saml_group_sync_available?).and_return(enabled)
end
let(:group_link) { create(:saml_group_link, saml_group_name: 'Owners', group: group) }
let(:subgroup_link) { create(:saml_group_link, saml_group_name: 'Developers', group: create(:group, parent: group)) }
let!(:group_link) { create(:saml_group_link, saml_group_name: 'Owners', group: group) }
let!(:subgroup_link) { create(:saml_group_link, saml_group_name: 'Developers', group: create(:group, parent: group)) }
context 'when group sync is not available' do
before do
......@@ -74,6 +74,8 @@ RSpec.describe Gitlab::Auth::GroupSaml::MembershipUpdater do
it 'does not enqueue group sync' do
expect(GroupSamlGroupSyncWorker).not_to receive(:perform_async)
update_membership
end
end
......@@ -99,6 +101,33 @@ RSpec.describe Gitlab::Auth::GroupSaml::MembershipUpdater do
update_membership
end
end
context 'when auth hash contains no groups' do
let!(:auth_hash) do
Gitlab::Auth::GroupSaml::AuthHash.new(
OmniAuth::AuthHash.new(extra: { raw_info: OneLogin::RubySaml::Attributes.new })
)
end
it 'enqueues group sync' do
expect(GroupSamlGroupSyncWorker).to receive(:perform_async).with(user.id, group.id, [])
update_membership
end
end
context 'when auth hash groups do not match group links' do
before do
group_link.update!(saml_group_name: 'Web Developers')
subgroup_link.destroy!
end
it 'enqueues group sync' do
expect(GroupSamlGroupSyncWorker).to receive(:perform_async).with(user.id, group.id, [])
update_membership
end
end
end
end
end
......@@ -87,6 +87,19 @@ RSpec.describe GroupSamlGroupSyncWorker do
perform([top_level_group_link.id, group_link.id])
end
end
context 'when the worker receives no group link ids' do
before do
group.add_user(user, Gitlab::Access::DEVELOPER)
end
it 'calls the sync service and removes existing users' do
expect_sync_service_call(group_links: [])
expect_metadata_logging_call({ added: 0, updated: 0, removed: 1 })
perform([])
end
end
end
end
......
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