diff --git a/lib/cloud_controller/config_schemas/base/api_schema.rb b/lib/cloud_controller/config_schemas/base/api_schema.rb index ab882134df2..6438bc707ae 100644 --- a/lib/cloud_controller/config_schemas/base/api_schema.rb +++ b/lib/cloud_controller/config_schemas/base/api_schema.rb @@ -295,7 +295,7 @@ class ApiSchema < VCAP::Config max_concurrent_service_broker_requests: Integer, shared_isolation_segment_name: String, - rate_limiter_v2_api: { + optional(:rate_limiter_v2_api) => { enabled: bool, per_process_general_limit: Integer, global_general_limit: Integer, diff --git a/middleware/base_rate_limiter.rb b/middleware/base_rate_limiter.rb index 6a85911566d..a5a906215cf 100644 --- a/middleware/base_rate_limiter.rb +++ b/middleware/base_rate_limiter.rb @@ -1,16 +1,14 @@ +require 'mixins/client_ip' +require 'mixins/user_reset_interval' + module CloudFoundry module Middleware - RequestCount = Struct.new(:requests, :valid_until) - - class BaseRequestCounter + class RequestCounter include CloudFoundry::Middleware::UserResetInterval - def initialize - reset - end + RequestCount = Struct.new(:requests, :valid_until) - # needed for testing - def reset + def initialize @mutex = Mutex.new @data = {} end @@ -58,7 +56,7 @@ def initialize(suffix) @remaining = nil end - def as_hash + def to_hash return {} if [@limit, @reset, @remaining].all?(&:nil?) { "#{@prefix}-Limit#{@suffix}" => @limit, "#{@prefix}-Reset#{@suffix}" => @reset, "#{@prefix}-Remaining#{@suffix}" => @remaining } @@ -81,7 +79,7 @@ def call(env) request = ActionDispatch::Request.new(env) - unless skip_rate_limiting?(env, request) + if apply_rate_limiting?(env) user_guid = user_token?(env) ? env['cf.user_guid'] : client_ip(request) count, valid_until = @request_counter.get(user_guid, @reset_interval, @logger) @@ -97,21 +95,25 @@ def call(env) end status, headers, body = @app.call(env) - [status, headers.merge(rate_limit_headers.as_hash), body] + [status, headers.merge(rate_limit_headers.to_hash), body] end private - def skip_rate_limiting?(env, request) - raise NotImplementedError + def apply_rate_limiting?(env) + raise 'method should be implemented in concrete class' end def global_request_limit(env) - raise NotImplementedError + raise 'method should be implemented in concrete class' end def rate_limit_error(env) - raise NotImplementedError + raise 'method should be implemented in concrete class' + end + + def per_process_request_limit(env) + raise 'method should be implemented in concrete class' end def exceeded_rate_limit(count, env) @@ -131,8 +133,9 @@ def user_token?(env) !!env['cf.user_guid'] end - def basic_auth?(auth) - (auth.provided? && auth.basic?) + def basic_auth?(env) + auth = Rack::Auth::Basic::Request.new(env) + auth.provided? && auth.basic? end def admin? @@ -145,7 +148,7 @@ def too_many_requests!(env, rate_limit_headers) headers['Content-Type'] = 'text/plain; charset=utf-8' message = rate_limit_error(env).to_json headers['Content-Length'] = message.length.to_s - [429, rate_limit_headers.as_hash.merge(headers), [message]] + [429, rate_limit_headers.to_hash.merge(headers), [message]] end end end diff --git a/middleware/rate_limiter.rb b/middleware/rate_limiter.rb index faa3f72ec6a..d2a8ef4ce8e 100644 --- a/middleware/rate_limiter.rb +++ b/middleware/rate_limiter.rb @@ -1,12 +1,10 @@ -require 'mixins/client_ip' -require 'mixins/user_reset_interval' require 'base_rate_limiter' module CloudFoundry module Middleware - REQUEST_COUNTER = BaseRequestCounter.new - class RateLimiter < BaseRateLimiter + REQUEST_COUNTER = RequestCounter.new + def initialize(app, opts) @per_process_general_limit = opts[:per_process_general_limit] @global_general_limit = opts[:global_general_limit] @@ -17,9 +15,9 @@ def initialize(app, opts) private - def skip_rate_limiting?(env, request) - auth = Rack::Auth::Basic::Request.new(env) - basic_auth?(auth) || internal_api?(request) || root_api?(request) || admin? + def apply_rate_limiting?(env) + request = ActionDispatch::Request.new(env) + !basic_auth?(env) && !internal_api?(request) && !root_api?(request) && !admin? end def root_api?(request) diff --git a/middleware/rate_limiter_v2_api.rb b/middleware/rate_limiter_v2_api.rb index 1860c970ecd..c67db31349c 100644 --- a/middleware/rate_limiter_v2_api.rb +++ b/middleware/rate_limiter_v2_api.rb @@ -1,25 +1,27 @@ -require 'mixins/client_ip' -require 'mixins/user_reset_interval' require 'base_rate_limiter' module CloudFoundry module Middleware - REQUEST_COUNTER_V2_API = BaseRequestCounter.new - class RateLimiterV2API < BaseRateLimiter + REQUEST_COUNTER = RequestCounter.new + def initialize(app, opts) @per_process_general_limit = opts[:per_process_general_limit] @global_general_limit = opts[:global_general_limit] @per_process_admin_limit = opts[:per_process_admin_limit] @global_admin_limit = opts[:global_admin_limit] - super(app, opts[:logger], REQUEST_COUNTER_V2_API, opts[:interval], 'V2-API') + super(app, opts[:logger], REQUEST_COUNTER, opts[:interval], 'V2-API') end private - def skip_rate_limiting?(env, request) - auth = Rack::Auth::Basic::Request.new(env) - basic_auth?(auth) || !request.fullpath.match(%r{\A/v2/(?!(info)).+}) + def apply_rate_limiting?(env) + !basic_auth?(env) && v2_api?(env) + end + + def v2_api?(env) + request = ActionDispatch::Request.new(env) + request.fullpath.match(%r{\A/v2/(?!(info)).+}) end def global_request_limit(env) @@ -31,8 +33,7 @@ def per_process_request_limit(env) end def rate_limit_error(env) - error_name = 'RateLimitV2APIExceeded' - api_error = CloudController::Errors::ApiError.new_from_details(error_name) + api_error = CloudController::Errors::ApiError.new_from_details('RateLimitV2APIExceeded') ErrorPresenter.new(api_error, Rails.env.test?, V2ErrorHasher.new(api_error)).to_hash end end diff --git a/spec/support/shared_examples/middleware/rate_limiting.rb b/spec/support/shared_examples/middleware/rate_limiting.rb new file mode 100644 index 00000000000..ee49ab8db62 --- /dev/null +++ b/spec/support/shared_examples/middleware/rate_limiting.rb @@ -0,0 +1,11 @@ +RSpec.shared_examples_for 'endpoint exempts from rate limiting' do |header_suffix=''| + it 'exempts them from rate limiting' do + allow(ActionDispatch::Request).to receive(:new).and_return(fake_request) + _, response_headers, _ = middleware.call(env) + expect(request_counter).not_to have_received(:get) + expect(request_counter).not_to have_received(:increment) + expect(response_headers["X-RateLimit-Limit#{header_suffix}"]).to be_nil + expect(response_headers["X-RateLimit-Remaining#{header_suffix}"]).to be_nil + expect(response_headers["X-RateLimit-Reset#{header_suffix}"]).to be_nil + end +end diff --git a/spec/unit/middleware/rate_limiter_spec.rb b/spec/unit/middleware/rate_limiter_spec.rb index 1da3a866d93..15c68d5c81f 100644 --- a/spec/unit/middleware/rate_limiter_spec.rb +++ b/spec/unit/middleware/rate_limiter_spec.rb @@ -16,12 +16,11 @@ module Middleware } ) end - let(:REQUEST_COUNTER) { double } + let(:request_counter) { double } before(:each) { - middleware.instance_variable_set('@REQUEST_COUNTER', REQUEST_COUNTER) - REQUEST_COUNTER.reset - allow(REQUEST_COUNTER).to receive(:get).and_return([0, Time.now.utc]) - allow(REQUEST_COUNTER).to receive(:increment) + middleware.instance_variable_set('@request_counter', request_counter) + allow(request_counter).to receive(:get).and_return([0, Time.now.utc]) + allow(request_counter).to receive(:increment) } let(:app) { double(:app, call: [200, {}, 'a body']) } @@ -52,18 +51,18 @@ module Middleware describe 'X-RateLimit-Remaining' do it 'shows the user the number of remaining requests rounded down to nearest 10%' do - allow(REQUEST_COUNTER).to receive(:get).and_return([0, Time.now.utc]) + allow(request_counter).to receive(:get).and_return([0, Time.now.utc]) _, response_headers, _ = middleware.call(user_1_env) expect(response_headers['X-RateLimit-Remaining']).to eq('900') - allow(REQUEST_COUNTER).to receive(:get).and_return([10, Time.now.utc]) + allow(request_counter).to receive(:get).and_return([10, Time.now.utc]) _, response_headers, _ = middleware.call(user_1_env) expect(response_headers['X-RateLimit-Remaining']).to eq('800') end - it 'tracks user\'s remaining requests independently' do - expect(REQUEST_COUNTER).to receive(:get).with(user_1_guid, interval, logger).and_return([0, Time.now.utc]) - expect(REQUEST_COUNTER).to receive(:get).with(user_2_guid, interval, logger).and_return([10, Time.now.utc]) + it "tracks user's remaining requests independently" do + expect(request_counter).to receive(:get).with(user_1_guid, interval, logger).and_return([0, Time.now.utc]) + expect(request_counter).to receive(:get).with(user_2_guid, interval, logger).and_return([10, Time.now.utc]) _, response_headers, _ = middleware.call(user_1_env) expect(response_headers['X-RateLimit-Remaining']).to eq('900') @@ -76,7 +75,7 @@ module Middleware describe 'X-RateLimit-Reset' do it 'shows the user when the interval will expire' do valid_until = Time.now.utc.beginning_of_hour + interval.minutes - allow(REQUEST_COUNTER).to receive(:get).and_return([0, valid_until]) + allow(request_counter).to receive(:get).and_return([0, valid_until]) _, response_headers, _ = middleware.call(user_1_env) expect(response_headers['X-RateLimit-Reset'].to_i).to eq(valid_until.utc.to_i) end @@ -85,7 +84,7 @@ module Middleware it 'increments the counter and allows the request to continue' do _, _, _ = middleware.call(user_1_env) - expect(REQUEST_COUNTER).to have_received(:increment).with(user_1_guid) + expect(request_counter).to have_received(:increment).with(user_1_guid) expect(app).to have_received(:call) end @@ -99,8 +98,8 @@ module Middleware describe 'when the user has basic auth credentials' do it 'exempts them from rate limiting' do _, response_headers, _ = middleware.call(basic_auth_env) - expect(REQUEST_COUNTER).not_to have_received(:get) - expect(REQUEST_COUNTER).not_to have_received(:increment) + expect(request_counter).not_to have_received(:get) + expect(request_counter).not_to have_received(:increment) expect(response_headers['X-RateLimit-Limit']).to be_nil expect(response_headers['X-RateLimit-Remaining']).to be_nil expect(response_headers['X-RateLimit-Reset']).to be_nil @@ -111,14 +110,8 @@ module Middleware context 'when the user is hitting a path starting with "/internal"' do let(:fake_request) { instance_double(ActionDispatch::Request, fullpath: '/internal/pants/1234') } - it 'exempts them from rate limiting' do - allow(ActionDispatch::Request).to receive(:new).and_return(fake_request) - _, response_headers, _ = middleware.call(unauthenticated_env) - expect(REQUEST_COUNTER).not_to have_received(:get) - expect(REQUEST_COUNTER).not_to have_received(:increment) - expect(response_headers['X-RateLimit-Limit']).to be_nil - expect(response_headers['X-RateLimit-Remaining']).to be_nil - expect(response_headers['X-RateLimit-Reset']).to be_nil + it_behaves_like 'endpoint exempts from rate limiting' do + let(:env) { unauthenticated_env } end end @@ -128,8 +121,8 @@ module Middleware it 'rate limits them' do allow(ActionDispatch::Request).to receive(:new).and_return(fake_request) - expect(REQUEST_COUNTER).to receive(:get).with('forwarded_ip', interval, logger).and_return([0, Time.now.utc]) - expect(REQUEST_COUNTER).to receive(:increment).with('forwarded_ip') + expect(request_counter).to receive(:get).with('forwarded_ip', interval, logger).and_return([0, Time.now.utc]) + expect(request_counter).to receive(:increment).with('forwarded_ip') _, response_headers, _ = middleware.call(unauthenticated_env) expect(response_headers['X-RateLimit-Limit']).to_not be_nil expect(response_headers['X-RateLimit-Remaining']).to_not be_nil @@ -142,56 +135,32 @@ module Middleware context 'when the user is hitting a root path /' do let(:fake_request) { instance_double(ActionDispatch::Request, fullpath: '/') } - it 'exempts them from rate limiting' do - allow(ActionDispatch::Request).to receive(:new).and_return(fake_request) - _, response_headers, _ = middleware.call(unauthenticated_env) - expect(REQUEST_COUNTER).not_to have_received(:get) - expect(REQUEST_COUNTER).not_to have_received(:increment) - expect(response_headers['X-RateLimit-Limit']).to be_nil - expect(response_headers['X-RateLimit-Remaining']).to be_nil - expect(response_headers['X-RateLimit-Reset']).to be_nil + it_behaves_like 'endpoint exempts from rate limiting' do + let(:env) { unauthenticated_env } end end context 'when the user is hitting a root path /v2/info' do let(:fake_request) { instance_double(ActionDispatch::Request, fullpath: '/v2/info') } - it 'exempts them from rate limiting' do - allow(ActionDispatch::Request).to receive(:new).and_return(fake_request) - _, response_headers, _ = middleware.call(unauthenticated_env) - expect(REQUEST_COUNTER).not_to have_received(:get) - expect(REQUEST_COUNTER).not_to have_received(:increment) - expect(response_headers['X-RateLimit-Limit']).to be_nil - expect(response_headers['X-RateLimit-Remaining']).to be_nil - expect(response_headers['X-RateLimit-Reset']).to be_nil + it_behaves_like 'endpoint exempts from rate limiting' do + let(:env) { unauthenticated_env } end end context 'when the user is hitting a root path /v3' do let(:fake_request) { instance_double(ActionDispatch::Request, fullpath: '/v3') } - it 'exempts them from rate limiting' do - allow(ActionDispatch::Request).to receive(:new).and_return(fake_request) - _, response_headers, _ = middleware.call(unauthenticated_env) - expect(REQUEST_COUNTER).not_to have_received(:get) - expect(REQUEST_COUNTER).not_to have_received(:increment) - expect(response_headers['X-RateLimit-Limit']).to be_nil - expect(response_headers['X-RateLimit-Remaining']).to be_nil - expect(response_headers['X-RateLimit-Reset']).to be_nil + it_behaves_like 'endpoint exempts from rate limiting' do + let(:env) { unauthenticated_env } end end context 'when the user is hitting a root path /healthz' do let(:fake_request) { instance_double(ActionDispatch::Request, fullpath: '/healthz') } - it 'exempts them from rate limiting' do - allow(ActionDispatch::Request).to receive(:new).and_return(fake_request) - _, response_headers, _ = middleware.call(unauthenticated_env) - expect(REQUEST_COUNTER).not_to have_received(:get) - expect(REQUEST_COUNTER).not_to have_received(:increment) - expect(response_headers['X-RateLimit-Limit']).to be_nil - expect(response_headers['X-RateLimit-Remaining']).to be_nil - expect(response_headers['X-RateLimit-Reset']).to be_nil + it_behaves_like 'endpoint exempts from rate limiting' do + let(:env) { unauthenticated_env } end end end @@ -220,15 +189,15 @@ module Middleware valid_until_2 = Time.now.utc allow(ActionDispatch::Request).to receive(:new).and_return(fake_request) - expect(REQUEST_COUNTER).to receive(:get).with(forwarded_ip, interval, logger).and_return([0, valid_until]) - expect(REQUEST_COUNTER).to receive(:increment).with(forwarded_ip) + expect(request_counter).to receive(:get).with(forwarded_ip, interval, logger).and_return([0, valid_until]) + expect(request_counter).to receive(:increment).with(forwarded_ip) _, response_headers, _ = middleware.call(unauthenticated_env) expect(response_headers['X-RateLimit-Remaining']).to eq('90') expect(response_headers['X-RateLimit-Reset']).to eq(valid_until.to_i.to_s) allow(ActionDispatch::Request).to receive(:new).and_return(fake_request_2) - expect(REQUEST_COUNTER).to receive(:get).with(forwarded_ip_2, interval, logger).and_return([2, valid_until_2]) - expect(REQUEST_COUNTER).to receive(:increment).with(forwarded_ip_2) + expect(request_counter).to receive(:get).with(forwarded_ip_2, interval, logger).and_return([2, valid_until_2]) + expect(request_counter).to receive(:increment).with(forwarded_ip_2) _, response_headers, _ = middleware.call(unauthenticated_env) expect(response_headers['X-RateLimit-Remaining']).to eq('70') expect(response_headers['X-RateLimit-Reset']).to eq(valid_until_2.to_i.to_s) @@ -251,18 +220,18 @@ module Middleware it 'identifies them by the request ip' do valid_until = Time.now.utc.beginning_of_hour valid_until_2 = Time.now.utc.beginning_of_hour + 5.minutes - allow(REQUEST_COUNTER).to receive(:get).with(ip, interval, logger).and_return([0, valid_until]) - allow(REQUEST_COUNTER).to receive(:get).with(ip_2, interval, logger).and_return([2, valid_until_2]) + allow(request_counter).to receive(:get).with(ip, interval, logger).and_return([0, valid_until]) + allow(request_counter).to receive(:get).with(ip_2, interval, logger).and_return([2, valid_until_2]) allow(ActionDispatch::Request).to receive(:new).and_return(fake_request) _, response_headers, _ = middleware.call(unauthenticated_env) - expect(REQUEST_COUNTER).to have_received(:increment).with(ip) + expect(request_counter).to have_received(:increment).with(ip) expect(response_headers['X-RateLimit-Remaining']).to eq('90') expect(response_headers['X-RateLimit-Reset']).to eq(valid_until.utc.to_i.to_s) allow(ActionDispatch::Request).to receive(:new).and_return(fake_request_2) _, response_headers, _ = middleware.call(unauthenticated_env) - expect(REQUEST_COUNTER).to have_received(:increment).with(ip_2) + expect(request_counter).to have_received(:increment).with(ip_2) expect(response_headers['X-RateLimit-Remaining']).to eq('70') expect(response_headers['X-RateLimit-Reset']).to eq(valid_until_2.utc.to_i.to_s) end @@ -282,8 +251,8 @@ module Middleware expect(response_headers).not_to include('X-RateLimit-Remaining') expect(status).to eq(200) expect(app).to have_received(:call).at_least(:once) - expect(REQUEST_COUNTER).to_not have_received(:get) - expect(REQUEST_COUNTER).to_not have_received(:increment) + expect(request_counter).to_not have_received(:get) + expect(request_counter).to_not have_received(:increment) end end @@ -292,7 +261,7 @@ module Middleware let(:middleware_env) do { 'cf.user_guid' => 'user-id-1', 'PATH_INFO' => path_info } end - before(:each) { allow(REQUEST_COUNTER).to receive(:get).and_return([per_process_general_limit + 1, Time.now.utc]) } + before(:each) { allow(request_counter).to receive(:get).and_return([per_process_general_limit + 1, Time.now.utc]) } it 'returns 429 response' do status, _, _ = middleware.call(middleware_env) @@ -301,18 +270,18 @@ module Middleware it 'does not increment the request counter' do _, _, _ = middleware.call(middleware_env) - expect(REQUEST_COUNTER).to_not have_received(:increment) + expect(request_counter).to_not have_received(:increment) end it 'prevents "X-RateLimit-Remaining" from going lower than zero' do - allow(REQUEST_COUNTER).to receive(:get).and_return([per_process_general_limit + 100, Time.now.utc]) + allow(request_counter).to receive(:get).and_return([per_process_general_limit + 100, Time.now.utc]) _, response_headers, _ = middleware.call(middleware_env) expect(response_headers['X-RateLimit-Remaining']).to eq('0') end it 'contains the correct headers' do valid_until = Time.now.utc - allow(REQUEST_COUNTER).to receive(:get).and_return([per_process_general_limit + 1, valid_until]) + allow(request_counter).to receive(:get).and_return([per_process_general_limit + 1, valid_until]) error_presenter = instance_double(ErrorPresenter, to_hash: { foo: 'bar' }) allow(ErrorPresenter).to receive(:new).and_return(error_presenter) @@ -358,7 +327,7 @@ module Middleware let(:unauthenticated_env) { { 'some' => 'env', 'PATH_INFO' => path_info } } it 'suggests they log in' do - allow(REQUEST_COUNTER).to receive(:get).and_return([per_process_unauthenticated_limit + 1, Time.now.utc]) + allow(request_counter).to receive(:get).and_return([per_process_unauthenticated_limit + 1, Time.now.utc]) _, response_headers, body = middleware.call(unauthenticated_env) expect(response_headers['X-RateLimit-Remaining']).to eq('0') json_body = JSON.parse(body.first) @@ -372,7 +341,8 @@ module Middleware end end - RSpec.describe REQUEST_COUNTER do + RSpec.describe RequestCounter do + let(:request_counter) { RequestCounter.new } let(:reset_interval_in_minutes) { 60 } let(:logger) { double('logger', info: nil) } let(:user_guid) { 'user-id' } @@ -381,52 +351,51 @@ module Middleware describe 'get' do before(:each) do Timecop.freeze - REQUEST_COUNTER.reset end after(:each) do Timecop.return end it 'should return next offset valid until interval and 0 requests for a new user' do new_valid_until = Time.now.utc.beginning_of_hour + reset_interval_in_minutes.minutes - expect(REQUEST_COUNTER).to receive(:next_reset_interval).and_return(new_valid_until) + expect(request_counter).to receive(:next_reset_interval).and_return(new_valid_until) - count, valid_until = REQUEST_COUNTER.get(user_guid, reset_interval_in_minutes, logger) + count, valid_until = request_counter.get(user_guid, reset_interval_in_minutes, logger) expect(count).to eq(0) expect(valid_until).to eq(new_valid_until) end it 'should return offset valid untils for different users' do new_valid_until_1 = Time.now.utc.beginning_of_hour + reset_interval_in_minutes.minutes - expect(REQUEST_COUNTER).to receive(:next_reset_interval).with(user_guid, reset_interval_in_minutes).and_return(new_valid_until_1) - _, valid_until = REQUEST_COUNTER.get(user_guid, reset_interval_in_minutes, logger) + expect(request_counter).to receive(:next_reset_interval).with(user_guid, reset_interval_in_minutes).and_return(new_valid_until_1) + _, valid_until = request_counter.get(user_guid, reset_interval_in_minutes, logger) expect(valid_until).to eq(new_valid_until_1) new_valid_until_2 = Time.now.utc.beginning_of_hour + reset_interval_in_minutes.minutes - 5.minutes - expect(REQUEST_COUNTER).to receive(:next_reset_interval).with(user_guid_2, reset_interval_in_minutes).and_return(new_valid_until_2) - _, valid_until = REQUEST_COUNTER.get(user_guid_2, reset_interval_in_minutes, logger) + expect(request_counter).to receive(:next_reset_interval).with(user_guid_2, reset_interval_in_minutes).and_return(new_valid_until_2) + _, valid_until = request_counter.get(user_guid_2, reset_interval_in_minutes, logger) expect(valid_until).to eq(new_valid_until_2) end it 'should return valid until and requests for an existing user' do - expect(REQUEST_COUNTER).to receive(:next_reset_interval).and_return(Time.now.utc.beginning_of_hour + reset_interval_in_minutes.minutes) - _, original_valid_until = REQUEST_COUNTER.get(user_guid, reset_interval_in_minutes, logger) - REQUEST_COUNTER.increment(user_guid) + expect(request_counter).to receive(:next_reset_interval).and_return(Time.now.utc.beginning_of_hour + reset_interval_in_minutes.minutes) + _, original_valid_until = request_counter.get(user_guid, reset_interval_in_minutes, logger) + request_counter.increment(user_guid) Timecop.travel(original_valid_until - 1.minutes) do - count, valid_until = REQUEST_COUNTER.get(user_guid, reset_interval_in_minutes, logger) + count, valid_until = request_counter.get(user_guid, reset_interval_in_minutes, logger) expect(count).to eq(1) expect(valid_until).to eq(original_valid_until) end end it 'should return new valid until and 0 requests for an existing user with expired rate limit' do - expect(REQUEST_COUNTER).to receive(:next_reset_interval).and_return(Time.now.utc.beginning_of_hour + reset_interval_in_minutes.minutes) - _, original_valid_until = REQUEST_COUNTER.get(user_guid, reset_interval_in_minutes, logger) - REQUEST_COUNTER.increment(user_guid) + expect(request_counter).to receive(:next_reset_interval).and_return(Time.now.utc.beginning_of_hour + reset_interval_in_minutes.minutes) + _, original_valid_until = request_counter.get(user_guid, reset_interval_in_minutes, logger) + request_counter.increment(user_guid) Timecop.travel(original_valid_until + 1.minutes) do new_valid_until = Time.now.utc.beginning_of_hour + reset_interval_in_minutes.minutes - expect(REQUEST_COUNTER).to receive(:next_reset_interval).and_return(new_valid_until) - count, valid_until = REQUEST_COUNTER.get(user_guid, reset_interval_in_minutes, logger) + expect(request_counter).to receive(:next_reset_interval).and_return(new_valid_until) + count, valid_until = request_counter.get(user_guid, reset_interval_in_minutes, logger) expect(count).to eq(0) expect(valid_until).to eq(new_valid_until) expect(logger).to have_received(:info).with "Resetting request count of 1 for user 'user-id'" diff --git a/spec/unit/middleware/rate_limiter_v2_api_spec.rb b/spec/unit/middleware/rate_limiter_v2_api_spec.rb index 2954dadbe57..4931a6735d0 100644 --- a/spec/unit/middleware/rate_limiter_v2_api_spec.rb +++ b/spec/unit/middleware/rate_limiter_v2_api_spec.rb @@ -7,20 +7,20 @@ module Middleware RateLimiterV2API.new( app, { - logger: logger, - per_process_general_limit: per_process_general_limit, - global_general_limit: global_general_limit, - per_process_admin_limit: per_process_admin_limit, - global_admin_limit: global_admin_limit, - interval: interval, + logger: logger, + per_process_general_limit: per_process_general_limit, + global_general_limit: global_general_limit, + per_process_admin_limit: per_process_admin_limit, + global_admin_limit: global_admin_limit, + interval: interval, } ) end - let(:REQUEST_COUNTER_V2_API) { double } + let(:request_counter) { double } before(:each) { - middleware.instance_variable_set('@V2_API_REQUEST_COUNTER', REQUEST_COUNTER_V2_API) - allow(REQUEST_COUNTER_V2_API).to receive(:get).and_return([0, Time.now.utc]) - allow(REQUEST_COUNTER_V2_API).to receive(:increment) + middleware.instance_variable_set('@request_counter', request_counter) + allow(request_counter).to receive(:get).and_return([0, Time.now.utc]) + allow(request_counter).to receive(:increment) } let(:app) { double(:app, call: [200, {}, 'a body']) } @@ -33,7 +33,7 @@ module Middleware let(:path_info) { '/v2/service_instances' } let(:default_env) { { some: 'env' } } - let(:basic_auth_env) { { 'HTTP_AUTHORIZATION' => ActionController::HttpAuthentication::Basic.encode_credentials('user', 'pass'), 'PATH_INFO' => path_info } } + let(:basic_auth_env) { { 'HTTP_AUTHORIZATION' => ActionController::HttpAuthentication::Basic.encode_credentials('user', 'pass') } } let(:user_1_guid) { 'user-id-1' } let(:user_2_guid) { 'user-id-2' } let(:user_1_env) { { 'cf.user_guid' => user_1_guid, 'PATH_INFO' => path_info } } @@ -49,18 +49,18 @@ module Middleware describe 'X-RateLimit-Remaining-V2-API' do it 'shows the user the number of remaining requests rounded down to nearest 10%' do - allow(REQUEST_COUNTER_V2_API).to receive(:get).and_return([0, Time.now.utc]) + allow(request_counter).to receive(:get).and_return([0, Time.now.utc]) _, response_headers, _ = middleware.call(user_1_env) expect(response_headers['X-RateLimit-Remaining-V2-API']).to eq('180') - allow(REQUEST_COUNTER_V2_API).to receive(:get).and_return([10, Time.now.utc]) + allow(request_counter).to receive(:get).and_return([10, Time.now.utc]) _, response_headers, _ = middleware.call(user_1_env) expect(response_headers['X-RateLimit-Remaining-V2-API']).to eq('80') end - it 'tracks user\'s remaining requests independently' do - expect(REQUEST_COUNTER_V2_API).to receive(:get).with(user_1_guid, interval, logger).and_return([0, Time.now.utc]) - expect(REQUEST_COUNTER_V2_API).to receive(:get).with(user_2_guid, interval, logger).and_return([10, Time.now.utc]) + it "tracks user's remaining requests independently" do + expect(request_counter).to receive(:get).with(user_1_guid, interval, logger).and_return([0, Time.now.utc]) + expect(request_counter).to receive(:get).with(user_2_guid, interval, logger).and_return([10, Time.now.utc]) _, response_headers, _ = middleware.call(user_1_env) expect(response_headers['X-RateLimit-Remaining-V2-API']).to eq('180') @@ -73,7 +73,7 @@ module Middleware describe 'X-RateLimit-Reset-V2-API' do it 'shows the user when the interval will expire' do valid_until = Time.now.utc.beginning_of_hour + interval.minutes - allow(REQUEST_COUNTER_V2_API).to receive(:get).and_return([0, valid_until]) + allow(request_counter).to receive(:get).and_return([0, valid_until]) _, response_headers, _ = middleware.call(user_1_env) expect(response_headers['X-RateLimit-Reset-V2-API'].to_i).to eq(valid_until.utc.to_i) end @@ -93,11 +93,11 @@ module Middleware describe 'X-RateLimit-Remaining-V2-API' do it 'shows the user the number of remaining requests rounded down to nearest 10%' do - allow(REQUEST_COUNTER_V2_API).to receive(:get).and_return([0, Time.now.utc]) + allow(request_counter).to receive(:get).and_return([0, Time.now.utc]) _, response_headers, _ = middleware.call(user_1_env) expect(response_headers['X-RateLimit-Remaining-V2-API']).to eq('900') - allow(REQUEST_COUNTER_V2_API).to receive(:get).and_return([10, Time.now.utc]) + allow(request_counter).to receive(:get).and_return([10, Time.now.utc]) _, response_headers, _ = middleware.call(user_1_env) expect(response_headers['X-RateLimit-Remaining-V2-API']).to eq('800') end @@ -106,7 +106,7 @@ module Middleware describe 'X-RateLimit-Reset-V2-API' do it 'shows the user when the interval will expire' do valid_until = Time.now.utc.beginning_of_hour + interval.minutes - allow(REQUEST_COUNTER_V2_API).to receive(:get).and_return([0, valid_until]) + allow(request_counter).to receive(:get).and_return([0, valid_until]) _, response_headers, _ = middleware.call(user_1_env) expect(response_headers['X-RateLimit-Reset-V2-API'].to_i).to eq(valid_until.utc.to_i) end @@ -115,7 +115,7 @@ module Middleware it 'increments the counter and allows the request to continue' do _, _, _ = middleware.call(user_1_env) - expect(REQUEST_COUNTER_V2_API).to have_received(:increment).with(user_1_guid) + expect(request_counter).to have_received(:increment).with(user_1_guid) expect(app).to have_received(:call) end @@ -133,8 +133,8 @@ module Middleware it 'exempts them from rate limiting' do allow(ActionDispatch::Request).to receive(:new).and_return(fake_request) _, response_headers, _ = middleware.call(default_env) - expect(REQUEST_COUNTER_V2_API).not_to have_received(:get) - expect(REQUEST_COUNTER_V2_API).not_to have_received(:increment) + expect(request_counter).not_to have_received(:get) + expect(request_counter).not_to have_received(:increment) expect(response_headers['X-RateLimit-Limit-V2-API']).to be_nil expect(response_headers['X-RateLimit-Remaining-V2-API']).to be_nil expect(response_headers['X-RateLimit-Reset-V2-API']).to be_nil @@ -147,8 +147,8 @@ module Middleware it 'rate limits them' do allow(ActionDispatch::Request).to receive(:new).and_return(fake_request) - expect(REQUEST_COUNTER_V2_API).to receive(:get).with('forwarded_ip', interval, logger).and_return([0, Time.now.utc]) - expect(REQUEST_COUNTER_V2_API).to receive(:increment).with('forwarded_ip') + expect(request_counter).to receive(:get).with('forwarded_ip', interval, logger).and_return([0, Time.now.utc]) + expect(request_counter).to receive(:increment).with('forwarded_ip') _, response_headers, _ = middleware.call(default_env) expect(response_headers['X-RateLimit-Limit-V2-API']).to_not be_nil expect(response_headers['X-RateLimit-Remaining-V2-API']).to_not be_nil @@ -160,70 +160,40 @@ module Middleware context 'when the user is hitting a root path /' do let(:fake_request) { instance_double(ActionDispatch::Request, fullpath: '/') } - it 'exempts them from rate limiting' do - allow(ActionDispatch::Request).to receive(:new).and_return(fake_request) - _, response_headers, _ = middleware.call(default_env) - expect(REQUEST_COUNTER_V2_API).not_to have_received(:get) - expect(REQUEST_COUNTER_V2_API).not_to have_received(:increment) - expect(response_headers['X-RateLimit-Limit-V2-API']).to be_nil - expect(response_headers['X-RateLimit-Remaining-V2-API']).to be_nil - expect(response_headers['X-RateLimit-Reset-V2-API']).to be_nil + it_behaves_like 'endpoint exempts from rate limiting', '-V2-API' do + let(:env) { default_env } end end context 'when the user is hitting a root path /v2/info' do let(:fake_request) { instance_double(ActionDispatch::Request, fullpath: '/v2/info') } - it 'exempts them from rate limiting' do - allow(ActionDispatch::Request).to receive(:new).and_return(fake_request) - _, response_headers, _ = middleware.call(default_env) - expect(REQUEST_COUNTER_V2_API).not_to have_received(:get) - expect(REQUEST_COUNTER_V2_API).not_to have_received(:increment) - expect(response_headers['X-RateLimit-Limit-V2-API']).to be_nil - expect(response_headers['X-RateLimit-Remaining-V2-API']).to be_nil - expect(response_headers['X-RateLimit-Reset-V2-API']).to be_nil + it_behaves_like 'endpoint exempts from rate limiting', '-V2-API' do + let(:env) { default_env } end end context 'when the user is hitting a root path /v3' do let(:fake_request) { instance_double(ActionDispatch::Request, fullpath: '/v3') } - it 'exempts them from rate limiting' do - allow(ActionDispatch::Request).to receive(:new).and_return(fake_request) - _, response_headers, _ = middleware.call(default_env) - expect(REQUEST_COUNTER_V2_API).not_to have_received(:get) - expect(REQUEST_COUNTER_V2_API).not_to have_received(:increment) - expect(response_headers['X-RateLimit-Limit-V2-API']).to be_nil - expect(response_headers['X-RateLimit-Remaining-V2-API']).to be_nil - expect(response_headers['X-RateLimit-Reset-V2-API']).to be_nil + it_behaves_like 'endpoint exempts from rate limiting', '-V2-API' do + let(:env) { default_env } end end context 'when the user is hitting a root path /v3/services' do let(:fake_request) { instance_double(ActionDispatch::Request, fullpath: '/v3/services') } - it 'exempts them from rate limiting' do - allow(ActionDispatch::Request).to receive(:new).and_return(fake_request) - _, response_headers, _ = middleware.call(default_env) - expect(REQUEST_COUNTER_V2_API).not_to have_received(:get) - expect(REQUEST_COUNTER_V2_API).not_to have_received(:increment) - expect(response_headers['X-RateLimit-Limit-V2-API']).to be_nil - expect(response_headers['X-RateLimit-Remaining-V2-API']).to be_nil - expect(response_headers['X-RateLimit-Reset-V2-API']).to be_nil + it_behaves_like 'endpoint exempts from rate limiting', '-V2-API' do + let(:env) { default_env } end end context 'when the user is hitting a root path /healthz' do let(:fake_request) { instance_double(ActionDispatch::Request, fullpath: '/healthz') } - it 'exempts them from rate limiting' do - allow(ActionDispatch::Request).to receive(:new).and_return(fake_request) - _, response_headers, _ = middleware.call(default_env) - expect(REQUEST_COUNTER_V2_API).not_to have_received(:get) - expect(REQUEST_COUNTER_V2_API).not_to have_received(:increment) - expect(response_headers['X-RateLimit-Limit-V2-API']).to be_nil - expect(response_headers['X-RateLimit-Remaining-V2-API']).to be_nil - expect(response_headers['X-RateLimit-Reset-V2-API']).to be_nil + it_behaves_like 'endpoint exempts from rate limiting', '-V2-API' do + let(:env) { default_env } end end end @@ -232,8 +202,8 @@ module Middleware describe 'when the user has basic auth credentials' do it 'exempts them from rate limiting' do _, response_headers, _ = middleware.call(basic_auth_env) - expect(REQUEST_COUNTER_V2_API).not_to have_received(:get) - expect(REQUEST_COUNTER_V2_API).not_to have_received(:increment) + expect(request_counter).not_to have_received(:get) + expect(request_counter).not_to have_received(:increment) expect(response_headers['X-RateLimit-Limit-V2-API']).to be_nil expect(response_headers['X-RateLimit-Remaining-V2-API']).to be_nil expect(response_headers['X-RateLimit-Reset-V2-API']).to be_nil @@ -258,15 +228,15 @@ module Middleware valid_until_2 = Time.now.utc allow(ActionDispatch::Request).to receive(:new).and_return(fake_request) - expect(REQUEST_COUNTER_V2_API).to receive(:get).with(forwarded_ip, interval, logger).and_return([0, valid_until]) - expect(REQUEST_COUNTER_V2_API).to receive(:increment).with(forwarded_ip) + expect(request_counter).to receive(:get).with(forwarded_ip, interval, logger).and_return([0, valid_until]) + expect(request_counter).to receive(:increment).with(forwarded_ip) _, response_headers, _ = middleware.call(default_env) expect(response_headers['X-RateLimit-Remaining-V2-API']).to eq('180') expect(response_headers['X-RateLimit-Reset-V2-API']).to eq(valid_until.to_i.to_s) allow(ActionDispatch::Request).to receive(:new).and_return(fake_request_2) - expect(REQUEST_COUNTER_V2_API).to receive(:get).with(forwarded_ip_2, interval, logger).and_return([2, valid_until_2]) - expect(REQUEST_COUNTER_V2_API).to receive(:increment).with(forwarded_ip_2) + expect(request_counter).to receive(:get).with(forwarded_ip_2, interval, logger).and_return([2, valid_until_2]) + expect(request_counter).to receive(:increment).with(forwarded_ip_2) _, response_headers, _ = middleware.call(default_env) expect(response_headers['X-RateLimit-Remaining-V2-API']).to eq('160') expect(response_headers['X-RateLimit-Reset-V2-API']).to eq(valid_until_2.to_i.to_s) @@ -283,18 +253,18 @@ module Middleware it 'identifies them by the request ip' do valid_until = Time.now.utc.beginning_of_hour valid_until_2 = Time.now.utc.beginning_of_hour + 5.minutes - allow(REQUEST_COUNTER_V2_API).to receive(:get).with(ip, interval, logger).and_return([0, valid_until]) - allow(REQUEST_COUNTER_V2_API).to receive(:get).with(ip_2, interval, logger).and_return([2, valid_until_2]) + allow(request_counter).to receive(:get).with(ip, interval, logger).and_return([0, valid_until]) + allow(request_counter).to receive(:get).with(ip_2, interval, logger).and_return([2, valid_until_2]) allow(ActionDispatch::Request).to receive(:new).and_return(fake_request) _, response_headers, _ = middleware.call(default_env) - expect(REQUEST_COUNTER_V2_API).to have_received(:increment).with(ip) + expect(request_counter).to have_received(:increment).with(ip) expect(response_headers['X-RateLimit-Remaining-V2-API']).to eq('180') expect(response_headers['X-RateLimit-Reset-V2-API']).to eq(valid_until.utc.to_i.to_s) allow(ActionDispatch::Request).to receive(:new).and_return(fake_request_2) _, response_headers, _ = middleware.call(default_env) - expect(REQUEST_COUNTER_V2_API).to have_received(:increment).with(ip_2) + expect(request_counter).to have_received(:increment).with(ip_2) expect(response_headers['X-RateLimit-Remaining-V2-API']).to eq('160') expect(response_headers['X-RateLimit-Reset-V2-API']).to eq(valid_until_2.utc.to_i.to_s) end @@ -306,7 +276,7 @@ module Middleware let(:middleware_env) do { 'cf.user_guid' => 'user-id-1', 'PATH_INFO' => path_info } end - before(:each) { allow(REQUEST_COUNTER_V2_API).to receive(:get).and_return([per_process_general_limit + 1, Time.now.utc]) } + before(:each) { allow(request_counter).to receive(:get).and_return([per_process_general_limit + 1, Time.now.utc]) } it 'returns 429 response' do status, _, _ = middleware.call(middleware_env) @@ -315,18 +285,18 @@ module Middleware it 'does not increment the request counter' do _, _, _ = middleware.call(middleware_env) - expect(REQUEST_COUNTER_V2_API).to_not have_received(:increment) + expect(request_counter).to_not have_received(:increment) end it 'prevents "X-RateLimit-Remaining-V2-API" from going lower than zero' do - allow(REQUEST_COUNTER_V2_API).to receive(:get).and_return([per_process_general_limit + 100, Time.now.utc]) + allow(request_counter).to receive(:get).and_return([per_process_general_limit + 100, Time.now.utc]) _, response_headers, _ = middleware.call(middleware_env) expect(response_headers['X-RateLimit-Remaining-V2-API']).to eq('0') end it 'contains the correct headers' do valid_until = Time.now.utc - allow(REQUEST_COUNTER_V2_API).to receive(:get).and_return([per_process_general_limit + 1, valid_until]) + allow(request_counter).to receive(:get).and_return([per_process_general_limit + 1, valid_until]) error_presenter = instance_double(ErrorPresenter, to_hash: { foo: 'bar' }) allow(ErrorPresenter).to receive(:new).and_return(error_presenter) @@ -348,16 +318,20 @@ module Middleware 'code' => 10018, 'description' => 'Rate Limit of V2 API Exceeded. Please consider to use V3 API', 'error_code' => 'CF-RateLimitV2APIExceeded', - ) + ) end context 'when the user is admin' do let(:path_info) { '/v2/foo' } let(:default_env) { { 'some' => 'env', 'PATH_INFO' => path_info } } + before do + allow(VCAP::CloudController::SecurityContext).to receive(:admin_read_only?).and_return(true) + end + it 'contains the correct headers' do valid_until = Time.now.utc - allow(REQUEST_COUNTER_V2_API).to receive(:get).and_return([per_process_admin_limit + 1, valid_until]) + allow(request_counter).to receive(:get).and_return([per_process_admin_limit + 1, valid_until]) error_presenter = instance_double(ErrorPresenter, to_hash: { foo: 'bar' }) allow(ErrorPresenter).to receive(:new).and_return(error_presenter)