Commit 55a906f6 authored by Dmitriy Zaporozhets's avatar Dmitriy Zaporozhets

Merge branch 'fix_access_control_notes' into 'master'

Fix broken access control for note attachments

From https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/332 by Hannes Rosenögger.

See merge request !1528
parents 112db32f ebd39fc0
v 7.8.0 (unreleased) v 7.8.0 (unreleased)
- Fix broken access control for note attachments (Hannes Rosenögger)
- Replace highlight.js with rouge-fork rugments (Stefan Tatschner) - Replace highlight.js with rouge-fork rugments (Stefan Tatschner)
- Make project search case insensitive (Hannes Rosenögger) - Make project search case insensitive (Hannes Rosenögger)
- Include issue/mr participants in list of recipients for reassign/close/reopen emails - Include issue/mr participants in list of recipients for reassign/close/reopen emails
......
...@@ -5,8 +5,11 @@ class FilesController < ApplicationController ...@@ -5,8 +5,11 @@ class FilesController < ApplicationController
if uploader.file_storage? if uploader.file_storage?
if can?(current_user, :read_project, note.project) if can?(current_user, :read_project, note.project)
# Replace old notes location in /public with the new one in / and send the file
path = uploader.file.path.gsub("#{Rails.root}/public", Rails.root.to_s)
disposition = uploader.image? ? 'inline' : 'attachment' disposition = uploader.image? ? 'inline' : 'attachment'
send_file uploader.file.path, disposition: disposition send_file path, disposition: disposition
else else
not_found! not_found!
end end
......
...@@ -23,7 +23,7 @@ class Group < Namespace ...@@ -23,7 +23,7 @@ class Group < Namespace
validate :avatar_type, if: ->(user) { user.avatar_changed? } validate :avatar_type, if: ->(user) { user.avatar_changed? }
validates :avatar, file_size: { maximum: 200.kilobytes.to_i } validates :avatar, file_size: { maximum: 200.kilobytes.to_i }
mount_uploader :avatar, AttachmentUploader mount_uploader :avatar, AvatarUploader
after_create :post_create_hook after_create :post_create_hook
after_destroy :post_destroy_hook after_destroy :post_destroy_hook
......
...@@ -138,7 +138,7 @@ class Project < ActiveRecord::Base ...@@ -138,7 +138,7 @@ class Project < ActiveRecord::Base
if: ->(project) { project.avatar && project.avatar_changed? } if: ->(project) { project.avatar && project.avatar_changed? }
validates :avatar, file_size: { maximum: 200.kilobytes.to_i } validates :avatar, file_size: { maximum: 200.kilobytes.to_i }
mount_uploader :avatar, AttachmentUploader mount_uploader :avatar, AvatarUploader
# Scopes # Scopes
scope :sorted_by_activity, -> { reorder(last_activity_at: :desc) } scope :sorted_by_activity, -> { reorder(last_activity_at: :desc) }
......
...@@ -177,7 +177,7 @@ class User < ActiveRecord::Base ...@@ -177,7 +177,7 @@ class User < ActiveRecord::Base
end end
end end
mount_uploader :avatar, AttachmentUploader mount_uploader :avatar, AvatarUploader
# Scopes # Scopes
scope :admins, -> { where(admin: true) } scope :admins, -> { where(admin: true) }
......
...@@ -3,10 +3,8 @@ ...@@ -3,10 +3,8 @@
class AttachmentUploader < CarrierWave::Uploader::Base class AttachmentUploader < CarrierWave::Uploader::Base
storage :file storage :file
after :store, :reset_events_cache
def store_dir def store_dir
"uploads/#{model.class.to_s.underscore}/#{mounted_as}/#{model.id}" "#{Rails.root}/uploads/#{model.class.to_s.underscore}/#{mounted_as}/#{model.id}"
end end
def image? def image?
...@@ -29,8 +27,4 @@ class AttachmentUploader < CarrierWave::Uploader::Base ...@@ -29,8 +27,4 @@ class AttachmentUploader < CarrierWave::Uploader::Base
def file_storage? def file_storage?
self.class.storage == CarrierWave::Storage::File self.class.storage == CarrierWave::Storage::File
end end
def reset_events_cache(file)
model.reset_events_cache if model.is_a?(User)
end
end end
# encoding: utf-8
class AvatarUploader < CarrierWave::Uploader::Base
storage :file
after :store, :reset_events_cache
def store_dir
"uploads/#{model.class.to_s.underscore}/#{mounted_as}/#{model.id}"
end
def image?
img_ext = %w(png jpg jpeg gif bmp tiff)
if file.respond_to?(:extension)
img_ext.include?(file.extension.downcase)
else
# Not all CarrierWave storages respond to :extension
ext = file.path.split('.').last.downcase
img_ext.include?(ext)
end
rescue
false
end
def file_storage?
self.class.storage == CarrierWave::Storage::File
end
def reset_events_cache(file)
model.reset_events_cache if model.is_a?(User)
end
end
class MoveNoteFolder < ActiveRecord::Migration
def up
system(
"if [ -d '#{Rails.root}/public/uploads/note' ];
then mv #{Rails.root}/public/uploads/note #{Rails.root}/uploads/note;
echo 'note folder has been moved successfully';
else
echo 'note folder has already been moved or does not exist yet. Nothing to do here.'; fi")
end
def down
system(
"if [ -d '#{Rails.root}/uploads/note' ];
then mv #{Rails.root}/uploads/note #{Rails.root}/public/uploads/note;
echo 'note folder has been moved successfully';
else
echo 'note folder has already been moved or does not exist yet. Nothing to do here.'; fi")
end
end
...@@ -110,7 +110,7 @@ class Spinach::Features::Groups < Spinach::FeatureSteps ...@@ -110,7 +110,7 @@ class Spinach::Features::Groups < Spinach::FeatureSteps
end end
step 'I should see new group "Owned" avatar' do step 'I should see new group "Owned" avatar' do
Group.find_by(name: "Owned").avatar.should be_instance_of AttachmentUploader Group.find_by(name: "Owned").avatar.should be_instance_of AvatarUploader
Group.find_by(name: "Owned").avatar.url.should == "/uploads/group/avatar/#{ Group.find_by(name:"Owned").id }/gitlab_logo.png" Group.find_by(name: "Owned").avatar.url.should == "/uploads/group/avatar/#{ Group.find_by(name:"Owned").id }/gitlab_logo.png"
end end
......
...@@ -29,7 +29,7 @@ class Spinach::Features::Profile < Spinach::FeatureSteps ...@@ -29,7 +29,7 @@ class Spinach::Features::Profile < Spinach::FeatureSteps
end end
step 'I should see new avatar' do step 'I should see new avatar' do
@user.avatar.should be_instance_of AttachmentUploader @user.avatar.should be_instance_of AvatarUploader
@user.avatar.url.should == "/uploads/user/avatar/#{ @user.id }/gitlab_logo.png" @user.avatar.url.should == "/uploads/user/avatar/#{ @user.id }/gitlab_logo.png"
end end
......
...@@ -35,7 +35,7 @@ class Spinach::Features::Project < Spinach::FeatureSteps ...@@ -35,7 +35,7 @@ class Spinach::Features::Project < Spinach::FeatureSteps
end end
step 'I should see new project avatar' do step 'I should see new project avatar' do
@project.avatar.should be_instance_of AttachmentUploader @project.avatar.should be_instance_of AvatarUploader
url = @project.avatar.url url = @project.avatar.url
url.should == "/uploads/project/avatar/#{ @project.id }/gitlab_logo.png" url.should == "/uploads/project/avatar/#{ @project.id }/gitlab_logo.png"
end end
......
module Backup module Backup
class Manager class Manager
BACKUP_CONTENTS = %w{repositories/ db/ uploads/ backup_information.yml} BACKUP_CONTENTS = %w{repositories/ db/ public/ uploads/ backup_information.yml}
def pack def pack
# saving additional informations # saving additional informations
......
module Backup module Backup
class Uploads class Uploads
attr_reader :app_uploads_dir, :backup_uploads_dir, :backup_dir attr_reader :app_public_uploads_dir, :app_private_uploads_dir, :backup_public_uploads_dir,
:backup_private_uploads_dir, :backup_dir, :backup_public_dir
def initialize def initialize
@app_uploads_dir = File.realpath(Rails.root.join('public', 'uploads')) @app_public_uploads_dir = File.realpath(Rails.root.join('public', 'uploads'))
@app_private_uploads_dir = File.realpath(Rails.root.join('uploads'))
@backup_dir = Gitlab.config.backup.path @backup_dir = Gitlab.config.backup.path
@backup_uploads_dir = File.join(Gitlab.config.backup.path, 'uploads') @backup_public_dir = File.join(backup_dir, 'public')
@backup_public_uploads_dir = File.join(backup_dir, 'public', 'uploads')
@backup_private_uploads_dir = File.join(backup_dir, 'uploads')
end end
# Copy uploads from public/uploads to backup/uploads # Copy uploads from public/uploads to backup/public/uploads and from /uploads to backup/uploads
def dump def dump
FileUtils.mkdir_p(backup_uploads_dir) FileUtils.mkdir_p(backup_public_uploads_dir)
FileUtils.cp_r(app_uploads_dir, backup_dir) FileUtils.cp_r(app_public_uploads_dir, backup_public_dir)
FileUtils.mkdir_p(backup_private_uploads_dir)
FileUtils.cp_r(app_private_uploads_dir, backup_dir)
end end
def restore def restore
backup_existing_uploads_dir backup_existing_public_uploads_dir
backup_existing_private_uploads_dir
FileUtils.cp_r(backup_uploads_dir, app_uploads_dir) FileUtils.cp_r(backup_public_uploads_dir, app_public_uploads_dir)
FileUtils.cp_r(backup_private_uploads_dir, app_private_uploads_dir)
end end
def backup_existing_uploads_dir def backup_existing_public_uploads_dir
timestamped_uploads_path = File.join(app_uploads_dir, '..', "uploads.#{Time.now.to_i}") timestamped_public_uploads_path = File.join(app_public_uploads_dir, '..', "uploads.#{Time.now.to_i}")
if File.exists?(app_uploads_dir) if File.exists?(app_public_uploads_dir)
FileUtils.mv(app_uploads_dir, timestamped_uploads_path) FileUtils.mv(app_public_uploads_dir, timestamped_public_uploads_path)
end
end
def backup_existing_private_uploads_dir
timestamped_private_uploads_path = File.join(app_private_uploads_dir, '..', "uploads.#{Time.now.to_i}")
if File.exists?(app_private_uploads_dir)
FileUtils.mv(app_private_uploads_dir, timestamped_private_uploads_path)
end end
end end
end end
......
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