Add require! macro to include field information in MissingFields error (

#686)

## Type of change
```
- [ ] Bug fix
- [ ] New feature development
- [x] Tech debt (refactoring, code cleanup, dependency upgrades, etc)
- [ ] Build/deploy pipeline (DevOps)
- [ ] Other
```

## Objective
Previously the MissingFields error didn't contain any usable
information, plus it was unwieldy to use. With the use of a simple macro
we can improve the error messaging and also simplify the error handling
code.

crates/bitwarden/src/admin_console/policy.rs

@@ -6,7 +6,7 @@ use serde::{Deserialize, Serialize};
 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 {
@@ -41,11 +41,11 @@ impl TryFrom<PolicyResponseModel> for Policy {
 
     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(),
             data: policy.data,
-            enabled: policy.enabled.ok_or(Error::MissingFields)?,
+            enabled: require!(policy.enabled),
         })
     }
 }

crates/bitwarden/src/auth/login/access_token.rs

@@ -14,7 +14,7 @@ use crate::{
         AccessToken, JWTToken,
     },
     client::{LoginMethod, ServiceAccountLoginMethod},
-    error::{Error, Result},
+    error::{require, Error, Result},
     secrets_manager::state::{self, ClientState},
     Client,
 };
@@ -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)?;
 

crates/bitwarden/src/auth/login/api_key.rs

@@ -9,7 +9,7 @@ use crate::{
         JWTToken,
     },
     client::{LoginMethod, UserLoginMethod},
-    error::{Error, Result},
+    error::{require, Result},
     Client,
 };
 
@@ -44,12 +44,8 @@ pub(crate) async fn login_api_key(
             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()?;
 
         client.initialize_user_crypto(&input.password, user_key, private_key)?;
     }

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

@@ -13,7 +13,7 @@ use crate::{
         auth_request::new_auth_request,
     },
     client::{LoginMethod, UserLoginMethod},
-    error::{Error, Result},
+    error::{require, Result},
     mobile::crypto::{AuthRequestMethod, InitUserCryptoMethod, InitUserCryptoRequest},
     Client,
 };
@@ -50,7 +50,7 @@ pub(crate) async fn send_new_auth_request(
         fingerprint: auth.fingerprint,
         email,
         device_identifier,
-        auth_request_id: res.id.ok_or(Error::MissingFields)?,
+        auth_request_id: require!(res.id),
         access_code: auth.access_code,
         private_key: auth.private_key,
     })
@@ -103,11 +103,11 @@ pub(crate) async fn complete_auth_request(
 
         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()?,
             },
             None => AuthRequestMethod::UserKey {
-                protected_user_key: res.key.ok_or(Error::MissingFields)?.parse()?,
+                protected_user_key: require!(res.key).parse()?,
             },
         };
 
@@ -116,7 +116,7 @@ pub(crate) async fn complete_auth_request(
             .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),
                 method: InitUserCryptoMethod::AuthRequest {
                     request_private_key: auth_req.private_key,
                     method,

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

@@ -24,7 +24,7 @@ pub(crate) async fn login_password(
 ) -> 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};
 
     info!("password logging in");
     debug!("{:#?}, {:#?}", client, input);
@@ -49,12 +49,8 @@ pub(crate) async fn login_password(
             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()?;
 
         client.initialize_user_crypto(&input.password, user_key, private_key)?;
     }

crates/bitwarden/src/error.rs

@@ -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),
@@ -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 {
+    ($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>;

crates/bitwarden/src/platform/domain.rs

@@ -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 {
@@ -15,9 +15,9 @@ impl TryFrom<bitwarden_api_api::models::GlobalDomains> for GlobalDomains {
 
     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),
         })
     }
 }

crates/bitwarden/src/platform/get_user_api_key.rs

@@ -10,7 +10,7 @@ use serde::{Deserialize, Serialize};
 use super::SecretVerificationRequest;
 use crate::{
     client::{LoginMethod, UserLoginMethod},
-    error::{Error, Result},
+    error::{require, Error, Result},
     Client,
 };
 
@@ -75,9 +75,7 @@ pub struct UserApiKeyResponse {
 
 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 })
     }
 }

crates/bitwarden/src/platform/sync.rs

@@ -9,7 +9,7 @@ use super::domain::GlobalDomains;
 use crate::{
     admin_console::Policy,
     client::{encryption_settings::EncryptionSettings, Client},
-    error::{Error, Result},
+    error::{require, Error, Result},
     vault::{Cipher, Collection, Folder},
 };
 
@@ -25,10 +25,7 @@ pub(crate) async fn sync(client: &mut Client, input: &SyncRequest) -> Result<Syn
     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())
         .organizations
         .as_deref()
         .unwrap_or_default()
@@ -86,8 +83,8 @@ impl SyncResponse {
         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);
 
         fn try_into_iter<In, InItem, Out, OutItem>(iter: In) -> Result<Out, InItem::Error>
         where
@@ -99,13 +96,13 @@ impl SyncResponse {
         }
 
         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))?,
             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))?,
         })
     }
 }
@@ -115,7 +112,7 @@ impl ProfileOrganizationResponse {
         response: ProfileOrganizationResponseModel,
     ) -> Result<ProfileOrganizationResponse> {
         Ok(ProfileOrganizationResponse {
-            id: response.id.ok_or(Error::MissingFields)?,
+            id: require!(response.id),
         })
     }
 }
@@ -126,9 +123,9 @@ impl ProfileResponse {
         _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),
             //key: response.key,
             //private_key: response.private_key,
             organizations: response

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

@@ -7,7 +7,7 @@ use uuid::Uuid;
 
 use crate::{
     client::Client,
-    error::{Error, Result},
+    error::{require, Result},
 };
 
 #[derive(Serialize, Deserialize, Debug, JsonSchema)]
@@ -62,7 +62,7 @@ impl ProjectDeleteResponse {
         response: BulkDeleteResponseModel,
     ) -> Result<ProjectDeleteResponse> {
         Ok(ProjectDeleteResponse {
-            id: response.id.ok_or(Error::MissingFields)?,
+            id: require!(response.id),
             error: response.error,
         })
     }

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

@@ -7,7 +7,7 @@ use uuid::Uuid;
 
 use crate::{
     client::encryption_settings::EncryptionSettings,
-    error::{Error, Result},
+    error::{require, Result},
 };
 
 #[derive(Serialize, Deserialize, Debug, JsonSchema)]
@@ -25,27 +25,19 @@ impl ProjectResponse {
         response: ProjectResponseModel,
         enc: &EncryptionSettings,
     ) -> Result<Self> {
-        let organization_id = response.organization_id.ok_or(Error::MissingFields)?;
+        let organization_id = require!(response.organization_id);
 
-        let name = response
-            .name
-            .ok_or(Error::MissingFields)?
+        let name = require!(response.name)
             .parse::<EncString>()?
             .decrypt(enc, &Some(organization_id))?;
 
         Ok(ProjectResponse {
-            id: response.id.ok_or(Error::MissingFields)?,
+            id: require!(response.id),
             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()?,
         })
     }
 }

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

@@ -7,7 +7,7 @@ use uuid::Uuid;
 
 use crate::{
     client::Client,
-    error::{Error, Result},
+    error::{require, Result},
 };
 
 #[derive(Serialize, Deserialize, Debug, JsonSchema)]
@@ -62,7 +62,7 @@ impl SecretDeleteResponse {
         response: BulkDeleteResponseModel,
     ) -> Result<SecretDeleteResponse> {
         Ok(SecretDeleteResponse {
-            id: response.id.ok_or(Error::MissingFields)?,
+            id: require!(response.id),
             error: response.error,
         })
     }

crates/bitwarden/src/secrets_manager/secrets/list.rs

@@ -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)]
@@ -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,
         })