diff options
author | Robert Speicher <robert@gitlab.com> | 2016-04-25 23:50:38 +0300 |
---|---|---|
committer | Robert Speicher <rspeicher@gmail.com> | 2016-04-26 00:03:43 +0300 |
commit | 15f8fb31641f4dabce98bc6a14a1828f053cdedc (patch) | |
tree | b863132a42246fe37085bbc82e24bf8df8837bf9 /app | |
parent | 2ca0e9d62e38a767cdd95884e269ad6eb300204a (diff) |
Merge branch 'fix-impersonation-issue' into 'master'
Prevent privilege escalation via "impersonate" feature
Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/15548
See merge request !1956
Diffstat (limited to 'app')
-rw-r--r-- | app/controllers/admin/application_controller.rb | 8 | ||||
-rw-r--r-- | app/controllers/admin/impersonation_controller.rb | 32 | ||||
-rw-r--r-- | app/controllers/admin/impersonations_controller.rb | 24 | ||||
-rw-r--r-- | app/controllers/admin/users_controller.rb | 16 | ||||
-rw-r--r-- | app/views/layouts/header/_default.html.haml | 2 |
5 files changed, 42 insertions, 40 deletions
diff --git a/app/controllers/admin/application_controller.rb b/app/controllers/admin/application_controller.rb index 9083bfb41cf..cf795d977ce 100644 --- a/app/controllers/admin/application_controller.rb +++ b/app/controllers/admin/application_controller.rb @@ -6,12 +6,6 @@ class Admin::ApplicationController < ApplicationController layout 'admin' 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 + render_404 unless current_user.is_admin? end end diff --git a/app/controllers/admin/impersonation_controller.rb b/app/controllers/admin/impersonation_controller.rb deleted file mode 100644 index 0382402afa6..00000000000 --- a/app/controllers/admin/impersonation_controller.rb +++ /dev/null @@ -1,32 +0,0 @@ -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/impersonations_controller.rb b/app/controllers/admin/impersonations_controller.rb new file mode 100644 index 00000000000..2db824c87ef --- /dev/null +++ b/app/controllers/admin/impersonations_controller.rb @@ -0,0 +1,24 @@ +class Admin::ImpersonationsController < Admin::ApplicationController + skip_before_action :authenticate_admin! + before_action :authenticate_impersonator! + + def destroy + original_user = current_user + + warden.set_user(impersonator, scope: :user) + + session[:impersonator_id] = nil + + redirect_to admin_user_path(original_user) + end + + private + + def impersonator + @impersonator ||= User.find(session[:impersonator_id]) if session[:impersonator_id] + end + + def authenticate_impersonator! + render_404 unless impersonator && impersonator.is_admin? && !impersonator.blocked? + end +end diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb index d7c927d444c..740d4dd5a8f 100644 --- a/app/controllers/admin/users_controller.rb +++ b/app/controllers/admin/users_controller.rb @@ -31,6 +31,22 @@ class Admin::UsersController < Admin::ApplicationController user end + def impersonate + if user.blocked? + flash[:alert] = "You cannot impersonate a blocked user" + + redirect_to admin_user_path(user) + else + session[:impersonator_id] = current_user.id + + warden.set_user(user, scope: :user) + + flash[:alert] = "You are now impersonating #{user.username}" + + redirect_to root_path + end + end + def block if user.block redirect_back_or_admin_user(notice: "Successfully blocked") diff --git a/app/views/layouts/header/_default.html.haml b/app/views/layouts/header/_default.html.haml index 3ca30d3baab..6b850ac5d9c 100644 --- a/app/views/layouts/header/_default.html.haml +++ b/app/views/layouts/header/_default.html.haml @@ -15,7 +15,7 @@ = 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 + = link_to admin_impersonation_path, method: :delete, title: 'Stop impersonation', data: { toggle: 'tooltip', placement: 'bottom' } do = icon('user-secret fw') - if current_user.is_admin? %li |