From 3bffabccb62a425c46162791957d100c98659ba7 Mon Sep 17 00:00:00 2001 From: Thomas Citharel Date: Mon, 8 Mar 2021 11:43:07 +0100 Subject: [PATCH 1/3] Only federate group draft posts to members Closes #615 Signed-off-by: Thomas Citharel --- lib/federation/activity_pub/activity_pub.ex | 2 +- lib/federation/activity_pub/audience.ex | 26 +++++++++++------ lib/federation/activity_pub/types/posts.ex | 6 ++-- lib/federation/activity_pub/utils.ex | 8 +++++- .../activity_stream/converter/post.ex | 14 ++++------ test/graphql/resolvers/post_test.exs | 28 +++++++++++++++++++ 6 files changed, 62 insertions(+), 22 deletions(-) diff --git a/lib/federation/activity_pub/activity_pub.ex b/lib/federation/activity_pub/activity_pub.ex index ead2d141..651aa768 100644 --- a/lib/federation/activity_pub/activity_pub.ex +++ b/lib/federation/activity_pub/activity_pub.ex @@ -710,7 +710,7 @@ defmodule Mobilizon.Federation.ActivityPub do @spec publish(Actor.t(), Activity.t()) :: :ok def publish(actor, %Activity{recipients: recipients} = activity) do Logger.debug("Publishing an activity") - Logger.debug(inspect(activity)) + Logger.debug(inspect(activity, pretty: true)) public = Visibility.is_public?(activity) Logger.debug("is public ? #{public}") diff --git a/lib/federation/activity_pub/audience.ex b/lib/federation/activity_pub/audience.ex index 1e491cb3..f129e59b 100644 --- a/lib/federation/activity_pub/audience.ex +++ b/lib/federation/activity_pub/audience.ex @@ -123,19 +123,29 @@ defmodule Mobilizon.Federation.ActivityPub.Audience do end def calculate_to_and_cc_from_mentions(%Post{ - attributed_to: %Actor{members_url: members_url}, - visibility: visibility + attributed_to: %Actor{members_url: members_url, followers_url: followers_url}, + visibility: visibility, + draft: draft }) do - case visibility do - :public -> - %{"to" => [@ap_public, members_url], "cc" => []} + cond do + # If the post is draft we send it only to members + draft == true -> + %{"to" => [members_url], "cc" => []} - :unlisted -> - %{"to" => [members_url], "cc" => [@ap_public]} + # If public everyone + visibility == :public -> + %{"to" => [@ap_public, members_url], "cc" => [followers_url]} - :private -> + # Otherwise just followers + visibility == :unlisted -> + %{"to" => [followers_url, members_url], "cc" => [@ap_public]} + + visibility == :private -> # Private is restricted to only the members %{"to" => [members_url], "cc" => []} + + true -> + %{"to" => [], "cc" => []} end end diff --git a/lib/federation/activity_pub/types/posts.ex b/lib/federation/activity_pub/types/posts.ex index 47a9a034..8421f70d 100644 --- a/lib/federation/activity_pub/types/posts.ex +++ b/lib/federation/activity_pub/types/posts.ex @@ -24,10 +24,8 @@ defmodule Mobilizon.Federation.ActivityPub.Types.Posts do {:ok, %Actor{} = group} <- Actors.get_group_by_actor_id(group_id), %Actor{} = creator <- Actors.get_actor(creator_id), post_as_data <- - Convertible.model_to_as(%{post | attributed_to: group, author: creator}), - audience <- - Audience.calculate_to_and_cc_from_mentions(post) do - create_data = make_create_data(post_as_data, Map.merge(audience, additional)) + Convertible.model_to_as(%{post | attributed_to: group, author: creator}) do + create_data = make_create_data(post_as_data, additional) {:ok, post, create_data} else diff --git a/lib/federation/activity_pub/utils.ex b/lib/federation/activity_pub/utils.ex index 6d39f303..988d3233 100644 --- a/lib/federation/activity_pub/utils.ex +++ b/lib/federation/activity_pub/utils.ex @@ -421,7 +421,13 @@ defmodule Mobilizon.Federation.ActivityPub.Utils do ["https://www.w3.org/ns/activitystreams#Public"]} else if actor_type == :Group do - {[actor.followers_url, actor.members_url], []} + to = + (object["to"] || []) + |> MapSet.new() + |> MapSet.intersection(MapSet.new([actor.followers_url, actor.members_url])) + |> MapSet.to_list() + + {to, []} else {[actor.followers_url], []} end diff --git a/lib/federation/activity_stream/converter/post.ex b/lib/federation/activity_stream/converter/post.ex index 45c7ad5d..79e725d0 100644 --- a/lib/federation/activity_stream/converter/post.ex +++ b/lib/federation/activity_stream/converter/post.ex @@ -7,7 +7,7 @@ defmodule Mobilizon.Federation.ActivityStream.Converter.Post do """ alias Mobilizon.Actors.Actor alias Mobilizon.Federation.ActivityPub - alias Mobilizon.Federation.ActivityPub.Utils + alias Mobilizon.Federation.ActivityPub.{Audience, Utils} alias Mobilizon.Federation.ActivityStream.{Converter, Convertible} alias Mobilizon.Federation.ActivityStream.Converter.Media, as: MediaConverter alias Mobilizon.Posts.Post @@ -36,18 +36,15 @@ defmodule Mobilizon.Federation.ActivityStream.Converter.Post do def model_to_as( %Post{ author: %Actor{url: actor_url}, - attributed_to: %Actor{url: creator_url, followers_url: followers_url} + attributed_to: %Actor{ + url: creator_url + } } = post ) do - to = - if post.visibility == :public, - do: ["https://www.w3.org/ns/activitystreams#Public"], - else: [followers_url] + audience = Audience.calculate_to_and_cc_from_mentions(post) %{ "type" => "Article", - "to" => to, - "cc" => [], "actor" => actor_url, "id" => post.url, "name" => post.title, @@ -56,6 +53,7 @@ defmodule Mobilizon.Federation.ActivityStream.Converter.Post do "published" => (post.publish_at || post.inserted_at) |> to_date(), "attachment" => [] } + |> Map.merge(audience) |> maybe_add_post_picture(post) |> maybe_add_inline_media(post) end diff --git a/test/graphql/resolvers/post_test.exs b/test/graphql/resolvers/post_test.exs index e20e384b..968d7930 100644 --- a/test/graphql/resolvers/post_test.exs +++ b/test/graphql/resolvers/post_test.exs @@ -107,6 +107,8 @@ defmodule Mobilizon.GraphQL.Resolvers.PostTest do %Post{} = post_private = insert(:post, attributed_to: group, author: actor, visibility: :private) + insert(:follower, target_actor: group, approved: true) + {:ok, user: user, group: group, @@ -485,6 +487,32 @@ defmodule Mobilizon.GraphQL.Resolvers.PostTest do assert res["data"]["createPost"]["slug"] == "my-post-#{ShortUUID.encode!(id)}" end + test "create_post/3 creates a draft post for a group", %{ + conn: conn, + user: user, + group: group + } do + res = + conn + |> auth_conn(user) + |> AbsintheHelpers.graphql_query( + query: @create_post, + variables: %{ + title: @post_title, + body: "My new post is here", + attributedToId: group.id, + draft: true + } + ) + + assert is_nil(res["errors"]) + + assert res["data"]["createPost"]["title"] == @post_title + id = res["data"]["createPost"]["id"] + assert res["data"]["createPost"]["slug"] == "my-post-#{ShortUUID.encode!(id)}" + assert res["data"]["createPost"]["draft"] == true + end + test "create_post/3 doesn't create a post if no group is defined", %{ conn: conn, user: user From 6a23f03b70423ac1426f22d9877410b57955acd7 Mon Sep 17 00:00:00 2001 From: Thomas Citharel Date: Mon, 8 Mar 2021 15:58:25 +0100 Subject: [PATCH 2/3] Federate draft status Signed-off-by: Thomas Citharel --- .../activity_stream/converter/event.ex | 4 +- .../activity_stream/converter/post.ex | 7 +- .../transmogrifier/posts_test.exs | 92 +++++++++++++++++++ 3 files changed, 99 insertions(+), 4 deletions(-) create mode 100644 test/federation/activity_pub/transmogrifier/posts_test.exs diff --git a/lib/federation/activity_stream/converter/event.ex b/lib/federation/activity_stream/converter/event.ex index a45200b5..e1b8efa8 100644 --- a/lib/federation/activity_stream/converter/event.ex +++ b/lib/federation/activity_stream/converter/event.ex @@ -70,7 +70,7 @@ defmodule Mobilizon.Federation.ActivityStream.Converter.Event do status: object |> Map.get("ical:status", "CONFIRMED") |> String.downcase(), online_address: object |> Map.get("attachment", []) |> get_online_address(), phone_address: object["phoneAddress"], - draft: false, + draft: object["draft"] == true, url: object["id"], uuid: object["uuid"], tags: tags, @@ -119,7 +119,7 @@ defmodule Mobilizon.Federation.ActivityStream.Converter.Event do "commentsEnabled" => event.options.comment_moderation == :allow_all, "anonymousParticipationEnabled" => event.options.anonymous_participation, "attachment" => [], - # "draft" => event.draft, + "draft" => event.draft, "ical:status" => event.status |> to_string |> String.upcase(), "id" => event.url, "url" => event.url diff --git a/lib/federation/activity_stream/converter/post.ex b/lib/federation/activity_stream/converter/post.ex index 79e725d0..020fda4a 100644 --- a/lib/federation/activity_stream/converter/post.ex +++ b/lib/federation/activity_stream/converter/post.ex @@ -51,7 +51,8 @@ defmodule Mobilizon.Federation.ActivityStream.Converter.Post do "content" => post.body, "attributedTo" => creator_url, "published" => (post.publish_at || post.inserted_at) |> to_date(), - "attachment" => [] + "attachment" => [], + "draft" => post.draft } |> Map.merge(audience) |> maybe_add_post_picture(post) @@ -79,7 +80,8 @@ defmodule Mobilizon.Federation.ActivityStream.Converter.Post do local: false, publish_at: object["published"], picture_id: picture_id, - medias: medias + medias: medias, + draft: object["draft"] == true } else {:error, err} -> {:error, err} @@ -91,6 +93,7 @@ defmodule Mobilizon.Federation.ActivityStream.Converter.Post do defp get_actor(nil), do: {:error, "nil property found for actor data"} defp get_actor(actor), do: actor |> Utils.get_url() |> ActivityPub.get_or_fetch_actor_by_url() + defp to_date(nil), do: nil defp to_date(%DateTime{} = date), do: DateTime.to_iso8601(date) defp to_date(%NaiveDateTime{} = date), do: NaiveDateTime.to_iso8601(date) diff --git a/test/federation/activity_pub/transmogrifier/posts_test.exs b/test/federation/activity_pub/transmogrifier/posts_test.exs new file mode 100644 index 00000000..2ccbb4fe --- /dev/null +++ b/test/federation/activity_pub/transmogrifier/posts_test.exs @@ -0,0 +1,92 @@ +defmodule Mobilizon.Federation.ActivityPub.Transmogrifier.PostsTest do + use Mobilizon.DataCase + + import Mobilizon.Factory + import Mox + alias Mobilizon.Actors.Actor + alias Mobilizon.Federation.ActivityPub.{Activity, Transmogrifier} + alias Mobilizon.Federation.ActivityStream.Convertible + alias Mobilizon.Posts.Post + alias Mobilizon.Service.HTTP.ActivityPub.Mock + + describe "handle incoming posts" do + setup :verify_on_exit! + + test "it ignores an incoming post if we already have it" do + post = insert(:post) + post = Repo.preload(post, [:author, :attributed_to, :picture, :media]) + + activity = %{ + "type" => "Create", + "to" => ["https://www.w3.org/ns/activitystreams#Public"], + "actor" => post.author.url, + "attributedTo" => post.attributed_to.url, + "object" => Convertible.model_to_as(post) + } + + data = + File.read!("test/fixtures/mobilizon-post-activity-group.json") + |> Jason.decode!() + |> Map.merge(activity) + + assert {:ok, nil, _} = Transmogrifier.handle_incoming(data) + end + + test "it receives a draft post correctly as a member" do + %Actor{} = group = insert(:group, domain: "remote.tld", url: "https://remote.tld/@group") + %Actor{} = author = insert(:actor, domain: "remote.tld", url: "https://remote.tld/@author") + insert(:member, parent: group, actor: author, role: :moderator) + insert(:member, parent: group, role: :member) + + object = + Convertible.model_to_as(%Post{ + url: "https://remote.tld/@group/some-slug", + author: author, + attributed_to: group, + picture: nil, + media: [], + body: "my body", + title: "my title", + draft: true + }) + + data = + File.read!("test/fixtures/mobilizon-post-activity-group.json") + |> Jason.decode!() + |> Map.put("object", object) + + assert {:ok, %Activity{}, %Post{draft: true}} = Transmogrifier.handle_incoming(data) + end + + test "it publishes a previously draft post correctly as a member" do + %Actor{} = group = insert(:group, domain: "remote.tld", url: "https://remote.tld/@group") + %Actor{} = author = insert(:actor, domain: "remote.tld", url: "https://remote.tld/@author") + insert(:member, parent: group, actor: author, role: :moderator) + insert(:member, parent: group, role: :member) + + %Post{} = + post = + insert(:post, + url: "https://remote.tld/@group/some-slug", + author: author, + attributed_to: group, + draft: true + ) + + activity = %{ + "type" => "Update", + "to" => ["https://www.w3.org/ns/activitystreams#Public"], + "actor" => post.author.url, + "attributedTo" => post.attributed_to.url, + "object" => Convertible.model_to_as(%Post{post | draft: false}) + } + + data = + File.read!("test/fixtures/mobilizon-post-activity-group.json") + |> Jason.decode!() + |> Map.merge(activity) + + assert {:ok, %Activity{}, %Post{draft: false}} = Transmogrifier.handle_incoming(data) + end + end +end From 53c8349139e06379e5f97dab66e2c73c8e64757d Mon Sep 17 00:00:00 2001 From: Thomas Citharel Date: Mon, 8 Mar 2021 16:02:09 +0100 Subject: [PATCH 3/3] Fix activities test Signed-off-by: Thomas Citharel --- test/service/clean_old_activity_test.exs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/service/clean_old_activity_test.exs b/test/service/clean_old_activity_test.exs index 37c4c720..b9de8cfc 100644 --- a/test/service/clean_old_activity_test.exs +++ b/test/service/clean_old_activity_test.exs @@ -7,7 +7,7 @@ defmodule Mobilizon.Service.CleanOldActivityTest do alias Mobilizon.Service.CleanOldActivity @activity_inserted_at_1 DateTime.from_iso8601("2019-01-02T10:33:39.207493Z") |> elem(1) - @activity_inserted_at_2 DateTime.from_iso8601("2021-03-02T10:33:39.207493Z") |> elem(1) + @activity_inserted_at_2 DateTime.utc_now() setup do group1 = insert(:group)