Receive and store remote site bans (fixes #3399) #5515

Nutomic · 2025-03-17T11:09:12Z

Continued from #4571

Todo:

New fields for instance_actions: received_ban and ban_expires
Scheduled task for ban expiration
Remove ban_nonlocal_user_from_local_communities(), send ban activity for remote users
Receive site bans from federated instances and write to instance_actions
Add modlog column mod_ban.instance_id
When receiving remote ban, only remove content in communities from that instance
Check for ban before posting to remote community
Remove person.banned column?

crates/db_schema/src/impls/post.rs

Nutomic · 2025-03-18T14:38:49Z

crates/apub/src/activities/block/block_user.rs

+            Some(site.instance_id),
+            true,
+          )
+          .await?;


Here is the main logic for receiving federated site bans.

crates/api/src/lib.rs

api_tests/src/post.spec.ts

dessalines · 2025-03-19T19:11:47Z

migrations/2025-03-17-110023_site_person_ban/up.sql

+    ADD COLUMN instance_id int NOT NULL REFERENCES instance ON UPDATE CASCADE ON DELETE CASCADE;
+
+-- insert existing bans into instance_actions table, assuming they were all banned from home instance
+INSERT INTO instance_actions (person_id, instance_id, received_ban, ban_expires)


We've never federated instance bans before this, so I think it's safer to only ban them from our local instance, not their home instance.

Bans are already federated if they were made on the user's home instance, eg:

https://lemmy.ml/u/[email protected]
https://lemmy.ml/modlog?page=1&actionType=All&userId=17706104

Should we also add rows for local instance bans also? Seems like we don't know where these bans actually took place then, so it might be safer to do both.

crates/api/src/local_user/ban_person.rs

dessalines · 2025-03-19T20:08:29Z

crates/apub/src/objects/private_message.rs

-    } else {
-      Ok(())
-    }
+    InstanceActions::check_ban(&mut context.pool(), person.id, person.instance_id).await?;


Should check both.

crates/db_schema/src/impls/instance.rs

crates/db_schema/src/impls/post.rs

crates/db_views/src/person/person_view.rs

Nutomic · 2025-03-25T11:12:32Z

crates/db_schema/src/impls/post.rs

@@ -160,6 +161,10 @@ impl Post {
      update = update.filter(post::community_id.eq(for_community_id));
    }

+    if let Some(for_instance_id) = for_instance_id {
+      update = update.filter(post::instance_id.eq(for_instance_id));
+    }


post.instance_id is the instance where the community is hosted, right? Otherwise this would be wrong. Not entirely clear to me where this gets set.

Yikes, it looks like instance_id isn't being set (and its default 0!).

I'll make an issue to remove that column, because it should rely on the community's instance_id anyway.

dessalines · 2025-03-25T13:06:39Z

crates/db_views/src/local_user/local_user_view.rs

  }

-  pub async fn read(pool: &mut DbPool<'_>, local_user_id: LocalUserId) -> Result<Self, Error> {
+  pub async fn read(pool: &mut DbPool<'_>, local_user_id: LocalUserId) -> LemmyResult<Self> {
+    let local_instance_id = SiteView::read_local(pool).await?.instance.id;


I'd much prefer all these read_locals were brought up a level. Instead make local_instance_id a function param.

Lets try to keep view and schema functions to a single DB query whenever possible, and keep multiple reads at the API / APUB level.

I dont like that idea because it makes all the callsites more verbose. These methods are called in many dozens of places so it would have to be passed everywhere, its much easier to read the instance id only a couple of times here. Anyway thanks to caching it doesnt perform a actual db read in most cases.

dessalines · 2025-03-25T13:16:33Z

crates/db_views/src/site/site_view.rs

-        .optional()?
-        .ok_or(LemmyErrorType::LocalSiteNotSetup)?,
-    )
+    static CACHE: CacheLock<SiteView> = LazyLock::new(build_cache);


Remove caching from LocalSite::read, and replace every case of it with this one. Possibly even remove that function or make it non-public.

See my comment here: #5515 (comment)

dessalines · 2025-03-25T13:18:50Z

crates/db_views/src/utils.rs

+#[diesel::dsl::auto_type]
+pub fn person_with_instance_actions(local_instance_id: InstanceId) -> _ {
+  person::table.inner_join(
+    instance_actions::table.on(
+      instance_actions::instance_id
+        .eq(local_instance_id)
+        .and(instance_actions::person_id.eq(person::id)),
+    ),


Rename this to local_instance_person_join, and remove line 179.

Then do person::table.left_join(local_instance_person_join(local_instance_id))

Then its more flexible.

Also I think it should be a left join.

Fixed to left join. This function is always used together with person::table so may as well make the calling code simpler by including it here.

I'd rather any filtering / common join statements be independent of the table, and only include the .on clause. Check out here for an example.

If I included the original search_combined table in all those, it would've been impossible.

Alright changed

Nutomic · 2025-03-25T14:09:46Z

api_tests/src/post.spec.ts

@@ -499,9 +500,9 @@ test("Enforce site ban federation for local user", async () => {
  // alpha ban should be federated to beta
  let alphaUserOnBeta1 = await waitUntil(
    () => resolvePerson(beta, alphaUserActorId!),
-    res => res.person?.person.banned ?? false,
+    res => res.person?.instance_actions?.received_ban != null,


This check is now failing, because PersonView only returns the ban for the local instance, but not the ban from user's home instance. Not sure what we should do here, return two instance_action fields for local instance and for user's home instance?

Yep, probably named local_instance_actions and home_instance_actions. This unfortunately is going to require aliasing similar to creator_community_actions.

We need to do the same for every view that joins to the instance table: one for the person's home instance, and another for our local instance.

Grepping for every case of instance_actions in db_views/structs should be enough.

Let me know if you want me to do this, as I went through this process recently.

Done. One disadvantage is that home_instance_actions is also included for local users, so there is the same data twice. Also need to update the js client to get tests passing.

dessalines

I'm realizing that the way I did instance_actions is a bit wrong, so I'll start working on a PR to correct it to this one.

In short, I'm currently only doing instance actions for my user. In reality, similar to creator_community_actions, we need it for the creator of the content. So we need a total of three:

instance_actions : for my own actions to the item's home
creator_local_instance_actions : for creator to our local instance
creator_home_instance_actions : for creator to item's home instance

dessalines · 2025-03-27T12:53:21Z

crates/db_schema/src/impls/local_site.rs

-    static CACHE: CacheLock<LocalSite> = LazyLock::new(build_cache);
-    Ok(
-      CACHE
-        .try_get_with((), async {
-          let conn = &mut get_conn(pool).await?;
-          local_site::table.first(conn).await
-        })
-        .await?,
-    )


Nutomic · 2025-03-28T10:55:09Z

Im not sure that is right. instance_actions only stores data for bans and user-instance blocks. Blocks are not federated so we only need those for the local instance id. For bans we care about these specific cases:

user banned from local instance
user banned from his home instance
user banned from a third instance, this only matters in communities belonging to that instance

I suppose the last one is what you are talking about, so we need to add community_instance_actions to post_view and comment_view. However its better to merge this pr first and make further changes directly to main branch. Otherwise there will be too many conflicts with other changes and constant rebasing.

dessalines · 2025-03-28T12:47:00Z

The main issue is that the current instance_actions are always using my user, as opposed to the item creator. But I agree we can merge this to main, since I'm already cleaning this up anyway.

this was originally removed in LemmyNet#5546, but LemmyNet#5515 introduced a new usage of it. due to the order of PR merges this slipped through and broke the build on main. fixes LemmyNet#5561

Nutomic added 2 commits March 17, 2025 12:08

Receive and store remote site bans (fixes #3399)

08a324c

db schema changes, handly expiration

a47e48d

Nutomic mentioned this pull request Mar 17, 2025

Add columns post.pending, comment.pending #5499

Merged

Nutomic added 3 commits March 17, 2025 15:39

partial federation changes

900eb1d

add column mod_ban.instance_id

aa8767c

remove unused

858621a

Nutomic force-pushed the site-person-ban branch from 82693f5 to 858621a Compare March 18, 2025 10:10

Nutomic added 4 commits March 18, 2025 12:45

wip: remove person.banned

71e1e19

fix down migration

f60f6fd

fmt, keep banned status

2ad4caf

fixes

11d69db

Nutomic force-pushed the site-person-ban branch from 06ca08c to 11d69db Compare March 18, 2025 14:00

Nutomic added 2 commits March 18, 2025 15:12

simplify

0eac34e

update post removed

2302931

Nutomic commented Mar 18, 2025

View reviewed changes

crates/db_schema/src/impls/post.rs Show resolved Hide resolved

Nutomic marked this pull request as ready for review March 18, 2025 14:27

Nutomic requested review from dessalines, phiresky, dullbananas and SleeplessOne1917 as code owners March 18, 2025 14:27

Merge branch 'main' into site-person-ban

d19200f

Nutomic commented Mar 18, 2025

View reviewed changes

Nutomic added 2 commits March 19, 2025 09:47

Merge branch 'main' into site-person-ban

49f7636

fix api tests

da732c0

Nutomic force-pushed the site-person-ban branch from 4716839 to da732c0 Compare March 19, 2025 10:55

dessalines reviewed Mar 19, 2025

View reviewed changes

Nutomic mentioned this pull request Mar 20, 2025

Remove api v3 routes. #5508 #5516

Open

Nutomic added 3 commits March 20, 2025 12:19

ban from local instance

44ed5f2

banned() helpers

cbde6c5

cleanup undo_block_user

34ef0cb

Nutomic added 7 commits March 20, 2025 12:38

remove order by

e65ee24

cache SiteView::read_local, add instance

e6690b0

use local instance id for PersonView

5879013

add helper function person_with_instance_actions

ac07ba7

use exists

00a6554

comments update removed

81503dc

remove method is_mod_or_admin

02e79c2

Nutomic commented Mar 25, 2025

View reviewed changes

dessalines reviewed Mar 25, 2025

View reviewed changes

Nutomic added 2 commits March 25, 2025 14:38

fix tests

a943060

no unwrap

d1473c2

Nutomic commented Mar 25, 2025

View reviewed changes

dessalines mentioned this pull request Mar 25, 2025

Remove rest of page limit #5429

Draft

Nutomic added 7 commits March 25, 2025 15:13

change local_instance_person_join

564c7da

remove LocalSite::read

afcdb56

wip: home_instance_actions

b805891

add home_instance_actions to all structs

d79a934

fixes

7acc461

fix tests

1e6e1b9

fmt

27f1eb9

dessalines reviewed Mar 27, 2025

View reviewed changes

Merge branch 'main' into site-person-ban

f538d83

Merge branch 'main' into site-person-ban

22131e2

dessalines merged commit 636811e into main Mar 28, 2025
0 of 2 checks passed

This was referenced Mar 29, 2025

[Bug]: main branch fails to build #5561

Open

fix removal of post.instance_id #5562

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Receive and store remote site bans (fixes #3399) #5515

Receive and store remote site bans (fixes #3399) #5515

Nutomic commented Mar 17, 2025 •

edited

Loading

Nutomic Mar 18, 2025

dessalines Mar 19, 2025

Nutomic Mar 20, 2025

dessalines Mar 25, 2025

dessalines Mar 19, 2025

Nutomic Mar 25, 2025

dessalines Mar 25, 2025

dessalines Mar 25, 2025

dessalines Mar 25, 2025

Nutomic Mar 25, 2025

dessalines Mar 25, 2025

Nutomic Mar 25, 2025

dessalines Mar 25, 2025 •

edited

Loading

Nutomic Mar 25, 2025

dessalines Mar 25, 2025

Nutomic Mar 25, 2025

Nutomic Mar 25, 2025

dessalines Mar 25, 2025

Nutomic Mar 26, 2025 •

edited

Loading

dessalines left a comment •

edited

Loading

dessalines Mar 27, 2025

Nutomic commented Mar 28, 2025 •

edited

Loading

dessalines commented Mar 28, 2025

Receive and store remote site bans (fixes #3399) #5515

Receive and store remote site bans (fixes #3399) #5515

Conversation

Nutomic commented Mar 17, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dessalines Mar 25, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Nutomic Mar 26, 2025 • edited Loading

Choose a reason for hiding this comment

dessalines left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Nutomic commented Mar 28, 2025 • edited Loading

dessalines commented Mar 28, 2025

Nutomic commented Mar 17, 2025 •

edited

Loading

dessalines Mar 25, 2025 •

edited

Loading

Nutomic Mar 26, 2025 •

edited

Loading

dessalines left a comment •

edited

Loading

Nutomic commented Mar 28, 2025 •

edited

Loading