From 0fb907441c827cadc767641b29d5d2c0e554f7a4 Mon Sep 17 00:00:00 2001 From: Claire Date: Tue, 25 Jan 2022 22:37:12 +0100 Subject: [PATCH] Add ability to set hCaptcha either on registration form or on e-mail validation Upshot of CAPTCHA on e-mail validation is it does not need to break the in-band registration API. --- .../auth/confirmations_controller.rb | 50 +++++++++++++++++++ app/controllers/concerns/captcha_concern.rb | 12 ++++- app/models/form/admin_settings.rb | 4 +- app/serializers/rest/instance_serializer.rb | 2 +- app/views/admin/settings/edit.html.haml | 2 +- .../auth/confirmations/captcha.html.haml | 11 ++++ config/locales-glitch/en.yml | 17 +++++-- config/routes.rb | 1 + config/settings.yml | 2 +- 9 files changed, 91 insertions(+), 10 deletions(-) create mode 100644 app/views/auth/confirmations/captcha.html.haml diff --git a/app/controllers/auth/confirmations_controller.rb b/app/controllers/auth/confirmations_controller.rb index 0b5a2f3c9db..e9a646f910c 100644 --- a/app/controllers/auth/confirmations_controller.rb +++ b/app/controllers/auth/confirmations_controller.rb @@ -1,12 +1,18 @@ # frozen_string_literal: true class Auth::ConfirmationsController < Devise::ConfirmationsController + include CaptchaConcern + layout 'auth' before_action :set_body_classes before_action :set_pack + before_action :set_confirmation_user!, only: [:show, :confirm_captcha] before_action :require_unconfirmed! + before_action :extend_csp_for_captcha!, only: [:show, :confirm_captcha] + before_action :require_captcha_if_needed!, only: [:show] + skip_before_action :require_functional! def new @@ -15,8 +21,52 @@ class Auth::ConfirmationsController < Devise::ConfirmationsController resource.email = current_user.unconfirmed_email || current_user.email if user_signed_in? end + def show + clear_captcha! + + old_session_values = session.to_hash + reset_session + session.update old_session_values.except('session_id') + + super + end + + def confirm_captcha + check_captcha! do |message| + flash.now[:alert] = message + render :captcha + return + end + + show + end + private + def require_captcha_if_needed! + render :captcha if captcha_required? + end + + def set_confirmation_user! + # We need to reimplement looking up the user because + # Devise::ConfirmationsController#show looks up and confirms in one + # step. + confirmation_token = params[:confirmation_token] + return if confirmation_token.nil? + @confirmation_user = User.find_first_by_auth_conditions(confirmation_token: confirmation_token) + end + + def captcha_user_bypass? + return true if @confirmation_user.nil? || @confirmation_user.confirmed? + + invite = Invite.find(@confirmation_user.invite_id) if @confirmation_user.invite_id.present? + 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/concerns/captcha_concern.rb b/app/controllers/concerns/captcha_concern.rb index 4a942c98881..02069d205cf 100644 --- a/app/controllers/concerns/captcha_concern.rb +++ b/app/controllers/concerns/captcha_concern.rb @@ -15,17 +15,21 @@ module CaptchaConcern end def captcha_enabled? - captcha_available? && Setting.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 end + def captcha_user_bypass? + current_user.present? || (@invite.present? && @invite.valid_for_use? && !@invite.max_uses.nil?) + end + def captcha_required? return false if ENV['OMNIAUTH_ONLY'] == 'true' return false unless Setting.registrations_mode != 'none' || @invite&.valid_for_use? - captcha_enabled? && !current_user && !(@invite.present? && @invite.valid_for_use? && !@invite.max_uses.nil?) && !captcha_recently_passed? + captcha_enabled? && !captcha_user_bypass? && !captcha_recently_passed? end def clear_captcha! @@ -65,4 +69,8 @@ module CaptchaConcern 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 34f14e3124c..7abb0d6c617 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_enabled + captcha_mode ).freeze BOOLEAN_KEYS = %i( @@ -59,7 +59,6 @@ class Form::AdminSettings trendable_by_default noindex require_invite_text - captcha_enabled ).freeze UPLOAD_KEYS = %i( @@ -83,6 +82,7 @@ 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 94cc3ffe34e..d343cca2006 100644 --- a/app/serializers/rest/instance_serializer.rb +++ b/app/serializers/rest/instance_serializer.rb @@ -116,6 +116,6 @@ class REST::InstanceSerializer < ActiveModel::Serializer end def captcha_enabled? - ENV['HCAPTCHA_SECRET_KEY'].present? && ENV['HCAPTCHA_SITE_KEY'].present? && Setting.captcha_enabled + ENV['HCAPTCHA_SECRET_KEY'].present? && ENV['HCAPTCHA_SITE_KEY'].present? && Setting.captcha_mode == 'registration-form' end end diff --git a/app/views/admin/settings/edit.html.haml b/app/views/admin/settings/edit.html.haml index 49b03a9e358..fc042f845d7 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_enabled, as: :boolean, wrapper: :with_label, label: t('admin.settings.captcha_enabled.title'), hint: t('admin.settings.captcha_enabled.desc_html') + = 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') %hr.spacer/ diff --git a/app/views/auth/confirmations/captcha.html.haml b/app/views/auth/confirmations/captcha.html.haml new file mode 100644 index 00000000000..850bc1479ca --- /dev/null +++ b/app/views/auth/confirmations/captcha.html.haml @@ -0,0 +1,11 @@ +- content_for :page_title do + = t('auth.confirm_captcha') + += form_tag auth_captcha_confirmation_url, method: 'POST', class: 'simple_form' do + = hidden_field_tag :confirmation_token, params[:confirmation_token] + + .field-group + = render_captcha_if_needed + + .actions + %button.button= t('challenge.continue') diff --git a/config/locales-glitch/en.yml b/config/locales-glitch/en.yml index 9bd66c969fc..6ad5a53650e 100644 --- a/config/locales-glitch/en.yml +++ b/config/locales-glitch/en.yml @@ -2,9 +2,18 @@ en: admin: settings: - captcha_enabled: - desc_html: Enable hCaptcha integration, requiring new users to solve a challenge when signing up. Note that this disables app-based registration, may prevent your instance from being listed as having open registrations, and requires third-party scripts from hCaptcha to be embedded in the registration pages. This may have security and privacy concerns. - title: Require new users to go through a CAPTCHA to sign up + 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 enable_keybase: desc_html: Allow your users to prove their identity via keybase title: Enable keybase integration @@ -20,6 +29,8 @@ en: show_replies_in_public_timelines: desc_html: In addition to public self-replies (threads), show public replies in local and public timelines. title: Show replies in public timelines + auth: + confirm_captcha: User verification generic: use_this: Use this settings: diff --git a/config/routes.rb b/config/routes.rb index 65dd7ad638a..d0eeda1e864 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -44,6 +44,7 @@ Rails.application.routes.draw do resource :setup, only: [:show, :update], controller: :setup resource :challenge, only: [:create], controller: :challenges get 'sessions/security_key_options', to: 'sessions#webauthn_options' + post 'captcha_confirmation', to: 'confirmations#confirm_captcha', as: :captcha_confirmation end end diff --git a/config/settings.yml b/config/settings.yml index 7d192f3691c..b5437caeea6 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_enabled: false + captcha_mode: 'disabled' development: <<: *defaults