Add Gitlab::Middleware::Multipart

8 files changed, 149 insertions, 11 deletions

diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb
index 15863d444f8..92053e5a7c6 100644
--- a/spec/helpers/application_helper_spec.rb
+++ b/spec/helpers/application_helper_spec.rb
@@ -1,6 +1,8 @@
 require 'spec_helper'
 
 describe ApplicationHelper do
+  include UploadHelpers
+
   describe 'current_controller?' do
     it 'returns true when controller matches argument' do
       stub_controller_name('foo')
@@ -52,10 +54,8 @@ describe ApplicationHelper do
   end
 
   describe 'project_icon' do
-    let(:avatar_file_path) { File.join(Rails.root, 'spec', 'fixtures', 'banana_sample.gif') }
-
     it 'returns an url for the avatar' do
-      project = create(:project, avatar: File.open(avatar_file_path))
+      project = create(:project, avatar: File.open(uploaded_image_temp_path))
 
       avatar_url = "http://#{Gitlab.config.gitlab.host}/uploads/project/avatar/#{project.id}/banana_sample.gif"
       expect(helper.project_icon("#{project.namespace.to_param}/#{project.to_param}").to_s).
@@ -74,10 +74,8 @@ describe ApplicationHelper do
   end
 
   describe 'avatar_icon' do
-    let(:avatar_file_path) { File.join(Rails.root, 'spec', 'fixtures', 'banana_sample.gif') }
-
     it 'returns an url for the avatar' do
-      user = create(:user, avatar: File.open(avatar_file_path))
+      user = create(:user, avatar: File.open(uploaded_image_temp_path))
 
       expect(helper.avatar_icon(user.email).to_s).
         to match("/uploads/user/avatar/#{user.id}/banana_sample.gif")
@@ -88,7 +86,7 @@ describe ApplicationHelper do
       # Must be stubbed after the stub above, and separately
       stub_config_setting(url: Settings.send(:build_gitlab_url))
 
-      user = create(:user, avatar: File.open(avatar_file_path))
+      user = create(:user, avatar: File.open(uploaded_image_temp_path))
 
       expect(helper.avatar_icon(user.email).to_s).
         to match("/gitlab/uploads/user/avatar/#{user.id}/banana_sample.gif")
@@ -102,7 +100,7 @@ describe ApplicationHelper do
 
     describe 'using a User' do
       it 'returns an URL for the avatar' do
-        user = create(:user, avatar: File.open(avatar_file_path))
+        user = create(:user, avatar: File.open(uploaded_image_temp_path))
 
         expect(helper.avatar_icon(user).to_s).
           to match("/uploads/user/avatar/#{user.id}/banana_sample.gif")
diff --git a/spec/lib/gitlab/import_export/avatar_restorer_spec.rb b/spec/lib/gitlab/import_export/avatar_restorer_spec.rb
index 5ae178414cc..08a42fd27a2 100644
--- a/spec/lib/gitlab/import_export/avatar_restorer_spec.rb
+++ b/spec/lib/gitlab/import_export/avatar_restorer_spec.rb
@@ -1,12 +1,14 @@
 require 'spec_helper'
 
 describe Gitlab::ImportExport::AvatarRestorer, lib: true do
+  include UploadHelpers
+
   let(:shared) { Gitlab::ImportExport::Shared.new(relative_path: 'test') }
   let(:project) { create(:empty_project) }
 
   before do
     allow_any_instance_of(described_class).to receive(:avatar_export_file)
-                                                .and_return(Rails.root + "spec/fixtures/dk.png")
+                                                .and_return(uploaded_image_temp_path)
   end
 
   after do
diff --git a/spec/lib/gitlab/middleware/multipart_spec.rb b/spec/lib/gitlab/middleware/multipart_spec.rb
new file mode 100644
index 00000000000..c79c6494576
--- /dev/null
+++ b/spec/lib/gitlab/middleware/multipart_spec.rb
@@ -0,0 +1,74 @@
+require 'spec_helper'
+
+require 'tempfile'
+
+describe Gitlab::Middleware::Multipart do
+  let(:app) { double(:app) }
+  let(:middleware) { described_class.new(app) }
+
+  it 'opens top-level files' do
+    Tempfile.open do |tempfile|
+      env = post_env({ 'file' => tempfile.path }, { 'file.name' => 'filename' }, Gitlab::Workhorse.secret, 'gitlab-workhorse')
+
+      expect(app).to receive(:call) do |env|
+        file = Rack::Request.new(env).params['file']
+        expect(file).to be_a(File)
+        expect(file.path).to eq(tempfile.path)
+      end
+
+      middleware.call(env)
+    end
+  end
+
+  it 'rejects headers signed with the wrong secret' do
+    env = post_env({ 'file' => '/var/empty/nonesuch' }, {}, 'x' * 32, 'gitlab-workhorse')
+
+    expect { middleware.call(env) }.to raise_error(JWT::VerificationError)
+  end
+
+  it 'rejects headers signed with the wrong issuer' do
+    env = post_env({ 'file' => '/var/empty/nonesuch' }, {}, Gitlab::Workhorse.secret, 'acme-inc')
+
+    expect { middleware.call(env) }.to raise_error(JWT::InvalidIssuerError)
+  end
+
+  it 'opens files one level deep' do
+    Tempfile.open do |tempfile|
+      in_params = { 'user' => { 'avatar' => { '.name' => 'filename' } } }
+      env = post_env({ 'user[avatar]' => tempfile.path }, in_params, Gitlab::Workhorse.secret, 'gitlab-workhorse')
+
+      expect(app).to receive(:call) do |env|
+        file = Rack::Request.new(env).params['user']['avatar']
+        expect(file).to be_a(File)
+        expect(file.path).to eq(tempfile.path)
+      end
+
+      middleware.call(env)
+    end
+  end
+
+  it 'opens files two levels deep' do
+    Tempfile.open do |tempfile|
+      in_params = { 'project' => { 'milestone' => { 'themesong' => { '.name' => 'filename' } } } }
+      env = post_env({ 'project[milestone][themesong]' => tempfile.path }, in_params, Gitlab::Workhorse.secret, 'gitlab-workhorse')
+
+      expect(app).to receive(:call) do |env|
+        file = Rack::Request.new(env).params['project']['milestone']['themesong']
+        expect(file).to be_a(File)
+        expect(file.path).to eq(tempfile.path)
+      end
+
+      middleware.call(env)
+    end
+  end
+
+  def post_env(rewritten_fields, params, secret, issuer)
+    token = JWT.encode({ 'iss' => issuer, 'rewritten_fields' => rewritten_fields }, secret, 'HS256')
+    Rack::MockRequest.env_for(
+      '/',
+      method: 'post',
+      params: params,
+      described_class::RACK_ENV_KEY => token
+    )
+  end
+end
diff --git a/spec/requests/api/groups_spec.rb b/spec/requests/api/groups_spec.rb
index a75ba824e85..cdeb965b413 100644
--- a/spec/requests/api/groups_spec.rb
+++ b/spec/requests/api/groups_spec.rb
@@ -2,13 +2,13 @@ require 'spec_helper'
 
 describe API::Groups, api: true  do
   include ApiHelpers
+  include UploadHelpers
 
   let(:user1) { create(:user, can_create_group: false) }
   let(:user2) { create(:user) }
   let(:user3) { create(:user) }
   let(:admin) { create(:admin) }
-  let(:avatar_file_path) { File.join(Rails.root, 'spec', 'fixtures', 'banana_sample.gif') }
-  let!(:group1) { create(:group, avatar: File.open(avatar_file_path)) }
+  let!(:group1) { create(:group, avatar: File.open(uploaded_image_temp_path)) }
   let!(:group2) { create(:group, :private) }
   let!(:project1) { create(:project, namespace: group1) }
   let!(:project2) { create(:project, namespace: group2) }
diff --git a/spec/support/upload_helpers.rb b/spec/support/upload_helpers.rb
new file mode 100644
index 00000000000..5eead80c935
--- /dev/null
+++ b/spec/support/upload_helpers.rb
@@ -0,0 +1,16 @@
+require 'fileutils'
+
+module UploadHelpers
+  extend self
+
+  def uploaded_image_temp_path
+    basename = 'banana_sample.gif'
+    orig_path = File.join(Rails.root, 'spec', 'fixtures', basename)
+    tmp_path = File.join(Rails.root, 'tmp', 'tests', basename)
+    # Because we use 'move_to_store' on all uploaders, we create a new
+    # tempfile on each call: the file we return here will be renamed in most
+    # cases.
+    FileUtils.copy(orig_path, tmp_path)
+    tmp_path
+  end
+end
diff --git a/spec/uploaders/attachment_uploader_spec.rb b/spec/uploaders/attachment_uploader_spec.rb
new file mode 100644
index 00000000000..6098be5cd45
--- /dev/null
+++ b/spec/uploaders/attachment_uploader_spec.rb
@@ -0,0 +1,18 @@
+require 'spec_helper'
+
+describe AttachmentUploader do
+  let(:issue) { build(:issue) }
+  subject { described_class.new(issue) }
+
+  describe '#move_to_cache' do
+    it 'is true' do
+      expect(subject.move_to_cache).to eq(true)
+    end
+  end
+
+  describe '#move_to_store' do
+    it 'is true' do
+      expect(subject.move_to_store).to eq(true)
+    end
+  end
+end
diff --git a/spec/uploaders/avatar_uploader_spec.rb b/spec/uploaders/avatar_uploader_spec.rb
new file mode 100644
index 00000000000..1f0e8732587
--- /dev/null
+++ b/spec/uploaders/avatar_uploader_spec.rb
@@ -0,0 +1,18 @@
+require 'spec_helper'
+
+describe AvatarUploader do
+  let(:user) { build(:user) }
+  subject { described_class.new(user) }
+
+  describe '#move_to_cache' do
+    it 'is true' do
+      expect(subject.move_to_cache).to eq(true)
+    end
+  end
+
+  describe '#move_to_store' do
+    it 'is true' do
+      expect(subject.move_to_store).to eq(true)
+    end
+  end
+end
diff --git a/spec/uploaders/file_uploader_spec.rb b/spec/uploaders/file_uploader_spec.rb
index e8300abed5d..bc86adfef68 100644
--- a/spec/uploaders/file_uploader_spec.rb
+++ b/spec/uploaders/file_uploader_spec.rb
@@ -42,4 +42,16 @@ describe FileUploader do
       expect(@uploader.image_or_video?).to be false
     end
   end
+
+  describe '#move_to_cache' do
+    it 'is true' do
+      expect(@uploader.move_to_cache).to eq(true)
+    end
+  end
+
+  describe '#move_to_store' do
+    it 'is true' do
+      expect(@uploader.move_to_store).to eq(true)
+    end
+  end
 end