From 5c53f91f5ca07a69de1f108eca12c6339e48f9fd Mon Sep 17 00:00:00 2001
From: Nicolas Dular <ndular@gitlab.com>
Date: Fri, 7 Mar 2025 11:38:52 +0100
Subject: [PATCH] Use the policy check for related epic links API
The readable_by? method is lacking the correct permissions for public
groups on the Issue (WorkItem) model. This change uses the policy
instead of the overriden readable_by? method to get around this bug.
This led to differently returned related links when the feature flag was
enabled.
There is no performance change, since the Epic model has never overriden
the readable_by method and always used the read policy check.
---
ee/lib/api/related_epic_links.rb | 2 +-
.../requests/api/related_epic_links_spec.rb | 20 ++++++++++++++++++-
2 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/ee/lib/api/related_epic_links.rb b/ee/lib/api/related_epic_links.rb
index ce68e8bc7e2c..5caefd92b974 100644
--- a/ee/lib/api/related_epic_links.rb
+++ b/ee/lib/api/related_epic_links.rb
@@ -80,7 +80,7 @@ def find_permissioned_epic!(iid, group_id: nil, permission: :admin_epic_link_rel
# EpicLinks can link to other Epics the user has no access to.
# For these epics we need to check permissions.
related_links = related_links.select do |related_link|
- related_link.source.readable_by?(current_user) && related_link.target.readable_by?(current_user)
+ current_user.can?(:read_epic, related_link.source) && current_user.can?(:read_epic, related_link.target)
end
source_and_target_epics = related_links.reduce(Set.new) { |acc, link| acc << link.source << link.target }
diff --git a/ee/spec/requests/api/related_epic_links_spec.rb b/ee/spec/requests/api/related_epic_links_spec.rb
index 37786415b24c..d830a18ba57f 100644
--- a/ee/spec/requests/api/related_epic_links_spec.rb
+++ b/ee/spec/requests/api/related_epic_links_spec.rb
@@ -72,7 +72,7 @@
describe 'GET /groups/:id/related_epic_links' do
let_it_be(:created_at) { Date.new(2021, 10, 14) }
let_it_be(:updated_at) { Date.new(2021, 10, 14) }
- let_it_be(:group_2) { create(:group, :private) }
+ let_it_be_with_reload(:group_2) { create(:group, :private) }
let_it_be(:related_epic_link_1) do
create(
@@ -108,6 +108,22 @@ def perform_request(user = nil, params = {})
end
end
+ context 'when epics are public' do
+ before do
+ group.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC)
+ end
+
+ it 'returns related epic links' do
+ perform_request(user)
+
+ expect(response).to have_gitlab_http_status(:ok)
+ expect(json_response).to be_an Array
+ expect(json_response.length).to eq(1)
+ expect(json_response.pluck("id")).to match_array([related_epic_link_1.id])
+ expect(response).to match_response_schema('public_api/v4/related_epic_links', dir: 'ee')
+ end
+ end
+
context 'when user has access to the group' do
before do
group.add_guest(user)
@@ -121,6 +137,7 @@ def perform_request(user = nil, params = {})
expect(response).to have_gitlab_http_status(:ok)
expect(json_response).to be_an Array
expect(json_response.length).to eq(1)
+ expect(json_response.pluck("id")).to match_array([related_epic_link_1.id])
expect(json_response[0]['source_epic']['id']).to eq(related_epic_link_1.source.id)
expect(json_response[0]['target_epic']['id']).to eq(related_epic_link_1.target.id)
expect(response).to match_response_schema('public_api/v4/related_epic_links', dir: 'ee')
@@ -283,6 +300,7 @@ def perform_request(user = nil, params = {})
expect(json_response).to be_an Array
expect(json_response.length).to eq(2)
expect(response).to match_response_schema('public_api/v4/related_epics', dir: 'ee')
+ expect(json_response.pluck("related_epic_link_id")).to match_array([related_epic_link_1.id, related_epic_link_2.id])
end
it 'returns multiple links without N + 1' do
--
GitLab