Merge branch 'dm-remove-private-token' into 'master'

Remove Private Tokens

Closes #38595 and #38447

See merge request gitlab-org/gitlab-ce!14838

9 files changed, 57 insertions, 140 deletions

diff --git a/lib/api/api.rb b/lib/api/api.rb
index 7db18e25a5f..c37e596eb9d 100644
--- a/lib/api/api.rb
+++ b/lib/api/api.rb
@@ -142,7 +142,6 @@ module API
     mount ::API::Runner
     mount ::API::Runners
     mount ::API::Services
-    mount ::API::Session
     mount ::API::Settings
     mount ::API::SidekiqMetrics
     mount ::API::Snippets
diff --git a/lib/api/api_guard.rb b/lib/api/api_guard.rb
index 87b9db66efd..b9c7d443f6c 100644
--- a/lib/api/api_guard.rb
+++ b/lib/api/api_guard.rb
@@ -42,72 +42,42 @@ module API
 
     # Helper Methods for Grape Endpoint
     module HelperMethods
-      def find_current_user
-        user =
-          find_user_from_private_token ||
-          find_user_from_oauth_token ||
-          find_user_from_warden
+      def find_current_user!
+        user = find_user_from_access_token || find_user_from_warden
+        return unless user
 
-        return nil unless user
-
-        raise UnauthorizedError unless Gitlab::UserAccess.new(user).allowed? && user.can?(:access_api)
+        forbidden!('User is blocked') unless Gitlab::UserAccess.new(user).allowed? && user.can?(:access_api)
 
         user
       end
 
-      def private_token
-        params[PRIVATE_TOKEN_PARAM] || env[PRIVATE_TOKEN_HEADER]
-      end
-
-      private
-
-      def find_user_from_private_token
-        token_string = private_token.to_s
-        return nil unless token_string.present?
+      def access_token
+        return @access_token if defined?(@access_token)
 
-        user =
-          find_user_by_authentication_token(token_string) ||
-          find_user_by_personal_access_token(token_string)
-
-        raise UnauthorizedError unless user
-
-        user
+        @access_token = find_oauth_access_token || find_personal_access_token
       end
 
-      # Invokes the doorkeeper guard.
-      #
-      # If token is presented and valid, then it sets @current_user.
-      #
-      # If the token does not have sufficient scopes to cover the requred scopes,
-      # then it raises InsufficientScopeError.
-      #
-      # If the token is expired, then it raises ExpiredError.
-      #
-      # If the token is revoked, then it raises RevokedError.
-      #
-      # If the token is not found (nil), then it returns nil
-      #
-      # Arguments:
-      #
-      #   scopes: (optional) scopes required for this guard.
-      #           Defaults to empty array.
-      #
-      def find_user_from_oauth_token
-        access_token = find_oauth_access_token
+      def validate_access_token!(scopes: [])
         return unless access_token
 
-        find_user_by_access_token(access_token)
+        case AccessTokenValidationService.new(access_token, request: request).validate(scopes: scopes)
+        when AccessTokenValidationService::INSUFFICIENT_SCOPE
+          raise InsufficientScopeError.new(scopes)
+        when AccessTokenValidationService::EXPIRED
+          raise ExpiredError
+        when AccessTokenValidationService::REVOKED
+          raise RevokedError
+        end
       end
 
-      def find_user_by_authentication_token(token_string)
-        User.find_by_authentication_token(token_string)
-      end
+      private
 
-      def find_user_by_personal_access_token(token_string)
-        access_token = PersonalAccessToken.find_by_token(token_string)
+      def find_user_from_access_token
         return unless access_token
 
-        find_user_by_access_token(access_token)
+        validate_access_token!
+
+        access_token.user || raise(UnauthorizedError)
       end
 
       # Check the Rails session for valid authentication details
@@ -125,34 +95,26 @@ module API
       end
 
       def find_oauth_access_token
-        return @oauth_access_token if defined?(@oauth_access_token)
-
         token = Doorkeeper::OAuth::Token.from_request(doorkeeper_request, *Doorkeeper.configuration.access_token_methods)
-        return @oauth_access_token = nil unless token
+        return unless token
 
-        @oauth_access_token = OauthAccessToken.by_token(token)
-        raise UnauthorizedError unless @oauth_access_token
+        # Expiration, revocation and scopes are verified in `find_user_by_access_token`
+        access_token = OauthAccessToken.by_token(token)
+        raise UnauthorizedError unless access_token
 
-        @oauth_access_token.revoke_previous_refresh_token!
-        @oauth_access_token
+        access_token.revoke_previous_refresh_token!
+        access_token
       end
 
-      def find_user_by_access_token(access_token)
-        scopes = scopes_registered_for_endpoint
+      def find_personal_access_token
+        token = (params[PRIVATE_TOKEN_PARAM] || env[PRIVATE_TOKEN_HEADER]).to_s
+        return unless token.present?
 
-        case AccessTokenValidationService.new(access_token, request: request).validate(scopes: scopes)
-        when AccessTokenValidationService::INSUFFICIENT_SCOPE
-          raise InsufficientScopeError.new(scopes)
-
-        when AccessTokenValidationService::EXPIRED
-          raise ExpiredError
+        # Expiration, revocation and scopes are verified in `find_user_by_access_token`
+        access_token = PersonalAccessToken.find_by(token: token)
+        raise UnauthorizedError unless access_token
 
-        when AccessTokenValidationService::REVOKED
-          raise RevokedError
-
-        when AccessTokenValidationService::VALID
-          access_token.user
-        end
+        access_token
       end
 
       def doorkeeper_request
@@ -236,7 +198,7 @@ module API
     class InsufficientScopeError < StandardError
       attr_reader :scopes
       def initialize(scopes)
-        @scopes = scopes
+        @scopes = scopes.map { |s| s.try(:name) || s }
       end
     end
   end
diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index efe874b2e6b..67cecb6a7ad 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -57,10 +57,6 @@ module API
       expose :admin?, as: :is_admin
     end
 
-    class UserWithPrivateDetails < UserWithAdmin
-      expose :private_token
-    end
-
     class Email < Grape::Entity
       expose :id, :email
     end
diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb
index 7a2ec865860..1c12166e434 100644
--- a/lib/api/helpers.rb
+++ b/lib/api/helpers.rb
@@ -41,6 +41,8 @@ module API
 
       sudo!
 
+      validate_access_token!(scopes: scopes_registered_for_endpoint) unless sudo?
+
       @current_user
     end
 
@@ -385,7 +387,7 @@ module API
       return @initial_current_user if defined?(@initial_current_user)
 
       begin
-        @initial_current_user = Gitlab::Auth::UniqueIpsLimiter.limit_user! { find_current_user }
+        @initial_current_user = Gitlab::Auth::UniqueIpsLimiter.limit_user! { find_current_user! }
       rescue APIGuard::UnauthorizedError
         unauthorized!
       end
@@ -393,24 +395,23 @@ module API
 
     def sudo!
       return unless sudo_identifier
-      return unless initial_current_user
+
+      unauthorized! unless initial_current_user
 
       unless initial_current_user.admin?
         forbidden!('Must be admin to use sudo')
       end
 
-      # Only private tokens should be used for the SUDO feature
-      unless private_token == initial_current_user.private_token
-        forbidden!('Private token must be specified in order to use sudo')
+      unless access_token
+        forbidden!('Must be authenticated using an OAuth or Personal Access Token to use sudo')
       end
 
+      validate_access_token!(scopes: [:sudo])
+
       sudoed_user = find_user(sudo_identifier)
+      not_found!("User with ID or username '#{sudo_identifier}'") unless sudoed_user
 
-      if sudoed_user
-        @current_user = sudoed_user
-      else
-        not_found!("No user id or username for: #{sudo_identifier}")
-      end
+      @current_user = sudoed_user
     end
 
     def sudo_identifier
diff --git a/lib/api/session.rb b/lib/api/session.rb
deleted file mode 100644
index 016415c3023..00000000000
--- a/lib/api/session.rb
+++ /dev/null
@@ -1,20 +0,0 @@
-module API
-  class Session < Grape::API
-    desc 'Login to get token' do
-      success Entities::UserWithPrivateDetails
-    end
-    params do
-      optional :login, type: String, desc: 'The username'
-      optional :email, type: String, desc: 'The email of the user'
-      requires :password, type: String, desc: 'The password of the user'
-      at_least_one_of :login, :email
-    end
-    post "/session" do
-      user = Gitlab::Auth.find_with_user_password(params[:email] || params[:login], params[:password])
-
-      return unauthorized! unless user
-      return render_api_error!('401 Unauthorized. You have 2FA enabled. Please use a personal access token to access the API', 401) if user.two_factor_enabled?
-      present user, with: Entities::UserWithPrivateDetails
-    end
-  end
-end
diff --git a/lib/api/users.rb b/lib/api/users.rb
index b6f97a1eac2..d80b364bd09 100644
--- a/lib/api/users.rb
+++ b/lib/api/users.rb
@@ -507,9 +507,7 @@ module API
       end
       get do
         entity =
-          if sudo?
-            Entities::UserWithPrivateDetails
-          elsif current_user.admin?
+          if current_user.admin?
             Entities::UserWithAdmin
           else
             Entities::UserPublic
diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb
index 87aeb76b66a..0ad9285c0ea 100644
--- a/lib/gitlab/auth.rb
+++ b/lib/gitlab/auth.rb
@@ -1,11 +1,11 @@
 module Gitlab
   module Auth
-    MissingPersonalTokenError = Class.new(StandardError)
+    MissingPersonalAccessTokenError = Class.new(StandardError)
 
     REGISTRY_SCOPES = [:read_registry].freeze
 
     # Scopes used for GitLab API access
-    API_SCOPES = [:api, :read_user].freeze
+    API_SCOPES = [:api, :read_user, :sudo].freeze
 
     # Scopes used for OpenID Connect
     OPENID_SCOPES = [:openid].freeze
@@ -38,7 +38,7 @@ module Gitlab
 
         # If sign-in is disabled and LDAP is not configured, recommend a
         # personal access token on failed auth attempts
-        raise Gitlab::Auth::MissingPersonalTokenError
+        raise Gitlab::Auth::MissingPersonalAccessTokenError
       end
 
       def find_with_user_password(login, password)
@@ -106,7 +106,7 @@ module Gitlab
         user = find_with_user_password(login, password)
         return unless user
 
-        raise Gitlab::Auth::MissingPersonalTokenError if user.two_factor_enabled?
+        raise Gitlab::Auth::MissingPersonalAccessTokenError if user.two_factor_enabled?
 
         Gitlab::Auth::Result.new(user, nil, :gitlab_or_ldap, full_authentication_abilities)
       end
@@ -128,7 +128,7 @@ module Gitlab
         token = PersonalAccessTokensFinder.new(state: 'active').find_by(token: password)
 
         if token && valid_scoped_token?(token, available_scopes)
-          Gitlab::Auth::Result.new(token.user, nil, :personal_token, abilities_for_scope(token.scopes))
+          Gitlab::Auth::Result.new(token.user, nil, :personal_access_token, abilities_for_scope(token.scopes))
         end
       end
 
@@ -226,8 +226,10 @@ module Gitlab
         []
       end
 
-      def available_scopes
-        API_SCOPES + registry_scopes
+      def available_scopes(current_user = nil)
+        scopes = API_SCOPES + registry_scopes
+        scopes.delete(:sudo) if current_user && !current_user.admin?
+        scopes
       end
 
       # Other available scopes
diff --git a/lib/tasks/gitlab/users.rake b/lib/tasks/gitlab/users.rake
deleted file mode 100644
index 3a16ace60bd..00000000000
--- a/lib/tasks/gitlab/users.rake
+++ /dev/null
@@ -1,11 +0,0 @@
-namespace :gitlab do
-  namespace :users do
-    desc "GitLab | Clear the authentication token for all users"
-    task clear_all_authentication_tokens: :environment  do |t, args|
-      # Do small batched updates because these updates will be slow and locking
-      User.select(:id).find_in_batches(batch_size: 100) do |batch|
-        User.where(id: batch.map(&:id)).update_all(authentication_token: nil)
-      end
-    end
-  end
-end
diff --git a/lib/tasks/tokens.rake b/lib/tasks/tokens.rake
index ad1818ff1fa..693597afdf8 100644
--- a/lib/tasks/tokens.rake
+++ b/lib/tasks/tokens.rake
@@ -1,12 +1,7 @@
 require_relative '../../app/models/concerns/token_authenticatable.rb'
 
 namespace :tokens do
-  desc "Reset all GitLab user auth tokens"
-  task reset_all_auth: :environment do
-    reset_all_users_token(:reset_authentication_token!)
-  end
-
-  desc "Reset all GitLab email tokens"
+  desc "Reset all GitLab incoming email tokens"
   task reset_all_email: :environment do
     reset_all_users_token(:reset_incoming_email_token!)
   end
@@ -31,11 +26,6 @@ class TmpUser < ActiveRecord::Base
 
   self.table_name = 'users'
 
-  def reset_authentication_token!
-    write_new_token(:authentication_token)
-    save!(validate: false)
-  end
-
   def reset_incoming_email_token!
     write_new_token(:incoming_email_token)
     save!(validate: false)