BigW Consortium Gitlab

Commit 200e8582 by Timothy Andrew

Revert "Merge remote-tracking branch 'dev/security-9-1' into 9-1-stable"

This reverts commit cc1b069f, reversing changes made to 729c75f7.
parent cc1b069f
...@@ -21,7 +21,7 @@ class AutocompleteController < ApplicationController ...@@ -21,7 +21,7 @@ class AutocompleteController < ApplicationController
@users = [current_user, *@users].uniq @users = [current_user, *@users].uniq
end end
if params[:author_id].present? && current_user if params[:author_id].present?
author = User.find_by_id(params[:author_id]) author = User.find_by_id(params[:author_id])
@users = [author, *@users].uniq if author @users = [author, *@users].uniq if author
end end
......
...@@ -13,13 +13,6 @@ class FileUploader < GitlabUploader ...@@ -13,13 +13,6 @@ class FileUploader < GitlabUploader
) )
end end
# Not using `GitlabUploader.base_dir` because all project namespaces are in
# the `public/uploads` dir.
#
def self.base_dir
root_dir
end
# Returns the part of `store_dir` that can change based on the model's current # Returns the part of `store_dir` that can change based on the model's current
# path # path
# #
......
...@@ -3,28 +3,16 @@ class GitlabUploader < CarrierWave::Uploader::Base ...@@ -3,28 +3,16 @@ class GitlabUploader < CarrierWave::Uploader::Base
File.join(CarrierWave.root, upload_record.path) File.join(CarrierWave.root, upload_record.path)
end end
def self.root_dir def self.base_dir
'uploads' 'uploads'
end end
# When object storage is used, keep the `root_dir` as `base_dir`. delegate :base_dir, to: :class
# The files aren't really in folders there, they just have a name.
# The files that contain user input in their name, also contain a hash, so
# the names are still unique
#
# This method is overridden in the `FileUploader`
def self.base_dir
return root_dir unless file_storage?
File.join(root_dir, 'system')
end
def self.file_storage? def file_storage?
self.storage == CarrierWave::Storage::File self.class.storage == CarrierWave::Storage::File
end end
delegate :base_dir, :file_storage?, to: :class
# Reduce disk IO # Reduce disk IO
def move_to_cache def move_to_cache
true true
......
...@@ -33,7 +33,6 @@ class NamespaceValidator < ActiveModel::EachValidator ...@@ -33,7 +33,6 @@ class NamespaceValidator < ActiveModel::EachValidator
u u
unsubscribes unsubscribes
users users
system
].freeze ].freeze
WILDCARD_ROUTES = %w[tree commits wikis new edit create update logs_tree WILDCARD_ROUTES = %w[tree commits wikis new edit create update logs_tree
...@@ -48,9 +47,9 @@ class NamespaceValidator < ActiveModel::EachValidator ...@@ -48,9 +47,9 @@ class NamespaceValidator < ActiveModel::EachValidator
def self.reserved?(value, strict: false) def self.reserved?(value, strict: false)
if strict if strict
STRICT_RESERVED.include?(value.to_s.downcase) STRICT_RESERVED.include?(value)
else else
RESERVED.include?(value.to_s.downcase) RESERVED.include?(value)
end end
end end
......
...@@ -15,7 +15,7 @@ class ProjectPathValidator < ActiveModel::EachValidator ...@@ -15,7 +15,7 @@ class ProjectPathValidator < ActiveModel::EachValidator
# 'tree' as project name and 'deploy_keys' as route. # 'tree' as project name and 'deploy_keys' as route.
# #
RESERVED = (NamespaceValidator::STRICT_RESERVED - RESERVED = (NamespaceValidator::STRICT_RESERVED -
%w[dashboard help ci admin search notes services assets profile public system]).freeze %w[dashboard help ci admin search notes services assets profile public]).freeze
def self.valid?(value) def self.valid?(value)
!reserved?(value) !reserved?(value)
......
---
title: Move uploads from 'public/uploads' to 'public/uploads/system'
merge_request:
author:
---
title: Make Asciidoc & other markup go through pipeline to prevent XSS
merge_request:
author:
---
title: Validate URLs in markdown using URI to detect the host correctly
merge_request:
author:
---
title: Restrict API X-Frame-Options to same origin
merge_request:
author:
---
title: Allow users autocomplete by author_id only for authenticated users
merge_request:
author:
---
title: Fix for XSS in project import view caused by Hamlit filter usage.
merge_request:
author:
scope path: :uploads do scope path: :uploads do
# Note attachments and User/Group/Project avatars # Note attachments and User/Group/Project avatars
get "system/:model/:mounted_as/:id/:filename", get ":model/:mounted_as/:id/:filename",
to: "uploads#show", to: "uploads#show",
constraints: { model: /note|user|group|project/, mounted_as: /avatar|attachment/, filename: /[^\/]+/ } constraints: { model: /note|user|group|project/, mounted_as: /avatar|attachment/, filename: /[^\/]+/ }
# Appearance # Appearance
get "system/:model/:mounted_as/:id/:filename", get ":model/:mounted_as/:id/:filename",
to: "uploads#show", to: "uploads#show",
constraints: { model: /appearance/, mounted_as: /logo|header_logo/, filename: /.+/ } constraints: { model: /appearance/, mounted_as: /logo|header_logo/, filename: /.+/ }
......
# See http://doc.gitlab.com/ce/development/migration_style_guide.html
# for more information on how to write migrations for GitLab.
class RenameSystemNamespaces < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
include Gitlab::ShellAdapter
disable_ddl_transaction!
class User < ActiveRecord::Base
self.table_name = 'users'
end
class Namespace < ActiveRecord::Base
self.table_name = 'namespaces'
belongs_to :parent, class_name: 'RenameSystemNamespaces::Namespace'
has_one :route, as: :source
has_many :children, class_name: 'RenameSystemNamespaces::Namespace', foreign_key: :parent_id
belongs_to :owner, class_name: 'RenameSystemNamespaces::User'
# Overridden to have the correct `source_type` for the `route` relation
def self.name
'Namespace'
end
def full_path
if route && route.path.present?
@full_path ||= route.path
else
update_route if persisted?
build_full_path
end
end
def build_full_path
if parent && path
parent.full_path + '/' + path
else
path
end
end
def update_route
prepare_route
route.save
end
def prepare_route
route || build_route(source: self)
route.path = build_full_path
route.name = build_full_name
@full_path = nil
@full_name = nil
end
def build_full_name
if parent && name
parent.human_name + ' / ' + name
else
name
end
end
def human_name
owner&.name
end
end
class Route < ActiveRecord::Base
self.table_name = 'routes'
belongs_to :source, polymorphic: true
end
class Project < ActiveRecord::Base
self.table_name = 'projects'
def repository_storage_path
Gitlab.config.repositories.storages[repository_storage]['path']
end
end
DOWNTIME = false
def up
return unless system_namespace
old_path = system_namespace.path
old_full_path = system_namespace.full_path
# Only remove the last occurrence of the path name to get the parent namespace path
namespace_path = remove_last_occurrence(old_full_path, old_path)
new_path = rename_path(namespace_path, old_path)
new_full_path = join_namespace_path(namespace_path, new_path)
Namespace.where(id: system_namespace).update_all(path: new_path) # skips callbacks & validations
replace_statement = replace_sql(Route.arel_table[:path], old_full_path, new_full_path)
route_matches = [old_full_path, "#{old_full_path}/%"]
update_column_in_batches(:routes, :path, replace_statement) do |table, query|
query.where(Route.arel_table[:path].matches_any(route_matches))
end
clear_cache_for_namespace(system_namespace)
# tasks here are based on `Namespace#move_dir`
move_repositories(system_namespace, old_full_path, new_full_path)
move_namespace_folders(uploads_dir, old_full_path, new_full_path) if file_storage?
move_namespace_folders(pages_dir, old_full_path, new_full_path)
end
def down
# nothing to do
end
def remove_last_occurrence(string, pattern)
string.reverse.sub(pattern.reverse, "").reverse
end
def move_namespace_folders(directory, old_relative_path, new_relative_path)
old_path = File.join(directory, old_relative_path)
return unless File.directory?(old_path)
new_path = File.join(directory, new_relative_path)
FileUtils.mv(old_path, new_path)
end
def move_repositories(namespace, old_full_path, new_full_path)
repo_paths_for_namespace(namespace).each do |repository_storage_path|
# Ensure old directory exists before moving it
gitlab_shell.add_namespace(repository_storage_path, old_full_path)
unless gitlab_shell.mv_namespace(repository_storage_path, old_full_path, new_full_path)
say "Exception moving path #{repository_storage_path} from #{old_full_path} to #{new_full_path}"
end
end
end
def rename_path(namespace_path, path_was)
counter = 0
path = "#{path_was}#{counter}"
while route_exists?(join_namespace_path(namespace_path, path))
counter += 1
path = "#{path_was}#{counter}"
end
path
end
def route_exists?(full_path)
Route.where(Route.arel_table[:path].matches(full_path)).any?
end
def join_namespace_path(namespace_path, path)
if namespace_path.present?
File.join(namespace_path, path)
else
path
end
end
def system_namespace
@system_namespace ||= Namespace.where(parent_id: nil).
where(arel_table[:path].matches(system_namespace_path)).
first
end
def system_namespace_path
"system"
end
def clear_cache_for_namespace(namespace)
project_ids = projects_for_namespace(namespace).pluck(:id)
update_column_in_batches(:projects, :description_html, nil) do |table, query|
query.where(table[:id].in(project_ids))
end
update_column_in_batches(:issues, :description_html, nil) do |table, query|
query.where(table[:project_id].in(project_ids))
end
update_column_in_batches(:merge_requests, :description_html, nil) do |table, query|
query.where(table[:target_project_id].in(project_ids))
end
update_column_in_batches(:notes, :note_html, nil) do |table, query|
query.where(table[:project_id].in(project_ids))
end
update_column_in_batches(:milestones, :description_html, nil) do |table, query|
query.where(table[:project_id].in(project_ids))
end
end
def projects_for_namespace(namespace)
namespace_ids = child_ids_for_parent(namespace, ids: [namespace.id])
namespace_or_children = Project.arel_table[:namespace_id].in(namespace_ids)
Project.unscoped.where(namespace_or_children)
end
# This won't scale to huge trees, but it should do for a handful of namespaces
# called `system`.
def child_ids_for_parent(namespace, ids: [])
namespace.children.each do |child|
ids << child.id
child_ids_for_parent(child, ids: ids) if child.children.any?
end
ids
end
def repo_paths_for_namespace(namespace)
projects_for_namespace(namespace).distinct.
select(:repository_storage).map(&:repository_storage_path)
end
def uploads_dir
File.join(Rails.root, "public", "uploads")
end
def pages_dir
Settings.pages.path
end
def file_storage?
CarrierWave::Uploader::Base.storage == CarrierWave::Storage::File
end
def arel_table
Namespace.arel_table
end
end
# See http://doc.gitlab.com/ce/development/migration_style_guide.html
# for more information on how to write migrations for GitLab.
class MoveUploadsToSystemDir < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
disable_ddl_transaction!
DOWNTIME = false
DIRECTORIES_TO_MOVE = %w(user project note group appeareance)
def up
return unless file_storage?
FileUtils.mkdir_p(new_upload_dir)
DIRECTORIES_TO_MOVE.each do |dir|
source = File.join(old_upload_dir, dir)
destination = File.join(new_upload_dir, dir)
next unless File.directory?(source)
next if File.directory?(destination)
say "Moving #{source} -> #{destination}"
FileUtils.mv(source, destination)
FileUtils.ln_s(destination, source)
end
end
def down
return unless file_storage?
return unless File.directory?(new_upload_dir)
DIRECTORIES_TO_MOVE.each do |dir|
source = File.join(new_upload_dir, dir)
destination = File.join(old_upload_dir, dir)
next unless File.directory?(source)
next if File.directory?(destination) && !File.symlink?(destination)
say "Moving #{source} -> #{destination}"
FileUtils.rm(destination) if File.symlink?(destination)
FileUtils.mv(source, destination)
end
end
def file_storage?
CarrierWave::Uploader::Base.storage == CarrierWave::Storage::File
end
def base_directory
Rails.root
end
def old_upload_dir
File.join(base_directory, "public", "uploads")
end
def new_upload_dir
File.join(base_directory, "public", "uploads", "system")
end
end
# See http://doc.gitlab.com/ce/development/migration_style_guide.html
# for more information on how to write migrations for GitLab.
class UpdateUploadPathsToSystem < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
AFFECTED_MODELS = %w(User Project Note Namespace Appearance)
def up
update_column_in_batches(:uploads, :path, replace_sql(arel_table[:path], base_directory, new_upload_dir)) do |_table, query|
query.where(uploads_to_switch_to_new_path)
end
end
def down
update_column_in_batches(:uploads, :path, replace_sql(arel_table[:path], new_upload_dir, base_directory)) do |_table, query|
query.where(uploads_to_switch_to_old_path)
end
end
# "SELECT \"uploads\".* FROM \"uploads\" WHERE \"uploads\".\"model_type\" IN ('User', 'Project', 'Note', 'Namespace', 'Appearance') AND (\"uploads\".\"path\" ILIKE 'uploads/%' AND NOT (\"uploads\".\"path\" ILIKE 'uploads/system/%'))"
def uploads_to_switch_to_new_path
affected_uploads.and(starting_with_base_directory).and(starting_with_new_upload_directory.not)
end
# "SELECT \"uploads\".* FROM \"uploads\" WHERE \"uploads\".\"model_type\" IN ('User', 'Project', 'Note', 'Namespace', 'Appearance') AND (\"uploads\".\"path\" ILIKE 'uploads/%' AND \"uploads\".\"path\" ILIKE 'uploads/system/%')"
def uploads_to_switch_to_old_path
affected_uploads.and(starting_with_new_upload_directory)
end
def starting_with_base_directory
arel_table[:path].matches("#{base_directory}/%")
end
def starting_with_new_upload_directory
arel_table[:path].matches("#{new_upload_dir}/%")
end
def affected_uploads
arel_table[:model_type].in(AFFECTED_MODELS)
end
def base_directory
"uploads"
end
def new_upload_dir
File.join(base_directory, "system")
end
def arel_table
Arel::Table.new(:uploads)
end
end
# See http://doc.gitlab.com/ce/development/migration_style_guide.html
# for more information on how to write migrations for GitLab.
class CleanUploadSymlinks < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
disable_ddl_transaction!
DOWNTIME = false
DIRECTORIES_TO_MOVE = %w(user project note group appeareance)
def up
return unless file_storage?
DIRECTORIES_TO_MOVE.each do |dir|
symlink_location = File.join(old_upload_dir, dir)
next unless File.symlink?(symlink_location)
say "removing symlink: #{symlink_location}"
FileUtils.rm(symlink_location)
end
end
def down
return unless file_storage?
DIRECTORIES_TO_MOVE.each do |dir|
symlink = File.join(old_upload_dir, dir)
destination = File.join(new_upload_dir, dir)
next if File.directory?(symlink)
next unless File.directory?(destination)
say "Creating symlink #{symlink} -> #{destination}"
FileUtils.ln_s(destination, symlink)
end
end
def file_storage?
CarrierWave::Uploader::Base.storage == CarrierWave::Storage::File
end
def base_directory
Rails.root
end
def old_upload_dir
File.join(base_directory, "public", "uploads")
end
def new_upload_dir
File.join(base_directory, "public", "uploads", "system")
end
end
...@@ -81,7 +81,7 @@ class Spinach::Features::Groups < Spinach::FeatureSteps ...@@ -81,7 +81,7 @@ class Spinach::Features::Groups < Spinach::FeatureSteps
step 'I should see new group "Owned" avatar' do step 'I should see new group "Owned" avatar' do
expect(owned_group.avatar).to be_instance_of AvatarUploader expect(owned_group.avatar).to be_instance_of AvatarUploader
expect(owned_group.avatar.url).to eq "/uploads/system/group/avatar/#{Group.find_by(name: "Owned").id}/banana_sample.gif" expect(owned_group.avatar.url).to eq "/uploads/group/avatar/#{Group.find_by(name: "Owned").id}/banana_sample.gif"
end end
step 'I should see the "Remove avatar" button' do step 'I should see the "Remove avatar" button' do
......
...@@ -36,7 +36,7 @@ class Spinach::Features::Profile < Spinach::FeatureSteps ...@@ -36,7 +36,7 @@ class Spinach::Features::Profile < Spinach::FeatureSteps
step 'I should see new avatar' do step 'I should see new avatar' do
expect(@user.avatar).to be_instance_of AvatarUploader expect(@user.avatar).to be_instance_of AvatarUploader
expect(@user.avatar.url).to eq "/uploads/system/user/avatar/#{@user.id}/banana_sample.gif" expect(@user.avatar.url).to eq "/uploads/user/avatar/#{@user.id}/banana_sample.gif"
end end
step 'I should see the "Remove avatar" button' do step 'I should see the "Remove avatar" button' do
......
...@@ -37,7 +37,7 @@ class Spinach::Features::Project < Spinach::FeatureSteps ...@@ -37,7 +37,7 @@ class Spinach::Features::Project < Spinach::FeatureSteps
step 'I should see new project avatar' do step 'I should see new project avatar' do
expect(@project.avatar).to be_instance_of AvatarUploader expect(@project.avatar).to be_instance_of AvatarUploader
url = @project.avatar.url url = @project.avatar.url
expect(url).to eq "/uploads/system/project/avatar/#{@project.id}/banana_sample.gif" expect(url).to eq "/uploads/project/avatar/#{@project.id}/banana_sample.gif"
end end
step 'I should see the "Remove avatar" button' do step 'I should see the "Remove avatar" button' do
......
...@@ -44,7 +44,6 @@ module API ...@@ -44,7 +44,6 @@ module API
end end
before { allow_access_with_scope :api } before { allow_access_with_scope :api }
before { header['X-Frame-Options'] = 'SAMEORIGIN' }
rescue_from Gitlab::Access::AccessDeniedError do rescue_from Gitlab::Access::AccessDeniedError do
rack_response({ 'message' => '403 Forbidden' }.to_json, 403) rack_response({ 'message' => '403 Forbidden' }.to_json, 403)
......
...@@ -105,7 +105,7 @@ module Gitlab ...@@ -105,7 +105,7 @@ module Gitlab
# here is based on Rails' foreign_key_name() method, which unfortunately # here is based on Rails' foreign_key_name() method, which unfortunately
# is private so we can't rely on it directly. # is private so we can't rely on it directly.
def concurrent_foreign_key_name(table, column) def concurrent_foreign_key_name(table, column)
"fk_#{Digest::SHA256.hexdigest('#{table}_#{column}_fk').first(10)}" "fk_#{Digest::SHA256.hexdigest("#{table}_#{column}_fk").first(10)}"
end end
# Long-running migrations may take more than the timeout allowed by # Long-running migrations may take more than the timeout allowed by
...@@ -490,29 +490,6 @@ module Gitlab ...@@ -490,29 +490,6 @@ module Gitlab
columns(table).find { |column| column.name == name } columns(table).find { |column| column.name == name }
end end
# This will replace the first occurance of a string in a column with
# the replacement
# On postgresql we can use `regexp_replace` for that.
# On mysql we find the location of the pattern, and overwrite it
# with the replacement
def replace_sql(column, pattern, replacement)
quoted_pattern = Arel::Nodes::Quoted.new(pattern.to_s)
quoted_replacement = Arel::Nodes::Quoted.new(replacement.to_s)
if Database.mysql?
locate = Arel::Nodes::NamedFunction.
new('locate', [quoted_pattern, column])
insert_in_place = Arel::Nodes::NamedFunction.
new('insert', [column, locate, pattern.size, quoted_replacement])
Arel::Nodes::SqlLiteral.new(insert_in_place.to_sql)
else
replace = Arel::Nodes::NamedFunction.
new("regexp_replace", [column, quoted_pattern, quoted_replacement])
Arel::Nodes::SqlLiteral.new(replace.to_sql)
end
end
end end
end end
end end
...@@ -156,32 +156,22 @@ describe AutocompleteController do ...@@ -156,32 +156,22 @@ describe AutocompleteController do
end end
context 'author of issuable included' do context 'author of issuable included' do
let(:body) { JSON.parse(response.body) } before do
sign_in(user)
context 'authenticated' do end
before do
sign_in(user)
end
it 'includes the author' do
get(:users, author_id: non_member.id)
expect(body.first["username"]).to eq non_member.username let(:body) { JSON.parse(response.body) }
end
it 'rejects non existent user ids' do it 'includes the author' do
get(:users, author_id: 99999) get(:users, author_id: non_member.id)
expect(body.collect { |u| u['id'] }).not_to include(99999) expect(body.first["username"]).to eq non_member.username
end
end end
context 'without authenticating' do it 'rejects non existent user ids' do
it 'returns empty result' do get(:users, author_id: 99999)
get(:users, author_id: non_member.id)
expect(body).to be_empty expect(body.collect { |u| u['id'] }).not_to include(99999)
end
end end
end end
......
FactoryGirl.define do
factory :upload do
model { build(:project) }
path { "uploads/system/project/avatar/avatar.jpg" }
size 100.kilobytes
uploader "AvatarUploader"
end
end
...@@ -63,11 +63,11 @@ feature 'Admin Appearance', feature: true do ...@@ -63,11 +63,11 @@ feature 'Admin Appearance', feature: true do
end end
def logo_selector def logo_selector
'//img[@src^="/uploads/system/appearance/logo"]' '//img[@src^="/uploads/appearance/logo"]'
end end
def header_logo_selector def header_logo_selector
'//img[@src^="/uploads/system/appearance/header_logo"]' '//img[@src^="/uploads/appearance/header_logo"]'
end end
def logo_fixture def logo_fixture
......
...@@ -18,7 +18,7 @@ feature 'User uploads avatar to group', feature: true do ...@@ -18,7 +18,7 @@ feature 'User uploads avatar to group', feature: true do
visit group_path(group) visit group_path(group)
expect(page).to have_selector(%Q(img[src$="/uploads/system/group/avatar/#{group.id}/dk.png"])) expect(page).to have_selector(%Q(img[src$="/uploads/group/avatar/#{group.id}/dk.png"]))
# Cheating here to verify something that isn't user-facing, but is important # Cheating here to verify something that isn't user-facing, but is important
expect(group.reload.avatar.file).to exist expect(group.reload.avatar.file).to exist
......
...@@ -16,7 +16,7 @@ feature 'User uploads avatar to profile', feature: true do ...@@ -16,7 +16,7 @@ feature 'User uploads avatar to profile', feature: true do
visit user_path(user) visit user_path(user)
expect(page).to have_selector(%Q(img[src$="/uploads/system/user/avatar/#{user.id}/dk.png"])) expect(page).to have_selector(%Q(img[src$="/uploads/user/avatar/#{user.id}/dk.png"]))
# Cheating here to verify something that isn't user-facing, but is important # Cheating here to verify something that isn't user-facing, but is important
expect(user.reload.avatar.file).to exist expect(user.reload.avatar.file).to exist
......
# coding: utf-8
require 'spec_helper' require 'spec_helper'
describe ApplicationHelper do describe ApplicationHelper do
...@@ -58,7 +57,7 @@ describe ApplicationHelper do ...@@ -58,7 +57,7 @@ describe ApplicationHelper do
it 'returns an url for the avatar' do it 'returns an url for the avatar' do
project = create(:empty_project, avatar: File.open(uploaded_image_temp_path)) project = create(:empty_project, avatar: File.open(uploaded_image_temp_path))
avatar_url = "http://#{Gitlab.config.gitlab.host}/uploads/system/project/avatar/#{project.id}/banana_sample.gif" avatar_url = "http://#{Gitlab.config.gitlab.host}/uploads/project/avatar/#{project.id}/banana_sample.gif"
expect(helper.project_icon(project.full_path).to_s). expect(helper.project_icon(project.full_path).to_s).
to eq "<img src=\"#{avatar_url}\" alt=\"Banana sample\" />" to eq "<img src=\"#{avatar_url}\" alt=\"Banana sample\" />"
end end
...@@ -79,7 +78,7 @@ describe ApplicationHelper do ...@@ -79,7 +78,7 @@ describe ApplicationHelper do
user = create(:user, avatar: File.open(uploaded_image_temp_path)) user = create(:user, avatar: File.open(uploaded_image_temp_path))
expect(helper.avatar_icon(user.email).to_s). expect(helper.avatar_icon(user.email).to_s).
to match("/uploads/system/user/avatar/#{user.id}/banana_sample.gif") to match("/uploads/user/avatar/#{user.id}/banana_sample.gif")
end end
it 'returns an url for the avatar with relative url' do it 'returns an url for the avatar with relative url' do
...@@ -90,7 +89,7 @@ describe ApplicationHelper do ...@@ -90,7 +89,7 @@ describe ApplicationHelper do
user = create(:user, avatar: File.open(uploaded_image_temp_path)) user = create(:user, avatar: File.open(uploaded_image_temp_path))
expect(helper.avatar_icon(user.email).to_s). expect(helper.avatar_icon(user.email).to_s).
to match("/gitlab/uploads/system/user/avatar/#{user.id}/banana_sample.gif") to match("/gitlab/uploads/user/avatar/#{user.id}/banana_sample.gif")
end end
it 'calls gravatar_icon when no User exists with the given email' do it 'calls gravatar_icon when no User exists with the given email' do
...@@ -104,7 +103,7 @@ describe ApplicationHelper do ...@@ -104,7 +103,7 @@ describe ApplicationHelper do
user = create(:user, avatar: File.open(uploaded_image_temp_path)) user = create(:user, avatar: File.open(uploaded_image_temp_path))
expect(helper.avatar_icon(user).to_s). expect(helper.avatar_icon(user).to_s).
to match("/uploads/system/user/avatar/#{user.id}/banana_sample.gif") to match("/uploads/user/avatar/#{user.id}/banana_sample.gif")
end end
end end
end end
......
...@@ -52,7 +52,7 @@ describe EmailsHelper do ...@@ -52,7 +52,7 @@ describe EmailsHelper do
) )
expect(header_logo).to eq( expect(header_logo).to eq(
%{<img style="height: 50px" src="/uploads/system/appearance/header_logo/#{appearance.id}/dk.png" alt="Dk" />} %{<img style="height: 50px" src="/uploads/appearance/header_logo/#{appearance.id}/dk.png" alt="Dk" />}
) )
end end
end end
......
...@@ -9,7 +9,7 @@ describe GroupsHelper do ...@@ -9,7 +9,7 @@ describe GroupsHelper do
group.avatar = fixture_file_upload(avatar_file_path) group.avatar = fixture_file_upload(avatar_file_path)
group.save! group.save!
expect(group_icon(group.path).to_s). expect(group_icon(group.path).to_s).
to match("/uploads/system/group/avatar/#{group.id}/banana_sample.gif") to match("/uploads/group/avatar/#{group.id}/banana_sample.gif")
end end
it 'gives default avatar_icon when no avatar is present' do it 'gives default avatar_icon when no avatar is present' do
......
...@@ -60,7 +60,7 @@ describe PageLayoutHelper do ...@@ -60,7 +60,7 @@ describe PageLayoutHelper do
%w(project user group).each do |type| %w(project user group).each do |type|
context "with @#{type} assigned" do context "with @#{type} assigned" do
it "uses #{type.titlecase} avatar if available" do it "uses #{type.titlecase} avatar if available" do
object = double(avatar_url: 'http://example.com/uploads/system/avatar.png') object = double(avatar_url: 'http://example.com/uploads/avatar.png')
assign(type, object) assign(type, object)
expect(helper.page_image).to eq object.avatar_url expect(helper.page_image).to eq object.avatar_url
......
...@@ -44,7 +44,7 @@ describe('Commit component', () => { ...@@ -44,7 +44,7 @@ describe('Commit component', () => {
shortSha: 'b7836edd', shortSha: 'b7836edd',
title: 'Commit message', title: 'Commit message',
author: { author: {
avatar_url: 'https://gitlab.com/uploads/system/user/avatar/300478/avatar.png', avatar_url: 'https://gitlab.com/uploads/user/avatar/300478/avatar.png',
web_url: 'https://gitlab.com/jschatz1', web_url: 'https://gitlab.com/jschatz1',
username: 'jschatz1', username: 'jschatz1',
}, },
......
...@@ -682,37 +682,4 @@ describe Gitlab::Database::MigrationHelpers, lib: true do ...@@ -682,37 +682,4 @@ describe Gitlab::Database::MigrationHelpers, lib: true do
expect(model.column_for(:users, :kittens)).to be_nil expect(model.column_for(:users, :kittens)).to be_nil
end end
end end
describe '#replace_sql' do
context 'using postgres' do
before do
allow(Gitlab::Database).to receive(:mysql?).and_return(false)
end
it 'builds the sql with correct functions' do
expect(model.replace_sql(Arel::Table.new(:users)[:first_name], "Alice", "Eve").to_s).
to include('regexp_replace')
end
end
context 'using mysql' do
before do
allow(Gitlab::Database).to receive(:mysql?).and_return(true)
end
it 'builds the sql with the correct functions' do
expect(model.replace_sql(Arel::Table.new(:users)[:first_name], "Alice", "Eve").to_s).
to include('locate', 'insert')
end
end
describe 'results' do
let!(:user) { create(:user, name: 'Kathy Alice Aliceson') }
it 'replaces the correct part of the string' do
model.update_column_in_batches(:users, :name, model.replace_sql(Arel::Table.new(:users)[:name], 'Alice', 'Eve'))
expect(user.reload.name).to eq('Kathy Eve Aliceson')
end
end
end
end end
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
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
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
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 = create(:project, path: "project-path", namespace: system_namespace)
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
create(:project, namespace: system_namespace, path: "system-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) { create(:project, namespace: system_namespace) }
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 = create(:group, path: "subgroup", parent: system_namespace)
project = create(:project, path: "system0", namespace: subgroup)
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
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
...@@ -36,12 +36,6 @@ describe Namespace, models: true do ...@@ -36,12 +36,6 @@ describe Namespace, models: true do
it { expect(group).not_to be_valid } it { expect(group).not_to be_valid }
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 context 'top-level group' do
let(:group) { build(:group, path: 'tree') } let(:group) { build(:group, path: 'tree') }
...@@ -176,8 +170,8 @@ describe Namespace, models: true do ...@@ -176,8 +170,8 @@ describe Namespace, models: true do
let(:parent) { create(:group, name: 'parent', path: 'parent') } let(:parent) { create(:group, name: 'parent', path: 'parent') }
let(:child) { create(:group, name: 'child', path: 'child', parent: parent) } let(:child) { create(:group, name: 'child', path: 'child', parent: parent) }
let!(:project) { create(:project_empty_repo, path: 'the-project', namespace: child) } let!(:project) { create(:project_empty_repo, path: 'the-project', namespace: child) }
let(:uploads_dir) { File.join(CarrierWave.root, 'uploads', 'system') } let(:uploads_dir) { File.join(CarrierWave.root, 'uploads') }
let(:pages_dir) { File.join(TestEnv.pages_path) } let(:pages_dir) { TestEnv.pages_path }
before do before do
FileUtils.mkdir_p(File.join(uploads_dir, 'parent', 'child', 'the-project')) FileUtils.mkdir_p(File.join(uploads_dir, 'parent', 'child', 'the-project'))
...@@ -211,6 +205,20 @@ describe Namespace, models: true do ...@@ -211,6 +205,20 @@ describe Namespace, models: true do
expect(File.directory?(expected_pages_path)).to be(true) expect(File.directory?(expected_pages_path)).to be(true)
end end
end end
context 'renaming parent' do
it 'correctly moves the repository, uploads and pages' do
expected_repository_path = File.join(TestEnv.repos_path, 'renamed', 'child', 'the-project.git')
expected_upload_path = File.join(uploads_dir, 'renamed', 'child', 'the-project')
expected_pages_path = File.join(pages_dir, 'renamed', 'child', 'the-project')
parent.update_attributes!(path: 'renamed')
expect(File.directory?(expected_repository_path)).to be(true)
expect(File.directory?(expected_upload_path)).to be(true)
expect(File.directory?(expected_pages_path)).to be(true)
end
end
end end
end end
......
...@@ -787,7 +787,7 @@ describe Project, models: true do ...@@ -787,7 +787,7 @@ describe Project, models: true do
end end
let(:avatar_path) do let(:avatar_path) do
"/uploads/system/project/avatar/#{project.id}/uploads/avatar.png" "/uploads/project/avatar/#{project.id}/uploads/avatar.png"
end end
it { should eq "http://#{Gitlab.config.gitlab.host}#{avatar_path}" } it { should eq "http://#{Gitlab.config.gitlab.host}#{avatar_path}" }
......
...@@ -81,7 +81,7 @@ describe 'OpenID Connect requests' do ...@@ -81,7 +81,7 @@ describe 'OpenID Connect requests' do
'email_verified' => true, 'email_verified' => true,
'website' => 'https://example.com', 'website' => 'https://example.com',
'profile' => 'http://localhost/alice', 'profile' => 'http://localhost/alice',
'picture' => "http://localhost/uploads/system/user/avatar/#{user.id}/dk.png", 'picture' => "http://localhost/uploads/user/avatar/#{user.id}/dk.png",
}) })
end end
end end
......
...@@ -14,7 +14,7 @@ describe Projects::ParticipantsService, services: true do ...@@ -14,7 +14,7 @@ describe Projects::ParticipantsService, services: true do
groups = participants.groups groups = participants.groups
expect(groups.size).to eq 1 expect(groups.size).to eq 1
expect(groups.first[:avatar_url]).to eq "#{base_url}/uploads/system/group/avatar/#{group.id}/dk.png" expect(groups.first[:avatar_url]).to eq "#{base_url}/uploads/group/avatar/#{group.id}/dk.png"
end end
it 'should return an url for the avatar with relative url' do it 'should return an url for the avatar with relative url' do
...@@ -25,7 +25,7 @@ describe Projects::ParticipantsService, services: true do ...@@ -25,7 +25,7 @@ describe Projects::ParticipantsService, services: true do
groups = participants.groups groups = participants.groups
expect(groups.size).to eq 1 expect(groups.size).to eq 1
expect(groups.first[:avatar_url]).to eq "#{base_url}/gitlab/uploads/system/group/avatar/#{group.id}/dk.png" expect(groups.first[:avatar_url]).to eq "#{base_url}/gitlab/uploads/group/avatar/#{group.id}/dk.png"
end end
end end
end end
......
...@@ -3,17 +3,6 @@ require 'spec_helper' ...@@ -3,17 +3,6 @@ require 'spec_helper'
describe AttachmentUploader do describe AttachmentUploader do
let(:uploader) { described_class.new(build_stubbed(:user)) } 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 describe '#move_to_cache' do
it 'is true' do it 'is true' do
expect(uploader.move_to_cache).to eq(true) expect(uploader.move_to_cache).to eq(true)
......
...@@ -3,17 +3,6 @@ require 'spec_helper' ...@@ -3,17 +3,6 @@ require 'spec_helper'
describe AvatarUploader do describe AvatarUploader do
let(:uploader) { described_class.new(build_stubbed(:user)) } 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 describe '#move_to_cache' do
it 'is false' do it 'is false' do
expect(uploader.move_to_cache).to eq(false) expect(uploader.move_to_cache).to eq(false)
......
...@@ -15,16 +15,6 @@ describe FileUploader do ...@@ -15,16 +15,6 @@ describe FileUploader do
end end
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 describe 'initialize' do
it 'generates a secret if none is provided' do it 'generates a secret if none is provided' do
expect(SecureRandom).to receive(:hex).and_return('secret') expect(SecureRandom).to receive(:hex).and_return('secret')
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment