Commit 26998c68 authored by Robert Speicher's avatar Robert Speicher

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
parents 88b25329 7376ffc3
......@@ -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
......
......@@ -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
......
......@@ -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)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment