topaLE
/
vaultwarden
mirror of https://github.com/dani-garcia/vaultwarden
1
0
Fork 0
Code Issues Projects Releases Wiki Activity
Browse Source

Fix multi delete slowdown 

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>
pull/6144/head
BlackDex 6 days ago
parent
5d84f17600
commit
b47c16dab6
No known key found for this signature in database GPG Key ID: 58C80A2AA6C765E1
2 changed files with 72 additions and 39 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Unified View
Diff Options
Show Stats Download Patch File Download Diff File
  1. 99
      src/api/core/ciphers.rs
  2. 12
      src/api/notifications.rs

99
src/api/core/ciphers.rs
View File

@ -1405,7 +1405,7 @@ async fn delete_attachment_admin(
#[post("/ciphers/<cipher_id>/delete")] #[post("/ciphers/<cipher_id>/delete")]
async fn delete_cipher_post(cipher_id: CipherId, headers: Headers, mut conn: DbConn, nt: Notify<'_>) -> EmptyResult { 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 // permanent delete
} }
@ -1416,13 +1416,13 @@ async fn delete_cipher_post_admin(
mut conn: DbConn, mut conn: DbConn,
nt: Notify<'_>, nt: Notify<'_>,
) -> EmptyResult { ) -> 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 // permanent delete
} }
#[put("/ciphers/<cipher_id>/delete")] #[put("/ciphers/<cipher_id>/delete")]
async fn delete_cipher_put(cipher_id: CipherId, headers: Headers, mut conn: DbConn, nt: Notify<'_>) -> EmptyResult { 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 // soft delete
} }
@ -1433,18 +1433,19 @@ async fn delete_cipher_put_admin(
mut conn: DbConn, mut conn: DbConn,
nt: Notify<'_>, nt: Notify<'_>,
) -> EmptyResult { ) -> 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>")] #[delete("/ciphers/<cipher_id>")]
async fn delete_cipher(cipher_id: CipherId, headers: Headers, mut conn: DbConn, nt: Notify<'_>) -> EmptyResult { 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 // permanent delete
} }
#[delete("/ciphers/<cipher_id>/admin")] #[delete("/ciphers/<cipher_id>/admin")]
async fn delete_cipher_admin(cipher_id: CipherId, headers: Headers, mut conn: DbConn, nt: Notify<'_>) -> EmptyResult { 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 // permanent delete
} }
@ -1455,7 +1456,8 @@ async fn delete_cipher_selected(
conn: DbConn, conn: DbConn,
nt: Notify<'_>, nt: Notify<'_>,
) -> EmptyResult { ) -> 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>")] #[post("/ciphers/delete", data = "<data>")]
@ -1465,7 +1467,8 @@ async fn delete_cipher_selected_post(
conn: DbConn, conn: DbConn,
nt: Notify<'_>, nt: Notify<'_>,
) -> EmptyResult { ) -> 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>")] #[put("/ciphers/delete", data = "<data>")]
@ -1475,7 +1478,8 @@ async fn delete_cipher_selected_put(
conn: DbConn, conn: DbConn,
nt: Notify<'_>, nt: Notify<'_>,
) -> EmptyResult { ) -> 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>")] #[delete("/ciphers/admin", data = "<data>")]
@ -1485,7 +1489,8 @@ async fn delete_cipher_selected_admin(
conn: DbConn, conn: DbConn,
nt: Notify<'_>, nt: Notify<'_>,
) -> EmptyResult { ) -> 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>")] #[post("/ciphers/delete-admin", data = "<data>")]
@ -1495,7 +1500,8 @@ async fn delete_cipher_selected_post_admin(
conn: DbConn, conn: DbConn,
nt: Notify<'_>, nt: Notify<'_>,
) -> EmptyResult { ) -> 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>")] #[put("/ciphers/delete-admin", data = "<data>")]
@ -1505,7 +1511,8 @@ async fn delete_cipher_selected_put_admin(
conn: DbConn, conn: DbConn,
nt: Notify<'_>, nt: Notify<'_>,
) -> EmptyResult { ) -> 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")] #[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( async fn _delete_cipher_by_uuid(
cipher_id: &CipherId, cipher_id: &CipherId,
headers: &Headers, headers: &Headers,
conn: &mut DbConn, conn: &mut DbConn,
soft_delete: bool, delete_options: &CipherDeleteOptions,
nt: &Notify<'_>, nt: &Notify<'_>,
) -> EmptyResult { ) -> EmptyResult {
let Some(mut cipher) = Cipher::find_by_uuid(cipher_id, conn).await else { 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") 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.deleted_at = Some(Utc::now().naive_utc());
cipher.save(conn).await?; 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 { } else {
cipher.delete(conn).await?; 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 { 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) 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>, data: Json<CipherIdsData>,
headers: Headers, headers: Headers,
mut conn: DbConn, mut conn: DbConn,
soft_delete: bool, delete_options: CipherDeleteOptions,
nt: Notify<'_>, nt: Notify<'_>,
) -> EmptyResult { ) -> EmptyResult {
let data = data.into_inner(); let data = data.into_inner();
for cipher_id in data.ids { 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; 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(()) Ok(())
} }

12
src/api/notifications.rs
View File

@ -619,7 +619,7 @@ fn create_ping() -> Vec<u8> {
serialize(Value::Array(vec![6.into()])) 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)] #[derive(Copy, Clone, Eq, PartialEq)]
pub enum UpdateType { pub enum UpdateType {
SyncCipherUpdate = 0, SyncCipherUpdate = 0,
@ -632,7 +632,7 @@ pub enum UpdateType {
SyncOrgKeys = 6, SyncOrgKeys = 6,
SyncFolderCreate = 7, SyncFolderCreate = 7,
SyncFolderUpdate = 8, SyncFolderUpdate = 8,
SyncCipherDelete = 9, // SyncCipherDelete = 9, // Redirects to `SyncLoginDelete` on upstream
SyncSettings = 10, SyncSettings = 10,
LogOut = 11, LogOut = 11,
@ -644,6 +644,14 @@ pub enum UpdateType {
AuthRequest = 15, AuthRequest = 15,
AuthRequestResponse = 16, 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, None = 100,
} }

Write Preview
Loading…
Cancel
Save