Refactor users management (#7)

* Improve general settings management by admin

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<User>;
     fn find_by_user_id(&self, id: &UserID) -> Option<User>;
     fn get_entire_users_list(&self) -> 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 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<User>);
 
+#[derive(Message)]
+#[rtype(result = "Option<UserID>")]
+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<dyn UsersBackend>,
+    manager: Box<dyn UsersSyncBackend>,
 }
 
 impl UsersActor {
     pub fn new<E>(manager: E) -> Self
     where
-        E: UsersBackend + 'static,
+        E: UsersSyncBackend + 'static,
     {
         Self {
             manager: Box::new(manager),
@@ -138,6 +141,20 @@ impl Handler<LoginRequest> for UsersActor {
     }
 }
 
+impl Handler<CreateAccount> for UsersActor {
+    type Result = <CreateAccount as actix::Message>::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<ChangePasswordRequest> for UsersActor {
     type Result = <ChangePasswordRequest as actix::Message>::Result;
 
@@ -220,14 +237,14 @@ impl Handler<GetAllUsersRequest> for UsersActor {
     }
 }
 
-impl Handler<UpdateUserRequest> for UsersActor {
-    type Result = <UpdateUserRequest as actix::Message>::Result;
+impl Handler<UpdateUserSettings> for UsersActor {
+    type Result = <UpdateUserSettings as actix::Message>::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
             }
         }

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<User> = users
-            .send(users_actor::FindUserByUsername(update.username.to_string()))
+        let edited_user: Option<User> = 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::<Vec<_>>();
-        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
                 ));
             }

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(),

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<P: AsRef<[u8]>>(pwd: P, hash: &str) -> bool {
     }
 }
 
-impl UsersBackend for EntityManager<User> {
+impl UsersSyncBackend for EntityManager<User> {
     fn find_by_username_or_email(&self, u: &str) -> Option<User> {
         for entry in self.iter() {
             if entry.username.eq(u) || entry.email.eq(u) {
@@ -62,6 +62,28 @@ impl UsersBackend for EntityManager<User> {
         self.cloned()
     }
 
+    fn create_user_account(&mut self, settings: GeneralSettings) -> Res<UserID> {
+        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> {
             user
         })
     }
-
-    fn update_or_insert_user(&mut self, user: User) -> Res {
-        self.update_or_push(user)
-    }
 }

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::*;

src/utils/err.rs

@@ -1,2 +1,34 @@
 use std::error::Error;
+use std::fmt;
+use std::fmt::{Display, Formatter};
+
 pub type Res<A = ()> = Result<A, Box<dyn Error>>;
+
+#[derive(Debug, Clone)]
+pub struct ExecError(pub String);
+
+impl ExecError {
+    pub fn new(msg: &str) -> ExecError {
+        ExecError(msg.to_string())
+    }
+
+    pub fn boxed_new<D: Display>(msg: D) -> Box<ExecError> {
+        Box::new(ExecError(msg.to_string()))
+    }
+
+    pub fn boxed_string(msg: String) -> Box<ExecError> {
+        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<D: Display>(msg: D) -> Res {
+    Err(ExecError::boxed_new(msg))
+}