Merge branch 'fix/sm/31771-do-not-allow-jobs-to-be-erased-new' into 'master'

Do not allow jobs to be erased

Closes #31771

See merge request gitlab-org/gitlab-ce!15216

5 files changed, 141 insertions, 1 deletions

diff --git a/spec/controllers/projects/jobs_controller_spec.rb b/spec/controllers/projects/jobs_controller_spec.rb
index f9688949a19..7490f8fefce 100644
--- a/spec/controllers/projects/jobs_controller_spec.rb
+++ b/spec/controllers/projects/jobs_controller_spec.rb
@@ -371,8 +371,10 @@ describe Projects::JobsController do
   end
 
   describe 'POST erase' do
+    let(:role) { :master }
+
     before do
-      project.add_developer(user)
+      project.team << [user, role]
       sign_in(user)
 
       post_erase
@@ -404,6 +406,27 @@ describe Projects::JobsController do
       end
     end
 
+    context 'when user is developer' do
+      let(:role) { :developer }
+      let(:job) { create(:ci_build, :erasable, :trace, pipeline: pipeline, user: triggered_by) }
+
+      context 'when triggered by same user' do
+        let(:triggered_by) { user }
+
+        it 'has successful status' do
+          expect(response).to have_gitlab_http_status(:found)
+        end
+      end
+
+      context 'when triggered by different user' do
+        let(:triggered_by) { create(:user) }
+
+        it 'does not have successful status' do
+          expect(response).not_to have_gitlab_http_status(:found)
+        end
+      end
+    end
+
     def post_erase
       post :erase, namespace_id: project.namespace,
                    project_id: project,
diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
index 5ed2e1ca99a..1795ee8e9a4 100644
--- a/spec/models/ci/build_spec.rb
+++ b/spec/models/ci/build_spec.rb
@@ -270,6 +270,23 @@ describe Ci::Build do
     end
   end
 
+  describe '#triggered_by?' do
+    subject { build.triggered_by?(user) }
+
+    context 'when user is owner' do
+      let(:build) { create(:ci_build, pipeline: pipeline, user: user) }
+
+      it { is_expected.to be_truthy }
+    end
+
+    context 'when user is not owner' do
+      let(:another_user) { create(:user) }
+      let(:build) { create(:ci_build, pipeline: pipeline, user: another_user) }
+
+      it { is_expected.to be_falsy }
+    end
+  end
+
   describe '#detailed_status' do
     it 'returns a detailed status' do
       expect(build.detailed_status(user))
diff --git a/spec/policies/ci/build_policy_spec.rb b/spec/policies/ci/build_policy_spec.rb
index 8e1bc3d1543..298a9d16425 100644
--- a/spec/policies/ci/build_policy_spec.rb
+++ b/spec/policies/ci/build_policy_spec.rb
@@ -150,5 +150,82 @@ describe Ci::BuildPolicy do
         end
       end
     end
+
+    describe 'rules for erase build' do
+      let(:project) { create(:project, :repository) }
+      let(:build) { create(:ci_build, pipeline: pipeline, ref: 'some-ref', user: owner) }
+
+      context 'when a developer erases a build' do
+        before do
+          project.add_developer(user)
+        end
+
+        context 'when developers can push to the branch' do
+          before do
+            create(:protected_branch, :developers_can_push,
+                   name: build.ref, project: project)
+          end
+
+          context 'when the build was created by the developer' do
+            let(:owner) { user }
+
+            it { expect(policy).to be_allowed :erase_build }
+          end
+
+          context 'when the build was created by the other' do
+            let(:owner) { create(:user) }
+
+            it { expect(policy).to be_disallowed :erase_build }
+          end
+        end
+
+        context 'when no one can push or merge to the branch' do
+          let(:owner) { user }
+
+          before do
+            create(:protected_branch, :no_one_can_push, :no_one_can_merge,
+                   name: build.ref, project: project)
+          end
+
+          it { expect(policy).to be_disallowed :erase_build }
+        end
+      end
+
+      context 'when a master erases a build' do
+        before do
+          project.add_master(user)
+        end
+
+        context 'when masters can push to the branch' do
+          before do
+            create(:protected_branch, :masters_can_push,
+                   name: build.ref, project: project)
+          end
+
+          context 'when the build was created by the master' do
+            let(:owner) { user }
+
+            it { expect(policy).to be_allowed :erase_build }
+          end
+
+          context 'when the build was created by the other' do
+            let(:owner) { create(:user) }
+
+            it { expect(policy).to be_allowed :erase_build }
+          end
+        end
+
+        context 'when no one can push or merge to the branch' do
+          let(:owner) { user }
+
+          before do
+            create(:protected_branch, :no_one_can_push, :no_one_can_merge,
+                   name: build.ref, project: project)
+          end
+
+          it { expect(policy).to be_disallowed :erase_build }
+        end
+      end
+    end
   end
 end
diff --git a/spec/requests/api/jobs_spec.rb b/spec/requests/api/jobs_spec.rb
index 1765907c1b4..2a83213e87a 100644
--- a/spec/requests/api/jobs_spec.rb
+++ b/spec/requests/api/jobs_spec.rb
@@ -500,7 +500,11 @@ describe API::Jobs do
   end
 
   describe 'POST /projects/:id/jobs/:job_id/erase' do
+    let(:role) { :master }
+
     before do
+      project.team << [user, role]
+
       post api("/projects/#{project.id}/jobs/#{job.id}/erase", user)
     end
 
@@ -529,6 +533,23 @@ describe API::Jobs do
         expect(response).to have_gitlab_http_status(403)
       end
     end
+
+    context 'when a developer erases a build' do
+      let(:role) { :developer }
+      let(:job) { create(:ci_build, :trace, :artifacts, :success, project: project, pipeline: pipeline, user: owner) }
+
+      context 'when the build was created by the developer' do
+        let(:owner) { user }
+
+        it { expect(response).to have_gitlab_http_status(201) }
+      end
+
+      context 'when the build was created by the other' do
+        let(:owner) { create(:user) }
+
+        it { expect(response).to have_gitlab_http_status(403) }
+      end
+    end
   end
 
   describe 'POST /projects/:id/jobs/:job_id/artifacts/keep' do
diff --git a/spec/requests/api/v3/builds_spec.rb b/spec/requests/api/v3/builds_spec.rb
index 3f58b7ef384..a73bb456b52 100644
--- a/spec/requests/api/v3/builds_spec.rb
+++ b/spec/requests/api/v3/builds_spec.rb
@@ -408,6 +408,8 @@ describe API::V3::Builds do
 
   describe 'POST /projects/:id/builds/:build_id/erase' do
     before do
+      project.add_master(user)
+
       post v3_api("/projects/#{project.id}/builds/#{build.id}/erase", user)
     end