diff options
| -rw-r--r-- | CHANGELOG.md | 7 | ||||
| -rw-r--r-- | GITALY_SERVER_VERSION | 2 | ||||
| -rw-r--r-- | VERSION | 2 | ||||
| -rw-r--r-- | app/policies/project_policy.rb | 1 | ||||
| -rw-r--r-- | spec/policies/project_policy_spec.rb | 103 |
5 files changed, 72 insertions, 43 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 1356338f635..53920c6ecc7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,13 @@ documentation](doc/development/changelog.md) for instructions on adding your own entry. +## 13.0.4 (2020-06-03) + +### Security (1 change) + +- Prevent fetching repository code with unauthorized ci token. + + ## 13.0.3 (2020-05-29) ### Fixed (8 changes, 1 of them is from the community) diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index 2cb4f2f6326..1b8bd35d6e0 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -13.0.3 +13.0.4 @@ -1 +1 @@ -13.0.3 +13.0.4 diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index a24c0471d6c..8df4fc5e88c 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -463,6 +463,7 @@ class ProjectPolicy < BasePolicy rule { repository_disabled }.policy do prevent :push_code prevent :download_code + prevent :build_download_code prevent :fork_project prevent :read_commit_status prevent :read_pipeline diff --git a/spec/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb index 09d54eb9df6..f91d5658626 100644 --- a/spec/policies/project_policy_spec.rb +++ b/spec/policies/project_policy_spec.rb @@ -5,6 +5,7 @@ require 'spec_helper' describe ProjectPolicy do include ExternalAuthorizationServiceHelpers include_context 'ProjectPolicy context' + let_it_be(:other_user) { create(:user) } let_it_be(:guest) { create(:user) } let_it_be(:reporter) { create(:user) } let_it_be(:developer) { create(:user) } @@ -163,7 +164,7 @@ describe ProjectPolicy do subject { described_class.new(owner, project) } it 'disallows all permissions when the feature is disabled' do - project.project_feature.update(merge_requests_access_level: ProjectFeature::DISABLED) + project.project_feature.update!(merge_requests_access_level: ProjectFeature::DISABLED) mr_permissions = [:create_merge_request_from, :read_merge_request, :update_merge_request, :admin_merge_request, @@ -215,7 +216,7 @@ describe ProjectPolicy do subject { described_class.new(owner, project) } before do - project.project_feature.update(builds_access_level: ProjectFeature::DISABLED) + project.project_feature.update!(builds_access_level: ProjectFeature::DISABLED) end context 'without metrics_dashboard_allowed' do @@ -260,7 +261,7 @@ describe ProjectPolicy do subject { described_class.new(guest, project) } before do - project.project_feature.update(builds_access_level: ProjectFeature::PRIVATE) + project.project_feature.update!(builds_access_level: ProjectFeature::PRIVATE) end it 'disallows pipeline and commit_status permissions' do @@ -275,50 +276,70 @@ describe ProjectPolicy do end context 'repository feature' do - subject { described_class.new(owner, project) } + let(:repository_permissions) do + [ + :create_pipeline, :update_pipeline, :admin_pipeline, :destroy_pipeline, + :create_build, :read_build, :update_build, :admin_build, :destroy_build, + :create_pipeline_schedule, :read_pipeline_schedule, :update_pipeline_schedule, :admin_pipeline_schedule, :destroy_pipeline_schedule, + :create_environment, :read_environment, :update_environment, :admin_environment, :destroy_environment, + :create_cluster, :read_cluster, :update_cluster, :admin_cluster, + :create_deployment, :read_deployment, :update_deployment, :admin_deployment, :destroy_deployment, + :destroy_release, :download_code, :build_download_code + ] + end + + context 'when user is a project member' do + subject { described_class.new(owner, project) } - before do - project.project_feature.update(repository_access_level: ProjectFeature::DISABLED) - end + context 'when it is disabled' do + before do + project.project_feature.update!( + repository_access_level: ProjectFeature::DISABLED, + merge_requests_access_level: ProjectFeature::DISABLED, + builds_access_level: ProjectFeature::DISABLED, + forking_access_level: ProjectFeature::DISABLED + ) + end - context 'without metrics_dashboard_allowed' do - before do - project.project_feature.update(metrics_dashboard_access_level: ProjectFeature::DISABLED) - end + context 'without metrics_dashboard_allowed' do + before do + project.project_feature.update(metrics_dashboard_access_level: ProjectFeature::DISABLED) + end - it 'disallows all permissions when the feature is disabled' do - repository_permissions = [ - :create_pipeline, :update_pipeline, :admin_pipeline, :destroy_pipeline, - :create_build, :read_build, :update_build, :admin_build, :destroy_build, - :create_pipeline_schedule, :read_pipeline_schedule, :update_pipeline_schedule, :admin_pipeline_schedule, :destroy_pipeline_schedule, - :create_environment, :read_environment, :update_environment, :admin_environment, :destroy_environment, - :create_cluster, :read_cluster, :update_cluster, :admin_cluster, - :create_deployment, :read_deployment, :update_deployment, :admin_deployment, :destroy_deployment, - :destroy_release - ] + it 'disallows all permissions when the feature is disabled' do + expect_disallowed(*repository_permissions) + end + end - expect_disallowed(*repository_permissions) + context 'with metrics_dashboard_allowed' do + before do + project.project_feature.update(metrics_dashboard_access_level: ProjectFeature::ENABLED) + end + + it 'disallows all permissions but read_environment when the feature is disabled' do + expect_disallowed(*(repository_permissions - [:read_environment])) + expect_allowed(:read_environment) + end + end end end - context 'with metrics_dashboard_allowed' do - before do - project.project_feature.update(metrics_dashboard_access_level: ProjectFeature::ENABLED) - end + context 'when user is some other user' do + subject { described_class.new(other_user, project) } - it 'disallows all permissions when the feature is disabled' do - repository_permissions = [ - :create_pipeline, :update_pipeline, :admin_pipeline, :destroy_pipeline, - :create_build, :read_build, :update_build, :admin_build, :destroy_build, - :create_pipeline_schedule, :read_pipeline_schedule, :update_pipeline_schedule, :admin_pipeline_schedule, :destroy_pipeline_schedule, - :create_environment, :update_environment, :admin_environment, :destroy_environment, - :create_cluster, :read_cluster, :update_cluster, :admin_cluster, - :create_deployment, :read_deployment, :update_deployment, :admin_deployment, :destroy_deployment, - :destroy_release - ] + context 'when access level is private' do + before do + project.project_feature.update!( + repository_access_level: ProjectFeature::PRIVATE, + merge_requests_access_level: ProjectFeature::PRIVATE, + builds_access_level: ProjectFeature::PRIVATE, + forking_access_level: ProjectFeature::PRIVATE + ) + end - expect_disallowed(*repository_permissions) - expect_allowed(:read_environment) + it 'disallows all permissions' do + expect_disallowed(*repository_permissions) + end end end end @@ -601,7 +622,7 @@ describe ProjectPolicy do context 'feature enabled' do before do - project.project_feature.update(metrics_dashboard_access_level: ProjectFeature::ENABLED) + project.project_feature.update!(metrics_dashboard_access_level: ProjectFeature::ENABLED) end context 'with reporter' do @@ -665,7 +686,7 @@ describe ProjectPolicy do context 'feature enabled' do before do - project.project_feature.update(metrics_dashboard_access_level: ProjectFeature::ENABLED) + project.project_feature.update!(metrics_dashboard_access_level: ProjectFeature::ENABLED) end context 'with reporter' do @@ -750,7 +771,7 @@ describe ProjectPolicy do context 'feature disabled' do before do - project.project_feature.update(metrics_dashboard_access_level: ProjectFeature::DISABLED) + project.project_feature.update!(metrics_dashboard_access_level: ProjectFeature::DISABLED) end context 'with reporter' do |