Perform same checks when setting kdf (#6141)

This issue mostly arises when push is enabled, but it also overloaded websocket connections.
We would send a notification on every deleted cipher, which could be up-to 500 items.
If push is enabled, it could overload the Push servers, and it would return a 429 error.

This PR fixes this by not sending out a message on every single cipher during a multi delete actions.
It will send a single push message to sync the vault once finished.

The only caveat here is that there seems to be a bug with the mobile clients which ignores these global sync notifications.
But, preventing a 429, which could cause a long term block of the sending server by the push servers, this is probably the best way, and, it is the same as Bitwarden it self does.

Fixes #4693

Signed-off-by: BlackDex <black.dex@gmail.com>

src/api/core/accounts.rs

@@ -66,15 +66,22 @@ pub fn routes() -> Vec<rocket::Route> {
     ]
 }
 
+#[derive(Debug, Deserialize)]
+#[serde(rename_all = "camelCase")]
+pub struct KDFData {
+    kdf: i32,
+    kdf_iterations: i32,
+    kdf_memory: Option<i32>,
+    kdf_parallelism: Option<i32>,
+}
+
 #[derive(Debug, Deserialize)]
 #[serde(rename_all = "camelCase")]
 pub struct RegisterData {
     email: String,
 
-    kdf: Option<i32>,
+    #[serde(flatten)]
-    kdf_iterations: Option<i32>,
+    kdf: KDFData,
-    kdf_memory: Option<i32>,
-    kdf_parallelism: Option<i32>,
 
     #[serde(alias = "userSymmetricKey")]
     key: String,
@@ -269,16 +276,7 @@ pub async fn _register(data: Json<RegisterData>, email_verification: bool, mut c
     // Make sure we don't leave a lingering invitation.
     Invitation::take(&email, &mut conn).await;
 
-    if let Some(client_kdf_type) = data.kdf {
+    set_kdf_data(&mut user, data.kdf)?;
-        user.client_kdf_type = client_kdf_type;
-    }
-
-    if let Some(client_kdf_iter) = data.kdf_iterations {
-        user.client_kdf_iter = client_kdf_iter;
-    }
-
-    user.client_kdf_memory = data.kdf_memory;
-    user.client_kdf_parallelism = data.kdf_parallelism;
 
     user.set_password(&data.master_password_hash, Some(data.key), true, None);
     user.password_hint = password_hint;
@@ -469,25 +467,15 @@ async fn post_password(data: Json<ChangePassData>, headers: Headers, mut conn: D
 #[derive(Deserialize)]
 #[serde(rename_all = "camelCase")]
 struct ChangeKdfData {
-    kdf: i32,
+    #[serde(flatten)]
-    kdf_iterations: i32,
+    kdf: KDFData,
-    kdf_memory: Option<i32>,
-    kdf_parallelism: Option<i32>,
 
     master_password_hash: String,
     new_master_password_hash: String,
     key: String,
 }
 
-#[post("/accounts/kdf", data = "<data>")]
+fn set_kdf_data(user: &mut User, data: KDFData) -> EmptyResult {
-async fn post_kdf(data: Json<ChangeKdfData>, headers: Headers, mut conn: DbConn, nt: Notify<'_>) -> EmptyResult {
-    let data: ChangeKdfData = data.into_inner();
-    let mut user = headers.user;
-
-    if !user.check_valid_password(&data.master_password_hash) {
-        err!("Invalid password")
-    }
-
     if data.kdf == UserKdfType::Pbkdf2 as i32 && data.kdf_iterations < 100_000 {
         err!("PBKDF2 KDF iterations must be at least 100000.")
     }
@@ -518,6 +506,21 @@ async fn post_kdf(data: Json<ChangeKdfData>, headers: Headers, mut conn: DbConn,
     }
     user.client_kdf_iter = data.kdf_iterations;
     user.client_kdf_type = data.kdf;
+
+    Ok(())
+}
+
+#[post("/accounts/kdf", data = "<data>")]
+async fn post_kdf(data: Json<ChangeKdfData>, headers: Headers, mut conn: DbConn, nt: Notify<'_>) -> EmptyResult {
+    let data: ChangeKdfData = data.into_inner();
+    let mut user = headers.user;
+
+    if !user.check_valid_password(&data.master_password_hash) {
+        err!("Invalid password")
+    }
+
+    set_kdf_data(&mut user, data.kdf)?;
+
     user.set_password(&data.new_master_password_hash, Some(data.key), true, None);
     let save_result = user.save(&mut conn).await;
 

src/api/core/ciphers.rs

@@ -1405,7 +1405,7 @@ async fn delete_attachment_admin(
 
 #[post("/ciphers/<cipher_id>/delete")]
 async fn delete_cipher_post(cipher_id: CipherId, headers: Headers, mut conn: DbConn, nt: Notify<'_>) -> EmptyResult {
-    _delete_cipher_by_uuid(&cipher_id, &headers, &mut conn, false, &nt).await
+    _delete_cipher_by_uuid(&cipher_id, &headers, &mut conn, &CipherDeleteOptions::HardSingle, &nt).await
     // permanent delete
 }
 
@@ -1416,13 +1416,13 @@ async fn delete_cipher_post_admin(
     mut conn: DbConn,
     nt: Notify<'_>,
 ) -> EmptyResult {
-    _delete_cipher_by_uuid(&cipher_id, &headers, &mut conn, false, &nt).await
+    _delete_cipher_by_uuid(&cipher_id, &headers, &mut conn, &CipherDeleteOptions::HardSingle, &nt).await
     // permanent delete
 }
 
 #[put("/ciphers/<cipher_id>/delete")]
 async fn delete_cipher_put(cipher_id: CipherId, headers: Headers, mut conn: DbConn, nt: Notify<'_>) -> EmptyResult {
-    _delete_cipher_by_uuid(&cipher_id, &headers, &mut conn, true, &nt).await
+    _delete_cipher_by_uuid(&cipher_id, &headers, &mut conn, &CipherDeleteOptions::SoftSingle, &nt).await
     // soft delete
 }
 
@@ -1433,18 +1433,19 @@ async fn delete_cipher_put_admin(
     mut conn: DbConn,
     nt: Notify<'_>,
 ) -> EmptyResult {
-    _delete_cipher_by_uuid(&cipher_id, &headers, &mut conn, true, &nt).await
+    _delete_cipher_by_uuid(&cipher_id, &headers, &mut conn, &CipherDeleteOptions::SoftSingle, &nt).await
+    // soft delete
 }
 
 #[delete("/ciphers/<cipher_id>")]
 async fn delete_cipher(cipher_id: CipherId, headers: Headers, mut conn: DbConn, nt: Notify<'_>) -> EmptyResult {
-    _delete_cipher_by_uuid(&cipher_id, &headers, &mut conn, false, &nt).await
+    _delete_cipher_by_uuid(&cipher_id, &headers, &mut conn, &CipherDeleteOptions::HardSingle, &nt).await
     // permanent delete
 }
 
 #[delete("/ciphers/<cipher_id>/admin")]
 async fn delete_cipher_admin(cipher_id: CipherId, headers: Headers, mut conn: DbConn, nt: Notify<'_>) -> EmptyResult {
-    _delete_cipher_by_uuid(&cipher_id, &headers, &mut conn, false, &nt).await
+    _delete_cipher_by_uuid(&cipher_id, &headers, &mut conn, &CipherDeleteOptions::HardSingle, &nt).await
     // permanent delete
 }
 
@@ -1455,7 +1456,8 @@ async fn delete_cipher_selected(
     conn: DbConn,
     nt: Notify<'_>,
 ) -> EmptyResult {
-    _delete_multiple_ciphers(data, headers, conn, false, nt).await // permanent delete
+    _delete_multiple_ciphers(data, headers, conn, CipherDeleteOptions::HardMulti, nt).await
+    // permanent delete
 }
 
 #[post("/ciphers/delete", data = "<data>")]
@@ -1465,7 +1467,8 @@ async fn delete_cipher_selected_post(
     conn: DbConn,
     nt: Notify<'_>,
 ) -> EmptyResult {
-    _delete_multiple_ciphers(data, headers, conn, false, nt).await // permanent delete
+    _delete_multiple_ciphers(data, headers, conn, CipherDeleteOptions::HardMulti, nt).await
+    // permanent delete
 }
 
 #[put("/ciphers/delete", data = "<data>")]
@@ -1475,7 +1478,8 @@ async fn delete_cipher_selected_put(
     conn: DbConn,
     nt: Notify<'_>,
 ) -> EmptyResult {
-    _delete_multiple_ciphers(data, headers, conn, true, nt).await // soft delete
+    _delete_multiple_ciphers(data, headers, conn, CipherDeleteOptions::SoftMulti, nt).await
+    // soft delete
 }
 
 #[delete("/ciphers/admin", data = "<data>")]
@@ -1485,7 +1489,8 @@ async fn delete_cipher_selected_admin(
     conn: DbConn,
     nt: Notify<'_>,
 ) -> EmptyResult {
-    _delete_multiple_ciphers(data, headers, conn, false, nt).await // permanent delete
+    _delete_multiple_ciphers(data, headers, conn, CipherDeleteOptions::HardMulti, nt).await
+    // permanent delete
 }
 
 #[post("/ciphers/delete-admin", data = "<data>")]
@@ -1495,7 +1500,8 @@ async fn delete_cipher_selected_post_admin(
     conn: DbConn,
     nt: Notify<'_>,
 ) -> EmptyResult {
-    _delete_multiple_ciphers(data, headers, conn, false, nt).await // permanent delete
+    _delete_multiple_ciphers(data, headers, conn, CipherDeleteOptions::HardMulti, nt).await
+    // permanent delete
 }
 
 #[put("/ciphers/delete-admin", data = "<data>")]
@@ -1505,7 +1511,8 @@ async fn delete_cipher_selected_put_admin(
     conn: DbConn,
     nt: Notify<'_>,
 ) -> EmptyResult {
-    _delete_multiple_ciphers(data, headers, conn, true, nt).await // soft delete
+    _delete_multiple_ciphers(data, headers, conn, CipherDeleteOptions::SoftMulti, nt).await
+    // soft delete
 }
 
 #[put("/ciphers/<cipher_id>/restore")]
@@ -1659,11 +1666,19 @@ async fn delete_all(
     }
 }
 
+#[derive(PartialEq)]
+pub enum CipherDeleteOptions {
+    SoftSingle,
+    SoftMulti,
+    HardSingle,
+    HardMulti,
+}
+
 async fn _delete_cipher_by_uuid(
     cipher_id: &CipherId,
     headers: &Headers,
     conn: &mut DbConn,
-    soft_delete: bool,
+    delete_options: &CipherDeleteOptions,
     nt: &Notify<'_>,
 ) -> EmptyResult {
     let Some(mut cipher) = Cipher::find_by_uuid(cipher_id, conn).await else {
@@ -1674,35 +1689,42 @@ async fn _delete_cipher_by_uuid(
         err!("Cipher can't be deleted by user")
     }
 
-    if soft_delete {
+    if *delete_options == CipherDeleteOptions::SoftSingle || *delete_options == CipherDeleteOptions::SoftMulti {
         cipher.deleted_at = Some(Utc::now().naive_utc());
         cipher.save(conn).await?;
-        nt.send_cipher_update(
+        if *delete_options == CipherDeleteOptions::SoftSingle {
-            UpdateType::SyncCipherUpdate,
+            nt.send_cipher_update(
-            &cipher,
+                UpdateType::SyncCipherUpdate,
-            &cipher.update_users_revision(conn).await,
+                &cipher,
-            &headers.device,
+                &cipher.update_users_revision(conn).await,
-            None,
+                &headers.device,
-            conn,
+                None,
-        )
+                conn,
-        .await;
+            )
+            .await;
+        }
     } else {
         cipher.delete(conn).await?;
-        nt.send_cipher_update(
+        if *delete_options == CipherDeleteOptions::HardSingle {
-            UpdateType::SyncCipherDelete,
+            nt.send_cipher_update(
-            &cipher,
+                UpdateType::SyncLoginDelete,
-            &cipher.update_users_revision(conn).await,
+                &cipher,
-            &headers.device,
+                &cipher.update_users_revision(conn).await,
-            None,
+                &headers.device,
-            conn,
+                None,
-        )
+                conn,
-        .await;
+            )
+            .await;
+        }
     }
 
     if let Some(org_id) = cipher.organization_uuid {
-        let event_type = match soft_delete {
+        let event_type = if *delete_options == CipherDeleteOptions::SoftSingle
-            true => EventType::CipherSoftDeleted as i32,
+            || *delete_options == CipherDeleteOptions::SoftMulti
-            false => EventType::CipherDeleted as i32,
+        {
+            EventType::CipherSoftDeleted as i32
+        } else {
+            EventType::CipherDeleted as i32
         };
 
         log_event(event_type, &cipher.uuid, &org_id, &headers.user.uuid, headers.device.atype, &headers.ip.ip, conn)
@@ -1722,17 +1744,20 @@ async fn _delete_multiple_ciphers(
     data: Json<CipherIdsData>,
     headers: Headers,
     mut conn: DbConn,
-    soft_delete: bool,
+    delete_options: CipherDeleteOptions,
     nt: Notify<'_>,
 ) -> EmptyResult {
     let data = data.into_inner();
 
     for cipher_id in data.ids {
-        if let error @ Err(_) = _delete_cipher_by_uuid(&cipher_id, &headers, &mut conn, soft_delete, &nt).await {
+        if let error @ Err(_) = _delete_cipher_by_uuid(&cipher_id, &headers, &mut conn, &delete_options, &nt).await {
             return error;
         };
     }
 
+    // Multi delete actions do not send out a push for each cipher, we need to send a general sync here
+    nt.send_user_update(UpdateType::SyncCiphers, &headers.user, &headers.device.push_uuid, &mut conn).await;
+
     Ok(())
 }
 

src/api/notifications.rs

@@ -619,7 +619,7 @@ fn create_ping() -> Vec<u8> {
     serialize(Value::Array(vec![6.into()]))
 }
 
-#[allow(dead_code)]
+// https://github.com/bitwarden/server/blob/375af7c43b10d9da03525d41452f95de3f921541/src/Core/Enums/PushType.cs
 #[derive(Copy, Clone, Eq, PartialEq)]
 pub enum UpdateType {
     SyncCipherUpdate = 0,
@@ -632,7 +632,7 @@ pub enum UpdateType {
     SyncOrgKeys = 6,
     SyncFolderCreate = 7,
     SyncFolderUpdate = 8,
-    SyncCipherDelete = 9,
+    // SyncCipherDelete = 9, // Redirects to `SyncLoginDelete` on upstream
     SyncSettings = 10,
 
     LogOut = 11,
@@ -644,6 +644,14 @@ pub enum UpdateType {
     AuthRequest = 15,
     AuthRequestResponse = 16,
 
+    // SyncOrganizations = 17, // Not supported
+    // SyncOrganizationStatusChanged = 18, // Not supported
+    // SyncOrganizationCollectionSettingChanged = 19, // Not supported
+
+    // Notification = 20, // Not supported
+    // NotificationStatus = 21, // Not supported
+
+    // RefreshSecurityTasks = 22, // Not supported
     None = 100,
 }