Merge branch 'if-40385-prohibit_impersonation' into 'master'

Add config to prohibit impersonation

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

app/controllers/admin/impersonations_controller.rb

@@ -5,23 +5,12 @@ class Admin::ImpersonationsController < Admin::ApplicationController
   before_action :authenticate_impersonator!
 
   def destroy
-    original_user = current_user
-
-    warden.set_user(impersonator, scope: :user)
-
-    Gitlab::AppLogger.info("User #{impersonator.username} has stopped impersonating #{original_user.username}")
-
-    session[:impersonator_id] = nil
-
+    original_user = stop_impersonation
     redirect_to admin_user_path(original_user), status: :found
   end
 
   private
 
-  def impersonator
-    @impersonator ||= User.find(session[:impersonator_id]) if session[:impersonator_id]
-  end
-
   def authenticate_impersonator!
     render_404 unless impersonator && impersonator.admin? && !impersonator.blocked?
   end

app/controllers/admin/users_controller.rb

@@ -2,6 +2,7 @@
 
 class Admin::UsersController < Admin::ApplicationController
   before_action :user, except: [:index, :new, :create]
+  before_action :check_impersonation_availability, only: :impersonate
 
   def index
     @users = User.order_name_asc.filter(params[:filter])
@@ -227,4 +228,8 @@ class Admin::UsersController < Admin::ApplicationController
 
     result[:status] == :success
   end
+
+  def check_impersonation_availability
+    access_denied! unless Gitlab.config.gitlab.impersonation_enabled
+  end
 end

app/controllers/application_controller.rb

@@ -28,6 +28,7 @@ class ApplicationController < ActionController::Base
   before_action :configure_permitted_parameters, if: :devise_controller?
   before_action :require_email, unless: :devise_controller?
   before_action :set_usage_stats_consent_flag
+  before_action :check_impersonation_availability
 
   around_action :set_locale
 
@@ -462,4 +463,28 @@ class ApplicationController < ActionController::Base
       .new(settings, current_user, application_setting_params)
       .execute
   end
+
+  def check_impersonation_availability
+    return unless session[:impersonator_id]
+
+    unless Gitlab.config.gitlab.impersonation_enabled
+      stop_impersonation
+      access_denied! _('Impersonation has been disabled')
+    end
+  end
+
+  def stop_impersonation
+    impersonated_user = current_user
+
+    Gitlab::AppLogger.info("User #{impersonator.username} has stopped impersonating #{impersonated_user.username}")
+
+    warden.set_user(impersonator, scope: :user)
+    session[:impersonator_id] = nil
+
+    impersonated_user
+  end
+
+  def impersonator
+    @impersonator ||= User.find(session[:impersonator_id]) if session[:impersonator_id]
+  end
 end

app/helpers/users_helper.rb

@@ -70,6 +70,10 @@ module UsersHelper
     end
   end
 
+  def impersonation_enabled?
+    Gitlab.config.gitlab.impersonation_enabled
+  end
+
   private
 
   def get_profile_tabs

app/services/access_token_validation_service.rb

@@ -6,6 +6,7 @@ class AccessTokenValidationService
   EXPIRED = :expired
   REVOKED = :revoked
   INSUFFICIENT_SCOPE = :insufficient_scope
+  IMPERSONATION_DISABLED = :impersonation_disabled
 
   attr_reader :token, :request
 
@@ -24,6 +25,11 @@ class AccessTokenValidationService
     elsif !self.include_any_scope?(scopes)
       return INSUFFICIENT_SCOPE
 
+    elsif token.respond_to?(:impersonation) &&
+        token.impersonation &&
+        !Gitlab.config.gitlab.impersonation_enabled
+      return IMPERSONATION_DISABLED
+
     else
       return VALID
     end

app/views/admin/users/_head.html.haml

@@ -8,7 +8,7 @@
     %span.cred (Admin)
 
   .float-right
-    - if @user != current_user && @user.can?(:log_in)
+    - if impersonation_enabled? && @user != current_user && @user.can?(:log_in)
       = link_to 'Impersonate', impersonate_admin_user_path(@user), method: :post, class: "btn btn-nr btn-grouped btn-info"
     = link_to edit_admin_user_path(@user), class: "btn btn-nr btn-grouped" do
       %i.fa.fa-pencil-square-o

changelogs/unreleased/40385-prohibit_impersonation.yml

+---
+title: Add config to prohibit impersonation
+merge_request: 23338
+author:
+type: added

config/gitlab.yml.example

@@ -114,6 +114,9 @@ production: &base
     # The default is 'shared/cache/archive/' relative to the root of the Rails app.
     # repository_downloads_path: shared/cache/archive/
 
+    ## Impersonation settings
+    impersonation_enabled: true
+
   ## Reply by email
   # Allow users to comment on issues and merge requests by replying to notification emails.
   # For documentation on how to set this up, see http://doc.gitlab.com/ce/administration/reply_by_email.html

config/initializers/1_settings.rb

@@ -153,6 +153,7 @@ Settings.gitlab['domain_whitelist'] ||= []
 Settings.gitlab['import_sources'] ||= Gitlab::ImportSources.values
 Settings.gitlab['trusted_proxies'] ||= []
 Settings.gitlab['no_todos_messages'] ||= YAML.load_file(Rails.root.join('config', 'no_todos_messages.yml'))
+Settings.gitlab['impersonation_enabled'] ||= true if Settings.gitlab['impersonation_enabled'].nil?
 Settings.gitlab['usage_ping_enabled'] = true if Settings.gitlab['usage_ping_enabled'].nil?
 
 #

doc/api/README.md

@@ -225,6 +225,43 @@ For more information, refer to the
 Impersonation tokens are used exactly like regular personal access tokens, and can be passed in either the
 `private_token` parameter or the `Private-Token` header.
 
+#### Disable impersonation
+
+> [Introduced](https://gitlab.com/gitlab-org/gitlab-ce/issues/40385) in GitLab
+11.6.
+
+By default, impersonation is enabled. To disable impersonation, GitLab must be
+reconfigured:
+
+**For Omnibus installations**
+
+1. Edit `/etc/gitlab/gitlab.rb`:
+
+    ```ruby
+    gitlab_rails['impersonation_enabled'] = false
+    ```
+
+1. Save the file and [reconfigure](../administration/restart_gitlab.md#omnibus-gitlab-reconfigure)
+   GitLab for the changes to take effect.
+
+To re-enable impersonation, remove this configuration and reconfigure GitLab.
+
+---
+
+**For installations from source**
+
+1. Edit `config/gitlab.yml`:
+
+    ```yaml
+    gitlab:
+      impersonation_enabled: false
+    ```
+
+1. Save the file and [restart](../administration/restart_gitlab.md#installations-from-source)
+   GitLab for the changes to take effect.
+
+To re-enable impersonation, remove this configuration and restart GitLab.
+
 ### Sudo
 
 NOTE: **Note:**
@@ -540,7 +577,7 @@ When you try to access an API URL that does not exist you will receive 404 Not F
 ```
 HTTP/1.1 404 Not Found
 Content-Type: application/json
-{
+{ f
     "error": "404 Not Found"
 }
 ```

lib/api/api_guard.rb

@@ -94,6 +94,7 @@ module API
                          Gitlab::Auth::TokenNotFoundError,
                          Gitlab::Auth::ExpiredError,
                          Gitlab::Auth::RevokedError,
+                         Gitlab::Auth::ImpersonationDisabled,
                          Gitlab::Auth::InsufficientScopeError]
 
         base.__send__(:rescue_from, *error_classes, oauth2_bearer_token_error_handler) # rubocop:disable GitlabSecurity/PublicSend
@@ -121,6 +122,11 @@ module API
                 :invalid_token,
                 "Token was revoked. You have to re-authorize from the user.")
 
+            when Gitlab::Auth::ImpersonationDisabled
+              Rack::OAuth2::Server::Resource::Bearer::Unauthorized.new(
+                :invalid_token,
+                "Token is an impersonation token but impersonation was disabled.")
+
             when Gitlab::Auth::InsufficientScopeError
               # FIXME: ForbiddenError (inherited from Bearer::Forbidden of Rack::Oauth2)
               # does not include WWW-Authenticate header, which breaks the standard.

lib/gitlab/auth/user_auth_finders.rb

@@ -7,6 +7,7 @@ module Gitlab
     TokenNotFoundError = Class.new(AuthenticationError)
     ExpiredError = Class.new(AuthenticationError)
     RevokedError = Class.new(AuthenticationError)
+    ImpersonationDisabled = Class.new(AuthenticationError)
     UnauthorizedError = Class.new(AuthenticationError)
 
     class InsufficientScopeError < AuthenticationError
@@ -67,6 +68,8 @@ module Gitlab
           raise ExpiredError
         when AccessTokenValidationService::REVOKED
           raise RevokedError
+        when AccessTokenValidationService::IMPERSONATION_DISABLED
+          raise ImpersonationDisabled
         end
       end
 

locale/gitlab.pot

@@ -3448,6 +3448,9 @@ msgstr ""
 msgid "ImageDiffViewer|Swipe"
 msgstr ""
 
+msgid "Impersonation has been disabled"
+msgstr ""
+
 msgid "Import"
 msgstr ""
 

spec/controllers/admin/users_controller_spec.rb

@@ -264,5 +264,17 @@ describe Admin::UsersController do
         expect(flash[:alert]).to eq("You are now impersonating #{user.username}")
       end
     end
+
+    context "when impersonation is disabled" do
+      before do
+        stub_config_setting(impersonation_enabled: false)
+      end
+
+      it "shows error page" do
+        post :impersonate, id: user.username
+
+        expect(response).to have_gitlab_http_status(404)
+      end
+    end
   end
 end

spec/features/admin/admin_users_spec.rb

@@ -205,75 +205,118 @@ describe "Admin::Users" do
     describe 'Impersonation' do
       let(:another_user) { create(:user) }
 
-      before do
-        visit admin_user_path(another_user)
-      end
-
       context 'before impersonating' do
-        it 'shows impersonate button for other users' do
-          expect(page).to have_content('Impersonate')
+        subject { visit admin_user_path(user_to_visit) }
+
+        let(:user_to_visit) { another_user }
+
+        context 'for other users' do
+          it 'shows impersonate button for other users' do
+            subject
+
+            expect(page).to have_content('Impersonate')
+          end
         end
 
-        it 'does not show impersonate button for admin itself' do
-          visit admin_user_path(current_user)
+        context 'for admin itself' do
+          let(:user_to_visit) { current_user }
 
-          expect(page).not_to have_content('Impersonate')
+          it 'does not show impersonate button for admin itself' do
+            subject
+
+            expect(page).not_to have_content('Impersonate')
+          end
         end
 
-        it 'does not show impersonate button for blocked user' do
-          another_user.block
+        context 'for blocked user' do
+          before do
+            another_user.block
+          end
 
-          visit admin_user_path(another_user)
+          it 'does not show impersonate button for blocked user' do
+            subject
 
-          expect(page).not_to have_content('Impersonate')
+            expect(page).not_to have_content('Impersonate')
+          end
+        end
+
+        context 'when impersonation is disabled' do
+          before do
+            stub_config_setting(impersonation_enabled: false)
+          end
 
-          another_user.activate
+          it 'does not show impersonate button' do
+            subject
+
+            expect(page).not_to have_content('Impersonate')
+          end
         end
       end
 
       context 'when impersonating' do
+        subject { click_link 'Impersonate' }
+
         before do
-          click_link 'Impersonate'
+          visit admin_user_path(another_user)
         end
 
         it 'logs in as the user when impersonate is clicked' do
+          subject
+
           expect(page.find(:css, '.header-user .profile-link')['data-user']).to eql(another_user.username)
         end
 
         it 'sees impersonation log out icon' do
-          icon = first('.fa.fa-user-secret')
+          subject
 
+          icon = first('.fa.fa-user-secret')
           expect(icon).not_to be nil
         end
 
-        it 'logs out of impersonated user back to original user' do
-          find(:css, 'li.impersonation a').click
-
-          expect(page.find(:css, '.header-user .profile-link')['data-user']).to eq(current_user.username)
-        end
+        context 'a user with an expired password' do
+          before do
+            another_user.update(password_expires_at: Time.now - 5.minutes)
+          end
 
-        it 'is redirected back to the impersonated users page in the admin after stopping' do
-          find(:css, 'li.impersonation a').click
+          it 'does not redirect to password change page' do
+            subject
 
-          expect(current_path).to eq("/admin/users/#{another_user.username}")
+            expect(current_path).to eq('/')
+          end
         end
       end
 
-      context 'when impersonating a user with an expired password' do
+      context 'ending impersonation' do
+        subject { find(:css, 'li.impersonation a').click }
+
         before do
-          another_user.update(password_expires_at: Time.now - 5.minutes)
+          visit admin_user_path(another_user)
           click_link 'Impersonate'
         end
 
-        it 'does not redirect to password change page' do
-          expect(current_path).to eq('/')
+        it 'logs out of impersonated user back to original user' do
+          subject
+
+          expect(page.find(:css, '.header-user .profile-link')['data-user']).to eq(current_user.username)
         end
 
         it 'is redirected back to the impersonated users page in the admin after stopping' do
-          find(:css, 'li.impersonation a').click
+          subject
 
           expect(current_path).to eq("/admin/users/#{another_user.username}")
         end
+
+        context 'a user with an expired password' do
+          before do
+            another_user.update(password_expires_at: Time.now - 5.minutes)
+          end
+
+          it 'is redirected back to the impersonated users page in the admin after stopping' do
+            subject
+
+            expect(current_path).to eq("/admin/users/#{another_user.username}")
+          end
+        end
       end
     end
 

spec/lib/gitlab/auth/user_auth_finders_spec.rb

@@ -279,5 +279,20 @@ describe Gitlab::Auth::UserAuthFinders do
         expect { validate_access_token!(scopes: [:sudo]) }.to raise_error(Gitlab::Auth::InsufficientScopeError)
       end
     end
+
+    context 'with impersonation token' do
+      let(:personal_access_token) { create(:personal_access_token, :impersonation, user: user) }
+
+      context 'when impersonation is disabled' do
+        before do
+          stub_config_setting(impersonation_enabled: false)
+          allow_any_instance_of(described_class).to receive(:access_token).and_return(personal_access_token)
+        end
+
+        it 'returns Gitlab::Auth::ImpersonationDisabled' do
+          expect { validate_access_token! }.to raise_error(Gitlab::Auth::ImpersonationDisabled)
+        end
+      end
+    end
   end
 end

spec/requests/api/helpers_spec.rb

@@ -206,6 +206,19 @@ describe API::Helpers do
 
         expect { current_user }.to raise_error Gitlab::Auth::ExpiredError
       end
+
+      context 'when impersonation is disabled' do
+        let(:personal_access_token) { create(:personal_access_token, :impersonation, user: user) }
+
+        before do
+          stub_config_setting(impersonation_enabled: false)
+          env[Gitlab::Auth::UserAuthFinders::PRIVATE_TOKEN_HEADER] = personal_access_token.token
+        end
+
+        it 'does not allow impersonation tokens' do
+          expect { current_user }.to raise_error Gitlab::Auth::ImpersonationDisabled
+        end
+      end
     end
   end