From fb65d1976be383cf776fd320e9c9c30617394438 Mon Sep 17 00:00:00 2001 From: Dionisio Date: Fri, 13 Feb 2026 08:54:51 +0100 Subject: [PATCH] fix: resolve file management operations not working (#83) - Fix rename: context menu was nullifying target reference before rename dialog could use it - Fix delete files/folders: auth extractors were mandatory, causing 401 when auth not configured - Fix view-file: async fetch race condition with context menu cleanup - Fix orphaned ID mappings on file deletion - Fix Authorization: Bearer null headers sent without token - Add OptionalUserId and OptionalAuthUser infallible extractors --- doc/ADMIN-SETTINGS.md | 162 ++++++++++++++++++ .../repositories/file_fs_write_repository.rs | 7 + src/interfaces/api/handlers/file_handler.rs | 25 ++- src/interfaces/api/handlers/folder_handler.rs | 7 +- src/interfaces/api/handlers/trash_handler.rs | 23 +-- src/interfaces/middleware/auth.rs | 40 +++++ static/js/app.js | 19 +- static/js/contextMenus.js | 48 +++--- static/js/favorites.js | 26 +-- 9 files changed, 294 insertions(+), 63 deletions(-) create mode 100644 doc/ADMIN-SETTINGS.md diff --git a/doc/ADMIN-SETTINGS.md b/doc/ADMIN-SETTINGS.md new file mode 100644 index 00000000..69a35387 --- /dev/null +++ b/doc/ADMIN-SETTINGS.md @@ -0,0 +1,162 @@ +# OxiCloud — Admin Settings + +## Overview + +OxiCloud provides an admin panel API for managing server settings, OIDC configuration, user management, and dashboard statistics. All admin endpoints require a valid JWT token with `role = "admin"`. + +## Architecture + +| Layer | Component | File | +|---|---|---| +| Domain Port | `SettingsRepository` trait | `src/domain/repositories/settings_repository.rs` | +| Application Service | `AdminSettingsService` | `src/application/services/admin_settings_service.rs` | +| Application DTOs | Settings and user management DTOs | `src/application/dtos/settings_dto.rs` | +| Infrastructure | `SettingsPgRepository` | `src/infrastructure/repositories/pg/settings_pg_repository.rs` | +| Interfaces | `admin_handler` functions | `src/interfaces/api/handlers/admin_handler.rs` | + +## REST API + +All routes under `/api/admin`, require admin JWT. + +### Settings + +| Method | Path | Handler | Description | +|---|---|---|---| +| `GET` | `/api/admin/settings/oidc` | `get_oidc_settings` | Get current OIDC configuration | +| `PUT` | `/api/admin/settings/oidc` | `save_oidc_settings` | Update OIDC configuration | +| `POST` | `/api/admin/settings/oidc/test` | `test_oidc_connection` | Test OIDC provider connectivity | +| `GET` | `/api/admin/settings/general` | `get_general_settings` | Get general server settings | + +### Dashboard + +| Method | Path | Handler | Description | +|---|---|---|---| +| `GET` | `/api/admin/dashboard` | `get_dashboard_stats` | Server dashboard statistics | + +### User Management + +| Method | Path | Handler | Description | +|---|---|---|---| +| `GET` | `/api/admin/users` | `list_users` | List all users (paginated) | +| `GET` | `/api/admin/users/{id}` | `get_user` | Get user details | +| `DELETE` | `/api/admin/users/{id}` | `delete_user` | Delete a user | +| `PUT` | `/api/admin/users/{id}/role` | `update_user_role` | Change user role | +| `PUT` | `/api/admin/users/{id}/active` | `update_user_active` | Activate/deactivate user | +| `PUT` | `/api/admin/users/{id}/quota` | `update_user_quota` | Set storage quota | + +### Safety Guards + +- **Self-deletion blocked**: Admins cannot delete their own account +- **Self-role-change blocked**: Admins cannot change their own role +- **Self-deactivation blocked**: Admins cannot deactivate themselves + +## OIDC Settings Management + +### Get Settings Response + +```json +{ + "enabled": true, + "issuer_url": "https://keycloak.example.com/realms/main", + "client_id": "oxicloud", + "client_secret_set": true, + "scopes": "openid profile email", + "auto_provision": true, + "admin_groups": "oxicloud-admins", + "disable_password_login": false, + "provider_name": "KeyCloak", + "callback_url": "https://oxicloud.example.com/api/auth/oidc/callback", + "env_overrides": ["issuer_url", "client_id"] +} +``` + +The `env_overrides` field lists which settings are overridden by environment variables (env vars take priority over DB settings). + +### Save Settings Request + +```json +{ + "enabled": true, + "issuer_url": "https://keycloak.example.com/realms/main", + "client_id": "oxicloud", + "client_secret": "new-secret", + "scopes": "openid profile email", + "auto_provision": true, + "admin_groups": "oxicloud-admins", + "disable_password_login": false, + "provider_name": "KeyCloak" +} +``` + +After saving, the service hot-reloads OIDC via `auth_app_service.reload_oidc()` or `disable_oidc()`. + +### Test OIDC Connection + +```json +// Request +{ "issuer_url": "https://keycloak.example.com/realms/main" } + +// Response +{ + "success": true, + "message": "Successfully connected to OIDC provider", + "issuer": "https://keycloak.example.com/realms/main", + "authorization_endpoint": "https://keycloak.example.com/realms/main/protocol/openid-connect/auth", + "token_endpoint": "https://keycloak.example.com/realms/main/protocol/openid-connect/token", + "userinfo_endpoint": "https://keycloak.example.com/realms/main/protocol/openid-connect/userinfo", + "provider_name_suggestion": "KeyCloak" +} +``` + +## Dashboard Statistics + +```json +{ + "server_version": "0.3.2", + "auth_enabled": true, + "oidc_configured": true, + "quotas_enabled": false, + "total_users": 42, + "active_users": 38, + "admin_users": 2, + "total_quota_bytes": 107374182400, + "total_used_bytes": 53687091200, + "storage_usage_percent": 50.0, + "users_over_80_percent": 5, + "users_over_quota": 1 +} +``` + +## User Management DTOs + +```rust +pub struct UpdateUserRoleDto { pub role: String } // "user" | "admin" +pub struct UpdateUserActiveDto { pub active: bool } +pub struct UpdateUserQuotaDto { pub quota_bytes: i64 } +pub struct ListUsersQueryDto { pub limit: Option, pub offset: Option } +``` + +## Config Priority + +Settings are resolved with the following priority (highest first): + +1. **Environment variables** (`OXICLOUD_OIDC_*`) +2. **Database settings** (`auth.admin_settings` table) +3. **Defaults** + +## Database Schema + +```sql +CREATE TABLE IF NOT EXISTS auth.admin_settings ( + key TEXT PRIMARY KEY, + value TEXT NOT NULL, + category TEXT NOT NULL, + is_secret BOOLEAN DEFAULT FALSE, + updated_by VARCHAR(36), + updated_at TIMESTAMP WITH TIME ZONE DEFAULT CURRENT_TIMESTAMP +); +``` + +## Frontend + +The admin panel is served from `static/admin.html`. diff --git a/src/infrastructure/repositories/file_fs_write_repository.rs b/src/infrastructure/repositories/file_fs_write_repository.rs index 15aadd8f..45f09bfb 100644 --- a/src/infrastructure/repositories/file_fs_write_repository.rs +++ b/src/infrastructure/repositories/file_fs_write_repository.rs @@ -431,6 +431,13 @@ impl FileWritePort for FileFsWriteRepository { } self.delete_file_non_blocking(abs_path).await.map_err(map_repo_err)?; + + // Clean up the ID mapping so we don't leave orphaned entries + if let Err(e) = self.id_mapping_service.remove_id(id).await { + tracing::warn!("Failed to remove ID mapping for deleted file {}: {}", id, e); + } + let _ = self.id_mapping_service.save_changes().await; + Ok(()) } diff --git a/src/interfaces/api/handlers/file_handler.rs b/src/interfaces/api/handlers/file_handler.rs index bf254735..98d5d87c 100644 --- a/src/interfaces/api/handlers/file_handler.rs +++ b/src/interfaces/api/handlers/file_handler.rs @@ -13,7 +13,7 @@ use http_range_header::parse_range_header; use crate::application::ports::compression_ports::{CompressionPort, CompressionLevel}; use crate::application::ports::file_ports::OptimizedFileContent; use crate::common::di::AppState; -use crate::interfaces::middleware::auth::CurrentUserId; +use crate::interfaces::middleware::auth::OptionalUserId; /** * Type aliases for dependency injection state. @@ -506,22 +506,35 @@ impl FileHandler { /// /// All logic (trash fallback, dedup ref-count, hash computation) is handled /// by `FileManagementUseCase::delete_with_cleanup`. + /// + /// When auth is available, uses trash-first deletion; otherwise falls back + /// to permanent delete so the endpoint works with or without auth. pub async fn delete_file( State(state): State, - CurrentUserId(user_id): CurrentUserId, + OptionalUserId(user_id): OptionalUserId, Path(id): Path, ) -> impl IntoResponse { let mgmt = &state.applications.file_management_service; - match mgmt.delete_with_cleanup(&id, &user_id).await { - Ok(was_trashed) => { + let result = if let Some(uid) = user_id { + // Auth available: trash-first with dedup cleanup + mgmt.delete_with_cleanup(&id, &uid).await.map(|was_trashed| { if was_trashed { tracing::info!("File moved to trash: {}", id); } else { tracing::info!("File permanently deleted: {}", id); } - StatusCode::NO_CONTENT.into_response() - } + }) + } else { + // No auth: permanent delete + tracing::warn!("No auth context – permanently deleting file: {}", id); + mgmt.delete_file(&id).await.map(|_| { + tracing::info!("File permanently deleted (no auth): {}", id); + }) + }; + + match result { + Ok(_) => StatusCode::NO_CONTENT.into_response(), Err(err) => { tracing::error!("Error deleting file: {}", err); let status = if err.to_string().contains("not found") || err.to_string().contains("NotFound") { diff --git a/src/interfaces/api/handlers/folder_handler.rs b/src/interfaces/api/handlers/folder_handler.rs index 690fd9ec..79e4e840 100644 --- a/src/interfaces/api/handlers/folder_handler.rs +++ b/src/interfaces/api/handlers/folder_handler.rs @@ -13,7 +13,7 @@ use crate::application::dtos::pagination::PaginationRequestDto; use crate::common::errors::ErrorKind; use crate::application::ports::inbound::FolderUseCase; use crate::common::di::AppState as GlobalAppState; -use crate::interfaces::middleware::auth::AuthUser; +use crate::interfaces::middleware::auth::OptionalAuthUser; type AppState = Arc; @@ -205,15 +205,16 @@ impl FolderHandler { /// Deletes a folder with trash functionality pub async fn delete_folder_with_trash( State(state): State, - auth_user: AuthUser, + OptionalAuthUser(auth_user): OptionalAuthUser, Path(id): Path, ) -> impl IntoResponse { + let user_id = auth_user.as_ref().map(|u| u.id.as_str()).unwrap_or("anonymous"); // Check if trash service is available if let Some(trash_service) = &state.trash_service { tracing::info!("Moving folder to trash: {}", id); // Try to move to trash first - match trash_service.move_to_trash(&id, "folder", &auth_user.id).await { + match trash_service.move_to_trash(&id, "folder", user_id).await { Ok(_) => { tracing::info!("Folder successfully moved to trash: {}", id); return StatusCode::NO_CONTENT.into_response(); diff --git a/src/interfaces/api/handlers/trash_handler.rs b/src/interfaces/api/handlers/trash_handler.rs index 4716fc90..7fdf0b30 100644 --- a/src/interfaces/api/handlers/trash_handler.rs +++ b/src/interfaces/api/handlers/trash_handler.rs @@ -6,7 +6,7 @@ use tracing::{debug, error, warn, instrument}; // use crate::application::ports::trash_ports::TrashUseCase; use crate::common::di::AppState; -use crate::interfaces::middleware::auth::AuthUser; +use crate::interfaces::middleware::auth::{AuthUser, OptionalAuthUser}; /// Gets all items in the trash for the current user #[instrument(skip_all)] @@ -50,11 +50,12 @@ pub async fn get_trash_items( #[instrument(skip_all)] pub async fn move_to_trash( State(state): State, - auth_user: AuthUser, + OptionalAuthUser(auth_user): OptionalAuthUser, Path((item_type, item_id)): Path<(String, String)>, ) -> (StatusCode, Json) { + let user_id = auth_user.as_ref().map(|u| u.id.as_str()).unwrap_or("anonymous"); debug!("Request to move to trash: type={}, id={}, user={}", - item_type, item_id, auth_user.id); + item_type, item_id, user_id); let trash_service = match state.trash_service.as_ref() { Some(service) => service, @@ -64,7 +65,7 @@ pub async fn move_to_trash( }))); } }; - let result = trash_service.move_to_trash(&item_id, &item_type, &auth_user.id).await; + let result = trash_service.move_to_trash(&item_id, &item_type, user_id).await; match result { Ok(_) => { @@ -87,11 +88,12 @@ pub async fn move_to_trash( #[instrument(skip_all)] pub async fn move_file_to_trash( State(state): State, - auth_user: AuthUser, + OptionalAuthUser(auth_user): OptionalAuthUser, Path(item_id): Path, ) -> (StatusCode, Json) { + let user_id = auth_user.as_ref().map(|u| u.id.as_str()).unwrap_or("anonymous"); debug!("Request to move file to trash: id={}, user={}", - item_id, auth_user.id); + item_id, user_id); let trash_service = match state.trash_service.as_ref() { Some(service) => service, @@ -103,7 +105,7 @@ pub async fn move_file_to_trash( }; // Specify that it is a file - let result = trash_service.move_to_trash(&item_id, "file", &auth_user.id).await; + let result = trash_service.move_to_trash(&item_id, "file", user_id).await; match result { Ok(_) => { @@ -126,11 +128,12 @@ pub async fn move_file_to_trash( #[instrument(skip_all)] pub async fn move_folder_to_trash( State(state): State, - auth_user: AuthUser, + OptionalAuthUser(auth_user): OptionalAuthUser, Path(item_id): Path, ) -> (StatusCode, Json) { + let user_id = auth_user.as_ref().map(|u| u.id.as_str()).unwrap_or("anonymous"); debug!("Request to move folder to trash: id={}, user={}", - item_id, auth_user.id); + item_id, user_id); let trash_service = match state.trash_service.as_ref() { Some(service) => service, @@ -142,7 +145,7 @@ pub async fn move_folder_to_trash( }; // Specify that it is a folder - let result = trash_service.move_to_trash(&item_id, "folder", &auth_user.id).await; + let result = trash_service.move_to_trash(&item_id, "folder", user_id).await; match result { Ok(_) => { diff --git a/src/interfaces/middleware/auth.rs b/src/interfaces/middleware/auth.rs index 04721b71..0b51bf53 100644 --- a/src/interfaces/middleware/auth.rs +++ b/src/interfaces/middleware/auth.rs @@ -1,4 +1,5 @@ use std::sync::Arc; +use std::convert::Infallible; use axum::{ extract::{State, Request, FromRequestParts}, http::{StatusCode, HeaderMap, header, request::Parts}, @@ -63,6 +64,45 @@ where } } +/// Optional user ID extractor – never fails. +/// Yields `Some(id)` when auth middleware ran, `None` otherwise. +#[derive(Clone, Debug)] +pub struct OptionalUserId(pub Option); + +impl FromRequestParts for OptionalUserId +where + S: Send + Sync, +{ + type Rejection = Infallible; + + async fn from_request_parts(parts: &mut Parts, _state: &S) -> Result { + Ok(OptionalUserId( + parts.extensions.get::().map(|cu| cu.id.clone()), + )) + } +} + +/// Optional auth user extractor – never fails. +/// Yields `Some(AuthUser)` when auth middleware ran, `None` otherwise. +#[derive(Clone, Debug)] +pub struct OptionalAuthUser(pub Option); + +impl FromRequestParts for OptionalAuthUser +where + S: Send + Sync, +{ + type Rejection = Infallible; + + async fn from_request_parts(parts: &mut Parts, _state: &S) -> Result { + Ok(OptionalAuthUser( + parts.extensions.get::().map(|cu| AuthUser { + id: cu.id.clone(), + username: cu.username.clone(), + }), + )) + } +} + // Error for authentication operations #[derive(Debug, thiserror::Error)] pub enum AuthError { diff --git a/static/js/app.js b/static/js/app.js index 949a3758..acfe5b8c 100644 --- a/static/js/app.js +++ b/static/js/app.js @@ -680,12 +680,15 @@ async function loadFiles(options = {}) { } const token = localStorage.getItem('oxicloud_token'); + const headers = { + 'Cache-Control': 'no-cache, no-store, must-revalidate', + 'Pragma': 'no-cache' + }; + if (token) { + headers['Authorization'] = `Bearer ${token}`; + } const requestOptions = { - headers: { - 'Authorization': `Bearer ${token}`, - 'Cache-Control': 'no-cache, no-store, must-revalidate', - 'Pragma': 'no-cache' - }, + headers, cache: 'no-store' // Instruct the browser not to use cache }; @@ -1727,10 +1730,10 @@ async function findUserHomeFolder(username) { const controller = new AbortController(); const timeoutId = setTimeout(() => controller.abort(), 3000); // Reduced timeout to 3 seconds + const folderToken = localStorage.getItem('oxicloud_token'); + const folderHeaders = folderToken ? { 'Authorization': `Bearer ${folderToken}` } : {}; const response = await fetch('/api/folders', { - headers: { - 'Authorization': `Bearer ${localStorage.getItem('oxicloud_token')}` - }, + headers: folderHeaders, signal: controller.signal }); diff --git a/static/js/contextMenus.js b/static/js/contextMenus.js index 08281a4e..bf53cef3 100644 --- a/static/js/contextMenus.js +++ b/static/js/contextMenus.js @@ -81,11 +81,11 @@ const contextMenus = { // File context menu options document.getElementById('view-file-option').addEventListener('click', () => { if (window.app.contextMenuTargetFile) { - // Fetch file details to get the mime type + // Capture reference before context menu cleanup nullifies it + const file = window.app.contextMenuTargetFile; const token = localStorage.getItem('oxicloud_token'); - fetch(`/api/files/${window.app.contextMenuTargetFile.id}?metadata=true`, { - headers: { 'Authorization': `Bearer ${token}` } - }) + const headers = token ? { 'Authorization': `Bearer ${token}` } : {}; + fetch(`/api/files/${file.id}?metadata=true`, { headers }) .then(response => response.json()) .then(fileDetails => { // Check if viewable file type (images, PDFs, text files) @@ -97,26 +97,17 @@ const contextMenus = { window.fileViewer.open(fileDetails); } else { // If no viewer is available, download directly - window.fileOps.downloadFile( - window.app.contextMenuTargetFile.id, - window.app.contextMenuTargetFile.name - ); + window.fileOps.downloadFile(file.id, file.name); } } else { // For non-viewable files, download - window.fileOps.downloadFile( - window.app.contextMenuTargetFile.id, - window.app.contextMenuTargetFile.name - ); + window.fileOps.downloadFile(file.id, file.name); } }) .catch(error => { console.error('Error fetching file details:', error); // On error, fallback to download - window.fileOps.downloadFile( - window.app.contextMenuTargetFile.id, - window.app.contextMenuTargetFile.name - ); + window.fileOps.downloadFile(file.id, file.name); }); } window.ui.closeFileContextMenu(); @@ -242,6 +233,8 @@ const contextMenus = { const renameDialog = document.getElementById('rename-dialog'); window.app.renameMode = 'folder'; + // Store the folder reference so it survives context menu cleanup + window.app.renameTarget = folder; renameInput.value = folder.name; // Update header text const headerSpan = renameDialog.querySelector('.rename-dialog-header span'); @@ -260,6 +253,8 @@ const contextMenus = { const renameDialog = document.getElementById('rename-dialog'); window.app.renameMode = 'file'; + // Store the file reference so it survives context menu cleanup + window.app.renameTarget = file; renameInput.value = file.name; // Update header text const headerSpan = renameDialog.querySelector('.rename-dialog-header span'); @@ -275,6 +270,7 @@ const contextMenus = { closeRenameDialog() { document.getElementById('rename-dialog').style.display = 'none'; window.app.contextMenuTargetFolder = null; + window.app.renameTarget = null; }, /** @@ -322,14 +318,21 @@ const contextMenus = { return; } - if (window.app.renameMode === 'file' && window.app.contextMenuTargetFile) { - const success = await window.fileOps.renameFile(window.app.contextMenuTargetFile.id, newName); + // Use renameTarget which was saved before the context menu was closed + const target = window.app.renameTarget; + if (!target) { + console.error('No rename target available'); + return; + } + + if (window.app.renameMode === 'file') { + const success = await window.fileOps.renameFile(target.id, newName); if (success) { contextMenus.closeRenameDialog(); window.loadFiles(); } - } else if (window.app.contextMenuTargetFolder) { - const success = await window.fileOps.renameFolder(window.app.contextMenuTargetFolder.id, newName); + } else if (window.app.renameMode === 'folder') { + const success = await window.fileOps.renameFolder(target.id, newName); if (success) { contextMenus.closeRenameDialog(); window.loadFiles(); @@ -350,9 +353,8 @@ const contextMenus = { async loadAllFolders(itemId, mode) { try { const token = localStorage.getItem('oxicloud_token'); - const response = await fetch('/api/folders', { - headers: { 'Authorization': `Bearer ${token}` } - }); + const headers = token ? { 'Authorization': `Bearer ${token}` } : {}; + const response = await fetch('/api/folders', { headers }); if (response.ok) { const folders = await response.json(); const folderSelectContainer = document.getElementById('folder-select-container'); diff --git a/static/js/favorites.js b/static/js/favorites.js index 08ea4842..edac2eec 100644 --- a/static/js/favorites.js +++ b/static/js/favorites.js @@ -31,11 +31,11 @@ const favorites = { const controller = new AbortController(); const timeoutId = setTimeout(() => controller.abort(), 3000); // 3s timeout + const favToken = localStorage.getItem('oxicloud_token'); + const favHeaders = favToken ? { 'Authorization': `Bearer ${favToken}` } : {}; const response = await fetch('/api/favorites', { method: 'GET', - headers: { - 'Authorization': `Bearer ${localStorage.getItem('oxicloud_token')}` - }, + headers: favHeaders, signal: controller.signal }).catch(err => { console.warn('Network error checking favorites API:', err); @@ -67,10 +67,10 @@ const favorites = { async syncWithServer() { try { // Get server favorites + const syncToken = localStorage.getItem('oxicloud_token'); + const syncHeaders = syncToken ? { 'Authorization': `Bearer ${syncToken}` } : {}; const response = await fetch('/api/favorites', { - headers: { - 'Authorization': `Bearer ${localStorage.getItem('oxicloud_token')}` - } + headers: syncHeaders }); if (!response.ok) { @@ -149,12 +149,12 @@ const favorites = { */ async addToServerFavorites(id, type) { try { + const addToken = localStorage.getItem('oxicloud_token'); + const addHeaders = { 'Content-Type': 'application/json' }; + if (addToken) addHeaders['Authorization'] = `Bearer ${addToken}`; const response = await fetch(`/api/favorites/${type}/${id}`, { method: 'POST', - headers: { - 'Authorization': `Bearer ${localStorage.getItem('oxicloud_token')}`, - 'Content-Type': 'application/json' - } + headers: addHeaders }); if (!response.ok) { @@ -175,11 +175,11 @@ const favorites = { */ async removeFromServerFavorites(id, type) { try { + const rmToken = localStorage.getItem('oxicloud_token'); + const rmHeaders = rmToken ? { 'Authorization': `Bearer ${rmToken}` } : {}; const response = await fetch(`/api/favorites/${type}/${id}`, { method: 'DELETE', - headers: { - 'Authorization': `Bearer ${localStorage.getItem('oxicloud_token')}` - } + headers: rmHeaders }); if (!response.ok) {