SECURITY: do not delete avatars uploads when deleting accounts

We rely on the clean up uploads job to do this safely
This commit is contained in:
Sam 2018-12-13 16:26:07 +11:00
parent f74ef71130
commit 7ee9a6a7ec
2 changed files with 9 additions and 4 deletions

View File

@ -3,8 +3,8 @@ require_dependency 'upload_creator'
class UserAvatar < ActiveRecord::Base class UserAvatar < ActiveRecord::Base
belongs_to :user belongs_to :user
belongs_to :gravatar_upload, class_name: 'Upload', dependent: :destroy belongs_to :gravatar_upload, class_name: 'Upload'
belongs_to :custom_upload, class_name: 'Upload', dependent: :destroy belongs_to :custom_upload, class_name: 'Upload'
def contains_upload?(id) def contains_upload?(id)
gravatar_upload_id == id || custom_upload_id == id gravatar_upload_id == id || custom_upload_id == id
@ -46,7 +46,6 @@ class UserAvatar < ActiveRecord::Base
user.update!(uploaded_avatar_id: upload.id) user.update!(uploaded_avatar_id: upload.id)
end end
gravatar_upload&.destroy!
self.update!(gravatar_upload: upload) self.update!(gravatar_upload: upload)
end end
end end

View File

@ -30,6 +30,11 @@ describe UserAvatar do
expect(avatar.gravatar_upload).to eq(Upload.last) expect(avatar.gravatar_upload).to eq(Upload.last)
expect(avatar.last_gravatar_download_attempt).to eq(Time.now) expect(avatar.last_gravatar_download_attempt).to eq(Time.now)
expect(user.reload.uploaded_avatar).to eq(nil) expect(user.reload.uploaded_avatar).to eq(nil)
expect do
avatar.destroy
end.to_not change { Upload.count }
end end
describe 'when user has an existing custom upload' do describe 'when user has an existing custom upload' do
@ -57,7 +62,8 @@ describe UserAvatar do
avatar.update_gravatar! avatar.update_gravatar!
expect(Upload.find_by(id: upload.id)).to eq(nil) # old upload to be cleaned up via clean_up_uploads
expect(Upload.find_by(id: upload.id)).not_to eq(nil)
new_upload = Upload.last new_upload = Upload.last