From 12c172b14c37eb0b3f31a394552fdc07eec08923 Mon Sep 17 00:00:00 2001
From: Heinrich Lee Yu <heinrich@gitlab.com>
Date: Thu, 16 May 2024 11:04:39 +0800
Subject: [PATCH] Fix query in User#authorized_groups
Child project members of an invited group are not given access to the
inviting group. Only direct members are given access.
---
app/models/user.rb | 34 +++++++++++-----------------------
spec/models/user_spec.rb | 22 ++++++++++++----------
2 files changed, 23 insertions(+), 33 deletions(-)
diff --git a/app/models/user.rb b/app/models/user.rb
index e58e57645fd7..9dd8d16787bf 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -1243,7 +1243,17 @@ def update_invalid_gpg_signatures
# Returns the groups a user has access to, either through a membership or a project authorization
def authorized_groups
Group.unscoped do
- authorized_groups_with_shared_membership
+ direct_groups_cte = Gitlab::SQL::CTE.new(:direct_groups, groups)
+ direct_groups_cte_alias = direct_groups_cte.table.alias(Group.table_name)
+
+ Group
+ .with(direct_groups_cte.to_arel)
+ .from_union([
+ Group.from(direct_groups_cte_alias),
+ Group.id_in(authorized_projects.select(:namespace_id)),
+ Group.joins(:shared_with_group_links)
+ .where(group_group_links: { shared_with_group_id: Group.from(direct_groups_cte_alias) })
+ ])
end
end
@@ -2526,28 +2536,6 @@ def group_callouts_by_feature_name
@group_callouts_by_feature_name ||= group_callouts.index_by(&:source_feature_name)
end
- def authorized_groups_without_shared_membership
- Group.from_union(
- [
- groups,
- Group.id_in(authorized_projects.select(:namespace_id))
- ]
- )
- 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 has_current_license?
false
end
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 6345ed6d2a03..0c426c428b44 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -4795,14 +4795,14 @@ def login_method(login)
end
describe '#authorized_groups' do
- let!(:user) { create(:user) }
- let!(:private_group) { create(:group) }
- let!(:child_group) { create(:group, parent: private_group) }
+ let_it_be(:user) { create(:user) }
+ let_it_be(:private_group) { create(:group) }
+ let_it_be(:child_group) { create(:group, parent: private_group) }
- let!(:project_group) { create(:group) }
- let!(:project) { create(:project, group: project_group) }
+ let_it_be(:project_group) { create(:group) }
+ let_it_be(:project) { create(:project, group: project_group) }
- before do
+ before_all do
private_group.add_member(user, Gitlab::Access::MAINTAINER)
project.add_maintainer(user)
end
@@ -4812,16 +4812,18 @@ def login_method(login)
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) }
+ let_it_be(:shared_group) { create(:group) }
+ let_it_be(:other_group) { create(:group) }
+ let_it_be(:shared_with_project_group) { create(:group) }
- before do
+ before_all 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)
+ create(:group_group_link, shared_group: shared_with_project_group, shared_with_group: project_group)
end
it { is_expected.to include shared_group }
- it { is_expected.not_to include other_group }
+ it { is_expected.not_to include other_group, shared_with_project_group }
end
context 'when a new column is added to namespaces table' do
--
GitLab