Skip to content

Commit

Permalink
[PM-5692] Extract generators to separate crate (#511)
Browse files Browse the repository at this point in the history
Continuation on #402. This PR extracts the generators to their own crate
to establish clear boundaries between the code. There is still some
logic in the client to expose generators that we may want to revisit.

Also extracts explicit errors for the different generators for better
error handling downstream when/if desired.
  • Loading branch information
Hinton authored Jan 19, 2024
1 parent 9cad470 commit b174e49
Show file tree
Hide file tree
Showing 33 changed files with 843 additions and 636 deletions.
3 changes: 2 additions & 1 deletion .github/workflows/build-rust-crates.yml
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,10 @@ jobs:

package:
- bitwarden
- bitwarden-crypto
- bitwarden-api-api
- bitwarden-api-identity
- bitwarden-crypto
- bitwarden-generators

steps:
- name: Checkout
Expand Down
11 changes: 11 additions & 0 deletions .github/workflows/publish-rust-crates.yml
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,11 @@ on:
required: true
default: true
type: boolean
publish_bitwarden-generators:
description: "Publish bitwarden-generators crate"
required: true
default: true
type: boolean

defaults:
run:
Expand Down Expand Up @@ -67,6 +72,7 @@ jobs:
PUBLISH_BITWARDEN_API_API: ${{ github.event.inputs.publish_bitwarden-api-api }}
PUBLISH_BITWARDEN_API_IDENTITY: ${{ github.event.inputs.publish_bitwarden-api-identity }}
PUBLISH_BITWARDEN_CRYPTO: ${{ github.event.inputs.publish_bitwarden-crypto }}
PUBLISH_BITWARDEN_GENERATORS: ${{ github.event.inputs.publish_bitwarden-generators }}
run: |
if [[ "$PUBLISH_BITWARDEN" == "false" ]] && [[ "$PUBLISH_BITWARDEN_API_API" == "false" ]] && [[ "$PUBLISH_BITWARDEN_API_IDENTITY" == "false" ]]; then
echo "==================================="
Expand Down Expand Up @@ -98,6 +104,11 @@ jobs:
PACKAGES_LIST="$PACKAGES_LIST bitwarden-crypto"
fi
if [[ "$PUBLISH_BITWARDEN_GENERATORS" == "true" ]]; then
PACKAGES_COMMAND="$PACKAGES_COMMAND -p bitwarden-generators"
PACKAGES_LIST="$PACKAGES_LIST bitwarden-generators"
fi
echo "Packages command: " $PACKAGES_COMMAND
echo "Packages list: " $PACKAGES_LIST
Expand Down
7 changes: 7 additions & 0 deletions .github/workflows/version-bump.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ on:
- bitwarden-api-api
- bitwarden-api-identity
- bitwarden-crypto
- bitwarden-generators
- bitwarden-json
- cli
- napi
Expand Down Expand Up @@ -123,6 +124,12 @@ jobs:
if: ${{ inputs.project == 'bitwarden-crypto' }}
run: cargo-set-version set-version -p bitwarden-crypto ${{ inputs.version_number }}

### bitwarden-generators

- name: Bump bitwarden-generators crate Version
if: ${{ inputs.project == 'bitwarden-generators' }}
run: cargo-set-version set-version -p bitwarden-generators ${{ inputs.version_number }}

### cli

- name: Bump cli Version
Expand Down
18 changes: 18 additions & 0 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 crates/bitwarden-crypto/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ license-file = "LICENSE"
repository = "https://github.com/bitwarden/sdk"
homepage = "https://bitwarden.com"
description = """
Bitwarden Cryptographic primitives
Internal crate for the bitwarden crate. Do not use.
"""
keywords = ["bitwarden"]
edition = "2021"
Expand Down
6 changes: 6 additions & 0 deletions crates/bitwarden-crypto/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
# Bitwarden Crypto

This is an internal crate for the Bitwarden SDK do not depend on this directly and use the
[`bitwarden`](https://crates.io/crates/bitwarden) crate instead.

This crate does not follow semantic versioning and the public interface may change at any time.
33 changes: 33 additions & 0 deletions crates/bitwarden-generators/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
[package]
name = "bitwarden-generators"
version = "0.1.0"
authors = ["Bitwarden Inc"]
license-file = "LICENSE"
repository = "https://github.com/bitwarden/sdk"
homepage = "https://bitwarden.com"
description = """
Internal crate for the bitwarden crate. Do not use.
"""
keywords = ["bitwarden"]
edition = "2021"
rust-version = "1.57"

[features]
mobile = ["uniffi"] # Mobile-specific features

[dependencies]
bitwarden-crypto = { path = "../bitwarden-crypto", version = "=0.1.0" }
rand = ">=0.8.5, <0.9"
reqwest = { version = ">=0.11, <0.12", features = [
"json",
], default-features = false }
schemars = { version = ">=0.8.9, <0.9", features = ["uuid1", "chrono"] }
serde = { version = ">=1.0, <2.0", features = ["derive"] }
serde_json = ">=1.0.96, <2.0"
thiserror = ">=1.0.40, <2.0"
uniffi = { version = "=0.25.2", optional = true }

[dev-dependencies]
rand_chacha = "0.3.1"
tokio = { version = "1.35.1", features = ["rt", "macros"] }
wiremock = "0.5.22"
6 changes: 6 additions & 0 deletions crates/bitwarden-generators/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
# Bitwarden Generators

This is an internal crate for the Bitwarden SDK do not depend on this directly and use the
[`bitwarden`](https://crates.io/crates/bitwarden) crate instead.

This crate does not follow semantic versioning and the public interface may change at any time.
11 changes: 11 additions & 0 deletions crates/bitwarden-generators/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
mod passphrase;
pub use passphrase::{passphrase, PassphraseError, PassphraseGeneratorRequest};
mod password;
mod util;
pub use password::{password, PasswordError, PasswordGeneratorRequest};
mod username;
pub use username::{username, ForwarderServiceType, UsernameError, UsernameGeneratorRequest};
mod username_forwarders;

#[cfg(feature = "mobile")]
uniffi::setup_scaffolding!();
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,17 @@ use bitwarden_crypto::EFF_LONG_WORD_LIST;
use rand::{seq::SliceRandom, Rng, RngCore};
use schemars::JsonSchema;
use serde::{Deserialize, Serialize};
use thiserror::Error;

use crate::{error::Result, util::capitalize_first_letter};
use crate::util::capitalize_first_letter;

#[derive(Debug, Error)]
pub enum PassphraseError {
#[error("'num_words' must be between {} and {}", minimum, maximum)]
InvalidNumWords { minimum: u8, maximum: u8 },
#[error("'word_separator' cannot be empty")]
EmptyWordSeparator,
}

/// Passphrase generator request options.
#[derive(Serialize, Deserialize, Debug, JsonSchema)]
Expand Down Expand Up @@ -46,16 +55,19 @@ struct ValidPassphraseGeneratorOptions {

impl PassphraseGeneratorRequest {
/// Validates the request and returns an immutable struct with valid options to use with the passphrase generator.
fn validate_options(self) -> Result<ValidPassphraseGeneratorOptions> {
fn validate_options(self) -> Result<ValidPassphraseGeneratorOptions, PassphraseError> {
// TODO: Add password generator policy checks

if !(MINIMUM_PASSPHRASE_NUM_WORDS..=MAXIMUM_PASSPHRASE_NUM_WORDS).contains(&self.num_words)
{
return Err(format!("'num_words' must be between {MINIMUM_PASSPHRASE_NUM_WORDS} and {MAXIMUM_PASSPHRASE_NUM_WORDS}").into());
return Err(PassphraseError::InvalidNumWords {
minimum: MINIMUM_PASSPHRASE_NUM_WORDS,
maximum: MAXIMUM_PASSPHRASE_NUM_WORDS,
});
}

if self.word_separator.chars().next().is_none() {
return Err("'word_separator' cannot be empty".into());
return Err(PassphraseError::EmptyWordSeparator);
};

Ok(ValidPassphraseGeneratorOptions {
Expand All @@ -67,13 +79,8 @@ impl PassphraseGeneratorRequest {
}
}

/// Implementation of the random passphrase generator. This is not accessible to the public API.
/// See [`ClientGenerator::passphrase`](crate::ClientGenerator::passphrase) for the API function.
///
/// # Arguments:
/// * `options`: Valid parameters used to generate the passphrase. To create it, use
/// [`PassphraseGeneratorRequest::validate_options`](PassphraseGeneratorRequest::validate_options).
pub(super) fn passphrase(request: PassphraseGeneratorRequest) -> Result<String> {
/// Implementation of the random passphrase generator.
pub fn passphrase(request: PassphraseGeneratorRequest) -> Result<String, PassphraseError> {
let options = request.validate_options()?;
Ok(passphrase_with_rng(rand::thread_rng(), options))
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,15 @@ use std::collections::BTreeSet;
use rand::{distributions::Distribution, seq::SliceRandom, RngCore};
use schemars::JsonSchema;
use serde::{Deserialize, Serialize};

use crate::error::Result;
use thiserror::Error;

#[derive(Debug, Error)]
pub enum PasswordError {
#[error("No character set enabled")]
NoCharacterSetEnabled,
#[error("Invalid password length")]
InvalidLength,
}

/// Password generator request options.
#[derive(Serialize, Deserialize, Debug, JsonSchema)]
Expand Down Expand Up @@ -32,7 +39,7 @@ pub struct PasswordGeneratorRequest {
/// When set, the value must be between 1 and 9. This value is ignored is lowercase is false
pub min_lowercase: Option<u8>,
/// 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
/// When set, the value must be between 1 and 9. This value is ignored is uppercase is false
pub min_uppercase: Option<u8>,
/// 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
Expand Down Expand Up @@ -128,16 +135,16 @@ struct PasswordGeneratorOptions {

impl PasswordGeneratorRequest {
/// Validates the request and returns an immutable struct with valid options to use with the password generator.
fn validate_options(self) -> Result<PasswordGeneratorOptions> {
fn validate_options(self) -> Result<PasswordGeneratorOptions, PasswordError> {
// 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("At least one character set must be enabled".into());
return Err(PasswordError::NoCharacterSetEnabled);
}

if self.length < 4 {
return Err("A password must be at least 4 characters long".into());
return Err(PasswordError::InvalidLength);
}

// Make sure the minimum values are zero when the character
Expand All @@ -159,7 +166,7 @@ impl PasswordGeneratorRequest {
// 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("Password length can't be less than the sum of the minimums".into());
return Err(PasswordError::InvalidLength);
}

let lower = (
Expand Down Expand Up @@ -208,9 +215,8 @@ impl PasswordGeneratorRequest {
}
}

/// 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<String> {
/// Implementation of the random password generator.
pub fn password(input: PasswordGeneratorRequest) -> Result<String, PasswordError> {
let options = input.validate_options()?;
Ok(password_with_rng(rand::thread_rng(), options))
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,25 @@
use bitwarden_crypto::EFF_LONG_WORD_LIST;
use rand::{distributions::Distribution, seq::SliceRandom, Rng, RngCore};
use reqwest::StatusCode;
use schemars::JsonSchema;
use serde::{Deserialize, Serialize};
use thiserror::Error;

use crate::{error::Result, util::capitalize_first_letter};
use crate::util::capitalize_first_letter;

#[derive(Debug, Error)]
pub enum UsernameError {
#[error("Invalid API Key")]
InvalidApiKey,
#[error("Unknown error")]
Unknown,

#[error("Received error message from server: [{}] {}", .status, .message)]
ResponseContent { status: StatusCode, message: String },

#[error(transparent)]
Reqwest(#[from] reqwest::Error),
}

#[derive(Serialize, Deserialize, Debug, JsonSchema)]
#[serde(rename_all = "camelCase", deny_unknown_fields)]
Expand Down Expand Up @@ -85,10 +101,14 @@ pub enum UsernameGeneratorRequest {
impl ForwarderServiceType {
// Generate a username using the specified email forwarding service
// This requires an HTTP client to be passed in, as the service will need to make API calls
pub async fn generate(self, http: &reqwest::Client, website: Option<String>) -> Result<String> {
pub async fn generate(
self,
http: &reqwest::Client,
website: Option<String>,
) -> Result<String, UsernameError> {
use ForwarderServiceType::*;

use crate::tool::generators::username_forwarders::*;
use crate::username_forwarders::*;

match self {
AddyIo {
Expand All @@ -107,14 +127,14 @@ impl ForwarderServiceType {
}
}

/// Implementation of the username generator. This is not accessible to the public API.
/// See [`ClientGenerator::username`](crate::ClientGenerator::username) for the API function.
/// Implementation of the username generator.
///
/// Note: The HTTP client is passed in as a required parameter for convenience,
/// as some username generators require making API calls.
pub(super) async fn username(
pub async fn username(
input: UsernameGeneratorRequest,
http: &reqwest::Client,
) -> Result<String> {
) -> Result<String, UsernameError> {
use rand::thread_rng;
use UsernameGeneratorRequest::*;
match input {
Expand Down
Loading

0 comments on commit b174e49

Please sign in to comment.