From 9957ea232f713af73bc02f2f2917c4b6aec73c03 Mon Sep 17 00:00:00 2001 From: Sebastian Pravda Date: Sun, 27 Nov 2022 22:01:11 +0100 Subject: [PATCH] refactor: code cleanup --- api/src/routes/admin.rs | 12 +- api/src/routes/candidate.rs | 134 ++++++----------------- api/src/routes/mod.rs | 12 +- core/src/candidate_details.rs | 24 ++-- core/src/database/mutation/candidate.rs | 2 +- core/src/database/mutation/parent.rs | 2 +- core/src/services/application_service.rs | 10 +- core/src/services/candidate_service.rs | 22 ++-- core/src/util.rs | 10 ++ 9 files changed, 85 insertions(+), 143 deletions(-) diff --git a/api/src/routes/admin.rs b/api/src/routes/admin.rs index 904527c..15f6823 100644 --- a/api/src/routes/admin.rs +++ b/api/src/routes/admin.rs @@ -13,6 +13,8 @@ use sea_orm_rocket::Connection; use crate::{guards::request::{auth::AdminAuth}, pool::Db, requests}; +use super::to_custom_error; + #[post("/login", data = "")] pub async fn login( conn: Connection<'_, Db>, @@ -100,7 +102,7 @@ pub async fn create_candidate( form.personal_id_number.clone(), ) .await - .map_err(|e| Custom(Status::InternalServerError, e.to_string()))?; + .map_err(to_custom_error)?; Ok( Json( @@ -131,7 +133,7 @@ pub async fn list_candidates( let candidates = CandidateService::list_candidates(private_key, db, field, page) .await - .map_err(|e| Custom(Status::from_code(e.code()).unwrap(), e.to_string()))?; + .map_err(to_custom_error)?; Ok(Json(candidates)) } @@ -151,7 +153,7 @@ pub async fn get_candidate( id ) .await - .map_err(|e| Custom(Status::from_code(e.code()).unwrap(), e.to_string()))?; + .map_err(to_custom_error)?; Ok(Json(details)) } @@ -167,7 +169,7 @@ pub async fn reset_candidate_password( let response = CandidateService::reset_password(private_key, db, id) .await - .map_err(|e| Custom(Status::from_code(e.code()).unwrap(), e.to_string()))?; + .map_err(to_custom_error)?; Ok( Json(response) @@ -183,7 +185,7 @@ pub async fn get_candidate_portfolio( let portfolio = PortfolioService::get_portfolio(id, private_key) .await - .map_err(|e| Custom(Status::from_code(e.code()).unwrap(), e.to_string()))?; + .map_err(to_custom_error)?; Ok(portfolio) } diff --git a/api/src/routes/candidate.rs b/api/src/routes/candidate.rs index 2bb1233..93b4d6f 100644 --- a/api/src/routes/candidate.rs +++ b/api/src/routes/candidate.rs @@ -16,6 +16,8 @@ use crate::guards::data::letter::Letter; use crate::guards::data::portfolio::Portfolio; use crate::{guards::request::auth::CandidateAuth, pool::Db, requests}; +use super::to_custom_error; + #[post("/login", data = "")] pub async fn login( conn: Connection<'_, Db>, @@ -25,31 +27,19 @@ pub async fn login( ) -> Result> { let ip_addr: SocketAddr = SocketAddr::new(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), 0); let db = conn.into_inner(); - let session_token_key = CandidateService::login( + let (session_token, private_key) = CandidateService::login( db, login_form.application_id, login_form.password.to_string(), ip_addr.ip().to_string(), ) - .await; - - let Ok(session_token_key) = session_token_key else { - let e = session_token_key.unwrap_err(); - return Err(Custom( - Status::from_code(e.code()).unwrap_or(Status::InternalServerError), - e.to_string(), - )); - }; - - let session_token = session_token_key.0; - let private_key = session_token_key.1; + .await + .map_err(to_custom_error)?; cookies.add_private(Cookie::new("id", session_token.clone())); cookies.add_private(Cookie::new("key", private_key.clone())); - let response = format!("{} {}", session_token, private_key); - - return Ok(response); + return Ok("".to_string()); } #[post("/logout")] @@ -61,14 +51,14 @@ pub async fn logout(conn: Connection<'_, Db>, _session: CandidateAuth, cookies: let session_id = Uuid::try_parse(cookie.value()) // unwrap would be safe here because of the auth guard .map_err(|e| Custom(Status::BadRequest, e.to_string()))?; - let res = CandidateService::logout(db, session_id) + CandidateService::logout(db, session_id) .await - .map_err(|e| Custom(Status::from_code(e.code()).unwrap_or(Status::InternalServerError), e.to_string()))?; + .map_err(to_custom_error)?; cookies.remove_private(Cookie::named("id")); cookies.remove_private(Cookie::named("key")); - Ok(res) + Ok(()) } #[get("/whoami")] @@ -86,19 +76,11 @@ pub async fn post_details( ) -> Result, Custom> { let db = conn.into_inner(); let form = details.into_inner(); - let candidate: entity::candidate::Model = session.into(); // TODO: don't return candidate from session + let candidate: entity::candidate::Model = session.into(); - let candidate_parent = - ApplicationService::add_all_details(db, candidate.application, &form).await; - - if candidate_parent.is_err() { - // TODO cleanup - let e = candidate_parent.err().unwrap(); - return Err(Custom( - Status::from_code(e.code()).unwrap_or_default(), - e.to_string(), - )); - } + let _candidate_parent = ApplicationService::add_all_details(db, candidate.application, &form) + .await + .map_err(to_custom_error)?; Ok( Json(form) @@ -117,14 +99,10 @@ pub async fn get_details( // let handle = tokio::spawn(async move { let details = ApplicationService::decrypt_all_details(private_key, db, candidate.application) .await - .map_err(|e| { - Custom( - Status::from_code(e.code()).unwrap_or_default(), - e.to_string(), - ) - }); + .map(|x| Json(x)) + .map_err(to_custom_error); - details.map(|d| Json(d)) + details } #[post("/cover_letter", data = "")] pub async fn upload_cover_letter( @@ -133,40 +111,25 @@ pub async fn upload_cover_letter( ) -> Result> { let candidate: entity::candidate::Model = session.into(); - let candidate = - PortfolioService::add_cover_letter_to_cache(candidate.application, letter.into()).await; - - if candidate.is_err() { - // TODO cleanup - let e = candidate.err().unwrap(); - return Err(Custom( - Status::from_code(e.code()).unwrap_or_default(), - e.to_string(), - )); - } + PortfolioService::add_cover_letter_to_cache(candidate.application, letter.into()) + .await + .map_err(to_custom_error)?; Ok("Letter added".to_string()) } #[get("/submission_progress")] pub async fn submission_progress( - conn: Connection<'_, Db>, session: CandidateAuth ) -> Result, Custom> { let candidate: entity::candidate::Model = session.into(); let progress = PortfolioService::get_submission_progress(candidate.application) .await - .map_err(|e| { - Custom( - Status::from_code(e.code()).unwrap_or_default(), - e.to_string(), - ) - })?; + .map(|x| Json(x)) + .map_err(to_custom_error); - Ok( - Json(progress) - ) + progress } // TODO: JSON #[get["/is_cover_letter"]] @@ -185,17 +148,9 @@ pub async fn upload_portfolio_letter( ) -> Result> { let candidate: entity::candidate::Model = session.into(); - let candidate = - PortfolioService::add_portfolio_letter_to_cache(candidate.application, letter.into()).await; - - if candidate.is_err() { - // TODO cleanup - let e = candidate.err().unwrap(); - return Err(Custom( - Status::from_code(e.code()).unwrap_or_default(), - e.to_string(), - )); - } + PortfolioService::add_portfolio_letter_to_cache(candidate.application, letter.into()) + .await + .map_err(to_custom_error)?; Ok("Letter added".to_string()) } @@ -217,17 +172,9 @@ pub async fn upload_portfolio_zip( ) -> Result> { let candidate: entity::candidate::Model = session.into(); - let candidate = - PortfolioService::add_portfolio_zip_to_cache(candidate.application, portfolio.into()).await; - - if candidate.is_err() { - // TODO cleanup - let e = candidate.err().unwrap(); - return Err(Custom( - Status::from_code(e.code()).unwrap_or_default(), - e.to_string(), - )); - } + PortfolioService::add_portfolio_zip_to_cache(candidate.application, portfolio.into()) + .await + .map_err(to_custom_error)?; Ok("Portfolio added".to_string()) } @@ -259,19 +206,15 @@ pub async fn submit_portfolio( // TODO: Více kontrol? if e.code() == 500 { // Cleanup - PortfolioService::delete_portfolio(candidate.application) - .await - .unwrap(); + PortfolioService::delete_portfolio(candidate.application).await.unwrap(); } - return Err(Custom( - Status::from_code(e.code()).unwrap_or_default(), - e.to_string(), - )); + return Err(to_custom_error(e)); } Ok("Portfolio submitted".to_string()) } +#[deprecated = "Use /submission_progress instead"] #[get("/is_prepared")] pub async fn is_portfolio_prepared(session: CandidateAuth) -> Result> { let candidate: entity::candidate::Model = session.into(); @@ -289,6 +232,7 @@ pub async fn is_portfolio_prepared(session: CandidateAuth) -> Result Result> { let candidate: entity::candidate::Model = session.into(); @@ -311,17 +255,11 @@ pub async fn download_portfolio(session: CandidateAuth) -> Result, Custo let private_key = session.get_private_key(); let candidate: entity::candidate::Model = session.into(); - let file = PortfolioService::get_portfolio(candidate.application, private_key).await; + let file = PortfolioService::get_portfolio(candidate.application, private_key) + .await + .map_err(to_custom_error); - if file.is_err() { - let e = file.err().unwrap(); - return Err(Custom( - Status::from_code(e.code()).unwrap_or_default(), - e.to_string(), - )); - } - - Ok(file.unwrap()) + file } #[cfg(test)] diff --git a/api/src/routes/mod.rs b/api/src/routes/mod.rs index 90ca91f..24af610 100644 --- a/api/src/routes/mod.rs +++ b/api/src/routes/mod.rs @@ -1,2 +1,12 @@ +use portfolio_core::error::ServiceError; +use rocket::{response::status::Custom, http::Status}; + pub mod admin; -pub mod candidate; \ No newline at end of file +pub mod candidate; + +pub fn to_custom_error(e: ServiceError) -> Custom { + Custom( + Status::from_code(e.code()).unwrap_or_default(), + e.to_string() + ) +} \ No newline at end of file diff --git a/core/src/candidate_details.rs b/core/src/candidate_details.rs index fb3b071..0f7db19 100644 --- a/core/src/candidate_details.rs +++ b/core/src/candidate_details.rs @@ -11,7 +11,8 @@ pub const NAIVE_DATE_FMT: &str = "%Y-%m-%d"; pub struct EncryptedString(String); impl EncryptedString { - pub async fn new(s: &str, recipients: &Vec<&str>) -> Result { + pub async fn new(s: &str, recipients: &Vec) -> Result { + let recipients = recipients.iter().map(|s| &**s).collect(); match crypto::encrypt_password_with_recipients(&s, &recipients).await { Ok(encrypted) => Ok(Self(encrypted)), Err(_) => Err(ServiceError::CryptoEncryptFailed), @@ -89,7 +90,7 @@ pub struct EncryptedApplicationDetails { impl EncryptedApplicationDetails { pub async fn new( form: &ApplicationDetails, - recipients: Vec<&str>, + recipients: Vec, ) -> Result { let birthdate_str = form.birthdate.format(NAIVE_DATE_FMT).to_string(); let d = tokio::try_join!( @@ -250,13 +251,15 @@ pub struct ApplicationDetails { pub mod tests { use std::sync::Mutex; - use chrono::NaiveDate; use once_cell::sync::Lazy; use crate::crypto; use super::{ApplicationDetails, EncryptedApplicationDetails, EncryptedString}; + const PUBLIC_KEY: &str = "age1u889gp407hsz309wn09kxx9anl6uns30m27lfwnctfyq9tq4qpus8tzmq5"; + const PRIVATE_KEY: &str = "AGE-SECRET-KEY-14QG24502DMUUQDT2SPMX2YXPSES0X8UD6NT0PCTDAT6RH8V5Q3GQGSRXPS"; + pub static APPLICATION_DETAILS: Lazy> = Lazy::new(|| Mutex::new(ApplicationDetails { name: "name".to_string(), @@ -297,12 +300,9 @@ pub mod tests { #[tokio::test] async fn test_encrypted_application_details_new() { - const PUBLIC_KEY: &str = "age1u889gp407hsz309wn09kxx9anl6uns30m27lfwnctfyq9tq4qpus8tzmq5"; - const PRIVATE_KEY: &str = - "AGE-SECRET-KEY-14QG24502DMUUQDT2SPMX2YXPSES0X8UD6NT0PCTDAT6RH8V5Q3GQGSRXPS"; let encrypted_details = EncryptedApplicationDetails::new( &APPLICATION_DETAILS.lock().unwrap().clone(), - vec![PUBLIC_KEY], + vec![PUBLIC_KEY.to_string()], ) .await .unwrap(); @@ -329,13 +329,9 @@ pub mod tests { #[tokio::test] async fn test_encrypted_application_details_decrypt() { - const PUBLIC_KEY: &str = "age1u889gp407hsz309wn09kxx9anl6uns30m27lfwnctfyq9tq4qpus8tzmq5"; - const PRIVATE_KEY: &str = - "AGE-SECRET-KEY-14QG24502DMUUQDT2SPMX2YXPSES0X8UD6NT0PCTDAT6RH8V5Q3GQGSRXPS"; - let encrypted_details = EncryptedApplicationDetails::new( &APPLICATION_DETAILS.lock().unwrap().clone(), - vec![PUBLIC_KEY], + vec![PUBLIC_KEY.to_string()], ) .await .unwrap(); @@ -377,7 +373,7 @@ pub mod tests { const PRIVATE_KEY: &str = "AGE-SECRET-KEY-14QG24502DMUUQDT2SPMX2YXPSES0X8UD6NT0PCTDAT6RH8V5Q3GQGSRXPS"; - let encrypted = EncryptedString::new("test", &vec![PUBLIC_KEY]) + let encrypted = EncryptedString::new("test", &vec![PUBLIC_KEY.to_string()]) .await .unwrap(); @@ -395,7 +391,7 @@ pub mod tests { const PRIVATE_KEY: &str = "AGE-SECRET-KEY-14QG24502DMUUQDT2SPMX2YXPSES0X8UD6NT0PCTDAT6RH8V5Q3GQGSRXPS"; - let encrypted = EncryptedString::new("test", &vec![PUBLIC_KEY]) + let encrypted = EncryptedString::new("test", &vec![PUBLIC_KEY.to_string()]) .await .unwrap(); diff --git a/core/src/database/mutation/candidate.rs b/core/src/database/mutation/candidate.rs index a687ebe..8d85bab 100644 --- a/core/src/database/mutation/candidate.rs +++ b/core/src/database/mutation/candidate.rs @@ -114,7 +114,7 @@ mod tests { let encrypted_details: EncryptedApplicationDetails = EncryptedApplicationDetails::new( &APPLICATION_DETAILS.lock().unwrap().clone(), - vec!["age1u889gp407hsz309wn09kxx9anl6uns30m27lfwnctfyq9tq4qpus8tzmq5"], + vec!["age1u889gp407hsz309wn09kxx9anl6uns30m27lfwnctfyq9tq4qpus8tzmq5".to_string()], ).await.unwrap(); Mutation::add_candidate_details(&db, candidate, encrypted_details).await.unwrap(); diff --git a/core/src/database/mutation/parent.rs b/core/src/database/mutation/parent.rs index 4c49ded..51586c8 100644 --- a/core/src/database/mutation/parent.rs +++ b/core/src/database/mutation/parent.rs @@ -83,7 +83,7 @@ mod tests { let encrypted_details: EncryptedApplicationDetails = EncryptedApplicationDetails::new( &APPLICATION_DETAILS.lock().unwrap().clone(), - vec!["age1u889gp407hsz309wn09kxx9anl6uns30m27lfwnctfyq9tq4qpus8tzmq5"], + vec!["age1u889gp407hsz309wn09kxx9anl6uns30m27lfwnctfyq9tq4qpus8tzmq5".to_string()], ) .await .unwrap(); diff --git a/core/src/services/application_service.rs b/core/src/services/application_service.rs index 5eedd36..85b54e4 100644 --- a/core/src/services/application_service.rs +++ b/core/src/services/application_service.rs @@ -1,7 +1,7 @@ use entity::{candidate, parent}; use sea_orm::DbConn; -use crate::{error::ServiceError, candidate_details::{ApplicationDetails, EncryptedApplicationDetails}, Query}; +use crate::{error::ServiceError, candidate_details::{ApplicationDetails, EncryptedApplicationDetails}, Query, util::get_recipients}; use super::{parent_service::ParentService, candidate_service::CandidateService}; @@ -41,13 +41,7 @@ impl ApplicationService { .await? .ok_or(ServiceError::ParentNotFound)?; - let admin_public_keys = Query::get_all_admin_public_keys(db).await?; - - let mut admin_public_keys_refrence: Vec<&str> = - admin_public_keys.iter().map(|s| &**s).collect(); - - let mut recipients = vec![&*candidate.public_key]; - recipients.append(&mut admin_public_keys_refrence); + let recipients = get_recipients(db, &candidate.public_key).await?; let enc_details = EncryptedApplicationDetails::new(form, recipients).await?; diff --git a/core/src/services/candidate_service.rs b/core/src/services/candidate_service.rs index ee4be52..80c37b3 100644 --- a/core/src/services/candidate_service.rs +++ b/core/src/services/candidate_service.rs @@ -7,7 +7,7 @@ use crate::{ candidate_details::{EncryptedApplicationDetails, EncryptedString}, crypto::{self, hash_password}, error::ServiceError, - Mutation, Query, responses::{BaseCandidateResponse, CreateCandidateResponse}, + Mutation, Query, responses::{BaseCandidateResponse, CreateCandidateResponse}, util::get_recipients, }; use super::{session_service::{AdminUser, SessionService}, application_service::ApplicationService}; @@ -69,8 +69,7 @@ impl CandidateService { // Check if user with that application id already exists if Query::find_candidate_by_id(db, application_id) - .await - .unwrap() + .await? .is_some() { return Err(ServiceError::UserAlreadyExists); @@ -85,10 +84,7 @@ impl CandidateService { crypto::encrypt_password(priv_key_plain_text, plain_text_password.to_string()).await?; - let admin_public_keys = Query::get_all_admin_public_keys(db).await?; - let mut admin_public_keys_ref = admin_public_keys.iter().map(|s| &**s).collect(); - let mut recipients = vec![&*pubkey]; - recipients.append(&mut admin_public_keys_ref); + let recipients = get_recipients(db, &pubkey).await?; let enc_personal_id_number = EncryptedString::new( &personal_id_number, @@ -237,14 +233,10 @@ impl CandidateService { let session_id = SessionService::new_session(db, Some(candidate_id), None, password.clone(), ip_addr) - .await; - match session_id { - Ok(session_id) => { - let private_key = Self::decrypt_private_key(candidate, password).await?; - Ok((session_id, private_key)) - } - Err(e) => Err(e), - } + .await?; + + let private_key = Self::decrypt_private_key(candidate, password).await?; + Ok((session_id, private_key)) } pub async fn auth(db: &DbConn, session_uuid: Uuid) -> Result { diff --git a/core/src/util.rs b/core/src/util.rs index a988644..cc71e0e 100644 --- a/core/src/util.rs +++ b/core/src/util.rs @@ -2,6 +2,16 @@ use entity::{admin, candidate, parent, session}; use sea_orm::{Schema, Database, DbConn}; use sea_orm::{sea_query::TableCreateStatement, ConnectionTrait, DbBackend}; +use crate::Query; +use crate::error::ServiceError; + +pub async fn get_recipients(db: &DbConn, candidate_pubkey: &str) -> Result, ServiceError> { + let mut admin_public_keys = Query::get_all_admin_public_keys(db).await?; + let mut recipients = vec![candidate_pubkey.to_string()]; + recipients.append(&mut admin_public_keys); + Ok(recipients) +} + pub async fn get_memory_sqlite_connection() -> sea_orm::DbConn { let base_url = "sqlite::memory:";