Refactor users management (#8)

* Update users backend interface signatures

src/actors/users_actor.rs

@@ -1,28 +1,30 @@
-use actix::{Actor, Context, Handler, Message, MessageResult};
 use std::net::IpAddr;
 
+use actix::{Actor, Context, Handler, Message, MessageResult};
+
 use crate::data::user::{FactorID, GeneralSettings, GrantedClients, TwoFactor, User, UserID};
 use crate::utils::err::Res;
 
 /// User storage interface
 pub trait UsersSyncBackend {
-    fn find_by_username_or_email(&self, u: &str) -> Option<User>;
+    fn find_by_username_or_email(&self, u: &str) -> Res<Option<User>>;
-    fn find_by_user_id(&self, id: &UserID) -> Option<User>;
+    fn find_by_user_id(&self, id: &UserID) -> Res<Option<User>>;
-    fn get_entire_users_list(&self) -> Vec<User>;
+    fn get_entire_users_list(&self) -> Res<Vec<User>>;
     fn create_user_account(&mut self, settings: GeneralSettings) -> Res<UserID>;
     fn set_general_user_settings(&mut self, settings: GeneralSettings) -> Res;
-    fn change_user_password(&mut self, id: &UserID, password: &str, temporary: bool) -> bool;
+    fn change_user_password(&mut self, id: &UserID, password: &str, temporary: bool) -> Res;
-    fn verify_user_password(&self, user: &UserID, password: &str) -> bool;
+    fn verify_user_password(&self, user: &UserID, password: &str) -> Res<bool>;
-    fn add_2fa_factor(&mut self, user: &UserID, factor: TwoFactor) -> bool;
+    fn add_2fa_factor(&mut self, user: &UserID, factor: TwoFactor) -> Res;
-    fn remove_2fa_factor(&mut self, user: &UserID, factor: FactorID) -> bool;
+    fn remove_2fa_factor(&mut self, user: &UserID, factor: FactorID) -> Res;
-    fn save_new_successful_2fa_authentication(&mut self, id: &UserID, ip: IpAddr) -> bool;
+    fn save_new_successful_2fa_authentication(&mut self, id: &UserID, ip: IpAddr) -> Res;
-    fn clear_2fa_login_history(&mut self, id: &UserID) -> bool;
+    fn clear_2fa_login_history(&mut self, id: &UserID) -> Res;
-    fn delete_account(&mut self, id: &UserID) -> bool;
+    fn delete_account(&mut self, id: &UserID) -> Res;
-    fn set_granted_2fa_clients(&mut self, id: &UserID, clients: GrantedClients) -> bool;
+    fn set_granted_2fa_clients(&mut self, id: &UserID, clients: GrantedClients) -> Res;
 }
 
 #[derive(Debug)]
 pub enum LoginResult {
+    Error,
     AccountNotFound,
     InvalidPassword,
     AccountDisabled,
@@ -55,11 +57,8 @@ pub struct FindUserByUsername(pub String);
 pub struct FindUserByUsernameResult(pub Option<User>);
 
 #[derive(Message)]
-#[rtype(GetAllUsersResult)]
+#[rtype(result = "Vec<User>")]
-pub struct GetAllUsersRequest;
+pub struct GetAllUsers;
-
-#[derive(Debug)]
-pub struct GetAllUsersResult(pub Vec<User>);
 
 #[derive(Message)]
 #[rtype(result = "Option<UserID>")]
@@ -125,9 +124,17 @@ impl Handler<LoginRequest> for UsersActor {
 
     fn handle(&mut self, msg: LoginRequest, _ctx: &mut Self::Context) -> Self::Result {
         match self.manager.find_by_username_or_email(&msg.login) {
-            None => MessageResult(LoginResult::AccountNotFound),
+            Err(e) => {
-            Some(user) => {
+                log::error!("Failed to find user! {}", e);
-                if !self.manager.verify_user_password(&user.uid, &msg.password) {
+                MessageResult(LoginResult::Error)
+            }
+            Ok(None) => MessageResult(LoginResult::AccountNotFound),
+            Ok(Some(user)) => {
+                if self
+                    .manager
+                    .verify_user_password(&user.uid, &msg.password)
+                    .unwrap_or(false)
+                {
                     return MessageResult(LoginResult::InvalidPassword);
                 }
 
@@ -159,8 +166,16 @@ impl Handler<ChangePasswordRequest> for UsersActor {
     type Result = <ChangePasswordRequest as actix::Message>::Result;
 
     fn handle(&mut self, msg: ChangePasswordRequest, _ctx: &mut Self::Context) -> Self::Result {
-        self.manager
+        match self
+            .manager
             .change_user_password(&msg.user_id, &msg.new_password, msg.temporary)
+        {
+            Ok(_) => true,
+            Err(e) => {
+                log::error!("Failed to change user password! {:?}", e);
+                false
+            }
+        }
     }
 }
 
@@ -168,7 +183,13 @@ impl Handler<Add2FAFactor> for UsersActor {
     type Result = <Add2FAFactor as actix::Message>::Result;
 
     fn handle(&mut self, msg: Add2FAFactor, _ctx: &mut Self::Context) -> Self::Result {
-        self.manager.add_2fa_factor(&msg.0, msg.1)
+        match self.manager.add_2fa_factor(&msg.0, msg.1) {
+            Ok(_) => true,
+            Err(e) => {
+                log::error!("Failed to add 2FA factor! {}", e);
+                false
+            }
+        }
     }
 }
 
@@ -176,7 +197,13 @@ impl Handler<Remove2FAFactor> for UsersActor {
     type Result = <Remove2FAFactor as actix::Message>::Result;
 
     fn handle(&mut self, msg: Remove2FAFactor, _ctx: &mut Self::Context) -> Self::Result {
-        self.manager.remove_2fa_factor(&msg.0, msg.1)
+        match self.manager.remove_2fa_factor(&msg.0, msg.1) {
+            Ok(_) => true,
+            Err(e) => {
+                log::error!("Failed to remove 2FA factor! {}", e);
+                false
+            }
+        }
     }
 }
 
@@ -184,22 +211,46 @@ impl Handler<AddSuccessful2FALogin> for UsersActor {
     type Result = <AddSuccessful2FALogin as actix::Message>::Result;
 
     fn handle(&mut self, msg: AddSuccessful2FALogin, _ctx: &mut Self::Context) -> Self::Result {
-        self.manager
+        match self
+            .manager
             .save_new_successful_2fa_authentication(&msg.0, msg.1)
+        {
+            Ok(_) => true,
+            Err(e) => {
+                log::error!("Failed to save successful 2FA authentication! {}", e);
+                false
+            }
+        }
     }
 }
 
 impl Handler<Clear2FALoginHistory> for UsersActor {
     type Result = <Clear2FALoginHistory as actix::Message>::Result;
     fn handle(&mut self, msg: Clear2FALoginHistory, _ctx: &mut Self::Context) -> Self::Result {
-        self.manager.clear_2fa_login_history(&msg.0)
+        match self.manager.clear_2fa_login_history(&msg.0) {
+            Ok(_) => true,
+            Err(e) => {
+                log::error!(
+                    "Failed to clear 2FA login history of user {:?} ! {}",
+                    msg.0,
+                    e
+                );
+                false
+            }
+        }
     }
 }
 
 impl Handler<SetGrantedClients> for UsersActor {
     type Result = <SetGrantedClients as actix::Message>::Result;
     fn handle(&mut self, msg: SetGrantedClients, _ctx: &mut Self::Context) -> Self::Result {
-        self.manager.set_granted_2fa_clients(&msg.0, msg.1)
+        match self.manager.set_granted_2fa_clients(&msg.0, msg.1) {
+            Ok(_) => true,
+            Err(e) => {
+                log::error!("Failed to set granted 2FA clients! {}", e);
+                false
+            }
+        }
     }
 }
 
@@ -207,7 +258,13 @@ impl Handler<GetUserRequest> for UsersActor {
     type Result = MessageResult<GetUserRequest>;
 
     fn handle(&mut self, msg: GetUserRequest, _ctx: &mut Self::Context) -> Self::Result {
-        MessageResult(GetUserResult(self.manager.find_by_user_id(&msg.0)))
+        MessageResult(GetUserResult(match self.manager.find_by_user_id(&msg.0) {
+            Ok(r) => r,
+            Err(e) => {
+                log::error!("Failed to find user by id! {}", e);
+                None
+            }
+        }))
     }
 }
 
@@ -215,7 +272,12 @@ impl Handler<VerifyUserPasswordRequest> for UsersActor {
     type Result = <VerifyUserPasswordRequest as actix::Message>::Result;
 
     fn handle(&mut self, msg: VerifyUserPasswordRequest, _ctx: &mut Self::Context) -> Self::Result {
-        self.manager.verify_user_password(&msg.0, &msg.1)
+        self.manager
+            .verify_user_password(&msg.0, &msg.1)
+            .unwrap_or_else(|e| {
+                log::error!("Failed to verify user password! {}", e);
+                false
+            })
     }
 }
 
@@ -224,16 +286,27 @@ impl Handler<FindUserByUsername> for UsersActor {
 
     fn handle(&mut self, msg: FindUserByUsername, _ctx: &mut Self::Context) -> Self::Result {
         MessageResult(FindUserByUsernameResult(
-            self.manager.find_by_username_or_email(&msg.0),
+            self.manager
+                .find_by_username_or_email(&msg.0)
+                .unwrap_or_else(|e| {
+                    log::error!("Failed to find user by username or email! {}", e);
+                    None
+                }),
         ))
     }
 }
 
-impl Handler<GetAllUsersRequest> for UsersActor {
+impl Handler<GetAllUsers> for UsersActor {
-    type Result = MessageResult<GetAllUsersRequest>;
+    type Result = <GetAllUsers as actix::Message>::Result;
 
-    fn handle(&mut self, _msg: GetAllUsersRequest, _ctx: &mut Self::Context) -> Self::Result {
+    fn handle(&mut self, _msg: GetAllUsers, _ctx: &mut Self::Context) -> Self::Result {
-        MessageResult(GetAllUsersResult(self.manager.get_entire_users_list()))
+        match self.manager.get_entire_users_list() {
+            Ok(r) => r,
+            Err(e) => {
+                log::error!("Failed to get entire users list! {}", e);
+                vec![]
+            }
+        }
     }
 }
 
@@ -255,6 +328,12 @@ impl Handler<DeleteUserRequest> for UsersActor {
     type Result = <DeleteUserRequest as actix::Message>::Result;
 
     fn handle(&mut self, msg: DeleteUserRequest, _ctx: &mut Self::Context) -> Self::Result {
-        self.manager.delete_account(&msg.0)
+        match self.manager.delete_account(&msg.0) {
+            Ok(_) => true,
+            Err(e) => {
+                log::error!("Failed to delete user account! {}", e);
+                false
+            }
+        }
     }
 }

src/controllers/admin_controller.rs

@@ -221,7 +221,7 @@ pub async fn users_route(
         }
     }
 
-    let users = users.send(users_actor::GetAllUsersRequest).await.unwrap().0;
+    let users = users.send(users_actor::GetAllUsers).await.unwrap();
 
     HttpResponse::Ok().body(
         UsersListTemplate {

src/data/users_file_entity.rs

@@ -1,27 +1,28 @@
+use std::net::IpAddr;
+
 use crate::actors::users_actor::UsersSyncBackend;
 use crate::data::entity_manager::EntityManager;
 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;
 
 impl EntityManager<User> {
     /// Update user information
-    fn update_user<F>(&mut self, id: &UserID, update: F) -> bool
+    fn update_user<F>(&mut self, id: &UserID, update: F) -> Res
     where
         F: FnOnce(User) -> User,
     {
-        let user = match self.find_by_user_id(id) {
+        let user = match self.find_by_user_id(id)? {
-            None => return false,
+            None => return new_error(format!("Failed to find user {:?}", id)),
             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;
+            return Err(e);
         }
 
-        true
+        Ok(())
     }
 }
 
@@ -40,26 +41,26 @@ fn verify_password<P: AsRef<[u8]>>(pwd: P, hash: &str) -> bool {
 }
 
 impl UsersSyncBackend for EntityManager<User> {
-    fn find_by_username_or_email(&self, u: &str) -> Option<User> {
+    fn find_by_username_or_email(&self, u: &str) -> Res<Option<User>> {
         for entry in self.iter() {
             if entry.username.eq(u) || entry.email.eq(u) {
-                return Some(entry.clone());
+                return Ok(Some(entry.clone()));
             }
         }
-        None
+        Ok(None)
     }
 
-    fn find_by_user_id(&self, id: &UserID) -> Option<User> {
+    fn find_by_user_id(&self, id: &UserID) -> Res<Option<User>> {
         for entry in self.iter() {
             if entry.uid.eq(id) {
-                return Some(entry.clone());
+                return Ok(Some(entry.clone()));
             }
         }
-        None
+        Ok(None)
     }
 
-    fn get_entire_users_list(&self) -> Vec<User> {
+    fn get_entire_users_list(&self) -> Res<Vec<User>> {
-        self.cloned()
+        Ok(self.cloned())
     }
 
     fn create_user_account(&mut self, settings: GeneralSettings) -> Res<UserID> {
@@ -73,25 +74,14 @@ impl UsersSyncBackend for EntityManager<User> {
     }
 
     fn set_general_user_settings(&mut self, settings: GeneralSettings) -> Res {
-        let res = self.update_user(&settings.uid.clone(), |mut user| {
+        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 {
+    fn change_user_password(&mut self, id: &UserID, password: &str, temporary: bool) -> Res {
-        let new_hash = match hash_password(password) {
+        let new_hash = 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;
@@ -101,27 +91,28 @@ impl UsersSyncBackend for EntityManager<User> {
         })
     }
 
-    fn verify_user_password(&self, user: &UserID, password: &str) -> bool {
+    fn verify_user_password(&self, user: &UserID, password: &str) -> Res<bool> {
-        self.find_by_user_id(user)
+        Ok(self
+            .find_by_user_id(user)?
             .map(|u| verify_password(password, &u.password))
-            .unwrap_or(false)
+            .unwrap_or(false))
     }
 
-    fn add_2fa_factor(&mut self, id: &UserID, factor: TwoFactor) -> bool {
+    fn add_2fa_factor(&mut self, id: &UserID, factor: TwoFactor) -> Res {
         self.update_user(id, |mut user| {
             user.two_factor.push(factor);
             user
         })
     }
 
-    fn remove_2fa_factor(&mut self, id: &UserID, factor_id: FactorID) -> bool {
+    fn remove_2fa_factor(&mut self, id: &UserID, factor_id: FactorID) -> Res {
         self.update_user(id, |mut user| {
             user.two_factor.retain(|f| f.id != factor_id);
             user
         })
     }
 
-    fn save_new_successful_2fa_authentication(&mut self, id: &UserID, ip: IpAddr) -> bool {
+    fn save_new_successful_2fa_authentication(&mut self, id: &UserID, ip: IpAddr) -> Res {
         self.update_user(id, |mut user| {
             user.last_successful_2fa.insert(ip, time());
 
@@ -132,35 +123,28 @@ impl UsersSyncBackend for EntityManager<User> {
         })
     }
 
-    fn clear_2fa_login_history(&mut self, id: &UserID) -> bool {
+    fn clear_2fa_login_history(&mut self, id: &UserID) -> Res {
         self.update_user(id, |mut user| {
             user.last_successful_2fa = Default::default();
             user
         })
     }
 
-    fn delete_account(&mut self, id: &UserID) -> bool {
+    fn delete_account(&mut self, id: &UserID) -> Res {
-        let user = match self.find_by_user_id(id) {
+        let user = match self.find_by_user_id(id)? {
             None => {
-                log::warn!(
+                return new_error(format!(
                     "Could not delete account {:?} because it was not found!",
                     id
-                );
+                ));
-                return false;
             }
             Some(s) => s,
         };
 
-        match self.remove(&user) {
+        self.remove(&user)
-            Ok(_) => true,
-            Err(e) => {
-                log::error!("Failed to update delete account! {:?}", e);
-                false
-            }
-        }
     }
 
-    fn set_granted_2fa_clients(&mut self, id: &UserID, clients: GrantedClients) -> bool {
+    fn set_granted_2fa_clients(&mut self, id: &UserID, clients: GrantedClients) -> Res {
         self.update_user(id, |mut user| {
             user.authorized_clients = clients.to_user();
             user

src/main.rs

@@ -61,7 +61,9 @@ async fn main() -> std::io::Result<()> {
             .expect("Failed to create initial user!");
 
         // Set default admin password
-        users.change_user_password(&default_admin.uid, DEFAULT_ADMIN_PASSWORD, true);
+        users
+            .change_user_password(&default_admin.uid, DEFAULT_ADMIN_PASSWORD, true)
+            .expect("Failed to change default admin password!");
     }
 
     let users_actor = UsersActor::new(users).start();