diff --git a/spec/controllers/sessions_controller_spec.rb b/spec/controllers/sessions_controller_spec.rb index f1157d159ab..56ed11737ad 100644 --- a/spec/controllers/sessions_controller_spec.rb +++ b/spec/controllers/sessions_controller_spec.rb @@ -30,29 +30,10 @@ describe SessionsController do expect(SecurityEvent.last.details[:with]).to eq('standard') end - context 'unique ip limit is enabled and set to 1', :redis 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) - allow(Gitlab::Auth::UniqueIpsLimiter).to receive_message_chain(:config, :unique_ips_limit_per_user).and_return(1) - end - - it 'allows user authenticating from the same ip' do - allow(Gitlab::RequestContext).to receive(:client_ip).and_return('ip') + include_examples 'user login operation with unique ip limit' do + def operation post(:create, user: { login: user.username, password: user.password }) expect(subject.current_user).to eq user - - post(:create, user: { login: user.username, password: user.password }) - expect(subject.current_user).to eq user - end - - it 'blocks user authenticating from two distinct ips' do - allow(Gitlab::RequestContext).to receive(:client_ip).and_return('ip') - post(:create, user: { login: user.username, password: user.password }) - expect(subject.current_user).to eq user - - allow(Gitlab::RequestContext).to receive(:client_ip).and_return('ip2') - expect { post(:create, user: { login: user.username, password: user.password }) }.to raise_error(Gitlab::Auth::TooManyIps) end end end diff --git a/spec/lib/gitlab/auth_spec.rb b/spec/lib/gitlab/auth_spec.rb index ee70ef34f4f..860189dcf3c 100644 --- a/spec/lib/gitlab/auth_spec.rb +++ b/spec/lib/gitlab/auth_spec.rb @@ -58,27 +58,11 @@ describe Gitlab::Auth, lib: true do expect(gl_auth.find_for_git_client(user.username, 'password', project: nil, ip: 'ip')).to eq(Gitlab::Auth::Result.new(user, nil, :gitlab_or_ldap, full_authentication_abilities)) end + include_examples 'user login operation with unique ip limit' do + let(:user) { create(:user, password: 'password') } - context 'unique ip limit is enabled and set to 1', :redis 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) - allow(Gitlab::Auth::UniqueIpsLimiter).to receive_message_chain(:config, :unique_ips_limit_per_user).and_return(1) - end - - it 'allows user authenticating from the same ip' do - user = create(:user, password: 'password') - allow(Gitlab::RequestContext).to receive(:client_ip).and_return('ip') + def operation expect(gl_auth.find_for_git_client(user.username, 'password', project: nil, ip: 'ip')).to eq(Gitlab::Auth::Result.new(user, nil, :gitlab_or_ldap, full_authentication_abilities)) - expect(gl_auth.find_for_git_client(user.username, 'password', project: nil, ip: 'ip')).to eq(Gitlab::Auth::Result.new(user, nil, :gitlab_or_ldap, full_authentication_abilities)) - end - - it 'blocks user authenticating from two distinct ips' do - user = create(:user, password: 'password') - allow(Gitlab::RequestContext).to receive(:client_ip).and_return('ip') - expect(gl_auth.find_for_git_client(user.username, 'password', project: nil, ip: 'ip')).to eq(Gitlab::Auth::Result.new(user, nil, :gitlab_or_ldap, full_authentication_abilities)) - allow(Gitlab::RequestContext).to receive(:client_ip).and_return('ip2') - expect { gl_auth.find_for_git_client(user.username, 'password', project: nil, ip: 'ip2') }.to raise_error(Gitlab::Auth::TooManyIps) end end @@ -220,6 +204,12 @@ describe Gitlab::Auth, lib: true do expect( gl_auth.find_with_user_password(username, password) ).not_to eql user end + include_examples 'user login operation with unique ip limit' do + def operation + expect(gl_auth.find_with_user_password(username, password)).to eql user + end + end + context "with ldap enabled" do before do allow(Gitlab::LDAP::Config).to receive(:enabled?).and_return(true) diff --git a/spec/requests/api/doorkeeper_access_spec.rb b/spec/requests/api/doorkeeper_access_spec.rb index 1cd0701d955..b2864f448a8 100644 --- a/spec/requests/api/doorkeeper_access_spec.rb +++ b/spec/requests/api/doorkeeper_access_spec.rb @@ -1,6 +1,29 @@ require 'spec_helper' -describe API::API, api: true do +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 let!(:user) { create(:user) } @@ -13,22 +36,9 @@ describe API::API, api: true do expect(response).to have_http_status(200) end - include_context 'limit login to only one ip' do - it 'allows login twice from the same ip' do + include_examples 'user login request with unique ip limit' do + def request get api('/user'), access_token: token.token - expect(response).to have_http_status(200) - - get api('/user'), access_token: token.token - expect(response).to have_http_status(200) - end - - it 'blocks login from two different ips' do - get api('/user'), access_token: token.token - expect(response).to have_http_status(200) - - change_ip('ip2') - get api('/user'), access_token: token.token - expect(response).to have_http_status(403) end end end @@ -46,22 +56,9 @@ describe API::API, api: true do expect(response).to have_http_status(200) end - include_context 'limit login to only one ip' do - it 'allows login twice from the same ip' do + include_examples 'user login request with unique ip limit' do + def request get api('/user', user) - expect(response).to have_http_status(200) - - get api('/user', user) - expect(response).to have_http_status(200) - end - - it 'blocks login from two different ips' do - get api('/user', user) - expect(response).to have_http_status(200) - - change_ip('ip2') - get api('/user', user) - expect(response).to have_http_status(403) end end end diff --git a/spec/support/unique_ip_check_shared_examples.rb b/spec/support/unique_ip_check_shared_examples.rb index ab693b91d4a..c868a1c7a7c 100644 --- a/spec/support/unique_ip_check_shared_examples.rb +++ b/spec/support/unique_ip_check_shared_examples.rb @@ -1,7 +1,11 @@ -shared_context 'limit login to only one ip', :redis do +shared_context 'limit login to only one ip' do + 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(1000) + 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) end @@ -13,11 +17,13 @@ end shared_examples 'user login operation 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') expect { operation }.not_to raise_error expect { operation }.not_to raise_error end it 'blocks user authenticating from two distinct ips' do + change_ip('ip') expect { operation }.not_to raise_error change_ip('ip2')