diff --git a/app/auth.py b/app/auth.py index d6b2ec39..0e39c95b 100644 --- a/app/auth.py +++ b/app/auth.py @@ -83,11 +83,14 @@ def require_login(func): if not request.session.get("user"): request.session["redirect_after_login"] = str(request.url) return RedirectResponse(url="/login", status_code=status.HTTP_302_FOUND) - # Check if the wrapped function is a coroutine function + # Pass request as a keyword argument so that endpoints whose first + # parameter is a path variable (e.g. pipeline_id) are not accidentally + # bound to the request object when FastAPI supplies all arguments as + # keyword arguments. if inspect.iscoroutinefunction(func): - return await func(request, *args, **kwargs) + return await func(*args, request=request, **kwargs) else: - return func(request, *args, **kwargs) + return func(*args, request=request, **kwargs) return wrapper diff --git a/tests/test_auth.py b/tests/test_auth.py index c16fe318..ccb2dd7c 100644 --- a/tests/test_auth.py +++ b/tests/test_auth.py @@ -1,5 +1,6 @@ """Tests for app/auth.py module.""" +import asyncio import hashlib from unittest.mock import AsyncMock, MagicMock, patch @@ -168,13 +169,54 @@ class TestRequireLogin: mock_request.session = {"user": {"id": "test"}} # Call the decorated sync function - import asyncio - result = asyncio.run(sync_endpoint(request=mock_request, param="test_value")) assert result["message"] == "sync" assert result["param"] == "test_value" + @pytest.mark.asyncio + async def test_path_param_before_request_async(self): + """Regression: endpoints with a path param before request must not get + 'multiple values for argument' when AUTH_ENABLED=True. + + FastAPI passes all resolved parameters as keyword arguments to the + wrapper. The wrapper must forward ``request`` as a keyword argument + too, otherwise the positional ``request`` object would bind to the + first parameter (e.g. ``pipeline_id``) while FastAPI simultaneously + supplies ``pipeline_id`` as a keyword argument → TypeError. + """ + with patch("app.auth.AUTH_ENABLED", True): + from app.auth import require_login + + @require_login + async def endpoint_with_path_param(pipeline_id: int, request: Request, extra: str = ""): + return {"pipeline_id": pipeline_id, "extra": extra} + + mock_request = MagicMock(spec=Request) + mock_request.session = {"user": {"id": "test"}} + + # Simulate how FastAPI calls the wrapper: all args as keyword args. + result = await endpoint_with_path_param(request=mock_request, pipeline_id=42, extra="hello") + + assert result["pipeline_id"] == 42 + assert result["extra"] == "hello" + + def test_path_param_before_request_sync(self): + """Regression: same as above but for synchronous endpoint functions.""" + with patch("app.auth.AUTH_ENABLED", True): + from app.auth import require_login + + @require_login + def sync_endpoint_with_path_param(item_id: int, request: Request): + return {"item_id": item_id} + + mock_request = MagicMock(spec=Request) + mock_request.session = {"user": {"id": "test"}} + + result = asyncio.run(sync_endpoint_with_path_param(request=mock_request, item_id=7)) + + assert result["item_id"] == 7 + @pytest.mark.integration class TestWhoamiEndpoint: