200 character limit file uploads, always return a JSON response, rest… (#385)

* 200 character limit file uploads, always return a JSON response, restructure company, add validation for file upload ids and add endpoint for getting temp files

Author: zupzup

src/constants.rs

@@ -18,7 +18,7 @@ pub const RECOURSE_DEADLINE_SECONDS: u64 = 86400 * 2; // 2 days
 
 // Validation
 pub const MAX_FILE_SIZE_BYTES: usize = 1_000_000; // ~1 MB
-pub const MAX_FILE_NAME_CHARACTERS: usize = 50;
+pub const MAX_FILE_NAME_CHARACTERS: usize = 200;
 pub const VALID_FILE_MIME_TYPES: [&str; 3] = ["image/jpeg", "image/png", "application/pdf"];
 pub const VALID_CURRENCIES: [&str; 2] = ["sat", "crsat"];
 

src/service/bill_service.rs

@@ -158,6 +158,10 @@ pub enum Error {
     #[error("Bill is in recourse and waiting for payment")]
     BillIsInRecourseAndWaitingForPayment,
 
+    /// error returned if the given file upload id is not a temp file we have
+    #[error("No file found for file upload id")]
+    NoFileForFileUploadId,
+
     /// errors that stem from interacting with a blockchain
     #[error("Blockchain error: {0}")]
     Blockchain(#[from] blockchain::Error),
@@ -209,6 +213,7 @@ impl<'r, 'o: 'r> Responder<'r, 'o> for Error {
             | Error::CallerIsNotRecoursee
             | Error::RequestAlreadyRejected
             | Error::CallerIsNotHolder
+            | Error::NoFileForFileUploadId
             | Error::InvalidOperation => {
                 let body =
                     ErrorResponse::new("bad_request", self.to_string(), 400).to_json_string();
@@ -2030,7 +2035,8 @@ impl BillServiceApi for BillService {
             let files = self
                 .file_upload_store
                 .read_temp_upload_files(upload_id)
-                .await?;
+                .await
+                .map_err(|_| Error::NoFileForFileUploadId)?;
             for (file_name, file_bytes) in files {
                 bill_files.push(
                     self.encrypt_and_save_uploaded_file(

src/service/company_service.rs

@@ -143,7 +143,8 @@ impl CompanyService {
             let files = self
                 .file_upload_store
                 .read_temp_upload_files(upload_id)
-                .await?;
+                .await
+                .map_err(|_| crate::service::Error::NoFileForFileUploadId)?;
             if !files.is_empty() {
                 let (file_name, file_bytes) = &files[0];
                 let file = self

src/service/contact_service.rs

@@ -106,7 +106,8 @@ impl ContactService {
             let files = self
                 .file_upload_store
                 .read_temp_upload_files(upload_id)
-                .await?;
+                .await
+                .map_err(|_| crate::service::Error::NoFileForFileUploadId)?;
             if !files.is_empty() {
                 let (file_name, file_bytes) = &files[0];
                 let file = self

src/service/file_upload_service.rs

@@ -12,11 +12,14 @@ pub trait FileUploadServiceApi: Send + Sync {
     /// validates the given uploaded file
     async fn validate_attached_file(&self, file: &dyn util::file::UploadFileHandler) -> Result<()>;
 
-    /// uploads files for use in a bill
+    /// uploads files
     async fn upload_files(
         &self,
         files: Vec<&dyn util::file::UploadFileHandler>,
     ) -> Result<UploadFilesResponse>;
+
+    /// returns a temp upload file
+    async fn get_temp_file(&self, file_upload_id: &str) -> Result<Option<(String, Vec<u8>)>>;
 }
 
 #[derive(Clone)]
@@ -101,6 +104,20 @@ impl FileUploadServiceApi for FileUploadService {
         }
         Ok(UploadFilesResponse { file_upload_id })
     }
+
+    async fn get_temp_file(&self, file_upload_id: &str) -> Result<Option<(String, Vec<u8>)>> {
+        let mut files = self
+            .file_upload_store
+            .read_temp_upload_files(file_upload_id)
+            .await
+            .map_err(|_| crate::service::Error::NoFileForFileUploadId)?;
+        // return the first file in the folder
+        if !files.is_empty() {
+            let (file_name, file_bytes) = files.remove(0);
+            return Ok(Some((file_name, file_bytes)));
+        }
+        Ok(None)
+    }
 }
 
 #[cfg(test)]
@@ -333,4 +350,51 @@ mod tests {
 
         assert!(res.is_ok());
     }
+
+    #[tokio::test]
+    async fn get_temp_file_baseline() {
+        let mut storage = MockFileUploadStoreApi::new();
+        storage.expect_read_temp_upload_files().returning(|_| {
+            Ok(vec![(
+                "some_file".to_string(),
+                "hello_world".as_bytes().to_vec(),
+            )])
+        });
+        let service = get_service(storage);
+
+        let res = service.get_temp_file("1234").await;
+        assert!(res.is_ok());
+        assert_eq!(
+            res.unwrap(),
+            Some(("some_file".to_string(), "hello_world".as_bytes().to_vec()))
+        );
+    }
+
+    #[tokio::test]
+    async fn get_temp_file_no_file() {
+        let mut storage = MockFileUploadStoreApi::new();
+        storage
+            .expect_read_temp_upload_files()
+            .returning(|_| Ok(vec![]));
+        let service = get_service(storage);
+
+        let res = service.get_temp_file("1234").await;
+        assert!(res.is_ok());
+        assert_eq!(res.unwrap(), None);
+    }
+
+    #[tokio::test]
+    async fn get_temp_file_err() {
+        let mut storage = MockFileUploadStoreApi::new();
+        storage.expect_read_temp_upload_files().returning(|_| {
+            Err(persistence::Error::Io(std::io::Error::new(
+                std::io::ErrorKind::Other,
+                "test error",
+            )))
+        });
+        let service = get_service(storage);
+
+        let res = service.get_temp_file("1234").await;
+        assert!(res.is_err());
+    }
 }

src/service/identity_service.rs

@@ -122,7 +122,8 @@ impl IdentityService {
             let files = self
                 .file_upload_store
                 .read_temp_upload_files(upload_id)
-                .await?;
+                .await
+                .map_err(|_| crate::service::Error::NoFileForFileUploadId)?;
             if !files.is_empty() {
                 let (file_name, file_bytes) = &files[0];
                 let file = self

src/service/mod.rs

@@ -83,6 +83,10 @@ pub enum Error {
     /// std io
     #[error("Io error: {0}")]
     Io(#[from] std::io::Error),
+
+    /// error returned if the given file upload id is not a temp file we have
+    #[error("No file found for file upload id")]
+    NoFileForFileUploadId,
 }
 
 /// Map from service errors directly to rocket status codes. This allows us to
@@ -91,6 +95,15 @@ pub enum Error {
 impl<'r, 'o: 'r> Responder<'r, 'o> for Error {
     fn respond_to(self, req: &rocket::Request) -> rocket::response::Result<'o> {
         match self {
+            Error::NoFileForFileUploadId => {
+                let body =
+                    ErrorResponse::new("bad_request", self.to_string(), 400).to_json_string();
+                Response::build()
+                    .status(Status::BadRequest)
+                    .header(ContentType::JSON)
+                    .sized_body(body.len(), Cursor::new(body))
+                    .ok()
+            }
             // for now, DHT errors are InternalServerError
             Error::Dht(e) => {
                 error!("{e}");

src/util/file.rs

@@ -50,6 +50,17 @@ impl UploadFileHandler for TempFile<'_> {
     }
 }
 
+pub fn validate_file_upload_id(file_upload_id: &Option<String>) -> crate::service::Result<()> {
+    if let Some(ref id) = file_upload_id {
+        if id.is_empty() {
+            return Err(crate::service::Error::Validation(
+                "Empty string is not a valid file upload id".to_string(),
+            ));
+        }
+    }
+    Ok(())
+}
+
 /// Function to sanitize the filename by removing unwanted characters.
 pub fn sanitize_filename(filename: &str) -> String {
     filename
@@ -146,4 +157,10 @@ mod tests {
             String::from("file_name_00000000-0000-0000-0000-000000000000.tar.gz")
         );
     }
+
+    #[test]
+    fn validate_file_upload_id_baseline() {
+        assert!(validate_file_upload_id(&Some(String::from(""))).is_err(),);
+        assert!(validate_file_upload_id(&Some(String::from("test"))).is_ok(),);
+    }
 }

src/web/data.rs

@@ -1,4 +1,4 @@
-use crate::service::contact_service::LightIdentityPublicDataWithAddress;
+use crate::service::contact_service::{ContactType, LightIdentityPublicDataWithAddress};
 use crate::service::identity_service::IdentityType;
 use crate::service::{
     bill_service::LightBitcreditBillToReturn,
@@ -13,6 +13,18 @@ use serde::{Deserialize, Serialize};
 use std::fmt;
 use utoipa::ToSchema;
 
+/// A dummy response type signaling success of a request
+#[derive(Debug, Serialize, ToSchema)]
+pub struct SuccessResponse {
+    pub success: bool,
+}
+
+impl SuccessResponse {
+    pub fn new() -> Self {
+        Self { success: true }
+    }
+}
+
 #[derive(Debug, Serialize, ToSchema)]
 pub struct EndorsementsResponse {
     pub endorsements: Vec<Endorsement>,
@@ -450,3 +462,68 @@ pub struct SeedPhrase {
     /// The seed phrase of the current private key
     pub seed_phrase: String,
 }
+
+// Company
+#[derive(Debug, Serialize, Deserialize, Clone)]
+pub struct CreateCompanyPayload {
+    pub name: String,
+    pub country_of_registration: String,
+    pub city_of_registration: String,
+    #[serde(flatten)]
+    pub postal_address: PostalAddress,
+    pub email: String,
+    pub registration_number: String,
+    pub registration_date: String,
+    pub proof_of_registration_file_upload_id: Option<String>,
+    pub logo_file_upload_id: Option<String>,
+}
+
+#[derive(Debug, Serialize, Deserialize, Clone)]
+pub struct EditCompanyPayload {
+    pub id: String,
+    pub name: Option<String>,
+    pub email: Option<String>,
+    #[serde(flatten)]
+    pub postal_address: OptionalPostalAddress,
+    pub logo_file_upload_id: Option<String>,
+}
+
+#[derive(Debug, Serialize, Deserialize, Clone)]
+pub struct AddSignatoryPayload {
+    pub id: String,
+    pub signatory_node_id: String,
+}
+
+#[derive(Debug, Serialize, Deserialize, Clone)]
+pub struct RemoveSignatoryPayload {
+    pub id: String,
+    pub signatory_node_id: String,
+}
+
+#[derive(Debug, Serialize, Deserialize, Clone)]
+pub struct ListSignatoriesResponse {
+    pub signatories: Vec<SignatoryResponse>,
+}
+
+#[derive(Debug, Serialize, Deserialize, Clone)]
+pub struct SignatoryResponse {
+    #[serde(rename = "type")]
+    pub t: ContactType,
+    pub node_id: String,
+    pub name: String,
+    #[serde(flatten)]
+    pub postal_address: PostalAddress,
+    pub avatar_file: Option<File>,
+}
+
+impl From<Contact> for SignatoryResponse {
+    fn from(value: Contact) -> Self {
+        Self {
+            t: value.t,
+            node_id: value.node_id,
+            name: value.name,
+            postal_address: value.postal_address,
+            avatar_file: value.avatar_file,
+        }
+    }
+}