SECURITY: Users can pick non-avatar uploads.
https://meta.discourse.org/t/bug-report-idor-on-avatar-pick-function-discussions-udacity-com/103564
This commit is contained in:
parent
899caf35ba
commit
5c2e194d01
|
@ -3,19 +3,13 @@ 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?(
|
UserUpload.exists?(upload_id: upload.id, user_id: user.id)
|
||||||
upload_id: upload.id,
|
|
||||||
user_id: [upload.user_id, user.id]
|
|
||||||
)
|
|
||||||
end
|
end
|
||||||
|
|
||||||
def can_edit_user?(user)
|
def can_edit_user?(user)
|
||||||
|
|
|
@ -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
|
||||||
UserAvatar.new(user_id: user.id)
|
Fabricate(:user_avatar, user: user)
|
||||||
end
|
end
|
||||||
|
|
||||||
let :users_upload do
|
let :users_upload do
|
||||||
|
@ -54,19 +54,24 @@ 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
|
||||||
yes_my_upload = not_my_upload
|
UserUpload.create!(
|
||||||
UserUpload.create!(upload_id: yes_my_upload.id, user_id: user_avatar.user_id)
|
upload_id: not_my_upload.id,
|
||||||
expect(guardian.can_pick_avatar?(user_avatar, yes_my_upload)).to eq(true)
|
user_id: user_avatar.user_id
|
||||||
|
)
|
||||||
|
|
||||||
UserUpload.destroy_all
|
expect(guardian.can_pick_avatar?(user_avatar, not_my_upload))
|
||||||
|
.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
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue