Update policies to make archived projects completely read-only

app/policies/ci/build_policy.rb

@@ -11,7 +11,7 @@ module Ci
     end
 
     condition(:owner_of_job) do
-      can?(:developer_access) && @subject.triggered_by?(@user)
+      @subject.triggered_by?(@user)
     end
 
     rule { protected_ref }.policy do
@@ -19,6 +19,6 @@ module Ci
       prevent :erase_build
     end
 
-    rule { can?(:master_access) | owner_of_job }.enable :erase_build
+    rule { can?(:admin_build) | (can?(:update_build) & owner_of_job) }.enable :erase_build
   end
 end

app/policies/ci/pipeline_schedule_policy.rb

@@ -7,23 +7,17 @@ module Ci
     end
 
     condition(:owner_of_schedule) do
-      can?(:developer_access) && pipeline_schedule.owned_by?(@user)
+      pipeline_schedule.owned_by?(@user)
     end
 
-    condition(:non_owner_of_schedule) do
-      !pipeline_schedule.owned_by?(@user)
-    end
+    rule { can?(:create_pipeline) }.enable :play_pipeline_schedule
 
-    rule { can?(:developer_access) }.policy do
-      enable :play_pipeline_schedule
-    end
-
-    rule { can?(:master_access) | owner_of_schedule }.policy do
+    rule { can?(:admin_pipeline) | (can?(:update_build) & owner_of_schedule) }.policy do
       enable :update_pipeline_schedule
       enable :admin_pipeline_schedule
     end
 
-    rule { can?(:master_access) & non_owner_of_schedule }.policy do
+    rule { can?(:admin_pipeline_schedule) & ~owner_of_schedule }.policy do
       enable :take_ownership_pipeline_schedule
     end
 

app/policies/note_policy.rb

@@ -3,7 +3,6 @@ class NotePolicy < BasePolicy
   delegate { @subject.noteable if @subject.noteable.lockable? }
 
   condition(:is_author) { @user && @subject.author == @user }
-  condition(:for_merge_request, scope: :subject) { @subject.for_merge_request? }
   condition(:is_noteable_author) { @user && @subject.noteable.author_id == @user.id }
 
   condition(:editable, scope: :subject) { @subject.editable? }
@@ -16,7 +15,7 @@ class NotePolicy < BasePolicy
     enable :resolve_note
   end
 
-  rule { for_merge_request & is_noteable_author }.policy do
+  rule { is_noteable_author }.policy do
     enable :resolve_note
   end
 end

app/policies/project_policy.rb

@@ -1,10 +1,38 @@
 class ProjectPolicy < BasePolicy
-  def self.create_read_update_admin(name)
+  READONLY_FEATURES_WHEN_ARCHIVED = %i[
+    issue
+    list
+    merge_request
+    label
+    milestone
+    project_snippet
+    wiki
+    note
+    pipeline
+    pipeline_schedule
+    build
+    trigger
+    environment
+    deployment
+    commit_status
+    container_image
+    pages
+    cluster
+  ].freeze
+
+  def self.create_read_update_admin_destroy(name)
+    [
+      :"read_#{name}",
+      *create_update_admin_destroy(name)
+    ]
+  end
+
+  def self.create_update_admin_destroy(name)
     [
       :"create_#{name}",
-      :"read_#{name}",
       :"update_#{name}",
-      :"admin_#{name}"
+      :"admin_#{name}",
+      :"destroy_#{name}"
     ]
   end
 
@@ -15,7 +43,7 @@ class ProjectPolicy < BasePolicy
   end
 
   desc "Project has public builds enabled"
-  condition(:public_builds, scope: :subject) { project.public_builds? }
+  condition(:public_builds, scope: :subject, score: 0) { project.public_builds? }
 
   # For guest access we use #team_member? so we can use
   # project.members, which gets cached in subject scope.
@@ -35,7 +63,7 @@ class ProjectPolicy < BasePolicy
   condition(:master) { team_access_level >= Gitlab::Access::MASTER }
 
   desc "Project is public"
-  condition(:public_project, scope: :subject) { project.public? }
+  condition(:public_project, scope: :subject, score: 0) { project.public? }
 
   desc "Project is visible to internal users"
   condition(:internal_access) do
@@ -46,7 +74,7 @@ class ProjectPolicy < BasePolicy
   condition(:group_member, scope: :subject) { project_group_member? }
 
   desc "Project is archived"
-  condition(:archived, scope: :subject) { project.archived? }
+  condition(:archived, scope: :subject, score: 0) { project.archived? }
 
   condition(:default_issues_tracker, scope: :subject) { project.default_issues_tracker? }
 
@@ -56,10 +84,10 @@ class ProjectPolicy < BasePolicy
   end
 
   desc "Project has an external wiki"
-  condition(:has_external_wiki, scope: :subject) { project.has_external_wiki? }
+  condition(:has_external_wiki, scope: :subject, score: 0) { project.has_external_wiki? }
 
   desc "Project has request access enabled"
-  condition(:request_access_enabled, scope: :subject) { project.request_access_enabled }
+  condition(:request_access_enabled, scope: :subject, score: 0) { project.request_access_enabled }
 
   desc "Has merge requests allowing pushes to user"
   condition(:has_merge_requests_allowing_pushes, scope: :subject) do
@@ -231,37 +259,45 @@ class ProjectPolicy < BasePolicy
   end
 
   rule { archived }.policy do
-    prevent :create_merge_request
-    prevent :push_to_delete_protected_branch
     prevent :push_code
-    prevent :update_merge_request
-    prevent :admin_merge_request
+    prevent :push_to_delete_protected_branch
+    prevent :request_access
+    prevent :upload_file
+    prevent :resolve_note
+
+    READONLY_FEATURES_WHEN_ARCHIVED.each do |feature|
+      prevent(*create_update_admin_destroy(feature))
+    end
+  end
+
+  rule { issues_disabled }.policy do
+    prevent(*create_read_update_admin_destroy(:issue))
   end
 
   rule { merge_requests_disabled | repository_disabled }.policy do
-    prevent(*create_read_update_admin(:merge_request))
+    prevent(*create_read_update_admin_destroy(:merge_request))
   end
 
   rule { issues_disabled & merge_requests_disabled }.policy do
-    prevent(*create_read_update_admin(:label))
-    prevent(*create_read_update_admin(:milestone))
+    prevent(*create_read_update_admin_destroy(:label))
+    prevent(*create_read_update_admin_destroy(:milestone))
   end
 
   rule { snippets_disabled }.policy do
-    prevent(*create_read_update_admin(:project_snippet))
+    prevent(*create_read_update_admin_destroy(:project_snippet))
   end
 
   rule { wiki_disabled & ~has_external_wiki }.policy do
-    prevent(*create_read_update_admin(:wiki))
+    prevent(*create_read_update_admin_destroy(:wiki))
     prevent(:download_wiki_code)
   end
 
   rule { builds_disabled | repository_disabled }.policy do
-    prevent(*create_read_update_admin(:build))
-    prevent(*(create_read_update_admin(:pipeline) - [:read_pipeline]))
-    prevent(*create_read_update_admin(:pipeline_schedule))
-    prevent(*create_read_update_admin(:environment))
-    prevent(*create_read_update_admin(:deployment))
+    prevent(*create_update_admin_destroy(:pipeline))
+    prevent(*create_read_update_admin_destroy(:build))
+    prevent(*create_read_update_admin_destroy(:pipeline_schedule))
+    prevent(*create_read_update_admin_destroy(:environment))
+    prevent(*create_read_update_admin_destroy(:deployment))
   end
 
   rule { repository_disabled }.policy do
@@ -272,7 +308,7 @@ class ProjectPolicy < BasePolicy
   end
 
   rule { container_registry_disabled }.policy do
-    prevent(*create_read_update_admin(:container_image))
+    prevent(*create_read_update_admin_destroy(:container_image))
   end
 
   rule { anonymous & ~public_project }.prevent_all
@@ -314,13 +350,6 @@ class ProjectPolicy < BasePolicy
     enable :read_pipeline_schedule
   end
 
-  rule { issues_disabled }.policy do
-    prevent :create_issue
-    prevent :update_issue
-    prevent :admin_issue
-    prevent :read_issue
-  end
-
   # These rules are included to allow maintainers of projects to push to certain
   # to run pipelines for the branches they have access to.
   rule { can?(:public_access) & has_merge_requests_allowing_pushes }.policy do

spec/policies/project_policy_spec.rb

@@ -136,13 +136,49 @@ describe ProjectPolicy do
     end
   end
 
+  shared_examples 'archived project policies' do
+    let(:feature_write_abilities) do
+      described_class::READONLY_FEATURES_WHEN_ARCHIVED.flat_map do |feature|
+        described_class.create_update_admin_destroy(feature)
+      end
+    end
+
+    let(:other_write_abilities) do
+      %i[
+        push_to_delete_protected_branch
+        push_code
+        request_access
+        upload_file
+        resolve_note
+      ]
+    end
+
+    context 'when the project is archived' do
+      before do
+        project.archived = true
+      end
+
+      it 'disables write actions on all relevant project features' do
+        expect_disallowed(*feature_write_abilities)
+      end
+
+      it 'disables some other important write actions' do
+        expect_disallowed(*other_write_abilities)
+      end
+
+      it 'does not disable other other abilities' do
+        expect_allowed(*(regular_abilities - feature_write_abilities - other_write_abilities))
+      end
+    end
+  end
+
   shared_examples 'project policies as anonymous' do
     context 'abilities for public projects' do
       context 'when a project has pending invites' do
         let(:group) { create(:group, :public) }
         let(:project) { create(:project, :public, namespace: group) }
         let(:user_permissions) { [:create_project, :create_issue, :create_note, :upload_file] }
-        let(:anonymous_permissions) { guest_permissions - user_permissions }
+        let(:anonymous_permissions) { guest_permissions - user_permissions  }
 
         subject { described_class.new(nil, project) }
 
@@ -154,6 +190,10 @@ describe ProjectPolicy do
           expect_allowed(*anonymous_permissions)
           expect_disallowed(*user_permissions)
         end
+
+        it_behaves_like 'archived project policies' do
+          let(:regular_abilities) { anonymous_permissions }
+        end
       end
     end
 
@@ -184,6 +224,10 @@ describe ProjectPolicy do
         expect_disallowed(*owner_permissions)
       end
 
+      it_behaves_like 'archived project policies' do
+        let(:regular_abilities) { guest_permissions }
+      end
+
       context 'public builds enabled' do
         it do
           expect_allowed(*guest_permissions)
@@ -224,12 +268,15 @@ describe ProjectPolicy do
       it do
         expect_allowed(*guest_permissions)
         expect_allowed(*reporter_permissions)
-        expect_allowed(*reporter_permissions)
         expect_allowed(*team_member_reporter_permissions)
         expect_disallowed(*developer_permissions)
         expect_disallowed(*master_permissions)
         expect_disallowed(*owner_permissions)
       end
+
+      it_behaves_like 'archived project policies' do
+        let(:regular_abilities) { reporter_permissions }
+      end
     end
   end
 
@@ -247,6 +294,10 @@ describe ProjectPolicy do
         expect_disallowed(*master_permissions)
         expect_disallowed(*owner_permissions)
       end
+
+      it_behaves_like 'archived project policies' do
+        let(:regular_abilities) { developer_permissions }
+      end
     end
   end
 
@@ -264,6 +315,10 @@ describe ProjectPolicy do
         expect_allowed(*master_permissions)
         expect_disallowed(*owner_permissions)
       end
+
+      it_behaves_like 'archived project policies' do
+        let(:regular_abilities) { master_permissions }
+      end
     end
   end
 
@@ -281,6 +336,10 @@ describe ProjectPolicy do
         expect_allowed(*master_permissions)
         expect_allowed(*owner_permissions)
       end
+
+      it_behaves_like 'archived project policies' do
+        let(:regular_abilities) { owner_permissions }
+      end
     end
   end
 
@@ -298,6 +357,10 @@ describe ProjectPolicy do
         expect_allowed(*master_permissions)
         expect_allowed(*owner_permissions)
       end
+
+      it_behaves_like 'archived project policies' do
+        let(:regular_abilities) { owner_permissions }
+      end
     end
   end