Cleanup common code in Unique Ips tests

lib/api/api.rb

@@ -61,7 +61,7 @@ module API
     end
 
     rescue_from Gitlab::Auth::TooManyIps do |e|
-      rack_response({'message'=>'403 Forbidden'}.to_json, 403)
+      rack_response({ 'message' => '403 Forbidden' }.to_json, 403)
     end
 
     rescue_from :all do |exception|

lib/gitlab/auth/unique_ips_limiter.rb

@@ -27,7 +27,7 @@ module Gitlab
         end
 
         def limit_user!(user = nil)
-          user = yield if user.nil?
+          user = yield if user.nil? && block_given?
           limit_user_id!(user.id) unless user.nil?
           user
         end

spec/lib/gitlab/auth/unique_ips_limiter_spec.rb

 require 'spec_helper'
 
 describe Gitlab::Auth::UniqueIpsLimiter, :redis, lib: true do
+  include_context 'enable unique ips sign in limit'
   let(:user) { create(:user) }
 
   describe '#count_unique_ips' do
     context 'non unique IPs' do
       it 'properly counts them' do
-        expect(Gitlab::Auth::UniqueIpsLimiter.count_unique_ips(user.id, '192.168.1.1')).to eq(1)
-        expect(Gitlab::Auth::UniqueIpsLimiter.count_unique_ips(user.id, '192.168.1.1')).to eq(1)
+        expect(Gitlab::Auth::UniqueIpsLimiter.count_unique_ips(user.id, 'ip1')).to eq(1)
+        expect(Gitlab::Auth::UniqueIpsLimiter.count_unique_ips(user.id, 'ip1')).to eq(1)
       end
     end
 
     context 'unique IPs' do
       it 'properly counts them' do
-        expect(Gitlab::Auth::UniqueIpsLimiter.count_unique_ips(user.id, '192.168.1.2')).to eq(1)
-        expect(Gitlab::Auth::UniqueIpsLimiter.count_unique_ips(user.id, '192.168.1.3')).to eq(2)
+        expect(Gitlab::Auth::UniqueIpsLimiter.count_unique_ips(user.id, 'ip2')).to eq(1)
+        expect(Gitlab::Auth::UniqueIpsLimiter.count_unique_ips(user.id, 'ip3')).to eq(2)
       end
     end
 
@@ -22,58 +23,35 @@ describe Gitlab::Auth::UniqueIpsLimiter, :redis, lib: true do
       cur_time = Time.now
       allow(Time).to receive(:now).and_return(cur_time)
 
-      expect(Gitlab::Auth::UniqueIpsLimiter.count_unique_ips(user.id, '192.168.1.2')).to eq(1)
-      expect(Gitlab::Auth::UniqueIpsLimiter.count_unique_ips(user.id, '192.168.1.3')).to eq(2)
+      expect(Gitlab::Auth::UniqueIpsLimiter.count_unique_ips(user.id, 'ip2')).to eq(1)
+      expect(Gitlab::Auth::UniqueIpsLimiter.count_unique_ips(user.id, 'ip3')).to eq(2)
 
       allow(Time).to receive(:now).and_return(cur_time + Gitlab::Auth::UniqueIpsLimiter.config.unique_ips_limit_time_window)
 
-      expect(Gitlab::Auth::UniqueIpsLimiter.count_unique_ips(user.id, '192.168.1.4')).to eq(1)
-      expect(Gitlab::Auth::UniqueIpsLimiter.count_unique_ips(user.id, '192.168.1.5')).to eq(2)
+      expect(Gitlab::Auth::UniqueIpsLimiter.count_unique_ips(user.id, 'ip4')).to eq(1)
+      expect(Gitlab::Auth::UniqueIpsLimiter.count_unique_ips(user.id, 'ip5')).to eq(2)
     end
   end
 
   describe '#limit_user!' do
-    context 'when unique ips limit is enabled' do
-      before do
-        allow(Gitlab::Auth::UniqueIpsLimiter).to receive_message_chain(:config, :unique_ips_limit_enabled).and_return(true)
-        allow(Gitlab::Auth::UniqueIpsLimiter).to receive_message_chain(:config, :unique_ips_limit_time_window).and_return(10)
-      end
-
-      context 'when ip limit is set to 1' do
-        before do
-          allow(Gitlab::Auth::UniqueIpsLimiter).to receive_message_chain(:config, :unique_ips_limit_per_user).and_return(1)
-        end
-
-        it 'blocks user trying to login from second ip' do
-          allow(Gitlab::RequestContext).to receive(:client_ip).and_return('192.168.1.1')
-          expect(Gitlab::Auth::UniqueIpsLimiter.limit_user! { user }).to eq(user)
-
-          allow(Gitlab::RequestContext).to receive(:client_ip).and_return('192.168.1.2')
-          expect { Gitlab::Auth::UniqueIpsLimiter.limit_user! { user } }.to raise_error(Gitlab::Auth::TooManyIps)
-        end
-
-        it 'allows user trying to login from the same ip twice' do
-          allow(Gitlab::RequestContext).to receive(:client_ip).and_return('192.168.1.1')
-          expect(Gitlab::Auth::UniqueIpsLimiter.limit_user! { user }).to eq(user)
-          expect(Gitlab::Auth::UniqueIpsLimiter.limit_user! { user }).to eq(user)
-        end
+    include_examples 'user login operation with unique ip limit' do
+      def operation
+        Gitlab::Auth::UniqueIpsLimiter.limit_user! { user }
       end
+    end
 
-      context 'when ip limit is set to 2' do
-        before do
-          allow(Gitlab::Auth::UniqueIpsLimiter).to receive_message_chain(:config, :unique_ips_limit_per_user).and_return(2)
-        end
+    context 'allow 2 unique ips' do
+      before { current_application_settings.update!(unique_ips_limit_per_user: 2) }
 
-        it 'blocks user trying to login from third ip' do
-          allow(Gitlab::RequestContext).to receive(:client_ip).and_return('192.168.1.1')
-          expect(Gitlab::Auth::UniqueIpsLimiter.limit_user! { user }).to eq(user)
+      it 'blocks user trying to login from third ip' do
+        change_ip('ip1')
+        expect(Gitlab::Auth::UniqueIpsLimiter.limit_user! { user }).to eq(user)
 
-          allow(Gitlab::RequestContext).to receive(:client_ip).and_return('192.168.1.2')
-          expect(Gitlab::Auth::UniqueIpsLimiter.limit_user! { user }).to eq(user)
+        change_ip('ip2')
+        expect(Gitlab::Auth::UniqueIpsLimiter.limit_user! { user }).to eq(user)
 
-          allow(Gitlab::RequestContext).to receive(:client_ip).and_return('192.168.1.3')
-          expect { Gitlab::Auth::UniqueIpsLimiter.limit_user! { user } }.to raise_error(Gitlab::Auth::TooManyIps)
-        end
+        change_ip('ip3')
+        expect { Gitlab::Auth::UniqueIpsLimiter.limit_user! { user } }.to raise_error(Gitlab::Auth::TooManyIps)
       end
     end
   end

spec/requests/api/doorkeeper_access_spec.rb

 require 'spec_helper'
 
-shared_examples 'user login request with unique ip limit' do
-  include_context 'limit login to only one ip' do
-    it 'allows user authenticating from the same ip' do
-      change_ip('ip')
-      request
-      expect(response).to have_http_status(200)
-
-      request
-      expect(response).to have_http_status(200)
-    end
-
-    it 'blocks user authenticating from two distinct ips' do
-      change_ip('ip')
-      request
-      expect(response).to have_http_status(200)
-
-      change_ip('ip2')
-      request
-      expect(response).to have_http_status(403)
-    end
-  end
-end
-
 describe API::API, api: true do
   include ApiHelpers
 

spec/support/unique_ip_check_shared_examples.rb

-shared_context 'limit login to only one ip' do
+shared_context 'enable unique ips sign in limit' do
+  include StubENV
   before(:each) do
     Gitlab::Redis.with(&:flushall)
   end
 
   before do
-    allow(Gitlab::Auth::UniqueIpsLimiter).to receive_message_chain(:config, :unique_ips_limit_enabled).and_return(true)
-    allow(Gitlab::Auth::UniqueIpsLimiter).to receive_message_chain(:config, :unique_ips_limit_time_window).and_return(10000)
-    allow(Gitlab::Auth::UniqueIpsLimiter).to receive_message_chain(:config, :unique_ips_limit_per_user).and_return(1)
+    stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false')
+
+    current_application_settings.update!(
+      unique_ips_limit_enabled: true,
+      unique_ips_limit_time_window: 10000
+    )
   end
 
   def change_ip(ip)
@@ -15,7 +19,9 @@ shared_context 'limit login to only one ip' do
 end
 
 shared_examples 'user login operation with unique ip limit' do
-  include_context 'limit login to only one ip' do
+  include_context 'enable unique ips sign in limit' do
+    before { current_application_settings.update!(unique_ips_limit_per_user: 1) }
+
     it 'allows user authenticating from the same ip' do
       change_ip('ip')
       expect { operation }.not_to raise_error
@@ -31,3 +37,28 @@ shared_examples 'user login operation with unique ip limit' do
     end
   end
 end
+
+shared_examples 'user login request with unique ip limit' do
+  include_context 'enable unique ips sign in limit' do
+    before { current_application_settings.update!(unique_ips_limit_per_user: 1) }
+
+    it 'allows user authenticating from the same ip' do
+      change_ip('ip')
+      request
+      expect(response).to have_http_status(200)
+
+      request
+      expect(response).to have_http_status(200)
+    end
+
+    it 'blocks user authenticating from two distinct ips' do
+      change_ip('ip')
+      request
+      expect(response).to have_http_status(200)
+
+      change_ip('ip2')
+      request
+      expect(response).to have_http_status(403)
+    end
+  end
+end