From 3bb626f91cb50bd2eff58681e22db942b7d6a087 Mon Sep 17 00:00:00 2001
From: James Newton <hello@jamesnewton.com>
Date: Wed, 28 Oct 2015 16:39:23 +0100
Subject: [PATCH] refactor login as to be impersonation with better
login/logout
Modifies the existing "login as" feature to be called impersonation, as
well as keeping track of who is impersonating to revert back to that
user without having to log out.
---
app/assets/stylesheets/framework/header.scss | 4 ++
.../admin/application_controller.rb | 6 +++
.../admin/impersonation_controller.rb | 32 ++++++++++++
app/controllers/admin/users_controller.rb | 6 ---
app/views/admin/users/_head.html.haml | 2 +-
app/views/layouts/header/_default.html.haml | 4 ++
config/routes.rb | 4 +-
.../admin/users_controller_spec.rb | 15 ------
spec/features/admin/admin_users_spec.rb | 50 ++++++++++++++-----
9 files changed, 88 insertions(+), 35 deletions(-)
create mode 100644 app/controllers/admin/impersonation_controller.rb
diff --git a/app/assets/stylesheets/framework/header.scss b/app/assets/stylesheets/framework/header.scss
index 91e6975e269..02ea91602e8 100644
--- a/app/assets/stylesheets/framework/header.scss
+++ b/app/assets/stylesheets/framework/header.scss
@@ -118,6 +118,10 @@ header {
}
}
}
+
+ .impersonation i {
+ color: $red-normal;
+ }
}
@mixin collapsed-header {
diff --git a/app/controllers/admin/application_controller.rb b/app/controllers/admin/application_controller.rb
index 56e24386463..9083bfb41cf 100644
--- a/app/controllers/admin/application_controller.rb
+++ b/app/controllers/admin/application_controller.rb
@@ -8,4 +8,10 @@ class Admin::ApplicationController < ApplicationController
def authenticate_admin!
return render_404 unless current_user.is_admin?
end
+
+ def authorize_impersonator!
+ if session[:impersonator_id]
+ User.find_by!(username: session[:impersonator_id]).admin?
+ end
+ end
end
diff --git a/app/controllers/admin/impersonation_controller.rb b/app/controllers/admin/impersonation_controller.rb
new file mode 100644
index 00000000000..0382402afa6
--- /dev/null
+++ b/app/controllers/admin/impersonation_controller.rb
@@ -0,0 +1,32 @@
+class Admin::ImpersonationController < Admin::ApplicationController
+ skip_before_action :authenticate_admin!, only: :destroy
+
+ before_action :user
+ before_action :authorize_impersonator!
+
+ def create
+ session[:impersonator_id] = current_user.username
+ session[:impersonator_return_to] = request.env['HTTP_REFERER']
+
+ warden.set_user(user, scope: 'user')
+
+ flash[:alert] = "You are impersonating #{user.username}."
+
+ redirect_to root_path
+ end
+
+ def destroy
+ redirect = session[:impersonator_return_to]
+
+ warden.set_user(user, scope: 'user')
+
+ session[:impersonator_return_to] = nil
+ session[:impersonator_id] = nil
+
+ redirect_to redirect || root_path
+ end
+
+ def user
+ @user ||= User.find_by!(username: params[:id] || session[:impersonator_id])
+ end
+end
diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb
index c63d0793e31..d7c927d444c 100644
--- a/app/controllers/admin/users_controller.rb
+++ b/app/controllers/admin/users_controller.rb
@@ -63,12 +63,6 @@ class Admin::UsersController < Admin::ApplicationController
end
end
- def login_as
- sign_in(user)
- flash[:alert] = "Logged in as #{user.username}"
- redirect_to root_path
- end
-
def disable_two_factor
user.disable_two_factor!
redirect_to admin_user_path(user),
diff --git a/app/views/admin/users/_head.html.haml b/app/views/admin/users/_head.html.haml
index 4245d0f1eda..8d1cab4137c 100644
--- a/app/views/admin/users/_head.html.haml
+++ b/app/views/admin/users/_head.html.haml
@@ -7,7 +7,7 @@
.pull-right
- unless @user == current_user
- = link_to 'Log in as this user', login_as_admin_user_path(@user), method: :post, class: "btn btn-grouped btn-info"
+ = link_to 'Impersonate', impersonate_admin_user_path(@user), method: :post, class: "btn btn-grouped btn-info"
= link_to edit_admin_user_path(@user), class: "btn btn-grouped" do
%i.fa.fa-pencil-square-o
Edit
diff --git a/app/views/layouts/header/_default.html.haml b/app/views/layouts/header/_default.html.haml
index c31b1cbe9a8..13eeb0fe20b 100644
--- a/app/views/layouts/header/_default.html.haml
+++ b/app/views/layouts/header/_default.html.haml
@@ -13,6 +13,10 @@
%li.visible-sm.visible-xs
= link_to search_path, title: 'Search', data: {toggle: 'tooltip', placement: 'bottom'} do
= icon('search')
+ - if session[:impersonator_id]
+ %li.impersonation
+ = link_to stop_impersonation_admin_users_path, method: :delete, title: 'Stop impersonation', data: { toggle: 'tooltip', placement: 'bottom' } do
+ = icon('user-secret fw')
- if current_user.is_admin?
%li
= link_to admin_root_path, title: 'Admin area', data: {toggle: 'tooltip', placement: 'bottom'} do
diff --git a/config/routes.rb b/config/routes.rb
index f6812c9280a..6ebedaefafe 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -212,6 +212,8 @@ Gitlab::Application.routes.draw do
resources :keys, only: [:show, :destroy]
resources :identities, only: [:index, :edit, :update, :destroy]
+ delete 'stop_impersonation' => 'impersonation#destroy', on: :collection
+
member do
get :projects
get :keys
@@ -221,7 +223,7 @@ Gitlab::Application.routes.draw do
put :unblock
put :unlock
put :confirm
- post :login_as
+ post 'impersonate' => 'impersonation#create'
patch :disable_two_factor
delete 'remove/:email_id', action: 'remove_email', as: 'remove_email'
end
diff --git a/spec/controllers/admin/users_controller_spec.rb b/spec/controllers/admin/users_controller_spec.rb
index fcbe62cace8..8b7af4d3a0a 100644
--- a/spec/controllers/admin/users_controller_spec.rb
+++ b/spec/controllers/admin/users_controller_spec.rb
@@ -7,21 +7,6 @@ describe Admin::UsersController do
sign_in(admin)
end
- describe 'POST login_as' do
- let(:user) { create(:user) }
-
- it 'logs admin as another user' do
- expect(warden.authenticate(scope: :user)).not_to eq(user)
- post :login_as, id: user.username
- expect(warden.authenticate(scope: :user)).to eq(user)
- end
-
- it 'redirects user to homepage' do
- post :login_as, id: user.username
- expect(response).to redirect_to(root_path)
- end
- end
-
describe 'DELETE #user with projects' do
let(:user) { create(:user) }
let(:project) { create(:project, namespace: user.namespace) }
diff --git a/spec/features/admin/admin_users_spec.rb b/spec/features/admin/admin_users_spec.rb
index c2c7364f6c5..4c756a8e732 100644
--- a/spec/features/admin/admin_users_spec.rb
+++ b/spec/features/admin/admin_users_spec.rb
@@ -111,24 +111,50 @@ describe "Admin::Users", feature: true do
expect(page).to have_content(@user.name)
end
- describe 'Login as another user' do
- it 'should show login button for other users and check that it works' do
- another_user = create(:user)
+ describe 'Impersonation' do
+ let(:another_user) { create(:user) }
+ before { visit admin_user_path(another_user) }
- visit admin_user_path(another_user)
-
- click_link 'Log in as this user'
+ context 'before impersonating' do
+ it 'shows impersonate button for other users' do
+ expect(page).to have_content('Impersonate')
+ end
- expect(page).to have_content("Logged in as #{another_user.username}")
+ it 'should not show impersonate button for admin itself' do
+ visit admin_user_path(@user)
- page.within '.sidebar-user .username' do
- expect(page).to have_content(another_user.username)
+ expect(page).not_to have_content('Impersonate')
end
end
- it 'should not show login button for admin itself' do
- visit admin_user_path(@user)
- expect(page).not_to have_content('Log in as this user')
+ context 'when impersonating' do
+ before { click_link 'Impersonate' }
+
+ it 'logs in as the user when impersonate is clicked' do
+ page.within '.sidebar-user .username' do
+ expect(page).to have_content(another_user.username)
+ end
+ end
+
+ it 'sees impersonation log out icon' do
+ icon = first('.fa.fa-user-secret')
+
+ expect(icon).to_not eql nil
+ end
+
+ it 'can log out of impersonated user back to original user' do
+ find(:css, 'li.impersonation a').click
+
+ page.within '.sidebar-user .username' do
+ expect(page).to have_content(@user.username)
+ end
+ end
+
+ it 'is redirected back to the impersonated users page in the admin after stopping' do
+ find(:css, 'li.impersonation a').click
+
+ expect(current_path).to eql "/admin/users/#{another_user.username}"
+ end
end
end
--
GitLab