Merge branch 'security-59549-add-capcha-for-failed-logins' into 'master'

Require a captcha after unique failed logins from the same IP

See merge request gitlab/gitlabhq!3270

7 files changed, 62 insertions, 11 deletions

diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb
index 1880bead3ee..7b682cc0cc5 100644
--- a/app/controllers/sessions_controller.rb
+++ b/app/controllers/sessions_controller.rb
@@ -21,10 +21,13 @@ class SessionsController < Devise::SessionsController
   prepend_before_action :ensure_password_authentication_enabled!, if: -> { action_name == 'create' && password_based_login? }
 
   before_action :auto_sign_in_with_provider, only: [:new]
+  before_action :store_unauthenticated_sessions, only: [:new]
+  before_action :save_failed_login, if: :action_new_and_failed_login?
   before_action :load_recaptcha
 
-  after_action :log_failed_login, if: -> { action_name == 'new' && failed_login? }
-  helper_method :captcha_enabled?
+  after_action :log_failed_login, if: :action_new_and_failed_login?
+
+  helper_method :captcha_enabled?, :captcha_on_login_required?
 
   # protect_from_forgery is already prepended in ApplicationController but
   # authenticate_with_two_factor which signs in the user is prepended before
@@ -38,6 +41,7 @@ class SessionsController < Devise::SessionsController
   protect_from_forgery with: :exception, prepend: true
 
   CAPTCHA_HEADER = 'X-GitLab-Show-Login-Captcha'.freeze
+  MAX_FAILED_LOGIN_ATTEMPTS = 5
 
   def new
     set_minimum_password_length
@@ -81,10 +85,14 @@ class SessionsController < Devise::SessionsController
     request.headers[CAPTCHA_HEADER] && Gitlab::Recaptcha.enabled?
   end
 
+  def captcha_on_login_required?
+    Gitlab::Recaptcha.enabled_on_login? && unverified_anonymous_user?
+  end
+
   # From https://github.com/plataformatec/devise/wiki/How-To:-Use-Recaptcha-with-Devise#devisepasswordscontroller
   def check_captcha
     return unless user_params[:password].present?
-    return unless captcha_enabled?
+    return unless captcha_enabled? || captcha_on_login_required?
     return unless Gitlab::Recaptcha.load_configurations!
 
     if verify_recaptcha
@@ -126,10 +134,28 @@ class SessionsController < Devise::SessionsController
     Gitlab::AppLogger.info("Failed Login: username=#{user_params[:login]} ip=#{request.remote_ip}")
   end
 
+  def action_new_and_failed_login?
+    action_name == 'new' && failed_login?
+  end
+
+  def save_failed_login
+    session[:failed_login_attempts] ||= 0
+    session[:failed_login_attempts] += 1
+  end
+
   def failed_login?
     (options = request.env["warden.options"]) && options[:action] == "unauthenticated"
   end
 
+  # storing sessions per IP lets us check if there are associated multiple
+  # anonymous sessions with one IP and prevent situations when there are
+  # multiple attempts of logging in
+  def store_unauthenticated_sessions
+    return if current_user
+
+    Gitlab::AnonymousSession.new(request.remote_ip, session_id: request.session.id).store_session_id_per_ip
+  end
+
   # Handle an "initial setup" state, where there's only one user, it's an admin,
   # and they require a password change.
   # rubocop: disable CodeReuse/ActiveRecord
@@ -240,6 +266,18 @@ class SessionsController < Devise::SessionsController
     @ldap_servers ||= Gitlab::Auth::LDAP::Config.available_servers
   end
 
+  def unverified_anonymous_user?
+    exceeded_failed_login_attempts? || exceeded_anonymous_sessions?
+  end
+
+  def exceeded_failed_login_attempts?
+    session.fetch(:failed_login_attempts, 0) > MAX_FAILED_LOGIN_ATTEMPTS
+  end
+
+  def exceeded_anonymous_sessions?
+    Gitlab::AnonymousSession.new(request.remote_ip).stored_sessions >= MAX_FAILED_LOGIN_ATTEMPTS
+  end
+
   def authentication_method
     if user_params[:otp_attempt]
       "two-factor"
diff --git a/app/helpers/application_settings_helper.rb b/app/helpers/application_settings_helper.rb
index 0ab19f1d2d2..8d1f2b59da9 100644
--- a/app/helpers/application_settings_helper.rb
+++ b/app/helpers/application_settings_helper.rb
@@ -231,6 +231,7 @@ module ApplicationSettingsHelper
       :recaptcha_enabled,
       :recaptcha_private_key,
       :recaptcha_site_key,
+      :login_recaptcha_protection_enabled,
       :receive_max_input_size,
       :repository_checks_enabled,
       :repository_storages,
diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb
index 2a99c6e5c59..855b371368d 100644
--- a/app/models/application_setting.rb
+++ b/app/models/application_setting.rb
@@ -75,11 +75,11 @@ class ApplicationSetting < ApplicationRecord
 
   validates :recaptcha_site_key,
             presence: true,
-            if: :recaptcha_enabled
+            if: :recaptcha_or_login_protection_enabled
 
   validates :recaptcha_private_key,
             presence: true,
-            if: :recaptcha_enabled
+            if: :recaptcha_or_login_protection_enabled
 
   validates :akismet_api_key,
             presence: true,
@@ -292,4 +292,8 @@ class ApplicationSetting < ApplicationRecord
   def self.cache_backend
     Gitlab::ThreadMemoryCache.cache_backend
   end
+
+  def recaptcha_or_login_protection_enabled
+    recaptcha_enabled || login_recaptcha_protection_enabled
+  end
 end
diff --git a/app/models/application_setting_implementation.rb b/app/models/application_setting_implementation.rb
index 55ac1e129cf..9b7e4c0fbbe 100644
--- a/app/models/application_setting_implementation.rb
+++ b/app/models/application_setting_implementation.rb
@@ -64,6 +64,7 @@ module ApplicationSettingImplementation
         polling_interval_multiplier: 1,
         project_export_enabled: true,
         recaptcha_enabled: false,
+        login_recaptcha_protection_enabled: false,
         repository_checks_enabled: true,
         repository_storages: ['default'],
         require_two_factor_authentication: false,
diff --git a/app/views/admin/application_settings/_spam.html.haml b/app/views/admin/application_settings/_spam.html.haml
index d24e46b2815..f0a19075115 100644
--- a/app/views/admin/application_settings/_spam.html.haml
+++ b/app/views/admin/application_settings/_spam.html.haml
@@ -7,11 +7,15 @@
         = f.check_box :recaptcha_enabled, class: 'form-check-input'
         = f.label :recaptcha_enabled, class: 'form-check-label' do
           Enable reCAPTCHA
-        - recaptcha_v2_link_url = 'https://developers.google.com/recaptcha/docs/versions'
-        - recaptcha_v2_link_start = '<a href="%{url}" target="_blank" rel="noopener noreferrer">'.html_safe % { url: recaptcha_v2_link_url }
         %span.form-text.text-muted#recaptcha_help_block
-          = _('Helps prevent bots from creating accounts. We currently only support %{recaptcha_v2_link_start}reCAPTCHA v2%{recaptcha_v2_link_end}').html_safe % { recaptcha_v2_link_start: recaptcha_v2_link_start, recaptcha_v2_link_end: '</a>'.html_safe }
-
+          = _('Helps prevent bots from creating accounts.')
+    .form-group
+      .form-check
+        = f.check_box :login_recaptcha_protection_enabled, class: 'form-check-input'
+        = f.label :login_recaptcha_protection_enabled, class: 'form-check-label' do
+          Enable reCAPTCHA for login
+        %span.form-text.text-muted#recaptcha_help_block
+          = _('Helps prevent bots from brute-force attacks.')
     .form-group
       = f.label :recaptcha_site_key, 'reCAPTCHA Site Key', class: 'label-bold'
       = f.text_field :recaptcha_site_key, class: 'form-control'
@@ -21,6 +25,7 @@
 
     .form-group
       = f.label :recaptcha_private_key, 'reCAPTCHA Private Key', class: 'label-bold'
+    .form-group
       = f.text_field :recaptcha_private_key, class: 'form-control'
 
     .form-group
diff --git a/app/views/admin/application_settings/reporting.html.haml b/app/views/admin/application_settings/reporting.html.haml
index 46e3d1c4570..c60e44b3864 100644
--- a/app/views/admin/application_settings/reporting.html.haml
+++ b/app/views/admin/application_settings/reporting.html.haml
@@ -9,7 +9,9 @@
     %button.btn.btn-default.js-settings-toggle{ type: 'button' }
       = expanded_by_default? ? _('Collapse') : _('Expand')
     %p
-      = _('Enable reCAPTCHA or Akismet and set IP limits.')
+      - recaptcha_v2_link_url = 'https://developers.google.com/recaptcha/docs/versions'
+      - recaptcha_v2_link_start = '<a href="%{url}" target="_blank" rel="noopener noreferrer">'.html_safe % { url: recaptcha_v2_link_url }
+      = _('Enable reCAPTCHA or Akismet and set IP limits. For reCAPTCHA, we currently only support %{recaptcha_v2_link_start}v2%{recaptcha_v2_link_end}').html_safe % { recaptcha_v2_link_start: recaptcha_v2_link_start, recaptcha_v2_link_end: '</a>'.html_safe }
   .settings-content
     = render 'spam'
 
diff --git a/app/views/devise/sessions/_new_base.html.haml b/app/views/devise/sessions/_new_base.html.haml
index 0b1d3d1ddb3..6e9efcb0597 100644
--- a/app/views/devise/sessions/_new_base.html.haml
+++ b/app/views/devise/sessions/_new_base.html.haml
@@ -16,7 +16,7 @@
         - else
           = link_to _('Forgot your password?'), new_password_path(:user)
     %div
-    - if captcha_enabled?
+    - if captcha_enabled? || captcha_on_login_required?
       = recaptcha_tags
 
   .submit-container.move-submit-down