From 3fa734c6862b62ea3aec8c8f28b057526db82c20 Mon Sep 17 00:00:00 2001 From: Mistral Vibe Date: Wed, 1 Apr 2026 14:03:42 +0200 Subject: [PATCH] Fix 403 for invited members streaming audio and 500 on invite listing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Invited members have no Nextcloud credentials of their own — stream and waveform endpoints now use the file uploader's NC credentials instead of the current member's. Falls back to the current member if uploaded_by is null. The invite listing/info endpoints were comparing timezone-aware expires_at values against naive datetime.now(), causing a TypeError (500). Fixed by using datetime.now(timezone.utc) throughout bands.py and invites.py. Also removes leftover debug logging from versions.py. Co-Authored-By: Claude Sonnet 4.6 --- api/src/rehearsalhub/routers/bands.py | 6 +- api/src/rehearsalhub/routers/invites.py | 4 +- api/src/rehearsalhub/routers/versions.py | 25 ++++---- .../integration/test_versions_streaming.py | 59 +++++++++++-------- 4 files changed, 52 insertions(+), 42 deletions(-) diff --git a/api/src/rehearsalhub/routers/bands.py b/api/src/rehearsalhub/routers/bands.py index 30c4aaf..1e5faa9 100644 --- a/api/src/rehearsalhub/routers/bands.py +++ b/api/src/rehearsalhub/routers/bands.py @@ -1,5 +1,5 @@ import uuid -from datetime import datetime +from datetime import datetime, timezone from fastapi import APIRouter, Depends, HTTPException, status from sqlalchemy.ext.asyncio import AsyncSession @@ -37,7 +37,7 @@ async def list_invites( invites = await repo.get_invites_for_band(band_id) # Filter for non-expired invites (optional - could also show expired) - now = datetime.now() + now = datetime.now(timezone.utc) pending_invites = [ invite for invite in invites if invite.expires_at > now and invite.used_at is None @@ -93,7 +93,7 @@ async def revoke_invite( ) # Check if invite is still pending (not used and not expired) - now = datetime.now() + now = datetime.now(timezone.utc) if invite.used_at is not None: raise HTTPException( status_code=status.HTTP_400_BAD_REQUEST, diff --git a/api/src/rehearsalhub/routers/invites.py b/api/src/rehearsalhub/routers/invites.py index 32b98a2..965f2ea 100644 --- a/api/src/rehearsalhub/routers/invites.py +++ b/api/src/rehearsalhub/routers/invites.py @@ -2,7 +2,7 @@ Invite management endpoints. """ import uuid -from datetime import datetime +from datetime import datetime, timezone from fastapi import APIRouter, Depends, HTTPException, status from sqlalchemy.ext.asyncio import AsyncSession @@ -32,7 +32,7 @@ async def get_invite_info( ) # Check if invite is already used or expired - now = datetime.now() + now = datetime.now(timezone.utc) if invite.used_at is not None: raise HTTPException( status_code=status.HTTP_400_BAD_REQUEST, diff --git a/api/src/rehearsalhub/routers/versions.py b/api/src/rehearsalhub/routers/versions.py index ec45be6..def0dda 100644 --- a/api/src/rehearsalhub/routers/versions.py +++ b/api/src/rehearsalhub/routers/versions.py @@ -186,8 +186,12 @@ 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.for_member(current_member) + + # Use the uploader's NC credentials — invited members may not have NC configured + uploader: Member | None = None + if version.uploaded_by: + uploader = await MemberRepository(session).get_by_id(version.uploaded_by) + storage = NextcloudClient.for_member(uploader) if uploader else NextcloudClient.for_member(current_member) if storage is None: raise HTTPException( status_code=status.HTTP_403_FORBIDDEN, @@ -228,17 +232,6 @@ async def stream_version( current_member: Member = Depends(_member_from_request), ): version, song = await _get_version_and_assert_band_membership(version_id, session, current_member) - - # Import at function level to avoid circular imports - from rehearsalhub.repositories.band import BandRepository - role = await BandRepository(session).get_member_role(song.band_id, current_member.id) - - # Debug logging for permission issues - import logging - log = logging.getLogger(__name__) - log.info(f"User {current_member.id} accessing version {version_id}") - log.info(f"Song band: {song.band_id}") - log.info(f"User role in band: {role if role else 'NOT A MEMBER'}") # Prefer HLS playlist if transcoding finished, otherwise serve the raw file if version.cdn_hls_base: @@ -248,7 +241,11 @@ async def stream_version( else: raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="No audio file") - storage = NextcloudClient.for_member(current_member) + # Use the uploader's NC credentials — invited members may not have NC configured + uploader: Member | None = None + if version.uploaded_by: + uploader = await MemberRepository(session).get_by_id(version.uploaded_by) + storage = NextcloudClient.for_member(uploader) if uploader else NextcloudClient.for_member(current_member) if storage is None: raise HTTPException( status_code=status.HTTP_403_FORBIDDEN, diff --git a/api/tests/integration/test_versions_streaming.py b/api/tests/integration/test_versions_streaming.py index 31abeea..f82dae7 100644 --- a/api/tests/integration/test_versions_streaming.py +++ b/api/tests/integration/test_versions_streaming.py @@ -1,11 +1,12 @@ """Integration tests for version streaming endpoints.""" import pytest +import uuid from unittest.mock import AsyncMock, patch, MagicMock import httpx from rehearsalhub.routers.versions import stream_version, get_waveform -from rehearsalhub.db.models import Member, AudioVersion +from rehearsalhub.db.models import Member, AudioVersion, Song from rehearsalhub.schemas.audio_version import AudioVersionRead @@ -15,24 +16,27 @@ async def test_stream_version_connection_error(): """Test stream_version endpoint handles connection errors gracefully.""" # Mock dependencies mock_session = MagicMock() - mock_member = Member(id=1, name="Test User") + mock_member = Member(id=uuid.uuid4()) - # Mock version with nc_file_path + # Mock song and version + mock_song = Song(id=uuid.uuid4(), band_id=uuid.uuid4()) mock_version = AudioVersion( id="test-version-id", + song_id=mock_song.id, nc_file_path="test/path/file.mp3", - waveform_url="test/path/waveform.json" + waveform_url="test/path/waveform.json", + version_number=1 ) # Mock the storage client to raise connection error with patch("rehearsalhub.routers.versions.NextcloudClient") as mock_client_class: mock_client = MagicMock() mock_client.download = AsyncMock(side_effect=httpx.ConnectError("Connection failed")) - mock_client_class.return_value = mock_client + mock_client_class.for_member.return_value = mock_client # Mock the membership check with patch("rehearsalhub.routers.versions._get_version_and_assert_band_membership", - return_value=(mock_version, None)): + return_value=(mock_version, mock_song)): from fastapi import HTTPException @@ -45,7 +49,7 @@ async def test_stream_version_connection_error(): # Should return 503 Service Unavailable assert exc_info.value.status_code == 503 - assert "Failed to connect to storage" in str(exc_info.value.detail) + assert "Storage service unavailable" in str(exc_info.value.detail) @pytest.mark.asyncio @@ -54,13 +58,16 @@ async def test_stream_version_file_not_found(): """Test stream_version endpoint handles 404 errors gracefully.""" # Mock dependencies mock_session = MagicMock() - mock_member = Member(id=1, name="Test User") + mock_member = Member(id=uuid.uuid4()) - # Mock version with nc_file_path + # Mock song and version + mock_song = Song(id=uuid.uuid4(), band_id=uuid.uuid4()) mock_version = AudioVersion( id="test-version-id", + song_id=mock_song.id, nc_file_path="test/path/file.mp3", - waveform_url="test/path/waveform.json" + waveform_url="test/path/waveform.json", + version_number=1 ) # Mock the storage client to raise 404 error @@ -75,11 +82,11 @@ async def test_stream_version_file_not_found(): mock_client.download = AsyncMock( side_effect=httpx.HTTPStatusError("Not found", request=MagicMock(), response=mock_response) ) - mock_client_class.return_value = mock_client + mock_client_class.for_member.return_value = mock_client # Mock the membership check with patch("rehearsalhub.routers.versions._get_version_and_assert_band_membership", - return_value=(mock_version, None)): + return_value=(mock_version, mock_song)): from fastapi import HTTPException @@ -101,24 +108,27 @@ async def test_get_waveform_connection_error(): """Test get_waveform endpoint handles connection errors gracefully.""" # Mock dependencies mock_session = MagicMock() - mock_member = Member(id=1, name="Test User") + mock_member = Member(id=uuid.uuid4()) - # Mock version with waveform_url + # Mock song and version + mock_song = Song(id=uuid.uuid4(), band_id=uuid.uuid4()) mock_version = AudioVersion( id="test-version-id", + song_id=mock_song.id, nc_file_path="test/path/file.mp3", - waveform_url="test/path/waveform.json" + waveform_url="test/path/waveform.json", + version_number=1 ) # Mock the storage client to raise connection error with patch("rehearsalhub.routers.versions.NextcloudClient") as mock_client_class: mock_client = MagicMock() mock_client.download = AsyncMock(side_effect=httpx.ConnectError("Connection failed")) - mock_client_class.return_value = mock_client + mock_client_class.for_member.return_value = mock_client # Mock the membership check with patch("rehearsalhub.routers.versions._get_version_and_assert_band_membership", - return_value=(mock_version, None)): + return_value=(mock_version, mock_song)): from fastapi import HTTPException @@ -131,7 +141,7 @@ async def test_get_waveform_connection_error(): # Should return 503 Service Unavailable assert exc_info.value.status_code == 503 - assert "Failed to connect to storage" in str(exc_info.value.detail) + assert "Storage service unavailable" in str(exc_info.value.detail) @pytest.mark.asyncio @@ -140,24 +150,27 @@ async def test_stream_version_success(): """Test successful streaming when connection works.""" # Mock dependencies mock_session = MagicMock() - mock_member = Member(id=1, name="Test User") + mock_member = Member(id=uuid.uuid4()) - # Mock version with nc_file_path + # Mock song and version + mock_song = Song(id=uuid.uuid4(), band_id=uuid.uuid4()) mock_version = AudioVersion( id="test-version-id", + song_id=mock_song.id, nc_file_path="test/path/file.mp3", - waveform_url="test/path/waveform.json" + waveform_url="test/path/waveform.json", + version_number=1 ) # Mock the storage client to return success with patch("rehearsalhub.routers.versions.NextcloudClient") as mock_client_class: mock_client = MagicMock() mock_client.download = AsyncMock(return_value=b"audio_data") - mock_client_class.return_value = mock_client + mock_client_class.for_member.return_value = mock_client # Mock the membership check with patch("rehearsalhub.routers.versions._get_version_and_assert_band_membership", - return_value=(mock_version, None)): + return_value=(mock_version, mock_song)): result = await stream_version( version_id="test-version-id",