Assure Sensitive fields can't be serialized

This does not only mean that they cannot be written to disk, but also
that extra work has to be done to serialize them over the wire.

This is very much by design, as they can be in structs that are
seemingly serializable and contain sensitive data, but they must
never actually be serialized.

For use in the UI, an extra type must be used that marks the secret
differently, for instance by field name.

crates/gitbutler-core/src/types/sensitive.rs

@@ -6,11 +6,11 @@ impl<T> Serialize for Sensitive<T>
 where
     T: Serialize,
 {
-    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+    fn serialize<S>(&self, _serializer: S) -> Result<S::Ok, S::Error>
     where
         S: Serializer,
     {
-        self.0.serialize(serializer)
+        unreachable!("BUG: Sensitive data cannot be serialized - it needs to be extracted and put into a struct for serialization explicitly")
     }
 }
 impl<'de, T> Deserialize<'de> for Sensitive<T>

crates/gitbutler-core/src/users/user.rs

@@ -35,29 +35,24 @@ impl User {
     ///
     /// It's cached after the first retrieval.
     pub fn access_token(&self) -> Result<Sensitive<String>> {
-        match self.access_token.borrow().as_ref() {
-            Some(token) => Ok(token.clone()),
-            None => {
-                let err_msg = "BUG: access token for user must have been stored - delete user.json and login again to fix";
-                let secret =
-                    crate::secret::retrieve(Self::ACCESS_TOKEN_HANDLE)?.context(err_msg)?;
-                *self.access_token.borrow_mut() = Some(secret.clone());
-                Ok(secret)
-            }
+        if let Some(token) = self.access_token.borrow().as_ref() {
+            return Ok(token.clone());
         }
+        let err_msg = "BUG: access token for user must have been stored - delete user.json and login again to fix";
+        let secret = crate::secret::retrieve(Self::ACCESS_TOKEN_HANDLE)?.context(err_msg)?;
+        *self.access_token.borrow_mut() = Some(secret.clone());
+        Ok(secret)
     }
 
     /// Obtain the GitHub access token, if it is stored either on this instance or in the secrets store.
     ///
     /// Note that if retrieved from the secrets store, it will be cached on instance.
     pub fn github_access_token(&self) -> Result<Option<Sensitive<String>>> {
-        match self.github_access_token.borrow().as_ref() {
-            Some(token) => Ok(Some(token.clone())),
-            None => {
-                let secret = crate::secret::retrieve(Self::GITHUB_ACCESS_TOKEN_HANDLE)?;
-                self.github_access_token.borrow_mut().clone_from(&secret);
-                Ok(secret)
-            }
+        if let Some(token) = self.github_access_token.borrow().as_ref() {
+            return Ok(Some(token.clone()));
         }
+        let secret = crate::secret::retrieve(Self::GITHUB_ACCESS_TOKEN_HANDLE)?;
+        self.github_access_token.borrow_mut().clone_from(&secret);
+        Ok(secret)
     }
 }

crates/gitbutler-core/tests/secret/users.rs

@@ -1,4 +1,5 @@
 use crate::{credentials, credentials::count as count_secrets};
+use gitbutler_core::users::User;
 use serial_test::serial;
 use std::path::{Path, PathBuf};
 use tempfile::tempdir;
@@ -31,18 +32,23 @@ fn auto_migration_of_secrets_on_when_getting_and_setting_user() -> anyhow::Resul
             expected_secrets,
             "it automatically entered the secrets to the secrets store after getting the existing user"
         );
-        assert_eq!(
-            user.access_token()?.0,
-            "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee",
-            "it can make the access token available"
-        );
-        if has_github_token {
+
+        let assert_access_token_values = |user: &User| -> anyhow::Result<()> {
             assert_eq!(
-                user.github_access_token()?.map(|s| s.0),
-                Some("gho_AAAAAAAAAAAAABBBBBBBBBBBBBBBCCCCCCCC".into()),
+                user.access_token()?.0,
+                "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee",
                 "it can make the access token available"
             );
-        }
+            if has_github_token {
+                assert_eq!(
+                    user.github_access_token()?.map(|s| s.0),
+                    Some("gho_AAAAAAAAAAAAABBBBBBBBBBBBBBBCCCCCCCC".into()),
+                    "it can make the access token available"
+                );
+            }
+            Ok(())
+        };
+        assert_access_token_values(&user)?;
 
         let assert_no_secret_in_plain_text = || -> anyhow::Result<()> {
             let buf = std::fs::read(&user_json_path)?;
@@ -61,6 +67,9 @@ fn auto_migration_of_secrets_on_when_getting_and_setting_user() -> anyhow::Resul
         };
         assert_no_secret_in_plain_text()?;
 
+        let user = users.get_user()?.expect("stored user can be read");
+        assert_access_token_values(&user)?;
+
         users.delete_user()?;
         assert_eq!(
             count_secrets(),

crates/gitbutler-tauri/src/users.rs

@@ -3,6 +3,7 @@ pub mod commands {
         assets,
         users::{controller::Controller, User},
     };
+    use serde::{Deserialize, Serialize};
     use tauri::{AppHandle, Manager};
     use tracing::instrument;
 
@@ -10,12 +11,12 @@ pub mod commands {
 
     #[tauri::command(async)]
     #[instrument(skip(handle), err(Debug))]
-    pub async fn get_user(handle: AppHandle) -> Result<Option<User>, Error> {
+    pub async fn get_user(handle: AppHandle) -> Result<Option<UserWithSecrets>, Error> {
         let app = handle.state::<Controller>();
         let proxy = handle.state::<assets::Proxy>();
 
         match app.get_user()? {
-            Some(user) => Ok(Some(proxy.proxy_user(user).await)),
+            Some(user) => Ok(Some(proxy.proxy_user(user).await.try_into()?)),
             None => Ok(None),
         }
     }
@@ -40,4 +41,59 @@ pub mod commands {
 
         Ok(())
     }
+
+    #[derive(Debug, Deserialize, Serialize)]
+    pub struct UserWithSecrets {
+        id: u64,
+        name: Option<String>,
+        given_name: Option<String>,
+        family_name: Option<String>,
+        email: String,
+        picture: String,
+        locale: Option<String>,
+        created_at: String,
+        updated_at: String,
+        access_token: String,
+        role: Option<String>,
+        github_access_token: Option<String>,
+        github_username: Option<String>,
+    }
+
+    impl TryFrom<User> for UserWithSecrets {
+        type Error = anyhow::Error;
+
+        fn try_from(value: User) -> Result<Self, Self::Error> {
+            let access_token = value.access_token()?;
+            let github_access_token = value.github_access_token()?;
+            let User {
+                id,
+                name,
+                given_name,
+                family_name,
+                email,
+                picture,
+                locale,
+                created_at,
+                updated_at,
+                role,
+                github_username,
+                ..
+            } = value;
+            Ok(UserWithSecrets {
+                id,
+                name,
+                given_name,
+                family_name,
+                email,
+                picture,
+                locale,
+                created_at,
+                updated_at,
+                access_token: access_token.0,
+                role,
+                github_access_token: github_access_token.map(|s| s.0),
+                github_username,
+            })
+        }
+    }
 }