diff --git a/lib/api/api_guard.rb b/lib/api/api_guard.rb index 01e15ffee84..e2a1a51b300 100644 --- a/lib/api/api_guard.rb +++ b/lib/api/api_guard.rb @@ -6,9 +6,6 @@ module API module APIGuard extend ActiveSupport::Concern - PRIVATE_TOKEN_HEADER = "HTTP_PRIVATE_TOKEN".freeze - PRIVATE_TOKEN_PARAM = :private_token - included do |base| # OAuth2 Resource Server Authentication use Rack::OAuth2::Server::Resource::Bearer, 'The API' do |request| diff --git a/lib/gitlab/auth/request_authenticator.rb b/lib/gitlab/auth/request_authenticator.rb index 1490136ee4f..f500609d1a3 100644 --- a/lib/gitlab/auth/request_authenticator.rb +++ b/lib/gitlab/auth/request_authenticator.rb @@ -7,8 +7,6 @@ module Gitlab attr_reader :request - delegate :params, :env, to: :request - def initialize(request) @request = request end diff --git a/lib/gitlab/auth/user_auth_finders.rb b/lib/gitlab/auth/user_auth_finders.rb index dbe2a3a27d1..db900908ead 100644 --- a/lib/gitlab/auth/user_auth_finders.rb +++ b/lib/gitlab/auth/user_auth_finders.rb @@ -6,13 +6,13 @@ module Gitlab # Check the Rails session for valid authentication details def find_user_from_warden - env['warden']&.authenticate if verified_request? + current_request.env['warden']&.authenticate if verified_request? end def find_user_from_rss_token - return unless request.format.atom? + return unless current_request.format.atom? - token = params[:rss_token].presence + token = current_request.params[:rss_token].presence return unless token handle_return_value!(User.find_by_rss_token(token)) @@ -23,7 +23,7 @@ module Gitlab validate_access_token! - handle_return_value!(access_token&.user) + handle_return_value!(access_token.user) end def validate_access_token!(scopes: []) @@ -54,8 +54,8 @@ module Gitlab end def private_token - params[PRIVATE_TOKEN_PARAM].presence || - env[PRIVATE_TOKEN_HEADER].presence + current_request.params[PRIVATE_TOKEN_PARAM].presence || + current_request.env[PRIVATE_TOKEN_HEADER].presence end def find_personal_access_token @@ -67,7 +67,6 @@ module Gitlab end def find_oauth_access_token - current_request = ensure_action_dispatch_request(request) token = Doorkeeper::OAuth::Token.from_request(current_request, *Doorkeeper.configuration.access_token_methods) return unless token @@ -80,7 +79,7 @@ module Gitlab # Check if the request is GET/HEAD, or if CSRF token is valid. def verified_request? - Gitlab::RequestForgeryProtection.verified?(request.env) + Gitlab::RequestForgeryProtection.verified?(current_request.env) end def ensure_action_dispatch_request(request) @@ -88,6 +87,10 @@ module Gitlab ActionDispatch::Request.new(request.env) end + + def current_request + @current_request ||= ensure_action_dispatch_request(request) + end end end end diff --git a/spec/requests/api/helpers_spec.rb b/spec/requests/api/helpers_spec.rb index fc1444e4018..dbb82136919 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,11 +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({}) @@ -38,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 } @@ -133,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/ @@ -147,33 +152,33 @@ 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 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 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 end @@ -351,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' @@ -359,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' @@ -369,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 @@ -383,7 +388,7 @@ 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 @@ -397,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 @@ -421,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' @@ -429,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'