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

Add config to prohibit impersonation

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

4 files changed, 112 insertions, 29 deletions

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 5d3fbba264f..4e4c8b215c2 100644
--- a/spec/lib/gitlab/auth/user_auth_finders_spec.rb
+++ b/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
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