diff --git a/Cargo.lock b/Cargo.lock index 3ab9287ef..091df9ce3 100644 --- a/Cargo.lock +++ b/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", diff --git a/Cargo.toml b/Cargo.toml index 77907e31e..83528d201 100644 --- a/Cargo.toml +++ b/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 diff --git a/crates/gitbutler-branch/Cargo.toml b/crates/gitbutler-branch/Cargo.toml index 566ee1089..8d28087a2 100644 --- a/crates/gitbutler-branch/Cargo.toml +++ b/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" diff --git a/crates/gitbutler-branch/src/assets.rs b/crates/gitbutler-branch/src/assets.rs index 3f4b357d9..712a5cd9d 100644 --- a/crates/gitbutler-branch/src/assets.rs +++ b/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; diff --git a/crates/gitbutler-core/Cargo.toml b/crates/gitbutler-core/Cargo.toml index 8e2d25527..6b8728889 100644 --- a/crates/gitbutler-core/Cargo.toml +++ b/crates/gitbutler-core/Cargo.toml @@ -6,8 +6,8 @@ authors = ["GitButler "] publish = false [[test]] -name = "secret" -path = "tests/secret/mod.rs" +name = "core" +path = "tests/core.rs" [dev-dependencies] once_cell = "1.19" diff --git a/crates/gitbutler-core/src/lib.rs b/crates/gitbutler-core/src/lib.rs index f62b323a6..eacfae098 100644 --- a/crates/gitbutler-core/src/lib.rs +++ b/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; diff --git a/crates/gitbutler-repo/Cargo.toml b/crates/gitbutler-repo/Cargo.toml index cf60b1cb6..a35f23f90 100644 --- a/crates/gitbutler-repo/Cargo.toml +++ b/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" ] } diff --git a/crates/gitbutler-repo/tests/credentials.rs b/crates/gitbutler-repo/tests/credentials.rs index 3b3bca8d2..d05450ad9 100644 --- a/crates/gitbutler-repo/tests/credentials.rs +++ b/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}; diff --git a/crates/gitbutler-sync/Cargo.toml b/crates/gitbutler-sync/Cargo.toml index 969cf65b2..5fdffcee2 100644 --- a/crates/gitbutler-sync/Cargo.toml +++ b/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 diff --git a/crates/gitbutler-sync/src/cloud.rs b/crates/gitbutler-sync/src/cloud.rs index eb62dafbd..d92af2a9f 100644 --- a/crates/gitbutler-sync/src/cloud.rs +++ b/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( diff --git a/crates/gitbutler-tauri/Cargo.toml b/crates/gitbutler-tauri/Cargo.toml index c90191c75..83010c72b 100644 --- a/crates/gitbutler-tauri/Cargo.toml +++ b/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] diff --git a/crates/gitbutler-tauri/src/main.rs b/crates/gitbutler-tauri/src/main.rs index 82d68e772..297d837f2 100644 --- a/crates/gitbutler-tauri/src/main.rs +++ b/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( diff --git a/crates/gitbutler-tauri/src/users.rs b/crates/gitbutler-tauri/src/users.rs index 2a2b271d8..5fe974eeb 100644 --- a/crates/gitbutler-tauri/src/users.rs +++ b/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; diff --git a/crates/gitbutler-tauri/src/watcher.rs b/crates/gitbutler-tauri/src/watcher.rs index fcd32b089..00038a139 100644 --- a/crates/gitbutler-tauri/src/watcher.rs +++ b/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; diff --git a/crates/gitbutler-testsupport/Cargo.toml b/crates/gitbutler-testsupport/Cargo.toml index e1fe66f69..fd3d1e207 100644 --- a/crates/gitbutler-testsupport/Cargo.toml +++ b/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 diff --git a/crates/gitbutler-testsupport/src/suite.rs b/crates/gitbutler-testsupport/src/suite.rs index 102dfc02c..5644bdddb 100644 --- a/crates/gitbutler-testsupport/src/suite.rs +++ b/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, 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"); diff --git a/crates/gitbutler-user/Cargo.toml b/crates/gitbutler-user/Cargo.toml new file mode 100644 index 000000000..cea3c0860 --- /dev/null +++ b/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 diff --git a/crates/gitbutler-user/src/controller.rs b/crates/gitbutler-user/src/controller.rs new file mode 100644 index 000000000..2b7a4840b --- /dev/null +++ b/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) -> Controller { + Controller::new(Storage::from_path(path)) + } + + /// Return the current login, or `None` if there is none yet. + pub fn get_user(&self) -> Result> { + 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 { + 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) +} diff --git a/crates/gitbutler-user/src/lib.rs b/crates/gitbutler-user/src/lib.rs new file mode 100644 index 000000000..7c26c7204 --- /dev/null +++ b/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; diff --git a/crates/gitbutler-user/src/storage.rs b/crates/gitbutler-user/src/storage.rs new file mode 100644 index 000000000..f9fd8a7c1 --- /dev/null +++ b/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) -> Storage { + Storage::new(core_storage::Storage::new(path)) + } + + pub fn get(&self) -> Result> { + 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)?) + } +} diff --git a/crates/gitbutler-user/src/user.rs b/crates/gitbutler-user/src/user.rs new file mode 100644 index 000000000..1c24379b7 --- /dev/null +++ b/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, + pub given_name: Option, + pub family_name: Option, + pub email: String, + pub picture: String, + pub locale: Option, + 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>>, + pub role: Option, + /// 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>>, + #[serde(default)] + pub github_username: Option, +} + +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> { + 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>> { + 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) + } +} diff --git a/crates/gitbutler-core/tests/fixtures/users/login-only.v1 b/crates/gitbutler-user/tests/fixtures/users/login-only.v1 similarity index 100% rename from crates/gitbutler-core/tests/fixtures/users/login-only.v1 rename to crates/gitbutler-user/tests/fixtures/users/login-only.v1 diff --git a/crates/gitbutler-core/tests/fixtures/users/with-github.v1 b/crates/gitbutler-user/tests/fixtures/users/with-github.v1 similarity index 100% rename from crates/gitbutler-core/tests/fixtures/users/with-github.v1 rename to crates/gitbutler-user/tests/fixtures/users/with-github.v1 diff --git a/crates/gitbutler-user/tests/mod.rs b/crates/gitbutler-user/tests/mod.rs new file mode 100644 index 000000000..503cf97d2 --- /dev/null +++ b/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; diff --git a/crates/gitbutler-core/tests/secret/credentials.rs b/crates/gitbutler-user/tests/secret/credentials.rs similarity index 100% rename from crates/gitbutler-core/tests/secret/credentials.rs rename to crates/gitbutler-user/tests/secret/credentials.rs diff --git a/crates/gitbutler-core/tests/secret/mod.rs b/crates/gitbutler-user/tests/secret/mod.rs similarity index 100% rename from crates/gitbutler-core/tests/secret/mod.rs rename to crates/gitbutler-user/tests/secret/mod.rs diff --git a/crates/gitbutler-core/tests/secret/users.rs b/crates/gitbutler-user/tests/secret/users.rs similarity index 93% rename from crates/gitbutler-core/tests/secret/users.rs rename to crates/gitbutler-user/tests/secret/users.rs index 68a204bce..ca639ff44 100644 --- a/crates/gitbutler-core/tests/secret/users.rs +++ b/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" diff --git a/crates/gitbutler-watcher/Cargo.toml b/crates/gitbutler-watcher/Cargo.toml index b074b694a..102e73513 100644 --- a/crates/gitbutler-watcher/Cargo.toml +++ b/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" diff --git a/crates/gitbutler-watcher/src/handler.rs b/crates/gitbutler-watcher/src/handler.rs index cc68bbbf0..6ebab6994 100644 --- a/crates/gitbutler-watcher/src/handler.rs +++ b/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};