extract user as its own crate out of core

Cargo.lock

@@ -2120,6 +2120,7 @@ dependencies = [
  "gitbutler-project",
  "gitbutler-repo",
  "gitbutler-testsupport",
+ "gitbutler-user",
  "glob",
  "hex",
  "itertools 0.13.0",
@@ -2334,6 +2335,7 @@ dependencies = [
  "gitbutler-git",
  "gitbutler-project",
  "gitbutler-testsupport",
+ "gitbutler-user",
  "resolve-path",
  "serde_json",
  "tempfile",
@@ -2353,6 +2355,7 @@ dependencies = [
  "gitbutler-core",
  "gitbutler-oplog",
  "gitbutler-project",
+ "gitbutler-user",
  "itertools 0.13.0",
  "tracing",
 ]
@@ -2377,6 +2380,7 @@ dependencies = [
  "gitbutler-project",
  "gitbutler-repo",
  "gitbutler-testsupport",
+ "gitbutler-user",
  "gitbutler-watcher",
  "log",
  "nonzero_ext",
@@ -2413,6 +2417,7 @@ dependencies = [
  "gitbutler-core",
  "gitbutler-project",
  "gitbutler-repo",
+ "gitbutler-user",
  "keyring",
  "once_cell",
  "pretty_assertions",
@@ -2420,6 +2425,19 @@ dependencies = [
  "tempfile",
 ]
 
+[[package]]
+name = "gitbutler-user"
+version = "0.1.0"
+dependencies = [
+ "anyhow",
+ "gitbutler-core",
+ "keyring",
+ "serde",
+ "serde_json",
+ "serial_test",
+ "tempfile",
+]
+
 [[package]]
 name = "gitbutler-watcher"
 version = "0.0.0"
@@ -2434,6 +2452,7 @@ dependencies = [
  "gitbutler-oplog",
  "gitbutler-project",
  "gitbutler-sync",
+ "gitbutler-user",
  "gix",
  "notify",
  "thiserror",

Cargo.toml

@@ -16,6 +16,7 @@ members = [
     "crates/gitbutler-feedback", 
     "crates/gitbutler-config",
     "crates/gitbutler-project",
+    "crates/gitbutler-user",
 ]
 resolver = "2"
 
@@ -43,6 +44,7 @@ gitbutler-command-context = { path = "crates/gitbutler-command-context" }
 gitbutler-feedback = { path = "crates/gitbutler-feedback" }
 gitbutler-config = { path = "crates/gitbutler-config" }
 gitbutler-project = { path = "crates/gitbutler-project" }
+gitbutler-user = { path = "crates/gitbutler-user" }
 
 [profile.release]
 codegen-units = 1 # Compile crates one after another so the compiler can optimize better

crates/gitbutler-branch/Cargo.toml

@@ -14,6 +14,7 @@ gitbutler-core.workspace = true
 gitbutler-oplog.workspace = true
 gitbutler-branchstate.workspace = true
 gitbutler-repo.workspace = true
+gitbutler-user.workspace = true
 serde = { workspace = true, features = ["std"]}
 bstr = "1.9.1"
 diffy = "0.3.0"

crates/gitbutler-branch/src/assets.rs

@@ -1,6 +1,7 @@
 use anyhow::Result;
 use futures::future::join_all;
-use gitbutler_core::{users, virtual_branches::Author};
+use gitbutler_core::virtual_branches::Author;
+use gitbutler_user as users;
 use std::{collections::HashMap, path, sync, time::Duration};
 use url::Url;
 

crates/gitbutler-core/Cargo.toml

@@ -6,8 +6,8 @@ authors = ["GitButler <gitbutler@gitbutler.com>"]
 publish = false
 
 [[test]]
-name = "secret"
-path = "tests/secret/mod.rs"
+name = "core"
+path = "tests/core.rs"
 
 [dev-dependencies]
 once_cell = "1.19"

crates/gitbutler-core/src/lib.rs

@@ -29,7 +29,7 @@ pub mod ssh;
 pub mod storage;
 pub mod time;
 pub mod types;
-pub mod users;
+// pub mod users;
 pub mod virtual_branches;
 #[cfg(target_os = "windows")]
 pub mod windows;

crates/gitbutler-repo/Cargo.toml

@@ -26,4 +26,5 @@ path = "tests/mod.rs"
 
 [dev-dependencies]
 gitbutler-testsupport.workspace = true
+gitbutler-user.workspace = true
 serde_json = { version = "1.0", features = [ "std", "arbitrary_precision" ] }

crates/gitbutler-repo/tests/credentials.rs

@@ -2,9 +2,9 @@ use std::path::PathBuf;
 use std::str;
 
 use gitbutler_command_context::ProjectRepo;
-use gitbutler_core::users;
 use gitbutler_project as projects;
 use gitbutler_repo::credentials::{Credential, Helper, SshCredential};
+use gitbutler_user as users;
 
 use gitbutler_testsupport::{temp_dir, test_repository};
 

crates/gitbutler-sync/Cargo.toml

@@ -15,3 +15,4 @@ gitbutler-oplog.workspace = true
 gitbutler-branchstate.workspace = true
 gitbutler-command-context.workspace = true
 gitbutler-project.workspace = true
+gitbutler-user.workspace = true

crates/gitbutler-sync/src/cloud.rs

@@ -6,12 +6,13 @@ use anyhow::{anyhow, Context, Result};
 use gitbutler_branchstate::VirtualBranchesAccess;
 use gitbutler_command_context::ProjectRepo;
 use gitbutler_core::error::Code;
+use gitbutler_core::git;
 use gitbutler_core::git::Url;
 use gitbutler_core::id::Id;
-use gitbutler_core::{git, users};
 use gitbutler_oplog::oplog::Oplog;
 use gitbutler_project as projects;
 use gitbutler_project::{CodePushState, Project};
+use gitbutler_user as users;
 use itertools::Itertools;
 
 pub async fn sync_with_gitbutler(

crates/gitbutler-tauri/Cargo.toml

@@ -55,6 +55,7 @@ gitbutler-command-context.workspace = true
 gitbutler-feedback.workspace = true
 gitbutler-config.workspace = true
 gitbutler-project.workspace = true
+gitbutler-user.workspace = true
 open = "5"
 
 [dependencies.tauri]

crates/gitbutler-tauri/src/main.rs

@@ -108,10 +108,10 @@ fn main() {
                     let projects_storage_controller = gitbutler_project::storage::Storage::new(storage_controller.clone());
                     app_handle.manage(projects_storage_controller.clone());
 
-                    let users_storage_controller = gitbutler_core::users::storage::Storage::new(storage_controller.clone());
+                    let users_storage_controller = gitbutler_user::storage::Storage::new(storage_controller.clone());
                     app_handle.manage(users_storage_controller.clone());
 
-                    let users_controller = gitbutler_core::users::Controller::new(users_storage_controller.clone());
+                    let users_controller = gitbutler_user::Controller::new(users_storage_controller.clone());
                     app_handle.manage(users_controller.clone());
 
                     let projects_controller = gitbutler_project::Controller::new(

crates/gitbutler-tauri/src/users.rs

@@ -1,6 +1,6 @@
 pub mod commands {
     use gitbutler_branch::assets;
-    use gitbutler_core::users::{controller::Controller, User};
+    use gitbutler_user::{controller::Controller, User};
     use serde::{Deserialize, Serialize};
     use tauri::{AppHandle, Manager};
     use tracing::instrument;

crates/gitbutler-tauri/src/watcher.rs

@@ -3,9 +3,9 @@ use std::sync::Arc;
 use anyhow::{Context, Result};
 use futures::executor::block_on;
 use gitbutler_branch::assets;
-use gitbutler_core::users;
 use gitbutler_project as projects;
 use gitbutler_project::{Project, ProjectId};
+use gitbutler_user as users;
 use tauri::{AppHandle, Manager};
 use tracing::instrument;
 

crates/gitbutler-testsupport/Cargo.toml

@@ -23,3 +23,4 @@ gitbutler-repo = { path = "../gitbutler-repo" }
 gitbutler-branchstate = { path = "../gitbutler-branchstate" }
 gitbutler-command-context.workspace = true
 gitbutler-project.workspace = true
+gitbutler-user.workspace = true

crates/gitbutler-testsupport/src/suite.rs

@@ -13,7 +13,7 @@ use crate::{init_opts, init_opts_bare, VAR_NO_CLEANUP};
 pub struct Suite {
     pub local_app_data: Option<TempDir>,
     pub storage: gitbutler_core::storage::Storage,
-    pub users: gitbutler_core::users::Controller,
+    pub users: gitbutler_user::Controller,
     pub projects: gitbutler_project::Controller,
     pub keys: gitbutler_core::keys::Controller,
 }
@@ -30,7 +30,7 @@ impl Default for Suite {
     fn default() -> Self {
         let local_app_data = temp_dir();
         let storage = gitbutler_core::storage::Storage::new(local_app_data.path());
-        let users = gitbutler_core::users::Controller::from_path(local_app_data.path());
+        let users = gitbutler_user::Controller::from_path(local_app_data.path());
         let projects = gitbutler_project::Controller::from_path(local_app_data.path());
         let keys = gitbutler_core::keys::Controller::from_path(local_app_data.path());
         Self {
@@ -47,9 +47,9 @@ impl Suite {
     pub fn local_app_data(&self) -> &Path {
         self.local_app_data.as_ref().unwrap().path()
     }
-    pub fn sign_in(&self) -> gitbutler_core::users::User {
+    pub fn sign_in(&self) -> gitbutler_user::User {
         crate::secrets::setup_blackhole_store();
-        let user: gitbutler_core::users::User =
+        let user: gitbutler_user::User =
             serde_json::from_str(include_str!("fixtures/user/minimal.v1"))
                 .expect("valid v1 user file");
         self.users.set_user(&user).expect("failed to add user");

crates/gitbutler-user/Cargo.toml

@@ -0,0 +1,19 @@
+[package]
+name = "gitbutler-user"
+version = "0.1.0"
+edition = "2021"
+
+[dependencies]
+gitbutler-core.workspace = true
+anyhow = "1.0.86"
+serde = { workspace = true, features = ["std"]}
+serde_json = { version = "1.0", features = [ "std", "arbitrary_precision" ] }
+
+[[test]]
+name="user"
+path = "tests/mod.rs"
+
+[dev-dependencies]
+serial_test = "3.1.1"
+tempfile = "3.10"
+keyring.workspace = true

crates/gitbutler-user/src/controller.rs

@@ -0,0 +1,70 @@
+use super::{storage::Storage, User};
+use anyhow::Context;
+use anyhow::Result;
+use gitbutler_core::secret;
+use std::path::PathBuf;
+
+/// TODO(ST): rename to `Login` - seems more akin to what it does
+/// This type deals with user-related data which is only known if the user is logged in to GitButler.
+///
+/// ### Migrations: V1 -> V2
+///
+/// V2 is implied by not storing the `access_token` in plain text anymore, nor the GitHub token even if present.
+/// It happens automatically on [Self::get_user()] and [Self::set_user()]
+#[derive(Clone)]
+pub struct Controller {
+    storage: Storage,
+}
+
+impl Controller {
+    pub fn new(storage: Storage) -> Controller {
+        Controller { storage }
+    }
+
+    pub fn from_path(path: impl Into<PathBuf>) -> Controller {
+        Controller::new(Storage::from_path(path))
+    }
+
+    /// Return the current login, or `None` if there is none yet.
+    pub fn get_user(&self) -> Result<Option<User>> {
+        let user = self.storage.get().context("failed to get user")?;
+        if let Some(user) = &user {
+            write_without_secrets_if_secrets_present(&self.storage, user.clone())?;
+        }
+        Ok(user)
+    }
+
+    /// Note that secrets are never written in plain text, but we assure they are stored.
+    pub fn set_user(&self, user: &User) -> Result<()> {
+        if !write_without_secrets_if_secrets_present(&self.storage, user.clone())? {
+            self.storage.set(user).context("failed to set user")
+        } else {
+            Ok(())
+        }
+    }
+
+    pub fn delete_user(&self) -> Result<()> {
+        self.storage.delete().context("failed to delete user")?;
+        let namespace = secret::Namespace::BuildKind;
+        secret::delete(User::ACCESS_TOKEN_HANDLE, namespace).ok();
+        secret::delete(User::GITHUB_ACCESS_TOKEN_HANDLE, namespace).ok();
+        Ok(())
+    }
+}
+
+/// As `user` sports interior mutability right now, let's play it safe and work with fully owned items only.
+fn write_without_secrets_if_secrets_present(storage: &Storage, user: User) -> Result<bool> {
+    let mut needs_write = false;
+    let namespace = secret::Namespace::BuildKind;
+    if let Some(gb_token) = user.access_token.borrow_mut().take() {
+        needs_write |= secret::persist(User::ACCESS_TOKEN_HANDLE, &gb_token, namespace).is_ok();
+    }
+    if let Some(gh_token) = user.github_access_token.borrow_mut().take() {
+        needs_write |=
+            secret::persist(User::GITHUB_ACCESS_TOKEN_HANDLE, &gh_token, namespace).is_ok();
+    }
+    if needs_write {
+        storage.set(&user)?;
+    }
+    Ok(needs_write)
+}

crates/gitbutler-user/src/lib.rs

@@ -0,0 +1,6 @@
+pub mod controller;
+pub mod storage;
+mod user;
+
+pub use controller::*;
+pub use user::User;

crates/gitbutler-user/src/storage.rs

@@ -0,0 +1,39 @@
+use anyhow::Result;
+use std::path::PathBuf;
+
+use gitbutler_core::storage as core_storage;
+
+use crate::User;
+
+const USER_FILE: &str = "user.json";
+
+#[derive(Debug, Clone)]
+pub struct Storage {
+    inner: core_storage::Storage,
+}
+
+impl Storage {
+    pub fn new(storage: core_storage::Storage) -> Storage {
+        Storage { inner: storage }
+    }
+
+    pub fn from_path(path: impl Into<PathBuf>) -> Storage {
+        Storage::new(core_storage::Storage::new(path))
+    }
+
+    pub fn get(&self) -> Result<Option<User>> {
+        match self.inner.read(USER_FILE)? {
+            Some(data) => Ok(Some(serde_json::from_str(&data)?)),
+            None => Ok(None),
+        }
+    }
+
+    pub fn set(&self, user: &User) -> Result<()> {
+        let data = serde_json::to_string(user)?;
+        Ok(self.inner.write(USER_FILE, &data)?)
+    }
+
+    pub fn delete(&self) -> Result<()> {
+        Ok(self.inner.delete(USER_FILE)?)
+    }
+}

crates/gitbutler-user/src/user.rs

@@ -0,0 +1,63 @@
+use anyhow::{Context, Result};
+use gitbutler_core::secret;
+use gitbutler_core::types::Sensitive;
+use serde::{Deserialize, Serialize};
+use std::cell::RefCell;
+
+#[derive(Debug, Deserialize, Serialize, Clone, Default)]
+pub struct User {
+    pub id: u64,
+    pub name: Option<String>,
+    pub given_name: Option<String>,
+    pub family_name: Option<String>,
+    pub email: String,
+    pub picture: String,
+    pub locale: Option<String>,
+    pub created_at: String,
+    pub updated_at: String,
+    /// The presence of a GitButler access token is required for a valid user, but it's optional
+    /// as it's not actually stored anymore, but fetch on demand in a separate step as its
+    /// storage location is the [secrets store](crate::secret).
+    #[serde(skip_serializing)]
+    pub(super) access_token: RefCell<Option<Sensitive<String>>>,
+    pub role: Option<String>,
+    /// The semantics here are the same as for `access_token`, but this token is truly optional.
+    #[serde(skip_serializing)]
+    pub(super) github_access_token: RefCell<Option<Sensitive<String>>>,
+    #[serde(default)]
+    pub github_username: Option<String>,
+}
+
+impl User {
+    pub(super) const ACCESS_TOKEN_HANDLE: &'static str = "gitbutler_access_token";
+    pub(super) const GITHUB_ACCESS_TOKEN_HANDLE: &'static str = "github_access_token";
+
+    /// Return the access token of the user after fetching it from the secrets store.
+    ///
+    /// It's cached after the first retrieval.
+    pub fn access_token(&self) -> Result<Sensitive<String>> {
+        if let Some(token) = self.access_token.borrow().as_ref() {
+            return Ok(token.clone());
+        }
+        let err_msg = "access token for user was deleted from keychain - login is now invalid";
+        let secret = secret::retrieve(Self::ACCESS_TOKEN_HANDLE, secret::Namespace::BuildKind)?
+            .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>>> {
+        if let Some(token) = self.github_access_token.borrow().as_ref() {
+            return Ok(Some(token.clone()));
+        }
+        let secret = secret::retrieve(
+            Self::GITHUB_ACCESS_TOKEN_HANDLE,
+            secret::Namespace::BuildKind,
+        )?;
+        self.github_access_token.borrow_mut().clone_from(&secret);
+        Ok(secret)
+    }
+}

crates/gitbutler-user/tests/mod.rs

@@ -0,0 +1,3 @@
+// TODO(kv): These tests should live in the crate where the secret handling is implemented.
+// For purposes of separating thing out of gitbutler-core, moving them here termporarely
+pub mod secret;

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

@@ -1,9 +1,12 @@
-use crate::{credentials, credentials::count as count_secrets};
-use gitbutler_core::users::User;
+// use gitbutler_user::{credentials, credentials::count as count_secrets};
+use gitbutler_user::User;
 use serial_test::serial;
 use std::path::{Path, PathBuf};
 use tempfile::tempdir;
 
+use crate::secret::credentials;
+use crate::secret::credentials::count as count_secrets;
+
 /// Validate that secrets previously stored in plain-text are auto-migrated into the secrets store.
 /// From there, data-structures for use by the frontend need to be 'enriched' with secrets before sending them,
 /// or before using them.
@@ -14,7 +17,7 @@ fn auto_migration_of_secrets_on_when_getting_and_setting_user() -> anyhow::Resul
         credentials::setup();
         let app_data = tempdir()?;
 
-        let users = gitbutler_core::users::Controller::from_path(app_data.path());
+        let users = gitbutler_user::Controller::from_path(app_data.path());
         assert!(
             users.get_user()?.is_none(),
             "Users are bound to logins, so there is none by default"

crates/gitbutler-watcher/Cargo.toml

@@ -16,12 +16,13 @@ gitbutler-oplog.workspace = true
 thiserror.workspace = true
 anyhow = "1.0.86"
 futures = "0.3.30"
-tokio = { workspace = true, features = [ "macros" ] }
+tokio = { workspace = true, features = ["macros"] }
 tokio-util = "0.7.11"
 tracing = "0.1.40"
 gix = { workspace = true, features = ["excludes"] }
 gitbutler-command-context.workspace = true
 gitbutler-project.workspace = true
+gitbutler-user.workspace = true
 
 
 backoff = "0.4.0"

crates/gitbutler-watcher/src/handler.rs

@@ -6,7 +6,7 @@ use gitbutler_branch::assets;
 use gitbutler_branch::VirtualBranches;
 use gitbutler_command_context::ProjectRepo;
 use gitbutler_core::error::Marker;
-use gitbutler_core::{git, users};
+use gitbutler_core::git;
 use gitbutler_oplog::{
     entry::{OperationKind, SnapshotDetails},
     oplog::Oplog,
@@ -14,6 +14,7 @@ use gitbutler_oplog::{
 use gitbutler_project as projects;
 use gitbutler_project::ProjectId;
 use gitbutler_sync::cloud::sync_with_gitbutler;
+use gitbutler_user as users;
 use tracing::instrument;
 
 use super::{events, Change};