FIX: respect moderator group permissions in guardian (#10713)

Since 9e4ed03, moderators can view groups with visibility level set to "Group owners, members and moderators".

This fixes an issue where moderators can see the group in /g but then get a 404 when clicking on individual groups.
This commit is contained in:
Penar Musaraj 2020-09-21 12:32:43 -04:00 committed by GitHub
parent f1743ff69c
commit 577293c438
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 7 additions and 5 deletions

View File

@ -206,6 +206,7 @@ class Guardian
return false if group.blank? return false if group.blank?
return true if is_admin? || group.members_visibility_level == Group.visibility_levels[:public] return true if is_admin? || group.members_visibility_level == Group.visibility_levels[:public]
return true if is_staff? && group.members_visibility_level == Group.visibility_levels[:staff] return true if is_staff? && group.members_visibility_level == Group.visibility_levels[:staff]
return true if is_staff? && group.members_visibility_level == Group.visibility_levels[:members]
return true if authenticated? && group.members_visibility_level == Group.visibility_levels[:logged_on_users] return true if authenticated? && group.members_visibility_level == Group.visibility_levels[:logged_on_users]
return false if user.blank? return false if user.blank?
@ -222,6 +223,7 @@ class Guardian
return false if groups.blank? return false if groups.blank?
return true if is_admin? || groups.all? { |g| g.visibility_level == Group.visibility_levels[:public] } return true if is_admin? || groups.all? { |g| g.visibility_level == Group.visibility_levels[:public] }
return true if is_staff? && groups.all? { |g| g.visibility_level == Group.visibility_levels[:staff] } return true if is_staff? && groups.all? { |g| g.visibility_level == Group.visibility_levels[:staff] }
return true if is_staff? && groups.all? { |g| g.visibility_level == Group.visibility_levels[:members] }
return true if authenticated? && groups.all? { |g| g.visibility_level == Group.visibility_levels[:logged_on_users] } return true if authenticated? && groups.all? { |g| g.visibility_level == Group.visibility_levels[:logged_on_users] }
return false if user.blank? return false if user.blank?

View File

@ -3193,7 +3193,7 @@ describe Guardian do
group.add_owner(owner) group.add_owner(owner)
group.reload group.reload
expect(Guardian.new(moderator).can_see_group?(group)).to eq(false) expect(Guardian.new(moderator).can_see_group?(group)).to eq(true)
expect(Guardian.new.can_see_group?(group)).to eq(false) expect(Guardian.new.can_see_group?(group)).to eq(false)
expect(Guardian.new(another_user).can_see_group?(group)).to eq(false) expect(Guardian.new(another_user).can_see_group?(group)).to eq(false)
expect(Guardian.new(admin).can_see_group?(group)).to eq(true) expect(Guardian.new(admin).can_see_group?(group)).to eq(true)
@ -3277,7 +3277,7 @@ describe Guardian do
group.add_owner(owner) group.add_owner(owner)
group.reload group.reload
expect(Guardian.new(moderator).can_see_group_members?(group)).to eq(false) expect(Guardian.new(moderator).can_see_group_members?(group)).to eq(true)
expect(Guardian.new.can_see_group_members?(group)).to eq(false) expect(Guardian.new.can_see_group_members?(group)).to eq(false)
expect(Guardian.new(another_user).can_see_group_members?(group)).to eq(false) expect(Guardian.new(another_user).can_see_group_members?(group)).to eq(false)
expect(Guardian.new(admin).can_see_group_members?(group)).to eq(true) expect(Guardian.new(admin).can_see_group_members?(group)).to eq(true)
@ -3383,7 +3383,7 @@ describe Guardian do
group.reload group.reload
expect(Guardian.new(another_user).can_see_groups?([group])).to eq(false) expect(Guardian.new(another_user).can_see_groups?([group])).to eq(false)
expect(Guardian.new(moderator).can_see_groups?([group])).to eq(false) expect(Guardian.new(moderator).can_see_groups?([group])).to eq(true)
expect(Guardian.new.can_see_groups?([group])).to eq(false) expect(Guardian.new.can_see_groups?([group])).to eq(false)
expect(Guardian.new(admin).can_see_groups?([group])).to eq(true) expect(Guardian.new(admin).can_see_groups?([group])).to eq(true)
expect(Guardian.new(member).can_see_groups?([group])).to eq(true) expect(Guardian.new(member).can_see_groups?([group])).to eq(true)
@ -3422,7 +3422,7 @@ describe Guardian do
group1.add_owner(owner) group1.add_owner(owner)
group1.reload group1.reload
expect(Guardian.new(moderator).can_see_groups?([group1, group2])).to eq(false) expect(Guardian.new(moderator).can_see_groups?([group1, group2])).to eq(true)
expect(Guardian.new.can_see_groups?([group1, group2])).to eq(false) expect(Guardian.new.can_see_groups?([group1, group2])).to eq(false)
expect(Guardian.new(admin).can_see_groups?([group1, group2])).to eq(true) expect(Guardian.new(admin).can_see_groups?([group1, group2])).to eq(true)
expect(Guardian.new(member).can_see_groups?([group1, group2])).to eq(false) expect(Guardian.new(member).can_see_groups?([group1, group2])).to eq(false)

View File

@ -845,7 +845,7 @@ describe GroupsController do
end end
it 'should not be able to update a group it cannot see' do it 'should not be able to update a group it cannot see' do
group.update!(visibility_level: 2) group.update!(visibility_level: Group.visibility_levels[:owners])
put "/groups/#{group.id}.json", params: { group: { name: 'testing' } } put "/groups/#{group.id}.json", params: { group: { name: 'testing' } }