Skip to content

Commit

Permalink
Add a token the filename for exported user data
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jhass committed Apr 22, 2015
1 parent 7648b58 commit 0a70e51
Show file tree
Hide file tree
Showing 5 changed files with 15 additions and 13 deletions.
2 changes: 1 addition & 1 deletion app/controllers/users_controller.rb
Expand Up @@ -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
Expand Down
8 changes: 2 additions & 6 deletions app/uploaders/exported_photos.rb
Expand Up @@ -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"
Expand All @@ -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
4 changes: 2 additions & 2 deletions app/uploaders/exported_user.rb
Expand Up @@ -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"
Expand All @@ -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
7 changes: 7 additions & 0 deletions 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
7 changes: 3 additions & 4 deletions spec/controllers/users_controller_spec.rb
Expand Up @@ -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

Expand All @@ -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!
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 0a70e51

Please sign in to comment.