From 39a5ef4fad812acf2371eea31365abbd92dab056 Mon Sep 17 00:00:00 2001 From: Bradley Nelson Date: Mon, 7 Sep 2026 00:28:23 -0600 Subject: [PATCH] fix(mounts): scope external mounts to drives --- docs/external-mounts.md | 23 ++++++ frontend/src/lib/api/endpoints/admin.ts | 3 +- .../src/routes/admin/[[tab]]/+page.svelte | 37 ++++++++-- .../src/routes/admin/[[tab]]/page.test.ts | 20 +++++- ...000000_external_mount_drive_visibility.sql | 12 ++++ src/application/services/folder_service.rs | 72 ++++++++++++++++++- .../api/handlers/admin_external_mounts.rs | 53 ++++++++++++-- 7 files changed, 205 insertions(+), 15 deletions(-) create mode 100644 docs/external-mounts.md create mode 100644 migrations/20261025000000_external_mount_drive_visibility.sql diff --git a/docs/external-mounts.md b/docs/external-mounts.md new file mode 100644 index 00000000..73ab9461 --- /dev/null +++ b/docs/external-mounts.md @@ -0,0 +1,23 @@ +# External mounts + +External mounts expose files from a provider such as a host directory inside an +OxiCloud drive. Enable the feature with `OXICLOUD_ENABLE_EXTERNAL_MOUNTS=true`, +then configure mounts from **Administration > External Mounts**. + +Each mount must be attached to a drive. The mount-root is a normal folder in +that drive, and access to all provider content is inherited from the drive's +membership and roles: + +- a mount in a personal drive is visible only to that drive's owner; +- a mount in a shared drive is visible to members who can read the drive; +- mutations additionally require the corresponding drive permission and are + always rejected when the mount is configured as read-only. + +Removing a mount deletes only its OxiCloud mount-root and configuration. It does +not delete the provider's root directory. Deleting files or folders *inside* a +writable mount is permanent because external mounts do not use OxiCloud trash. + +Mounts created before drive selection was introduced remain attached to their +existing drive, normally the configuring administrator's personal drive. To +make one available through a shared drive, remove it and recreate it with that +shared drive selected; removing the old mount does not remove host content. diff --git a/frontend/src/lib/api/endpoints/admin.ts b/frontend/src/lib/api/endpoints/admin.ts index 9d1eae3d..9171616f 100644 --- a/frontend/src/lib/api/endpoints/admin.ts +++ b/frontend/src/lib/api/endpoints/admin.ts @@ -793,6 +793,7 @@ export interface ExternalMount { export interface CreateExternalMountInput { name: string; host_path: string; + drive_id: string; kind?: string; read_only?: boolean; } @@ -804,7 +805,7 @@ export function listExternalMounts(): Promise { }); } -/** POST /api/admin/external-mounts — create a mount in the admin's drive. */ +/** POST /api/admin/external-mounts — create a mount in the selected drive. */ export async function createExternalMount(input: CreateExternalMountInput): Promise { const res = await apiFetch('/api/admin/external-mounts', { method: 'POST', diff --git a/frontend/src/routes/admin/[[tab]]/+page.svelte b/frontend/src/routes/admin/[[tab]]/+page.svelte index 919a3770..3c620e08 100644 --- a/frontend/src/routes/admin/[[tab]]/+page.svelte +++ b/frontend/src/routes/admin/[[tab]]/+page.svelte @@ -253,26 +253,41 @@ // External mounts let mounts = $state(null); + let mountDrives = $state([]); let mountsError = $state(null); - let newMount = $state({ name: '', host_path: '', read_only: false }); + let newMount = $state({ + name: '', + host_path: '', + drive_id: '', + read_only: false + }); let mountCreating = $state(false); async function loadMounts() { mountsError = null; try { - mounts = await listExternalMounts(); + [mounts, mountDrives] = await Promise.all([listExternalMounts(), listAllDrives()]); } catch (e) { mountsError = errorMessage(e); } } + function mountDriveName(driveId: string): string { + return mountDrives.find((drive) => drive.id === driveId)?.name ?? driveId; + } + async function createMount() { - if (!newMount.name.trim() || !newMount.host_path.trim()) return; + if (!newMount.name.trim() || !newMount.host_path.trim() || !newMount.drive_id) return; mountCreating = true; try { const created = await createExternalMount(newMount); mounts = [...(mounts ?? []), created]; - newMount = { name: '', host_path: '', read_only: false }; + newMount = { + name: '', + host_path: '', + drive_id: newMount.drive_id, + read_only: false + }; } catch (e) { mountsError = errorMessage(e); } finally { @@ -3267,11 +3282,21 @@ bind:value={newMount.host_path} data-testid="mount-path" /> + - @@ -3289,6 +3314,7 @@ {t('admin.mounts.name', 'Name')} {t('admin.mounts.kind', 'Kind')} + {t('admin.mounts.drive', 'Drive')} {t('admin.mounts.path', 'Path')} {t('admin.mounts.readonly', 'Read-only')} @@ -3299,6 +3325,7 @@ {m.name} {m.kind} + {mountDriveName(m.drive_id)} {m.mount_path} {m.read_only ? t('common.yes', 'Yes') : t('common.no', 'No')} diff --git a/frontend/src/routes/admin/[[tab]]/page.test.ts b/frontend/src/routes/admin/[[tab]]/page.test.ts index 94a37b2b..5b74c61c 100644 --- a/frontend/src/routes/admin/[[tab]]/page.test.ts +++ b/frontend/src/routes/admin/[[tab]]/page.test.ts @@ -50,6 +50,7 @@ vi.mock('$lib/api/endpoints/admin', () => ({ deleteUser: vi.fn(), getDashboard: vi.fn(), listExternalMounts: vi.fn(), + listAllDrives: vi.fn(), getMigration: vi.fn(), getOidcSettings: vi.fn(), getPluginLogs: vi.fn(), @@ -129,6 +130,17 @@ const mount = { config: { path: '/srv/media', read_only: true } }; +const mountDrive = { + id: 'd1', + name: 'Shared media', + kind: 'shared', + root_folder_id: 'root-d1', + used_bytes: 0, + policies: {}, + created_at: '2026-09-01T00:00:00Z', + updated_at: '2026-09-01T00:00:00Z' +}; + beforeEach(() => { vi.clearAllMocks(); // Reset the tab mock so a test that sets `setTab('users')` @@ -167,6 +179,7 @@ beforeEach(() => { user_state: 'unset' }); m(admin.listExternalMounts).mockResolvedValue([mount]); + m(admin.listAllDrives).mockResolvedValue([mountDrive]); }); it('loads the dashboard on mount', async () => { @@ -227,8 +240,10 @@ it('loads external mounts when the mounts tab is opened and lists them', async ( setTab('mounts'); render(AdminPage); await waitFor(() => expect(admin.listExternalMounts).toHaveBeenCalled()); + await waitFor(() => expect(admin.listAllDrives).toHaveBeenCalled()); // The configured mount is rendered in the table. expect(await screen.findByText('Media')).toBeTruthy(); + expect((await screen.findAllByText('Shared media')).length).toBeGreaterThan(0); }); it('creates a mount from the mounts form', async () => { @@ -250,10 +265,13 @@ it('creates a mount from the mounts form', async () => { await fireEvent.input(screen.getByTestId('mount-path'), { target: { value: '/srv/photos' } }); + await fireEvent.change(screen.getByTestId('mount-drive'), { + target: { value: 'd1' } + }); await fireEvent.click(screen.getByTestId('mount-create')); await waitFor(() => expect(admin.createExternalMount).toHaveBeenCalledWith( - expect.objectContaining({ name: 'Photos', host_path: '/srv/photos' }) + expect.objectContaining({ name: 'Photos', host_path: '/srv/photos', drive_id: 'd1' }) ) ); }); diff --git a/migrations/20261025000000_external_mount_drive_visibility.sql b/migrations/20261025000000_external_mount_drive_visibility.sql new file mode 100644 index 00000000..67912653 --- /dev/null +++ b/migrations/20261025000000_external_mount_drive_visibility.sql @@ -0,0 +1,12 @@ +-- External mounts inherit access from the drive containing their mount-root +-- folder. Existing mounts remain in their current drive; for the historical +-- default that is the configuring administrator's personal drive. +UPDATE storage.external_mounts +SET visibility = 'drive' +WHERE visibility = 'owner'; + +ALTER TABLE storage.external_mounts + ALTER COLUMN visibility SET DEFAULT 'drive'; + +COMMENT ON COLUMN storage.external_mounts.visibility IS + 'Compatibility marker. External mount visibility is inherited from the mount-root folder drive and its role grants.'; diff --git a/src/application/services/folder_service.rs b/src/application/services/folder_service.rs index d8365127..ac2ed941 100644 --- a/src/application/services/folder_service.rs +++ b/src/application/services/folder_service.rs @@ -1783,12 +1783,18 @@ mod mount_authz_integration { use crate::application::services::external_mount_router::{MountRouter, ResolvedId}; use crate::application::services::file_retrieval_service::FileRetrievalService; use crate::application::services::mount_registry::MountRegistry; + use crate::domain::repositories::drive_repository::DriveRepository; + use crate::domain::repositories::folder_repository::FolderRepository; + use crate::domain::services::authorization::Subject; use crate::domain::services::external_mount_id::{NodeId, encode_child_id}; use crate::infrastructure::repositories::pg::{ - ExternalMountPgRepository, FileBlobReadRepository, SubjectGroupPgRepository, + DrivePgRepository, ExternalMountPgRepository, FileBlobReadRepository, + SubjectGroupPgRepository, }; use crate::infrastructure::services::mount_provider_factory::DefaultMountProviderFactory; - use crate::mount_it_support::{fresh_db, insert_mount, make_user, provision_folder}; + use crate::mount_it_support::{ + Provisioned, fresh_db, insert_mount, make_user, provision_folder, + }; use std::sync::Arc; fn opts<'a>() -> ListResourcesOptions<'a> { @@ -2206,6 +2212,68 @@ mod mount_authz_integration { assert_eq!(err.kind, crate::domain::errors::ErrorKind::NotFound); } + /// A mount attached to a shared drive inherits that drive's grants rather + /// than the identity of the administrator who configured the provider. + #[tokio::test] + async fn shared_drive_member_lists_mount_non_member_denied() { + let (_c, pool) = fresh_db().await; + let host = tempfile::tempdir().unwrap(); + std::fs::write(host.path().join("shared.txt"), b"shared").unwrap(); + + let admin_id = make_user(&pool, "mount-admin").await; + let member_id = make_user(&pool, "drive-member").await; + let drive = DrivePgRepository::new(pool.clone()) + .create_shared_drive_atomic("Shared media", Subject::User(member_id), None, admin_id) + .await + .expect("create shared drive"); + let folder = FolderDbRepository::new(pool.clone()) + .create_folder( + "Media".to_string(), + Some(drive.drive.root_folder_id.to_string()), + admin_id, + ) + .await + .expect("create mount root in shared drive"); + let mount_folder_id = Uuid::parse_str(folder.id()).expect("folder UUID"); + let provisioned = Provisioned { + owner_id: admin_id, + drive_id: drive.drive.id, + mount_folder_id, + }; + insert_mount(&pool, &provisioned, host.path().to_str().unwrap()).await; + + let registry = Arc::new(MountRegistry::empty()); + registry + .reload( + &ExternalMountPgRepository::new(pool.clone()), + &DefaultMountProviderFactory::new(), + ) + .await; + let folder_service = FolderService::new( + Arc::new(FolderDbRepository::new(pool.clone())), + acl(&pool), + Arc::new( + crate::application::services::file_lifecycle_service::FileLifecycleService::new(), + ), + Arc::new(MountRouter::new(registry.clone())), + ); + let cfg = registry.get(&mount_folder_id).expect("mount registered"); + + let (entries, _) = folder_service + .list_mount_dir_with_perms(&cfg, &NodeId::default(), member_id, opts()) + .await + .expect("shared drive member may list mount"); + assert_eq!(entries.len(), 1); + assert_eq!(entries[0].name, "shared.txt"); + + let non_member = make_user(&pool, "non-member").await; + let err = folder_service + .list_mount_dir_with_perms(&cfg, &NodeId::default(), non_member, opts()) + .await + .expect_err("non-member must be denied"); + assert_eq!(err.kind, crate::domain::errors::ErrorKind::NotFound); + } + /// Download path authz: owner can stat/open a mount file; stranger denied. #[tokio::test] async fn owner_reads_mount_file_stranger_denied() { diff --git a/src/interfaces/api/handlers/admin_external_mounts.rs b/src/interfaces/api/handlers/admin_external_mounts.rs index c4a3c21b..20a35bf1 100644 --- a/src/interfaces/api/handlers/admin_external_mounts.rs +++ b/src/interfaces/api/handlers/admin_external_mounts.rs @@ -1,7 +1,7 @@ //! Admin CRUD for external file mounts (`/api/admin/external-mounts`). //! //! Creating a mount: validate the backend config, create a mount-root folder -//! under the admin's drive, insert the `external_mounts` row, then hot-reload +//! under the selected drive, insert the `external_mounts` row, then hot-reload //! the in-memory registry. Deleting: remove the row + the folder and reload. //! Every endpoint is admin-gated. @@ -20,7 +20,7 @@ use crate::application::ports::external_mount_ports::{ ExternalMountRecord, ExternalMountRepositoryPort, MountProviderFactory, NewExternalMount, }; use crate::common::di::AppState; -use crate::domain::repositories::drive_repository::DriveRepository; +use crate::domain::repositories::drive_repository::{DriveRepository, DriveRepositoryError}; use crate::domain::repositories::folder_repository::FolderRepository; use crate::infrastructure::repositories::pg::ExternalMountPgRepository; use crate::infrastructure::services::mount_provider_factory::DefaultMountProviderFactory; @@ -60,6 +60,8 @@ impl From for ExternalMountResponse { pub struct CreateExternalMountRequest { /// Display name (also the mount-root folder name). pub name: String, + /// Drive that owns the mount and controls access through its grants. + pub drive_id: Uuid, /// Absolute host path for the `local_fs` provider. pub host_path: String, /// Provider kind. Defaults to `local_fs`. @@ -96,7 +98,7 @@ pub async fn list_external_mounts( Ok(Json(out)) } -/// `POST /api/admin/external-mounts` — create a mount in the admin's drive. +/// `POST /api/admin/external-mounts` — create a mount in the selected drive. pub async fn create_external_mount( State(state): State>, headers: HeaderMap, @@ -116,12 +118,19 @@ pub async fn create_external_mount( .await .map_err(|e| AppError::bad_request(format!("invalid mount configuration: {e}")))?; - // Create the mount-root folder under the admin's default drive root. + // The mount-root is a normal folder in the selected drive. All access to + // the external provider is authorized against this folder, so personal + // and shared drive membership automatically applies to the mount. let drive = state .drive_repo - .find_default_for_user(admin_id) + .get_by_id(req.drive_id) .await - .map_err(|e| AppError::internal_error(format!("find default drive: {e}")))?; + .map_err(|e| match e { + DriveRepositoryError::NotFound(_) => { + AppError::bad_request("Destination drive not found") + } + other => AppError::internal_error(format!("find destination drive: {other}")), + })?; let root_folder_id = drive.drive.root_folder_id.to_string(); let folder = state @@ -153,6 +162,7 @@ pub async fn create_external_mount( event = "external_mount.config", action = "create", mount_id = %mount_folder_id, + drive_id = %req.drive_id, caller_id = %admin_id, kind = %req.kind, reason = "external_mount_admin", @@ -212,3 +222,34 @@ pub async fn delete_external_mount( Ok(StatusCode::NO_CONTENT) } + +#[cfg(test)] +mod tests { + use super::CreateExternalMountRequest; + + #[test] + fn create_request_requires_destination_drive() { + let missing_drive = + serde_json::from_value::(serde_json::json!({ + "name": "Media", + "host_path": "/srv/media" + })); + + assert!(missing_drive.is_err()); + } + + #[test] + fn create_request_accepts_destination_drive() { + let drive_id = uuid::Uuid::new_v4(); + let request = serde_json::from_value::(serde_json::json!({ + "name": "Media", + "host_path": "/srv/media", + "drive_id": drive_id + })) + .expect("valid external mount request"); + + assert_eq!(request.drive_id, drive_id); + assert_eq!(request.kind, "local_fs"); + assert!(!request.read_only); + } +}