From 7cf473c50076f31bb01bad92501a8c2353874b96 Mon Sep 17 00:00:00 2001 From: Ilja Date: Mon, 13 Jun 2022 11:00:49 +0200 Subject: [PATCH] delete statusses is now privileged by :status_delete Instead of superusers, you now need a role with privilige :status_delete to delete other users statusses I also cleaned up some other stuff I saw --- lib/pleroma/web/common_api.ex | 2 +- .../controllers/instance_controller_test.exs | 7 +++-- test/pleroma/web/common_api_test.exs | 26 ++++++++++--------- .../controllers/status_controller_test.exs | 22 +++++----------- 4 files changed, 24 insertions(+), 33 deletions(-) diff --git a/lib/pleroma/web/common_api.ex b/lib/pleroma/web/common_api.ex index 1b95ee89c..ce1d5a7cc 100644 --- a/lib/pleroma/web/common_api.ex +++ b/lib/pleroma/web/common_api.ex @@ -144,7 +144,7 @@ def delete(activity_id, user) do {:find_activity, Activity.get_by_id(activity_id)}, {_, %Object{} = object, _} <- {:find_object, Object.normalize(activity, fetch: false), activity}, - true <- User.superuser?(user) || user.ap_id == object.data["actor"], + true <- User.privileged?(user, :status_delete) || user.ap_id == object.data["actor"], {:ok, delete_data, _} <- Builder.delete(user, object.data["id"]), {:ok, delete, _} <- Pipeline.common_pipeline(delete_data, local: true) do {:ok, delete} diff --git a/test/pleroma/web/admin_api/controllers/instance_controller_test.exs b/test/pleroma/web/admin_api/controllers/instance_controller_test.exs index b757ce469..e75222f99 100644 --- a/test/pleroma/web/admin_api/controllers/instance_controller_test.exs +++ b/test/pleroma/web/admin_api/controllers/instance_controller_test.exs @@ -85,9 +85,8 @@ test "DELETE /instances/:instance", %{conn: conn} do clear_config([:instance, :admin_privileges], []) - response = - conn - |> delete("/api/pleroma/admin/instances/lain.com") - |> json_response(:forbidden) + conn + |> delete("/api/pleroma/admin/instances/lain.com") + |> json_response(:forbidden) end end diff --git a/test/pleroma/web/common_api_test.exs b/test/pleroma/web/common_api_test.exs index b502aaa03..4d960e945 100644 --- a/test/pleroma/web/common_api_test.exs +++ b/test/pleroma/web/common_api_test.exs @@ -4,7 +4,7 @@ defmodule Pleroma.Web.CommonAPITest do use Oban.Testing, repo: Pleroma.Repo - use Pleroma.DataCase + use Pleroma.DataCase, async: false alias Pleroma.Activity alias Pleroma.Chat @@ -321,7 +321,7 @@ test "it allows users to delete their posts" do refute Activity.get_by_id(post.id) end - test "it does not allow a user to delete their posts" do + test "it does not allow a user to delete posts from another user" do user = insert(:user) other_user = insert(:user) @@ -331,7 +331,8 @@ test "it does not allow a user to delete their posts" do assert Activity.get_by_id(post.id) end - test "it allows moderators to delete other user's posts" do + test "it allows privileged users to delete other user's posts" do + clear_config([:instance, :moderator_privileges], [:status_delete]) user = insert(:user) moderator = insert(:user, is_moderator: true) @@ -343,19 +344,20 @@ test "it allows moderators to delete other user's posts" do refute Activity.get_by_id(post.id) end - test "it allows admins to delete other user's posts" do + test "it doesn't allow unprivileged mods or admins to delete other user's posts" do + clear_config([:instance, :admin_privileges], []) + clear_config([:instance, :moderator_privileges], []) user = insert(:user) - moderator = insert(:user, is_admin: true) + moderator = insert(:user, is_moderator: true, is_admin: true) {:ok, post} = CommonAPI.post(user, %{status: "namu amida butsu"}) - assert {:ok, delete} = CommonAPI.delete(post.id, moderator) - assert delete.local - - refute Activity.get_by_id(post.id) + assert {:error, "Could not delete"} = CommonAPI.delete(post.id, moderator) + assert Activity.get_by_id(post.id) end - test "superusers deleting non-local posts won't federate the delete" do + test "privileged users deleting non-local posts won't federate the delete" do + clear_config([:instance, :admin_privileges], [:status_delete]) # This is the user of the ingested activity _user = insert(:user, @@ -364,7 +366,7 @@ test "superusers deleting non-local posts won't federate the delete" do last_refreshed_at: NaiveDateTime.utc_now() ) - moderator = insert(:user, is_admin: true) + admin = insert(:user, is_admin: true) data = File.read!("test/fixtures/mastodon-post-activity.json") @@ -374,7 +376,7 @@ test "superusers deleting non-local posts won't federate the delete" do with_mock Pleroma.Web.Federator, publish: fn _ -> nil end do - assert {:ok, delete} = CommonAPI.delete(post.id, moderator) + assert {:ok, delete} = CommonAPI.delete(post.id, admin) assert delete.local refute called(Pleroma.Web.Federator.publish(:_)) end diff --git a/test/pleroma/web/mastodon_api/controllers/status_controller_test.exs b/test/pleroma/web/mastodon_api/controllers/status_controller_test.exs index dc6912b7b..4ea92e329 100644 --- a/test/pleroma/web/mastodon_api/controllers/status_controller_test.exs +++ b/test/pleroma/web/mastodon_api/controllers/status_controller_test.exs @@ -3,7 +3,7 @@ # SPDX-License-Identifier: AGPL-3.0-only defmodule Pleroma.Web.MastodonAPI.StatusControllerTest do - use Pleroma.Web.ConnCase + use Pleroma.Web.ConnCase, async: false use Oban.Testing, repo: Pleroma.Repo alias Pleroma.Activity @@ -968,30 +968,20 @@ test "when you didn't create it" do assert Activity.get_by_id(activity.id) == activity end - test "when you're an admin or moderator", %{conn: conn} do - activity1 = insert(:note_activity) - activity2 = insert(:note_activity) - admin = insert(:user, is_admin: true) + test "when you're privileged to", %{conn: conn} do + clear_config([:instance, :moderator_privileges], [:status_delete]) + activity = insert(:note_activity) moderator = insert(:user, is_moderator: true) - res_conn = - conn - |> assign(:user, admin) - |> assign(:token, insert(:oauth_token, user: admin, scopes: ["write:statuses"])) - |> delete("/api/v1/statuses/#{activity1.id}") - - assert %{} = json_response_and_validate_schema(res_conn, 200) - res_conn = conn |> assign(:user, moderator) |> assign(:token, insert(:oauth_token, user: moderator, scopes: ["write:statuses"])) - |> delete("/api/v1/statuses/#{activity2.id}") + |> delete("/api/v1/statuses/#{activity.id}") assert %{} = json_response_and_validate_schema(res_conn, 200) - refute Activity.get_by_id(activity1.id) - refute Activity.get_by_id(activity2.id) + refute Activity.get_by_id(activity.id) end end