Change silences to always require approval on follow (#11975)

* Change silenced accounts to require approval on follow

* Also require approval for follows by people explicitly muted by target accounts

* Do not auto-accept silenced or muted accounts when switching from locked to unlocked

* Add `follow_requests_count` to verify_credentials

* Show “Follow requests” menu item if needed even if account is locked

* Add tests

* Correctly reflect that follow requests weren't auto-accepted when local account is silenced

* Accept follow requests from user-muted accounts to avoid leaking mutes
This commit is contained in:
ThibG 2019-09-27 21:13:51 +02:00 committed by Eugen Rochko
parent 2f90a38f44
commit 18b451c0e6
9 changed files with 105 additions and 9 deletions

View File

@ -33,7 +33,7 @@ class Api::V1::AccountsController < Api::BaseController
def follow def follow
FollowService.new.call(current_user.account, @account, reblogs: truthy_param?(:reblogs)) FollowService.new.call(current_user.account, @account, reblogs: truthy_param?(:reblogs))
options = @account.locked? ? {} : { following_map: { @account.id => { reblogs: truthy_param?(:reblogs) } }, requested_map: { @account.id => false } } options = @account.locked? || current_user.account.silenced? ? {} : { following_map: { @account.id => { reblogs: truthy_param?(:reblogs) } }, requested_map: { @account.id => false } }
render json: @account, serializer: REST::RelationshipSerializer, relationships: relationships(options) render json: @account, serializer: REST::RelationshipSerializer, relationships: relationships(options)
end end

View File

@ -77,17 +77,15 @@ class GettingStarted extends ImmutablePureComponent {
}; };
componentDidMount () { componentDidMount () {
const { myAccount, fetchFollowRequests, multiColumn } = this.props; const { fetchFollowRequests, multiColumn } = this.props;
if (!multiColumn && window.innerWidth >= NAVIGATION_PANEL_BREAKPOINT) { if (!multiColumn && window.innerWidth >= NAVIGATION_PANEL_BREAKPOINT) {
this.context.router.history.replace('/timelines/home'); this.context.router.history.replace('/timelines/home');
return; return;
} }
if (myAccount.get('locked')) {
fetchFollowRequests(); fetchFollowRequests();
} }
}
render () { render () {
const { intl, myAccount, multiColumn, unreadFollowRequests } = this.props; const { intl, myAccount, multiColumn, unreadFollowRequests } = this.props;
@ -134,7 +132,7 @@ class GettingStarted extends ImmutablePureComponent {
height += 48*3; height += 48*3;
if (myAccount.get('locked')) { if (myAccount.get('locked') || unreadFollowRequests > 0) {
navItems.push(<ColumnLink key={i++} icon='user-plus' text={intl.formatMessage(messages.follow_requests)} badge={badgeDisplay(unreadFollowRequests, 40)} to='/follow_requests' />); navItems.push(<ColumnLink key={i++} icon='user-plus' text={intl.formatMessage(messages.follow_requests)} badge={badgeDisplay(unreadFollowRequests, 40)} to='/follow_requests' />);
height += 48; height += 48;
} }

View File

@ -21,7 +21,7 @@ class ActivityPub::Activity::Follow < ActivityPub::Activity
follow_request = FollowRequest.create!(account: @account, target_account: target_account, uri: @json['id']) follow_request = FollowRequest.create!(account: @account, target_account: target_account, uri: @json['id'])
if target_account.locked? if target_account.locked? || @account.silenced?
NotifyService.new.call(target_account, follow_request) NotifyService.new.call(target_account, follow_request)
else else
AuthorizeFollowService.new.call(@account, target_account) AuthorizeFollowService.new.call(@account, target_account)

View File

@ -12,6 +12,7 @@ class REST::CredentialAccountSerializer < REST::AccountSerializer
language: user.setting_default_language, language: user.setting_default_language,
note: object.note, note: object.note,
fields: object.fields.map(&:to_h), fields: object.fields.map(&:to_h),
follow_requests_count: FollowRequest.where(target_account: object).limit(40).count,
} }
end end
end end

View File

@ -30,7 +30,7 @@ class FollowService < BaseService
ActivityTracker.increment('activity:interactions') ActivityTracker.increment('activity:interactions')
if target_account.locked? || target_account.activitypub? if target_account.locked? || source_account.silenced? || target_account.activitypub?
request_follow(source_account, target_account, reblogs: reblogs) request_follow(source_account, target_account, reblogs: reblogs)
elsif target_account.local? elsif target_account.local?
direct_follow(source_account, target_account, reblogs: reblogs) direct_follow(source_account, target_account, reblogs: reblogs)

View File

@ -20,7 +20,9 @@ class UpdateAccountService < BaseService
private private
def authorize_all_follow_requests(account) def authorize_all_follow_requests(account)
AuthorizeFollowWorker.push_bulk(FollowRequest.where(target_account: account).select(:account_id, :target_account_id)) do |req| follow_requests = FollowRequest.where(target_account: account)
follow_requests = follow_requests.select { |req| !req.account.silenced? }
AuthorizeFollowWorker.push_bulk(follow_requests) do |req|
[req.account_id, req.target_account_id] [req.account_id, req.target_account_id]
end end
end end

View File

@ -31,6 +31,36 @@ RSpec.describe ActivityPub::Activity::Follow do
end end
end end
context 'silenced account following an unlocked account' do
before do
sender.touch(:silenced_at)
subject.perform
end
it 'does not create a follow from sender to recipient' do
expect(sender.following?(recipient)).to be false
end
it 'creates a follow request' do
expect(sender.requested?(recipient)).to be true
end
end
context 'unlocked account muting the sender' do
before do
recipient.mute!(sender)
subject.perform
end
it 'creates a follow from sender to recipient' do
expect(sender.following?(recipient)).to be true
end
it 'does not create a follow request' do
expect(sender.requested?(recipient)).to be false
end
end
context 'locked account' do context 'locked account' do
before do before do
recipient.update(locked: true) recipient.update(locked: true)

View File

@ -30,6 +30,33 @@ RSpec.describe FollowService, type: :service do
end end
end end
describe 'unlocked account, from silenced account' do
let(:bob) { Fabricate(:user, email: 'bob@example.com', account: Fabricate(:account, username: 'bob')).account }
before do
sender.touch(:silenced_at)
subject.call(sender, bob.acct)
end
it 'creates a follow request with reblogs' do
expect(FollowRequest.find_by(account: sender, target_account: bob, show_reblogs: true)).to_not be_nil
end
end
describe 'unlocked account, from a muted account' do
let(:bob) { Fabricate(:user, email: 'bob@example.com', account: Fabricate(:account, username: 'bob')).account }
before do
bob.mute!(sender)
subject.call(sender, bob.acct)
end
it 'creates a following relation with reblogs' do
expect(sender.following?(bob)).to be true
expect(sender.muting_reblogs?(bob)).to be false
end
end
describe 'unlocked account' do describe 'unlocked account' do
let(:bob) { Fabricate(:user, email: 'bob@example.com', account: Fabricate(:account, username: 'bob')).account } let(:bob) { Fabricate(:user, email: 'bob@example.com', account: Fabricate(:account, username: 'bob')).account }

View File

@ -0,0 +1,38 @@
require 'rails_helper'
RSpec.describe UpdateAccountService, type: :service do
subject { UpdateAccountService.new }
describe 'switching form locked to unlocked accounts' do
let(:account) { Fabricate(:account, locked: true) }
let(:alice) { Fabricate(:user, email: 'alice@example.com', account: Fabricate(:account, username: 'alice')).account }
let(:bob) { Fabricate(:user, email: 'bob@example.com', account: Fabricate(:account, username: 'bob')).account }
let(:eve) { Fabricate(:user, email: 'eve@example.com', account: Fabricate(:account, username: 'eve')).account }
before do
bob.touch(:silenced_at)
account.mute!(eve)
FollowService.new.call(alice, account)
FollowService.new.call(bob, account)
FollowService.new.call(eve, account)
subject.call(account, { locked: false })
end
it 'auto-accepts pending follow requests' do
expect(alice.following?(account)).to be true
expect(alice.requested?(account)).to be false
end
it 'does not auto-accept pending follow requests from silenced users' do
expect(bob.following?(account)).to be false
expect(bob.requested?(account)).to be true
end
it 'auto-accepts pending follow requests from muted users so as to not leak mute' do
expect(eve.following?(account)).to be true
expect(eve.requested?(account)).to be false
end
end
end