From 50589115e04a5d572627cfced97f41115df6e149 Mon Sep 17 00:00:00 2001 From: Nutomic Date: Tue, 26 Sep 2023 03:39:18 +0200 Subject: [PATCH] Fix federation of admin actions (fixes #3980) (#3988) * Fix federation of admin actions (fixes #3980) * clippy --------- Co-authored-by: Dessalines --- api_tests/src/post.spec.ts | 13 ++++++++++--- crates/apub/src/activities/block/block_user.rs | 2 +- .../src/activities/community/collection_add.rs | 2 +- .../activities/community/collection_remove.rs | 2 +- .../apub/src/activities/community/lock_page.rs | 10 ++-------- crates/apub/src/activities/community/update.rs | 2 +- .../src/activities/create_or_update/post.rs | 2 +- crates/apub/src/activities/deletion/mod.rs | 4 ++-- crates/apub/src/activities/mod.rs | 18 +++++++----------- crates/utils/translations | 2 +- 10 files changed, 27 insertions(+), 30 deletions(-) diff --git a/api_tests/src/post.spec.ts b/api_tests/src/post.spec.ts index ff792bb26..d87361875 100644 --- a/api_tests/src/post.spec.ts +++ b/api_tests/src/post.spec.ts @@ -329,22 +329,29 @@ test("Remove a post from admin and community on same instance", async () => { throw "Missing beta community"; } await followBeta(alpha); - let postRes = await createPost(alpha, betaCommunity.community.id); + let gammaCommunity = await resolveCommunity( + gamma, + betaCommunity.community.actor_id, + ); + let postRes = await createPost(gamma, gammaCommunity.community!.community.id); expect(postRes.post_view.post).toBeDefined(); // Get the id for beta let betaPost = await waitForPost(beta, postRes.post_view.post); expect(betaPost).toBeDefined(); + let alphaPost0 = await waitForPost(alpha, postRes.post_view.post); + expect(alphaPost0).toBeDefined(); + // The beta admin removes it (the community lives on beta) let removePostRes = await removePost(beta, true, betaPost.post); expect(removePostRes.post_view.post.removed).toBe(true); // Make sure lemmy alpha sees post is removed let alphaPost = await waitUntil( - () => getPost(alpha, postRes.post_view.post.id), + () => getPost(alpha, alphaPost0.post.id), p => p?.post_view.post.removed ?? false, ); - expect(alphaPost.post_view?.post.removed).toBe(true); + expect(alphaPost?.post_view.post.removed).toBe(true); assertPostFederation(alphaPost.post_view, removePostRes.post_view); // Undelete diff --git a/crates/apub/src/activities/block/block_user.rs b/crates/apub/src/activities/block/block_user.rs index a4fd7c9b8..2d1f760c2 100644 --- a/crates/apub/src/activities/block/block_user.rs +++ b/crates/apub/src/activities/block/block_user.rs @@ -141,7 +141,7 @@ impl ActivityHandler for BlockUser { } SiteOrCommunity::Community(community) => { verify_person_in_community(&self.actor, &community, context).await?; - verify_mod_action(&self.actor, self.object.inner(), community.id, context).await?; + verify_mod_action(&self.actor, &community, context).await?; } } Ok(()) diff --git a/crates/apub/src/activities/community/collection_add.rs b/crates/apub/src/activities/community/collection_add.rs index 49e84f593..ba962359e 100644 --- a/crates/apub/src/activities/community/collection_add.rs +++ b/crates/apub/src/activities/community/collection_add.rs @@ -119,7 +119,7 @@ impl ActivityHandler for CollectionAdd { verify_is_public(&self.to, &self.cc)?; let community = self.community(context).await?; verify_person_in_community(&self.actor, &community, context).await?; - verify_mod_action(&self.actor, &self.object, community.id, context).await?; + verify_mod_action(&self.actor, &community, context).await?; Ok(()) } diff --git a/crates/apub/src/activities/community/collection_remove.rs b/crates/apub/src/activities/community/collection_remove.rs index 72d58c114..c71e282bc 100644 --- a/crates/apub/src/activities/community/collection_remove.rs +++ b/crates/apub/src/activities/community/collection_remove.rs @@ -114,7 +114,7 @@ impl ActivityHandler for CollectionRemove { verify_is_public(&self.to, &self.cc)?; let community = self.community(context).await?; verify_person_in_community(&self.actor, &community, context).await?; - verify_mod_action(&self.actor, &self.object, community.id, context).await?; + verify_mod_action(&self.actor, &community, context).await?; Ok(()) } diff --git a/crates/apub/src/activities/community/lock_page.rs b/crates/apub/src/activities/community/lock_page.rs index b6caef9dc..634e5ab2f 100644 --- a/crates/apub/src/activities/community/lock_page.rs +++ b/crates/apub/src/activities/community/lock_page.rs @@ -52,7 +52,7 @@ impl ActivityHandler for LockPage { let community = self.community(context).await?; verify_person_in_community(&self.actor, &community, context).await?; check_community_deleted_or_removed(&community)?; - verify_mod_action(&self.actor, self.object.inner(), community.id, context).await?; + verify_mod_action(&self.actor, &community, context).await?; Ok(()) } @@ -86,13 +86,7 @@ impl ActivityHandler for UndoLockPage { let community = self.community(context).await?; verify_person_in_community(&self.actor, &community, context).await?; check_community_deleted_or_removed(&community)?; - verify_mod_action( - &self.actor, - self.object.object.inner(), - community.id, - context, - ) - .await?; + verify_mod_action(&self.actor, &community, context).await?; Ok(()) } diff --git a/crates/apub/src/activities/community/update.rs b/crates/apub/src/activities/community/update.rs index 6a4fcbfdd..11040f6b9 100644 --- a/crates/apub/src/activities/community/update.rs +++ b/crates/apub/src/activities/community/update.rs @@ -76,7 +76,7 @@ impl ActivityHandler for UpdateCommunity { verify_is_public(&self.to, &self.cc)?; let community = self.community(context).await?; verify_person_in_community(&self.actor, &community, context).await?; - verify_mod_action(&self.actor, self.object.id.inner(), community.id, context).await?; + verify_mod_action(&self.actor, &community, context).await?; ApubCommunity::verify(&self.object, &community.actor_id.clone().into(), context).await?; Ok(()) } diff --git a/crates/apub/src/activities/create_or_update/post.rs b/crates/apub/src/activities/create_or_update/post.rs index 018f84da4..5d58834e6 100644 --- a/crates/apub/src/activities/create_or_update/post.rs +++ b/crates/apub/src/activities/create_or_update/post.rs @@ -127,7 +127,7 @@ impl ActivityHandler for CreateOrUpdatePage { CreateOrUpdateType::Update => { let is_mod_action = self.object.is_mod_action(context).await?; if is_mod_action { - verify_mod_action(&self.actor, self.object.id.inner(), community.id, context).await?; + verify_mod_action(&self.actor, &community, context).await?; } else { verify_domains_match(self.actor.inner(), self.object.id.inner())?; verify_urls_match(self.actor.inner(), self.object.creator()?.inner())?; diff --git a/crates/apub/src/activities/deletion/mod.rs b/crates/apub/src/activities/deletion/mod.rs index 74112fb58..26cd6b1ab 100644 --- a/crates/apub/src/activities/deletion/mod.rs +++ b/crates/apub/src/activities/deletion/mod.rs @@ -189,7 +189,7 @@ pub(in crate::activities) async fn verify_delete_activity( verify_person_in_community(&activity.actor, &community, context).await?; } // community deletion is always a mod (or admin) action - verify_mod_action(&activity.actor, activity.object.id(), community.id, context).await?; + verify_mod_action(&activity.actor, &community, context).await?; } DeletableObjects::Post(p) => { verify_is_public(&activity.to, &[])?; @@ -231,7 +231,7 @@ async fn verify_delete_post_or_comment( ) -> Result<(), LemmyError> { verify_person_in_community(actor, community, context).await?; if is_mod_action { - verify_mod_action(actor, object_id, community.id, context).await?; + verify_mod_action(actor, community, context).await?; } else { // domain of post ap_id and post.creator ap_id are identical, so we just check the former verify_domains_match(actor.inner(), object_id)?; diff --git a/crates/apub/src/activities/mod.rs b/crates/apub/src/activities/mod.rs index 958065ffa..1cb266c21 100644 --- a/crates/apub/src/activities/mod.rs +++ b/crates/apub/src/activities/mod.rs @@ -37,12 +37,9 @@ use lemmy_api_common::{ context::LemmyContext, send_activity::{ActivityChannel, SendActivityData}, }; -use lemmy_db_schema::{ - newtypes::CommunityId, - source::{ - activity::{ActivitySendTargets, ActorType, SentActivity, SentActivityForm}, - community::Community, - }, +use lemmy_db_schema::source::{ + activity::{ActivitySendTargets, ActorType, SentActivity, SentActivityForm}, + community::Community, }; use lemmy_db_views_actor::structs::{CommunityPersonBanView, CommunityView}; use lemmy_utils::error::{LemmyError, LemmyErrorExt, LemmyErrorType, LemmyResult}; @@ -113,22 +110,21 @@ pub(crate) async fn verify_person_in_community( #[tracing::instrument(skip_all)] pub(crate) async fn verify_mod_action( mod_id: &ObjectId, - object_id: &Url, - community_id: CommunityId, + community: &Community, context: &Data, ) -> Result<(), LemmyError> { let mod_ = mod_id.dereference(context).await?; let is_mod_or_admin = - CommunityView::is_mod_or_admin(&mut context.pool(), mod_.id, community_id).await?; + CommunityView::is_mod_or_admin(&mut context.pool(), mod_.id, community.id).await?; if is_mod_or_admin { return Ok(()); } - // mod action comes from the same instance as the moderated object, so it was presumably done + // mod action comes from the same instance as the community, so it was presumably done // by an instance admin. // TODO: federate instance admin status and check it here - if mod_id.inner().domain() == object_id.domain() { + if mod_id.inner().domain() == community.actor_id.domain() { return Ok(()); } diff --git a/crates/utils/translations b/crates/utils/translations index de9de2c53..b122306e5 160000 --- a/crates/utils/translations +++ b/crates/utils/translations @@ -1 +1 @@ -Subproject commit de9de2c53bee034d3824ecaa9a2104f8f341332e +Subproject commit b122306e52d94807528068a7e8f8011c29d31db1