From 23dcb47ce5070d5ff10377451e6c0ce5d8bf9afd Mon Sep 17 00:00:00 2001 From: Thomas Citharel <tcit@tcit.fr> Date: Mon, 19 Oct 2020 19:21:39 +0200 Subject: [PATCH] Make sure only group moderators can update/delete events, posts Signed-off-by: Thomas Citharel <tcit@tcit.fr> --- js/src/views/Resources/ResourceFolder.vue | 100 +++++++--------- lib/federation/activity_pub/transmogrifier.ex | 47 ++++++-- lib/federation/activity_pub/types/actors.ex | 3 + lib/federation/activity_pub/types/comments.ex | 3 + .../activity_pub/types/discussions.ex | 3 + lib/federation/activity_pub/types/entity.ex | 26 +++++ lib/federation/activity_pub/types/events.ex | 4 + lib/federation/activity_pub/types/posts.ex | 7 +- .../activity_pub/types/resources.ex | 3 + .../activity_pub/types/todo_lists.ex | 3 + lib/federation/activity_pub/types/todos.ex | 3 + .../activity_pub/types/tombstones.ex | 3 + lib/federation/activity_pub/utils.ex | 77 +++++++++++-- lib/graphql/error.ex | 1 + lib/graphql/resolvers/post.ex | 4 +- lib/graphql/resolvers/resource.ex | 16 ++- .../transmogrifier/delete_test.exs | 109 +++++++++++++++++- .../transmogrifier/update_test.exs | 102 +++++++++++----- 18 files changed, 400 insertions(+), 114 deletions(-) diff --git a/js/src/views/Resources/ResourceFolder.vue b/js/src/views/Resources/ResourceFolder.vue index 1dcb85952..8def62dc9 100644 --- a/js/src/views/Resources/ResourceFolder.vue +++ b/js/src/views/Resources/ResourceFolder.vue @@ -114,6 +114,7 @@ :resource="localResource" :group="resource.actor" @delete="deleteResource" + @rename="handleRename" @move="handleMove" v-else /> @@ -143,7 +144,7 @@ <section class="modal-card-body"> <resource-selector :initialResource="updatedResource" - :username="resource.actor.preferredUsername" + :username="usernameWithDomain(resource.actor)" @updateResource="moveResource" @closeMoveModal="moveModal = false" /> @@ -200,6 +201,7 @@ import { Component, Mixins, Prop, Watch } from "vue-property-decorator"; import ResourceItem from "@/components/Resource/ResourceItem.vue"; import FolderItem from "@/components/Resource/FolderItem.vue"; import Draggable from "vuedraggable"; +import { RefetchQueryDescription } from "apollo-client/core/watchQueryOptions"; import { CURRENT_ACTOR_CLIENT } from "../../graphql/actor"; import { IActor, usernameWithDomain } from "../../types/actor"; import RouteName from "../../router/name"; @@ -232,6 +234,9 @@ import ResourceSelector from "../../components/Resource/ResourceSelector.vue"; username: this.$route.params.preferredUsername, }; }, + error({ graphQLErrors }) { + this.handleErrors(graphQLErrors); + }, }, config: CONFIG, currentActor: CURRENT_ACTOR_CLIENT, @@ -291,6 +296,13 @@ export default class Resources extends Mixins(ResourceMixin) { mapServiceTypeToIcon = mapServiceTypeToIcon; + get actualPath(): string { + const path = Array.isArray(this.$route.params.path) + ? this.$route.params.path.join("/") + : this.$route.params.path || this.path; + return path[0] !== "/" ? `/${path}` : path; + } + async createResource(): Promise<void> { if (!this.resource.actor) return; try { @@ -305,34 +317,7 @@ export default class Resources extends Mixins(ResourceMixin) { this.resource.id && this.resource.id.startsWith("root_") ? null : this.resource.id, type: this.newResource.type, }, - update: (store, { data: { createResource } }) => { - if (createResource == null) return; - if (!this.resource.actor) return; - const cachedData = store.readQuery<{ resource: IResource }>({ - query: GET_RESOURCE, - variables: { - path: this.resource.path, - username: this.resource.actor.preferredUsername, - }, - }); - if (cachedData == null) return; - const { resource } = cachedData; - if (resource == null) { - console.error("Cannot update resource cache, because of null value."); - return; - } - const newResource: IResource = createResource; - resource.children.elements = resource.children.elements.concat([newResource]); - - store.writeQuery({ - query: GET_RESOURCE, - variables: { - path: this.resource.path, - username: this.resource.actor.preferredUsername, - }, - data: { resource }, - }); - }, + refetchQueries: () => this.postRefreshQueries(), }); this.createLinkResourceModal = false; this.createResourceModal = false; @@ -429,6 +414,19 @@ export default class Resources extends Mixins(ResourceMixin) { }); } + // eslint-disable-next-line class-methods-use-this + private postRefreshQueries(): RefetchQueryDescription { + return [ + { + query: GET_RESOURCE, + variables: { + path: this.actualPath, + username: this.$route.params.preferredUsername, + }, + }, + ]; + } + async deleteResource(resourceID: string): Promise<void> { try { await this.$apollo.mutate({ @@ -436,37 +434,7 @@ export default class Resources extends Mixins(ResourceMixin) { variables: { id: resourceID, }, - update: (store, { data: { deleteResource } }) => { - if (deleteResource == null) return; - if (!this.resource.actor) return; - const cachedData = store.readQuery<{ resource: IResource }>({ - query: GET_RESOURCE, - variables: { - path: this.resource.path, - username: this.resource.actor.preferredUsername, - }, - }); - if (cachedData == null) return; - const { resource } = cachedData; - if (resource == null) { - console.error("Cannot update resource cache, because of null value."); - return; - } - const oldResource: IResource = deleteResource; - - resource.children.elements = resource.children.elements.filter( - (resourceElement) => resourceElement.id !== oldResource.id - ); - - store.writeQuery({ - query: GET_RESOURCE, - variables: { - path: this.resource.path, - username: this.resource.actor.preferredUsername, - }, - data: { resource }, - }); - }, + refetchQueries: () => this.postRefreshQueries(), }); this.validCheckedResources = this.validCheckedResources.filter((id) => id !== resourceID); delete this.checkedResources[resourceID]; @@ -476,6 +444,7 @@ export default class Resources extends Mixins(ResourceMixin) { } handleRename(resource: IResource): void { + console.log("handleRename"); this.renameModal = true; this.updatedResource = { ...resource }; } @@ -506,6 +475,7 @@ export default class Resources extends Mixins(ResourceMixin) { parentId: resource.parent ? resource.parent.id : null, path: resource.path, }, + refetchQueries: () => this.postRefreshQueries(), update: (store, { data }) => { if (!data || data.updateResource == null || parentPath == null) return; if (!this.resource.actor) return; @@ -577,9 +547,19 @@ export default class Resources extends Mixins(ResourceMixin) { console.error(e); } } + + handleErrors(errors: any[]): void { + if (errors.some((error) => error.status_code === 404)) { + this.$router.replace({ name: RouteName.PAGE_NOT_FOUND }); + } + } } </script> <style lang="scss" scoped> +.container.section { + background: $white; +} + nav.breadcrumb ul { align-items: center; diff --git a/lib/federation/activity_pub/transmogrifier.ex b/lib/federation/activity_pub/transmogrifier.ex index 544f9a515..09e4559ae 100644 --- a/lib/federation/activity_pub/transmogrifier.ex +++ b/lib/federation/activity_pub/transmogrifier.ex @@ -381,12 +381,15 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier do update_data ) do with actor <- Utils.get_actor(update_data), - {:ok, %Actor{url: actor_url, suspended: false}} <- + {:ok, %Actor{url: actor_url, suspended: false} = actor} <- ActivityPub.get_or_fetch_actor_by_url(actor), {:ok, %Event{} = old_event} <- object |> Utils.get_url() |> ActivityPub.fetch_object_from_url(), object_data <- Converter.Event.as_to_model_data(object), - {:origin_check, true} <- {:origin_check, Utils.origin_check?(actor_url, update_data)}, + {:origin_check, true} <- + {:origin_check, + Utils.origin_check?(actor_url, update_data) || + Utils.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} @@ -431,11 +434,15 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier do {:origin_check, true} <- {:origin_check, Utils.origin_check?(actor_url, update_data["object"]) || - Utils.activity_actor_is_group_member?(actor, old_post)}, + Utils.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} else + {:origin_check, _} -> + Logger.warn("Actor tried to update a post but doesn't has the required role") + :error + _e -> :error end @@ -452,7 +459,10 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier do {:ok, %Resource{} = old_resource} <- object |> Utils.get_url() |> ActivityPub.fetch_object_from_url(), object_data <- Converter.Resource.as_to_model_data(object), - {:origin_check, true} <- {:origin_check, Utils.origin_check?(actor_url, update_data)}, + {:origin_check, true} <- + {:origin_check, + Utils.origin_check?(actor_url, update_data) || + Utils.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} @@ -549,11 +559,11 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier do with actor_url <- Utils.get_actor(data), {:ok, %Actor{} = actor} <- ActivityPub.get_or_fetch_actor_by_url(actor_url), object_id <- Utils.get_url(object), - {:ok, object} <- can_delete_group_object(object_id), + {:ok, object} <- is_group_object_gone(object_id), {:origin_check, true} <- {:origin_check, Utils.origin_check_from_id?(actor_url, object_id) || - Utils.activity_actor_is_group_member?(actor, object)}, + Utils.can_delete_group_object?(actor, object)}, {:ok, activity, object} <- ActivityPub.delete(object, actor, false) do {:ok, activity, object} else @@ -567,6 +577,29 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier do end end + def handle_incoming( + %{"type" => "Move", "object" => %{"type" => type} = object, "actor" => _actor} = data + ) + when type in ["ResourceCollection", "Document"] do + with actor <- Utils.get_actor(data), + {:ok, %Actor{url: actor_url, suspended: false} = actor} <- + ActivityPub.get_or_fetch_actor_by_url(actor), + {:ok, %Resource{} = old_resource} <- + object |> Utils.get_url() |> ActivityPub.fetch_object_from_url(), + object_data <- Converter.Resource.as_to_model_data(object), + {:origin_check, true} <- + {:origin_check, + Utils.origin_check?(actor_url, data) || + Utils.can_update_group_object?(actor, old_resource)}, + {:ok, activity, new_resource} <- ActivityPub.move(:resource, old_resource, object_data) do + {:ok, activity, new_resource} + else + e -> + Logger.error(inspect(e)) + :error + end + end + def handle_incoming( %{ "type" => "Join", @@ -1020,7 +1053,7 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier do end end - defp can_delete_group_object(object_id) do + defp is_group_object_gone(object_id) do case ActivityPub.fetch_object_from_url(object_id, force: true) do {:error, error_message, object} when error_message in ["Gone", "Not found"] -> {:ok, object} diff --git a/lib/federation/activity_pub/types/actors.ex b/lib/federation/activity_pub/types/actors.ex index 77dfc8871..bc147b9d8 100644 --- a/lib/federation/activity_pub/types/actors.ex +++ b/lib/federation/activity_pub/types/actors.ex @@ -88,6 +88,9 @@ defmodule Mobilizon.Federation.ActivityPub.Types.Actors do def group_actor(%Actor{} = actor), do: actor + def role_needed_to_update(%Actor{} = _group), do: :administrator + def role_needed_to_delete(%Actor{} = _group), do: :administrator + defp prepare_args_for_actor(args) do args |> maybe_sanitize_username() diff --git a/lib/federation/activity_pub/types/comments.ex b/lib/federation/activity_pub/types/comments.ex index 1c1f04f6d..019a2958a 100644 --- a/lib/federation/activity_pub/types/comments.ex +++ b/lib/federation/activity_pub/types/comments.ex @@ -98,6 +98,9 @@ defmodule Mobilizon.Federation.ActivityPub.Types.Comments do def group_actor(_), do: nil + 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 + # Prepare and sanitize arguments for comments defp prepare_args_for_comment(args) do with in_reply_to_comment <- diff --git a/lib/federation/activity_pub/types/discussions.ex b/lib/federation/activity_pub/types/discussions.ex index 0d5248f0e..57eb77033 100644 --- a/lib/federation/activity_pub/types/discussions.ex +++ b/lib/federation/activity_pub/types/discussions.ex @@ -89,6 +89,9 @@ 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_update(%Discussion{}), do: :moderator + def role_needed_to_delete(%Discussion{}), do: :moderator + @spec maybe_publish_graphql_subscription(Discussion.t()) :: :ok defp maybe_publish_graphql_subscription(%Discussion{} = discussion) do Absinthe.Subscription.publish(Endpoint, discussion, diff --git a/lib/federation/activity_pub/types/entity.ex b/lib/federation/activity_pub/types/entity.ex index ffb3974c4..5aa3253aa 100644 --- a/lib/federation/activity_pub/types/entity.ex +++ b/lib/federation/activity_pub/types/entity.ex @@ -57,6 +57,8 @@ defprotocol Mobilizon.Federation.ActivityPub.Types.Managable do end defprotocol Mobilizon.Federation.ActivityPub.Types.Ownable do + @type group_role :: :member | :moderator | :administrator | nil + @spec group_actor(Entity.t()) :: Actor.t() | nil @doc "Returns an eventual group for the entity" def group_actor(entity) @@ -64,6 +66,12 @@ defprotocol Mobilizon.Federation.ActivityPub.Types.Ownable do @spec actor(Entity.t()) :: Actor.t() | nil @doc "Returns the actor for the entity" def actor(entity) + + @spec role_needed_to_update(Entity.t()) :: group_role() + def role_needed_to_update(entity) + + @spec role_needed_to_delete(Entity.t()) :: group_role() + def role_needed_to_delete(entity) end defimpl Managable, for: Event do @@ -74,6 +82,8 @@ end defimpl Ownable, for: Event do defdelegate group_actor(entity), to: Events defdelegate actor(entity), to: Events + defdelegate role_needed_to_update(entity), to: Events + defdelegate role_needed_to_delete(entity), to: Events end defimpl Managable, for: Comment do @@ -84,6 +94,8 @@ end defimpl Ownable, for: Comment do defdelegate group_actor(entity), to: Comments defdelegate actor(entity), to: Comments + defdelegate role_needed_to_update(entity), to: Comments + defdelegate role_needed_to_delete(entity), to: Comments end defimpl Managable, for: Post do @@ -94,6 +106,8 @@ end defimpl Ownable, for: Post do defdelegate group_actor(entity), to: Posts defdelegate actor(entity), to: Posts + defdelegate role_needed_to_update(entity), to: Posts + defdelegate role_needed_to_delete(entity), to: Posts end defimpl Managable, for: Actor do @@ -104,6 +118,8 @@ end defimpl Ownable, for: Actor do defdelegate group_actor(entity), to: Actors defdelegate actor(entity), to: Actors + defdelegate role_needed_to_update(entity), to: Actors + defdelegate role_needed_to_delete(entity), to: Actors end defimpl Managable, for: TodoList do @@ -114,6 +130,8 @@ end defimpl Ownable, for: TodoList do defdelegate group_actor(entity), to: TodoLists defdelegate actor(entity), to: TodoLists + defdelegate role_needed_to_update(entity), to: TodoLists + defdelegate role_needed_to_delete(entity), to: TodoLists end defimpl Managable, for: Todo do @@ -124,6 +142,8 @@ end defimpl Ownable, for: Todo do defdelegate group_actor(entity), to: Todos defdelegate actor(entity), to: Todos + defdelegate role_needed_to_update(entity), to: Todos + defdelegate role_needed_to_delete(entity), to: Todos end defimpl Managable, for: Resource do @@ -134,6 +154,8 @@ end defimpl Ownable, for: Resource do defdelegate group_actor(entity), to: Resources defdelegate actor(entity), to: Resources + defdelegate role_needed_to_update(entity), to: Resources + defdelegate role_needed_to_delete(entity), to: Resources end defimpl Managable, for: Discussion do @@ -144,11 +166,15 @@ end defimpl Ownable, for: Discussion do defdelegate group_actor(entity), to: Discussions defdelegate actor(entity), to: Discussions + defdelegate role_needed_to_update(entity), to: Discussions + defdelegate role_needed_to_delete(entity), to: Discussions end defimpl Ownable, for: Tombstone do defdelegate group_actor(entity), to: Tombstones defdelegate actor(entity), to: Tombstones + defdelegate role_needed_to_update(entity), to: Tombstones + defdelegate role_needed_to_delete(entity), to: Tombstones end defimpl Managable, for: Member do diff --git a/lib/federation/activity_pub/types/events.ex b/lib/federation/activity_pub/types/events.ex index febfd2a25..2cd0a9d5e 100644 --- a/lib/federation/activity_pub/types/events.ex +++ b/lib/federation/activity_pub/types/events.ex @@ -88,6 +88,10 @@ defmodule Mobilizon.Federation.ActivityPub.Types.Events do def group_actor(_), do: nil + 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 + def join(%Event{} = event, %Actor{} = actor, _local, additional) do with {:maximum_attendee_capacity, true} <- {:maximum_attendee_capacity, check_attendee_capacity(event)}, diff --git a/lib/federation/activity_pub/types/posts.ex b/lib/federation/activity_pub/types/posts.ex index 00921adaa..bddfaf475 100644 --- a/lib/federation/activity_pub/types/posts.ex +++ b/lib/federation/activity_pub/types/posts.ex @@ -44,7 +44,7 @@ defmodule Mobilizon.Federation.ActivityPub.Types.Posts do {:ok, %Post{attributed_to_id: group_id, author_id: creator_id} = post} <- Posts.update_post(post, args), {:ok, true} <- Cachex.del(:activity_pub, "post_#{post.slug}"), - {:ok, %Actor{} = group} <- Actors.get_group_by_actor_id(group_id), + {:ok, %Actor{url: group_url} = group} <- Actors.get_group_by_actor_id(group_id), %Actor{url: creator_url} = creator <- Actors.get_actor(creator_id), post_as_data <- Convertible.model_to_as(%{post | attributed_to: group, author: creator}), @@ -52,7 +52,7 @@ defmodule Mobilizon.Federation.ActivityPub.Types.Posts do "to" => [group.members_url], "cc" => [], "actor" => creator_url, - "attributedTo" => [creator_url] + "attributedTo" => [group_url] } do update_data = make_update_data(post_as_data, Map.merge(audience, additional)) @@ -95,4 +95,7 @@ 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_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 025b8af21..c57afcb86 100644 --- a/lib/federation/activity_pub/types/resources.ex +++ b/lib/federation/activity_pub/types/resources.ex @@ -155,4 +155,7 @@ defmodule Mobilizon.Federation.ActivityPub.Types.Resources do do: Actors.get_actor(creator_id) def group_actor(%Resource{actor_id: actor_id}), do: Actors.get_actor(actor_id) + + 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 b2b170f16..1b9bc590b 100644 --- a/lib/federation/activity_pub/types/todo_lists.ex +++ b/lib/federation/activity_pub/types/todo_lists.ex @@ -67,4 +67,7 @@ defmodule Mobilizon.Federation.ActivityPub.Types.TodoLists do def actor(%TodoList{}), do: nil def group_actor(%TodoList{actor_id: actor_id}), do: Actors.get_actor(actor_id) + + 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 50e573eb9..dab23ca95 100644 --- a/lib/federation/activity_pub/types/todos.ex +++ b/lib/federation/activity_pub/types/todos.ex @@ -79,4 +79,7 @@ defmodule Mobilizon.Federation.ActivityPub.Types.Todos do nil end end + + 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 b3f36cb6e..0787d47be 100644 --- a/lib/federation/activity_pub/types/tombstones.ex +++ b/lib/federation/activity_pub/types/tombstones.ex @@ -11,4 +11,7 @@ defmodule Mobilizon.Federation.ActivityPub.Types.Tombstones do def actor(_), do: nil def group_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 4abf339d8..6f9a64413 100644 --- a/lib/federation/activity_pub/utils.ex +++ b/lib/federation/activity_pub/utils.ex @@ -287,15 +287,41 @@ 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) do - Logger.debug( - "Checking if activity actor #{actor_url} is a member from group from #{object.url}" - ) - + 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} -> - Logger.debug("Group object ID is #{group_id}") - Actors.is_member?(actor_id, group_id) + %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 @@ -633,4 +659,39 @@ 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 end diff --git a/lib/graphql/error.ex b/lib/graphql/error.ex index 2e20a228b..6d5def169 100644 --- a/lib/graphql/error.ex +++ b/lib/graphql/error.ex @@ -88,6 +88,7 @@ defmodule Mobilizon.GraphQL.Error do defp metadata(:post_not_found), do: {404, dgettext("errors", "Post not found")} defp metadata(:event_not_found), do: {404, dgettext("errors", "Event not found")} defp metadata(:group_not_found), do: {404, dgettext("errors", "Group not found")} + defp metadata(:resource_not_found), do: {404, dgettext("errors", "Resource not found")} defp metadata(:unknown), do: {500, dgettext("errors", "Something went wrong")} defp metadata(code) do diff --git a/lib/graphql/resolvers/post.ex b/lib/graphql/resolvers/post.ex index 5f98a72e0..abd9666fc 100644 --- a/lib/graphql/resolvers/post.ex +++ b/lib/graphql/resolvers/post.ex @@ -149,7 +149,7 @@ defmodule Mobilizon.GraphQL.Resolvers.Post do } = _resolution ) do with {:uuid, {:ok, _uuid}} <- {:uuid, Ecto.UUID.cast(id)}, - %Actor{id: actor_id} <- Users.get_actor_for_user(user), + %Actor{id: actor_id, url: actor_url} <- Users.get_actor_for_user(user), {:post, %Post{attributed_to: %Actor{id: group_id} = group} = post} <- {:post, Posts.get_post_with_preloads(id)}, args <- @@ -158,7 +158,7 @@ defmodule Mobilizon.GraphQL.Resolvers.Post do end), {:member, true} <- {:member, Actors.is_member?(actor_id, group_id)}, {:ok, _, %Post{} = post} <- - ActivityPub.update(post, args, true, %{}) do + ActivityPub.update(post, args, true, %{"actor" => actor_url}) do {:ok, post} else {:uuid, :error} -> diff --git a/lib/graphql/resolvers/resource.ex b/lib/graphql/resolvers/resource.ex index 29011a526..d68439f58 100644 --- a/lib/graphql/resolvers/resource.ex +++ b/lib/graphql/resolvers/resource.ex @@ -83,8 +83,9 @@ defmodule Mobilizon.GraphQL.Resolvers.Resource do {:resource, Resources.get_resource_by_group_and_path_with_preloads(group_id, path)} do {:ok, resource} else + {:group, _} -> {:error, :group_not_found} {:member, false} -> {:error, dgettext("errors", "Profile is not member of group")} - {:resource, _} -> {:error, dgettext("errors", "No such resource")} + {:resource, _} -> {:error, :resource_not_found} end end @@ -137,12 +138,12 @@ defmodule Mobilizon.GraphQL.Resolvers.Resource do } } = _resolution ) do - with %Actor{id: actor_id} <- Users.get_actor_for_user(user), + with %Actor{id: actor_id, url: actor_url} <- Users.get_actor_for_user(user), {:resource, %Resource{actor_id: group_id} = resource} <- {:resource, Resources.get_resource_with_preloads(resource_id)}, {:member, true} <- {:member, Actors.is_member?(actor_id, group_id)}, {:ok, _, %Resource{} = resource} <- - ActivityPub.update(resource, args, true, %{}) do + ActivityPub.update(resource, args, true, %{"actor" => actor_url}) do {:ok, resource} else {:resource, _} -> @@ -195,8 +196,13 @@ defmodule Mobilizon.GraphQL.Resolvers.Resource do } } = _resolution ) do - with {:ok, data} when is_map(data) <- Parser.parse(resource_url) do - {:ok, struct(Metadata, data)} + case Parser.parse(resource_url) do + {:ok, data} when is_map(data) -> + {:ok, struct(Metadata, data)} + + {:error, _err} -> + Logger.warn("Error while fetching preview from #{inspect(resource_url)}") + {:error, :unknown_resource} end end diff --git a/test/federation/activity_pub/transmogrifier/delete_test.exs b/test/federation/activity_pub/transmogrifier/delete_test.exs index 96cab59d9..30717266a 100644 --- a/test/federation/activity_pub/transmogrifier/delete_test.exs +++ b/test/federation/activity_pub/transmogrifier/delete_test.exs @@ -6,11 +6,12 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier.DeleteTest do import ExUnit.CaptureLog import Mox - alias Mobilizon.{Actors, Discussions, Events, Posts} + alias Mobilizon.{Actors, Discussions, Events, Posts, Resources} alias Mobilizon.Actors.Actor alias Mobilizon.Discussions.Comment alias Mobilizon.Events.Event alias Mobilizon.Posts.Post + alias Mobilizon.Resources.Resource alias Mobilizon.Federation.ActivityPub.{Activity, Transmogrifier} alias Mobilizon.Federation.ActivityStream.Convertible alias Mobilizon.Service.HTTP.ActivityPub.Mock @@ -145,8 +146,9 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier.DeleteTest do end describe "handle incoming delete activities for group posts" do - test "works for remote deletions" do + test "works for remote deletions by moderators" do %Actor{url: remote_actor_url} = + remote_actor = insert(:actor, domain: "remote.domain", url: "https://remote.domain/@remote", @@ -154,6 +156,41 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier.DeleteTest do ) group = insert(:group) + insert(:member, actor: remote_actor, parent: group, role: :moderator) + %Post{} = post = insert(:post, attributed_to: group) + + data = Convertible.model_to_as(post) + refute is_nil(Posts.get_post_by_url(data["id"])) + + delete_data = + File.read!("test/fixtures/mastodon-delete.json") + |> Jason.decode!() + + object = + data + |> Map.put("type", "Article") + + delete_data = + delete_data + |> Map.put("actor", remote_actor_url) + |> Map.put("object", object) + + {:ok, _activity, _actor} = Transmogrifier.handle_incoming(delete_data) + + assert is_nil(Posts.get_post_by_url(data["id"])) + end + + test "doesn't work for remote deletions if the actor is just a group member" do + %Actor{url: remote_actor_url} = + remote_actor = + insert(:actor, + domain: "remote.domain", + url: "https://remote.domain/@remote", + preferred_username: "remote" + ) + + group = insert(:group) + insert(:member, actor: remote_actor, parent: group, role: :member) %Post{} = post = insert(:post, attributed_to: group) data = Convertible.model_to_as(post) @@ -209,4 +246,72 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier.DeleteTest do refute is_nil(Posts.get_post_by_url(data["id"])) end end + + describe "handle incoming delete activities for resources" do + test "works for remote deletions" do + %Actor{url: remote_actor_url} = + remote_actor = + insert(:actor, + domain: "remote.domain", + url: "http://remote.domain/@remote", + preferred_username: "remote" + ) + + group = insert(:group) + insert(:member, actor: remote_actor, parent: group, role: :member) + %Resource{} = resource = insert(:resource, actor: group) + + data = Convertible.model_to_as(resource) + refute is_nil(Resources.get_resource_by_url(data["id"])) + + delete_data = + File.read!("test/fixtures/mastodon-delete.json") + |> Jason.decode!() + + object = + data + |> Map.put("type", "Document") + + delete_data = + delete_data + |> Map.put("actor", remote_actor_url) + |> Map.put("object", object) + + {:ok, _activity, _actor} = Transmogrifier.handle_incoming(delete_data) + + assert is_nil(Resources.get_resource_by_url(data["id"])) + end + + test "doesn't work for remote deletions if the actor is not a group member" do + %Actor{url: remote_actor_url} = + insert(:actor, + domain: "remote.domain", + url: "http://remote.domain/@remote", + preferred_username: "remote" + ) + + group = insert(:group) + %Post{} = post = insert(:post, attributed_to: group) + + data = Convertible.model_to_as(post) + refute is_nil(Posts.get_post_by_url(data["id"])) + + delete_data = + File.read!("test/fixtures/mastodon-delete.json") + |> Jason.decode!() + + object = + data + |> Map.put("type", "Article") + + delete_data = + delete_data + |> Map.put("actor", remote_actor_url) + |> Map.put("object", object) + + :error = Transmogrifier.handle_incoming(delete_data) + + refute is_nil(Posts.get_post_by_url(data["id"])) + end + end end diff --git a/test/federation/activity_pub/transmogrifier/update_test.exs b/test/federation/activity_pub/transmogrifier/update_test.exs index 0971ffb8a..be9786855 100644 --- a/test/federation/activity_pub/transmogrifier/update_test.exs +++ b/test/federation/activity_pub/transmogrifier/update_test.exs @@ -5,7 +5,7 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier.UpdateTest do import Mobilizon.Factory alias Mobilizon.{Actors, Events, Posts} - alias Mobilizon.Actors.Actor + alias Mobilizon.Actors.{Actor, Member} alias Mobilizon.Events.Event alias Mobilizon.Posts.Post alias Mobilizon.Federation.ActivityPub.{Activity, Transmogrifier} @@ -85,11 +85,43 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier.UpdateTest do end end - test "it works for incoming update activities on group posts" do + # test "it works for incoming update activities which lock the account" do + # data = File.read!("test/fixtures/mastodon-post-activity.json") |> Jason.decode!() + + # {:ok, %Activity{data: data, local: false}} = Transmogrifier.handle_incoming(data) + # update_data = File.read!("test/fixtures/mastodon-update.json") |> Jason.decode!() + + # object = + # update_data["object"] + # |> Map.put("actor", data["actor"]) + # |> Map.put("id", data["actor"]) + # |> Map.put("manuallyApprovesFollowers", true) + + # update_data = + # update_data + # |> Map.put("actor", data["actor"]) + # |> Map.put("object", object) + + # {:ok, %Activity{data: data, local: false}} = Transmogrifier.handle_incoming(update_data) + + # user = User.get_cached_by_ap_id(data["actor"]) + # assert user.info["locked"] == true + # end + end + + describe "handle incoming updates activities for group posts" do + test "it works for incoming update activities on group posts when remote actor is a moderator" do use_cassette "activity_pub/group_post_update_activities" do - %Actor{url: remote_actor_url} = remote_actor = insert(:actor, domain: "remote.domain") + %Actor{url: remote_actor_url} = + remote_actor = + insert(:actor, + domain: "remote.domain", + url: "https://remote.domain/@remote", + preferred_username: "remote" + ) + group = insert(:group) - insert(:member, actor: remote_actor, parent: group) + %Member{} = member = insert(:member, actor: remote_actor, parent: group, role: :moderator) %Post{} = post = insert(:post, attributed_to: group) data = Convertible.model_to_as(post) @@ -99,7 +131,6 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier.UpdateTest do object = data - |> Map.put("actor", remote_actor_url) |> Map.put("name", "My updated post") |> Map.put("type", "Article") @@ -119,6 +150,44 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier.UpdateTest do end end + test "it works for incoming update activities on group posts" do + use_cassette "activity_pub/group_post_update_activities" do + %Actor{url: remote_actor_url} = + remote_actor = + insert(:actor, + domain: "remote.domain", + url: "https://remote.domain/@remote", + preferred_username: "remote" + ) + + group = insert(:group) + %Member{} = member = insert(:member, actor: remote_actor, parent: group) + %Post{} = post = insert(:post, attributed_to: group) + + data = Convertible.model_to_as(post) + refute is_nil(Posts.get_post_by_url(data["id"])) + + update_data = File.read!("test/fixtures/mastodon-update.json") |> Jason.decode!() + + object = + data + |> Map.put("name", "My updated post") + |> Map.put("type", "Article") + + update_data = + update_data + |> Map.put("actor", remote_actor_url) + |> Map.put("object", object) + + :error = Transmogrifier.handle_incoming(update_data) + + %Post{id: updated_post_id, title: updated_post_title} = Posts.get_post_by_url(data["id"]) + + assert updated_post_id == post.id + refute updated_post_title == "My updated post" + end + end + test "it fails for incoming update activities on group posts when the actor is not a member from the group" do use_cassette "activity_pub/group_post_update_activities" do %Actor{url: remote_actor_url} = @@ -154,28 +223,5 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier.UpdateTest do refute updated_post_title == "My updated post" end end - - # test "it works for incoming update activities which lock the account" do - # data = File.read!("test/fixtures/mastodon-post-activity.json") |> Jason.decode!() - - # {:ok, %Activity{data: data, local: false}} = Transmogrifier.handle_incoming(data) - # update_data = File.read!("test/fixtures/mastodon-update.json") |> Jason.decode!() - - # object = - # update_data["object"] - # |> Map.put("actor", data["actor"]) - # |> Map.put("id", data["actor"]) - # |> Map.put("manuallyApprovesFollowers", true) - - # update_data = - # update_data - # |> Map.put("actor", data["actor"]) - # |> Map.put("object", object) - - # {:ok, %Activity{data: data, local: false}} = Transmogrifier.handle_incoming(update_data) - - # user = User.get_cached_by_ap_id(data["actor"]) - # assert user.info["locked"] == true - # end end end