Commit 023b7230 authored by Robert Speicher's avatar Robert Speicher

Merge branch '224771-shared-group-not-visible' into 'master'

Resolve "Shared group not visible"

See merge request gitlab-org/gitlab!46412
parents bdd0e0db 2625c5b5
...@@ -908,11 +908,10 @@ class User < ApplicationRecord ...@@ -908,11 +908,10 @@ class User < ApplicationRecord
# Returns the groups a user has access to, either through a membership or a project authorization # Returns the groups a user has access to, either through a membership or a project authorization
def authorized_groups def authorized_groups
Group.unscoped do if Feature.enabled?(:shared_group_membership_auth, self)
Group.from_union([ authorized_groups_with_shared_membership
groups, else
authorized_projects.joins(:namespace).select('namespaces.*') authorized_groups_without_shared_membership
])
end end
end end
...@@ -1807,6 +1806,26 @@ class User < ApplicationRecord ...@@ -1807,6 +1806,26 @@ class User < ApplicationRecord
private 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 def default_private_profile_to_false
return unless private_profile_changed? && private_profile.nil? 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 ...@@ -346,9 +346,8 @@ module EE
def authorized_groups def authorized_groups
::Group.unscoped do ::Group.unscoped do
::Group.from_union([ ::Group.from_union([
groups, super,
available_minimal_access_groups, available_minimal_access_groups
authorized_projects.joins(:namespace).select('namespaces.*')
]) ])
end end
end end
......
...@@ -3,8 +3,8 @@ ...@@ -3,8 +3,8 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe GroupDescendantsFinder do RSpec.describe GroupDescendantsFinder do
let(:user) { create(:user) } let_it_be(:user) { create(:user) }
let(:group) { create(:group) } let_it_be(:group) { create(:group) }
let(:params) { {} } let(:params) { {} }
subject(:finder) do subject(:finder) do
...@@ -129,6 +129,39 @@ RSpec.describe GroupDescendantsFinder do ...@@ -129,6 +129,39 @@ RSpec.describe GroupDescendantsFinder do
end end
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 context 'with nested groups' do
let!(:project) { create(:project, namespace: group) } let!(:project) { create(:project, namespace: group) }
let!(:subgroup) { create(:group, :private, parent: group) } let!(:subgroup) { create(:group, :private, parent: group) }
......
...@@ -2906,6 +2906,34 @@ RSpec.describe User do ...@@ -2906,6 +2906,34 @@ RSpec.describe User do
subject { user.authorized_groups } subject { user.authorized_groups }
it { is_expected.to contain_exactly private_group, project_group } 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 end
describe '#membership_groups' do 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