Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
FEATURE: Add global admin api key rate limiter (#12527)
  • Loading branch information
davidtaylorhq committed Jun 3, 2021
1 parent 58b30fb commit 4134173
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 11 deletions.
2 changes: 1 addition & 1 deletion config/discourse_defaults.conf
Expand Up @@ -225,7 +225,7 @@ s3_install_cors_rule =
max_user_api_reqs_per_minute = 20
max_user_api_reqs_per_day = 2880

max_admin_api_reqs_per_key_per_minute = 60
max_admin_api_reqs_per_minute = 60

max_reqs_per_ip_per_minute = 200
max_reqs_per_ip_per_10_seconds = 50
Expand Down
20 changes: 14 additions & 6 deletions lib/auth/default_current_user_provider.rb
Expand Up @@ -131,7 +131,7 @@ def current_user
raise Discourse::InvalidAccess.new(I18n.t('invalid_api_credentials'), nil, custom_message: "invalid_api_credentials") unless current_user
raise Discourse::InvalidAccess if current_user.suspended? || !current_user.active
@env[API_KEY_ENV] = true
rate_limit_admin_api_requests(api_key)
rate_limit_admin_api_requests!
end

# user api key handling
Expand Down Expand Up @@ -377,15 +377,23 @@ def header_api_key?
!!@env[HEADER_API_KEY]
end

def rate_limit_admin_api_requests(api_key)
def rate_limit_admin_api_requests!
return if Rails.env == "profile"

RateLimiter.new(
limit = GlobalSetting.max_admin_api_reqs_per_minute.to_i
if GlobalSetting.respond_to?(:max_admin_api_reqs_per_key_per_minute)
Discourse.deprecate("DISCOURSE_MAX_ADMIN_API_REQS_PER_KEY_PER_MINUTE is deprecated. Please use DISCOURSE_MAX_ADMIN_API_REQS_PER_MINUTE")
limit = [ GlobalSetting.max_admin_api_reqs_per_key_per_minute.to_i, limit].max
end

global_limit = RateLimiter.new(
nil,
"admin_api_min_#{ApiKey.hash_key(api_key)}",
GlobalSetting.max_admin_api_reqs_per_key_per_minute,
"admin_api_min",
limit,
60
).performed!
)

global_limit.performed!
end

def can_write?
Expand Down
5 changes: 3 additions & 2 deletions spec/components/auth/default_current_user_provider_spec.rb
Expand Up @@ -195,10 +195,11 @@ def provider(url, opts = nil)
RateLimiter.enable
end

it "rate limits api requests per api key" do
global_setting :max_admin_api_reqs_per_key_per_minute, 3
it "rate limits admin api requests" do
global_setting :max_admin_api_reqs_per_minute, 3

freeze_time
RateLimiter.new(nil, "admin_api_min", 3, 60).clear!

api_key = ApiKey.create!(created_by_id: -1)
params = { "HTTP_API_KEY" => api_key.key, "HTTP_API_USERNAME" => user.username.downcase }
Expand Down
5 changes: 3 additions & 2 deletions spec/integration/rate_limiting_spec.rb
Expand Up @@ -49,12 +49,13 @@

it 'can cleanly limit requests and sets a Retry-After header' do
freeze_time
#request.set_header("action_dispatch.show_exceptions", true)

RateLimiter.clear_all!

admin = Fabricate(:admin)
api_key = Fabricate(:api_key, user: admin)

global_setting :max_admin_api_reqs_per_key_per_minute, 1
global_setting :max_admin_api_reqs_per_minute, 1

get '/admin/api/keys.json', headers: {
HTTP_API_KEY: api_key.key,
Expand Down

0 comments on commit 4134173

Please sign in to comment.