From 94bcf453219da73015cc977835717516b9dc0a67 Mon Sep 17 00:00:00 2001
From: Claire <claire.github-309c@sitedethib.com>
Date: Wed, 25 Aug 2021 22:52:41 +0200
Subject: Fix authentication failures after going halfway through a sign-in
 attempt (#16607)

* Add tests

* Add security-related tests

My first (unpublished) attempt at fixing the issues introduced (extremely
hard-to-exploit) security vulnerabilities, addressing them in a test.

* Fix authentication failures after going halfway through a sign-in attempt

* Refactor `authenticate_with_sign_in_token` and `authenticate_with_two_factor` to make the two authentication steps more obvious
---
 .../sign_in_token_authentication_concern.rb        | 20 ++++++++++++--------
 .../concerns/two_factor_authentication_concern.rb  | 22 +++++++++++++---------
 2 files changed, 25 insertions(+), 17 deletions(-)

(limited to 'app/controllers/concerns')

diff --git a/app/controllers/concerns/sign_in_token_authentication_concern.rb b/app/controllers/concerns/sign_in_token_authentication_concern.rb
index cbee84569..384c5c50c 100644
--- a/app/controllers/concerns/sign_in_token_authentication_concern.rb
+++ b/app/controllers/concerns/sign_in_token_authentication_concern.rb
@@ -16,14 +16,18 @@ module SignInTokenAuthenticationConcern
   end
 
   def authenticate_with_sign_in_token
-    user = self.resource = find_user
-
-    if user.present? && session[:attempt_user_id].present? && session[:attempt_user_updated_at] != user.updated_at.to_s
-      restart_session
-    elsif user_params.key?(:sign_in_token_attempt) && session[:attempt_user_id]
-      authenticate_with_sign_in_token_attempt(user)
-    elsif user.present? && user.external_or_valid_password?(user_params[:password])
-      prompt_for_sign_in_token(user)
+    if user_params[:email].present?
+      user = self.resource = find_user_from_params
+      prompt_for_sign_in_token(user) if user&.external_or_valid_password?(user_params[:password])
+    elsif session[:attempt_user_id]
+      user = self.resource = User.find_by(id: session[:attempt_user_id])
+      return if user.nil?
+
+      if session[:attempt_user_updated_at] != user.updated_at.to_s
+        restart_session
+      elsif user_params.key?(:sign_in_token_attempt)
+        authenticate_with_sign_in_token_attempt(user)
+      end
     end
   end
 
diff --git a/app/controllers/concerns/two_factor_authentication_concern.rb b/app/controllers/concerns/two_factor_authentication_concern.rb
index 909ab7717..2583d324b 100644
--- a/app/controllers/concerns/two_factor_authentication_concern.rb
+++ b/app/controllers/concerns/two_factor_authentication_concern.rb
@@ -35,16 +35,20 @@ module TwoFactorAuthenticationConcern
   end
 
   def authenticate_with_two_factor
-    user = self.resource = find_user
+    if user_params[:email].present?
+      user = self.resource = find_user_from_params
+      prompt_for_two_factor(user) if user&.external_or_valid_password?(user_params[:password])
+    elsif session[:attempt_user_id]
+      user = self.resource = User.find_by(id: session[:attempt_user_id])
+      return if user.nil?
 
-    if user.present? && session[:attempt_user_id].present? && session[:attempt_user_updated_at] != user.updated_at.to_s
-      restart_session
-    elsif user.webauthn_enabled? && user_params.key?(:credential) && session[:attempt_user_id]
-      authenticate_with_two_factor_via_webauthn(user)
-    elsif user_params.key?(:otp_attempt) && session[:attempt_user_id]
-      authenticate_with_two_factor_via_otp(user)
-    elsif user.present? && user.external_or_valid_password?(user_params[:password])
-      prompt_for_two_factor(user)
+      if session[:attempt_user_updated_at] != user.updated_at.to_s
+        restart_session
+      elsif user.webauthn_enabled? && user_params.key?(:credential)
+        authenticate_with_two_factor_via_webauthn(user)
+      elsif user_params.key?(:otp_attempt)
+        authenticate_with_two_factor_via_otp(user)
+      end
     end
   end
 
-- 
cgit 


From 0fb9536d3888cd7b6013c239d5be85f095a6e8ad Mon Sep 17 00:00:00 2001
From: Eugen Rochko <eugen@zeonfederated.com>
Date: Sun, 5 Dec 2021 21:48:39 +0100
Subject: Add batch suspend for accounts in admin UI (#17009)

---
 app/controllers/admin/accounts_controller.rb       | 33 +++++++-
 .../admin/pending_accounts_controller.rb           | 52 -------------
 app/controllers/concerns/accountable_concern.rb    |  4 +-
 app/helpers/admin/action_logs_helper.rb            |  2 +
 app/helpers/admin/dashboard_helper.rb              | 39 +++++++++-
 app/javascript/styles/mastodon/accounts.scss       | 30 ++++++-
 app/javascript/styles/mastodon/tables.scss         |  5 ++
 app/javascript/styles/mastodon/widgets.scss        | 18 +++++
 app/models/account.rb                              |  2 +
 app/models/account_filter.rb                       | 91 +++++++++++++---------
 app/models/admin/action_log.rb                     |  2 +-
 app/models/admin/action_log_filter.rb              |  2 +
 app/models/form/account_batch.rb                   | 51 +++++++++---
 app/views/admin/accounts/_account.html.haml        | 59 ++++++++------
 app/views/admin/accounts/index.html.haml           | 56 ++++++++-----
 app/views/admin/dashboard/index.html.haml          |  2 +-
 app/views/admin/instances/show.html.haml           |  2 +-
 app/views/admin/ip_blocks/_ip_block.html.haml      |  6 +-
 .../admin/pending_accounts/_account.html.haml      | 16 ----
 app/views/admin/pending_accounts/index.html.haml   | 33 --------
 .../admin_mailer/new_pending_account.text.erb      |  2 +-
 config/locales/en.yml                              | 11 ++-
 config/navigation.rb                               |  2 +-
 config/routes.rb                                   | 12 +--
 spec/controllers/admin/accounts_controller_spec.rb | 14 +---
 spec/models/account_filter_spec.rb                 | 42 +---------
 26 files changed, 311 insertions(+), 277 deletions(-)
 delete mode 100644 app/controllers/admin/pending_accounts_controller.rb
 delete mode 100644 app/views/admin/pending_accounts/_account.html.haml
 delete mode 100644 app/views/admin/pending_accounts/index.html.haml

(limited to 'app/controllers/concerns')

diff --git a/app/controllers/admin/accounts_controller.rb b/app/controllers/admin/accounts_controller.rb
index 1dd7430e0..948e70d5b 100644
--- a/app/controllers/admin/accounts_controller.rb
+++ b/app/controllers/admin/accounts_controller.rb
@@ -2,13 +2,24 @@
 
 module Admin
   class AccountsController < BaseController
-    before_action :set_account, except: [:index]
+    before_action :set_account, except: [:index, :batch]
     before_action :require_remote_account!, only: [:redownload]
     before_action :require_local_account!, only: [:enable, :memorialize, :approve, :reject]
 
     def index
       authorize :account, :index?
+
       @accounts = filtered_accounts.page(params[:page])
+      @form     = Form::AccountBatch.new
+    end
+
+    def batch
+      @form = Form::AccountBatch.new(form_account_batch_params.merge(current_account: current_account, action: action_from_button))
+      @form.save
+    rescue ActionController::ParameterMissing
+      flash[:alert] = I18n.t('admin.accounts.no_account_selected')
+    ensure
+      redirect_to admin_accounts_path(filter_params)
     end
 
     def show
@@ -38,13 +49,13 @@ module Admin
     def approve
       authorize @account.user, :approve?
       @account.user.approve!
-      redirect_to admin_pending_accounts_path, notice: I18n.t('admin.accounts.approved_msg', username: @account.acct)
+      redirect_to admin_accounts_path(status: 'pending'), notice: I18n.t('admin.accounts.approved_msg', username: @account.acct)
     end
 
     def reject
       authorize @account.user, :reject?
       DeleteAccountService.new.call(@account, reserve_email: false, reserve_username: false)
-      redirect_to admin_pending_accounts_path, notice: I18n.t('admin.accounts.rejected_msg', username: @account.acct)
+      redirect_to admin_accounts_path(status: 'pending'), notice: I18n.t('admin.accounts.rejected_msg', username: @account.acct)
     end
 
     def destroy
@@ -121,11 +132,25 @@ module Admin
     end
 
     def filtered_accounts
-      AccountFilter.new(filter_params).results
+      AccountFilter.new(filter_params.with_defaults(order: 'recent')).results
     end
 
     def filter_params
       params.slice(*AccountFilter::KEYS).permit(*AccountFilter::KEYS)
     end
+
+    def form_account_batch_params
+      params.require(:form_account_batch).permit(:action, account_ids: [])
+    end
+
+    def action_from_button
+      if params[:suspend]
+        'suspend'
+      elsif params[:approve]
+        'approve'
+      elsif params[:reject]
+        'reject'
+      end
+    end
   end
 end
diff --git a/app/controllers/admin/pending_accounts_controller.rb b/app/controllers/admin/pending_accounts_controller.rb
deleted file mode 100644
index b62a9bc84..000000000
--- a/app/controllers/admin/pending_accounts_controller.rb
+++ /dev/null
@@ -1,52 +0,0 @@
-# frozen_string_literal: true
-
-module Admin
-  class PendingAccountsController < BaseController
-    before_action :set_accounts, only: :index
-
-    def index
-      @form = Form::AccountBatch.new
-    end
-
-    def batch
-      @form = Form::AccountBatch.new(form_account_batch_params.merge(current_account: current_account, action: action_from_button))
-      @form.save
-    rescue ActionController::ParameterMissing
-      flash[:alert] = I18n.t('admin.accounts.no_account_selected')
-    ensure
-      redirect_to admin_pending_accounts_path(current_params)
-    end
-
-    def approve_all
-      Form::AccountBatch.new(current_account: current_account, account_ids: User.pending.pluck(:account_id), action: 'approve').save
-      redirect_to admin_pending_accounts_path(current_params)
-    end
-
-    def reject_all
-      Form::AccountBatch.new(current_account: current_account, account_ids: User.pending.pluck(:account_id), action: 'reject').save
-      redirect_to admin_pending_accounts_path(current_params)
-    end
-
-    private
-
-    def set_accounts
-      @accounts = Account.joins(:user).merge(User.pending.recent).includes(user: :invite_request).page(params[:page])
-    end
-
-    def form_account_batch_params
-      params.require(:form_account_batch).permit(:action, account_ids: [])
-    end
-
-    def action_from_button
-      if params[:approve]
-        'approve'
-      elsif params[:reject]
-        'reject'
-      end
-    end
-
-    def current_params
-      params.slice(:page).permit(:page)
-    end
-  end
-end
diff --git a/app/controllers/concerns/accountable_concern.rb b/app/controllers/concerns/accountable_concern.rb
index 3cdcffc51..87d62478d 100644
--- a/app/controllers/concerns/accountable_concern.rb
+++ b/app/controllers/concerns/accountable_concern.rb
@@ -3,7 +3,7 @@
 module AccountableConcern
   extend ActiveSupport::Concern
 
-  def log_action(action, target)
-    Admin::ActionLog.create(account: current_account, action: action, target: target)
+  def log_action(action, target, options = {})
+    Admin::ActionLog.create(account: current_account, action: action, target: target, recorded_changes: options.stringify_keys)
   end
 end
diff --git a/app/helpers/admin/action_logs_helper.rb b/app/helpers/admin/action_logs_helper.rb
index e9a298a24..ae96f7a34 100644
--- a/app/helpers/admin/action_logs_helper.rb
+++ b/app/helpers/admin/action_logs_helper.rb
@@ -36,6 +36,8 @@ module Admin::ActionLogsHelper
 
   def log_target_from_history(type, attributes)
     case type
+    when 'User'
+      attributes['username']
     when 'CustomEmoji'
       attributes['shortcode']
     when 'DomainBlock', 'DomainAllow', 'EmailDomainBlock', 'UnavailableDomain'
diff --git a/app/helpers/admin/dashboard_helper.rb b/app/helpers/admin/dashboard_helper.rb
index 4ee2cdef4..32aaf9f5e 100644
--- a/app/helpers/admin/dashboard_helper.rb
+++ b/app/helpers/admin/dashboard_helper.rb
@@ -1,10 +1,41 @@
 # frozen_string_literal: true
 
 module Admin::DashboardHelper
-  def feature_hint(feature, enabled)
-    indicator   = safe_join([enabled ? t('simple_form.yes') : t('simple_form.no'), fa_icon('power-off fw')], ' ')
-    class_names = enabled ? 'pull-right positive-hint' : 'pull-right neutral-hint'
+  def relevant_account_ip(account, ip_query)
+    default_ip = [account.user_current_sign_in_ip || account.user_sign_up_ip]
 
-    safe_join([feature, content_tag(:span, indicator, class: class_names)])
+    matched_ip = begin
+      ip_query_addr = IPAddr.new(ip_query)
+      account.user.recent_ips.find { |(_, ip)| ip_query_addr.include?(ip) } || default_ip
+    rescue IPAddr::Error
+      default_ip
+    end.last
+
+    if matched_ip
+      link_to matched_ip, admin_accounts_path(ip: matched_ip)
+    else
+      '-'
+    end
+  end
+
+  def relevant_account_timestamp(account)
+    timestamp, exact = begin
+      if account.user_current_sign_in_at && account.user_current_sign_in_at < 24.hours.ago
+        [account.user_current_sign_in_at, true]
+      elsif account.user_current_sign_in_at
+        [account.user_current_sign_in_at, false]
+      elsif account.user_pending?
+        [account.user_created_at, true]
+      elsif account.last_status_at.present?
+        [account.last_status_at, true]
+      else
+        [nil, false]
+      end
+    end
+
+    return '-' if timestamp.nil?
+    return t('generic.today') unless exact
+
+    content_tag(:time, l(timestamp), class: 'time-ago', datetime: timestamp.iso8601, title: l(timestamp))
   end
 end
diff --git a/app/javascript/styles/mastodon/accounts.scss b/app/javascript/styles/mastodon/accounts.scss
index b8a6c8018..485fe4a9d 100644
--- a/app/javascript/styles/mastodon/accounts.scss
+++ b/app/javascript/styles/mastodon/accounts.scss
@@ -326,7 +326,12 @@
   }
 }
 
-.batch-table__row--muted .pending-account__header {
+.batch-table__row--muted {
+  color: lighten($ui-base-color, 26%);
+}
+
+.batch-table__row--muted .pending-account__header,
+.batch-table__row--muted .accounts-table {
   &,
   a,
   strong {
@@ -334,10 +339,31 @@
   }
 }
 
-.batch-table__row--attention .pending-account__header {
+.batch-table__row--muted .accounts-table {
+  tbody td.accounts-table__extra,
+  &__count,
+  &__count small {
+    color: lighten($ui-base-color, 26%);
+  }
+}
+
+.batch-table__row--attention {
+  color: $gold-star;
+}
+
+.batch-table__row--attention .pending-account__header,
+.batch-table__row--attention .accounts-table {
   &,
   a,
   strong {
     color: $gold-star;
   }
 }
+
+.batch-table__row--attention .accounts-table {
+  tbody td.accounts-table__extra,
+  &__count,
+  &__count small {
+    color: $gold-star;
+  }
+}
diff --git a/app/javascript/styles/mastodon/tables.scss b/app/javascript/styles/mastodon/tables.scss
index 62f5554ff..36bc07a72 100644
--- a/app/javascript/styles/mastodon/tables.scss
+++ b/app/javascript/styles/mastodon/tables.scss
@@ -237,6 +237,11 @@ a.table-action-link {
         flex: 1 1 auto;
       }
 
+      &__quote {
+        padding: 12px;
+        padding-top: 0;
+      }
+
       &__extra {
         flex: 0 0 auto;
         text-align: right;
diff --git a/app/javascript/styles/mastodon/widgets.scss b/app/javascript/styles/mastodon/widgets.scss
index 4e03868a6..43284eb48 100644
--- a/app/javascript/styles/mastodon/widgets.scss
+++ b/app/javascript/styles/mastodon/widgets.scss
@@ -443,6 +443,24 @@
     }
   }
 
+  tbody td.accounts-table__extra {
+    width: 120px;
+    text-align: right;
+    color: $darker-text-color;
+    padding-right: 16px;
+
+    a {
+      text-decoration: none;
+      color: inherit;
+
+      &:focus,
+      &:hover,
+      &:active {
+        text-decoration: underline;
+      }
+    }
+  }
+
   &__comment {
     width: 50%;
     vertical-align: initial !important;
diff --git a/app/models/account.rb b/app/models/account.rb
index d289c5e53..238ea1d65 100644
--- a/app/models/account.rb
+++ b/app/models/account.rb
@@ -125,6 +125,8 @@ class Account < ApplicationRecord
            :unconfirmed_email,
            :current_sign_in_ip,
            :current_sign_in_at,
+           :created_at,
+           :sign_up_ip,
            :confirmed?,
            :approved?,
            :pending?,
diff --git a/app/models/account_filter.rb b/app/models/account_filter.rb
index 2b001385f..defd531ac 100644
--- a/app/models/account_filter.rb
+++ b/app/models/account_filter.rb
@@ -2,18 +2,15 @@
 
 class AccountFilter
   KEYS = %i(
-    local
-    remote
-    by_domain
-    active
-    pending
-    silenced
-    suspended
+    origin
+    status
+    permissions
     username
+    by_domain
     display_name
     email
     ip
-    staff
+    invited_by
     order
   ).freeze
 
@@ -21,11 +18,10 @@ class AccountFilter
 
   def initialize(params)
     @params = params
-    set_defaults!
   end
 
   def results
-    scope = Account.includes(:user).reorder(nil)
+    scope = Account.includes(:account_stat, user: [:session_activations, :invite_request]).without_instance_actor.reorder(nil)
 
     params.each do |key, value|
       scope.merge!(scope_for(key, value.to_s.strip)) if value.present?
@@ -36,30 +32,16 @@ class AccountFilter
 
   private
 
-  def set_defaults!
-    params['local']  = '1' if params['remote'].blank?
-    params['active'] = '1' if params['suspended'].blank? && params['silenced'].blank? && params['pending'].blank?
-    params['order']  = 'recent' if params['order'].blank?
-  end
-
   def scope_for(key, value)
     case key.to_s
-    when 'local'
-      Account.local.without_instance_actor
-    when 'remote'
-      Account.remote
+    when 'origin'
+      origin_scope(value)
+    when 'permissions'
+      permissions_scope(value)
+    when 'status'
+      status_scope(value)
     when 'by_domain'
       Account.where(domain: value)
-    when 'active'
-      Account.without_suspended
-    when 'pending'
-      accounts_with_users.merge(User.pending)
-    when 'disabled'
-      accounts_with_users.merge(User.disabled)
-    when 'silenced'
-      Account.silenced
-    when 'suspended'
-      Account.suspended
     when 'username'
       Account.matches_username(value)
     when 'display_name'
@@ -68,8 +50,8 @@ class AccountFilter
       accounts_with_users.merge(User.matches_email(value))
     when 'ip'
       valid_ip?(value) ? accounts_with_users.merge(User.matches_ip(value)) : Account.none
-    when 'staff'
-      accounts_with_users.merge(User.staff)
+    when 'invited_by'
+      invited_by_scope(value)
     when 'order'
       order_scope(value)
     else
@@ -77,21 +59,56 @@ class AccountFilter
     end
   end
 
+  def origin_scope(value)
+    case value.to_s
+    when 'local'
+      Account.local
+    when 'remote'
+      Account.remote
+    else
+      raise "Unknown origin: #{value}"
+    end
+  end
+
+  def status_scope(value)
+    case value.to_s
+    when 'active'
+      Account.without_suspended
+    when 'pending'
+      accounts_with_users.merge(User.pending)
+    when 'suspended'
+      Account.suspended
+    else
+      raise "Unknown status: #{value}"
+    end
+  end
+
   def order_scope(value)
-    case value
+    case value.to_s
     when 'active'
-      params['remote'] ? Account.joins(:account_stat).by_recent_status : Account.joins(:user).by_recent_sign_in
+      accounts_with_users.left_joins(:account_stat).order(Arel.sql('coalesce(users.current_sign_in_at, account_stats.last_status_at, to_timestamp(0)) desc, accounts.id desc'))
     when 'recent'
       Account.recent
-    when 'alphabetic'
-      Account.alphabetic
     else
       raise "Unknown order: #{value}"
     end
   end
 
+  def invited_by_scope(value)
+    Account.left_joins(user: :invite).merge(Invite.where(user_id: value.to_s))
+  end
+
+  def permissions_scope(value)
+    case value.to_s
+    when 'staff'
+      accounts_with_users.merge(User.staff)
+    else
+      raise "Unknown permissions: #{value}"
+    end
+  end
+
   def accounts_with_users
-    Account.joins(:user)
+    Account.left_joins(:user)
   end
 
   def valid_ip?(value)
diff --git a/app/models/admin/action_log.rb b/app/models/admin/action_log.rb
index 1d1db1b7a..852bff713 100644
--- a/app/models/admin/action_log.rb
+++ b/app/models/admin/action_log.rb
@@ -17,7 +17,7 @@ class Admin::ActionLog < ApplicationRecord
   serialize :recorded_changes
 
   belongs_to :account
-  belongs_to :target, polymorphic: true
+  belongs_to :target, polymorphic: true, optional: true
 
   default_scope -> { order('id desc') }
 
diff --git a/app/models/admin/action_log_filter.rb b/app/models/admin/action_log_filter.rb
index 6e19dcf70..2af9d7c9c 100644
--- a/app/models/admin/action_log_filter.rb
+++ b/app/models/admin/action_log_filter.rb
@@ -11,6 +11,8 @@ class Admin::ActionLogFilter
     assigned_to_self_report: { target_type: 'Report', action: 'assigned_to_self' }.freeze,
     change_email_user: { target_type: 'User', action: 'change_email' }.freeze,
     confirm_user: { target_type: 'User', action: 'confirm' }.freeze,
+    approve_user: { target_type: 'User', action: 'approve' }.freeze,
+    reject_user: { target_type: 'User', action: 'reject' }.freeze,
     create_account_warning: { target_type: 'AccountWarning', action: 'create' }.freeze,
     create_announcement: { target_type: 'Announcement', action: 'create' }.freeze,
     create_custom_emoji: { target_type: 'CustomEmoji', action: 'create' }.freeze,
diff --git a/app/models/form/account_batch.rb b/app/models/form/account_batch.rb
index f1e1c8a65..4bf1775bb 100644
--- a/app/models/form/account_batch.rb
+++ b/app/models/form/account_batch.rb
@@ -3,6 +3,7 @@
 class Form::AccountBatch
   include ActiveModel::Model
   include Authorization
+  include AccountableConcern
   include Payloadable
 
   attr_accessor :account_ids, :action, :current_account
@@ -25,19 +26,21 @@ class Form::AccountBatch
       suppress_follow_recommendation!
     when 'unsuppress_follow_recommendation'
       unsuppress_follow_recommendation!
+    when 'suspend'
+      suspend!
     end
   end
 
   private
 
   def follow!
-    accounts.find_each do |target_account|
+    accounts.each do |target_account|
       FollowService.new.call(current_account, target_account)
     end
   end
 
   def unfollow!
-    accounts.find_each do |target_account|
+    accounts.each do |target_account|
       UnfollowService.new.call(current_account, target_account)
     end
   end
@@ -61,23 +64,31 @@ class Form::AccountBatch
   end
 
   def approve!
-    users = accounts.includes(:user).map(&:user)
-
-    users.each { |user| authorize(user, :approve?) }
-         .each(&:approve!)
+    accounts.includes(:user).find_each do |account|
+      approve_account(account)
+    end
   end
 
   def reject!
-    records = accounts.includes(:user)
+    accounts.includes(:user).find_each do |account|
+      reject_account(account)
+    end
+  end
 
-    records.each { |account| authorize(account.user, :reject?) }
-           .each { |account| DeleteAccountService.new.call(account, reserve_email: false, reserve_username: false) }
+  def suspend!
+    accounts.find_each do |account|
+      if account.user_pending?
+        reject_account(account)
+      else
+        suspend_account(account)
+      end
+    end
   end
 
   def suppress_follow_recommendation!
     authorize(:follow_recommendation, :suppress?)
 
-    accounts.each do |account|
+    accounts.find_each do |account|
       FollowRecommendationSuppression.create(account: account)
     end
   end
@@ -87,4 +98,24 @@ class Form::AccountBatch
 
     FollowRecommendationSuppression.where(account_id: account_ids).destroy_all
   end
+
+  def reject_account(account)
+    authorize(account.user, :reject?)
+    log_action(:reject, account.user, username: account.username)
+    account.suspend!(origin: :local)
+    AccountDeletionWorker.perform_async(account.id, reserve_username: false)
+  end
+
+  def suspend_account(account)
+    authorize(account, :suspend?)
+    log_action(:suspend, account)
+    account.suspend!(origin: :local)
+    Admin::SuspensionWorker.perform_async(account.id)
+  end
+
+  def approve_account(account)
+    authorize(account.user, :approve?)
+    log_action(:approve, account.user)
+    account.user.approve!
+  end
 end
diff --git a/app/views/admin/accounts/_account.html.haml b/app/views/admin/accounts/_account.html.haml
index c9bd8c686..2df91301e 100644
--- a/app/views/admin/accounts/_account.html.haml
+++ b/app/views/admin/accounts/_account.html.haml
@@ -1,24 +1,35 @@
-%tr
-  %td
-    = admin_account_link_to(account)
-  %td
-    %div.account-badges= account_badge(account, all: true)
-  %td
-    - if account.user_current_sign_in_ip
-      %samp.ellipsized-ip{ title: account.user_current_sign_in_ip }= account.user_current_sign_in_ip
-    - else
-      \-
-  %td
-    - if account.user_current_sign_in_at
-      %time.time-ago{ datetime: account.user_current_sign_in_at.iso8601, title: l(account.user_current_sign_in_at) }= l account.user_current_sign_in_at
-    - elsif account.last_status_at.present?
-      %time.time-ago{ datetime: account.last_status_at.iso8601, title: l(account.last_status_at) }= l account.last_status_at
-    - else
-      \-
-  %td
-    - if account.local? && account.user_pending?
-      = table_link_to 'check', t('admin.accounts.approve'), approve_admin_account_path(account.id), method: :post, data: { confirm: t('admin.accounts.are_you_sure') } if can?(:approve, account.user)
-      = table_link_to 'times', t('admin.accounts.reject'), reject_admin_account_path(account.id), method: :post, data: { confirm: t('admin.accounts.are_you_sure') } if can?(:reject, account.user)
-    - else
-      = table_link_to 'circle', t('admin.accounts.web'), web_path("accounts/#{account.id}")
-      = table_link_to 'globe', t('admin.accounts.public'), ActivityPub::TagManager.instance.url_for(account)
+.batch-table__row{ class: [!account.suspended? && account.user_pending? && 'batch-table__row--attention', account.suspended? && 'batch-table__row--muted'] }
+  %label.batch-table__row__select.batch-table__row__select--aligned.batch-checkbox
+    = f.check_box :account_ids, { multiple: true, include_hidden: false }, account.id
+  .batch-table__row__content.batch-table__row__content--unpadded
+    %table.accounts-table
+      %tbody
+        %tr
+          %td
+            = account_link_to account, path: admin_account_path(account.id)
+          %td.accounts-table__count.optional
+            - if account.suspended? || account.user_pending?
+              \-
+            - else
+              = friendly_number_to_human account.statuses_count
+            %small= t('accounts.posts', count: account.statuses_count).downcase
+          %td.accounts-table__count.optional
+            - if account.suspended? || account.user_pending?
+              \-
+            - else
+              = friendly_number_to_human account.followers_count
+            %small= t('accounts.followers', count: account.followers_count).downcase
+          %td.accounts-table__count
+            = relevant_account_timestamp(account)
+            %small= t('accounts.last_active')
+          %td.accounts-table__extra
+            - if account.local?
+              - if account.user_email
+                = link_to account.user_email.split('@').last, admin_accounts_path(email: "%@#{account.user_email.split('@').last}"), title: account.user_email
+              - else
+                \-
+              %br/
+              %samp.ellipsized-ip= relevant_account_ip(account, params[:ip])
+    - if !account.suspended? && account.user_pending? && account.user&.invite_request&.text&.present?
+      .batch-table__row__content__quote
+        %p= account.user&.invite_request&.text
diff --git a/app/views/admin/accounts/index.html.haml b/app/views/admin/accounts/index.html.haml
index 398ab4bb4..7c0045145 100644
--- a/app/views/admin/accounts/index.html.haml
+++ b/app/views/admin/accounts/index.html.haml
@@ -1,34 +1,37 @@
 - content_for :page_title do
   = t('admin.accounts.title')
 
+- content_for :header_tags do
+  = javascript_pack_tag 'admin', async: true, crossorigin: 'anonymous'
+
 .filters
   .filter-subset
     %strong= t('admin.accounts.location.title')
     %ul
-      %li= filter_link_to t('admin.accounts.location.local'), remote: nil
-      %li= filter_link_to t('admin.accounts.location.remote'), remote: '1'
+      %li= filter_link_to t('generic.all'), origin: nil
+      %li= filter_link_to t('admin.accounts.location.local'), origin: 'local'
+      %li= filter_link_to t('admin.accounts.location.remote'), origin: 'remote'
   .filter-subset
     %strong= t('admin.accounts.moderation.title')
     %ul
-      %li= link_to safe_join([t('admin.accounts.moderation.pending'), "(#{number_with_delimiter(User.pending.count)})"], ' '), admin_pending_accounts_path
-      %li= filter_link_to t('admin.accounts.moderation.active'), silenced: nil, suspended: nil, pending: nil
-      %li= filter_link_to t('admin.accounts.moderation.silenced'), silenced: '1', suspended: nil, pending: nil
-      %li= filter_link_to t('admin.accounts.moderation.suspended'), suspended: '1', silenced: nil, pending: nil
+      %li= filter_link_to t('generic.all'), status: nil
+      %li= filter_link_to t('admin.accounts.moderation.active'), status: 'active'
+      %li= filter_link_to t('admin.accounts.moderation.suspended'), status: 'suspended'
+      %li= filter_link_to safe_join([t('admin.accounts.moderation.pending'), "(#{number_with_delimiter(User.pending.count)})"], ' '), status: 'pending'
   .filter-subset
     %strong= t('admin.accounts.role')
     %ul
-      %li= filter_link_to t('admin.accounts.moderation.all'), staff: nil
-      %li= filter_link_to t('admin.accounts.roles.staff'), staff: '1'
+      %li= filter_link_to t('admin.accounts.moderation.all'), permissions: nil
+      %li= filter_link_to t('admin.accounts.roles.staff'), permissions: 'staff'
   .filter-subset
     %strong= t 'generic.order_by'
     %ul
       %li= filter_link_to t('relationships.most_recent'), order: nil
-      %li= filter_link_to t('admin.accounts.username'), order: 'alphabetic'
       %li= filter_link_to t('relationships.last_active'), order: 'active'
 
 = form_tag admin_accounts_url, method: 'GET', class: 'simple_form' do
   .fields-group
-    - AccountFilter::KEYS.each do |key|
+    - (AccountFilter::KEYS - %i(origin status permissions)).each do |key|
       - if params[key].present?
         = hidden_field_tag key, params[key]
 
@@ -41,16 +44,27 @@
       %button.button= t('admin.accounts.search')
       = link_to t('admin.accounts.reset'), admin_accounts_path, class: 'button negative'
 
-.table-wrapper
-  %table.table
-    %thead
-      %tr
-        %th= t('admin.accounts.username')
-        %th= t('admin.accounts.role')
-        %th= t('admin.accounts.most_recent_ip')
-        %th= t('admin.accounts.most_recent_activity')
-        %th
-    %tbody
-      = render partial: 'account', collection: @accounts
+= form_for(@form, url: batch_admin_accounts_path) do |f|
+  = hidden_field_tag :page, params[:page] || 1
+
+  - AccountFilter::KEYS.each do |key|
+    = hidden_field_tag key, params[key] if params[key].present?
+
+  .batch-table
+    .batch-table__toolbar
+      %label.batch-table__toolbar__select.batch-checkbox-all
+        = check_box_tag :batch_checkbox_all, nil, false
+      .batch-table__toolbar__actions
+        - if @accounts.any? { |account| account.user_pending? }
+          = f.button safe_join([fa_icon('check'), t('admin.accounts.approve')]), name: :approve, class: 'table-action-link', type: :submit, data: { confirm: t('admin.reports.are_you_sure') }
+
+          = f.button safe_join([fa_icon('times'), t('admin.accounts.reject')]), name: :reject, class: 'table-action-link', type: :submit, data: { confirm: t('admin.reports.are_you_sure') }
+
+        = f.button safe_join([fa_icon('lock'), t('admin.accounts.perform_full_suspension')]), name: :suspend, class: 'table-action-link', type: :submit, data: { confirm: t('admin.reports.are_you_sure') }
+    .batch-table__body
+      - if @accounts.empty?
+        = nothing_here 'nothing-here--under-tabs'
+      - else
+        = render partial: 'account', collection: @accounts, locals: { f: f }
 
 = paginate @accounts
diff --git a/app/views/admin/dashboard/index.html.haml b/app/views/admin/dashboard/index.html.haml
index 895333a58..4b581f5ea 100644
--- a/app/views/admin/dashboard/index.html.haml
+++ b/app/views/admin/dashboard/index.html.haml
@@ -38,7 +38,7 @@
       %span= t('admin.dashboard.pending_reports_html', count: @pending_reports_count)
       = fa_icon 'chevron-right fw'
 
-    = link_to admin_pending_accounts_path, class: 'dashboard__quick-access' do
+    = link_to admin_accounts_path(status: 'pending'), class: 'dashboard__quick-access' do
       %span= t('admin.dashboard.pending_users_html', count: @pending_users_count)
       = fa_icon 'chevron-right fw'
 
diff --git a/app/views/admin/instances/show.html.haml b/app/views/admin/instances/show.html.haml
index 462529338..d6542ac3e 100644
--- a/app/views/admin/instances/show.html.haml
+++ b/app/views/admin/instances/show.html.haml
@@ -15,7 +15,7 @@
 
 .dashboard__counters
   %div
-    = link_to admin_accounts_path(remote: '1', by_domain: @instance.domain) do
+    = link_to admin_accounts_path(origin: 'remote', by_domain: @instance.domain) do
       .dashboard__counters__num= number_with_delimiter @instance.accounts_count
       .dashboard__counters__label= t 'admin.accounts.title'
   %div
diff --git a/app/views/admin/ip_blocks/_ip_block.html.haml b/app/views/admin/ip_blocks/_ip_block.html.haml
index e07e2b444..b8d3ac0e8 100644
--- a/app/views/admin/ip_blocks/_ip_block.html.haml
+++ b/app/views/admin/ip_blocks/_ip_block.html.haml
@@ -1,9 +1,9 @@
 .batch-table__row
   %label.batch-table__row__select.batch-table__row__select--aligned.batch-checkbox
     = f.check_box :ip_block_ids, { multiple: true, include_hidden: false }, ip_block.id
-  .batch-table__row__content
-    .batch-table__row__content__text
-      %samp= "#{ip_block.ip}/#{ip_block.ip.prefix}"
+  .batch-table__row__content.pending-account
+    .pending-account__header
+      %samp= link_to "#{ip_block.ip}/#{ip_block.ip.prefix}", admin_accounts_path(ip: "#{ip_block.ip}/#{ip_block.ip.prefix}")
       - if ip_block.comment.present?
         •
         = ip_block.comment
diff --git a/app/views/admin/pending_accounts/_account.html.haml b/app/views/admin/pending_accounts/_account.html.haml
deleted file mode 100644
index 5b475b59a..000000000
--- a/app/views/admin/pending_accounts/_account.html.haml
+++ /dev/null
@@ -1,16 +0,0 @@
-.batch-table__row
-  %label.batch-table__row__select.batch-table__row__select--aligned.batch-checkbox
-    = f.check_box :account_ids, { multiple: true, include_hidden: false }, account.id
-  .batch-table__row__content.pending-account
-    .pending-account__header
-      = link_to admin_account_path(account.id) do
-        %strong= account.user_email
-        = "(@#{account.username})"
-      %br/
-      %samp= account.user_current_sign_in_ip
-      •
-      = t 'admin.accounts.time_in_queue', time: time_ago_in_words(account.user&.created_at)
-
-    - if account.user&.invite_request&.text&.present?
-      .pending-account__body
-        %p= account.user&.invite_request&.text
diff --git a/app/views/admin/pending_accounts/index.html.haml b/app/views/admin/pending_accounts/index.html.haml
deleted file mode 100644
index 8384a1c9f..000000000
--- a/app/views/admin/pending_accounts/index.html.haml
+++ /dev/null
@@ -1,33 +0,0 @@
-- content_for :page_title do
-  = t('admin.pending_accounts.title', count: User.pending.count)
-
-- content_for :header_tags do
-  = javascript_pack_tag 'admin', async: true, crossorigin: 'anonymous'
-
-= form_for(@form, url: batch_admin_pending_accounts_path) do |f|
-  = hidden_field_tag :page, params[:page] || 1
-
-  .batch-table
-    .batch-table__toolbar
-      %label.batch-table__toolbar__select.batch-checkbox-all
-        = check_box_tag :batch_checkbox_all, nil, false
-      .batch-table__toolbar__actions
-        = f.button safe_join([fa_icon('check'), t('admin.accounts.approve')]), name: :approve, class: 'table-action-link', type: :submit, data: { confirm: t('admin.reports.are_you_sure') }
-
-        = f.button safe_join([fa_icon('times'), t('admin.accounts.reject')]), name: :reject, class: 'table-action-link', type: :submit, data: { confirm: t('admin.reports.are_you_sure') }
-    .batch-table__body
-      - if @accounts.empty?
-        = nothing_here 'nothing-here--under-tabs'
-      - else
-        = render partial: 'account', collection: @accounts, locals: { f: f }
-
-= paginate @accounts
-
-%hr.spacer/
-
-%div.action-buttons
-  %div
-    = link_to t('admin.accounts.approve_all'), approve_all_admin_pending_accounts_path, method: :post, data: { confirm: t('admin.accounts.are_you_sure') }, class: 'button'
-
-  %div
-    = link_to t('admin.accounts.reject_all'), reject_all_admin_pending_accounts_path, method: :post, data: { confirm: t('admin.accounts.are_you_sure') }, class: 'button button--destructive'
diff --git a/app/views/admin_mailer/new_pending_account.text.erb b/app/views/admin_mailer/new_pending_account.text.erb
index a466ee2de..bcc251819 100644
--- a/app/views/admin_mailer/new_pending_account.text.erb
+++ b/app/views/admin_mailer/new_pending_account.text.erb
@@ -9,4 +9,4 @@
 <%= quote_wrap(@account.user&.invite_request&.text) %>
 <% end %>
 
-<%= raw t('application_mailer.view')%> <%= admin_pending_accounts_url %>
+<%= raw t('application_mailer.view')%> <%= admin_accounts_url(status: 'pending') %>
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 1aa96ba0f..0aa25ae86 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -99,7 +99,6 @@ en:
     accounts:
       add_email_domain_block: Block e-mail domain
       approve: Approve
-      approve_all: Approve all
       approved_msg: Successfully approved %{username}'s sign-up application
       are_you_sure: Are you sure?
       avatar: Avatar
@@ -153,7 +152,6 @@ en:
         active: Active
         all: All
         pending: Pending
-        silenced: Limited
         suspended: Suspended
         title: Moderation
       moderation_notes: Moderation notes
@@ -171,7 +169,6 @@ en:
       redownload: Refresh profile
       redownloaded_msg: Successfully refreshed %{username}'s profile from origin
       reject: Reject
-      reject_all: Reject all
       rejected_msg: Successfully rejected %{username}'s sign-up application
       remove_avatar: Remove avatar
       remove_header: Remove header
@@ -210,7 +207,6 @@ en:
       suspended: Suspended
       suspension_irreversible: The data of this account has been irreversibly deleted. You can unsuspend the account to make it usable but it will not recover any data it previously had.
       suspension_reversible_hint_html: The account has been suspended, and the data will be fully removed on %{date}. Until then, the account can be restored without any ill effects. If you wish to remove all of the account's data immediately, you can do so below.
-      time_in_queue: Waiting in queue %{time}
       title: Accounts
       unconfirmed_email: Unconfirmed email
       undo_sensitized: Undo force-sensitive
@@ -226,6 +222,7 @@ en:
       whitelisted: Allowed for federation
     action_logs:
       action_types:
+        approve_user: Approve User
         assigned_to_self_report: Assign Report
         change_email_user: Change E-mail for User
         confirm_user: Confirm User
@@ -255,6 +252,7 @@ en:
         enable_user: Enable User
         memorialize_account: Memorialize Account
         promote_user: Promote User
+        reject_user: Reject User
         remove_avatar_user: Remove Avatar
         reopen_report: Reopen Report
         reset_password_user: Reset Password
@@ -271,6 +269,7 @@ en:
         update_domain_block: Update Domain Block
         update_status: Update Post
       actions:
+        approve_user_html: "%{name} approved sign-up from %{target}"
         assigned_to_self_report_html: "%{name} assigned report %{target} to themselves"
         change_email_user_html: "%{name} changed the e-mail address of user %{target}"
         confirm_user_html: "%{name} confirmed e-mail address of user %{target}"
@@ -300,6 +299,7 @@ en:
         enable_user_html: "%{name} enabled login for user %{target}"
         memorialize_account_html: "%{name} turned %{target}'s account into a memoriam page"
         promote_user_html: "%{name} promoted user %{target}"
+        reject_user_html: "%{name} rejected sign-up from %{target}"
         remove_avatar_user_html: "%{name} removed %{target}'s avatar"
         reopen_report_html: "%{name} reopened report %{target}"
         reset_password_user_html: "%{name} reset password of user %{target}"
@@ -519,8 +519,6 @@ en:
         title: Create new IP rule
       no_ip_block_selected: No IP rules were changed as none were selected
       title: IP rules
-    pending_accounts:
-      title: Pending accounts (%{count})
     relationships:
       title: "%{acct}'s relationships"
     relays:
@@ -980,6 +978,7 @@ en:
     none: None
     order_by: Order by
     save_changes: Save changes
+    today: today
     validation_errors:
       one: Something isn't quite right yet! Please review the error below
       other: Something isn't quite right yet! Please review %{count} errors below
diff --git a/config/navigation.rb b/config/navigation.rb
index 99743c222..fc03a2a77 100644
--- a/config/navigation.rb
+++ b/config/navigation.rb
@@ -41,7 +41,7 @@ SimpleNavigation::Configuration.run do |navigation|
     n.item :moderation, safe_join([fa_icon('gavel fw'), t('moderation.title')]), admin_reports_url, if: proc { current_user.staff? } do |s|
       s.item :action_logs, safe_join([fa_icon('bars fw'), t('admin.action_logs.title')]), admin_action_logs_url
       s.item :reports, safe_join([fa_icon('flag fw'), t('admin.reports.title')]), admin_reports_url, highlights_on: %r{/admin/reports}
-      s.item :accounts, safe_join([fa_icon('users fw'), t('admin.accounts.title')]), admin_accounts_url, highlights_on: %r{/admin/accounts|/admin/pending_accounts}
+      s.item :accounts, safe_join([fa_icon('users fw'), t('admin.accounts.title')]), admin_accounts_url(origin: 'local'), highlights_on: %r{/admin/accounts|/admin/pending_accounts}
       s.item :invites, safe_join([fa_icon('user-plus fw'), t('admin.invites.title')]), admin_invites_path
       s.item :follow_recommendations, safe_join([fa_icon('user-plus fw'), t('admin.follow_recommendations.title')]), admin_follow_recommendations_path, highlights_on: %r{/admin/follow_recommendations}
       s.item :instances, safe_join([fa_icon('cloud fw'), t('admin.instances.title')]), admin_instances_url(limited: whitelist_mode? ? nil : '1'), highlights_on: %r{/admin/instances|/admin/domain_blocks|/admin/domain_allows}, if: -> { current_user.admin? }
diff --git a/config/routes.rb b/config/routes.rb
index 5f73129ea..31b398e2c 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -251,6 +251,10 @@ Rails.application.routes.draw do
         post :reject
       end
 
+      collection do
+        post :batch
+      end
+
       resource :change_email, only: [:show, :update]
       resource :reset, only: [:create]
       resource :action, only: [:new, :create], controller: 'account_actions'
@@ -271,14 +275,6 @@ Rails.application.routes.draw do
       end
     end
 
-    resources :pending_accounts, only: [:index] do
-      collection do
-        post :approve_all
-        post :reject_all
-        post :batch
-      end
-    end
-
     resources :users, only: [] do
       resource :two_factor_authentication, only: [:destroy]
       resource :sign_in_token_authentication, only: [:create, :destroy]
diff --git a/spec/controllers/admin/accounts_controller_spec.rb b/spec/controllers/admin/accounts_controller_spec.rb
index 608606ff9..a5ef396ae 100644
--- a/spec/controllers/admin/accounts_controller_spec.rb
+++ b/spec/controllers/admin/accounts_controller_spec.rb
@@ -21,12 +21,9 @@ RSpec.describe Admin::AccountsController, type: :controller do
       expect(AccountFilter).to receive(:new) do |params|
         h = params.to_h
 
-        expect(h[:local]).to eq '1'
-        expect(h[:remote]).to eq '1'
+        expect(h[:origin]).to eq 'local'
         expect(h[:by_domain]).to eq 'domain'
-        expect(h[:active]).to eq '1'
-        expect(h[:silenced]).to eq '1'
-        expect(h[:suspended]).to eq '1'
+        expect(h[:status]).to eq 'active'
         expect(h[:username]).to eq 'username'
         expect(h[:display_name]).to eq 'display name'
         expect(h[:email]).to eq 'local-part@domain'
@@ -36,12 +33,9 @@ RSpec.describe Admin::AccountsController, type: :controller do
       end
 
       get :index, params: {
-        local: '1',
-        remote: '1',
+        origin: 'local',
         by_domain: 'domain',
-        active: '1',
-        silenced: '1',
-        suspended: '1',
+        status: 'active',
         username: 'username',
         display_name: 'display name',
         email: 'local-part@domain',
diff --git a/spec/models/account_filter_spec.rb b/spec/models/account_filter_spec.rb
index 0cdb373f6..c2bd8c220 100644
--- a/spec/models/account_filter_spec.rb
+++ b/spec/models/account_filter_spec.rb
@@ -2,10 +2,10 @@ require 'rails_helper'
 
 describe AccountFilter do
   describe 'with empty params' do
-    it 'defaults to recent local not-suspended account list' do
+    it 'excludes instance actor by default' do
       filter = described_class.new({})
 
-      expect(filter.results).to eq Account.local.without_instance_actor.recent.without_suspended
+      expect(filter.results).to eq Account.without_instance_actor
     end
   end
 
@@ -16,42 +16,4 @@ describe AccountFilter do
       expect { filter.results }.to raise_error(/wrong/)
     end
   end
-
-  describe 'with valid params' do
-    it 'combines filters on Account' do
-      filter = described_class.new(
-        by_domain: 'test.com',
-        silenced: true,
-        username: 'test',
-        display_name: 'name',
-        email: 'user@example.com',
-      )
-
-      allow(Account).to receive(:where).and_return(Account.none)
-      allow(Account).to receive(:silenced).and_return(Account.none)
-      allow(Account).to receive(:matches_display_name).and_return(Account.none)
-      allow(Account).to receive(:matches_username).and_return(Account.none)
-      allow(User).to receive(:matches_email).and_return(User.none)
-
-      filter.results
-
-      expect(Account).to have_received(:where).with(domain: 'test.com')
-      expect(Account).to have_received(:silenced)
-      expect(Account).to have_received(:matches_username).with('test')
-      expect(Account).to have_received(:matches_display_name).with('name')
-      expect(User).to have_received(:matches_email).with('user@example.com')
-    end
-
-    describe 'that call account methods' do
-      %i(local remote silenced suspended).each do |option|
-        it "delegates the #{option} option" do
-          allow(Account).to receive(option).and_return(Account.none)
-          filter = described_class.new({ option => true })
-          filter.results
-
-          expect(Account).to have_received(option).at_least(1)
-        end
-      end
-    end
-  end
 end
-- 
cgit 


From 41503507ec34175a759e5ae7c04fc2afb77ff2b7 Mon Sep 17 00:00:00 2001
From: heguro <65112898+heguro@users.noreply.github.com>
Date: Mon, 6 Dec 2021 05:50:12 +0900
Subject: Fix redirection when succeeded WebAuthn (#17098)

---
 app/controllers/concerns/two_factor_authentication_concern.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'app/controllers/concerns')

diff --git a/app/controllers/concerns/two_factor_authentication_concern.rb b/app/controllers/concerns/two_factor_authentication_concern.rb
index 2583d324b..27f2367a8 100644
--- a/app/controllers/concerns/two_factor_authentication_concern.rb
+++ b/app/controllers/concerns/two_factor_authentication_concern.rb
@@ -57,7 +57,7 @@ module TwoFactorAuthenticationConcern
 
     if valid_webauthn_credential?(user, webauthn_credential)
       on_authentication_success(user, :webauthn)
-      render json: { redirect_path: root_path }, status: :ok
+      render json: { redirect_path: after_sign_in_path_for(user) }, status: :ok
     else
       on_authentication_failure(user, :webauthn, :invalid_credential)
       render json: { error: t('webauthn_credentials.invalid_credential') }, status: :unprocessable_entity
-- 
cgit 


From 8e84ebf0cb211c1d94145399b05c9f2ad0e4d4b0 Mon Sep 17 00:00:00 2001
From: Eugen Rochko <eugen@zeonfederated.com>
Date: Sun, 16 Jan 2022 13:23:50 +0100
Subject: Remove IP tracking columns from users table (#16409)

---
 .../api/v1/admin/accounts_controller.rb            |  2 +-
 app/controllers/auth/sessions_controller.rb        |  2 +-
 app/controllers/concerns/user_tracking_concern.rb  |  6 +--
 app/helpers/admin/dashboard_helper.rb              | 10 ++--
 app/models/account.rb                              |  1 -
 app/models/account_filter.rb                       |  2 +-
 app/models/user.rb                                 | 58 ++++++----------------
 app/models/user_ip.rb                              | 19 +++++++
 app/serializers/rest/admin/account_serializer.rb   | 13 +++--
 app/serializers/rest/admin/ip_serializer.rb        |  5 ++
 app/views/admin/accounts/show.html.haml            | 10 ++--
 .../admin_mailer/new_pending_account.text.erb      |  2 +-
 app/workers/scheduler/ip_cleanup_scheduler.rb      |  2 +-
 config/initializers/devise.rb                      | 15 +++---
 db/migrate/20210616214526_create_user_ips.rb       |  5 ++
 ...6214135_remove_current_sign_in_ip_from_users.rb | 12 +++++
 db/schema.rb                                       | 24 ++++++++-
 db/views/user_ips_v01.sql                          | 26 ++++++++++
 spec/controllers/auth/sessions_controller_spec.rb  |  2 +-
 19 files changed, 141 insertions(+), 75 deletions(-)
 create mode 100644 app/models/user_ip.rb
 create mode 100644 app/serializers/rest/admin/ip_serializer.rb
 create mode 100644 db/migrate/20210616214526_create_user_ips.rb
 create mode 100644 db/post_migrate/20210616214135_remove_current_sign_in_ip_from_users.rb
 create mode 100644 db/views/user_ips_v01.sql

(limited to 'app/controllers/concerns')

diff --git a/app/controllers/api/v1/admin/accounts_controller.rb b/app/controllers/api/v1/admin/accounts_controller.rb
index 63cc521ed..9b8f2fb05 100644
--- a/app/controllers/api/v1/admin/accounts_controller.rb
+++ b/app/controllers/api/v1/admin/accounts_controller.rb
@@ -94,7 +94,7 @@ class Api::V1::Admin::AccountsController < Api::BaseController
   private
 
   def set_accounts
-    @accounts = filtered_accounts.order(id: :desc).includes(user: [:invite_request, :invite]).to_a_paginated_by_id(limit_param(LIMIT), params_slice(:max_id, :since_id, :min_id))
+    @accounts = filtered_accounts.order(id: :desc).includes(user: [:invite_request, :invite, :ips]).to_a_paginated_by_id(limit_param(LIMIT), params_slice(:max_id, :since_id, :min_id))
   end
 
   def set_account
diff --git a/app/controllers/auth/sessions_controller.rb b/app/controllers/auth/sessions_controller.rb
index 0184bfb52..3337a43c4 100644
--- a/app/controllers/auth/sessions_controller.rb
+++ b/app/controllers/auth/sessions_controller.rb
@@ -147,7 +147,7 @@ class Auth::SessionsController < Devise::SessionsController
 
     clear_attempt_from_session
 
-    user.update_sign_in!(request, new_sign_in: true)
+    user.update_sign_in!(new_sign_in: true)
     sign_in(user)
     flash.delete(:notice)
 
diff --git a/app/controllers/concerns/user_tracking_concern.rb b/app/controllers/concerns/user_tracking_concern.rb
index efda37fae..45f3aab0d 100644
--- a/app/controllers/concerns/user_tracking_concern.rb
+++ b/app/controllers/concerns/user_tracking_concern.rb
@@ -3,7 +3,7 @@
 module UserTrackingConcern
   extend ActiveSupport::Concern
 
-  UPDATE_SIGN_IN_HOURS = 24
+  UPDATE_SIGN_IN_FREQUENCY = 24.hours.freeze
 
   included do
     before_action :update_user_sign_in
@@ -12,10 +12,10 @@ module UserTrackingConcern
   private
 
   def update_user_sign_in
-    current_user.update_sign_in!(request) if user_needs_sign_in_update?
+    current_user.update_sign_in! if user_needs_sign_in_update?
   end
 
   def user_needs_sign_in_update?
-    user_signed_in? && (current_user.current_sign_in_at.nil? || current_user.current_sign_in_at < UPDATE_SIGN_IN_HOURS.hours.ago)
+    user_signed_in? && (current_user.current_sign_in_at.nil? || current_user.current_sign_in_at < UPDATE_SIGN_IN_FREQUENCY.ago)
   end
 end
diff --git a/app/helpers/admin/dashboard_helper.rb b/app/helpers/admin/dashboard_helper.rb
index 32aaf9f5e..d4a30b97e 100644
--- a/app/helpers/admin/dashboard_helper.rb
+++ b/app/helpers/admin/dashboard_helper.rb
@@ -2,17 +2,17 @@
 
 module Admin::DashboardHelper
   def relevant_account_ip(account, ip_query)
-    default_ip = [account.user_current_sign_in_ip || account.user_sign_up_ip]
+    ips = account.user.ips.to_a
 
     matched_ip = begin
       ip_query_addr = IPAddr.new(ip_query)
-      account.user.recent_ips.find { |(_, ip)| ip_query_addr.include?(ip) } || default_ip
+      ips.find { |ip| ip_query_addr.include?(ip.ip) } || ips.first
     rescue IPAddr::Error
-      default_ip
-    end.last
+      ips.first
+    end
 
     if matched_ip
-      link_to matched_ip, admin_accounts_path(ip: matched_ip)
+      link_to matched_ip.ip, admin_accounts_path(ip: matched_ip.ip)
     else
       '-'
     end
diff --git a/app/models/account.rb b/app/models/account.rb
index 238ea1d65..c459125c7 100644
--- a/app/models/account.rb
+++ b/app/models/account.rb
@@ -123,7 +123,6 @@ class Account < ApplicationRecord
 
   delegate :email,
            :unconfirmed_email,
-           :current_sign_in_ip,
            :current_sign_in_at,
            :created_at,
            :sign_up_ip,
diff --git a/app/models/account_filter.rb b/app/models/account_filter.rb
index defd531ac..dcb174122 100644
--- a/app/models/account_filter.rb
+++ b/app/models/account_filter.rb
@@ -21,7 +21,7 @@ class AccountFilter
   end
 
   def results
-    scope = Account.includes(:account_stat, user: [:session_activations, :invite_request]).without_instance_actor.reorder(nil)
+    scope = Account.includes(:account_stat, user: [:ips, :invite_request]).without_instance_actor.reorder(nil)
 
     params.each do |key, value|
       scope.merge!(scope_for(key, value.to_s.strip)) if value.present?
diff --git a/app/models/user.rb b/app/models/user.rb
index 374b82d05..49dcb8156 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -14,8 +14,6 @@
 #  sign_in_count             :integer          default(0), not null
 #  current_sign_in_at        :datetime
 #  last_sign_in_at           :datetime
-#  current_sign_in_ip        :inet
-#  last_sign_in_ip           :inet
 #  admin                     :boolean          default(FALSE), not null
 #  confirmation_token        :string
 #  confirmed_at              :datetime
@@ -81,6 +79,7 @@ class User < ApplicationRecord
   has_many :invites, inverse_of: :user
   has_many :markers, inverse_of: :user, dependent: :destroy
   has_many :webauthn_credentials, dependent: :destroy
+  has_many :ips, class_name: 'UserIp', inverse_of: :user
 
   has_one :invite_request, class_name: 'UserInviteRequest', inverse_of: :user, dependent: :destroy
   accepts_nested_attributes_for :invite_request, reject_if: ->(attributes) { attributes['text'].blank? && !Setting.require_invite_text }
@@ -107,7 +106,7 @@ class User < ApplicationRecord
   scope :inactive, -> { where(arel_table[:current_sign_in_at].lt(ACTIVE_DURATION.ago)) }
   scope :active, -> { confirmed.where(arel_table[:current_sign_in_at].gteq(ACTIVE_DURATION.ago)).joins(:account).where(accounts: { suspended_at: nil }) }
   scope :matches_email, ->(value) { where(arel_table[:email].matches("#{value}%")) }
-  scope :matches_ip, ->(value) { where('current_sign_in_ip <<= ?', value).or(where('users.sign_up_ip <<= ?', value)).or(where('users.last_sign_in_ip <<= ?', value)).or(where(id: SessionActivation.select(:user_id).where('ip <<= ?', value))) }
+  scope :matches_ip, ->(value) { left_joins(:ips).where('user_ips.ip <<= ?', value) }
   scope :emailable, -> { confirmed.enabled.joins(:account).merge(Account.searchable) }
 
   before_validation :sanitize_languages
@@ -174,15 +173,11 @@ class User < ApplicationRecord
     prepare_new_user! if new_user && approved?
   end
 
-  def update_sign_in!(request, new_sign_in: false)
+  def update_sign_in!(new_sign_in: false)
     old_current, new_current = current_sign_in_at, Time.now.utc
     self.last_sign_in_at     = old_current || new_current
     self.current_sign_in_at  = new_current
 
-    old_current, new_current = current_sign_in_ip, request.remote_ip
-    self.last_sign_in_ip     = old_current || new_current
-    self.current_sign_in_ip  = new_current
-
     if new_sign_in
       self.sign_in_count ||= 0
       self.sign_in_count  += 1
@@ -201,7 +196,7 @@ class User < ApplicationRecord
   end
 
   def suspicious_sign_in?(ip)
-    !otp_required_for_login? && !skip_sign_in_token? && current_sign_in_at.present? && !recent_ip?(ip)
+    !otp_required_for_login? && !skip_sign_in_token? && current_sign_in_at.present? && !ips.where(ip: ip).exists?
   end
 
   def functional?
@@ -277,31 +272,28 @@ class User < ApplicationRecord
     @shows_application ||= settings.show_application
   end
 
-  # rubocop:disable Naming/MethodParameterName
-  def token_for_app(a)
-    return nil if a.nil? || a.owner != self
-    Doorkeeper::AccessToken.find_or_create_by(application_id: a.id, resource_owner_id: id) do |t|
-      t.scopes = a.scopes
-      t.expires_in = Doorkeeper.configuration.access_token_expires_in
+  def token_for_app(app)
+    return nil if app.nil? || app.owner != self
+
+    Doorkeeper::AccessToken.find_or_create_by(application_id: app.id, resource_owner_id: id) do |t|
+      t.scopes            = app.scopes
+      t.expires_in        = Doorkeeper.configuration.access_token_expires_in
       t.use_refresh_token = Doorkeeper.configuration.refresh_token_enabled?
     end
   end
-  # rubocop:enable Naming/MethodParameterName
 
   def activate_session(request)
-    session_activations.activate(session_id: SecureRandom.hex,
-                                 user_agent: request.user_agent,
-                                 ip: request.remote_ip).session_id
+    session_activations.activate(
+      session_id: SecureRandom.hex,
+      user_agent: request.user_agent,
+      ip: request.remote_ip
+    ).session_id
   end
 
   def clear_other_sessions(id)
     session_activations.exclusive(id)
   end
 
-  def session_active?(id)
-    session_activations.active? id
-  end
-
   def web_push_subscription(session)
     session.web_push_subscription.nil? ? nil : session.web_push_subscription
   end
@@ -364,22 +356,6 @@ class User < ApplicationRecord
     setting_display_media == 'hide_all'
   end
 
-  def recent_ips
-    @recent_ips ||= begin
-      arr = []
-
-      session_activations.each do |session_activation|
-        arr << [session_activation.updated_at, session_activation.ip]
-      end
-
-      arr << [current_sign_in_at, current_sign_in_ip] if current_sign_in_ip.present?
-      arr << [last_sign_in_at, last_sign_in_ip] if last_sign_in_ip.present?
-      arr << [created_at, sign_up_ip] if sign_up_ip.present?
-
-      arr.sort_by { |pair| pair.first || Time.now.utc }.uniq(&:last).reverse!
-    end
-  end
-
   def sign_in_token_expired?
     sign_in_token_sent_at.nil? || sign_in_token_sent_at < 5.minutes.ago
   end
@@ -410,10 +386,6 @@ class User < ApplicationRecord
 
   private
 
-  def recent_ip?(ip)
-    recent_ips.any? { |(_, recent_ip)| recent_ip == ip }
-  end
-
   def send_pending_devise_notifications
     pending_devise_notifications.each do |notification, args, kwargs|
       render_and_send_devise_message(notification, *args, **kwargs)
diff --git a/app/models/user_ip.rb b/app/models/user_ip.rb
new file mode 100644
index 000000000..a8e802e13
--- /dev/null
+++ b/app/models/user_ip.rb
@@ -0,0 +1,19 @@
+# frozen_string_literal: true
+# == Schema Information
+#
+# Table name: user_ips
+#
+#  user_id :bigint(8)        primary key
+#  ip      :inet
+#  used_at :datetime
+#
+
+class UserIp < ApplicationRecord
+  self.primary_key = :user_id
+
+  belongs_to :user, foreign_key: :user_id
+
+  def readonly?
+    true
+  end
+end
diff --git a/app/serializers/rest/admin/account_serializer.rb b/app/serializers/rest/admin/account_serializer.rb
index f579d3302..3480e8c5a 100644
--- a/app/serializers/rest/admin/account_serializer.rb
+++ b/app/serializers/rest/admin/account_serializer.rb
@@ -9,6 +9,7 @@ class REST::Admin::AccountSerializer < ActiveModel::Serializer
   attribute :created_by_application_id, if: :created_by_application?
   attribute :invited_by_account_id, if: :invited?
 
+  has_many :ips, serializer: REST::Admin::IpSerializer
   has_one :account, serializer: REST::AccountSerializer
 
   def id
@@ -19,10 +20,6 @@ class REST::Admin::AccountSerializer < ActiveModel::Serializer
     object.user_email
   end
 
-  def ip
-    object.user_current_sign_in_ip.to_s.presence
-  end
-
   def role
     object.user_role
   end
@@ -74,4 +71,12 @@ class REST::Admin::AccountSerializer < ActiveModel::Serializer
   def created_by_application?
     object.user&.created_by_application_id&.present?
   end
+
+  def ips
+    object.user&.ips
+  end
+
+  def ip
+    ips&.first
+  end
 end
diff --git a/app/serializers/rest/admin/ip_serializer.rb b/app/serializers/rest/admin/ip_serializer.rb
new file mode 100644
index 000000000..d11699dc4
--- /dev/null
+++ b/app/serializers/rest/admin/ip_serializer.rb
@@ -0,0 +1,5 @@
+# frozen_string_literal: true
+
+class REST::Admin::IpSerializer < ActiveModel::Serializer
+  attributes :ip, :used_at
+end
diff --git a/app/views/admin/accounts/show.html.haml b/app/views/admin/accounts/show.html.haml
index 64cfc9a77..3867d1b19 100644
--- a/app/views/admin/accounts/show.html.haml
+++ b/app/views/admin/accounts/show.html.haml
@@ -156,12 +156,14 @@
               %time.formatted{ datetime: @account.created_at.iso8601, title: l(@account.created_at) }= l @account.created_at
             %td
 
-          - @account.user.recent_ips.each_with_index do |(_, ip), i|
+          - recent_ips = @account.user.ips.order(used_at: :desc).to_a
+
+          - recent_ips.each_with_index do |recent_ip, i|
             %tr
               - if i.zero?
-                %th{ rowspan: @account.user.recent_ips.size }= t('admin.accounts.most_recent_ip')
-              %td= ip
-              %td= table_link_to 'search', t('admin.accounts.search_same_ip'), admin_accounts_path(ip: ip)
+                %th{ rowspan: recent_ips.size }= t('admin.accounts.most_recent_ip')
+              %td= recent_ip.ip
+              %td= table_link_to 'search', t('admin.accounts.search_same_ip'), admin_accounts_path(ip: recent_ip.ip)
 
           %tr
             %th= t('admin.accounts.most_recent_activity')
diff --git a/app/views/admin_mailer/new_pending_account.text.erb b/app/views/admin_mailer/new_pending_account.text.erb
index bcc251819..a8a2a35fa 100644
--- a/app/views/admin_mailer/new_pending_account.text.erb
+++ b/app/views/admin_mailer/new_pending_account.text.erb
@@ -3,7 +3,7 @@
 <%= raw t('admin_mailer.new_pending_account.body') %>
 
 <%= @account.user_email %> (@<%= @account.username %>)
-<%= @account.user_current_sign_in_ip %>
+<%= @account.user_sign_up_ip %>
 <% if @account.user&.invite_request&.text.present? %>
 
 <%= quote_wrap(@account.user&.invite_request&.text) %>
diff --git a/app/workers/scheduler/ip_cleanup_scheduler.rb b/app/workers/scheduler/ip_cleanup_scheduler.rb
index 918c10ac9..adc99c605 100644
--- a/app/workers/scheduler/ip_cleanup_scheduler.rb
+++ b/app/workers/scheduler/ip_cleanup_scheduler.rb
@@ -16,7 +16,7 @@ class Scheduler::IpCleanupScheduler
 
   def clean_ip_columns!
     SessionActivation.where('updated_at < ?', IP_RETENTION_PERIOD.ago).in_batches.destroy_all
-    User.where('current_sign_in_at < ?', IP_RETENTION_PERIOD.ago).in_batches.update_all(last_sign_in_ip: nil, current_sign_in_ip: nil, sign_up_ip: nil)
+    User.where('current_sign_in_at < ?', IP_RETENTION_PERIOD.ago).in_batches.update_all(sign_up_ip: nil)
     LoginActivity.where('created_at < ?', IP_RETENTION_PERIOD.ago).in_batches.destroy_all
   end
 
diff --git a/config/initializers/devise.rb b/config/initializers/devise.rb
index 5232e6cfd..b434c68fa 100644
--- a/config/initializers/devise.rb
+++ b/config/initializers/devise.rb
@@ -1,11 +1,8 @@
 require 'devise/strategies/authenticatable'
 
 Warden::Manager.after_set_user except: :fetch do |user, warden|
-  if user.session_active?(warden.cookies.signed['_session_id'] || warden.raw_session['auth_id'])
-    session_id = warden.cookies.signed['_session_id'] || warden.raw_session['auth_id']
-  else
-    session_id = user.activate_session(warden.request)
-  end
+  session_id = warden.cookies.signed['_session_id'] || warden.raw_session['auth_id']
+  session_id = user.activate_session(warden.request) unless user.session_activations.active?(session_id)
 
   warden.cookies.signed['_session_id'] = {
     value: session_id,
@@ -17,9 +14,13 @@ Warden::Manager.after_set_user except: :fetch do |user, warden|
 end
 
 Warden::Manager.after_fetch do |user, warden|
-  if user.session_active?(warden.cookies.signed['_session_id'] || warden.raw_session['auth_id'])
+  session_id = warden.cookies.signed['_session_id'] || warden.raw_session['auth_id']
+
+  if session_id && (session = user.session_activations.find_by(session_id: session_id))
+    session.update(ip: warden.request.remote_ip) if session.ip != warden.request.remote_ip
+
     warden.cookies.signed['_session_id'] = {
-      value: warden.cookies.signed['_session_id'] || warden.raw_session['auth_id'],
+      value: session_id,
       expires: 1.year.from_now,
       httponly: true,
       secure: (Rails.env.production? || ENV['LOCAL_HTTPS'] == 'true'),
diff --git a/db/migrate/20210616214526_create_user_ips.rb b/db/migrate/20210616214526_create_user_ips.rb
new file mode 100644
index 000000000..68e81a9d8
--- /dev/null
+++ b/db/migrate/20210616214526_create_user_ips.rb
@@ -0,0 +1,5 @@
+class CreateUserIps < ActiveRecord::Migration[6.1]
+  def change
+    create_view :user_ips
+  end
+end
diff --git a/db/post_migrate/20210616214135_remove_current_sign_in_ip_from_users.rb b/db/post_migrate/20210616214135_remove_current_sign_in_ip_from_users.rb
new file mode 100644
index 000000000..b53b247f2
--- /dev/null
+++ b/db/post_migrate/20210616214135_remove_current_sign_in_ip_from_users.rb
@@ -0,0 +1,12 @@
+# frozen_string_literal: true
+
+class RemoveCurrentSignInIpFromUsers < ActiveRecord::Migration[5.2]
+  disable_ddl_transaction!
+
+  def change
+    safety_assured do
+      remove_column :users, :current_sign_in_ip, :inet
+      remove_column :users, :last_sign_in_ip, :inet
+    end
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index a1d169b23..d1446c652 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -923,8 +923,6 @@ ActiveRecord::Schema.define(version: 2021_12_13_040746) do
     t.integer "sign_in_count", default: 0, null: false
     t.datetime "current_sign_in_at"
     t.datetime "last_sign_in_at"
-    t.inet "current_sign_in_ip"
-    t.inet "last_sign_in_ip"
     t.boolean "admin", default: false, null: false
     t.string "confirmation_token"
     t.datetime "confirmed_at"
@@ -1120,6 +1118,28 @@ ActiveRecord::Schema.define(version: 2021_12_13_040746) do
   SQL
   add_index "instances", ["domain"], name: "index_instances_on_domain", unique: true
 
+  create_view "user_ips", sql_definition: <<-SQL
+      SELECT t0.user_id,
+      t0.ip,
+      max(t0.used_at) AS used_at
+     FROM ( SELECT users.id AS user_id,
+              users.sign_up_ip AS ip,
+              users.created_at AS used_at
+             FROM users
+            WHERE (users.sign_up_ip IS NOT NULL)
+          UNION ALL
+           SELECT session_activations.user_id,
+              session_activations.ip,
+              session_activations.updated_at
+             FROM session_activations
+          UNION ALL
+           SELECT login_activities.user_id,
+              login_activities.ip,
+              login_activities.created_at
+             FROM login_activities
+            WHERE (login_activities.success = true)) t0
+    GROUP BY t0.user_id, t0.ip;
+  SQL
   create_view "account_summaries", materialized: true, sql_definition: <<-SQL
       SELECT accounts.id AS account_id,
       mode() WITHIN GROUP (ORDER BY t0.language) AS language,
diff --git a/db/views/user_ips_v01.sql b/db/views/user_ips_v01.sql
new file mode 100644
index 000000000..50a8201cd
--- /dev/null
+++ b/db/views/user_ips_v01.sql
@@ -0,0 +1,26 @@
+SELECT
+  user_id,
+  ip,
+  max(used_at) AS used_at
+FROM (
+  SELECT
+    id AS user_id,
+    sign_up_ip AS ip,
+    created_at AS used_at
+  FROM users
+  WHERE sign_up_ip IS NOT NULL
+  UNION ALL
+  SELECT
+    user_id,
+    ip,
+    updated_at
+  FROM session_activations
+  UNION ALL
+  SELECT
+    user_id,
+    ip,
+    created_at
+  FROM login_activities
+  WHERE success = 't'
+) AS t0
+GROUP BY user_id, ip
diff --git a/spec/controllers/auth/sessions_controller_spec.rb b/spec/controllers/auth/sessions_controller_spec.rb
index f718f5dd9..2368cc2bf 100644
--- a/spec/controllers/auth/sessions_controller_spec.rb
+++ b/spec/controllers/auth/sessions_controller_spec.rb
@@ -400,7 +400,7 @@ RSpec.describe Auth::SessionsController, type: :controller do
     end
 
     context 'when 2FA is disabled and IP is unfamiliar' do
-      let!(:user) { Fabricate(:user, email: 'x@y.com', password: 'abcdefgh', current_sign_in_at: 3.weeks.ago, current_sign_in_ip: '0.0.0.0') }
+      let!(:user) { Fabricate(:user, email: 'x@y.com', password: 'abcdefgh', current_sign_in_at: 3.weeks.ago) }
 
       before do
         request.remote_ip  = '10.10.10.10'
-- 
cgit 


From c7e2b9cf64ab2db13e85470ae695b3b4c03fae79 Mon Sep 17 00:00:00 2001
From: Claire <claire.github-309c@sitedethib.com>
Date: Mon, 17 Jan 2022 12:50:02 +0100
Subject: Move controller theming code to concern

---
 app/controllers/application_controller.rb   | 76 +--------------------------
 app/controllers/concerns/theming_concern.rb | 80 +++++++++++++++++++++++++++++
 2 files changed, 81 insertions(+), 75 deletions(-)
 create mode 100644 app/controllers/concerns/theming_concern.rb

(limited to 'app/controllers/concerns')

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 0b78e36d6..08cca0734 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -10,6 +10,7 @@ class ApplicationController < ActionController::Base
   include SessionTrackingConcern
   include CacheConcern
   include DomainControlHelper
+  include ThemingConcern
 
   helper_method :current_account
   helper_method :current_session
@@ -73,11 +74,6 @@ class ApplicationController < ActionController::Base
     new_user_session_path
   end
 
-  def use_pack(pack_name)
-    @core = resolve_pack_with_common(Themes.instance.core, pack_name)
-    @theme = resolve_pack_with_common(Themes.instance.flavour(current_flavour), pack_name, current_skin)
-  end
-
   protected
 
   def truthy_param?(key)
@@ -159,74 +155,4 @@ class ApplicationController < ActionController::Base
       format.json { render json: { error: Rack::Utils::HTTP_STATUS_CODES[code] }, status: code }
     end
   end
-
-  private
-
-  def valid_pack_data?(data, pack_name)
-    data['pack'].is_a?(Hash) && [String, Hash].any? { |c| data['pack'][pack_name].is_a?(c) }
-  end
-
-  def nil_pack(data)
-    {
-      use_common: true,
-      flavour: data['name'],
-      pack: nil,
-      preload: nil,
-      skin: nil,
-      supported_locales: data['locales'],
-    }
-  end
-
-  def pack(data, pack_name, skin)
-    pack_data = {
-      use_common: true,
-      flavour: data['name'],
-      pack: pack_name,
-      preload: nil,
-      skin: nil,
-      supported_locales: data['locales'],
-    }
-
-    return pack_data unless data['pack'][pack_name].is_a?(Hash)
-
-    pack_data[:use_common] = false if data['pack'][pack_name]['use_common'] == false
-    pack_data[:pack] = nil unless data['pack'][pack_name]['filename']
-
-    preloads = data['pack'][pack_name]['preload']
-    pack_data[:preload] = [preloads] if preloads.is_a?(String)
-    pack_data[:preload] = preloads if preloads.is_a?(Array)
-
-    if skin != 'default' && data['skin'][skin]
-      pack_data[:skin] = skin if data['skin'][skin].include?(pack_name)
-    else # default skin
-      pack_data[:skin] = 'default' if data['pack'][pack_name]['stylesheet']
-    end
-
-    pack_data
-  end
-
-  def resolve_pack(data, pack_name, skin)
-    return pack(data, pack_name, skin) if valid_pack_data?(data, pack_name)
-    return if data['name'].blank?
-
-    fallbacks = []
-    if data.key?('fallback')
-      fallbacks = data['fallback'] if data['fallback'].is_a?(Array)
-      fallbacks = [data['fallback']] if data['fallback'].is_a?(String)
-    elsif data['name'] != Setting.default_settings['flavour']
-      fallbacks = [Setting.default_settings['flavour']]
-    end
-
-    fallbacks.each do |fallback|
-      return resolve_pack(Themes.instance.flavour(fallback), pack_name) if Themes.instance.flavour(fallback)
-    end
-
-    nil
-  end
-
-  def resolve_pack_with_common(data, pack_name, skin = 'default')
-    result = resolve_pack(data, pack_name, skin) || nil_pack(data)
-    result[:common] = resolve_pack(data, 'common', skin) if result.delete(:use_common)
-    result
-  end
 end
diff --git a/app/controllers/concerns/theming_concern.rb b/app/controllers/concerns/theming_concern.rb
new file mode 100644
index 000000000..6622bcff6
--- /dev/null
+++ b/app/controllers/concerns/theming_concern.rb
@@ -0,0 +1,80 @@
+# frozen_string_literal: true
+
+module ThemingConcern
+  extend ActiveSupport::Concern
+
+  def use_pack(pack_name)
+    @core = resolve_pack_with_common(Themes.instance.core, pack_name)
+    @theme = resolve_pack_with_common(Themes.instance.flavour(current_flavour), pack_name, current_skin)
+  end
+
+  private
+
+  def valid_pack_data?(data, pack_name)
+    data['pack'].is_a?(Hash) && [String, Hash].any? { |c| data['pack'][pack_name].is_a?(c) }
+  end
+
+  def nil_pack(data)
+    {
+      use_common: true,
+      flavour: data['name'],
+      pack: nil,
+      preload: nil,
+      skin: nil,
+      supported_locales: data['locales'],
+    }
+  end
+
+  def pack(data, pack_name, skin)
+    pack_data = {
+      use_common: true,
+      flavour: data['name'],
+      pack: pack_name,
+      preload: nil,
+      skin: nil,
+      supported_locales: data['locales'],
+    }
+
+    return pack_data unless data['pack'][pack_name].is_a?(Hash)
+
+    pack_data[:use_common] = false if data['pack'][pack_name]['use_common'] == false
+    pack_data[:pack] = nil unless data['pack'][pack_name]['filename']
+
+    preloads = data['pack'][pack_name]['preload']
+    pack_data[:preload] = [preloads] if preloads.is_a?(String)
+    pack_data[:preload] = preloads if preloads.is_a?(Array)
+
+    if skin != 'default' && data['skin'][skin]
+      pack_data[:skin] = skin if data['skin'][skin].include?(pack_name)
+    else # default skin
+      pack_data[:skin] = 'default' if data['pack'][pack_name]['stylesheet']
+    end
+
+    pack_data
+  end
+
+  def resolve_pack(data, pack_name, skin)
+    return pack(data, pack_name, skin) if valid_pack_data?(data, pack_name)
+    return if data['name'].blank?
+
+    fallbacks = []
+    if data.key?('fallback')
+      fallbacks = data['fallback'] if data['fallback'].is_a?(Array)
+      fallbacks = [data['fallback']] if data['fallback'].is_a?(String)
+    elsif data['name'] != Setting.default_settings['flavour']
+      fallbacks = [Setting.default_settings['flavour']]
+    end
+
+    fallbacks.each do |fallback|
+      return resolve_pack(Themes.instance.flavour(fallback), pack_name) if Themes.instance.flavour(fallback)
+    end
+
+    nil
+  end
+
+  def resolve_pack_with_common(data, pack_name, skin = 'default')
+    result = resolve_pack(data, pack_name, skin) || nil_pack(data)
+    result[:common] = resolve_pack(data, 'common', skin) if result.delete(:use_common)
+    result
+  end
+end
-- 
cgit 


From b9ed7e0f55f082de429d5e7d545a5424de0052dd Mon Sep 17 00:00:00 2001
From: Claire <claire.github-309c@sitedethib.com>
Date: Mon, 17 Jan 2022 13:06:06 +0100
Subject: Please CodeClimate

---
 app/controllers/concerns/theming_concern.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

(limited to 'app/controllers/concerns')

diff --git a/app/controllers/concerns/theming_concern.rb b/app/controllers/concerns/theming_concern.rb
index 6622bcff6..1ee3256c0 100644
--- a/app/controllers/concerns/theming_concern.rb
+++ b/app/controllers/concerns/theming_concern.rb
@@ -46,8 +46,8 @@ module ThemingConcern
 
     if skin != 'default' && data['skin'][skin]
       pack_data[:skin] = skin if data['skin'][skin].include?(pack_name)
-    else # default skin
-      pack_data[:skin] = 'default' if data['pack'][pack_name]['stylesheet']
+    elsif data['pack'][pack_name]['stylesheet']
+      pack_data[:skin] = 'default'
     end
 
     pack_data
-- 
cgit 


From 1b493c9fee954b5bd4c4b00f9f945a5d97e2d699 Mon Sep 17 00:00:00 2001
From: Claire <claire.github-309c@sitedethib.com>
Date: Mon, 24 Jan 2022 19:06:19 +0100
Subject: Add optional hCaptcha support

Fixes #1649

This requires setting `HCAPTCHA_SECRET_KEY` and `HCAPTCHA_SITE_KEY`, then
enabling the admin setting at
`/admin/settings/edit#form_admin_settings_captcha_enabled`

Subsequently, a hCaptcha widget will be displayed on `/about` and
`/auth/sign_up` unless:
- the user is already signed-up already
- the user has used an invite link
- the user has already solved the captcha (and registration failed for another
  reason)

The Content-Security-Policy headers are altered automatically to allow the
third-party hCaptcha scripts on `/about` and `/auth/sign_up` following the same
rules as above.
---
 .env.production.sample                           |  4 ++
 Gemfile                                          |  2 +
 Gemfile.lock                                     |  3 ++
 app/controllers/about_controller.rb              |  2 +
 app/controllers/api/v1/accounts_controller.rb    |  4 +-
 app/controllers/auth/registrations_controller.rb | 17 ++++++
 app/controllers/concerns/captcha_concern.rb      | 66 ++++++++++++++++++++++++
 app/helpers/admin/settings_helper.rb             |  4 ++
 app/javascript/flavours/glitch/styles/forms.scss |  4 ++
 app/models/form/admin_settings.rb                |  2 +
 app/views/about/_registration.html.haml          |  3 ++
 app/views/admin/settings/edit.html.haml          |  5 +-
 app/views/auth/registrations/new.html.haml       |  3 ++
 config/locales-glitch/en.yml                     |  3 ++
 config/settings.yml                              |  1 +
 15 files changed, 121 insertions(+), 2 deletions(-)
 create mode 100644 app/controllers/concerns/captcha_concern.rb

(limited to 'app/controllers/concerns')

diff --git a/.env.production.sample b/.env.production.sample
index 13e89b40d..7de5e00f4 100644
--- a/.env.production.sample
+++ b/.env.production.sample
@@ -285,3 +285,7 @@ MAX_POLL_OPTION_CHARS=100
 # Units are in bytes
 MAX_EMOJI_SIZE=51200
 MAX_REMOTE_EMOJI_SIZE=204800
+
+# Optional hCaptcha support
+# HCAPTCHA_SECRET_KEY=
+# HCAPTCHA_SITE_KEY=
diff --git a/Gemfile b/Gemfile
index eae5f11b7..282ab65e4 100644
--- a/Gemfile
+++ b/Gemfile
@@ -156,3 +156,5 @@ gem 'concurrent-ruby', require: false
 gem 'connection_pool', require: false
 
 gem 'xorcist', '~> 1.1'
+
+gem "hcaptcha", "~> 7.1"
diff --git a/Gemfile.lock b/Gemfile.lock
index 8d72732eb..cc9a53e41 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -271,6 +271,8 @@ GEM
       railties (>= 4.0.1)
     hashdiff (1.0.1)
     hashie (4.1.0)
+    hcaptcha (7.1.0)
+      json
     highline (2.0.3)
     hiredis (0.6.3)
     hkdf (0.3.0)
@@ -719,6 +721,7 @@ DEPENDENCIES
   fog-openstack (~> 0.3)
   fuubar (~> 2.5)
   hamlit-rails (~> 0.2)
+  hcaptcha (~> 7.1)
   hiredis (~> 0.6)
   htmlentities (~> 4.3)
   http (~> 5.0)
diff --git a/app/controllers/about_controller.rb b/app/controllers/about_controller.rb
index 620c0ff78..5a35dbbcb 100644
--- a/app/controllers/about_controller.rb
+++ b/app/controllers/about_controller.rb
@@ -2,6 +2,7 @@
 
 class AboutController < ApplicationController
   include RegistrationSpamConcern
+  include CaptchaConcern
 
   before_action :set_pack
 
@@ -12,6 +13,7 @@ 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 5c47158e0..8916c3f96 100644
--- a/app/controllers/api/v1/accounts_controller.rb
+++ b/app/controllers/api/v1/accounts_controller.rb
@@ -1,6 +1,8 @@
 # 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]
@@ -83,7 +85,7 @@ class Api::V1::AccountsController < Api::BaseController
   end
 
   def check_enabled_registrations
-    forbidden if single_user_mode? || omniauth_only? || !allowed_registrations?
+    forbidden if single_user_mode? || omniauth_only? || !allowed_registrations? || captcha_enabled?
   end
 
   def allowed_registrations?
diff --git a/app/controllers/auth/registrations_controller.rb b/app/controllers/auth/registrations_controller.rb
index 6b1f3fa82..3c9b38a4b 100644
--- a/app/controllers/auth/registrations_controller.rb
+++ b/app/controllers/auth/registrations_controller.rb
@@ -2,6 +2,7 @@
 
 class Auth::RegistrationsController < Devise::RegistrationsController
   include RegistrationSpamConcern
+  include CaptchaConcern
 
   layout :determine_layout
 
@@ -15,6 +16,8 @@ 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]
 
@@ -135,4 +138,18 @@ 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!
+    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
new file mode 100644
index 000000000..5a23e59e3
--- /dev/null
+++ b/app/controllers/concerns/captcha_concern.rb
@@ -0,0 +1,66 @@
+# frozen_string_literal: true
+
+module CaptchaConcern
+  extend ActiveSupport::Concern
+  include Hcaptcha::Adapters::ViewMethods
+
+  CAPTCHA_TIMEOUT = 2.hours.freeze
+
+  included do
+    helper_method :render_captcha_if_needed
+  end
+
+  def captcha_available?
+    ENV['HCAPTCHA_SECRET_KEY'].present? && ENV['HCAPTCHA_SITE_KEY'].present?
+  end
+
+  def captcha_enabled?
+    captcha_available? && Setting.captcha_enabled
+  end
+
+  def captcha_recently_passed?
+    session[:captcha_passed_at].present? && session[:captcha_passed_at] >= CAPTCHA_TIMEOUT.ago
+  end
+
+  def captcha_required?
+    captcha_enabled? && !current_user && !(@invite.present? && @invite.valid_for_use? && !@invite.max_uses.nil?) && !captcha_recently_passed?
+  end
+
+  def clear_captcha!
+    session.delete(:captcha_passed_at)
+  end
+
+  def check_captcha!
+    return true unless captcha_required?
+
+    if verify_hcaptcha
+      session[:captcha_passed_at] = Time.now.utc
+      return true
+    else
+      if block_given?
+        message = flash[:hcaptcha_error]
+        flash.delete(:hcaptcha_error)
+        yield message
+      end
+      return false
+    end
+  end
+
+  def extend_csp_for_captcha!
+    policy = request.content_security_policy
+    return unless captcha_required? && policy.present?
+
+    %w(script_src frame_src style_src connect_src).each do |directive|
+      values = policy.send(directive)
+      values << 'https://hcaptcha.com' unless values.include?('https://hcaptcha.com') || values.include?('https:')
+      values << 'https://*.hcaptcha.com' unless values.include?('https://*.hcaptcha.com') || values.include?('https:')
+      policy.send(directive, *values)
+    end
+  end
+
+  def render_captcha_if_needed
+    return unless captcha_required?
+
+    hcaptcha_tags
+  end
+end
diff --git a/app/helpers/admin/settings_helper.rb b/app/helpers/admin/settings_helper.rb
index baf14ab25..f99a2b8c8 100644
--- a/app/helpers/admin/settings_helper.rb
+++ b/app/helpers/admin/settings_helper.rb
@@ -8,4 +8,8 @@ module Admin::SettingsHelper
     link = link_to t('admin.site_uploads.delete'), admin_site_upload_path(upload), data: { method: :delete }
     safe_join([hint, link], '<br/>'.html_safe)
   end
+
+  def captcha_available?
+    ENV['HCAPTCHA_SECRET_KEY'].present? && ENV['HCAPTCHA_SITE_KEY'].present?
+  end
 end
diff --git a/app/javascript/flavours/glitch/styles/forms.scss b/app/javascript/flavours/glitch/styles/forms.scss
index 3433abcdd..64d441fb2 100644
--- a/app/javascript/flavours/glitch/styles/forms.scss
+++ b/app/javascript/flavours/glitch/styles/forms.scss
@@ -1058,3 +1058,7 @@ code {
     display: none;
   }
 }
+
+.simple_form .h-captcha {
+  text-align: center;
+}
diff --git a/app/models/form/admin_settings.rb b/app/models/form/admin_settings.rb
index 3202d1fc2..34f14e312 100644
--- a/app/models/form/admin_settings.rb
+++ b/app/models/form/admin_settings.rb
@@ -40,6 +40,7 @@ class Form::AdminSettings
     noindex
     outgoing_spoilers
     require_invite_text
+    captcha_enabled
   ).freeze
 
   BOOLEAN_KEYS = %i(
@@ -58,6 +59,7 @@ class Form::AdminSettings
     trendable_by_default
     noindex
     require_invite_text
+    captcha_enabled
   ).freeze
 
   UPLOAD_KEYS = %i(
diff --git a/app/views/about/_registration.html.haml b/app/views/about/_registration.html.haml
index e4d614d71..5bb5d08a2 100644
--- a/app/views/about/_registration.html.haml
+++ b/app/views/about/_registration.html.haml
@@ -21,6 +21,9 @@
     .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 b9daae8f0..49b03a9e3 100644
--- a/app/views/admin/settings/edit.html.haml
+++ b/app/views/admin/settings/edit.html.haml
@@ -42,7 +42,10 @@
 
   .fields-group
     = f.input :require_invite_text, as: :boolean, wrapper: :with_label, label: t('admin.settings.registrations.require_invite_text.title'), hint: t('admin.settings.registrations.require_invite_text.desc_html'), disabled: !approved_registrations?
-  .fields-group
+
+  - 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')
 
   %hr.spacer/
 
diff --git a/app/views/auth/registrations/new.html.haml b/app/views/auth/registrations/new.html.haml
index 6981195ed..5cb558297 100644
--- a/app/views/auth/registrations/new.html.haml
+++ b/app/views/auth/registrations/new.html.haml
@@ -38,6 +38,9 @@
   .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 5cc2625fc..c96f21c92 100644
--- a/config/locales-glitch/en.yml
+++ b/config/locales-glitch/en.yml
@@ -2,6 +2,9 @@
 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, 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
       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 094209822..7d192f369 100644
--- a/config/settings.yml
+++ b/config/settings.yml
@@ -77,6 +77,7 @@ defaults: &defaults
   show_domain_blocks_rationale: 'disabled'
   outgoing_spoilers: ''
   require_invite_text: false
+  captcha_enabled: false
 
 development:
   <<: *defaults
-- 
cgit 


From 04050fbd46d7758873b33dd6f5648cc37183b078 Mon Sep 17 00:00:00 2001
From: Claire <claire.github-309c@sitedethib.com>
Date: Mon, 24 Jan 2022 21:29:50 +0100
Subject: Please CodeClimate

---
 Gemfile                                     | 2 +-
 app/controllers/concerns/captcha_concern.rb | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

(limited to 'app/controllers/concerns')

diff --git a/Gemfile b/Gemfile
index 282ab65e4..67c50d19f 100644
--- a/Gemfile
+++ b/Gemfile
@@ -157,4 +157,4 @@ gem 'connection_pool', require: false
 
 gem 'xorcist', '~> 1.1'
 
-gem "hcaptcha", "~> 7.1"
+gem 'hcaptcha', '~> 7.1'
diff --git a/app/controllers/concerns/captcha_concern.rb b/app/controllers/concerns/captcha_concern.rb
index 5a23e59e3..5bc4ba920 100644
--- a/app/controllers/concerns/captcha_concern.rb
+++ b/app/controllers/concerns/captcha_concern.rb
@@ -35,14 +35,14 @@ module CaptchaConcern
 
     if verify_hcaptcha
       session[:captcha_passed_at] = Time.now.utc
-      return true
+      true
     else
       if block_given?
         message = flash[:hcaptcha_error]
         flash.delete(:hcaptcha_error)
         yield message
       end
-      return false
+      false
     end
   end
 
-- 
cgit 


From bf351d72af76e80a29674770d9991de955f95d34 Mon Sep 17 00:00:00 2001
From: Claire <claire.github-309c@sitedethib.com>
Date: Mon, 24 Jan 2022 22:12:57 +0100
Subject: Disable captcha if registrations are disabled for various reasons

---
 app/controllers/concerns/captcha_concern.rb | 2 ++
 1 file changed, 2 insertions(+)

(limited to 'app/controllers/concerns')

diff --git a/app/controllers/concerns/captcha_concern.rb b/app/controllers/concerns/captcha_concern.rb
index 5bc4ba920..4a942c988 100644
--- a/app/controllers/concerns/captcha_concern.rb
+++ b/app/controllers/concerns/captcha_concern.rb
@@ -23,6 +23,8 @@ module CaptchaConcern
   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?
   end
 
-- 
cgit 


From 0fb907441c827cadc767641b29d5d2c0e554f7a4 Mon Sep 17 00:00:00 2001
From: Claire <claire.github-309c@sitedethib.com>
Date: Tue, 25 Jan 2022 22:37:12 +0100
Subject: 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.
---
 app/controllers/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 +-
 app/views/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

(limited to 'app/controllers/concerns')

diff --git a/app/controllers/auth/confirmations_controller.rb b/app/controllers/auth/confirmations_controller.rb
index 0b5a2f3c9..e9a646f91 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 4a942c988..02069d205 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 34f14e312..7abb0d6c6 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 94cc3ffe3..d343cca20 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 49b03a9e3..fc042f845 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 000000000..850bc1479
--- /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 9bd66c969..6ad5a5365 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 65dd7ad63..d0eeda1e8 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 7d192f369..b5437caee 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
-- 
cgit 


From b7cf3941b3783220e6b3bc9a6d3975ceecdc64cb Mon Sep 17 00:00:00 2001
From: Claire <claire.github-309c@sitedethib.com>
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 ---
 config/locales-glitch/en.yml                     | 15 +++----------
 config/settings.yml                              |  2 +-
 spec/views/about/show.html.haml_spec.rb          |  1 -
 14 files changed, 15 insertions(+), 84 deletions(-)

(limited to 'app/controllers/concerns')

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
-- 
cgit 


From f5639e1cbe0eb9de88a8f4b1c82833fdcffe62b8 Mon Sep 17 00:00:00 2001
From: Claire <claire.github-309c@sitedethib.com>
Date: Fri, 28 Jan 2022 14:24:37 +0100
Subject: Change public profile pages to be disabled for unconfirmed users
 (#17385)

Fixes #17382

Note that unconfirmed and unapproved accounts can still be searched for
and their (empty) account retrieved using the REST API.
---
 app/controllers/concerns/account_owned_concern.rb  |  5 +++++
 .../concerns/account_controller_concern_spec.rb    | 23 ++++++++++++++++++++++
 2 files changed, 28 insertions(+)

(limited to 'app/controllers/concerns')

diff --git a/app/controllers/concerns/account_owned_concern.rb b/app/controllers/concerns/account_owned_concern.rb
index 62e379846..25149d03f 100644
--- a/app/controllers/concerns/account_owned_concern.rb
+++ b/app/controllers/concerns/account_owned_concern.rb
@@ -8,6 +8,7 @@ module AccountOwnedConcern
     before_action :set_account, if: :account_required?
     before_action :check_account_approval, if: :account_required?
     before_action :check_account_suspension, if: :account_required?
+    before_action :check_account_confirmation, if: :account_required?
   end
 
   private
@@ -28,6 +29,10 @@ module AccountOwnedConcern
     not_found if @account.local? && @account.user_pending?
   end
 
+  def check_account_confirmation
+    not_found if @account.local? && !@account.user_confirmed?
+  end
+
   def check_account_suspension
     if @account.suspended_permanently?
       permanent_suspension_response
diff --git a/spec/controllers/concerns/account_controller_concern_spec.rb b/spec/controllers/concerns/account_controller_concern_spec.rb
index 835645414..99975f4c4 100644
--- a/spec/controllers/concerns/account_controller_concern_spec.rb
+++ b/spec/controllers/concerns/account_controller_concern_spec.rb
@@ -11,10 +11,33 @@ describe ApplicationController, type: :controller do
     end
   end
 
+  around do |example|
+    registrations_mode = Setting.registrations_mode
+    example.run
+    Setting.registrations_mode = registrations_mode
+  end
+
   before do
     routes.draw { get 'success' => 'anonymous#success' }
   end
 
+  context 'when account is unconfirmed' do
+    it 'returns http not found' do
+      account = Fabricate(:user, confirmed_at: nil).account
+      get 'success', params: { account_username: account.username }
+      expect(response).to have_http_status(404)
+    end
+  end
+
+  context 'when account is not approved' do
+    it 'returns http not found' do
+      Setting.registrations_mode = 'approved'
+      account = Fabricate(:user, approved: false).account
+      get 'success', params: { account_username: account.username }
+      expect(response).to have_http_status(404)
+    end
+  end
+
   context 'when account is suspended' do
     it 'returns http gone' do
       account = Fabricate(:account, suspended: true)
-- 
cgit