From 02fd5563722cc2a653e127d4082940b1289974a2 Mon Sep 17 00:00:00 2001 From: Mistral Vibe Date: Sun, 29 Mar 2026 20:06:12 +0200 Subject: [PATCH] feat: remove global Nextcloud config, enforce member-specific storage providers - Remove global Nextcloud settings from config - Make NextcloudClient require explicit credentials - Update for_member() to return None when no credentials - Modify services to accept optional storage client - Update routers to pass member storage to services - Add 403 responses when no storage provider configured - Update internal endpoints to use member storage credentials This change enforces that each member must configure their own Nextcloud storage provider. If no provider is configured, file operations will return 403 FORBIDDEN instead of falling back to global placeholders. --- api/src/rehearsalhub/config.py | 5 - api/src/rehearsalhub/routers/bands.py | 7 +- api/src/rehearsalhub/routers/internal.py | 11 ++- api/src/rehearsalhub/routers/songs.py | 6 +- api/src/rehearsalhub/routers/versions.py | 108 +++++++++++++++++++++- api/src/rehearsalhub/services/band.py | 2 +- api/src/rehearsalhub/services/song.py | 2 +- api/src/rehearsalhub/storage/nextcloud.py | 45 ++++++--- 8 files changed, 155 insertions(+), 31 deletions(-) diff --git a/api/src/rehearsalhub/config.py b/api/src/rehearsalhub/config.py index 67f193a..5be2b29 100644 --- a/api/src/rehearsalhub/config.py +++ b/api/src/rehearsalhub/config.py @@ -17,11 +17,6 @@ class Settings(BaseSettings): redis_url: str = "redis://localhost:6379/0" job_queue_key: str = "rh:jobs" - # Nextcloud - nextcloud_url: str = "http://nextcloud" - nextcloud_user: str = "ncadmin" - nextcloud_pass: str = "" - # App domain: str = "localhost" debug: bool = False diff --git a/api/src/rehearsalhub/routers/bands.py b/api/src/rehearsalhub/routers/bands.py index 1b8ba99..0190acd 100644 --- a/api/src/rehearsalhub/routers/bands.py +++ b/api/src/rehearsalhub/routers/bands.py @@ -9,6 +9,7 @@ from rehearsalhub.dependencies import get_current_member from rehearsalhub.schemas.band import BandCreate, BandRead, BandReadWithMembers, BandUpdate from rehearsalhub.repositories.band import BandRepository from rehearsalhub.services.band import BandService +from rehearsalhub.storage.nextcloud import NextcloudClient router = APIRouter(prefix="/bands", tags=["bands"]) @@ -29,7 +30,8 @@ async def create_band( session: AsyncSession = Depends(get_session), current_member: Member = Depends(get_current_member), ): - svc = BandService(session) + storage = NextcloudClient.for_member(current_member) + svc = BandService(session, storage) try: band = await svc.create_band(data, current_member.id, creator=current_member) except ValueError as e: @@ -45,7 +47,8 @@ async def get_band( session: AsyncSession = Depends(get_session), current_member: Member = Depends(get_current_member), ): - svc = BandService(session) + storage = NextcloudClient.for_member(current_member) + svc = BandService(session, storage) try: await svc.assert_membership(band_id, current_member.id) except PermissionError: diff --git a/api/src/rehearsalhub/routers/internal.py b/api/src/rehearsalhub/routers/internal.py index 7755c66..3b79248 100644 --- a/api/src/rehearsalhub/routers/internal.py +++ b/api/src/rehearsalhub/routers/internal.py @@ -117,7 +117,16 @@ async def nc_upload( ) uploader_id = result.scalar_one_or_none() - song_svc = SongService(session) + # Get the uploader's storage credentials + storage = None + if uploader_id: + uploader_result = await session.execute( + select(Member).where(Member.id == uploader_id).limit(1) + ) + uploader = uploader_result.scalar_one_or_none() + storage = NextcloudClient.for_member(uploader) if uploader else None + + song_svc = SongService(session, storage=storage) version = await song_svc.register_version( song.id, AudioVersionCreate( diff --git a/api/src/rehearsalhub/routers/songs.py b/api/src/rehearsalhub/routers/songs.py index dc5d2a7..ea4f40b 100644 --- a/api/src/rehearsalhub/routers/songs.py +++ b/api/src/rehearsalhub/routers/songs.py @@ -48,7 +48,8 @@ async def list_songs( await band_svc.assert_membership(band_id, current_member.id) except PermissionError: raise HTTPException(status_code=status.HTTP_403_FORBIDDEN, detail="Not a member") - song_svc = SongService(session) + storage = NextcloudClient.for_member(current_member) + song_svc = SongService(session, storage=storage) return await song_svc.list_songs(band_id) @@ -131,7 +132,8 @@ async def create_song( if band is None: raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Band not found") - song_svc = SongService(session) + storage = NextcloudClient.for_member(current_member) + song_svc = SongService(session, storage=storage) song = await song_svc.create_song(band_id, data, current_member.id, band.slug, creator=current_member) read = SongRead.model_validate(song) read.version_count = 0 diff --git a/api/src/rehearsalhub/routers/versions.py b/api/src/rehearsalhub/routers/versions.py index bdc34a1..ea87a91 100644 --- a/api/src/rehearsalhub/routers/versions.py +++ b/api/src/rehearsalhub/routers/versions.py @@ -1,7 +1,9 @@ import uuid +import asyncio from pathlib import Path from typing import Any +import httpx from fastapi import APIRouter, Depends, HTTPException, Query, Request, status from fastapi.responses import Response from fastapi.security.utils import get_authorization_scheme_param @@ -33,6 +35,45 @@ _AUDIO_CONTENT_TYPES: dict[str, str] = { } +async def _download_with_retry(storage: NextcloudClient, file_path: str, max_retries: int = 3) -> bytes: + """Download file from Nextcloud with retry logic for transient errors.""" + last_error = None + + for attempt in range(max_retries): + try: + data = await storage.download(file_path) + return data + except httpx.ConnectError as e: + last_error = e + if attempt < max_retries - 1: + # Exponential backoff: 1s, 2s, 4s + wait_time = 2 ** attempt + await asyncio.sleep(wait_time) + continue + except httpx.HTTPStatusError as e: + # Don't retry on 4xx errors (client errors) + if e.response.status_code >= 500: + last_error = e + if attempt < max_retries - 1: + wait_time = 2 ** attempt + await asyncio.sleep(wait_time) + continue + else: + raise + except Exception as e: + last_error = e + break + + # If we exhausted retries, raise the last error + if last_error: + raise last_error + else: + raise HTTPException( + status_code=status.HTTP_503_SERVICE_UNAVAILABLE, + detail="Failed to download file from storage" + ) + + async def _member_from_request( request: Request, token: str | None = Query(None), @@ -124,7 +165,8 @@ async def create_version( except PermissionError: raise HTTPException(status_code=status.HTTP_403_FORBIDDEN, detail="Not a member") - song_svc = SongService(session) + storage = NextcloudClient.for_member(current_member) + song_svc = SongService(session, storage=storage) version = await song_svc.register_version(song_id, data, current_member.id) return AudioVersionRead.model_validate(version) @@ -138,8 +180,36 @@ async def get_waveform( version, _ = await _get_version_and_assert_band_membership(version_id, session, current_member) if not version.waveform_url: raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Waveform not ready") - storage = NextcloudClient() - data = await storage.download(version.waveform_url) + + storage = NextcloudClient.for_member(current_member) + if storage is None: + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail="No storage provider configured for this account" + ) + try: + data = await _download_with_retry(storage, version.waveform_url) + except httpx.ConnectError as e: + raise HTTPException( + status_code=status.HTTP_503_SERVICE_UNAVAILABLE, + detail=f"Failed to connect to storage: {str(e)}" + ) + except httpx.HTTPStatusError as e: + if e.response.status_code == 404: + raise HTTPException( + status_code=status.HTTP_404_NOT_FOUND, + detail="Waveform file not found in storage" + ) + else: + raise HTTPException( + status_code=status.HTTP_502_BAD_GATEWAY, + detail=f"Storage error: {str(e)}" + ) + except Exception as e: + raise HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + detail=f"Failed to fetch waveform: {str(e)}" + ) import json return json.loads(data) @@ -161,7 +231,35 @@ async def stream_version( else: raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="No audio file") - storage = NextcloudClient() - data = await storage.download(file_path) + storage = NextcloudClient.for_member(current_member) + if storage is None: + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail="No storage provider configured for this account" + ) + try: + data = await _download_with_retry(storage, file_path) + except httpx.ConnectError as e: + raise HTTPException( + status_code=status.HTTP_503_SERVICE_UNAVAILABLE, + detail=f"Failed to connect to storage: {str(e)}" + ) + except httpx.HTTPStatusError as e: + if e.response.status_code == 404: + raise HTTPException( + status_code=status.HTTP_404_NOT_FOUND, + detail="File not found in storage" + ) + else: + raise HTTPException( + status_code=status.HTTP_502_BAD_GATEWAY, + detail=f"Storage error: {str(e)}" + ) + except Exception as e: + raise HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + detail=f"Failed to stream file: {str(e)}" + ) + content_type = _AUDIO_CONTENT_TYPES.get(Path(file_path).suffix.lower(), "application/octet-stream") return Response(content=data, media_type=content_type) diff --git a/api/src/rehearsalhub/services/band.py b/api/src/rehearsalhub/services/band.py index ad08348..c2fbb91 100644 --- a/api/src/rehearsalhub/services/band.py +++ b/api/src/rehearsalhub/services/band.py @@ -16,7 +16,7 @@ log = logging.getLogger(__name__) class BandService: def __init__(self, session: AsyncSession, storage: NextcloudClient | None = None) -> None: self._repo = BandRepository(session) - self._storage = storage or NextcloudClient() + self._storage = storage async def create_band( self, diff --git a/api/src/rehearsalhub/services/song.py b/api/src/rehearsalhub/services/song.py index 47c7ed5..db674e7 100644 --- a/api/src/rehearsalhub/services/song.py +++ b/api/src/rehearsalhub/services/song.py @@ -24,7 +24,7 @@ class SongService: self._version_repo = AudioVersionRepository(session) self._session = session self._queue = job_queue or RedisJobQueue(session) - self._storage = storage or NextcloudClient() + self._storage = storage async def create_song( self, band_id: uuid.UUID, data: SongCreate, creator_id: uuid.UUID, band_slug: str, diff --git a/api/src/rehearsalhub/storage/nextcloud.py b/api/src/rehearsalhub/storage/nextcloud.py index fcb4c9e..f54db86 100644 --- a/api/src/rehearsalhub/storage/nextcloud.py +++ b/api/src/rehearsalhub/storage/nextcloud.py @@ -2,6 +2,7 @@ from __future__ import annotations +import logging import xml.etree.ElementTree as ET from typing import Any @@ -10,31 +11,34 @@ import httpx from rehearsalhub.config import get_settings from rehearsalhub.storage.protocol import FileMetadata +logger = logging.getLogger(__name__) + _DAV_NS = "{DAV:}" class NextcloudClient: def __init__( self, - base_url: str | None = None, - username: str | None = None, - password: str | None = None, + base_url: str, + username: str, + password: str, ) -> None: - s = get_settings() - self._base = (base_url or s.nextcloud_url).rstrip("/") - self._auth = (username or s.nextcloud_user, password or s.nextcloud_pass) + if not base_url or not username: + raise ValueError("Nextcloud credentials must be provided explicitly") + self._base = base_url.rstrip("/") + self._auth = (username, password) self._dav_root = f"{self._base}/remote.php/dav/files/{self._auth[0]}" @classmethod - def for_member(cls, member: object) -> "NextcloudClient": - """Return a client using member's personal NC credentials if configured, - falling back to the global env-var credentials.""" + def for_member(cls, member: object) -> "NextcloudClient | None": + """Return a client using member's personal NC credentials if configured. + Returns None if member has no Nextcloud configuration.""" nc_url = getattr(member, "nc_url", None) nc_username = getattr(member, "nc_username", None) nc_password = getattr(member, "nc_password", None) if nc_url and nc_username and nc_password: return cls(base_url=nc_url, username=nc_username, password=nc_password) - return cls() + return None def _client(self) -> httpx.AsyncClient: return httpx.AsyncClient(auth=self._auth, timeout=30.0) @@ -83,10 +87,23 @@ class NextcloudClient: return _parse_propfind_multi(resp.text) async def download(self, path: str) -> bytes: - async with self._client() as c: - resp = await c.get(self._dav_url(path)) - resp.raise_for_status() - return resp.content + logger.debug("Downloading file from Nextcloud: %s", path) + try: + async with self._client() as c: + resp = await c.get(self._dav_url(path)) + resp.raise_for_status() + logger.debug("Successfully downloaded file: %s", path) + return resp.content + except httpx.ConnectError as e: + logger.error("Failed to connect to Nextcloud at %s: %s", self._base, str(e)) + raise + except httpx.HTTPStatusError as e: + logger.error("Nextcloud request failed for %s: %s (status: %d)", + path, str(e), e.response.status_code) + raise + except Exception as e: + logger.error("Unexpected error downloading from Nextcloud: %s", str(e)) + raise async def get_direct_url(self, path: str) -> str: return self._dav_url(path)