cjw/Oxicloud
1
0
Fork 0
Code Issues Pull Requests Actions 4 Packages Projects Releases Wiki Activity

feat(authz): check permission on read handlers + check create permission on folder

Browse Source
This commit is contained in:
Edouard Vanbelle
2026-05-21 11:07:04 +02:00
parent cba9be8c21
commit 3362e277ab
21 changed files with 428 additions and 180 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/interfaces/api/handlers/chunked_upload_handler.rs
+3
View File
@@ -90,6 +90,8 @@ impl ChunkedUploadHandler {
/// "expires_at": 86400
/// }
/// ```
/// TODO: how is implemented security (owneship, permission ?)
/// current caveat: upload can start without know is path permits upload
pub(super) async fn create_upload_impl(
State(state): State<Arc<AppState>>,
auth_user: AuthUser,
@@ -264,6 +266,7 @@ impl ChunkedUploadHandler {
/// POST /api/uploads/:upload_id/complete - Finalize upload
///
/// Assembles all chunks into the final file and creates the file record
// TODO: how is implemented security (owneship, permission ?)
pub(super) async fn complete_upload_impl(
State(state): State<Arc<AppState>>,
auth_user: AuthUser,
src/interfaces/api/handlers/file_handler.rs
+59 -33
View File
@@ -11,17 +11,17 @@ use serde::Deserialize;
use std::collections::HashMap;
use utoipa::ToSchema;
use crate::application::dtos::file_dto::FileDto;
use crate::application::ports::file_ports::OptimizedFileContent;
use crate::application::ports::file_ports::{
FileManagementUseCase, FileRetrievalUseCase, FileUploadUseCase,
};
use crate::application::ports::storage_ports::{FileReadPort, StorageUsagePort};
use crate::application::ports::thumbnail_ports::ThumbnailPort;
use crate::application::ports::{file_ports::OptimizedFileContent, folder_ports::FolderUseCase};
use crate::common::di::AppState;
use crate::infrastructure::services::audio_metadata_service::AudioMetadataService;
use crate::interfaces::errors::AppError;
use crate::interfaces::middleware::auth::AuthUser;
use crate::{application::dtos::file_dto::FileDto, domain::services::authorization::Permission};
use std::sync::Arc;
/**
@@ -85,6 +85,7 @@ impl FileHandler {
tracing::debug!("📤 Processing streaming file upload (hash-on-write)");
// caveat: if folder_id field is given after check can fails
while let Some(field) = multipart.next_field().await.unwrap_or(None) {
let name = field.name().unwrap_or("").to_string();
@@ -115,24 +116,24 @@ impl FileHandler {
.unwrap_or("application/octet-stream")
.to_string();
// ── SECURITY: Verify folder ownership before upload (IDOR V-03 fix) ──
if let Some(ref fid) = folder_id {
use crate::application::ports::folder_ports::FolderUseCase;
let folder_service = &state.applications.folder_service;
if folder_service
.get_folder_with_perms(fid, auth_user.id)
// ── Fail-fast pre-check: verify the caller can Create inside
// the target folder BEFORE spooling the multipart body to disk.
// The upload service re-checks at write time — this is a
// UX/resource optimization, not the security boundary.
if let Some(ref fid) = folder_id
&& let Err(err) = state
.applications
.folder_service_concrete
.has_permission(auth_user.id, Permission::Create, fid)
.await
.is_err()
{
tracing::warn!(
"⛔ UPLOAD REJECTED (IDOR): user='{}' attempted upload to folder '{}' owned by another user",
auth_user.username,
fid,
);
return Err(Self::domain_error_response(
crate::common::errors::DomainError::not_found("Folder", fid),
));
}
{
tracing::warn!(
"⛔ UPLOAD REJECTED: user='{}' folder='{}' err='{}'",
auth_user.username,
fid,
err
);
return Err(Self::domain_error_response(err));
}
// ── Early quota check (before spooling to disk) ──────
@@ -328,6 +329,16 @@ impl FileHandler {
) -> impl IntoResponse {
use crate::application::ports::thumbnail_ports::ThumbnailSize;
// check first that user can access this resource
if let Err(err) = state
.applications
.file_management_service
.has_permission(auth_user.id, Permission::Read, &id)
.await
{
return AppError::from(err).into_response();
}
let thumbnail_service = &state.core.thumbnail_service;
let thumb_size = match size.as_str() {
@@ -385,7 +396,7 @@ impl FileHandler {
let file_retrieval_service = &state.applications.file_retrieval_service;
let file = match file_retrieval_service
.get_file_owned(&id, auth_user.id)
.get_file_with_perms(&id, auth_user.id)
.await
{
Ok(f) => f,
@@ -478,6 +489,16 @@ impl FileHandler {
) -> impl IntoResponse {
use crate::application::ports::thumbnail_ports::ThumbnailSize;
// check first that user can access this resource
if let Err(err) = state
.applications
.file_management_service
.has_permission(auth_user.id, Permission::Update, &id)
.await
{
return AppError::from(err).into_response();
}
let thumbnail_service = &state.core.thumbnail_service;
// Validate size
@@ -508,7 +529,7 @@ impl FileHandler {
// Validate file ownership
let file_retrieval_service = &state.applications.file_retrieval_service;
if let Err(err) = file_retrieval_service
.get_file_owned(&id, auth_user.id)
.get_file_with_perms(&id, auth_user.id)
.await
{
return AppError::from(err).into_response();
@@ -545,7 +566,7 @@ impl FileHandler {
let retrieval = &state.applications.file_retrieval_service;
// ── Get file metadata (ownership-scoped) ────────────────────────
let file_dto = match retrieval.get_file_owned(&id, auth_user.id).await {
let file_dto = match retrieval.get_file_with_perms(&id, auth_user.id).await {
Ok(f) => f,
Err(err) => {
return AppError::from(err).into_response();
@@ -603,7 +624,7 @@ impl FileHandler {
Self::content_disposition(&file_dto.name, &file_dto.mime_type, &params);
match retrieval
.get_file_range_stream_owned(&id, auth_user.id, start, Some(end + 1))
.get_file_range_stream_with_perms(&id, auth_user.id, start, Some(end + 1))
.await
{
Ok(stream) => {
@@ -713,7 +734,10 @@ impl FileHandler {
tracing::info!("API: Listing files with folder_id: {:?}", folder_id);
let retrieval = &state.applications.file_retrieval_service;
match retrieval.list_files_owned(folder_id, auth_user.id).await {
match retrieval
.list_files_with_perms(folder_id, auth_user.id)
.await
{
Ok(files) => {
// Compute lightweight ETag from max modified_at + count
let max_mod = files.iter().map(|f| f.modified_at).max().unwrap_or(0);
@@ -751,6 +775,7 @@ impl FileHandler {
/// Delegates to [`Self::upload_file_inner`] and, on success, spawns
/// a background task to generate all thumbnail sizes before serialising
/// the `FileDto` once.
/// TODO: should move thumbnail generation to a generic hook ? (onfileUploaded, other services will beneficiate it)
pub(super) async fn upload_file_with_thumbnails_impl(
State(state): State<GlobalState>,
auth_user: AuthUser,
@@ -797,6 +822,7 @@ impl FileHandler {
});
}
// TODO: same remark: a hook to handle easily audio service
// Extract audio metadata for supported audio files in background.
if let Some(ref audio_service) = state.applications.audio_metadata_service
&& AudioMetadataService::is_audio_file(&file.mime_type)
@@ -825,15 +851,14 @@ impl FileHandler {
auth_user: AuthUser,
Path(file_id): Path<String>,
) -> impl IntoResponse {
// Verify ownership
let file_read = &state.repositories.file_read_repository;
if let Err(e) = file_read.verify_file_owner(&file_id, auth_user.id).await {
let msg = e.to_string();
return (
StatusCode::NOT_FOUND,
Json(serde_json::json!({ "error": msg })),
)
.into_response();
// check first that user can access this resource
if let Err(err) = state
.applications
.file_management_service
.has_permission(auth_user.id, Permission::Read, &file_id)
.await
{
return AppError::from(err).into_response();
}
let metadata_repo = &state.repositories.file_metadata_repository;
@@ -927,6 +952,7 @@ impl FileHandler {
}
/// Moves a file to a different folder (ownership-verified)
/// TODO: dead function ?
pub async fn move_file(
State(state): State<GlobalState>,
auth_user: AuthUser,
src/interfaces/api/handlers/folder_handler.rs
+12 -37
View File
@@ -51,7 +51,7 @@ impl FolderHandler {
"create_folder: parent_id is None for user '{}', resolving home folder",
auth_user.username
);
match service.list_folders_for_owner(None, auth_user.id).await {
match service.list_folders_with_perms(None, auth_user.id).await {
Ok(folders) => {
if let Some(home) = folders.first() {
tracing::info!(
@@ -89,22 +89,8 @@ impl FolderHandler {
auth_user: AuthUser,
Path(id): Path<String>,
) -> impl IntoResponse {
match service.get_folder(&id).await {
Ok(folder) => {
// Access check: folder must belong to the requesting user
if let Some(ref owner) = folder.owner_id
&& owner != &auth_user.id.to_string()
{
tracing::warn!(
"get_folder: user '{}' attempted to access folder '{}' owned by '{}'",
auth_user.id,
id,
owner
);
return AppError::not_found("Folder not found").into_response();
}
(StatusCode::OK, Json(folder)).into_response()
}
match service.get_folder_with_perms(&id, auth_user.id).await {
Ok(folder) => (StatusCode::OK, Json(folder)).into_response(),
Err(err) => AppError::from(err).into_response(),
}
}
@@ -146,7 +132,7 @@ impl FolderHandler {
pagination: Query<PaginationRequestDto>,
) -> axum::response::Response {
match service
.list_folders_for_owner_paginated(Some(&id), auth_user.id, &pagination)
.list_folders_paginated_with_perms(Some(&id), auth_user.id, &pagination)
.await
{
Ok(paginated_result) => (StatusCode::OK, Json(paginated_result)).into_response(),
@@ -163,7 +149,7 @@ impl FolderHandler {
auth_user: &AuthUser,
) -> axum::response::Response {
match service
.list_folders_for_owner(parent_id, auth_user.id)
.list_folders_with_perms(parent_id, auth_user.id)
.await
{
Ok(folders) => (StatusCode::OK, Json(folders)).into_response(),
@@ -206,8 +192,8 @@ impl FolderHandler {
// Run both queries concurrently — no sequential wait.
let (folders_result, files_result) = tokio::join!(
folder_service.list_folders_for_owner(Some(&id), auth_user.id),
file_service.list_files_owned(Some(&id), auth_user.id)
folder_service.list_folders_with_perms(Some(&id), auth_user.id),
file_service.list_files_with_perms(Some(&id), auth_user.id)
);
match (folders_result, files_result) {
@@ -226,7 +212,6 @@ impl FolderHandler {
.unwrap()
.into_response();
}
let listing = FolderListingDto { folders, files };
let mut resp = (StatusCode::OK, Json(listing)).into_response();
resp.headers_mut()
@@ -286,6 +271,7 @@ impl FolderHandler {
) -> impl IntoResponse {
let user_id = auth_user.id;
// Check if trash service is available
// FIXME: permissions !!
if let Some(trash_service) = &state.trash_service {
tracing::info!("Moving folder to trash: {}", id);
@@ -328,22 +314,11 @@ impl FolderHandler {
// Get folder information and verify ownership
let folder_service = &state.applications.folder_service;
match folder_service.get_folder(&id).await {
match folder_service
.get_folder_with_perms(&id, auth_user.id)
.await
{
Ok(folder) => {
// Access check: folder must belong to the requesting user
if folder.owner_id.as_deref() != Some(&auth_user.id.to_string()) {
tracing::warn!(
"download_folder_zip: user '{}' attempted to download folder '{}' owned by '{:?}'",
auth_user.id,
id,
folder.owner_id
);
return (
StatusCode::NOT_FOUND,
Json(serde_json::json!({ "error": "Folder not found" })),
)
.into_response();
}
tracing::info!("Preparing ZIP for folder: {} ({})", folder.name, id);
// Use ZIP service from DI container
src/interfaces/api/handlers/share_handler.rs
+1
View File
@@ -561,6 +561,7 @@ fn share_browse_error_response(err: crate::common::errors::DomainError) -> Respo
AppError::from(err).into_response()
}
// TODO: remove this and use the classic /api/files & /api/folders get, but with the token as session ?
#[utoipa::path(
get,
path = "/api/s/{token}/contents",
src/interfaces/api/handlers/webdav_handler.rs
+3 -3
View File
@@ -189,7 +189,7 @@ async fn handle_webdav_methods(
async fn resolve_webdav_path(state: &Arc<AppState>, user_id: Uuid, path: &str) -> Option<String> {
let folder_service = &state.applications.folder_service;
let home_folders = folder_service
.list_folders_for_owner(None, user_id)
.list_folders_with_perms(None, user_id)
.await
.ok()?;
let home = home_folders.first()?;
@@ -514,7 +514,7 @@ async fn build_streaming_propfind_response(
page_size: pagination.page_size,
};
let result = folder_service
.list_folders_for_owner_paginated(fid_ref, user_id, &pag)
.list_folders_paginated_with_perms(fid_ref, user_id, &pag)
.await
.map_err(|e| std::io::Error::other(e.to_string()))?;
@@ -544,7 +544,7 @@ async fn build_streaming_propfind_response(
let mut offset: i64 = 0;
loop {
let batch: Vec<FileDto> = file_retrieval_service
.list_files_batch_for_owner(fid_ref, user_id, offset, PROPFIND_BATCH_SIZE)
.list_files_batch_with_perms(fid_ref, user_id, offset, PROPFIND_BATCH_SIZE)
.await
.map_err(|e| std::io::Error::other(e.to_string()))?;
src/interfaces/api/handlers/wopi_handler.rs
+1 -1
View File
@@ -401,7 +401,7 @@ async fn authorize_wopi_access<S: FileRetrievalUseCase>(
requested_action: &str,
) -> Result<(crate::application::dtos::file_dto::FileDto, bool), StatusCode> {
let file = file_retrieval
.get_file_owned(file_id, caller_id)
.get_file_with_perms(file_id, caller_id)
.await
.map_err(|_| StatusCode::NOT_FOUND)?;
// Owner verified — grant write unless explicitly requesting view-only.