DEV: Don't allow users to immediately reinvite (#15722)

- Limit bulk re-invite to 1 time per day
- Move bulk invite by csv behind a site setting (hidden by default)
- Bump invite expiry from 30 -> 90 days

## Updates to rate_limiter
When limiting reinvites I found that **staff** are never limited in any way. So I updated the **rate_limiter** model to allow for a few things:
- add an optional param of `staff_limit`, which (when included and passed values, and the user passes `.staff?`) will override the default `max` & `secs` values and apply them to the user.
- in the case you **do** pass values to `staff_limit` but the user **does not** pass `staff?` the standard `max` & `secs` values will be applied to the user.

This should give us enough flexibility to 
1. continue to apply a strict rate limit to a standard user
2. but also apply a secondary (less strict) limit to staff
This commit is contained in:
janzenisaac 2022-02-03 13:07:40 -06:00 committed by GitHub
parent 69cbdb9f97
commit cffc2836cb
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 76 additions and 15 deletions

View File

@ -10,9 +10,11 @@
{{#d-section class="user-invite-buttons"}}
{{d-button class="btn-default" icon="plus" action=(action "createInvite") label="user.invited.create"}}
{{#if canBulkInvite}}
{{#unless site.mobileView}}
{{d-button class="btn-default" icon="upload" action=(action "createInviteCsv") label="user.invited.bulk_invite.text"}}
{{/unless}}
{{#if siteSettings.allow_bulk_invite}}
{{#unless site.mobileView}}
{{d-button class="btn-default" icon="upload" action=(action "createInviteCsv") label="user.invited.bulk_invite.text"}}
{{/unless}}
{{/if}}
{{/if}}
{{#if showBulkActionButtons}}
{{#if inviteExpired}}

View File

@ -361,6 +361,12 @@ class InvitesController < ApplicationController
def resend_all_invites
guardian.ensure_can_resend_all_invites!(current_user)
begin
RateLimiter.new(current_user, "bulk-reinvite-per-day", 1, 1.day, apply_limit_to_staff: true).performed!
rescue RateLimiter::LimitExceeded
return render_json_error(I18n.t("rate_limiter.slow_down"))
end
Invite.pending(current_user)
.where('invites.email IS NOT NULL')
.find_each { |invite| invite.resend_invite }

View File

@ -1465,6 +1465,7 @@ en:
watched_word_regexp_error: "The regular expression for '%{action}' watched words is invalid. Please check your <a href='%{base_path}/admin/customize/watched_words'>Watched Word settings</a>, or disable the 'watched words regular expressions' site setting."
site_settings:
allow_bulk_invite: "Allow bulk invites by uploading a CSV file"
disabled: "disabled"
display_local_time_in_user_card: "Display the local time based on a user's timezone when their user card is opened."
censored_words: "Words that will be automatically replaced with &#9632;&#9632;&#9632;&#9632;"

View File

@ -584,7 +584,7 @@ users:
client: true
default: true
invite_expiry_days:
default: 30
default: 90
client: true
max: 36500
invites_per_page:
@ -2347,6 +2347,10 @@ uncategorized:
default: false
hidden: true
allow_bulk_invite:
default: true
client: true
max_bulk_invites:
default: 50000
hidden: true

View File

@ -37,7 +37,7 @@ class RateLimiter
"#{RateLimiter.key_prefix}:#{@user && @user.id}:#{type}"
end
def initialize(user, type, max, secs, global: false, aggressive: false, error_code: nil)
def initialize(user, type, max, secs, global: false, aggressive: false, error_code: nil, apply_limit_to_staff: false, staff_limit: { max: nil, secs: nil })
@user = user
@type = type
@key = build_key(type)
@ -46,6 +46,14 @@ class RateLimiter
@global = global
@aggressive = aggressive
@error_code = error_code
@apply_limit_to_staff = apply_limit_to_staff
@staff_limit = staff_limit
# override the default values if staff user, and staff specific max is passed
if @user&.staff? && !@apply_limit_to_staff && @staff_limit[:max].present?
@max = @staff_limit[:max]
@secs = @staff_limit[:secs]
end
end
def clear!
@ -115,8 +123,7 @@ class RateLimiter
def performed!(raise_error: true)
return true if rate_unlimited?
now = Time.now.to_i
if ((max || 0) <= 0) || rate_limiter_allowed?(now)
if ((@max || 0) <= 0) || rate_limiter_allowed?(now)
raise RateLimiter::LimitExceeded.new(seconds_to_wait(now), @type, @error_code) if raise_error
false
else
@ -153,7 +160,6 @@ class RateLimiter
private
def rate_limiter_allowed?(now)
lua, lua_sha = nil
if @aggressive
lua = PERFORM_LUA_AGGRESSIVE
@ -193,7 +199,7 @@ class RateLimiter
end
def rate_unlimited?
!!(RateLimiter.disabled? || (@user && @user.staff?))
!!(RateLimiter.disabled? || (@user&.staff? && !@apply_limit_to_staff && @staff_limit[:max].nil?))
end
def eval_lua(lua, sha, keys, args)

View File

@ -6,7 +6,11 @@ require 'rate_limiter'
describe RateLimiter do
fab!(:user) { Fabricate(:user) }
fab!(:admin) { Fabricate(:admin) }
let(:rate_limiter) { RateLimiter.new(user, "peppermint-butler", 2, 60) }
let(:apply_staff_rate_limiter) { RateLimiter.new(admin, "peppermint-servant", 5, 40, apply_limit_to_staff: true) }
let(:staff_rate_limiter) { RateLimiter.new(user, "peppermind-servant", 5, 40, staff_limit: { max: 10, secs: 80 }) }
let(:admin_staff_rate_limiter) { RateLimiter.new(admin, "peppermind-servant", 5, 40, staff_limit: { max: 10, secs: 80 }) }
context 'disabled' do
before do
@ -32,6 +36,8 @@ describe RateLimiter do
before do
RateLimiter.enable
rate_limiter.clear!
staff_rate_limiter.clear!
admin_staff_rate_limiter.clear!
end
context 'aggressive rate limiter' do
@ -46,7 +52,6 @@ describe RateLimiter do
limiter.performed!
limiter.performed!
freeze_time 29.seconds.from_now
expect do
@ -189,6 +194,34 @@ describe RateLimiter do
user.moderator = true
expect { rate_limiter.performed! }.not_to raise_error
end
it "applies max / secs to staff when apply_limit_to_staff flag is true" do
5.times { apply_staff_rate_limiter.performed! }
freeze_time 10.seconds.from_now
expect { apply_staff_rate_limiter.performed! }.to raise_error do |error|
expect(error).to be_a(RateLimiter::LimitExceeded)
expect(error).to having_attributes(available_in: 30)
end
end
it "applies staff_limit max when present for staff" do
expect(admin_staff_rate_limiter.can_perform?).to eq(true)
expect(admin_staff_rate_limiter.remaining).to eq(10)
end
it "applies staff_limit secs when present for staff" do
10.times { admin_staff_rate_limiter.performed! }
freeze_time 10.seconds.from_now
expect { admin_staff_rate_limiter.performed! }.to raise_error do |error|
expect(error).to be_a(RateLimiter::LimitExceeded)
expect(error).to having_attributes(available_in: 70)
end
end
it "applies standard max to non-staff users when staff_limit values are present" do
expect(staff_rate_limiter.can_perform?).to eq(true)
expect(staff_rate_limiter.remaining).to eq(5)
end
end
context "rollback!" do

View File

@ -907,15 +907,15 @@ describe InvitesController do
freeze_time
user = Fabricate(:admin)
new_invite = Fabricate(:invite, invited_by: user)
expired_invite = Fabricate(:invite, invited_by: user)
admin = Fabricate(:admin)
new_invite = Fabricate(:invite, invited_by: admin)
expired_invite = Fabricate(:invite, invited_by: admin)
expired_invite.update!(expires_at: 2.days.ago)
redeemed_invite = Fabricate(:invite, invited_by: user)
redeemed_invite = Fabricate(:invite, invited_by: admin)
Fabricate(:invited_user, invite: redeemed_invite, user: Fabricate(:user))
redeemed_invite.update!(expires_at: 5.days.ago)
sign_in(user)
sign_in(admin)
post '/invites/reinvite-all'
expect(response.status).to eq(200)
@ -953,6 +953,15 @@ describe InvitesController do
expect(Jobs::BulkInvite.jobs.size).to eq(1)
end
it 'limits admins when bulk inviting' do
sign_in(admin)
post '/invites/upload_csv.json', params: { file: file, name: 'discourse.csv' }
expect(response.status).to eq(200)
post '/invites/upload_csv.json', params: { file: file, name: 'discourse.csv' }
expect(response.status).to eq(422)
expect(Jobs::BulkInvite.jobs.size).to eq(1)
end
it 'allows admin to bulk invite when DiscourseConnect enabled' do
SiteSetting.discourse_connect_url = "https://example.com"
SiteSetting.enable_discourse_connect = true