diff options
| -rw-r--r-- | app/assets/stylesheets/framework/header.scss | 4 | ||||
| -rw-r--r-- | app/controllers/admin/application_controller.rb | 6 | ||||
| -rw-r--r-- | app/controllers/admin/impersonation_controller.rb | 32 | ||||
| -rw-r--r-- | app/controllers/admin/users_controller.rb | 6 | ||||
| -rw-r--r-- | app/views/admin/users/_head.html.haml | 2 | ||||
| -rw-r--r-- | app/views/layouts/header/_default.html.haml | 4 | ||||
| -rw-r--r-- | config/routes.rb | 4 | ||||
| -rw-r--r-- | spec/controllers/admin/users_controller_spec.rb | 15 | ||||
| -rw-r--r-- | spec/features/admin/admin_users_spec.rb | 50 |
9 files changed, 88 insertions, 35 deletions
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 c08a7b80744..3ca30d3baab 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 56a31d980f9..095c562be8d 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -222,6 +222,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 @@ -231,7 +233,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 |