Dont allow login if account is banned or deleted (fixes #2372) (#2374)

crates/api/src/local_user/login.rs

@@ -3,7 +3,7 @@ use actix_web::web::Data;
 use bcrypt::verify;
 use lemmy_api_common::{
   person::{Login, LoginResponse},
-  utils::{blocking, check_registration_application},
+  utils::{blocking, check_registration_application, check_user_valid},
 };
 use lemmy_db_schema::source::site::Site;
 use lemmy_db_views::structs::LocalUserView;
@@ -39,6 +39,11 @@ impl Perform for Login {
     if !valid {
       return Err(LemmyError::from_message("password_incorrect"));
     }
+    check_user_valid(
+      local_user_view.person.banned,
+      local_user_view.person.ban_expires,
+      local_user_view.person.deleted,
+    )?;
 
     let site = blocking(context.pool(), Site::read_local_site).await??;
     if site.require_email_verification && !local_user_view.local_user.email_verified {

crates/api_common/src/utils.rs

@@ -1,5 +1,7 @@
 use crate::{request::purge_image_from_pictrs, sensitive::Sensitive, site::FederatedInstances};
+use chrono::NaiveDateTime;
 use lemmy_db_schema::{
+  impls::person::is_banned,
   newtypes::{CommunityId, LocalUserId, PersonId, PostId},
   source::{
     comment::Comment,
@@ -129,15 +131,11 @@ pub async fn get_local_user_view_from_jwt(
   let local_user_id = LocalUserId(claims.sub);
   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.is_banned() {
-    return Err(LemmyError::from_message("site_ban"));
-  }
-
-  // Check for user deletion
-  if local_user_view.person.deleted {
-    return Err(LemmyError::from_message("deleted"));
-  }
+  check_user_valid(
+    local_user_view.person.banned,
+    local_user_view.person.ban_expires,
+    local_user_view.person.deleted,
+  )?;
 
   check_validator_time(&local_user_view.local_user.validator_time, &claims)?;
 
@@ -146,7 +144,7 @@ pub async fn get_local_user_view_from_jwt(
 
 /// Checks if user's token was issued before user's password reset.
 pub fn check_validator_time(
-  validator_time: &chrono::NaiveDateTime,
+  validator_time: &NaiveDateTime,
   claims: &Claims,
 ) -> Result<(), LemmyError> {
   let user_validation_time = validator_time.timestamp();
@@ -170,11 +168,13 @@ pub async fn get_local_user_view_from_jwt_opt(
 }
 
 #[tracing::instrument(skip_all)]
-pub async fn get_local_user_settings_view_from_jwt(
-  jwt: &Sensitive<String>,
+pub async fn get_local_user_settings_view_from_jwt_opt(
+  jwt: Option<&Sensitive<String>>,
   pool: &DbPool,
   secret: &Secret,
-) -> Result<LocalUserSettingsView, LemmyError> {
+) -> Result<Option<LocalUserSettingsView>, LemmyError> {
+  match jwt {
+    Some(jwt) => {
       let claims = Claims::decode(jwt.as_ref(), &secret.jwt_secret)
         .map_err(|e| e.with_message("not_logged_in"))?
         .claims;
@@ -183,29 +183,36 @@ pub async fn get_local_user_settings_view_from_jwt(
         LocalUserSettingsView::read(conn, local_user_id)
       })
       .await??;
-  // Check for a site ban
-  if local_user_view.person.is_banned() {
-    return Err(LemmyError::from_message("site_ban"));
-  }
+      check_user_valid(
+        local_user_view.person.banned,
+        local_user_view.person.ban_expires,
+        local_user_view.person.deleted,
+      )?;
 
       check_validator_time(&local_user_view.local_user.validator_time, &claims)?;
 
-  Ok(local_user_view)
+      Ok(Some(local_user_view))
     }
-
-#[tracing::instrument(skip_all)]
-pub async fn get_local_user_settings_view_from_jwt_opt(
-  jwt: Option<&Sensitive<String>>,
-  pool: &DbPool,
-  secret: &Secret,
-) -> Result<Option<LocalUserSettingsView>, LemmyError> {
-  match jwt {
-    Some(jwt) => Ok(Some(
-      get_local_user_settings_view_from_jwt(jwt, pool, secret).await?,
-    )),
     None => Ok(None),
   }
 }
+pub fn check_user_valid(
+  banned: bool,
+  ban_expires: Option<NaiveDateTime>,
+  deleted: bool,
+) -> Result<(), LemmyError> {
+  // Check for a site ban
+  if is_banned(banned, ban_expires) {
+    return Err(LemmyError::from_message("site_ban"));
+  }
+
+  // check for account deletion
+  if deleted {
+    return Err(LemmyError::from_message("deleted"));
+  }
+
+  Ok(())
+}
 
 #[tracing::instrument(skip_all)]
 pub async fn check_community_ban(

crates/db_schema/src/impls/person.rs

@@ -1,7 +1,7 @@
 use crate::{
   newtypes::{DbUrl, PersonId},
   schema::person::dsl::*,
-  source::person::{Person, PersonForm, PersonSafe},
+  source::person::{Person, PersonForm},
   traits::{ApubActor, Crud},
   utils::{functions::lower, naive_now},
 };
@@ -258,10 +258,6 @@ impl Person {
       .get_result::<Self>(conn)
   }
 
-  pub fn is_banned(&self) -> bool {
-    is_banned(self.banned, self.ban_expires)
-  }
-
   pub fn leave_admin(conn: &PgConnection, person_id: PersonId) -> Result<Self, Error> {
     diesel::update(person.find(person_id))
       .set(admin.eq(false))
@@ -278,13 +274,7 @@ impl Person {
   }
 }
 
-impl PersonSafe {
-  pub fn is_banned(&self) -> bool {
-    is_banned(self.banned, self.ban_expires)
-  }
-}
-
-fn is_banned(banned_: bool, expires: Option<chrono::NaiveDateTime>) -> bool {
+pub fn is_banned(banned_: bool, expires: Option<chrono::NaiveDateTime>) -> bool {
   if let Some(expires) = expires {
     banned_ && expires.gt(&naive_now())
   } else {