From c006cbb2a04bce4afe90c12719986c68386aa4cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa?= Date: Tue, 5 Dec 2023 14:53:03 +0100 Subject: [PATCH] [PM-3434] Password generator (#261) ## Type of change ``` - [ ] Bug fix - [x] New feature development - [ ] Tech debt (refactoring, code cleanup, dependency upgrades, etc) - [ ] Build/deploy pipeline (DevOps) - [ ] Other ``` ## Objective Implement password generation The `min_` fields were set to bool incorrectly I assume, so I changed them to u8. At the moment if all the minimums turn out larger than the length, I just expand the length, but that seems wrong. I feel like it would be best to return an error in that case instead of changing any values behind the user, thoughts? --- .../src/tool/generators/client_generator.rs | 21 + .../bitwarden/src/tool/generators/password.rs | 396 +++++++++++++++++- crates/bw/src/main.rs | 2 +- 3 files changed, 401 insertions(+), 18 deletions(-) diff --git a/crates/bitwarden/src/tool/generators/client_generator.rs b/crates/bitwarden/src/tool/generators/client_generator.rs index 8af8ecd19..4675ad99d 100644 --- a/crates/bitwarden/src/tool/generators/client_generator.rs +++ b/crates/bitwarden/src/tool/generators/client_generator.rs @@ -10,6 +10,27 @@ pub struct ClientGenerator<'a> { } impl<'a> ClientGenerator<'a> { + /// Generates a random password. + /// + /// The character sets and password length can be customized using the `input` parameter. + /// + /// # Examples + /// + /// ``` + /// use bitwarden::{Client, tool::PasswordGeneratorRequest, error::Result}; + /// async fn test() -> Result<()> { + /// let input = PasswordGeneratorRequest { + /// lowercase: true, + /// uppercase: true, + /// numbers: true, + /// length: 20, + /// ..Default::default() + /// }; + /// let password = Client::new(None).generator().password(input).await.unwrap(); + /// println!("{}", password); + /// Ok(()) + /// } + /// ``` pub async fn password(&self, input: PasswordGeneratorRequest) -> Result { password(input) } diff --git a/crates/bitwarden/src/tool/generators/password.rs b/crates/bitwarden/src/tool/generators/password.rs index 237394f56..cf986495c 100644 --- a/crates/bitwarden/src/tool/generators/password.rs +++ b/crates/bitwarden/src/tool/generators/password.rs @@ -1,32 +1,394 @@ -use crate::error::Result; +use std::collections::BTreeSet; + +use rand::{distributions::Distribution, seq::SliceRandom, RngCore}; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; -/// Password generator request. If all options are false, the default is to -/// generate a password with: -/// - lowercase -/// - uppercase -/// - numbers -/// -/// The default length is 16. -#[derive(Serialize, Deserialize, Debug, JsonSchema, Default)] +use crate::error::{Error, Result}; + +/// Password generator request options. +#[derive(Serialize, Deserialize, Debug, JsonSchema)] #[serde(rename_all = "camelCase", deny_unknown_fields)] #[cfg_attr(feature = "mobile", derive(uniffi::Record))] pub struct PasswordGeneratorRequest { + /// Include lowercase characters (a-z). pub lowercase: bool, + /// Include uppercase characters (A-Z). pub uppercase: bool, + /// Include numbers (0-9). pub numbers: bool, + /// Include special characters: ! @ # $ % ^ & * pub special: bool, - pub length: Option, + /// The length of the generated password. + /// Note that the password length must be greater than the sum of all the minimums. + pub length: u8, + + /// When set to true, the generated password will not contain ambiguous characters. + /// The ambiguous characters are: I, O, l, 0, 1 + pub avoid_ambiguous: bool, // TODO: Should we rename this to include_all_characters? + + /// The minimum number of lowercase characters in the generated password. + /// When set, the value must be between 1 and 9. This value is ignored is lowercase is false + pub min_lowercase: Option, + /// The minimum number of uppercase characters in the generated password. + /// When set, the value must be between 1 and 9. This value is ignored is uppercase is false + pub min_uppercase: Option, + /// The minimum number of numbers in the generated password. + /// When set, the value must be between 1 and 9. This value is ignored is numbers is false + pub min_number: Option, + /// The minimum number of special characters in the generated password. + /// When set, the value must be between 1 and 9. This value is ignored is special is false + pub min_special: Option, +} + +const DEFAULT_PASSWORD_LENGTH: u8 = 16; + +impl Default for PasswordGeneratorRequest { + fn default() -> Self { + Self { + lowercase: true, + uppercase: true, + numbers: true, + special: false, + length: DEFAULT_PASSWORD_LENGTH, + avoid_ambiguous: false, + min_lowercase: None, + min_uppercase: None, + min_number: None, + min_special: None, + } + } +} + +const UPPER_CHARS_AMBIGUOUS: &[char] = &['I', 'O']; +const LOWER_CHARS_AMBIGUOUS: &[char] = &['l']; +const NUMBER_CHARS_AMBIGUOUS: &[char] = &['0', '1']; +const SPECIAL_CHARS: &[char] = &['!', '@', '#', '$', '%', '^', '&', '*']; + +/// A set of characters used to generate a password. This set is backed by a BTreeSet +/// to have consistent ordering between runs. This is not important during normal execution, +/// but it's necessary for the tests to be repeatable. +/// To create an instance, use [`CharSet::default()`](CharSet::default) +#[derive(Clone, Default)] +struct CharSet(BTreeSet); +impl CharSet { + /// Includes the given characters in the set. Any duplicate items will be ignored + pub fn include(self, other: impl IntoIterator) -> Self { + self.include_if(true, other) + } + + /// Includes the given characters in the set if the predicate is true. Any duplicate items will be ignored + pub fn include_if(mut self, predicate: bool, other: impl IntoIterator) -> Self { + if predicate { + self.0.extend(other); + } + self + } + + /// Excludes the given characters from the set. Any missing items will be ignored + pub fn exclude_if<'a>( + self, + predicate: bool, + other: impl IntoIterator, + ) -> Self { + if predicate { + let other: BTreeSet<_> = other.into_iter().copied().collect(); + Self(self.0.difference(&other).copied().collect()) + } else { + self + } + } +} +impl<'a> IntoIterator for &'a CharSet { + type Item = char; + type IntoIter = std::iter::Copied>; + fn into_iter(self) -> Self::IntoIter { + self.0.iter().copied() + } +} +impl Distribution for CharSet { + fn sample(&self, rng: &mut R) -> char { + let idx = rng.gen_range(0..self.0.len()); + *self.0.iter().nth(idx).expect("Valid index") + } +} + +/// Represents a set of valid options to generate a password with. +/// To get an instance of it, use [`PasswordGeneratorRequest::validate_options`](PasswordGeneratorRequest::validate_options) +struct PasswordGeneratorOptions { + pub(super) lower: (CharSet, usize), + pub(super) upper: (CharSet, usize), + pub(super) number: (CharSet, usize), + pub(super) special: (CharSet, usize), + pub(super) all: (CharSet, usize), + + pub(super) length: usize, +} + +impl PasswordGeneratorRequest { + /// Validates the request and returns an immutable struct with valid options to use with the password generator. + fn validate_options(self) -> Result { + // TODO: Add password generator policy checks + + // We always have to have at least one character set enabled + if !self.lowercase && !self.uppercase && !self.numbers && !self.special { + return Err(Error::Internal( + "At least one character set must be enabled", + )); + } + + if self.length < 4 { + return Err(Error::Internal( + "A password must be at least 4 characters long", + )); + } + + // Make sure the minimum values are zero when the character + // set is disabled, and at least one when it's enabled + fn get_minimum(min: Option, enabled: bool) -> usize { + if enabled { + usize::max(min.unwrap_or(1) as usize, 1) + } else { + 0 + } + } + + let length = self.length as usize; + let min_lowercase = get_minimum(self.min_lowercase, self.lowercase); + let min_uppercase = get_minimum(self.min_uppercase, self.uppercase); + let min_number = get_minimum(self.min_number, self.numbers); + let min_special = get_minimum(self.min_special, self.special); + + // Check that the minimum lengths aren't larger than the password length + let minimum_length = min_lowercase + min_uppercase + min_number + min_special; + if minimum_length > length { + return Err(Error::Internal( + "Password length can't be less than the sum of the minimums", + )); + } + + let lower = ( + CharSet::default() + .include_if(self.lowercase, 'a'..='z') + .exclude_if(self.avoid_ambiguous, LOWER_CHARS_AMBIGUOUS), + min_lowercase, + ); + + let upper = ( + CharSet::default() + .include_if(self.uppercase, 'A'..='Z') + .exclude_if(self.avoid_ambiguous, UPPER_CHARS_AMBIGUOUS), + min_uppercase, + ); + + let number = ( + CharSet::default() + .include_if(self.numbers, '0'..='9') + .exclude_if(self.avoid_ambiguous, NUMBER_CHARS_AMBIGUOUS), + min_number, + ); + + let special = ( + CharSet::default().include_if(self.special, SPECIAL_CHARS.iter().copied()), + min_special, + ); + + let all = ( + CharSet::default() + .include(&lower.0) + .include(&upper.0) + .include(&number.0) + .include(&special.0), + length - minimum_length, + ); + + Ok(PasswordGeneratorOptions { + lower, + upper, + number, + special, + all, + length, + }) + } +} + +/// Implementation of the random password generator. This is not accessible to the public API. +/// See [`ClientGenerator::password`](crate::ClientGenerator::password) for the API function. +pub(super) fn password(input: PasswordGeneratorRequest) -> Result { + let options = input.validate_options()?; + Ok(password_with_rng(rand::thread_rng(), options)) +} + +fn password_with_rng(mut rng: impl RngCore, options: PasswordGeneratorOptions) -> String { + let mut buf: Vec = Vec::with_capacity(options.length); + + let opts = [ + &options.all, + &options.upper, + &options.lower, + &options.number, + &options.special, + ]; + for (set, qty) in opts { + buf.extend(set.sample_iter(&mut rng).take(*qty)); + } + + buf.shuffle(&mut rng); - pub avoid_ambiguous: Option, // TODO: Should we rename this to include_all_characters? - pub min_lowercase: Option, - pub min_uppercase: Option, - pub min_number: Option, - pub min_special: Option, + buf.iter().collect() } -pub(super) fn password(_input: PasswordGeneratorRequest) -> Result { - Ok("pa11w0rd".to_string()) +#[cfg(test)] +mod test { + use std::collections::BTreeSet; + + use rand::SeedableRng; + + use super::*; + + // We convert the slices to BTreeSets to be able to use `is_subset` + fn ref_to_set<'a>(chars: impl IntoIterator) -> BTreeSet { + chars.into_iter().copied().collect() + } + fn to_set(chars: impl IntoIterator) -> BTreeSet { + chars.into_iter().collect() + } + + #[test] + fn test_password_gen_all_charsets_enabled() { + let mut rng = rand_chacha::ChaCha8Rng::from_seed([0u8; 32]); + + let options = PasswordGeneratorRequest { + lowercase: true, + uppercase: true, + numbers: true, + special: true, + avoid_ambiguous: false, + ..Default::default() + } + .validate_options() + .unwrap(); + + assert_eq!(to_set(&options.lower.0), to_set('a'..='z')); + assert_eq!(to_set(&options.upper.0), to_set('A'..='Z')); + assert_eq!(to_set(&options.number.0), to_set('0'..='9')); + assert_eq!(to_set(&options.special.0), ref_to_set(SPECIAL_CHARS)); + + let pass = password_with_rng(&mut rng, options); + assert_eq!(pass, "Z!^B5r%hUa23dFM@"); + } + + #[test] + fn test_password_gen_only_letters_enabled() { + let mut rng = rand_chacha::ChaCha8Rng::from_seed([0u8; 32]); + + let options = PasswordGeneratorRequest { + lowercase: true, + uppercase: true, + numbers: false, + special: false, + avoid_ambiguous: false, + ..Default::default() + } + .validate_options() + .unwrap(); + + assert_eq!(to_set(&options.lower.0), to_set('a'..='z')); + assert_eq!(to_set(&options.upper.0), to_set('A'..='Z')); + assert_eq!(to_set(&options.number.0), to_set([])); + assert_eq!(to_set(&options.special.0), to_set([])); + + let pass = password_with_rng(&mut rng, options); + assert_eq!(pass, "NQiFrGufQMiNUAmj"); + } + + #[test] + fn test_password_gen_only_numbers_and_lower_enabled_no_ambiguous() { + let mut rng = rand_chacha::ChaCha8Rng::from_seed([0u8; 32]); + + let options = PasswordGeneratorRequest { + lowercase: true, + uppercase: false, + numbers: true, + special: false, + avoid_ambiguous: true, + ..Default::default() + } + .validate_options() + .unwrap(); + + assert!(to_set(&options.lower.0).is_subset(&to_set('a'..='z'))); + assert!(to_set(&options.lower.0).is_disjoint(&ref_to_set(LOWER_CHARS_AMBIGUOUS))); + + assert!(to_set(&options.number.0).is_subset(&to_set('0'..='9'))); + assert!(to_set(&options.number.0).is_disjoint(&ref_to_set(NUMBER_CHARS_AMBIGUOUS))); + + assert_eq!(to_set(&options.upper.0), to_set([])); + assert_eq!(to_set(&options.special.0), to_set([])); + + let pass = password_with_rng(&mut rng, options); + assert_eq!(pass, "mnjabfz5ct272prf"); + } + + #[test] + fn test_password_gen_only_upper_and_special_enabled_no_ambiguous() { + let mut rng = rand_chacha::ChaCha8Rng::from_seed([0u8; 32]); + + let options = PasswordGeneratorRequest { + lowercase: false, + uppercase: true, + numbers: false, + special: true, + avoid_ambiguous: true, + ..Default::default() + } + .validate_options() + .unwrap(); + + assert!(to_set(&options.upper.0).is_subset(&to_set('A'..='Z'))); + assert!(to_set(&options.upper.0).is_disjoint(&ref_to_set(UPPER_CHARS_AMBIGUOUS))); + + assert_eq!(to_set(&options.special.0), ref_to_set(SPECIAL_CHARS)); + + assert_eq!(to_set(&options.lower.0), to_set([])); + assert_eq!(to_set(&options.number.0), to_set([])); + + let pass = password_with_rng(&mut rng, options); + assert_eq!(pass, "B*GBQANS%UZPQD!K"); + } + + #[test] + fn test_password_gen_minimum_limits() { + let mut rng = rand_chacha::ChaCha8Rng::from_seed([0u8; 32]); + + let options = PasswordGeneratorRequest { + lowercase: true, + uppercase: true, + numbers: true, + special: true, + avoid_ambiguous: false, + length: 24, + min_lowercase: Some(5), + min_uppercase: Some(5), + min_number: Some(5), + min_special: Some(5), + } + .validate_options() + .unwrap(); + + assert_eq!(to_set(&options.lower.0), to_set('a'..='z')); + assert_eq!(to_set(&options.upper.0), to_set('A'..='Z')); + assert_eq!(to_set(&options.number.0), to_set('0'..='9')); + assert_eq!(to_set(&options.special.0), ref_to_set(SPECIAL_CHARS)); + + assert_eq!(options.lower.1, 5); + assert_eq!(options.upper.1, 5); + assert_eq!(options.number.1, 5); + assert_eq!(options.special.1, 5); + + let pass = password_with_rng(&mut rng, options); + assert_eq!(pass, "236q5!a#R%PG5rI%k1!*@uRt"); + } } diff --git a/crates/bw/src/main.rs b/crates/bw/src/main.rs index c4664c7f4..236aef22d 100644 --- a/crates/bw/src/main.rs +++ b/crates/bw/src/main.rs @@ -213,7 +213,7 @@ async fn process_commands() -> Result<()> { uppercase: args.uppercase, numbers: args.numbers, special: args.special, - length: Some(args.length), + length: args.length, ..Default::default() }) .await?;