Add config to disable impersonationif-40385-prohibit_impersonation

Adds gitlab.impersonation_enabled config option defaulting to true to
keep the current default behaviour.

Only the act of impersonation is modified, impersonation token
management is not affected.

17 files changed, 213 insertions, 43 deletions

diff --git a/app/controllers/admin/impersonations_controller.rb b/app/controllers/admin/impersonations_controller.rb
index 08d7e3b4fa2..65fe22bd8f4 100644
--- a/app/controllers/admin/impersonations_controller.rb
+++ b/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
diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb
index b783c0e2a6f..e93be1c1ba2 100644
--- a/app/controllers/admin/users_controller.rb
+++ b/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
diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 9b40ffb26a2..8c794b903b0 100644
--- a/app/controllers/application_controller.rb
+++ b/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
 
@@ -483,4 +484,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
diff --git a/app/helpers/users_helper.rb b/app/helpers/users_helper.rb
index 42b533ad772..bde9ca0cbf2 100644
--- a/app/helpers/users_helper.rb
+++ b/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
diff --git a/app/services/access_token_validation_service.rb b/app/services/access_token_validation_service.rb
index 2a337918d21..40aa9250885 100644
--- a/app/services/access_token_validation_service.rb
+++ b/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
diff --git a/app/views/admin/users/_head.html.haml b/app/views/admin/users/_head.html.haml
index bfbc16d37a0..a733f420d11 100644
--- a/app/views/admin/users/_head.html.haml
+++ b/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
diff --git a/changelogs/unreleased/40385-prohibit_impersonation.yml b/changelogs/unreleased/40385-prohibit_impersonation.yml
new file mode 100644
index 00000000000..dd061b17939
--- /dev/null
+++ b/changelogs/unreleased/40385-prohibit_impersonation.yml
@@ -0,0 +1,5 @@
+---
+title: Add config to prohibit impersonation
+merge_request: 23338
+author:
+type: added
diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example
index 09e21b2c6f2..58b7c248aaf 100644
--- a/config/gitlab.yml.example
+++ b/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
diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb
index bd02b85c7ce..82e3b490378 100644
--- a/config/initializers/1_settings.rb
+++ b/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?
 
 #
diff --git a/doc/api/README.md b/doc/api/README.md
index 0e8d1aaf86e..848a6e6b72b 100644
--- a/doc/api/README.md
+++ b/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"
 }
 ```
diff --git a/lib/api/api_guard.rb b/lib/api/api_guard.rb
index 61357b3f1d6..af9b519ed9e 100644
--- a/lib/api/api_guard.rb
+++ b/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.
diff --git a/lib/gitlab/auth/user_auth_finders.rb b/lib/gitlab/auth/user_auth_finders.rb
index c304adc64db..bcd277683ba 100644
--- a/lib/gitlab/auth/user_auth_finders.rb
+++ b/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
@@ -56,6 +57,8 @@ module Gitlab
           raise ExpiredError
         when AccessTokenValidationService::REVOKED
           raise RevokedError
+        when AccessTokenValidationService::IMPERSONATION_DISABLED
+          raise ImpersonationDisabled
         end
       end
 
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 5ad0b21b2e4..2da3e363ab7 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -3439,6 +3439,9 @@ msgstr ""
 msgid "ImageDiffViewer|Swipe"
 msgstr ""
 
+msgid "Impersonation has been disabled"
+msgstr ""
+
 msgid "Import"
 msgstr ""
 
diff --git a/spec/controllers/admin/users_controller_spec.rb b/spec/controllers/admin/users_controller_spec.rb
index f350641a643..3dd0b2623ac 100644
--- a/spec/controllers/admin/users_controller_spec.rb
+++ b/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
diff --git a/spec/features/admin/admin_users_spec.rb b/spec/features/admin/admin_users_spec.rb
index f7c7a257075..d5516b334b9 100644
--- a/spec/features/admin/admin_users_spec.rb
+++ b/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
 
diff --git a/spec/lib/gitlab/auth/user_auth_finders_spec.rb b/spec/lib/gitlab/auth/user_auth_finders_spec.rb
index 454ad1589b9..fc7fc1da674 100644
--- a/spec/lib/gitlab/auth/user_auth_finders_spec.rb
+++ b/spec/lib/gitlab/auth/user_auth_finders_spec.rb
@@ -220,5 +220,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
diff --git a/spec/requests/api/helpers_spec.rb b/spec/requests/api/helpers_spec.rb
index cca449e9e56..2c40e266f5f 100644
--- a/spec/requests/api/helpers_spec.rb
+++ b/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