From 7c039340ed175a464a7304b7bc686468e0f3353b Mon Sep 17 00:00:00 2001
From: Dessalines <tyhou13@gmx.com>
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 d71f4b6e..65d97c4a 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<i32>,
   pool: &DbPool,
 ) -> Result<Vec<i32>, 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 781ef8f9..9b5611f2 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 47e21091..ae5ba894 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 c861136a..e675d02a 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 f19d3626..e027cfc9 100644
--- a/crates/db_queries/src/lib.rs
+++ b/crates/db_queries/src/lib.rs
@@ -47,7 +47,7 @@ pub trait Followable<T> {
   fn follow(conn: &PgConnection, form: &T) -> Result<Self, Error>
   where
     Self: Sized;
-  fn follow_accepted(conn: &PgConnection, community_id: i32, user_id: i32) -> Result<Self, Error>
+  fn follow_accepted(conn: &PgConnection, community_id: i32, person_id: i32) -> Result<Self, Error>
   where
     Self: Sized;
   fn unfollow(conn: &PgConnection, form: &T) -> Result<usize, Error>
@@ -69,7 +69,7 @@ pub trait Likeable<T> {
   fn like(conn: &PgConnection, form: &T) -> Result<Self, Error>
   where
     Self: Sized;
-  fn remove(conn: &PgConnection, user_id: i32, item_id: i32) -> Result<usize, Error>
+  fn remove(conn: &PgConnection, person_id: i32, item_id: i32) -> Result<usize, Error>
   where
     Self: Sized;
 }
diff --git a/crates/db_queries/src/source/local_user.rs b/crates/db_queries/src/source/local_user.rs
index 58579f96..4ca557c5 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<LocalUser, Error>;
   fn add_admin(conn: &PgConnection, local_user_id: i32, added: bool) -> Result<LocalUser, Error>;
-  fn find_by_person(conn: &PgConnection, from_person_id: i32) -> Result<LocalUser, Error>;
 }
 
 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::<Self>(conn)
   }
 
-  // TODO is this used?
   fn add_admin(conn: &PgConnection, local_user_id: i32, added: bool) -> Result<Self, Error> {
     diesel::update(local_user.find(local_user_id))
       .set(admin.eq(added))
       .get_result::<Self>(conn)
   }
-
-  // TODO is this used?
-  fn find_by_person(conn: &PgConnection, for_person_id: i32) -> Result<LocalUser, Error> {
-    local_user
-      .filter(person_id.eq(for_person_id))
-      .first::<LocalUser>(conn)
-  }
 }
 
 impl Crud<LocalUserForm> for LocalUser {
@@ -129,18 +119,3 @@ impl Crud<LocalUserForm> for LocalUser {
       .get_result::<Self>(conn)
   }
 }
-
-// TODO is this used?
-pub trait LocalUserSettings_ {
-  fn read(conn: &PgConnection, user_id: i32) -> Result<LocalUserSettings, Error>;
-}
-
-// TODO is this used?
-impl LocalUserSettings_ for LocalUserSettings {
-  fn read(conn: &PgConnection, user_id: i32) -> Result<Self, Error> {
-    local_user
-      .select(LocalUser::safe_settings_columns_tuple())
-      .find(user_id)
-      .first::<Self>(conn)
-  }
-}
diff --git a/crates/db_queries/src/source/person.rs b/crates/db_queries/src/source/person.rs
index f167e7f7..81df7b71 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::<Self>(conn)
   }
 
-  // TODO is this used?
   fn find_by_name(conn: &PgConnection, from_name: &str) -> Result<Person, Error> {
     person
       .filter(deleted.eq(false))
diff --git a/crates/db_schema/src/source/person.rs b/crates/db_schema/src/source/person.rs
index 7fc3692f..b3af21c3 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<DbUrl>,
 }
 
-/// 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 79c42003..d3e97c64 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 d377af63..ee4811b6 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<Self, Error> {
-    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<Self, Error> {
+    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::<LocalUserViewTuple>(conn)?;
     Ok(Self {
+      local_user,
       person,
       counts,
+    })
+  }
+
+  pub fn read_person(conn: &PgConnection, person_id: i32) -> Result<Self, Error> {
+    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::<LocalUserViewTuple>(conn)?;
+    Ok(Self {
       local_user,
+      person,
+      counts,
     })
   }
 
   // TODO check where this is used
   pub fn read_from_name(conn: &PgConnection, name: &str) -> Result<Self, Error> {
-    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::<LocalUserViewTuple>(conn)?;
     Ok(Self {
@@ -57,18 +75,18 @@ impl LocalUserView {
   }
 
   pub fn find_by_email_or_name(conn: &PgConnection, name_or_email: &str) -> Result<Self, Error> {
-    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::<LocalUserViewTuple>(conn)?;
     Ok(Self {
@@ -79,14 +97,14 @@ impl LocalUserView {
   }
 
   pub fn find_by_email(conn: &PgConnection, from_email: &str) -> Result<Self, Error> {
-    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::<LocalUserViewTuple>(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<Self, Error> {
-    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<Self, Error> {
+    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::<LocalUserSettingsViewTuple>(conn)?;
     Ok(Self {
diff --git a/crates/routes/src/webfinger.rs b/crates/routes/src/webfinger.rs
index c444451e..8ab2a5b6 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 3d9232e6..3529f5f1 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<Jwt, jsonwebtoken::errors::Error> {
+  pub fn jwt(local_user_id: i32, hostname: String) -> Result<Jwt, jsonwebtoken::errors::Error> {
     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 bd2b5684..5e76e8bc 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 0762b948..7198fcb7 100644
--- a/crates/websocket/src/handlers.rs
+++ b/crates/websocket/src/handlers.rs
@@ -155,7 +155,7 @@ impl Handler<JoinUserRoom> for ChatServer {
   type Result = ();
 
   fn handle(&mut self, msg: JoinUserRoom, _: &mut Context<Self>) {
-    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 89f3f2b3..b3d98d06 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,
 }
 
-- 
2.44.1