Change CAPTCHA handling to be only on email verification

This simplifies the implementation considerably, and while not providing
ideal UX, it's the most flexible approach.

14 files changed, 15 insertions, 84 deletions

diff --git a/app/controllers/about_controller.rb b/app/controllers/about_controller.rb
index 5a35dbbcb..620c0ff78 100644
--- a/app/controllers/about_controller.rb
+++ b/app/controllers/about_controller.rb
@@ -2,7 +2,6 @@
 
 class AboutController < ApplicationController
   include RegistrationSpamConcern
-  include CaptchaConcern
 
   before_action :set_pack
 
@@ -13,7 +12,6 @@ class AboutController < ApplicationController
   before_action :set_instance_presenter
   before_action :set_expires_in, only: [:more, :terms]
   before_action :set_registration_form_time, only: :show
-  before_action :extend_csp_for_captcha!, only: :show
 
   skip_before_action :require_functional!, only: [:more, :terms]
 
diff --git a/app/controllers/api/v1/accounts_controller.rb b/app/controllers/api/v1/accounts_controller.rb
index 8916c3f96..5c47158e0 100644
--- a/app/controllers/api/v1/accounts_controller.rb
+++ b/app/controllers/api/v1/accounts_controller.rb
@@ -1,8 +1,6 @@
 # frozen_string_literal: true
 
 class Api::V1::AccountsController < Api::BaseController
-  include CaptchaConcern
-
   before_action -> { authorize_if_got_token! :read, :'read:accounts' }, except: [:create, :follow, :unfollow, :remove_from_followers, :block, :unblock, :mute, :unmute]
   before_action -> { doorkeeper_authorize! :follow, :'write:follows' }, only: [:follow, :unfollow, :remove_from_followers]
   before_action -> { doorkeeper_authorize! :follow, :'write:mutes' }, only: [:mute, :unmute]
@@ -85,7 +83,7 @@ class Api::V1::AccountsController < Api::BaseController
   end
 
   def check_enabled_registrations
-    forbidden if single_user_mode? || omniauth_only? || !allowed_registrations? || captcha_enabled?
+    forbidden if single_user_mode? || omniauth_only? || !allowed_registrations?
   end
 
   def allowed_registrations?
diff --git a/app/controllers/auth/confirmations_controller.rb b/app/controllers/auth/confirmations_controller.rb
index e9a646f91..17ad56fa8 100644
--- a/app/controllers/auth/confirmations_controller.rb
+++ b/app/controllers/auth/confirmations_controller.rb
@@ -22,8 +22,6 @@ class Auth::ConfirmationsController < Devise::ConfirmationsController
   end
 
   def show
-    clear_captcha!
-
     old_session_values = session.to_hash
     reset_session
     session.update old_session_values.except('session_id')
@@ -63,10 +61,6 @@ class Auth::ConfirmationsController < Devise::ConfirmationsController
     invite.present? && !invite.max_uses.nil?
   end
 
-  def captcha_context
-    'email-confirmation'
-  end
-
   def set_pack
     use_pack 'auth'
   end
diff --git a/app/controllers/auth/registrations_controller.rb b/app/controllers/auth/registrations_controller.rb
index 0db9cb84d..6b1f3fa82 100644
--- a/app/controllers/auth/registrations_controller.rb
+++ b/app/controllers/auth/registrations_controller.rb
@@ -2,7 +2,6 @@
 
 class Auth::RegistrationsController < Devise::RegistrationsController
   include RegistrationSpamConcern
-  include CaptchaConcern
 
   layout :determine_layout
 
@@ -16,8 +15,6 @@ class Auth::RegistrationsController < Devise::RegistrationsController
   before_action :require_not_suspended!, only: [:update]
   before_action :set_cache_headers, only: [:edit, :update]
   before_action :set_registration_form_time, only: :new
-  before_action :extend_csp_for_captcha!, only: [:new, :create]
-  before_action :check_captcha!, only: :create
 
   skip_before_action :require_functional!, only: [:edit, :update]
 
@@ -138,23 +135,4 @@ class Auth::RegistrationsController < Devise::RegistrationsController
   def set_cache_headers
     response.headers['Cache-Control'] = 'no-cache, no-store, max-age=0, must-revalidate'
   end
-
-  def sign_up(resource_name, resource)
-    clear_captcha!
-
-    old_session_values = session.to_hash
-    reset_session
-    session.update old_session_values.except('session_id')
-
-    super
-  end
-
-  def check_captcha!
-    super do |error|
-      build_resource(sign_up_params)
-      resource.validate
-      resource.errors.add(:base, error)
-      respond_with resource
-    end
-  end
 end
diff --git a/app/controllers/concerns/captcha_concern.rb b/app/controllers/concerns/captcha_concern.rb
index 02069d205..538c1ffb1 100644
--- a/app/controllers/concerns/captcha_concern.rb
+++ b/app/controllers/concerns/captcha_concern.rb
@@ -4,10 +4,8 @@ module CaptchaConcern
   extend ActiveSupport::Concern
   include Hcaptcha::Adapters::ViewMethods
 
-  CAPTCHA_TIMEOUT = 2.hours.freeze
-
   included do
-    helper_method :render_captcha_if_needed
+    helper_method :render_captcha
   end
 
   def captcha_available?
@@ -15,32 +13,21 @@ module CaptchaConcern
   end
 
   def captcha_enabled?
-    captcha_available? && Setting.captcha_mode == captcha_context
-  end
-
-  def captcha_recently_passed?
-    session[:captcha_passed_at].present? && session[:captcha_passed_at] >= CAPTCHA_TIMEOUT.ago
+    captcha_available? && Setting.captcha_enabled
   end
 
   def captcha_user_bypass?
-    current_user.present? || (@invite.present? && @invite.valid_for_use? && !@invite.max_uses.nil?)
+    false
   end
 
   def captcha_required?
-    return false if ENV['OMNIAUTH_ONLY'] == 'true'
-    return false unless Setting.registrations_mode != 'none' || @invite&.valid_for_use?
-    captcha_enabled? && !captcha_user_bypass? && !captcha_recently_passed?
-  end
-
-  def clear_captcha!
-    session.delete(:captcha_passed_at)
+    captcha_enabled? && !captcha_user_bypass?
   end
 
   def check_captcha!
     return true unless captcha_required?
 
     if verify_hcaptcha
-      session[:captcha_passed_at] = Time.now.utc
       true
     else
       if block_given?
@@ -64,13 +51,9 @@ module CaptchaConcern
     end
   end
 
-  def render_captcha_if_needed
+  def render_captcha
     return unless captcha_required?
 
     hcaptcha_tags
   end
-
-  def captcha_context
-    'registration-form'
-  end
 end
diff --git a/app/models/form/admin_settings.rb b/app/models/form/admin_settings.rb
index 7abb0d6c6..34f14e312 100644
--- a/app/models/form/admin_settings.rb
+++ b/app/models/form/admin_settings.rb
@@ -40,7 +40,7 @@ class Form::AdminSettings
     noindex
     outgoing_spoilers
     require_invite_text
-    captcha_mode
+    captcha_enabled
   ).freeze
 
   BOOLEAN_KEYS = %i(
@@ -59,6 +59,7 @@ class Form::AdminSettings
     trendable_by_default
     noindex
     require_invite_text
+    captcha_enabled
   ).freeze
 
   UPLOAD_KEYS = %i(
@@ -82,7 +83,6 @@ class Form::AdminSettings
   validates :bootstrap_timeline_accounts, existing_username: { multiple: true }
   validates :show_domain_blocks, inclusion: { in: %w(disabled users all) }
   validates :show_domain_blocks_rationale, inclusion: { in: %w(disabled users all) }
-  validates :captcha_mode, inclusion: { in: %w(disabled registration-form email-confirmation) }
 
   def initialize(_attributes = {})
     super
diff --git a/app/serializers/rest/instance_serializer.rb b/app/serializers/rest/instance_serializer.rb
index d343cca20..48bbb55c8 100644
--- a/app/serializers/rest/instance_serializer.rb
+++ b/app/serializers/rest/instance_serializer.rb
@@ -98,7 +98,7 @@ class REST::InstanceSerializer < ActiveModel::Serializer
   end
 
   def registrations
-    Setting.registrations_mode != 'none' && !Rails.configuration.x.single_user_mode && !captcha_enabled?
+    Setting.registrations_mode != 'none' && !Rails.configuration.x.single_user_mode
   end
 
   def approval_required
@@ -114,8 +114,4 @@ class REST::InstanceSerializer < ActiveModel::Serializer
   def instance_presenter
     @instance_presenter ||= InstancePresenter.new
   end
-
-  def captcha_enabled?
-    ENV['HCAPTCHA_SECRET_KEY'].present? && ENV['HCAPTCHA_SITE_KEY'].present? && Setting.captcha_mode == 'registration-form'
-  end
 end
diff --git a/app/views/about/_registration.html.haml b/app/views/about/_registration.html.haml
index 5bb5d08a2..e4d614d71 100644
--- a/app/views/about/_registration.html.haml
+++ b/app/views/about/_registration.html.haml
@@ -21,9 +21,6 @@
     .fields-group
       = f.input :agreement, as: :boolean, wrapper: :with_label, label: t('auth.checkbox_agreement_html', rules_path: about_more_path, terms_path: terms_path), required: true, disabled: closed_registrations?
 
-    .fields-group
-      = render_captcha_if_needed
-
     .actions
       = f.button :button, sign_up_message, type: :submit, class: 'button button-primary', disabled: closed_registrations?
 
diff --git a/app/views/admin/settings/edit.html.haml b/app/views/admin/settings/edit.html.haml
index fc042f845..49b03a9e3 100644
--- a/app/views/admin/settings/edit.html.haml
+++ b/app/views/admin/settings/edit.html.haml
@@ -45,7 +45,7 @@
 
   - if captcha_available?
     .fields-group
-      = f.input :captcha_mode, as: :radio_buttons, collection: %w(disabled registration-form email-confirmation), include_blank: false, wrapper: :with_block_label, label_method: ->(type) { safe_join([t("admin.settings.captcha.#{type}.title"), content_tag(:span, t("admin.settings.captcha.#{type}.desc_html"), class: 'hint')])}, label: t('admin.settings.captcha.title'), hint: t('admin.settings.captcha.desc_html')
+      = f.input :captcha_enabled, as: :boolean, wrapper: :with_label, label: t('admin.settings.captcha_enabled.title'), hint: t('admin.settings.captcha_enabled.desc_html')
 
   %hr.spacer/
 
diff --git a/app/views/auth/confirmations/captcha.html.haml b/app/views/auth/confirmations/captcha.html.haml
index 850bc1479..0f7cf9c59 100644
--- a/app/views/auth/confirmations/captcha.html.haml
+++ b/app/views/auth/confirmations/captcha.html.haml
@@ -5,7 +5,7 @@
   = hidden_field_tag :confirmation_token, params[:confirmation_token]
 
   .field-group
-    = render_captcha_if_needed
+    = render_captcha
 
   .actions
     %button.button= t('challenge.continue')
diff --git a/app/views/auth/registrations/new.html.haml b/app/views/auth/registrations/new.html.haml
index 5cb558297..6981195ed 100644
--- a/app/views/auth/registrations/new.html.haml
+++ b/app/views/auth/registrations/new.html.haml
@@ -38,9 +38,6 @@
   .fields-group
     = f.input :agreement, as: :boolean, wrapper: :with_label, label: whitelist_mode? ? t('auth.checkbox_agreement_without_rules_html', terms_path: terms_path) : t('auth.checkbox_agreement_html', rules_path: about_more_path, terms_path: terms_path), required: true
 
-  .field-group
-    = render_captcha_if_needed
-
   .actions
     = f.button :button, @invite.present? ? t('auth.register') : sign_up_message, type: :submit
 
diff --git a/config/locales-glitch/en.yml b/config/locales-glitch/en.yml
index 6ad5a5365..ab7f1b976 100644
--- a/config/locales-glitch/en.yml
+++ b/config/locales-glitch/en.yml
@@ -2,18 +2,9 @@
 en:
   admin:
     settings:
-      captcha:
-        desc_html: Configure hCaptcha integration, relying on third-party scripts. This may have security and privacy implications.
-        email-confirmation:
-          desc_html: Require new users to go through hCaptcha at the e-mail validation step. Bots will not be deterred from creating accounts, but they may be prevented from confirming them, leaving them to be automatically cleaned up after a couple days. This does not interfere with app-based registrations.
-          title: CAPTCHA on email validation
-        disabled:
-          desc_html: Do not require a CAPTCHA
-          title: Disabled
-        registration-form:
-          desc_html: Require new users to go through hCaptcha on the registration form, so that CAPTCHA requirement is immediately apparent to them. This disables app-based registrations and may prevent your instance from being listed as having open registrations.
-          title: CAPTCHA on registration forms
-        title: CAPTCHA configuration
+      captcha_enabled:
+        desc_html: Enable hCaptcha integration, requiring new users to solve a challenge when confirming their email address. This requires third-party scripts from hCaptcha to be embedded in the email verification page, which may have security and privacy concerns. Users that have been invited through a limited-use invite will not need to solve a CAPTCHA challenge.
+        title: Require new users to go through a CAPTCHA to confirm their account
       enable_keybase:
         desc_html: Allow your users to prove their identity via keybase
         title: Enable keybase integration
diff --git a/config/settings.yml b/config/settings.yml
index b5437caee..7d192f369 100644
--- a/config/settings.yml
+++ b/config/settings.yml
@@ -77,7 +77,7 @@ defaults: &defaults
   show_domain_blocks_rationale: 'disabled'
   outgoing_spoilers: ''
   require_invite_text: false
-  captcha_mode: 'disabled'
+  captcha_enabled: false
 
 development:
   <<: *defaults
diff --git a/spec/views/about/show.html.haml_spec.rb b/spec/views/about/show.html.haml_spec.rb
index 12c96ea49..d608bbf5d 100644
--- a/spec/views/about/show.html.haml_spec.rb
+++ b/spec/views/about/show.html.haml_spec.rb
@@ -10,7 +10,6 @@ describe 'about/show.html.haml', without_verify_partial_doubles: true do
     allow(view).to receive(:site_title).and_return('example site')
     allow(view).to receive(:new_user).and_return(User.new)
     allow(view).to receive(:use_seamless_external_login?).and_return(false)
-    allow(view).to receive(:render_captcha_if_needed).and_return(nil)
   end
 
   it 'has valid open graph tags' do