[PM-6104] Add locking support to bitwarden-json to improve bindings t…

…hread safety (#591)

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

## Objective
I've added the lock into bitwarden-json because we already do locking in
bitwarden-wasm and this way we can remove it from there.

crates/bitwarden-c/src/c.rs

@@ -8,7 +8,7 @@ use crate::{box_ptr, ffi_ref};
 #[tokio::main]
 pub async extern "C" fn run_command(
     c_str_ptr: *const c_char,
-    client_ptr: *mut Client,
+    client_ptr: *const Client,
 ) -> *mut c_char {
     let client = unsafe { ffi_ref!(client_ptr) };
     let input_str = str::from_utf8(unsafe { CStr::from_ptr(c_str_ptr).to_bytes() }).unwrap();

crates/bitwarden-c/src/macros/ffi.rs

@@ -3,7 +3,7 @@
 macro_rules! ffi_ref {
     ($name:ident) => {{
         assert!(!$name.is_null());
-        &mut *$name
+        &*$name
     }};
 }
 

crates/bitwarden-json/Cargo.toml

@@ -18,6 +18,7 @@ internal = ["bitwarden/internal"] # Internal testing methods
 secrets = ["bitwarden/secrets"]   # Secrets manager API
 
 [dependencies]
+async-lock = ">=3.3.0, <4.0"
 log = ">=0.4.18, <0.5"
 schemars = ">=0.8.12, <0.9"
 serde = { version = ">=1.0, <2.0", features = ["derive"] }

crates/bitwarden-json/src/client.rs

@@ -1,3 +1,4 @@
+use async_lock::Mutex;
 use bitwarden::client::client_settings::ClientSettings;
 
 #[cfg(feature = "secrets")]
@@ -7,15 +8,15 @@ use crate::{
     response::{Response, ResponseIntoString},
 };
 
-pub struct Client(bitwarden::Client);
+pub struct Client(Mutex<bitwarden::Client>);
 
 impl Client {
     pub fn new(settings_input: Option<String>) -> Self {
         let settings = Self::parse_settings(settings_input);
-        Self(bitwarden::Client::new(settings))
+        Self(Mutex::new(bitwarden::Client::new(settings)))
     }
 
-    pub async fn run_command(&mut self, input_str: &str) -> String {
+    pub async fn run_command(&self, input_str: &str) -> String {
         const SUBCOMMANDS_TO_CLEAN: &[&str] = &["Secrets"];
         let mut cmd_value: serde_json::Value = match serde_json::from_str(input_str) {
             Ok(cmd) => cmd,
@@ -44,41 +45,43 @@ impl Client {
             }
         };
 
+        let mut client = self.0.lock().await;
+
         match cmd {
             #[cfg(feature = "internal")]
-            Command::PasswordLogin(req) => self.0.auth().login_password(&req).await.into_string(),
+            Command::PasswordLogin(req) => client.auth().login_password(&req).await.into_string(),
             #[cfg(feature = "secrets")]
             Command::AccessTokenLogin(req) => {
-                self.0.auth().login_access_token(&req).await.into_string()
+                client.auth().login_access_token(&req).await.into_string()
             }
             #[cfg(feature = "internal")]
-            Command::GetUserApiKey(req) => self.0.get_user_api_key(&req).await.into_string(),
+            Command::GetUserApiKey(req) => client.get_user_api_key(&req).await.into_string(),
             #[cfg(feature = "internal")]
-            Command::ApiKeyLogin(req) => self.0.auth().login_api_key(&req).await.into_string(),
+            Command::ApiKeyLogin(req) => client.auth().login_api_key(&req).await.into_string(),
             #[cfg(feature = "internal")]
-            Command::Sync(req) => self.0.sync(&req).await.into_string(),
+            Command::Sync(req) => client.sync(&req).await.into_string(),
             #[cfg(feature = "internal")]
-            Command::Fingerprint(req) => self.0.platform().fingerprint(&req).into_string(),
+            Command::Fingerprint(req) => client.platform().fingerprint(&req).into_string(),
 
             #[cfg(feature = "secrets")]
             Command::Secrets(cmd) => match cmd {
-                SecretsCommand::Get(req) => self.0.secrets().get(&req).await.into_string(),
+                SecretsCommand::Get(req) => client.secrets().get(&req).await.into_string(),
                 SecretsCommand::GetByIds(req) => {
-                    self.0.secrets().get_by_ids(req).await.into_string()
+                    client.secrets().get_by_ids(req).await.into_string()
                 }
-                SecretsCommand::Create(req) => self.0.secrets().create(&req).await.into_string(),
-                SecretsCommand::List(req) => self.0.secrets().list(&req).await.into_string(),
-                SecretsCommand::Update(req) => self.0.secrets().update(&req).await.into_string(),
-                SecretsCommand::Delete(req) => self.0.secrets().delete(req).await.into_string(),
+                SecretsCommand::Create(req) => client.secrets().create(&req).await.into_string(),
+                SecretsCommand::List(req) => client.secrets().list(&req).await.into_string(),
+                SecretsCommand::Update(req) => client.secrets().update(&req).await.into_string(),
+                SecretsCommand::Delete(req) => client.secrets().delete(req).await.into_string(),
             },
 
             #[cfg(feature = "secrets")]
             Command::Projects(cmd) => match cmd {
-                ProjectsCommand::Get(req) => self.0.projects().get(&req).await.into_string(),
-                ProjectsCommand::Create(req) => self.0.projects().create(&req).await.into_string(),
-                ProjectsCommand::List(req) => self.0.projects().list(&req).await.into_string(),
-                ProjectsCommand::Update(req) => self.0.projects().update(&req).await.into_string(),
-                ProjectsCommand::Delete(req) => self.0.projects().delete(req).await.into_string(),
+                ProjectsCommand::Get(req) => client.projects().get(&req).await.into_string(),
+                ProjectsCommand::Create(req) => client.projects().create(&req).await.into_string(),
+                ProjectsCommand::List(req) => client.projects().list(&req).await.into_string(),
+                ProjectsCommand::Update(req) => client.projects().update(&req).await.into_string(),
+                ProjectsCommand::Delete(req) => client.projects().delete(req).await.into_string(),
             },
         }
     }

crates/bitwarden-napi/src/client.rs

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

crates/bitwarden-py/src/client.rs

@@ -13,12 +13,12 @@ impl BitwardenClient {
     }
 
     #[pyo3(text_signature = "($self, command_input)")]
-    fn run_command(&mut self, command_input: String) -> String {
-        run_command(&mut self.0, &command_input)
+    fn run_command(&self, command_input: String) -> String {
+        run_command(&self.0, &command_input)
     }
 }
 
 #[tokio::main]
-async fn run_command(client: &mut JsonClient, input_str: &str) -> String {
+async fn run_command(client: &JsonClient, input_str: &str) -> String {
     client.run_command(input_str).await
 }

crates/bitwarden-wasm/src/client.rs

@@ -1,5 +1,5 @@
 extern crate console_error_panic_hook;
-use std::{rc::Rc, sync::RwLock};
+use std::rc::Rc;
 
 use bitwarden_json::client::Client as JsonClient;
 use js_sys::Promise;
@@ -26,10 +26,10 @@ fn convert_level(level: LogLevel) -> Level {
     }
 }
 
-// Rc<RwLock<...>> is to avoid needing to take ownership of the Client during our async run_command
+// Rc<...> is to avoid needing to take ownership of the Client during our async run_command
 // function https://github.com/rustwasm/wasm-bindgen/issues/2195#issuecomment-799588401
 #[wasm_bindgen]
-pub struct BitwardenClient(Rc<RwLock<JsonClient>>);
+pub struct BitwardenClient(Rc<JsonClient>);
 
 #[wasm_bindgen]
 impl BitwardenClient {
@@ -42,20 +42,14 @@ impl BitwardenClient {
             panic!("failed to initialize logger: {:?}", e);
         }
 
-        Self(Rc::new(RwLock::new(bitwarden_json::client::Client::new(
-            settings_input,
-        ))))
+        Self(Rc::new(bitwarden_json::client::Client::new(settings_input)))
     }
 
     #[wasm_bindgen]
-    pub fn run_command(&mut self, js_input: String) -> Promise {
+    pub fn run_command(&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;
+            let result = rc.run_command(&js_input).await;
             Ok(result.into())
         })
     }