From b7cf3941b3783220e6b3bc9a6d3975ceecdc64cb Mon Sep 17 00:00:00 2001 From: Claire Date: Tue, 25 Jan 2022 23:56:57 +0100 Subject: 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. --- app/controllers/about_controller.rb | 2 -- app/controllers/api/v1/accounts_controller.rb | 4 +--- app/controllers/auth/confirmations_controller.rb | 6 ------ app/controllers/auth/registrations_controller.rb | 22 ------------------- app/controllers/concerns/captcha_concern.rb | 27 +++++------------------- app/models/form/admin_settings.rb | 4 ++-- app/serializers/rest/instance_serializer.rb | 6 +----- app/views/about/_registration.html.haml | 3 --- app/views/admin/settings/edit.html.haml | 2 +- app/views/auth/confirmations/captcha.html.haml | 2 +- app/views/auth/registrations/new.html.haml | 3 --- 11 files changed, 11 insertions(+), 70 deletions(-) (limited to 'app') 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 -- cgit