diff --git a/lib/api/api_guard.rb b/lib/api/api_guard.rb index 0a93e71858e..66ad2b77f75 100644 --- a/lib/api/api_guard.rb +++ b/lib/api/api_guard.rb @@ -72,8 +72,6 @@ module API end end - private - def raise_unauthorized_error! raise UnauthorizedError end diff --git a/lib/gitlab/auth/user_auth_finders.rb b/lib/gitlab/auth/user_auth_finders.rb index 2f4aeff14ac..d1f5bf84873 100644 --- a/lib/gitlab/auth/user_auth_finders.rb +++ b/lib/gitlab/auth/user_auth_finders.rb @@ -19,22 +19,6 @@ module Gitlab user end - def private_token - request.params[:private_token].presence || - request.headers['PRIVATE-TOKEN'].presence - end - - def find_user_by_authentication_token(token_string) - User.find_by_authentication_token(token_string) - end - - def find_user_by_personal_access_token(token_string) - access_token = PersonalAccessToken.find_by_token(token_string) - return unless access_token - - find_user_by_access_token(access_token) - end - def find_user_by_rss_token return unless request.path.ends_with?('atom') || request.format.atom? @@ -55,6 +39,24 @@ module Gitlab find_user_by_access_token(access_token) end + private + + def private_token + request.params[:private_token].presence || + request.headers['PRIVATE-TOKEN'].presence + end + + def find_user_by_authentication_token(token_string) + User.find_by_authentication_token(token_string) + end + + def find_user_by_personal_access_token(token_string) + access_token = PersonalAccessToken.find_by_token(token_string) + return unless access_token + + find_user_by_access_token(access_token) + end + def find_oauth_access_token return @oauth_access_token if defined?(@oauth_access_token) diff --git a/spec/requests/rack_attack_global_spec.rb b/spec/requests/rack_attack_global_spec.rb index 6984ff4ab36..9a0513d66a7 100644 --- a/spec/requests/rack_attack_global_spec.rb +++ b/spec/requests/rack_attack_global_spec.rb @@ -1,27 +1,6 @@ require 'spec_helper' describe 'Rack Attack global throttles' do - # If the tests are being flaky as described below, then this constant - # can be set to greater than 1 to make multiple attempts to get a 429. - # - # In tests exceeding the rate limit within a time period (which we know we - # have accomplished because we've made exactly 1 more request than allowed - # while time is stopped) we expect a 429. But sometimes we get a 200, - # sometimes for more than one request, but eventually we get a 429. This - # constant and its usages should be removed if we figure out why this happens. - # More on this: - # https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/14708#note_45151688 - NUM_TRIES_FOR_REJECTION = 1 - - # Extra time travel past what should be strictly necessary to ensure the - # throttle we are testing is using a cache key where the request count is 0. - # - # Why add this? Sometimes we get a 429 when we expect a 200. This constant and - # its usages should be removed if we figure out why this happens. - # More on this: - # https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/14708#note_45151688 - NEXT_TIME_PERIOD_BUFFER = 0.seconds - let(:settings) { Gitlab::CurrentSettings.current_application_settings } # Start with really high limits and override them with low limits to ensure @@ -405,11 +384,7 @@ describe 'Rack Attack global throttles' do end def expect_rejection(&block) - # NUM_TRIES_FOR_REJECTION.times do |i| - yield - # break if response.status == 429 # success - # Rails.logger.warn "Flaky test expected HTTP status 429 but got #{response.status}. Will attempt again (#{i + 1}/#{NUM_TRIES_FOR_REJECTION})" if i + 1 < NUM_TRIES_FOR_REJECTION - # end + yield expect(response).to have_http_status(429) end