Merge branch '1870-impersonation-stuck-on-password-change-ce' into 'master'

[CE] Impersonation no longer gets stuck on password change.

Closes #13888

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

4 files changed, 34 insertions, 4 deletions

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 2087fe81411..b2ec491146f 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -196,7 +196,11 @@ class ApplicationController < ActionController::Base
   end
 
   def check_password_expiration
-    if current_user && current_user.password_expires_at && current_user.password_expires_at < Time.now && !current_user.ldap_user?
+    return if session[:impersonator_id] || current_user&.ldap_user?
+
+    password_expires_at = current_user&.password_expires_at
+
+    if password_expires_at && password_expires_at < Time.now
       return redirect_to new_profile_password_path
     end
   end
diff --git a/changelogs/unreleased/1870-impersonation-stuck-on-password-change.yml b/changelogs/unreleased/1870-impersonation-stuck-on-password-change.yml
new file mode 100644
index 00000000000..b217cb44bf7
--- /dev/null
+++ b/changelogs/unreleased/1870-impersonation-stuck-on-password-change.yml
@@ -0,0 +1,5 @@
+---
+title: Impersonation no longer gets stuck on password change.
+merge_request: 15497
+author:
+type: fixed
diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb
index b73ca0c2346..768c7e99c96 100644
--- a/spec/controllers/application_controller_spec.rb
+++ b/spec/controllers/application_controller_spec.rb
@@ -6,6 +6,10 @@ describe ApplicationController do
   describe '#check_password_expiration' do
     let(:controller) { described_class.new }
 
+    before do
+      allow(controller).to receive(:session).and_return({})
+    end
+
     it 'redirects if the user is over their password expiry' do
       user.password_expires_at = Time.new(2002)
 
diff --git a/spec/features/admin/admin_users_spec.rb b/spec/features/admin/admin_users_spec.rb
index b47f9055d29..a69b428d117 100644
--- a/spec/features/admin/admin_users_spec.rb
+++ b/spec/features/admin/admin_users_spec.rb
@@ -167,19 +167,36 @@ describe "Admin::Users" do
         it 'sees impersonation log out icon' do
           icon = first('.fa.fa-user-secret')
 
-          expect(icon).not_to eql nil
+          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 eql(current_user.username)
+          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
 
-          expect(current_path).to eql "/admin/users/#{another_user.username}"
+          expect(current_path).to eq("/admin/users/#{another_user.username}")
+        end
+      end
+
+      context 'when impersonating a user with an expired password' do
+        before do
+          another_user.update(password_expires_at: Time.now - 5.minutes)
+          click_link 'Impersonate'
+        end
+
+        it 'does not redirect to password change page' do
+          expect(current_path).to eq('/')
+        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 eq("/admin/users/#{another_user.username}")
         end
       end
     end