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

[PM-13371] Repository split - Avoid depdending on Bitwarden #1124

Merged
merged 7 commits into from
Oct 11, 2024
Merged
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
6 changes: 0 additions & 6 deletions .github/workflows/build-rust-crates.yml
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,6 @@ jobs:
env:
RUSTFLAGS: "-D warnings"

- name: Build Internal
if: ${{ matrix.package == 'bitwarden' }}
run: cargo build -p ${{ matrix.package }} --features internal --release
env:
RUSTFLAGS: "-D warnings"

release-dry-run:
name: Release dry-run
runs-on: ubuntu-latest
Expand Down
12 changes: 3 additions & 9 deletions Cargo.lock

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

3 changes: 1 addition & 2 deletions crates/bitwarden-json/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,7 @@ repository.workspace = true
license-file.workspace = true

[features]
internal = ["bitwarden/internal"] # Internal testing methods
secrets = ["bitwarden/secrets"] # Secrets manager API
secrets = ["bitwarden/secrets"] # Secrets manager API
Copy link
Member

Choose a reason for hiding this comment

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

If bitwarden-json is going to be a secrets manager only crate now, we can probably remove the secrets feature as well right?


[dependencies]
bitwarden = { workspace = true }
Expand Down
14 changes: 0 additions & 14 deletions crates/bitwarden-json/src/client.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
#[cfg(feature = "internal")]
use bitwarden::vault::ClientVaultExt;
use bitwarden::ClientSettings;
#[cfg(feature = "secrets")]
use bitwarden::{
Expand Down Expand Up @@ -54,22 +52,10 @@ impl Client {
let client = &self.0;

match cmd {
#[cfg(feature = "internal")]
Command::PasswordLogin(req) => client.auth().login_password(&req).await.into_string(),
#[cfg(feature = "secrets")]
Command::LoginAccessToken(req) => {
client.auth().login_access_token(&req).await.into_string()
}
#[cfg(feature = "internal")]
Command::GetUserApiKey(req) => {
client.platform().get_user_api_key(req).await.into_string()
}
#[cfg(feature = "internal")]
Command::ApiKeyLogin(req) => client.auth().login_api_key(&req).await.into_string(),
#[cfg(feature = "internal")]
Command::Sync(req) => client.vault().sync(&req).await.into_string(),
#[cfg(feature = "internal")]
Command::Fingerprint(req) => client.platform().fingerprint(&req).into_string(),

#[cfg(feature = "secrets")]
Command::Secrets(cmd) => match cmd {
Expand Down
46 changes: 0 additions & 46 deletions crates/bitwarden-json/src/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,38 +13,12 @@ use bitwarden::{
},
},
};
#[cfg(feature = "internal")]
use bitwarden::{
auth::login::{ApiKeyLoginRequest, PasswordLoginRequest},
platform::{FingerprintRequest, SecretVerificationRequest},
vault::SyncRequest,
};
use schemars::JsonSchema;
use serde::{Deserialize, Serialize};

#[derive(Serialize, Deserialize, JsonSchema, Debug)]
#[serde(rename_all = "camelCase", deny_unknown_fields)]
pub enum Command {
#[cfg(feature = "internal")]
/// Login with username and password
///
/// This command is for initiating an authentication handshake with Bitwarden.
/// Authorization may fail due to requiring 2fa or captcha challenge completion
/// despite accurate credentials.
///
/// This command is not capable of handling authentication requiring 2fa or captcha.
///
/// Returns: [PasswordLoginResponse](bitwarden::auth::login::PasswordLoginResponse)
PasswordLogin(PasswordLoginRequest),

#[cfg(feature = "internal")]
/// Login with API Key
///
/// This command is for initiating an authentication handshake with Bitwarden.
///
/// Returns: [ApiKeyLoginResponse](bitwarden::auth::login::ApiKeyLoginResponse)
ApiKeyLogin(ApiKeyLoginRequest),

#[cfg(feature = "secrets")]
/// Login with Secrets Manager Access Token
///
Expand All @@ -53,26 +27,6 @@ pub enum Command {
/// Returns: [ApiKeyLoginResponse](bitwarden::auth::login::ApiKeyLoginResponse)
LoginAccessToken(AccessTokenLoginRequest),

#[cfg(feature = "internal")]
/// > Requires Authentication
/// Get the API key of the currently authenticated user
///
/// Returns: [UserApiKeyResponse](bitwarden::platform::UserApiKeyResponse)
GetUserApiKey(SecretVerificationRequest),

#[cfg(feature = "internal")]
/// Get the user's passphrase
///
/// Returns: String
Fingerprint(FingerprintRequest),

#[cfg(feature = "internal")]
/// > Requires Authentication
/// Retrieve all user data, ciphers and organizations the user is a part of
///
/// Returns: [SyncResponse](bitwarden::vault::SyncResponse)
Sync(SyncRequest),

#[cfg(feature = "secrets")]
Secrets(SecretsCommand),
#[cfg(feature = "secrets")]
Expand Down
4 changes: 1 addition & 3 deletions crates/bitwarden-uniffi/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,13 @@ repository.workspace = true
license-file.workspace = true

[features]
docs = ["dep:schemars"] # Docs

[lib]
crate-type = ["lib", "staticlib", "cdylib"]
bench = false

[dependencies]
async-trait = "0.1.80"
bitwarden = { workspace = true, features = ["internal", "uniffi"] }
bitwarden-core = { workspace = true, features = ["uniffi"] }
bitwarden-crypto = { workspace = true, features = ["uniffi"] }
bitwarden-exporters = { workspace = true, features = ["uniffi"] }
Expand All @@ -28,8 +26,8 @@ bitwarden-generators = { workspace = true, features = ["uniffi"] }
bitwarden-send = { workspace = true, features = ["uniffi"] }
bitwarden-vault = { workspace = true, features = ["uniffi"] }
chrono = { workspace = true, features = ["std"] }
log = { workspace = true }
env_logger = "0.11.1"
log = { workspace = true }
schemars = { workspace = true, optional = true }
thiserror = { workspace = true }
uniffi = { workspace = true }
Expand Down
2 changes: 1 addition & 1 deletion crates/bitwarden-uniffi/src/auth/mod.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use std::sync::Arc;

use bitwarden::{
use bitwarden_core::{
auth::{
password::MasterPasswordPolicyOptions, AuthRequestResponse, KeyConnectorResponse,
RegisterKeyResponse, RegisterTdeKeyResponse,
Expand Down
2 changes: 1 addition & 1 deletion crates/bitwarden-uniffi/src/crypto.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use std::sync::Arc;

use bitwarden::{
use bitwarden_core::{
mobile::crypto::{
DeriveKeyConnectorRequest, DerivePinKeyResponse, InitOrgCryptoRequest,
InitUserCryptoRequest, UpdatePasswordResponse,
Expand Down
52 changes: 47 additions & 5 deletions crates/bitwarden-uniffi/src/error.rs
Original file line number Diff line number Diff line change
@@ -1,21 +1,24 @@
use std::fmt::{Display, Formatter};

use bitwarden_exporters::ExportError;
use bitwarden_generators::{PassphraseError, PasswordError, UsernameError};

// Name is converted from *Error to *Exception, so we can't just name the enum Error because
// Exception already exists
#[derive(uniffi::Error, Debug)]
#[uniffi(flat_error)]
pub enum BitwardenError {
E(bitwarden::error::Error),
E(Error),
}

impl From<bitwarden::Error> for BitwardenError {
fn from(e: bitwarden::Error) -> Self {
impl From<bitwarden_core::Error> for BitwardenError {
fn from(e: bitwarden_core::Error) -> Self {

Check warning on line 15 in crates/bitwarden-uniffi/src/error.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden-uniffi/src/error.rs#L15

Added line #L15 was not covered by tests
Self::E(e.into())
}
}

impl From<bitwarden::error::Error> for BitwardenError {
fn from(e: bitwarden::error::Error) -> Self {
impl From<Error> for BitwardenError {
fn from(e: Error) -> Self {

Check warning on line 21 in crates/bitwarden-uniffi/src/error.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden-uniffi/src/error.rs#L21

Added line #L21 was not covered by tests
Self::E(e)
}
}
Expand All @@ -37,3 +40,42 @@
}

pub type Result<T, E = BitwardenError> = std::result::Result<T, E>;

#[derive(thiserror::Error, Debug)]
pub enum Error {
#[error(transparent)]
Core(#[from] bitwarden_core::Error),
Copy link
Member

Choose a reason for hiding this comment

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

This will probably be a breaking change for the mobile apps, we will need to notify them before we make a new release.

Copy link
Member Author

Choose a reason for hiding this comment

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

True, I reverted it we should find some time to resolve the external error issue in uniffi.


// Generators
#[error(transparent)]
UsernameError(#[from] UsernameError),
#[error(transparent)]
PassphraseError(#[from] PassphraseError),
#[error(transparent)]
PasswordError(#[from] PasswordError),

// Vault
#[error(transparent)]
Cipher(#[from] bitwarden_vault::CipherError),
#[error(transparent)]
Totp(#[from] bitwarden_vault::TotpError),

#[error(transparent)]
ExportError(#[from] ExportError),

// Fido
#[error(transparent)]
MakeCredential(#[from] bitwarden_fido::MakeCredentialError),
#[error(transparent)]
GetAssertion(#[from] bitwarden_fido::GetAssertionError),
#[error(transparent)]
SilentlyDiscoverCredentials(#[from] bitwarden_fido::SilentlyDiscoverCredentialsError),
#[error(transparent)]
CredentialsForAutofillError(#[from] bitwarden_fido::CredentialsForAutofillError),
#[error(transparent)]
DecryptFido2AutofillCredentialsError(
#[from] bitwarden_fido::DecryptFido2AutofillCredentialsError,
),
#[error(transparent)]
Fido2Client(#[from] bitwarden_fido::Fido2ClientError),
}
10 changes: 5 additions & 5 deletions crates/bitwarden-uniffi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
use std::sync::Arc;

use auth::ClientAuth;
use bitwarden::ClientSettings;
use bitwarden_core::ClientSettings;

pub mod auth;
pub mod crypto;
Expand All @@ -23,7 +23,7 @@
use vault::ClientVault;

#[derive(uniffi::Object)]
pub struct Client(bitwarden::Client);
pub struct Client(bitwarden_core::Client);

#[uniffi::export(async_runtime = "tokio")]
impl Client {
Expand All @@ -35,7 +35,7 @@
#[cfg(target_os = "android")]
android_support::init();

Arc::new(Self(bitwarden::Client::new(settings)))
Arc::new(Self(bitwarden_core::Client::new(settings)))

Check warning on line 38 in crates/bitwarden-uniffi/src/lib.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden-uniffi/src/lib.rs#L38

Added line #L38 was not covered by tests
}

/// Crypto operations
Expand Down Expand Up @@ -84,9 +84,9 @@
.get(&url)
.send()
.await
.map_err(bitwarden::Error::Reqwest)?;
.map_err(bitwarden_core::Error::Reqwest)?;

Check warning on line 87 in crates/bitwarden-uniffi/src/lib.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden-uniffi/src/lib.rs#L87

Added line #L87 was not covered by tests

Ok(res.text().await.map_err(bitwarden::Error::Reqwest)?)
Ok(res.text().await.map_err(bitwarden_core::Error::Reqwest)?)

Check warning on line 89 in crates/bitwarden-uniffi/src/lib.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden-uniffi/src/lib.rs#L89

Added line #L89 was not covered by tests
}
}

Expand Down
41 changes: 20 additions & 21 deletions crates/bitwarden-uniffi/src/platform/fido2.rs
Original file line number Diff line number Diff line change
@@ -1,19 +1,18 @@
use std::sync::Arc;

use bitwarden::{
error::Error,
fido::{
CheckUserOptions, ClientData, ClientFido2Ext, Fido2CallbackError as BitFido2CallbackError,
GetAssertionRequest, GetAssertionResult, MakeCredentialRequest, MakeCredentialResult,
PublicKeyCredentialAuthenticatorAssertionResponse,
PublicKeyCredentialAuthenticatorAttestationResponse, PublicKeyCredentialRpEntity,
PublicKeyCredentialUserEntity,
},
vault::{Cipher, CipherView, Fido2CredentialNewView},
use bitwarden_fido::{
CheckUserOptions, ClientData, ClientFido2Ext, Fido2CallbackError as BitFido2CallbackError,
Fido2CredentialAutofillView, GetAssertionRequest, GetAssertionResult, MakeCredentialRequest,
MakeCredentialResult, Origin, PublicKeyCredentialAuthenticatorAssertionResponse,
PublicKeyCredentialAuthenticatorAttestationResponse, PublicKeyCredentialRpEntity,
PublicKeyCredentialUserEntity,
};
use bitwarden_fido::{Fido2CredentialAutofillView, Origin};
use bitwarden_vault::{Cipher, CipherView, Fido2CredentialNewView};

use crate::{error::Result, Client};
use crate::{
error::{Error, Result},
Client,
};

#[derive(uniffi::Object)]
pub struct ClientFido2(pub(crate) Arc<Client>);
Expand Down Expand Up @@ -180,7 +179,7 @@
user_verified: bool,
}

impl From<CheckUserResult> for bitwarden::fido::CheckUserResult {
impl From<CheckUserResult> for bitwarden_fido::CheckUserResult {
fn from(val: CheckUserResult) -> Self {
Self {
user_present: val.user_present,
Expand Down Expand Up @@ -268,7 +267,7 @@
struct UniffiTraitBridge<T>(T);

#[async_trait::async_trait]
impl bitwarden::fido::Fido2CredentialStore for UniffiTraitBridge<&dyn Fido2CredentialStore> {
impl bitwarden_fido::Fido2CredentialStore for UniffiTraitBridge<&dyn Fido2CredentialStore> {
async fn find_credentials(
&self,
ids: Option<Vec<Vec<u8>>>,
Expand Down Expand Up @@ -306,9 +305,9 @@
RequestExistingCredential(CipherView),
}

impl From<bitwarden::fido::UIHint<'_, CipherView>> for UIHint {
fn from(hint: bitwarden::fido::UIHint<'_, CipherView>) -> Self {
use bitwarden::fido::UIHint as BWUIHint;
impl From<bitwarden_fido::UIHint<'_, CipherView>> for UIHint {
fn from(hint: bitwarden_fido::UIHint<'_, CipherView>) -> Self {

Check warning on line 309 in crates/bitwarden-uniffi/src/platform/fido2.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden-uniffi/src/platform/fido2.rs#L309

Added line #L309 was not covered by tests
use bitwarden_fido::UIHint as BWUIHint;
match hint {
BWUIHint::InformExcludedCredentialFound(cipher) => {
UIHint::InformExcludedCredentialFound(cipher.clone())
Expand All @@ -333,12 +332,12 @@
}

#[async_trait::async_trait]
impl bitwarden::fido::Fido2UserInterface for UniffiTraitBridge<&dyn Fido2UserInterface> {
impl bitwarden_fido::Fido2UserInterface for UniffiTraitBridge<&dyn Fido2UserInterface> {
async fn check_user<'a>(
&self,
options: CheckUserOptions,
hint: bitwarden::fido::UIHint<'a, CipherView>,
) -> Result<bitwarden::fido::CheckUserResult, BitFido2CallbackError> {
hint: bitwarden_fido::UIHint<'a, CipherView>,
) -> Result<bitwarden_fido::CheckUserResult, BitFido2CallbackError> {

Check warning on line 340 in crates/bitwarden-uniffi/src/platform/fido2.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden-uniffi/src/platform/fido2.rs#L340

Added line #L340 was not covered by tests
self.0
.check_user(options.clone(), hint.into())
.await
Expand All @@ -359,7 +358,7 @@
&self,
options: CheckUserOptions,
new_credential: Fido2CredentialNewView,
) -> Result<(CipherView, bitwarden::fido::CheckUserResult), BitFido2CallbackError> {
) -> Result<(CipherView, bitwarden_fido::CheckUserResult), BitFido2CallbackError> {

Check warning on line 361 in crates/bitwarden-uniffi/src/platform/fido2.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden-uniffi/src/platform/fido2.rs#L361

Added line #L361 was not covered by tests
self.0
.check_user_and_pick_credential_for_creation(options, new_credential)
.await
Expand Down
Loading
Loading