Merge branch 'issue/2170' into 'develop'

[#2170] Can't remove dead relay

See merge request pleroma/pleroma!3041

CHANGELOG.md

@@ -8,6 +8,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 ### Added
 - Mix tasks for controlling user account confirmation status in bulk (`mix pleroma.user confirm_all` and `mix pleroma.user unconfirm_all`)
 - Mix task for sending confirmation emails to all unconfirmed users (`mix pleroma.email send_confirmation_mails`)
+- Mix task option for force-unfollowing relays
 
 ### Changed
 

docs/API/admin_api.md

@@ -349,9 +349,9 @@ Response:
 
 ### Unfollow a Relay
 
-Params:
+- Params:
-
+  - `relay_url`
-* `relay_url`
+  - *optional* `force`: forcefully unfollow a relay even when the relay is not available. (default is `false`)
 
 Response:
 

lib/mix/tasks/pleroma/relay.ex

@@ -21,10 +21,19 @@ def run(["follow", target]) do
     end
   end
 
-  def run(["unfollow", target]) do
+  def run(["unfollow", target | rest]) do
     start_pleroma()
 
-    with {:ok, _activity} <- Relay.unfollow(target) do
+    {options, [], []} =
+      OptionParser.parse(
+        rest,
+        strict: [force: :boolean],
+        aliases: [f: :force]
+      )
+
+    force = Keyword.get(options, :force, false)
+
+    with {:ok, _activity} <- Relay.unfollow(target, %{force: force}) do
       # put this task to sleep to allow the genserver to push out the messages
       :timer.sleep(500)
     else

lib/pleroma/user.ex

@@ -915,9 +915,7 @@ defp do_unfollow(%User{} = follower, %User{} = followed) do
         FollowingRelationship.unfollow(follower, followed)
         {:ok, followed} = update_follower_count(followed)
 
-        {:ok, follower} =
+        {:ok, follower} = update_following_count(follower)
-          follower
-          |> update_following_count()
 
         {:ok, follower, followed}
 

lib/pleroma/web/activity_pub/relay.ex

@@ -30,12 +30,16 @@ def follow(target_instance) do
     end
   end
 
-  @spec unfollow(String.t()) :: {:ok, Activity.t()} | {:error, any()}
+  @spec unfollow(String.t(), map()) :: {:ok, Activity.t()} | {:error, any()}
-  def unfollow(target_instance) do
+  def unfollow(target_instance, opts \\ %{}) do
     with %User{} = local_user <- get_actor(),
-         {:ok, %User{} = target_user} <- User.get_or_fetch_by_ap_id(target_instance),
+         {:ok, target_user} <- fetch_target_user(target_instance, opts),
          {:ok, activity} <- ActivityPub.unfollow(local_user, target_user) do
-      User.unfollow(local_user, target_user)
+      case target_user.id do
+        nil -> User.update_following_count(local_user)
+        _ -> User.unfollow(local_user, target_user)
+      end
+
       Logger.info("relay: unfollowed instance: #{target_instance}: id=#{activity.data["id"]}")
       {:ok, activity}
     else
@@ -43,6 +47,14 @@ def unfollow(target_instance) do
     end
   end
 
+  defp fetch_target_user(ap_id, opts) do
+    case {opts[:force], User.get_or_fetch_by_ap_id(ap_id)} do
+      {_, {:ok, %User{} = user}} -> {:ok, user}
+      {true, _} -> {:ok, %User{ap_id: ap_id}}
+      {_, error} -> error
+    end
+  end
+
   @spec publish(any()) :: {:ok, Activity.t()} | {:error, any()}
   def publish(%Activity{data: %{"type" => "Create"}} = activity) do
     with %User{} = user <- get_actor(),

lib/pleroma/web/admin_api/controllers/relay_controller.ex

@@ -33,11 +33,7 @@ def index(conn, _params) do
 
   def follow(%{assigns: %{user: admin}, body_params: %{relay_url: target}} = conn, _) do
     with {:ok, _message} <- Relay.follow(target) do
-      ModerationLog.insert_log(%{
+      ModerationLog.insert_log(%{action: "relay_follow", actor: admin, target: target})
-        action: "relay_follow",
-        actor: admin,
-        target: target
-      })
 
       json(conn, %{actor: target, followed_back: target in Relay.following()})
     else
@@ -48,13 +44,9 @@ def follow(%{assigns: %{user: admin}, body_params: %{relay_url: target}} = conn,
     end
   end
 
-  def unfollow(%{assigns: %{user: admin}, body_params: %{relay_url: target}} = conn, _) do
+  def unfollow(%{assigns: %{user: admin}, body_params: %{relay_url: target} = params} = conn, _) do
-    with {:ok, _message} <- Relay.unfollow(target) do
+    with {:ok, _message} <- Relay.unfollow(target, %{force: params[:force]}) do
-      ModerationLog.insert_log(%{
+      ModerationLog.insert_log(%{action: "relay_unfollow", actor: admin, target: target})
-        action: "relay_unfollow",
-        actor: admin,
-        target: target
-      })
 
       json(conn, target)
     else

lib/pleroma/web/api_spec/operations/admin/relay_operation.ex

@@ -56,7 +56,7 @@ def unfollow_operation do
       operationId: "AdminAPI.RelayController.unfollow",
       security: [%{"oAuth" => ["write:follows"]}],
       parameters: admin_api_params(),
-      requestBody: request_body("Parameters", relay_url()),
+      requestBody: request_body("Parameters", relay_unfollow()),
       responses: %{
         200 =>
           Operation.response("Status", "application/json", %Schema{
@@ -91,4 +91,14 @@ defp relay_url do
       }
     }
   end
+
+  defp relay_unfollow do
+    %Schema{
+      type: :object,
+      properties: %{
+        relay_url: %Schema{type: :string, format: :uri},
+        force: %Schema{type: :boolean, default: false}
+      }
+    }
+  end
 end

test/tasks/relay_test.exs

@@ -81,6 +81,80 @@ test "relay is unfollowed" do
       assert undo_activity.data["object"]["id"] == cancelled_activity.data["id"]
       refute "#{target_instance}/followers" in User.following(local_user)
     end
+
+    test "unfollow when relay is dead" do
+      user = insert(:user)
+      target_instance = user.ap_id
+
+      Mix.Tasks.Pleroma.Relay.run(["follow", target_instance])
+
+      %User{ap_id: follower_id} = local_user = Relay.get_actor()
+      target_user = User.get_cached_by_ap_id(target_instance)
+      follow_activity = Utils.fetch_latest_follow(local_user, target_user)
+      User.follow(local_user, target_user)
+
+      assert "#{target_instance}/followers" in User.following(local_user)
+
+      Tesla.Mock.mock(fn %{method: :get, url: ^target_instance} ->
+        %Tesla.Env{status: 404}
+      end)
+
+      Pleroma.Repo.delete(user)
+      Cachex.clear(:user_cache)
+
+      Mix.Tasks.Pleroma.Relay.run(["unfollow", target_instance])
+
+      cancelled_activity = Activity.get_by_ap_id(follow_activity.data["id"])
+      assert cancelled_activity.data["state"] == "accept"
+
+      assert [] ==
+               ActivityPub.fetch_activities(
+                 [],
+                 %{
+                   type: "Undo",
+                   actor_id: follower_id,
+                   skip_preload: true,
+                   invisible_actors: true
+                 }
+               )
+    end
+
+    test "force unfollow when relay is dead" do
+      user = insert(:user)
+      target_instance = user.ap_id
+
+      Mix.Tasks.Pleroma.Relay.run(["follow", target_instance])
+
+      %User{ap_id: follower_id} = local_user = Relay.get_actor()
+      target_user = User.get_cached_by_ap_id(target_instance)
+      follow_activity = Utils.fetch_latest_follow(local_user, target_user)
+      User.follow(local_user, target_user)
+
+      assert "#{target_instance}/followers" in User.following(local_user)
+
+      Tesla.Mock.mock(fn %{method: :get, url: ^target_instance} ->
+        %Tesla.Env{status: 404}
+      end)
+
+      Pleroma.Repo.delete(user)
+      Cachex.clear(:user_cache)
+
+      Mix.Tasks.Pleroma.Relay.run(["unfollow", target_instance, "--force"])
+
+      cancelled_activity = Activity.get_by_ap_id(follow_activity.data["id"])
+      assert cancelled_activity.data["state"] == "cancelled"
+
+      [undo_activity] =
+        ActivityPub.fetch_activities(
+          [],
+          %{type: "Undo", actor_id: follower_id, skip_preload: true, invisible_actors: true}
+        )
+
+      assert undo_activity.data["type"] == "Undo"
+      assert undo_activity.data["actor"] == local_user.ap_id
+      assert undo_activity.data["object"]["id"] == cancelled_activity.data["id"]
+      refute "#{target_instance}/followers" in User.following(local_user)
+    end
   end
 
   describe "mix pleroma.relay list" do

test/web/activity_pub/relay_test.exs

@@ -63,6 +63,46 @@ test "returns activity" do
       assert activity.data["to"] == [user.ap_id]
       refute "#{user.ap_id}/followers" in User.following(service_actor)
     end
+
+    test "force unfollow when target service is dead" do
+      user = insert(:user)
+      user_ap_id = user.ap_id
+      user_id = user.id
+
+      Tesla.Mock.mock(fn %{method: :get, url: ^user_ap_id} ->
+        %Tesla.Env{status: 404}
+      end)
+
+      service_actor = Relay.get_actor()
+      CommonAPI.follow(service_actor, user)
+      assert "#{user.ap_id}/followers" in User.following(service_actor)
+
+      assert Pleroma.Repo.get_by(
+               Pleroma.FollowingRelationship,
+               follower_id: service_actor.id,
+               following_id: user_id
+             )
+
+      Pleroma.Repo.delete(user)
+      Cachex.clear(:user_cache)
+
+      assert {:ok, %Activity{} = activity} = Relay.unfollow(user_ap_id, %{force: true})
+
+      assert refresh_record(service_actor).following_count == 0
+
+      refute Pleroma.Repo.get_by(
+               Pleroma.FollowingRelationship,
+               follower_id: service_actor.id,
+               following_id: user_id
+             )
+
+      assert activity.actor == "#{Pleroma.Web.Endpoint.url()}/relay"
+      assert user.ap_id in activity.recipients
+      assert activity.data["type"] == "Undo"
+      assert activity.data["actor"] == service_actor.ap_id
+      assert activity.data["to"] == [user_ap_id]
+      refute "#{user.ap_id}/followers" in User.following(service_actor)
+    end
   end
 
   describe "publish/1" do