Merge branch 'security-not-allow-to-impersonate-tokens-while-impersonation-is-off' into 'master'

Block pushing with impersonation token if impersonation is disabled

See merge request gitlab-org/security/gitlab!1571

app/controllers/admin/impersonation_tokens_controller.rb

@@ -2,6 +2,7 @@
 
 class Admin::ImpersonationTokensController < Admin::ApplicationController
   before_action :user
+  before_action :verify_impersonation_enabled!
 
   feature_category :authentication_and_authorization
 
@@ -41,6 +42,10 @@ class Admin::ImpersonationTokensController < Admin::ApplicationController
   end
   # rubocop: enable CodeReuse/ActiveRecord
 
+  def verify_impersonation_enabled!
+    access_denied! unless helpers.impersonation_enabled?
+  end
+
   def finder(options = {})
     PersonalAccessTokensFinder.new({ user: user, impersonation: true }.merge(options))
   end

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

@@ -42,6 +42,7 @@
     = link_to _("SSH keys"), keys_admin_user_path(@user)
   = nav_link(controller: :identities) do
     = link_to _("Identities"), admin_user_identities_path(@user)
+  - if impersonation_enabled?
     = nav_link(controller: :impersonation_tokens) do
       = link_to _("Impersonation Tokens"), admin_user_impersonation_tokens_path(@user)
 .gl-mb-3

lib/gitlab/auth.rb

@@ -193,7 +193,10 @@ module Gitlab
       def personal_access_token_check(password, project)
         return unless password.present?
 
-        token = PersonalAccessTokensFinder.new(state: 'active').find_by_token(password)
+        finder_options = { state: 'active' }
+        finder_options[:impersonation] = false unless Gitlab.config.gitlab.impersonation_enabled
+
+        token = PersonalAccessTokensFinder.new(finder_options).find_by_token(password)
 
         return unless token
 

spec/features/admin/admin_users_impersonation_tokens_spec.rb

@@ -83,4 +83,16 @@ RSpec.describe 'Admin > Users > Impersonation Tokens', :js do
       expect(no_personal_access_tokens_message).to have_text("This user has no active impersonation tokens.")
     end
   end
+
+  describe "impersonation disabled state" do
+    before do
+      stub_config_setting(impersonation_enabled: false)
+    end
+
+    it "does not show impersonation tokens tab" do
+      visit admin_user_path(user)
+
+      expect(page).not_to have_content("Impersonation Tokens")
+    end
+  end
 end

spec/lib/gitlab/auth_spec.rb

@@ -336,6 +336,15 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do
         expect_results_with_abilities(impersonation_token, described_class.full_authentication_abilities)
       end
 
+      it 'fails if it is an impersonation token but impersonation is blocked' do
+        stub_config_setting(impersonation_enabled: false)
+
+        impersonation_token = create(:personal_access_token, :impersonation, scopes: ['api'])
+
+        expect(gl_auth.find_for_git_client('', impersonation_token.token, project: nil, ip: 'ip'))
+          .to eq(Gitlab::Auth::Result.new(nil, nil, nil, nil))
+      end
+
       it 'limits abilities based on scope' do
         personal_access_token = create(:personal_access_token, scopes: %w[read_user sudo])
 

spec/requests/admin/impersonation_tokens_controller_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Admin::ImpersonationTokensController, :enable_admin_mode do
+  let(:admin) { create(:admin) }
+  let!(:user) { create(:user) }
+
+  before do
+    sign_in(admin)
+  end
+
+  context "when impersonation is disabled" do
+    before do
+      stub_config_setting(impersonation_enabled: false)
+    end
+
+    it "shows error page for index page" do
+      get admin_user_impersonation_tokens_path(user_id: user.username)
+
+      expect(response).to have_gitlab_http_status(:not_found)
+    end
+
+    it "responds with 404 for create action" do
+      post admin_user_impersonation_tokens_path(user_id: user.username)
+
+      expect(response).to have_gitlab_http_status(:not_found)
+    end
+
+    it "responds with 404 for revoke action" do
+      token = create(:personal_access_token, :impersonation, user: user)
+
+      put revoke_admin_user_impersonation_token_path(user_id: user.username, id: token.id)
+
+      expect(response).to have_gitlab_http_status(:not_found)
+    end
+  end
+end

spec/requests/git_http_spec.rb

@@ -706,6 +706,32 @@ RSpec.describe 'Git HTTP requests' do
                       end
                     end
                   end
+
+                  context 'when token is impersonated' do
+                    context 'when impersonation is off' do
+                      before do
+                        stub_config_setting(impersonation_enabled: false)
+                      end
+
+                      it 'responds to uploads with status 401 unauthorized' do
+                        write_access_token = create(:personal_access_token, :impersonation, user: user, scopes: [:write_repository])
+
+                        upload(path, user: user.username, password: write_access_token.token) do |response|
+                          expect(response).to have_gitlab_http_status(:unauthorized)
+                        end
+                      end
+                    end
+
+                    context 'when impersonation is on' do
+                      it 'responds to uploads with status 200' do
+                        write_access_token = create(:personal_access_token, :impersonation, user: user, scopes: [:write_repository])
+
+                        upload(path, user: user.username, password: write_access_token.token) do |response|
+                          expect(response).to have_gitlab_http_status(:ok)
+                        end
+                      end
+                    end
+                  end
                 end
               end