From fc6ab00e30a900dbe82d6e5581f475078d637c96 Mon Sep 17 00:00:00 2001 From: Pierre Hubert Date: Sat, 26 Nov 2022 16:06:16 +0100 Subject: [PATCH] Refactor users management (#7) * Improve general settings management by admin --- src/actors/users_actor.rs | 43 +++++++++----- src/controllers/admin_controller.rs | 88 +++++++++++++++++------------ src/data/user.rs | 25 +++++++- src/data/users_file_entity.rs | 34 ++++++++--- src/main.rs | 2 +- src/utils/err.rs | 32 +++++++++++ 6 files changed, 166 insertions(+), 58 deletions(-) diff --git a/src/actors/users_actor.rs b/src/actors/users_actor.rs index 3ad3d6d..292df33 100644 --- a/src/actors/users_actor.rs +++ b/src/actors/users_actor.rs @@ -1,14 +1,16 @@ use actix::{Actor, Context, Handler, Message, MessageResult}; use std::net::IpAddr; -use crate::data::user::{FactorID, GrantedClients, TwoFactor, User, UserID}; +use crate::data::user::{FactorID, GeneralSettings, GrantedClients, TwoFactor, User, UserID}; use crate::utils::err::Res; /// User storage interface -pub trait UsersBackend { +pub trait UsersSyncBackend { fn find_by_username_or_email(&self, u: &str) -> Option; fn find_by_user_id(&self, id: &UserID) -> Option; fn get_entire_users_list(&self) -> Vec; + fn create_user_account(&mut self, settings: GeneralSettings) -> Res; + fn set_general_user_settings(&mut self, settings: GeneralSettings) -> Res; fn change_user_password(&mut self, id: &UserID, password: &str, temporary: bool) -> bool; fn verify_user_password(&self, user: &UserID, password: &str) -> bool; fn add_2fa_factor(&mut self, user: &UserID, factor: TwoFactor) -> bool; @@ -17,9 +19,6 @@ pub trait UsersBackend { fn clear_2fa_login_history(&mut self, id: &UserID) -> bool; fn delete_account(&mut self, id: &UserID) -> bool; fn set_granted_2fa_clients(&mut self, id: &UserID, clients: GrantedClients) -> bool; - - // FIXME : remove this - fn update_or_insert_user(&mut self, user: User) -> Res; } #[derive(Debug)] @@ -62,6 +61,10 @@ pub struct GetAllUsersRequest; #[derive(Debug)] pub struct GetAllUsersResult(pub Vec); +#[derive(Message)] +#[rtype(result = "Option")] +pub struct CreateAccount(pub GeneralSettings); + #[derive(Message)] #[rtype(result = "bool")] pub struct ChangePasswordRequest { @@ -92,20 +95,20 @@ pub struct SetGrantedClients(pub UserID, pub GrantedClients); #[derive(Message)] #[rtype(result = "bool")] -pub struct UpdateUserRequest(pub User); +pub struct UpdateUserSettings(pub GeneralSettings); #[derive(Message)] #[rtype(result = "bool")] pub struct DeleteUserRequest(pub UserID); pub struct UsersActor { - manager: Box, + manager: Box, } impl UsersActor { pub fn new(manager: E) -> Self where - E: UsersBackend + 'static, + E: UsersSyncBackend + 'static, { Self { manager: Box::new(manager), @@ -138,6 +141,20 @@ impl Handler for UsersActor { } } +impl Handler for UsersActor { + type Result = ::Result; + + fn handle(&mut self, msg: CreateAccount, _ctx: &mut Self::Context) -> Self::Result { + match self.manager.create_user_account(msg.0) { + Ok(id) => Some(id), + Err(e) => { + log::error!("Failed to create user account! {}", e); + None + } + } + } +} + impl Handler for UsersActor { type Result = ::Result; @@ -220,14 +237,14 @@ impl Handler for UsersActor { } } -impl Handler for UsersActor { - type Result = ::Result; +impl Handler for UsersActor { + type Result = ::Result; - fn handle(&mut self, msg: UpdateUserRequest, _ctx: &mut Self::Context) -> Self::Result { - match self.manager.update_or_insert_user(msg.0) { + fn handle(&mut self, msg: UpdateUserSettings, _ctx: &mut Self::Context) -> Self::Result { + match self.manager.set_general_user_settings(msg.0) { Ok(_) => true, Err(e) => { - log::error!("Failed to update user information! {:?}", e); + log::error!("Failed to update general user information! {:?}", e); false } } diff --git a/src/controllers/admin_controller.rs b/src/controllers/admin_controller.rs index 8c5cf1e..53df18b 100644 --- a/src/controllers/admin_controller.rs +++ b/src/controllers/admin_controller.rs @@ -11,7 +11,7 @@ use crate::controllers::settings_controller::BaseSettingsPage; use crate::data::action_logger::{Action, ActionLogger}; use crate::data::client::{Client, ClientID, ClientManager}; use crate::data::current_user::CurrentUser; -use crate::data::user::{GrantedClients, User, UserID}; +use crate::data::user::{GeneralSettings, GrantedClients, User, UserID}; use crate::utils::string_utils::rand_str; #[derive(Template)] @@ -74,39 +74,57 @@ pub async fn users_route( let mut success = None; if let Some(update) = update_query { - let current_user: Option = users - .send(users_actor::FindUserByUsername(update.username.to_string())) + let edited_user: Option = users + .send(users_actor::GetUserRequest(update.uid.clone())) .await .unwrap() .0; - let is_creating = current_user.is_none(); + let is_creating = edited_user.is_none(); - let mut user = current_user.unwrap_or_default(); - user.uid = update.0.uid; - user.username = update.0.username; - user.first_name = update.0.first_name; - user.last_name = update.0.last_name; - user.email = update.0.email; - user.enabled = update.0.enabled.is_some(); - user.two_factor_exemption_after_successful_login = update - .0 - .two_factor_exemption_after_successful_login - .is_some(); - user.admin = update.0.admin.is_some(); + let settings = GeneralSettings { + uid: update.0.uid, + username: update.0.username, + first_name: update.0.first_name, + last_name: update.0.last_name, + email: update.0.email, + enabled: update.0.enabled.is_some(), + two_factor_exemption_after_successful_login: update + .0 + .two_factor_exemption_after_successful_login + .is_some(), + is_admin: update.0.admin.is_some(), + }; + let mut edited_user = edited_user.unwrap_or_default(); + edited_user.update_general_settings(settings.clone()); - let res = users - .send(users_actor::UpdateUserRequest(user.clone())) - .await - .unwrap(); + let res = match is_creating { + true => { + match users + .send(users_actor::CreateAccount(settings)) + .await + .unwrap() + { + Some(id) => { + edited_user.uid = id; + true + } + None => false, + } + } + false => users + .send(users_actor::UpdateUserSettings(settings)) + .await + .unwrap(), + }; // Update the list of factors let factors_to_keep = update.0.two_factor.split(';').collect::>(); - for factor in &user.two_factor { + for factor in &edited_user.two_factor { if !factors_to_keep.contains(&factor.id.0.as_str()) { - logger.log(Action::AdminRemoveUserFactor(&user, factor)); + logger.log(Action::AdminRemoveUserFactor(&edited_user, factor)); users .send(users_actor::Remove2FAFactor( - user.uid.clone(), + edited_user.uid.clone(), factor.id.clone(), )) .await @@ -130,14 +148,14 @@ pub async fn users_route( _ => GrantedClients::NoClient, }; - if user.granted_clients() != granted_clients { + if edited_user.granted_clients() != granted_clients { logger.log(Action::AdminSetNewGrantedClientsList( - &user, + &edited_user, &granted_clients, )); users .send(users_actor::SetGrantedClients( - user.uid.clone(), + edited_user.uid.clone(), granted_clients, )) .await @@ -146,9 +164,9 @@ pub async fn users_route( // Clear user 2FA history if requested if update.0.clear_2fa_history.is_some() { - logger.log(Action::AdminClear2FAHistory(&user)); + logger.log(Action::AdminClear2FAHistory(&edited_user)); users - .send(users_actor::Clear2FALoginHistory(user.uid.clone())) + .send(users_actor::Clear2FALoginHistory(edited_user.uid.clone())) .await .unwrap(); } @@ -157,12 +175,12 @@ pub async fn users_route( let new_password = match update.0.gen_new_password.is_some() { false => None, true => { - logger.log(Action::AdminResetUserPassword(&user)); + logger.log(Action::AdminResetUserPassword(&edited_user)); let temp_pass = rand_str(TEMPORARY_PASSWORDS_LEN); users .send(users_actor::ChangePasswordRequest { - user_id: user.uid.clone(), + user_id: edited_user.uid.clone(), new_password: temp_pass.clone(), temporary: true, }) @@ -184,19 +202,19 @@ pub async fn users_route( } else { success = Some(match is_creating { true => { - logger.log(Action::AdminCreateUser(&user)); - format!("User {} was successfully created!", user.full_name()) + logger.log(Action::AdminCreateUser(&edited_user)); + format!("User {} was successfully created!", edited_user.full_name()) } false => { - logger.log(Action::AdminUpdateUser(&user)); - format!("User {} was successfully updated!", user.full_name()) + logger.log(Action::AdminUpdateUser(&edited_user)); + format!("User {} was successfully updated!", edited_user.full_name()) } }); if let Some(pass) = new_password { danger = Some(format!( "{}'s temporary password is {}", - user.full_name(), + edited_user.full_name(), pass )); } diff --git a/src/data/user.rs b/src/data/user.rs index 2b6dee6..5ac8009 100644 --- a/src/data/user.rs +++ b/src/data/user.rs @@ -11,6 +11,18 @@ use crate::utils::time::{fmt_time, time}; #[derive(Clone, Debug, Eq, PartialEq, serde::Serialize, serde::Deserialize)] pub struct UserID(pub String); +#[derive(Debug, Clone)] +pub struct GeneralSettings { + pub uid: UserID, + pub username: String, + pub first_name: String, + pub last_name: String, + pub email: String, + pub enabled: bool, + pub two_factor_exemption_after_successful_login: bool, + pub is_admin: bool, +} + #[derive(Eq, PartialEq, Clone, Debug)] pub enum GrantedClients { AllClients, @@ -177,6 +189,17 @@ impl User { > time() } + pub fn update_general_settings(&mut self, settings: GeneralSettings) { + self.username = settings.username; + self.first_name = settings.first_name; + self.last_name = settings.last_name; + self.email = settings.email; + self.enabled = settings.enabled; + self.two_factor_exemption_after_successful_login = + settings.two_factor_exemption_after_successful_login; + self.admin = settings.is_admin; + } + pub fn add_factor(&mut self, factor: TwoFactor) { self.two_factor.push(factor); } @@ -256,7 +279,7 @@ impl Eq for User {} impl Default for User { fn default() -> Self { Self { - uid: UserID(uuid::Uuid::new_v4().to_string()), + uid: UserID("".to_string()), first_name: "".to_string(), last_name: "".to_string(), username: "".to_string(), diff --git a/src/data/users_file_entity.rs b/src/data/users_file_entity.rs index 6d26826..761f23c 100644 --- a/src/data/users_file_entity.rs +++ b/src/data/users_file_entity.rs @@ -1,7 +1,7 @@ -use crate::actors::users_actor::UsersBackend; +use crate::actors::users_actor::UsersSyncBackend; use crate::data::entity_manager::EntityManager; -use crate::data::user::{FactorID, GrantedClients, TwoFactor, User, UserID}; -use crate::utils::err::Res; +use crate::data::user::{FactorID, GeneralSettings, GrantedClients, TwoFactor, User, UserID}; +use crate::utils::err::{new_error, Res}; use crate::utils::time::time; use std::net::IpAddr; @@ -39,7 +39,7 @@ fn verify_password>(pwd: P, hash: &str) -> bool { } } -impl UsersBackend for EntityManager { +impl UsersSyncBackend for EntityManager { fn find_by_username_or_email(&self, u: &str) -> Option { for entry in self.iter() { if entry.username.eq(u) || entry.email.eq(u) { @@ -62,6 +62,28 @@ impl UsersBackend for EntityManager { self.cloned() } + fn create_user_account(&mut self, settings: GeneralSettings) -> Res { + let mut user = User { + uid: UserID(uuid::Uuid::new_v4().to_string()), + ..Default::default() + }; + user.update_general_settings(settings); + self.insert(user.clone())?; + Ok(user.uid) + } + + fn set_general_user_settings(&mut self, settings: GeneralSettings) -> Res { + let res = self.update_user(&settings.uid.clone(), |mut user| { + user.update_general_settings(settings); + user + }); + + match res { + true => Ok(()), + false => new_error("Failed to update user general settings!".to_string()), + } + } + fn change_user_password(&mut self, id: &UserID, password: &str, temporary: bool) -> bool { let new_hash = match hash_password(password) { Ok(h) => h, @@ -144,8 +166,4 @@ impl UsersBackend for EntityManager { user }) } - - fn update_or_insert_user(&mut self, user: User) -> Res { - self.update_or_push(user) - } } diff --git a/src/main.rs b/src/main.rs index e8153c5..ced9ab5 100644 --- a/src/main.rs +++ b/src/main.rs @@ -12,7 +12,7 @@ use actix_web::{get, middleware, web, App, HttpResponse, HttpServer}; use basic_oidc::actors::bruteforce_actor::BruteForceActor; use basic_oidc::actors::openid_sessions_actor::OpenIDSessionsActor; -use basic_oidc::actors::users_actor::{UsersActor, UsersBackend}; +use basic_oidc::actors::users_actor::{UsersActor, UsersSyncBackend}; use basic_oidc::constants::*; use basic_oidc::controllers::assets_controller::assets_route; use basic_oidc::controllers::*; diff --git a/src/utils/err.rs b/src/utils/err.rs index 427140e..38c107e 100644 --- a/src/utils/err.rs +++ b/src/utils/err.rs @@ -1,2 +1,34 @@ use std::error::Error; +use std::fmt; +use std::fmt::{Display, Formatter}; + pub type Res = Result>; + +#[derive(Debug, Clone)] +pub struct ExecError(pub String); + +impl ExecError { + pub fn new(msg: &str) -> ExecError { + ExecError(msg.to_string()) + } + + pub fn boxed_new(msg: D) -> Box { + Box::new(ExecError(msg.to_string())) + } + + pub fn boxed_string(msg: String) -> Box { + Box::new(ExecError(msg)) + } +} + +impl Display for ExecError { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + write!(f, "Encountered error: {}", self.0) + } +} + +impl Error for ExecError {} + +pub fn new_error(msg: D) -> Res { + Err(ExecError::boxed_new(msg)) +}