Merge pull request #2590 from ClearlyClaire/glitch-soc/merge-upstream

Merge upstream changes up to bf153b384b

.rubocop_todo.yml

@@ -121,10 +121,6 @@ Style/GlobalStdStream:
 # Configuration parameters: MinBodyLength, AllowConsecutiveConditionals.
 Style/GuardClause:
   Exclude:
-    - 'app/controllers/admin/confirmations_controller.rb'
-    - 'app/controllers/auth/confirmations_controller.rb'
-    - 'app/controllers/auth/passwords_controller.rb'
-    - 'app/controllers/settings/two_factor_authentication/webauthn_credentials_controller.rb'
     - 'app/lib/activitypub/activity/block.rb'
     - 'app/lib/request.rb'
     - 'app/lib/request_pool.rb'

Gemfile

@@ -123,7 +123,7 @@ group :test do
   gem 'database_cleaner-active_record'
 
   # Used to mock environment variables
-  gem 'climate_control', '~> 0.2'
+  gem 'climate_control'
 
   # Generating fake data for specs
   gem 'faker', '~> 3.2'

Gemfile.lock

@@ -185,7 +185,7 @@ GEM
       elasticsearch (>= 7.12.0, < 7.14.0)
       elasticsearch-dsl
     chunky_png (1.4.0)
-    climate_control (0.2.0)
+    climate_control (1.2.0)
     cocoon (1.2.15)
     color_diff (0.1)
     concurrent-ruby (1.2.3)
@@ -746,8 +746,8 @@ GEM
     temple (0.10.3)
     terminal-table (3.0.2)
       unicode-display_width (>= 1.1.1, < 3)
-    terrapin (0.6.0)
+    terrapin (1.0.1)
-      climate_control (>= 0.0.3, < 1.0)
+      climate_control
     test-prof (1.3.1)
     thor (1.3.0)
     tilt (2.3.0)
@@ -836,7 +836,7 @@ DEPENDENCIES
   capybara (~> 3.39)
   charlock_holmes (~> 0.7.7)
   chewy (~> 7.3)
-  climate_control (~> 0.2)
+  climate_control
   cocoon (~> 1.2)
   color_diff (~> 0.1)
   concurrent-ruby

app/controllers/admin/confirmations_controller.rb

@@ -3,7 +3,7 @@
 module Admin
   class ConfirmationsController < BaseController
     before_action :set_user
-    before_action :check_confirmation, only: [:resend]
+    before_action :redirect_confirmed_user, only: [:resend], if: :user_confirmed?
 
     def create
       authorize @user, :confirm?
@@ -25,11 +25,13 @@ module Admin
 
     private
 
-    def check_confirmation
+    def redirect_confirmed_user
-      if @user.confirmed?
+      flash[:error] = I18n.t('admin.accounts.resend_confirmation.already_confirmed')
-        flash[:error] = I18n.t('admin.accounts.resend_confirmation.already_confirmed')
+      redirect_to admin_accounts_path
-        redirect_to admin_accounts_path
+    end
-      end
+
+    def user_confirmed?
+      @user.confirmed?
     end
   end
 end

app/controllers/admin/export_domain_blocks_controller.rb

@@ -49,7 +49,7 @@ module Admin
         next
       end
 
-      @warning_domains = Instance.where(domain: @domain_blocks.map(&:domain)).where('EXISTS (SELECT 1 FROM follows JOIN accounts ON follows.account_id = accounts.id OR follows.target_account_id = accounts.id WHERE accounts.domain = instances.domain)').pluck(:domain)
+      @warning_domains = instances_from_imported_blocks.pluck(:domain)
     rescue ActionController::ParameterMissing
       flash.now[:alert] = I18n.t('admin.export_domain_blocks.no_file')
       set_dummy_import!
@@ -58,6 +58,10 @@ module Admin
 
     private
 
+    def instances_from_imported_blocks
+      Instance.with_domain_follows(@domain_blocks.map(&:domain))
+    end
+
     def export_filename
       'domain_blocks.csv'
     end

app/controllers/auth/confirmations_controller.rb

@@ -8,7 +8,7 @@ class Auth::ConfirmationsController < Devise::ConfirmationsController
   before_action :set_body_classes
   before_action :set_pack
   before_action :set_confirmation_user!, only: [:show, :confirm_captcha]
-  before_action :require_unconfirmed!
+  before_action :redirect_confirmed_user, if: :signed_in_confirmed_user?
 
   before_action :extend_csp_for_captcha!, only: [:show, :confirm_captcha]
   before_action :require_captcha_if_needed!, only: [:show]
@@ -70,10 +70,12 @@ class Auth::ConfirmationsController < Devise::ConfirmationsController
     use_pack 'auth'
   end
 
-  def require_unconfirmed!
+  def redirect_confirmed_user
-    if user_signed_in? && current_user.confirmed? && current_user.unconfirmed_email.blank?
+    redirect_to(current_user.approved? ? root_path : edit_user_registration_path)
-      redirect_to(current_user.approved? ? root_path : edit_user_registration_path)
+  end
-    end
+
+  def signed_in_confirmed_user?
+    user_signed_in? && current_user.confirmed? && current_user.unconfirmed_email.blank?
   end
 
   def set_body_classes

app/controllers/auth/passwords_controller.rb

@@ -2,7 +2,7 @@
 
 class Auth::PasswordsController < Devise::PasswordsController
   skip_before_action :check_self_destruct!
-  before_action :check_validity_of_reset_password_token, only: :edit
+  before_action :redirect_invalid_reset_token, only: :edit, unless: :reset_password_token_is_valid?
   before_action :set_pack
   before_action :set_body_classes
 
@@ -20,11 +20,9 @@ class Auth::PasswordsController < Devise::PasswordsController
 
   private
 
-  def check_validity_of_reset_password_token
+  def redirect_invalid_reset_token
-    unless reset_password_token_is_valid?
+    flash[:error] = I18n.t('auth.invalid_reset_password_token')
-      flash[:error] = I18n.t('auth.invalid_reset_password_token')
+    redirect_to new_password_path(resource_name)
-      redirect_to new_password_path(resource_name)
-    end
   end
 
   def set_body_classes

app/controllers/settings/two_factor_authentication/webauthn_credentials_controller.rb

@@ -6,8 +6,8 @@ module Settings
       skip_before_action :check_self_destruct!
       skip_before_action :require_functional!
 
-      before_action :require_otp_enabled
+      before_action :redirect_invalid_otp, unless: -> { current_user.otp_enabled? }
-      before_action :require_webauthn_enabled, only: [:index, :destroy]
+      before_action :redirect_invalid_webauthn, only: [:index, :destroy], unless: -> { current_user.webauthn_enabled? }
 
       def index; end
       def new; end
@@ -89,18 +89,14 @@ module Settings
         use_pack 'auth'
       end
 
-      def require_otp_enabled
+      def redirect_invalid_otp
-        unless current_user.otp_enabled?
+        flash[:error] = t('webauthn_credentials.otp_required')
-          flash[:error] = t('webauthn_credentials.otp_required')
+        redirect_to settings_two_factor_authentication_methods_path
-          redirect_to settings_two_factor_authentication_methods_path
-        end
       end
 
-      def require_webauthn_enabled
+      def redirect_invalid_webauthn
-        unless current_user.webauthn_enabled?
+        flash[:error] = t('webauthn_credentials.not_enabled')
-          flash[:error] = t('webauthn_credentials.not_enabled')
+        redirect_to settings_two_factor_authentication_methods_path
-          redirect_to settings_two_factor_authentication_methods_path
-        end
       end
     end
   end

app/models/instance.rb

@@ -25,11 +25,25 @@ class Instance < ApplicationRecord
   scope :matches_domain, ->(value) { where(arel_table[:domain].matches("%#{value}%")) }
   scope :domain_starts_with, ->(value) { where(arel_table[:domain].matches("#{sanitize_sql_like(value)}%", false, true)) }
   scope :by_domain_and_subdomains, ->(domain) { where("reverse('.' || domain) LIKE reverse(?)", "%.#{domain}") }
+  scope :with_domain_follows, ->(domains) { where(domain: domains).where(domain_account_follows) }
 
   def self.refresh
     Scenic.database.refresh_materialized_view(table_name, concurrently: true, cascade: false)
   end
 
+  def self.domain_account_follows
+    Arel.sql(
+      <<~SQL.squish
+        EXISTS (
+          SELECT 1
+          FROM follows
+          JOIN accounts ON follows.account_id = accounts.id OR follows.target_account_id = accounts.id
+          WHERE accounts.domain = instances.domain
+        )
+      SQL
+    )
+  end
+
   def readonly?
     true
   end

lib/mastodon/cli/maintenance.rb

@@ -244,10 +244,10 @@ module Mastodon::CLI
       end
 
       say 'Reindexing textual indexes on accounts…'
-      database_connection.execute('REINDEX INDEX search_index;')
+      rebuild_index(:search_index)
-      database_connection.execute('REINDEX INDEX index_accounts_on_uri;')
+      rebuild_index(:index_accounts_on_uri)
-      database_connection.execute('REINDEX INDEX index_accounts_on_url;')
+      rebuild_index(:index_accounts_on_url)
-      database_connection.execute('REINDEX INDEX index_accounts_on_domain_and_id;') if migrator_version >= 2023_05_24_190515
+      rebuild_index(:index_accounts_on_domain_and_id) if migrator_version >= 2023_05_24_190515
     end
 
     def deduplicate_users!
@@ -274,7 +274,7 @@ module Mastodon::CLI
         database_connection.add_index :users, ['reset_password_token'], name: 'index_users_on_reset_password_token', unique: true, where: 'reset_password_token IS NOT NULL', opclass: :text_pattern_ops
       end
 
-      database_connection.execute('REINDEX INDEX index_users_on_unconfirmed_email;') if migrator_version >= 2023_07_02_151753
+      rebuild_index(:index_users_on_unconfirmed_email) if migrator_version >= 2023_07_02_151753
     end
 
     def deduplicate_users_process_email
@@ -735,5 +735,9 @@ module Mastodon::CLI
     def db_table_exists?(table)
       database_connection.table_exists?(table)
     end
+
+    def rebuild_index(name)
+      database_connection.execute("REINDEX INDEX #{name}")
+    end
   end
 end

spec/models/instance_spec.rb

@@ -0,0 +1,104 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe Instance do
+  describe 'Scopes' do
+    before { described_class.refresh }
+
+    describe '#searchable' do
+      let(:expected_domain) { 'host.example' }
+      let(:blocked_domain) { 'other.example' }
+
+      before do
+        Fabricate :account, domain: expected_domain
+        Fabricate :account, domain: blocked_domain
+        Fabricate :domain_block, domain: blocked_domain
+      end
+
+      it 'returns records not domain blocked' do
+        results = described_class.searchable.pluck(:domain)
+
+        expect(results)
+          .to include(expected_domain)
+          .and not_include(blocked_domain)
+      end
+    end
+
+    describe '#matches_domain' do
+      let(:host_domain) { 'host.example.com' }
+      let(:host_under_domain) { 'host_under.example.com' }
+      let(:other_domain) { 'other.example' }
+
+      before do
+        Fabricate :account, domain: host_domain
+        Fabricate :account, domain: host_under_domain
+        Fabricate :account, domain: other_domain
+      end
+
+      it 'returns matching records' do
+        expect(described_class.matches_domain('host.exa').pluck(:domain))
+          .to include(host_domain)
+          .and not_include(other_domain)
+
+        expect(described_class.matches_domain('ple.com').pluck(:domain))
+          .to include(host_domain)
+          .and not_include(other_domain)
+
+        expect(described_class.matches_domain('example').pluck(:domain))
+          .to include(host_domain)
+          .and include(other_domain)
+
+        expect(described_class.matches_domain('host_').pluck(:domain)) # Preserve SQL wildcards
+          .to include(host_domain)
+          .and include(host_under_domain)
+          .and not_include(other_domain)
+      end
+    end
+
+    describe '#by_domain_and_subdomains' do
+      let(:exact_match_domain) { 'example.com' }
+      let(:subdomain_domain) { 'foo.example.com' }
+      let(:partial_domain) { 'grexample.com' }
+
+      before do
+        Fabricate(:account, domain: exact_match_domain)
+        Fabricate(:account, domain: subdomain_domain)
+        Fabricate(:account, domain: partial_domain)
+      end
+
+      it 'returns matching instances' do
+        results = described_class.by_domain_and_subdomains('example.com').pluck(:domain)
+
+        expect(results)
+          .to include(exact_match_domain)
+          .and include(subdomain_domain)
+          .and not_include(partial_domain)
+      end
+    end
+
+    describe '#with_domain_follows' do
+      let(:example_domain) { 'example.host' }
+      let(:other_domain) { 'other.host' }
+      let(:none_domain) { 'none.host' }
+
+      before do
+        example_account = Fabricate(:account, domain: example_domain)
+        other_account = Fabricate(:account, domain: other_domain)
+        Fabricate(:account, domain: none_domain)
+
+        Fabricate :follow, account: example_account
+        Fabricate :follow, target_account: other_account
+      end
+
+      it 'returns instances with domain accounts that have follows' do
+        results = described_class.with_domain_follows(['example.host', 'other.host', 'none.host']).pluck(:domain)
+
+        expect(results)
+          .to include(example_domain)
+          .and include(other_domain)
+          .and not_include(none_domain)
+      end
+    end
+  end
+end