From f7f10c55771f2d763cb134465c4e4d05f0b3f1e5 Mon Sep 17 00:00:00 2001 From: Dietrich Date: Wed, 16 Jun 2021 10:45:23 +0200 Subject: [PATCH] Document and simplify --- pslink/src/forms.rs | 7 -- pslink/src/lib.rs | 2 - pslink/src/models.rs | 61 ++++++++++-- pslink/src/queries.rs | 212 +++++++----------------------------------- pslink/src/views.rs | 10 +- 5 files changed, 89 insertions(+), 203 deletions(-) delete mode 100644 pslink/src/forms.rs diff --git a/pslink/src/forms.rs b/pslink/src/forms.rs deleted file mode 100644 index 122b47f..0000000 --- a/pslink/src/forms.rs +++ /dev/null @@ -1,7 +0,0 @@ -use serde::Deserialize; -#[derive(Deserialize, Debug)] -pub struct LinkForm { - pub title: String, - pub target: String, - pub code: String, -} diff --git a/pslink/src/lib.rs b/pslink/src/lib.rs index 703509c..c03239f 100644 --- a/pslink/src/lib.rs +++ b/pslink/src/lib.rs @@ -1,6 +1,5 @@ extern crate sqlx; -pub mod forms; pub mod models; pub mod queries; mod views; @@ -206,7 +205,6 @@ static_loader! { /// /// # Errors /// This produces a [`ServerError`] if: -/// * Tera failed to build its templates /// * The server failed to bind to the designated port. #[allow(clippy::future_not_send, clippy::too_many_lines)] pub async fn webservice( diff --git a/pslink/src/models.rs b/pslink/src/models.rs index 674e92d..076a77d 100644 --- a/pslink/src/models.rs +++ b/pslink/src/models.rs @@ -1,6 +1,6 @@ use std::str::FromStr; -use crate::{forms::LinkForm, Secret, ServerConfig, ServerError}; +use crate::{Secret, ServerConfig, ServerError}; use argonautica::Hasher; use async_trait::async_trait; @@ -14,6 +14,7 @@ use shared::{ use sqlx::Row; use tracing::{error, info, instrument}; +/// The operations a User should support. #[async_trait] pub trait UserDbOperations { async fn get_user(id: i64, server_config: &ServerConfig) -> Result; @@ -31,6 +32,10 @@ pub trait UserDbOperations { #[async_trait] impl UserDbOperations for User { + /// get a user by its id + /// + /// # Errors + /// fails with [`ServerError`] if the user does not exist or the database cannot be acessed. #[instrument()] async fn get_user(id: i64, server_config: &ServerConfig) -> Result { let user = sqlx::query!("Select * from users where id = ? ", id) @@ -70,6 +75,10 @@ impl UserDbOperations for User { user.map_err(ServerError::Database) } + /// get all users + /// + /// # Errors + /// fails with [`ServerError`] if the database cannot be acessed. #[instrument()] async fn get_all_users(server_config: &ServerConfig) -> Result, ServerError> { let user = sqlx::query("Select * from users") @@ -91,6 +100,10 @@ impl UserDbOperations for User { user.map_err(ServerError::Database) } + /// change a user + /// + /// # Errors + /// fails with [`ServerError`] if the user does not exist, some constraints are not satisfied or the database cannot be acessed. #[instrument()] async fn update_user(&self, server_config: &ServerConfig) -> Result<(), ServerError> { let role_i64 = self.role.to_i64(); @@ -129,6 +142,10 @@ impl UserDbOperations for User { Ok(()) } + /// set the language setting of a user + /// + /// # Errors + /// fails with [`ServerError`] if the user does not exist or the database cannot be acessed. #[instrument()] async fn set_language( self, @@ -161,6 +178,8 @@ impl UserDbOperations for User { } } +/// Relevant parameters when creating a new user +/// Use the [`NewUser::new`] constructor to store the password encrypted. Otherwise it will not work. #[derive(Debug, Deserialize)] pub struct NewUser { pub username: String, @@ -171,6 +190,8 @@ pub struct NewUser { impl NewUser { /// Create a new user that can then be inserted in the database /// + /// The password is encrypted using the secret before creating. + /// /// # Errors /// fails with [`ServerError`] if the password could not be encrypted. #[instrument()] @@ -189,6 +210,9 @@ impl NewUser { }) } + /// encrypt the password. + /// + /// This function uses the Secret from the config settings to encrypt the password #[instrument()] pub(crate) fn hash_password(password: &str, secret: &Secret) -> Result { dotenv().ok(); @@ -223,6 +247,7 @@ impl NewUser { } } +/// Operations that should be supported by links #[async_trait] pub trait LinkDbOperations { async fn get_link_by_code(code: &str, server_config: &ServerConfig) -> Result; @@ -236,6 +261,10 @@ pub trait LinkDbOperations { #[async_trait] impl LinkDbOperations for Link { + /// Get a link by its code (the short url code) + /// + /// # Errors + /// fails with [`ServerError`] if the database cannot be acessed or the link is not found. #[instrument()] async fn get_link_by_code( code: &str, @@ -251,6 +280,11 @@ impl LinkDbOperations for Link { tracing::info!("Found link: {:?}", &link); link.map_err(ServerError::Database) } + + /// Get a link by its id + /// + /// # Errors + /// fails with [`ServerError`] if the database cannot be acessed or the link is not found. #[instrument()] async fn get_link_by_id(id: i64, server_config: &ServerConfig) -> Result { let link = sqlx::query_as!(Self, "Select * from links where id = ? ", id) @@ -260,6 +294,10 @@ impl LinkDbOperations for Link { link.map_err(ServerError::Database) } + /// Delete a link by its code + /// + /// # Errors + /// fails with [`ServerError`] if the database cannot be acessed or the link is not found. #[instrument()] async fn delete_link_by_code( code: &str, @@ -271,6 +309,11 @@ impl LinkDbOperations for Link { Ok(()) } + /// Update a link with new values, carful when changing the code the old link becomes invalid. + /// This could be a problem when it is printed or published somewhere. + /// + /// # Errors + /// fails with [`ServerError`] if the database cannot be acessed or the link is not found. #[instrument()] async fn update_link(&self, server_config: &ServerConfig) -> Result<(), ServerError> { info!("{:?}", self); @@ -299,6 +342,7 @@ impl LinkDbOperations for Link { } } +/// Relevant parameters when creating a new link. #[derive(Serialize, Debug)] pub struct NewLink { pub title: String, @@ -309,15 +353,7 @@ pub struct NewLink { } impl NewLink { - pub(crate) fn from_link_form(form: LinkForm, uid: i64) -> Self { - Self { - title: form.title, - target: form.target, - code: form.code, - author: uid, - created_at: chrono::Local::now().naive_utc(), - } - } + /// Take a [`LinkDelta`] and create a [`NewLink`] instance. `created_at` is populated with the current time. pub(crate) fn from_link_delta(link: LinkDelta, uid: i64) -> Self { Self { title: link.title, @@ -328,6 +364,10 @@ impl NewLink { } } + /// Insert the new link into the database + /// + /// # Errors + /// fails with [`ServerError`] if the database cannot be acessed or constraints are not met. pub(crate) async fn insert(self, server_config: &ServerConfig) -> Result<(), ServerError> { sqlx::query!( "Insert into links ( @@ -348,6 +388,7 @@ impl NewLink { } } +/// Whenever a link is clicked the click is registered for statistical purposes. #[derive(Serialize)] pub struct NewClick { pub link: i64, diff --git a/pslink/src/queries.rs b/pslink/src/queries.rs index cb715c7..eb402b3 100644 --- a/pslink/src/queries.rs +++ b/pslink/src/queries.rs @@ -1,7 +1,6 @@ use std::str::FromStr; use actix_identity::Identity; -use actix_web::web; use enum_map::EnumMap; use serde::Serialize; use shared::{ @@ -17,12 +16,11 @@ use tracing::{info, instrument, warn}; use super::models::NewUser; use crate::{ - forms::LinkForm, models::{LinkDbOperations, NewClick, NewLink, UserDbOperations}, ServerConfig, ServerError, }; -/// The possible roles a user could have. +/// This type is used to guard the Roles. The typesystem enforces that the user can only be extracted if permissions are considered. #[derive(Debug, Clone)] pub enum RoleGuard { NotAuthenticated, @@ -34,10 +32,10 @@ pub enum RoleGuard { impl RoleGuard { fn create(user: &User) -> Self { match user.role { - shared::apirequests::users::Role::NotAuthenticated => Self::NotAuthenticated, - shared::apirequests::users::Role::Disabled => Self::Disabled, - shared::apirequests::users::Role::Regular => Self::Regular { user: user.clone() }, - shared::apirequests::users::Role::Admin => Self::Admin { user: user.clone() }, + Role::NotAuthenticated => Self::NotAuthenticated, + Role::Disabled => Self::Disabled, + Role::Regular => Self::Regular { user: user.clone() }, + Role::Admin => Self::Admin { user: user.clone() }, } } /// Determin if the user is admin or the given user id is his own. This is used for things where users can edit or view their own entries, whereas admins can do so for all entries. @@ -50,7 +48,7 @@ impl RoleGuard { } } -/// queries the user matching the given [`actix_identity::Identity`] and determins its authentication and permission level. Returns a [`Role`] containing the user if it is authenticated. +/// queries the user matching the given [`actix_identity::Identity`] and determins its authentication and permission level. Returns a [`RoleGuard`] containing the user if it is authenticated. /// /// # Errors /// Fails only if there are issues using the database. @@ -78,6 +76,8 @@ pub struct ListWithOwner { /// Returns a List of `FullLink` meaning `Links` enriched by their author and statistics. This returns all links if the user is either Admin or Regular user. /// +/// Todo: this function only naively protects agains SQL-injections use better variants. +/// /// # Errors /// Fails with [`ServerError`] if access to the database fails. #[instrument(skip(id))] @@ -153,6 +153,9 @@ pub async fn list_all_allowed( } } +/// Generate a filter statement for the SQL-Query according to the parameters... +/// +/// Todo: this function only naively protects agains SQL-injections use better variants. fn generate_filter_sql(filters: &EnumMap) -> String { let mut result = String::new(); let filterstring = filters @@ -190,6 +193,8 @@ fn generate_filter_sql(filters: &EnumMap) -> String } result } + +/// A macro to translate the Ordering Type into a sql ordering string. macro_rules! ts { ($ordering:expr) => { match $ordering { @@ -198,6 +203,8 @@ macro_rules! ts { }; }; } + +/// Generate a order statement for the SQL-Query according to the parameters... fn generate_order_sql(order: &Operation) -> String { let filterstring = match order.column { LinkOverviewColumns::Code => { @@ -219,10 +226,10 @@ fn generate_order_sql(order: &Operation) -> Strin filterstring } -/// Only admins can list all users +/// Only admins can list all users other users will only see themselves. /// /// # Errors -/// Fails with [`ServerError`] if access to the database fails or this user does not have permissions. +/// Fails with [`ServerError`] if access to the database fails. #[instrument(skip(id))] pub async fn list_users( id: &Identity, @@ -265,6 +272,9 @@ pub async fn list_users( } } +/// Generate a filter statement for the SQL-Query according to the parameters... +/// +/// Todo: this function only naively protects agains SQL-injections use better variants. fn generate_filter_users_sql(filters: &EnumMap) -> String { let mut result = String::new(); let filterstring = filters @@ -296,6 +306,8 @@ fn generate_filter_users_sql(filters: &EnumMap) -> } result } + +/// Generate a order statement for the SQL-Query according to the parameters... fn generate_order_users_sql(order: &Operation) -> String { let filterstring = match order.column { UserOverviewColumns::Id => { @@ -372,42 +384,7 @@ pub async fn get_user_by_name( #[instrument(skip(id))] pub async fn create_user( id: &Identity, - data: &web::Form, - server_config: &ServerConfig, -) -> Result, ServerError> { - info!("Creating a User: {:?}", &data); - let auth = authenticate(id, server_config).await?; - match auth { - RoleGuard::Admin { user } => { - let new_user = NewUser::new( - data.username.clone(), - data.email.clone(), - &data.password, - &server_config.secret, - )?; - - new_user.insert_user(server_config).await?; - - // querry the new user - let new_user = get_user_by_name(&data.username, server_config).await?; - Ok(Item { - user, - item: new_user, - }) - } - RoleGuard::Regular { .. } | RoleGuard::Disabled | RoleGuard::NotAuthenticated => { - Err(ServerError::User("Permission denied!".to_owned())) - } - } -} -/// Create a new user and save it to the database -/// -/// # Errors -/// Fails with [`ServerError`] if access to the database fails, this user does not have permissions or the user already exists. -#[instrument(skip(id))] -pub async fn create_user_json( - id: &Identity, - data: &web::Json, + data: UserDelta, server_config: &ServerConfig, ) -> Result, ServerError> { info!("Creating a User: {:?}", &data); @@ -448,6 +425,7 @@ pub async fn create_user_json( } } } + /// Take a [`actix_web::web::Form`] and update the corresponding entry in the database. /// The password is only updated if a new password of at least 4 characters is provided. /// The `user_id` is never changed. @@ -456,9 +434,9 @@ pub async fn create_user_json( /// Fails with [`ServerError`] if access to the database fails, this user does not have permissions, or the given data is malformed. #[instrument(skip(id))] -pub async fn update_user_json( +pub async fn update_user( id: &Identity, - data: &web::Json, + data: &UserDelta, server_config: &ServerConfig, ) -> Result, ServerError> { let auth = authenticate(id, server_config).await?; @@ -469,10 +447,10 @@ pub async fn update_user_json( RoleGuard::Admin { .. } | RoleGuard::Regular { .. } => { info!("Updating userinfo: "); let password = match &data.password { - Some(password) => { + Some(password) if password.len() > 4 => { Secret::new(NewUser::hash_password(password, &server_config.secret)?) } - None => unmodified_user.password, + _ => unmodified_user.password, }; let new_user = User { id: uid, @@ -501,61 +479,6 @@ pub async fn update_user_json( } } -/// Take a [`actix_web::web::Form`] and update the corresponding entry in the database. -/// The password is only updated if a new password of at least 4 characters is provided. -/// The `user_id` is never changed. -/// -/// # Errors -/// Fails with [`ServerError`] if access to the database fails, this user does not have permissions, or the given data is malformed. -#[allow(clippy::missing_panics_doc)] -#[instrument(skip(id))] -pub async fn update_user( - id: &Identity, - user_id: &str, - server_config: &ServerConfig, - data: &web::Form, -) -> Result, ServerError> { - if let Ok(uid) = user_id.parse::() { - let auth = authenticate(id, server_config).await?; - let unmodified_user = User::get_user(uid, server_config).await?; - if auth.admin_or_self(uid) { - match auth { - RoleGuard::Admin { .. } | RoleGuard::Regular { .. } => { - info!("Updating userinfo: "); - let password = if data.password.len() > 3 { - Secret::new(NewUser::hash_password( - &data.password, - &server_config.secret, - )?) - } else { - unmodified_user.password - }; - let new_user = User { - id: uid, - username: data.username.clone(), - email: data.email.clone(), - password, - role: unmodified_user.role, - language: unmodified_user.language, - }; - new_user.update_user(server_config).await?; - let changed_user = User::get_user(uid, server_config).await?; - Ok(Item { - user: changed_user.clone(), - item: changed_user, - }) - } - RoleGuard::NotAuthenticated | RoleGuard::Disabled => { - unreachable!("Should be unreachable because of the `admin_or_self`") - } - } - } else { - Err(ServerError::User("Not a valid UID".to_owned())) - } - } else { - Err(ServerError::User("Permission denied".to_owned())) - } -} /// Demote an admin user to a normal user or promote a normal user to admin privileges. /// /// # Errors @@ -707,44 +630,6 @@ pub async fn delete_link( } } -/// Update a link if the user is admin or it is its own link. -/// -/// # Errors -/// Fails with [`ServerError`] if access to the database fails or this user does not have permissions. -#[instrument(skip(id))] -pub async fn update_link( - id: &Identity, - link_code: &str, - data: web::Form, - server_config: &ServerConfig, -) -> Result, ServerError> { - info!("Changing link to: {:?} {:?}", &data, &link_code); - let auth = authenticate(id, server_config).await?; - match auth { - RoleGuard::Admin { .. } | RoleGuard::Regular { .. } => { - let query: Item = get_link(id, link_code, server_config).await?; - if auth.admin_or_self(query.item.author) { - let mut link = query.item; - let LinkForm { - title, - target, - code, - } = data.into_inner(); - link.code = code.clone(); - link.target = target; - link.title = title; - link.update_link(server_config).await?; - get_link(id, &code, server_config).await - } else { - Err(ServerError::User("Not Allowed".to_owned())) - } - } - RoleGuard::Disabled | RoleGuard::NotAuthenticated => { - Err(ServerError::User("Not Allowed".to_owned())) - } - } -} - /// Create a new link /// /// # Errors @@ -752,7 +637,7 @@ pub async fn update_link( #[instrument(skip(id))] pub async fn create_link( id: &Identity, - data: web::Form, + data: LinkDelta, server_config: &ServerConfig, ) -> Result, ServerError> { let auth = authenticate(id, server_config).await?; @@ -760,38 +645,7 @@ pub async fn create_link( RoleGuard::Admin { user } | RoleGuard::Regular { user } => { let code = data.code.clone(); info!("Creating link for: {}", &code); - let new_link = NewLink::from_link_form(data.into_inner(), user.id); - info!("Creating link for: {:?}", &new_link); - - new_link.insert(server_config).await?; - let new_link: Link = get_link_simple(&code, server_config).await?; - Ok(Item { - user, - item: new_link, - }) - } - RoleGuard::Disabled | RoleGuard::NotAuthenticated => { - Err(ServerError::User("Permission denied!".to_owned())) - } - } -} - -/// Create a new link -/// -/// # Errors -/// Fails with [`ServerError`] if access to the database fails or this user does not have permissions. -#[instrument(skip(id))] -pub async fn create_link_json( - id: &Identity, - data: web::Json, - server_config: &ServerConfig, -) -> Result, ServerError> { - let auth = authenticate(id, server_config).await?; - match auth { - RoleGuard::Admin { user } | RoleGuard::Regular { user } => { - let code = data.code.clone(); - info!("Creating link for: {}", &code); - let new_link = NewLink::from_link_delta(data.into_inner(), user.id); + let new_link = NewLink::from_link_delta(data, user.id); info!("Creating link for: {:?}", &new_link); new_link.insert(server_config).await?; @@ -812,9 +666,9 @@ pub async fn create_link_json( /// # Errors /// Fails with [`ServerError`] if access to the database fails or this user does not have permissions. #[instrument(skip(ident))] -pub async fn update_link_json( +pub async fn update_link( ident: &Identity, - data: web::Json, + data: LinkDelta, server_config: &ServerConfig, ) -> Result, ServerError> { let auth = authenticate(ident, server_config).await?; @@ -829,7 +683,7 @@ pub async fn update_link_json( target, code, .. - } = data.into_inner(); + } = data; link.code = code.clone(); link.target = target; link.title = title; diff --git a/pslink/src/views.rs b/pslink/src/views.rs index f018fbc..e8ade3f 100644 --- a/pslink/src/views.rs +++ b/pslink/src/views.rs @@ -178,11 +178,11 @@ pub async fn download_png( #[instrument(skip(id))] pub async fn process_create_user_json( config: web::Data, - form: web::Json, + data: web::Json, id: Identity, ) -> Result { info!("Listing Users to Json api"); - match queries::create_user_json(&id, &form, &config).await { + match queries::create_user(&id, data.into_inner(), &config).await { Ok(item) => Ok(HttpResponse::Ok().json2(&Status::Success(Message { message: format!("Successfully saved user: {}", item.item.username), }))), @@ -197,7 +197,7 @@ pub async fn process_update_user_json( id: Identity, ) -> Result { info!("Listing Users to Json api"); - match queries::update_user_json(&id, &form, &config).await { + match queries::update_user(&id, &form, &config).await { Ok(item) => Ok(HttpResponse::Ok().json2(&Status::Success(Message { message: format!("Successfully saved user: {}", item.item.username), }))), @@ -360,7 +360,7 @@ pub async fn process_create_link_json( data: web::Json, id: Identity, ) -> Result { - let new_link = queries::create_link_json(&id, data, &config).await; + let new_link = queries::create_link(&id, data.into_inner(), &config).await; match new_link { Ok(item) => Ok(HttpResponse::Ok().json2(&Status::Success(Message { message: format!("Successfully saved link: {}", item.item.code), @@ -375,7 +375,7 @@ pub async fn process_update_link_json( data: web::Json, id: Identity, ) -> Result { - let new_link = queries::update_link_json(&id, data, &config).await; + let new_link = queries::update_link(&id, data.into_inner(), &config).await; match new_link { Ok(item) => Ok(HttpResponse::Ok().json2(&Status::Success(Message { message: format!("Successfully updated link: {}", item.item.code),