Issue #224: File Manager Permissions

The File Manager (Library) backend had no permission enforcement - endpoints were returning data to any authenticated user regardless of their group permissions.

Closes #224

CHANGELOG.md

@@ -56,6 +56,14 @@ All notable changes to Bambuddy will be documented in this file.
   - Removed ~2000 lines of legacy JSON-based backup/restore code
 
 ### Fixes
+- **File Manager permissions not enforced** (Issue #224) - Fixed backend not checking `library:read` permission for File Manager endpoints:
+  - Added `library:read` permission check to all list/view endpoints (files, folders, stats)
+  - Added `library:upload` permission check to upload and folder creation endpoints
+  - Added `queue:create` permission check to add-to-queue endpoint
+  - Added `printers:control` permission check to direct print endpoint
+  - Added ownership-based permission checks to file move operation
+  - Users without `library:read` permission can no longer view files in the File Manager
+  - Users can now only delete/update their own files unless they have `*_all` permissions
 - **JWT secret key not persistent across restarts** - Fixed JWT secret key generation to properly use data directory, ensuring tokens remain valid across container restarts
 - **Images/thumbnails returning 401 when auth enabled** - Fixed auth middleware to allow public access to image/media endpoints (thumbnails, photos, QR codes, timelapses, camera streams) since browser elements like `<img>` don't send Authorization headers
 - **Library thumbnails missing after restore** - Fixed library files using absolute paths that break after restore on different systems:

backend/app/api/routes/library.py

@@ -16,7 +16,6 @@ from sqlalchemy.ext.asyncio import AsyncSession
 from sqlalchemy.orm import selectinload
 
 from backend.app.core.auth import (
-    require_auth_if_enabled,
     require_ownership_permission,
     require_permission_if_auth_enabled,
 )
@@ -237,7 +236,11 @@ IMAGE_EXTENSIONS = {".png", ".jpg", ".jpeg", ".gif", ".webp", ".bmp", ".tiff", "
 
 @router.get("/folders", response_model=list[FolderTreeItem])
 @router.get("/folders/", response_model=list[FolderTreeItem])
-async def list_folders(response: Response, db: AsyncSession = Depends(get_db)):
+async def list_folders(
+    response: Response,
+    db: AsyncSession = Depends(get_db),
+    _: User | None = Depends(require_permission_if_auth_enabled(Permission.LIBRARY_READ)),
+):
     """Get all folders as a tree structure."""
     # Prevent browser caching of folder list
     response.headers["Cache-Control"] = "no-cache, no-store, must-revalidate"
@@ -289,7 +292,11 @@ async def list_folders(response: Response, db: AsyncSession = Depends(get_db)):
 
 
 @router.get("/folders/by-project/{project_id}", response_model=list[FolderResponse])
-async def get_folders_by_project(project_id: int, db: AsyncSession = Depends(get_db)):
+async def get_folders_by_project(
+    project_id: int,
+    db: AsyncSession = Depends(get_db),
+    _: User | None = Depends(require_permission_if_auth_enabled(Permission.LIBRARY_READ)),
+):
     """Get all folders linked to a specific project."""
     result = await db.execute(
         select(LibraryFolder, Project.name)
@@ -326,7 +333,11 @@ async def get_folders_by_project(project_id: int, db: AsyncSession = Depends(get
 
 
 @router.get("/folders/by-archive/{archive_id}", response_model=list[FolderResponse])
-async def get_folders_by_archive(archive_id: int, db: AsyncSession = Depends(get_db)):
+async def get_folders_by_archive(
+    archive_id: int,
+    db: AsyncSession = Depends(get_db),
+    _: User | None = Depends(require_permission_if_auth_enabled(Permission.LIBRARY_READ)),
+):
     """Get all folders linked to a specific archive."""
     result = await db.execute(
         select(LibraryFolder, PrintArchive.print_name)
@@ -364,7 +375,11 @@ async def get_folders_by_archive(archive_id: int, db: AsyncSession = Depends(get
 
 @router.post("/folders", response_model=FolderResponse)
 @router.post("/folders/", response_model=FolderResponse)
-async def create_folder(data: FolderCreate, db: AsyncSession = Depends(get_db)):
+async def create_folder(
+    data: FolderCreate,
+    db: AsyncSession = Depends(get_db),
+    _: User | None = Depends(require_permission_if_auth_enabled(Permission.LIBRARY_UPLOAD)),
+):
     """Create a new folder."""
     # Verify parent exists if specified
     if data.parent_id is not None:
@@ -415,7 +430,11 @@ async def create_folder(data: FolderCreate, db: AsyncSession = Depends(get_db)):
 
 
 @router.get("/folders/{folder_id}", response_model=FolderResponse)
-async def get_folder(folder_id: int, db: AsyncSession = Depends(get_db)):
+async def get_folder(
+    folder_id: int,
+    db: AsyncSession = Depends(get_db),
+    _: User | None = Depends(require_permission_if_auth_enabled(Permission.LIBRARY_READ)),
+):
     """Get a folder by ID."""
     result = await db.execute(
         select(LibraryFolder, Project.name, PrintArchive.print_name)
@@ -449,8 +468,17 @@ async def get_folder(folder_id: int, db: AsyncSession = Depends(get_db)):
 
 
 @router.put("/folders/{folder_id}", response_model=FolderResponse)
-async def update_folder(folder_id: int, data: FolderUpdate, db: AsyncSession = Depends(get_db)):
-    """Update a folder."""
+async def update_folder(
+    folder_id: int,
+    data: FolderUpdate,
+    db: AsyncSession = Depends(get_db),
+    _: User | None = Depends(require_permission_if_auth_enabled(Permission.LIBRARY_UPDATE_ALL)),
+):
+    """Update a folder.
+
+    Note: Folders require library:update_all permission since they don't have
+    ownership tracking.
+    """
     result = await db.execute(select(LibraryFolder).where(LibraryFolder.id == folder_id))
     folder = result.scalar_one_or_none()
 
@@ -595,6 +623,7 @@ async def list_files(
     folder_id: int | None = None,
     include_root: bool = True,
     db: AsyncSession = Depends(get_db),
+    _: User | None = Depends(require_permission_if_auth_enabled(Permission.LIBRARY_READ)),
 ):
     """List files, optionally filtered by folder.
 
@@ -669,7 +698,7 @@ async def upload_file(
     folder_id: int | None = None,
     generate_stl_thumbnails: bool = Query(default=True),
     db: AsyncSession = Depends(get_db),
-    current_user: User | None = Depends(require_auth_if_enabled),
+    current_user: User | None = Depends(require_permission_if_auth_enabled(Permission.LIBRARY_UPLOAD)),
 ):
     """Upload a file to the library."""
     try:
@@ -805,7 +834,7 @@ async def extract_zip_file(
     create_folder_from_zip: bool = Query(default=False),
     generate_stl_thumbnails: bool = Query(default=True),
     db: AsyncSession = Depends(get_db),
-    current_user: User | None = Depends(require_auth_if_enabled),
+    current_user: User | None = Depends(require_permission_if_auth_enabled(Permission.LIBRARY_UPLOAD)),
 ):
     """Upload and extract a ZIP file to the library.
 
@@ -1064,9 +1093,13 @@ async def extract_zip_file(
 async def batch_generate_stl_thumbnails(
     request: BatchThumbnailRequest,
     db: AsyncSession = Depends(get_db),
+    _: User | None = Depends(require_permission_if_auth_enabled(Permission.LIBRARY_UPDATE_ALL)),
 ):
     """Generate thumbnails for STL files in batch.
 
+    Note: Requires library:update_all permission since this is a batch operation
+    that may affect files owned by different users.
+
     Can generate thumbnails for:
     - Specific file IDs (file_ids)
     - All STL files in a folder (folder_id)
@@ -1188,6 +1221,7 @@ def is_sliced_file(filename: str) -> bool:
 async def add_files_to_queue(
     request: AddToQueueRequest,
     db: AsyncSession = Depends(get_db),
+    _: User | None = Depends(require_permission_if_auth_enabled(Permission.QUEUE_CREATE)),
 ):
     """Add library files to the print queue.
 
@@ -1266,6 +1300,7 @@ async def add_files_to_queue(
 async def get_library_file_plates(
     file_id: int,
     db: AsyncSession = Depends(get_db),
+    _: User | None = Depends(require_permission_if_auth_enabled(Permission.LIBRARY_READ)),
 ):
     """Get available plates from a multi-plate 3MF library file.
 
@@ -1477,6 +1512,7 @@ async def get_library_file_filament_requirements(
     file_id: int,
     plate_id: int | None = None,
     db: AsyncSession = Depends(get_db),
+    _: User | None = Depends(require_permission_if_auth_enabled(Permission.LIBRARY_READ)),
 ):
     """Get filament requirements from a library file.
 
@@ -1599,6 +1635,7 @@ async def print_library_file(
     printer_id: int,
     body: FilePrintRequest | None = None,
     db: AsyncSession = Depends(get_db),
+    _: User | None = Depends(require_permission_if_auth_enabled(Permission.PRINTERS_CONTROL)),
 ):
     """Print a library file directly.
 
@@ -1786,7 +1823,11 @@ async def print_library_file(
 
 
 @router.get("/files/{file_id}", response_model=FileResponseSchema)
-async def get_file(file_id: int, db: AsyncSession = Depends(get_db)):
+async def get_file(
+    file_id: int,
+    db: AsyncSession = Depends(get_db),
+    _: User | None = Depends(require_permission_if_auth_enabled(Permission.LIBRARY_READ)),
+):
     """Get a file by ID with full details."""
     result = await db.execute(
         select(LibraryFile).options(selectinload(LibraryFile.created_by)).where(LibraryFile.id == file_id)
@@ -1961,7 +2002,11 @@ async def delete_file(
 
 
 @router.get("/files/{file_id}/download")
-async def download_file(file_id: int, db: AsyncSession = Depends(get_db)):
+async def download_file(
+    file_id: int,
+    db: AsyncSession = Depends(get_db),
+    _: User | None = Depends(require_permission_if_auth_enabled(Permission.LIBRARY_READ)),
+):
     """Download a file."""
     result = await db.execute(select(LibraryFile).where(LibraryFile.id == file_id))
     file = result.scalar_one_or_none()
@@ -2008,7 +2053,11 @@ async def get_thumbnail(file_id: int, db: AsyncSession = Depends(get_db)):
 
 
 @router.get("/files/{file_id}/gcode")
-async def get_gcode(file_id: int, db: AsyncSession = Depends(get_db)):
+async def get_gcode(
+    file_id: int,
+    db: AsyncSession = Depends(get_db),
+    _: User | None = Depends(require_permission_if_auth_enabled(Permission.LIBRARY_READ)),
+):
     """Get gcode for a file (for preview)."""
     result = await db.execute(select(LibraryFile).where(LibraryFile.id == file_id))
     file = result.scalar_one_or_none()
@@ -2046,8 +2095,22 @@ async def get_gcode(file_id: int, db: AsyncSession = Depends(get_db)):
 
 
 @router.post("/files/move")
-async def move_files(data: FileMoveRequest, db: AsyncSession = Depends(get_db)):
-    """Move multiple files to a folder."""
+async def move_files(
+    data: FileMoveRequest,
+    db: AsyncSession = Depends(get_db),
+    auth_result: tuple[User | None, bool] = Depends(
+        require_ownership_permission(
+            Permission.LIBRARY_UPDATE_ALL,
+            Permission.LIBRARY_UPDATE_OWN,
+        )
+    ),
+):
+    """Move multiple files to a folder.
+
+    Files not owned by the user are skipped (unless user has *_all permission).
+    """
+    user, can_modify_all = auth_result
+
     # Verify folder exists if specified
     if data.folder_id is not None:
         folder_result = await db.execute(select(LibraryFolder).where(LibraryFolder.id == data.folder_id))
@@ -2056,14 +2119,19 @@ async def move_files(data: FileMoveRequest, db: AsyncSession = Depends(get_db)):
 
     # Update files
     moved = 0
+    skipped = 0
     for file_id in data.file_ids:
         result = await db.execute(select(LibraryFile).where(LibraryFile.id == file_id))
         file = result.scalar_one_or_none()
         if file:
+            # Ownership check
+            if not can_modify_all and file.created_by_id != user.id:
+                skipped += 1
+                continue
             file.folder_id = data.folder_id
             moved += 1
 
-    return {"status": "success", "moved": moved}
+    return {"status": "success", "moved": moved, "skipped": skipped}
 
 
 @router.post("/bulk-delete", response_model=BulkDeleteResponse)
@@ -2133,7 +2201,10 @@ async def bulk_delete(
 
 
 @router.get("/stats")
-async def get_library_stats(db: AsyncSession = Depends(get_db)):
+async def get_library_stats(
+    db: AsyncSession = Depends(get_db),
+    _: User | None = Depends(require_permission_if_auth_enabled(Permission.LIBRARY_READ)),
+):
     """Get library statistics."""
     # Total files
     total_files_result = await db.execute(select(func.count(LibraryFile.id)))

backend/tests/integration/test_library_api.py

@@ -848,3 +848,210 @@ class TestLibraryPathHelpers:
 
         assert to_absolute_path(None) is None
         assert to_absolute_path("") is None
+
+
+class TestLibraryPermissions:
+    """Tests for library permission enforcement."""
+
+    @pytest.fixture
+    async def auth_setup(self, db_session):
+        """Set up auth with users of different permission levels."""
+        from backend.app.core.auth import create_access_token, get_password_hash
+        from backend.app.models.group import Group
+        from backend.app.models.settings import Settings
+        from backend.app.models.user import User
+
+        # Enable auth
+        settings = Settings(key="auth_enabled", value="true")
+        db_session.add(settings)
+        await db_session.commit()
+
+        # Groups are auto-seeded during db init, but we need to commit them
+        await db_session.commit()
+
+        # Get groups
+        from sqlalchemy import select
+
+        admin_group = (await db_session.execute(select(Group).where(Group.name == "Administrators"))).scalar_one()
+        operator_group = (await db_session.execute(select(Group).where(Group.name == "Operators"))).scalar_one()
+        viewer_group = (await db_session.execute(select(Group).where(Group.name == "Viewers"))).scalar_one()
+
+        password_hash = get_password_hash("password")
+
+        # Create users
+        admin_user = User(username="admin_lib", password_hash=password_hash, role="admin", is_active=True)
+        admin_user.groups.append(admin_group)
+
+        operator_user = User(username="operator_lib", password_hash=password_hash, is_active=True)
+        operator_user.groups.append(operator_group)
+
+        viewer_user = User(username="viewer_lib", password_hash=password_hash, is_active=True)
+        viewer_user.groups.append(viewer_group)
+
+        db_session.add_all([admin_user, operator_user, viewer_user])
+        await db_session.commit()
+
+        # Create tokens
+        admin_token = create_access_token(data={"sub": admin_user.username})
+        operator_token = create_access_token(data={"sub": operator_user.username})
+        viewer_token = create_access_token(data={"sub": viewer_user.username})
+
+        return {
+            "admin_user": admin_user,
+            "operator_user": operator_user,
+            "viewer_user": viewer_user,
+            "admin_token": admin_token,
+            "operator_token": operator_token,
+            "viewer_token": viewer_token,
+        }
+
+    @pytest.fixture
+    async def test_file(self, db_session, auth_setup):
+        """Create a test file owned by the operator user."""
+        from backend.app.models.library import LibraryFile
+
+        operator_user = auth_setup["operator_user"]
+        lib_file = LibraryFile(
+            filename="test.txt",
+            file_path="data/archive/library/files/test.txt",
+            file_type="txt",
+            file_size=100,
+            created_by_id=operator_user.id,
+        )
+        db_session.add(lib_file)
+        await db_session.commit()
+        await db_session.refresh(lib_file)
+        return lib_file
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_list_files_requires_library_read(self, async_client: AsyncClient, db_session, auth_setup):
+        """Verify list_files requires library:read permission."""
+        viewer_token = auth_setup["viewer_token"]
+
+        # Viewers have library:read, should succeed
+        response = await async_client.get("/api/v1/library/files", headers={"Authorization": f"Bearer {viewer_token}"})
+        assert response.status_code == 200
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_list_files_denied_without_permission(self, async_client: AsyncClient, db_session):
+        """Verify list_files denied without auth when auth is enabled."""
+        from backend.app.models.settings import Settings
+
+        # Enable auth
+        settings = Settings(key="auth_enabled", value="true")
+        db_session.add(settings)
+        await db_session.commit()
+
+        # Request without token should fail
+        response = await async_client.get("/api/v1/library/files")
+        assert response.status_code == 401
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_delete_file_own_by_owner(self, async_client: AsyncClient, db_session, auth_setup, test_file):
+        """Verify operator can delete their own files."""
+        from pathlib import Path
+
+        # Create actual file on disk so delete doesn't fail
+        from backend.app.core.config import settings as app_settings
+
+        file_path = Path(app_settings.base_dir) / test_file.file_path
+        file_path.parent.mkdir(parents=True, exist_ok=True)
+        file_path.write_text("test content")
+
+        operator_token = auth_setup["operator_token"]
+
+        response = await async_client.delete(
+            f"/api/v1/library/files/{test_file.id}", headers={"Authorization": f"Bearer {operator_token}"}
+        )
+        assert response.status_code == 200
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_delete_file_own_denied_for_others_file(self, async_client: AsyncClient, db_session, auth_setup):
+        """Verify operator cannot delete files owned by others."""
+        # Create another operator user with a file
+        from sqlalchemy import select
+
+        from backend.app.core.auth import create_access_token
+        from backend.app.models.group import Group
+        from backend.app.models.library import LibraryFile
+        from backend.app.models.user import User
+
+        operator_group = (await db_session.execute(select(Group).where(Group.name == "Operators"))).scalar_one()
+
+        from backend.app.core.auth import get_password_hash as get_pw_hash
+
+        other_user = User(username="other_op", password_hash=get_pw_hash("password"), is_active=True)
+        other_user.groups.append(operator_group)
+        db_session.add(other_user)
+        await db_session.commit()
+        await db_session.refresh(other_user)
+
+        # Create file owned by other user
+        other_file = LibraryFile(
+            filename="other.txt",
+            file_path="data/archive/library/files/other.txt",
+            file_type="txt",
+            file_size=100,
+            created_by_id=other_user.id,
+        )
+        db_session.add(other_file)
+        await db_session.commit()
+        await db_session.refresh(other_file)
+
+        # Original operator should not be able to delete it
+        operator_token = auth_setup["operator_token"]
+        response = await async_client.delete(
+            f"/api/v1/library/files/{other_file.id}", headers={"Authorization": f"Bearer {operator_token}"}
+        )
+        assert response.status_code == 403
+        assert "your own files" in response.json()["detail"].lower()
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_delete_file_admin_can_delete_any(self, async_client: AsyncClient, db_session, auth_setup):
+        """Verify admin can delete any file."""
+        from pathlib import Path
+
+        from backend.app.core.config import settings as app_settings
+        from backend.app.models.library import LibraryFile
+
+        # Create file owned by operator
+        operator_user = auth_setup["operator_user"]
+        lib_file = LibraryFile(
+            filename="admin_can_delete.txt",
+            file_path="data/archive/library/files/admin_can_delete.txt",
+            file_type="txt",
+            file_size=100,
+            created_by_id=operator_user.id,
+        )
+        db_session.add(lib_file)
+        await db_session.commit()
+        await db_session.refresh(lib_file)
+
+        # Create actual file on disk
+        file_path = Path(app_settings.base_dir) / lib_file.file_path
+        file_path.parent.mkdir(parents=True, exist_ok=True)
+        file_path.write_text("test content")
+
+        # Admin should be able to delete it
+        admin_token = auth_setup["admin_token"]
+        response = await async_client.delete(
+            f"/api/v1/library/files/{lib_file.id}", headers={"Authorization": f"Bearer {admin_token}"}
+        )
+        assert response.status_code == 200
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_viewer_cannot_delete_files(self, async_client: AsyncClient, db_session, auth_setup, test_file):
+        """Verify viewer cannot delete any files."""
+        viewer_token = auth_setup["viewer_token"]
+
+        response = await async_client.delete(
+            f"/api/v1/library/files/{test_file.id}", headers={"Authorization": f"Bearer {viewer_token}"}
+        )
+        # Viewers don't have delete_own or delete_all permissions
+        assert response.status_code == 403