From 0a70e51f741621710f96011a8905c4e8ba9611f6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jonne=20Ha=C3=9F?= <me@jhass.eu>
Date: Wed, 22 Apr 2015 19:50:25 +0200
Subject: [PATCH] Add a token the filename for exported user data

Also redirect to it for download, for Amazon S3
compatibility.

Prior to this patch an attacker could obtain an
users export by guessing the filename with a high
chance of success. Fully authenticating the
download request is a lot harder due to our diverse
deployment scenarios.

This brings the used method in line with the photo
export feature.

Thanks to @tomekr for the report.
---
 app/controllers/users_controller.rb       | 2 +-
 app/uploaders/exported_photos.rb          | 8 ++------
 app/uploaders/exported_user.rb            | 4 ++--
 app/uploaders/secure_uploader.rb          | 7 +++++++
 spec/controllers/users_controller_spec.rb | 7 +++----
 5 files changed, 15 insertions(+), 13 deletions(-)
 create mode 100644 app/uploaders/secure_uploader.rb

diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
index eff00e1388f..00932312af2 100644
--- a/app/controllers/users_controller.rb
+++ b/app/controllers/users_controller.rb
@@ -140,7 +140,7 @@ def export_profile
   end
 
   def download_profile
-    send_data File.open(current_user.export.path).read, type: :json, filename: current_user.export.filename
+    redirect_to current_user.export.url
   end
 
   def export_photos
diff --git a/app/uploaders/exported_photos.rb b/app/uploaders/exported_photos.rb
index d68f94e0e14..0e4465cdd26 100644
--- a/app/uploaders/exported_photos.rb
+++ b/app/uploaders/exported_photos.rb
@@ -2,7 +2,7 @@
 #   licensed under the Affero General Public License version 3 or later.  See
 #   the COPYRIGHT file.
 
-class ExportedPhotos < CarrierWave::Uploader::Base
+class ExportedPhotos < SecureUploader
 
   def store_dir
     "uploads/users"
@@ -12,10 +12,6 @@ def filename
     "#{model.username}_photos_#{secure_token}.zip" if original_filename.present?
   end
 
-  protected
-  def secure_token(bytes = 16)
-    var = :"@#{mounted_as}_secure_token"
-    model.instance_variable_get(var) or model.instance_variable_set(var, SecureRandom.urlsafe_base64(bytes))
-  end
+
 
 end
diff --git a/app/uploaders/exported_user.rb b/app/uploaders/exported_user.rb
index ff323ecce7b..a7c4d5a2b21 100644
--- a/app/uploaders/exported_user.rb
+++ b/app/uploaders/exported_user.rb
@@ -2,7 +2,7 @@
 #   licensed under the Affero General Public License version 3 or later.  See
 #   the COPYRIGHT file.
 
-class ExportedUser < CarrierWave::Uploader::Base
+class ExportedUser < SecureUploader
 
   def store_dir
     "uploads/users"
@@ -13,7 +13,7 @@ def extension_white_list
   end
 
   def filename
-    "#{model.username}_diaspora_data.json.gz"
+    "#{model.username}_diaspora_data_#{secure_token}.json.gz"
   end
 
 end
diff --git a/app/uploaders/secure_uploader.rb b/app/uploaders/secure_uploader.rb
new file mode 100644
index 00000000000..08bbed1c733
--- /dev/null
+++ b/app/uploaders/secure_uploader.rb
@@ -0,0 +1,7 @@
+class SecureUploader < CarrierWave::Uploader::Base
+  protected
+  def secure_token(bytes = 16)
+    var = :"@#{mounted_as}_secure_token"
+    model.instance_variable_get(var) or model.instance_variable_set(var, SecureRandom.urlsafe_base64(bytes))
+  end
+end
diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb
index 19a3819b94f..032160cecb0 100644
--- a/spec/controllers/users_controller_spec.rb
+++ b/spec/controllers/users_controller_spec.rb
@@ -24,8 +24,7 @@
     it "downloads a user's export file" do
       @user.perform_export!
       get :download_profile
-      parsed = JSON.parse(ActiveSupport::Gzip.decompress(response.body))
-      expect(parsed['user']['username']).to eq @user.username
+      expect(response).to redirect_to(@user.export.url)
     end
   end
 
@@ -37,7 +36,7 @@
       expect(response).to redirect_to(edit_user_path)
     end
   end
-  
+
   describe '#download_photos' do
     it "redirects to user's photos zip file"  do
       @user.perform_export_photos!
@@ -74,7 +73,7 @@
       get :public, :username => @user.username, :format => :atom
       expect(response.body).to include('a href')
     end
-    
+
     it 'includes reshares in the atom feed' do
       reshare = FactoryGirl.create(:reshare, :author => @user.person)
       get :public, :username => @user.username, :format => :atom