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

diff --git a/crates/api/src/local_user/login.rs b/crates/api/src/local_user/login.rs
index cd17d61ab711d79693f9fe8548de5ef377fc2c9a..06db70e125ff978481ce6cab64fafb2d3f20455a 100644 (file)
--- a/crates/api/src/local_user/login.rs
+++ b/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 {

diff --git a/crates/api_common/src/utils.rs b/crates/api_common/src/utils.rs
index 6e82a117ca52e273137118b9d874f154037d7404..c189e71de99a281d81cbc752bc4195014646f934 100644 (file)
--- a/crates/api_common/src/utils.rs
+++ b/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();
@@ -169,30 +167,6 @@ 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>,
-  pool: &DbPool,
-  secret: &Secret,
-) -> Result<LocalUserSettingsView, LemmyError> {
-  let claims = Claims::decode(jwt.as_ref(), &secret.jwt_secret)
-    .map_err(|e| e.with_message("not_logged_in"))?
-    .claims;
-  let local_user_id = LocalUserId(claims.sub);
-  let local_user_view = blocking(pool, move |conn| {
-    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_validator_time(&local_user_view.local_user.validator_time, &claims)?;
-
-  Ok(local_user_view)
-}
-
 #[tracing::instrument(skip_all)]
 pub async fn get_local_user_settings_view_from_jwt_opt(
   jwt: Option<&Sensitive<String>>,
@@ -200,12 +174,45 @@ pub async fn get_local_user_settings_view_from_jwt_opt(
   secret: &Secret,
 ) -> Result<Option<LocalUserSettingsView>, LemmyError> {
   match jwt {
-    Some(jwt) => Ok(Some(
-      get_local_user_settings_view_from_jwt(jwt, pool, secret).await?,
-    )),
+    Some(jwt) => {
+      let claims = Claims::decode(jwt.as_ref(), &secret.jwt_secret)
+        .map_err(|e| e.with_message("not_logged_in"))?
+        .claims;
+      let local_user_id = LocalUserId(claims.sub);
+      let local_user_view = blocking(pool, move |conn| {
+        LocalUserSettingsView::read(conn, local_user_id)
+      })
+      .await??;
+      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(Some(local_user_view))
+    }
     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(

diff --git a/crates/db_schema/src/impls/person.rs b/crates/db_schema/src/impls/person.rs
index 73807d2e39f1542fc15bf7adf8aab8e03229bb1b..6e4398f6834fafcc3fcb051b79daf80fcd7c06ee 100644 (file)
--- a/crates/db_schema/src/impls/person.rs
+++ b/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 {