Add support for private pinned posts (#16954)

* Add support for private pinned toots

* Allow local user to pin private toots

* Change wording to avoid "direct message"

18 files changed, 164 insertions, 28 deletions

diff --git a/app/controllers/accounts_controller.rb b/app/controllers/accounts_controller.rb
index 8210918d869..ddd38cbb01f 100644
--- a/app/controllers/accounts_controller.rb
+++ b/app/controllers/accounts_controller.rb
@@ -28,7 +28,7 @@ class AccountsController < ApplicationController
           return
         end
 
-        @pinned_statuses = cache_collection(@account.pinned_statuses, Status) if show_pinned_statuses?
+        @pinned_statuses = cached_filtered_status_pins if show_pinned_statuses?
         @statuses        = cached_filtered_status_page
         @rss_url         = rss_url
 
@@ -64,6 +64,10 @@ class AccountsController < ApplicationController
     [replies_requested?, media_requested?, tag_requested?, params[:max_id].present?, params[:min_id].present?].none?
   end
 
+  def filtered_pinned_statuses
+    @account.pinned_statuses.where(visibility: [:public, :unlisted])
+  end
+
   def filtered_statuses
     default_statuses.tap do |statuses|
       statuses.merge!(hashtag_scope)    if tag_requested?
@@ -142,6 +146,13 @@ class AccountsController < ApplicationController
     request.path.split('.').first.end_with?(Addressable::URI.parse("/tagged/#{params[:tag]}").normalize)
   end
 
+  def cached_filtered_status_pins
+    cache_collection(
+      filtered_pinned_statuses,
+      Status
+    )
+  end
+
   def cached_filtered_status_page
     cache_collection_paginated_by_id(
       filtered_statuses,
diff --git a/app/controllers/activitypub/collections_controller.rb b/app/controllers/activitypub/collections_controller.rb
index c8b6dcc88d4..e4e994a9855 100644
--- a/app/controllers/activitypub/collections_controller.rb
+++ b/app/controllers/activitypub/collections_controller.rb
@@ -21,6 +21,7 @@ class ActivityPub::CollectionsController < ActivityPub::BaseController
     case params[:id]
     when 'featured'
       @items = for_signed_account { cache_collection(@account.pinned_statuses, Status) }
+      @items = @items.map { |item| item.distributable? ? item : ActivityPub::TagManager.instance.uri_for(item) }
     when 'tags'
       @items = for_signed_account { @account.featured_tags }
     when 'devices'
diff --git a/app/controllers/api/v1/accounts/statuses_controller.rb b/app/controllers/api/v1/accounts/statuses_controller.rb
index 92ccb80615d..2c027ea7691 100644
--- a/app/controllers/api/v1/accounts/statuses_controller.rb
+++ b/app/controllers/api/v1/accounts/statuses_controller.rb
@@ -46,9 +46,7 @@ class Api::V1::Accounts::StatusesController < Api::BaseController
   end
 
   def pinned_scope
-    return Status.none if @account.blocking?(current_account)
-
-    @account.pinned_statuses
+    @account.pinned_statuses.permitted_for(@account, current_account)
   end
 
   def no_replies_scope
diff --git a/app/javascript/mastodon/components/status_action_bar.js b/app/javascript/mastodon/components/status_action_bar.js
index 85c76edee4b..d125359e9e7 100644
--- a/app/javascript/mastodon/components/status_action_bar.js
+++ b/app/javascript/mastodon/components/status_action_bar.js
@@ -225,6 +225,7 @@ class StatusActionBar extends ImmutablePureComponent {
 
     const anonymousAccess    = !me;
     const publicStatus       = ['public', 'unlisted'].includes(status.get('visibility'));
+    const pinnableStatus     = ['public', 'unlisted', 'private'].includes(status.get('visibility'));
     const mutingConversation = status.get('muted');
     const account            = status.get('account');
     const writtenByMe        = status.getIn(['account', 'id']) === me;
@@ -242,7 +243,7 @@ class StatusActionBar extends ImmutablePureComponent {
 
     menu.push({ text: intl.formatMessage(status.get('bookmarked') ? messages.removeBookmark : messages.bookmark), action: this.handleBookmarkClick });
 
-    if (writtenByMe && publicStatus) {
+    if (writtenByMe && pinnableStatus) {
       menu.push({ text: intl.formatMessage(status.get('pinned') ? messages.unpin : messages.pin), action: this.handlePinClick });
     }
 
diff --git a/app/javascript/mastodon/features/status/components/action_bar.js b/app/javascript/mastodon/features/status/components/action_bar.js
index ffa2510c0db..e60119bc47a 100644
--- a/app/javascript/mastodon/features/status/components/action_bar.js
+++ b/app/javascript/mastodon/features/status/components/action_bar.js
@@ -188,6 +188,7 @@ class ActionBar extends React.PureComponent {
     const { status, relationship, intl } = this.props;
 
     const publicStatus       = ['public', 'unlisted'].includes(status.get('visibility'));
+    const pinnableStatus     = ['public', 'unlisted', 'private'].includes(status.get('visibility'));
     const mutingConversation = status.get('muted');
     const account            = status.get('account');
     const writtenByMe        = status.getIn(['account', 'id']) === me;
@@ -201,7 +202,7 @@ class ActionBar extends React.PureComponent {
     }
 
     if (writtenByMe) {
-      if (publicStatus) {
+      if (pinnableStatus) {
         menu.push({ text: intl.formatMessage(status.get('pinned') ? messages.unpin : messages.pin), action: this.handlePinClick });
         menu.push(null);
       }
diff --git a/app/lib/activitypub/activity/accept.rb b/app/lib/activitypub/activity/accept.rb
index 7010ff43e8e..5126e23c6a9 100644
--- a/app/lib/activitypub/activity/accept.rb
+++ b/app/lib/activitypub/activity/accept.rb
@@ -3,7 +3,7 @@
 class ActivityPub::Activity::Accept < ActivityPub::Activity
   def perform
     return accept_follow_for_relay if relay_follow?
-    return follow_request_from_object.authorize! unless follow_request_from_object.nil?
+    return accept_follow!(follow_request_from_object) unless follow_request_from_object.nil?
 
     case @object['type']
     when 'Follow'
@@ -19,7 +19,16 @@ class ActivityPub::Activity::Accept < ActivityPub::Activity
     return if target_account.nil? || !target_account.local?
 
     follow_request = FollowRequest.find_by(account: target_account, target_account: @account)
-    follow_request&.authorize!
+    accept_follow!(follow_request)
+  end
+
+  def accept_follow!(request)
+    return if request.nil?
+
+    is_first_follow = !request.target_account.followers.local.exists?
+    request.authorize!
+
+    RemoteAccountRefreshWorker.perform_async(request.target_account_id) if is_first_follow
   end
 
   def accept_follow_for_relay
diff --git a/app/lib/activitypub/activity/add.rb b/app/lib/activitypub/activity/add.rb
index 688ab00b334..845eeaef796 100644
--- a/app/lib/activitypub/activity/add.rb
+++ b/app/lib/activitypub/activity/add.rb
@@ -4,8 +4,7 @@ class ActivityPub::Activity::Add < ActivityPub::Activity
   def perform
     return unless @json['target'].present? && value_or_id(@json['target']) == @account.featured_collection_url
 
-    status   = status_from_uri(object_uri)
-    status ||= fetch_remote_original_status
+    status = status_from_object
 
     return unless !status.nil? && status.account_id == @account.id && !@account.pinned?(status)
 
diff --git a/app/services/activitypub/fetch_featured_collection_service.rb b/app/services/activitypub/fetch_featured_collection_service.rb
index 72352aca6f2..9fce478c14b 100644
--- a/app/services/activitypub/fetch_featured_collection_service.rb
+++ b/app/services/activitypub/fetch_featured_collection_service.rb
@@ -23,7 +23,7 @@ class ActivityPub::FetchFeaturedCollectionService < BaseService
 
   def process_items(items)
     status_ids = items.map { |item| value_or_id(item) }
-                      .filter_map { |uri| ActivityPub::FetchRemoteStatusService.new.call(uri) unless ActivityPub::TagManager.instance.local_uri?(uri) }
+                      .filter_map { |uri| ActivityPub::FetchRemoteStatusService.new.call(uri, on_behalf_of: local_follower) unless ActivityPub::TagManager.instance.local_uri?(uri) }
                       .filter_map { |status| status.id if status.account_id == @account.id }
     to_remove = []
     to_add    = status_ids
@@ -46,4 +46,8 @@ class ActivityPub::FetchFeaturedCollectionService < BaseService
   def supported_context?
     super(@json)
   end
+
+  def local_follower
+    @local_follower ||= account.followers.local.without_suspended.first
+  end
 end
diff --git a/app/validators/status_pin_validator.rb b/app/validators/status_pin_validator.rb
index 2c7bce674bc..2fdd5b34fd4 100644
--- a/app/validators/status_pin_validator.rb
+++ b/app/validators/status_pin_validator.rb
@@ -4,7 +4,7 @@ class StatusPinValidator < ActiveModel::Validator
   def validate(pin)
     pin.errors.add(:base, I18n.t('statuses.pin_errors.reblog')) if pin.status.reblog?
     pin.errors.add(:base, I18n.t('statuses.pin_errors.ownership')) if pin.account_id != pin.status.account_id
-    pin.errors.add(:base, I18n.t('statuses.pin_errors.private')) unless %w(public unlisted).include?(pin.status.visibility)
+    pin.errors.add(:base, I18n.t('statuses.pin_errors.direct')) if pin.status.direct_visibility?
     pin.errors.add(:base, I18n.t('statuses.pin_errors.limit')) if pin.account.status_pins.count > 4 && pin.account.local?
   end
 end
diff --git a/app/workers/remote_account_refresh_worker.rb b/app/workers/remote_account_refresh_worker.rb
new file mode 100644
index 00000000000..9632936b547
--- /dev/null
+++ b/app/workers/remote_account_refresh_worker.rb
@@ -0,0 +1,24 @@
+# frozen_string_literal: true
+
+class RemoteAccountRefreshWorker
+  include Sidekiq::Worker
+  include ExponentialBackoff
+  include JsonLdHelper
+
+  sidekiq_options queue: 'pull', retry: 3
+
+  def perform(id)
+    account = Account.find_by(id: id)
+    return if account.nil? || account.local?
+
+    ActivityPub::FetchRemoteAccountService.new.call(account.uri)
+  rescue Mastodon::UnexpectedResponseError => e
+    response = e.response
+
+    if response_error_unsalvageable?(response)
+      # Give up
+    else
+      raise e
+    end
+  end
+end
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 32b48dbfffe..693a7b40084 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -1300,9 +1300,9 @@ en:
     open_in_web: Open in web
     over_character_limit: character limit of %{max} exceeded
     pin_errors:
+      direct: Posts that are only visible to mentioned users cannot be pinned
       limit: You have already pinned the maximum number of posts
       ownership: Someone else's post cannot be pinned
-      private: Non-public posts cannot be pinned
       reblog: A boost cannot be pinned
     poll:
       total_people:
diff --git a/spec/controllers/accounts_controller_spec.rb b/spec/controllers/accounts_controller_spec.rb
index ac426b01e63..7c5ba8754d5 100644
--- a/spec/controllers/accounts_controller_spec.rb
+++ b/spec/controllers/accounts_controller_spec.rb
@@ -35,6 +35,7 @@ RSpec.describe AccountsController, type: :controller do
     before do
       status_media.media_attachments << Fabricate(:media_attachment, account: account, type: :image)
       account.pinned_statuses << status_pinned
+      account.pinned_statuses << status_private
     end
 
     shared_examples 'preliminary checks' do
diff --git a/spec/controllers/activitypub/collections_controller_spec.rb b/spec/controllers/activitypub/collections_controller_spec.rb
index d584136ff56..21a0339458b 100644
--- a/spec/controllers/activitypub/collections_controller_spec.rb
+++ b/spec/controllers/activitypub/collections_controller_spec.rb
@@ -4,6 +4,7 @@ require 'rails_helper'
 
 RSpec.describe ActivityPub::CollectionsController, type: :controller do
   let!(:account) { Fabricate(:account) }
+  let!(:private_pinned) { Fabricate(:status, account: account, text: 'secret private stuff', visibility: :private) }
   let(:remote_account) { nil }
 
   shared_examples 'cachable response' do
@@ -27,6 +28,7 @@ RSpec.describe ActivityPub::CollectionsController, type: :controller do
 
     Fabricate(:status_pin, account: account)
     Fabricate(:status_pin, account: account)
+    Fabricate(:status_pin, account: account, status: private_pinned)
     Fabricate(:status, account: account, visibility: :private)
   end
 
@@ -50,7 +52,15 @@ RSpec.describe ActivityPub::CollectionsController, type: :controller do
 
         it 'returns orderedItems with pinned statuses' do
           expect(body[:orderedItems]).to be_an Array
-          expect(body[:orderedItems].size).to eq 2
+          expect(body[:orderedItems].size).to eq 3
+        end
+
+        it 'includes URI of private pinned status' do
+          expect(body[:orderedItems]).to include(ActivityPub::TagManager.instance.uri_for(private_pinned))
+        end
+
+        it 'does not include contents of private pinned status' do
+          expect(response.body).not_to include(private_pinned.text)
         end
 
         context 'when account is permanently suspended' do
@@ -96,7 +106,16 @@ RSpec.describe ActivityPub::CollectionsController, type: :controller do
           it 'returns orderedItems with pinned statuses' do
             json = body_as_json
             expect(json[:orderedItems]).to be_an Array
-            expect(json[:orderedItems].size).to eq 2
+            expect(json[:orderedItems].size).to eq 3
+          end
+
+          it 'includes URI of private pinned status' do
+            json = body_as_json
+            expect(json[:orderedItems]).to include(ActivityPub::TagManager.instance.uri_for(private_pinned))
+          end
+
+          it 'does not include contents of private pinned status' do
+            expect(response.body).not_to include(private_pinned.text)
           end
         end
 
diff --git a/spec/controllers/api/v1/accounts/statuses_controller_spec.rb b/spec/controllers/api/v1/accounts/statuses_controller_spec.rb
index 693cd1ac66b..0a18ddcbdd4 100644
--- a/spec/controllers/api/v1/accounts/statuses_controller_spec.rb
+++ b/spec/controllers/api/v1/accounts/statuses_controller_spec.rb
@@ -39,7 +39,7 @@ describe Api::V1::Accounts::StatusesController do
       end
     end
 
-    context 'with only pinned' do
+    context 'with only own pinned' do
       before do
         Fabricate(:status_pin, account: user.account, status: Fabricate(:status, account: user.account))
       end
@@ -50,5 +50,38 @@ describe Api::V1::Accounts::StatusesController do
         expect(response).to have_http_status(200)
       end
     end
+
+    context "with someone else's pinned statuses" do
+      let(:account)        { Fabricate(:account, username: 'bob', domain: 'example.com') }
+      let(:status)         { Fabricate(:status, account: account) }
+      let(:private_status) { Fabricate(:status, account: account, visibility: :private) }
+      let!(:pin)           { Fabricate(:status_pin, account: account, status: status) }
+      let!(:private_pin)   { Fabricate(:status_pin, account: account, status: private_status) }
+
+      it 'returns http success' do
+        get :index, params: { account_id: account.id, pinned: true }
+        expect(response).to have_http_status(200)
+      end
+
+      context 'when user does not follow account' do
+        it 'lists the public status only' do
+          get :index, params: { account_id: account.id, pinned: true }
+          json = body_as_json
+          expect(json.map { |item| item[:id].to_i }).to eq [status.id]
+        end
+      end
+
+      context 'when user follows account' do
+        before do
+          user.account.follow!(account)
+        end
+
+        it 'lists both the public and the private statuses' do
+          get :index, params: { account_id: account.id, pinned: true }
+          json = body_as_json
+          expect(json.map { |item| item[:id].to_i }.sort).to eq [status.id, private_status.id].sort
+        end
+      end
+    end
   end
 end
diff --git a/spec/lib/activitypub/activity/accept_spec.rb b/spec/lib/activitypub/activity/accept_spec.rb
index 883bab6ac92..304cf2208af 100644
--- a/spec/lib/activitypub/activity/accept_spec.rb
+++ b/spec/lib/activitypub/activity/accept_spec.rb
@@ -23,6 +23,7 @@ RSpec.describe ActivityPub::Activity::Accept do
     subject { described_class.new(json, sender) }
 
     before do
+      allow(RemoteAccountRefreshWorker).to receive(:perform_async)
       Fabricate(:follow_request, account: recipient, target_account: sender)
       subject.perform
     end
@@ -34,6 +35,10 @@ RSpec.describe ActivityPub::Activity::Accept do
     it 'removes the follow request' do
       expect(recipient.requested?(sender)).to be false
     end
+
+    it 'queues a refresh' do
+      expect(RemoteAccountRefreshWorker).to have_received(:perform_async).with(sender.id)
+    end
   end
 
   context 'given a relay' do
diff --git a/spec/lib/activitypub/activity/add_spec.rb b/spec/lib/activitypub/activity/add_spec.rb
index 16db71c8808..e6408b610f9 100644
--- a/spec/lib/activitypub/activity/add_spec.rb
+++ b/spec/lib/activitypub/activity/add_spec.rb
@@ -1,8 +1,8 @@
 require 'rails_helper'
 
 RSpec.describe ActivityPub::Activity::Add do
-  let(:sender) { Fabricate(:account, featured_collection_url: 'https://example.com/featured') }
-  let(:status) { Fabricate(:status, account: sender) }
+  let(:sender) { Fabricate(:account, featured_collection_url: 'https://example.com/featured', domain: 'example.com') }
+  let(:status) { Fabricate(:status, account: sender, visibility: :private) }
 
   let(:json) do
     {
@@ -24,6 +24,8 @@ RSpec.describe ActivityPub::Activity::Add do
     end
 
     context 'when status was not known before' do
+      let(:service_stub) {