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.
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user