Receive & store remote site bans #4571
Conversation
| // home instance. We can continue to use community bans to federate data removal for | ||
| // backwards compatibility, initially but at some point, when most instances have been | ||
| // upgraded to have the logic of storing remote site bans, we can start doing data | ||
| // removal here & remove the logic of federating community bans on remote site bans. |
There was a problem hiding this comment.
I agree with this, I'd love to remove my weird logic for banning them from local communities.
I dont think it makes sense to have two different places to store site bans, that can easily cause confusion and bugs. You should remove the |
|
This might be out of scope for this PR if this is being strictly for site bans, but this is also a problem with other moderation actions. As an instance admin, I can issue a community ban for a remote user on a remote instance. I'm not sure if there are many cases where I would need this beyond dealing with some federation related issues like the recent kbin.social activity spam, so maybe it would make sense to remove a local admin's ability to issue a community ban if they're not also a community moderator? Once this is addressed on community level, posts and comments can still be removed locally without the admin being a community moderator. I'm not sure if there is an elegant solution besides adding a column like Let me know if I should move this to its own issue. |
| if let Some(site) = Site::read_from_instance_id(pool, instance_id).await? { | ||
| let is_banned_from_site_of_community = SitePersonBanView::get(pool, person.id, site.id).await?; | ||
| if is_banned_from_site_of_community { | ||
| Err(LemmyErrorType::BannedFromCommunity)? |
There was a problem hiding this comment.
Thoughts on adding a separate error here, like BannedFromSiteHostingCommunity or something?
| published TIMESTAMP WITH TIME ZONE DEFAULT NOW() NOT NULL, | ||
| expires TIMESTAMP WITH TIME ZONE, | ||
| PRIMARY KEY (person_id, site_id) | ||
| ); |
| pub struct SitePersonBanView { | ||
| pub site: Site, | ||
| pub person: Person, | ||
| } |
There was a problem hiding this comment.
The view might not be necessary, since we're not returning this view specifically, but only fields joined from site_person_ban
Same for CommunityPersonBanView, but that's up to you whether to remove it or not.
| @@ -84,3 +84,28 @@ pub struct SiteUpdateForm { | |||
| pub public_key: Option<String>, | |||
| pub content_warning: Option<Option<String>>, | |||
| } | |||
|
|
|||
| #[derive(PartialEq, Eq, Debug)] | |||
There was a problem hiding this comment.
Add #[skip_serializing_none]
|
Main change needed of course is that we need to remove the |
Currently, remote site bans are only received if they originate from the user's home instance. This has some downsides:
This PR adds a new
site_person_bantable, which stores data about all incoming federated bans. Additionally, this PR adds logic to prevent participating in communities if the user is banned on the host instance.TODO:
person.bannedcolumn for remote bans (that column is now exclusively reserved for local bans and should never be overwritten by federation)