Fix some clippy lints (#139)

Co-authored-by: Oscar Hinton <[email protected]>

.github/workflows/lint.yml

@@ -47,3 +47,8 @@ jobs:
         run: cargo doc --no-deps --features internal
         env:
           RUSTDOCFLAGS: "-D warnings"
+
+      - name: Cargo clippy
+        run: cargo clippy --all-features
+        env:
+          RUSTFLAGS: "-D warnings"

crates/bitwarden-c/src/c.rs

@@ -15,23 +15,23 @@ pub async extern "C" fn run_command(
     println!("{}", input_str);
 
     let result = client.run_command(input_str).await;
-    return match std::ffi::CString::new(result) {
+    match std::ffi::CString::new(result) {
         Ok(cstr) => cstr.into_raw(),
         Err(_) => panic!("failed to return command result: null encountered"),
-    };
+    }
 }
 
 // Init client, potential leak! You need to call free_mem after this!
 #[no_mangle]
 pub extern "C" fn init(c_str_ptr: *const c_char) -> *mut Client {
     env_logger::init();
     if c_str_ptr.is_null() {
-        return box_ptr!(Client::new(None));
+        box_ptr!(Client::new(None))
     } else {
         let input_string = str::from_utf8(unsafe { CStr::from_ptr(c_str_ptr).to_bytes() })
             .unwrap()
             .to_owned();
-        return box_ptr!(Client::new(Some(input_string)));
+        box_ptr!(Client::new(Some(input_string)))
     }
 }
 

crates/bitwarden-c/src/lib.rs

@@ -1,3 +1,6 @@
+// These are the C bindings, we're going to have to use unsafe raw pointers
+#![allow(clippy::not_unsafe_ptr_arg_deref)]
+
 #[cfg(not(target_arch = "wasm32"))]
 pub use c::*;
 

crates/bitwarden-napi/src/client.rs

@@ -38,6 +38,6 @@ impl BitwardenClient {
 
     #[napi]
     pub async unsafe fn run_command(&mut self, command_input: String) -> String {
-        self.0.run_command(&command_input).await.into()
+        self.0.run_command(&command_input).await
     }
 }

crates/bitwarden-wasm/src/client.rs

@@ -50,6 +50,9 @@ impl BitwardenClient {
     #[wasm_bindgen]
     pub fn run_command(&mut self, js_input: String) -> Promise {
         let rc = self.0.clone();
+        // TODO: We should probably switch to an async-aware RwLock here,
+        // but it probably doesn't matter much in a single threaded environment
+        #[allow(clippy::await_holding_lock)]
         future_to_promise(async move {
             let mut client = rc.write().unwrap();
             let result = client.run_command(&js_input).await;

crates/bitwarden/src/auth/api/request/mod.rs

@@ -58,5 +58,5 @@ async fn send_identity_connect_request(
     let status = response.status();
     let text = response.text().await?;
 
-    parse_identity_response(status, &text)
+    parse_identity_response(status, text)
 }

crates/bitwarden/src/auth/api/request/renew_token_request.rs

@@ -11,12 +11,11 @@ pub struct RenewTokenRequest {
 
 impl RenewTokenRequest {
     pub fn new(refresh_token: String, client_id: String) -> Self {
-        let obj = Self {
+        Self {
             refresh_token,
             client_id,
             grant_type: "refresh_token".to_string(),
-        };
-        obj
+        }
     }
 
     pub(crate) async fn send(

crates/bitwarden/src/auth/api/response/identity_token_response.rs

@@ -14,30 +14,30 @@ pub enum IdentityTokenResponse {
     Authenticated(IdentityTokenSuccessResponse),
     Payload(IdentityTokenPayloadResponse),
     Refreshed(IdentityTokenRefreshResponse),
-    TwoFactorRequired(IdentityTwoFactorResponse),
+    TwoFactorRequired(Box<IdentityTwoFactorResponse>),
     CaptchaRequired(IdentityCaptchaResponse),
 }
 
 pub fn parse_identity_response(
     status: StatusCode,
-    response: &str,
+    response: String,
 ) -> Result<IdentityTokenResponse> {
-    if let Ok(r) = serde_json::from_str::<IdentityTokenSuccessResponse>(response) {
+    if let Ok(r) = serde_json::from_str::<IdentityTokenSuccessResponse>(&response) {
         Ok(IdentityTokenResponse::Authenticated(r))
-    } else if let Ok(r) = serde_json::from_str::<IdentityTokenPayloadResponse>(response) {
+    } else if let Ok(r) = serde_json::from_str::<IdentityTokenPayloadResponse>(&response) {
         Ok(IdentityTokenResponse::Payload(r))
-    } else if let Ok(r) = serde_json::from_str::<IdentityTokenRefreshResponse>(response) {
+    } else if let Ok(r) = serde_json::from_str::<IdentityTokenRefreshResponse>(&response) {
         Ok(IdentityTokenResponse::Refreshed(r))
-    } else if let Ok(r) = serde_json::from_str::<IdentityTwoFactorResponse>(response) {
-        Ok(IdentityTokenResponse::TwoFactorRequired(r))
-    } else if let Ok(r) = serde_json::from_str::<IdentityCaptchaResponse>(response) {
+    } else if let Ok(r) = serde_json::from_str::<IdentityTwoFactorResponse>(&response) {
+        Ok(IdentityTokenResponse::TwoFactorRequired(Box::new(r)))
+    } else if let Ok(r) = serde_json::from_str::<IdentityCaptchaResponse>(&response) {
         Ok(IdentityTokenResponse::CaptchaRequired(r))
-    } else if let Ok(r) = serde_json::from_str::<IdentityTokenFailResponse>(response) {
+    } else if let Ok(r) = serde_json::from_str::<IdentityTokenFailResponse>(&response) {
         Err(Error::IdentityFail(r))
     } else {
         Err(Error::ResponseContent {
-            status: status,
-            message: response.to_owned(),
+            status,
+            message: response,
         })
     }
 }
@@ -51,16 +51,16 @@ mod test {
         let expected = IdentityTokenSuccessResponse::default();
         let success = serde_json::to_string(&expected).unwrap();
         let expected = IdentityTokenResponse::Authenticated(expected);
-        let actual = parse_identity_response(StatusCode::OK, &success).unwrap();
+        let actual = parse_identity_response(StatusCode::OK, success).unwrap();
         assert_eq!(expected, actual);
     }
 
     #[test]
     fn two_factor() {
-        let expected = IdentityTwoFactorResponse::default();
+        let expected = Box::new(IdentityTwoFactorResponse::default());
         let two_factor = serde_json::to_string(&expected).unwrap();
         let expected = IdentityTokenResponse::TwoFactorRequired(expected);
-        let actual = parse_identity_response(StatusCode::BAD_REQUEST, &two_factor).unwrap();
+        let actual = parse_identity_response(StatusCode::BAD_REQUEST, two_factor).unwrap();
         assert_eq!(expected, actual);
     }
 
@@ -69,7 +69,7 @@ mod test {
         let expected = IdentityCaptchaResponse::default();
         let captcha = serde_json::to_string(&expected).unwrap();
         let expected = IdentityTokenResponse::CaptchaRequired(expected);
-        let actual = parse_identity_response(StatusCode::BAD_REQUEST, &captcha).unwrap();
+        let actual = parse_identity_response(StatusCode::BAD_REQUEST, captcha).unwrap();
         assert_eq!(expected, actual);
     }
 }

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

@@ -133,7 +133,7 @@ pub(crate) async fn access_token_login(
 
         let payload: Payload = serde_json::from_slice(&decrypted_payload)?;
 
-        let encryption_key = BASE64_ENGINE.decode(&payload.encryption_key)?;
+        let encryption_key = BASE64_ENGINE.decode(payload.encryption_key)?;
 
         let encryption_key = SymmetricCryptoKey::try_from(encryption_key.as_slice())?;
 
@@ -192,7 +192,7 @@ async fn request_identity_tokens(
 ) -> Result<IdentityTokenResponse> {
     let config = client.get_api_configurations().await;
     PasswordTokenRequest::new(&input.email, password_hash)
-        .send(&config)
+        .send(config)
         .await
 }
 
@@ -203,7 +203,7 @@ async fn request_api_identity_tokens(
 ) -> Result<IdentityTokenResponse> {
     let config = client.get_api_configurations().await;
     ApiTokenRequest::new(&input.client_id, &input.client_secret)
-        .send(&config)
+        .send(config)
         .await
 }
 
@@ -213,7 +213,7 @@ async fn request_access_token(
 ) -> Result<IdentityTokenResponse> {
     let config = client.get_api_configurations().await;
     AccessTokenRequest::new(input.service_account_id, &input.client_secret)
-        .send(&config)
+        .send(config)
         .await
 }
 
@@ -254,7 +254,7 @@ pub(crate) async fn renew_token(client: &mut Client) -> Result<()> {
                 client_secret,
                 ..
             } => {
-                AccessTokenRequest::new(*service_account_id, &client_secret)
+                AccessTokenRequest::new(*service_account_id, client_secret)
                     .send(&client.__api_configurations)
                     .await?
             }

crates/bitwarden/src/client/access_token.rs

@@ -46,7 +46,7 @@ impl FromStr for AccessToken {
             crate::crypto::stretch_key(encryption_key, "accesstoken", Some("sm-access-token"));
 
         Ok(AccessToken {
-            service_account_id: service_account_id,
+            service_account_id,
             client_secret: client_secret.to_owned(),
             encryption_key,
         })

crates/bitwarden/src/client/encryption_settings.rs

@@ -54,7 +54,7 @@ impl FromStr for SymmetricCryptoKey {
 
     fn from_str(s: &str) -> Result<Self, Self::Err> {
         let bytes = BASE64_ENGINE
-            .decode(&s)
+            .decode(s)
             .map_err(|_| CryptoError::InvalidKey)?;
         SymmetricCryptoKey::try_from(bytes.as_slice())
     }
@@ -120,7 +120,7 @@ impl EncryptionSettings {
                 _ => return Err(CryptoError::InvalidKey.into()),
             };
 
-            let dec = decrypt_aes256(&iv, &mac, &data, Some(mac_key), key)?;
+            let dec = decrypt_aes256(&iv, &mac, data, Some(mac_key), key)?;
             SymmetricCryptoKey::try_from(dec.as_slice())?
         };
 
@@ -178,10 +178,7 @@ impl EncryptionSettings {
         }
 
         match org_id {
-            Some(org_id) => match self.org_keys.get(org_id) {
-                Some(k) => Some(k),
-                None => return None,
-            },
+            Some(org_id) => self.org_keys.get(org_id),
             None => Some(&self.user_key),
         }
     }
@@ -203,17 +200,17 @@ impl EncryptionSettings {
 pub fn decrypt(cipher: &CipherString, key: &SymmetricCryptoKey) -> Result<Vec<u8>> {
     match cipher {
         CipherString::AesCbc256_HmacSha256_B64 { iv, mac, data } => {
-            let dec = decrypt_aes256(iv, mac, data, key.mac_key, key.key)?;
+            let dec = decrypt_aes256(iv, mac, data.clone(), key.mac_key, key.key)?;
             Ok(dec)
         }
-        _ => return Err(CryptoError::InvalidKey.into()),
+        _ => Err(CryptoError::InvalidKey.into()),
     }
 }
 
 pub fn decrypt_aes256(
     iv: &[u8; 16],
     mac: &[u8; 32],
-    data: &Vec<u8>,
+    data: Vec<u8>,
     mac_key: Option<GenericArray<u8, U32>>,
     key: GenericArray<u8, U32>,
 ) -> Result<Vec<u8>> {
@@ -223,14 +220,14 @@ pub fn decrypt_aes256(
     };
 
     // Validate HMAC
-    let res = validate_mac(&mac_key, iv, data)?;
+    let res = validate_mac(&mac_key, iv, &data)?;
     if res != *mac {
         return Err(CryptoError::InvalidMac.into());
     }
 
     // Decrypt data
     let iv = GenericArray::from_slice(iv);
-    let mut data = data.clone();
+    let mut data = data;
     let decrypted_key_slice = cbc::Decryptor::<aes::Aes256>::new(&key, iv)
         .decrypt_padded_mut::<Pkcs7>(&mut data)
         .map_err(|_| CryptoError::KeyDecrypt)?;

crates/bitwarden/src/client/mod.rs

@@ -4,6 +4,7 @@ pub(crate) use client::*;
 pub(crate) mod access_token;
 #[cfg(any(feature = "internal", feature = "mobile"))]
 pub(crate) mod auth_settings;
+#[allow(clippy::module_inception)]
 mod client;
 pub mod client_settings;
 pub(crate) mod encryption_settings;

crates/bitwarden/src/crypto.rs

@@ -306,7 +306,7 @@ pub(crate) fn stretch_key(secret: [u8; 16], name: &str, info: Option<&str>) -> S
     // TODO: Are these the final `key` and `info` parameters or should we change them? I followed the pattern used for sends
     let res = Hmac::<sha2::Sha256>::new_from_slice(format!("bitwarden-{}", name).as_bytes())
         .unwrap()
-        .chain_update(&secret)
+        .chain_update(secret)
         .finalize()
         .into_bytes();
 
@@ -317,7 +317,7 @@ pub(crate) fn stretch_key(secret: [u8; 16], name: &str, info: Option<&str>) -> S
     // TODO: Should we have a default value for info?
     //  Should it be required?
     let i = info.map(|i| i.as_bytes()).unwrap_or(&[]);
-    hkdf.expand(&i, &mut key).unwrap();
+    hkdf.expand(i, &mut key).unwrap();
 
     SymmetricCryptoKey::try_from(key.as_slice()).unwrap()
 }
@@ -343,7 +343,7 @@ fn hash_word(hash: [u8; 32]) -> Result<String> {
     let minimum_entropy = 64;
 
     let entropy_per_word = (EFF_LONG_WORD_LIST.len() as f64).log2();
-    let num_words = ((minimum_entropy as f64 / entropy_per_word).ceil() as f64).to_owned() as i64;
+    let num_words = ((minimum_entropy as f64 / entropy_per_word).ceil()).to_owned() as i64;
 
     let hash_arr: Vec<u8> = hash.to_vec();
     let entropy_available = hash_arr.len() * 4;
@@ -358,7 +358,7 @@ fn hash_word(hash: [u8; 32]) -> Result<String> {
     let mut hash_number = BigUint::from_bytes_be(&hash_arr);
     for _ in 0..num_words {
         let remainder = hash_number.clone() % EFF_LONG_WORD_LIST.len();
-        hash_number = hash_number / EFF_LONG_WORD_LIST.len();
+        hash_number /= EFF_LONG_WORD_LIST.len();
 
         phrase.push(EFF_LONG_WORD_LIST[remainder.to_usize().unwrap()].to_string());
     }
@@ -382,7 +382,7 @@ impl Encryptable<CipherString> for String {
 
 impl Decryptable<String> for CipherString {
     fn decrypt(&self, enc: &EncryptionSettings, org_id: &Option<Uuid>) -> Result<String> {
-        enc.decrypt(&self, org_id)
+        enc.decrypt(self, org_id)
     }
 }
 
@@ -406,7 +406,7 @@ impl<T: Encryptable<Output>, Output> Encryptable<Vec<Output>> for Vec<T> {
 
 impl<T: Decryptable<Output>, Output> Decryptable<Vec<Output>> for Vec<T> {
     fn decrypt(&self, enc: &EncryptionSettings, org_id: &Option<Uuid>) -> Result<Vec<Output>> {
-        self.into_iter().map(|e| e.decrypt(enc, org_id)).collect()
+        self.iter().map(|e| e.decrypt(enc, org_id)).collect()
     }
 }
 
@@ -432,12 +432,24 @@ impl<T: Decryptable<Output>, Output, Id: Hash + Eq + Copy> Decryptable<HashMap<I
         enc: &EncryptionSettings,
         org_id: &Option<Uuid>,
     ) -> Result<HashMap<Id, Output>> {
-        self.into_iter()
+        self.iter()
             .map(|(id, e)| Ok((*id, e.decrypt(enc, org_id)?)))
             .collect::<Result<HashMap<_, _>>>()
     }
 }
 
+impl<T: Encryptable<Output>, Output> Encryptable<Output> for Box<T> {
+    fn encrypt(self, enc: &EncryptionSettings, org_id: &Option<Uuid>) -> Result<Output> {
+        (*self).encrypt(enc, org_id)
+    }
+}
+
+impl<T: Decryptable<Output>, Output> Decryptable<Output> for Box<T> {
+    fn decrypt(&self, enc: &EncryptionSettings, org_id: &Option<Uuid>) -> Result<Output> {
+        (**self).decrypt(enc, org_id)
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::stretch_key;

crates/bitwarden/src/mobile/vault/ciphers/cipher_decrypt.rs

@@ -6,11 +6,11 @@ use crate::vault::{Cipher, CipherView};
 #[derive(Serialize, Deserialize, Debug, JsonSchema)]
 #[serde(rename_all = "camelCase", deny_unknown_fields)]
 pub struct CipherDecryptRequest {
-    pub cipher: Cipher,
+    pub cipher: Box<Cipher>,
 }
 
 #[derive(Serialize, Deserialize, Debug, JsonSchema)]
 #[serde(rename_all = "camelCase", deny_unknown_fields)]
 pub struct CipherDecryptResponse {
-    pub cipher: CipherView,
+    pub cipher: Box<CipherView>,
 }

crates/bitwarden/src/mobile/vault/ciphers/cipher_encrypt.rs

@@ -6,11 +6,11 @@ use crate::vault::{Cipher, CipherView};
 #[derive(Serialize, Deserialize, Debug, JsonSchema)]
 #[serde(rename_all = "camelCase", deny_unknown_fields)]
 pub struct CipherEncryptRequest {
-    pub cipher: CipherView,
+    pub cipher: Box<CipherView>,
 }
 
 #[derive(Serialize, Deserialize, Debug, JsonSchema)]
 #[serde(rename_all = "camelCase", deny_unknown_fields)]
 pub struct CipherEncryptResponse {
-    pub cipher: Cipher,
+    pub cipher: Box<Cipher>,
 }