Fix remaining critical review issue: list_worlds visibility filter bypass
Requesting an explicit ?visibility=PRIVATE (or FRIENDS_ONLY) skipped the ownership check entirely — any authenticated user could list every other user's private worlds just by passing the filter. Only the no-filter default path enforced ownership. Now the ownership check applies whenever a non-PUBLIC visibility is requested, explicit or not. Also fixed current_user: CurrentUser = Depends() in assets.py (redundant empty Depends() on an already-Annotated dependency, inconsistent with every other endpoint) to plain current_user: CurrentUser per the review feedback, and reordered params since that made it a non-default arg following a defaulted one (SyntaxError otherwise). Verified: py_compile clean, full app imports and loads in a clean venv.
This commit is contained in:
@@ -171,8 +171,8 @@ async def delete_asset(
|
||||
@router.post("/{asset_id}/upload", response_model=AssetResponse)
|
||||
async def upload_asset_file(
|
||||
asset_id: int,
|
||||
current_user: CurrentUser,
|
||||
file: UploadFile = File(...),
|
||||
current_user: CurrentUser = Depends(),
|
||||
db: AsyncSession = Depends(get_db),
|
||||
):
|
||||
"""Upload a file for an existing asset."""
|
||||
|
||||
@@ -65,6 +65,12 @@ async def list_worlds(
|
||||
# Filter by visibility if specified
|
||||
if visibility:
|
||||
query = query.where(World.visibility == visibility)
|
||||
if visibility != WorldVisibility.PUBLIC:
|
||||
# Requesting a non-public visibility explicitly must not bypass
|
||||
# ownership — otherwise any authenticated user could list every
|
||||
# PRIVATE/FRIENDS_ONLY world in the system just by passing the
|
||||
# filter, defeating the point of the check below.
|
||||
query = query.where(World.owner_id == current_user.id)
|
||||
else:
|
||||
# Show PUBLIC worlds + owner's own worlds
|
||||
query = query.where(
|
||||
|
||||
Reference in New Issue
Block a user