From c68b03c0ddb0bc881132521ed0a1c6682a3af735 Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Tue, 23 Jan 2024 13:26:18 +1100 Subject: [PATCH 01/10] Add invisble_captcha gem --- Gemfile | 1 + Gemfile.lock | 3 +++ 2 files changed, 4 insertions(+) diff --git a/Gemfile b/Gemfile index d9e5029d2e..46a3c3bd29 100644 --- a/Gemfile +++ b/Gemfile @@ -142,6 +142,7 @@ gem "faraday" gem "private_address_check" gem 'newrelic_rpm' +gem 'invisible_captcha' group :production, :staging do gem 'sd_notify' # For better Systemd process management. Used by Puma. diff --git a/Gemfile.lock b/Gemfile.lock index 8974a3d8ad..fa548b1e02 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -343,6 +343,8 @@ GEM ruby-vips (>= 2.0.17, < 3) immigrant (0.3.6) activerecord (>= 3.0) + invisible_captcha (2.1.0) + rails (>= 5.2) io-console (0.7.1) ipaddress (0.8.3) irb (1.11.0) @@ -857,6 +859,7 @@ DEPENDENCIES i18n-js (~> 3.9.0) image_processing immigrant + invisible_captcha jquery-rails (= 4.4.0) jquery-ui-rails (~> 4.2) json From d8876c40b899f2cfe259ac3d02146234c3038b31 Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Tue, 23 Jan 2024 16:30:12 +1100 Subject: [PATCH 02/10] Add invisible_captcha on the user registration page The default action when a user submit the form too quickly is to redirect to :back with flash error message. As we are using CableReady it's not working for us, so I render_alert_timestamp_error_message to show the error message to the user. --- app/controllers/spree/users_controller.rb | 12 ++++++++++++ app/views/layouts/_signup_tab.html.haml | 1 + config/initializers/invisible_captcha.rb | 6 ++++++ spec/system/consumer/authentication_spec.rb | 15 +++++++++++++++ 4 files changed, 34 insertions(+) create mode 100644 config/initializers/invisible_captcha.rb diff --git a/app/controllers/spree/users_controller.rb b/app/controllers/spree/users_controller.rb index 6fdcebe89b..60b7b33e26 100644 --- a/app/controllers/spree/users_controller.rb +++ b/app/controllers/spree/users_controller.rb @@ -8,6 +8,7 @@ module Spree layout 'darkswarm' + invisible_captcha only: [:create], on_timestamp_spam: :render_alert_timestamp_error_message skip_before_action :set_current_order, only: :show prepend_before_action :load_object, only: [:show, :edit, :update] prepend_before_action :authorize_actions, only: :new @@ -101,5 +102,16 @@ module Spree def user_params ::PermittedAttributes::User.new(params).call end + + def render_alert_timestamp_error_message + render cable_ready: cable_car.inner_html( + "#signup-feedback", + partial("layouts/alert", + locals: { + type: "alert", + message: InvisibleCaptcha.timestamp_error_message + }) + ) + end end end diff --git a/app/views/layouts/_signup_tab.html.haml b/app/views/layouts/_signup_tab.html.haml index 5d2af646e5..f25bf2024d 100644 --- a/app/views/layouts/_signup_tab.html.haml +++ b/app/views/layouts/_signup_tab.html.haml @@ -23,3 +23,4 @@ .row .large-12.columns = form.submit t(:action_signup), { class: "button primary", tabindex: 4 } + = form.invisible_captcha diff --git a/config/initializers/invisible_captcha.rb b/config/initializers/invisible_captcha.rb new file mode 100644 index 0000000000..bf00691a0c --- /dev/null +++ b/config/initializers/invisible_captcha.rb @@ -0,0 +1,6 @@ +# frozen_string_literal: true + +InvisibleCaptcha.setup do |config| + # Disable timestamp check for test environment + config.timestamp_enabled = !Rails.env.test? +end diff --git a/spec/system/consumer/authentication_spec.rb b/spec/system/consumer/authentication_spec.rb index 7951704440..9581b4c467 100644 --- a/spec/system/consumer/authentication_spec.rb +++ b/spec/system/consumer/authentication_spec.rb @@ -108,6 +108,21 @@ describe "Authentication" do expect(page).to have_content "doesn't match" end + it "Failing to sign up because the user is too quick" do + InvisibleCaptcha.timestamp_enabled = true + InvisibleCaptcha.timestamp_threshold = 30 + + fill_in "Your email", with: user.email + fill_in "Choose a password", with: "test12345" + fill_in "Confirm password", with: "test12345" + click_signup_button + + expect(page).to have_content "Sorry, that was too quick! Please resubmit." + + InvisibleCaptcha.timestamp_enabled = false + InvisibleCaptcha.timestamp_threshold = 30 + end + it "Signing up successfully" do fill_in "Your email", with: "test@foo.com" fill_in "Choose a password", with: "test12345" From 910d0a91f7da07c648c92887c44e5fe5356c3b54 Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Tue, 23 Jan 2024 16:35:13 +1100 Subject: [PATCH 03/10] Small style refactor. - Replace `context` by `describe`, although they are equivalent, they don't convey the same intent. - Remove "should" in `it` descrption --- spec/controllers/spree/users_controller_spec.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/spec/controllers/spree/users_controller_spec.rb b/spec/controllers/spree/users_controller_spec.rb index 3837cf8b71..8f6455055c 100644 --- a/spec/controllers/spree/users_controller_spec.rb +++ b/spec/controllers/spree/users_controller_spec.rb @@ -7,7 +7,7 @@ describe Spree::UsersController, type: :controller do include AuthenticationHelper - describe "show" do + describe "#show" do let!(:u1) { create(:user) } let!(:u2) { create(:user) } let!(:distributor1) { create(:distributor_enterprise) } @@ -55,7 +55,7 @@ describe Spree::UsersController, type: :controller do end end - describe "registered_email" do + describe "#registered_email" do routes { Openfoodnetwork::Application.routes } let!(:user) { create(:user) } @@ -71,16 +71,16 @@ describe Spree::UsersController, type: :controller do end end - context '#load_object' do - it 'should redirect to signup path if user is not found' do + describe '#load_object' do + it 'redirects to signup path if user is not found' do allow(controller).to receive_messages(spree_current_user: nil) put :update, params: { user: { email: 'foobar@example.com' } } expect(response).to redirect_to('/login') end end - context '#create' do - it 'should create a new user' do + describe '#create' do + it 'creates a new user' do post :create, params: { user: { email: 'foobar@example.com', password: 'foobar123', password_confirmation: 'foobar123', locale: 'es' } } From 0d474f6e291a087b49fe8bae05a19959f53bfec0 Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Tue, 23 Jan 2024 17:00:19 +1100 Subject: [PATCH 04/10] Fix Rubocop warning --- Gemfile | 1 + 1 file changed, 1 insertion(+) diff --git a/Gemfile b/Gemfile index 46a3c3bd29..804caea69a 100644 --- a/Gemfile +++ b/Gemfile @@ -142,6 +142,7 @@ gem "faraday" gem "private_address_check" gem 'newrelic_rpm' + gem 'invisible_captcha' group :production, :staging do From f4ea71eb3c233fc9d674821f7b95d1096ef5b350 Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Wed, 24 Jan 2024 10:04:49 +1100 Subject: [PATCH 05/10] Fix check for locale cookie For some reason the app set an `_ofn_session_id cookie`, which broke the assertion expecting no cookie. --- spec/system/consumer/multilingual_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/system/consumer/multilingual_spec.rb b/spec/system/consumer/multilingual_spec.rb index 87886017ca..1020d81601 100644 --- a/spec/system/consumer/multilingual_spec.rb +++ b/spec/system/consumer/multilingual_spec.rb @@ -27,7 +27,7 @@ describe 'Multilingual' do visit root_path expect(get_i18n_locale).to eq 'en' expect(get_i18n_translation('label_shops')).to eq 'Shops' - expect(cookies).to be_empty + expect(cookies_name).not_to include('locale') expect(page).to have_content 'SHOPS' visit root_path(locale: 'es') From 048619d660cc30112449d0a353acd61597c49657 Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Wed, 24 Jan 2024 15:56:25 +1100 Subject: [PATCH 06/10] Move disabling timestamp check to the spec helper --- config/initializers/invisible_captcha.rb | 6 ------ spec/base_spec_helper.rb | 3 +++ 2 files changed, 3 insertions(+), 6 deletions(-) delete mode 100644 config/initializers/invisible_captcha.rb diff --git a/config/initializers/invisible_captcha.rb b/config/initializers/invisible_captcha.rb deleted file mode 100644 index bf00691a0c..0000000000 --- a/config/initializers/invisible_captcha.rb +++ /dev/null @@ -1,6 +0,0 @@ -# frozen_string_literal: true - -InvisibleCaptcha.setup do |config| - # Disable timestamp check for test environment - config.timestamp_enabled = !Rails.env.test? -end diff --git a/spec/base_spec_helper.rb b/spec/base_spec_helper.rb index 6e36c6ed46..ff344f3564 100644 --- a/spec/base_spec_helper.rb +++ b/spec/base_spec_helper.rb @@ -55,6 +55,9 @@ I18n.exception_handler = proc do |exception, *_| raise exception.to_exception end +# Disable timestamp check for test environment +InvisibleCaptcha.timestamp_enabled = false + RSpec.configure do |config| # Remove this line if you're not using ActiveRecord or ActiveRecord fixtures config.fixture_path = "#{Rails.root.join('spec/fixtures')}" From 0638b9eea4aaa1f1823444fb7972de27bb1580ca Mon Sep 17 00:00:00 2001 From: David Cook Date: Thu, 25 Jan 2024 17:06:25 +1100 Subject: [PATCH 07/10] Use unique email address for test Using the existing user email is only needed for the 'already registered' test. : --- spec/system/consumer/authentication_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/system/consumer/authentication_spec.rb b/spec/system/consumer/authentication_spec.rb index 9581b4c467..f4f2344590 100644 --- a/spec/system/consumer/authentication_spec.rb +++ b/spec/system/consumer/authentication_spec.rb @@ -102,7 +102,7 @@ describe "Authentication" do end it "Failing to sign up because password confirmation doesn't match or is blank" do - fill_in "Your email", with: user.email + fill_in "Your email", with: "test@foo.com" fill_in "Choose a password", with: "ForgotToRetype" click_signup_button expect(page).to have_content "doesn't match" @@ -112,7 +112,7 @@ describe "Authentication" do InvisibleCaptcha.timestamp_enabled = true InvisibleCaptcha.timestamp_threshold = 30 - fill_in "Your email", with: user.email + fill_in "Your email", with: "test@foo.com" fill_in "Choose a password", with: "test12345" fill_in "Confirm password", with: "test12345" click_signup_button From 2535435b558c45c766530cc87aca46d42f1823c4 Mon Sep 17 00:00:00 2001 From: David Cook Date: Thu, 25 Jan 2024 17:09:18 +1100 Subject: [PATCH 08/10] Test success with invisible_captcha --- spec/system/consumer/authentication_spec.rb | 44 ++++++++++++++------- 1 file changed, 29 insertions(+), 15 deletions(-) diff --git a/spec/system/consumer/authentication_spec.rb b/spec/system/consumer/authentication_spec.rb index f4f2344590..c12cb4fb0c 100644 --- a/spec/system/consumer/authentication_spec.rb +++ b/spec/system/consumer/authentication_spec.rb @@ -108,21 +108,6 @@ describe "Authentication" do expect(page).to have_content "doesn't match" end - it "Failing to sign up because the user is too quick" do - InvisibleCaptcha.timestamp_enabled = true - InvisibleCaptcha.timestamp_threshold = 30 - - fill_in "Your email", with: "test@foo.com" - fill_in "Choose a password", with: "test12345" - fill_in "Confirm password", with: "test12345" - click_signup_button - - expect(page).to have_content "Sorry, that was too quick! Please resubmit." - - InvisibleCaptcha.timestamp_enabled = false - InvisibleCaptcha.timestamp_threshold = 30 - end - it "Signing up successfully" do fill_in "Your email", with: "test@foo.com" fill_in "Choose a password", with: "test12345" @@ -135,6 +120,35 @@ describe "Authentication" do 'your account.' end.to enqueue_job ActionMailer::MailDeliveryJob end + + describe "invisible_captcha gem" do + around do |example| + InvisibleCaptcha.timestamp_enabled = true + InvisibleCaptcha.timestamp_threshold = 30 + example.run + InvisibleCaptcha.timestamp_enabled = false + end + + it "Failing to sign up because the user is too quick" do + fill_in "Your email", with: "test@foo.com" + fill_in "Choose a password", with: "test12345" + fill_in "Confirm password", with: "test12345" + click_signup_button + + expect(page).to have_content "Sorry, that was too quick! Please resubmit." + end + + it "succeeding after time threshold" do + Timecop.travel(30.seconds.from_now) do + fill_in "Your email", with: "test@foo.com" + fill_in "Choose a password", with: "test12345" + fill_in "Confirm password", with: "test12345" + click_signup_button + + expect(page).to have_content 'A message with a confirmation link has been sent' + end + end + end end describe "forgetting passwords" do From 55ad1ede4eb5365f159af8c3063ed030fdf121c2 Mon Sep 17 00:00:00 2001 From: David Cook Date: Thu, 25 Jan 2024 17:22:57 +1100 Subject: [PATCH 09/10] Add messages to ensure they get translated --- config/locales/en.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/config/locales/en.yml b/config/locales/en.yml index 4289cc584c..c68fcb9326 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -4774,3 +4774,9 @@ See the %{link} to find out more about %{sitename}'s features and to start using pagination: next: Next previous: Previous + + + # Gem to prevent bot form submissions + invisible_captcha: + sentence_for_humans: "If you are human, ignore this field" + timestamp_error_message: "Sorry, that was too quick! Please resubmit." From 6ffe12582029ad2a66fc2b0c393895fd7c8a281c Mon Sep 17 00:00:00 2001 From: David Cook Date: Thu, 25 Jan 2024 17:24:27 +1100 Subject: [PATCH 10/10] Override default messages Just in case any bots have been trained to handle the gem's default messages. --- config/locales/en.yml | 4 ++-- spec/system/consumer/authentication_spec.rb | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/config/locales/en.yml b/config/locales/en.yml index c68fcb9326..fac2ffa211 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -4778,5 +4778,5 @@ See the %{link} to find out more about %{sitename}'s features and to start using # Gem to prevent bot form submissions invisible_captcha: - sentence_for_humans: "If you are human, ignore this field" - timestamp_error_message: "Sorry, that was too quick! Please resubmit." + sentence_for_humans: "Please leave empty" + timestamp_error_message: "Please try again after 5 seconds." diff --git a/spec/system/consumer/authentication_spec.rb b/spec/system/consumer/authentication_spec.rb index c12cb4fb0c..7062d99a2e 100644 --- a/spec/system/consumer/authentication_spec.rb +++ b/spec/system/consumer/authentication_spec.rb @@ -135,7 +135,7 @@ describe "Authentication" do fill_in "Confirm password", with: "test12345" click_signup_button - expect(page).to have_content "Sorry, that was too quick! Please resubmit." + expect(page).to have_content "Please try again after 5 seconds." end it "succeeding after time threshold" do