Merge remote-tracking branch 'dev/12-9-stable' into 12-9-stable

carlos-wong · Jun 3, 2020 · a725264 · a725264
2 parents 02da922 + 884fd7a
commit a725264
Show file tree

Hide file tree

Showing 5 changed files with 54 additions and 13 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,13 @@
 documentation](doc/development/changelog.md) for instructions on adding your own
 entry.
 
+## 12.9.9 (2020-06-03)
+
+### Security (1 change)
+
+- Prevent fetching repository code with unauthorized ci token.
+
+
 ## 12.9.8 (2020-05-27)
 
 ### Security (13 changes)

diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION
@@ -1 +1 @@
-12.9.8
+12.9.9
diff --git a/VERSION b/VERSION
@@ -1 +1 @@
-12.9.8
+12.9.9
diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb
@@ -385,6 +385,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
@@ -5,6 +5,7 @@
 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) }
@@ -295,7 +296,7 @@ def expect_disallowed(*permissions)
     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,
@@ -347,7 +348,7 @@ def expect_disallowed(*permissions)
       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
 
       it 'disallows all permissions except pipeline when the feature is disabled' do
@@ -367,7 +368,7 @@ def expect_disallowed(*permissions)
       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
@@ -382,22 +383,54 @@ def expect_disallowed(*permissions)
   end
 
   context 'repository feature' do
-    subject { described_class.new(owner, project) }
-
-    it 'disallows all permissions when the feature is disabled' do
-      project.project_feature.update(repository_access_level: ProjectFeature::DISABLED)
-
-      repository_permissions = [
+    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
+        :destroy_release, :download_code, :build_download_code
       ]
+    end
+
+    context 'when user is a project member' do
+      subject { described_class.new(owner, project) }
+
+      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
 
-      expect_disallowed(*repository_permissions)
+        it 'disallows all permissions' do
+          expect_disallowed(*repository_permissions)
+        end
+      end
+    end
+
+    context 'when user is some other user' do
+      subject { described_class.new(other_user, project) }
+
+      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
+
+        it 'disallows all permissions' do
+          expect_disallowed(*repository_permissions)
+        end
+      end
     end
   end