From b78826c2c80567192b4e2ce5f8714a094299be04 Mon Sep 17 00:00:00 2001 From: Nutomic Date: Thu, 28 Jul 2022 23:14:07 +0200 Subject: [PATCH] Dont allow login if account is banned or deleted (fixes #2372) (#2374) --- crates/api/src/local_user/login.rs | 7 ++- crates/api_common/src/utils.rs | 81 +++++++++++++++------------- crates/db_schema/src/impls/person.rs | 14 +---- 3 files changed, 52 insertions(+), 50 deletions(-) diff --git a/crates/api/src/local_user/login.rs b/crates/api/src/local_user/login.rs index cd17d61ab..06db70e12 100644 --- 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 6e82a117c..c189e71de 100644 --- 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, - pool: &DbPool, - secret: &Secret, -) -> Result { - 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>, @@ -200,12 +174,45 @@ pub async fn get_local_user_settings_view_from_jwt_opt( secret: &Secret, ) -> Result, 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, + 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 73807d2e3..6e4398f68 100644 --- 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::(conn) } - pub fn is_banned(&self) -> bool { - is_banned(self.banned, self.ban_expires) - } - pub fn leave_admin(conn: &PgConnection, person_id: PersonId) -> Result { 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) -> bool { +pub fn is_banned(banned_: bool, expires: Option) -> bool { if let Some(expires) = expires { banned_ && expires.gt(&naive_now()) } else {