Merge pull request #1730 from jjlin/attachment-upload-v2

Add support for v2 attachment upload APIs

src/api/core/ciphers.rs

@@ -1,12 +1,11 @@
 use std::collections::{HashMap, HashSet};
-use std::path::Path;
+use std::path::{Path, PathBuf};
 
 use chrono::{NaiveDateTime, Utc};
 use rocket::{http::ContentType, request::Form, Data, Route};
 use rocket_contrib::json::Json;
 use serde_json::Value;
 
-use data_encoding::HEXLOWER;
 use multipart::server::{save::SavedData, Multipart, SaveResult};
 
 use crate::{
@@ -40,8 +39,10 @@ pub fn routes() -> Vec<Route> {
         post_ciphers_create,
         post_ciphers_import,
         get_attachment,
-        post_attachment,
+        post_attachment_v2,
-        post_attachment_admin,
+        post_attachment_v2_data,
+        post_attachment,       // legacy
+        post_attachment_admin, // legacy
         post_attachment_share,
         delete_attachment_post,
         delete_attachment_post_admin,
@@ -755,6 +756,12 @@ fn share_cipher_by_uuid(
     Ok(Json(cipher.to_json(&headers.host, &headers.user.uuid, &conn)))
 }
 
+/// v2 API for downloading an attachment. This just redirects the client to
+/// the actual location of an attachment.
+///
+/// Upstream added this v2 API to support direct download of attachments from
+/// their object storage service. For self-hosted instances, it basically just
+/// redirects to the same location as before the v2 API.
 #[get("/ciphers/<uuid>/attachment/<attachment_id>")]
 fn get_attachment(uuid: String, attachment_id: String, headers: Headers, conn: DbConn) -> JsonResult {
     match Attachment::find_by_id(&attachment_id, &conn) {
@@ -764,16 +771,79 @@ fn get_attachment(uuid: String, attachment_id: String, headers: Headers, conn: D
     }
 }
 
-#[post("/ciphers/<uuid>/attachment", format = "multipart/form-data", data = "<data>")]
+#[derive(Deserialize)]
-fn post_attachment(
+#[allow(non_snake_case)]
+struct AttachmentRequestData {
+    Key: String,
+    FileName: String,
+    FileSize: i32,
+    // We check org owner/admin status via is_write_accessible_to_user(),
+    // so we can just ignore this field.
+    //
+    // AdminRequest: bool,
+}
+
+enum FileUploadType {
+    Direct = 0,
+    // Azure = 1, // only used upstream
+}
+
+/// v2 API for creating an attachment associated with a cipher.
+/// This redirects the client to the API it should use to upload the attachment.
+/// For upstream's cloud-hosted service, it's an Azure object storage API.
+/// For self-hosted instances, it's another API on the local instance.
+#[post("/ciphers/<uuid>/attachment/v2", data = "<data>")]
+fn post_attachment_v2(
     uuid: String,
-    data: Data,
+    data: JsonUpcase<AttachmentRequestData>,
-    content_type: &ContentType,
     headers: Headers,
     conn: DbConn,
-    nt: Notify,
 ) -> JsonResult {
     let cipher = match Cipher::find_by_uuid(&uuid, &conn) {
+        Some(cipher) => cipher,
+        None => err!("Cipher doesn't exist"),
+    };
+
+    if !cipher.is_write_accessible_to_user(&headers.user.uuid, &conn) {
+        err!("Cipher is not write accessible")
+    }
+
+    let attachment_id = crypto::generate_attachment_id();
+    let data: AttachmentRequestData = data.into_inner().data;
+    let attachment =
+        Attachment::new(attachment_id.clone(), cipher.uuid.clone(), data.FileName, data.FileSize, Some(data.Key));
+    attachment.save(&conn).expect("Error saving attachment");
+
+    let url = format!("/ciphers/{}/attachment/{}", cipher.uuid, attachment_id);
+
+    Ok(Json(json!({ // AttachmentUploadDataResponseModel
+        "Object": "attachment-fileUpload",
+        "AttachmentId": attachment_id,
+        "Url": url,
+        "FileUploadType": FileUploadType::Direct as i32,
+        "CipherResponse": cipher.to_json(&headers.host, &headers.user.uuid, &conn),
+        "CipherMiniResponse": null,
+    })))
+}
+
+/// Saves the data content of an attachment to a file. This is common code
+/// shared between the v2 and legacy attachment APIs.
+///
+/// When used with the legacy API, this function is responsible for creating
+/// the attachment database record, so `attachment` is None.
+///
+/// When used with the v2 API, post_attachment_v2() has already created the
+/// database record, which is passed in as `attachment`.
+fn save_attachment(
+    mut attachment: Option<Attachment>,
+    cipher_uuid: String,
+    data: Data,
+    content_type: &ContentType,
+    headers: &Headers,
+    conn: &DbConn,
+    nt: Notify,
+) -> Result<Cipher, crate::error::Error> {
+    let cipher = match Cipher::find_by_uuid(&cipher_uuid, conn) {
         Some(cipher) => cipher,
         None => err_discard!("Cipher doesn't exist", data),
     };
@@ -782,15 +852,18 @@ fn post_attachment(
         err_discard!("Cipher is not write accessible", data)
     }
 
-    let mut params = content_type.params();
+    // In the v2 API, the attachment record has already been created,
-    let boundary_pair = params.next().expect("No boundary provided");
+    // so the size limit needs to be adjusted to account for that.
-    let boundary = boundary_pair.1;
+    let size_adjust = match &attachment {
+        None => 0,                     // Legacy API
+        Some(a) => a.file_size as i64, // v2 API
+    };
 
     let size_limit = if let Some(ref user_uuid) = cipher.user_uuid {
         match CONFIG.user_attachment_limit() {
             Some(0) => err_discard!("Attachments are disabled", data),
             Some(limit_kb) => {
-                let left = (limit_kb * 1024) - Attachment::size_by_user(user_uuid, &conn);
+                let left = (limit_kb * 1024) - Attachment::size_by_user(user_uuid, &conn) + size_adjust;
                 if left <= 0 {
                     err_discard!("Attachment size limit reached! Delete some files to open space", data)
                 }
@@ -802,7 +875,7 @@ fn post_attachment(
         match CONFIG.org_attachment_limit() {
             Some(0) => err_discard!("Attachments are disabled", data),
             Some(limit_kb) => {
-                let left = (limit_kb * 1024) - Attachment::size_by_org(org_uuid, &conn);
+                let left = (limit_kb * 1024) - Attachment::size_by_org(org_uuid, &conn) + size_adjust;
                 if left <= 0 {
                     err_discard!("Attachment size limit reached! Delete some files to open space", data)
                 }
@@ -814,7 +887,12 @@ fn post_attachment(
         err_discard!("Cipher is neither owned by a user nor an organization", data);
     };
 
-    let base_path = Path::new(&CONFIG.attachments_folder()).join(&cipher.uuid);
+    let mut params = content_type.params();
+    let boundary_pair = params.next().expect("No boundary provided");
+    let boundary = boundary_pair.1;
+
+    let base_path = Path::new(&CONFIG.attachments_folder()).join(&cipher_uuid);
+    let mut path = PathBuf::new();
 
     let mut attachment_key = None;
     let mut error = None;
@@ -830,35 +908,81 @@ fn post_attachment(
                     }
                 }
                 "data" => {
-                    // This is provided by the client, don't trust it
+                    // In the legacy API, this is the encrypted filename
-                    let name = field.headers.filename.expect("No filename provided");
+                    // provided by the client, stored to the database as-is.
-
+                    // In the v2 API, this value doesn't matter, as it was
-                    let file_name = HEXLOWER.encode(&crypto::get_random(vec![0; 10]));
+                    // already provided and stored via an earlier API call.
-                    let path = base_path.join(&file_name);
+                    let encrypted_filename = field.headers.filename;
+
+                    // This random ID is used as the name of the file on disk.
+                    // In the legacy API, we need to generate this value here.
+                    // In the v2 API, we use the value from post_attachment_v2().
+                    let file_id = match &attachment {
+                        Some(attachment) => attachment.id.clone(), // v2 API
+                        None => crypto::generate_attachment_id(),  // Legacy API
+                    };
+                    path = base_path.join(&file_id);
 
                     let size =
                         match field.data.save().memory_threshold(0).size_limit(size_limit).with_path(path.clone()) {
                             SaveResult::Full(SavedData::File(_, size)) => size as i32,
                             SaveResult::Full(other) => {
-                                std::fs::remove_file(path).ok();
                                 error = Some(format!("Attachment is not a file: {:?}", other));
                                 return;
                             }
                             SaveResult::Partial(_, reason) => {
-                                std::fs::remove_file(path).ok();
                                 error = Some(format!("Attachment size limit exceeded with this file: {:?}", reason));
                                 return;
                             }
                             SaveResult::Error(e) => {
-                                std::fs::remove_file(path).ok();
                                 error = Some(format!("Error: {:?}", e));
                                 return;
                             }
                         };
 
-                    let mut attachment = Attachment::new(file_name, cipher.uuid.clone(), name, size);
+                    if let Some(attachment) = &mut attachment {
-                    attachment.akey = attachment_key.clone();
+                        // v2 API
-                    attachment.save(&conn).expect("Error saving attachment");
+
+                        // Check the actual size against the size initially provided by
+                        // the client. Upstream allows +/- 1 MiB deviation from this
+                        // size, but it's not clear when or why this is needed.
+                        const LEEWAY: i32 = 1024 * 1024; // 1 MiB
+                        let min_size = attachment.file_size - LEEWAY;
+                        let max_size = attachment.file_size + LEEWAY;
+
+                        if min_size <= size && size <= max_size {
+                            if size != attachment.file_size {
+                                // Update the attachment with the actual file size.
+                                attachment.file_size = size;
+                                attachment.save(conn).expect("Error updating attachment");
+                            }
+                        } else {
+                            attachment.delete(conn).ok();
+
+                            let err_msg = "Attachment size mismatch".to_string();
+                            error!("{} (expected within [{}, {}], got {})", err_msg, min_size, max_size, size);
+                            error = Some(err_msg);
+                        }
+                    } else {
+                        // Legacy API
+
+                        if encrypted_filename.is_none() {
+                            error = Some("No filename provided".to_string());
+                            return;
+                        }
+                        if attachment_key.is_none() {
+                            error = Some("No attachment key provided".to_string());
+                            return;
+                        }
+                        let attachment = Attachment::new(
+                            file_id,
+                            cipher_uuid.clone(),
+                            encrypted_filename.unwrap(),
+                            size,
+                            attachment_key.clone(),
+                        );
+                        attachment.save(conn).expect("Error saving attachment");
+                    }
                 }
                 _ => error!("Invalid multipart name"),
             }
@@ -866,11 +990,56 @@ fn post_attachment(
         .expect("Error processing multipart data");
 
     if let Some(ref e) = error {
+        std::fs::remove_file(path).ok();
         err!(e);
     }
 
     nt.send_cipher_update(UpdateType::CipherUpdate, &cipher, &cipher.update_users_revision(&conn));
 
+    Ok(cipher)
+}
+
+/// v2 API for uploading the actual data content of an attachment.
+/// This route needs a rank specified so that Rocket prioritizes the
+/// /ciphers/<uuid>/attachment/v2 route, which would otherwise conflict
+/// with this one.
+#[post("/ciphers/<uuid>/attachment/<attachment_id>", format = "multipart/form-data", data = "<data>", rank = 1)]
+fn post_attachment_v2_data(
+    uuid: String,
+    attachment_id: String,
+    data: Data,
+    content_type: &ContentType,
+    headers: Headers,
+    conn: DbConn,
+    nt: Notify,
+) -> EmptyResult {
+    let attachment = match Attachment::find_by_id(&attachment_id, &conn) {
+        Some(attachment) if uuid == attachment.cipher_uuid => Some(attachment),
+        Some(_) => err!("Attachment doesn't belong to cipher"),
+        None => err!("Attachment doesn't exist"),
+    };
+
+    save_attachment(attachment, uuid, data, content_type, &headers, &conn, nt)?;
+
+    Ok(())
+}
+
+/// Legacy API for creating an attachment associated with a cipher.
+#[post("/ciphers/<uuid>/attachment", format = "multipart/form-data", data = "<data>")]
+fn post_attachment(
+    uuid: String,
+    data: Data,
+    content_type: &ContentType,
+    headers: Headers,
+    conn: DbConn,
+    nt: Notify,
+) -> JsonResult {
+    // Setting this as None signifies to save_attachment() that it should create
+    // the attachment database record as well as saving the data to disk.
+    let attachment = None;
+
+    let cipher = save_attachment(attachment, uuid, data, content_type, &headers, &conn, nt)?;
+
     Ok(Json(cipher.to_json(&headers.host, &headers.user.uuid, &conn)))
 }
 

src/api/core/sends.rs

@@ -173,7 +173,7 @@ fn post_send_file(data: Data, content_type: &ContentType, headers: Headers, conn
 
     // Create the Send
     let mut send = create_send(data.data, headers.user.uuid.clone())?;
-    let file_id: String = data_encoding::HEXLOWER.encode(&crate::crypto::get_random(vec![0; 32]));
+    let file_id = crate::crypto::generate_send_id();
 
     if send.atype != SendType::File as i32 {
         err!("Send content is not a file");

src/crypto.rs

@@ -3,6 +3,7 @@
 //
 use std::num::NonZeroU32;
 
+use data_encoding::HEXLOWER;
 use ring::{digest, hmac, pbkdf2};
 
 use crate::error::Error;
@@ -28,8 +29,6 @@ pub fn verify_password_hash(secret: &[u8], salt: &[u8], previous: &[u8], iterati
 // HMAC
 //
 pub fn hmac_sign(key: &str, data: &str) -> String {
-    use data_encoding::HEXLOWER;
-
     let key = hmac::Key::new(hmac::HMAC_SHA1_FOR_LEGACY_USE_ONLY, key.as_bytes());
     let signature = hmac::sign(&key, data.as_bytes());
 
@@ -52,6 +51,20 @@ pub fn get_random(mut array: Vec<u8>) -> Vec<u8> {
     array
 }
 
+pub fn generate_id(num_bytes: usize) -> String {
+    HEXLOWER.encode(&get_random(vec![0; num_bytes]))
+}
+
+pub fn generate_send_id() -> String {
+    // Send IDs are globally scoped, so make them longer to avoid collisions.
+    generate_id(32) // 256 bits
+}
+
+pub fn generate_attachment_id() -> String {
+    // Attachment IDs are scoped to a cipher, so they can be smaller.
+    generate_id(10) // 80 bits
+}
+
 pub fn generate_token(token_size: u32) -> Result<String, Error> {
     // A u64 can represent all whole numbers up to 19 digits long.
     if token_size > 19 {

src/db/models/attachment.rs

@@ -1,3 +1,5 @@
+use std::io::ErrorKind;
+
 use serde_json::Value;
 
 use super::Cipher;
@@ -12,7 +14,7 @@ db_object! {
     pub struct Attachment {
         pub id: String,
         pub cipher_uuid: String,
-        pub file_name: String,
+        pub file_name: String, // encrypted
         pub file_size: i32,
         pub akey: Option<String>,
     }
@@ -20,13 +22,13 @@ db_object! {
 
 /// Local methods
 impl Attachment {
-    pub const fn new(id: String, cipher_uuid: String, file_name: String, file_size: i32) -> Self {
+    pub const fn new(id: String, cipher_uuid: String, file_name: String, file_size: i32, akey: Option<String>) -> Self {
         Self {
             id,
             cipher_uuid,
             file_name,
             file_size,
-            akey: None,
+            akey,
         }
     }
 
@@ -34,18 +36,17 @@ impl Attachment {
         format!("{}/{}/{}", CONFIG.attachments_folder(), self.cipher_uuid, self.id)
     }
 
-    pub fn to_json(&self, host: &str) -> Value {
+    pub fn get_url(&self, host: &str) -> String {
-        use crate::util::get_display_size;
+        format!("{}/attachments/{}/{}", host, self.cipher_uuid, self.id)
-
+    }
-        let web_path = format!("{}/attachments/{}/{}", host, self.cipher_uuid, self.id);
-        let display_size = get_display_size(self.file_size);
 
+    pub fn to_json(&self, host: &str) -> Value {
         json!({
             "Id": self.id,
-            "Url": web_path,
+            "Url": self.get_url(host),
             "FileName": self.file_name,
             "Size": self.file_size.to_string(),
-            "SizeName": display_size,
+            "SizeName": crate::util::get_display_size(self.file_size),
             "Key": self.akey,
             "Object": "attachment"
         })
@@ -91,7 +92,7 @@ impl Attachment {
         }
     }
 
-    pub fn delete(self, conn: &DbConn) -> EmptyResult {
+    pub fn delete(&self, conn: &DbConn) -> EmptyResult {
         db_run! { conn: {
             crate::util::retry(
                 || diesel::delete(attachments::table.filter(attachments::id.eq(&self.id))).execute(conn),
@@ -99,8 +100,19 @@ impl Attachment {
             )
             .map_res("Error deleting attachment")?;
 
-            crate::util::delete_file(&self.get_file_path())?;
+            let file_path = &self.get_file_path();
-            Ok(())
+
+            match crate::util::delete_file(file_path) {
+                // Ignore "file not found" errors. This can happen when the
+                // upstream caller has already cleaned up the file as part of
+                // its own error handling.
+                Err(e) if e.kind() == ErrorKind::NotFound => {
+                    debug!("File '{}' already deleted.", file_path);
+                    Ok(())
+                }
+                Err(e) => Err(e.into()),
+                _ => Ok(()),
+            }
         }}
     }