Revert "SECURITY: User could non-avatar uploads."

This reverts commit 89581fa301.
This commit is contained in:
Guo Xiang Tan 2018-12-18 13:37:31 +08:00
parent 89581fa301
commit 899caf35ba
2 changed files with 16 additions and 15 deletions

View File

@ -3,13 +3,19 @@ module UserGuardian
def can_pick_avatar?(user_avatar, upload) def can_pick_avatar?(user_avatar, upload)
return false unless self.user return false unless self.user
return true if is_admin? return true if is_admin?
# can always pick blank avatar # can always pick blank avatar
return true if !upload return true if !upload
return true if user_avatar.contains_upload?(upload.id) return true if user_avatar.contains_upload?(upload.id)
return true if upload.user_id == user_avatar.user_id || upload.user_id == user.id return true if upload.user_id == user_avatar.user_id || upload.user_id == user.id
UserUpload.exists?(upload_id: upload.id, user_id: user.id) UserUpload.exists?(
upload_id: upload.id,
user_id: [upload.user_id, user.id]
)
end end
def can_edit_user?(user) def can_edit_user?(user)

View File

@ -14,8 +14,8 @@ describe UserGuardian do
Fabricate.build(:admin, id: 3) Fabricate.build(:admin, id: 3)
end end
let(:user_avatar) do let :user_avatar do
Fabricate(:user_avatar, user: user) UserAvatar.new(user_id: user.id)
end end
let :users_upload do let :users_upload do
@ -54,24 +54,19 @@ describe UserGuardian do
it "can not set uploads not owned by current user" do it "can not set uploads not owned by current user" do
expect(guardian.can_pick_avatar?(user_avatar, users_upload)).to eq(true) expect(guardian.can_pick_avatar?(user_avatar, users_upload)).to eq(true)
expect(guardian.can_pick_avatar?(user_avatar, already_uploaded)).to eq(true) expect(guardian.can_pick_avatar?(user_avatar, already_uploaded)).to eq(true)
UserUpload.create!(
upload_id: not_my_upload.id,
user_id: not_my_upload.user_id
)
expect(guardian.can_pick_avatar?(user_avatar, not_my_upload)).to eq(false) expect(guardian.can_pick_avatar?(user_avatar, not_my_upload)).to eq(false)
expect(guardian.can_pick_avatar?(user_avatar, nil)).to eq(true) expect(guardian.can_pick_avatar?(user_avatar, nil)).to eq(true)
end end
it "can handle uploads that are associated but not directly owned" do it "can handle uploads that are associated but not directly owned" do
UserUpload.create!( yes_my_upload = not_my_upload
upload_id: not_my_upload.id, UserUpload.create!(upload_id: yes_my_upload.id, user_id: user_avatar.user_id)
user_id: user_avatar.user_id expect(guardian.can_pick_avatar?(user_avatar, yes_my_upload)).to eq(true)
)
expect(guardian.can_pick_avatar?(user_avatar, not_my_upload)) UserUpload.destroy_all
.to eq(true)
UserUpload.create!(upload_id: yes_my_upload.id, user_id: yes_my_upload.user_id)
expect(guardian.can_pick_avatar?(user_avatar, yes_my_upload)).to eq(true)
end end
end end