Merge branch 'sh-add-captcha-prometheus-counters' into 'master'

Add Prometheus metrics to track reCAPTCHA success/failures

See merge request gitlab-org/gitlab-ce!20086

3 files changed, 34 insertions, 1 deletions

diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb
index 7aa277b3614..1de6ae24622 100644
--- a/app/controllers/sessions_controller.rb
+++ b/app/controllers/sessions_controller.rb
@@ -62,7 +62,11 @@ class SessionsController < Devise::SessionsController
     return unless captcha_enabled?
     return unless Gitlab::Recaptcha.load_configurations!
 
-    unless verify_recaptcha
+    if verify_recaptcha
+      increment_successful_login_captcha_counter
+    else
+      increment_failed_login_captcha_counter
+
       self.resource = resource_class.new
       flash[:alert] = 'There was an error with the reCAPTCHA. Please solve the reCAPTCHA again.'
       flash.delete :recaptcha_error
@@ -71,6 +75,20 @@ class SessionsController < Devise::SessionsController
     end
   end
 
+  def increment_failed_login_captcha_counter
+    Gitlab::Metrics.counter(
+      :failed_login_captcha_total,
+      'Number of failed CAPTCHA attempts for logins'.freeze
+    ).increment
+  end
+
+  def increment_successful_login_captcha_counter
+    Gitlab::Metrics.counter(
+      :successful_login_captcha_total,
+      'Number of successful CAPTCHA attempts for logins'.freeze
+    ).increment
+  end
+
   def log_failed_login
     Gitlab::AppLogger.info("Failed Login: username=#{user_params[:login]} ip=#{request.remote_ip}")
   end
diff --git a/doc/administration/monitoring/prometheus/gitlab_metrics.md b/doc/administration/monitoring/prometheus/gitlab_metrics.md
index cea6764df41..c0e98099e42 100644
--- a/doc/administration/monitoring/prometheus/gitlab_metrics.md
+++ b/doc/administration/monitoring/prometheus/gitlab_metrics.md
@@ -48,6 +48,8 @@ The following metrics are available:
 | filesystem_circuitbreaker_latency_seconds | Gauge | 9.5 | Time spent validating if a storage is accessible |
 | filesystem_circuitbreaker         | Gauge     | 9.5   | Whether or not the circuit for a certain shard is broken or not |
 | circuitbreaker_storage_check_duration_seconds | Histogram | 10.3 | Time a single storage probe took |
+| failed_login_captcha_total        | Gauge | 11.0 | Counter of failed CAPTCHA attempts during login |
+| successful_login_captcha_total    | Gauge | 11.0 | Counter of successful CAPTCHA attempts during login |
 
 ### Ruby metrics
 
diff --git a/spec/controllers/sessions_controller_spec.rb b/spec/controllers/sessions_controller_spec.rb
index cdec26bd421..7c00652317b 100644
--- a/spec/controllers/sessions_controller_spec.rb
+++ b/spec/controllers/sessions_controller_spec.rb
@@ -93,6 +93,12 @@ describe SessionsController do
         it 'displays an error when the reCAPTCHA is not solved' do
           # Without this, `verify_recaptcha` arbitraily returns true in test env
           Recaptcha.configuration.skip_verify_env.delete('test')
+          counter = double(:counter)
+
+          expect(counter).to receive(:increment)
+          expect(Gitlab::Metrics).to receive(:counter)
+                                      .with(:failed_login_captcha_total, anything)
+                                      .and_return(counter)
 
           post(:create, user: user_params)
 
@@ -104,6 +110,13 @@ describe SessionsController do
         it 'successfully logs in a user when reCAPTCHA is solved' do
           # Avoid test ordering issue and ensure `verify_recaptcha` returns true
           Recaptcha.configuration.skip_verify_env << 'test'
+          counter = double(:counter)
+
+          expect(counter).to receive(:increment)
+          expect(Gitlab::Metrics).to receive(:counter)
+                                      .with(:successful_login_captcha_total, anything)
+                                      .and_return(counter)
+          expect(Gitlab::Metrics).to receive(:counter).and_call_original
 
           post(:create, user: user_params)