diff --git a/src/ic_oss/src/file.rs b/src/ic_oss/src/file.rs index 46197c8..c2aabc6 100644 --- a/src/ic_oss/src/file.rs +++ b/src/ic_oss/src/file.rs @@ -31,7 +31,7 @@ pub struct UploadFileChunksResult { impl Client { pub fn new(agent: Arc, bucket: Principal) -> Client { Client { - chunk_size: MAX_CHUNK_SIZE, + chunk_size: CHUNK_SIZE, concurrency: 20, agent, bucket, @@ -40,7 +40,7 @@ impl Client { } pub fn set_chunk_size(&mut self, chunk_size: u32) { - if chunk_size > 1024 && chunk_size <= MAX_CHUNK_SIZE { + if chunk_size > 1024 && chunk_size <= CHUNK_SIZE { self.chunk_size = chunk_size; } } diff --git a/src/ic_oss_bucket/src/api_http.rs b/src/ic_oss_bucket/src/api_http.rs index 59f1d50..6a49993 100644 --- a/src/ic_oss_bucket/src/api_http.rs +++ b/src/ic_oss_bucket/src/api_http.rs @@ -4,7 +4,7 @@ use hyperx::header::{Charset, ContentDisposition, DispositionParam, DispositionT use hyperx::header::{ContentRangeSpec, Header, IfRange, Range, Raw}; use ic_http_certification::{HeaderField, HttpRequest}; use ic_oss_types::{ - file::{UrlFileParam, MAX_CHUNK_SIZE, MAX_FILE_SIZE_PER_CALL}, + file::{UrlFileParam, CHUNK_SIZE, MAX_FILE_SIZE_PER_CALL}, to_cbor_bytes, }; use ic_stable_structures::Storable; @@ -343,10 +343,10 @@ fn range_response( metadata: store::FileMetadata, (start, end): (u64, u64), ) -> HttpStreamingResponse { - let chunk_index = start / MAX_CHUNK_SIZE as u64; - let chunk_offset = (start % MAX_CHUNK_SIZE as u64) as usize; - let chunk_end = end / MAX_CHUNK_SIZE as u64; - let end_offset = (end % MAX_CHUNK_SIZE as u64) as usize; + let chunk_index = start / CHUNK_SIZE as u64; + let chunk_offset = (start % CHUNK_SIZE as u64) as usize; + let chunk_end = end / CHUNK_SIZE as u64; + let end_offset = (end % CHUNK_SIZE as u64) as usize; let mut body = ByteBuf::with_capacity((end + 1 - start) as usize); for i in chunk_index..=chunk_end { @@ -357,7 +357,7 @@ fn range_response( let end = if i == chunk_end { end_offset } else { - MAX_CHUNK_SIZE as usize - 1 + CHUNK_SIZE as usize - 1 }; if end >= chunk.len() { diff --git a/src/ic_oss_bucket/src/api_update.rs b/src/ic_oss_bucket/src/api_update.rs index 6aa3767..953f952 100644 --- a/src/ic_oss_bucket/src/api_update.rs +++ b/src/ic_oss_bucket/src/api_update.rs @@ -66,7 +66,7 @@ fn create_file( Err("content size mismatch".to_string())?; } - for (i, chunk) in content.chunks(MAX_CHUNK_SIZE as usize).enumerate() { + for (i, chunk) in content.chunks(CHUNK_SIZE as usize).enumerate() { store::fs::update_chunk(id, i as u32, now_ms, chunk.to_vec(), |_| Ok(()))?; } diff --git a/src/ic_oss_bucket/src/permission.rs b/src/ic_oss_bucket/src/permission.rs index 65043c9..dab514e 100644 --- a/src/ic_oss_bucket/src/permission.rs +++ b/src/ic_oss_bucket/src/permission.rs @@ -1,5 +1,7 @@ use candid::Principal; -use ic_oss_types::permission::{Operation, Permission, PermissionChecker, Policies, Resource}; +use ic_oss_types::permission::{ + Operation, Permission, PermissionChecker, PermissionCheckerAny, Policies, Resource, +}; use crate::store::fs; @@ -10,7 +12,7 @@ pub fn check_bucket_read(ps: &Policies, bucket: &Principal) -> bool { operation: Operation::Read, constraint: Some(Resource::Other("Info".to_string())), }, - bucket.to_string().as_str(), + bucket.to_string(), ) } @@ -21,20 +23,16 @@ pub fn check_folder_list(ps: &Policies, bucket: &Principal, parent: u32) -> bool operation: Operation::List, constraint: Some(Resource::Folder), }, - bucket.to_string().as_str(), + bucket.to_string(), ) { - let ancestors: Vec = fs::get_ancestors(parent) - .into_iter() - .map(|f| f.id.to_string()) - .collect(); - let rs: Vec<&str> = ancestors.iter().map(|id| id.as_str()).collect(); - if !ps.has_permission( + let ancestors = fs::get_ancestors(parent); + if !ps.has_permission_any( &Permission { resource: Resource::Folder, operation: Operation::List, constraint: None, }, - rs.as_slice(), + &ancestors, ) { return false; } @@ -49,20 +47,16 @@ pub fn check_folder_read(ps: &Policies, bucket: &Principal, id: u32) -> bool { operation: Operation::Read, constraint: Some(Resource::Folder), }, - bucket.to_string().as_str(), + bucket.to_string(), ) { - let ancestors: Vec = fs::get_ancestors(id) - .into_iter() - .map(|f| f.id.to_string()) - .collect(); - let rs: Vec<&str> = ancestors.iter().map(|id| id.as_str()).collect(); - if !ps.has_permission( + let ancestors = fs::get_ancestors(id); + if !ps.has_permission_any( &Permission { resource: Resource::Folder, operation: Operation::Read, constraint: Some(Resource::Folder), }, - rs.as_slice(), + &ancestors, ) { return false; } @@ -77,20 +71,16 @@ pub fn check_file_list(ps: &Policies, bucket: &Principal, parent: u32) -> bool { operation: Operation::List, constraint: Some(Resource::File), }, - bucket.to_string().as_str(), + bucket.to_string(), ) { - let ancestors: Vec = fs::get_ancestors(parent) - .into_iter() - .map(|f| f.id.to_string()) - .collect(); - let rs: Vec<&str> = ancestors.iter().map(|id| id.as_str()).collect(); - if !ps.has_permission( + let ancestors = fs::get_ancestors(parent); + if !ps.has_permission_any( &Permission { resource: Resource::Folder, operation: Operation::List, constraint: Some(Resource::File), }, - rs.as_slice(), + &ancestors, ) { return false; } @@ -105,27 +95,23 @@ pub fn check_file_read(ps: &Policies, bucket: &Principal, id: u32, parent: u32) operation: Operation::Read, constraint: None, }, - id.to_string().as_str(), + id.to_string(), ) && !ps.has_permission( &Permission { resource: Resource::Bucket, operation: Operation::Read, constraint: Some(Resource::File), }, - bucket.to_string().as_str(), + bucket.to_string(), ) { - let ancestors: Vec = fs::get_ancestors(parent) - .into_iter() - .map(|f| f.id.to_string()) - .collect(); - let rs: Vec<&str> = ancestors.iter().map(|id| id.as_str()).collect(); - if !ps.has_permission( + let ancestors = fs::get_ancestors(parent); + if !ps.has_permission_any( &Permission { resource: Resource::Folder, operation: Operation::Read, constraint: Some(Resource::File), }, - rs.as_slice(), + &ancestors, ) { return false; } @@ -140,20 +126,16 @@ pub fn check_file_create(ps: &Policies, bucket: &Principal, parent: u32) -> bool operation: Operation::Write, constraint: Some(Resource::File), }, - bucket.to_string().as_str(), + bucket.to_string(), ) { - let ancestors: Vec = fs::get_ancestors(parent) - .into_iter() - .map(|f| f.id.to_string()) - .collect(); - let rs: Vec<&str> = ancestors.iter().map(|id| id.as_str()).collect(); - if !ps.has_permission( + let ancestors = fs::get_ancestors(parent); + if !ps.has_permission_any( &Permission { resource: Resource::Folder, operation: Operation::Write, constraint: Some(Resource::File), }, - rs.as_slice(), + &ancestors, ) { return false; } @@ -168,20 +150,16 @@ pub fn check_file_delete(ps: &Policies, bucket: &Principal, parent: u32) -> bool operation: Operation::Delete, constraint: Some(Resource::File), }, - bucket.to_string().as_str(), + bucket.to_string(), ) { - let ancestors: Vec = fs::get_ancestors(parent) - .into_iter() - .map(|f| f.id.to_string()) - .collect(); - let rs: Vec<&str> = ancestors.iter().map(|id| id.as_str()).collect(); - if !ps.has_permission( + let ancestors = fs::get_ancestors(parent); + if !ps.has_permission_any( &Permission { resource: Resource::Folder, operation: Operation::Delete, constraint: Some(Resource::File), }, - rs.as_slice(), + &ancestors, ) { return false; } @@ -196,7 +174,7 @@ pub fn check_file_update(ps: &Policies, bucket: &Principal, id: u32, parent: u32 operation: Operation::Write, constraint: None, }, - id.to_string().as_str(), + id.to_string(), ) { return check_file_create(ps, bucket, parent); } @@ -210,20 +188,16 @@ pub fn check_folder_create(ps: &Policies, bucket: &Principal, parent: u32) -> bo operation: Operation::Write, constraint: Some(Resource::Folder), }, - bucket.to_string().as_str(), + bucket.to_string(), ) { - let ancestors: Vec = fs::get_ancestors(parent) - .into_iter() - .map(|f| f.id.to_string()) - .collect(); - let rs: Vec<&str> = ancestors.iter().map(|id| id.as_str()).collect(); - if !ps.has_permission( + let ancestors = fs::get_ancestors(parent); + if !ps.has_permission_any( &Permission { resource: Resource::Folder, operation: Operation::Write, constraint: Some(Resource::Folder), }, - rs.as_slice(), + &ancestors, ) { return false; } @@ -238,20 +212,16 @@ pub fn check_folder_delete(ps: &Policies, bucket: &Principal, parent: u32) -> bo operation: Operation::Delete, constraint: Some(Resource::Folder), }, - bucket.to_string().as_str(), + bucket.to_string(), ) { - let ancestors: Vec = fs::get_ancestors(parent) - .into_iter() - .map(|f| f.id.to_string()) - .collect(); - let rs: Vec<&str> = ancestors.iter().map(|id| id.as_str()).collect(); - if !ps.has_permission( + let ancestors = fs::get_ancestors(parent); + if !ps.has_permission_any( &Permission { resource: Resource::Folder, operation: Operation::Delete, constraint: Some(Resource::Folder), }, - rs.as_slice(), + &ancestors, ) { return false; } @@ -266,7 +236,7 @@ pub fn check_folder_update(ps: &Policies, bucket: &Principal, id: u32, parent: u operation: Operation::Write, constraint: None, }, - id.to_string().as_str(), + id.to_string(), ) { return check_folder_create(ps, bucket, parent); } diff --git a/src/ic_oss_bucket/src/store.rs b/src/ic_oss_bucket/src/store.rs index f20673c..70f60dd 100644 --- a/src/ic_oss_bucket/src/store.rs +++ b/src/ic_oss_bucket/src/store.rs @@ -7,7 +7,7 @@ use ic_http_certification::{ use ic_oss_types::{ cwt::{Token, BUCKET_TOKEN_AAD}, file::{ - FileChunk, FileInfo, UpdateFileInput, MAX_CHUNK_SIZE, MAX_FILE_SIZE, MAX_FILE_SIZE_PER_CALL, + FileChunk, FileInfo, UpdateFileInput, CHUNK_SIZE, MAX_FILE_SIZE, MAX_FILE_SIZE_PER_CALL, }, folder::{FolderInfo, FolderName, UpdateFolderInput}, permission::Policies, @@ -215,7 +215,7 @@ pub struct Chunk(pub Vec); impl Storable for Chunk { const BOUND: Bound = Bound::Bounded { - max_size: MAX_CHUNK_SIZE, + max_size: CHUNK_SIZE, is_fixed_size: false, }; @@ -353,6 +353,23 @@ impl FoldersTree { res } + fn ancestors_map(&self, mut parent: u32, f: F) -> Vec + where + F: Fn(u32, &FolderMetadata) -> U, + { + let mut res = Vec::with_capacity(10); + while parent != 0 { + match self.get(&parent) { + None => break, + Some(folder) => { + res.push(f(parent, folder)); + parent = folder.parent; + } + } + } + res + } + fn list_folders(&self, parent: u32) -> Vec { match self.0.get(&parent) { None => Vec::new(), @@ -760,10 +777,10 @@ pub mod fs { FS_METADATA.with(|r| r.borrow().get(&id)) } - pub fn get_ancestors(start: u32) -> Vec { + pub fn get_ancestors(start: u32) -> Vec { FOLDERS.with(|r| { let m = r.borrow(); - m.ancestors(start) + m.ancestors_map(start, |id, _| id.to_string()) }) } @@ -1084,10 +1101,10 @@ pub mod fs { Err("empty chunk".to_string())?; } - if chunk.len() > MAX_CHUNK_SIZE as usize { + if chunk.len() > CHUNK_SIZE as usize { Err(format!( "chunk size too large, max size is {} bytes", - MAX_CHUNK_SIZE + CHUNK_SIZE ))?; } diff --git a/src/ic_oss_types/src/file.rs b/src/ic_oss_types/src/file.rs index 7bd9ba7..4bed5d7 100644 --- a/src/ic_oss_types/src/file.rs +++ b/src/ic_oss_types/src/file.rs @@ -7,7 +7,7 @@ use url::Url; use crate::{ByteN, MapValue}; -pub const MAX_CHUNK_SIZE: u32 = 256 * 1024; +pub const CHUNK_SIZE: u32 = 256 * 1024; pub const MAX_FILE_SIZE: u64 = 384 * 1024 * 1024 * 1024; // 384G pub const MAX_FILE_SIZE_PER_CALL: u64 = 1024 * 2000; // should less than 2M diff --git a/src/ic_oss_types/src/permission.rs b/src/ic_oss_types/src/permission.rs index 3f65acc..ca532d9 100644 --- a/src/ic_oss_types/src/permission.rs +++ b/src/ic_oss_types/src/permission.rs @@ -218,8 +218,11 @@ impl Resources { self.0.is_empty() || self.0.contains("*") } - fn check(&self, value: &str) -> bool { - self.is_all() || self.0.contains(value) + fn check(&self, value: T) -> bool + where + T: AsRef, + { + self.is_all() || self.0.contains(value.as_ref()) } } @@ -278,7 +281,11 @@ impl TryFrom<&str> for Resources { } pub trait PermissionChecker { - fn has_permission(&self, permission: &Permission, resources: T) -> bool; + fn has_permission(&self, permission: &Permission, resource_path: T) -> bool; +} + +pub trait PermissionCheckerAny { + fn has_permission_any(&self, permission: &Permission, resources_path: &[T]) -> bool; } /// Policy string format: Permission:Resource1,Resource2,... @@ -290,22 +297,22 @@ pub struct Policy { pub resources: Resources, } -impl PermissionChecker<&str> for Policy { - fn has_permission(&self, permission: &Permission, resource_path: &str) -> bool { - self.permission.check(permission) && self.resources.check(resource_path) - } -} - -impl PermissionChecker<[&str; N]> for Policy { - fn has_permission(&self, permission: &Permission, resources_any: [&str; N]) -> bool { - self.permission.check(permission) && resources_any.iter().any(|r| self.resources.check(r)) +impl PermissionChecker for Policy +where + T: AsRef, +{ + fn has_permission(&self, permission: &Permission, resource_path: T) -> bool { + self.permission.check(permission) && self.resources.check(resource_path.as_ref()) } } -impl PermissionChecker<&[&str]> for Policy { - fn has_permission(&self, permission: &Permission, resources_any: &[&str]) -> bool { +impl PermissionCheckerAny for Policy +where + T: AsRef, +{ + fn has_permission_any(&self, permission: &Permission, resources_path: &[T]) -> bool { self.permission.check(permission) - && (self.resources.is_all() || resources_any.iter().any(|r| self.resources.check(r))) + && (self.resources.is_all() || resources_path.iter().any(|r| self.resources.check(r))) } } @@ -376,27 +383,25 @@ impl AsRef> for Policies { } } -impl PermissionChecker<&str> for Policies { - fn has_permission(&self, permission: &Permission, resource_path: &str) -> bool { +impl PermissionChecker for Policies +where + T: AsRef, +{ + fn has_permission(&self, permission: &Permission, resource_path: T) -> bool { self.0 .iter() - .any(|p| p.has_permission(permission, resource_path)) + .any(|p| p.has_permission(permission, resource_path.as_ref())) } } -impl PermissionChecker<[&str; N]> for Policies { - fn has_permission(&self, permission: &Permission, resources_any: [&str; N]) -> bool { +impl PermissionCheckerAny for Policies +where + T: AsRef, +{ + fn has_permission_any(&self, permission: &Permission, resources_any: &[T]) -> bool { self.0 .iter() - .any(|p| p.has_permission(permission, resources_any)) - } -} - -impl PermissionChecker<&[&str]> for Policies { - fn has_permission(&self, permission: &Permission, resources_any: &[&str]) -> bool { - self.0 - .iter() - .any(|p| p.has_permission(permission, resources_any)) + .any(|p| p.has_permission_any(permission, resources_any)) } } @@ -844,13 +849,21 @@ mod tests { )); // Folder.*:2,3,5 - assert!(ps.has_permission( + assert!(ps.has_permission_any( + &Permission { + resource: Resource::Folder, + operation: Operation::Delete, + constraint: Some(Resource::File), + }, + &["4", "5"] + )); + assert!(ps.has_permission_any( &Permission { resource: Resource::Folder, operation: Operation::Delete, constraint: Some(Resource::File), }, - ["4", "5"] + &[4.to_string(), 5.to_string()] )); // Folder.Read