Merge branch 'security-persist-tmp-snippet-uploads-11-11' into '11-11-stable'

Persist tmp snippet uploads at users

See merge request gitlab/gitlabhq!3165

2 files changed, 110 insertions, 71 deletions

diff --git a/spec/controllers/snippets_controller_spec.rb b/spec/controllers/snippets_controller_spec.rb
index f8666a1986f..3aba02bf3ff 100644
--- a/spec/controllers/snippets_controller_spec.rb
+++ b/spec/controllers/snippets_controller_spec.rb
@@ -209,8 +209,8 @@ describe SnippetsController do
     context 'when the snippet description contains a file' do
       include FileMoverHelpers
 
-      let(:picture_file) { '/-/system/temp/secret56/picture.jpg' }
-      let(:text_file) { '/-/system/temp/secret78/text.txt' }
+      let(:picture_file) { "/-/system/user/#{user.id}/secret56/picture.jpg" }
+      let(:text_file) { "/-/system/user/#{user.id}/secret78/text.txt" }
       let(:description) do
         "Description with picture: ![picture](/uploads#{picture_file}) and "\
         "text: [text.txt](/uploads#{text_file})"
diff --git a/spec/controllers/uploads_controller_spec.rb b/spec/controllers/uploads_controller_spec.rb
index d27658e02cb..0876502a899 100644
--- a/spec/controllers/uploads_controller_spec.rb
+++ b/spec/controllers/uploads_controller_spec.rb
@@ -24,121 +24,160 @@ describe UploadsController do
   let!(:user) { create(:user, avatar: fixture_file_upload("spec/fixtures/dk.png", "image/png")) }
 
   describe 'POST create' do
-    let(:model)   { 'personal_snippet' }
-    let(:snippet) { create(:personal_snippet, :public) }
     let(:jpg)     { fixture_file_upload('spec/fixtures/rails_sample.jpg', 'image/jpg') }
     let(:txt)     { fixture_file_upload('spec/fixtures/doc_sample.txt', 'text/plain') }
 
-    context 'when a user does not have permissions to upload a file' do
-      it "returns 401 when the user is not logged in" do
-        post :create, params: { model: model, id: snippet.id }, format: :json
+    context 'snippet uploads' do
+      let(:model)   { 'personal_snippet' }
+      let(:snippet) { create(:personal_snippet, :public) }
 
-        expect(response).to have_gitlab_http_status(401)
-      end
+      context 'when a user does not have permissions to upload a file' do
+        it "returns 401 when the user is not logged in" do
+          post :create, params: { model: model, id: snippet.id }, format: :json
 
-      it "returns 404 when user can't comment on a snippet" do
-        private_snippet = create(:personal_snippet, :private)
+          expect(response).to have_gitlab_http_status(401)
+        end
 
-        sign_in(user)
-        post :create, params: { model: model, id: private_snippet.id }, format: :json
+        it "returns 404 when user can't comment on a snippet" do
+          private_snippet = create(:personal_snippet, :private)
 
-        expect(response).to have_gitlab_http_status(404)
-      end
-    end
+          sign_in(user)
+          post :create, params: { model: model, id: private_snippet.id }, format: :json
 
-    context 'when a user is logged in' do
-      before do
-        sign_in(user)
+          expect(response).to have_gitlab_http_status(404)
+        end
       end
 
-      it "returns an error without file" do
-        post :create, params: { model: model, id: snippet.id }, format: :json
+      context 'when a user is logged in' do
+        before do
+          sign_in(user)
+        end
 
-        expect(response).to have_gitlab_http_status(422)
-      end
+        it "returns an error without file" do
+          post :create, params: { model: model, id: snippet.id }, format: :json
 
-      it "returns an error with invalid model" do
-        expect { post :create, params: { model: 'invalid', id: snippet.id }, format: :json }
-        .to raise_error(ActionController::UrlGenerationError)
-      end
+          expect(response).to have_gitlab_http_status(422)
+        end
 
-      it "returns 404 status when object not found" do
-        post :create, params: { model: model, id: 9999 }, format: :json
+        it "returns an error with invalid model" do
+          expect { post :create, params: { model: 'invalid', id: snippet.id }, format: :json }
+            .to raise_error(ActionController::UrlGenerationError)
+        end
 
-        expect(response).to have_gitlab_http_status(404)
-      end
+        it "returns 404 status when object not found" do
+          post :create, params: { model: model, id: 9999 }, format: :json
 
-      context 'with valid image' do
-        before do
-          post :create, params: { model: 'personal_snippet', id: snippet.id, file: jpg }, format: :json
+          expect(response).to have_gitlab_http_status(404)
         end
 
-        it 'returns a content with original filename, new link, and correct type.' do
-          expect(response.body).to match '\"alt\":\"rails_sample\"'
-          expect(response.body).to match "\"url\":\"/uploads"
+        context 'with valid image' do
+          before do
+            post :create, params: { model: 'personal_snippet', id: snippet.id, file: jpg }, format: :json
+          end
+
+          it 'returns a content with original filename, new link, and correct type.' do
+            expect(response.body).to match '\"alt\":\"rails_sample\"'
+            expect(response.body).to match "\"url\":\"/uploads"
+          end
+
+          it 'creates a corresponding Upload record' do
+            upload = Upload.last
+
+            aggregate_failures do
+              expect(upload).to exist
+              expect(upload.model).to eq snippet
+            end
+          end
         end
 
-        it 'creates a corresponding Upload record' do
-          upload = Upload.last
+        context 'with valid non-image file' do
+          before do
+            post :create, params: { model: 'personal_snippet', id: snippet.id, file: txt }, format: :json
+          end
 
-          aggregate_failures do
-            expect(upload).to exist
-            expect(upload.model).to eq snippet
+          it 'returns a content with original filename, new link, and correct type.' do
+            expect(response.body).to match '\"alt\":\"doc_sample.txt\"'
+            expect(response.body).to match "\"url\":\"/uploads"
+          end
+
+          it 'creates a corresponding Upload record' do
+            upload = Upload.last
+
+            aggregate_failures do
+              expect(upload).to exist
+              expect(upload.model).to eq snippet
+            end
           end
         end
       end
+    end
+
+    context 'user uploads' do
+      let(:model) { 'user' }
+
+      it 'returns 401 when the user has no access' do
+        post :create, params: { model: 'user', id: user.id }, format: :json
 
-      context 'with valid non-image file' do
+        expect(response).to have_gitlab_http_status(401)
+      end
+
+      context 'when user is logged in' do
         before do
-          post :create, params: { model: 'personal_snippet', id: snippet.id, file: txt }, format: :json
+          sign_in(user)
+        end
+
+        subject do
+          post :create, params: { model: model, id: user.id, file: jpg }, format: :json
         end
 
         it 'returns a content with original filename, new link, and correct type.' do
-          expect(response.body).to match '\"alt\":\"doc_sample.txt\"'
-          expect(response.body).to match "\"url\":\"/uploads"
+          subject
+
+          expect(response.body).to match '\"alt\":\"rails_sample\"'
+          expect(response.body).to match "\"url\":\"/uploads/-/system/user/#{user.id}/"
         end
 
         it 'creates a corresponding Upload record' do
+          expect { subject }.to change { Upload.count }
+
           upload = Upload.last
 
           aggregate_failures do
             expect(upload).to exist
-            expect(upload.model).to eq snippet
+            expect(upload.model).to eq user
           end
         end
-      end
 
-      context 'temporal with valid image' do
-        subject do
-          post :create, params: { model: 'personal_snippet', file: jpg }, format: :json
-        end
+        context 'with valid non-image file' do
+          subject do
+            post :create, params: { model: model, id: user.id, file: txt }, format: :json
+          end
 
-        it 'returns a content with original filename, new link, and correct type.' do
-          subject
+          it 'returns a content with original filename, new link, and correct type.' do
+            subject
 
-          expect(response.body).to match '\"alt\":\"rails_sample\"'
-          expect(response.body).to match "\"url\":\"/uploads/-/system/temp"
-        end
+            expect(response.body).to match '\"alt\":\"doc_sample.txt\"'
+            expect(response.body).to match "\"url\":\"/uploads/-/system/user/#{user.id}/"
+          end
 
-        it 'does not create an Upload record' do
-          expect { subject }.not_to change { Upload.count }
-        end
-      end
+          it 'creates a corresponding Upload record' do
+            expect { subject }.to change { Upload.count }
 
-      context 'temporal with valid non-image file' do
-        subject do
-          post :create, params: { model: 'personal_snippet', file: txt }, format: :json
+            upload = Upload.last
+
+            aggregate_failures do
+              expect(upload).to exist
+              expect(upload.model).to eq user
+            end
+          end
         end
 
-        it 'returns a content with original filename, new link, and correct type.' do
-          subject
+        it 'returns 404 when given user is not the logged in one' do
+          another_user = create(:user)
 
-          expect(response.body).to match '\"alt\":\"doc_sample.txt\"'
-          expect(response.body).to match "\"url\":\"/uploads/-/system/temp"
-        end
+          post :create, params: { model: model, id: another_user.id, file: txt }, format: :json
 
-        it 'does not create an Upload record' do
-          expect { subject }.not_to change { Upload.count }
+          expect(response).to have_gitlab_http_status(404)
         end
       end
     end