From f29918e7071160f277ac5834d83e409d8fa20063 Mon Sep 17 00:00:00 2001 From: ThibG Date: Tue, 12 Sep 2017 23:10:40 +0200 Subject: [WiP] Whenever a remote keypair changes, unfollow them and re-subscribe to … (#4907) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Whenever a remote keypair changes, unfollow them and re-subscribe to them In Mastodon (it could be different for other OStatus or AP-enabled software), a keypair change is indicative of whole user (or instance) data loss. In this situation, the “new” user might be different, and almost certainly has an empty followers list. In this case, Mastodon instances will disagree on follower lists, leading to unreliable delivery and “shadow followers”, that is users believed by a remote instance to be followers, without the affected user knowing. Drawbacks of this change are: 1. If an user legitimately changes public key for some reason without losing data (not possible in Mastodon at the moment), they will have their remote followers unsubscribed/re-subscribed needlessly. 2. Depending of the number of remote followers, this may generate quite some traffic. 3. If the user change is an attempt at usurpation, the remote followers will unknowingly follow the usurper. Note that this is *not* a change of behavior, Mastodon already behaves like that, although delivery might be unreliable, and the usurper would not have known the former user's followers. * Rename ResubscribeWorker to RefollowWorker * Process followers in batches --- app/workers/refollow_worker.rb | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 app/workers/refollow_worker.rb (limited to 'app/workers') diff --git a/app/workers/refollow_worker.rb b/app/workers/refollow_worker.rb new file mode 100644 index 000000000..9c42d4271 --- /dev/null +++ b/app/workers/refollow_worker.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +class RefollowWorker + include Sidekiq::Worker + + sidekiq_options queue: 'pull', retry: false + + def perform(target_account_id) + target_account = Account.find(target_account_id) + + target_account.followers.where(domain: nil).find_each do |follower| + # Locally unfollow remote account + follower.unfollow!(target_account) + + # Schedule re-follow + begin + FollowService.new.call(follower, target_account) + rescue Mastodon::NotPermittedError, ActiveRecord::RecordNotFound, Mastodon::UnexpectedResponseError, HTTP::Error, OpenSSL::SSL::SSLError + next + end + end + end +end -- cgit From af00220d795670e10bc8c7378837c4a5a287b556 Mon Sep 17 00:00:00 2001 From: ThibG Date: Thu, 14 Sep 2017 00:05:25 +0200 Subject: Fix refollowing (#4931) * Make RefollowWorker ActivityPub-only to avoid potential identifier mismatches * Don't call RefollowWorker on new accounts --- app/services/activitypub/process_account_service.rb | 4 ++-- app/services/resolve_remote_account_service.rb | 2 -- app/workers/refollow_worker.rb | 1 + 3 files changed, 3 insertions(+), 4 deletions(-) (limited to 'app/workers') diff --git a/app/services/activitypub/process_account_service.rb b/app/services/activitypub/process_account_service.rb index badb26720..a45681078 100644 --- a/app/services/activitypub/process_account_service.rb +++ b/app/services/activitypub/process_account_service.rb @@ -15,11 +15,11 @@ class ActivityPub::ProcessAccountService < BaseService @account = Account.find_by(uri: @uri) @collections = {} + old_public_key = @account&.public_key create_account if @account.nil? upgrade_account if @account.ostatus? - old_public_key = @account.public_key update_account - RefollowWorker.perform_async(@account.id) if old_public_key != @account.public_key + RefollowWorker.perform_async(@account.id) if !old_public_key.nil? && old_public_key != @account.public_key @account rescue Oj::ParseError diff --git a/app/services/resolve_remote_account_service.rb b/app/services/resolve_remote_account_service.rb index 753601501..7031c98f5 100644 --- a/app/services/resolve_remote_account_service.rb +++ b/app/services/resolve_remote_account_service.rb @@ -85,10 +85,8 @@ class ResolveRemoteAccountService < BaseService def handle_ostatus create_account if @account.nil? - old_public_key = @account.public_key update_account update_account_profile if update_profile? - RefollowWorker.perform_async(@account.id) if old_public_key != @account.public_key end def update_profile? diff --git a/app/workers/refollow_worker.rb b/app/workers/refollow_worker.rb index 9c42d4271..66bcd27c3 100644 --- a/app/workers/refollow_worker.rb +++ b/app/workers/refollow_worker.rb @@ -7,6 +7,7 @@ class RefollowWorker def perform(target_account_id) target_account = Account.find(target_account_id) + return unless target_account.protocol == :activitypub target_account.followers.where(domain: nil).find_each do |follower| # Locally unfollow remote account -- cgit From 1aad015bbbe7957827c2b921a21c53ce11c6ac36 Mon Sep 17 00:00:00 2001 From: abcang Date: Thu, 14 Sep 2017 22:12:43 +0900 Subject: Revert unique retry job (#4937) * Revert "Enable UniqueRetryJobMiddleware even when called from sidekiq worker (#4836)" This reverts commit 6859d4c0289e767955aac3f345074220fe200604. * Revert "Do not execute the job with the same arguments as the retry job (#4814)" This reverts commit be7ffa2d7539d5a1946a3933cb9d242b9fac0ddc. --- app/workers/pubsubhubbub/subscribe_worker.rb | 2 +- config/application.rb | 1 - config/initializers/sidekiq.rb | 6 ------ lib/mastodon/unique_retry_job_middleware.rb | 20 -------------------- 4 files changed, 1 insertion(+), 28 deletions(-) delete mode 100644 lib/mastodon/unique_retry_job_middleware.rb (limited to 'app/workers') diff --git a/app/workers/pubsubhubbub/subscribe_worker.rb b/app/workers/pubsubhubbub/subscribe_worker.rb index 130c967e0..7560c2671 100644 --- a/app/workers/pubsubhubbub/subscribe_worker.rb +++ b/app/workers/pubsubhubbub/subscribe_worker.rb @@ -3,7 +3,7 @@ class Pubsubhubbub::SubscribeWorker include Sidekiq::Worker - sidekiq_options queue: 'push', retry: 10, unique: :until_executed, dead: false, unique_retry: true + sidekiq_options queue: 'push', retry: 10, unique: :until_executed, dead: false sidekiq_retry_in do |count| case count diff --git a/config/application.rb b/config/application.rb index f98f7af16..b6ce74147 100644 --- a/config/application.rb +++ b/config/application.rb @@ -10,7 +10,6 @@ require_relative '../app/lib/exceptions' require_relative '../lib/paperclip/gif_transcoder' require_relative '../lib/paperclip/video_transcoder' require_relative '../lib/mastodon/version' -require_relative '../lib/mastodon/unique_retry_job_middleware' Dotenv::Railtie.load diff --git a/config/initializers/sidekiq.rb b/config/initializers/sidekiq.rb index 0ee77730e..b70784d79 100644 --- a/config/initializers/sidekiq.rb +++ b/config/initializers/sidekiq.rb @@ -9,14 +9,8 @@ end Sidekiq.configure_server do |config| config.redis = redis_params - config.client_middleware do |chain| - chain.add Mastodon::UniqueRetryJobMiddleware - end end Sidekiq.configure_client do |config| config.redis = redis_params - config.client_middleware do |chain| - chain.add Mastodon::UniqueRetryJobMiddleware - end end diff --git a/lib/mastodon/unique_retry_job_middleware.rb b/lib/mastodon/unique_retry_job_middleware.rb deleted file mode 100644 index 75da8a0c9..000000000 --- a/lib/mastodon/unique_retry_job_middleware.rb +++ /dev/null @@ -1,20 +0,0 @@ -# frozen_string_literal: true - -class Mastodon::UniqueRetryJobMiddleware - def call(_worker_class, item, _queue, _redis_pool) - return if item['unique_retry'] && retried?(item) - yield - end - - private - - def retried?(item) - # Use unique digest key of SidekiqUniqueJobs - unique_key = SidekiqUniqueJobs::UNIQUE_DIGEST_KEY - unique_digest = item[unique_key] - class_name = item['class'] - retries = Sidekiq::RetrySet.new - - retries.any? { |job| job.item['class'] == class_name && job.item[unique_key] == unique_digest } - end -end -- cgit From 67559361e8db437ffee29af9a4fc4ea9e2daf51f Mon Sep 17 00:00:00 2001 From: sdukhovni Date: Thu, 14 Sep 2017 16:26:38 -0400 Subject: Add scheduled worker to purge old user IPs (#4951) * Add scheduled worker to purge old user IPs * Use ruby 1.9 hash syntax --- app/workers/scheduler/ip_cleanup_scheduler.rb | 12 ++++++++++++ config/sidekiq.yml | 3 +++ 2 files changed, 15 insertions(+) create mode 100644 app/workers/scheduler/ip_cleanup_scheduler.rb (limited to 'app/workers') diff --git a/app/workers/scheduler/ip_cleanup_scheduler.rb b/app/workers/scheduler/ip_cleanup_scheduler.rb new file mode 100644 index 000000000..9f1593c91 --- /dev/null +++ b/app/workers/scheduler/ip_cleanup_scheduler.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true +require 'sidekiq-scheduler' + +class Scheduler::IpCleanupScheduler + include Sidekiq::Worker + + def perform + time_ago = 5.years.ago + SessionActivation.where('updated_at < ?', time_ago).destroy_all + User.where('last_sign_in_at < ?', time_ago).update_all(last_sign_in_ip: nil) + end +end diff --git a/config/sidekiq.yml b/config/sidekiq.yml index a502f5593..5e4310e7e 100644 --- a/config/sidekiq.yml +++ b/config/sidekiq.yml @@ -24,3 +24,6 @@ subscriptions_cleanup_scheduler: cron: '2 2 * * 0' class: Scheduler::SubscriptionsCleanupScheduler + ip_cleanup_scheduler: + cron: '0 4 * * *' + class: Scheduler::IpCleanupScheduler -- cgit From 0698c610a6270f2c184dd97534afb0359cd23994 Mon Sep 17 00:00:00 2001 From: unarist Date: Sun, 17 Sep 2017 01:18:00 +0900 Subject: Fix an error in ReplyDistributionWorker when replied status was deleted (#4974) Reply distribution is proceed by Sidekiq, so replied status may be deleted before this. --- app/workers/activitypub/reply_distribution_worker.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'app/workers') diff --git a/app/workers/activitypub/reply_distribution_worker.rb b/app/workers/activitypub/reply_distribution_worker.rb index f9127340f..fe99fc05f 100644 --- a/app/workers/activitypub/reply_distribution_worker.rb +++ b/app/workers/activitypub/reply_distribution_worker.rb @@ -7,9 +7,9 @@ class ActivityPub::ReplyDistributionWorker def perform(status_id) @status = Status.find(status_id) - @account = @status.thread.account + @account = @status.thread&.account - return if skip_distribution? + return if @account.nil? || skip_distribution? ActivityPub::DeliveryWorker.push_bulk(inboxes) do |inbox_url| [signed_payload, @status.account_id, inbox_url] -- cgit