diff --git a/app/services/post_status_service.rb b/app/services/post_status_service.rb index c55a54df3..3e4a71522 100644 --- a/app/services/post_status_service.rb +++ b/app/services/post_status_service.rb @@ -80,6 +80,7 @@ class PostStatusService < BaseService @status = @account.statuses.new(status_attributes) process_mentions_service.call(@status, save_records: false) safeguard_mentions!(@status) + safeguard_private_mention_quote!(@status) attach_quote!(@status) antispam = Antispam.new(@status) @@ -92,6 +93,16 @@ class PostStatusService < BaseService end end + def safeguard_private_mention_quote!(status) + return if @quoted_status.nil? || @visibility.to_sym != :direct + + # The mentions array test here is awkward because the relationship is not persisted at this time + return if @quoted_status.account_id == @account.id || status.mentions.to_a.any? { |mention| mention.account_id == @quoted_status.account_id && !mention.silent } + + status.errors.add(:base, I18n.t('statuses.errors.quoted_user_not_mentioned')) + raise ActiveRecord::RecordInvalid, status + end + def attach_quote!(status) return if @quoted_status.nil? @@ -114,6 +125,7 @@ class PostStatusService < BaseService def schedule_status! status_for_validation = @account.statuses.build(status_attributes) + safeguard_private_mention_quote!(status_for_validation) antispam = Antispam.new(status_for_validation) antispam.local_preflight_check! diff --git a/config/locales/en.yml b/config/locales/en.yml index b647c5ddf..949eb6d90 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1930,6 +1930,7 @@ en: 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. + quoted_user_not_mentioned: Cannot quote a non-mentioned user in a Private Mention post. 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/requests/api/v1/statuses_spec.rb b/spec/requests/api/v1/statuses_spec.rb index ed41e5420..d3def6104 100644 --- a/spec/requests/api/v1/statuses_spec.rb +++ b/spec/requests/api/v1/statuses_spec.rb @@ -228,7 +228,7 @@ RSpec.describe '/api/v1/statuses' do end context 'with a self-quote post' do - let(:quoted_status) { Fabricate(:status, account: user.account) } + let!(:quoted_status) { Fabricate(:status, account: user.account) } let(:params) do { status: 'Hello world, this is a self-quote', @@ -237,7 +237,48 @@ RSpec.describe '/api/v1/statuses' do end it 'returns a quote post, as well as rate limit headers', :aggregate_failures do - subject + expect { subject }.to change(user.account.statuses, :count).by(1) + + 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 quote to a non-mentioned user in a Private Mention' do + let!(:quoted_status) { Fabricate(:status, quote_approval_policy: Status::QUOTE_APPROVAL_POLICY_FLAGS[:public] << 16) } + let(:params) do + { + status: 'Hello, this is a quote', + quoted_status_id: quoted_status.id, + visibility: :direct, + } + end + + it 'returns an error and does not create a post', :aggregate_failures do + expect { subject }.to_not change(user.account.statuses, :count) + + expect(response).to have_http_status(422) + expect(response.content_type) + .to start_with('application/json') + end + end + + context 'with a quote to a mentioned user in a Private Mention' do + let!(:quoted_status) { Fabricate(:status, quote_approval_policy: Status::QUOTE_APPROVAL_POLICY_FLAGS[:public] << 16) } + let(:params) do + { + status: "Hello @#{quoted_status.account.acct}, this is a quote", + quoted_status_id: quoted_status.id, + visibility: :direct, + } + end + + it 'returns a quote post, as well as rate limit headers', :aggregate_failures do + expect { subject }.to change(user.account.statuses, :count).by(1) expect(response).to have_http_status(200) expect(response.content_type)