Merge branch 'sh-fix-repository-storage-api' into 'master'

Prevent regular users from moving projects to different storage shards (master)

See merge request gitlab/gitlab-ee!653

ee/changelogs/unreleased/sh-fix-repository-storage-api.yml

+---
+title: Prevent regular users from moving projects to different storage shards
+merge_request:
+author:
+type: security

ee/lib/ee/api/projects.rb

@@ -33,9 +33,16 @@ module EE
           def verify_update_project_attrs!(project, attrs)
             super
 
+            verify_storage_attrs!(attrs)
             verify_mirror_attrs!(project, attrs)
           end
 
+          def verify_storage_attrs!(attrs)
+            unless current_user.admin?
+              attrs.delete(:repository_storage)
+            end
+          end
+
           def verify_mirror_attrs!(project, attrs)
             unless can?(current_user, :admin_mirror, project)
               attrs.delete(:mirror)

ee/spec/requests/api/projects_spec.rb

@@ -77,16 +77,60 @@ describe API::Projects do
   describe 'PUT /projects/:id' do
     let(:project) { create(:project, namespace: user.namespace) }
 
-    before do
-      enable_external_authorization_service_check
+    context 'when updating external classification' do
+      before do
+        enable_external_authorization_service_check
+      end
+
+      it 'updates the classification label' do
+        put(api("/projects/#{project.id}", user), external_authorization_classification_label: 'new label')
+
+        expect(response).to have_gitlab_http_status(200)
+
+        expect(project.reload.external_authorization_classification_label).to eq('new label')
+      end
     end
 
-    it 'updates the classification label when enabled' do
-      put(api("/projects/#{project.id}", user), external_authorization_classification_label: 'new label')
+    context 'when updating repository storage' do
+      let(:unknown_storage) { 'new-storage' }
+      let(:new_project) { create(:project, :repository, namespace: user.namespace) }
+
+      context 'as a user' do
+        it 'returns 200 but does not change repository_storage' do
+          expect {
+            Sidekiq::Testing.fake! do
+              put(api("/projects/#{new_project.id}", user), repository_storage: unknown_storage, issues_enabled: false)
+            end
+          }.not_to change(ProjectUpdateRepositoryStorageWorker.jobs, :size)
 
-      expect(response).to have_gitlab_http_status(200)
+          expect(response).to have_gitlab_http_status(200)
+          expect(json_response['issues_enabled']).to eq(false)
+          expect(new_project.reload.repository.storage).to eq('default')
+        end
+      end
 
-      expect(project.reload.external_authorization_classification_label).to eq('new label')
+      context 'as an admin' do
+        let(:admin) { create(:admin) }
+
+        it 'returns 500 when repository storage is unknown' do
+          put(api("/projects/#{new_project.id}", admin), repository_storage: unknown_storage)
+
+          expect(response).to have_gitlab_http_status(500)
+          expect(json_response['message']).to match('ArgumentError')
+        end
+
+        it 'returns 200 when repository storage has changed' do
+          stub_storage_settings('extra' => { 'path' => 'tmp/tests/extra_storage' })
+
+          expect {
+            Sidekiq::Testing.fake! do
+              put(api("/projects/#{new_project.id}", admin), repository_storage: 'extra')
+            end
+          }.to change(ProjectUpdateRepositoryStorageWorker.jobs, :size).by(1)
+
+          expect(response).to have_gitlab_http_status(200)
+        end
+      end
     end
 
     context 'when updating mirror related attributes' do