From 5af9a69a3b9df4b08ebc40c87c4cb9236860fe16 Mon Sep 17 00:00:00 2001
From: Bianca Nenciu <nbianca@users.noreply.github.com>
Date: Mon, 12 Nov 2018 16:34:12 +0200
Subject: [PATCH] FIX: Do not check for suspicious login when impersonating.
 (#6534)

* FIX: Do not check for suspicious login when impersonating.

* DEV: Add 'impersonate' parameter to log_on_user.
---
 .../admin/impersonate_controller.rb           |  2 +-
 app/models/user_auth_token.rb                 |  4 +--
 lib/auth/current_user_provider.rb             |  2 +-
 lib/auth/default_current_user_provider.rb     |  5 ++--
 lib/current_user.rb                           |  4 +--
 spec/models/user_auth_token_spec.rb           | 25 +++++++++++++++++++
 spec/rails_helper.rb                          |  2 +-
 7 files changed, 35 insertions(+), 9 deletions(-)

diff --git a/app/controllers/admin/impersonate_controller.rb b/app/controllers/admin/impersonate_controller.rb
index 1e109521b9b..31727d0f267 100644
--- a/app/controllers/admin/impersonate_controller.rb
+++ b/app/controllers/admin/impersonate_controller.rb
@@ -12,7 +12,7 @@ class Admin::ImpersonateController < Admin::AdminController
     StaffActionLogger.new(current_user).log_impersonate(user)
 
     # Log on as the user
-    log_on_user(user)
+    log_on_user(user, impersonate: true)
 
     render body: nil
   end
diff --git a/app/models/user_auth_token.rb b/app/models/user_auth_token.rb
index b36af49d815..cc484b6eb26 100644
--- a/app/models/user_auth_token.rb
+++ b/app/models/user_auth_token.rb
@@ -61,7 +61,7 @@ class UserAuthToken < ActiveRecord::Base
     ips.none? { |ip| user_location == login_location(ip) }
   end
 
-  def self.generate!(user_id: , user_agent: nil, client_ip: nil, path: nil, staff: nil)
+  def self.generate!(user_id: , user_agent: nil, client_ip: nil, path: nil, staff: nil, impersonate: false)
     token = SecureRandom.hex(16)
     hashed_token = hash_token(token)
     user_auth_token = UserAuthToken.create!(
@@ -82,7 +82,7 @@ class UserAuthToken < ActiveRecord::Base
         path: path,
         auth_token: hashed_token)
 
-    if staff
+    if staff && !impersonate
       Jobs.enqueue(:suspicious_login,
         user_id: user_id,
         client_ip: client_ip,
diff --git a/lib/auth/current_user_provider.rb b/lib/auth/current_user_provider.rb
index 1affbc8d870..f03a1135cb9 100644
--- a/lib/auth/current_user_provider.rb
+++ b/lib/auth/current_user_provider.rb
@@ -12,7 +12,7 @@ class Auth::CurrentUserProvider
   end
 
   # log on a user and set cookies and session etc.
-  def log_on_user(user, session, cookies)
+  def log_on_user(user, session, cookies, opts = {})
     raise NotImplementedError
   end
 
diff --git a/lib/auth/default_current_user_provider.rb b/lib/auth/default_current_user_provider.rb
index 7ecf7c2392a..ed1230ea8e9 100644
--- a/lib/auth/default_current_user_provider.rb
+++ b/lib/auth/default_current_user_provider.rb
@@ -149,13 +149,14 @@ class Auth::DefaultCurrentUserProvider
     end
   end
 
-  def log_on_user(user, session, cookies)
+  def log_on_user(user, session, cookies, opts = {})
     @user_token = UserAuthToken.generate!(
       user_id: user.id,
       user_agent: @env['HTTP_USER_AGENT'],
       path: @env['REQUEST_PATH'],
       client_ip: @request.ip,
-      staff: user.staff?)
+      staff: user.staff?,
+      impersonate: opts.impersonate)
 
     cookies[TOKEN_COOKIE] = cookie_hash(@user_token.unhashed_auth_token)
     unstage_user(user)
diff --git a/lib/current_user.rb b/lib/current_user.rb
index a5035b97646..b43ef40caff 100644
--- a/lib/current_user.rb
+++ b/lib/current_user.rb
@@ -13,8 +13,8 @@ module CurrentUser
     @current_user_provider = Discourse.current_user_provider.new({})
   end
 
-  def log_on_user(user)
-    current_user_provider.log_on_user(user, session, cookies)
+  def log_on_user(user, opts = {})
+    current_user_provider.log_on_user(user, session, cookies, opts)
     user.logged_in
   end
 
diff --git a/spec/models/user_auth_token_spec.rb b/spec/models/user_auth_token_spec.rb
index f3a989a66b5..8e17c25335b 100644
--- a/spec/models/user_auth_token_spec.rb
+++ b/spec/models/user_auth_token_spec.rb
@@ -283,4 +283,29 @@ describe UserAuthToken do
     expect(lookup.auth_token_seen).to eq(true)
   end
 
+  context "suspicious login" do
+
+    let(:user) { Fabricate(:user) }
+    let(:admin) { Fabricate(:admin) }
+
+    it "is not checked when generated for non-staff" do
+      UserAuthToken.generate!(user_id: user.id, staff: user.staff?)
+
+      expect(Jobs::SuspiciousLogin.jobs.size).to eq(0)
+    end
+
+    it "is checked when generated for staff" do
+      UserAuthToken.generate!(user_id: admin.id, staff: admin.staff?)
+
+      expect(Jobs::SuspiciousLogin.jobs.size).to eq(1)
+    end
+
+    it "is not checked when generated by impersonate" do
+      UserAuthToken.generate!(user_id: admin.id, staff: admin.staff?, impersonate: true)
+
+      expect(Jobs::SuspiciousLogin.jobs.size).to eq(0)
+    end
+
+  end
+
 end
diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb
index 2a78230fbed..21df664d02a 100644
--- a/spec/rails_helper.rb
+++ b/spec/rails_helper.rb
@@ -187,7 +187,7 @@ RSpec.configure do |config|
   end
 
   class TestCurrentUserProvider < Auth::DefaultCurrentUserProvider
-    def log_on_user(user, session, cookies)
+    def log_on_user(user, session, cookies, opts = {})
       session[:current_user_id] = user.id
       super
     end