Commit eba4b940 authored by Jan Beckmann's avatar Jan Beckmann Committed by Sean McGivern

Disallow reopening of locked merge requests

Fixes #56864
parent 66481881
......@@ -17,6 +17,7 @@ class IssuablePolicy < BasePolicy
enable :reopen_issue
enable :read_merge_request
enable :update_merge_request
enable :reopen_merge_request
end
rule { locked & ~is_project_member }.policy do
......
# frozen_string_literal: true
class MergeRequestPolicy < IssuablePolicy
rule { locked }.policy do
prevent :reopen_merge_request
end
end
......@@ -231,6 +231,7 @@ class ProjectPolicy < BasePolicy
enable :admin_merge_request
enable :admin_milestone
enable :update_merge_request
enable :reopen_merge_request
enable :create_commit_status
enable :update_commit_status
enable :create_build
......
......@@ -3,7 +3,7 @@
module MergeRequests
class ReopenService < MergeRequests::BaseService
def execute(merge_request)
return merge_request unless can?(current_user, :update_merge_request, merge_request)
return merge_request unless can?(current_user, :reopen_merge_request, merge_request)
if merge_request.reopen
create_event(merge_request)
......
- can_update_merge_request = can?(current_user, :update_merge_request, @merge_request)
- can_reopen_merge_request = can?(current_user, :reopen_merge_request, @merge_request)
- if @merge_request.closed_without_fork?
.alert.alert-danger
......@@ -33,10 +34,11 @@
- if can_update_merge_request
%li{ class: [merge_request_button_visibility(@merge_request, true), 'js-close-item'] }
= link_to 'Close', merge_request_path(@merge_request, merge_request: { state_event: :close }), method: :put, title: 'Close merge request'
- if can_reopen_merge_request
%li{ class: merge_request_button_visibility(@merge_request, false) }
= link_to 'Reopen', merge_request_path(@merge_request, merge_request: { state_event: :reopen }), method: :put, class: 'reopen-mr-link', title: 'Reopen 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 qa-edit-button"
= render 'shared/issuable/close_reopen_button', issuable: @merge_request, can_update: can_update_merge_request, can_reopen: can_update_merge_request
= render 'shared/issuable/close_reopen_button', issuable: @merge_request, can_update: can_update_merge_request, can_reopen: can_reopen_merge_request
---
title: Disallow reopening of a locked merge request
merge_request: 24882
author: Jan Beckmann
type: fixed
......@@ -276,7 +276,7 @@ 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) |
Additionally locked issues can not be reopened.
Additionally, locked issues and merge requests can not be reopened.
## Filtering notes
......
......@@ -13,7 +13,7 @@ describe IssuablePolicy, models: true do
context 'when user is able to read project' do
it 'enables user to read and update issuables' do
expect(policies).to be_allowed(:read_issue, :update_issue, :reopen_issue, :read_merge_request, :update_merge_request)
expect(policies).to be_allowed(:read_issue, :update_issue, :reopen_issue, :read_merge_request, :update_merge_request, :reopen_merge_request)
end
end
......@@ -24,12 +24,12 @@ describe IssuablePolicy, models: true do
it 'enables user to read and update issuables' do
project.add_maintainer(user)
expect(policies).to be_allowed(:read_issue, :update_issue, :reopen_issue, :read_merge_request, :update_merge_request)
expect(policies).to be_allowed(:read_issue, :update_issue, :reopen_issue, :read_merge_request, :update_merge_request, :reopen_merge_request)
end
end
it 'disallows user from reading and updating issuables from that project' do
expect(policies).to be_disallowed(:read_issue, :update_issue, :reopen_issue, :read_merge_request, :update_merge_request)
expect(policies).to be_disallowed(:read_issue, :update_issue, :reopen_issue, :read_merge_request, :update_merge_request, :reopen_merge_request)
end
end
end
......
require 'spec_helper'
describe MergeRequestPolicy do
let(:guest) { create(:user) }
let(:author) { create(:user) }
let(:developer) { create(:user) }
let(:project) { create(:project, :public) }
def permissions(user, merge_request)
described_class.new(user, merge_request)
end
before do
project.add_guest(guest)
project.add_guest(author)
project.add_developer(developer)
end
context 'when merge request is unlocked' do
let(:merge_request) { create(:merge_request, :closed, source_project: project, target_project: project, author: author) }
it 'allows author to reopen merge request' do
expect(permissions(author, merge_request)).to be_allowed(:reopen_merge_request)
end
it 'allows developer to reopen merge request' do
expect(permissions(developer, merge_request)).to be_allowed(:reopen_merge_request)
end
it 'prevents guest from reopening merge request' do
expect(permissions(guest, merge_request)).to be_disallowed(:reopen_merge_request)
end
end
context 'when merge request is locked' do
let(:merge_request_locked) { create(:merge_request, :closed, discussion_locked: true, source_project: project, target_project: project, author: author) }
it 'prevents author from reopening merge request' do
expect(permissions(author, merge_request_locked)).to be_disallowed(:reopen_merge_request)
end
it 'prevents developer from reopening merge request' do
expect(permissions(developer, merge_request_locked)).to be_disallowed(:reopen_merge_request)
end
it 'prevents guests from reopening merge request' do
expect(permissions(guest, merge_request_locked)).to be_disallowed(:reopen_merge_request)
end
end
end
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