From 8184a6564454faf0f9ae9dfee1377c3407d08447 Mon Sep 17 00:00:00 2001
From: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
Date: Thu, 19 Feb 2015 08:57:35 -0800
Subject: [PATCH] Revert "Fix broken access control and refactor avatar upload"
This reverts commit 7d5f86f6cbd187e75a6ba164ad6bfd036977dd07.
---
app/controllers/files_controller.rb | 4 +-
app/models/group.rb | 2 +-
app/models/project.rb | 2 +-
app/models/user.rb | 2 +-
app/uploaders/attachment_uploader.rb | 8 +++-
app/uploaders/avatar_uploader.rb | 32 ---------------
db/migrate/20150213111727_move_note_folder.rb | 19 ---------
features/steps/groups.rb | 2 +-
features/steps/profile/profile.rb | 2 +-
features/steps/project/project.rb | 2 +-
lib/backup/manager.rb | 2 +-
lib/backup/uploads.rb | 40 ++++++-------------
uploads/.gitkeep | 0
13 files changed, 27 insertions(+), 90 deletions(-)
delete mode 100644 app/uploaders/avatar_uploader.rb
delete mode 100644 db/migrate/20150213111727_move_note_folder.rb
delete mode 100644 uploads/.gitkeep
diff --git a/app/controllers/files_controller.rb b/app/controllers/files_controller.rb
index 561af8084c3..9671245d3f4 100644
--- a/app/controllers/files_controller.rb
+++ b/app/controllers/files_controller.rb
@@ -6,9 +6,7 @@ class FilesController < ApplicationController
if uploader.file_storage?
if can?(current_user, :read_project, note.project)
disposition = uploader.image? ? 'inline' : 'attachment'
- # 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)
- send_file path, disposition: disposition
+ send_file uploader.file.path, disposition: disposition
else
not_found!
end
diff --git a/app/models/group.rb b/app/models/group.rb
index da9621a2a1a..d6ec0be6081 100644
--- a/app/models/group.rb
+++ b/app/models/group.rb
@@ -23,7 +23,7 @@ class Group < Namespace
validate :avatar_type, if: ->(user) { user.avatar_changed? }
validates :avatar, file_size: { maximum: 200.kilobytes.to_i }
- mount_uploader :avatar, AvatarUploader
+ mount_uploader :avatar, AttachmentUploader
after_create :post_create_hook
after_destroy :post_destroy_hook
diff --git a/app/models/project.rb b/app/models/project.rb
index e2c7f76eb09..56e1aa29040 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -138,7 +138,7 @@ class Project < ActiveRecord::Base
if: ->(project) { project.avatar && project.avatar_changed? }
validates :avatar, file_size: { maximum: 200.kilobytes.to_i }
- mount_uploader :avatar, AvatarUploader
+ mount_uploader :avatar, AttachmentUploader
# Scopes
scope :sorted_by_activity, -> { reorder(last_activity_at: :desc) }
diff --git a/app/models/user.rb b/app/models/user.rb
index 3bbbd23c1bd..a9776b633a6 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -177,7 +177,7 @@ class User < ActiveRecord::Base
end
end
- mount_uploader :avatar, AvatarUploader
+ mount_uploader :avatar, AttachmentUploader
# Scopes
scope :admins, -> { where(admin: true) }
diff --git a/app/uploaders/attachment_uploader.rb b/app/uploaders/attachment_uploader.rb
index 22742d287a4..b122b6c8658 100644
--- a/app/uploaders/attachment_uploader.rb
+++ b/app/uploaders/attachment_uploader.rb
@@ -3,8 +3,10 @@
class AttachmentUploader < CarrierWave::Uploader::Base
storage :file
+ after :store, :reset_events_cache
+
def store_dir
- "#{Rails.root}/uploads/#{model.class.to_s.underscore}/#{mounted_as}/#{model.id}"
+ "uploads/#{model.class.to_s.underscore}/#{mounted_as}/#{model.id}"
end
def image?
@@ -27,4 +29,8 @@ class AttachmentUploader < CarrierWave::Uploader::Base
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
diff --git a/app/uploaders/avatar_uploader.rb b/app/uploaders/avatar_uploader.rb
deleted file mode 100644
index 7cad044555b..00000000000
--- a/app/uploaders/avatar_uploader.rb
+++ /dev/null
@@ -1,32 +0,0 @@
-# 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
diff --git a/db/migrate/20150213111727_move_note_folder.rb b/db/migrate/20150213111727_move_note_folder.rb
deleted file mode 100644
index ca7f87d984f..00000000000
--- a/db/migrate/20150213111727_move_note_folder.rb
+++ /dev/null
@@ -1,19 +0,0 @@
-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
diff --git a/features/steps/groups.rb b/features/steps/groups.rb
index 0a9b4ccba53..610e7fd3a48 100644
--- a/features/steps/groups.rb
+++ b/features/steps/groups.rb
@@ -110,7 +110,7 @@ class Spinach::Features::Groups < Spinach::FeatureSteps
end
step 'I should see new group "Owned" avatar' do
- Group.find_by(name: "Owned").avatar.should be_instance_of AvatarUploader
+ Group.find_by(name: "Owned").avatar.should be_instance_of AttachmentUploader
Group.find_by(name: "Owned").avatar.url.should == "/uploads/group/avatar/#{ Group.find_by(name:"Owned").id }/gitlab_logo.png"
end
diff --git a/features/steps/profile/profile.rb b/features/steps/profile/profile.rb
index 4efd2176782..a907b0b7dcf 100644
--- a/features/steps/profile/profile.rb
+++ b/features/steps/profile/profile.rb
@@ -29,7 +29,7 @@ class Spinach::Features::Profile < Spinach::FeatureSteps
end
step 'I should see new avatar' do
- @user.avatar.should be_instance_of AvatarUploader
+ @user.avatar.should be_instance_of AttachmentUploader
@user.avatar.url.should == "/uploads/user/avatar/#{ @user.id }/gitlab_logo.png"
end
diff --git a/features/steps/project/project.rb b/features/steps/project/project.rb
index d39c8e7d2db..033d45e0253 100644
--- a/features/steps/project/project.rb
+++ b/features/steps/project/project.rb
@@ -35,7 +35,7 @@ class Spinach::Features::Project < Spinach::FeatureSteps
end
step 'I should see new project avatar' do
- @project.avatar.should be_instance_of AvatarUploader
+ @project.avatar.should be_instance_of AttachmentUploader
url = @project.avatar.url
url.should == "/uploads/project/avatar/#{ @project.id }/gitlab_logo.png"
end
diff --git a/lib/backup/manager.rb b/lib/backup/manager.rb
index 06cd40a5b1c..ab8db4e9837 100644
--- a/lib/backup/manager.rb
+++ b/lib/backup/manager.rb
@@ -1,6 +1,6 @@
module Backup
class Manager
- BACKUP_CONTENTS = %w{repositories/ db/ public/ uploads/ backup_information.yml}
+ BACKUP_CONTENTS = %w{repositories/ db/ uploads/ backup_information.yml}
def pack
# saving additional informations
diff --git a/lib/backup/uploads.rb b/lib/backup/uploads.rb
index 75d8e18a862..e50e1ff4f13 100644
--- a/lib/backup/uploads.rb
+++ b/lib/backup/uploads.rb
@@ -1,45 +1,29 @@
module Backup
class Uploads
- attr_reader :app_public_uploads_dir, :app_private_uploads_dir, :backup_public_uploads_dir,
- :backup_private_uploads_dir, :backup_dir, :backup_public_dir
+ attr_reader :app_uploads_dir, :backup_uploads_dir, :backup_dir
def initialize
- @app_public_uploads_dir = File.realpath(Rails.root.join('public', 'uploads'))
- @app_private_uploads_dir = File.realpath(Rails.root.join('uploads'))
+ @app_uploads_dir = File.realpath(Rails.root.join('public', 'uploads'))
@backup_dir = Gitlab.config.backup.path
- @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')
+ @backup_uploads_dir = File.join(Gitlab.config.backup.path, 'uploads')
end
- # Copy uploads from public/uploads to backup/public/uploads and from /uploads to backup/uploads
+ # Copy uploads from public/uploads to backup/uploads
def dump
- FileUtils.mkdir_p(backup_public_uploads_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)
+ FileUtils.mkdir_p(backup_uploads_dir)
+ FileUtils.cp_r(app_uploads_dir, backup_dir)
end
def restore
- backup_existing_public_uploads_dir
- backup_existing_private_uploads_dir
+ backup_existing_uploads_dir
- FileUtils.cp_r(backup_public_uploads_dir, app_public_uploads_dir)
- FileUtils.cp_r(backup_private_uploads_dir, app_private_uploads_dir)
+ FileUtils.cp_r(backup_uploads_dir, app_uploads_dir)
end
- def backup_existing_public_uploads_dir
- timestamped_public_uploads_path = File.join(app_public_uploads_dir, '..', "uploads.#{Time.now.to_i}")
- if File.exists?(app_public_uploads_dir)
- 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)
+ def backup_existing_uploads_dir
+ timestamped_uploads_path = File.join(app_uploads_dir, '..', "uploads.#{Time.now.to_i}")
+ if File.exists?(app_uploads_dir)
+ FileUtils.mv(app_uploads_dir, timestamped_uploads_path)
end
end
end
diff --git a/uploads/.gitkeep b/uploads/.gitkeep
deleted file mode 100644
index e69de29bb2d..00000000000
--
GitLab