Refactor blocked user tracker class

config/initializers/warden.rb

@@ -4,30 +4,31 @@ Rails.application.configure do |config|
   end
 
   Warden::Manager.before_failure(scope: :user) do |env, opts|
-    Gitlab::Auth::BlockedUserTracker.log_if_user_blocked(env)
+    Gitlab::Auth::BlockedUserTracker.new(env).tap do |tracker|
+      tracker.log_blocked_user_activity! if tracker.user_blocked?
 
-    Gitlab::Auth::Activity.new(opts).user_authentication_failed!
+      Gitlab::Auth::Activity.new(tracker.user, opts).user_authentication_failed!
+    end
   end
 
   Warden::Manager.after_authentication(scope: :user) do |user, auth, opts|
     ActiveSession.cleanup(user)
-
-    Gitlab::Auth::Activity.new(opts).user_authenticated!
+    Gitlab::Auth::Activity.new(user, opts).user_authenticated!
   end
 
   Warden::Manager.after_set_user(scope: :user, only: :fetch) do |user, auth, opts|
     ActiveSession.set(user, auth.request)
-
-    Gitlab::Auth::Activity.new(opts).user_session_fetched!
+    Gitlab::Auth::Activity.new(user, opts).user_session_fetched!
   end
 
   Warden::Manager.after_set_user(scope: :user, only: :set_user) do |user, auth, opts|
-    Gitlab::Auth::Activity.new(opts).user_session_override!
+    Gitlab::Auth::Activity.new(user, opts).user_session_override!
   end
 
-  Warden::Manager.before_logout(scope: :user) do |user, auth, opts|
-    ActiveSession.destroy(user || auth.user, auth.request.session.id)
-
-    Gitlab::Auth::Activity.new(opts).user_signed_out!
+  Warden::Manager.before_logout(scope: :user) do |warden_user, auth, opts|
+    (warden_user || auth.user).tap do |user|
+      ActiveSession.destroy(user, auth.request.session.id)
+      Gitlab::Auth::Activity.new(user, opts).user_signed_out!
+    end
   end
 end

lib/gitlab/auth/activity.rb

@@ -16,7 +16,8 @@ module Gitlab
         user_signed_out: 'Counter of total user sign out events'
       }.freeze
 
-      def initialize(opts)
+      def initialize(user, opts)
+        @user = user
         @opts = opts
       end
 
@@ -29,6 +30,8 @@ module Gitlab
         when :invalid
           self.class.user_password_invalid_counter.increment
         end
+
+        # case blocked user
       end
 
       def user_authenticated!

lib/gitlab/auth/blocked_user_tracker.rb

@@ -2,34 +2,52 @@
 module Gitlab
   module Auth
     class BlockedUserTracker
+      include Gitlab::Utils::StrongMemoize
+
       ACTIVE_RECORD_REQUEST_PARAMS = 'action_dispatch.request.request_parameters'
 
-      def self.log_if_user_blocked(env)
-        message = env.dig('warden.options', :message)
+      def initialize(env)
+        @env = env
+      end
 
-        # Devise calls User#active_for_authentication? on the User model and then
-        # throws an exception to Warden with User#inactive_message:
-        # https://github.com/plataformatec/devise/blob/v4.2.1/lib/devise/hooks/activatable.rb#L8
-        #
-        # Since Warden doesn't pass the user record to the failure handler, we
-        # need to do a database lookup with the username. We can limit the
-        # lookups to happen when the user was blocked by checking the inactive
-        # message passed along by Warden.
-        return unless message == User::BLOCKED_MESSAGE
+      ##
+      # Devise calls User#active_for_authentication? on the User model and then
+      # throws an exception to Warden with User#inactive_message:
+      # https://github.com/plataformatec/devise/blob/v4.2.1/lib/devise/hooks/activatable.rb#L8
+      #
+      # Since Warden doesn't pass the user record to the failure handler, we
+      # need to do a database lookup with the username. We can limit the
+      # lookups to happen when the user was blocked by checking the inactive
+      # message passed along by Warden.
+      #
+      def has_user_blocked_message?
+        strong_memoize(:user_blocked_message) do
+          message = @env.dig('warden.options', :message)
+          message == User::BLOCKED_MESSAGE
+        end
+      end
 
-        # Check for either LDAP or regular GitLab account logins
-        login = env.dig(ACTIVE_RECORD_REQUEST_PARAMS, 'username') ||
-          env.dig(ACTIVE_RECORD_REQUEST_PARAMS, 'user', 'login')
+      def user
+        return unless has_user_blocked_message?
 
-        return unless login.present?
+        strong_memoize(:user) do
+          # Check for either LDAP or regular GitLab account logins
+          login = @env.dig(ACTIVE_RECORD_REQUEST_PARAMS, 'username') ||
+            @env.dig(ACTIVE_RECORD_REQUEST_PARAMS, 'user', 'login')
 
-        user = User.by_login(login)
+          User.by_login(login) if login.present?
+        end
+      end
 
-        return unless user&.blocked?
+      def user_blocked?
+        user&.blocked?
+      end
 
-        Gitlab::AppLogger.info("Failed login for blocked user: user=#{user.username} ip=#{env['REMOTE_ADDR']}")
-        SystemHooksService.new.execute_hooks_for(user, :failed_login)
+      def log_blocked_user_activity!
+        return unless user_blocked?
 
+        Gitlab::AppLogger.info("Failed login for blocked user: user=#{user.username} ip=#{@env['REMOTE_ADDR']}")
+        SystemHooksService.new.execute_hooks_for(user, :failed_login)
         true
       rescue TypeError
       end

spec/lib/gitlab/auth/blocked_user_tracker_spec.rb

@@ -3,24 +3,26 @@ require 'spec_helper'
 describe Gitlab::Auth::BlockedUserTracker do
   set(:user) { create(:user) }
 
-  describe '.log_if_user_blocked' do
+  # TODO, add more specs
+
+  describe '#log_blocked_user_activity!' do
     it 'does not log if user failed to login due to undefined reason' do
       expect_any_instance_of(SystemHooksService).not_to receive(:execute_hooks_for)
 
-      expect(described_class.log_if_user_blocked({})).to be_nil
+      expect(described_class.new({}).log_blocked_user_activity!).to be_nil
     end
 
     it 'gracefully handles malformed environment variables' do
       env = { 'warden.options' => 'test' }
 
-      expect(described_class.log_if_user_blocked(env)).to be_nil
+      expect(described_class.new(env).log_blocked_user_activity!).to be_nil
     end
 
     context 'failed login due to blocked user' do
       let(:base_env) { { 'warden.options' => { message: User::BLOCKED_MESSAGE } } }
       let(:env) { base_env.merge(request_env) }
 
-      subject { described_class.log_if_user_blocked(env) }
+      subject { described_class.new(env).log_blocked_user_activity! }
 
       before do
         expect_any_instance_of(SystemHooksService).to receive(:execute_hooks_for).with(user, :failed_login)