From bc414096e32da9a983448df08ef9316344e41014 Mon Sep 17 00:00:00 2001
From: div-seungha <div@cluml.com>
Date: Wed, 18 Dec 2024 12:44:33 +0900
Subject: [PATCH] Add `theme` field in `Account` to represent user's screen
 color theme

---
 CHANGELOG.md           |  22 ++-
 Cargo.toml             |   2 +-
 src/account.rs         |   7 +-
 src/migration.rs       | 294 ++++++++++++++++++++++++++++++++++++++---
 src/tables/accounts.rs |  11 ++
 5 files changed, 307 insertions(+), 29 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 0bd1c682..2617637e 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5,6 +5,13 @@ file is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and
 this project adheres to [Semantic
 Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [Unreleased]
+
+### Added
+
+- Added `Account::theme` field to represent user's selected screen color theme
+  on the user interface.
+
 ## [0.33.1] - 2024-12-20
 
 ### Fixed
@@ -339,7 +346,7 @@ Versioning](https://semver.org/spec/v2.0.0.html).
 - Changed the return type of `Store::network_map` to `IndexedTable<Network>`
   to enhance security by preventing direct exposure of internal structure.
 - Replaced `IndexedMap::get_by_id` function with `Indexed::get_by_id`, providing
-   a more structured and type-safe result.
+  a more structured and type-safe result.
   - Previously, the function returned a binary representation of the key-value
     pair: `Result<(Option<impl AsRef<[u8]>>, Option<impl AsRef<[u8]>>)>`.
   - Now, it returns `Result<Option<T>>`, where T is the entry type.
@@ -355,8 +362,8 @@ Versioning](https://semver.org/spec/v2.0.0.html).
   more straightforward and human-readable format compared to the raw binary
   format exposed by `IndexSet`.
 - Replaced the `IndexedTable<Category>::get`, `IndexedTable<Qualifier>::get` and
- `IndexedTable<Status>::get` method with the more general function
- `IndexedTable<R>::get_by_id`. This change enhances flexibility by allowing
+  `IndexedTable<Status>::get` method with the more general function
+  `IndexedTable<R>::get_by_id`. This change enhances flexibility by allowing
   retrieval based on any type R rather than being limited to a specific category.
   Existing code using get for categories should be updated to use get_by_id with
   the appropriate type.
@@ -398,7 +405,7 @@ Versioning](https://semver.org/spec/v2.0.0.html).
 - Added new functions to facilitate insert, remove, and update operations,
   ensuring a more controlled and secure template management.
 - Introduced `Structured`, `Unstructured`, `StructuredClusteringAlgorithm` and
- `UnstructuredClusteringAlgorithm` to describe data stored in `Table<Template>`.
+  `UnstructuredClusteringAlgorithm` to describe data stored in `Table<Template>`.
 - Introduced `TriageResponse` to describe data stored in `IndexedTable<TriageResponse>`.
 - Introduced `TriageResponseUpdate` to support `TriageResponse` record update.
 - Added new functions to facilitate insert, remove, and update operations,
@@ -430,7 +437,7 @@ Versioning](https://semver.org/spec/v2.0.0.html).
   enhance security by preventing direct exposure of `Map`.
 - The `get_by_id` method in the `IndexedMap` struct has been updated to return a
   key-value pair (`(Vec<u8>, Vec<u8>)`) instead of just the value (`impl
-  AsRef<[u8]>`). This change accommodates scenarios where the information stored
+AsRef<[u8]>`). This change accommodates scenarios where the information stored
   in a key may not be present in the value for some Column Families. Previously,
   if you called `get_by_id` with a specific ID, you would receive the
   corresponding value as `Option<impl AsRef<[u8]>>`. Now, calling `get_by_id`
@@ -619,7 +626,7 @@ Versioning](https://semver.org/spec/v2.0.0.html).
 - Removed `batch_info` and `scores` arguments from `Model::from_storage` function.
   These arguments were previously used for custom initialization of the
   `batch_info` and `scores` fields within the model. This change means that when
-  you create a model using `Model::from_storage`, the  `batch_info` and `scores`
+  you create a model using `Model::from_storage`, the `batch_info` and `scores`
   fields will now be initialized with their default values. If you previously
   relied on custom values for these fields, you will need to update your code accordingly.
 
@@ -707,6 +714,7 @@ Versioning](https://semver.org/spec/v2.0.0.html).
   Please note that if you are currently using a version of the application
   earlier than 0.12.0, database migration support has been deprecated and will
   no longer be available.
+
   - Users on versions prior to 0.12.0 will need to manually manage their database
     schema updates if they choose to continue using these older versions.
   - We highly recommend upgrading to the latest version (0.12.0 or later) to
@@ -744,6 +752,7 @@ Versioning](https://semver.org/spec/v2.0.0.html).
   code if you have been using the old naming convention. We apologise for any
   inconvenience this may cause, but we believe this change will bring greater
   consistency and readability to the codebase.
+
 - Removed `src_port` field from `FtpBruteForce` and `LdapBruteForce` events.
   to align with the event fields provided by hog.
 - Modified `LdapPlainText` fields to appropriate LDAP event fields from wrong
@@ -752,6 +761,7 @@ Versioning](https://semver.org/spec/v2.0.0.html).
 - Modified `FtpBruteForce` by adding an `is_internal` field which is a boolean
   indicating whether it is internal or not.
 
+[Unreleased]: https://github.com/petabi/review-database/compare/0.33.1...main
 [0.33.1]: https://github.com/petabi/review-database/compare/0.33.0...0.33.1
 [0.33.0]: https://github.com/petabi/review-database/compare/0.32.0...0.33.0
 [0.32.0]: https://github.com/petabi/review-database/compare/0.31.0...0.32.0
diff --git a/Cargo.toml b/Cargo.toml
index eef5060e..405e6270 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "review-database"
-version = "0.33.1"
+version = "0.34.0-alpha.1"
 edition = "2021"
 
 [dependencies]
diff --git a/src/account.rs b/src/account.rs
index f356fac8..f43e3dc1 100644
--- a/src/account.rs
+++ b/src/account.rs
@@ -36,6 +36,7 @@ pub struct Account {
     pub name: String,
     pub department: String,
     pub language: Option<String>,
+    pub theme: Option<String>,
     pub(crate) creation_time: DateTime<Utc>,
     pub(crate) last_signin_time: Option<DateTime<Utc>>,
     pub allow_access_from: Option<Vec<IpAddr>>,
@@ -60,6 +61,7 @@ impl Account {
         name: String,
         department: String,
         language: Option<String>,
+        theme: Option<String>,
         allow_access_from: Option<Vec<IpAddr>>,
         max_parallel_sessions: Option<u32>,
     ) -> Result<Self> {
@@ -72,6 +74,7 @@ impl Account {
             role,
             name,
             department,
+            theme,
             language,
             creation_time: now,
             last_signin_time: None,
@@ -170,7 +173,7 @@ impl SaltedPassword {
     /// # Errors
     ///
     /// Returns an error if the salt cannot be generated.
-    fn new_with_hash_algorithm(
+    pub(crate) fn new_with_hash_algorithm(
         password: &str,
         hash_algorithm: &PasswordHashAlgorithm,
     ) -> Result<Self> {
@@ -292,6 +295,7 @@ mod tests {
             None,
             None,
             None,
+            None,
         );
         assert!(account.is_ok());
 
@@ -319,6 +323,7 @@ mod tests {
             department: String::new(),
             name: String::new(),
             language: None,
+            theme: None,
             creation_time: Utc::now(),
             last_signin_time: None,
             allow_access_from: None,
diff --git a/src/migration.rs b/src/migration.rs
index 79601af8..1c660893 100644
--- a/src/migration.rs
+++ b/src/migration.rs
@@ -38,7 +38,7 @@ use crate::{Agent, AgentStatus, Giganto, Indexed, IterableMap};
 /// // the database format won't be changed in the future alpha or beta versions.
 /// const COMPATIBLE_VERSION: &str = ">=0.5.0-alpha.2,<=0.5.0-alpha.4";
 /// ```
-const COMPATIBLE_VERSION_REQ: &str = ">=0.30.0,<0.34.0-alpha";
+const COMPATIBLE_VERSION_REQ: &str = ">0.33.1,<=0.34.0-alpha.1";
 
 /// Migrates data exists in `PostgresQL` to Rocksdb if necessary.
 ///
@@ -136,6 +136,11 @@ pub fn migrate_data_dir<P: AsRef<Path>>(data_dir: P, backup_dir: P) -> Result<()
             Version::parse("0.30.0")?,
             migrate_0_29_to_0_30_0,
         ),
+        (
+            VersionReq::parse(">=0.33.1,<0.34.0-alpha.1")?,
+            Version::parse("0.34.0-alpha.1")?,
+            migrate_0_33_to_0_34_0,
+        ),
     ];
 
     let mut store = super::Store::new(data_dir, backup_dir)?;
@@ -211,6 +216,79 @@ fn read_version_file(path: &Path) -> Result<Version> {
     Version::parse(&ver).context("cannot parse VERSION")
 }
 
+fn migrate_0_33_to_0_34_0(store: &super::Store) -> Result<()> {
+    migrate_0_34_account(store)
+}
+
+fn migrate_0_34_account(store: &super::Store) -> Result<()> {
+    use bincode::Options;
+    use chrono::{DateTime, Utc};
+
+    use crate::account::{PasswordHashAlgorithm, Role, SaltedPassword};
+    use crate::types::Account;
+
+    #[derive(Deserialize, Serialize)]
+    pub struct OldAccount {
+        pub username: String,
+        password: SaltedPassword,
+        pub role: Role,
+        pub name: String,
+        pub department: String,
+        pub language: Option<String>,
+        creation_time: DateTime<Utc>,
+        last_signin_time: Option<DateTime<Utc>>,
+        pub allow_access_from: Option<Vec<IpAddr>>,
+        pub max_parallel_sessions: Option<u32>,
+        password_hash_algorithm: PasswordHashAlgorithm,
+        password_last_modified_at: DateTime<Utc>,
+    }
+
+    impl From<OldAccount> for Account {
+        fn from(input: OldAccount) -> Self {
+            Self {
+                username: input.username,
+                password: input.password,
+                role: input.role,
+                name: input.name,
+                department: input.department,
+                language: input.language,
+                theme: None,
+                creation_time: input.creation_time,
+                last_signin_time: input.last_signin_time,
+                allow_access_from: input.allow_access_from,
+                max_parallel_sessions: input.max_parallel_sessions,
+                password_hash_algorithm: input.password_hash_algorithm,
+                password_last_modified_at: input.password_last_modified_at,
+            }
+        }
+    }
+
+    let map = store.account_map();
+    let raw = map.raw();
+    let mut accounts = vec![];
+    for (key, old_value) in raw.iter_forward()? {
+        match bincode::DefaultOptions::new().deserialize::<OldAccount>(&old_value) {
+            Ok(old) => {
+                raw.delete(&key)?;
+                accounts.push(old.into());
+            }
+            Err(e) => {
+                return Err(anyhow::anyhow!("Failed to deserialize account: {}", e));
+            }
+        }
+    }
+    for account in accounts {
+        map.insert(&account).map_err(|e| {
+            anyhow::anyhow!(
+                "Failed to insert migrated account {}: {}",
+                account.username,
+                e
+            )
+        })?;
+    }
+    Ok(())
+}
+
 fn migrate_0_29_to_0_30_0(store: &super::Store) -> Result<()> {
     migrate_0_30_tidb(store)?;
     migrate_0_30_event_struct(store)
@@ -1016,7 +1094,6 @@ fn migrate_0_29_account(store: &super::Store) -> Result<()> {
     use chrono::{DateTime, Utc};
 
     use crate::account::{PasswordHashAlgorithm, Role, SaltedPassword};
-    use crate::types::Account;
 
     #[derive(Deserialize, Serialize)]
     pub struct OldAccount {
@@ -1032,7 +1109,23 @@ fn migrate_0_29_account(store: &super::Store) -> Result<()> {
         password_hash_algorithm: PasswordHashAlgorithm,
     }
 
-    impl From<OldAccount> for Account {
+    #[derive(Deserialize, Serialize)]
+    pub struct AccountV29 {
+        pub username: String,
+        password: SaltedPassword,
+        pub role: Role,
+        pub name: String,
+        pub department: String,
+        language: Option<String>,
+        creation_time: DateTime<Utc>,
+        last_signin_time: Option<DateTime<Utc>>,
+        pub allow_access_from: Option<Vec<IpAddr>>,
+        pub max_parallel_sessions: Option<u32>,
+        password_hash_algorithm: PasswordHashAlgorithm,
+        password_last_modified_at: DateTime<Utc>,
+    }
+
+    impl From<OldAccount> for AccountV29 {
         fn from(input: OldAccount) -> Self {
             Self {
                 username: input.username,
@@ -1053,14 +1146,11 @@ fn migrate_0_29_account(store: &super::Store) -> Result<()> {
 
     let map = store.account_map();
     let raw = map.raw();
-    let mut accounts = vec![];
     for (key, old_value) in raw.iter_forward()? {
         let old = bincode::DefaultOptions::new().deserialize::<OldAccount>(&old_value)?;
-        raw.delete(&key)?;
-        accounts.push(old.into());
-    }
-    for account in accounts {
-        map.insert(&account)?;
+        let new: AccountV29 = old.into();
+        let new_value = bincode::DefaultOptions::new().serialize::<AccountV29>(&new)?;
+        raw.update((&key, &old_value), (&key, &new_value))?;
     }
     Ok(())
 }
@@ -3441,12 +3531,12 @@ mod tests {
     fn migrate_0_26_to_0_29_account() {
         use std::net::IpAddr;
 
+        use anyhow::Result;
         use bincode::Options;
         use chrono::{DateTime, Utc};
         use serde::{Deserialize, Serialize};
 
         use crate::account::{PasswordHashAlgorithm, Role, SaltedPassword};
-        use crate::types::Account;
 
         #[derive(Deserialize, Serialize)]
         pub struct OldAccount {
@@ -3462,7 +3552,58 @@ mod tests {
             password_hash_algorithm: PasswordHashAlgorithm,
         }
 
-        impl From<OldAccount> for Account {
+        #[derive(Clone, Deserialize, Serialize, PartialEq, Debug)]
+        pub struct AccountV29 {
+            pub username: String,
+            password: SaltedPassword,
+            pub role: Role,
+            pub name: String,
+            pub department: String,
+            language: Option<String>,
+            creation_time: DateTime<Utc>,
+            last_signin_time: Option<DateTime<Utc>>,
+            pub allow_access_from: Option<Vec<IpAddr>>,
+            pub max_parallel_sessions: Option<u32>,
+            password_hash_algorithm: PasswordHashAlgorithm,
+            password_last_modified_at: DateTime<Utc>,
+        }
+
+        impl AccountV29 {
+            const DEFAULT_HASH_ALGORITHM: PasswordHashAlgorithm = PasswordHashAlgorithm::Argon2id;
+            #[allow(clippy::too_many_arguments)]
+            fn new(
+                username: &str,
+                password: &str,
+                role: Role,
+                name: String,
+                department: String,
+                language: Option<String>,
+                allow_access_from: Option<Vec<IpAddr>>,
+                max_parallel_sessions: Option<u32>,
+            ) -> Result<Self> {
+                let password = SaltedPassword::new_with_hash_algorithm(
+                    password,
+                    &Self::DEFAULT_HASH_ALGORITHM,
+                )?;
+                let now = Utc::now();
+                Ok(Self {
+                    username: username.to_string(),
+                    password,
+                    role,
+                    name,
+                    department,
+                    language,
+                    creation_time: now,
+                    last_signin_time: None,
+                    allow_access_from,
+                    max_parallel_sessions,
+                    password_hash_algorithm: Self::DEFAULT_HASH_ALGORITHM,
+                    password_last_modified_at: now,
+                })
+            }
+        }
+
+        impl From<OldAccount> for AccountV29 {
             fn from(input: OldAccount) -> Self {
                 Self {
                     username: input.username,
@@ -3481,8 +3622,8 @@ mod tests {
             }
         }
 
-        impl From<Account> for OldAccount {
-            fn from(input: Account) -> Self {
+        impl From<AccountV29> for OldAccount {
+            fn from(input: AccountV29) -> Self {
                 Self {
                     username: input.username,
                     password: input.password,
@@ -3502,7 +3643,7 @@ mod tests {
         let map = settings.store.account_map();
         let raw = map.raw();
 
-        let mut test = Account::new(
+        let mut test = AccountV29::new(
             "test",
             "password",
             Role::SecurityAdministrator,
@@ -3526,13 +3667,21 @@ mod tests {
         assert!(super::migrate_0_29_account(&settings.store).is_ok());
 
         let map = settings.store.account_map();
-        let res = map.get(&test.username);
-        assert!(res.is_ok());
-        let account = res.unwrap();
-        if let Some(a) = &account {
-            test.password_last_modified_at = a.password_last_modified_at;
-        }
-        assert_eq!(account, Some(test));
+        let raw = map.raw();
+        let raw_value = raw
+            .get(test.username.as_bytes())
+            .expect("Failed to get raw value from database");
+        assert!(raw_value.is_some());
+
+        let raw_owned = raw_value.unwrap();
+        let raw_bytes = raw_owned.as_ref();
+
+        let account: AccountV29 = bincode::DefaultOptions::new()
+            .deserialize(raw_bytes)
+            .expect("Failed to deserialize into AccountV29");
+
+        test.password_last_modified_at = account.password_last_modified_at;
+        assert_eq!(account, test);
     }
 
     #[test]
@@ -3614,4 +3763,107 @@ mod tests {
             assert_eq!(rule.category, EventCategory::Reconnaissance);
         });
     }
+
+    #[test]
+    fn migrate_0_34_account() {
+        use std::net::IpAddr;
+
+        use bincode::Options;
+        use chrono::{DateTime, Utc};
+        use serde::{Deserialize, Serialize};
+
+        use crate::account::{PasswordHashAlgorithm, Role, SaltedPassword};
+        use crate::types::Account;
+
+        #[derive(Deserialize, Serialize)]
+        pub struct OldAccount {
+            pub username: String,
+            password: SaltedPassword,
+            pub role: Role,
+            pub name: String,
+            pub department: String,
+            language: Option<String>,
+            creation_time: DateTime<Utc>,
+            last_signin_time: Option<DateTime<Utc>>,
+            pub allow_access_from: Option<Vec<IpAddr>>,
+            pub max_parallel_sessions: Option<u32>,
+            password_hash_algorithm: PasswordHashAlgorithm,
+            password_last_modified_at: DateTime<Utc>,
+        }
+
+        impl From<OldAccount> for Account {
+            fn from(input: OldAccount) -> Self {
+                Self {
+                    username: input.username,
+                    password: input.password,
+                    role: input.role,
+                    name: input.name,
+                    department: input.department,
+                    language: input.language,
+                    theme: None,
+                    creation_time: input.creation_time,
+                    last_signin_time: input.last_signin_time,
+                    allow_access_from: input.allow_access_from,
+                    max_parallel_sessions: input.max_parallel_sessions,
+                    password_hash_algorithm: input.password_hash_algorithm,
+                    password_last_modified_at: input.password_last_modified_at,
+                }
+            }
+        }
+
+        impl From<Account> for OldAccount {
+            fn from(input: Account) -> Self {
+                Self {
+                    username: input.username,
+                    password: input.password,
+                    role: input.role,
+                    name: input.name,
+                    department: input.department,
+                    language: input.language,
+                    creation_time: input.creation_time,
+                    last_signin_time: input.last_signin_time,
+                    allow_access_from: input.allow_access_from,
+                    max_parallel_sessions: input.max_parallel_sessions,
+                    password_hash_algorithm: input.password_hash_algorithm,
+                    password_last_modified_at: input.password_last_modified_at,
+                }
+            }
+        }
+
+        let settings = TestSchema::new();
+        let map = settings.store.account_map();
+        let raw = map.raw();
+
+        let test = Account::new(
+            "test",
+            "password",
+            Role::SecurityAdministrator,
+            "name".to_string(),
+            "department".to_string(),
+            None,
+            None,
+            None,
+            None,
+        )
+        .unwrap();
+
+        let old: OldAccount = test.clone().into();
+        let value = bincode::DefaultOptions::new()
+            .serialize(&old)
+            .expect("serializable");
+
+        assert!(raw.put(old.username.as_bytes(), &value).is_ok());
+
+        let (db_dir, backup_dir) = settings.close();
+        let settings = TestSchema::new_with_dir(db_dir, backup_dir);
+
+        assert!(super::migrate_0_34_account(&settings.store).is_ok());
+
+        let map = settings.store.account_map();
+        let res = map.get(&test.username);
+        assert!(res.is_ok());
+        let account = res.unwrap();
+
+        assert_eq!(account, Some(test));
+    }
 }
diff --git a/src/tables/accounts.rs b/src/tables/accounts.rs
index da4f8932..eec4dc66 100644
--- a/src/tables/accounts.rs
+++ b/src/tables/accounts.rs
@@ -77,6 +77,7 @@ impl<'d> Table<'d, Account> {
         name: &Option<(String, String)>,
         department: &Option<(String, String)>,
         language: &Option<(Option<String>, Option<String>)>,
+        theme: &Option<(Option<String>, Option<String>)>,
         allow_access_from: &Option<(Option<Vec<IpAddr>>, Option<Vec<IpAddr>>)>,
         max_parallel_sessions: &Option<(Option<u32>, Option<u32>)>,
     ) -> Result<(), anyhow::Error> {
@@ -117,6 +118,12 @@ impl<'d> Table<'d, Account> {
                     }
                     account.language.clone_from(new);
                 }
+                if let Some((old, new)) = theme {
+                    if account.theme != *old {
+                        bail!("old value mismatch");
+                    }
+                    account.theme.clone_from(new);
+                }
                 if let Some((old, new)) = &allow_access_from {
                     if account.allow_access_from != *old {
                         bail!("old value mismatch");
@@ -177,6 +184,7 @@ mod tests {
             None,
             None,
             None,
+            None,
         )
         .unwrap();
         table.put(&acc1).unwrap();
@@ -191,6 +199,7 @@ mod tests {
             None,
             None,
             None,
+            None,
         )
         .unwrap();
         table.put(&acc2).unwrap();
@@ -221,6 +230,7 @@ mod tests {
             None,
             None,
             None,
+            None,
         )
         .unwrap();
         table.put(&acc1).unwrap();
@@ -239,6 +249,7 @@ mod tests {
             None,
             None,
             None,
+            None,
         )
         .unwrap();
         table.put(&acc2).unwrap();