Commit c39941b0 authored by Igor Drozdov's avatar Igor Drozdov

Merge branch...

Merge branch '212817-http-status-code-for-todos-api-when-an-epic-cannot-be-found-by-a-legitimate-user-is-wrong' into 'master'

Resolve "HTTP status code for Todos API when an epic cannot be found by a legitimate user is wrongly returning 403 instead of 404"

Closes #212817

See merge request gitlab-org/gitlab!28310
parents 1ff92c8e 49474df5
---
title: Fix HTTP status code for Todos API when an epic cannot be found
merge_request: 28310
author:
type: fixed
...@@ -104,12 +104,9 @@ module EE ...@@ -104,12 +104,9 @@ module EE
end end
end end
override :find_project_issue
# rubocop: disable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord
def find_project_issue(iid, project_id = nil) def find_group_epic(iid)
project = project_id ? find_project!(project_id) : user_project EpicsFinder.new(current_user, group_id: user_group.id).find_by!(iid: iid)
::IssuesFinder.new(current_user, project_id: project.id).find_by!(iid: iid)
end end
# rubocop: enable CodeReuse/ActiveRecord # rubocop: enable CodeReuse/ActiveRecord
......
...@@ -9,14 +9,8 @@ module EE ...@@ -9,14 +9,8 @@ module EE
helpers do helpers do
extend ::Gitlab::Utils::Override extend ::Gitlab::Utils::Override
# rubocop: disable CodeReuse/ActiveRecord
def epic
@epic ||= user_group.epics.find_by(iid: params[:epic_iid])
end
# rubocop: enable CodeReuse/ActiveRecord
def authorize_can_read! def authorize_can_read!
authorize!(:read_epic, epic) authorize!(:read_epic, user_group)
end end
end end
...@@ -29,6 +23,7 @@ module EE ...@@ -29,6 +23,7 @@ module EE
end end
post ":id/epics/:epic_iid/todo" do post ":id/epics/:epic_iid/todo" do
authorize_can_read! authorize_can_read!
epic = find_group_epic(params[:epic_iid])
todo = ::TodoService.new.mark_todo(epic, current_user).first todo = ::TodoService.new.mark_todo(epic, current_user).first
if todo if todo
......
...@@ -142,7 +142,7 @@ describe API::Todos do ...@@ -142,7 +142,7 @@ describe API::Todos do
post api("/groups/#{group.id}/epics/#{non_existing_record_iid}/todo", user) post api("/groups/#{group.id}/epics/#{non_existing_record_iid}/todo", user)
expect(response).to have_gitlab_http_status(:forbidden) expect(response).to have_gitlab_http_status(:not_found)
end end
it 'returns an error if the epic is not accessible' do it 'returns an error if the epic is not accessible' do
......
...@@ -179,8 +179,10 @@ module API ...@@ -179,8 +179,10 @@ module API
end end
# rubocop: disable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord
def find_project_issue(iid) def find_project_issue(iid, project_id = nil)
IssuesFinder.new(current_user, project_id: user_project.id).find_by!(iid: iid) project = project_id ? find_project!(project_id) : user_project
::IssuesFinder.new(current_user, project_id: project.id).find_by!(iid: iid)
end end
# rubocop: enable CodeReuse/ActiveRecord # rubocop: enable CodeReuse/ActiveRecord
......
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