Merge branch 'fix/ci-build-policy' into 'master'

Fix build access policies when pipelines are public

See merge request !8711

3 files changed, 97 insertions, 2 deletions

diff --git a/app/policies/ci/build_policy.rb b/app/policies/ci/build_policy.rb
index 7b1752df0e1..8b25332b73c 100644
--- a/app/policies/ci/build_policy.rb
+++ b/app/policies/ci/build_policy.rb
@@ -1,8 +1,6 @@
 module Ci
   class BuildPolicy < CommitStatusPolicy
     def rules
-      can! :read_build if @subject.project.public_builds?
-
       super
 
       # If we can't read build we should also not have that
diff --git a/changelogs/unreleased/fix-ci-build-policy.yml b/changelogs/unreleased/fix-ci-build-policy.yml
new file mode 100644
index 00000000000..26003713ed4
--- /dev/null
+++ b/changelogs/unreleased/fix-ci-build-policy.yml
@@ -0,0 +1,4 @@
+---
+title: Improve build policy and access abilities
+merge_request: 8711
+author:
diff --git a/spec/policies/ci/build_policy_spec.rb b/spec/policies/ci/build_policy_spec.rb
new file mode 100644
index 00000000000..0f280f32eac
--- /dev/null
+++ b/spec/policies/ci/build_policy_spec.rb
@@ -0,0 +1,93 @@
+require 'spec_helper'
+
+describe Ci::BuildPolicy, :models do
+  let(:user) { create(:user) }
+  let(:build) { create(:ci_build, pipeline: pipeline) }
+  let(:pipeline) { create(:ci_empty_pipeline, project: project) }
+
+  let(:policies) do
+    described_class.abilities(user, build).to_set
+  end
+
+  shared_context 'public pipelines disabled' do
+    before { project.update_attribute(:public_builds, false) }
+  end
+
+  describe '#rules' do
+    context 'when user does not have access to the project' do
+      let(:project) { create(:empty_project, :private) }
+
+      context 'when public builds are enabled' do
+        it 'does not include ability to read build' do
+          expect(policies).not_to include :read_build
+        end
+      end
+
+      context 'when public builds are disabled' do
+        include_context 'public pipelines disabled'
+
+        it 'does not include ability to read build' do
+          expect(policies).not_to include :read_build
+        end
+      end
+    end
+
+    context 'when anonymous user has access to the project' do
+      let(:project) { create(:empty_project, :public) }
+
+      context 'when public builds are enabled' do
+        it 'includes ability to read build' do
+          expect(policies).to include :read_build
+        end
+      end
+
+      context 'when public builds are disabled' do
+        include_context 'public pipelines disabled'
+
+        it 'does not include ability to read build' do
+          expect(policies).not_to include :read_build
+        end
+      end
+    end
+
+    context 'when team member has access to the project' do
+      let(:project) { create(:empty_project, :public) }
+
+      context 'team member is a guest' do
+        before { project.team << [user, :guest] }
+
+        context 'when public builds are enabled' do
+          it 'includes ability to read build' do
+            expect(policies).to include :read_build
+          end
+        end
+
+        context 'when public builds are disabled' do
+          include_context 'public pipelines disabled'
+
+          it 'does not include ability to read build' do
+            expect(policies).not_to include :read_build
+          end
+        end
+      end
+
+      context 'team member is a reporter' do
+        before { project.team << [user, :reporter] }
+
+        context 'when public builds are enabled' do
+          it 'includes ability to read build' do
+            expect(policies).to include :read_build
+          end
+        end
+
+        context 'when public builds are disabled' do
+          include_context 'public pipelines disabled'
+
+          it 'does not include ability to read build' do
+            expect(policies).to include :read_build
+          end
+        end
+      end
+    end
+  end
+end