From 8dd9a8b6e00cbd91f8455218397c8da716fc9b00 Mon Sep 17 00:00:00 2001
From: Oren Kanner <okanner@gmail.com>
Date: Thu, 17 Nov 2016 23:21:02 -0500
Subject: [PATCH] Allow admins to stop impersonating users without e-mail
 addresses

Resolves #24576

Modify the guard clause of the `ApplicationController#require_email`
before action to skip requests where an admin is impersonating the
current user.
---
 app/controllers/application_controller.rb     |  2 +-
 .../24576_cant_stop_impersonating.yml         |  4 +++
 .../admin/impersonations_controller_spec.rb   | 28 ++++++++++++++-----
 3 files changed, 26 insertions(+), 8 deletions(-)
 create mode 100644 changelogs/unreleased/24576_cant_stop_impersonating.yml

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 517ad4f03f3..2713c0f1dc8 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -224,7 +224,7 @@ class ApplicationController < ActionController::Base
   end
 
   def require_email
-    if current_user && current_user.temp_oauth_email?
+    if current_user && current_user.temp_oauth_email? && session[:impersonator_id].nil?
       redirect_to profile_path, notice: 'Please complete your profile with email address' and return
     end
   end
diff --git a/changelogs/unreleased/24576_cant_stop_impersonating.yml b/changelogs/unreleased/24576_cant_stop_impersonating.yml
new file mode 100644
index 00000000000..8fa6eeca756
--- /dev/null
+++ b/changelogs/unreleased/24576_cant_stop_impersonating.yml
@@ -0,0 +1,4 @@
+---
+title: Allow admins to stop impersonating users without e-mail addresses
+merge_request: 7550
+author: Oren Kanner
diff --git a/spec/controllers/admin/impersonations_controller_spec.rb b/spec/controllers/admin/impersonations_controller_spec.rb
index 8be662974a0..8f1f0ba89ff 100644
--- a/spec/controllers/admin/impersonations_controller_spec.rb
+++ b/spec/controllers/admin/impersonations_controller_spec.rb
@@ -76,18 +76,32 @@ describe Admin::ImpersonationsController do
           end
 
           context "when the impersonator is not blocked" do
-            it "redirects to the impersonated user's page" do
-              expect(Gitlab::AppLogger).to receive(:info).with("User #{impersonator.username} has stopped impersonating #{user.username}").and_call_original
+            shared_examples_for "successfully stops impersonating" do
+              it "redirects to the impersonated user's page" do
+                expect(Gitlab::AppLogger).to receive(:info).with("User #{impersonator.username} has stopped impersonating #{user.username}").and_call_original
 
-              delete :destroy
+                delete :destroy
 
-              expect(response).to redirect_to(admin_user_path(user))
+                expect(response).to redirect_to(admin_user_path(user))
+              end
+
+              it "signs us in as the impersonator" do
+                delete :destroy
+
+                expect(warden.user).to eq(impersonator)
+              end
             end
 
-            it "signs us in as the impersonator" do
-              delete :destroy
+            # base case
+            it_behaves_like "successfully stops impersonating"
 
-              expect(warden.user).to eq(impersonator)
+            context "and the user has a temporary oauth e-mail address" do
+              before do
+                allow(user).to receive(:temp_oauth_email?).and_return(true)
+                allow(controller).to receive(:current_user).and_return(user)
+              end
+
+              it_behaves_like "successfully stops impersonating"
             end
           end
         end