Commit 2625c5b5 authored by Alex Pooley's avatar Alex Pooley

Include shared groups in users authorized groups

Authorized groups are all groups where user is defined as a direct
member. This is in contrast to an indirect membership through
inheritance.

The system has previously incorrectly or incompletely ignored shared
group memberships. This commit will transfer direct membership from one
group in to another group when shared.

Introduces the shared_group_membership_auth feature flag.
parent e2f4a758
......@@ -908,11 +908,10 @@ class User < ApplicationRecord
# Returns the groups a user has access to, either through a membership or a project authorization
def authorized_groups
Group.unscoped do
Group.from_union([
groups,
authorized_projects.joins(:namespace).select('namespaces.*')
])
if Feature.enabled?(:shared_group_membership_auth, self)
authorized_groups_with_shared_membership
else
authorized_groups_without_shared_membership
end
end
......@@ -1807,6 +1806,26 @@ class User < ApplicationRecord
private
def authorized_groups_without_shared_membership
Group.from_union([
groups,
authorized_projects.joins(:namespace).select('namespaces.*')
])
end
def authorized_groups_with_shared_membership
cte = Gitlab::SQL::CTE.new(:direct_groups, authorized_groups_without_shared_membership)
cte_alias = cte.table.alias(Group.table_name)
Group
.with(cte.to_arel)
.from_union([
Group.from(cte_alias),
Group.joins(:shared_with_group_links)
.where(group_group_links: { shared_with_group_id: Group.from(cte_alias) })
])
end
def default_private_profile_to_false
return unless private_profile_changed? && private_profile.nil?
......
---
name: shared_group_membership_auth
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/46412
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/224771
milestone: '13.6'
type: development
group: group::access
default_enabled: false
......@@ -346,9 +346,8 @@ module EE
def authorized_groups
::Group.unscoped do
::Group.from_union([
groups,
available_minimal_access_groups,
authorized_projects.joins(:namespace).select('namespaces.*')
super,
available_minimal_access_groups
])
end
end
......
......@@ -3,8 +3,8 @@
require 'spec_helper'
RSpec.describe GroupDescendantsFinder do
let(:user) { create(:user) }
let(:group) { create(:group) }
let_it_be(:user) { create(:user) }
let_it_be(:group) { create(:group) }
let(:params) { {} }
subject(:finder) do
......@@ -129,6 +129,39 @@ RSpec.describe GroupDescendantsFinder do
end
end
context 'with shared groups' do
let_it_be(:other_group) { create(:group) }
let_it_be(:shared_group_link) do
create(:group_group_link,
shared_group: group,
shared_with_group: other_group)
end
context 'without common ancestor' do
it { expect(finder.execute).to be_empty }
end
context 'with common ancestor' do
let_it_be(:common_ancestor) { create(:group) }
let_it_be(:other_group) { create(:group, parent: common_ancestor) }
let_it_be(:group) { create(:group, parent: common_ancestor) }
context 'querying under the common ancestor' do
it { expect(finder.execute).to be_empty }
end
context 'querying the common ancestor' do
subject(:finder) do
described_class.new(current_user: user, parent_group: common_ancestor, params: params)
end
it 'contains shared subgroups' do
expect(finder.execute).to contain_exactly(group, other_group)
end
end
end
end
context 'with nested groups' do
let!(:project) { create(:project, namespace: group) }
let!(:subgroup) { create(:group, :private, parent: group) }
......
......@@ -2906,6 +2906,34 @@ RSpec.describe User do
subject { user.authorized_groups }
it { is_expected.to contain_exactly private_group, project_group }
context 'with shared memberships' do
let!(:shared_group) { create(:group) }
let!(:other_group) { create(:group) }
before do
create(:group_group_link, shared_group: shared_group, shared_with_group: private_group)
create(:group_group_link, shared_group: private_group, shared_with_group: other_group)
end
context 'when shared_group_membership_auth is enabled' do
before do
stub_feature_flags(shared_group_membership_auth: user)
end
it { is_expected.to include shared_group }
it { is_expected.not_to include other_group }
end
context 'when shared_group_membership_auth is disabled' do
before do
stub_feature_flags(shared_group_membership_auth: false)
end
it { is_expected.not_to include shared_group }
it { is_expected.not_to include other_group }
end
end
end
describe '#membership_groups' do
......
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