Commit ec43e364 authored by Bob Van Landuyt's avatar Bob Van Landuyt

Prevent new merge requests for archived projects

This prevents creating merge requests targeting archived projects.

This could happen when a project was already forked, but then the
source was archived.
parent 267dba0a
...@@ -34,8 +34,12 @@ class Projects::ApplicationController < ApplicationController ...@@ -34,8 +34,12 @@ class Projects::ApplicationController < ApplicationController
def can_collaborate_with_project?(project = nil, ref: nil) def can_collaborate_with_project?(project = nil, ref: nil)
project ||= @project project ||= @project
can_create_merge_request =
can?(current_user, :create_merge_request_in_project, project) &&
current_user.already_forked?(project)
can?(current_user, :push_code, project) || can?(current_user, :push_code, project) ||
(current_user && current_user.already_forked?(project)) || can_create_merge_request ||
user_access(project).can_push_to_branch?(ref) user_access(project).can_push_to_branch?(ref)
end end
......
...@@ -12,6 +12,7 @@ class MergeRequestTargetProjectFinder ...@@ -12,6 +12,7 @@ class MergeRequestTargetProjectFinder
if @source_project.fork_network if @source_project.fork_network
@source_project.fork_network.projects @source_project.fork_network.projects
.public_or_visible_to_user(current_user) .public_or_visible_to_user(current_user)
.non_archived
.with_feature_available_for_user(:merge_requests, current_user) .with_feature_available_for_user(:merge_requests, current_user)
else else
Project.where(id: source_project) Project.where(id: source_project)
......
...@@ -59,7 +59,7 @@ module BlobHelper ...@@ -59,7 +59,7 @@ module BlobHelper
button_tag label, class: "#{common_classes} disabled has-tooltip", title: "It is not possible to #{action} files that are stored in LFS using the web interface", data: { container: 'body' } button_tag label, class: "#{common_classes} disabled has-tooltip", title: "It is not possible to #{action} files that are stored in LFS using the web interface", data: { container: 'body' }
elsif can_modify_blob?(blob, project, ref) elsif can_modify_blob?(blob, project, ref)
button_tag label, class: "#{common_classes}", 'data-target' => "#modal-#{modal_type}-blob", 'data-toggle' => 'modal' button_tag label, class: "#{common_classes}", 'data-target' => "#modal-#{modal_type}-blob", 'data-toggle' => 'modal'
elsif can?(current_user, :fork_project, project) elsif can?(current_user, :create_merge_request_in_project, project)
edit_fork_button_tag(common_classes, project, label, edit_modify_file_fork_params(action), action) edit_fork_button_tag(common_classes, project, label, edit_modify_file_fork_params(action), action)
end end
end end
...@@ -334,7 +334,7 @@ module BlobHelper ...@@ -334,7 +334,7 @@ module BlobHelper
# Web IDE (Beta) requires the user to have this feature enabled # Web IDE (Beta) requires the user to have this feature enabled
elsif !current_user || (current_user && can_modify_blob?(blob, project, ref)) elsif !current_user || (current_user && can_modify_blob?(blob, project, ref))
edit_link_tag(text, edit_path, common_classes) edit_link_tag(text, edit_path, common_classes)
elsif current_user && can?(current_user, :fork_project, project) elsif can?(current_user, :create_merge_request_in_project, project)
edit_fork_button_tag(common_classes, project, text, edit_blob_fork_params(edit_path)) edit_fork_button_tag(common_classes, project, text, edit_blob_fork_params(edit_path))
end end
end end
......
...@@ -140,6 +140,7 @@ class ProjectPolicy < BasePolicy ...@@ -140,6 +140,7 @@ class ProjectPolicy < BasePolicy
rule { can?(:guest_access) }.policy do rule { can?(:guest_access) }.policy do
enable :read_project enable :read_project
enable :create_merge_request_in_project
enable :read_board enable :read_board
enable :read_list enable :read_list
enable :read_wiki enable :read_wiki
...@@ -250,6 +251,7 @@ class ProjectPolicy < BasePolicy ...@@ -250,6 +251,7 @@ class ProjectPolicy < BasePolicy
prevent :request_access prevent :request_access
prevent :upload_file prevent :upload_file
prevent :resolve_note prevent :resolve_note
prevent :create_merge_request_in_project
READONLY_FEATURES_WHEN_ARCHIVED.each do |feature| READONLY_FEATURES_WHEN_ARCHIVED.each do |feature|
prevent(*create_update_admin_destroy(feature)) prevent(*create_update_admin_destroy(feature))
...@@ -261,6 +263,7 @@ class ProjectPolicy < BasePolicy ...@@ -261,6 +263,7 @@ class ProjectPolicy < BasePolicy
end end
rule { merge_requests_disabled | repository_disabled }.policy do rule { merge_requests_disabled | repository_disabled }.policy do
prevent :create_merge_request_in_project
prevent(*create_read_update_admin_destroy(:merge_request)) prevent(*create_read_update_admin_destroy(:merge_request))
end end
...@@ -306,6 +309,7 @@ class ProjectPolicy < BasePolicy ...@@ -306,6 +309,7 @@ class ProjectPolicy < BasePolicy
rule { can?(:public_access) }.policy do rule { can?(:public_access) }.policy do
enable :read_project enable :read_project
enable :create_merge_request_in_project
enable :read_board enable :read_board
enable :read_list enable :read_list
enable :read_wiki enable :read_wiki
......
...@@ -196,8 +196,12 @@ class MergeRequestPresenter < Gitlab::View::Presenter::Delegated ...@@ -196,8 +196,12 @@ class MergeRequestPresenter < Gitlab::View::Presenter::Delegated
end end
def user_can_collaborate_with_project? def user_can_collaborate_with_project?
can_create_merge_request =
can?(current_user, :create_merge_request_in_project, project) &&
current_user.already_forked?(project)
can?(current_user, :push_code, project) || can?(current_user, :push_code, project) ||
(current_user && current_user.already_forked?(project)) || can_create_merge_request ||
can_push_to_source_branch? can_push_to_source_branch?
end end
......
...@@ -72,7 +72,7 @@ module MergeRequests ...@@ -72,7 +72,7 @@ module MergeRequests
params.delete(:target_project_id) params.delete(:target_project_id)
unless can?(current_user, :read_project, @source_project) && unless can?(current_user, :read_project, @source_project) &&
can?(current_user, :read_project, @project) can?(current_user, :create_merge_request_in_project, @project)
raise Gitlab::Access::AccessDeniedError raise Gitlab::Access::AccessDeniedError
end end
......
...@@ -40,6 +40,14 @@ describe 'Merge request > User cherry-picks', :js do ...@@ -40,6 +40,14 @@ describe 'Merge request > User cherry-picks', :js do
expect(page).to have_link 'Cherry-pick' expect(page).to have_link 'Cherry-pick'
end end
it 'hides the cherry pick button for an archived project' do
project.update!(archived: true)
visit project_merge_request_path(project, merge_request)
expect(page).not_to have_link 'Cherry-pick'
end
end end
end end
end end
...@@ -12,6 +12,27 @@ describe 'Projects > Files > User edits files' do ...@@ -12,6 +12,27 @@ describe 'Projects > Files > User edits files' do
sign_in(user) sign_in(user)
end end
shared_examples 'unavailable for an archived project' do
it 'does not show the edit link for an archived project', :js do
project.update!(archived: true)
visit project_tree_path(project, project.repository.root_ref)
click_link('.gitignore')
aggregate_failures 'available edit buttons' do
# We're showing a link, if the user can edit directly, this is becomes a
# button when the user can fork the project.
expect(page).not_to have_link('Edit')
expect(page).not_to have_button('Edit')
expect(page).not_to have_link('Web IDE')
expect(page).not_to have_button('Web IDE')
expect(page).not_to have_button('Replace')
expect(page).not_to have_button('Delete')
end
end
end
context 'when an user has write access' do context 'when an user has write access' do
before do before do
project.add_master(user) project.add_master(user)
...@@ -85,6 +106,8 @@ describe 'Projects > Files > User edits files' do ...@@ -85,6 +106,8 @@ describe 'Projects > Files > User edits files' do
expect(page).to have_css('.line_holder.new') expect(page).to have_css('.line_holder.new')
end end
it_behaves_like 'unavailable for an archived project'
end end
context 'when an user does not have write access' do context 'when an user does not have write access' do
...@@ -168,6 +191,10 @@ describe 'Projects > Files > User edits files' do ...@@ -168,6 +191,10 @@ describe 'Projects > Files > User edits files' do
expect(page).to have_content("From #{forked_project.full_path}") expect(page).to have_content("From #{forked_project.full_path}")
expect(page).to have_content("into #{project2.full_path}") expect(page).to have_content("into #{project2.full_path}")
end end
it_behaves_like 'unavailable for an archived project' do
let(:project) { project2 }
end
end end
end end
end end
...@@ -45,6 +45,18 @@ feature 'Merge Request button' do ...@@ -45,6 +45,18 @@ feature 'Merge Request button' do
end end
end end
end end
context 'when the project is archived' do
it 'hides the link' do
project.update!(archived: true)
visit url
within("#content-body") do
expect(page).not_to have_link(label)
end
end
end
end end
context 'logged in as non-member' do context 'logged in as non-member' do
......
...@@ -56,4 +56,12 @@ describe 'User reverts a merge request', :js do ...@@ -56,4 +56,12 @@ describe 'User reverts a merge request', :js do
expect(page).to have_content('The merge request has been successfully reverted. You can now submit a merge request to get this change into the original branch.') expect(page).to have_content('The merge request has been successfully reverted. You can now submit a merge request to get this change into the original branch.')
end end
it 'cannot revert a merge requests for an archived project' do
project.update!(archived: true)
visit(merge_request_path(merge_request))
expect(page).not_to have_link('Revert')
end
end end
...@@ -19,6 +19,12 @@ describe MergeRequestTargetProjectFinder do ...@@ -19,6 +19,12 @@ describe MergeRequestTargetProjectFinder do
expect(finder.execute).to contain_exactly(forked_project) expect(finder.execute).to contain_exactly(forked_project)
end end
it 'does not contain archived projects' do
base_project.update!(archived: true)
expect(finder.execute).to contain_exactly(other_fork, forked_project)
end
end end
context 'public projects' do context 'public projects' do
......
...@@ -14,7 +14,7 @@ describe ProjectPolicy do ...@@ -14,7 +14,7 @@ describe ProjectPolicy do
read_project read_board read_list read_wiki read_issue read_project read_board read_list read_wiki read_issue
read_project_for_iids read_issue_iid read_merge_request_iid read_label read_project_for_iids read_issue_iid read_merge_request_iid read_label
read_milestone read_project_snippet read_project_member read_note read_milestone read_project_snippet read_project_member read_note
create_project create_issue create_note upload_file create_project create_issue create_note upload_file create_merge_request_in_project
] ]
end end
...@@ -136,6 +136,20 @@ describe ProjectPolicy do ...@@ -136,6 +136,20 @@ describe ProjectPolicy do
end end
end end
context 'merge requests feature' do
subject { described_class.new(owner, project) }
it 'disallows all permissions when the feature is disabled' do
project.project_feature.update(merge_requests_access_level: ProjectFeature::DISABLED)
mr_permissions = [:create_merge_request, :read_merge_request,
:update_merge_request, :admin_merge_request,
:create_merge_request_in_project]
expect_disallowed(*mr_permissions)
end
end
shared_examples 'archived project policies' do shared_examples 'archived project policies' do
let(:feature_write_abilities) do let(:feature_write_abilities) do
described_class::READONLY_FEATURES_WHEN_ARCHIVED.flat_map do |feature| described_class::READONLY_FEATURES_WHEN_ARCHIVED.flat_map do |feature|
...@@ -145,6 +159,7 @@ describe ProjectPolicy do ...@@ -145,6 +159,7 @@ describe ProjectPolicy do
let(:other_write_abilities) do let(:other_write_abilities) do
%i[ %i[
create_merge_request_in_project
push_to_delete_protected_branch push_to_delete_protected_branch
push_code push_code
request_access request_access
......
require 'spec_helper' require 'spec_helper'
describe MergeRequests::CreateService do describe MergeRequests::CreateService do
include ProjectForksHelper
let(:project) { create(:project, :repository) } let(:project) { create(:project, :repository) }
let(:user) { create(:user) } let(:user) { create(:user) }
let(:assignee) { create(:user) } let(:assignee) { create(:user) }
...@@ -300,7 +302,7 @@ describe MergeRequests::CreateService do ...@@ -300,7 +302,7 @@ describe MergeRequests::CreateService do
end end
context 'when source and target projects are different' do context 'when source and target projects are different' do
let(:target_project) { create(:project) } let(:target_project) { fork_project(project, nil, repository: true) }
let(:opts) do let(:opts) do
{ {
...@@ -334,6 +336,26 @@ describe MergeRequests::CreateService do ...@@ -334,6 +336,26 @@ describe MergeRequests::CreateService do
.to raise_error Gitlab::Access::AccessDeniedError .to raise_error Gitlab::Access::AccessDeniedError
end end
end end
context 'when the user has access to both projects' do
before do
target_project.add_developer(user)
project.add_developer(user)
end
it 'creates the merge request' do
merge_request = described_class.new(project, user, opts).execute
expect(merge_request).to be_persisted
end
it 'does not create the merge request when the target project is archived' do
target_project.update!(archived: true)
expect { described_class.new(project, user, opts).execute }
.to raise_error Gitlab::Access::AccessDeniedError
end
end
end end
context 'when user sets source project id' do context 'when user sets source project id' do
......
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