Fix 403 for invited members streaming audio and 500 on invite listing
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 <noreply@anthropic.com>
This commit is contained in:
@@ -1,5 +1,5 @@
|
|||||||
import uuid
|
import uuid
|
||||||
from datetime import datetime
|
from datetime import datetime, timezone
|
||||||
|
|
||||||
from fastapi import APIRouter, Depends, HTTPException, status
|
from fastapi import APIRouter, Depends, HTTPException, status
|
||||||
from sqlalchemy.ext.asyncio import AsyncSession
|
from sqlalchemy.ext.asyncio import AsyncSession
|
||||||
@@ -37,7 +37,7 @@ async def list_invites(
|
|||||||
invites = await repo.get_invites_for_band(band_id)
|
invites = await repo.get_invites_for_band(band_id)
|
||||||
|
|
||||||
# Filter for non-expired invites (optional - could also show expired)
|
# Filter for non-expired invites (optional - could also show expired)
|
||||||
now = datetime.now()
|
now = datetime.now(timezone.utc)
|
||||||
pending_invites = [
|
pending_invites = [
|
||||||
invite for invite in invites
|
invite for invite in invites
|
||||||
if invite.expires_at > now and invite.used_at is None
|
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)
|
# 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:
|
if invite.used_at is not None:
|
||||||
raise HTTPException(
|
raise HTTPException(
|
||||||
status_code=status.HTTP_400_BAD_REQUEST,
|
status_code=status.HTTP_400_BAD_REQUEST,
|
||||||
|
|||||||
@@ -2,7 +2,7 @@
|
|||||||
Invite management endpoints.
|
Invite management endpoints.
|
||||||
"""
|
"""
|
||||||
import uuid
|
import uuid
|
||||||
from datetime import datetime
|
from datetime import datetime, timezone
|
||||||
|
|
||||||
from fastapi import APIRouter, Depends, HTTPException, status
|
from fastapi import APIRouter, Depends, HTTPException, status
|
||||||
from sqlalchemy.ext.asyncio import AsyncSession
|
from sqlalchemy.ext.asyncio import AsyncSession
|
||||||
@@ -32,7 +32,7 @@ async def get_invite_info(
|
|||||||
)
|
)
|
||||||
|
|
||||||
# Check if invite is already used or expired
|
# Check if invite is already used or expired
|
||||||
now = datetime.now()
|
now = datetime.now(timezone.utc)
|
||||||
if invite.used_at is not None:
|
if invite.used_at is not None:
|
||||||
raise HTTPException(
|
raise HTTPException(
|
||||||
status_code=status.HTTP_400_BAD_REQUEST,
|
status_code=status.HTTP_400_BAD_REQUEST,
|
||||||
|
|||||||
@@ -187,7 +187,11 @@ async def get_waveform(
|
|||||||
if not version.waveform_url:
|
if not version.waveform_url:
|
||||||
raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Waveform not ready")
|
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:
|
if storage is None:
|
||||||
raise HTTPException(
|
raise HTTPException(
|
||||||
status_code=status.HTTP_403_FORBIDDEN,
|
status_code=status.HTTP_403_FORBIDDEN,
|
||||||
@@ -229,17 +233,6 @@ async def stream_version(
|
|||||||
):
|
):
|
||||||
version, song = await _get_version_and_assert_band_membership(version_id, session, current_member)
|
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
|
# Prefer HLS playlist if transcoding finished, otherwise serve the raw file
|
||||||
if version.cdn_hls_base:
|
if version.cdn_hls_base:
|
||||||
file_path = f"{version.cdn_hls_base}/playlist.m3u8"
|
file_path = f"{version.cdn_hls_base}/playlist.m3u8"
|
||||||
@@ -248,7 +241,11 @@ async def stream_version(
|
|||||||
else:
|
else:
|
||||||
raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="No audio file")
|
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:
|
if storage is None:
|
||||||
raise HTTPException(
|
raise HTTPException(
|
||||||
status_code=status.HTTP_403_FORBIDDEN,
|
status_code=status.HTTP_403_FORBIDDEN,
|
||||||
|
|||||||
@@ -1,11 +1,12 @@
|
|||||||
"""Integration tests for version streaming endpoints."""
|
"""Integration tests for version streaming endpoints."""
|
||||||
|
|
||||||
import pytest
|
import pytest
|
||||||
|
import uuid
|
||||||
from unittest.mock import AsyncMock, patch, MagicMock
|
from unittest.mock import AsyncMock, patch, MagicMock
|
||||||
import httpx
|
import httpx
|
||||||
|
|
||||||
from rehearsalhub.routers.versions import stream_version, get_waveform
|
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
|
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."""
|
"""Test stream_version endpoint handles connection errors gracefully."""
|
||||||
# Mock dependencies
|
# Mock dependencies
|
||||||
mock_session = MagicMock()
|
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(
|
mock_version = AudioVersion(
|
||||||
id="test-version-id",
|
id="test-version-id",
|
||||||
|
song_id=mock_song.id,
|
||||||
nc_file_path="test/path/file.mp3",
|
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
|
# Mock the storage client to raise connection error
|
||||||
with patch("rehearsalhub.routers.versions.NextcloudClient") as mock_client_class:
|
with patch("rehearsalhub.routers.versions.NextcloudClient") as mock_client_class:
|
||||||
mock_client = MagicMock()
|
mock_client = MagicMock()
|
||||||
mock_client.download = AsyncMock(side_effect=httpx.ConnectError("Connection failed"))
|
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
|
# Mock the membership check
|
||||||
with patch("rehearsalhub.routers.versions._get_version_and_assert_band_membership",
|
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
|
from fastapi import HTTPException
|
||||||
|
|
||||||
@@ -45,7 +49,7 @@ async def test_stream_version_connection_error():
|
|||||||
|
|
||||||
# Should return 503 Service Unavailable
|
# Should return 503 Service Unavailable
|
||||||
assert exc_info.value.status_code == 503
|
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
|
@pytest.mark.asyncio
|
||||||
@@ -54,13 +58,16 @@ async def test_stream_version_file_not_found():
|
|||||||
"""Test stream_version endpoint handles 404 errors gracefully."""
|
"""Test stream_version endpoint handles 404 errors gracefully."""
|
||||||
# Mock dependencies
|
# Mock dependencies
|
||||||
mock_session = MagicMock()
|
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(
|
mock_version = AudioVersion(
|
||||||
id="test-version-id",
|
id="test-version-id",
|
||||||
|
song_id=mock_song.id,
|
||||||
nc_file_path="test/path/file.mp3",
|
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
|
# Mock the storage client to raise 404 error
|
||||||
@@ -75,11 +82,11 @@ async def test_stream_version_file_not_found():
|
|||||||
mock_client.download = AsyncMock(
|
mock_client.download = AsyncMock(
|
||||||
side_effect=httpx.HTTPStatusError("Not found", request=MagicMock(), response=mock_response)
|
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
|
# Mock the membership check
|
||||||
with patch("rehearsalhub.routers.versions._get_version_and_assert_band_membership",
|
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
|
from fastapi import HTTPException
|
||||||
|
|
||||||
@@ -101,24 +108,27 @@ async def test_get_waveform_connection_error():
|
|||||||
"""Test get_waveform endpoint handles connection errors gracefully."""
|
"""Test get_waveform endpoint handles connection errors gracefully."""
|
||||||
# Mock dependencies
|
# Mock dependencies
|
||||||
mock_session = MagicMock()
|
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(
|
mock_version = AudioVersion(
|
||||||
id="test-version-id",
|
id="test-version-id",
|
||||||
|
song_id=mock_song.id,
|
||||||
nc_file_path="test/path/file.mp3",
|
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
|
# Mock the storage client to raise connection error
|
||||||
with patch("rehearsalhub.routers.versions.NextcloudClient") as mock_client_class:
|
with patch("rehearsalhub.routers.versions.NextcloudClient") as mock_client_class:
|
||||||
mock_client = MagicMock()
|
mock_client = MagicMock()
|
||||||
mock_client.download = AsyncMock(side_effect=httpx.ConnectError("Connection failed"))
|
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
|
# Mock the membership check
|
||||||
with patch("rehearsalhub.routers.versions._get_version_and_assert_band_membership",
|
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
|
from fastapi import HTTPException
|
||||||
|
|
||||||
@@ -131,7 +141,7 @@ async def test_get_waveform_connection_error():
|
|||||||
|
|
||||||
# Should return 503 Service Unavailable
|
# Should return 503 Service Unavailable
|
||||||
assert exc_info.value.status_code == 503
|
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
|
@pytest.mark.asyncio
|
||||||
@@ -140,24 +150,27 @@ async def test_stream_version_success():
|
|||||||
"""Test successful streaming when connection works."""
|
"""Test successful streaming when connection works."""
|
||||||
# Mock dependencies
|
# Mock dependencies
|
||||||
mock_session = MagicMock()
|
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(
|
mock_version = AudioVersion(
|
||||||
id="test-version-id",
|
id="test-version-id",
|
||||||
|
song_id=mock_song.id,
|
||||||
nc_file_path="test/path/file.mp3",
|
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
|
# Mock the storage client to return success
|
||||||
with patch("rehearsalhub.routers.versions.NextcloudClient") as mock_client_class:
|
with patch("rehearsalhub.routers.versions.NextcloudClient") as mock_client_class:
|
||||||
mock_client = MagicMock()
|
mock_client = MagicMock()
|
||||||
mock_client.download = AsyncMock(return_value=b"audio_data")
|
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
|
# Mock the membership check
|
||||||
with patch("rehearsalhub.routers.versions._get_version_and_assert_band_membership",
|
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(
|
result = await stream_version(
|
||||||
version_id="test-version-id",
|
version_id="test-version-id",
|
||||||
|
|||||||
Reference in New Issue
Block a user