Restrict reopening locked issues for issue authors
This commit is contained in:
parent
be1ef711ed
commit
d729ea19da
10 changed files with 55 additions and 24 deletions
|
@ -14,6 +14,7 @@ class IssuablePolicy < BasePolicy
|
||||||
rule { assignee_or_author }.policy do
|
rule { assignee_or_author }.policy do
|
||||||
enable :read_issue
|
enable :read_issue
|
||||||
enable :update_issue
|
enable :update_issue
|
||||||
|
enable :reopen_issue
|
||||||
enable :read_merge_request
|
enable :read_merge_request
|
||||||
enable :update_merge_request
|
enable :update_merge_request
|
||||||
end
|
end
|
||||||
|
|
|
@ -19,4 +19,8 @@ class IssuePolicy < IssuablePolicy
|
||||||
prevent :update_issue
|
prevent :update_issue
|
||||||
prevent :admin_issue
|
prevent :admin_issue
|
||||||
end
|
end
|
||||||
|
|
||||||
|
rule { locked }.policy do
|
||||||
|
prevent :reopen_issue
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -176,6 +176,7 @@ class ProjectPolicy < BasePolicy
|
||||||
enable :fork_project
|
enable :fork_project
|
||||||
enable :create_project_snippet
|
enable :create_project_snippet
|
||||||
enable :update_issue
|
enable :update_issue
|
||||||
|
enable :reopen_issue
|
||||||
enable :admin_issue
|
enable :admin_issue
|
||||||
enable :admin_label
|
enable :admin_label
|
||||||
enable :admin_list
|
enable :admin_list
|
||||||
|
|
|
@ -3,7 +3,7 @@
|
||||||
module Issues
|
module Issues
|
||||||
class ReopenService < Issues::BaseService
|
class ReopenService < Issues::BaseService
|
||||||
def execute(issue)
|
def execute(issue)
|
||||||
return issue unless can?(current_user, :update_issue, issue)
|
return issue unless can?(current_user, :reopen_issue, issue)
|
||||||
|
|
||||||
if issue.reopen
|
if issue.reopen
|
||||||
event_service.reopen_issue(issue, current_user)
|
event_service.reopen_issue(issue, current_user)
|
||||||
|
|
|
@ -6,6 +6,7 @@
|
||||||
- page_card_attributes @issue.card_attributes
|
- page_card_attributes @issue.card_attributes
|
||||||
|
|
||||||
- can_update_issue = can?(current_user, :update_issue, @issue)
|
- can_update_issue = can?(current_user, :update_issue, @issue)
|
||||||
|
- can_reopen_issue = can?(current_user, :reopen_issue, @issue)
|
||||||
- can_report_spam = @issue.submittable_as_spam_by?(current_user)
|
- can_report_spam = @issue.submittable_as_spam_by?(current_user)
|
||||||
- can_create_issue = show_new_issue_link?(@project)
|
- can_create_issue = show_new_issue_link?(@project)
|
||||||
|
|
||||||
|
@ -40,6 +41,7 @@
|
||||||
%li= link_to 'Report abuse', new_abuse_report_path(user_id: @issue.author.id, ref_url: issue_url(@issue))
|
%li= link_to 'Report abuse', new_abuse_report_path(user_id: @issue.author.id, ref_url: issue_url(@issue))
|
||||||
- if can_update_issue
|
- if can_update_issue
|
||||||
%li= link_to 'Close issue', issue_path(@issue, issue: { state_event: :close }, format: 'json'), class: "btn-close js-btn-issue-action #{issue_button_visibility(@issue, true)}", title: 'Close issue'
|
%li= link_to 'Close issue', issue_path(@issue, issue: { state_event: :close }, format: 'json'), class: "btn-close js-btn-issue-action #{issue_button_visibility(@issue, true)}", title: 'Close issue'
|
||||||
|
- if can_reopen_issue
|
||||||
%li= link_to 'Reopen issue', issue_path(@issue, issue: { state_event: :reopen }, format: 'json'), class: "btn-reopen js-btn-issue-action #{issue_button_visibility(@issue, false)}", title: 'Reopen issue'
|
%li= link_to 'Reopen issue', issue_path(@issue, issue: { state_event: :reopen }, format: 'json'), class: "btn-reopen js-btn-issue-action #{issue_button_visibility(@issue, false)}", title: 'Reopen issue'
|
||||||
- if can_report_spam
|
- if can_report_spam
|
||||||
%li= link_to 'Submit as spam', mark_as_spam_project_issue_path(@project, @issue), method: :post, class: 'btn-spam', title: 'Submit as spam'
|
%li= link_to 'Submit as spam', mark_as_spam_project_issue_path(@project, @issue), method: :post, class: 'btn-spam', title: 'Submit as spam'
|
||||||
|
@ -48,7 +50,7 @@
|
||||||
%li.divider
|
%li.divider
|
||||||
%li= link_to 'New issue', new_project_issue_path(@project), title: 'New issue', id: 'new_issue_link'
|
%li= link_to 'New issue', new_project_issue_path(@project), title: 'New issue', id: 'new_issue_link'
|
||||||
|
|
||||||
= render 'shared/issuable/close_reopen_button', issuable: @issue, can_update: can_update_issue
|
= render 'shared/issuable/close_reopen_button', issuable: @issue, can_update: can_update_issue, can_reopen: can_reopen_issue
|
||||||
|
|
||||||
- if can_report_spam
|
- if can_report_spam
|
||||||
= link_to 'Submit as spam', mark_as_spam_project_issue_path(@project, @issue), method: :post, class: 'd-none d-sm-none d-md-block btn btn-grouped btn-spam', title: 'Submit as spam'
|
= link_to 'Submit as spam', mark_as_spam_project_issue_path(@project, @issue), method: :post, class: 'd-none d-sm-none d-md-block btn btn-grouped btn-spam', title: 'Submit as spam'
|
||||||
|
|
|
@ -39,4 +39,4 @@
|
||||||
- if can_update_merge_request
|
- if can_update_merge_request
|
||||||
= link_to 'Edit', edit_project_merge_request_path(@project, @merge_request), class: "d-none d-sm-none d-md-block btn btn-grouped js-issuable-edit"
|
= link_to 'Edit', edit_project_merge_request_path(@project, @merge_request), class: "d-none d-sm-none d-md-block btn btn-grouped js-issuable-edit"
|
||||||
|
|
||||||
= render 'shared/issuable/close_reopen_button', issuable: @merge_request, can_update: can_update_merge_request
|
= render 'shared/issuable/close_reopen_button', issuable: @merge_request, can_update: can_update_merge_request, can_reopen: can_update_merge_request
|
||||||
|
|
|
@ -2,13 +2,15 @@
|
||||||
- display_issuable_type = issuable_display_type(issuable)
|
- display_issuable_type = issuable_display_type(issuable)
|
||||||
- button_method = issuable_close_reopen_button_method(issuable)
|
- button_method = issuable_close_reopen_button_method(issuable)
|
||||||
|
|
||||||
- if can_update && is_current_user
|
- if can_update
|
||||||
|
- if is_current_user
|
||||||
= link_to "Close #{display_issuable_type}", close_issuable_path(issuable), method: button_method,
|
= link_to "Close #{display_issuable_type}", close_issuable_path(issuable), method: button_method,
|
||||||
class: "d-none d-sm-none d-md-block btn btn-grouped btn-close js-btn-issue-action #{issuable_button_visibility(issuable, true)}", title: "Close #{display_issuable_type}"
|
class: "d-none d-sm-none d-md-block btn btn-grouped btn-close js-btn-issue-action #{issuable_button_visibility(issuable, true)}", title: "Close #{display_issuable_type}"
|
||||||
|
- else
|
||||||
|
= render 'shared/issuable/close_reopen_report_toggle', issuable: issuable
|
||||||
|
- if can_reopen && is_current_user
|
||||||
= link_to "Reopen #{display_issuable_type}", reopen_issuable_path(issuable), method: button_method,
|
= link_to "Reopen #{display_issuable_type}", reopen_issuable_path(issuable), method: button_method,
|
||||||
class: "d-none d-sm-none d-md-block btn btn-grouped btn-reopen js-btn-issue-action #{issuable_button_visibility(issuable, false)}", title: "Reopen #{display_issuable_type}"
|
class: "d-none d-sm-none d-md-block btn btn-grouped btn-reopen js-btn-issue-action #{issuable_button_visibility(issuable, false)}", title: "Reopen #{display_issuable_type}"
|
||||||
- elsif can_update && !is_current_user
|
|
||||||
= render 'shared/issuable/close_reopen_report_toggle', issuable: issuable
|
|
||||||
- else
|
- else
|
||||||
= link_to 'Report abuse', new_abuse_report_path(user_id: issuable.author.id, ref_url: issuable_url(issuable)),
|
= link_to 'Report abuse', new_abuse_report_path(user_id: issuable.author.id, ref_url: issuable_url(issuable)),
|
||||||
class: 'd-none d-sm-none d-md-block btn btn-grouped btn-close-color', title: 'Report abuse'
|
class: 'd-none d-sm-none d-md-block btn btn-grouped btn-close-color', title: 'Report abuse'
|
||||||
|
|
5
changelogs/unreleased/39665-restrict-issue-reopen.yml
Normal file
5
changelogs/unreleased/39665-restrict-issue-reopen.yml
Normal file
|
@ -0,0 +1,5 @@
|
||||||
|
---
|
||||||
|
title: Restrict reopening locked issues for non authorized issue authors
|
||||||
|
merge_request: 21299
|
||||||
|
author:
|
||||||
|
type: changed
|
|
@ -271,6 +271,8 @@ edit existing comments. Non-team members are restricted from adding or editing c
|
||||||
| :-----------: | :----------: |
|
| :-----------: | :----------: |
|
||||||
| ![Comment form member](img/lock_form_member.png) | ![Comment form non-member](img/lock_form_non_member.png) |
|
| ![Comment form member](img/lock_form_member.png) | ![Comment form non-member](img/lock_form_non_member.png) |
|
||||||
|
|
||||||
|
Additionally locked issues can not be reopened.
|
||||||
|
|
||||||
[ce-5022]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5022
|
[ce-5022]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5022
|
||||||
[ce-7125]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7125
|
[ce-7125]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7125
|
||||||
[ce-7527]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7527
|
[ce-7527]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7527
|
||||||
|
|
|
@ -112,6 +112,7 @@ describe IssuePolicy do
|
||||||
let(:project) { create(:project, :public) }
|
let(:project) { create(:project, :public) }
|
||||||
let(:issue) { create(:issue, project: project, assignees: [assignee], author: author) }
|
let(:issue) { create(:issue, project: project, assignees: [assignee], author: author) }
|
||||||
let(:issue_no_assignee) { create(:issue, project: project) }
|
let(:issue_no_assignee) { create(:issue, project: project) }
|
||||||
|
let(:issue_locked) { create(:issue, project: project, discussion_locked: true, author: author, assignees: [assignee]) }
|
||||||
|
|
||||||
before do
|
before do
|
||||||
project.add_guest(guest)
|
project.add_guest(guest)
|
||||||
|
@ -124,36 +125,49 @@ describe IssuePolicy do
|
||||||
|
|
||||||
it 'allows guests to read issues' do
|
it 'allows guests to read issues' do
|
||||||
expect(permissions(guest, issue)).to be_allowed(:read_issue, :read_issue_iid)
|
expect(permissions(guest, issue)).to be_allowed(:read_issue, :read_issue_iid)
|
||||||
expect(permissions(guest, issue)).to be_disallowed(:update_issue, :admin_issue)
|
expect(permissions(guest, issue)).to be_disallowed(:update_issue, :admin_issue, :reopen_issue)
|
||||||
|
|
||||||
expect(permissions(guest, issue_no_assignee)).to be_allowed(:read_issue, :read_issue_iid)
|
expect(permissions(guest, issue_no_assignee)).to be_allowed(:read_issue, :read_issue_iid)
|
||||||
expect(permissions(guest, issue_no_assignee)).to be_disallowed(:update_issue, :admin_issue)
|
expect(permissions(guest, issue_no_assignee)).to be_disallowed(:update_issue, :admin_issue, :reopen_issue)
|
||||||
|
|
||||||
|
expect(permissions(guest, issue_locked)).to be_allowed(:read_issue, :read_issue_iid)
|
||||||
|
expect(permissions(guest, issue_locked)).to be_disallowed(:update_issue, :admin_issue, :reopen_issue)
|
||||||
end
|
end
|
||||||
|
|
||||||
it 'allows reporters to read, update, and admin issues' do
|
it 'allows reporters to read, update, reopen, and admin issues' do
|
||||||
expect(permissions(reporter, issue)).to be_allowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue)
|
expect(permissions(reporter, issue)).to be_allowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :reopen_issue)
|
||||||
expect(permissions(reporter, issue_no_assignee)).to be_allowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue)
|
expect(permissions(reporter, issue_no_assignee)).to be_allowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :reopen_issue)
|
||||||
|
expect(permissions(reporter, issue_locked)).to be_allowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue)
|
||||||
|
expect(permissions(reporter, issue_locked)).to be_disallowed(:reopen_issue)
|
||||||
end
|
end
|
||||||
|
|
||||||
it 'allows reporters from group links to read, update, and admin issues' do
|
it 'allows reporters from group links to read, update, reopen and admin issues' do
|
||||||
expect(permissions(reporter_from_group_link, issue)).to be_allowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue)
|
expect(permissions(reporter_from_group_link, issue)).to be_allowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :reopen_issue)
|
||||||
expect(permissions(reporter_from_group_link, issue_no_assignee)).to be_allowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue)
|
expect(permissions(reporter_from_group_link, issue_no_assignee)).to be_allowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :reopen_issue)
|
||||||
|
expect(permissions(reporter_from_group_link, issue_locked)).to be_allowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue)
|
||||||
|
expect(permissions(reporter_from_group_link, issue_locked)).to be_disallowed(:reopen_issue)
|
||||||
end
|
end
|
||||||
|
|
||||||
it 'allows issue authors to read and update their issues' do
|
it 'allows issue authors to read, reopen and update their issues' do
|
||||||
expect(permissions(author, issue)).to be_allowed(:read_issue, :read_issue_iid, :update_issue)
|
expect(permissions(author, issue)).to be_allowed(:read_issue, :read_issue_iid, :update_issue, :reopen_issue)
|
||||||
expect(permissions(author, issue)).to be_disallowed(:admin_issue)
|
expect(permissions(author, issue)).to be_disallowed(:admin_issue)
|
||||||
|
|
||||||
expect(permissions(author, issue_no_assignee)).to be_allowed(:read_issue, :read_issue_iid)
|
expect(permissions(author, issue_no_assignee)).to be_allowed(:read_issue, :read_issue_iid)
|
||||||
expect(permissions(author, issue_no_assignee)).to be_disallowed(:update_issue, :admin_issue)
|
expect(permissions(author, issue_no_assignee)).to be_disallowed(:update_issue, :admin_issue, :reopen_issue)
|
||||||
|
|
||||||
|
expect(permissions(author, issue_locked)).to be_allowed(:read_issue, :read_issue_iid, :update_issue)
|
||||||
|
expect(permissions(author, issue_locked)).to be_disallowed(:admin_issue, :reopen_issue)
|
||||||
end
|
end
|
||||||
|
|
||||||
it 'allows issue assignees to read and update their issues' do
|
it 'allows issue assignees to read, reopen and update their issues' do
|
||||||
expect(permissions(assignee, issue)).to be_allowed(:read_issue, :read_issue_iid, :update_issue)
|
expect(permissions(assignee, issue)).to be_allowed(:read_issue, :read_issue_iid, :update_issue, :reopen_issue)
|
||||||
expect(permissions(assignee, issue)).to be_disallowed(:admin_issue)
|
expect(permissions(assignee, issue)).to be_disallowed(:admin_issue)
|
||||||
|
|
||||||
expect(permissions(assignee, issue_no_assignee)).to be_allowed(:read_issue, :read_issue_iid)
|
expect(permissions(assignee, issue_no_assignee)).to be_allowed(:read_issue, :read_issue_iid)
|
||||||
expect(permissions(assignee, issue_no_assignee)).to be_disallowed(:update_issue, :admin_issue)
|
expect(permissions(assignee, issue_no_assignee)).to be_disallowed(:update_issue, :admin_issue, :reopen_issue)
|
||||||
|
|
||||||
|
expect(permissions(assignee, issue_locked)).to be_allowed(:read_issue, :read_issue_iid, :update_issue)
|
||||||
|
expect(permissions(assignee, issue_locked)).to be_disallowed(:admin_issue, :reopen_issue)
|
||||||
end
|
end
|
||||||
|
|
||||||
context 'with confidential issues' do
|
context 'with confidential issues' do
|
||||||
|
|
Loading…
Reference in a new issue