diff --git a/app/policies/ci/build_policy.rb b/app/policies/ci/build_policy.rb index 8b25332b73ceeeb2635ae78d1d0665b36dca8017..7b1752df0e1a230f43ed10ce41a8360de16cb017 100644 --- a/app/policies/ci/build_policy.rb +++ b/app/policies/ci/build_policy.rb @@ -1,6 +1,8 @@ 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/app/policies/project_policy.rb b/app/policies/project_policy.rb index 8ac4bd9df6d4ca1ef1f16aecdf2b2b0dc326885a..d5aadfce76a8267e4842858c3ffaca0567f157ba 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -12,9 +12,6 @@ class ProjectPolicy < BasePolicy guest_access! public_access! - # Allow to read builds for internal projects - can! :read_build if project.public_builds? - if project.request_access_enabled && !(owner || user.admin? || project.team.member?(user) || project_group_member?(user)) can! :request_access @@ -46,6 +43,11 @@ class ProjectPolicy < BasePolicy can! :create_note can! :upload_file can! :read_cycle_analytics + + if project.public_builds? + can! :read_pipeline + can! :read_build + end end def reporter_access! diff --git a/changelogs/unreleased/zj-guest-reads-public-builds.yml b/changelogs/unreleased/zj-guest-reads-public-builds.yml new file mode 100644 index 0000000000000000000000000000000000000000..1859addd606c46bab7383e10af7727d14d83cce8 --- /dev/null +++ b/changelogs/unreleased/zj-guest-reads-public-builds.yml @@ -0,0 +1,4 @@ +--- +title: Guests can read builds when public +merge_request: 6842 +author: diff --git a/features/steps/shared/project.rb b/features/steps/shared/project.rb index cab85a48396b69465c2e0b2013ff532836529360..b51152c79c6d77f68503b9d90c5e5069b411a684 100644 --- a/features/steps/shared/project.rb +++ b/features/steps/shared/project.rb @@ -9,7 +9,7 @@ module SharedProject step "project exists in some group namespace" do @group = create(:group, name: 'some group') - @project = create(:project, namespace: @group) + @project = create(:project, namespace: @group, public_builds: false) end # Create a specific project called "Shop" diff --git a/spec/features/projects/guest_navigation_menu_spec.rb b/spec/features/projects/guest_navigation_menu_spec.rb index c22441f8929bb46bd8ba857dc334c414d9c7da94..8120a51c5157dd6d8eb2c8b4065e98b4989a2147 100644 --- a/spec/features/projects/guest_navigation_menu_spec.rb +++ b/spec/features/projects/guest_navigation_menu_spec.rb @@ -1,8 +1,8 @@ require 'spec_helper' describe "Guest navigation menu" do - let(:project) { create :empty_project, :private } - let(:guest) { create :user } + let(:project) { create(:empty_project, :private, public_builds: false) } + let(:guest) { create(:user) } before do project.team << [guest, :guest] diff --git a/spec/features/security/project/private_access_spec.rb b/spec/features/security/project/private_access_spec.rb index 290ddb4c6ddab0fa312d1998830f0809224536ff..f52e23f94338c2365e0fd9c961054e9991bac2fc 100644 --- a/spec/features/security/project/private_access_spec.rb +++ b/spec/features/security/project/private_access_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe "Private Project Access", feature: true do include AccessMatchers - let(:project) { create(:project, :private) } + let(:project) { create(:project, :private, public_builds: false) } describe "Project should be private" do describe '#private?' do @@ -260,6 +260,18 @@ describe "Private Project Access", feature: true do it { is_expected.to be_denied_for(:user) } it { is_expected.to be_denied_for(:external) } it { is_expected.to be_denied_for(:visitor) } + + context 'when public builds is enabled' do + before do + project.update(public_builds: true) + end + + it { is_expected.to be_allowed_for(:guest).of(project) } + end + + context 'when public buils are disabled' do + it { is_expected.to be_denied_for(:guest).of(project) } + end end describe "GET /:project_path/pipelines/:id" do @@ -275,6 +287,18 @@ describe "Private Project Access", feature: true do it { is_expected.to be_denied_for(:user) } it { is_expected.to be_denied_for(:external) } it { is_expected.to be_denied_for(:visitor) } + + context 'when public builds is enabled' do + before do + project.update(public_builds: true) + end + + it { is_expected.to be_allowed_for(:guest).of(project) } + end + + context 'when public buils are disabled' do + it { is_expected.to be_denied_for(:guest).of(project) } + end end describe "GET /:project_path/builds" do @@ -289,6 +313,18 @@ describe "Private Project Access", feature: true do it { is_expected.to be_denied_for(:user) } it { is_expected.to be_denied_for(:external) } it { is_expected.to be_denied_for(:visitor) } + + context 'when public builds is enabled' do + before do + project.update(public_builds: true) + end + + it { is_expected.to be_allowed_for(:guest).of(project) } + end + + context 'when public buils are disabled' do + it { is_expected.to be_denied_for(:guest).of(project) } + end end describe "GET /:project_path/builds/:id" do @@ -305,6 +341,23 @@ describe "Private Project Access", feature: true do it { is_expected.to be_denied_for(:user) } it { is_expected.to be_denied_for(:external) } it { is_expected.to be_denied_for(:visitor) } + + context 'when public builds is enabled' do + before do + project.update(public_builds: true) + end + + it { is_expected.to be_allowed_for(:guest).of(project) } + end + + context 'when public buils are disabled' do + before do + project.public_builds = false + project.save + end + + it { is_expected.to be_denied_for(:guest).of(project) } + end end describe "GET /:project_path/environments" do diff --git a/spec/lib/gitlab/cycle_analytics/permissions_spec.rb b/spec/lib/gitlab/cycle_analytics/permissions_spec.rb index dc4f7dc69db8ef997dd614cd8fee63c99e93f972..2d85e712db0e35693a565de09c852ca4c3bd2778 100644 --- a/spec/lib/gitlab/cycle_analytics/permissions_spec.rb +++ b/spec/lib/gitlab/cycle_analytics/permissions_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Gitlab::CycleAnalytics::Permissions do - let(:project) { create(:empty_project) } + let(:project) { create(:empty_project, public_builds: false) } let(:user) { create(:user) } subject { described_class.get(user: user, project: project) } diff --git a/spec/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb index b49e4f3a8bc7ee31706c630aea8755aa824b98cf..eeab9827d994530f639ca91fb4015655725723a0 100644 --- a/spec/policies/project_policy_spec.rb +++ b/spec/policies/project_policy_spec.rb @@ -111,14 +111,36 @@ describe ProjectPolicy, models: true do context 'guests' do let(:current_user) { guest } + let(:reporter_public_build_permissions) do + reporter_permissions - [:read_build, :read_pipeline] + end + it do is_expected.to include(*guest_permissions) - is_expected.not_to include(*reporter_permissions) + is_expected.not_to include(*reporter_public_build_permissions) is_expected.not_to include(*team_member_reporter_permissions) is_expected.not_to include(*developer_permissions) is_expected.not_to include(*master_permissions) is_expected.not_to include(*owner_permissions) end + + context 'public builds enabled' do + it do + is_expected.to include(*guest_permissions) + is_expected.to include(:read_build, :read_pipeline) + end + end + + context 'public builds disabled' do + before do + project.update(public_builds: false) + end + + it do + is_expected.to include(*guest_permissions) + is_expected.not_to include(:read_build, :read_pipeline) + end + end end context 'reporter' do diff --git a/spec/requests/api/builds_spec.rb b/spec/requests/api/builds_spec.rb index 0ea991b18b88ec5d2f5a01995a6484039c8bc34e..7be7acebb194aa65a66ffb8eec5b9d748bac7c69 100644 --- a/spec/requests/api/builds_spec.rb +++ b/spec/requests/api/builds_spec.rb @@ -5,7 +5,7 @@ describe API::Builds, api: true do let(:user) { create(:user) } let(:api_user) { user } - let!(:project) { create(:project, creator_id: user.id) } + let!(:project) { create(:project, creator_id: user.id, public_builds: false) } let!(:developer) { create(:project_member, :developer, user: user, project: project) } let(:reporter) { create(:project_member, :reporter, project: project) } let(:guest) { create(:project_member, :guest, project: project) } diff --git a/spec/requests/projects/cycle_analytics_events_spec.rb b/spec/requests/projects/cycle_analytics_events_spec.rb index f5e0fdcda2dd5b3c7d95d851c72224f44dd17f1b..e0368e6001f433943d57fca25482b5106a126dc9 100644 --- a/spec/requests/projects/cycle_analytics_events_spec.rb +++ b/spec/requests/projects/cycle_analytics_events_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe 'cycle analytics events' do let(:user) { create(:user) } - let(:project) { create(:project) } + let(:project) { create(:project, public_builds: false) } let(:issue) { create(:issue, project: project, created_at: 2.days.ago) } describe 'GET /:namespace/:project/cycle_analytics/events/issues' do diff --git a/spec/workers/pipeline_notification_worker_spec.rb b/spec/workers/pipeline_notification_worker_spec.rb index 739f9b639670283788e2ffaad2de069300827e06..603ae52ed1e88ab6e7863cf558ebb22fdbfdab53 100644 --- a/spec/workers/pipeline_notification_worker_spec.rb +++ b/spec/workers/pipeline_notification_worker_spec.rb @@ -11,7 +11,7 @@ describe PipelineNotificationWorker do status: status) end - let(:project) { create(:project) } + let(:project) { create(:project, public_builds: false) } let(:user) { create(:user) } let(:pusher) { user } let(:watcher) { pusher }