summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--CHANGELOG1
-rw-r--r--app/controllers/admin/application_settings_controller.rb2
-rw-r--r--app/controllers/projects/notes_controller.rb2
-rw-r--r--app/controllers/registrations_controller.rb5
-rw-r--r--app/models/application_setting.rb2
-rw-r--r--app/services/notes/create_service.rb19
-rw-r--r--app/services/notes/post_process_service.rb30
-rw-r--r--app/views/admin/application_settings/_form.html.haml16
-rw-r--r--app/workers/new_note_worker.rb12
-rw-r--r--db/migrate/20160128212447_remove_ip_blocking_settings_from_application_settings.rb6
-rw-r--r--db/schema.rb4
-rw-r--r--lib/dnsxl_check.rb105
-rw-r--r--lib/gitlab/ip_check.rb34
-rw-r--r--spec/lib/dnsxl_check_spec.rb68
-rw-r--r--spec/services/notes/create_service_spec.rb4
-rw-r--r--spec/services/notes/post_process_service_spec.rb26
16 files changed, 80 insertions, 256 deletions
diff --git a/CHANGELOG b/CHANGELOG
index fe0504ec996..9dec6f9809e 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -15,6 +15,7 @@ v 8.5.0 (unreleased)
- Track project import failure
- Fix visibility level text in admin area (Zeger-Jan van de Weg)
- Update the ExternalIssue regex pattern (Blake Hitchcock)
+ - Revert "Add IP check against DNSBLs at account sign-up"
- Deprecate API "merge_request/:merge_request_id/comments". Use "merge_requests/:merge_request_id/notes" instead
- Deprecate API "merge_request/:merge_request_id/...". Use "merge_requests/:merge_request_id/..." instead
diff --git a/app/controllers/admin/application_settings_controller.rb b/app/controllers/admin/application_settings_controller.rb
index 094eef28a43..9943745208e 100644
--- a/app/controllers/admin/application_settings_controller.rb
+++ b/app/controllers/admin/application_settings_controller.rb
@@ -74,8 +74,6 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController
:metrics_timeout,
:metrics_method_call_threshold,
:metrics_sample_interval,
- :ip_blocking_enabled,
- :dnsbl_servers_list,
:recaptcha_enabled,
:recaptcha_site_key,
:recaptcha_private_key,
diff --git a/app/controllers/projects/notes_controller.rb b/app/controllers/projects/notes_controller.rb
index 4a2599dda37..1b9dd568043 100644
--- a/app/controllers/projects/notes_controller.rb
+++ b/app/controllers/projects/notes_controller.rb
@@ -106,7 +106,7 @@ class Projects::NotesController < Projects::ApplicationController
{ notes_left: [note], notes_right: [] }
else
{ notes_left: [], notes_right: [note] }
- end
+ end
else
template = "projects/notes/_diff_notes_with_reply"
locals = { notes: [note] }
diff --git a/app/controllers/registrations_controller.rb b/app/controllers/registrations_controller.rb
index 5efdd613e79..c48175a4c5a 100644
--- a/app/controllers/registrations_controller.rb
+++ b/app/controllers/registrations_controller.rb
@@ -8,11 +8,6 @@ class RegistrationsController < Devise::RegistrationsController
def create
if !Gitlab::Recaptcha.load_configurations! || verify_recaptcha
- if Gitlab::IpCheck.new(request.remote_ip).spam?
- flash[:alert] = 'Could not create an account. This IP is listed for spam.'
- return render action: 'new'
- end
-
super
else
flash[:alert] = "There was an error with the reCAPTCHA code below. Please re-enter the code."
diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb
index 2f3487b53ac..59563b8823c 100644
--- a/app/models/application_setting.rb
+++ b/app/models/application_setting.rb
@@ -43,8 +43,6 @@
# metrics_port :integer default(8089)
# sentry_enabled :boolean default(FALSE)
# sentry_dsn :string
-# ip_blocking_enabled :boolean default(FALSE)
-# dns_blacklist_threshold :float default(0.33)
#
class ApplicationSetting < ActiveRecord::Base
diff --git a/app/services/notes/create_service.rb b/app/services/notes/create_service.rb
index a8486e6a5a1..8d9661167b5 100644
--- a/app/services/notes/create_service.rb
+++ b/app/services/notes/create_service.rb
@@ -6,27 +6,12 @@ module Notes
note.system = false
if note.save
- notification_service.new_note(note)
-
- # Skip system notes, like status changes and cross-references and awards
- unless note.system || note.is_award
- event_service.leave_note(note, note.author)
- note.create_cross_references!
- execute_hooks(note)
- end
+ # Finish the harder work in the background
+ NewNoteWorker.perform_in(2.seconds, note.id, params)
end
note
end
- def hook_data(note)
- Gitlab::NoteDataBuilder.build(note, current_user)
- end
-
- def execute_hooks(note)
- note_data = hook_data(note)
- note.project.execute_hooks(note_data, :note_hooks)
- note.project.execute_services(note_data, :note_hooks)
- end
end
end
diff --git a/app/services/notes/post_process_service.rb b/app/services/notes/post_process_service.rb
new file mode 100644
index 00000000000..f37d3c50cdd
--- /dev/null
+++ b/app/services/notes/post_process_service.rb
@@ -0,0 +1,30 @@
+module Notes
+ class PostProcessService
+
+ attr_accessor :note
+
+ def initialize(note)
+ @note = note
+ end
+
+ def execute
+ # Skip system notes, like status changes and cross-references and awards
+ unless @note.system || @note.is_award
+ EventCreateService.new.leave_note(@note, @note.author)
+ @note.create_cross_references!
+ execute_note_hooks
+ end
+ end
+
+ def hook_data
+ Gitlab::NoteDataBuilder.build(@note, @note.author)
+ end
+
+ def execute_note_hooks
+ note_data = hook_data
+ @note.project.execute_hooks(note_data, :note_hooks)
+ @note.project.execute_services(note_data, :note_hooks)
+ end
+
+ end
+end
diff --git a/app/views/admin/application_settings/_form.html.haml b/app/views/admin/application_settings/_form.html.haml
index baadca09518..c4fb2accdd0 100644
--- a/app/views/admin/application_settings/_form.html.haml
+++ b/app/views/admin/application_settings/_form.html.haml
@@ -215,22 +215,6 @@
.form-group
.col-sm-offset-2.col-sm-10
.checkbox
- = f.label :ip_blocking_enabled do
- = f.check_box :ip_blocking_enabled
- Enable IP check against blacklist at sign-up
- .help-block Helps preventing accounts creation from 'known spam sources'
-
- .form-group
- = f.label :dnsbl_servers_list, class: 'control-label col-sm-2' do
- DNSBL servers list
- .col-sm-10
- = f.text_field :dnsbl_servers_list, class: 'form-control'
- .help-block
- Please enter DNSBL servers separated with comma
-
- .form-group
- .col-sm-offset-2.col-sm-10
- .checkbox
= f.label :recaptcha_enabled do
= f.check_box :recaptcha_enabled
Enable reCAPTCHA
diff --git a/app/workers/new_note_worker.rb b/app/workers/new_note_worker.rb
new file mode 100644
index 00000000000..1b3232cd365
--- /dev/null
+++ b/app/workers/new_note_worker.rb
@@ -0,0 +1,12 @@
+class NewNoteWorker
+ include Sidekiq::Worker
+
+ sidekiq_options queue: :default
+
+ def perform(note_id, note_params)
+ note = Note.find(note_id)
+
+ NotificationService.new.new_note(note)
+ Notes::PostProcessService.new(note).execute
+ end
+end
diff --git a/db/migrate/20160128212447_remove_ip_blocking_settings_from_application_settings.rb b/db/migrate/20160128212447_remove_ip_blocking_settings_from_application_settings.rb
new file mode 100644
index 00000000000..41821cdcc42
--- /dev/null
+++ b/db/migrate/20160128212447_remove_ip_blocking_settings_from_application_settings.rb
@@ -0,0 +1,6 @@
+class RemoveIpBlockingSettingsFromApplicationSettings < ActiveRecord::Migration
+ def change
+ remove_column :application_settings, :ip_blocking_enabled, :boolean, default: false
+ remove_column :application_settings, :dnsbl_servers_list, :text
+ end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 97594011a02..2a2911bfbc7 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -11,7 +11,7 @@
#
# It's strongly recommended that you check this file into your version control system.
-ActiveRecord::Schema.define(version: 20160120172143) do
+ActiveRecord::Schema.define(version: 20160128212447) do
# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"
@@ -64,8 +64,6 @@ ActiveRecord::Schema.define(version: 20160120172143) do
t.integer "metrics_sample_interval", default: 15
t.boolean "sentry_enabled", default: false
t.string "sentry_dsn"
- t.boolean "ip_blocking_enabled", default: false
- t.text "dnsbl_servers_list"
end
create_table "audit_events", force: :cascade do |t|
diff --git a/lib/dnsxl_check.rb b/lib/dnsxl_check.rb
deleted file mode 100644
index 1e506b2d9cb..00000000000
--- a/lib/dnsxl_check.rb
+++ /dev/null
@@ -1,105 +0,0 @@
-require 'resolv'
-
-class DNSXLCheck
-
- class Resolver
- def self.search(query)
- begin
- Resolv.getaddress(query)
- true
- rescue Resolv::ResolvError
- false
- end
- end
- end
-
- IP_REGEXP = /\A(?:(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\.){3}(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\z/
- DEFAULT_THRESHOLD = 0.33
-
- def self.create_from_list(list)
- dnsxl_check = DNSXLCheck.new
-
- list.each do |entry|
- dnsxl_check.add_list(entry.domain, entry.weight)
- end
-
- dnsxl_check
- end
-
- def test(ip)
- if use_threshold?
- test_with_threshold(ip)
- else
- test_strict(ip)
- end
- end
-
- def test_with_threshold(ip)
- return false if lists.empty?
-
- search(ip)
- final_score >= threshold
- end
-
- def test_strict(ip)
- return false if lists.empty?
-
- search(ip)
- @score > 0
- end
-
- def use_threshold=(value)
- @use_threshold = value == true
- end
-
- def use_threshold?
- @use_threshold &&= true
- end
-
- def threshold=(threshold)
- raise ArgumentError, "'threshold' value must be grather than 0 and less than or equal to 1" unless threshold > 0 && threshold <= 1
- @threshold = threshold
- end
-
- def threshold
- @threshold ||= DEFAULT_THRESHOLD
- end
-
- def add_list(domain, weight)
- @lists ||= []
- @lists << { domain: domain, weight: weight }
- end
-
- def lists
- @lists ||= []
- end
-
- private
-
- def search(ip)
- raise ArgumentError, "'ip' value must be in #{IP_REGEXP} format" unless ip.match(IP_REGEXP)
-
- @score = 0
-
- reversed = reverse_ip(ip)
- search_in_rbls(reversed)
- end
-
- def reverse_ip(ip)
- ip.split('.').reverse.join('.')
- end
-
- def search_in_rbls(reversed_ip)
- lists.each do |rbl|
- query = "#{reversed_ip}.#{rbl[:domain]}"
- @score += rbl[:weight] if Resolver.search(query)
- end
- end
-
- def final_score
- weights = lists.map{ |rbl| rbl[:weight] }.reduce(:+).to_i
- return 0 if weights == 0
-
- (@score.to_f / weights.to_f).round(2)
- end
-end
diff --git a/lib/gitlab/ip_check.rb b/lib/gitlab/ip_check.rb
deleted file mode 100644
index f2e9b50d225..00000000000
--- a/lib/gitlab/ip_check.rb
+++ /dev/null
@@ -1,34 +0,0 @@
-module Gitlab
- class IpCheck
-
- def initialize(ip)
- @ip = ip
-
- application_settings = ApplicationSetting.current
- @ip_blocking_enabled = application_settings.ip_blocking_enabled
- @dnsbl_servers_list = application_settings.dnsbl_servers_list
- end
-
- def spam?
- @ip_blocking_enabled && blacklisted?
- end
-
- private
-
- def blacklisted?
- on_dns_blacklist?
- end
-
- def on_dns_blacklist?
- dnsbl_check = DNSXLCheck.new
- prepare_dnsbl_list(dnsbl_check)
- dnsbl_check.test(@ip)
- end
-
- def prepare_dnsbl_list(dnsbl_check)
- @dnsbl_servers_list.split(',').map(&:strip).reject(&:empty?).each do |domain|
- dnsbl_check.add_list(domain, 1)
- end
- end
- end
-end
diff --git a/spec/lib/dnsxl_check_spec.rb b/spec/lib/dnsxl_check_spec.rb
deleted file mode 100644
index a35a1be0c90..00000000000
--- a/spec/lib/dnsxl_check_spec.rb
+++ /dev/null
@@ -1,68 +0,0 @@
-require 'spec_helper'
-require 'ostruct'
-
-describe 'DNSXLCheck', lib: true, no_db: true do
- let(:spam_ip) { '127.0.0.2' }
- let(:no_spam_ip) { '127.0.0.3' }
- let(:invalid_ip) { 'a.b.c.d' }
- let!(:dnsxl_check) { DNSXLCheck.create_from_list([OpenStruct.new({ domain: 'test', weight: 1 })]) }
-
- before(:context) do
- class DNSXLCheck::Resolver
- class << self
- alias_method :old_search, :search
- def search(query)
- return false if query.match(/always\.failing\.domain\z/)
- return true if query.match(/\A2\.0\.0\.127\./)
- return false if query.match(/\A3\.0\.0\.127\./)
- end
- end
- end
- end
-
- describe '#test' do
- before do
- dnsxl_check.threshold = 0.75
- dnsxl_check.add_list('always.failing.domain', 1)
- end
-
- context 'when threshold is used' do
- before { dnsxl_check.use_threshold= true }
-
- it { expect(dnsxl_check.test(spam_ip)).to be_falsey }
- end
-
- context 'when threshold is not used' do
- before { dnsxl_check.use_threshold= false }
-
- it { expect(dnsxl_check.test(spam_ip)).to be_truthy }
- end
- end
-
- describe '#test_with_threshold' do
- it { expect{ dnsxl_check.test_with_threshold(invalid_ip) }.to raise_error(ArgumentError) }
-
- it { expect(dnsxl_check.test_with_threshold(spam_ip)).to be_truthy }
- it { expect(dnsxl_check.test_with_threshold(no_spam_ip)).to be_falsey }
- end
-
- describe '#test_strict' do
- before do
- dnsxl_check.threshold = 1
- dnsxl_check.add_list('always.failing.domain', 1)
- end
-
- it { expect{ dnsxl_check.test_strict(invalid_ip) }.to raise_error(ArgumentError) }
-
- it { expect(dnsxl_check.test_with_threshold(spam_ip)).to be_falsey }
- it { expect(dnsxl_check.test_with_threshold(no_spam_ip)).to be_falsey }
- it { expect(dnsxl_check.test_strict(spam_ip)).to be_truthy }
- it { expect(dnsxl_check.test_strict(no_spam_ip)).to be_falsey }
- end
-
- describe '#threshold=' do
- it { expect{ dnsxl_check.threshold = 0 }.to raise_error(ArgumentError) }
- it { expect{ dnsxl_check.threshold = 1.1 }.to raise_error(ArgumentError) }
- it { expect{ dnsxl_check.threshold = 0.5 }.not_to raise_error }
- end
-end
diff --git a/spec/services/notes/create_service_spec.rb b/spec/services/notes/create_service_spec.rb
index a797a2fe4aa..ff23f13e1cb 100644
--- a/spec/services/notes/create_service_spec.rb
+++ b/spec/services/notes/create_service_spec.rb
@@ -14,9 +14,7 @@ describe Notes::CreateService, services: true do
noteable_type: 'Issue',
noteable_id: issue.id
}
-
- expect(project).to receive(:execute_hooks)
- expect(project).to receive(:execute_services)
+
@note = Notes::CreateService.new(project, user, opts).execute
end
diff --git a/spec/services/notes/post_process_service_spec.rb b/spec/services/notes/post_process_service_spec.rb
new file mode 100644
index 00000000000..1a3f339bd64
--- /dev/null
+++ b/spec/services/notes/post_process_service_spec.rb
@@ -0,0 +1,26 @@
+require 'spec_helper'
+
+describe Notes::PostProcessService, services: true do
+ let(:project) { create(:empty_project) }
+ let(:issue) { create(:issue, project: project) }
+ let(:user) { create(:user) }
+
+ describe :execute do
+ before do
+ project.team << [user, :master]
+ note_opts = {
+ note: 'Awesome comment',
+ noteable_type: 'Issue',
+ noteable_id: issue.id
+ }
+
+ @note = Notes::CreateService.new(project, user, note_opts).execute
+ end
+
+ it do
+ expect(project).to receive(:execute_hooks)
+ expect(project).to receive(:execute_services)
+ Notes::PostProcessService.new(@note).execute
+ end
+ end
+end