Merge branch 'bvl-security-9-2-28917-contain-uploads-in-system-dir' into 'security-9-2'

(security-9-2) Upload files into `public/upload/system` instead of `public/upload`

See merge request !2104

Conflicts:
  app/validators/dynamic_path_validator.rb

Fixed conflicts based on 3c7c859c359bf5d3955dd300d6861ff33af21ca7

22 files changed, 487 insertions, 21 deletions

diff --git a/spec/factories/uploads.rb b/spec/factories/uploads.rb
new file mode 100644
index 00000000000..1383420fb44
--- /dev/null
+++ b/spec/factories/uploads.rb
@@ -0,0 +1,8 @@
+FactoryGirl.define do
+  factory :upload do
+    model { build(:project) }
+    path { "uploads/system/project/avatar/avatar.jpg" }
+    size 100.kilobytes
+    uploader "AvatarUploader"
+  end
+end
diff --git a/spec/features/admin/admin_appearance_spec.rb b/spec/features/admin/admin_appearance_spec.rb
index 96d715ef383..595366ce352 100644
--- a/spec/features/admin/admin_appearance_spec.rb
+++ b/spec/features/admin/admin_appearance_spec.rb
@@ -63,11 +63,11 @@ feature 'Admin Appearance', feature: true do
   end
 
   def logo_selector
-    '//img[@src^="/uploads/appearance/logo"]'
+    '//img[@src^="/uploads/system/appearance/logo"]'
   end
 
   def header_logo_selector
-    '//img[@src^="/uploads/appearance/header_logo"]'
+    '//img[@src^="/uploads/system/appearance/header_logo"]'
   end
 
   def logo_fixture
diff --git a/spec/features/uploads/user_uploads_avatar_to_group_spec.rb b/spec/features/uploads/user_uploads_avatar_to_group_spec.rb
index f88a515f7fc..d9d6f2e2382 100644
--- a/spec/features/uploads/user_uploads_avatar_to_group_spec.rb
+++ b/spec/features/uploads/user_uploads_avatar_to_group_spec.rb
@@ -18,7 +18,7 @@ feature 'User uploads avatar to group', feature: true do
 
     visit group_path(group)
 
-    expect(page).to have_selector(%Q(img[src$="/uploads/group/avatar/#{group.id}/dk.png"]))
+    expect(page).to have_selector(%Q(img[src$="/uploads/system/group/avatar/#{group.id}/dk.png"]))
 
     # Cheating here to verify something that isn't user-facing, but is important
     expect(group.reload.avatar.file).to exist
diff --git a/spec/features/uploads/user_uploads_avatar_to_profile_spec.rb b/spec/features/uploads/user_uploads_avatar_to_profile_spec.rb
index 0dfd29045e5..eb8dbd76aab 100644
--- a/spec/features/uploads/user_uploads_avatar_to_profile_spec.rb
+++ b/spec/features/uploads/user_uploads_avatar_to_profile_spec.rb
@@ -16,7 +16,7 @@ feature 'User uploads avatar to profile', feature: true do
 
     visit user_path(user)
 
-    expect(page).to have_selector(%Q(img[src$="/uploads/user/avatar/#{user.id}/dk.png"]))
+    expect(page).to have_selector(%Q(img[src$="/uploads/system/user/avatar/#{user.id}/dk.png"]))
 
     # Cheating here to verify something that isn't user-facing, but is important
     expect(user.reload.avatar.file).to exist
diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb
index 01bdf01ad22..1c4ea46f9cd 100644
--- a/spec/helpers/application_helper_spec.rb
+++ b/spec/helpers/application_helper_spec.rb
@@ -1,3 +1,4 @@
+# coding: utf-8
 require 'spec_helper'
 
 describe ApplicationHelper do
@@ -57,7 +58,7 @@ describe ApplicationHelper do
     it 'returns an url for the avatar' do
       project = create(:empty_project, avatar: File.open(uploaded_image_temp_path))
 
-      avatar_url = "http://#{Gitlab.config.gitlab.host}/uploads/project/avatar/#{project.id}/banana_sample.gif"
+      avatar_url = "http://#{Gitlab.config.gitlab.host}/uploads/system/project/avatar/#{project.id}/banana_sample.gif"
       expect(helper.project_icon(project.full_path).to_s).
         to eq "<img src=\"#{avatar_url}\" alt=\"Banana sample\" />"
     end
@@ -78,7 +79,7 @@ describe ApplicationHelper do
       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")
+        to match("/uploads/system/user/avatar/#{user.id}/banana_sample.gif")
     end
 
     it 'returns an url for the avatar with relative url' do
@@ -89,7 +90,7 @@ describe ApplicationHelper do
       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")
+        to match("/gitlab/uploads/system/user/avatar/#{user.id}/banana_sample.gif")
     end
 
     it 'calls gravatar_icon when no User exists with the given email' do
@@ -103,7 +104,7 @@ describe ApplicationHelper do
         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")
+          to match("/uploads/system/user/avatar/#{user.id}/banana_sample.gif")
       end
     end
   end
diff --git a/spec/helpers/emails_helper_spec.rb b/spec/helpers/emails_helper_spec.rb
index cd112dbb2fb..c68e4f56b05 100644
--- a/spec/helpers/emails_helper_spec.rb
+++ b/spec/helpers/emails_helper_spec.rb
@@ -52,7 +52,7 @@ describe EmailsHelper do
         )
 
         expect(header_logo).to eq(
-          %{<img style="height: 50px" src="/uploads/appearance/header_logo/#{appearance.id}/dk.png" alt="Dk" />}
+          %{<img style="height: 50px" src="/uploads/system/appearance/header_logo/#{appearance.id}/dk.png" alt="Dk" />}
         )
       end
     end
diff --git a/spec/helpers/groups_helper_spec.rb b/spec/helpers/groups_helper_spec.rb
index c8b0d86425f..0337afa4452 100644
--- a/spec/helpers/groups_helper_spec.rb
+++ b/spec/helpers/groups_helper_spec.rb
@@ -9,7 +9,7 @@ describe GroupsHelper do
       group.avatar = fixture_file_upload(avatar_file_path)
       group.save!
       expect(group_icon(group.path).to_s).
-        to match("/uploads/group/avatar/#{group.id}/banana_sample.gif")
+        to match("/uploads/system/group/avatar/#{group.id}/banana_sample.gif")
     end
 
     it 'gives default avatar_icon when no avatar is present' do
diff --git a/spec/helpers/page_layout_helper_spec.rb b/spec/helpers/page_layout_helper_spec.rb
index 2cc0b40b2d0..dff2784f21f 100644
--- a/spec/helpers/page_layout_helper_spec.rb
+++ b/spec/helpers/page_layout_helper_spec.rb
@@ -60,7 +60,7 @@ describe PageLayoutHelper do
     %w(project user group).each do |type|
       context "with @#{type} assigned" do
         it "uses #{type.titlecase} avatar if available" do
-          object = double(avatar_url: 'http://example.com/uploads/avatar.png')
+          object = double(avatar_url: 'http://example.com/uploads/system/avatar.png')
           assign(type, object)
 
           expect(helper.page_image).to eq object.avatar_url
diff --git a/spec/javascripts/vue_shared/components/commit_spec.js b/spec/javascripts/vue_shared/components/commit_spec.js
index df547299d75..01dabf5320e 100644
--- a/spec/javascripts/vue_shared/components/commit_spec.js
+++ b/spec/javascripts/vue_shared/components/commit_spec.js
@@ -22,7 +22,7 @@ describe('Commit component', () => {
         shortSha: 'b7836edd',
         title: 'Commit message',
         author: {
-          avatar_url: 'https://gitlab.com/uploads/user/avatar/300478/avatar.png',
+          avatar_url: 'https://gitlab.com/uploads/system/user/avatar/300478/avatar.png',
           web_url: 'https://gitlab.com/jschatz1',
           username: 'jschatz1',
         },
@@ -44,7 +44,7 @@ describe('Commit component', () => {
         shortSha: 'b7836edd',
         title: 'Commit message',
         author: {
-          avatar_url: 'https://gitlab.com/uploads/user/avatar/300478/avatar.png',
+          avatar_url: 'https://gitlab.com/uploads/system/user/avatar/300478/avatar.png',
           web_url: 'https://gitlab.com/jschatz1',
           username: 'jschatz1',
         },
diff --git a/spec/migrations/clean_upload_symlinks_spec.rb b/spec/migrations/clean_upload_symlinks_spec.rb
new file mode 100644
index 00000000000..cecb3ddac53
--- /dev/null
+++ b/spec/migrations/clean_upload_symlinks_spec.rb
@@ -0,0 +1,46 @@
+require 'spec_helper'
+require Rails.root.join('db', 'post_migrate', '20170406111121_clean_upload_symlinks.rb')
+
+describe CleanUploadSymlinks do
+  let(:migration) { described_class.new }
+  let(:test_dir) { File.join(Rails.root, "tmp", "tests", "move_uploads_test") }
+  let(:uploads_dir) { File.join(test_dir, "public", "uploads") }
+  let(:new_uploads_dir) { File.join(uploads_dir, "system") }
+  let(:original_path) { File.join(new_uploads_dir, 'user') }
+  let(:symlink_path) { File.join(uploads_dir, 'user') }
+
+  before do
+    FileUtils.remove_dir(test_dir) if File.directory?(test_dir)
+    FileUtils.mkdir_p(uploads_dir)
+    allow(migration).to receive(:base_directory).and_return(test_dir)
+    allow(migration).to receive(:say)
+  end
+
+  describe "#up" do
+    before do
+      FileUtils.mkdir_p(original_path)
+      FileUtils.ln_s(original_path, symlink_path)
+    end
+
+    it 'removes the symlink' do
+      migration.up
+
+      expect(File.symlink?(symlink_path)).to be(false)
+    end
+  end
+
+  describe '#down' do
+    before do
+      FileUtils.mkdir_p(File.join(original_path))
+      FileUtils.touch(File.join(original_path, 'dummy.file'))
+    end
+
+    it 'creates a symlink' do
+      expected_path = File.join(symlink_path, "dummy.file")
+      migration.down
+
+      expect(File.exist?(expected_path)).to be(true)
+      expect(File.symlink?(symlink_path)).to be(true)
+    end
+  end
+end
diff --git a/spec/migrations/move_uploads_to_system_dir_spec.rb b/spec/migrations/move_uploads_to_system_dir_spec.rb
new file mode 100644
index 00000000000..37d66452447
--- /dev/null
+++ b/spec/migrations/move_uploads_to_system_dir_spec.rb
@@ -0,0 +1,68 @@
+require "spec_helper"
+require Rails.root.join("db", "migrate", "20170316163845_move_uploads_to_system_dir.rb")
+
+describe MoveUploadsToSystemDir do
+  let(:migration) { described_class.new }
+  let(:test_dir) { File.join(Rails.root, "tmp", "move_uploads_test") }
+  let(:uploads_dir) { File.join(test_dir, "public", "uploads") }
+  let(:new_uploads_dir) { File.join(uploads_dir, "system") }
+
+  before do
+    FileUtils.remove_dir(test_dir) if File.directory?(test_dir)
+    FileUtils.mkdir_p(uploads_dir)
+    allow(migration).to receive(:base_directory).and_return(test_dir)
+    allow(migration).to receive(:say)
+  end
+
+  describe "#up" do
+    before do
+      FileUtils.mkdir_p(File.join(uploads_dir, 'user'))
+      FileUtils.touch(File.join(uploads_dir, 'user', 'dummy.file'))
+    end
+
+    it 'moves the directory to the new path' do
+      expected_path = File.join(new_uploads_dir, 'user', 'dummy.file')
+
+      migration.up
+
+      expect(File.exist?(expected_path)).to be(true)
+    end
+
+    it 'creates a symlink in the old location' do
+      symlink_path = File.join(uploads_dir, 'user')
+      expected_path = File.join(symlink_path, 'dummy.file')
+
+      migration.up
+
+      expect(File.exist?(expected_path)).to be(true)
+      expect(File.symlink?(symlink_path)).to be(true)
+    end
+  end
+
+  describe "#down" do
+    before do
+      FileUtils.mkdir_p(File.join(new_uploads_dir, 'user'))
+      FileUtils.touch(File.join(new_uploads_dir, 'user', 'dummy.file'))
+    end
+
+    it 'moves the directory to the old path' do
+      expected_path = File.join(uploads_dir, 'user', 'dummy.file')
+
+      migration.down
+
+      expect(File.exist?(expected_path)).to be(true)
+    end
+
+    it 'removes the symlink if it existed' do
+      FileUtils.ln_s(File.join(new_uploads_dir, 'user'), File.join(uploads_dir, 'user'))
+
+      directory = File.join(uploads_dir, 'user')
+      expected_path = File.join(directory, 'dummy.file')
+
+      migration.down
+
+      expect(File.exist?(expected_path)).to be(true)
+      expect(File.symlink?(directory)).to be(false)
+    end
+  end
+end
diff --git a/spec/migrations/rename_system_namespaces_spec.rb b/spec/migrations/rename_system_namespaces_spec.rb
new file mode 100644
index 00000000000..ad1b83d8e2e
--- /dev/null
+++ b/spec/migrations/rename_system_namespaces_spec.rb
@@ -0,0 +1,252 @@
+require "spec_helper"
+require Rails.root.join("db", "migrate", "20170316163800_rename_system_namespaces.rb")
+
+describe RenameSystemNamespaces, truncate: true do
+  let(:migration) { described_class.new }
+  let(:test_dir) { File.join(Rails.root, "tmp", "tests", "rename_namespaces_test") }
+  let(:uploads_dir) { File.join(test_dir, "public", "uploads") }
+  let(:system_namespace) do
+    namespace = build(:namespace, path: "system")
+    namespace.save(validate: false)
+    namespace
+  end
+
+  def save_invalid_routable(routable)
+    routable.__send__(:prepare_route)
+    routable.save(validate: false)
+  end
+
+  before do
+    FileUtils.remove_dir(test_dir) if File.directory?(test_dir)
+    FileUtils.mkdir_p(uploads_dir)
+    FileUtils.remove_dir(TestEnv.repos_path) if File.directory?(TestEnv.repos_path)
+    allow(migration).to receive(:say)
+    allow(migration).to receive(:uploads_dir).and_return(uploads_dir)
+  end
+
+  describe "#system_namespace" do
+    it "only root namespaces called with path `system`" do
+      system_namespace
+      system_namespace_with_parent = build(:namespace, path: 'system', parent: create(:namespace))
+      system_namespace_with_parent.save(validate: false)
+
+      expect(migration.system_namespace.id).to eq(system_namespace.id)
+    end
+  end
+
+  describe "#up" do
+    before do
+      system_namespace
+    end
+
+    it "doesn't break if there are no namespaces called system" do
+      Namespace.delete_all
+
+      migration.up
+    end
+
+    it "renames namespaces called system" do
+      migration.up
+
+      expect(system_namespace.reload.path).to eq("system0")
+    end
+
+    it "renames the route to the namespace" do
+      migration.up
+
+      expect(system_namespace.reload.full_path).to eq("system0")
+    end
+
+    it "renames the route for projects of the namespace" do
+      project = build(:project, path: "project-path", namespace: system_namespace)
+      save_invalid_routable(project)
+
+      migration.up
+
+      expect(project.route.reload.path).to eq("system0/project-path")
+    end
+
+    it "doesn't touch routes of namespaces that look like system" do
+      namespace = create(:group, path: 'systemlookalike')
+      project = create(:project, namespace: namespace, path: 'the-project')
+
+      migration.up
+
+      expect(project.route.reload.path).to eq('systemlookalike/the-project')
+      expect(namespace.route.reload.path).to eq('systemlookalike')
+    end
+
+    it "moves the the repository for a project in the namespace" do
+      project = build(:project, namespace: system_namespace, path: "system-project")
+      save_invalid_routable(project)
+      TestEnv.copy_repo(project)
+      expected_repo = File.join(TestEnv.repos_path, "system0", "system-project.git")
+
+      migration.up
+
+      expect(File.directory?(expected_repo)).to be(true)
+    end
+
+    it "moves the uploads for the namespace" do
+      allow(migration).to receive(:move_namespace_folders).with(Settings.pages.path, "system", "system0")
+      expect(migration).to receive(:move_namespace_folders).with(uploads_dir, "system", "system0")
+
+      migration.up
+    end
+
+    it "moves the pages for the namespace" do
+      allow(migration).to receive(:move_namespace_folders).with(uploads_dir, "system", "system0")
+      expect(migration).to receive(:move_namespace_folders).with(Settings.pages.path, "system", "system0")
+
+      migration.up
+    end
+
+    describe "clears the markdown cache for projects in the system namespace" do
+      let!(:project) do
+        project = build(:project, namespace: system_namespace)
+        save_invalid_routable(project)
+        project
+      end
+
+      it 'removes description_html from projects' do
+        migration.up
+
+        expect(project.reload.description_html).to be_nil
+      end
+
+      it 'removes issue descriptions' do
+        issue = create(:issue, project: project, description_html: 'Issue description')
+
+        migration.up
+
+        expect(issue.reload.description_html).to be_nil
+      end
+
+      it 'removes merge request descriptions' do
+        merge_request = create(:merge_request,
+                               source_project: project,
+                               target_project: project,
+                               description_html: 'MergeRequest description')
+
+        migration.up
+
+        expect(merge_request.reload.description_html).to be_nil
+      end
+
+      it 'removes note html' do
+        note = create(:note,
+                      project: project,
+                      noteable: create(:issue, project: project),
+                      note_html: 'note description')
+
+        migration.up
+
+        expect(note.reload.note_html).to be_nil
+      end
+
+      it 'removes milestone description' do
+        milestone = create(:milestone,
+                           project: project,
+                           description_html: 'milestone description')
+
+        migration.up
+
+        expect(milestone.reload.description_html).to be_nil
+      end
+    end
+
+    context "system namespace -> subgroup -> system0 project" do
+      it "updates the route of the project correctly" do
+        subgroup = build(:group, path: "subgroup", parent: system_namespace)
+        save_invalid_routable(subgroup)
+        project = build(:project, path: "system0", namespace: subgroup)
+        save_invalid_routable(project)
+
+        migration.up
+
+        expect(project.route.reload.path).to eq("system0/subgroup/system0")
+      end
+    end
+  end
+
+  describe "#move_repositories" do
+    let(:namespace) { create(:group, name: "hello-group") }
+    it "moves a project for a namespace" do
+      create(:project, namespace: namespace, path: "hello-project")
+      expected_path = File.join(TestEnv.repos_path, "bye-group", "hello-project.git")
+
+      migration.move_repositories(namespace, "hello-group", "bye-group")
+
+      expect(File.directory?(expected_path)).to be(true)
+    end
+
+    it "moves a namespace in a subdirectory correctly" do
+      child_namespace = create(:group, name: "sub-group", parent: namespace)
+      create(:project, namespace: child_namespace, path: "hello-project")
+
+      expected_path = File.join(TestEnv.repos_path, "hello-group", "renamed-sub-group", "hello-project.git")
+
+      migration.move_repositories(child_namespace, "hello-group/sub-group", "hello-group/renamed-sub-group")
+
+      expect(File.directory?(expected_path)).to be(true)
+    end
+
+    it "moves a parent namespace with subdirectories" do
+      child_namespace = create(:group, name: "sub-group", parent: namespace)
+      create(:project, namespace: child_namespace, path: "hello-project")
+      expected_path = File.join(TestEnv.repos_path, "renamed-group", "sub-group", "hello-project.git")
+
+      migration.move_repositories(child_namespace, "hello-group", "renamed-group")
+
+      expect(File.directory?(expected_path)).to be(true)
+    end
+  end
+
+  describe "#move_namespace_folders" do
+    it "moves a namespace with files" do
+      source = File.join(uploads_dir, "parent-group", "sub-group")
+      FileUtils.mkdir_p(source)
+      destination = File.join(uploads_dir, "parent-group", "moved-group")
+      FileUtils.touch(File.join(source, "test.txt"))
+      expected_file = File.join(destination, "test.txt")
+
+      migration.move_namespace_folders(uploads_dir, File.join("parent-group", "sub-group"), File.join("parent-group", "moved-group"))
+
+      expect(File.exist?(expected_file)).to be(true)
+    end
+
+    it "moves a parent namespace uploads" do
+      source = File.join(uploads_dir, "parent-group", "sub-group")
+      FileUtils.mkdir_p(source)
+      destination = File.join(uploads_dir, "moved-parent", "sub-group")
+      FileUtils.touch(File.join(source, "test.txt"))
+      expected_file = File.join(destination, "test.txt")
+
+      migration.move_namespace_folders(uploads_dir, "parent-group", "moved-parent")
+
+      expect(File.exist?(expected_file)).to be(true)
+    end
+  end
+
+  describe "#child_ids_for_parent" do
+    it "collects child ids for all levels" do
+      parent = create(:namespace)
+      first_child = create(:namespace, parent: parent)
+      second_child = create(:namespace, parent: parent)
+      third_child = create(:namespace, parent: second_child)
+      all_ids = [parent.id, first_child.id, second_child.id, third_child.id]
+
+      collected_ids = migration.child_ids_for_parent(parent, ids: [parent.id])
+
+      expect(collected_ids).to contain_exactly(*all_ids)
+    end
+  end
+
+  describe "#remove_last_ocurrence" do
+    it "removes only the last occurance of a string" do
+      input = "this/is/system/namespace/with/system"
+
+      expect(migration.remove_last_occurrence(input, "system")).to eq("this/is/system/namespace/with/")
+    end
+  end
+end
diff --git a/spec/migrations/update_upload_paths_to_system_spec.rb b/spec/migrations/update_upload_paths_to_system_spec.rb
new file mode 100644
index 00000000000..7df44515424
--- /dev/null
+++ b/spec/migrations/update_upload_paths_to_system_spec.rb
@@ -0,0 +1,53 @@
+require "spec_helper"
+require Rails.root.join("db", "post_migrate", "20170317162059_update_upload_paths_to_system.rb")
+
+describe UpdateUploadPathsToSystem do
+  let(:migration) { described_class.new }
+
+  before do
+    allow(migration).to receive(:say)
+  end
+
+  describe "#uploads_to_switch_to_new_path" do
+    it "contains only uploads with the old path for the correct models" do
+      _upload_for_other_type = create(:upload, model: create(:ci_pipeline), path: "uploads/ci_pipeline/avatar.jpg")
+      _upload_with_system_path = create(:upload, model: create(:empty_project), path: "uploads/system/project/avatar.jpg")
+      _upload_with_other_path = create(:upload, model: create(:empty_project), path: "thelongsecretforafileupload/avatar.jpg")
+      old_upload = create(:upload, model: create(:empty_project), path: "uploads/project/avatar.jpg")
+      group_upload = create(:upload, model: create(:group), path: "uploads/group/avatar.jpg")
+
+      expect(Upload.where(migration.uploads_to_switch_to_new_path)).to contain_exactly(old_upload, group_upload)
+    end
+  end
+
+  describe "#uploads_to_switch_to_old_path" do
+    it "contains only uploads with the new path for the correct models" do
+      _upload_for_other_type = create(:upload, model: create(:ci_pipeline), path: "uploads/ci_pipeline/avatar.jpg")
+      upload_with_system_path = create(:upload, model: create(:empty_project), path: "uploads/system/project/avatar.jpg")
+      _upload_with_other_path = create(:upload, model: create(:empty_project), path: "thelongsecretforafileupload/avatar.jpg")
+      _old_upload = create(:upload, model: create(:empty_project), path: "uploads/project/avatar.jpg")
+
+      expect(Upload.where(migration.uploads_to_switch_to_old_path)).to contain_exactly(upload_with_system_path)
+    end
+  end
+
+  describe "#up", truncate: true do
+    it "updates old upload records to the new path" do
+      old_upload = create(:upload, model: create(:empty_project), path: "uploads/project/avatar.jpg")
+
+      migration.up
+
+      expect(old_upload.reload.path).to eq("uploads/system/project/avatar.jpg")
+    end
+  end
+
+  describe "#down", truncate: true do
+    it "updates the new system patsh to the old paths" do
+      new_upload = create(:upload, model: create(:empty_project), path: "uploads/system/project/avatar.jpg")
+
+      migration.down
+
+      expect(new_upload.reload.path).to eq("uploads/project/avatar.jpg")
+    end
+  end
+end
diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb
index 3d60e52f23f..365df733c30 100644
--- a/spec/models/group_spec.rb
+++ b/spec/models/group_spec.rb
@@ -185,7 +185,7 @@ describe Group, models: true do
         group.add_master(user)
       end
 
-      let(:avatar_path) { "/uploads/group/avatar/#{group.id}/dk.png" }
+      let(:avatar_path) { "/uploads/system/group/avatar/#{group.id}/dk.png" }
 
       it { should eq "http://#{Gitlab.config.gitlab.host}#{avatar_path}" }
     end
diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb
index 312302afdbb..5fc43eab283 100644
--- a/spec/models/namespace_spec.rb
+++ b/spec/models/namespace_spec.rb
@@ -43,6 +43,12 @@ describe Namespace, models: true do
         end
       end
 
+      context "is case insensitive" do
+        let(:group) { build(:group, path: "System") }
+
+        it { expect(group).not_to be_valid }
+      end
+
       context 'top-level group' do
         let(:group) { build(:group, path: 'tree') }
 
@@ -178,8 +184,8 @@ describe Namespace, models: true do
       let(:parent) { create(:group, name: 'parent', path: 'parent') }
       let(:child) { create(:group, name: 'child', path: 'child', parent: parent) }
       let!(:project) { create(:project_empty_repo, path: 'the-project', namespace: child) }
-      let(:uploads_dir) { File.join(CarrierWave.root, 'uploads') }
-      let(:pages_dir) { TestEnv.pages_path }
+      let(:uploads_dir) { File.join(CarrierWave.root, GitlabUploader.base_dir) }
+      let(:pages_dir) { File.join(TestEnv.pages_path) }
 
       before do
         FileUtils.mkdir_p(File.join(uploads_dir, 'parent', 'child', 'the-project'))
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 429b3dd83af..692f28ea5e7 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -812,7 +812,7 @@ describe Project, models: true do
 
     context 'when avatar file is uploaded' do
       let(:project) { create(:empty_project, :with_avatar) }
-      let(:avatar_path) { "/uploads/project/avatar/#{project.id}/dk.png" }
+      let(:avatar_path) { "/uploads/system/project/avatar/#{project.id}/dk.png" }
 
       it { should eq "http://#{Gitlab.config.gitlab.host}#{avatar_path}" }
     end
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 9be4996192b..76a66888658 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -948,7 +948,7 @@ describe User, models: true do
     subject { user.avatar_url }
 
     context 'when avatar file is uploaded' do
-      let(:avatar_path) { "/uploads/user/avatar/#{user.id}/dk.png" }
+      let(:avatar_path) { "/uploads/system/user/avatar/#{user.id}/dk.png" }
 
       it { should eq "http://#{Gitlab.config.gitlab.host}#{avatar_path}" }
     end
diff --git a/spec/requests/openid_connect_spec.rb b/spec/requests/openid_connect_spec.rb
index a4f85c22943..75d8fc92a43 100644
--- a/spec/requests/openid_connect_spec.rb
+++ b/spec/requests/openid_connect_spec.rb
@@ -79,7 +79,7 @@ describe 'OpenID Connect requests' do
           'email_verified' => true,
           'website'        => 'https://example.com',
           'profile'        => 'http://localhost/alice',
-          'picture'        => "http://localhost/uploads/user/avatar/#{user.id}/dk.png",
+          'picture'        => "http://localhost/uploads/system/user/avatar/#{user.id}/dk.png",
         })
       end
     end
diff --git a/spec/services/projects/participants_service_spec.rb b/spec/services/projects/participants_service_spec.rb
index 063b3bd76eb..d524c9aff17 100644
--- a/spec/services/projects/participants_service_spec.rb
+++ b/spec/services/projects/participants_service_spec.rb
@@ -14,7 +14,7 @@ describe Projects::ParticipantsService, services: true do
         groups = participants.groups
 
         expect(groups.size).to eq 1
-        expect(groups.first[:avatar_url]).to eq "#{base_url}/uploads/group/avatar/#{group.id}/dk.png"
+        expect(groups.first[:avatar_url]).to eq "#{base_url}/uploads/system/group/avatar/#{group.id}/dk.png"
       end
 
       it 'should return an url for the avatar with relative url' do
@@ -25,7 +25,7 @@ describe Projects::ParticipantsService, services: true do
         groups = participants.groups
 
         expect(groups.size).to eq 1
-        expect(groups.first[:avatar_url]).to eq "#{base_url}/gitlab/uploads/group/avatar/#{group.id}/dk.png"
+        expect(groups.first[:avatar_url]).to eq "#{base_url}/gitlab/uploads/system/group/avatar/#{group.id}/dk.png"
       end
     end
   end
diff --git a/spec/uploaders/attachment_uploader_spec.rb b/spec/uploaders/attachment_uploader_spec.rb
index ea714fb08f0..d82dbe871d5 100644
--- a/spec/uploaders/attachment_uploader_spec.rb
+++ b/spec/uploaders/attachment_uploader_spec.rb
@@ -3,6 +3,17 @@ require 'spec_helper'
 describe AttachmentUploader do
   let(:uploader) { described_class.new(build_stubbed(:user)) }
 
+  describe "#store_dir" do
+    it "stores in the system dir" do
+      expect(uploader.store_dir).to start_with("uploads/system/user")
+    end
+
+    it "uses the old path when using object storage" do
+      expect(described_class).to receive(:file_storage?).and_return(false)
+      expect(uploader.store_dir).to start_with("uploads/user")
+    end
+  end
+
   describe '#move_to_cache' do
     it 'is true' do
       expect(uploader.move_to_cache).to eq(true)
diff --git a/spec/uploaders/avatar_uploader_spec.rb b/spec/uploaders/avatar_uploader_spec.rb
index c4d558805ab..201fe6949aa 100644
--- a/spec/uploaders/avatar_uploader_spec.rb
+++ b/spec/uploaders/avatar_uploader_spec.rb
@@ -3,6 +3,17 @@ require 'spec_helper'
 describe AvatarUploader do
   let(:uploader) { described_class.new(build_stubbed(:user)) }
 
+  describe "#store_dir" do
+    it "stores in the system dir" do
+      expect(uploader.store_dir).to start_with("uploads/system/user")
+    end
+
+    it "uses the old path when using object storage" do
+      expect(described_class).to receive(:file_storage?).and_return(false)
+      expect(uploader.store_dir).to start_with("uploads/user")
+    end
+  end
+
   describe '#move_to_cache' do
     it 'is false' do
       expect(uploader.move_to_cache).to eq(false)
diff --git a/spec/uploaders/file_uploader_spec.rb b/spec/uploaders/file_uploader_spec.rb
index d9113ef4095..47e9365e13d 100644
--- a/spec/uploaders/file_uploader_spec.rb
+++ b/spec/uploaders/file_uploader_spec.rb
@@ -15,6 +15,16 @@ describe FileUploader do
     end
   end
 
+  describe "#store_dir" do
+    it "stores in the namespace path" do
+      project = build_stubbed(:empty_project)
+      uploader = described_class.new(project)
+
+      expect(uploader.store_dir).to include(project.path_with_namespace)
+      expect(uploader.store_dir).not_to include("system")
+    end
+  end
+
   describe 'initialize' do
     it 'generates a secret if none is provided' do
       expect(SecureRandom).to receive(:hex).and_return('secret')