Revert "Fix broken access control and refactor avatar upload"

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

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

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) }

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) }

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

app/uploaders/avatar_uploader.rb

-# 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

db/migrate/20150213111727_move_note_folder.rb

-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

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
 

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
 

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

lib/backup/manager.rb

 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

lib/backup/uploads.rb

 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