diff --git a/lib/auth/default_current_user_provider.rb b/lib/auth/default_current_user_provider.rb index 3ff739896fd..0800286ea9d 100644 --- a/lib/auth/default_current_user_provider.rb +++ b/lib/auth/default_current_user_provider.rb @@ -165,7 +165,15 @@ class Auth::DefaultCurrentUserProvider ) end raise Discourse::InvalidAccess if current_user.suspended? || !current_user.active - admin_api_key_limiter.performed! if !Rails.env.profile? + + if !Rails.env.profile? + admin_api_key_limiter.performed! + + # Don't enforce the default per ip limits for authenticated admin api + # requests + (@env["DISCOURSE_RATE_LIMITERS"] || []).each(&:rollback!) + end + @env[API_KEY_ENV] = true end diff --git a/spec/requests/api/rate_limits_spec.rb b/spec/requests/api/rate_limits_spec.rb new file mode 100644 index 00000000000..d91d47472b5 --- /dev/null +++ b/spec/requests/api/rate_limits_spec.rb @@ -0,0 +1,155 @@ +# frozen_string_literal: true + +class MockRateLimiter + LimitExceeded = RateLimiter::LimitExceeded + + def self.disable + end + + def self.enable + end + + def self.performed!(*args, **kwargs) + end + + def self.rollback!(*args, **kwargs) + end + + def initialize(*args, **kwargs) + @args = args + @kwargs = kwargs + end + + def can_perform? + true + end + + def performed! + MockRateLimiter.performed!(*@args, **@kwargs) + end + + def rollback! + MockRateLimiter.rollback!(*@args, **@kwargs) + end +end + +RSpec.describe "rate limits" do + before_all { @key = Fabricate(:api_key).key } + + let(:api_key) { @key } + let!(:api_username) { "system" } + + use_redis_snapshotting + + around(:each) { |example| stub_const(Object, :RateLimiter, MockRateLimiter) { example.run } } + + it "doesn't rate limit authenticated admin api requests" do + MockRateLimiter + .expects(:performed!) + .with( + nil, + "global_limit_60_192.0.2.1", + 200, + 60, + global: true, + error_code: "ip_60_secs_limit", + aggressive: true, + ) + .once + MockRateLimiter + .expects(:performed!) + .with( + nil, + "global_limit_10_192.0.2.1", + 50, + 10, + global: true, + error_code: "ip_10_secs_limit", + aggressive: true, + ) + .once + + MockRateLimiter + .expects(:performed!) + .with(nil, "admin_api_min", 60, 60, error_code: "admin_api_key_rate_limit") + .once + + MockRateLimiter + .expects(:rollback!) + .with( + nil, + "global_limit_60_192.0.2.1", + 200, + 60, + global: true, + error_code: "ip_60_secs_limit", + aggressive: true, + ) + .once + MockRateLimiter + .expects(:rollback!) + .with( + nil, + "global_limit_10_192.0.2.1", + 50, + 10, + global: true, + error_code: "ip_10_secs_limit", + aggressive: true, + ) + .once + + get( + "/admin/backups.json", + headers: { + "Api-Key" => api_key, + "Api-Username" => api_username, + }, + env: { + REMOTE_ADDR: "192.0.2.1", + }, + ) + + expect(response.status).to eq(200) + end + + it "doesn't rollback rate limits for unauthenticated admin api requests" do + MockRateLimiter + .expects(:performed!) + .with( + nil, + "global_limit_60_192.0.2.1", + 200, + 60, + global: true, + error_code: "ip_60_secs_limit", + aggressive: true, + ) + .once + MockRateLimiter + .expects(:performed!) + .with( + nil, + "global_limit_10_192.0.2.1", + 50, + 10, + global: true, + error_code: "ip_10_secs_limit", + aggressive: true, + ) + .once + + get( + "/admin/backups.json", + headers: { + "Api-Key" => "bogus key", + "Api-Username" => api_username, + }, + env: { + REMOTE_ADDR: "192.0.2.1", + }, + ) + + expect(response.status).to eq(404) + end +end