When site banning a federated user, also remove their content from our local communities. (#4464)

* When banning a federated user, also remove their content from our local
communities.

- This works by:
  - Before a site ban, find all posts and comments to local communities
  - Send a federated community ban action for each local comm.
  - This also removes their content in the apub receive code.
- Adding back in federated community ban api tests.
- Adding in two more api tests for site bans.
- Fixes #4118

* Add local community ban, and nonlocal person check.

* Ignoring errors.

* Move local check into function.

* Addressing PR comments 2

api_tests/package.json

@@ -19,16 +19,16 @@
     "api-test-image": "jest -i image.spec.ts"
   },
   "devDependencies": {
-    "@types/jest": "^29.5.11",
-    "@types/node": "^20.10.4",
-    "@typescript-eslint/eslint-plugin": "^6.14.0",
-    "@typescript-eslint/parser": "^6.14.0",
+    "@types/jest": "^29.5.12",
+    "@types/node": "^20.11.19",
+    "@typescript-eslint/eslint-plugin": "^7.0.2",
+    "@typescript-eslint/parser": "^7.0.2",
     "download-file-sync": "^1.0.4",
     "eslint": "^8.55.0",
     "eslint-plugin-prettier": "^5.0.1",
     "jest": "^29.5.0",
-    "lemmy-js-client": "0.19.3-alpha.2",
-    "prettier": "^3.1.1",
+    "lemmy-js-client": "0.19.4-alpha.4",
+    "prettier": "^3.2.5",
     "ts-jest": "^29.1.0",
     "typescript": "^5.3.3"
   }

api_tests/src/community.spec.ts

@@ -29,9 +29,7 @@ import {
   delta,
   betaAllowedInstances,
   searchPostLocal,
-  resolveBetaCommunity,
   longDelay,
-  delay,
   editCommunity,
 } from "./shared";
 import { EditCommunity, EditSite } from "lemmy-js-client";

api_tests/src/image.spec.ts

@@ -23,7 +23,6 @@ import {
   resolvePost,
   setupLogins,
   unfollowRemotes,
-  waitForPost,
 } from "./shared";
 const downloadFileSync = require("download-file-sync");
 

api_tests/src/post.spec.ts

@@ -23,7 +23,6 @@ import {
   unfollowRemotes,
   resolvePerson,
   banPersonFromSite,
-  searchPostLocal,
   followCommunity,
   banPersonFromCommunity,
   reportPost,
@@ -38,10 +37,9 @@ import {
   alphaUrl,
   loginUser,
   createCommunity,
-  getPosts,
 } from "./shared";
 import { PostView } from "lemmy-js-client/dist/types/PostView";
-import { EditSite, ListingType, ResolveObject } from "lemmy-js-client";
+import { EditSite, ResolveObject } from "lemmy-js-client";
 
 let betaCommunity: CommunityView | undefined;
 
@@ -239,12 +237,14 @@ test("Collection of featured posts gets federated", async () => {
     beta,
     community.community_view.community.actor_id,
   );
+  expect(betaCommunity).toBeDefined();
 
   const betaPost = await waitForPost(
     beta,
     post.post_view.post,
     post => post?.post.featured_community === true,
   );
+  expect(betaPost).toBeDefined();
 });
 
 test("Lock a post", async () => {
@@ -428,30 +428,34 @@ test("Search for a post", async () => {
   expect(betaPost?.post.name).toBeDefined();
 });
 
-test("Enforce site ban for federated user", async () => {
+test("Enforce site ban federation for local user", async () => {
   if (!betaCommunity) {
     throw "Missing beta community";
   }
+
   // create a test user
-  let alpha_user = await registerUser(alpha, alphaUrl);
-  let alphaUserPerson = (await getSite(alpha_user)).my_user?.local_user_view
+  let alphaUserHttp = await registerUser(alpha, alphaUrl);
+  let alphaUserPerson = (await getSite(alphaUserHttp)).my_user?.local_user_view
     .person;
   let alphaUserActorId = alphaUserPerson?.actor_id;
   if (!alphaUserActorId) {
     throw "Missing alpha user actor id";
   }
   expect(alphaUserActorId).toBeDefined();
-  let alphaPerson = (await resolvePerson(alpha_user, alphaUserActorId!)).person;
+  await followBeta(alphaUserHttp);
+
+  let alphaPerson = (await resolvePerson(alphaUserHttp, alphaUserActorId!))
+    .person;
   if (!alphaPerson) {
     throw "Missing alpha person";
   }
   expect(alphaPerson).toBeDefined();
 
   // alpha makes post in beta community, it federates to beta instance
-  let postRes1 = await createPost(alpha_user, betaCommunity.community.id);
+  let postRes1 = await createPost(alphaUserHttp, betaCommunity.community.id);
   let searchBeta1 = await waitForPost(beta, postRes1.post_view.post);
 
-  // ban alpha from its instance
+  // ban alpha from its own instance
   let banAlpha = await banPersonFromSite(
     alpha,
     alphaPerson.person.id,
@@ -468,10 +472,11 @@ test("Enforce site ban for federated user", async () => {
   expect(alphaUserOnBeta1.person?.person.banned).toBe(true);
 
   // existing alpha post should be removed on beta
-  await waitUntil(
+  let betaBanRes = await waitUntil(
     () => getPost(beta, searchBeta1.post.id),
     s => s.post_view.post.removed,
   );
+  expect(betaBanRes.post_view.post.removed).toBe(true);
 
   // Unban alpha
   let unBanAlpha = await banPersonFromSite(
@@ -487,21 +492,84 @@ test("Enforce site ban for federated user", async () => {
     throw "Missing alpha person";
   }
   let newAlphaUserJwt = await loginUser(alpha, alphaUserPerson.name);
-  alpha_user.setHeaders({
+  alphaUserHttp.setHeaders({
     Authorization: "Bearer " + newAlphaUserJwt.jwt ?? "",
   });
   // alpha makes new post in beta community, it federates
-  let postRes2 = await createPost(alpha_user, betaCommunity!.community.id);
+  let postRes2 = await createPost(alphaUserHttp, betaCommunity!.community.id);
   await waitForPost(beta, postRes2.post_view.post);
 
+  await unfollowRemotes(alpha);
+});
+
+test("Enforce site ban federation for federated user", async () => {
+  if (!betaCommunity) {
+    throw "Missing beta community";
+  }
+
+  // create a test user
+  let alphaUserHttp = await registerUser(alpha, alphaUrl);
+  let alphaUserPerson = (await getSite(alphaUserHttp)).my_user?.local_user_view
+    .person;
+  let alphaUserActorId = alphaUserPerson?.actor_id;
+  if (!alphaUserActorId) {
+    throw "Missing alpha user actor id";
+  }
+  expect(alphaUserActorId).toBeDefined();
+  await followBeta(alphaUserHttp);
+
   let alphaUserOnBeta2 = await resolvePerson(beta, alphaUserActorId!);
   expect(alphaUserOnBeta2.person?.person.banned).toBe(false);
+
+  if (!alphaUserOnBeta2.person) {
+    throw "Missing alpha person";
+  }
+
+  // alpha makes post in beta community, it federates to beta instance
+  let postRes1 = await createPost(alphaUserHttp, betaCommunity.community.id);
+  let searchBeta1 = await waitForPost(beta, postRes1.post_view.post);
+  expect(searchBeta1.post).toBeDefined();
+
+  // Now ban and remove their data from beta
+  let banAlphaOnBeta = await banPersonFromSite(
+    beta,
+    alphaUserOnBeta2.person.person.id,
+    true,
+    true,
+  );
+  expect(banAlphaOnBeta.banned).toBe(true);
+
+  // The beta site ban should NOT be federated to alpha
+  let alphaPerson2 = (await getSite(alphaUserHttp)).my_user!.local_user_view
+    .person;
+  expect(alphaPerson2.banned).toBe(false);
+
+  // existing alpha post should be removed on beta
+  let betaBanRes = await waitUntil(
+    () => getPost(beta, searchBeta1.post.id),
+    s => s.post_view.post.removed,
+  );
+  expect(betaBanRes.post_view.post.removed).toBe(true);
+
+  // existing alpha's post to the beta community should be removed on alpha
+  let alphaPostAfterRemoveOnBeta = await waitUntil(
+    () => getPost(alpha, postRes1.post_view.post.id),
+    s => s.post_view.post.removed,
+  );
+  expect(betaBanRes.post_view.post.removed).toBe(true);
+  expect(alphaPostAfterRemoveOnBeta.post_view.post.removed).toBe(true);
+  expect(
+    alphaPostAfterRemoveOnBeta.post_view.creator_banned_from_community,
+  ).toBe(true);
+
+  await unfollowRemotes(alpha);
 });
 
-test.skip("Enforce community ban for federated user", async () => {
+test("Enforce community ban for federated user", async () => {
   if (!betaCommunity) {
     throw "Missing beta community";
   }
+  await followBeta(alpha);
   let alphaShortname = `@lemmy_alpha@lemmy-alpha:8541`;
   let alphaPerson = (await resolvePerson(beta, alphaShortname)).person;
   if (!alphaPerson) {
@@ -511,38 +579,46 @@ test.skip("Enforce community ban for federated user", async () => {
 
   // make a post in beta, it goes through
   let postRes1 = await createPost(alpha, betaCommunity.community.id);
-  let searchBeta1 = await searchPostLocal(beta, postRes1.post_view.post);
-  expect(searchBeta1.posts[0]).toBeDefined();
+  let searchBeta1 = await waitForPost(beta, postRes1.post_view.post);
+  expect(searchBeta1.post).toBeDefined();
 
   // ban alpha from beta community
   let banAlpha = await banPersonFromCommunity(
     beta,
     alphaPerson.person.id,
-    2,
+    searchBeta1.community.id,
     true,
     true,
   );
   expect(banAlpha.banned).toBe(true);
 
   // ensure that the post by alpha got removed
-  await expect(getPost(alpha, searchBeta1.posts[0].post.id)).rejects.toBe(
-    Error("unknown"),
+  let removePostRes = await waitUntil(
+    () => getPost(alpha, postRes1.post_view.post.id),
+    s => s.post_view.post.removed,
   );
+  expect(removePostRes.post_view.post.removed).toBe(true);
+  expect(removePostRes.post_view.creator_banned_from_community).toBe(true);
+  expect(removePostRes.community_view.banned_from_community).toBe(true);
 
   // Alpha tries to make post on beta, but it fails because of ban
-  await expect(createPost(alpha, betaCommunity.community.id)).rejects.toBe(
-    Error("banned_from_community"),
-  );
+  await expect(
+    createPost(alpha, betaCommunity.community.id),
+  ).rejects.toStrictEqual(Error("banned_from_community"));
 
   // Unban alpha
   let unBanAlpha = await banPersonFromCommunity(
     beta,
     alphaPerson.person.id,
-    2,
+    searchBeta1.community.id,
     false,
     false,
   );
   expect(unBanAlpha.banned).toBe(false);
+
+  // Need to re-follow the community
+  await followBeta(alpha);
+
   let postRes3 = await createPost(alpha, betaCommunity.community.id);
   expect(postRes3.post_view.post).toBeDefined();
   expect(postRes3.post_view.community.local).toBe(false);
@@ -550,19 +626,25 @@ test.skip("Enforce community ban for federated user", async () => {
   expect(postRes3.post_view.counts.score).toBe(1);
 
   // Make sure that post makes it to beta community
-  let searchBeta2 = await searchPostLocal(beta, postRes3.post_view.post);
-  expect(searchBeta2.posts[0]).toBeDefined();
+  let postRes4 = await waitForPost(beta, postRes3.post_view.post);
+  expect(postRes4.post).toBeDefined();
+  expect(postRes4.creator_banned_from_community).toBe(false);
+
+  await unfollowRemotes(alpha);
 });
 
 test("A and G subscribe to B (center) A posts, it gets announced to G", async () => {
   if (!betaCommunity) {
     throw "Missing beta community";
   }
+  await followBeta(alpha);
+
   let postRes = await createPost(alpha, betaCommunity.community.id);
   expect(postRes.post_view.post).toBeDefined();
 
   let betaPost = (await resolvePost(gamma, postRes.post_view.post)).post;
   expect(betaPost?.post.name).toBeDefined();
+  await unfollowRemotes(alpha);
 });
 
 test("Report a post", async () => {
@@ -571,6 +653,7 @@ test("Report a post", async () => {
   if (!betaCommunity) {
     throw "Missing beta community";
   }
+  await followBeta(alpha);
   let postRes = await createPost(beta, betaCommunity.community.id);
   expect(postRes.post_view.post).toBeDefined();
 
@@ -598,9 +681,11 @@ test("Report a post", async () => {
   expect(betaReport.original_post_url).toBe(alphaReport.original_post_url);
   expect(betaReport.original_post_body).toBe(alphaReport.original_post_body);
   expect(betaReport.reason).toBe(alphaReport.reason);
+  await unfollowRemotes(alpha);
 });
 
 test("Fetch post via redirect", async () => {
+  await followBeta(alpha);
   let alphaPost = await createPost(alpha, betaCommunity!.community.id);
   expect(alphaPost.post_view.post).toBeDefined();
   // Make sure that post is liked on beta
@@ -621,4 +706,5 @@ test("Fetch post via redirect", async () => {
   let gammaPost = await gamma.resolveObject(form);
   expect(gammaPost).toBeDefined();
   expect(gammaPost.post?.post.ap_id).toBe(alphaPost.post_view.post.ap_id);
+  await unfollowRemotes(alpha);
 });

api_tests/src/shared.ts

@@ -394,7 +394,7 @@ export async function banPersonFromSite(
   let form: BanPerson = {
     person_id,
     ban,
-    remove_data: remove_data,
+    remove_data,
   };
   return api.banPerson(form);
 }

crates/api/src/lib.rs

@@ -1,13 +1,29 @@
+use activitypub_federation::config::Data;
 use actix_web::{http::header::Header, HttpRequest};
 use actix_web_httpauth::headers::authorization::{Authorization, Bearer};
 use base64::{engine::general_purpose::STANDARD_NO_PAD as base64, Engine};
 use captcha::Captcha;
 use lemmy_api_common::{
   claims::Claims,
+  community::BanFromCommunity,
   context::LemmyContext,
-  utils::{check_user_valid, local_site_to_slur_regex, AUTH_COOKIE_NAME},
+  send_activity::{ActivityChannel, SendActivityData},
+  utils::{check_expire_time, check_user_valid, local_site_to_slur_regex, AUTH_COOKIE_NAME},
+};
+use lemmy_db_schema::{
+  source::{
+    community::{
+      CommunityFollower,
+      CommunityFollowerForm,
+      CommunityPersonBan,
+      CommunityPersonBanForm,
+    },
+    local_site::LocalSite,
+    moderator::{ModBanFromCommunity, ModBanFromCommunityForm},
+    person::Person,
+  },
+  traits::{Bannable, Crud, Followable},
 };
-use lemmy_db_schema::source::local_site::LocalSite;
 use lemmy_db_views::structs::LocalUserView;
 use lemmy_utils::{
   error::{LemmyError, LemmyErrorExt, LemmyErrorExt2, LemmyErrorType, LemmyResult},
@@ -141,6 +157,97 @@ pub(crate) fn build_totp_2fa(
   .with_lemmy_type(LemmyErrorType::CouldntGenerateTotp)
 }
 
+/// Site bans are only federated for local users.
+/// This is a problem, because site-banning non-local users will still leave content
+/// they've posted to our local communities, on other servers.
+///
+/// So when doing a site ban for a non-local user, you need to federate/send a
+/// community ban for every local community they've participated in.
+/// See https://github.com/LemmyNet/lemmy/issues/4118
+#[tracing::instrument(skip_all)]
+pub(crate) async fn ban_nonlocal_user_from_local_communities(
+  local_user_view: &LocalUserView,
+  target: &Person,
+  ban: bool,
+  reason: &Option<String>,
+  remove_data: &Option<bool>,
+  expires: &Option<i64>,
+  context: &Data<LemmyContext>,
+) -> LemmyResult<()> {
+  // Only run this code for federated users
+  if !target.local {
+    let ids = Person::list_local_community_ids(&mut context.pool(), target.id).await?;
+
+    for community_id in ids {
+      let expires_dt = check_expire_time(*expires)?;
+
+      // Ban / unban them from our local communities
+      let community_user_ban_form = CommunityPersonBanForm {
+        community_id,
+        person_id: target.id,
+        expires: Some(expires_dt),
+      };
+
+      if ban {
+        // Ignore all errors for these
+        CommunityPersonBan::ban(&mut context.pool(), &community_user_ban_form)
+          .await
+          .ok();
+
+        // Also unsubscribe them from the community, if they are subscribed
+        let community_follower_form = CommunityFollowerForm {
+          community_id,
+          person_id: target.id,
+          pending: false,
+        };
+
+        CommunityFollower::unfollow(&mut context.pool(), &community_follower_form)
+          .await
+          .ok();
+      } else {
+        CommunityPersonBan::unban(&mut context.pool(), &community_user_ban_form)
+          .await
+          .ok();
+      }
+
+      // Mod tables
+      let form = ModBanFromCommunityForm {
+        mod_person_id: local_user_view.person.id,
+        other_person_id: target.id,
+        community_id,
+        reason: reason.clone(),
+        banned: Some(ban),
+        expires: expires_dt,
+      };
+
+      ModBanFromCommunity::create(&mut context.pool(), &form).await?;
+
+      // Federate the ban from community
+      let ban_from_community = BanFromCommunity {
+        community_id,
+        person_id: target.id,
+        ban,
+        reason: reason.clone(),
+        remove_data: *remove_data,
+        expires: *expires,
+      };
+
+      ActivityChannel::submit_activity(
+        SendActivityData::BanFromCommunity {
+          moderator: local_user_view.person.clone(),
+          community_id,
+          target: target.clone(),
+          data: ban_from_community,
+        },
+        context,
+      )
+      .await?;
+    }
+  }
+
+  Ok(())
+}
+
 #[tracing::instrument(skip_all)]
 pub async fn local_user_view_from_jwt(
   jwt: &str,

crates/api/src/local_user/ban_person.rs

@@ -1,3 +1,4 @@
+use crate::ban_nonlocal_user_from_local_communities;
 use activitypub_federation::config::Data;
 use actix_web::web::Json;
 use lemmy_api_common::{
@@ -47,7 +48,7 @@ pub async fn ban_from_site(
   .with_lemmy_type(LemmyErrorType::CouldntUpdateUser)?;
 
   // if its a local user, invalidate logins
-  let local_user = LocalUserView::read_person(&mut context.pool(), data.person_id).await;
+  let local_user = LocalUserView::read_person(&mut context.pool(), person.id).await;
   if let Ok(local_user) = local_user {
     LoginToken::invalidate_all(&mut context.pool(), local_user.local_user.id).await?;
   }
@@ -61,7 +62,7 @@ pub async fn ban_from_site(
   // Mod tables
   let form = ModBanForm {
     mod_person_id: local_user_view.person.id,
-    other_person_id: data.person_id,
+    other_person_id: person.id,
     reason: data.reason.clone(),
     banned: Some(data.ban),
     expires,
@@ -69,7 +70,18 @@ pub async fn ban_from_site(
 
   ModBan::create(&mut context.pool(), &form).await?;
 
-  let person_view = PersonView::read(&mut context.pool(), data.person_id).await?;
+  let person_view = PersonView::read(&mut context.pool(), person.id).await?;
+
+  ban_nonlocal_user_from_local_communities(
+    &local_user_view,
+    &person,
+    data.ban,
+    &data.reason,
+    &data.remove_data,
+    &data.expires,
+    &context,
+  )
+  .await?;
 
   ActivityChannel::submit_activity(
     SendActivityData::BanFromSite {

crates/db_schema/src/impls/comment.rs

@@ -1,6 +1,6 @@
 use crate::{
   newtypes::{CommentId, DbUrl, PersonId},
-  schema::comment::dsl::{ap_id, comment, content, creator_id, deleted, path, removed, updated},
+  schema::comment,
   source::comment::{
     Comment,
     CommentInsertForm,
@@ -30,11 +30,11 @@ impl Comment {
   ) -> Result<Vec<Self>, Error> {
     let conn = &mut get_conn(pool).await?;
 
-    diesel::update(comment.filter(creator_id.eq(for_creator_id)))
+    diesel::update(comment::table.filter(comment::creator_id.eq(for_creator_id)))
       .set((
-        content.eq(DELETED_REPLACEMENT_TEXT),
-        deleted.eq(true),
-        updated.eq(naive_now()),
+        comment::content.eq(DELETED_REPLACEMENT_TEXT),
+        comment::deleted.eq(true),
+        comment::updated.eq(naive_now()),
       ))
       .get_results::<Self>(conn)
       .await
@@ -46,8 +46,11 @@ impl Comment {
     new_removed: bool,
   ) -> Result<Vec<Self>, Error> {
     let conn = &mut get_conn(pool).await?;
-    diesel::update(comment.filter(creator_id.eq(for_creator_id)))
-      .set((removed.eq(new_removed), updated.eq(naive_now())))
+    diesel::update(comment::table.filter(comment::creator_id.eq(for_creator_id)))
+      .set((
+        comment::removed.eq(new_removed),
+        comment::updated.eq(naive_now()),
+      ))
       .get_results::<Self>(conn)
       .await
   }
@@ -64,9 +67,9 @@ impl Comment {
       .run(|conn| {
         Box::pin(async move {
           // Insert, to get the id
-          let inserted_comment = insert_into(comment)
+          let inserted_comment = insert_into(comment::table)
             .values(comment_form)
-            .on_conflict(ap_id)
+            .on_conflict(comment::ap_id)
             .do_update()
             .set(comment_form)
             .get_result::<Self>(conn)
@@ -84,8 +87,8 @@ impl Comment {
             format!("{}.{}", 0, comment_id)
           });
 
-          let updated_comment = diesel::update(comment.find(comment_id))
-            .set(path.eq(ltree))
+          let updated_comment = diesel::update(comment::table.find(comment_id))
+            .set(comment::path.eq(ltree))
             .get_result::<Self>(conn)
             .await?;
 
@@ -133,8 +136,8 @@ where ca.comment_id = c.id"
     let conn = &mut get_conn(pool).await?;
     let object_id: DbUrl = object_id.into();
     Ok(
-      comment
-        .filter(ap_id.eq(object_id))
+      comment::table
+        .filter(comment::ap_id.eq(object_id))
         .first::<Comment>(conn)
         .await
         .ok()
@@ -171,7 +174,7 @@ impl Crud for Comment {
     comment_form: &Self::UpdateForm,
   ) -> Result<Self, Error> {
     let conn = &mut get_conn(pool).await?;
-    diesel::update(comment.find(comment_id))
+    diesel::update(comment::table.find(comment_id))
       .set(comment_form)
       .get_result::<Self>(conn)
       .await

crates/db_schema/src/impls/person.rs

@@ -1,6 +1,6 @@
 use crate::{
   newtypes::{CommunityId, DbUrl, InstanceId, PersonId},
-  schema::{instance, local_user, person, person_follower},
+  schema::{comment, community, instance, local_user, person, person_follower, post},
   source::person::{
     Person,
     PersonFollower,
@@ -11,7 +11,7 @@ use crate::{
   traits::{ApubActor, Crud, Followable},
   utils::{functions::lower, get_conn, naive_now, DbPool},
 };
-use diesel::{dsl::insert_into, result::Error, ExpressionMethods, JoinOnDsl, QueryDsl};
+use diesel::{dsl::insert_into, result::Error, CombineDsl, ExpressionMethods, JoinOnDsl, QueryDsl};
 use diesel_async::RunQueryDsl;
 
 #[async_trait]
@@ -84,6 +84,29 @@ impl Person {
       .get_result::<Self>(conn)
       .await
   }
+
+  /// Lists local community ids for all posts and comments for a given creator.
+  pub async fn list_local_community_ids(
+    pool: &mut DbPool<'_>,
+    for_creator_id: PersonId,
+  ) -> Result<Vec<CommunityId>, Error> {
+    let conn = &mut get_conn(pool).await?;
+    comment::table
+      .inner_join(post::table)
+      .inner_join(community::table.on(post::community_id.eq(community::id)))
+      .filter(community::local.eq(true))
+      .filter(comment::creator_id.eq(for_creator_id))
+      .select(community::id)
+      .union(
+        post::table
+          .inner_join(community::table)
+          .filter(community::local.eq(true))
+          .filter(post::creator_id.eq(for_creator_id))
+          .select(community::id),
+      )
+      .load::<CommunityId>(conn)
+      .await
+  }
 }
 
 impl PersonInsertForm {