From 6c25ef3e12b9cd93f6ef7616ffe3147976bd77b4 Mon Sep 17 00:00:00 2001 From: Erin Shepherd Date: Thu, 29 Feb 2024 21:08:25 +0100 Subject: [PATCH] Refresh Users much more aggressively when processing Move activities The default refresh interval of 1 day is woefully inadequate here; users expect to be able to add the alias to their new account and press the move button on their old account and have it work. This allows callers to specify a maximum age before a refetch is triggered. We set that to 5s for the move code, as a nice compromise between Making Things Work and ensuring that this can't be used to hammer a remote server --- lib/pleroma/user.ex | 17 +++++++++-------- lib/pleroma/web/activity_pub/transmogrifier.ex | 7 ++++++- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index 8449af620..14414adc4 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -969,15 +969,16 @@ defp maybe_send_registration_email(%User{email: email} = user) when is_binary(em defp maybe_send_registration_email(_), do: {:ok, :noop} - def needs_update?(%User{local: true}), do: false + def needs_update?(user, options \\ []) + def needs_update?(%User{local: true}, _options), do: false + def needs_update?(%User{local: false, last_refreshed_at: nil}, _options), do: true - def needs_update?(%User{local: false, last_refreshed_at: nil}), do: true - - def needs_update?(%User{local: false} = user) do - NaiveDateTime.diff(NaiveDateTime.utc_now(), user.last_refreshed_at) >= 86_400 + def needs_update?(%User{local: false} = user, options) do + NaiveDateTime.diff(NaiveDateTime.utc_now(), user.last_refreshed_at) >= + Keyword.get(options, :maximum_age, 86_400) end - def needs_update?(_), do: true + def needs_update?(_, _options), do: true # "Locked" (self-locked) users demand explicit authorization of follow requests @spec can_direct_follow_local(User.t(), User.t()) :: true | false @@ -1980,10 +1981,10 @@ def html_filter_policy(_), do: Config.get([:markup, :scrub_policy]) def fetch_by_ap_id(ap_id), do: ActivityPub.make_user_from_ap_id(ap_id) - def get_or_fetch_by_ap_id(ap_id) do + def get_or_fetch_by_ap_id(ap_id, options \\ []) do cached_user = get_cached_by_ap_id(ap_id) - maybe_fetched_user = needs_update?(cached_user) && fetch_by_ap_id(ap_id) + maybe_fetched_user = needs_update?(cached_user, options) && fetch_by_ap_id(ap_id) case {cached_user, maybe_fetched_user} do {_, {:ok, %User{} = user}} -> diff --git a/lib/pleroma/web/activity_pub/transmogrifier.ex b/lib/pleroma/web/activity_pub/transmogrifier.ex index 435343bf0..a72a431b2 100644 --- a/lib/pleroma/web/activity_pub/transmogrifier.ex +++ b/lib/pleroma/web/activity_pub/transmogrifier.ex @@ -576,7 +576,12 @@ def handle_incoming( _options ) do with %User{} = origin_user <- User.get_cached_by_ap_id(origin_actor), - {:ok, %User{} = target_user} <- User.get_or_fetch_by_ap_id(target_actor), + # Use a dramatically shortened maximum age before refresh here because it is reasonable + # for a user to + # 1. Add the alias to their new account and then + # 2. Press the button on their new account + # within a very short period of time and expect it to work + {:ok, %User{} = target_user} <- User.get_or_fetch_by_ap_id(target_actor, maximum_age: 5), true <- origin_actor in target_user.also_known_as do ActivityPub.move(origin_user, target_user, false) else