From d7875adad242881876e316477b0610cbad466893 Mon Sep 17 00:00:00 2001 From: Claire Date: Tue, 19 Dec 2023 11:27:37 +0100 Subject: [PATCH 01/26] Fix call to inefficient `delete_matched` cache method in domain blocks (#28367) --- .../api/v1/accounts/notes_controller.rb | 2 +- .../api/v1/accounts/pins_controller.rb | 2 +- .../v1/accounts/relationships_controller.rb | 5 +- app/controllers/api/v1/accounts_controller.rb | 2 +- .../api/v1/follow_requests_controller.rb | 4 +- app/controllers/relationships_controller.rb | 2 +- app/models/account_domain_block.rb | 10 +-- app/models/concerns/account_interactions.rb | 6 -- app/models/concerns/relationship_cacheable.rb | 4 +- .../account_relationships_presenter.rb | 63 ++++++++++++++----- .../account_relationships_presenter_spec.rb | 61 +++++++++++++----- 11 files changed, 104 insertions(+), 57 deletions(-) diff --git a/app/controllers/api/v1/accounts/notes_controller.rb b/app/controllers/api/v1/accounts/notes_controller.rb index 032e807d11..6d115631a2 100644 --- a/app/controllers/api/v1/accounts/notes_controller.rb +++ b/app/controllers/api/v1/accounts/notes_controller.rb @@ -25,6 +25,6 @@ class Api::V1::Accounts::NotesController < Api::BaseController end def relationships_presenter - AccountRelationshipsPresenter.new([@account.id], current_user.account_id) + AccountRelationshipsPresenter.new([@account], current_user.account_id) end end diff --git a/app/controllers/api/v1/accounts/pins_controller.rb b/app/controllers/api/v1/accounts/pins_controller.rb index 73f845c614..0eb13c048c 100644 --- a/app/controllers/api/v1/accounts/pins_controller.rb +++ b/app/controllers/api/v1/accounts/pins_controller.rb @@ -25,6 +25,6 @@ class Api::V1::Accounts::PinsController < Api::BaseController end def relationships_presenter - AccountRelationshipsPresenter.new([@account.id], current_user.account_id) + AccountRelationshipsPresenter.new([@account], current_user.account_id) end end diff --git a/app/controllers/api/v1/accounts/relationships_controller.rb b/app/controllers/api/v1/accounts/relationships_controller.rb index 503f85c97d..038d6700f5 100644 --- a/app/controllers/api/v1/accounts/relationships_controller.rb +++ b/app/controllers/api/v1/accounts/relationships_controller.rb @@ -5,11 +5,10 @@ class Api::V1::Accounts::RelationshipsController < Api::BaseController before_action :require_user! def index - accounts = Account.without_suspended.where(id: account_ids).select('id') + @accounts = Account.without_suspended.where(id: account_ids).select(:id, :domain).to_a # .where doesn't guarantee that our results are in the same order # we requested them, so return the "right" order to the requestor. - @accounts = accounts.index_by(&:id).values_at(*account_ids).compact - render json: @accounts, each_serializer: REST::RelationshipSerializer, relationships: relationships + render json: @accounts.index_by(&:id).values_at(*account_ids).compact, each_serializer: REST::RelationshipSerializer, relationships: relationships end private diff --git a/app/controllers/api/v1/accounts_controller.rb b/app/controllers/api/v1/accounts_controller.rb index ddb94d5ca4..321a57ac57 100644 --- a/app/controllers/api/v1/accounts_controller.rb +++ b/app/controllers/api/v1/accounts_controller.rb @@ -86,7 +86,7 @@ class Api::V1::AccountsController < Api::BaseController end def relationships(**options) - AccountRelationshipsPresenter.new([@account.id], current_user.account_id, **options) + AccountRelationshipsPresenter.new([@account], current_user.account_id, **options) end def account_params diff --git a/app/controllers/api/v1/follow_requests_controller.rb b/app/controllers/api/v1/follow_requests_controller.rb index 7c197ce6ba..ee717ebbcc 100644 --- a/app/controllers/api/v1/follow_requests_controller.rb +++ b/app/controllers/api/v1/follow_requests_controller.rb @@ -25,11 +25,11 @@ class Api::V1::FollowRequestsController < Api::BaseController private def account - Account.find(params[:id]) + @account ||= Account.find(params[:id]) end def relationships(**options) - AccountRelationshipsPresenter.new([params[:id]], current_user.account_id, **options) + AccountRelationshipsPresenter.new([account], current_user.account_id, **options) end def load_accounts diff --git a/app/controllers/relationships_controller.rb b/app/controllers/relationships_controller.rb index e87b5a656f..dd794f3199 100644 --- a/app/controllers/relationships_controller.rb +++ b/app/controllers/relationships_controller.rb @@ -33,7 +33,7 @@ class RelationshipsController < ApplicationController end def set_relationships - @relationships = AccountRelationshipsPresenter.new(@accounts.pluck(:id), current_user.account_id) + @relationships = AccountRelationshipsPresenter.new(@accounts, current_user.account_id) end def form_account_batch_params diff --git a/app/models/account_domain_block.rb b/app/models/account_domain_block.rb index af1e6a68dc..db2e37184f 100644 --- a/app/models/account_domain_block.rb +++ b/app/models/account_domain_block.rb @@ -18,16 +18,12 @@ class AccountDomainBlock < ApplicationRecord belongs_to :account validates :domain, presence: true, uniqueness: { scope: :account_id }, domain: true - after_commit :remove_blocking_cache - after_commit :remove_relationship_cache + after_commit :invalidate_domain_blocking_cache private - def remove_blocking_cache + def invalidate_domain_blocking_cache Rails.cache.delete("exclude_domains_for:#{account_id}") - end - - def remove_relationship_cache - Rails.cache.delete_matched("relationship:#{account_id}:*") + Rails.cache.delete(['exclude_domains', account_id, domain]) end end diff --git a/app/models/concerns/account_interactions.rb b/app/models/concerns/account_interactions.rb index 3c64ebd9fa..2de15031f1 100644 --- a/app/models/concerns/account_interactions.rb +++ b/app/models/concerns/account_interactions.rb @@ -60,12 +60,6 @@ module AccountInteractions end end - def domain_blocking_map(target_account_ids, account_id) - accounts_map = Account.where(id: target_account_ids).select('id, domain').each_with_object({}) { |a, h| h[a.id] = a.domain } - blocked_domains = domain_blocking_map_by_domain(accounts_map.values.compact, account_id) - accounts_map.reduce({}) { |h, (id, domain)| h.merge(id => blocked_domains[domain]) } - end - def domain_blocking_map_by_domain(target_domains, account_id) follow_mapping(AccountDomainBlock.where(account_id: account_id, domain: target_domains), :domain) end diff --git a/app/models/concerns/relationship_cacheable.rb b/app/models/concerns/relationship_cacheable.rb index 0d9359f7e7..c32a8d62c6 100644 --- a/app/models/concerns/relationship_cacheable.rb +++ b/app/models/concerns/relationship_cacheable.rb @@ -10,7 +10,7 @@ module RelationshipCacheable private def remove_relationship_cache - Rails.cache.delete("relationship:#{account_id}:#{target_account_id}") - Rails.cache.delete("relationship:#{target_account_id}:#{account_id}") + Rails.cache.delete(['relationship', account_id, target_account_id]) + Rails.cache.delete(['relationship', target_account_id, account_id]) end end diff --git a/app/presenters/account_relationships_presenter.rb b/app/presenters/account_relationships_presenter.rb index 5d2b5435dd..8482ef54da 100644 --- a/app/presenters/account_relationships_presenter.rb +++ b/app/presenters/account_relationships_presenter.rb @@ -5,8 +5,9 @@ class AccountRelationshipsPresenter :muting, :requested, :requested_by, :domain_blocking, :endorsed, :account_note - def initialize(account_ids, current_account_id, **options) - @account_ids = account_ids.map { |a| a.is_a?(Account) ? a.id : a.to_i } + def initialize(accounts, current_account_id, **options) + @accounts = accounts.to_a + @account_ids = @accounts.pluck(:id) @current_account_id = current_account_id @following = cached[:following].merge(Account.following_map(@uncached_account_ids, @current_account_id)) @@ -16,10 +17,11 @@ class AccountRelationshipsPresenter @muting = cached[:muting].merge(Account.muting_map(@uncached_account_ids, @current_account_id)) @requested = cached[:requested].merge(Account.requested_map(@uncached_account_ids, @current_account_id)) @requested_by = cached[:requested_by].merge(Account.requested_by_map(@uncached_account_ids, @current_account_id)) - @domain_blocking = cached[:domain_blocking].merge(Account.domain_blocking_map(@uncached_account_ids, @current_account_id)) @endorsed = cached[:endorsed].merge(Account.endorsed_map(@uncached_account_ids, @current_account_id)) @account_note = cached[:account_note].merge(Account.account_note_map(@uncached_account_ids, @current_account_id)) + @domain_blocking = domain_blocking_map + cache_uncached! @following.merge!(options[:following_map] || {}) @@ -36,6 +38,31 @@ class AccountRelationshipsPresenter private + def domain_blocking_map + target_domains = @accounts.pluck(:domain).compact.uniq + blocks_by_domain = {} + + # Fetch from cache + cache_keys = target_domains.map { |domain| domain_cache_key(domain) } + Rails.cache.read_multi(*cache_keys).each do |key, blocking| + blocks_by_domain[key.last] = blocking + end + + uncached_domains = target_domains - blocks_by_domain.keys + + # Read uncached values from database + AccountDomainBlock.where(account_id: @current_account_id, domain: uncached_domains).pluck(:domain).each do |domain| + blocks_by_domain[domain] = true + end + + # Write database reads to cache + to_cache = uncached_domains.to_h { |domain| [domain_cache_key(domain), blocks_by_domain[domain]] } + Rails.cache.write_multi(to_cache, expires_in: 1.day) + + # Return formatted value + @accounts.each_with_object({}) { |account, h| h[account.id] = blocks_by_domain[account.domain] } + end + def cached return @cached if defined?(@cached) @@ -47,28 +74,23 @@ class AccountRelationshipsPresenter muting: {}, requested: {}, requested_by: {}, - domain_blocking: {}, endorsed: {}, account_note: {}, } - @uncached_account_ids = [] + @uncached_account_ids = @account_ids.uniq - @account_ids.each do |account_id| - maps_for_account = Rails.cache.read("relationship:#{@current_account_id}:#{account_id}") - - if maps_for_account.is_a?(Hash) - @cached.deep_merge!(maps_for_account) - else - @uncached_account_ids << account_id - end + cache_ids = @account_ids.map { |account_id| relationship_cache_key(account_id) } + Rails.cache.read_multi(*cache_ids).each do |key, maps_for_account| + @cached.deep_merge!(maps_for_account) + @uncached_account_ids.delete(key.last) end @cached end def cache_uncached! - @uncached_account_ids.each do |account_id| + to_cache = @uncached_account_ids.to_h do |account_id| maps_for_account = { following: { account_id => following[account_id] }, followed_by: { account_id => followed_by[account_id] }, @@ -77,12 +99,21 @@ class AccountRelationshipsPresenter muting: { account_id => muting[account_id] }, requested: { account_id => requested[account_id] }, requested_by: { account_id => requested_by[account_id] }, - domain_blocking: { account_id => domain_blocking[account_id] }, endorsed: { account_id => endorsed[account_id] }, account_note: { account_id => account_note[account_id] }, } - Rails.cache.write("relationship:#{@current_account_id}:#{account_id}", maps_for_account, expires_in: 1.day) + [relationship_cache_key(account_id), maps_for_account] end + + Rails.cache.write_multi(to_cache, expires_in: 1.day) + end + + def domain_cache_key(domain) + ['exclude_domains', @current_account_id, domain] + end + + def relationship_cache_key(account_id) + ['relationship', @current_account_id, account_id] end end diff --git a/spec/presenters/account_relationships_presenter_spec.rb b/spec/presenters/account_relationships_presenter_spec.rb index 5c2ba54e00..282cae4f06 100644 --- a/spec/presenters/account_relationships_presenter_spec.rb +++ b/spec/presenters/account_relationships_presenter_spec.rb @@ -5,30 +5,57 @@ require 'rails_helper' RSpec.describe AccountRelationshipsPresenter do describe '.initialize' do before do - allow(Account).to receive(:following_map).with(account_ids, current_account_id).and_return(default_map) - allow(Account).to receive(:followed_by_map).with(account_ids, current_account_id).and_return(default_map) - allow(Account).to receive(:blocking_map).with(account_ids, current_account_id).and_return(default_map) - allow(Account).to receive(:muting_map).with(account_ids, current_account_id).and_return(default_map) - allow(Account).to receive(:requested_map).with(account_ids, current_account_id).and_return(default_map) - allow(Account).to receive(:requested_by_map).with(account_ids, current_account_id).and_return(default_map) - allow(Account).to receive(:domain_blocking_map).with(account_ids, current_account_id).and_return(default_map) + allow(Account).to receive(:following_map).with(accounts.pluck(:id), current_account_id).and_return(default_map) + allow(Account).to receive(:followed_by_map).with(accounts.pluck(:id), current_account_id).and_return(default_map) + allow(Account).to receive(:blocking_map).with(accounts.pluck(:id), current_account_id).and_return(default_map) + allow(Account).to receive(:muting_map).with(accounts.pluck(:id), current_account_id).and_return(default_map) + allow(Account).to receive(:requested_map).with(accounts.pluck(:id), current_account_id).and_return(default_map) + allow(Account).to receive(:requested_by_map).with(accounts.pluck(:id), current_account_id).and_return(default_map) end - let(:presenter) { described_class.new(account_ids, current_account_id, **options) } + let(:presenter) { described_class.new(accounts, current_account_id, **options) } let(:current_account_id) { Fabricate(:account).id } - let(:account_ids) { [Fabricate(:account).id] } - let(:default_map) { { 1 => true } } + let(:accounts) { [Fabricate(:account)] } + let(:default_map) { { accounts[0].id => true } } context 'when options are not set' do let(:options) { {} } it 'sets default maps' do - expect(presenter.following).to eq default_map - expect(presenter.followed_by).to eq default_map - expect(presenter.blocking).to eq default_map - expect(presenter.muting).to eq default_map - expect(presenter.requested).to eq default_map - expect(presenter.domain_blocking).to eq default_map + expect(presenter).to have_attributes( + following: default_map, + followed_by: default_map, + blocking: default_map, + muting: default_map, + requested: default_map, + domain_blocking: { accounts[0].id => nil } + ) + end + end + + context 'with a warm cache' do + let(:options) { {} } + + before do + described_class.new(accounts, current_account_id, **options) + + allow(Account).to receive(:following_map).with([], current_account_id).and_return({}) + allow(Account).to receive(:followed_by_map).with([], current_account_id).and_return({}) + allow(Account).to receive(:blocking_map).with([], current_account_id).and_return({}) + allow(Account).to receive(:muting_map).with([], current_account_id).and_return({}) + allow(Account).to receive(:requested_map).with([], current_account_id).and_return({}) + allow(Account).to receive(:requested_by_map).with([], current_account_id).and_return({}) + end + + it 'sets returns expected values' do + expect(presenter).to have_attributes( + following: default_map, + followed_by: default_map, + blocking: default_map, + muting: default_map, + requested: default_map, + domain_blocking: { accounts[0].id => nil } + ) end end @@ -84,7 +111,7 @@ RSpec.describe AccountRelationshipsPresenter do let(:options) { { domain_blocking_map: { 7 => true } } } it 'sets @domain_blocking merged with default_map and options[:domain_blocking_map]' do - expect(presenter.domain_blocking).to eq default_map.merge(options[:domain_blocking_map]) + expect(presenter.domain_blocking).to eq({ accounts[0].id => nil }.merge(options[:domain_blocking_map])) end end end From 279be076794c21304aee2a1c1469a6f39b8fbe50 Mon Sep 17 00:00:00 2001 From: Claire Date: Tue, 12 Dec 2023 09:29:46 +0100 Subject: [PATCH 02/26] Fix `LinkCrawlWorker` error when encountering empty OEmbed response (#28268) --- app/services/fetch_oembed_service.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/services/fetch_oembed_service.rb b/app/services/fetch_oembed_service.rb index 1ae592238e..dc84b16b68 100644 --- a/app/services/fetch_oembed_service.rb +++ b/app/services/fetch_oembed_service.rb @@ -100,7 +100,7 @@ class FetchOEmbedService end def validate(oembed) - oembed if oembed[:version].to_s == '1.0' && oembed[:type].present? + oembed if oembed.present? && oembed[:version].to_s == '1.0' && oembed[:type].present? end def html From 7abc61887f30d7f1d1145e5be104a098b4b14f87 Mon Sep 17 00:00:00 2001 From: Claire Date: Tue, 12 Dec 2023 15:31:37 +0100 Subject: [PATCH 03/26] Fix empty column explainer getting cropped under certain conditions (#28337) --- app/javascript/styles/mastodon/components.scss | 5 ----- 1 file changed, 5 deletions(-) diff --git a/app/javascript/styles/mastodon/components.scss b/app/javascript/styles/mastodon/components.scss index fbad5e2dbf..7d152fbf7c 100644 --- a/app/javascript/styles/mastodon/components.scss +++ b/app/javascript/styles/mastodon/components.scss @@ -4493,11 +4493,6 @@ a.status-card { align-items: center; justify-content: center; - @supports (display: grid) { - // hack to fix Chrome <57 - contain: strict; - } - & > span { max-width: 500px; } From a12b7551cfa4a95ea8ca7ab1ae1a3d96d1a3ad24 Mon Sep 17 00:00:00 2001 From: Claire Date: Wed, 13 Dec 2023 08:47:32 +0100 Subject: [PATCH 04/26] Fix N+1s because of association preloaders not actually getting called (#28339) --- app/lib/inline_renderer.rb | 4 ++-- app/models/announcement.rb | 4 ++-- app/models/concerns/account_search.rb | 4 ++-- app/models/notification.rb | 2 +- app/serializers/initial_state_serializer.rb | 4 ++-- app/services/account_search_service.rb | 2 +- app/services/batched_remove_status_service.rb | 4 ++-- 7 files changed, 12 insertions(+), 12 deletions(-) diff --git a/app/lib/inline_renderer.rb b/app/lib/inline_renderer.rb index eda3da2c29..0aebb13fce 100644 --- a/app/lib/inline_renderer.rb +++ b/app/lib/inline_renderer.rb @@ -37,13 +37,13 @@ class InlineRenderer private def preload_associations_for_status - ActiveRecord::Associations::Preloader.new(records: @object, associations: { + ActiveRecord::Associations::Preloader.new(records: [@object], associations: { active_mentions: :account, reblog: { active_mentions: :account, }, - }) + }).call end def current_user diff --git a/app/models/announcement.rb b/app/models/announcement.rb index c5d6dd62e1..2cd7c1d5ed 100644 --- a/app/models/announcement.rb +++ b/app/models/announcement.rb @@ -78,9 +78,9 @@ class Announcement < ApplicationRecord else scope.select("name, custom_emoji_id, count(*) as count, exists(select 1 from announcement_reactions r where r.account_id = #{account.id} and r.announcement_id = announcement_reactions.announcement_id and r.name = announcement_reactions.name) as me") end - end + end.to_a - ActiveRecord::Associations::Preloader.new(records: records, associations: :custom_emoji) + ActiveRecord::Associations::Preloader.new(records: records, associations: :custom_emoji).call records end diff --git a/app/models/concerns/account_search.rb b/app/models/concerns/account_search.rb index 9f7720f11b..5bd624025f 100644 --- a/app/models/concerns/account_search.rb +++ b/app/models/concerns/account_search.rb @@ -122,7 +122,7 @@ module AccountSearch tsquery = generate_query_for_search(terms) find_by_sql([BASIC_SEARCH_SQL, { limit: limit, offset: offset, tsquery: tsquery }]).tap do |records| - ActiveRecord::Associations::Preloader.new(records: records, associations: :account_stat) + ActiveRecord::Associations::Preloader.new(records: records, associations: [:account_stat, { user: :role }]).call end end @@ -131,7 +131,7 @@ module AccountSearch sql_template = following ? ADVANCED_SEARCH_WITH_FOLLOWING : ADVANCED_SEARCH_WITHOUT_FOLLOWING find_by_sql([sql_template, { id: account.id, limit: limit, offset: offset, tsquery: tsquery }]).tap do |records| - ActiveRecord::Associations::Preloader.new(records: records, associations: :account_stat) + ActiveRecord::Associations::Preloader.new(records: records, associations: [:account_stat, { user: :role }]).call end end diff --git a/app/models/notification.rb b/app/models/notification.rb index 60f834a633..54212d675f 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -111,7 +111,7 @@ class Notification < ApplicationRecord # Instead of using the usual `includes`, manually preload each type. # If polymorphic associations are loaded with the usual `includes`, other types of associations will be loaded more. - ActiveRecord::Associations::Preloader.new(records: grouped_notifications, associations: associations) + ActiveRecord::Associations::Preloader.new(records: grouped_notifications, associations: associations).call end unique_target_statuses = notifications.filter_map(&:target_status).uniq diff --git a/app/serializers/initial_state_serializer.rb b/app/serializers/initial_state_serializer.rb index b707d6fcb6..d1c03a4131 100644 --- a/app/serializers/initial_state_serializer.rb +++ b/app/serializers/initial_state_serializer.rb @@ -86,8 +86,8 @@ class InitialStateSerializer < ActiveModel::Serializer ActiveRecord::Associations::Preloader.new( records: [object.current_account, object.admin, object.owner, object.disabled_account, object.moved_to_account].compact, - associations: [:account_stat, :user, { moved_to_account: [:account_stat, :user] }] - ) + associations: [:account_stat, { user: :role, moved_to_account: [:account_stat, { user: :role }] }] + ).call store[object.current_account.id.to_s] = ActiveModelSerializers::SerializableResource.new(object.current_account, serializer: REST::AccountSerializer) if object.current_account store[object.admin.id.to_s] = ActiveModelSerializers::SerializableResource.new(object.admin, serializer: REST::AccountSerializer) if object.admin diff --git a/app/services/account_search_service.rb b/app/services/account_search_service.rb index b437ff4759..7b85b09d8f 100644 --- a/app/services/account_search_service.rb +++ b/app/services/account_search_service.rb @@ -218,7 +218,7 @@ class AccountSearchService < BaseService records = query_builder.build.limit(limit_for_non_exact_results).offset(offset).objects.compact - ActiveRecord::Associations::Preloader.new(records: records, associations: :account_stat) + ActiveRecord::Associations::Preloader.new(records: records, associations: [:account_stat, { user: :role }]).call records rescue Faraday::ConnectionFailed, Parslet::ParseFailed diff --git a/app/services/batched_remove_status_service.rb b/app/services/batched_remove_status_service.rb index c54cc1d350..de4ee16e91 100644 --- a/app/services/batched_remove_status_service.rb +++ b/app/services/batched_remove_status_service.rb @@ -11,7 +11,7 @@ class BatchedRemoveStatusService < BaseService ActiveRecord::Associations::Preloader.new( records: statuses, associations: options[:skip_side_effects] ? :reblogs : [:account, :tags, reblogs: :account] - ) + ).call statuses_and_reblogs = statuses.flat_map { |status| [status] + status.reblogs } @@ -23,7 +23,7 @@ class BatchedRemoveStatusService < BaseService ActiveRecord::Associations::Preloader.new( records: statuses_with_account_conversations, associations: [mentions: :account] - ) + ).call statuses_with_account_conversations.each(&:unlink_from_conversations!) From 0a01bc01d2fc8967f75cd24ab3347d13a9d7f211 Mon Sep 17 00:00:00 2001 From: MitarashiDango Date: Sat, 13 Jan 2024 00:58:28 +0900 Subject: [PATCH 05/26] Fix Undo Announce activity is not sent, when not followed by the reblogged post author (#18482) Co-authored-by: Claire --- app/lib/status_reach_finder.rb | 34 ++++++++++----------- app/services/reblog_service.rb | 6 +--- spec/lib/activitypub/tag_manager_spec.rb | 8 +++++ spec/services/reblog_service_spec.rb | 4 --- spec/services/remove_status_service_spec.rb | 18 +++++++++++ 5 files changed, 44 insertions(+), 26 deletions(-) diff --git a/app/lib/status_reach_finder.rb b/app/lib/status_reach_finder.rb index 36fb0e80fb..17e42e3ec3 100644 --- a/app/lib/status_reach_finder.rb +++ b/app/lib/status_reach_finder.rb @@ -16,28 +16,28 @@ class StatusReachFinder private def reached_account_inboxes + Account.where(id: reached_account_ids).inboxes + end + + def reached_account_ids # When the status is a reblog, there are no interactions with it # directly, we assume all interactions are with the original one if @status.reblog? - [] + [reblog_of_account_id] else - Account.where(id: reached_account_ids).inboxes - end - end - - def reached_account_ids - [ - replied_to_account_id, - reblog_of_account_id, - mentioned_account_ids, - reblogs_account_ids, - favourites_account_ids, - replies_account_ids, - ].tap do |arr| - arr.flatten! - arr.compact! - arr.uniq! + [ + replied_to_account_id, + reblog_of_account_id, + mentioned_account_ids, + reblogs_account_ids, + favourites_account_ids, + replies_account_ids, + ].tap do |arr| + arr.flatten! + arr.compact! + arr.uniq! + end end end diff --git a/app/services/reblog_service.rb b/app/services/reblog_service.rb index 6ec0944744..c026746121 100644 --- a/app/services/reblog_service.rb +++ b/app/services/reblog_service.rb @@ -43,11 +43,7 @@ class ReblogService < BaseService def create_notification(reblog) reblogged_status = reblog.reblog - if reblogged_status.account.local? - LocalNotificationWorker.perform_async(reblogged_status.account_id, reblog.id, reblog.class.name, 'reblog') - elsif reblogged_status.account.activitypub? && !reblogged_status.account.following?(reblog.account) - ActivityPub::DeliveryWorker.perform_async(build_json(reblog), reblog.account_id, reblogged_status.account.inbox_url) - end + LocalNotificationWorker.perform_async(reblogged_status.account_id, reblog.id, reblog.class.name, 'reblog') if reblogged_status.account.local? end def bump_potential_friendship(account, reblog) diff --git a/spec/lib/activitypub/tag_manager_spec.rb b/spec/lib/activitypub/tag_manager_spec.rb index 2bff125a6a..55e9b4bb51 100644 --- a/spec/lib/activitypub/tag_manager_spec.rb +++ b/spec/lib/activitypub/tag_manager_spec.rb @@ -112,6 +112,14 @@ RSpec.describe ActivityPub::TagManager do expect(subject.cc(status)).to include(subject.uri_for(foo)) expect(subject.cc(status)).to_not include(subject.uri_for(alice)) end + + it 'returns poster of reblogged post, if reblog' do + bob = Fabricate(:account, username: 'bob', domain: 'example.com', inbox_url: 'http://example.com/bob') + alice = Fabricate(:account, username: 'alice') + status = Fabricate(:status, visibility: :public, account: bob) + reblog = Fabricate(:status, visibility: :public, account: alice, reblog: status) + expect(subject.cc(reblog)).to include(subject.uri_for(bob)) + end end describe '#local_uri?' do diff --git a/spec/services/reblog_service_spec.rb b/spec/services/reblog_service_spec.rb index 7b85e37ed8..357b315af0 100644 --- a/spec/services/reblog_service_spec.rb +++ b/spec/services/reblog_service_spec.rb @@ -86,9 +86,5 @@ RSpec.describe ReblogService, type: :service do it 'distributes to followers' do expect(ActivityPub::DistributionWorker).to have_received(:perform_async) end - - it 'sends an announce activity to the author' do - expect(a_request(:post, bob.inbox_url)).to have_been_made.once - end end end diff --git a/spec/services/remove_status_service_spec.rb b/spec/services/remove_status_service_spec.rb index c19b4fac15..7e81f3f6a4 100644 --- a/spec/services/remove_status_service_spec.rb +++ b/spec/services/remove_status_service_spec.rb @@ -110,4 +110,22 @@ RSpec.describe RemoveStatusService, type: :service do )).to have_been_made.once end end + + context 'when removed status is a reblog of a non-follower' do + let!(:original_status) { Fabricate(:status, account: bill, text: 'Hello ThisIsASecret', visibility: :public) } + let!(:status) { ReblogService.new.call(alice, original_status) } + + it 'sends Undo activity to followers' do + subject.call(status) + expect(a_request(:post, bill.inbox_url).with( + body: hash_including({ + 'type' => 'Undo', + 'object' => hash_including({ + 'type' => 'Announce', + 'object' => ActivityPub::TagManager.instance.uri_for(original_status), + }), + }) + )).to have_been_made.once + end + end end From e8c575414233df9d8c1a44140f91aa8e03bdc161 Mon Sep 17 00:00:00 2001 From: gunchleoc Date: Mon, 25 Sep 2023 12:59:37 +0200 Subject: [PATCH 06/26] Fix line wrapping of language selection button with long locale codes (#27100) --- app/javascript/styles/mastodon/components.scss | 1 + 1 file changed, 1 insertion(+) diff --git a/app/javascript/styles/mastodon/components.scss b/app/javascript/styles/mastodon/components.scss index 7d152fbf7c..5f773c1cf3 100644 --- a/app/javascript/styles/mastodon/components.scss +++ b/app/javascript/styles/mastodon/components.scss @@ -284,6 +284,7 @@ font-size: 11px; padding: 0 3px; line-height: 27px; + white-space: nowrap; &:hover, &:active, From dfc8fcc6f0a266c72c1e37ee7b2982a5b9ee57f0 Mon Sep 17 00:00:00 2001 From: Claire Date: Mon, 25 Sep 2023 15:06:57 +0200 Subject: [PATCH 07/26] Fix width of large text icon buttons (#27127) --- .../mastodon/features/compose/components/text_icon_button.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/javascript/mastodon/features/compose/components/text_icon_button.jsx b/app/javascript/mastodon/features/compose/components/text_icon_button.jsx index 46b5d7fada..166d022b88 100644 --- a/app/javascript/mastodon/features/compose/components/text_icon_button.jsx +++ b/app/javascript/mastodon/features/compose/components/text_icon_button.jsx @@ -4,7 +4,7 @@ import { PureComponent } from 'react'; const iconStyle = { height: null, lineHeight: '27px', - width: `${18 * 1.28571429}px`, + minWidth: `${18 * 1.28571429}px`, }; export default class TextIconButton extends PureComponent { From 8f2dac05672094f2b76944ec1935fd0338333131 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Fri, 20 Oct 2023 04:08:13 +0200 Subject: [PATCH 08/26] Fix missing background behind dismissable banner in web UI (#27479) --- .../mastodon/features/explore/statuses.jsx | 32 ++++++++----------- 1 file changed, 14 insertions(+), 18 deletions(-) diff --git a/app/javascript/mastodon/features/explore/statuses.jsx b/app/javascript/mastodon/features/explore/statuses.jsx index f32a4a5368..0d8d212b25 100644 --- a/app/javascript/mastodon/features/explore/statuses.jsx +++ b/app/javascript/mastodon/features/explore/statuses.jsx @@ -45,24 +45,20 @@ class Statuses extends PureComponent { const emptyMessage = ; return ( - <> - - - - - - + } + alwaysPrepend + timelineId='explore' + statusIds={statusIds} + scrollKey='explore-statuses' + hasMore={hasMore} + isLoading={isLoading} + onLoadMore={this.handleLoadMore} + emptyMessage={emptyMessage} + bindToDocument={!multiColumn} + withCounters + /> ); } From 3ecc991f6328170a452c69839719bfe92911cbc4 Mon Sep 17 00:00:00 2001 From: Brian Holley Date: Mon, 13 Nov 2023 16:39:54 -0800 Subject: [PATCH 09/26] Fix "Hide these posts from home" list setting not refreshing when switching lists (#27763) --- app/javascript/mastodon/features/list_timeline/index.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/javascript/mastodon/features/list_timeline/index.jsx b/app/javascript/mastodon/features/list_timeline/index.jsx index a5acc416e0..472c5ee8aa 100644 --- a/app/javascript/mastodon/features/list_timeline/index.jsx +++ b/app/javascript/mastodon/features/list_timeline/index.jsx @@ -201,7 +201,7 @@ class ListTimeline extends PureComponent {
- + From 4d96d716c449d81eb1835b94329e00f316a48fa3 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Wed, 22 Nov 2023 12:38:07 +0100 Subject: [PATCH 10/26] Fix unsupported time zone or locale preventing sign-up (#28035) Co-authored-by: Claire --- app/models/user.rb | 14 ++++++++++---- .../preferences/appearance_controller_spec.rb | 6 ------ spec/models/user_spec.rb | 18 ++++++++++++------ 3 files changed, 22 insertions(+), 16 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index fa445af811..309a00d1cc 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -96,11 +96,9 @@ class User < ApplicationRecord accepts_nested_attributes_for :invite_request, reject_if: ->(attributes) { attributes['text'].blank? && !Setting.require_invite_text } validates :invite_request, presence: true, on: :create, if: :invite_text_required? - validates :locale, inclusion: I18n.available_locales.map(&:to_s), if: :locale? validates_with BlacklistedEmailValidator, if: -> { ENV['EMAIL_DOMAIN_LISTS_APPLY_AFTER_CONFIRMATION'] == 'true' || !confirmed? } validates_with EmailMxValidator, if: :validate_email_dns? validates :agreement, acceptance: { allow_nil: false, accept: [true, 'true', '1'] }, on: :create - validates :time_zone, inclusion: { in: ActiveSupport::TimeZone.all.map { |tz| tz.tzinfo.name } }, allow_blank: true # Honeypot/anti-spam fields attr_accessor :registration_form_time, :website, :confirm_password @@ -124,6 +122,8 @@ class User < ApplicationRecord before_validation :sanitize_languages before_validation :sanitize_role + before_validation :sanitize_time_zone + before_validation :sanitize_locale before_create :set_approved after_commit :send_pending_devise_notifications after_create_commit :trigger_webhooks @@ -451,9 +451,15 @@ class User < ApplicationRecord end def sanitize_role - return if role.nil? + self.role = nil if role.present? && role.everyone? + end - self.role = nil if role.everyone? + def sanitize_time_zone + self.time_zone = nil if time_zone.present? && ActiveSupport::TimeZone[time_zone].nil? + end + + def sanitize_locale + self.locale = nil if locale.present? && I18n.available_locales.exclude?(locale.to_sym) end def prepare_new_user! diff --git a/spec/controllers/settings/preferences/appearance_controller_spec.rb b/spec/controllers/settings/preferences/appearance_controller_spec.rb index 9a98a41886..083bf49544 100644 --- a/spec/controllers/settings/preferences/appearance_controller_spec.rb +++ b/spec/controllers/settings/preferences/appearance_controller_spec.rb @@ -31,11 +31,5 @@ describe Settings::Preferences::AppearanceController do expect(response).to redirect_to(settings_preferences_appearance_path) end - - it 'renders show on failure' do - put :update, params: { user: { locale: 'fake option' } } - - expect(response).to render_template('preferences/appearance/show') - end end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index bb61c02a63..0dad96cc8d 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -27,12 +27,6 @@ RSpec.describe User do expect(user).to model_have_error_on_field(:account) end - it 'is invalid without a valid locale' do - user = Fabricate.build(:user, locale: 'toto') - user.valid? - expect(user).to model_have_error_on_field(:locale) - end - it 'is invalid without a valid email' do user = Fabricate.build(:user, email: 'john@') user.valid? @@ -45,6 +39,18 @@ RSpec.describe User do expect(user.valid?).to be true end + it 'cleans out invalid locale' do + user = Fabricate.build(:user, locale: 'toto') + expect(user.valid?).to be true + expect(user.locale).to be_nil + end + + it 'cleans out invalid timezone' do + user = Fabricate.build(:user, time_zone: 'toto') + expect(user.valid?).to be true + expect(user.time_zone).to be_nil + end + it 'cleans out empty string from languages' do user = Fabricate.build(:user, chosen_languages: ['']) user.valid? From c609b726cba4874597d203c80cce647cd0932fbc Mon Sep 17 00:00:00 2001 From: Claire Date: Thu, 7 Dec 2023 10:05:08 +0100 Subject: [PATCH 11/26] Fix error when processing link preview with an array as `inLanguage` (#28252) --- app/lib/link_details_extractor.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/app/lib/link_details_extractor.rb b/app/lib/link_details_extractor.rb index a96612cab0..bb031986d6 100644 --- a/app/lib/link_details_extractor.rb +++ b/app/lib/link_details_extractor.rb @@ -37,6 +37,7 @@ class LinkDetailsExtractor def language lang = json['inLanguage'] + lang = lang.first if lang.is_a?(Array) lang.is_a?(Hash) ? (lang['alternateName'] || lang['name']) : lang end From 01caa18e5ba6eea8f102f31916ed47e5c781e862 Mon Sep 17 00:00:00 2001 From: Claire Date: Tue, 2 Jan 2024 13:27:51 +0100 Subject: [PATCH 12/26] Fix streaming API redirection ignoring the port of `streaming_api_base_url` (#28558) --- app/controllers/api/v1/streaming_controller.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/controllers/api/v1/streaming_controller.rb b/app/controllers/api/v1/streaming_controller.rb index 0cdd00d62f..0cb6e856f2 100644 --- a/app/controllers/api/v1/streaming_controller.rb +++ b/app/controllers/api/v1/streaming_controller.rb @@ -13,7 +13,9 @@ class Api::V1::StreamingController < Api::BaseController def streaming_api_url Addressable::URI.parse(request.url).tap do |uri| - uri.host = Addressable::URI.parse(Rails.configuration.x.streaming_api_base_url).host + base_url = Addressable::URI.parse(Rails.configuration.x.streaming_api_base_url) + uri.host = base_url.host + uri.port = base_url.port end.to_s end end From c0a9db3611b3cda655409bbf28ec5531194cd20e Mon Sep 17 00:00:00 2001 From: Claire Date: Wed, 10 Jan 2024 16:05:46 +0100 Subject: [PATCH 13/26] Fix potential redirection loop of streaming endpoint (#28665) --- app/controllers/api/v1/streaming_controller.rb | 7 ++++++- spec/controllers/api/v1/streaming_controller_spec.rb | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/app/controllers/api/v1/streaming_controller.rb b/app/controllers/api/v1/streaming_controller.rb index 0cb6e856f2..adb14676e1 100644 --- a/app/controllers/api/v1/streaming_controller.rb +++ b/app/controllers/api/v1/streaming_controller.rb @@ -2,7 +2,7 @@ class Api::V1::StreamingController < Api::BaseController def index - if Rails.configuration.x.streaming_api_base_url == request.host + if same_host? not_found else redirect_to streaming_api_url, status: 301, allow_other_host: true @@ -11,6 +11,11 @@ class Api::V1::StreamingController < Api::BaseController private + def same_host? + base_url = Addressable::URI.parse(Rails.configuration.x.streaming_api_base_url) + request.host == base_url.host && request.port == (base_url.port || 80) + end + def streaming_api_url Addressable::URI.parse(request.url).tap do |uri| base_url = Addressable::URI.parse(Rails.configuration.x.streaming_api_base_url) diff --git a/spec/controllers/api/v1/streaming_controller_spec.rb b/spec/controllers/api/v1/streaming_controller_spec.rb index 7014ed9b2b..825bb11978 100644 --- a/spec/controllers/api/v1/streaming_controller_spec.rb +++ b/spec/controllers/api/v1/streaming_controller_spec.rb @@ -5,7 +5,7 @@ require 'rails_helper' describe Api::V1::StreamingController do around(:each) do |example| before = Rails.configuration.x.streaming_api_base_url - Rails.configuration.x.streaming_api_base_url = Rails.configuration.x.web_domain + Rails.configuration.x.streaming_api_base_url = "wss://#{Rails.configuration.x.web_domain}" example.run Rails.configuration.x.streaming_api_base_url = before end From 1998c561b2ffe7200781778f281fcc0890ebf6df Mon Sep 17 00:00:00 2001 From: Claire Date: Fri, 22 Dec 2023 19:56:22 +0100 Subject: [PATCH 14/26] Convert signature verification specs to request specs (#28443) --- .../concerns/signature_verification_spec.rb | 305 ---------------- spec/requests/signature_verification_spec.rb | 332 ++++++++++++++++++ 2 files changed, 332 insertions(+), 305 deletions(-) delete mode 100644 spec/controllers/concerns/signature_verification_spec.rb create mode 100644 spec/requests/signature_verification_spec.rb diff --git a/spec/controllers/concerns/signature_verification_spec.rb b/spec/controllers/concerns/signature_verification_spec.rb deleted file mode 100644 index 650cd21eaf..0000000000 --- a/spec/controllers/concerns/signature_verification_spec.rb +++ /dev/null @@ -1,305 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' - -describe SignatureVerification do - let(:wrapped_actor_class) do - Class.new do - attr_reader :wrapped_account - - def initialize(wrapped_account) - @wrapped_account = wrapped_account - end - - delegate :uri, :keypair, to: :wrapped_account - end - end - - controller(ApplicationController) do - include SignatureVerification - - before_action :require_actor_signature!, only: [:signature_required] - - def success - head 200 - end - - def alternative_success - head 200 - end - - def signature_required - head 200 - end - end - - before do - routes.draw do - match :via => [:get, :post], 'success' => 'anonymous#success' - match :via => [:get, :post], 'signature_required' => 'anonymous#signature_required' - end - end - - context 'without signature header' do - before do - get :success - end - - describe '#signed_request?' do - it 'returns false' do - expect(controller.signed_request?).to be false - end - end - - describe '#signed_request_account' do - it 'returns nil' do - expect(controller.signed_request_account).to be_nil - end - end - end - - context 'with signature header' do - let!(:author) { Fabricate(:account, domain: 'example.com', uri: 'https://example.com/actor') } - - context 'without body' do - before do - get :success - - fake_request = Request.new(:get, request.url) - fake_request.on_behalf_of(author) - - request.headers.merge!(fake_request.headers) - end - - describe '#signed_request?' do - it 'returns true' do - expect(controller.signed_request?).to be true - end - end - - describe '#signed_request_account' do - it 'returns an account' do - expect(controller.signed_request_account).to eq author - end - - it 'returns nil when path does not match' do - request.path = '/alternative-path' - expect(controller.signed_request_account).to be_nil - end - - it 'returns nil when method does not match' do - post :success - expect(controller.signed_request_account).to be_nil - end - end - end - - context 'with a valid actor that is not an Account' do - let(:actor) { wrapped_actor_class.new(author) } - - before do - get :success - - fake_request = Request.new(:get, request.url) - fake_request.on_behalf_of(author) - - request.headers.merge!(fake_request.headers) - - allow(ActivityPub::TagManager.instance).to receive(:uri_to_actor).with(anything) do - actor - end - end - - describe '#signed_request?' do - it 'returns true' do - expect(controller.signed_request?).to be true - end - end - - describe '#signed_request_account' do - it 'returns nil' do - expect(controller.signed_request_account).to be_nil - end - end - - describe '#signed_request_actor' do - it 'returns the expected actor' do - expect(controller.signed_request_actor).to eq actor - end - end - end - - context 'with request with unparsable Date header' do - before do - get :success - - fake_request = Request.new(:get, request.url) - fake_request.add_headers({ 'Date' => 'wrong date' }) - fake_request.on_behalf_of(author) - - request.headers.merge!(fake_request.headers) - end - - describe '#signed_request?' do - it 'returns true' do - expect(controller.signed_request?).to be true - end - end - - describe '#signed_request_account' do - it 'returns nil' do - expect(controller.signed_request_account).to be_nil - end - end - - describe '#signature_verification_failure_reason' do - it 'contains an error description' do - controller.signed_request_account - expect(controller.signature_verification_failure_reason[:error]).to eq 'Invalid Date header: not RFC 2616 compliant date: "wrong date"' - end - end - end - - context 'with request older than a day' do - before do - get :success - - fake_request = Request.new(:get, request.url) - fake_request.add_headers({ 'Date' => 2.days.ago.utc.httpdate }) - fake_request.on_behalf_of(author) - - request.headers.merge!(fake_request.headers) - end - - describe '#signed_request?' do - it 'returns true' do - expect(controller.signed_request?).to be true - end - end - - describe '#signed_request_account' do - it 'returns nil' do - expect(controller.signed_request_account).to be_nil - end - end - - describe '#signature_verification_failure_reason' do - it 'contains an error description' do - controller.signed_request_account - expect(controller.signature_verification_failure_reason[:error]).to eq 'Signed request date outside acceptable time window' - end - end - end - - context 'with inaccessible key' do - before do - get :success - - author = Fabricate(:account, domain: 'localhost:5000', uri: 'http://localhost:5000/actor') - fake_request = Request.new(:get, request.url) - fake_request.on_behalf_of(author) - author.destroy - - request.headers.merge!(fake_request.headers) - - stub_request(:get, 'http://localhost:5000/actor#main-key').to_raise(Mastodon::HostValidationError) - end - - describe '#signed_request?' do - it 'returns true' do - expect(controller.signed_request?).to be true - end - end - - describe '#signed_request_account' do - it 'returns nil' do - expect(controller.signed_request_account).to be_nil - end - end - end - - context 'with body' do - before do - allow(controller).to receive(:actor_refresh_key!).and_return(author) - post :success, body: 'Hello world' - - fake_request = Request.new(:post, request.url, body: 'Hello world') - fake_request.on_behalf_of(author) - - request.headers.merge!(fake_request.headers) - end - - describe '#signed_request?' do - it 'returns true' do - expect(controller.signed_request?).to be true - end - end - - describe '#signed_request_account' do - it 'returns an account' do - expect(controller.signed_request_account).to eq author - end - end - - context 'when path does not match' do - before do - request.path = '/alternative-path' - end - - describe '#signed_request_account' do - it 'returns nil' do - expect(controller.signed_request_account).to be_nil - end - end - - describe '#signature_verification_failure_reason' do - it 'contains an error description' do - controller.signed_request_account - expect(controller.signature_verification_failure_reason[:error]).to include('using rsa-sha256 (RSASSA-PKCS1-v1_5 with SHA-256)') - expect(controller.signature_verification_failure_reason[:signed_string]).to include("(request-target): post /alternative-path\n") - end - end - end - - context 'when method does not match' do - before do - get :success - end - - describe '#signed_request_account' do - it 'returns nil' do - expect(controller.signed_request_account).to be_nil - end - end - end - - context 'when body has been tampered' do - before do - post :success, body: 'doo doo doo' - end - - describe '#signed_request_account' do - it 'returns nil when body has been tampered' do - expect(controller.signed_request_account).to be_nil - end - end - end - end - end - - context 'when a signature is required' do - before do - get :signature_required - end - - context 'without signature header' do - it 'returns HTTP 401' do - expect(response).to have_http_status(401) - end - - it 'returns an error' do - expect(Oj.load(response.body)['error']).to eq 'Request not signed' - end - end - end -end diff --git a/spec/requests/signature_verification_spec.rb b/spec/requests/signature_verification_spec.rb new file mode 100644 index 0000000000..b753750b84 --- /dev/null +++ b/spec/requests/signature_verification_spec.rb @@ -0,0 +1,332 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe 'signature verification concern' do + before do + stub_tests_controller + + # Signature checking is time-dependent, so travel to a fixed date + travel_to '2023-12-20T10:00:00Z' + end + + after { Rails.application.reload_routes! } + + # Include the private key so the tests can be easily adjusted and reviewed + let(:actor_keypair) do + OpenSSL::PKey.read(<<~PEM_TEXT) + -----BEGIN RSA PRIVATE KEY----- + MIIEowIBAAKCAQEAqIAYvNFGbZ5g4iiK6feSdXD4bDStFM58A7tHycYXaYtzZQpI + eHXAmaXuZzXIwtrP4N0gIk8JNwZvXj2UPS+S07t0V9wNK94he01LV5EMz/GN4eNn + FmDL64HIEuKLvV8TvgjbUPRD6Y5X0UpKi2ZIFLSb96Q5w0Z/k7ntpVKV52y8kz5F + jr/O/0JuHryZe0yItzJh8kzFfeMf0EXzfSnaKvT7P9jhgC6uTre+jXyvVZjiHDrn + qvvucdI3I7DRfXo1OqARBrLjy+TdseUAjNYJ+OuPRI1URIWQI01DCHqcohVu9+Ar + +BiCjFp3ua+XMuJvrvbD61d1Fvig/9nbBRR+8QIDAQABAoIBAAgySHnFWI6gItR3 + fkfiqIm80cHCN3Xk1C6iiVu+3oBOZbHpW9R7vl9e/WOA/9O+LPjiSsQOegtWnVvd + RRjrl7Hj20VDlZKv5Mssm6zOGAxksrcVbqwdj+fUJaNJCL0AyyseH0x/IE9T8rDC + I1GH+3tB3JkhkIN/qjipdX5ab8MswEPu8IC4ViTpdBgWYY/xBcAHPw4xuL0tcwzh + FBlf4DqoEVQo8GdK5GAJ2Ny0S4xbXHUURzx/R4y4CCts7niAiLGqd9jmLU1kUTMk + QcXfQYK6l+unLc7wDYAz7sFEHh04M48VjWwiIZJnlCqmQbLda7uhhu8zkF1DqZTu + ulWDGQECgYEA0TIAc8BQBVab979DHEEmMdgqBwxLY3OIAk0b+r50h7VBGWCDPRsC + STD73fQY3lNet/7/jgSGwwAlAJ5PpMXxXiZAE3bUwPmHzgF7pvIOOLhA8O07tHSO + L2mvQe6NPzjZ+6iAO2U9PkClxcvGvPx2OBvisfHqZLmxC9PIVxzruQECgYEAzjM6 + BTUXa6T/qHvLFbN699BXsUOGmHBGaLRapFDBfVvgZrwqYQcZpBBhesLdGTGSqwE7 + gWsITPIJ+Ldo+38oGYyVys+w/V67q6ud7hgSDTW3hSvm+GboCjk6gzxlt9hQ0t9X + 8vfDOYhEXvVUJNv3mYO60ENqQhILO4bQ0zi+VfECgYBb/nUccfG+pzunU0Cb6Dp3 + qOuydcGhVmj1OhuXxLFSDG84Tazo7juvHA9mp7VX76mzmDuhpHPuxN2AzB2SBEoE + cSW0aYld413JRfWukLuYTc6hJHIhBTCRwRQFFnae2s1hUdQySm8INT2xIc+fxBXo + zrp+Ljg5Wz90SAnN5TX0AQKBgDaatDOq0o/r+tPYLHiLtfWoE4Dau+rkWJDjqdk3 + lXWn/e3WyHY3Vh/vQpEqxzgju45TXjmwaVtPATr+/usSykCxzP0PMPR3wMT+Rm1F + rIoY/odij+CaB7qlWwxj0x/zRbwB7x1lZSp4HnrzBpxYL+JUUwVRxPLIKndSBTza + GvVRAoGBAIVBcNcRQYF4fvZjDKAb4fdBsEuHmycqtRCsnkGOz6ebbEQznSaZ0tZE + +JuouZaGjyp8uPjNGD5D7mIGbyoZ3KyG4mTXNxDAGBso1hrNDKGBOrGaPhZx8LgO + 4VXJ+ybXrATf4jr8ccZYsZdFpOphPzz+j55Mqg5vac5P1XjmsGTb + -----END RSA PRIVATE KEY----- + PEM_TEXT + end + + context 'without a Signature header' do + it 'does not treat the request as signed' do + get '/activitypub/success' + + expect(response).to have_http_status(200) + expect(body_as_json).to match( + signed_request: false, + signature_actor_id: nil, + error: 'Request not signed' + ) + end + + context 'when a signature is required' do + it 'returns http unauthorized with appropriate error' do + get '/activitypub/signature_required' + + expect(response).to have_http_status(401) + expect(body_as_json).to match( + error: 'Request not signed' + ) + end + end + end + + context 'with an HTTP Signature from a known account' do + let!(:actor) { Fabricate(:account, domain: 'remote.domain', uri: 'https://remote.domain/users/bob', private_key: nil, public_key: actor_keypair.public_key.to_pem) } + + context 'with a valid signature on a GET request' do + let(:signature_header) do + 'keyId="https://remote.domain/users/bob#main-key",algorithm="rsa-sha256",headers="date host (request-target)",signature="Z8ilar3J7bOwqZkMp7sL8sRs4B1FT+UorbmvWoE+A5UeoOJ3KBcUmbsh+k3wQwbP5gMNUrra9rEWabpasZGphLsbDxfbsWL3Cf0PllAc7c1c7AFEwnewtExI83/qqgEkfWc2z7UDutXc2NfgAx89Ox8DXU/fA2GG0jILjB6UpFyNugkY9rg6oI31UnvfVi3R7sr3/x8Ea3I9thPvqI2byF6cojknSpDAwYzeKdngX3TAQEGzFHz3SDWwyp3jeMWfwvVVbM38FxhvAnSumw7YwWW4L7M7h4M68isLimoT3yfCn2ucBVL5Dz8koBpYf/40w7QidClAwCafZQFC29yDOg=="' # rubocop:disable Layout/LineLength + end + + it 'successfuly verifies signature', :aggregate_failures do + expect(signature_header).to eq build_signature_string(actor_keypair, 'https://remote.domain/users/bob#main-key', 'get /activitypub/success', { 'Date' => 'Wed, 20 Dec 2023 10:00:00 GMT', 'Host' => 'www.example.com' }) + + get '/activitypub/success', headers: { + 'Host' => 'www.example.com', + 'Date' => 'Wed, 20 Dec 2023 10:00:00 GMT', + 'Signature' => signature_header, + } + + expect(response).to have_http_status(200) + expect(body_as_json).to match( + signed_request: true, + signature_actor_id: actor.id.to_s + ) + end + end + + context 'with a mismatching path' do + it 'fails to verify signature', :aggregate_failures do + get '/activitypub/alternative-path', headers: { + 'Host' => 'www.example.com', + 'Date' => 'Wed, 20 Dec 2023 10:00:00 GMT', + 'Signature' => 'keyId="https://remote.domain/users/bob#main-key",algorithm="rsa-sha256",headers="date host (request-target)",signature="Z8ilar3J7bOwqZkMp7sL8sRs4B1FT+UorbmvWoE+A5UeoOJ3KBcUmbsh+k3wQwbP5gMNUrra9rEWabpasZGphLsbDxfbsWL3Cf0PllAc7c1c7AFEwnewtExI83/qqgEkfWc2z7UDutXc2NfgAx89Ox8DXU/fA2GG0jILjB6UpFyNugkY9rg6oI31UnvfVi3R7sr3/x8Ea3I9thPvqI2byF6cojknSpDAwYzeKdngX3TAQEGzFHz3SDWwyp3jeMWfwvVVbM38FxhvAnSumw7YwWW4L7M7h4M68isLimoT3yfCn2ucBVL5Dz8koBpYf/40w7QidClAwCafZQFC29yDOg=="', # rubocop:disable Layout/LineLength + } + + expect(body_as_json).to match( + signed_request: true, + signature_actor_id: nil, + error: anything + ) + end + end + + context 'with a mismatching method' do + it 'fails to verify signature', :aggregate_failures do + post '/activitypub/success', headers: { + 'Host' => 'www.example.com', + 'Date' => 'Wed, 20 Dec 2023 10:00:00 GMT', + 'Signature' => 'keyId="https://remote.domain/users/bob#main-key",algorithm="rsa-sha256",headers="date host (request-target)",signature="Z8ilar3J7bOwqZkMp7sL8sRs4B1FT+UorbmvWoE+A5UeoOJ3KBcUmbsh+k3wQwbP5gMNUrra9rEWabpasZGphLsbDxfbsWL3Cf0PllAc7c1c7AFEwnewtExI83/qqgEkfWc2z7UDutXc2NfgAx89Ox8DXU/fA2GG0jILjB6UpFyNugkY9rg6oI31UnvfVi3R7sr3/x8Ea3I9thPvqI2byF6cojknSpDAwYzeKdngX3TAQEGzFHz3SDWwyp3jeMWfwvVVbM38FxhvAnSumw7YwWW4L7M7h4M68isLimoT3yfCn2ucBVL5Dz8koBpYf/40w7QidClAwCafZQFC29yDOg=="', # rubocop:disable Layout/LineLength + } + + expect(body_as_json).to match( + signed_request: true, + signature_actor_id: nil, + error: anything + ) + end + end + + context 'with an unparsable date' do + let(:signature_header) do + 'keyId="https://remote.domain/users/bob#main-key",algorithm="rsa-sha256",headers="date host (request-target)",signature="d4B7nfx8RJcfdJDu1J//5WzPzK/hgtPkdzZx49lu5QhnE7qdV3lgyVimmhCFrO16bwvzIp9iRMyRLkNFxLiEeVaa1gqeKbldGSnU0B0OMjx7rFBa65vLuzWQOATDitVGiBEYqoK4v0DMuFCz2DtFaA/DIUZ3sty8bZ/Ea3U1nByLOO6MacARA3zhMSI0GNxGqsSmZmG0hPLavB3jIXoE3IDoQabMnC39jrlcO/a8h1iaxBm2WD8TejrImJullgqlJIFpKhIHI3ipQkvTGPlm9dx0y+beM06qBvWaWQcmT09eRIUefVsOAzIhUtS/7FVb/URhZvircIJDa7vtiFcmZQ=="' # rubocop:disable Layout/LineLength + end + + it 'fails to verify signature', :aggregate_failures do + expect(signature_header).to eq build_signature_string(actor_keypair, 'https://remote.domain/users/bob#main-key', 'get /activitypub/success', { 'Date' => 'wrong date', 'Host' => 'www.example.com' }) + + get '/activitypub/success', headers: { + 'Host' => 'www.example.com', + 'Date' => 'wrong date', + 'Signature' => signature_header, + } + + expect(body_as_json).to match( + signed_request: true, + signature_actor_id: nil, + error: 'Invalid Date header: not RFC 2616 compliant date: "wrong date"' + ) + end + end + + context 'with a request older than a day' do + let(:signature_header) do + 'keyId="https://remote.domain/users/bob#main-key",algorithm="rsa-sha256",headers="date host (request-target)",signature="G1NuJv4zgoZ3B/ZIjzDWZHK4RC+5pYee74q8/LJEMCWXhcnAomcb9YHaqk1QYfQvcBUIXw3UZ3Q9xO8F9y0i8G5mzJHfQ+OgHqCoJk8EmGwsUXJMh5s1S5YFCRt8TT12TmJZz0VMqLq85ubueSYBM7QtUE/FzFIVLvz4RysgXxaXQKzdnM6+gbUEEKdCURpXdQt2NXQhp4MAmZH3+0lQoR6VxdsK0hx0Ji2PNp1nuqFTlYqNWZazVdLBN+9rETLRmvGXknvg9jOxTTppBVWnkAIl26HtLS3wwFVvz4pJzi9OQDOvLziehVyLNbU61hky+oJ215e2HuKSe2hxHNl1MA=="' # rubocop:disable Layout/LineLength + end + + it 'fails to verify signature', :aggregate_failures do + expect(signature_header).to eq build_signature_string(actor_keypair, 'https://remote.domain/users/bob#main-key', 'get /activitypub/success', { 'Date' => 'Wed, 18 Dec 2023 10:00:00 GMT', 'Host' => 'www.example.com' }) + + get '/activitypub/success', headers: { + 'Host' => 'www.example.com', + 'Date' => 'Wed, 18 Dec 2023 10:00:00 GMT', + 'Signature' => signature_header, + } + + expect(body_as_json).to match( + signed_request: true, + signature_actor_id: nil, + error: 'Signed request date outside acceptable time window' + ) + end + end + + context 'with a valid signature on a POST request' do + let(:digest_header) { 'SHA-256=ZOyIygCyaOW6GjVnihtTFtIS9PNmskdyMlNKiuyjfzw=' } + let(:signature_header) do + 'keyId="https://remote.domain/users/bob#main-key",algorithm="rsa-sha256",headers="host date digest (request-target)",signature="gmhMjgMROGElJU3fpehV2acD5kMHeELi8EFP2UPHOdQ54H0r55AxIpji+J3lPe+N2qSb/4H1KXIh6f0lRu8TGSsu12OQmg5hiO8VA9flcA/mh9Lpk+qwlQZIPRqKP9xUEfqD+Z7ti5wPzDKrWAUK/7FIqWgcT/mlqB1R1MGkpMFc/q4CIs2OSNiWgA4K+Kp21oQxzC2kUuYob04gAZ7cyE/FTia5t08uv6lVYFdRsn4XNPn1MsHgFBwBMRG79ng3SyhoG4PrqBEi5q2IdLq3zfre/M6He3wlCpyO2VJNdGVoTIzeZ0Zz8jUscPV3XtWUchpGclLGSaKaq/JyNZeiYQ=="' # rubocop:disable Layout/LineLength + end + + it 'successfuly verifies signature', :aggregate_failures do + expect(digest_header).to eq digest_value('Hello world') + expect(signature_header).to eq build_signature_string(actor_keypair, 'https://remote.domain/users/bob#main-key', 'post /activitypub/success', { 'Host' => 'www.example.com', 'Date' => 'Wed, 20 Dec 2023 10:00:00 GMT', 'Digest' => digest_header }) + + post '/activitypub/success', params: 'Hello world', headers: { + 'Host' => 'www.example.com', + 'Date' => 'Wed, 20 Dec 2023 10:00:00 GMT', + 'Digest' => digest_header, + 'Signature' => signature_header, + } + + expect(response).to have_http_status(200) + expect(body_as_json).to match( + signed_request: true, + signature_actor_id: actor.id.to_s + ) + end + end + + context 'when the Digest of a POST request is not signed' do + let(:digest_header) { 'SHA-256=ZOyIygCyaOW6GjVnihtTFtIS9PNmskdyMlNKiuyjfzw=' } + let(:signature_header) do + 'keyId="https://remote.domain/users/bob#main-key",algorithm="rsa-sha256",headers="host date (request-target)",signature="CPD704CG8aCm8X8qIP8kkkiGp1qwFLk/wMVQHOGP0Txxan8c2DZtg/KK7eN8RG8tHx8br/yS2hJs51x4kXImYukGzNJd7ihE3T8lp+9RI1tCcdobTzr/VcVJHDFySdQkg266GCMijRQRZfNvqlJLiisr817PI+gNVBI5qV+vnVd1XhWCEZ+YSmMe8UqYARXAYNqMykTheojqGpTeTFGPUpTQA2Fmt2BipwIjcFDm2Hpihl2kB0MUS0x3zPmHDuadvzoBbN6m3usPDLgYrpALlh+wDs1dYMntcwdwawRKY1oE1XNtgOSum12wntDq3uYL4gya2iPdcw3c929b4koUzw=="' # rubocop:disable Layout/LineLength + end + + it 'fails to verify signature', :aggregate_failures do + expect(digest_header).to eq digest_value('Hello world') + expect(signature_header).to eq build_signature_string(actor_keypair, 'https://remote.domain/users/bob#main-key', 'post /activitypub/success', { 'Host' => 'www.example.com', 'Date' => 'Wed, 20 Dec 2023 10:00:00 GMT' }) + + post '/activitypub/success', params: 'Hello world', headers: { + 'Host' => 'www.example.com', + 'Date' => 'Wed, 20 Dec 2023 10:00:00 GMT', + 'Digest' => digest_header, + 'Signature' => signature_header, + } + + expect(body_as_json).to match( + signed_request: true, + signature_actor_id: nil, + error: 'Mastodon requires the Digest header to be signed when doing a POST request' + ) + end + end + + context 'with a tampered body on a POST request' do + let(:digest_header) { 'SHA-256=ZOyIygCyaOW6GjVnihtTFtIS9PNmskdyMlNKiuyjfzw=' } + let(:signature_header) do + 'keyId="https://remote.domain/users/bob#main-key",algorithm="rsa-sha256",headers="host date digest (request-target)",signature="gmhMjgMROGElJU3fpehV2acD5kMHeELi8EFP2UPHOdQ54H0r55AxIpji+J3lPe+N2qSb/4H1KXIh6f0lRu8TGSsu12OQmg5hiO8VA9flcA/mh9Lpk+qwlQZIPRqKP9xUEfqD+Z7ti5wPzDKrWAUK/7FIqWgcT/mlqB1R1MGkpMFc/q4CIs2OSNiWgA4K+Kp21oQxzC2kUuYob04gAZ7cyE/FTia5t08uv6lVYFdRsn4XNPn1MsHgFBwBMRG79ng3SyhoG4PrqBEi5q2IdLq3zfre/M6He3wlCpyO2VJNdGVoTIzeZ0Zz8jUscPV3XtWUchpGclLGSaKaq/JyNZeiYQ=="' # rubocop:disable Layout/LineLength + end + + it 'fails to verify signature', :aggregate_failures do + expect(digest_header).to_not eq digest_value('Hello world!') + expect(signature_header).to eq build_signature_string(actor_keypair, 'https://remote.domain/users/bob#main-key', 'post /activitypub/success', { 'Host' => 'www.example.com', 'Date' => 'Wed, 20 Dec 2023 10:00:00 GMT', 'Digest' => digest_header }) + + post '/activitypub/success', params: 'Hello world!', headers: { + 'Host' => 'www.example.com', + 'Date' => 'Wed, 20 Dec 2023 10:00:00 GMT', + 'Digest' => 'SHA-256=ZOyIygCyaOW6GjVnihtTFtIS9PNmskdyMlNKiuyjfzw=', + 'Signature' => signature_header, + } + + expect(body_as_json).to match( + signed_request: true, + signature_actor_id: nil, + error: 'Invalid Digest value. Computed SHA-256 digest: wFNeS+K3n/2TKRMFQ2v4iTFOSj+uwF7P/Lt98xrZ5Ro=; given: ZOyIygCyaOW6GjVnihtTFtIS9PNmskdyMlNKiuyjfzw=' + ) + end + end + + context 'with a tampered path in a POST request' do + it 'fails to verify signature', :aggregate_failures do + post '/activitypub/alternative-path', params: 'Hello world', headers: { + 'Host' => 'www.example.com', + 'Date' => 'Wed, 20 Dec 2023 10:00:00 GMT', + 'Digest' => 'SHA-256=ZOyIygCyaOW6GjVnihtTFtIS9PNmskdyMlNKiuyjfzw=', + 'Signature' => 'keyId="https://remote.domain/users/bob#main-key",algorithm="rsa-sha256",headers="host date digest (request-target)",signature="gmhMjgMROGElJU3fpehV2acD5kMHeELi8EFP2UPHOdQ54H0r55AxIpji+J3lPe+N2qSb/4H1KXIh6f0lRu8TGSsu12OQmg5hiO8VA9flcA/mh9Lpk+qwlQZIPRqKP9xUEfqD+Z7ti5wPzDKrWAUK/7FIqWgcT/mlqB1R1MGkpMFc/q4CIs2OSNiWgA4K+Kp21oQxzC2kUuYob04gAZ7cyE/FTia5t08uv6lVYFdRsn4XNPn1MsHgFBwBMRG79ng3SyhoG4PrqBEi5q2IdLq3zfre/M6He3wlCpyO2VJNdGVoTIzeZ0Zz8jUscPV3XtWUchpGclLGSaKaq/JyNZeiYQ=="', # rubocop:disable Layout/LineLength + } + + expect(response).to have_http_status(200) + expect(body_as_json).to match( + signed_request: true, + signature_actor_id: nil, + error: anything + ) + end + end + end + + context 'with an inaccessible key' do + before do + stub_request(:get, 'https://remote.domain/users/alice#main-key').to_return(status: 404) + end + + it 'fails to verify signature', :aggregate_failures do + get '/activitypub/success', headers: { + 'Host' => 'www.example.com', + 'Date' => 'Wed, 20 Dec 2023 10:00:00 GMT', + 'Signature' => 'keyId="https://remote.domain/users/alice#main-key",algorithm="rsa-sha256",headers="date host (request-target)",signature="Z8ilar3J7bOwqZkMp7sL8sRs4B1FT+UorbmvWoE+A5UeoOJ3KBcUmbsh+k3wQwbP5gMNUrra9rEWabpasZGphLsbDxfbsWL3Cf0PllAc7c1c7AFEwnewtExI83/qqgEkfWc2z7UDutXc2NfgAx89Ox8DXU/fA2GG0jILjB6UpFyNugkY9rg6oI31UnvfVi3R7sr3/x8Ea3I9thPvqI2byF6cojknSpDAwYzeKdngX3TAQEGzFHz3SDWwyp3jeMWfwvVVbM38FxhvAnSumw7YwWW4L7M7h4M68isLimoT3yfCn2ucBVL5Dz8koBpYf/40w7QidClAwCafZQFC29yDOg=="', # rubocop:disable Layout/LineLength + } + + expect(body_as_json).to match( + signed_request: true, + signature_actor_id: nil, + error: 'Unable to fetch key JSON at https://remote.domain/users/alice#main-key' + ) + end + end + + private + + def stub_tests_controller + stub_const('ActivityPub::TestsController', activitypub_tests_controller) + + Rails.application.routes.draw do + # NOTE: RouteSet#draw removes all routes, so we need to re-insert one + resource :instance_actor, path: 'actor', only: [:show] + + match :via => [:get, :post], '/activitypub/success' => 'activitypub/tests#success' + match :via => [:get, :post], '/activitypub/alternative-path' => 'activitypub/tests#alternative_success' + match :via => [:get, :post], '/activitypub/signature_required' => 'activitypub/tests#signature_required' + end + end + + def activitypub_tests_controller + Class.new(ApplicationController) do + include SignatureVerification + + before_action :require_actor_signature!, only: [:signature_required] + + def success + render json: { + signed_request: signed_request?, + signature_actor_id: signed_request_actor&.id&.to_s, + }.merge(signature_verification_failure_reason || {}) + end + + alias_method :alternative_success, :success + alias_method :signature_required, :success + end + end + + def digest_value(body) + "SHA-256=#{Digest::SHA256.base64digest(body)}" + end + + def build_signature_string(keypair, key_id, request_target, headers) + algorithm = 'rsa-sha256' + signed_headers = headers.merge({ '(request-target)' => request_target }) + signed_string = signed_headers.map { |key, value| "#{key.downcase}: #{value}" }.join("\n") + signature = Base64.strict_encode64(keypair.sign(OpenSSL::Digest.new('SHA256'), signed_string)) + + "keyId=\"#{key_id}\",algorithm=\"#{algorithm}\",headers=\"#{signed_headers.keys.join(' ').downcase}\",signature=\"#{signature}\"" + end +end From 3837ec2227159d5b05f07d4d9c4a01647f29ad6f Mon Sep 17 00:00:00 2001 From: Claire Date: Wed, 3 Jan 2024 12:29:26 +0100 Subject: [PATCH 15/26] Fix Mastodon not correctly processing HTTP Signatures with query strings (#28476) --- .../concerns/signature_verification.rb | 22 ++++++- app/lib/request.rb | 11 +++- spec/requests/signature_verification_spec.rb | 66 +++++++++++++++++++ 3 files changed, 95 insertions(+), 4 deletions(-) diff --git a/app/controllers/concerns/signature_verification.rb b/app/controllers/concerns/signature_verification.rb index f0a344f1c9..35391e64c4 100644 --- a/app/controllers/concerns/signature_verification.rb +++ b/app/controllers/concerns/signature_verification.rb @@ -91,14 +91,23 @@ module SignatureVerification raise SignatureVerificationError, "Public key not found for key #{signature_params['keyId']}" if actor.nil? signature = Base64.decode64(signature_params['signature']) - compare_signed_string = build_signed_string + compare_signed_string = build_signed_string(include_query_string: true) return actor unless verify_signature(actor, signature, compare_signed_string).nil? + # Compatibility quirk with older Mastodon versions + compare_signed_string = build_signed_string(include_query_string: false) + return actor unless verify_signature(actor, signature, compare_signed_string).nil? + actor = stoplight_wrap_request { actor_refresh_key!(actor) } raise SignatureVerificationError, "Could not refresh public key #{signature_params['keyId']}" if actor.nil? + compare_signed_string = build_signed_string(include_query_string: true) + return actor unless verify_signature(actor, signature, compare_signed_string).nil? + + # Compatibility quirk with older Mastodon versions + compare_signed_string = build_signed_string(include_query_string: false) return actor unless verify_signature(actor, signature, compare_signed_string).nil? fail_with! "Verification failed for #{actor.to_log_human_identifier} #{actor.uri} using rsa-sha256 (RSASSA-PKCS1-v1_5 with SHA-256)", signed_string: compare_signed_string, signature: signature_params['signature'] @@ -180,11 +189,18 @@ module SignatureVerification nil end - def build_signed_string + def build_signed_string(include_query_string: true) signed_headers.map do |signed_header| case signed_header when Request::REQUEST_TARGET - "#{Request::REQUEST_TARGET}: #{request.method.downcase} #{request.path}" + if include_query_string + "#{Request::REQUEST_TARGET}: #{request.method.downcase} #{request.original_fullpath}" + else + # Current versions of Mastodon incorrectly omit the query string from the (request-target) pseudo-header. + # Therefore, temporarily support such incorrect signatures for compatibility. + # TODO: remove eventually some time after release of the fixed version + "#{Request::REQUEST_TARGET}: #{request.method.downcase} #{request.path}" + end when '(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? diff --git a/app/lib/request.rb b/app/lib/request.rb index 5f128af734..8d4120868d 100644 --- a/app/lib/request.rb +++ b/app/lib/request.rb @@ -77,6 +77,7 @@ class Request @url = Addressable::URI.parse(url).normalize @http_client = options.delete(:http_client) @allow_local = options.delete(:allow_local) + @full_path = options.delete(:with_query_string) @options = options.merge(socket_class: use_proxy? || @allow_local ? ProxySocket : Socket) @options = @options.merge(timeout_class: PerOperationWithDeadline, timeout_options: TIMEOUT) @options = @options.merge(proxy_url) if use_proxy? @@ -146,7 +147,7 @@ class Request private def set_common_headers! - @headers[REQUEST_TARGET] = "#{@verb} #{@url.path}" + @headers[REQUEST_TARGET] = request_target @headers['User-Agent'] = Mastodon::Version.user_agent @headers['Host'] = @url.host @headers['Date'] = Time.now.utc.httpdate @@ -157,6 +158,14 @@ class Request @headers['Digest'] = "SHA-256=#{Digest::SHA256.base64digest(@options[:body])}" end + def request_target + if @url.query.nil? || !@full_path + "#{@verb} #{@url.path}" + else + "#{@verb} #{@url.path}?#{@url.query}" + end + end + def signature algorithm = 'rsa-sha256' signature = Base64.strict_encode64(@keypair.sign(OpenSSL::Digest.new('SHA256'), signed_string)) diff --git a/spec/requests/signature_verification_spec.rb b/spec/requests/signature_verification_spec.rb index b753750b84..401828c4a3 100644 --- a/spec/requests/signature_verification_spec.rb +++ b/spec/requests/signature_verification_spec.rb @@ -94,6 +94,72 @@ describe 'signature verification concern' do end end + context 'with a valid signature on a GET request that has a query string' do + let(:signature_header) do + 'keyId="https://remote.domain/users/bob#main-key",algorithm="rsa-sha256",headers="date host (request-target)",signature="SDMa4r/DQYMXYxVgYO2yEqGWWUXugKjVuz0I8dniQAk+aunzBaF2aPu+4grBfawAshlx1Xytl8lhb0H2MllEz16/tKY7rUrb70MK0w8ohXgpb0qs3YvQgdj4X24L1x2MnkFfKHR/J+7TBlnivq0HZqXm8EIkPWLv+eQxu8fbowLwHIVvRd/3t6FzvcfsE0UZKkoMEX02542MhwSif6cu7Ec/clsY9qgKahb9JVGOGS1op9Lvg/9y1mc8KCgD83U5IxVygYeYXaVQ6gixA9NgZiTCwEWzHM5ELm7w5hpdLFYxYOHg/3G3fiqJzpzNQAcCD4S4JxfE7hMI0IzVlNLT6A=="' # rubocop:disable Layout/LineLength + end + + it 'successfuly verifies signature', :aggregate_failures do + expect(signature_header).to eq build_signature_string(actor_keypair, 'https://remote.domain/users/bob#main-key', 'get /activitypub/success?foo=42', { 'Date' => 'Wed, 20 Dec 2023 10:00:00 GMT', 'Host' => 'www.example.com' }) + + get '/activitypub/success?foo=42', headers: { + 'Host' => 'www.example.com', + 'Date' => 'Wed, 20 Dec 2023 10:00:00 GMT', + 'Signature' => signature_header, + } + + expect(response).to have_http_status(200) + expect(body_as_json).to match( + signed_request: true, + signature_actor_id: actor.id.to_s + ) + end + end + + context 'when the query string is missing from the signature verification (compatibility quirk)' do + let(:signature_header) do + 'keyId="https://remote.domain/users/bob#main-key",algorithm="rsa-sha256",headers="date host (request-target)",signature="Z8ilar3J7bOwqZkMp7sL8sRs4B1FT+UorbmvWoE+A5UeoOJ3KBcUmbsh+k3wQwbP5gMNUrra9rEWabpasZGphLsbDxfbsWL3Cf0PllAc7c1c7AFEwnewtExI83/qqgEkfWc2z7UDutXc2NfgAx89Ox8DXU/fA2GG0jILjB6UpFyNugkY9rg6oI31UnvfVi3R7sr3/x8Ea3I9thPvqI2byF6cojknSpDAwYzeKdngX3TAQEGzFHz3SDWwyp3jeMWfwvVVbM38FxhvAnSumw7YwWW4L7M7h4M68isLimoT3yfCn2ucBVL5Dz8koBpYf/40w7QidClAwCafZQFC29yDOg=="' # rubocop:disable Layout/LineLength + end + + it 'successfuly verifies signature', :aggregate_failures do + expect(signature_header).to eq build_signature_string(actor_keypair, 'https://remote.domain/users/bob#main-key', 'get /activitypub/success', { 'Date' => 'Wed, 20 Dec 2023 10:00:00 GMT', 'Host' => 'www.example.com' }) + + get '/activitypub/success?foo=42', headers: { + 'Host' => 'www.example.com', + 'Date' => 'Wed, 20 Dec 2023 10:00:00 GMT', + 'Signature' => signature_header, + } + + expect(response).to have_http_status(200) + expect(body_as_json).to match( + signed_request: true, + signature_actor_id: actor.id.to_s + ) + end + end + + context 'with mismatching query string' do + let(:signature_header) do + 'keyId="https://remote.domain/users/bob#main-key",algorithm="rsa-sha256",headers="date host (request-target)",signature="SDMa4r/DQYMXYxVgYO2yEqGWWUXugKjVuz0I8dniQAk+aunzBaF2aPu+4grBfawAshlx1Xytl8lhb0H2MllEz16/tKY7rUrb70MK0w8ohXgpb0qs3YvQgdj4X24L1x2MnkFfKHR/J+7TBlnivq0HZqXm8EIkPWLv+eQxu8fbowLwHIVvRd/3t6FzvcfsE0UZKkoMEX02542MhwSif6cu7Ec/clsY9qgKahb9JVGOGS1op9Lvg/9y1mc8KCgD83U5IxVygYeYXaVQ6gixA9NgZiTCwEWzHM5ELm7w5hpdLFYxYOHg/3G3fiqJzpzNQAcCD4S4JxfE7hMI0IzVlNLT6A=="' # rubocop:disable Layout/LineLength + end + + it 'fails to verify signature', :aggregate_failures do + expect(signature_header).to eq build_signature_string(actor_keypair, 'https://remote.domain/users/bob#main-key', 'get /activitypub/success?foo=42', { 'Date' => 'Wed, 20 Dec 2023 10:00:00 GMT', 'Host' => 'www.example.com' }) + + get '/activitypub/success?foo=43', headers: { + 'Host' => 'www.example.com', + 'Date' => 'Wed, 20 Dec 2023 10:00:00 GMT', + 'Signature' => signature_header, + } + + expect(body_as_json).to match( + signed_request: true, + signature_actor_id: nil, + error: anything + ) + end + end + context 'with a mismatching path' do it 'fails to verify signature', :aggregate_failures do get '/activitypub/alternative-path', headers: { From 499bc716a5dc5b5ef2ca710ae8dd0af8b0028127 Mon Sep 17 00:00:00 2001 From: Jeong Arm Date: Tue, 16 Jan 2024 17:35:54 +0900 Subject: [PATCH 16/26] Ignore RecordNotUnique errors in LinkCrawlWorker (#28748) --- app/workers/link_crawl_worker.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/workers/link_crawl_worker.rb b/app/workers/link_crawl_worker.rb index b3d8aa2646..c63af1e43a 100644 --- a/app/workers/link_crawl_worker.rb +++ b/app/workers/link_crawl_worker.rb @@ -7,7 +7,7 @@ class LinkCrawlWorker def perform(status_id) FetchLinkCardService.new.call(Status.find(status_id)) - rescue ActiveRecord::RecordNotFound + rescue ActiveRecord::RecordNotFound, ActiveRecord::RecordNotUnique true end end From 2dbf176d239720edfc2efd3ae4082830d7137062 Mon Sep 17 00:00:00 2001 From: Jonathan de Jong Date: Fri, 19 Jan 2024 10:18:21 +0100 Subject: [PATCH 17/26] Retry 401 errors on replies fetching (#28788) Co-authored-by: Claire --- app/helpers/jsonld_helper.rb | 12 ++++++------ app/services/activitypub/fetch_replies_service.rb | 15 ++++++++++++++- 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/app/helpers/jsonld_helper.rb b/app/helpers/jsonld_helper.rb index ce3ff094f6..b3d0d032c4 100644 --- a/app/helpers/jsonld_helper.rb +++ b/app/helpers/jsonld_helper.rb @@ -155,7 +155,7 @@ module JsonLdHelper end end - def fetch_resource(uri, id, on_behalf_of = nil) + def fetch_resource(uri, id, on_behalf_of = nil, request_options: {}) unless id json = fetch_resource_without_id_validation(uri, on_behalf_of) @@ -164,14 +164,14 @@ module JsonLdHelper uri = json['id'] end - json = fetch_resource_without_id_validation(uri, on_behalf_of) + json = fetch_resource_without_id_validation(uri, on_behalf_of, request_options: request_options) json.present? && json['id'] == uri ? json : nil end - def fetch_resource_without_id_validation(uri, on_behalf_of = nil, raise_on_temporary_error = false) + def fetch_resource_without_id_validation(uri, on_behalf_of = nil, raise_on_temporary_error = false, request_options: {}) on_behalf_of ||= Account.representative - build_request(uri, on_behalf_of).perform do |response| + build_request(uri, on_behalf_of, options: request_options).perform do |response| raise Mastodon::UnexpectedResponseError, response unless response_successful?(response) || response_error_unsalvageable?(response) || !raise_on_temporary_error body_to_json(response.body_with_limit) if response.code == 200 @@ -204,8 +204,8 @@ module JsonLdHelper response.code == 501 || ((400...500).cover?(response.code) && ![401, 408, 429].include?(response.code)) end - def build_request(uri, on_behalf_of = nil) - Request.new(:get, uri).tap do |request| + def build_request(uri, on_behalf_of = nil, options: {}) + Request.new(:get, uri, **options).tap do |request| request.on_behalf_of(on_behalf_of) if on_behalf_of request.add_headers('Accept' => 'application/activity+json, application/ld+json') end diff --git a/app/services/activitypub/fetch_replies_service.rb b/app/services/activitypub/fetch_replies_service.rb index b5c7759ec5..a9dd327e96 100644 --- a/app/services/activitypub/fetch_replies_service.rb +++ b/app/services/activitypub/fetch_replies_service.rb @@ -37,7 +37,20 @@ class ActivityPub::FetchRepliesService < BaseService return unless @allow_synchronous_requests return if non_matching_uri_hosts?(@account.uri, collection_or_uri) - fetch_resource_without_id_validation(collection_or_uri, nil, true) + # NOTE: For backward compatibility reasons, Mastodon signs outgoing + # queries incorrectly by default. + # + # While this is relevant for all URLs with query strings, this is + # the only code path where this happens in practice. + # + # Therefore, retry with correct signatures if this fails. + begin + fetch_resource_without_id_validation(collection_or_uri, nil, true) + rescue Mastodon::UnexpectedResponseError => e + raise unless e.response && e.response.code == 401 && Addressable::URI.parse(collection_or_uri).query.present? + + fetch_resource_without_id_validation(collection_or_uri, nil, true, request_options: { with_query_string: true }) + end end def filtered_replies From 6fe2a47357f893096f86866cb37d0bb052573757 Mon Sep 17 00:00:00 2001 From: Claire Date: Fri, 19 Jan 2024 13:19:49 +0100 Subject: [PATCH 18/26] Add rate-limit of TOTP authentication attempts at controller level (#28801) --- app/controllers/auth/sessions_controller.rb | 22 +++++++++++++++++++ .../two_factor_authentication_concern.rb | 5 +++++ config/locales/en.yml | 1 + .../auth/sessions_controller_spec.rb | 20 +++++++++++++++++ 4 files changed, 48 insertions(+) diff --git a/app/controllers/auth/sessions_controller.rb b/app/controllers/auth/sessions_controller.rb index 06a3deee2b..8212e2e630 100644 --- a/app/controllers/auth/sessions_controller.rb +++ b/app/controllers/auth/sessions_controller.rb @@ -1,6 +1,10 @@ # frozen_string_literal: true class Auth::SessionsController < Devise::SessionsController + include Redisable + + MAX_2FA_ATTEMPTS_PER_HOUR = 10 + layout 'auth' skip_before_action :require_no_authentication, only: [:create] @@ -134,9 +138,23 @@ class Auth::SessionsController < Devise::SessionsController session.delete(:attempt_user_updated_at) end + def clear_2fa_attempt_from_user(user) + redis.del(second_factor_attempts_key(user)) + end + + def check_second_factor_rate_limits(user) + attempts, = redis.multi do |multi| + multi.incr(second_factor_attempts_key(user)) + multi.expire(second_factor_attempts_key(user), 1.hour) + end + + attempts >= MAX_2FA_ATTEMPTS_PER_HOUR + end + def on_authentication_success(user, security_measure) @on_authentication_success_called = true + clear_2fa_attempt_from_user(user) clear_attempt_from_session user.update_sign_in!(new_sign_in: true) @@ -168,4 +186,8 @@ class Auth::SessionsController < Devise::SessionsController user_agent: request.user_agent ) end + + def second_factor_attempts_key(user) + "2fa_auth_attempts:#{user.id}:#{Time.now.utc.hour}" + end end diff --git a/app/controllers/concerns/two_factor_authentication_concern.rb b/app/controllers/concerns/two_factor_authentication_concern.rb index 9eb45b90d6..90fa392a13 100644 --- a/app/controllers/concerns/two_factor_authentication_concern.rb +++ b/app/controllers/concerns/two_factor_authentication_concern.rb @@ -65,6 +65,11 @@ module TwoFactorAuthenticationConcern end def authenticate_with_two_factor_via_otp(user) + if check_second_factor_rate_limits(user) + flash.now[:alert] = I18n.t('users.rate_limited') + return prompt_for_two_factor(user) + end + if valid_otp_attempt?(user) on_authentication_success(user, :otp) else diff --git a/config/locales/en.yml b/config/locales/en.yml index 71b8f27aac..2b3bfb4c71 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1826,6 +1826,7 @@ en: go_to_sso_account_settings: Go to your identity provider's account settings invalid_otp_token: Invalid two-factor code otp_lost_help_html: If you lost access to both, you may get in touch with %{email} + rate_limited: Too many authentication attempts, try again later. seamless_external_login: You are logged in via an external service, so password and e-mail settings are not available. signed_in_as: 'Signed in as:' verification: diff --git a/spec/controllers/auth/sessions_controller_spec.rb b/spec/controllers/auth/sessions_controller_spec.rb index c727a76333..95055ade4c 100644 --- a/spec/controllers/auth/sessions_controller_spec.rb +++ b/spec/controllers/auth/sessions_controller_spec.rb @@ -263,6 +263,26 @@ RSpec.describe Auth::SessionsController do end end + context 'when repeatedly using an invalid TOTP code before using a valid code' do + before do + stub_const('Auth::SessionsController::MAX_2FA_ATTEMPTS_PER_HOUR', 2) + end + + it 'does not log the user in' do + # Travel to the beginning of an hour to avoid crossing rate-limit buckets + travel_to '2023-12-20T10:00:00Z' + + Auth::SessionsController::MAX_2FA_ATTEMPTS_PER_HOUR.times do + post :create, params: { user: { otp_attempt: '1234' } }, session: { attempt_user_id: user.id, attempt_user_updated_at: user.updated_at.to_s } + expect(controller.current_user).to be_nil + end + + post :create, params: { user: { otp_attempt: user.current_otp } }, session: { attempt_user_id: user.id, attempt_user_updated_at: user.updated_at.to_s } + expect(controller.current_user).to be_nil + expect(flash[:alert]).to match I18n.t('users.rate_limited') + end + end + context 'when using a valid OTP' do before do post :create, params: { user: { otp_attempt: user.current_otp } }, session: { attempt_user_id: user.id, attempt_user_updated_at: user.updated_at.to_s } From b377f82b1dcc7961f2f5e17b2830c1e61ad7a504 Mon Sep 17 00:00:00 2001 From: Claire Date: Fri, 19 Jan 2024 13:43:10 +0100 Subject: [PATCH 19/26] Fix processing of compacted single-item JSON-LD collections (#28816) --- .../fetch_featured_collection_service.rb | 4 +-- .../activitypub/fetch_replies_service.rb | 4 +-- .../synchronize_followers_service.rb | 4 +-- app/services/keys/query_service.rb | 2 +- .../fetch_featured_collection_service_spec.rb | 34 +++++++++++++++++-- .../activitypub/fetch_replies_service_spec.rb | 12 +++++++ 6 files changed, 51 insertions(+), 9 deletions(-) diff --git a/app/services/activitypub/fetch_featured_collection_service.rb b/app/services/activitypub/fetch_featured_collection_service.rb index e8a31dade9..d5d52c3cde 100644 --- a/app/services/activitypub/fetch_featured_collection_service.rb +++ b/app/services/activitypub/fetch_featured_collection_service.rb @@ -23,9 +23,9 @@ class ActivityPub::FetchFeaturedCollectionService < BaseService case collection['type'] when 'Collection', 'CollectionPage' - collection['items'] + as_array(collection['items']) when 'OrderedCollection', 'OrderedCollectionPage' - collection['orderedItems'] + as_array(collection['orderedItems']) end end diff --git a/app/services/activitypub/fetch_replies_service.rb b/app/services/activitypub/fetch_replies_service.rb index a9dd327e96..e2ecdef165 100644 --- a/app/services/activitypub/fetch_replies_service.rb +++ b/app/services/activitypub/fetch_replies_service.rb @@ -26,9 +26,9 @@ class ActivityPub::FetchRepliesService < BaseService case collection['type'] when 'Collection', 'CollectionPage' - collection['items'] + as_array(collection['items']) when 'OrderedCollection', 'OrderedCollectionPage' - collection['orderedItems'] + as_array(collection['orderedItems']) end end diff --git a/app/services/activitypub/synchronize_followers_service.rb b/app/services/activitypub/synchronize_followers_service.rb index 9bd6034a57..a9aab653ca 100644 --- a/app/services/activitypub/synchronize_followers_service.rb +++ b/app/services/activitypub/synchronize_followers_service.rb @@ -59,9 +59,9 @@ class ActivityPub::SynchronizeFollowersService < BaseService case collection['type'] when 'Collection', 'CollectionPage' - collection['items'] + as_array(collection['items']) when 'OrderedCollection', 'OrderedCollectionPage' - collection['orderedItems'] + as_array(collection['orderedItems']) end end diff --git a/app/services/keys/query_service.rb b/app/services/keys/query_service.rb index 14c9d9205b..33e13293f3 100644 --- a/app/services/keys/query_service.rb +++ b/app/services/keys/query_service.rb @@ -69,7 +69,7 @@ class Keys::QueryService < BaseService return if json['items'].blank? - @devices = json['items'].map do |device| + @devices = as_array(json['items']).map do |device| Device.new(device_id: device['id'], name: device['name'], identity_key: device.dig('identityKey', 'publicKeyBase64'), fingerprint_key: device.dig('fingerprintKey', 'publicKeyBase64'), claim_url: device['claim']) end rescue HTTP::Error, OpenSSL::SSL::SSLError, Mastodon::Error => e diff --git a/spec/services/activitypub/fetch_featured_collection_service_spec.rb b/spec/services/activitypub/fetch_featured_collection_service_spec.rb index 5975c81a10..a096296ffc 100644 --- a/spec/services/activitypub/fetch_featured_collection_service_spec.rb +++ b/spec/services/activitypub/fetch_featured_collection_service_spec.rb @@ -31,7 +31,7 @@ RSpec.describe ActivityPub::FetchFeaturedCollectionService, type: :service do } end - let(:status_json_pinned_unknown_unreachable) do + let(:status_json_pinned_unknown_reachable) do { '@context': 'https://www.w3.org/ns/activitystreams', type: 'Note', @@ -65,7 +65,7 @@ RSpec.describe ActivityPub::FetchFeaturedCollectionService, type: :service do stub_request(:get, 'https://example.com/account/pinned/known').to_return(status: 200, body: Oj.dump(status_json_pinned_known)) stub_request(:get, 'https://example.com/account/pinned/unknown-inlined').to_return(status: 200, body: Oj.dump(status_json_pinned_unknown_inlined)) stub_request(:get, 'https://example.com/account/pinned/unknown-unreachable').to_return(status: 404) - stub_request(:get, 'https://example.com/account/pinned/unknown-reachable').to_return(status: 200, body: Oj.dump(status_json_pinned_unknown_unreachable)) + stub_request(:get, 'https://example.com/account/pinned/unknown-reachable').to_return(status: 200, body: Oj.dump(status_json_pinned_unknown_reachable)) subject.call(actor, note: true, hashtag: false) end @@ -103,6 +103,21 @@ RSpec.describe ActivityPub::FetchFeaturedCollectionService, type: :service do end it_behaves_like 'sets pinned posts' + + context 'when there is a single item, with the array compacted away' do + let(:items) { 'https://example.com/account/pinned/unknown-reachable' } + + before do + stub_request(:get, 'https://example.com/account/pinned/unknown-reachable').to_return(status: 200, body: Oj.dump(status_json_pinned_unknown_reachable)) + subject.call(actor, note: true, hashtag: false) + end + + it 'sets expected posts as pinned posts' do + expect(actor.pinned_statuses.pluck(:uri)).to contain_exactly( + 'https://example.com/account/pinned/unknown-reachable' + ) + end + end end context 'when the endpoint is a paginated Collection' do @@ -124,6 +139,21 @@ RSpec.describe ActivityPub::FetchFeaturedCollectionService, type: :service do end it_behaves_like 'sets pinned posts' + + context 'when there is a single item, with the array compacted away' do + let(:items) { 'https://example.com/account/pinned/unknown-reachable' } + + before do + stub_request(:get, 'https://example.com/account/pinned/unknown-reachable').to_return(status: 200, body: Oj.dump(status_json_pinned_unknown_reachable)) + subject.call(actor, note: true, hashtag: false) + end + + it 'sets expected posts as pinned posts' do + expect(actor.pinned_statuses.pluck(:uri)).to contain_exactly( + 'https://example.com/account/pinned/unknown-reachable' + ) + end + end end end end diff --git a/spec/services/activitypub/fetch_replies_service_spec.rb b/spec/services/activitypub/fetch_replies_service_spec.rb index bf8e296764..62841d7d20 100644 --- a/spec/services/activitypub/fetch_replies_service_spec.rb +++ b/spec/services/activitypub/fetch_replies_service_spec.rb @@ -34,6 +34,18 @@ RSpec.describe ActivityPub::FetchRepliesService, type: :service do describe '#call' do context 'when the payload is a Collection with inlined replies' do + context 'when there is a single reply, with the array compacted away' do + let(:items) { 'http://example.com/self-reply-1' } + + it 'queues the expected worker' do + allow(FetchReplyWorker).to receive(:push_bulk) + + subject.call(status, payload) + + expect(FetchReplyWorker).to have_received(:push_bulk).with(['http://example.com/self-reply-1']) + end + end + context 'when passing the collection itself' do it 'spawns workers for up to 5 replies on the same server' do expect(FetchReplyWorker).to receive(:push_bulk).with(['http://example.com/self-reply-1', 'http://example.com/self-reply-2', 'http://example.com/self-reply-3', 'http://example.com/self-reply-4', 'http://example.com/self-reply-5']) From 779237f0546b9675e8a6f0b0d3a2dd37d396270f Mon Sep 17 00:00:00 2001 From: Claire Date: Fri, 19 Jan 2024 19:52:59 +0100 Subject: [PATCH 20/26] Fix error when processing remote files with unusually long names (#28823) --- lib/paperclip/response_with_limit_adapter.rb | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/lib/paperclip/response_with_limit_adapter.rb b/lib/paperclip/response_with_limit_adapter.rb index deb89717a4..ff7a938abb 100644 --- a/lib/paperclip/response_with_limit_adapter.rb +++ b/lib/paperclip/response_with_limit_adapter.rb @@ -16,7 +16,7 @@ module Paperclip private def cache_current_values - @original_filename = filename_from_content_disposition.presence || filename_from_path.presence || 'data' + @original_filename = truncated_filename @tempfile = copy_to_tempfile(@target) @content_type = ContentTypeDetector.new(@tempfile.path).detect @size = File.size(@tempfile) @@ -43,6 +43,13 @@ module Paperclip source.response.connection.close end + def truncated_filename + filename = filename_from_content_disposition.presence || filename_from_path.presence || 'data' + extension = File.extname(filename) + basename = File.basename(filename, extension) + [basename[...20], extension[..4]].compact_blank.join + end + def filename_from_content_disposition disposition = @target.response.headers['content-disposition'] disposition&.match(/filename="([^"]*)"/)&.captures&.first From c5c464804d7330211709a2be67fd8869000ce635 Mon Sep 17 00:00:00 2001 From: Claire Date: Tue, 23 Jan 2024 15:02:19 +0100 Subject: [PATCH 21/26] Update dependency puma to v6.4.2 --- Gemfile.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 76925a2bf2..41045b5e50 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -479,7 +479,7 @@ GEM net-smtp (0.3.3) net-protocol net-ssh (7.1.0) - nio4r (2.5.9) + nio4r (2.7.0) nokogiri (1.15.4) mini_portile2 (~> 2.8.2) racc (~> 1.4) @@ -534,7 +534,7 @@ GEM premailer (~> 1.7, >= 1.7.9) private_address_check (0.5.0) public_suffix (5.0.3) - puma (6.3.1) + puma (6.4.2) nio4r (~> 2.0) pundit (2.3.0) activesupport (>= 3.0.0) From 7a22999f925728ab8c51a9e1a262307d73a1c156 Mon Sep 17 00:00:00 2001 From: Claire Date: Tue, 23 Jan 2024 15:03:45 +0100 Subject: [PATCH 22/26] Bump ruby version to 3.2.3 --- .ruby-version | 2 +- Dockerfile | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.ruby-version b/.ruby-version index be94e6f53d..b347b11eac 100644 --- a/.ruby-version +++ b/.ruby-version @@ -1 +1 @@ -3.2.2 +3.2.3 diff --git a/Dockerfile b/Dockerfile index f73bdcf786..980d705092 100644 --- a/Dockerfile +++ b/Dockerfile @@ -2,7 +2,7 @@ # This needs to be bookworm-slim because the Ruby image is built on bookworm-slim ARG NODE_VERSION="20.6-bookworm-slim" -FROM ghcr.io/moritzheiber/ruby-jemalloc:3.2.2-slim as ruby +FROM ghcr.io/moritzheiber/ruby-jemalloc:3.2.3-slim as ruby FROM node:${NODE_VERSION} as build COPY --link --from=ruby /opt/ruby /opt/ruby From 4eb98ef7552835d0b9327c50da2d26b54725b5ae Mon Sep 17 00:00:00 2001 From: Claire Date: Mon, 15 Jan 2024 11:45:48 +0100 Subject: [PATCH 23/26] Ignore the devise-two-factor advisory as we have rate limits in place (#28733) --- .bundler-audit.yml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .bundler-audit.yml diff --git a/.bundler-audit.yml b/.bundler-audit.yml new file mode 100644 index 0000000000..0671df390f --- /dev/null +++ b/.bundler-audit.yml @@ -0,0 +1,6 @@ +--- +ignore: + # devise-two-factor advisory about brute-forcing TOTP + # We have rate-limits on authentication endpoints in place (including second + # factor verification) since Mastodon v3.2.0 + - CVE-2024-0227 From 1ab050eb5201699e3d5d8c1abf673d7cd97c699b Mon Sep 17 00:00:00 2001 From: Claire Date: Wed, 24 Jan 2024 14:59:09 +0100 Subject: [PATCH 24/26] Change PostgreSQL version check to check for PostgreSQL 10+ --- lib/tasks/db.rake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/tasks/db.rake b/lib/tasks/db.rake index e8a64b8fb2..f51a2459c5 100644 --- a/lib/tasks/db.rake +++ b/lib/tasks/db.rake @@ -17,7 +17,7 @@ namespace :db do task :pre_migration_check do version = ActiveRecord::Base.connection.select_one("SELECT current_setting('server_version_num') AS v")['v'].to_i - abort 'This version of Mastodon requires PostgreSQL 9.5 or newer. Please update PostgreSQL before updating Mastodon' if version < 90_500 + abort 'This version of Mastodon requires PostgreSQL 10.0 or newer. Please update PostgreSQL before updating Mastodon' if version < 100_000 end Rake::Task['db:migrate'].enhance(['db:pre_migration_check']) From 4633bb8ce09f216170c3f8370a8e48f3a41872cb Mon Sep 17 00:00:00 2001 From: Claire Date: Tue, 23 Jan 2024 14:58:27 +0100 Subject: [PATCH 25/26] Bump version to v4.2.4 --- CHANGELOG.md | 26 ++++++++++++++++++++++++++ docker-compose.yml | 6 +++--- lib/mastodon/version.rb | 2 +- 3 files changed, 30 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5f970519e4..8c733f9a96 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,32 @@ All notable changes to this project will be documented in this file. +## [4.2.4] - 2024-01-24 + +### Fixed + +- Fix error when processing remote files with unusually long names ([ClearlyClaire](https://github.com/mastodon/mastodon/pull/28823)) +- Fix processing of compacted single-item JSON-LD collections ([ClearlyClaire](https://github.com/mastodon/mastodon/pull/28816)) +- Retry 401 errors on replies fetching ([ShadowJonathan](https://github.com/mastodon/mastodon/pull/28788)) +- Fix `RecordNotUnique` errors in LinkCrawlWorker ([tribela](https://github.com/mastodon/mastodon/pull/28748)) +- Fix Mastodon not correctly processing HTTP Signatures with query strings ([ClearlyClaire](https://github.com/mastodon/mastodon/pull/28443), [ClearlyClaire](https://github.com/mastodon/mastodon/pull/28476)) +- Fix potential redirection loop of streaming endpoint ([ClearlyClaire](https://github.com/mastodon/mastodon/pull/28665)) +- Fix streaming API redirection ignoring the port of `streaming_api_base_url` ([ClearlyClaire](https://github.com/mastodon/mastodon/pull/28558)) +- Fix error when processing link preview with an array as `inLanguage` ([ClearlyClaire](https://github.com/mastodon/mastodon/pull/28252)) +- Fix unsupported time zone or locale preventing sign-up ([Gargron](https://github.com/mastodon/mastodon/pull/28035)) +- Fix "Hide these posts from home" list setting not refreshing when switching lists ([brianholley](https://github.com/mastodon/mastodon/pull/27763)) +- Fix missing background behind dismissable banner in web UI ([Gargron](https://github.com/mastodon/mastodon/pull/27479)) +- Fix line wrapping of language selection button with long locale codes ([gunchleoc](https://github.com/mastodon/mastodon/pull/27100), [ClearlyClaire](https://github.com/mastodon/mastodon/pull/27127)) +- Fix `Undo Announce` activity not being sent to non-follower authors ([MitarashiDango](https://github.com/mastodon/mastodon/pull/18482)) +- Fix N+1s because of association preloaders not actually getting called ([ClearlyClaire](https://github.com/mastodon/mastodon/pull/28339)) +- Fix empty column explainer getting cropped under certain conditions ([ClearlyClaire](https://github.com/mastodon/mastodon/pull/28337)) +- Fix `LinkCrawlWorker` error when encountering empty OEmbed response ([ClearlyClaire](https://github.com/mastodon/mastodon/pull/28268)) +- Fix call to inefficient `delete_matched` cache method in domain blocks ([ClearlyClaire](https://github.com/mastodon/mastodon/pull/28367)) + +### Security + +- Add rate-limit of TOTP authentication attempts at controller level ([ClearlyClaire](https://github.com/mastodon/mastodon/pull/28801)) + ## [4.2.3] - 2023-12-05 ### Fixed diff --git a/docker-compose.yml b/docker-compose.yml index dda71cb091..b096012de1 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -56,7 +56,7 @@ services: web: build: . - image: ghcr.io/mastodon/mastodon:v4.2.3 + image: ghcr.io/mastodon/mastodon:v4.2.4 restart: always env_file: .env.production command: bash -c "rm -f /mastodon/tmp/pids/server.pid; bundle exec rails s -p 3000" @@ -77,7 +77,7 @@ services: streaming: build: . - image: ghcr.io/mastodon/mastodon:v4.2.3 + image: ghcr.io/mastodon/mastodon:v4.2.4 restart: always env_file: .env.production command: node ./streaming @@ -95,7 +95,7 @@ services: sidekiq: build: . - image: ghcr.io/mastodon/mastodon:v4.2.3 + image: ghcr.io/mastodon/mastodon:v4.2.4 restart: always env_file: .env.production command: bundle exec sidekiq diff --git a/lib/mastodon/version.rb b/lib/mastodon/version.rb index 2e68df4744..6382712cd7 100644 --- a/lib/mastodon/version.rb +++ b/lib/mastodon/version.rb @@ -13,7 +13,7 @@ module Mastodon end def patch - 3 + 4 end def default_prerelease From a6641f828b9e6f5806be01754318279c2532ae82 Mon Sep 17 00:00:00 2001 From: Claire Date: Thu, 1 Feb 2024 15:56:46 +0100 Subject: [PATCH 26/26] Merge pull request from GHSA-3fjr-858r-92rw * Fix insufficient origin validation * Bump version to v4.2.5 --- CHANGELOG.md | 6 ++++++ SECURITY.md | 4 +--- .../concerns/signature_verification.rb | 2 +- app/helpers/jsonld_helper.rb | 4 ++-- app/lib/activitypub/activity.rb | 2 +- app/lib/activitypub/linked_data_signature.rb | 2 +- .../activitypub/fetch_remote_account_service.rb | 2 +- .../activitypub/fetch_remote_actor_service.rb | 6 +++--- .../activitypub/fetch_remote_key_service.rb | 17 ++--------------- .../activitypub/fetch_remote_status_service.rb | 8 ++++---- .../activitypub/process_account_service.rb | 2 +- app/services/fetch_resource_service.rb | 10 +++++++++- docker-compose.yml | 6 +++--- lib/mastodon/version.rb | 2 +- .../activitypub/linked_data_signature_spec.rb | 4 ++-- .../fetch_remote_account_service_spec.rb | 2 +- .../fetch_remote_actor_service_spec.rb | 2 +- .../fetch_remote_key_service_spec.rb | 2 +- spec/services/fetch_resource_service_spec.rb | 10 +++++----- spec/services/resolve_url_service_spec.rb | 1 + 20 files changed, 47 insertions(+), 47 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8c733f9a96..a34545be17 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,12 @@ All notable changes to this project will be documented in this file. +## [4.2.5] - 2024-02-01 + +### Security + +- Fix insufficient origin validation (CVE-2024-23832, [GHSA-3fjr-858r-92rw](https://github.com/mastodon/mastodon/security/advisories/GHSA-3fjr-858r-92rw)) + ## [4.2.4] - 2024-01-24 ### Fixed diff --git a/SECURITY.md b/SECURITY.md index 954ff73a24..667096324e 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -17,6 +17,4 @@ A "vulnerability in Mastodon" is a vulnerability in the code distributed through | ------- | ---------------- | | 4.2.x | Yes | | 4.1.x | Yes | -| 4.0.x | No | -| 3.5.x | Until 2023-12-31 | -| < 3.5 | No | +| < 4.1 | No | diff --git a/app/controllers/concerns/signature_verification.rb b/app/controllers/concerns/signature_verification.rb index 35391e64c4..92f1eb5a16 100644 --- a/app/controllers/concerns/signature_verification.rb +++ b/app/controllers/concerns/signature_verification.rb @@ -266,7 +266,7 @@ module SignatureVerification stoplight_wrap_request { ResolveAccountService.new.call(key_id.delete_prefix('acct:'), suppress_errors: false) } elsif !ActivityPub::TagManager.instance.local_uri?(key_id) account = ActivityPub::TagManager.instance.uri_to_actor(key_id) - account ||= stoplight_wrap_request { ActivityPub::FetchRemoteKeyService.new.call(key_id, id: false, suppress_errors: false) } + account ||= stoplight_wrap_request { ActivityPub::FetchRemoteKeyService.new.call(key_id, suppress_errors: false) } account end rescue Mastodon::PrivateNetworkAddressError => e diff --git a/app/helpers/jsonld_helper.rb b/app/helpers/jsonld_helper.rb index b3d0d032c4..cc05b7a403 100644 --- a/app/helpers/jsonld_helper.rb +++ b/app/helpers/jsonld_helper.rb @@ -155,8 +155,8 @@ module JsonLdHelper end end - def fetch_resource(uri, id, on_behalf_of = nil, request_options: {}) - unless id + def fetch_resource(uri, id_is_known, on_behalf_of = nil, request_options: {}) + unless id_is_known json = fetch_resource_without_id_validation(uri, on_behalf_of) return if !json.is_a?(Hash) || unsupported_uri_scheme?(json['id']) diff --git a/app/lib/activitypub/activity.rb b/app/lib/activitypub/activity.rb index 51384ef984..322f3e27ad 100644 --- a/app/lib/activitypub/activity.rb +++ b/app/lib/activitypub/activity.rb @@ -154,7 +154,7 @@ class ActivityPub::Activity if object_uri.start_with?('http') return if ActivityPub::TagManager.instance.local_uri?(object_uri) - ActivityPub::FetchRemoteStatusService.new.call(object_uri, id: true, on_behalf_of: @account.followers.local.first, request_id: @options[:request_id]) + ActivityPub::FetchRemoteStatusService.new.call(object_uri, on_behalf_of: @account.followers.local.first, request_id: @options[:request_id]) elsif @object['url'].present? ::FetchRemoteStatusService.new.call(@object['url'], request_id: @options[:request_id]) end diff --git a/app/lib/activitypub/linked_data_signature.rb b/app/lib/activitypub/linked_data_signature.rb index faea63e8f1..9459fdd8b7 100644 --- a/app/lib/activitypub/linked_data_signature.rb +++ b/app/lib/activitypub/linked_data_signature.rb @@ -19,7 +19,7 @@ class ActivityPub::LinkedDataSignature return unless type == 'RsaSignature2017' creator = ActivityPub::TagManager.instance.uri_to_actor(creator_uri) - creator = ActivityPub::FetchRemoteKeyService.new.call(creator_uri, id: false) if creator&.public_key.blank? + creator = ActivityPub::FetchRemoteKeyService.new.call(creator_uri) if creator&.public_key.blank? return if creator.nil? diff --git a/app/services/activitypub/fetch_remote_account_service.rb b/app/services/activitypub/fetch_remote_account_service.rb index 567dd8a14a..7b083d889b 100644 --- a/app/services/activitypub/fetch_remote_account_service.rb +++ b/app/services/activitypub/fetch_remote_account_service.rb @@ -2,7 +2,7 @@ class ActivityPub::FetchRemoteAccountService < ActivityPub::FetchRemoteActorService # Does a WebFinger roundtrip on each call, unless `only_key` is true - def call(uri, id: true, prefetched_body: nil, break_on_redirect: false, only_key: false, suppress_errors: true, request_id: nil) + def call(uri, prefetched_body: nil, break_on_redirect: false, only_key: false, suppress_errors: true, request_id: nil) actor = super return actor if actor.nil? || actor.is_a?(Account) diff --git a/app/services/activitypub/fetch_remote_actor_service.rb b/app/services/activitypub/fetch_remote_actor_service.rb index 8df8c75876..86a134bb4e 100644 --- a/app/services/activitypub/fetch_remote_actor_service.rb +++ b/app/services/activitypub/fetch_remote_actor_service.rb @@ -10,15 +10,15 @@ class ActivityPub::FetchRemoteActorService < BaseService SUPPORTED_TYPES = %w(Application Group Organization Person Service).freeze # Does a WebFinger roundtrip on each call, unless `only_key` is true - def call(uri, id: true, prefetched_body: nil, break_on_redirect: false, only_key: false, suppress_errors: true, request_id: nil) + def call(uri, prefetched_body: nil, break_on_redirect: false, only_key: false, suppress_errors: true, request_id: nil) return if domain_not_allowed?(uri) return ActivityPub::TagManager.instance.uri_to_actor(uri) if ActivityPub::TagManager.instance.local_uri?(uri) @json = begin if prefetched_body.nil? - fetch_resource(uri, id) + fetch_resource(uri, true) else - body_to_json(prefetched_body, compare_id: id ? uri : nil) + body_to_json(prefetched_body, compare_id: uri) end rescue Oj::ParseError raise Error, "Error parsing JSON-LD document #{uri}" diff --git a/app/services/activitypub/fetch_remote_key_service.rb b/app/services/activitypub/fetch_remote_key_service.rb index 8eb97c1e66..e96b5ad3bb 100644 --- a/app/services/activitypub/fetch_remote_key_service.rb +++ b/app/services/activitypub/fetch_remote_key_service.rb @@ -6,23 +6,10 @@ class ActivityPub::FetchRemoteKeyService < BaseService class Error < StandardError; end # Returns actor that owns the key - def call(uri, id: true, prefetched_body: nil, suppress_errors: true) + def call(uri, suppress_errors: true) raise Error, 'No key URI given' if uri.blank? - if prefetched_body.nil? - if id - @json = fetch_resource_without_id_validation(uri) - if actor_type? - @json = fetch_resource(@json['id'], true) - elsif uri != @json['id'] - raise Error, "Fetched URI #{uri} has wrong id #{@json['id']}" - end - else - @json = fetch_resource(uri, id) - end - else - @json = body_to_json(prefetched_body, compare_id: id ? uri : nil) - end + @json = fetch_resource(uri, false) raise Error, "Unable to fetch key JSON at #{uri}" if @json.nil? raise Error, "Unsupported JSON-LD context for document #{uri}" unless supported_context?(@json) diff --git a/app/services/activitypub/fetch_remote_status_service.rb b/app/services/activitypub/fetch_remote_status_service.rb index a491b32b26..5a3eeeaf4e 100644 --- a/app/services/activitypub/fetch_remote_status_service.rb +++ b/app/services/activitypub/fetch_remote_status_service.rb @@ -8,14 +8,14 @@ class ActivityPub::FetchRemoteStatusService < BaseService DISCOVERIES_PER_REQUEST = 1000 # Should be called when uri has already been checked for locality - def call(uri, id: true, prefetched_body: nil, on_behalf_of: nil, expected_actor_uri: nil, request_id: nil) + def call(uri, prefetched_body: nil, on_behalf_of: nil, expected_actor_uri: nil, request_id: nil) return if domain_not_allowed?(uri) @request_id = request_id || "#{Time.now.utc.to_i}-status-#{uri}" @json = if prefetched_body.nil? - fetch_resource(uri, id, on_behalf_of) + fetch_resource(uri, true, on_behalf_of) else - body_to_json(prefetched_body, compare_id: id ? uri : nil) + body_to_json(prefetched_body, compare_id: uri) end return unless supported_context? @@ -65,7 +65,7 @@ class ActivityPub::FetchRemoteStatusService < BaseService def account_from_uri(uri) actor = ActivityPub::TagManager.instance.uri_to_resource(uri, Account) - actor = ActivityPub::FetchRemoteAccountService.new.call(uri, id: true, request_id: @request_id) if actor.nil? || actor.possibly_stale? + actor = ActivityPub::FetchRemoteAccountService.new.call(uri, request_id: @request_id) if actor.nil? || actor.possibly_stale? actor end diff --git a/app/services/activitypub/process_account_service.rb b/app/services/activitypub/process_account_service.rb index 1304ca8242..dc845ce97c 100644 --- a/app/services/activitypub/process_account_service.rb +++ b/app/services/activitypub/process_account_service.rb @@ -277,7 +277,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, break_on_redirect: true, request_id: @options[:request_id]) + account ||= ActivityPub::FetchRemoteAccountService.new.call(@json['movedTo'], break_on_redirect: true, request_id: @options[:request_id]) account end diff --git a/app/services/fetch_resource_service.rb b/app/services/fetch_resource_service.rb index a3406e5a57..71c6cca790 100644 --- a/app/services/fetch_resource_service.rb +++ b/app/services/fetch_resource_service.rb @@ -48,7 +48,15 @@ class FetchResourceService < BaseService body = response.body_with_limit json = body_to_json(body) - [json['id'], { prefetched_body: body, id: true }] if supported_context?(json) && (equals_or_includes_any?(json['type'], ActivityPub::FetchRemoteActorService::SUPPORTED_TYPES) || expected_type?(json)) + return unless supported_context?(json) && (equals_or_includes_any?(json['type'], ActivityPub::FetchRemoteActorService::SUPPORTED_TYPES) || expected_type?(json)) + + if json['id'] != @url + return if terminal + + return process(json['id'], terminal: true) + end + + [@url, { prefetched_body: body }] elsif !terminal link_header = response['Link'] && parse_link_header(response) diff --git a/docker-compose.yml b/docker-compose.yml index b096012de1..8aa93b1c08 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -56,7 +56,7 @@ services: web: build: . - image: ghcr.io/mastodon/mastodon:v4.2.4 + image: ghcr.io/mastodon/mastodon:v4.2.5 restart: always env_file: .env.production command: bash -c "rm -f /mastodon/tmp/pids/server.pid; bundle exec rails s -p 3000" @@ -77,7 +77,7 @@ services: streaming: build: . - image: ghcr.io/mastodon/mastodon:v4.2.4 + image: ghcr.io/mastodon/mastodon:v4.2.5 restart: always env_file: .env.production command: node ./streaming @@ -95,7 +95,7 @@ services: sidekiq: build: . - image: ghcr.io/mastodon/mastodon:v4.2.4 + image: ghcr.io/mastodon/mastodon:v4.2.5 restart: always env_file: .env.production command: bundle exec sidekiq diff --git a/lib/mastodon/version.rb b/lib/mastodon/version.rb index 6382712cd7..f9f801c771 100644 --- a/lib/mastodon/version.rb +++ b/lib/mastodon/version.rb @@ -13,7 +13,7 @@ module Mastodon end def patch - 4 + 5 end def default_prerelease diff --git a/spec/lib/activitypub/linked_data_signature_spec.rb b/spec/lib/activitypub/linked_data_signature_spec.rb index 03d0c5a878..e821cee6b3 100644 --- a/spec/lib/activitypub/linked_data_signature_spec.rb +++ b/spec/lib/activitypub/linked_data_signature_spec.rb @@ -60,7 +60,7 @@ RSpec.describe ActivityPub::LinkedDataSignature do allow(ActivityPub::FetchRemoteKeyService).to receive(:new).and_return(service_stub) - allow(service_stub).to receive(:call).with('http://example.com/alice', id: false) do + allow(service_stub).to receive(:call).with('http://example.com/alice') do sender.update!(public_key: old_key) sender end @@ -68,7 +68,7 @@ RSpec.describe ActivityPub::LinkedDataSignature do it 'fetches key and returns creator' do expect(subject.verify_actor!).to eq sender - expect(service_stub).to have_received(:call).with('http://example.com/alice', id: false).once + expect(service_stub).to have_received(:call).with('http://example.com/alice').once end end diff --git a/spec/services/activitypub/fetch_remote_account_service_spec.rb b/spec/services/activitypub/fetch_remote_account_service_spec.rb index ac7484d96d..f33a928da6 100644 --- a/spec/services/activitypub/fetch_remote_account_service_spec.rb +++ b/spec/services/activitypub/fetch_remote_account_service_spec.rb @@ -18,7 +18,7 @@ RSpec.describe ActivityPub::FetchRemoteAccountService, type: :service do end describe '#call' do - let(:account) { subject.call('https://example.com/alice', id: true) } + let(:account) { subject.call('https://example.com/alice') } shared_examples 'sets profile data' do it 'returns an account' do diff --git a/spec/services/activitypub/fetch_remote_actor_service_spec.rb b/spec/services/activitypub/fetch_remote_actor_service_spec.rb index 93d31b69d5..944a2f8b1c 100644 --- a/spec/services/activitypub/fetch_remote_actor_service_spec.rb +++ b/spec/services/activitypub/fetch_remote_actor_service_spec.rb @@ -18,7 +18,7 @@ RSpec.describe ActivityPub::FetchRemoteActorService, type: :service do end describe '#call' do - let(:account) { subject.call('https://example.com/alice', id: true) } + let(:account) { subject.call('https://example.com/alice') } shared_examples 'sets profile data' do it 'returns an account' do diff --git a/spec/services/activitypub/fetch_remote_key_service_spec.rb b/spec/services/activitypub/fetch_remote_key_service_spec.rb index e210d20ec7..0b14da4f44 100644 --- a/spec/services/activitypub/fetch_remote_key_service_spec.rb +++ b/spec/services/activitypub/fetch_remote_key_service_spec.rb @@ -55,7 +55,7 @@ RSpec.describe ActivityPub::FetchRemoteKeyService, type: :service do end describe '#call' do - let(:account) { subject.call(public_key_id, id: false) } + let(:account) { subject.call(public_key_id) } context 'when the key is a sub-object from the actor' do before do diff --git a/spec/services/fetch_resource_service_spec.rb b/spec/services/fetch_resource_service_spec.rb index 0f1068471f..78037a06ce 100644 --- a/spec/services/fetch_resource_service_spec.rb +++ b/spec/services/fetch_resource_service_spec.rb @@ -57,7 +57,7 @@ RSpec.describe FetchResourceService, type: :service do let(:json) do { - id: 1, + id: 'http://example.com/foo', '@context': ActivityPub::TagManager::CONTEXT, type: 'Note', }.to_json @@ -83,27 +83,27 @@ RSpec.describe FetchResourceService, type: :service do let(:content_type) { 'application/activity+json; charset=utf-8' } let(:body) { json } - it { is_expected.to eq [1, { prefetched_body: body, id: true }] } + it { is_expected.to eq ['http://example.com/foo', { prefetched_body: body }] } end context 'when content type is ld+json with profile' do let(:content_type) { 'application/ld+json; profile="https://www.w3.org/ns/activitystreams"' } let(:body) { json } - it { is_expected.to eq [1, { prefetched_body: body, id: true }] } + it { is_expected.to eq ['http://example.com/foo', { prefetched_body: body }] } end context 'when link header is present' do let(:headers) { { 'Link' => '; rel="alternate"; type="application/activity+json"' } } - it { is_expected.to eq [1, { prefetched_body: json, id: true }] } + it { is_expected.to eq ['http://example.com/foo', { prefetched_body: json }] } end context 'when content type is text/html' do let(:content_type) { 'text/html' } let(:body) { '' } - it { is_expected.to eq [1, { prefetched_body: json, id: true }] } + it { is_expected.to eq ['http://example.com/foo', { prefetched_body: json }] } end end end diff --git a/spec/services/resolve_url_service_spec.rb b/spec/services/resolve_url_service_spec.rb index 7991aa6ef1..38d35a3a18 100644 --- a/spec/services/resolve_url_service_spec.rb +++ b/spec/services/resolve_url_service_spec.rb @@ -139,6 +139,7 @@ describe ResolveURLService, type: :service do stub_request(:get, url).to_return(status: 302, headers: { 'Location' => status_url }) body = ActiveModelSerializers::SerializableResource.new(status, serializer: ActivityPub::NoteSerializer, adapter: ActivityPub::Adapter).to_json stub_request(:get, status_url).to_return(body: body, headers: { 'Content-Type' => 'application/activity+json' }) + stub_request(:get, uri).to_return(body: body, headers: { 'Content-Type' => 'application/activity+json' }) end it 'returns status by url' do