Skip to content

Commit

Permalink
support global secrets (without namespace for build types) and provid…
Browse files Browse the repository at this point in the history
…e commands in `tauri` crate

That way globals secrets can be used from the UI directly.
  • Loading branch information
Byron committed Jul 2, 2024
1 parent b08a9e8 commit de00e4f
Show file tree
Hide file tree
Showing 7 changed files with 100 additions and 40 deletions.
42 changes: 30 additions & 12 deletions crates/gitbutler-core/src/secret.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<String>) -> 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<String>, namespace: Namespace) -> Result<()> {
let entry = entry_for(handle, namespace)?;
if secret.0.is_empty() {
entry.delete_password()?;
} else {
Expand All @@ -17,30 +32,33 @@ pub fn persist(handle: &str, secret: &Sensitive<String>) -> Result<()> {
Ok(())
}

/// Obtain the previously [stored](persist()) secret known as `handle`.
pub fn retrieve(handle: &str) -> Result<Option<Sensitive<String>>> {
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<Option<Sensitive<String>>> {
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<String>) {
*NAMESPACE.lock().unwrap() = identifier.into()
}

fn entry_for(handle: &str) -> Result<keyring::Entry> {
let ns = NAMESPACE.lock().unwrap();
fn entry_for(handle: &str, namespace: Namespace) -> Result<keyring::Entry> {
let ns = match namespace {
Namespace::BuildKind => NAMESPACE.lock().unwrap().clone(),
Namespace::Global => "gitbutler".into(),
};
Ok(keyring::Entry::new(
&format!(
"{prefix}-{handle}",
Expand Down
15 changes: 9 additions & 6 deletions crates/gitbutler-core/src/users/controller.rs
Original file line number Diff line number Diff line change
@@ -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.
///
Expand Down Expand Up @@ -45,20 +45,23 @@ 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(())
}
}

/// As `user` sports interior mutability right now, let's play it safe and work with fully owned items only.
fn write_without_secrets_if_secrets_present(storage: &Storage, user: User) -> Result<bool> {
let mut needs_write = false;
let namespace = secret::Namespace::BuildKind;
if let Some(gb_token) = user.access_token.borrow_mut().take() {
needs_write |= 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)?;
Expand Down
9 changes: 7 additions & 2 deletions crates/gitbutler-core/src/users/user.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use crate::secret;
use crate::types::Sensitive;
use anyhow::{Context, Result};
use serde::{Deserialize, Serialize};
Expand Down Expand Up @@ -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)
}
Expand All @@ -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)
}
Expand Down
46 changes: 28 additions & 18 deletions crates/gitbutler-core/tests/secret/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,41 +9,51 @@ 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(())
}

#[test]
#[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;
2 changes: 2 additions & 0 deletions crates/gitbutler-tauri/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,6 @@ pub mod remotes;
pub mod undo;
pub mod users;
pub mod virtual_branches;
pub mod secret;

pub mod zip;
6 changes: 4 additions & 2 deletions crates/gitbutler-tauri/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down
20 changes: 20 additions & 0 deletions crates/gitbutler-tauri/src/secret.rs
Original file line number Diff line number Diff line change
@@ -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<Option<String>, Error> {
Ok(secret::retrieve(handle, secret::Namespace::Global)?.map(|s| s.0))
}

#[tauri::command(async)]
#[instrument(skip(secret), err(Debug), fields(secret = "<redacted>"))]
pub async fn secret_set_global(handle: &str, secret: String) -> Result<(), Error> {
Ok(secret::persist(
handle,
&Sensitive(secret),
secret::Namespace::Global,
)?)
}

0 comments on commit de00e4f

Please sign in to comment.