Commit 8993801f authored by Pawel Chojnacki's avatar Pawel Chojnacki

Test various login scenarios if the limit gets enforced

parent 66dc7159
...@@ -60,6 +60,10 @@ module API ...@@ -60,6 +60,10 @@ module API
error! e.message, e.status, e.headers error! e.message, e.status, e.headers
end end
rescue_from Gitlab::Auth::TooManyIps do |e|
rack_response({'message'=>'403 Forbidden'}.to_json, 403)
end
rescue_from :all do |exception| rescue_from :all do |exception|
handle_api_exception(exception) handle_api_exception(exception)
end end
......
...@@ -336,7 +336,7 @@ module API ...@@ -336,7 +336,7 @@ module API
def initial_current_user def initial_current_user
return @initial_current_user if defined?(@initial_current_user) return @initial_current_user if defined?(@initial_current_user)
Gitlab::Auth::UniqueIpsLimiter.limit_user! do
@initial_current_user ||= find_user_by_private_token(scopes: @scopes) @initial_current_user ||= find_user_by_private_token(scopes: @scopes)
@initial_current_user ||= doorkeeper_guard(scopes: @scopes) @initial_current_user ||= doorkeeper_guard(scopes: @scopes)
@initial_current_user ||= find_user_from_warden @initial_current_user ||= find_user_from_warden
...@@ -347,6 +347,7 @@ module API ...@@ -347,6 +347,7 @@ module API
@initial_current_user @initial_current_user
end end
end
def sudo! def sudo!
return unless sudo_identifier return unless sudo_identifier
......
...@@ -22,7 +22,7 @@ module Gitlab ...@@ -22,7 +22,7 @@ module Gitlab
user_with_password_for_git(login, password) || user_with_password_for_git(login, password) ||
Gitlab::Auth::Result.new Gitlab::Auth::Result.new
Gitlab::Auth::UniqueIpsLimiter.limit_user! { result.actor } Gitlab::Auth::UniqueIpsLimiter.limit_user!(result.actor)
rate_limit!(ip, success: result.success?, login: login) rate_limit!(ip, success: result.success?, login: login)
......
...@@ -62,7 +62,7 @@ module Gitlab ...@@ -62,7 +62,7 @@ module Gitlab
rescue TooManyIps => ex rescue TooManyIps => ex
Rails.logger.info ex.message Rails.logger.info ex.message
[429, { 'Content-Type' => 'text/plain', 'Retry-After' => UniqueIpsLimiter.config.unique_ips_limit_time_window }, ["Retry later\n"]] [403, { 'Content-Type' => 'text/plain', 'Retry-After' => UniqueIpsLimiter.config.unique_ips_limit_time_window }, ["Too many logins from different IPs\n"]]
end end
end end
end end
......
...@@ -25,9 +25,35 @@ describe SessionsController do ...@@ -25,9 +25,35 @@ describe SessionsController do
expect(subject.current_user). to eq user expect(subject.current_user). to eq user
end end
it "creates an audit log record" do it 'creates an audit log record' do
expect { post(:create, user: { login: user.username, password: user.password }) }.to change { SecurityEvent.count }.by(1) expect { post(:create, user: { login: user.username, password: user.password }) }.to change { SecurityEvent.count }.by(1)
expect(SecurityEvent.last.details[:with]).to eq("standard") 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')
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
end end
end end
......
require 'spec_helper' require 'spec_helper'
describe Gitlab::Auth::UniqueIpsLimiter, lib: true do describe Gitlab::Auth::UniqueIpsLimiter, :redis, lib: true do
let(:user) { create(:user) } let(:user) { create(:user) }
before(:each) do
Gitlab::Redis.with do |redis|
redis.del("user_unique_ips:#{user.id}")
end
end
describe '#count_unique_ips' do describe '#count_unique_ips' do
context 'non unique IPs' do context 'non unique IPs' do
it 'properly counts them' do it 'properly counts them' do
...@@ -25,7 +19,7 @@ describe Gitlab::Auth::UniqueIpsLimiter, lib: true do ...@@ -25,7 +19,7 @@ describe Gitlab::Auth::UniqueIpsLimiter, lib: true do
end end
it 'resets count after specified time window' do it 'resets count after specified time window' do
cur_time = Time.now.to_i cur_time = Time.now
allow(Time).to receive(:now).and_return(cur_time) 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.2')).to eq(1)
...@@ -51,15 +45,15 @@ describe Gitlab::Auth::UniqueIpsLimiter, lib: true do ...@@ -51,15 +45,15 @@ describe Gitlab::Auth::UniqueIpsLimiter, lib: true do
end end
it 'blocks user trying to login from second ip' do it 'blocks user trying to login from second ip' do
RequestStore[:client_ip] = '192.168.1.1' 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)
RequestStore[:client_ip] = '192.168.1.2' 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) expect { Gitlab::Auth::UniqueIpsLimiter.limit_user! { user } }.to raise_error(Gitlab::Auth::TooManyIps)
end end
it 'allows user trying to login from the same ip twice' do it 'allows user trying to login from the same ip twice' do
RequestStore[:client_ip] = '192.168.1.1' 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)
expect(Gitlab::Auth::UniqueIpsLimiter.limit_user! { user }).to eq(user) expect(Gitlab::Auth::UniqueIpsLimiter.limit_user! { user }).to eq(user)
end end
...@@ -71,13 +65,13 @@ describe Gitlab::Auth::UniqueIpsLimiter, lib: true do ...@@ -71,13 +65,13 @@ describe Gitlab::Auth::UniqueIpsLimiter, lib: true do
end end
it 'blocks user trying to login from third ip' do it 'blocks user trying to login from third ip' do
RequestStore[:client_ip] = '192.168.1.1' 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)
RequestStore[:client_ip] = '192.168.1.2' allow(Gitlab::RequestContext).to receive(:client_ip).and_return('192.168.1.2')
expect(Gitlab::Auth::UniqueIpsLimiter.limit_user! { user }).to eq(user) expect(Gitlab::Auth::UniqueIpsLimiter.limit_user! { user }).to eq(user)
RequestStore[:client_ip] = '192.168.1.3' 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) expect { Gitlab::Auth::UniqueIpsLimiter.limit_user! { user } }.to raise_error(Gitlab::Auth::TooManyIps)
end end
end end
......
...@@ -58,6 +58,30 @@ describe Gitlab::Auth, lib: true do ...@@ -58,6 +58,30 @@ 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)) 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 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
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))
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
context 'while using LFS authenticate' do context 'while using LFS authenticate' do
it 'recognizes user lfs tokens' do it 'recognizes user lfs tokens' do
user = create(:user) user = create(:user)
......
...@@ -4,27 +4,65 @@ describe API::API, api: true do ...@@ -4,27 +4,65 @@ describe API::API, api: true do
include ApiHelpers include ApiHelpers
let!(:user) { create(:user) } let!(:user) { create(:user) }
let!(:application) { Doorkeeper::Application.create!(name: "MyApp", redirect_uri: "https://app.com", owner: user) } let!(:application) { Doorkeeper::Application.create!(name: 'MyApp', redirect_uri: 'https://app.com', owner: user) }
let!(:token) { Doorkeeper::AccessToken.create! application_id: application.id, resource_owner_id: user.id, scopes: "api" } let!(:token) { Doorkeeper::AccessToken.create! application_id: application.id, resource_owner_id: user.id, scopes: 'api' }
describe "when unauthenticated" do describe 'when unauthenticated' do
it "returns authentication success" do it 'returns authentication success' do
get api("/user"), access_token: token.token get api('/user'), access_token: token.token
expect(response).to have_http_status(200) expect(response).to have_http_status(200)
end end
include_context 'limit login to only one ip' do
it 'allows login twice from the same ip' do
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 end
describe "when token invalid" do describe 'when token invalid' do
it "returns authentication error" do it 'returns authentication error' do
get api("/user"), access_token: "123a" get api('/user'), access_token: '123a'
expect(response).to have_http_status(401) expect(response).to have_http_status(401)
end end
end end
describe "authorization by private token" do describe 'authorization by private token' do
it "returns authentication success" do it 'returns authentication success' do
get api("/user", user) get api('/user', user)
expect(response).to have_http_status(200) expect(response).to have_http_status(200)
end end
include_context 'limit login to only one ip' do
it 'allows login twice from the same ip' do
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 end
end end
shared_context 'limit login to only one ip', :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(1000)
allow(Gitlab::Auth::UniqueIpsLimiter).to receive_message_chain(:config, :unique_ips_limit_per_user).and_return(1)
end
def change_ip(ip)
allow(Gitlab::RequestContext).to receive(:client_ip).and_return(ip)
end
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
expect { operation }.not_to raise_error
expect { operation }.not_to raise_error
end
it 'blocks user authenticating from two distinct ips' do
expect { operation }.not_to raise_error
change_ip('ip2')
expect { operation }.to raise_error(Gitlab::Auth::TooManyIps)
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment