Applied some code review comments

4 files changed, 29 insertions, 32 deletions

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index bab5ae05ce9..488b50426fe 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -99,8 +99,7 @@ class ApplicationController < ActionController::Base
     return try(:authenticated_user)
   end
 
-  # This filter handles private tokens, personal access tokens, and atom
-  # requests with rss tokens
+  # This filter handles personal access tokens, and atom requests with rss tokens
   def authenticate_sessionless_user!
     user = Gitlab::Auth::RequestAuthenticator.new(request).find_sessionless_user
 
diff --git a/lib/api/api_guard.rb b/lib/api/api_guard.rb
index 9c68830ae34..01e15ffee84 100644
--- a/lib/api/api_guard.rb
+++ b/lib/api/api_guard.rb
@@ -45,7 +45,6 @@ module API
       include Gitlab::Utils::StrongMemoize
 
       def find_current_user!
-        set_raise_unauthorized_error
         user = find_user_from_access_token || find_user_from_warden
         return unless user
 
@@ -75,10 +74,6 @@ module API
 
       private
 
-      def private_token
-        params[PRIVATE_TOKEN_PARAM].presence || env[PRIVATE_TOKEN_HEADER].presence
-      end
-
       # An array of scopes that were registered (using `allow_access_with_scope`)
       # for the current endpoint class. It also returns scopes registered on
       # `API::API`, since these are meant to apply to all API routes.
diff --git a/lib/gitlab/auth/request_authenticator.rb b/lib/gitlab/auth/request_authenticator.rb
index eb16701bad5..1490136ee4f 100644
--- a/lib/gitlab/auth/request_authenticator.rb
+++ b/lib/gitlab/auth/request_authenticator.rb
@@ -7,8 +7,10 @@ module Gitlab
 
       attr_reader :request
 
+      delegate :params, :env, to: :request
+
       def initialize(request)
-        @request = ensure_action_dispatch_request(request)
+        @request = request
       end
 
       def user
@@ -16,7 +18,9 @@ module Gitlab
       end
 
       def find_sessionless_user
-        find_user_from_access_token || find_user_by_rss_token
+        find_user_from_access_token || find_user_from_rss_token
+      rescue StandardError
+        nil
       end
     end
   end
diff --git a/lib/gitlab/auth/user_auth_finders.rb b/lib/gitlab/auth/user_auth_finders.rb
index 86f1c13d4b8..dbe2a3a27d1 100644
--- a/lib/gitlab/auth/user_auth_finders.rb
+++ b/lib/gitlab/auth/user_auth_finders.rb
@@ -1,16 +1,19 @@
 module Gitlab
   module Auth
     module UserAuthFinders
+      PRIVATE_TOKEN_HEADER = 'HTTP_PRIVATE_TOKEN'.freeze
+      PRIVATE_TOKEN_PARAM = :private_token
+
       # Check the Rails session for valid authentication details
       def find_user_from_warden
-        request.env['warden']&.authenticate if verified_request?
+        env['warden']&.authenticate if verified_request?
       end
 
-      def find_user_by_rss_token
+      def find_user_from_rss_token
         return unless request.format.atom?
 
-        token = request.params[:rss_token].presence
-        return unless token.present?
+        token = params[:rss_token].presence
+        return unless token
 
         handle_return_value!(User.find_by_rss_token(token))
       end
@@ -24,14 +27,22 @@ module Gitlab
       end
 
       def validate_access_token!(scopes: [])
+        return unless access_token
+
+        case AccessTokenValidationService.new(access_token, request: request).validate(scopes: scopes)
+        when AccessTokenValidationService::INSUFFICIENT_SCOPE
+          raise API::APIGuard::InsufficientScopeError.new(scopes)
+        when AccessTokenValidationService::EXPIRED
+          raise API::APIGuard::ExpiredError
+        when AccessTokenValidationService::REVOKED
+          raise API::APIGuard::RevokedError
+        end
       end
 
       private
 
       def handle_return_value!(value, &block)
-        unless value
-          raise_unauthorized_error? ? raise_unauthorized_error! : return
-        end
+        raise API::APIGuard::UnauthorizedError unless value
 
         block_given? ? yield(value) : value
       end
@@ -43,13 +54,13 @@ module Gitlab
       end
 
       def private_token
-        request.params[:private_token].presence ||
-          request.headers['PRIVATE-TOKEN'].presence
+        params[PRIVATE_TOKEN_PARAM].presence ||
+          env[PRIVATE_TOKEN_HEADER].presence
       end
 
       def find_personal_access_token
-        token = private_token.to_s
-        return unless token.present?
+        token = private_token
+        return unless token
 
         # Expiration, revocation and scopes are verified in `validate_access_token!`
         handle_return_value!(PersonalAccessToken.find_by(token: token))
@@ -77,18 +88,6 @@ module Gitlab
 
         ActionDispatch::Request.new(request.env)
       end
-
-      def raise_unauthorized_error?
-        defined?(@raise_unauthorized_error) ? @raise_unauthorized_error : false
-      end
-
-      def set_raise_unauthorized_error
-        @raise_unauthorized_error = true
-      end
-
-      def raise_unauthorized_error!
-        raise API::APIGuard::UnauthorizedError
-      end
     end
   end
 end