Commit 0a70e51f authored by Jonne Haß's avatar Jonne Haß
Browse files

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.
parent 7648b58c

app/controllers/users_controller.rb

+1 −1
Original line number Diff line number Diff line
@@ -140,7 +140,7 @@ class UsersController < ApplicationController 
  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

app/uploaders/exported_photos.rb

+2 −6
Original line number Diff line number Diff line
@@ -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 @@ class ExportedPhotos < CarrierWave::Uploader::Base 
    "#{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

app/uploaders/exported_user.rb

+2 −2
Original line number Diff line number Diff line
@@ -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 @@ class ExportedUser < CarrierWave::Uploader::Base 
  end



  def filename

    "#{model.username}_diaspora_data.json.gz"

    "#{model.username}_diaspora_data_#{secure_token}.json.gz"

  end



end

app/uploaders/secure_uploader.rb

0 → 100644
+7 −0
Original line number Diff line number Diff line 
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

spec/controllers/users_controller_spec.rb

+3 −4
Original line number Diff line number Diff line
@@ -24,8 +24,7 @@ describe UsersController, :type => :controller do 
    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