Add the :role_needed_to_access permission check and refactor

Signed-off-by: Thomas Citharel <tcit@tcit.fr>

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

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

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
 

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
 

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
 

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

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

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

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

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

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

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

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