Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Credentials storage XDG #109

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 71 additions & 4 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ flate2 = "1"
futures = "0.3"
git2 = { version = "0.18.1", optional = true }
hex = "0.4.3"
home = "0.5.5"
human-panic = "1"
protoc-bin-vendored = { version = "3.0.0", optional = true }
ring = "0.16"
Expand All @@ -52,6 +51,7 @@ tracing-subscriber = "0.3"
url = { version = "2.4", features = ["serde"] }
walkdir = "2"
async-recursion = "1.0.5"
dirs = "5.0.1"

[dependencies.reqwest]
version = "0.11"
Expand Down
82 changes: 56 additions & 26 deletions src/credentials.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,19 @@

use eyre::{Context, ContextCompat};
use serde::{Deserialize, Serialize};
use std::{collections::HashMap, env, path::PathBuf};
use std::{
collections::HashMap,
env::{var, VarError},
io::ErrorKind,
path::PathBuf,
};
use tokio::fs;

use crate::registry::RegistryUri;

/// Global configuration directory for `buffrs`
pub const BUFFRS_HOME: &str = ".buffrs";
pub const BUFFRS_DIR: &str = "buffrs";
/// Filename of the credential store
pub const CREDENTIALS_FILE: &str = "credentials.toml";

Expand All @@ -32,54 +38,78 @@ pub struct Credentials {
}

impl Credentials {
fn location() -> eyre::Result<PathBuf> {
let home = home::home_dir().wrap_err("Failed to locate home directory")?;
/// Get buffrs config directory.
fn buffrs_home() -> Result<PathBuf, VarError> {
var("BUFFRS_HOME").map(PathBuf::from)
}

let home = env::var("BUFFRS_HOME").map(PathBuf::from).unwrap_or(home);
/// XDG path: uses whichever config directory is appropriate for the platform.
///
/// For example, on Linux this might be `~/.config/buffrs/credentials.toml`.
fn xdg_path() -> PathBuf {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd call this "config_path" or something similar. Knowledge of the XDG spec shouldn't be required to understand what this is about.

Self::buffrs_home()
.unwrap_or_else(|_| dirs::config_dir().expect("get config dir").join(BUFFRS_DIR))
.join(CREDENTIALS_FILE)
}

Ok(home.join(BUFFRS_HOME).join(CREDENTIALS_FILE))
/// Legacy path: hard-coded to `~/.buffrs/credentials.toml`.
fn legacy_path() -> PathBuf {
Self::buffrs_home()
.unwrap_or_else(|_| dirs::home_dir().expect("get home dir").join(BUFFRS_HOME))
.join(CREDENTIALS_FILE)
}

/// Possible locations for the credentials file
fn possible_locations() -> Vec<PathBuf> {
vec![Self::xdg_path(), Self::legacy_path()]
}

/// Checks if the credentials exists
pub async fn exists() -> eyre::Result<bool> {
fs::try_exists(Self::location()?)
.await
.wrap_err("Failed to detect credentials")
for location in &Self::possible_locations() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this may be cleaner in functional style: Self::possible_locations().iter().map(...).all().

if fs::try_exists(&location).await? {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really want to terminate on the first error? Imagine we fail to access the system-wide credentials because we lack permission, shouldn't we keep going and try user locations too?

BTW fs::try_exists is only in nightly so I'm surprised this compiles in CI (probably something worth looking into, will file an issue for it).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This refers to tokio::fs::try_exists, which is probably not restricted to nightly?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Womp, you're right! This is why I don't like tokio shadowing std stuff... 😅 Ignore that part (and I'll close the issue).

return Ok(true);
}
}

return Ok(false);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Flagged by clippy, this return is superfluous.

}

/// Reads the credentials from the file system
pub async fn read() -> eyre::Result<Self> {
let toml = fs::read_to_string(Self::location()?)
.await
.wrap_err("Failed to read credentials")?;
for location in &Self::possible_locations() {
let contents = match fs::read_to_string(location).await {
Ok(string) => string,
Err(error) if error.kind() == ErrorKind::NotFound => continue,
Err(error) => return Err(error).wrap_err("opening credentials file"),
};

let raw: RawCredentialCollection =
toml::from_str(&toml).wrap_err("Failed to parse credentials")?;
let raw: RawCredentialCollection =
toml::from_str(&contents).wrap_err("Failed to parse credentials")?;

return Ok(raw.into());
}

Ok(raw.into())
eyre::bail!("cannot parse credentials")
}

/// Writes the credentials to the file system
pub async fn write(&self) -> eyre::Result<()> {
fs::create_dir(
Self::location()?
.parent()
.wrap_err("Invalid credentials location")?,
)
.await
.ok();
let location = Self::xdg_path();

fs::create_dir(location.parent().wrap_err("Invalid credentials location")?)
.await
.ok();

let data: RawCredentialCollection = self.clone().into();

fs::write(Self::location()?, toml::to_string(&data)?.into_bytes())
fs::write(location, toml::to_string(&data)?.into_bytes())
.await
.wrap_err("Failed to write credentials")
}

/// Loads the credentials from the file system
///
/// Note: Initializes the credential file if its not present
pub async fn load() -> eyre::Result<Self> {
/// Loads the credentials from the file system, or creates default if not present.
pub async fn load_or_init() -> eyre::Result<Self> {
let Ok(credentials) = Self::read().await else {
let credentials = Credentials::default();
credentials.write().await?;
Expand Down
2 changes: 1 addition & 1 deletion src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ async fn main() -> eyre::Result<()> {
.unwrap();

let cli = Cli::parse();
let credentials = Credentials::load().await?;
let credentials = Credentials::load_or_init().await?;

match cli.command {
Command::Init { lib, api, package } => {
Expand Down
Loading