From 456c0691cd1a7f73d8e2e5bcf3d47372c8db27be Mon Sep 17 00:00:00 2001 From: Alex Buijs Date: Fri, 9 Aug 2019 13:20:09 +0200 Subject: [PATCH 1/3] Add invisible_captcha gem --- Gemfile | 1 + Gemfile.lock | 3 +++ 2 files changed, 4 insertions(+) diff --git a/Gemfile b/Gemfile index 22746f9c5ae..487f3c5476a 100644 --- a/Gemfile +++ b/Gemfile @@ -51,6 +51,7 @@ gem 'jwt', '~> 2.1.0' # Spam and anti-bot protection gem 'recaptcha', '~> 4.11', require: 'recaptcha/rails' gem 'akismet', '~> 2.0' +gem 'invisible_captcha', '~> 0.12.1' # Two-factor authentication gem 'devise-two-factor', '~> 3.0.0' diff --git a/Gemfile.lock b/Gemfile.lock index 80f36a9457c..8fe54e7d557 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -437,6 +437,8 @@ GEM influxdb (0.2.3) cause json + invisible_captcha (0.12.1) + rails (>= 3.2.0) ipaddress (0.8.3) jaeger-client (0.10.0) opentracing (~> 0.3) @@ -1126,6 +1128,7 @@ DEPENDENCIES httparty (~> 0.16.4) icalendar influxdb (~> 0.2) + invisible_captcha (~> 0.12.1) jira-ruby (~> 1.4) js_regex (~> 3.1) json-schema (~> 2.8.0) From a8da0de528f3a522c6d77b92ca5621c63ae9a69a Mon Sep 17 00:00:00 2001 From: Alex Buijs Date: Fri, 9 Aug 2019 17:40:54 +0200 Subject: [PATCH 2/3] Add invisible captcha With a time treshold of 4 seconds and a firstname and lastname honeypot input fields when signing up --- app/controllers/registrations_controller.rb | 7 +++ app/views/devise/shared/_signup_box.html.haml | 2 + config/initializers/invisible_captcha.rb | 7 +++ config/locales/invisible_captcha.en.yml | 4 ++ .../registrations_controller_spec.rb | 60 +++++++++++++++++++ spec/features/invites_spec.rb | 1 + spec/features/users/signup_spec.rb | 4 ++ 7 files changed, 85 insertions(+) create mode 100644 config/initializers/invisible_captcha.rb create mode 100644 config/locales/invisible_captcha.en.yml diff --git a/app/controllers/registrations_controller.rb b/app/controllers/registrations_controller.rb index 638934694e0..33e7ca061d3 100644 --- a/app/controllers/registrations_controller.rb +++ b/app/controllers/registrations_controller.rb @@ -6,6 +6,7 @@ class RegistrationsController < Devise::RegistrationsController include RecaptchaExperimentHelper prepend_before_action :check_captcha, only: :create + invisible_captcha only: :create, on_timestamp_spam: :on_timestamp_spam_callback before_action :whitelist_query_limiting, only: [:destroy] before_action :ensure_terms_accepted, if: -> { action_name == 'create' && Gitlab::CurrentSettings.current_application_settings.enforce_terms? } @@ -134,4 +135,10 @@ class RegistrationsController < Devise::RegistrationsController def terms_accepted? Gitlab::Utils.to_boolean(params[:terms_opt_in]) end + + def on_timestamp_spam_callback + return unless Feature.enabled?(:invisible_captcha) + + redirect_to new_user_session_path, alert: InvisibleCaptcha.timestamp_error_message + end end diff --git a/app/views/devise/shared/_signup_box.html.haml b/app/views/devise/shared/_signup_box.html.haml index 074edf645ba..2cd77af6877 100644 --- a/app/views/devise/shared/_signup_box.html.haml +++ b/app/views/devise/shared/_signup_box.html.haml @@ -5,6 +5,8 @@ = form_for(resource, as: "new_#{resource_name}", url: registration_path(resource_name), html: { class: "new_new_user gl-show-field-errors", "aria-live" => "assertive" }) do |f| .devise-errors = render "devise/shared/error_messages", resource: resource + - if Feature.enabled?(:invisible_captcha) + = invisible_captcha .name.form-group = f.label :name, _('Full name'), class: 'label-bold' = f.text_field :name, class: "form-control top js-block-emoji js-validate-length", :data => { :max_length => max_name_length, :max_length_message => s_("SignUp|Name is too long (maximum is %{max_length} characters).") % { max_length: max_name_length }, :qa_selector => 'new_user_name_field' }, required: true, title: _("This field is required.") diff --git a/config/initializers/invisible_captcha.rb b/config/initializers/invisible_captcha.rb new file mode 100644 index 00000000000..5177c730596 --- /dev/null +++ b/config/initializers/invisible_captcha.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +InvisibleCaptcha.setup do |config| + config.honeypots = %w(firstname lastname) + config.timestamp_enabled = true + config.timestamp_threshold = 4 +end diff --git a/config/locales/invisible_captcha.en.yml b/config/locales/invisible_captcha.en.yml new file mode 100644 index 00000000000..5978549c0c3 --- /dev/null +++ b/config/locales/invisible_captcha.en.yml @@ -0,0 +1,4 @@ +en: + invisible_captcha: + sentence_for_humans: If you are human, please ignore this field. + timestamp_error_message: That was a bit too quick! Please resubmit. diff --git a/spec/controllers/registrations_controller_spec.rb b/spec/controllers/registrations_controller_spec.rb index faf3c990cb2..796089d126b 100644 --- a/spec/controllers/registrations_controller_spec.rb +++ b/spec/controllers/registrations_controller_spec.rb @@ -5,6 +5,10 @@ require 'spec_helper' describe RegistrationsController do include TermsHelper + before do + stub_feature_flags(invisible_captcha: false) + end + describe '#create' do let(:base_user_params) { { name: 'new_user', username: 'new_username', email: 'new@user.com', password: 'Any_password' } } let(:user_params) { { user: base_user_params } } @@ -88,6 +92,62 @@ describe RegistrationsController do end end + context 'when invisible captcha is enabled' do + before do + stub_feature_flags(invisible_captcha: true) + InvisibleCaptcha.timestamp_threshold = treshold + end + + let(:treshold) { 4 } + let(:session_params) { { invisible_captcha_timestamp: form_rendered_time.iso8601 } } + let(:form_rendered_time) { Time.current } + let(:submit_time) { form_rendered_time + treshold } + + describe 'the honeypot has not been filled and the signup form has not been submitted too quickly' do + it 'creates an account' do + travel_to(submit_time) do + expect { post(:create, params: user_params, session: session_params) }.to change(User, :count).by(1) + end + end + end + + describe 'the honeypot has been filled' do + let(:user_params) { super().merge(firstname: 'Roy', lastname: 'Batty') } + + it 'refuses to create an account and renders an empty body' do + travel_to(submit_time) do + expect { post(:create, params: user_params, session: session_params) }.not_to change(User, :count) + expect(response).to have_gitlab_http_status(200) + expect(response.body).to be_empty + end + end + end + + context 'the sign up form has been submitted without the invisible_captcha_timestamp parameter' do + let(:session_params) { nil } + + it 'refuses to create an account and displays a flash alert' do + travel_to(submit_time) do + expect { post(:create, params: user_params, session: session_params) }.not_to change(User, :count) + expect(response).to redirect_to(new_user_session_path) + expect(flash[:alert]).to include 'That was a bit too quick! Please resubmit.' + end + end + end + + context 'the sign up form has been submitted too quickly' do + let(:submit_time) { form_rendered_time } + + it 'refuses to create an account and displays a flash alert' do + travel_to(submit_time) do + expect { post(:create, params: user_params, session: session_params) }.not_to change(User, :count) + expect(response).to redirect_to(new_user_session_path) + expect(flash[:alert]).to include 'That was a bit too quick! Please resubmit.' + end + end + end + end + context 'when terms are enforced' do before do enforce_terms diff --git a/spec/features/invites_spec.rb b/spec/features/invites_spec.rb index 855cf22642e..832c4a57aa3 100644 --- a/spec/features/invites_spec.rb +++ b/spec/features/invites_spec.rb @@ -10,6 +10,7 @@ describe 'Invites' do let(:group_invite) { group.group_members.invite.last } before do + stub_feature_flags(invisible_captcha: false) project.add_maintainer(owner) group.add_user(owner, Gitlab::Access::OWNER) group.add_developer('user@example.com', owner) diff --git a/spec/features/users/signup_spec.rb b/spec/features/users/signup_spec.rb index f5897bffaf0..cf57fafc4f5 100644 --- a/spec/features/users/signup_spec.rb +++ b/spec/features/users/signup_spec.rb @@ -5,6 +5,10 @@ require 'spec_helper' describe 'Signup' do include TermsHelper + before do + stub_feature_flags(invisible_captcha: false) + end + let(:new_user) { build_stubbed(:user) } describe 'username validation', :js do From cdbe66490fe9d4d664562ee21e4b1be28298b411 Mon Sep 17 00:00:00 2001 From: Alex Buijs Date: Wed, 14 Aug 2019 14:05:24 +0200 Subject: [PATCH 3/3] Add logging and counter for invisible captcha --- app/controllers/concerns/invisible_captcha.rb | 51 ++++++++++++++++ app/controllers/registrations_controller.rb | 8 +-- .../registrations_controller_spec.rb | 60 +++++++++++++------ 3 files changed, 95 insertions(+), 24 deletions(-) create mode 100644 app/controllers/concerns/invisible_captcha.rb diff --git a/app/controllers/concerns/invisible_captcha.rb b/app/controllers/concerns/invisible_captcha.rb new file mode 100644 index 00000000000..c9f66e5c194 --- /dev/null +++ b/app/controllers/concerns/invisible_captcha.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +module InvisibleCaptcha + extend ActiveSupport::Concern + + included do + invisible_captcha only: :create, on_spam: :on_honeypot_spam_callback, on_timestamp_spam: :on_timestamp_spam_callback + end + + def on_honeypot_spam_callback + return unless Feature.enabled?(:invisible_captcha) + + invisible_captcha_honeypot_counter.increment + log_request('Invisible_Captcha_Honeypot_Request') + + head(200) + end + + def on_timestamp_spam_callback + return unless Feature.enabled?(:invisible_captcha) + + invisible_captcha_timestamp_counter.increment + log_request('Invisible_Captcha_Timestamp_Request') + + redirect_to new_user_session_path, alert: InvisibleCaptcha.timestamp_error_message + end + + def invisible_captcha_honeypot_counter + @invisible_captcha_honeypot_counter ||= + Gitlab::Metrics.counter(:bot_blocked_by_invisible_captcha_honeypot, + 'Counter of blocked sign up attempts with filled honeypot') + end + + def invisible_captcha_timestamp_counter + @invisible_captcha_timestamp_counter ||= + Gitlab::Metrics.counter(:bot_blocked_by_invisible_captcha_timestamp, + 'Counter of blocked sign up attempts with invalid timestamp') + end + + def log_request(message) + request_information = { + message: message, + env: :invisible_captcha_signup_bot_detected, + ip: request.ip, + request_method: request.request_method, + fullpath: request.fullpath + } + + Gitlab::AuthLogger.error(request_information) + end +end diff --git a/app/controllers/registrations_controller.rb b/app/controllers/registrations_controller.rb index 33e7ca061d3..db10515c0b4 100644 --- a/app/controllers/registrations_controller.rb +++ b/app/controllers/registrations_controller.rb @@ -4,9 +4,9 @@ class RegistrationsController < Devise::RegistrationsController include Recaptcha::Verify include AcceptsPendingInvitations include RecaptchaExperimentHelper + include InvisibleCaptcha prepend_before_action :check_captcha, only: :create - invisible_captcha only: :create, on_timestamp_spam: :on_timestamp_spam_callback before_action :whitelist_query_limiting, only: [:destroy] before_action :ensure_terms_accepted, if: -> { action_name == 'create' && Gitlab::CurrentSettings.current_application_settings.enforce_terms? } @@ -135,10 +135,4 @@ class RegistrationsController < Devise::RegistrationsController def terms_accepted? Gitlab::Utils.to_boolean(params[:terms_opt_in]) end - - def on_timestamp_spam_callback - return unless Feature.enabled?(:invisible_captcha) - - redirect_to new_user_session_path, alert: InvisibleCaptcha.timestamp_error_message - end end diff --git a/spec/controllers/registrations_controller_spec.rb b/spec/controllers/registrations_controller_spec.rb index 796089d126b..d05482f095e 100644 --- a/spec/controllers/registrations_controller_spec.rb +++ b/spec/controllers/registrations_controller_spec.rb @@ -102,6 +102,15 @@ describe RegistrationsController do let(:session_params) { { invisible_captcha_timestamp: form_rendered_time.iso8601 } } let(:form_rendered_time) { Time.current } let(:submit_time) { form_rendered_time + treshold } + let(:auth_log_attributes) do + { + message: auth_log_message, + env: :invisible_captcha_signup_bot_detected, + ip: '0.0.0.0', + request_method: 'POST', + fullpath: '/users' + } + end describe 'the honeypot has not been filled and the signup form has not been submitted too quickly' do it 'creates an account' do @@ -111,11 +120,16 @@ describe RegistrationsController do end end - describe 'the honeypot has been filled' do + describe 'honeypot spam detection' do let(:user_params) { super().merge(firstname: 'Roy', lastname: 'Batty') } + let(:auth_log_message) { 'Invisible_Captcha_Honeypot_Request' } - it 'refuses to create an account and renders an empty body' do + it 'logs the request, refuses to create an account and renders an empty body' do travel_to(submit_time) do + expect(Gitlab::Metrics).to receive(:counter) + .with(:bot_blocked_by_invisible_captcha_honeypot, 'Counter of blocked sign up attempts with filled honeypot') + .and_call_original + expect(Gitlab::AuthLogger).to receive(:error).with(auth_log_attributes).once expect { post(:create, params: user_params, session: session_params) }.not_to change(User, :count) expect(response).to have_gitlab_http_status(200) expect(response.body).to be_empty @@ -123,26 +137,38 @@ describe RegistrationsController do end end - context 'the sign up form has been submitted without the invisible_captcha_timestamp parameter' do - let(:session_params) { nil } + describe 'timestamp spam detection' do + let(:auth_log_message) { 'Invisible_Captcha_Timestamp_Request' } - it 'refuses to create an account and displays a flash alert' do - travel_to(submit_time) do - expect { post(:create, params: user_params, session: session_params) }.not_to change(User, :count) - expect(response).to redirect_to(new_user_session_path) - expect(flash[:alert]).to include 'That was a bit too quick! Please resubmit.' + context 'the sign up form has been submitted without the invisible_captcha_timestamp parameter' do + let(:session_params) { nil } + + it 'logs the request, refuses to create an account and displays a flash alert' do + travel_to(submit_time) do + expect(Gitlab::Metrics).to receive(:counter) + .with(:bot_blocked_by_invisible_captcha_timestamp, 'Counter of blocked sign up attempts with invalid timestamp') + .and_call_original + expect(Gitlab::AuthLogger).to receive(:error).with(auth_log_attributes).once + expect { post(:create, params: user_params, session: session_params) }.not_to change(User, :count) + expect(response).to redirect_to(new_user_session_path) + expect(flash[:alert]).to include 'That was a bit too quick! Please resubmit.' + end end end - end - context 'the sign up form has been submitted too quickly' do - let(:submit_time) { form_rendered_time } + context 'the sign up form has been submitted too quickly' do + let(:submit_time) { form_rendered_time } - it 'refuses to create an account and displays a flash alert' do - travel_to(submit_time) do - expect { post(:create, params: user_params, session: session_params) }.not_to change(User, :count) - expect(response).to redirect_to(new_user_session_path) - expect(flash[:alert]).to include 'That was a bit too quick! Please resubmit.' + it 'logs the request, refuses to create an account and displays a flash alert' do + travel_to(submit_time) do + expect(Gitlab::Metrics).to receive(:counter) + .with(:bot_blocked_by_invisible_captcha_timestamp, 'Counter of blocked sign up attempts with invalid timestamp') + .and_call_original + expect(Gitlab::AuthLogger).to receive(:error).with(auth_log_attributes).once + expect { post(:create, params: user_params, session: session_params) }.not_to change(User, :count) + expect(response).to redirect_to(new_user_session_path) + expect(flash[:alert]).to include 'That was a bit too quick! Please resubmit.' + end end end end