From 00584c2f27a122d9216ad3b489422e61c70d4fad Mon Sep 17 00:00:00 2001 From: Emi Tatsuo Date: Wed, 16 Dec 2020 19:20:54 -0500 Subject: [PATCH] Switch to using UIDs --- src/types/request.rs | 2 +- src/user_management/manager.rs | 73 ++++++++++++++++++-------- src/user_management/mod.rs | 29 ++++++++++- src/user_management/user.rs | 95 +++++++++++++++++++++++----------- 4 files changed, 147 insertions(+), 52 deletions(-) diff --git a/src/types/request.rs b/src/types/request.rs index 0f9825d..1141bdb 100644 --- a/src/types/request.rs +++ b/src/types/request.rs @@ -296,7 +296,7 @@ impl Request { where UserData: Serialize + DeserializeOwned { - Ok(self.manager.get_user(self.certificate())?) + Ok(self.manager.get_user_by_cert(self.certificate())?) } #[cfg(feature="user_management")] diff --git a/src/user_management/manager.rs b/src/user_management/manager.rs index c7b6bd6..7d1087c 100644 --- a/src/user_management/manager.rs +++ b/src/user_management/manager.rs @@ -1,5 +1,7 @@ use serde::{Serialize, de::DeserializeOwned}; +use std::convert::TryInto; + use crate::user_management::{User, Result}; use crate::user_management::user::{RegisteredUser, NotSignedInUser, PartialUser}; @@ -21,8 +23,9 @@ pub struct UserManager { /// [`lookup_user()`]: Self::lookup_user /// [`lookup_certificate()`]: Self::lookup_certificate pub db: sled::Db, - pub (crate) users: sled::Tree, // user_id:String maps to data:UserData - pub (crate) certificates: sled::Tree, // certificate:u64 maps to data:CertificateData + pub (crate) users: sled::Tree, // user_id:u64 maps to data:PartialUser + pub (crate) certificates: sled::Tree, // fingerprint:[u8; 32] maps to uid:u64 + pub (crate) usernames: sled::Tree, // username:String maps to uid:u64 } impl UserManager { @@ -35,6 +38,7 @@ impl UserManager { Ok(Self { users: db.open_tree("gay.emii.kochab.users")?, certificates: db.open_tree("gay.emii.kochab.certificates")?, + usernames: db.open_tree("gay.emii.kochab.usernames")?, db, }) } @@ -44,29 +48,60 @@ impl UserManager { /// # Errors /// An error is thrown if there is an error reading from the database or if data /// recieved from the database is corrupt - pub fn lookup_certificate(&self, cert: [u8; 32]) -> Result> { + /// + /// [`None`] can be returned if their is no user with this certificate. + pub fn lookup_certificate( + &self, + cert: [u8; 32] + ) -> Result>> { if let Some(bytes) = self.certificates.get(cert)? { - Ok(Some(std::str::from_utf8(bytes.as_ref())?.to_string())) + let id = u64::from_le_bytes(bytes.as_ref().try_into()?); + Ok(Some( + self.lookup_user(id)? + .ok_or(super::DeserializeError::InvalidReference(id))? + )) } else { Ok(None) } } - /// Lookup information about a user by username + /// Get the user with the specified username + /// + /// # Errors + /// An error is thrown if there is an error reading from the database or if data + /// recieved from the database is corrupt + /// + /// [`None`] can be returned if their is no user with this username. + pub fn lookup_username( + &self, + username: impl AsRef + ) -> Result>> { + if let Some(bytes) = self.usernames.get(username.as_ref())? { + let id = u64::from_le_bytes(bytes.as_ref().try_into()?); + Ok(Some( + self.lookup_user(id)? + .ok_or(super::DeserializeError::InvalidReference(id))? + )) + } else { + Ok(None) + } + } + + /// Lookup a user by uid /// /// # Errors /// An error is thrown if there is an error reading from the database or if data /// recieved from the database is corrupt pub fn lookup_user( &self, - username: impl AsRef + uid: u64, ) -> Result>> where UserData: Serialize + DeserializeOwned { - if let Some(bytes) = self.users.get(username.as_ref())? { + if let Some(bytes) = self.users.get(uid.to_le_bytes())? { let inner: PartialUser = bincode::deserialize_from(bytes.as_ref())?; - Ok(Some(RegisteredUser::new(username.as_ref().to_owned(), None, self.clone(), inner))) + Ok(Some(RegisteredUser::new(uid, None, self.clone(), inner))) } else { Ok(None) } @@ -79,20 +114,18 @@ impl UserManager { /// the database is corrupt pub fn all_users( &self, - ) -> Vec> + ) -> impl Iterator>> where UserData: Serialize + DeserializeOwned { + let this = self.clone(); self.users.iter() - .map(|result| { - let (username, bytes) = result.expect("Failed to connect to database"); - let inner: PartialUser = bincode::deserialize_from(bytes.as_ref()) - .expect("Received malformed data from database"); - let username = String::from_utf8(username.to_vec()) - .expect("Malformed username in database"); - RegisteredUser::new(username, None, self.clone(), inner) + .map(move|result| { + let (uid, bytes) = result?; + let inner: PartialUser = bincode::deserialize_from(bytes.as_ref())?; + let uid = u64::from_le_bytes(uid.as_ref().try_into()?); + Ok(RegisteredUser::new(uid, None, this.clone(), inner)) }) - .collect() } /// Attempt to determine the user who sent a request based on the certificate. @@ -103,7 +136,7 @@ impl UserManager { /// /// # Panics /// Pancis if the database is corrupt - pub fn get_user( + pub fn get_user_by_cert( &self, cert: Option<&[u8; 32]> ) -> Result> @@ -111,9 +144,7 @@ impl UserManager { UserData: Serialize + DeserializeOwned { if let Some(certificate) = cert { - if let Some(username) = self.lookup_certificate(*certificate)? { - let user_inner = self.lookup_user(&username)? - .expect("Database corruption: Certificate data refers to non-existant user"); + if let Some(user_inner) = self.lookup_certificate(*certificate)? { Ok(User::SignedIn(user_inner.with_cert(*certificate))) } else { Ok(User::NotSignedIn(NotSignedInUser { diff --git a/src/user_management/mod.rs b/src/user_management/mod.rs index fe9141c..681c44a 100644 --- a/src/user_management/mod.rs +++ b/src/user_management/mod.rs @@ -93,12 +93,24 @@ impl From for UserManagerError { } } +impl From for UserManagerError { + fn from(error: std::array::TryFromSliceError) -> Self { + Self::DeserializeError(error.into()) + } +} + impl From for UserManagerError { fn from(error: std::str::Utf8Error) -> Self { Self::DeserializeError(error.into()) } } +impl From for UserManagerError { + fn from(error: DeserializeError) -> Self { + Self::DeserializeError(error) + } +} + #[cfg(feature = "user_management_advanced")] impl From for UserManagerError { fn from(error: argon2::Error) -> Self { @@ -159,12 +171,18 @@ pub enum DeserializeError { /// version, or you're using a database that was created by a version of kochab that /// was released after the one you're currently using. However, as of right now, no /// such release exists or is planned. - UidError(std::str::Utf8Error), + UidError(std::array::TryFromSliceError), /// There was an error deserializing a username /// /// Indicates data corruption or a misaligned database version UsernameError(std::str::Utf8Error), + + /// A certificate or username was linked to a non-existant user id + /// + /// This error should not occur, except in very unlikely scenarios, or if there's a + /// bug with kochab + InvalidReference(u64), } impl From for DeserializeError { @@ -173,6 +191,12 @@ impl From for DeserializeError { } } +impl From for DeserializeError { + fn from(error: std::array::TryFromSliceError) -> Self { + Self::UidError(error) + } +} + impl From for DeserializeError { fn from(error: std::str::Utf8Error) -> Self { Self::UsernameError(error) @@ -185,6 +209,7 @@ impl std::error::Error for DeserializeError { Self::StructError(e) => Some(e), Self::UidError(e) => Some(e), Self::UsernameError(e) => Some(e), + Self::InvalidReference(_) => None, } } } @@ -198,6 +223,8 @@ impl std::fmt::Display for DeserializeError { write!(f, "Got wrong number of bytes while deserializing user ID: {}", e), Self::UsernameError(e) => write!(f, "Got invalid UTF-8 instead of username: {}", e), + Self::InvalidReference(uid) => + write!(f, "Database refers to nonexistant user with userid {}", uid), } } } diff --git a/src/user_management/user.rs b/src/user_management/user.rs index 56695b3..34e9924 100644 --- a/src/user_management/user.rs +++ b/src/user_management/user.rs @@ -48,6 +48,7 @@ lazy_static::lazy_static! { pub (crate) struct PartialUser { pub data: UserData, pub certificates: Vec<[u8; 32]>, + pub username: String, #[cfg(feature = "user_management_advanced")] pub pass_hash: Option<(Vec, [u8; 32])>, } @@ -58,12 +59,12 @@ impl PartialUser { /// /// This MUST be called if the user data is modified using the AsMut trait, or else /// changes will not be written to the database - fn store(&self, tree: &sled::Tree, username: impl AsRef<[u8]>) -> Result<()> + fn store(&self, tree: &sled::Tree, uid: u64) -> Result<()> where UserData: Serialize { tree.insert( - &username, + uid.to_le_bytes(), bincode::serialize(&self)?, )?; Ok(()) @@ -117,26 +118,48 @@ impl NotSignedInUser { if self.manager.users.contains_key(username.as_str())? { Err(super::UserManagerError::UsernameNotUnique) } else { - info!("User {} registered!", username); - let mut newser = RegisteredUser::new( + // Create the partial user that will go into the database. We can't create + // the full user yet, since the ID won't be generated until we perform the + // insert. + let partial = PartialUser { username, + data: UserData::default(), + certificates: vec![self.certificate], + #[cfg(feature = "user_management_advanced")] + pass_hash: None, + }; + let serialized = bincode::serialize(&partial)?; + + // Insert the user into the three relevant tables, thus finalizing their + // creation. This also produces the user id. + let id = (&self.manager.users, &self.manager.certificates, &self.manager.usernames) + .transaction(|(tx_usr, tx_crt, tx_nam)| { + let id = tx_usr.generate_id()?; + let id_bytes = id.to_le_bytes(); + + tx_usr.insert( + &id_bytes, + serialized.as_slice(), + )?; + tx_crt.insert( + &partial.certificates[0], + &id_bytes, + )?; + tx_nam.insert( + partial.username.as_bytes(), + &id_bytes, + )?; + Ok(id) + })?; + info!("User {}#{:08x} registered!", partial.username, id); + + Ok(RegisteredUser::new( + id, Some(self.certificate), self.manager, - PartialUser { - data: UserData::default(), - certificates: Vec::with_capacity(1), - #[cfg(feature = "user_management_advanced")] - pass_hash: None, - }, - ); - - // As a nice bonus, calling add_certificate with a user not yet in the - // database creates the user and adds the certificate in a single transaction. - // Because of this, we can delegate here ^^ - newser.add_certificate(self.certificate)?; - - Ok(newser) + partial, + )) } } @@ -172,7 +195,7 @@ impl NotSignedInUser { username: &str, password: Option<&[u8]>, ) -> Result>> { - if let Some(mut user) = self.manager.lookup_user(username)? { + if let Some(mut user) = self.manager.lookup_username(username)? { // Perform password check, if caller wants if let Some(password) = password { if !user.check_password(password)? { @@ -195,7 +218,7 @@ impl NotSignedInUser { /// /// For more information about the user lifecycle and sign-in stages, see [`User`] pub struct RegisteredUser { - username: String, + uid: u64, active_certificate: Option<[u8; 32]>, manager: UserManager, inner: PartialUser, @@ -207,13 +230,13 @@ impl RegisteredUser { /// Create a new user from parts pub (crate) fn new( - username: String, + uid: u64, active_certificate: Option<[u8; 32]>, manager: UserManager, inner: PartialUser ) -> Self { Self { - username, + uid, active_certificate, manager, inner, @@ -246,9 +269,19 @@ impl RegisteredUser { /// Get the user's current username. /// - /// NOTE: This is not guaranteed not to change. + /// NOTE: This is not guaranteed not to change. If you need an immutable reference to + /// this user, prefer their [UID], which is guaranteed static. + /// + /// [UID]: Self::uid() pub fn username(&self) -> &String { - &self.username + &self.inner.username + } + + /// Get the user's id. + /// + /// This is not guaranteed not to change. + pub fn uid(&self) -> u64 { + self.uid } #[cfg(feature = "user_management_advanced")] @@ -328,7 +361,7 @@ impl RegisteredUser { where UserData: Serialize { - self.inner.store(&self.manager.users, &self.username)?; + self.inner.store(&self.manager.users, self.uid)?; self.has_changed = false; Ok(()) } @@ -346,16 +379,17 @@ impl RegisteredUser { self.inner.certificates.push(certificate); let inner_serialized = bincode::serialize(&self.inner)?; + let uid_bytes = self.uid.to_le_bytes(); (&self.manager.users, &self.manager.certificates) .transaction(|(tx_usr, tx_crt)| { tx_usr.insert( - self.username.as_str(), + &uid_bytes, inner_serialized.clone(), )?; tx_crt.insert( &certificate, - self.username.as_bytes(), + &uid_bytes, )?; Ok(()) })?; @@ -384,9 +418,12 @@ impl RegisteredUser { pub fn delete(self) -> Result<()> { let certificates = self.all_certificates(); - (&self.manager.users, &self.manager.certificates).transaction(|(tx_usr, tx_crt)| { + (&self.manager.users, &self.manager.certificates, &self.manager.usernames).transaction(|(tx_usr, tx_crt, tx_nam)| { + tx_nam.remove( + self.inner.username.as_str(), + )?; tx_usr.remove( - self.username.as_str(), + &self.uid.to_le_bytes(), )?; for cert in certificates { tx_crt.remove(