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
14 changes: 10 additions & 4 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 @@ -27,14 +27,20 @@ pub async fn like_comment(
local_user_view: LocalUserView,
) -> LemmyResult<Json<CommentResponse>> {
let local_site = LocalSite::read(&mut context.pool()).await?;
let comment_id = data.comment_id;

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(comment_id),
&local_site,
local_user_view.person.id,
&mut context.pool(),
)
.await?;
check_bot_account(&local_user_view.person)?;

let comment_id = data.comment_id;
let orig_comment = CommentView::read(
&mut context.pool(),
comment_id,
Expand Down
15 changes: 11 additions & 4 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 @@ -31,13 +32,19 @@ pub async fn like_post(
local_user_view: LocalUserView,
) -> LemmyResult<Json<PostResponse>> {
let local_site = LocalSite::read(&mut context.pool()).await?;
let post_id = data.post_id;

// 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(post_id),
&local_site,
local_user_view.person.id,
&mut context.pool(),
)
.await?;
check_bot_account(&local_user_view.person)?;

// Check for a community ban
let post_id = data.post_id;
let post = Post::read(&mut context.pool(), post_id).await?;

check_community_user_action(
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
42 changes: 33 additions & 9 deletions crates/api_common/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ use chrono::{DateTime, Days, Local, TimeZone, Utc};
use enum_map::{enum_map, EnumMap};
use lemmy_db_schema::{
aggregates::structs::{PersonPostAggregates, PersonPostAggregatesForm},
newtypes::{CommunityId, DbUrl, InstanceId, PersonId, PostId},
newtypes::{CommentId, CommunityId, DbUrl, InstanceId, PersonId, PostId},
source::{
comment::{Comment, CommentUpdateForm},
comment::{Comment, CommentLike, CommentUpdateForm},
community::{Community, CommunityModerator, CommunityUpdateForm},
community_block::CommunityBlock,
email_verification::{EmailVerification, EmailVerificationForm},
Expand All @@ -27,12 +27,13 @@ use lemmy_db_schema::{
password_reset_request::PasswordResetRequest,
person::{Person, PersonUpdateForm},
person_block::PersonBlock,
post::{Post, PostRead},
post::{Post, PostLike, PostRead},
registration_application::RegistrationApplication,
site::Site,
},
traits::Crud,
traits::{Crud, Likeable},
utils::DbPool,
FederationMode,
RegistrationMode,
};
use lemmy_db_views::{
Expand Down Expand Up @@ -296,13 +297,36 @@ pub async fn check_person_instance_community_block(
Ok(())
}

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

#[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)?
} else {
Ok(())
pub async fn check_local_vote_mode(
score: i16,
vote_item: VoteItem,
local_site: &LocalSite,
person_id: PersonId,
pool: &mut DbPool<'_>,
) -> 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;

// Undo previous vote for item if new vote fails
if downvote_fail || upvote_fail {
match vote_item {
VoteItem::Post(post_id) => PostLike::remove(pool, person_id, post_id).await?,
VoteItem::Comment(comment_id) => CommentLike::remove(pool, person_id, comment_id).await?,
};
}
Ok(())
}

/// Dont allow bots to do certain actions, like voting
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
22 changes: 16 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,22 @@ 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 && downvote_setting != FederationMode::All;
let upvote_fail = 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
10 changes: 9 additions & 1 deletion 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
Loading