From 50948b46aabc0756d85bc6641f0bd3bcc09bf7d4 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Tue, 20 Sep 2022 23:51:21 +0200 Subject: Add ability to filter followed accounts' posts by language (#19095) --- app/workers/refollow_worker.rb | 7 ++++--- app/workers/unfollow_follow_worker.rb | 9 +++++---- 2 files changed, 9 insertions(+), 7 deletions(-) (limited to 'app/workers') diff --git a/app/workers/refollow_worker.rb b/app/workers/refollow_worker.rb index 319b00109..4b712d3aa 100644 --- a/app/workers/refollow_worker.rb +++ b/app/workers/refollow_worker.rb @@ -10,8 +10,9 @@ class RefollowWorker return unless target_account.activitypub? target_account.passive_relationships.where(account: Account.where(domain: nil)).includes(:account).reorder(nil).find_each do |follow| - reblogs = follow.show_reblogs? - notify = follow.notify? + reblogs = follow.show_reblogs? + notify = follow.notify? + languages = follow.languages # Locally unfollow remote account follower = follow.account @@ -19,7 +20,7 @@ class RefollowWorker # Schedule re-follow begin - FollowService.new.call(follower, target_account, reblogs: reblogs, notify: notify, bypass_limit: true) + FollowService.new.call(follower, target_account, reblogs: reblogs, notify: notify, languages: languages, bypass_limit: true) rescue Mastodon::NotPermittedError, ActiveRecord::RecordNotFound, Mastodon::UnexpectedResponseError, HTTP::Error, OpenSSL::SSL::SSLError next end diff --git a/app/workers/unfollow_follow_worker.rb b/app/workers/unfollow_follow_worker.rb index 0bd5ff472..7203b4888 100644 --- a/app/workers/unfollow_follow_worker.rb +++ b/app/workers/unfollow_follow_worker.rb @@ -10,11 +10,12 @@ class UnfollowFollowWorker old_target_account = Account.find(old_target_account_id) new_target_account = Account.find(new_target_account_id) - follow = follower_account.active_relationships.find_by(target_account: old_target_account) - reblogs = follow&.show_reblogs? - notify = follow&.notify? + follow = follower_account.active_relationships.find_by(target_account: old_target_account) + reblogs = follow&.show_reblogs? + notify = follow&.notify? + languages = follow&.languages - FollowService.new.call(follower_account, new_target_account, reblogs: reblogs, notify: notify, bypass_locked: bypass_locked, bypass_limit: true) + FollowService.new.call(follower_account, new_target_account, reblogs: reblogs, notify: notify, languages: languages, bypass_locked: bypass_locked, bypass_limit: true) UnfollowService.new.call(follower_account, old_target_account, skip_unmerge: true) rescue ActiveRecord::RecordNotFound, Mastodon::NotPermittedError true -- cgit From 8cf7006d4efbcfdd4a4ab688db1bcc73a2915a47 Mon Sep 17 00:00:00 2001 From: Claire Date: Wed, 21 Sep 2022 22:45:57 +0200 Subject: Refactor ActivityPub handling to prepare for non-Account actors (#19212) * Move ActivityPub::FetchRemoteAccountService to ActivityPub::FetchRemoteActorService ActivityPub::FetchRemoteAccountService is kept as a wrapper for when the actor is specifically required to be an Account * Refactor SignatureVerification to allow non-Account actors * fixup! Move ActivityPub::FetchRemoteAccountService to ActivityPub::FetchRemoteActorService * Refactor ActivityPub::FetchRemoteKeyService to potentially return non-Account actors * Refactor inbound ActivityPub payload processing to accept non-Account actors * Refactor inbound ActivityPub processing to accept activities relayed through non-Account * Refactor how Account key URIs are built * Refactor Request and drop unused key_id_format parameter * Rename ActivityPub::Dereferencer `signature_account` to `signature_actor` --- app/controllers/accounts_controller.rb | 2 +- app/controllers/activitypub/claims_controller.rb | 2 +- .../activitypub/collections_controller.rb | 2 +- .../followers_synchronizations_controller.rb | 2 +- app/controllers/activitypub/inboxes_controller.rb | 10 +- app/controllers/activitypub/outboxes_controller.rb | 2 +- app/controllers/activitypub/replies_controller.rb | 2 +- app/controllers/concerns/signature_verification.rb | 52 +++--- app/controllers/follower_accounts_controller.rb | 2 +- app/controllers/following_accounts_controller.rb | 2 +- app/controllers/statuses_controller.rb | 2 +- app/controllers/tags_controller.rb | 2 +- app/lib/activitypub/activity.rb | 10 +- app/lib/activitypub/dereferencer.rb | 6 +- app/lib/activitypub/linked_data_signature.rb | 6 +- app/lib/activitypub/tag_manager.rb | 8 + app/lib/request.rb | 18 +-- .../activitypub/public_key_serializer.rb | 2 +- .../activitypub/fetch_remote_account_service.rb | 78 +-------- .../activitypub/fetch_remote_actor_service.rb | 80 +++++++++ .../activitypub/fetch_remote_key_service.rb | 22 +-- .../activitypub/process_collection_service.rb | 11 +- app/services/fetch_resource_service.rb | 2 +- app/services/keys/claim_service.rb | 2 +- app/services/resolve_url_service.rb | 4 +- app/workers/activitypub/delivery_worker.rb | 2 +- app/workers/activitypub/processing_worker.rb | 12 +- spec/controllers/accounts_controller_spec.rb | 2 +- .../activitypub/collections_controller_spec.rb | 2 +- .../followers_synchronizations_controller_spec.rb | 2 +- .../activitypub/inboxes_controller_spec.rb | 2 +- .../activitypub/outboxes_controller_spec.rb | 2 +- .../activitypub/replies_controller_spec.rb | 2 +- .../concerns/signature_verification_spec.rb | 45 ++++++ spec/controllers/statuses_controller_spec.rb | 2 +- spec/lib/activitypub/activity/announce_spec.rb | 2 +- spec/lib/activitypub/dereferencer_spec.rb | 8 +- spec/lib/activitypub/linked_data_signature_spec.rb | 14 +- .../activitypub/fetch_remote_actor_service_spec.rb | 180 +++++++++++++++++++++ .../activitypub/process_collection_service_spec.rb | 6 +- spec/services/fetch_resource_service_spec.rb | 2 +- 41 files changed, 436 insertions(+), 180 deletions(-) create mode 100644 app/services/activitypub/fetch_remote_actor_service.rb create mode 100644 spec/services/activitypub/fetch_remote_actor_service_spec.rb (limited to 'app/workers') diff --git a/app/controllers/accounts_controller.rb b/app/controllers/accounts_controller.rb index fe7d934dc..d92f91b30 100644 --- a/app/controllers/accounts_controller.rb +++ b/app/controllers/accounts_controller.rb @@ -7,7 +7,7 @@ class AccountsController < ApplicationController include AccountControllerConcern include SignatureAuthentication - before_action :require_signature!, if: -> { request.format == :json && authorized_fetch_mode? } + before_action :require_account_signature!, if: -> { request.format == :json && authorized_fetch_mode? } before_action :set_cache_headers before_action :set_body_classes diff --git a/app/controllers/activitypub/claims_controller.rb b/app/controllers/activitypub/claims_controller.rb index 08ad952df..339333e46 100644 --- a/app/controllers/activitypub/claims_controller.rb +++ b/app/controllers/activitypub/claims_controller.rb @@ -6,7 +6,7 @@ class ActivityPub::ClaimsController < ActivityPub::BaseController skip_before_action :authenticate_user! - before_action :require_signature! + before_action :require_account_signature! before_action :set_claim_result def create diff --git a/app/controllers/activitypub/collections_controller.rb b/app/controllers/activitypub/collections_controller.rb index e4e994a98..d94a285ea 100644 --- a/app/controllers/activitypub/collections_controller.rb +++ b/app/controllers/activitypub/collections_controller.rb @@ -4,7 +4,7 @@ class ActivityPub::CollectionsController < ActivityPub::BaseController include SignatureVerification include AccountOwnedConcern - before_action :require_signature!, if: :authorized_fetch_mode? + before_action :require_account_signature!, if: :authorized_fetch_mode? before_action :set_items before_action :set_size before_action :set_type diff --git a/app/controllers/activitypub/followers_synchronizations_controller.rb b/app/controllers/activitypub/followers_synchronizations_controller.rb index 940b77cf0..4e445bcb1 100644 --- a/app/controllers/activitypub/followers_synchronizations_controller.rb +++ b/app/controllers/activitypub/followers_synchronizations_controller.rb @@ -4,7 +4,7 @@ class ActivityPub::FollowersSynchronizationsController < ActivityPub::BaseContro include SignatureVerification include AccountOwnedConcern - before_action :require_signature! + before_action :require_account_signature! before_action :set_items before_action :set_cache_headers diff --git a/app/controllers/activitypub/inboxes_controller.rb b/app/controllers/activitypub/inboxes_controller.rb index 92dcb5ac7..5ee85474e 100644 --- a/app/controllers/activitypub/inboxes_controller.rb +++ b/app/controllers/activitypub/inboxes_controller.rb @@ -6,7 +6,7 @@ class ActivityPub::InboxesController < ActivityPub::BaseController include AccountOwnedConcern before_action :skip_unknown_actor_activity - before_action :require_signature! + before_action :require_actor_signature! skip_before_action :authenticate_user! def create @@ -49,17 +49,17 @@ class ActivityPub::InboxesController < ActivityPub::BaseController end def upgrade_account - if signed_request_account.ostatus? + if signed_request_account&.ostatus? signed_request_account.update(last_webfingered_at: nil) ResolveAccountWorker.perform_async(signed_request_account.acct) end - DeliveryFailureTracker.reset!(signed_request_account.inbox_url) + DeliveryFailureTracker.reset!(signed_request_actor.inbox_url) end def process_collection_synchronization raw_params = request.headers['Collection-Synchronization'] - return if raw_params.blank? || ENV['DISABLE_FOLLOWERS_SYNCHRONIZATION'] == 'true' + return if raw_params.blank? || ENV['DISABLE_FOLLOWERS_SYNCHRONIZATION'] == 'true' || signed_request_account.nil? # Re-using the syntax for signature parameters tree = SignatureParamsParser.new.parse(raw_params) @@ -71,6 +71,6 @@ class ActivityPub::InboxesController < ActivityPub::BaseController end def process_payload - ActivityPub::ProcessingWorker.perform_async(signed_request_account.id, body, @account&.id) + ActivityPub::ProcessingWorker.perform_async(signed_request_actor.id, body, @account&.id, signed_request_actor.class.name) end end diff --git a/app/controllers/activitypub/outboxes_controller.rb b/app/controllers/activitypub/outboxes_controller.rb index cd3992502..60d201f76 100644 --- a/app/controllers/activitypub/outboxes_controller.rb +++ b/app/controllers/activitypub/outboxes_controller.rb @@ -6,7 +6,7 @@ class ActivityPub::OutboxesController < ActivityPub::BaseController include SignatureVerification include AccountOwnedConcern - before_action :require_signature!, if: :authorized_fetch_mode? + before_action :require_account_signature!, if: :authorized_fetch_mode? before_action :set_statuses before_action :set_cache_headers diff --git a/app/controllers/activitypub/replies_controller.rb b/app/controllers/activitypub/replies_controller.rb index 4ff7cfa08..8e0f9de2e 100644 --- a/app/controllers/activitypub/replies_controller.rb +++ b/app/controllers/activitypub/replies_controller.rb @@ -7,7 +7,7 @@ class ActivityPub::RepliesController < ActivityPub::BaseController DESCENDANTS_LIMIT = 60 - before_action :require_signature!, if: :authorized_fetch_mode? + before_action :require_account_signature!, if: :authorized_fetch_mode? before_action :set_status before_action :set_cache_headers before_action :set_replies diff --git a/app/controllers/concerns/signature_verification.rb b/app/controllers/concerns/signature_verification.rb index 4da068aed..2394574b3 100644 --- a/app/controllers/concerns/signature_verification.rb +++ b/app/controllers/concerns/signature_verification.rb @@ -45,10 +45,14 @@ module SignatureVerification end end - def require_signature! + def require_account_signature! render plain: signature_verification_failure_reason, status: signature_verification_failure_code unless signed_request_account end + def require_actor_signature! + render plain: signature_verification_failure_reason, status: signature_verification_failure_code unless signed_request_actor + end + def signed_request? request.headers['Signature'].present? end @@ -68,7 +72,11 @@ module SignatureVerification end def signed_request_account - return @signed_request_account if defined?(@signed_request_account) + signed_request_actor.is_a?(Account) ? signed_request_actor : nil + end + + def signed_request_actor + return @signed_request_actor if defined?(@signed_request_actor) raise SignatureVerificationError, 'Request not signed' unless signed_request? raise SignatureVerificationError, 'Incompatible request signature. keyId and signature are required' if missing_required_signature_parameters? @@ -78,22 +86,22 @@ module SignatureVerification verify_signature_strength! verify_body_digest! - account = account_from_key_id(signature_params['keyId']) + actor = actor_from_key_id(signature_params['keyId']) - raise SignatureVerificationError, "Public key not found for key #{signature_params['keyId']}" if account.nil? + 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 - return account unless verify_signature(account, signature, compare_signed_string).nil? + return actor unless verify_signature(actor, signature, compare_signed_string).nil? - account = stoplight_wrap_request { account.possibly_stale? ? account.refresh! : account_refresh_key(account) } + actor = stoplight_wrap_request { actor_refresh_key!(actor) } - raise SignatureVerificationError, "Public key not found for key #{signature_params['keyId']}" if account.nil? + raise SignatureVerificationError, "Public key not found for key #{signature_params['keyId']}" if actor.nil? - return account unless verify_signature(account, signature, compare_signed_string).nil? + return actor unless verify_signature(actor, signature, compare_signed_string).nil? - fail_with! "Verification failed for #{account.username}@#{account.domain} #{account.uri} using rsa-sha256 (RSASSA-PKCS1-v1_5 with SHA-256)" + fail_with! "Verification failed for #{actor.to_log_human_identifier} #{actor.uri} using rsa-sha256 (RSASSA-PKCS1-v1_5 with SHA-256)" rescue SignatureVerificationError => e fail_with! e.message rescue HTTP::Error, OpenSSL::SSL::SSLError => e @@ -112,7 +120,7 @@ module SignatureVerification def fail_with!(message) @signature_verification_failure_reason = message - @signed_request_account = nil + @signed_request_actor = nil end def signature_params @@ -160,10 +168,10 @@ module SignatureVerification raise SignatureVerificationError, "Invalid Digest value. Computed SHA-256 digest: #{body_digest}; given: #{sha256[1]}" end - def verify_signature(account, signature, compare_signed_string) - if account.keypair.public_key.verify(OpenSSL::Digest.new('SHA256'), signature, compare_signed_string) - @signed_request_account = account - @signed_request_account + def verify_signature(actor, signature, compare_signed_string) + if actor.keypair.public_key.verify(OpenSSL::Digest.new('SHA256'), signature, compare_signed_string) + @signed_request_actor = actor + @signed_request_actor end rescue OpenSSL::PKey::RSAError nil @@ -226,7 +234,7 @@ module SignatureVerification signature_params['keyId'].blank? || signature_params['signature'].blank? end - def account_from_key_id(key_id) + def actor_from_key_id(key_id) domain = key_id.start_with?('acct:') ? key_id.split('@').last : key_id if domain_not_allowed?(domain) @@ -237,13 +245,13 @@ module SignatureVerification if key_id.start_with?('acct:') stoplight_wrap_request { ResolveAccountService.new.call(key_id.gsub(/\Aacct:/, ''), suppress_errors: false) } elsif !ActivityPub::TagManager.instance.local_uri?(key_id) - account = ActivityPub::TagManager.instance.uri_to_resource(key_id, Account) + 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 end rescue Mastodon::PrivateNetworkAddressError => e raise SignatureVerificationError, "Requests to private network addresses are disallowed (tried to query #{e.host})" - rescue Mastodon::HostValidationError, ActivityPub::FetchRemoteAccountService::Error, ActivityPub::FetchRemoteKeyService::Error, Webfinger::Error => e + rescue Mastodon::HostValidationError, ActivityPub::FetchRemoteActorService::Error, ActivityPub::FetchRemoteKeyService::Error, Webfinger::Error => e raise SignatureVerificationError, e.message end @@ -255,12 +263,14 @@ module SignatureVerification .run end - def account_refresh_key(account) - return if account.local? || !account.activitypub? - ActivityPub::FetchRemoteAccountService.new.call(account.uri, only_key: true, suppress_errors: false) + def actor_refresh_key!(actor) + return if actor.local? || !actor.activitypub? + return actor.refresh! if actor.respond_to?(:refresh!) && actor.possibly_stale? + + ActivityPub::FetchRemoteActorService.new.call(actor.uri, only_key: true, suppress_errors: false) rescue Mastodon::PrivateNetworkAddressError => e raise SignatureVerificationError, "Requests to private network addresses are disallowed (tried to query #{e.host})" - rescue Mastodon::HostValidationError, ActivityPub::FetchRemoteAccountService::Error, Webfinger::Error => e + rescue Mastodon::HostValidationError, ActivityPub::FetchRemoteActorService::Error, Webfinger::Error => e raise SignatureVerificationError, e.message end end diff --git a/app/controllers/follower_accounts_controller.rb b/app/controllers/follower_accounts_controller.rb index f3f8336c9..da7bb4ed2 100644 --- a/app/controllers/follower_accounts_controller.rb +++ b/app/controllers/follower_accounts_controller.rb @@ -4,7 +4,7 @@ class FollowerAccountsController < ApplicationController include AccountControllerConcern include SignatureVerification - before_action :require_signature!, if: -> { request.format == :json && authorized_fetch_mode? } + before_action :require_account_signature!, if: -> { request.format == :json && authorized_fetch_mode? } before_action :set_cache_headers skip_around_action :set_locale, if: -> { request.format == :json } diff --git a/app/controllers/following_accounts_controller.rb b/app/controllers/following_accounts_controller.rb index 69f0321f8..c37e3b68c 100644 --- a/app/controllers/following_accounts_controller.rb +++ b/app/controllers/following_accounts_controller.rb @@ -4,7 +4,7 @@ class FollowingAccountsController < ApplicationController include AccountControllerConcern include SignatureVerification - before_action :require_signature!, if: -> { request.format == :json && authorized_fetch_mode? } + before_action :require_account_signature!, if: -> { request.format == :json && authorized_fetch_mode? } before_action :set_cache_headers skip_around_action :set_locale, if: -> { request.format == :json } diff --git a/app/controllers/statuses_controller.rb b/app/controllers/statuses_controller.rb index c52170d08..7d9db4d5b 100644 --- a/app/controllers/statuses_controller.rb +++ b/app/controllers/statuses_controller.rb @@ -8,7 +8,7 @@ class StatusesController < ApplicationController layout 'public' - before_action :require_signature!, only: [:show, :activity], if: -> { request.format == :json && authorized_fetch_mode? } + before_action :require_account_signature!, only: [:show, :activity], if: -> { request.format == :json && authorized_fetch_mode? } before_action :set_status before_action :set_instance_presenter before_action :set_link_headers diff --git a/app/controllers/tags_controller.rb b/app/controllers/tags_controller.rb index b82da8f0c..6dbc2667a 100644 --- a/app/controllers/tags_controller.rb +++ b/app/controllers/tags_controller.rb @@ -8,7 +8,7 @@ class TagsController < ApplicationController layout 'public' - before_action :require_signature!, if: -> { request.format == :json && authorized_fetch_mode? } + before_action :require_account_signature!, if: -> { request.format == :json && authorized_fetch_mode? } before_action :authenticate_user!, if: :whitelist_mode? before_action :set_local before_action :set_tag diff --git a/app/lib/activitypub/activity.rb b/app/lib/activitypub/activity.rb index 7ff06ea39..f4c67cccd 100644 --- a/app/lib/activitypub/activity.rb +++ b/app/lib/activitypub/activity.rb @@ -116,12 +116,12 @@ class ActivityPub::Activity def dereference_object! return unless @object.is_a?(String) - dereferencer = ActivityPub::Dereferencer.new(@object, permitted_origin: @account.uri, signature_account: signed_fetch_account) + dereferencer = ActivityPub::Dereferencer.new(@object, permitted_origin: @account.uri, signature_actor: signed_fetch_actor) @object = dereferencer.object unless dereferencer.object.nil? end - def signed_fetch_account + def signed_fetch_actor return Account.find(@options[:delivered_to_account_id]) if @options[:delivered_to_account_id].present? first_mentioned_local_account || first_local_follower @@ -163,15 +163,15 @@ class ActivityPub::Activity end def followed_by_local_accounts? - @account.passive_relationships.exists? || @options[:relayed_through_account]&.passive_relationships&.exists? + @account.passive_relationships.exists? || (@options[:relayed_through_actor].is_a?(Account) && @options[:relayed_through_actor].passive_relationships&.exists?) end def requested_through_relay? - @options[:relayed_through_account] && Relay.find_by(inbox_url: @options[:relayed_through_account].inbox_url)&.enabled? + @options[:relayed_through_actor] && Relay.find_by(inbox_url: @options[:relayed_through_actor].inbox_url)&.enabled? end def reject_payload! - Rails.logger.info("Rejected #{@json['type']} activity #{@json['id']} from #{@account.uri}#{@options[:relayed_through_account] && "via #{@options[:relayed_through_account].uri}"}") + Rails.logger.info("Rejected #{@json['type']} activity #{@json['id']} from #{@account.uri}#{@options[:relayed_through_actor] && "via #{@options[:relayed_through_actor].uri}"}") nil end end diff --git a/app/lib/activitypub/dereferencer.rb b/app/lib/activitypub/dereferencer.rb index bea69608f..4d7756d71 100644 --- a/app/lib/activitypub/dereferencer.rb +++ b/app/lib/activitypub/dereferencer.rb @@ -3,10 +3,10 @@ class ActivityPub::Dereferencer include JsonLdHelper - def initialize(uri, permitted_origin: nil, signature_account: nil) + def initialize(uri, permitted_origin: nil, signature_actor: nil) @uri = uri @permitted_origin = permitted_origin - @signature_account = signature_account + @signature_actor = signature_actor end def object @@ -46,7 +46,7 @@ class ActivityPub::Dereferencer req.add_headers('Accept' => 'application/activity+json, application/ld+json') req.add_headers(headers) if headers - req.on_behalf_of(@signature_account) if @signature_account + req.on_behalf_of(@signature_actor) if @signature_actor req.perform do |res| if res.code == 200 diff --git a/app/lib/activitypub/linked_data_signature.rb b/app/lib/activitypub/linked_data_signature.rb index e853a970e..f90adaf6c 100644 --- a/app/lib/activitypub/linked_data_signature.rb +++ b/app/lib/activitypub/linked_data_signature.rb @@ -9,7 +9,7 @@ class ActivityPub::LinkedDataSignature @json = json.with_indifferent_access end - def verify_account! + def verify_actor! return unless @json['signature'].is_a?(Hash) type = @json['signature']['type'] @@ -18,7 +18,7 @@ class ActivityPub::LinkedDataSignature return unless type == 'RsaSignature2017' - creator = ActivityPub::TagManager.instance.uri_to_resource(creator_uri, Account) + creator = ActivityPub::TagManager.instance.uri_to_actor(creator_uri) creator ||= ActivityPub::FetchRemoteKeyService.new.call(creator_uri, id: false) return if creator.nil? @@ -35,7 +35,7 @@ class ActivityPub::LinkedDataSignature def sign!(creator, sign_with: nil) options = { 'type' => 'RsaSignature2017', - 'creator' => [ActivityPub::TagManager.instance.uri_for(creator), '#main-key'].join, + 'creator' => ActivityPub::TagManager.instance.key_uri_for(creator), 'created' => Time.now.utc.iso8601, } diff --git a/app/lib/activitypub/tag_manager.rb b/app/lib/activitypub/tag_manager.rb index f6b9741fa..3d6b28ef5 100644 --- a/app/lib/activitypub/tag_manager.rb +++ b/app/lib/activitypub/tag_manager.rb @@ -44,6 +44,10 @@ class ActivityPub::TagManager end end + def key_uri_for(target) + [uri_for(target), '#main-key'].join + end + def uri_for_username(username) account_url(username: username) end @@ -155,6 +159,10 @@ class ActivityPub::TagManager path_params[param] end + def uri_to_actor(uri) + uri_to_resource(uri, Account) + end + def uri_to_resource(uri, klass) return if uri.nil? diff --git a/app/lib/request.rb b/app/lib/request.rb index eac04c798..648aa3085 100644 --- a/app/lib/request.rb +++ b/app/lib/request.rb @@ -40,12 +40,11 @@ class Request set_digest! if options.key?(:body) end - def on_behalf_of(account, key_id_format = :uri, sign_with: nil) - raise ArgumentError, 'account must not be nil' if account.nil? + def on_behalf_of(actor, sign_with: nil) + raise ArgumentError, 'actor must not be nil' if actor.nil? - @account = account - @keypair = sign_with.present? ? OpenSSL::PKey::RSA.new(sign_with) : @account.keypair - @key_id_format = key_id_format + @actor = actor + @keypair = sign_with.present? ? OpenSSL::PKey::RSA.new(sign_with) : @actor.keypair self end @@ -79,7 +78,7 @@ class Request end def headers - (@account ? @headers.merge('Signature' => signature) : @headers).without(REQUEST_TARGET) + (@actor ? @headers.merge('Signature' => signature) : @headers).without(REQUEST_TARGET) end class << self @@ -128,12 +127,7 @@ class Request end def key_id - case @key_id_format - when :acct - @account.to_webfinger_s - when :uri - [ActivityPub::TagManager.instance.uri_for(@account), '#main-key'].join - end + ActivityPub::TagManager.instance.key_uri_for(@actor) end def http_client diff --git a/app/serializers/activitypub/public_key_serializer.rb b/app/serializers/activitypub/public_key_serializer.rb index 62ed49e81..8621517e7 100644 --- a/app/serializers/activitypub/public_key_serializer.rb +++ b/app/serializers/activitypub/public_key_serializer.rb @@ -6,7 +6,7 @@ class ActivityPub::PublicKeySerializer < ActivityPub::Serializer attributes :id, :owner, :public_key_pem def id - [ActivityPub::TagManager.instance.uri_for(object), '#main-key'].join + ActivityPub::TagManager.instance.key_uri_for(object) end def owner diff --git a/app/services/activitypub/fetch_remote_account_service.rb b/app/services/activitypub/fetch_remote_account_service.rb index d7d739c59..ca7a8c6ca 100644 --- a/app/services/activitypub/fetch_remote_account_service.rb +++ b/app/services/activitypub/fetch_remote_account_service.rb @@ -1,80 +1,12 @@ # frozen_string_literal: true -class ActivityPub::FetchRemoteAccountService < BaseService - include JsonLdHelper - include DomainControlHelper - include WebfingerHelper - - class Error < StandardError; end - - SUPPORTED_TYPES = %w(Application Group Organization Person Service).freeze - +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) - return if domain_not_allowed?(uri) - return ActivityPub::TagManager.instance.uri_to_resource(uri, Account) if ActivityPub::TagManager.instance.local_uri?(uri) - - @json = begin - if prefetched_body.nil? - fetch_resource(uri, id) - else - body_to_json(prefetched_body, compare_id: id ? uri : nil) - end - rescue Oj::ParseError - raise Error, "Error parsing JSON-LD document #{uri}" - end - - raise Error, "Error fetching actor JSON at #{uri}" if @json.nil? - raise Error, "Unsupported JSON-LD context for document #{uri}" unless supported_context? - raise Error, "Unexpected object type for actor #{uri} (expected any of: #{SUPPORTED_TYPES})" unless expected_type? - raise Error, "Actor #{uri} has moved to #{@json['movedTo']}" if break_on_redirect && @json['movedTo'].present? - - @uri = @json['id'] - @username = @json['preferredUsername'] - @domain = Addressable::URI.parse(@uri).normalized_host - - check_webfinger! unless only_key - - ActivityPub::ProcessAccountService.new.call(@username, @domain, @json, only_key: only_key, verified_webfinger: !only_key) - rescue Error => e - Rails.logger.debug "Fetching account #{uri} failed: #{e.message}" - raise unless suppress_errors - end - - private - - def check_webfinger! - webfinger = webfinger!("acct:#{@username}@#{@domain}") - confirmed_username, confirmed_domain = split_acct(webfinger.subject) - - if @username.casecmp(confirmed_username).zero? && @domain.casecmp(confirmed_domain).zero? - raise Error, "Webfinger response for #{@username}@#{@domain} does not loop back to #{@uri}" if webfinger.link('self', 'href') != @uri - return - end - - webfinger = webfinger!("acct:#{confirmed_username}@#{confirmed_domain}") - @username, @domain = split_acct(webfinger.subject) - - unless confirmed_username.casecmp(@username).zero? && confirmed_domain.casecmp(@domain).zero? - raise Webfinger::RedirectError, "Too many webfinger redirects for URI #{uri} (stopped at #{@username}@#{@domain})" - end - - raise Error, "Webfinger response for #{@username}@#{@domain} does not loop back to #{@uri}" if webfinger.link('self', 'href') != @uri - rescue Webfinger::RedirectError => e - raise Error, e.message - rescue Webfinger::Error => e - raise Error, "Webfinger error when resolving #{@username}@#{@domain}: #{e.message}" - end - - def split_acct(acct) - acct.gsub(/\Aacct:/, '').split('@') - end - - def supported_context? - super(@json) - end + actor = super + return actor if actor.nil? || actor.is_a?(Account) - def expected_type? - equals_or_includes_any?(@json['type'], SUPPORTED_TYPES) + Rails.logger.debug "Fetching account #{uri} failed: Expected Account, got #{actor.class.name}" + raise Error, "Expected Account, got #{actor.class.name}" unless suppress_errors end end diff --git a/app/services/activitypub/fetch_remote_actor_service.rb b/app/services/activitypub/fetch_remote_actor_service.rb new file mode 100644 index 000000000..17bf2f287 --- /dev/null +++ b/app/services/activitypub/fetch_remote_actor_service.rb @@ -0,0 +1,80 @@ +# frozen_string_literal: true + +class ActivityPub::FetchRemoteActorService < BaseService + include JsonLdHelper + include DomainControlHelper + include WebfingerHelper + + class Error < StandardError; end + + 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) + 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) + else + body_to_json(prefetched_body, compare_id: id ? uri : nil) + end + rescue Oj::ParseError + raise Error, "Error parsing JSON-LD document #{uri}" + end + + raise Error, "Error fetching actor JSON at #{uri}" if @json.nil? + raise Error, "Unsupported JSON-LD context for document #{uri}" unless supported_context? + raise Error, "Unexpected object type for actor #{uri} (expected any of: #{SUPPORTED_TYPES})" unless expected_type? + raise Error, "Actor #{uri} has moved to #{@json['movedTo']}" if break_on_redirect && @json['movedTo'].present? + + @uri = @json['id'] + @username = @json['preferredUsername'] + @domain = Addressable::URI.parse(@uri).normalized_host + + check_webfinger! unless only_key + + ActivityPub::ProcessAccountService.new.call(@username, @domain, @json, only_key: only_key, verified_webfinger: !only_key) + rescue Error => e + Rails.logger.debug "Fetching actor #{uri} failed: #{e.message}" + raise unless suppress_errors + end + + private + + def check_webfinger! + webfinger = webfinger!("acct:#{@username}@#{@domain}") + confirmed_username, confirmed_domain = split_acct(webfinger.subject) + + if @username.casecmp(confirmed_username).zero? && @domain.casecmp(confirmed_domain).zero? + raise Error, "Webfinger response for #{@username}@#{@domain} does not loop back to #{@uri}" if webfinger.link('self', 'href') != @uri + return + end + + webfinger = webfinger!("acct:#{confirmed_username}@#{confirmed_domain}") + @username, @domain = split_acct(webfinger.subject) + + unless confirmed_username.casecmp(@username).zero? && confirmed_domain.casecmp(@domain).zero? + raise Webfinger::RedirectError, "Too many webfinger redirects for URI #{uri} (stopped at #{@username}@#{@domain})" + end + + raise Error, "Webfinger response for #{@username}@#{@domain} does not loop back to #{@uri}" if webfinger.link('self', 'href') != @uri + rescue Webfinger::RedirectError => e + raise Error, e.message + rescue Webfinger::Error => e + raise Error, "Webfinger error when resolving #{@username}@#{@domain}: #{e.message}" + end + + def split_acct(acct) + acct.gsub(/\Aacct:/, '').split('@') + end + + def supported_context? + super(@json) + end + + def expected_type? + equals_or_includes_any?(@json['type'], SUPPORTED_TYPES) + end +end diff --git a/app/services/activitypub/fetch_remote_key_service.rb b/app/services/activitypub/fetch_remote_key_service.rb index 01008d883..fe8f60b55 100644 --- a/app/services/activitypub/fetch_remote_key_service.rb +++ b/app/services/activitypub/fetch_remote_key_service.rb @@ -5,7 +5,7 @@ class ActivityPub::FetchRemoteKeyService < BaseService class Error < StandardError; end - # Returns account that owns the key + # Returns actor that owns the key def call(uri, id: true, prefetched_body: nil, suppress_errors: true) raise Error, 'No key URI given' if uri.blank? @@ -27,7 +27,7 @@ class ActivityPub::FetchRemoteKeyService < BaseService 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) raise Error, "Unexpected object type for key #{uri}" unless expected_type? - return find_account(@json['id'], @json, suppress_errors) if person? + return find_actor(@json['id'], @json, suppress_errors) if person? @owner = fetch_resource(owner_uri, true) @@ -36,7 +36,7 @@ class ActivityPub::FetchRemoteKeyService < BaseService raise Error, "Unexpected object type for actor #{owner_uri} (expected any of: #{SUPPORTED_TYPES})" unless expected_owner_type? raise Error, "publicKey id for #{owner_uri} does not correspond to #{@json['id']}" unless confirmed_owner? - find_account(owner_uri, @owner, suppress_errors) + find_actor(owner_uri, @owner, suppress_errors) rescue Error => e Rails.logger.debug "Fetching key #{uri} failed: #{e.message}" raise unless suppress_errors @@ -44,18 +44,18 @@ class ActivityPub::FetchRemoteKeyService < BaseService private - def find_account(uri, prefetched_body, suppress_errors) - account = ActivityPub::TagManager.instance.uri_to_resource(uri, Account) - account ||= ActivityPub::FetchRemoteAccountService.new.call(uri, prefetched_body: prefetched_body, suppress_errors: suppress_errors) - account + def find_actor(uri, prefetched_body, suppress_errors) + actor = ActivityPub::TagManager.instance.uri_to_actor(uri) + actor ||= ActivityPub::FetchRemoteActorService.new.call(uri, prefetched_body: prefetched_body, suppress_errors: suppress_errors) + actor end def expected_type? - person? || public_key? + actor? || public_key? end - def person? - equals_or_includes_any?(@json['type'], ActivityPub::FetchRemoteAccountService::SUPPORTED_TYPES) + def actor? + equals_or_includes_any?(@json['type'], ActivityPub::FetchRemoteActorService::SUPPORTED_TYPES) end def public_key? @@ -67,7 +67,7 @@ class ActivityPub::FetchRemoteKeyService < BaseService end def expected_owner_type? - equals_or_includes_any?(@owner['type'], ActivityPub::FetchRemoteAccountService::SUPPORTED_TYPES) + equals_or_includes_any?(@owner['type'], ActivityPub::FetchRemoteActorService::SUPPORTED_TYPES) end def confirmed_owner? diff --git a/app/services/activitypub/process_collection_service.rb b/app/services/activitypub/process_collection_service.rb index eb008c40a..fffe30195 100644 --- a/app/services/activitypub/process_collection_service.rb +++ b/app/services/activitypub/process_collection_service.rb @@ -3,8 +3,8 @@ class ActivityPub::ProcessCollectionService < BaseService include JsonLdHelper - def call(body, account, **options) - @account = account + def call(body, actor, **options) + @account = actor @json = original_json = Oj.load(body, mode: :strict) @options = options @@ -16,6 +16,7 @@ class ActivityPub::ProcessCollectionService < BaseService end return if !supported_context? || (different_actor? && verify_account!.nil?) || suspended_actor? || @account.local? + return unless @account.is_a?(Account) if @json['signature'].present? # We have verified the signature, but in the compaction step above, might @@ -66,8 +67,10 @@ class ActivityPub::ProcessCollectionService < BaseService end def verify_account! - @options[:relayed_through_account] = @account - @account = ActivityPub::LinkedDataSignature.new(@json).verify_account! + @options[:relayed_through_actor] = @account + @account = ActivityPub::LinkedDataSignature.new(@json).verify_actor! + @account = nil unless @account.is_a?(Account) + @account rescue JSON::LD::JsonLdError => e Rails.logger.debug "Could not verify LD-Signature for #{value_or_id(@json['actor'])}: #{e.message}" nil diff --git a/app/services/fetch_resource_service.rb b/app/services/fetch_resource_service.rb index 6c0093cd4..73204e55d 100644 --- a/app/services/fetch_resource_service.rb +++ b/app/services/fetch_resource_service.rb @@ -47,7 +47,7 @@ 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::FetchRemoteAccountService::SUPPORTED_TYPES) || expected_type?(json)) + [json['id'], { prefetched_body: body, id: true }] if supported_context?(json) && (equals_or_includes_any?(json['type'], ActivityPub::FetchRemoteActorService::SUPPORTED_TYPES) || expected_type?(json)) elsif !terminal link_header = response['Link'] && parse_link_header(response) diff --git a/app/services/keys/claim_service.rb b/app/services/keys/claim_service.rb index 69568a0d1..ae9e24a24 100644 --- a/app/services/keys/claim_service.rb +++ b/app/services/keys/claim_service.rb @@ -72,7 +72,7 @@ class Keys::ClaimService < BaseService def build_post_request(uri) Request.new(:post, uri).tap do |request| - request.on_behalf_of(@source_account, :uri) + request.on_behalf_of(@source_account) request.add_headers(HEADERS) end end diff --git a/app/services/resolve_url_service.rb b/app/services/resolve_url_service.rb index e2c745673..37c856cf8 100644 --- a/app/services/resolve_url_service.rb +++ b/app/services/resolve_url_service.rb @@ -20,8 +20,8 @@ class ResolveURLService < BaseService private def process_url - if equals_or_includes_any?(type, ActivityPub::FetchRemoteAccountService::SUPPORTED_TYPES) - ActivityPub::FetchRemoteAccountService.new.call(resource_url, prefetched_body: body) + if equals_or_includes_any?(type, ActivityPub::FetchRemoteActorService::SUPPORTED_TYPES) + ActivityPub::FetchRemoteActorService.new.call(resource_url, prefetched_body: body) elsif equals_or_includes_any?(type, ActivityPub::Activity::Create::SUPPORTED_TYPES + ActivityPub::Activity::Create::CONVERTED_TYPES) status = FetchRemoteStatusService.new.call(resource_url, body) authorize_with @on_behalf_of, status, :show? unless status.nil? diff --git a/app/workers/activitypub/delivery_worker.rb b/app/workers/activitypub/delivery_worker.rb index 788f2cf80..d9153132b 100644 --- a/app/workers/activitypub/delivery_worker.rb +++ b/app/workers/activitypub/delivery_worker.rb @@ -37,7 +37,7 @@ class ActivityPub::DeliveryWorker def build_request(http_client) Request.new(:post, @inbox_url, body: @json, http_client: http_client).tap do |request| - request.on_behalf_of(@source_account, :uri, sign_with: @options[:sign_with]) + request.on_behalf_of(@source_account, sign_with: @options[:sign_with]) request.add_headers(HEADERS) request.add_headers({ 'Collection-Synchronization' => synchronization_header }) if ENV['DISABLE_FOLLOWERS_SYNCHRONIZATION'] != 'true' && @options[:synchronize_followers] end diff --git a/app/workers/activitypub/processing_worker.rb b/app/workers/activitypub/processing_worker.rb index 37e316354..4d06ad079 100644 --- a/app/workers/activitypub/processing_worker.rb +++ b/app/workers/activitypub/processing_worker.rb @@ -5,11 +5,15 @@ class ActivityPub::ProcessingWorker sidekiq_options backtrace: true, retry: 8 - def perform(account_id, body, delivered_to_account_id = nil) - account = Account.find_by(id: account_id) - return if account.nil? + def perform(actor_id, body, delivered_to_account_id = nil, actor_type = 'Account') + case actor_type + when 'Account' + actor = Account.find_by(id: actor_id) + end - ActivityPub::ProcessCollectionService.new.call(body, account, override_timestamps: true, delivered_to_account_id: delivered_to_account_id, delivery: true) + return if actor.nil? + + ActivityPub::ProcessCollectionService.new.call(body, actor, override_timestamps: true, delivered_to_account_id: delivered_to_account_id, delivery: true) rescue ActiveRecord::RecordInvalid => e Rails.logger.debug "Error processing incoming ActivityPub object: #{e}" end diff --git a/spec/controllers/accounts_controller_spec.rb b/spec/controllers/accounts_controller_spec.rb index 662a89927..12266c800 100644 --- a/spec/controllers/accounts_controller_spec.rb +++ b/spec/controllers/accounts_controller_spec.rb @@ -420,7 +420,7 @@ RSpec.describe AccountsController, type: :controller do let(:remote_account) { Fabricate(:account, domain: 'example.com') } before do - allow(controller).to receive(:signed_request_account).and_return(remote_account) + allow(controller).to receive(:signed_request_actor).and_return(remote_account) get :show, params: { username: account.username, format: format } end diff --git a/spec/controllers/activitypub/collections_controller_spec.rb b/spec/controllers/activitypub/collections_controller_spec.rb index 4d87f80ce..f78d9abbf 100644 --- a/spec/controllers/activitypub/collections_controller_spec.rb +++ b/spec/controllers/activitypub/collections_controller_spec.rb @@ -24,7 +24,7 @@ RSpec.describe ActivityPub::CollectionsController, type: :controller do end before do - allow(controller).to receive(:signed_request_account).and_return(remote_account) + allow(controller).to receive(:signed_request_actor).and_return(remote_account) Fabricate(:status_pin, account: account) Fabricate(:status_pin, account: account) diff --git a/spec/controllers/activitypub/followers_synchronizations_controller_spec.rb b/spec/controllers/activitypub/followers_synchronizations_controller_spec.rb index e233bd560..c19bb8cae 100644 --- a/spec/controllers/activitypub/followers_synchronizations_controller_spec.rb +++ b/spec/controllers/activitypub/followers_synchronizations_controller_spec.rb @@ -15,7 +15,7 @@ RSpec.describe ActivityPub::FollowersSynchronizationsController, type: :controll end before do - allow(controller).to receive(:signed_request_account).and_return(remote_account) + allow(controller).to receive(:signed_request_actor).and_return(remote_account) end describe 'GET #show' do diff --git a/spec/controllers/activitypub/inboxes_controller_spec.rb b/spec/controllers/activitypub/inboxes_controller_spec.rb index 973ad83bb..2f023197b 100644 --- a/spec/controllers/activitypub/inboxes_controller_spec.rb +++ b/spec/controllers/activitypub/inboxes_controller_spec.rb @@ -6,7 +6,7 @@ RSpec.describe ActivityPub::InboxesController, type: :controller do let(:remote_account) { nil } before do - allow(controller).to receive(:signed_request_account).and_return(remote_account) + allow(controller).to receive(:signed_request_actor).and_return(remote_account) end describe 'POST #create' do diff --git a/spec/controllers/activitypub/outboxes_controller_spec.rb b/spec/controllers/activitypub/outboxes_controller_spec.rb index 04f036447..74bf46a5e 100644 --- a/spec/controllers/activitypub/outboxes_controller_spec.rb +++ b/spec/controllers/activitypub/outboxes_controller_spec.rb @@ -28,7 +28,7 @@ RSpec.describe ActivityPub::OutboxesController, type: :controller do end before do - allow(controller).to receive(:signed_request_account).and_return(remote_account) + allow(controller).to receive(:signed_request_actor).and_return(remote_account) end describe 'GET #show' do diff --git a/spec/controllers/activitypub/replies_controller_spec.rb b/spec/controllers/activitypub/replies_controller_spec.rb index a35957f24..aee1a8b1a 100644 --- a/spec/controllers/activitypub/replies_controller_spec.rb +++ b/spec/controllers/activitypub/replies_controller_spec.rb @@ -168,7 +168,7 @@ RSpec.describe ActivityPub::RepliesController, type: :controller do before do stub_const 'ActivityPub::RepliesController::DESCENDANTS_LIMIT', 5 - allow(controller).to receive(:signed_request_account).and_return(remote_querier) + allow(controller).to receive(:signed_request_actor).and_return(remote_querier) Fabricate(:status, thread: status, visibility: :public) Fabricate(:status, thread: status, visibility: :public) diff --git a/spec/controllers/concerns/signature_verification_spec.rb b/spec/controllers/concerns/signature_verification_spec.rb index 05fb1445b..6e73643b4 100644 --- a/spec/controllers/concerns/signature_verification_spec.rb +++ b/spec/controllers/concerns/signature_verification_spec.rb @@ -3,6 +3,16 @@ require 'rails_helper' describe ApplicationController, type: :controller do + class WrappedActor + attr_reader :wrapped_account + + def initialize(wrapped_account) + @wrapped_account = wrapped_account + end + + delegate :uri, :keypair, to: :wrapped_account + end + controller do include SignatureVerification @@ -73,6 +83,41 @@ describe ApplicationController, type: :controller do end end + context 'with a valid actor that is not an Account' do + let(:actor) { WrappedActor.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 older than a day' do before do get :success diff --git a/spec/controllers/statuses_controller_spec.rb b/spec/controllers/statuses_controller_spec.rb index 05fae67fa..6ed5d4bbb 100644 --- a/spec/controllers/statuses_controller_spec.rb +++ b/spec/controllers/statuses_controller_spec.rb @@ -426,7 +426,7 @@ describe StatusesController do let(:remote_account) { Fabricate(:account, domain: 'example.com') } before do - allow(controller).to receive(:signed_request_account).and_return(remote_account) + allow(controller).to receive(:signed_request_actor).and_return(remote_account) end context 'when account blocks account' do diff --git a/spec/lib/activitypub/activity/announce_spec.rb b/spec/lib/activitypub/activity/announce_spec.rb index 41806b258..e9cd6c68c 100644 --- a/spec/lib/activitypub/activity/announce_spec.rb +++ b/spec/lib/activitypub/activity/announce_spec.rb @@ -115,7 +115,7 @@ RSpec.describe ActivityPub::Activity::Announce do let(:object_json) { 'https://example.com/actor/hello-world' } - subject { described_class.new(json, sender, relayed_through_account: relay_account) } + subject { described_class.new(json, sender, relayed_through_actor: relay_account) } before do stub_request(:get, 'https://example.com/actor/hello-world').to_return(body: Oj.dump(unknown_object_json)) diff --git a/spec/lib/activitypub/dereferencer_spec.rb b/spec/lib/activitypub/dereferencer_spec.rb index ce30513d7..e50b497c7 100644 --- a/spec/lib/activitypub/dereferencer_spec.rb +++ b/spec/lib/activitypub/dereferencer_spec.rb @@ -4,10 +4,10 @@ RSpec.describe ActivityPub::Dereferencer do describe '#object' do let(:object) { { '@context': 'https://www.w3.org/ns/activitystreams', id: 'https://example.com/foo', type: 'Note', content: 'Hoge' } } let(:permitted_origin) { 'https://example.com' } - let(:signature_account) { nil } + let(:signature_actor) { nil } let(:uri) { nil } - subject { described_class.new(uri, permitted_origin: permitted_origin, signature_account: signature_account).object } + subject { described_class.new(uri, permitted_origin: permitted_origin, signature_actor: signature_actor).object } before do stub_request(:get, 'https://example.com/foo').to_return(body: Oj.dump(object), headers: { 'Content-Type' => 'application/activity+json' }) @@ -21,7 +21,7 @@ RSpec.describe ActivityPub::Dereferencer do end context 'with signature account' do - let(:signature_account) { Fabricate(:account) } + let(:signature_actor) { Fabricate(:account) } it 'makes signed request' do subject @@ -52,7 +52,7 @@ RSpec.describe ActivityPub::Dereferencer do end context 'with signature account' do - let(:signature_account) { Fabricate(:account) } + let(:signature_actor) { Fabricate(:account) } it 'makes signed request' do subject diff --git a/spec/lib/activitypub/linked_data_signature_spec.rb b/spec/lib/activitypub/linked_data_signature_spec.rb index 2222c46fb..d55a7c7fa 100644 --- a/spec/lib/activitypub/linked_data_signature_spec.rb +++ b/spec/lib/activitypub/linked_data_signature_spec.rb @@ -20,7 +20,7 @@ RSpec.describe ActivityPub::LinkedDataSignature do stub_jsonld_contexts! end - describe '#verify_account!' do + describe '#verify_actor!' do context 'when signature matches' do let(:raw_signature) do { @@ -32,7 +32,7 @@ RSpec.describe ActivityPub::LinkedDataSignature do let(:signature) { raw_signature.merge('type' => 'RsaSignature2017', 'signatureValue' => sign(sender, raw_signature, raw_json)) } it 'returns creator' do - expect(subject.verify_account!).to eq sender + expect(subject.verify_actor!).to eq sender end end @@ -40,7 +40,7 @@ RSpec.describe ActivityPub::LinkedDataSignature do let(:signature) { nil } it 'returns nil' do - expect(subject.verify_account!).to be_nil + expect(subject.verify_actor!).to be_nil end end @@ -55,7 +55,7 @@ RSpec.describe ActivityPub::LinkedDataSignature do let(:signature) { raw_signature.merge('type' => 'RsaSignature2017', 'signatureValue' => 's69F3mfddd99dGjmvjdjjs81e12jn121Gkm1') } it 'returns nil' do - expect(subject.verify_account!).to be_nil + expect(subject.verify_actor!).to be_nil end end end @@ -73,14 +73,14 @@ RSpec.describe ActivityPub::LinkedDataSignature do end it 'can be verified again' do - expect(described_class.new(subject).verify_account!).to eq sender + expect(described_class.new(subject).verify_actor!).to eq sender end end - def sign(from_account, options, document) + def sign(from_actor, options, document) options_hash = Digest::SHA256.hexdigest(canonicalize(options.merge('@context' => ActivityPub::LinkedDataSignature::CONTEXT))) document_hash = Digest::SHA256.hexdigest(canonicalize(document)) to_be_verified = options_hash + document_hash - Base64.strict_encode64(from_account.keypair.sign(OpenSSL::Digest.new('SHA256'), to_be_verified)) + Base64.strict_encode64(from_actor.keypair.sign(OpenSSL::Digest.new('SHA256'), to_be_verified)) end end diff --git a/spec/services/activitypub/fetch_remote_actor_service_spec.rb b/spec/services/activitypub/fetch_remote_actor_service_spec.rb new file mode 100644 index 000000000..20117c66d --- /dev/null +++ b/spec/services/activitypub/fetch_remote_actor_service_spec.rb @@ -0,0 +1,180 @@ +require 'rails_helper' + +RSpec.describe ActivityPub::FetchRemoteActorService, type: :service do + subject { ActivityPub::FetchRemoteActorService.new } + + let!(:actor) do + { + '@context': 'https://www.w3.org/ns/activitystreams', + id: 'https://example.com/alice', + type: 'Person', + preferredUsername: 'alice', + name: 'Alice', + summary: 'Foo bar', + inbox: 'http://example.com/alice/inbox', + } + end + + describe '#call' do + let(:account) { subject.call('https://example.com/alice', id: true) } + + shared_examples 'sets profile data' do + it 'returns an account' do + expect(account).to be_an Account + end + + it 'sets display name' do + expect(account.display_name).to eq 'Alice' + end + + it 'sets note' do + expect(account.note).to eq 'Foo bar' + end + + it 'sets URL' do + expect(account.url).to eq 'https://example.com/alice' + end + end + + context 'when the account does not have a inbox' do + let!(:webfinger) { { subject: 'acct:alice@example.com', links: [{ rel: 'self', href: 'https://example.com/alice' }] } } + + before do + actor[:inbox] = nil + + stub_request(:get, 'https://example.com/alice').to_return(body: Oj.dump(actor)) + stub_request(:get, 'https://example.com/.well-known/webfinger?resource=acct:alice@example.com').to_return(body: Oj.dump(webfinger), headers: { 'Content-Type': 'application/jrd+json' }) + end + + it 'fetches resource' do + account + expect(a_request(:get, 'https://example.com/alice')).to have_been_made.once + end + + it 'looks up webfinger' do + account + expect(a_request(:get, 'https://example.com/.well-known/webfinger?resource=acct:alice@example.com')).to have_been_made.once + end + + it 'returns nil' do + expect(account).to be_nil + end + end + + context 'when URI and WebFinger share the same host' do + let!(:webfinger) { { subject: 'acct:alice@example.com', links: [{ rel: 'self', href: 'https://example.com/alice' }] } } + + before do + stub_request(:get, 'https://example.com/alice').to_return(body: Oj.dump(actor)) + stub_request(:get, 'https://example.com/.well-known/webfinger?resource=acct:alice@example.com').to_return(body: Oj.dump(webfinger), headers: { 'Content-Type': 'application/jrd+json' }) + end + + it 'fetches resource' do + account + expect(a_request(:get, 'https://example.com/alice')).to have_been_made.once + end + + it 'looks up webfinger' do + account + expect(a_request(:get, 'https://example.com/.well-known/webfinger?resource=acct:alice@example.com')).to have_been_made.once + end + + it 'sets username and domain from webfinger' do + expect(account.username).to eq 'alice' + expect(account.domain).to eq 'example.com' + end + + include_examples 'sets profile data' + end + + context 'when WebFinger presents different domain than URI' do + let!(:webfinger) { { subject: 'acct:alice@iscool.af', links: [{ rel: 'self', href: 'https://example.com/alice' }] } } + + before do + stub_request(:get, 'https://example.com/alice').to_return(body: Oj.dump(actor)) + stub_request(:get, 'https://example.com/.well-known/webfinger?resource=acct:alice@example.com').to_return(body: Oj.dump(webfinger), headers: { 'Content-Type': 'application/jrd+json' }) + stub_request(:get, 'https://iscool.af/.well-known/webfinger?resource=acct:alice@iscool.af').to_return(body: Oj.dump(webfinger), headers: { 'Content-Type': 'application/jrd+json' }) + end + + it 'fetches resource' do + account + expect(a_request(:get, 'https://example.com/alice')).to have_been_made.once + end + + it 'looks up webfinger' do + account + expect(a_request(:get, 'https://example.com/.well-known/webfinger?resource=acct:alice@example.com')).to have_been_made.once + end + + it 'looks up "redirected" webfinger' do + account + expect(a_request(:get, 'https://iscool.af/.well-known/webfinger?resource=acct:alice@iscool.af')).to have_been_made.once + end + + it 'sets username and domain from final webfinger' do + expect(account.username).to eq 'alice' + expect(account.domain).to eq 'iscool.af' + end + + include_examples 'sets profile data' + end + + context 'when WebFinger returns a different URI' do + let!(:webfinger) { { subject: 'acct:alice@example.com', links: [{ rel: 'self', href: 'https://example.com/bob' }] } } + + before do + stub_request(:get, 'https://example.com/alice').to_return(body: Oj.dump(actor)) + stub_request(:get, 'https://example.com/.well-known/webfinger?resource=acct:alice@example.com').to_return(body: Oj.dump(webfinger), headers: { 'Content-Type': 'application/jrd+json' }) + end + + it 'fetches resource' do + account + expect(a_request(:get, 'https://example.com/alice')).to have_been_made.once + end + + it 'looks up webfinger' do + account + expect(a_request(:get, 'https://example.com/.well-known/webfinger?resource=acct:alice@example.com')).to have_been_made.once + end + + it 'does not create account' do + expect(account).to be_nil + end + end + + context 'when WebFinger returns a different URI after a redirection' do + let!(:webfinger) { { subject: 'acct:alice@iscool.af', links: [{ rel: 'self', href: 'https://example.com/bob' }] } } + + before do + stub_request(:get, 'https://example.com/alice').to_return(body: Oj.dump(actor)) + stub_request(:get, 'https://example.com/.well-known/webfinger?resource=acct:alice@example.com').to_return(body: Oj.dump(webfinger), headers: { 'Content-Type': 'application/jrd+json' }) + stub_request(:get, 'https://iscool.af/.well-known/webfinger?resource=acct:alice@iscool.af').to_return(body: Oj.dump(webfinger), headers: { 'Content-Type': 'application/jrd+json' }) + end + + it 'fetches resource' do + account + expect(a_request(:get, 'https://example.com/alice')).to have_been_made.once + end + + it 'looks up webfinger' do + account + expect(a_request(:get, 'https://example.com/.well-known/webfinger?resource=acct:alice@example.com')).to have_been_made.once + end + + it 'looks up "redirected" webfinger' do + account + expect(a_request(:get, 'https://iscool.af/.well-known/webfinger?resource=acct:alice@iscool.af')).to have_been_made.once + end + + it 'does not create account' do + expect(account).to be_nil + end + end + + context 'with wrong id' do + it 'does not create account' do + expect(subject.call('https://fake.address/@foo', prefetched_body: Oj.dump(actor))).to be_nil + end + end + end +end diff --git a/spec/services/activitypub/process_collection_service_spec.rb b/spec/services/activitypub/process_collection_service_spec.rb index 3eccaab5b..093a188a2 100644 --- a/spec/services/activitypub/process_collection_service_spec.rb +++ b/spec/services/activitypub/process_collection_service_spec.rb @@ -68,7 +68,7 @@ RSpec.describe ActivityPub::ProcessCollectionService, type: :service do let(:forwarder) { Fabricate(:account, domain: 'example.com', uri: 'http://example.com/other_account') } it 'does not process payload if no signature exists' do - expect_any_instance_of(ActivityPub::LinkedDataSignature).to receive(:verify_account!).and_return(nil) + expect_any_instance_of(ActivityPub::LinkedDataSignature).to receive(:verify_actor!).and_return(nil) expect(ActivityPub::Activity).not_to receive(:factory) subject.call(json, forwarder) @@ -77,7 +77,7 @@ RSpec.describe ActivityPub::ProcessCollectionService, type: :service do it 'processes payload with actor if valid signature exists' do payload['signature'] = { 'type' => 'RsaSignature2017' } - expect_any_instance_of(ActivityPub::LinkedDataSignature).to receive(:verify_account!).and_return(actor) + expect_any_instance_of(ActivityPub::LinkedDataSignature).to receive(:verify_actor!).and_return(actor) expect(ActivityPub::Activity).to receive(:factory).with(instance_of(Hash), actor, instance_of(Hash)) subject.call(json, forwarder) @@ -86,7 +86,7 @@ RSpec.describe ActivityPub::ProcessCollectionService, type: :service do it 'does not process payload if invalid signature exists' do payload['signature'] = { 'type' => 'RsaSignature2017' } - expect_any_instance_of(ActivityPub::LinkedDataSignature).to receive(:verify_account!).and_return(nil) + expect_any_instance_of(ActivityPub::LinkedDataSignature).to receive(:verify_actor!).and_return(nil) expect(ActivityPub::Activity).not_to receive(:factory) subject.call(json, forwarder) diff --git a/spec/services/fetch_resource_service_spec.rb b/spec/services/fetch_resource_service_spec.rb index ded05ffbc..c0c96ab69 100644 --- a/spec/services/fetch_resource_service_spec.rb +++ b/spec/services/fetch_resource_service_spec.rb @@ -66,7 +66,7 @@ RSpec.describe FetchResourceService, type: :service do it 'signs request' do subject - expect(a_request(:get, url).with(headers: { 'Signature' => /keyId="#{Regexp.escape(ActivityPub::TagManager.instance.uri_for(Account.representative) + '#main-key')}"/ })).to have_been_made + expect(a_request(:get, url).with(headers: { 'Signature' => /keyId="#{Regexp.escape(ActivityPub::TagManager.instance.key_uri_for(Account.representative))}"/ })).to have_been_made end context 'when content type is application/atom+xml' do -- cgit From 5c9abdeff1d0cf3e14d84c5ae298e6a5beccaf18 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Tue, 27 Sep 2022 03:08:19 +0200 Subject: Add retention policy for cached content and media (#19232) --- app/lib/redis_configuration.rb | 6 +-- app/lib/vacuum.rb | 3 ++ app/lib/vacuum/access_tokens_vacuum.rb | 18 +++++++ app/lib/vacuum/backups_vacuum.rb | 25 ++++++++++ app/lib/vacuum/feeds_vacuum.rb | 34 +++++++++++++ app/lib/vacuum/media_attachments_vacuum.rb | 40 ++++++++++++++++ app/lib/vacuum/preview_cards_vacuum.rb | 39 +++++++++++++++ app/lib/vacuum/statuses_vacuum.rb | 54 +++++++++++++++++++++ app/lib/vacuum/system_keys_vacuum.rb | 13 +++++ app/models/content_retention_policy.rb | 25 ++++++++++ app/models/form/admin_settings.rb | 4 ++ app/views/admin/settings/edit.html.haml | 8 +++- app/workers/scheduler/backup_cleanup_scheduler.rb | 17 ------- .../scheduler/doorkeeper_cleanup_scheduler.rb | 13 ----- app/workers/scheduler/feed_cleanup_scheduler.rb | 35 -------------- app/workers/scheduler/media_cleanup_scheduler.rb | 17 ------- app/workers/scheduler/vacuum_scheduler.rb | 56 ++++++++++++++++++++++ config/locales/simple_form.en.yml | 8 ++++ config/settings.yml | 1 + config/sidekiq.yml | 16 +------ spec/fabricators/access_grant_fabricator.rb | 6 +++ spec/fabricators/preview_card_fabricator.rb | 1 + spec/lib/vacuum/access_tokens_vacuum_spec.rb | 33 +++++++++++++ spec/lib/vacuum/backups_vacuum_spec.rb | 24 ++++++++++ spec/lib/vacuum/feeds_vacuum_spec.rb | 30 ++++++++++++ spec/lib/vacuum/media_attachments_vacuum_spec.rb | 47 ++++++++++++++++++ spec/lib/vacuum/preview_cards_vacuum_spec.rb | 36 ++++++++++++++ spec/lib/vacuum/statuses_vacuum_spec.rb | 36 ++++++++++++++ spec/lib/vacuum/system_keys_vacuum_spec.rb | 22 +++++++++ .../scheduler/feed_cleanup_scheduler_spec.rb | 26 ---------- .../scheduler/media_cleanup_scheduler_spec.rb | 15 ------ 31 files changed, 566 insertions(+), 142 deletions(-) create mode 100644 app/lib/vacuum.rb create mode 100644 app/lib/vacuum/access_tokens_vacuum.rb create mode 100644 app/lib/vacuum/backups_vacuum.rb create mode 100644 app/lib/vacuum/feeds_vacuum.rb create mode 100644 app/lib/vacuum/media_attachments_vacuum.rb create mode 100644 app/lib/vacuum/preview_cards_vacuum.rb create mode 100644 app/lib/vacuum/statuses_vacuum.rb create mode 100644 app/lib/vacuum/system_keys_vacuum.rb create mode 100644 app/models/content_retention_policy.rb delete mode 100644 app/workers/scheduler/backup_cleanup_scheduler.rb delete mode 100644 app/workers/scheduler/doorkeeper_cleanup_scheduler.rb delete mode 100644 app/workers/scheduler/feed_cleanup_scheduler.rb delete mode 100644 app/workers/scheduler/media_cleanup_scheduler.rb create mode 100644 app/workers/scheduler/vacuum_scheduler.rb create mode 100644 spec/fabricators/access_grant_fabricator.rb create mode 100644 spec/lib/vacuum/access_tokens_vacuum_spec.rb create mode 100644 spec/lib/vacuum/backups_vacuum_spec.rb create mode 100644 spec/lib/vacuum/feeds_vacuum_spec.rb create mode 100644 spec/lib/vacuum/media_attachments_vacuum_spec.rb create mode 100644 spec/lib/vacuum/preview_cards_vacuum_spec.rb create mode 100644 spec/lib/vacuum/statuses_vacuum_spec.rb create mode 100644 spec/lib/vacuum/system_keys_vacuum_spec.rb delete mode 100644 spec/workers/scheduler/feed_cleanup_scheduler_spec.rb delete mode 100644 spec/workers/scheduler/media_cleanup_scheduler_spec.rb (limited to 'app/workers') diff --git a/app/lib/redis_configuration.rb b/app/lib/redis_configuration.rb index e14d6c8b6..f0e86d985 100644 --- a/app/lib/redis_configuration.rb +++ b/app/lib/redis_configuration.rb @@ -7,9 +7,7 @@ class RedisConfiguration @pool = ConnectionPool.new(size: new_pool_size) { new.connection } end - def with - pool.with { |redis| yield redis } - end + delegate :with, to: :pool def pool @pool ||= establish_pool(pool_size) @@ -17,7 +15,7 @@ class RedisConfiguration def pool_size if Sidekiq.server? - Sidekiq.options[:concurrency] + Sidekiq[:concurrency] else ENV['MAX_THREADS'] || 5 end diff --git a/app/lib/vacuum.rb b/app/lib/vacuum.rb new file mode 100644 index 000000000..9db1ec90b --- /dev/null +++ b/app/lib/vacuum.rb @@ -0,0 +1,3 @@ +# frozen_string_literal: true + +module Vacuum; end diff --git a/app/lib/vacuum/access_tokens_vacuum.rb b/app/lib/vacuum/access_tokens_vacuum.rb new file mode 100644 index 000000000..4f3878027 --- /dev/null +++ b/app/lib/vacuum/access_tokens_vacuum.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +class Vacuum::AccessTokensVacuum + def perform + vacuum_revoked_access_tokens! + vacuum_revoked_access_grants! + end + + private + + def vacuum_revoked_access_tokens! + Doorkeeper::AccessToken.where('revoked_at IS NOT NULL').where('revoked_at < NOW()').delete_all + end + + def vacuum_revoked_access_grants! + Doorkeeper::AccessGrant.where('revoked_at IS NOT NULL').where('revoked_at < NOW()').delete_all + end +end diff --git a/app/lib/vacuum/backups_vacuum.rb b/app/lib/vacuum/backups_vacuum.rb new file mode 100644 index 000000000..3b83072f3 --- /dev/null +++ b/app/lib/vacuum/backups_vacuum.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +class Vacuum::BackupsVacuum + def initialize(retention_period) + @retention_period = retention_period + end + + def perform + vacuum_expired_backups! if retention_period? + end + + private + + def vacuum_expired_backups! + backups_past_retention_period.in_batches.destroy_all + end + + def backups_past_retention_period + Backup.unscoped.where(Backup.arel_table[:created_at].lt(@retention_period.ago)) + end + + def retention_period? + @retention_period.present? + end +end diff --git a/app/lib/vacuum/feeds_vacuum.rb b/app/lib/vacuum/feeds_vacuum.rb new file mode 100644 index 000000000..f46bcf75f --- /dev/null +++ b/app/lib/vacuum/feeds_vacuum.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +class Vacuum::FeedsVacuum + def perform + vacuum_inactive_home_feeds! + vacuum_inactive_list_feeds! + end + + private + + def vacuum_inactive_home_feeds! + inactive_users.select(:id, :account_id).find_in_batches do |users| + feed_manager.clean_feeds!(:home, users.map(&:account_id)) + end + end + + def vacuum_inactive_list_feeds! + inactive_users_lists.select(:id).find_in_batches do |lists| + feed_manager.clean_feeds!(:list, lists.map(&:id)) + end + end + + def inactive_users + User.confirmed.inactive + end + + def inactive_users_lists + List.where(account_id: inactive_users.select(:account_id)) + end + + def feed_manager + FeedManager.instance + end +end diff --git a/app/lib/vacuum/media_attachments_vacuum.rb b/app/lib/vacuum/media_attachments_vacuum.rb new file mode 100644 index 000000000..7fb347ce4 --- /dev/null +++ b/app/lib/vacuum/media_attachments_vacuum.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +class Vacuum::MediaAttachmentsVacuum + TTL = 1.day.freeze + + def initialize(retention_period) + @retention_period = retention_period + end + + def perform + vacuum_cached_files! if retention_period? + vacuum_orphaned_records! + end + + private + + def vacuum_cached_files! + media_attachments_past_retention_period.find_each do |media_attachment| + media_attachment.file.destroy + media_attachment.thumbnail.destroy + media_attachment.save + end + end + + def vacuum_orphaned_records! + orphaned_media_attachments.in_batches.destroy_all + end + + def media_attachments_past_retention_period + MediaAttachment.unscoped.remote.cached.where(MediaAttachment.arel_table[:created_at].lt(@retention_period.ago)).where(MediaAttachment.arel_table[:updated_at].lt(@retention_period.ago)) + end + + def orphaned_media_attachments + MediaAttachment.unscoped.unattached.where(MediaAttachment.arel_table[:created_at].lt(TTL.ago)) + end + + def retention_period? + @retention_period.present? + end +end diff --git a/app/lib/vacuum/preview_cards_vacuum.rb b/app/lib/vacuum/preview_cards_vacuum.rb new file mode 100644 index 000000000..84ef100ed --- /dev/null +++ b/app/lib/vacuum/preview_cards_vacuum.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +class Vacuum::PreviewCardsVacuum + TTL = 1.day.freeze + + def initialize(retention_period) + @retention_period = retention_period + end + + def perform + vacuum_cached_images! if retention_period? + vacuum_orphaned_records! + end + + private + + def vacuum_cached_images! + preview_cards_past_retention_period.find_each do |preview_card| + preview_card.image.destroy + preview_card.save + end + end + + def vacuum_orphaned_records! + orphaned_preview_cards.in_batches.destroy_all + end + + def preview_cards_past_retention_period + PreviewCard.cached.where(PreviewCard.arel_table[:updated_at].lt(@retention_period.ago)) + end + + def orphaned_preview_cards + PreviewCard.where('NOT EXISTS (SELECT 1 FROM preview_cards_statuses WHERE preview_cards_statuses.preview_card_id = preview_cards.id)').where(PreviewCard.arel_table[:created_at].lt(TTL.ago)) + end + + def retention_period? + @retention_period.present? + end +end diff --git a/app/lib/vacuum/statuses_vacuum.rb b/app/lib/vacuum/statuses_vacuum.rb new file mode 100644 index 000000000..41d6ba270 --- /dev/null +++ b/app/lib/vacuum/statuses_vacuum.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +class Vacuum::StatusesVacuum + include Redisable + + def initialize(retention_period) + @retention_period = retention_period + end + + def perform + vacuum_statuses! if retention_period? + end + + private + + def vacuum_statuses! + statuses_scope.find_in_batches do |statuses| + # Side-effects not covered by foreign keys, such + # as the search index, must be handled first. + + remove_from_account_conversations(statuses) + remove_from_search_index(statuses) + + # Foreign keys take care of most associated records + # for us. Media attachments will be orphaned. + + Status.where(id: statuses.map(&:id)).delete_all + end + end + + def statuses_scope + Status.unscoped.kept.where(account: Account.remote).where(Status.arel_table[:id].lt(retention_period_as_id)).select(:id, :visibility) + end + + def retention_period_as_id + Mastodon::Snowflake.id_at(@retention_period.ago, with_random: false) + end + + def analyze_statuses! + ActiveRecord::Base.connection.execute('ANALYZE statuses') + end + + def remove_from_account_conversations(statuses) + Status.where(id: statuses.select(&:direct_visibility?).map(&:id)).includes(:account, mentions: :account).each(&:unlink_from_conversations) + end + + def remove_from_search_index(statuses) + with_redis { |redis| redis.sadd('chewy:queue:StatusesIndex', statuses.map(&:id)) } if Chewy.enabled? + end + + def retention_period? + @retention_period.present? + end +end diff --git a/app/lib/vacuum/system_keys_vacuum.rb b/app/lib/vacuum/system_keys_vacuum.rb new file mode 100644 index 000000000..ceee2fd16 --- /dev/null +++ b/app/lib/vacuum/system_keys_vacuum.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +class Vacuum::SystemKeysVacuum + def perform + vacuum_expired_system_keys! + end + + private + + def vacuum_expired_system_keys! + SystemKey.expired.delete_all + end +end diff --git a/app/models/content_retention_policy.rb b/app/models/content_retention_policy.rb new file mode 100644 index 000000000..b5e922c8c --- /dev/null +++ b/app/models/content_retention_policy.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +class ContentRetentionPolicy + def self.current + new + end + + def media_cache_retention_period + retention_period Setting.media_cache_retention_period + end + + def content_cache_retention_period + retention_period Setting.content_cache_retention_period + end + + def backups_retention_period + retention_period Setting.backups_retention_period + end + + private + + def retention_period(value) + value.days if value.is_a?(Integer) && value.positive? + end +end diff --git a/app/models/form/admin_settings.rb b/app/models/form/admin_settings.rb index 97fabc6ac..3a7150916 100644 --- a/app/models/form/admin_settings.rb +++ b/app/models/form/admin_settings.rb @@ -32,6 +32,9 @@ class Form::AdminSettings show_domain_blocks_rationale noindex require_invite_text + media_cache_retention_period + content_cache_retention_period + backups_retention_period ).freeze BOOLEAN_KEYS = %i( @@ -64,6 +67,7 @@ class Form::AdminSettings validates :bootstrap_timeline_accounts, existing_username: { multiple: true } validates :show_domain_blocks, inclusion: { in: %w(disabled users all) } validates :show_domain_blocks_rationale, inclusion: { in: %w(disabled users all) } + validates :media_cache_retention_period, :content_cache_retention_period, :backups_retention_period, numericality: { only_integer: true } def initialize(_attributes = {}) super diff --git a/app/views/admin/settings/edit.html.haml b/app/views/admin/settings/edit.html.haml index 64687b7a6..1dfd21643 100644 --- a/app/views/admin/settings/edit.html.haml +++ b/app/views/admin/settings/edit.html.haml @@ -45,7 +45,6 @@ .fields-group = f.input :require_invite_text, as: :boolean, wrapper: :with_label, label: t('admin.settings.registrations.require_invite_text.title'), hint: t('admin.settings.registrations.require_invite_text.desc_html'), disabled: !approved_registrations? - .fields-group %hr.spacer/ @@ -100,5 +99,12 @@ = f.input :site_terms, wrapper: :with_block_label, as: :text, label: t('admin.settings.site_terms.title'), hint: t('admin.settings.site_terms.desc_html'), input_html: { rows: 8 } = f.input :custom_css, wrapper: :with_block_label, as: :text, input_html: { rows: 8 }, label: t('admin.settings.custom_css.title'), hint: t('admin.settings.custom_css.desc_html') + %hr.spacer/ + + .fields-group + = f.input :media_cache_retention_period, wrapper: :with_block_label, input_html: { pattern: '[0-9]+' } + = f.input :content_cache_retention_period, wrapper: :with_block_label, input_html: { pattern: '[0-9]+' } + = f.input :backups_retention_period, wrapper: :with_block_label, input_html: { pattern: '[0-9]+' } + .actions = f.button :button, t('generic.save_changes'), type: :submit diff --git a/app/workers/scheduler/backup_cleanup_scheduler.rb b/app/workers/scheduler/backup_cleanup_scheduler.rb deleted file mode 100644 index 85d5312c0..000000000 --- a/app/workers/scheduler/backup_cleanup_scheduler.rb +++ /dev/null @@ -1,17 +0,0 @@ -# frozen_string_literal: true - -class Scheduler::BackupCleanupScheduler - include Sidekiq::Worker - - sidekiq_options retry: 0 - - def perform - old_backups.reorder(nil).find_each(&:destroy!) - end - - private - - def old_backups - Backup.where('created_at < ?', 7.days.ago) - end -end diff --git a/app/workers/scheduler/doorkeeper_cleanup_scheduler.rb b/app/workers/scheduler/doorkeeper_cleanup_scheduler.rb deleted file mode 100644 index 9303a352f..000000000 --- a/app/workers/scheduler/doorkeeper_cleanup_scheduler.rb +++ /dev/null @@ -1,13 +0,0 @@ -# frozen_string_literal: true - -class Scheduler::DoorkeeperCleanupScheduler - include Sidekiq::Worker - - sidekiq_options retry: 0 - - def perform - Doorkeeper::AccessToken.where('revoked_at IS NOT NULL').where('revoked_at < NOW()').delete_all - Doorkeeper::AccessGrant.where('revoked_at IS NOT NULL').where('revoked_at < NOW()').delete_all - SystemKey.expired.delete_all - end -end diff --git a/app/workers/scheduler/feed_cleanup_scheduler.rb b/app/workers/scheduler/feed_cleanup_scheduler.rb deleted file mode 100644 index aa0cc8b8d..000000000 --- a/app/workers/scheduler/feed_cleanup_scheduler.rb +++ /dev/null @@ -1,35 +0,0 @@ -# frozen_string_literal: true - -class Scheduler::FeedCleanupScheduler - include Sidekiq::Worker - include Redisable - - sidekiq_options retry: 0 - - def perform - clean_home_feeds! - clean_list_feeds! - end - - private - - def clean_home_feeds! - feed_manager.clean_feeds!(:home, inactive_account_ids) - end - - def clean_list_feeds! - feed_manager.clean_feeds!(:list, inactive_list_ids) - end - - def inactive_account_ids - @inactive_account_ids ||= User.confirmed.inactive.pluck(:account_id) - end - - def inactive_list_ids - List.where(account_id: inactive_account_ids).pluck(:id) - end - - def feed_manager - FeedManager.instance - end -end diff --git a/app/workers/scheduler/media_cleanup_scheduler.rb b/app/workers/scheduler/media_cleanup_scheduler.rb deleted file mode 100644 index 24d30a6be..000000000 --- a/app/workers/scheduler/media_cleanup_scheduler.rb +++ /dev/null @@ -1,17 +0,0 @@ -# frozen_string_literal: true - -class Scheduler::MediaCleanupScheduler - include Sidekiq::Worker - - sidekiq_options retry: 0 - - def perform - unattached_media.find_each(&:destroy) - end - - private - - def unattached_media - MediaAttachment.reorder(nil).unattached.where('created_at < ?', 1.day.ago) - end -end diff --git a/app/workers/scheduler/vacuum_scheduler.rb b/app/workers/scheduler/vacuum_scheduler.rb new file mode 100644 index 000000000..ce88ff204 --- /dev/null +++ b/app/workers/scheduler/vacuum_scheduler.rb @@ -0,0 +1,56 @@ +# frozen_string_literal: true + +class Scheduler::VacuumScheduler + include Sidekiq::Worker + + sidekiq_options retry: 0 + + def perform + vacuum_operations.each do |operation| + operation.perform + rescue => e + Rails.logger.error("Error while running #{operation.class.name}: #{e}") + end + end + + private + + def vacuum_operations + [ + statuses_vacuum, + media_attachments_vacuum, + preview_cards_vacuum, + backups_vacuum, + access_tokens_vacuum, + feeds_vacuum, + ] + end + + def statuses_vacuum + Vacuum::StatusesVacuum.new(content_retention_policy.content_cache_retention_period) + end + + def media_attachments_vacuum + Vacuum::MediaAttachmentsVacuum.new(content_retention_policy.media_cache_retention_period) + end + + def preview_cards_vacuum + Vacuum::PreviewCardsVacuum.new(content_retention_policy.media_cache_retention_period) + end + + def backups_vacuum + Vacuum::BackupsVacuum.new(content_retention_policy.backups_retention_period) + end + + def access_tokens_vacuum + Vacuum::AccessTokensVacuum.new + end + + def feeds_vacuum + Vacuum::FeedsVacuum.new + end + + def content_retention_policy + ContentRetentionPolicy.current + end +end diff --git a/config/locales/simple_form.en.yml b/config/locales/simple_form.en.yml index ddc83e896..db5b45e41 100644 --- a/config/locales/simple_form.en.yml +++ b/config/locales/simple_form.en.yml @@ -73,6 +73,10 @@ en: actions: hide: Completely hide the filtered content, behaving as if it did not exist warn: Hide the filtered content behind a warning mentioning the filter's title + form_admin_settings: + backups_retention_period: Keep generated user archives for the specified number of days. + content_cache_retention_period: Posts from other servers will be deleted after the specified number of days when set to a positive value. This may be irreversible. + media_cache_retention_period: Downloaded media files will be deleted after the specified number of days when set to a positive value, and re-downloaded on demand. form_challenge: current_password: You are entering a secure area imports: @@ -207,6 +211,10 @@ en: actions: hide: Hide completely warn: Hide with a warning + form_admin_settings: + backups_retention_period: User archive retention period + content_cache_retention_period: Content cache retention period + media_cache_retention_period: Media cache retention period interactions: must_be_follower: Block notifications from non-followers must_be_following: Block notifications from people you don't follow diff --git a/config/settings.yml b/config/settings.yml index eaa05071e..41742118b 100644 --- a/config/settings.yml +++ b/config/settings.yml @@ -70,6 +70,7 @@ defaults: &defaults show_domain_blocks: 'disabled' show_domain_blocks_rationale: 'disabled' require_invite_text: false + backups_retention_period: 7 development: <<: *defaults diff --git a/config/sidekiq.yml b/config/sidekiq.yml index 9ec6eb5ec..e3156aa34 100644 --- a/config/sidekiq.yml +++ b/config/sidekiq.yml @@ -25,22 +25,14 @@ every: '5m' class: Scheduler::IndexingScheduler queue: scheduler - media_cleanup_scheduler: + vacuum_scheduler: cron: '<%= Random.rand(0..59) %> <%= Random.rand(3..5) %> * * *' - class: Scheduler::MediaCleanupScheduler - queue: scheduler - feed_cleanup_scheduler: - cron: '<%= Random.rand(0..59) %> <%= Random.rand(0..2) %> * * *' - class: Scheduler::FeedCleanupScheduler + class: Scheduler::VacuumScheduler queue: scheduler follow_recommendations_scheduler: cron: '<%= Random.rand(0..59) %> <%= Random.rand(6..9) %> * * *' class: Scheduler::FollowRecommendationsScheduler queue: scheduler - doorkeeper_cleanup_scheduler: - cron: '<%= Random.rand(0..59) %> <%= Random.rand(0..2) %> * * 0' - class: Scheduler::DoorkeeperCleanupScheduler - queue: scheduler user_cleanup_scheduler: cron: '<%= Random.rand(0..59) %> <%= Random.rand(4..6) %> * * *' class: Scheduler::UserCleanupScheduler @@ -49,10 +41,6 @@ cron: '<%= Random.rand(0..59) %> <%= Random.rand(3..5) %> * * *' class: Scheduler::IpCleanupScheduler queue: scheduler - backup_cleanup_scheduler: - cron: '<%= Random.rand(0..59) %> <%= Random.rand(3..5) %> * * *' - class: Scheduler::BackupCleanupScheduler - queue: scheduler pghero_scheduler: cron: '0 0 * * *' class: Scheduler::PgheroScheduler diff --git a/spec/fabricators/access_grant_fabricator.rb b/spec/fabricators/access_grant_fabricator.rb new file mode 100644 index 000000000..ae1945f2b --- /dev/null +++ b/spec/fabricators/access_grant_fabricator.rb @@ -0,0 +1,6 @@ +Fabricator :access_grant, from: 'Doorkeeper::AccessGrant' do + application + resource_owner_id { Fabricate(:user).id } + expires_in 3_600 + redirect_uri { Doorkeeper.configuration.native_redirect_uri } +end diff --git a/spec/fabricators/preview_card_fabricator.rb b/spec/fabricators/preview_card_fabricator.rb index f119c117d..99b5edc43 100644 --- a/spec/fabricators/preview_card_fabricator.rb +++ b/spec/fabricators/preview_card_fabricator.rb @@ -3,4 +3,5 @@ Fabricator(:preview_card) do title { Faker::Lorem.sentence } description { Faker::Lorem.paragraph } type 'link' + image { attachment_fixture('attachment.jpg') } end diff --git a/spec/lib/vacuum/access_tokens_vacuum_spec.rb b/spec/lib/vacuum/access_tokens_vacuum_spec.rb new file mode 100644 index 000000000..0244c3449 --- /dev/null +++ b/spec/lib/vacuum/access_tokens_vacuum_spec.rb @@ -0,0 +1,33 @@ +require 'rails_helper' + +RSpec.describe Vacuum::AccessTokensVacuum do + subject { described_class.new } + + describe '#perform' do + let!(:revoked_access_token) { Fabricate(:access_token, revoked_at: 1.minute.ago) } + let!(:active_access_token) { Fabricate(:access_token) } + + let!(:revoked_access_grant) { Fabricate(:access_grant, revoked_at: 1.minute.ago) } + let!(:active_access_grant) { Fabricate(:access_grant) } + + before do + subject.perform + end + + it 'deletes revoked access tokens' do + expect { revoked_access_token.reload }.to raise_error ActiveRecord::RecordNotFound + end + + it 'deletes revoked access grants' do + expect { revoked_access_grant.reload }.to raise_error ActiveRecord::RecordNotFound + end + + it 'does not delete active access tokens' do + expect { active_access_token.reload }.to_not raise_error + end + + it 'does not delete active access grants' do + expect { active_access_grant.reload }.to_not raise_error + end + end +end diff --git a/spec/lib/vacuum/backups_vacuum_spec.rb b/spec/lib/vacuum/backups_vacuum_spec.rb new file mode 100644 index 000000000..4e2de083f --- /dev/null +++ b/spec/lib/vacuum/backups_vacuum_spec.rb @@ -0,0 +1,24 @@ +require 'rails_helper' + +RSpec.describe Vacuum::BackupsVacuum do + let(:retention_period) { 7.days } + + subject { described_class.new(retention_period) } + + describe '#perform' do + let!(:expired_backup) { Fabricate(:backup, created_at: (retention_period + 1.day).ago) } + let!(:current_backup) { Fabricate(:backup) } + + before do + subject.perform + end + + it 'deletes backups past the retention period' do + expect { expired_backup.reload }.to raise_error ActiveRecord::RecordNotFound + end + + it 'does not delete backups within the retention period' do + expect { current_backup.reload }.to_not raise_error + end + end +end diff --git a/spec/lib/vacuum/feeds_vacuum_spec.rb b/spec/lib/vacuum/feeds_vacuum_spec.rb new file mode 100644 index 000000000..0aec26740 --- /dev/null +++ b/spec/lib/vacuum/feeds_vacuum_spec.rb @@ -0,0 +1,30 @@ +require 'rails_helper' + +RSpec.describe Vacuum::FeedsVacuum do + subject { described_class.new } + + describe '#perform' do + let!(:active_user) { Fabricate(:user, current_sign_in_at: 2.days.ago) } + let!(:inactive_user) { Fabricate(:user, current_sign_in_at: 22.days.ago) } + + before do + redis.zadd(feed_key_for(inactive_user), 1, 1) + redis.zadd(feed_key_for(active_user), 1, 1) + redis.zadd(feed_key_for(inactive_user, 'reblogs'), 2, 2) + redis.sadd(feed_key_for(inactive_user, 'reblogs:2'), 3) + + subject.perform + end + + it 'clears feeds of inactive users and lists' do + expect(redis.zcard(feed_key_for(inactive_user))).to eq 0 + expect(redis.zcard(feed_key_for(active_user))).to eq 1 + expect(redis.exists?(feed_key_for(inactive_user, 'reblogs'))).to be false + expect(redis.exists?(feed_key_for(inactive_user, 'reblogs:2'))).to be false + end + end + + def feed_key_for(user, subtype = nil) + FeedManager.instance.key(:home, user.account_id, subtype) + end +end diff --git a/spec/lib/vacuum/media_attachments_vacuum_spec.rb b/spec/lib/vacuum/media_attachments_vacuum_spec.rb new file mode 100644 index 000000000..be8458d9b --- /dev/null +++ b/spec/lib/vacuum/media_attachments_vacuum_spec.rb @@ -0,0 +1,47 @@ +require 'rails_helper' + +RSpec.describe Vacuum::MediaAttachmentsVacuum do + let(:retention_period) { 7.days } + + subject { described_class.new(retention_period) } + + let(:remote_status) { Fabricate(:status, account: Fabricate(:account, domain: 'example.com')) } + let(:local_status) { Fabricate(:status) } + + describe '#perform' do + let!(:old_remote_media) { Fabricate(:media_attachment, remote_url: 'https://example.com/foo.png', status: remote_status, created_at: (retention_period + 1.day).ago, updated_at: (retention_period + 1.day).ago) } + let!(:old_local_media) { Fabricate(:media_attachment, status: local_status, created_at: (retention_period + 1.day).ago, updated_at: (retention_period + 1.day).ago) } + let!(:new_remote_media) { Fabricate(:media_attachment, remote_url: 'https://example.com/foo.png', status: remote_status) } + let!(:new_local_media) { Fabricate(:media_attachment, status: local_status) } + let!(:old_unattached_media) { Fabricate(:media_attachment, account_id: nil, created_at: 10.days.ago) } + let!(:new_unattached_media) { Fabricate(:media_attachment, account_id: nil, created_at: 1.hour.ago) } + + before do + subject.perform + end + + it 'deletes cache of remote media attachments past the retention period' do + expect(old_remote_media.reload.file).to be_blank + end + + it 'does not touch local media attachments past the retention period' do + expect(old_local_media.reload.file).to_not be_blank + end + + it 'does not delete cache of remote media attachments within the retention period' do + expect(new_remote_media.reload.file).to_not be_blank + end + + it 'does not touch local media attachments within the retention period' do + expect(new_local_media.reload.file).to_not be_blank + end + + it 'deletes unattached media attachments past TTL' do + expect { old_unattached_media.reload }.to raise_error(ActiveRecord::RecordNotFound) + end + + it 'does not delete unattached media attachments within TTL' do + expect(new_unattached_media.reload).to be_persisted + end + end +end diff --git a/spec/lib/vacuum/preview_cards_vacuum_spec.rb b/spec/lib/vacuum/preview_cards_vacuum_spec.rb new file mode 100644 index 000000000..4a4a599fa --- /dev/null +++ b/spec/lib/vacuum/preview_cards_vacuum_spec.rb @@ -0,0 +1,36 @@ +require 'rails_helper' + +RSpec.describe Vacuum::PreviewCardsVacuum do + let(:retention_period) { 7.days } + + subject { described_class.new(retention_period) } + + describe '#perform' do + let!(:orphaned_preview_card) { Fabricate(:preview_card, created_at: 2.days.ago) } + let!(:old_preview_card) { Fabricate(:preview_card, updated_at: (retention_period + 1.day).ago) } + let!(:new_preview_card) { Fabricate(:preview_card) } + + before do + old_preview_card.statuses << Fabricate(:status) + new_preview_card.statuses << Fabricate(:status) + + subject.perform + end + + it 'deletes cache of preview cards last updated before the retention period' do + expect(old_preview_card.reload.image).to be_blank + end + + it 'does not delete cache of preview cards last updated within the retention period' do + expect(new_preview_card.reload.image).to_not be_blank + end + + it 'does not delete attached preview cards' do + expect(new_preview_card.reload).to be_persisted + end + + it 'deletes preview cards not attached to any status' do + expect { orphaned_preview_card.reload }.to raise_error ActiveRecord::RecordNotFound + end + end +end diff --git a/spec/lib/vacuum/statuses_vacuum_spec.rb b/spec/lib/vacuum/statuses_vacuum_spec.rb new file mode 100644 index 000000000..83f3c5c9f --- /dev/null +++ b/spec/lib/vacuum/statuses_vacuum_spec.rb @@ -0,0 +1,36 @@ +require 'rails_helper' + +RSpec.describe Vacuum::StatusesVacuum do + let(:retention_period) { 7.days } + + let(:remote_account) { Fabricate(:account, domain: 'example.com') } + + subject { described_class.new(retention_period) } + + describe '#perform' do + let!(:remote_status_old) { Fabricate(:status, account: remote_account, created_at: (retention_period + 2.days).ago) } + let!(:remote_status_recent) { Fabricate(:status, account: remote_account, created_at: (retention_period - 2.days).ago) } + let!(:local_status_old) { Fabricate(:status, created_at: (retention_period + 2.days).ago) } + let!(:local_status_recent) { Fabricate(:status, created_at: (retention_period - 2.days).ago) } + + before do + subject.perform + end + + it 'deletes remote statuses past the retention period' do + expect { remote_status_old.reload }.to raise_error ActiveRecord::RecordNotFound + end + + it 'does not delete local statuses past the retention period' do + expect { local_status_old.reload }.to_not raise_error + end + + it 'does not delete remote statuses within the retention period' do + expect { remote_status_recent.reload }.to_not raise_error + end + + it 'does not delete local statuses within the retention period' do + expect { local_status_recent.reload }.to_not raise_error + end + end +end diff --git a/spec/lib/vacuum/system_keys_vacuum_spec.rb b/spec/lib/vacuum/system_keys_vacuum_spec.rb new file mode 100644 index 000000000..565892f02 --- /dev/null +++ b/spec/lib/vacuum/system_keys_vacuum_spec.rb @@ -0,0 +1,22 @@ +require 'rails_helper' + +RSpec.describe Vacuum::SystemKeysVacuum do + subject { described_class.new } + + describe '#perform' do + let!(:expired_system_key) { Fabricate(:system_key, created_at: (SystemKey::ROTATION_PERIOD * 4).ago) } + let!(:current_system_key) { Fabricate(:system_key) } + + before do + subject.perform + end + + it 'deletes the expired key' do + expect { expired_system_key.reload }.to raise_error ActiveRecord::RecordNotFound + end + + it 'does not delete the current key' do + expect { current_system_key.reload }.to_not raise_error + end + end +end diff --git a/spec/workers/scheduler/feed_cleanup_scheduler_spec.rb b/spec/workers/scheduler/feed_cleanup_scheduler_spec.rb deleted file mode 100644 index 82d794594..000000000 --- a/spec/workers/scheduler/feed_cleanup_scheduler_spec.rb +++ /dev/null @@ -1,26 +0,0 @@ -require 'rails_helper' - -describe Scheduler::FeedCleanupScheduler do - subject { described_class.new } - - let!(:active_user) { Fabricate(:user, current_sign_in_at: 2.days.ago) } - let!(:inactive_user) { Fabricate(:user, current_sign_in_at: 22.days.ago) } - - it 'clears feeds of inactives' do - redis.zadd(feed_key_for(inactive_user), 1, 1) - redis.zadd(feed_key_for(active_user), 1, 1) - redis.zadd(feed_key_for(inactive_user, 'reblogs'), 2, 2) - redis.sadd(feed_key_for(inactive_user, 'reblogs:2'), 3) - - subject.perform - - expect(redis.zcard(feed_key_for(inactive_user))).to eq 0 - expect(redis.zcard(feed_key_for(active_user))).to eq 1 - expect(redis.exists?(feed_key_for(inactive_user, 'reblogs'))).to be false - expect(redis.exists?(feed_key_for(inactive_user, 'reblogs:2'))).to be false - end - - def feed_key_for(user, subtype = nil) - FeedManager.instance.key(:home, user.account_id, subtype) - end -end diff --git a/spec/workers/scheduler/media_cleanup_scheduler_spec.rb b/spec/workers/scheduler/media_cleanup_scheduler_spec.rb deleted file mode 100644 index 8a0da67e1..000000000 --- a/spec/workers/scheduler/media_cleanup_scheduler_spec.rb +++ /dev/null @@ -1,15 +0,0 @@ -require 'rails_helper' - -describe Scheduler::MediaCleanupScheduler do - subject { described_class.new } - - let!(:old_media) { Fabricate(:media_attachment, account_id: nil, created_at: 10.days.ago) } - let!(:new_media) { Fabricate(:media_attachment, account_id: nil, created_at: 1.hour.ago) } - - it 'removes old media records' do - subject.perform - - expect { old_media.reload }.to raise_error(ActiveRecord::RecordNotFound) - expect(new_media.reload).to be_persisted - end -end -- cgit