Merge branch 'rs-remove-ip-blocking' into 'master'

Partially revert "Add IP check against DNSBLs at account sign-up"

This partially reverts 6a5cd3ca - we keep the migration and add a new
migration that reverts it in order to keep migration history intact.

See merge request !2643

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

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,

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."

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

app/views/admin/application_settings/_form.html.haml

@@ -212,22 +212,6 @@
   %fieldset
     %legend Spam and Anti-bot Protection
-    .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

db/migrate/20160128212447_remove_ip_blocking_settings_from_application_settings.rb

+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

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|

lib/dnsxl_check.rb

-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

lib/gitlab/ip_check.rb

-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

spec/lib/dnsxl_check_spec.rb

-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