From 6a43b24062256244e3394e12eccc6d5f08f34bf6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jarka=20Ko=C5=A1anov=C3=A1?= <jarka@gitlab.com>
Date: Wed, 27 Jul 2022 08:33:45 +0000
Subject: [PATCH] Check permissions when filtering by contact or organization
Merge branch 'security-crm-fix' into 'master'
See merge request gitlab-org/security/gitlab!2607
Changelog: security
---
app/finders/issuable_finder.rb | 20 ++++++
.../graphql/resolvers/issues_resolver_spec.rb | 11 ++-
.../finders/issues_finder_shared_examples.rb | 71 ++++++++++++-------
3 files changed, 75 insertions(+), 27 deletions(-)
diff --git a/app/finders/issuable_finder.rb b/app/finders/issuable_finder.rb
index 8ecf0c158e0b3..47b2a460e6f7e 100644
--- a/app/finders/issuable_finder.rb
+++ b/app/finders/issuable_finder.rb
@@ -483,10 +483,14 @@ def by_non_archived(items)
end
def by_crm_contact(items)
+ return items unless can_filter_by_crm_contact?
+
Issuables::CrmContactFilter.new(params: original_params).filter(items)
end
def by_crm_organization(items)
+ return items unless can_filter_by_crm_organization?
+
Issuables::CrmOrganizationFilter.new(params: original_params).filter(items)
end
@@ -499,4 +503,20 @@ def or_filters_enabled?
def feature_flag_scope
params.group || params.project
end
+
+ def can_filter_by_crm_contact?
+ current_user&.can?(:read_crm_contact, root_group)
+ end
+
+ def can_filter_by_crm_organization?
+ current_user&.can?(:read_crm_organization, root_group)
+ end
+
+ def root_group
+ strong_memoize(:root_group) do
+ base_group = params.group || params.project&.group
+
+ base_group&.root_ancestor
+ end
+ end
end
diff --git a/spec/graphql/resolvers/issues_resolver_spec.rb b/spec/graphql/resolvers/issues_resolver_spec.rb
index a5b5a8e4f7228..89e45810033fa 100644
--- a/spec/graphql/resolvers/issues_resolver_spec.rb
+++ b/spec/graphql/resolvers/issues_resolver_spec.rb
@@ -30,6 +30,9 @@
before_all do
project.add_developer(current_user)
project.add_reporter(reporter)
+
+ create(:crm_settings, group: group, enabled: true)
+
create(:label_link, label: label1, target: issue1)
create(:label_link, label: label1, target: issue2)
create(:label_link, label: label2, target: issue2)
@@ -399,6 +402,8 @@
let_it_be(:crm_issue3) { create(:issue, project: project) }
before_all do
+ group.add_developer(current_user)
+
create(:issue_customer_relations_contact, issue: crm_issue1, contact: contact1)
create(:issue_customer_relations_contact, issue: crm_issue2, contact: contact2)
create(:issue_customer_relations_contact, issue: crm_issue3, contact: contact3)
@@ -631,13 +636,13 @@
end
it 'finds a specific issue with iid', :request_store do
- result = batch_sync(max_queries: 7) { resolve_issues(iid: issue1.iid).to_a }
+ result = batch_sync(max_queries: 8) { resolve_issues(iid: issue1.iid).to_a }
expect(result).to contain_exactly(issue1)
end
it 'batches queries that only include IIDs', :request_store do
- result = batch_sync(max_queries: 7) do
+ result = batch_sync(max_queries: 8) do
[issue1, issue2]
.map { |issue| resolve_issues(iid: issue.iid.to_s) }
.flat_map(&:to_a)
@@ -647,7 +652,7 @@
end
it 'finds a specific issue with iids', :request_store do
- result = batch_sync(max_queries: 7) do
+ result = batch_sync(max_queries: 8) do
resolve_issues(iids: [issue1.iid]).to_a
end
diff --git a/spec/support/shared_examples/finders/issues_finder_shared_examples.rb b/spec/support/shared_examples/finders/issues_finder_shared_examples.rb
index 9d8f37a3e64a9..049ead9fb899c 100644
--- a/spec/support/shared_examples/finders/issues_finder_shared_examples.rb
+++ b/spec/support/shared_examples/finders/issues_finder_shared_examples.rb
@@ -914,42 +914,65 @@
end
end
- context 'filtering by crm contact' do
- let_it_be(:contact1) { create(:contact, group: group) }
- let_it_be(:contact2) { create(:contact, group: group) }
+ context 'crm filtering' do
+ let_it_be(:root_group) { create(:group) }
+ let_it_be(:group) { create(:group, parent: root_group) }
+ let_it_be(:project_crm) { create(:project, :public, group: group) }
+ let_it_be(:organization) { create(:organization, group: root_group) }
+ let_it_be(:contact1) { create(:contact, group: root_group, organization: organization) }
+ let_it_be(:contact2) { create(:contact, group: root_group, organization: organization) }
- let_it_be(:contact1_item1) { create(factory, project: project1) }
- let_it_be(:contact1_item2) { create(factory, project: project1) }
- let_it_be(:contact2_item1) { create(factory, project: project1) }
+ let_it_be(:contact1_item1) { create(factory, project: project_crm) }
+ let_it_be(:contact1_item2) { create(factory, project: project_crm) }
+ let_it_be(:contact2_item1) { create(factory, project: project_crm) }
+ let_it_be(:item_no_contact) { create(factory, project: project_crm) }
- let(:params) { { crm_contact_id: contact1.id } }
+ let_it_be(:all_project_issues) do
+ [contact1_item1, contact1_item2, contact2_item1, item_no_contact]
+ end
+
+ before do
+ create(:crm_settings, group: root_group, enabled: true)
- it 'returns for that contact' do
create(:issue_customer_relations_contact, issue: contact1_item1, contact: contact1)
create(:issue_customer_relations_contact, issue: contact1_item2, contact: contact1)
create(:issue_customer_relations_contact, issue: contact2_item1, contact: contact2)
-
- expect(items).to contain_exactly(contact1_item1, contact1_item2)
end
- end
- context 'filtering by crm organization' do
- let_it_be(:organization) { create(:organization, group: group) }
- let_it_be(:contact1) { create(:contact, group: group, organization: organization) }
- let_it_be(:contact2) { create(:contact, group: group, organization: organization) }
+ context 'filtering by crm contact' do
+ let(:params) { { project_id: project_crm.id, crm_contact_id: contact1.id } }
- let_it_be(:contact1_item1) { create(factory, project: project1) }
- let_it_be(:contact1_item2) { create(factory, project: project1) }
- let_it_be(:contact2_item1) { create(factory, project: project1) }
+ context 'when the user can read crm contacts' do
+ it 'returns for that contact' do
+ root_group.add_reporter(user)
- let(:params) { { crm_organization_id: organization.id } }
+ expect(items).to contain_exactly(contact1_item1, contact1_item2)
+ end
+ end
- it 'returns for that contact' do
- create(:issue_customer_relations_contact, issue: contact1_item1, contact: contact1)
- create(:issue_customer_relations_contact, issue: contact1_item2, contact: contact1)
- create(:issue_customer_relations_contact, issue: contact2_item1, contact: contact2)
+ context 'when the user can not read crm contacts' do
+ it 'does not filter by contact' do
+ expect(items).to match_array(all_project_issues)
+ end
+ end
+ end
+
+ context 'filtering by crm organization' do
+ let(:params) { { project_id: project_crm.id, crm_organization_id: organization.id } }
+
+ context 'when the user can read crm organization' do
+ it 'returns for that organization' do
+ root_group.add_reporter(user)
- expect(items).to contain_exactly(contact1_item1, contact1_item2, contact2_item1)
+ expect(items).to contain_exactly(contact1_item1, contact1_item2, contact2_item1)
+ end
+ end
+
+ context 'when the user can not read crm organization' do
+ it 'does not filter by organization' do
+ expect(items).to match_array(all_project_issues)
+ end
+ end
end
end
--
GitLab