DEV: Reuse can_invite_to_forum? in can_invite_to? (#14392)
This commit resolves refactors can_invite_to? to use can_invite_to_forum? for checking the site-wide permissions and then perform topic specific checkups. Similarly, can_invite_to? is always used with a topic object and this is now enforced. There was another problem before when `must_approve_users` site setting was not checked when inviting users to forum, but was checked when inviting to a topic. Another minor security issue was that group owners could invite to group topics even if they did not have the minimum trust level to do it.
This commit is contained in:
parent
7737d56dd0
commit
76a7b75d8a
|
@ -357,33 +357,19 @@ class Guardian
|
|||
end
|
||||
|
||||
def can_invite_to_forum?(groups = nil)
|
||||
return false if !authenticated?
|
||||
|
||||
invites_available = SiteSetting.max_invites_per_day.to_i.positive?
|
||||
trust_level_requirement_met = @user.has_trust_level?(SiteSetting.min_trust_level_to_allow_invite.to_i)
|
||||
|
||||
if !is_staff?
|
||||
return false if !invites_available
|
||||
return false if !trust_level_requirement_met
|
||||
end
|
||||
|
||||
if groups.present?
|
||||
return is_admin? || groups.all? { |g| can_edit_group?(g) }
|
||||
end
|
||||
|
||||
true
|
||||
authenticated? &&
|
||||
(is_staff? || !SiteSetting.must_approve_users?) &&
|
||||
(is_staff? || SiteSetting.max_invites_per_day.to_i.positive?) &&
|
||||
(is_staff? || @user.has_trust_level?(SiteSetting.min_trust_level_to_allow_invite.to_i)) &&
|
||||
(is_admin? || groups.blank? || groups.all? { |g| can_edit_group?(g) })
|
||||
end
|
||||
|
||||
def can_invite_to?(object, groups = nil)
|
||||
return false unless authenticated?
|
||||
is_topic = object.is_a?(Topic)
|
||||
return true if is_admin? && !is_topic
|
||||
return false if SiteSetting.max_invites_per_day.to_i == 0 && !is_staff?
|
||||
return false if SiteSetting.must_approve_users? && !is_staff?
|
||||
return false unless can_see?(object)
|
||||
return false if !can_invite_to_forum?(groups)
|
||||
return false if !object.is_a?(Topic) || !can_see?(object)
|
||||
return false if groups.present?
|
||||
|
||||
if is_topic
|
||||
if object.is_a?(Topic)
|
||||
if object.private_message?
|
||||
return true if is_admin?
|
||||
return false unless SiteSetting.enable_personal_messages?
|
||||
|
@ -391,19 +377,16 @@ class Guardian
|
|||
end
|
||||
|
||||
if (category = object.category) && category.read_restricted
|
||||
if (groups = category.groups&.where(automatic: false))&.any?
|
||||
return groups.any? { |g| can_edit_group?(g) } ? true : false
|
||||
else
|
||||
return false
|
||||
end
|
||||
return category.groups&.where(automatic: false).any? { |g| can_edit_group?(g) }
|
||||
end
|
||||
end
|
||||
|
||||
user.has_trust_level?(SiteSetting.min_trust_level_to_allow_invite.to_i)
|
||||
true
|
||||
end
|
||||
|
||||
def can_invite_via_email?(object)
|
||||
return false unless can_invite_to?(object)
|
||||
return false if !can_invite_to?(object)
|
||||
|
||||
(SiteSetting.enable_local_logins || SiteSetting.enable_discourse_connect) &&
|
||||
(!SiteSetting.must_approve_users? || is_staff?)
|
||||
end
|
||||
|
|
|
@ -566,6 +566,7 @@ describe Guardian do
|
|||
end
|
||||
|
||||
it 'returns true for a group owner' do
|
||||
group_owner.update!(trust_level: SiteSetting.min_trust_level_to_allow_invite)
|
||||
expect(Guardian.new(group_owner).can_invite_to?(group_private_topic)).to be_truthy
|
||||
end
|
||||
|
||||
|
@ -595,6 +596,7 @@ describe Guardian do
|
|||
end
|
||||
|
||||
it 'should return true for a group owner' do
|
||||
group_owner.update!(trust_level: SiteSetting.min_trust_level_to_allow_invite)
|
||||
expect(Guardian.new(group_owner).can_invite_to?(topic)).to eq(true)
|
||||
end
|
||||
|
||||
|
|
|
@ -7,7 +7,7 @@ describe Topic do
|
|||
let(:now) { Time.zone.local(2013, 11, 20, 8, 0) }
|
||||
fab!(:user) { Fabricate(:user) }
|
||||
fab!(:another_user) { Fabricate(:user) }
|
||||
fab!(:trust_level_2) { Fabricate(:user, trust_level: TrustLevel[2]) }
|
||||
fab!(:trust_level_2) { Fabricate(:user, trust_level: SiteSetting.min_trust_level_to_allow_invite) }
|
||||
|
||||
context 'validations' do
|
||||
let(:topic) { Fabricate.build(:topic) }
|
||||
|
@ -899,7 +899,7 @@ describe Topic do
|
|||
end
|
||||
|
||||
describe 'when user can invite via email' do
|
||||
before { user.update!(trust_level: TrustLevel[2]) }
|
||||
before { user.update!(trust_level: SiteSetting.min_trust_level_to_allow_invite) }
|
||||
|
||||
it 'should create an invite' do
|
||||
Jobs.run_immediately!
|
||||
|
|
|
@ -3868,7 +3868,7 @@ RSpec.describe TopicsController do
|
|||
fab!(:topic) { Fabricate(:topic, user: user) }
|
||||
|
||||
it 'should return the right response' do
|
||||
user.update!(trust_level: TrustLevel[2])
|
||||
user.update!(trust_level: SiteSetting.min_trust_level_to_allow_invite)
|
||||
|
||||
expect do
|
||||
post "/t/#{topic.id}/invite.json", params: {
|
||||
|
@ -3891,6 +3891,10 @@ RSpec.describe TopicsController do
|
|||
|
||||
let!(:recipient) { 'jake@adventuretime.ooo' }
|
||||
|
||||
before do
|
||||
user.update!(trust_level: SiteSetting.min_trust_level_to_allow_invite)
|
||||
end
|
||||
|
||||
it "should attach group to the invite" do
|
||||
post "/t/#{group_private_topic.id}/invite.json", params: {
|
||||
user: recipient,
|
||||
|
|
Loading…
Reference in New Issue