From 70b9d8da4c24bc2317220bedb81b5d2ecf34c351 Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Fri, 3 Mar 2017 18:10:22 +0100 Subject: [PATCH] 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 -- GitLab