From aecdaf5a8c001a6e0e75a20072564de754ab5f8b Mon Sep 17 00:00:00 2001 From: ThibG Date: Mon, 14 Sep 2020 13:04:29 +0200 Subject: [PATCH 01/20] Do not serve account actors at all in limited federation mode (#14800) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Do not serve account actors at all in limited federation mode When an account is fetched without a signature from an allowed instance, return an error. This isn't really an improvement in security, as the only information that was previously returned was required protocol-level info, and the only personal bit was the existence of the account. The existence of the account can still be checked by issuing a webfinger query, as those are accepted without signatures. However, this change makes it so that unallowed instances won't create account records on their end when they find a reference to an unknown account. The previous behavior of rendering a limited list of fields, instead of not rendering the actor at all, was in order to prevent situations in which two instances in Authorized Fetch mode or Limited Federation mode would fail to reach each other because resolving an account would require a signed query… from an account which can only be fetched with a signed query itself. However, this should now be fine as fetching accounts is done by signing on behalf of the special instance actor, which does not require any kind of valid signature to be fetched. * Fix tests --- app/controllers/accounts_controller.rb | 11 ++--------- spec/controllers/accounts_controller_spec.rb | 20 ++------------------ 2 files changed, 4 insertions(+), 27 deletions(-) diff --git a/app/controllers/accounts_controller.rb b/app/controllers/accounts_controller.rb index db77b628c..f5e82692e 100644 --- a/app/controllers/accounts_controller.rb +++ b/app/controllers/accounts_controller.rb @@ -7,6 +7,7 @@ class AccountsController < ApplicationController include AccountControllerConcern include SignatureAuthentication + before_action :require_signature!, if: -> { request.format == :json && authorized_fetch_mode? } before_action :set_cache_headers before_action :set_body_classes @@ -49,7 +50,7 @@ class AccountsController < ApplicationController format.json do expires_in 3.minutes, public: !(authorized_fetch_mode? && signed_request_account.present?) - render_with_cache json: @account, content_type: 'application/activity+json', serializer: ActivityPub::ActorSerializer, adapter: ActivityPub::Adapter, fields: restrict_fields_to + render_with_cache json: @account, content_type: 'application/activity+json', serializer: ActivityPub::ActorSerializer, adapter: ActivityPub::Adapter end end end @@ -149,12 +150,4 @@ class AccountsController < ApplicationController def params_slice(*keys) params.slice(*keys).permit(*keys) end - - def restrict_fields_to - if signed_request_account.present? || public_fetch_mode? - # Return all fields - else - %i(id type preferred_username inbox public_key endpoints) - end - end end diff --git a/spec/controllers/accounts_controller_spec.rb b/spec/controllers/accounts_controller_spec.rb index 93bf2c83f..b04f4650b 100644 --- a/spec/controllers/accounts_controller_spec.rb +++ b/spec/controllers/accounts_controller_spec.rb @@ -348,24 +348,8 @@ RSpec.describe AccountsController, type: :controller do context 'in authorized fetch mode' do let(:authorized_fetch_mode) { true } - it 'returns http success' do - expect(response).to have_http_status(200) - end - - it 'returns application/activity+json' do - expect(response.content_type).to eq 'application/activity+json' - end - - it_behaves_like 'cachable response' - - it 'returns Vary header with Signature' do - expect(response.headers['Vary']).to include 'Signature' - end - - it 'renders bare minimum account' do - json = body_as_json - expect(json).to include(:id, :type, :preferredUsername, :inbox, :publicKey) - expect(json).to_not include(:name, :summary) + it 'returns http unauthorized' do + expect(response).to have_http_status(401) end end end From 0abfa06b2f4d57363be8690aaf8e8ca3e1bfb221 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Sun, 30 Aug 2020 12:33:59 +0200 Subject: [PATCH 02/20] Fix inefficiencies in fan-out-on-write service (#14682) --- app/services/fan_out_on_write_service.rb | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/app/services/fan_out_on_write_service.rb b/app/services/fan_out_on_write_service.rb index 276eac0c1..21931c2f1 100644 --- a/app/services/fan_out_on_write_service.rb +++ b/app/services/fan_out_on_write_service.rb @@ -6,8 +6,6 @@ class FanOutOnWriteService < BaseService def call(status) raise Mastodon::RaceConditionError if status.visibility.nil? - render_anonymous_payload(status) - if status.direct_visibility? deliver_to_own_conversation(status) elsif status.limited_visibility? @@ -20,6 +18,8 @@ class FanOutOnWriteService < BaseService return if status.account.silenced? || !status.public_visibility? || status.reblog? + render_anonymous_payload(status) + deliver_to_hashtags(status) return if status.reply? && status.in_reply_to_account_id != status.account_id @@ -58,8 +58,10 @@ class FanOutOnWriteService < BaseService def deliver_to_mentioned_followers(status) Rails.logger.debug "Delivering status #{status.id} to limited followers" - FeedInsertWorker.push_bulk(status.mentions.includes(:account).map(&:account).select { |mentioned_account| mentioned_account.local? && mentioned_account.following?(status.account) }) do |follower| - [status.id, follower.id, :home] + status.mentions.joins(:account).merge(status.account.followers_for_local_distribution).select(:id).reorder(nil).find_in_batches do |followers| + FeedInsertWorker.push_bulk(followers) do |follower| + [status.id, follower.id, :home] + end end end From c98b7751ca6f7c638997c26b0807af5b51915593 Mon Sep 17 00:00:00 2001 From: Takeshi Umeda Date: Tue, 1 Sep 2020 01:11:27 +0900 Subject: [PATCH 03/20] Fix limited follower id in fan-out-on-write service (#14709) --- app/services/fan_out_on_write_service.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/services/fan_out_on_write_service.rb b/app/services/fan_out_on_write_service.rb index 21931c2f1..e05d02cef 100644 --- a/app/services/fan_out_on_write_service.rb +++ b/app/services/fan_out_on_write_service.rb @@ -58,9 +58,9 @@ class FanOutOnWriteService < BaseService def deliver_to_mentioned_followers(status) Rails.logger.debug "Delivering status #{status.id} to limited followers" - status.mentions.joins(:account).merge(status.account.followers_for_local_distribution).select(:id).reorder(nil).find_in_batches do |followers| - FeedInsertWorker.push_bulk(followers) do |follower| - [status.id, follower.id, :home] + status.mentions.joins(:account).merge(status.account.followers_for_local_distribution).select(:id, :account_id).reorder(nil).find_in_batches do |mentions| + FeedInsertWorker.push_bulk(mentions) do |mention| + [status.id, mention.account_id, :home] end end end From 4acfc3ce83a0f7492137ef0a3b0c78cce0773e6e Mon Sep 17 00:00:00 2001 From: ThibG Date: Sat, 1 Aug 2020 18:20:37 +0200 Subject: [PATCH 04/20] Fix handling of Reject Follow when a matching follow relationship exists (#14479) * Add tests * Fix handling of Reject Follow when a matching follow relationship exists Regression from #12199 --- app/lib/activitypub/activity/reject.rb | 2 +- spec/lib/activitypub/activity/reject_spec.rb | 110 ++++++++++++++++--- 2 files changed, 98 insertions(+), 14 deletions(-) diff --git a/app/lib/activitypub/activity/reject.rb b/app/lib/activitypub/activity/reject.rb index 8d771ed81..886dddb23 100644 --- a/app/lib/activitypub/activity/reject.rb +++ b/app/lib/activitypub/activity/reject.rb @@ -4,7 +4,7 @@ class ActivityPub::Activity::Reject < ActivityPub::Activity def perform return reject_follow_for_relay if relay_follow? return follow_request_from_object.reject! unless follow_request_from_object.nil? - return UnfollowService.new.call(follow_from_object.target_account, @account) unless follow_from_object.nil? + return UnfollowService.new.call(follow_from_object.account, @account) unless follow_from_object.nil? case @object['type'] when 'Follow' diff --git a/spec/lib/activitypub/activity/reject_spec.rb b/spec/lib/activitypub/activity/reject_spec.rb index e7205df8d..fed4cd8cd 100644 --- a/spec/lib/activitypub/activity/reject_spec.rb +++ b/spec/lib/activitypub/activity/reject_spec.rb @@ -3,6 +3,14 @@ require 'rails_helper' RSpec.describe ActivityPub::Activity::Reject do let(:sender) { Fabricate(:account) } let(:recipient) { Fabricate(:account) } + let(:object_json) do + { + id: 'bar', + type: 'Follow', + actor: ActivityPub::TagManager.instance.uri_for(recipient), + object: ActivityPub::TagManager.instance.uri_for(sender), + } + end let(:json) do { @@ -10,29 +18,105 @@ RSpec.describe ActivityPub::Activity::Reject do id: 'foo', type: 'Reject', actor: ActivityPub::TagManager.instance.uri_for(sender), - object: { - id: 'bar', - type: 'Follow', - actor: ActivityPub::TagManager.instance.uri_for(recipient), - object: ActivityPub::TagManager.instance.uri_for(sender), - }, + object: object_json, }.with_indifferent_access end describe '#perform' do subject { described_class.new(json, sender) } - before do - Fabricate(:follow_request, account: recipient, target_account: sender) - subject.perform + context 'rejecting a pending follow request by target' do + before do + Fabricate(:follow_request, account: recipient, target_account: sender) + subject.perform + end + + it 'does not create a follow relationship' do + expect(recipient.following?(sender)).to be false + end + + it 'removes the follow request' do + expect(recipient.requested?(sender)).to be false + end end - it 'does not create a follow relationship' do - expect(recipient.following?(sender)).to be false + context 'rejecting a pending follow request by uri' do + before do + Fabricate(:follow_request, account: recipient, target_account: sender, uri: 'bar') + subject.perform + end + + it 'does not create a follow relationship' do + expect(recipient.following?(sender)).to be false + end + + it 'removes the follow request' do + expect(recipient.requested?(sender)).to be false + end end - it 'removes the follow request' do - expect(recipient.requested?(sender)).to be false + context 'rejecting a pending follow request by uri only' do + let(:object_json) { 'bar' } + + before do + Fabricate(:follow_request, account: recipient, target_account: sender, uri: 'bar') + subject.perform + end + + it 'does not create a follow relationship' do + expect(recipient.following?(sender)).to be false + end + + it 'removes the follow request' do + expect(recipient.requested?(sender)).to be false + end + end + + context 'rejecting an existing follow relationship by target' do + before do + Fabricate(:follow, account: recipient, target_account: sender) + subject.perform + end + + it 'removes the follow relationship' do + expect(recipient.following?(sender)).to be false + end + + it 'does not create a follow request' do + expect(recipient.requested?(sender)).to be false + end + end + + context 'rejecting an existing follow relationship by uri' do + before do + Fabricate(:follow, account: recipient, target_account: sender, uri: 'bar') + subject.perform + end + + it 'removes the follow relationship' do + expect(recipient.following?(sender)).to be false + end + + it 'does not create a follow request' do + expect(recipient.requested?(sender)).to be false + end + end + + context 'rejecting an existing follow relationship by uri only' do + let(:object_json) { 'bar' } + + before do + Fabricate(:follow, account: recipient, target_account: sender, uri: 'bar') + subject.perform + end + + it 'removes the follow relationship' do + expect(recipient.following?(sender)).to be false + end + + it 'does not create a follow request' do + expect(recipient.requested?(sender)).to be false + end end end From 8f79ed0487fb17ad59182b49b3fbe46043cbaedd Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Sun, 13 Sep 2020 12:52:17 +0200 Subject: [PATCH 05/20] Fix reported statuses not being included in warning e-mail (#14778) --- app/models/admin/account_action.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/admin/account_action.rb b/app/models/admin/account_action.rb index b30a82369..9edd152f5 100644 --- a/app/models/admin/account_action.rb +++ b/app/models/admin/account_action.rb @@ -142,7 +142,7 @@ class Admin::AccountAction end def status_ids - @report.status_ids if @report && include_statuses + report.status_ids if report && include_statuses end def reports From ce6aaed4325d1a5dc15a799856d26b3d22222633 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Thu, 8 Oct 2020 00:34:57 +0200 Subject: [PATCH 06/20] Remove dependency on goldfinger gem (#14919) There are edge cases where requests to certain hosts timeout when using the vanilla HTTP.rb gem, which the goldfinger gem uses. Now that we no longer need to support OStatus servers, webfinger logic is so simple that there is no point encapsulating it in a gem, so we can just use our own Request class. With that, we benefit from more robust timeout code and IPv4/IPv6 resolution. Fix #14091 --- Gemfile | 1 - Gemfile.lock | 6 -- app/helpers/webfinger_helper.rb | 33 +------ app/lib/webfinger.rb | 93 +++++++++++++++++++ app/models/account_alias.rb | 2 +- app/models/account_migration.rb | 2 +- app/models/form/redirect.rb | 2 +- app/models/remote_follow.rb | 10 +- .../fetch_remote_account_service.rb | 7 +- app/services/process_mentions_service.rb | 2 +- app/services/resolve_account_service.rb | 9 +- app/views/well_known/host_meta/show.xml.ruby | 1 - .../remote_follow_controller_spec.rb | 10 +- .../well_known/host_meta_controller_spec.rb | 2 +- spec/lib/feed_manager_spec.rb | 1 + 15 files changed, 114 insertions(+), 67 deletions(-) create mode 100644 app/lib/webfinger.rb diff --git a/Gemfile b/Gemfile index 414bd2c30..0891371be 100644 --- a/Gemfile +++ b/Gemfile @@ -54,7 +54,6 @@ gem 'doorkeeper', '~> 5.4' gem 'ed25519', '~> 1.2' gem 'fast_blank', '~> 1.0' gem 'fastimage' -gem 'goldfinger', '~> 2.1' gem 'hiredis', '~> 0.6' gem 'redis-namespace', '~> 1.7' gem 'health_check', git: 'https://github.com/ianheggie/health_check', ref: '0b799ead604f900ed50685e9b2d469cd2befba5b' diff --git a/Gemfile.lock b/Gemfile.lock index 3d4fce643..62c20bf07 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -250,11 +250,6 @@ GEM ruby-progressbar (~> 1.4) globalid (0.4.2) activesupport (>= 4.2.0) - goldfinger (2.1.1) - addressable (~> 2.5) - http (~> 4.0) - nokogiri (~> 1.8) - oj (~> 3.0) hamlit (2.11.0) temple (>= 0.8.2) thor @@ -708,7 +703,6 @@ DEPENDENCIES fog-core (<= 2.1.0) fog-openstack (~> 0.3) fuubar (~> 2.5) - goldfinger (~> 2.1) hamlit-rails (~> 0.2) health_check! hiredis (~> 0.6) diff --git a/app/helpers/webfinger_helper.rb b/app/helpers/webfinger_helper.rb index ab7ca4698..482f4e19e 100644 --- a/app/helpers/webfinger_helper.rb +++ b/app/helpers/webfinger_helper.rb @@ -1,38 +1,7 @@ # frozen_string_literal: true -# Monkey-patch on monkey-patch. -# Because it conflicts with the request.rb patch. -class HTTP::Timeout::PerOperationOriginal < HTTP::Timeout::PerOperation - def connect(socket_class, host, port, nodelay = false) - ::Timeout.timeout(@connect_timeout, HTTP::TimeoutError) do - @socket = socket_class.open(host, port) - @socket.setsockopt(Socket::IPPROTO_TCP, Socket::TCP_NODELAY, 1) if nodelay - end - end -end - module WebfingerHelper def webfinger!(uri) - hidden_service_uri = /\.(onion|i2p)(:\d+)?$/.match(uri) - - raise Mastodon::HostValidationError, 'Instance does not support hidden service connections' if !Rails.configuration.x.access_to_hidden_service && hidden_service_uri - - opts = { - ssl: !hidden_service_uri, - - headers: { - 'User-Agent': Mastodon::Version.user_agent, - }, - - timeout_class: HTTP::Timeout::PerOperationOriginal, - - timeout_options: { - write_timeout: 10, - connect_timeout: 5, - read_timeout: 10, - }, - } - - Goldfinger::Client.new(uri, opts.merge(Rails.configuration.x.http_client_proxy)).finger + Webfinger.new(uri).perform end end diff --git a/app/lib/webfinger.rb b/app/lib/webfinger.rb new file mode 100644 index 000000000..b2374c494 --- /dev/null +++ b/app/lib/webfinger.rb @@ -0,0 +1,93 @@ +# frozen_string_literal: true + +class Webfinger + class Error < StandardError; end + + class Response + def initialize(body) + @json = Oj.load(body, mode: :strict) + end + + def subject + @json['subject'] + end + + def link(rel, attribute) + links.dig(rel, attribute) + end + + private + + def links + @links ||= @json['links'].map { |link| [link['rel'], link] }.to_h + end + end + + def initialize(uri) + _, @domain = uri.split('@') + + raise ArgumentError, 'Webfinger requested for local account' if @domain.nil? + + @uri = uri + end + + def perform + Response.new(body_from_webfinger) + rescue Oj::ParseError + raise Webfinger::Error, "Invalid JSON in response for #{@uri}" + rescue Addressable::URI::InvalidURIError + raise Webfinger::Error, "Invalid URI for #{@uri}" + end + + private + + def body_from_webfinger(url = standard_url, use_fallback = true) + webfinger_request(url).perform do |res| + if res.code == 200 + res.body_with_limit + elsif res.code == 404 && use_fallback + body_from_host_meta + else + raise Webfinger::Error, "Request for #{@uri} returned HTTP #{res.code}" + end + end + end + + def body_from_host_meta + host_meta_request.perform do |res| + if res.code == 200 + body_from_webfinger(url_from_template(res.body_with_limit), false) + else + raise Webfinger::Error, "Request for #{@uri} returned HTTP #{res.code}" + end + end + end + + def url_from_template(str) + link = Nokogiri::XML(str).at_xpath('//xmlns:Link[@rel="lrdd"]') + + if link.present? + link['template'].gsub('{uri}', @uri) + else + raise Webfinger::Error, "Request for #{@uri} returned host-meta without link to Webfinger" + end + rescue Nokogiri::XML::XPath::SyntaxError + raise Webfinger::Error, "Invalid XML encountered in host-meta for #{@uri}" + end + + def host_meta_request + Request.new(:get, host_meta_url).add_headers('Accept' => 'application/xrd+xml, application/xml, text/xml') + end + + def webfinger_request(url) + Request.new(:get, url).add_headers('Accept' => 'application/jrd+json, application/json') + end + + def standard_url + "https://#{@domain}/.well-known/webfinger?resource=#{@uri}" + end + + def host_meta_url + "https://#{@domain}/.well-known/host-meta" + end +end diff --git a/app/models/account_alias.rb b/app/models/account_alias.rb index 792e9e8d4..3d659142a 100644 --- a/app/models/account_alias.rb +++ b/app/models/account_alias.rb @@ -33,7 +33,7 @@ class AccountAlias < ApplicationRecord def set_uri target_account = ResolveAccountService.new.call(acct) self.uri = ActivityPub::TagManager.instance.uri_for(target_account) unless target_account.nil? - rescue Goldfinger::Error, HTTP::Error, OpenSSL::SSL::SSLError, Mastodon::Error + rescue Webfinger::Error, HTTP::Error, OpenSSL::SSL::SSLError, Mastodon::Error # Validation will take care of it end diff --git a/app/models/account_migration.rb b/app/models/account_migration.rb index 681b5b2cd..4fae98ed7 100644 --- a/app/models/account_migration.rb +++ b/app/models/account_migration.rb @@ -54,7 +54,7 @@ class AccountMigration < ApplicationRecord def set_target_account self.target_account = ResolveAccountService.new.call(acct) - rescue Goldfinger::Error, HTTP::Error, OpenSSL::SSL::SSLError, Mastodon::Error + rescue Webfinger::Error, HTTP::Error, OpenSSL::SSL::SSLError, Mastodon::Error # Validation will take care of it end diff --git a/app/models/form/redirect.rb b/app/models/form/redirect.rb index a7961f8e8..19ee9faed 100644 --- a/app/models/form/redirect.rb +++ b/app/models/form/redirect.rb @@ -32,7 +32,7 @@ class Form::Redirect def set_target_account @target_account = ResolveAccountService.new.call(acct) - rescue Goldfinger::Error, HTTP::Error, OpenSSL::SSL::SSLError, Mastodon::Error + rescue Webfinger::Error, HTTP::Error, OpenSSL::SSL::SSLError, Mastodon::Error # Validation will take care of it end diff --git a/app/models/remote_follow.rb b/app/models/remote_follow.rb index 30b84f7d5..911c06713 100644 --- a/app/models/remote_follow.rb +++ b/app/models/remote_follow.rb @@ -56,7 +56,7 @@ class RemoteFollow if domain.nil? @addressable_template = Addressable::Template.new("#{authorize_interaction_url}?uri={uri}") - elsif redirect_url_link.nil? || redirect_url_link.template.nil? + elsif redirect_uri_template.nil? missing_resource_error else @addressable_template = Addressable::Template.new(redirect_uri_template) @@ -64,16 +64,12 @@ class RemoteFollow end def redirect_uri_template - redirect_url_link.template - end - - def redirect_url_link - acct_resource&.link('http://ostatus.org/schema/1.0/subscribe') + acct_resource&.link('http://ostatus.org/schema/1.0/subscribe', 'template') end def acct_resource @acct_resource ||= webfinger!("acct:#{acct}") - rescue Goldfinger::Error, HTTP::ConnectionError + rescue Webfinger::Error, HTTP::ConnectionError nil end diff --git a/app/services/activitypub/fetch_remote_account_service.rb b/app/services/activitypub/fetch_remote_account_service.rb index 83fbf6d07..e5bd0c47c 100644 --- a/app/services/activitypub/fetch_remote_account_service.rb +++ b/app/services/activitypub/fetch_remote_account_service.rb @@ -39,17 +39,16 @@ class ActivityPub::FetchRemoteAccountService < BaseService webfinger = webfinger!("acct:#{@username}@#{@domain}") confirmed_username, confirmed_domain = split_acct(webfinger.subject) - return webfinger.link('self')&.href == @uri if @username.casecmp(confirmed_username).zero? && @domain.casecmp(confirmed_domain).zero? + return webfinger.link('self', 'href') == @uri if @username.casecmp(confirmed_username).zero? && @domain.casecmp(confirmed_domain).zero? webfinger = webfinger!("acct:#{confirmed_username}@#{confirmed_domain}") @username, @domain = split_acct(webfinger.subject) - self_reference = webfinger.link('self') return false unless @username.casecmp(confirmed_username).zero? && @domain.casecmp(confirmed_domain).zero? - return false if self_reference&.href != @uri + return false if webfinger.link('self', 'href') != @uri true - rescue Goldfinger::Error + rescue Webfinger::Error false end diff --git a/app/services/process_mentions_service.rb b/app/services/process_mentions_service.rb index 79af3fc54..8e260811d 100644 --- a/app/services/process_mentions_service.rb +++ b/app/services/process_mentions_service.rb @@ -29,7 +29,7 @@ class ProcessMentionsService < BaseService if mention_undeliverable?(mentioned_account) begin mentioned_account = resolve_account_service.call(Regexp.last_match(1)) - rescue Goldfinger::Error, HTTP::Error, OpenSSL::SSL::SSLError, Mastodon::UnexpectedResponseError + rescue Webfinger::Error, HTTP::Error, OpenSSL::SSL::SSLError, Mastodon::UnexpectedResponseError mentioned_account = nil end end diff --git a/app/services/resolve_account_service.rb b/app/services/resolve_account_service.rb index ba77552c6..3f7bb7cc5 100644 --- a/app/services/resolve_account_service.rb +++ b/app/services/resolve_account_service.rb @@ -26,11 +26,10 @@ class ResolveAccountService < BaseService @account ||= Account.find_remote(@username, @domain) - return @account if @account&.local? || !webfinger_update_due? + return @account if @account&.local? || @domain.nil? || !webfinger_update_due? # At this point we are in need of a Webfinger query, which may # yield us a different username/domain through a redirect - process_webfinger!(@uri) # Because the username/domain pair may be different than what @@ -47,7 +46,7 @@ class ResolveAccountService < BaseService # either needs to be created, or updated from fresh data process_account! - rescue Goldfinger::Error, WebfingerRedirectError, Oj::ParseError => e + rescue Webfinger::Error, WebfingerRedirectError, Oj::ParseError => e Rails.logger.debug "Webfinger query for #{@uri} failed: #{e}" nil end @@ -118,11 +117,11 @@ class ResolveAccountService < BaseService end def activitypub_ready? - !@webfinger.link('self').nil? && ['application/activity+json', 'application/ld+json; profile="https://www.w3.org/ns/activitystreams"'].include?(@webfinger.link('self').type) + ['application/activity+json', 'application/ld+json; profile="https://www.w3.org/ns/activitystreams"'].include?(@webfinger.link('self', 'type')) end def actor_url - @actor_url ||= @webfinger.link('self').href + @actor_url ||= @webfinger.link('self', 'href') end def actor_json diff --git a/app/views/well_known/host_meta/show.xml.ruby b/app/views/well_known/host_meta/show.xml.ruby index 0a6bdc322..b4e867c5f 100644 --- a/app/views/well_known/host_meta/show.xml.ruby +++ b/app/views/well_known/host_meta/show.xml.ruby @@ -5,7 +5,6 @@ doc << Ox::Element.new('XRD').tap do |xrd| xrd << Ox::Element.new('Link').tap do |link| link['rel'] = 'lrdd' - link['type'] = 'application/xrd+xml' link['template'] = @webfinger_template end end diff --git a/spec/controllers/remote_follow_controller_spec.rb b/spec/controllers/remote_follow_controller_spec.rb index 3ef8f14d9..7312dde58 100644 --- a/spec/controllers/remote_follow_controller_spec.rb +++ b/spec/controllers/remote_follow_controller_spec.rb @@ -43,8 +43,7 @@ describe RemoteFollowController do end it 'renders new when template is nil' do - link_with_nil_template = double(template: nil) - resource_with_link = double(link: link_with_nil_template) + resource_with_link = double(link: nil) allow_any_instance_of(WebfingerHelper).to receive(:webfinger!).with('acct:user@example.com').and_return(resource_with_link) post :create, params: { account_username: @account.to_param, remote_follow: { acct: 'user@example.com' } } @@ -55,8 +54,7 @@ describe RemoteFollowController do context 'when webfinger values are good' do before do - link_with_template = double(template: 'http://example.com/follow_me?acct={uri}') - resource_with_link = double(link: link_with_template) + resource_with_link = double(link: 'http://example.com/follow_me?acct={uri}') allow_any_instance_of(WebfingerHelper).to receive(:webfinger!).with('acct:user@example.com').and_return(resource_with_link) post :create, params: { account_username: @account.to_param, remote_follow: { acct: 'user@example.com' } } end @@ -78,8 +76,8 @@ describe RemoteFollowController do expect(response).to render_template(:new) end - it 'renders new with error when goldfinger fails' do - allow_any_instance_of(WebfingerHelper).to receive(:webfinger!).with('acct:user@example.com').and_raise(Goldfinger::Error) + it 'renders new with error when webfinger fails' do + allow_any_instance_of(WebfingerHelper).to receive(:webfinger!).with('acct:user@example.com').and_raise(Webfinger::Error) post :create, params: { account_username: @account.to_param, remote_follow: { acct: 'user@example.com' } } expect(response).to render_template(:new) diff --git a/spec/controllers/well_known/host_meta_controller_spec.rb b/spec/controllers/well_known/host_meta_controller_spec.rb index b43ae19d8..643ba9cd3 100644 --- a/spec/controllers/well_known/host_meta_controller_spec.rb +++ b/spec/controllers/well_known/host_meta_controller_spec.rb @@ -12,7 +12,7 @@ describe WellKnown::HostMetaController, type: :controller do expect(response.body).to eq < - + XML end diff --git a/spec/lib/feed_manager_spec.rb b/spec/lib/feed_manager_spec.rb index 22eddd2ab..2d1f6cf96 100644 --- a/spec/lib/feed_manager_spec.rb +++ b/spec/lib/feed_manager_spec.rb @@ -108,6 +108,7 @@ RSpec.describe FeedManager do it 'returns false for status by followee mentioning another account' do bob.follow!(alice) + jeff.follow!(alice) status = PostStatusService.new.call(alice, text: 'Hey @jeff') expect(FeedManager.instance.filter?(:home, status, bob.id)).to be false end From 3f4cceebd66c0e209239bf5a917bbda8de57d189 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Sun, 30 Aug 2020 01:54:30 +0200 Subject: [PATCH 07/20] Fix videos with near-60 fps being rejected (#14684) Fix #14668 --- app/models/media_attachment.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/media_attachment.rb b/app/models/media_attachment.rb index 3d93ec75b..663bb0896 100644 --- a/app/models/media_attachment.rb +++ b/app/models/media_attachment.rb @@ -338,7 +338,7 @@ class MediaAttachment < ApplicationRecord raise Mastodon::StreamValidationError, 'Video has no video stream' if movie.width.nil? || movie.frame_rate.nil? raise Mastodon::DimensionsValidationError, "#{movie.width}x#{movie.height} videos are not supported" if movie.width * movie.height > MAX_VIDEO_MATRIX_LIMIT - raise Mastodon::DimensionsValidationError, "#{movie.frame_rate.to_i}fps videos are not supported" if movie.frame_rate > MAX_VIDEO_FRAME_RATE + raise Mastodon::DimensionsValidationError, "#{movie.frame_rate.floor}fps videos are not supported" if movie.frame_rate.floor > MAX_VIDEO_FRAME_RATE end def set_meta From 58c59af573d7cb285317bdb27d745b38cf045378 Mon Sep 17 00:00:00 2001 From: Takeshi Umeda Date: Tue, 25 Aug 2020 01:09:46 +0900 Subject: [PATCH 08/20] Fix an error when file_file_size is nil in tootctl media remove (#14657) --- lib/mastodon/media_cli.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/mastodon/media_cli.rb b/lib/mastodon/media_cli.rb index 2a4e3e379..45181f59e 100644 --- a/lib/mastodon/media_cli.rb +++ b/lib/mastodon/media_cli.rb @@ -31,7 +31,7 @@ module Mastodon processed, aggregate = parallelize_with_progress(MediaAttachment.cached.where.not(remote_url: '').where('created_at < ?', time_ago)) do |media_attachment| next if media_attachment.file.blank? - size = media_attachment.file_file_size + (media_attachment.thumbnail_file_size || 0) + size = (media_attachment.file_file_size || 0) + (media_attachment.thumbnail_file_size || 0) unless options[:dry_run] media_attachment.file.destroy From 856cb96a2b4823b62df19f67686921890adfc2f8 Mon Sep 17 00:00:00 2001 From: ThibG Date: Sun, 2 Aug 2020 11:20:17 +0200 Subject: [PATCH 09/20] Fix new audio player features not working on Safari (#14465) Fixes #14462 --- app/javascript/mastodon/features/audio/index.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/javascript/mastodon/features/audio/index.js b/app/javascript/mastodon/features/audio/index.js index 1ab1c3117..a4e00ba96 100644 --- a/app/javascript/mastodon/features/audio/index.js +++ b/app/javascript/mastodon/features/audio/index.js @@ -269,8 +269,9 @@ class Audio extends React.PureComponent { } _initAudioContext () { - const context = new AudioContext(); - const source = context.createMediaElementSource(this.audio); + const AudioContext = window.AudioContext || window.webkitAudioContext; + const context = new AudioContext(); + const source = context.createMediaElementSource(this.audio); this.visualizer.setAudioContext(context, source); source.connect(context.destination); From 399c5f09009e05d22e9acd8bb75f3f803b58e365 Mon Sep 17 00:00:00 2001 From: ThibG Date: Sun, 2 Aug 2020 11:21:10 +0200 Subject: [PATCH 10/20] Change content-type to be always computed from file data (#14452) * Change content-type to be always computed from file data Restore previous behavior, detecting the content-type isn't very expensive, and some instances may serve files as application/octet-stream regardless of their true type, making fetching media from them fail, while it used to work pre-3.2.0. * Add test --- lib/paperclip/response_with_limit_adapter.rb | 2 +- spec/lib/activitypub/activity/create_spec.rb | 27 ++++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/lib/paperclip/response_with_limit_adapter.rb b/lib/paperclip/response_with_limit_adapter.rb index 7d897b8d6..8711b1349 100644 --- a/lib/paperclip/response_with_limit_adapter.rb +++ b/lib/paperclip/response_with_limit_adapter.rb @@ -19,7 +19,7 @@ module Paperclip @original_filename = filename_from_content_disposition || filename_from_path || 'data' @size = @target.response.content_length @tempfile = copy_to_tempfile(@target) - @content_type = @target.response.mime_type || ContentTypeDetector.new(@tempfile.path).detect + @content_type = ContentTypeDetector.new(@tempfile.path).detect end def copy_to_tempfile(source) diff --git a/spec/lib/activitypub/activity/create_spec.rb b/spec/lib/activitypub/activity/create_spec.rb index 2ac4acc12..51e0b8caf 100644 --- a/spec/lib/activitypub/activity/create_spec.rb +++ b/spec/lib/activitypub/activity/create_spec.rb @@ -18,6 +18,7 @@ RSpec.describe ActivityPub::Activity::Create do stub_request(:get, 'http://example.com/attachment.png').to_return(request_fixture('avatar.txt')) stub_request(:get, 'http://example.com/emoji.png').to_return(body: attachment_fixture('emojo.png')) + stub_request(:get, 'http://example.com/emojib.png').to_return(body: attachment_fixture('emojo.png'), headers: { 'Content-Type' => 'application/octet-stream' }) end describe '#perform' do @@ -451,6 +452,32 @@ RSpec.describe ActivityPub::Activity::Create do end end + context 'with emojis served with invalid content-type' do + let(:object_json) do + { + id: [ActivityPub::TagManager.instance.uri_for(sender), '#bar'].join, + type: 'Note', + content: 'Lorem ipsum :tinkong:', + tag: [ + { + type: 'Emoji', + icon: { + url: 'http://example.com/emojib.png', + }, + name: 'tinkong', + }, + ], + } + end + + it 'creates status' do + status = sender.statuses.first + + expect(status).to_not be_nil + expect(status.emojis.map(&:shortcode)).to include('tinkong') + end + end + context 'with emojis missing name' do let(:object_json) do { From 469c4c78a3ce2f7065c7273fd2800f9a39191a21 Mon Sep 17 00:00:00 2001 From: ThibG Date: Sun, 2 Aug 2020 18:47:09 +0200 Subject: [PATCH 11/20] Fix audio player on Safari (#14485) --- app/javascript/mastodon/features/audio/index.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/javascript/mastodon/features/audio/index.js b/app/javascript/mastodon/features/audio/index.js index a4e00ba96..5b8172694 100644 --- a/app/javascript/mastodon/features/audio/index.js +++ b/app/javascript/mastodon/features/audio/index.js @@ -115,6 +115,10 @@ class Audio extends React.PureComponent { } togglePlay = () => { + if (!this.audioContext) { + this._initAudioContext(); + } + if (this.state.paused) { this.setState({ paused: false }, () => this.audio.play()); } else { @@ -133,10 +137,6 @@ class Audio extends React.PureComponent { handlePlay = () => { this.setState({ paused: false }); - if (this.canvas && !this.audioContext) { - this._initAudioContext(); - } - if (this.audioContext && this.audioContext.state === 'suspended') { this.audioContext.resume(); } From 1995a5cb34337d18ba305c56715194fbaa68786e Mon Sep 17 00:00:00 2001 From: ThibG Date: Sun, 2 Aug 2020 19:03:10 +0200 Subject: [PATCH 12/20] Fix audio/video player not using CDN_HOST in media paths on public pages (#14486) --- app/views/statuses/_detailed_status.html.haml | 4 ++-- app/views/statuses/_simple_status.html.haml | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/views/statuses/_detailed_status.html.haml b/app/views/statuses/_detailed_status.html.haml index 85b2ceea4..b3e9c44fc 100644 --- a/app/views/statuses/_detailed_status.html.haml +++ b/app/views/statuses/_detailed_status.html.haml @@ -29,11 +29,11 @@ - if !status.media_attachments.empty? - if status.media_attachments.first.video? - video = status.media_attachments.first - = react_component :video, src: video.file.url(:original), preview: video.thumbnail.present? ? video.thumbnail.url : video.file.url(:small), blurhash: video.blurhash, sensitive: status.sensitive?, width: 670, height: 380, detailed: true, inline: true, alt: video.description do + = react_component :video, src: full_asset_url(video.file.url(:original)), preview: full_asset_url(video.thumbnail.present? ? video.thumbnail.url : video.file.url(:small)), blurhash: video.blurhash, sensitive: status.sensitive?, width: 670, height: 380, detailed: true, inline: true, alt: video.description do = render partial: 'statuses/attachment_list', locals: { attachments: status.media_attachments } - elsif status.media_attachments.first.audio? - audio = status.media_attachments.first - = react_component :audio, src: audio.file.url(:original), poster: audio.thumbnail.present? ? audio.thumbnail.url : status.account.avatar_static_url, backgroundColor: audio.file.meta.dig('colors', 'background'), foregroundColor: audio.file.meta.dig('colors', 'foreground'), accentColor: audio.file.meta.dig('colors', 'accent'), width: 670, height: 380, alt: audio.description, duration: audio.file.meta.dig('original', 'duration') do + = react_component :audio, src: full_asset_url(audio.file.url(:original)), poster: full_asset_url(audio.thumbnail.present? ? audio.thumbnail.url : status.account.avatar_static_url), backgroundColor: audio.file.meta.dig('colors', 'background'), foregroundColor: audio.file.meta.dig('colors', 'foreground'), accentColor: audio.file.meta.dig('colors', 'accent'), width: 670, height: 380, alt: audio.description, duration: audio.file.meta.dig('original', 'duration') do = render partial: 'statuses/attachment_list', locals: { attachments: status.media_attachments } - else = react_component :media_gallery, height: 380, sensitive: status.sensitive?, standalone: true, autoplay: autoplay, media: status.media_attachments.map { |a| ActiveModelSerializers::SerializableResource.new(a, serializer: REST::MediaAttachmentSerializer).as_json } do diff --git a/app/views/statuses/_simple_status.html.haml b/app/views/statuses/_simple_status.html.haml index 67c6c0fd0..363757945 100644 --- a/app/views/statuses/_simple_status.html.haml +++ b/app/views/statuses/_simple_status.html.haml @@ -35,11 +35,11 @@ - if !status.media_attachments.empty? - if status.media_attachments.first.video? - video = status.media_attachments.first - = react_component :video, src: video.file.url(:original), preview: video.thumbnail.present? ? video.thumbnail.url : video.file.url(:small), blurhash: video.blurhash, sensitive: status.sensitive?, width: 610, height: 343, inline: true, alt: video.description do + = react_component :video, src: full_asset_url(video.file.url(:original)), preview: full_asset_url(video.thumbnail.present? ? video.thumbnail.url : video.file.url(:small)), blurhash: video.blurhash, sensitive: status.sensitive?, width: 610, height: 343, inline: true, alt: video.description do = render partial: 'statuses/attachment_list', locals: { attachments: status.media_attachments } - elsif status.media_attachments.first.audio? - audio = status.media_attachments.first - = react_component :audio, src: audio.file.url(:original), poster: audio.thumbnail.present? ? audio.thumbnail.url : status.account.avatar_static_url, backgroundColor: audio.file.meta.dig('colors', 'background'), foregroundColor: audio.file.meta.dig('colors', 'foreground'), accentColor: audio.file.meta.dig('colors', 'accent'), width: 610, height: 343, alt: audio.description, duration: audio.file.meta.dig('original', 'duration') do + = react_component :audio, src: full_asset_url(audio.file.url(:original)), poster: full_asset_url(audio.thumbnail.present? ? audio.thumbnail.url : status.account.avatar_static_url), backgroundColor: audio.file.meta.dig('colors', 'background'), foregroundColor: audio.file.meta.dig('colors', 'foreground'), accentColor: audio.file.meta.dig('colors', 'accent'), width: 610, height: 343, alt: audio.description, duration: audio.file.meta.dig('original', 'duration') do = render partial: 'statuses/attachment_list', locals: { attachments: status.media_attachments } - else = react_component :media_gallery, height: 343, sensitive: status.sensitive?, autoplay: autoplay, media: status.media_attachments.map { |a| ActiveModelSerializers::SerializableResource.new(a, serializer: REST::MediaAttachmentSerializer).as_json } do From 3b699f17320de7fc1d1adc40e8edbd8ee58c9d57 Mon Sep 17 00:00:00 2001 From: ThibG Date: Sun, 2 Aug 2020 18:47:44 +0200 Subject: [PATCH 13/20] Fix thumbnail color extraction (#14464) * Fix contrast calculation for thumbnail color extraction Luminance calculation was using 0-255 RGB values instead of 0-1 sRGB values, leading to incorrectly-computed contrast values. Since we use ColorDiff already, just use its XYZ colorspace conversion code to get the value. * Require at least 3:1 contrast for both accent and foreground colors * Lower required contrast for the accent color --- lib/paperclip/color_extractor.rb | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/paperclip/color_extractor.rb b/lib/paperclip/color_extractor.rb index 44fe5ff1d..c8bb771a0 100644 --- a/lib/paperclip/color_extractor.rb +++ b/lib/paperclip/color_extractor.rb @@ -5,6 +5,7 @@ require 'mime/types/columnar' module Paperclip class ColorExtractor < Paperclip::Processor MIN_CONTRAST = 3.0 + ACCENT_MIN_CONTRAST = 2.0 FREQUENCY_THRESHOLD = 0.01 def make @@ -26,8 +27,9 @@ module Paperclip foreground_palette.each do |color| distance = ColorDiff.between(background_color, color) + contrast = w3c_contrast(background_color, color) - if distance > max_distance + if distance > max_distance && contrast >= ACCENT_MIN_CONTRAST max_distance = distance max_distance_color = color end @@ -77,8 +79,8 @@ module Paperclip private def w3c_contrast(color1, color2) - luminance1 = (0.2126 * color1.r + 0.7152 * color1.g + 0.0722 * color1.b) + 0.05 - luminance2 = (0.2126 * color2.r + 0.7152 * color2.g + 0.0722 * color2.b) + 0.05 + luminance1 = color1.to_xyz.y * 0.01 + 0.05 + luminance2 = color2.to_xyz.y * 0.01 + 0.05 if luminance1 > luminance2 luminance1 / luminance2 From 6db143e424b7566519153e6a0c831cd77ceff227 Mon Sep 17 00:00:00 2001 From: ThibG Date: Sat, 8 Aug 2020 17:57:56 +0200 Subject: [PATCH 14/20] Fix crash when failing to load emoji picker (#14525) Fixes #14523 --- .../features/compose/components/emoji_picker_dropdown.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/javascript/mastodon/features/compose/components/emoji_picker_dropdown.js b/app/javascript/mastodon/features/compose/components/emoji_picker_dropdown.js index 360a7af6a..e8a36a923 100644 --- a/app/javascript/mastodon/features/compose/components/emoji_picker_dropdown.js +++ b/app/javascript/mastodon/features/compose/components/emoji_picker_dropdown.js @@ -315,7 +315,7 @@ class EmojiPickerDropdown extends React.PureComponent { this.setState({ loading: false }); }).catch(() => { - this.setState({ loading: false }); + this.setState({ loading: false, active: false }); }); } From 8b448aecef9495353a1cd18d9e5d95b576cdede2 Mon Sep 17 00:00:00 2001 From: ThibG Date: Mon, 10 Aug 2020 01:51:06 +0200 Subject: [PATCH 15/20] Fix `tootctl media` commands not handling snowflake ids for media_attachments (#14536) --- lib/mastodon/media_cli.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/mastodon/media_cli.rb b/lib/mastodon/media_cli.rb index 45181f59e..54da5b2cd 100644 --- a/lib/mastodon/media_cli.rb +++ b/lib/mastodon/media_cli.rb @@ -89,7 +89,7 @@ module Mastodon path_segments = object.key.split('/') path_segments.delete('cache') - if path_segments.size != 7 + unless [7, 10].include?(path_segments.size) progress.log(pastel.yellow("Unrecognized file found: #{object.key}")) next end @@ -133,7 +133,7 @@ module Mastodon path_segments = key.split(File::SEPARATOR) path_segments.delete('cache') - if path_segments.size != 7 + unless [7, 10].include?(path_segments.size) progress.log(pastel.yellow("Unrecognized file found: #{key}")) next end @@ -258,7 +258,7 @@ module Mastodon path_segments = path.split('/')[2..-1] path_segments.delete('cache') - if path_segments.size != 7 + unless [7, 10].include?(path_segments.size) say('Not a media URL', :red) exit(1) end @@ -311,7 +311,7 @@ module Mastodon segments = object.key.split('/') segments.delete('cache') - next if segments.size != 7 + next unless [7, 10].include?(segments.size) model_name = segments.first.classify record_id = segments[2..-2].join.to_i From aea0161e83ba0d154a3b3824e4d14d31773486b0 Mon Sep 17 00:00:00 2001 From: ThibG Date: Mon, 24 Aug 2020 14:11:47 +0200 Subject: [PATCH 16/20] Add support for inlined objects in activity audience (#14514) * Add support for inlined objects in activity audience * Add tests --- app/lib/activitypub/activity.rb | 2 +- app/lib/activitypub/activity/announce.rb | 14 +++++++++--- app/lib/activitypub/activity/create.rb | 16 +++++++------- .../lib/activitypub/activity/announce_spec.rb | 20 +++++++++++++++++ spec/lib/activitypub/activity/create_spec.rb | 22 +++++++++++++++++++ 5 files changed, 62 insertions(+), 12 deletions(-) diff --git a/app/lib/activitypub/activity.rb b/app/lib/activitypub/activity.rb index ab946470b..f0ef4d553 100644 --- a/app/lib/activitypub/activity.rb +++ b/app/lib/activitypub/activity.rb @@ -172,7 +172,7 @@ class ActivityPub::Activity end def first_mentioned_local_account - audience = (as_array(@json['to']) + as_array(@json['cc'])).uniq + audience = (as_array(@json['to']) + as_array(@json['cc'])).map { |x| value_or_id(x) }.uniq local_usernames = audience.select { |uri| ActivityPub::TagManager.instance.local_uri?(uri) } .map { |uri| ActivityPub::TagManager.instance.uri_to_local_id(uri, :username) } diff --git a/app/lib/activitypub/activity/announce.rb b/app/lib/activitypub/activity/announce.rb index 9e108985a..349e8f77e 100644 --- a/app/lib/activitypub/activity/announce.rb +++ b/app/lib/activitypub/activity/announce.rb @@ -34,12 +34,20 @@ class ActivityPub::Activity::Announce < ActivityPub::Activity private + def audience_to + as_array(@json['to']).map { |x| value_or_id(x) } + end + + def audience_cc + as_array(@json['cc']).map { |x| value_or_id(x) } + end + def visibility_from_audience - if equals_or_includes?(@json['to'], ActivityPub::TagManager::COLLECTIONS[:public]) + if audience_to.include?(ActivityPub::TagManager::COLLECTIONS[:public]) :public - elsif equals_or_includes?(@json['cc'], ActivityPub::TagManager::COLLECTIONS[:public]) + elsif audience_cc.include?(ActivityPub::TagManager::COLLECTIONS[:public]) :unlisted - elsif equals_or_includes?(@json['to'], @account.followers_url) + elsif audience_to.include?(@account.followers_url) :private else :direct diff --git a/app/lib/activitypub/activity/create.rb b/app/lib/activitypub/activity/create.rb index 08dd98e94..a60b79d15 100644 --- a/app/lib/activitypub/activity/create.rb +++ b/app/lib/activitypub/activity/create.rb @@ -65,11 +65,11 @@ class ActivityPub::Activity::Create < ActivityPub::Activity end def audience_to - @object['to'] || @json['to'] + as_array(@object['to'] || @json['to']).map { |x| value_or_id(x) } end def audience_cc - @object['cc'] || @json['cc'] + as_array(@object['cc'] || @json['cc']).map { |x| value_or_id(x) } end def process_status @@ -122,7 +122,7 @@ class ActivityPub::Activity::Create < ActivityPub::Activity end def process_audience - (as_array(audience_to) + as_array(audience_cc)).uniq.each do |audience| + (audience_to + audience_cc).uniq.each do |audience| next if audience == ActivityPub::TagManager::COLLECTIONS[:public] # Unlike with tags, there is no point in resolving accounts we don't already @@ -352,11 +352,11 @@ class ActivityPub::Activity::Create < ActivityPub::Activity end def visibility_from_audience - if equals_or_includes?(audience_to, ActivityPub::TagManager::COLLECTIONS[:public]) + if audience_to.include?(ActivityPub::TagManager::COLLECTIONS[:public]) :public - elsif equals_or_includes?(audience_cc, ActivityPub::TagManager::COLLECTIONS[:public]) + elsif audience_cc.include?(ActivityPub::TagManager::COLLECTIONS[:public]) :unlisted - elsif equals_or_includes?(audience_to, @account.followers_url) + elsif audience_to.include?(@account.followers_url) :private else :direct @@ -365,7 +365,7 @@ class ActivityPub::Activity::Create < ActivityPub::Activity def audience_includes?(account) uri = ActivityPub::TagManager.instance.uri_for(account) - equals_or_includes?(audience_to, uri) || equals_or_includes?(audience_cc, uri) + audience_to.include?(uri) || audience_cc.include?(uri) end def replied_to_status @@ -477,7 +477,7 @@ class ActivityPub::Activity::Create < ActivityPub::Activity def addresses_local_accounts? return true if @options[:delivered_to_account_id] - local_usernames = (as_array(audience_to) + as_array(audience_cc)).uniq.select { |uri| ActivityPub::TagManager.instance.local_uri?(uri) }.map { |uri| ActivityPub::TagManager.instance.uri_to_local_id(uri, :username) } + local_usernames = (audience_to + audience_cc).uniq.select { |uri| ActivityPub::TagManager.instance.local_uri?(uri) }.map { |uri| ActivityPub::TagManager.instance.uri_to_local_id(uri, :username) } return false if local_usernames.empty? diff --git a/spec/lib/activitypub/activity/announce_spec.rb b/spec/lib/activitypub/activity/announce_spec.rb index 60fd96a18..b93fcbe66 100644 --- a/spec/lib/activitypub/activity/announce_spec.rb +++ b/spec/lib/activitypub/activity/announce_spec.rb @@ -73,6 +73,26 @@ RSpec.describe ActivityPub::Activity::Announce do expect(sender.reblogged?(sender.statuses.first)).to be true end end + + context 'self-boost of a previously unknown status with correct attributedTo, inlined Collection in audience' do + let(:object_json) do + { + id: 'https://example.com/actor#bar', + type: 'Note', + content: 'Lorem ipsum', + attributedTo: 'https://example.com/actor', + to: { + 'type': 'OrderedCollection', + 'id': 'http://example.com/followers', + 'first': 'http://example.com/followers?page=true', + } + } + end + + it 'creates a reblog by sender of status' do + expect(sender.reblogged?(sender.statuses.first)).to be true + end + end end context 'when the status belongs to a local user' do diff --git a/spec/lib/activitypub/activity/create_spec.rb b/spec/lib/activitypub/activity/create_spec.rb index 51e0b8caf..d2e9fe33c 100644 --- a/spec/lib/activitypub/activity/create_spec.rb +++ b/spec/lib/activitypub/activity/create_spec.rb @@ -121,6 +121,28 @@ RSpec.describe ActivityPub::Activity::Create do end end + context 'private with inlined Collection in audience' do + let(:object_json) do + { + id: [ActivityPub::TagManager.instance.uri_for(sender), '#bar'].join, + type: 'Note', + content: 'Lorem ipsum', + to: { + 'type': 'OrderedCollection', + 'id': 'http://example.com/followers', + 'first': 'http://example.com/followers?page=true', + } + } + end + + it 'creates status' do + status = sender.statuses.first + + expect(status).to_not be_nil + expect(status.visibility).to eq 'private' + end + end + context 'limited' do let(:recipient) { Fabricate(:account) } From dd3a86eb04d7445e32df44b66ec34332b78b7902 Mon Sep 17 00:00:00 2001 From: Tdxdxoz Date: Mon, 24 Aug 2020 20:13:44 +0800 Subject: [PATCH 17/20] Fix: also use custom private boost icon for detailed status (#14471) * use custom private boost icon for detail status * only use className --- app/javascript/mastodon/components/status_action_bar.js | 3 ++- .../mastodon/features/status/components/action_bar.js | 3 ++- app/javascript/styles/mastodon/boost.scss | 2 +- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/app/javascript/mastodon/components/status_action_bar.js b/app/javascript/mastodon/components/status_action_bar.js index 231c517e9..b47855a7e 100644 --- a/app/javascript/mastodon/components/status_action_bar.js +++ b/app/javascript/mastodon/components/status_action_bar.js @@ -7,6 +7,7 @@ import DropdownMenuContainer from '../containers/dropdown_menu_container'; import { defineMessages, injectIntl } from 'react-intl'; import ImmutablePureComponent from 'react-immutable-pure-component'; import { me, isStaff } from '../initial_state'; +import classNames from 'classnames'; const messages = defineMessages({ delete: { id: 'status.delete', defaultMessage: 'Delete' }, @@ -329,7 +330,7 @@ class StatusActionBar extends ImmutablePureComponent { return (
{obfuscatedCount(status.get('replies_count'))}
- + {shareButton} diff --git a/app/javascript/mastodon/features/status/components/action_bar.js b/app/javascript/mastodon/features/status/components/action_bar.js index 1c5d5ca0c..d0f1c57d0 100644 --- a/app/javascript/mastodon/features/status/components/action_bar.js +++ b/app/javascript/mastodon/features/status/components/action_bar.js @@ -6,6 +6,7 @@ import ImmutablePropTypes from 'react-immutable-proptypes'; import DropdownMenuContainer from '../../../containers/dropdown_menu_container'; import { defineMessages, injectIntl } from 'react-intl'; import { me, isStaff } from '../../../initial_state'; +import classNames from 'classnames'; const messages = defineMessages({ delete: { id: 'status.delete', defaultMessage: 'Delete' }, @@ -273,7 +274,7 @@ class ActionBar extends React.PureComponent { return (
-
+
{shareButton}
diff --git a/app/javascript/styles/mastodon/boost.scss b/app/javascript/styles/mastodon/boost.scss index 3489428f8..0be3533bc 100644 --- a/app/javascript/styles/mastodon/boost.scss +++ b/app/javascript/styles/mastodon/boost.scss @@ -6,7 +6,7 @@ button.icon-button i.fa-retweet { } } -.status-private button.icon-button i.fa-retweet { +button.icon-button.reblogPrivate i.fa-retweet { background-image: url("data:image/svg+xml;utf8,"); &:hover { From aa98655cf61e732fb3cfe7626347b79189f61b77 Mon Sep 17 00:00:00 2001 From: ThibG Date: Mon, 24 Aug 2020 16:56:21 +0200 Subject: [PATCH 18/20] Fix dereferencing remote statuses not using the correct account (#14656) Follow-up to #14359 In the case of limited toots, the receiver may not be explicitly part of the audience. If a specific user's inbox URI was specified, it makes sense to dereference the toot from the corresponding user, instead of trying to find someone in the explicit audience. --- app/lib/activitypub/activity.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/lib/activitypub/activity.rb b/app/lib/activitypub/activity.rb index f0ef4d553..a379a7ef4 100644 --- a/app/lib/activitypub/activity.rb +++ b/app/lib/activitypub/activity.rb @@ -168,6 +168,8 @@ class ActivityPub::Activity end def signed_fetch_account + return Account.find(@options[:delivered_to_account_id]) if @options[:delivered_to_account_id].present? + first_mentioned_local_account || first_local_follower end From 4ea7193f0a65a28886b954e99733cc42e6b9f572 Mon Sep 17 00:00:00 2001 From: ThibG Date: Mon, 24 Aug 2020 18:21:07 +0200 Subject: [PATCH 19/20] Add support for latest HTTP Signatures spec draft (#14556) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Add support for latest HTTP Signatures spec draft https://www.ietf.org/id/draft-ietf-httpbis-message-signatures-00.html - add support for the “hs2019” signature algorithm (assumed to be equivalent to RSA-SHA256, since we do not have a mechanism to specify the algorithm within the key metadata yet) - add support for (created) and (expires) pseudo-headers and related signature parameters, when using the hs2019 signature algorithm - adjust default “headers” parameter while being backwards-compatible with previous implementation - change the acceptable time window logic from 12 hours surrounding the “date” header to accepting signatures created up to 1 hour in the future and expiring up to 1 hour in the past (but only allowing expiration dates up to 12 hours after the creation date) This doesn't conform with the current draft, as it doesn't permit accounting for clock skew. This, however, should be addressed in a next version of the draft: https://github.com/httpwg/http-extensions/pull/1235 * Add additional signature requirements * Rewrite signature params parsing using Parslet * Make apparent which signature algorithm Mastodon on verification failure Mastodon uses RSASSA-PKCS1-v1_5, which is not recommended for new applications, and new implementers may thus unknowingly use RSASSA-PSS. * Add workaround for PeerTube's invalid signature header The previous parser allowed incorrect Signature headers, such as those produced by old versions of the `http-signature` node.js package, and seemingly used by PeerTube. This commit adds a workaround for that. * Fix `signature_key_id` raising an exception Previously, parsing failures would result in `signature_key_id` being nil, but the parser changes made that result in an exception. This commit changes the `signature_key_id` method to return `nil` in case of parsing failures. * Move extra HTTP signature helper methods to private methods * Relax (request-target) requirement to (request-target) || digest This lets requests from Plume work without lowering security significantly. --- .../concerns/signature_verification.rb | 163 ++++++++++++------ 1 file changed, 108 insertions(+), 55 deletions(-) diff --git a/app/controllers/concerns/signature_verification.rb b/app/controllers/concerns/signature_verification.rb index 10efbf2e0..18f549de9 100644 --- a/app/controllers/concerns/signature_verification.rb +++ b/app/controllers/concerns/signature_verification.rb @@ -7,6 +7,44 @@ module SignatureVerification include DomainControlHelper + EXPIRATION_WINDOW_LIMIT = 12.hours + CLOCK_SKEW_MARGIN = 1.hour + + class SignatureVerificationError < StandardError; end + + class SignatureParamsParser < Parslet::Parser + rule(:token) { match("[0-9a-zA-Z!#$%&'*+.^_`|~-]").repeat(1).as(:token) } + rule(:quoted_string) { str('"') >> (qdtext | quoted_pair).repeat.as(:quoted_string) >> str('"') } + # qdtext and quoted_pair are not exactly according to spec but meh + rule(:qdtext) { match('[^\\\\"]') } + rule(:quoted_pair) { str('\\') >> any } + rule(:bws) { match('\s').repeat } + rule(:param) { (token.as(:key) >> bws >> str('=') >> bws >> (token | quoted_string).as(:value)).as(:param) } + rule(:comma) { bws >> str(',') >> bws } + # Old versions of node-http-signature add an incorrect "Signature " prefix to the header + rule(:buggy_prefix) { str('Signature ') } + rule(:params) { buggy_prefix.maybe >> (param >> (comma >> param).repeat).as(:params) } + root(:params) + end + + class SignatureParamsTransformer < Parslet::Transform + rule(params: subtree(:p)) do + (p.is_a?(Array) ? p : [p]).each_with_object({}) { |(key, val), h| h[key] = val } + end + + rule(param: { key: simple(:key), value: simple(:val) }) do + [key, val] + end + + rule(quoted_string: simple(:string)) do + string.to_s + end + + rule(token: simple(:string)) do + string.to_s + end + end + def require_signature! render plain: signature_verification_failure_reason, status: signature_verification_failure_code unless signed_request_account end @@ -24,72 +62,40 @@ module SignatureVerification end def signature_key_id - raw_signature = request.headers['Signature'] - signature_params = {} - - raw_signature.split(',').each do |part| - parsed_parts = part.match(/([a-z]+)="([^"]+)"/i) - next if parsed_parts.nil? || parsed_parts.size != 3 - signature_params[parsed_parts[1]] = parsed_parts[2] - end - signature_params['keyId'] + rescue SignatureVerificationError + nil end def signed_request_account return @signed_request_account if defined?(@signed_request_account) - unless signed_request? - @signature_verification_failure_reason = 'Request not signed' - @signed_request_account = nil - return - end + raise SignatureVerificationError, 'Request not signed' unless signed_request? + raise SignatureVerificationError, 'Incompatible request signature. keyId and signature are required' if missing_required_signature_parameters? + raise SignatureVerificationError, 'Unsupported signature algorithm (only rsa-sha256 and hs2019 are supported)' unless %w(rsa-sha256 hs2019).include?(signature_algorithm) + raise SignatureVerificationError, 'Signed request date outside acceptable time window' unless matches_time_window? - if request.headers['Date'].present? && !matches_time_window? - @signature_verification_failure_reason = 'Signed request date outside acceptable time window' - @signed_request_account = nil - return - end - - raw_signature = request.headers['Signature'] - signature_params = {} - - raw_signature.split(',').each do |part| - parsed_parts = part.match(/([a-z]+)="([^"]+)"/i) - next if parsed_parts.nil? || parsed_parts.size != 3 - signature_params[parsed_parts[1]] = parsed_parts[2] - end - - if incompatible_signature?(signature_params) - @signature_verification_failure_reason = 'Incompatible request signature' - @signed_request_account = nil - return - end + verify_signature_strength! account = account_from_key_id(signature_params['keyId']) - if account.nil? - @signature_verification_failure_reason = "Public key not found for key #{signature_params['keyId']}" - @signed_request_account = nil - return - end + raise SignatureVerificationError, "Public key not found for key #{signature_params['keyId']}" if account.nil? signature = Base64.decode64(signature_params['signature']) - compare_signed_string = build_signed_string(signature_params['headers']) + compare_signed_string = build_signed_string return account unless verify_signature(account, signature, compare_signed_string).nil? account = stoplight_wrap_request { account.possibly_stale? ? account.refresh! : account_refresh_key(account) } - if account.nil? - @signature_verification_failure_reason = "Public key not found for key #{signature_params['keyId']}" - @signed_request_account = nil - return - end + raise SignatureVerificationError, "Public key not found for key #{signature_params['keyId']}" if account.nil? return account unless verify_signature(account, signature, compare_signed_string).nil? - @signature_verification_failure_reason = "Verification failed for #{account.username}@#{account.domain} #{account.uri}" + @signature_verification_failure_reason = "Verification failed for #{account.username}@#{account.domain} #{account.uri} using rsa-sha256 (RSASSA-PKCS1-v1_5 with SHA-256)" + @signed_request_account = nil + rescue SignatureVerificationError => e + @signature_verification_failure_reason = e.message @signed_request_account = nil end @@ -99,6 +105,31 @@ module SignatureVerification private + def signature_params + @signature_params ||= begin + raw_signature = request.headers['Signature'] + tree = SignatureParamsParser.new.parse(raw_signature) + SignatureParamsTransformer.new.apply(tree) + end + rescue Parslet::ParseFailed + raise SignatureVerificationError, 'Error parsing signature parameters' + end + + def signature_algorithm + signature_params.fetch('algorithm', 'hs2019') + end + + def signed_headers + signature_params.fetch('headers', signature_algorithm == 'hs2019' ? '(created)' : 'date').downcase.split(' ') + end + + def verify_signature_strength! + raise SignatureVerificationError, 'Mastodon requires the Date header or (created) pseudo-header to be signed' unless signed_headers.include?('date') || signed_headers.include?('(created)') + raise SignatureVerificationError, 'Mastodon requires the Digest header or (request-target) pseudo-header to be signed' unless signed_headers.include?(Request::REQUEST_TARGET) || signed_headers.include?('digest') + raise SignatureVerificationError, 'Mastodon requires the Host header to be signed' unless signed_headers.include?('host') + raise SignatureVerificationError, 'Mastodon requires the Digest header to be signed when doing a POST request' if request.post? && !signed_headers.include?('digest') + end + def verify_signature(account, signature, compare_signed_string) if account.keypair.public_key.verify(OpenSSL::Digest::SHA256.new, signature, compare_signed_string) @signed_request_account = account @@ -108,12 +139,20 @@ module SignatureVerification nil end - def build_signed_string(signed_headers) - signed_headers = 'date' if signed_headers.blank? - - signed_headers.downcase.split(' ').map do |signed_header| + def build_signed_string + signed_headers.map do |signed_header| if signed_header == Request::REQUEST_TARGET "#{Request::REQUEST_TARGET}: #{request.method.downcase} #{request.path}" + elsif signed_header == '(created)' + raise SignatureVerificationError, 'Invalid pseudo-header (created) for rsa-sha256' unless signature_algorithm == 'hs2019' + raise SignatureVerificationError, 'Pseudo-header (created) used but corresponding argument missing' if signature_params['created'].blank? + + "(created): #{signature_params['created']}" + elsif signed_header == '(expires)' + raise SignatureVerificationError, 'Invalid pseudo-header (expires) for rsa-sha256' unless signature_algorithm == 'hs2019' + raise SignatureVerificationError, 'Pseudo-header (expires) used but corresponding argument missing' if signature_params['expires'].blank? + + "(expires): #{signature_params['expires']}" elsif signed_header == 'digest' "digest: #{body_digest}" else @@ -123,13 +162,28 @@ module SignatureVerification end def matches_time_window? + created_time = nil + expires_time = nil + begin - time_sent = Time.httpdate(request.headers['Date']) + if signature_algorithm == 'hs2019' && signature_params['created'].present? + created_time = Time.at(signature_params['created'].to_i).utc + elsif request.headers['Date'].present? + created_time = Time.httpdate(request.headers['Date']).utc + end + + expires_time = Time.at(signature_params['expires'].to_i).utc if signature_params['expires'].present? rescue ArgumentError return false end - (Time.now.utc - time_sent).abs <= 12.hours + expires_time ||= created_time + 5.minutes unless created_time.nil? + expires_time = [expires_time, created_time + EXPIRATION_WINDOW_LIMIT].min unless created_time.nil? + + return false if created_time.present? && created_time > Time.now.utc + CLOCK_SKEW_MARGIN + return false if expires_time.present? && Time.now.utc > expires_time + CLOCK_SKEW_MARGIN + + true end def body_digest @@ -140,9 +194,8 @@ module SignatureVerification name.split(/-/).map(&:capitalize).join('-') end - def incompatible_signature?(signature_params) - signature_params['keyId'].blank? || - signature_params['signature'].blank? + def missing_required_signature_parameters? + signature_params['keyId'].blank? || signature_params['signature'].blank? end def account_from_key_id(key_id) From a583e540232fe7f3c0902dec0ba97252eb4357cc Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Mon, 19 Oct 2020 15:58:53 +0200 Subject: [PATCH 20/20] Bump version to 3.2.1 --- CHANGELOG.md | 29 +++++++++++++++++++++++++++++ lib/mastodon/version.rb | 2 +- 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 18790e860..6f94ebea2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,35 @@ Changelog All notable changes to this project will be documented in this file. +## [3.2.1] - 2020-10-19 +### Added + +- Add support for latest HTTP Signatures spec draft ([ThibG](https://github.com/tootsuite/mastodon/pull/14556)) +- Add support for inlined objects in ActivityPub `to`/`cc` ([ThibG](https://github.com/tootsuite/mastodon/pull/14514)) + +### Changed + +- Change actors to not be served at all without authentication in limited federation mode ([ThibG](https://github.com/tootsuite/mastodon/pull/14800)) + - Previously, a bare version of an actor was served when not authenticated, i.e. username and public key + - Because all actor fetch requests are signed using a separate system actor, that is no longer required + +### Fixed + +- Fix `tootctl media` commands not recognizing very large IDs ([ThibG](https://github.com/tootsuite/mastodon/pull/14536)) +- Fix crash when failing to load emoji picker in web UI ([ThibG](https://github.com/tootsuite/mastodon/pull/14525)) +- Fix contrast requirements in thumbnail color extraction ([ThibG](https://github.com/tootsuite/mastodon/pull/14464)) +- Fix audio/video player not using `CDN_HOST` on public pages ([ThibG](https://github.com/tootsuite/mastodon/pull/14486)) +- Fix private boost icon not being used on public pages ([OmmyZhang](https://github.com/tootsuite/mastodon/pull/14471)) +- Fix audio player on Safari in web UI ([ThibG](https://github.com/tootsuite/mastodon/pull/14485), [ThibG](https://github.com/tootsuite/mastodon/pull/14465)) +- Fix dereferencing remote statuses not using the correct account for signature when receiving a targeted inbox delivery ([ThibG](https://github.com/tootsuite/mastodon/pull/14656)) +- Fix nil error in `tootctl media remove` ([noellabo](https://github.com/tootsuite/mastodon/pull/14657)) +- Fix videos with near-60 fps being rejected ([Gargron](https://github.com/tootsuite/mastodon/pull/14684)) +- Fix reported statuses not being included in warning e-mail ([Gargron](https://github.com/tootsuite/mastodon/pull/14778)) +- Fix `Reject` activities of `Follow` objects not correctly destroying a follow relationship ([ThibG](https://github.com/tootsuite/mastodon/pull/14479)) +- Fix inefficiencies in fan-out-on-write service ([Gargron](https://github.com/tootsuite/mastodon/pull/14682), [noellabo](https://github.com/tootsuite/mastodon/pull/14709)) +- Fix timeout errors when trying to webfinger some IPv6 configurations ([Gargron](https://github.com/tootsuite/mastodon/pull/14919)) +- Fix files served as `application/octet-stream` being rejected without attempting mime type detection ([ThibG](https://github.com/tootsuite/mastodon/pull/14452)) + ## [3.2.0] - 2020-07-27 ### Added diff --git a/lib/mastodon/version.rb b/lib/mastodon/version.rb index 7aa6cb2c7..344ea996e 100644 --- a/lib/mastodon/version.rb +++ b/lib/mastodon/version.rb @@ -13,7 +13,7 @@ module Mastodon end def patch - 0 + 1 end def flags