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.

13 files changed, 118 insertions, 7 deletions

diff --git a/app/controllers/projects/application_controller.rb b/app/controllers/projects/application_controller.rb
index 6d9b42a2c04..cbb14b55399 100644
--- a/app/controllers/projects/application_controller.rb
+++ b/app/controllers/projects/application_controller.rb
@@ -34,8 +34,12 @@ class Projects::ApplicationController < ApplicationController
   def can_collaborate_with_project?(project = nil, ref: nil)
     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) ||
-      (current_user && current_user.already_forked?(project)) ||
+      can_create_merge_request ||
       user_access(project).can_push_to_branch?(ref)
   end
 
diff --git a/app/finders/merge_request_target_project_finder.rb b/app/finders/merge_request_target_project_finder.rb
index f358938344e..188ec447a94 100644
--- a/app/finders/merge_request_target_project_finder.rb
+++ b/app/finders/merge_request_target_project_finder.rb
@@ -12,6 +12,7 @@ class MergeRequestTargetProjectFinder
     if @source_project.fork_network
       @source_project.fork_network.projects
         .public_or_visible_to_user(current_user)
+        .non_archived
         .with_feature_available_for_user(:merge_requests, current_user)
     else
       Project.where(id: source_project)
diff --git a/app/helpers/blob_helper.rb b/app/helpers/blob_helper.rb
index 2b440e4d584..ac7b8d6672e 100644
--- a/app/helpers/blob_helper.rb
+++ b/app/helpers/blob_helper.rb
@@ -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' }
     elsif can_modify_blob?(blob, project, ref)
       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)
     end
   end
@@ -334,7 +334,7 @@ module BlobHelper
       # Web IDE (Beta) requires the user to have this feature enabled
     elsif !current_user || (current_user && can_modify_blob?(blob, project, ref))
       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))
     end
   end
diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb
index b4970b605ca..72e30c932a9 100644
--- a/app/policies/project_policy.rb
+++ b/app/policies/project_policy.rb
@@ -140,6 +140,7 @@ class ProjectPolicy < BasePolicy
 
   rule { can?(:guest_access) }.policy do
     enable :read_project
+    enable :create_merge_request_in_project
     enable :read_board
     enable :read_list
     enable :read_wiki
@@ -250,6 +251,7 @@ class ProjectPolicy < BasePolicy
     prevent :request_access
     prevent :upload_file
     prevent :resolve_note
+    prevent :create_merge_request_in_project
 
     READONLY_FEATURES_WHEN_ARCHIVED.each do |feature|
       prevent(*create_update_admin_destroy(feature))
@@ -261,6 +263,7 @@ class ProjectPolicy < BasePolicy
   end
 
   rule { merge_requests_disabled | repository_disabled }.policy do
+    prevent :create_merge_request_in_project
     prevent(*create_read_update_admin_destroy(:merge_request))
   end
 
@@ -306,6 +309,7 @@ class ProjectPolicy < BasePolicy
 
   rule { can?(:public_access) }.policy do
     enable :read_project
+    enable :create_merge_request_in_project
     enable :read_board
     enable :read_list
     enable :read_wiki
diff --git a/app/presenters/merge_request_presenter.rb b/app/presenters/merge_request_presenter.rb
index 9f3f2637183..950d3fde2ea 100644
--- a/app/presenters/merge_request_presenter.rb
+++ b/app/presenters/merge_request_presenter.rb
@@ -196,8 +196,12 @@ class MergeRequestPresenter < Gitlab::View::Presenter::Delegated
   end
 
   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) ||
-      (current_user && current_user.already_forked?(project)) ||
+      can_create_merge_request ||
       can_push_to_source_branch?
   end
 
diff --git a/app/services/merge_requests/create_service.rb b/app/services/merge_requests/create_service.rb
index c57a2445341..4c2c8398461 100644
--- a/app/services/merge_requests/create_service.rb
+++ b/app/services/merge_requests/create_service.rb
@@ -72,7 +72,7 @@ module MergeRequests
       params.delete(:target_project_id)
 
       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
       end
diff --git a/spec/features/merge_request/user_cherry_picks_spec.rb b/spec/features/merge_request/user_cherry_picks_spec.rb
index 494096b21c0..61d1bdaa95a 100644
--- a/spec/features/merge_request/user_cherry_picks_spec.rb
+++ b/spec/features/merge_request/user_cherry_picks_spec.rb
@@ -40,6 +40,14 @@ describe 'Merge request > User cherry-picks', :js do
 
         expect(page).to have_link 'Cherry-pick'
       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
diff --git a/spec/features/projects/files/user_edits_files_spec.rb b/spec/features/projects/files/user_edits_files_spec.rb
index 523a9f3f4fe..ee9deb30890 100644
--- a/spec/features/projects/files/user_edits_files_spec.rb
+++ b/spec/features/projects/files/user_edits_files_spec.rb
@@ -12,6 +12,27 @@ describe 'Projects > Files > User edits files' do
     sign_in(user)
   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
     before do
       project.add_master(user)
@@ -85,6 +106,8 @@ describe 'Projects > Files > User edits files' do
 
       expect(page).to have_css('.line_holder.new')
     end
+
+    it_behaves_like 'unavailable for an archived project'
   end
 
   context 'when an user does not have write access' 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("into #{project2.full_path}")
       end
+
+      it_behaves_like 'unavailable for an archived project' do
+        let(:project) { project2 }
+      end
     end
   end
 end
diff --git a/spec/features/projects/merge_request_button_spec.rb b/spec/features/projects/merge_request_button_spec.rb
index 40689964b91..b571d5a0e26 100644
--- a/spec/features/projects/merge_request_button_spec.rb
+++ b/spec/features/projects/merge_request_button_spec.rb
@@ -45,6 +45,18 @@ feature 'Merge Request button' do
           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
 
     context 'logged in as non-member' do
diff --git a/spec/features/projects/merge_requests/user_reverts_merge_request_spec.rb b/spec/features/projects/merge_requests/user_reverts_merge_request_spec.rb
index a41d683dbbb..f3e97bc9eb2 100644
--- a/spec/features/projects/merge_requests/user_reverts_merge_request_spec.rb
+++ b/spec/features/projects/merge_requests/user_reverts_merge_request_spec.rb
@@ -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.')
   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
diff --git a/spec/finders/merge_request_target_project_finder_spec.rb b/spec/finders/merge_request_target_project_finder_spec.rb
index c81bfd7932c..f302cf80ce8 100644
--- a/spec/finders/merge_request_target_project_finder_spec.rb
+++ b/spec/finders/merge_request_target_project_finder_spec.rb
@@ -19,6 +19,12 @@ describe MergeRequestTargetProjectFinder do
 
       expect(finder.execute).to contain_exactly(forked_project)
     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
 
   context 'public projects' do
diff --git a/spec/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb
index f20e4a021da..4c01c320841 100644
--- a/spec/policies/project_policy_spec.rb
+++ b/spec/policies/project_policy_spec.rb
@@ -14,7 +14,7 @@ describe ProjectPolicy do
       read_project read_board read_list read_wiki read_issue
       read_project_for_iids read_issue_iid read_merge_request_iid read_label
       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
 
@@ -136,6 +136,20 @@ describe ProjectPolicy do
     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
     let(:feature_write_abilities) do
       described_class::READONLY_FEATURES_WHEN_ARCHIVED.flat_map do |feature|
@@ -145,6 +159,7 @@ describe ProjectPolicy do
 
     let(:other_write_abilities) do
       %i[
+        create_merge_request_in_project
         push_to_delete_protected_branch
         push_code
         request_access
diff --git a/spec/services/merge_requests/create_service_spec.rb b/spec/services/merge_requests/create_service_spec.rb
index 44a83c436cb..736a50b2c15 100644
--- a/spec/services/merge_requests/create_service_spec.rb
+++ b/spec/services/merge_requests/create_service_spec.rb
@@ -1,6 +1,8 @@
 require 'spec_helper'
 
 describe MergeRequests::CreateService do
+  include ProjectForksHelper
+
   let(:project) { create(:project, :repository) }
   let(:user) { create(:user) }
   let(:assignee) { create(:user) }
@@ -300,7 +302,7 @@ describe MergeRequests::CreateService do
     end
 
     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
         {
@@ -334,6 +336,26 @@ describe MergeRequests::CreateService do
             .to raise_error Gitlab::Access::AccessDeniedError
         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
 
     context 'when user sets source project id' do