From 0995043d040ed431c11f1785b4ebb0a9dfe355eb Mon Sep 17 00:00:00 2001 From: Thomas Citharel Date: Fri, 23 Jul 2021 09:42:50 +0200 Subject: [PATCH] Add the :role_needed_to_access permission check and refactor Signed-off-by: Thomas Citharel --- lib/federation/activity_pub/permission.ex | 102 ++++++++++++++++++ lib/federation/activity_pub/transmogrifier.ex | 12 +-- lib/federation/activity_pub/types/actors.ex | 1 + lib/federation/activity_pub/types/comments.ex | 1 + .../activity_pub/types/discussions.ex | 1 + lib/federation/activity_pub/types/entity.ex | 12 +++ lib/federation/activity_pub/types/events.ex | 2 + lib/federation/activity_pub/types/posts.ex | 2 + .../activity_pub/types/resources.ex | 1 + .../activity_pub/types/todo_lists.ex | 1 + lib/federation/activity_pub/types/todos.ex | 1 + .../activity_pub/types/tombstones.ex | 1 + lib/federation/activity_pub/utils.ex | 73 ------------- 13 files changed, 131 insertions(+), 79 deletions(-) create mode 100644 lib/federation/activity_pub/permission.ex diff --git a/lib/federation/activity_pub/permission.ex b/lib/federation/activity_pub/permission.ex new file mode 100644 index 000000000..e9d4377dc --- /dev/null +++ b/lib/federation/activity_pub/permission.ex @@ -0,0 +1,102 @@ +defmodule Mobilizon.Federation.ActivityPub.Permission do + @moduledoc """ + Module to check group members permissions on objects + """ + + alias Mobilizon.Actors + alias Mobilizon.Actors.Actor + alias Mobilizon.Federation.ActivityPub.Types.{Entity, Ownable} + require Logger + + @doc """ + Check that actor can access the object + """ + @spec can_access_group_object?(Actor.t(), Entity.t()) :: boolean() + def can_access_group_object?(%Actor{} = actor, object) do + can_manage_group_object?(:role_needed_to_access, actor, object) + end + + @doc """ + Check that actor can update the object + """ + @spec can_update_group_object?(Actor.t(), Entity.t()) :: boolean() + def can_update_group_object?(%Actor{} = actor, object) do + can_manage_group_object?(:role_needed_to_update, actor, object) + end + + @doc """ + Check that actor can delete the object + """ + @spec can_delete_group_object?(Actor.t(), Entity.t()) :: boolean() + def can_delete_group_object?(%Actor{} = actor, object) do + can_manage_group_object?(:role_needed_to_delete, actor, object) + end + + @spec can_manage_group_object?( + :role_needed_to_access | :role_needed_to_update | :role_needed_to_delete, + Actor.t(), + any() + ) :: boolean() + defp can_manage_group_object?(action_function, %Actor{url: actor_url} = actor, object) do + if Ownable.group_actor(object) != nil do + case apply(Ownable, action_function, [object]) do + role when role in [:member, :moderator, :administrator] -> + activity_actor_is_group_member?(actor, object, role) + + _ -> + case action_function do + :role_needed_to_access -> + Logger.warn("Actor #{actor_url} can't access #{object.url}") + + :role_needed_to_update -> + Logger.warn("Actor #{actor_url} can't update #{object.url}") + + :role_needed_to_delete -> + Logger.warn("Actor #{actor_url} can't delete #{object.url}") + end + + false + end + else + true + end + end + + @spec activity_actor_is_group_member?(Actor.t(), Entity.t(), atom()) :: boolean() + defp activity_actor_is_group_member?( + %Actor{id: actor_id, url: actor_url}, + object, + role + ) do + case Ownable.group_actor(object) do + %Actor{type: :Group, id: group_id, url: group_url} -> + Logger.debug("Group object url is #{group_url}") + + case role do + :moderator -> + Logger.debug( + "Checking if activity actor #{actor_url} is a moderator from group from #{object.url}" + ) + + Actors.is_moderator?(actor_id, group_id) + + :administrator -> + Logger.debug( + "Checking if activity actor #{actor_url} is an administrator from group from #{object.url}" + ) + + Actors.is_administrator?(actor_id, group_id) + + _ -> + Logger.debug( + "Checking if activity actor #{actor_url} is a member from group from #{object.url}" + ) + + Actors.is_member?(actor_id, group_id) + end + + _ -> + false + end + end +end diff --git a/lib/federation/activity_pub/transmogrifier.ex b/lib/federation/activity_pub/transmogrifier.ex index 172673e0c..27b6ff5a4 100644 --- a/lib/federation/activity_pub/transmogrifier.ex +++ b/lib/federation/activity_pub/transmogrifier.ex @@ -17,7 +17,7 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier do alias Mobilizon.Todos.{Todo, TodoList} alias Mobilizon.Federation.ActivityPub - alias Mobilizon.Federation.ActivityPub.{Activity, Relay, Utils} + alias Mobilizon.Federation.ActivityPub.{Activity, Permission, Relay, Utils} alias Mobilizon.Federation.ActivityPub.Actor, as: ActivityPubActor alias Mobilizon.Federation.ActivityPub.Types.Ownable alias Mobilizon.Federation.ActivityStream.{Converter, Convertible} @@ -409,7 +409,7 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier do {:origin_check, true} <- {:origin_check, Utils.origin_check?(actor_url, update_data) || - Utils.can_update_group_object?(actor, old_event)}, + Permission.can_update_group_object?(actor, old_event)}, {:ok, %Activity{} = activity, %Event{} = new_event} <- ActivityPub.update(old_event, object_data, false) do {:ok, activity, new_event} @@ -454,7 +454,7 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier do {:origin_check, true} <- {:origin_check, Utils.origin_check?(actor_url, update_data["object"]) || - Utils.can_update_group_object?(actor, old_post)}, + Permission.can_update_group_object?(actor, old_post)}, {:ok, %Activity{} = activity, %Post{} = new_post} <- ActivityPub.update(old_post, object_data, false) do {:ok, activity, new_post} @@ -482,7 +482,7 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier do {:origin_check, true} <- {:origin_check, Utils.origin_check?(actor_url, update_data) || - Utils.can_update_group_object?(actor, old_resource)}, + Permission.can_update_group_object?(actor, old_resource)}, {:ok, %Activity{} = activity, %Resource{} = new_resource} <- ActivityPub.update(old_resource, object_data, false) do {:ok, activity, new_resource} @@ -585,7 +585,7 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier do {:origin_check, true} <- {:origin_check, Utils.origin_check_from_id?(actor_url, object_id) || - Utils.can_delete_group_object?(actor, object)}, + Permission.can_delete_group_object?(actor, object)}, {:ok, activity, object} <- ActivityPub.delete(object, actor, false) do {:ok, activity, object} else @@ -629,7 +629,7 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier do {:origin_check, true} <- {:origin_check, Utils.origin_check?(actor_url, data) || - Utils.can_update_group_object?(actor, old_resource)}, + Permission.can_update_group_object?(actor, old_resource)}, {:ok, activity, new_resource} <- ActivityPub.move(:resource, old_resource, object_data) do {:ok, activity, new_resource} else diff --git a/lib/federation/activity_pub/types/actors.ex b/lib/federation/activity_pub/types/actors.ex index 067b66082..0d726771e 100644 --- a/lib/federation/activity_pub/types/actors.ex +++ b/lib/federation/activity_pub/types/actors.ex @@ -104,6 +104,7 @@ defmodule Mobilizon.Federation.ActivityPub.Types.Actors do def group_actor(%Actor{} = actor), do: actor + def role_needed_to_access(%Actor{} = _group), do: :member def role_needed_to_update(%Actor{} = _group), do: :administrator def role_needed_to_delete(%Actor{} = _group), do: :administrator diff --git a/lib/federation/activity_pub/types/comments.ex b/lib/federation/activity_pub/types/comments.ex index e7d1a3d3a..58f742f43 100644 --- a/lib/federation/activity_pub/types/comments.ex +++ b/lib/federation/activity_pub/types/comments.ex @@ -104,6 +104,7 @@ defmodule Mobilizon.Federation.ActivityPub.Types.Comments do def group_actor(_), do: nil + def role_needed_to_access(%Comment{}), do: :member def role_needed_to_update(%Comment{attributed_to: %Actor{} = _group}), do: :administrator def role_needed_to_delete(%Comment{attributed_to_id: _attributed_to_id}), do: :administrator diff --git a/lib/federation/activity_pub/types/discussions.ex b/lib/federation/activity_pub/types/discussions.ex index 3f06baed1..3afc58596 100644 --- a/lib/federation/activity_pub/types/discussions.ex +++ b/lib/federation/activity_pub/types/discussions.ex @@ -110,6 +110,7 @@ defmodule Mobilizon.Federation.ActivityPub.Types.Discussions do def group_actor(%Discussion{actor_id: actor_id}), do: Actors.get_actor(actor_id) + def role_needed_to_access(%Discussion{}), do: :member def role_needed_to_update(%Discussion{}), do: :moderator def role_needed_to_delete(%Discussion{}), do: :moderator diff --git a/lib/federation/activity_pub/types/entity.ex b/lib/federation/activity_pub/types/entity.ex index 5aa3253aa..2ba4831ab 100644 --- a/lib/federation/activity_pub/types/entity.ex +++ b/lib/federation/activity_pub/types/entity.ex @@ -67,6 +67,9 @@ defprotocol Mobilizon.Federation.ActivityPub.Types.Ownable do @doc "Returns the actor for the entity" def actor(entity) + @spec role_needed_to_access(Entity.t()) :: group_role() + def role_needed_to_access(entity) + @spec role_needed_to_update(Entity.t()) :: group_role() def role_needed_to_update(entity) @@ -82,6 +85,7 @@ end defimpl Ownable, for: Event do defdelegate group_actor(entity), to: Events defdelegate actor(entity), to: Events + defdelegate role_needed_to_access(entity), to: Events defdelegate role_needed_to_update(entity), to: Events defdelegate role_needed_to_delete(entity), to: Events end @@ -94,6 +98,7 @@ end defimpl Ownable, for: Comment do defdelegate group_actor(entity), to: Comments defdelegate actor(entity), to: Comments + defdelegate role_needed_to_access(entity), to: Comments defdelegate role_needed_to_update(entity), to: Comments defdelegate role_needed_to_delete(entity), to: Comments end @@ -106,6 +111,7 @@ end defimpl Ownable, for: Post do defdelegate group_actor(entity), to: Posts defdelegate actor(entity), to: Posts + defdelegate role_needed_to_access(entity), to: Posts defdelegate role_needed_to_update(entity), to: Posts defdelegate role_needed_to_delete(entity), to: Posts end @@ -118,6 +124,7 @@ end defimpl Ownable, for: Actor do defdelegate group_actor(entity), to: Actors defdelegate actor(entity), to: Actors + defdelegate role_needed_to_access(entity), to: Actors defdelegate role_needed_to_update(entity), to: Actors defdelegate role_needed_to_delete(entity), to: Actors end @@ -130,6 +137,7 @@ end defimpl Ownable, for: TodoList do defdelegate group_actor(entity), to: TodoLists defdelegate actor(entity), to: TodoLists + defdelegate role_needed_to_access(entity), to: TodoLists defdelegate role_needed_to_update(entity), to: TodoLists defdelegate role_needed_to_delete(entity), to: TodoLists end @@ -142,6 +150,7 @@ end defimpl Ownable, for: Todo do defdelegate group_actor(entity), to: Todos defdelegate actor(entity), to: Todos + defdelegate role_needed_to_access(entity), to: Todos defdelegate role_needed_to_update(entity), to: Todos defdelegate role_needed_to_delete(entity), to: Todos end @@ -154,6 +163,7 @@ end defimpl Ownable, for: Resource do defdelegate group_actor(entity), to: Resources defdelegate actor(entity), to: Resources + defdelegate role_needed_to_access(entity), to: Resources defdelegate role_needed_to_update(entity), to: Resources defdelegate role_needed_to_delete(entity), to: Resources end @@ -166,6 +176,7 @@ end defimpl Ownable, for: Discussion do defdelegate group_actor(entity), to: Discussions defdelegate actor(entity), to: Discussions + defdelegate role_needed_to_access(entity), to: Discussions defdelegate role_needed_to_update(entity), to: Discussions defdelegate role_needed_to_delete(entity), to: Discussions end @@ -173,6 +184,7 @@ end defimpl Ownable, for: Tombstone do defdelegate group_actor(entity), to: Tombstones defdelegate actor(entity), to: Tombstones + defdelegate role_needed_to_access(entity), to: Tombstones defdelegate role_needed_to_update(entity), to: Tombstones defdelegate role_needed_to_delete(entity), to: Tombstones end diff --git a/lib/federation/activity_pub/types/events.ex b/lib/federation/activity_pub/types/events.ex index 32e814296..4a334a944 100644 --- a/lib/federation/activity_pub/types/events.ex +++ b/lib/federation/activity_pub/types/events.ex @@ -95,6 +95,8 @@ defmodule Mobilizon.Federation.ActivityPub.Types.Events do def group_actor(_), do: nil + def role_needed_to_access(%Event{draft: false}), do: :member + def role_needed_to_access(%Event{}), do: :moderator def role_needed_to_update(%Event{attributed_to: %Actor{} = _group}), do: :moderator def role_needed_to_delete(%Event{attributed_to_id: _attributed_to_id}), do: :moderator def role_needed_to_delete(_), do: nil diff --git a/lib/federation/activity_pub/types/posts.ex b/lib/federation/activity_pub/types/posts.ex index 8421f70d9..a507d768d 100644 --- a/lib/federation/activity_pub/types/posts.ex +++ b/lib/federation/activity_pub/types/posts.ex @@ -91,6 +91,8 @@ defmodule Mobilizon.Federation.ActivityPub.Types.Posts do def group_actor(%Post{attributed_to_id: attributed_to_id}), do: Actors.get_actor(attributed_to_id) + def role_needed_to_access(%Post{draft: false}), do: :member + def role_needed_to_access(%Post{}), do: :moderator def role_needed_to_update(%Post{}), do: :moderator def role_needed_to_delete(%Post{}), do: :moderator end diff --git a/lib/federation/activity_pub/types/resources.ex b/lib/federation/activity_pub/types/resources.ex index 61157afc6..66b65e593 100644 --- a/lib/federation/activity_pub/types/resources.ex +++ b/lib/federation/activity_pub/types/resources.ex @@ -170,6 +170,7 @@ defmodule Mobilizon.Federation.ActivityPub.Types.Resources do def group_actor(%Resource{actor_id: actor_id}), do: Actors.get_actor(actor_id) + def role_needed_to_access(%Resource{}), do: :member def role_needed_to_update(%Resource{}), do: :member def role_needed_to_delete(%Resource{}), do: :member end diff --git a/lib/federation/activity_pub/types/todo_lists.ex b/lib/federation/activity_pub/types/todo_lists.ex index 1b9bc590b..58eb79993 100644 --- a/lib/federation/activity_pub/types/todo_lists.ex +++ b/lib/federation/activity_pub/types/todo_lists.ex @@ -68,6 +68,7 @@ defmodule Mobilizon.Federation.ActivityPub.Types.TodoLists do def group_actor(%TodoList{actor_id: actor_id}), do: Actors.get_actor(actor_id) + def role_needed_to_access(%TodoList{}), do: :member def role_needed_to_update(%TodoList{}), do: :member def role_needed_to_delete(%TodoList{}), do: :member end diff --git a/lib/federation/activity_pub/types/todos.ex b/lib/federation/activity_pub/types/todos.ex index dab23ca95..bae754783 100644 --- a/lib/federation/activity_pub/types/todos.ex +++ b/lib/federation/activity_pub/types/todos.ex @@ -80,6 +80,7 @@ defmodule Mobilizon.Federation.ActivityPub.Types.Todos do end end + def role_needed_to_access(%Todo{}), do: :member def role_needed_to_update(%Todo{}), do: :member def role_needed_to_delete(%Todo{}), do: :member end diff --git a/lib/federation/activity_pub/types/tombstones.ex b/lib/federation/activity_pub/types/tombstones.ex index 0787d47be..5586a0051 100644 --- a/lib/federation/activity_pub/types/tombstones.ex +++ b/lib/federation/activity_pub/types/tombstones.ex @@ -12,6 +12,7 @@ defmodule Mobilizon.Federation.ActivityPub.Types.Tombstones do def group_actor(_), do: nil + def role_needed_to_access(%Actor{}), do: nil def role_needed_to_update(%Actor{}), do: nil def role_needed_to_delete(%Actor{}), do: nil end diff --git a/lib/federation/activity_pub/utils.ex b/lib/federation/activity_pub/utils.ex index fcb1d3407..f4742adc5 100644 --- a/lib/federation/activity_pub/utils.ex +++ b/lib/federation/activity_pub/utils.ex @@ -15,7 +15,6 @@ defmodule Mobilizon.Federation.ActivityPub.Utils do alias Mobilizon.Federation.ActivityPub alias Mobilizon.Federation.ActivityPub.{Activity, Federator, Relay} alias Mobilizon.Federation.ActivityPub.Actor, as: ActivityPubActor - alias Mobilizon.Federation.ActivityPub.Types.Ownable alias Mobilizon.Federation.ActivityStream.Converter alias Mobilizon.Federation.HTTPSignatures alias Mobilizon.Web.Endpoint @@ -291,43 +290,6 @@ defmodule Mobilizon.Federation.ActivityPub.Utils do def origin_check_from_id?(id, %{"id" => other_id} = _params) when is_binary(other_id), do: origin_check_from_id?(id, other_id) - def activity_actor_is_group_member?( - %Actor{id: actor_id, url: actor_url}, - object, - role \\ :member - ) do - case Ownable.group_actor(object) do - %Actor{type: :Group, id: group_id, url: group_url} -> - Logger.debug("Group object url is #{group_url}") - - case role do - :moderator -> - Logger.debug( - "Checking if activity actor #{actor_url} is a moderator from group from #{object.url}" - ) - - Actors.is_moderator?(actor_id, group_id) - - :administrator -> - Logger.debug( - "Checking if activity actor #{actor_url} is an administrator from group from #{object.url}" - ) - - Actors.is_administrator?(actor_id, group_id) - - _ -> - Logger.debug( - "Checking if activity actor #{actor_url} is a member from group from #{object.url}" - ) - - Actors.is_member?(actor_id, group_id) - end - - _ -> - false - end - end - @doc """ Return AS Link data from @@ -662,41 +624,6 @@ defmodule Mobilizon.Federation.ActivityPub.Utils do :ok end - def can_update_group_object?(%Actor{} = actor, object) do - can_manage_group_object?(:role_needed_to_update, actor, object) - end - - def can_delete_group_object?(%Actor{} = actor, object) do - can_manage_group_object?(:role_needed_to_delete, actor, object) - end - - @spec can_manage_group_object?( - :role_needed_to_update | :role_needed_to_delete, - Actor.t(), - any() - ) :: boolean() - defp can_manage_group_object?(action_function, %Actor{url: actor_url} = actor, object) do - if Ownable.group_actor(object) != nil do - case apply(Ownable, action_function, [object]) do - role when role in [:member, :moderator, :administrator] -> - activity_actor_is_group_member?(actor, object, role) - - _ -> - case action_function do - :role_needed_to_update -> - Logger.warn("Actor #{actor_url} can't update #{object.url}") - - :role_needed_to_delete -> - Logger.warn("Actor #{actor_url} can't delete #{object.url}") - end - - false - end - else - true - end - end - @spec label_in_collection?(any(), any()) :: boolean() defp label_in_collection?(url, coll) when is_binary(coll), do: url == coll defp label_in_collection?(url, coll) when is_list(coll), do: url in coll