Improve specs for blocked user tracker class

lib/gitlab/auth/blocked_user_tracker.rb

@@ -10,21 +10,8 @@ module Gitlab
         @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.
-      #
-      def has_user_blocked_message?
-        strong_memoize(:user_blocked_message) do
-          message = @env.dig('warden.options', :message)
-          message == User::BLOCKED_MESSAGE
-        end
+      def user_blocked?
+        user&.blocked?
       end
 
       def user
@@ -37,10 +24,7 @@ module Gitlab
 
           User.by_login(login) if login.present?
         end
-      end
-
-      def user_blocked?
-        user&.blocked?
+      rescue TypeError
       end
 
       def log_blocked_user_activity!
@@ -51,6 +35,25 @@ module Gitlab
         true
       rescue TypeError
       end
+
+      private
+
+      ##
+      # 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
     end
   end
 end

spec/lib/gitlab/auth/blocked_user_tracker_spec.rb

@@ -3,26 +3,30 @@ require 'spec_helper'
 describe Gitlab::Auth::BlockedUserTracker do
   set(:user) { create(:user) }
 
-  # 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.new({}).log_blocked_user_activity!).to be_nil
+      tracker = described_class.new({})
+
+      expect(tracker.user).to be_nil
+      expect(tracker.user_blocked?).to be_falsey
+      expect(tracker.log_blocked_user_activity!).to be_nil
     end
 
     it 'gracefully handles malformed environment variables' do
-      env = { 'warden.options' => 'test' }
+      tracker = described_class.new({ 'warden.options' => 'test' })
 
-      expect(described_class.new(env).log_blocked_user_activity!).to be_nil
+      expect(tracker.user).to be_nil
+      expect(tracker.user_blocked?).to be_falsey
+      expect(tracker.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.new(env).log_blocked_user_activity! }
+      subject { described_class.new(env) }
 
       before do
         expect_any_instance_of(SystemHooksService).to receive(:execute_hooks_for).with(user, :failed_login)
@@ -34,14 +38,17 @@ describe Gitlab::Auth::BlockedUserTracker do
         it 'logs a blocked user' do
           user.block!
 
-          expect(subject).to be_truthy
+          expect(subject.user).to be_blocked
+          expect(subject.user_blocked?).to be true
+          expect(subject.log_blocked_user_activity!).to be_truthy
         end
 
         it 'logs a blocked user by e-mail' do
           user.block!
           env[described_class::ACTIVE_RECORD_REQUEST_PARAMS]['user']['login'] = user.email
 
-          expect(subject).to be_truthy
+          expect(subject.user).to be_blocked
+          expect(subject.log_blocked_user_activity!).to be_truthy
         end
       end
 
@@ -51,13 +58,17 @@ describe Gitlab::Auth::BlockedUserTracker do
         it 'logs a blocked user' do
           user.block!
 
-          expect(subject).to be_truthy
+          expect(subject.user).to be_blocked
+          expect(subject.user_blocked?).to be true
+          expect(subject.log_blocked_user_activity!).to be_truthy
         end
 
         it 'logs a LDAP blocked user' do
           user.ldap_block!
 
-          expect(subject).to be_truthy
+          expect(subject.user).to be_blocked
+          expect(subject.user_blocked?).to be true
+          expect(subject.log_blocked_user_activity!).to be_truthy
         end
       end
     end