From 65d334b9473eb36944196ccc51991d79bc136e42 Mon Sep 17 00:00:00 2001 From: Pierre Hubert Date: Sat, 19 Nov 2022 17:52:35 +0100 Subject: [PATCH] Refactor users management * Shard `src/data/user.rs` into two different files * One for user data structure (same file) * One for user manipulation (new file: `user_file_entity.rs`) * Isolate password hashing and verification --- src/actors/users_actor.rs | 14 +++- src/controllers/admin_controller.rs | 34 +++++---- src/controllers/settings_controller.rs | 9 ++- src/data/mod.rs | 1 + src/data/user.rs | 93 ------------------------ src/data/users_file_entity.rs | 98 ++++++++++++++++++++++++++ src/main.rs | 9 +-- 7 files changed, 145 insertions(+), 113 deletions(-) create mode 100644 src/data/users_file_entity.rs diff --git a/src/actors/users_actor.rs b/src/actors/users_actor.rs index be97d40..099545c 100644 --- a/src/actors/users_actor.rs +++ b/src/actors/users_actor.rs @@ -26,6 +26,10 @@ pub struct GetUserRequest(pub UserID); #[derive(Debug)] pub struct GetUserResult(pub Option); +#[derive(Message)] +#[rtype(result = "bool")] +pub struct VerifyUserPasswordRequest(pub UserID, pub String); + #[derive(Message)] #[rtype(FindUserByUsernameResult)] pub struct FindUserByUsername(pub String); @@ -85,7 +89,7 @@ impl Handler for UsersActor { match self.manager.find_by_username_or_email(&msg.login) { None => MessageResult(LoginResult::AccountNotFound), Some(user) => { - if !user.verify_password(&msg.password) { + if !self.manager.verify_user_password(&user.uid, &msg.password) { return MessageResult(LoginResult::InvalidPassword); } @@ -132,6 +136,14 @@ impl Handler for UsersActor { } } +impl Handler for UsersActor { + type Result = ::Result; + + fn handle(&mut self, msg: VerifyUserPasswordRequest, _ctx: &mut Self::Context) -> Self::Result { + self.manager.verify_user_password(&msg.0, &msg.1) + } +} + impl Handler for UsersActor { type Result = MessageResult; diff --git a/src/controllers/admin_controller.rs b/src/controllers/admin_controller.rs index 712b114..3c74045 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::{hash_password, User, UserID}; +use crate::data::user::{User, UserID}; use crate::utils::string_utils::rand_str; #[derive(Template)] @@ -111,19 +111,6 @@ pub async fn users_route( _ => Some(Vec::new()), }; - let new_password = match update.0.gen_new_password.is_some() { - false => None, - true => { - logger.log(Action::AdminResetUserPassword(&user)); - - let temp_pass = rand_str(TEMPORARY_PASSWORDS_LEN); - user.password = hash_password(&temp_pass).expect("Failed to hash password"); - user.need_reset_password = true; - user.last_successful_2fa = Default::default(); - Some(temp_pass) - } - }; - if update.0.clear_2fa_history.is_some() { logger.log(Action::AdminClear2FAHistory(&user)); user.last_successful_2fa = Default::default(); @@ -134,6 +121,25 @@ pub async fn users_route( .await .unwrap(); + let new_password = match update.0.gen_new_password.is_some() { + false => None, + true => { + logger.log(Action::AdminResetUserPassword(&user)); + + let temp_pass = rand_str(TEMPORARY_PASSWORDS_LEN); + users + .send(users_actor::ChangePasswordRequest { + user_id: user.uid.clone(), + new_password: temp_pass.clone(), + temporary: true, + }) + .await + .unwrap(); + + Some(temp_pass) + } + }; + if !res { danger = Some( match is_creating { diff --git a/src/controllers/settings_controller.rs b/src/controllers/settings_controller.rs index 5e5269b..d772687 100644 --- a/src/controllers/settings_controller.rs +++ b/src/controllers/settings_controller.rs @@ -103,7 +103,14 @@ pub async fn change_password_route( ); } else if let Some(req) = req { // Invalid password - if !user.verify_password(&req.old_pass) { + if !users + .send(users_actor::VerifyUserPasswordRequest( + user.uid.clone(), + req.old_pass.clone(), + )) + .await + .unwrap() + { danger = Some("Old password is invalid!".to_string()); bruteforce .send(bruteforce_actor::RecordFailedAttempt { diff --git a/src/data/mod.rs b/src/data/mod.rs index d752edd..7e431b6 100644 --- a/src/data/mod.rs +++ b/src/data/mod.rs @@ -15,4 +15,5 @@ pub mod remote_ip; pub mod session_identity; pub mod totp_key; pub mod user; +pub mod users_file_entity; pub mod webauthn_manager; diff --git a/src/data/user.rs b/src/data/user.rs index 81351c0..d160409 100644 --- a/src/data/user.rs +++ b/src/data/user.rs @@ -3,11 +3,9 @@ use std::net::IpAddr; use crate::constants::SECOND_FACTOR_EXEMPTION_AFTER_SUCCESSFUL_LOGIN; use crate::data::client::ClientID; -use crate::data::entity_manager::EntityManager; use crate::data::login_redirect::LoginRedirect; use crate::data::totp_key::TotpKey; use crate::data::webauthn_manager::WebauthnPubKey; -use crate::utils::err::Res; use crate::utils::time::{fmt_time, time}; #[derive(Clone, Debug, Eq, PartialEq, serde::Serialize, serde::Deserialize)] @@ -133,10 +131,6 @@ impl User { } } - pub fn verify_password>(&self, pass: P) -> bool { - verify_password(pass, &self.password) - } - pub fn has_two_factor(&self) -> bool { !self.two_factor.is_empty() } @@ -247,90 +241,3 @@ impl Default for User { } } } - -pub fn hash_password>(pwd: P) -> Res { - Ok(bcrypt::hash(pwd, bcrypt::DEFAULT_COST)?) -} - -pub fn verify_password>(pwd: P, hash: &str) -> bool { - match bcrypt::verify(pwd, hash) { - Ok(r) => r, - Err(e) => { - log::warn!("Failed to verify password! {:?}", e); - false - } - } -} - -impl EntityManager { - pub fn find_by_username_or_email(&self, u: &str) -> Option { - for entry in self.iter() { - if entry.username.eq(u) || entry.email.eq(u) { - return Some(entry.clone()); - } - } - None - } - - pub fn find_by_user_id(&self, id: &UserID) -> Option { - for entry in self.iter() { - if entry.uid.eq(id) { - return Some(entry.clone()); - } - } - None - } - - /// Update user information - fn update_user(&mut self, id: &UserID, update: F) -> bool - where - F: FnOnce(User) -> User, - { - let user = match self.find_by_user_id(id) { - None => return false, - Some(user) => user, - }; - - if let Err(e) = self.replace_entries(|u| u.uid.eq(id), &update(user)) { - log::error!("Failed to update user information! {:?}", e); - return false; - } - - true - } - - pub fn change_user_password(&mut self, id: &UserID, password: &str, temporary: bool) -> bool { - let new_hash = match hash_password(password) { - Ok(h) => h, - Err(e) => { - log::error!("Failed to hash user password! {}", e); - return false; - } - }; - - self.update_user(id, |mut user| { - user.password = new_hash; - user.need_reset_password = temporary; - user.two_factor_exemption_after_successful_login = Default::default(); - user - }) - } - - pub fn save_new_successful_2fa_authentication(&mut self, id: &UserID, ip: IpAddr) -> bool { - self.update_user(id, |mut user| { - user.last_successful_2fa.insert(ip, time()); - - // Remove outdated successful attempts - user.remove_outdated_successful_2fa_attempts(); - - user - }) - } - - pub fn clear_2fa_login_history(&mut self, id: &UserID) -> bool { - self.update_user(id, |mut user| { - user.last_successful_2fa = Default::default(); - user - }) - } -} diff --git a/src/data/users_file_entity.rs b/src/data/users_file_entity.rs new file mode 100644 index 0000000..6e91f61 --- /dev/null +++ b/src/data/users_file_entity.rs @@ -0,0 +1,98 @@ +use crate::data::entity_manager::EntityManager; +use crate::data::user::{User, UserID}; +use crate::utils::err::Res; +use crate::utils::time::time; +use std::net::IpAddr; + +fn hash_password>(pwd: P) -> Res { + Ok(bcrypt::hash(pwd, bcrypt::DEFAULT_COST)?) +} + +fn verify_password>(pwd: P, hash: &str) -> bool { + match bcrypt::verify(pwd, hash) { + Ok(r) => r, + Err(e) => { + log::warn!("Failed to verify password! {:?}", e); + false + } + } +} + +impl EntityManager { + pub fn find_by_username_or_email(&self, u: &str) -> Option { + for entry in self.iter() { + if entry.username.eq(u) || entry.email.eq(u) { + return Some(entry.clone()); + } + } + None + } + + pub fn find_by_user_id(&self, id: &UserID) -> Option { + for entry in self.iter() { + if entry.uid.eq(id) { + return Some(entry.clone()); + } + } + None + } + + /// Update user information + fn update_user(&mut self, id: &UserID, update: F) -> bool + where + F: FnOnce(User) -> User, + { + let user = match self.find_by_user_id(id) { + None => return false, + Some(user) => user, + }; + + if let Err(e) = self.replace_entries(|u| u.uid.eq(id), &update(user)) { + log::error!("Failed to update user information! {:?}", e); + return false; + } + + true + } + + pub fn change_user_password(&mut self, id: &UserID, password: &str, temporary: bool) -> bool { + let new_hash = match hash_password(password) { + Ok(h) => h, + Err(e) => { + log::error!("Failed to hash user password! {}", e); + return false; + } + }; + + self.update_user(id, |mut user| { + user.password = new_hash; + user.need_reset_password = temporary; + user.two_factor_exemption_after_successful_login = Default::default(); + user + }) + } + + pub fn verify_user_password(&self, user: &UserID, password: &str) -> bool { + self.find_by_user_id(user) + .map(|u| verify_password(password, &u.password)) + .unwrap_or(false) + } + + pub fn save_new_successful_2fa_authentication(&mut self, id: &UserID, ip: IpAddr) -> bool { + self.update_user(id, |mut user| { + user.last_successful_2fa.insert(ip, time()); + + // Remove outdated successful attempts + user.remove_outdated_successful_2fa_attempts(); + + user + }) + } + + pub fn clear_2fa_login_history(&mut self, id: &UserID) -> bool { + self.update_user(id, |mut user| { + user.last_successful_2fa = Default::default(); + user + }) + } +} diff --git a/src/main.rs b/src/main.rs index affbe37..2688a40 100644 --- a/src/main.rs +++ b/src/main.rs @@ -20,7 +20,7 @@ use basic_oidc::data::app_config::AppConfig; use basic_oidc::data::client::ClientManager; use basic_oidc::data::entity_manager::EntityManager; use basic_oidc::data::jwt_signer::JWTSigner; -use basic_oidc::data::user::{hash_password, User}; +use basic_oidc::data::user::User; use basic_oidc::data::webauthn_manager::WebAuthManager; use basic_oidc::middlewares::auth_middleware::AuthMiddleware; @@ -51,16 +51,17 @@ async fn main() -> std::io::Result<()> { log::info!("Create default {} user", DEFAULT_ADMIN_USERNAME); let default_admin = User { username: DEFAULT_ADMIN_USERNAME.to_string(), - password: hash_password(DEFAULT_ADMIN_PASSWORD).unwrap(), - need_reset_password: true, authorized_clients: None, admin: true, ..Default::default() }; users - .insert(default_admin) + .insert(default_admin.clone()) .expect("Failed to create initial user!"); + + // Set default admin password + users.change_user_password(&default_admin.uid, DEFAULT_ADMIN_PASSWORD, true); } let users_actor = UsersActor::new(users).start();