Discource-C
/
discourse
mirror of https://github.com/discourse/discourse.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

FIX: Remove public topic invite functionality (#18488) 

This can no longer be used from the user interface and could be used to
generate useless topic invites notifications. This commit adds site 
setting max_topic_invitations_per_minute to prevent invite spam.
This commit is contained in:
Bianca Nenciu 2022-10-10 19:21:51 +03:00 committed by GitHub
parent b06cc8a836
commit 08ab09c928
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 90 additions and 125 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

29
app/controllers/topics_controller.rb
View File

@ -687,11 +687,12 @@ class TopicsController < ApplicationController
def invite_group
group = Group.find_by(name: params[:group])
raise Discourse::NotFound unless group
raise Discourse::NotFound if !group
topic = Topic.find_by(id: params[:topic_id])
raise Discourse::NotFound if !topic
unless pm_has_slots?(topic)
if !pm_has_slots?(topic)
return render_json_error(
I18n.t("pm_reached_recipients_limit", recipients_limit: SiteSetting.max_allowed_message_recipients)
)
@ -708,29 +709,29 @@ class TopicsController < ApplicationController
def invite
topic = Topic.find_by(id: params[:topic_id])
raise Discourse::InvalidParameters.new unless topic
raise Discourse::NotFound if !topic
username_or_email = params[:user] ? fetch_username : fetch_email
if !topic.private_message?
return render_json_error(I18n.t("topic_invite.not_pm"))
end
groups = Group.lookup_groups(
group_ids: params[:group_ids],
group_names: params[:group_names]
)
unless pm_has_slots?(topic)
if !pm_has_slots?(topic)
return render_json_error(
I18n.t("pm_reached_recipients_limit", recipients_limit: SiteSetting.max_allowed_message_recipients)
)
end
guardian.ensure_can_invite_to!(topic)
group_ids = groups.map(&:id)
username_or_email = params[:user] ? fetch_username : fetch_email
group_ids = Group.lookup_groups(
group_ids: params[:group_ids],
group_names: params[:group_names]
).pluck(:id)
begin
if topic.invite(current_user, username_or_email, group_ids, params[:custom_message])
user = User.find_by_username_or_email(username_or_email)
if user
if user = User.find_by_username_or_email(username_or_email)
render_json_dump BasicUserSerializer.new(user, scope: guardian, root: 'user')
else
render json: success_json

16
app/models/topic.rb
View File

@ -1029,11 +1029,9 @@ class Topic < ActiveRecord::Base
end
def invite(invited_by, username_or_email, group_ids = nil, custom_message = nil)
target_user = User.find_by_username_or_email(username_or_email)
guardian = Guardian.new(invited_by)
is_email = username_or_email =~ /^.+@.+$/
if target_user
if target_user = User.find_by_username_or_email(username_or_email)
if topic_allowed_users.exists?(user_id: target_user.id)
raise UserExists.new(I18n.t("topic_invite.user_exists"))
end
@ -1064,7 +1062,7 @@ class Topic < ActiveRecord::Base
else
!!invite_to_topic(invited_by, target_user, group_ids, guardian)
end
elsif is_email && guardian.can_invite_via_email?(self)
elsif username_or_email =~ /^.+@.+$/ && guardian.can_invite_via_email?(self)
!!Invite.generate(invited_by,
email: username_or_email,
topic: self,
@ -1781,6 +1779,13 @@ class Topic < ActiveRecord::Base
SiteSetting.max_topic_invitations_per_day,
1.day.to_i
).performed!
RateLimiter.new(
invited_by,
"topic-invitations-per-minute",
SiteSetting.max_topic_invitations_per_minute,
1.day.to_i
).performed!
end
def cannot_permanently_delete_reason(user)
@ -1853,8 +1858,9 @@ class Topic < ActiveRecord::Base
))
end
rate_limit_topic_invitation(invited_by)
Topic.transaction do
rate_limit_topic_invitation(invited_by)
topic_allowed_users.create!(user_id: target_user.id) unless topic_allowed_users.exists?(user_id: target_user.id)
user_in_allowed_group = (user.group_ids & topic_allowed_groups.map(&:group_id)).present?

2
config/locales/server.en.yml
View File

@ -279,6 +279,7 @@ en:
topic_invite:
failed_to_invite: "The user cannot be invited into this topic without a group membership in either one of the following groups: %{group_names}."
not_pm: "You can only invite to private messages."
user_exists: "Sorry, that user has already been invited. You may only invite a user to a topic once."
muted_topic: "Sorry, that user muted this topic."
receiver_does_not_allow_pm: "Sorry, that user does not allow you to send them private messages."
@ -1791,6 +1792,7 @@ en:
max_personal_messages_per_day: "Maximum number of new personal message topics a user can create per day."
max_invites_per_day: "Maximum number of invites a user can send per day."
max_topic_invitations_per_day: "Maximum number of topic invitations a user can send per day."
max_topic_invitations_per_minute: "Maximum number of topic invitations a user can send per minute."
max_logins_per_ip_per_hour: "Maximum number of logins allowed per IP address per hour"
max_logins_per_ip_per_minute: "Maximum number of logins allowed per IP address per minute"

1
config/site_settings.yml
View File

@ -1893,6 +1893,7 @@ rate_limits:
max_edits_per_day: 30
max_invites_per_day: 10
max_topic_invitations_per_day: 30
max_topic_invitations_per_minute: 5
max_topics_in_first_day: 3
max_replies_in_first_day: 10
tl2_additional_likes_per_day_multiplier: 1.5

71
spec/models/topic_spec.rb
View File

@ -723,7 +723,6 @@ RSpec.describe Topic do
context 'with rate limits' do
before do
SiteSetting.max_topic_invitations_per_day = 1
RateLimiter.enable
Group.refresh_automatic_groups!
end
@ -732,30 +731,68 @@ RSpec.describe Topic do
RateLimiter.clear_all!
end
it "rate limits topic invitations" do
start = Time.now.tomorrow.beginning_of_day
freeze_time(start)
context 'when per day' do
before do
SiteSetting.max_topic_invitations_per_day = 1
end
topic = Fabricate(:topic, user: trust_level_2)
it "rate limits topic invitations" do
start = Time.now.tomorrow.beginning_of_day
freeze_time(start)
topic.invite(topic.user, user.username)
topic = Fabricate(:topic, user: trust_level_2)
expect {
topic.invite(topic.user, user1.username)
}.to raise_error(RateLimiter::LimitExceeded)
topic.invite(topic.user, user.username)
expect {
topic.invite(topic.user, user1.username)
}.to raise_error(RateLimiter::LimitExceeded)
end
it "rate limits PM invitations" do
start = Time.now.tomorrow.beginning_of_day
freeze_time(start)
topic = Fabricate(:private_message_topic, user: trust_level_2)
topic.invite(topic.user, user.username)
expect {
topic.invite(topic.user, user1.username)
}.to raise_error(RateLimiter::LimitExceeded)
end
end
it "rate limits PM invitations" do
start = Time.now.tomorrow.beginning_of_day
freeze_time(start)
context 'when per minute' do
before do
SiteSetting.max_topic_invitations_per_minute = 1
end
topic = Fabricate(:private_message_topic, user: trust_level_2)
it "rate limits topic invitations" do
start = Time.now.tomorrow.beginning_of_minute
freeze_time(start)
topic.invite(topic.user, user.username)
topic = Fabricate(:topic, user: trust_level_2)
expect {
topic.invite(topic.user, user1.username)
}.to raise_error(RateLimiter::LimitExceeded)
topic.invite(topic.user, user.username)
expect {
topic.invite(topic.user, user1.username)
}.to raise_error(RateLimiter::LimitExceeded)
end
it "rate limits PM invitations" do
start = Time.now.tomorrow.beginning_of_minute
freeze_time(start)
topic = Fabricate(:private_message_topic, user: trust_level_2)
topic.invite(topic.user, user.username)
expect {
topic.invite(topic.user, user1.username)
}.to raise_error(RateLimiter::LimitExceeded)
end
end
end

2
spec/requests/api/topics_spec.rb
View File

@ -229,7 +229,7 @@ RSpec.describe 'topics' do
let(:username) { Fabricate(:user).username }
let(:request_body) { { user: username } }
let(:id) { Fabricate(:topic).id }
let(:id) { Fabricate(:private_message_topic).id }
run_test! do |response|
data = JSON.parse(response.body)

94
spec/requests/topics_controller_spec.rb
View File

@ -4047,77 +4047,17 @@ RSpec.describe TopicsController do
sign_in(user)
end
context 'as a valid user' do
context 'when topic id is not PM' do
fab!(:user_topic) { Fabricate(:topic, user: user) }
it 'should return the right response' do
user.update!(trust_level: SiteSetting.min_trust_level_to_allow_invite)
expect do
post "/t/#{user_topic.id}/invite.json", params: {
email: 'someguy@email.com'
}
end.to change { Invite.where(invited_by_id: user.id).count }.by(1)
expect(response.status).to eq(200)
expect(Jobs::InviteEmail.jobs.first['args'].first['invite_to_topic']).to be_truthy
end
end
context 'when user is a group manager' do
fab!(:group) { Fabricate(:group).tap { |g| g.add_owner(user) } }
fab!(:private_category) { Fabricate(:private_category, group: group) }
fab!(:group_private_topic) do
Fabricate(:topic, category: private_category, user: user)
end
let!(:recipient) { 'jake@adventuretime.ooo' }
before_all 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,
group_ids: "#{group.id},9999999"
post "/t/#{user_topic.id}/invite.json", params: {
email: 'someguy@email.com'
}
expect(response.status).to eq(200)
expect(Invite.find_by(email: recipient).groups).to contain_exactly(group)
end
context 'when group is available to automatic groups only' do
before do
group.update!(automatic: true)
end
it 'should return the right response' do
post "/t/#{group_private_topic.id}/invite.json", params: {
user: user
}
expect(response.status).to eq(403)
end
end
context 'when user is not part of the required group' do
it 'should return the right response' do
post "/t/#{group_private_topic.id}/invite.json", params: {
user: user
}
expect(response.status).to eq(422)
response_body = response.parsed_body
expect(response_body["errors"]).to eq([
I18n.t("topic_invite.failed_to_invite",
group_names: group.name
)
])
end
expect(response.status).to eq(422)
end
end
@ -4129,13 +4069,13 @@ RSpec.describe TopicsController do
email: user.email
}
expect(response.status).to eq(400)
expect(response.status).to eq(404)
end
end
it 'requires an email parameter' do
post "/t/#{topic.id}/invite.json"
expect(response.status).to eq(400)
expect(response.status).to eq(422)
end
context "when PM has reached maximum allowed numbers of recipients" do
@ -4180,28 +4120,6 @@ RSpec.describe TopicsController do
end
end
end
context "when inviting a group to a topic" do
fab!(:group) { Fabricate(:group) }
before do
sign_in(admin)
end
it "should work correctly" do
email = 'hiro@from.heros'
post "/t/#{topic.id}/invite.json", params: {
email: email, group_ids: group.id
}
expect(response.status).to eq(200)
groups = Invite.find_by(email: email).groups
expect(groups.count).to eq(1)
expect(groups.first.id).to eq(group.id)
end
end
end
describe 'invite_group' do