From 5a07b193dc3d97c9c72ba9a36a877ecc52284ef5 Mon Sep 17 00:00:00 2001 From: BlackDex Date: Thu, 22 Sep 2022 19:40:04 +0200 Subject: [PATCH 1/4] Add support for send v2 API endpoints This PR adds support for the Send v2 API. It should prevent 404 errors which could cause some issues with some configurations on some reverse proxies. In the long run, we can probably remove the old file upload API, but for now lets leave it there, since Bitwarden also still has this endpoint in the code. Might fixes #2753 --- src/api/core/ciphers.rs | 10 ++-- src/api/core/sends.rs | 130 +++++++++++++++++++++++++++++++++++++--- 2 files changed, 127 insertions(+), 13 deletions(-) diff --git a/src/api/core/ciphers.rs b/src/api/core/ciphers.rs index 5256c28a..1870d12b 100644 --- a/src/api/core/ciphers.rs +++ b/src/api/core/ciphers.rs @@ -947,10 +947,12 @@ async fn save_attachment( let mut data = data.into_inner(); - // There seems to be a bug somewhere regarding uploading attachments using the Android Client (Maybe iOS too?) - // See: https://github.com/dani-garcia/vaultwarden/issues/2644 - // Since all other clients seem to match TempFile::File and not TempFile::Buffered lets catch this and return an error for now. - // We need to figure out how to solve this, but for now it's better to not accept these attachments since they will be broken. + // There is a bug regarding uploading attachments/sends using the Mobile clients + // See: https://github.com/dani-garcia/vaultwarden/issues/2644 && https://github.com/bitwarden/mobile/issues/2018 + // This has been fixed via a PR: https://github.com/bitwarden/mobile/pull/2031, but hasn't landed in a new release yet. + // On the vaultwarden side this is temporarily fixed by using a custom multer library + // See: https://github.com/dani-garcia/vaultwarden/pull/2675 + // In any case we will match TempFile::File and not TempFile::Buffered, since Buffered will alter the contents. if let TempFile::Buffered { content: _, } = &data.data diff --git a/src/api/core/sends.rs b/src/api/core/sends.rs index 3d150b31..e9d374f3 100644 --- a/src/api/core/sends.rs +++ b/src/api/core/sends.rs @@ -17,6 +17,9 @@ use crate::{ const SEND_INACCESSIBLE_MSG: &str = "Send does not exist or is no longer available"; +// The max file size allowed by Bitwarden clients and add an extra 5% to avoid issues +const SIZE_525_MB: u64 = 550_502_400; + pub fn routes() -> Vec { routes![ get_sends, @@ -28,7 +31,9 @@ pub fn routes() -> Vec { put_send, delete_send, put_remove_password, - download_send + download_send, + post_send_file_v2, + post_send_file_v2_data ] } @@ -58,6 +63,7 @@ struct SendData { Notes: Option, Text: Option, File: Option, + FileLength: Option, } /// Enforces the `Disable Send` policy. A non-owner/admin user belonging to @@ -185,6 +191,14 @@ struct UploadData<'f> { data: TempFile<'f>, } +#[derive(FromForm)] +struct UploadDataV2<'f> { + data: TempFile<'f>, +} + +// @deprecated Mar 25 2021: This method has been deprecated in favor of direct uploads (v2). +// This method still exists to support older clients, probably need to remove it sometime. +// Upstream: https://github.com/bitwarden/server/blob/d0c793c95181dfb1b447eb450f85ba0bfd7ef643/src/Api/Controllers/SendsController.cs#L164-L167 #[post("/sends/file", format = "multipart/form-data", data = "")] async fn post_send_file(data: Form>, headers: Headers, conn: DbConn, nt: Notify<'_>) -> JsonResult { enforce_disable_send_policy(&headers, &conn).await?; @@ -197,9 +211,6 @@ async fn post_send_file(data: Form>, headers: Headers, conn: DbCo enforce_disable_hide_email_policy(&model, &headers, &conn).await?; - // Get the file length and add an extra 5% to avoid issues - const SIZE_525_MB: u64 = 550_502_400; - let size_limit = match CONFIG.user_attachment_limit() { Some(0) => err!("File uploads are disabled"), Some(limit_kb) => { @@ -217,10 +228,12 @@ async fn post_send_file(data: Form>, headers: Headers, conn: DbCo err!("Send content is not a file"); } - // There seems to be a bug somewhere regarding uploading attachments using the Android Client (Maybe iOS too?) - // See: https://github.com/dani-garcia/vaultwarden/issues/2644 - // Since all other clients seem to match TempFile::File and not TempFile::Buffered lets catch this and return an error for now. - // We need to figure out how to solve this, but for now it's better to not accept these attachments since they will be broken. + // There is a bug regarding uploading attachments/sends using the Mobile clients + // See: https://github.com/dani-garcia/vaultwarden/issues/2644 && https://github.com/bitwarden/mobile/issues/2018 + // This has been fixed via a PR: https://github.com/bitwarden/mobile/pull/2031, but hasn't landed in a new release yet. + // On the vaultwarden side this is temporarily fixed by using a custom multer library + // See: https://github.com/dani-garcia/vaultwarden/pull/2675 + // In any case we will match TempFile::File and not TempFile::Buffered, since Buffered will alter the contents. if let TempFile::Buffered { content: _, } = &data @@ -252,11 +265,110 @@ async fn post_send_file(data: Form>, headers: Headers, conn: DbCo // Save the changes in the database send.save(&conn).await?; - nt.send_send_update(UpdateType::SyncSendUpdate, &send, &send.update_users_revision(&conn).await).await; + nt.send_send_update(UpdateType::SyncSendCreate, &send, &send.update_users_revision(&conn).await).await; Ok(Json(send.to_json())) } +// Upstream: https://github.com/bitwarden/server/blob/d0c793c95181dfb1b447eb450f85ba0bfd7ef643/src/Api/Controllers/SendsController.cs#L190 +#[post("/sends/file/v2", data = "")] +async fn post_send_file_v2(data: JsonUpcase, headers: Headers, conn: DbConn) -> JsonResult { + enforce_disable_send_policy(&headers, &conn).await?; + + let data = data.into_inner().data; + + if data.Type != SendType::File as i32 { + err!("Send content is not a file"); + } + + enforce_disable_hide_email_policy(&data, &headers, &conn).await?; + + let file_length = match &data.FileLength { + Some(m) => Some(m.into_i32()?), + _ => None, + }; + + let size_limit = match CONFIG.user_attachment_limit() { + Some(0) => err!("File uploads are disabled"), + Some(limit_kb) => { + let left = (limit_kb * 1024) - Attachment::size_by_user(&headers.user.uuid, &conn).await; + if left <= 0 { + err!("Attachment storage limit reached! Delete some attachments to free up space") + } + std::cmp::Ord::max(left as u64, SIZE_525_MB) + } + None => SIZE_525_MB, + }; + + if file_length.is_some() && file_length.unwrap() as u64 > size_limit { + err!("Attachment storage limit exceeded with this file"); + } + + let mut send = create_send(data, headers.user.uuid)?; + + let file_id = crate::crypto::generate_send_id(); + + let mut data_value: Value = serde_json::from_str(&send.data)?; + if let Some(o) = data_value.as_object_mut() { + o.insert(String::from("Id"), Value::String(file_id.clone())); + o.insert(String::from("Size"), Value::Number(file_length.unwrap().into())); + o.insert(String::from("SizeName"), Value::String(crate::util::get_display_size(file_length.unwrap()))); + } + send.data = serde_json::to_string(&data_value)?; + send.save(&conn).await?; + + Ok(Json(json!({ + "fileUploadType": 0, // 0 == Direct | 1 == Azure + "object": "send-fileUpload", + "url": format!("/sends/{}/file/{}", send.uuid, file_id), + "sendResponse": send.to_json() + }))) +} + +// https://github.com/bitwarden/server/blob/d0c793c95181dfb1b447eb450f85ba0bfd7ef643/src/Api/Controllers/SendsController.cs#L243 +#[post("/sends//file/", format = "multipart/form-data", data = "")] +async fn post_send_file_v2_data( + send_uuid: String, + file_id: String, + data: Form>, + headers: Headers, + conn: DbConn, + nt: Notify<'_>, +) -> EmptyResult { + enforce_disable_send_policy(&headers, &conn).await?; + + let mut data = data.into_inner(); + + // There is a bug regarding uploading attachments/sends using the Mobile clients + // See: https://github.com/dani-garcia/vaultwarden/issues/2644 && https://github.com/bitwarden/mobile/issues/2018 + // This has been fixed via a PR: https://github.com/bitwarden/mobile/pull/2031, but hasn't landed in a new release yet. + // On the vaultwarden side this is temporarily fixed by using a custom multer library + // See: https://github.com/dani-garcia/vaultwarden/pull/2675 + // In any case we will match TempFile::File and not TempFile::Buffered, since Buffered will alter the contents. + if let TempFile::Buffered { + content: _, + } = &data.data + { + err!("Error reading attachment data. Please try an other client."); + } + + if let Some(send) = Send::find_by_uuid(&send_uuid, &conn).await { + let folder_path = tokio::fs::canonicalize(&CONFIG.sends_folder()).await?.join(&send_uuid); + let file_path = folder_path.join(&file_id); + tokio::fs::create_dir_all(&folder_path).await?; + + if let Err(_err) = data.data.persist_to(&file_path).await { + data.data.move_copy_to(file_path).await? + } + + nt.send_send_update(UpdateType::SyncSendCreate, &send, &send.update_users_revision(&conn).await).await; + } else { + err!("Send not found. Unable to save the file."); + } + + Ok(()) +} + #[derive(Deserialize)] #[allow(non_snake_case)] pub struct SendAccessData { From 73c64af27e500ba1baae019316fd1ca961480b9b Mon Sep 17 00:00:00 2001 From: BlackDex Date: Thu, 15 Sep 2022 16:51:52 +0200 Subject: [PATCH 2/4] Update build workflow Currently the branch protection is set on specific workflows which needs to be run every time a PR is created (or a push). Because it isn't possible to tell the branch protection only to do it's job if specific files are touched or not, we just need to make sure these jobs are always started. Also, because we now check the builds for an MSRV, and the title would change all the time, that would cause the branch protection to be updated everytime the MSRV would change. This is now also addressed by naming that job 'msrv' instead of the version number. --- .github/workflows/build.yml | 9 ++++++--- .github/workflows/hadolint.yml | 12 ++++-------- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 90763d1b..47080daf 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -30,7 +30,10 @@ jobs: matrix: channel: - "rust-toolchain" # The version defined in rust-toolchain - - "1.60.0" # The supported MSRV + - "msrv" # The supported MSRV + include: + - channel: "msrv" + version: "1.60.0" name: Build and Test ${{ matrix.channel }} @@ -63,7 +66,7 @@ jobs: with: profile: minimal override: true - toolchain: ${{ matrix.channel }} + toolchain: ${{ matrix.version }} # End Install the MSRV channel to be used @@ -193,5 +196,5 @@ jobs: if: ${{ matrix.channel == 'rust-toolchain' }} with: name: vaultwarden - path: target/${{ matrix.target-triple }}/release/vaultwarden + path: target/release/vaultwarden # End Upload artifact to Github Actions diff --git a/.github/workflows/hadolint.yml b/.github/workflows/hadolint.yml index 5865bf63..738c3167 100644 --- a/.github/workflows/hadolint.yml +++ b/.github/workflows/hadolint.yml @@ -1,13 +1,9 @@ name: Hadolint -on: - push: - paths: - - "docker/**" - - pull_request: - paths: - - "docker/**" +on: [ + push, + pull_request + ] jobs: hadolint: From db5c98ec3bfe9fb55460eb87e11bb91d7186b9cd Mon Sep 17 00:00:00 2001 From: Aaron Date: Thu, 15 Sep 2022 15:36:21 -0700 Subject: [PATCH 3/4] fix: tooltip typo --- src/static/templates/admin/diagnostics.hbs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/static/templates/admin/diagnostics.hbs b/src/static/templates/admin/diagnostics.hbs index 99f27193..482ce631 100644 --- a/src/static/templates/admin/diagnostics.hbs +++ b/src/static/templates/admin/diagnostics.hbs @@ -141,7 +141,7 @@
Date & Time (UTC) Ok - Error + Error
Server: {{page_data.server_time}} @@ -151,7 +151,7 @@
Domain configuration Match No Match - HTTPS + HTTPS No HTTPS
From fc6e65e4b08ea949f7a09237a377a914b872785b Mon Sep 17 00:00:00 2001 From: Aaron Date: Fri, 16 Sep 2022 10:32:36 -0700 Subject: [PATCH 4/4] fix: update warning and success case verbiage --- src/static/templates/admin/diagnostics.hbs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/static/templates/admin/diagnostics.hbs b/src/static/templates/admin/diagnostics.hbs index 482ce631..20d6f371 100644 --- a/src/static/templates/admin/diagnostics.hbs +++ b/src/static/templates/admin/diagnostics.hbs @@ -140,8 +140,8 @@ Server: {{page_data.server_time_local}}
Date & Time (UTC) - Ok - Error + Ok + Error
Server: {{page_data.server_time}}