From 5a88b7f6835d12935be27bb6d4ee21f7201ca2f7 Mon Sep 17 00:00:00 2001 From: Claire Date: Fri, 25 Jul 2025 14:35:24 +0200 Subject: [PATCH] Add experimental basic quote post authoring (#35355) --- app/controllers/api/v1/statuses_controller.rb | 13 ++ app/lib/activitypub/activity.rb | 18 +++ app/lib/activitypub/activity/accept.rb | 26 ++++ app/lib/activitypub/activity/reject.rb | 17 +++ app/lib/activitypub/case_transform.rb | 4 +- app/lib/status_cache_hydrator.rb | 2 + .../status/interaction_policy_concern.rb | 63 +++++++++ app/models/status.rb | 8 +- app/policies/status_policy.rb | 5 + .../activitypub/note_serializer.rb | 25 +++- app/serializers/rest/status_serializer.rb | 9 ++ app/services/post_status_service.rb | 13 +- .../activitypub/quote_request_worker.rb | 22 +++ .../status_update_distribution_worker.rb | 4 +- config/locales/en.yml | 1 + spec/lib/activitypub/activity/accept_spec.rb | 125 +++++++++++++++++- spec/lib/activitypub/activity/reject_spec.rb | 22 +++ spec/lib/status_cache_hydrator_spec.rb | 12 ++ .../status/interaction_policy_concern_spec.rb | 42 ++++++ spec/policies/status_policy_spec.rb | 86 ++++++++++++ spec/requests/api/v1/statuses_spec.rb | 21 +++ .../activitypub/note_serializer_spec.rb | 16 +++ spec/services/post_status_service_spec.rb | 8 ++ .../activitypub/quote_request_worker_spec.rb | 30 +++++ .../status_update_distribution_worker_spec.rb | 68 +++++++--- 25 files changed, 619 insertions(+), 41 deletions(-) create mode 100644 app/models/concerns/status/interaction_policy_concern.rb create mode 100644 app/workers/activitypub/quote_request_worker.rb create mode 100644 spec/models/concerns/status/interaction_policy_concern_spec.rb create mode 100644 spec/workers/activitypub/quote_request_worker_spec.rb diff --git a/app/controllers/api/v1/statuses_controller.rb b/app/controllers/api/v1/statuses_controller.rb index e25b161afd8..f047ba60466 100644 --- a/app/controllers/api/v1/statuses_controller.rb +++ b/app/controllers/api/v1/statuses_controller.rb @@ -10,6 +10,7 @@ class Api::V1::StatusesController < Api::BaseController before_action :set_statuses, only: [:index] before_action :set_status, only: [:show, :context] before_action :set_thread, only: [:create] + before_action :set_quoted_status, only: [:create] before_action :check_statuses_limit, only: [:index] override_rate_limit_headers :create, family: :statuses @@ -76,6 +77,7 @@ class Api::V1::StatusesController < Api::BaseController current_user.account, text: status_params[:status], thread: @thread, + quoted_status: @quoted_status, media_ids: status_params[:media_ids], sensitive: status_params[:sensitive], spoiler_text: status_params[:spoiler_text], @@ -147,6 +149,16 @@ class Api::V1::StatusesController < Api::BaseController render json: { error: I18n.t('statuses.errors.in_reply_not_found') }, status: 404 end + def set_quoted_status + return unless Mastodon::Feature.outgoing_quotes_enabled? + + @quoted_status = Status.find(status_params[:quoted_status_id]) if status_params[:quoted_status_id].present? + authorize(@quoted_status, :quote?) if @quoted_status.present? + rescue ActiveRecord::RecordNotFound, Mastodon::NotPermittedError + # TODO: distinguish between non-existing and non-quotable posts + render json: { error: I18n.t('statuses.errors.quoted_status_not_found') }, status: 404 + end + def check_statuses_limit raise(Mastodon::ValidationError) if status_ids.size > DEFAULT_STATUSES_LIMIT end @@ -163,6 +175,7 @@ class Api::V1::StatusesController < Api::BaseController params.permit( :status, :in_reply_to_id, + :quoted_status_id, :sensitive, :spoiler_text, :visibility, diff --git a/app/lib/activitypub/activity.rb b/app/lib/activitypub/activity.rb index 93b45e80188..64ee9acd052 100644 --- a/app/lib/activitypub/activity.rb +++ b/app/lib/activitypub/activity.rb @@ -116,6 +116,20 @@ class ActivityPub::Activity fetch_remote_original_status end + def quote_from_request_json(json) + quoted_status_uri = value_or_id(json['object']) + quoting_status_uri = value_or_id(json['instrument']) + return if quoting_status_uri.nil? || quoted_status_uri.nil? + + quoting_status = status_from_uri(quoting_status_uri) + return unless quoting_status.present? && quoting_status.quote.present? + + quoted_status = status_from_uri(quoted_status_uri) + return unless quoted_status.present? && quoted_status.account == @account && quoting_status.quote.quoted_status == quoted_status + + quoting_status.quote + end + def dereference_object! return unless @object.is_a?(String) @@ -143,6 +157,10 @@ class ActivityPub::Activity @follow_request_from_object ||= FollowRequest.find_by(target_account: @account, uri: object_uri) unless object_uri.nil? end + def quote_request_from_object + @quote_request_from_object ||= Quote.find_by(quoted_account: @account, activity_uri: object_uri) unless object_uri.nil? + end + def follow_from_object @follow_from_object ||= ::Follow.find_by(target_account: @account, uri: object_uri) unless object_uri.nil? end diff --git a/app/lib/activitypub/activity/accept.rb b/app/lib/activitypub/activity/accept.rb index 5126e23c6a9..144ba9645c5 100644 --- a/app/lib/activitypub/activity/accept.rb +++ b/app/lib/activitypub/activity/accept.rb @@ -4,10 +4,13 @@ class ActivityPub::Activity::Accept < ActivityPub::Activity def perform return accept_follow_for_relay if relay_follow? return accept_follow!(follow_request_from_object) unless follow_request_from_object.nil? + return accept_quote!(quote_request_from_object) unless quote_request_from_object.nil? case @object['type'] when 'Follow' accept_embedded_follow + when 'QuoteRequest' + accept_embedded_quote_request end end @@ -31,6 +34,29 @@ class ActivityPub::Activity::Accept < ActivityPub::Activity RemoteAccountRefreshWorker.perform_async(request.target_account_id) if is_first_follow end + def accept_embedded_quote_request + approval_uri = value_or_id(first_of_value(@json['result'])) + return if approval_uri.nil? + + quote = quote_from_request_json(@object) + return unless quote.present? && quote.status.local? + + accept_quote!(quote) + end + + def accept_quote!(quote) + approval_uri = value_or_id(first_of_value(@json['result'])) + return if unsupported_uri_scheme?(approval_uri) || quote.quoted_account != @account || !quote.status.local? + + # NOTE: we are not going through `ActivityPub::VerifyQuoteService` as the `Accept` is as authoritative + # as the stamp, but this means we are not checking the stamp, which may lead to inconsistencies + # in case of an implementation bug + quote.update!(state: :accepted, approval_uri: approval_uri) + + DistributionWorker.perform_async(quote.status_id, { 'update' => true }) + ActivityPub::StatusUpdateDistributionWorker.perform_async(quote.status_id, { 'updated_at' => Time.now.utc.iso8601 }) + end + def accept_follow_for_relay relay.update!(state: :accepted) end diff --git a/app/lib/activitypub/activity/reject.rb b/app/lib/activitypub/activity/reject.rb index 886dddb2355..3dafaba1882 100644 --- a/app/lib/activitypub/activity/reject.rb +++ b/app/lib/activitypub/activity/reject.rb @@ -5,10 +5,13 @@ class ActivityPub::Activity::Reject < ActivityPub::Activity return reject_follow_for_relay if relay_follow? return follow_request_from_object.reject! unless follow_request_from_object.nil? return UnfollowService.new.call(follow_from_object.account, @account) unless follow_from_object.nil? + return reject_quote!(quote_request_from_object) unless quote_request_from_object.nil? case @object['type'] when 'Follow' reject_embedded_follow + when 'QuoteRequest' + reject_embedded_quote_request end end @@ -29,6 +32,20 @@ class ActivityPub::Activity::Reject < ActivityPub::Activity relay.update!(state: :rejected) end + def reject_embedded_quote_request + quote = quote_from_request_json(@object) + return unless quote.present? && quote.status.local? + + reject_quote!(quoting_status.quote) + end + + def reject_quote!(quote) + return unless quote.quoted_account == @account && quote.status.local? + + # TODO: broadcast an update? + quote.reject! + end + def relay @relay ||= Relay.find_by(follow_activity_id: object_uri) unless object_uri.nil? end diff --git a/app/lib/activitypub/case_transform.rb b/app/lib/activitypub/case_transform.rb index bf5de722103..b9e1d3a62b2 100644 --- a/app/lib/activitypub/case_transform.rb +++ b/app/lib/activitypub/case_transform.rb @@ -12,9 +12,7 @@ module ActivityPub::CaseTransform when Hash then value.deep_transform_keys! { |key| camel_lower(key) } when Symbol then camel_lower(value.to_s).to_sym when String - camel_lower_cache[value] ||= if value.start_with?('_:') - "_:#{value.delete_prefix('_:').underscore.camelize(:lower)}" - elsif LanguagesHelper::ISO_639_1_REGIONAL.key?(value.to_sym) + camel_lower_cache[value] ||= if value.start_with?('_misskey') || LanguagesHelper::ISO_639_1_REGIONAL.key?(value.to_sym) value else value.underscore.camelize(:lower) diff --git a/app/lib/status_cache_hydrator.rb b/app/lib/status_cache_hydrator.rb index 674945c4039..5260a723b31 100644 --- a/app/lib/status_cache_hydrator.rb +++ b/app/lib/status_cache_hydrator.rb @@ -71,6 +71,8 @@ class StatusCacheHydrator payload[:bookmarked] = Bookmark.exists?(account_id: account_id, status_id: status.id) payload[:pinned] = StatusPin.exists?(account_id: account_id, status_id: status.id) if status.account_id == account_id payload[:filtered] = mapped_applied_custom_filter(account_id, status) + # TODO: performance optimization by not loading `Account` twice + payload[:quote_approval][:current_user] = status.quote_policy_for_account(Account.find_by(id: account_id)) if payload[:quote_approval] payload[:quote] = hydrate_quote_payload(payload[:quote], status.quote, account_id, nested:) if payload[:quote] end diff --git a/app/models/concerns/status/interaction_policy_concern.rb b/app/models/concerns/status/interaction_policy_concern.rb new file mode 100644 index 00000000000..7e7642209db --- /dev/null +++ b/app/models/concerns/status/interaction_policy_concern.rb @@ -0,0 +1,63 @@ +# frozen_string_literal: true + +module Status::InteractionPolicyConcern + extend ActiveSupport::Concern + + QUOTE_APPROVAL_POLICY_FLAGS = { + unknown: (1 << 0), + public: (1 << 1), + followers: (1 << 2), + followed: (1 << 3), + }.freeze + + def quote_policy_as_keys(kind) + case kind + when :automatic + policy = quote_approval_policy >> 16 + when :manual + policy = quote_approval_policy & 0xFFFF + end + + QUOTE_APPROVAL_POLICY_FLAGS.keys.select { |key| policy.anybits?(QUOTE_APPROVAL_POLICY_FLAGS[key]) }.map(&:to_s) + end + + # Returns `:automatic`, `:manual`, `:unknown` or `:denied` + def quote_policy_for_account(other_account, preloaded_relations: {}) + return :denied if other_account.nil? + + following_author = nil + + # Post author is always allowed to quote themselves + return :automatic if account_id == other_account.id + + automatic_policy = quote_approval_policy >> 16 + manual_policy = quote_approval_policy & 0xFFFF + + # Checking for public policy first because it's less expensive than looking at mentions + return :automatic if automatic_policy.anybits?(QUOTE_APPROVAL_POLICY_FLAGS[:public]) + + # Mentioned users are always allowed to quote + if active_mentions.loaded? + return :automatic if active_mentions.any? { |mention| mention.account_id == other_account.id } + elsif active_mentions.exists?(account: other_account) + return :automatic + end + + if automatic_policy.anybits?(QUOTE_APPROVAL_POLICY_FLAGS[:followers]) + following_author = preloaded_relations[:following] ? preloaded_relations[:following][account_id] : other_account.following?(account) if following_author.nil? + return :automatic if following_author + end + + # We don't know we are allowed by the automatic policy, considering the manual one + return :manual if manual_policy.anybits?(QUOTE_APPROVAL_POLICY_FLAGS[:public]) + + if manual_policy.anybits?(QUOTE_APPROVAL_POLICY_FLAGS[:followers]) + following_author = preloaded_relations[:following] ? preloaded_relations[:following][account_id] : other_account.following?(account) if following_author.nil? + return :manual if following_author + end + + return :unknown if (automatic_policy | manual_policy).anybits?(QUOTE_APPROVAL_POLICY_FLAGS[:unknown]) + + :denied + end +end diff --git a/app/models/status.rb b/app/models/status.rb index e6e9450264b..51150bec49e 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -43,16 +43,10 @@ class Status < ApplicationRecord include Status::SnapshotConcern include Status::ThreadingConcern include Status::Visibility + include Status::InteractionPolicyConcern MEDIA_ATTACHMENTS_LIMIT = 4 - QUOTE_APPROVAL_POLICY_FLAGS = { - unknown: (1 << 0), - public: (1 << 1), - followers: (1 << 2), - followed: (1 << 3), - }.freeze - rate_limit by: :account, family: :statuses self.discard_column = :deleted_at diff --git a/app/policies/status_policy.rb b/app/policies/status_policy.rb index 540e266427f..d9bb7201c00 100644 --- a/app/policies/status_policy.rb +++ b/app/policies/status_policy.rb @@ -19,6 +19,11 @@ class StatusPolicy < ApplicationPolicy end end + # This is about requesting a quote post, not validating it + def quote? + record.quote_policy_for_account(current_account, preloaded_relations: @preloaded_relations) != :denied + end + def reblog? !requires_mention? && (!private? || owned?) && show? && !blocking_author? end diff --git a/app/serializers/activitypub/note_serializer.rb b/app/serializers/activitypub/note_serializer.rb index 7b29e6d69be..95a869658c3 100644 --- a/app/serializers/activitypub/note_serializer.rb +++ b/app/serializers/activitypub/note_serializer.rb @@ -3,7 +3,7 @@ class ActivityPub::NoteSerializer < ActivityPub::Serializer include FormattingHelper - context_extensions :atom_uri, :conversation, :sensitive, :voters_count + context_extensions :atom_uri, :conversation, :sensitive, :voters_count, :quotes attributes :id, :type, :summary, :in_reply_to, :published, :url, @@ -30,6 +30,11 @@ class ActivityPub::NoteSerializer < ActivityPub::Serializer attribute :voters_count, if: :poll_and_voters_count? + attribute :quote, if: :quote? + attribute :quote, key: :_misskey_quote, if: :quote? + attribute :quote, key: :quote_uri, if: :quote? + attribute :quote_authorization, if: :quote_authorization? + def id ActivityPub::TagManager.instance.uri_for(object) end @@ -194,6 +199,24 @@ class ActivityPub::NoteSerializer < ActivityPub::Serializer object.preloadable_poll&.voters_count end + def quote? + object.quote&.present? + end + + def quote_authorization? + object.quote&.approval_uri.present? + end + + def quote + # TODO: handle inlining self-quotes + ActivityPub::TagManager.instance.uri_for(object.quote.quoted_status) + end + + def quote_authorization + # TODO: approval of local quotes may work differently, perhaps? + object.quote.approval_uri + end + class MediaAttachmentSerializer < ActivityPub::Serializer context_extensions :blurhash, :focal_point diff --git a/app/serializers/rest/status_serializer.rb b/app/serializers/rest/status_serializer.rb index 29e77e7d5b1..4ade8132111 100644 --- a/app/serializers/rest/status_serializer.rb +++ b/app/serializers/rest/status_serializer.rb @@ -32,6 +32,7 @@ class REST::StatusSerializer < ActiveModel::Serializer has_one :quote, key: :quote, serializer: REST::QuoteSerializer has_one :preview_card, key: :card, serializer: REST::PreviewCardSerializer has_one :preloadable_poll, key: :poll, serializer: REST::PollSerializer + has_one :quote_approval, if: -> { Mastodon::Feature.outgoing_quotes_enabled? } def quote object.quote if object.quote&.acceptable? @@ -159,6 +160,14 @@ class REST::StatusSerializer < ActiveModel::Serializer object.active_mentions.to_a.sort_by(&:id) end + def quote_approval + { + automatic: object.quote_policy_as_keys(:automatic), + manual: object.quote_policy_as_keys(:manual), + current_user: object.quote_policy_for_account(current_user&.account), + } + end + private def relationships diff --git a/app/services/post_status_service.rb b/app/services/post_status_service.rb index ac4b535ea9d..73e78f00478 100644 --- a/app/services/post_status_service.rb +++ b/app/services/post_status_service.rb @@ -96,13 +96,11 @@ class PostStatusService < BaseService # NOTE: for now this is only for convenience in testing, as we don't support the request flow nor serialize quotes in ActivityPub # we only support incoming quotes so far - status.quote = Quote.new(quoted_status: @quoted_status) - status.quote.accept! if @status.account == @quoted_status.account || @quoted_status.active_mentions.exists?(mentions: { account_id: status.account_id }) - - # TODO: the following has yet to be implemented: - # - handle approval of local users (requires the interactionPolicy PR) - # - produce a QuoteAuthorization for quotes of local users - # - send a QuoteRequest for quotes of remote users + status.quote = Quote.create(quoted_status: @quoted_status, status: status) + if @quoted_status.local? && StatusPolicy.new(@status.account, @quoted_status).quote? + # TODO: produce a QuoteAuthorization + status.quote.accept! + end end def safeguard_mentions!(status) @@ -146,6 +144,7 @@ class PostStatusService < BaseService DistributionWorker.perform_async(@status.id) ActivityPub::DistributionWorker.perform_async(@status.id) PollExpirationNotifyWorker.perform_at(@status.poll.expires_at, @status.poll.id) if @status.poll + ActivityPub::QuoteRequestWorker.perform_async(@status.quote.id) if @status.quote&.quoted_status.present? && !@status.quote&.quoted_status&.local? end def validate_media! diff --git a/app/workers/activitypub/quote_request_worker.rb b/app/workers/activitypub/quote_request_worker.rb new file mode 100644 index 00000000000..de5e054f69f --- /dev/null +++ b/app/workers/activitypub/quote_request_worker.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +class ActivityPub::QuoteRequestWorker < ActivityPub::RawDistributionWorker + def perform(quote_id) + @quote = Quote.find(quote_id) + @account = @quote.account + + distribute! + rescue ActiveRecord::RecordNotFound + true + end + + protected + + def inboxes + @inboxes ||= [@quote.quoted_account&.inbox_url].compact + end + + def payload + @payload ||= Oj.dump(serialize_payload(@quote, ActivityPub::QuoteRequestSerializer, signer: @account)) + end +end diff --git a/app/workers/activitypub/status_update_distribution_worker.rb b/app/workers/activitypub/status_update_distribution_worker.rb index a79ede2bf61..7f70fcaecc6 100644 --- a/app/workers/activitypub/status_update_distribution_worker.rb +++ b/app/workers/activitypub/status_update_distribution_worker.rb @@ -17,10 +17,10 @@ class ActivityPub::StatusUpdateDistributionWorker < ActivityPub::DistributionWor def activity ActivityPub::ActivityPresenter.new( - id: [ActivityPub::TagManager.instance.uri_for(@status), '#updates/', @status.edited_at.to_i].join, + id: [ActivityPub::TagManager.instance.uri_for(@status), '#updates/', @options[:updated_at]&.to_datetime&.to_i || @status.edited_at.to_i].join, type: 'Update', actor: ActivityPub::TagManager.instance.uri_for(@status.account), - published: @status.edited_at, + published: @options[:updated_at]&.to_datetime || @status.edited_at, to: ActivityPub::TagManager.instance.to(@status), cc: ActivityPub::TagManager.instance.cc(@status), virtual_object: @status diff --git a/config/locales/en.yml b/config/locales/en.yml index 4df63f4c738..204340f504c 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1873,6 +1873,7 @@ en: edited_at_html: Edited %{date} errors: in_reply_not_found: The post you are trying to reply to does not appear to exist. + quoted_status_not_found: The post you are trying to quote does not appear to exist. over_character_limit: character limit of %{max} exceeded pin_errors: direct: Posts that are only visible to mentioned users cannot be pinned diff --git a/spec/lib/activitypub/activity/accept_spec.rb b/spec/lib/activitypub/activity/accept_spec.rb index 6d7c05e6165..615287389c3 100644 --- a/spec/lib/activitypub/activity/accept_spec.rb +++ b/spec/lib/activitypub/activity/accept_spec.rb @@ -3,7 +3,7 @@ require 'rails_helper' RSpec.describe ActivityPub::Activity::Accept do - let(:sender) { Fabricate(:account) } + let(:sender) { Fabricate(:account, domain: 'example.com') } let(:recipient) { Fabricate(:account) } describe '#perform' do @@ -48,5 +48,128 @@ RSpec.describe ActivityPub::Activity::Accept do end end end + + context 'with a QuoteRequest' do + let(:status) { Fabricate(:status, account: recipient) } + let(:quoted_status) { Fabricate(:status, account: sender) } + let(:quote) { Fabricate(:quote, status: status, quoted_status: quoted_status) } + let(:approval_uri) { "https://#{sender.domain}/approvals/1" } + + let(:json) do + { + '@context': [ + 'https://www.w3.org/ns/activitystreams', + { + QuoteRequest: 'https://w3id.org/fep/044f#QuoteRequest', + }, + ], + id: 'foo', + type: 'Accept', + actor: ActivityPub::TagManager.instance.uri_for(sender), + object: { + id: quote.activity_uri, + type: 'QuoteRequest', + actor: ActivityPub::TagManager.instance.uri_for(recipient), + object: ActivityPub::TagManager.instance.uri_for(quoted_status), + instrument: ActivityPub::TagManager.instance.uri_for(status), + }, + result: approval_uri, + }.with_indifferent_access + end + + it 'marks the quote as approved and distribute an update' do + expect { subject.perform } + .to change { quote.reload.accepted? }.from(false).to(true) + .and change { quote.reload.approval_uri }.to(approval_uri) + expect(DistributionWorker) + .to have_enqueued_sidekiq_job(status.id, { 'update' => true }) + expect(ActivityPub::StatusUpdateDistributionWorker) + .to have_enqueued_sidekiq_job(status.id, { 'updated_at' => be_a(String) }) + end + + context 'when the quoted status is not from the sender of the Accept' do + let(:quoted_status) { Fabricate(:status, account: Fabricate(:account, domain: 'example.com')) } + + it 'does not mark the quote as approved and does not distribute an update' do + expect { subject.perform } + .to not_change { quote.reload.accepted? }.from(false) + .and not_change { quote.reload.approval_uri }.from(nil) + expect(DistributionWorker) + .to_not have_enqueued_sidekiq_job(status.id, { 'update' => true }) + expect(ActivityPub::StatusUpdateDistributionWorker) + .to_not have_enqueued_sidekiq_job(status.id, anything) + end + end + + context 'when the quoting status is from an unrelated user' do + let(:status) { Fabricate(:status, account: Fabricate(:account, domain: 'foobar.com')) } + + it 'does not mark the quote as approved and does not distribute an update' do + expect { subject.perform } + .to not_change { quote.reload.accepted? }.from(false) + .and not_change { quote.reload.approval_uri }.from(nil) + expect(DistributionWorker) + .to_not have_enqueued_sidekiq_job(status.id, { 'update' => true }) + expect(ActivityPub::StatusUpdateDistributionWorker) + .to_not have_enqueued_sidekiq_job(status.id, anything) + end + end + + context 'when approval_uri is missing' do + let(:approval_uri) { nil } + + it 'does not mark the quote as approved and does not distribute an update' do + expect { subject.perform } + .to not_change { quote.reload.accepted? }.from(false) + .and not_change { quote.reload.approval_uri }.from(nil) + expect(DistributionWorker) + .to_not have_enqueued_sidekiq_job(status.id, { 'update' => true }) + expect(ActivityPub::StatusUpdateDistributionWorker) + .to_not have_enqueued_sidekiq_job(status.id, anything) + end + end + + context 'when the QuoteRequest is referenced by its identifier' do + let(:json) do + { + '@context': [ + 'https://www.w3.org/ns/activitystreams', + { + QuoteRequest: 'https://w3id.org/fep/044f#QuoteRequest', + }, + ], + id: 'foo', + type: 'Accept', + actor: ActivityPub::TagManager.instance.uri_for(sender), + object: quote.activity_uri, + result: approval_uri, + }.with_indifferent_access + end + + it 'marks the quote as approved and distribute an update' do + expect { subject.perform } + .to change { quote.reload.accepted? }.from(false).to(true) + .and change { quote.reload.approval_uri }.to(approval_uri) + expect(DistributionWorker) + .to have_enqueued_sidekiq_job(status.id, { 'update' => true }) + expect(ActivityPub::StatusUpdateDistributionWorker) + .to have_enqueued_sidekiq_job(status.id, { 'updated_at' => be_a(String) }) + end + + context 'when approval_uri is missing' do + let(:approval_uri) { nil } + + it 'does not mark the quote as approved and does not distribute an update' do + expect { subject.perform } + .to not_change { quote.reload.accepted? }.from(false) + .and not_change { quote.reload.approval_uri }.from(nil) + expect(DistributionWorker) + .to_not have_enqueued_sidekiq_job(status.id, { 'update' => true }) + expect(ActivityPub::StatusUpdateDistributionWorker) + .to_not have_enqueued_sidekiq_job(status.id, anything) + end + end + end + end end end diff --git a/spec/lib/activitypub/activity/reject_spec.rb b/spec/lib/activitypub/activity/reject_spec.rb index 1afb0cd4033..ee8557f1239 100644 --- a/spec/lib/activitypub/activity/reject_spec.rb +++ b/spec/lib/activitypub/activity/reject_spec.rb @@ -125,5 +125,27 @@ RSpec.describe ActivityPub::Activity::Reject do expect(relay.reload.rejected?).to be true end end + + context 'with a QuoteRequest' do + let(:status) { Fabricate(:status, account: recipient) } + let(:quoted_status) { Fabricate(:status, account: sender) } + let(:quote) { Fabricate(:quote, status: status, quoted_status: quoted_status, activity_uri: 'https://abc-123/456') } + let(:approval_uri) { "https://#{sender.domain}/approvals/1" } + + let(:object_json) do + { + id: 'https://abc-123/456', + type: 'QuoteRequest', + actor: ActivityPub::TagManager.instance.uri_for(recipient), + object: ActivityPub::TagManager.instance.uri_for(quoted_status), + instrument: ActivityPub::TagManager.instance.uri_for(status), + }.with_indifferent_access + end + + it 'marks the quote as rejected' do + expect { subject.perform } + .to change { quote.reload.rejected? }.from(false).to(true) + end + end end end diff --git a/spec/lib/status_cache_hydrator_spec.rb b/spec/lib/status_cache_hydrator_spec.rb index e56393da1dd..f450997976a 100644 --- a/spec/lib/status_cache_hydrator_spec.rb +++ b/spec/lib/status_cache_hydrator_spec.rb @@ -28,6 +28,18 @@ RSpec.describe StatusCacheHydrator do end end + context 'when handling a status with a quote policy', feature: :outgoing_quotes do + let(:status) { Fabricate(:status, quote_approval_policy: Status::QUOTE_APPROVAL_POLICY_FLAGS[:followers] << 16) } + + before do + account.follow!(status.account) + end + + it 'renders the same attributes as a full render' do + expect(subject).to eql(compare_to_hash) + end + end + context 'when handling a filtered status' do let(:status) { Fabricate(:status, text: 'this toot is about that banned word') } diff --git a/spec/models/concerns/status/interaction_policy_concern_spec.rb b/spec/models/concerns/status/interaction_policy_concern_spec.rb new file mode 100644 index 00000000000..af42f2bba3d --- /dev/null +++ b/spec/models/concerns/status/interaction_policy_concern_spec.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe Status::InteractionPolicyConcern do + let(:status) { Fabricate(:status, quote_approval_policy: (0b0101 << 16) | 0b0010) } + + describe '#quote_policy_as_keys' do + it 'returns the expected values' do + expect(status.quote_policy_as_keys(:automatic)).to eq ['unknown', 'followers'] + expect(status.quote_policy_as_keys(:manual)).to eq ['public'] + end + end + + describe '#quote_policy_for_account' do + let(:account) { Fabricate(:account) } + + context 'when the account is not following the user' do + it 'returns :manual because of the public entry in the manual policy' do + expect(status.quote_policy_for_account(account)).to eq :manual + end + end + + context 'when the account is following the user' do + before do + account.follow!(status.account) + end + + it 'returns :automatic because of the followers entry in the automatic policy' do + expect(status.quote_policy_for_account(account)).to eq :automatic + end + end + + context 'when the account falls into the unknown bucket' do + let(:status) { Fabricate(:status, quote_approval_policy: (0b0001 << 16) | 0b0100) } + + it 'returns :automatic because of the followers entry in the automatic policy' do + expect(status.quote_policy_for_account(account)).to eq :unknown + end + end + end +end diff --git a/spec/policies/status_policy_spec.rb b/spec/policies/status_policy_spec.rb index 69c0bad026b..63622970455 100644 --- a/spec/policies/status_policy_spec.rb +++ b/spec/policies/status_policy_spec.rb @@ -86,6 +86,92 @@ RSpec.describe StatusPolicy, type: :model do end end + context 'with the permission of quote?' do + permissions :quote? do + it 'grants access when direct and account is viewer' do + status.visibility = :direct + + expect(subject).to permit(status.account, status) + end + + it 'grants access when direct and viewer is mentioned' do + status.visibility = :direct + status.mentions = [Fabricate(:mention, account: alice)] + + expect(subject).to permit(alice, status) + end + + it 'grants access when direct and non-owner viewer is mentioned and mentions are loaded' do + status.visibility = :direct + status.mentions = [Fabricate(:mention, account: bob)] + status.active_mentions.load + + expect(subject).to permit(bob, status) + end + + it 'denies access when direct and viewer is not mentioned' do + viewer = Fabricate(:account) + status.visibility = :direct + + expect(subject).to_not permit(viewer, status) + end + + it 'denies access when private and viewer is not mentioned' do + viewer = Fabricate(:account) + status.visibility = :private + + expect(subject).to_not permit(viewer, status) + end + + it 'grants access when private and viewer is mentioned' do + status.visibility = :private + status.mentions = [Fabricate(:mention, account: bob)] + + expect(subject).to permit(bob, status) + end + + it 'denies access when private and non-viewer is mentioned' do + viewer = Fabricate(:account) + status.visibility = :private + status.mentions = [Fabricate(:mention, account: bob)] + + expect(subject).to_not permit(viewer, status) + end + + it 'denies access when private and account is following viewer' do + follow = Fabricate(:follow) + status.visibility = :private + status.account = follow.target_account + + expect(subject).to_not permit(follow.account, status) + end + + it 'denies access when public but policy does not allow anyone' do + viewer = Fabricate(:account) + expect(subject).to_not permit(viewer, status) + end + + it 'grants access when public and policy allows everyone' do + status.quote_approval_policy = Status::QUOTE_APPROVAL_POLICY_FLAGS[:public] + viewer = Fabricate(:account) + expect(subject).to permit(viewer, status) + end + + it 'denies access when public and policy allows followers but viewer is not one' do + status.quote_approval_policy = Status::QUOTE_APPROVAL_POLICY_FLAGS[:followers] + viewer = Fabricate(:account) + expect(subject).to_not permit(viewer, status) + end + + it 'grants access when public and policy allows followers and viewer is one' do + status.quote_approval_policy = Status::QUOTE_APPROVAL_POLICY_FLAGS[:followers] + viewer = Fabricate(:account) + viewer.follow!(status.account) + expect(subject).to permit(viewer, status) + end + end + end + context 'with the permission of reblog?' do permissions :reblog? do it 'denies access when private' do diff --git a/spec/requests/api/v1/statuses_spec.rb b/spec/requests/api/v1/statuses_spec.rb index 285fa936552..ac15ae24623 100644 --- a/spec/requests/api/v1/statuses_spec.rb +++ b/spec/requests/api/v1/statuses_spec.rb @@ -158,6 +158,27 @@ RSpec.describe '/api/v1/statuses' do end end + context 'with a self-quote post', feature: :outgoing_quotes do + let(:quoted_status) { Fabricate(:status, account: user.account) } + let(:params) do + { + status: 'Hello world, this is a self-quote', + quoted_status_id: quoted_status.id, + } + end + + it 'returns a quote post, as well as rate limit headers', :aggregate_failures do + subject + + expect(response).to have_http_status(200) + expect(response.content_type) + .to start_with('application/json') + expect(response.parsed_body[:quote]).to be_present + expect(response.headers['X-RateLimit-Limit']).to eq RateLimiter::FAMILIES[:statuses][:limit].to_s + expect(response.headers['X-RateLimit-Remaining']).to eq (RateLimiter::FAMILIES[:statuses][:limit] - 1).to_s + end + end + context 'with a safeguard' do let!(:alice) { Fabricate(:account, username: 'alice') } let!(:bob) { Fabricate(:account, username: 'bob') } diff --git a/spec/serializers/activitypub/note_serializer_spec.rb b/spec/serializers/activitypub/note_serializer_spec.rb index a6976193b20..d1af3f068f5 100644 --- a/spec/serializers/activitypub/note_serializer_spec.rb +++ b/spec/serializers/activitypub/note_serializer_spec.rb @@ -41,4 +41,20 @@ RSpec.describe ActivityPub::NoteSerializer do .and(not_include(reply_by_other_first.uri)) # Replies from others .and(not_include(reply_by_account_visibility_direct.uri)) # Replies with direct visibility end + + context 'with a quote' do + let(:quoted_status) { Fabricate(:status) } + let(:approval_uri) { 'https://example.com/foo/bar' } + let!(:quote) { Fabricate(:quote, status: parent, quoted_status: quoted_status, approval_uri: approval_uri) } + + it 'has the expected shape' do + expect(subject).to include({ + 'type' => 'Note', + 'quote' => ActivityPub::TagManager.instance.uri_for(quote.quoted_status), + 'quoteUri' => ActivityPub::TagManager.instance.uri_for(quote.quoted_status), + '_misskey_quote' => ActivityPub::TagManager.instance.uri_for(quote.quoted_status), + 'quoteAuthorization' => approval_uri, + }) + end + end end diff --git a/spec/services/post_status_service_spec.rb b/spec/services/post_status_service_spec.rb index 8836b9e0a63..7e47506a9f7 100644 --- a/spec/services/post_status_service_spec.rb +++ b/spec/services/post_status_service_spec.rb @@ -291,6 +291,14 @@ RSpec.describe PostStatusService do ) end + it 'correctly requests a quote for remote posts' do + account = Fabricate(:account) + quoted_status = Fabricate(:status, account: Fabricate(:account, domain: 'example.com')) + + expect { subject.call(account, text: 'test', quoted_status: quoted_status) } + .to enqueue_sidekiq_job(ActivityPub::QuoteRequestWorker) + end + it 'returns existing status when used twice with idempotency key' do account = Fabricate(:account) status1 = subject.call(account, text: 'test', idempotency: 'meepmeep') diff --git a/spec/workers/activitypub/quote_request_worker_spec.rb b/spec/workers/activitypub/quote_request_worker_spec.rb new file mode 100644 index 00000000000..3d0131baaad --- /dev/null +++ b/spec/workers/activitypub/quote_request_worker_spec.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe ActivityPub::QuoteRequestWorker do + subject { described_class.new } + + let(:quoted_account) { Fabricate(:account, inbox_url: 'http://example.com', domain: 'example.com') } + let(:quoted_status) { Fabricate(:status, account: quoted_account) } + let(:status) { Fabricate(:status, text: 'foo') } + let(:quote) { Fabricate(:quote, status: status, quoted_status: quoted_status, activity_uri: 'TODO') } # TODO: activity URI + + describe '#perform' do + it 'sends the expected QuoteRequest activity' do + subject.perform(quote.id) + + expect(ActivityPub::DeliveryWorker) + .to have_enqueued_sidekiq_job(match_object_shape, quote.account_id, 'http://example.com', {}) + end + + def match_object_shape + match_json_values( + type: 'QuoteRequest', + actor: ActivityPub::TagManager.instance.uri_for(quote.account), + object: ActivityPub::TagManager.instance.uri_for(quoted_status), + instrument: anything # TODO: inline post in request? + ) + end + end +end diff --git a/spec/workers/activitypub/status_update_distribution_worker_spec.rb b/spec/workers/activitypub/status_update_distribution_worker_spec.rb index e9a70d11d19..58d11db41cb 100644 --- a/spec/workers/activitypub/status_update_distribution_worker_spec.rb +++ b/spec/workers/activitypub/status_update_distribution_worker_spec.rb @@ -9,36 +9,64 @@ RSpec.describe ActivityPub::StatusUpdateDistributionWorker do let(:follower) { Fabricate(:account, protocol: :activitypub, inbox_url: 'http://example.com', domain: 'example.com') } describe '#perform' do - before do - follower.follow!(status.account) - - status.snapshot! - status.text = 'bar' - status.edited_at = Time.now.utc - status.snapshot! - status.save! - end - - context 'with public status' do + context 'with an explicitly edited status' do before do - status.update(visibility: :public) + follower.follow!(status.account) + + status.snapshot! + status.text = 'bar' + status.edited_at = Time.now.utc + status.snapshot! + status.save! end - it 'delivers to followers' do - expect_push_bulk_to_match(ActivityPub::DeliveryWorker, [[match_json_values(type: 'Update'), status.account.id, 'http://example.com', anything]]) do - subject.perform(status.id) + context 'with public status' do + before do + status.update(visibility: :public) + end + + it 'delivers to followers' do + expect { subject.perform(status.id) } + .to enqueue_sidekiq_job(ActivityPub::DeliveryWorker).with(match_json_values(type: 'Update'), status.account_id, 'http://example.com', anything) + end + end + + context 'with private status' do + before do + status.update(visibility: :private) + end + + it 'delivers to followers' do + expect { subject.perform(status.id) } + .to enqueue_sidekiq_job(ActivityPub::DeliveryWorker).with(match_json_values(type: 'Update'), status.account_id, 'http://example.com', anything) end end end - context 'with private status' do + context 'with an implicitly edited status' do before do - status.update(visibility: :private) + follower.follow!(status.account) end - it 'delivers to followers' do - expect_push_bulk_to_match(ActivityPub::DeliveryWorker, [[match_json_values(type: 'Update'), status.account.id, 'http://example.com', anything]]) do - subject.perform(status.id) + context 'with public status' do + before do + status.update(visibility: :public) + end + + it 'delivers to followers' do + expect { subject.perform(status.id, { 'updated_at' => Time.now.utc.iso8601 }) } + .to enqueue_sidekiq_job(ActivityPub::DeliveryWorker).with(match_json_values(type: 'Update'), status.account_id, 'http://example.com', anything) + end + end + + context 'with private status' do + before do + status.update(visibility: :private) + end + + it 'delivers to followers' do + expect { subject.perform(status.id, { 'updated_at' => Time.now.utc.iso8601 }) } + .to enqueue_sidekiq_job(ActivityPub::DeliveryWorker).with(match_json_values(type: 'Update'), status.account_id, 'http://example.com', anything) end end end