From e5cf3f51fb568361a247d715facb6cd9bb15bb16 Mon Sep 17 00:00:00 2001
From: Pawel Chojnacki <pawel@chojnacki.ws>
Date: Mon, 6 Feb 2017 13:48:46 +0100
Subject: [PATCH 01/12] Allow limiting logging in users from too many different
 IPs.

---
 app/controllers/sessions_controller.rb        | 10 ++-
 app/models/application_setting.rb             |  3 +
 config/application.rb                         |  5 ++
 config/initializers/doorkeeper.rb             |  6 +-
 config/initializers/request_context.rb        |  3 +
 ...nique_ips_limit_to_application_settings.rb | 17 ++++
 db/schema.rb                                  |  5 +-
 lib/gitlab/auth.rb                            | 22 +++--
 lib/gitlab/auth/unique_ips_limiter.rb         | 70 +++++++++++++++
 lib/gitlab/request_context.rb                 | 25 ++++++
 .../gitlab/auth/unique_ips_limiter_spec.rb    | 88 +++++++++++++++++++
 spec/lib/gitlab/request_context_spec.rb       | 40 +++++++++
 12 files changed, 278 insertions(+), 16 deletions(-)
 create mode 100644 config/initializers/request_context.rb
 create mode 100644 db/migrate/20170210131347_add_unique_ips_limit_to_application_settings.rb
 create mode 100644 lib/gitlab/auth/unique_ips_limiter.rb
 create mode 100644 lib/gitlab/request_context.rb
 create mode 100644 spec/lib/gitlab/auth/unique_ips_limiter_spec.rb
 create mode 100644 spec/lib/gitlab/request_context_spec.rb

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

From 66dc71599cb698d380e14be7230ae3495c78d266 Mon Sep 17 00:00:00 2001
From: Pawel Chojnacki <pawel@chojnacki.ws>
Date: Thu, 16 Feb 2017 11:21:30 +0100
Subject: [PATCH 02/12] Cleanup formatting

---
 lib/gitlab/auth/unique_ips_limiter.rb           | 2 +-
 lib/gitlab/request_context.rb                   | 2 +-
 spec/lib/gitlab/auth/unique_ips_limiter_spec.rb | 2 --
 spec/lib/gitlab/request_context_spec.rb         | 4 ++--
 4 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/lib/gitlab/auth/unique_ips_limiter.rb b/lib/gitlab/auth/unique_ips_limiter.rb
index 21307eb35e4..01850ae31e8 100644
--- a/lib/gitlab/auth/unique_ips_limiter.rb
+++ b/lib/gitlab/auth/unique_ips_limiter.rb
@@ -62,7 +62,7 @@ module Gitlab
         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"]]
+          [429, { 'Content-Type' => 'text/plain', 'Retry-After' => UniqueIpsLimiter.config.unique_ips_limit_time_window }, ["Retry later\n"]]
         end
       end
     end
diff --git a/lib/gitlab/request_context.rb b/lib/gitlab/request_context.rb
index 5daf04dc92b..36a5d94d98a 100644
--- a/lib/gitlab/request_context.rb
+++ b/lib/gitlab/request_context.rb
@@ -22,4 +22,4 @@ module Gitlab
       @app.call(env)
     end
   end
-end
\ No newline at end of file
+end
diff --git a/spec/lib/gitlab/auth/unique_ips_limiter_spec.rb b/spec/lib/gitlab/auth/unique_ips_limiter_spec.rb
index 8e9fea0724a..ccaddddf98f 100644
--- a/spec/lib/gitlab/auth/unique_ips_limiter_spec.rb
+++ b/spec/lib/gitlab/auth/unique_ips_limiter_spec.rb
@@ -10,7 +10,6 @@ describe Gitlab::Auth::UniqueIpsLimiter, lib: true do
   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)
@@ -39,7 +38,6 @@ describe Gitlab::Auth::UniqueIpsLimiter, lib: true do
     end
   end
 
-
   describe '#limit_user!' do
     context 'when unique ips limit is enabled' do
       before do
diff --git a/spec/lib/gitlab/request_context_spec.rb b/spec/lib/gitlab/request_context_spec.rb
index 3565fab6ded..69c5549c39c 100644
--- a/spec/lib/gitlab/request_context_spec.rb
+++ b/spec/lib/gitlab/request_context_spec.rb
@@ -3,12 +3,12 @@ require 'spec_helper'
 describe Gitlab::RequestContext, lib: true do
   describe '#client_ip' do
     subject { Gitlab::RequestContext.client_ip }
-    let(:app) { -> env {} }
+    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({})
+        RequestStore::Middleware.new(-> (env) { example.run }).call({})
       end
 
       context 'request' do

From 8993801f0cefdc64b46b8fe30622cc78eaa03173 Mon Sep 17 00:00:00 2001
From: Pawel Chojnacki <pawel@chojnacki.ws>
Date: Fri, 17 Feb 2017 12:52:27 +0100
Subject: [PATCH 03/12] Test various login scenarios if the limit gets enforced

---
 lib/api/api.rb                                |  4 ++
 lib/api/helpers.rb                            | 15 ++---
 lib/gitlab/auth.rb                            |  2 +-
 lib/gitlab/auth/unique_ips_limiter.rb         |  2 +-
 spec/controllers/sessions_controller_spec.rb  | 30 +++++++++-
 .../gitlab/auth/unique_ips_limiter_spec.rb    | 22 +++----
 spec/lib/gitlab/auth_spec.rb                  | 24 ++++++++
 spec/requests/api/doorkeeper_access_spec.rb   | 60 +++++++++++++++----
 .../unique_ip_check_shared_examples.rb        | 27 +++++++++
 9 files changed, 150 insertions(+), 36 deletions(-)
 create mode 100644 spec/support/unique_ip_check_shared_examples.rb

diff --git a/lib/api/api.rb b/lib/api/api.rb
index 89449ce8813..6f37fa9d8e9 100644
--- a/lib/api/api.rb
+++ b/lib/api/api.rb
@@ -60,6 +60,10 @@ module API
       error! e.message, e.status, e.headers
     end
 
+    rescue_from Gitlab::Auth::TooManyIps do |e|
+      rack_response({'message'=>'403 Forbidden'}.to_json, 403)
+    end
+
     rescue_from :all do |exception|
       handle_api_exception(exception)
     end
diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb
index a43252a4661..f325f0a3050 100644
--- a/lib/api/helpers.rb
+++ b/lib/api/helpers.rb
@@ -336,16 +336,17 @@ module API
 
     def 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 ||= doorkeeper_guard(scopes: @scopes)
+        @initial_current_user ||= find_user_from_warden
 
-      @initial_current_user ||= find_user_by_private_token(scopes: @scopes)
-      @initial_current_user ||= doorkeeper_guard(scopes: @scopes)
-      @initial_current_user ||= find_user_from_warden
+        unless @initial_current_user && Gitlab::UserAccess.new(@initial_current_user).allowed?
+          @initial_current_user = nil
+        end
 
-      unless @initial_current_user && Gitlab::UserAccess.new(@initial_current_user).allowed?
-        @initial_current_user = nil
+        @initial_current_user
       end
-
-      @initial_current_user
     end
 
     def sudo!
diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb
index be055080853..8e2aee2d7a0 100644
--- a/lib/gitlab/auth.rb
+++ b/lib/gitlab/auth.rb
@@ -22,7 +22,7 @@ module Gitlab
           user_with_password_for_git(login, password) ||
           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)
 
diff --git a/lib/gitlab/auth/unique_ips_limiter.rb b/lib/gitlab/auth/unique_ips_limiter.rb
index 01850ae31e8..7f849ef4c38 100644
--- a/lib/gitlab/auth/unique_ips_limiter.rb
+++ b/lib/gitlab/auth/unique_ips_limiter.rb
@@ -62,7 +62,7 @@ module Gitlab
         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"]]
+          [403, { 'Content-Type' => 'text/plain', 'Retry-After' => UniqueIpsLimiter.config.unique_ips_limit_time_window }, ["Too many logins from different IPs\n"]]
         end
       end
     end
diff --git a/spec/controllers/sessions_controller_spec.rb b/spec/controllers/sessions_controller_spec.rb
index b56c7880b64..f1157d159ab 100644
--- a/spec/controllers/sessions_controller_spec.rb
+++ b/spec/controllers/sessions_controller_spec.rb
@@ -25,9 +25,35 @@ describe SessionsController do
           expect(subject.current_user). to eq user
         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(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
diff --git a/spec/lib/gitlab/auth/unique_ips_limiter_spec.rb b/spec/lib/gitlab/auth/unique_ips_limiter_spec.rb
index ccaddddf98f..f2472b4310f 100644
--- a/spec/lib/gitlab/auth/unique_ips_limiter_spec.rb
+++ b/spec/lib/gitlab/auth/unique_ips_limiter_spec.rb
@@ -1,14 +1,8 @@
 require 'spec_helper'
 
-describe Gitlab::Auth::UniqueIpsLimiter, lib: true do
+describe Gitlab::Auth::UniqueIpsLimiter, :redis, 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
@@ -25,7 +19,7 @@ describe Gitlab::Auth::UniqueIpsLimiter, lib: true do
     end
 
     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)
 
       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
         end
 
         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)
 
-          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)
         end
 
         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)
         end
@@ -71,13 +65,13 @@ describe Gitlab::Auth::UniqueIpsLimiter, lib: true do
         end
 
         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)
 
-          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)
 
-          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)
         end
       end
diff --git a/spec/lib/gitlab/auth_spec.rb b/spec/lib/gitlab/auth_spec.rb
index b234de4c772..ee70ef34f4f 100644
--- a/spec/lib/gitlab/auth_spec.rb
+++ b/spec/lib/gitlab/auth_spec.rb
@@ -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))
     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
       it 'recognizes user lfs tokens' do
         user = create(:user)
diff --git a/spec/requests/api/doorkeeper_access_spec.rb b/spec/requests/api/doorkeeper_access_spec.rb
index bd9ecaf2685..1cd0701d955 100644
--- a/spec/requests/api/doorkeeper_access_spec.rb
+++ b/spec/requests/api/doorkeeper_access_spec.rb
@@ -4,27 +4,65 @@ describe API::API, api: true  do
   include ApiHelpers
 
   let!(:user) { create(: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!(: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' }
 
-  describe "when unauthenticated" do
-    it "returns authentication success" do
-      get api("/user"), access_token: token.token
+  describe 'when unauthenticated' do
+    it 'returns authentication success' do
+      get api('/user'), access_token: token.token
       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
+        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
 
-  describe "when token invalid" do
-    it "returns authentication error" do
-      get api("/user"), access_token: "123a"
+  describe 'when token invalid' do
+    it 'returns authentication error' do
+      get api('/user'), access_token: '123a'
       expect(response).to have_http_status(401)
     end
   end
 
-  describe "authorization by private token" do
-    it "returns authentication success" do
-      get api("/user", user)
+  describe 'authorization by private token' do
+    it 'returns authentication success' do
+      get api('/user', user)
       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
+        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
diff --git a/spec/support/unique_ip_check_shared_examples.rb b/spec/support/unique_ip_check_shared_examples.rb
new file mode 100644
index 00000000000..ab693b91d4a
--- /dev/null
+++ b/spec/support/unique_ip_check_shared_examples.rb
@@ -0,0 +1,27 @@
+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

From b1da4f7de3019059051e35c806bb03bdacb41fcf Mon Sep 17 00:00:00 2001
From: Pawel Chojnacki <pawel@chojnacki.ws>
Date: Fri, 17 Feb 2017 13:24:32 +0100
Subject: [PATCH 04/12] Cleanup RSpec tests

---
 spec/controllers/sessions_controller_spec.rb  | 23 +-------
 spec/lib/gitlab/auth_spec.rb                  | 28 +++------
 spec/requests/api/doorkeeper_access_spec.rb   | 59 +++++++++----------
 .../unique_ip_check_shared_examples.rb        | 10 +++-
 4 files changed, 47 insertions(+), 73 deletions(-)

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')

From 5173c0939386b8fbf4632d755849bcafc863872e Mon Sep 17 00:00:00 2001
From: Pawel Chojnacki <pawel@chojnacki.ws>
Date: Fri, 17 Feb 2017 13:34:27 +0100
Subject: [PATCH 05/12] Add changelog

---
 .../27520-option-to-prevent-signing-in-from-multiple-ips.yml  | 4 ++++
 1 file changed, 4 insertions(+)
 create mode 100644 changelogs/unreleased/27520-option-to-prevent-signing-in-from-multiple-ips.yml

diff --git a/changelogs/unreleased/27520-option-to-prevent-signing-in-from-multiple-ips.yml b/changelogs/unreleased/27520-option-to-prevent-signing-in-from-multiple-ips.yml
new file mode 100644
index 00000000000..3050b072863
--- /dev/null
+++ b/changelogs/unreleased/27520-option-to-prevent-signing-in-from-multiple-ips.yml
@@ -0,0 +1,4 @@
+---
+title: Option to prevent signing in from multiple ips
+merge_request: 8998
+author:

From 80fbced2e0b8d291173e1002f150bc5551e87359 Mon Sep 17 00:00:00 2001
From: Pawel Chojnacki <pawel@chojnacki.ws>
Date: Fri, 17 Feb 2017 14:04:10 +0100
Subject: [PATCH 06/12] Add admin settings entries

---
 .../admin/application_settings_controller.rb  |  3 +++
 app/models/application_setting.rb             | 10 +++++++++
 .../application_settings/_form.html.haml      | 22 +++++++++++++++++++
 3 files changed, 35 insertions(+)

diff --git a/app/controllers/admin/application_settings_controller.rb b/app/controllers/admin/application_settings_controller.rb
index d807e6263ee..8d831ffdd70 100644
--- a/app/controllers/admin/application_settings_controller.rb
+++ b/app/controllers/admin/application_settings_controller.rb
@@ -138,6 +138,9 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController
       :two_factor_grace_period,
       :user_default_external,
       :user_oauth_applications,
+      :unique_ips_limit_per_user,
+      :unique_ips_limit_time_window,
+      :unique_ips_limit_enabled,
       :version_check_enabled,
       :terminal_max_session_time,
 
diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb
index 2f64fb1a6a2..be632930895 100644
--- a/app/models/application_setting.rb
+++ b/app/models/application_setting.rb
@@ -64,6 +64,16 @@ class ApplicationSetting < ActiveRecord::Base
             presence: true,
             if: :akismet_enabled
 
+  validates :unique_ips_limit_per_user,
+            numericality: { greater_than_or_equal_to: 1 },
+            presence: true,
+            if: :unique_ips_limit_enabled
+
+  validates :unique_ips_limit_time_window,
+            numericality: { greater_than_or_equal_to: 0 },
+            presence: true,
+            if: :unique_ips_limit_enabled
+
   validates :koding_url,
             presence: true,
             if: :koding_enabled
diff --git a/app/views/admin/application_settings/_form.html.haml b/app/views/admin/application_settings/_form.html.haml
index 057b584e1bc..7f25f76d0cb 100644
--- a/app/views/admin/application_settings/_form.html.haml
+++ b/app/views/admin/application_settings/_form.html.haml
@@ -360,6 +360,28 @@
           Generate API key at
           %a{ href: 'http://www.akismet.com', target: 'blank' } http://www.akismet.com
 
+    .form-group
+      .col-sm-offset-2.col-sm-10
+        .checkbox
+          = f.label :unique_ips_limit_enabled do
+            = f.check_box :unique_ips_limit_enabled
+            Limit sign in from multiple ips
+          %span.help-block#recaptcha_help_block Helps prevent malicious users hide their activity
+
+    .form-group
+      = f.label :unique_ips_limit_per_user, 'IPs per user', class: 'control-label col-sm-2'
+      .col-sm-10
+        = f.number_field :unique_ips_limit_per_user, class: 'form-control'
+        .help-block
+          Maximum number of unique IPs per user
+
+    .form-group
+      = f.label :unique_ips_limit_time_window, 'IP expiration time', class: 'control-label col-sm-2'
+      .col-sm-10
+        = f.number_field :unique_ips_limit_time_window, class: 'form-control'
+        .help-block
+          How long an IP will be counted towards the limit
+
   %fieldset
     %legend Abuse reports
     .form-group

From 9cc0ff8f468c54e23172492d97f6d9b428d3ad2e Mon Sep 17 00:00:00 2001
From: Pawel Chojnacki <pawel@chojnacki.ws>
Date: Fri, 17 Feb 2017 14:44:57 +0100
Subject: [PATCH 07/12] Cleanup common code in Unique Ips tests

---
 lib/api/api.rb                                |  2 +-
 lib/gitlab/auth/unique_ips_limiter.rb         |  2 +-
 .../gitlab/auth/unique_ips_limiter_spec.rb    | 66 +++++++------------
 spec/requests/api/doorkeeper_access_spec.rb   | 23 -------
 .../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)
+    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 1' do
-        before do
-          allow(Gitlab::Auth::UniqueIpsLimiter).to receive_message_chain(:config, :unique_ips_limit_per_user).and_return(1)
-        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 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)
+      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 raise_error(Gitlab::Auth::TooManyIps)
-        end
+        change_ip('ip2')
+        expect(Gitlab::Auth::UniqueIpsLimiter.limit_user! { user }).to eq(user)
 
-        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
-      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
-          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 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

From 0ef8a643489ad1a3da4431155326f0a6e206d870 Mon Sep 17 00:00:00 2001
From: Pawel Chojnacki <pawel@chojnacki.ws>
Date: Mon, 20 Feb 2017 15:09:05 +0100
Subject: [PATCH 08/12] Remove unecessary calls to limit_user!, UniqueIps
 Middleware, and address MR review

 - cleanup formating in haml
 - clarify time window is in seconds
 - cleanup straneous chunks in db/schema
 - rename count_uniqe_ips to update_and_return_ips_count
 - other
---
 app/controllers/application_controller.rb     |  4 +++
 app/controllers/sessions_controller.rb        | 10 +++---
 .../application_settings/_form.html.haml      |  5 +--
 config/application.rb                         |  4 ---
 config/initializers/doorkeeper.rb             |  6 ++--
 ...nique_ips_limit_to_application_settings.rb | 12 +++----
 db/schema.rb                                  |  2 +-
 lib/gitlab/auth.rb                            |  3 +-
 lib/gitlab/auth/too_many_ips.rb               | 17 ++++++++++
 lib/gitlab/auth/unique_ips_limiter.rb         | 34 ++-----------------
 lib/gitlab/request_context.rb                 |  1 -
 spec/controllers/sessions_controller_spec.rb  |  1 +
 .../gitlab/auth/unique_ips_limiter_spec.rb    | 33 +++++++++---------
 spec/lib/gitlab/auth_spec.rb                  |  2 +-
 spec/lib/gitlab/request_context_spec.rb       | 10 ------
 15 files changed, 59 insertions(+), 85 deletions(-)
 create mode 100644 lib/gitlab/auth/too_many_ips.rb

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index cc7b7f247e8..0d3ee4f4c1f 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -40,6 +40,10 @@ class ApplicationController < ActionController::Base
     render_403
   end
 
+  rescue_from Gitlab::Auth::TooManyIps do |e|
+    head :forbidden, retry_after: UniqueIpsLimiter.config.unique_ips_limit_time_window
+  end
+
   def redirect_back_or_default(default: root_path, options: {})
     redirect_to request.referer.present? ? :back : default, options
   end
diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb
index 3f5b92d9a99..7d81c96262f 100644
--- a/app/controllers/sessions_controller.rb
+++ b/app/controllers/sessions_controller.rb
@@ -67,12 +67,10 @@ class SessionsController < Devise::SessionsController
   end
 
   def find_user
-    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
+    if session[:otp_user_id]
+      User.find(session[:otp_user_id])
+    elsif user_params[:login]
+      User.by_login(user_params[:login])
     end
   end
 
diff --git a/app/views/admin/application_settings/_form.html.haml b/app/views/admin/application_settings/_form.html.haml
index 7f25f76d0cb..00366b0a8c9 100644
--- a/app/views/admin/application_settings/_form.html.haml
+++ b/app/views/admin/application_settings/_form.html.haml
@@ -366,7 +366,8 @@
           = f.label :unique_ips_limit_enabled do
             = f.check_box :unique_ips_limit_enabled
             Limit sign in from multiple ips
-          %span.help-block#recaptcha_help_block Helps prevent malicious users hide their activity
+          %span.help-block#unique_ip_help_block
+            Helps prevent malicious users hide their activity
 
     .form-group
       = f.label :unique_ips_limit_per_user, 'IPs per user', class: 'control-label col-sm-2'
@@ -380,7 +381,7 @@
       .col-sm-10
         = f.number_field :unique_ips_limit_time_window, class: 'form-control'
         .help-block
-          How long an IP will be counted towards the limit
+          How many seconds an IP will be counted towards the limit
 
   %fieldset
     %legend Abuse reports
diff --git a/config/application.rb b/config/application.rb
index c4dea9e92b0..8003c055b27 100644
--- a/config/application.rb
+++ b/config/application.rb
@@ -8,8 +8,6 @@ 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
@@ -114,8 +112,6 @@ 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 44b658e5872..88cd0f5f652 100644
--- a/config/initializers/doorkeeper.rb
+++ b/config/initializers/doorkeeper.rb
@@ -12,10 +12,8 @@ Doorkeeper.configure do
   end
 
   resource_owner_from_credentials do |routes|
-    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
+    user = Gitlab::Auth.find_with_user_password(params[:username], params[:password])
+    user unless user.try(:two_factor_enabled?)
   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/db/migrate/20170210131347_add_unique_ips_limit_to_application_settings.rb b/db/migrate/20170210131347_add_unique_ips_limit_to_application_settings.rb
index 2aa305f6b58..cbcf9a30b3c 100644
--- a/db/migrate/20170210131347_add_unique_ips_limit_to_application_settings.rb
+++ b/db/migrate/20170210131347_add_unique_ips_limit_to_application_settings.rb
@@ -4,14 +4,14 @@ class AddUniqueIpsLimitToApplicationSettings < ActiveRecord::Migration
   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)
+    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)
+    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 8aed4e13b28..be54b177fa6 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -586,9 +586,9 @@ ActiveRecord::Schema.define(version: 20170305203726) do
   end
 
   add_index "labels", ["group_id", "project_id", "title"], name: "index_labels_on_group_id_and_project_id_and_title", unique: true, using: :btree
+  add_index "labels", ["type", "project_id"], name: "index_labels_on_type_and_project_id", using: :btree
   add_index "labels", ["project_id"], name: "index_labels_on_project_id", using: :btree
   add_index "labels", ["title"], name: "index_labels_on_title", using: :btree
-  add_index "labels", ["type", "project_id"], name: "index_labels_on_type_and_project_id", using: :btree
 
   create_table "lfs_objects", force: :cascade do |t|
     t.string "oid", null: false
diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb
index 8e2aee2d7a0..0a0bd0e781c 100644
--- a/lib/gitlab/auth.rb
+++ b/lib/gitlab/auth.rb
@@ -22,9 +22,8 @@ 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)
+        Gitlab::Auth::UniqueIpsLimiter.limit_user!(result.actor)
 
         result
       end
diff --git a/lib/gitlab/auth/too_many_ips.rb b/lib/gitlab/auth/too_many_ips.rb
new file mode 100644
index 00000000000..ed862791551
--- /dev/null
+++ b/lib/gitlab/auth/too_many_ips.rb
@@ -0,0 +1,17 @@
+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
+  end
+end
diff --git a/lib/gitlab/auth/unique_ips_limiter.rb b/lib/gitlab/auth/unique_ips_limiter.rb
index 4b2b758be8a..7b1aa736769 100644
--- a/lib/gitlab/auth/unique_ips_limiter.rb
+++ b/lib/gitlab/auth/unique_ips_limiter.rb
@@ -1,19 +1,5 @@
 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'
 
@@ -21,7 +7,7 @@ module Gitlab
         def limit_user_id!(user_id)
           if config.unique_ips_limit_enabled
             ip = RequestContext.client_ip
-            unique_ips = count_unique_ips(user_id, ip)
+            unique_ips = update_and_return_ips_count(user_id, ip)
             raise TooManyIps.new(user_id, ip, unique_ips) if unique_ips > config.unique_ips_limit_per_user
           end
         end
@@ -36,8 +22,8 @@ module Gitlab
           Gitlab::CurrentSettings.current_application_settings
         end
 
-        def count_unique_ips(user_id, ip)
-          time = Time.now.to_i
+        def update_and_return_ips_count(user_id, ip)
+          time = Time.now.utc.to_i
           key = "#{USER_UNIQUE_IPS_PREFIX}:#{user_id}"
 
           Gitlab::Redis.with do |redis|
@@ -51,20 +37,6 @@ module Gitlab
           end
         end
       end
-
-      def initialize(app)
-        @app = app
-      end
-
-      def call(env)
-        begin
-          @app.call(env)
-        rescue TooManyIps => ex
-
-          Rails.logger.info ex.message
-          [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
diff --git a/lib/gitlab/request_context.rb b/lib/gitlab/request_context.rb
index 36a5d94d98a..548adf4775a 100644
--- a/lib/gitlab/request_context.rb
+++ b/lib/gitlab/request_context.rb
@@ -14,7 +14,6 @@ module Gitlab
     end
 
     def call(env)
-      raise RequestStoreNotActive.new unless RequestStore.active?
       req = Rack::Request.new(env)
 
       RequestStore[:client_ip] = req.ip
diff --git a/spec/controllers/sessions_controller_spec.rb b/spec/controllers/sessions_controller_spec.rb
index 56ed11737ad..fe11bdf4a78 100644
--- a/spec/controllers/sessions_controller_spec.rb
+++ b/spec/controllers/sessions_controller_spec.rb
@@ -33,6 +33,7 @@ describe SessionsController do
         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
           end
         end
diff --git a/spec/lib/gitlab/auth/unique_ips_limiter_spec.rb b/spec/lib/gitlab/auth/unique_ips_limiter_spec.rb
index 2b21a76c59d..3214786d172 100644
--- a/spec/lib/gitlab/auth/unique_ips_limiter_spec.rb
+++ b/spec/lib/gitlab/auth/unique_ips_limiter_spec.rb
@@ -7,36 +7,35 @@ describe Gitlab::Auth::UniqueIpsLimiter, :redis, lib: true do
   describe '#count_unique_ips' do
     context 'non unique IPs' do
       it 'properly counts them' do
-        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)
+        expect(described_class.update_and_return_ips_count(user.id, 'ip1')).to eq(1)
+        expect(described_class.update_and_return_ips_count(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, 'ip2')).to eq(1)
-        expect(Gitlab::Auth::UniqueIpsLimiter.count_unique_ips(user.id, 'ip3')).to eq(2)
+        expect(described_class.update_and_return_ips_count(user.id, 'ip2')).to eq(1)
+        expect(described_class.update_and_return_ips_count(user.id, 'ip3')).to eq(2)
       end
     end
 
     it 'resets count after specified time window' do
-      cur_time = Time.now
-      allow(Time).to receive(:now).and_return(cur_time)
+      Timecop.freeze do
+        expect(described_class.update_and_return_ips_count(user.id, 'ip2')).to eq(1)
+        expect(described_class.update_and_return_ips_count(user.id, 'ip3')).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, 'ip4')).to eq(1)
-      expect(Gitlab::Auth::UniqueIpsLimiter.count_unique_ips(user.id, 'ip5')).to eq(2)
+        Timecop.travel(Time.now.utc + described_class.config.unique_ips_limit_time_window) do
+          expect(described_class.update_and_return_ips_count(user.id, 'ip4')).to eq(1)
+          expect(described_class.update_and_return_ips_count(user.id, 'ip5')).to eq(2)
+        end
+      end
     end
   end
 
   describe '#limit_user!' do
     include_examples 'user login operation with unique ip limit' do
       def operation
-        Gitlab::Auth::UniqueIpsLimiter.limit_user! { user }
+        described_class.limit_user! { user }
       end
     end
 
@@ -45,13 +44,13 @@ describe Gitlab::Auth::UniqueIpsLimiter, :redis, lib: true do
 
       it 'blocks user trying to login from third ip' do
         change_ip('ip1')
-        expect(Gitlab::Auth::UniqueIpsLimiter.limit_user! { user }).to eq(user)
+        expect(described_class.limit_user! { user }).to eq(user)
 
         change_ip('ip2')
-        expect(Gitlab::Auth::UniqueIpsLimiter.limit_user! { user }).to eq(user)
+        expect(described_class.limit_user! { user }).to eq(user)
 
         change_ip('ip3')
-        expect { Gitlab::Auth::UniqueIpsLimiter.limit_user! { user } }.to raise_error(Gitlab::Auth::TooManyIps)
+        expect { described_class.limit_user! { user } }.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 860189dcf3c..daf8f5c1d6c 100644
--- a/spec/lib/gitlab/auth_spec.rb
+++ b/spec/lib/gitlab/auth_spec.rb
@@ -206,7 +206,7 @@ describe Gitlab::Auth, lib: true do
 
     include_examples 'user login operation with unique ip limit' do
       def operation
-        expect(gl_auth.find_with_user_password(username, password)).to eql user
+        expect(gl_auth.find_with_user_password(username, password)).to eq(user)
       end
     end
 
diff --git a/spec/lib/gitlab/request_context_spec.rb b/spec/lib/gitlab/request_context_spec.rb
index 69c5549c39c..b2828f7e5e0 100644
--- a/spec/lib/gitlab/request_context_spec.rb
+++ b/spec/lib/gitlab/request_context_spec.rb
@@ -26,15 +26,5 @@ describe Gitlab::RequestContext, lib: true 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

From 2ff139ddee49dca7109b9f547e54c6e472c7195b Mon Sep 17 00:00:00 2001
From: Pawel Chojnacki <pawel@chojnacki.ws>
Date: Tue, 21 Feb 2017 22:17:23 +0100
Subject: [PATCH 09/12] Make Warden set_user hook validate user ip uniquness

 + rename shared context
---
 app/controllers/application_controller.rb     |  2 +-
 config/initializers/warden.rb                 |  5 +++++
 spec/controllers/sessions_controller_spec.rb  |  6 ++---
 .../gitlab/auth/unique_ips_limiter_spec.rb    |  2 +-
 spec/requests/api/doorkeeper_access_spec.rb   | 22 +++++++++----------
 .../unique_ip_check_shared_examples.rb        | 14 ++++++------
 6 files changed, 28 insertions(+), 23 deletions(-)
 create mode 100644 config/initializers/warden.rb

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 0d3ee4f4c1f..1c66c530cd2 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -41,7 +41,7 @@ class ApplicationController < ActionController::Base
   end
 
   rescue_from Gitlab::Auth::TooManyIps do |e|
-    head :forbidden, retry_after: UniqueIpsLimiter.config.unique_ips_limit_time_window
+    head :forbidden, retry_after: Gitlab::Auth::UniqueIpsLimiter.config.unique_ips_limit_time_window
   end
 
   def redirect_back_or_default(default: root_path, options: {})
diff --git a/config/initializers/warden.rb b/config/initializers/warden.rb
new file mode 100644
index 00000000000..3d83fb92d56
--- /dev/null
+++ b/config/initializers/warden.rb
@@ -0,0 +1,5 @@
+Rails.application.configure do |config|
+  Warden::Manager.after_set_user do |user, auth, opts|
+    Gitlab::Auth::UniqueIpsLimiter.limit_user!(user)
+  end
+end
diff --git a/spec/controllers/sessions_controller_spec.rb b/spec/controllers/sessions_controller_spec.rb
index fe11bdf4a78..a06c29dd91a 100644
--- a/spec/controllers/sessions_controller_spec.rb
+++ b/spec/controllers/sessions_controller_spec.rb
@@ -30,11 +30,11 @@ describe SessionsController do
           expect(SecurityEvent.last.details[:with]).to eq('standard')
         end
 
-        include_examples 'user login operation with unique ip limit' do
-          def operation
+        include_examples 'user login request with unique ip limit', 302 do
+          def request
             post(:create, user: { login: user.username, password: user.password })
-
             expect(subject.current_user).to eq user
+            subject.sign_out user
           end
         end
       end
diff --git a/spec/lib/gitlab/auth/unique_ips_limiter_spec.rb b/spec/lib/gitlab/auth/unique_ips_limiter_spec.rb
index 3214786d172..94dcddcc30c 100644
--- a/spec/lib/gitlab/auth/unique_ips_limiter_spec.rb
+++ b/spec/lib/gitlab/auth/unique_ips_limiter_spec.rb
@@ -1,7 +1,7 @@
 require 'spec_helper'
 
 describe Gitlab::Auth::UniqueIpsLimiter, :redis, lib: true do
-  include_context 'enable unique ips sign in limit'
+  include_context 'unique ips sign in limit'
   let(:user) { create(:user) }
 
   describe '#count_unique_ips' do
diff --git a/spec/requests/api/doorkeeper_access_spec.rb b/spec/requests/api/doorkeeper_access_spec.rb
index 634b2261c5e..2974875510a 100644
--- a/spec/requests/api/doorkeeper_access_spec.rb
+++ b/spec/requests/api/doorkeeper_access_spec.rb
@@ -4,12 +4,12 @@ describe API::API, api: true do
   include ApiHelpers
 
   let!(:user) { create(: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!(: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" }
 
-  describe 'when unauthenticated' do
-    it 'returns authentication success' do
-      get api('/user'), access_token: token.token
+  describe "unauthenticated" do
+    it "returns authentication success" do
+      get api("/user"), access_token: token.token
       expect(response).to have_http_status(200)
     end
 
@@ -20,16 +20,16 @@ describe API::API, api: true do
     end
   end
 
-  describe 'when token invalid' do
-    it 'returns authentication error' do
-      get api('/user'), access_token: '123a'
+  describe "when token invalid" do
+    it "returns authentication error" do
+      get api("/user"), access_token: "123a"
       expect(response).to have_http_status(401)
     end
   end
 
-  describe 'authorization by private token' do
-    it 'returns authentication success' do
-      get api('/user', user)
+  describe "authorization by private token" do
+    it "returns authentication success" do
+      get api("/user", user)
       expect(response).to have_http_status(200)
     end
 
diff --git a/spec/support/unique_ip_check_shared_examples.rb b/spec/support/unique_ip_check_shared_examples.rb
index 024fb132778..772e6722fc1 100644
--- a/spec/support/unique_ip_check_shared_examples.rb
+++ b/spec/support/unique_ip_check_shared_examples.rb
@@ -1,4 +1,4 @@
-shared_context 'enable unique ips sign in limit' do
+shared_context 'unique ips sign in limit' do
   include StubENV
   before(:each) do
     Gitlab::Redis.with(&:flushall)
@@ -19,7 +19,7 @@ shared_context 'enable unique ips sign in limit' do
 end
 
 shared_examples 'user login operation with unique ip limit' do
-  include_context 'enable unique ips sign in limit' do
+  include_context '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
@@ -38,23 +38,23 @@ shared_examples 'user login operation with unique ip limit' do
   end
 end
 
-shared_examples 'user login request with unique ip limit' do
-  include_context 'enable unique ips sign in limit' do
+shared_examples 'user login request with unique ip limit' do |success_status = 200|
+  include_context '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)
+      expect(response).to have_http_status(success_status)
 
       request
-      expect(response).to have_http_status(200)
+      expect(response).to have_http_status(success_status)
     end
 
     it 'blocks user authenticating from two distinct ips' do
       change_ip('ip')
       request
-      expect(response).to have_http_status(200)
+      expect(response).to have_http_status(success_status)
 
       change_ip('ip2')
       request

From 98bd48c66bd45241d857528791f4e2dd21bfc17a Mon Sep 17 00:00:00 2001
From: Pawel Chojnacki <pawel@chojnacki.ws>
Date: Wed, 22 Feb 2017 14:13:06 +0100
Subject: [PATCH 10/12] Cleanup test phases by introducing request_from_ip and
 operation_from_ip helers

---
 .../unique_ip_check_shared_examples.rb        | 40 +++++++++----------
 1 file changed, 19 insertions(+), 21 deletions(-)

diff --git a/spec/support/unique_ip_check_shared_examples.rb b/spec/support/unique_ip_check_shared_examples.rb
index 772e6722fc1..7cf5a65eeed 100644
--- a/spec/support/unique_ip_check_shared_examples.rb
+++ b/spec/support/unique_ip_check_shared_examples.rb
@@ -16,6 +16,17 @@ shared_context 'unique ips sign in limit' do
   def change_ip(ip)
     allow(Gitlab::RequestContext).to receive(:client_ip).and_return(ip)
   end
+
+  def request_from_ip(ip)
+    change_ip(ip)
+    request
+    response
+  end
+
+  def operation_from_ip(ip)
+    change_ip(ip)
+    operation
+  end
 end
 
 shared_examples 'user login operation with unique ip limit' do
@@ -23,17 +34,13 @@ shared_examples 'user login operation with unique ip 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
-      expect { operation }.not_to raise_error
+      expect { operation_from_ip('ip') }.not_to raise_error
+      expect { operation_from_ip('ip') }.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')
-      expect { operation }.to raise_error(Gitlab::Auth::TooManyIps)
+      expect { operation_from_ip('ip') }.not_to raise_error
+      expect { operation_from_ip('ip2') }.to raise_error(Gitlab::Auth::TooManyIps)
     end
   end
 end
@@ -43,22 +50,13 @@ shared_examples 'user login request with unique ip limit' do |success_status = 2
     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(success_status)
-
-      request
-      expect(response).to have_http_status(success_status)
+      expect(request_from_ip('ip')).to have_http_status(success_status)
+      expect(request_from_ip('ip')).to have_http_status(success_status)
     end
 
     it 'blocks user authenticating from two distinct ips' do
-      change_ip('ip')
-      request
-      expect(response).to have_http_status(success_status)
-
-      change_ip('ip2')
-      request
-      expect(response).to have_http_status(403)
+      expect(request_from_ip('ip')).to have_http_status(success_status)
+      expect(request_from_ip('ip2')).to have_http_status(403)
     end
   end
 end

From 8a9bc24ef87739580c19ee8455bd8224d3c18b3e Mon Sep 17 00:00:00 2001
From: Pawel Chojnacki <pawel@chojnacki.ws>
Date: Tue, 28 Feb 2017 11:12:11 +0100
Subject: [PATCH 11/12] align schema.rb with upstream and fix rubocop warning
 about not freezing mutable constants and empty error classes

---
 db/schema.rb                          | 5 +++--
 lib/gitlab/auth/unique_ips_limiter.rb | 2 +-
 lib/gitlab/request_context.rb         | 3 +--
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/db/schema.rb b/db/schema.rb
index be54b177fa6..3898eed81bd 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -111,10 +111,11 @@ ActiveRecord::Schema.define(version: 20170305203726) do
     t.boolean "plantuml_enabled"
     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.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
+    t.string "default_artifacts_expire_in", default: "0", null: false
   end
 
   create_table "audit_events", force: :cascade do |t|
@@ -255,8 +256,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/unique_ips_limiter.rb b/lib/gitlab/auth/unique_ips_limiter.rb
index 7b1aa736769..4d401eb1b5d 100644
--- a/lib/gitlab/auth/unique_ips_limiter.rb
+++ b/lib/gitlab/auth/unique_ips_limiter.rb
@@ -1,7 +1,7 @@
 module Gitlab
   module Auth
     class UniqueIpsLimiter
-      USER_UNIQUE_IPS_PREFIX = 'user_unique_ips'
+      USER_UNIQUE_IPS_PREFIX = 'user_unique_ips'.freeze
 
       class << self
         def limit_user_id!(user_id)
diff --git a/lib/gitlab/request_context.rb b/lib/gitlab/request_context.rb
index 548adf4775a..1dce18d1733 100644
--- a/lib/gitlab/request_context.rb
+++ b/lib/gitlab/request_context.rb
@@ -1,6 +1,5 @@
 module Gitlab
-  class RequestStoreNotActive < StandardError
-  end
+  RequestStoreNotActive = Class.new(StandardError)
 
   class RequestContext
     class << self

From 70b9d8da4c24bc2317220bedb81b5d2ecf34c351 Mon Sep 17 00:00:00 2001
From: Pawel Chojnacki <pawel@chojnacki.ws>
Date: Fri, 3 Mar 2017 18:10:22 +0100
Subject: [PATCH 12/12] Remove unecessary defaults for uniq ip block, cleanup
 refactoring leftovers

---
 ...31347_add_unique_ips_limit_to_application_settings.rb | 4 ++--
 db/schema.rb                                             | 9 ++++-----
 lib/gitlab/auth/unique_ips_limiter.rb                    | 3 ++-
 lib/gitlab/request_context.rb                            | 2 --
 spec/lib/gitlab/request_context_spec.rb                  | 2 +-
 5 files changed, 9 insertions(+), 11 deletions(-)

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
index cbcf9a30b3c..9ab970134be 100644
--- a/db/migrate/20170210131347_add_unique_ips_limit_to_application_settings.rb
+++ b/db/migrate/20170210131347_add_unique_ips_limit_to_application_settings.rb
@@ -4,8 +4,8 @@ class AddUniqueIpsLimitToApplicationSettings < ActiveRecord::Migration
   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 :application_settings, :unique_ips_limit_per_user, :integer
+    add_column :application_settings, :unique_ips_limit_time_window, :integer
     add_column_with_default :application_settings, :unique_ips_limit_enabled, :boolean, default: false
   end
 
diff --git a/db/schema.rb b/db/schema.rb
index 3898eed81bd..911cb22c8e5 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -111,11 +111,10 @@ ActiveRecord::Schema.define(version: 20170305203726) do
     t.boolean "plantuml_enabled"
     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
     t.string "default_artifacts_expire_in", default: "0", null: false
+    t.integer "unique_ips_limit_per_user"
+    t.integer "unique_ips_limit_time_window"
+    t.boolean "unique_ips_limit_enabled", default: false, null: false
   end
 
   create_table "audit_events", force: :cascade do |t|
@@ -587,9 +586,9 @@ ActiveRecord::Schema.define(version: 20170305203726) do
   end
 
   add_index "labels", ["group_id", "project_id", "title"], name: "index_labels_on_group_id_and_project_id_and_title", unique: true, using: :btree
-  add_index "labels", ["type", "project_id"], name: "index_labels_on_type_and_project_id", using: :btree
   add_index "labels", ["project_id"], name: "index_labels_on_project_id", using: :btree
   add_index "labels", ["title"], name: "index_labels_on_title", using: :btree
+  add_index "labels", ["type", "project_id"], name: "index_labels_on_type_and_project_id", using: :btree
 
   create_table "lfs_objects", force: :cascade do |t|
     t.string "oid", null: false
diff --git a/lib/gitlab/auth/unique_ips_limiter.rb b/lib/gitlab/auth/unique_ips_limiter.rb
index 4d401eb1b5d..bf2239ca150 100644
--- a/lib/gitlab/auth/unique_ips_limiter.rb
+++ b/lib/gitlab/auth/unique_ips_limiter.rb
@@ -8,12 +8,13 @@ module Gitlab
           if config.unique_ips_limit_enabled
             ip = RequestContext.client_ip
             unique_ips = update_and_return_ips_count(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? && block_given?
+          user ||= yield if block_given?
           limit_user_id!(user.id) unless user.nil?
           user
         end
diff --git a/lib/gitlab/request_context.rb b/lib/gitlab/request_context.rb
index 1dce18d1733..fef536ecb0b 100644
--- a/lib/gitlab/request_context.rb
+++ b/lib/gitlab/request_context.rb
@@ -1,6 +1,4 @@
 module Gitlab
-  RequestStoreNotActive = Class.new(StandardError)
-
   class RequestContext
     class << self
       def client_ip
diff --git a/spec/lib/gitlab/request_context_spec.rb b/spec/lib/gitlab/request_context_spec.rb
index b2828f7e5e0..a91c8655cdd 100644
--- a/spec/lib/gitlab/request_context_spec.rb
+++ b/spec/lib/gitlab/request_context_spec.rb
@@ -22,7 +22,7 @@ describe Gitlab::RequestContext, lib: true do
         it { is_expected.to eq(ip) }
       end
 
-      context 'before RequestContext mw run' do
+      context 'before RequestContext middleware run' do
         it { is_expected.to be_nil }
       end
     end