Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent silenced local users from notifying remote users not following them #10575

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 18 additions & 2 deletions app/lib/activitypub/tag_manager.rb
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,14 @@ def to(status)
when 'unlisted', 'private'
[account_followers_url(status.account)]
when 'direct', 'limited'
status.active_mentions.map { |mention| uri_for(mention.account) }
if status.account.silenced?
# Only notify followers if the account is locally silenced
account_ids = status.active_mentions.pluck(:account_id)
to = status.account.followers.where(id: account_ids).map { |account| uri_for(account) }
to.concat(FollowRequest.where(target_account_id: status.account_id, account_id: account_ids).map { |request| uri_for(request.account) })
else
status.active_mentions.map { |mention| uri_for(mention.account) }
end
end
end

Expand All @@ -86,7 +93,16 @@ def cc(status)
cc << COLLECTIONS[:public]
end

cc.concat(status.active_mentions.map { |mention| uri_for(mention.account) }) unless status.direct_visibility? || status.limited_visibility?
unless status.direct_visibility? || status.limited_visibility?
if status.account.silenced?
# Only notify followers if the account is locally silenced
account_ids = status.active_mentions.pluck(:account_id)
cc.concat(status.account.followers.where(id: account_ids).map { |account| uri_for(account) })
cc.concat(FollowRequest.where(target_account_id: status.account_id, account_id: account_ids).map { |request| uri_for(request.account) })
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's up with this line?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what you mean. It's to allow silenced local users to mention (and notify) remote users who have requested to follow them.
This is the same logic as in NotifyService for local notifications.

else
cc.concat(status.active_mentions.map { |mention| uri_for(mention.account) })
end
end

cc
end
Expand Down
32 changes: 32 additions & 0 deletions spec/lib/activitypub/tag_manager_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,22 @@
status.mentions.create(account: mentioned)
expect(subject.to(status)).to eq [subject.uri_for(mentioned)]
end

it "returns URIs of mentions for direct silenced author's status only if they are followers or requesting to be" do
bob = Fabricate(:account, username: 'bob')
alice = Fabricate(:account, username: 'alice')
foo = Fabricate(:account)
author = Fabricate(:account, username: 'author', silenced: true)
status = Fabricate(:status, visibility: :direct, account: author)
bob.follow!(author)
FollowRequest.create!(account: foo, target_account: author)
status.mentions.create(account: alice)
status.mentions.create(account: bob)
status.mentions.create(account: foo)
expect(subject.to(status)).to include(subject.uri_for(bob))
expect(subject.to(status)).to include(subject.uri_for(foo))
expect(subject.to(status)).to_not include(subject.uri_for(alice))
end
end

describe '#cc' do
Expand Down Expand Up @@ -70,6 +86,22 @@
status.mentions.create(account: mentioned)
expect(subject.cc(status)).to include(subject.uri_for(mentioned))
end

it "returns URIs of mentions for silenced author's non-direct status only if they are followers or requesting to be" do
bob = Fabricate(:account, username: 'bob')
alice = Fabricate(:account, username: 'alice')
foo = Fabricate(:account)
author = Fabricate(:account, username: 'author', silenced: true)
status = Fabricate(:status, visibility: :public, account: author)
bob.follow!(author)
FollowRequest.create!(account: foo, target_account: author)
status.mentions.create(account: alice)
status.mentions.create(account: bob)
status.mentions.create(account: foo)
expect(subject.cc(status)).to include(subject.uri_for(bob))
expect(subject.cc(status)).to include(subject.uri_for(foo))
expect(subject.cc(status)).to_not include(subject.uri_for(alice))
end
end

describe '#local_uri?' do
Expand Down
27 changes: 24 additions & 3 deletions spec/services/process_mentions_service_spec.rb
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
require 'rails_helper'

RSpec.describe ProcessMentionsService, type: :service do
let(:account) { Fabricate(:account, username: 'alice') }
let(:status) { Fabricate(:status, account: account, text: "Hello @#{remote_user.acct}") }
let(:account) { Fabricate(:account, username: 'alice') }
let(:visibility) { :public }
let(:status) { Fabricate(:status, account: account, text: "Hello @#{remote_user.acct}", visibility: visibility) }

context 'OStatus' do
context 'OStatus with public toot' do
let(:remote_user) { Fabricate(:account, username: 'remote_user', protocol: :ostatus, domain: 'example.com', salmon_url: 'http://salmon.example.com') }

subject { ProcessMentionsService.new }
Expand All @@ -23,6 +24,26 @@
end
end

context 'OStatus with private toot' do
let(:visibility) { :private }
let(:remote_user) { Fabricate(:account, username: 'remote_user', protocol: :ostatus, domain: 'example.com', salmon_url: 'http://salmon.example.com') }

subject { ProcessMentionsService.new }

before do
stub_request(:post, remote_user.salmon_url)
subject.call(status)
end

it 'does not create a mention' do
expect(remote_user.mentions.where(status: status).count).to eq 0
end

it 'does not post to remote user\'s Salmon end point' do
expect(a_request(:post, remote_user.salmon_url)).to_not have_been_made
end
end

context 'ActivityPub' do
let(:remote_user) { Fabricate(:account, username: 'remote_user', protocol: :activitypub, domain: 'example.com', inbox_url: 'http://example.com/inbox') }

Expand Down