From 4b864ba42365d3252b2d1ac4f5d16360d6f382da Mon Sep 17 00:00:00 2001 From: Thomas Citharel Date: Fri, 30 Jul 2021 17:25:45 +0200 Subject: [PATCH] Allow to use inline "Join" when processing an Accept Signed-off-by: Thomas Citharel --- lib/federation/activity_pub/transmogrifier.ex | 44 +++++++++++++++---- .../activity_pub/transmogrifier/join_test.exs | 40 +++++++++++++++++ 2 files changed, 75 insertions(+), 9 deletions(-) diff --git a/lib/federation/activity_pub/transmogrifier.ex b/lib/federation/activity_pub/transmogrifier.ex index a8b044371..95f535702 100644 --- a/lib/federation/activity_pub/transmogrifier.ex +++ b/lib/federation/activity_pub/transmogrifier.ex @@ -837,7 +837,7 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier do # Handle incoming `Accept` activities wrapping a `Join` activity on an event defp do_handle_incoming_accept_join(join_object, %Actor{} = actor_accepting) do - case get_participant(join_object) do + case get_participant(join_object, actor_accepting) do {:ok, participant} -> do_handle_incoming_accept_join_event(participant, actor_accepting) @@ -868,9 +868,9 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier do %Actor{} = actor_accepting ) when role in [:not_approved, :rejected] do - # TODO: The actor that accepts the Join activity may another one that the event organizer ? - # Or maybe for groups it's the group that sends the Accept activity - with {:same_actor, true} <- {:same_actor, actor_accepting.id == event.organizer_actor_id}, + with %Event{} = event <- Events.get_event_with_preload!(event.id), + {:can_accept_event_join, true} <- + {:can_accept_event_join, can_accept_event_join?(actor_accepting, event)}, {:ok, %Activity{} = activity, %Participant{role: :participant} = participant} <- ActivityPub.accept( :join, @@ -881,8 +881,8 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier do Participation.send_emails_to_local_user(participant) do {:ok, activity, participant} else - {:same_actor} -> - {:error, "Actor who accepted the join wasn't the event organizer. Quite odd."} + {:can_accept_event_join, false} -> + {:error, "Actor who accepted the join didn't have permission to do so."} {:ok, %Participant{role: :participant} = _follow} -> {:error, "Participant"} @@ -917,7 +917,7 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier do defp do_handle_incoming_reject_join(join_object, %Actor{} = actor_accepting) do with {:join_event, {:ok, %Participant{event: event, role: role} = participant}} when role != :rejected <- - {:join_event, get_participant(join_object)}, + {:join_event, get_participant(join_object, actor_accepting)}, # TODO: The actor that accepts the Join activity may another one that the event organizer ? # Or maybe for groups it's the group that sends the Accept activity {:same_actor, true} <- {:same_actor, actor_accepting.id == event.organizer_actor_id}, @@ -1025,14 +1025,22 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier do end end - defp get_participant(join_object) do + defp get_participant(join_object, %Actor{} = actor_accepting, loop \\ 1) do with join_object_id when not is_nil(join_object_id) <- Utils.get_url(join_object), {:not_found, %Participant{} = participant} <- {:not_found, Events.get_participant_by_url(join_object_id)} do {:ok, participant} else {:not_found, _err} -> - {:error, "Participant URL not found"} + with true <- is_map(join_object), + true <- loop < 2, + true <- Utils.are_same_origin?(actor_accepting.url, join_object["id"]), + {:ok, _activity, %Participant{url: participant_url}} <- handle_incoming(join_object) do + get_participant(participant_url, actor_accepting, 2) + else + _ -> + {:error, "Participant URL not found"} + end _ -> {:error, "ActivityPub ID not found in Accept Join object"} @@ -1129,4 +1137,22 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier do _ -> {:error, :remove_object_not_found} end end + + defp can_accept_event_join?( + %Actor{url: actor_url} = actor, + %Event{attributed_to: %Actor{type: :Group, url: group_url} = _group} = event + ) do + actor_url == group_url || Permission.can_update_group_object?(actor, event) + end + + defp can_accept_event_join?( + %Actor{id: actor_id}, + %Event{organizer_actor: %Actor{id: organizer_actor_id}} + ) do + organizer_actor_id == actor_id + end + + defp can_accept_event_join?(_actor, _event) do + false + end end diff --git a/test/federation/activity_pub/transmogrifier/join_test.exs b/test/federation/activity_pub/transmogrifier/join_test.exs index d26c4eb89..3948bf40e 100644 --- a/test/federation/activity_pub/transmogrifier/join_test.exs +++ b/test/federation/activity_pub/transmogrifier/join_test.exs @@ -64,6 +64,46 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier.JoinTest do # We don't accept already accepted Accept activities :error = Transmogrifier.handle_incoming(accept_data) end + + test "it accepts Accept activities with an inline Join from same origin" do + %Actor{} = organizer = insert(:actor) + + %Actor{url: participant_actor_url} = + insert(:actor, domain: "somewhere.else", url: "https://somewhere.else/@participant") + + %Actor{} = + group = insert(:group, domain: "somewhere.else", url: "https://somewhere.else/@group") + + insert(:member, actor: organizer, parent: group, role: :moderator) + + %Actor{} = + actor_member_2 = + insert(:actor, domain: "somewhere.else", url: "https://somewhere.else/@member") + + insert(:member, actor: actor_member_2, parent: group, role: :moderator) + + %Event{url: event_url} = + insert(:event, organizer_actor: organizer, join_options: :restricted, attributed_to: group) + + join_data = + File.read!("test/fixtures/mobilizon-join-activity.json") + |> Jason.decode!() + |> Map.put("actor", participant_actor_url) + |> Map.put("object", event_url) + |> Map.put("participationMessage", @join_message) + |> Map.put("id", "https://somewhere.else/@participant/join/event/1") + + accept_data = + File.read!("test/fixtures/mastodon-accept-activity.json") + |> Jason.decode!() + |> Map.put("actor", actor_member_2.url) + |> Map.put("object", join_data) + + {:ok, accept_activity, _} = Transmogrifier.handle_incoming(accept_data) + assert accept_activity.data["object"]["id"] == join_data["id"] + assert accept_activity.data["object"]["id"] =~ "/join/" + assert accept_activity.data["id"] =~ "/accept/join/" + end end describe "handle incoming reject join activities" do