From 4da0a33524b54fdd65a1f279506f8ac42b3091d8 Mon Sep 17 00:00:00 2001 From: Bianca Nenciu Date: Wed, 21 Jul 2021 14:41:04 +0300 Subject: [PATCH] FIX: Allow only groups with flairs to be selected (#13744) It used the same permission check as for primary groups which is wrong because not all groups that can be primary have a flair. --- app/services/user_updater.rb | 2 +- lib/guardian.rb | 6 ++++++ spec/components/guardian_spec.rb | 27 +++++++++++++++++++++++++++ 3 files changed, 34 insertions(+), 1 deletion(-) diff --git a/app/services/user_updater.rb b/app/services/user_updater.rb index 1192419727d..adb2cc6dba1 100644 --- a/app/services/user_updater.rb +++ b/app/services/user_updater.rb @@ -124,7 +124,7 @@ class UserUpdater if attributes[:flair_group_id] && attributes[:flair_group_id] != user.flair_group_id && (attributes[:flair_group_id].blank? || - guardian.can_use_primary_group?(user, attributes[:flair_group_id])) + guardian.can_use_flair_group?(user, attributes[:flair_group_id])) user.flair_group_id = attributes[:flair_group_id] end diff --git a/lib/guardian.rb b/lib/guardian.rb index 33ef954c7c9..54822b95b92 100644 --- a/lib/guardian.rb +++ b/lib/guardian.rb @@ -323,6 +323,12 @@ class Guardian (group ? !group.automatic : false) end + def can_use_flair_group?(user, group_id = nil) + return false if !user || !group_id || !user.group_ids.include?(group_id.to_i) + flair_icon, flair_upload_id = Group.where(id: group_id.to_i).pluck_first(:flair_icon, :flair_upload_id) + flair_icon.present? || flair_upload_id.present? + end + def can_change_primary_group?(user) user && is_staff? end diff --git a/spec/components/guardian_spec.rb b/spec/components/guardian_spec.rb index 2d49dfc92c1..e07f63f5e81 100644 --- a/spec/components/guardian_spec.rb +++ b/spec/components/guardian_spec.rb @@ -2692,6 +2692,33 @@ describe Guardian do end end + describe 'can_use_flair_group?' do + fab!(:group) { Fabricate(:group, title: 'Groupie', flair_icon: 'icon') } + + it 'is false without a logged in user' do + expect(Guardian.new(nil).can_use_flair_group?(user)).to eq(false) + end + + it 'is false if the group does not exist' do + expect(Guardian.new(user).can_use_flair_group?(user, nil)).to eq(false) + expect(Guardian.new(user).can_use_flair_group?(user, Group.last.id + 1)).to eq(false) + end + + it 'is false if the user is not a part of the group' do + expect(Guardian.new(user).can_use_flair_group?(user, group.id)).to eq(false) + end + + it 'is false if the group does not have a flair' do + group.update(flair_icon: nil) + expect(Guardian.new(user).can_use_flair_group?(user, group.id)).to eq(false) + end + + it 'is true if the user is a part of the group and the group has a flair' do + user.update(groups: [group]) + expect(Guardian.new(user).can_use_flair_group?(user, group.id)).to eq(true) + end + end + describe 'can_change_trust_level?' do it 'is false without a logged in user' do