refactor: [torrust#157] extract service for user registration

Decoupling services from actix-web framework.

src/app.rs

@@ -17,7 +17,7 @@ use crate::services::torrent::{
     DbTorrentAnnounceUrlRepository, DbTorrentFileRepository, DbTorrentInfoRepository, DbTorrentListingGenerator,
     DbTorrentRepository,
 };
-use crate::services::user::DbUserRepository;
+use crate::services::user::{self, DbUserProfileRepository, DbUserRepository};
 use crate::services::{proxy, settings, torrent};
 use crate::tracker::statistics_importer::StatisticsImporter;
 use crate::{mailer, routes, tracker};
@@ -28,6 +28,7 @@ pub struct Running {
     pub tracker_data_importer_handle: tokio::task::JoinHandle<()>,
 }
 
+#[allow(clippy::too_many_lines)]
 pub async fn run(configuration: Configuration) -> Running {
     logging::setup();
 
@@ -58,6 +59,7 @@ pub async fn run(configuration: Configuration) -> Running {
     // Repositories
     let category_repository = Arc::new(DbCategoryRepository::new(database.clone()));
     let user_repository = Arc::new(DbUserRepository::new(database.clone()));
+    let user_profile_repository = Arc::new(DbUserProfileRepository::new(database.clone()));
     let torrent_repository = Arc::new(DbTorrentRepository::new(database.clone()));
     let torrent_info_repository = Arc::new(DbTorrentInfoRepository::new(database.clone()));
     let torrent_file_repository = Arc::new(DbTorrentFileRepository::new(database.clone()));
@@ -79,6 +81,12 @@ pub async fn run(configuration: Configuration) -> Running {
         torrent_announce_url_repository.clone(),
         torrent_listing_generator.clone(),
     ));
+    let registration_service = Arc::new(user::RegistrationService::new(
+        configuration.clone(),
+        mailer_service.clone(),
+        user_repository.clone(),
+        user_profile_repository.clone(),
+    ));
 
     // Build app container
 
@@ -93,6 +101,7 @@ pub async fn run(configuration: Configuration) -> Running {
         // Repositories
         category_repository,
         user_repository,
+        user_profile_repository,
         torrent_repository,
         torrent_info_repository,
         torrent_file_repository,
@@ -103,6 +112,7 @@ pub async fn run(configuration: Configuration) -> Running {
         proxy_service,
         settings_service,
         torrent_index,
+        registration_service,
     ));
 
     // Start repeating task to import tracker torrent data and updating

src/common.rs

@@ -9,7 +9,7 @@ use crate::services::torrent::{
     DbTorrentAnnounceUrlRepository, DbTorrentFileRepository, DbTorrentInfoRepository, DbTorrentListingGenerator,
     DbTorrentRepository,
 };
-use crate::services::user::DbUserRepository;
+use crate::services::user::{self, DbUserProfileRepository, DbUserRepository};
 use crate::services::{proxy, settings, torrent};
 use crate::tracker::statistics_importer::StatisticsImporter;
 use crate::{mailer, tracker};
@@ -28,6 +28,7 @@ pub struct AppData {
     // Repositories
     pub category_repository: Arc<DbCategoryRepository>,
     pub user_repository: Arc<DbUserRepository>,
+    pub user_profile_repository: Arc<DbUserProfileRepository>,
     pub torrent_repository: Arc<DbTorrentRepository>,
     pub torrent_info_repository: Arc<DbTorrentInfoRepository>,
     pub torrent_file_repository: Arc<DbTorrentFileRepository>,
@@ -38,6 +39,7 @@ pub struct AppData {
     pub proxy_service: Arc<proxy::Service>,
     pub settings_service: Arc<settings::Service>,
     pub torrent_service: Arc<torrent::Index>,
+    pub registration_service: Arc<user::RegistrationService>,
 }
 
 impl AppData {
@@ -53,6 +55,7 @@ impl AppData {
         // Repositories
         category_repository: Arc<DbCategoryRepository>,
         user_repository: Arc<DbUserRepository>,
+        user_profile_repository: Arc<DbUserProfileRepository>,
         torrent_repository: Arc<DbTorrentRepository>,
         torrent_info_repository: Arc<DbTorrentInfoRepository>,
         torrent_file_repository: Arc<DbTorrentFileRepository>,
@@ -63,6 +66,7 @@ impl AppData {
         proxy_service: Arc<proxy::Service>,
         settings_service: Arc<settings::Service>,
         torrent_service: Arc<torrent::Index>,
+        registration_service: Arc<user::RegistrationService>,
     ) -> AppData {
         AppData {
             cfg,
@@ -75,6 +79,7 @@ impl AppData {
             // Repositories
             category_repository,
             user_repository,
+            user_profile_repository,
             torrent_repository,
             torrent_info_repository,
             torrent_file_repository,
@@ -85,6 +90,7 @@ impl AppData {
             proxy_service,
             settings_service,
             torrent_service,
+            registration_service,
         }
     }
 }

src/databases/database.rs

@@ -101,7 +101,7 @@ pub trait Database: Sync + Send {
         Self: Sized;
 
     /// Add new user and return the newly inserted `user_id`.
-    async fn insert_user_and_get_id(&self, username: &str, email: &str, password: &str) -> Result<i64, Error>;
+    async fn insert_user_and_get_id(&self, username: &str, email: &str, password: &str) -> Result<UserId, Error>;
 
     /// Get `User` from `user_id`.
     async fn get_user_from_id(&self, user_id: i64) -> Result<User, Error>;

src/mailer.rs

@@ -9,6 +9,7 @@ use serde::{Deserialize, Serialize};
 
 use crate::config::Configuration;
 use crate::errors::ServiceError;
+use crate::routes::API_VERSION;
 use crate::utils::clock;
 
 pub struct Service {
@@ -146,7 +147,7 @@ impl Service {
             base_url = cfg_base_url;
         }
 
-        format!("{base_url}/user/email/verify/{token}")
+        format!("{base_url}/{API_VERSION}/user/email/verify/{token}")
     }
 }
 

src/routes/user.rs

@@ -1,37 +1,38 @@
 use actix_web::{web, HttpRequest, HttpResponse, Responder};
-use argon2::password_hash::SaltString;
-use argon2::{Argon2, PasswordHash, PasswordHasher, PasswordVerifier};
-use jsonwebtoken::{decode, Algorithm, DecodingKey, Validation};
-use log::{debug, info};
-use pbkdf2::password_hash::rand_core::OsRng;
+use argon2::{Argon2, PasswordHash, PasswordVerifier};
+use log::debug;
 use pbkdf2::Pbkdf2;
 use serde::{Deserialize, Serialize};
 
 use crate::common::WebAppData;
-use crate::config::EmailOnSignup;
 use crate::errors::{ServiceError, ServiceResult};
-use crate::mailer::VerifyClaims;
 use crate::models::response::{OkResponse, TokenResponse};
 use crate::models::user::UserAuthentication;
 use crate::routes::API_VERSION;
 use crate::utils::clock;
-use crate::utils::regex::validate_email_address;
 
 pub fn init(cfg: &mut web::ServiceConfig) {
     cfg.service(
         web::scope(&format!("/{API_VERSION}/user"))
-            .service(web::resource("/register").route(web::post().to(register)))
+            // Registration
+            .service(web::resource("/register").route(web::post().to(registration_handler)))
+            // code-review: should this be part of the REST API?
+            // - This endpoint should only verify the email.
+            // - There should be an independent service (web app) serving the email verification page.
+            //   The wep app can user this endpoint to verify the email and render the page accordingly.
+            .service(web::resource("/email/verify/{token}").route(web::get().to(email_verification_handler)))
+            // Authentication
             .service(web::resource("/login").route(web::post().to(login)))
-            // 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("/token/verify").route(web::post().to(verify_token)))
             .service(web::resource("/token/renew").route(web::post().to(renew_token)))
-            .service(web::resource("/email/verify/{token}").route(web::get().to(verify_email))),
+            // 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))),
     );
 }
 
 #[derive(Clone, Debug, Deserialize, Serialize)]
-pub struct Register {
+pub struct RegistrationForm {
     pub username: String,
     pub email: Option<String>,
     pub password: String,
@@ -53,93 +54,21 @@ pub struct Token {
 ///
 /// # Errors
 ///
-/// This function will return a `ServiceError::EmailMissing` if email is required, but missing.
-/// This function will return a `ServiceError::EmailInvalid` if supplied email is badly formatted.
-/// This function will return a `ServiceError::PasswordsDontMatch` if the supplied passwords do not match.
-/// This function will return a `ServiceError::PasswordTooShort` if the supplied password is too short.
-/// This function will return a `ServiceError::PasswordTooLong` if the supplied password is too long.
-/// This function will return a `ServiceError::UsernameInvalid` if the supplied username is badly formatted.
-/// This function will return an error if unable to successfully hash the password.
-/// This function will return an error if unable to insert user into the database.
-/// This function will return a `ServiceError::FailedToSendVerificationEmail` if unable to send the required verification email.
-pub async fn register(req: HttpRequest, mut payload: web::Json<Register>, app_data: WebAppData) -> ServiceResult<impl Responder> {
-    info!("registering user: {}", payload.username);
-
-    let settings = app_data.cfg.settings.read().await;
-
-    match settings.auth.email_on_signup {
-        EmailOnSignup::Required => {
-            if payload.email.is_none() {
-                return Err(ServiceError::EmailMissing);
-            }
-        }
-        EmailOnSignup::None => payload.email = None,
-        EmailOnSignup::Optional => {}
-    }
-
-    if let Some(email) = &payload.email {
-        // check if email address is valid
-        if !validate_email_address(email) {
-            return Err(ServiceError::EmailInvalid);
-        }
-    }
-
-    if payload.password != payload.confirm_password {
-        return Err(ServiceError::PasswordsDontMatch);
-    }
-
-    let password_length = payload.password.len();
-
-    if password_length <= settings.auth.min_password_length {
-        return Err(ServiceError::PasswordTooShort);
-    }
-
-    if password_length >= settings.auth.max_password_length {
-        return Err(ServiceError::PasswordTooLong);
-    }
-
-    let salt = SaltString::generate(&mut OsRng);
-
-    // Argon2 with default params (Argon2id v19)
-    let argon2 = Argon2::default();
-
-    // Hash password to PHC string ($argon2id$v=19$...)
-    let password_hash = argon2.hash_password(payload.password.as_bytes(), &salt)?.to_string();
-
-    if payload.username.contains('@') {
-        return Err(ServiceError::UsernameInvalid);
-    }
-
-    let email = payload.email.as_ref().unwrap_or(&String::new()).to_string();
-
-    let user_id = app_data
-        .database
-        .insert_user_and_get_id(&payload.username, &email, &password_hash)
-        .await?;
-
-    // if this is the first created account, give administrator rights
-    if user_id == 1 {
-        let _ = app_data.database.grant_admin_role(user_id).await;
-    }
-
+/// This function will return an error if the user could not be registered.
+pub async fn registration_handler(
+    req: HttpRequest,
+    registration_form: web::Json<RegistrationForm>,
+    app_data: WebAppData,
+) -> ServiceResult<impl Responder> {
     let conn_info = req.connection_info().clone();
+    // todo: we should add this in the configuration. It does not work is the
+    // server is behind a reverse proxy.
+    let api_base_url = format!("{}://{}", conn_info.scheme(), conn_info.host());
 
-    if settings.mail.email_verification_enabled && payload.email.is_some() {
-        let mail_res = app_data
-            .mailer
-            .send_verification_mail(
-                payload.email.as_ref().expect("variable `email` is checked above"),
-                &payload.username,
-                user_id,
-                format!("{}://{}", conn_info.scheme(), conn_info.host()).as_str(),
-            )
-            .await;
-
-        if mail_res.is_err() {
-            let _ = app_data.database.delete_user(user_id).await;
-            return Err(ServiceError::FailedToSendVerificationEmail);
-        }
-    }
+    let _user_id = app_data
+        .registration_service
+        .register_user(&registration_form, &api_base_url)
+        .await?;
 
     Ok(HttpResponse::Ok())
 }
@@ -266,35 +195,17 @@ pub async fn renew_token(payload: web::Json<Token>, app_data: WebAppData) -> Ser
     }))
 }
 
-pub async fn verify_email(req: HttpRequest, app_data: WebAppData) -> String {
-    let settings = app_data.cfg.settings.read().await;
+pub async fn email_verification_handler(req: HttpRequest, app_data: WebAppData) -> String {
+    // Get token from URL path
     let token = match req.match_info().get("token").ok_or(ServiceError::InternalServerError) {
         Ok(token) => token,
         Err(err) => return err.to_string(),
     };
 
-    let token_data = match decode::<VerifyClaims>(
-        token,
-        &DecodingKey::from_secret(settings.auth.secret_key.as_bytes()),
-        &Validation::new(Algorithm::HS256),
-    ) {
-        Ok(token_data) => {
-            if !token_data.claims.iss.eq("email-verification") {
-                return ServiceError::TokenInvalid.to_string();
-            }
-
-            token_data.claims
-        }
-        Err(_) => return ServiceError::TokenInvalid.to_string(),
-    };
-
-    drop(settings);
-
-    if app_data.database.verify_email(token_data.sub).await.is_err() {
-        return ServiceError::InternalServerError.to_string();
-    };
-
-    String::from("Email verified, you can close this page.")
+    match app_data.registration_service.verify_email(token).await {
+        Ok(_) => String::from("Email verified, you can close this page."),
+        Err(error) => error.to_string(),
+    }
 }
 
 /// Ban a user from the Index

src/services/category.rs

@@ -30,7 +30,7 @@ impl Service {
     /// * The user does not have the required permissions.
     /// * There is a database error.
     pub async fn add_category(&self, category_name: &str, user_id: &UserId) -> Result<i64, ServiceError> {
-        let user = self.user_repository.get_compact_user(user_id).await?;
+        let user = self.user_repository.get_compact(user_id).await?;
 
         // Check if user is administrator
         // todo: extract authorization service
@@ -56,7 +56,7 @@ impl Service {
     /// * The user does not have the required permissions.
     /// * There is a database error.
     pub async fn delete_category(&self, category_name: &str, user_id: &UserId) -> Result<(), ServiceError> {
-        let user = self.user_repository.get_compact_user(user_id).await?;
+        let user = self.user_repository.get_compact(user_id).await?;
 
         // Check if user is administrator
         // todo: extract authorization service

src/services/proxy.rs

@@ -39,7 +39,7 @@ impl Service {
     /// * The image is too big.
     /// * The user quota is met.
     pub async fn get_image_by_url(&self, url: &str, user_id: &UserId) -> Result<Bytes, Error> {
-        let user = self.user_repository.get_compact_user(user_id).await.ok();
+        let user = self.user_repository.get_compact(user_id).await.ok();
 
         self.image_cache_service.get_image_by_url(url, user).await
     }

src/services/settings.rs

@@ -25,7 +25,7 @@ impl Service {
     ///
     /// It returns an error if the user does not have the required permissions.
     pub async fn get_all(&self, user_id: &UserId) -> Result<TorrustBackend, ServiceError> {
-        let user = self.user_repository.get_compact_user(user_id).await?;
+        let user = self.user_repository.get_compact(user_id).await?;
 
         // Check if user is administrator
         // todo: extract authorization service
@@ -42,7 +42,7 @@ impl Service {
     ///
     /// It returns an error if the user does not have the required permissions.
     pub async fn update_all(&self, torrust_backend: TorrustBackend, user_id: &UserId) -> Result<TorrustBackend, ServiceError> {
-        let user = self.user_repository.get_compact_user(user_id).await?;
+        let user = self.user_repository.get_compact(user_id).await?;
 
         // Check if user is administrator
         // todo: extract authorization service