Compare commits

...

10 commits

Author SHA1 Message Date
Eugen Rochko
add3b63a0c Bump version to 2.4.5 2018-08-24 20:11:40 +02:00
Eugen Rochko
221c8c771f Prevent ActivityPub movedTo recursion (#8092)
Fix #8051
2018-08-24 20:11:16 +02:00
ThibG
a16d41e9fb Fix FixAccountsUniqueIndex migration (#8212) 2018-08-24 20:10:18 +02:00
ThibG
f6ad5edbaa Make some migration script more robust (fixes #8007) (#8170)
Include a dummy Account class in the migration script containing only the
attributes relevant to the migration in order to not rely as much on the
codebase being in sync with the database schema.
2018-08-24 20:09:57 +02:00
Akihiko Odaki
3f4812c583 Fix index detection a migration to revert index change (#8026) 2018-08-24 20:09:13 +02:00
Eugen Rochko
86792bd309 Upgrade Doorkeeper to 5.0.0 (#8409)
See doorkeeper-gem/doorkeeper#1137
2018-08-24 20:06:50 +02:00
Eugen Rochko
612d02028c Bump version to 2.4.4 2018-08-22 20:56:43 +02:00
Eugen Rochko
f100e84372 Improve federated ID validation (#8372)
* Fix URI not being sufficiently validated with prefetched JSON

* Add additional id validation to OStatus documents, when possible
2018-08-22 20:56:26 +02:00
ThibG
31a209cb01 Upgrade doorkeeper to 4.4.2 (#8321) 2018-08-22 19:31:27 +02:00
Eugen Rochko
13a062a5d9 Upgrade Doorkeeper to 4.4.1 (#8197) 2018-08-22 19:30:02 +02:00
19 changed files with 173 additions and 25 deletions

View file

@ -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'

View file

@ -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

View file

@ -73,8 +73,10 @@ module JsonLdHelper
end
end
def body_to_json(body)
body.is_a?(String) ? Oj.load(body, mode: :strict) : body
def body_to_json(body, compare_id: nil)
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

View file

@ -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

View file

@ -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']

View file

@ -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?

View file

@ -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?

View file

@ -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

View file

@ -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

View file

@ -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

View file

@ -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)

View file

@ -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

View file

@ -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

View file

@ -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

View file

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

View file

@ -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

View file

@ -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

View file

@ -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

View file

@ -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