Skip to content

Commit

Permalink
Move admin flag from person to local_user (fixes #3060) (#3403)
Browse files Browse the repository at this point in the history
* Move admin flag from person to local_user (fixes #3060)

The person table is for federated data, but admin flag can only
apply to local users. Thats why it really belongs in the local_user
table. This will also prevent the federation code from accidentally
overwriting the admin flag

* fmt

* try to fix api tests

* lint

* fix person view

* ci

* ci

---------

Co-authored-by: Dessalines <[email protected]>
  • Loading branch information
Nutomic and dessalines authored Aug 24, 2023
1 parent 51ccf31 commit 6047257
Show file tree
Hide file tree
Showing 27 changed files with 140 additions and 81 deletions.
2 changes: 1 addition & 1 deletion crates/api/src/comment_report/list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ pub async fn list_comment_reports(
page,
limit,
}
.list(&mut context.pool(), &local_user_view.person)
.list(&mut context.pool(), &local_user_view)
.await?;

Ok(Json(ListCommentReportsResponse { comment_reports }))
Expand Down
2 changes: 1 addition & 1 deletion crates/api/src/community/add_mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ pub async fn add_mod_to_community(
// Verify that only mods or admins can add mod
is_mod_or_admin(&mut context.pool(), local_user_view.person.id, community_id).await?;
let community = Community::read(&mut context.pool(), community_id).await?;
if local_user_view.person.admin && !community.local {
if local_user_view.local_user.admin && !community.local {
return Err(LemmyErrorType::NotAModerator)?;
}

Expand Down
14 changes: 6 additions & 8 deletions crates/api/src/local_user/add_admin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ use lemmy_api_common::{
};
use lemmy_db_schema::{
source::{
local_user::{LocalUser, LocalUserUpdateForm},
moderator::{ModAdd, ModAddForm},
person::{Person, PersonUpdateForm},
},
traits::Crud,
};
Expand All @@ -27,13 +27,11 @@ impl Perform for AddAdmin {
// Make sure user is an admin
is_admin(&local_user_view)?;

let added = data.added;
let added_person_id = data.person_id;
let added_admin = Person::update(
let added_admin = LocalUser::update(
&mut context.pool(),
added_person_id,
&PersonUpdateForm {
admin: Some(added),
data.local_user_id,
&LocalUserUpdateForm {
admin: Some(data.added),
..Default::default()
},
)
Expand All @@ -43,7 +41,7 @@ impl Perform for AddAdmin {
// Mod tables
let form = ModAddForm {
mod_person_id: local_user_view.person.id,
other_person_id: added_admin.id,
other_person_id: added_admin.person_id,
removed: Some(!data.added),
};

Expand Down
9 changes: 5 additions & 4 deletions crates/api/src/local_user/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use lemmy_db_schema::{
source::person_block::{PersonBlock, PersonBlockForm},
traits::Blockable,
};
use lemmy_db_views::structs::LocalUserView;
use lemmy_db_views_actor::structs::PersonView;
use lemmy_utils::error::{LemmyError, LemmyErrorExt, LemmyErrorType};

Expand All @@ -34,9 +35,8 @@ impl Perform for BlockPerson {
target_id,
};

let target_person_view = PersonView::read(&mut context.pool(), target_id).await?;

if target_person_view.person.admin {
let target_user = LocalUserView::read_person(&mut context.pool(), target_id).await;
if target_user.map(|t| t.local_user.admin) == Ok(true) {
return Err(LemmyErrorType::CantBlockAdmin)?;
}

Expand All @@ -50,8 +50,9 @@ impl Perform for BlockPerson {
.with_lemmy_type(LemmyErrorType::PersonBlockAlreadyExists)?;
}

let person_view = PersonView::read(&mut context.pool(), target_id).await?;
Ok(BlockPersonResponse {
person_view: target_person_view,
person_view,
blocked: data.block,
})
}
Expand Down
2 changes: 1 addition & 1 deletion crates/api/src/local_user/login.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ impl Perform for Login {

// Check if the user's email is verified if email verification is turned on
// However, skip checking verification if the user is an admin
if !local_user_view.person.admin
if !local_user_view.local_user.admin
&& site_view.local_site.require_email_verification
&& !local_user_view.local_user.email_verified
{
Expand Down
2 changes: 1 addition & 1 deletion crates/api/src/local_user/report_count.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ impl Perform for GetReportCount {
let local_user_view = local_user_view_from_jwt(&data.auth, context).await?;

let person_id = local_user_view.person.id;
let admin = local_user_view.person.admin;
let admin = local_user_view.local_user.admin;
let community_id = data.community_id;

let comment_reports =
Expand Down
2 changes: 1 addition & 1 deletion crates/api/src/post_report/list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ impl Perform for ListPostReports {
page,
limit,
}
.list(&mut context.pool(), &local_user_view.person)
.list(&mut context.pool(), &local_user_view)
.await?;

Ok(ListPostReportsResponse { post_reports })
Expand Down
10 changes: 5 additions & 5 deletions crates/api/src/site/leave_admin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ use lemmy_db_schema::{
source::{
actor_language::SiteLanguage,
language::Language,
local_user::{LocalUser, LocalUserUpdateForm},
moderator::{ModAdd, ModAddForm},
person::{Person, PersonUpdateForm},
tagline::Tagline,
},
traits::Crud,
Expand Down Expand Up @@ -39,18 +39,18 @@ impl Perform for LeaveAdmin {
return Err(LemmyErrorType::CannotLeaveAdmin)?;
}

let person_id = local_user_view.person.id;
Person::update(
LocalUser::update(
&mut context.pool(),
person_id,
&PersonUpdateForm {
local_user_view.local_user.id,
&LocalUserUpdateForm {
admin: Some(false),
..Default::default()
},
)
.await?;

// Mod tables
let person_id = local_user_view.person.id;
let form = ModAddForm {
mod_person_id: person_id,
other_person_id: person_id,
Expand Down
4 changes: 2 additions & 2 deletions crates/api_common/src/person.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::sensitive::Sensitive;
use lemmy_db_schema::{
newtypes::{CommentReplyId, CommunityId, LanguageId, PersonId, PersonMentionId},
newtypes::{CommentReplyId, CommunityId, LanguageId, LocalUserId, PersonId, PersonMentionId},
CommentSortType,
ListingType,
SortType,
Expand Down Expand Up @@ -207,7 +207,7 @@ pub struct MarkAllAsRead {
#[cfg_attr(feature = "full", ts(export))]
/// Adds an admin to a site.
pub struct AddAdmin {
pub person_id: PersonId,
pub local_user_id: LocalUserId,
pub added: bool,
pub auth: Sensitive<String>,
}
Expand Down
6 changes: 3 additions & 3 deletions crates/api_common/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,8 @@ pub async fn is_mod_or_admin_opt(
}

pub fn is_admin(local_user_view: &LocalUserView) -> Result<(), LemmyError> {
if !local_user_view.person.admin {
Err(LemmyErrorType::NotAnAdmin)?;
if !local_user_view.local_user.admin {
return Err(LemmyErrorType::NotAnAdmin)?;
}
Ok(())
}
Expand Down Expand Up @@ -500,7 +500,7 @@ pub async fn check_registration_application(
if (local_site.registration_mode == RegistrationMode::RequireApplication
|| local_site.registration_mode == RegistrationMode::Closed)
&& !local_user_view.local_user.accepted_application
&& !local_user_view.person.admin
&& !local_user_view.local_user.admin
{
// Fetch the registration, see if its denied
let local_user_id = local_user_view.local_user.id;
Expand Down
4 changes: 2 additions & 2 deletions crates/api_crud/src/user/create.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,6 @@ pub async fn register(
.public_key(actor_keypair.public_key)
.inbox_url(Some(generate_inbox_url(&actor_id)?))
.shared_inbox_url(Some(generate_shared_inbox_url(&actor_id)?))
// If its the initial site setup, they are an admin
.admin(Some(!local_site.site_setup))
.instance_id(site_view.site.instance_id)
.build();

Expand All @@ -137,6 +135,8 @@ pub async fn register(
.show_nsfw(Some(data.show_nsfw))
.accepted_application(accepted_application)
.default_listing_type(Some(local_site.default_post_listing_type))
// If its the initial site setup, they are an admin
.admin(Some(!local_site.site_setup))
.build();

let inserted_local_user = LocalUser::create(&mut context.pool(), &local_user_form).await?;
Expand Down
1 change: 0 additions & 1 deletion crates/apub/src/objects/person.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,6 @@ impl Object for ApubPerson {
actor_id: Some(person.id.into()),
bio,
local: Some(false),
admin: Some(false),
bot_account: Some(person.kind == UserTypes::Service),
private_key: None,
public_key: person.public_key.public_key_pem,
Expand Down
1 change: 0 additions & 1 deletion crates/db_schema/src/impls/person.rs
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,6 @@ mod tests {
bio: None,
local: true,
bot_account: false,
admin: false,
private_key: None,
public_key: "nada".to_owned(),
last_refreshed_at: inserted_person.published,
Expand Down
4 changes: 2 additions & 2 deletions crates/db_schema/src/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -396,9 +396,10 @@ diesel::table! {
totp_2fa_secret -> Nullable<Text>,
totp_2fa_url -> Nullable<Text>,
open_links_in_new_tab -> Bool,
infinite_scroll_enabled -> Bool,
blur_nsfw -> Bool,
auto_expand -> Bool,
infinite_scroll_enabled -> Bool,
admin -> Bool,
}
}

Expand Down Expand Up @@ -566,7 +567,6 @@ diesel::table! {
#[max_length = 255]
shared_inbox_url -> Nullable<Varchar>,
matrix_user_id -> Nullable<Text>,
admin -> Bool,
bot_account -> Bool,
ban_expires -> Nullable<Timestamp>,
instance_id -> Int4,
Expand Down
4 changes: 4 additions & 0 deletions crates/db_schema/src/source/local_user.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ pub struct LocalUser {
pub auto_expand: bool,
/// Whether infinite scroll is enabled.
pub infinite_scroll_enabled: bool,
/// Whether the person is an admin.
pub admin: bool,
}

#[derive(Clone, TypedBuilder)]
Expand Down Expand Up @@ -88,6 +90,7 @@ pub struct LocalUserInsertForm {
pub blur_nsfw: Option<bool>,
pub auto_expand: Option<bool>,
pub infinite_scroll_enabled: Option<bool>,
pub admin: Option<bool>,
}

#[derive(Clone, Default)]
Expand Down Expand Up @@ -115,4 +118,5 @@ pub struct LocalUserUpdateForm {
pub blur_nsfw: Option<bool>,
pub auto_expand: Option<bool>,
pub infinite_scroll_enabled: Option<bool>,
pub admin: Option<bool>,
}
4 changes: 0 additions & 4 deletions crates/db_schema/src/source/person.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,6 @@ pub struct Person {
pub shared_inbox_url: Option<DbUrl>,
/// A matrix id, usually given an @person:matrix.org
pub matrix_user_id: Option<String>,
/// Whether the person is an admin.
pub admin: bool,
/// Whether the person is a bot account.
pub bot_account: bool,
/// When their ban, if it exists, expires, if at all.
Expand Down Expand Up @@ -84,7 +82,6 @@ pub struct PersonInsertForm {
pub inbox_url: Option<DbUrl>,
pub shared_inbox_url: Option<DbUrl>,
pub matrix_user_id: Option<String>,
pub admin: Option<bool>,
pub bot_account: Option<bool>,
pub ban_expires: Option<chrono::NaiveDateTime>,
}
Expand All @@ -108,7 +105,6 @@ pub struct PersonUpdateForm {
pub inbox_url: Option<DbUrl>,
pub shared_inbox_url: Option<Option<DbUrl>>,
pub matrix_user_id: Option<Option<String>>,
pub admin: Option<bool>,
pub bot_account: Option<bool>,
pub ban_expires: Option<Option<chrono::NaiveDateTime>>,
}
Expand Down
Loading

0 comments on commit 6047257

Please sign in to comment.