diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js index 929965de5c1..b0143b12cfe 100644 --- a/app/assets/javascripts/notes.js +++ b/app/assets/javascripts/notes.js @@ -1478,7 +1478,7 @@ const normalizeNewlines = function(str) { const cachedNoteBodyText = $noteBodyText.html(); // 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.find('.note-headline-meta a').html(''); @@ -1491,7 +1491,7 @@ const normalizeNewlines = function(str) { }) .fail(() => { // Submission failed, revert back to original note - $noteBodyText.html(cachedNoteBodyText); + $noteBodyText.html(_.escape(cachedNoteBodyText)); $editingNote.removeClass('being-posted fade-in'); $editingNote.find('.fa.fa-spinner').remove(); diff --git a/app/controllers/autocomplete_controller.rb b/app/controllers/autocomplete_controller.rb index 907717dcb96..fe331a883c1 100644 --- a/app/controllers/autocomplete_controller.rb +++ b/app/controllers/autocomplete_controller.rb @@ -21,7 +21,7 @@ class AutocompleteController < ApplicationController @users = [current_user, *@users].uniq end - if params[:author_id].present? + if params[:author_id].present? && current_user author = User.find_by_id(params[:author_id]) @users = [author, *@users].uniq if author end diff --git a/app/policies/project_snippet_policy.rb b/app/policies/project_snippet_policy.rb index cf8ff92617f..bc5c4f32f79 100644 --- a/app/policies/project_snippet_policy.rb +++ b/app/policies/project_snippet_policy.rb @@ -1,5 +1,10 @@ class ProjectSnippetPolicy < BasePolicy 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? return unless @user diff --git a/app/uploaders/file_uploader.rb b/app/uploaders/file_uploader.rb index 7e94218c23d..652277e3b78 100644 --- a/app/uploaders/file_uploader.rb +++ b/app/uploaders/file_uploader.rb @@ -13,6 +13,13 @@ 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 # diff --git a/app/uploaders/gitlab_uploader.rb b/app/uploaders/gitlab_uploader.rb index 02afddb8c6a..e4e6d6f46b1 100644 --- a/app/uploaders/gitlab_uploader.rb +++ b/app/uploaders/gitlab_uploader.rb @@ -3,16 +3,28 @@ class GitlabUploader < CarrierWave::Uploader::Base File.join(CarrierWave.root, upload_record.path) end - def self.base_dir + def self.root_dir 'uploads' 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? - def file_storage? - storage.is_a?(CarrierWave::Storage::File) + File.join(root_dir, 'system') end + def self.file_storage? + self.storage == CarrierWave::Storage::File + end + + delegate :base_dir, :file_storage?, to: :class + def file_cache_storage? cache_storage.is_a?(CarrierWave::Storage::File) end diff --git a/config/routes/uploads.rb b/config/routes/uploads.rb index ae8747d766d..a49e244af1a 100644 --- a/config/routes/uploads.rb +++ b/config/routes/uploads.rb @@ -1,6 +1,6 @@ scope path: :uploads do # Note attachments and User/Group/Project avatars - get ":model/:mounted_as/:id/:filename", + get "system/:model/:mounted_as/:id/:filename", to: "uploads#show", constraints: { model: /note|user|group|project/, mounted_as: /avatar|attachment/, filename: /[^\/]+/ } @@ -15,7 +15,7 @@ scope path: :uploads do constraints: { filename: /[^\/]+/ } # Appearance - get ":model/:mounted_as/:id/:filename", + get "system/:model/:mounted_as/:id/:filename", to: "uploads#show", constraints: { model: /appearance/, mounted_as: /logo|header_logo/, filename: /.+/ } diff --git a/db/migrate/20170316163800_rename_system_namespaces.rb b/db/migrate/20170316163800_rename_system_namespaces.rb new file mode 100644 index 00000000000..b5408fbf112 --- /dev/null +++ b/db/migrate/20170316163800_rename_system_namespaces.rb @@ -0,0 +1,231 @@ +# 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 diff --git a/db/migrate/20170316163845_move_uploads_to_system_dir.rb b/db/migrate/20170316163845_move_uploads_to_system_dir.rb new file mode 100644 index 00000000000..564ee10b5ab --- /dev/null +++ b/db/migrate/20170316163845_move_uploads_to_system_dir.rb @@ -0,0 +1,59 @@ +# 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 diff --git a/db/post_migrate/20170317162059_update_upload_paths_to_system.rb b/db/post_migrate/20170317162059_update_upload_paths_to_system.rb new file mode 100644 index 00000000000..9a77b0bbdfb --- /dev/null +++ b/db/post_migrate/20170317162059_update_upload_paths_to_system.rb @@ -0,0 +1,55 @@ +# 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 diff --git a/db/post_migrate/20170406111121_clean_upload_symlinks.rb b/db/post_migrate/20170406111121_clean_upload_symlinks.rb new file mode 100644 index 00000000000..3ac9a6c10bc --- /dev/null +++ b/db/post_migrate/20170406111121_clean_upload_symlinks.rb @@ -0,0 +1,52 @@ +# 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 diff --git a/db/post_migrate/20170606202615_move_appearance_to_system_dir.rb b/db/post_migrate/20170606202615_move_appearance_to_system_dir.rb new file mode 100644 index 00000000000..561de59ec69 --- /dev/null +++ b/db/post_migrate/20170606202615_move_appearance_to_system_dir.rb @@ -0,0 +1,57 @@ +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 diff --git a/db/schema.rb b/db/schema.rb index 83172a92b49..b93630a410d 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,8 +11,7 @@ # # 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 enable_extension "plpgsql" enable_extension "pg_trgm" diff --git a/features/steps/groups.rb b/features/steps/groups.rb index 83d8abbab1f..25bb374b868 100644 --- a/features/steps/groups.rb +++ b/features/steps/groups.rb @@ -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/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 step 'I should see the "Remove avatar" button' do diff --git a/features/steps/profile/profile.rb b/features/steps/profile/profile.rb index 24cfbaad7fe..254c26bb6af 100644 --- a/features/steps/profile/profile.rb +++ b/features/steps/profile/profile.rb @@ -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/user/avatar/#{@user.id}/banana_sample.gif" + expect(@user.avatar.url).to eq "/uploads/system/user/avatar/#{@user.id}/banana_sample.gif" end step 'I should see the "Remove avatar" button' do diff --git a/features/steps/project/project.rb b/features/steps/project/project.rb index de32c9afcca..7d34331db46 100644 --- a/features/steps/project/project.rb +++ b/features/steps/project/project.rb @@ -38,7 +38,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/project/avatar/#{@project.id}/banana_sample.gif" + expect(url).to eq "/uploads/system/project/avatar/#{@project.id}/banana_sample.gif" end step 'I should see the "Remove avatar" button' do diff --git a/lib/api/api.rb b/lib/api/api.rb index 88f91c07194..d767af36e8e 100644 --- a/lib/api/api.rb +++ b/lib/api/api.rb @@ -45,6 +45,7 @@ module API end before { allow_access_with_scope :api } + before { header['X-Frame-Options'] = 'SAMEORIGIN' } before { Gitlab::I18n.locale = current_user&.preferred_language } after { Gitlab::I18n.use_default_locale } diff --git a/lib/banzai/reference_parser/base_parser.rb b/lib/banzai/reference_parser/base_parser.rb index d99a3bfa625..1e2536231d8 100644 --- a/lib/banzai/reference_parser/base_parser.rb +++ b/lib/banzai/reference_parser/base_parser.rb @@ -62,7 +62,7 @@ module Banzai nodes.select do |node| if node.has_attribute?(project_attr) - can_read_reference?(user, projects[node]) + can_read_reference?(user, projects[node], node) else true end @@ -231,7 +231,7 @@ module Banzai # see reference comments. # Override this method on subclasses # to check if user can read resource - def can_read_reference?(user, ref_project) + def can_read_reference?(user, ref_project, node) raise NotImplementedError end diff --git a/lib/banzai/reference_parser/commit_parser.rb b/lib/banzai/reference_parser/commit_parser.rb index 8c54a041cb8..30dc87248b4 100644 --- a/lib/banzai/reference_parser/commit_parser.rb +++ b/lib/banzai/reference_parser/commit_parser.rb @@ -32,7 +32,7 @@ module Banzai private - def can_read_reference?(user, ref_project) + def can_read_reference?(user, ref_project, node) can?(user, :download_code, ref_project) end end diff --git a/lib/banzai/reference_parser/commit_range_parser.rb b/lib/banzai/reference_parser/commit_range_parser.rb index 0878b6afba3..a50e6f8ef8f 100644 --- a/lib/banzai/reference_parser/commit_range_parser.rb +++ b/lib/banzai/reference_parser/commit_range_parser.rb @@ -36,7 +36,7 @@ module Banzai private - def can_read_reference?(user, ref_project) + def can_read_reference?(user, ref_project, node) can?(user, :download_code, ref_project) end end diff --git a/lib/banzai/reference_parser/external_issue_parser.rb b/lib/banzai/reference_parser/external_issue_parser.rb index 6e7b7669578..6307c1b571a 100644 --- a/lib/banzai/reference_parser/external_issue_parser.rb +++ b/lib/banzai/reference_parser/external_issue_parser.rb @@ -23,7 +23,7 @@ module Banzai private - def can_read_reference?(user, ref_project) + def can_read_reference?(user, ref_project, node) can?(user, :read_issue, ref_project) end end diff --git a/lib/banzai/reference_parser/label_parser.rb b/lib/banzai/reference_parser/label_parser.rb index aa76c64ac5f..30e2a012f09 100644 --- a/lib/banzai/reference_parser/label_parser.rb +++ b/lib/banzai/reference_parser/label_parser.rb @@ -9,7 +9,7 @@ module Banzai private - def can_read_reference?(user, ref_project) + def can_read_reference?(user, ref_project, node) can?(user, :read_label, ref_project) end end diff --git a/lib/banzai/reference_parser/merge_request_parser.rb b/lib/banzai/reference_parser/merge_request_parser.rb index 8b0662749fd..75cbc7fdac4 100644 --- a/lib/banzai/reference_parser/merge_request_parser.rb +++ b/lib/banzai/reference_parser/merge_request_parser.rb @@ -40,6 +40,10 @@ module Banzai self.class.data_attribute ) end + + def can_read_reference?(user, ref_project, node) + can?(user, :read_merge_request, ref_project) + end end end end diff --git a/lib/banzai/reference_parser/milestone_parser.rb b/lib/banzai/reference_parser/milestone_parser.rb index d3968d6b229..68675abe22a 100644 --- a/lib/banzai/reference_parser/milestone_parser.rb +++ b/lib/banzai/reference_parser/milestone_parser.rb @@ -9,7 +9,7 @@ module Banzai private - def can_read_reference?(user, ref_project) + def can_read_reference?(user, ref_project, node) can?(user, :read_milestone, ref_project) end end diff --git a/lib/banzai/reference_parser/snippet_parser.rb b/lib/banzai/reference_parser/snippet_parser.rb index 63b592137bb..3ade168b566 100644 --- a/lib/banzai/reference_parser/snippet_parser.rb +++ b/lib/banzai/reference_parser/snippet_parser.rb @@ -9,8 +9,8 @@ module Banzai private - def can_read_reference?(user, ref_project) - can?(user, :read_project_snippet, ref_project) + def can_read_reference?(user, ref_project, node) + can?(user, :read_project_snippet, referenced_by([node]).first) end end end diff --git a/lib/banzai/reference_parser/user_parser.rb b/lib/banzai/reference_parser/user_parser.rb index 09b66cbd8fb..3efbd2fd631 100644 --- a/lib/banzai/reference_parser/user_parser.rb +++ b/lib/banzai/reference_parser/user_parser.rb @@ -103,7 +103,7 @@ module Banzai flat_map { |p| p.team.members.to_a } end - def can_read_reference?(user, ref_project) + def can_read_reference?(user, ref_project, node) can?(user, :read_project, ref_project) end end diff --git a/lib/gitlab/path_regex.rb b/lib/gitlab/path_regex.rb index 9ff6829cd49..10eb99fb461 100644 --- a/lib/gitlab/path_regex.rb +++ b/lib/gitlab/path_regex.rb @@ -49,6 +49,7 @@ module Gitlab sent_notifications services snippets + system teams u unicorn_test diff --git a/lib/gitlab/uploads_transfer.rb b/lib/gitlab/uploads_transfer.rb index 7d0c47c5361..b5f41240529 100644 --- a/lib/gitlab/uploads_transfer.rb +++ b/lib/gitlab/uploads_transfer.rb @@ -1,7 +1,7 @@ module Gitlab class UploadsTransfer < ProjectTransfer def root_dir - File.join(CarrierWave.root, GitlabUploader.base_dir) + File.join(CarrierWave.root, FileUploader.base_dir) end end end diff --git a/spec/controllers/autocomplete_controller_spec.rb b/spec/controllers/autocomplete_controller_spec.rb index 2c9d1ffc9c2..4c3a5ec49ef 100644 --- a/spec/controllers/autocomplete_controller_spec.rb +++ b/spec/controllers/autocomplete_controller_spec.rb @@ -170,22 +170,32 @@ describe AutocompleteController do end context 'author of issuable included' do - before do - sign_in(user) - end - let(:body) { JSON.parse(response.body) } - it 'includes the author' do - get(:users, author_id: non_member.id) + context 'authenticated' do + before do + sign_in(user) + end - expect(body.first["username"]).to eq non_member.username + it 'includes the author' do + get(:users, author_id: non_member.id) + + expect(body.first["username"]).to eq non_member.username + end + + it 'rejects non existent user ids' do + get(:users, author_id: 99999) + + expect(body.collect { |u| u['id'] }).not_to include(99999) + end end - it 'rejects non existent user ids' do - get(:users, author_id: 99999) + context 'without authenticating' do + it 'returns empty result' do + get(:users, author_id: non_member.id) - expect(body.collect { |u| u['id'] }).not_to include(99999) + expect(body).to be_empty + end end end diff --git a/spec/factories/uploads.rb b/spec/factories/uploads.rb new file mode 100644 index 00000000000..1383420fb44 --- /dev/null +++ b/spec/factories/uploads.rb @@ -0,0 +1,8 @@ +FactoryGirl.define do + factory :upload do + model { build(:project) } + path { "uploads/system/project/avatar/avatar.jpg" } + size 100.kilobytes + uploader "AvatarUploader" + end +end diff --git a/spec/features/admin/admin_appearance_spec.rb b/spec/features/admin/admin_appearance_spec.rb index 96d715ef383..595366ce352 100644 --- a/spec/features/admin/admin_appearance_spec.rb +++ b/spec/features/admin/admin_appearance_spec.rb @@ -63,11 +63,11 @@ feature 'Admin Appearance', feature: true do end def logo_selector - '//img[@src^="/uploads/appearance/logo"]' + '//img[@src^="/uploads/system/appearance/logo"]' end def header_logo_selector - '//img[@src^="/uploads/appearance/header_logo"]' + '//img[@src^="/uploads/system/appearance/header_logo"]' end def logo_fixture diff --git a/spec/features/uploads/user_uploads_avatar_to_group_spec.rb b/spec/features/uploads/user_uploads_avatar_to_group_spec.rb index f88a515f7fc..d9d6f2e2382 100644 --- a/spec/features/uploads/user_uploads_avatar_to_group_spec.rb +++ b/spec/features/uploads/user_uploads_avatar_to_group_spec.rb @@ -18,7 +18,7 @@ feature 'User uploads avatar to group', feature: true do visit group_path(group) - expect(page).to have_selector(%Q(img[src$="/uploads/group/avatar/#{group.id}/dk.png"])) + expect(page).to have_selector(%Q(img[src$="/uploads/system/group/avatar/#{group.id}/dk.png"])) # Cheating here to verify something that isn't user-facing, but is important expect(group.reload.avatar.file).to exist diff --git a/spec/features/uploads/user_uploads_avatar_to_profile_spec.rb b/spec/features/uploads/user_uploads_avatar_to_profile_spec.rb index 0dfd29045e5..eb8dbd76aab 100644 --- a/spec/features/uploads/user_uploads_avatar_to_profile_spec.rb +++ b/spec/features/uploads/user_uploads_avatar_to_profile_spec.rb @@ -16,7 +16,7 @@ feature 'User uploads avatar to profile', feature: true do visit user_path(user) - expect(page).to have_selector(%Q(img[src$="/uploads/user/avatar/#{user.id}/dk.png"])) + expect(page).to have_selector(%Q(img[src$="/uploads/system/user/avatar/#{user.id}/dk.png"])) # Cheating here to verify something that isn't user-facing, but is important expect(user.reload.avatar.file).to exist diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb index 785fb724132..49df91b236f 100644 --- a/spec/helpers/application_helper_spec.rb +++ b/spec/helpers/application_helper_spec.rb @@ -1,3 +1,4 @@ +# coding: utf-8 require 'spec_helper' describe ApplicationHelper do @@ -58,13 +59,13 @@ describe ApplicationHelper do describe 'project_icon' do it 'returns an url for the avatar' do 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). to eq "" 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). to eq "" @@ -84,12 +85,12 @@ describe ApplicationHelper do it 'returns an url for the avatar' do 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) 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) end @@ -102,7 +103,7 @@ describe ApplicationHelper do user = create(:user, avatar: File.open(uploaded_image_temp_path)) expect(helper.avatar_icon(user.email).to_s). - to match("/gitlab/uploads/user/avatar/#{user.id}/banana_sample.gif") + to match("/gitlab/uploads/system/user/avatar/#{user.id}/banana_sample.gif") end it 'calls gravatar_icon when no User exists with the given email' do @@ -116,7 +117,7 @@ describe ApplicationHelper do user = create(:user, avatar: File.open(uploaded_image_temp_path)) expect(helper.avatar_icon(user).to_s). - to match("/uploads/user/avatar/#{user.id}/banana_sample.gif") + to match("/uploads/system/user/avatar/#{user.id}/banana_sample.gif") end end end diff --git a/spec/helpers/emails_helper_spec.rb b/spec/helpers/emails_helper_spec.rb index cd112dbb2fb..c68e4f56b05 100644 --- a/spec/helpers/emails_helper_spec.rb +++ b/spec/helpers/emails_helper_spec.rb @@ -52,7 +52,7 @@ describe EmailsHelper do ) expect(header_logo).to eq( - %{} + %{} ) end end diff --git a/spec/helpers/groups_helper_spec.rb b/spec/helpers/groups_helper_spec.rb index c8b0d86425f..0337afa4452 100644 --- a/spec/helpers/groups_helper_spec.rb +++ b/spec/helpers/groups_helper_spec.rb @@ -9,7 +9,7 @@ describe GroupsHelper do group.avatar = fixture_file_upload(avatar_file_path) group.save! expect(group_icon(group.path).to_s). - to match("/uploads/group/avatar/#{group.id}/banana_sample.gif") + to match("/uploads/system/group/avatar/#{group.id}/banana_sample.gif") end it 'gives default avatar_icon when no avatar is present' do diff --git a/spec/helpers/page_layout_helper_spec.rb b/spec/helpers/page_layout_helper_spec.rb index 2cc0b40b2d0..dff2784f21f 100644 --- a/spec/helpers/page_layout_helper_spec.rb +++ b/spec/helpers/page_layout_helper_spec.rb @@ -60,7 +60,7 @@ describe PageLayoutHelper do %w(project user group).each do |type| context "with @#{type} assigned" do it "uses #{type.titlecase} avatar if available" do - object = double(avatar_url: 'http://example.com/uploads/avatar.png') + object = double(avatar_url: 'http://example.com/uploads/system/avatar.png') assign(type, object) expect(helper.page_image).to eq object.avatar_url diff --git a/spec/javascripts/notes_spec.js b/spec/javascripts/notes_spec.js index 24335614e09..bfd8b8648a6 100644 --- a/spec/javascripts/notes_spec.js +++ b/spec/javascripts/notes_spec.js @@ -461,6 +461,45 @@ import '~/notes'; }); }); + describe('update comment with script tags', () => { + const sampleComment = ''; + const updatedComment = ''; + const note = { + id: 1234, + html: `