Fix federation of admin actions (fixes #3980) (#3988)

* Fix federation of admin actions (fixes #3980)

* clippy

---------

Co-authored-by: Dessalines <tyhou13@gmx.com>
pull/3994/head
Nutomic 2023-09-26 03:39:18 +02:00 committed by GitHub
parent 9a9ece8fa4
commit 50589115e0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 27 additions and 30 deletions

View File

@ -329,22 +329,29 @@ test("Remove a post from admin and community on same instance", async () => {
throw "Missing beta community"; throw "Missing beta community";
} }
await followBeta(alpha); 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(); expect(postRes.post_view.post).toBeDefined();
// Get the id for beta // Get the id for beta
let betaPost = await waitForPost(beta, postRes.post_view.post); let betaPost = await waitForPost(beta, postRes.post_view.post);
expect(betaPost).toBeDefined(); 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) // The beta admin removes it (the community lives on beta)
let removePostRes = await removePost(beta, true, betaPost.post); let removePostRes = await removePost(beta, true, betaPost.post);
expect(removePostRes.post_view.post.removed).toBe(true); expect(removePostRes.post_view.post.removed).toBe(true);
// Make sure lemmy alpha sees post is removed // Make sure lemmy alpha sees post is removed
let alphaPost = await waitUntil( let alphaPost = await waitUntil(
() => getPost(alpha, postRes.post_view.post.id), () => getPost(alpha, alphaPost0.post.id),
p => p?.post_view.post.removed ?? false, 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); assertPostFederation(alphaPost.post_view, removePostRes.post_view);
// Undelete // Undelete

View File

@ -141,7 +141,7 @@ impl ActivityHandler for BlockUser {
} }
SiteOrCommunity::Community(community) => { SiteOrCommunity::Community(community) => {
verify_person_in_community(&self.actor, &community, context).await?; 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(()) Ok(())

View File

@ -119,7 +119,7 @@ impl ActivityHandler for CollectionAdd {
verify_is_public(&self.to, &self.cc)?; verify_is_public(&self.to, &self.cc)?;
let community = self.community(context).await?; let community = self.community(context).await?;
verify_person_in_community(&self.actor, &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(()) Ok(())
} }

View File

@ -114,7 +114,7 @@ impl ActivityHandler for CollectionRemove {
verify_is_public(&self.to, &self.cc)?; verify_is_public(&self.to, &self.cc)?;
let community = self.community(context).await?; let community = self.community(context).await?;
verify_person_in_community(&self.actor, &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(()) Ok(())
} }

View File

@ -52,7 +52,7 @@ impl ActivityHandler for LockPage {
let community = self.community(context).await?; let community = self.community(context).await?;
verify_person_in_community(&self.actor, &community, context).await?; verify_person_in_community(&self.actor, &community, context).await?;
check_community_deleted_or_removed(&community)?; 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(()) Ok(())
} }
@ -86,13 +86,7 @@ impl ActivityHandler for UndoLockPage {
let community = self.community(context).await?; let community = self.community(context).await?;
verify_person_in_community(&self.actor, &community, context).await?; verify_person_in_community(&self.actor, &community, context).await?;
check_community_deleted_or_removed(&community)?; check_community_deleted_or_removed(&community)?;
verify_mod_action( verify_mod_action(&self.actor, &community, context).await?;
&self.actor,
self.object.object.inner(),
community.id,
context,
)
.await?;
Ok(()) Ok(())
} }

View File

@ -76,7 +76,7 @@ impl ActivityHandler for UpdateCommunity {
verify_is_public(&self.to, &self.cc)?; verify_is_public(&self.to, &self.cc)?;
let community = self.community(context).await?; let community = self.community(context).await?;
verify_person_in_community(&self.actor, &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?; ApubCommunity::verify(&self.object, &community.actor_id.clone().into(), context).await?;
Ok(()) Ok(())
} }

View File

@ -127,7 +127,7 @@ impl ActivityHandler for CreateOrUpdatePage {
CreateOrUpdateType::Update => { CreateOrUpdateType::Update => {
let is_mod_action = self.object.is_mod_action(context).await?; let is_mod_action = self.object.is_mod_action(context).await?;
if is_mod_action { 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 { } else {
verify_domains_match(self.actor.inner(), self.object.id.inner())?; verify_domains_match(self.actor.inner(), self.object.id.inner())?;
verify_urls_match(self.actor.inner(), self.object.creator()?.inner())?; verify_urls_match(self.actor.inner(), self.object.creator()?.inner())?;

View File

@ -189,7 +189,7 @@ pub(in crate::activities) async fn verify_delete_activity(
verify_person_in_community(&activity.actor, &community, context).await?; verify_person_in_community(&activity.actor, &community, context).await?;
} }
// community deletion is always a mod (or admin) action // 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) => { DeletableObjects::Post(p) => {
verify_is_public(&activity.to, &[])?; verify_is_public(&activity.to, &[])?;
@ -231,7 +231,7 @@ async fn verify_delete_post_or_comment(
) -> Result<(), LemmyError> { ) -> Result<(), LemmyError> {
verify_person_in_community(actor, community, context).await?; verify_person_in_community(actor, community, context).await?;
if is_mod_action { if is_mod_action {
verify_mod_action(actor, object_id, community.id, context).await?; verify_mod_action(actor, community, context).await?;
} else { } else {
// domain of post ap_id and post.creator ap_id are identical, so we just check the former // 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)?; verify_domains_match(actor.inner(), object_id)?;

View File

@ -37,12 +37,9 @@ use lemmy_api_common::{
context::LemmyContext, context::LemmyContext,
send_activity::{ActivityChannel, SendActivityData}, send_activity::{ActivityChannel, SendActivityData},
}; };
use lemmy_db_schema::{ use lemmy_db_schema::source::{
newtypes::CommunityId, activity::{ActivitySendTargets, ActorType, SentActivity, SentActivityForm},
source::{ community::Community,
activity::{ActivitySendTargets, ActorType, SentActivity, SentActivityForm},
community::Community,
},
}; };
use lemmy_db_views_actor::structs::{CommunityPersonBanView, CommunityView}; use lemmy_db_views_actor::structs::{CommunityPersonBanView, CommunityView};
use lemmy_utils::error::{LemmyError, LemmyErrorExt, LemmyErrorType, LemmyResult}; use lemmy_utils::error::{LemmyError, LemmyErrorExt, LemmyErrorType, LemmyResult};
@ -113,22 +110,21 @@ pub(crate) async fn verify_person_in_community(
#[tracing::instrument(skip_all)] #[tracing::instrument(skip_all)]
pub(crate) async fn verify_mod_action( pub(crate) async fn verify_mod_action(
mod_id: &ObjectId<ApubPerson>, mod_id: &ObjectId<ApubPerson>,
object_id: &Url, community: &Community,
community_id: CommunityId,
context: &Data<LemmyContext>, context: &Data<LemmyContext>,
) -> Result<(), LemmyError> { ) -> Result<(), LemmyError> {
let mod_ = mod_id.dereference(context).await?; let mod_ = mod_id.dereference(context).await?;
let is_mod_or_admin = 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 { if is_mod_or_admin {
return Ok(()); 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. // by an instance admin.
// TODO: federate instance admin status and check it here // 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(()); return Ok(());
} }

@ -1 +1 @@
Subproject commit de9de2c53bee034d3824ecaa9a2104f8f341332e Subproject commit b122306e52d94807528068a7e8f8011c29d31db1