From cedb893ad6cc60215cb58b7de1e7702cbfc7ba7c Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Tue, 25 Jun 2024 09:58:53 +0200 Subject: [PATCH 01/13] put 'core::serde' into its own module file It started small, but now is large enough to warrant that. --- crates/gitbutler-core/src/lib.rs | 97 +----------------------------- crates/gitbutler-core/src/serde.rs | 94 +++++++++++++++++++++++++++++ 2 files changed, 95 insertions(+), 96 deletions(-) create mode 100644 crates/gitbutler-core/src/serde.rs diff --git a/crates/gitbutler-core/src/lib.rs b/crates/gitbutler-core/src/lib.rs index 65a4a2fce..275da532d 100644 --- a/crates/gitbutler-core/src/lib.rs +++ b/crates/gitbutler-core/src/lib.rs @@ -29,6 +29,7 @@ pub mod project_repository; pub mod projects; pub mod rebase; pub mod remotes; +pub mod serde; pub mod ssh; pub mod storage; pub mod synchronize; @@ -40,99 +41,3 @@ pub mod virtual_branches; pub mod windows; pub mod writer; pub mod zip; -pub mod serde { - use crate::virtual_branches::branch::HunkHash; - use bstr::{BString, ByteSlice}; - use serde::Serialize; - - pub fn as_string_lossy(v: &BString, s: S) -> Result - where - S: serde::Serializer, - { - v.to_str_lossy().serialize(s) - } - - pub fn hash_to_hex(v: &HunkHash, s: S) -> Result - where - S: serde::Serializer, - { - format!("{v:x}").serialize(s) - } - - pub fn as_time_seconds_from_unix_epoch(v: &git2::Time, s: S) -> Result - where - S: serde::Serializer, - { - v.seconds().serialize(s) - } - - pub mod oid_opt { - use serde::{Deserialize, Deserializer, Serialize}; - - pub fn serialize(v: &Option, s: S) -> Result - where - S: serde::Serializer, - { - v.as_ref().map(|v| v.to_string()).serialize(s) - } - - pub fn deserialize<'de, D>(d: D) -> Result, D::Error> - where - D: Deserializer<'de>, - { - let hex = as Deserialize>::deserialize(d)?; - hex.map(|v| { - v.parse() - .map_err(|err: git2::Error| serde::de::Error::custom(err.to_string())) - }) - .transpose() - } - } - - pub mod oid_vec { - use serde::{Deserialize, Deserializer, Serialize}; - - pub fn serialize(v: &[git2::Oid], s: S) -> Result - where - S: serde::Serializer, - { - let vec: Vec = v.iter().map(|v| v.to_string()).collect(); - vec.serialize(s) - } - - pub fn deserialize<'de, D>(d: D) -> Result, D::Error> - where - D: Deserializer<'de>, - { - let hex = as Deserialize>::deserialize(d)?; - let hex: Result, D::Error> = hex - .into_iter() - .map(|v| { - git2::Oid::from_str(v.as_str()) - .map_err(|err: git2::Error| serde::de::Error::custom(err.to_string())) - }) - .collect(); - hex - } - } - - pub mod oid { - use serde::{Deserialize, Deserializer, Serialize}; - - pub fn serialize(v: &git2::Oid, s: S) -> Result - where - S: serde::Serializer, - { - v.to_string().serialize(s) - } - - pub fn deserialize<'de, D>(d: D) -> Result - where - D: Deserializer<'de>, - { - let hex = String::deserialize(d)?; - hex.parse() - .map_err(|err: git2::Error| serde::de::Error::custom(err.to_string())) - } - } -} diff --git a/crates/gitbutler-core/src/serde.rs b/crates/gitbutler-core/src/serde.rs new file mode 100644 index 000000000..1b99317f8 --- /dev/null +++ b/crates/gitbutler-core/src/serde.rs @@ -0,0 +1,94 @@ +use crate::virtual_branches::branch::HunkHash; +use bstr::{BString, ByteSlice}; +use serde::Serialize; + +pub fn as_string_lossy(v: &BString, s: S) -> Result +where + S: serde::Serializer, +{ + v.to_str_lossy().serialize(s) +} + +pub fn hash_to_hex(v: &HunkHash, s: S) -> Result +where + S: serde::Serializer, +{ + format!("{v:x}").serialize(s) +} + +pub fn as_time_seconds_from_unix_epoch(v: &git2::Time, s: S) -> Result +where + S: serde::Serializer, +{ + v.seconds().serialize(s) +} + +pub mod oid_opt { + use serde::{Deserialize, Deserializer, Serialize}; + + pub fn serialize(v: &Option, s: S) -> Result + where + S: serde::Serializer, + { + v.as_ref().map(|v| v.to_string()).serialize(s) + } + + pub fn deserialize<'de, D>(d: D) -> Result, D::Error> + where + D: Deserializer<'de>, + { + let hex = as Deserialize>::deserialize(d)?; + hex.map(|v| { + v.parse() + .map_err(|err: git2::Error| serde::de::Error::custom(err.to_string())) + }) + .transpose() + } +} + +pub mod oid_vec { + use serde::{Deserialize, Deserializer, Serialize}; + + pub fn serialize(v: &[git2::Oid], s: S) -> Result + where + S: serde::Serializer, + { + let vec: Vec = v.iter().map(|v| v.to_string()).collect(); + vec.serialize(s) + } + + pub fn deserialize<'de, D>(d: D) -> Result, D::Error> + where + D: Deserializer<'de>, + { + let hex = as Deserialize>::deserialize(d)?; + let hex: Result, D::Error> = hex + .into_iter() + .map(|v| { + git2::Oid::from_str(v.as_str()) + .map_err(|err: git2::Error| serde::de::Error::custom(err.to_string())) + }) + .collect(); + hex + } +} + +pub mod oid { + use serde::{Deserialize, Deserializer, Serialize}; + + pub fn serialize(v: &git2::Oid, s: S) -> Result + where + S: serde::Serializer, + { + v.to_string().serialize(s) + } + + pub fn deserialize<'de, D>(d: D) -> Result + where + D: Deserializer<'de>, + { + let hex = String::deserialize(d)?; + hex.parse() + .map_err(|err: git2::Error| serde::de::Error::custom(err.to_string())) + } +} From f3e6c7ba9441fbbffd86d15481f277175d1e3b4e Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Tue, 25 Jun 2024 09:40:37 +0200 Subject: [PATCH 02/13] Add the capability to store and retrieve secrets This is a global facility without any state, and provides a simplified interface to the `keyring` crate. --- Cargo.lock | 157 ++++++++++++++++++++++ crates/gitbutler-core/Cargo.toml | 2 + crates/gitbutler-core/src/lib.rs | 1 + crates/gitbutler-core/src/secret.rs | 24 ++++ crates/gitbutler-core/tests/core.rs | 1 + crates/gitbutler-core/tests/secret/mod.rs | 105 +++++++++++++++ 6 files changed, 290 insertions(+) create mode 100644 crates/gitbutler-core/src/secret.rs create mode 100644 crates/gitbutler-core/tests/secret/mod.rs diff --git a/Cargo.lock b/Cargo.lock index 15b4b3d62..dbf03bf15 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -546,6 +546,15 @@ dependencies = [ "generic-array", ] +[[package]] +name = "block-padding" +version = "0.3.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a8894febbff9f758034a5b8e12d87918f56dfc64a8e1fe757d65e29041538d93" +dependencies = [ + "generic-array", +] + [[package]] name = "blocking" version = "1.5.1" @@ -733,6 +742,15 @@ dependencies = [ "toml 0.7.8", ] +[[package]] +name = "cbc" +version = "0.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "26b52a9543ae338f279b96b0b9fed9c8093744685043739079ce85cd58f289a6" +dependencies = [ + "cipher", +] + [[package]] name = "cc" version = "1.0.98" @@ -2120,6 +2138,7 @@ dependencies = [ "glob", "hex", "itertools 0.13.0", + "keyring", "lazy_static", "log", "md5", @@ -2131,6 +2150,7 @@ dependencies = [ "resolve-path", "serde", "serde_json", + "serial_test", "sha2", "ssh-key", "ssh2", @@ -3161,6 +3181,15 @@ version = "0.4.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7f24254aa9a54b5c858eaee2f5bccdb46aaf0e486a595ed5fd8f86ba55232a70" +[[package]] +name = "hkdf" +version = "0.12.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7b5f8eb2ad728638ea2c7d47a21db23b7b58a72ed6a38256b8a1849f15fbbdf7" +dependencies = [ + "hmac", +] + [[package]] name = "hmac" version = "0.12.1" @@ -3518,6 +3547,7 @@ version = "0.1.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a0c10553d664a4d0bcff9f4215d0aac67a639cc68ef660840afe309b807bc9f5" dependencies = [ + "block-padding", "generic-array", ] @@ -3694,6 +3724,20 @@ dependencies = [ "winapi-build", ] +[[package]] +name = "keyring" +version = "2.3.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "363387f0019d714aa60cc30ab4fe501a747f4c08fc58f069dd14be971bd495a0" +dependencies = [ + "byteorder", + "lazy_static", + "linux-keyutils", + "secret-service", + "security-framework", + "windows-sys 0.52.0", +] + [[package]] name = "kqueue" version = "1.0.8" @@ -3820,6 +3864,16 @@ version = "0.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "dd1bc4d24ad230d21fb898d1116b1801d7adfc449d42026475862ab48b11e70e" +[[package]] +name = "linux-keyutils" +version = "0.2.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "761e49ec5fd8a5a463f9b84e877c373d888935b71c6be78f3767fe2ae6bed18e" +dependencies = [ + "bitflags 2.5.0", + "libc", +] + [[package]] name = "linux-raw-sys" version = "0.3.8" @@ -4132,6 +4186,30 @@ dependencies = [ "winapi 0.3.9", ] +[[package]] +name = "num" +version = "0.4.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3135b08af27d103b0a51f2ae0f8632117b7b185ccf931445affa8df530576a41" +dependencies = [ + "num-bigint", + "num-complex", + "num-integer", + "num-iter", + "num-rational", + "num-traits", +] + +[[package]] +name = "num-bigint" +version = "0.4.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c165a9ab64cf766f73521c0dd2cfdff64f488b8f0b3e621face3462d3db536d7" +dependencies = [ + "num-integer", + "num-traits", +] + [[package]] name = "num-bigint-dig" version = "0.8.4" @@ -4149,6 +4227,15 @@ dependencies = [ "zeroize", ] +[[package]] +name = "num-complex" +version = "0.4.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "73f88a1307638156682bada9d7604135552957b7818057dcef22705b4d509495" +dependencies = [ + "num-traits", +] + [[package]] name = "num-conv" version = "0.1.0" @@ -4175,6 +4262,17 @@ dependencies = [ "num-traits", ] +[[package]] +name = "num-rational" +version = "0.4.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f83d14da390562dca69fc84082e73e548e1ad308d24accdedd2720017cb37824" +dependencies = [ + "num-bigint", + "num-integer", + "num-traits", +] + [[package]] name = "num-traits" version = "0.2.18" @@ -5512,6 +5610,15 @@ dependencies = [ "winapi-util", ] +[[package]] +name = "scc" +version = "2.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "76ad2bbb0ae5100a07b7a6f2ed7ab5fd0045551a4c507989b7a620046ea3efdc" +dependencies = [ + "sdd", +] + [[package]] name = "schannel" version = "0.1.23" @@ -5533,6 +5640,12 @@ version = "1.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "94143f37725109f92c262ed2cf5e59bce7498c01bcc1502d7b9afe439a4e9f49" +[[package]] +name = "sdd" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b84345e4c9bd703274a082fb80caaa99b7612be48dfaa1dd9266577ec412309d" + [[package]] name = "seahash" version = "4.1.0" @@ -5553,6 +5666,25 @@ dependencies = [ "zeroize", ] +[[package]] +name = "secret-service" +version = "3.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b5204d39df37f06d1944935232fd2dfe05008def7ca599bf28c0800366c8a8f9" +dependencies = [ + "aes", + "cbc", + "futures-util", + "generic-array", + "hkdf", + "num", + "once_cell", + "rand 0.8.5", + "serde", + "sha2", + "zbus", +] + [[package]] name = "security-framework" version = "2.10.0" @@ -5699,6 +5831,31 @@ dependencies = [ "syn 2.0.58", ] +[[package]] +name = "serial_test" +version = "3.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4b4b487fe2acf240a021cf57c6b2b4903b1e78ca0ecd862a71b71d2a51fed77d" +dependencies = [ + "futures", + "log", + "once_cell", + "parking_lot 0.12.3", + "scc", + "serial_test_derive", +] + +[[package]] +name = "serial_test_derive" +version = "3.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "82fe9db325bcef1fbcde82e078a5cc4efdf787e96b3b9cf45b50b529f2083d67" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.58", +] + [[package]] name = "serialize-to-javascript" version = "0.1.1" diff --git a/crates/gitbutler-core/Cargo.toml b/crates/gitbutler-core/Cargo.toml index b8bf0eeef..ee4a288a0 100644 --- a/crates/gitbutler-core/Cargo.toml +++ b/crates/gitbutler-core/Cargo.toml @@ -12,6 +12,7 @@ pretty_assertions = "1.4" gitbutler-testsupport.workspace = true gitbutler-git = { workspace = true, features = ["test-askpass-path"] } glob = "0.3.1" +serial_test = "3.1.1" [dependencies] toml = "0.8.13" @@ -28,6 +29,7 @@ git2.workspace = true git2-hooks = "0.3" gix = { workspace = true, features = ["dirwalk"] } itertools = "0.13" +keyring = "2.3.3" lazy_static = "1.4.0" md5 = "0.7.0" hex = "0.4.3" diff --git a/crates/gitbutler-core/src/lib.rs b/crates/gitbutler-core/src/lib.rs index 275da532d..dd607eb0b 100644 --- a/crates/gitbutler-core/src/lib.rs +++ b/crates/gitbutler-core/src/lib.rs @@ -29,6 +29,7 @@ pub mod project_repository; pub mod projects; pub mod rebase; pub mod remotes; +pub mod secret; pub mod serde; pub mod ssh; pub mod storage; diff --git a/crates/gitbutler-core/src/secret.rs b/crates/gitbutler-core/src/secret.rs new file mode 100644 index 000000000..b891d5899 --- /dev/null +++ b/crates/gitbutler-core/src/secret.rs @@ -0,0 +1,24 @@ +//! This module contains facilities to handle the persistence of secrets. +//! +//! These are stateless and global, while discouraging storing secrets +//! in memory beyond their use. +use crate::types::Sensitive; +use anyhow::Result; + +/// Persist `secret` so that it can be retrieved by the given `handle`. +pub fn persist(handle: &str, secret: &Sensitive) -> Result<()> { + Ok(entry_for(handle)?.set_password(&secret.0)?) +} + +/// Obtain the previously [stored](persist()) secret known as `handle`. +pub fn retrieve(handle: &str) -> Result>> { + match entry_for(handle)?.get_password() { + Ok(secret) => Ok(Some(Sensitive(secret))), + Err(keyring::Error::NoEntry) => Ok(None), + Err(err) => Err(err.into()), + } +} + +fn entry_for(handle: &str) -> Result { + Ok(keyring::Entry::new("gitbutler", handle)?) +} diff --git a/crates/gitbutler-core/tests/core.rs b/crates/gitbutler-core/tests/core.rs index 22e13a0fb..bda150f67 100644 --- a/crates/gitbutler-core/tests/core.rs +++ b/crates/gitbutler-core/tests/core.rs @@ -7,6 +7,7 @@ mod git; mod keys; mod lock; mod ops; +mod secret; mod types; pub mod virtual_branches; mod zip; diff --git a/crates/gitbutler-core/tests/secret/mod.rs b/crates/gitbutler-core/tests/secret/mod.rs new file mode 100644 index 000000000..c2aedb379 --- /dev/null +++ b/crates/gitbutler-core/tests/secret/mod.rs @@ -0,0 +1,105 @@ +use gitbutler_core::secret; +use gitbutler_core::types::Sensitive; +use serial_test::serial; + +#[test] +#[serial] +fn retrieve_unknown_is_none() { + setup(); + assert!(secret::retrieve("does not exist for sure") + .expect("no error to ask for non-existing") + .is_none()); +} + +#[test] +#[serial] +fn store_and_retrieve() -> anyhow::Result<()> { + setup(); + secret::persist("new", &Sensitive("secret".into()))?; + let secret = secret::retrieve("new")?.expect("it was just stored"); + assert_eq!( + secret.0, "secret", + "note that this works only if the engine supports actual persistence, \ + which should be the default outside of tests" + ); + Ok(()) +} + +fn setup() { + keyring::set_default_credential_builder(Box::::default()); +} + +mod credentials { + use keyring::credential::{CredentialApi, CredentialBuilderApi, CredentialPersistence}; + use keyring::Credential; + use std::any::Any; + use std::collections::BTreeMap; + use std::sync::{Arc, Mutex}; + + #[derive(Default)] + struct Store { + store: BTreeMap, + } + + type SharedStore = Arc>; + + struct Entry { + handle: String, + inner: SharedStore, + } + + impl CredentialApi for Entry { + fn set_password(&self, password: &str) -> keyring::Result<()> { + self.inner + .lock() + .unwrap() + .store + .insert(self.handle.clone(), password.into()); + Ok(()) + } + + fn get_password(&self) -> keyring::Result { + match self.inner.lock().unwrap().store.get(&self.handle) { + Some(secret) => Ok(secret.clone()), + None => Err(keyring::Error::NoEntry), + } + } + + fn delete_password(&self) -> keyring::Result<()> { + todo!() + } + + fn as_any(&self) -> &dyn Any { + self + } + } + + #[derive(Default)] + pub(super) struct Builder { + store: SharedStore, + } + + impl CredentialBuilderApi for Builder { + fn build( + &self, + _target: Option<&str>, + _service: &str, + user: &str, + ) -> keyring::Result> { + let credential = Entry { + handle: user.to_string(), + inner: self.store.clone(), + }; + Ok(Box::new(credential)) + } + + fn as_any(&self) -> &dyn std::any::Any { + self + } + + /// We keep information in memory + fn persistence(&self) -> CredentialPersistence { + CredentialPersistence::ProcessOnly + } + } +} From 929eb52224e573c68043ac2c8d7b60684f27f9d0 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Mon, 24 Jun 2024 17:18:33 +0200 Subject: [PATCH 03/13] auto-migrate secrets from user.json to their location in the keyring This will also remove them from their plain-text location. Further, when the secrets are required they will be obtained specifically, instead of always having them at hand. The frontend doesn't ever show these, but needs them, and it now asks for them when it needs them. --- Cargo.lock | 6 +- Cargo.toml | 1 + crates/gitbutler-core/Cargo.toml | 5 +- crates/gitbutler-core/src/assets.rs | 23 ++--- .../src/project_repository/repository.rs | 6 +- crates/gitbutler-core/src/secret.rs | 5 + crates/gitbutler-core/src/users/controller.rs | 57 ++++++++--- crates/gitbutler-core/src/users/user.rs | 47 ++++++++- crates/gitbutler-core/tests/core.rs | 1 - .../tests/fixtures/users/login-only.v1 | 15 +++ .../tests/fixtures/users/with-github.v1 | 15 +++ .../gitbutler-core/tests/git/credentials.rs | 22 +++-- .../tests/secret/credentials.rs | 96 +++++++++++++++++++ crates/gitbutler-core/tests/secret/mod.rs | 87 ++--------------- crates/gitbutler-core/tests/secret/users.rs | 94 ++++++++++++++++++ crates/gitbutler-testsupport/Cargo.toml | 2 + .../src/fixtures/user/minimal.v1 | 15 +++ crates/gitbutler-testsupport/src/lib.rs | 6 ++ crates/gitbutler-testsupport/src/secrets.rs | 45 +++++++++ crates/gitbutler-testsupport/src/suite.rs | 11 +-- 20 files changed, 426 insertions(+), 133 deletions(-) create mode 100644 crates/gitbutler-core/tests/fixtures/users/login-only.v1 create mode 100644 crates/gitbutler-core/tests/fixtures/users/with-github.v1 create mode 100644 crates/gitbutler-core/tests/secret/credentials.rs create mode 100644 crates/gitbutler-core/tests/secret/users.rs create mode 100644 crates/gitbutler-testsupport/src/fixtures/user/minimal.v1 create mode 100644 crates/gitbutler-testsupport/src/secrets.rs diff --git a/Cargo.lock b/Cargo.lock index dbf03bf15..8cf1647f2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2246,8 +2246,10 @@ dependencies = [ "anyhow", "git2", "gitbutler-core", + "keyring", "once_cell", "pretty_assertions", + "serde_json", "tempfile", ] @@ -5759,9 +5761,9 @@ dependencies = [ [[package]] name = "serde_json" -version = "1.0.117" +version = "1.0.118" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "455182ea6142b14f93f4bc5320a2b31c1f266b66a4a5c858b013302a5d8cbfc3" +checksum = "d947f6b3163d8857ea16c4fa0dd4840d52f3041039a85decd46867eb1abef2e4" dependencies = [ "indexmap 2.2.6", "itoa 1.0.11", diff --git a/Cargo.toml b/Cargo.toml index 0f97ab10f..e0b6f3c68 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -17,6 +17,7 @@ uuid = { version = "1.8.0", features = ["serde"] } serde = { version = "1.0", features = ["derive"] } thiserror = "1.0.61" tokio = { version = "1.38.0", default-features = false } +keyring = "2.3.3" gitbutler-git = { path = "crates/gitbutler-git" } gitbutler-core = { path = "crates/gitbutler-core" } diff --git a/crates/gitbutler-core/Cargo.toml b/crates/gitbutler-core/Cargo.toml index ee4a288a0..4f397e0b9 100644 --- a/crates/gitbutler-core/Cargo.toml +++ b/crates/gitbutler-core/Cargo.toml @@ -5,6 +5,9 @@ edition = "2021" authors = ["GitButler "] publish = false +[[test]] +name = "secret" +path = "tests/secret/mod.rs" [dev-dependencies] once_cell = "1.19" @@ -29,7 +32,7 @@ git2.workspace = true git2-hooks = "0.3" gix = { workspace = true, features = ["dirwalk"] } itertools = "0.13" -keyring = "2.3.3" +keyring.workspace = true lazy_static = "1.4.0" md5 = "0.7.0" hex = "0.4.3" diff --git a/crates/gitbutler-core/src/assets.rs b/crates/gitbutler-core/src/assets.rs index afb468d3f..08b81e679 100644 --- a/crates/gitbutler-core/src/assets.rs +++ b/crates/gitbutler-core/src/assets.rs @@ -27,20 +27,17 @@ impl Proxy { } } - pub async fn proxy_user(&self, user: users::User) -> users::User { - match Url::parse(&user.picture) { - Ok(picture) => users::User { - picture: self.proxy(&picture).await.map_or_else( - |error| { - tracing::error!(?error, "failed to proxy user picture"); - user.picture.clone() - }, - |url| url.to_string(), - ), - ..user - }, - Err(_) => user, + pub async fn proxy_user(&self, mut user: users::User) -> users::User { + if let Ok(picture) = Url::parse(&user.picture) { + user.picture = self.proxy(&picture).await.map_or_else( + |error| { + tracing::error!(?error, "failed to proxy user picture"); + user.picture.clone() + }, + |url| url.to_string(), + ); } + user } async fn proxy_virtual_branch_commit( diff --git a/crates/gitbutler-core/src/project_repository/repository.rs b/crates/gitbutler-core/src/project_repository/repository.rs index a85248f32..dd2cfd386 100644 --- a/crates/gitbutler-core/src/project_repository/repository.rs +++ b/crates/gitbutler-core/src/project_repository/repository.rs @@ -378,10 +378,10 @@ impl Repository { "pushing code to gb repo", ); - let access_token = user - .map(|user| user.access_token.clone()) - .context("access token is missing") + let user = user + .context("need user to push to gitbutler") .context(Code::ProjectGitAuth)?; + let access_token = user.access_token()?; let mut callbacks = git2::RemoteCallbacks::new(); if self.project.omit_certificate_check.unwrap_or(false) { diff --git a/crates/gitbutler-core/src/secret.rs b/crates/gitbutler-core/src/secret.rs index b891d5899..a2842d66c 100644 --- a/crates/gitbutler-core/src/secret.rs +++ b/crates/gitbutler-core/src/secret.rs @@ -19,6 +19,11 @@ pub fn retrieve(handle: &str) -> Result>> { } } +/// Delete the secret at `handle` permanently. +pub fn delete(handle: &str) -> Result<()> { + Ok(entry_for(handle)?.delete_password()?) +} + fn entry_for(handle: &str) -> Result { Ok(keyring::Entry::new("gitbutler", handle)?) } diff --git a/crates/gitbutler-core/src/users/controller.rs b/crates/gitbutler-core/src/users/controller.rs index d3277c567..f8eb19463 100644 --- a/crates/gitbutler-core/src/users/controller.rs +++ b/crates/gitbutler-core/src/users/controller.rs @@ -1,9 +1,16 @@ use anyhow::Context; +use anyhow::Result; use std::path::PathBuf; use super::{storage::Storage, User}; -/// TODO(ST): useless intermediary - remove +/// 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, @@ -18,21 +25,43 @@ impl Controller { Controller::new(Storage::from_path(path)) } - pub fn get_user(&self) -> anyhow::Result> { - match self.storage.get().context("failed to get user") { - Ok(user) => Ok(user), - Err(err) => { - self.storage.delete().ok(); - Err(err) - } + /// 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 set_user(&self, user: &User) -> anyhow::Result<()> { - self.storage.set(user).context("failed to set user") - } - - pub fn delete_user(&self) -> anyhow::Result<()> { - self.storage.delete().context("failed to delete user") + pub fn delete_user(&self) -> Result<()> { + self.storage.delete().context("failed to delete user")?; + crate::secret::delete(User::ACCESS_TOKEN_HANDLE).ok(); + crate::secret::delete(User::GITHUB_ACCESS_TOKEN_HANDLE).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; + if let Some(gb_token) = user.access_token.borrow_mut().take() { + needs_write |= crate::secret::persist(User::ACCESS_TOKEN_HANDLE, &gb_token).is_ok(); + } + if let Some(gh_token) = user.github_access_token.borrow_mut().take() { + needs_write |= crate::secret::persist(User::GITHUB_ACCESS_TOKEN_HANDLE, &gh_token).is_ok(); + } + if needs_write { + storage.set(&user)?; + } + Ok(needs_write) +} diff --git a/crates/gitbutler-core/src/users/user.rs b/crates/gitbutler-core/src/users/user.rs index 1f31c6d59..2c86c5023 100644 --- a/crates/gitbutler-core/src/users/user.rs +++ b/crates/gitbutler-core/src/users/user.rs @@ -1,5 +1,7 @@ use crate::types::Sensitive; +use anyhow::{Context, Result}; use serde::{Deserialize, Serialize}; +use std::cell::RefCell; #[derive(Debug, Deserialize, Serialize, Clone, Default)] pub struct User { @@ -12,9 +14,50 @@ pub struct User { pub locale: Option, pub created_at: String, pub updated_at: String, - pub access_token: Sensitive, + /// 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, - pub github_access_token: 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> { + 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) + } + } + } + + /// 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>> { + 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) + } + } + } +} diff --git a/crates/gitbutler-core/tests/core.rs b/crates/gitbutler-core/tests/core.rs index bda150f67..22e13a0fb 100644 --- a/crates/gitbutler-core/tests/core.rs +++ b/crates/gitbutler-core/tests/core.rs @@ -7,7 +7,6 @@ mod git; mod keys; mod lock; mod ops; -mod secret; mod types; pub mod virtual_branches; mod zip; diff --git a/crates/gitbutler-core/tests/fixtures/users/login-only.v1 b/crates/gitbutler-core/tests/fixtures/users/login-only.v1 new file mode 100644 index 000000000..23d1b4cb4 --- /dev/null +++ b/crates/gitbutler-core/tests/fixtures/users/login-only.v1 @@ -0,0 +1,15 @@ +{ + "id": 13612, + "name": "Sebastian Thiel", + "given_name": null, + "family_name": null, + "email": "sebastian.thiel@icloud.com", + "picture": "https://avatars.githubusercontent.com/u/63622?v=4", + "locale": null, + "created_at": "2024-03-26T13:17:05Z", + "updated_at": "2024-06-24T15:21:45Z", + "access_token": "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee", + "role": null, + "github_access_token": null, + "github_username": null +} diff --git a/crates/gitbutler-core/tests/fixtures/users/with-github.v1 b/crates/gitbutler-core/tests/fixtures/users/with-github.v1 new file mode 100644 index 000000000..3f3dbce80 --- /dev/null +++ b/crates/gitbutler-core/tests/fixtures/users/with-github.v1 @@ -0,0 +1,15 @@ +{ + "id": 13612, + "name": "Sebastian Thiel", + "given_name": null, + "family_name": null, + "email": "sebastian.thiel@icloud.com", + "picture": "https://avatars.githubusercontent.com/u/63622?v=4", + "locale": null, + "created_at": "2024-03-26T13:17:05Z", + "updated_at": "2024-06-24T15:21:45Z", + "access_token": "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee", + "role": null, + "github_access_token": "gho_AAAAAAAAAAAAABBBBBBBBBBBBBBBCCCCCCCC", + "github_username": null +} diff --git a/crates/gitbutler-core/tests/git/credentials.rs b/crates/gitbutler-core/tests/git/credentials.rs index c2e162363..72c151ba9 100644 --- a/crates/gitbutler-core/tests/git/credentials.rs +++ b/crates/gitbutler-core/tests/git/credentials.rs @@ -1,7 +1,6 @@ use std::path::PathBuf; use std::str; -use gitbutler_core::types::Sensitive; use gitbutler_core::{ git::credentials::{Credential, Helper, SshCredential}, keys, project_repository, projects, users, @@ -12,7 +11,7 @@ use gitbutler_testsupport::{temp_dir, test_repository}; #[derive(Default)] struct TestCase<'a> { remote_url: &'a str, - github_access_token: Option>, + with_github_login: bool, preferred_key: projects::AuthKey, } @@ -20,11 +19,14 @@ impl TestCase<'_> { fn run(&self) -> Vec<(String, Vec)> { let local_app_data = temp_dir(); + gitbutler_testsupport::secrets::setup_blackhole_store(); let users = users::Controller::from_path(local_app_data.path()); - let user = users::User { - github_access_token: self.github_access_token.map(|s| Sensitive(s.0.to_string())), - ..Default::default() - }; + let user: users::User = serde_json::from_str(if self.with_github_login { + include_str!("../../tests/fixtures/users/with-github.v1") + } else { + include_str!("../../tests/fixtures/users/login-only.v1") + }) + .expect("valid v1 sample user"); users.set_user(&user).unwrap(); let keys = keys::Controller::from_path(local_app_data.path()); @@ -56,7 +58,7 @@ mod not_github { fn https() { let test_case = TestCase { remote_url: "https://gitlab.com/test-gitbutler/test.git", - github_access_token: Some(Sensitive("token")), + with_github_login: true, preferred_key: projects::AuthKey::Local { private_key_path: PathBuf::from("/tmp/id_rsa"), }, @@ -80,7 +82,7 @@ mod not_github { fn ssh() { let test_case = TestCase { remote_url: "git@gitlab.com:test-gitbutler/test.git", - github_access_token: Some(Sensitive("token")), + with_github_login: true, preferred_key: projects::AuthKey::Local { private_key_path: PathBuf::from("/tmp/id_rsa"), }, @@ -115,7 +117,7 @@ mod github { fn https() { let test_case = TestCase { remote_url: "https://github.com/gitbutlerapp/gitbutler.git", - github_access_token: Some(Sensitive("token")), + with_github_login: true, preferred_key: projects::AuthKey::Local { private_key_path: PathBuf::from("/tmp/id_rsa"), }, @@ -139,7 +141,7 @@ mod github { fn ssh() { let test_case = TestCase { remote_url: "git@github.com:gitbutlerapp/gitbutler.git", - github_access_token: Some(Sensitive("token")), + with_github_login: true, preferred_key: projects::AuthKey::Local { private_key_path: PathBuf::from("/tmp/id_rsa"), }, diff --git a/crates/gitbutler-core/tests/secret/credentials.rs b/crates/gitbutler-core/tests/secret/credentials.rs new file mode 100644 index 000000000..707d767ae --- /dev/null +++ b/crates/gitbutler-core/tests/secret/credentials.rs @@ -0,0 +1,96 @@ +use keyring::credential::{CredentialApi, CredentialBuilderApi, CredentialPersistence}; +use keyring::Credential; +use std::any::Any; +use std::collections::BTreeMap; +use std::sync::{Arc, Mutex}; + +#[derive(Default)] +pub(super) struct Store(BTreeMap); + +pub(super) type SharedStore = Arc>; + +struct Entry { + handle: String, + store: SharedStore, +} + +impl CredentialApi for Entry { + fn set_password(&self, password: &str) -> keyring::Result<()> { + self.store + .lock() + .unwrap() + .0 + .insert(self.handle.clone(), password.into()); + Ok(()) + } + + fn get_password(&self) -> keyring::Result { + match self.store.lock().unwrap().0.get(&self.handle) { + Some(secret) => Ok(secret.clone()), + None => Err(keyring::Error::NoEntry), + } + } + + fn delete_password(&self) -> keyring::Result<()> { + self.store.lock().unwrap().0.remove(&self.handle); + Ok(()) + } + + fn as_any(&self) -> &dyn Any { + self + } +} + +pub(super) struct Builder { + pub(super) store: SharedStore, +} + +impl CredentialBuilderApi for Builder { + fn build( + &self, + _target: Option<&str>, + _service: &str, + user: &str, + ) -> keyring::Result> { + let credential = Entry { + handle: user.to_string(), + store: self.store.clone(), + }; + Ok(Box::new(credential)) + } + + fn as_any(&self) -> &dyn std::any::Any { + self + } + + /// We keep information in memory + fn persistence(&self) -> CredentialPersistence { + CredentialPersistence::ProcessOnly + } +} + +static CURRENT_STORE: Mutex> = Mutex::new(None); + +/// Initialize the credentials store to be isolated and usable for testing. +/// +/// Note that this is a resource shared in the process, and deterministic tests must +/// use the `[serial]` annotation. +pub fn setup() { + let store = SharedStore::default(); + *CURRENT_STORE.lock().unwrap() = Some(store.clone()); + + keyring::set_default_credential_builder(Box::new(Builder { store })); +} + +/// Return the amount of stored secrets +pub fn count() -> usize { + CURRENT_STORE + .lock() + .unwrap() + .as_ref() + .expect("BUG: call setup") + .lock() + .unwrap() + .0 + .len() +} diff --git a/crates/gitbutler-core/tests/secret/mod.rs b/crates/gitbutler-core/tests/secret/mod.rs index c2aedb379..3c3cb7774 100644 --- a/crates/gitbutler-core/tests/secret/mod.rs +++ b/crates/gitbutler-core/tests/secret/mod.rs @@ -1,3 +1,6 @@ +//! Note that these tests *must* be run in their own process, as they rely on having a deterministic +//! credential store. Due to its global nature, tests cannot run in parallel +//! (or mixed with parallel tests that set their own credential store) use gitbutler_core::secret; use gitbutler_core::types::Sensitive; use serial_test::serial; @@ -5,7 +8,7 @@ use serial_test::serial; #[test] #[serial] fn retrieve_unknown_is_none() { - setup(); + credentials::setup(); assert!(secret::retrieve("does not exist for sure") .expect("no error to ask for non-existing") .is_none()); @@ -14,7 +17,7 @@ fn retrieve_unknown_is_none() { #[test] #[serial] fn store_and_retrieve() -> anyhow::Result<()> { - setup(); + credentials::setup(); secret::persist("new", &Sensitive("secret".into()))?; let secret = secret::retrieve("new")?.expect("it was just stored"); assert_eq!( @@ -25,81 +28,5 @@ fn store_and_retrieve() -> anyhow::Result<()> { Ok(()) } -fn setup() { - keyring::set_default_credential_builder(Box::::default()); -} - -mod credentials { - use keyring::credential::{CredentialApi, CredentialBuilderApi, CredentialPersistence}; - use keyring::Credential; - use std::any::Any; - use std::collections::BTreeMap; - use std::sync::{Arc, Mutex}; - - #[derive(Default)] - struct Store { - store: BTreeMap, - } - - type SharedStore = Arc>; - - struct Entry { - handle: String, - inner: SharedStore, - } - - impl CredentialApi for Entry { - fn set_password(&self, password: &str) -> keyring::Result<()> { - self.inner - .lock() - .unwrap() - .store - .insert(self.handle.clone(), password.into()); - Ok(()) - } - - fn get_password(&self) -> keyring::Result { - match self.inner.lock().unwrap().store.get(&self.handle) { - Some(secret) => Ok(secret.clone()), - None => Err(keyring::Error::NoEntry), - } - } - - fn delete_password(&self) -> keyring::Result<()> { - todo!() - } - - fn as_any(&self) -> &dyn Any { - self - } - } - - #[derive(Default)] - pub(super) struct Builder { - store: SharedStore, - } - - impl CredentialBuilderApi for Builder { - fn build( - &self, - _target: Option<&str>, - _service: &str, - user: &str, - ) -> keyring::Result> { - let credential = Entry { - handle: user.to_string(), - inner: self.store.clone(), - }; - Ok(Box::new(credential)) - } - - fn as_any(&self) -> &dyn std::any::Any { - self - } - - /// We keep information in memory - fn persistence(&self) -> CredentialPersistence { - CredentialPersistence::ProcessOnly - } - } -} +pub(crate) mod credentials; +mod users; diff --git a/crates/gitbutler-core/tests/secret/users.rs b/crates/gitbutler-core/tests/secret/users.rs new file mode 100644 index 000000000..89ce25afe --- /dev/null +++ b/crates/gitbutler-core/tests/secret/users.rs @@ -0,0 +1,94 @@ +use crate::{credentials, credentials::count as count_secrets}; +use serial_test::serial; +use std::path::{Path, PathBuf}; +use tempfile::tempdir; + +/// 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. +#[test] +#[serial] +fn auto_migration_of_secrets_on_when_getting_and_setting_user() -> anyhow::Result<()> { + for (name, has_github_token) in [("login-only.v1", false), ("with-github.v1", true)] { + credentials::setup(); + let app_data = tempdir()?; + + let users = gitbutler_core::users::Controller::from_path(app_data.path()); + assert!( + users.get_user()?.is_none(), + "Users are bound to logins, so there is none by default" + ); + assert_eq!(count_secrets(), 0, "no secret is associated with anything"); + + let buf = std::fs::read(user_fixture(name))?; + let user_json_path = app_data.path().join("user.json"); + std::fs::write(&user_json_path, &buf)?; + + let user = users.get_user()?.expect("previous v1 user was read"); + let expected_secrets = if has_github_token { 2 } else { 1 }; + assert_eq!( + count_secrets(), + 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 { + assert_eq!( + user.github_access_token()?.map(|s| s.0), + Some("gho_AAAAAAAAAAAAABBBBBBBBBBBBBBBCCCCCCCC".into()), + "it can make the access token available" + ); + } + + let assert_no_secret_in_plain_text = || -> anyhow::Result<()> { + let buf = std::fs::read(&user_json_path)?; + let value: serde_json::Value = serde_json::from_slice(&buf)?; + assert_eq!( + value.get("access_token"), + None, + "access token wasn't written back (right after getting it)" + ); + assert_eq!( + value.get("github_access_token"), + None, + "access token wasn't written back" + ); + Ok(()) + }; + assert_no_secret_in_plain_text()?; + + users.delete_user()?; + assert_eq!( + count_secrets(), + 0, + "deletion of a user automatically deletes its secretes" + ); + assert!( + !user_json_path.exists(), + "it deletes the whole file, i.e. all associated user data" + ); + + users.set_user(&user)?; + assert_eq!( + count_secrets(), + expected_secrets, + "the in-memory users had its secrets cached, so they are picked up and stored officially" + ); + assert_no_secret_in_plain_text()?; + } + + Ok(()) +} + +fn user_fixture(name: &str) -> PathBuf { + let fixture = Path::new("tests/fixtures/users").join(name); + assert!( + fixture.exists(), + "BUG: fixture at {fixture:?} ought to exist" + ); + fixture +} diff --git a/crates/gitbutler-testsupport/Cargo.toml b/crates/gitbutler-testsupport/Cargo.toml index de230d00a..a94ec1ea2 100644 --- a/crates/gitbutler-testsupport/Cargo.toml +++ b/crates/gitbutler-testsupport/Cargo.toml @@ -15,4 +15,6 @@ once_cell = "1.19" git2.workspace = true pretty_assertions = "1.4" tempfile = "3.10.1" +keyring.workspace = true +serde_json = "1.0" gitbutler-core = { path = "../gitbutler-core" } diff --git a/crates/gitbutler-testsupport/src/fixtures/user/minimal.v1 b/crates/gitbutler-testsupport/src/fixtures/user/minimal.v1 new file mode 100644 index 000000000..a84e39f50 --- /dev/null +++ b/crates/gitbutler-testsupport/src/fixtures/user/minimal.v1 @@ -0,0 +1,15 @@ +{ + "id": 0, + "name": "test", + "given_name": null, + "family_name": null, + "email": "test@email.com", + "picture": "", + "locale": null, + "created_at": "", + "updated_at": "", + "access_token": "token", + "role": null, + "github_access_token": null, + "github_username": null +} diff --git a/crates/gitbutler-testsupport/src/lib.rs b/crates/gitbutler-testsupport/src/lib.rs index fdf99ab06..1fe667bcd 100644 --- a/crates/gitbutler-testsupport/src/lib.rs +++ b/crates/gitbutler-testsupport/src/lib.rs @@ -60,3 +60,9 @@ pub fn init_opts_bare() -> git2::RepositoryInitOptions { opts.bare(true); opts } + +/// A secrets store to prevent secrets to be written into the systems own store. +/// +/// Note that this can't be used if secrets themselves are under test as it' doesn't act +/// like a real store, i.e. stored secrets can't be retrieved anymore. +pub mod secrets; diff --git a/crates/gitbutler-testsupport/src/secrets.rs b/crates/gitbutler-testsupport/src/secrets.rs new file mode 100644 index 000000000..7c4beb60c --- /dev/null +++ b/crates/gitbutler-testsupport/src/secrets.rs @@ -0,0 +1,45 @@ +use std::any::Any; + +/// Assure we have a mock secrets store so tests don't start writing secrets into the user's actual store, +/// as this will affect their GitButler instance. +pub fn setup_blackhole_store() { + keyring::set_default_credential_builder(Box::new(BlackholeBuilder)) +} + +/// A builder that is completely mocked, able to read no secret, but allowing to write any without error. +struct BlackholeBuilder; + +struct BlackholeCredential; + +impl keyring::credential::CredentialApi for BlackholeCredential { + fn set_password(&self, _password: &str) -> keyring::Result<()> { + Ok(()) + } + + fn get_password(&self) -> keyring::Result { + Err(keyring::Error::NoEntry) + } + + fn delete_password(&self) -> keyring::Result<()> { + Ok(()) + } + + fn as_any(&self) -> &dyn Any { + self + } +} + +impl keyring::credential::CredentialBuilderApi for BlackholeBuilder { + fn build( + &self, + _target: Option<&str>, + _service: &str, + _user: &str, + ) -> keyring::Result> { + Ok(Box::new(BlackholeCredential)) + } + + fn as_any(&self) -> &dyn Any { + self + } +} diff --git a/crates/gitbutler-testsupport/src/suite.rs b/crates/gitbutler-testsupport/src/suite.rs index cc8af71a2..b6d188dc7 100644 --- a/crates/gitbutler-testsupport/src/suite.rs +++ b/crates/gitbutler-testsupport/src/suite.rs @@ -4,7 +4,6 @@ use std::{ path::{Path, PathBuf}, }; -use gitbutler_core::types::Sensitive; use gitbutler_core::{git::RepositoryExt, project_repository}; use tempfile::{tempdir, TempDir}; @@ -48,12 +47,10 @@ impl Suite { self.local_app_data.as_ref().unwrap().path() } pub fn sign_in(&self) -> gitbutler_core::users::User { - let user = gitbutler_core::users::User { - name: Some("test".to_string()), - email: "test@email.com".to_string(), - access_token: Sensitive("token".to_string()), - ..Default::default() - }; + crate::secrets::setup_blackhole_store(); + let user: gitbutler_core::users::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"); user } From bc8f3b74c7d0f2968bb77f8187ba9da2fc060f86 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Tue, 25 Jun 2024 17:28:02 +0200 Subject: [PATCH 04/13] `tauri` specializes the secret-store depending on application type That way, there is no overlap in the technically global store, just like before when secrets were stored in plain text. --- crates/gitbutler-core/src/secret.rs | 20 ++++++++++++++++++-- crates/gitbutler-tauri/src/main.rs | 3 +++ 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/crates/gitbutler-core/src/secret.rs b/crates/gitbutler-core/src/secret.rs index a2842d66c..cfe93e4e1 100644 --- a/crates/gitbutler-core/src/secret.rs +++ b/crates/gitbutler-core/src/secret.rs @@ -4,6 +4,7 @@ //! in memory beyond their use. use crate::types::Sensitive; use anyhow::Result; +use std::sync::Mutex; /// Persist `secret` so that it can be retrieved by the given `handle`. pub fn persist(handle: &str, secret: &Sensitive) -> Result<()> { @@ -24,6 +25,21 @@ pub fn delete(handle: &str) -> Result<()> { Ok(entry_for(handle)?.delete_password()?) } -fn entry_for(handle: &str) -> Result { - Ok(keyring::Entry::new("gitbutler", handle)?) +/// Use this `identifier` as 'namespace' for identifying secrets. +/// Each namespace has its own set of secrets, useful for different application versions. +/// +/// Note that the namespace will default to `gitbutler` if empty. +pub fn set_application_namespace(identifier: impl Into) { + *NAMESPACE.lock().unwrap() = identifier.into() } + +fn entry_for(handle: &str) -> Result { + let ns = NAMESPACE.lock().unwrap(); + Ok(keyring::Entry::new( + if ns.is_empty() { "gitbutler" } else { &ns }, + handle, + )?) +} + +/// How to further specialize secrets to avoid name clashes in the globally shared keystore. +static NAMESPACE: Mutex = Mutex::new(String::new()); diff --git a/crates/gitbutler-tauri/src/main.rs b/crates/gitbutler-tauri/src/main.rs index 1ba536ac4..92fabb31e 100644 --- a/crates/gitbutler-tauri/src/main.rs +++ b/crates/gitbutler-tauri/src/main.rs @@ -23,6 +23,9 @@ use tauri_plugin_log::LogTarget; fn main() { let tauri_context = generate_context!(); + gitbutler_core::secret::set_application_namespace( + &tauri_context.config().tauri.bundle.identifier, + ); tokio::runtime::Builder::new_multi_thread() .enable_all() From ee5ff95649144d39b13df6d5e2f2f143407816b0 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Tue, 25 Jun 2024 17:21:40 +0200 Subject: [PATCH 05/13] 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 | 4 +- crates/gitbutler-core/src/users/user.rs | 27 ++++----- crates/gitbutler-core/tests/secret/users.rs | 27 ++++++--- crates/gitbutler-tauri/src/users.rs | 60 +++++++++++++++++++- 4 files changed, 89 insertions(+), 29 deletions(-) diff --git a/crates/gitbutler-core/src/types/sensitive.rs b/crates/gitbutler-core/src/types/sensitive.rs index 29346a011..a40d75cfd 100644 --- a/crates/gitbutler-core/src/types/sensitive.rs +++ b/crates/gitbutler-core/src/types/sensitive.rs @@ -6,11 +6,11 @@ impl Serialize for Sensitive where T: Serialize, { - fn serialize(&self, serializer: S) -> Result + fn serialize(&self, _serializer: S) -> Result 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 diff --git a/crates/gitbutler-core/src/users/user.rs b/crates/gitbutler-core/src/users/user.rs index 2c86c5023..f36376580 100644 --- a/crates/gitbutler-core/src/users/user.rs +++ b/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> { - 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>> { - 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) } } diff --git a/crates/gitbutler-core/tests/secret/users.rs b/crates/gitbutler-core/tests/secret/users.rs index 89ce25afe..ebcca5ff1 100644 --- a/crates/gitbutler-core/tests/secret/users.rs +++ b/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(), diff --git a/crates/gitbutler-tauri/src/users.rs b/crates/gitbutler-tauri/src/users.rs index 8338ae5a1..ccacd84d5 100644 --- a/crates/gitbutler-tauri/src/users.rs +++ b/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, Error> { + pub async fn get_user(handle: AppHandle) -> Result, Error> { let app = handle.state::(); let proxy = handle.state::(); 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, + given_name: Option, + family_name: Option, + email: String, + picture: String, + locale: Option, + created_at: String, + updated_at: String, + access_token: String, + role: Option, + github_access_token: Option, + github_username: Option, + } + + impl TryFrom for UserWithSecrets { + type Error = anyhow::Error; + + fn try_from(value: User) -> Result { + 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, + }) + } + } } From fbfeba0637df5d730bf9e4728e7d36d1d54dda4d Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Tue, 25 Jun 2024 21:44:57 +0200 Subject: [PATCH 06/13] auto-delete a user if its access token can't be found --- crates/gitbutler-core/src/users/user.rs | 2 +- crates/gitbutler-core/tests/secret/users.rs | 18 +++++++++++++++++- crates/gitbutler-tauri/src/users.rs | 8 +++++++- 3 files changed, 25 insertions(+), 3 deletions(-) diff --git a/crates/gitbutler-core/src/users/user.rs b/crates/gitbutler-core/src/users/user.rs index f36376580..f56461829 100644 --- a/crates/gitbutler-core/src/users/user.rs +++ b/crates/gitbutler-core/src/users/user.rs @@ -38,7 +38,7 @@ impl User { 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 err_msg = "access token for user was deleted from keychain - login is now invalid"; let secret = crate::secret::retrieve(Self::ACCESS_TOKEN_HANDLE)?.context(err_msg)?; *self.access_token.borrow_mut() = Some(secret.clone()); Ok(secret) diff --git a/crates/gitbutler-core/tests/secret/users.rs b/crates/gitbutler-core/tests/secret/users.rs index ebcca5ff1..68a204bce 100644 --- a/crates/gitbutler-core/tests/secret/users.rs +++ b/crates/gitbutler-core/tests/secret/users.rs @@ -85,9 +85,25 @@ fn auto_migration_of_secrets_on_when_getting_and_setting_user() -> anyhow::Resul assert_eq!( count_secrets(), expected_secrets, - "the in-memory users had its secrets cached, so they are picked up and stored officially" + "the in-memory users had its secrets cached, so they are picked up and stored officially. \ + This is important, as the frontend sends these initially" ); assert_no_secret_in_plain_text()?; + + // forget all passwords + credentials::setup(); + let user = users + .get_user()? + .expect("user still on disk and passwords are accessed lazily"); + assert!( + user.access_token().is_err(), + "this is critical - we have a user without access token, this fails early" + ); + assert!( + users.get_user()?.is_some(), + "Client code needs to handle this case and delete the user, \ + otherwise it's there and errors forever" + ); } Ok(()) diff --git a/crates/gitbutler-tauri/src/users.rs b/crates/gitbutler-tauri/src/users.rs index ccacd84d5..baa74af1d 100644 --- a/crates/gitbutler-tauri/src/users.rs +++ b/crates/gitbutler-tauri/src/users.rs @@ -16,7 +16,13 @@ pub mod commands { let proxy = handle.state::(); match app.get_user()? { - Some(user) => Ok(Some(proxy.proxy_user(user).await.try_into()?)), + Some(user) => { + if let Err(err) = user.access_token() { + app.delete_user()?; + return Err(err.context("Please login to GitButler again").into()); + } + Ok(Some(proxy.proxy_user(user).await.try_into()?)) + } None => Ok(None), } } From 9da051b089382c6cd44d68cb42c1adc078894a0c Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Wed, 26 Jun 2024 16:46:12 +0200 Subject: [PATCH 07/13] improve naming of tokens stored in the keychain: do not 'abuse' user field Currently, the user is used as handle name, but instead, just name it `GitButler`. --- crates/gitbutler-core/src/secret.rs | 7 +++++-- crates/gitbutler-core/tests/secret/credentials.rs | 6 +++--- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/crates/gitbutler-core/src/secret.rs b/crates/gitbutler-core/src/secret.rs index cfe93e4e1..0e0c18091 100644 --- a/crates/gitbutler-core/src/secret.rs +++ b/crates/gitbutler-core/src/secret.rs @@ -36,8 +36,11 @@ pub fn set_application_namespace(identifier: impl Into) { fn entry_for(handle: &str) -> Result { let ns = NAMESPACE.lock().unwrap(); Ok(keyring::Entry::new( - if ns.is_empty() { "gitbutler" } else { &ns }, - handle, + &format!( + "{prefix}-{handle}", + prefix = if ns.is_empty() { "development" } else { &ns } + ), + "GitButler", )?) } diff --git a/crates/gitbutler-core/tests/secret/credentials.rs b/crates/gitbutler-core/tests/secret/credentials.rs index 707d767ae..bdd1d8440 100644 --- a/crates/gitbutler-core/tests/secret/credentials.rs +++ b/crates/gitbutler-core/tests/secret/credentials.rs @@ -49,11 +49,11 @@ impl CredentialBuilderApi for Builder { fn build( &self, _target: Option<&str>, - _service: &str, - user: &str, + service: &str, + _user: &str, ) -> keyring::Result> { let credential = Entry { - handle: user.to_string(), + handle: service.to_string(), store: self.store.clone(), }; Ok(Box::new(credential)) From 7e7555567b288ec6a1ebacff0276e52e6ad0b3ef Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Wed, 26 Jun 2024 16:57:41 +0200 Subject: [PATCH 08/13] automatically delete empty passwords from the key store The frontend uses this to invalidate the GitHub token, even though it can also deal with `null`. Let's keep the keystore clean and only keep entries that contain an actual password. Note that the consumers, i.e. the frontend, handle an empty password for the short time it's in memory. --- crates/gitbutler-core/src/secret.rs | 8 +++++++- crates/gitbutler-core/tests/secret/mod.rs | 17 +++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/crates/gitbutler-core/src/secret.rs b/crates/gitbutler-core/src/secret.rs index 0e0c18091..e34c3ab51 100644 --- a/crates/gitbutler-core/src/secret.rs +++ b/crates/gitbutler-core/src/secret.rs @@ -8,7 +8,13 @@ use std::sync::Mutex; /// Persist `secret` so that it can be retrieved by the given `handle`. pub fn persist(handle: &str, secret: &Sensitive) -> Result<()> { - Ok(entry_for(handle)?.set_password(&secret.0)?) + let entry = entry_for(handle)?; + if secret.0.is_empty() { + entry.delete_password()?; + } else { + entry.set_password(&secret.0)?; + } + Ok(()) } /// Obtain the previously [stored](persist()) secret known as `handle`. diff --git a/crates/gitbutler-core/tests/secret/mod.rs b/crates/gitbutler-core/tests/secret/mod.rs index 3c3cb7774..ed35783cc 100644 --- a/crates/gitbutler-core/tests/secret/mod.rs +++ b/crates/gitbutler-core/tests/secret/mod.rs @@ -28,5 +28,22 @@ fn store_and_retrieve() -> anyhow::Result<()> { Ok(()) } +#[test] +#[serial] +fn store_empty_equals_deletion() -> anyhow::Result<()> { + credentials::setup(); + secret::persist("new", &Sensitive("secret".into()))?; + assert_eq!(credentials::count(), 1); + + secret::persist("new", &Sensitive("".into()))?; + assert_eq!( + secret::retrieve("new")?.map(|s| s.0), + None, + "empty passwords are automatically deleted" + ); + assert_eq!(credentials::count(), 0); + Ok(()) +} + pub(crate) mod credentials; mod users; From b08a9e8cb12912e547877dd8e056a529d8a5635f Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Tue, 25 Jun 2024 21:42:04 +0200 Subject: [PATCH 09/13] Prevent MacOS popups each startup on MacOS MacOS is the only known platform that exhibits this behaviour - if an app is recompiled, the hash of the binary is used to identify it towards the keychain. As this changes each time, the keychain will ask for permission, which is fair. However, it's also an impediment which leads to the implementation of a keystore that uses git credentials as backend. For this to work, the latest version of `gitoxide` is required for now. --- Cargo.lock | 256 +++++++++++++++------------- Cargo.toml | 3 +- crates/gitbutler-core/Cargo.toml | 4 +- crates/gitbutler-core/src/secret.rs | 165 ++++++++++++++++++ crates/gitbutler-tauri/src/main.rs | 8 + 5 files changed, 314 insertions(+), 122 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8cf1647f2..826d15777 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2172,7 +2172,7 @@ name = "gitbutler-git" version = "0.0.0" dependencies = [ "futures", - "gix-path", + "gix-path 0.10.8 (registry+https://github.com/rust-lang/crates.io-index)", "nix 0.29.0", "rand 0.8.5", "serde", @@ -2273,14 +2273,14 @@ dependencies = [ [[package]] name = "gix" version = "0.63.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "984c5018adfa7a4536ade67990b3ebc6e11ab57b3d6cd9968de0947ca99b4b06" +source = "git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e#55cbc1b9d6f210298a86502a7f20f9736c7e963e" dependencies = [ "gix-actor", "gix-attributes", "gix-command", "gix-commitgraph", "gix-config", + "gix-credentials", "gix-date", "gix-diff", "gix-dir", @@ -2295,11 +2295,13 @@ dependencies = [ "gix-index", "gix-lock", "gix-macros", + "gix-negotiate", "gix-object", "gix-odb", "gix-pack", - "gix-path", + "gix-path 0.10.8 (git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e)", "gix-pathspec", + "gix-prompt", "gix-ref", "gix-refspec", "gix-revision", @@ -2307,14 +2309,13 @@ dependencies = [ "gix-sec", "gix-submodule", "gix-tempfile", - "gix-trace", + "gix-trace 0.1.9 (git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e)", "gix-traverse", "gix-url", "gix-utils", "gix-validate", "gix-worktree", "once_cell", - "parking_lot 0.12.3", "smallvec", "thiserror", ] @@ -2322,8 +2323,7 @@ dependencies = [ [[package]] name = "gix-actor" version = "0.31.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d69c59d392c7e6c94385b6fd6089d6df0fe945f32b4357687989f3aee253cd7f" +source = "git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e#55cbc1b9d6f210298a86502a7f20f9736c7e963e" dependencies = [ "bstr", "gix-date", @@ -2336,14 +2336,13 @@ dependencies = [ [[package]] name = "gix-attributes" version = "0.22.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "eefb48f42eac136a4a0023f49a54ec31be1c7a9589ed762c45dcb9b953f7ecc8" +source = "git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e#55cbc1b9d6f210298a86502a7f20f9736c7e963e" dependencies = [ "bstr", "gix-glob", - "gix-path", + "gix-path 0.10.8 (git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e)", "gix-quote", - "gix-trace", + "gix-trace 0.1.9 (git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e)", "kstring", "smallvec", "thiserror", @@ -2353,8 +2352,7 @@ dependencies = [ [[package]] name = "gix-bitmap" version = "0.2.11" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a371db66cbd4e13f0ed9dc4c0fea712d7276805fccc877f77e96374d317e87ae" +source = "git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e#55cbc1b9d6f210298a86502a7f20f9736c7e963e" dependencies = [ "thiserror", ] @@ -2362,8 +2360,7 @@ dependencies = [ [[package]] name = "gix-chunk" version = "0.4.8" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "45c8751169961ba7640b513c3b24af61aa962c967aaf04116734975cd5af0c52" +source = "git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e#55cbc1b9d6f210298a86502a7f20f9736c7e963e" dependencies = [ "thiserror", ] @@ -2371,20 +2368,18 @@ dependencies = [ [[package]] name = "gix-command" version = "0.3.7" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6c22e086314095c43ffe5cdc5c0922d5439da4fd726f3b0438c56147c34dc225" +source = "git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e#55cbc1b9d6f210298a86502a7f20f9736c7e963e" dependencies = [ "bstr", - "gix-path", - "gix-trace", + "gix-path 0.10.8 (git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e)", + "gix-trace 0.1.9 (git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e)", "shell-words", ] [[package]] name = "gix-commitgraph" version = "0.24.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f7b102311085da4af18823413b5176d7c500fb2272eaf391cfa8635d8bcb12c4" +source = "git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e#55cbc1b9d6f210298a86502a7f20f9736c7e963e" dependencies = [ "bstr", "gix-chunk", @@ -2397,14 +2392,13 @@ dependencies = [ [[package]] name = "gix-config" version = "0.37.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "53fafe42957e11d98e354a66b6bd70aeea00faf2f62dd11164188224a507c840" +source = "git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e#55cbc1b9d6f210298a86502a7f20f9736c7e963e" dependencies = [ "bstr", "gix-config-value", "gix-features", "gix-glob", - "gix-path", + "gix-path 0.10.8 (git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e)", "gix-ref", "gix-sec", "memchr", @@ -2418,21 +2412,35 @@ dependencies = [ [[package]] name = "gix-config-value" version = "0.14.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fbd06203b1a9b33a78c88252a625031b094d9e1b647260070c25b09910c0a804" +source = "git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e#55cbc1b9d6f210298a86502a7f20f9736c7e963e" dependencies = [ "bitflags 2.5.0", "bstr", - "gix-path", + "gix-path 0.10.8 (git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e)", "libc", "thiserror", ] +[[package]] +name = "gix-credentials" +version = "0.24.2" +source = "git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e#55cbc1b9d6f210298a86502a7f20f9736c7e963e" +dependencies = [ + "bstr", + "gix-command", + "gix-config-value", + "gix-path 0.10.8 (git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e)", + "gix-prompt", + "gix-sec", + "gix-trace 0.1.9 (git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e)", + "gix-url", + "thiserror", +] + [[package]] name = "gix-date" -version = "0.8.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "367ee9093b0c2b04fd04c5c7c8b6a1082713534eab537597ae343663a518fa99" +version = "0.8.7" +source = "git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e#55cbc1b9d6f210298a86502a7f20f9736c7e963e" dependencies = [ "bstr", "itoa 1.0.11", @@ -2443,8 +2451,7 @@ dependencies = [ [[package]] name = "gix-diff" version = "0.44.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "40b9bd8b2d07b6675a840b56a6c177d322d45fa082672b0dad8f063b25baf0a4" +source = "git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e#55cbc1b9d6f210298a86502a7f20f9736c7e963e" dependencies = [ "bstr", "gix-hash", @@ -2455,8 +2462,7 @@ dependencies = [ [[package]] name = "gix-dir" version = "0.5.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "60c99f8c545abd63abe541d20ab6cda347de406c0a3f1c80aadc12d9b0e94974" +source = "git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e#55cbc1b9d6f210298a86502a7f20f9736c7e963e" dependencies = [ "bstr", "gix-discover", @@ -2464,9 +2470,9 @@ dependencies = [ "gix-ignore", "gix-index", "gix-object", - "gix-path", + "gix-path 0.10.8 (git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e)", "gix-pathspec", - "gix-trace", + "gix-trace 0.1.9 (git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e)", "gix-utils", "gix-worktree", "thiserror", @@ -2475,14 +2481,13 @@ dependencies = [ [[package]] name = "gix-discover" version = "0.32.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fc27c699b63da66b50d50c00668bc0b7e90c3a382ef302865e891559935f3dbf" +source = "git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e#55cbc1b9d6f210298a86502a7f20f9736c7e963e" dependencies = [ "bstr", "dunce", "gix-fs", "gix-hash", - "gix-path", + "gix-path 0.10.8 (git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e)", "gix-ref", "gix-sec", "thiserror", @@ -2491,16 +2496,17 @@ dependencies = [ [[package]] name = "gix-features" version = "0.38.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ac7045ac9fe5f9c727f38799d002a7ed3583cd777e3322a7c4b43e3cf437dc69" +source = "git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e#55cbc1b9d6f210298a86502a7f20f9736c7e963e" dependencies = [ "crc32fast", + "crossbeam-channel", "flate2", "gix-hash", - "gix-trace", + "gix-trace 0.1.9 (git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e)", "gix-utils", "libc", "once_cell", + "parking_lot 0.12.3", "prodash", "sha1_smol", "thiserror", @@ -2510,8 +2516,7 @@ dependencies = [ [[package]] name = "gix-filter" version = "0.11.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "00ce6ea5ac8fca7adbc63c48a1b9e0492c222c386aa15f513405f1003f2f4ab2" +source = "git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e#55cbc1b9d6f210298a86502a7f20f9736c7e963e" dependencies = [ "bstr", "encoding_rs", @@ -2520,9 +2525,9 @@ dependencies = [ "gix-hash", "gix-object", "gix-packetline-blocking", - "gix-path", + "gix-path 0.10.8 (git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e)", "gix-quote", - "gix-trace", + "gix-trace 0.1.9 (git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e)", "gix-utils", "smallvec", "thiserror", @@ -2531,8 +2536,7 @@ dependencies = [ [[package]] name = "gix-fs" version = "0.11.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c3338ff92a2164f5209f185ec0cd316f571a72676bb01d27e22f2867ba69f77a" +source = "git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e#55cbc1b9d6f210298a86502a7f20f9736c7e963e" dependencies = [ "fastrand 2.1.0", "gix-features", @@ -2542,20 +2546,18 @@ dependencies = [ [[package]] name = "gix-glob" version = "0.16.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c2a29ad0990cf02c48a7aac76ed0dbddeb5a0d070034b83675cc3bbf937eace4" +source = "git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e#55cbc1b9d6f210298a86502a7f20f9736c7e963e" dependencies = [ "bitflags 2.5.0", "bstr", "gix-features", - "gix-path", + "gix-path 0.10.8 (git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e)", ] [[package]] name = "gix-hash" version = "0.14.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f93d7df7366121b5018f947a04d37f034717e113dcf9ccd85c34b58e57a74d5e" +source = "git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e#55cbc1b9d6f210298a86502a7f20f9736c7e963e" dependencies = [ "faster-hex", "thiserror", @@ -2564,8 +2566,7 @@ dependencies = [ [[package]] name = "gix-hashtable" version = "0.5.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7ddf80e16f3c19ac06ce415a38b8591993d3f73aede049cb561becb5b3a8e242" +source = "git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e#55cbc1b9d6f210298a86502a7f20f9736c7e963e" dependencies = [ "gix-hash", "hashbrown 0.14.3", @@ -2575,21 +2576,19 @@ dependencies = [ [[package]] name = "gix-ignore" version = "0.11.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "640dbeb4f5829f9fc14d31f654a34a0350e43a24e32d551ad130d99bf01f63f1" +source = "git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e#55cbc1b9d6f210298a86502a7f20f9736c7e963e" dependencies = [ "bstr", "gix-glob", - "gix-path", - "gix-trace", + "gix-path 0.10.8 (git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e)", + "gix-trace 0.1.9 (git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e)", "unicode-bom", ] [[package]] name = "gix-index" version = "0.33.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2d8c5a5f1c58edcbc5692b174cda2703aba82ed17d7176ff4c1752eb48b1b167" +source = "git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e#55cbc1b9d6f210298a86502a7f20f9736c7e963e" dependencies = [ "bitflags 2.5.0", "bstr", @@ -2616,8 +2615,7 @@ dependencies = [ [[package]] name = "gix-lock" version = "14.0.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e3bc7fe297f1f4614774989c00ec8b1add59571dc9b024b4c00acb7dedd4e19d" +source = "git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e#55cbc1b9d6f210298a86502a7f20f9736c7e963e" dependencies = [ "gix-tempfile", "gix-utils", @@ -2627,19 +2625,32 @@ dependencies = [ [[package]] name = "gix-macros" version = "0.1.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "999ce923619f88194171a67fb3e6d613653b8d4d6078b529b15a765da0edcc17" +source = "git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e#55cbc1b9d6f210298a86502a7f20f9736c7e963e" dependencies = [ "proc-macro2", "quote", "syn 2.0.58", ] +[[package]] +name = "gix-negotiate" +version = "0.13.1" +source = "git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e#55cbc1b9d6f210298a86502a7f20f9736c7e963e" +dependencies = [ + "bitflags 2.5.0", + "gix-commitgraph", + "gix-date", + "gix-hash", + "gix-object", + "gix-revwalk", + "smallvec", + "thiserror", +] + [[package]] name = "gix-object" version = "0.42.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1fe2dc4a41191c680c942e6ebd630c8107005983c4679214fdb1007dcf5ae1df" +source = "git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e#55cbc1b9d6f210298a86502a7f20f9736c7e963e" dependencies = [ "bstr", "gix-actor", @@ -2657,8 +2668,7 @@ dependencies = [ [[package]] name = "gix-odb" version = "0.61.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e92b9790e2c919166865d0825b26cc440a387c175bed1b43a2fa99c0e9d45e98" +source = "git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e#55cbc1b9d6f210298a86502a7f20f9736c7e963e" dependencies = [ "arc-swap", "gix-date", @@ -2667,7 +2677,7 @@ dependencies = [ "gix-hash", "gix-object", "gix-pack", - "gix-path", + "gix-path 0.10.8 (git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e)", "gix-quote", "parking_lot 0.12.3", "tempfile", @@ -2677,8 +2687,7 @@ dependencies = [ [[package]] name = "gix-pack" version = "0.51.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7a8da51212dbff944713edb2141ed7e002eea326b8992070374ce13a6cb610b3" +source = "git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e#55cbc1b9d6f210298a86502a7f20f9736c7e963e" dependencies = [ "clru", "gix-chunk", @@ -2686,10 +2695,8 @@ dependencies = [ "gix-hash", "gix-hashtable", "gix-object", - "gix-path", - "gix-tempfile", + "gix-path 0.10.8 (git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e)", "memmap2", - "parking_lot 0.12.3", "smallvec", "thiserror", ] @@ -2697,12 +2704,11 @@ dependencies = [ [[package]] name = "gix-packetline-blocking" version = "0.17.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c31d42378a3d284732e4d589979930d0d253360eccf7ec7a80332e5ccb77e14a" +source = "git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e#55cbc1b9d6f210298a86502a7f20f9736c7e963e" dependencies = [ "bstr", "faster-hex", - "gix-trace", + "gix-trace 0.1.9 (git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e)", "thiserror", ] @@ -2713,7 +2719,19 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ca987128ffb056d732bd545db5db3d8b103d252fbf083c2567bb0796876619a4" dependencies = [ "bstr", - "gix-trace", + "gix-trace 0.1.9 (registry+https://github.com/rust-lang/crates.io-index)", + "home", + "once_cell", + "thiserror", +] + +[[package]] +name = "gix-path" +version = "0.10.8" +source = "git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e#55cbc1b9d6f210298a86502a7f20f9736c7e963e" +dependencies = [ + "bstr", + "gix-trace 0.1.9 (git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e)", "home", "once_cell", "thiserror", @@ -2722,23 +2740,33 @@ dependencies = [ [[package]] name = "gix-pathspec" version = "0.7.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a76cab098dc10ba2d89f634f66bf196dea4d7db4bf10b75c7a9c201c55a2ee19" +source = "git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e#55cbc1b9d6f210298a86502a7f20f9736c7e963e" dependencies = [ "bitflags 2.5.0", "bstr", "gix-attributes", "gix-config-value", "gix-glob", - "gix-path", + "gix-path 0.10.8 (git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e)", + "thiserror", +] + +[[package]] +name = "gix-prompt" +version = "0.8.5" +source = "git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e#55cbc1b9d6f210298a86502a7f20f9736c7e963e" +dependencies = [ + "gix-command", + "gix-config-value", + "parking_lot 0.12.3", + "rustix 0.38.32", "thiserror", ] [[package]] name = "gix-quote" version = "0.4.12" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cbff4f9b9ea3fa7a25a70ee62f545143abef624ac6aa5884344e70c8b0a1d9ff" +source = "git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e#55cbc1b9d6f210298a86502a7f20f9736c7e963e" dependencies = [ "bstr", "gix-utils", @@ -2748,17 +2776,15 @@ dependencies = [ [[package]] name = "gix-ref" version = "0.44.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3394a2997e5bc6b22ebc1e1a87b41eeefbcfcff3dbfa7c4bd73cb0ac8f1f3e2e" +source = "git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e#55cbc1b9d6f210298a86502a7f20f9736c7e963e" dependencies = [ "gix-actor", - "gix-date", "gix-features", "gix-fs", "gix-hash", "gix-lock", "gix-object", - "gix-path", + "gix-path 0.10.8 (git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e)", "gix-tempfile", "gix-utils", "gix-validate", @@ -2770,8 +2796,7 @@ dependencies = [ [[package]] name = "gix-refspec" version = "0.23.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "dde848865834a54fe4d9b4573f15d0e9a68eaf3d061b42d3ed52b4b8acf880b2" +source = "git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e#55cbc1b9d6f210298a86502a7f20f9736c7e963e" dependencies = [ "bstr", "gix-hash", @@ -2784,24 +2809,20 @@ dependencies = [ [[package]] name = "gix-revision" version = "0.27.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "63e08f8107ed1f93a83bcfbb4c38084c7cb3f6cd849793f1d5eec235f9b13b2b" +source = "git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e#55cbc1b9d6f210298a86502a7f20f9736c7e963e" dependencies = [ "bstr", "gix-date", "gix-hash", - "gix-hashtable", "gix-object", "gix-revwalk", - "gix-trace", "thiserror", ] [[package]] name = "gix-revwalk" version = "0.13.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4181db9cfcd6d1d0fd258e91569dbb61f94cb788b441b5294dd7f1167a3e788f" +source = "git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e#55cbc1b9d6f210298a86502a7f20f9736c7e963e" dependencies = [ "gix-commitgraph", "gix-date", @@ -2815,11 +2836,10 @@ dependencies = [ [[package]] name = "gix-sec" version = "0.10.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fddc27984a643b20dd03e97790555804f98cf07404e0e552c0ad8133266a79a1" +source = "git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e#55cbc1b9d6f210298a86502a7f20f9736c7e963e" dependencies = [ "bitflags 2.5.0", - "gix-path", + "gix-path 0.10.8 (git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e)", "libc", "windows-sys 0.52.0", ] @@ -2827,12 +2847,11 @@ dependencies = [ [[package]] name = "gix-submodule" version = "0.11.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "921cd49924ac14b6611b22e5fb7bbba74d8780dc7ad26153304b64d1272460ac" +source = "git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e#55cbc1b9d6f210298a86502a7f20f9736c7e963e" dependencies = [ "bstr", "gix-config", - "gix-path", + "gix-path 0.10.8 (git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e)", "gix-pathspec", "gix-refspec", "gix-url", @@ -2842,8 +2861,7 @@ dependencies = [ [[package]] name = "gix-tempfile" version = "14.0.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d3b0e276cd08eb2a22e9f286a4f13a222a01be2defafa8621367515375644b99" +source = "git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e#55cbc1b9d6f210298a86502a7f20f9736c7e963e" dependencies = [ "gix-fs", "libc", @@ -2858,11 +2876,15 @@ version = "0.1.9" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f924267408915fddcd558e3f37295cc7d6a3e50f8bd8b606cee0808c3915157e" +[[package]] +name = "gix-trace" +version = "0.1.9" +source = "git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e#55cbc1b9d6f210298a86502a7f20f9736c7e963e" + [[package]] name = "gix-traverse" version = "0.39.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f20cb69b63eb3e4827939f42c05b7756e3488ef49c25c412a876691d568ee2a0" +source = "git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e#55cbc1b9d6f210298a86502a7f20f9736c7e963e" dependencies = [ "bitflags 2.5.0", "gix-commitgraph", @@ -2878,12 +2900,11 @@ dependencies = [ [[package]] name = "gix-url" version = "0.27.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0db829ebdca6180fbe32be7aed393591df6db4a72dbbc0b8369162390954d1cf" +source = "git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e#55cbc1b9d6f210298a86502a7f20f9736c7e963e" dependencies = [ "bstr", "gix-features", - "gix-path", + "gix-path 0.10.8 (git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e)", "home", "thiserror", "url", @@ -2892,8 +2913,7 @@ dependencies = [ [[package]] name = "gix-utils" version = "0.1.12" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "35192df7fd0fa112263bad8021e2df7167df4cc2a6e6d15892e1e55621d3d4dc" +source = "git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e#55cbc1b9d6f210298a86502a7f20f9736c7e963e" dependencies = [ "bstr", "fastrand 2.1.0", @@ -2903,8 +2923,7 @@ dependencies = [ [[package]] name = "gix-validate" version = "0.8.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "82c27dd34a49b1addf193c92070bcbf3beaf6e10f16a78544de6372e146a0acf" +source = "git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e#55cbc1b9d6f210298a86502a7f20f9736c7e963e" dependencies = [ "bstr", "thiserror", @@ -2913,8 +2932,7 @@ dependencies = [ [[package]] name = "gix-worktree" version = "0.34.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "53f6b7de83839274022aff92157d7505f23debf739d257984a300a35972ca94e" +source = "git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e#55cbc1b9d6f210298a86502a7f20f9736c7e963e" dependencies = [ "bstr", "gix-attributes", @@ -2925,7 +2943,7 @@ dependencies = [ "gix-ignore", "gix-index", "gix-object", - "gix-path", + "gix-path 0.10.8 (git+https://github.com/Byron/gitoxide?rev=55cbc1b9d6f210298a86502a7f20f9736c7e963e)", "gix-validate", ] @@ -7146,9 +7164,9 @@ checksum = "d4c87d22b6e3f4a18d4d40ef354e97c90fcb14dd91d7dc0aa9d8a1172ebf7202" [[package]] name = "url" -version = "2.5.0" +version = "2.5.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "31e6302e3bb753d46e83516cae55ae196fc0c309407cf11ab35cc51a4c2a4633" +checksum = "22784dbdf76fdde8af1aeda5622b546b422b6fc585325248a2bf9f5e41e94d6c" dependencies = [ "form_urlencoded", "idna", diff --git a/Cargo.toml b/Cargo.toml index e0b6f3c68..001ffb302 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -11,7 +11,8 @@ members = [ resolver = "2" [workspace.dependencies] -gix = { version = "0.63.0", default-features = false, features = [] } # add performance features here as needed +# Add the `tracing` or `tracing-detail` features to see more of gitoxide in the logs. Useful to see which programs it invokes. +gix = { git = "https://github.com/Byron/gitoxide", rev = "55cbc1b9d6f210298a86502a7f20f9736c7e963e", default-features = false, features = [] } git2 = { version = "0.18.3", features = ["vendored-openssl", "vendored-libgit2"] } uuid = { version = "1.8.0", features = ["serde"] } serde = { version = "1.0", features = ["derive"] } diff --git a/crates/gitbutler-core/Cargo.toml b/crates/gitbutler-core/Cargo.toml index 4f397e0b9..8e2d25527 100644 --- a/crates/gitbutler-core/Cargo.toml +++ b/crates/gitbutler-core/Cargo.toml @@ -30,7 +30,7 @@ fslock = "0.2.1" futures = "0.3" git2.workspace = true git2-hooks = "0.3" -gix = { workspace = true, features = ["dirwalk"] } +gix = { workspace = true, features = ["dirwalk", "credentials", "parallel"] } itertools = "0.13" keyring.workspace = true lazy_static = "1.4.0" @@ -51,7 +51,7 @@ tempfile = "3.10" thiserror.workspace = true tokio = { workspace = true, features = [ "rt-multi-thread", "rt", "macros" ] } tracing = "0.1.40" -url = { version = "2.5", features = ["serde"] } +url = { version = "2.5.2", features = ["serde"] } urlencoding = "2.1.3" uuid.workspace = true walkdir = "2.5.0" diff --git a/crates/gitbutler-core/src/secret.rs b/crates/gitbutler-core/src/secret.rs index e34c3ab51..2519ef695 100644 --- a/crates/gitbutler-core/src/secret.rs +++ b/crates/gitbutler-core/src/secret.rs @@ -52,3 +52,168 @@ fn entry_for(handle: &str) -> Result { /// How to further specialize secrets to avoid name clashes in the globally shared keystore. static NAMESPACE: Mutex = Mutex::new(String::new()); + +/// A keystore that uses git-credentials under to hood. It's useful on Systems that nag the user +/// with popups if the underlying binary changes, and is available if `git` can be found and executed. +pub mod git_credentials { + use anyhow::Result; + use keyring::credential::{CredentialApi, CredentialBuilderApi, CredentialPersistence}; + use keyring::Credential; + use std::any::Any; + use std::sync::Arc; + use tracing::instrument; + + pub(super) struct Store(gix::config::File<'static>); + + impl Store { + /// Create an instance by resolving the global environment just well enough. + /// + /// # Limitation + /// + /// This does not fully resolve includes, so it's not truly production ready but should be + /// fine for developer setups. + fn from_globals() -> Result { + Ok(Store(gix::config::File::from_globals()?)) + } + + /// Provide credentials preconfigured for the given secrets `handle`. + /// They can then be queried. + fn credentials( + &self, + handle: &str, + password: Option<&str>, + ) -> Result<( + gix::credentials::helper::Cascade, + gix::credentials::helper::Action, + gix::prompt::Options<'static>, + )> { + let url = gix::Url::from_parts( + gix::url::Scheme::Https, + Some("gitbutler-secrets".into()), + password.map(ToOwned::to_owned), + Some("gitbutler.com".into()), + None, + format!("/{handle}").into(), + false, + )?; + gix::config::credential_helpers( + url, + &self.0, + true, + &mut gix::config::section::is_trusted, + gix::open::permissions::Environment::isolated(), + true, /* use http path by default */ + ) + .map(|mut t| { + let ctx = t.1.context_mut().expect("get always has context"); + // Assure the context has fields for all parts in the URL, even + // if later we choose to use store or erase actions. + // Usually these are naturally populated, + // but not if we do everything by hand. + // This is not a shortcoming in `gitoxide` - it simply doesn't touch + // the output of previous invocations to not unintentionally affect them. + ctx.destructure_url_in_place(true /* use http path */) + .expect("input URL is valid"); + t.2.mode = gix::prompt::Mode::Disable; + t + }) + .map_err(Into::into) + } + } + + pub(super) type SharedStore = Arc; + + struct Entry { + handle: String, + store: SharedStore, + } + + impl CredentialApi for Entry { + #[instrument(skip(self, password), err(Debug))] + fn set_password(&self, password: &str) -> keyring::Result<()> { + // credential helper on macos can't overwrite existing values apparently, workaround that. + #[cfg(target_os = "macos")] + self.delete_password().ok(); + let (mut cascade, action, prompt) = self + .store + .credentials(&self.handle, Some(password)) + .map_err(|err| keyring::Error::PlatformFailure(err.into()))?; + let ctx = action.context().expect("available for get").to_owned(); + let action = gix::credentials::helper::NextAction::from(ctx).store(); + cascade + .invoke(action, prompt) + .map_err(|err| keyring::Error::PlatformFailure(err.into()))?; + Ok(()) + } + + #[instrument(skip(self), err(Debug))] + fn get_password(&self) -> keyring::Result { + let (mut cascade, get_action, prompt) = self + .store + .credentials(&self.handle, None) + .map_err(|err| keyring::Error::PlatformFailure(err.into()))?; + match cascade.invoke(get_action, prompt) { + Ok(Some(out)) => Ok(out.identity.password), + Ok(None) => Err(keyring::Error::NoEntry), + Err(err) => { + tracing::debug!(err = ?err, "credential-helper invoke failed - usually this means it wanted to prompt which is disabled"); + Err(keyring::Error::NoEntry) + } + } + } + + #[instrument(skip(self), err(Debug))] + fn delete_password(&self) -> keyring::Result<()> { + let (mut cascade, action, prompt) = self + .store + .credentials(&self.handle, None) + .map_err(|err| keyring::Error::PlatformFailure(err.into()))?; + let ctx = action.context().expect("available for get").to_owned(); + let action = gix::credentials::helper::NextAction::from(ctx).erase(); + cascade + .invoke(action, prompt) + .map_err(|err| keyring::Error::PlatformFailure(err.into()))?; + Ok(()) + } + + fn as_any(&self) -> &dyn Any { + self + } + } + + pub(super) struct Builder { + pub(super) store: SharedStore, + } + + impl CredentialBuilderApi for Builder { + fn build( + &self, + _target: Option<&str>, + service: &str, + _user: &str, + ) -> keyring::Result> { + let credential = Entry { + handle: service.to_string(), + store: self.store.clone(), + }; + Ok(Box::new(credential)) + } + + fn as_any(&self) -> &dyn std::any::Any { + self + } + + /// We keep information in memory + fn persistence(&self) -> CredentialPersistence { + CredentialPersistence::UntilReboot + } + } + + /// Initialize the credentials store so that secrets are using `git credential`. + #[instrument(err(Debug))] + pub fn setup() -> Result<()> { + let store = Arc::new(Store::from_globals()?); + keyring::set_default_credential_builder(Box::new(Builder { store })); + Ok(()) + } +} diff --git a/crates/gitbutler-tauri/src/main.rs b/crates/gitbutler-tauri/src/main.rs index 92fabb31e..4fa285615 100644 --- a/crates/gitbutler-tauri/src/main.rs +++ b/crates/gitbutler-tauri/src/main.rs @@ -69,6 +69,14 @@ fn main() { logs::init(&app_handle); + // On MacOS, in dev mode with debug assertions, we encounter popups each time + // the binary is rebuilt. To counter that, use a git-credential based implementation. + // This isn't an issue for actual release build (i.e. nightly, production), + // hence the specific condition. + if cfg!(debug_assertions) && cfg!(target_os = "macos") { + gitbutler_core::secret::git_credentials::setup().ok(); + } + // SAFETY(qix-): This is safe because we're initializing the askpass broker here, // SAFETY(qix-): before any other threads would ever access it. unsafe { From de00e4f049e5c5afe73c4c6cd41c04f4c661024b Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Fri, 28 Jun 2024 20:31:32 +0200 Subject: [PATCH 10/13] support global secrets (without namespace for build types) and provide commands in `tauri` crate That way globals secrets can be used from the UI directly. --- crates/gitbutler-core/src/secret.rs | 42 ++++++++++++----- crates/gitbutler-core/src/users/controller.rs | 15 +++--- crates/gitbutler-core/src/users/user.rs | 9 +++- crates/gitbutler-core/tests/secret/mod.rs | 46 +++++++++++-------- crates/gitbutler-tauri/src/lib.rs | 2 + crates/gitbutler-tauri/src/main.rs | 6 ++- crates/gitbutler-tauri/src/secret.rs | 20 ++++++++ 7 files changed, 100 insertions(+), 40 deletions(-) create mode 100644 crates/gitbutler-tauri/src/secret.rs diff --git a/crates/gitbutler-core/src/secret.rs b/crates/gitbutler-core/src/secret.rs index 2519ef695..f46622be9 100644 --- a/crates/gitbutler-core/src/secret.rs +++ b/crates/gitbutler-core/src/secret.rs @@ -2,13 +2,28 @@ //! //! These are stateless and global, while discouraging storing secrets //! in memory beyond their use. + use crate::types::Sensitive; use anyhow::Result; use std::sync::Mutex; -/// Persist `secret` so that it can be retrieved by the given `handle`. -pub fn persist(handle: &str, secret: &Sensitive) -> Result<()> { - let entry = entry_for(handle)?; +/// Determines how a secret's name should be modified to produce a namespace. +/// +/// Namespaces can be used to partition secrets, depending on some criteria. +#[derive(Debug, Clone, Copy)] +pub enum Namespace { + /// Each application build, like `dev`, `production` and `nightly` have their + /// own set of secrets. They do not overlap, which reflects how data-files + /// are stored as well. + BuildKind, + /// All secrets are in a single namespace. There is no partitioning. + /// This can be useful for secrets to be shared across all build kinds. + Global, +} + +/// Persist `secret` in `namespace` so that it can be retrieved by the given `handle`. +pub fn persist(handle: &str, secret: &Sensitive, namespace: Namespace) -> Result<()> { + let entry = entry_for(handle, namespace)?; if secret.0.is_empty() { entry.delete_password()?; } else { @@ -17,30 +32,33 @@ pub fn persist(handle: &str, secret: &Sensitive) -> Result<()> { Ok(()) } -/// Obtain the previously [stored](persist()) secret known as `handle`. -pub fn retrieve(handle: &str) -> Result>> { - match entry_for(handle)?.get_password() { +/// Obtain the previously [stored](persist()) secret known as `handle` from `namespace`. +pub fn retrieve(handle: &str, namespace: Namespace) -> Result>> { + match entry_for(handle, namespace)?.get_password() { Ok(secret) => Ok(Some(Sensitive(secret))), Err(keyring::Error::NoEntry) => Ok(None), Err(err) => Err(err.into()), } } -/// Delete the secret at `handle` permanently. -pub fn delete(handle: &str) -> Result<()> { - Ok(entry_for(handle)?.delete_password()?) +/// Delete the secret at `handle` permanently from `namespace`. +pub fn delete(handle: &str, namespace: Namespace) -> Result<()> { + Ok(entry_for(handle, namespace)?.delete_password()?) } /// Use this `identifier` as 'namespace' for identifying secrets. /// Each namespace has its own set of secrets, useful for different application versions. /// -/// Note that the namespace will default to `gitbutler` if empty. +/// Note that the namespace will be `development` if `identifier` is empty (or wasn't set). pub fn set_application_namespace(identifier: impl Into) { *NAMESPACE.lock().unwrap() = identifier.into() } -fn entry_for(handle: &str) -> Result { - let ns = NAMESPACE.lock().unwrap(); +fn entry_for(handle: &str, namespace: Namespace) -> Result { + let ns = match namespace { + Namespace::BuildKind => NAMESPACE.lock().unwrap().clone(), + Namespace::Global => "gitbutler".into(), + }; Ok(keyring::Entry::new( &format!( "{prefix}-{handle}", diff --git a/crates/gitbutler-core/src/users/controller.rs b/crates/gitbutler-core/src/users/controller.rs index f8eb19463..7632a2431 100644 --- a/crates/gitbutler-core/src/users/controller.rs +++ b/crates/gitbutler-core/src/users/controller.rs @@ -1,9 +1,9 @@ +use super::{storage::Storage, User}; +use crate::secret; use anyhow::Context; use anyhow::Result; use std::path::PathBuf; -use super::{storage::Storage, User}; - /// 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. /// @@ -45,8 +45,9 @@ impl Controller { pub fn delete_user(&self) -> Result<()> { self.storage.delete().context("failed to delete user")?; - crate::secret::delete(User::ACCESS_TOKEN_HANDLE).ok(); - crate::secret::delete(User::GITHUB_ACCESS_TOKEN_HANDLE).ok(); + let namespace = secret::Namespace::BuildKind; + secret::delete(User::ACCESS_TOKEN_HANDLE, namespace).ok(); + secret::delete(User::GITHUB_ACCESS_TOKEN_HANDLE, namespace).ok(); Ok(()) } } @@ -54,11 +55,13 @@ impl Controller { /// 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 |= crate::secret::persist(User::ACCESS_TOKEN_HANDLE, &gb_token).is_ok(); + 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 |= crate::secret::persist(User::GITHUB_ACCESS_TOKEN_HANDLE, &gh_token).is_ok(); + needs_write |= + secret::persist(User::GITHUB_ACCESS_TOKEN_HANDLE, &gh_token, namespace).is_ok(); } if needs_write { storage.set(&user)?; diff --git a/crates/gitbutler-core/src/users/user.rs b/crates/gitbutler-core/src/users/user.rs index f56461829..0172b4b6c 100644 --- a/crates/gitbutler-core/src/users/user.rs +++ b/crates/gitbutler-core/src/users/user.rs @@ -1,3 +1,4 @@ +use crate::secret; use crate::types::Sensitive; use anyhow::{Context, Result}; use serde::{Deserialize, Serialize}; @@ -39,7 +40,8 @@ impl User { return Ok(token.clone()); } let err_msg = "access token for user was deleted from keychain - login is now invalid"; - let secret = crate::secret::retrieve(Self::ACCESS_TOKEN_HANDLE)?.context(err_msg)?; + let secret = secret::retrieve(Self::ACCESS_TOKEN_HANDLE, secret::Namespace::BuildKind)? + .context(err_msg)?; *self.access_token.borrow_mut() = Some(secret.clone()); Ok(secret) } @@ -51,7 +53,10 @@ impl User { 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)?; + 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/secret/mod.rs b/crates/gitbutler-core/tests/secret/mod.rs index ed35783cc..7e1a5480f 100644 --- a/crates/gitbutler-core/tests/secret/mod.rs +++ b/crates/gitbutler-core/tests/secret/mod.rs @@ -9,22 +9,26 @@ use serial_test::serial; #[serial] fn retrieve_unknown_is_none() { credentials::setup(); - assert!(secret::retrieve("does not exist for sure") - .expect("no error to ask for non-existing") - .is_none()); + for ns in all_namespaces() { + assert!(secret::retrieve("does not exist for sure", *ns) + .expect("no error to ask for non-existing") + .is_none()); + } } #[test] #[serial] fn store_and_retrieve() -> anyhow::Result<()> { credentials::setup(); - secret::persist("new", &Sensitive("secret".into()))?; - let secret = secret::retrieve("new")?.expect("it was just stored"); - assert_eq!( - secret.0, "secret", - "note that this works only if the engine supports actual persistence, \ + for ns in all_namespaces() { + secret::persist("new", &Sensitive("secret".into()), *ns)?; + let secret = secret::retrieve("new", *ns)?.expect("it was just stored"); + assert_eq!( + secret.0, "secret", + "note that this works only if the engine supports actual persistence, \ which should be the default outside of tests" - ); + ); + } Ok(()) } @@ -32,18 +36,24 @@ fn store_and_retrieve() -> anyhow::Result<()> { #[serial] fn store_empty_equals_deletion() -> anyhow::Result<()> { credentials::setup(); - secret::persist("new", &Sensitive("secret".into()))?; - assert_eq!(credentials::count(), 1); + for ns in all_namespaces() { + secret::persist("new", &Sensitive("secret".into()), *ns)?; + assert_eq!(credentials::count(), 1); - secret::persist("new", &Sensitive("".into()))?; - assert_eq!( - secret::retrieve("new")?.map(|s| s.0), - None, - "empty passwords are automatically deleted" - ); - assert_eq!(credentials::count(), 0); + secret::persist("new", &Sensitive("".into()), *ns)?; + assert_eq!( + secret::retrieve("new", *ns)?.map(|s| s.0), + None, + "empty passwords are automatically deleted" + ); + assert_eq!(credentials::count(), 0); + } Ok(()) } +fn all_namespaces() -> &'static [secret::Namespace] { + &[secret::Namespace::Global, secret::Namespace::BuildKind] +} + pub(crate) mod credentials; mod users; diff --git a/crates/gitbutler-tauri/src/lib.rs b/crates/gitbutler-tauri/src/lib.rs index 6b0034b3a..551357bea 100644 --- a/crates/gitbutler-tauri/src/lib.rs +++ b/crates/gitbutler-tauri/src/lib.rs @@ -30,4 +30,6 @@ pub mod remotes; pub mod undo; pub mod users; pub mod virtual_branches; +pub mod secret; + pub mod zip; diff --git a/crates/gitbutler-tauri/src/main.rs b/crates/gitbutler-tauri/src/main.rs index 4fa285615..10793d0dc 100644 --- a/crates/gitbutler-tauri/src/main.rs +++ b/crates/gitbutler-tauri/src/main.rs @@ -15,8 +15,8 @@ use gitbutler_core::{assets, git, storage}; use gitbutler_tauri::{ - app, askpass, commands, config, github, keys, logs, menu, projects, remotes, undo, users, - virtual_branches, watcher, zip, + app, askpass, commands, config, github, keys, logs, menu, projects, remotes, secret, undo, + users, virtual_branches, watcher, zip, }; use tauri::{generate_context, Manager}; use tauri_plugin_log::LogTarget; @@ -215,6 +215,8 @@ fn main() { virtual_branches::commands::squash_branch_commit, virtual_branches::commands::fetch_from_remotes, virtual_branches::commands::move_commit, + secret::secret_get_global, + secret::secret_set_global, undo::list_snapshots, undo::restore_snapshot, undo::snapshot_diff, diff --git a/crates/gitbutler-tauri/src/secret.rs b/crates/gitbutler-tauri/src/secret.rs new file mode 100644 index 000000000..020be05df --- /dev/null +++ b/crates/gitbutler-tauri/src/secret.rs @@ -0,0 +1,20 @@ +use crate::error::Error; +use gitbutler_core::secret; +use gitbutler_core::types::Sensitive; +use tracing::instrument; + +#[tauri::command(async)] +#[instrument(err(Debug))] +pub async fn secret_get_global(handle: &str) -> Result, Error> { + Ok(secret::retrieve(handle, secret::Namespace::Global)?.map(|s| s.0)) +} + +#[tauri::command(async)] +#[instrument(skip(secret), err(Debug), fields(secret = ""))] +pub async fn secret_set_global(handle: &str, secret: String) -> Result<(), Error> { + Ok(secret::persist( + handle, + &Sensitive(secret), + secret::Namespace::Global, + )?) +} From 05506f49faf5da231ba111ef4a0a654dedee7e6e Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Mon, 24 Jun 2024 17:34:37 +0200 Subject: [PATCH 11/13] migrate AI tokens from the git-configuration to the keystore. All AI related options are stored in the user-level git configuration file. Upon first access, they will be removed from there and placed into the keystore as part of the migration. The UI is provided with functions to store and save secrets which it will use specifically to interact with these keys. It's explicitly out of scope to *not* show the keys in plain-text anymore after entering them. --- app/src/lib/ai/service.ts | 25 +++++++++++++++++++++++-- app/src/lib/backend/gitConfigService.ts | 4 ++++ app/src/routes/settings/ai/+page.svelte | 12 ++++++++++-- crates/gitbutler-tauri/src/app.rs | 5 +++++ crates/gitbutler-tauri/src/commands.rs | 6 ++++++ crates/gitbutler-tauri/src/lib.rs | 2 +- crates/gitbutler-tauri/src/main.rs | 1 + 7 files changed, 50 insertions(+), 5 deletions(-) diff --git a/app/src/lib/ai/service.ts b/app/src/lib/ai/service.ts index 30554ba0b..6c3d1b2a1 100644 --- a/app/src/lib/ai/service.ts +++ b/app/src/lib/ai/service.ts @@ -14,6 +14,7 @@ import { MessageRole, type Prompt } from '$lib/ai/types'; +import { invoke } from '$lib/backend/ipc'; import { buildFailureFromAny, isFailure, ok, type Result } from '$lib/result'; import { splitMessage } from '$lib/utils/commitMessage'; import OpenAI from 'openai'; @@ -90,7 +91,17 @@ export class AIService { } async getOpenAIKey() { - return await this.gitConfig.get(GitAIConfigKey.OpenAIKey); + const secretInConfig = await this.gitConfig.get(GitAIConfigKey.OpenAIKey); + if (secretInConfig !== undefined) { + await invoke('secret_set_global', { + handle: 'aiOpenAIKey', + secret: secretInConfig + }); + await this.gitConfig.remove(GitAIConfigKey.OpenAIKey); + return secretInConfig; + } else { + return await invoke('secret_get_global', { handle: 'aiOpenAIKey' }); + } } async getOpenAIModleName() { @@ -108,7 +119,17 @@ export class AIService { } async getAnthropicKey() { - return await this.gitConfig.get(GitAIConfigKey.AnthropicKey); + const secretInConfig = await this.gitConfig.get(GitAIConfigKey.AnthropicKey); + if (secretInConfig !== undefined) { + await invoke('secret_set_global', { + handle: 'aiAnthropicKey', + secret: secretInConfig + }); + await this.gitConfig.remove(GitAIConfigKey.AnthropicKey); + return secretInConfig; + } else { + return await invoke('secret_get_global', { handle: 'aiAnthropicKey' }); + } } async getAnthropicModelName() { diff --git a/app/src/lib/backend/gitConfigService.ts b/app/src/lib/backend/gitConfigService.ts index 9e1843a1e..2197d132a 100644 --- a/app/src/lib/backend/gitConfigService.ts +++ b/app/src/lib/backend/gitConfigService.ts @@ -5,6 +5,10 @@ export class GitConfigService { return (await invoke('git_get_global_config', { key })) || undefined; } + async remove(key: string): Promise { + return await invoke('git_remove_global_config', { key }); + } + async getWithDefault(key: string, defaultValue: T): Promise { const value = await invoke('git_get_global_config', { key }); return value || defaultValue; diff --git a/app/src/routes/settings/ai/+page.svelte b/app/src/routes/settings/ai/+page.svelte index ebe028904..ed8cf3428 100644 --- a/app/src/routes/settings/ai/+page.svelte +++ b/app/src/routes/settings/ai/+page.svelte @@ -15,6 +15,7 @@ import TextBox from '$lib/shared/TextBox.svelte'; import { UserService } from '$lib/stores/user'; import { getContext } from '$lib/utils/context'; + import { invoke } from '@tauri-apps/api/tauri'; import { onMount, tick } from 'svelte'; const gitConfigService = getContext(GitConfigService); @@ -34,10 +35,17 @@ let ollamaEndpoint: string | undefined; let ollamaModel: string | undefined; - function setConfiguration(key: GitAIConfigKey, value: string | undefined) { + async function setConfiguration(key: GitAIConfigKey, value: string | undefined) { if (!initialized) return; - gitConfigService.set(key, value || ''); + if (key === GitAIConfigKey.OpenAIKey || key === GitAIConfigKey.AnthropicKey) { + await invoke('secret_set_global', { + handle: key.split('.')[1], + secret: value + }); + } else { + gitConfigService.set(key, value || ''); + } } $: setConfiguration(GitAIConfigKey.ModelProvider, modelKind); diff --git a/crates/gitbutler-tauri/src/app.rs b/crates/gitbutler-tauri/src/app.rs index 09121a091..a5390b6f9 100644 --- a/crates/gitbutler-tauri/src/app.rs +++ b/crates/gitbutler-tauri/src/app.rs @@ -79,6 +79,11 @@ impl App { Ok(value.to_string()) } + pub fn git_remove_global_config(key: &str) -> Result<()> { + let mut config = git2::Config::open_default()?; + Ok(config.remove(key)?) + } + pub fn git_get_global_config(key: &str) -> Result> { let config = git2::Config::open_default()?; let value = config.get_string(key); diff --git a/crates/gitbutler-tauri/src/commands.rs b/crates/gitbutler-tauri/src/commands.rs index 85157fc2b..4036c2dce 100644 --- a/crates/gitbutler-tauri/src/commands.rs +++ b/crates/gitbutler-tauri/src/commands.rs @@ -103,6 +103,12 @@ pub async fn git_set_global_config( Ok(result) } +#[tauri::command(async)] +#[instrument(err(Debug))] +pub async fn git_remove_global_config(key: &str) -> Result<(), Error> { + Ok(app::App::git_remove_global_config(key)?) +} + #[tauri::command(async)] #[instrument(skip(_handle), err(Debug))] pub async fn git_get_global_config( diff --git a/crates/gitbutler-tauri/src/lib.rs b/crates/gitbutler-tauri/src/lib.rs index 551357bea..9f92d795d 100644 --- a/crates/gitbutler-tauri/src/lib.rs +++ b/crates/gitbutler-tauri/src/lib.rs @@ -27,9 +27,9 @@ pub mod github; pub mod keys; pub mod projects; pub mod remotes; +pub mod secret; pub mod undo; pub mod users; pub mod virtual_branches; -pub mod secret; pub mod zip; diff --git a/crates/gitbutler-tauri/src/main.rs b/crates/gitbutler-tauri/src/main.rs index 10793d0dc..b5866dae6 100644 --- a/crates/gitbutler-tauri/src/main.rs +++ b/crates/gitbutler-tauri/src/main.rs @@ -168,6 +168,7 @@ fn main() { commands::delete_all_data, commands::mark_resolved, commands::git_set_global_config, + commands::git_remove_global_config, commands::git_get_global_config, commands::git_test_push, commands::git_test_fetch, From 7f618fd248993c16a5a37948b16943ecee7576e1 Mon Sep 17 00:00:00 2001 From: Mattias Granlund Date: Mon, 1 Jul 2024 16:09:01 +0200 Subject: [PATCH 12/13] Extract separate service for secrets - add `buildContext` for getting/setting contexts by types - config -> secret migration attempted if secret not found --- app/src/lib/ai/service.test.ts | 57 ++++++++++++++++++------- app/src/lib/ai/service.ts | 34 ++++----------- app/src/lib/secrets/secretsService.ts | 57 +++++++++++++++++++++++++ app/src/lib/utils/context.ts | 12 ++++++ app/src/routes/+layout.ts | 4 +- app/src/routes/settings/ai/+page.svelte | 22 +++++----- 6 files changed, 132 insertions(+), 54 deletions(-) create mode 100644 app/src/lib/secrets/secretsService.ts diff --git a/app/src/lib/ai/service.test.ts b/app/src/lib/ai/service.test.ts index 6fb58f0f7..82e9d7a72 100644 --- a/app/src/lib/ai/service.test.ts +++ b/app/src/lib/ai/service.test.ts @@ -2,7 +2,7 @@ import { AnthropicAIClient } from '$lib/ai/anthropicClient'; import { ButlerAIClient } from '$lib/ai/butlerClient'; import { OpenAIClient } from '$lib/ai/openAIClient'; import { SHORT_DEFAULT_BRANCH_TEMPLATE, SHORT_DEFAULT_COMMIT_TEMPLATE } from '$lib/ai/prompts'; -import { AIService, GitAIConfigKey, KeyOption, buildDiff } from '$lib/ai/service'; +import { AISecretHandle, AIService, GitAIConfigKey, KeyOption, buildDiff } from '$lib/ai/service'; import { AnthropicModelName, ModelKind, @@ -16,17 +16,21 @@ import { Hunk } from '$lib/vbranches/types'; import { plainToInstance } from 'class-transformer'; import { expect, test, describe, vi } from 'vitest'; import type { GbConfig, GitConfigService } from '$lib/backend/gitConfigService'; +import type { SecretsService } from '$lib/secrets/secretsService'; const defaultGitConfig = Object.freeze({ [GitAIConfigKey.ModelProvider]: ModelKind.OpenAI, [GitAIConfigKey.OpenAIKeyOption]: KeyOption.ButlerAPI, - [GitAIConfigKey.OpenAIKey]: undefined, [GitAIConfigKey.OpenAIModelName]: OpenAIModelName.GPT35Turbo, [GitAIConfigKey.AnthropicKeyOption]: KeyOption.ButlerAPI, - [GitAIConfigKey.AnthropicKey]: undefined, [GitAIConfigKey.AnthropicModelName]: AnthropicModelName.Haiku }); +const defaultSecretsConfig = Object.freeze({ + [AISecretHandle.AnthropicKey]: undefined, + [AISecretHandle.OpenAIKey]: undefined +}); + class DummyGitConfigService implements GitConfigService { constructor(private config: { [index: string]: string | undefined }) {} async getGbConfig(_projectId: string): Promise { @@ -46,6 +50,24 @@ class DummyGitConfigService implements GitConfigService { async set(key: string, value: T): Promise { return (this.config[key] = value); } + async remove(key: string): Promise { + delete this.config[key]; + } +} + +class DummySecretsService implements SecretsService { + private config: { [index: string]: string | undefined }; + constructor(config?: { [index: string]: string | undefined }) { + this.config = config || {}; + } + + async get(key: string): Promise { + return this.config[key]; + } + + async set(handle: string, secret: string): Promise { + this.config[handle] = secret; + } } const fetchMock = vi.fn(); @@ -108,7 +130,8 @@ const exampleHunks = [hunk1, hunk2]; function buildDefaultAIService() { const gitConfig = new DummyGitConfigService(structuredClone(defaultGitConfig)); - return new AIService(gitConfig, cloud); + const secretsService = new DummySecretsService(structuredClone(defaultSecretsConfig)); + return new AIService(gitConfig, secretsService, cloud); } describe.concurrent('AIService', () => { @@ -130,10 +153,10 @@ describe.concurrent('AIService', () => { test('When token is bring your own, When a openAI token is present. It returns OpenAIClient', async () => { const gitConfig = new DummyGitConfigService({ ...defaultGitConfig, - [GitAIConfigKey.OpenAIKeyOption]: KeyOption.BringYourOwn, - [GitAIConfigKey.OpenAIKey]: 'sk-asdfasdf' + [GitAIConfigKey.OpenAIKeyOption]: KeyOption.BringYourOwn }); - const aiService = new AIService(gitConfig, cloud); + const secretsService = new DummySecretsService({ [AISecretHandle.OpenAIKey]: 'sk-asdfasdf' }); + const aiService = new AIService(gitConfig, secretsService, cloud); expect(unwrap(await aiService.buildClient())).toBeInstanceOf(OpenAIClient); }); @@ -141,10 +164,10 @@ describe.concurrent('AIService', () => { test('When token is bring your own, When a openAI token is blank. It returns undefined', async () => { const gitConfig = new DummyGitConfigService({ ...defaultGitConfig, - [GitAIConfigKey.OpenAIKeyOption]: KeyOption.BringYourOwn, - [GitAIConfigKey.OpenAIKey]: undefined + [GitAIConfigKey.OpenAIKeyOption]: KeyOption.BringYourOwn }); - const aiService = new AIService(gitConfig, cloud); + const secretsService = new DummySecretsService(); + const aiService = new AIService(gitConfig, secretsService, cloud); expect(await aiService.buildClient()).toStrictEqual( buildFailureFromAny( @@ -157,10 +180,12 @@ describe.concurrent('AIService', () => { const gitConfig = new DummyGitConfigService({ ...defaultGitConfig, [GitAIConfigKey.ModelProvider]: ModelKind.Anthropic, - [GitAIConfigKey.AnthropicKeyOption]: KeyOption.BringYourOwn, - [GitAIConfigKey.AnthropicKey]: 'sk-ant-api03-asdfasdf' + [GitAIConfigKey.AnthropicKeyOption]: KeyOption.BringYourOwn }); - const aiService = new AIService(gitConfig, cloud); + const secretsService = new DummySecretsService({ + [AISecretHandle.AnthropicKey]: 'test-key' + }); + const aiService = new AIService(gitConfig, secretsService, cloud); expect(unwrap(await aiService.buildClient())).toBeInstanceOf(AnthropicAIClient); }); @@ -169,10 +194,10 @@ describe.concurrent('AIService', () => { const gitConfig = new DummyGitConfigService({ ...defaultGitConfig, [GitAIConfigKey.ModelProvider]: ModelKind.Anthropic, - [GitAIConfigKey.AnthropicKeyOption]: KeyOption.BringYourOwn, - [GitAIConfigKey.AnthropicKey]: undefined + [GitAIConfigKey.AnthropicKeyOption]: KeyOption.BringYourOwn }); - const aiService = new AIService(gitConfig, cloud); + const secretsService = new DummySecretsService(); + const aiService = new AIService(gitConfig, secretsService, cloud); expect(await aiService.buildClient()).toStrictEqual( buildFailureFromAny( diff --git a/app/src/lib/ai/service.ts b/app/src/lib/ai/service.ts index 6c3d1b2a1..239d7a5a6 100644 --- a/app/src/lib/ai/service.ts +++ b/app/src/lib/ai/service.ts @@ -14,12 +14,12 @@ import { MessageRole, type Prompt } from '$lib/ai/types'; -import { invoke } from '$lib/backend/ipc'; import { buildFailureFromAny, isFailure, ok, type Result } from '$lib/result'; import { splitMessage } from '$lib/utils/commitMessage'; import OpenAI from 'openai'; import type { GitConfigService } from '$lib/backend/gitConfigService'; import type { HttpClient } from '$lib/backend/httpClient'; +import type { SecretsService } from '$lib/secrets/secretsService'; import type { Hunk } from '$lib/vbranches/types'; const maxDiffLengthLimitForAPI = 5000; @@ -29,14 +29,17 @@ export enum KeyOption { ButlerAPI = 'butlerAPI' } +export enum AISecretHandle { + OpenAIKey = 'aiOpenAIKey', + AnthropicKey = 'aiAnthropicKey' +} + export enum GitAIConfigKey { ModelProvider = 'gitbutler.aiModelProvider', OpenAIKeyOption = 'gitbutler.aiOpenAIKeyOption', OpenAIModelName = 'gitbutler.aiOpenAIModelName', - OpenAIKey = 'gitbutler.aiOpenAIKey', AnthropicKeyOption = 'gitbutler.aiAnthropicKeyOption', AnthropicModelName = 'gitbutler.aiAnthropicModelName', - AnthropicKey = 'gitbutler.aiAnthropicKey', DiffLengthLimit = 'gitbutler.diffLengthLimit', OllamaEndpoint = 'gitbutler.aiOllamaEndpoint', OllamaModelName = 'gitbutler.aiOllamaModelName' @@ -73,6 +76,7 @@ function shuffle(items: T[]): T[] { export class AIService { constructor( private gitConfig: GitConfigService, + private secretsService: SecretsService, private cloud: HttpClient ) {} @@ -91,17 +95,7 @@ export class AIService { } async getOpenAIKey() { - const secretInConfig = await this.gitConfig.get(GitAIConfigKey.OpenAIKey); - if (secretInConfig !== undefined) { - await invoke('secret_set_global', { - handle: 'aiOpenAIKey', - secret: secretInConfig - }); - await this.gitConfig.remove(GitAIConfigKey.OpenAIKey); - return secretInConfig; - } else { - return await invoke('secret_get_global', { handle: 'aiOpenAIKey' }); - } + return await this.secretsService.get(AISecretHandle.OpenAIKey); } async getOpenAIModleName() { @@ -119,17 +113,7 @@ export class AIService { } async getAnthropicKey() { - const secretInConfig = await this.gitConfig.get(GitAIConfigKey.AnthropicKey); - if (secretInConfig !== undefined) { - await invoke('secret_set_global', { - handle: 'aiAnthropicKey', - secret: secretInConfig - }); - await this.gitConfig.remove(GitAIConfigKey.AnthropicKey); - return secretInConfig; - } else { - return await invoke('secret_get_global', { handle: 'aiAnthropicKey' }); - } + return await this.secretsService.get(AISecretHandle.AnthropicKey); } async getAnthropicModelName() { diff --git a/app/src/lib/secrets/secretsService.ts b/app/src/lib/secrets/secretsService.ts new file mode 100644 index 000000000..f9ee2321a --- /dev/null +++ b/app/src/lib/secrets/secretsService.ts @@ -0,0 +1,57 @@ +import { AISecretHandle } from '$lib/ai/service'; +import { invoke } from '$lib/backend/ipc'; +import { buildContext } from '$lib/utils/context'; +import type { GitConfigService } from '$lib/backend/gitConfigService'; + +const MIGRATION_HANDLES = [ + AISecretHandle.AnthropicKey.toString(), + AISecretHandle.OpenAIKey.toString() +]; + +export type SecretsService = { + get(handle: string): Promise; + set(handle: string, secret: string): Promise; +}; + +export const [getSecretsService, setSecretsService] = + buildContext('secretsService'); + +export class RustSecretService implements SecretsService { + constructor(private gitConfigService: GitConfigService) {} + + async get(handle: string) { + console.warn('getting ', handle); + const secret = await invoke('secret_get_global', { handle }); + if (secret) return secret; + + console.warn('migrating', handle); + if (MIGRATION_HANDLES.includes(handle)) { + const key = 'gitbutler.' + handle; + const migratedSecret = await this.migrate(key, handle); + if (migratedSecret !== undefined) return migratedSecret; + } + } + + async set(handle: string, secret: string) { + await invoke('secret_set_global', { + handle, + secret + }); + } + + /** + * Migrates a specific key from git config to secrets. + * + * TODO: Remove this code and the dependency on GitConfigService in the future. + */ + private async migrate(key: string, handle: string): Promise { + const secretInConfig = await this.gitConfigService.get(key); + if (secretInConfig === undefined) return; + + this.set(handle, secretInConfig); + await this.gitConfigService.remove(key); + + console.warn(`Migrated Git config "${key}" to secret store.`); + return secretInConfig; + } +} diff --git a/app/src/lib/utils/context.ts b/app/src/lib/utils/context.ts index 3b9a95726..e57b5e10b 100644 --- a/app/src/lib/utils/context.ts +++ b/app/src/lib/utils/context.ts @@ -100,3 +100,15 @@ export function getContextStoreBySymbol = Readable>( if (!instance) throw new Error(`no instance of \`Readable<${key.toString()}[]>\` in context`); return instance; } + +export function buildContext(name: string): [() => T, (value: T | undefined) => void] { + const identifier = Symbol(name); + return [ + () => { + return svelteGetContext(identifier); + }, + (value: T | undefined) => { + setContext(identifier, value); + } + ]; +} diff --git a/app/src/routes/+layout.ts b/app/src/routes/+layout.ts index b6c5dfe88..bc7ffd6df 100644 --- a/app/src/routes/+layout.ts +++ b/app/src/routes/+layout.ts @@ -10,6 +10,7 @@ import { UpdaterService } from '$lib/backend/updater'; import { LineManagerFactory } from '$lib/commitLines/lineManager'; import { GitHubService } from '$lib/github/service'; import { RemotesService } from '$lib/remotes/service'; +import { RustSecretService } from '$lib/secrets/secretsService'; import { UserService } from '$lib/stores/user'; import { mockTauri } from '$lib/testing/index'; import lscache from 'lscache'; @@ -54,7 +55,8 @@ export async function load() { const githubService = new GitHubService(userService.accessToken$, remoteUrl$); const gitConfig = new GitConfigService(); - const aiService = new AIService(gitConfig, httpClient); + const secretsService = new RustSecretService(gitConfig); + const aiService = new AIService(gitConfig, secretsService, httpClient); const remotesService = new RemotesService(); const aiPromptService = new AIPromptService(); const lineManagerFactory = new LineManagerFactory(); diff --git a/app/src/routes/settings/ai/+page.svelte b/app/src/routes/settings/ai/+page.svelte index ed8cf3428..0e40e43a1 100644 --- a/app/src/routes/settings/ai/+page.svelte +++ b/app/src/routes/settings/ai/+page.svelte @@ -1,10 +1,11 @@