Fix several more multi select push issues (#6151)

* Fix several more multi select push issues

There were some more items which would still overload the push endpoint.
This PR fixes the remaining items (I hope).

I also encountered a missing endpoint for restoring multiple ciphers from the trash via the admin console.

Overall, we could improve a lot of these items in a different way. Like bundle all SQL Queries etc...
But that takes more time, and this fixes overloading the Bitwarden push servers, and speeds up these specific actions.

Signed-off-by: BlackDex <[email protected]>

* Update src/api/core/ciphers.rs

Co-authored-by: Daniel García <[email protected]>

---------

Signed-off-by: BlackDex <[email protected]>
Co-authored-by: Daniel García <[email protected]>

Author: BlackDex

src/api/core/ciphers.rs

@@ -78,6 +78,7 @@ pub fn routes() -> Vec<Route> {
         restore_cipher_put,
         restore_cipher_put_admin,
         restore_cipher_selected,
+        restore_cipher_selected_admin,
         delete_all,
         move_cipher_selected,
         move_cipher_selected_put,
@@ -318,7 +319,7 @@ async fn post_ciphers_create(
     // or otherwise), we can just ignore this field entirely.
     data.cipher.last_known_revision_date = None;
 
-    share_cipher_by_uuid(&cipher.uuid, data, &headers, &mut conn, &nt).await
+    share_cipher_by_uuid(&cipher.uuid, data, &headers, &mut conn, &nt, None).await
 }
 
 /// Called when creating a new user-owned cipher.
@@ -920,7 +921,7 @@ async fn post_cipher_share(
 ) -> JsonResult {
     let data: ShareCipherData = data.into_inner();
 
-    share_cipher_by_uuid(&cipher_id, data, &headers, &mut conn, &nt).await
+    share_cipher_by_uuid(&cipher_id, data, &headers, &mut conn, &nt, None).await
 }
 
 #[put("/ciphers/<cipher_id>/share", data = "<data>")]
@@ -933,7 +934,7 @@ async fn put_cipher_share(
 ) -> JsonResult {
     let data: ShareCipherData = data.into_inner();
 
-    share_cipher_by_uuid(&cipher_id, data, &headers, &mut conn, &nt).await
+    share_cipher_by_uuid(&cipher_id, data, &headers, &mut conn, &nt, None).await
 }
 
 #[derive(Deserialize)]
@@ -973,11 +974,16 @@ async fn put_cipher_share_selected(
         };
 
         match shared_cipher_data.cipher.id.take() {
-            Some(id) => share_cipher_by_uuid(&id, shared_cipher_data, &headers, &mut conn, &nt).await?,
+            Some(id) => {
+                share_cipher_by_uuid(&id, shared_cipher_data, &headers, &mut conn, &nt, Some(UpdateType::None)).await?
+            }
             None => err!("Request missing ids field"),
         };
     }
 
+    // Multi share 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(())
 }
 
@@ -987,6 +993,7 @@ async fn share_cipher_by_uuid(
     headers: &Headers,
     conn: &mut DbConn,
     nt: &Notify<'_>,
+    override_ut: Option<UpdateType>,
 ) -> JsonResult {
     let mut cipher = match Cipher::find_by_uuid(cipher_id, conn).await {
         Some(cipher) => {
@@ -1018,7 +1025,10 @@ async fn share_cipher_by_uuid(
     };
 
     // When LastKnownRevisionDate is None, it is a new cipher, so send CipherCreate.
-    let ut = if data.cipher.last_known_revision_date.is_some() {
+    // If there is an override, like when handling multiple items, we want to prevent a push notification for every single item
+    let ut = if let Some(ut) = override_ut {
+        ut
+    } else if data.cipher.last_known_revision_date.is_some() {
         UpdateType::SyncCipherUpdate
     } else {
         UpdateType::SyncCipherCreate
@@ -1517,7 +1527,7 @@ async fn delete_cipher_selected_put_admin(
 
 #[put("/ciphers/<cipher_id>/restore")]
 async fn restore_cipher_put(cipher_id: CipherId, headers: Headers, mut conn: DbConn, nt: Notify<'_>) -> JsonResult {
-    _restore_cipher_by_uuid(&cipher_id, &headers, &mut conn, &nt).await
+    _restore_cipher_by_uuid(&cipher_id, &headers, false, &mut conn, &nt).await
 }
 
 #[put("/ciphers/<cipher_id>/restore-admin")]
@@ -1527,7 +1537,17 @@ async fn restore_cipher_put_admin(
     mut conn: DbConn,
     nt: Notify<'_>,
 ) -> JsonResult {
-    _restore_cipher_by_uuid(&cipher_id, &headers, &mut conn, &nt).await
+    _restore_cipher_by_uuid(&cipher_id, &headers, false, &mut conn, &nt).await
+}
+
+#[put("/ciphers/restore-admin", data = "<data>")]
+async fn restore_cipher_selected_admin(
+    data: Json<CipherIdsData>,
+    headers: Headers,
+    mut conn: DbConn,
+    nt: Notify<'_>,
+) -> JsonResult {
+    _restore_multiple_ciphers(data, &headers, &mut conn, &nt).await
 }
 
 #[put("/ciphers/restore", data = "<data>")]
@@ -1555,35 +1575,47 @@ async fn move_cipher_selected(
     nt: Notify<'_>,
 ) -> EmptyResult {
     let data = data.into_inner();
-    let user_id = headers.user.uuid;
+    let user_id = &headers.user.uuid;
 
     if let Some(ref folder_id) = data.folder_id {
-        if Folder::find_by_uuid_and_user(folder_id, &user_id, &mut conn).await.is_none() {
+        if Folder::find_by_uuid_and_user(folder_id, user_id, &mut conn).await.is_none() {
             err!("Invalid folder", "Folder does not exist or belongs to another user");
         }
     }
 
-    for cipher_id in data.ids {
-        let Some(cipher) = Cipher::find_by_uuid(&cipher_id, &mut conn).await else {
-            err!("Cipher doesn't exist")
-        };
-
-        if !cipher.is_accessible_to_user(&user_id, &mut conn).await {
-            err!("Cipher is not accessible by user")
+    let cipher_count = data.ids.len();
+    let mut single_cipher: Option<Cipher> = None;
+
+    // TODO: Convert this to use a single query (or at least less) to update all items
+    // Find all ciphers a user has access to, all others will be ignored
+    let accessible_ciphers = Cipher::find_by_user_and_ciphers(user_id, &data.ids, &mut conn).await;
+    let accessible_ciphers_count = accessible_ciphers.len();
+    for cipher in accessible_ciphers {
+        cipher.move_to_folder(data.folder_id.clone(), user_id, &mut conn).await?;
+        if cipher_count == 1 {
+            single_cipher = Some(cipher);
         }
+    }
 
-        // Move cipher
-        cipher.move_to_folder(data.folder_id.clone(), &user_id, &mut conn).await?;
-
+    if let Some(cipher) = single_cipher {
         nt.send_cipher_update(
             UpdateType::SyncCipherUpdate,
             &cipher,
-            std::slice::from_ref(&user_id),
+            std::slice::from_ref(user_id),
             &headers.device,
             None,
             &mut conn,
         )
         .await;
+    } else {
+        // Multi move 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;
+    }
+
+    if cipher_count != accessible_ciphers_count {
+        err!(format!(
+            "Not all ciphers are moved! {accessible_ciphers_count} of the selected {cipher_count} were moved."
+        ))
     }
 
     Ok(())
@@ -1764,6 +1796,7 @@ async fn _delete_multiple_ciphers(
 async fn _restore_cipher_by_uuid(
     cipher_id: &CipherId,
     headers: &Headers,
+    multi_restore: bool,
     conn: &mut DbConn,
     nt: &Notify<'_>,
 ) -> JsonResult {
@@ -1778,15 +1811,17 @@ async fn _restore_cipher_by_uuid(
     cipher.deleted_at = None;
     cipher.save(conn).await?;
 
-    nt.send_cipher_update(
-        UpdateType::SyncCipherUpdate,
-        &cipher,
-        &cipher.update_users_revision(conn).await,
-        &headers.device,
-        None,
-        conn,
-    )
-    .await;
+    if !multi_restore {
+        nt.send_cipher_update(
+            UpdateType::SyncCipherUpdate,
+            &cipher,
+            &cipher.update_users_revision(conn).await,
+            &headers.device,
+            None,
+            conn,
+        )
+        .await;
+    }
 
     if let Some(org_id) = &cipher.organization_uuid {
         log_event(
@@ -1814,12 +1849,15 @@ async fn _restore_multiple_ciphers(
 
     let mut ciphers: Vec<Value> = Vec::new();
     for cipher_id in data.ids {
-        match _restore_cipher_by_uuid(&cipher_id, headers, conn, nt).await {
+        match _restore_cipher_by_uuid(&cipher_id, headers, true, conn, nt).await {
             Ok(json) => ciphers.push(json.into_inner()),
             err => return err,
         }
     }
 
+    // Multi move 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, conn).await;
+
     Ok(Json(json!({
       "data": ciphers,
       "object": "list",

src/db/models/auth_request.rs

@@ -6,7 +6,7 @@ use macros::UuidFromParam;
 use serde_json::Value;
 
 db_object! {
-    #[derive(Debug, Identifiable, Queryable, Insertable, AsChangeset, Deserialize, Serialize)]
+    #[derive(Identifiable, Queryable, Insertable, AsChangeset, Deserialize, Serialize)]
     #[diesel(table_name = auth_requests)]
     #[diesel(treat_none_as_null = true)]
     #[diesel(primary_key(uuid))]

src/db/models/cipher.rs

@@ -783,7 +783,12 @@ impl Cipher {
     // true, then the non-interesting ciphers will not be returned. As a
     // result, those ciphers will not appear in "My Vault" for the org
     // owner/admin, but they can still be accessed via the org vault view.
-    pub async fn find_by_user(user_uuid: &UserId, visible_only: bool, conn: &mut DbConn) -> Vec<Self> {
+    pub async fn find_by_user(
+        user_uuid: &UserId,
+        visible_only: bool,
+        cipher_uuids: &Vec<CipherId>,
+        conn: &mut DbConn,
+    ) -> Vec<Self> {
         if CONFIG.org_groups_enabled() {
             db_run! {conn: {
                 let mut query = ciphers::table
@@ -821,7 +826,14 @@ impl Cipher {
                 if !visible_only {
                     query = query.or_filter(
                         users_organizations::atype.le(MembershipType::Admin as i32) // Org admin/owner
-                        );
+                    );
+                }
+
+                // Only filter for one specific cipher
+                if !cipher_uuids.is_empty() {
+                    query = query.filter(
+                        ciphers::uuid.eq_any(cipher_uuids)
+                    );
                 }
 
                 query
@@ -850,11 +862,18 @@ impl Cipher {
                     .or_filter(users_collections::user_uuid.eq(user_uuid)) // Access to collection
                     .into_boxed();
 
-                    if !visible_only {
-                        query = query.or_filter(
-                            users_organizations::atype.le(MembershipType::Admin as i32) // Org admin/owner
-                            );
-                    }
+                if !visible_only {
+                    query = query.or_filter(
+                        users_organizations::atype.le(MembershipType::Admin as i32) // Org admin/owner
+                    );
+                }
+
+                // Only filter for one specific cipher
+                if !cipher_uuids.is_empty() {
+                    query = query.filter(
+                        ciphers::uuid.eq_any(cipher_uuids)
+                    );
+                }
 
                 query
                     .select(ciphers::all_columns)
@@ -866,7 +885,23 @@ impl Cipher {
 
     // Find all ciphers visible to the specified user.
     pub async fn find_by_user_visible(user_uuid: &UserId, conn: &mut DbConn) -> Vec<Self> {
-        Self::find_by_user(user_uuid, true, conn).await
+        Self::find_by_user(user_uuid, true, &vec![], conn).await
+    }
+
+    pub async fn find_by_user_and_ciphers(
+        user_uuid: &UserId,
+        cipher_uuids: &Vec<CipherId>,
+        conn: &mut DbConn,
+    ) -> Vec<Self> {
+        Self::find_by_user(user_uuid, true, cipher_uuids, conn).await
+    }
+
+    pub async fn find_by_user_and_cipher(
+        user_uuid: &UserId,
+        cipher_uuid: &CipherId,
+        conn: &mut DbConn,
+    ) -> Option<Self> {
+        Self::find_by_user(user_uuid, true, &vec![cipher_uuid.clone()], conn).await.pop()
     }
 
     // Find all ciphers directly owned by the specified user.