Merge pull request #3150 from ClearlyClaire/glitch-soc/merge-4.4

Merge upstream changes up to 208cb8276a into stable-4.4

app/javascript/flavours/glitch/features/picture_in_picture/components/footer.tsx

@@ -24,6 +24,7 @@ import { openModal } from 'flavours/glitch/actions/modal';
 import { IconButton } from 'flavours/glitch/components/icon_button';
 import { useIdentity } from 'flavours/glitch/identity_context';
 import { me } from 'flavours/glitch/initial_state';
+import type { Account } from 'flavours/glitch/models/account';
 import type { Status } from 'flavours/glitch/models/status';
 import { makeGetStatus } from 'flavours/glitch/selectors';
 import type { RootState } from 'flavours/glitch/store';
@@ -69,10 +70,7 @@ export const Footer: React.FC<{
   const dispatch = useAppDispatch();
   const getStatus = useMemo(() => makeGetStatus(), []) as GetStatusSelector;
   const status = useAppSelector((state) => getStatus(state, { id: statusId }));
-  const accountId = status?.get('account') as string | undefined;
-  const account = useAppSelector((state) =>
-    accountId ? state.accounts.get(accountId) : undefined,
-  );
+  const account = status?.get('account') as Account | undefined;
   const askReplyConfirmation = useAppSelector(
     (state) => (state.compose.get('text') as string).trim().length !== 0,
   );

app/javascript/mastodon/features/picture_in_picture/components/footer.tsx

@@ -21,6 +21,7 @@ import { openModal } from 'mastodon/actions/modal';
 import { IconButton } from 'mastodon/components/icon_button';
 import { useIdentity } from 'mastodon/identity_context';
 import { me } from 'mastodon/initial_state';
+import type { Account } from 'mastodon/models/account';
 import type { Status } from 'mastodon/models/status';
 import { makeGetStatus } from 'mastodon/selectors';
 import type { RootState } from 'mastodon/store';
@@ -66,10 +67,7 @@ export const Footer: React.FC<{
   const dispatch = useAppDispatch();
   const getStatus = useMemo(() => makeGetStatus(), []) as GetStatusSelector;
   const status = useAppSelector((state) => getStatus(state, { id: statusId }));
-  const accountId = status?.get('account') as string | undefined;
-  const account = useAppSelector((state) =>
-    accountId ? state.accounts.get(accountId) : undefined,
-  );
+  const account = status?.get('account') as Account | undefined;
   const askReplyConfirmation = useAppSelector(
     (state) => (state.compose.get('text') as string).trim().length !== 0,
   );

app/lib/activitypub/activity/create.rb

@@ -206,8 +206,8 @@ class ActivityPub::Activity::Create < ActivityPub::Activity
     @quote.save
 
     embedded_quote = safe_prefetched_embed(@account, @status_parser.quoted_object, @json['context'])
-    ActivityPub::VerifyQuoteService.new.call(@quote, fetchable_quoted_uri: @quote_uri, prefetched_quoted_object: embedded_quote, request_id: @options[:request_id])
-  rescue Mastodon::UnexpectedResponseError, *Mastodon::HTTP_CONNECTION_ERRORS
+    ActivityPub::VerifyQuoteService.new.call(@quote, fetchable_quoted_uri: @quote_uri, prefetched_quoted_object: embedded_quote, request_id: @options[:request_id], depth: @options[:depth])
+  rescue Mastodon::RecursionLimitExceededError, Mastodon::UnexpectedResponseError, *Mastodon::HTTP_CONNECTION_ERRORS
     ActivityPub::RefetchAndVerifyQuoteWorker.perform_in(rand(30..600).seconds, @quote.id, @quote_uri, { 'request_id' => @options[:request_id] })
   end
 

app/lib/activitypub/parser/status_parser.rb

@@ -154,9 +154,6 @@ class ActivityPub::Parser::StatusParser
     # Remove the special-meaning actor URI
     allowed_actors.delete(@options[:actor_uri])
 
-    # Tagged users are always allowed, so remove them
-    allowed_actors -= as_array(@object['tag']).filter_map { |tag| tag['href'] if equals_or_includes?(tag['type'], 'Mention') }
-
     # Any unrecognized actor is marked as unknown
     flags |= Status::QUOTE_APPROVAL_POLICY_FLAGS[:unknown] unless allowed_actors.empty?
 

app/lib/status_reach_finder.rb

@@ -30,8 +30,10 @@ class StatusReachFinder
       [
         replied_to_account_id,
         reblog_of_account_id,
+        quote_of_account_id,
         mentioned_account_ids,
         reblogs_account_ids,
+        quotes_account_ids,
         favourites_account_ids,
         replies_account_ids,
       ].tap do |arr|
@@ -46,6 +48,10 @@ class StatusReachFinder
     @status.in_reply_to_account_id if distributable?
   end
 
+  def quote_of_account_id
+    @status.quote&.quoted_account_id
+  end
+
   def reblog_of_account_id
     @status.reblog.account_id if @status.reblog?
   end
@@ -54,6 +60,11 @@ class StatusReachFinder
     @status.mentions.pluck(:account_id)
   end
 
+  # Beware: Quotes can be created without the author having had access to the status
+  def quotes_account_ids
+    @status.quotes.pluck(:account_id) if distributable? || unsafe?
+  end
+
   # Beware: Reblogs can be created without the author having had access to the status
   def reblogs_account_ids
     @status.reblogs.rewhere(deleted_at: [nil, @status.deleted_at]).pluck(:account_id) if distributable? || unsafe?

app/models/account_suggestions/friends_of_friends_source.rb

@@ -26,6 +26,7 @@ class AccountSuggestions::FriendsOfFriendsSource < AccountSuggestions::Source
         AND NOT EXISTS (SELECT 1 FROM mutes m WHERE m.target_account_id = follows.target_account_id AND m.account_id = :id)
         AND (accounts.domain IS NULL OR NOT EXISTS (SELECT 1 FROM account_domain_blocks b WHERE b.account_id = :id AND b.domain = accounts.domain))
         AND NOT EXISTS (SELECT 1 FROM follows f WHERE f.target_account_id = follows.target_account_id AND f.account_id = :id)
+        AND NOT EXISTS (SELECT 1 FROM follow_requests f WHERE f.target_account_id = follows.target_account_id AND f.account_id = :id)
         AND follows.target_account_id <> :id
         AND accounts.discoverable
         AND accounts.suspended_at IS NULL

app/models/status.rb

@@ -86,6 +86,7 @@ class Status < ApplicationRecord
   has_many :mentions, dependent: :destroy, inverse_of: :status
   has_many :mentioned_accounts, through: :mentions, source: :account, class_name: 'Account'
   has_many :media_attachments, dependent: :nullify
+  has_many :quotes, foreign_key: 'quoted_status_id', inverse_of: :quoted_status, dependent: :nullify
 
   # The `dependent` option is enabled by the initial `mentions` association declaration
   has_many :active_mentions, -> { active }, class_name: 'Mention', inverse_of: :status # rubocop:disable Rails/HasManyOrHasOneDependent

app/models/trends/statuses.rb

@@ -90,14 +90,28 @@ class Trends::Statuses < Trends::Base
 
   def eligible?(status)
     status.created_at.past? &&
-      status.public_visibility? &&
-      status.account.discoverable? &&
-      !status.account.silenced? &&
-      !status.account.sensitized? &&
-      (status.spoiler_text.blank? || Setting.trending_status_cw) &&
-      !status.sensitive? &&
+      opted_into_trends?(status) &&
+      !sensitive_content?(status) &&
       !status.reply? &&
-      valid_locale?(status.language)
+      valid_locale?(status.language) &&
+      (status.quote.nil? || trendable_quote?(status.quote))
+  end
+
+  def opted_into_trends?(status)
+    status.public_visibility? &&
+      status.account.discoverable? &&
+      !status.account.silenced?
+  end
+
+  def sensitive_content?(status)
+    status.account.sensitized? || (status.spoiler_text.present? && !Setting.trending_status_cw) || status.sensitive?
+  end
+
+  def trendable_quote?(quote)
+    quote.acceptable? &&
+      quote.quoted_status.present? &&
+      opted_into_trends?(quote.quoted_status) &&
+      !sensitive_content?(quote.quoted_status)
   end
 
   def calculate_scores(statuses, at_time)

app/services/activitypub/fetch_remote_status_service.rb

@@ -8,9 +8,10 @@ class ActivityPub::FetchRemoteStatusService < BaseService
   DISCOVERIES_PER_REQUEST = 1000
 
   # Should be called when uri has already been checked for locality
-  def call(uri, prefetched_body: nil, on_behalf_of: nil, expected_actor_uri: nil, request_id: nil)
+  def call(uri, prefetched_body: nil, on_behalf_of: nil, expected_actor_uri: nil, request_id: nil, depth: nil)
     return if domain_not_allowed?(uri)
 
+    @depth = depth || 0
     @request_id = request_id || "#{Time.now.utc.to_i}-status-#{uri}"
     @json = if prefetched_body.nil?
               fetch_status(uri, true, on_behalf_of)
@@ -52,7 +53,7 @@ class ActivityPub::FetchRemoteStatusService < BaseService
       return nil if discoveries > DISCOVERIES_PER_REQUEST
     end
 
-    ActivityPub::Activity.factory(activity_json, actor, request_id: @request_id).perform
+    ActivityPub::Activity.factory(activity_json, actor, request_id: @request_id, depth: @depth).perform
   end
 
   private

app/services/activitypub/verify_quote_service.rb

@@ -3,9 +3,12 @@
 class ActivityPub::VerifyQuoteService < BaseService
   include JsonLdHelper
 
+  MAX_SYNCHRONOUS_DEPTH = 2
+
   # Optionally fetch quoted post, and verify the quote is authorized
-  def call(quote, fetchable_quoted_uri: nil, prefetched_quoted_object: nil, prefetched_approval: nil, request_id: nil)
+  def call(quote, fetchable_quoted_uri: nil, prefetched_quoted_object: nil, prefetched_approval: nil, request_id: nil, depth: nil)
     @request_id = request_id
+    @depth = depth || 0
     @quote = quote
     @fetching_error = nil
 
@@ -42,14 +45,7 @@ class ActivityPub::VerifyQuoteService < BaseService
       true
     end
 
-    # Always allow someone to quote posts in which they are mentioned
-    if @quote.quoted_status.active_mentions.exists?(mentions: { account_id: @quote.account_id })
-      @quote.accept!
-
-      true
-    else
-      false
-    end
+    false
   end
 
   def fetch_approval_object(uri, prefetched_body: nil)
@@ -72,10 +68,12 @@ class ActivityPub::VerifyQuoteService < BaseService
     return if uri.nil? || @quote.quoted_status.present?
 
     status = ActivityPub::TagManager.instance.uri_to_resource(uri, Status)
-    status ||= ActivityPub::FetchRemoteStatusService.new.call(uri, on_behalf_of: @quote.account.followers.local.first, prefetched_body:, request_id: @request_id)
+    raise Mastodon::RecursionLimitExceededError if @depth > MAX_SYNCHRONOUS_DEPTH && status.nil?
+
+    status ||= ActivityPub::FetchRemoteStatusService.new.call(uri, on_behalf_of: @quote.account.followers.local.first, prefetched_body:, request_id: @request_id, depth: @depth + 1)
 
     @quote.update(quoted_status: status) if status.present?
-  rescue Mastodon::UnexpectedResponseError, *Mastodon::HTTP_CONNECTION_ERRORS => e
+  rescue Mastodon::RecursionLimitExceededError, Mastodon::UnexpectedResponseError, *Mastodon::HTTP_CONNECTION_ERRORS => e
     @fetching_error = e
   end
 
@@ -90,7 +88,7 @@ class ActivityPub::VerifyQuoteService < BaseService
     # It's not safe to fetch if the inlined object is cross-origin or doesn't match expectations
     return if object['id'] != uri || non_matching_uri_hosts?(@quote.approval_uri, object['id'])
 
-    status = ActivityPub::FetchRemoteStatusService.new.call(object['id'], prefetched_body: object, on_behalf_of: @quote.account.followers.local.first, request_id: @request_id)
+    status = ActivityPub::FetchRemoteStatusService.new.call(object['id'], prefetched_body: object, on_behalf_of: @quote.account.followers.local.first, request_id: @request_id, depth: @depth)
 
     if status.present?
       @quote.update(quoted_status: status)

config/locales/en.yml

@@ -1876,8 +1876,8 @@ en:
       ownership: Someone else's post cannot be pinned
       reblog: A boost cannot be pinned
     quote_policies:
-      followers: Followers and mentioned users
-      nobody: Only mentioned users
+      followers: Only your followers
+      nobody: Nobody
       public: Everyone
     title: '%{name}: "%{quote}"'
     visibilities:

config/locales/simple_form.en.yml

@@ -56,7 +56,7 @@ en:
         scopes: Which APIs the application will be allowed to access. If you select a top-level scope, you don't need to select individual ones.
         setting_aggregate_reblogs: Do not show new boosts for posts that have been recently boosted (only affects newly-received boosts)
         setting_always_send_emails: Normally e-mail notifications won't be sent when you are actively using Mastodon
-        setting_default_quote_policy: Mentioned users are always allowed to quote. This setting will only take effect for posts created with the next Mastodon version, but you can select your preference in preparation
+        setting_default_quote_policy: This setting will only take effect for posts created with the next Mastodon version, but you can select your preference in preparation.
         setting_default_sensitive: Sensitive media is hidden by default and can be revealed with a click
         setting_display_media_default: Hide media marked as sensitive
         setting_display_media_hide_all: Always hide media

lib/exceptions.rb

@@ -14,6 +14,7 @@ module Mastodon
   class InvalidParameterError < Error; end
   class SignatureVerificationError < Error; end
   class MalformedHeaderError < Error; end
+  class RecursionLimitExceededError < Error; end
 
   class UnexpectedResponseError < Error
     attr_reader :response

spec/models/account_suggestions/friends_of_friends_source_spec.rb

@@ -16,10 +16,12 @@ RSpec.describe AccountSuggestions::FriendsOfFriendsSource do
     let!(:jerk) { Fabricate(:account, discoverable: true, hide_collections: false) }
     let!(:larry) { Fabricate(:account, discoverable: true, hide_collections: false) }
     let!(:morty) { Fabricate(:account, discoverable: true, hide_collections: false, memorial: true) }
+    let!(:joyce) { Fabricate(:account, discoverable: true, hide_collections: false) }
 
     context 'with follows and blocks' do
       before do
         bob.block!(jerk)
+        bob.request_follow!(joyce)
         FollowRecommendationMute.create!(account: bob, target_account: neil)
 
         # bob follows eugen, alice and larry
@@ -28,8 +30,8 @@ RSpec.describe AccountSuggestions::FriendsOfFriendsSource do
         # alice follows eve and mallory
         [john, mallory].each { |account| alice.follow!(account) }
 
-        # eugen follows eve, john, jerk, larry, neil and morty
-        [eve, mallory, jerk, larry, neil, morty].each { |account| eugen.follow!(account) }
+        # eugen follows eve, john, jerk, larry, neil, morty and joyce
+        [eve, mallory, jerk, larry, neil, morty, joyce].each { |account| eugen.follow!(account) }
       end
 
       it 'returns eligible accounts', :aggregate_failures do
@@ -55,6 +57,9 @@ RSpec.describe AccountSuggestions::FriendsOfFriendsSource do
 
         # morty is not included because his account is in memoriam
         expect(results).to_not include([morty.id, :friends_of_friends])
+
+        # joyce is not included because there is already a pending follow request
+        expect(results).to_not include([joyce.id, :friends_of_friends])
       end
     end
 

spec/models/trends/statuses_spec.rb

@@ -111,12 +111,16 @@ RSpec.describe Trends::Statuses do
     let!(:yesterday) { today - 1.day }
 
     let!(:status_foo) { Fabricate(:status, text: 'Foo', language: 'en', trendable: true, created_at: yesterday) }
-    let!(:status_bar) { Fabricate(:status, text: 'Bar', language: 'en', trendable: true, created_at: today) }
+    let!(:status_bar) { Fabricate(:status, text: 'Bar', language: 'en', trendable: true, created_at: today, quote: Quote.new(state: :accepted, quoted_status: status_foo)) }
     let!(:status_baz) { Fabricate(:status, text: 'Baz', language: 'en', trendable: true, created_at: today) }
+    let!(:untrendable) { Fabricate(:status, text: 'Untrendable', language: 'en', trendable: true, visibility: :unlisted) }
+    let!(:untrendable_quote) { Fabricate(:status, text: 'Untrendable quote!', language: 'en', trendable: true, created_at: today, quote: Quote.new(state: :accepted, quoted_status: untrendable)) }
 
     before do
       default_threshold_value.times { reblog(status_foo, today) }
       default_threshold_value.times { reblog(status_bar, today) }
+      default_threshold_value.times { reblog(untrendable, today) }
+      default_threshold_value.times { reblog(untrendable_quote, today) }
       (default_threshold_value - 1).times { reblog(status_baz, today) }
     end
 
@@ -129,7 +133,7 @@ RSpec.describe Trends::Statuses do
         results = subject.query.limit(10).to_a
 
         expect(results).to eq [status_bar, status_foo]
-        expect(results).to_not include(status_baz)
+        expect(results).to_not include(status_baz, untrendable, untrendable_quote)
       end
     end
 

spec/services/activitypub/verify_quote_service_spec.rb

@@ -267,9 +267,9 @@ RSpec.describe ActivityPub::VerifyQuoteService do
         quoted_status.mentions << Mention.new(account: account)
       end
 
-      it 'updates the status' do
+      it 'does not the status' do
         expect { subject.call(quote) }
-          .to change(quote, :state).to('accepted')
+          .to_not change(quote, :state).from('pending')
       end
     end
   end