Improve federated ID validation (#8372)

* Fix URI not being sufficiently validated with prefetched JSON

* Add additional id validation to OStatus documents, when possible

4 files changed, 99 insertions, 2 deletions

diff --git a/spec/services/activitypub/fetch_remote_account_service_spec.rb b/spec/services/activitypub/fetch_remote_account_service_spec.rb
index dba55c034..aa13f0a9b 100644
--- a/spec/services/activitypub/fetch_remote_account_service_spec.rb
+++ b/spec/services/activitypub/fetch_remote_account_service_spec.rb
@@ -59,7 +59,6 @@ RSpec.describe ActivityPub::FetchRemoteAccountService, type: :service do
       it 'returns nil' do
         expect(account).to be_nil
       end
-
     end
 
     context 'when URI and WebFinger share the same host' do
@@ -119,5 +118,11 @@ RSpec.describe ActivityPub::FetchRemoteAccountService, type: :service do
 
       include_examples 'sets profile data'
     end
+
+    context 'with wrong id' do
+      it 'does not create account' do
+        expect(subject.call('https://fake.address/@foo', prefetched_body: Oj.dump(actor))).to be_nil
+      end
+    end
   end
 end
diff --git a/spec/services/activitypub/fetch_remote_status_service_spec.rb b/spec/services/activitypub/fetch_remote_status_service_spec.rb
index 549eb80fa..9ae409996 100644
--- a/spec/services/activitypub/fetch_remote_status_service_spec.rb
+++ b/spec/services/activitypub/fetch_remote_status_service_spec.rb
@@ -70,5 +70,27 @@ RSpec.describe ActivityPub::FetchRemoteStatusService, type: :service do
         expect(strip_tags(status.text)).to eq "Nyan Cat 10 hours remix https://#{valid_domain}/watch?v=12345"
       end
     end
+
+    context 'with wrong id' do
+      let(:note) do
+        {
+          '@context': 'https://www.w3.org/ns/activitystreams',
+          id: "https://real.address/@foo/1234",
+          type: 'Note',
+          content: 'Lorem ipsum',
+          attributedTo: ActivityPub::TagManager.instance.uri_for(sender),
+        }
+      end
+
+      let(:object) do
+        temp = note.dup
+        temp[:id] = 'https://fake.address/@foo/5678'
+        temp
+      end
+
+      it 'does not create status' do
+        expect(sender.statuses.first).to be_nil
+      end
+    end
   end
 end
diff --git a/spec/services/fetch_remote_account_service_spec.rb b/spec/services/fetch_remote_account_service_spec.rb
index 1c3abe8f3..20dd505d0 100644
--- a/spec/services/fetch_remote_account_service_spec.rb
+++ b/spec/services/fetch_remote_account_service_spec.rb
@@ -1,7 +1,7 @@
 require 'rails_helper'
 
 RSpec.describe FetchRemoteAccountService, type: :service do
-  let(:url) { 'https://example.com' }
+  let(:url) { 'https://example.com/alice' }
   let(:prefetched_body) { nil }
   let(:protocol) { :ostatus }
   subject { FetchRemoteAccountService.new.call(url, prefetched_body, protocol) }
@@ -46,6 +46,24 @@ RSpec.describe FetchRemoteAccountService, type: :service do
     end
 
     include_examples 'return Account'
+
+    it 'does not update account information if XML comes from an unverified domain' do
+      feed_xml = <<-XML.squish
+        <?xml version="1.0" encoding="UTF-8"?>
+        <feed xml:lang="en-US" xmlns="http://www.w3.org/2005/Atom" xmlns:thr="http://purl.org/syndication/thread/1.0" xmlns:georss="http://www.georss.org/georss" xmlns:activity="http://activitystrea.ms/spec/1.0/" xmlns:media="http://purl.org/syndication/atommedia" xmlns:poco="http://portablecontacts.net/spec/1.0" xmlns:ostatus="http://ostatus.org/schema/1.0" xmlns:statusnet="http://status.net/schema/api/1/">
+          <author>
+            <activity:object-type>http://activitystrea.ms/schema/1.0/person</activity:object-type>
+            <uri>http://kickass.zone/users/localhost</uri>
+            <name>localhost</name>
+            <poco:preferredUsername>localhost</poco:preferredUsername>
+            <poco:displayName>Villain!!!</poco:displayName>
+          </author>
+        </feed>
+      XML
+
+      returned_account = described_class.new.call('https://real-fake-domains.com/alice', feed_xml, :ostatus)
+      expect(returned_account.display_name).to_not eq 'Villain!!!'
+    end
   end
 
   context 'when prefetched_body is nil' do
diff --git a/spec/services/fetch_remote_status_service_spec.rb b/spec/services/fetch_remote_status_service_spec.rb
index 0df9c329a..f9db024b9 100644
--- a/spec/services/fetch_remote_status_service_spec.rb
+++ b/spec/services/fetch_remote_status_service_spec.rb
@@ -32,4 +32,56 @@ RSpec.describe FetchRemoteStatusService, type: :service do
       expect(status.text).to eq 'Lorem ipsum'
     end
   end
+
+  context 'protocol is :ostatus' do
+    subject { described_class.new }
+
+    before do
+      Fabricate(:account, username: 'tracer', domain: 'real.domain', remote_url: 'https://real.domain/users/tracer')
+    end
+
+    it 'does not create status with author at different domain' do
+      status_body = <<-XML.squish
+        <?xml version="1.0"?>
+        <entry xmlns="http://www.w3.org/2005/Atom" xmlns:thr="http://purl.org/syndication/thread/1.0" xmlns:activity="http://activitystrea.ms/spec/1.0/" xmlns:poco="http://portablecontacts.net/spec/1.0" xmlns:media="http://purl.org/syndication/atommedia" xmlns:ostatus="http://ostatus.org/schema/1.0" xmlns:mastodon="http://mastodon.social/schema/1.0">
+          <id>tag:real.domain,2017-04-27:objectId=4487555:objectType=Status</id>
+          <published>2017-04-27T13:49:25Z</published>
+          <updated>2017-04-27T13:49:25Z</updated>
+          <activity:object-type>http://activitystrea.ms/schema/1.0/note</activity:object-type>
+          <activity:verb>http://activitystrea.ms/schema/1.0/post</activity:verb>
+          <author>
+            <id>https://real.domain/users/tracer</id>
+            <activity:object-type>http://activitystrea.ms/schema/1.0/person</activity:object-type>
+            <uri>https://real.domain/users/tracer</uri>
+            <name>tracer</name>
+          </author>
+          <content type="html">Overwatch rocks</content>
+        </entry>
+      XML
+
+      expect(subject.call('https://fake.domain/foo', status_body, :ostatus)).to be_nil
+    end
+
+    it 'does not create status with wrong id when id uses http format' do
+      status_body = <<-XML.squish
+        <?xml version="1.0"?>
+        <entry xmlns="http://www.w3.org/2005/Atom" xmlns:thr="http://purl.org/syndication/thread/1.0" xmlns:activity="http://activitystrea.ms/spec/1.0/" xmlns:poco="http://portablecontacts.net/spec/1.0" xmlns:media="http://purl.org/syndication/atommedia" xmlns:ostatus="http://ostatus.org/schema/1.0" xmlns:mastodon="http://mastodon.social/schema/1.0">
+          <id>https://other-real.domain/statuses/123</id>
+          <published>2017-04-27T13:49:25Z</published>
+          <updated>2017-04-27T13:49:25Z</updated>
+          <activity:object-type>http://activitystrea.ms/schema/1.0/note</activity:object-type>
+          <activity:verb>http://activitystrea.ms/schema/1.0/post</activity:verb>
+          <author>
+            <id>https://real.domain/users/tracer</id>
+            <activity:object-type>http://activitystrea.ms/schema/1.0/person</activity:object-type>
+            <uri>https://real.domain/users/tracer</uri>
+            <name>tracer</name>
+          </author>
+          <content type="html">Overwatch rocks</content>
+        </entry>
+      XML
+
+      expect(subject.call('https://real.domain/statuses/456', status_body, :ostatus)).to be_nil
+    end
+  end
 end