From 9e4df9701a7b86569b4ec2bead29b3d928e9aaf7 Mon Sep 17 00:00:00 2001 From: Colton Hurst Date: Thu, 15 Aug 2024 17:52:25 -0400 Subject: [PATCH] [SM-1174] Access Token Login State by Default in bws (#930) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 🎟ī¸ Tracking https://bitwarden.atlassian.net/browse/SM-1174 ## 📔 Objective This PR switches `bws` to use access token login state by default. This will help prevent many users from running into rate limiting issues. The two related config keys in a profile: - `state_dir` -> this is the same as it has been; a custom directory for where state files will be stored - `state_opt_out` -> this is a new key. If it's `true` or `1`, state files will not be used **Note**: the `gnu` build failures are already happening on `main` and have been brought up internally ## ⏰ Reminders before review - Contributor guidelines followed - All formatters and local linters executed and passed - Written new unit and / or integration tests where applicable - Protected functional changes with optionality (feature flags) - Used internationalization (i18n) for all UI strings - CI builds passed - Communicated to DevOps any deployment requirements - Updated any necessary documentation (Confluence, contributing docs) or informed the documentation team ## đŸĻŽ Reviewer guidelines - 👍 (`:+1:`) or similar for great changes - 📝 (`:memo:`) or ℹī¸ (`:information_source:`) for notes or general info - ❓ (`:question:`) for questions - 🤔 (`:thinking:`) or 💭 (`:thought_balloon:`) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion - 🎨 (`:art:`) for suggestions / improvements - ❌ (`:x:`) or ⚠ī¸ (`:warning:`) for more significant problems or concerns needing attention - 🌱 (`:seedling:`) or â™ģī¸ (`:recycle:`) for future improvements or indications of technical debt - ⛏ (`:pick:`) for minor or nitpick changes --- crates/bws/src/cli.rs | 3 ++- crates/bws/src/command/mod.rs | 9 ++++++- crates/bws/src/config.rs | 3 +++ crates/bws/src/main.rs | 29 +++++++++++++++++++---- crates/bws/src/state.rs | 31 +++++++++++++++++------- crates/bws/src/util.rs | 44 +++++++++++++++++++++++++++++++++++ 6 files changed, 105 insertions(+), 14 deletions(-) create mode 100644 crates/bws/src/util.rs diff --git a/crates/bws/src/cli.rs b/crates/bws/src/cli.rs index e0ee4264d..f37a7ab5d 100644 --- a/crates/bws/src/cli.rs +++ b/crates/bws/src/cli.rs @@ -11,7 +11,7 @@ pub(crate) const PROFILE_KEY_VAR_NAME: &str = "BWS_PROFILE"; pub(crate) const SERVER_URL_KEY_VAR_NAME: &str = "BWS_SERVER_URL"; pub(crate) const DEFAULT_CONFIG_FILENAME: &str = "config"; -pub(crate) const DEFAULT_CONFIG_DIRECTORY: &str = ".bws"; +pub(crate) const DEFAULT_CONFIG_DIRECTORY: &str = ".config/bws"; #[allow(non_camel_case_types)] #[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, ValueEnum, Debug)] @@ -20,6 +20,7 @@ pub(crate) enum ProfileKey { server_api, server_identity, state_dir, + state_opt_out, } #[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, ValueEnum, Debug)] diff --git a/crates/bws/src/command/mod.rs b/crates/bws/src/command/mod.rs index 0fe257c42..adcea5964 100644 --- a/crates/bws/src/command/mod.rs +++ b/crates/bws/src/command/mod.rs @@ -8,7 +8,7 @@ use clap::CommandFactory; use clap_complete::Shell; use color_eyre::eyre::{bail, Result}; -use crate::{config, Cli, ProfileKey}; +use crate::{config, util, Cli, ProfileKey}; pub(crate) fn completions(shell: Option) -> Result<()> { let Some(shell) = shell.or_else(Shell::from_env) else { @@ -48,6 +48,13 @@ pub(crate) fn config( (None, None) => bail!("Missing `name` and `value`"), (None, Some(_)) => bail!("Missing `value`"), (Some(_), None) => bail!("Missing `name`"), + (Some(ProfileKey::state_opt_out), Some(value)) => { + if util::string_to_bool(value.as_str()).is_err() { + bail!("Profile key \"state_opt_out\" must be \"true\" or \"false\""); + } else { + (ProfileKey::state_opt_out, value) + } + } (Some(name), Some(value)) => (name, value), }; diff --git a/crates/bws/src/config.rs b/crates/bws/src/config.rs index 6eae00e44..9fd91849e 100644 --- a/crates/bws/src/config.rs +++ b/crates/bws/src/config.rs @@ -21,6 +21,7 @@ pub(crate) struct Profile { pub server_api: Option, pub server_identity: Option, pub state_dir: Option, + pub state_opt_out: Option, } impl ProfileKey { @@ -30,6 +31,7 @@ impl ProfileKey { ProfileKey::server_api => p.server_api = Some(value), ProfileKey::server_identity => p.server_identity = Some(value), ProfileKey::state_dir => p.state_dir = Some(value), + ProfileKey::state_opt_out => p.state_opt_out = Some(value), } } } @@ -118,6 +120,7 @@ impl Profile { server_api: None, server_identity: None, state_dir: None, + state_opt_out: None, }) } pub(crate) fn api_url(&self) -> Result { diff --git a/crates/bws/src/main.rs b/crates/bws/src/main.rs index de9656a22..5d67bc103 100644 --- a/crates/bws/src/main.rs +++ b/crates/bws/src/main.rs @@ -7,6 +7,7 @@ use bitwarden::{ use bitwarden_cli::install_color_eyre; use clap::{CommandFactory, Parser}; use color_eyre::eyre::{bail, Result}; +use config::Profile; use log::error; use render::OutputSettings; @@ -15,6 +16,7 @@ mod command; mod config; mod render; mod state; +mod util; use crate::cli::*; @@ -84,10 +86,19 @@ async fn process_commands() -> Result<()> { }) .transpose()?; - let state_file = state::get_state_file( - profile.and_then(|p| p.state_dir).map(Into::into), - access_token_obj.access_token_id.to_string(), - )?; + let state_file = match get_state_opt_out(&profile) { + true => None, + false => match state::get_state_file( + profile.and_then(|p| p.state_dir).map(Into::into), + access_token_obj.access_token_id.to_string(), + ) { + Ok(state_file) => Some(state_file), + Err(e) => { + eprintln!("Warning: {}\nRetrieving the state file failed. Attempting to continue without using state. Please set \"state_dir\" in your config file to avoid authentication limits.", e); + None + } + }, + }; let client = bitwarden::Client::new(settings); @@ -150,3 +161,13 @@ fn get_config_profile( }; Ok(profile) } + +fn get_state_opt_out(profile: &Option) -> bool { + if let Some(profile) = profile { + if let Some(state_opt_out) = &profile.state_opt_out { + return util::string_to_bool(state_opt_out).unwrap_or(false); + } + } + + false +} diff --git a/crates/bws/src/state.rs b/crates/bws/src/state.rs index b42ca84da..b5756a056 100644 --- a/crates/bws/src/state.rs +++ b/crates/bws/src/state.rs @@ -1,17 +1,32 @@ use std::path::PathBuf; -use color_eyre::eyre::Result; +use color_eyre::eyre::{bail, Result}; +use directories::BaseDirs; + +use crate::DEFAULT_CONFIG_DIRECTORY; + +pub(crate) const DEFAULT_STATE_DIRECTORY: &str = "state"; pub(crate) fn get_state_file( state_dir: Option, access_token_id: String, -) -> Result> { - if let Some(mut state_dir) = state_dir { - std::fs::create_dir_all(&state_dir)?; - state_dir.push(access_token_id); +) -> Result { + let mut state_dir = match state_dir { + Some(state_dir) => state_dir, + None => { + if let Some(base_dirs) = BaseDirs::new() { + base_dirs + .home_dir() + .join(DEFAULT_CONFIG_DIRECTORY) + .join(DEFAULT_STATE_DIRECTORY) + } else { + bail!("A valid home directory doesn't exist"); + } + } + }; - return Ok(Some(state_dir)); - } + std::fs::create_dir_all(&state_dir)?; + state_dir.push(access_token_id); - Ok(None) + Ok(state_dir) } diff --git a/crates/bws/src/util.rs b/crates/bws/src/util.rs new file mode 100644 index 000000000..a3541ffea --- /dev/null +++ b/crates/bws/src/util.rs @@ -0,0 +1,44 @@ +const STRING_TO_BOOL_ERROR_MESSAGE: &str = "Could not convert string to bool"; + +pub(crate) fn string_to_bool(value: &str) -> Result { + match value.trim().to_lowercase().as_str() { + "true" | "1" => Ok(true), + "false" | "0" => Ok(false), + _ => Err(STRING_TO_BOOL_ERROR_MESSAGE), + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_string_to_bool_true_true() { + let result = string_to_bool("true"); + assert_eq!(result, Ok(true)); + } + + #[test] + fn test_string_to_bool_one_true() { + let result = string_to_bool("1"); + assert_eq!(result, Ok(true)); + } + + #[test] + fn test_string_to_bool_false_false() { + let result = string_to_bool("false"); + assert_eq!(result, Ok(false)); + } + + #[test] + fn test_string_to_bool_zero_false() { + let result = string_to_bool("0"); + assert_eq!(result, Ok(false)); + } + + #[test] + fn test_string_to_bool_bad_string_errors() { + let result = string_to_bool("hello world"); + assert_eq!(result, Err(STRING_TO_BOOL_ERROR_MESSAGE)); + } +}