diff options
Diffstat (limited to 'spec/requests')
| -rw-r--r-- | spec/requests/api/helpers_spec.rb | 46 | ||||
| -rw-r--r-- | spec/requests/api/merge_requests_spec.rb | 8 | ||||
| -rw-r--r-- | spec/requests/api/projects_spec.rb | 2 | ||||
| -rw-r--r-- | spec/requests/api/users_spec.rb | 8 | ||||
| -rw-r--r-- | spec/requests/api/v3/merge_requests_spec.rb | 2 | ||||
| -rw-r--r-- | spec/requests/api/v3/projects_spec.rb | 2 | ||||
| -rw-r--r-- | spec/requests/rack_attack_global_spec.rb | 362 |
7 files changed, 405 insertions, 25 deletions
diff --git a/spec/requests/api/helpers_spec.rb b/spec/requests/api/helpers_spec.rb index 6c0996c543d..0462f494e15 100644 --- a/spec/requests/api/helpers_spec.rb +++ b/spec/requests/api/helpers_spec.rb @@ -11,7 +11,6 @@ describe API::Helpers do let(:admin) { create(:admin) } let(:key) { create(:key, user: user) } - let(:params) { {} } let(:csrf_token) { SecureRandom.base64(ActionController::RequestForgeryProtection::AUTHENTICITY_TOKEN_LENGTH) } let(:env) do { @@ -19,10 +18,13 @@ describe API::Helpers do 'rack.session' => { _csrf_token: csrf_token }, - 'REQUEST_METHOD' => 'GET' + 'REQUEST_METHOD' => 'GET', + 'CONTENT_TYPE' => 'text/plain;charset=utf-8' } end let(:header) { } + let(:request) { Grape::Request.new(env)} + let(:params) { request.params } before do allow_any_instance_of(self.class).to receive(:options).and_return({}) @@ -37,6 +39,10 @@ describe API::Helpers do raise Exception.new("#{status} - #{message}") end + def set_param(key, value) + request.update_param(key, value) + end + describe ".current_user" do subject { current_user } @@ -132,13 +138,13 @@ describe API::Helpers do let(:personal_access_token) { create(:personal_access_token, user: user) } it "returns a 401 response for an invalid token" do - env[API::APIGuard::PRIVATE_TOKEN_HEADER] = 'invalid token' + env[Gitlab::Auth::UserAuthFinders::PRIVATE_TOKEN_HEADER] = 'invalid token' expect { current_user }.to raise_error /401/ end it "returns a 403 response for a user without access" do - env[API::APIGuard::PRIVATE_TOKEN_HEADER] = personal_access_token.token + env[Gitlab::Auth::UserAuthFinders::PRIVATE_TOKEN_HEADER] = personal_access_token.token allow_any_instance_of(Gitlab::UserAccess).to receive(:allowed?).and_return(false) expect { current_user }.to raise_error /403/ @@ -146,35 +152,35 @@ describe API::Helpers do it 'returns a 403 response for a user who is blocked' do user.block! - env[API::APIGuard::PRIVATE_TOKEN_HEADER] = personal_access_token.token + env[Gitlab::Auth::UserAuthFinders::PRIVATE_TOKEN_HEADER] = personal_access_token.token expect { current_user }.to raise_error /403/ end it "sets current_user" do - env[API::APIGuard::PRIVATE_TOKEN_HEADER] = personal_access_token.token + env[Gitlab::Auth::UserAuthFinders::PRIVATE_TOKEN_HEADER] = personal_access_token.token expect(current_user).to eq(user) end it "does not allow tokens without the appropriate scope" do personal_access_token = create(:personal_access_token, user: user, scopes: ['read_user']) - env[API::APIGuard::PRIVATE_TOKEN_HEADER] = personal_access_token.token + env[Gitlab::Auth::UserAuthFinders::PRIVATE_TOKEN_HEADER] = personal_access_token.token - expect { current_user }.to raise_error API::APIGuard::InsufficientScopeError + expect { current_user }.to raise_error Gitlab::Auth::InsufficientScopeError end it 'does not allow revoked tokens' do personal_access_token.revoke! - env[API::APIGuard::PRIVATE_TOKEN_HEADER] = personal_access_token.token + env[Gitlab::Auth::UserAuthFinders::PRIVATE_TOKEN_HEADER] = personal_access_token.token - expect { current_user }.to raise_error API::APIGuard::RevokedError + expect { current_user }.to raise_error Gitlab::Auth::RevokedError end it 'does not allow expired tokens' do personal_access_token.update_attributes!(expires_at: 1.day.ago) - env[API::APIGuard::PRIVATE_TOKEN_HEADER] = personal_access_token.token + env[Gitlab::Auth::UserAuthFinders::PRIVATE_TOKEN_HEADER] = personal_access_token.token - expect { current_user }.to raise_error API::APIGuard::ExpiredError + expect { current_user }.to raise_error Gitlab::Auth::ExpiredError end end end @@ -350,7 +356,7 @@ describe API::Helpers do context 'when using param' do context 'when providing username' do before do - params[API::Helpers::SUDO_PARAM] = user.username + set_param(API::Helpers::SUDO_PARAM, user.username) end it_behaves_like 'successful sudo' @@ -358,7 +364,7 @@ describe API::Helpers do context 'when providing user ID' do before do - params[API::Helpers::SUDO_PARAM] = user.id.to_s + set_param(API::Helpers::SUDO_PARAM, user.id.to_s) end it_behaves_like 'successful sudo' @@ -368,7 +374,7 @@ describe API::Helpers do context 'when user does not exist' do before do - params[API::Helpers::SUDO_PARAM] = 'nonexistent' + set_param(API::Helpers::SUDO_PARAM, 'nonexistent') end it 'raises an error' do @@ -382,11 +388,11 @@ describe API::Helpers do token.scopes = %w[api] token.save! - params[API::Helpers::SUDO_PARAM] = user.id.to_s + set_param(API::Helpers::SUDO_PARAM, user.id.to_s) end it 'raises an error' do - expect { current_user }.to raise_error API::APIGuard::InsufficientScopeError + expect { current_user }.to raise_error Gitlab::Auth::InsufficientScopeError end end end @@ -396,7 +402,7 @@ describe API::Helpers do token.user = user token.save! - params[API::Helpers::SUDO_PARAM] = user.id.to_s + set_param(API::Helpers::SUDO_PARAM, user.id.to_s) end it 'raises an error' do @@ -420,7 +426,7 @@ describe API::Helpers do context 'passed as param' do before do - params[API::APIGuard::PRIVATE_TOKEN_PARAM] = token.token + set_param(Gitlab::Auth::UserAuthFinders::PRIVATE_TOKEN_PARAM, token.token) end it_behaves_like 'sudo' @@ -428,7 +434,7 @@ describe API::Helpers do context 'passed as header' do before do - env[API::APIGuard::PRIVATE_TOKEN_HEADER] = token.token + env[Gitlab::Auth::UserAuthFinders::PRIVATE_TOKEN_HEADER] = token.token end it_behaves_like 'sudo' diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index a928ba79a4d..91616da6d9a 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -172,15 +172,15 @@ describe API::MergeRequests do context "when authenticated" do it 'avoids N+1 queries' do - control_count = ActiveRecord::QueryRecorder.new do + control = ActiveRecord::QueryRecorder.new do get api("/projects/#{project.id}/merge_requests", user) - end.count + end create(:merge_request, state: 'closed', milestone: milestone1, author: user, assignee: user, source_project: project, target_project: project, title: "Test", created_at: base_time) expect do get api("/projects/#{project.id}/merge_requests", user) - end.not_to exceed_query_limit(control_count) + end.not_to exceed_query_limit(control) end it "returns an array of all merge_requests" do @@ -628,7 +628,7 @@ describe API::MergeRequests do context 'forked projects' do let!(:user2) { create(:user) } - let!(:forked_project) { fork_project(project, user2) } + let!(:forked_project) { fork_project(project, user2, repository: true) } let!(:unrelated_project) { create(:project, namespace: create(:user).namespace, creator_id: user2.id) } before do diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 50f6c8b7d64..a41345da05b 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -437,6 +437,7 @@ describe API::Projects do project.each_pair do |k, v| next if %i[has_external_issue_tracker issues_enabled merge_requests_enabled wiki_enabled].include?(k) + expect(json_response[k.to_s]).to eq(v) end @@ -643,6 +644,7 @@ describe API::Projects do expect(response).to have_gitlab_http_status(201) project.each_pair do |k, v| next if %i[has_external_issue_tracker path].include?(k) + expect(json_response[k.to_s]).to eq(v) end end diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index 2aeae6f9ec7..2428e63e149 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -510,6 +510,14 @@ describe API::Users do expect(user.reload.notification_email).to eq('new@email.com') end + it 'skips reconfirmation when requested' do + put api("/users/#{user.id}", admin), { skip_reconfirmation: true } + + user.reload + + expect(user.confirmed_at).to be_present + end + it 'updates user with his own username' do put api("/users/#{user.id}", admin), username: user.username diff --git a/spec/requests/api/v3/merge_requests_spec.rb b/spec/requests/api/v3/merge_requests_spec.rb index 91897e5ee01..2e2b9449429 100644 --- a/spec/requests/api/v3/merge_requests_spec.rb +++ b/spec/requests/api/v3/merge_requests_spec.rb @@ -314,7 +314,7 @@ describe API::MergeRequests do context 'forked projects' do let!(:user2) { create(:user) } - let!(:forked_project) { fork_project(project, user2) } + let!(:forked_project) { fork_project(project, user2, repository: true) } let!(:unrelated_project) { create(:project, namespace: create(:user).namespace, creator_id: user2.id) } before do diff --git a/spec/requests/api/v3/projects_spec.rb b/spec/requests/api/v3/projects_spec.rb index f62ad747c73..27288b98d1c 100644 --- a/spec/requests/api/v3/projects_spec.rb +++ b/spec/requests/api/v3/projects_spec.rb @@ -404,6 +404,7 @@ describe API::V3::Projects do project.each_pair do |k, v| next if %i[has_external_issue_tracker issues_enabled merge_requests_enabled wiki_enabled].include?(k) + expect(json_response[k.to_s]).to eq(v) end @@ -547,6 +548,7 @@ describe API::V3::Projects do expect(response).to have_gitlab_http_status(201) project.each_pair do |k, v| next if %i[has_external_issue_tracker path].include?(k) + expect(json_response[k.to_s]).to eq(v) end end diff --git a/spec/requests/rack_attack_global_spec.rb b/spec/requests/rack_attack_global_spec.rb new file mode 100644 index 00000000000..0fec14d0cce --- /dev/null +++ b/spec/requests/rack_attack_global_spec.rb @@ -0,0 +1,362 @@ +require 'spec_helper' + +describe 'Rack Attack global throttles' do + let(:settings) { Gitlab::CurrentSettings.current_application_settings } + + # Start with really high limits and override them with low limits to ensure + # the right settings are being exercised + let(:settings_to_set) do + { + throttle_unauthenticated_requests_per_period: 100, + throttle_unauthenticated_period_in_seconds: 1, + throttle_authenticated_api_requests_per_period: 100, + throttle_authenticated_api_period_in_seconds: 1, + throttle_authenticated_web_requests_per_period: 100, + throttle_authenticated_web_period_in_seconds: 1 + } + end + + let(:requests_per_period) { 1 } + 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(: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 + + # Make time-dependent tests deterministic + Timecop.freeze { example.run } + + 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 + 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 + before do + # Set low limits + settings_to_set[:throttle_unauthenticated_requests_per_period] = requests_per_period + settings_to_set[:throttle_unauthenticated_period_in_seconds] = period_in_seconds + end + + context 'when the throttle is enabled' do + before do + settings_to_set[:throttle_unauthenticated_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_does_not_require_authentication + expect(response).to have_http_status 200 + end + + # the last straw + expect_rejection { get url_that_does_not_require_authentication } + end + + it 'allows requests after throttling and then waiting for the next period' do + requests_per_period.times do + get url_that_does_not_require_authentication + expect(response).to have_http_status 200 + end + + expect_rejection { get url_that_does_not_require_authentication } + + Timecop.travel(period.from_now) do + requests_per_period.times do + get url_that_does_not_require_authentication + expect(response).to have_http_status 200 + end + + expect_rejection { get url_that_does_not_require_authentication } + end + end + + it 'counts requests from different IPs separately' do + requests_per_period.times do + get url_that_does_not_require_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') + + # would be over limit for the same IP + get url_that_does_not_require_authentication + expect(response).to have_http_status 200 + end + end + + context 'when the throttle is disabled' do + before do + settings_to_set[:throttle_unauthenticated_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_does_not_require_authentication + expect(response).to have_http_status 200 + end + end + end + end + + describe 'API requests authenticated with personal access token', :api do + let(:user) { create(:user) } + let(:token) { create(:personal_access_token, user: user) } + let(:other_user) { create(:user) } + let(:other_user_token) { create(:personal_access_token, user: other_user) } + let(:throttle_setting_prefix) { 'throttle_authenticated_api' } + + context 'with the token in the query string' do + let(:get_args) { [api(api_partial_url, personal_access_token: token)] } + let(:other_user_get_args) { [api(api_partial_url, personal_access_token: other_user_token)] } + + it_behaves_like 'rate-limited token-authenticated requests' + end + + context 'with the token in the headers' do + let(:get_args) { api_get_args_with_token_headers(api_partial_url, personal_access_token_headers(token)) } + let(:other_user_get_args) { api_get_args_with_token_headers(api_partial_url, personal_access_token_headers(other_user_token)) } + + it_behaves_like 'rate-limited token-authenticated requests' + end + end + + describe 'API requests authenticated with OAuth token', :api 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' } + + context 'with the token in the query string' do + let(:get_args) { [api(api_partial_url, oauth_access_token: token)] } + let(:other_user_get_args) { [api(api_partial_url, oauth_access_token: other_user_token)] } + + it_behaves_like 'rate-limited token-authenticated requests' + end + + context 'with the token in the headers' do + let(:get_args) { api_get_args_with_token_headers(api_partial_url, oauth_token_headers(token)) } + let(:other_user_get_args) { api_get_args_with_token_headers(api_partial_url, oauth_token_headers(other_user_token)) } + + it_behaves_like 'rate-limited token-authenticated requests' + end + end + + describe '"web" (non-API) requests authenticated with RSS token' do + let(:user) { create(:user) } + let(:other_user) { create(:user) } + let(:throttle_setting_prefix) { 'throttle_authenticated_web' } + + context 'with the token in the query string' do + let(:get_args) { [rss_url(user), nil] } + let(:other_user_get_args) { [rss_url(other_user), nil] } + + it_behaves_like 'rate-limited token-authenticated requests' + end + end + + describe 'web requests authenticated with regular login' do + let(:user) { create(:user) } + + 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 + 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 + end + + def api_get_args_with_token_headers(partial_url, token_headers) + ["/api/#{API::API.version}#{partial_url}", nil, token_headers] + end + + def rss_url(user) + "/dashboard/projects.atom?rss_token=#{user.rss_token}" + end + + def private_token_headers(user) + { 'HTTP_PRIVATE_TOKEN' => user.private_token } + end + + def personal_access_token_headers(personal_access_token) + { 'HTTP_PRIVATE_TOKEN' => personal_access_token.token } + end + + def oauth_token_headers(oauth_access_token) + { 'AUTHORIZATION' => "Bearer #{oauth_access_token.token}" } + end + + def expect_rejection(&block) + yield + + expect(response).to have_http_status(429) + end +end |