From 6f090f981b26274c87b9df88e1bcb2719f46814b Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Mon, 25 Mar 2019 18:47:04 -0500 Subject: [PATCH 1/3] Attempt to fix incorrect federation of default instance avatars --- lib/pleroma/user.ex | 8 ++++++++ lib/pleroma/web/activity_pub/utils.ex | 15 +++++++++++++++ lib/pleroma/web/activity_pub/views/user_view.ex | 5 +---- 3 files changed, 24 insertions(+), 4 deletions(-) diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index 41289b4d0..ee5eb8efa 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -90,6 +90,14 @@ def avatar_url(user) do end end + # Do not return instance default avatar for federation + def avatar_url_ap(user) do + case user.avatar do + %{"url" => [%{"href" => href} | _]} -> href + _ -> nil + end + end + def banner_url(user) do case user.info.banner do %{"url" => [%{"href" => href} | _]} -> href diff --git a/lib/pleroma/web/activity_pub/utils.ex b/lib/pleroma/web/activity_pub/utils.ex index 2e9ffe41c..6d74738f0 100644 --- a/lib/pleroma/web/activity_pub/utils.ex +++ b/lib/pleroma/web/activity_pub/utils.ex @@ -230,6 +230,21 @@ def update_object_in_activities(%{data: %{"id" => id}} = object) do end) end + # Only federate user icon if not nil + # Prevents federating instance default avatars + def maybe_make_icon(user) do + if User.avatar_url_ap(user) do + %{ + "icon" => %{ + "type" => "Image", + "url" => User.avatar_url_ap(user) + } + } + else + [] + end + end + #### Like-related helpers @doc """ diff --git a/lib/pleroma/web/activity_pub/views/user_view.ex b/lib/pleroma/web/activity_pub/views/user_view.ex index 3d00dcbf2..f5c86d360 100644 --- a/lib/pleroma/web/activity_pub/views/user_view.ex +++ b/lib/pleroma/web/activity_pub/views/user_view.ex @@ -87,16 +87,13 @@ def render("user.json", %{user: user}) do "publicKeyPem" => public_key }, "endpoints" => endpoints, - "icon" => %{ - "type" => "Image", - "url" => User.avatar_url(user) - }, "image" => %{ "type" => "Image", "url" => User.banner_url(user) }, "tag" => user.info.source_data["tag"] || [] } + |> Map.merge(Utils.maybe_make_icon(user)) |> Map.merge(Utils.make_json_ld_header()) end From c410296120152095e7b9f5324a798f8446cf3bb3 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Mon, 25 Mar 2019 19:00:35 -0500 Subject: [PATCH 2/3] Try sending an empty map --- lib/pleroma/web/activity_pub/utils.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/pleroma/web/activity_pub/utils.ex b/lib/pleroma/web/activity_pub/utils.ex index 6d74738f0..86b6e7029 100644 --- a/lib/pleroma/web/activity_pub/utils.ex +++ b/lib/pleroma/web/activity_pub/utils.ex @@ -241,7 +241,7 @@ def maybe_make_icon(user) do } } else - [] + %{} end end From 10a7a4a868fbc5ff09516892b99e11100d5d3660 Mon Sep 17 00:00:00 2001 From: lain Date: Tue, 26 Mar 2019 16:40:09 +0100 Subject: [PATCH 3/3] AP UserView: Refactor banner / avatar display code, add test. --- lib/pleroma/user.ex | 16 ++++--------- lib/pleroma/web/activity_pub/utils.ex | 15 ------------ .../web/activity_pub/views/user_view.ex | 20 ++++++++++++---- .../web/activity_pub/views/user_view_test.exs | 23 +++++++++++++++++++ 4 files changed, 42 insertions(+), 32 deletions(-) diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index ee5eb8efa..efb4540b9 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -83,25 +83,17 @@ def superuser?(%User{local: true, info: %User.Info{is_admin: true}}), do: true def superuser?(%User{local: true, info: %User.Info{is_moderator: true}}), do: true def superuser?(_), do: false - def avatar_url(user) do + def avatar_url(user, options \\ []) do case user.avatar do %{"url" => [%{"href" => href} | _]} -> href - _ -> "#{Web.base_url()}/images/avi.png" + _ -> !options[:no_default] && "#{Web.base_url()}/images/avi.png" end end - # Do not return instance default avatar for federation - def avatar_url_ap(user) do - case user.avatar do - %{"url" => [%{"href" => href} | _]} -> href - _ -> nil - end - end - - def banner_url(user) do + def banner_url(user, options \\ []) do case user.info.banner do %{"url" => [%{"href" => href} | _]} -> href - _ -> "#{Web.base_url()}/images/banner.png" + _ -> !options[:no_default] && "#{Web.base_url()}/images/banner.png" end end diff --git a/lib/pleroma/web/activity_pub/utils.ex b/lib/pleroma/web/activity_pub/utils.ex index 86b6e7029..2e9ffe41c 100644 --- a/lib/pleroma/web/activity_pub/utils.ex +++ b/lib/pleroma/web/activity_pub/utils.ex @@ -230,21 +230,6 @@ def update_object_in_activities(%{data: %{"id" => id}} = object) do end) end - # Only federate user icon if not nil - # Prevents federating instance default avatars - def maybe_make_icon(user) do - if User.avatar_url_ap(user) do - %{ - "icon" => %{ - "type" => "Image", - "url" => User.avatar_url_ap(user) - } - } - else - %{} - end - end - #### Like-related helpers @doc """ diff --git a/lib/pleroma/web/activity_pub/views/user_view.ex b/lib/pleroma/web/activity_pub/views/user_view.ex index f5c86d360..5926a3294 100644 --- a/lib/pleroma/web/activity_pub/views/user_view.ex +++ b/lib/pleroma/web/activity_pub/views/user_view.ex @@ -87,13 +87,10 @@ def render("user.json", %{user: user}) do "publicKeyPem" => public_key }, "endpoints" => endpoints, - "image" => %{ - "type" => "Image", - "url" => User.banner_url(user) - }, "tag" => user.info.source_data["tag"] || [] } - |> Map.merge(Utils.maybe_make_icon(user)) + |> Map.merge(maybe_make_image(&User.avatar_url/2, "icon", user)) + |> Map.merge(maybe_make_image(&User.banner_url/2, "image", user)) |> Map.merge(Utils.make_json_ld_header()) end @@ -291,4 +288,17 @@ def collection(collection, iri, page, show_items \\ true, total \\ nil) do map end end + + defp maybe_make_image(func, key, user) do + if image = func.(user, no_default: true) do + %{ + key => %{ + "type" => "Image", + "url" => image + } + } + else + %{} + end + end end diff --git a/test/web/activity_pub/views/user_view_test.exs b/test/web/activity_pub/views/user_view_test.exs index 0bc1d4728..9fb9455d2 100644 --- a/test/web/activity_pub/views/user_view_test.exs +++ b/test/web/activity_pub/views/user_view_test.exs @@ -16,6 +16,29 @@ test "Renders a user, including the public key" do assert String.contains?(result["publicKey"]["publicKeyPem"], "BEGIN PUBLIC KEY") end + test "Does not add an avatar image if the user hasn't set one" do + user = insert(:user) + {:ok, user} = Pleroma.Web.WebFinger.ensure_keys_present(user) + + result = UserView.render("user.json", %{user: user}) + refute result["icon"] + refute result["image"] + + user = + insert(:user, + avatar: %{"url" => [%{"href" => "https://someurl"}]}, + info: %{ + banner: %{"url" => [%{"href" => "https://somebanner"}]} + } + ) + + {:ok, user} = Pleroma.Web.WebFinger.ensure_keys_present(user) + + result = UserView.render("user.json", %{user: user}) + assert result["icon"]["url"] == "https://someurl" + assert result["image"]["url"] == "https://somebanner" + end + describe "endpoints" do test "local users have a usable endpoints structure" do user = insert(:user)