From 7c039340ed175a464a7304b7bc686468e0f3353b Mon Sep 17 00:00:00 2001 From: Dessalines Date: Thu, 11 Mar 2021 17:47:44 -0500 Subject: [PATCH] 2nd pass. JWT now uses local_user_id --- crates/api/src/lib.rs | 20 +++--- crates/api/src/local_user.rs | 14 ++-- crates/api/src/websocket.rs | 3 +- crates/api_structs/src/lib.rs | 8 +-- crates/db_queries/src/lib.rs | 4 +- crates/db_queries/src/source/local_user.rs | 29 +-------- crates/db_queries/src/source/person.rs | 1 - crates/db_schema/src/source/person.rs | 2 +- crates/db_views/src/comment_view.rs | 2 +- crates/db_views/src/local_user_view.rs | 74 ++++++++++++++-------- crates/routes/src/webfinger.rs | 8 +-- crates/utils/src/claims.rs | 4 +- crates/utils/src/lib.rs | 2 +- crates/websocket/src/handlers.rs | 2 +- crates/websocket/src/messages.rs | 2 +- 15 files changed, 82 insertions(+), 93 deletions(-) diff --git a/crates/api/src/lib.rs b/crates/api/src/lib.rs index d71f4b6ee..65d97c4a4 100644 --- a/crates/api/src/lib.rs +++ b/crates/api/src/lib.rs @@ -103,11 +103,9 @@ pub(crate) async fn get_local_user_view_from_jwt( Ok(claims) => claims.claims, Err(_e) => return Err(ApiError::err("not_logged_in").into()), }; - let person_id = claims.id; - let local_user_view = blocking(pool, move |conn| { - LocalUserView::read_person(conn, person_id) - }) - .await??; + let local_user_id = claims.id; + let local_user_view = + blocking(pool, move |conn| LocalUserView::read(conn, local_user_id)).await??; // Check for a site ban if local_user_view.person.banned { return Err(ApiError::err("site_ban").into()); @@ -133,9 +131,9 @@ pub(crate) async fn get_local_user_settings_view_from_jwt( Ok(claims) => claims.claims, Err(_e) => return Err(ApiError::err("not_logged_in").into()), }; - let person_id = claims.id; + let local_user_id = claims.id; let local_user_view = blocking(pool, move |conn| { - LocalUserSettingsView::read(conn, person_id) + LocalUserSettingsView::read(conn, local_user_id) }) .await??; // Check for a site ban @@ -185,21 +183,21 @@ pub(crate) async fn check_downvotes_enabled(score: i16, pool: &DbPool) -> Result /// or if a community_id is supplied validates the user is a moderator /// of that community and returns the community id in a vec /// -/// * `user_id` - the user id of the moderator +/// * `person_id` - the person id of the moderator /// * `community_id` - optional community id to check for moderator privileges /// * `pool` - the diesel db pool pub(crate) async fn collect_moderated_communities( - user_id: i32, + person_id: i32, community_id: Option, pool: &DbPool, ) -> Result, LemmyError> { if let Some(community_id) = community_id { // if the user provides a community_id, just check for mod/admin privileges - is_mod_or_admin(pool, user_id, community_id).await?; + is_mod_or_admin(pool, person_id, community_id).await?; Ok(vec![community_id]) } else { let ids = blocking(pool, move |conn: &'_ _| { - CommunityModerator::get_person_moderated_communities(conn, user_id) + CommunityModerator::get_person_moderated_communities(conn, person_id) }) .await??; Ok(ids) diff --git a/crates/api/src/local_user.rs b/crates/api/src/local_user.rs index 781ef8f9a..9b5611f2c 100644 --- a/crates/api/src/local_user.rs +++ b/crates/api/src/local_user.rs @@ -129,7 +129,7 @@ impl Perform for Login { // Return the jwt Ok(LoginResponse { - jwt: Claims::jwt(local_user_view.person.id, Settings::get().hostname())?, + jwt: Claims::jwt(local_user_view.local_user.id, Settings::get().hostname())?, }) } } @@ -243,7 +243,7 @@ impl Perform for Register { send_notifications_to_email: Some(false), }; - match blocking(context.pool(), move |conn| { + let inserted_local_user = match blocking(context.pool(), move |conn| { LocalUser::register(conn, &local_user_form) }) .await? @@ -332,7 +332,7 @@ impl Perform for Register { // Return the jwt Ok(LoginResponse { - jwt: Claims::jwt(inserted_person.id, Settings::get().hostname())?, + jwt: Claims::jwt(inserted_local_user.id, Settings::get().hostname())?, }) } } @@ -479,7 +479,7 @@ impl Perform for SaveUserSettings { Person::update(conn, person_id, &person_form) }) .await?; - let updated_person: Person = match person_res { + let _updated_person: Person = match person_res { Ok(p) => p, Err(_) => { return Err(ApiError::err("user_already_exists").into()); @@ -505,8 +505,8 @@ impl Perform for SaveUserSettings { LocalUser::update(conn, local_user_id, &local_user_form) }) .await?; - match local_user_res { - Ok(user) => user, + let updated_local_user = match local_user_res { + Ok(u) => u, Err(e) => { let err_type = if e.to_string() == "duplicate key value violates unique constraint \"local_user_email_key\"" @@ -522,7 +522,7 @@ impl Perform for SaveUserSettings { // Return the jwt Ok(LoginResponse { - jwt: Claims::jwt(updated_person.id, Settings::get().hostname())?, + jwt: Claims::jwt(updated_local_user.id, Settings::get().hostname())?, }) } } diff --git a/crates/api/src/websocket.rs b/crates/api/src/websocket.rs index 47e21091f..ae5ba8940 100644 --- a/crates/api/src/websocket.rs +++ b/crates/api/src/websocket.rs @@ -21,8 +21,7 @@ impl Perform for UserJoin { if let Some(ws_id) = websocket_id { context.chat_server().do_send(JoinUserRoom { - // TODO this should probably be the local_user_id - user_id: local_user_view.person.id, + local_user_id: local_user_view.local_user.id, id: ws_id, }); } diff --git a/crates/api_structs/src/lib.rs b/crates/api_structs/src/lib.rs index c861136a1..e675d02ae 100644 --- a/crates/api_structs/src/lib.rs +++ b/crates/api_structs/src/lib.rs @@ -90,7 +90,7 @@ fn do_send_local_notifs( // TODO // At some point, make it so you can't tag the parent creator either // This can cause two notifications, one for reply and the other for mention - recipient_ids.push(mention_user_view.person.id); + recipient_ids.push(mention_user_view.local_user.id); let user_mention_form = PersonMentionForm { recipient_id: mention_user_view.person.id, @@ -102,7 +102,7 @@ fn do_send_local_notifs( // Let the uniqueness handle this fail PersonMention::create(&conn, &user_mention_form).ok(); - // Send an email to those users that have notifications on + // Send an email to those local users that have notifications on if do_send_email && mention_user_view.local_user.send_notifications_to_email { send_email_to_user( mention_user_view, @@ -121,7 +121,7 @@ fn do_send_local_notifs( if parent_comment.creator_id != person.id { if let Ok(parent_user_view) = LocalUserView::read_person(&conn, parent_comment.creator_id) { - recipient_ids.push(parent_user_view.person.id); + recipient_ids.push(parent_user_view.local_user.id); if do_send_email && parent_user_view.local_user.send_notifications_to_email { send_email_to_user( @@ -139,7 +139,7 @@ fn do_send_local_notifs( None => { if post.creator_id != person.id { if let Ok(parent_user_view) = LocalUserView::read_person(&conn, post.creator_id) { - recipient_ids.push(parent_user_view.person.id); + recipient_ids.push(parent_user_view.local_user.id); if do_send_email && parent_user_view.local_user.send_notifications_to_email { send_email_to_user( diff --git a/crates/db_queries/src/lib.rs b/crates/db_queries/src/lib.rs index f19d36263..e027cfc91 100644 --- a/crates/db_queries/src/lib.rs +++ b/crates/db_queries/src/lib.rs @@ -47,7 +47,7 @@ pub trait Followable { fn follow(conn: &PgConnection, form: &T) -> Result where Self: Sized; - fn follow_accepted(conn: &PgConnection, community_id: i32, user_id: i32) -> Result + fn follow_accepted(conn: &PgConnection, community_id: i32, person_id: i32) -> Result where Self: Sized; fn unfollow(conn: &PgConnection, form: &T) -> Result @@ -69,7 +69,7 @@ pub trait Likeable { fn like(conn: &PgConnection, form: &T) -> Result where Self: Sized; - fn remove(conn: &PgConnection, user_id: i32, item_id: i32) -> Result + fn remove(conn: &PgConnection, person_id: i32, item_id: i32) -> Result where Self: Sized; } diff --git a/crates/db_queries/src/source/local_user.rs b/crates/db_queries/src/source/local_user.rs index 58579f962..4ca557c59 100644 --- a/crates/db_queries/src/source/local_user.rs +++ b/crates/db_queries/src/source/local_user.rs @@ -1,9 +1,9 @@ -use crate::{Crud, ToSafeSettings}; +use crate::Crud; use bcrypt::{hash, DEFAULT_COST}; use diesel::{dsl::*, result::Error, *}; use lemmy_db_schema::{ schema::local_user::dsl::*, - source::local_user::{LocalUser, LocalUserForm, LocalUserSettings}, + source::local_user::{LocalUser, LocalUserForm}, }; mod safe_type { @@ -70,7 +70,6 @@ pub trait LocalUser_ { new_password: &str, ) -> Result; fn add_admin(conn: &PgConnection, local_user_id: i32, added: bool) -> Result; - fn find_by_person(conn: &PgConnection, from_person_id: i32) -> Result; } impl LocalUser_ for LocalUser { @@ -83,7 +82,6 @@ impl LocalUser_ for LocalUser { Self::create(&conn, &edited_user) } - // TODO do more individual updates like these fn update_password( conn: &PgConnection, local_user_id: i32, @@ -96,19 +94,11 @@ impl LocalUser_ for LocalUser { .get_result::(conn) } - // TODO is this used? fn add_admin(conn: &PgConnection, local_user_id: i32, added: bool) -> Result { diesel::update(local_user.find(local_user_id)) .set(admin.eq(added)) .get_result::(conn) } - - // TODO is this used? - fn find_by_person(conn: &PgConnection, for_person_id: i32) -> Result { - local_user - .filter(person_id.eq(for_person_id)) - .first::(conn) - } } impl Crud for LocalUser { @@ -129,18 +119,3 @@ impl Crud for LocalUser { .get_result::(conn) } } - -// TODO is this used? -pub trait LocalUserSettings_ { - fn read(conn: &PgConnection, user_id: i32) -> Result; -} - -// TODO is this used? -impl LocalUserSettings_ for LocalUserSettings { - fn read(conn: &PgConnection, user_id: i32) -> Result { - local_user - .select(LocalUser::safe_settings_columns_tuple()) - .find(user_id) - .first::(conn) - } -} diff --git a/crates/db_queries/src/source/person.rs b/crates/db_queries/src/source/person.rs index f167e7f7a..81df7b717 100644 --- a/crates/db_queries/src/source/person.rs +++ b/crates/db_queries/src/source/person.rs @@ -192,7 +192,6 @@ impl Person_ for Person { .get_result::(conn) } - // TODO is this used? fn find_by_name(conn: &PgConnection, from_name: &str) -> Result { person .filter(deleted.eq(false)) diff --git a/crates/db_schema/src/source/person.rs b/crates/db_schema/src/source/person.rs index 7fc3692f8..b3af21c3d 100644 --- a/crates/db_schema/src/source/person.rs +++ b/crates/db_schema/src/source/person.rs @@ -26,7 +26,7 @@ pub struct Person { pub shared_inbox_url: Option, } -/// A safe representation of user, without the sensitive info +/// A safe representation of person, without the sensitive info #[derive(Clone, Queryable, Identifiable, PartialEq, Debug, Serialize)] #[table_name = "person"] pub struct PersonSafe { diff --git a/crates/db_views/src/comment_view.rs b/crates/db_views/src/comment_view.rs index 79c420036..d3e97c642 100644 --- a/crates/db_views/src/comment_view.rs +++ b/crates/db_views/src/comment_view.rs @@ -367,7 +367,7 @@ impl<'a> CommentQueryBuilder<'a> { query = match self.listing_type { // ListingType::Subscribed => query.filter(community_follower::subscribed.eq(true)), - ListingType::Subscribed => query.filter(community_follower::person_id.is_not_null()), // TODO could be this: and(community_follower::user_id.eq(user_id_join)), + ListingType::Subscribed => query.filter(community_follower::person_id.is_not_null()), // TODO could be this: and(community_follower::person_id.eq(person_id_join)), ListingType::Local => query.filter(community::local.eq(true)), _ => query, }; diff --git a/crates/db_views/src/local_user_view.rs b/crates/db_views/src/local_user_view.rs index d377af633..ee4811b65 100644 --- a/crates/db_views/src/local_user_view.rs +++ b/crates/db_views/src/local_user_view.rs @@ -11,42 +11,60 @@ use serde::Serialize; #[derive(Debug, Serialize, Clone)] pub struct LocalUserView { + pub local_user: LocalUser, pub person: Person, pub counts: PersonAggregates, - pub local_user: LocalUser, } -type LocalUserViewTuple = (Person, PersonAggregates, LocalUser); +type LocalUserViewTuple = (LocalUser, Person, PersonAggregates); impl LocalUserView { - pub fn read_person(conn: &PgConnection, person_id: i32) -> Result { - let (person, counts, local_user) = person::table - .find(person_id) - .inner_join(person_aggregates::table) - .inner_join(local_user::table) + pub fn read(conn: &PgConnection, local_user_id: i32) -> Result { + let (local_user, person, counts) = local_user::table + .find(local_user_id) + .inner_join(person::table) + .inner_join(person_aggregates::table.on(person::id.eq(person_aggregates::person_id))) .select(( + local_user::all_columns, person::all_columns, person_aggregates::all_columns, - local_user::all_columns, )) .first::(conn)?; Ok(Self { + local_user, person, counts, + }) + } + + pub fn read_person(conn: &PgConnection, person_id: i32) -> Result { + let (local_user, person, counts) = local_user::table + .filter(person::id.eq(person_id)) + .inner_join(person::table) + .inner_join(person_aggregates::table.on(person::id.eq(person_aggregates::person_id))) + .select(( + local_user::all_columns, + person::all_columns, + person_aggregates::all_columns, + )) + .first::(conn)?; + Ok(Self { local_user, + person, + counts, }) } // TODO check where this is used pub fn read_from_name(conn: &PgConnection, name: &str) -> Result { - let (person, counts, local_user) = person::table + let (local_user, person, counts) = local_user::table .filter(person::name.eq(name)) - .inner_join(person_aggregates::table) - .inner_join(local_user::table) + .inner_join(person::table) + .inner_join(person_aggregates::table.on(person::id.eq(person_aggregates::person_id))) .select(( + local_user::all_columns, person::all_columns, person_aggregates::all_columns, - local_user::all_columns, )) .first::(conn)?; Ok(Self { @@ -57,18 +75,18 @@ impl LocalUserView { } pub fn find_by_email_or_name(conn: &PgConnection, name_or_email: &str) -> Result { - let (person, counts, local_user) = person::table - .inner_join(person_aggregates::table) - .inner_join(local_user::table) + let (local_user, person, counts) = local_user::table + .inner_join(person::table) + .inner_join(person_aggregates::table.on(person::id.eq(person_aggregates::person_id))) .filter( person::name .ilike(name_or_email) .or(local_user::email.ilike(name_or_email)), ) .select(( + local_user::all_columns, person::all_columns, person_aggregates::all_columns, - local_user::all_columns, )) .first::(conn)?; Ok(Self { @@ -79,14 +97,14 @@ impl LocalUserView { } pub fn find_by_email(conn: &PgConnection, from_email: &str) -> Result { - let (person, counts, local_user) = person::table - .inner_join(person_aggregates::table) - .inner_join(local_user::table) + let (local_user, person, counts) = local_user::table + .inner_join(person::table) + .inner_join(person_aggregates::table.on(person::id.eq(person_aggregates::person_id))) .filter(local_user::email.eq(from_email)) .select(( + local_user::all_columns, person::all_columns, person_aggregates::all_columns, - local_user::all_columns, )) .first::(conn)?; Ok(Self { @@ -99,23 +117,23 @@ impl LocalUserView { #[derive(Debug, Serialize, Clone)] pub struct LocalUserSettingsView { + pub local_user: LocalUserSettings, pub person: PersonSafe, pub counts: PersonAggregates, - pub local_user: LocalUserSettings, } -type LocalUserSettingsViewTuple = (PersonSafe, PersonAggregates, LocalUserSettings); +type LocalUserSettingsViewTuple = (LocalUserSettings, PersonSafe, PersonAggregates); impl LocalUserSettingsView { - pub fn read(conn: &PgConnection, person_id: i32) -> Result { - let (person, counts, local_user) = person::table - .find(person_id) - .inner_join(person_aggregates::table) - .inner_join(local_user::table) + pub fn read(conn: &PgConnection, local_user_id: i32) -> Result { + let (local_user, person, counts) = local_user::table + .find(local_user_id) + .inner_join(person::table) + .inner_join(person_aggregates::table.on(person::id.eq(person_aggregates::person_id))) .select(( + LocalUser::safe_settings_columns_tuple(), Person::safe_columns_tuple(), person_aggregates::all_columns, - LocalUser::safe_settings_columns_tuple(), )) .first::(conn)?; Ok(Self { diff --git a/crates/routes/src/webfinger.rs b/crates/routes/src/webfinger.rs index c444451e9..8ab2a5b6a 100644 --- a/crates/routes/src/webfinger.rs +++ b/crates/routes/src/webfinger.rs @@ -7,7 +7,7 @@ use lemmy_utils::{ settings::structs::Settings, LemmyError, WEBFINGER_COMMUNITY_REGEX, - WEBFINGER_USER_REGEX, + WEBFINGER_USERNAME_REGEX, }; use lemmy_websocket::LemmyContext; use serde::Deserialize; @@ -41,7 +41,7 @@ async fn get_webfinger_response( .map(|c| c.get(1)) .flatten(); - let user_regex_parsed = WEBFINGER_USER_REGEX + let username_regex_parsed = WEBFINGER_USERNAME_REGEX .captures(&info.resource) .map(|c| c.get(1)) .flatten(); @@ -55,9 +55,9 @@ async fn get_webfinger_response( .await? .map_err(|_| ErrorBadRequest(LemmyError::from(anyhow!("not_found"))))? .actor_id - } else if let Some(person_name) = user_regex_parsed { + } else if let Some(person_name) = username_regex_parsed { let person_name = person_name.as_str().to_owned(); - // Make sure the requested user exists. + // Make sure the requested person exists. blocking(context.pool(), move |conn| { Person::find_by_name(conn, &person_name) }) diff --git a/crates/utils/src/claims.rs b/crates/utils/src/claims.rs index 3d9232e6b..3529f5f19 100644 --- a/crates/utils/src/claims.rs +++ b/crates/utils/src/claims.rs @@ -23,9 +23,9 @@ impl Claims { ) } - pub fn jwt(user_id: i32, hostname: String) -> Result { + pub fn jwt(local_user_id: i32, hostname: String) -> Result { let my_claims = Claims { - id: user_id, + id: local_user_id, iss: hostname, }; encode( diff --git a/crates/utils/src/lib.rs b/crates/utils/src/lib.rs index bd2b56844..5e76e8bc5 100644 --- a/crates/utils/src/lib.rs +++ b/crates/utils/src/lib.rs @@ -87,7 +87,7 @@ lazy_static! { Settings::get().hostname() )) .expect("compile webfinger regex"); - pub static ref WEBFINGER_USER_REGEX: Regex = Regex::new(&format!( + pub static ref WEBFINGER_USERNAME_REGEX: Regex = Regex::new(&format!( "^acct:([a-z0-9_]{{3, 20}})@{}$", Settings::get().hostname() )) diff --git a/crates/websocket/src/handlers.rs b/crates/websocket/src/handlers.rs index 0762b9485..7198fcb78 100644 --- a/crates/websocket/src/handlers.rs +++ b/crates/websocket/src/handlers.rs @@ -155,7 +155,7 @@ impl Handler for ChatServer { type Result = (); fn handle(&mut self, msg: JoinUserRoom, _: &mut Context) { - self.join_user_room(msg.user_id, msg.id).ok(); + self.join_user_room(msg.local_user_id, msg.id).ok(); } } diff --git a/crates/websocket/src/messages.rs b/crates/websocket/src/messages.rs index 89f3f2b3e..b3d98d066 100644 --- a/crates/websocket/src/messages.rs +++ b/crates/websocket/src/messages.rs @@ -91,7 +91,7 @@ pub struct SendComment { #[derive(Message)] #[rtype(result = "()")] pub struct JoinUserRoom { - pub user_id: UserId, + pub local_user_id: UserId, pub id: ConnectionId, }