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

Add require! macro to include field information in MissingFields error #686

Merged
merged 4 commits into from
Apr 1, 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
10 changes: 5 additions & 5 deletions crates/bitwarden/src/admin_console/policy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
use serde_repr::{Deserialize_repr, Serialize_repr};
use uuid::Uuid;

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

#[derive(Serialize, Deserialize, Debug, JsonSchema)]
pub struct Policy {
Expand Down Expand Up @@ -41,11 +41,11 @@

fn try_from(policy: PolicyResponseModel) -> Result<Self> {
Ok(Self {
id: policy.id.ok_or(Error::MissingFields)?,
organization_id: policy.organization_id.ok_or(Error::MissingFields)?,
r#type: policy.r#type.ok_or(Error::MissingFields)?.into(),
id: require!(policy.id),
organization_id: require!(policy.organization_id),
r#type: require!(policy.r#type).into(),

Check warning on line 46 in crates/bitwarden/src/admin_console/policy.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden/src/admin_console/policy.rs#L44-L46

Added lines #L44 - L46 were not covered by tests
data: policy.data,
enabled: policy.enabled.ok_or(Error::MissingFields)?,
enabled: require!(policy.enabled),

Check warning on line 48 in crates/bitwarden/src/admin_console/policy.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden/src/admin_console/policy.rs#L48

Added line #L48 was not covered by tests
})
}
}
Expand Down
6 changes: 2 additions & 4 deletions crates/bitwarden/src/auth/login/access_token.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use crate::{
AccessToken, JWTToken,
},
client::{LoginMethod, ServiceAccountLoginMethod},
error::{Error, Result},
error::{require, Error, Result},
secrets_manager::state::{self, ClientState},
Client,
};
Expand Down Expand Up @@ -69,9 +69,7 @@ pub(crate) async fn login_access_token(
let access_token_obj: JWTToken = r.access_token.parse()?;

// This should always be Some() when logging in with an access token
let organization_id = access_token_obj
.organization
.ok_or(Error::MissingFields)?
let organization_id = require!(access_token_obj.organization)
.parse()
.map_err(|_| Error::InvalidResponse)?;

Expand Down
10 changes: 3 additions & 7 deletions crates/bitwarden/src/auth/login/api_key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
JWTToken,
},
client::{LoginMethod, UserLoginMethod},
error::{Error, Result},
error::{require, Result},
Client,
};

Expand Down Expand Up @@ -44,12 +44,8 @@
kdf,
}));

let user_key: EncString = r.key.as_deref().ok_or(Error::MissingFields)?.parse()?;
let private_key: EncString = r
.private_key
.as_deref()
.ok_or(Error::MissingFields)?
.parse()?;
let user_key: EncString = require!(r.key.as_deref()).parse()?;
let private_key: EncString = require!(r.private_key.as_deref()).parse()?;

Check warning on line 48 in crates/bitwarden/src/auth/login/api_key.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden/src/auth/login/api_key.rs#L47-L48

Added lines #L47 - L48 were not covered by tests

client.initialize_user_crypto(&input.password, user_key, private_key)?;
}
Expand Down
12 changes: 6 additions & 6 deletions crates/bitwarden/src/auth/login/auth_request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
auth_request::new_auth_request,
},
client::{LoginMethod, UserLoginMethod},
error::{Error, Result},
error::{require, Result},
mobile::crypto::{AuthRequestMethod, InitUserCryptoMethod, InitUserCryptoRequest},
Client,
};
Expand Down Expand Up @@ -50,7 +50,7 @@
fingerprint: auth.fingerprint,
email,
device_identifier,
auth_request_id: res.id.ok_or(Error::MissingFields)?,
auth_request_id: require!(res.id),

Check warning on line 53 in crates/bitwarden/src/auth/login/auth_request.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden/src/auth/login/auth_request.rs#L53

Added line #L53 was not covered by tests
access_code: auth.access_code,
private_key: auth.private_key,
})
Expand Down Expand Up @@ -103,11 +103,11 @@

let method = match res.master_password_hash {
Some(_) => AuthRequestMethod::MasterKey {
protected_master_key: res.key.ok_or(Error::MissingFields)?.parse()?,
auth_request_key: r.key.ok_or(Error::MissingFields)?.parse()?,
protected_master_key: require!(res.key).parse()?,
auth_request_key: require!(r.key).parse()?,

Check warning on line 107 in crates/bitwarden/src/auth/login/auth_request.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden/src/auth/login/auth_request.rs#L106-L107

Added lines #L106 - L107 were not covered by tests
},
None => AuthRequestMethod::UserKey {
protected_user_key: res.key.ok_or(Error::MissingFields)?.parse()?,
protected_user_key: require!(res.key).parse()?,

Check warning on line 110 in crates/bitwarden/src/auth/login/auth_request.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden/src/auth/login/auth_request.rs#L110

Added line #L110 was not covered by tests
},
};

Expand All @@ -116,7 +116,7 @@
.initialize_user_crypto(InitUserCryptoRequest {
kdf_params: kdf,
email: auth_req.email,
private_key: r.private_key.ok_or(Error::MissingFields)?,
private_key: require!(r.private_key),

Check warning on line 119 in crates/bitwarden/src/auth/login/auth_request.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden/src/auth/login/auth_request.rs#L119

Added line #L119 was not covered by tests
method: InitUserCryptoMethod::AuthRequest {
request_private_key: auth_req.private_key,
method,
Expand Down
10 changes: 3 additions & 7 deletions crates/bitwarden/src/auth/login/password.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
) -> Result<PasswordLoginResponse> {
use bitwarden_crypto::{EncString, HashPurpose};

use crate::{auth::determine_password_hash, client::UserLoginMethod, error::Error};
use crate::{auth::determine_password_hash, client::UserLoginMethod, error::require};

Check warning on line 27 in crates/bitwarden/src/auth/login/password.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden/src/auth/login/password.rs#L27

Added line #L27 was not covered by tests

info!("password logging in");
debug!("{:#?}, {:#?}", client, input);
Expand All @@ -49,12 +49,8 @@
kdf: input.kdf.to_owned(),
}));

let user_key: EncString = r.key.as_deref().ok_or(Error::MissingFields)?.parse()?;
let private_key: EncString = r
.private_key
.as_deref()
.ok_or(Error::MissingFields)?
.parse()?;
let user_key: EncString = require!(r.key.as_deref()).parse()?;
let private_key: EncString = require!(r.private_key.as_deref()).parse()?;

Check warning on line 53 in crates/bitwarden/src/auth/login/password.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden/src/auth/login/password.rs#L52-L53

Added lines #L52 - L53 were not covered by tests

client.initialize_user_crypto(&input.password, user_key, private_key)?;
}
Expand Down
18 changes: 16 additions & 2 deletions crates/bitwarden/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ pub enum Error {

#[error("The response received was invalid and could not be processed")]
InvalidResponse,
#[error("The response received was missing some of the required fields")]
MissingFields,
#[error("The response received was missing some of the required fields: {0}")]
MissingFields(&'static str),

#[error("Cryptography error, {0}")]
Crypto(#[from] bitwarden_crypto::CryptoError),
Expand Down Expand Up @@ -133,4 +133,18 @@ macro_rules! impl_bitwarden_error {
impl_bitwarden_error!(ApiError);
impl_bitwarden_error!(IdentityError);

/// This macro is used to require that a value is present or return an error otherwise.
/// It is equivalent to using `val.ok_or(Error::MissingFields)?`, but easier to use and
/// with a more descriptive error message.
/// Note that this macro will return early from the function if the value is not present.
macro_rules! require {
Copy link
Member

Choose a reason for hiding this comment

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

Let's add some documentation here.

($val:expr) => {
match $val {
Some(val) => val,
None => return Err($crate::error::Error::MissingFields(stringify!($val))),
}
};
}
pub(crate) use require;

pub type Result<T, E = Error> = std::result::Result<T, E>;
8 changes: 4 additions & 4 deletions crates/bitwarden/src/platform/domain.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use schemars::JsonSchema;
use serde::{Deserialize, Serialize};

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

#[derive(Serialize, Deserialize, Debug, JsonSchema)]
pub struct GlobalDomains {
Expand All @@ -15,9 +15,9 @@

fn try_from(global_domains: bitwarden_api_api::models::GlobalDomains) -> Result<Self> {
Ok(Self {
r#type: global_domains.r#type.ok_or(Error::MissingFields)?,
domains: global_domains.domains.ok_or(Error::MissingFields)?,
excluded: global_domains.excluded.ok_or(Error::MissingFields)?,
r#type: require!(global_domains.r#type),
domains: require!(global_domains.domains),
excluded: require!(global_domains.excluded),

Check warning on line 20 in crates/bitwarden/src/platform/domain.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden/src/platform/domain.rs#L18-L20

Added lines #L18 - L20 were not covered by tests
})
}
}
8 changes: 3 additions & 5 deletions crates/bitwarden/src/platform/get_user_api_key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
use super::SecretVerificationRequest;
use crate::{
client::{LoginMethod, UserLoginMethod},
error::{Error, Result},
error::{require, Error, Result},
Client,
};

Expand Down Expand Up @@ -75,9 +75,7 @@

impl UserApiKeyResponse {
pub(crate) fn process_response(response: ApiKeyResponseModel) -> Result<UserApiKeyResponse> {
match response.api_key {
Some(api_key) => Ok(UserApiKeyResponse { api_key }),
None => Err(Error::MissingFields),
}
let api_key = require!(response.api_key);
Ok(UserApiKeyResponse { api_key })

Check warning on line 79 in crates/bitwarden/src/platform/get_user_api_key.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden/src/platform/get_user_api_key.rs#L78-L79

Added lines #L78 - L79 were not covered by tests
}
}
29 changes: 13 additions & 16 deletions crates/bitwarden/src/platform/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
use crate::{
admin_console::Policy,
client::{encryption_settings::EncryptionSettings, Client},
error::{Error, Result},
error::{require, Error, Result},
vault::{Cipher, Collection, Folder},
};

Expand All @@ -25,10 +25,7 @@
let sync =
bitwarden_api_api::apis::sync_api::sync_get(&config.api, input.exclude_subdomains).await?;

let org_keys: Vec<_> = sync
.profile
.as_ref()
.ok_or(Error::MissingFields)?
let org_keys: Vec<_> = require!(sync.profile.as_ref())

Check warning on line 28 in crates/bitwarden/src/platform/sync.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden/src/platform/sync.rs#L28

Added line #L28 was not covered by tests
.organizations
.as_deref()
.unwrap_or_default()
Expand Down Expand Up @@ -86,8 +83,8 @@
response: SyncResponseModel,
enc: &EncryptionSettings,
) -> Result<SyncResponse> {
let profile = *response.profile.ok_or(Error::MissingFields)?;
let ciphers = response.ciphers.ok_or(Error::MissingFields)?;
let profile = require!(response.profile);
let ciphers = require!(response.ciphers);

Check warning on line 87 in crates/bitwarden/src/platform/sync.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden/src/platform/sync.rs#L86-L87

Added lines #L86 - L87 were not covered by tests

fn try_into_iter<In, InItem, Out, OutItem>(iter: In) -> Result<Out, InItem::Error>
where
Expand All @@ -99,13 +96,13 @@
}

Ok(SyncResponse {
profile: ProfileResponse::process_response(profile, enc)?,
folders: try_into_iter(response.folders.ok_or(Error::MissingFields)?)?,
collections: try_into_iter(response.collections.ok_or(Error::MissingFields)?)?,
profile: ProfileResponse::process_response(*profile, enc)?,
folders: try_into_iter(require!(response.folders))?,
collections: try_into_iter(require!(response.collections))?,

Check warning on line 101 in crates/bitwarden/src/platform/sync.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden/src/platform/sync.rs#L99-L101

Added lines #L99 - L101 were not covered by tests
ciphers: try_into_iter(ciphers)?,
domains: response.domains.map(|d| (*d).try_into()).transpose()?,
policies: try_into_iter(response.policies.ok_or(Error::MissingFields)?)?,
sends: try_into_iter(response.sends.ok_or(Error::MissingFields)?)?,
policies: try_into_iter(require!(response.policies))?,
sends: try_into_iter(require!(response.sends))?,

Check warning on line 105 in crates/bitwarden/src/platform/sync.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden/src/platform/sync.rs#L104-L105

Added lines #L104 - L105 were not covered by tests
})
}
}
Expand All @@ -115,7 +112,7 @@
response: ProfileOrganizationResponseModel,
) -> Result<ProfileOrganizationResponse> {
Ok(ProfileOrganizationResponse {
id: response.id.ok_or(Error::MissingFields)?,
id: require!(response.id),

Check warning on line 115 in crates/bitwarden/src/platform/sync.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden/src/platform/sync.rs#L115

Added line #L115 was not covered by tests
})
}
}
Expand All @@ -126,9 +123,9 @@
_enc: &EncryptionSettings,
) -> Result<ProfileResponse> {
Ok(ProfileResponse {
id: response.id.ok_or(Error::MissingFields)?,
name: response.name.ok_or(Error::MissingFields)?,
email: response.email.ok_or(Error::MissingFields)?,
id: require!(response.id),
name: require!(response.name),
email: require!(response.email),

Check warning on line 128 in crates/bitwarden/src/platform/sync.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden/src/platform/sync.rs#L126-L128

Added lines #L126 - L128 were not covered by tests
//key: response.key,
//private_key: response.private_key,
organizations: response
Expand Down
4 changes: 2 additions & 2 deletions crates/bitwarden/src/secrets_manager/projects/delete.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

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

#[derive(Serialize, Deserialize, Debug, JsonSchema)]
Expand Down Expand Up @@ -62,7 +62,7 @@
response: BulkDeleteResponseModel,
) -> Result<ProjectDeleteResponse> {
Ok(ProjectDeleteResponse {
id: response.id.ok_or(Error::MissingFields)?,
id: require!(response.id),

Check warning on line 65 in crates/bitwarden/src/secrets_manager/projects/delete.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden/src/secrets_manager/projects/delete.rs#L65

Added line #L65 was not covered by tests
error: response.error,
})
}
Expand Down
20 changes: 6 additions & 14 deletions crates/bitwarden/src/secrets_manager/projects/project_response.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

use crate::{
client::encryption_settings::EncryptionSettings,
error::{Error, Result},
error::{require, Result},
};

#[derive(Serialize, Deserialize, Debug, JsonSchema)]
Expand All @@ -25,27 +25,19 @@
response: ProjectResponseModel,
enc: &EncryptionSettings,
) -> Result<Self> {
let organization_id = response.organization_id.ok_or(Error::MissingFields)?;
let organization_id = require!(response.organization_id);

Check warning on line 28 in crates/bitwarden/src/secrets_manager/projects/project_response.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden/src/secrets_manager/projects/project_response.rs#L28

Added line #L28 was not covered by tests

let name = response
.name
.ok_or(Error::MissingFields)?
let name = require!(response.name)

Check warning on line 30 in crates/bitwarden/src/secrets_manager/projects/project_response.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden/src/secrets_manager/projects/project_response.rs#L30

Added line #L30 was not covered by tests
.parse::<EncString>()?
.decrypt(enc, &Some(organization_id))?;

Ok(ProjectResponse {
id: response.id.ok_or(Error::MissingFields)?,
id: require!(response.id),

Check warning on line 35 in crates/bitwarden/src/secrets_manager/projects/project_response.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden/src/secrets_manager/projects/project_response.rs#L35

Added line #L35 was not covered by tests
organization_id,
name,

creation_date: response
.creation_date
.ok_or(Error::MissingFields)?
.parse()?,
revision_date: response
.revision_date
.ok_or(Error::MissingFields)?
.parse()?,
creation_date: require!(response.creation_date).parse()?,
revision_date: require!(response.revision_date).parse()?,

Check warning on line 40 in crates/bitwarden/src/secrets_manager/projects/project_response.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden/src/secrets_manager/projects/project_response.rs#L39-L40

Added lines #L39 - L40 were not covered by tests
})
}
}
4 changes: 2 additions & 2 deletions crates/bitwarden/src/secrets_manager/secrets/delete.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

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

#[derive(Serialize, Deserialize, Debug, JsonSchema)]
Expand Down Expand Up @@ -62,7 +62,7 @@
response: BulkDeleteResponseModel,
) -> Result<SecretDeleteResponse> {
Ok(SecretDeleteResponse {
id: response.id.ok_or(Error::MissingFields)?,
id: require!(response.id),

Check warning on line 65 in crates/bitwarden/src/secrets_manager/secrets/delete.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden/src/secrets_manager/secrets/delete.rs#L65

Added line #L65 was not covered by tests
error: response.error,
})
}
Expand Down
10 changes: 4 additions & 6 deletions crates/bitwarden/src/secrets_manager/secrets/list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use uuid::Uuid;

use crate::{
client::{encryption_settings::EncryptionSettings, Client},
error::{Error, Result},
error::{require, Result},
};

#[derive(Serialize, Deserialize, Debug, JsonSchema)]
Expand Down Expand Up @@ -93,16 +93,14 @@ impl SecretIdentifierResponse {
response: SecretsWithProjectsInnerSecret,
enc: &EncryptionSettings,
) -> Result<SecretIdentifierResponse> {
let organization_id = response.organization_id.ok_or(Error::MissingFields)?;
let organization_id = require!(response.organization_id);

let key = response
.key
.ok_or(Error::MissingFields)?
let key = require!(response.key)
.parse::<EncString>()?
.decrypt(enc, &Some(organization_id))?;

Ok(SecretIdentifierResponse {
id: response.id.ok_or(Error::MissingFields)?,
id: require!(response.id),
organization_id,
key,
})
Expand Down
Loading
Loading