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 |
|
Outdated, feel free to reopen. |
|
Maybe we should leave open, because this is pretty important, and also someone else might be able to pick it up, if they see that its still open. Otherwise it could get lost. Or maybe just mark as draft, and tag with help wanted. |
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)