Skip to content
Snippets Groups Projects
Commit deba648e authored by Timothy Andrew's avatar Timothy Andrew
Browse files

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

This reverts commit f6ba1e08, reversing
changes made to 810cc51b.
parent f6ba1e08
No related branches found
No related tags found
No related merge requests found
Hide whitespace changes
Inline Side-by-side
Showing
with 25 additions and 488 deletions
app/controllers/autocomplete_controller.rb
+ 1
− 1
View file @ deba648e
@@ -22,7 +22,7 @@ class AutocompleteController < ApplicationController
@users = [current_user, *@users]
end
 
if params[:author_id].present? && current_user
if params[:author_id].present?
author = User.find_by_id(params[:author_id])
@users = [author, *@users].uniq if author
end
app/uploaders/file_uploader.rb
+ 0
− 7
View file @ deba648e
@@ -13,13 +13,6 @@ class FileUploader < GitlabUploader
)
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
# path
#
app/uploaders/gitlab_uploader.rb
+ 4
− 16
View file @ deba648e
@@ -3,28 +3,16 @@ class GitlabUploader < CarrierWave::Uploader::Base
File.join(CarrierWave.root, upload_record.path)
end
 
def self.root_dir
def self.base_dir
'uploads'
end
 
# When object storage is used, keep the `root_dir` as `base_dir`.
# 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
delegate :base_dir, to: :class
 
def self.file_storage?
self.storage == CarrierWave::Storage::File
def file_storage?
self.class.storage == CarrierWave::Storage::File
end
 
delegate :base_dir, :file_storage?, to: :class
# Reduce disk IO
def move_to_cache
true
app/validators/namespace_validator.rb
+ 2
− 3
View file @ deba648e
@@ -33,7 +33,6 @@ class NamespaceValidator < ActiveModel::EachValidator
u
unsubscribes
users
system
].freeze
 
WILDCARD_ROUTES = %w[tree commits wikis new edit create update logs_tree
@@ -48,9 +47,9 @@ class NamespaceValidator < ActiveModel::EachValidator
 
def self.reserved?(value, strict: false)
if strict
STRICT_RESERVED.include?(value.to_s.downcase)
STRICT_RESERVED.include?(value)
else
RESERVED.include?(value.to_s.downcase)
RESERVED.include?(value)
end
end
 
app/validators/project_path_validator.rb
+ 1
− 1
View file @ deba648e
@@ -15,7 +15,7 @@ class ProjectPathValidator < ActiveModel::EachValidator
# 'tree' as project name and 'deploy_keys' as route.
#
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)
!reserved?(value)
changelogs/unreleased/28917-contain-uploads-in-system-dir.yml deleted 100644 → 0
+ 0
− 4
View file @ f6ba1e08
---
title: Move uploads from 'public/uploads' to 'public/uploads/system'
merge_request:
author:
changelogs/unreleased/dz-api-x-frame.yml deleted 100644 → 0
+ 0
− 4
View file @ f6ba1e08
---
title: Restrict API X-Frame-Options to same origin
merge_request:
author:
changelogs/unreleased/dz-restrict-autocomplete.yml deleted 100644 → 0
+ 0
− 4
View file @ f6ba1e08
---
title: Allow users autocomplete by author_id only for authenticated users
merge_request:
author:
config/routes/uploads.rb
+ 2
− 2
View file @ deba648e
scope path: :uploads do
# Note attachments and User/Group/Project avatars
get "system/:model/:mounted_as/:id/:filename",
get ":model/:mounted_as/:id/:filename",
to: "uploads#show",
constraints: { model: /note|user|group|project/, mounted_as: /avatar|attachment/, filename: /[^\/]+/ }
 
# Appearance
get "system/:model/:mounted_as/:id/:filename",
get ":model/:mounted_as/:id/:filename",
to: "uploads#show",
constraints: { model: /appearance/, mounted_as: /logo|header_logo/, filename: /.+/ }
 
db/migrate/20170316163800_rename_system_namespaces.rb deleted 100644 → 0
+ 0
− 231
View file @ f6ba1e08
# 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
db/migrate/20170316163845_move_uploads_to_system_dir.rb deleted 100644 → 0
+ 0
− 59
View file @ f6ba1e08
# 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
db/post_migrate/20170317162059_update_upload_paths_to_system.rb deleted 100644 → 0
+ 0
− 55
View file @ f6ba1e08
# 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
db/post_migrate/20170406111121_clean_upload_symlinks.rb deleted 100644 → 0
+ 0
− 52
View file @ f6ba1e08
# 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
features/steps/groups.rb
+ 1
− 1
View file @ deba648e
@@ -81,7 +81,7 @@ class Spinach::Features::Groups < Spinach::FeatureSteps
 
step 'I should see new group "Owned" avatar' do
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
 
step 'I should see the "Remove avatar" button' do
features/steps/profile/profile.rb
+ 1
− 1
View file @ deba648e
@@ -36,7 +36,7 @@ class Spinach::Features::Profile < Spinach::FeatureSteps
 
step 'I should see new avatar' do
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
 
step 'I should see the "Remove avatar" button' do
features/steps/project/project.rb
+ 1
− 1
View file @ deba648e
@@ -37,7 +37,7 @@ class Spinach::Features::Project < Spinach::FeatureSteps
step 'I should see new project avatar' do
expect(@project.avatar).to be_instance_of AvatarUploader
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
 
step 'I should see the "Remove avatar" button' do
lib/api/api.rb
+ 0
− 1
View file @ deba648e
@@ -44,7 +44,6 @@ module API
end
 
before { allow_access_with_scope :api }
before { header['X-Frame-Options'] = 'SAMEORIGIN' }
 
rescue_from Gitlab::Access::AccessDeniedError do
rack_response({ 'message' => '403 Forbidden' }.to_json, 403)
lib/gitlab/database/migration_helpers.rb
+ 1
− 24
View file @ deba648e
@@ -80,7 +80,7 @@ module Gitlab
# here is based on Rails' foreign_key_name() method, which unfortunately
# is private so we can't rely on it directly.
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
 
# Long-running migrations may take more than the timeout allowed by
@@ -226,29 +226,6 @@ module Gitlab
raise error
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
lib/tasks/gitlab/db.rake
+ 1
− 1
View file @ deba648e
@@ -62,7 +62,7 @@ namespace :gitlab do
 
ref = Shellwords.escape(args[:ref])
 
migrations = `git diff #{ref}.. --diff-filter=A --name-only -- db/migrate`.lines
migrations = `git diff #{ref}.. --name-only -- db/migrate`.lines
.map { |file| Rails.root.join(file.strip).to_s }
.select { |file| File.file?(file) }
 
spec/controllers/autocomplete_controller_spec.rb
+ 10
− 20
View file @ deba648e
@@ -156,32 +156,22 @@ describe AutocompleteController do
end
 
context 'author of issuable included' do
let(:body) { JSON.parse(response.body) }
context 'authenticated' do
before do
sign_in(user)
end
it 'includes the author' do
get(:users, author_id: non_member.id)
before do
sign_in(user)
end
 
expect(body.first["username"]).to eq non_member.username
end
let(:body) { JSON.parse(response.body) }
 
it 'rejects non existent user ids' do
get(:users, author_id: 99999)
it 'includes the author' do
get(:users, author_id: non_member.id)
 
expect(body.collect { |u| u['id'] }).not_to include(99999)
end
expect(body.first["username"]).to eq non_member.username
end
 
context 'without authenticating' do
it 'returns empty result' do
get(:users, author_id: non_member.id)
it 'rejects non existent user ids' do
get(:users, author_id: 99999)
 
expect(body).to be_empty
end
expect(body.collect { |u| u['id'] }).not_to include(99999)
end
end
 
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