Homogenising the type of the request handled by UserAuthFinder. Also tests fixed

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|

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

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

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'