From 01ffaba3d2f9c5c94234e558f98f6795a6919b6c Mon Sep 17 00:00:00 2001 From: Phantasm Date: Mon, 22 Dec 2025 23:55:38 +0100 Subject: [PATCH] MastoAPI: Fix unauth visibility checks when fetching by Activity FlakeID - Adds another Pleroma.ActivityPub.Visibility.visible_for_user?/2 func - Modifies existing tests to include a local Activity referencing a remote Object - Changes Announce Activity test factory to reference Objects instead of Activities and use a different Actor for the Announce - Changes ap_id of remote user in Announce test factory to match Objects - Adds `object_local` option to Note factories that explicitly changes the domain in the URL to not match the endpoint URL in the test env to properly work with the new visibility func, since we don't store locality of Object unlike Activities --- lib/pleroma/web/activity_pub/visibility.ex | 19 +++ .../controllers/status_controller_test.exs | 159 +++++++++++++++--- test/support/factory.ex | 35 +++- 3 files changed, 182 insertions(+), 31 deletions(-) diff --git a/lib/pleroma/web/activity_pub/visibility.ex b/lib/pleroma/web/activity_pub/visibility.ex index 97fc7fa1b..b393947fe 100644 --- a/lib/pleroma/web/activity_pub/visibility.ex +++ b/lib/pleroma/web/activity_pub/visibility.ex @@ -73,6 +73,25 @@ defmodule Pleroma.Web.ActivityPub.Visibility do |> Pleroma.List.member?(user) end + def visible_for_user?(%Activity{data: _, object: %Object{data: _} = object} = activity, nil) do + activity_visibility? = restrict_unauthenticated_access?(activity) + activity_public? = public?(activity) and not local_public?(activity) + object_visibility? = restrict_unauthenticated_access?(object) + object_public? = public?(object) and not local_public?(object) + + # Activity could be local, but object might not (Announce/Like) + cond do + activity_visibility? == true and object_visibility? == true -> + false + + activity_visibility? or object_visibility? -> + false + + true -> + activity_public? and object_public? + end + end + def visible_for_user?(%{__struct__: module} = message, nil) when module in [Activity, Object] do if restrict_unauthenticated_access?(message), 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 5c24df864..ef6bcc9b1 100644 --- a/test/pleroma/web/mastodon_api/controllers/status_controller_test.exs +++ b/test/pleroma/web/mastodon_api/controllers/status_controller_test.exs @@ -759,9 +759,17 @@ defmodule Pleroma.Web.MastodonAPI.StatusControllerTest do end defp local_and_remote_activities do + remote_user = insert(:user, local: false, domain: "example.com") + announce_user = insert(:user) local = insert(:note_activity) - remote = insert(:note_activity, local: false) - {:ok, local: local, remote: remote} + remote = insert(:note_activity, local: false, object_local: false, user: remote_user) + remote_note = insert(:note_activity, local: false, object_local: false) + + local_activity_remote_object = + insert(:announce_activity, note_activity: remote_note, user: announce_user) + + {:ok, + local: local, remote: remote, local_activity_remote_object: local_activity_remote_object} end defp local_and_remote_context_activities do @@ -814,7 +822,12 @@ defmodule Pleroma.Web.MastodonAPI.StatusControllerTest do setup do: clear_config([:restrict_unauthenticated, :activities, :remote], true) - test "if user is unauthenticated", %{conn: conn, local: local, remote: remote} do + test "if user is unauthenticated", %{ + conn: conn, + local: local, + remote: remote, + local_activity_remote_object: local_activity_remote_object + } do res_conn = get(conn, "/api/v1/statuses/#{local.id}") assert json_response_and_validate_schema(res_conn, :not_found) == %{ @@ -823,18 +836,31 @@ defmodule Pleroma.Web.MastodonAPI.StatusControllerTest do res_conn = get(conn, "/api/v1/statuses/#{remote.id}") + assert json_response_and_validate_schema(res_conn, :not_found) == %{ + "error" => "Record not found" + } + + res_conn = get(conn, "/api/v1/statuses/#{local_activity_remote_object.id}") + assert json_response_and_validate_schema(res_conn, :not_found) == %{ "error" => "Record not found" } end - test "if user is authenticated", %{local: local, remote: remote} do + test "if user is authenticated", %{ + local: local, + remote: remote, + local_activity_remote_object: local_activity_remote_object + } do %{conn: conn} = oauth_access(["read"]) res_conn = get(conn, "/api/v1/statuses/#{local.id}") assert %{"id" => _} = json_response_and_validate_schema(res_conn, 200) res_conn = get(conn, "/api/v1/statuses/#{remote.id}") assert %{"id" => _} = json_response_and_validate_schema(res_conn, 200) + + res_conn = get(conn, "/api/v1/statuses/#{local_activity_remote_object.id}") + assert %{"id" => _} = json_response_and_validate_schema(res_conn, 200) end end @@ -843,9 +869,20 @@ defmodule Pleroma.Web.MastodonAPI.StatusControllerTest do setup do: clear_config([:restrict_unauthenticated, :activities, :local], true) - test "if user is unauthenticated", %{conn: conn, local: local, remote: remote} do + test "if user is unauthenticated", %{ + conn: conn, + local: local, + remote: remote, + local_activity_remote_object: local_activity_remote_object + } do res_conn = get(conn, "/api/v1/statuses/#{local.id}") + assert json_response_and_validate_schema(res_conn, :not_found) == %{ + "error" => "Record not found" + } + + res_conn = get(conn, "/api/v1/statuses/#{local_activity_remote_object.id}") + assert json_response_and_validate_schema(res_conn, :not_found) == %{ "error" => "Record not found" } @@ -854,13 +891,20 @@ defmodule Pleroma.Web.MastodonAPI.StatusControllerTest do assert %{"id" => _} = json_response_and_validate_schema(res_conn, 200) end - test "if user is authenticated", %{local: local, remote: remote} do + test "if user is authenticated", %{ + local: local, + remote: remote, + local_activity_remote_object: local_activity_remote_object + } do %{conn: conn} = oauth_access(["read"]) res_conn = get(conn, "/api/v1/statuses/#{local.id}") assert %{"id" => _} = json_response_and_validate_schema(res_conn, 200) res_conn = get(conn, "/api/v1/statuses/#{remote.id}") assert %{"id" => _} = json_response_and_validate_schema(res_conn, 200) + + res_conn = get(conn, "/api/v1/statuses/#{local_activity_remote_object.id}") + assert %{"id" => _} = json_response_and_validate_schema(res_conn, 200) end end @@ -869,24 +913,42 @@ defmodule Pleroma.Web.MastodonAPI.StatusControllerTest do setup do: clear_config([:restrict_unauthenticated, :activities, :remote], true) - test "if user is unauthenticated", %{conn: conn, local: local, remote: remote} do + test "if user is unauthenticated", %{ + conn: conn, + local: local, + remote: remote, + local_activity_remote_object: local_activity_remote_object + } do res_conn = get(conn, "/api/v1/statuses/#{local.id}") assert %{"id" => _} = json_response_and_validate_schema(res_conn, 200) res_conn = get(conn, "/api/v1/statuses/#{remote.id}") + assert json_response_and_validate_schema(res_conn, :not_found) == %{ + "error" => "Record not found" + } + + res_conn = get(conn, "/api/v1/statuses/#{local_activity_remote_object.id}") + assert json_response_and_validate_schema(res_conn, :not_found) == %{ "error" => "Record not found" } end - test "if user is authenticated", %{local: local, remote: remote} do + test "if user is authenticated", %{ + local: local, + remote: remote, + local_activity_remote_object: local_activity_remote_object + } do %{conn: conn} = oauth_access(["read"]) res_conn = get(conn, "/api/v1/statuses/#{local.id}") assert %{"id" => _} = json_response_and_validate_schema(res_conn, 200) res_conn = get(conn, "/api/v1/statuses/#{remote.id}") assert %{"id" => _} = json_response_and_validate_schema(res_conn, 200) + + res_conn = get(conn, "/api/v1/statuses/#{local_activity_remote_object.id}") + assert %{"id" => _} = json_response_and_validate_schema(res_conn, 200) end end @@ -946,18 +1008,35 @@ defmodule Pleroma.Web.MastodonAPI.StatusControllerTest do setup do: clear_config([:restrict_unauthenticated, :activities, :remote], true) - test "if user is unauthenticated", %{conn: conn, local: local, remote: remote} do - res_conn = get(conn, "/api/v1/statuses?id[]=#{local.id}&id[]=#{remote.id}") + test "if user is unauthenticated", %{ + conn: conn, + local: local, + remote: remote, + local_activity_remote_object: local_activity_remote_object + } do + res_conn = + get( + conn, + "/api/v1/statuses?id[]=#{local.id}&id[]=#{remote.id}&id[]=#{local_activity_remote_object.id}" + ) assert json_response_and_validate_schema(res_conn, 200) == [] end - test "if user is authenticated", %{local: local, remote: remote} do + test "if user is authenticated", %{ + local: local, + remote: remote, + local_activity_remote_object: local_activity_remote_object + } do %{conn: conn} = oauth_access(["read"]) - res_conn = get(conn, "/api/v1/statuses?id[]=#{local.id}&id[]=#{remote.id}") + res_conn = + get( + conn, + "/api/v1/statuses?id[]=#{local.id}&id[]=#{remote.id}&id[]=#{local_activity_remote_object.id}" + ) - assert length(json_response_and_validate_schema(res_conn, 200)) == 2 + assert length(json_response_and_validate_schema(res_conn, 200)) == 3 end end @@ -966,19 +1045,36 @@ defmodule Pleroma.Web.MastodonAPI.StatusControllerTest do setup do: clear_config([:restrict_unauthenticated, :activities, :local], true) - test "if user is unauthenticated", %{conn: conn, local: local, remote: remote} do - res_conn = get(conn, "/api/v1/statuses?id[]=#{local.id}&id[]=#{remote.id}") + test "if user is unauthenticated", %{ + conn: conn, + local: local, + remote: remote, + local_activity_remote_object: local_activity_remote_object + } do + res_conn = + get( + conn, + "/api/v1/statuses?id[]=#{local.id}&id[]=#{remote.id}&id[]=#{local_activity_remote_object.id}" + ) remote_id = remote.id assert [%{"id" => ^remote_id}] = json_response_and_validate_schema(res_conn, 200) end - test "if user is authenticated", %{local: local, remote: remote} do + test "if user is authenticated", %{ + local: local, + remote: remote, + local_activity_remote_object: local_activity_remote_object + } do %{conn: conn} = oauth_access(["read"]) - res_conn = get(conn, "/api/v1/statuses?id[]=#{local.id}&id[]=#{remote.id}") + res_conn = + get( + conn, + "/api/v1/statuses?id[]=#{local.id}&id[]=#{remote.id}&id[]=#{local_activity_remote_object.id}" + ) - assert length(json_response_and_validate_schema(res_conn, 200)) == 2 + assert length(json_response_and_validate_schema(res_conn, 200)) == 3 end end @@ -987,19 +1083,36 @@ defmodule Pleroma.Web.MastodonAPI.StatusControllerTest do setup do: clear_config([:restrict_unauthenticated, :activities, :remote], true) - test "if user is unauthenticated", %{conn: conn, local: local, remote: remote} do - res_conn = get(conn, "/api/v1/statuses?id[]=#{local.id}&id[]=#{remote.id}") + test "if user is unauthenticated", %{ + conn: conn, + local: local, + remote: remote, + local_activity_remote_object: local_activity_remote_object + } do + res_conn = + get( + conn, + "/api/v1/statuses?id[]=#{local.id}&id[]=#{remote.id}&id[]=#{local_activity_remote_object.id}" + ) local_id = local.id assert [%{"id" => ^local_id}] = json_response_and_validate_schema(res_conn, 200) end - test "if user is authenticated", %{local: local, remote: remote} do + test "if user is authenticated", %{ + local: local, + remote: remote, + local_activity_remote_object: local_activity_remote_object + } do %{conn: conn} = oauth_access(["read"]) - res_conn = get(conn, "/api/v1/statuses?id[]=#{local.id}&id[]=#{remote.id}") + res_conn = + get( + conn, + "/api/v1/statuses?id[]=#{local.id}&id[]=#{remote.id}&id[]=#{local_activity_remote_object.id}" + ) - assert length(json_response_and_validate_schema(res_conn, 200)) == 2 + assert length(json_response_and_validate_schema(res_conn, 200)) == 3 end end diff --git a/test/support/factory.ex b/test/support/factory.ex index 88c4ed8e5..d57090be6 100644 --- a/test/support/factory.ex +++ b/test/support/factory.ex @@ -102,11 +102,18 @@ defmodule Pleroma.Factory do user = attrs[:user] || insert(:user) + object_id = if attrs[:object_local] == false do + # Must not match our Endpoint URL in the test env + "https://example.com/objects/#{Ecto.UUID.generate()}" + else + Pleroma.Web.ActivityPub.Utils.generate_object_id() + end + data = %{ "type" => "Note", "content" => text, "source" => text, - "id" => Pleroma.Web.ActivityPub.Utils.generate_object_id(), + "id" => object_id, "actor" => user.ap_id, "to" => ["https://www.w3.org/ns/activitystreams#Public"], "published" => DateTime.utc_now() |> DateTime.to_iso8601(), @@ -361,14 +368,24 @@ defmodule Pleroma.Factory do def note_activity_factory(attrs \\ %{}) do user = attrs[:user] || insert(:user) - note = attrs[:note] || insert(:note, user: user) + object_local = if attrs[:object_local] == false, do: false, else: true + note = attrs[:note] || insert(:note, user: user, object_local: object_local) + + activity_id = if attrs[:local] == false do + # Same domain as in note Object factory, it doesn't make sense + # to create mismatched Create Activities with an ID coming from + # a different domain than the Object + "https://example.com/activities/#{Ecto.UUID.generate()}" + else + Pleroma.Web.ActivityPub.Utils.generate_activity_id() + end data_attrs = attrs[:data_attrs] || %{} - attrs = Map.drop(attrs, [:user, :note, :data_attrs]) + attrs = Map.drop(attrs, [:user, :note, :data_attrs, :object_local]) data = %{ - "id" => Pleroma.Web.ActivityPub.Utils.generate_activity_id(), + "id" => activity_id, "type" => "Create", "actor" => note.data["actor"], "to" => note.data["to"], @@ -408,15 +425,17 @@ defmodule Pleroma.Factory do def announce_activity_factory(attrs \\ %{}) do note_activity = attrs[:note_activity] || insert(:note_activity) + object = Object.normalize(note_activity, fetch: false) user = attrs[:user] || insert(:user) data = %{ + "id" => Pleroma.Web.ActivityPub.Utils.generate_activity_id(), "type" => "Announce", - "actor" => note_activity.actor, - "object" => note_activity.data["id"], - "to" => [user.follower_address, note_activity.data["actor"]], + "actor" => user.ap_id, + "object" => object.data["id"], + "to" => [user.follower_address, object.data["actor"]], "cc" => ["https://www.w3.org/ns/activitystreams#Public"], - "context" => note_activity.data["context"] + "context" => object.data["context"] } %Pleroma.Activity{