From 2f1196228fdbc7ca4157fefd5b25641725c3f839 Mon Sep 17 00:00:00 2001 From: Emi Tatsuo Date: Thu, 19 Nov 2020 16:54:29 -0500 Subject: [PATCH] Return a full SignedInUser from lookup_user(), restrict access to PartialUser Okay this is a much nicer API tbh. Why didn't I do this from the start? 'cause I'm bad at planning that's why. We got there eventually though! --- src/user_management/manager.rs | 16 ++++------ src/user_management/user.rs | 54 +++++++++++----------------------- 2 files changed, 23 insertions(+), 47 deletions(-) diff --git a/src/user_management/manager.rs b/src/user_management/manager.rs index da25cf9..015132a 100644 --- a/src/user_management/manager.rs +++ b/src/user_management/manager.rs @@ -73,13 +73,14 @@ impl UserManager { /// recieved from the database is corrupt pub fn lookup_user( &self, - username: impl AsRef<[u8]> - ) -> Result>> + username: impl AsRef + ) -> Result>> where UserData: Serialize + DeserializeOwned { - if let Some(bytes) = self.users.get(username)? { - Ok(Some(bincode::deserialize_from(bytes.as_ref())?)) + if let Some(bytes) = self.users.get(username.as_ref())? { + let inner: PartialUser = bincode::deserialize_from(bytes.as_ref())?; + Ok(Some(SignedInUser::new(username.as_ref().to_owned(), None, self.clone(), inner))) } else { Ok(None) } @@ -105,12 +106,7 @@ impl UserManager { if let Some(certificate_data) = self.lookup_certificate(cert_hash)? { let user_inner = self.lookup_user(&certificate_data.owner_username)? .expect("Database corruption: Certificate data refers to non-existant user"); - Ok(User::SignedIn(SignedInUser::new( - certificate_data.owner_username, - certificate_data.certificate, - self.clone(), - user_inner, - ))) + Ok(User::SignedIn(user_inner.with_cert(certificate_data.certificate))) } else { Ok(User::NotSignedIn(NotSignedInUser { certificate: certificate.clone(), diff --git a/src/user_management/user.rs b/src/user_management/user.rs index a1ad27c..96cdad2 100644 --- a/src/user_management/user.rs +++ b/src/user_management/user.rs @@ -10,14 +10,9 @@ //! information, like the user's username and active certificate. //! //! [`SignedInUser`] is particularly signifigant in that this is the struct used to modify -//! the data stored for the current user. This is accomplished through the +//! the data stored for almost all users. This is accomplished through the //! [`as_mut()`](SignedInUser::as_mut) method. Changes made this way must be persisted //! using [`save()`](SignedInUser::save()) or by dropping the user. -//! -//! [`PartialUser`] is the main way of modifying data stored for users who aren't -//! currently connecting. These are mainly obtained through the -//! [`UserManager::lookup_user()`] method. Unlinke with [`SignedInUser`], these are not -//! committed on drop, and [`PartialUser::store()`] must be manually called use rustls::Certificate; use serde::{Deserialize, Serialize, de::DeserializeOwned}; use sled::Transactional; @@ -47,7 +42,7 @@ lazy_static::lazy_static! { /// In order to generate a full user obj, you need to perform a lookup with a specific /// certificate. #[derive(Clone, Debug, Deserialize, Serialize)] -pub struct PartialUser { +pub (crate) struct PartialUser { pub data: UserData, pub certificates: Vec, pub pass_hash: Option<(Vec, [u8; 32])>, @@ -55,18 +50,11 @@ pub struct PartialUser { impl PartialUser { - /// A list of certificate hashes registered to this user - /// - /// Can be looked up using [`UserManager::lookup_certificate()`] to get full information - pub const fn certificates(&self) -> &Vec { - &self.certificates - } - /// Write user data to the database /// /// This MUST be called if the user data is modified using the AsMut trait, or else /// changes will not be written to the database - pub fn store(&self, tree: &sled::Tree, username: impl AsRef<[u8]>) -> Result<()> + fn store(&self, tree: &sled::Tree, username: impl AsRef<[u8]>) -> Result<()> where UserData: Serialize { @@ -78,23 +66,6 @@ impl PartialUser { } } -impl AsRef for PartialUser { - /// Access any data the application has stored for the user. - fn as_ref(&self) -> &UserData { - &self.data - } -} - -impl AsMut for PartialUser { - /// Modify the data stored for a user - /// - /// IMPORTANT: Changes will not be written to the database until - /// [`PartialUser::store()`] is called - fn as_mut(&mut self) -> &mut UserData { - &mut self.data - } -} - /// Any information about the connecting user #[derive(Clone, Debug)] pub enum User { @@ -146,7 +117,7 @@ impl NotSignedInUser { let newser = SignedInUser::new( username.clone(), - self.certificate.clone(), + Some(self.certificate.clone()), self.manager, PartialUser { data: UserData::default(), @@ -210,7 +181,7 @@ impl NotSignedInUser { /// For more information about the user lifecycle and sign-in stages, see [`User`] pub struct SignedInUser { username: String, - active_certificate: Certificate, + active_certificate: Option, manager: UserManager, inner: PartialUser, /// Indicates that [`SignedInUser::as_mut()`] has been called, but [`SignedInUser::save()`] has not @@ -222,7 +193,7 @@ impl SignedInUser { /// Create a new user from parts pub (crate) fn new( username: String, - active_certificate: Certificate, + active_certificate: Option, manager: UserManager, inner: PartialUser ) -> Self { @@ -235,9 +206,18 @@ impl SignedInUser { } } + /// Update the active certificate + pub (crate) fn with_cert(mut self, cert: Certificate) -> Self { + self.active_certificate = Some(cert); + self + } + /// Get the [`Certificate`] that the user is currently using to connect. - pub fn active_certificate(&self) -> &Certificate { - &self.active_certificate + /// + /// If this user was retrieved by a [`UserManager::lookup_user()`], this will be + /// [`None`]. In all other cases, this will be [`Some`]. + pub fn active_certificate(&self) -> Option<&Certificate> { + self.active_certificate.as_ref() } /// Produce a list of all [`Certificate`]s registered to this account