Merge tag 'v2.4.5' into HEAD

Gemfile

@@ -41,7 +41,7 @@ gem 'omniauth-cas', '~> 1.1'
 gem 'omniauth-saml', '~> 1.10'
 gem 'omniauth', '~> 1.2'
 
-gem 'doorkeeper', '~> 4.2', '< 4.3'
+gem 'doorkeeper', '~> 5.0'
 gem 'fast_blank', '~> 1.0'
 gem 'fastimage'
 gem 'goldfinger', '~> 2.1'

Gemfile.lock

@@ -181,7 +181,7 @@ GEM
     docile (1.3.0)
     domain_name (0.5.20180417)
       unf (>= 0.0.5, < 1.0.0)
-    doorkeeper (4.2.6)
+    doorkeeper (5.0.0)
       railties (>= 4.2)
     dotenv (2.2.2)
     dotenv-rails (2.2.2)
@@ -269,7 +269,7 @@ GEM
     httplog (1.0.2)
       colorize (~> 0.8)
       rack (>= 1.0)
-    i18n (1.0.1)
+    i18n (1.1.0)
       concurrent-ruby (~> 1.0)
     i18n-tasks (0.9.21)
       activesupport (>= 4.0.2)
@@ -347,7 +347,7 @@ GEM
       net-ssh (>= 2.6.5)
     net-ssh (4.2.0)
     nio4r (2.3.0)
-    nokogiri (1.8.2)
+    nokogiri (1.8.4)
       mini_portile2 (~> 2.3.0)
     nokogumbo (1.5.0)
       nokogiri
@@ -415,7 +415,7 @@ GEM
     puma (3.11.4)
     pundit (1.1.0)
       activesupport (>= 3.0.0)
-    rack (2.0.4)
+    rack (2.0.5)
     rack-attack (5.2.0)
       rack
     rack-cors (1.0.2)
@@ -423,7 +423,7 @@ GEM
       rack
     rack-proxy (0.6.4)
       rack
-    rack-test (1.0.0)
+    rack-test (1.1.0)
       rack (>= 1.0, < 3)
     rails (5.2.0)
       actioncable (= 5.2.0)
@@ -670,7 +670,7 @@ DEPENDENCIES
   devise (~> 4.4)
   devise-two-factor (~> 3.0)
   devise_pam_authenticatable2 (~> 9.1)
-  doorkeeper (~> 4.2, < 4.3)
+  doorkeeper (~> 5.0)
   dotenv-rails (~> 2.2, < 2.3)
   fabrication (~> 2.20)
   faker (~> 1.8)
@@ -764,4 +764,4 @@ RUBY VERSION
    ruby 2.5.0p0
 
 BUNDLED WITH
-   1.16.2
+   1.16.3

app/helpers/jsonld_helper.rb

@@ -73,8 +73,10 @@ module JsonLdHelper
     end
   end
 
-  def body_to_json(body)
+  def body_to_json(body, compare_id: nil)
-    body.is_a?(String) ? Oj.load(body, mode: :strict) : body
+    json = body.is_a?(String) ? Oj.load(body, mode: :strict) : body
+    return if compare_id.present? && json['id'] != compare_id
+    json
   rescue Oj::ParseError
     nil
   end

app/lib/ostatus/activity/creation.rb

@@ -7,7 +7,7 @@ class OStatus::Activity::Creation < OStatus::Activity::Base
       return [nil, false]
     end
 
-    return [nil, false] if @account.suspended?
+    return [nil, false] if @account.suspended? || invalid_origin?
 
     RedisLock.acquire(lock_options) do |lock|
       if lock.acquired?
@@ -204,6 +204,15 @@ class OStatus::Activity::Creation < OStatus::Activity::Base
     end
   end
 
+  def invalid_origin?
+    return false unless id.start_with?('http') # Legacy IDs cannot be checked
+
+    needle = Addressable::URI.parse(id).normalized_host
+
+    !(needle.casecmp(@account.domain).zero? ||
+      needle.casecmp(Addressable::URI.parse(@account.remote_url.presence || @account.uri).normalized_host).zero?)
+  end
+
   def lock_options
     { redis: Redis.current, key: "create:#{id}" }
   end

app/services/activitypub/fetch_remote_account_service.rb

@@ -7,14 +7,14 @@ class ActivityPub::FetchRemoteAccountService < BaseService
 
   # Should be called when uri has already been checked for locality
   # Does a WebFinger roundtrip on each call
-  def call(uri, id: true, prefetched_body: nil)
+  def call(uri, id: true, prefetched_body: nil, break_on_redirect: false)
     @json = if prefetched_body.nil?
               fetch_resource(uri, id)
             else
-              body_to_json(prefetched_body)
+              body_to_json(prefetched_body, compare_id: id ? uri : nil)
             end
 
-    return unless supported_context? && expected_type?
+    return if !supported_context? || !expected_type? || (break_on_redirect && @json['movedTo'].present?)
 
     @uri      = @json['id']
     @username = @json['preferredUsername']

app/services/activitypub/fetch_remote_key_service.rb

@@ -17,7 +17,7 @@ class ActivityPub::FetchRemoteKeyService < BaseService
         @json = fetch_resource(uri, id)
       end
     else
-      @json = body_to_json(prefetched_body)
+      @json = body_to_json(prefetched_body, compare_id: id ? uri : nil)
     end
 
     return unless supported_context?(@json) && expected_type?

app/services/activitypub/fetch_remote_status_service.rb

@@ -8,7 +8,7 @@ class ActivityPub::FetchRemoteStatusService < BaseService
     @json = if prefetched_body.nil?
               fetch_resource(uri, id, on_behalf_of)
             else
-              body_to_json(prefetched_body)
+              body_to_json(prefetched_body, compare_id: id ? uri : nil)
             end
 
     return unless supported_context? && expected_type?

app/services/activitypub/process_account_service.rb

@@ -175,7 +175,7 @@ class ActivityPub::ProcessAccountService < BaseService
 
   def moved_account
     account   = ActivityPub::TagManager.instance.uri_to_resource(@json['movedTo'], Account)
-    account ||= ActivityPub::FetchRemoteAccountService.new.call(@json['movedTo'], id: true)
+    account ||= ActivityPub::FetchRemoteAccountService.new.call(@json['movedTo'], id: true, break_on_redirect: true)
     account
   end
 

app/services/fetch_remote_account_service.rb

@@ -27,7 +27,7 @@ class FetchRemoteAccountService < BaseService
 
     account = author_from_xml(xml.at_xpath('/xmlns:feed', xmlns: OStatus::TagManager::XMLNS), false)
 
-    UpdateRemoteProfileService.new.call(xml, account) unless account.nil?
+    UpdateRemoteProfileService.new.call(xml, account) if account.present? && trusted_domain?(url, account)
 
     account
   rescue TypeError
@@ -37,4 +37,9 @@ class FetchRemoteAccountService < BaseService
     Rails.logger.debug 'Invalid XML or missing namespace'
     nil
   end
+
+  def trusted_domain?(url, account)
+    domain = Addressable::URI.parse(url).normalized_host
+    domain.casecmp(account.domain).zero? || domain.casecmp(Addressable::URI.parse(account.remote_url.presence || account.uri).normalized_host).zero?
+  end
 end

db/migrate/20171201000000_change_account_id_nonnullable_in_lists.rb

@@ -1,5 +1,3 @@
-require Rails.root.join('lib', 'mastodon', 'migration_helpers')
-
 class ChangeAccountIdNonnullableInLists < ActiveRecord::Migration[5.1]
   def change
     change_column_null :lists, :account_id, false

db/migrate/20180514140000_revert_index_change_on_statuses_for_api_v1_accounts_account_id_statuses.rb

@@ -5,7 +5,7 @@ class RevertIndexChangeOnStatusesForApiV1AccountsAccountIdStatuses < ActiveRecor
 
   def change
     safety_assured do
-      add_index :statuses, [:account_id, :id, :visibility, :updated_at], order: { id: :desc }, algorithm: :concurrently, name: :index_statuses_20180106 unless index_exists?(:statuses, name: "index_statuses_20180106")
+      add_index :statuses, [:account_id, :id, :visibility, :updated_at], order: { id: :desc }, algorithm: :concurrently, name: :index_statuses_20180106 unless index_name_exists?(:statuses, "index_statuses_20180106")
     end
 
     # These index may not exists (see migration 20180514130000)

db/migrate/20180528141303_fix_accounts_unique_index.rb

@@ -1,4 +1,17 @@
 class FixAccountsUniqueIndex < ActiveRecord::Migration[5.2]
+  class Account < ApplicationRecord
+    # Dummy class, to make migration possible across version changes
+    has_one :user, inverse_of: :account
+
+    def local?
+      domain.nil?
+    end
+
+    def acct
+      local? ? username : "#{username}@#{domain}"
+    end
+  end
+
   disable_ddl_transaction!
 
   def up

db/migrate/20180814171349_add_confidential_to_doorkeeper_application.rb

@@ -0,0 +1,23 @@
+require Rails.root.join('lib', 'mastodon', 'migration_helpers')
+
+class AddConfidentialToDoorkeeperApplication < ActiveRecord::Migration[5.2]
+  include Mastodon::MigrationHelpers
+
+  disable_ddl_transaction!
+
+  def up
+    safety_assured do
+      add_column_with_default(
+        :oauth_applications,
+        :confidential,
+        :boolean,
+        allow_null: false,
+        default: true # maintaining backwards compatibility: require secrets
+      )
+    end
+  end
+
+  def down
+    remove_column :oauth_applications, :confidential
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2018_07_07_154237) do
+ActiveRecord::Schema.define(version: 2018_08_14_171349) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -339,6 +339,7 @@ ActiveRecord::Schema.define(version: 2018_07_07_154237) do
     t.string "website"
     t.string "owner_type"
     t.bigint "owner_id"
+    t.boolean "confidential", default: true, null: false
     t.index ["owner_id", "owner_type"], name: "index_oauth_applications_on_owner_id_and_owner_type"
     t.index ["uid"], name: "index_oauth_applications_on_uid", unique: true
   end

lib/mastodon/version.rb

@@ -13,7 +13,7 @@ module Mastodon
     end
 
     def patch
-      3
+      5
     end
 
     def pre

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

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

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

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