Add logging for rejected ActivityPub payloads and add tests (#10062)
This commit is contained in:
		
							parent
							
								
									3547e3e59e
								
							
						
					
					
						commit
						397f180493
					
				
					 5 changed files with 539 additions and 327 deletions
				
			
		|  | @ -180,4 +180,9 @@ class ActivityPub::Activity | ||||||
|   def requested_through_relay? |   def requested_through_relay? | ||||||
|     @options[:relayed_through_account] && Relay.find_by(inbox_url: @options[:relayed_through_account].inbox_url)&.enabled? |     @options[:relayed_through_account] && Relay.find_by(inbox_url: @options[:relayed_through_account].inbox_url)&.enabled? | ||||||
|   end |   end | ||||||
|  | 
 | ||||||
|  |   def reject_payload! | ||||||
|  |     Rails.logger.info("Rejected #{@json['type']} activity #{@json['id']} from #{@account.uri}#{@options[:relayed_through_account] && "via #{@options[:relayed_through_account].uri}"}") | ||||||
|  |     nil | ||||||
|  |   end | ||||||
| end | end | ||||||
|  |  | ||||||
|  | @ -2,9 +2,11 @@ | ||||||
| 
 | 
 | ||||||
| class ActivityPub::Activity::Announce < ActivityPub::Activity | class ActivityPub::Activity::Announce < ActivityPub::Activity | ||||||
|   def perform |   def perform | ||||||
|  |     return reject_payload! if delete_arrived_first?(@json['id']) || !related_to_local_activity? | ||||||
|  | 
 | ||||||
|     original_status = status_from_object |     original_status = status_from_object | ||||||
| 
 | 
 | ||||||
|     return if original_status.nil? || delete_arrived_first?(@json['id']) || !announceable?(original_status) || !related_to_local_activity? |     return reject_payload! if original_status.nil? || !announceable?(original_status) | ||||||
| 
 | 
 | ||||||
|     status = Status.find_by(account: @account, reblog: original_status) |     status = Status.find_by(account: @account, reblog: original_status) | ||||||
| 
 | 
 | ||||||
|  |  | ||||||
|  | @ -2,7 +2,7 @@ | ||||||
| 
 | 
 | ||||||
| class ActivityPub::Activity::Create < ActivityPub::Activity | class ActivityPub::Activity::Create < ActivityPub::Activity | ||||||
|   def perform |   def perform | ||||||
|     return if unsupported_object_type? || invalid_origin?(@object['id']) || Tombstone.exists?(uri: @object['id']) || !related_to_local_activity? |     return reject_payload! if unsupported_object_type? || invalid_origin?(@object['id']) || Tombstone.exists?(uri: @object['id']) || !related_to_local_activity? | ||||||
| 
 | 
 | ||||||
|     RedisLock.acquire(lock_options) do |lock| |     RedisLock.acquire(lock_options) do |lock| | ||||||
|       if lock.acquired? |       if lock.acquired? | ||||||
|  |  | ||||||
|  | @ -18,12 +18,13 @@ RSpec.describe ActivityPub::Activity::Announce do | ||||||
|   subject { described_class.new(json, sender) } |   subject { described_class.new(json, sender) } | ||||||
| 
 | 
 | ||||||
|   before do |   before do | ||||||
|     Fabricate(:account).follow!(sender) |  | ||||||
|     sender.update(uri: ActivityPub::TagManager.instance.uri_for(sender)) |     sender.update(uri: ActivityPub::TagManager.instance.uri_for(sender)) | ||||||
|   end |   end | ||||||
| 
 | 
 | ||||||
|   describe '#perform' do |   describe '#perform' do | ||||||
|  |     context 'when sender is followed by a local account' do | ||||||
|       before do |       before do | ||||||
|  |         Fabricate(:account).follow!(sender) | ||||||
|         subject.perform |         subject.perform | ||||||
|       end |       end | ||||||
| 
 | 
 | ||||||
|  | @ -68,4 +69,84 @@ RSpec.describe ActivityPub::Activity::Announce do | ||||||
|         end |         end | ||||||
|       end |       end | ||||||
|     end |     end | ||||||
|  | 
 | ||||||
|  |     context 'when the status belongs to a local user' do | ||||||
|  |       before do | ||||||
|  |         subject.perform | ||||||
|  |       end | ||||||
|  | 
 | ||||||
|  |       let(:object_json) do | ||||||
|  |         ActivityPub::TagManager.instance.uri_for(status) | ||||||
|  |       end | ||||||
|  | 
 | ||||||
|  |       it 'creates a reblog by sender of status' do | ||||||
|  |         expect(sender.reblogged?(status)).to be true | ||||||
|  |       end | ||||||
|  |     end | ||||||
|  | 
 | ||||||
|  |     context 'when the sender is relayed' do | ||||||
|  |       let!(:relay_account) { Fabricate(:account, inbox_url: 'https://relay.example.com/inbox') } | ||||||
|  |       let!(:relay) { Fabricate(:relay, inbox_url: 'https://relay.example.com/inbox') } | ||||||
|  | 
 | ||||||
|  |       subject { described_class.new(json, sender, relayed_through_account: relay_account) } | ||||||
|  | 
 | ||||||
|  |       context 'and the relay is enabled' do | ||||||
|  |         before do | ||||||
|  |           relay.update(state: :accepted) | ||||||
|  |           subject.perform | ||||||
|  |         end | ||||||
|  | 
 | ||||||
|  |         let(:object_json) do | ||||||
|  |           { | ||||||
|  |             id: [ActivityPub::TagManager.instance.uri_for(sender), '#bar'].join, | ||||||
|  |             type: 'Note', | ||||||
|  |             content: 'Lorem ipsum', | ||||||
|  |             to: 'http://example.com/followers', | ||||||
|  |           } | ||||||
|  |         end | ||||||
|  | 
 | ||||||
|  |         it 'creates a reblog by sender of status' do | ||||||
|  |           expect(sender.statuses.count).to eq 2 | ||||||
|  |         end | ||||||
|  |       end | ||||||
|  | 
 | ||||||
|  |       context 'and the relay is disabled' do | ||||||
|  |         before do | ||||||
|  |           subject.perform | ||||||
|  |         end | ||||||
|  | 
 | ||||||
|  |         let(:object_json) do | ||||||
|  |           { | ||||||
|  |             id: [ActivityPub::TagManager.instance.uri_for(sender), '#bar'].join, | ||||||
|  |             type: 'Note', | ||||||
|  |             content: 'Lorem ipsum', | ||||||
|  |             to: 'http://example.com/followers', | ||||||
|  |           } | ||||||
|  |         end | ||||||
|  | 
 | ||||||
|  |         it 'does not create anything' do | ||||||
|  |           expect(sender.statuses.count).to eq 0 | ||||||
|  |         end | ||||||
|  |       end | ||||||
|  |     end | ||||||
|  | 
 | ||||||
|  |     context 'when the sender has no relevance to local activity' do | ||||||
|  |       before do | ||||||
|  |         subject.perform | ||||||
|  |       end | ||||||
|  | 
 | ||||||
|  |       let(:object_json) do | ||||||
|  |         { | ||||||
|  |           id: [ActivityPub::TagManager.instance.uri_for(sender), '#bar'].join, | ||||||
|  |           type: 'Note', | ||||||
|  |           content: 'Lorem ipsum', | ||||||
|  |           to: 'http://example.com/followers', | ||||||
|  |         } | ||||||
|  |       end | ||||||
|  | 
 | ||||||
|  |       it 'does not create anything' do | ||||||
|  |         expect(sender.statuses.count).to eq 0 | ||||||
|  |       end | ||||||
|  |     end | ||||||
|  |   end | ||||||
| end | end | ||||||
|  |  | ||||||
|  | @ -13,8 +13,6 @@ RSpec.describe ActivityPub::Activity::Create do | ||||||
|     }.with_indifferent_access |     }.with_indifferent_access | ||||||
|   end |   end | ||||||
| 
 | 
 | ||||||
|   subject { described_class.new(json, sender) } |  | ||||||
| 
 |  | ||||||
|   before do |   before do | ||||||
|     sender.update(uri: ActivityPub::TagManager.instance.uri_for(sender)) |     sender.update(uri: ActivityPub::TagManager.instance.uri_for(sender)) | ||||||
| 
 | 
 | ||||||
|  | @ -23,6 +21,9 @@ RSpec.describe ActivityPub::Activity::Create do | ||||||
|   end |   end | ||||||
| 
 | 
 | ||||||
|   describe '#perform' do |   describe '#perform' do | ||||||
|  |     context 'when fetching' do | ||||||
|  |       subject { described_class.new(json, sender) } | ||||||
|  | 
 | ||||||
|       before do |       before do | ||||||
|         subject.perform |         subject.perform | ||||||
|       end |       end | ||||||
|  | @ -407,4 +408,127 @@ RSpec.describe ActivityPub::Activity::Create do | ||||||
|         end |         end | ||||||
|       end |       end | ||||||
|     end |     end | ||||||
|  | 
 | ||||||
|  |     context 'when sender is followed by local users' do | ||||||
|  |       subject { described_class.new(json, sender, delivery: true) } | ||||||
|  | 
 | ||||||
|  |       before do | ||||||
|  |         Fabricate(:account).follow!(sender) | ||||||
|  |         subject.perform | ||||||
|  |       end | ||||||
|  | 
 | ||||||
|  |       let(:object_json) do | ||||||
|  |         { | ||||||
|  |           id: [ActivityPub::TagManager.instance.uri_for(sender), '#bar'].join, | ||||||
|  |           type: 'Note', | ||||||
|  |           content: 'Lorem ipsum', | ||||||
|  |         } | ||||||
|  |       end | ||||||
|  | 
 | ||||||
|  |       it 'creates status' do | ||||||
|  |         status = sender.statuses.first | ||||||
|  | 
 | ||||||
|  |         expect(status).to_not be_nil | ||||||
|  |         expect(status.text).to eq 'Lorem ipsum' | ||||||
|  |       end | ||||||
|  |     end | ||||||
|  | 
 | ||||||
|  |     context 'when sender replies to local status' do | ||||||
|  |       let!(:local_status) { Fabricate(:status) } | ||||||
|  | 
 | ||||||
|  |       subject { described_class.new(json, sender, delivery: true) } | ||||||
|  | 
 | ||||||
|  |       before do | ||||||
|  |         subject.perform | ||||||
|  |       end | ||||||
|  | 
 | ||||||
|  |       let(:object_json) do | ||||||
|  |         { | ||||||
|  |           id: [ActivityPub::TagManager.instance.uri_for(sender), '#bar'].join, | ||||||
|  |           type: 'Note', | ||||||
|  |           content: 'Lorem ipsum', | ||||||
|  |           inReplyTo: ActivityPub::TagManager.instance.uri_for(local_status), | ||||||
|  |         } | ||||||
|  |       end | ||||||
|  | 
 | ||||||
|  |       it 'creates status' do | ||||||
|  |         status = sender.statuses.first | ||||||
|  | 
 | ||||||
|  |         expect(status).to_not be_nil | ||||||
|  |         expect(status.text).to eq 'Lorem ipsum' | ||||||
|  |       end | ||||||
|  |     end | ||||||
|  | 
 | ||||||
|  |     context 'when sender targets a local user' do | ||||||
|  |       let!(:local_account) { Fabricate(:account) } | ||||||
|  | 
 | ||||||
|  |       subject { described_class.new(json, sender, delivery: true) } | ||||||
|  | 
 | ||||||
|  |       before do | ||||||
|  |         subject.perform | ||||||
|  |       end | ||||||
|  | 
 | ||||||
|  |       let(:object_json) do | ||||||
|  |         { | ||||||
|  |           id: [ActivityPub::TagManager.instance.uri_for(sender), '#bar'].join, | ||||||
|  |           type: 'Note', | ||||||
|  |           content: 'Lorem ipsum', | ||||||
|  |           to: ActivityPub::TagManager.instance.uri_for(local_account), | ||||||
|  |         } | ||||||
|  |       end | ||||||
|  | 
 | ||||||
|  |       it 'creates status' do | ||||||
|  |         status = sender.statuses.first | ||||||
|  | 
 | ||||||
|  |         expect(status).to_not be_nil | ||||||
|  |         expect(status.text).to eq 'Lorem ipsum' | ||||||
|  |       end | ||||||
|  |     end | ||||||
|  | 
 | ||||||
|  |     context 'when sender cc\'s a local user' do | ||||||
|  |       let!(:local_account) { Fabricate(:account) } | ||||||
|  | 
 | ||||||
|  |       subject { described_class.new(json, sender, delivery: true) } | ||||||
|  | 
 | ||||||
|  |       before do | ||||||
|  |         subject.perform | ||||||
|  |       end | ||||||
|  | 
 | ||||||
|  |       let(:object_json) do | ||||||
|  |         { | ||||||
|  |           id: [ActivityPub::TagManager.instance.uri_for(sender), '#bar'].join, | ||||||
|  |           type: 'Note', | ||||||
|  |           content: 'Lorem ipsum', | ||||||
|  |           cc: ActivityPub::TagManager.instance.uri_for(local_account), | ||||||
|  |         } | ||||||
|  |       end | ||||||
|  | 
 | ||||||
|  |       it 'creates status' do | ||||||
|  |         status = sender.statuses.first | ||||||
|  | 
 | ||||||
|  |         expect(status).to_not be_nil | ||||||
|  |         expect(status.text).to eq 'Lorem ipsum' | ||||||
|  |       end | ||||||
|  |     end | ||||||
|  | 
 | ||||||
|  |     context 'when the sender has no relevance to local activity' do | ||||||
|  |       subject { described_class.new(json, sender, delivery: true) } | ||||||
|  | 
 | ||||||
|  |       before do | ||||||
|  |         subject.perform | ||||||
|  |       end | ||||||
|  | 
 | ||||||
|  |       let(:object_json) do | ||||||
|  |         { | ||||||
|  |           id: [ActivityPub::TagManager.instance.uri_for(sender), '#bar'].join, | ||||||
|  |           type: 'Note', | ||||||
|  |           content: 'Lorem ipsum', | ||||||
|  |         } | ||||||
|  |       end | ||||||
|  | 
 | ||||||
|  |       it 'does not create anything' do | ||||||
|  |         expect(sender.statuses.count).to eq 0 | ||||||
|  |       end | ||||||
|  |     end | ||||||
|  |   end | ||||||
| end | end | ||||||
|  |  | ||||||
		Loading…
	
		Reference in a new issue