From a50deb1c1563383477c67ed2d36075be8e725abc Mon Sep 17 00:00:00 2001 From: Eugenia Grieff Date: Mon, 12 Dec 2022 15:45:31 +0100 Subject: [PATCH 1/2] Define abilities in epic policy Check permission against epic instead of group --- .../graphql/mutations/epic_tree/reorder_spec.rb | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/ee/spec/requests/api/graphql/mutations/epic_tree/reorder_spec.rb b/ee/spec/requests/api/graphql/mutations/epic_tree/reorder_spec.rb index 66680503147ce8..07867b803b6de9 100644 --- a/ee/spec/requests/api/graphql/mutations/epic_tree/reorder_spec.rb +++ b/ee/spec/requests/api/graphql/mutations/epic_tree/reorder_spec.rb @@ -78,6 +78,22 @@ def mutation_response expect(mutation_response['errors']).to contain_exactly('You don\'t have permissions to move the objects.') end end + + context 'when moving an issue' do + before do + group.add_guest(current_user) + variables[:moved][:id] = GitlabSchema.id_from_object(epic_issue2).to_s + variables[:moved][:adjacent_reference_id] = GitlabSchema.id_from_object(epic_issue1).to_s + end + + it_behaves_like 'a mutation that does not update the tree' + + it 'returns the error message' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(mutation_response['errors']).to eq(['You don\'t have permissions to move the objects.']) + end + end end context 'when the user has permission' do -- GitLab From 83541ea6de2200cda666ff850a516bef8ab4ffaf Mon Sep 17 00:00:00 2001 From: Eugenia Grieff Date: Thu, 8 Dec 2022 14:30:00 +0100 Subject: [PATCH 2/2] Expose canAdminRelation for epics Expose permissions to modify epic tree items Changelog: added EE: true --- ee/app/helpers/ee/issuables_helper.rb | 1 + ee/spec/helpers/ee/issuables_helper_spec.rb | 36 +++++++++++++------ .../mutations/epic_tree/reorder_spec.rb | 16 --------- 3 files changed, 27 insertions(+), 26 deletions(-) diff --git a/ee/app/helpers/ee/issuables_helper.rb b/ee/app/helpers/ee/issuables_helper.rb index 9dcd2d0de8b4cb..b72b6429fa75f4 100644 --- a/ee/app/helpers/ee/issuables_helper.rb +++ b/ee/app/helpers/ee/issuables_helper.rb @@ -27,6 +27,7 @@ def issuable_initial_data(issuable) data[:issueLinksEndpoint] = group_epic_issues_path(parent, issuable) data[:issuesWebUrl] = issues_group_path(parent) data[:projectsEndpoint] = expose_path(api_v4_groups_projects_path(id: parent.id)) + data[:canAdminRelation] = can?(current_user, :admin_epic_relation, issuable) end data diff --git a/ee/spec/helpers/ee/issuables_helper_spec.rb b/ee/spec/helpers/ee/issuables_helper_spec.rb index 025bfc8ee94410..2ef3161b968256 100644 --- a/ee/spec/helpers/ee/issuables_helper_spec.rb +++ b/ee/spec/helpers/ee/issuables_helper_spec.rb @@ -2,27 +2,28 @@ require 'spec_helper' -RSpec.describe IssuablesHelper do +RSpec.describe IssuablesHelper, feature_category: :team_planning do let_it_be(:user) { create(:user) } describe '#issuable_initial_data' do + let(:permission) { true } + before do allow(helper).to receive(:current_user).and_return(user) - allow(helper).to receive(:can?).and_return(true) + allow(helper).to receive(:can?).and_return(permission) stub_commonmark_sourcepos_disabled end context 'for an epic' do let_it_be(:epic) { create(:epic, author: user, description: 'epic text', confidential: true) } - it 'returns the correct data' do - @group = epic.group - - expected_data = { - canAdmin: true, - canDestroy: true, - canUpdate: true, - canUpdateTimelineEvent: true, + let(:expected_data) do + { + canAdmin: permission, + canAdminRelation: permission, + canDestroy: permission, + canUpdate: permission, + canUpdateTimelineEvent: permission, confidential: epic.confidential, endpoint: "/groups/#{@group.full_path}/-/epics/#{epic.iid}", epicLinksEndpoint: "/groups/#{@group.full_path}/-/epics/#{epic.iid}/links", @@ -46,8 +47,23 @@ projectsEndpoint: "/api/v4/groups/#{@group.id}/projects", updateEndpoint: "/groups/#{@group.full_path}/-/epics/#{epic.iid}.json" } + end + + before do + @group = epic.group + end + + it 'returns the correct data when permissions allowed' do expect(helper.issuable_initial_data(epic)).to eq(expected_data) end + + context 'when permissions denied' do + let(:permission) { false } + + it 'returns the correct data' do + expect(helper.issuable_initial_data(epic)).to eq(expected_data) + end + end end context 'for an issue' do diff --git a/ee/spec/requests/api/graphql/mutations/epic_tree/reorder_spec.rb b/ee/spec/requests/api/graphql/mutations/epic_tree/reorder_spec.rb index 07867b803b6de9..66680503147ce8 100644 --- a/ee/spec/requests/api/graphql/mutations/epic_tree/reorder_spec.rb +++ b/ee/spec/requests/api/graphql/mutations/epic_tree/reorder_spec.rb @@ -78,22 +78,6 @@ def mutation_response expect(mutation_response['errors']).to contain_exactly('You don\'t have permissions to move the objects.') end end - - context 'when moving an issue' do - before do - group.add_guest(current_user) - variables[:moved][:id] = GitlabSchema.id_from_object(epic_issue2).to_s - variables[:moved][:adjacent_reference_id] = GitlabSchema.id_from_object(epic_issue1).to_s - end - - it_behaves_like 'a mutation that does not update the tree' - - it 'returns the error message' do - post_graphql_mutation(mutation, current_user: current_user) - - expect(mutation_response['errors']).to eq(['You don\'t have permissions to move the objects.']) - end - end end context 'when the user has permission' do -- GitLab