Refresh Users much more aggressively when processing Move activities
Some checks failed
ci/woodpecker/pr/lint Pipeline was successful
ci/woodpecker/pr/test Pipeline failed
ci/woodpecker/pr/build-arm64 unknown status
ci/woodpecker/pr/build-amd64 unknown status
ci/woodpecker/pr/docs unknown status

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
This commit is contained in:
Erin Shepherd 2024-02-29 21:08:25 +01:00
parent 889b57df82
commit f18e2ba42c
2 changed files with 15 additions and 9 deletions

View file

@ -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} 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, options) do
NaiveDateTime.diff(NaiveDateTime.utc_now(), user.last_refreshed_at) >=
def needs_update?(%User{local: false} = user) do Keyword.get(options, :maximum_age, 86_400)
NaiveDateTime.diff(NaiveDateTime.utc_now(), user.last_refreshed_at) >= 86_400
end end
def needs_update?(_), do: true def needs_update?(_, _options), do: true
# "Locked" (self-locked) users demand explicit authorization of follow requests # "Locked" (self-locked) users demand explicit authorization of follow requests
@spec can_direct_follow_local(User.t(), User.t()) :: true | false @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 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) 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 case {cached_user, maybe_fetched_user} do
{_, {:ok, %User{} = user}} -> {_, {:ok, %User{} = user}} ->

View file

@ -576,7 +576,12 @@ def handle_incoming(
_options _options
) do ) do
with %User{} = origin_user <- User.get_cached_by_ap_id(origin_actor), 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 true <- origin_actor in target_user.also_known_as do
ActivityPub.move(origin_user, target_user, false) ActivityPub.move(origin_user, target_user, false)
else else