From 298684ff77ec1f8b5cbb1fcd3ba11d02dfe591e9 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 @@ defmodule Pleroma.User do 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 @@ defmodule Pleroma.User do 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 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do _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