From 8101048da08bb050da37d502fa5b8286c654aed3 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Mon, 22 May 2023 13:27:31 +0100 Subject: [PATCH] refactor: [#157] extract service to ban users Decoupling services from actix-web framework. --- src/app.rs | 10 ++++- src/common.rs | 8 +++- src/routes/user.rs | 34 +++------------- src/services/user.rs | 94 +++++++++++++++++++++++++++++++++++++++++++- 4 files changed, 113 insertions(+), 33 deletions(-) diff --git a/src/app.rs b/src/app.rs index 020d50fb..2a7d3d6b 100644 --- a/src/app.rs +++ b/src/app.rs @@ -17,7 +17,7 @@ use crate::services::torrent::{ DbTorrentAnnounceUrlRepository, DbTorrentFileRepository, DbTorrentInfoRepository, DbTorrentListingGenerator, DbTorrentRepository, }; -use crate::services::user::{self, DbUserProfileRepository, DbUserRepository}; +use crate::services::user::{self, DbBannedUserList, DbUserProfileRepository, DbUserRepository}; use crate::services::{proxy, settings, torrent}; use crate::tracker::statistics_importer::StatisticsImporter; use crate::{mailer, routes, tracker}; @@ -65,6 +65,7 @@ pub async fn run(configuration: Configuration) -> Running { let torrent_file_repository = Arc::new(DbTorrentFileRepository::new(database.clone())); let torrent_announce_url_repository = Arc::new(DbTorrentAnnounceUrlRepository::new(database.clone())); let torrent_listing_generator = Arc::new(DbTorrentListingGenerator::new(database.clone())); + let banned_user_list = Arc::new(DbBannedUserList::new(database.clone())); // Services let category_service = Arc::new(category::Service::new(category_repository.clone(), user_repository.clone())); let proxy_service = Arc::new(proxy::Service::new(image_cache_service.clone(), user_repository.clone())); @@ -87,6 +88,11 @@ pub async fn run(configuration: Configuration) -> Running { user_repository.clone(), user_profile_repository.clone(), )); + let ban_service = Arc::new(user::BanService::new( + user_repository.clone(), + user_profile_repository.clone(), + banned_user_list.clone(), + )); // Build app container @@ -107,12 +113,14 @@ pub async fn run(configuration: Configuration) -> Running { torrent_file_repository, torrent_announce_url_repository, torrent_listing_generator, + banned_user_list, // Services category_service, proxy_service, settings_service, torrent_index, registration_service, + ban_service, )); // Start repeating task to import tracker torrent data and updating diff --git a/src/common.rs b/src/common.rs index 4049687d..5db886f3 100644 --- a/src/common.rs +++ b/src/common.rs @@ -9,7 +9,7 @@ use crate::services::torrent::{ DbTorrentAnnounceUrlRepository, DbTorrentFileRepository, DbTorrentInfoRepository, DbTorrentListingGenerator, DbTorrentRepository, }; -use crate::services::user::{self, DbUserProfileRepository, DbUserRepository}; +use crate::services::user::{self, DbBannedUserList, DbUserProfileRepository, DbUserRepository}; use crate::services::{proxy, settings, torrent}; use crate::tracker::statistics_importer::StatisticsImporter; use crate::{mailer, tracker}; @@ -34,12 +34,14 @@ pub struct AppData { pub torrent_file_repository: Arc, pub torrent_announce_url_repository: Arc, pub torrent_listing_generator: Arc, + pub banned_user_list: Arc, // Services pub category_service: Arc, pub proxy_service: Arc, pub settings_service: Arc, pub torrent_service: Arc, pub registration_service: Arc, + pub ban_service: Arc, } impl AppData { @@ -61,12 +63,14 @@ impl AppData { torrent_file_repository: Arc, torrent_announce_url_repository: Arc, torrent_listing_generator: Arc, + banned_user_list: Arc, // Services category_service: Arc, proxy_service: Arc, settings_service: Arc, torrent_service: Arc, registration_service: Arc, + ban_service: Arc, ) -> AppData { AppData { cfg, @@ -85,12 +89,14 @@ impl AppData { torrent_file_repository, torrent_announce_url_repository, torrent_listing_generator, + banned_user_list, // Services category_service, proxy_service, settings_service, torrent_service, registration_service, + ban_service, } } } diff --git a/src/routes/user.rs b/src/routes/user.rs index 017d24cb..5c5973da 100644 --- a/src/routes/user.rs +++ b/src/routes/user.rs @@ -1,6 +1,5 @@ use actix_web::{web, HttpRequest, HttpResponse, Responder}; use argon2::{Argon2, PasswordHash, PasswordVerifier}; -use log::debug; use pbkdf2::Pbkdf2; use serde::{Deserialize, Serialize}; @@ -27,7 +26,7 @@ pub fn init(cfg: &mut web::ServiceConfig) { .service(web::resource("/token/renew").route(web::post().to(renew_token))) // User Access Ban // code-review: should not this be a POST method? We add the user to the blacklist. We do not delete the user. - .service(web::resource("/ban/{user}").route(web::delete().to(ban))), + .service(web::resource("/ban/{user}").route(web::delete().to(ban_handler))), ); } @@ -214,35 +213,12 @@ pub async fn email_verification_handler(req: HttpRequest, app_data: WebAppData) /// /// # Errors /// -/// This function will return a `ServiceError::InternalServerError` if unable get user from the request. -/// This function will return an error if unable to get user profile from supplied username. -/// This function will return an error if unable to ser the ban of the user in the database. -pub async fn ban(req: HttpRequest, app_data: WebAppData) -> ServiceResult { - debug!("banning user"); - - let user = app_data.auth.get_user_compact_from_request(&req).await?; - - // check if user is administrator - if !user.administrator { - return Err(ServiceError::Unauthorized); - } - +/// This function will return if the user could not be banned. +pub async fn ban_handler(req: HttpRequest, app_data: WebAppData) -> ServiceResult { + let user_id = app_data.auth.get_user_id_from_request(&req).await?; let to_be_banned_username = req.match_info().get("user").ok_or(ServiceError::InternalServerError)?; - debug!("user to be banned: {}", to_be_banned_username); - - let user_profile = app_data - .database - .get_user_profile_from_username(to_be_banned_username) - .await?; - - let reason = "no reason".to_string(); - - // user will be banned until the year 9999 - let date_expiry = chrono::NaiveDateTime::parse_from_str("9999-01-01 00:00:00", "%Y-%m-%d %H:%M:%S") - .expect("Could not parse date from 9999-01-01 00:00:00."); - - app_data.database.ban_user(user_profile.user_id, &reason, date_expiry).await?; + app_data.ban_service.ban_user(to_be_banned_username, &user_id).await?; Ok(HttpResponse::Ok().json(OkResponse { data: format!("Banned user: {to_be_banned_username}"), diff --git a/src/services/user.rs b/src/services/user.rs index ca303c0b..fb5c82f6 100644 --- a/src/services/user.rs +++ b/src/services/user.rs @@ -4,7 +4,7 @@ use std::sync::Arc; use argon2::password_hash::SaltString; use argon2::{Argon2, PasswordHasher}; use jsonwebtoken::{decode, Algorithm, DecodingKey, Validation}; -use log::info; +use log::{debug, info}; use pbkdf2::password_hash::rand_core::OsRng; use crate::config::{Configuration, EmailOnSignup}; @@ -12,7 +12,7 @@ use crate::databases::database::{Database, Error}; use crate::errors::ServiceError; use crate::mailer; use crate::mailer::VerifyClaims; -use crate::models::user::{UserCompact, UserId}; +use crate::models::user::{UserCompact, UserId, UserProfile}; use crate::routes::user::RegistrationForm; use crate::utils::regex::validate_email_address; @@ -182,6 +182,56 @@ impl RegistrationService { } } +pub struct BanService { + user_repository: Arc, + user_profile_repository: Arc, + banned_user_list: Arc, +} + +impl BanService { + #[must_use] + pub fn new( + user_repository: Arc, + user_profile_repository: Arc, + banned_user_list: Arc, + ) -> Self { + Self { + user_repository, + user_profile_repository, + banned_user_list, + } + } + + /// Ban a user from the Index. + /// + /// # Errors + /// + /// This function will return a: + /// + /// * `ServiceError::InternalServerError` if unable get user from the request. + /// * An error if unable to get user profile from supplied username. + /// * An error if unable to set the ban of the user in the database. + pub async fn ban_user(&self, username_to_be_banned: &str, user_id: &UserId) -> Result<(), ServiceError> { + debug!("user with ID {user_id} banning username: {username_to_be_banned}"); + + let user = self.user_repository.get_compact(user_id).await?; + + // Check if user is administrator + if !user.administrator { + return Err(ServiceError::Unauthorized); + } + + let user_profile = self + .user_profile_repository + .get_user_profile_from_username(username_to_be_banned) + .await?; + + self.banned_user_list.add(&user_profile.user_id).await?; + + Ok(()) + } +} + pub struct DbUserRepository { database: Arc>, } @@ -252,4 +302,44 @@ impl DbUserProfileRepository { pub async fn verify_email(&self, user_id: &UserId) -> Result<(), Error> { self.database.verify_email(*user_id).await } + + /// It get the user profile from the username. + /// + /// # Errors + /// + /// It returns an error if there is a database error. + pub async fn get_user_profile_from_username(&self, username: &str) -> Result { + self.database.get_user_profile_from_username(username).await + } +} + +pub struct DbBannedUserList { + database: Arc>, +} + +impl DbBannedUserList { + #[must_use] + pub fn new(database: Arc>) -> Self { + Self { database } + } + + /// It add a user to the banned users list. + /// + /// # Errors + /// + /// It returns an error if there is a database error. + pub async fn add(&self, user_id: &UserId) -> Result<(), Error> { + // todo: add reason and `date_expiry` parameters to request. + + // code-review: add the user ID of the user who banned the user. + + // For the time being, we will not use a reason for banning a user. + let reason = "no reason".to_string(); + + // User will be banned until the year 9999 + let date_expiry = chrono::NaiveDateTime::parse_from_str("9999-01-01 00:00:00", "%Y-%m-%d %H:%M:%S") + .expect("Could not parse date from 9999-01-01 00:00:00."); + + self.database.ban_user(*user_id, &reason, date_expiry).await + } }