From 3e619d1f9e2527e61e9b5964ee3409b6d04a5223 Mon Sep 17 00:00:00 2001 From: Mayra Cabrera Date: Thu, 5 Sep 2019 21:15:55 +0000 Subject: Refactors rack attack requests spec * Creates a shared_examples files under shared_examples/requests * Moves web specs into the shared examples file * Moves let definitions to the block that uses them --- spec/requests/rack_attack_global_spec.rb | 222 +-------------------- .../requests/rack_attack_shared_examples.rb | 221 ++++++++++++++++++++ 2 files changed, 231 insertions(+), 212 deletions(-) create mode 100644 spec/support/shared_examples/requests/rack_attack_shared_examples.rb diff --git a/spec/requests/rack_attack_global_spec.rb b/spec/requests/rack_attack_global_spec.rb index 478f09a7881..cf459ba99c1 100644 --- a/spec/requests/rack_attack_global_spec.rb +++ b/spec/requests/rack_attack_global_spec.rb @@ -20,11 +20,6 @@ describe 'Rack Attack global throttles' do let(:period_in_seconds) { 10000 } let(:period) { period_in_seconds.seconds } - let(:url_that_does_not_require_authentication) { '/users/sign_in' } - let(:url_that_requires_authentication) { '/dashboard/snippets' } - let(:url_api_internal) { '/api/v4/internal/check' } - let(:api_partial_url) { '/todos' } - around do |example| # Instead of test environment's :null_store so the throttles can increment Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new @@ -35,112 +30,10 @@ describe 'Rack Attack global throttles' do Rack::Attack.cache.store = Rails.cache end - # Requires let variables: - # * throttle_setting_prefix (e.g. "throttle_authenticated_api" or "throttle_authenticated_web") - # * get_args - # * other_user_get_args - shared_examples_for 'rate-limited token-authenticated requests' do - before do - # Set low limits - settings_to_set[:"#{throttle_setting_prefix}_requests_per_period"] = requests_per_period - settings_to_set[:"#{throttle_setting_prefix}_period_in_seconds"] = period_in_seconds - end - - context 'when the throttle is enabled' do - before do - settings_to_set[:"#{throttle_setting_prefix}_enabled"] = true - stub_application_setting(settings_to_set) - end - - it 'rejects requests over the rate limit' do - # At first, allow requests under the rate limit. - requests_per_period.times do - get(*get_args) - expect(response).to have_http_status 200 - end - - # the last straw - expect_rejection { get(*get_args) } - end - - it 'allows requests after throttling and then waiting for the next period' do - requests_per_period.times do - get(*get_args) - expect(response).to have_http_status 200 - end - - expect_rejection { get(*get_args) } - - Timecop.travel(period.from_now) do - requests_per_period.times do - get(*get_args) - expect(response).to have_http_status 200 - end - - expect_rejection { get(*get_args) } - end - end - - it 'counts requests from different users separately, even from the same IP' do - requests_per_period.times do - get(*get_args) - expect(response).to have_http_status 200 - end - - # would be over the limit if this wasn't a different user - get(*other_user_get_args) - expect(response).to have_http_status 200 - end - - it 'counts all requests from the same user, even via different IPs' do - requests_per_period.times do - get(*get_args) - expect(response).to have_http_status 200 - end - - expect_any_instance_of(Rack::Attack::Request).to receive(:ip).and_return('1.2.3.4') - - expect_rejection { get(*get_args) } - end - - it 'logs RackAttack info into structured logs' do - requests_per_period.times do - get(*get_args) - expect(response).to have_http_status 200 - end - - arguments = { - message: 'Rack_Attack', - env: :throttle, - remote_ip: '127.0.0.1', - request_method: 'GET', - path: get_args.first, - user_id: user.id, - username: user.username - } - - expect(Gitlab::AuthLogger).to receive(:error).with(arguments).once - - expect_rejection { get(*get_args) } - end - end - - context 'when the throttle is disabled' do - before do - settings_to_set[:"#{throttle_setting_prefix}_enabled"] = false - stub_application_setting(settings_to_set) - end - - it 'allows requests over the rate limit' do - (1 + requests_per_period).times do - get(*get_args) - expect(response).to have_http_status 200 - end - end - end - end - describe 'unauthenticated requests' do + let(:url_that_does_not_require_authentication) { '/users/sign_in' } + let(:url_api_internal) { '/api/v4/internal/check' } + before do # Set low limits settings_to_set[:throttle_unauthenticated_requests_per_period] = requests_per_period @@ -245,6 +138,7 @@ describe 'Rack Attack global throttles' do let(:other_user) { create(:user) } let(:other_user_token) { create(:personal_access_token, user: other_user) } let(:throttle_setting_prefix) { 'throttle_authenticated_api' } + let(:api_partial_url) { '/todos' } context 'with the token in the query string' do let(:get_args) { [api(api_partial_url, personal_access_token: token)] } @@ -265,10 +159,13 @@ describe 'Rack Attack global throttles' do let(:user) { create(:user) } let(:application) { Doorkeeper::Application.create!(name: "MyApp", redirect_uri: "https://app.com", owner: user) } let(:token) { Doorkeeper::AccessToken.create!(application_id: application.id, resource_owner_id: user.id, scopes: "api") } + let(:other_user) { create(:user) } let(:other_user_application) { Doorkeeper::Application.create!(name: "MyApp", redirect_uri: "https://app.com", owner: other_user) } let(:other_user_token) { Doorkeeper::AccessToken.create!(application_id: application.id, resource_owner_id: other_user.id, scopes: "api") } + let(:throttle_setting_prefix) { 'throttle_authenticated_api' } + let(:api_partial_url) { '/todos' } context 'with the token in the query string' do let(:get_args) { [api(api_partial_url, oauth_access_token: token)] } @@ -299,110 +196,11 @@ describe 'Rack Attack global throttles' do end describe 'web requests authenticated with regular login' do + let(:throttle_setting_prefix) { 'throttle_authenticated_web' } let(:user) { create(:user) } + let(:url_that_requires_authentication) { '/dashboard/snippets' } - before do - login_as(user) - - # Set low limits - settings_to_set[:throttle_authenticated_web_requests_per_period] = requests_per_period - settings_to_set[:throttle_authenticated_web_period_in_seconds] = period_in_seconds - end - - context 'when the throttle is enabled' do - before do - settings_to_set[:throttle_authenticated_web_enabled] = true - stub_application_setting(settings_to_set) - end - - it 'rejects requests over the rate limit' do - # At first, allow requests under the rate limit. - requests_per_period.times do - get url_that_requires_authentication - expect(response).to have_http_status 200 - end - - # the last straw - expect_rejection { get url_that_requires_authentication } - end - - it 'allows requests after throttling and then waiting for the next period' do - requests_per_period.times do - get url_that_requires_authentication - expect(response).to have_http_status 200 - end - - expect_rejection { get url_that_requires_authentication } - - Timecop.travel(period.from_now) do - requests_per_period.times do - get url_that_requires_authentication - expect(response).to have_http_status 200 - end - - expect_rejection { get url_that_requires_authentication } - end - end - - it 'counts requests from different users separately, even from the same IP' do - requests_per_period.times do - get url_that_requires_authentication - expect(response).to have_http_status 200 - end - - # would be over the limit if this wasn't a different user - login_as(create(:user)) - - get url_that_requires_authentication - expect(response).to have_http_status 200 - end - - it 'counts all requests from the same user, even via different IPs' do - requests_per_period.times do - get url_that_requires_authentication - expect(response).to have_http_status 200 - end - - expect_any_instance_of(Rack::Attack::Request).to receive(:ip).and_return('1.2.3.4') - - expect_rejection { get url_that_requires_authentication } - end - - it 'logs RackAttack info into structured logs' do - requests_per_period.times do - get url_that_requires_authentication - expect(response).to have_http_status 200 - end - - arguments = { - message: 'Rack_Attack', - env: :throttle, - remote_ip: '127.0.0.1', - request_method: 'GET', - path: '/dashboard/snippets', - user_id: user.id, - username: user.username - } - - expect(Gitlab::AuthLogger).to receive(:error).with(arguments).once - - get url_that_requires_authentication - end - end - - context 'when the throttle is disabled' do - before do - settings_to_set[:throttle_authenticated_web_enabled] = false - stub_application_setting(settings_to_set) - end - - it 'allows requests over the rate limit' do - (1 + requests_per_period).times do - get url_that_requires_authentication - expect(response).to have_http_status 200 - end - end - end + it_behaves_like 'rate-limited web authenticated requests' end def api_get_args_with_token_headers(partial_url, token_headers) diff --git a/spec/support/shared_examples/requests/rack_attack_shared_examples.rb b/spec/support/shared_examples/requests/rack_attack_shared_examples.rb new file mode 100644 index 00000000000..afc6f59b773 --- /dev/null +++ b/spec/support/shared_examples/requests/rack_attack_shared_examples.rb @@ -0,0 +1,221 @@ +# frozen_string_literal: true +# +# Requires let variables: +# * throttle_setting_prefix: "throttle_authenticated_api", "throttle_authenticated_web", "throttle_protected_paths" +# * get_args +# * other_user_get_args +# * requests_per_period +# * period_in_seconds +# * period +shared_examples_for 'rate-limited token-authenticated requests' do + before do + # Set low limits + settings_to_set[:"#{throttle_setting_prefix}_requests_per_period"] = requests_per_period + settings_to_set[:"#{throttle_setting_prefix}_period_in_seconds"] = period_in_seconds + end + + context 'when the throttle is enabled' do + before do + settings_to_set[:"#{throttle_setting_prefix}_enabled"] = true + stub_application_setting(settings_to_set) + end + + it 'rejects requests over the rate limit' do + # At first, allow requests under the rate limit. + requests_per_period.times do + get(*get_args) + expect(response).to have_http_status 200 + end + + # the last straw + expect_rejection { get(*get_args) } + end + + it 'allows requests after throttling and then waiting for the next period' do + requests_per_period.times do + get(*get_args) + expect(response).to have_http_status 200 + end + + expect_rejection { get(*get_args) } + + Timecop.travel(period.from_now) do + requests_per_period.times do + get(*get_args) + expect(response).to have_http_status 200 + end + + expect_rejection { get(*get_args) } + end + end + + it 'counts requests from different users separately, even from the same IP' do + requests_per_period.times do + get(*get_args) + expect(response).to have_http_status 200 + end + + # would be over the limit if this wasn't a different user + get(*other_user_get_args) + expect(response).to have_http_status 200 + end + + it 'counts all requests from the same user, even via different IPs' do + requests_per_period.times do + get(*get_args) + expect(response).to have_http_status 200 + end + + expect_any_instance_of(Rack::Attack::Request).to receive(:ip).and_return('1.2.3.4') + + expect_rejection { get(*get_args) } + end + + it 'logs RackAttack info into structured logs' do + requests_per_period.times do + get(*get_args) + expect(response).to have_http_status 200 + end + + arguments = { + message: 'Rack_Attack', + env: :throttle, + remote_ip: '127.0.0.1', + request_method: 'GET', + path: get_args.first, + user_id: user.id, + username: user.username + } + + expect(Gitlab::AuthLogger).to receive(:error).with(arguments).once + + expect_rejection { get(*get_args) } + end + end + + context 'when the throttle is disabled' do + before do + settings_to_set[:"#{throttle_setting_prefix}_enabled"] = false + stub_application_setting(settings_to_set) + end + + it 'allows requests over the rate limit' do + (1 + requests_per_period).times do + get(*get_args) + expect(response).to have_http_status 200 + end + end + end +end + +# Requires let variables: +# * throttle_setting_prefix: "throttle_authenticated_web" or "throttle_protected_paths" +# * user +# * url_that_requires_authentication +# * requests_per_period +# * period_in_seconds +# * period +shared_examples_for 'rate-limited web authenticated requests' do + before do + login_as(user) + + # Set low limits + settings_to_set[:"#{throttle_setting_prefix}_requests_per_period"] = requests_per_period + settings_to_set[:"#{throttle_setting_prefix}_period_in_seconds"] = period_in_seconds + end + + context 'when the throttle is enabled' do + before do + settings_to_set[:"#{throttle_setting_prefix}_enabled"] = true + stub_application_setting(settings_to_set) + end + + it 'rejects requests over the rate limit' do + # At first, allow requests under the rate limit. + requests_per_period.times do + get url_that_requires_authentication + expect(response).to have_http_status 200 + end + + # the last straw + expect_rejection { get url_that_requires_authentication } + end + + it 'allows requests after throttling and then waiting for the next period' do + requests_per_period.times do + get url_that_requires_authentication + expect(response).to have_http_status 200 + end + + expect_rejection { get url_that_requires_authentication } + + Timecop.travel(period.from_now) do + requests_per_period.times do + get url_that_requires_authentication + expect(response).to have_http_status 200 + end + + expect_rejection { get url_that_requires_authentication } + end + end + + it 'counts requests from different users separately, even from the same IP' do + requests_per_period.times do + get url_that_requires_authentication + expect(response).to have_http_status 200 + end + + # would be over the limit if this wasn't a different user + login_as(create(:user)) + + get url_that_requires_authentication + expect(response).to have_http_status 200 + end + + it 'counts all requests from the same user, even via different IPs' do + requests_per_period.times do + get url_that_requires_authentication + expect(response).to have_http_status 200 + end + + expect_any_instance_of(Rack::Attack::Request).to receive(:ip).and_return('1.2.3.4') + + expect_rejection { get url_that_requires_authentication } + end + + it 'logs RackAttack info into structured logs' do + requests_per_period.times do + get url_that_requires_authentication + expect(response).to have_http_status 200 + end + + arguments = { + message: 'Rack_Attack', + env: :throttle, + remote_ip: '127.0.0.1', + request_method: 'GET', + path: '/dashboard/snippets', + user_id: user.id, + username: user.username + } + + expect(Gitlab::AuthLogger).to receive(:error).with(arguments).once + + get url_that_requires_authentication + end + end + + context 'when the throttle is disabled' do + before do + settings_to_set[:"#{throttle_setting_prefix}_enabled"] = false + stub_application_setting(settings_to_set) + end + + it 'allows requests over the rate limit' do + (1 + requests_per_period).times do + get url_that_requires_authentication + expect(response).to have_http_status 200 + end + end + end +end -- cgit v1.2.1