Skip to content

Commit 0e29981

Browse files
zupzupzupzup
authored
Upload file sizes and validation (#593)
1 parent 518982f commit 0e29981

File tree

12 files changed

+134
-11
lines changed

12 files changed

+134
-11
lines changed

‎CHANGELOG.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
* Add endpoints to fetch files as base64 for identity, contacts, companies and bills
44
* Add option to remove files for identity, contacts and companies - if the file upload id in the payload is missing, it's ignored, if it's explicitly set to undefined, the file is removed
55
* Fix blank email validation for contacts and identities
6+
* Add different file size limits for pictures (avatar/logo - 20k) and documents (invoices, registration, passport - 1mb) as well as an upper limit for bill files (100)
7+
* This limit is checked at creation/update time, not at the time of uploading a temporary file
68

79
# 0.4.2
810

‎crates/bcr-ebill-api/src/constants.rs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
// Validation
2-
pub const MAX_FILE_SIZE_BYTES: usize = 1_000_000; // ~1 MB
2+
pub const MAX_DOCUMENT_FILE_SIZE_BYTES: usize = 1_000_000; // ~1 MB
3+
pub const MAX_PICTURE_FILE_SIZE_BYTES: usize = 20_000; // ~20 KB
4+
pub const MAX_BILL_ATTACHMENTS: usize = 100;
35
pub const MAX_FILE_NAME_CHARACTERS: usize = 200;
46
pub const VALID_FILE_MIME_TYPES: [&str; 3] = ["image/jpeg", "image/png", "application/pdf"];
57

‎crates/bcr-ebill-api/src/service/bill_service/issue.rs‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,10 @@
11
use super::{BillAction, BillServiceApi, Result, error::Error, service::BillService};
2-
use crate::{data::validate_node_id_network, get_config, util};
2+
use crate::{
3+
constants::MAX_BILL_ATTACHMENTS,
4+
data::validate_node_id_network,
5+
get_config,
6+
util::{self, file::UploadFileType},
7+
};
38
use bcr_ebill_core::{
49
File, PublicKey, Validate, ValidationError,
510
bill::{
@@ -23,7 +28,14 @@ impl BillService {
2328
bill_id: &BillId,
2429
public_key: &PublicKey,
2530
relay_url: &str,
31+
upload_file_type: UploadFileType,
2632
) -> Result<File> {
33+
// validate file size for upload file type
34+
if !upload_file_type.check_file_size(file_bytes.len()) {
35+
return Err(Error::Validation(ValidationError::FileIsTooBig(
36+
upload_file_type.max_file_size(),
37+
)));
38+
}
2739
let file_hash = util::sha256_hash(file_bytes);
2840
let encrypted = util::crypto::encrypt_ecies(file_bytes, public_key)?;
2941
let nostr_hash = self.file_upload_client.upload(relay_url, encrypted).await?;
@@ -139,6 +151,10 @@ impl BillService {
139151
public_key: keys.pub_key(),
140152
};
141153

154+
if data.file_upload_ids.len() > MAX_BILL_ATTACHMENTS {
155+
return Err(Error::Validation(ValidationError::TooManyFiles));
156+
}
157+
142158
let mut bill_files: Vec<File> = vec![];
143159
if let Some(nostr_relay) = nostr_relays.first() {
144160
for file_upload_id in data.file_upload_ids.iter() {
@@ -154,6 +170,7 @@ impl BillService {
154170
&bill_id,
155171
&public_key,
156172
nostr_relay,
173+
UploadFileType::Document,
157174
)
158175
.await?,
159176
);

‎crates/bcr-ebill-api/src/service/bill_service/service.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ use crate::persistence::contact::ContactStoreApi;
2626
use crate::persistence::file_upload::FileUploadStoreApi;
2727
use crate::persistence::identity::{IdentityChainStoreApi, IdentityStoreApi};
2828
use crate::util::BcrKeys;
29+
use crate::util::file::UploadFileType;
2930
use crate::{external, util};
3031
use async_trait::async_trait;
3132
use bcr_ebill_core::bill::validation::get_expiration_deadline_base_for_req_to_pay;
@@ -655,6 +656,7 @@ impl BillService {
655656
bill_id,
656657
&node_id.pub_key(),
657658
relay_url,
659+
UploadFileType::Document,
658660
)
659661
.await?;
660662
result.push(file_storage::to_url(relay_url, &uploaded_file.nostr_hash)?);

‎crates/bcr-ebill-api/src/service/company_service.rs‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ use crate::get_config;
1919
use crate::persistence::company::{CompanyChainStoreApi, CompanyStoreApi};
2020
use crate::persistence::identity::IdentityChainStoreApi;
2121
use crate::util::BcrKeys;
22+
use crate::util::file::UploadFileType;
2223
use crate::{
2324
persistence::{
2425
contact::ContactStoreApi, file_upload::FileUploadStoreApi, identity::IdentityStoreApi,
@@ -152,6 +153,7 @@ impl CompanyService {
152153
id: &NodeId,
153154
public_key: &PublicKey,
154155
relay_url: &str,
156+
upload_file_type: UploadFileType,
155157
) -> Result<Option<File>> {
156158
if let Some(upload_id) = upload_id {
157159
debug!("processing upload file for company {id}: {upload_id:?}");
@@ -160,6 +162,12 @@ impl CompanyService {
160162
.read_temp_upload_file(upload_id)
161163
.await
162164
.map_err(|_| crate::service::Error::NoFileForFileUploadId)?;
165+
// validate file size for upload file type
166+
if !upload_file_type.check_file_size(file_bytes.len()) {
167+
return Err(crate::service::Error::Validation(
168+
ValidationError::FileIsTooBig(upload_file_type.max_file_size()),
169+
));
170+
}
163171
let file = self
164172
.encrypt_and_upload_file(file_name, file_bytes, id, public_key, relay_url)
165173
.await?;
@@ -302,6 +310,7 @@ impl CompanyServiceApi for CompanyService {
302310
&id,
303311
&full_identity.key_pair.pub_key(),
304312
nostr_relay,
313+
UploadFileType::Document,
305314
)
306315
.await?;
307316

@@ -311,6 +320,7 @@ impl CompanyServiceApi for CompanyService {
311320
&id,
312321
&full_identity.key_pair.pub_key(),
313322
nostr_relay,
323+
UploadFileType::Picture,
314324
)
315325
.await?;
316326
(proof_of_registration_file, logo_file)
@@ -521,6 +531,7 @@ impl CompanyServiceApi for CompanyService {
521531
id,
522532
&full_identity.key_pair.pub_key(),
523533
nostr_relay,
534+
UploadFileType::Picture,
524535
)
525536
.await?
526537
};
@@ -537,6 +548,7 @@ impl CompanyServiceApi for CompanyService {
537548
id,
538549
&full_identity.key_pair.pub_key(),
539550
nostr_relay,
551+
UploadFileType::Document,
540552
)
541553
.await?
542554
};

‎crates/bcr-ebill-api/src/service/contact_service.rs‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ use crate::{
2525
persistence::{
2626
contact::ContactStoreApi, file_upload::FileUploadStoreApi, identity::IdentityStoreApi,
2727
},
28-
util,
28+
util::{self, file::UploadFileType},
2929
};
3030

3131
use super::Result;
@@ -156,6 +156,7 @@ impl ContactService {
156156
id: &NodeId,
157157
public_key: &PublicKey,
158158
relay_url: &str,
159+
upload_file_type: UploadFileType,
159160
) -> Result<Option<File>> {
160161
if let Some(upload_id) = upload_id {
161162
debug!("processing upload file for contact {id}: {upload_id:?}");
@@ -164,6 +165,12 @@ impl ContactService {
164165
.read_temp_upload_file(upload_id)
165166
.await
166167
.map_err(|_| crate::service::Error::NoFileForFileUploadId)?;
168+
// validate file size for upload file type
169+
if !upload_file_type.check_file_size(file_bytes.len()) {
170+
return Err(crate::service::Error::Validation(
171+
ValidationError::FileIsTooBig(upload_file_type.max_file_size()),
172+
));
173+
}
167174
let file = self
168175
.encrypt_and_save_uploaded_file(file_name, file_bytes, id, public_key, relay_url)
169176
.await?;
@@ -373,6 +380,7 @@ impl ContactServiceApi for ContactService {
373380
node_id,
374381
&identity_public_key,
375382
nostr_relay,
383+
UploadFileType::Picture,
376384
)
377385
.await?;
378386
// only override the picture, if there is a new one
@@ -388,6 +396,7 @@ impl ContactServiceApi for ContactService {
388396
node_id,
389397
&identity_public_key,
390398
nostr_relay,
399+
UploadFileType::Document,
391400
)
392401
.await?;
393402
// only override the document, if there is a new one
@@ -446,6 +455,7 @@ impl ContactServiceApi for ContactService {
446455
node_id,
447456
&identity_public_key,
448457
nostr_relay,
458+
UploadFileType::Picture,
449459
)
450460
.await?;
451461

@@ -455,6 +465,7 @@ impl ContactServiceApi for ContactService {
455465
node_id,
456466
&identity_public_key,
457467
nostr_relay,
468+
UploadFileType::Document,
458469
)
459470
.await?;
460471
(avatar_file, proof_document_file)
@@ -561,6 +572,7 @@ impl ContactServiceApi for ContactService {
561572
node_id,
562573
&identity_public_key,
563574
nostr_relay,
575+
UploadFileType::Picture,
564576
)
565577
.await?;
566578

@@ -570,6 +582,7 @@ impl ContactServiceApi for ContactService {
570582
node_id,
571583
&identity_public_key,
572584
nostr_relay,
585+
UploadFileType::Document,
573586
)
574587
.await?;
575588
(avatar_file, proof_document_file)

‎crates/bcr-ebill-api/src/service/file_upload_service.rs‎

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
use super::{Error, Result};
2-
use crate::constants::{MAX_FILE_NAME_CHARACTERS, MAX_FILE_SIZE_BYTES, VALID_FILE_MIME_TYPES};
2+
use crate::constants::{
3+
MAX_DOCUMENT_FILE_SIZE_BYTES, MAX_FILE_NAME_CHARACTERS, VALID_FILE_MIME_TYPES,
4+
};
35
use crate::data::UploadFileResult;
46
use crate::persistence::file_upload::FileUploadStoreApi;
57
use crate::{persistence, util};
@@ -41,9 +43,13 @@ impl ServiceTraitBounds for FileUploadService {}
4143
#[cfg_attr(not(target_arch = "wasm32"), async_trait)]
4244
impl FileUploadServiceApi for FileUploadService {
4345
async fn validate_attached_file(&self, file: &dyn util::file::UploadFileHandler) -> Result<()> {
44-
if file.len() > MAX_FILE_SIZE_BYTES as u64 {
46+
if file.is_empty() {
47+
return Err(Error::Validation(ValidationError::FileIsEmpty));
48+
}
49+
50+
if file.len() > MAX_DOCUMENT_FILE_SIZE_BYTES {
4551
return Err(Error::Validation(ValidationError::FileIsTooBig(
46-
MAX_FILE_SIZE_BYTES,
52+
MAX_DOCUMENT_FILE_SIZE_BYTES,
4753
)));
4854
}
4955

@@ -232,7 +238,8 @@ mod tests {
232238
async fn validate_attached_file_checks_file_size() {
233239
let mut file = MockUploadFileHandler::new();
234240
file.expect_len()
235-
.returning(move || MAX_FILE_SIZE_BYTES as u64 * 2);
241+
.returning(move || MAX_DOCUMENT_FILE_SIZE_BYTES * 2);
242+
file.expect_is_empty().returning(move || false);
236243

237244
let service = get_service(MockFileUploadStoreApiMock::new());
238245
let res = service.validate_attached_file(&file).await;
@@ -244,6 +251,7 @@ mod tests {
244251
async fn validate_attached_file_checks_file_name() {
245252
let mut file = MockUploadFileHandler::new();
246253
file.expect_len().returning(move || 100);
254+
file.expect_is_empty().returning(move || false);
247255
file.expect_name().returning(move || None);
248256

249257
let service = get_service(MockFileUploadStoreApiMock::new());
@@ -256,6 +264,7 @@ mod tests {
256264
async fn validate_attached_file_checks_file_name_empty() {
257265
let mut file = MockUploadFileHandler::new();
258266
file.expect_len().returning(move || 100);
267+
file.expect_is_empty().returning(move || false);
259268
file.expect_name().returning(move || Some(String::from("")));
260269

261270
let service = get_service(MockFileUploadStoreApiMock::new());
@@ -270,6 +279,7 @@ mod tests {
270279
file.expect_len().returning(move || 100);
271280
file.expect_name()
272281
.returning(move || Some("abc".repeat(100)));
282+
file.expect_is_empty().returning(move || false);
273283

274284
let service = get_service(MockFileUploadStoreApiMock::new());
275285
let res = service.validate_attached_file(&file).await;
@@ -283,6 +293,7 @@ mod tests {
283293
file.expect_len().returning(move || 100);
284294
file.expect_name()
285295
.returning(move || Some(String::from("goodname")));
296+
file.expect_is_empty().returning(move || false);
286297
file.expect_detect_content_type()
287298
.returning(move || Err(std::io::Error::other("test error")));
288299

@@ -298,6 +309,7 @@ mod tests {
298309
file.expect_len().returning(move || 100);
299310
file.expect_name()
300311
.returning(move || Some(String::from("goodname")));
312+
file.expect_is_empty().returning(move || false);
301313
file.expect_detect_content_type()
302314
.returning(move || Ok(None));
303315

@@ -307,12 +319,24 @@ mod tests {
307319
assert!(res.is_err());
308320
}
309321

322+
#[tokio::test]
323+
async fn validate_attached_file_checks_file_empty() {
324+
let mut file = MockUploadFileHandler::new();
325+
file.expect_is_empty().returning(move || true);
326+
327+
let service = get_service(MockFileUploadStoreApiMock::new());
328+
let res = service.validate_attached_file(&file).await;
329+
330+
assert!(res.is_err());
331+
}
332+
310333
#[tokio::test]
311334
async fn validate_attached_file_checks_file_type_not_in_list() {
312335
let mut file = MockUploadFileHandler::new();
313336
file.expect_len().returning(move || 100);
314337
file.expect_name()
315338
.returning(move || Some(String::from("goodname")));
339+
file.expect_is_empty().returning(move || false);
316340
file.expect_detect_content_type()
317341
.returning(move || Ok(Some(String::from("invalidfile"))));
318342

@@ -326,6 +350,7 @@ mod tests {
326350
async fn validate_attached_file_checks_valid() {
327351
let mut file = MockUploadFileHandler::new();
328352
file.expect_len().returning(move || 100);
353+
file.expect_is_empty().returning(move || false);
329354
file.expect_name()
330355
.returning(move || Some(String::from("goodname")));
331356
file.expect_detect_content_type()

0 commit comments

Comments
 (0)