Allow limiting logging in users from too many different IPs.

12 files changed, 278 insertions, 16 deletions

diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb
index 7d81c96262f..3f5b92d9a99 100644
--- a/app/controllers/sessions_controller.rb
+++ b/app/controllers/sessions_controller.rb
@@ -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
 
diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb
index 255e8c4ff78..2f64fb1a6a2 100644
--- a/app/models/application_setting.rb
+++ b/app/models/application_setting.rb
@@ -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,
diff --git a/config/application.rb b/config/application.rb
index f1a986d1731..c4dea9e92b0 100644
--- a/config/application.rb
+++ b/config/application.rb
@@ -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
diff --git a/config/initializers/doorkeeper.rb b/config/initializers/doorkeeper.rb
index 88cd0f5f652..44b658e5872 100644
--- a/config/initializers/doorkeeper.rb
+++ b/config/initializers/doorkeeper.rb
@@ -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.
diff --git a/config/initializers/request_context.rb b/config/initializers/request_context.rb
new file mode 100644
index 00000000000..0b485fc1adc
--- /dev/null
+++ b/config/initializers/request_context.rb
@@ -0,0 +1,3 @@
+Rails.application.configure do |config|
+  config.middleware.insert_after RequestStore::Middleware, Gitlab::RequestContext
+end
diff --git a/db/migrate/20170210131347_add_unique_ips_limit_to_application_settings.rb b/db/migrate/20170210131347_add_unique_ips_limit_to_application_settings.rb
new file mode 100644
index 00000000000..2aa305f6b58
--- /dev/null
+++ b/db/migrate/20170210131347_add_unique_ips_limit_to_application_settings.rb
@@ -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
diff --git a/db/schema.rb b/db/schema.rb
index 9deed46530e..8aed4e13b28 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -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
diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb
index 0a5abc92190..be055080853 100644
--- a/lib/gitlab/auth.rb
+++ b/lib/gitlab/auth.rb
@@ -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
 
diff --git a/lib/gitlab/auth/unique_ips_limiter.rb b/lib/gitlab/auth/unique_ips_limiter.rb
new file mode 100644
index 00000000000..21307eb35e4
--- /dev/null
+++ b/lib/gitlab/auth/unique_ips_limiter.rb
@@ -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
diff --git a/lib/gitlab/request_context.rb b/lib/gitlab/request_context.rb
new file mode 100644
index 00000000000..5daf04dc92b
--- /dev/null
+++ b/lib/gitlab/request_context.rb
@@ -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
\ No newline at end of file
diff --git a/spec/lib/gitlab/auth/unique_ips_limiter_spec.rb b/spec/lib/gitlab/auth/unique_ips_limiter_spec.rb
new file mode 100644
index 00000000000..8e9fea0724a
--- /dev/null
+++ b/spec/lib/gitlab/auth/unique_ips_limiter_spec.rb
@@ -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
diff --git a/spec/lib/gitlab/request_context_spec.rb b/spec/lib/gitlab/request_context_spec.rb
new file mode 100644
index 00000000000..3565fab6ded
--- /dev/null
+++ b/spec/lib/gitlab/request_context_spec.rb
@@ -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