diff options
author | Pawel Chojnacki <pawel@chojnacki.ws> | 2017-02-17 14:44:57 +0100 |
---|---|---|
committer | Pawel Chojnacki <pawel@chojnacki.ws> | 2017-03-06 15:41:25 +0100 |
commit | 9cc0ff8f468c54e23172492d97f6d9b428d3ad2e (patch) | |
tree | 5de31ec4dea0df354cc246b809dff5ef376316d5 | |
parent | 80fbced2e0b8d291173e1002f150bc5551e87359 (diff) | |
download | gitlab-ce-9cc0ff8f468c54e23172492d97f6d9b428d3ad2e.tar.gz |
Cleanup common code in Unique Ips tests
-rw-r--r-- | lib/api/api.rb | 2 | ||||
-rw-r--r-- | lib/gitlab/auth/unique_ips_limiter.rb | 2 | ||||
-rw-r--r-- | spec/lib/gitlab/auth/unique_ips_limiter_spec.rb | 66 | ||||
-rw-r--r-- | spec/requests/api/doorkeeper_access_spec.rb | 23 | ||||
-rw-r--r-- | spec/support/unique_ip_check_shared_examples.rb | 41 |
5 files changed, 60 insertions, 74 deletions
diff --git a/lib/api/api.rb b/lib/api/api.rb index 6f37fa9d8e9..efbb56ecd2c 100644 --- a/lib/api/api.rb +++ b/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| diff --git a/lib/gitlab/auth/unique_ips_limiter.rb b/lib/gitlab/auth/unique_ips_limiter.rb index 7f849ef4c38..4b2b758be8a 100644 --- a/lib/gitlab/auth/unique_ips_limiter.rb +++ b/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 diff --git a/spec/lib/gitlab/auth/unique_ips_limiter_spec.rb b/spec/lib/gitlab/auth/unique_ips_limiter_spec.rb index f2472b4310f..2b21a76c59d 100644 --- a/spec/lib/gitlab/auth/unique_ips_limiter_spec.rb +++ b/spec/lib/gitlab/auth/unique_ips_limiter_spec.rb @@ -1,20 +1,21 @@ 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 diff --git a/spec/requests/api/doorkeeper_access_spec.rb b/spec/requests/api/doorkeeper_access_spec.rb index b2864f448a8..634b2261c5e 100644 --- a/spec/requests/api/doorkeeper_access_spec.rb +++ b/spec/requests/api/doorkeeper_access_spec.rb @@ -1,28 +1,5 @@ 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 diff --git a/spec/support/unique_ip_check_shared_examples.rb b/spec/support/unique_ip_check_shared_examples.rb index c868a1c7a7c..024fb132778 100644 --- a/spec/support/unique_ip_check_shared_examples.rb +++ b/spec/support/unique_ip_check_shared_examples.rb @@ -1,12 +1,16 @@ -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 |