Commit 565ead61 authored by DJ Mountney's avatar DJ Mountney Committed by Bob Van Landuyt

Bring in security changes from the 9.2.5 release

Ran:
 - git format-patch v9.2.2..v9.2.5 --stdout > patchfile.patch
 - git checkout -b 9-2-5-security-patch origin/v9.2.2
 - git apply patchfile.patch
 - git commit
 - [Got the sha ref for the commit]
 - git checkout -b upstream-9-2-security master
 - git cherry-pick <SHA of the patchfile commit>
 - [Resolved conflicts]
 - git cherry-pick --continue
parent ba564a09
...@@ -1478,7 +1478,7 @@ const normalizeNewlines = function(str) { ...@@ -1478,7 +1478,7 @@ const normalizeNewlines = function(str) {
const cachedNoteBodyText = $noteBodyText.html(); const cachedNoteBodyText = $noteBodyText.html();
// Show updated comment content temporarily // Show updated comment content temporarily
$noteBodyText.html(formContent); $noteBodyText.html(_.escape(formContent));
$editingNote.removeClass('is-editing fade-in-full').addClass('being-posted fade-in-half'); $editingNote.removeClass('is-editing fade-in-full').addClass('being-posted fade-in-half');
$editingNote.find('.note-headline-meta a').html('<i class="fa fa-spinner fa-spin" aria-label="Comment is being updated" aria-hidden="true"></i>'); $editingNote.find('.note-headline-meta a').html('<i class="fa fa-spinner fa-spin" aria-label="Comment is being updated" aria-hidden="true"></i>');
...@@ -1491,7 +1491,7 @@ const normalizeNewlines = function(str) { ...@@ -1491,7 +1491,7 @@ const normalizeNewlines = function(str) {
}) })
.fail(() => { .fail(() => {
// Submission failed, revert back to original note // Submission failed, revert back to original note
$noteBodyText.html(cachedNoteBodyText); $noteBodyText.html(_.escape(cachedNoteBodyText));
$editingNote.removeClass('being-posted fade-in'); $editingNote.removeClass('being-posted fade-in');
$editingNote.find('.fa.fa-spinner').remove(); $editingNote.find('.fa.fa-spinner').remove();
......
...@@ -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? if params[:author_id].present? && current_user
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
......
class ProjectSnippetPolicy < BasePolicy class ProjectSnippetPolicy < BasePolicy
def rules def rules
# We have to check both project feature visibility and a snippet visibility and take the stricter one
# This will be simplified - check https://gitlab.com/gitlab-org/gitlab-ce/issues/27573
return unless @subject.project.feature_available?(:snippets, @user)
return unless Ability.allowed?(@user, :read_project, @subject.project)
can! :read_project_snippet if @subject.public? can! :read_project_snippet if @subject.public?
return unless @user return unless @user
......
...@@ -13,6 +13,13 @@ class FileUploader < GitlabUploader ...@@ -13,6 +13,13 @@ 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,16 +3,26 @@ class GitlabUploader < CarrierWave::Uploader::Base ...@@ -3,16 +3,26 @@ class GitlabUploader < CarrierWave::Uploader::Base
File.join(CarrierWave.root, upload_record.path) File.join(CarrierWave.root, upload_record.path)
end end
def self.base_dir def self.root_dir
'uploads' 'uploads'
end end
delegate :base_dir, to: :class # 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?
end
def file_storage? def self.file_storage?
storage.is_a?(CarrierWave::Storage::File) self.storage.is_a?(CarrierWave::Storage::File)
end end
delegate :base_dir, :file_storage?, to: :class
def file_cache_storage? def file_cache_storage?
cache_storage.is_a?(CarrierWave::Storage::File) cache_storage.is_a?(CarrierWave::Storage::File)
end end
......
scope path: :uploads do scope path: :uploads do
# Note attachments and User/Group/Project avatars # Note attachments and User/Group/Project avatars
get ":model/:mounted_as/:id/:filename", get "system/: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: /[^\/]+/ }
...@@ -15,7 +15,7 @@ scope path: :uploads do ...@@ -15,7 +15,7 @@ scope path: :uploads do
constraints: { filename: /[^\/]+/ } constraints: { filename: /[^\/]+/ }
# Appearance # Appearance
get ":model/:mounted_as/:id/:filename", get "system/: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 appearance).freeze
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
class MoveAppearanceToSystemDir < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
disable_ddl_transaction!
DOWNTIME = false
DIRECTORY_TO_MOVE = 'appearance'.freeze
def up
source = File.join(old_upload_dir, DIRECTORY_TO_MOVE)
destination = File.join(new_upload_dir, DIRECTORY_TO_MOVE)
move_directory(source, destination)
end
def down
source = File.join(new_upload_dir, DIRECTORY_TO_MOVE)
destination = File.join(old_upload_dir, DIRECTORY_TO_MOVE)
move_directory(source, destination)
end
def move_directory(source, destination)
unless file_storage?
say 'Not using file storage, skipping'
return
end
unless File.directory?(source)
say "#{source} did not exist, skipping"
return
end
if File.directory?(destination)
say "#{destination} already existed, skipping"
return
end
say "Moving #{source} -> #{destination}"
FileUtils.mv(source, destination)
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
...@@ -11,8 +11,7 @@ ...@@ -11,8 +11,7 @@
# #
# It's strongly recommended that you check this file into your version control system. # It's strongly recommended that you check this file into your version control system.
ActiveRecord::Schema.define(version: 20170603200744) do ActiveRecord::Schema.define(version: 20170606202615) do
# These are extensions that must be enabled in order to support this database # These are extensions that must be enabled in order to support this database
enable_extension "plpgsql" enable_extension "plpgsql"
enable_extension "pg_trgm" enable_extension "pg_trgm"
......
...@@ -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/group/avatar/#{Group.find_by(name: "Owned").id}/banana_sample.gif" expect(owned_group.avatar.url).to eq "/uploads/system/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/user/avatar/#{@user.id}/banana_sample.gif" expect(@user.avatar.url).to eq "/uploads/system/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
......
...@@ -38,7 +38,7 @@ class Spinach::Features::Project < Spinach::FeatureSteps ...@@ -38,7 +38,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/project/avatar/#{@project.id}/banana_sample.gif" expect(url).to eq "/uploads/system/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
......
...@@ -45,6 +45,7 @@ module API ...@@ -45,6 +45,7 @@ module API
end end
before { allow_access_with_scope :api } before { allow_access_with_scope :api }
before { header['X-Frame-Options'] = 'SAMEORIGIN' }
before { Gitlab::I18n.locale = current_user&.preferred_language } before { Gitlab::I18n.locale = current_user&.preferred_language }
after { Gitlab::I18n.use_default_locale } after { Gitlab::I18n.use_default_locale }
......
...@@ -62,7 +62,7 @@ module Banzai ...@@ -62,7 +62,7 @@ module Banzai
nodes.select do |node| nodes.select do |node|
if node.has_attribute?(project_attr) if node.has_attribute?(project_attr)
can_read_reference?(user, projects[node]) can_read_reference?(user, projects[node], node)
else else
true true
end end
...@@ -231,7 +231,7 @@ module Banzai ...@@ -231,7 +231,7 @@ module Banzai
# see reference comments. # see reference comments.
# Override this method on subclasses # Override this method on subclasses
# to check if user can read resource # to check if user can read resource
def can_read_reference?(user, ref_project) def can_read_reference?(user, ref_project, node)
raise NotImplementedError raise NotImplementedError
end end
......
...@@ -32,7 +32,7 @@ module Banzai ...@@ -32,7 +32,7 @@ module Banzai
private private
def can_read_reference?(user, ref_project) def can_read_reference?(user, ref_project, node)
can?(user, :download_code, ref_project) can?(user, :download_code, ref_project)
end end
end end
......
...@@ -36,7 +36,7 @@ module Banzai ...@@ -36,7 +36,7 @@ module Banzai
private private
def can_read_reference?(user, ref_project) def can_read_reference?(user, ref_project, node)
can?(user, :download_code, ref_project) can?(user, :download_code, ref_project)
end end
end end
......
...@@ -23,7 +23,7 @@ module Banzai ...@@ -23,7 +23,7 @@ module Banzai
private private
def can_read_reference?(user, ref_project) def can_read_reference?(user, ref_project, node)
can?(user, :read_issue, ref_project) can?(user, :read_issue, ref_project)
end end
end end
......
...@@ -9,7 +9,7 @@ module Banzai ...@@ -9,7 +9,7 @@ module Banzai
private private
def can_read_reference?(user, ref_project) def can_read_reference?(user, ref_project, node)
can?(user, :read_label, ref_project) can?(user, :read_label, ref_project)
end end
end end
......
...@@ -40,6 +40,10 @@ module Banzai ...@@ -40,6 +40,10 @@ module Banzai
self.class.data_attribute self.class.data_attribute
) )
end end
def can_read_reference?(user, ref_project, node)
can?(user, :read_merge_request, ref_project)
end
end end
end end
end end
...@@ -9,7 +9,7 @@ module Banzai ...@@ -9,7 +9,7 @@ module Banzai
private private
def can_read_reference?(user, ref_project) def can_read_reference?(user, ref_project, node)
can?(user, :read_milestone, ref_project) can?(user, :read_milestone, ref_project)
end end
end end
......
...@@ -9,8 +9,8 @@ module Banzai ...@@ -9,8 +9,8 @@ module Banzai
private private
def can_read_reference?(user, ref_project) def can_read_reference?(user, ref_project, node)
can?(user, :read_project_snippet, ref_project) can?(user, :read_project_snippet, referenced_by([node]).first)
end end
end end
end end
......
...@@ -103,7 +103,7 @@ module Banzai ...@@ -103,7 +103,7 @@ module Banzai
flat_map { |p| p.team.members.to_a } flat_map { |p| p.team.members.to_a }
end end
def can_read_reference?(user, ref_project) def can_read_reference?(user, ref_project, node)
can?(user, :read_project, ref_project) can?(user, :read_project, ref_project)
end end
end end
......
...@@ -49,6 +49,7 @@ module Gitlab ...@@ -49,6 +49,7 @@ module Gitlab
sent_notifications sent_notifications
services services
snippets snippets
system
teams teams
u u
unicorn_test unicorn_test
......
module Gitlab module Gitlab
class UploadsTransfer < ProjectTransfer class UploadsTransfer < ProjectTransfer
def root_dir def root_dir
File.join(CarrierWave.root, GitlabUploader.base_dir) File.join(CarrierWave.root, FileUploader.base_dir)
end end
end end
end end
...@@ -170,12 +170,13 @@ describe AutocompleteController do ...@@ -170,12 +170,13 @@ describe AutocompleteController do
end end
context 'author of issuable included' do context 'author of issuable included' do
let(:body) { JSON.parse(response.body) }
context 'authenticated' do
before do before do
sign_in(user) sign_in(user)
end end
let(:body) { JSON.parse(response.body) }
it 'includes the author' do it 'includes the author' do
get(:users, author_id: non_member.id) get(:users, author_id: non_member.id)
...@@ -189,6 +190,15 @@ describe AutocompleteController do ...@@ -189,6 +190,15 @@ describe AutocompleteController do
end end
end end
context 'without authenticating' do
it 'returns empty result' do
get(:users, author_id: non_member.id)
expect(body).to be_empty
end
end
end
context 'skip_users parameter included' do context 'skip_users parameter included' do
before { sign_in(user) } before { sign_in(user) }
......
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/appearance/logo"]' '//img[@src^="/uploads/system/appearance/logo"]'
end end
def header_logo_selector def header_logo_selector
'//img[@src^="/uploads/appearance/header_logo"]' '//img[@src^="/uploads/system/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/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 # 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/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 # 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,13 +59,13 @@ describe ApplicationHelper do ...@@ -58,13 +59,13 @@ describe ApplicationHelper do
describe 'project_icon' do describe 'project_icon' 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 = "/uploads/project/avatar/#{project.id}/banana_sample.gif" avatar_url = "/uploads/system/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\" />"
allow(ActionController::Base).to receive(:asset_host).and_return(gitlab_host) allow(ActionController::Base).to receive(:asset_host).and_return(gitlab_host)
avatar_url = "#{gitlab_host}/uploads/project/avatar/#{project.id}/banana_sample.gif" avatar_url = "#{gitlab_host}/uploads/system/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\" />"
...@@ -84,12 +85,12 @@ describe ApplicationHelper do ...@@ -84,12 +85,12 @@ describe ApplicationHelper do
it 'returns an url for the avatar' do it 'returns an url for the avatar' do
user = create(:user, avatar: File.open(uploaded_image_temp_path)) user = create(:user, avatar: File.open(uploaded_image_temp_path))
avatar_url = "/uploads/user/avatar/#{user.id}/banana_sample.gif" avatar_url = "/uploads/system/user/avatar/#{user.id}/banana_sample.gif"
expect(helper.avatar_icon(user.email).to_s).to match(avatar_url) expect(helper.avatar_icon(user.email).to_s).to match(avatar_url)
allow(ActionController::Base).to receive(:asset_host).and_return(gitlab_host) allow(ActionController::Base).to receive(:asset_host).and_return(gitlab_host)
avatar_url = "#{gitlab_host}/uploads/user/avatar/#{user.id}/banana_sample.gif" avatar_url = "#{gitlab_host}/uploads/system/user/avatar/#{user.id}/banana_sample.gif"
expect(helper.avatar_icon(user.email).to_s).to match(avatar_url) expect(helper.avatar_icon(user.email).to_s).to match(avatar_url)
end end
...@@ -102,7 +103,7 @@ describe ApplicationHelper do ...@@ -102,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.email).to_s). 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 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
...@@ -116,7 +117,7 @@ describe ApplicationHelper do ...@@ -116,7 +117,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/user/avatar/#{user.id}/banana_sample.gif") to match("/uploads/system/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/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
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/group/avatar/#{group.id}/banana_sample.gif") to match("/uploads/system/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/avatar.png') object = double(avatar_url: 'http://example.com/uploads/system/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
......
...@@ -461,6 +461,45 @@ import '~/notes'; ...@@ -461,6 +461,45 @@ import '~/notes';
}); });
}); });
describe('update comment with script tags', () => {
const sampleComment = '<script></script>';
const updatedComment = '<script></script>';
const note = {
id: 1234,
html: `<li class="note note-row-1234 timeline-entry" id="note_1234">
<div class="note-text">${sampleComment}</div>
</li>`,
note: sampleComment,
valid: true
};
let $form;
let $notesContainer;
beforeEach(() => {
this.notes = new Notes('', []);
window.gon.current_username = 'root';
window.gon.current_user_fullname = 'Administrator';
$form = $('form.js-main-target-form');
$notesContainer = $('ul.main-notes-list');
$form.find('textarea.js-note-text').html(sampleComment);
});
it('should not render a script tag', () => {
const deferred = $.Deferred();
spyOn($, 'ajax').and.returnValue(deferred.promise());
$('.js-comment-button').click();
deferred.resolve(note);
const $noteEl = $notesContainer.find(`#note_${note.id}`);
$noteEl.find('.js-note-edit').click();
$noteEl.find('textarea.js-note-text').html(updatedComment);
$noteEl.find('.js-comment-save-button').click();
const $updatedNoteEl = $notesContainer.find(`#note_${note.id}`).find('.js-task-list-container');
expect($updatedNoteEl.find('.note-text').text().trim()).toEqual('');
});
});
describe('getFormData', () => { describe('getFormData', () => {
let $form; let $form;
let sampleComment; let sampleComment;
......
...@@ -22,7 +22,7 @@ describe('Commit component', () => { ...@@ -22,7 +22,7 @@ describe('Commit component', () => {
shortSha: 'b7836edd', shortSha: 'b7836edd',
title: 'Commit message', title: 'Commit message',
author: { 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', web_url: 'https://gitlab.com/jschatz1',
path: '/jschatz1', path: '/jschatz1',
username: 'jschatz1', username: 'jschatz1',
...@@ -45,7 +45,7 @@ describe('Commit component', () => { ...@@ -45,7 +45,7 @@ describe('Commit component', () => {
shortSha: 'b7836edd', shortSha: 'b7836edd',
title: 'Commit message', title: 'Commit message',
author: { 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', web_url: 'https://gitlab.com/jschatz1',
path: '/jschatz1', path: '/jschatz1',
username: 'jschatz1', username: 'jschatz1',
......
...@@ -30,7 +30,7 @@ describe Banzai::ReferenceParser::BaseParser, lib: true do ...@@ -30,7 +30,7 @@ describe Banzai::ReferenceParser::BaseParser, lib: true do
it 'checks if user can read the resource' do it 'checks if user can read the resource' do
link['data-project'] = project.id.to_s link['data-project'] = project.id.to_s
expect(subject).to receive(:can_read_reference?).with(user, project) expect(subject).to receive(:can_read_reference?).with(user, project, link)
subject.nodes_visible_to_user(user, [link]) subject.nodes_visible_to_user(user, [link])
end end
......
...@@ -4,20 +4,199 @@ describe Banzai::ReferenceParser::SnippetParser, lib: true do ...@@ -4,20 +4,199 @@ describe Banzai::ReferenceParser::SnippetParser, lib: true do
include ReferenceParserHelpers include ReferenceParserHelpers
let(:project) { create(:empty_project, :public) } let(:project) { create(:empty_project, :public) }
let(:user) { create(:user) } let(:user) { create(:user) }
let(:snippet) { create(:snippet, project: project) } let(:external_user) { create(:user, :external) }
let(:project_member) { create(:user) }
subject { described_class.new(project, user) } subject { described_class.new(project, user) }
let(:link) { empty_html_link } let(:link) { empty_html_link }
def visible_references(snippet_visibility, user = nil)
snippet = create(:project_snippet, snippet_visibility, project: project)
link['data-project'] = project.id.to_s
link['data-snippet'] = snippet.id.to_s
subject.nodes_visible_to_user(user, [link])
end
before do
project.add_user(project_member, :developer)
end
describe '#nodes_visible_to_user' do describe '#nodes_visible_to_user' do
context 'when the link has a data-issue attribute' do context 'when a project is public and the snippets feature is enabled for everyone' do
before { link['data-snippet'] = snippet.id.to_s } before do
project.project_feature.update_attribute(:snippets_access_level, ProjectFeature::ENABLED)
end
it 'creates a reference for guest for a public snippet' do
expect(visible_references(:public)).to eq([link])
end
it 'creates a reference for a regular user for a public snippet' do
expect(visible_references(:public, user)).to eq([link])
end
it 'creates a reference for a regular user for an internal snippet' do
expect(visible_references(:internal, user)).to eq([link])
end
it 'does not create a reference for an external user for an internal snippet' do
expect(visible_references(:internal, external_user)).to be_empty
end
it 'creates a reference for a project member for a private snippet' do
expect(visible_references(:private, project_member)).to eq([link])
end
it 'does not create a reference for a regular user for a private snippet' do
expect(visible_references(:private, user)).to be_empty
end
end
context 'when a project is public and the snippets feature is enabled for project team members' do
before do
project.project_feature.update_attribute(:snippets_access_level, ProjectFeature::PRIVATE)
end
it 'creates a reference for a project member for a public snippet' do
expect(visible_references(:public, project_member)).to eq([link])
end
it 'does not create a reference for guest for a public snippet' do
expect(visible_references(:public, nil)).to be_empty
end
it 'does not create a reference for a regular user for a public snippet' do
expect(visible_references(:public, user)).to be_empty
end
it 'creates a reference for a project member for an internal snippet' do
expect(visible_references(:internal, project_member)).to eq([link])
end
it 'does not create a reference for a regular user for an internal snippet' do
expect(visible_references(:internal, user)).to be_empty
end
it 'creates a reference for a project member for a private snippet' do
expect(visible_references(:private, project_member)).to eq([link])
end
it 'does not create a reference for a regular user for a private snippet' do
expect(visible_references(:private, user)).to be_empty
end
end
context 'when a project is internal and the snippets feature is enabled for everyone' do
before do
project.update_attribute(:visibility, Gitlab::VisibilityLevel::INTERNAL)
project.project_feature.update_attribute(:snippets_access_level, ProjectFeature::ENABLED)
end
it 'does not create a reference for guest for a public snippet' do
expect(visible_references(:public)).to be_empty
end
it 'does not create a reference for an external user for a public snippet' do
expect(visible_references(:public, external_user)).to be_empty
end
it 'creates a reference for a regular user for a public snippet' do
expect(visible_references(:public, user)).to eq([link])
end
it 'creates a reference for a regular user for an internal snippet' do
expect(visible_references(:internal, user)).to eq([link])
end
it 'does not create a reference for an external user for an internal snippet' do
expect(visible_references(:internal, external_user)).to be_empty
end
it 'creates a reference for a project member for a private snippet' do
expect(visible_references(:private, project_member)).to eq([link])
end
it 'does not create a reference for a regular user for a private snippet' do
expect(visible_references(:private, user)).to be_empty
end
end
it_behaves_like "referenced feature visibility", "snippets" context 'when a project is internal and the snippets feature is enabled for project team members' do
before do
project.update_attribute(:visibility, Gitlab::VisibilityLevel::INTERNAL)
project.project_feature.update_attribute(:snippets_access_level, ProjectFeature::PRIVATE)
end
it 'creates a reference for a project member for a public snippet' do
expect(visible_references(:public, project_member)).to eq([link])
end
it 'does not create a reference for guest for a public snippet' do
expect(visible_references(:public, nil)).to be_empty
end
it 'does not create reference for a regular user for a public snippet' do
expect(visible_references(:public, user)).to be_empty
end
it 'creates a reference for a project member for an internal snippet' do
expect(visible_references(:internal, project_member)).to eq([link])
end
it 'does not create a reference for a regular user for an internal snippet' do
expect(visible_references(:internal, user)).to be_empty
end
it 'creates a reference for a project member for a private snippet' do
expect(visible_references(:private, project_member)).to eq([link])
end
it 'does not create reference for a regular user for a private snippet' do
expect(visible_references(:private, user)).to be_empty
end
end
context 'when a project is private and the snippets feature is enabled for project team members' do
before do
project.update_attribute(:visibility, Gitlab::VisibilityLevel::PRIVATE)
project.project_feature.update_attribute(:snippets_access_level, ProjectFeature::PRIVATE)
end
it 'creates a reference for a project member for a public snippet' do
expect(visible_references(:public, project_member)).to eq([link])
end
it 'does not create a reference for guest for a public snippet' do
expect(visible_references(:public, nil)).to be_empty
end
it 'does not create a reference for a regular user for a public snippet' do
expect(visible_references(:public, user)).to be_empty
end
it 'creates a reference for a project member for an internal snippet' do
expect(visible_references(:internal, project_member)).to eq([link])
end
it 'does not create a reference for a regular user for an internal snippet' do
expect(visible_references(:internal, user)).to be_empty
end
it 'creates a reference for a project member for a private snippet' do
expect(visible_references(:private, project_member)).to eq([link])
end
it 'does not create a reference for a regular user for a private snippet' do
expect(visible_references(:private, user)).to be_empty
end
end end
end end
describe '#referenced_by' do describe '#referenced_by' do
let(:snippet) { create(:snippet, project: project) }
describe 'when the link has a data-snippet attribute' do describe 'when the link has a data-snippet attribute' do
context 'using an existing snippet ID' do context 'using an existing snippet ID' do
it 'returns an Array of snippets' do it 'returns an Array of snippets' do
...@@ -31,7 +210,7 @@ describe Banzai::ReferenceParser::SnippetParser, lib: true do ...@@ -31,7 +210,7 @@ describe Banzai::ReferenceParser::SnippetParser, lib: true do
it 'returns an empty Array' do it 'returns an empty Array' do
link['data-snippet'] = '' link['data-snippet'] = ''
expect(subject.referenced_by([link])).to eq([]) expect(subject.referenced_by([link])).to be_empty
end end
end end
end end
......
require 'spec_helper'
describe Gitlab::UploadsTransfer do
it 'leaves avatar uploads where they are' do
project_with_avatar = create(:empty_project, :with_avatar)
described_class.new.rename_namespace('project', 'project-renamed')
expect(File.exist?(project_with_avatar.avatar.path)).to be_truthy
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
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
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
...@@ -179,7 +179,7 @@ describe Group, models: true do ...@@ -179,7 +179,7 @@ describe Group, models: true do
let!(:group) { create(:group, :access_requestable, :with_avatar) } let!(:group) { create(:group, :access_requestable, :with_avatar) }
let(:user) { create(:user) } let(:user) { create(:user) }
let(:gitlab_host) { "http://#{Gitlab.config.gitlab.host}" } let(:gitlab_host) { "http://#{Gitlab.config.gitlab.host}" }
let(:avatar_path) { "/uploads/group/avatar/#{group.id}/dk.png" } let(:avatar_path) { "/uploads/system/group/avatar/#{group.id}/dk.png" }
context 'when avatar file is uploaded' do context 'when avatar file is uploaded' do
before { group.add_master(user) } before { group.add_master(user) }
......
...@@ -43,6 +43,12 @@ describe Namespace, models: true do ...@@ -43,6 +43,12 @@ describe Namespace, models: true do
end end
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') }
...@@ -178,8 +184,8 @@ describe Namespace, models: true do ...@@ -178,8 +184,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') } let(:uploads_dir) { File.join(CarrierWave.root, FileUploader.base_dir) }
let(:pages_dir) { TestEnv.pages_path } let(:pages_dir) { File.join(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'))
......
...@@ -812,7 +812,7 @@ describe Project, models: true do ...@@ -812,7 +812,7 @@ describe Project, models: true do
context 'when avatar file is uploaded' do context 'when avatar file is uploaded' do
let(:project) { create(:empty_project, :with_avatar) } 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" }
let(:gitlab_host) { "http://#{Gitlab.config.gitlab.host}" } let(:gitlab_host) { "http://#{Gitlab.config.gitlab.host}" }
it 'shows correct url' do it 'shows correct url' do
......
...@@ -987,7 +987,7 @@ describe User, models: true do ...@@ -987,7 +987,7 @@ describe User, models: true do
context 'when avatar file is uploaded' do context 'when avatar file is uploaded' do
let(:gitlab_host) { "http://#{Gitlab.config.gitlab.host}" } let(:gitlab_host) { "http://#{Gitlab.config.gitlab.host}" }
let(:avatar_path) { "/uploads/user/avatar/#{user.id}/dk.png" } let(:avatar_path) { "/uploads/system/user/avatar/#{user.id}/dk.png" }
it 'shows correct avatar url' do it 'shows correct avatar url' do
expect(user.avatar_url).to eq(avatar_path) expect(user.avatar_url).to eq(avatar_path)
......
...@@ -3,7 +3,7 @@ require 'spec_helper' ...@@ -3,7 +3,7 @@ require 'spec_helper'
describe ProjectSnippetPolicy, models: true do describe ProjectSnippetPolicy, models: true do
let(:regular_user) { create(:user) } let(:regular_user) { create(:user) }
let(:external_user) { create(:user, :external) } let(:external_user) { create(:user, :external) }
let(:project) { create(:empty_project) } let(:project) { create(:empty_project, :public) }
let(:author_permissions) do let(:author_permissions) do
[ [
...@@ -107,7 +107,7 @@ describe ProjectSnippetPolicy, models: true do ...@@ -107,7 +107,7 @@ describe ProjectSnippetPolicy, models: true do
end end
context 'snippet author' do context 'snippet author' do
let(:snippet) { create(:project_snippet, :private, author: regular_user) } let(:snippet) { create(:project_snippet, :private, author: regular_user, project: project) }
subject { described_class.abilities(regular_user, snippet).to_set } subject { described_class.abilities(regular_user, snippet).to_set }
......
...@@ -79,7 +79,7 @@ describe 'OpenID Connect requests' do ...@@ -79,7 +79,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/user/avatar/#{user.id}/dk.png" 'picture' => "http://localhost/uploads/system/user/avatar/#{user.id}/dk.png"
}) })
end end
end end
......
...@@ -13,7 +13,7 @@ describe Projects::ParticipantsService, services: true do ...@@ -13,7 +13,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("/uploads/group/avatar/#{group.id}/dk.png") expect(groups.first[:avatar_url]).to eq("/uploads/system/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
...@@ -24,7 +24,7 @@ describe Projects::ParticipantsService, services: true do ...@@ -24,7 +24,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("/gitlab/uploads/group/avatar/#{group.id}/dk.png") expect(groups.first[:avatar_url]).to eq("/gitlab/uploads/system/group/avatar/#{group.id}/dk.png")
end end
end end
end end
......
...@@ -3,6 +3,17 @@ require 'spec_helper' ...@@ -3,6 +3,17 @@ 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,6 +3,17 @@ require 'spec_helper' ...@@ -3,6 +3,17 @@ 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,6 +15,16 @@ describe FileUploader do ...@@ -15,6 +15,16 @@ 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