From 23bfd8c13c803f4efdb9eaf8e6e3c1ffd17640e8 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Tue, 4 Jul 2017 05:01:05 +0800
Subject: Consistently check permission for creating pipelines,

updating builds and updating pipelines. We check against
being able to merge or push if the ref is protected.
---
 spec/policies/ci/build_policy_spec.rb    | 52 +++++++++++---------------------
 spec/policies/ci/pipeline_policy_spec.rb | 47 +++++++++++++++++++++++++++++
 2 files changed, 64 insertions(+), 35 deletions(-)
 create mode 100644 spec/policies/ci/pipeline_policy_spec.rb

(limited to 'spec/policies')

diff --git a/spec/policies/ci/build_policy_spec.rb b/spec/policies/ci/build_policy_spec.rb
index 48a139d4b83..b4c6f3141fb 100644
--- a/spec/policies/ci/build_policy_spec.rb
+++ b/spec/policies/ci/build_policy_spec.rb
@@ -96,55 +96,37 @@ describe Ci::BuildPolicy, :models do
       end
     end
 
-    describe 'rules for manual actions' do
+    describe 'rules for protected branch' do
       let(:project) { create(:project) }
 
       before do
         project.add_developer(user)
-      end
-
-      context 'when branch build is assigned to is protected' do
-        before do
-          create(:protected_branch, :no_one_can_push,
-                 name: 'some-ref', project: project)
-        end
 
-        context 'when build is a manual action' do
-          let(:build) do
-            create(:ci_build, :manual, ref: 'some-ref', pipeline: pipeline)
-          end
-
-          it 'does not include ability to update build' do
-            expect(policies).not_to include :update_build
-          end
-        end
+        create(:protected_branch, branch_policy,
+               name: build.ref, project: project)
+      end
 
-        context 'when build is not a manual action' do
-          let(:build) do
-            create(:ci_build, ref: 'some-ref', pipeline: pipeline)
-          end
+      context 'when no one can push or merge to the branch' do
+        let(:branch_policy) { :no_one_can_push }
 
-          it 'includes ability to update build' do
-            expect(policies).to include :update_build
-          end
+        it 'does not include ability to update build' do
+          expect(policies).not_to include :update_build
         end
       end
 
-      context 'when branch build is assigned to is not protected' do
-        context 'when build is a manual action' do
-          let(:build) { create(:ci_build, :manual, pipeline: pipeline) }
+      context 'when developers can push to the branch' do
+        let(:branch_policy) { :developers_can_push }
 
-          it 'includes ability to update build' do
-            expect(policies).to include :update_build
-          end
+        it 'includes ability to update build' do
+          expect(policies).to include :update_build
         end
+      end
 
-        context 'when build is not a manual action' do
-          let(:build) { create(:ci_build, pipeline: pipeline) }
+      context 'when developers can push to the branch' do
+        let(:branch_policy) { :developers_can_merge }
 
-          it 'includes ability to update build' do
-            expect(policies).to include :update_build
-          end
+        it 'includes ability to update build' do
+          expect(policies).to include :update_build
         end
       end
     end
diff --git a/spec/policies/ci/pipeline_policy_spec.rb b/spec/policies/ci/pipeline_policy_spec.rb
new file mode 100644
index 00000000000..4ecf07a1bf2
--- /dev/null
+++ b/spec/policies/ci/pipeline_policy_spec.rb
@@ -0,0 +1,47 @@
+require 'spec_helper'
+
+describe Ci::PipelinePolicy, :models do
+  let(:user) { create(:user) }
+  let(:pipeline) { create(:ci_empty_pipeline, project: project) }
+
+  let(:policies) do
+    described_class.abilities(user, pipeline).to_set
+  end
+
+  describe 'rules' do
+    describe 'rules for protected branch' do
+      let(:project) { create(:project) }
+
+      before do
+        project.add_developer(user)
+
+        create(:protected_branch, branch_policy,
+               name: pipeline.ref, project: project)
+      end
+
+      context 'when no one can push or merge to the branch' do
+        let(:branch_policy) { :no_one_can_push }
+
+        it 'does not include ability to update pipeline' do
+          expect(policies).not_to include :update_pipeline
+        end
+      end
+
+      context 'when developers can push to the branch' do
+        let(:branch_policy) { :developers_can_push }
+
+        it 'includes ability to update pipeline' do
+          expect(policies).to include :update_pipeline
+        end
+      end
+
+      context 'when developers can push to the branch' do
+        let(:branch_policy) { :developers_can_merge }
+
+        it 'includes ability to update pipeline' do
+          expect(policies).to include :update_pipeline
+        end
+      end
+    end
+  end
+end
-- 
cgit v1.2.3


From 005870d5ce1a00b3405d0ae3a639d0c4befcb7a2 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Tue, 4 Jul 2017 05:20:44 +0800
Subject: Fix bad conflict resolution

---
 spec/policies/ci/build_policy_spec.rb    |  6 +++---
 spec/policies/ci/pipeline_policy_spec.rb | 10 +++++-----
 2 files changed, 8 insertions(+), 8 deletions(-)

(limited to 'spec/policies')

diff --git a/spec/policies/ci/build_policy_spec.rb b/spec/policies/ci/build_policy_spec.rb
index 2a8e6653eb8..9e2b0506bf3 100644
--- a/spec/policies/ci/build_policy_spec.rb
+++ b/spec/policies/ci/build_policy_spec.rb
@@ -110,7 +110,7 @@ describe Ci::BuildPolicy, :models do
         let(:branch_policy) { :no_one_can_push }
 
         it 'does not include ability to update build' do
-          expect(policies).to be_disallowed :update_build
+          expect(policy).to be_disallowed :update_build
         end
       end
 
@@ -118,7 +118,7 @@ describe Ci::BuildPolicy, :models do
         let(:branch_policy) { :developers_can_push }
 
         it 'includes ability to update build' do
-          expect(policies).to be_allowed :update_build
+          expect(policy).to be_allowed :update_build
         end
       end
 
@@ -126,7 +126,7 @@ describe Ci::BuildPolicy, :models do
         let(:branch_policy) { :developers_can_merge }
 
         it 'includes ability to update build' do
-          expect(policies).to be_allowed :update_build
+          expect(policy).to be_allowed :update_build
         end
       end
     end
diff --git a/spec/policies/ci/pipeline_policy_spec.rb b/spec/policies/ci/pipeline_policy_spec.rb
index db09be96875..cc04230411f 100644
--- a/spec/policies/ci/pipeline_policy_spec.rb
+++ b/spec/policies/ci/pipeline_policy_spec.rb
@@ -4,8 +4,8 @@ describe Ci::PipelinePolicy, :models do
   let(:user) { create(:user) }
   let(:pipeline) { create(:ci_empty_pipeline, project: project) }
 
-  let(:policies) do
-    described_class.abilities(user, pipeline).to_set
+  let(:policy) do
+    described_class.new(user, pipeline)
   end
 
   describe 'rules' do
@@ -23,7 +23,7 @@ describe Ci::PipelinePolicy, :models do
         let(:branch_policy) { :no_one_can_push }
 
         it 'does not include ability to update pipeline' do
-          expect(policies).to be_disallowed :update_pipeline
+          expect(policy).to be_disallowed :update_pipeline
         end
       end
 
@@ -31,7 +31,7 @@ describe Ci::PipelinePolicy, :models do
         let(:branch_policy) { :developers_can_push }
 
         it 'includes ability to update pipeline' do
-          expect(policies).to be_allowed :update_pipeline
+          expect(policy).to be_allowed :update_pipeline
         end
       end
 
@@ -39,7 +39,7 @@ describe Ci::PipelinePolicy, :models do
         let(:branch_policy) { :developers_can_merge }
 
         it 'includes ability to update pipeline' do
-          expect(policies).to be_allowed :update_pipeline
+          expect(policy).to be_allowed :update_pipeline
         end
       end
     end
-- 
cgit v1.2.3


From c86e74b284826e2f53bbcba763edd113a7022ffc Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Tue, 18 Jul 2017 21:08:48 +0800
Subject: Restore some tests from master

---
 spec/policies/ci/build_policy_spec.rb | 38 ++++++++++++++++++++++++++++++-----
 1 file changed, 33 insertions(+), 5 deletions(-)

(limited to 'spec/policies')

diff --git a/spec/policies/ci/build_policy_spec.rb b/spec/policies/ci/build_policy_spec.rb
index 86e57fdf607..9041460ea91 100644
--- a/spec/policies/ci/build_policy_spec.rb
+++ b/spec/policies/ci/build_policy_spec.rb
@@ -98,16 +98,17 @@ describe Ci::BuildPolicy, :models do
 
     describe 'rules for protected branch' do
       let(:project) { create(:project) }
+      let(:build) { create(:ci_build, ref: 'some-ref', pipeline: pipeline) }
 
       before do
         project.add_developer(user)
-
-        create(:protected_branch, branch_policy,
-               name: build.ref, project: project)
       end
 
       context 'when no one can push or merge to the branch' do
-        let(:branch_policy) { :no_one_can_push }
+        before do
+          create(:protected_branch, :no_one_can_push,
+                 name: 'some-ref', project: project)
+        end
 
         it 'does not include ability to update build' do
           expect(policy).to be_disallowed :update_build
@@ -115,7 +116,34 @@ describe Ci::BuildPolicy, :models do
       end
 
       context 'when developers can push to the branch' do
-        let(:branch_policy) { :developers_can_merge }
+        before do
+          create(:protected_branch, :developers_can_merge,
+                 name: 'some-ref', project: project)
+        end
+
+        it 'includes ability to update build' do
+          expect(policy).to be_allowed :update_build
+        end
+      end
+
+      context 'when no one can create the tag' do
+        before do
+          create(:protected_tag, :no_one_can_create,
+                 name: 'some-ref', project: project)
+
+          build.update(tag: true)
+        end
+
+        it 'does not include ability to update build' do
+          expect(policy).to be_disallowed :update_build
+        end
+      end
+
+      context 'when no one can create the tag but it is not a tag' do
+        before do
+          create(:protected_tag, :no_one_can_create,
+                 name: 'some-ref', project: project)
+        end
 
         it 'includes ability to update build' do
           expect(policy).to be_allowed :update_build
-- 
cgit v1.2.3


From a27cf281b17641f3f33712633099369867415309 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Tue, 18 Jul 2017 22:04:22 +0800
Subject: Unify build policy tests and pipeline policy tests

---
 spec/policies/ci/build_policy_spec.rb    | 10 ++++-----
 spec/policies/ci/pipeline_policy_spec.rb | 35 ++++++++++++++++++++++++--------
 2 files changed, 32 insertions(+), 13 deletions(-)

(limited to 'spec/policies')

diff --git a/spec/policies/ci/build_policy_spec.rb b/spec/policies/ci/build_policy_spec.rb
index 9041460ea91..e3ea3c960a4 100644
--- a/spec/policies/ci/build_policy_spec.rb
+++ b/spec/policies/ci/build_policy_spec.rb
@@ -96,7 +96,7 @@ describe Ci::BuildPolicy, :models do
       end
     end
 
-    describe 'rules for protected branch' do
+    describe 'rules for protected ref' do
       let(:project) { create(:project) }
       let(:build) { create(:ci_build, ref: 'some-ref', pipeline: pipeline) }
 
@@ -107,7 +107,7 @@ describe Ci::BuildPolicy, :models do
       context 'when no one can push or merge to the branch' do
         before do
           create(:protected_branch, :no_one_can_push,
-                 name: 'some-ref', project: project)
+                 name: build.ref, project: project)
         end
 
         it 'does not include ability to update build' do
@@ -118,7 +118,7 @@ describe Ci::BuildPolicy, :models do
       context 'when developers can push to the branch' do
         before do
           create(:protected_branch, :developers_can_merge,
-                 name: 'some-ref', project: project)
+                 name: build.ref, project: project)
         end
 
         it 'includes ability to update build' do
@@ -129,7 +129,7 @@ describe Ci::BuildPolicy, :models do
       context 'when no one can create the tag' do
         before do
           create(:protected_tag, :no_one_can_create,
-                 name: 'some-ref', project: project)
+                 name: build.ref, project: project)
 
           build.update(tag: true)
         end
@@ -142,7 +142,7 @@ describe Ci::BuildPolicy, :models do
       context 'when no one can create the tag but it is not a tag' do
         before do
           create(:protected_tag, :no_one_can_create,
-                 name: 'some-ref', project: project)
+                 name: build.ref, project: project)
         end
 
         it 'includes ability to update build' do
diff --git a/spec/policies/ci/pipeline_policy_spec.rb b/spec/policies/ci/pipeline_policy_spec.rb
index cc04230411f..b11b06d301f 100644
--- a/spec/policies/ci/pipeline_policy_spec.rb
+++ b/spec/policies/ci/pipeline_policy_spec.rb
@@ -9,18 +9,18 @@ describe Ci::PipelinePolicy, :models do
   end
 
   describe 'rules' do
-    describe 'rules for protected branch' do
+    describe 'rules for protected ref' do
       let(:project) { create(:project) }
 
       before do
         project.add_developer(user)
-
-        create(:protected_branch, branch_policy,
-               name: pipeline.ref, project: project)
       end
 
       context 'when no one can push or merge to the branch' do
-        let(:branch_policy) { :no_one_can_push }
+        before do
+          create(:protected_branch, :no_one_can_push,
+                 name: pipeline.ref, project: project)
+        end
 
         it 'does not include ability to update pipeline' do
           expect(policy).to be_disallowed :update_pipeline
@@ -28,15 +28,34 @@ describe Ci::PipelinePolicy, :models do
       end
 
       context 'when developers can push to the branch' do
-        let(:branch_policy) { :developers_can_push }
+        before do
+          create(:protected_branch, :developers_can_merge,
+                 name: pipeline.ref, project: project)
+        end
 
         it 'includes ability to update pipeline' do
           expect(policy).to be_allowed :update_pipeline
         end
       end
 
-      context 'when developers can push to the branch' do
-        let(:branch_policy) { :developers_can_merge }
+      context 'when no one can create the tag' do
+        before do
+          create(:protected_tag, :no_one_can_create,
+                 name: pipeline.ref, project: project)
+
+          pipeline.update(tag: true)
+        end
+
+        it 'does not include ability to update pipeline' do
+          expect(policy).to be_disallowed :update_pipeline
+        end
+      end
+
+      context 'when no one can create the tag but it is not a tag' do
+        before do
+          create(:protected_tag, :no_one_can_create,
+                 name: pipeline.ref, project: project)
+        end
 
         it 'includes ability to update pipeline' do
           expect(policy).to be_allowed :update_pipeline
-- 
cgit v1.2.3