Allow limiting logging in users from too many different IPs.
This commit is contained in:
parent
27729aa3a4
commit
e5cf3f51fb
12 changed files with 278 additions and 16 deletions
|
@ -67,10 +67,12 @@ class SessionsController < Devise::SessionsController
|
|||
end
|
||||
|
||||
def find_user
|
||||
if session[:otp_user_id]
|
||||
User.find(session[:otp_user_id])
|
||||
elsif user_params[:login]
|
||||
User.by_login(user_params[:login])
|
||||
Gitlab::Auth::UniqueIpsLimiter.limit_user! do
|
||||
if session[:otp_user_id]
|
||||
User.find(session[:otp_user_id])
|
||||
elsif user_params[:login]
|
||||
User.by_login(user_params[:login])
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
|
|
|
@ -184,6 +184,9 @@ class ApplicationSetting < ActiveRecord::Base
|
|||
domain_whitelist: Settings.gitlab['domain_whitelist'],
|
||||
gravatar_enabled: Settings.gravatar['enabled'],
|
||||
help_page_text: nil,
|
||||
unique_ips_limit_per_user: 10,
|
||||
unique_ips_limit_time_window: 3600,
|
||||
unique_ips_limit_enabled: false,
|
||||
housekeeping_bitmaps_enabled: true,
|
||||
housekeeping_enabled: true,
|
||||
housekeeping_full_repack_period: 50,
|
||||
|
|
|
@ -7,6 +7,9 @@ Bundler.require(:default, Rails.env)
|
|||
module Gitlab
|
||||
class Application < Rails::Application
|
||||
require_dependency Rails.root.join('lib/gitlab/redis')
|
||||
require_dependency Rails.root.join('lib/gitlab/request_context')
|
||||
require_dependency Rails.root.join('lib/gitlab/auth')
|
||||
require_dependency Rails.root.join('lib/gitlab/auth/unique_ips_limiter')
|
||||
|
||||
# Settings in config/environments/* take precedence over those specified here.
|
||||
# Application configuration should go into files in config/initializers
|
||||
|
@ -111,6 +114,8 @@ module Gitlab
|
|||
|
||||
config.middleware.insert_before Warden::Manager, Rack::Attack
|
||||
|
||||
config.middleware.insert_before Warden::Manager, Gitlab::Auth::UniqueIpsLimiter
|
||||
|
||||
# Allow access to GitLab API from other domains
|
||||
config.middleware.insert_before Warden::Manager, Rack::Cors do
|
||||
allow do
|
||||
|
|
|
@ -12,8 +12,10 @@ Doorkeeper.configure do
|
|||
end
|
||||
|
||||
resource_owner_from_credentials do |routes|
|
||||
user = Gitlab::Auth.find_with_user_password(params[:username], params[:password])
|
||||
user unless user.try(:two_factor_enabled?)
|
||||
Gitlab::Auth::UniqueIpsLimiter.limit_user! do
|
||||
user = Gitlab::Auth.find_with_user_password(params[:username], params[:password])
|
||||
user unless user.try(:two_factor_enabled?)
|
||||
end
|
||||
end
|
||||
|
||||
# If you want to restrict access to the web interface for adding oauth authorized applications, you need to declare the block below.
|
||||
|
|
3
config/initializers/request_context.rb
Normal file
3
config/initializers/request_context.rb
Normal file
|
@ -0,0 +1,3 @@
|
|||
Rails.application.configure do |config|
|
||||
config.middleware.insert_after RequestStore::Middleware, Gitlab::RequestContext
|
||||
end
|
|
@ -0,0 +1,17 @@
|
|||
class AddUniqueIpsLimitToApplicationSettings < ActiveRecord::Migration
|
||||
include Gitlab::Database::MigrationHelpers
|
||||
DOWNTIME = false
|
||||
disable_ddl_transaction!
|
||||
|
||||
def up
|
||||
add_column_with_default(:application_settings, :unique_ips_limit_per_user, :integer, default: 10)
|
||||
add_column_with_default(:application_settings, :unique_ips_limit_time_window, :integer, default: 3600)
|
||||
add_column_with_default(:application_settings, :unique_ips_limit_enabled, :boolean, default: false)
|
||||
end
|
||||
|
||||
def down
|
||||
remove_column(:application_settings, :unique_ips_limit_per_user)
|
||||
remove_column(:application_settings, :unique_ips_limit_time_window)
|
||||
remove_column(:application_settings, :unique_ips_limit_enabled)
|
||||
end
|
||||
end
|
|
@ -112,6 +112,9 @@ ActiveRecord::Schema.define(version: 20170305203726) do
|
|||
t.integer "max_pages_size", default: 100, null: false
|
||||
t.integer "terminal_max_session_time", default: 0, null: false
|
||||
t.string "default_artifacts_expire_in", default: "0", null: false
|
||||
t.integer "unique_ips_limit_per_user", default: 10, null: false
|
||||
t.integer "unique_ips_limit_time_window", default: 3600, null: false
|
||||
t.boolean "unique_ips_limit_enabled", default: false, null: false
|
||||
end
|
||||
|
||||
create_table "audit_events", force: :cascade do |t|
|
||||
|
@ -252,8 +255,8 @@ ActiveRecord::Schema.define(version: 20170305203726) do
|
|||
t.integer "lock_version"
|
||||
end
|
||||
|
||||
add_index "ci_commits", ["gl_project_id", "ref", "status"], name: "index_ci_commits_on_gl_project_id_and_ref_and_status", using: :btree
|
||||
add_index "ci_commits", ["gl_project_id", "sha"], name: "index_ci_commits_on_gl_project_id_and_sha", using: :btree
|
||||
add_index "ci_commits", ["gl_project_id", "status"], name: "index_ci_commits_on_gl_project_id_and_status", using: :btree
|
||||
add_index "ci_commits", ["gl_project_id"], name: "index_ci_commits_on_gl_project_id", using: :btree
|
||||
add_index "ci_commits", ["status"], name: "index_ci_commits_on_status", using: :btree
|
||||
add_index "ci_commits", ["user_id"], name: "index_ci_commits_on_user_id", using: :btree
|
||||
|
|
|
@ -22,23 +22,27 @@ module Gitlab
|
|||
user_with_password_for_git(login, password) ||
|
||||
Gitlab::Auth::Result.new
|
||||
|
||||
Gitlab::Auth::UniqueIpsLimiter.limit_user! { result.actor }
|
||||
|
||||
rate_limit!(ip, success: result.success?, login: login)
|
||||
|
||||
result
|
||||
end
|
||||
|
||||
def find_with_user_password(login, password)
|
||||
user = User.by_login(login)
|
||||
Gitlab::Auth::UniqueIpsLimiter.limit_user! do
|
||||
user = User.by_login(login)
|
||||
|
||||
# If no user is found, or it's an LDAP server, try LDAP.
|
||||
# LDAP users are only authenticated via LDAP
|
||||
if user.nil? || user.ldap_user?
|
||||
# Second chance - try LDAP authentication
|
||||
return nil unless Gitlab::LDAP::Config.enabled?
|
||||
# If no user is found, or it's an LDAP server, try LDAP.
|
||||
# LDAP users are only authenticated via LDAP
|
||||
if user.nil? || user.ldap_user?
|
||||
# Second chance - try LDAP authentication
|
||||
return nil unless Gitlab::LDAP::Config.enabled?
|
||||
|
||||
Gitlab::LDAP::Authentication.login(login, password)
|
||||
else
|
||||
user if user.valid_password?(password)
|
||||
Gitlab::LDAP::Authentication.login(login, password)
|
||||
else
|
||||
user if user.valid_password?(password)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
|
|
70
lib/gitlab/auth/unique_ips_limiter.rb
Normal file
70
lib/gitlab/auth/unique_ips_limiter.rb
Normal file
|
@ -0,0 +1,70 @@
|
|||
module Gitlab
|
||||
module Auth
|
||||
class TooManyIps < StandardError
|
||||
attr_reader :user_id, :ip, :unique_ips_count
|
||||
|
||||
def initialize(user_id, ip, unique_ips_count)
|
||||
@user_id = user_id
|
||||
@ip = ip
|
||||
@unique_ips_count = unique_ips_count
|
||||
end
|
||||
|
||||
def message
|
||||
"User #{user_id} from IP: #{ip} tried logging from too many ips: #{unique_ips_count}"
|
||||
end
|
||||
end
|
||||
|
||||
class UniqueIpsLimiter
|
||||
USER_UNIQUE_IPS_PREFIX = 'user_unique_ips'
|
||||
|
||||
class << self
|
||||
def limit_user_id!(user_id)
|
||||
if config.unique_ips_limit_enabled
|
||||
ip = RequestContext.client_ip
|
||||
unique_ips = count_unique_ips(user_id, ip)
|
||||
raise TooManyIps.new(user_id, ip, unique_ips) if unique_ips > config.unique_ips_limit_per_user
|
||||
end
|
||||
end
|
||||
|
||||
def limit_user!(user = nil)
|
||||
user = yield if user.nil?
|
||||
limit_user_id!(user.id) unless user.nil?
|
||||
user
|
||||
end
|
||||
|
||||
def config
|
||||
Gitlab::CurrentSettings.current_application_settings
|
||||
end
|
||||
|
||||
def count_unique_ips(user_id, ip)
|
||||
time = Time.now.to_i
|
||||
key = "#{USER_UNIQUE_IPS_PREFIX}:#{user_id}"
|
||||
|
||||
Gitlab::Redis.with do |redis|
|
||||
unique_ips_count = nil
|
||||
redis.multi do |r|
|
||||
r.zadd(key, time, ip)
|
||||
r.zremrangebyscore(key, 0, time - config.unique_ips_limit_time_window)
|
||||
unique_ips_count = r.zcard(key)
|
||||
end
|
||||
unique_ips_count.value
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
def initialize(app)
|
||||
@app = app
|
||||
end
|
||||
|
||||
def call(env)
|
||||
begin
|
||||
@app.call(env)
|
||||
rescue TooManyIps => ex
|
||||
|
||||
Rails.logger.info ex.message
|
||||
[429, {'Content-Type' => 'text/plain', 'Retry-After' => UniqueIpsLimiter.config.unique_ips_limit_time_window }, ["Retry later\n"]]
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
25
lib/gitlab/request_context.rb
Normal file
25
lib/gitlab/request_context.rb
Normal file
|
@ -0,0 +1,25 @@
|
|||
module Gitlab
|
||||
class RequestStoreNotActive < StandardError
|
||||
end
|
||||
|
||||
class RequestContext
|
||||
class << self
|
||||
def client_ip
|
||||
RequestStore[:client_ip]
|
||||
end
|
||||
end
|
||||
|
||||
def initialize(app)
|
||||
@app = app
|
||||
end
|
||||
|
||||
def call(env)
|
||||
raise RequestStoreNotActive.new unless RequestStore.active?
|
||||
req = Rack::Request.new(env)
|
||||
|
||||
RequestStore[:client_ip] = req.ip
|
||||
|
||||
@app.call(env)
|
||||
end
|
||||
end
|
||||
end
|
88
spec/lib/gitlab/auth/unique_ips_limiter_spec.rb
Normal file
88
spec/lib/gitlab/auth/unique_ips_limiter_spec.rb
Normal file
|
@ -0,0 +1,88 @@
|
|||
require 'spec_helper'
|
||||
|
||||
describe Gitlab::Auth::UniqueIpsLimiter, lib: true do
|
||||
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
|
||||
|
||||
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)
|
||||
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)
|
||||
end
|
||||
end
|
||||
|
||||
it 'resets count after specified time window' do
|
||||
cur_time = Time.now.to_i
|
||||
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)
|
||||
|
||||
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)
|
||||
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
|
||||
RequestStore[:client_ip] = '192.168.1.1'
|
||||
expect(Gitlab::Auth::UniqueIpsLimiter.limit_user! { user }).to eq(user)
|
||||
|
||||
RequestStore[:client_ip] = '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
|
||||
RequestStore[:client_ip] = '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
|
||||
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
|
||||
|
||||
it 'blocks user trying to login from third ip' do
|
||||
RequestStore[:client_ip] = '192.168.1.1'
|
||||
expect(Gitlab::Auth::UniqueIpsLimiter.limit_user! { user }).to eq(user)
|
||||
|
||||
RequestStore[:client_ip] = '192.168.1.2'
|
||||
expect(Gitlab::Auth::UniqueIpsLimiter.limit_user! { user }).to eq(user)
|
||||
|
||||
RequestStore[:client_ip] = '192.168.1.3'
|
||||
expect { Gitlab::Auth::UniqueIpsLimiter.limit_user! { user } }.to raise_error(Gitlab::Auth::TooManyIps)
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
40
spec/lib/gitlab/request_context_spec.rb
Normal file
40
spec/lib/gitlab/request_context_spec.rb
Normal file
|
@ -0,0 +1,40 @@
|
|||
require 'spec_helper'
|
||||
|
||||
describe Gitlab::RequestContext, lib: true do
|
||||
describe '#client_ip' do
|
||||
subject { Gitlab::RequestContext.client_ip }
|
||||
let(:app) { -> env {} }
|
||||
let(:env) { Hash.new }
|
||||
|
||||
context 'when RequestStore::Middleware is used' do
|
||||
around(:each) do |example|
|
||||
RequestStore::Middleware.new(-> env { example.run }).call({})
|
||||
end
|
||||
|
||||
context 'request' do
|
||||
let(:ip) { '192.168.1.11' }
|
||||
|
||||
before do
|
||||
allow_any_instance_of(Rack::Request).to receive(:ip).and_return(ip)
|
||||
Gitlab::RequestContext.new(app).call(env)
|
||||
end
|
||||
|
||||
it { is_expected.to eq(ip) }
|
||||
end
|
||||
|
||||
context 'before RequestContext mw run' do
|
||||
it { is_expected.to be_nil }
|
||||
end
|
||||
end
|
||||
|
||||
context 'RequestStore is not active' do
|
||||
it { is_expected.to be_nil }
|
||||
|
||||
context 'when RequestContext mw is run' do
|
||||
subject { -> { Gitlab::RequestContext.new(app).call(env) } }
|
||||
|
||||
it { is_expected.to raise_error(Gitlab::RequestStoreNotActive) }
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
Loading…
Reference in a new issue