Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding local site settings to reject federated upvotes or downvotes. #5038

Merged
merged 8 commits into from
Oct 2, 2024
5 changes: 2 additions & 3 deletions crates/api/src/comment/like.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use lemmy_api_common::{
comment::{CommentResponse, CreateCommentLike},
context::LemmyContext,
send_activity::{ActivityChannel, SendActivityData},
utils::{check_bot_account, check_community_user_action, check_downvotes_enabled},
utils::{check_bot_account, check_community_user_action, check_local_vote_mode, VoteItem},
};
use lemmy_db_schema::{
newtypes::LocalUserId,
Expand All @@ -30,8 +30,7 @@ pub async fn like_comment(

let mut recipient_ids = Vec::<LocalUserId>::new();

// Don't do a downvote if site has downvotes disabled
check_downvotes_enabled(data.score, &local_site)?;
check_local_vote_mode(data.score, VoteItem::Comment, &local_site)?;
check_bot_account(&local_user_view.person)?;

let comment_id = data.comment_id;
Expand Down
6 changes: 3 additions & 3 deletions crates/api/src/post/like.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,9 @@ use lemmy_api_common::{
utils::{
check_bot_account,
check_community_user_action,
check_downvotes_enabled,
check_local_vote_mode,
mark_post_as_read,
VoteItem,
},
};
use lemmy_db_schema::{
Expand All @@ -32,8 +33,7 @@ pub async fn like_post(
) -> LemmyResult<Json<PostResponse>> {
let local_site = LocalSite::read(&mut context.pool()).await?;

// Don't do a downvote if site has downvotes disabled
check_downvotes_enabled(data.score, &local_site)?;
check_local_vote_mode(data.score, VoteItem::Post, &local_site)?;
check_bot_account(&local_user_view.person)?;

// Check for a community ban
Expand Down
20 changes: 15 additions & 5 deletions crates/api_common/src/site.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ use lemmy_db_schema::{
tagline::Tagline,
},
CommentSortType,
FederationMode,
ListingType,
ModlogActionType,
PostListingMode,
Expand Down Expand Up @@ -170,7 +171,6 @@ pub struct CreateSite {
pub description: Option<String>,
pub icon: Option<String>,
pub banner: Option<String>,
pub enable_downvotes: Option<bool>,
pub enable_nsfw: Option<bool>,
pub community_creation_admin_only: Option<bool>,
pub require_email_verification: Option<bool>,
Expand Down Expand Up @@ -208,6 +208,10 @@ pub struct CreateSite {
pub registration_mode: Option<RegistrationMode>,
pub oauth_registration: Option<bool>,
pub content_warning: Option<String>,
pub post_upvotes: Option<FederationMode>,
pub post_downvotes: Option<FederationMode>,
pub comment_upvotes: Option<FederationMode>,
pub comment_downvotes: Option<FederationMode>,
}

#[skip_serializing_none]
Expand All @@ -224,8 +228,6 @@ pub struct EditSite {
pub icon: Option<String>,
/// A url for your site's banner.
pub banner: Option<String>,
/// Whether to enable downvotes.
pub enable_downvotes: Option<bool>,
/// Whether to enable NSFW.
pub enable_nsfw: Option<bool>,
/// Limits community creation to admins only.
Expand Down Expand Up @@ -291,13 +293,21 @@ pub struct EditSite {
/// A list of blocked URLs
pub blocked_urls: Option<Vec<String>>,
pub registration_mode: Option<RegistrationMode>,
/// Whether or not external auth methods can auto-register users.
pub oauth_registration: Option<bool>,
/// Whether to email admins for new reports.
pub reports_email_admins: Option<bool>,
/// If present, nsfw content is visible by default. Should be displayed by frontends/clients
/// when the site is first opened by a user.
pub content_warning: Option<String>,
/// Whether or not external auth methods can auto-register users.
pub oauth_registration: Option<bool>,
/// What kind of post upvotes your site allows.
pub post_upvotes: Option<FederationMode>,
/// What kind of post downvotes your site allows.
pub post_downvotes: Option<FederationMode>,
/// What kind of comment upvotes your site allows.
pub comment_upvotes: Option<FederationMode>,
/// What kind of comment downvotes your site allows.
pub comment_downvotes: Option<FederationMode>,
}

#[derive(Debug, Serialize, Deserialize, Clone)]
Expand Down
25 changes: 22 additions & 3 deletions crates/api_common/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ use lemmy_db_schema::{
},
traits::Crud,
utils::DbPool,
FederationMode,
RegistrationMode,
};
use lemmy_db_views::{
Expand Down Expand Up @@ -296,10 +297,28 @@ pub async fn check_person_instance_community_block(
Ok(())
}

/// A vote item type used to check the vote mode.
pub enum VoteItem {
Post,
Comment,
}

#[tracing::instrument(skip_all)]
pub fn check_downvotes_enabled(score: i16, local_site: &LocalSite) -> LemmyResult<()> {
if score == -1 && !local_site.enable_downvotes {
Err(LemmyErrorType::DownvotesAreDisabled)?
pub fn check_local_vote_mode(
score: i16,
vote_item: VoteItem,
local_site: &LocalSite,
) -> LemmyResult<()> {
let (downvote_setting, upvote_setting) = match vote_item {
VoteItem::Post => (local_site.post_downvotes, local_site.post_upvotes),
VoteItem::Comment => (local_site.comment_downvotes, local_site.comment_upvotes),
};

let downvote_fail = score == -1 && downvote_setting == FederationMode::Disable;
let upvote_fail = score == 1 && upvote_setting == FederationMode::Disable;

if downvote_fail || upvote_fail {
Err(LemmyErrorType::VoteNotAllowed)?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought I left a comment here, about using the same logic as in federation mode to undo previous vote if an invalid mode is made. Eg comment upvoted then downvoted but downvotes are disabled, so it should delete the previous upvote.

Btw federation tests are failing, otherwise looks good.

Copy link
Member Author

@dessalines dessalines Oct 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotcha, added. The tests passed okay on my local machine, I think it was just that intermittent federation test error in CI that you just fixed.

} else {
Ok(())
}
Expand Down
5 changes: 4 additions & 1 deletion crates/api_crud/src/site/create.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,6 @@ pub async fn create_site(
let local_site_form = LocalSiteUpdateForm {
// Set the site setup to true
site_setup: Some(true),
enable_downvotes: data.enable_downvotes,
registration_mode: data.registration_mode,
community_creation_admin_only: data.community_creation_admin_only,
require_email_verification: data.require_email_verification,
Expand All @@ -110,6 +109,10 @@ pub async fn create_site(
captcha_enabled: data.captcha_enabled,
captcha_difficulty: data.captcha_difficulty.clone(),
default_post_listing_mode: data.default_post_listing_mode,
post_upvotes: data.post_upvotes,
post_downvotes: data.post_downvotes,
comment_upvotes: data.comment_upvotes,
comment_downvotes: data.comment_downvotes,
..Default::default()
};

Expand Down
5 changes: 4 additions & 1 deletion crates/api_crud/src/site/update.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,6 @@ pub async fn update_site(
.ok();

let local_site_form = LocalSiteUpdateForm {
enable_downvotes: data.enable_downvotes,
registration_mode: data.registration_mode,
community_creation_admin_only: data.community_creation_admin_only,
require_email_verification: data.require_email_verification,
Expand All @@ -121,6 +120,10 @@ pub async fn update_site(
reports_email_admins: data.reports_email_admins,
default_post_listing_mode: data.default_post_listing_mode,
oauth_registration: data.oauth_registration,
post_upvotes: data.post_upvotes,
post_downvotes: data.post_downvotes,
comment_upvotes: data.comment_upvotes,
comment_downvotes: data.comment_downvotes,
..Default::default()
};

Expand Down
24 changes: 18 additions & 6 deletions crates/apub/src/activities/voting/vote.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use activitypub_federation::{
traits::{ActivityHandler, Actor},
};
use lemmy_api_common::{context::LemmyContext, utils::check_bot_account};
use lemmy_db_schema::source::local_site::LocalSite;
use lemmy_db_schema::{source::local_site::LocalSite, FederationMode};
use lemmy_utils::error::{LemmyError, LemmyResult};
use url::Url;

Expand Down Expand Up @@ -68,12 +68,24 @@ impl ActivityHandler for Vote {

check_bot_account(&actor.0)?;

let enable_downvotes = LocalSite::read(&mut context.pool())
// Check for enabled federation votes
let local_site = LocalSite::read(&mut context.pool())
.await
.map(|l| l.enable_downvotes)
.unwrap_or(true);
if self.kind == VoteType::Dislike && !enable_downvotes {
// If this is a downvote but downvotes are ignored, only undo any existing vote
.unwrap_or_default();

let (downvote_setting, upvote_setting) = match object {
PostOrComment::Post(_) => (local_site.post_downvotes, local_site.post_upvotes),
PostOrComment::Comment(_) => (local_site.comment_downvotes, local_site.comment_upvotes),
};

// Don't allow dislikes for either disabled, or local only votes
let downvote_fail = self.kind == VoteType::Dislike
&& [FederationMode::Disable, FederationMode::Local].contains(&downvote_setting);
let upvote_fail = self.kind == VoteType::Like
&& [FederationMode::Disable, FederationMode::Local].contains(&upvote_setting);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simply do self.kind == VoteType::Like && upvote_setting != FederationMode::All


if downvote_fail || upvote_fail {
// If this is a rejection, undo the vote
match object {
PostOrComment::Post(p) => undo_vote_post(actor, &p, context).await,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Nutomic One reason that people gave for rejecting votes, is that they'd like to decrease the DB load, and avoid federated vote inserts entirely. But L89 seems like it inserts, then removes these votes (which would fire plenty of DB triggers also). Wouldn't it be better to just not insert them in the first place?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No this logic is used when a user first upvotes and then downvotes a post. So we cant apply the downvote, but leaving it as upvoted doesnt make sense either. Instead we remove the vote entirely. You can clarify the comments.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does that mean I need to add this check elsewhere also?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most of the logic is in mod.rs so it may be easier to make these changes there. For federation this should be all, of course you still need to add api checks.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding db writes, my assumption is that trying to delete nonexistent row doesnt write anything.

PostOrComment::Comment(c) => undo_vote_comment(actor, &c, context).await,
Expand Down
21 changes: 21 additions & 0 deletions crates/db_schema/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,27 @@ pub enum CommunityVisibility {
LocalOnly,
}

#[derive(
EnumString, Display, Debug, Serialize, Deserialize, Clone, Copy, PartialEq, Eq, Default, Hash,
)]
#[cfg_attr(feature = "full", derive(DbEnum, TS))]
#[cfg_attr(
feature = "full",
ExistingTypePath = "crate::schema::sql_types::FederationModeEnum"
)]
#[cfg_attr(feature = "full", DbValueStyle = "verbatim")]
#[cfg_attr(feature = "full", ts(export))]
/// The federation mode for an item
pub enum FederationMode {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I figure this could be used in the future to block things like federated comments, federated posts, etc.

#[default]
/// Allows all
All,
/// Allows only local
Local,
/// Disables
Disable,
}

/// Wrapper for assert_eq! macro. Checks that vec matches the given length, and prints the
/// vec on failure.
#[macro_export]
Expand Down
12 changes: 10 additions & 2 deletions crates/db_schema/src/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ pub mod sql_types {
#[diesel(postgres_type(name = "community_visibility"))]
pub struct CommunityVisibility;

#[derive(diesel::sql_types::SqlType)]
#[diesel(postgres_type(name = "federation_mode_enum"))]
pub struct FederationModeEnum;

#[derive(diesel::sql_types::SqlType)]
#[diesel(postgres_type(name = "listing_type_enum"))]
pub struct ListingTypeEnum;
Expand Down Expand Up @@ -368,12 +372,12 @@ diesel::table! {
use super::sql_types::PostListingModeEnum;
use super::sql_types::PostSortTypeEnum;
use super::sql_types::CommentSortTypeEnum;
use super::sql_types::FederationModeEnum;

local_site (id) {
id -> Int4,
site_id -> Int4,
site_setup -> Bool,
enable_downvotes -> Bool,
community_creation_admin_only -> Bool,
require_email_verification -> Bool,
application_question -> Nullable<Text>,
Expand All @@ -398,6 +402,10 @@ diesel::table! {
default_post_sort_type -> PostSortTypeEnum,
default_comment_sort_type -> CommentSortTypeEnum,
oauth_registration -> Bool,
post_upvotes -> FederationModeEnum,
post_downvotes -> FederationModeEnum,
comment_upvotes -> FederationModeEnum,
comment_downvotes -> FederationModeEnum,
}
}

Expand Down Expand Up @@ -769,7 +777,7 @@ diesel::table! {
featured_local -> Bool,
url_content_type -> Nullable<Text>,
alt_text -> Nullable<Text>,
scheduled_publish_time -> Nullable<Timestamptz>
scheduled_publish_time -> Nullable<Timestamptz>,
}
}

Expand Down
32 changes: 24 additions & 8 deletions crates/db_schema/src/source/local_site.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use crate::schema::local_site;
use crate::{
newtypes::{LocalSiteId, SiteId},
CommentSortType,
FederationMode,
ListingType,
PostListingMode,
PostSortType,
Expand All @@ -27,8 +28,6 @@ pub struct LocalSite {
pub site_id: SiteId,
/// True if the site is set up.
pub site_setup: bool,
/// Whether downvotes are enabled.
pub enable_downvotes: bool,
/// Whether only admins can create communities.
pub community_creation_admin_only: bool,
/// Whether emails are required.
Expand Down Expand Up @@ -72,6 +71,14 @@ pub struct LocalSite {
pub default_comment_sort_type: CommentSortType,
/// Whether or not external auth methods can auto-register users.
pub oauth_registration: bool,
/// What kind of post upvotes your site allows.
pub post_upvotes: FederationMode,
/// What kind of post downvotes your site allows.
pub post_downvotes: FederationMode,
/// What kind of comment upvotes your site allows.
pub comment_upvotes: FederationMode,
/// What kind of comment downvotes your site allows.
pub comment_downvotes: FederationMode,
}

#[derive(Clone, derive_new::new)]
Expand All @@ -82,8 +89,6 @@ pub struct LocalSiteInsertForm {
#[new(default)]
pub site_setup: Option<bool>,
#[new(default)]
pub enable_downvotes: Option<bool>,
#[new(default)]
pub community_creation_admin_only: Option<bool>,
#[new(default)]
pub require_email_verification: Option<bool>,
Expand Down Expand Up @@ -114,8 +119,6 @@ pub struct LocalSiteInsertForm {
#[new(default)]
pub registration_mode: Option<RegistrationMode>,
#[new(default)]
pub oauth_registration: Option<bool>,
#[new(default)]
pub reports_email_admins: Option<bool>,
#[new(default)]
pub federation_signed_fetch: Option<bool>,
Expand All @@ -125,14 +128,23 @@ pub struct LocalSiteInsertForm {
pub default_post_sort_type: Option<PostSortType>,
#[new(default)]
pub default_comment_sort_type: Option<CommentSortType>,
#[new(default)]
pub oauth_registration: Option<bool>,
#[new(default)]
pub post_upvotes: Option<FederationMode>,
#[new(default)]
pub post_downvotes: Option<FederationMode>,
#[new(default)]
pub comment_upvotes: Option<FederationMode>,
#[new(default)]
pub comment_downvotes: Option<FederationMode>,
}

#[derive(Clone, Default)]
#[cfg_attr(feature = "full", derive(AsChangeset))]
#[cfg_attr(feature = "full", diesel(table_name = local_site))]
pub struct LocalSiteUpdateForm {
pub site_setup: Option<bool>,
pub enable_downvotes: Option<bool>,
pub community_creation_admin_only: Option<bool>,
pub require_email_verification: Option<bool>,
pub application_question: Option<Option<String>>,
Expand All @@ -148,11 +160,15 @@ pub struct LocalSiteUpdateForm {
pub captcha_enabled: Option<bool>,
pub captcha_difficulty: Option<String>,
pub registration_mode: Option<RegistrationMode>,
pub oauth_registration: Option<bool>,
pub reports_email_admins: Option<bool>,
pub updated: Option<Option<DateTime<Utc>>>,
pub federation_signed_fetch: Option<bool>,
pub default_post_listing_mode: Option<PostListingMode>,
pub default_post_sort_type: Option<PostSortType>,
pub default_comment_sort_type: Option<CommentSortType>,
pub oauth_registration: Option<bool>,
pub post_upvotes: Option<FederationMode>,
pub post_downvotes: Option<FederationMode>,
pub comment_upvotes: Option<FederationMode>,
pub comment_downvotes: Option<FederationMode>,
}
2 changes: 1 addition & 1 deletion crates/utils/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ pub enum LemmyErrorType {
PersonIsBlocked,
CommunityIsBlocked,
InstanceIsBlocked,
DownvotesAreDisabled,
VoteNotAllowed,
InstanceIsPrivate,
/// Password must be between 10 and 60 characters
InvalidPassword,
Expand Down
Loading